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

[ECO-4760] Min iOS 14 + Idiomatic Public Swift Interface #1955

Open
wants to merge 23 commits into
base: main
Choose a base branch
from

Conversation

umair-ably
Copy link
Contributor

@umair-ably umair-ably commented Aug 5, 2024

  • Increases the minimum supported iOS version to be iOS 14
  • Majority of the changes provide a more idiomatic public Swift interface utilising NS_SWIFT_NAME
  • There's also a new build step which copies any required linking headers to the correct path for Swift/Obj-C mixed projects. Leaving this in as it does not run when there are no Swift files, but it'll make revisiting the Swift migration work easier.
  • Any lines including NS_EXTENSION_UNAVAILABLE are also required for mixed language framework targets. Also leaving this in so we can see if there are any side effects from this change well ahead of further migration work.
  • Tests have also been updated to be in line with the new interface

Summary by CodeRabbit

  • New Features

    • Enhanced Swift compatibility with improved naming conventions for enumerations and interfaces.
    • Updated the library's API to a more modern structure, enhancing clarity and maintainability.
  • Bug Fixes

    • Enhanced error handling by transitioning to a unified error handling approach.
  • Documentation

    • Updated deployment targets for iOS and tvOS from version 10.0 to 14.0 to leverage newer features.
  • Refactor

    • Comprehensive refactoring of client and message types to remove the ART prefix, streamlining the codebase.
  • Tests

    • Adjusted tests to accommodate new naming conventions and ensure continued functionality.

Introduces Swift interoperability
…kes the entire public interface more idiomatic for Swift
# Conflicts:
#	Test/Tests/ARTDefaultTests.swift
#	Test/Tests/ClientInformationTests.swift
#	Test/Tests/PushActivationStateMachineTests.swift
#	Test/Tests/RealtimeClientChannelTests.swift
#	Test/Tests/RealtimeClientConnectionTests.swift
#	Test/Tests/RealtimeClientPresenceTests.swift
#	Test/Tests/RealtimeClientTests.swift
@umair-ably umair-ably self-assigned this Aug 5, 2024
@github-actions github-actions bot temporarily deployed to staging/pull/1955/features August 5, 2024 12:22 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1955/jazzydoc August 5, 2024 12:26 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1955/features August 5, 2024 13:51 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1955/jazzydoc August 5, 2024 13:54 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1955/features August 5, 2024 14:10 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1955/jazzydoc August 5, 2024 14:15 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1955/features August 5, 2024 14:24 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1955/features August 5, 2024 14:28 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1955/jazzydoc August 5, 2024 14:31 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1955/features August 5, 2024 14:42 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1955/jazzydoc August 5, 2024 14:46 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1955/features August 5, 2024 14:48 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1955/jazzydoc August 5, 2024 14:51 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1955/features August 6, 2024 00:21 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1955/jazzydoc August 6, 2024 00:25 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1955/jazzydoc August 6, 2024 10:34 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1955/features August 6, 2024 10:37 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1955/jazzydoc August 6, 2024 10:40 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1955/features August 6, 2024 10:40 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1955/jazzydoc August 6, 2024 10:45 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1955/features August 6, 2024 11:19 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1955/jazzydoc August 6, 2024 11:23 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1955/features August 6, 2024 12:01 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1955/jazzydoc August 6, 2024 12:04 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1955/features August 6, 2024 13:07 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1955/jazzydoc August 6, 2024 13:10 Inactive
Copy link
Collaborator

@maratal maratal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, left a few naming refinements suggestions. Also changes in the project file need to be inspected for necessity.

@@ -3856,13 +3887,19 @@
PRODUCT_BUNDLE_IDENTIFIER = "io.ably.$(PRODUCT_NAME)";
PRODUCT_NAME = Ably;
PROVISIONING_PROFILE_SPECIFIER = "ably-cocoa-soak-test";
STRIP_SWIFT_SYMBOLS = NO;
SUPPORTED_PLATFORMS = "iphoneos iphonesimulator";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this is intentional change or just was commited together with other changes.

Source/ARTURLSessionServerTrust.m Show resolved Hide resolved
Source/PrivateHeaders/Ably/ARTNSMutableRequest+ARTRest.h Outdated Show resolved Hide resolved
ARTEncoderFormatJson,
ARTEncoderFormatMsgPack
};
ARTEncoderFormatJson NS_SWIFT_NAME(json),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it how swift translates those names by default?

Copy link
Contributor Author

@umair-ably umair-ably Aug 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't have thought so, how would it know how to?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By subtracting ARTEncoderFormat from ARTEncoderFormatJson and make it lowercase. You can see this in the tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahh yes I see it for the log level one too... no idea how it knows how to do this though!

I'd rather leave this code in as it also acts as documentation - it makes it much easier to see intended behaviour from a glance alone

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no idea how it knows how to do this though!

As I said above it's just cutting the prefix of the enum name out, not that hard =) @lawrence-forooghian wdyt

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i get that bit haha I meant where is the setting so it knows to do this, and why does it not just drop the ART prefix for everything in Swift?

That would be a rude thing to do by default 😆

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ARTLogLevel and ARTEncoderFormat are also two distinct prefixes, but it still handles them

What do you mean? It does it for every enum.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this makes it clearer. It knows which prefix to drop because we define our enums using the macro

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in 41589f4

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aren't there more of those enums?

Source/include/Ably/ARTLog.h Outdated Show resolved Hide resolved
Test/Tests/LogAdapterTests.swift Outdated Show resolved Hide resolved
Copy link

coderabbitai bot commented Aug 28, 2024

Walkthrough

The changes involve a comprehensive refactoring of the Ably client library, replacing ART prefixed types with simplified names across multiple files. This includes modifications to client options, error handling, message types, and connection management. Additionally, deployment targets for iOS and tvOS have been updated to support newer versions, ensuring compatibility with modern APIs and features. The overall structure and logic of the existing codebase remain intact while enhancing clarity and maintainability.

Changes

Files/Paths Change Summary
Ably-SoakTest-AppUITests/SoakTest.swift Refactored SoakTest class, replacing ARTClientOptions with ClientOptions, and updated real-time functionality types.
Ably-SoakTest-AppUITests/SoakTestReachability.swift Changed logger parameter type in SoakTestReachability initializer from ARTLog to InternalLog.
Ably-SoakTest-AppUITests/SoakTestURLSession.swift Updated cancellation handling types from [ARTCancellable] to [Cancellable], and modified method return types accordingly.
Ably-SoakTest-AppUITests/SoakTestWebSocket.swift Renamed various types related to messaging and connections, including ARTProtocolMessage to ProtocolMessage, enhancing clarity.
Ably.podspec Updated iOS and tvOS deployment targets from '10.0' to '14.0'.
Ably.xcodeproj/project.pbxproj Adjusted build settings, including IPHONEOS_DEPLOYMENT_TARGET to '14.0' and updated LastSwiftMigration values.
Examples/AblyPush/AblyPushExample/AblyHelper.swift Replaced ART prefixed types with simplified names, particularly in push notification handling.
Examples/AblyPush/AblyPushExample/App.swift Updated method calls from ARTPush to Push in remote notification registration.
Examples/AblyPush/AblyPushExample/ContentView.swift Changed state variable types from ARTDeviceDetails and ARTErrorInfo to DeviceDetails and ErrorInfo.
Examples/SPM/Package.swift Updated minimum iOS deployment target from version 9.0 to 14.0.
Examples/SPM/Tests/SPMTests/SPMTests.swift Refactored client options initialization, changing from ARTClientOptions to ClientOptions.
Examples/Tests/Podfile Updated iOS platform version from '10.0' to '14.0' and added a post_install hook for build settings.
Test/Test Utilities/TestUtilities.swift Refactored various types to remove ART prefixes, enhancing readability and maintainability.
Test/Tests/RealtimeClientTests.swift Renamed ART prefixed types to their simplified counterparts, maintaining functionality in tests.
Test/Tests/RestClientChannelTests.swift Updated client and channel types from ARTRest to Rest, and related types for consistency.
Test/Tests/RestClientPresenceTests.swift Refactored presence-related classes and types from ART prefixed to simplified names.
Test/Tests/RestClientStatsTests.swift Transitioned from using ARTStats and related types to Stats and its counterparts.
Test/Tests/RestClientTests.swift Renamed ARTClientOptions to ClientOptions and ARTRest to Rest, updating tests accordingly.
Test/Tests/StatsTests.swift Replaced ARTStats and related types with Stats, ensuring tests align with new naming conventions.
Test/Tests/StringifiableTests.swift Renamed ARTStringifiable to Stringifiable, maintaining test functionality.
Test/Tests/UtilitiesTests.swift Refactored utility tests to remove ART prefixes from types, enhancing clarity.
Source/include/Ably/ARTStatus.h Enhanced Swift compatibility with NS_SWIFT_NAME annotations for enumerations and interfaces, improving usability in Swift codebases.

Poem

🐇 In the meadow where rabbits play,
We hop and cheer for changes today!
With names so clear, our paths align,
Simplified ways, oh, how they shine!
From ART to ease, we bound with glee,
A brighter code, as happy as can be! 🌼


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

# Conflicts:
#	Test/Tests/AuthTests.swift
#	Test/Tests/RealtimeClientChannelTests.swift
#	Test/Tests/RestClientTests.swift
@github-actions github-actions bot temporarily deployed to staging/pull/1955/features August 28, 2024 10:35 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1955/jazzydoc August 28, 2024 10:39 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1955/features August 28, 2024 11:09 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1955/jazzydoc August 28, 2024 11:12 Inactive
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 13

Outside diff range, codebase verification and nitpick comments (9)
Test/Tests/ObjectLifetimesTests.swift (2)

48-48: Inconsistent Class Renaming Detected

The renaming of classes to align with the updated naming conventions is inconsistent. The following files still contain instances of the old class names ARTRealtimeChannel and ARTConnection:

  • Test/Tests/RealtimeClientTests.swift
  • Test/Tests/RealtimeClientConnectionTests.swift
  • Test/Tests/ObjectLifetimesTests.swift
  • Test/Test Utilities/TestUtilities.swift
  • Ably-SoakTest-AppUITests/SoakTestWebSocket.swift

Please update these occurrences to maintain consistency across the codebase.

Analysis chain

Consistent Renaming Across Multiple Classes

The renaming of various classes (ARTRealtimeChannel to RealtimeChannel, ARTConnection to Connection, etc.) is consistent with the library's updated naming conventions. This change enhances clarity and should be checked for consistency in all related files.

The changes are approved, but ensure that all related files are updated to reflect these new class names.

Also applies to: 58-58, 83-83, 105-105

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all instances of the old class names are updated.

# Test: Search for the old class names. Expect: No occurrences.
rg --type swift -A 5 $'ARTRealtimeChannel|ARTConnection'

Length of output: 5009


20-20: The output from the shell script indicates that there are several instances of ARTRealtime still present in the codebase, specifically in multiple files such as Test/Tests/RealtimeClientPresenceTests.swift, Test/Tests/RealtimeClientConnectionTests.swift, Test/Tests/ObjectLifymedTests.swift, Test/Test Utilities/TestUtilities.swift, Test/Test Utilities/TestProxyTransportFactory.swift, and Ably-SoakTest-AppUITyleTests/SoateWebSocket.swift.

This confirms that the renaming from ARTRealtime to Realtime has not been fully implemented across all files. Therefore, the review comment's suggestion to ensure consistency across the codebase has not been fully executed.

Here's a follow-up action based on this information:

Ensure Complete Renaming from ARTRealtime to Realtime

The renaming from ARTRealtime to Realtime is incomplete. Please update all instances of ARTRealtime to Realtime in the following files:

  • Test/Tests/RealtimeClientPresenceTests.swift
  • Test/Tests/RealtimeClientConnectionTests.swift
  • Test/Tests/ObjectLifymedTests.swift
  • Test/Test Utilities/TestUtilities.swift
  • Test/Test Utilities/TestProxyTransportFactory.swift
  • Ably-SoakTest-AppUITyleTests/SoateWebSocket.swift

Please ensure that all references are updated to maintain consistency across the codebase.

Analysis chain

Class Name Updated to Realtime

The renaming of ARTRealtime to Realtime is part of the library's effort to simplify and modernize its naming convention. Ensure that this change is consistently applied across all files where ARTRealtime was previously used.

The change is approved, but a thorough search should be conducted to verify that all instances of ARTRealtime are updated.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all instances of `ARTRealtime` are updated to `Realtime`.

# Test: Search for the old class name. Expect: No occurrences.
rg --type swift -A 5 $'ARTRealtime'

Length of output: 10634

Test/Tests/RealtimeClientTests.swift (2)

51-51: Code Quality: Direct validation against a constant should be avoided.

It's generally a good practice to avoid direct comparisons against constant values in tests as it can lead to brittle tests if the constants change. Consider using a variable or a function to obtain the expected version dynamically.


1289-1289: Error Handling: Ensure proper setup in asynchronous test.

The test setup within a URLSession data task might lead to unexpected behavior if the setup fails. It's generally better to separate network calls from test setups to ensure that tests are not affected by external dependencies.

Consider initializing the Realtime client outside of the URLSession completion handler to decouple the network response from the test setup.

Test/Test Utilities/TestUtilities.swift (3)

414-414: Use Swift's optional comparison directly.

The function for comparing JsonCompatible objects can be simplified by directly comparing optional values in Swift, which supports optional comparison out of the box.

Simplify the comparison:

func ==(lhs: JsonCompatible?, rhs: JsonCompatible?) -> Bool {
    return lhs?.toJSON() == rhs?.toJSON()
}

432-432: Redundant initialization of optional variables.

The variables completion and error in PublishTestMessage are redundantly initialized to nil, which is unnecessary in Swift as optionals are nil by default.

Remove the redundant initializations:

var completion: ((ErrorInfo?) -> Void)?
var error: ErrorInfo?

Also applies to: 433-433

Tools
SwiftLint

[Warning] 432-432: Initializing an optional variable with nil is redundant

(redundant_optional_initialization)


484-484: Use Swift syntactic sugar for optionals.

The functions publishTestMessage and publishFirstTestMessage use the long form Optional<Int> instead of the shorthand Int?.

