From a673f16d1fd2569496e291fe1425a9082ffb0705 Mon Sep 17 00:00:00 2001 From: Berislav Date: Thu, 14 Jun 2018 20:40:21 +0200 Subject: [PATCH 01/10] Added subscription scope to named queries --- lib/namedQuery/expose/extension.js | 6 +++ lib/namedQuery/expose/schema.js | 1 + lib/namedQuery/namedQuery.client.js | 38 +++++++++++++++++-- .../testing/bootstrap/queries/index.js | 2 + .../testing/bootstrap/queries/postList.js | 2 + .../queries/postListExposureScoped.js | 29 ++++++++++++++ lib/namedQuery/testing/client.test.js | 28 ++++++++++++++ package.js | 2 + 8 files changed, 105 insertions(+), 3 deletions(-) mode change 100644 => 100755 lib/namedQuery/expose/extension.js mode change 100644 => 100755 lib/namedQuery/expose/schema.js mode change 100644 => 100755 lib/namedQuery/namedQuery.client.js mode change 100644 => 100755 lib/namedQuery/testing/bootstrap/queries/index.js mode change 100644 => 100755 lib/namedQuery/testing/bootstrap/queries/postList.js create mode 100755 lib/namedQuery/testing/bootstrap/queries/postListExposureScoped.js mode change 100644 => 100755 lib/namedQuery/testing/client.test.js mode change 100644 => 100755 package.js diff --git a/lib/namedQuery/expose/extension.js b/lib/namedQuery/expose/extension.js old mode 100644 new mode 100755 index a556d21..b53d072 --- a/lib/namedQuery/expose/extension.js +++ b/lib/namedQuery/expose/extension.js @@ -154,6 +154,12 @@ _.extend(NamedQuery.prototype, { const self = this; Meteor.publishComposite(this.name, function(params = {}) { + const isScoped = !!self.options.scoped; + + if (isScoped) { + this.enableScope(); + } + self._unblockIfNecessary(this); self.doValidateParams(params); self._callFirewall(this, this.userId, params); diff --git a/lib/namedQuery/expose/schema.js b/lib/namedQuery/expose/schema.js old mode 100644 new mode 100755 index bdd942d..dd1fae0 --- a/lib/namedQuery/expose/schema.js +++ b/lib/namedQuery/expose/schema.js @@ -19,4 +19,5 @@ export const ExposeSchema = { validateParams: Match.Maybe( Match.OneOf(Object, Function) ), + scoped: Match.Maybe(Boolean), }; diff --git a/lib/namedQuery/namedQuery.client.js b/lib/namedQuery/namedQuery.client.js old mode 100644 new mode 100755 index da78ab1..b35344d --- a/lib/namedQuery/namedQuery.client.js +++ b/lib/namedQuery/namedQuery.client.js @@ -178,8 +178,40 @@ export default class extends Base { delete body.$options.skip; } - return recursiveFetch( - createGraph(this.collection, body) - ); + const rootNode = createGraph(this.collection, body, { + scopeField: `_sub_${this.subscriptionHandle.subscriptionId}`, + }); + + const subscriptionHandle = this.subscriptionHandle; + + const unmaskFns = []; + function maskFind(node) { + const oldFind = node.collection.find; + node.collection.find = function (query, ...args) { + const scopedQuery = { + ...(query || {}), + [`_sub_${subscriptionHandle.subscriptionId}`]: {$exists: true}, + }; + return oldFind.call(this, scopedQuery, ...args); + }; + + unmaskFns.push(function () { + node.collection.find = oldFind; + }); + + node.collectionNodes.map(n => maskFind(n)); + } + + if (this.options.scoped) { + maskFind(rootNode); + } + + const results = recursiveFetch(rootNode); + + if (this.options.scoped) { + unmaskFns.map(f => f()); + } + + return results; } } diff --git a/lib/namedQuery/testing/bootstrap/queries/index.js b/lib/namedQuery/testing/bootstrap/queries/index.js old mode 100644 new mode 100755 index 9a76638..f4bd973 --- a/lib/namedQuery/testing/bootstrap/queries/index.js +++ b/lib/namedQuery/testing/bootstrap/queries/index.js @@ -1,6 +1,7 @@ import postList from './postList'; import postListCached from './postListCached'; import postListExposure from './postListExposure'; +import postListExposureScoped from './postListExposureScoped'; import postListParamsCheck from './postListParamsCheck'; import postListParamsCheckServer from './postListParamsCheckServer'; import postListResolver from './postListResolver'; @@ -10,6 +11,7 @@ export { postList, postListCached, postListExposure, + postListExposureScoped, postListParamsCheck, postListParamsCheckServer, postListResolver, diff --git a/lib/namedQuery/testing/bootstrap/queries/postList.js b/lib/namedQuery/testing/bootstrap/queries/postList.js old mode 100644 new mode 100755 index 880a9ca..1763416 --- a/lib/namedQuery/testing/bootstrap/queries/postList.js +++ b/lib/namedQuery/testing/bootstrap/queries/postList.js @@ -19,6 +19,8 @@ const postList = createQuery('postList', { name: 1 } } +}, { + scoped: true, }); export default postList; \ No newline at end of file diff --git a/lib/namedQuery/testing/bootstrap/queries/postListExposureScoped.js b/lib/namedQuery/testing/bootstrap/queries/postListExposureScoped.js new file mode 100755 index 0000000..8651523 --- /dev/null +++ b/lib/namedQuery/testing/bootstrap/queries/postListExposureScoped.js @@ -0,0 +1,29 @@ +import { createQuery } from 'meteor/cultofcoders:grapher'; + +const postListExposureScoped = createQuery('postListExposureScoped', { + posts: { + title: 1, + author: { + name: 1 + }, + group: { + name: 1 + } + } +}, { + scoped: true, +}); + +if (Meteor.isServer) { + postListExposureScoped.expose({ + firewall(userId, params) { + }, + embody: { + $filter({filters, params}) { + filters.title = params.title + } + } + }); +} + +export default postListExposureScoped; \ No newline at end of file diff --git a/lib/namedQuery/testing/client.test.js b/lib/namedQuery/testing/client.test.js old mode 100644 new mode 100755 index f2aacb3..609c5c3 --- a/lib/namedQuery/testing/client.test.js +++ b/lib/namedQuery/testing/client.test.js @@ -1,5 +1,7 @@ import postListExposure from './bootstrap/queries/postListExposure.js'; +import postListExposureScoped from './bootstrap/queries/postListExposureScoped'; import { createQuery } from 'meteor/cultofcoders:grapher'; +import Posts from '../../query/testing/bootstrap/posts/collection'; describe('Named Query', function() { it('Should return proper values', function(done) { @@ -93,6 +95,32 @@ describe('Named Query', function() { }); }); + it('Should work with reactive scoped queries', function(done) { + const query = postListExposureScoped.clone({ title: 'User Post - 3' }); + + const handle = query.subscribe(); + Tracker.autorun(c => { + if (handle.ready()) { + c.stop(); + const data = query.fetch(); + handle.stop(); + + assert.isTrue(data.length > 0); + + const docMap = Posts._collection._docs._map; + const scopeField = `_sub_${handle.subscriptionId}`; + data.forEach(post => { + // no scope field returned from find + assert.isUndefined(post[scopeField]); + assert.isObject(docMap[post._id]); + assert.equal(docMap[post._id][scopeField], 1); + }); + + done(); + } + }); + }); + it('Should work with reactive queries via import', function(done) { const query = postListExposure.clone({ title: 'User Post - 3', diff --git a/package.js b/package.js old mode 100644 new mode 100755 index 0cf4f1b..ab1be86 --- a/package.js +++ b/package.js @@ -31,6 +31,7 @@ Package.onUse(function(api) { 'reywood:publish-composite@1.5.2', 'dburles:mongo-collection-instances@0.3.5', 'herteby:denormalize@0.6.5', + 'peerlibrary:subscription-scope', ]; api.use(packages); @@ -49,6 +50,7 @@ Package.onTest(function(api) { 'reywood:publish-composite@1.5.2', 'dburles:mongo-collection-instances@0.3.5', 'herteby:denormalize@0.6.5', + 'peerlibrary:subscription-scope', 'mongo', ]; From 8fedac5b3b68987d670a87feb7fd851af4209f3c Mon Sep 17 00:00:00 2001 From: Berislav Date: Fri, 15 Jun 2018 09:48:43 +0200 Subject: [PATCH 02/10] Better handling of find method override for scoped queries Conflicts: lib/namedQuery/namedQuery.client.js --- lib/namedQuery/namedQuery.client.js | 29 ++++++++--------------------- 1 file changed, 8 insertions(+), 21 deletions(-) diff --git a/lib/namedQuery/namedQuery.client.js b/lib/namedQuery/namedQuery.client.js index b35344d..a374a71 100755 --- a/lib/namedQuery/namedQuery.client.js +++ b/lib/namedQuery/namedQuery.client.js @@ -5,6 +5,8 @@ import prepareForProcess from '../query/lib/prepareForProcess.js'; import { _ } from 'meteor/underscore'; import callWithPromise from '../query/lib/callWithPromise'; import Base from './namedQuery.base'; +import {LocalCollection} from 'meteor/minimongo'; + export default class extends Base { /** @@ -183,33 +185,18 @@ export default class extends Base { }); const subscriptionHandle = this.subscriptionHandle; - - const unmaskFns = []; - function maskFind(node) { - const oldFind = node.collection.find; - node.collection.find = function (query, ...args) { - const scopedQuery = { - ...(query || {}), - [`_sub_${subscriptionHandle.subscriptionId}`]: {$exists: true}, - }; - return oldFind.call(this, scopedQuery, ...args); - }; - - unmaskFns.push(function () { - node.collection.find = oldFind; - }); - - node.collectionNodes.map(n => maskFind(n)); - } - + // if query is scoped, all calls to find from recursive fetch should contain scopedQuery() if (this.options.scoped) { - maskFind(rootNode); + LocalCollection.prototype.__originalFind = LocalCollection.prototype.find; + LocalCollection.prototype.find = function(query, ...args) { + return LocalCollection.prototype.__originalFind.call(this, {$and: [subscriptionHandle.scopeQuery(), query || {}]}, ...args); + }; } const results = recursiveFetch(rootNode); if (this.options.scoped) { - unmaskFns.map(f => f()); + LocalCollection.prototype.find = LocalCollection.prototype.__originalFind; } return results; From 5adca6e570cdf369c978b127818841e804830aab Mon Sep 17 00:00:00 2001 From: Berislav Date: Mon, 24 Sep 2018 07:28:54 -0700 Subject: [PATCH 03/10] Fixed version of peerlibary:subscription-scope --- package.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.js b/package.js index ab1be86..b821adf 100755 --- a/package.js +++ b/package.js @@ -31,7 +31,7 @@ Package.onUse(function(api) { 'reywood:publish-composite@1.5.2', 'dburles:mongo-collection-instances@0.3.5', 'herteby:denormalize@0.6.5', - 'peerlibrary:subscription-scope', + 'peerlibrary:subscription-scope@0.4.0', ]; api.use(packages); @@ -50,7 +50,7 @@ Package.onTest(function(api) { 'reywood:publish-composite@1.5.2', 'dburles:mongo-collection-instances@0.3.5', 'herteby:denormalize@0.6.5', - 'peerlibrary:subscription-scope', + 'peerlibrary:subscription-scope@0.4.0', 'mongo', ]; From 6d329f5645208f088ed347cb91ffd0f54c3b69d1 Mon Sep 17 00:00:00 2001 From: Berislav Date: Mon, 24 Sep 2018 08:33:52 -0700 Subject: [PATCH 04/10] Updated recursiveFetch on client to handle scoped queries --- lib/namedQuery/namedQuery.client.js | 28 +++++++--------------------- lib/query/lib/recursiveFetch.js | 11 ++++++++--- 2 files changed, 15 insertions(+), 24 deletions(-) diff --git a/lib/namedQuery/namedQuery.client.js b/lib/namedQuery/namedQuery.client.js index a374a71..89d5d88 100755 --- a/lib/namedQuery/namedQuery.client.js +++ b/lib/namedQuery/namedQuery.client.js @@ -2,7 +2,7 @@ import CountSubscription from '../query/counts/countSubscription'; import createGraph from '../query/lib/createGraph.js'; import recursiveFetch from '../query/lib/recursiveFetch.js'; import prepareForProcess from '../query/lib/prepareForProcess.js'; -import { _ } from 'meteor/underscore'; +import {_} from 'meteor/underscore'; import callWithPromise from '../query/lib/callWithPromise'; import Base from './namedQuery.base'; import {LocalCollection} from 'meteor/minimongo'; @@ -180,25 +180,11 @@ export default class extends Base { delete body.$options.skip; } - const rootNode = createGraph(this.collection, body, { - scopeField: `_sub_${this.subscriptionHandle.subscriptionId}`, - }); - - const subscriptionHandle = this.subscriptionHandle; - // if query is scoped, all calls to find from recursive fetch should contain scopedQuery() - if (this.options.scoped) { - LocalCollection.prototype.__originalFind = LocalCollection.prototype.find; - LocalCollection.prototype.find = function(query, ...args) { - return LocalCollection.prototype.__originalFind.call(this, {$and: [subscriptionHandle.scopeQuery(), query || {}]}, ...args); - }; - } - - const results = recursiveFetch(rootNode); - - if (this.options.scoped) { - LocalCollection.prototype.find = LocalCollection.prototype.__originalFind; - } - - return results; + return recursiveFetch( + createGraph(this.collection, body), + undefined, { + scoped: this.options.scoped, + subscriptionHandle: this.subscriptionHandle, + }); } } diff --git a/lib/query/lib/recursiveFetch.js b/lib/query/lib/recursiveFetch.js index 5cf9c9a..ce38832 100755 --- a/lib/query/lib/recursiveFetch.js +++ b/lib/query/lib/recursiveFetch.js @@ -7,10 +7,15 @@ import prepareForDelivery from './prepareForDelivery'; * * @param node * @param parentObject + * @param fetchOptions * @returns {*} */ -function fetch(node, parentObject) { +function fetch(node, parentObject, fetchOptions = {}) { let {filters, options} = applyProps(node); + // add subscription filter + if (fetchOptions.scoped && fetchOptions.subscriptionHandle) { + _.extend(filters, fetchOptions.subscriptionHandle.scopeQuery()); + } let results = []; @@ -53,8 +58,8 @@ function fetch(node, parentObject) { return results; } -export default (node, params) => { - node.results = fetch(node); +export default (node, params, fetchOptions) => { + node.results = fetch(node, null, fetchOptions); prepareForDelivery(node, params); From 77c4b4b2e11558b1615f1a18fa2020b2f2c9b7b5 Mon Sep 17 00:00:00 2001 From: Berislav Date: Mon, 24 Sep 2018 08:59:24 -0700 Subject: [PATCH 05/10] Downgraded subscription-scope package because of issues with tests --- package.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.js b/package.js index 17063a6..96adcab 100755 --- a/package.js +++ b/package.js @@ -31,7 +31,7 @@ Package.onUse(function(api) { 'reywood:publish-composite@1.5.2', 'dburles:mongo-collection-instances@0.3.5', 'herteby:denormalize@0.6.5', - 'peerlibrary:subscription-scope@0.4.0', + 'peerlibrary:subscription-scope@0.1.0', ]; api.use(packages); @@ -50,7 +50,7 @@ Package.onTest(function(api) { 'reywood:publish-composite@1.5.2', 'dburles:mongo-collection-instances@0.3.5', 'herteby:denormalize@0.6.5', - 'peerlibrary:subscription-scope@0.4.0', + 'peerlibrary:subscription-scope@0.1.0', 'mongo', ]; From 480614cdc4bcf596794a9ebfb5f887e5eb3207fd Mon Sep 17 00:00:00 2001 From: Berislav Date: Mon, 24 Sep 2018 11:44:35 -0700 Subject: [PATCH 06/10] Scoped query PoC --- lib/namedQuery/namedQuery.client.js | 1 + lib/query/lib/applyProps.js | 0 lib/query/lib/createGraph.js | 16 ++++++++++++++++ lib/query/lib/recursiveCompose.js | 23 +++++++++++++++++++++-- lib/query/lib/recursiveFetch.js | 5 +++++ 5 files changed, 43 insertions(+), 2 deletions(-) mode change 100644 => 100755 lib/query/lib/applyProps.js mode change 100644 => 100755 lib/query/lib/createGraph.js mode change 100644 => 100755 lib/query/lib/recursiveCompose.js diff --git a/lib/namedQuery/namedQuery.client.js b/lib/namedQuery/namedQuery.client.js index 180ec73..bf402f6 100755 --- a/lib/namedQuery/namedQuery.client.js +++ b/lib/namedQuery/namedQuery.client.js @@ -184,6 +184,7 @@ export default class extends Base { createGraph(this.collection, body), undefined, { scoped: this.options.scoped, + queryScoped: true, subscriptionHandle: this.subscriptionHandle, }); } diff --git a/lib/query/lib/applyProps.js b/lib/query/lib/applyProps.js old mode 100644 new mode 100755 diff --git a/lib/query/lib/createGraph.js b/lib/query/lib/createGraph.js old mode 100644 new mode 100755 index c00cf4d..dc2d660 --- a/lib/query/lib/createGraph.js +++ b/lib/query/lib/createGraph.js @@ -99,6 +99,22 @@ export function addFieldNode(body, fieldName, root) { } } +/** + * Returns namespace for node when using query path scoping. + * + * @param node + * @returns {String} + */ +export function getNodeNamespace(node) { + const parts = []; + let n = node; + while (n) { + parts.push(n.collection._name); + n = n.parent; + } + return parts.reverse().join('_'); +} + /** * @param collection * @param body diff --git a/lib/query/lib/recursiveCompose.js b/lib/query/lib/recursiveCompose.js old mode 100644 new mode 100755 index 7b787fe..a12d84f --- a/lib/query/lib/recursiveCompose.js +++ b/lib/query/lib/recursiveCompose.js @@ -1,4 +1,19 @@ import applyProps from './applyProps.js'; +import {getNodeNamespace} from './createGraph'; + +function patchCursor(cursor, ns) { + const originalObserve = cursor.observe; + cursor.observe = function (callbacks) { + const newCallbacks = {}; + if (callbacks.added) { + newCallbacks.added = doc => { + doc[`__query_path_${ns}`] = ns; + callbacks.added(doc); + }; + } + originalObserve.call(cursor, newCallbacks); + }; +} function compose(node, userId) { return { @@ -18,7 +33,9 @@ function compose(node, userId) { }); } - return accessor.find(filters, options, userId); + const cursor = accessor.find(filters, options, userId); + patchCursor(cursor, getNodeNamespace(node)); + return cursor; } }, @@ -31,7 +48,9 @@ export default (node, userId, config = {bypassFirewalls: false}) => { find() { let {filters, options} = applyProps(node); - return node.collection.find(filters, options, userId); + const cursor = node.collection.find(filters, options, userId); + patchCursor(cursor, getNodeNamespace(node)); + return cursor; }, children: _.map(node.collectionNodes, n => { diff --git a/lib/query/lib/recursiveFetch.js b/lib/query/lib/recursiveFetch.js index ce38832..d02933a 100755 --- a/lib/query/lib/recursiveFetch.js +++ b/lib/query/lib/recursiveFetch.js @@ -1,6 +1,7 @@ import applyProps from './applyProps.js'; import { assembleMetadata, removeLinkStorages, storeOneResults } from './prepareForDelivery'; import prepareForDelivery from './prepareForDelivery'; +import {getNodeNamespace} from './createGraph'; /** * This is always run client side to build the data graph out of client-side collections. @@ -16,6 +17,10 @@ function fetch(node, parentObject, fetchOptions = {}) { if (fetchOptions.scoped && fetchOptions.subscriptionHandle) { _.extend(filters, fetchOptions.subscriptionHandle.scopeQuery()); } + // does not make sense without scoped parameter + if (fetchOptions.scoped && fetchOptions.queryScoped) { + _.extend(filters, {[`__query_path_${getNodeNamespace(node)}`]: {$exists: true}}); + } let results = []; From 3ed4bf589c54c137c490b32353b661057962b1ea Mon Sep 17 00:00:00 2001 From: Berislav Date: Mon, 24 Sep 2018 11:56:32 -0700 Subject: [PATCH 07/10] Updated scope query value --- lib/query/lib/recursiveCompose.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/query/lib/recursiveCompose.js b/lib/query/lib/recursiveCompose.js index a12d84f..b2631be 100755 --- a/lib/query/lib/recursiveCompose.js +++ b/lib/query/lib/recursiveCompose.js @@ -7,7 +7,7 @@ function patchCursor(cursor, ns) { const newCallbacks = {}; if (callbacks.added) { newCallbacks.added = doc => { - doc[`__query_path_${ns}`] = ns; + doc[`__query_path_${ns}`] = 1; callbacks.added(doc); }; } From a04edaf59fb9fe0c7ba5d6f4bcfa3dc511b41488 Mon Sep 17 00:00:00 2001 From: Berislav Date: Mon, 24 Sep 2018 12:01:22 -0700 Subject: [PATCH 08/10] Fixed observe callbacks --- lib/query/lib/recursiveCompose.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/query/lib/recursiveCompose.js b/lib/query/lib/recursiveCompose.js index b2631be..b84b25c 100755 --- a/lib/query/lib/recursiveCompose.js +++ b/lib/query/lib/recursiveCompose.js @@ -4,7 +4,7 @@ import {getNodeNamespace} from './createGraph'; function patchCursor(cursor, ns) { const originalObserve = cursor.observe; cursor.observe = function (callbacks) { - const newCallbacks = {}; + const newCallbacks = Object.assign({}, callbacks); if (callbacks.added) { newCallbacks.added = doc => { doc[`__query_path_${ns}`] = 1; From 70fd03a743021b417d35be68da4768e50b9ab9a3 Mon Sep 17 00:00:00 2001 From: Berislav Date: Wed, 24 Oct 2018 08:09:39 -0700 Subject: [PATCH 09/10] Updates to scoped query handling --- lib/namedQuery/expose/extension.js | 2 +- lib/namedQuery/expose/schema.js | 3 +- lib/namedQuery/namedQuery.client.js | 3 +- .../testing/bootstrap/queries/index.js | 1 + .../bootstrap/queries/userListScoped.js | 26 +++++++++++ lib/namedQuery/testing/client.test.js | 44 +++++++++++++++++++ lib/namedQuery/testing/server.test.js | 0 lib/query/lib/recursiveCompose.js | 23 +++++++--- lib/query/lib/recursiveFetch.js | 6 +-- lib/query/testing/bootstrap/fixtures.js | 14 ++++++ lib/query/testing/bootstrap/index.js | 2 + .../testing/bootstrap/users/collection.js | 2 + lib/query/testing/bootstrap/users/links.js | 9 ++++ 13 files changed, 121 insertions(+), 14 deletions(-) create mode 100755 lib/namedQuery/testing/bootstrap/queries/userListScoped.js mode change 100644 => 100755 lib/namedQuery/testing/server.test.js mode change 100644 => 100755 lib/query/testing/bootstrap/index.js create mode 100755 lib/query/testing/bootstrap/users/collection.js create mode 100755 lib/query/testing/bootstrap/users/links.js diff --git a/lib/namedQuery/expose/extension.js b/lib/namedQuery/expose/extension.js index b53d072..1e5c63b 100755 --- a/lib/namedQuery/expose/extension.js +++ b/lib/namedQuery/expose/extension.js @@ -174,7 +174,7 @@ _.extend(NamedQuery.prototype, { const rootNode = createGraph(self.collection, body); - return recursiveCompose(rootNode); + return recursiveCompose(rootNode, undefined, {scoped: isScoped}); }); }, diff --git a/lib/namedQuery/expose/schema.js b/lib/namedQuery/expose/schema.js index dd1fae0..f28e43e 100755 --- a/lib/namedQuery/expose/schema.js +++ b/lib/namedQuery/expose/schema.js @@ -18,6 +18,5 @@ export const ExposeSchema = { ), validateParams: Match.Maybe( Match.OneOf(Object, Function) - ), - scoped: Match.Maybe(Boolean), + ) }; diff --git a/lib/namedQuery/namedQuery.client.js b/lib/namedQuery/namedQuery.client.js index bf402f6..0d4d8e6 100755 --- a/lib/namedQuery/namedQuery.client.js +++ b/lib/namedQuery/namedQuery.client.js @@ -184,8 +184,7 @@ export default class extends Base { createGraph(this.collection, body), undefined, { scoped: this.options.scoped, - queryScoped: true, - subscriptionHandle: this.subscriptionHandle, + subscriptionHandle: this.subscriptionHandle }); } } diff --git a/lib/namedQuery/testing/bootstrap/queries/index.js b/lib/namedQuery/testing/bootstrap/queries/index.js index f4bd973..ad35e81 100755 --- a/lib/namedQuery/testing/bootstrap/queries/index.js +++ b/lib/namedQuery/testing/bootstrap/queries/index.js @@ -6,6 +6,7 @@ import postListParamsCheck from './postListParamsCheck'; import postListParamsCheckServer from './postListParamsCheckServer'; import postListResolver from './postListResolver'; import postListResolverCached from './postListResolverCached'; +import userListScoped from './userListScoped'; export { postList, diff --git a/lib/namedQuery/testing/bootstrap/queries/userListScoped.js b/lib/namedQuery/testing/bootstrap/queries/userListScoped.js new file mode 100755 index 0000000..23b4f2d --- /dev/null +++ b/lib/namedQuery/testing/bootstrap/queries/userListScoped.js @@ -0,0 +1,26 @@ +import { createQuery } from 'meteor/cultofcoders:grapher'; + +const userListScoped = createQuery('userListScoped', { + users: { + name: 1, + friends: { + name: 1 + } + } +}, { + scoped: true +}); + +if (Meteor.isServer) { + userListScoped.expose({ + firewall() { + }, + embody: { + $filter({filters, params}) { + filters.name = params.name; + } + } + }) +} + +export default userListScoped; diff --git a/lib/namedQuery/testing/client.test.js b/lib/namedQuery/testing/client.test.js index 8eda271..371783f 100755 --- a/lib/namedQuery/testing/client.test.js +++ b/lib/namedQuery/testing/client.test.js @@ -1,7 +1,9 @@ import postListExposure from './bootstrap/queries/postListExposure.js'; import postListExposureScoped from './bootstrap/queries/postListExposureScoped'; +import userListScoped from './bootstrap/queries/userListScoped'; import { createQuery } from 'meteor/cultofcoders:grapher'; import Posts from '../../query/testing/bootstrap/posts/collection'; +import Users from '../../query/testing/bootstrap/users/collection'; describe('Named Query', function() { it('Should return proper values', function(done) { @@ -130,11 +132,13 @@ describe('Named Query', function() { const docMap = Posts._collection._docs._map; const scopeField = `_sub_${handle.subscriptionId}`; + const queryPathField = '_query_path_posts'; data.forEach(post => { // no scope field returned from find assert.isUndefined(post[scopeField]); assert.isObject(docMap[post._id]); assert.equal(docMap[post._id][scopeField], 1); + assert.equal(docMap[post._id][queryPathField], 1); }); done(); @@ -142,6 +146,46 @@ describe('Named Query', function() { }); }); + it('Should work with reactive recursive scoped queries', function (done) { + const query = userListScoped.clone({name: 'User - 3'}); + + const handle = query.subscribe(); + Tracker.autorun(c => { + if (handle.ready()) { + c.stop(); + const data = query.fetch(); + handle.stop(); + + assert.equal(data.length, 1); + // User 3 has users 0,1,2 as friends + const [user3] = data; + assert.equal(user3.friends.length, 3); + + const docMap = Users._collection._docs._map; + // users collection on the client should have 4 items (user 3 and friends - user 0,1,2) + assert.equal(_.keys(docMap).length, 4); + + const scopeField = `_sub_${handle.subscriptionId}`; + const rootQueryPathField = '_query_path_users'; + const nestedQueryPathField = '_query_path_users_users'; + Object.entries(docMap).forEach(([userId, userDoc]) => { + const isRoot = userId === user3._id; + assert.equal(userDoc[scopeField], 1); + if (isRoot) { + assert.equal(userDoc[rootQueryPathField], 1); + assert.isTrue(!(nestedQueryPathField in userDoc)); + } + else { + assert.equal(userDoc[nestedQueryPathField], 1); + assert.isTrue(!(rootQueryPathField in userDoc)); + } + }); + + done(); + } + }); + }); + it('Should work with reactive queries via import', function(done) { const query = postListExposure.clone({ title: 'User Post - 3', diff --git a/lib/namedQuery/testing/server.test.js b/lib/namedQuery/testing/server.test.js old mode 100644 new mode 100755 diff --git a/lib/query/lib/recursiveCompose.js b/lib/query/lib/recursiveCompose.js index b84b25c..7650b74 100755 --- a/lib/query/lib/recursiveCompose.js +++ b/lib/query/lib/recursiveCompose.js @@ -1,13 +1,20 @@ import applyProps from './applyProps.js'; import {getNodeNamespace} from './createGraph'; +/** + * Adds _query_path fields to the cursor docs which are used for scoped query filtering on the client. + * + * @param cursor + * @param ns + */ function patchCursor(cursor, ns) { const originalObserve = cursor.observe; cursor.observe = function (callbacks) { const newCallbacks = Object.assign({}, callbacks); if (callbacks.added) { newCallbacks.added = doc => { - doc[`__query_path_${ns}`] = 1; + doc = _.clone(doc); + doc[`_query_path_${ns}`] = 1; callbacks.added(doc); }; } @@ -15,7 +22,7 @@ function patchCursor(cursor, ns) { }; } -function compose(node, userId) { +function compose(node, userId, config) { return { find(parent) { if (parent) { @@ -34,7 +41,9 @@ function compose(node, userId) { } const cursor = accessor.find(filters, options, userId); - patchCursor(cursor, getNodeNamespace(node)); + if (config.scoped) { + patchCursor(cursor, getNodeNamespace(node)); + } return cursor; } }, @@ -43,20 +52,22 @@ function compose(node, userId) { } } -export default (node, userId, config = {bypassFirewalls: false}) => { +export default (node, userId, config = {bypassFirewalls: false, scoped: false}) => { return { find() { let {filters, options} = applyProps(node); const cursor = node.collection.find(filters, options, userId); - patchCursor(cursor, getNodeNamespace(node)); + if (config.scoped) { + patchCursor(cursor, getNodeNamespace(node)); + } return cursor; }, children: _.map(node.collectionNodes, n => { const userIdToPass = (config.bypassFirewalls) ? undefined : userId; - return compose(n, userIdToPass); + return compose(n, userIdToPass, config); }) } } \ No newline at end of file diff --git a/lib/query/lib/recursiveFetch.js b/lib/query/lib/recursiveFetch.js index d02933a..8c2ff29 100755 --- a/lib/query/lib/recursiveFetch.js +++ b/lib/query/lib/recursiveFetch.js @@ -17,9 +17,9 @@ function fetch(node, parentObject, fetchOptions = {}) { if (fetchOptions.scoped && fetchOptions.subscriptionHandle) { _.extend(filters, fetchOptions.subscriptionHandle.scopeQuery()); } - // does not make sense without scoped parameter - if (fetchOptions.scoped && fetchOptions.queryScoped) { - _.extend(filters, {[`__query_path_${getNodeNamespace(node)}`]: {$exists: true}}); + // add query path filter + if (fetchOptions.scoped) { + _.extend(filters, {[`_query_path_${getNodeNamespace(node)}`]: {$exists: true}}); } let results = []; diff --git a/lib/query/testing/bootstrap/fixtures.js b/lib/query/testing/bootstrap/fixtures.js index 0e8705f..af97455 100755 --- a/lib/query/testing/bootstrap/fixtures.js +++ b/lib/query/testing/bootstrap/fixtures.js @@ -7,16 +7,19 @@ import Comments from './comments/collection'; import Posts from './posts/collection'; import Tags from './tags/collection'; import Groups from './groups/collection'; +import Users from './users/collection'; Authors.remove({}); Comments.remove({}); Posts.remove({}); Tags.remove({}); Groups.remove({}); +Users.remove({}); const AUTHORS = 6; const POST_PER_USER = 6; const COMMENTS_PER_POST = 6; +const USERS = 4; const TAGS = ['JavaScript', 'Meteor', 'React', 'Other']; const GROUPS = ['JavaScript', 'Meteor', 'React', 'Other']; const COMMENT_TEXT_SAMPLES = [ @@ -81,4 +84,15 @@ _.each(authors, (author) => { }) }); +const friendIds = []; +// each user is created so his friends are previously added users +_.range(USERS).forEach(idx => { + const id = Users.insert({ + name: `User - ${idx}`, + friendIds, + }); + + friendIds.push(id); +}); + console.log('[ok] fixtures have been loaded.'); diff --git a/lib/query/testing/bootstrap/index.js b/lib/query/testing/bootstrap/index.js old mode 100644 new mode 100755 index 0eda4b7..24b3cbe --- a/lib/query/testing/bootstrap/index.js +++ b/lib/query/testing/bootstrap/index.js @@ -4,10 +4,12 @@ import './authors/links'; import './tags/links'; import './groups/links'; import './security/links'; +import './users/links'; import Posts from './posts/collection'; import Groups from './groups/collection'; import Authors from './authors/collection'; +import Users from './users/collection'; if (Meteor.isServer) { Posts.expose(); diff --git a/lib/query/testing/bootstrap/users/collection.js b/lib/query/testing/bootstrap/users/collection.js new file mode 100755 index 0000000..e4648e6 --- /dev/null +++ b/lib/query/testing/bootstrap/users/collection.js @@ -0,0 +1,2 @@ +const Users = new Mongo.Collection('users'); +export default Users; diff --git a/lib/query/testing/bootstrap/users/links.js b/lib/query/testing/bootstrap/users/links.js new file mode 100755 index 0000000..1c8a47f --- /dev/null +++ b/lib/query/testing/bootstrap/users/links.js @@ -0,0 +1,9 @@ +import Users from './collection'; + +Users.addLinks({ + friends: { + collection: Users, + field: 'friendIds', + type: 'many' + }, +}); From e72b206ae1e76f17b7c0d4ba68fc624d2023384f Mon Sep 17 00:00:00 2001 From: Berislav Date: Wed, 24 Oct 2018 13:05:13 -0700 Subject: [PATCH 10/10] Scoped query docs & some improvements --- docs/named_queries.md | 89 +++++++++++++++++++ .../bootstrap/queries/userListScoped.js | 3 + lib/namedQuery/testing/client.test.js | 17 +++- lib/query/lib/createGraph.js | 4 +- lib/query/testing/bootstrap/fixtures.js | 1 + lib/query/testing/bootstrap/users/links.js | 5 ++ 6 files changed, 114 insertions(+), 5 deletions(-) mode change 100644 => 100755 docs/named_queries.md diff --git a/docs/named_queries.md b/docs/named_queries.md old mode 100644 new mode 100755 index 318e7d8..d76563a --- a/docs/named_queries.md +++ b/docs/named_queries.md @@ -426,6 +426,95 @@ query.expose({ }) ``` +## Scoped publications + +Scoped publications add a scope (context) to their published documents with the goal of client being able to distinguish between documents of different publications or different grapher paths in the same publication. + +Problems often arise when using only server-side filtering. + +Consider the following situation: + +```js +// the query +const usersQuery = Users.createQuery('getUsers', { + name: 1, + friends: { + name: 1, + }, +}, { + scoped: true, +}); + +// server-side exposure +usersQuery.expose({ + embody: { + $filter({filters, params}) { + filters.name = params.name; + } + } +}); + +// links +Users.addLinks({ + friends: { + collection: Users, + field: 'friendIds', + type: 'many' + }, +}); +``` + +Notice that `friends` is a link from Users to Users collection. Also, we have server-side filtering (see exposure). +On the client, we want to fetch reactively one user by name, but we are going to get all of his friends, too, and that is because of the `friends` link. +```js +// querying for user John +withQuery(props => { + return usersQuery.clone({ + name: 'John', + }); +}, { + reactive: true, +})(SomeComponent); +``` + +Client receives queried user (John) and all of his friends into the local Users collection. +By passing `{scoped: true}` query parameter to the `createQuery()`, client-side recursive fetching is now able to distinguish between queried user and his friends. + +### Technical details +Continuing on the example above, there are two pieces on how server and client achieve this functionality. + +#### Subscription scope +Each subscription adds `_sub_` field to its documents. For example, a User document could look like this: +``` +{ + name: 'John', + _sub_1: 1, + _sub_2: 1 +} +``` +This way we ensure that there is no mixup between the subscriptions (i.e. between two reactive queries on the client). + +#### Query path scope +Now suppose Alice is John's friend. Both Alice and John would have the same `_sub_` field for our example query and we would get both instead of only John. +This part is solved by adding "query path" field to the docs, in format `_query_path_` where namespace is path constructed from collection name and link names, for example: +``` +{ + name: 'John', + _sub_1: 1, + _query_path_users: 1, +}, +{ + name: 'Alice', + _sub_1: 1, + // deeper nesting than John's + _query_path_users_friends: 1 +} +``` + +where Alice has namespace equal to `users_friends` and client-side recursive fetching can now distinguish between the documents that should be returned as a query result (John) and as a `friends` link results for John (which is Alice). + +By adding query path field into the documents, we ensure that there is no mixup between the documents in the same reactive query (i.e. subscription). + ## Conclusion We can now safely expose our queries to the client, and the client can use it in a simple and uniform way. diff --git a/lib/namedQuery/testing/bootstrap/queries/userListScoped.js b/lib/namedQuery/testing/bootstrap/queries/userListScoped.js index 23b4f2d..5ef54f2 100755 --- a/lib/namedQuery/testing/bootstrap/queries/userListScoped.js +++ b/lib/namedQuery/testing/bootstrap/queries/userListScoped.js @@ -5,6 +5,9 @@ const userListScoped = createQuery('userListScoped', { name: 1, friends: { name: 1 + }, + subordinates: { + name: 1, } } }, { diff --git a/lib/namedQuery/testing/client.test.js b/lib/namedQuery/testing/client.test.js index 371783f..d0aa1dd 100755 --- a/lib/namedQuery/testing/client.test.js +++ b/lib/namedQuery/testing/client.test.js @@ -157,7 +157,7 @@ describe('Named Query', function() { handle.stop(); assert.equal(data.length, 1); - // User 3 has users 0,1,2 as friends + // User 3 has users 0,1,2 as friends and user 2 as subordinate const [user3] = data; assert.equal(user3.friends.length, 3); @@ -167,17 +167,26 @@ describe('Named Query', function() { const scopeField = `_sub_${handle.subscriptionId}`; const rootQueryPathField = '_query_path_users'; - const nestedQueryPathField = '_query_path_users_users'; + const friendsQueryPathField = '_query_path_users_friends'; + const adversaryQueryPathField = '_query_path_users_subordinates'; Object.entries(docMap).forEach(([userId, userDoc]) => { const isRoot = userId === user3._id; assert.equal(userDoc[scopeField], 1); if (isRoot) { assert.equal(userDoc[rootQueryPathField], 1); - assert.isTrue(!(nestedQueryPathField in userDoc)); + assert.isTrue(!(friendsQueryPathField in userDoc)); + assert.isTrue(!(adversaryQueryPathField in userDoc)); } else { - assert.equal(userDoc[nestedQueryPathField], 1); + assert.equal(userDoc[friendsQueryPathField], 1); assert.isTrue(!(rootQueryPathField in userDoc)); + + if (userDoc.name === 'User - 2') { + assert.equal(userDoc[adversaryQueryPathField], 1); + } + else { + assert.isTrue(!(adversaryQueryPathField in userDoc)); + } } }); diff --git a/lib/query/lib/createGraph.js b/lib/query/lib/createGraph.js index dc2d660..e88ece9 100755 --- a/lib/query/lib/createGraph.js +++ b/lib/query/lib/createGraph.js @@ -109,7 +109,9 @@ export function getNodeNamespace(node) { const parts = []; let n = node; while (n) { - parts.push(n.collection._name); + const name = n.linker ? n.linker.linkName : n.collection._name; + parts.push(name); + // console.log('linker', node.linker ? node.linker.linkName : node.collection._name); n = n.parent; } return parts.reverse().join('_'); diff --git a/lib/query/testing/bootstrap/fixtures.js b/lib/query/testing/bootstrap/fixtures.js index af97455..d70b579 100755 --- a/lib/query/testing/bootstrap/fixtures.js +++ b/lib/query/testing/bootstrap/fixtures.js @@ -90,6 +90,7 @@ _.range(USERS).forEach(idx => { const id = Users.insert({ name: `User - ${idx}`, friendIds, + subordinateIds: idx === 3 ? [friendIds[2]] : [], }); friendIds.push(id); diff --git a/lib/query/testing/bootstrap/users/links.js b/lib/query/testing/bootstrap/users/links.js index 1c8a47f..9533da2 100755 --- a/lib/query/testing/bootstrap/users/links.js +++ b/lib/query/testing/bootstrap/users/links.js @@ -6,4 +6,9 @@ Users.addLinks({ field: 'friendIds', type: 'many' }, + subordinates: { + collection: Users, + field: 'subordinateIds', + type: 'many' + } });