Automation: main-next integrate

.changeset/brown-roses-drive.md

@@ -0,0 +1,11 @@
+---
+"@fluidframework/sequence": minor
+---
+
+Incorporate change and change properties into the same method, and update the change signature to match the new add signature.
+
+Instead of having 2 separate methods to change the endpoints of an interval and the properties, combine them into a single method that will change the endpoints, properties, or both, depending on the arguments passed in. The signature of this combined method is now updated as well, so the new usage of the change method is to call it with an interval id as the first parameter and an object containing the desired portions of the interval to update as the second parameter. For the object parameter, the endpoints field should be an object containing the new start and end values for the interval, and the properties field should be an object containing the new properties for the interval. Either the endpoints field or the properties field can be omitted, and if neither are present, change will return undefined. The new usage of the change method is as follows:
+
+Change interval endpoints: change(id, { endpoints: { start: 1, end: 4 } });
+Change interval properties: change(id { props: { a: 1 } });
+Change interval endpoints and properties: change(id, { endpoints: { start: 1, end: 4 }, props: { a: 1 } });

packages/dds/sequence/api-report/sequence.api.md

@@ -149,7 +149,14 @@ export interface IIntervalCollection<TInterval extends ISerializableInterval> ex
     // (undocumented)
     readonly attached: boolean;
     attachIndex(index: IntervalIndex<TInterval>): void;
+    // @deprecated
     change(id: string, start: SequencePlace, end: SequencePlace): TInterval | undefined;
+    change(id: string, { start, end, props }: {
+        start?: SequencePlace;
+        end?: SequencePlace;
+        props?: PropertySet;
+    }): TInterval | undefined;
+    // @deprecated
     changeProperties(id: string, props: PropertySet): void;
     // (undocumented)
     CreateBackwardIteratorWithEndPosition(endPosition: number): Iterator<TInterval>;

packages/dds/sequence/src/intervalCollection.ts

@@ -865,19 +865,33 @@ export interface IIntervalCollection<TInterval extends ISerializableInterval>
 	removeIntervalById(id: string): TInterval | undefined;
 	/**
 	 * Changes the properties on an existing interval.
+	 * @deprecated - call change with the id and and object containing the new properties
 	 * @param id - Id of the interval whose properties should be changed
 	 * @param props - Property set to apply to the interval. Shallow merging is used between any existing properties
 	 * and `prop`, i.e. the interval will end up with a property object equivalent to `{ ...oldProps, ...props }`.
 	 */
 	changeProperties(id: string, props: PropertySet): void;
 	/**
 	 * Changes the endpoints of an existing interval.
+	 * @deprecated - call change with the start and end parameters encapsulated in an object
 	 * @param id - Id of the interval to change
 	 * @param start - New start value. To leave the endpoint unchanged, pass the current value.
 	 * @param end - New end value. To leave the endpoint unchanged, pass the current value.
 	 * @returns the interval that was changed, if it existed in the collection.
 	 */
 	change(id: string, start: SequencePlace, end: SequencePlace): TInterval | undefined;
+	/**
+	 * Changes the endpoints, properties, or both of an existing interval.
+	 * @param id - Id of the Interval to change
+	 * @returns the interval that was changed, if it existed in the collection.
+	 * Pass the desired new start position, end position, and/or properties in an object. Start and end positions must be changed
+	 * simultaneously - they must either both be specified or both undefined. To only change the properties, leave both endpoints
+	 * undefined. To only change the endpoints, leave the properties undefined.
+	 */
+	change(
+		id: string,
+		{ start, end, props }: { start?: SequencePlace; end?: SequencePlace; props?: PropertySet },
+	): TInterval | undefined;
 
 	attachDeserializer(onDeserialize: DeserializeCallback): void;
 	/**
@@ -1390,70 +1404,83 @@ export class IntervalCollection<TInterval extends ISerializableInterval>
 
 	/**
 	 * {@inheritdoc IIntervalCollection.changeProperties}
+	 * @deprecated - call change with the id and an object containing the new props values
 	 */
 	public changeProperties(id: string, props: PropertySet) {
-		if (!this.attached) {
-			throw new LoggingError("Attach must be called before accessing intervals");
-		}
-		if (typeof id !== "string") {
-			throw new LoggingError("Change API requires an ID that is a string");
-		}
-		if (!props) {
-			throw new LoggingError("changeProperties should be called with a property set");
-		}
-		// prevent the overwriting of an interval label, it should remain unchanged
-		// once it has been inserted into the collection.
-		if (props[reservedRangeLabelsKey] !== undefined) {
-			throw new LoggingError(
-				"The label property should not be modified once inserted to the collection",
-			);
-		}
-
-		const interval = this.getIntervalById(id);
-		if (interval) {
-			const deltaProps = interval.addProperties(
-				props,
-				true,
-				this.isCollaborating ? UnassignedSequenceNumber : UniversalSequenceNumber,
-			);
-			const serializedInterval: ISerializedInterval = interval.serialize();
-
-			// Emit a change op that will only change properties. Add the ID to
-			// the property bag provided by the caller.
-			serializedInterval.start = undefined as any;
-			serializedInterval.end = undefined as any;
-
-			serializedInterval.properties = props;
-			serializedInterval.properties[reservedIntervalIdKey] = interval.getIntervalId();
-			const localSeq = this.getNextLocalSeq();
-			this.localSeqToSerializedInterval.set(localSeq, serializedInterval);
-			this.emitter.emit("change", undefined, serializedInterval, { localSeq });
-			this.emit("propertyChanged", interval, deltaProps, true, undefined);
-		}
+		this.change(id, { props });
 	}
 