Use shorthand for optionals:

@discardableResult func publishTestMessage(_ rest: Rest, channelName: String, completion: Optional<(ErrorInfo?)->()>) -> PublishTestMessage {
    return PublishTestMessage(client: rest, channelName: channelName, failOnError: false, completion: completion)
}

Also applies to: 494-494

Tools
SwiftLint

[Warning] 484-484: Shorthand syntactic sugar should be used, i.e. Int? instead of Optional

(syntactic_sugar)

Test/Tests/RealtimeClientConnectionTests.swift (1)

Line range hint 5403-5426: Consider handling potential nil values gracefully.

The test function uses force unwrapping in several places, which could lead to runtime crashes if the values are nil. Consider using optional binding or providing default values to enhance the robustness of the test.

Consider refactoring the force unwraps to prevent potential runtime crashes:

guard let transport = client.internal.transport as? TestProxyTransport else {
    XCTFail("TestProxyTransport is not set"); return
}
guard let originalConnectedMessage = transport.protocolMessagesReceived.filter({ $0.action == .connected }).first else {
    XCTFail("First CONNECTED message not received"); return
}
Test/Tests/PushAdminTests.swift (1)

Line range hint 33-33: Consider adding tests for new Swift-specific features or changes.

While the renaming and refactoring are well-handled, it would be beneficial to add specific tests that verify the correct functioning of the new Swift-specific features or any other significant changes made in this update.

Would you like me to help by proposing some specific test cases or opening a GitHub issue to track this task?

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 55ad795 and 41589f4.

Files selected for processing (105)
  • Ably-SoakTest-AppUITests/SoakTest.swift (6 hunks)
  • Ably-SoakTest-AppUITests/SoakTestReachability.swift (1 hunks)
  • Ably-SoakTest-AppUITests/SoakTestURLSession.swift (2 hunks)
  • Ably-SoakTest-AppUITests/SoakTestWebSocket.swift (14 hunks)
  • Ably.podspec (1 hunks)
  • Ably.xcodeproj/project.pbxproj (21 hunks)
  • Examples/AblyPush/AblyPushExample/AblyHelper.swift (9 hunks)
  • Examples/AblyPush/AblyPushExample/App.swift (1 hunks)
  • Examples/AblyPush/AblyPushExample/ContentView.swift (1 hunks)
  • Examples/SPM/Package.swift (1 hunks)
  • Examples/SPM/Tests/SPMTests/SPMTests.swift (1 hunks)
  • Examples/Tests/Podfile (2 hunks)
  • Examples/Tests/Tests.xcodeproj/project.pbxproj (4 hunks)
  • Examples/Tests/TestsTests/TestsTests.swift (3 hunks)
  • Package.swift (1 hunks)
  • Source/ARTPush.m (1 hunks)
  • Source/ARTPushActivationStateMachine.m (1 hunks)
  • Source/ARTTypes.m (2 hunks)
  • Source/ARTURLSessionServerTrust.m (1 hunks)
  • Source/PrivateHeaders/Ably/ARTConstants.h (1 hunks)
  • Source/PrivateHeaders/Ably/ARTDataEncoder.h (2 hunks)
  • Source/PrivateHeaders/Ably/ARTLocalDeviceStorage.h (1 hunks)
  • Source/PrivateHeaders/Ably/ARTNSMutableRequest+ARTRest.h (1 hunks)
  • Source/PrivateHeaders/Ably/ARTPush+Private.h (1 hunks)
  • Source/PrivateHeaders/Ably/ARTPushActivationEvent.h (4 hunks)
  • Source/PrivateHeaders/Ably/ARTPushActivationState.h (4 hunks)
  • Source/PrivateHeaders/Ably/ARTPushActivationStateMachine.h (1 hunks)
  • Source/PrivateHeaders/Ably/ARTPushAdmin+Private.h (1 hunks)
  • Source/PrivateHeaders/Ably/ARTPushChannel+Private.h (1 hunks)
  • Source/PrivateHeaders/Ably/ARTPushChannelSubscriptions+Private.h (1 hunks)
  • Source/PrivateHeaders/Ably/ARTPushDeviceRegistrations+Private.h (1 hunks)
  • Source/SocketRocket/Internal/Proxy/ARTSRProxyConnect.m (1 hunks)
  • Source/SocketRocket/Internal/Utilities/ARTSRURLUtilities.m (1 hunks)
  • Source/include/Ably/ARTAuth.h (2 hunks)
  • Source/include/Ably/ARTBaseMessage.h (1 hunks)
  • Source/include/Ably/ARTChannelOptions.h (1 hunks)
  • Source/include/Ably/ARTClientOptions.h (1 hunks)
  • Source/include/Ably/ARTConnection.h (1 hunks)
  • Source/include/Ably/ARTCrypto.h (4 hunks)
  • Source/include/Ably/ARTDataQuery.h (2 hunks)
  • Source/include/Ably/ARTDefault.h (1 hunks)
  • Source/include/Ably/ARTDeviceDetails.h (1 hunks)
  • Source/include/Ably/ARTDeviceIdentityTokenDetails.h (1 hunks)
  • Source/include/Ably/ARTDevicePushDetails.h (1 hunks)
  • Source/include/Ably/ARTDeviceStorage.h (1 hunks)
  • Source/include/Ably/ARTEncoder.h (1 hunks)
  • Source/include/Ably/ARTEventEmitter.h (2 hunks)
  • Source/include/Ably/ARTHTTPPaginatedResponse.h (1 hunks)
  • Source/include/Ably/ARTLocalDevice.h (1 hunks)
  • Source/include/Ably/ARTLog.h (1 hunks)
  • Source/include/Ably/ARTMessage.h (1 hunks)
  • Source/include/Ably/ARTPaginatedResult.h (1 hunks)
  • Source/include/Ably/ARTPresence.h (1 hunks)
  • Source/include/Ably/ARTPresenceMessage.h (1 hunks)
  • Source/include/Ably/ARTProtocolMessage.h (2 hunks)
  • Source/include/Ably/ARTPush.h (3 hunks)
  • Source/include/Ably/ARTPushAdmin.h (2 hunks)
  • Source/include/Ably/ARTPushChannel.h (2 hunks)
  • Source/include/Ably/ARTPushChannelSubscription.h (1 hunks)
  • Source/include/Ably/ARTPushChannelSubscriptions.h (2 hunks)
  • Source/include/Ably/ARTPushDeviceRegistrations.h (2 hunks)
  • Source/include/Ably/ARTRealtime.h (2 hunks)
  • Source/include/Ably/ARTRealtimeChannel.h (1 hunks)
  • Source/include/Ably/ARTRealtimeChannelOptions.h (1 hunks)
  • Source/include/Ably/ARTRealtimeChannels.h (2 hunks)
  • Source/include/Ably/ARTRealtimePresence.h (3 hunks)
  • Source/include/Ably/ARTRest.h (2 hunks)
  • Source/include/Ably/ARTRestChannel.h (2 hunks)
  • Source/include/Ably/ARTRestChannels.h (2 hunks)
  • Source/include/Ably/ARTRestPresence.h (3 hunks)
  • Source/include/Ably/ARTStats.h (9 hunks)
  • Source/include/Ably/ARTStatus.h (3 hunks)
  • Source/include/Ably/ARTStringifiable.h (1 hunks)
  • Source/include/Ably/ARTTypes.h (13 hunks)
  • Test/Test Utilities/MockDeviceStorage.swift (2 hunks)
  • Test/Test Utilities/MockErrorChecker.swift (1 hunks)
  • Test/Test Utilities/MockInternalLogCore.swift (1 hunks)
  • Test/Test Utilities/MockVersion2Log.swift (1 hunks)
  • Test/Test Utilities/TestProxyTransportFactory.swift (1 hunks)
  • Test/Test Utilities/TestUtilities.swift (36 hunks)
  • Test/Tests/ARTDefaultTests.swift (1 hunks)
  • Test/Tests/AuthTests.swift (177 hunks)
  • Test/Tests/ClientInformationTests.swift (2 hunks)
  • Test/Tests/CryptoTests.swift (10 hunks)
  • Test/Tests/DefaultErrorCheckerTests.swift (4 hunks)
  • Test/Tests/DefaultInternalLogCoreTests.swift (3 hunks)
  • Test/Tests/DeltaCodecTests.swift (6 hunks)
  • Test/Tests/LogAdapterTests.swift (2 hunks)
  • Test/Tests/ObjectLifetimesTests.swift (7 hunks)
  • Test/Tests/PushActivationStateMachineTests.swift (52 hunks)
  • Test/Tests/PushAdminTests.swift (42 hunks)
  • Test/Tests/PushChannelTests.swift (10 hunks)
  • Test/Tests/PushTests.swift (26 hunks)
  • Test/Tests/ReadmeExamplesTests.swift (11 hunks)
  • Test/Tests/RealtimeClientChannelsTests.swift (7 hunks)
  • Test/Tests/RealtimeClientConnectionTests.swift (141 hunks)
  • Test/Tests/RealtimeClientTests.swift (68 hunks)
  • Test/Tests/RestClientChannelTests.swift (46 hunks)
  • Test/Tests/RestClientChannelsTests.swift (7 hunks)
  • Test/Tests/RestClientPresenceTests.swift (21 hunks)
  • Test/Tests/RestClientStatsTests.swift (20 hunks)
  • Test/Tests/RestClientTests.swift (90 hunks)
  • Test/Tests/StatsTests.swift (9 hunks)
  • Test/Tests/StringifiableTests.swift (9 hunks)
  • Test/Tests/UtilitiesTests.swift (10 hunks)
Files skipped from review due to trivial changes (44)
  • Examples/AblyPush/AblyPushExample/App.swift
  • Source/PrivateHeaders/Ably/ARTConstants.h
  • Source/PrivateHeaders/Ably/ARTDataEncoder.h
  • Source/PrivateHeaders/Ably/ARTLocalDeviceStorage.h
  • Source/PrivateHeaders/Ably/ARTNSMutableRequest+ARTRest.h
  • Source/PrivateHeaders/Ably/ARTPush+Private.h
  • Source/PrivateHeaders/Ably/ARTPushActivationStateMachine.h
  • Source/PrivateHeaders/Ably/ARTPushAdmin+Private.h
  • Source/PrivateHeaders/Ably/ARTPushChannel+Private.h
  • Source/PrivateHeaders/Ably/ARTPushChannelSubscriptions+Private.h
  • Source/PrivateHeaders/Ably/ARTPushDeviceRegistrations+Private.h
  • Source/include/Ably/ARTAuth.h
  • Source/include/Ably/ARTBaseMessage.h
  • Source/include/Ably/ARTChannelOptions.h
  • Source/include/Ably/ARTClientOptions.h
  • Source/include/Ably/ARTConnection.h
  • Source/include/Ably/ARTCrypto.h
  • Source/include/Ably/ARTDefault.h
  • Source/include/Ably/ARTDeviceDetails.h
  • Source/include/Ably/ARTDeviceIdentityTokenDetails.h
  • Source/include/Ably/ARTDevicePushDetails.h
  • Source/include/Ably/ARTDeviceStorage.h
  • Source/include/Ably/ARTHTTPPaginatedResponse.h
  • Source/include/Ably/ARTLocalDevice.h
  • Source/include/Ably/ARTLog.h
  • Source/include/Ably/ARTMessage.h
  • Source/include/Ably/ARTPresence.h
  • Source/include/Ably/ARTPush.h
  • Source/include/Ably/ARTPushChannelSubscription.h
  • Source/include/Ably/ARTPushChannelSubscriptions.h
  • Source/include/Ably/ARTPushDeviceRegistrations.h
  • Source/include/Ably/ARTRealtime.h
  • Source/include/Ably/ARTRealtimeChannel.h
  • Source/include/Ably/ARTRealtimeChannels.h
  • Source/include/Ably/ARTRealtimePresence.h
  • Source/include/Ably/ARTRest.h
  • Source/include/Ably/ARTRestChannel.h
  • Source/include/Ably/ARTRestChannels.h
  • Source/include/Ably/ARTRestPresence.h
  • Source/include/Ably/ARTStringifiable.h
  • Source/include/Ably/ARTTypes.h
  • Test/Tests/ClientInformationTests.swift
  • Test/Tests/ReadmeExamplesTests.swift
  • Test/Tests/StringifiableTests.swift
Additional context used
SwiftLint
Examples/Tests/TestsTests/TestsTests.swift

[Warning] 66-66: Prefer at least one space after slashes for comments

(comment_spacing)


[Warning] 76-76: Prefer at least one space after slashes for comments

(comment_spacing)

Test/Tests/RealtimeClientChannelsTests.swift

[Error] 33-33: Force casts should be avoided

(force_cast)

Test/Tests/RestClientChannelsTests.swift

[Error] 124-124: Force casts should be avoided

(force_cast)

Test/Tests/CryptoTests.swift

[Error] 153-153: Force casts should be avoided

(force_cast)


[Error] 176-176: Force casts should be avoided

(force_cast)


[Error] 217-217: Force casts should be avoided

(force_cast)


[Error] 238-238: Force casts should be avoided

(force_cast)

Test/Tests/StatsTests.swift

[Error] 11-11: Force tries should be avoided

(force_try)


[Error] 12-12: Force tries should be avoided

(force_try)


[Error] 20-20: Force tries should be avoided

(force_try)


[Error] 21-21: Force tries should be avoided

(force_try)


[Error] 40-40: Force tries should be avoided

(force_try)


[Error] 48-48: Force tries should be avoided

(force_try)


[Error] 49-49: Force tries should be avoided

(force_try)


[Error] 56-56: Force tries should be avoided

(force_try)


[Error] 57-57: Force tries should be avoided

(force_try)


[Error] 86-86: Force tries should be avoided

(force_try)


[Error] 191-191: Force tries should be avoided

(force_try)


[Error] 312-312: Force tries should be avoided

(force_try)


[Error] 386-386: Force tries should be avoided

(force_try)

Test/Tests/PushTests.swift

[Warning] 410-410: Prefer using UTF-8 encoded strings when converting between String and Data

(non_optional_string_data_conversion)

Test/Tests/RealtimeClientTests.swift

