Skip to content

Fix: Code review create a seperate mechanism class instead of sub class #158

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rishabhabbu-ctrl merged 3 commits into from
Aug 6, 2026
Merged

Fix: Code review create a seperate mechanism class instead of sub class #158

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cac1c57
Code review create a seperate mechanism class instead of sub class
rishabhabbu-ctrl Aug 6, 2026
56ea301
Separated logic for V1 and V2 retry mechanism
AbhijeetMallick Aug 6, 2026
2dfcc78
Fix tests
rishabhabbu-ctrl Aug 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Clickstream.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
6926910B2FC59EF0006A48E0 /* CourierExpiryManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 692691092FC59EDF006A48E0 /* CourierExpiryManager.swift */; };
6926910D2FC5A151006A48E0 /* CourierEventCleanupManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6926910C2FC59EF7006A48E0 /* CourierEventCleanupManager.swift */; };
6926910E2FC5A151006A48E0 /* CourierEventCleanupManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6926910C2FC59EF7006A48E0 /* CourierEventCleanupManager.swift */; };
697B75833024637600E11219 /* CourierRetryMechanismV2.swift in Sources */ = {isa = PBXBuildFile; fileRef = 697B75823024636B00E11219 /* CourierRetryMechanismV2.swift */; };
697B75843024637600E11219 /* CourierRetryMechanismV2.swift in Sources */ = {isa = PBXBuildFile; fileRef = 697B75823024636B00E11219 /* CourierRetryMechanismV2.swift */; };
69CF74AF2E8CE5EC008438D0 /* EventSampler.swift in Sources */ = {isa = PBXBuildFile; fileRef = 69CF74AE2E8CE5E2008438D0 /* EventSampler.swift */; };
69CF74B02E8CE5EC008438D0 /* EventSampler.swift in Sources */ = {isa = PBXBuildFile; fileRef = 69CF74AE2E8CE5E2008438D0 /* EventSampler.swift */; };
69CF74B52E8CE616008438D0 /* EventSamplerConstants.swift in Sources */ = {isa = PBXBuildFile; fileRef = 69CF74B42E8CE60D008438D0 /* EventSamplerConstants.swift */; };
Expand Down Expand Up @@ -333,6 +335,7 @@
68E7BD1B2456E6F10072549A /* ClickstreamTests.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = ClickstreamTests.xctest; sourceTree = BUILT_PRODUCTS_DIR; };
692691092FC59EDF006A48E0 /* CourierExpiryManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CourierExpiryManager.swift; sourceTree = "<group>"; };
6926910C2FC59EF7006A48E0 /* CourierEventCleanupManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CourierEventCleanupManager.swift; sourceTree = "<group>"; };
697B75823024636B00E11219 /* CourierRetryMechanismV2.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CourierRetryMechanismV2.swift; sourceTree = "<group>"; };
69CF74AE2E8CE5E2008438D0 /* EventSampler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EventSampler.swift; sourceTree = "<group>"; };
69CF74B42E8CE60D008438D0 /* EventSamplerConstants.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EventSamplerConstants.swift; sourceTree = "<group>"; };
69CF74B72E8CE74F008438D0 /* EventSamplerConfiguration.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EventSamplerConfiguration.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1399,6 +1402,7 @@
D8F38D5F2E950C5D00C4281E /* CourierNetworkBuilder.swift */,
D8F38D602E950C5D00C4281E /* CourierNetworkService.swift */,
D8F38D612E950C5D00C4281E /* CourierRetryMechanism.swift */,
697B75823024636B00E11219 /* CourierRetryMechanismV2.swift */,
);
path = Courier;
sourceTree = "<group>";
Expand Down Expand Up @@ -1633,6 +1637,7 @@
BD6BDB7229FBC1360006B04A /* EventVisualizerLandingViewController.swift in Sources */,
9A0BBA8D2BC3CA5500F7DC01 /* PerformanceTracer.swift in Sources */,
D872F6042E8F8646006F077E /* WebsocketRetryMechanism.swift in Sources */,
697B75843024637600E11219 /* CourierRetryMechanismV2.swift in Sources */,
D872F6062E8F8646006F077E /* WebsocketNetworkService.swift in Sources */,
D8A7F7F52EC6D72D00DFC9AE /* ClickstreamCourierRetryPolicy.swift in Sources */,
D8F38D9C2E9E0DF300C4281E /* ClickstreamCourierUserCredentials.swift in Sources */,
Expand Down Expand Up @@ -1860,6 +1865,7 @@
BD6BDB8729FBC1360006B04A /* HealthTracker.swift in Sources */,
D8E4B89C2EA9EF940001D3D6 /* CourierHandlerTests.swift in Sources */,
BDE467832A5BD24500BFA976 /* NetworkReachabilityMock.swift in Sources */,
697B75833024637600E11219 /* CourierRetryMechanismV2.swift in Sources */,
D8F38D902E9CB08300C4281E /* QoS+Int.swift in Sources */,
9F6049202FC979B700190D5C /* CSBinaryEventTests.swift in Sources */,
D8F38D502E94CCD300C4281E /* ClickstreamNetworkOptionsTests.swift in Sources */,
Expand Down
1 change: 1 addition & 0 deletions ClickstreamTests/EventProcessorTests/EventProcessorDependenciesTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ class EventProcessorDependenciesTests: XCTestCase {

let courierNetworkBuilder = CourierNetworkBuilder(networkConfigs: config,
retryMech: courierRetryMech,
retryMechV2: nil,
performOnQueue: mockQueue)

let socketEventBatchCreator = DefaultEventBatchCreator(with: socketNetworkBuilder, performOnQueue: mockQueue)
Expand Down
1 change: 1 addition & 0 deletions ClickstreamTests/EventSchedulerTests/EventSchedulerDependenciesTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ class EventSchedulerDependenciesTests: XCTestCase {
courierNetworkBuilder = CourierNetworkBuilder(
networkConfigs: config,
retryMech: courierRetryMech,
retryMechV2: nil,
performOnQueue: mockQueue
)
}
Expand Down
36 changes: 25 additions & 11 deletions Sources/Clickstream/NetworkManager/Core/Dependencies/NetworkManagerDependencies.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,17 +86,19 @@ final class NetworkManagerDependencies {
keepAliveService: keepAliveService)
}()

