Fix handling of SharedDirectory ops on subdirs created on a detached …

…data store (#16583)

## Description

SharedDirectory was using -1 all the time for locally created
subdirectories.
This leads to bugs when considering the logic in
isMessageForCurrentInstanceOfSubDirectory when the SharedDirectory is
additionally detached: no ack op will ever be received for that subdir
(since an op is never sent), but clients who load using the attachment
summary may start sending ops which refers to the same subdirectory.

merge-tree has a similar problem which is resolved by using -1 or 0
depending on the attach state of the DDS, see UnassignedSequenceNumber
vs UniversalSequenceNumber. The fix here brings SharedDirectory handling
closer to merge-tree's.

Cherry-pick of #16578

packages/dds/map/src/directory.ts

@@ -1250,7 +1250,7 @@ class SubDirectory extends TypedEventEmitter<IDirectoryEvents> implements IDirec
 		const isNew = this.createSubDirectoryCore(
 			subdirName,
 			true,
-			-1,
+			this.getLocalSeq(),
 			this.runtime.clientId ?? "detached",
 		);
 		const subDir = this._subdirectories.get(subdirName);
@@ -1274,6 +1274,17 @@ class SubDirectory extends TypedEventEmitter<IDirectoryEvents> implements IDirec
 		return subDir;
 	}
 
+	/**
+	 * @returns A sequenceNumber which should be used for local changes.
+	 * @remarks - While detached, 0 is used rather than -1 to represent a change which should be universally known (as opposed to known
+	 * only by the local client). This ensures that if the directory is later attached, none of its data needs to be updated (the values
+	 * last set while detached will now be known to any new client, until they are changed).
+	 * TODO: Convert these conventions to named constants. The semantics used here match those for merge-tree.
+	 */
+	private getLocalSeq(): number {
+		return this.directory.isAttached() ? -1 : 0;
+	}
+
 	/**
 	 * {@inheritDoc IDirectory.getSubDirectory}
 	 */