[Error] 340-340: Force casts should be avoided

(force_cast)


[Error] 1532-1532: Force casts should be avoided

(force_cast)

Test/Test Utilities/TestUtilities.swift

[Error] 185-185: Force casts should be avoided

(force_cast)


[Warning] 432-432: Initializing an optional variable with nil is redundant

(redundant_optional_initialization)


[Warning] 433-433: Initializing an optional variable with nil is redundant

(redundant_optional_initialization)


[Warning] 484-484: Shorthand syntactic sugar should be used, i.e. Int? instead of Optional

(syntactic_sugar)


[Warning] 494-494: Shorthand syntactic sugar should be used, i.e. Int? instead of Optional

(syntactic_sugar)


[Error] 1698-1698: Force casts should be avoided

(force_cast)

Test/Tests/PushActivationStateMachineTests.swift

[Error] 616-616: Force casts should be avoided

(force_cast)

Test/Tests/AuthTests.swift

[Error] 1016-1016: Force casts should be avoided

(force_cast)


[Error] 3011-3011: Force casts should be avoided

(force_cast)


[Error] 3220-3220: Force casts should be avoided

(force_cast)


[Error] 3235-3235: Force casts should be avoided

(force_cast)


[Error] 3285-3285: Force casts should be avoided

(force_cast)


[Error] 3298-3298: Force casts should be avoided

(force_cast)


[Error] 3322-3322: Force casts should be avoided

(force_cast)


[Error] 3901-3901: Force casts should be avoided

(force_cast)


[Error] 3954-3954: Force tries should be avoided

(force_try)


[Error] 4042-4042: Force tries should be avoided

(force_try)

Test/Tests/RealtimeClientConnectionTests.swift

[Error] 102-102: Force tries should be avoided

(force_try)


[Error] 103-103: Force tries should be avoided

(force_try)


[Error] 139-139: Force casts should be avoided

(force_cast)


[Error] 1671-1671: Force casts should be avoided

(force_cast)


[Error] 1768-1768: Force casts should be avoided

(force_cast)


[Error] 2615-2615: Force casts should be avoided

(force_cast)


[Error] 2644-2644: Force casts should be avoided

(force_cast)


[Error] 2658-2658: Force casts should be avoided

(force_cast)


[Error] 5232-5232: Force casts should be avoided

(force_cast)

Additional comments not posted (320)
Test/Test Utilities/MockErrorChecker.swift (1)

6-6: Verify compatibility of ErrorInfo with previous error handling.

The change from ARTErrorInfo to ErrorInfo in the isTokenError method should be verified for compatibility and sufficiency in providing necessary error details.

Test/Tests/ARTDefaultTests.swift (1)

8-9: Verify consistency of renaming from ARTDefault to Default.

Ensure that the renaming of ARTDefault to Default is consistently applied across the codebase and that the Default class provides the same functionality.

Verification successful

Renaming from ARTDefault to Default is consistent.

The ARTDefault is not defined within the codebase and is imported from an external module (Ably). The renaming to Default has been applied correctly where applicable. The presence of ARTDefault in the import statement does not indicate an inconsistency.

  • The ARTDefault import in Test/Tests/ARTDefaultTests.swift is from an external module.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the renaming is consistent and that `Default` provides the same functionality as `ARTDefault`.

# Test: Search for any remaining references to `ARTDefault` and ensure `Default` is used instead.
rg --type swift -A 5 $'ARTDefault'
rg --type swift -A 5 $'Default'

Length of output: 57373


Script:

#!/bin/bash
# Search for the definition of `ARTDefault` in the codebase.
rg --type swift -A 5 $'class ARTDefault'
rg --type swift -A 5 $'struct ARTDefault'
rg --type swift -A 5 $'enum ARTDefault'

# Search for any references to `ARTDefault` to understand its usage.
rg --type swift -A 5 $'ARTDefault'

Length of output: 1030

Examples/SPM/Tests/SPMTests/SPMTests.swift (1)

6-8: Verify functionality and requirements of new client classes.

Ensure that the new ClientOptions, Rest, and Realtime classes provide the same or enhanced functionality as their predecessors, and verify if there are any changes in behavior or additional requirements.

Examples/Tests/Podfile (2)

1-1: Approved: Updated iOS platform version.

The update to iOS 14 aligns with modern iOS development standards and the PR objectives.


15-21: Approved: Added post_install hook for consistent deployment targets.

The addition of the post_install hook to set the IPHONEOS_DEPLOYMENT_TARGET to '14.0' for all targets ensures consistency and prevents potential version mismatch issues during the build process.

Test/Test Utilities/MockVersion2Log.swift (3)

4-4: Approved: Updated logLevel type to LogLevel.

The change from ARTLogLevel to LogLevel simplifies the logging abstraction, making it more idiomatic and potentially more flexible.


6-6: Approved: Updated lastReceivedLogMessageArguments type.

Reflecting the logLevel type change in lastReceivedLogMessageArguments ensures consistency in the logging mechanism.


8-8: Approved: Updated log function signature.

The update to use LogLevel in the log function further aligns with the goal of making the Swift interface more idiomatic and maintainable.

Examples/SPM/Package.swift (1)

10-10: Approved: Updated iOS deployment target to iOS 14.

The update to .iOS(.v14) ensures compatibility with newer iOS features and aligns with the PR's objectives to modernize the library.

Test/Test Utilities/MockInternalLogCore.swift (4)

7-7: Updated Logging Level Type in Tuple

The change in the tuple lastReceivedLogMessageArguments from ARTLogLevel to LogLevel is consistent and necessary for the logging level type update. Verify that this change integrates well with other parts of the system where this tuple is used.

Verification successful

Integration of Updated Logging Level Type Verified

The change in the tuple lastReceivedLogMessageArguments to use LogLevel is well-integrated. The tests in LogAdapterTests.swift and DefaultInternalLogCoreTests.swift confirm that the new LogLevel type is correctly handled across the system. No issues were found with this integration.

  • Test/Tests/LogAdapterTests.swift: Tests verify correct assignment and comparison of LogLevel.
  • Test/Tests/DefaultInternalLogCoreTests.swift: Tests confirm the correct usage of LogLevel.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the integration of the updated tuple.

# Test: Search for usages of `lastReceivedLogMessageArguments`. Expect: Correct handling of the new `LogLevel`.
rg --type swift -A 5 $'lastReceivedLogMessageArguments'

Length of output: 3467


5-5: Updated Logging Level Type

The update from ARTLogLevel to LogLevel for logLevel aligns with the PR's goal of making the Swift interface more idiomatic. Ensure that LogLevel is correctly defined and compatible with existing logging mechanisms.

Verification successful

LogLevel Definition and Compatibility Verified

The LogLevel is correctly defined as a Swift alias for the ARTLogLevel enum using NS_SWIFT_NAME. It is compatible with existing logging mechanisms and is used consistently throughout the codebase. No issues found.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the definition and compatibility of `LogLevel`.

# Test: Search for the definition of `LogLevel`. Expect: Definition and compatibility details.
ast-grep --lang swift --pattern $'enum LogLevel { $$$ }'

Length of output: 57


Script:

#!/bin/bash
# Search for `LogLevel` in the codebase to find its definition and usage.
rg 'LogLevel' -A 3

Length of output: 27177


13-13: Refactored Logging Method

The log method's update to use LogLevel is part of the broader refactoring to standardize logging levels. This change should be tested to ensure it does not introduce any regressions in logging behavior.

Verification successful

Logging Method is Adequately Tested

The log method is used in multiple test files, such as RestClientTests.swift, LogAdapterTests.swift, and DefaultInternalLogCoreTests.swift. This indicates that the logging behavior, including the use of LogLevel, is being tested. Therefore, the refactoring to standardize logging levels should not introduce regressions in logging behavior.

  • Test Files Involved:
    • Test/Tests/RestClientTests.swift
    • Test/Tests/LogAdapterTests.swift
    • Test/Tests/DefaultInternalLogCoreTests.swift
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the logging behavior with the updated `log` method.

# Test: Search for test cases involving the `log` method. Expect: Test cases that validate the new logging behavior.
rg --type swift -A 5 $'func testLog'

Length of output: 37


Script:

#!/bin/bash
# Search for the usage of the `log` method in Swift files, especially in test files.
rg --type swift 'log('

# Search for references to `LogLevel` in Swift files to check if they are being tested.
rg --type swift 'LogLevel'

Length of output: 2481


Script:

#!/bin/bash
# Correct the regex error and search for the usage of the `log` method in Swift files, especially in test files.
rg --type swift 'log\('

Length of output: 1228


9-9: Updated Logging Level Type with Objective-C Exposure

The update of lastReceivedLogMessageArgumentLevel to use LogLevel while retaining the @objc attribute is crucial for maintaining interoperability with Objective-C. Confirm that LogLevel is exposed to Objective-C without issues.

Ably-SoakTest-AppUITests/SoakTestReachability.swift (1)

17-17: Updated Logging Dependency in Initializer

The change from ARTLog to InternalLog in the initializer suggests a shift towards a more unified logging interface. Ensure that InternalLog is correctly implemented and that this change does not impact the functionality of reachability checks.

Ably.podspec (2)

14-14: Updated iOS Deployment Target

The update of the iOS deployment target to '14.0' is a significant change that aligns with modern iOS development standards. Ensure that this change is well-documented and consider the impact on users who might be on older iOS versions.


15-15: Updated tvOS Deployment Target

Similarly, the update of the tvOS deployment target to '14.0' ensures compatibility with newer features and improvements. As with iOS, verify that this change is documented and assess its impact on the existing user base.

Source/include/Ably/ARTPushAdmin.h (2)

11-11: Swift Naming Convention Applied Correctly

The NS_SWIFT_NAME(PushAdminProtocol) annotation is correctly applied to the protocol, making it more idiomatic for Swift developers.


31-31: Swift Naming Convention Applied Correctly

The NS_SWIFT_NAME(PushAdmin) annotation is correctly applied to the interface, enhancing the usability for Swift developers.

Source/include/Ably/ARTRealtimeChannelOptions.h (6)

12-12: Swift Naming Convention Applied Correctly

The NS_SWIFT_NAME(presence) annotation is correctly applied to the enumeration case, making it more idiomatic for Swift developers.


16-16: Swift Naming Convention Applied Correctly

The NS_SWIFT_NAME(publish) annotation is correctly applied to the enumeration case, making it more idiomatic for Swift developers.


20-20: Swift Naming Convention Applied Correctly

The NS_SWIFT_NAME(subscribe) annotation is correctly applied to the enumeration case, making it more idiomatic for Swift developers.


24-24: Swift Naming Convention Applied Correctly

The NS_SWIFT_NAME(presenceSubscribe) annotation is correctly applied to the enumeration case, making it more idiomatic for Swift developers.


25-25: Swift Naming Convention Applied Correctly

The NS_SWIFT_NAME(ChannelMode) annotation is correctly applied to the enumeration, enhancing the usability for Swift developers.


32-32: Swift Naming Convention Applied Correctly

The NS_SWIFT_NAME(RealtimeChannelOptions) annotation is correctly applied to the interface, enhancing the usability for Swift developers.

Source/ARTURLSessionServerTrust.m (1)

16-16: Simplified TLS Configuration

The conditional logic for setting TLSMinimumSupportedProtocolVersion has been simplified. This change should be verified to ensure it does not negatively impact systems that do not meet the version requirements.

Run the following script to verify the behavior on older systems:

Test/Tests/DefaultErrorCheckerTests.swift (4)

8-8: Code change is appropriate for the new error handling class.

The use of ErrorInfo.create aligns with the updated error handling strategy, ensuring that the test remains valid under the new class structure.


18-18: Code change is appropriate for the new error handling class.

The use of ErrorInfo.create is consistent with the new error handling strategy, ensuring that the test's intent is preserved.


28-28: Code change is appropriate for the new error handling class.

The use of ErrorInfo.create is consistent with the new error handling strategy, ensuring that the test's intent is preserved.


38-38: Code change is appropriate for the new error handling class.

The use of ErrorInfo.create is consistent with the new error handling strategy, ensuring that the test's intent is preserved.

Source/include/Ably/ARTDataQuery.h (3)

9-11: Swift Naming Conventions Applied Correctly

The updates to the ARTQueryDirection enumeration with NS_SWIFT_NAME annotations are correctly applied, making the API more idiomatic for Swift developers.

The changes are approved as they enhance the Swift usability of the library.


16-16: Swift Interface Naming Enhanced

The addition of NS_SWIFT_NAME(DataQuery) to the ARTDataQuery class interface is a positive change, improving the clarity and usability of the API in Swift contexts.

The changes are approved as they contribute to a more idiomatic Swift API.


44-44: Consistent Swift Naming Adoption

The NS_SWIFT_NAME(RealtimeHistoryQuery) annotation for the ARTRealtimeHistoryQuery class interface aligns with the goal of enhancing Swift compatibility and usability.

The changes are approved as they ensure consistency in adopting Swift-friendly naming conventions across the library.

Source/include/Ably/ARTPaginatedResult.h (1)

11-11: Enhanced Swift Usability for Paginated Results

The addition of NS_SWIFT_NAME(PaginatedResult) to the ARTPaginatedResult class interface significantly improves the readability and usability of the API in Swift projects.

The changes are approved as they enhance the Swift interface without altering the underlying functionality.

Test/Test Utilities/TestProxyTransportFactory.swift (1)

12-12: Updated Parameter Type for Better Configuration Handling

The update to the transport function to accept ClientOptions instead of ARTClientOptions suggests a more unified or simplified approach to options handling. Ensure compatibility with existing implementations that may still expect the old ARTClientOptions type.

The changes are approved, but verification of compatibility in the broader codebase is recommended.

Run the following script to verify the usage of ClientOptions:

Verification successful

Transition to ClientOptions is Consistent Across the Codebase

The change to use ClientOptions in the transport function is consistent with its widespread usage throughout the codebase. This suggests that the transition from ARTClientOptions to ClientOptions is complete and compatible. No further action is needed.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of `ClientOptions` across the codebase.

# Test: Search for the usage of `ClientOptions`. Expect: Consistent usage across the codebase.
rg --type swift -A 5 $'ClientOptions'

