Refactoring based on review feedback

Signed-off-by: Milton Moura <[email protected]>
nordeck · Nov 19, 2024 · 3c3652c · 3c3652c
1 parent 3e7b07d
commit 3c3652c
Show file tree

Hide file tree

Showing 14 changed files with 326 additions and 238 deletions.
diff --git a/packages/react-sdk/src/App.tsx b/packages/react-sdk/src/App.tsx
@@ -15,20 +15,105 @@
  */
 
 import { useWidgetApi } from '@matrix-widget-toolkit/react';
+import { getLogger } from 'loglevel';
+import { useCallback, useEffect } from 'react';
 import { useTranslation } from 'react-i18next';
 import { Layout, LayoutProps } from './components/Layout';
 import { PageLoader } from './components/common/PageLoader';
-import { useOwnedWhiteboard, useWhiteboardManager } from './state';
+import {
+  isValidWhiteboardDocumentSnapshot,
+  useActiveWhiteboardInstance,
+  useOwnedWhiteboard,
+  useWhiteboardManager,
+} from './state';
+import {
+  cancelableSnapshotTimer,
+  usePersistOnJoinOrInvite,
+} from './state/usePersistOnJoinOrInvite';
+import { useGetDocumentSnapshotQuery } from './store/api';
 
 export type AppProps = {
   layoutProps?: LayoutProps;
 };
 
