Unload expired sessions from store (#3033)

* 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

Author: lisbet-alvarez

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) => {

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 };

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,14 +389,18 @@ 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');
     assert
       .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',
     );

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.
 

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,

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);

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();