Length of output: 79351

Test/Test Utilities/MockDeviceStorage.swift (2)

3-3: Class Interface Updated

The update from ARTDeviceStorage to DeviceStorage broadens the potential use of MockDeviceStorage, aligning with modern Swift idioms and potentially improving integration flexibility.

The code changes are approved.


13-13: Initializer Parameter Type Updated

Changing the parameter type from ARTPushActivationState? to PushActivationState? in the initializer enhances type consistency and possibly simplifies the interaction with other components of the system.

The code changes are approved.

Ably-SoakTest-AppUITests/SoakTestURLSession.swift (3)

14-14: Cancellation Interface Generalized

The update from [ARTCancellable] to [Cancellable] broadens the potential use of cancellables, aligning with modern Swift idioms and potentially improving integration flexibility.

The code changes are approved.


20-20: Method Return Type Updated

Changing the return type from ARTCancellable & NSObjectProtocol to Cancellable & NSObjectProtocol in the get method enhances type consistency and possibly simplifies the interaction with other components of the system.

The code changes are approved.


65-65: Class Interface Updated

The update from ARTCancellable to Cancellable broadens the potential use of CancellableInQueue, aligning with modern Swift idioms and potentially improving integration flexibility.

The code changes are approved.

Source/include/Ably/ARTPresenceMessage.h (2)

11-28: Swift Naming Annotations Added

The addition of Swift naming annotations (NS_SWIFT_NAME) to the ARTPresenceAction enumeration cases improves their usability in Swift, making the API more idiomatic and accessible to Swift developers.

The code changes are approved.


31-31: Swift Name Annotation Added to Function

The addition of a Swift name annotation (NS_SWIFT_NAME) to the ARTPresenceActionToStr function improves its clarity and usability in Swift, aligning with modern Swift idioms.

The code changes are approved.

Source/PrivateHeaders/Ably/ARTPushActivationEvent.h (13)

10-10: Swift Naming Approved: PushActivationEvent

The NS_SWIFT_NAME annotation correctly translates the Objective-C class name to a more idiomatic Swift name.


19-19: Swift Naming Approved: PushActivationErrorEvent

The NS_SWIFT_NAME annotation correctly reflects the class's purpose and maintains consistency with the base class naming.


30-30: Swift Naming Approved: PushActivationDeviceIdentityEvent

The NS_SWIFT_NAME annotation provides a clear, descriptive name in Swift, aligning well with the class's functionality.


45-45: Swift Naming Approved: PushActivationEventCalledActivate

The NS_SWIFT_NAME annotation is aptly chosen to reflect the specific event action in Swift.


49-49: Swift Naming Approved: PushActivationEventCalledDeactivate

The NS_SWIFT_NAME annotation properly conveys the event's action in Swift.


53-53: Swift Naming Approved: PushActivationEventGotPushDeviceDetails

The NS_SWIFT_NAME annotation accurately reflects the event's purpose in Swift.


57-57: Swift Naming Approved: PushActivationEventGettingPushDeviceDetailsFailed

The NS_SWIFT_NAME annotation is descriptive and clearly indicates an error event in Swift.


61-61: Swift Naming Approved: PushActivationEventGotDeviceRegistration

The NS_SWIFT_NAME annotation provides a clear and appropriate name for the event in Swift.


65-65: Swift Naming Approved: PushActivationEventGettingDeviceRegistrationFailed

The NS_SWIFT_NAME annotation is descriptive and clearly indicates an error event in Swift.


69-69: Swift Naming Approved: PushActivationEventRegistrationSynced

The NS_SWIFT_NAME annotation provides a clear and appropriate name for the event in Swift.


73-73: Swift Naming Approved: PushActivationEventSyncRegistrationFailed

The NS_SWIFT_NAME annotation is descriptive and clearly indicates an error event in Swift.


77-77: Swift Naming Approved: PushActivationEventDeregistered

The NS_SWIFT_NAME annotation is aptly chosen to reflect the specific event action in Swift.


81-81: Swift Naming Approved: PushActivationEventDeregistrationFailed

The NS_SWIFT_NAME annotation is descriptive and clearly indicates an error event in Swift.

Test/Tests/DefaultInternalLogCoreTests.swift (3)

7-7: Refactored ClientOptions instantiation.

The change from ARTClientOptions to ClientOptions aligns with the PR's goal of making the interface more idiomatic for Swift. Ensure that ClientOptions is fully compatible with the expected functionalities of ARTClientOptions.

The code changes are approved.


24-24: Refactored ClientOptions instantiation in a different test scenario.

Similar to the previous change, this modification ensures consistency across test cases. Verify that the behavior of ClientOptions when the log level is set to .none remains consistent with the previous implementation.

The code changes are approved.


42-42: Updated LogLevel enumeration usage.

The change from [ARTLogLevel] to [LogLevel] suggests a renaming or refactoring of the log level enumeration to be more Swift-idiomatic. Confirm that all log levels are correctly mapped from the old ARTLogLevel to the new LogLevel.

The code changes are approved.

Examples/Tests/TestsTests/TestsTests.swift (4)

15-15: Updated ClientOptions declaration.

The change from ARTClientOptions to ClientOptions is consistent with the PR's objectives. Ensure that ClientOptions maintains all necessary properties and methods that were available in ARTClientOptions.

The code changes are approved.


49-51: Refactored Realtime client instantiation.

This change updates the instantiation of the Realtime client from ARTRealtime. Verify that Realtime has all the functionalities and behaviors of ARTRealtime, especially with respect to initialization with client options.

The code changes are approved.


66-68: Refactored Realtime variable declaration and instantiation.

The renaming from ARTRealtime to Realtime is part of making the API more Swift-idiomatic. Ensure that the Realtime class behaves as expected in a background queue scenario.

The code changes are approved.

Tools
SwiftLint

[Warning] 66-66: Prefer at least one space after slashes for comments

(comment_spacing)


76-78: Refactored Rest client instantiation.

The change from ARTRest to Rest aligns with the PR's goal of idiomatic Swift API. Verify that Rest maintains all functionalities of ARTRest, particularly in handling channel history in a background queue.

The code changes are approved.

Tools
SwiftLint

[Warning] 76-76: Prefer at least one space after slashes for comments

(comment_spacing)

Source/PrivateHeaders/Ably/ARTPushActivationState.h (11)

11-11: Added Swift-friendly name for ARTPushActivationState.

The use of NS_SWIFT_NAME(PushActivationState) makes the API more accessible to Swift developers. This change does not alter functionality but improves usability in Swift.

The code changes are approved.


29-29: Added Swift-friendly name for ARTPushActivationPersistentState.

Similar to the previous change, this enhances the Swift API's readability without affecting the underlying functionality.

The code changes are approved.


35-35: Added Swift-friendly name for ARTPushActivationStateNotActivated.

This change continues the pattern of making the API more idiomatic for Swift developers. Ensure that all references to this state in Swift code are updated accordingly.

The code changes are approved.


39-39: Added Swift-friendly name for ARTPushActivationStateWaitingForDeviceRegistration.

This naming update aids in making the codebase more intuitive for Swift developers, aligning with Swift naming conventions.

The code changes are approved.


43-43: Added Swift-friendly name for ARTPushActivationStateWaitingForPushDeviceDetails.

Continuing with the theme of enhancing Swift interoperability, this change ensures that the state is easily identifiable in Swift code.

The code changes are approved.


47-47: Added Swift-friendly name for ARTPushActivationStateWaitingForNewPushDeviceDetails.

This update makes the state's purpose clearer in Swift, facilitating easier code navigation and maintenance.

The code changes are approved.


51-51: Added Swift-friendly name for ARTPushActivationStateWaitingForRegistrationSync.

This change is part of the broader effort to make the API more Swift-idiomatic, improving the developer experience for Swift users.

The code changes are approved.


64-64: Added Swift-friendly name for ARTPushActivationStateAfterRegistrationSyncFailed.

By providing a more descriptive name in Swift, this change helps clarify the state's role and context within the API.

The code changes are approved.


68-68: Added Swift-friendly name for ARTPushActivationStateWaitingForDeregistration.

This naming update enhances clarity and consistency in the Swift interface, making the API more accessible to Swift developers.

The code changes are approved.


74-74: Added Swift-friendly name for ARTPushActivationDeprecatedPersistentState.

This change, while focusing on a deprecated state, ensures that even legacy parts of the API are accessible and understandable in Swift.

The code changes are approved.


83-83: Added Swift-friendly name for ARTPushActivationStateAfterRegistrationUpdateFailed.

The update to a more descriptive Swift name aids in better understanding and usage of this state within the Swift context.

The code changes are approved.

Source/SocketRocket/Internal/Utilities/ARTSRURLUtilities.m (2)

83-84: Review the use of pragma to suppress warnings.

The use of #pragma clang diagnostic ignored "-Wswitch" is typically a sign that there are unhandled cases within the switch statement. While this suppresses warnings, it's crucial to ensure that all necessary cases are indeed handled or explicitly documented why they are not.

Verification successful

Pragma usage verified, but consider documentation.

The #pragma clang diagnostic ignored "-Wswitch" directive in ARTSRURLUtilities.m appears unnecessary as all known NSURLNetworkServiceType cases are handled in the switch statement. If the pragma is retained for future-proofing, consider adding documentation to explain its presence.

  • All NSURLNetworkServiceType cases are handled in the switch statement.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for unhandled cases in switch statements related to network service types.

# Test: Search for switch statements handling network service types. Expect: Comments or documentation on why certain cases are ignored.
rg --type objc -A 5 -B 5 $'switch.*networkServiceType' Source/SocketRocket/Internal/Utilities/ARTSRURLUtilities.m

Length of output: 473


Script:

#!/bin/bash
# Description: List all possible NSURLNetworkServiceType values to check against the switch statement.

# Test: Search for NSURLNetworkServiceType enum values in the codebase.
rg --type objc 'NSURLNetworkServiceType'

Length of output: 994


84-84: Conditional compilation directive adjusted for platform compatibility.

The update to include TARGET_OS_MACCATALYST and TARGET_OS_MAC while excluding TARGET_OS_TV and TARGET_OS_IOS for handling NSURLNetworkServiceTypeVoIP is a strategic decision likely aimed at aligning with platform capabilities and restrictions. This change should be tested to ensure it behaves as expected across the specified platforms.

Test/Tests/RealtimeClientChannelsTests.swift (5)

6-6: Swift extension adapted to new class names.

The extension for RealtimeChannels being conformant to Sequence is updated correctly to reflect the new class naming. This change is consistent with the PR's objectives to make the interface more idiomatic.


16-16: Client instantiation reflects updated class names.

The instantiation of Realtime client using updated options setup reflects the new naming conventions accurately. This change is crucial for maintaining consistency across the test suite.


43-43: Client instantiation and channel handling in tests.

The client is correctly instantiated with the new Realtime class, and the handling of channels reflects the updated naming conventions. This ensures that the tests are aligned with the changes in the main codebase.


59-59: Channel options instantiation and usage updated.

The instantiation and usage of RealtimeChannelOptions in various test cases are correctly updated to reflect the new naming conventions. This change ensures that the tests remain consistent with the main library's interface.

Also applies to: 61-61, 73-73


96-96: State assertion updated to new enum values.

The assertion for channel state now uses the updated RealtimeChannelState.detached, which is part of the renaming effort to make the API more idiomatic for Swift developers. This change is correctly applied and crucial for the integrity of the tests.

Test/Tests/RestClientChannelsTests.swift (5)

6-6: Swift extension adapted to new class names.

The extension for RestChannels being conformant to Sequence is updated correctly to reflect the new class naming. This change is consistent with the PR's objectives to make the interface more idiomatic.


20-20: Variable declarations reflect updated class and type names.

The declarations of client and cipherParams with updated class and type names (Rest and CipherParams) are correctly applied, ensuring that the test setup aligns with the main library's changes.

Also applies to: 23-23


38-38: Client instantiation and channel handling in tests.

The client is correctly instantiated with the new Rest class, and the handling of channels reflects the updated naming conventions. This ensures that the tests are aligned with the changes in the main codebase.


44-44: Channel collection handling reflects new class names.

The handling of channel collections using the new RestChannels class is correctly implemented, aligning with the renaming efforts across the codebase.


60-60: Channel options handling updated to new class names.

The handling of ChannelOptions in various test cases is correctly updated to reflect the new naming conventions. This change ensures that the tests remain consistent with the main library's interface.

Also applies to: 69-69, 83-83

Source/include/Ably/ARTEncoder.h (2)

15-22: Swift Naming Conventions Applied

The addition of NS_SWIFT_NAME(EncoderFormat) to the ARTEncoderFormat enum is intended to provide a more Swift-friendly name when accessed from Swift code. This change aligns with Swift naming conventions and improves the readability of the code when used in Swift environments.

The change is approved as it enhances the semantic clarity and usability of the code in Swift.


27-27: Protocol Naming Enhanced for Swift

The annotation NS_SWIFT_NAME(AblyEncoder) on the ARTEncoder protocol improves clarity and usability for Swift developers by providing a more intuitive name in Swift contexts. This change is consistent with the goal of making the library more idiomatic for Swift usage.

The change is approved as it aligns with the best practices for Swift interoperability.

Test/Tests/ObjectLifetimesTests.swift (1)

5-6: Updated Options Type for Consistency

The change from ARTClientOptions to ClientOptions simplifies the options handling within the library. This update should be reflected across all test files to maintain consistency.

The change is approved, but ensure that all references to the old ARTClientOptions are updated throughout the test suite.

Source/include/Ably/ARTEventEmitter.h (2)

9-9: Swift Naming Annotations Added

The addition of NS_SWIFT_NAME annotations to various entities (EventIdentification, Event, EventListener, EventEmitter) improves their usability in Swift by providing more intuitive names. This change aligns with the goal of making the Objective-C code more idiomatic for Swift developers.

The changes are approved as they enhance the clarity and usability of the API for Swift developers.

Also applies to: 17-17, 28-28, 35-35


9-9: Marking init as Unavailable

The init method in the ARTEventIdentification protocol has been marked as unavailable. This change reinforces the intended usage pattern of the protocol, ensuring that instances are not created directly. This is a significant change that affects how the protocol is used and should be clearly documented to avoid confusion.

