Changes for proposal "Mocking Centrifuge Client for Testing Without a Server"

[shalom-aviv]

Pull Request Description

This Pull Request introduces changes aimed at improving the testability and flexibility of the Centrifuge client for iOS, written in Swift. The main focus is on introducing protocols for key entities and optimizing their usage. Additional information is here Proposal: Mocking Centrifuge Client for Testing Without a Server

Key Changes:

1. Protocols for Core Classes:

   • Introduced the Client protocol for the CentrifugeClient class.
   • Introduced the ClientSubscription protocol for the CentrifugeSubscription class.
   • These protocols replicate the current interfaces of their respective classes, enabling interaction through abstractions.

2. Integration Example Updates:

   • Replaced the usage of CentrifugeClient and CentrifugeSubscription classes in the integration example with their corresponding Client and ClientSubscription protocols.
   • This demonstrates the ability to work through interfaces, simplifying testing and making implementation replacements easier.

3. Public Initializers for Models:

   • All models (e.g., events, errors) used by the Centrifuge client now have public initializers.
   • This is necessary for creating instances of objects in client code, such as during testing.

4. Property Optimization:

   • Properties that are immutable after initialization have been converted from var to let.
   • This improves code readability and safety. If changes are required, clients can create new structures using the public initializers.

5. Demonstration of Protocol Usage:

   • Added an example demonstrating forced subscription to a channel when the application state changes (disconnect/connect). This shows that the protocol-based implementation is fully functional and compatible.

Suggestions for Future Improvements:

• Consider fully transitioning to interaction through protocols instead of direct access to SDK classes.
• Define more descriptive names for the protocols to better convey their purpose.
• Reevaluate the approach to creating the Centrifuge client (e.g., using a factory instead of the current static constructor).

Notes:

All changes have been made with backward compatibility in mind. The existing logic of the client and subscriptions remains unchanged. This Pull Request simplifies Centrifuge integration into client projects and makes testing easier.

Demo project:

changes.demo.mp4

[shalom-aviv]

Hi, @FZambia

What do you think about this pull request? ))

[FZambia]

Hmm.. the complexity added here is sufficient and I need more time to think and more opinions on that.

Probably this may be addressed in alternative ways by mocking transport level and thus triggering required events, or by keeping protocols outside, by wrapping on app level or wrapper library level?

Is your main motivation to test your app or add more tests to centrifuge-swift? If it's application tests – then I guess it's already possible to do without introducing protocols, just constructors for types maybe. If testing centrifuge-swift itself – then I like protocol mock idea more.

[shalom-aviv]

Hmm.. the complexity added here is sufficient and I need more time to think and more opinions on that.

Probably this may be addressed in alternative ways by mocking transport level and thus triggering required events, or by keeping protocols outside, by wrapping on app level or wrapper library level?

Is your main motivation to test your app or add more tests to centrifuge-swift? If it's application tests – then I guess it's already possible to do without introducing protocols, just constructors for types maybe. If testing centrifuge-swift itself – then I like protocol mock idea more.

Hi

Thank you for your feedback and suggestions. I wanted to clarify a few points about the changes in my merge request and the reasoning behind them.

1. Public constructors for structures:
   While adding public constructors for structures is certainly helpful and simplifies some parts of mocking, it’s insufficient for the use case I’m targeting. One key issue lies with the CentrifugeSubscription, which is a class containing business logic. Allowing clients to create this object directly via a public initializer would be unsafe and might lead to unexpected behaviors. This is why I believe introducing a protocol for CentrifugeSubscription is a more robust approach, as it enables safe abstraction while maintaining control over the logic encapsulated within the subscription.
2. Tests and goals of the MR:
   Could you clarify which tests for the library you were referring to? My merge request is primarily aimed at simplifying mocking for client applications, not adding tests directly to centrifuge-swift. Currently, mocking the Centrifuge client in my app is achieved via an intermediate layer that:

• Fully duplicates the Centrifuge client’s interface.
• Maps Centrifuge structures to similar application-level types.
• Proxies delegate calls from Centrifuge to the app-level delegate.
  While functional, this adds unnecessary complexity to production code. Adding protocols and public constructors to centrifuge-swift would allow us to eliminate this intermediate layer entirely, streamlining the implementation and reducing technical debt in our app.

3. Proposed compromise:
   I understand your concerns about the added complexity, so here’s a possible compromise. I could remove the protocol for the Centrifuge client itself (as I can create and use one in the client app if necessary). However, I strongly believe that keeping a protocol for CentrifugeSubscription is the correct approach. This would allow for safer and more flexible mocking of subscriptions without exposing unsafe initializers or requiring significant changes to the library’s internal logic.

Let me know your thoughts on this. I’d be happy to adjust the implementation if there’s a better way to address these concerns.

[shalom-aviv]

@FZambia is too much ???