diff --git a/spec/note_cache_mocking.js b/spec/note_cache_mocking.js new file mode 100644 index 000000000..e96252dfd --- /dev/null +++ b/spec/note_cache_mocking.js @@ -0,0 +1,70 @@ +const Note = require('../src/services/note_cache/entities/note'); +const Branch = require('../src/services/note_cache/entities/branch'); +const Attribute = require('../src/services/note_cache/entities/attribute'); +const noteCache = require('../src/services/note_cache/note_cache'); +const randtoken = require('rand-token').generator({source: 'crypto'}); + +/** @return {Note} */ +function findNoteByTitle(searchResults, title) { + return searchResults + .map(sr => noteCache.notes[sr.noteId]) + .find(note => note.title === title); +} + +class NoteBuilder { + constructor(note) { + this.note = note; + } + + label(name, value = '', isInheritable = false) { + new Attribute(noteCache, { + attributeId: id(), + noteId: this.note.noteId, + type: 'label', + isInheritable, + name, + value + }); + + return this; + } + + relation(name, targetNote) { + new Attribute(noteCache, { + attributeId: id(), + noteId: this.note.noteId, + type: 'relation', + name, + value: targetNote.noteId + }); + + return this; + } + + child(childNoteBuilder, prefix = "") { + new Branch(noteCache, { + branchId: id(), + noteId: childNoteBuilder.note.noteId, + parentNoteId: this.note.noteId, + prefix + }); + + return this; + } +} + +function id() { + return randtoken.generate(10); +} + +function note(title) { + const note = new Note(noteCache, {noteId: id(), title}); + + return new NoteBuilder(note); +} + +module.exports = { + NoteBuilder, + findNoteByTitle, + note +}; diff --git a/spec/search.spec.js b/spec/search.spec.js index 25a812216..2a20a4ba9 100644 --- a/spec/search.spec.js +++ b/spec/search.spec.js @@ -5,7 +5,7 @@ const Attribute = require('../src/services/note_cache/entities/attribute'); const ParsingContext = require('../src/services/search/parsing_context'); const dateUtils = require('../src/services/date_utils'); const noteCache = require('../src/services/note_cache/note_cache'); -const randtoken = require('rand-token').generator({source: 'crypto'}); +const {NoteBuilder, findNoteByTitle, note} = require('./note_cache_mocking'); describe("Search", () => { let rootNote; @@ -463,63 +463,36 @@ describe("Search", () => { await test("relationCount", "1", 1); await test("relationCount", "2", 0); }); + + it("test order by", async () => { + const italy = note("Italy").label("capital", "Rome"); + const slovakia = note("Slovakia").label("capital", "Bratislava"); + const austria = note("Austria").label("capital", "Vienna"); + const ukraine = note("Ukraine").label("capital", "Kiev"); + + rootNote + .child(note("Europe") + .child(ukraine) + .child(slovakia) + .child(austria) + .child(italy)); + + const parsingContext = new ParsingContext(); + + let searchResults = await searchService.findNotesWithQuery('# note.parents.title = Europe orderBy note.title', parsingContext); + expect(searchResults.length).toEqual(4); + expect(noteCache.notes[searchResults[0].noteId].title).toEqual("Austria"); + expect(noteCache.notes[searchResults[1].noteId].title).toEqual("Italy"); + expect(noteCache.notes[searchResults[2].noteId].title).toEqual("Slovakia"); + expect(noteCache.notes[searchResults[3].noteId].title).toEqual("Ukraine"); + + searchResults = await searchService.findNotesWithQuery('# note.parents.title = Europe orderBy note.labels.capital', parsingContext); + expect(searchResults.length).toEqual(4); + expect(noteCache.notes[searchResults[0].noteId].title).toEqual("Slovakia"); + expect(noteCache.notes[searchResults[1].noteId].title).toEqual("Ukraine"); + expect(noteCache.notes[searchResults[2].noteId].title).toEqual("Italy"); + expect(noteCache.notes[searchResults[3].noteId].title).toEqual("Austria"); + }); + + // FIXME: test what happens when we order without any filter criteria }); - -/** @return {Note} */ -function findNoteByTitle(searchResults, title) { - return searchResults - .map(sr => noteCache.notes[sr.noteId]) - .find(note => note.title === title); -} - -class NoteBuilder { - constructor(note) { - this.note = note; - } - - label(name, value = '', isInheritable = false) { - new Attribute(noteCache, { - attributeId: id(), - noteId: this.note.noteId, - type: 'label', - isInheritable, - name, - value - }); - - return this; - } - - relation(name, targetNote) { - new Attribute(noteCache, { - attributeId: id(), - noteId: this.note.noteId, - type: 'relation', - name, - value: targetNote.noteId - }); - - return this; - } - - child(childNoteBuilder, prefix = "") { - new Branch(noteCache, { - branchId: id(), - noteId: childNoteBuilder.note.noteId, - parentNoteId: this.note.noteId, - prefix - }); - - return this; - } -} - -function id() { - return randtoken.generate(10); -} - -function note(title) { - const note = new Note(noteCache, {noteId: id(), title}); - - return new NoteBuilder(note); -} diff --git a/spec/value_extractor.spec.js b/spec/value_extractor.spec.js new file mode 100644 index 000000000..40a9e193e --- /dev/null +++ b/spec/value_extractor.spec.js @@ -0,0 +1,86 @@ +const {NoteBuilder, findNoteByTitle, note} = require('./note_cache_mocking'); +const ValueExtractor = require('../src/services/search/value_extractor'); +const noteCache = require('../src/services/note_cache/note_cache'); + +describe("Value extractor", () => { + beforeEach(() => { + noteCache.reset(); + }); + + it("simple title extraction", async () => { + const europe = note("Europe").note; + + const valueExtractor = new ValueExtractor(["note", "title"]); + + expect(valueExtractor.validate()).toBeFalsy(); + expect(valueExtractor.extract(europe)).toEqual("Europe"); + }); + + it("label extraction", async () => { + const austria = note("Austria") + .label("Capital", "Vienna") + .note; + + let valueExtractor = new ValueExtractor(["note", "labels", "capital"]); + + expect(valueExtractor.validate()).toBeFalsy(); + expect(valueExtractor.extract(austria)).toEqual("vienna"); + + valueExtractor = new ValueExtractor(["#capital"]); + + expect(valueExtractor.validate()).toBeFalsy(); + expect(valueExtractor.extract(austria)).toEqual("vienna"); + }); + + it("parent/child property extraction", async () => { + const vienna = note("Vienna"); + const europe = note("Europe") + .child(note("Austria") + .child(vienna)); + + let valueExtractor = new ValueExtractor(["note", "children", "children", "title"]); + + expect(valueExtractor.validate()).toBeFalsy(); + expect(valueExtractor.extract(europe.note)).toEqual("Vienna"); + + valueExtractor = new ValueExtractor(["note", "parents", "parents", "title"]); + + expect(valueExtractor.validate()).toBeFalsy(); + expect(valueExtractor.extract(vienna.note)).toEqual("Europe"); + }); + + it("extract through relation", async () => { + const czechRepublic = note("Czech Republic").label("capital", "Prague"); + const slovakia = note("Slovakia").label("capital", "Bratislava"); + const austria = note("Austria") + .relation('neighbor', czechRepublic.note) + .relation('neighbor', slovakia.note); + + let valueExtractor = new ValueExtractor(["note", "relations", "neighbor", "labels", "capital"]); + + expect(valueExtractor.validate()).toBeFalsy(); + expect(valueExtractor.extract(austria.note)).toEqual("prague"); + + valueExtractor = new ValueExtractor(["~neighbor", "labels", "capital"]); + + expect(valueExtractor.validate()).toBeFalsy(); + expect(valueExtractor.extract(austria.note)).toEqual("prague"); + }); +}); + +describe("Invalid value extractor property path", () => { + it('each path must start with "note" (or label/relation)', + () => expect(new ValueExtractor(["neighbor"]).validate()).toBeTruthy()); + + it("extra path element after terminal label", + () => expect(new ValueExtractor(["~neighbor", "labels", "capital", "noteId"]).validate()).toBeTruthy()); + + it("extra path element after terminal title", + () => expect(new ValueExtractor(["note", "title", "isProtected"]).validate()).toBeTruthy()); + + it("relation name and note property is missing", + () => expect(new ValueExtractor(["note", "relations"]).validate()).toBeTruthy()); + + it("relation is specified but target note property is not specified", + () => expect(new ValueExtractor(["note", "relations", "myrel"]).validate()).toBeTruthy()); +}); diff --git a/src/services/note_cache/entities/note.js b/src/services/note_cache/entities/note.js index ec869d0d1..409589263 100644 --- a/src/services/note_cache/entities/note.js +++ b/src/services/note_cache/entities/note.js @@ -107,6 +107,18 @@ class Note { return this.attributes.find(attr => attr.type === type && attr.name === name); } + getLabelValue(name) { + const label = this.attributes.find(attr => attr.type === 'label' && attr.name === name); + + return label ? label.value : null; + } + + getRelationTarget(name) { + const relation = this.attributes.find(attr => attr.type === 'relation' && attr.name === name); + + return relation ? relation.targetNote : null; + } + get isArchived() { return this.hasAttribute('label', 'archived'); } diff --git a/src/services/search.js b/src/services/search.js index 774614ca8..2efa62984 100644 --- a/src/services/search.js +++ b/src/services/search.js @@ -1,3 +1,18 @@ +"use strict"; + +/** + * Missing things from the OLD search: + * - orderBy + * - limit + * - in - replaced with note.ancestors + * - content in attribute search + * - not - pherhaps not necessary + * + * other potential additions: + * - targetRelations - either named or not + * - any relation without name + */ + const repository = require('./repository'); const sql = require('./sql'); const log = require('./log'); diff --git a/src/services/search/expressions/order_by_and_limit.js b/src/services/search/expressions/order_by_and_limit.js new file mode 100644 index 000000000..6d1ad8ebc --- /dev/null +++ b/src/services/search/expressions/order_by_and_limit.js @@ -0,0 +1,58 @@ +"use strict"; + +const Expression = require('./expression'); +const NoteSet = require('../note_set'); + +class OrderByAndLimitExp extends Expression { + constructor(orderDefinitions, limit) { + super(); + + this.orderDefinitions = orderDefinitions; + + for (const od of this.orderDefinitions) { + od.smaller = od.direction === "asc" ? -1 : 1; + od.larger = od.direction === "asc" ? 1 : -1; + } + + this.limit = limit; + + /** @type {Expression} */ + this.subExpression = null; // it's expected to be set after construction + } + + execute(inputNoteSet, searchContext) { + let {notes} = this.subExpression.execute(inputNoteSet, searchContext); + + notes.sort((a, b) => { + for (const {valueExtractor, smaller, larger} of this.orderDefinitions) { + let valA = valueExtractor.extract(a); + let valB = valueExtractor.extract(b); + + if (!isNaN(valA) && !isNaN(valB)) { + valA = parseFloat(valA); + valB = parseFloat(valB); + } + + if (valA < valB) { + return smaller; + } else if (valA > valB) { + return larger; + } + // else go to next order definition + } + + return 0; + }); + + if (this.limit) { + notes = notes.slice(0, this.limit); + } + + const noteSet = new NoteSet(notes); + noteSet.sorted = true; + + return noteSet; + } +} + +module.exports = OrderByAndLimitExp; diff --git a/src/services/search/note_set.js b/src/services/search/note_set.js index 8488692c2..3f8dc9dcf 100644 --- a/src/services/search/note_set.js +++ b/src/services/search/note_set.js @@ -4,6 +4,8 @@ class NoteSet { constructor(notes = []) { /** @type {Note[]} */ this.notes = notes; + /** @type {boolean} */ + this.sorted = false; } add(note) { diff --git a/src/services/search/parser.js b/src/services/search/parser.js index adad9cece..73dd5ea55 100644 --- a/src/services/search/parser.js +++ b/src/services/search/parser.js @@ -12,7 +12,9 @@ const AttributeExistsExp = require('./expressions/attribute_exists'); const LabelComparisonExp = require('./expressions/label_comparison'); const NoteCacheFulltextExp = require('./expressions/note_cache_fulltext'); const NoteContentFulltextExp = require('./expressions/note_content_fulltext'); +const OrderByAndLimitExp = require('./expressions/order_by_and_limit'); const comparatorBuilder = require('./comparator_builder'); +const ValueExtractor = require('./value_extractor'); function getFulltext(tokens, parsingContext) { parsingContext.highlightedTokens.push(...tokens); @@ -35,7 +37,7 @@ function isOperator(str) { return str.match(/^[=<>*]+$/); } -function getExpression(tokens, parsingContext) { +function getExpression(tokens, parsingContext, level = 0) { if (tokens.length === 0) { return null; } @@ -104,7 +106,7 @@ function getExpression(tokens, parsingContext) { return; } - i += 3; + i += 2; return new PropertyComparisonExp(propertyName, comparator); } @@ -151,6 +153,57 @@ function getExpression(tokens, parsingContext) { } } + function parseOrderByAndLimit() { + const orderDefinitions = []; + let limit; + + if (tokens[i] === 'orderby') { + do { + const propertyPath = []; + let direction = "asc"; + + do { + i++; + + propertyPath.push(tokens[i]); + + i++; + } while (tokens[i] === '.'); + + if (["asc", "desc"].includes(tokens[i + 1])) { + direction = tokens[i + 1]; + i++; + } + + const valueExtractor = new ValueExtractor(propertyPath); + + if (valueExtractor.validate()) { + parsingContext.addError(valueExtractor.validate()); + } + + orderDefinitions.push({ + valueExtractor, + direction + }); + } while (tokens[i] === ','); + } + + if (tokens[i] === 'limit') { + limit = parseInt(tokens[i + 1]); + } + + return new OrderByAndLimitExp(orderDefinitions, limit); + } + + function getAggregateExpression() { + if (op === null || op === 'and') { + return AndExp.of(expressions); + } + else if (op === 'or') { + return OrExp.of(expressions); + } + } + for (i = 0; i < tokens.length; i++) { const token = tokens[i]; @@ -159,7 +212,7 @@ function getExpression(tokens, parsingContext) { } if (Array.isArray(token)) { - expressions.push(getExpression(token, parsingContext)); + expressions.push(getExpression(token, parsingContext, level++)); } else if (token.startsWith('#')) { const labelName = token.substr(1); @@ -171,6 +224,22 @@ function getExpression(tokens, parsingContext) { expressions.push(parseRelation(relationName)); } + else if (['orderby', 'limit'].includes(token)) { + if (level !== 0) { + parsingContext.addError('orderBy can appear only on the top expression level'); + continue; + } + + const exp = parseOrderByAndLimit(); + + if (!exp) { + continue; + } + + exp.subExpression = getAggregateExpression(); + + return exp; + } else if (token === 'note') { i++; @@ -198,12 +267,7 @@ function getExpression(tokens, parsingContext) { } } - if (op === null || op === 'and') { - return AndExp.of(expressions); - } - else if (op === 'or') { - return OrExp.of(expressions); - } + return getAggregateExpression(); } function parse({fulltextTokens, expressionTokens, parsingContext}) { diff --git a/src/services/search/parsing_context.js b/src/services/search/parsing_context.js index 59bc487d8..58ab77d39 100644 --- a/src/services/search/parsing_context.js +++ b/src/services/search/parsing_context.js @@ -12,6 +12,7 @@ class ParsingContext { // we record only the first error, subsequent ones are usually consequence of the first if (!this.error) { this.error = error; + console.log(this.error); } } } diff --git a/src/services/search/search.js b/src/services/search/search.js index 79517ae49..1e7437b58 100644 --- a/src/services/search/search.js +++ b/src/services/search/search.js @@ -34,15 +34,17 @@ async function findNotesWithExpression(expression) { .filter(notePathArray => notePathArray.includes(hoistedNoteService.getHoistedNoteId())) .map(notePathArray => new SearchResult(notePathArray)); - // sort results by depth of the note. This is based on the assumption that more important results - // are closer to the note root. - searchResults.sort((a, b) => { - if (a.notePathArray.length === b.notePathArray.length) { - return a.notePathTitle < b.notePathTitle ? -1 : 1; - } + if (!noteSet.sorted) { + // sort results by depth of the note. This is based on the assumption that more important results + // are closer to the note root. + searchResults.sort((a, b) => { + if (a.notePathArray.length === b.notePathArray.length) { + return a.notePathTitle < b.notePathTitle ? -1 : 1; + } - return a.notePathArray.length < b.notePathArray.length ? -1 : 1; - }); + return a.notePathArray.length < b.notePathArray.length ? -1 : 1; + }); + } return searchResults; } diff --git a/src/services/search/value_extractor.js b/src/services/search/value_extractor.js new file mode 100644 index 000000000..ea1bb38d8 --- /dev/null +++ b/src/services/search/value_extractor.js @@ -0,0 +1,110 @@ +"use strict"; + +/** + * Search string is lower cased for case insensitive comparison. But when retrieving properties + * we need case sensitive form so we have this translation object. + */ +const PROP_MAPPING = { + "noteid": "noteId", + "title": "title", + "type": "type", + "mime": "mime", + "isprotected": "isProtected", + "isarhived": "isArchived", + "datecreated": "dateCreated", + "datemodified": "dateModified", + "utcdatecreated": "utcDateCreated", + "utcdatemodified": "utcDateModified", + "contentlength": "contentLength", + "parentcount": "parentCount", + "childrencount": "childrenCount", + "attributecount": "attributeCount", + "labelcount": "labelCount", + "relationcount": "relationCount" +}; + +class ValueExtractor { + constructor(propertyPath) { + this.propertyPath = propertyPath.map(pathEl => pathEl.toLowerCase()); + + if (this.propertyPath[0].startsWith('#')) { + this.propertyPath = ['note', 'labels', this.propertyPath[0].substr(1), ...this.propertyPath.slice( 1, this.propertyPath.length)]; + } + else if (this.propertyPath[0].startsWith('~')) { + this.propertyPath = ['note', 'relations', this.propertyPath[0].substr(1), ...this.propertyPath.slice( 1, this.propertyPath.length)]; + } + } + + validate() { + if (this.propertyPath[0] !== 'note') { + return `property specifier must start with 'note', but starts with '${this.propertyPath[0]}'`; + } + + for (let i = 1; i < this.propertyPath.length; i++) { + const pathEl = this.propertyPath[i]; + + if (pathEl === 'labels') { + if (i !== this.propertyPath.length - 2) { + return `label is a terminal property specifier and must be at the end`; + } + + i++; + } + else if (pathEl === 'relations') { + if (i >= this.propertyPath.length - 2) { + return `relation name or property name is missing`; + } + + i++; + } + else if (pathEl in PROP_MAPPING) { + if (i !== this.propertyPath.length - 1) { + return `${pathEl} is a terminal property specifier and must be at the end`; + } + } + else if (!["parents", "children"].includes(pathEl)) { + return `Unrecognized property specifier ${pathEl}`; + } + } + } + + extract(note) { + let cursor = note; + + let i; + + const cur = () => this.propertyPath[i]; + + for (i = 0; i < this.propertyPath.length; i++) { + if (!cursor) { + return cursor; + } + + if (cur() === 'labels') { + i++; + + return cursor.getLabelValue(cur()); + } + + if (cur() === 'relations') { + i++; + + cursor = cursor.getRelationTarget(cur()); + } + else if (cur() === 'parents') { + cursor = cursor.parents[0]; + } + else if (cur() === 'children') { + cursor = cursor.children[0]; + } + else if (cur() in PROP_MAPPING) { + return cursor[PROP_MAPPING[cur()]]; + } + else { + // FIXME + } + } + } +} + +module.exports = ValueExtractor;