[client-sync] Fix "in:" for IMAP search

Summary:
Prior to this diff, the "in:" search query syntax didn't work for IMAP.
This diff implements "in:" by changing the IMAP search backend to take
folder context into account and emit the appropriate queries for each
folder. Queries that include "in:foo" will replace the corresponding AST
nodes with 'ALL' or 'NOT ALL' depending on whether or not the current folder
is "foo". We also now filter which folders we search based on which
folders are referenced in the query.

Test Plan: Run locally, verify that in: works quickly

Reviewers: evan, juan, spang

Reviewed By: juan, spang

Differential Revision: https://phab.nylas.com/D4284
This commit is contained in:
Mark Hahnenberg 2017-03-28 15:03:37 -07:00
parent 69aa71e575
commit 78b9cf81c5
3 changed files with 160 additions and 36 deletions

View file

@ -4,66 +4,94 @@ import IMAPSearchQueryBackend from '../../../src/services/search/search-query-ba
describe('IMAPSearchQueryBackend', () => { describe('IMAPSearchQueryBackend', () => {
it('correctly codegens TEXT', () => { it('correctly codegens TEXT', () => {
const ast = SearchQueryParser.parse('foo'); const ast = SearchQueryParser.parse('foo');
const result = IMAPSearchQueryBackend.compile(ast); const result = IMAPSearchQueryBackend.compile(ast, {name: 'INBOX'});
expect(result).toEqual([['TEXT', 'foo']]); expect(result).toEqual([['TEXT', 'foo']]);
}); });
it('correctly codegens FROM', () => { it('correctly codegens FROM', () => {
const ast = SearchQueryParser.parse('from:mark'); const ast = SearchQueryParser.parse('from:mark');
const result = IMAPSearchQueryBackend.compile(ast); const result = IMAPSearchQueryBackend.compile(ast, {name: 'INBOX'});
expect(result).toEqual([['FROM', 'mark']]); expect(result).toEqual([['FROM', 'mark']]);
}); });
it('correctly codegens TO', () => { it('correctly codegens TO', () => {
const ast = SearchQueryParser.parse('to:mark'); const ast = SearchQueryParser.parse('to:mark');
const result = IMAPSearchQueryBackend.compile(ast); const result = IMAPSearchQueryBackend.compile(ast, {name: 'INBOX'});
expect(result).toEqual([['TO', 'mark']]); expect(result).toEqual([['TO', 'mark']]);
}); });
it('correctly codegens SUBJECT', () => { it('correctly codegens SUBJECT', () => {
const ast = SearchQueryParser.parse('subject:foobar'); const ast = SearchQueryParser.parse('subject:foobar');
const result = IMAPSearchQueryBackend.compile(ast); const result = IMAPSearchQueryBackend.compile(ast, {name: 'INBOX'});
expect(result).toEqual([['SUBJECT', 'foobar']]); expect(result).toEqual([['SUBJECT', 'foobar']]);
}); });
it('correctly codegens UNREAD', () => { it('correctly codegens UNREAD', () => {
const ast = SearchQueryParser.parse('is:unread'); const ast = SearchQueryParser.parse('is:unread');
const result = IMAPSearchQueryBackend.compile(ast); const result = IMAPSearchQueryBackend.compile(ast, {name: 'INBOX'});
expect(result).toEqual(['UNSEEN']); expect(result).toEqual(['UNSEEN']);
}); });
it('correctly codegens SEEN', () => { it('correctly codegens SEEN', () => {
const ast = SearchQueryParser.parse('is:read'); const ast = SearchQueryParser.parse('is:read');
const result = IMAPSearchQueryBackend.compile(ast); const result = IMAPSearchQueryBackend.compile(ast, {name: 'INBOX'});
expect(result).toEqual(['SEEN']); expect(result).toEqual(['SEEN']);
}); });
it('correctly codegens FLAGGED', () => { it('correctly codegens FLAGGED', () => {
const ast = SearchQueryParser.parse('is:starred'); const ast = SearchQueryParser.parse('is:starred');
const result = IMAPSearchQueryBackend.compile(ast); const result = IMAPSearchQueryBackend.compile(ast, {name: 'INBOX'});
expect(result).toEqual(['FLAGGED']); expect(result).toEqual(['FLAGGED']);
}); });
it('correctly codegens UNFLAGGED', () => { it('correctly codegens UNFLAGGED', () => {
const ast = SearchQueryParser.parse('is:unstarred'); const ast = SearchQueryParser.parse('is:unstarred');
const result = IMAPSearchQueryBackend.compile(ast); const result = IMAPSearchQueryBackend.compile(ast, {name: 'INBOX'});
expect(result).toEqual(['UNFLAGGED']); expect(result).toEqual(['UNFLAGGED']);
}); });
it('correctly codegens AND', () => { it('correctly codegens AND', () => {
const ast1 = SearchQueryParser.parse('is:starred AND is:unread'); const ast1 = SearchQueryParser.parse('is:starred AND is:unread');
const result1 = IMAPSearchQueryBackend.compile(ast1); const result1 = IMAPSearchQueryBackend.compile(ast1, {name: 'INBOX'});
expect(result1).toEqual(['FLAGGED', 'UNSEEN']); expect(result1).toEqual(['FLAGGED', 'UNSEEN']);
const ast2 = SearchQueryParser.parse('is:starred is:unread'); const ast2 = SearchQueryParser.parse('is:starred is:unread');
const result2 = IMAPSearchQueryBackend.compile(ast2); const result2 = IMAPSearchQueryBackend.compile(ast2, {name: 'INBOX'});
expect(result2).toEqual(['FLAGGED', 'UNSEEN']); expect(result2).toEqual(['FLAGGED', 'UNSEEN']);
}); });
it('correctly codegens OR', () => { it('correctly codegens OR', () => {
const ast = SearchQueryParser.parse('is:starred OR is:unread'); const ast = SearchQueryParser.parse('is:starred OR is:unread');
const result = IMAPSearchQueryBackend.compile(ast); const result = IMAPSearchQueryBackend.compile(ast, {name: 'INBOX'});
expect(result).toEqual([['OR', 'FLAGGED', 'UNSEEN']]); expect(result).toEqual([['OR', 'FLAGGED', 'UNSEEN']]);
}); });
it('correctly ignores "in:foo"', () => { it('correctly codegens "in:foo"', () => {
const ast = SearchQueryParser.parse('is:starred OR in:foo'); const ast1 = SearchQueryParser.parse('is:starred OR in:foo');
const result = IMAPSearchQueryBackend.compile(ast); const result1 = IMAPSearchQueryBackend.compile(ast1, {name: 'INBOX'});
expect(result).toEqual([['OR', 'FLAGGED', 'ALL']]); expect(result1).toEqual([['OR', 'FLAGGED', '!ALL']]);
const result2 = IMAPSearchQueryBackend.compile(ast1, {name: 'foo'});
expect(result2).toEqual([['OR', 'FLAGGED', 'ALL']]);
const ast2 = SearchQueryParser.parse('in:foo');
const result3 = IMAPSearchQueryBackend.compile(ast2, {name: 'foo'});
expect(result3).toEqual(['ALL']);
const result4 = IMAPSearchQueryBackend.compile(ast2, {name: 'INBOX'});
expect(result4).toEqual(['!ALL']);
}); });
it('correctly joins adjacent AND queries', () => { it('correctly joins adjacent AND queries', () => {
const ast = SearchQueryParser.parse('is:starred AND is:unread AND foo'); const ast = SearchQueryParser.parse('is:starred AND is:unread AND foo');
const result = IMAPSearchQueryBackend.compile(ast); const result = IMAPSearchQueryBackend.compile(ast, {name: 'INBOX'});
expect(result).toEqual(['FLAGGED', 'UNSEEN', ['TEXT', 'foo']]); expect(result).toEqual(['FLAGGED', 'UNSEEN', ['TEXT', 'foo']]);
}); });
it('correctly deduces the set of folders', () => {
const ast1 = SearchQueryParser.parse('is:starred');
const result1 = IMAPSearchQueryBackend.folderNamesForQuery(ast1);
expect(result1).toEqual(IMAPSearchQueryBackend.ALL_FOLDERS());
const ast2 = SearchQueryParser.parse('in:foo');
const result2 = IMAPSearchQueryBackend.folderNamesForQuery(ast2);
expect(result2).toEqual(['foo']);
const ast3 = SearchQueryParser.parse('in:foo AND in:bar');
const result3 = IMAPSearchQueryBackend.folderNamesForQuery(ast3);
expect(result3).toEqual([]);
const ast4 = SearchQueryParser.parse('in:foo OR bar');
const result4 = IMAPSearchQueryBackend.folderNamesForQuery(ast4);
expect(result4).toEqual(IMAPSearchQueryBackend.ALL_FOLDERS());
const ast5 = SearchQueryParser.parse('in:foo OR in:bar');
const result5 = IMAPSearchQueryBackend.folderNamesForQuery(ast5);
expect(result5).toEqual(['foo', 'bar']);
});
}); });

