Merge remote-tracking branch 'origin/main' into transition-refactoring

# Conflicts: # Source/ARTRealtime.m # Source/ARTRealtimeChannel.m # Test/Tests/RealtimeClientPresenceTests.swift
ably · Feb 23, 2024 · e1ea275 · e1ea275
2 parents fea657b + b23b804
commit e1ea275
Show file tree

Hide file tree

Showing 8 changed files with 82 additions and 36 deletions.
diff --git a/.github/workflows/integration-test.yaml b/.github/workflows/integration-test.yaml
@@ -115,5 +115,5 @@ jobs:
           TEST_OBSERVABILITY_SERVER_AUTH_KEY: ${{ secrets.TEST_OBSERVABILITY_SERVER_AUTH_KEY }}
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
         run: |
-           Scripts/upload_test_results.sh --job-index ${{ strategy.job-index }}
+           Scripts/upload_test_results.sh --job-name "check (${{ matrix.platform }}, ${{ matrix.lane }})"
 
diff --git a/Scripts/upload_test_results.sh b/Scripts/upload_test_results.sh
@@ -6,7 +6,7 @@
 # Options:
 # -u / --upload-server-base-url <url>: Allows you to specify a URL to use as the upload server base URL. Defaults to https://test-observability.herokuapp.com.
 # -i / --iteration <number>: If running the tests in a loop inside a single CI job, indicates which iteration of the loop is currently executing. Defaults to 1.
-# -j / --job-index <number>: The index to which the current job corresponds in the response from the "list jobs for a workflow run attempt" GitHub API (https://docs.github.com/en/rest/actions/workflow-jobs?apiVersion=2022-11-28#list-jobs-for-a-workflow-run-attempt). If you specify `GITHUB_TOKEN` but not `--job-index`, and the response from this API contains more than one job, the script will fail.
+# -j / --job-name <name> (optional): The `name` property of the object corresponding to the current job in the response from the "list jobs for a workflow run attempt" GitHub API (https://docs.github.com/en/rest/actions/workflow-jobs?apiVersion=2022-11-28#list-jobs-for-a-workflow-run-attempt). If there is more than one object in the response whose `name` property equals this value, the action will fail. If you specify `GITHUB_TOKEN` but not `--job-name`, and the response from this API contains more than one job, the script will fail.
 #
 # Optional environment variables:
 #
@@ -103,7 +103,7 @@ while [[ "$#" -gt 0 ]]; do
     case $1 in
         -i|--iteration) iteration="$2"; shift ;;
         -u|--upload-server-base-url) upload_server_base_url="$2"; shift ;;
-        -j|--job-index) job_index="$2"; shift ;;
+        -j|--job-name) job_name="$2"; shift ;;
         *) echo "Unknown parameter passed: $1"; exit 1 ;;
     esac
     shift
@@ -217,32 +217,45 @@ then
   temp_github_jobs_response_file=$(mktemp)
   gh api "/repos/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID}/attempts/${GITHUB_RUN_ATTEMPT}/jobs" > $temp_github_jobs_response_file
 
-  number_of_jobs=$(jq '.jobs | length' < "${temp_github_jobs_response_file}")
+  matching_jobs_file=$(mktemp)
 
-  if [[ -z $job_index && $number_of_jobs -gt 1 ]]
+  if [[ -z $job_name ]]
   then
-    echo -e "Got ${number_of_jobs} jobs from GitHub API but don’t know which one to pick. You need to provide a --job-index argument." 2>&1
-    exit 1
-  fi
+    number_of_jobs=$(jq '.jobs | length' < "${temp_github_jobs_response_file}")
 
-  if [[ -n $job_index ]]
-  then
-    if [[ $job_index -gt $number_of_jobs ]]
+    if [[ $number_of_jobs -eq 0 ]]
+    then
+      echo -e "Got no jobs from GitHub API." 2>&1
+      exit 1
+    fi
+
+    if [[ $number_of_jobs -gt 1 ]]
     then
-      echo -e "The --job-index argument has value ${job_index}, but there are only ${number_of_jobs} jobs. This script does not currently handle pagination." 2>&1
+      echo -e "Got ${number_of_jobs} jobs from GitHub API but don’t know which one to pick. You need to provide a --job-name argument." 2>&1
       exit 1
     fi
+
+    jq '.jobs' < "${temp_github_jobs_response_file}" > "${matching_jobs_file}"
   else
-    if [[ $number_of_jobs -eq 0 ]]
+    jq --arg job_name "${job_name}" '.jobs | map(select(.name == $job_name))' < "${temp_github_jobs_response_file}" > "${matching_jobs_file}"
+
+    number_of_matching_jobs=$(jq '. | length' < "${matching_jobs_file}")
+
+    if [[ $number_of_matching_jobs -eq 0 ]]
+    then
+      echo -e "The GitHub API response contains no job whose \`name\` is ${job_name}. This script does not currently handle pagination." 2>&1
+      exit 1
+    fi
+
+    if [[ $number_of_matching_jobs -gt 1 ]]
     then
-      echo -e "The GitHub API response contains no jobs." 2>&1
+      echo -e "The GitHub API response contains multiple jobs whose \`name\` is ${job_name}." 2>&1
       exit 1
     fi
