From fda1f56ad1015461dc252abd13d49d6bace86db4 Mon Sep 17 00:00:00 2001 From: Zhihe Li Date: Mon, 3 Nov 2025 17:28:11 -0500 Subject: [PATCH] =?UTF-8?q?feat:=20=F0=9F=8E=B8=20Add=20support=20for=20jo?= =?UTF-8?q?ins=20with=20filters=20(#3010)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit adds support for being able to join to other tables when filtering. Support was also added for being able to find null values. --- addons/api/addon/utils/sqlite-query.js | 116 ++++++-- .../api/tests/unit/utils/sqlite-query-test.js | 270 ++++++++++++------ .../app/routes/scopes/scope/targets/index.js | 94 +++--- 3 files changed, 348 insertions(+), 132 deletions(-) diff --git a/addons/api/addon/utils/sqlite-query.js b/addons/api/addon/utils/sqlite-query.js index 4ee231f6d..4b46a6a28 100644 --- a/addons/api/addon/utils/sqlite-query.js +++ b/addons/api/addon/utils/sqlite-query.js @@ -31,12 +31,14 @@ export function generateSQLExpressions( const conditions = []; const parameters = []; const tableName = underscore(resource); + const joins = []; - addFilterConditions({ filters, parameters, conditions }); + addFilterConditions({ filters, parameters, conditions, joins, tableName }); addSearchConditions({ search, resource, tableName, parameters, conditions }); const selectClause = constructSelectClause(select, tableName); - const orderByClause = constructOrderByClause(resource, sort); + const joinClause = constructJoinClause(joins); + const orderByClause = constructOrderByClause(resource, tableName, sort); const whereClause = conditions.length ? `WHERE ${and(conditions)}` : ''; const paginationClause = page && pageSize ? `LIMIT ? OFFSET ?` : ''; @@ -49,6 +51,7 @@ export function generateSQLExpressions( // This is mainly to help us read and test the generated SQL as it has no effect on the actual SQL execution. sql: ` ${selectClause} + ${joinClause} ${whereClause} ${orderByClause} ${paginationClause}` @@ -58,7 +61,13 @@ export function generateSQLExpressions( }; } -function addFilterConditions({ filters, parameters, conditions }) { +function addFilterConditions({ + filters, + parameters, + conditions, + joins, + tableName, +}) { if (!filters) { return; } @@ -69,7 +78,7 @@ function addFilterConditions({ filters, parameters, conditions }) { ? filterArrayOrObject : filterArrayOrObject.values; - if (!filterValueArray || !filterValueArray.length) { + if (!filterValueArray || !filterValueArray.length || key === 'joins') { continue; } @@ -86,8 +95,13 @@ function addFilterConditions({ filters, parameters, conditions }) { allOperatorsEqual ) { const operation = firstOperator === 'equals' ? 'in' : 'notIn'; + const nullOperator = + firstOperator === 'equals' ? 'IS NULL' : 'IS NOT NULL'; const values = filterValueArray - .filter((f) => f) + .filter( + (filterObjValue) => + filterObjValue && Object.values(filterObjValue)[0] !== null, + ) .map((filterObjValue) => { let value = Object.values(filterObjValue)[0]; if (typeOf(value) === 'date') { @@ -96,7 +110,14 @@ function addFilterConditions({ filters, parameters, conditions }) { parameters.push(value); return value; }); - const filterCondition = `${key}${OPERATORS[operation](values)}`; + + // Add a check for null values as an IN clause will mistakenly use `=` for null values. + // We'll add an extra OR clause to handle nulls separately. + const isNullValue = filterValueArray.some( + (filterObjValue) => Object.values(filterObjValue)[0] === null, + ); + + const filterCondition = `"${tableName}".${key}${OPERATORS[operation](values)}${isNullValue ? ` OR "${tableName}".${key} ${nullOperator}` : ''}`; conditions.push(parenthetical(filterCondition)); continue; } @@ -106,6 +127,16 @@ function addFilterConditions({ filters, parameters, conditions }) { .map((filterObjValue) => { let [operation, value] = Object.entries(filterObjValue)[0]; + // Handle null values: convert equals/notEquals to IS NULL or IS NOT NULL + if (value === null) { + if (operation === 'equals') { + return `"${tableName}".${key} IS NULL`; + } + if (operation === 'notEquals') { + return `"${tableName}".${key} IS NOT NULL`; + } + } + // SQLite needs to be working with ISO strings if (typeOf(value) === 'date') { value = value.toISOString(); @@ -118,7 +149,7 @@ function addFilterConditions({ filters, parameters, conditions }) { parameters.push(value); } - return `${key}${OPERATORS[operation]}`; + return `"${tableName}".${key}${OPERATORS[operation]}`; }); const { logicalOperator } = filterArrayOrObject; @@ -131,6 +162,43 @@ function addFilterConditions({ filters, parameters, conditions }) { ), ); } + + if (filters.joins?.length > 0) { + const tableNameIndex = {}; + + filters.joins.forEach((join) => { + const { + resource, + query, + joinFrom = 'id', + joinOn, + joinType = 'INNER', + } = join; + const joinTableName = underscore(resource); + tableNameIndex[resource] ??= 1; + // Alias in the possible scenario we join the same table more than once + const alias = `${joinTableName}${tableNameIndex[resource]}`; + + joins.push({ + type: joinType, + table: joinTableName, + alias, + condition: `"${tableName}".${joinFrom} = ${alias}.${joinOn}`, + }); + + // Add the conditions from the join query + if (query?.filters) { + addFilterConditions({ + filters: query.filters, + parameters, + conditions, + tableName: alias, + }); + } + + tableNameIndex[resource]++; + }); + } } function addSearchConditions({ @@ -161,7 +229,7 @@ function addSearchConditions({ // much more efficient with FTS queries when using rowids or MATCH (or both). // We could have also used a join here but a subquery is simpler. conditions.push( - `rowid IN (SELECT rowid FROM ${tableName}_fts WHERE ${tableName}_fts MATCH ?)`, + `"${tableName}".rowid IN (SELECT rowid FROM ${tableName}_fts WHERE ${tableName}_fts MATCH ?)`, ); } @@ -170,7 +238,7 @@ function constructSelectClause(select = [{ field: '*' }], tableName) { const nonDistinctColumns = select.filter(({ isDistinct }) => !isDistinct); const buildColumnExpression = ({ field, isCount, alias, isDistinct }) => { - let column = field; + let column = field === '*' ? field : `"${tableName}".${field}`; // If there's just distinct with nothing else, this will be handled separately // and we will just return the column back as is @@ -219,11 +287,27 @@ function constructSelectClause(select = [{ field: '*' }], tableName) { return `SELECT ${selectColumns} FROM "${tableName}"`; } -function constructOrderByClause(resource, sort) { - const defaultOrderByClause = 'ORDER BY created_time DESC'; +function constructJoinClause(joins) { + if (!joins || joins.length === 0) { + return ''; + } + + return joins + .map( + ({ type, table, alias, condition }) => + `${type} JOIN "${table}" ${alias} ON ${condition}`, + ) + .join(' '); +} + +function constructOrderByClause(resource, tableName, sort) { + const defaultOrderByClause = `ORDER BY "${tableName}".created_time DESC`; const { attributes, customSort, direction, isCoalesced } = sort; const sortDirection = direction === 'desc' ? 'DESC' : 'ASC'; + const attributesWithTableName = attributes?.map( + (attribute) => `"${tableName}".${attribute}`, + ); // We have to check if the attributes are valid for the resource // as we can't use parameterized queries for ORDER BY @@ -239,21 +323,21 @@ function constructOrderByClause(resource, sort) { }, '', ); - return `ORDER BY CASE ${attributes.join(', ')} ${whenClauses}END ${sortDirection}`; + return `ORDER BY CASE ${attributesWithTableName.join(', ')} ${whenClauses}END ${sortDirection}`; } else if (attributes?.length > 0) { - const commaSeparatedVals = attributes.join(', '); + const commaSeparatedVals = attributesWithTableName.join(', '); // In places where `collate nocase` is used, it is to ensure case is ignored on the initial sort. // Then, a sort on the same condition is performed to ensure upper-case strings are given preference in a tie. if (isCoalesced) { - return `ORDER BY COALESCE(${attributes.join(', ')}) COLLATE NOCASE ${sortDirection}, COALESCE(${commaSeparatedVals}) ${sortDirection}`; + return `ORDER BY COALESCE(${commaSeparatedVals}) COLLATE NOCASE ${sortDirection}, COALESCE(${commaSeparatedVals}) ${sortDirection}`; } const attributesWithNoCollate = attributes - .map((attr) => `${attr} COLLATE NOCASE ${sortDirection}`) + .map((attr) => `"${tableName}".${attr} COLLATE NOCASE ${sortDirection}`) .join(', '); const attributesWithDirection = attributes - .map((attr) => `${attr} ${sortDirection}`) + .map((attr) => `"${tableName}".${attr} ${sortDirection}`) .join(', '); return `ORDER BY ${attributesWithNoCollate}, ${attributesWithDirection}`; } else if (modelMapping[resource]?.created_time) { diff --git a/addons/api/tests/unit/utils/sqlite-query-test.js b/addons/api/tests/unit/utils/sqlite-query-test.js index a7e134b61..27d2685d1 100644 --- a/addons/api/tests/unit/utils/sqlite-query-test.js +++ b/addons/api/tests/unit/utils/sqlite-query-test.js @@ -30,7 +30,7 @@ module('Unit | Utility | sqlite-query', function (hooks) { sql, ` SELECT * FROM "token" - WHERE (id = ?)`.removeExtraWhiteSpace(), + WHERE ("token".id = ?)`.removeExtraWhiteSpace(), ); assert.deepEqual(parameters, ['tokenKey']); }); @@ -45,7 +45,7 @@ module('Unit | Utility | sqlite-query', function (hooks) { sql, ` SELECT count(*) as total FROM "target" - ORDER BY created_time DESC`.removeExtraWhiteSpace(), + ORDER BY "target".created_time DESC`.removeExtraWhiteSpace(), ); assert.deepEqual(parameters, []); }); @@ -55,26 +55,26 @@ module('Unit | Utility | sqlite-query', function (hooks) { { 'single field': { select: [{ field: 'name' }], - expectedSelect: 'name', + expectedSelect: '"target".name', }, 'multiple fields': { select: [{ field: 'name' }, { field: 'type' }, { field: 'id' }], - expectedSelect: 'name, type, id', + expectedSelect: '"target".name, "target".type, "target".id', }, 'field with alias': { select: [{ field: 'name', alias: 'resource_name' }], - expectedSelect: 'name as resource_name', + expectedSelect: '"target".name as resource_name', }, 'multiple fields with aliases': { select: [ { field: 'name', alias: 'resource_name' }, { field: 'type', alias: 'resource_type' }, ], - expectedSelect: 'name as resource_name, type as resource_type', + expectedSelect: '"target".name as resource_name, "target".type as resource_type', }, 'count with alias': { select: [{ field: 'id', isCount: true, alias: 'total_count' }], - expectedSelect: 'count(id) as total_count', + expectedSelect: 'count("target".id) as total_count', }, 'count without alias': { select: [{ field: '*', isCount: true }], @@ -82,7 +82,7 @@ module('Unit | Utility | sqlite-query', function (hooks) { }, 'count distinct': { select: [{ field: 'status', isCount: true, isDistinct: true }], - expectedSelect: 'count(DISTINCT status)', + expectedSelect: 'count(DISTINCT "target".status)', }, 'count distinct with alias': { select: [ @@ -93,43 +93,43 @@ module('Unit | Utility | sqlite-query', function (hooks) { alias: 'unique_types', }, ], - expectedSelect: 'count(DISTINCT type) as unique_types', + expectedSelect: 'count(DISTINCT "target".type) as unique_types', }, 'mixed regular and count fields': { select: [ { field: 'name' }, { field: 'id', isCount: true, alias: 'count' }, ], - expectedSelect: 'name, count(id) as count', + expectedSelect: '"target".name, count("target".id) as count', }, 'field with distinct': { select: [{ field: 'type', isDistinct: true }], - expectedSelect: 'DISTINCT type', + expectedSelect: 'DISTINCT "target".type', }, 'multiple distinct fields': { select: [ { field: 'type', isDistinct: true }, { field: 'status', isDistinct: true }, ], - expectedSelect: 'DISTINCT type, status', + expectedSelect: 'DISTINCT "target".type, "target".status', }, 'multiple distinct with count fields': { select: [ { field: 'type', isDistinct: true, isCount: true }, { field: 'status', isDistinct: true, isCount: true }, ], - expectedSelect: 'count(DISTINCT type), count(DISTINCT status)', + expectedSelect: 'count(DISTINCT "target".type), count(DISTINCT "target".status)', }, 'distinct with alias': { select: [{ field: 'type', isDistinct: true, alias: 'unique_type' }], - expectedSelect: 'DISTINCT type as unique_type', + expectedSelect: 'DISTINCT "target".type as unique_type', }, 'multiple distinct fields with aliases': { select: [ { field: 'type', isDistinct: true, alias: 'unique_type' }, { field: 'status', isDistinct: true, alias: 'unique_status' }, ], - expectedSelect: 'DISTINCT type as unique_type, status as unique_status', + expectedSelect: 'DISTINCT "target".type as unique_type, "target".status as unique_status', }, 'count distinct multiple fields': { select: [ @@ -147,7 +147,7 @@ module('Unit | Utility | sqlite-query', function (hooks) { }, ], expectedSelect: - 'count(DISTINCT type) as type_count, count(DISTINCT status) as status_count', + 'count(DISTINCT "target".type) as type_count, count(DISTINCT "target".status) as status_count', }, }, function (assert, { select, expectedSelect }) { @@ -161,7 +161,7 @@ module('Unit | Utility | sqlite-query', function (hooks) { sql, ` SELECT ${expectedSelect} FROM "target" - ORDER BY created_time DESC`.removeExtraWhiteSpace(), + ORDER BY "target".created_time DESC`.removeExtraWhiteSpace(), ); assert.deepEqual(parameters, []); }, @@ -176,7 +176,7 @@ module('Unit | Utility | sqlite-query', function (hooks) { type: [{ equals: 'ssh' }], }, }, - expectedWhereClause: 'WHERE (type = ?)', + expectedWhereClause: 'WHERE ("target".type = ?)', expectedParams: ['ssh'], }, notEquals: { @@ -185,7 +185,7 @@ module('Unit | Utility | sqlite-query', function (hooks) { type: [{ notEquals: 'ssh' }], }, }, - expectedWhereClause: 'WHERE (type != ?)', + expectedWhereClause: 'WHERE ("target".type != ?)', expectedParams: ['ssh'], }, contains: { @@ -194,7 +194,7 @@ module('Unit | Utility | sqlite-query', function (hooks) { type: [{ contains: 'ssh' }], }, }, - expectedWhereClause: 'WHERE (type LIKE ?)', + expectedWhereClause: 'WHERE ("target".type LIKE ?)', expectedParams: ['%ssh%'], }, greaterThan: { @@ -204,7 +204,8 @@ module('Unit | Utility | sqlite-query', function (hooks) { numberField: [{ gt: 10 }], }, }, - expectedWhereClause: 'WHERE (created_time > ?) AND (numberField > ?)', + expectedWhereClause: + 'WHERE ("target".created_time > ?) AND ("target".numberField > ?)', expectedParams: [isoDateString, 10], }, lessThan: { @@ -214,7 +215,8 @@ module('Unit | Utility | sqlite-query', function (hooks) { numberField: [{ lt: 10 }], }, }, - expectedWhereClause: 'WHERE (created_time < ?) AND (numberField < ?)', + expectedWhereClause: + 'WHERE ("target".created_time < ?) AND ("target".numberField < ?)', expectedParams: [isoDateString, 10], }, greaterThanOrEqual: { @@ -224,7 +226,8 @@ module('Unit | Utility | sqlite-query', function (hooks) { numberField: [{ gte: 10 }], }, }, - expectedWhereClause: 'WHERE (created_time >= ?) AND (numberField >= ?)', + expectedWhereClause: + 'WHERE ("target".created_time >= ?) AND ("target".numberField >= ?)', expectedParams: [isoDateString, 10], }, lessThanOrEqual: { @@ -234,7 +237,8 @@ module('Unit | Utility | sqlite-query', function (hooks) { numberField: [{ lte: 10 }], }, }, - expectedWhereClause: 'WHERE (created_time <= ?) AND (numberField <= ?)', + expectedWhereClause: + 'WHERE ("target".created_time <= ?) AND ("target".numberField <= ?)', expectedParams: [isoDateString, 10], }, logicalOperators: { @@ -252,9 +256,42 @@ module('Unit | Utility | sqlite-query', function (hooks) { }, }, expectedWhereClause: - 'WHERE (id NOT IN (?, ?)) AND (status IN (?, ?)) AND (type = ?)', + 'WHERE ("target".id NOT IN (?, ?)) AND ("target".status IN (?, ?)) AND ("target".type = ?)', expectedParams: ['id1', 'id2', 'active', 'pending', 'ssh'], }, + equalsNull: { + query: { + filters: { + description: [{ equals: null }], + }, + }, + expectedWhereClause: 'WHERE ("target".description IS NULL)', + expectedParams: [], + }, + notEqualsNull: { + query: { + filters: { + description: [{ notEquals: null }], + }, + }, + expectedWhereClause: 'WHERE ("target".description IS NOT NULL)', + expectedParams: [], + }, + mixedNullAndValues: { + query: { + filters: { + status: [ + { equals: 'active' }, + { equals: 'pending' }, + { equals: null }, + ], + type: [{ notEquals: null }, { notEquals: 'ssh' }], + }, + }, + expectedWhereClause: + 'WHERE ("target".status IN (?, ?) OR "target".status IS NULL) AND ("target".type NOT IN (?) OR "target".type IS NOT NULL)', + expectedParams: ['active', 'pending', 'ssh'], + }, }, function (assert, { query, expectedWhereClause, expectedParams }) { @@ -264,7 +301,7 @@ module('Unit | Utility | sqlite-query', function (hooks) { ` SELECT * FROM "target" ${expectedWhereClause} - ORDER BY created_time DESC`.removeExtraWhiteSpace(), + ORDER BY "target".created_time DESC`.removeExtraWhiteSpace(), ); assert.deepEqual(parameters, expectedParams); }, @@ -286,7 +323,7 @@ module('Unit | Utility | sqlite-query', function (hooks) { sql, ` SELECT * FROM "target" - ORDER BY name COLLATE NOCASE ${expectedDirection}, name ${expectedDirection}`.removeExtraWhiteSpace(), + ORDER BY "target".name COLLATE NOCASE ${expectedDirection}, "target".name ${expectedDirection}`.removeExtraWhiteSpace(), ); assert.deepEqual(parameters, []); }, @@ -300,20 +337,20 @@ module('Unit | Utility | sqlite-query', function (hooks) { attributes: ['name', 'id'], isCoalesced: true, }, - expectedOrderByClause: `ORDER BY COALESCE(name, id) COLLATE NOCASE DESC, COALESCE(name, id) DESC`, + expectedOrderByClause: `ORDER BY COALESCE("target".name, "target".id) COLLATE NOCASE DESC, COALESCE("target".name, "target".id) DESC`, }, 'sort on multiple attributes': { sort: { attributes: ['name', 'id'], }, - expectedOrderByClause: `ORDER BY name COLLATE NOCASE DESC, id COLLATE NOCASE DESC, name DESC, id DESC`, + expectedOrderByClause: `ORDER BY "target".name COLLATE NOCASE DESC, "target".id COLLATE NOCASE DESC, "target".name DESC, "target".id DESC`, }, 'sort on mapped attributes': { sort: { attributes: ['type'], customSort: { attributeMap: { ssh: 'SSH', tcp: 'Generic TCP' } }, }, - expectedOrderByClause: `ORDER BY CASE type WHEN 'ssh' THEN 'SSH' WHEN 'tcp' THEN 'Generic TCP' END DESC`, + expectedOrderByClause: `ORDER BY CASE "target".type WHEN 'ssh' THEN 'SSH' WHEN 'tcp' THEN 'Generic TCP' END DESC`, }, }, function (assert, { sort, expectedOrderByClause }) { @@ -349,66 +386,131 @@ module('Unit | Utility | sqlite-query', function (hooks) { sql, ` SELECT * FROM "target" - ORDER BY created_time DESC + ORDER BY "target".created_time DESC LIMIT ? OFFSET ?`.removeExtraWhiteSpace(), ); assert.deepEqual(parameters, expectedParams); }, ); - test('it generates FTS5 search with filters', function (assert) { - const query = { - search: 'favorite', - filters: { - id: { - logicalOperator: 'and', - values: [{ notEquals: 'id1' }, { notEquals: 'id2' }], + test.each( + 'it generates FTS5 search correctly', + { + 'string search with filters': { + query: { + search: 'favorite', }, - status: [{ equals: 'active' }, { equals: 'pending' }], + expectedSql: ` + SELECT * FROM "target" + WHERE "target".rowid IN (SELECT rowid FROM target_fts WHERE target_fts MATCH ?) + ORDER BY "target".created_time DESC`, + expectedParams: ['"favorite"*'], }, - }; - - const { sql, parameters } = generateSQLExpressions('target', query); - assert.strictEqual( - sql, - ` - SELECT * FROM "target" - WHERE (id NOT IN (?, ?)) AND (status IN (?, ?)) AND rowid IN (SELECT rowid FROM target_fts WHERE target_fts MATCH ?) - ORDER BY created_time DESC`.removeExtraWhiteSpace(), - ); - assert.deepEqual(parameters, [ - 'id1', - 'id2', - 'active', - 'pending', - '"favorite"*', - ]); - }); - - test('it generates FTS5 search with object parameter', function (assert) { - const query = { - search: { - text: 'favorite', - fields: ['name', 'description'], + 'object search with field-specific searches': { + query: { + search: { + text: 'favorite', + fields: ['name', 'description'], + }, + }, + expectedSql: ` + SELECT * FROM "target" + WHERE "target".rowid IN (SELECT rowid FROM target_fts WHERE target_fts MATCH ?) + ORDER BY "target".created_time DESC`, + expectedParams: ['name:"favorite"* OR description:"favorite"*'], }, - filters: { - type: [{ equals: 'ssh' }], - }, - }; + }, + function (assert, { query, expectedSql, expectedParams }) { + const { sql, parameters } = generateSQLExpressions('target', query); + assert.strictEqual(sql, expectedSql.removeExtraWhiteSpace()); + assert.deepEqual(parameters, expectedParams); + }, + ); - const { sql, parameters } = generateSQLExpressions('target', query); - assert.strictEqual( - sql, - ` - SELECT * FROM "target" - WHERE (type = ?) AND rowid IN (SELECT rowid FROM target_fts WHERE target_fts MATCH ?) - ORDER BY created_time DESC`.removeExtraWhiteSpace(), - ); - assert.deepEqual(parameters, [ - 'ssh', - 'name:"favorite"* OR description:"favorite"*', - ]); - }); + test.each( + 'it generates joins with filters', + { + 'basic joins with filters': { + query: { + filters: { + type: [{ equals: 'ssh' }], + joins: [ + { + resource: 'session', + query: { + filters: { + status: [{ equals: 'active' }], + }, + }, + joinOn: 'target_id', + joinType: 'INNER', + }, + { + resource: 'session-recording', + query: { + filters: { + state: [{ equals: 'available' }], + }, + }, + joinFrom: 'idx', + joinOn: 'target_id', + joinType: 'LEFT', + }, + ], + }, + }, + select: [{ field: 'data' }], + expectedSql: ` + SELECT "target".data FROM "target" + INNER JOIN "session" session1 ON "target".id = session1.target_id LEFT JOIN "session_recording" session_recording1 ON "target".idx = session_recording1.target_id + WHERE ("target".type = ?) AND ("session1".status = ?) AND ("session_recording1".state = ?) + ORDER BY "target".created_time DESC`, + expectedParams: ['ssh', 'active', 'available'], + }, + 'multiple joins same table': { + query: { + filters: { + joins: [ + { + resource: 'session', + query: { + filters: { + status: [{ equals: 'active' }], + }, + }, + joinOn: 'target_id', + joinType: 'INNER', + }, + { + resource: 'session', + query: { + filters: { + status: [{ equals: 'pending' }], + }, + }, + joinOn: 'host_id', + joinType: 'LEFT', + }, + ], + }, + }, + select: [{ field: 'id' }], + expectedSql: ` + SELECT "target".id FROM "target" + INNER JOIN "session" session1 ON "target".id = session1.target_id LEFT JOIN "session" session2 ON "target".id = session2.host_id + WHERE ("session1".status = ?) AND ("session2".status = ?) + ORDER BY "target".created_time DESC`, + expectedParams: ['active', 'pending'], + }, + }, + function (assert, { query, select, expectedSql, expectedParams }) { + const { sql, parameters } = generateSQLExpressions('target', query, { + select, + }); + assert.strictEqual(sql, expectedSql.removeExtraWhiteSpace()); + assert.deepEqual(parameters, expectedParams); + }, + ); test('it generates SQL with all clauses combined', function (assert) { const query = { @@ -433,9 +535,9 @@ module('Unit | Utility | sqlite-query', function (hooks) { assert.strictEqual( sql, ` - SELECT data FROM "target" - WHERE (type = ?) AND (status IN (?, ?)) AND (created_time >= ?) AND rowid IN (SELECT rowid FROM target_fts WHERE target_fts MATCH ?) - ORDER BY name COLLATE NOCASE DESC, name DESC + SELECT "target".data FROM "target" + WHERE ("target".type = ?) AND ("target".status IN (?, ?)) AND ("target".created_time >= ?) AND "target".rowid IN (SELECT rowid FROM target_fts WHERE target_fts MATCH ?) + ORDER BY "target".name COLLATE NOCASE DESC, "target".name DESC LIMIT ? OFFSET ?`.removeExtraWhiteSpace(), ); @@ -506,7 +608,7 @@ module('Unit | Utility | sqlite-query', function (hooks) { targetSql, ` SELECT * FROM "target" - ORDER BY created_time DESC`.removeExtraWhiteSpace(), + ORDER BY "target".created_time DESC`.removeExtraWhiteSpace(), ); assert.deepEqual(targetParameters, []); diff --git a/ui/admin/app/routes/scopes/scope/targets/index.js b/ui/admin/app/routes/scopes/scope/targets/index.js index b45faa513..cb913a3bc 100644 --- a/ui/admin/app/routes/scopes/scope/targets/index.js +++ b/ui/admin/app/routes/scopes/scope/targets/index.js @@ -9,6 +9,8 @@ import { restartableTask, timeout } from 'ember-concurrency'; import { STATUS_SESSION_ACTIVE, STATUS_SESSION_PENDING, + STATUS_SESSION_CANCELING, + STATUS_SESSION_TERMINATED, } from 'api/models/session'; import { TYPE_TARGET_SSH, TYPE_TARGET_TCP } from 'api/models/target'; @@ -92,19 +94,25 @@ export default class ScopesScopeTargetsIndexRoute extends Route { }); if (this.can.can('list model', scope, { collection: 'sessions' })) { - const sessions = await this.store.query('session', { - scope_id, - query: { - filters: { - scope_id: [{ equals: scope_id }], - status: [ - { equals: STATUS_SESSION_ACTIVE }, - { equals: STATUS_SESSION_PENDING }, - ], + await this.store.query( + 'session', + { + scope_id, + query: { + filters: { + scope_id: [{ equals: scope_id }], + status: [ + { equals: STATUS_SESSION_ACTIVE }, + { equals: STATUS_SESSION_PENDING }, + ], + }, }, + page: 1, + pageSize: 1, }, - }); - this.addActiveSessionFilters(filters, availableSessions, sessions); + { pushToStore: false }, + ); + this.addActiveSessionFilters(filters, availableSessions); } const typeMap = { @@ -186,13 +194,8 @@ export default class ScopesScopeTargetsIndexRoute extends Route { * Add the filters for active sessions to the filter object. * @param filters * @param availableSessions - * @param sessions */ - addActiveSessionFilters = (filters, availableSessions, sessions) => { - const uniqueTargetIdsWithSessions = new Set( - sessions.map((session) => session.target_id), - ); - + addActiveSessionFilters = (filters, availableSessions) => { // Don't add any filtering if the user selects both which is equivalent to no filters if (availableSessions.length === 2) { return; @@ -200,24 +203,51 @@ export default class ScopesScopeTargetsIndexRoute extends Route { availableSessions.forEach((availability) => { if (availability === 'yes') { - filters.id.logicalOperator = 'or'; - uniqueTargetIdsWithSessions.forEach((targetId) => { - filters.id.values.push({ equals: targetId }); - }); - - // If there's no sessions just set it to a dummy value - // so the search returns no results - if (uniqueTargetIdsWithSessions.size === 0) { - filters.id.values.push({ equals: 'none' }); - } + filters.joins = [ + { + resource: 'session', + query: { + filters: { + status: { + logicalOperator: 'or', + values: [ + { equals: STATUS_SESSION_ACTIVE }, + { equals: STATUS_SESSION_PENDING }, + ], + }, + }, + }, + joinOn: 'target_id', + joinType: 'INNER', + }, + ]; } if (availability === 'no') { - filters.id.logicalOperator = 'and'; - - uniqueTargetIdsWithSessions.forEach((targetId) => { - filters.id.values.push({ notEquals: targetId }); - }); + filters.joins = [ + { + resource: 'session', + query: { + filters: { + status: { + logicalOperator: 'or', + values: [ + { equals: STATUS_SESSION_CANCELING }, + { equals: STATUS_SESSION_TERMINATED }, + // We include null here because traditionally with a LEFT JOIN we'd want to do a check on + // `OR target_id IS NOT NULL` but this is tough to get right due to the presence of other + // possible filters and operator precedence with the OR. We can check for a null status instead + // which functionally is the same since status is normally a required field and that means + // a session does not exist for a particular target. + { equals: null }, + ], + }, + }, + }, + joinOn: 'target_id', + joinType: 'LEFT', + }, + ]; } }); };