Page MenuHomeSealhub
Differential D213 Diff 1011 lib/datastore/query.js

Changeset View

Standalone View

View Options

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-orlikUnsubmitted
Not Done
Inline Actions

Dlaczego nie zwracamy tutaj już this? Nie można przez to chainować, prawda?

kuba-orlik: Dlaczego nie zwracamy tutaj już `this`? Nie można przez to chainować, prawda?
piotr-ptaszynskiAuthorUnsubmitted
Not Done
Inline Actions

Wydaje 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) {
kuba-orlikUnsubmitted
Done
Inline Actions

Jeż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 11 LinesQuery.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();
const combined_match = {};
for (let stage of stages) {
if (stage.$lookup) {
this._lookup(stage);
} else if (stage.$match) {
Object.assign(combined_match, stage.$match);
} else {
throw new Error("Unsupported query: " + Object.keys(stage));
}
}
if (Object.keys(combined_match).length)
this.matches.push(combined_match);
}
_lookup(stage) {
const id = stage.$lookup.as;
this.lookups[id] = stage;
}
dump() {
return Object.values(this.lookups).concat({
$match: { $or: this.matches },
});
}
toPipeline() {
return Object.values(this.lookups)
.reduce((acc, stage) => this._pushToPipeline(acc, stage), [])
.concat({
$match: { $or: this.matches },
});
}
match(body) {
return Query.fromCustomPipeline([
{ $match: body },
...this.toPipeline(),
]);
}
};
Query.And = class extends Query {
constructor(...queries) {
super();
this.stages = queries
.map(query => query.stages)
.reduce((acc, stages) => acc.concat(stages), []);
}
};
Query.Not = class extends Query { Query.Not = class extends Query {
constructor(query) { constructor(query) {
super(); super();
query.toPipeline().map(stage => { query.toPipeline().map(stage => {
if (!stage.$match) { if (!stage.$match) {
return stage; return stage;
} }
for (let field_name in stage.$match) { for (let field_name in stage.$match) {
this.match({ this.match({
[field_name]: { $not: stage.$match[field_name] }, [field_name]: { $not: stage.$match[field_name] },
}); });
} }
}); });
} }
}; };
kuba-orlikUnsubmitted
Not Done
Inline Actions

Przy 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");
kuba-orlikUnsubmitted
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…
kuba-orlikUnsubmitted
Done
Inline Actions

Plik 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ą…
kuba-orlikUnsubmitted
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…
kuba-orlikUnsubmitted
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…
kuba-orlikUnsubmitted
Done
Inline Actions

Strategii 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…