From 2b16ba490430a3ecef07ea4c0407f352ec23bc93 Mon Sep 17 00:00:00 2001 From: Mason Houtz Date: Fri, 29 Jan 2021 12:14:48 -0800 Subject: [PATCH] added keyword search to history text filter, tests --- .../History/caching/loadHistoryContents.js | 40 +- .../History/caching/monitorHistoryContent.js | 71 +--- .../caching/monitorHistoryContent.test.js | 8 +- .../components/History/model/SearchParams.js | 135 ++++++- .../History/model/SearchParams.test.js | 378 ++++++++++++++++++ 5 files changed, 530 insertions(+), 102 deletions(-) create mode 100644 client/src/components/History/model/SearchParams.test.js diff --git a/client/src/components/History/caching/loadHistoryContents.js b/client/src/components/History/caching/loadHistoryContents.js index 47de04d480e..82b471c8505 100644 --- a/client/src/components/History/caching/loadHistoryContents.js +++ b/client/src/components/History/caching/loadHistoryContents.js @@ -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 diff --git a/client/src/components/History/caching/monitorHistoryContent.js b/client/src/components/History/caching/monitorHistoryContent.js index 7be70b722a0..c0ac9e1f573 100644 --- a/client/src/components/History/caching/monitorHistoryContent.js +++ b/client/src/components/History/caching/monitorHistoryContent.js @@ -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; -} diff --git a/client/src/components/History/caching/monitorHistoryContent.test.js b/client/src/components/History/caching/monitorHistoryContent.test.js index ad386d1bbf2..60f35bbefcf 100644 --- a/client/src/components/History/caching/monitorHistoryContent.test.js +++ b/client/src/components/History/caching/monitorHistoryContent.test.js @@ -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(); }); }); diff --git a/client/src/components/History/model/SearchParams.js b/client/src/components/History/model/SearchParams.js index fb6a7c40c17..92d70b8b342 100644 --- a/client/src/components/History/model/SearchParams.js +++ b/client/src/components/History/model/SearchParams.js @@ -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()); diff --git a/client/src/components/History/model/SearchParams.test.js b/client/src/components/History/model/SearchParams.test.js new file mode 100644 index 00000000000..6ad8cea18c5 --- /dev/null +++ b/client/src/components/History/model/SearchParams.test.js @@ -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(); + }); + }); +});