Skip to content

Commit 3abbc4d

Browse files
Propose an alternative to PathEvent.bubbles to make intent clearer
After noticing that the path-based API spec PR [1] didn't include map-entry events nor the bubbling-exclusion mechanism that's implemented in ably-js, I wanted to get a better understanding of this exclusion mechanism and why it exists. From what I can tell, it exists to make sure that if there's a map at path "myMap", and this map emits a LiveMapUpdate having key "myKey", then a subscription that covers the path "myMap" will only receive one event (for "myMap"), as opposed to two (for "myMap" and "myMap.myKey"). If that _is_ the only reason that this mechanism exists, then I don't think it's very obvious currently; the intended behaviour isn't documented. I think the implementation could be clearer if it makes the rules explicit: - for a given LiveObjectUpdate, a given subscription receives at most one event per path-to-object - in the case where a subscription covers both the "myMap" and "myMap.myKey" paths, "myMap" wins That's what I've tried to do here. Perhaps I've misunderstood the intent of `bubbles` (which, given that the tests still pass, would suggest a test gap), or perhaps others don't find what I'm proposing clearer (in which case, `bubbles` needs better documentation that explains its motivation). Thoughts, please. [1] ably/specification#427 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 8de8895 commit 3abbc4d

2 files changed

Lines changed: 57 additions & 75 deletions

File tree

src/plugins/liveobjects/liveobject.ts

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -299,40 +299,44 @@ export abstract class LiveObject<
299299

