Skip to content

Commit fa18bff

Browse files
authored
feat: 🎸 Refactor session recording filters (#2981)
This commit has a few notable changes: 1. A simple attempt at establishing a pattern for using SELECT statements. This will be useful in the future for joins/subquery support. 2. Expanded usage of searching to be able to pass instead of just a string, an object that lets a consumer choose which columns to search on. 3. Decouple our session recording filters from being loaded from the route model hook which is a departure from our convention. This has the benefit of actually letting the page first load and having the filters load async so the page doesn't wait for the filters to finish. This is also much faster to query for each individual filter option from SQLite rather than returning all data to filter in application code.
1 parent 72f7c8a commit fa18bff

File tree

18 files changed

+537
-229
lines changed

18 files changed

+537
-229
lines changed

‎addons/api/addon/handlers/sqlite-handler.js‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,13 @@ export default class SqliteHandler {
4949
const schema = store.modelFor(type);
5050
const serializer = store.serializerFor(type);
5151

52-
let { page, pageSize, query: queryObj, ...remainingQuery } = query;
52+
let {
53+
page,
54+
pageSize,
55+
select,
56+
query: queryObj,
57+
...remainingQuery
58+
} = query;
5359
let payload,
5460
listToken,
5561
writeToDbPromise,
@@ -119,7 +125,7 @@ export default class SqliteHandler {
119125
const { sql, parameters } = generateSQLExpressions(type, queryObj, {
120126
page,
121127
pageSize,
122-
select: ['data'],
128+
select: select ?? [{ field: 'data' }],
123129
});
124130

125131
const rows = await this.sqlite.fetchResource({
@@ -129,7 +135,7 @@ export default class SqliteHandler {
129135

130136
const { sql: countSql, parameters: countParams } =
131137
generateSQLExpressions(type, queryObj, {
132-
select: ['count(*) as total'],
138+
select: [{ field: '*', isCount: true, alias: 'total' }],
133139
});
134140
const count = await this.sqlite.fetchResource({
135141
sql: countSql,
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/**
2+
* Copyright (c) HashiCorp, Inc.
3+
* SPDX-License-Identifier: BUSL-1.1
4+
*/
5+
6+
import Service, { service } from '@ember/service';
7+
import { modelMapping } from 'api/services/sqlite';
8+
import { assert } from '@ember/debug';
9+
import { generateSQLExpressions } from '../utils/sqlite-query';
10+
11+
/**
12+
* Database service for querying Boundary resources from storage.
13+
* Provides a high-level interface for executing queries against the local database.
14+
*/
15+
export default class DbService extends Service {
16+
@service sqlite;
17+
18+
/**
19+
* Query resources from the database by type.
20+
*
21+
* @param {string} type - The resource type (e.g., 'scope', 'user', 'target').
22+
* Must be a supported model type defined in modelMapping.
23+
* @param {Object} query - Query configuration object
24+
* @returns {Promise<Object>} Promise resolving to query results from sqlite service
25+
* @throws {Error} Assertion error if resource type is not supported
26+
*/
27+
query(type, query) {
28+
const supportedModels = Object.keys(modelMapping);
29+
assert('Resource type is not supported.', supportedModels.includes(type));
30+
31+
let { page, pageSize, select, query: queryObj } = query;
32+
33+
const { sql, parameters } = generateSQLExpressions(type, queryObj, {
34+
page,
35+
pageSize,
36+
select,
37+
});
38+
39+
return this.sqlite.fetchResource({
40+
sql,
41+
parameters,
42+
});
43+
}
44+
}

‎addons/api/addon/services/sqlite.js‎

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,8 @@ export const modelMapping = {
100100
target_name: 'create_time_values.target.name',
101101
target_scope_id: 'create_time_values.target.scope.id',
102102
target_scope_name: 'create_time_values.target.scope.name',
103+
target_scope_parent_scope_id:
104+
'create_time_values.target.scope.parent_scope_id',
103105
created_time: 'created_time',
104106
},
105107
session: {
@@ -114,21 +116,6 @@ export const modelMapping = {
114116
},
115117
};
116118

117-
// A list of tables that we support searching using FTS5 in SQLite.
118-
export const searchTables = new Set([
119-
'target',
120-
'alias',
121-
'group',
122-
'role',
123-
'user',
124-
'credential-store',
125-
'scope',
126-
'auth-method',
127-
'host-catalog',
128-
'session-recording',
129-
'session',
130-
]);
131-
132119
export default class SqliteDbService extends Service {
133120
// =attributes
134121

‎addons/api/addon/utils/sqlite-query.js‎

Lines changed: 72 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@
44
*/
55

66
import { modelMapping } from 'api/services/sqlite';
7-
import { searchTables } from 'api/services/sqlite';
87
import { typeOf } from '@ember/utils';
98
import { underscore } from '@ember/string';
9+
import { assert } from '@ember/debug';
1010

1111
/**
1212
* Takes a POJO representing a filter query and builds a SQL query.
@@ -35,17 +35,15 @@ export function generateSQLExpressions(
3535
addFilterConditions({ filters, parameters, conditions });
3636
addSearchConditions({ search, resource, tableName, parameters, conditions });
3737

38+
const selectClause = constructSelectClause(select, tableName);
3839
const orderByClause = constructOrderByClause(resource, sort);
39-
4040
const whereClause = conditions.length ? `WHERE ${and(conditions)}` : '';
4141

4242
const paginationClause = page && pageSize ? `LIMIT ? OFFSET ?` : '';
4343
if (paginationClause) {
4444
parameters.push(pageSize, (page - 1) * pageSize);
4545
}
4646

47-
const selectClause = `SELECT ${select ? select.join(', ') : '*'} FROM "${tableName}"`;
48-
4947
return {
5048
// Replace any empty newlines or leading whitespace on each line to be consistent with formatting
5149
// This is mainly to help us read and test the generated SQL as it has no effect on the actual SQL execution.
@@ -142,32 +140,83 @@ function addSearchConditions({
142140
parameters,
143141
conditions,
144142
}) {
145-
if (!search) {
143+
if (!search || !modelMapping[resource]) {
146144
return;
147145
}
148146

149-
if (searchTables.has(resource)) {
150-
// Use the special prefix indicator "*" for full-text search
151-
parameters.push(`"${search}"*`);
152-
// Use a subquery to match against the FTS table with rowids as SQLite is
153-
// much more efficient with FTS queries when using rowids or MATCH (or both).
154-
// We could have also used a join here but a subquery is simpler.
155-
conditions.push(
156-
`rowid IN (SELECT rowid FROM ${tableName}_fts WHERE ${tableName}_fts MATCH ?)`,
147+
// Use the special prefix indicator "*" for full-text search
148+
if (typeOf(search) === 'object') {
149+
if (!search?.text) {
150+
return;
151+
}
152+
153+
parameters.push(
154+
or(search.fields.map((field) => `${field}:"${search.text}"*`)),
157155
);
158-
return;
156+
} else {
157+
parameters.push(`"${search}"*`);
159158
}
160159

161-
const fields = Object.keys(modelMapping[resource]);
162-
const searchConditions = parenthetical(
163-
or(
164-
fields.map((field) => {
165-
parameters.push(`%${search}%`);
166-
return `${field}${OPERATORS['contains']}`;
167-
}),
168-
),
160+
// Use a subquery to match against the FTS table with rowids as SQLite is
161+
// much more efficient with FTS queries when using rowids or MATCH (or both).
162+
// We could have also used a join here but a subquery is simpler.
163+
conditions.push(
164+
`rowid IN (SELECT rowid FROM ${tableName}_fts WHERE ${tableName}_fts MATCH ?)`,
169165
);
170-
conditions.push(searchConditions);
166+
}
167+
168+
function constructSelectClause(select = [{ field: '*' }], tableName) {
169+
const distinctColumns = select.filter(({ isDistinct }) => isDistinct);
170+
const nonDistinctColumns = select.filter(({ isDistinct }) => !isDistinct);
171+
172+
const buildColumnExpression = ({ field, isCount, alias, isDistinct }) => {
173+
let column = field;
174+
175+
// If there's just distinct with nothing else, this will be handled separately
176+
// and we will just return the column back as is
177+
if (isCount && isDistinct) {
178+
column = `count(DISTINCT ${column})`;
179+
} else if (isCount) {
180+
column = `count(${column})`;
181+
}
182+
183+
if (alias) {
184+
column = `${column} as ${alias}`;
185+
}
186+
187+
return column;
188+
};
189+
190+
let selectColumns;
191+
192+
if (distinctColumns.length > 0) {
193+
// Check if any columns also have COUNT (or other aggregate function in the future)
194+
const hasAggregateDistinct = distinctColumns.some((col) => col.isCount);
195+
196+
if (hasAggregateDistinct) {
197+
selectColumns = distinctColumns.map(buildColumnExpression).join(', ');
198+
199+
// Add back in the non distinct columns
200+
if (nonDistinctColumns.length > 0) {
201+
const regularPart = nonDistinctColumns
202+
.map(buildColumnExpression)
203+
.join(', ');
204+
selectColumns = `${selectColumns}, ${regularPart}`;
205+
}
206+
} else {
207+
assert(
208+
'Can not combine non-distincts with multi column distincts',
209+
nonDistinctColumns.length === 0,
210+
);
211+
212+
// Only do multi column DISTINCT with no aggregate functions
213+
selectColumns = `DISTINCT ${distinctColumns.map(buildColumnExpression).join(', ')}`;
214+
}
215+
} else {
216+
selectColumns = select.map(buildColumnExpression).join(', ');
217+
}
218+
219+
return `SELECT ${selectColumns} FROM "${tableName}"`;
171220
}
172221

173222
function constructOrderByClause(resource, sort) {

‎addons/api/addon/workers/utils/schema.js‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,7 @@ CREATE TABLE IF NOT EXISTS session_recording (
352352
target_name TEXT,
353353
target_scope_id TEXT,
354354
target_scope_name TEXT,
355+
target_scope_parent_scope_id TEXT,
355356
created_time TEXT NOT NULL,
356357
data TEXT NOT NULL
357358
);
@@ -371,21 +372,22 @@ CREATE VIRTUAL TABLE IF NOT EXISTS session_recording_fts USING fts5(
371372
target_name,
372373
target_scope_id,
373374
target_scope_name,
375+
target_scope_parent_scope_id,
374376
created_time,
375377
content='',
376378
);
377379
378380
CREATE TRIGGER IF NOT EXISTS session_recording_ai AFTER INSERT ON session_recording BEGIN
379381
INSERT INTO session_recording_fts(
380-
rowid, id, type, state, start_time, end_time, duration, scope_id, user_id, user_name, target_id, target_name, target_scope_id, target_scope_name, created_time
382+
rowid, id, type, state, start_time, end_time, duration, scope_id, user_id, user_name, target_id, target_name, target_scope_id, target_scope_name, target_scope_parent_scope_id, created_time
381383
) VALUES (
382-
new.rowid, new.id, new.type, new.state, new.start_time, new.end_time, new.duration, new.scope_id, new.user_id, new.user_name, new.target_id, new.target_name, new.target_scope_id, new.target_scope_name, new.created_time
384+
new.rowid, new.id, new.type, new.state, new.start_time, new.end_time, new.duration, new.scope_id, new.user_id, new.user_name, new.target_id, new.target_name, new.target_scope_id, new.target_scope_name, new.target_scope_parent_scope_id, new.created_time
383385
);
384386
END;
385387
386388
CREATE TRIGGER IF NOT EXISTS session_recording_ad AFTER DELETE ON session_recording BEGIN
387-
INSERT INTO session_recording_fts(session_recording_fts, rowid, id, type, state, start_time, end_time, duration, scope_id, user_id, user_name, target_id, target_name, target_scope_id, target_scope_name, created_time)
388-
VALUES('delete', old.rowid, old.id, old.type, old.state, old.start_time, old.end_time, old.duration, old.scope_id, old.user_id, old.user_name, old.target_id, old.target_name, old.target_scope_id, old.target_scope_name, old.created_time);
389+
INSERT INTO session_recording_fts(session_recording_fts, rowid, id, type, state, start_time, end_time, duration, scope_id, user_id, user_name, target_id, target_name, target_scope_id, target_scope_name, target_scope_parent_scope_id, created_time)
390+
VALUES('delete', old.rowid, old.id, old.type, old.state, old.start_time, old.end_time, old.duration, old.scope_id, old.user_id, old.user_name, old.target_id, old.target_name, old.target_scope_id, old.target_scope_name, old.target_scope_parent_scope_id, old.created_time);
389391
END;`;
390392

391393
const createSessionTables = `
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
/**
2+
* Copyright (c) HashiCorp, Inc.
3+
* SPDX-License-Identifier: BUSL-1.1
4+
*/
5+
6+
export { default } from 'api/services/db';
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/**
2+
* Copyright (c) HashiCorp, Inc.
3+
* SPDX-License-Identifier: BUSL-1.1
4+
*/
5+
6+
import { module, test } from 'qunit';
7+
import { setupTest } from 'dummy/tests/helpers';
8+
9+
module('Unit | Service | db', function (hooks) {
10+
setupTest(hooks);
11+
12+
test('query throws assertion error for unsupported resource type', function (assert) {
13+
let service = this.owner.lookup('service:db');
14+
15+
assert.throws(() => {
16+
service.query('unsupported-type', { query: {} });
17+
}, /Resource type is not supported/);
18+
});
19+
});

‎addons/api/tests/unit/services/sqlite-test.js‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,10 @@
66
import { module, test } from 'qunit';
77
import { setupTest } from 'dummy/tests/helpers';
88
import { setupSqlite } from 'api/test-support/helpers/sqlite';
9-
import { modelMapping, searchTables } from 'api/services/sqlite';
9+
import { modelMapping } from 'api/services/sqlite';
1010
import { underscore } from '@ember/string';
1111

1212
const supportedModels = Object.keys(modelMapping);
13-
const supportedFtsTables = [...searchTables];
1413

1514
module('Unit | Service | sqlite', function (hooks) {
1615
setupTest(hooks);
@@ -38,7 +37,7 @@ module('Unit | Service | sqlite', function (hooks) {
3837

3938
test.each(
4039
'Mapping matches fts table columns',
41-
supportedFtsTables,
40+
supportedModels,
4241
async function (assert, resource) {
4342
const service = this.owner.lookup('service:sqlite');
4443

0 commit comments

Comments
 (0)