The change is approved, but additional documentation should be provided to explain this decision to developers.

Test/Tests/DeltaCodecTests.swift (4)

82-82: Duplicate Comment: Verify the properties and methods of RealtimeChannelOptions.

Refer to the previous comment regarding the verification of RealtimeChannelOptions.


112-112: Duplicate Comment: Verify the compatibility of Message with ARTMessage.

Refer to the previous comment regarding the verification of the compatibility of Message with ARTMessage.


29-29: Verify the properties and methods of RealtimeChannelOptions.

Ensure that RealtimeChannelOptions maintains the same properties and methods as ARTRealtimeChannelOptions to confirm that the functionality remains unchanged.


48-48: Verify the compatibility of Message with ARTMessage.

Ensure that the Message type can be used interchangeably with ARTMessage in terms of properties and methods to confirm that the test functionality remains intact.

Examples/AblyPush/AblyPushExample/AblyHelper.swift (9)

43-43: Consistent Change: Updated method signature of activatePush.

The update to use ErrorInfo instead of ARTErrorInfo is consistent with the previous change in the callback type. Ensure that the new error handling is tested thoroughly.


156-156: Consistent Change: Updated method signature of didActivateAblyPush.

The update to use ErrorInfo instead of ARTErrorInfo is consistent with the previous changes in error handling types. Ensure that the new error handling is tested thoroughly.


165-165: Consistent Change: Updated method signature of didDeactivateAblyPush.

The update to use ErrorInfo instead of ARTErrorInfo is consistent with the previous changes in error handling types. Ensure that the new error handling is tested thoroughly.


172-172: Consistent Change: Updated method signature of didUpdateAblyPush.

The update to use ErrorInfo instead of ARTErrorInfo is consistent with the previous changes in error handling types. Ensure that the new error handling is tested thoroughly.


52-55: Test the functionality of activateLocationPush.

The internal logic of the method has been modified. It is crucial to ensure that the method still functions as intended and handles errors appropriately.

Verification successful

The activateLocationPush method handles errors and registration correctly.

The method uses a closure to handle the result of startMonitoringLocationPushes, with proper error handling and registration logic. Ensure that the functionality is tested to confirm its behavior in different scenarios.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the functionality of `activateLocationPush`.

# Test: Search for the method implementation and its error handling. Expect: Proper error handling and functionality.
ast-grep --lang swift --pattern $'func activateLocationPush() {
  $$$
}'

Length of output: 1030


96-96: Verify the compatibility of Message with ARTMessage.

Ensure that the Message type can be used interchangeably with ARTMessage in terms of properties and methods to confirm that the functionality remains intact.


19-19: Verify the compatibility of ErrorInfo with ARTErrorInfo.

Ensure that the ErrorInfo type can be used interchangeably with ARTErrorInfo in terms of properties and methods to confirm that the functionality remains intact.


71-71: Verify the compatibility of DeviceDetails and ErrorInfo with ARTDeviceDetails and ARTErrorInfo.

Ensure that the DeviceDetails and ErrorInfo types can be used interchangeably with ARTDeviceDetails and ARTErrorInfo in terms of properties and methods to confirm that the functionality remains intact.


11-11: Verify the properties and methods of Realtime.

Ensure that Realtime maintains the same properties and methods as ARTRealtime to confirm that the functionality remains unchanged.

Ably-SoakTest-AppUITests/SoakTest.swift (4)

122-122: Consistent Change: Updated function signature of channelsOperations.

The update to use Realtime instead of ARTRealtime is consistent with the previous change in the function realtimeOperations. Ensure that the new class is tested thoroughly.


209-209: Verify the methods of the extension for RealtimeChannels.

Ensure that the extension for RealtimeChannels maintains the same methods as the extension for ARTRealtimeChannels to confirm that the functionality remains unchanged.

Verification successful

Verification successful: RealtimeChannels extension maintains functionality.

The RealtimeChannels extension implements the Sequence protocol and defines the makeIterator method, consistent with the previous ARTRealtimeChannels extension, ensuring unchanged functionality.

  • Located in Test/Tests/RealtimeClientChannelsTests.swift and Ably-SoakTest-AppUITests/SoakTest.swift.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the methods of the extension for `RealtimeChannels`.

# Test: Search for the extension definition and its methods. Expect: Similar to `ARTRealtimeChannels`.
ast-grep --lang swift --pattern $'extension RealtimeChannels {
  $$$
}'

Length of output: 531


Script:

#!/bin/bash
# Description: Verify the methods of the extension for `RealtimeChannels`.

# Test: Search for the extension definition and its methods. Expect: Similar to `ARTRealtimeChannels`.
ast-grep --lang swift --pattern $'extension RealtimeChannels {
  $_
}'

Length of output: 529


Script:

#!/bin/bash
# Description: Use ripgrep to find the methods in the `RealtimeChannels` extension.

# Search for the extension definition and its methods.
rg 'extension RealtimeChannels' -A 20

Length of output: 3275


183-183: Verify the properties and methods of RealtimeChannel.

Ensure that RealtimeChannel maintains the same properties and methods as ARTRealtimeChannel to confirm that the functionality remains unchanged.


94-94: Verify the properties and methods of Realtime.

Ensure that Realtime maintains the same properties and methods as ARTRealtime to confirm that the functionality remains unchanged.

Verification successful

Realtime is likely a wrapper or subclass of ARTRealtime.

The extensions found for Realtime, especially Realtime: ARTHasInternal, suggest that it maintains similar properties and methods as ARTRealtime. This implies that the change to use Realtime should not affect functionality.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the properties and methods of `Realtime`.

# Test: Search for the class definition and its properties/methods. Expect: Similar to `ARTRealtime`.
ast-grep --lang swift --pattern $'class Realtime {
  $$$
}'

Length of output: 507


Script:

#!/bin/bash
# Description: Verify the properties and methods of `Realtime`.

# Test: Search for the class definition and its properties/methods. Expect: Similar to `ARTRealtime`.
ast-grep --lang swift --pattern 'class Realtime { $$$ }'

Length of output: 58


Script:

#!/bin/bash
# Description: Search for occurrences of `Realtime` in the codebase.

# Test: Use ripgrep to find any references to `Realtime`.
rg 'Realtime'

Length of output: 266724


Script:

#!/bin/bash
# Description: Search for type aliases or extensions related to `Realtime`.

# Test: Use ripgrep to find any typealias or extension that defines `Realtime`.
rg 'typealias Realtime|extension Realtime'

Length of output: 648

Examples/AblyPush/AblyPushExample/ContentView.swift (3)

8-8: Refactor approved: Updated type for deviceDetails.

The change from ARTDeviceDetails to DeviceDetails aligns with making the interface more idiomatic. Ensure that DeviceDetails is correctly defined and accessible.


9-9: Refactor approved: Updated type for deviceDetailsError.

The change from ARTErrorInfo to ErrorInfo aligns with making the interface more idiomatic. Ensure that ErrorInfo is correctly defined and accessible.


14-14: Refactor approved: Updated type for deviceTokensError.

The change from ARTErrorInfo to ErrorInfo aligns with making the interface more idiomatic. Ensure that ErrorInfo is correctly defined and accessible.

Source/ARTPush.m (1)

109-109: Annotation added: NS_EXTENSION_UNAVAILABLE.

The addition of NS_EXTENSION_UNAVAILABLE("") to getActivationMachine is appropriate to restrict its use in app extensions, aligning with best practices for iOS development. Verify that this change does not unexpectedly affect the accessibility of this method in other contexts.

Source/include/Ably/ARTStats.h (10)

14-27: Refactor approved: Swift-friendly names for enum values.

The addition of NS_SWIFT_NAME to the ARTStatsGranularity enum values (minute, hour, day, month) enhances Swift interoperability. Verify that these names do not conflict with existing Swift types or variables in the broader application context.


32-32: Refactor approved: Swift name for ARTStatsQuery.

The addition of NS_SWIFT_NAME(StatsQuery) to the ARTStatsQuery interface improves its usability in Swift. Verify that this name does not conflict with existing Swift types or variables in the broader application context.


45-45: Refactor approved: Swift name for ARTStatsMessageCount.

The addition of NS_SWIFT_NAME(StatsMessageCount) to the ARTStatsMessageCount interface improves its usability in Swift. Verify that this name does not conflict with existing Swift types or variables in the broader application context.


73-73: Refactor approved: Swift name for ARTStatsMessageTypes.

The addition of NS_SWIFT_NAME(StatsMessageTypes) to the ARTStatsMessageTypes interface improves its usability in Swift. Verify that this name does not conflict with existing Swift types or variables in the broader application context.


107-107: Refactor approved: Swift name for ARTStatsMessageTraffic.

The addition of NS_SWIFT_NAME(StatsMessageTraffic) to the ARTStatsMessageTraffic interface improves its usability in Swift. Verify that this name does not conflict with existing Swift types or variables in the broader application context.


147-147: Refactor approved: Swift name for ARTStatsResourceCount.

The addition of NS_SWIFT_NAME(StatsResourceCount) to the ARTStatsResourceCount interface improves its usability in Swift. Verify that this name does not conflict with existing Swift types or variables in the broader application context.


193-193: Refactor approved: Swift name for ARTStatsConnectionTypes.

The addition of NS_SWIFT_NAME(StatsConnectionTypes) to the ARTStatsConnectionTypes interface improves its usability in Swift. Verify that this name does not conflict with existing Swift types or variables in the broader application context.


227-227: Refactor approved: Swift name for ARTStatsRequestCount.

The addition of NS_SWIFT_NAME(StatsRequestCount) to the ARTStatsRequestCount interface improves its usability in Swift. Verify that this name does not conflict with existing Swift types or variables in the broader application context.


261-261: Refactor approved: Swift name for ARTStatsPushCount.

The addition of NS_SWIFT_NAME(StatsPushCount) to the ARTStatsPushCount interface improves its usability in Swift. Verify that this name does not conflict with existing Swift types or variables in the broader application context.


313-313: Refactor approved: Swift name for ARTStats.

The addition of NS_SWIFT_NAME(Stats) to the ARTStats interface improves its usability in Swift. Verify that this name does not conflict with existing Swift types or variables in the broader application context.

Test/Tests/CryptoTests.swift (8)

23-33: Refactor: Renaming of cryptographic parameters and methods.

The renaming from ARTCrypto to Crypto and ARTCipherParams to CipherParams is consistent across the test cases, which aligns with the PR's goal of making the interface more idiomatic for Swift developers.

The changes are approved as they improve readability and maintain Swift idiomatic practices.


46-57: Refactor: Consistent use of new cryptographic class names in test cases.

The use of Crypto.getDefaultParams instead of the older ARTCrypto.getDefaultParams is correctly applied here, ensuring that the tests are aligned with the updated class names.

The changes are approved as they ensure consistency with the new naming conventions.


63-66: Refactor: Updated cryptographic parameter handling in test cases.

The tests correctly handle the keyLength calculations using the new Crypto class, which is part of the broader renaming effort.

The changes are approved as they correctly apply the new class names without altering the underlying test logic.


72-72: Error Handling: Proper exception handling for invalid cryptographic parameters.

The test correctly expects an exception when invalid parameters are supplied, which is good practice for robust error handling in cryptographic operations.

The changes are approved as they maintain good error handling practices.


81-90: Refactor: Updated method names and parameters in cryptographic key generation tests.

The renaming and parameter updates in the generateRandomKey method calls are consistent with the new class structure, improving clarity and maintainability.

The changes are approved as they enhance the clarity and idiomatic usage of Swift in cryptographic contexts.


98-98: Correctness: Ensuring cryptographic hash function accuracy.

The test for generateHashSHA256 correctly checks the output against a known hash value, ensuring the method's reliability and correctness.

The changes are approved as they correctly implement cryptographic standards without altering the intended functionality.


Line range hint 103-118: Complexity: Handling encryption with dynamic initialization vectors (IVs).

This test case introduces complexity by dynamically generating IVs and ensuring encryption consistency, which is crucial for security in cryptographic systems.

The changes are approved as they add necessary complexity to test realistic scenarios in encryption.

Tools
SwiftLint

[Warning] 106-106: Prefer using UTF-8 encoded strings when converting between String and Data

(non_optional_string_data_conversion)


Line range hint 134-238: Refactor: Comprehensive testing of message encoding and encryption.

The extensive tests for message encoding and encryption using the updated Crypto and CipherParams classes demonstrate thorough testing practices and ensure that the cryptographic functionality behaves as expected.

The changes are approved as they provide comprehensive testing coverage and maintain high standards in cryptographic testing.

Test/Tests/RestClientStatsTests.swift (3)

Line range hint 6-23: Refactor: Updated function signatures to use new client options and stats types.

The refactoring of postTestStats and queryStats functions to use ClientOptions and StatsQuery aligns with the PR's objective to make the interface more idiomatic for Swift developers.

The changes are approved as they improve the clarity and usability of the API.


Line range hint 79-209: Refactor: Consistent use of new types in REST client statistics tests.

The tests have been updated to use the new Rest, StatsQuery, and PaginatedResult<Stats> types, which simplifies the API and enhances readability.

The changes are approved as they ensure consistency with the new naming conventions and improve the test structure.


Line range hint 227-311: Complexity: Handling pagination in REST client statistics tests.

The tests for paginated results are well-structured and ensure that the REST client handles stats queries correctly across multiple pages, which is crucial for real-world applications.

The changes are approved as they effectively test the pagination functionality and maintain high standards in testing practices.

Ably-SoakTest-AppUITests/SoakTestWebSocket.swift (4)

Line range hint 18-46: Refactor: Streamlined method signatures in transport factory.

The update to the transport method to use ClientOptions and the removal of the connectionSerial parameter simplifies the method's interface, which could enhance its usability and maintainability.

The changes are approved as they simplify the interface and align with the PR's goals of enhancing clarity.


Line range hint 99-140: Refactor: Updated protocol message handling in WebSocket tests.

The renaming of ARTProtocolMessage to ProtocolMessage and related updates in the protocolMessage, messageToClient, and ackMessages methods improve readability and maintainability of the test code.

