New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deep merging of documents #998

Closed
mitar opened this Issue Apr 27, 2013 · 16 comments

Comments

Projects
None yet
8 participants
@mitar
Collaborator

mitar commented Apr 27, 2013

Currently Meteor allows partially publishing and document which you can then merge when you publish additional fields. If I understand correctly from the documentation (where this feature is documented just in one comment to the code, I may add) this works only on top-level fields. Are there any plans to add support also for nested fields? And even partially published lists?

In fact, documentation is a bit confusing on this. So both partial and full document publish issue an "added" call. In one part, documentation is saying that in such case when there are multiple sources of the same document which document "wins" is arbitrary. And then in account package it is written in a code comment that fields are merged.

@n1mmy

This comment has been minimized.

Show comment
Hide comment
@n1mmy

n1mmy May 6, 2013

Member

Yeah, right now only top-level fields are merged. This is a kinda surprising and annoying behavior in some cases (eg, accounts). It also related to the issue where the whole top-level subfield is sent, even if only a small part of it changed.

We want to figure out how to add this to DDP without making DDP very mongo specific, eg using dots for subfields.

Member

n1mmy commented May 6, 2013

Yeah, right now only top-level fields are merged. This is a kinda surprising and annoying behavior in some cases (eg, accounts). It also related to the issue where the whole top-level subfield is sent, even if only a small part of it changed.

We want to figure out how to add this to DDP without making DDP very mongo specific, eg using dots for subfields.

@mitar

This comment has been minimized.

Show comment
Hide comment
@mitar

mitar May 6, 2013

Collaborator

I think the main point is that there is no documentation about this really. Can you at least document this for now, so it is clear what is current behavior and caveats?

DerbyJS defines paths which they argue are mongo agnostic. You could also have something like that and merge things path-specific?

Collaborator

mitar commented May 6, 2013

I think the main point is that there is no documentation about this really. Can you at least document this for now, so it is clear what is current behavior and caveats?

DerbyJS defines paths which they argue are mongo agnostic. You could also have something like that and merge things path-specific?

@mizzao

This comment has been minimized.

Show comment
Hide comment
@mizzao

mizzao Mar 14, 2014

Contributor

Yikes, I just spent two days debugging what I thought was a bug in my app / my smart packages / collection-hooks. Did we ever get around to documenting this?

The non-deterministic behavior a la - you will get the fields from some publication at random, probably the one that went first - really wreaks havoc.

Contributor

mizzao commented Mar 14, 2014

Yikes, I just spent two days debugging what I thought was a bug in my app / my smart packages / collection-hooks. Did we ever get around to documenting this?

The non-deterministic behavior a la - you will get the fields from some publication at random, probably the one that went first - really wreaks havoc.

@glasser

This comment has been minimized.

Show comment
Hide comment
@glasser

glasser Mar 19, 2014

Member

It's documented at http://docs.meteor.com/#meteor_subscribe

"If more than one subscription sends conflicting values for a field (same collection name, document ID, and field name), then the value on the client will be one of the published values, chosen arbitrarily."

Member

glasser commented Mar 19, 2014

It's documented at http://docs.meteor.com/#meteor_subscribe

"If more than one subscription sends conflicting values for a field (same collection name, document ID, and field name), then the value on the client will be one of the published values, chosen arbitrarily."

@glasser glasser closed this Mar 19, 2014

@mizzao

This comment has been minimized.

Show comment
Hide comment
@mizzao

mizzao Mar 19, 2014

Contributor

@glasser: I think @mitar's point was that the single sentence referring to this in the documentation is apt to be overlooked and isn't what the user would expect given the merging of top-level fields.

I don't think this issue should be closed. It's something that would be great to have fixed at some point, per @n1mmy's comment above.

Contributor

mizzao commented Mar 19, 2014

@glasser: I think @mitar's point was that the single sentence referring to this in the documentation is apt to be overlooked and isn't what the user would expect given the merging of top-level fields.

I don't think this issue should be closed. It's something that would be great to have fixed at some point, per @n1mmy's comment above.

@mitar

This comment has been minimized.

Show comment
Hide comment
@mitar

mitar Mar 19, 2014

Collaborator

So the issue is the lack of documentation that fields are merged.

Collaborator

