From 37d42350e7b2608632f4711827d53efa7614b79c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Vannicatte?= Date: Wed, 10 Aug 2022 11:31:13 +0200 Subject: [PATCH] fix(clients): better missing region error thrown --- templates/java/api.mustache | 21 ++++--------------- .../algoliasearch/builds/initClients.mustache | 10 +++------ .../client/builds/checkParameters.mustache | 9 ++------ .../client/personalization/parameters.json | 19 ++++++++++++++++- tests/CTS/client/predict/parameters.json | 19 ++++++++++++++++- .../client/query-suggestions/parameters.json | 19 ++++++++++++++++- tests/CTS/client/sources/parameters.json | 4 ++-- 7 files changed, 65 insertions(+), 36 deletions(-) diff --git a/templates/java/api.mustache b/templates/java/api.mustache index 20bc90d51b..558d5e87cf 100644 --- a/templates/java/api.mustache +++ b/templates/java/api.mustache @@ -99,20 +99,7 @@ public class {{classname}} extends ApiClient { private static List getDefaultHosts(String region) throws AlgoliaRuntimeException { List hosts = new ArrayList(); - {{^fallbackToAliasHost}} - boolean found = false; - if (region == null) { - throw new AlgoliaRuntimeException("`region` is missing"); - } - for (String allowed : allowedRegions) { - if (allowed.equals(region)) { - found = true; - break; - } - } - {{/fallbackToAliasHost}} - {{#fallbackToAliasHost}} - boolean found = region == null; + boolean found = {{^fallbackToAliasHost}}false{{/fallbackToAliasHost}}{{#fallbackToAliasHost}}region == null{{/fallbackToAliasHost}}; if (region != null) { for (String allowed : allowedRegions) { if (allowed.equals(region)) { @@ -121,9 +108,9 @@ public class {{classname}} extends ApiClient { } } } - {{/fallbackToAliasHost}} - if (!found) { - throw new AlgoliaRuntimeException("`region` must be one of the following: {{#allowedRegions}}{{.}}{{^-last}}, {{/-last}}{{/allowedRegions}}"); + + if ({{^fallbackToAliasHost}}region == null || {{/fallbackToAliasHost}}!found){ + throw new AlgoliaRuntimeException("`region` {{^fallbackToAliasHost}}is required and {{/fallbackToAliasHost}}must be one of the following: {{#allowedRegions}}{{.}}{{^-last}}, {{/-last}}{{/allowedRegions}}"); } String url = {{#fallbackToAliasHost}}region == null ? "{{{hostWithFallback}}}" : {{/fallbackToAliasHost}} "{{{host}}}".replace("{region}", region); diff --git a/templates/javascript/clients/algoliasearch/builds/initClients.mustache b/templates/javascript/clients/algoliasearch/builds/initClients.mustache index a796a84cdb..1a3e604ba8 100644 --- a/templates/javascript/clients/algoliasearch/builds/initClients.mustache +++ b/templates/javascript/clients/algoliasearch/builds/initClients.mustache @@ -35,17 +35,13 @@ function initAbtesting(initOptions: InitClientOptions & InitClientRegion>): PersonalizationClient { - if (!initOptions.region) { - throw new Error('`region` is missing.'); - } - if ( - initOptions.region && + !initOptions.region || (initOptions.region && (typeof initOptions.region !== 'string' || - !personalizationRegions.includes(initOptions.region)) + !personalizationRegions.includes(initOptions.region))) ) { throw new Error( - `\`region\` must be one of the following: ${personalizationRegions.join(', ')}` + `\`region\` is required and must be one of the following: ${personalizationRegions.join(', ')}` ); } diff --git a/templates/javascript/clients/client/builds/checkParameters.mustache b/templates/javascript/clients/client/builds/checkParameters.mustache index e89f4f976f..db895b10fe 100644 --- a/templates/javascript/clients/client/builds/checkParameters.mustache +++ b/templates/javascript/clients/client/builds/checkParameters.mustache @@ -7,13 +7,8 @@ if (!apiKey || typeof apiKey !== 'string') { } {{#hasRegionalHost}} -{{^fallbackToAliasHost}} -if (!region) { - throw new Error("`region` is missing."); -} -{{/fallbackToAliasHost}} -if (region && (typeof region !== 'string' || !REGIONS.includes(region))) { - throw new Error(`\`region\` must be one of the following: ${REGIONS.join(', ')}`); +if ({{^fallbackToAliasHost}}!region || {{/fallbackToAliasHost}}(region && (typeof region !== 'string' || !REGIONS.includes(region)))) { + throw new Error(`\`region\` {{^fallbackToAliasHost}}is required and {{/fallbackToAliasHost}}must be one of the following: ${REGIONS.join(', ')}`); } {{/hasRegionalHost}} diff --git a/tests/CTS/client/personalization/parameters.json b/tests/CTS/client/personalization/parameters.json index 543c0a9968..655c28eb9d 100644 --- a/tests/CTS/client/personalization/parameters.json +++ b/tests/CTS/client/personalization/parameters.json @@ -1,4 +1,21 @@ [ + { + "testName": "throws when region is not given", + "autoCreateClient": false, + "steps": [ + { + "type": "createClient", + "parameters": { + "appId": "my-app-id", + "apiKey": "my-api-key", + "region": "" + }, + "expected": { + "error": "`region` is required and must be one of the following: eu, us" + } + } + ] + }, { "testName": "throws when incorrect region is given", "autoCreateClient": false, @@ -11,7 +28,7 @@ "region": "not_a_region" }, "expected": { - "error": "`region` must be one of the following: eu, us" + "error": "`region` is required and must be one of the following: eu, us" } } ] diff --git a/tests/CTS/client/predict/parameters.json b/tests/CTS/client/predict/parameters.json index 4d551cd3e5..7409c68c2a 100644 --- a/tests/CTS/client/predict/parameters.json +++ b/tests/CTS/client/predict/parameters.json @@ -1,4 +1,21 @@ [ + { + "testName": "throws when region is not given", + "autoCreateClient": false, + "steps": [ + { + "type": "createClient", + "parameters": { + "appId": "my-app-id", + "apiKey": "my-api-key", + "region": "" + }, + "expected": { + "error": "`region` is required and must be one of the following: ue, ew" + } + } + ] + }, { "testName": "throws when incorrect region is given", "autoCreateClient": false, @@ -11,7 +28,7 @@ "region": "not_a_region" }, "expected": { - "error": "`region` must be one of the following: ue, ew" + "error": "`region` is required and must be one of the following: ue, ew" } } ] diff --git a/tests/CTS/client/query-suggestions/parameters.json b/tests/CTS/client/query-suggestions/parameters.json index 543c0a9968..655c28eb9d 100644 --- a/tests/CTS/client/query-suggestions/parameters.json +++ b/tests/CTS/client/query-suggestions/parameters.json @@ -1,4 +1,21 @@ [ + { + "testName": "throws when region is not given", + "autoCreateClient": false, + "steps": [ + { + "type": "createClient", + "parameters": { + "appId": "my-app-id", + "apiKey": "my-api-key", + "region": "" + }, + "expected": { + "error": "`region` is required and must be one of the following: eu, us" + } + } + ] + }, { "testName": "throws when incorrect region is given", "autoCreateClient": false, @@ -11,7 +28,7 @@ "region": "not_a_region" }, "expected": { - "error": "`region` must be one of the following: eu, us" + "error": "`region` is required and must be one of the following: eu, us" } } ] diff --git a/tests/CTS/client/sources/parameters.json b/tests/CTS/client/sources/parameters.json index a6f215f1e4..1d535cd580 100644 --- a/tests/CTS/client/sources/parameters.json +++ b/tests/CTS/client/sources/parameters.json @@ -11,7 +11,7 @@ "region": "" }, "expected": { - "error": "`region` is missing." + "error": "`region` is required and must be one of the following: de, us" } } ] @@ -28,7 +28,7 @@ "region": "not_a_region" }, "expected": { - "error": "`region` must be one of the following: de, us" + "error": "`region` is required and must be one of the following: de, us" } } ]