diff --git a/packages/dds/tree/src/core/rebase/types.ts b/packages/dds/tree/src/core/rebase/types.ts index 3092c33b8e3d..32a5c15d7379 100644 --- a/packages/dds/tree/src/core/rebase/types.ts +++ b/packages/dds/tree/src/core/rebase/types.ts @@ -210,7 +210,6 @@ export interface LocalChangeMetadata extends CommitMetadata { * Returns a serializable object that encodes the change. * @remarks This is only available for local changes. * This change object can be {@link TreeBranchAlpha.applyChange | applied to another branch} in the same state as the one which generated it. - * The change object must be applied to a SharedTree with the same IdCompressor session ID as it was created from. * @privateRemarks * This is a `SerializedChange` from treeCheckout.ts. */ diff --git a/packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts b/packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts index aad0fcf42e1c..13c2de0d6480 100644 --- a/packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts +++ b/packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts @@ -204,6 +204,7 @@ export class DefaultEditBuilder implements ChangeFamilyEditor, IDefaultEditBuild fieldKinds, changeReceiver, codecOptions, + mintRevisionTag, ); } @@ -214,6 +215,10 @@ export class DefaultEditBuilder implements ChangeFamilyEditor, IDefaultEditBuild this.modularBuilder.exitTransaction(); } + public applyExternalChange(change: DefaultChangeset): void { + this.modularBuilder.applyExternalChange(change); + } + public addNodeExistsConstraint(path: UpPath): void { this.modularBuilder.addNodeExistsConstraint(path, this.mintRevisionTag()); } diff --git a/packages/dds/tree/src/feature-libraries/modular-schema/defaultRevisionReplacer.ts b/packages/dds/tree/src/feature-libraries/modular-schema/defaultRevisionReplacer.ts index 956b03844b2f..513da215777d 100644 --- a/packages/dds/tree/src/feature-libraries/modular-schema/defaultRevisionReplacer.ts +++ b/packages/dds/tree/src/feature-libraries/modular-schema/defaultRevisionReplacer.ts @@ -18,6 +18,7 @@ import { brand, brandConst, newIntegerRangeMap, + type IdAllocator, type RangeMap, type Mutable, } from "../../util/index.js"; @@ -40,10 +41,25 @@ export class DefaultRevisionReplacer implements RevisionReplacer { */ private maxSeen: ChangesetLocalId = brandConst(-1)(); + /** + * @param updatedRevision - The revision to assign to all replaced IDs. + * @param obsoleteRevisions - The set of revisions that should be replaced. + * @param idAllocator - If provided, IDs already allocated by this allocator will be reserved (avoided by the replacer), + * and the allocator will be updated to account for any new IDs allocated during replacement. + */ public constructor( public readonly updatedRevision: RevisionTag, private readonly obsoleteRevisions: Set, - ) {} + private readonly idAllocator?: IdAllocator, + ) { + if (idAllocator !== undefined) { + const reservedIdCount = idAllocator.getMaxId() + 1; + if (reservedIdCount > 0) { + this.localIds.set(brand(0), reservedIdCount, true); + this.maxSeen = brand(reservedIdCount - 1); + } + } + } public isObsolete(revision: RevisionTag | undefined): boolean { return this.obsoleteRevisions.has(revision); @@ -89,6 +105,12 @@ export class DefaultRevisionReplacer implements RevisionReplacer { remainderStart = offsetChangeAtomId(remainderStart, prior.length); remainderCount -= prior.length; } + + // Keep the allocator in sync with any new IDs we've allocated + if (this.idAllocator !== undefined && this.maxSeen >= this.idAllocator.getMaxId() + 1) { + this.idAllocator.allocate(this.maxSeen - this.idAllocator.getMaxId()); + } + return updated; } return id; diff --git a/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts b/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts index b8e959f7e6bb..0b48418c7a58 100644 --- a/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts +++ b/packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts @@ -81,6 +81,7 @@ import { getFirstFromCrossFieldMap, setInCrossFieldMap, } from "./crossFieldQueries.js"; +import { DefaultRevisionReplacer } from "./defaultRevisionReplacer.js"; import { type FieldChangeHandler, NodeAttachState, @@ -2686,6 +2687,7 @@ export class ModularEditBuilder extends EditBuilder { private readonly fieldKinds: ReadonlyMap, changeReceiver: (change: TaggedChange) => void, codecOptions: CodecWriteOptions, + private readonly mintRevisionTag?: () => RevisionTag, ) { super(family, changeReceiver); this.idAllocator = idAllocatorFromMaxId(); @@ -2707,6 +2709,27 @@ export class ModularEditBuilder extends EditBuilder { } } + /** + * Apply a changeset that originated from a different editor. + * @remarks + * The revision for the change will be changed to a new revision (using `mintRevisionTag`) before application. + * The change will also have its local IDs replaced to avoid collisions with any IDs produced by this editor. + */ + public applyExternalChange(change: ModularChangeset): void { + assert( + this.mintRevisionTag !== undefined, + "mintRevisionTag is required to apply external changes", + ); + const revision = this.mintRevisionTag(); + const replacer = new DefaultRevisionReplacer( + revision, + this.changeFamily.rebaser.getRevisions(change), + this.idAllocator, + ); + const newChange = this.changeFamily.rebaser.changeRevision(change, replacer); + this.applyChange(tagChange(newChange, revision)); + } + /** * Builds a new tree to use in an edit. * diff --git a/packages/dds/tree/src/shared-tree/treeCheckout.ts b/packages/dds/tree/src/shared-tree/treeCheckout.ts index de7aa8308721..d93fd05c2c87 100644 --- a/packages/dds/tree/src/shared-tree/treeCheckout.ts +++ b/packages/dds/tree/src/shared-tree/treeCheckout.ts @@ -63,6 +63,7 @@ import { type TaggedChange, } from "../core/index.js"; import { + DefaultRevisionReplacer, type FieldBatchCodec, type TreeCompressionStrategy, allowsRepoSuperset, @@ -711,13 +712,22 @@ export class TreeCheckout implements ITreeCheckoutFork { kind, isLocal: true, getChange: () => { + // Give all serialized changes a revision of "root" by convention. + // This is not necessary for correctness - changes are "copied" when later applied and will be given a different revision at that time. + const replacer = new DefaultRevisionReplacer( + "root", + this.changeFamily.rebaser.getRevisions(change), + ); + const root = this.changeFamily.rebaser.changeRevision(change, replacer); const context: ChangeEncodingContext = { idCompressor: this.idCompressor, originatorId: this.idCompressor.localSessionId, - revision, + revision: "root", + // By not passing the schema, we avoid compressing identifiers in identifier fields. + // This ensures the change is self-contained - it does not rely on the ID compressor at application time to know about IDs from the compressor at encode time. + schema: undefined, }; - const encodedChange = this.changeFamily.codecs.resolve(4).encode(change, context); - + const encodedChange = this.changeFamily.codecs.resolve(4).encode(root, context); assert( commit.parent !== undefined, 0xca4 /* Expected applied commit to be parented */, @@ -775,19 +785,25 @@ export class TreeCheckout implements ITreeCheckoutFork { throw new UsageError(`Cannot apply change. Invalid serialized change format.`); } const { revision, originatorId, change } = serializedChange; - if (originatorId !== this.idCompressor.localSessionId) { - throw new UsageError( - `Cannot apply change. A serialized changed must be applied to the same SharedTree as it was created from.`, - ); - } + assert(revision === "root", `Malformed serialized change, revision should be "root"`); const context: ChangeEncodingContext = { idCompressor: this.idCompressor, - originatorId: this.idCompressor.localSessionId, + originatorId, revision, }; const decodedChange = this.changeFamily.codecs.resolve(4).decode(change, context); - // Apply the change to the branch, but _not_ the `activeBranch` - we do not support squashing serialized commits in a transaction. - this.#transaction.branch.apply(tagChange(decodedChange, revision)); + + // Extract the inner data change from the SharedTreeChange envelope. + // Serialized changes are always single data changes (not schema changes). + const innerChange = decodedChange.changes[0]; + assert( + decodedChange.changes.length === 1 && innerChange?.type === "data", + 0x1b2 /* Expected a single data change in serialized change */, + ); + + // Delegate to the editor, which will replace the revision, shift local IDs to avoid + // collisions with other changes in the same transaction, and apply the change. + this.#transaction.activeBranchEditor.applyExternalChange(innerChange.innerChange); } // Revision is the revision of the commit, if any, which caused this change. diff --git a/packages/dds/tree/src/simple-tree/api/tree.ts b/packages/dds/tree/src/simple-tree/api/tree.ts index 0f394c2a81a0..ec2fe1d17c79 100644 --- a/packages/dds/tree/src/simple-tree/api/tree.ts +++ b/packages/dds/tree/src/simple-tree/api/tree.ts @@ -332,12 +332,11 @@ export interface TreeBranchAlpha extends TreeBranch, TreeContextAlpha { * Apply a serialized change to this branch. * @param change - the change to apply. * Changes are acquired via `getChange` in a branch's {@link TreeBranchEvents.changed | "changed"} event. - * @remarks Changes may only be applied to a SharedTree with the same IdCompressor instance and branch state from which they were generated. - * They may be created by one branch and applied to another, but only if both branches share the same history at the time of creation and application. + * @remarks Changes may only be applied to a SharedTree with the same branch state from which they were generated. + * They may be created by one branch and applied to another, but only if both branches share the same history at the time of creation and application, respectively. + * The two branches may use different IdCompressor instances (e.g. across different runtimes or processes). * - * @privateRemarks - * TODO: This method will support applying changes from different IdCompressor instances as long as they have the same local session ID. - * Update the tests and docs to match when that is done. + * Applying changes is not idempotent, that is, the same serialized change applied twice will have two effects - it will not be deduplicated. */ applyChange(change: JsonCompatibleReadOnly): void; } diff --git a/packages/dds/tree/src/test/simple-tree/api/tree.spec.ts b/packages/dds/tree/src/test/simple-tree/api/tree.spec.ts index 405d6eb393ea..de54ca118a76 100644 --- a/packages/dds/tree/src/test/simple-tree/api/tree.spec.ts +++ b/packages/dds/tree/src/test/simple-tree/api/tree.spec.ts @@ -309,7 +309,7 @@ describe("simple-tree tree", () => { assert.equal(viewA.root, 4); }); - it("fail to apply to a branch in another session", () => { + it("can be applied to a view with a different session", () => { const config = new TreeViewConfiguration({ schema: schema.number }); const viewA = getView(config); viewA.initialize(3); @@ -323,10 +323,9 @@ describe("simple-tree tree", () => { }); viewA.root = 4; - const c = change ?? assert.fail("change not captured"); - assert.throws(() => { - viewB.applyChange(c); - }, /cannot apply change.*same sharedtree/i); + assert(change !== undefined); + viewB.applyChange(change); + assert.equal(viewB.root, 4); }); it("error if malformed", () => { @@ -383,7 +382,33 @@ describe("simple-tree tree", () => { assert.equal(viewA.root, 4); }); - it("apply before transactions", () => { + it("applying the same change twice is not idempotent", () => { + const sf = new SchemaFactory("test"); + class List extends sf.array("List", sf.number) {} + const config = new TreeViewConfiguration({ schema: List }); + const viewA = getView(config); + viewA.initialize([1, 2, 3]); + const viewB = viewA.fork(); + + let change: JsonCompatibleReadOnly | undefined; + viewB.events.on("changed", (metadata) => { + assert(metadata.isLocal); + change = metadata.getChange(); + }); + + // Insert a node on viewB + viewB.root.insertAtEnd(4); + assert(change !== undefined); + + // Apply the same serialized change twice to viewA + viewA.applyChange(change); + viewA.applyChange(change); + + // Each application should produce a distinct effect - the node is inserted twice + assert.deepEqual([...viewA.root], [1, 2, 3, 4, 4]); + }); + + it("non-transaction change can be applied inside a transaction", () => { const config = new TreeViewConfiguration({ schema: schema.number }); const viewA = getView(config); viewA.initialize(3); @@ -395,16 +420,235 @@ describe("simple-tree tree", () => { change = metadata.getChange(); }); + // Make a non-transaction change on viewB viewB.root = 4; + assert(change !== undefined); + + // Apply that non-transaction change inside a transaction on viewA + const capturedChange = change; viewA.runTransaction(() => { - viewA.root = 5; - assert(change !== undefined); - // Even though the serialized change (= 4) is applied _after_ the transaction change (= 5), - // it is considered a change external to the transaction and so will be applied before the transaction changes, - // as is the general policy for external changes applied during a transaction. - viewA.applyChange(change); + viewA.applyChange(capturedChange); + }); + assert.equal(viewA.root, 4); + }); + + it("multiple non-transaction changes can be applied together in a transaction", () => { + const sf = new SchemaFactory("test"); + class List extends sf.array("List", sf.number) {} + const config = new TreeViewConfiguration({ schema: List }); + const viewA = getView(config); + viewA.initialize([1, 2, 3]); + const viewB = viewA.fork(); + + const changes: JsonCompatibleReadOnly[] = []; + viewB.events.on("changed", (metadata) => { + assert(metadata.isLocal); + changes.push(metadata.getChange()); + }); + + // Make two separate non-transaction changes on viewB + viewB.root.insertAtEnd(4); + viewB.root.insertAtEnd(5); + assert.equal(changes.length, 2); + + // Apply both non-transaction changes together inside a single transaction on viewA + viewA.runTransaction(() => { + viewA.applyChange(changes[0]); + viewA.applyChange(changes[1]); + }); + assert.deepEqual([...viewA.root], [1, 2, 3, 4, 5]); + }); + + it("changes from different branches can be applied together in a transaction", () => { + const sf = new SchemaFactory("test"); + class List extends sf.array("List", sf.number) {} + const config = new TreeViewConfiguration({ schema: List }); + const viewA = getView(config); + viewA.initialize([1, 2, 3]); + + // Capture changeA from viewA + let changeA: JsonCompatibleReadOnly | undefined; + viewA.events.on("changed", (metadata) => { + if (metadata.isLocal) { + changeA = metadata.getChange(); + } + }); + viewA.root.insertAtEnd(4); + assert(changeA !== undefined); + + // Fork viewA to create viewB, then capture changeB from viewB + const viewB = viewA.fork(); + let changeB: JsonCompatibleReadOnly | undefined; + viewB.events.on("changed", (metadata) => { + if (metadata.isLocal) { + changeB = metadata.getChange(); + } + }); + viewB.root.insertAtEnd(5); + assert(changeB !== undefined); + + // Apply both changes in a transaction on a third branch (forked from viewA's state after changeA) + const viewC = viewA.fork(); + const capturedA = changeA; + const capturedB = changeB; + viewC.runTransaction(() => { + viewC.applyChange(capturedA); + viewC.applyChange(capturedB); + }); + assert.deepEqual([...viewC.root], [1, 2, 3, 4, 5, 4]); + }); + + it("serialized changes can be mixed with editor edits in a transaction", () => { + const sf = new SchemaFactory("test"); + class List extends sf.array("List", sf.number) {} + const config = new TreeViewConfiguration({ schema: List }); + const viewA = getView(config); + viewA.initialize([1, 2, 3]); + + // Capture a serialized change from a fork + const viewB = viewA.fork(); + let serializedChange: JsonCompatibleReadOnly | undefined; + viewB.events.on("changed", (metadata) => { + if (metadata.isLocal) { + serializedChange = metadata.getChange(); + } + }); + viewB.root.insertAtEnd(99); + assert(serializedChange !== undefined); + + // In a transaction on viewA, mix direct edits with the serialized change + const captured = serializedChange; + viewA.runTransaction(() => { + viewA.root.insertAtEnd(10); + viewA.applyChange(captured); + viewA.root.insertAtEnd(20); + }); + const result = [...viewA.root]; + assert.equal(result.length, 6, "all three inserts should be present"); + assert(result.includes(10), "editor insert 10 should be present"); + assert(result.includes(20), "editor insert 20 should be present"); + assert(result.includes(99), "serialized insert 99 should be present"); + }); + + it("changes from different branches can overwrite a value field in a transaction", () => { + const config = new TreeViewConfiguration({ schema: schema.number }); + const viewA = getView(config); + viewA.initialize(3); + + let changeA: JsonCompatibleReadOnly | undefined; + viewA.events.on("changed", (metadata) => { + if (metadata.isLocal) { + changeA = metadata.getChange(); + } + }); + viewA.root = 4; + assert(changeA !== undefined); + + const viewB = viewA.fork(); + let changeB: JsonCompatibleReadOnly | undefined; + viewB.events.on("changed", (metadata) => { + if (metadata.isLocal) { + changeB = metadata.getChange(); + } + }); + viewB.root = 5; + assert(changeB !== undefined); + + const viewC = viewA.fork(); + const capturedA = changeA; + const capturedB = changeB; + viewC.runTransaction(() => { + viewC.applyChange(capturedA); + viewC.applyChange(capturedB); + }); + // changeA sets 3→4, changeB sets 4→5, so result should be 5 + assert.equal(viewC.root, 5); + }); + + it("applied change is rolled back when transaction is aborted", () => { + const config = new TreeViewConfiguration({ schema: schema.number }); + const viewA = getView(config); + viewA.initialize(3); + const viewB = viewA.fork(); + + let change: JsonCompatibleReadOnly | undefined; + viewB.events.on("changed", (metadata) => { + assert(metadata.isLocal); + change = metadata.getChange(); + }); + + viewB.root = 4; + assert(change !== undefined); + + const capturedChange = change; + Tree.runTransaction(viewA, () => { + viewA.applyChange(capturedChange); + assert.equal(viewA.root, 4); + return Tree.runTransaction.rollback; + }); + // The serialized change should be rolled back along with the transaction + assert.equal(viewA.root, 3); + }); + + it("can apply a change with an identifier field build to a view with a different id compressor", () => { + const sf = new SchemaFactory("test"); + class HasId extends sf.object("HasId", { id: sf.identifier }) {} + const config = new TreeViewConfiguration({ + schema: SchemaFactory.optional(HasId), + }); + + // Two independent views get different id compressors with different sessions + const viewA = getView(config); + viewA.initialize(undefined); + const viewB = getView(config); + viewB.initialize(undefined); + + let change: JsonCompatibleReadOnly | undefined; + viewA.events.on("changed", (metadata) => { + if (metadata.isLocal) { + change = metadata.getChange(); + } }); - assert.equal(viewA.root, 5); + + // Insert a node with a default identifier on viewA + viewA.root = new HasId({ id: undefined }); + assert(change !== undefined); + const identifierOnA = viewA.root.id; + + // Apply the serialized change to viewB (different compressor instance and session) + viewB.applyChange(change); + assert(viewB.root !== undefined); + assert.equal(viewB.root.id, identifierOnA); + }); + + it("each application gets a unique revision", () => { + const config = new TreeViewConfiguration({ schema: schema.number }); + const viewA = getView(config); + viewA.initialize(3); + const viewB = viewA.fork(); + + let change: JsonCompatibleReadOnly | undefined; + viewB.events.on("changed", (metadata) => { + assert(metadata.isLocal); + change = metadata.getChange(); + }); + + viewB.root = 4; + assert(change !== undefined); + + // Track the changes applied to viewA + const appliedChanges: JsonCompatibleReadOnly[] = []; + viewA.events.on("changed", (metadata) => { + if (metadata.isLocal) { + appliedChanges.push(metadata.getChange()); + } + }); + + viewA.applyChange(change); + viewA.applyChange(change); + + // Each application should produce a separate change event + assert.equal(appliedChanges.length, 2); }); }); });