Skip to content

Commit 6a450a0

Browse files
authored
Couch search indexes (#8037)
* Defined search index for object names. Add index for searching by object type * Feature detect if views are defined to support optimized search. If not, fall back on filter-based search * Suppress github codedcov annotations for now, they are not accurate and generate noise. * Allow nested describes. They're good. * Add a noop search function to couch search folder object provider. Actual search is provided by Couch provider, but need a stub to prevent in-memory indexing * Adhere to our own interface and ensure identifiers are always returned by default composition provider
1 parent e5631c9 commit 6a450a0

File tree

15 files changed

+321
-85
lines changed

15 files changed

+321
-85
lines changed

codecov.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
codecov:
22
require_ci_to_pass: false #This setting will update the bot regardless of whether or not tests pass
33

4+
# Disabling annotations for now. They are incorrectly labelling lines as lacking coverage when they are in fact covered by tests.
5+
github_checks:
6+
annotations: false
7+
48
coverage:
59
status:
610
project:

e2e/.eslintrc.cjs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
module.exports = {
33
extends: ['plugin:playwright/recommended'],
44
rules: {
5-
'playwright/max-nested-describe': ['error', { max: 1 }],
65
'playwright/expect-expect': 'off'
76
},
87
overrides: [

e2e/baseFixtures.js

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -103,25 +103,40 @@ const extendedTest = test.extend({
103103
* Default: `true`
104104
*/
105105
failOnConsoleError: [true, { option: true }],
106+
ignore404s: [[], { option: true }],
106107
/**
107108
* Extends the base page class to enable console log error detection.
108109
* @see {@link https://github.com/microsoft/playwright/discussions/11690 Github Discussion}
109110
*/
110-
page: async ({ page, failOnConsoleError }, use) => {
111+
page: async ({ page, failOnConsoleError, ignore404s }, use) => {
111112
// Capture any console errors during test execution
112-
const messages = [];
113+
let messages = [];
113114
page.on('console', (msg) => messages.push(msg));
114115

115116
await use(page);
116117

118+
if (ignore404s.length > 0) {
119+
messages = messages.filter((msg) => {
120+
let keep = true;
121+
122+
if (msg.text().match(/404 \((Object )?Not Found\)/) !== null) {
123+
keep = ignore404s.every((ignoreRule) => {
124+
return msg.location().url.match(ignoreRule) === null;
125+
});
126+
}
127+
128+
return keep;
129+
});
130+
}
131+
117132
// Assert against console errors during teardown
118133
if (failOnConsoleError) {
119-
messages.forEach((msg) =>
134+
messages.forEach((msg) => {
120135
// eslint-disable-next-line playwright/no-standalone-expect
121136
expect
122137
.soft(msg.type(), `Console error detected: ${_consoleMessageToString(msg)}`)
123-
.not.toEqual('error')
124-
);
138+
.not.toEqual('error');
139+
});
125140
}
126141
}
127142
});

e2e/tests/functional/search.e2e.spec.js

Lines changed: 93 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ import { expect, test } from '../../pluginFixtures.js';
3131
test.describe('Grand Search', () => {
3232
let grandSearchInput;
3333

34+
test.use({ ignore404s: [/_design\/object_names\/_view\/object_names$/] });
35+
3436
test.beforeEach(async ({ page }) => {
3537
grandSearchInput = page
3638
.getByLabel('OpenMCT Search')
@@ -191,19 +193,106 @@ test.describe('Grand Search', () => {
191193
await expect(searchResults).toContainText(folderName);
192194
});
193195

196+
test.describe('Search will test for the presence of the object_names index, and', () => {
197+
test('use index if available @couchdb @network', async ({ page }) => {
198+
await createObjectsForSearch(page);
199+
200+
let isObjectNamesViewAvailable = false;
201+
let isObjectNamesUsedForSearch = false;
202+
203+
page.on('request', async (request) => {
204+
const isObjectNamesRequest = request.url().endsWith('_view/object_names');
205+
const isHeadRequest = request.method().toLowerCase() === 'head';
206+
207+
if (isObjectNamesRequest && isHeadRequest) {
208+
const response = await request.response();
209+
isObjectNamesViewAvailable = response.status() === 200;
210+
}
211+
});
212+
213+
page.on('request', (request) => {
214+
const isObjectNamesRequest = request.url().endsWith('_view/object_names');
215+
const isPostRequest = request.method().toLowerCase() === 'post';
216+
217+
if (isObjectNamesRequest && isPostRequest) {
218+
isObjectNamesUsedForSearch = true;
219+
}
220+
});
221+
222+
// Full search for object
223+
await grandSearchInput.pressSequentially('Clock', { delay: 100 });
224+
225+
// Wait for search to finish
226+
await waitForSearchCompletion(page);
227+
228+
expect(isObjectNamesViewAvailable).toBe(true);
229+
expect(isObjectNamesUsedForSearch).toBe(true);
230+
});
231+
232+
test('fall-back on base index if index not available @couchdb @network', async ({ page }) => {
233+
await page.route('**/_view/object_names', (route) => {
234+
route.fulfill({
235+
status: 404
236+
});
237+
});
238+
await createObjectsForSearch(page);
239+
240+
let isObjectNamesViewAvailable = false;
241+
let isFindUsedForSearch = false;
242+
243+
page.on('request', async (request) => {
244+
const isObjectNamesRequest = request.url().endsWith('_view/object_names');
245+
const isHeadRequest = request.method().toLowerCase() === 'head';
246+
247+
if (isObjectNamesRequest && isHeadRequest) {
248+
const response = await request.response();
249+
isObjectNamesViewAvailable = response.status() === 200;
250+
}
251+
});
252+
253+
page.on('request', (request) => {
254+
const isFindRequest = request.url().endsWith('_find');
255+
const isPostRequest = request.method().toLowerCase() === 'post';
256+
257+
if (isFindRequest && isPostRequest) {
258+
isFindUsedForSearch = true;
259+
}
260+
});
261+
262+
// Full search for object
263+
await grandSearchInput.pressSequentially('Clock', { delay: 100 });
264+
265+
// Wait for search to finish
266+
await waitForSearchCompletion(page);
267+
console.info(
268+
`isObjectNamesViewAvailable: ${isObjectNamesViewAvailable} | isFindUsedForSearch: ${isFindUsedForSearch}`
269+
);
270+
expect(isObjectNamesViewAvailable).toBe(false);
271+
expect(isFindUsedForSearch).toBe(true);
272+
});
273+
});
274+
194275
test('Search results are debounced @couchdb @network', async ({ page }) => {
276+
// Unfortunately 404s are always logged to the JavaScript console and can't be suppressed
277+
// A 404 is now thrown when we test for the presence of the object names view used by search.
195278
test.info().annotations.push({
196279
type: 'issue',
197280
description: 'https://github.com/nasa/openmct/issues/6179'
198281
});
199282
await createObjectsForSearch(page);
200283

201284
let networkRequests = [];
285+
202286
page.on('request', (request) => {
203-
const searchRequest =
204-
request.url().endsWith('_find') || request.url().includes('by_keystring');
205-
const fetchRequest = request.resourceType() === 'fetch';
206-
if (searchRequest && fetchRequest) {
287+
const isSearchRequest =
288+
request.url().endsWith('object_names') ||
289+
request.url().endsWith('_find') ||
290+
request.url().includes('by_keystring');
291+
const isFetchRequest = request.resourceType() === 'fetch';
292+
// CouchDB search results in a one-time head request to test for the presence of an index.
293+
const isHeadRequest = request.method().toLowerCase() === 'head';
294+
295+
if (isSearchRequest && isFetchRequest && !isHeadRequest) {
207296
networkRequests.push(request);
208297
}
209298
});

e2e/tests/performance/memory/navigation.memory.perf.spec.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,6 @@ test.describe('Navigation memory leak is not detected in', () => {
213213
page,
214214
'example-imagery-memory-leak-test'
215215
);
216-
217216
// If we got here without timing out, then the root view object was garbage collected and no memory leak was detected.
218217
expect(result).toBe(true);
219218
});
@@ -317,6 +316,12 @@ test.describe('Navigation memory leak is not detected in', () => {
317316

318317
// Manually invoke the garbage collector once all references are removed.
319318
window.gc();
319+
window.gc();
320+
window.gc();
321+
322+
setTimeout(() => {
323+
window.gc();
324+
}, 1000);
320325

321326
return gcPromise;
322327
});

src/api/composition/CompositionCollection.js

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
* at runtime from the About dialog for additional information.
2121
*****************************************************************************/
2222

23+
import { isIdentifier } from '../objects/object-utils';
24+
2325
/**
2426
* @typedef {import('openmct').DomainObject} DomainObject
2527
*/
@@ -209,9 +211,15 @@ export default class CompositionCollection {
209211
this.#cleanUpMutables();
210212
const children = await this.#provider.load(this.domainObject);
211213
const childObjects = await Promise.all(
212-
children.map((c) => this.#publicAPI.objects.get(c, abortSignal))
214+
children.map((child) => {
215+
if (isIdentifier(child)) {
216+
return this.#publicAPI.objects.get(child, abortSignal);
217+
} else {
218+
return Promise.resolve(child);
219+
}
220+
})
213221
);
214-
childObjects.forEach((c) => this.add(c, true));
222+
childObjects.forEach((child) => this.add(child, true));
215223
this.#emit('load');
216224

217225
return childObjects;

src/api/composition/CompositionProvider.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,9 @@ export default class CompositionProvider {
9696
* object.
9797
* @param {DomainObject} domainObject the domain object
9898
* for which to load composition
99-
* @returns {Promise<Identifier[]>} a promise for
100-
* the Identifiers in this composition
99+
* @returns {Promise<Identifier[] | DomainObject[]>} a promise for
100+
* the Identifiers or Domain Objects in this composition. If Identifiers are returned,
101+
* they will be automatically resolved to domain objects by the API.
101102
*/
102103
load(domainObject) {
103104
throw new Error('This method must be implemented by a subclass.');

src/api/composition/DefaultCompositionProvider.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
*****************************************************************************/
2222
import { toRaw } from 'vue';
2323

24-
import { makeKeyString } from '../objects/object-utils.js';
24+
import { makeKeyString, parseKeyString } from '../objects/object-utils.js';
2525
import CompositionProvider from './CompositionProvider.js';
2626

2727
/**
@@ -75,7 +75,11 @@ export default class DefaultCompositionProvider extends CompositionProvider {
7575
* the Identifiers in this composition
7676
*/
7777
load(domainObject) {
78-
return Promise.all(domainObject.composition);
78+
const identifiers = domainObject.composition
79+
.filter((idOrKeystring) => idOrKeystring !== null && idOrKeystring !== undefined)
80+
.map((idOrKeystring) => parseKeyString(idOrKeystring));
81+
82+
return Promise.all(identifiers);
7983
}
8084
/**
8185
* Attach listeners for changes to the composition of a given domain object.

src/api/objects/ObjectAPI.js

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import ConflictError from './ConflictError.js';
2727
import InMemorySearchProvider from './InMemorySearchProvider.js';
2828
import InterceptorRegistry from './InterceptorRegistry.js';
2929
import MutableDomainObject from './MutableDomainObject.js';
30+
import { isIdentifier, isKeyString } from './object-utils.js';
3031
import RootObjectProvider from './RootObjectProvider.js';
3132
import RootRegistry from './RootRegistry.js';
3233
import Transaction from './Transaction.js';
@@ -742,11 +743,19 @@ export default class ObjectAPI {
742743
* @param {AbortSignal} abortSignal (optional) signal to abort fetch requests
743744
* @returns {Promise<Array<DomainObject>>} a promise containing an array of domain objects
744745
*/
745-
async getOriginalPath(identifier, path = [], abortSignal = null) {
746-
const domainObject = await this.get(identifier, abortSignal);
746+
async getOriginalPath(identifierOrObject, path = [], abortSignal = null) {
747+
let domainObject;
748+
749+
if (isKeyString(identifierOrObject) || isIdentifier(identifierOrObject)) {
750+
domainObject = await this.get(identifierOrObject, abortSignal);
751+
} else {
752+
domainObject = identifierOrObject;
753+
}
754+
747755
if (!domainObject) {
748756
return [];
749757
}
758+
750759
path.push(domainObject);
751760
const { location } = domainObject;
752761
if (location && !this.#pathContainsDomainObject(location, path)) {

src/plugins/CouchDBSearchFolder/plugin.js

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ export default function (folderName, couchPlugin, searchFilter) {
2424
location: 'ROOT'
2525
});
2626
}
27+
},
28+
search() {
29+
return Promise.resolve([]);
2730
}
2831
});
2932

@@ -35,9 +38,17 @@ export default function (folderName, couchPlugin, searchFilter) {
3538
);
3639
},
3740
load() {
38-
return couchProvider.getObjectsByFilter(searchFilter).then((objects) => {
39-
return objects.map((object) => object.identifier);
40-
});
41+
let searchResults;
42+
43+
if (searchFilter.viewName !== undefined) {
44+
// Use a view to search, instead of an _all_docs find
45+
searchResults = couchProvider.getObjectsByView(searchFilter);
46+
} else {
47+
// Use the _find endpoint to search _all_docs
48+
searchResults = couchProvider.getObjectsByFilter(searchFilter);
49+
}
50+
51+
return searchResults;
4152
}
4253
});
4354
};

0 commit comments

Comments
 (0)