private lazy var courierRetryMech: CourierRetryMechanism = {
CourierRetryMechanism(networkOptions: networkOptions,
private lazy var courierRetryMech: CourierRetryMechanism? = {
guard !networkOptions.enableCourierMechanismV2 else { return nil }
return CourierRetryMechanism(networkOptions: networkOptions,
networkService: courierNetworkService,
reachability: courierReachability,
appStateNotifier: courierAppStateNotifier,
performOnQueue: courierNetworkQueue,
persistence: courierPersistance)
}()

private lazy var courierRetryMechV2: CourierRetryMechV2 = {
CourierRetryMechV2(networkOptions: networkOptions,

private lazy var courierRetryMechV2: CourierRetryMechanismV2? = {
guard networkOptions.enableCourierMechanismV2 else { return nil }
return CourierRetryMechanismV2(networkOptions: networkOptions,
networkService: courierNetworkService,
reachability: courierReachability,
appStateNotifier: courierAppStateNotifier,
Expand All @@ -116,7 +118,8 @@ final class NetworkManagerDependencies {

func makeCourierNetworkBuilder() -> CourierNetworkBuilder {
CourierNetworkBuilder(networkConfigs: getNetworkConfig(),
retryMech: networkOptions.enableCourierMechanismV2 ? courierRetryMechV2 : courierRetryMech,
retryMech: courierRetryMech,
retryMechV2: courierRetryMechV2,
performOnQueue: courierNetworkQueue)
}

Expand All @@ -133,13 +136,24 @@ final class NetworkManagerDependencies {
authProvider: IConnectionServiceProvider,
pubSubAnalytics: ICourierEventHandler?) {

courierRetryMech.configureIdentifiers(with: identifiers,
topic: topic,
authProvider: authProvider,
pubSubAnalytics: pubSubAnalytics)
if let courierRetryMechV2 {
courierRetryMechV2.configureIdentifiers(with: identifiers,
topic: topic,
authProvider: authProvider,
pubSubAnalytics: pubSubAnalytics)
} else {
courierRetryMech?.configureIdentifiers(with: identifiers,
topic: topic,
authProvider: authProvider,
pubSubAnalytics: pubSubAnalytics)
}
}

func removeClientIdentifiers() {
courierRetryMech.removeIdentifiers()
if let courierRetryMechV2 {
courierRetryMechV2.removeIdentifiers()
} else {
courierRetryMech?.removeIdentifiers()
}
}
}
38 changes: 28 additions & 10 deletions Sources/Clickstream/NetworkManager/Courier/CourierNetworkBuilder.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,24 @@ final class CourierNetworkBuilder: NetworkBuildable {
typealias BatchType = CourierEventBatch

private let networkConfigs: NetworkConfigurable
private let retryMech: CourierRetryMechanism
private let retryMech: CourierRetryMechanism?
private let retryMechV2: CourierRetryMechanismV2?
private let performQueue: SerialQueue

var isAvailable: Bool {
return retryMech.isAvailble
if let retryMechV2 {
return retryMechV2.isAvailble
}
return retryMech?.isAvailble ?? false
}

init(networkConfigs: NetworkConfigurable,
retryMech: CourierRetryMechanism,
retryMech: CourierRetryMechanism?,
retryMechV2: CourierRetryMechanismV2?,
performOnQueue: SerialQueue) {
self.networkConfigs = networkConfigs
self.retryMech = retryMech
self.retryMechV2 = retryMechV2
self.performQueue = performOnQueue
}
}
Expand All @@ -51,7 +57,11 @@ extension CourierNetworkBuilder {

eventRequest.eventCount = eventBatch.events.count
eventRequest.qos = (eventBatch as? CourierEventBatch)?.qos
checkedSelf.retryMech.trackBatch(with: eventRequest)
if let retryMechV2 = checkedSelf.retryMechV2 {
retryMechV2.trackBatch(with: eventRequest)
} else {
checkedSelf.retryMech?.trackBatch(with: eventRequest)
}

#if EVENT_VISUALIZER_ENABLED
/// Update status of the event batch to sent to network
Expand All @@ -74,13 +84,21 @@ extension CourierNetworkBuilder {
}

func openConnectionForcefully() {
retryMech.openConnectionForcefully()
if let retryMechV2 {
retryMechV2.openConnectionForcefully()
} else {
retryMech?.openConnectionForcefully()
}
}

func stopTracking() {
performQueue.async { [weak self] in
performQueue.async { [weak self] in
guard let checkedSelf = self else { return }
checkedSelf.retryMech.stopTracking()
if let retryMechV2 = checkedSelf.retryMechV2 {
retryMechV2.stopTracking()
} else {
checkedSelf.retryMech?.stopTracking()
}
}
}
}
Expand Down
48 changes: 4 additions & 44 deletions Sources/Clickstream/NetworkManager/Courier/CourierRetryMechanism.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,19 @@
import Foundation
import CourierCore

class CourierRetryMechanism: Retryable {
final class CourierRetryMechanism: Retryable {

typealias EventRequestType = CourierEventRequest

private var networkOptions: ClickstreamNetworkOptions
private var reachability: NetworkReachability
private let networkService: NetworkService
fileprivate let performQueue: SerialQueue
private let performQueue: SerialQueue
private let appStateNotifier: AppStateNotifierService
private var terminationCountDown: DispatchSourceTimer?
private var networkServiceState: ConnectableState?
private var persistence: DefaultDatabaseDAO<CourierEventRequest>
fileprivate var retryTimer: DispatchSourceTimer?
private var retryTimer: DispatchSourceTimer?
private var identifiers: ClickstreamClientIdentifiers?
private var authProvider: IConnectionServiceProvider?
private var pubSubAnalytics: ICourierEventHandler?
Expand Down Expand Up @@ -457,7 +457,7 @@ extension CourierRetryMechanism {
retryTimer = nil
}

fileprivate func retryFailedBatches() {
private func retryFailedBatches() {
guard isAvailble && isCourierConnectable else {
stopObservingFailedBatches()
return
Expand Down Expand Up @@ -607,43 +607,3 @@ extension CourierRetryMechanism: ICourierEventHandler {
return eventRequest.reqGuid
}
}

final class CourierRetryMechV2: CourierRetryMechanism {
/// Serializes every access to `retryTimer`. The timer is created, cancelled and
/// nil-ed from several execution contexts — the Courier connection-status callback
/// thread (`connectionStatusListener`), the reachability / teardown paths
/// (`stopTracking` runs from `deinit`) and the timer's own event handler on
/// `performQueue`. Without this lock those accesses race, over-releasing the
/// `DispatchSourceTimer` and crashing in `_os_object_retain`.
private let retryTimerLock = NSLock()
}

extension CourierRetryMechV2 {

private func startObservingFailedBatches() {
retryTimerLock.lock()
defer { retryTimerLock.unlock() }

guard retryTimer == nil else { return }
// Build and start the timer atomically under the lock so a concurrent
// stopObservingFailedBatches() can never release a half-configured or
// still-suspended source.
let timer = DispatchSource.makeTimerSource(flags: .strict, queue: performQueue)
timer.schedule(deadline: .now() + Clickstream.courierConfigurations.maxRequestAckTimeout,
repeating: Clickstream.courierConfigurations.maxRequestAckTimeout)
timer.setEventHandler(handler: { [weak self] in
guard let checkedSelf = self else { return }
checkedSelf.retryFailedBatches()
})
retryTimer = timer
timer.resume()
}

private func stopObservingFailedBatches() {
retryTimerLock.lock()
defer { retryTimerLock.unlock() }

retryTimer?.cancel()
retryTimer = nil
}
}
Loading
Loading