The changes are approved as they ensure consistency with the new naming conventions and improve the clarity of the code.


Line range hint 171-273: Complexity: Handling error and presence messages in WebSocket tests.

The tests for error handling and presence message functionality are crucial for ensuring the WebSocket's robustness and reliability in adverse conditions.

The changes are approved as they add necessary complexity to test realistic scenarios in WebSocket communications.


Line range hint 320-352: Refactor: Enhanced presence message handling in WebSocket tests.

The update to use PresenceMessage instead of ARTPresenceMessage in the presence synchronization tests aligns with the renaming efforts and improves the test's clarity.

The changes are approved as they enhance the readability and consistency of the presence message handling.

Test/Tests/PushChannelTests.swift (3)

7-7: Review the changes in variable and type renaming

The renaming of ARTRest to Rest and ARTClientOptions to ClientOptions is part of the broader effort to make the library more idiomatic for Swift developers. These changes are correctly reflected in the test environment setup.

The changes are correctly implemented and improve the readability and Swift idiomatic nature of the code.

Also applies to: 13-13, 17-17


51-51: Review the renaming of ARTDeviceIdentityTokenDetails to DeviceIdentityTokenDetails

This renaming is also part of making the API more Swift-friendly. The changes are consistently applied across different test cases where device identity token details are used.

The renaming is correctly implemented and aligns with the objective of the PR to enhance the Swift idiomatic nature of the library.

Also applies to: 87-87, 113-113, 165-165, 199-199, 225-225


316-316: Review the changes in error handling

The renaming of ARTDataQueryError to DataQueryError and its usage in assertions reflects the updated error handling strategy. This change is part of the broader refactoring to align with Swift conventions.

The error handling changes are correctly implemented and improve the clarity and maintainability of the test code.

Also applies to: 330-330

Test/Tests/StatsTests.swift (1)

7-7: Review the changes in type renaming

The renaming of ARTStats and related types to Stats and similar changes are correctly applied across the test cases. These changes are part of the effort to make the library more idiomatic for Swift developers.

The renaming is correctly implemented and aligns with the objective of the PR to enhance the Swift idiomatic nature of the library.

Also applies to: 12-12, 16-16, 21-21, 25-25, 40-40, 44-44, 49-49, 52-52, 57-57, 86-86, 191-191, 312-312, 386-386

Source/include/Ably/ARTStatus.h (4)

31-104: Swift-friendly names for error codes approved.

The changes to ARTErrorCode with NS_SWIFT_NAME annotations enhance readability and usability in Swift.

The code changes are approved.


170-172: Swift-friendly names for client code errors approved.

The changes to ARTClientCodeError with NS_SWIFT_NAME annotations enhance readability and usability in Swift.

The code changes are approved.


193-193: Swift representation for ARTErrorInfo approved.

The NS_SWIFT_NAME(ErrorInfo) annotation makes the interface more idiomatic in Swift.

The code changes are approved.


262-262: Swift representation for ARTStatus approved.

The NS_SWIFT_NAME(Status) annotation makes the interface more idiomatic in Swift.

The code changes are approved.

Source/SocketRocket/Internal/Proxy/ARTSRProxyConnect.m (1)

Line range hint 262-262: Simplified TLS protocol handling approved.

The update to use only tls_protocol_version_TLSv12 for compatible versions enhances security and simplifies the code.

The code changes are approved.

Source/ARTPushActivationStateMachine.m (1)

422-422: Restriction on registerForAPNS in app extensions approved.

The NS_EXTENSION_UNAVAILABLE("") attribute correctly enforces the restriction, preventing the method's use in iOS app extensions.

The code changes are approved.

Test/Tests/RestClientPresenceTests.swift (39)

12-12: Updated class name usage.

The use of Rest instead of ARTRest aligns with the new naming conventions.


16-16: Updated array type usage.

The change from ARTRealtime to Realtime in the disposable array is consistent with the renaming effort.


35-35: Updated instance type check.

The instance type check now uses PaginatedResult<PresenceMessage> instead of ARTPaginatedResult<ARTPresenceMessage>, reflecting the updated class and type names.


50-50: Consistent type usage in pagination.

The continued use of PaginatedResult<PresenceMessage> in the pagination logic is correct and consistent with the changes.


70-70: Updated class name usage.

The use of Rest instead of ARTRest is consistent with the new naming conventions.


73-73: Updated query class name.

The change from ARTPresenceQuery to PresenceQuery simplifies the class name, making it more Swift-idiomatic.


89-89: Updated class name usage.

The use of Rest instead of ARTRest is consistent with the new naming conventions.


92-92: Updated class name usage in realtime context.

The change from ARTRealtime to Realtime is part of the effort to make the API more Swift-friendly.


102-102: Updated query class name usage.

The change from ARTPresenceQuery to PresenceQuery is consistent with the renaming effort and simplifies the class name.


123-123: Updated class name usage.

The use of Rest instead of ARTRest is consistent with the new naming conventions.


128-128: Updated array type usage.

The change from ARTRealtime to Realtime in the disposable array is consistent with the renaming effort.


142-142: Updated query class name usage.

The change from ARTRealtimePresenceQuery to RealtimePresenceQuery simplifies the class name, making it more Swift-idiomatic.


168-168: Updated class name usage.

The use of Rest instead of ARTRest is consistent with the new naming conventions.


173-173: Updated class name usage in realtime context.

The change from ARTRealtime to Realtime is part of the effort to make the API more Swift-friendly.


186-186: Updated instance type check.

The instance type check now uses PaginatedResult<PresenceMessage> instead of ARTPaginatedResult<ARTPresenceMessage>, reflecting the updated class and type names.


203-203: Consistent type usage in pagination.

The continued use of PaginatedResult<PresenceMessage> in the pagination logic is correct and consistent with the changes.


228-228: Updated class name usage.

The use of Rest instead of ARTRest is consistent with the new naming conventions.


233-233: Updated array type usage.

The change from ARTRealtime to Realtime in the disposable array is consistent with the renaming effort.


243-243: Updated query class name usage.

The change from ARTDataQuery to DataQuery simplifies the class name, making it more Swift-idiomatic.


283-283: Updated class name usage.

The use of Rest instead of ARTRest is consistent with the new naming conventions.


288-288: Updated class name usage in realtime context.

The change from ARTRealtime to Realtime is part of the effort to make the API more Swift-friendly.


292-292: Updated query class name usage.

The change from ARTDataQuery to DataQuery simplifies the class name, making it more Swift-idiomatic.


311-311: Correct error handling for limit exceeding.

The error handling for exceeding the limit is correctly updated to use the new DataQueryError enum.


319-319: Updated class name usage.

The use of Rest instead of ARTRest is consistent with the new naming conventions.


324-324: Updated array type usage.

The change from ARTRealtime to Realtime in the disposable array is consistent with the renaming effort.


338-338: Updated query class name usage.

The change from ARTRealtimePresenceQuery to RealtimePresenceQuery simplifies the class name, making it more Swift-idiomatic.


366-366: Updated class name usage.

The use of Rest instead of ARTRest is consistent with the new naming conventions.


371-371: Updated array type usage.

The change from ARTRealtime to Realtime in the disposable array is consistent with the renaming effort.


379-379: Updated query class name usage.

The change from ARTDataQuery to DataQuery simplifies the class name, making it more Swift-idiomatic.


421-421: Updated class name usage.

The use of Rest instead of ARTRest is consistent with the new naming conventions.


424-424: Updated query class name usage.

The change from ARTDataQuery to DataQuery simplifies the class name, making it more Swift-idiomatic.


430-430: Correct error handling for timestamp range.

The error handling for incorrect timestamp ranges is correctly updated to use the new DataQueryError enum.


436-436: Consistent error handling for timestamp range.

The error handling remains consistent for different query directions, using the new DataQueryError enum.


444-444: Updated class name usage.

The use of Rest instead of ARTRest is consistent with the new naming conventions.


455-455: Updated class name usage in realtime context.

The change from ARTRealtime to Realtime is part of the effort to make the API more Swift-friendly.


470-470: Updated method injection target.

The method injection target is updated to use DataEncoder.decode(_:encoding:) instead of ARTDataEncoder.decode(_:encoding:), reflecting the updated class names.


444-444: Updated class name usage.

The use of Rest instead of ARTRest is consistent with the new naming conventions.


455-455: Updated class name usage in realtime context.

The change from ARTRealtime to Realtime is part of the effort to make the API more Swift-friendly.


470-470: Updated method injection target.

The method injection target is updated to use DataEncoder.decode(_:encoding:) instead of ARTDataEncoder.decode(_:encoding:), reflecting the updated class names.

Test/Tests/PushTests.swift (20)

5-5: Renaming of ARTRest to Rest is consistent and clear.

The change aligns with the PR's objective to make the Swift interface more idiomatic. The renaming has been applied consistently across the file.

Also applies to: 36-36, 129-129, 235-235


53-53: Renaming of ARTPushActivationEventCalledActivate to PushActivationEventCalledActivate is consistent and clear.

The change aligns with the PR's objective to make the Swift interface more idiomatic. The renaming has been applied consistently across the file.

Also applies to: 66-66, 88-88, 102-102, 161-161, 197-197


129-129: Renaming of ARTClientOptions to ClientOptions is consistent and clear.

The change aligns with the PR's objective to make the Swift interface more idiomatic. The renaming has been applied consistently across the file.

Also applies to: 255-255, 283-283


221-221: Renaming of ARTRealtime to Realtime is consistent and clear.

The change aligns with the PR's objective to make the Swift interface more idiomatic. The renaming has been applied consistently across the file.

Also applies to: 263-263, 287-287


227-227: Renaming of ARTDeviceIdentityTokenDetails to DeviceIdentityTokenDetails is consistent and clear.

The change aligns with the PR's objective to make the Swift interface more idiomatic. The renaming has been applied consistently across the file.

Also applies to: 383-383, 545-545


109-109: Renaming of ARTPush to Push is consistent and clear.

The change aligns with the PR's objective to make the Swift interface more idiomatic. The renaming has been applied consistently across the file.

Also applies to: 201-202, 215-215, 432-432, 410-410


220-220: Renaming of ARTDeviceDetails to DeviceDetails is consistent and clear.

The change aligns with the PR's objective to make the Swift interface more idiomatic. The renaming has been applied consistently across the file.


220-220: Renaming of ARTPushChannelSubscription to PushChannelSubscription is consistent and clear.

The change aligns with the PR's objective to make the Swift interface more idiomatic. The renaming has been applied consistently across the file.


220-220: Renaming of ARTLocalDevice to LocalDevice is consistent and clear.

The change aligns with the PR's objective to make the Swift interface more idiomatic. The renaming has been applied consistently across the file.


137-137: Renaming of ARTPushActivationStateMachine to PushActivationStateMachine is consistent and clear.

The change aligns with the PR's objective to make the Swift interface more idiomatic. The renaming has been applied consistently across the file.

Also applies to: 484-484


235-235: Renaming of ARTAPNSDeviceTokenKey to APNSDeviceTokenKey is consistent and clear.

The change aligns with the PR's objective to make the Swift interface more idiomatic. The renaming has been applied consistently across the file.


235-235: Renaming of ARTDeviceIdKey to DeviceIdKey is consistent and clear.

The change aligns with the PR's objective to make the Swift interface more idiomatic. The renaming has been applied consistently across the file.


235-235: Renaming of ARTDeviceSecretKey to DeviceSecretKey is consistent and clear.

The change aligns with the PR's objective to make the Swift interface more idiomatic. The renaming has been applied consistently across the file.


235-235: Renaming of ARTDeviceIdentityTokenKey to DeviceIdentityTokenKey is consistent and clear.

The change aligns with the PR's objective to make the Swift interface more idiomatic. The renaming has been applied consistently across the file.


263-263: Renaming of ARTClientIdKey to ClientIdKey is consistent and clear.

The change aligns with the PR's objective to make the Swift interface more idiomatic. The renaming has been applied consistently across the file.


109-109: Renaming of ARTAblyErrorDomain to AblyErrorDomain is consistent and clear.

The change aligns with the PR's objective to make the Swift interface more idiomatic. The renaming has been applied consistently across the file.


129-129: Renaming of ARTTokenDetails to TokenDetails is consistent and clear.

The change aligns with the PR's objective to make the Swift interface more idiomatic. The renaming has been applied consistently across the file.

Also applies to: 255-255, 283-283, 321-321


197-197: Renaming of ARTAPNSDeviceToken-default to APNSDeviceToken-default is consistent and clear.

The change aligns with the PR's objective to make the Swift interface more idiomatic. The renaming has been applied consistently across the file.


197-197: Renaming of ARTAPNSDeviceToken-location to APNSDeviceToken-location is consistent and clear.

The change aligns with the PR's objective to make the Swift interface more idiomatic. The renaming has been applied consistently across the file.


423-423: Renaming of ARTPushRegistererDelegate to PushRegistererDelegate is consistent and clear.

The change aligns with the PR's objective to make the Swift interface more idiomatic. The renaming has been applied consistently across the file.

Test/Tests/RestClientChannelTests.swift (5)

39-39: Refactor: Update to new class naming convention.

The instantiation of Rest from ARTRest is consistent with the PR's objectives to make the Swift interface more idiomatic.

This change is approved as it aligns with the PR's goals.


42-43: Refactor: Update to new class and method naming convention.

The use of CipherParams and Crypto.generateRandomKey from ARTCipherParams and ARTCrypto.generateRandomKey is another example of the shift towards a more idiomatic Swift interface.

This change is approved as it aligns with the PR's goals.


49-49: Refactor: Consistent use of updated class names in test setup.

The use of ChannelOptions instead of ARTChannelOptions is correct and aligns with the new naming conventions introduced in the PR.

This change is approved as it aligns with the PR's goals.


133-138: Refactor: Use of updated class names in test environment setup.

The update from ARTRest to Rest in the TestEnvironment struct is consistent with the overall refactoring strategy of the PR.

This change is approved as it aligns with the PR's goals.


151-152: Refactor: Consistent application of new naming conventions across various test methods.

Throughout the file, the changes from ART prefixed classes to their Rest counterparts are applied consistently. This includes updates in method calls, error handling, and message publishing, which are crucial for maintaining functionality with the new interface.