mitar commented Mar 19, 2014

So the issue is the lack of documentation that fields are merged.

@mizzao

This comment has been minimized.

Show comment
Hide comment
@mizzao

mizzao Mar 19, 2014

Contributor

@mitar is it just documentation, or is it what @n1mmy said:

This is a kinda surprising and annoying behavior in some cases (eg, accounts). It also related to the issue where the whole top-level subfield is sent, even if only a small part of it changed.

We want to figure out how to add this to DDP without making DDP very mongo specific, eg using dots for subfields.

Contributor

mizzao commented Mar 19, 2014

@mitar is it just documentation, or is it what @n1mmy said:

This is a kinda surprising and annoying behavior in some cases (eg, accounts). It also related to the issue where the whole top-level subfield is sent, even if only a small part of it changed.

We want to figure out how to add this to DDP without making DDP very mongo specific, eg using dots for subfields.

@mitar

This comment has been minimized.

Show comment
Hide comment
@mitar

mitar Mar 19, 2014

Collaborator

The ticket is about the lack of documentation (which is a bug). Feature request (what @n1mmy is talking about) should not be on GitHub per Meteor ticket policy.

Collaborator

mitar commented Mar 19, 2014

The ticket is about the lack of documentation (which is a bug). Feature request (what @n1mmy is talking about) should not be on GitHub per Meteor ticket policy.

@mitar

This comment has been minimized.

Show comment
Hide comment
@mitar

mitar Mar 19, 2014

Collaborator

For discussion of merging, you should check here.

Collaborator

mitar commented Mar 19, 2014

For discussion of merging, you should check here.

@mizzao

This comment has been minimized.

Show comment
Hide comment
@mizzao

mizzao Mar 19, 2014

Contributor

@mitar that explains it. Somehow the "Meteor ticket policy" changed without me having noticed. 💃

Do pull requests still count as feature requests?

Contributor

mizzao commented Mar 19, 2014

@mitar that explains it. Somehow the "Meteor ticket policy" changed without me having noticed. 💃

Do pull requests still count as feature requests?

@mizzao

This comment has been minimized.

Show comment
Hide comment
@mizzao

mizzao Jul 16, 2014

Contributor

I just saw another manifestation of this on a StackOverflow question: http://stackoverflow.com/q/24780981/586086.

Wouldn't it be nice to have the client print out a warning message when merging fields that are objects, and those fields don't have the same keys? For example, if the client chose profile: { foo: ... } over profile: { foo: ..., bar: ... }, it should log in the console that it's dropping the bar field, that this can be non-deterministic, and that the developer should check their server-side code for intended behavior.

I really don't think the tiny sentence at the end of the Meteor.subscribe documentation explains this in a place where people will see it.

Contributor

mizzao commented Jul 16, 2014

I just saw another manifestation of this on a StackOverflow question: http://stackoverflow.com/q/24780981/586086.

Wouldn't it be nice to have the client print out a warning message when merging fields that are objects, and those fields don't have the same keys? For example, if the client chose profile: { foo: ... } over profile: { foo: ..., bar: ... }, it should log in the console that it's dropping the bar field, that this can be non-deterministic, and that the developer should check their server-side code for intended behavior.

I really don't think the tiny sentence at the end of the Meteor.subscribe documentation explains this in a place where people will see it.

@alanning

This comment has been minimized.

Show comment
Hide comment
@alanning

alanning Sep 4, 2014

Contributor

Would a PR for explaining this further and describing a workaround in the #meteor_subscribe documentation be considered / appreciated?

Something along the lines of:

Important Note:

If more than one subscription sends conflicting values for a field (same collection name, document ID, and field name), then the value on the client will be one of the published values, chosen arbitrarily.

Imagine publishing user profiles (from the Accounts package) in two separate publish functions: one for regular users, and one for admins who should see extra profile fields. The behavior of which fields get sent to the admin client will be arbitrary because only one of the publish functions can "win" (due to how DDP currently works).

The workaround is to publish all user profile fields appropriate for the current user's permission level in one publish function. This works because the top-level "profile" field will only be sent once with the appropriate sub-fields already included.

Contributor

alanning commented Sep 4, 2014

Would a PR for explaining this further and describing a workaround in the #meteor_subscribe documentation be considered / appreciated?