+const logger = getLogger('App');
+
 export const App = ({ layoutProps }: AppProps) => {
   const { t } = useTranslation('neoboard');
   const { value, loading } = useOwnedWhiteboard();
   const whiteboardManager = useWhiteboardManager();
-  const ownUserId = useWidgetApi().widgetParameters.userId;
+  const whiteboardInstance = useActiveWhiteboardInstance();
+  const widgetApi = useWidgetApi();
+  const ownUserId = widgetApi.widgetParameters.userId;
+  const { limitedHistoryVisibility, delayedPersist, lastMembershipEventTs } =
+    usePersistOnJoinOrInvite();
+  const { data: latestSnapshot } = useGetDocumentSnapshotQuery({
+    documentId: whiteboardInstance.getDocumentId(),
+    validator: isValidWhiteboardDocumentSnapshot,
+  });
+
+  const handlePersist = useCallback(() => {
+    if (latestSnapshot !== undefined && lastMembershipEventTs !== undefined) {
+      if (latestSnapshot.event.origin_server_ts < lastMembershipEventTs) {
+        logger.debug('Saving snapshot due to membership updates');
+        whiteboardInstance.persist(true);
+      }
+    }
+  }, [latestSnapshot, lastMembershipEventTs, whiteboardInstance]);
+
+  useEffect(() => {
+    // We don't need to do anything if we're not in a room with limited history visibility
+    // or the whiteboard is still loading
+    if (!limitedHistoryVisibility || whiteboardInstance.isLoading()) {
+      return;
+    }
+
+    // We're in a room with limited history visibility, so we check
+    // if a snapshot is required after recent membership changes (invites+joins)
+    if (latestSnapshot !== undefined && lastMembershipEventTs !== undefined) {
+      // Is the snapshot outdated when compared to the last membership event?
+      if (latestSnapshot.event.origin_server_ts < lastMembershipEventTs) {
+        // We don't delay persisting if the membership event was sent by the current user
+        if (!delayedPersist) {
+          logger.debug(
+            'Saving snapshot immediately due to current user sending the membership event',
+          );
+
+          whiteboardInstance.persist(true);
+        } else {
+          // Start a cancelable timer to persist the snapshot after a random delay
+          // If other clients run this earlier and send a snapshot, we don't need to persist
+          const delay = Math.floor(Math.random() * 20) + 10;
+          const timer = cancelableSnapshotTimer(handlePersist, delay * 1000);
+          logger.debug(
+            'Will try to save a snapshot in ',
+            delay,
+            's due to membership updates',
+          );
+
+          // cancel the timer if the component unmounts or the dependencies change
+          return () => {
+            logger.debug('Canceled delayed snapshot persistence timer');
+            timer.cancel();
+          };
+        }
+      } else {
+        logger.debug(
+          'We have a fresh snapshot after membership updates, no need to persist',
+        );
+      }
+    }
+    return;
+  }, [
+    delayedPersist,
+    handlePersist,
+    lastMembershipEventTs,
+    latestSnapshot,
+    limitedHistoryVisibility,
+    loading,
+    whiteboardInstance,
+    widgetApi,
+  ]);
 
   if (!ownUserId) {
     throw new Error('Unknown user id');

diff --git a/packages/react-sdk/src/components/FallbackSnapshotProvider/index.ts b/packages/react-sdk/src/components/FallbackSnapshotProvider/index.ts
diff --git a/packages/react-sdk/src/components/Layout/Layout.tsx b/packages/react-sdk/src/components/Layout/Layout.tsx
@@ -29,7 +29,6 @@ import { BoardBar } from '../BoardBar';
 import { CollaborationBar } from '../CollaborationBar';
 import { DeveloperTools } from '../DeveloperTools/DeveloperTools';
 import { ElementOverridesProvider } from '../ElementOverridesProvider';
-import { FallbackSnapshotProvider } from '../FallbackSnapshotProvider';
 import { FullscreenModeBar } from '../FullscreenModeBar';
 import { GuidedTour } from '../GuidedTour';
 import { HelpCenterBar } from '../HelpCenterBar';
@@ -78,54 +77,49 @@ export function Layout({ height = '100vh' }: LayoutProps) {
   }
 
   return (
-    <FallbackSnapshotProvider>
-      <SlidesProvider>
-        <ImageUploadProvider>
-          <ImportWhiteboardDialogProvider>
-            <GuidedTour disabled={isViewingPresentation} />
-
-            <Stack
-              height={!isFullscreenMode ? height : '100vh'}
-              direction="row"
-              bgcolor="background.paper"
+    <SlidesProvider>
+      <ImageUploadProvider>
+        <ImportWhiteboardDialogProvider>
+          <GuidedTour disabled={isViewingPresentation} />
+
+          <Stack
+            height={!isFullscreenMode ? height : '100vh'}
+            direction="row"
+            bgcolor="background.paper"
+          >
+            <AnimatedSidebar
+              visible={isSlideOverviewVisible && !isViewingPresentation}
+              direction="right"
             >
-              <AnimatedSidebar
-                visible={isSlideOverviewVisible && !isViewingPresentation}
-                direction="right"
-              >
-                <SlideOverviewBar />
-              </AnimatedSidebar>
-
-              <Box
-                component="main"
-                flex={1}
-                display="flex"
-                position="relative"
-                onDragEnter={handleUploadDragEnter}
-              >
-                {slideIds.map((slideId) => (
-                  <TabPanelStyled value={slideId} key={slideId}>
-                    <SlideProvider slideId={slideId}>
-                      <ElementOverridesProvider>
-                        <ContentArea />
-                        {uploadDragOverlay}
-                      </ElementOverridesProvider>
-                    </SlideProvider>
-                  </TabPanelStyled>
-                ))}
-              </Box>
-
-              <AnimatedSidebar
-                visible={isDeveloperToolsVisible}
-                direction="left"
-              >
-                <DeveloperTools />
-              </AnimatedSidebar>
-            </Stack>
-          </ImportWhiteboardDialogProvider>
-        </ImageUploadProvider>
-      </SlidesProvider>
-    </FallbackSnapshotProvider>
+              <SlideOverviewBar />
+            </AnimatedSidebar>
+
+            <Box
+              component="main"
+              flex={1}
+              display="flex"
+              position="relative"
+              onDragEnter={handleUploadDragEnter}
+            >
+              {slideIds.map((slideId) => (
+                <TabPanelStyled value={slideId} key={slideId}>
+                  <SlideProvider slideId={slideId}>
+                    <ElementOverridesProvider>
+                      <ContentArea />
+                      {uploadDragOverlay}
+                    </ElementOverridesProvider>
+                  </SlideProvider>
+                </TabPanelStyled>
+              ))}
+            </Box>
+
+            <AnimatedSidebar visible={isDeveloperToolsVisible} direction="left">
+              <DeveloperTools />
+            </AnimatedSidebar>
+          </Stack>
+        </ImportWhiteboardDialogProvider>
+      </ImageUploadProvider>
+    </SlidesProvider>
   );
 }
 

diff --git a/packages/react-sdk/src/lib/testUtils/documentTestUtils.tsx b/packages/react-sdk/src/lib/testUtils/documentTestUtils.tsx
@@ -130,7 +130,6 @@ export function mockWhiteboardManager(
     observeIsLoading: () => of(false),
     destroy: () => {},
     persist: vi.fn().mockResolvedValue(undefined),
-    getLatestDocumentSnapshot: () => undefined,
   };
 
   const whiteboardInstance = new WhiteboardInstanceImpl(

diff --git a/packages/react-sdk/src/model/roomHistoryVisibilityEvent.ts b/packages/react-sdk/src/model/roomHistoryVisibilityEvent.ts
@@ -29,7 +29,9 @@ const roomHistoryVisibilityEventSchema = Joi.object<
   RoomHistoryVisibilityEvent,
   true
 >({
-  history_visibility: Joi.string().required(),
+  history_visibility: Joi.string()
+    .required()
+    .valid('invited', 'joined', 'shared', 'world_readable'),
 }).unknown();
 
 export function isValidRoomHistoryVisibilityEvent(

diff --git a/packages/react-sdk/src/state/presentationManagerImpl.test.ts b/packages/react-sdk/src/state/presentationManagerImpl.test.ts
@@ -80,6 +80,7 @@ describe('presentationManager', () => {
       getPresentationManager: vi.fn(),
       getSlide: vi.fn(),
       getSlideIds: vi.fn(),
+      getDocumentId: vi.fn(),
       getWhiteboardId: vi.fn(),
       getWhiteboardStatistics: vi.fn(),
       import: vi.fn(),

diff --git a/packages/react-sdk/src/state/synchronizedDocumentImpl.ts b/packages/react-sdk/src/state/synchronizedDocumentImpl.ts
@@ -14,7 +14,6 @@
  * limitations under the License.
  */
 
-import { RoomEvent } from '@matrix-widget-toolkit/api';
 import { Base64 } from 'js-base64';
 import { clone } from 'lodash';
 import { getLogger } from 'loglevel';
@@ -35,7 +34,6 @@ import {
   throttleTime,
 } from 'rxjs';
 import { isDefined } from '../lib';
-import { DocumentSnapshot } from '../model';
 import {
   documentSnapshotApi,
   setSnapshotFailed,
@@ -200,8 +198,8 @@ export class SynchronizedDocumentImpl<T extends Record<string, unknown>>
   }
 
   private async persistDocument(doc: Document<T>, force = false) {
+    // If there is no outstanding snapshot and we are not forcing to persist, we can skip this
     if (!this.statistics.snapshotOutstanding && !force) {
-      // No outstanding snapshot, do nothing
       return;
     }
 
@@ -223,24 +221,6 @@ export class SynchronizedDocumentImpl<T extends Record<string, unknown>>
     await this.persistDocument(this.document, force);
   }
 
-  getLatestDocumentSnapshot(): RoomEvent<DocumentSnapshot> | undefined {
-    const state = this.store.getState();
-    const documentId = this.documentId;
-
-    const result = documentSnapshotApi.endpoints.getDocumentSnapshot.select({
-      documentId,
-      validator: () => true,
-    })(state);
-
-    if (!result.isLoading) {
-      const snapshotData = result.data;
-
-      if (snapshotData) {
-        return snapshotData.event;
-      }
-    }
-  }
-
   private async createDocumentSnapshot(
     documentId: string,
     data: Uint8Array,

diff --git a/packages/react-sdk/src/state/types.ts b/packages/react-sdk/src/state/types.ts
@@ -14,9 +14,9 @@
  * limitations under the License.
  */
 
-import { RoomEvent, StateEvent, WidgetApi } from '@matrix-widget-toolkit/api';
+import { StateEvent, WidgetApi } from '@matrix-widget-toolkit/api';
 import { BehaviorSubject, Observable } from 'rxjs';
-import { DocumentSnapshot, Whiteboard } from '../model';
+import { Whiteboard } from '../model';
 import { CommunicationChannelStatistics } from './communication';
 import {
   Document,
@@ -55,13 +55,10 @@ export type WhiteboardStatistics = {
   communicationChannel: CommunicationChannelStatistics;
 };
 
-export type PersistOptions = {
-  timestamp: number;
-  immediate: boolean;
-};
-
 /** An instance of a whiteboard that can be used to read and manipulate it. */
 export type WhiteboardInstance = {
+  /** Returns the id of the document */
+  getDocumentId(): string;
   /** Returns the id of the whiteboard. */
   getWhiteboardId(): string;
   /**
@@ -153,7 +150,7 @@ export type WhiteboardInstance = {
   destroy(): void;
 
   /** Persist the whiteboard state. */
-  persist(options?: PersistOptions): Promise<void>;
+  persist(force?: boolean): Promise<void>;
 };
 
 export type ElementUpdate = {
@@ -267,8 +264,6 @@ export type SynchronizedDocument<T extends Record<string, unknown>> = {
   destroy(): void;
   /** Persist the document immediately. */
   persist(force: boolean): Promise<void>;
-  /** Get the latest document snapshot, if available */
-  getLatestDocumentSnapshot(): RoomEvent<DocumentSnapshot> | undefined;
 };
 
 export type PresentationState =