All these changes are approved as they consistently apply the new naming conventions and maintain the functionality of the tests.

Also applies to: 174-175, 197-198, 220-221, 244-245, 276-277, 310-313, 359-359, 387-389, 420-422, 452-453, 483-485, 524-524, 548-548, 564-566, 604-604, 619-619, 637-646, 657-660, 686-691, 721-724, 749-749, 774-779, 803-811, 833-857, 887-891, 920-925, 956-973, 1034-1050, 1094-1109, 1116-1125, 1158-1165, 1197-1200, 1218-1226, 1243-1243, 1544-1558, 1585-1587, 1630-1643

Test/Tests/RealtimeClientTests.swift (5)

5-6: Refactor: Updated variable declaration to use new class name.

The update from ARTStatsQuery to StatsQuery is consistent with the PR's objective to make the Swift interface more idiomatic. This change should not affect the functionality as it's a straightforward renaming.


15-15: Refactor: Method signature updated to use new error class name.

The method signatures for checkError have been updated to use ErrorInfo instead of ARTErrorInfo. This change is part of the renaming effort and should not impact the functionality as long as ErrorInfo is functionally equivalent to ARTErrorInfo.

Also applies to: 23-23


37-37: Refactor: Static variable updated to use new class names.

The static variable sharedClient now uses Realtime and ClientOptions instead of ARTRealtime and ARTClientOptions. This change aligns with the renaming strategy and should be transparent in terms of functionality.


67-67: Refactor: Updated client instantiation to use new class names.

The instantiation of Realtime clients throughout the tests has been updated to use the new class names. This is a repetitive change but crucial for maintaining consistency across the test suite.

Also applies to: 107-107, 133-133, 160-160, 212-212, 247-247, 257-257, 273-273, 283-283, 308-308, 325-325, 370-370, 388-388, 440-440, 498-498, 559-559, 651-651, 713-713, 762-762, 806-806, 841-841, 898-898, 929-929, 980-980, 1022-1022, 1068-1068, 1124-1124, 1180-1180, 1236-1236, 1360-1360, 1399-1400, 1466-1466, 1484-1484, 1520-1520, 1568-1568, 1586-1586, 1604-1604, 1622-1622, 1640-1640


90-90: Best Practice: Default property values confirmed.

The tests confirm that the default values for echoMessages and autoConnect are set to true. This is good as it ensures that any changes to the default behavior of these options are caught during testing.

Also applies to: 96-96

Test/Test Utilities/TestUtilities.swift (2)

161-161: Use Swift's error handling to manage ErrorInfo creation.

The functions newErrorProtocolMessage and newPresenceProtocolMessage create instances of ProtocolMessage with potential errors. It's important to ensure that the creation of ErrorInfo is handled correctly to avoid runtime exceptions.

Also applies to: 168-168


107-107: Ensure proper error handling in commonAppSetup.

The function commonAppSetup has complex logic for setting up application configurations and might throw errors. It's crucial to ensure that all possible error states are handled appropriately, especially when dealing with network requests and JSON parsing.

Test/Tests/PushActivationStateMachineTests.swift (45)

5-8: Refactor: Updated variable types to reflect new class names.

The changes from ARTRest to Rest and ARTPushActivationStateMachine to PushActivationStateMachine are consistent with the PR's objective to make the interface more idiomatic for Swift users. This renaming improves readability and aligns with Swift naming conventions.


14-14: Refactor: Updated variable type for consistency.

The update from ARTPushActivationStateMachine to PushActivationStateMachine is correctly applied here as well, ensuring consistency across the file.


39-39: Refactor: Constructor updated to new class names.

The constructor correctly reflects the new class names, ensuring that the object is instantiated with the updated type. This change is crucial for maintaining functionality with the updated class definitions.


49-49: Refactor: Assertion updated to new state class name.

The test assertion has been updated from ARTPushActivationStateNotActivated to PushActivationStateNotActivated, which is part of the renaming effort to make the codebase more Swift-idiomatic.


55-55: Refactor: Constructor updated to new class names within test context.

This line correctly updates the instantiation of PushActivationStateMachine within a test case, reflecting the new class names accurately.


69-69: Refactor: Updated constructor usage in test setup.

The change in the constructor call from ARTPushActivationStateMachine to PushActivationStateMachine is correctly applied, ensuring that the test setup remains functional with the new class definitions.


76-78: Refactor: Consistent use of updated class names in test setup.

The test setup method correctly uses the new PushActivationStateMachine class, ensuring consistency and correctness in the test environment setup.


122-125: Refactor: Updated constructor usage in complex test scenario.

This change correctly reflects the updated class names in a more complex test setup scenario, ensuring that the new class definitions are used consistently throughout the test suite.


135-138: Refactor: Constructor usage updated in test scenario involving secrets.

The update to use PushActivationStateMachine in this test scenario is correctly applied, ensuring that the test remains functional with the new class definitions.


152-157: Refactor: Updated constructor usage in test scenario involving client IDs.

This line correctly updates the constructor usage to reflect the new class names, ensuring that the test logic is consistent with the updated API.


182-185: Refactor: Event handling updated to new class names.

The test logic here is updated to handle events using the new class names, ensuring that the event handling logic remains consistent with the rest of the codebase.


197-198: Refactor: Assertion updated to new state class name in test scenario.

The assertion here is updated to reflect the new state class name, ensuring that the test checks for the correct state after an event is processed.


205-207: Refactor: Event handling and assertion updated to new class names.

This change correctly updates the event handling and assertion to use the new class names, ensuring consistency and correctness in the test logic.


214-216: Refactor: Updated constructor usage in test setup for a specific state.

The test setup method here correctly uses the new PushActivationStateMachine class for setting up a specific state, ensuring that the test environment is correctly initialized.


223-224: Refactor: Assertion updated to new state class name in activation scenario.

This line correctly updates the assertion to check for the new state class name after an activation event, ensuring that the test remains accurate.


237-238: Refactor: Event handling and assertion updated to new class names in deactivation scenario.

The changes here update the event handling and assertion to reflect the new class names, ensuring that the deactivation logic is tested correctly.


248-259: Refactor: Complex event handling logic updated to new class names.

This segment updates a complex piece of event handling logic to use the new class names, ensuring that the test logic is consistent and correct.


270-273: Refactor: Event handling updated to new class names in registration scenario.

This change updates the event handling logic to use the new class names, ensuring that the registration logic is tested correctly.


Line range hint 281-293: Refactor: Complex event handling logic updated to handle registration failures.

This segment updates the event handling logic to correctly handle registration failures using the new class names, ensuring that the test logic remains robust and accurate.


309-309: Refactor: Assertion updated to new state class name after handling registration failure.

This line correctly updates the assertion to check for the new state class name after handling a registration failure, ensuring that the test checks for the correct state.


Line range hint 317-342: Refactor: Comprehensive event handling and assertions updated to new class names.

This large segment of code updates various aspects of event handling and assertions to reflect the new class names, ensuring comprehensive coverage and correctness in the test logic.


Line range hint 367-391: Refactor: Comprehensive event handling and assertions updated for registration failure scenarios.

This segment updates the event handling and assertions for registration failure scenarios to use the new class names, ensuring that these scenarios are tested accurately.


Line range hint 412-434: Refactor: Event handling logic updated to new class names in device registration scenario.

This change updates the event handling logic to reflect the new class names in a device registration scenario, ensuring that the test logic is consistent and correct.


Line range hint 444-457: Refactor: Event handling and assertion updated to new class names in failure handling scenario.

This segment updates the event handling and assertion to use the new class names in a scenario where device details handling fails, ensuring that the test logic remains robust.


464-472: Refactor: Constructor usage updated in test scenario involving device token persistence.

This line correctly updates the constructor usage to reflect the new class names in a test scenario that involves persisting a device token, ensuring that the test setup is correct.


484-486: Refactor: Updated constructor usage in test setup for device registration.

The test setup method here correctly uses the new PushActivationStateMachine class for setting up a device registration scenario, ensuring that the test environment is correctly initialized.


494-495: Refactor: Assertion updated to new state class name in device registration scenario.

This line correctly updates the assertion to check for the new state class name after a device registration event, ensuring that the test remains accurate.


516-525: Refactor: Comprehensive test logic updated to new class names in identity token handling scenario.

This segment updates various aspects of the test logic to reflect the new class names in a scenario involving handling of identity token details, ensuring comprehensive coverage and correctness.


535-548: Refactor: Event handling and assertion updated to new class names in registration failure scenario.

This segment updates the event handling and assertion to use the new class names in a registration failure scenario, ensuring that the test logic remains robust and accurate.


555-557: Refactor: Updated constructor usage in test setup for handling new device details.

The test setup method here correctly uses the new PushActivationStateMachine class for setting up a scenario involving new device details, ensuring that the test environment is correctly initialized.


571-572: Refactor: Assertion updated to new state class name in new device details scenario.

This line correctly updates the assertion to check for the new state class name after handling new device details, ensuring that the test checks for the correct state.


Line range hint 609-631: Refactor: Comprehensive test logic updated to new class names in registration sync scenario.

This large segment of code updates various aspects of the test logic to reflect the new class names in a registration sync scenario, ensuring comprehensive coverage and correctness.

Tools
SwiftLint

[Error] 616-616: Force casts should be avoided

(force_cast)


666-681: Refactor: Comprehensive event handling and assertions updated to new class names in registration synced scenario.

This segment updates the event handling and assertions for a registration synced scenario to use the new class names, ensuring that these scenarios are tested accurately.


Line range hint 690-717: Refactor: Comprehensive event handling and assertions updated for registration failure scenarios.

This segment updates the event handling and assertions for registration failure scenarios to use the new class names, ensuring that these scenarios are tested accurately.


732-761: Refactor: Comprehensive test logic updated to new class names in various registration scenarios.

This large segment of code updates various aspects of the test logic to reflect the new class names in multiple registration scenarios, ensuring comprehensive coverage and correctness.


767-769: Refactor: Updated constructor usage in test setup for handling registration sync failures.

The test setup method here correctly uses the new PushActivationStateMachine class for setting up a scenario involving registration sync failures, ensuring that the test environment is correctly initialized.


837-839: Refactor: Updated constructor usage in test setup for handling deregistration.

The test setup method here correctly uses the new PushActivationStateMachine class for setting up a scenario involving deregistration, ensuring that the test environment is correctly initialized.


846-847: Refactor: Assertion updated to new state class name in deregistration scenario.

This line correctly updates the assertion to check for the new state class name after a deregistration event, ensuring that the test remains accurate.


Line range hint 852-888: Refactor: Comprehensive event handling and assertions updated to new class names in deregistration scenario.

This large segment of code updates various aspects of the event handling and assertions to reflect the new class names in a deregistration scenario, ensuring comprehensive coverage and correctness.


901-914: Refactor: Event handling and assertion updated to new class names in deregistration failure scenario.

This segment updates the event handling and assertion to use the new class names in a deregistration failure scenario, ensuring that the test logic remains robust and accurate.


Line range hint 922-965: Refactor: Comprehensive test logic updated to new class names in client ID handling scenarios.

This large segment of code updates various aspects of the test logic to reflect the new class names in scenarios involving handling of different client IDs, ensuring comprehensive coverage and correctness.


974-1002: Refactor: Comprehensive event handling and assertions updated to new class names in event queuing scenario.

This large segment of code updates various aspects of the event handling and assertions to reflect the new class names in a scenario involving event queuing, ensuring comprehensive coverage and correctness.


1011-1018: Refactor: Comprehensive test logic updated to new class names in atomic event handling scenario.

This segment of code updates various aspects of the test logic to reflect the new class names in a scenario involving atomic event handling, ensuring comprehensive coverage and correctness.


1410-1428: Refactor: Updated delegate methods to new error class names.

The delegate methods have been updated to use ErrorInfo instead of ARTErrorInfo, aligning with the new error handling conventions in Swift and improving the idiomatic usage of Swift in the codebase.


Line range hint 1436-1461: Refactor: Updated custom callback methods to new class and error names.

The custom callback methods have been updated to reflect the new class and error names, ensuring that custom registration and deregistration logic is handled using the updated conventions.

Test/Tests/RestClientTests.swift (2)

8-11: Updated Class Name Usage: ClientOptions and Rest

The usage of ClientOptions instead of ARTClientOptions and Rest instead of ARTRest aligns with the PR's goal to make the interface more idiomatic for Swift users. This change is correctly reflected in the test setup.


27-28: Comprehensive Update to Use New Class Names and Options

Throughout the file, the new class names (Rest and ClientOptions) and updated options are used consistently. This change supports the PR's objective to streamline and modernize the API interface for Swift developers.

Also applies to: 51-52, 87-90, 117-120, 154-154, 178-178, 187-187, 192-192, 201-202, 204-204, 212-212, 222-222, 233-233, 242-244, 247-249, 253-253, 258-258, 266-267, 281-287, 289-290, 292-292, 298-298, 302-303, 313-315, 326-329, 341-343, 354-355, 368-368, 379-383, 390-394, 412-415, 440-443, 464-464, 474-478, 498-498, 514-514, 533-533, 557-558, 583-583, 613-613, 645-647, 649-649, 661-661, 692-699, 713-713, 737-737, 762-783, 802-802, 829-831, 835-835, 852-857, 860-861, 865-865, 872-874, 878-880, 900-901, 927-929, 953-956, 980-982, 1006-1086, 1110-1112, 1137-1139, 1165-1167, 1194-1197, 1220-1222, 1244-1246, 1272-1275, 1299-1301, 1308-1308, 1317-1317, 1326-1328, 1349-1349, 1356-1365, 1392-1395, 1409-1409, 1415-1415, 1422-1427, 1455-1460, 1474-1474, 1488-1492, 1506-1506, 1521-1525, 1563-1564, 1635-1635, 1673-1673, 1707-1718, 1733-1733, 1755-1763, 1776-1776, 1809-1809, 1837-1837, 1864-1864, 1889-1898, 1907-1907, 1913-1922, 1928-1937, 1945-1957, 1995-1995, 2048-2048, 2068-2071, 2089-2089, 2107-2112, 2148-2151

Test/Tests/AuthTests.swift (19)

