diff --git a/clients/algoliasearch-client-swift/Sources/Core/Networking/RetryStrategy/AlgoliaRetryStrategy.swift b/clients/algoliasearch-client-swift/Sources/Core/Networking/RetryStrategy/AlgoliaRetryStrategy.swift index 00e036e38e6..cda8a7a702f 100644 --- a/clients/algoliasearch-client-swift/Sources/Core/Networking/RetryStrategy/AlgoliaRetryStrategy.swift +++ b/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() diff --git a/scripts/cts/runCts.ts b/scripts/cts/runCts.ts index 4395a595896..613f57f14de 100644 --- a/scripts/cts/runCts.ts +++ b/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( diff --git a/scripts/cts/testServer/error.ts b/scripts/cts/testServer/error.ts index aec810a2fbb..6e3322f19c8 100644 --- a/scripts/cts/testServer/error.ts +++ b/scripts/cts/testServer/error.ts @@ -6,6 +6,7 @@ import type express from 'express'; import { setupServer } from './index.ts'; const errorState: Record = {}; +const neverCalledState: Record = {}; 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 { export function errorServerRetriedTwice(): Promise { 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 { + return setupServer('neverCalled', 6674, addNeverCalledRoutes); +} diff --git a/scripts/cts/testServer/index.ts b/scripts/cts/testServer/index.ts index de7521d77cd..752d650a762 100644 --- a/scripts/cts/testServer/index.ts +++ b/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): Promise errorServer(), errorServerRetriedOnce(), errorServerRetriedTwice(), + neverCalledServer(), + successServer(), gzipServer(), timeoutServerBis(), accountCopyIndexServer(), diff --git a/scripts/cts/testServer/success.ts b/scripts/cts/testServer/success.ts new file mode 100644 index 00000000000..35500f2d9d3 --- /dev/null +++ b/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 = {}; + +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 { + return setupServer('success', 6675, addRoutes); +} diff --git a/tests/CTS/client/search/api.json b/tests/CTS/client/search/api.json index 0dc964fb23d..89d284a8425 100644 --- a/tests/CTS/client/search/api.json +++ b/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" + } + } + } + ] } ]