+	/**
+	 * {@inheritdoc IIntervalCollection.change}
+	 * @deprecated - call change with the id and an object containing the new start, end, and/or props values
+	 */
+	public change(id: string, start: SequencePlace, end: SequencePlace): TInterval | undefined;
 	/**
 	 * {@inheritdoc IIntervalCollection.change}
 	 */
-	public change(id: string, start: SequencePlace, end: SequencePlace) {
+	public change(
+		id: string,
+		{ start, end, props }: { start?: SequencePlace; end?: SequencePlace; props?: PropertySet },
+	): TInterval | undefined;
+	public change(
+		arg1: string,
+		arg2: SequencePlace | { start?: SequencePlace; end?: SequencePlace; props?: PropertySet },
+		arg3?: SequencePlace,
+	): TInterval | undefined {
+		const id: string = arg1;
+		let start: SequencePlace | undefined;
+		let end: SequencePlace | undefined;
+		let props: PropertySet | undefined;
+		if (isSequencePlace(arg2)) {
+			start = arg2;
+			end = arg3;
+		} else {
+			start = arg2.start;
+			end = arg2.end;
+			props = arg2.props;
+		}
+
 		if (!this.localCollection) {
 			throw new LoggingError("Attach must be called before accessing intervals");
 		}
 
 		// Force id to be a string.
 		if (typeof id !== "string") {
-			throw new LoggingError("Change API requires an ID that is a string");
+			throw new UsageError("Change API requires an ID that is a string");
+		}
+
+		// Ensure that both start and end are defined or both are undefined.
+		if ((start === undefined) !== (end === undefined)) {
+			throw new UsageError(
+				"Change API requires both start and end to be defined or undefined",
+			);
+		}
+
+		// prevent the overwriting of an interval label, it should remain unchanged
+		// once it has been inserted into the collection.
+		if (props?.[reservedRangeLabelsKey] !== undefined) {
+			throw new UsageError(
+				"The label property should not be modified once inserted to the collection",
+			);
 		}
 
 		const interval = this.getIntervalById(id);
 		if (interval) {
-			const newInterval = this.localCollection.changeInterval(interval, start, end);
-			if (!newInterval) {
-				return undefined;
+			let deltaProps: PropertySet | undefined;
+			let newInterval: TInterval | undefined;
+			if (props !== undefined) {
+				deltaProps = interval.addProperties(
+					props,
+					true,
+					this.isCollaborating ? UnassignedSequenceNumber : UniversalSequenceNumber,
+				);
 			}
-			if (!this.isCollaborating && newInterval instanceof SequenceInterval) {
-				setSlideOnRemove(newInterval.start);
-				setSlideOnRemove(newInterval.end);
+			if (start !== undefined && end !== undefined) {
+				newInterval = this.localCollection.changeInterval(interval, start, end);
+				if (!this.isCollaborating && newInterval instanceof SequenceInterval) {
+					setSlideOnRemove(newInterval.start);
+					setSlideOnRemove(newInterval.end);
+				}
 			}
 			const serializedInterval: SerializedIntervalDelta = interval.serialize();
 			const { startPos, startSide, endPos, endSide } = endpointPosAndSide(start, end);
@@ -1463,15 +1490,21 @@ export class IntervalCollection<TInterval extends ISerializableInterval>
 			serializedInterval.startSide = startSide;
 			serializedInterval.endSide = endSide;
 			serializedInterval.stickiness = stickiness;
-			// Emit a property bag containing only the ID, as we don't intend for this op to change any properties.
+			// Emit a property bag containing the ID and the other (if any) properties changed
 			serializedInterval.properties = {
 				[reservedIntervalIdKey]: interval.getIntervalId(),
+				...props,
 			};
 			const localSeq = this.getNextLocalSeq();
 			this.localSeqToSerializedInterval.set(localSeq, serializedInterval);
 			this.emitter.emit("change", undefined, serializedInterval, { localSeq });
-			this.addPendingChange(id, serializedInterval);
-			this.emitChange(newInterval, interval, true, false);
+			if (deltaProps !== undefined) {
+				this.emit("propertyChanged", interval, deltaProps, true, undefined);
+			}
+			if (newInterval) {
+				this.addPendingChange(id, serializedInterval);
+				this.emitChange(newInterval, interval, true, false);
+			}
 			return newInterval;
 		}
 		// No interval to change

packages/dds/sequence/src/test/fuzz/fuzzUtils.ts

@@ -2,7 +2,6 @@
  * Copyright (c) Microsoft Corporation and contributors. All rights reserved.
  * Licensed under the MIT License.
  */
-
 import * as path from "path";
 import { strict as assert } from "assert";
 import {
@@ -67,24 +66,21 @@ export interface AddInterval extends IntervalCollectionSpec, RangeSpec {
 	endSide: Side;
 }
 
-export interface ChangeInterval extends IntervalCollectionSpec, RangeSpec {
+export interface ChangeInterval extends IntervalCollectionSpec {
 	type: "changeInterval";
+	start: number | undefined;
+	end: number | undefined;
 	id: string;
 	startSide: Side;
 	endSide: Side;
+	properties: PropertySet | undefined;
 }
 
 export interface DeleteInterval extends IntervalCollectionSpec {
 	type: "deleteInterval";
 	id: string;
 }
 
-export interface ChangeProperties extends IntervalCollectionSpec {
-	type: "changeProperties";
-	id: string;
-	properties: PropertySet;
-}
-
 export interface RevertSharedStringRevertibles {
 	type: "revertSharedStringRevertibles";
 	editsToRevert: number;
@@ -98,10 +94,9 @@ export interface RevertibleWeights {
 	addInterval: number;
 	deleteInterval: number;
 	changeInterval: number;
-	changeProperties: number;
 }
 
-export type IntervalOperation = AddInterval | ChangeInterval | DeleteInterval | ChangeProperties;
+export type IntervalOperation = AddInterval | ChangeInterval | DeleteInterval;
 export type OperationWithRevert = IntervalOperation | RevertSharedStringRevertibles;
 export type TextOperation = AddText | RemoveRange | ObliterateRange;
 
@@ -161,7 +156,6 @@ export const defaultIntervalOperationGenerationConfig: Required<IntervalOperatio
 			addInterval: 2,
 			deleteInterval: 2,
 			changeInterval: 2,
-			changeProperties: 2,
 			obliterateRange: 0,
 		},
 	};
@@ -239,14 +233,18 @@ export function makeReducer(
 		},
 		changeInterval: async (
 			{ client },
-			{ id, start, end, collectionName, startSide, endSide },
+			{ id, start, end, collectionName, startSide, endSide, properties },
 		) => {
 			const collection = client.channel.getIntervalCollection(collectionName);
-			collection.change(id, { pos: start, side: startSide }, { pos: end, side: endSide });
-		},
-		changeProperties: async ({ client }, { id, properties, collectionName }) => {
-			const collection = client.channel.getIntervalCollection(collectionName);
-			collection.changeProperties(id, { ...properties });
+			if (start !== undefined && end !== undefined) {
+				collection.change(id, {
+					start: { pos: start, side: startSide },
+					end: { pos: end, side: endSide },
+					props: properties,
+				});
+			} else {
+				collection.change(id, { props: properties });
+			}
 		},
 		revertSharedStringRevertibles: async ({ client }, { editsToRevert }) => {
 			assert(isRevertibleSharedString(client.channel));
@@ -373,10 +371,10 @@ export const baseModel: Omit<
 				case "removeRange":
 				case "addInterval":
 				case "changeInterval":
-					if (op.start > 0) {
+					if (op.start !== undefined && op.start > 0) {
 						op.start -= 1;
 					}
-					if (op.end > 0) {
+					if (op.end !== undefined && op.end > 0) {
 						op.end -= 1;
 					}
 					break;
@@ -392,7 +390,7 @@ export const baseModel: Omit<
 			) {
 				return;
 			}
-			if (op.end > 0) {
+			if (op.end !== undefined && op.end > 0) {
 				op.end -= 1;
 			}
 		},