From 3109233d4f835261d6f308ec1be0d39d00946f94 Mon Sep 17 00:00:00 2001 From: zadam Date: Wed, 22 Jul 2020 22:52:15 +0200 Subject: [PATCH] better reporting for search parsing errors --- package-lock.json | 6 +-- package.json | 2 +- spec/search/parser.spec.js | 46 +++++++++++-------- src/services/search/parsing_context.js | 1 + src/services/search/services/handle_parens.js | 6 +-- src/services/search/services/parse.js | 24 +++++++--- src/services/search/services/search.js | 3 +- 7 files changed, 53 insertions(+), 35 deletions(-) diff --git a/package-lock.json b/package-lock.json index 374aa5fd5..02c1da738 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2605,9 +2605,9 @@ } }, "dayjs": { - "version": "1.8.29", - "resolved": "https://registry.npmjs.org/dayjs/-/dayjs-1.8.29.tgz", - "integrity": "sha512-Vm6teig8ZWK7rH/lxzVGxZJCljPdmUr6q/3f4fr5F0VWNGVkZEjZOQJsAN8hUHUqn+NK4XHNEpJZS1MwLyDcLw==" + "version": "1.8.30", + "resolved": "https://registry.npmjs.org/dayjs/-/dayjs-1.8.30.tgz", + "integrity": "sha512-5s5IGuP5bVvIbOWkEDcfmXsUj24fZW1NMHVVSdSFF/kW8d+alZcI9SpBKC+baEyBe+z3fUp17y75ulstv5swUw==" }, "debug": { "version": "4.1.1", diff --git a/package.json b/package.json index f3f577880..e7a0485dd 100644 --- a/package.json +++ b/package.json @@ -31,7 +31,7 @@ "commonmark": "0.29.1", "cookie-parser": "1.4.5", "csurf": "1.11.0", - "dayjs": "1.8.29", + "dayjs": "1.8.30", "debug": "4.1.1", "ejs": "3.1.3", "electron-debug": "3.1.0", diff --git a/spec/search/parser.spec.js b/spec/search/parser.spec.js index c0fceb48b..652f94985 100644 --- a/spec/search/parser.spec.js +++ b/spec/search/parser.spec.js @@ -1,15 +1,19 @@ const ParsingContext = require("../../src/services/search/parsing_context.js"); const parse = require('../../src/services/search/services/parse.js'); -function tokens(...args) { - return args.map(arg => { +function tokens(toks, cur = 0) { + return toks.map(arg => { if (Array.isArray(arg)) { - return arg; + return tokens(arg, cur); } else { + cur += arg.length; + return { token: arg, - inQuotes: false + inQuotes: false, + startIndex: cur - arg.length, + endIndex: cur - 1 }; } }); @@ -18,7 +22,7 @@ function tokens(...args) { describe("Parser", () => { it("fulltext parser without content", () => { const rootExp = parse({ - fulltextTokens: tokens("hello", "hi"), + fulltextTokens: tokens(["hello", "hi"]), expressionTokens: [], parsingContext: new ParsingContext({includeNoteContent: false}) }); @@ -29,7 +33,7 @@ describe("Parser", () => { it("fulltext parser with content", () => { const rootExp = parse({ - fulltextTokens: tokens("hello", "hi"), + fulltextTokens: tokens(["hello", "hi"]), expressionTokens: [], parsingContext: new ParsingContext({includeNoteContent: true}) }); @@ -50,7 +54,7 @@ describe("Parser", () => { it("simple label comparison", () => { const rootExp = parse({ fulltextTokens: [], - expressionTokens: tokens("#mylabel", "=", "text"), + expressionTokens: tokens(["#mylabel", "=", "text"]), parsingContext: new ParsingContext() }); @@ -63,7 +67,7 @@ describe("Parser", () => { it("simple attribute negation", () => { let rootExp = parse({ fulltextTokens: [], - expressionTokens: tokens("#!mylabel"), + expressionTokens: tokens(["#!mylabel"]), parsingContext: new ParsingContext() }); @@ -74,7 +78,7 @@ describe("Parser", () => { rootExp = parse({ fulltextTokens: [], - expressionTokens: tokens("~!myrelation"), + expressionTokens: tokens(["~!myrelation"]), parsingContext: new ParsingContext() }); @@ -87,7 +91,7 @@ describe("Parser", () => { it("simple label AND", () => { const rootExp = parse({ fulltextTokens: [], - expressionTokens: tokens("#first", "=", "text", "and", "#second", "=", "text"), + expressionTokens: tokens(["#first", "=", "text", "and", "#second", "=", "text"]), parsingContext: new ParsingContext(true) }); @@ -104,7 +108,7 @@ describe("Parser", () => { it("simple label AND without explicit AND", () => { const rootExp = parse({ fulltextTokens: [], - expressionTokens: tokens("#first", "=", "text", "#second", "=", "text"), + expressionTokens: tokens(["#first", "=", "text", "#second", "=", "text"]), parsingContext: new ParsingContext() }); @@ -121,7 +125,7 @@ describe("Parser", () => { it("simple label OR", () => { const rootExp = parse({ fulltextTokens: [], - expressionTokens: tokens("#first", "=", "text", "or", "#second", "=", "text"), + expressionTokens: tokens(["#first", "=", "text", "or", "#second", "=", "text"]), parsingContext: new ParsingContext() }); @@ -137,8 +141,8 @@ describe("Parser", () => { it("fulltext and simple label", () => { const rootExp = parse({ - fulltextTokens: tokens("hello"), - expressionTokens: tokens("#mylabel", "=", "text"), + fulltextTokens: tokens(["hello"]), + expressionTokens: tokens(["#mylabel", "=", "text"]), parsingContext: new ParsingContext() }); @@ -155,7 +159,7 @@ describe("Parser", () => { it("label sub-expression", () => { const rootExp = parse({ fulltextTokens: [], - expressionTokens: tokens("#first", "=", "text", "or", tokens("#second", "=", "text", "and", "#third", "=", "text")), + expressionTokens: tokens(["#first", "=", "text", "or", ["#second", "=", "text", "and", "#third", "=", "text"]]), parsingContext: new ParsingContext() }); @@ -182,7 +186,7 @@ describe("Invalid expressions", () => { parse({ fulltextTokens: [], - expressionTokens: tokens("#first", "="), + expressionTokens: tokens(["#first", "="]), parsingContext }); @@ -191,24 +195,26 @@ describe("Invalid expressions", () => { it("comparison between labels is impossible", () => { let parsingContext = new ParsingContext(); + parsingContext.originalQuery = "#first = #second"; parse({ fulltextTokens: [], - expressionTokens: tokens("#first", "=", "#second"), + expressionTokens: tokens(["#first", "=", "#second"]), parsingContext }); - expect(parsingContext.error).toEqual(`Error near token "#second", it's possible to compare with constant only.`); + expect(parsingContext.error).toEqual(`Error near token "#second" in "#first = #second", it's possible to compare with constant only.`); parsingContext = new ParsingContext(); + parsingContext.originalQuery = "#first = note.relations.second"; parse({ fulltextTokens: [], - expressionTokens: tokens("#first", "=", "note", ".", "relations", "second"), + expressionTokens: tokens(["#first", "=", "note", ".", "relations", "second"]), parsingContext }); - expect(parsingContext.error).toEqual(`Error near token "note", it's possible to compare with constant only.`); + expect(parsingContext.error).toEqual(`Error near token "note" in "#first = note.relations.s...", it's possible to compare with constant only.`); const rootExp = parse({ fulltextTokens: [], diff --git a/src/services/search/parsing_context.js b/src/services/search/parsing_context.js index 59bc487d8..7867ed186 100644 --- a/src/services/search/parsing_context.js +++ b/src/services/search/parsing_context.js @@ -5,6 +5,7 @@ class ParsingContext { this.includeNoteContent = !!params.includeNoteContent; this.fuzzyAttributeSearch = !!params.fuzzyAttributeSearch; this.highlightedTokens = []; + this.originalQuery = ""; this.error = null; } diff --git a/src/services/search/services/handle_parens.js b/src/services/search/services/handle_parens.js index c391b2892..6c5889c83 100644 --- a/src/services/search/services/handle_parens.js +++ b/src/services/search/services/handle_parens.js @@ -1,7 +1,7 @@ /** * This will create a recursive object from list of tokens - tokens between parenthesis are grouped in a single array */ -function handle_parens(tokens) { +function handleParens(tokens) { if (tokens.length === 0) { return []; } @@ -34,10 +34,10 @@ function handle_parens(tokens) { tokens = [ ...tokens.slice(0, leftIdx), - handle_parens(tokens.slice(leftIdx + 1, rightIdx)), + handleParens(tokens.slice(leftIdx + 1, rightIdx)), ...tokens.slice(rightIdx + 1) ]; } } -module.exports = handle_parens; +module.exports = handleParens; diff --git a/src/services/search/services/parse.js b/src/services/search/services/parse.js index 020e84fe9..2c1cfaa64 100644 --- a/src/services/search/services/parse.js +++ b/src/services/search/services/parse.js @@ -51,6 +51,16 @@ function getExpression(tokens, parsingContext, level = 0) { let i; + function context(i) { + let {startIndex, endIndex} = tokens[i]; + startIndex = Math.max(0, startIndex - 20); + endIndex = Math.min(parsingContext.originalQuery.length, endIndex + 20); + + return '"' + (startIndex !== 0 ? "..." : "") + + parsingContext.originalQuery.substr(startIndex, endIndex - startIndex) + + (endIndex !== parsingContext.originalQuery.length ? "..." : "") + '"'; + } + function parseNoteProperty() { if (tokens[i].token !== '.') { parsingContext.addError('Expected "." to separate field path'); @@ -65,7 +75,7 @@ function getExpression(tokens, parsingContext, level = 0) { const operator = tokens[i].token; if (!isOperator(operator)) { - parsingContext.addError(`After content expected operator, but got "${tokens[i].token}"`); + parsingContext.addError(`After content expected operator, but got "${tokens[i].token}" in ${context(i)}`); return; } @@ -97,7 +107,7 @@ function getExpression(tokens, parsingContext, level = 0) { if (tokens[i].token === 'labels') { if (tokens[i + 1].token !== '.') { - parsingContext.addError(`Expected "." to separate field path, got "${tokens[i + 1].token}"`); + parsingContext.addError(`Expected "." to separate field path, got "${tokens[i + 1].token}" in ${context(i)}`); return; } @@ -108,7 +118,7 @@ function getExpression(tokens, parsingContext, level = 0) { if (tokens[i].token === 'relations') { if (tokens[i + 1].token !== '.') { - parsingContext.addError(`Expected "." to separate field path, got "${tokens[i + 1].token}"`); + parsingContext.addError(`Expected "." to separate field path, got "${tokens[i + 1].token}" in ${context(i)}`); return; } @@ -124,7 +134,7 @@ function getExpression(tokens, parsingContext, level = 0) { const comparator = comparatorBuilder(operator, comparedValue); if (!comparator) { - parsingContext.addError(`Can't find operator '${operator}'`); + parsingContext.addError(`Can't find operator '${operator}' in ${context(i)}`); return; } @@ -133,7 +143,7 @@ function getExpression(tokens, parsingContext, level = 0) { return new PropertyComparisonExp(propertyName, comparator); } - parsingContext.addError(`Unrecognized note property "${tokens[i].token}"`); + parsingContext.addError(`Unrecognized note property "${tokens[i].token}" in ${context(i)}`); } function parseAttribute(name) { @@ -161,7 +171,7 @@ function getExpression(tokens, parsingContext, level = 0) { if (!tokens[i + 2].inQuotes && (comparedValue.startsWith('#') || comparedValue.startsWith('~') || comparedValue === 'note')) { - parsingContext.addError(`Error near token "${comparedValue}", it's possible to compare with constant only.`); + parsingContext.addError(`Error near token "${comparedValue}" in ${context(i)}, it's possible to compare with constant only.`); return; } @@ -174,7 +184,7 @@ function getExpression(tokens, parsingContext, level = 0) { const comparator = comparatorBuilder(operator, comparedValue); if (!comparator) { - parsingContext.addError(`Can't find operator '${operator}'`); + parsingContext.addError(`Can't find operator '${operator}' in ${context(i)}`); } else { i += 2; diff --git a/src/services/search/services/search.js b/src/services/search/services/search.js index e76481b1c..d3eeb1c3d 100644 --- a/src/services/search/services/search.js +++ b/src/services/search/services/search.js @@ -57,7 +57,8 @@ function parseQueryToExpression(query, parsingContext) { const expression = parse({ fulltextTokens, expressionTokens: structuredExpressionTokens, - parsingContext + parsingContext, + originalQuery: query }); return expression;