-    job_index=0
   fi
 
-  github_job_api_url=$(jq --exit-status --raw-output ".jobs[${job_index}].url" < "${temp_github_jobs_response_file}")
-  github_job_html_url=$(jq --exit-status --raw-output ".jobs[${job_index}].html_url" < "${temp_github_jobs_response_file}")
+  github_job_api_url=$(jq --exit-status --raw-output ".[0].url" < "${matching_jobs_file}")
+  github_job_html_url=$(jq --exit-status --raw-output ".[0].html_url" < "${matching_jobs_file}")
 fi
 
 # 9. Create the JSON request body.

diff --git a/Source/ARTRealtime.m b/Source/ARTRealtime.m
@@ -615,7 +615,8 @@ - (void)performTransitionToState:(ARTRealtimeConnectionState)state withParams:(A
                                                                                         event:event
                                                                                        reason:params.errorInfo
                                                                                       retryIn:0
-                                                                                 retryAttempt:params.retryAttempt];
+                                                                                 retryAttempt:params.retryAttempt
+                                                                                      resumed:params.resumed];
 
     ARTLogDebug(self.logger, @"RT:%p realtime is transitioning from %tu - %@ to %tu - %@", self, stateChange.previous, ARTRealtimeConnectionStateToStr(stateChange.previous), stateChange.current, ARTRealtimeConnectionStateToStr(stateChange.current));
 

