Skip to content
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.

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

Allow discontinuities without an error #150

Merged
merged 2 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 5 additions & 5 deletions Example/AblyChatExample/Mocks/MockClients.swift
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ actor MockMessages: Messages {
return message
}

func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo> {
func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo?> {
fatalError("Not yet implemented")
}
}
Expand Down Expand Up @@ -193,7 +193,7 @@ actor MockRoomReactions: RoomReactions {
.init(mockAsyncSequence: createSubscription())
}

func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo> {
func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo?> {
fatalError("Not yet implemented")
}
}
Expand Down Expand Up @@ -242,7 +242,7 @@ actor MockTyping: Typing {
}
}

func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo> {
func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo?> {
fatalError("Not yet implemented")
}
}
Expand Down Expand Up @@ -346,7 +346,7 @@ actor MockPresence: Presence {
.init(mockAsyncSequence: createSubscription())
}

func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo> {
func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo?> {
fatalError("Not yet implemented")
}
}
Expand Down Expand Up @@ -381,7 +381,7 @@ actor MockOccupancy: Occupancy {
OccupancyEvent(connections: 10, presenceMembers: 5)
}

func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo> {
func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo?> {
fatalError("Not yet implemented")
}
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/AblyChat/DefaultMessages.swift
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ internal final class DefaultMessages: Messages, EmitsDiscontinuities {
}

// (CHA-M7) Users may subscribe to discontinuity events to know when there’s been a break in messages that they need to resolve. Their listener will be called when a discontinuity event is triggered from the room lifecycle.
internal func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo> {
internal func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo?> {
await featureChannel.subscribeToDiscontinuities()
}

Expand Down
2 changes: 1 addition & 1 deletion Sources/AblyChat/DefaultOccupancy.swift
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ internal final class DefaultOccupancy: Occupancy, EmitsDiscontinuities {
}

// (CHA-O5) Users may subscribe to discontinuity events to know when there’s been a break in occupancy. Their listener will be called when a discontinuity event is triggered from the room lifecycle. For occupancy, there shouldn’t need to be user action as most channels will send occupancy updates regularly as clients churn.
internal func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo> {
internal func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo?> {
await featureChannel.subscribeToDiscontinuities()
}
}
2 changes: 1 addition & 1 deletion Sources/AblyChat/DefaultPresence.swift
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ internal final class DefaultPresence: Presence, EmitsDiscontinuities {
}

// (CHA-PR8) Users may subscribe to discontinuity events to know when there’s been a break in presence. Their listener will be called when a discontinuity event is triggered from the room lifecycle. For presence, there shouldn’t need to be user action as the underlying core SDK will heal the presence set.
internal func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo> {
internal func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo?> {
await featureChannel.subscribeToDiscontinuities()
}

Expand Down
8 changes: 4 additions & 4 deletions Sources/AblyChat/DefaultRoomLifecycleContributor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import Ably
internal actor DefaultRoomLifecycleContributor: RoomLifecycleContributor, EmitsDiscontinuities, CustomDebugStringConvertible {
internal nonisolated let channel: DefaultRoomLifecycleContributorChannel
internal nonisolated let feature: RoomFeature
private var discontinuitySubscriptions: [Subscription<ARTErrorInfo>] = []
private var discontinuitySubscriptions: [Subscription<ARTErrorInfo?>] = []

internal init(channel: DefaultRoomLifecycleContributorChannel, feature: RoomFeature) {
self.channel = channel
Expand All @@ -12,14 +12,14 @@ internal actor DefaultRoomLifecycleContributor: RoomLifecycleContributor, EmitsD

// MARK: - Discontinuities

internal func emitDiscontinuity(_ error: ARTErrorInfo) {
internal func emitDiscontinuity(_ error: ARTErrorInfo?) {
for subscription in discontinuitySubscriptions {
subscription.emit(error)
}
}

internal func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo> {
let subscription = Subscription<ARTErrorInfo>(bufferingPolicy: .unbounded)
internal func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo?> {
let subscription = Subscription<ARTErrorInfo?>(bufferingPolicy: .unbounded)
// TODO: clean up old subscriptions (https://github.com/ably-labs/ably-chat-swift/issues/36)
discontinuitySubscriptions.append(subscription)
return subscription
Expand Down
2 changes: 1 addition & 1 deletion Sources/AblyChat/DefaultRoomReactions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ internal final class DefaultRoomReactions: RoomReactions, EmitsDiscontinuities {
}

// (CHA-ER5) Users may subscribe to discontinuity events to know when there’s been a break in reactions that they need to resolve. Their listener will be called when a discontinuity event is triggered from the room lifecycle.
internal func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo> {
internal func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo?> {
await featureChannel.subscribeToDiscontinuities()
}

Expand Down
2 changes: 1 addition & 1 deletion Sources/AblyChat/DefaultTyping.swift
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ internal final class DefaultTyping: Typing {
}

// (CHA-T7) Users may subscribe to discontinuity events to know when there’s been a break in typing indicators. Their listener will be called when a discontinuity event is triggered from the room lifecycle. For typing, there shouldn’t need to be user action as the underlying core SDK will heal the presence set.
internal func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo> {
internal func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo?> {
await featureChannel.subscribeToDiscontinuities()
}

Expand Down
2 changes: 1 addition & 1 deletion Sources/AblyChat/EmitsDiscontinuities.swift
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import Ably

public protocol EmitsDiscontinuities {
func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo>
func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo?>
}
2 changes: 1 addition & 1 deletion Sources/AblyChat/RoomFeature.swift
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ internal struct DefaultFeatureChannel: FeatureChannel {
internal var contributor: DefaultRoomLifecycleContributor
internal var roomLifecycleManager: RoomLifecycleManager

internal func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo> {
internal func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo?> {
await contributor.subscribeToDiscontinuities()
}

Expand Down
31 changes: 13 additions & 18 deletions Sources/AblyChat/RoomLifecycleManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ internal protocol RoomLifecycleContributor: Identifiable, Sendable {
/// Informs the contributor that there has been a break in channel continuity, which it should inform library users about.
///
/// It is marked as `async` purely to make it easier to write mocks for this method (i.e. to use an actor as a mock).
func emitDiscontinuity(_ error: ARTErrorInfo) async
func emitDiscontinuity(_ error: ARTErrorInfo?) async
}

internal protocol RoomLifecycleManager: Sendable {
Expand Down Expand Up @@ -111,7 +111,7 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
#if DEBUG
internal init(
testsOnly_status status: Status? = nil,
testsOnly_pendingDiscontinuityEvents pendingDiscontinuityEvents: [Contributor.ID: [ARTErrorInfo]]? = nil,
testsOnly_pendingDiscontinuityEvents pendingDiscontinuityEvents: [Contributor.ID: [ARTErrorInfo?]]? = nil,
testsOnly_idsOfContributorsWithTransientDisconnectTimeout idsOfContributorsWithTransientDisconnectTimeout: Set<Contributor.ID>? = nil,
contributors: [Contributor],
logger: InternalLogger,
Expand All @@ -130,7 +130,7 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor

private init(
status: Status?,
pendingDiscontinuityEvents: [Contributor.ID: [ARTErrorInfo]]?,
pendingDiscontinuityEvents: [Contributor.ID: [ARTErrorInfo?]]?,
idsOfContributorsWithTransientDisconnectTimeout: Set<Contributor.ID>?,
contributors: [Contributor],
logger: InternalLogger,
Expand Down Expand Up @@ -263,7 +263,7 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
}

// TODO: Not clear whether there can be multiple or just one (asked in https://github.com/ably/specification/pull/200/files#r1781927850)
var pendingDiscontinuityEvents: [ARTErrorInfo] = []
var pendingDiscontinuityEvents: [ARTErrorInfo?] = []
var transientDisconnectTimeout: TransientDisconnectTimeout?
/// Whether a state change to `ATTACHED` has already been observed for this contributor.
var hasBeenAttached: Bool
Expand All @@ -279,7 +279,7 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor

init(
contributors: [Contributor],
pendingDiscontinuityEvents: [Contributor.ID: [ARTErrorInfo]],
pendingDiscontinuityEvents: [Contributor.ID: [ARTErrorInfo?]],
idsOfContributorsWithTransientDisconnectTimeout: Set<Contributor.ID>
) {
storage = contributors.reduce(into: [:]) { result, contributor in
Expand Down Expand Up @@ -397,7 +397,7 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
return subscription
}

internal func testsOnly_pendingDiscontinuityEvents(for contributor: Contributor) -> [ARTErrorInfo] {
internal func testsOnly_pendingDiscontinuityEvents(for contributor: Contributor) -> [ARTErrorInfo?] {
contributorAnnotations[contributor].pendingDiscontinuityEvents
}

Expand Down Expand Up @@ -441,19 +441,16 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
break
}

guard let reason = stateChange.reason else {
// TODO: Decide the right thing to do here (https://github.com/ably-labs/ably-chat-swift/issues/74)
preconditionFailure("State change event with resumed == false should have a reason")
}
let reason = stateChange.reason

if hasOperationInProgress {
// CHA-RL4a3
logger.log(message: "Recording pending discontinuity event for contributor \(contributor)", level: .info)
logger.log(message: "Recording pending discontinuity event \(String(describing: reason)) for contributor \(contributor)", level: .info)

contributorAnnotations[contributor].pendingDiscontinuityEvents.append(reason)
} else {
// CHA-RL4a4
logger.log(message: "Emitting discontinuity event for contributor \(contributor)", level: .info)
logger.log(message: "Emitting discontinuity event \(String(describing: reason)) for contributor \(contributor)", level: .info)

await contributor.emitDiscontinuity(reason)
}
Expand All @@ -464,12 +461,10 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
if hasOperationInProgress {
if !stateChange.resumed, hadAlreadyAttached {
// CHA-RL4b1
logger.log(message: "Recording pending discontinuity event for contributor \(contributor)", level: .info)

guard let reason = stateChange.reason else {
// TODO: Decide the right thing to do here (https://github.com/ably-labs/ably-chat-swift/issues/74)
preconditionFailure("Non-initial ATTACHED state change with resumed == false should have a reason")
}
let reason = stateChange.reason

logger.log(message: "Recording pending discontinuity event \(String(describing: reason)) for contributor \(contributor)", level: .info)

contributorAnnotations[contributor].pendingDiscontinuityEvents.append(reason)
}
Expand Down Expand Up @@ -860,7 +855,7 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
logger.log(message: "Emitting pending discontinuity events", level: .info)
for contributor in contributors {
for pendingDiscontinuityEvent in contributorAnnotations[contributor].pendingDiscontinuityEvents {
logger.log(message: "Emitting pending discontinuity event \(pendingDiscontinuityEvent) to contributor \(contributor)", level: .info)
logger.log(message: "Emitting pending discontinuity event \(String(describing: pendingDiscontinuityEvent)) to contributor \(contributor)", level: .info)
await contributor.emitDiscontinuity(pendingDiscontinuityEvent)
}
}
Expand Down
18 changes: 9 additions & 9 deletions Tests/AblyChatTests/DefaultRoomLifecycleManagerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ struct DefaultRoomLifecycleManagerTests {

private func createManager(
forTestingWhatHappensWhenCurrentlyIn status: DefaultRoomLifecycleManager<MockRoomLifecycleContributor>.Status? = nil,
forTestingWhatHappensWhenHasPendingDiscontinuityEvents pendingDiscontinuityEvents: [MockRoomLifecycleContributor.ID: [ARTErrorInfo]]? = nil,
forTestingWhatHappensWhenHasPendingDiscontinuityEvents pendingDiscontinuityEvents: [MockRoomLifecycleContributor.ID: [ARTErrorInfo?]]? = nil,
forTestingWhatHappensWhenHasTransientDisconnectTimeoutForTheseContributorIDs idsOfContributorsWithTransientDisconnectTimeout: Set<MockRoomLifecycleContributor.ID>? = nil,
contributors: [MockRoomLifecycleContributor] = [],
clock: SimpleClock = MockSimpleClock()
Expand Down Expand Up @@ -262,7 +262,7 @@ struct DefaultRoomLifecycleManagerTests {
func attach_uponSuccess_emitsPendingDiscontinuityEvents() async throws {
// Given: A DefaultRoomLifecycleManager, all of whose contributors’ calls to `attach` succeed
let contributors = (1 ... 3).map { _ in createContributor(attachBehavior: .success) }
let pendingDiscontinuityEvents: [MockRoomLifecycleContributor.ID: [ARTErrorInfo]] = [
let pendingDiscontinuityEvents: [MockRoomLifecycleContributor.ID: [ARTErrorInfo?]] = [
contributors[1].id: [.init(domain: "SomeDomain", code: 123) /* arbitrary */ ],
contributors[2].id: [.init(domain: "SomeDomain", code: 456) /* arbitrary */ ],
]
Expand Down Expand Up @@ -333,7 +333,7 @@ struct DefaultRoomLifecycleManagerTests {
current: .attached,
previous: .detached, // arbitrary
event: .attached,
reason: .createUnknownError() // Not related to this test, just to avoid a crash in CHA-RL4b1 handling of this state change
reason: nil // arbitrary
)
)
)
Expand Down Expand Up @@ -961,7 +961,7 @@ struct DefaultRoomLifecycleManagerTests {
current: .attached,
previous: .attaching, // arbitrary
event: .attached,
reason: .createUnknownError() // Not related to this test, just to avoid a crash in CHA-RL4b1 handling of this state change
reason: nil // arbitrary
)
)
),
Expand Down Expand Up @@ -1010,7 +1010,7 @@ struct DefaultRoomLifecycleManagerTests {
current: .attached,
previous: .attaching, // arbitrary
event: .attached,
reason: .createUnknownError() // Not related to this test, just to avoid a crash in CHA-RL4b1 handling of this state change
reason: nil // arbitrary
)
)
),
Expand Down Expand Up @@ -1203,7 +1203,7 @@ struct DefaultRoomLifecycleManagerTests {
current: .attached,
previous: .attaching, // arbitrary
event: .attached,
reason: .createUnknownError() // Not related to this test, just to avoid a crash in CHA-RL4b1 handling of this state change
reason: nil // arbitrary
)

return .addSubscriptionAndEmitStateChange(contributorAttachedStateChange)
Expand Down Expand Up @@ -1248,7 +1248,7 @@ struct DefaultRoomLifecycleManagerTests {
current: .attached,
previous: .attaching, // arbitrary
event: .attached,
reason: .createUnknownError() // Not related to this test, just to avoid a crash in CHA-RL4b1 handling of this state change
reason: nil // arbitrary
)
) // Not related to this test, just so that the CHA-RL5d wait completes
),
Expand Down Expand Up @@ -1296,7 +1296,7 @@ struct DefaultRoomLifecycleManagerTests {
current: .attached,
previous: .attaching, // arbitrary
event: .attached,
reason: .createUnknownError() // Not related to this test, just to avoid a crash in CHA-RL4b1 handling of this state change
reason: nil // arbitrary
)
) // Not related to this test, just so that the CHA-RL5d wait completes
),
Expand Down Expand Up @@ -1794,7 +1794,7 @@ struct DefaultRoomLifecycleManagerTests {
current: .attached,
previous: .detached, // arbitrary
event: .attached,
reason: .createUnknownError() // Not related to this test, just to avoid a crash in CHA-RL4b1 handling of this state change
reason: nil // arbitrary
)
)
)
Expand Down
8 changes: 4 additions & 4 deletions Tests/AblyChatTests/Mocks/MockFeatureChannel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import Ably
final actor MockFeatureChannel: FeatureChannel {
let channel: RealtimeChannelProtocol
// TODO: clean up old subscriptions (https://github.com/ably-labs/ably-chat-swift/issues/36)
private var discontinuitySubscriptions: [Subscription<ARTErrorInfo>] = []
private var discontinuitySubscriptions: [Subscription<ARTErrorInfo?>] = []
private let resultOfWaitToBeAbleToPerformPresenceOperations: Result<Void, ARTErrorInfo>?

init(
Expand All @@ -15,13 +15,13 @@ final actor MockFeatureChannel: FeatureChannel {
resultOfWaitToBeAbleToPerformPresenceOperations = resultOfWaitToBeAblePerformPresenceOperations
}

func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo> {
let subscription = Subscription<ARTErrorInfo>(bufferingPolicy: .unbounded)
func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo?> {
let subscription = Subscription<ARTErrorInfo?>(bufferingPolicy: .unbounded)
discontinuitySubscriptions.append(subscription)
return subscription
}

func emitDiscontinuity(_ discontinuity: ARTErrorInfo) {
func emitDiscontinuity(_ discontinuity: ARTErrorInfo?) {
for subscription in discontinuitySubscriptions {
subscription.emit(discontinuity)
}
Expand Down
4 changes: 2 additions & 2 deletions Tests/AblyChatTests/Mocks/MockRoomLifecycleContributor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ actor MockRoomLifecycleContributor: RoomLifecycleContributor {
nonisolated let feature: RoomFeature
nonisolated let channel: MockRoomLifecycleContributorChannel

private(set) var emitDiscontinuityArguments: [ARTErrorInfo] = []
private(set) var emitDiscontinuityArguments: [ARTErrorInfo?] = []

init(feature: RoomFeature, channel: MockRoomLifecycleContributorChannel) {
self.feature = feature
self.channel = channel
}

func emitDiscontinuity(_ error: ARTErrorInfo) async {
func emitDiscontinuity(_ error: ARTErrorInfo?) async {
emitDiscontinuityArguments.append(error)
}
}