Skip to content

Commit 3dd9ede

Browse files
lawrence-forooghianclaude
lawrence-forooghian
and
claude
committed
Get rid of the updateSelfLater pattern
In 392fae3 I tried to be clever and treat the subscription storage as part of the mutable state (i.e. by making it a struct) but this got complicated because it needs to be able to update itself later (when the user unsubscribes), leading to a lot of unpleasant boilerplate. Let's instead use the EventEmitter reference type copied from ably-swift in 3df3687 and ac04f3c. To limit the scope of the changes, I've tried to not change the SubscriptionStorage type — which is now just a bridge between the EventEmitter API and the public API — too much. I've also not changed the public API, even though we have #28 which suggests updating the "unsubscribing within a listener" public API to match that of the aforementioned ably-swift EventEmitter (this API is still not final there and I don't want to churn us over to using it yet). Resolves #102. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 3df3687 commit 3dd9ede

7 files changed

Lines changed: 151 additions & 265 deletions

File tree

‎Sources/AblyLiveObjects/Internal/InternalDefaultLiveCounter.swift‎

Lines changed: 9 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ internal final class InternalDefaultLiveCounter: Sendable {
5252
) {
5353
mutableStateMutex = .init(
5454
dispatchQueue: internalQueue,
55-
initialValue: .init(liveObjectMutableState: .init(objectID: objectID), data: data),
55+
initialValue: .init(liveObjectMutableState: .init(objectID: objectID, internalQueue: internalQueue), data: data),
5656
)
5757
self.logger = logger
5858
self.userCallbackQueue = userCallbackQueue
@@ -150,44 +150,26 @@ internal final class InternalDefaultLiveCounter: Sendable {
150150
@discardableResult
151151
internal func subscribe(listener: @escaping LiveObjectUpdateCallback<DefaultLiveCounterUpdate>, coreSDK: CoreSDK) throws(ARTErrorInfo) -> any SubscribeResponse {
152152
try mutableStateMutex.withSync { mutableState throws(ARTErrorInfo) in
153-
// swiftlint:disable:next trailing_closure
154-
try mutableState.liveObjectMutableState.nosync_subscribe(listener: listener, coreSDK: coreSDK, updateSelfLater: { [weak self] action in
155-
guard let self else {
156-
return
157-
}
158-
159-
mutableStateMutex.withSync { mutableState in
160-
action(&mutableState.liveObjectMutableState)
161-
}
162-
})
153+
try mutableState.liveObjectMutableState.nosync_subscribe(listener: listener, coreSDK: coreSDK)
163154
}
164155
}
165156

166157
internal func unsubscribeAll() {
167158
mutableStateMutex.withSync { mutableState in
168-
mutableState.liveObjectMutableState.unsubscribeAll()
159+
mutableState.liveObjectMutableState.nosync_unsubscribeAll()
169160
}
170161
}
171162

172163
@discardableResult
173164
internal func on(event: LiveObjectLifecycleEvent, callback: @escaping LiveObjectLifecycleEventCallback) -> any OnLiveObjectLifecycleEventResponse {
174165
mutableStateMutex.withSync { mutableState in
175-
// swiftlint:disable:next trailing_closure
176-
mutableState.liveObjectMutableState.on(event: event, callback: callback, updateSelfLater: { [weak self] action in
177-
guard let self else {
178-
return
179-
}
180-
181-
mutableStateMutex.withSync { mutableState in
182-
action(&mutableState.liveObjectMutableState)
183-
}
184-
})
166+
mutableState.liveObjectMutableState.nosync_on(event: event, callback: callback)
185167
}
186168
}
187169

188170
internal func offAll() {
189171
mutableStateMutex.withSync { mutableState in
190-
mutableState.liveObjectMutableState.offAll()
172+
mutableState.liveObjectMutableState.nosync_offAll()
191173
}
192174
}
193175

@@ -198,7 +180,7 @@ internal final class InternalDefaultLiveCounter: Sendable {
198180
/// This is used to instruct this counter to emit updates during an `OBJECT_SYNC`.
199181
internal func nosync_emit(_ update: LiveObjectUpdate<DefaultLiveCounterUpdate>) {
200182
mutableStateMutex.withoutSync { mutableState in
201-
mutableState.liveObjectMutableState.emit(update, on: userCallbackQueue)
183+
mutableState.liveObjectMutableState.nosync_emit(update, on: userCallbackQueue)
202184
}
203185
}
204186

@@ -425,14 +407,14 @@ internal final class InternalDefaultLiveCounter: Sendable {
425407
logger: logger,
426408
)
427409
// RTLC7d1a
428-
liveObjectMutableState.emit(update, on: userCallbackQueue)
410+
liveObjectMutableState.nosync_emit(update, on: userCallbackQueue)
429411
// RTLC7d1b
430412
return true
431413
case .known(.counterInc):
432414
// RTLC7d5
433415
let update = applyCounterIncOperation(operation.counterInc)
434416
// RTLC7d5a
435-
liveObjectMutableState.emit(update, on: userCallbackQueue)
417+
liveObjectMutableState.nosync_emit(update, on: userCallbackQueue)
436418
// RTLC7d5b
437419
return true
438420
case .known(.objectDelete):
@@ -447,7 +429,7 @@ internal final class InternalDefaultLiveCounter: Sendable {
447429
)
448430

449431
// RTLC7d4a
450-
liveObjectMutableState.emit(.update(.init(amount: -dataBeforeApplyingOperation)), on: userCallbackQueue)
432+
liveObjectMutableState.nosync_emit(.update(.init(amount: -dataBeforeApplyingOperation)), on: userCallbackQueue)
451433
// RTLC7d4b
452434
return true
453435
default:

‎Sources/AblyLiveObjects/Internal/InternalDefaultLiveMap.swift‎

Lines changed: 12 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ internal final class InternalDefaultLiveMap: Sendable {
7878
) {
7979
mutableStateMutex = .init(
8080
dispatchQueue: internalQueue,
81-
initialValue: .init(liveObjectMutableState: .init(objectID: objectID), data: data, semantics: semantics),
81+
initialValue: .init(liveObjectMutableState: .init(objectID: objectID, internalQueue: internalQueue), data: data, semantics: semantics),
8282
)
8383
self.logger = logger
8484
self.userCallbackQueue = userCallbackQueue
@@ -232,44 +232,26 @@ internal final class InternalDefaultLiveMap: Sendable {
232232
@discardableResult
233233
internal func subscribe(listener: @escaping LiveObjectUpdateCallback<DefaultLiveMapUpdate>, coreSDK: CoreSDK) throws(ARTErrorInfo) -> any SubscribeResponse {
234234
try mutableStateMutex.withSync { mutableState throws(ARTErrorInfo) in
235-
// swiftlint:disable:next trailing_closure
236-
try mutableState.liveObjectMutableState.nosync_subscribe(listener: listener, coreSDK: coreSDK, updateSelfLater: { [weak self] action in
237-
guard let self else {
238-
return
239-
}
240-
241-
mutableStateMutex.withSync { mutableState in
242-
action(&mutableState.liveObjectMutableState)
243-
}
244-
})
235+
try mutableState.liveObjectMutableState.nosync_subscribe(listener: listener, coreSDK: coreSDK)
245236
}
246237
}
247238

248239
internal func unsubscribeAll() {
249240
mutableStateMutex.withSync { mutableState in
250-
mutableState.liveObjectMutableState.unsubscribeAll()
241+
mutableState.liveObjectMutableState.nosync_unsubscribeAll()
251242
}
252243
}
253244

254245
@discardableResult
255246
internal func on(event: LiveObjectLifecycleEvent, callback: @escaping LiveObjectLifecycleEventCallback) -> any OnLiveObjectLifecycleEventResponse {
256247
mutableStateMutex.withSync { mutableState in
257-
// swiftlint:disable:next trailing_closure
258-
mutableState.liveObjectMutableState.on(event: event, callback: callback, updateSelfLater: { [weak self] action in
259-
guard let self else {
260-
return
261-
}
262-
263-
mutableStateMutex.withSync { mutableState in
264-
action(&mutableState.liveObjectMutableState)
265-
}
266-
})
248+
mutableState.liveObjectMutableState.nosync_on(event: event, callback: callback)
267249
}
268250
}
269251

270252
internal func offAll() {
271253
mutableStateMutex.withSync { mutableState in
272-
mutableState.liveObjectMutableState.offAll()
254+
mutableState.liveObjectMutableState.nosync_offAll()
273255
}
274256
}
275257

@@ -280,7 +262,7 @@ internal final class InternalDefaultLiveMap: Sendable {
280262
/// This is used to instruct this map to emit updates during an `OBJECT_SYNC`.
281263
internal func nosync_emit(_ update: LiveObjectUpdate<DefaultLiveMapUpdate>) {
282264
mutableStateMutex.withoutSync { mutableState in
283-
mutableState.liveObjectMutableState.emit(update, on: userCallbackQueue)
265+
mutableState.liveObjectMutableState.nosync_emit(update, on: userCallbackQueue)
284266
}
285267
}
286268

@@ -663,7 +645,7 @@ internal final class InternalDefaultLiveMap: Sendable {
663645
clock: clock,
664646
)
665647
// RTLM15d1a
666-
liveObjectMutableState.emit(update, on: userCallbackQueue)
648+
liveObjectMutableState.nosync_emit(update, on: userCallbackQueue)
667649
// RTLM15d1b
668650
return true
669651
case .known(.mapSet):
@@ -688,7 +670,7 @@ internal final class InternalDefaultLiveMap: Sendable {
688670
clock: clock,
689671
)
690672
// RTLM15d6a
691-
liveObjectMutableState.emit(update, on: userCallbackQueue)
673+
liveObjectMutableState.nosync_emit(update, on: userCallbackQueue)
692674
// RTLM15d6b
693675
return true
694676
case .known(.mapRemove):
@@ -705,7 +687,7 @@ internal final class InternalDefaultLiveMap: Sendable {
705687
clock: clock,
706688
)
707689
// RTLM15d7a
708-
liveObjectMutableState.emit(update, on: userCallbackQueue)
690+
liveObjectMutableState.nosync_emit(update, on: userCallbackQueue)
709691
// RTLM15d7b
710692
return true
711693
case .known(.objectDelete):
@@ -720,7 +702,7 @@ internal final class InternalDefaultLiveMap: Sendable {
720702
)
721703

722704
// RTLM15d5a
723-
liveObjectMutableState.emit(.update(.init(update: dataBeforeApplyingOperation.mapValues { _ in .removed })), on: userCallbackQueue)
705+
liveObjectMutableState.nosync_emit(.update(.init(update: dataBeforeApplyingOperation.mapValues { _ in .removed })), on: userCallbackQueue)
724706
// RTLM15d5b
725707
return true
726708
case .known(.mapClear):
@@ -729,7 +711,7 @@ internal final class InternalDefaultLiveMap: Sendable {
729711
serial: applicableOperation.objectMessageSerial,
730712
)
731713
// RTLM15d8a
732-
liveObjectMutableState.emit(update, on: userCallbackQueue)
714+
liveObjectMutableState.nosync_emit(update, on: userCallbackQueue)
733715
// RTLM15d8b
734716
return true
735717
default:
@@ -950,7 +932,7 @@ internal final class InternalDefaultLiveMap: Sendable {
950932

951933
// RTO4b2a
952934
let mapUpdate = DefaultLiveMapUpdate(update: previousData.mapValues { _ in .removed })
953-
liveObjectMutableState.emit(.update(mapUpdate), on: userCallbackQueue)
935+
liveObjectMutableState.nosync_emit(.update(mapUpdate), on: userCallbackQueue)
954936
}
955937

956938
/// Needed for ``InternalLiveObject`` conformance.

‎Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift‎

Lines changed: 28 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ internal final class InternalDefaultRealtimeObjects: Sendable, InternalRealtimeO
135135
userCallbackQueue: userCallbackQueue,
136136
clock: clock,
137137
),
138+
internalQueue: internalQueue,
138139
garbageCollectionGracePeriod: garbageCollectionOptions.gracePeriod,
139140
),
140141
)
@@ -322,40 +323,22 @@ internal final class InternalDefaultRealtimeObjects: Sendable, InternalRealtimeO
322323
@discardableResult
323324
internal func on(event: ObjectsEvent, callback: @escaping ObjectsEventCallback) -> any OnObjectsEventResponse {
324325
mutableStateMutex.withSync { mutableState in
325-
// swiftlint:disable:next trailing_closure
326-
mutableState.on(event: event, callback: callback, updateSelfLater: { [weak self] action in
327-
guard let self else {
328-
return
329-
}
330-
331-
mutableStateMutex.withSync { mutableState in
332-
action(&mutableState)
333-
}
334-
})
326+
mutableState.nosync_on(event: event, callback: callback)
335327
}
336328
}
337329

338330
/// Adds a subscriber to the ``internalObjectsEventSubscriptionStorage`` (i.e. unaffected by `offAll()`).
339331
@discardableResult
340332
internal func onInternal(event: ObjectsEvent, callback: @escaping ObjectsEventCallback) -> any OnObjectsEventResponse {
341-
// TODO: Looking at this again later the whole process for adding a subscriber is really verbose and boilerplate-y, and I think the unfortunate result of me trying to be clever at some point; revisit in https://github.com/ably/ably-liveobjects-swift-plugin/issues/102
333+
// TODO: make the RTO20 wait-for-synced use this mechanism (https://github.com/ably/ably-liveobjects-swift-plugin/issues/120)
342334
mutableStateMutex.withSync { mutableState in
343-
// swiftlint:disable:next trailing_closure
344-
mutableState.onInternal(event: event, callback: callback, updateSelfLater: { [weak self] action in
345-
guard let self else {
346-
return
347-
}
348-
349-
mutableStateMutex.withSync { mutableState in
350-
action(&mutableState)
351-
}
352-
})
335+
mutableState.nosync_onInternal(event: event, callback: callback)
353336
}
354337
}
355338

356339
internal func offAll() {
357340
mutableStateMutex.withSync { mutableState in
358-
mutableState.offAll()
341+
mutableState.nosync_offAll()
359342
}
360343
}
361344

@@ -643,10 +626,10 @@ internal final class InternalDefaultRealtimeObjects: Sendable, InternalRealtimeO
643626
private struct MutableState {
644627
internal var objectsPool: ObjectsPool
645628
internal var onChannelAttachedHasObjects: Bool?
646-
internal var objectsEventSubscriptionStorage = SubscriptionStorage<ObjectsEvent, Void>()
629+
internal let objectsEventSubscriptionStorage: SubscriptionStorage<ObjectsEvent, Void>
647630

648631
/// Used when the object wishes to subscribe to its own events (i.e. unaffected by `offAll()`); used e.g. to wait for a sync before returning from `getRoot()`, per RTO1c.
649-
internal var internalObjectsEventSubscriptionStorage = SubscriptionStorage<ObjectsEvent, Void>()
632+
internal let internalObjectsEventSubscriptionStorage: SubscriptionStorage<ObjectsEvent, Void>
650633

651634
/// The RTO10b grace period for which we will retain tombstoned objects and map entries.
652635
internal var garbageCollectionGracePeriod: GarbageCollectionOptions.GracePeriod
@@ -678,6 +661,17 @@ internal final class InternalDefaultRealtimeObjects: Sendable, InternalRealtimeO
678661
/// The RTO17 sync state. Also stores the sync sequence data.
679662
internal var state = State.initialized
680663

664+
init(
665+
objectsPool: ObjectsPool,
666+
internalQueue: DispatchQueue,
667+
garbageCollectionGracePeriod: GarbageCollectionOptions.GracePeriod,
668+
) {
669+
self.objectsPool = objectsPool
670+
self.garbageCollectionGracePeriod = garbageCollectionGracePeriod
671+
objectsEventSubscriptionStorage = SubscriptionStorage(internalQueue: internalQueue)
672+
internalObjectsEventSubscriptionStorage = SubscriptionStorage(internalQueue: internalQueue)
673+
}
674+
681675
/// Has the same cases as `ObjectsSyncState` but with associated data to store the sync sequence data and represent the constraint that you only have a sync sequence if you're SYNCING.
682676
internal enum State {
683677
case initialized
@@ -726,7 +720,7 @@ internal final class InternalDefaultRealtimeObjects: Sendable, InternalRealtimeO
726720
return
727721
}
728722
// RTO17b
729-
emitObjectsEvent(event, on: userCallbackQueue)
723+
nosync_emitObjectsEvent(event, on: userCallbackQueue)
730724
}
731725

732726
internal mutating func nosync_onChannelAttached(
@@ -1018,45 +1012,28 @@ internal final class InternalDefaultRealtimeObjects: Sendable, InternalRealtimeO
10181012
}
10191013
}
10201014

1021-
internal typealias UpdateMutableState = @Sendable (_ action: (inout Self) -> Void) -> Void
1022-
10231015
@discardableResult
1024-
internal mutating func on(event: ObjectsEvent, callback: @escaping ObjectsEventCallback, updateSelfLater: @escaping UpdateMutableState) -> any OnObjectsEventResponse {
1025-
let updateSubscriptionStorage: SubscriptionStorage<ObjectsEvent, Void>.UpdateSubscriptionStorage = { action in
1026-
updateSelfLater { mutableState in
1027-
action(&mutableState.objectsEventSubscriptionStorage)
1028-
}
1029-
}
1030-
1031-
let subscription = objectsEventSubscriptionStorage.subscribe(
1016+
internal func nosync_on(event: ObjectsEvent, callback: @escaping ObjectsEventCallback) -> any OnObjectsEventResponse {
1017+
let subscription = objectsEventSubscriptionStorage.nosync_subscribe(
10321018
listener: { _, subscriptionInCallback in
10331019
let response = ObjectsEventResponse(subscription: subscriptionInCallback)
10341020
callback(response)
10351021
},
10361022
eventName: event,
1037-
updateSelfLater: updateSubscriptionStorage,
10381023
)
10391024

10401025
return ObjectsEventResponse(subscription: subscription)
10411026
}
10421027

10431028
/// Adds a subscriber to the ``internalObjectsEventSubscriptionStorage`` (i.e. unaffected by `offAll()`).
10441029
@discardableResult
1045-
internal mutating func onInternal(event: ObjectsEvent, callback: @escaping ObjectsEventCallback, updateSelfLater: @escaping UpdateMutableState) -> any OnObjectsEventResponse {
1046-
// TODO: Looking at this again later the whole process for adding a subscriber is really verbose and boilerplate-y, and I think the unfortunate result of me trying to be clever at some point; revisit in https://github.com/ably/ably-liveobjects-swift-plugin/issues/102. Also as things stand we end up not being able to use this method because we run into Swift exclusivity violations when we try to unsubscribe from within a listener that's invoked when the mutable state mutex is already held (see https://github.com/ably/ably-liveobjects-swift-plugin/issues/120), so e.g. the RTO20 wait-for-synced can't use this mechanism, which it should be able to.
1047-
let updateSubscriptionStorage: SubscriptionStorage<ObjectsEvent, Void>.UpdateSubscriptionStorage = { action in
1048-
updateSelfLater { mutableState in
1049-
action(&mutableState.internalObjectsEventSubscriptionStorage)
1050-
}
1051-
}
1052-
1053-
let subscription = internalObjectsEventSubscriptionStorage.subscribe(
1030+
internal func nosync_onInternal(event: ObjectsEvent, callback: @escaping ObjectsEventCallback) -> any OnObjectsEventResponse {
1031+
let subscription = internalObjectsEventSubscriptionStorage.nosync_subscribe(
10541032
listener: { _, subscriptionInCallback in
10551033
let response = ObjectsEventResponse(subscription: subscriptionInCallback)
10561034
callback(response)
10571035
},
10581036
eventName: event,
1059-
updateSelfLater: updateSubscriptionStorage,
10601037
)
10611038

10621039
return ObjectsEventResponse(subscription: subscription)
@@ -1071,13 +1048,13 @@ internal final class InternalDefaultRealtimeObjects: Sendable, InternalRealtimeO
10711048
}
10721049
}
10731050

1074-
internal mutating func offAll() {
1075-
objectsEventSubscriptionStorage.unsubscribeAll()
1051+
internal func nosync_offAll() {
1052+
objectsEventSubscriptionStorage.nosync_unsubscribeAll()
10761053
}
10771054

1078-
internal func emitObjectsEvent(_ event: ObjectsEvent, on queue: DispatchQueue) {
1079-
objectsEventSubscriptionStorage.emit(eventName: event, on: queue)
1080-
internalObjectsEventSubscriptionStorage.emit(eventName: event, on: queue)
1055+
internal func nosync_emitObjectsEvent(_ event: ObjectsEvent, on queue: DispatchQueue) {
1056+
objectsEventSubscriptionStorage.nosync_emit(eventName: event, on: queue)
1057+
internalObjectsEventSubscriptionStorage.nosync_emit(eventName: event, on: queue)
10811058
}
10821059
}
10831060
}

‎Sources/AblyLiveObjects/Internal/InternalLiveObject.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ internal extension InternalLiveObject {
3737
// Emit the deleted lifecycle event
3838
// Taken from https://github.com/ably/ably-js/blob/e280bff11a4a7627362c5185e764b7ebd0490570/src/plugins/objects/liveobject.ts#L168
3939
// TODO: Bring in line with spec once it exists (https://github.com/ably/ably-liveobjects-swift-plugin/issues/77)
40-
liveObjectMutableState.emitLifecycleEvent(.deleted, on: userCallbackQueue)
40+
liveObjectMutableState.nosync_emitLifecycleEvent(.deleted, on: userCallbackQueue)
4141
}
4242

4343
/// Applies an `OBJECT_DELETE` operation, per RTLO5.

0 commit comments

Comments
 (0)