Skip to content
This repository was archived by the owner on Apr 17, 2019. It is now read-only.
This repository was archived by the owner on Apr 17, 2019. It is now read-only.

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

Open

Description

@jleach

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.

Activity

  1. changed the title [-]Remove In-Memory Collection[/-] [+]Use protocol for collections management rather than in-memory collection.[/+] on Aug 16, 2015
  2. mikey0000 commented on Aug 16, 2015

    @mikey0000
    Collaborator

    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!

  3. nsolter commented on Aug 21, 2015

    @nsolter
    Contributor

    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.

  4. ubercolin commented on Aug 21, 2015

    @ubercolin

    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.

  5. jleach commented on Aug 27, 2015

    @jleach
    Author

    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.

  6. mikey0000 commented on Aug 27, 2015

    @mikey0000
    Collaborator

    @jleach sounds great!

  7. jleach commented on Sep 1, 2015

    @jleach
    Author

    @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.

  8. nsolter commented on Sep 2, 2015

    @nsolter
    Contributor

    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: emmerge/ObjectiveDDP@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.

  9. ubercolin commented on Sep 2, 2015

    @ubercolin

    @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?

  10. nsolter commented on Sep 3, 2015

    @nsolter
    Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions