This repository was archived by the owner on Feb 26, 2023. It is now read-only.
Repository navigation
This repository was archived by the owner on Feb 26, 2023. It is now read-only.
Use a superinterface to define methods on @Rest interfaces #339
Description
Extracted from #207
We currently "detect" some methods on @Rest annotated 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.
Activity
rockytriton commentedon May 23, 2013
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.
DayS commentedon May 24, 2013
I agree with the feature proposed by @pyricau. This could be less confusing.
We currently generate implementation for
setRestTemplate,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 ?
JoanZapata commentedon May 24, 2013
Because he wants to call setRestTemplate on all his @rest services, and they have no interface in common, I guess.
Why would you use more than 1 interface if every generated rest clients implement them all anyway ?
rockytriton commentedon May 24, 2013
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.
rockytriton commentedon May 27, 2013
What do you all think about his.
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.
DayS commentedon Jun 20, 2013
It seems good.
So. We should just create an interface like
EnhancedRestClient(Ixxxname convention for interface is not a good idea) containing all methods handled by AA.rockytriton commentedon Jul 13, 2013
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.
DayS commentedon Jul 13, 2013
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 :
rockytriton commentedon Jul 13, 2013
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).
rockytriton commentedon Jul 13, 2013
Also, don't forget the setErrorHandler (or whatever it should be called) method.
mnesarco commentedon Jul 13, 2013
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:
Frank D. Martínez M.
DayS commentedon Jul 14, 2013
@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 change
rootUrlat 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 :
rockytriton commentedon Jul 14, 2013
I like this idea of logically grouping them together.
DayS commentedon Jul 15, 2013
I keep this one open until we update the wiki.
dodgex commentedon Sep 7, 2013
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?restTemplate.exchangecallssame for
availableCookiesandauthenticationi use 3.0-SNAPSHOT
dodgex commentedon Sep 22, 2013
okay, just found the @RequiresXXX Annotations while trying to "fix" my issue mentioned in the comment...
DayS commentedon Oct 13, 2013
Documentation has been updated. Please let me know if you see mistakes.
dodgex commentedon Oct 14, 2013
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.DayS commentedon Oct 14, 2013
Fixed. Thanks for review 👍