diff --git a/Source/ARTRealtimeChannel.m b/Source/ARTRealtimeChannel.m
@@ -735,13 +735,11 @@ - (void)setAttached:(ARTProtocolMessage *)message {
     if (message.hasPresence) {
         [self.presenceMap startSync];
     }
-    else if ([self.presenceMap.members count] > 0 || [self.presenceMap.localMembers count] > 0) {
-        if (!message.resumed) {
-            // When an ATTACHED message is received without a HAS_PRESENCE flag and PresenceMap has existing members
-            [self.presenceMap startSync];
-            [self.presenceMap endSync];
-            ARTLogDebug(self.logger, @"R:%p C:%p (%@) PresenceMap has been reset", _realtime, self, self.name);
-        }
+    else {
+        // RTP1 - when an ATTACHED message is received without a HAS_PRESENCE flag, reset PresenceMap
+        [self.presenceMap startSync];
+        [self.presenceMap endSync];
+        ARTLogDebug(self.logger, @"R:%p C:%p (%@) PresenceMap has been reset", _realtime, self, self.name);
     }
 
     if (self.state_nosync == ARTRealtimeChannelAttached) {
@@ -751,6 +749,7 @@ - (void)setAttached:(ARTProtocolMessage *)message {
             }
             ARTChannelStateChange *stateChange = [[ARTChannelStateChange alloc] initWithCurrent:self.state_nosync previous:self.state_nosync event:ARTChannelEventUpdate reason:message.error resumed:message.resumed];
             [self emit:stateChange.event with:stateChange];
+            [self.presenceMap reenterLocalMembers]; // RTP17i
         }
         return;
     }
@@ -765,7 +764,7 @@ - (void)setAttached:(ARTProtocolMessage *)message {
     [_attachedEventEmitter emit:nil with:nil];
 
     [self.presence sendPendingPresence];
-    [self.presenceMap reenterLocalMembers]; // RTP17f, RTP17g
+    [self.presenceMap reenterLocalMembers]; // RTP17i
 }
 
 - (void)setDetached:(ARTProtocolMessage *)message {

diff --git a/Source/ARTTypes.m b/Source/ARTTypes.m
@@ -45,10 +45,10 @@ - (instancetype)initWithCurrent:(ARTRealtimeConnectionState)current previous:(AR
 }
 
 - (instancetype)initWithCurrent:(ARTRealtimeConnectionState)current previous:(ARTRealtimeConnectionState)previous event:(ARTRealtimeConnectionEvent)event reason:(ARTErrorInfo *)reason retryIn:(NSTimeInterval)retryIn {
-    return [self initWithCurrent:current previous:previous event:event reason:reason retryIn:retryIn retryAttempt:nil];
+    return [self initWithCurrent:current previous:previous event:event reason:reason retryIn:retryIn retryAttempt:nil resumed:NO];
 }
 
-- (instancetype)initWithCurrent:(ARTRealtimeConnectionState)current previous:(ARTRealtimeConnectionState)previous event:(ARTRealtimeConnectionEvent)event reason:(ARTErrorInfo *)reason retryIn:(NSTimeInterval)retryIn retryAttempt:(ARTRetryAttempt *)retryAttempt {
+- (instancetype)initWithCurrent:(ARTRealtimeConnectionState)current previous:(ARTRealtimeConnectionState)previous event:(ARTRealtimeConnectionEvent)event reason:(ARTErrorInfo *)reason retryIn:(NSTimeInterval)retryIn retryAttempt:(ARTRetryAttempt *)retryAttempt resumed:(BOOL)resumed {
     self = [self init];
     if (self) {
         _current = current;
@@ -57,6 +57,7 @@ - (instancetype)initWithCurrent:(ARTRealtimeConnectionState)current previous:(AR
         _reason = reason;
         _retryIn = retryIn;
         _retryAttempt = retryAttempt;
+        _resumed = resumed;
     }
     return self;
 }

diff --git a/Source/PrivateHeaders/Ably/ARTTypes+Private.h b/Source/PrivateHeaders/Ably/ARTTypes+Private.h
@@ -15,12 +15,18 @@ NS_ASSUME_NONNULL_BEGIN
  */
 @property (nonatomic, readonly, nullable) ARTRetryAttempt *retryAttempt;
 
+/**
+ * Indicates whether the connection was resumed.
+ */
+@property (assign, nonatomic) BOOL resumed;
+
 - (instancetype)initWithCurrent:(ARTRealtimeConnectionState)current
                        previous:(ARTRealtimeConnectionState)previous
                           event:(ARTRealtimeConnectionEvent)event
                          reason:(nullable ARTErrorInfo *)reason
                         retryIn:(NSTimeInterval)retryIn
-                   retryAttempt:(nullable ARTRetryAttempt *)retryAttempt;
+                   retryAttempt:(nullable ARTRetryAttempt *)retryAttempt
+                        resumed:(BOOL)resumed;
 
 @end
 

diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift
@@ -2872,7 +2872,7 @@ class RealtimeClientPresenceTests: XCTestCase {
         }
     }
 
-    // RTP17f, RTP17g
+    // RTP17i, RTP17g
     func test__200__Presence__PresenceMap_should_perform_re_entry_whenever_a_channel_moves_into_the_attached_state_and_presence_message_consists_of_enter_action_with_client_id_and_data() throws {
         let test = Test()
         let options = try AblyTests.commonAppSetup(for: test)
@@ -2917,20 +2917,31 @@ class RealtimeClientPresenceTests: XCTestCase {
 
         expect(client.connection.state).toEventually(equal(ARTRealtimeConnectionState.connected), timeout: testTimeout)
 
-        // RTP17f
+        // RTP17i
 
         expect(channel.state).toEventually(equal(ARTRealtimeChannelState.attached), timeout: testTimeout)
         expect(channel.internal.presenceMap.localMembers).to(haveCount(2))
 
         let newTransport = client.internal.transport as! TestProxyTransport
         expect(newTransport).toNot(beIdenticalTo(transport))
 
-        let sentPresenceMessages = newTransport.protocolMessagesSent.filter({ $0.action == .presence }).compactMap { $0.presence?.first }
+        var sentPresenceMessages = newTransport.protocolMessagesSent.filter({ $0.action == .presence }).compactMap { $0.presence?.first }
 
         expect(sentPresenceMessages).to(haveCount(2))
 
-        let client1PresenceMessage = try! XCTUnwrap(sentPresenceMessages.first(where: { $0.clientId == firstClient }))
-        let client2PresenceMessage = try! XCTUnwrap(sentPresenceMessages.first(where: { $0.clientId == secondClient }))
+        let client1PresenceMessage = try XCTUnwrap(sentPresenceMessages.first(where: { $0.clientId == firstClient }))
+        let client2PresenceMessage = try XCTUnwrap(sentPresenceMessages.first(where: { $0.clientId == secondClient }))
+
+        // RTP17i - already attached with resume flag set
+
+        let attachedMessage = ARTProtocolMessage()
+        attachedMessage.action = .attached
+        attachedMessage.channel = channel.name
+        attachedMessage.flags = 4 // resume flag
+
+        newTransport.receive(attachedMessage)
+        sentPresenceMessages = newTransport.protocolMessagesSent.filter({ $0.action == .presence }).compactMap { $0.presence?.first }
+        expect(sentPresenceMessages).to(haveCount(2)) // no changes in sentPresenceMessages => no presense messages sent
 
         // RTP17g
 

diff --git a/Test/Tests/RestClientChannelTests.swift b/Test/Tests/RestClientChannelTests.swift
@@ -1635,13 +1635,28 @@ class RestClientChannelTests: XCTestCase {
                 done()
             }
         }
-        waitUntil(timeout: testTimeout) { done in
+
+        func checkMetrics(completion: @escaping (ARTChannelDetails) -> ()) {
             restChannel.status { details, error in
                 XCTAssertNil(error)
                 guard let details = details else {
-                    fail("Channel details are empty"); done()
+                    fail("Channel details are empty");
                     return
                 }
+                if details.status.occupancy.metrics.presenceMembers == 1 {
+                    completion(details)
+                }
+                else {
+                    // `presenceMembers` is updated with a delay, so we poll every second until it's fulfilled by the realtime
+                    delay(1.0) {
+                        checkMetrics(completion: completion)
+                    }
+                }
+            }
+        }
+
+        waitUntil(timeout: testTimeout) { done in
+            checkMetrics { details in
                 XCTAssertEqual(details.status.occupancy.metrics.connections, 1) // CHM2a
                 XCTAssertEqual(details.status.occupancy.metrics.publishers, 1) // CHM2e
                 XCTAssertEqual(details.status.occupancy.metrics.subscribers, 1) // CHM2f