stephanenicolas / robospice

Repo of the Open Source Android library : RoboSpice. RoboSpice is a modular android library that makes writing asynchronous long running tasks easy. It is specialized in network requests, supports caching and offers REST requests out-of-the box using extension modules.
Apache License 2.0
2.95k stars 545 forks source link

Cache does not manage generics for JSON parsing #329

Open jorgevila opened 10 years ago

jorgevila commented 10 years ago

Hi Stephane,

let me try to describe our problem.

In our project, we use generics in order to represent the objects model for result types. For example, we have:

Container<ItemsList<ItemA>>
Container<ItemsList<ItemB>>

as possible objects received in JSON, which we parse with Jackson, using TypeReference ([http://svn.codehaus.org/jackson/trunk/src/java/org/codehaus/jackson/type/TypeReference.java]) and paseAndClose method with type so Jackson is able to parse generics and find the proper class for creating the objects

 *public Object parseAndClose(Reader reader, Type dataType)*
 *[http://javadoc.google-http-java-client.googlecode.com/hg/1.18.0-rc/index.html]*

So, for loadDataFromNetwork, we store in our implementation of each SpiceRequest the type to parse and use it for that.

Now, for cache, the PersisterFactory receives only a Class to get the Persister Object in order to parse, so we can not have the generics information in order to parse the JSON information stored on Cache, as the Class loses the Generics info due to type erasure.

Therefore, I made a try to make robospice accept Type on SpiceRequest, trying to keep retro-compatibility but it results on a little mess, mixing Class and TypeReference and more tests fail. You can see it here: [https://github.com/jorgevila/robospice/tree/cache-generics]

This branch tries to use the Jackson codehouse TypeReference concept to keep generics all through the flow, so we could have the generics information in the corresponding persister. This branch breaks a lot of tests, and I am not sure it is a good idea.

So, from my point of view, there is something that could help on these issues. Currently, the SpiceRequest has an abstract method loadDataFromNetwork. One idea would be that we could have loadDataFromCache and saveDataToCacheAndReturnData methods implemented as well by the SpiceRequests objects. But this would break the CacheManager funcionalities like getAllCacheKeys, etc.

In case it is not a good idea, instead of having a ObjectPersisterFactory, we could have the request to provide its own persister to the CacheManager. (This idea is the one I like the most)

You can see an example of this here: [https://github.com/jorgevila/robospice/tree/cache_generics_persister_on_request]

(I was not so brave to propose a PR yet ^_^)

Currently we are evaluating to use this last modification, as for now, we are using another approach but not a clear one. Taking advantage of the type erasure, where we make a little trick: asking the ObjectPersisterFactory for a

ObjectPersister<SpiceRequestClass>

, it is returning an

ObjectPersister<ResultTypeClass>   --> type stored in the SpiceRequestClass as we already had

, taking that with the type erasure the generic type is lost, the final ObjectPersister instance returned retrieves the data type T that DefaultRequestRunner is expecting.

Hope I have explained our concerns properly. I will be glad if you can share your thoughts on it.

Thank you

stephanenicolas commented 10 years ago

Sorry for the delay @jorgevila . This bug looks really interesting but I don't have much time to tackle it for now. I am on some hot new OSS stuff.

If you could come up with a PR + tests, that would be really great.

nateridderman commented 9 years ago

Thank you for posting this bug. I ran into the same issue.