Repository navigation
Use a superinterface to define methods on @Rest interfaces #339
Description
Activity
I definitely agree with this one, I'm running into the issue now. I have a shared method which will set the authentication on my rest services, but I need to pass in an Object and use reflection to find the methods instead of just using an interface.
I agree with the feature proposed by @pyricau. This could be less confusing.
We currently generate implementation forsetRestTemplate,getRestTemplateand alsosetRootUrl. Should we create an interface for restTemplate accessors and another for rootUrl accessors or a master inteface with all enhanced methods ?@rockytriton > I don't really understand your use case. Why do you need reflection ?
Why do you need reflection ?
Because he wants to call setRestTemplate on all his @rest services, and they have no interface in common, I guess.
We currently generate implementation for setRestTemplate, getRestTemplate and also setRootUrl. Should we create an interface for restTemplate accessors and another for rootUrl accessors or a master inteface with all enhanced methods ?
Why would you use more than 1 interface if every generated rest clients implement them all anyway ?
I have an app that uses several different REST services. I have a method which sets the basic authentication on them all and also a method which changes the service URL (you can configure which server to point to). I would like to pass the service object to these methods using more than one service class, hence why I would use an interface that they all implement.
What do you all think about his.
- Create an interface, something like IRestClient which is the only interface that can be extended by any @rest annotated interface.
- This interface has all 4 methods (get/setRootUrl and get/setRestTemplate).
- Every generated rest client generates all 4 methods whether or not the IRestClient is extended on the @rest annotated interface.
- Extending the IRestClient interface is optional, but as stated above, the methods will be there anyway.
This will greatly simplify the validation code for one thing. We would simply not allow any non-annotated methods. Unless of course you wanted to have backward compatibility but with all of the package name changes, this would be the time to redo things since they would have to edit all of their code anyway.
Another option would also be to add another interface IBasicAuth which would have methods for setting user/pass when using basic authentication. Just a simpler way to add the auth code, it could be implemented in the generated code as well.
It seems good.
So. We should just create an interface likeEnhancedRestClient(Ixxxname convention for interface is not a good idea) containing all methods handled by AA.I agree with you on the Ixxx naming convention, but it's really just a matter of preference, I've worked at many clients over the years where they feel very strongly one way or the other, but really it just comes down to preferences and how they do their naming standards in the organization. I like the name you came up with.
What do you think about making the errorHandler method you mentioned in #623 a part of this interface as well? Or did you think it would be better as a separate interface? Personally I think it makes sense to have it all in the same interface.
What do you think about set/getErrorHandler methods instead? There's also the option to have an addErrorHandler so that it supports multiple handlers and to be more consistant with many java API classes, though I don't think it would really be necessary to support multiple but who knows what other people may need.
Since #579 has been merged we have 5 more specific methods to handle. I think it's time to categorize them and create multiple interface.
Here is a proposal for the full list of methods :
- EnhancedRestClient
- getRestTemplate()
- setRestTemplate(RestTemplate restTemplate);
- getRootUrl()
- setRootUrl(String rootUrl)
- HeadersRestClient
- getCookie(String name)
- setCookie(String name, String value)
- getHeader(String name)
- setHeader(String name, String value)
- setAuthentication(HttpAuthentication value)
- setHttpBasicAuth(String username, String password)
- EnhancedRestClient
I was thinking about this too, but I couldn't figure out two different interfaces that made enough sense. Really the restTemplate and rootUrl don't have anything in common. I suppose that the cookie/header/auth do have the fact that they are all related to http headers in common.
I think it would make sense to either put them all in a single "Enhanced" interface or to also split the restTemplate and rootUrl into their own interface. Personally I think it's fine to just put them all in a single interface, it's not like adding the code that may or may not be used would have any detrimental effect on performance or anything like that. Really all the methods have in common that they represent an enhanced interface to the rest client.
Either way though I'm fine with separating them out too, just let me know what you think. I think I already have something working well with these changes, I just need to add more comments and some unit tests. Also I did a little bit of refactoring to the RestProcessor code to increase re-use and make smaller methods (I hope that's ok).
Also, don't forget the setErrorHandler (or whatever it should be called) method.
What do you think about this:
One single interface:
interface RestClientSupportHook {
RestClientSupport getRestClientSupport();
}One single abstract class:
abstract class RestClientSupport {
public abstract getRestTemplate();
public abstract setRestTemplate(RestTemplate restTemplate);
public abstract getRootUrl();
public abstract setRootUrl(String rootUrl);
public abstract getCookie(String name);
public abstract setCookie(String name, String value);
public abstract getHeader(String name);
public abstract setHeader(String name, String value);
public abstract setAuthentication(HttpAuthentication value);
public abstract setHttpBasicAuth(String username, String password);
}Then, the generated client class should extends the Abstract class, and
if the user wants access to those methods, just extend the interface:@rest
interface MyRestClient extends RestClientSupportHook { ... }Generates:
class MyRestClient_ extends RestClientSupport implements MyRestClient {
...
public RestClientSupport getRestClientSupport() { return this; }
..
}And finally can be used as:
@restservice
MyRestClient service;service.getRestClientSupport().setRootUrl(....);
On Sat, Jul 13, 2013 at 1:10 PM, Rocky D. Pulley
notifications@github.comwrote:Also, don't forget the setErrorHandler (or whatever it should be called)
method.—
Reply to this email directly or view it on GitHubhttps://github.com//issues/339#issuecomment-20923938
.Frank D. Martínez M.
@mnesarco > I think it's just overkill to do it that way. We could just let the user implement the interface and AA will generate the proper code.
@rockytriton > Yep. I forgot to add
setErrorHandler, thanks :)
Until now AA was generated only the necessary code. I think we should keep this in mind. For example, if I just want to changerootUrlat runtime I probably don't want AA to generate all the stuffs about headers, authentication, error handling (plus this method could changes the behavior of the code, depending of the decision in #623).
Plus it's common to make interfaces for just a few methods (ie: listeners).With that in mind I propose this :
- RestClientSupport
- getRestTemplate()
- setRestTemplate(RestTemplate restTemplate)
- RestClientRootUrl
- getRootUrl()
- setRootUrl(String rootUrl)
- RestClientHeaders
- getCookie(String name)
- setCookie(String name, String value)
- getHeader(String name)
- setHeader(String name, String value)
- setAuthentication(HttpAuthentication value)
- setHttpBasicAuth(String username, String password)
- RestClientErrorHandling
- setErrorHandler(RestErrorHandler errorHandler)
- RestClientSupport
I like this idea of logically grouping them together.
I keep this one open until we update the wiki.
Is there some Magic required to get my headers i set with
RestClientHeaders.setHeadersent to the server?if i look at the generated code, there is a
java.util.HashMap<String, String> availableHeadersbut beside that it is instanciated in the constructor and is made accessible throu get/setHeader, there is no use of it?- it is never assigned to the resttemplate
- it is never used in any of the
restTemplate.exchangecalls - it is never used at all (beside the constructor and set/get methods)
same for
availableCookiesandauthenticationi use 3.0-SNAPSHOT
okay, just found the @RequiresXXX Annotations while trying to "fix" my issue mentioned in the comment...
Documentation has been updated. Please let me know if you see mistakes.
I think the
String setHeader(String name)should beString getHeader(String name)
In this example your code should call
setHeader("myHeader", "My Value")before callinggetEvent(…). AA will is also handleString setHeader(String name)method to let you retrieve a specific header's value.Fixed. Thanks for review 👍
Extracted from #207
We currently "detect" some methods on
@Restannotated interfaces, and implement them if needed.Instead of that, we should force the users to let their interface extend another interface that has these
setRestTemplate()etc methods defined, and always implement it in the generated class.