Add generics support on RestTemplate methods #330
Conversation
There was a problem hiding this comment.
You actually need to catch Throwable here, due to a codemodel bug. Basically, at some point com.sun.codemodel.JCodeModel.ref(String) may be called by codeModel.parseType(fullyQualifiedClassName);. This methods tries to load the class, and loading the class may actually throw some Exception and / or Error.
|
Another question, quite tricky :) . I see that the test are passing, which is good. But are you sure the generated code is actually working ? I don't think so. For instance : @Get("/events/{year}/{location}")
List<Event> getEventsGenericsList(String location, int year) throws RestClientException;Generates the following code : @Override
@SuppressWarnings("unchecked")
public List<com.googlecode.androidannotations.test15.rest.Event> getEventsGenericsList(java.lang.String location, int year) {
HashMap<java.lang.String, Object> urlVariables = new HashMap<java.lang.String, Object>();
urlVariables.put("year", year);
urlVariables.put("location", location);
HttpHeaders httpHeaders = new HttpHeaders();
HttpEntity<Object> requestEntity = new HttpEntity<Object>(httpHeaders);
return restTemplate.exchange(rootUrl.concat("/events/{year}/{location}"), HttpMethod.GET, requestEntity, List.class, urlVariables).getBody();
}Appart from the fully qualified name used instead of imports (see my previous comments), there is a part that is really problematic here : The only solution for that is to create a List subclass that is parameterized, because the type erasure doesn't apply for parameterized class. Maybe something like |
|
Well... That's, indeed, a tricky question. I tested my code with simple generics use like List. A solution could be returning a String in exchange() method and then parse it with Jackson, for example. |
…Model methods was needed because this class is final.
|
I haven't completely checked these two commits yet, just did a style comment :) . Regarding your suggestion, it seems like a good idea at first, but then I realized that :
|
|
So... After some tests, using an inner static class which inherit of the generic class works. So, the generated code will be : So we let the serial / deserial mechanism to RestTemple. This solution raise two more questions :
|
|
I was thinking on a dedicated class ( |
|
I thought method local classes would help, but of course they won't because these are inner classes holding references to the outer class so they do not have an empty constructor. You cannot have a dedicated class for holding these classes, because of incremental compilation steps (at least not until we start maintaining a list of the compiled classes). However, one thing we know is that all annotations of a single class will be handled in one step, because the whole class is compiled. So using static inner classes should work, maybe with something as stupid as a counter... |
|
Ok. For now, I just generate theses classes on the same package as the @rest annotated class. Seems to work pretty fine :) |
|
So... I just finish to refactoring code and adding support of generics for rest methods :) I still generated specific classes into the same package as the @rest annotated class. |
|
"This pull request cannot be automatically merged." :) Please merge develop into this branch :) |
…e generics support
|
Fuuuu. There still have a bug with wilcards and typevars... I'm working on it |
|
I think that's ok now. Someone to make a review ? |
|
Now we're talking :) . I'll have a look. |
There was a problem hiding this comment.
Could you extract to a local variable the substring statement at least ?
Thx ;)
There was a problem hiding this comment.
Dead code here. This method is never called, can you delete it ?
|
Good work :) |
|
This pull request should have a milestone set. I'll set it to 3.0, correct me if I'm wrong. |
We still need to import Jackson, but this commit will allow developer to define rest method returning generics like List, Map<String, String>, and so on.