-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Adds support for multiple $in, pointer / relation queries on $or #769
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -397,31 +397,33 @@ DatabaseController.prototype.owningIds = function(className, key, relatedIds) { | |
// Modifies query so that it no longer has $in on relation fields, or | ||
// equal-to-pointer constraints on relation fields. | ||
// Returns a promise that resolves when query is mutated | ||
// TODO: this only handles one of these at a time - make it handle more | ||
DatabaseController.prototype.reduceInRelation = function(className, query, schema) { | ||
// Search for an in-relation or equal-to-relation | ||
for (var key in query) { | ||
if (query[key] && | ||
(query[key]['$in'] || query[key].__type == 'Pointer')) { | ||
var t = schema.getExpectedType(className, key); | ||
var match = t ? t.match(/^relation<(.*)>$/) : false; | ||
if (!match) { | ||
continue; | ||
} | ||
var relatedClassName = match[1]; | ||
var relatedIds; | ||
if (query[key]['$in']) { | ||
relatedIds = query[key]['$in'].map(r => r.objectId); | ||
} else { | ||
relatedIds = [query[key].objectId]; | ||
// Make it sequential for now, not sure of paralleization side effects | ||
return Object.keys(query).reduce((promise, key) => { | ||
return promise.then(() => { | ||
if (query[key] && | ||
(query[key]['$in'] || query[key].__type == 'Pointer')) { | ||
let t = schema.getExpectedType(className, key); | ||
let match = t ? t.match(/^relation<(.*)>$/) : false; | ||
if (!match) { | ||
return; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should produce an inconsistent return type from the function, not sure if that is expected behavior or not. You are returning There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's enclosed into a promise.then() , so that's pretty much the same as doing return Promise.resolve(); There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Still an inconsistent return type from a closure(lambda?), but nature of JS wins here. |
||
} | ||
let relatedClassName = match[1]; | ||
let relatedIds; | ||
if (query[key]['$in']) { | ||
relatedIds = query[key]['$in'].map(r => r.objectId); | ||
} else { | ||
relatedIds = [query[key].objectId]; | ||
} | ||
return this.owningIds(className, key, relatedIds).then((ids) => { | ||
delete query[key]; | ||
query.objectId = Object.assign({'$in': []}, query.objectId); | ||
query.objectId['$in'] = query.objectId['$in'].concat(ids); | ||
}); | ||
} | ||
return this.owningIds(className, key, relatedIds).then((ids) => { | ||
delete query[key]; | ||
query.objectId = {'$in': ids}; | ||
}); | ||
} | ||
} | ||
return Promise.resolve(); | ||
}); | ||
}, Promise.resolve()); | ||
}; | ||
|
||
// Modifies query so that it no longer has $relatedTo | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest using
Promise.all
rather thanreduce
when possible, you should get slightly better perf that way, and IMO it's more readable.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering about race conditions in that case, but that's fine for me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all
can lead to race conditions, but I think in this case it won't.