diff --git a/codecov.yml b/codecov.yml index aab53fc30c..d311f97a8d 100644 --- a/codecov.yml +++ b/codecov.yml @@ -1,6 +1,10 @@ codecov: require_ci_to_pass: false #This setting will update the bot regardless of whether or not tests pass +# Disabling annotations for now. They are incorrectly labelling lines as lacking coverage when they are in fact covered by tests. +github_checks: + annotations: false + coverage: status: project: diff --git a/e2e/.eslintrc.cjs b/e2e/.eslintrc.cjs index a8fefcb27e..92c13d932b 100644 --- a/e2e/.eslintrc.cjs +++ b/e2e/.eslintrc.cjs @@ -2,7 +2,6 @@ module.exports = { extends: ['plugin:playwright/recommended'], rules: { - 'playwright/max-nested-describe': ['error', { max: 1 }], 'playwright/expect-expect': 'off' }, overrides: [ diff --git a/e2e/baseFixtures.js b/e2e/baseFixtures.js index 6d0662a875..4476a09093 100644 --- a/e2e/baseFixtures.js +++ b/e2e/baseFixtures.js @@ -103,25 +103,40 @@ const extendedTest = test.extend({ * Default: `true` */ failOnConsoleError: [true, { option: true }], + ignore404s: [[], { option: true }], /** * Extends the base page class to enable console log error detection. * @see {@link https://github.com/microsoft/playwright/discussions/11690 Github Discussion} */ - page: async ({ page, failOnConsoleError }, use) => { + page: async ({ page, failOnConsoleError, ignore404s }, use) => { // Capture any console errors during test execution - const messages = []; + let messages = []; page.on('console', (msg) => messages.push(msg)); await use(page); + if (ignore404s.length > 0) { + messages = messages.filter((msg) => { + let keep = true; + + if (msg.text().match(/404 \((Object )?Not Found\)/) !== null) { + keep = ignore404s.every((ignoreRule) => { + return msg.location().url.match(ignoreRule) === null; + }); + } + + return keep; + }); + } + // Assert against console errors during teardown if (failOnConsoleError) { - messages.forEach((msg) => + messages.forEach((msg) => { // eslint-disable-next-line playwright/no-standalone-expect expect .soft(msg.type(), `Console error detected: ${_consoleMessageToString(msg)}`) - .not.toEqual('error') - ); + .not.toEqual('error'); + }); } } }); diff --git a/e2e/tests/functional/search.e2e.spec.js b/e2e/tests/functional/search.e2e.spec.js index f0a7357a13..e175e53bc5 100644 --- a/e2e/tests/functional/search.e2e.spec.js +++ b/e2e/tests/functional/search.e2e.spec.js @@ -31,6 +31,8 @@ import { expect, test } from '../../pluginFixtures.js'; test.describe('Grand Search', () => { let grandSearchInput; + test.use({ ignore404s: [/_design\/object_names\/_view\/object_names$/] }); + test.beforeEach(async ({ page }) => { grandSearchInput = page .getByLabel('OpenMCT Search') @@ -191,7 +193,88 @@ test.describe('Grand Search', () => { await expect(searchResults).toContainText(folderName); }); + test.describe('Search will test for the presence of the object_names index, and', () => { + test('use index if available @couchdb @network', async ({ page }) => { + await createObjectsForSearch(page); + + let isObjectNamesViewAvailable = false; + let isObjectNamesUsedForSearch = false; + + page.on('request', async (request) => { + const isObjectNamesRequest = request.url().endsWith('_view/object_names'); + const isHeadRequest = request.method().toLowerCase() === 'head'; + + if (isObjectNamesRequest && isHeadRequest) { + const response = await request.response(); + isObjectNamesViewAvailable = response.status() === 200; + } + }); + + page.on('request', (request) => { + const isObjectNamesRequest = request.url().endsWith('_view/object_names'); + const isPostRequest = request.method().toLowerCase() === 'post'; + + if (isObjectNamesRequest && isPostRequest) { + isObjectNamesUsedForSearch = true; + } + }); + + // Full search for object + await grandSearchInput.pressSequentially('Clock', { delay: 100 }); + + // Wait for search to finish + await waitForSearchCompletion(page); + + expect(isObjectNamesViewAvailable).toBe(true); + expect(isObjectNamesUsedForSearch).toBe(true); + }); + + test('fall-back on base index if index not available @couchdb @network', async ({ page }) => { + await page.route('**/_view/object_names', (route) => { + route.fulfill({ + status: 404 + }); + }); + await createObjectsForSearch(page); + + let isObjectNamesViewAvailable = false; + let isFindUsedForSearch = false; + + page.on('request', async (request) => { + const isObjectNamesRequest = request.url().endsWith('_view/object_names'); + const isHeadRequest = request.method().toLowerCase() === 'head'; + + if (isObjectNamesRequest && isHeadRequest) { + const response = await request.response(); + isObjectNamesViewAvailable = response.status() === 200; + } + }); + + page.on('request', (request) => { + const isFindRequest = request.url().endsWith('_find'); + const isPostRequest = request.method().toLowerCase() === 'post'; + + if (isFindRequest && isPostRequest) { + isFindUsedForSearch = true; + } + }); + + // Full search for object + await grandSearchInput.pressSequentially('Clock', { delay: 100 }); + + // Wait for search to finish + await waitForSearchCompletion(page); + console.info( + `isObjectNamesViewAvailable: ${isObjectNamesViewAvailable} | isFindUsedForSearch: ${isFindUsedForSearch}` + ); + expect(isObjectNamesViewAvailable).toBe(false); + expect(isFindUsedForSearch).toBe(true); + }); + }); + test('Search results are debounced @couchdb @network', async ({ page }) => { + // Unfortunately 404s are always logged to the JavaScript console and can't be suppressed + // A 404 is now thrown when we test for the presence of the object names view used by search. test.info().annotations.push({ type: 'issue', description: 'https://github.com/nasa/openmct/issues/6179' @@ -199,11 +282,17 @@ test.describe('Grand Search', () => { await createObjectsForSearch(page); let networkRequests = []; + page.on('request', (request) => { - const searchRequest = - request.url().endsWith('_find') || request.url().includes('by_keystring'); - const fetchRequest = request.resourceType() === 'fetch'; - if (searchRequest && fetchRequest) { + const isSearchRequest = + request.url().endsWith('object_names') || + request.url().endsWith('_find') || + request.url().includes('by_keystring'); + const isFetchRequest = request.resourceType() === 'fetch'; + // CouchDB search results in a one-time head request to test for the presence of an index. + const isHeadRequest = request.method().toLowerCase() === 'head'; + + if (isSearchRequest && isFetchRequest && !isHeadRequest) { networkRequests.push(request); } }); diff --git a/e2e/tests/performance/memory/navigation.memory.perf.spec.js b/e2e/tests/performance/memory/navigation.memory.perf.spec.js index 022c0bb1c2..854561cc15 100644 --- a/e2e/tests/performance/memory/navigation.memory.perf.spec.js +++ b/e2e/tests/performance/memory/navigation.memory.perf.spec.js @@ -213,7 +213,6 @@ test.describe('Navigation memory leak is not detected in', () => { page, 'example-imagery-memory-leak-test' ); - // If we got here without timing out, then the root view object was garbage collected and no memory leak was detected. expect(result).toBe(true); }); @@ -317,6 +316,12 @@ test.describe('Navigation memory leak is not detected in', () => { // Manually invoke the garbage collector once all references are removed. window.gc(); + window.gc(); + window.gc(); + + setTimeout(() => { + window.gc(); + }, 1000); return gcPromise; }); diff --git a/src/api/composition/CompositionCollection.js b/src/api/composition/CompositionCollection.js index e4da621344..3a32b1fd33 100644 --- a/src/api/composition/CompositionCollection.js +++ b/src/api/composition/CompositionCollection.js @@ -20,6 +20,8 @@ * at runtime from the About dialog for additional information. *****************************************************************************/ +import { isIdentifier } from '../objects/object-utils'; + /** * @typedef {import('openmct').DomainObject} DomainObject */ @@ -209,9 +211,15 @@ export default class CompositionCollection { this.#cleanUpMutables(); const children = await this.#provider.load(this.domainObject); const childObjects = await Promise.all( - children.map((c) => this.#publicAPI.objects.get(c, abortSignal)) + children.map((child) => { + if (isIdentifier(child)) { + return this.#publicAPI.objects.get(child, abortSignal); + } else { + return Promise.resolve(child); + } + }) ); - childObjects.forEach((c) => this.add(c, true)); + childObjects.forEach((child) => this.add(child, true)); this.#emit('load'); return childObjects; diff --git a/src/api/composition/CompositionProvider.js b/src/api/composition/CompositionProvider.js index c663352198..d785f921a7 100644 --- a/src/api/composition/CompositionProvider.js +++ b/src/api/composition/CompositionProvider.js @@ -96,8 +96,9 @@ export default class CompositionProvider { * object. * @param {DomainObject} domainObject the domain object * for which to load composition - * @returns {Promise} a promise for - * the Identifiers in this composition + * @returns {Promise} a promise for + * the Identifiers or Domain Objects in this composition. If Identifiers are returned, + * they will be automatically resolved to domain objects by the API. */ load(domainObject) { throw new Error('This method must be implemented by a subclass.'); diff --git a/src/api/composition/DefaultCompositionProvider.js b/src/api/composition/DefaultCompositionProvider.js index 91567c2fe8..47c6d84b50 100644 --- a/src/api/composition/DefaultCompositionProvider.js +++ b/src/api/composition/DefaultCompositionProvider.js @@ -21,7 +21,7 @@ *****************************************************************************/ import { toRaw } from 'vue'; -import { makeKeyString } from '../objects/object-utils.js'; +import { makeKeyString, parseKeyString } from '../objects/object-utils.js'; import CompositionProvider from './CompositionProvider.js'; /** @@ -75,7 +75,11 @@ export default class DefaultCompositionProvider extends CompositionProvider { * the Identifiers in this composition */ load(domainObject) { - return Promise.all(domainObject.composition); + const identifiers = domainObject.composition + .filter((idOrKeystring) => idOrKeystring !== null && idOrKeystring !== undefined) + .map((idOrKeystring) => parseKeyString(idOrKeystring)); + + return Promise.all(identifiers); } /** * Attach listeners for changes to the composition of a given domain object. diff --git a/src/api/objects/ObjectAPI.js b/src/api/objects/ObjectAPI.js index c1be1581f6..c812c8ff6c 100644 --- a/src/api/objects/ObjectAPI.js +++ b/src/api/objects/ObjectAPI.js @@ -27,6 +27,7 @@ import ConflictError from './ConflictError.js'; import InMemorySearchProvider from './InMemorySearchProvider.js'; import InterceptorRegistry from './InterceptorRegistry.js'; import MutableDomainObject from './MutableDomainObject.js'; +import { isIdentifier, isKeyString } from './object-utils.js'; import RootObjectProvider from './RootObjectProvider.js'; import RootRegistry from './RootRegistry.js'; import Transaction from './Transaction.js'; @@ -742,11 +743,19 @@ export default class ObjectAPI { * @param {AbortSignal} abortSignal (optional) signal to abort fetch requests * @returns {Promise>} a promise containing an array of domain objects */ - async getOriginalPath(identifier, path = [], abortSignal = null) { - const domainObject = await this.get(identifier, abortSignal); + async getOriginalPath(identifierOrObject, path = [], abortSignal = null) { + let domainObject; + + if (isKeyString(identifierOrObject) || isIdentifier(identifierOrObject)) { + domainObject = await this.get(identifierOrObject, abortSignal); + } else { + domainObject = identifierOrObject; + } + if (!domainObject) { return []; } + path.push(domainObject); const { location } = domainObject; if (location && !this.#pathContainsDomainObject(location, path)) { diff --git a/src/plugins/CouchDBSearchFolder/plugin.js b/src/plugins/CouchDBSearchFolder/plugin.js index aa0bad48ed..f823da4810 100644 --- a/src/plugins/CouchDBSearchFolder/plugin.js +++ b/src/plugins/CouchDBSearchFolder/plugin.js @@ -24,6 +24,9 @@ export default function (folderName, couchPlugin, searchFilter) { location: 'ROOT' }); } + }, + search() { + return Promise.resolve([]); } }); @@ -35,9 +38,17 @@ export default function (folderName, couchPlugin, searchFilter) { ); }, load() { - return couchProvider.getObjectsByFilter(searchFilter).then((objects) => { - return objects.map((object) => object.identifier); - }); + let searchResults; + + if (searchFilter.viewName !== undefined) { + // Use a view to search, instead of an _all_docs find + searchResults = couchProvider.getObjectsByView(searchFilter); + } else { + // Use the _find endpoint to search _all_docs + searchResults = couchProvider.getObjectsByFilter(searchFilter); + } + + return searchResults; } }); }; diff --git a/src/plugins/persistence/couch/CouchObjectProvider.js b/src/plugins/persistence/couch/CouchObjectProvider.js index 3348872d98..b5e53848fd 100644 --- a/src/plugins/persistence/couch/CouchObjectProvider.js +++ b/src/plugins/persistence/couch/CouchObjectProvider.js @@ -434,16 +434,69 @@ class CouchObjectProvider { return Promise.resolve([]); } - async getObjectsByView({ designDoc, viewName, keysToSearch }, abortSignal) { - const stringifiedKeys = JSON.stringify(keysToSearch); - const url = `${this.url}/_design/${designDoc}/_view/${viewName}?keys=${stringifiedKeys}&include_docs=true`; + async isViewDefined(designDoc, viewName) { + const url = `${this.url}/_design/${designDoc}/_view/${viewName}`; + const response = await fetch(url, { + method: 'HEAD' + }); + + return response.ok; + } + + /** + * @typedef GetObjectByViewOptions + * @property {String} designDoc the name of the design document that the view belongs to + * @property {String} viewName + * @property {Array.} [keysToSearch] a list of discrete view keys to search for. View keys are not object identifiers. + * @property {String} [startKey] limit the search to a range of keys starting with the provided `startKey`. One of `keysToSearch` OR `startKey` AND `endKey` must be provided + * @property {String} [endKey] limit the search to a range of keys ending with the provided `endKey`. One of `keysToSearch` OR `startKey` AND `endKey` must be provided + * @property {Number} [limit] limit the number of results returned + * @property {String} [objectIdField] The field (either key or value) to treat as an object key. If provided, include_docs will be set to false in the request, and the field will be used as an object identifier. A bulk request will be used to resolve objects from identifiers + */ + /** + * Return objects based on a call to a view. See https://docs.couchdb.org/en/stable/api/ddoc/views.html. + * @param {GetObjectByViewOptions} options + * @param {AbortSignal} abortSignal + * @returns {Promise>} + */ + async getObjectsByView( + { designDoc, viewName, keysToSearch, startKey, endKey, limit, objectIdField }, + abortSignal + ) { + let stringifiedKeys = JSON.stringify(keysToSearch); + const url = `${this.url}/_design/${designDoc}/_view/${viewName}`; + const requestBody = {}; + let requestBodyString; + + if (objectIdField === undefined) { + requestBody.include_docs = true; + } + + if (limit !== undefined) { + requestBody.limit = limit; + } + + if (startKey !== undefined && endKey !== undefined) { + /* spell-checker: disable */ + requestBody.startkey = startKey; + requestBody.endkey = endKey; + requestBodyString = JSON.stringify(requestBody); + requestBodyString = requestBodyString.replace('$START_KEY', startKey); + requestBodyString = requestBodyString.replace('$END_KEY', endKey); + /* spell-checker: enable */ + } else { + requestBody.keys = stringifiedKeys; + requestBodyString = JSON.stringify(requestBody); + } + let objectModels = []; try { const response = await fetch(url, { - method: 'GET', + method: 'POST', headers: { 'Content-Type': 'application/json' }, - signal: abortSignal + signal: abortSignal, + body: requestBodyString }); if (!response.ok) { @@ -454,13 +507,21 @@ class CouchObjectProvider { const result = await response.json(); const couchRows = result.rows; - couchRows.forEach((couchRow) => { - const couchDoc = couchRow.doc; - const objectModel = this.#getModel(couchDoc); - if (objectModel) { - objectModels.push(objectModel); - } - }); + if (objectIdField !== undefined) { + const objectIdsToResolve = []; + couchRows.forEach((couchRow) => { + objectIdsToResolve.push(couchRow[objectIdField]); + }); + objectModels = Object.values(await this.#bulkGet(objectIdsToResolve), abortSignal); + } else { + couchRows.forEach((couchRow) => { + const couchDoc = couchRow.doc; + const objectModel = this.#getModel(couchDoc); + if (objectModel) { + objectModels.push(objectModel); + } + }); + } } catch (error) { // do nothing } diff --git a/src/plugins/persistence/couch/CouchSearchProvider.js b/src/plugins/persistence/couch/CouchSearchProvider.js index 7c0740eff3..c5fa16765c 100644 --- a/src/plugins/persistence/couch/CouchSearchProvider.js +++ b/src/plugins/persistence/couch/CouchSearchProvider.js @@ -33,7 +33,11 @@ class CouchSearchProvider { #bulkPromise; #batchIds; #lastAbortSignal; - + #isSearchByNameViewDefined; + /** + * + * @param {import('./CouchObjectProvider').default} couchObjectProvider + */ constructor(couchObjectProvider) { this.couchObjectProvider = couchObjectProvider; this.searchTypes = couchObjectProvider.openmct.objects.SEARCH_TYPES; @@ -67,18 +71,47 @@ class CouchSearchProvider { } } - searchForObjects(query, abortSignal) { - const filter = { - selector: { - model: { - name: { - $regex: `(?i)${query}` + #isOptimizedSearchByNameSupported() { + let isOptimizedSearchAvailable; + + if (this.#isSearchByNameViewDefined === undefined) { + isOptimizedSearchAvailable = this.#isSearchByNameViewDefined = + this.couchObjectProvider.isViewDefined('object_names', 'object_names'); + } else { + isOptimizedSearchAvailable = this.#isSearchByNameViewDefined; + } + + return isOptimizedSearchAvailable; + } + + async searchForObjects(query, abortSignal) { + const preparedQuery = query.toLowerCase().trim(); + const supportsOptimizedSearchByName = await this.#isOptimizedSearchByNameSupported(); + + if (supportsOptimizedSearchByName) { + return this.couchObjectProvider.getObjectsByView( + { + designDoc: 'object_names', + viewName: 'object_names', + startKey: preparedQuery, + endKey: preparedQuery + `\ufff0`, + objectIdField: 'value', + limit: 1000 + }, + abortSignal + ); + } else { + const filter = { + selector: { + model: { + name: { + $regex: `(?i)${query}` + } } } - } - }; - - return this.couchObjectProvider.getObjectsByFilter(filter, abortSignal); + }; + return this.couchObjectProvider.getObjectsByFilter(filter); + } } async #deferBatchAnnotationSearch() { diff --git a/src/plugins/persistence/couch/pluginSpec.js b/src/plugins/persistence/couch/pluginSpec.js index a925bf407b..a46f67872a 100644 --- a/src/plugins/persistence/couch/pluginSpec.js +++ b/src/plugins/persistence/couch/pluginSpec.js @@ -373,44 +373,6 @@ describe('the plugin', () => { expect(requestMethod).toEqual('PUT'); }); }); - describe('implements server-side search', () => { - let mockPromise; - beforeEach(() => { - mockPromise = Promise.resolve({ - body: { - getReader() { - return { - read() { - return Promise.resolve({ - done: true, - value: undefined - }); - } - }; - } - } - }); - fetch.and.returnValue(mockPromise); - }); - - it("using Couch's 'find' endpoint", async () => { - await Promise.all(openmct.objects.search('test')); - const requestUrl = fetch.calls.mostRecent().args[0]; - - // we only want one call to fetch, not 2! - // see https://github.com/nasa/openmct/issues/4667 - expect(fetch).toHaveBeenCalledTimes(1); - expect(requestUrl.endsWith('_find')).toBeTrue(); - }); - - it('and supports search by object name', async () => { - await Promise.all(openmct.objects.search('test')); - const requestPayload = JSON.parse(fetch.calls.mostRecent().args[1].body); - - expect(requestPayload).toBeDefined(); - expect(requestPayload.selector.model.name.$regex).toEqual('(?i)test'); - }); - }); }); describe('the view', () => { diff --git a/src/plugins/persistence/couch/setup-couchdb.sh b/src/plugins/persistence/couch/setup-couchdb.sh index 88ae9f5323..a49d07ab09 100755 --- a/src/plugins/persistence/couch/setup-couchdb.sh +++ b/src/plugins/persistence/couch/setup-couchdb.sh @@ -99,6 +99,48 @@ create_replicator_table() { add_index_and_views() { echo "Adding index and views to $OPENMCT_DATABASE_NAME database" + # Add object names search index + response=$(curl --silent --user "${CURL_USERPASS_ARG}" --request PUT "$COUCH_BASE_LOCAL"/"$OPENMCT_DATABASE_NAME"/_design/object_names/\ + --header 'Content-Type: application/json' \ + --data '{ + "_id":"_design/object_names", + "views":{ + "object_names":{ + "map":"function(doc) { if (doc.model && doc.model.name) { const name = doc.model.name.toLowerCase().trim(); if (name.length > 0) { emit(name, doc._id); const tokens = name.split(/[^a-zA-Z0-9]/); tokens.forEach((token) => { if (token.length > 0) { emit(token, doc._id); } }); } } }" + } + } + }'); + + if [[ $response =~ "\"ok\":true" ]]; then + echo "Successfully created object_names" + elif [[ $response =~ "\"error\":\"conflict\"" ]]; then + echo "object_names already exists, skipping creation" + else + echo "Unable to create object_names" + echo $response + fi + + # Add object types search index + response=$(curl --silent --user "${CURL_USERPASS_ARG}" --request PUT "$COUCH_BASE_LOCAL"/"$OPENMCT_DATABASE_NAME"/_design/object_types/\ + --header 'Content-Type: application/json' \ + --data '{ + "_id":"_design/object_types", + "views":{ + "object_types":{ + "map":"function(doc) { if (doc.model && doc.model.type) { const type = doc.model.type.toLowerCase().trim(); if (type.length > 0) { emit(type, null); } } }" + } + } + }') + + if [[ $response =~ "\"ok\":true" ]]; then + echo "Successfully created object_types" + elif [[ $response =~ "\"error\":\"conflict\"" ]]; then + echo "object_types already exists, skipping creation" + else + echo "Unable to create object_types" + echo $response + fi + # Add type_tags_index response=$(curl --silent --user "${CURL_USERPASS_ARG}" --request POST "$COUCH_BASE_LOCAL"/"$OPENMCT_DATABASE_NAME"/_index/\ --header 'Content-Type: application/json' \ diff --git a/src/ui/layout/search/GrandSearch.vue b/src/ui/layout/search/GrandSearch.vue index b5d5397125..1d744a2693 100644 --- a/src/ui/layout/search/GrandSearch.vue +++ b/src/ui/layout/search/GrandSearch.vue @@ -111,9 +111,8 @@ export default { return null; } - const keyStringForObject = this.openmct.objects.makeKeyString(domainObject.identifier); const originalPathObjects = await this.openmct.objects.getOriginalPath( - keyStringForObject, + domainObject, [], abortSignal );