Changeset View
Standalone View
lib/datastore/query.js
| "use strict"; | "use strict"; | ||||
| const Promise = require("bluebird"); | const object_hash = require("object-hash"); | ||||
| const hash_item = value => | const QueryStep = require("./query-step.js"); | ||||
| require("object-hash")(value, { | const transformObject = require("../utils/transform-object.js"); | ||||
| algorithm: "md5", | |||||
| excludeKeys: key => key === "as", | |||||
| }); | |||||
| class Query { | class Query { | ||||
| constructor() { | constructor() { | ||||
| this.stages = []; | this.steps = []; | ||||
| } | } | ||||
| lookup(body, unwind = true) { | lookup(body) { | ||||
| body.as = hash_item(body); | const lookup_step = new QueryStep.Lookup(body); | ||||
| this.stages.push({ $lookup: body, unwinds: unwind }); | this.steps.push(lookup_step); | ||||
| return body.as; | return lookup_step.hash(); | ||||
| } | } | ||||
| match(body) { | match(body) { | ||||
| this.stages.push({ $match: body }); | for (let key of Object.keys(body)) { | ||||
| return this; | this.steps.push(new QueryStep.Match({ [key]: body[key] })); | ||||
| } | |||||
kuba-orlik: Dlaczego nie zwracamy tutaj już `this`? Nie można przez to chainować, prawda? | |||||
Not Done Inline ActionsWydaje mi się, że wprowadzało by to za dużo zamieszania. lookup nie może zwracać this, bo musi zwracać odpowiedniego hasza piotr-ptaszynski: Wydaje mi się, że wprowadzało by to za dużo zamieszania. `lookup` nie może zwracać `this`, bo… | |||||
| } | } | ||||
| dump() { | dump() { | ||||
| return this.stages; | return this.steps; | ||||
| } | } | ||||
| toPipeline() { | toPipeline() { | ||||
| return this.stages.reduce( | return this.steps.reduce( | ||||
| (acc, stage) => this._pushToPipeline(acc, stage), | (pipeline, query_step) => query_step.pushStage(pipeline), | ||||
| [] | [] | ||||
| ); | ); | ||||
| } | } | ||||
| _pushToPipeline(pipeline, stage) { | |||||
| if (!stage.$lookup) { | |||||
| pipeline.push(stage); | |||||
| } else { | |||||
| pipeline.push({ $lookup: stage.$lookup }); | |||||
| if (stage.unwinds) { | |||||
| pipeline.push({ $unwind: "$" + stage.$lookup.as }); | |||||
| } | |||||
| } | |||||
| return pipeline; | |||||
| } | |||||
| static fromSingleMatch(body) { | static fromSingleMatch(body) { | ||||
| const query = new Query(); | const query = new Query(); | ||||
| return query.match(body); | query.match(body); | ||||
| return query; | |||||
| } | } | ||||
| static fromCustomPipeline(stages) { | static fromCustomPipeline(stages) { | ||||
| const query = new Query(); | const query = new Query(); | ||||
| query.stages = stages; | let steps; | ||||
| const field_as_to_hash = {}; | |||||
| for (let i = 0; i < stages.length; ++i) { | |||||
| if (stages[i].$unwind) { | |||||
| continue; | |||||
| } | |||||
| const stage = transformObject( | |||||
| stages[i], | |||||
| prop => { | |||||
| if (prop.startsWith("$")) { | |||||
| return prop; | |||||
| } | |||||
| const fields = prop.split("."); | |||||
| return fields | |||||
| .map(field => field_as_to_hash[field] || field) | |||||
| .join("."); | |||||
| }, | |||||
| (prop, value) => { | |||||
| let fields; | |||||
| if (typeof value !== "string") { | |||||
| return value; | |||||
| } | |||||
| if (prop === "localField") { | |||||
| fields = value.split("."); | |||||
| } else if (value.startsWith("$")) { | |||||
| fields = value.substring(1).split("."); | |||||
| } else { | |||||
| return value; | |||||
| } | |||||
| return fields | |||||
| .map(field => field_as_to_hash[field] || field) | |||||
| .join("."); | |||||
| } | |||||
| ); | |||||
| steps = QueryStep.fromStage(stage, query._isUnwindStage(stages, i)); | |||||
| if (stage.$lookup) { | |||||
| const field_as = stage.$lookup.as; | |||||
| field_as_to_hash[field_as] = steps[0].hash(); | |||||
| } | |||||
| query.steps.push(...steps); | |||||
| } | |||||
| return query; | return query; | ||||
| } | } | ||||
| _isUnwindStage(stages, i) { | |||||
Done Inline ActionsJeżeli dobrze rozumiem tę funkcję, to imho nazwa _isUnwindStage pasowałaby lepiej ;) I mam pytanie - dlaczego poniżej sprawdzamy dokładnie wartość .$unwind? Nie wystarczyłoby sprawdzić, czy to pole istnieje? kuba-orlik: Jeżeli dobrze rozumiem tę funkcję, to imho nazwa `_isUnwindStage` pasowałaby lepiej ;)
I mam… | |||||
| if (!stages[i].$lookup) { | |||||
| return false; | |||||
| } | |||||
| return stages[i + 1] && stages[i + 1].$unwind; | |||||
| } | } | ||||
| } | |||||
| module.exports = Query; | |||||
| Query.DenyAll = class extends Query { | Query.DenyAll = class extends Query { | ||||
| constructor() { | constructor() { | ||||
| super(); | super(); | ||||
| super.match({ _id: { $exists: false } }); | super.match({ _id: { $exists: false } }); | ||||
| } | } | ||||
| lookup() { | lookup() { | ||||
| throw new Error("The query is not mutable!"); | throw new Error("The query is not mutable!"); | ||||
| Show All 10 Lines | Query.AllowAll = class extends Query { | ||||
| } | } | ||||
| lookup() { | lookup() { | ||||
| throw new Error("The query is not mutable!"); | throw new Error("The query is not mutable!"); | ||||
| } | } | ||||
| match() { | match() { | ||||
| throw new Error("The query is not mutable!"); | throw new Error("The query is not mutable!"); | ||||
| } | } | ||||
| }; | }; | ||||
| Query.Or = class extends Query { | Query.Or = require("./query_or.js"); | ||||
| constructor(...queries) { | |||||
| super(); | |||||
| this.lookups = {}; | |||||
| this.matches = []; | |||||
| for (let query of queries) { | |||||
| this.addQuery(query); | |||||
| } | |||||
| } | |||||
| addQuery(query) { | |||||
| let stages = query.dump(); | |||||
| for (let stage of stages) { | |||||
| if (stage.$lookup) { | |||||
| this._lookup(stage); | |||||
| } else if (stage.$match) { | |||||
| this._match(stage); | |||||
| } else { | |||||
| throw new Error("Unsupported query: " + Object.keys(stage)); | |||||
| } | |||||
| } | |||||
| } | |||||
| _lookup(stage) { | |||||
| const id = stage.$lookup.as; | |||||
| this.lookups[id] = stage; | |||||
| } | |||||
| _match(stage) { | |||||
| this.matches.push(stage); | |||||
| } | |||||
| dump() { | |||||
| return Object.values(this.lookups).concat({ | |||||
| $match: { $or: this.matches.map(stage => stage.$match) }, | |||||
| }); | |||||
| } | |||||
| toPipeline() { | |||||
| return Object.values(this.lookups) | |||||
| .reduce((acc, stage) => this._pushToPipeline(acc, stage), []) | |||||
| .concat({ | |||||
| $match: { $or: this.matches.map(stage => stage.$match) }, | |||||
| }); | |||||
| } | |||||
| }; | |||||
Not Done Inline ActionsPrzy testach zdałem sobie sprawę, że Query.Not nie działa poprawnie: > a = new Query();
Query { steps: [] }
> a.match({a: "A"})
undefined
> a.dump()
[ QueryStep { body: { a: 'A' } } ]
> const b = new Query()
undefined
> b.match({b: "B"})
undefined
> b.dump()
[ QueryStep { body: { b: 'B' } } ]
> new Query.Not(new Query.And(a, b)).dump()
[{"body":{"a":{"$not":"A"}}},{"body":{"b":{"$not":"B"}}}]W skrócie: ~(a ^ b) zwraca ~a ^ ~b, kiedy powinno ~a OR ~b Nie wiem, czy ta uwaga powinna tyczyć się tego konkretnie diffa, czy lepiej założyć na to osobnego taska - ale na pewno trzeba to poprawić. Zrób wedle uznania - jeżeli uważasz, że lepiej w osobnym tasku, to proszę od razu takowego załóż :) kuba-orlik: Przy testach zdałem sobie sprawę, że `Query.Not` nie działa poprawnie:
```
> a = new Query()… | |||||
| module.exports = Query; | Query.And = require("./query_and.js"); | ||||
Done Inline Actions(should) dlaczego traktujemy Or wyjątkowo? Myśle, że warto unikać takich przypadków brzegowych ;) kuba-orlik: (should) dlaczego traktujemy `Or` wyjątkowo? Myśle, że warto unikać takich przypadków… | |||||
Done Inline ActionsPlik query.js stał się bardzo duży. Warto AND przenieść do osobnego pliku. Jeżeli pojawią się cykliczne zależności, to pomoże trick: module.exports = Query;
Query.And = require("./and.js")(module.exports przed requirem) kuba-orlik: Plik `query.js` stał się bardzo duży. Warto `AND` przenieść do osobnego pliku. Jeżeli pojawią… | |||||
Done Inline Actions(question) Z tego co widzę, nie rozbijamy tutaj query na osobne $match-e, tak jak to ma w przypadku metodu _match. Czy to jest zamierzone? kuba-orlik: (question) Z tego co widzę, nie rozbijamy tutaj query na osobne `$match`-e, tak jak to ma w… | |||||
Done Inline Actions(should) Wciąż myli mi się, czy mamy do czynienia z stage w znaczeniu Mongowym, czy stage w znaczeniu "nasza wewnętrzna reprezentacja kroku agregacji". Myślę, że można to naprawić nazwami zmiennych - stage zostawiłbym do mongowych kroków agregacji, a dla naszej wewnętrznej stworzył nową nazwę - albo nawet klasę kuba-orlik: (should) Wciąż myli mi się, czy mamy do czynienia z `stage` w znaczeniu Mongowym, czy `stage` w… | |||||
Done Inline ActionsStrategii dostępu And brakuje jeszcze trochę elegancji. Podejrzewam że jest to spowodowane tym, że ta klasa ma dużo metod odpowiedzialnych za stwierdzanie, czy dany "stage" znajduje się w grafie itp. Myślę, że jak się je przeniesie do osobnej klasy odpowiedzialnej za pośrednią reprezentację "stage"'ów to niniejsza klasa zyska na elegancji kuba-orlik: Strategii dostępu `And` brakuje jeszcze trochę elegancji. Podejrzewam że jest to spowodowane… | |||||
Dlaczego nie zwracamy tutaj już this? Nie można przez to chainować, prawda?