New repositories approach

[Serchinastico]

This is my repositories approach, it doesn't bring anything new to the table, it's just a clean up and a better segregation of interfaces, to wrap up:

Repository

As in the old implementation, if you want to create your repositories just extend from this class. To configure your data sources, call addReadables, addWriteables, addCaches in your constructor:

  public SampleRepository(SampleReadable sampleReadable, SampleWriteable sampleWriteable,
      SampleInMemoryCache sampleInMemoryCache) {
    addReadables(sampleReadable);
    addWriteables(sampleWriteable);
    addCaches(sampleInMemoryCache);
  }

To do special stuff, like reading only from cache, reading only fresh data, etc, there is a method with a ReadPolicy that let's you define those special cases.

Readable, Writeable, Cache

This is the separation of the previous DataSource interface. Readable has get and getAll methods, Writeable has addOrUpdate, addOrUpdateAll, delete and deleteAll methods. Finally, Cache just merges these two interfaces. There is still the problem of having to implement methods you might not need (I might only need to implement get and don't need to implement getAll). You can use one of the EmptyReadable, EmptyWriteable and EmptyCache classes which provides an empty implementation for every method.

I did an MVP with an implementation based on annotations where data sources only need to annotate it's get method with a @Get annotation. If the annotation is not present then the repository goes to the following data source. I've done some profiling and the annotation-based implementation is 20 times slower, we are talking about repository calls of 7ms which I think it's not acceptable. We might use apt but it will take us longer.

Pagination is solved in the same fashion as in the previous implementation.

Let me know the parts you like, the parts you don't, and let's close the repositories.

[pedrovgs]

@davideme suggest us the usage of Hashable as class name and getId?

[pedrovgs]

After 2 hours talking about this ;) We've decided that this interface is not needed and we can use hashCode and equals method.

[pedrovgs]

And 30 mins later we've remembered why Cacheable interface was needed.

[davideme]

As discussed IRL, the goal of K getKey(); is to be able to cache results of a getAll and be accesible for the method getById.

[pedrovgs]

@Serchinastico After a first review the only points I think we should review are:

• Naming used for interfaces. IMHO these interfaces should be declared using DataSource as suffix. We should also add Rosie as Repository prefix.
• Concurrency handling. The current implementation does not handle concurrency. We have two options:
  ** Handle concurrency inside the repository.
  ** Leave concurrency in the data source implementation hand. This is the current implementation.
• Error handling. The current error handling is based on throw Exception. I don't like this approach but I guess this is the only option we have if we don't want to introduce Either in the project.
• Invalidation process. If I'm not wrong reading this PR, I don't see any code related to cache invalidation. We should review this.

I'm going to continue reading the PR, but these are the most important details we should review. IMHO the approach is quite good like to continue with the development.

[Serchinastico]

• That's ok, I didn't call it RosieRepository to avoid collisions with the old one, but eventually it will be called just as it was.
• I'd rather let concurrency handling as a future improvement, honestly, I didn't realize we needed it for our current Rosie project, my bad!
• I don't actually like the exception-guided handling either but I guess is the Javier way to do this stuff. I have not seen an Either implementation in Java yet, so developers might not be used to it and can be seen as harder to use.
• I didn't add it thinking that CacheDataSource implementations should do that by themselves when returning objects. First time I saw the isValid method I didn't know what was its use. I'm ok with adding it in the CacheDataSource interface if we really need it and use it in the Repository base class as we were doing until now.

[davideme]

@Serchinastico This implementation is similar to mine in iOS so except the remarks from @pedrovgs everything seems fine ;)

[flipper83]

I don't know if it's out of scope of this pull request, but I like have a T implementation of the datasources for Memory and for realm and have in other proyect, but we don't need to create that kind of data source every time. Here we have a memory implementation on the sample, but we can move it out to other proyect and reuse for any kind of T data, and make extendible for add new user interfaces.

[Serchinastico]

I have updated the PR with all your requests, including:

• Replaced old repositories with new ones (rename of Repository to RosieRepository included)
• Add DataSource suffix to data sources
• Add isValid method to caches to clean old stored values
• Add generic in-memory cache implementations
• Add tests

[pedrovgs]

Can ReadPolicy.POPULATE_CACHE be used without any other policy?. As far as I read we should not create a USE_READABLE_POPULATE_CACHE because if we do that we should also add another policies with the same meaning. Maybe could be enough just checking the policies set contains at least another politcy.

[Serchinastico]

I understand what you are saying, basically CACHE_POLICY & READ_POLICY are two different things and some combinations doesn't make any sense. We can hide it in another class and let only the values that are correct visible. I'll think about it

[pedrovgs]

We can create an issue to implement this in the future if you want ;)

[pedrovgs]

Awesome code @Serchinastico review my comments and you already have my 👍

[pedrovgs]

Why don't merge this PR and #17

[flipper83]

I reviewed this PR but I didn't put my 👍 sorry