privatize SQLiteCache

[jfirebaugh]

No code that uses DefaultFileSource wants to not use SQLiteCache. We should make the SQLiteCache instance a member of DefaultFileSource, thus enforcing its use, and remove it and the FileCache base class from the public API.

cc @kkaefer

[tmpsantos]

Not sure about this one.

There are use cases like Node that implements its own file source and it is moving towards on using memory backed SQLiteCache for sharing a cache between all the different file sources.

Also we would have to forward interfaces for the cache path and cache size on the DefaultFileSource.

And finally, I would like to experiment with different cache configurations, like stack two SQLiteCache being the first level in-memory and the other in-disk to see if we can get better performance on slow I/O environments.

[kkaefer]

I'm also partial on this one; the idea is to cleanly separate caching and file fetching logic. Removal of that separation will also make it harder to test these components, e.g. because we can no longer replace the cache with a mock object for testing the file source, or test the cache directly.

@tmpsantos The node module uses a separate FileSource altogether, not just another FileCache.

[jfirebaugh]

idea is to cleanly separate caching and file fetching logic

We now have a separate OnlineFileSource class. I think the right separation is to have OnlineFileSource responsible for the fetching logic, with no caching or offline logic, an OfflineFileSource which is responsible for caching / offline, and DefaultFileSource responsible for coordinating one of each. The only public class is DefaultFileSource. It would expose the public core API for caching / offline to the SDK bindings.

[jfirebaugh]

👀 @kkaefer @tmpsantos

Despite all the architecture discussion above, I actually had a very simple change in mind to start out with. WDYT?

[tmpsantos]

Despite all the architecture discussion above, I actually had a very simple change in mind to start out with. WDYT?

Looks good to me but we should handle something like empty path not creating a cache.

[jfirebaugh]

@tmpsantos Do we need that feature anywhere? If not, let's leave it out.

[tmpsantos]

@tmpsantos Do we need that feature anywhere? If not, let's leave it out.

Not that I'm aware of.