Skip to content

Commit

Permalink
All: Resolves #3871: Interpret only valid search filters (#5103)
Browse files Browse the repository at this point in the history
  • Loading branch information
JackGruber authored Jun 24, 2021
1 parent 5047cf1 commit 8cbcb78
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 12 deletions.
3 changes: 3 additions & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -1397,6 +1397,9 @@ packages/lib/services/searchengine/SearchEngineUtils.test.js.map
packages/lib/services/searchengine/filterParser.d.ts
packages/lib/services/searchengine/filterParser.js
packages/lib/services/searchengine/filterParser.js.map
packages/lib/services/searchengine/filterParser.test.d.ts
packages/lib/services/searchengine/filterParser.test.js
packages/lib/services/searchengine/filterParser.test.js.map
packages/lib/services/searchengine/queryBuilder.d.ts
packages/lib/services/searchengine/queryBuilder.js
packages/lib/services/searchengine/queryBuilder.js.map
Expand Down
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -1383,6 +1383,9 @@ packages/lib/services/searchengine/SearchEngineUtils.test.js.map
packages/lib/services/searchengine/filterParser.d.ts
packages/lib/services/searchengine/filterParser.js
packages/lib/services/searchengine/filterParser.js.map
packages/lib/services/searchengine/filterParser.test.d.ts
packages/lib/services/searchengine/filterParser.test.js
packages/lib/services/searchengine/filterParser.test.js.map
packages/lib/services/searchengine/queryBuilder.d.ts
packages/lib/services/searchengine/queryBuilder.js
packages/lib/services/searchengine/queryBuilder.js.map
Expand Down
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,8 @@ The filters are implicitly connected by and/or connectives depending on the foll
- To override this default behaviour, use the `any` filter, in which case the search terms will be connected by "OR" instead.
- There's an exception for the `notebook` filters which are connected by "OR". The reason being that no note can be in multiple notebooks at once.

Incorrect search filters are interpreted as a phrase search, e.g. misspelled `nootebook:Example` or non-existing `https://joplinapp.org`.

## Search order

Notes are sorted by "relevance". Currently it means the notes that contain the requested terms the most times are on top. For queries with multiple terms, it also matters how close to each other the terms are. This is a bit experimental so if you notice a search query that returns unexpected results, please report it in the forum, providing as many details as possible to replicate the issue.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
/* eslint-disable no-unused-vars */
import filterParser from './filterParser';

const filterParser = require('../../services/searchengine/filterParser.js').default;
// import filterParser from '../../services/searchengine/filterParser.js';

const makeTerm = (name, value, negated, quoted = false, wildcard = false) => {
const makeTerm = (name: string, value: string, negated: boolean, quoted: boolean = false, wildcard: boolean = false) => {
if (name === 'text') { return { name, value, negated, quoted, wildcard }; }
if (name === 'title' | name === 'body') { return { name, value, negated, wildcard }; }
if (name === 'title' || name === 'body') { return { name, value, negated, wildcard }; }
return { name, value, negated };
};

Expand Down Expand Up @@ -135,13 +132,16 @@ describe('filterParser should be correct filter for keyword', () => {

it('handle invalid filters', () => {
let searchString = 'titletitle:123';
expect(() => filterParser(searchString)).toThrow(new Error('Invalid filter: titletitle'));
expect(filterParser(searchString)).toContainEqual(makeTerm('text', '"titletitle:123"', false));

searchString = 'invalid:abc';
expect(() => filterParser(searchString)).toThrow(new Error('Invalid filter: invalid'));
expect(filterParser(searchString)).toContainEqual(makeTerm('text', '"invalid:abc"', false));

searchString = '-invalid:abc';
expect(filterParser(searchString)).toContainEqual(makeTerm('text', '"invalid:abc"', true));

searchString = ':abc';
expect(() => filterParser(searchString)).toThrow(new Error('Invalid filter: '));
expect(filterParser(searchString)).toContainEqual(makeTerm('text', '":abc"', false));

searchString = 'type:blah';
expect(() => filterParser(searchString)).toThrow(new Error('The value of filter "type" must be "note" or "todo"'));
Expand Down
7 changes: 4 additions & 3 deletions packages/lib/services/searchengine/filterParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const quote = (s: string) => {
};


const getTerms = (query: string): Term[] => {
const getTerms = (query: string, validFilters: Set<string>): Term[] => {
const terms: Term[] = [];
let inQuote = false;
let inTerm = false;
Expand Down Expand Up @@ -52,7 +52,8 @@ const getTerms = (query: string): Term[] => {
continue;
}

if (c === ':' && !inQuote && !inTerm) {
if (c === ':' && !inQuote && !inTerm &&
(validFilters.has(currentTerm) || currentTerm[0] === '-' && validFilters.has(currentTerm.substr(1, currentTerm.length)))) {
currentCol = currentTerm;
currentTerm = '';
inTerm = true; // to ignore any other ':' before a space eg.'sourceurl:https://www.google.com'
Expand All @@ -71,7 +72,7 @@ const parseQuery = (query: string): Term[] => {
'iscompleted', 'due', 'latitude', 'longitude',
'altitude', 'resource', 'sourceurl', 'id']);

const terms = getTerms(query);
const terms = getTerms(query, validFilters);

const result: Term[] = [];
for (let i = 0; i < terms.length; i++) {
Expand Down

0 comments on commit 8cbcb78

Please sign in to comment.