View file

@ -1,9 +1,84 @@
import _ from 'underscore'
import { import {
AndQueryExpression, AndQueryExpression,
SearchQueryExpressionVisitor, SearchQueryExpressionVisitor,
} from './search-query-ast'; } from './search-query-ast';
const TOP = 'top';
class IMAPSearchQueryFolderFinderVisitor extends SearchQueryExpressionVisitor {
visit(root) {
const result = this.visitAndGetResult(root);
if (result === TOP) {
return 'all';
}
return result;
}
visitAnd(node) {
const lhs = this.visitAndGetResult(node.e1);
const rhs = this.visitAndGetResult(node.e2);
if (lhs === TOP) {
this._result = rhs;
return;
}
if (rhs === TOP) {
this._result = lhs;
return;
}
this._result = _.intersection(lhs, rhs);
}
visitOr(node) {
const lhs = this.visitAndGetResult(node.e1);
const rhs = this.visitAndGetResult(node.e2);
if (lhs === TOP || rhs === TOP) {
this._result = TOP;
return;
}
this._result = _.union(lhs, rhs);
}
visitIn(node) {
const folderName = this.visitAndGetResult(node.text);
this._result = [folderName];
}
visitFrom(/* node */) {
this._result = TOP;
}
visitTo(/* node */) {
this._result = TOP;
}
visitSubject(/* node */) {
this._result = TOP;
}
visitGeneric(/* node */) {
this._result = TOP;
}
visitText(node) {
this._result = node.token.s;
}
visitUnread(/* node */) {
this._result = TOP;
}
visitStarred(/* node */) {
this._result = TOP;
}
}
class IMAPSearchQueryExpressionVisitor extends SearchQueryExpressionVisitor { class IMAPSearchQueryExpressionVisitor extends SearchQueryExpressionVisitor {
constructor(folder) {
super();
this._folder = folder;
}
visit(root) { visit(root) {
const result = this.visitAndGetResult(root); const result = this.visitAndGetResult(root);
if (root instanceof AndQueryExpression) { if (root instanceof AndQueryExpression) {
@ -67,22 +142,27 @@ class IMAPSearchQueryExpressionVisitor extends SearchQueryExpressionVisitor {
this._result = node.status ? 'FLAGGED' : 'UNFLAGGED'; this._result = node.status ? 'FLAGGED' : 'UNFLAGGED';
} }
visitIn(/* node */) { visitIn(node) {
// TODO: Somehow make the search switch folders. To make this work correctly const text = this.visitAndGetResult(node.text);
// with nested expressions we would probably end up generating a mini this._result = text === this._folder.name ? 'ALL' : '!ALL';
// program that would instruct the connection to switch to a folder and issue
// the proper search command for that subquery. At the end we would combine
// the results according to the query.
this._result = 'ALL';
} }
} }
export default class IMAPSearchQueryBackend { export default class IMAPSearchQueryBackend {
static compile(ast) { static ALL_FOLDERS() {
return (new IMAPSearchQueryBackend()).compile(ast); return 'all';
} }
compile(ast) { static compile(ast, folder) {
return (new IMAPSearchQueryExpressionVisitor()).visit(ast); return (new IMAPSearchQueryBackend()).compile(ast, folder);
}
static folderNamesForQuery(ast) {
return (new IMAPSearchQueryFolderFinderVisitor()).visit(ast);
}
compile(ast, folder) {
return (new IMAPSearchQueryExpressionVisitor(folder)).visit(ast);
} }
} }

View file

@ -66,10 +66,26 @@ class ImapSearchClient {
this._cancelled = false; this._cancelled = false;
} }
async _getFoldersForSearch(db) { async _getFoldersForSearch(db, query) {
const {Folder} = db;
const folderNames = IMAPSearchQueryBackend.folderNamesForQuery(query);
if (folderNames !== IMAPSearchQueryBackend.ALL_FOLDERS()) {
if (folderNames.length === 0) {
return [];
}
const result = await Folder.findAll({
where: {
accountId: this.account.id,
name: folderNames,
},
});
return result;
}
// We want to start the search with the 'inbox', 'sent' and 'archive' // We want to start the search with the 'inbox', 'sent' and 'archive'
// folders, if they exist. // folders, if they exist.
const {Folder} = db;
const folders = await Folder.findAll({ const folders = await Folder.findAll({
where: { where: {
accountId: this.account.id, accountId: this.account.id,
@ -87,14 +103,13 @@ class ImapSearchClient {
return folders.concat(accountFolders); return folders.concat(accountFolders);
} }
_getCriteriaForQuery(query) { _getCriteriaForQuery(query, folder) {
const parsedQuery = SearchQueryParser.parse(query); return IMAPSearchQueryBackend.compile(query, folder);
return IMAPSearchQueryBackend.compile(parsedQuery);
} }
async _search(db, query) { async _search(db, query) {
const folders = await this._getFoldersForSearch(db); const parsedQuery = SearchQueryParser.parse(query);
const criteria = this._getCriteriaForQuery(query); const folders = await this._getFoldersForSearch(db, parsedQuery);
let numTimeoutErrors = 0; let numTimeoutErrors = 0;
return Rx.Observable.create(async (observer) => { return Rx.Observable.create(async (observer) => {
const onConnected = async ([conn]) => { const onConnected = async ([conn]) => {
@ -102,6 +117,7 @@ class ImapSearchClient {
// searched folders if there is an error later down the line. // searched folders if there is an error later down the line.
while (folders.length > 0) { while (folders.length > 0) {
const folder = folders[0]; const folder = folders[0];
const criteria = this._getCriteriaForQuery(parsedQuery, folder);
const uids = await this._searchFolder(conn, folder, criteria); const uids = await this._searchFolder(conn, folder, criteria);
folders.shift(); folders.shift();
if (uids.length > 0) { if (uids.length > 0) {
@ -242,12 +258,12 @@ class ImapSearchClient {
} }
class GmailSearchClient extends ImapSearchClient { class GmailSearchClient extends ImapSearchClient {
async _getFoldersForSearch(db) { async _getFoldersForSearch(db/* , query*/) {
const allMail = await db.Folder.findOne({where: {role: 'all'}}); const allMail = await db.Folder.findOne({where: {role: 'all'}});
return [allMail]; return [allMail];
} }
_getCriteriaForQuery(query) { _getCriteriaForQuery(query/* , folder*/) {
return [['X-GM-RAW', query]]; return [['X-GM-RAW', query]];
} }
} }