Skip to content

Commit 35934b2

Browse files
committed
Don't use server side terminology and knowledge regarding timeserials / site codes in client side LiveObjects implementation
See the relevant discussion in the spec PR [1] [1] ably/specification#279 (comment)
1 parent 8f9bd30 commit 35934b2

File tree

6 files changed

+71
-71
lines changed

6 files changed

+71
-71
lines changed

src/plugins/objects/defaults.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ export const DEFAULTS = {
22
gcInterval: 1000 * 60 * 5, // 5 minutes
33
/**
44
* Must be > 2 minutes to ensure we keep tombstones long enough to avoid the possibility of receiving an operation
5-
* with an earlier origin timeserial that would not have been applied if the tombstone still existed.
5+
* with an earlier serial that would not have been applied if the tombstone still existed.
66
*
77
* Applies both for map entries tombstones and object tombstones.
88
*/

src/plugins/objects/livecounter.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -168,20 +168,20 @@ export class LiveCounter extends LiveObject<LiveCounterData, LiveCounterUpdate>
168168
);
169169
}
170170

171-
const opOriginTimeserial = msg.serial!;
171+
const opSerial = msg.serial!;
172172
const opSiteCode = msg.siteCode!;
173-
if (!this._canApplyOperation(opOriginTimeserial, opSiteCode)) {
173+
if (!this._canApplyOperation(opSerial, opSiteCode)) {
174174
this._client.Logger.logAction(
175175
this._client.logger,
176176
this._client.Logger.LOG_MICRO,
177177
'LiveCounter.applyOperation()',
178-
`skipping ${op.action} op: op timeserial ${opOriginTimeserial.toString()} <= site timeserial ${this._siteTimeserials[opSiteCode]?.toString()}; objectId=${this.getObjectId()}`,
178+
`skipping ${op.action} op: op serial ${opSerial.toString()} <= site serial ${this._siteTimeserials[opSiteCode]?.toString()}; objectId=${this.getObjectId()}`,
179179
);
180180
return;
181181
}
182-
// should update stored site timeserial immediately. doesn't matter if we successfully apply the op,
182+
// should update stored site serial immediately. doesn't matter if we successfully apply the op,
183183
// as it's important to mark that the op was processed by the object
184-
this._siteTimeserials[opSiteCode] = opOriginTimeserial;
184+
this._siteTimeserials[opSiteCode] = opSerial;
185185

186186
if (this.isTombstoned()) {
187187
// this object is tombstoned so the operation cannot be applied
@@ -250,8 +250,8 @@ export class LiveCounter extends LiveObject<LiveCounterData, LiveCounterUpdate>
250250
}
251251
}
252252

253-
// object's site timeserials are still updated even if it is tombstoned, so always use the site timeserials received from the op.
254-
// should default to empty map if site timeserials do not exist on the object state, so that any future operation may be applied to this object.
253+
// object's site serials are still updated even if it is tombstoned, so always use the site serials received from the operation.
254+
// should default to empty map if site serials do not exist on the object state, so that any future operation may be applied to this object.
255255
this._siteTimeserials = objectState.siteTimeserials ?? {};
256256

257257
if (this.isTombstoned()) {

src/plugins/objects/livemap.ts

Lines changed: 37 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ export type ObjectData = ObjectIdObjectData | ValueObjectData;
3636
export interface LiveMapEntry {
3737
tombstone: boolean;
3838
/**
39-
* Can't use timeserial from the operation that deleted the entry for the same reason as for {@link LiveObject} tombstones, see explanation there.
39+
* Can't use serial from the operation that deleted the entry for the same reason as for {@link LiveObject} tombstones, see explanation there.
4040
*/
4141
tombstonedAt: number | undefined;
4242
timeserial: string | undefined;
@@ -371,20 +371,20 @@ export class LiveMap<T extends API.LiveMapType> extends LiveObject<LiveMapData,
371371
);
372372
}
373373

