Restore filtering out documents before saving them on disk

web/packages/teleterm/src/ui/services/workspacesService/workspacesService.test.ts

@@ -33,7 +33,11 @@ import { NotificationsService } from '../notifications';
 import { ModalsService } from '../modals';
 
 import { getEmptyPendingAccessRequest } from './accessRequestsService';
-import { Workspace, WorkspacesService } from './workspacesService';
+import {
+  Workspace,
+  WorkspacesService,
+  WorkspacesState,
+} from './workspacesService';
 import { DocumentCluster, DocumentsService } from './documentsService';
 
 import type * as tshd from 'teleterm/services/tshd/types';
@@ -138,7 +142,7 @@ describe('restoring workspace', () => {
     });
   });
 
-  it('does not restore doc.authorize_web_session documents', async () => {
+  it('location is set to first document if it points to non-existing document', async () => {
     const cluster = makeRootCluster();
     const testWorkspace: Workspace = {
       accessRequests: {
@@ -149,22 +153,16 @@ describe('restoring workspace', () => {
       documents: [
         {
           kind: 'doc.terminal_shell',
-          uri: '/docs/terminal_shell_uri',
+          uri: '/docs/terminal_shell_uri_1',
           title: '/Users/alice/Documents',
         },
         {
-          kind: 'doc.authorize_web_session',
-          rootClusterUri: cluster.uri,
-          uri: '/docs/authorize_web_session_uri',
-          webSessionRequest: {
-            id: '',
-            token: '',
-            redirectUri: '',
-          },
-          title: 'Authorize Web Session',
+          kind: 'doc.terminal_shell',
+          uri: '/docs/terminal_shell_uri_2',
+          title: '/Users/alice/Documents',
         },
       ],
-      location: '/docs/authorize_web_session_uri',
+      location: '/docs/non-existing-doc',
     };
 
     const { workspacesService } = getTestSetup({
@@ -182,7 +180,70 @@ describe('restoring workspace', () => {
           title: '/Users/alice/Documents',
         },
       ],
-      location: '/docs/terminal_shell_uri', // location no longer points to the document that was omitted
+      location: '/docs/terminal_shell_uri',
+    });
+  });
+});
+
+describe('state persistence', () => {
+  it('doc.authorize_web_session is not stored to disk', () => {
+    const cluster = makeRootCluster();
+    const workspacesState: WorkspacesState = {
+      rootClusterUri: cluster.uri,
+      isInitialized: true,
+      workspaces: {
+        [cluster.uri]: {
+          accessRequests: {
+            isBarCollapsed: true,
+            pending: getEmptyPendingAccessRequest(),
+          },
+          localClusterUri: cluster.uri,
+          documents: [
+            {
+              kind: 'doc.terminal_shell',
+              uri: '/docs/terminal_shell_uri',
+              title: '/Users/alice/Documents',
+            },
+            {
+              kind: 'doc.authorize_web_session',
+              uri: '/docs/authorize_web_session',
+              rootClusterUri: cluster.uri,
+              title: 'Authorize Web Session',
+              webSessionRequest: {
+                id: '',
+                token: '',
+                redirectUri: '',
+              },
+            },
+          ],
+          location: '/docs/authorize_web_session',
+        },
+      },
+    };
+    const { workspacesService, statePersistenceService } = getTestSetup({
+      cluster,
+      persistedWorkspaces: {},
+    });
+
+    workspacesService.setState(() => workspacesState);
+
+    expect(statePersistenceService.saveWorkspacesState).toHaveBeenCalledTimes(
+      1
+    );
+    expect(statePersistenceService.saveWorkspacesState).toHaveBeenCalledWith({
+      rootClusterUri: cluster.uri,
+      workspaces: {
+        [cluster.uri]: expect.objectContaining({
+          documents: [
+            {
+              kind: 'doc.terminal_shell',
+              uri: '/docs/terminal_shell_uri',
+              title: '/Users/alice/Documents',
+            },
+          ],
+          location: '/docs/authorize_web_session',
+        }),
+      },
     });
   });
 });
@@ -349,5 +410,6 @@ function getTestSetup(options: {
     clusterDocument,
     modalsService,
     notificationsService,
+    statePersistenceService,
   };
 }

web/packages/teleterm/src/ui/services/workspacesService/workspacesService.ts

@@ -421,22 +421,20 @@ export class WorkspacesService extends ImmutableStore<WorkspacesState> {
         const workspaceDefaultState = this.getWorkspaceDefaultState(
           persistedWorkspace?.localClusterUri || cluster.uri
         );
-        const restorableDocuments = getRestorableDocuments(
-          persistedWorkspace?.documents || []
-        );
+        const persistedWorkspaceDocuments = persistedWorkspace?.documents;
 
         workspaces[cluster.uri] = {
           ...workspaceDefaultState,
           previous: this.canReopenPreviousDocuments({
-            previousDocuments: restorableDocuments,
+            previousDocuments: persistedWorkspaceDocuments,
             currentDocuments: workspaceDefaultState.documents,
           })
             ? {
                 location: getLocationToRestore(
-                  restorableDocuments,
+                  persistedWorkspaceDocuments,
                   persistedWorkspace.location
                 ),
-                documents: restorableDocuments,
+                documents: persistedWorkspaceDocuments,
               }
             : undefined,
           connectMyComputer: persistedWorkspace?.connectMyComputer,
@@ -575,20 +573,8 @@ export class WorkspacesService extends ImmutableStore<WorkspacesState> {
     };
     for (let w in this.state.workspaces) {
       const workspace = this.state.workspaces[w];
-      const documentsToPersist = (
+      const documentsToPersist = getDocumentsToPersist(
         workspace.previous?.documents || workspace.documents
-      ).map(d =>
-        d.kind === 'doc.authorize_web_session'
-          ? {
-              ...d,
-              // Do not store potentially sensitive properties.
-              webSessionRequest: {
-                id: '',
-                token: '',
-                redirectUri: '',
-              },
-            }
-          : d
       );
 
       stateToSave.workspaces[w] = {
@@ -650,19 +636,19 @@ export const useWorkspaceServiceState = () => {
   return useStoreSelector('workspacesService', identitySelector);
 };
 
-function getRestorableDocuments(documents: Document[]): Document[] {
+function getDocumentsToPersist(documents: Document[]): Document[] {
   return (
     documents
-      // We don't restore 'doc.authorize_web_session' because the user would not
-      // be able to authorize a session at a later time anyway.
+      // We don't persist 'doc.authorize_web_session' because we don't want to store
+      // a session token and id on disk.
+      // Moreover, the user would not be able to authorize a session at a later time anyway.
       .filter(d => d.kind !== 'doc.authorize_web_session')
   );
 }
 
-/** Assumes that there is at least one document to restore. */
 function getLocationToRestore(
   documents: Document[],
   location: DocumentUri
-): DocumentUri {
-  return documents.find(d => d.uri === location) ? location : documents[0]!.uri;
+): DocumentUri | undefined {
+  return documents.find(d => d.uri === location) ? location : documents[0]?.uri;
 }