Skip to content

Commit

Permalink
ContainerRuntime: Remove the contents property of batchBegin/batchEnd…
Browse files Browse the repository at this point in the history
…'s "op" event arg (microsoft#22791)

## Description

Fixes
[AB#19784](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/19784)

ContainerRuntime's 'batchBegin'/'batchEnd' events: Removing the
`contents` property on event arg `op`

The 'batchBegin'/'batchEnd' events on ContainerRuntime indicate when a
batch is beginning/finishing being processed.
The `contents` property on there is not useful or relevant when
reasoning over incoming changes at the batch level.
So it has been removed from the `op` event arg.

## Breaking Changes

Yes this is a breaking change.  See changeset.

---------

Co-authored-by: Kian Thompson <[email protected]>
  • Loading branch information
markfields and kian-thompson authored Nov 6, 2024
1 parent 85d0e82 commit d252af5
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 37 deletions.
12 changes: 12 additions & 0 deletions .changeset/fruity-sites-boil.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
"@fluidframework/runtime-definitions": minor
---
---
"section": other
---

ContainerRuntime's 'batchBegin'/'batchEnd' events: Removing the `contents` property on event arg `op`

The 'batchBegin'/'batchEnd' events on ContainerRuntime indicate when a batch is beginning/finishing being processed.
The `contents` property on there is not useful or relevant when reasoning over incoming changes at the batch level.
So it has been removed from the `op` event arg.
21 changes: 10 additions & 11 deletions packages/runtime/container-runtime/src/containerRuntime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2819,17 +2819,6 @@ export class ContainerRuntime
: false /* groupedBatch */,
);
} else {
if (!runtimeBatch) {
// The DeltaManager used to do this, but doesn't anymore as of Loader v2.4
// Anyone listening to our "op" event would expect the contents to be parsed per this same logic
if (
typeof messageCopy.contents === "string" &&
messageCopy.contents !== "" &&
messageCopy.type !== MessageType.ClientLeave
) {
messageCopy.contents = JSON.parse(messageCopy.contents);
}
}
this.processInboundMessages(
[{ message: messageCopy, localOpMetadata: undefined }],
{ batchStart: true, batchEnd: true }, // Single message
Expand Down Expand Up @@ -2996,6 +2985,16 @@ export class ContainerRuntime
this.updateDocumentDirtyState(false);
}

// The DeltaManager used to do this, but doesn't anymore as of Loader v2.4
// Anyone listening to our "op" event would expect the contents to be parsed per this same logic
if (
typeof message.contents === "string" &&
message.contents !== "" &&
message.type !== MessageType.ClientLeave
) {
message.contents = JSON.parse(message.contents);
}

this.emit("op", message, false /* runtimeMessage */);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,15 +94,8 @@ export interface IContainerRuntimeBase extends IEventProvider<IContainerRuntimeB

// @alpha @sealed (undocumented)
export interface IContainerRuntimeBaseEvents extends IEvent {
// (undocumented)
(event: "batchBegin", listener: (op: Omit<ISequencedDocumentMessage, "contents"> & {
contents: unknown;
}) => void): any;
// (undocumented)
(event: "batchEnd", listener: (error: any, op: Omit<ISequencedDocumentMessage, "contents"> & {
contents: unknown;
}) => void): any;
// (undocumented)
(event: "batchBegin", listener: (op: Omit<ISequencedDocumentMessage, "contents">) => void): any;
(event: "batchEnd", listener: (error: any, op: Omit<ISequencedDocumentMessage, "contents">) => void): any;
(event: "op", listener: (op: ISequencedDocumentMessage, runtimeMessage?: boolean) => void): any;
// (undocumented)
(event: "signal", listener: (message: IInboundSignalMessage, local: boolean) => void): any;
Expand Down
30 changes: 13 additions & 17 deletions packages/runtime/runtime-definitions/src/dataStoreContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,28 +122,24 @@ export type VisibilityState = (typeof VisibilityState)[keyof typeof VisibilitySt
* @sealed
*/
export interface IContainerRuntimeBaseEvents extends IEvent {
(
event: "batchBegin",
listener: (
op: Omit<ISequencedDocumentMessage, "contents"> & {
/** @deprecated Use the "op" event to get details about the message contents */
contents: unknown;
},
) => void,
);
/**
* Indicates the beginning of an incoming batch of ops
* @param op - The first op in the batch. Can be inspected to learn about the sequence numbers relevant for this batch.
*/
(event: "batchBegin", listener: (op: Omit<ISequencedDocumentMessage, "contents">) => void);
/**
* Indicates the end of an incoming batch of ops
* @param error - If an error occurred while processing the batch, it is provided here.
* @param op - The last op in the batch. Can be inspected to learn about the sequence numbers relevant for this batch.
*/
(
event: "batchEnd",
listener: (
error: any,
op: Omit<ISequencedDocumentMessage, "contents"> & {
/** @deprecated Use the "op" event to get details about the message contents */
contents: unknown;
},
) => void,
listener: (error: any, op: Omit<ISequencedDocumentMessage, "contents">) => void,
);
/**
* Indicates that an incoming op has been processed.
* @param runtimeMessage - tells if op is runtime op. If it is, it was unpacked, i.e. its type and content
* represent internal container runtime type / content.
* represent internal container runtime type / content. i.e. A grouped batch of N ops will result in N "op" events
*/
(event: "op", listener: (op: ISequencedDocumentMessage, runtimeMessage?: boolean) => void);
(event: "signal", listener: (message: IInboundSignalMessage, local: boolean) => void);
Expand Down

0 comments on commit d252af5

Please sign in to comment.