Write new pending message format in PendingStateManager (#16136)

In PendingStateManager.getLocalState(), write the new message format instead of the old format See changes in #14462 [AB#3826](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/3826) Follow-up item for 2.0.0-internal.7.0.0: [AB#4763](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/4763)
microsoft · Jul 18, 2023 · 691bdea · 691bdea
1 parent 8673e3c
commit 691bdea
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 110 deletions.
diff --git a/packages/runtime/container-runtime/src/pendingStateManager.ts b/packages/runtime/container-runtime/src/pendingStateManager.ts
@@ -14,7 +14,7 @@ import { ContainerMessageType } from "./containerRuntime";
 import { pkgVersion } from "./packageVersion";
 
 /**
- * ! TODO: Remove this interface in "2.0.0-internal.7.0.0" once we only read IPendingMessageNew
+ * ! TODO: Remove this interface in "2.0.0-internal.7.0.0" once we only read IPendingMessageNew (AB#4763)
  */
 export interface IPendingMessageOld {
 	type: "message";
@@ -40,7 +40,7 @@ export interface IPendingMessageNew {
 }
 
 /**
- * ! TODO: Remove this type in "2.0.0-internal.7.0.0"
+ * ! TODO: Remove this type in "2.0.0-internal.7.0.0" (AB#4763)
  */
 export type IPendingState = IPendingMessageOld | IPendingMessageNew;
 
@@ -112,21 +112,18 @@ export class PendingStateManager implements IDisposable {
 		if (!this.pendingMessages.isEmpty()) {
 			return {
 				pendingStates: this.pendingMessages.toArray().map((message) => {
-					// ! TODO: Remove conversion to IPendingMessageOld in "2.0.0-internal.6.0.0" AB#3826
-					const content = JSON.parse(message.content);
+					let content = message.content;
+					const parsedContent = JSON.parse(content);
 					// IdAllocations need their localOpMetadata stashed in the contents
 					// of the op to correctly resume the session when processing stashed ops
-					if (content.type === ContainerMessageType.IdAllocation) {
-						content.contents.stashedState = message.localOpMetadata;
+					if (parsedContent.type === ContainerMessageType.IdAllocation) {
+						parsedContent.contents.stashedState = message.localOpMetadata;
+						content = JSON.stringify(parsedContent);
 					}
-					return {
-						...message,
-						messageType: content.type,
-						content: content.contents,
-						// delete localOpMetadata since it may not be serializable
-						// and will be regenerated by applyStashedOp()
-						localOpMetadata: undefined,
-					};
+
+					// delete localOpMetadata since it may not be serializable
+					// and will be regenerated by applyStashedOp()
+					return { ...message, content, localOpMetadata: undefined };
 				}),
 			};
 		}
@@ -138,7 +135,7 @@ export class PendingStateManager implements IDisposable {
 	) {
 		/**
 		 * Convert old local state format to the new format (IPendingMessageOld to IPendingMessageNew)
-		 * ! TODO: Remove this conversion in "2.0.0-internal.7.0.0"
+		 * ! TODO: Remove this conversion in "2.0.0-internal.7.0.0" (AB#4763)
 		 */
 		if (initialLocalState?.pendingStates) {
 			for (const initialState of initialLocalState.pendingStates) {

diff --git a/packages/runtime/container-runtime/src/test/containerRuntime.spec.ts b/packages/runtime/container-runtime/src/test/containerRuntime.spec.ts
@@ -34,7 +34,7 @@ import {
 	ContainerRuntime,
 	IContainerRuntimeOptions,
 } from "../containerRuntime";
-import { IPendingMessageOld, PendingStateManager } from "../pendingStateManager";
+import { IPendingMessageNew, PendingStateManager } from "../pendingStateManager";
 import { DataStores } from "../dataStores";
 
 describe("Runtime", () => {
@@ -1138,7 +1138,8 @@ describe("Runtime", () => {
 				assert.notStrictEqual(state, undefined, "expect pending local state");
 				assert.strictEqual(state?.pendingStates.length, 1, "expect 1 pending message");
 				assert.deepStrictEqual(
-					(state?.pendingStates[0] as IPendingMessageOld).content.contents,
+					JSON.parse((state?.pendingStates[0] as IPendingMessageNew).content).contents
+						.contents,
 					{
 						prop1: 1,
 					},

diff --git a/packages/runtime/container-runtime/src/test/pendingStateManager.spec.ts b/packages/runtime/container-runtime/src/test/pendingStateManager.spec.ts
@@ -292,7 +292,7 @@ describe("Pending State Manager", () => {
 		}
 
 		describe("Constructor conversion", () => {
-			// TODO: Remove in 2.0.0-internal.7.0.0 once only new format is read in constructor
+			// TODO: Remove in 2.0.0-internal.7.0.0 once only new format is read in constructor (AB#4763)
 			describe("deserialized content", () => {
 				it("Empty local state", () => {
 					{
@@ -367,90 +367,8 @@ describe("Pending State Manager", () => {
 			});
 		});
 
-		it("getLocalState writes new flush format", async () => {
-			const pendingStateManager = createPendingStateManager([
-				{
-					type: "message",
-					referenceSequenceNumber: 0,
-					opMetadata: { batch: true },
-					content: '{"type":"component"}',
-				},
-				{ type: "message", referenceSequenceNumber: 0, content: '{"type":"component"}' },
-				{
-					type: "message",
-					referenceSequenceNumber: 0,
-					opMetadata: { batch: false },
-					content: '{"type":"component"}',
-				},
-				{
-					type: "message",
-					referenceSequenceNumber: 0,
-					opMetadata: { batch: true },
-					content: '{"type":"component"}',
-				},
-				{
-					type: "message",
-					referenceSequenceNumber: 0,
-					opMetadata: { batch: false },
-					content: '{"type":"component"}',
-				},
-				{ type: "message", referenceSequenceNumber: 0, content: '{"type":"component"}' },
-			]);
-
-			await pendingStateManager.applyStashedOpsAt(0);
-
-			assert.deepStrictEqual(pendingStateManager.getLocalState().pendingStates, [
-				{
-					type: "message",
-					referenceSequenceNumber: 0,
-					localOpMetadata: undefined,
-					opMetadata: { batch: true },
-					content: undefined,
-					messageType: "component",
-				},
-				{
-					type: "message",
-					referenceSequenceNumber: 0,
-					localOpMetadata: undefined,
-					content: undefined,
-					messageType: "component",
-				},
-				{
-					type: "message",
-					referenceSequenceNumber: 0,
-					localOpMetadata: undefined,
-					opMetadata: { batch: false },
-					content: undefined,
-					messageType: "component",
-				},
-				{
-					type: "message",
-					referenceSequenceNumber: 0,
-					localOpMetadata: undefined,
-					opMetadata: { batch: true },
-					content: undefined,
-					messageType: "component",
-				},
-				{
-					type: "message",
-					referenceSequenceNumber: 0,
-					localOpMetadata: undefined,
-					opMetadata: { batch: false },
-					content: undefined,
-					messageType: "component",
-				},
-				{
-					type: "message",
-					referenceSequenceNumber: 0,
-					localOpMetadata: undefined,
-					content: undefined,
-					messageType: "component",
-				},
-			]);
-		});
-
-		// TODO: change to new format in "2.0.0-internal.6.0.0" (AB#3826)
-		it("getLocalState writes old message format", async () => {
+		// TODO: remove when we only read new format in "2.0.0-internal.7.0.0" (AB#4763)
+		it("getLocalState writes new message format", async () => {
 			const pendingStateManager = createPendingStateManager([
 				{ type: "message", messageType: "component" },
 				{ type: "message", content: '{"type":"component"}' },
@@ -466,27 +384,25 @@ describe("Pending State Manager", () => {
 			assert.deepStrictEqual(pendingStateManager.getLocalState().pendingStates, [
 				{
 					type: "message",
-					messageType: "component",
-					content: undefined,
+					content: '{"type":"component"}',
 					localOpMetadata: undefined,
+					messageType: "component", // This prop is still there, but it is not on the IPendingMessageNew interface
 				},
 				{
 					type: "message",
-					messageType: "component",
-					content: undefined,
+					content: '{"type":"component"}',
 					localOpMetadata: undefined,
 				},
 				{
 					type: "message",
-					messageType: "component",
-					content: { prop1: "value" },
+					content: '{"type": "component", "contents": {"prop1": "value"}}',
 					localOpMetadata: undefined,
 				},
 				{
 					type: "message",
-					messageType: "component",
-					content: { prop1: "value" },
+					content: '{"type":"component","contents":{"prop1":"value"}}',
 					localOpMetadata: undefined,
+					messageType: "component", // This prop is still there, but it is not on the IPendingMessageNew interface
 				},
 			]);
 		});