374-
const opOriginTimeserial = msg.serial!;
374+
const opSerial = msg.serial!;
375375
const opSiteCode = msg.siteCode!;
376-
if (!this._canApplyOperation(opOriginTimeserial, opSiteCode)) {
376+
if (!this._canApplyOperation(opSerial, opSiteCode)) {
377377
this._client.Logger.logAction(
378378
this._client.logger,
379379
this._client.Logger.LOG_MICRO,
380380
'LiveMap.applyOperation()',
381-
`skipping ${op.action} op: op timeserial ${opOriginTimeserial.toString()} <= site timeserial ${this._siteTimeserials[opSiteCode]?.toString()}; objectId=${this.getObjectId()}`,
381+
`skipping ${op.action} op: op serial ${opSerial.toString()} <= site serial ${this._siteTimeserials[opSiteCode]?.toString()}; objectId=${this.getObjectId()}`,
382382
);
383383
return;
384384
}
385-
// should update stored site timeserial immediately. doesn't matter if we successfully apply the op,
385+
// should update stored site serial immediately. doesn't matter if we successfully apply the op,
386386
// as it's important to mark that the op was processed by the object
387-
this._siteTimeserials[opSiteCode] = opOriginTimeserial;
387+
this._siteTimeserials[opSiteCode] = opSerial;
388388

389389
if (this.isTombstoned()) {
390390
// this object is tombstoned so the operation cannot be applied
@@ -403,7 +403,7 @@ export class LiveMap<T extends API.LiveMapType> extends LiveObject<LiveMapData,
403403
// leave an explicit return here, so that TS knows that update object is always set after the switch statement.
404404
return;
405405
} else {
406-
update = this._applyMapSet(op.mapOp, opOriginTimeserial);
406+
update = this._applyMapSet(op.mapOp, opSerial);
407407
}
408408
break;
409409

@@ -413,7 +413,7 @@ export class LiveMap<T extends API.LiveMapType> extends LiveObject<LiveMapData,
413413
// leave an explicit return here, so that TS knows that update object is always set after the switch statement.
414414
return;
415415
} else {
416-
update = this._applyMapRemove(op.mapOp, opOriginTimeserial);
416+
update = this._applyMapRemove(op.mapOp, opSerial);
417417
}
418418
break;
419419

@@ -479,8 +479,8 @@ export class LiveMap<T extends API.LiveMapType> extends LiveObject<LiveMapData,
479479
}
480480
}
481481

482-
// object's site timeserials are still updated even if it is tombstoned, so always use the site timeserials received from the op.
483-
// should default to empty map if site timeserials do not exist on the object state, so that any future operation may be applied to this object.
482+
// object's site serials are still updated even if it is tombstoned, so always use the site serials received from the op.
483+
// should default to empty map if site serials do not exist on the object state, so that any future operation may be applied to this object.
484484
this._siteTimeserials = objectState.siteTimeserials ?? {};
485485

