Skip to content

Commit 3e2df11

Browse files
Maintain same root object when resetting the ObjectsPool
The correct behaviour wasn't clear from the spec when I wrote cb427d8, but new spec PR [1] makes it seem that this is the right thing to do (still needs clarifying though). [1] ably/specification#346
1 parent 2ee2c79 commit 3e2df11

File tree

4 files changed

+31
-8
lines changed

4 files changed

+31
-8
lines changed

Sources/AblyLiveObjects/Internal/DefaultLiveMap.swift

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,13 @@ internal final class InternalDefaultLiveMap: Sendable {
287287
}
288288
}
289289

290+
/// Resets the map's data, per RTO4b2. This is to be used when an `ATTACHED` ProtocolMessage indicates that the only object in a channel is an empty root map.
291+
internal func resetData() {
292+
mutex.withLock {
293+
mutableState.resetData()
294+
}
295+
}
296+
290297
// MARK: - Mutable state and the operations that affect it
291298

292299
private struct MutableState {
@@ -544,6 +551,12 @@ internal final class InternalDefaultLiveMap: Sendable {
544551
logger: logger,
545552
)
546553
}
554+
555+
/// Resets the map's data, per RTO4b2. This is to be used when an `ATTACHED` ProtocolMessage indicates that the only object in a channel is an empty root map.
556+
internal mutating func resetData() {
557+
// RTO4b2
558+
data = [:]
559+
}
547560
}
548561

549562
// MARK: - Helper Methods

Sources/AblyLiveObjects/Internal/ObjectsPool.swift

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,4 +209,16 @@ internal struct ObjectsPool {
209209

210210
logger.log("applySyncObjectsPool completed. Pool now contains \(entries.count) objects", level: .debug)
211211
}
212+
213+
/// Removes all entries except the root, and clears the root's data. This is to be used when an `ATTACHED` ProtocolMessage indicates that the only object in a channel is an empty root map, per RTO4b.
214+
internal mutating func reset() {
215+
let root = root
216+
217+
// RTO4b1
218+
entries = [Self.rootKey: .map(root)]
219+
220+
// RTO4b2
221+
// TODO: this one is unclear (are we meant to replace the root or just clear its data?) https://github.com/ably/specification/pull/333/files#r2183493458. I believe that the answer is that we should just clear its data but the spec point needs to be clearer, see https://github.com/ably/specification/pull/346/files#r2201434895.
222+
root.resetData()
223+
}
212224
}

Sources/AblyLiveObjects/InternalDefaultRealtimeObjects.swift

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -244,8 +244,7 @@ internal final class InternalDefaultRealtimeObjects: Sendable, LiveMapObjectPool
244244
}
245245

246246
// RTO4b1, RTO4b2: Reset the ObjectsPool to have a single empty root object
247-
// TODO: this one is unclear (are we meant to replace the root or just clear its data?) https://github.com/ably/specification/pull/333/files#r2183493458
248-
objectsPool = .init(logger: logger)
247+
objectsPool.reset()
249248

250249
// I have, for now, not directly implemented the "perform the actions for object sync completion" of RTO4b4 since my implementation doesn't quite match the model given there; here you only have a SyncObjectsPool if you have an OBJECT_SYNC in progress, which you might not have upon receiving an ATTACHED. Instead I've just implemented what seem like the relevant side effects. Can revisit this if "the actions for object sync completion" get more complex.
251250

Tests/AblyLiveObjectsTests/InternalDefaultRealtimeObjectsTests.swift

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -383,10 +383,10 @@ struct InternalDefaultRealtimeObjectsTests {
383383
#expect(newPool.entries["map:existing@123"] == nil) // Should be removed
384384
#expect(newPool.entries["counter:existing@456"] == nil) // Should be removed
385385

386-
// Verify root is a new zero-valued map (RTO4b2)
387-
// TODO: this one is unclear (are we meant to replace the root or just clear its data?) https://github.com/ably/specification/pull/333/files#r2183493458
386+
// Verify root is the same object, but with data cleared (RTO4b2)
387+
// TODO: this one is unclear (are we meant to replace the root or just clear its data?) https://github.com/ably/specification/pull/333/files#r2183493458. I believe that the answer is that we should just clear its data but the spec point needs to be clearer, see https://github.com/ably/specification/pull/346/files#r2201434895.
388388
let newRoot = newPool.root
389-
#expect(newRoot as AnyObject !== originalPool.root as AnyObject) // Should be a new instance
389+
#expect(newRoot as AnyObject === originalPool.root as AnyObject) // Should be same instance
390390
#expect(newRoot.testsOnly_data.isEmpty) // Should be zero-valued (empty)
391391

392392
// RTO4b3, RTO4b4, RTO4b5: SyncObjectsPool must be cleared, sync sequence cleared, BufferedObjectOperations cleared
@@ -410,15 +410,14 @@ struct InternalDefaultRealtimeObjectsTests {
410410
realtimeObjects.onChannelAttached(hasObjects: false)
411411
#expect(realtimeObjects.testsOnly_onChannelAttachedHasObjects == false)
412412
let newPool = realtimeObjects.testsOnly_objectsPool
413-
#expect(newPool.root as AnyObject !== originalRoot as AnyObject)
413+
#expect(newPool.root as AnyObject === originalRoot as AnyObject)
414414
#expect(newPool.entries.count == 1)
415415

416416
// Third call with hasObjects = true again (should do nothing)
417-
let secondResetRoot = newPool.root
418417
realtimeObjects.onChannelAttached(hasObjects: true)
419418
#expect(realtimeObjects.testsOnly_onChannelAttachedHasObjects == true)
420419
let finalPool = realtimeObjects.testsOnly_objectsPool
421-
#expect(finalPool.root as AnyObject === secondResetRoot as AnyObject) // Should be unchanged
420+
#expect(finalPool.root as AnyObject === originalRoot as AnyObject) // Should be unchanged
422421
}
423422

424423
/// Test that sync sequence is properly discarded even with complex sync state

0 commit comments

Comments
 (0)