added keyword search to history text filter, tests

2026-09-24 16:30:27 +08:00 · 2021-01-29 12:15:06 -08:00
parent b8fec3ce1b
commit 2b16ba4904
5 changed files with 530 additions and 102 deletions
@@ -39,7 +39,10 @@ export const loadHistoryContents = (cfg = {}) => (rawInputs$) => {

    const ajaxResponse$ = inputs$.pipe(
        hydrate([undefined, SearchParams]),
-        map(buildHistoryContentsUrl(windowSize)),
+        map(([id, params, hid]) => {
+            const baseUrl = `/api/histories/${id}/contents/near/${hid}/${windowSize}`;
+            return `${baseUrl}?${params.historyContentQueryString}`;
+        }),
        throttleDistinct({ timeout: onceEvery }),
        map(prependPath),
        dateAppender({ dateStore }),
@@ -94,41 +97,6 @@ export const loadHistoryContents = (cfg = {}) => (rawInputs$) => {
    );
 };

-// TODO: method in history model maybe? Or maybe Searchparams?
-export const buildHistoryContentsUrl = (windowSize) => (inputs) => {
-    const [historyId, filters, hid] = inputs;
-
-    // Filtering
-    const { showDeleted, showHidden, showVisible } = filters;
-    let deletedClause = "deleted=False";
-    let visibleClause = "visible=True";
-    if (showDeleted) {
-        deletedClause = "deleted=True";
-        visibleClause = "";
-    }
-    if (showHidden) {
-        deletedClause = "";
-        if (showVisible) {
-            visibleClause = "";
-        } else {
-            visibleClause = "visible=False";
-        }
-    }
-    if (showDeleted && showHidden) {
-        deletedClause = "deleted=True";
-        visibleClause = "visible=False";
-    }
-
-    const filterMap = filters.parseTextFilter();
-    const textfilters = Array.from(filterMap.entries()).map(([field, val]) => `${field}-contains=${val}`);
-
-    const parts = [deletedClause, visibleClause, ...textfilters];
-    const baseUrl = `/api/histories/${historyId}/contents/near/${hid}/${windowSize}`;
-    const qs = parts.filter((o) => o.length).join("&");
-
-    return `${baseUrl}?${qs}`;
-};
-
 /**
 * Once data was cached, there's no need to send everything back over into the
 * main thread since the cache watcher will pick up those new values. This just
@@ -82,68 +82,33 @@ export const SEEK = { ASC: "asc", DESC: "desc" };
 * Can't use skip because there might be big un-cached regions of the history
 * and we need to be able to select without loading everything
 */
-// prettier-ignore
 export const buildContentPouchRequest = (cfg = {}) => (inputs) => {
    const { limit = SearchParams.pageSize, seek = SEEK.DESC } = cfg;
-    const [ history_id, params, hid ] = inputs;
+    const [history_id, params, hid] = inputs;

-    // look up or down from target
+    // look up or down from target hid
    const targetId = buildContentId({ history_id, hid });
-    const comparator = (seek == SEEK.ASC) ? "$gt" : "$lte";
+    const comparator = seek == SEEK.ASC ? "$gt" : "$lte";

-    const filters = buildContentSelectorFromParams(params);
-    const fieldNames = new Set(['_id', 'history_id', ...Object.keys(filters)]);
-    const idxName = "idx-" + Array.from(fieldNames).join("-");
+    // index, will build if not existent
+    const filterFields = Array.from(params.criteria.keys()).map((f) => params.getPouchFieldName(f));
+    const fields = ["_id", "history_id", ...filterFields];
+    const ddoc = "idx-" + fields.sort().join("-");

-    const request = {
+    return {
        selector: {
-            _id: { [comparator]: targetId },
-            history_id: { $eq: history_id },
-            ...filters,
+            $and: [
+                // doc id + direction (up/down)
+                { _id: { [comparator]: targetId } },
+                { history_id: history_id },
+                ...params.pouchFilters,
+            ],
        },
-        sort: [{"_id": seek }],
+        sort: [{ _id: seek }],
        limit,
        index: {
-            fields: Array.from(fieldNames),
-            ddoc: idxName,
-        }
+            fields,
+            ddoc,
+        },
    };
-
-    return request;
 };
-
-/**
- * Build search selector for params filters:
- * deleted, visible, text search
- *
- * @param {SearchParams} params
- */
-// prettier-ignore
-export function buildContentSelectorFromParams(params) {
-    const selector = {
-        visible: { $eq: true },
-        isDeleted: { $eq: false },
-    };
-
-    if (params.showDeleted) {
-        delete selector.visible;
-        selector.isDeleted = { $eq: true };
-    }
-
-    if (params.showHidden) {
-        delete selector.isDeleted;
-        selector.visible = { $eq: false };
-    }
-
-    if (params.showDeleted && params.showHidden) {
-        selector.visible = { $eq: false };
-        selector.isDeleted = { $eq: true };
-    }
-
-    const textFields = params.parseTextFilter();
-    for (const [field, val] of textFields.entries()) {
-        selector[field] = { $regex: new RegExp(val, "gi") };
-    }
-
-    return selector;
-}
@@ -19,6 +19,8 @@ import historyContent from "../test/json/historyContent.json";
 beforeEach(wipeDatabase);
 afterEach(wipeDatabase);

+const selectorHasField = (selector, field) => selector.$and.some((row) => row[field] !== undefined);
+
 describe("buildContentPouchRequest", () => {
    test("should turn inputs into a pouch request suitable for find", () => {
        const fn = buildContentPouchRequest();
@@ -29,8 +31,10 @@ describe("buildContentPouchRequest", () => {
        const request = fn(inputs);

        expect(request).toBeDefined();
-        expect(request.selector._id).toBeDefined();
-        expect(request.selector.history_id).toBeDefined();
+        expect(request.selector).toBeDefined();
+        expect(request.selector.$and).toBeDefined();
+        expect(selectorHasField(request.selector, "_id")).toBeTruthy();
+        expect(selectorHasField(request.selector, "history_id")).toBeTruthy();
    });
 });

@@ -1,23 +1,49 @@
 import config from "config";
 import deepEqual from "deep-equal";
+import { isString } from "underscore";

 const pairSplitRE = /(\w+=\w+)|(\w+="(\w|\s)+")/g;
 const scrubFieldRE = /[^\w]/g;
 const scrubQuotesRE = /'|"/g;
-const scrubSpaceRE = /\s+/g;

-// Fields thata can be used for text searches
+// Fields that can be used for text searches
 const validTextFields = new Set([
    "name",
    "history_content_type",
-    "file_ext",
+    "type",
+    "format",
    "extension",
    "misc_info",
    "state",
    "hid",
+    "database",
+    "annotation",
+    "description",
    "tag",
+    "tags",
 ]);

+// alias search field to internal field (requestd name: pouch field name)
+const pouchFieldAlias = {
+    format: "file_ext",
+    database: "genome_build",
+    description: "annotation",
+    tag: "tags",
+    deleted: "isDeleted",
+    type: "history_content_type",
+};
+
+// maps user-field -> server querystring field
+// if maps to null, that filter not available on server
+const serverFieldAlias = {
+    tags: "tag",
+    file_ext: null,
+    genome_build: null,
+};
+
+// Convert actualy boolean into objectively incorrect python value our server accepts
+const dumbBool = (val) => (val ? "True" : "False");
+
 export class SearchParams {
    constructor(props = {}) {
        this.filterText = "";
@@ -40,8 +66,9 @@ export class SearchParams {
        return { filterText, showDeleted, showHidden };
    }

-    // Filtering, turns field=val into an object we can use to build selectors
-    parseTextFilter() {
+    // Filtering, parses single text input into a map of field->value
+    // in the case of multiples, maps to field -> [value, value]
+    get textCriteria() {
        const raw = this.filterText;

        const result = new Map();
@@ -50,17 +77,103 @@ export class SearchParams {
        let matches = raw.match(pairSplitRE);
        if (matches === null && raw.length) matches = [`name=${raw}`];

-        return matches.reduce((result, pair) => {
+        const criteria = matches.reduce((result, pair) => {
            const [field, val] = pair.split("=");
-            const cleanField = field.replace(scrubFieldRE, "");

-            if (validTextFields.has(cleanField)) {
-                const cleanVal = val.replace(scrubQuotesRE, "").replace(scrubSpaceRE, " ");
-                result.set(cleanField, cleanVal);
+            if (validTextFields.has(field)) {
+                let cleanVal = val.replace(scrubQuotesRE, "");
+                // set an array of criteria if we have multiples of the same field name
+                if (result.has(field)) {
+                    cleanVal = [result.get(field), cleanVal].flat();
+                }
+
+                result.set(field, cleanVal);
            }

            return result;
        }, result);
+
+        return criteria;
+    }
+
+    // all criteria, map of field->value, includes our non-standard boolean filters
+    get criteria() {
+        const criteria = new Map(this.textCriteria);
+        criteria.set("visible", !this.showHidden);
+        criteria.set("deleted", this.showDeleted);
+        return criteria;
+    }
+
+    // Generates an array of pouchDB selector objects
+    // { field: { $operator: value }}
+    get pouchFilters() {
+        const filters = Array.from(this.criteria)
+            // generate multiple objects for duplicated field=val entries
+            // these will be AND-ed together in the final pouch selector
+            // map userfield to pouchfield, userfield = what the user typed in the box
+            // pouchfield = the actual field in the cache
+            .map(([userField, val]) => [this.getPouchFieldName(userField), val])
+            .map(([pouchField, val]) => {
+                const vals = Array.isArray(val) ? val : [val];
+                return vals.map((v) => [pouchField, v]);
+            })
+            .flat()
+            .map(([pouchField, val]) => {
+                const comparator = isString(val) ? { $regex: new RegExp(val) } : { $eq: val };
+                return { [pouchField]: comparator };
+            });
+
+        return filters;
+    }
+
+    // render a query string for use in querying content from the contnts/near endoint
+    get historyContentQueryString() {
+        const parts = Array.from(this.criteria).map(([userField, val]) => {
+            const serverField = this.getServerFieldName(userField);
+
+            switch (serverField) {
+                // some client-side filters do not correspond to filters on the server
+                // they can be used to filter local results but will not affect the polling
+                // TODO: consider adding them as available filter options on the api?
+                case null:
+                    return "";
+
+                // This is advertised to work, but is broken on the current api
+                case "annotation":
+                case "description":
+                    return "";
+
+                // non-standard REST bools
+                // deleted serverField was reserved by pouchDB, needed to rename it to "isDeleted"
+                case "deleted":
+                case "visible":
+                    return `${serverField}=${dumbBool(val)}`;
+
+                // no text searching in some fields
+                case "hid":
+                case "state":
+                case "history_content_type":
+                case "type":
+                    return `${serverField}=${val}`;
+
+                // assume text-contains search
+                default:
+                    return `${serverField}-contains=${encodeURIComponent(val)}`;
+            }
+        });
+
+        return parts.filter((o) => o.length).join("&");
+    }
+
+    // maps friendly user field name to internal data field if necessary
+    getPouchFieldName(field) {
+        const cleanName = field.replace(scrubFieldRE, "");
+        return cleanName in pouchFieldAlias ? pouchFieldAlias[cleanName] : cleanName;
+    }
+
+    getServerFieldName(field) {
+        const cleanName = field.replace(scrubFieldRE, "");
+        return cleanName in serverFieldAlias ? serverFieldAlias[cleanName] : cleanName;
    }

    // output current state to log
@@ -79,7 +192,7 @@ export class SearchParams {

 // Statics

-SearchParams.pageSize = config.caching.pageSize;
+SearchParams.pageSize = config?.caching?.pageSize || 50;

 SearchParams.equals = function (a, b) {
    return deepEqual(a.export(), b.export());
@@ -0,0 +1,378 @@
+// I'm using this as the reference for the requirement:
+// https://training.galaxyproject.org/training-material/topics/galaxy-interface/tutorials/history/tutorial.html#advanced-searching
+
+import { SearchParams } from "./SearchParams";
+import { matchesSelector } from "pouchdb-selector-core";
+import { STATES } from "../model/states";
+// import { show } from "jest/helpers";
+
+// #region Test generators
+
+const testPouchSelector = (userField, searchVal, goodDoc, badDoc) => {
+    let pouchFieldName;
+    let params;
+    let selector;
+
+    beforeEach(() => {
+        params = new SearchParams();
+        params.filterText = `${userField}="${searchVal}"`;
+        selector = { $and: params.pouchFilters };
+        pouchFieldName = params.getPouchFieldName(userField);
+    });
+
+    describe("pouchdb selector", () => {
+        it("should have a row in the selector", () => {
+            const hasRow = selector.$and.some((row) => row[pouchFieldName] !== undefined);
+            expect(hasRow).toBeTruthy();
+        });
+
+        it("should return results with matching name field", () => {
+            expect(matchesSelector(goodDoc, selector)).toBeTruthy();
+        });
+
+        it("should not return results where the name string appears in other fields", () => {
+            expect(matchesSelector(badDoc, selector)).toBeFalsy();
+        });
+    });
+};
+
+const testUrlGeneration = (userField, searchVal) => {
+    describe("request querystring", () => {
+        it("should generate a query string with the expected server-side parameter", () => {
+            const params = new SearchParams();
+            params.filterText = `${userField}="${searchVal}"`;
+            const serverField = params.getServerFieldName(userField);
+            if (serverField) {
+                const qs = params.historyContentQueryString;
+                const queryParam = `${serverField}-contains=${encodeURIComponent(searchVal)}`;
+                expect(qs.includes(queryParam)).toBeTruthy();
+            }
+        });
+    });
+};
+
+// TODO: fix api to actually process the fields that it says it will
+const testNotInUrl = (userField, searchVal) => {
+    describe("request querystring", () => {
+        it("should not send this filter, because it does not work on the api as described", () => {
+            const params = new SearchParams();
+            params.filterText = `${userField}="${searchVal}"`;
+            const serverField = params.getServerFieldName(userField);
+            const qs = params.historyContentQueryString;
+            expect(qs.includes(serverField)).toBeFalsy();
+        });
+    });
+};
+
+//#endregion
+
+describe("history contents search field text parsing", () => {
+    describe("single field criteria", () => {
+        // name="FASTQC on"
+        // Any datasets with “FASTQC on” in the title, but avoids items which
+        // have “FASTQC on” in other fields like the description or annotation.
+        describe("name", () => {
+            const searchField = "name";
+            const searchTerm = "FASTQS on";
+
+            const goodDoc = {
+                _id: "anything",
+                name: "asfasdfasdfasdf FASTQS onsdfasdfasdf",
+                visible: true,
+                isDeleted: false,
+            };
+
+            const badDoc = {
+                _id: "anything",
+                otherField: "asfasdfasdfasdf FASTQS onsdfasdfasdf",
+                visible: true,
+                isDeleted: false,
+            };
+
+            testPouchSelector(searchField, searchTerm, goodDoc, badDoc);
+            testUrlGeneration(searchField, searchTerm);
+        });
+
+        // format=vcf
+        // Datasets with a specific format. Some formats are hierarchical, e.g.
+        // searching for fastq will find fastq files but also fastqsanger and
+        // fastqillumina files. You can see more formats in the upload dialogue
+        describe("format", () => {
+            const searchField = "format";
+            const searchTerm = "vcf";
+
+            const goodDoc = {
+                _id: "anything",
+                file_ext: "vcf",
+                visible: true,
+                isDeleted: false,
+            };
+
+            const badDoc = {
+                _id: "anything",
+                file_ext: "notgonnamatch",
+                visible: true,
+                isDeleted: false,
+            };
+
+            testPouchSelector(searchField, searchTerm, goodDoc, badDoc);
+            testUrlGeneration(searchField, searchTerm);
+        });
+
+        // database=hg19 Datasets with a specific reference genome
+        describe("database", () => {
+            const searchField = "database";
+            const searchTerm = "hg19";
+
+            const goodDoc = {
+                _id: "anything",
+                genome_build: "hg19",
+                visible: true,
+                isDeleted: false,
+            };
+
+            const badDoc = {
+                _id: "anything",
+                genome_build: "somethingelse",
+                visible: true,
+                isDeleted: false,
+            };
+
+            testPouchSelector(searchField, searchTerm, goodDoc, badDoc);
+            testUrlGeneration(searchField, searchTerm);
+        });
+
+        // annotation="first of five"
+        describe("annotation", () => {
+            const searchField = "annotation";
+            const searchTerm = "first of fiv";
+
+            const goodDoc = {
+                _id: "anything",
+                annotation: "this is the first of five",
+                visible: true,
+                isDeleted: false,
+            };
+
+            const badDoc = {
+                _id: "anything",
+                annotation: "somethingelse",
+                visible: true,
+                isDeleted: false,
+            };
+
+            testPouchSelector(searchField, searchTerm, goodDoc, badDoc);
+            testNotInUrl(searchField, searchTerm);
+        });
+
+        // NOTE: is annotation different from description?
+        // description="This is data of a Borneo Orangutan" for dataset summary
+        describe("description (same as annotation?)", () => {
+            const searchField = "description";
+            const searchTerm = "This is data of a Borneo Orangutan";
+
+            const goodDoc = {
+                _id: "anything",
+                annotation: "asdfasdfThis is data of a Borneo Orangutansdfasdf",
+                visible: true,
+                isDeleted: false,
+            };
+
+            const badDoc = {
+                _id: "anything",
+                annotation: "somethingelse",
+                visible: true,
+                isDeleted: false,
+            };
+
+            testPouchSelector(searchField, searchTerm, goodDoc, badDoc);
+            testNotInUrl(searchField, searchTerm);
+        });
+
+        // info="started mapping"
+        // for searching on job’s info field.
+        // describe("job info", () => {});
+
+        // tag=experiment1 tag=to_publish
+        // for searching on (a partial) dataset tag.
+        describe("tags", () => {
+            const searchField = "tags";
+            const searchTerm = "experiment1";
+
+            const goodDoc = {
+                _id: "anything",
+                tags: ["experiment1", "experiment2"],
+                visible: true,
+                isDeleted: false,
+            };
+
+            const badDoc = {
+                _id: "anything",
+                tags: ["nodice"],
+                visible: true,
+                isDeleted: false,
+            };
+
+            testPouchSelector(searchField, searchTerm, goodDoc, badDoc);
+            testUrlGeneration(searchField, searchTerm);
+        });
+
+        // hid=25
+        // A specific history item ID (based on the ordering in the history)
+        describe("hid", () => {
+            const searchField = "hid";
+            const searchTerm = 25;
+
+            const goodDoc = {
+                _id: "anything",
+                hid: 25,
+                visible: true,
+                isDeleted: false,
+            };
+
+            const badDoc = {
+                _id: "anything",
+                hid: 200,
+                visible: true,
+                isDeleted: false,
+            };
+
+            testPouchSelector(searchField, searchTerm, goodDoc, badDoc);
+
+            describe("request querystring", () => {
+                it("hid should show up with an =, no text-search", () => {
+                    const params = new SearchParams();
+                    params.filterText = `${searchField}="${searchTerm}"`;
+                    const qs = params.historyContentQueryString;
+                    const serverField = params.getServerFieldName(searchField);
+                    const fragment = `${serverField}=${searchTerm}`;
+                    expect(qs.includes(fragment)).toBeTruthy();
+                });
+            });
+        });
+
+        // state=error
+        // To show only datasets in a given state. Other options include ok,
+        // running, paused, and new.
+        describe("state", () => {
+            const searchField = "state";
+            const searchTerm = STATES.OK;
+
+            const goodDoc = {
+                _id: "anything",
+                hid: 25,
+                visible: true,
+                state: STATES.OK,
+                isDeleted: false,
+            };
+
+            const badDoc = {
+                _id: "anything",
+                hid: 200,
+                visible: true,
+                isDeleted: false,
+                state: STATES.ERROR,
+            };
+
+            testPouchSelector(searchField, searchTerm, goodDoc, badDoc);
+
+            describe("request querystring", () => {
+                it("state should show up with an =, no text-search", () => {
+                    const params = new SearchParams();
+                    params.filterText = `${searchField}="${searchTerm}"`;
+                    const qs = params.historyContentQueryString;
+                    const serverField = params.getServerFieldName(searchField);
+                    const fragment = `${serverField}=${searchTerm}`;
+                    expect(qs.includes(fragment)).toBeTruthy();
+                });
+            });
+        });
+    });
+
+    describe("multiple field criteria", () => {
+        it("should allow a boolean AND combination of 2 filters", () => {
+            const goodDoc = {
+                _id: "anything",
+                name: "sadfasdffooasdfasdf",
+                tags: ["abc", "def", "blah"],
+                visible: true,
+                isDeleted: false,
+            };
+
+            const onlyOneMatch = {
+                _id: "anything",
+                name: "sadfasdffooasdfasdf",
+                visible: true,
+                isDeleted: false,
+            };
+
+            const theOtherMatch = {
+                _id: "anything",
+                tags: ["abc", "def", "blah"],
+                visible: true,
+                isDeleted: false,
+            };
+
+            const params = new SearchParams();
+            params.filterText = "name=foo tag=blah";
+
+            const selector = { $and: params.pouchFilters };
+
+            expect(matchesSelector(goodDoc, selector)).toBeTruthy();
+            expect(matchesSelector(onlyOneMatch, selector)).toBeFalsy();
+            expect(matchesSelector(theOtherMatch, selector)).toBeFalsy();
+        });
+    });
+
+    // TODO: There is a bug in the mongo query syntax implementation that stops us from having
+    // two simultaneous criteria for the same field with the same type of operator, example
+    // can't have { $and: [ { tag: { $regex: /abc/ }, { tag: { $regex: /def/ }}]}
+    // https://github.com/pouchdb/pouchdb/issues/8265
+
+    // I'll have to come up with a better solution for that AND intersection feature.
+
+    xdescribe("deferring this functionality for now", () => {
+        it("should allow a boolean AND combination of 2 of the same filter", () => {
+            const doc = {
+                _id: "anything",
+                name: "abc def",
+                visible: true,
+                isDeleted: false,
+            };
+
+            // this should match
+            const params = new SearchParams();
+            params.filterText = "name=abc name=def";
+            const selector = { $and: params.pouchFilters };
+            expect(matchesSelector(doc, selector)).toBeTruthy();
+
+            // this should not, it fails because of the bug which overwrites the regex on the field,
+            // making it impossible to run 2 regexes against "name"
+            const params2 = new SearchParams();
+            params2.filterText = "name=sfsfssf name=def";
+            const selector2 = params2.buildHistoryContentSelector();
+            expect(matchesSelector(doc, selector2)).toBeFalsy();
+        });
+
+        it("should allow a boolean AND combination of 2 filters of for the same field, partial matches", () => {
+            const doc = {
+                _id: "anything",
+                tags: ["abcasdfasdf", "def"],
+                visible: true,
+                isDeleted: false,
+            };
+
+            // this should match
+            const params = new SearchParams();
+            params.filterText = "tags=abc tag=def";
+            const selector = { $and: params.pouchFilters };
+            expect(matchesSelector(doc, selector)).toBeTruthy();
+
+            // this should not
+            const params2 = new SearchParams();
+            params2.filterText = "tags=foo tag=def";
+            const selector2 = { $and: params2.pouchFilters };
+            expect(matchesSelector(doc, selector2)).toBeFalsy();
+        });
+    });
+});