Skip to content
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

Feature request: restrict which cursors are published #42

Open
Diggsey opened this issue Jun 16, 2015 · 9 comments
Open

Feature request: restrict which cursors are published #42

Diggsey opened this issue Jun 16, 2015 · 9 comments

Comments

@Diggsey
Copy link

Diggsey commented Jun 16, 2015

At the moment, everything returned by a top-level "find" is automatically published to the client, but I only want the children to be published.

Similarly, sometimes you only want certain fields to be published, but you need all the fields to run the child query. For example, I have a field user.secret which I use as part of a child query, but I don't want to publish that field to the client.

The first feature can be implemented by an option "internal: true" or similar, the second by adding a field specifier to the options, eg. "fields: { secret: false }"

@ghost
Copy link

ghost commented Jul 23, 2015

Is there currently a way to restrict certain fields from being returned from a top-level "find" even if you don't need them to run the child query? Queries such as the following don't seem to work in any context, which makes security management more difficult: db.inventory.find( { type: 'food' }, { item: 1, qty: 1 } )

@urossmolnik
Copy link

Yes there is. Meteor syntax is db.inventory.find( { type: 'food' }, { fields: { item: 1, qty: 1 } } ).
Documentation

@ghost
Copy link

ghost commented Jul 23, 2015

Looks like I need to reread the docs. Thank you.

@reywood
Copy link
Collaborator

reywood commented Aug 11, 2015

This may be somewhat related to the discussion in #28.

@Diggsey
Copy link
Author

Diggsey commented Aug 11, 2015

@reywood What are your thoughts on my PR #43 ?

@reywood
Copy link
Collaborator

reywood commented Aug 11, 2015

It relies on the private function _compileProjection which doesn't seem like a great idea. The Meteor folks could refactor or remove that function at any time. It also completely removes the observeChanges handler which is a major blow to performance in the default use case. Given these issues, I think PR #43 is a non-starter.

@Diggsey
Copy link
Author

Diggsey commented Aug 11, 2015

It relies on the private function _compileProjection which doesn't seem like a great idea.

True - it would be possible to instead simply duplicate that function here, it's fairly simple.

It also completely removes the observeChanges handler which is a major blow to performance in the default use case.

I don't understand this, if anything removing the call to observeChanges will improve performance. It's better to have one observation running rather than two, and internally observeChanges is implemented in terms of observe anyway.

@reywood
Copy link
Collaborator

reywood commented Aug 11, 2015

My mistake. I missed that you moved that functionality into the other observe handler. In poking around, it looks like the Meteor folks are moving the diff logic into a separate package called diff-sequence. I was unable to add this package to a new meteor project. Perhaps it's still under development. Probably worth waiting until they've sorted that out, or, as you said, copy this new implementation until they do. I do think this is a good idea.

I'm leaning towards a transform option rather than a fields option. The transform function would be used before sending the doc to the client, but the untransformed doc would be dispatched to child pubs. I think it gives the same functionality but with more flexibility. Your idea to do the diffing in the observe change handler would address the concerns I voiced in #28.

@sunstorymvp
Copy link

any news with internal: true?

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

Successfully merging a pull request may close this issue.

4 participants