Community
Participate
Working Groups
Created attachment 167996 [details] Proposed patch There should be an option to send default parameters specified when the RemoteCallable is registered. Many rest services in particular require sending a standard set of parameters with each call. The attached patch adds a flag that can be set in AbstractClientContainer to always send the default parameters with each request. Any parameters that are specified when the method is called override the defaults, and null values are not sent. The attached example (while not complete since it relies on internal code) shows how this can simplify usage of the rest api, especially for proxy services. I think it is too late for this to be added to the public api for Helios, but will it be possible to make it in as internal code?
Created attachment 167998 [details] Example usage
Created attachment 167999 [details] Test case using the example class
Thanks Cole for this contribution. It's much appreciated. I will take a look at this as soon as I can, but it will realistically be a couple of days as I have some other commitments. I'm not sure about when we will be able to apply it (i.e. whether before Helios or not), but we'll look to do it as soon as possible in whatever form. Again, thanks. Please don't let this go cold...if you get impatient please just post a comment on this bug to that effect to remind me...and again...thanks for contributing to ECF!
I've examined the patch and the example usage...in short the contribution looks fine to me. In addition, I think we should attempt to get this API into Helios...since you obviously need it...and others probably will as well. Given that the API changes are a) *additions* to AbstractClientContainer; b) protected methods, with your/Cole's approval I'm going to propose on ecf-dev mailing list that we get a waiver/exception to our API freeze and include these additions in the Helios release. The risk associated with this addition is very low (it will not break existing clients since it's an addition...and they methods are protected rather than public). So Cole...please give your +1 and I will propose that the committers vote on approving/including this API change in Helios...via the ecf-dev committers as per the ECF ramp-down policy: http://wiki.eclipse.org/ECF_3.0.0/Galileo_Ramp-Down_Policy
The API police wants you to add "@since 4.1" tags to the newly added methods/member. ;-)
+1 Sounds great. I should note that there is one public method: public void setAlwaysSendDefaultParameters(boolean alwaysSendDefaultParameters)
Scott, I just saw Markus's comment, would you like me to create a new patch with his recommendation? Also, since we are modifying the API anyway, should this be added to the interface (IRemoteServiceClientContainerAdapter)? I can create a patch for that as well. Thanks, Cole
(In reply to comment #7) > Scott, > > I just saw Markus's comment, would you like me to create a new patch with his > recommendation? > > Also, since we are modifying the API anyway, should this be added to the > interface (IRemoteServiceClientContainerAdapter)? I can create a patch for that > as well. > > Thanks, > Cole Hi Cole. 'Yes please' to both changes and new patch. I will send email to the ECF committers asking them to vote on the addition of this API (at this > M7 date), and you can prepare the new patch. BTW, before applying the patch to HEAD I will add any needed @since tags...as API tools will help me out here. I assume that the method setAlwaysSendDefaultParameters would be what would be added to IRemoteServiceClientContainerAdapter. If that's wrong let me know. I'll send email to ecf-dev shortly with the vote request and we'll try to get this done as quickly as possible (a couple of days).
(In reply to comment #8) > > Also, since we are modifying the API anyway, should this be added to the > > interface (IRemoteServiceClientContainerAdapter)? I can create a patch for that > > as well. Adding methods to an interface is a breaking API change and requires a major version increment. This is something we definitely do not wanna do after API freeze. ;)
(In reply to comment #9) > (In reply to comment #8) > > > Also, since we are modifying the API anyway, should this be added to the > > > interface (IRemoteServiceClientContainerAdapter)? I can create a patch for that > > > as well. > > Adding methods to an interface is a breaking API change and requires a major > version increment. This is something we definitely do not wanna do after API > freeze. ;) Yeesh, this seems conservative to me. But in any event, Cole please leave *off* the interface addition until later so as not to incur the wrath of the interface police.
Created attachment 168382 [details] Updated patch with javadocs Given the fact that adding to the interface would require a major version increment, I think we should just leave it in the abstract class and wait till next release for the interface. I updated the patch to include javadocs describing the methods and added the @since 4.1 tags.
(In reply to comment #10) > Yeesh, this seems conservative to me. But in any event, Cole please leave *off* > the interface addition until later so as not to incur the wrath of the interface > police. This has nothing to do with conservatism but with the necessity not to break existing implementers/consumers.
(In reply to comment #12) > (In reply to comment #10) > > Yeesh, this seems conservative to me. But in any event, Cole please leave *off* > > the interface addition until later so as not to incur the wrath of the interface > > police. > > This has nothing to do with conservatism but with the necessity not to break > existing implementers/consumers. My point was just that I think it's conservative to have interface additions be treated the same as (e.g.) method removal...in terms of risk of breaking consumers. Although I understand that method addition *can* break API consumers, it seems conservative to me to treat method addition the same as other changes (like method removal).
(In reply to comment #13) > My point was just that I think it's conservative to have interface additions be > treated the same as (e.g.) method removal...in terms of risk of breaking > consumers. Although I understand that method addition *can* break API > consumers, it seems conservative to me to treat method addition the same as > other changes (like method removal). Binary compatibility speaking: Removing an interface method breaks consumers iff they continue to call the method. It does not break implementers. OTOH adding an interface method breaks all existing implementers.
FWIW: Method removal is also flagged as a breaking API change by APITooling.
Bottom line, screw interfaces, embrace abstract classes instead. Much more flexible. ;-)
(In reply to comment #15) > FWIW: Method removal is also flagged as a breaking API change by APITooling. It breaks when java 6 @Override annotation is used on implemented interface method.
The patch 168382 has the approval of majority ECF committers on ecf-dev at eclipse.org. See the following link (along with replies/votes) http://dev.eclipse.org/mhonarc/lists/ecf-dev/msg03662.html Patch applied, examined, tested and released to HEAD. This will be in RC1 build...and subsequent buildes...of ECF 3.3/Helios. Thanks kindly to Cole (and others) for the contribution. Resolving as fixed.
