[pull] main from liveblocks:main

packages/liveblocks-server/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@liveblocks/server",
-  "version": "1.4.2-pre1",
+  "version": "1.5.0",
   "description": "Liveblocks backend server foundation.",
   "type": "module",
   "main": "./dist/index.js",
@@ -69,7 +69,7 @@
   },
   "sideEffects": false,
   "dependencies": {
-    "@liveblocks/core": "3.18.0",
+    "@liveblocks/core": "3.20.0-pre1",
     "async-mutex": "^0.4.0",
     "decoders": "^2.9.0",
     "itertools": "^2.7.1",

packages/liveblocks-server/src/Storage.ts

@@ -239,34 +239,26 @@ export class Storage {
     op: CreateOp & HasOpId,
     node: SerializedChild
   ): Promise<ApplyOpResult> {
-    let fix: FixOp | undefined;
-
     // The default intent, when not explicitly provided, is to insert, not set,
     // into the list.
-    const intent: "insert" | "set" = op.intent ?? "insert";
+    const intent: "insert" | "set" | "push" = op.intent ?? "insert";
 
     // istanbul ignore else
     if (intent === "insert") {
-      const insertedParentKey = await this.insertIntoList(op.id, node);
-
-      // If the inserted parent key is different from the input, it means there
-      // was a conflict and the node has been inserted in an alternative free
-      // list position. We should broadcast a modified Op to all clients that
-      // has the modified position, and send a "fix" op back to the originating
-      // client.
-      if (insertedParentKey !== node.parentKey) {
-        op = { ...op, parentKey: insertedParentKey };
-        fix = {
-          type: OpCode.SET_PARENT_KEY,
-          id: op.id,
-          parentKey: insertedParentKey,
-        };
-        return accept(op, fix);
-      }
-
-      // No conflict, node got inserted as intended
-      return accept(op);
+      // Insert at the client's preferred position, resolving any collision to a
+      // nearby free slot.
+      return this.acceptAndFix(
+        op,
+        node,
+        await this.insertIntoList(op.id, node)
+      );
+    } else if (intent === "push") {
+      // Server-authoritative append: place the node after the authoritative
+      // end of the list (see `appendToList`), regardless of the client's preference.
+      return this.acceptAndFix(op, node, await this.appendToList(op.id, node));
     } else if (intent === "set") {
+      let fix: FixOp | undefined;
+
       // The intent here is to "set", not insert, into the list, replacing the
       // existing item that
 
@@ -310,6 +302,25 @@ export class Storage {
     }
   }
 
+  /**
+   * Accept a freshly placed list item. If the server chose a different
+   * position in the end (conflict resolution), broadcast only the corrected Op
+   * to all clients and send a "fix" op back to the originating client.
+   */
+  private acceptAndFix(
+    op: CreateOp & HasOpId,
+    node: SerializedChild,
+    finalKey: string
+  ): ApplyOpResult {
+    if (finalKey !== node.parentKey) {
+      return accept(
+        { ...op, parentKey: finalKey },
+        { type: OpCode.SET_PARENT_KEY, id: op.id, parentKey: finalKey }
+      );
+    }
+    return accept(op);
+  }
+
   private async applyDeleteObjectKeyOp(
     op: DeleteObjectKeyOp & HasOpId
   ): Promise<ApplyOpResult> {
@@ -378,6 +389,33 @@ export class Storage {
     return node.parentKey;
   }
 
+  /**
+   * Server-authoritative append: places the node strictly after every existing
+   * sibling under its list parent. If the client's preferred key already sorts
+   * after the current last sibling it's kept as-is (guaranteed free, since
+   * it's beyond the max); otherwise the node is placed right after the last
+   * sibling. Because Ops are processed serially, the chosen key is always
+   * free, so concurrent pushes never collide.
+   *
+   * Returns the final key that was used for the insertion.
+   */
+  private async appendToList(
+    id: string,
+    node: SerializedChild
+  ): Promise<string> {
+    const lastPos = this.loadedDriver.get_last_sibling(node.parentId);
+    const preferredPos = asPos(node.parentKey);
+    const finalKey =
+      lastPos === undefined || preferredPos > lastPos
+        ? preferredPos
+        : makePosition(lastPos);
+    await this.loadedDriver.set_child(
+      id,
+      finalKey !== node.parentKey ? { ...node, parentKey: finalKey } : node
+    );
+    return finalKey;
+  }
+
   /**
    * Tries to move a node to the given position under the same parent. If
    * a conflicting sibling node already exist at this position, it will use

packages/liveblocks-server/src/decoders/Op.ts

@@ -17,7 +17,14 @@
 
 import { OpCode } from "@liveblocks/core";
 import type { Decoder } from "decoders";
-import { constant, object, optional, string, taggedUnion } from "decoders";
+import {
+  constant,
+  object,
+  oneOf,
+  optional,
+  string,
+  taggedUnion,
+} from "decoders";
 
 import type {
   ClientWireOp,
@@ -35,6 +42,8 @@ import { jsonObjectYolo, jsonYolo } from "./jsonYolo";
 
 type HasOpId = { opId: string };
 
+const intent = oneOf(["set", "push"] as const);
+
 const updateObjectOp: Decoder<UpdateObjectOp & HasOpId> = object({
   type: constant(OpCode.UPDATE_OBJECT),
   opId: string,
@@ -49,7 +58,7 @@ const createObjectOp: Decoder<CreateObjectOp & HasOpId> = object({
   parentId: string,
   parentKey: string,
   data: jsonObjectYolo,
-  intent: optional(constant("set")),
+  intent: optional(intent),
   deletedId: optional(string),
 });
 
@@ -59,7 +68,7 @@ const createListOp: Decoder<CreateListOp & HasOpId> = object({
   id: string,
   parentId: string,
   parentKey: string,
-  intent: optional(constant("set")),
+  intent: optional(intent),
   deletedId: optional(string),
 });
 
@@ -69,7 +78,7 @@ const createMapOp: Decoder<CreateMapOp & HasOpId> = object({
   id: string,
   parentId: string,
   parentKey: string,
-  intent: optional(constant("set")),
+  intent: optional(intent),
   deletedId: optional(string),
 });
 
@@ -80,7 +89,7 @@ const createRegisterOp: Decoder<CreateRegisterOp & HasOpId> = object({
   parentId: string,
   parentKey: string,
   data: jsonYolo,
-  intent: optional(constant("set")),
+  intent: optional(intent),
   deletedId: optional(string),
 });
 

packages/liveblocks-server/src/interfaces/IStorageDriver.ts

@@ -175,6 +175,13 @@ export interface IStorageDriverNodeAPI {
    */
   get_next_sibling(parentId: string, pos: Pos): Pos | undefined;
 
+  /**
+   * Return the position of the last (rightmost) child under parentId, or
+   * undefined if the node has no children. Positions compare
+   * lexicographically.
+   */
+  get_last_sibling(parentId: string): Pos | undefined;
+
   /**
    * Insert a child node with the given id.
    *

packages/liveblocks-server/src/plugins/InMemoryDriver.ts

@@ -502,6 +502,18 @@ export class InMemoryDriver implements IStorageDriver {
       return nextPos;
     }
 
+    function get_last_sibling(parentId: string): Pos | undefined {
+      let lastPos: Pos | undefined;
+      // Find the largest position under this parent
+      for (const siblingKey of revNodes.keysAt(parentId)) {
+        const siblingPos = asPos(siblingKey);
+        if (lastPos === undefined || siblingPos > lastPos) {
+          lastPos = siblingPos;
+        }
+      }
+      return lastPos;
+    }
+
     /**
      * Inserts a node in the storage tree, deleting any nodes that already exist
      * under this key (including all of its children), if any.
@@ -694,6 +706,12 @@ export class InMemoryDriver implements IStorageDriver {
        */
       get_next_sibling,
 
+      /**
+       * Return the position of the last (rightmost) child under parentId, or
+       * undefined if the node has no children.
+       */
+      get_last_sibling,
+
       /**
        * Insert a child node with the given id.
        *

packages/liveblocks-server/test/plugins/_generateFullTestSuite.ts

@@ -438,7 +438,7 @@ export function createObjectOp(
   parentId: string,
   parentKey: string,
   data: Partial<JsonObject>,
-  intent?: "set",
+  intent?: "set" | "push",
   deletedId?: string,
   opId = nanoid()
 ): CreateObjectOp & HasOpId {
@@ -458,7 +458,7 @@ export function createListOp(
   id: string,
   parentId: string,
   parentKey: string,
-  intent?: "set",
+  intent?: "set" | "push",
   deletedId?: string,
   opId = nanoid()
 ): CreateListOp & HasOpId {
@@ -478,7 +478,7 @@ export function createRegisterOp(
   parentId: string,
   parentKey: string,
   data: Json,
-  intent?: "set",
+  intent?: "set" | "push",
   deletedId?: string,
   opId = nanoid()
 ): CreateRegisterOp & HasOpId {
@@ -498,7 +498,7 @@ export function createMapOp(
   id: string,
   parentId: string,
   parentKey: string,
-  intent?: "set",
+  intent?: "set" | "push",
   deletedId?: string,
   opId = nanoid()
 ): CreateMapOp & HasOpId {
@@ -1589,6 +1589,81 @@ export function generateFullTestSuite<TDriver extends IStorageDriver>(config: {
         expect(db.get_next_sibling("0:0", FIRST_POSITION)).toBe(undefined);
       }));
 
+    test("get_last_sibling: returns undefined for empty parent", () =>
+      runTest(async (driver) => {
+        await driver.DANGEROUSLY_reset_nodes(EMPTY_DOC);
+        const db = await driver.load_nodes_api(blackHole);
+
+        expect(db.get_last_sibling("root")).toBe(undefined);
+        expect(db.get_last_sibling("non-existing")).toBe(undefined);
+      }));
+
+    test("get_last_sibling: returns the rightmost position", () =>
+      runTest(async (driver) => {
+        await driver.DANGEROUSLY_reset_nodes(EMPTY_DOC);
+        const db = await driver.load_nodes_api(blackHole);
+
+        await db.set_child("0:0", {
+          type: CrdtType.LIST,
+          parentId: "root",
+          parentKey: "myList",
+        });
+        await db.set_child("0:1", {
+          type: CrdtType.REGISTER,
+          parentId: "0:0",
+          parentKey: FIRST_POSITION,
+          data: "item1",
+        });
+        expect(db.get_last_sibling("0:0")).toBe(FIRST_POSITION);
+
+        // Insert later positions out of order; the rightmost one wins
+        await db.set_child("0:3", {
+          type: CrdtType.REGISTER,
+          parentId: "0:0",
+          parentKey: THIRD_POSITION,
+          data: "item3",
+        });
+        await db.set_child("0:2", {
+          type: CrdtType.REGISTER,
+          parentId: "0:0",
+          parentKey: SECOND_POSITION,
+          data: "item2",
+        });
+        expect(db.get_last_sibling("0:0")).toBe(THIRD_POSITION);
+      }));
+
+    test("get_last_sibling: updates after delete", () =>
+      runTest(async (driver) => {
+        await driver.DANGEROUSLY_reset_nodes(EMPTY_DOC);
+        const db = await driver.load_nodes_api(blackHole);
+
+        await db.set_child("0:0", {
+          type: CrdtType.LIST,
+          parentId: "root",
+          parentKey: "myList",
+        });
+        await db.set_child("0:1", {
+          type: CrdtType.REGISTER,
+          parentId: "0:0",
+          parentKey: FIRST_POSITION,
+          data: "item1",
+        });
+        await db.set_child("0:2", {
+          type: CrdtType.REGISTER,
+          parentId: "0:0",
+          parentKey: SECOND_POSITION,
+          data: "item2",
+        });
+
+        expect(db.get_last_sibling("0:0")).toBe(SECOND_POSITION);
+
+        await db.delete_node("0:2");
+        expect(db.get_last_sibling("0:0")).toBe(FIRST_POSITION);
+
+        await db.delete_node("0:1");
+        expect(db.get_last_sibling("0:0")).toBe(undefined);
+      }));
+
     test("move: changes parentKey of node", () =>
       runTest(async (driver) => {
         await driver.DANGEROUSLY_reset_nodes(EMPTY_DOC);