81-81: Review: Proper use of exception expectation in test

The test correctly sets up an expectation for an exception when TLS is disabled, ensuring that the client enforces HTTPS connections.


88-88: Review: Correct use of custom HTTP executor and header verification

The test effectively uses a custom HTTP executor to intercept and verify the Authorization header, ensuring the API key is correctly sent.


113-113: Review: Direct verification of authentication method

The test directly checks if the default authentication method is used when an API key is set, using clear and appropriate assertions.


125-125: Review: Effective verification of HTTP usage in token authentication

The test correctly sets up the client to use HTTP and verifies the URL scheme of the request, ensuring token authentication works over HTTP.


144-144: Review: Effective verification of HTTPS usage in token authentication

The test correctly sets up the client to use HTTPS and verifies the URL scheme of the request, ensuring token authentication works over HTTPS.


166-166: Review: Correct interception and verification of the Authorization header

The test effectively intercepts the network request and verifies that the token is correctly sent in the Authorization header for REST requests.


196-196: Review: Effective use of mock transport to verify token in query string

The test correctly uses a mock transport to capture the URL and verify that the token is included in the query string for Realtime connections.


239-239: Review: Direct verification of default authentication method when useTokenAuth is set

The test effectively uses a helper function to set the useTokenAuth option and verify that the default authentication method is used.


275-275: Review: Direct verification of default authentication method when authUrl is set

The test effectively uses a helper function to set the authUrl option and verify that the default authentication method is used.


310-310: Review: Direct verification of default authentication method when authCallback is set

The test effectively uses a helper function to set the authCallback option and verify that the default authentication method is used.


372-372: Review: Direct verification of default authentication method when tokenDetails is set

The test effectively uses a helper function to set the tokenDetails option and verify that the default authentication method is used.


407-407: Review: Direct verification of default authentication method when token is set

The test effectively uses a helper function to set the token option and verify that the default authentication method is used.


438-438: Review: Direct verification of default authentication method when key is set

The test effectively uses a helper function to set the key option and verify that the default authentication method is used.


239-239: Review: Correct handling of token errors without renewal options

The test effectively simulates a server error response and verifies that the correct error handling is implemented when there is no option to renew the token.


275-275: Review: Effective verification of connection state transition during token errors

The test correctly simulates a token error response and verifies that the connection transitions to the FAILED state, ensuring robust error handling in the connection lifecycle.


310-310: Review: Correct simulation of token reissuance and retry mechanisms

The test effectively simulates a token error, verifies the reissuance of the token, and retries the REST request, ensuring resilience in token handling.


338-338: Review: Comprehensive testing of error handling in token-related failures

The test effectively simulates multiple token-related failures and verifies the appropriate error handling, ensuring comprehensive testing of error scenarios.


372-372: Review: Effective verification of connection state transition during token creation failures in Realtime

The test correctly simulates a token creation failure in a Realtime context and verifies that the connection moves to the DISCONNECTED state and reports the error, ensuring robust error handling in the connection lifecycle.


407-407: Review: Effective verification of connection state transition during terminal token errors in Realtime

The test correctly simulates a terminal token error in a Realtime context and verifies that the connection moves to the FAILED state and reports the error, ensuring robust error handling in the connection lifecycle.

Ably.xcodeproj/project.pbxproj (24)

2880-2880: Updated Swift Migration Version

The LastSwiftMigration value has been updated to 1530, aligning with the PR's objective to utilize more recent Swift features and standards. This change is consistent and necessary for the migration to newer Swift versions.


2894-2899: Consistent Swift Migration Updates Across Configurations

The LastSwiftMigration value has been uniformly updated to 1530 in multiple configurations, ensuring consistency across the project. This is crucial for maintaining a uniform development environment.


3632-3632: Build Library for Distribution Setting

The BUILD_LIBRARY_FOR_DISTRIBUTION is set to NO, which is appropriate given the context of the PR. This setting helps in controlling the distribution of the library, especially when there are significant changes like Swift migration.


3642-3642: iOS Deployment Target Updated

The IPHONEOS_DEPLOYMENT_TARGET has been updated to 14.0, aligning with the PR's objective to support newer iOS versions. This is a necessary update to ensure compatibility with modern iOS features.


3661-3661: Consistency in Build Library for Distribution Setting

The setting BUILD_LIBRARY_FOR_DISTRIBUTION remains consistent across different configurations, set to NO. This ensures that the library's distribution settings are uniform, avoiding potential build issues.


3672-3672: Consistent iOS Deployment Target Update

The IPHONEOS_DEPLOYMENT_TARGET update to 14.0 is consistently applied across configurations, ensuring that all builds will support the same minimum iOS version.


3691-3695: Application Extension API Only Setting

The addition of APPLICATION_EXTENSION_API_ONLY set to YES and BUILD_LIBRARY_FOR_DISTRIBUTION set to NO are significant. These settings ensure that the library can be safely used in application extensions while controlling its distribution.


3743-3751: Header Search Paths and Swift Version Update

The HEADER_SEARCH_PATHS has been adjusted to include more specific paths, which is crucial for locating header files correctly. The SWIFT_VERSION update to 5.0 aligns with the PR's objectives to use the latest Swift features.


3763-3767: Consistency in Application Extension and Distribution Settings

The settings APPLICATION_EXTENSION_API_ONLY and BUILD_LIBRARY_FOR_DISTRIBUTION are consistently applied, ensuring that the library adheres to extension guidelines and controls distribution.


3808-3817: Comprehensive Update to Header Search Paths and Swift Settings

The HEADER_SEARCH_PATHS and SWIFT_VERSION settings are updated to enhance compatibility and leverage the latest Swift features. This comprehensive update supports the PR's goals for modernization and compatibility.


3829-3840: Build and Header Configuration Consistency

The BUILD_LIBRARY_FOR_DISTRIBUTION setting remains NO, and the HEADER_SEARCH_PATHS have been expanded. These changes are consistent and necessary for the library's build configuration.


3853-3857: Support for Mac Catalyst and Swift Configuration

The setting SUPPORTS_MACCATALYST set to YES and the SWIFT_VERSION update to 5.0 are important for supporting Mac Catalyst and using the latest Swift features. This aligns with the PR's objectives to enhance compatibility across platforms.


3864-3875: Release Configuration Consistency

The settings for BUILD_LIBRARY_FOR_DISTRIBUTION and HEADER_SEARCH_PATHS are consistently applied in the release configuration, ensuring uniformity and correctness in the build settings.


3888-3891: Mac Catalyst Support and Swift Configuration in Release

The SUPPORTS_MACCATALYST and SWIFT_VERSION settings are applied consistently in the release configuration, supporting Mac Catalyst and leveraging the latest Swift features.


3898-3898: Build Distribution and Analyzer Settings in Debug

The BUILD_LIBRARY_FOR_DISTRIBUTION setting is NO, and the analyzer settings are aggressive, which is suitable for a debug configuration to catch potential issues early.


3935-3935: Consistent Analyzer and Build Settings in Release

The analyzer settings and the BUILD_LIBRARY_FOR_DISTRIBUTION setting are consistently applied in the release configuration, ensuring that the library's quality and distribution are controlled.


3970-3970: Debug Configuration for Build Distribution and Analyzers

The BUILD_LIBRARY_FOR_DISTRIBUTION setting remains NO, and the analyzer settings are aggressive, suitable for debugging and ensuring code quality.


3999-3999: tvOS Deployment Target Update

The TVOS_DEPLOYMENT_TARGET has been updated to 14.0, aligning with the PR's objective to support newer tvOS versions. This update ensures compatibility with modern tvOS features.


4006-4006: Consistent Build and Analyzer Settings in Release

The BUILD_LIBRARY_FOR_DISTRIBUTION setting is NO, and the analyzer settings are consistently applied, ensuring that the library's release configuration is robust and controlled.


4033-4033: tvOS Deployment Target Consistency in Release

The TVOS_DEPLOYMENT_TARGET update to 14.0 is consistently applied in the release configuration, supporting the PR's goals for tvOS compatibility.


4063-4063: macOS Deployment Target Update

The MACOSX_DEPLOYMENT_TARGET has been updated to 10.14, which is necessary to support newer macOS versions and align with the PR's objectives for broader platform support.


4103-4103: Consistent macOS Deployment Target Update

The MACOSX_DEPLOYMENT_TARGET update to 10.14 is consistently applied, ensuring that the library supports the same macOS version across different configurations.


4148-4148: tvOS Deployment Target Update in Debug

The TVOS_DEPLOYMENT_TARGET update to 14.0 in the debug configuration aligns with the PR's objective to support newer tvOS versions, ensuring compatibility with modern features.


4185-4185: tvOS Deployment Target Consistency in Release

The TVOS_DEPLOYMENT_TARGET update to 14.0 is consistently applied in the release configuration, supporting the PR's goals for tvOS compatibility.

Test/Tests/LogAdapterTests.swift (4)

5-8: Class and method updates are well-implemented.

The renaming of MockARTLog to MockLog and the change in superclass to AblyLogger align well with the PR's objectives to make the Swift interface more idiomatic. The updated log method correctly handles the new LogLevel type.


14-17: Test method updates are appropriate.

The updates to use MockLog and the new LogLevel array in test_logMessage are consistent with the broader changes in the logging framework. These changes ensure that the tests will continue to verify the correct behavior under the new logging setup.


29-29: Logger instantiation update is correct.

The update to instantiate AblyLogger in test_logLevel aligns with the new logging framework. This ensures that the log level functionality is tested with the correct logger type.


37-37: Logger instantiation and log level setting are correctly updated.

The changes in test_setLogLevel to use AblyLogger and test the setting of logLevel are appropriate and ensure that the functionality is correctly verified under the new logging framework.

Package.swift (1)

9-9: Deployment target update is appropriate.

Updating the iOS deployment target from version 9 to 13 in Package.swift is a significant change that aligns with modern iOS development standards. This update ensures that the library can leverage newer iOS features and improvements.

Source/include/Ably/ARTProtocolMessage.h (1)

14-32: Swift naming conventions are correctly applied.

The application of NS_SWIFT_NAME to the ARTProtocolMessageAction enumeration and the ARTProtocolMessageActionToStr function enhances Swift interoperability. These changes make the API more idiomatic and user-friendly for Swift developers.

Also applies to: 36-36

Source/include/Ably/ARTPushChannel.h (2)

13-13: Swift Naming Convention Applied Correctly

The use of NS_SWIFT_NAME(PushChannelProtocol) for the protocol is correctly applied and enhances the Swift interoperability of the API.


87-87: Swift Naming Convention Applied Correctly

The use of NS_SWIFT_NAME(PushChannel) for the interface is correctly applied and enhances the Swift interoperability of the API.

Test/Tests/UtilitiesTests.swift (2)

15-16: Class Renaming in Tests Reflects Main Library Changes

The renaming of ARTEventListener to EventListener in the test variables is correctly applied, reflecting the changes in the main library to simplify and modernize the naming convention.


69-72: Updated Class Names in Test Methods

The updates in test methods to use the new class names (ProtocolMessage, Message, Realtime, Rest) are correctly applied. These changes ensure that the tests are aligned with the updated naming conventions in the main library.

Also applies to: 77-77, 112-126, 142-142, 180-193, 209-209, 468-468, 474-475, 481-483

Examples/Tests/Tests.xcodeproj/project.pbxproj (1)

420-420: iOS Deployment Target Updated to 14.0

The update of the IPHONEOS_DEPLOYMENT_TARGET to 14.0 across various sections of the project configuration file is correctly applied. This change aligns with the PR's objective to support newer iOS versions and ensures compatibility with iOS 14 features.

Also applies to: 471-471, 486-486, 500-500

Test/Tests/PushAdminTests.swift (3)

5-8: Renaming of types to align with Swift idioms is consistent and clear.

The renaming of ARTRest to Rest and ARTLocalDevice to LocalDevice is consistent with the PR's objective to make the Swift interface more idiomatic. This change should help Swift developers interact with the library more naturally.

The changes are approved as they enhance readability and maintain consistency.


22-22: Renaming of ARTPushChannelSubscription to PushChannelSubscription is appropriate.

This change simplifies the type name and removes unnecessary prefixes, making the code cleaner and more in line with Swift naming conventions.

The renaming is approved as it improves the clarity and usability of the library for Swift developers.


Line range hint 25-37: Comprehensive renaming across various test methods and properties.

The renaming of various properties and methods from ART prefixed classes to their simplified versions is consistent throughout the file. This change is part of the broader refactoring to make the library's Swift interface more idiomatic. It's important to ensure that these changes do not introduce any behavioral changes to the tests, which seems to be handled well here as the logic of the tests remains unchanged.

The renaming efforts are approved as they enhance the maintainability and readability of the test suite.

Also applies to: 50-63, 76-76, 83-88, 90-90, 107-115, 141-148, 186-186, 229-229, 262-262, 290-290, 317-317, 346-346, 362-362, 379-383, 409-409, 429-429, 445-445, 461-461, 477-477, 497-497, 520-520, 541-545, 575-575, 599-599, 675-675, 696-698, 709-711, 726-730, 759-763, 782-782, 803-803, 822-822, 854-858, 887-891, 912-912, 968-968, 1014-1014, 1058-1058

Test/Tests/RealtimeClientChannelsTests.swift Show resolved Hide resolved
Test/Tests/RestClientChannelsTests.swift Show resolved Hide resolved
Source/ARTTypes.m Show resolved Hide resolved
Source/ARTTypes.m Show resolved Hide resolved
Test/Tests/StatsTests.swift Show resolved Hide resolved
Test/Test Utilities/TestUtilities.swift Show resolved Hide resolved
Test/Test Utilities/TestUtilities.swift Show resolved Hide resolved
Test/Test Utilities/TestUtilities.swift Show resolved Hide resolved
Test/Test Utilities/TestUtilities.swift Show resolved Hide resolved
Test/Test Utilities/TestUtilities.swift Show resolved Hide resolved
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 41589f4 and 71bc5ff.

Files selected for processing (1)
  • Source/include/Ably/ARTStatus.h (3 hunks)
Files skipped from review as they are similar to previous changes (1)
  • Source/include/Ably/ARTStatus.h

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants