From 48d0087141ac984f52d0323fae35a77c9640e9be Mon Sep 17 00:00:00 2001 From: Lisbet Alvarez <107949262+lisbet-alvarez@users.noreply.github.com> Date: Tue, 11 Nov 2025 11:16:12 -0800 Subject: [PATCH] Unload expired sessions from store (#3033) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: 🐛 unload expired sessions from ember data store * fix: 🐛 add unloadAll & include terminated to admin ui * test: 💍 update tests * refactor: 💡 pr feedback for admin ui * refactor: 💡 remove unneeded query from desktop ui * refactor: 💡 check pushToStore var * Revert "refactor: 💡 remove unneeded query from desktop ui" * fix: 🐛 fix proxy details being removed after unload * refactor: 💡 remove log --- addons/api/addon/handlers/sqlite-handler.js | 20 ++++++--- .../app/routes/scopes/scope/targets/index.js | 3 +- .../tests/acceptance/targets/list-test.js | 10 +++-- ui/desktop/README.md | 4 +- .../scopes/scope/projects/targets/index.js | 42 ++++++++++++++----- ui/desktop/electron-app/src/models/session.js | 7 +++- .../acceptance/projects/targets/index-test.js | 15 +++++-- 7 files changed, 75 insertions(+), 26 deletions(-) diff --git a/addons/api/addon/handlers/sqlite-handler.js b/addons/api/addon/handlers/sqlite-handler.js index 85317364a..fc226b025 100644 --- a/addons/api/addon/handlers/sqlite-handler.js +++ b/addons/api/addon/handlers/sqlite-handler.js @@ -107,6 +107,7 @@ export default class SqliteHandler { payload, storeToken, tokenKey, + pushToStore, serializer, store, schema, @@ -213,6 +214,7 @@ export default class SqliteHandler { payload, storeToken, tokenKey, + pushToStore, serializer, store, schema, @@ -224,11 +226,6 @@ export default class SqliteHandler { ]); } - // Remove any records from the DB if the API indicates they've been deleted - if (payload.removed_ids?.length > 0) { - await this.sqlite.deleteResource(type, payload.removed_ids); - } - const normalizedPayload = serializer.normalizeResponse( store, schema, @@ -239,6 +236,19 @@ export default class SqliteHandler { const { data: payloadData } = normalizedPayload; + // Remove any records from the DB if the API indicates they've been deleted + // Additionally remove deleted records from ember data store for symmetry + if (payload.removed_ids?.length > 0) { + await this.sqlite.deleteResource(type, payload.removed_ids); + + if (pushToStore) { + payload.removed_ids.forEach((id) => { + const record = store.peekRecord(type, id); + store.unloadRecord(record); + }); + } + } + // Store the new data we just got back from the API refresh const items = payloadData.map((datum) => { const params = Object.values(modelMapping[type]).map((key) => { diff --git a/ui/admin/app/routes/scopes/scope/targets/index.js b/ui/admin/app/routes/scopes/scope/targets/index.js index cb913a3bc..2139df29a 100644 --- a/ui/admin/app/routes/scopes/scope/targets/index.js +++ b/ui/admin/app/routes/scopes/scope/targets/index.js @@ -98,6 +98,7 @@ export default class ScopesScopeTargetsIndexRoute extends Route { 'session', { scope_id, + include_terminated: true, query: { filters: { scope_id: [{ equals: scope_id }], @@ -155,7 +156,7 @@ export default class ScopesScopeTargetsIndexRoute extends Route { }, }, }, - { pushToStore: true, peekDb: true }, + { peekDb: true }, ); } return { targets, doTargetsExist, totalItems }; diff --git a/ui/admin/tests/acceptance/targets/list-test.js b/ui/admin/tests/acceptance/targets/list-test.js index b56c675f0..ae9ecc426 100644 --- a/ui/admin/tests/acceptance/targets/list-test.js +++ b/ui/admin/tests/acceptance/targets/list-test.js @@ -364,11 +364,11 @@ module('Acceptance | targets | list', function (hooks) { instances.sshTarget.id, 'session is associated with correct target', ); - const emberDataSessionModel = this.owner + const emberDataSessionModelBefore = this.owner .lookup('service:store') .peekRecord('session', instances.session.id); assert.strictEqual( - emberDataSessionModel.status, + emberDataSessionModelBefore.status, STATUS_SESSION_ACTIVE, 'ember data session model is active', ); @@ -389,6 +389,10 @@ module('Acceptance | targets | list', function (hooks) { instances.session.status = STATUS_SESSION_TERMINATED; await click(commonSelectors.HREF(urls.targets)); + + const emberDataSessionModelAfter = this.owner + .lookup('service:store') + .peekRecord('session', instances.session.id); assert .dom(selectors.TABLE_TARGETS_ROW(instances.sshTarget.id)) .exists('the target is still listed in the table'); @@ -396,7 +400,7 @@ module('Acceptance | targets | list', function (hooks) { .dom(selectors.TABLE_ACTIVE_SESSIONS(instances.sshTarget.id)) .doesNotExist('the target does not have an active session'); assert.strictEqual( - emberDataSessionModel.status, + emberDataSessionModelAfter.status, STATUS_SESSION_TERMINATED, 'the session ember data model status is updated', ); diff --git a/ui/desktop/README.md b/ui/desktop/README.md index c6d4b812b..9b0b122e0 100644 --- a/ui/desktop/README.md +++ b/ui/desktop/README.md @@ -122,8 +122,8 @@ The static production assets are saved into the `dist/` folder. The Boundary CLI is downloaded and extracted to `electron-app/cli/` folder as part of packaging. CLI version is defined in `electron-app/config/cli.js`. -Similar to running in development, you can also use `BYPASS_CLI_SETUP=true` to -bypass the download of the CLI, which can be useful for pre-release testing. See +Similar to running in development, you can also use `SETUP_CLI=true` to +enable the download of the CLI, which can be useful for pre-release testing. See [Developing Using Non-Release Versions of Boundary](#developing-using-non-release-versions-of-boundary) for more details. diff --git a/ui/desktop/app/routes/scopes/scope/projects/targets/index.js b/ui/desktop/app/routes/scopes/scope/projects/targets/index.js index 33b096534..072f41598 100644 --- a/ui/desktop/app/routes/scopes/scope/projects/targets/index.js +++ b/ui/desktop/app/routes/scopes/scope/projects/targets/index.js @@ -159,21 +159,22 @@ export default class ScopesScopeProjectsTargetsIndexRoute extends Route { // To correctly show targets with active sessions, the associated // sessions need to be queried to sync all the session models in // ember data and retrieve their updated `status` properties - const sessionsPromise = this.store.query( - 'session', - { - query: { - filters: { - target_id: targets.map((target) => ({ equals: target.id })), - }, + const sessionsPromise = this.store.query('session', { + query: { + filters: { + target_id: targets.map((target) => ({ equals: target.id })), }, }, - { pushToStore: true }, - ); + }); + let allAssociatedSessions; // Load the sessions and aliases for the targets on the current page try { - await Promise.all([aliasPromise, sessionsPromise]); + const loadedResults = await Promise.all([ + aliasPromise, + sessionsPromise, + ]); + allAssociatedSessions = loadedResults[1]; } catch (e) { __electronLog?.warn( 'Could not retrieve aliases and/or sessions for targets', @@ -183,6 +184,27 @@ export default class ScopesScopeProjectsTargetsIndexRoute extends Route { // the page in case the controller doesn't support aliases yet } + // Remove expired sessions in ember data store for symmetry with cache. + if (allAssociatedSessions) { + const targetIds = targets.map(({ id }) => id); + const allAssociatedSessionIds = new Set( + allAssociatedSessions.map(({ id }) => id), + ); + const storedSessionIds = new Set( + this.store + .peekAll('session') + .filter((s) => targetIds.includes(s?.target_id)) + .map(({ id }) => id), + ); + const removedSessions = storedSessionIds.difference( + allAssociatedSessionIds, + ); + removedSessions.forEach((id) => { + const record = this.store.peekRecord('session', id); + this.store.unloadRecord(record); + }); + } + return { targets, projects, diff --git a/ui/desktop/electron-app/src/models/session.js b/ui/desktop/electron-app/src/models/session.js index d3be7fd00..6885037d6 100644 --- a/ui/desktop/electron-app/src/models/session.js +++ b/ui/desktop/electron-app/src/models/session.js @@ -8,6 +8,7 @@ const { spawnAsyncJSONPromise, spawnSync, } = require('../helpers/spawn-promise.js'); +const log = require('electron-log/main'); class Session { #id; @@ -89,7 +90,6 @@ class Session { ).then((spawnedSession) => { this.#process = spawnedSession.childProcess; this.#proxyDetails = spawnedSession.response; - this.#process = spawnedSession.childProcess; this.#id = this.#proxyDetails.session_id; return this.#proxyDetails; }); @@ -102,7 +102,10 @@ class Session { return new Promise((resolve, reject) => { if (this.isRunning) { this.#process.on('close', () => resolve()); - this.#process.on('error', (e) => reject(e)); + this.#process.on('error', (e) => { + log.error('Process error in session stop method: ', e); + return reject(e); + }); // Cancel session before killing process const sanitizedToken = sanitizer.base62EscapeAndValidate(this.#token); diff --git a/ui/desktop/tests/acceptance/projects/targets/index-test.js b/ui/desktop/tests/acceptance/projects/targets/index-test.js index 36dfb88ff..82bc071f7 100644 --- a/ui/desktop/tests/acceptance/projects/targets/index-test.js +++ b/ui/desktop/tests/acceptance/projects/targets/index-test.js @@ -673,11 +673,14 @@ module('Acceptance | projects | targets | index', function (hooks) { assert.strictEqual(instances.session.status, STATUS_SESSION_ACTIVE); await visit(urls.targets); - const emberDataSessionModel = this.owner + const emberDataSessionModelBefore = this.owner .lookup('service:store') .peekRecord('session', instances.session.id); - assert.strictEqual(emberDataSessionModel.status, STATUS_SESSION_ACTIVE); + assert.strictEqual( + emberDataSessionModelBefore.status, + STATUS_SESSION_ACTIVE, + ); assert .dom(activeSessionFlyoutButtonSelector(instances.session.targetId)) @@ -695,7 +698,13 @@ module('Acceptance | projects | targets | index', function (hooks) { await click(`[href="${urls.targets}"]`); - assert.strictEqual(emberDataSessionModel.status, STATUS_SESSION_TERMINATED); + const emberDataSessionModelAfter = this.owner + .lookup('service:store') + .peekRecord('session', instances.session.id); + assert.strictEqual( + emberDataSessionModelAfter.status, + STATUS_SESSION_TERMINATED, + ); assert .dom(activeSessionFlyoutButtonSelector(instances.session.targetId)) .doesNotExist();