Something along the lines of:

Important Note:

If more than one subscription sends conflicting values for a field (same collection name, document ID, and field name), then the value on the client will be one of the published values, chosen arbitrarily.

Imagine publishing user profiles (from the Accounts package) in two separate publish functions: one for regular users, and one for admins who should see extra profile fields. The behavior of which fields get sent to the admin client will be arbitrary because only one of the publish functions can "win" (due to how DDP currently works).

The workaround is to publish all user profile fields appropriate for the current user's permission level in one publish function. This works because the top-level "profile" field will only be sent once with the appropriate sub-fields already included.

@Crenshinibon

This comment has been minimized.

Show comment
Hide comment
@Crenshinibon

Crenshinibon Oct 24, 2014

Another solution to this problem - mentioned in Discover Meteor - is to publish specific sets of information to different (client only) collections:

#the original client and server collection with all data
@ProposalVersions = new Meteor.Collection 'proposal-versions'

if Meteor.isClient
    #the client side only collection holding only the titles
    @MostRecentTitles = new Meteor.Collection 'most-recent-titles'


if Meteor.isServer
   #the standard publication for all data
    Meteor.publish 'proposal-versions', (pId) ->
        ProposalVersions.find {proposal: pId}

    #the special publication for only the titles
    Meteor.publish 'most-recent-titles', (pId) ->
        sub = @
        cur = ProposalVersions.find {proposal: pId},
            fields: {'data.title': 1, proposal: 1, versionNumber: 1}
            sort: {versionNumber: -1}
            limit: 1

        Mongo.Collection._publishCursor(cur, sub, 'most-recent-titles');
        sub.ready()   

Crenshinibon commented Oct 24, 2014

Another solution to this problem - mentioned in Discover Meteor - is to publish specific sets of information to different (client only) collections:

#the original client and server collection with all data
@ProposalVersions = new Meteor.Collection 'proposal-versions'

if Meteor.isClient
    #the client side only collection holding only the titles
    @MostRecentTitles = new Meteor.Collection 'most-recent-titles'


if Meteor.isServer
   #the standard publication for all data
    Meteor.publish 'proposal-versions', (pId) ->
        ProposalVersions.find {proposal: pId}

    #the special publication for only the titles
    Meteor.publish 'most-recent-titles', (pId) ->
        sub = @
        cur = ProposalVersions.find {proposal: pId},
            fields: {'data.title': 1, proposal: 1, versionNumber: 1}
            sort: {versionNumber: -1}
            limit: 1

        Mongo.Collection._publishCursor(cur, sub, 'most-recent-titles');
        sub.ready()   
@MaximDubrovin

This comment has been minimized.

Show comment
Hide comment
@MaximDubrovin

MaximDubrovin Dec 28, 2014

Contributor

«Workaround for Meteor DDP limitation and client part of Meteor.subscribe() nuance if you want to subscribe for more nested fields of already received documents»

https://medium.com/@MaxDubrovin/workaround-for-meteor-limitations-if-you-want-to-sub-for-more-nested-fields-of-already-received-docs-eb3fdbfe4e07

Contributor

MaximDubrovin commented Dec 28, 2014

«Workaround for Meteor DDP limitation and client part of Meteor.subscribe() nuance if you want to subscribe for more nested fields of already received documents»

https://medium.com/@MaxDubrovin/workaround-for-meteor-limitations-if-you-want-to-sub-for-more-nested-fields-of-already-received-docs-eb3fdbfe4e07

@brettle

This comment has been minimized.

Show comment
Hide comment
@brettle

brettle Oct 5, 2015

Contributor

@mitar, in this comment you suggest looking at the Meteor Roadmap for a discussion of merging. Which card are you referring to? None of the current cards immediately jump out to me as related to this issue. Thanks!

Contributor

brettle commented Oct 5, 2015

@mitar, in this comment you suggest looking at the Meteor Roadmap for a discussion of merging. Which card are you referring to? None of the current cards immediately jump out to me as related to this issue. Thanks!

@mitar

This comment has been minimized.

Show comment
Hide comment
@mitar

mitar Oct 5, 2015

Collaborator

I think this one.

Collaborator

mitar commented Oct 5, 2015

I think this one.

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