300300
/**
301301
* Notifies path-based subscriptions about changes to this object.
302-
* For LiveMapUpdate events, also creates non-bubbling events for each updated key.
302+
* For LiveMapUpdate events, each updated key also contributes a candidate
303+
* path one segment deeper than this object's own path.
303304
*/
304305
private _notifyPathSubscriptions(update: TUpdate): void {
305-
const paths = this.getFullPaths();
306+
const pathsToThis = this.getFullPaths();
306307

307-
if (paths.length === 0) {
308+
if (pathsToThis.length === 0) {
308309
// No paths to this object, skip notification
309310
return;
310311
}
311312

312313
// Do not expose object sync messages as they do not represent a single operation on an object
313314
const operationObjectMessage = update.objectMessage?.isOperationMessage() ? update.objectMessage : undefined;
314-
const pathEvents: PathEvent[] = paths.map((path) => ({
315-
path,
316-
message: operationObjectMessage,
317-
bubbles: true,
318-
}));
319-
320-
// For LiveMapUpdate, also create non-bubbling events for each updated key
321-
if (update._type === 'LiveMapUpdate') {
322-
const updatedKeys = Object.keys(update.update);
323-
324-
for (const key of updatedKeys) {
325-
for (const basePath of paths) {
326-
pathEvents.push({
327-
path: [...basePath, key],
328-
message: operationObjectMessage,
329-
bubbles: false,
330-
});
315+
316+
// Call notifyPathEvent() once for each path-to-this. Since
317+
// notifyPathEvent() emits at most one event on each subscription, this
318+
// means that we emit at most one event per path-to-this.
319+
for (const pathToThis of pathsToThis) {
320+
const priorityOrderedCandidatePaths: Path[] = [pathToThis];
321+
322+
// For LiveMapUpdate, also add a candidate path per updated key. We insert these after
323+
// pathToThis so that notifyPathEvent() picks pathToThis in the case where a given subscription
324+
// covers multiple candidate paths (that is, we favour the shorter path).
325+
if (update._type === 'LiveMapUpdate') {
326+
const updatedKeys = Object.keys(update.update);
327+
328+
for (const key of updatedKeys) {
329+
priorityOrderedCandidatePaths.push([...pathToThis, key]);
331330
}
332331
}
333-
}
334332

335-
this._realtimeObject.getPathObjectSubscriptionRegister().notifyPathEvents(pathEvents);
333+
const pathEvent: PathEvent = {
334+
priorityOrderedCandidatePaths,
335+
message: operationObjectMessage,
336+
};
337+
338+
this._realtimeObject.getPathObjectSubscriptionRegister().notifyPathEvent(pathEvent);
339+
}
336340
}
337341

338342
private _isNoopUpdate(update: TUpdate | LiveObjectUpdateNoop): update is LiveObjectUpdateNoop {

src/plugins/liveobjects/pathobjectsubscriptionregister.ts

Lines changed: 31 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,14 @@ export interface SubscriptionEntry {
2222
* Event data that LiveObjects provide when notifying of changes
2323
*/
2424
export interface PathEvent {
25-
/** The path where the event occurred */
26-
path: Path;
25+
/**
26+
* Candidate paths for surfacing this event to subscriptions, in order of
27+
* decreasing priority. For a given subscription, the first candidate path
28+
* it covers is used as the path of `event.object` passed to its listener.
29+
*/
30+
priorityOrderedCandidatePaths: Path[];
2731
/** Object message that caused this event */
2832
message?: ObjectMessage;
29-
/** Whether this event should bubble up to parent paths. Defaults to true if not specified. */
30-
bubbles?: boolean;
3133
}
3234

3335
/**
@@ -87,28 +89,26 @@ export class PathObjectSubscriptionRegister {
8789
}
8890

8991
/**
90-
* Notifies all matching subscriptions about an event that occurred at the specified path(s).
91-
*
92-
* @param events - Array of path events to process
93-
*/
94-
notifyPathEvents(events: PathEvent[]): void {
95-
for (const event of events) {
96-
this._processEvent(event);
97-
}
98-
}
99-
100-
/**
101-
* Processes a single path event and calls all matching subscription listeners.
92+
* Dispatches a {@link PathEvent} to subscriptions. Each subscription that
93+
* covers any of the event's {@link PathEvent.priorityOrderedCandidatePaths}
94+
* receives at most one notification, at the first covered path.
10295
*/
103-
private _processEvent(event: PathEvent): void {
96+
notifyPathEvent(event: PathEvent): void {
10497
for (const subscription of this._subscriptions.values()) {
105-
if (!this._shouldNotifySubscription(subscription, event)) {
98+
const chosenCoveredPath = event.priorityOrderedCandidatePaths.find((path) =>
99+
this._subscriptionCoversPath(subscription, path),
100+
);
101+
if (chosenCoveredPath === undefined) {
106102
continue;
107103
}
108104

109105
try {
110106
const subscriptionEvent: PathObjectSubscriptionEvent = {
111-
object: new DefaultPathObject(this._realtimeObject, this._realtimeObject.getPool().getRoot(), event.path),
107+
object: new DefaultPathObject(
108+
this._realtimeObject,
109+
this._realtimeObject.getPool().getRoot(),
110+
chosenCoveredPath,
111+
),
112112
message: event.message?.toUserFacingMessage(this._realtimeObject.getChannel()),
113113
};
114114

@@ -118,46 +118,35 @@ export class PathObjectSubscriptionRegister {
118118
this._client.Logger.logAction(
119119
this._client.logger,
120120
this._client.Logger.LOG_MINOR,
121-
'PathObjectSubscriptionRegister._processEvent()',
122-
`Error in PathObject subscription listener; path=${JSON.stringify(event.path)}, error=${error}`,
121+
'PathObjectSubscriptionRegister.notifyPathEvent()',
122+
`Error in PathObject subscription listener; path=${JSON.stringify(chosenCoveredPath)}, error=${error}`,
123123
);
124124
}
125125
}
126126
}
127127

128128
/**
129-
* Determines if a subscription should be notified about an event at the given path.
130-
* Implements depth-based filtering logic and bubbling control.
131-
*
132-
* Depth examples (when event.bubbles is true):
133-
* - subscription at ["users"] with depth=undefined: matches ["users"], ["users", "emma"], ["users", "emma", "visits"], etc.
134-
* - subscription at ["users"] with depth=1: matches ["users"] only
135-
* - subscription at ["users"] with depth=2: matches ["users"], ["users", "emma"] only
136-
* - subscription at ["users"] with depth=3: matches ["users"], ["users", "emma"], ["users", "emma", "visits"] only
129+
* Returns true if the given path falls within the area covered by the
130+
* subscription — that is, it starts with the subscription's path, and
131+
* extends it by at most `depth − 1` further segments.
137132
*
138-
* Non-bubbling examples (when event.bubbles is false):
139-
* - Event at ["users", "emma"] with bubbles=false:
140-
* - subscription at ["users"]: NOT triggered (no bubbling to parent)
141-
* - subscription at ["users", "emma"]: triggered (exact path match)
133+
* Coverage examples:
134+
* - subscription at ["users"] with depth=undefined: covers ["users"], ["users", "emma"], ["users", "emma", "visits"], etc.
135+
* - subscription at ["users"] with depth=1: covers ["users"] only
136+
* - subscription at ["users"] with depth=2: covers ["users"], ["users", "emma"] only
137+
* - subscription at ["users"] with depth=3: covers ["users"], ["users", "emma"], ["users", "emma", "visits"] only
142138
*
143139
* The depth calculation is: eventPath.length - subscriptionPath.length + 1
144140
* This means:
145141
* - Same level (["users"] -> ["users"]): 1 - 1 + 1 = 1 (depth=1)
146142
* - One level deeper (["users"] -> ["users", "emma"]): 2 - 1 + 1 = 2 (depth=2)
147143
* - Two levels deeper (["users"] -> ["users", "emma", "visits"]): 3 - 1 + 1 = 3 (depth=3)
148144
*/
149-
private _shouldNotifySubscription(subscription: SubscriptionEntry, event: PathEvent): boolean {
145+
private _subscriptionCoversPath(subscription: SubscriptionEntry, eventPath: Path): boolean {
150146
const subPath = subscription.path;
151-
const eventPath = event.path;
152147
const depth = subscription.options.depth;
153-
const bubbles = event.bubbles !== false; // Default to true if not specified
154148

155-
// If event doesn't bubble, only match exact paths
156-
if (!bubbles) {
157-
return this._pathsAreEqual(eventPath, subPath);
158-
}
159-
160-
// Otherwise check if the event path starts with the subscription path
149+
// Check if the event path starts with the subscription path
161150
if (!this._pathStartsWith(eventPath, subPath)) {
162151
return false;
163152
}
@@ -194,15 +183,4 @@ export class PathObjectSubscriptionRegister {
194183

195184
return true;
196185
}
197-
198-
/**
199-
* Checks if two paths are exactly equal.
200-
*
201-
* @param path1 - First path to compare
202-
* @param path2 - Second path to compare
203-
* @returns true if paths are exactly equal
204-
*/
205-
private _pathsAreEqual(path1: Path, path2: Path): boolean {
206-
return this._client.Utils.arrEquals(path1, path2);
207-
}
208186
}

0 commit comments

Comments
 (0)