From 2d2621d2f3f101a423b6c93a75ba012ee3d41107 Mon Sep 17 00:00:00 2001 From: Mark Hahnenberg Date: Tue, 28 Feb 2017 14:25:59 -0800 Subject: [PATCH] [client-app] Refactor search query codegen into proper backend Summary: Previously we were using the raw visitors that were confined to the flux attributes directory. We're going to add more search query backends, so this is mostly just moving things to a new, more general place. Test Plan: Run locally, verify parser specs still work, verify in-app search still works. Reviewers: spang, evan, juan Reviewed By: juan Differential Revision: https://phab.nylas.com/D4053 --- .../lib/search-query-subscription.es6 | 4 +- .../thread-search/lib/search-store.es6 | 4 +- .../search}/search-query-parser-spec.es6 | 68 +++++++++---------- .../src/flux/attributes/matcher.es6 | 5 +- .../client-app/src/global/nylas-exports.es6 | 3 +- .../search/search-query-ast.es6} | 28 ++++---- .../search/search-query-backend-local.es6} | 20 ++++-- .../services/search}/search-query-parser.es6 | 14 ++-- 8 files changed, 78 insertions(+), 68 deletions(-) rename packages/client-app/{internal_packages/thread-search/spec => spec/services/search}/search-query-parser-spec.es6 (63%) rename packages/client-app/src/{flux/models/thread-query-ast.es6 => services/search/search-query-ast.es6} (88%) rename packages/client-app/src/{flux/attributes/matcher-helpers.es6 => services/search/search-query-backend-local.es6} (91%) rename packages/client-app/{internal_packages/thread-search/lib => src/services/search}/search-query-parser.es6 (98%) diff --git a/packages/client-app/internal_packages/thread-search/lib/search-query-subscription.es6 b/packages/client-app/internal_packages/thread-search/lib/search-query-subscription.es6 index cba22567e..758385350 100644 --- a/packages/client-app/internal_packages/thread-search/lib/search-query-subscription.es6 +++ b/packages/client-app/internal_packages/thread-search/lib/search-query-subscription.es6 @@ -7,9 +7,9 @@ import { ComponentRegistry, FocusedContentStore, MutableQuerySubscription, + SearchQueryParser, } from 'nylas-exports' import SearchActions from './search-actions' -import {parseSearchQuery} from './search-query-parser' const {LongConnectionStatus} = NylasAPI @@ -60,7 +60,7 @@ class SearchQuerySubscription extends MutableQuerySubscription { dbQuery = dbQuery.where({accountId: this._accountIds[0]}) } try { - const parsedQuery = parseSearchQuery(this._searchQuery); + const parsedQuery = SearchQueryParser.parse(this._searchQuery); console.info('Successfully parsed and codegened search query', parsedQuery); dbQuery = dbQuery.structuredSearch(parsedQuery); } catch (e) { diff --git a/packages/client-app/internal_packages/thread-search/lib/search-store.es6 b/packages/client-app/internal_packages/thread-search/lib/search-store.es6 index c3724a801..33bb39267 100644 --- a/packages/client-app/internal_packages/thread-search/lib/search-store.es6 +++ b/packages/client-app/internal_packages/thread-search/lib/search-store.es6 @@ -7,8 +7,8 @@ import { DatabaseStore, ComponentRegistry, FocusedPerspectiveStore, + SearchQueryParser, } from 'nylas-exports'; -import {parseSearchQuery} from './search-query-parser' import SearchActions from './search-actions'; import SearchMailboxPerspective from './search-mailbox-perspective'; @@ -152,7 +152,7 @@ class SearchStore extends NylasStore { } try { - const parsedQuery = parseSearchQuery(this._searchQuery); + const parsedQuery = SearchQueryParser.parse(this._searchQuery); // console.info('Successfully parsed and codegened search query', parsedQuery); dbQuery = dbQuery.structuredSearch(parsedQuery); } catch (e) { diff --git a/packages/client-app/internal_packages/thread-search/spec/search-query-parser-spec.es6 b/packages/client-app/spec/services/search/search-query-parser-spec.es6 similarity index 63% rename from packages/client-app/internal_packages/thread-search/spec/search-query-parser-spec.es6 rename to packages/client-app/spec/services/search/search-query-parser-spec.es6 index 42f557876..3bc43b3cf 100644 --- a/packages/client-app/internal_packages/thread-search/spec/search-query-parser-spec.es6 +++ b/packages/client-app/spec/services/search/search-query-parser-spec.es6 @@ -1,7 +1,7 @@ import { - ThreadQueryAST, + SearchQueryAST, + SearchQueryParser, } from 'nylas-exports'; -import {parseSearchQuery} from '../lib/search-query-parser' const { SearchQueryToken, @@ -15,7 +15,7 @@ const { UnreadStatusQueryExpression, StarredStatusQueryExpression, InQueryExpression, -} = ThreadQueryAST; +} = SearchQueryAST; const token = (text) => { return new SearchQueryToken(text); } const and = (e1, e2) => { return new AndQueryExpression(e1, e2); } @@ -30,107 +30,107 @@ const unread = (status) => { return new UnreadStatusQueryExpression(status); } const starred = (status) => { return new StarredStatusQueryExpression(status); } -describe('parseSearchQuery', () => { +describe('SearchQueryParser.parse', () => { it('correctly parses simple queries', () => { - expect(parseSearchQuery('blah').equals( + expect(SearchQueryParser.parse('blah').equals( generic(text(token('blah'))) )).toBe(true) - expect(parseSearchQuery('"foo bar"').equals( + expect(SearchQueryParser.parse('"foo bar"').equals( generic(text(token('foo bar'))) )).toBe(true) - expect(parseSearchQuery('to:blah').equals( + expect(SearchQueryParser.parse('to:blah').equals( to(text(token('blah'))) )).toBe(true) - expect(parseSearchQuery('from:blah').equals( + expect(SearchQueryParser.parse('from:blah').equals( from(text(token('blah'))) )).toBe(true) - expect(parseSearchQuery('subject:blah').equals( + expect(SearchQueryParser.parse('subject:blah').equals( subject(text(token('blah'))) )).toBe(true) - expect(parseSearchQuery('to:mhahnenb@gmail.com').equals( + expect(SearchQueryParser.parse('to:mhahnenb@gmail.com').equals( to(text(token('mhahnenb@gmail.com'))) )).toBe(true) - expect(parseSearchQuery('to:"mhahnenb@gmail.com"').equals( + expect(SearchQueryParser.parse('to:"mhahnenb@gmail.com"').equals( to(text(token('mhahnenb@gmail.com'))) )).toBe(true) - expect(parseSearchQuery('to:"Mark mhahnenb@gmail.com"').equals( + expect(SearchQueryParser.parse('to:"Mark mhahnenb@gmail.com"').equals( to(text(token('Mark mhahnenb@gmail.com'))) )).toBe(true) - expect(parseSearchQuery('is:unread').equals( + expect(SearchQueryParser.parse('is:unread').equals( unread(true) )).toBe(true) - expect(parseSearchQuery('is:read').equals( + expect(SearchQueryParser.parse('is:read').equals( unread(false) )).toBe(true) - expect(parseSearchQuery('is:starred').equals( + expect(SearchQueryParser.parse('is:starred').equals( starred(true) )).toBe(true) - expect(parseSearchQuery('is:unstarred').equals( + expect(SearchQueryParser.parse('is:unstarred').equals( starred(false) )).toBe(true) - expect(parseSearchQuery('in:foo').equals( + expect(SearchQueryParser.parse('in:foo').equals( in_(text(token('foo'))) )).toBe(true) }); it('correctly parses reserved words as normal text in certain places', () => { - expect(parseSearchQuery('to:blah').equals( + expect(SearchQueryParser.parse('to:blah').equals( to(text(token('blah'))) )).toBe(true) - expect(parseSearchQuery('to:to').equals( + expect(SearchQueryParser.parse('to:to').equals( to(text(token('to'))) )).toBe(true) - expect(parseSearchQuery('to:subject').equals( + expect(SearchQueryParser.parse('to:subject').equals( to(text(token('subject'))) )).toBe(true) - expect(parseSearchQuery('to:from').equals( + expect(SearchQueryParser.parse('to:from').equals( to(text(token('from'))) )).toBe(true) - expect(parseSearchQuery('to:unread').equals( + expect(SearchQueryParser.parse('to:unread').equals( to(text(token('unread'))) )).toBe(true) - expect(parseSearchQuery('to:starred').equals( + expect(SearchQueryParser.parse('to:starred').equals( to(text(token('starred'))) )).toBe(true) }); it('correctly parses compound queries', () => { - expect(parseSearchQuery('foo bar').equals( + expect(SearchQueryParser.parse('foo bar').equals( and(generic(text(token('foo'))), generic(text(token('bar')))) )).toBe(true) - expect(parseSearchQuery('foo AND bar').equals( + expect(SearchQueryParser.parse('foo AND bar').equals( and(generic(text(token('foo'))), generic(text(token('bar')))) )).toBe(true) - expect(parseSearchQuery('foo OR bar').equals( + expect(SearchQueryParser.parse('foo OR bar').equals( or(generic(text(token('foo'))), generic(text(token('bar')))) )).toBe(true) - expect(parseSearchQuery('to:foo OR bar').equals( + expect(SearchQueryParser.parse('to:foo OR bar').equals( or(to(text(token('foo'))), generic(text(token('bar')))) )).toBe(true) - expect(parseSearchQuery('foo OR to:bar').equals( + expect(SearchQueryParser.parse('foo OR to:bar').equals( or(generic(text(token('foo'))), to(text(token('bar')))) )).toBe(true) - expect(parseSearchQuery('foo bar baz').equals( + expect(SearchQueryParser.parse('foo bar baz').equals( and(generic(text(token('foo'))), and(generic(text(token('bar'))), generic(text(token('baz'))))) )).toBe(true) - expect(parseSearchQuery('foo AND bar AND baz').equals( + expect(SearchQueryParser.parse('foo AND bar AND baz').equals( and(generic(text(token('foo'))), and(generic(text(token('bar'))), generic(text(token('baz'))))) )).toBe(true) - expect(parseSearchQuery('foo OR bar AND baz').equals( + expect(SearchQueryParser.parse('foo OR bar AND baz').equals( and( or(generic(text(token('foo'))), generic(text(token('bar')))), generic(text(token('baz')))) )).toBe(true) - expect(parseSearchQuery('foo OR bar OR baz').equals( + expect(SearchQueryParser.parse('foo OR bar OR baz').equals( or(generic(text(token('foo'))), or(generic(text(token('bar'))), generic(text(token('baz'))))) )).toBe(true) - expect(parseSearchQuery('foo is:unread').equals( + expect(SearchQueryParser.parse('foo is:unread').equals( and(generic(text(token('foo'))), unread(true)), )).toBe(true) - expect(parseSearchQuery('is:unread foo').equals( + expect(SearchQueryParser.parse('is:unread foo').equals( and(unread(true), generic(text(token('foo')))) )).toBe(true) }); diff --git a/packages/client-app/src/flux/attributes/matcher.es6 b/packages/client-app/src/flux/attributes/matcher.es6 index 396dd9755..6c689265f 100644 --- a/packages/client-app/src/flux/attributes/matcher.es6 +++ b/packages/client-app/src/flux/attributes/matcher.es6 @@ -1,5 +1,5 @@ import {tableNameForJoin} from '../models/utils'; -import {StructuredSearchQueryVisitor} from './matcher-helpers' +import LocalSearchQueryBackend from '../../services/search/search-query-backend-local' // https://www.sqlite.org/faq.html#q14 // That's right. Two single quotes in a row… @@ -275,8 +275,7 @@ class StructuredSearchMatcher extends Matcher { } whereSQL(klass) { - const visitor = new StructuredSearchQueryVisitor(`${klass.name}`); - return visitor.visit(this._searchQuery); + return (new LocalSearchQueryBackend(klass.name)).compile(this._searchQuery) } } diff --git a/packages/client-app/src/global/nylas-exports.es6 b/packages/client-app/src/global/nylas-exports.es6 index fc71bd3fa..7bf5a5c3b 100644 --- a/packages/client-app/src/global/nylas-exports.es6 +++ b/packages/client-app/src/global/nylas-exports.es6 @@ -105,7 +105,8 @@ lazyLoadAndRegisterModel(`JSONBlob`, 'json-blob'); lazyLoadAndRegisterModel(`ProviderSyncbackRequest`, 'provider-syncback-request'); // Thread Search Query AST -lazyLoad(`ThreadQueryAST`, 'flux/models/thread-query-ast'); +lazyLoad(`SearchQueryAST`, 'services/search/search-query-ast'); +lazyLoad(`SearchQueryParser`, 'services/search/search-query-parser'); // Tasks exports.TaskRegistry = TaskRegistry; diff --git a/packages/client-app/src/flux/models/thread-query-ast.es6 b/packages/client-app/src/services/search/search-query-ast.es6 similarity index 88% rename from packages/client-app/src/flux/models/thread-query-ast.es6 rename to packages/client-app/src/services/search/search-query-ast.es6 index 33b0f5a1d..6b250696c 100644 --- a/packages/client-app/src/flux/models/thread-query-ast.es6 +++ b/packages/client-app/src/services/search/search-query-ast.es6 @@ -1,5 +1,5 @@ -export class SearchQueryExpressionVisitor { +class SearchQueryExpressionVisitor { constructor() { this._result = null; } @@ -24,7 +24,7 @@ export class SearchQueryExpressionVisitor { visitIn(node) { throw new Error('Abstract function not implemented!', node); } } -export class QueryExpression { +class QueryExpression { constructor() { this._isMatchCompatible = null; } @@ -49,7 +49,7 @@ export class QueryExpression { } } -export class AndQueryExpression extends QueryExpression { +class AndQueryExpression extends QueryExpression { constructor(e1, e2) { super(); this.e1 = e1; @@ -72,7 +72,7 @@ export class AndQueryExpression extends QueryExpression { } } -export class OrQueryExpression extends QueryExpression { +class OrQueryExpression extends QueryExpression { constructor(e1, e2) { super(); this.e1 = e1; @@ -95,7 +95,7 @@ export class OrQueryExpression extends QueryExpression { } } -export class FromQueryExpression extends QueryExpression { +class FromQueryExpression extends QueryExpression { constructor(text) { super(); this.text = text; @@ -117,7 +117,7 @@ export class FromQueryExpression extends QueryExpression { } } -export class ToQueryExpression extends QueryExpression { +class ToQueryExpression extends QueryExpression { constructor(text) { super(); this.text = text; @@ -139,7 +139,7 @@ export class ToQueryExpression extends QueryExpression { } } -export class SubjectQueryExpression extends QueryExpression { +class SubjectQueryExpression extends QueryExpression { constructor(text) { super(); this.text = text; @@ -161,7 +161,7 @@ export class SubjectQueryExpression extends QueryExpression { } } -export class UnreadStatusQueryExpression extends QueryExpression { +class UnreadStatusQueryExpression extends QueryExpression { constructor(status) { super(); this.status = status; @@ -184,7 +184,7 @@ export class UnreadStatusQueryExpression extends QueryExpression { } } -export class StarredStatusQueryExpression extends QueryExpression { +class StarredStatusQueryExpression extends QueryExpression { constructor(status) { super(); this.status = status; @@ -206,7 +206,7 @@ export class StarredStatusQueryExpression extends QueryExpression { } } -export class GenericQueryExpression extends QueryExpression { +class GenericQueryExpression extends QueryExpression { constructor(text) { super(); this.text = text; @@ -228,7 +228,7 @@ export class GenericQueryExpression extends QueryExpression { } } -export class TextQueryExpression extends QueryExpression { +class TextQueryExpression extends QueryExpression { constructor(text) { super(); this.token = text; @@ -250,7 +250,7 @@ export class TextQueryExpression extends QueryExpression { } } -export class InQueryExpression extends QueryExpression { +class InQueryExpression extends QueryExpression { constructor(text) { super(); this.text = text; @@ -276,7 +276,7 @@ export class InQueryExpression extends QueryExpression { * Intermediate representation for multiple match-compatible nodes. Used when * translating the initial query AST into the proper SQL-compatible query. */ -export class MatchQueryExpression extends QueryExpression { +class MatchQueryExpression extends QueryExpression { constructor(rawMatchQuery) { super(); this.rawQuery = rawMatchQuery; @@ -302,7 +302,7 @@ export class MatchQueryExpression extends QueryExpression { } } -export class SearchQueryToken { +class SearchQueryToken { constructor(s) { this.s = s; } diff --git a/packages/client-app/src/flux/attributes/matcher-helpers.es6 b/packages/client-app/src/services/search/search-query-backend-local.es6 similarity index 91% rename from packages/client-app/src/flux/attributes/matcher-helpers.es6 rename to packages/client-app/src/services/search/search-query-backend-local.es6 index 1efa4e782..6af2e8873 100644 --- a/packages/client-app/src/flux/attributes/matcher-helpers.es6 +++ b/packages/client-app/src/services/search/search-query-backend-local.es6 @@ -5,7 +5,7 @@ import { UnreadStatusQueryExpression, StarredStatusQueryExpression, MatchQueryExpression, -} from '../models/thread-query-ast' +} from './search-query-ast' /* * This class visits a match-compatible subtree and condenses it into a single @@ -149,15 +149,14 @@ class MatchCompatibleQueryCondenser extends SearchQueryExpressionVisitor { * converting match-compatible subtrees into the appropriate subquery that * uses a MATCH clause. */ -export class StructuredSearchQueryVisitor extends SearchQueryExpressionVisitor { +class StructuredSearchQueryVisitor extends SearchQueryExpressionVisitor { constructor(className) { super(); this._className = className; } visit(root) { - const condenser = new MatchCompatibleQueryCondenser(); - return this.visitAndGetResult(condenser.visit(root)); + return this.visitAndGetResult(root); } visitAnd(node) { @@ -212,3 +211,16 @@ export class StructuredSearchQueryVisitor extends SearchQueryExpressionVisitor { } } +export default class LocalSearchQueryBackend { + constructor(modelClassName) { + this._modelClassName = modelClassName; + } + + compile(ast) { + const condenser = new MatchCompatibleQueryCondenser(); + const intermediateAST = condenser.visit(ast); + + const codegen = new StructuredSearchQueryVisitor(`${this._modelClassName}`); + return codegen.visit(intermediateAST); + } +} diff --git a/packages/client-app/internal_packages/thread-search/lib/search-query-parser.es6 b/packages/client-app/src/services/search/search-query-parser.es6 similarity index 98% rename from packages/client-app/internal_packages/thread-search/lib/search-query-parser.es6 rename to packages/client-app/src/services/search/search-query-parser.es6 index 5c2478089..b5497f3a9 100644 --- a/packages/client-app/internal_packages/thread-search/lib/search-query-parser.es6 +++ b/packages/client-app/src/services/search/search-query-parser.es6 @@ -1,8 +1,4 @@ import { - ThreadQueryAST, -} from 'nylas-exports'; - -const { SearchQueryToken, OrQueryExpression, AndQueryExpression, @@ -14,7 +10,7 @@ const { UnreadStatusQueryExpression, StarredStatusQueryExpression, InQueryExpression, -} = ThreadQueryAST; +} from './search-query-ast'; const nextStringToken = (text) => { if (text[0] !== '"') { @@ -309,6 +305,8 @@ const parseQueryWrapper = (text) => { return result; }; -module.exports = { - parseSearchQuery: parseQueryWrapper, -}; +export default class SearchQueryParser { + static parse(query) { + return parseQueryWrapper(query); + } +}