fix(swift): retry strategy + cts test (#5654)

Author: Fluf22

clients/algoliasearch-client-swift/Sources/Core/Networking/RetryStrategy/AlgoliaRetryStrategy.swift

@@ -42,7 +42,6 @@ class AlgoliaRetryStrategy: RetryStrategy {
             }
 
             var hostIterator = self.hosts
-                .sorted { $0.lastUpdated.compare($1.lastUpdated) == .orderedAscending }
                 .filter { $0.supports(callType) && $0.isUp }
                 .makeIterator()
 

scripts/cts/runCts.ts

@@ -8,13 +8,14 @@ import type { Language } from '../types.ts';
 import { assertValidAccountCopyIndex } from './testServer/accountCopyIndex.ts';
 import { printBenchmarkReport } from './testServer/benchmark.ts';
 import { assertChunkWrapperValid } from './testServer/chunkWrapper.ts';
-import { assertValidErrors } from './testServer/error.ts';
+import { assertNeverCalledServerWasNotCalled, assertValidErrors } from './testServer/error.ts';
 import { startTestServer } from './testServer/index.ts';
 import { assertPushMockValid } from './testServer/pushMock.ts';
 import { assertValidReplaceAllObjects } from './testServer/replaceAllObjects.ts';
 import { assertValidReplaceAllObjectsFailed } from './testServer/replaceAllObjectsFailed.ts';
 import { assertValidReplaceAllObjectsScopes } from './testServer/replaceAllObjectsScopes.ts';
 import { assertValidReplaceAllObjectsWithTransformation } from './testServer/replaceAllObjectsWithTransformation.ts';
+import { assertSuccessServerCalled } from './testServer/success.ts';
 import { assertValidTimeouts } from './testServer/timeout.ts';
 import { assertValidWaitForApiKey } from './testServer/waitFor.ts';
 
@@ -158,6 +159,8 @@ export async function runCts(
 
     assertValidErrors(languages.length);
     assertValidTimeouts(languages.length);
+    assertNeverCalledServerWasNotCalled();
+    assertSuccessServerCalled(languages.length);
     assertChunkWrapperValid(languages.length - skip('dart'));
     assertValidReplaceAllObjects(languages.length - skip('dart'));
     assertValidReplaceAllObjectsWithTransformation(

scripts/cts/testServer/error.ts

@@ -6,6 +6,7 @@ import type express from 'express';
 import { setupServer } from './index.ts';
 
 const errorState: Record<string, { errorCount: number; maxError: number }> = {};
+const neverCalledState: Record<string, number> = {};
 
 export function assertValidErrors(expectedCount: number): void {
   // assert that the retry strategy uses the correct timings, by checking the time between each request, and how long each request took before being timed out
@@ -26,6 +27,15 @@ export function assertValidErrors(expectedCount: number): void {
   }
 }
 
+export function assertNeverCalledServerWasNotCalled(): void {
+  for (const [lang, callCount] of Object.entries(neverCalledState)) {
+    expect(callCount).to.equal(
+      0,
+      `Never-called server was called ${callCount} times for ${lang}, but should never be called`,
+    );
+  }
+}
+
 function addRoutes(app: express.Express): void {
   app.post('/1/test/error/:lang', (req, res) => {
     const lang = req.params.lang;
@@ -58,3 +68,24 @@ export function errorServerRetriedOnce(): Promise<Server> {
 export function errorServerRetriedTwice(): Promise<Server> {
   return setupServer('errorRetriedTwice', 6673, addRoutes);
 }
+
+function addNeverCalledRoutes(app: express.Express): void {
+  app.get('/1/test/calling/:lang', (req, res) => {
+    const lang = req.params.lang;
+    if (!neverCalledState[lang]) {
+      neverCalledState[lang] = 0;
+    }
+
+    neverCalledState[lang]++;
+
+    // This should never be reached if the retry strategy correctly reuses successful hosts
+    res.status(500).json({
+      message: 'This fallback server should never be called when the first host succeeds',
+      callCount: neverCalledState[lang],
+    });
+  });
+}
+
+export function neverCalledServer(): Promise<Server> {
+  return setupServer('neverCalled', 6674, addNeverCalledRoutes);
+}

scripts/cts/testServer/index.ts

@@ -12,13 +12,14 @@ import { algoliaMockServer } from './algoliaMock.ts';
 import { apiKeyServer } from './apiKey.ts';
 import { benchmarkServer } from './benchmark.ts';
 import { chunkWrapperServer } from './chunkWrapper.ts';
-import { errorServer, errorServerRetriedOnce, errorServerRetriedTwice } from './error.ts';
+import { errorServer, errorServerRetriedOnce, errorServerRetriedTwice, neverCalledServer } from './error.ts';
 import { gzipServer } from './gzip.ts';
 import { pushMockServer, pushMockServerRetriedOnce } from './pushMock.ts';
 import { replaceAllObjectsServer } from './replaceAllObjects.ts';
 import { replaceAllObjectsServerFailed } from './replaceAllObjectsFailed.ts';
 import { replaceAllObjectsScopesServer } from './replaceAllObjectsScopes.ts';
 import { replaceAllObjectsWithTransformationServer } from './replaceAllObjectsWithTransformation.ts';
+import { successServer } from './success.ts';
 import { timeoutServer } from './timeout.ts';
 import { timeoutServerBis } from './timeoutBis.ts';
 import { waitForApiKeyServer } from './waitFor.ts';
@@ -31,6 +32,8 @@ export async function startTestServer(suites: Record<CTSType, boolean>): Promise
       errorServer(),
       errorServerRetriedOnce(),
       errorServerRetriedTwice(),
+      neverCalledServer(),
+      successServer(),
       gzipServer(),
       timeoutServerBis(),
       accountCopyIndexServer(),

scripts/cts/testServer/success.ts

@@ -0,0 +1,48 @@
+import type { Server } from 'http';
+
+import { expect } from 'chai';
+import type express from 'express';
+
+import { setupServer } from './index.ts';
+
+const successState: Record<string, number> = {};
+
+export function assertSuccessServerCalled(expectedCount: number): void {
+  // Verify that the success server was called the expected number of times per language
+  if (Object.keys(successState).length !== expectedCount) {
+    throw new Error(`Expected ${expectedCount} language(s) to test the success server`);
+  }
+
+  for (const [lang, callCount] of Object.entries(successState)) {
+    // python has sync and async tests, each making 2 requests
+    if (lang === 'python') {
+      expect(callCount).to.equal(
+        4,
+        `Success server was called ${callCount} times for ${lang}, expected 4 (2 sync + 2 async)`,
+      );
+      continue;
+    }
+
+    // Each test makes 2 consecutive requests, both should hit this server
+    expect(callCount).to.equal(2, `Success server was called ${callCount} times for ${lang}, expected 2`);
+  }
+}
+
+function addRoutes(app: express.Express): void {
+  app.get('/1/test/calling/:lang', (req, res) => {
+    const lang = req.params.lang;
+    if (!successState[lang]) {
+      successState[lang] = 0;
+    }
+
+    successState[lang]++;
+
+    res.status(200).json({
+      message: 'success server response',
+    });
+  });
+}
+
+export function successServer(): Promise<Server> {
+  return setupServer('success', 6675, addRoutes);
+}

tests/CTS/client/search/api.json

@@ -336,5 +336,52 @@
         }
       }
     ]
+  },
+  {
+    "testName": "does not retry on success",
+    "autoCreateClient": false,
+    "steps": [
+      {
+        "type": "createClient",
+        "parameters": {
+          "appId": "test-app-id",
+          "apiKey": "test-api-key",
+          "customHosts": [
+            {
+              "port": 6675
+            },
+            {
+              "port": 6674
+            }
+          ]
+        }
+      },
+      {
+        "type": "method",
+        "method": "customGet",
+        "parameters": {
+          "path": "1/test/calling/${{language}}"
+        },
+        "expected": {
+          "type": "response",
+          "match": {
+            "message": "success server response"
+          }
+        }
+      },
+      {
+        "type": "method",
+        "method": "customGet",
+        "parameters": {
+          "path": "1/test/calling/${{language}}"
+        },
+        "expected": {
+          "type": "response",
+          "match": {
+            "message": "success server response"
+          }
+        }
+      }
+    ]
   }
 ]