From 681c1af088e6ce2afc8ada2821e7e09c1893732a Mon Sep 17 00:00:00 2001 From: Andrew Bulat Date: Thu, 24 Oct 2024 21:44:49 +0100 Subject: [PATCH] Use zero-value timeserial if timeserial is missing for MapEntry in a MAP_CREATE operation --- src/plugins/liveobjects/livemap.ts | 34 +++++++++++++------------ src/plugins/liveobjects/statemessage.ts | 9 +++++-- src/plugins/liveobjects/timeserial.ts | 9 +++++++ 3 files changed, 34 insertions(+), 18 deletions(-) diff --git a/src/plugins/liveobjects/livemap.ts b/src/plugins/liveobjects/livemap.ts index 90582d3a7..5969e93e0 100644 --- a/src/plugins/liveobjects/livemap.ts +++ b/src/plugins/liveobjects/livemap.ts @@ -66,7 +66,9 @@ export class LiveMap extends LiveObject { const liveDataEntry: MapEntry = { ...entry, - timeserial: DefaultTimeserial.calculateTimeserial(client, entry.timeserial), + timeserial: entry.timeserial + ? DefaultTimeserial.calculateTimeserial(client, entry.timeserial) + : DefaultTimeserial.zeroValueTimeserial(client), // true only if we received explicit true. otherwise always false tombstone: entry.tombstone === true, data: liveData, @@ -141,7 +143,7 @@ export class LiveMap extends LiveObject { if (this._client.Utils.isNil(op.mapOp)) { this._throwNoPayloadError(op); } else { - this._applyMapSet(op.mapOp, msg.serial); + this._applyMapSet(op.mapOp, DefaultTimeserial.calculateTimeserial(this._client, msg.serial)); } break; @@ -149,7 +151,7 @@ export class LiveMap extends LiveObject { if (this._client.Utils.isNil(op.mapOp)) { this._throwNoPayloadError(op); } else { - this._applyMapRemove(op.mapOp, msg.serial); + this._applyMapRemove(op.mapOp, DefaultTimeserial.calculateTimeserial(this._client, msg.serial)); } break; @@ -193,7 +195,9 @@ export class LiveMap extends LiveObject { // we can do this by iterating over entries from MAP_CREATE op and apply changes on per-key basis as if we had MAP_SET, MAP_REMOVE operations. Object.entries(op.entries ?? {}).forEach(([key, entry]) => { // for MAP_CREATE op we must use dedicated timeserial field available on an entry, instead of a timeserial on a message - const opOriginTimeserial = entry.timeserial; + const opOriginTimeserial = entry.timeserial + ? DefaultTimeserial.calculateTimeserial(this._client, entry.timeserial) + : DefaultTimeserial.zeroValueTimeserial(this._client); if (entry.tombstone === true) { // entry in MAP_CREATE op is deleted, try to apply MAP_REMOVE op this._applyMapRemove({ key }, opOriginTimeserial); @@ -204,18 +208,17 @@ export class LiveMap extends LiveObject { }); } - private _applyMapSet(op: StateMapOp, opOriginTimeserialStr: string | undefined): void { + private _applyMapSet(op: StateMapOp, opOriginTimeserial: Timeserial): void { const { ErrorInfo, Utils } = this._client; - const opTimeserial = DefaultTimeserial.calculateTimeserial(this._client, opOriginTimeserialStr); const existingEntry = this._dataRef.data.get(op.key); - if (existingEntry && opTimeserial.before(existingEntry.timeserial)) { + if (existingEntry && opOriginTimeserial.before(existingEntry.timeserial)) { // the operation's origin timeserial < the entry's timeserial, ignore the operation. this._client.Logger.logAction( this._client.logger, this._client.Logger.LOG_MICRO, 'LiveMap._applyMapSet()', - `skipping updating key="${op.key}" as existing key entry has greater timeserial: ${existingEntry.timeserial.toString()}, than the op: ${opOriginTimeserialStr}; objectId=${this._objectId}`, + `skipping updating key="${op.key}" as existing key entry has greater timeserial: ${existingEntry.timeserial.toString()}, than the op: ${opOriginTimeserial.toString()}; objectId=${this._objectId}`, ); return; } @@ -242,40 +245,39 @@ export class LiveMap extends LiveObject { if (existingEntry) { existingEntry.tombstone = false; - existingEntry.timeserial = opTimeserial; + existingEntry.timeserial = opOriginTimeserial; existingEntry.data = liveData; } else { const newEntry: MapEntry = { tombstone: false, - timeserial: opTimeserial, + timeserial: opOriginTimeserial, data: liveData, }; this._dataRef.data.set(op.key, newEntry); } } - private _applyMapRemove(op: StateMapOp, opOriginTimeserialStr: string | undefined): void { - const opTimeserial = DefaultTimeserial.calculateTimeserial(this._client, opOriginTimeserialStr); + private _applyMapRemove(op: StateMapOp, opOriginTimeserial: Timeserial): void { const existingEntry = this._dataRef.data.get(op.key); - if (existingEntry && opTimeserial.before(existingEntry.timeserial)) { + if (existingEntry && opOriginTimeserial.before(existingEntry.timeserial)) { // the operation's origin timeserial < the entry's timeserial, ignore the operation. this._client.Logger.logAction( this._client.logger, this._client.Logger.LOG_MICRO, 'LiveMap._applyMapRemove()', - `skipping removing key="${op.key}" as existing key entry has greater timeserial: ${existingEntry.timeserial.toString()}, than the op: ${opOriginTimeserialStr}; objectId=${this._objectId}`, + `skipping removing key="${op.key}" as existing key entry has greater timeserial: ${existingEntry.timeserial.toString()}, than the op: ${opOriginTimeserial.toString()}; objectId=${this._objectId}`, ); return; } if (existingEntry) { existingEntry.tombstone = true; - existingEntry.timeserial = opTimeserial; + existingEntry.timeserial = opOriginTimeserial; existingEntry.data = undefined; } else { const newEntry: MapEntry = { tombstone: true, - timeserial: opTimeserial, + timeserial: opOriginTimeserial, data: undefined, }; this._dataRef.data.set(op.key, newEntry); diff --git a/src/plugins/liveobjects/statemessage.ts b/src/plugins/liveobjects/statemessage.ts index 2fd293a33..b4eefdadd 100644 --- a/src/plugins/liveobjects/statemessage.ts +++ b/src/plugins/liveobjects/statemessage.ts @@ -48,8 +48,13 @@ export interface StateCounterOp { export interface StateMapEntry { /** Indicates whether the map entry has been removed. */ tombstone?: boolean; - /** The *origin* timeserial of the last operation that was applied to the map entry. */ - timeserial: string; + /** + * The *origin* timeserial of the last operation that was applied to the map entry. + * + * It is optional in a MAP_CREATE operation and might be missing, in which case the client should default to using zero-value timeserial, + * which is the "earliest possible" timeserial. This will allow any other operation to update the field based on a timeserial comparison. + */ + timeserial?: string; /** The data that represents the value of the map entry. */ data: StateData; } diff --git a/src/plugins/liveobjects/timeserial.ts b/src/plugins/liveobjects/timeserial.ts index 4d764a693..0accdc81b 100644 --- a/src/plugins/liveobjects/timeserial.ts +++ b/src/plugins/liveobjects/timeserial.ts @@ -101,6 +101,15 @@ export class DefaultTimeserial implements Timeserial { ); } + /** + * Returns a zero-value Timeserial `@0-0` - "earliest possible" timeserial. + * + * @returns The timeserial object. + */ + static zeroValueTimeserial(client: BaseClient): Timeserial { + return new DefaultTimeserial(client, '', 0, 0); // @0-0 + } + /** * Compares this timeserial to the supplied timeserial, returning a number indicating their relative order. * @param timeserialToCompare The timeserial to compare against. Can be a string or a Timeserial object.