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

Add support for querying sharedb meta fields #7

Merged
merged 1 commit into from
Sep 28, 2018
Merged

Add support for querying sharedb meta fields #7

merged 1 commit into from
Sep 28, 2018

Conversation

jylauril
Copy link
Contributor

Changes

This change adds support for making queries against the ShareDB doc's meta property.

There are two distinct changes in this PR:

  1. Add a workaround for ShareDB's MemoryDB behavior of removing metadata from the snapshot before filtering is executed. Previously the snapshots that were passed in to _querySync did not have metadata unless explicitly defined in the query options, thus preventing any filtering to be done with the meta fields.

  2. Use a property segment to check if the property should be considered a MONGO_DOC_PROPERTY. Previously the check was using the entire property name which prevented nesting the property paths such as _m.mtime.

Notes

There has been some discussion about refactoring this adapter to use snapshot casting instead of query casting. The changes in this PR are not addressing any of that, but is instead providing an interim implementation so the meta fields can be used for querying purposes.

Copy link
Contributor

@ericyhwang ericyhwang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks for the improvement!

Shame it wasn't feasible to do this without changing MemoryDB itself or duplicating the logic in a method override... MemoryDB wasn't intended to be super bulletproof.

@ericyhwang ericyhwang merged commit 81b5c3b into share:master Sep 28, 2018
@ericyhwang
Copy link
Contributor

[email protected]

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

Successfully merging this pull request may close these issues.

None yet

2 participants