Doesn't really make sense with the above assert, does it?

Removing those asserts is a bit annoying for the AkonadiFakeData case though... they are convenient to catch programming errors early.

In the case of the cache they can indeed get in the way since we can have a partial picture of the whole data set.

Not sure how to best keep them but suggestions are welcome.

ditto.

That will go away since I push you toward a value based cache. ;-)

Why waste 50 ms here? It's not like we're the FakeJobs trying to simulate latency.

Wrong defines

Is it me or this whole file and the accompanying cpp is a duplication of the fake jobs? Any chance to have only one copy?

Looks like there's a lot of duplication coming from the fakestorage implementation in here. Any chance to avoid copying so much code?

Here lies the rub... The caches (note the plural) will have to be on the live query side somehow otherwise we'll never free un-needed data from the memory.

That also makes me realize that the comment I made on one of the earlier patches about the dependencies was wrong. You don't want to inject CachingStorage everywhere through the dependency manager like that.

What you want instead is to integrate it only to those queries where it matters. So it will likely be on a case by case basis when LiveQueryHelpers ctor is called.

We shouldn't know about the cache here I think...

The cache should be fed from within the caching storage. For instance in the caching storage fetchCollections method you can connect a slot to the job before returning it to feed the cache. It'll be called first before anyone else even tries to access the cache.

This comments holds for all the changes in this file.

Won't be necessary anymore if my other comments are addressed.

ditto.

ditto.

ditto.

ditto.

Well, 50 ms was wasteful, but you probably need to emit after going back to the event loop first still, to keep it async(). Otherwise it's all done at start() time which the caller don't necessarily control and might need opportunity to connect() after start() has been called.

My previous comment still holds.

My comment regarding the Cache ctor on the previous patch will likely have an impact here, several potential solutions though, not sure which one is best I'll let you experiment.

I think this comment still holds as well.

Assuming it would be kept, turning m_cachingStorage into a QScopedPointer would be better.

Space before * not after...

But I'm not sure I see the point of this pointer seeing the code you introduced in the cpp.

Useless.

ditto

ditto

Would be better in the patch introducing MemoryStore. Also add an empty line after it.