486486
if (this.isTombstoned()) {
@@ -591,15 +591,15 @@ export class LiveMap<T extends API.LiveMapType> extends LiveObject<LiveMapData,
591591
// in order to apply MAP_CREATE op for an existing map, we should merge their underlying entries keys.
592592
// 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.
593593
Object.entries(objectOperation.map.entries ?? {}).forEach(([key, entry]) => {
594-
// for MAP_CREATE op we must use dedicated timeserial field available on an entry, instead of a timeserial on a message
595-
const opOriginTimeserial = entry.timeserial;
594+
// for a MAP_CREATE operation we must use the serial value available on an entry, instead of a serial on a message
595+
const opSerial = entry.timeserial;
596596
let update: LiveMapUpdate | LiveObjectUpdateNoop;
597597
if (entry.tombstone === true) {
598598
// entry in MAP_CREATE op is removed, try to apply MAP_REMOVE op
599-
update = this._applyMapRemove({ key }, opOriginTimeserial);
599+
update = this._applyMapRemove({ key }, opSerial);
600600
} else {
601601
// entry in MAP_CREATE op is not removed, try to set it via MAP_SET op
602-
update = this._applyMapSet({ key, data: entry.data }, opOriginTimeserial);
602+
update = this._applyMapSet({ key, data: entry.data }, opSerial);
603603
}
604604

605605
// skip noop updates
@@ -649,17 +649,17 @@ export class LiveMap<T extends API.LiveMapType> extends LiveObject<LiveMapData,
649649
return this._mergeInitialDataFromCreateOperation(op);
650650
}
651651

652-
private _applyMapSet(op: MapOp, opOriginTimeserial: string | undefined): LiveMapUpdate | LiveObjectUpdateNoop {
652+
private _applyMapSet(op: MapOp, opSerial: string | undefined): LiveMapUpdate | LiveObjectUpdateNoop {
653653
const { ErrorInfo, Utils } = this._client;
654654

655655
const existingEntry = this._dataRef.data.get(op.key);
656-
if (existingEntry && !this._canApplyMapOperation(existingEntry.timeserial, opOriginTimeserial)) {
657-
// the operation's origin timeserial <= the entry's timeserial, ignore the operation.
656+
if (existingEntry && !this._canApplyMapOperation(existingEntry.timeserial, opSerial)) {
657+
// the operation's serial <= the entry's serial, ignore the operation.
658658
this._client.Logger.logAction(
659659
this._client.logger,
660660
this._client.Logger.LOG_MICRO,
661661
'LiveMap._applyMapSet()',
662-
`skipping update for key="${op.key}": op timeserial ${opOriginTimeserial?.toString()} <= entry timeserial ${existingEntry.timeserial?.toString()}; objectId=${this.getObjectId()}`,
662+
`skipping update for key="${op.key}": op serial ${opSerial?.toString()} <= entry serial ${existingEntry.timeserial?.toString()}; objectId=${this.getObjectId()}`,
663663
);
664664
return { noop: true };
665665
}
@@ -687,43 +687,43 @@ export class LiveMap<T extends API.LiveMapType> extends LiveObject<LiveMapData,
687687
if (existingEntry) {
688688
existingEntry.tombstone = false;
689689
existingEntry.tombstonedAt = undefined;
690-
existingEntry.timeserial = opOriginTimeserial;
690+
existingEntry.timeserial = opSerial;
691691
existingEntry.data = liveData;
692692
} else {
693693
const newEntry: LiveMapEntry = {
694694
tombstone: false,
695695
tombstonedAt: undefined,
696-
timeserial: opOriginTimeserial,
696+
timeserial: opSerial,
697697
data: liveData,
698698
};
699699
this._dataRef.data.set(op.key, newEntry);
700700
}
701701
return { update: { [op.key]: 'updated' } };
702702
}
703703

704-
private _applyMapRemove(op: MapOp, opOriginTimeserial: string | undefined): LiveMapUpdate | LiveObjectUpdateNoop {
704+
private _applyMapRemove(op: MapOp, opSerial: string | undefined): LiveMapUpdate | LiveObjectUpdateNoop {
705705
const existingEntry = this._dataRef.data.get(op.key);
706-
if (existingEntry && !this._canApplyMapOperation(existingEntry.timeserial, opOriginTimeserial)) {
707-
// the operation's origin timeserial <= the entry's timeserial, ignore the operation.
706+
if (existingEntry && !this._canApplyMapOperation(existingEntry.timeserial, opSerial)) {
707+
// the operation's serial <= the entry's serial, ignore the operation.
708708
this._client.Logger.logAction(
709709
this._client.logger,
710710
this._client.Logger.LOG_MICRO,
711711
'LiveMap._applyMapRemove()',
712-
`skipping remove for key="${op.key}": op timeserial ${opOriginTimeserial?.toString()} <= entry timeserial ${existingEntry.timeserial?.toString()}; objectId=${this.getObjectId()}`,
712+
`skipping remove for key="${op.key}": op serial ${opSerial?.toString()} <= entry serial ${existingEntry.timeserial?.toString()}; objectId=${this.getObjectId()}`,
713713
);
714714
return { noop: true };
715715
}
716716

717717
if (existingEntry) {
718718
existingEntry.tombstone = true;
719719
existingEntry.tombstonedAt = Date.now();
720-
existingEntry.timeserial = opOriginTimeserial;
720+
existingEntry.timeserial = opSerial;
721721
existingEntry.data = undefined;
722722
} else {
723723
const newEntry: LiveMapEntry = {
724724
tombstone: true,
725725
tombstonedAt: Date.now(),
726-
timeserial: opOriginTimeserial,
726+
timeserial: opSerial,
727727
data: undefined,
728728
};
729729
this._dataRef.data.set(op.key, newEntry);
@@ -733,31 +733,31 @@ export class LiveMap<T extends API.LiveMapType> extends LiveObject<LiveMapData,
733733
}
734734

735735
/**
736-
* Returns true if the origin timeserials of the given operation and entry indicate that
736+
* Returns true if the serials of the given operation and entry indicate that
737737
* the operation should be applied to the entry, following the CRDT semantics of this LiveMap.
738738
*/
739-
private _canApplyMapOperation(entryTimeserial: string | undefined, opTimeserial: string | undefined): boolean {
739+
private _canApplyMapOperation(mapEntrySerial: string | undefined, opSerial: string | undefined): boolean {
740740
// for LWW CRDT semantics (the only supported LiveMap semantic) an operation
741-
// should only be applied if its timeserial is strictly greater ("after") than an entry's timeserial.
741+
// should only be applied if its serial is strictly greater ("after") than an entry's serial.
742742

743-
if (!entryTimeserial && !opTimeserial) {
744-
// if both timeserials are nullish or empty strings, we treat them as the "earliest possible" timeserials,
743+
if (!mapEntrySerial && !opSerial) {
744+
// if both serials are nullish or empty strings, we treat them as the "earliest possible" serials,
745745
// in which case they are "equal", so the operation should not be applied
746746
return false;
747747
}
748748

749-
if (!entryTimeserial) {
750-
// any op timeserial is greater than non-existing entry timeserial
749+
if (!mapEntrySerial) {
750+
// any operation serial is greater than non-existing entry serial
751751
return true;
752752
}
753753

754-
if (!opTimeserial) {
755-
// non-existing op timeserial is lower than any entry timeserial
754+
if (!opSerial) {
755+
// non-existing operation serial is lower than any entry serial
756756
return false;
757757
}
758758

759-
// if both timeserials exist, compare them lexicographically
760-
return opTimeserial > entryTimeserial;
759+
// if both serials exist, compare them lexicographically
760+
return opSerial > mapEntrySerial;
761761
}
762762

763763
private _liveMapDataFromMapEntries(entries: Record<string, MapEntry>): LiveMapData {

src/plugins/objects/liveobject.ts

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,8 @@ export abstract class LiveObject<
5252
protected _createOperationIsMerged: boolean;
5353
private _tombstone: boolean;
5454
/**
55-
* Even though the `timeserial` from the operation that deleted the object contains the timestamp value,
56-
* the `timeserial` should be treated as an opaque string on the client, meaning we should not attempt to parse it.
55+
* Even though the {@link ObjectMessage.serial} value from the operation that deleted the object contains the timestamp value,
56+
* the serial should be treated as an opaque string on the client, meaning we should not attempt to parse it.
5757
*
5858
* Therefore, we need to set our own timestamp using local clock when the object is deleted client-side.
5959
* Strictly speaking, this does make an assumption about the client clock not being too heavily skewed behind the server,
@@ -70,7 +70,7 @@ export abstract class LiveObject<
7070
this._lifecycleEvents = new this._client.EventEmitter(this._client.logger);
7171
this._objectId = objectId;
7272
this._dataRef = this._getZeroValueData();
73-
// use empty timeserials vector by default, so any future operation can be applied to this object
73+
// use empty map of serials by default, so any future operation can be applied to this object
7474
this._siteTimeserials = {};
7575
this._createOperationIsMerged = false;
7676
this._tombstone = false;
@@ -181,22 +181,22 @@ export abstract class LiveObject<
181181
}
182182

183183
/**
184-
* Returns true if the given origin timeserial indicates that the operation to which it belongs should be applied to the object.
184+
* Returns true if the given serial indicates that the operation to which it belongs should be applied to the object.
185185
*
186-
* An operation should be applied if the origin timeserial is strictly greater than the timeserial in the site timeserials for the same site.
187-
* If the site timeserials do not contain a timeserial for the site of the origin timeserial, the operation should be applied.
186+
* An operation should be applied if its serial is strictly greater than the serial in the `siteTimeserials` map for the same site.
187+
* If `siteTimeserials` map does not contain a serial for the same site, the operation should be applied.
188188
*/
189-
protected _canApplyOperation(opOriginTimeserial: string | undefined, opSiteCode: string | undefined): boolean {
190-
if (!opOriginTimeserial) {
191-
throw new this._client.ErrorInfo(`Invalid timeserial: ${opOriginTimeserial}`, 92000, 500);
189+
protected _canApplyOperation(opSerial: string | undefined, opSiteCode: string | undefined): boolean {
190+
if (!opSerial) {
191+
throw new this._client.ErrorInfo(`Invalid serial: ${opSerial}`, 92000, 500);
192192
}
193193

194194
if (!opSiteCode) {
195195
throw new this._client.ErrorInfo(`Invalid site code: ${opSiteCode}`, 92000, 500);
196196
}
197197

198-
const siteTimeserial = this._siteTimeserials[opSiteCode];
199-
return !siteTimeserial || opOriginTimeserial > siteTimeserial;
198+
const siteSerial = this._siteTimeserials[opSiteCode];
199+
return !siteSerial || opSerial > siteSerial;
200200
}
201201

202202
protected _applyObjectDelete(): TUpdate {
@@ -216,7 +216,7 @@ export abstract class LiveObject<
216216
* Provided object state should hold a valid data for current LiveObject, e.g. counter data for LiveCounter, map data for LiveMap.
217217
*
218218
* Object states are received during sync sequence, and sync sequence is a source of truth for the current state of the objects,
219-
* so we can use the data received from the sync sequence directly and override any data values or site timeserials this LiveObject has
219+
* so we can use the data received from the sync sequence directly and override any data values or site serials this LiveObject has
220220
* without the need to merge them.
221221
*
222222
* Returns an update object that describes the changes applied based on the object's previous value.

src/plugins/objects/objectmessage.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,10 @@ export interface MapEntry {
5454
/** Indicates whether the map entry has been removed. */
5555
tombstone?: boolean;
5656
/**
57-
* The *origin* timeserial of the last operation that was applied to the map entry.
57+
* The {@link ObjectMessage.serial} value of the last operation that was applied to the map entry.
5858
*
5959
* It is optional in a MAP_CREATE operation and might be missing, in which case the client should use a nullish value for it
60-
* and treat it as the "earliest possible" timeserial for comparison purposes.
60+
* and treat it as the "earliest possible" serial for comparison purposes.
6161
*/
6262
timeserial?: string;
6363
/** The data that represents the value of the map entry. */
@@ -118,7 +118,7 @@ export interface ObjectOperation {
118118
export interface ObjectState {
119119
/** The identifier of the object. */
120120
objectId: string;
121-
/** A vector of origin timeserials keyed by site code of the last operation that was applied to this object. */
121+
/** A map of serials keyed by a {@link ObjectMessage.siteCode}, representing the last operations applied to this object */
122122
siteTimeserials: Record<string, string>;
123123
/** True if the object has been tombstoned. */
124124
tombstone: boolean;
@@ -162,9 +162,9 @@ export class ObjectMessage {
162162
* Mutually exclusive with the `operation` field. This field is only set on object messages if the `action` field of the `ProtocolMessage` encapsulating it is `OBJECT_SYNC`.
163163
*/
164164
object?: ObjectState;
165-
/** Timeserial format. Contains the origin timeserial for this object message. */
165+
/** An opaque string that uniquely identifies this object message. */
166166
serial?: string;
167-
/** Site code corresponding to this message's timeserial */
167+
/** An opaque string used as a key to update the map of serial values on an object. */
168168
siteCode?: string;
169169

170170
constructor(

0 commit comments

Comments
 (0)