ObjectiveDDP icon indicating copy to clipboard operation
ObjectiveDDP copied to clipboard

Use protocol for collections management rather than in-memory collection.

Open jleach opened this issue 10 years ago • 9 comments

Hi,

I was thinking it would be nice to remove the in-memory collection storage and replace it will a protocol so it can easily be replaced with CoreData, SQLite, Plist, whatever. If this seems like a change you're interested in perhaps I can scrounge up the time to help implement it; I'd like like to know it would be integrated into master at some point.

The default implementation for a data manager can be the current in-memory collection.

jleach avatar Aug 16 '15 03:08 jleach

Hi @jleach Yes I would be interested in this, I've been meaning to find time to do a few things to this project but kids keep me busy so any help from you would be greatly appreciated!

mikey0000 avatar Aug 16 '15 04:08 mikey0000

I'm interested in this as well. I am using core data and just using the [publication]_added,[publication]_changed, etc. notifications to subscribe to the data. I have some large collections, and storing them all in-memory is not feasible. At the very least, the in-memory storage should be optional. I can help with design and implementation on this as well.

nsolter avatar Aug 21 '15 17:08 nsolter

I'd be interested in this as well. I have a hacked up version of the meteorClient that removed the in-memory storage and uses a protocol to pump the changes to the delegate.

ubercolin avatar Aug 21 '15 23:08 ubercolin

I'll take a look this weekend to see how much effort is required to refactor. When I have a plan I'll (a) propose the protocol here for comments and (b) fork for the work that can be merged back into the main repo.

jleach avatar Aug 27 '15 22:08 jleach

@jleach sounds great!

mikey0000 avatar Aug 27 '15 22:08 mikey0000

@mikey0000 I was thinking that for the initial implementation I can just implement it so that if a delegate exists it uses it, otherwise it uses the default behaviour (in-memory collections). Here is a protocol to start the discussion: https://gist.github.com/jleach/03053aa74f519c936bd1

After this is done and out in the wild the next step can build an in-memory class that implements the current collection mechanics but is not part of the meteor client.

jleach avatar Sep 01 '15 20:09 jleach

One thing to keep in mind is that, without the in-memory collections, ObjectiveDDP won't be able to generate a complete object for a changed message like it is currently doing (see _parseObjectAndUpdateCollection). This means that the clients of ObjectiveDDP will need to explicitly handle the "fields" and "cleared" hashes (described in the spec: https://github.com/meteor/meteor/blob/devel/packages/ddp/DDP.md). Thus, a protocol for a cleared message probably might want to have explicit fields and cleared objects, or at least make it clear (no pun intended) that the dictionary you're getting for a changed message has nested fields and cleared dictionaries.

I have an ObjectiveDDP branch here that optionally disables in-memory collections: https://github.com/emmerge/ObjectiveDDP/compare/upstream_shadow...optional_inmemory_collections Note that I took the approach of having a changed message return an object of a different structure than the added message. Specifically, the changed object has no actual fields at the top-level other than _id Instead it has top-level hashes called "fields" and "cleared".

This turns out to be somewhat annoying to handle in the client, but the trade-off is worth it in my opinion to avoid the memory overhead of the in-memory collections.

nsolter avatar Sep 02 '15 10:09 nsolter

@jleach - I think the protocol looks good and is pretty similar to what I implemented.

Regarding @nsolter's note on updates, I think you could either do as Nick suggests and have two sets of fields, so your protocol method becomes something like:

- (void)meteorClient:(MeteorClient *)client didChangeObjectWithId:(NSString *)objectId collectionName:(NSString *)name fields:(NSDictionary *)fields removedFields:(NSArray *)removedFields;

or simply keep one dictionary of fields that merges the two sets together, and use an NSNull as the value for the removed fields. You'd obviously have to then know in the delegate to handle the NSNull, but might be cleaner?

ubercolin avatar Sep 02 '15 18:09 ubercolin

I'd lean toward two explicit sets of fields, but I'm fine with either approach.

nsolter avatar Sep 03 '15 07:09 nsolter