@@ -1661,7 +1672,12 @@ class SubDirectory extends TypedEventEmitter<IDirectoryEvents> implements IDirec
 	): ICreateSubDirLocalOpMetadata {
 		this.throwIfDisposed();
 		// Create the sub directory locally first.
-		this.createSubDirectoryCore(op.subdirName, true, -1, this.runtime.clientId ?? "detached");
+		this.createSubDirectoryCore(
+			op.subdirName,
+			true,
+			this.getLocalSeq(),
+			this.runtime.clientId ?? "detached",
+		);
 		this.updatePendingSubDirMessageCount(op);
 
 		const localOpMetadata: ICreateSubDirLocalOpMetadata = {

packages/dds/map/src/test/mocha/directory.spec.ts

@@ -18,6 +18,7 @@ import {
 import { MapFactory } from "../../map";
 import { DirectoryFactory, IDirectoryNewStorageFormat, SharedDirectory } from "../../directory";
 import { IDirectory, IDirectoryValueChanged, ISharedMap } from "../../interfaces";
+import { assertEquivalentDirectories } from "./directoryEquivalenceUtils";
 
 /* eslint-disable @typescript-eslint/no-unsafe-member-access */
 
@@ -444,7 +445,7 @@ describe("Directory", () => {
 
 				const subMapHandleUrl = subMap.handle.absolutePath;
 				const serialized = serialize(directory);
-				const expected = `{"ci":{"csn":0,"ccIds":[]},"storage":{"first":{"type":"Plain","value":"second"},"third":{"type":"Plain","value":"fourth"},"fifth":{"type":"Plain","value":"sixth"},"object":{"type":"Plain","value":{"type":"__fluid_handle__","url":"${subMapHandleUrl}"}}},"subdirectories":{"nested":{"ci":{"csn":-1,"ccIds":["${dataStoreRuntime.clientId}"]},"storage":{"deepKey1":{"type":"Plain","value":"deepValue1"}},"subdirectories":{"nested2":{"ci":{"csn":-1,"ccIds":["${dataStoreRuntime.clientId}"]},"subdirectories":{"nested3":{"ci":{"csn":-1,"ccIds":["${dataStoreRuntime.clientId}"]},"storage":{"deepKey2":{"type":"Plain","value":"deepValue2"}}}}}}}}}`;
+				const expected = `{"ci":{"csn":0,"ccIds":[]},"storage":{"first":{"type":"Plain","value":"second"},"third":{"type":"Plain","value":"fourth"},"fifth":{"type":"Plain","value":"sixth"},"object":{"type":"Plain","value":{"type":"__fluid_handle__","url":"${subMapHandleUrl}"}}},"subdirectories":{"nested":{"ci":{"csn":0,"ccIds":["${dataStoreRuntime.clientId}"]},"storage":{"deepKey1":{"type":"Plain","value":"deepValue1"}},"subdirectories":{"nested2":{"ci":{"csn":0,"ccIds":["${dataStoreRuntime.clientId}"]},"subdirectories":{"nested3":{"ci":{"csn":0,"ccIds":["${dataStoreRuntime.clientId}"]},"storage":{"deepKey2":{"type":"Plain","value":"deepValue2"}}}}}}}}}`;
 				assert.equal(serialized, expected);
 			});
 
@@ -466,7 +467,7 @@ describe("Directory", () => {
 
 				const subMapHandleUrl = subMap.handle.absolutePath;
 				const serialized = serialize(directory);
-				const expected = `{"ci":{"csn":0,"ccIds":[]},"storage":{"first":{"type":"Plain","value":"second"},"third":{"type":"Plain","value":"fourth"},"fifth":{"type":"Plain"},"object":{"type":"Plain","value":{"type":"__fluid_handle__","url":"${subMapHandleUrl}"}}},"subdirectories":{"nested":{"ci":{"csn":-1,"ccIds":["${dataStoreRuntime.clientId}"]},"storage":{"deepKey1":{"type":"Plain","value":"deepValue1"},"deepKeyUndefined":{"type":"Plain"}},"subdirectories":{"nested2":{"ci":{"csn":-1,"ccIds":["${dataStoreRuntime.clientId}"]},"subdirectories":{"nested3":{"ci":{"csn":-1,"ccIds":["${dataStoreRuntime.clientId}"]},"storage":{"deepKey2":{"type":"Plain","value":"deepValue2"}}}}}}}}}`;
+				const expected = `{"ci":{"csn":0,"ccIds":[]},"storage":{"first":{"type":"Plain","value":"second"},"third":{"type":"Plain","value":"fourth"},"fifth":{"type":"Plain"},"object":{"type":"Plain","value":{"type":"__fluid_handle__","url":"${subMapHandleUrl}"}}},"subdirectories":{"nested":{"ci":{"csn":0,"ccIds":["${dataStoreRuntime.clientId}"]},"storage":{"deepKey1":{"type":"Plain","value":"deepValue1"},"deepKeyUndefined":{"type":"Plain"}},"subdirectories":{"nested2":{"ci":{"csn":0,"ccIds":["${dataStoreRuntime.clientId}"]},"subdirectories":{"nested3":{"ci":{"csn":0,"ccIds":["${dataStoreRuntime.clientId}"]},"storage":{"deepKey2":{"type":"Plain","value":"deepValue2"}}}}}}}}}`;
 				assert.equal(serialized, expected);
 			});
 		});
@@ -859,7 +860,7 @@ describe("Directory", () => {
 				);
 			});
 
-			it("should correctly process a sub directory operation sent in local state", async () => {
+			it("should correctly process subdirectory operations sent in local state", async () => {
 				// Set the data store runtime to local.
 				dataStoreRuntime.local = true;
 
@@ -899,10 +900,15 @@ describe("Directory", () => {
 					directory.getSubDirectory(subDirName),
 					"The first directory does not have sub directory",
 				);
-				assert.ok(
-					directory2.getSubDirectory(subDirName),
-					"The second directory does not have sub directory",
-				);
+				const subDir2 = directory2.getSubDirectory(subDirName);
+
+				assert.ok(subDir2, "The second directory does not have sub directory");
+
+				subDir2.set("foo", "bar");
+
+				containerRuntimeFactory.processAllMessages();
+
+				assertEquivalentDirectories(directory, directory2);
 
 				// Delete the subdirectory in the second SharedDirectory.
 				directory2.deleteSubDirectory(subDirName);