Element sorting handling inserted templates (#33)

* sorting

* prevent loops while sorting

* version bump

Author: hero101

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "whiteboard-collaboration-service",
-  "version": "0.4.1",
+  "version": "0.4.2",
   "description": "Alkemio Whiteboard Collaboration Service for Excalidraw backend",
   "author": "Alkemio Foundation",
   "private": false,

package.json

@@ -505,8 +505,11 @@ export class Server {
   ): ThrottledSaveFunction {
     const throttledSave = throttle(
       async (roomId: string) => {
-        await this.saveRoom(roomId);
-        this.notifyRoomSaved(roomId);
+        const hasSaved = await this.saveRoom(roomId);
+
+        if (hasSaved) {
+          this.notifyRoomSaved(roomId);
+        }
       },
       wait,
       { leading: true, trailing: true },
@@ -572,21 +575,23 @@ export class Server {
     return snapshotContent;
   }
 
-  private async saveRoom(roomId: string) {
+  private async saveRoom(roomId: string): Promise<boolean> {
     const snapshot = this.snapshots.get(roomId);
     if (!snapshot) {
       this.logger.error(
         `No snapshot found for room '${roomId}' in the local storage!`,
       );
-      return;
+      return false;
     }
 
     const cleanContent = prepareContentForSave(snapshot);
     const { data } = await this.utilService.save(roomId, cleanContent);
     if (isSaveErrorData(data)) {
       this.logger.error(`Failed to save room '${roomId}': ${data.error}`);
+      return false;
     } else {
       this.logger.verbose?.(`Room '${roomId}' saved successfully`);
+      return true;
     }
   }
 }

src/excalidraw-backend/server.ts

@@ -1,7 +1,8 @@
+import { Logger } from '@nestjs/common';
+import { unionBy } from 'lodash';
 import { arrayToMap } from './array.to.map';
 import { ExcalidrawElement } from '../../excalidraw/types';
 import { arrayToMapBy } from './array.to.map.by';
-import { Logger } from '@nestjs/common';
 // import { orderByFractionalIndex, syncInvalidIndices } from './fractionalIndex';
 
 const shouldDiscardRemoteElement = (
@@ -97,20 +98,28 @@ export const reconcileElements = (
   // de-duplicate indices
   // const syncedElemented = syncInvalidIndices(orderedElements);
   try {
-    return orderByPrecedingElement(reconciledElements);
+    return tryOrderByPrecedingElement(reconciledElements);
   } catch (error) {
     logger.warn(`Element sorting failed with error: '${error.message}'`);
     return reconciledElements;
   }
 };
 
-const orderByPrecedingElement = (
+/**
+ * Will try to order elements by preceding element.
+ * Order is not guaranteed, if there are multiple "first" elements, or a preceding element that does not exist.
+ * Returns a half sorted array if there is an element with preceding element that does not exist.
+ * @param unOrderedElements
+ * @throws Error if there is more than one element with preceding element = '^'
+ */
+const tryOrderByPrecedingElement = (
   unOrderedElements: ExcalidrawElement[],
 ): ExcalidrawElement[] | never => {
   // for zero or one element return the same array, as it's already sorted
   if (unOrderedElements.length < 2) {
     return unOrderedElements;
   }
+  // const elementsWithPreceding = unOrderedElements.filter(el.
   // validated there is just one starting element
   const startElements = unOrderedElements.filter(
     (el) => el.__precedingElement__ === '^',
@@ -131,14 +140,24 @@ const orderByPrecedingElement = (
   // the array is starting with element that has no preceding element
   let parentElement = startElements[0];
   orderedElements.push(parentElement);
-  // Follow the chain of __precedingElement__
-  while (true) {
+  // keep track of visited elements to prevent cycles
+  const visitedElements = new Set<string>();
+  // Follow the chain of __precedingElement__ until we have sorted all
+  while (orderedElements.length != unOrderedElements.length) {
+    // prevent cycles in the chain
+    if (visitedElements.has(parentElement.id)) {
+      throw new Error('Cycle detected in __precedingElement__ chain');
+    }
+    visitedElements.add(parentElement.id);
+    // a child of a parent, is an element which __precedingElement__ is pointing to the parent
+    // is there an element which preceding element is the parent element
     const childElement = elementMapByPrecedingKey.get(parentElement.id);
-
+    // there is a parent, which has no child; the chain is broken
     if (!childElement) {
-      // we have reached the end of the chain
-      break;
+      // switch both arrays; combine ordered and switch the unordered at the end
+      return unionBy(orderedElements, unOrderedElements, 'id');
     }
+    // the final element is pointing to the one before it (preceding element)
 
     orderedElements.push(childElement);
     parentElement = childElement;