From ef259b94129a7722b974cc731f490e156d0ce88d Mon Sep 17 00:00:00 2001 From: Ross Ackland Date: Thu, 7 Sep 2023 14:42:25 +0100 Subject: [PATCH 1/3] Allowed string to be used for url by making constructor public, added Url validation and unit tests --- .../client/CurrencyCloudClient.java | 18 +++++++- .../CurrencyCloudClientSessionTest.java | 43 ++++++++++--------- 2 files changed, 38 insertions(+), 23 deletions(-) diff --git a/src/main/java/com/currencycloud/client/CurrencyCloudClient.java b/src/main/java/com/currencycloud/client/CurrencyCloudClient.java index 10b6eae4..aad6b327 100644 --- a/src/main/java/com/currencycloud/client/CurrencyCloudClient.java +++ b/src/main/java/com/currencycloud/client/CurrencyCloudClient.java @@ -20,6 +20,9 @@ import javax.annotation.Nullable; import java.math.BigDecimal; +import java.net.MalformedURLException; +import java.net.URI; +import java.net.URISyntaxException; import java.util.*; import java.util.regex.Pattern; @@ -119,7 +122,18 @@ public CurrencyCloudClient(Environment environment, String loginId, String apiKe this(environment.url, loginId, apiKey, HttpClientConfiguration.builder().build()); } - CurrencyCloudClient(String url, String loginId, String apiKey, HttpClientConfiguration httpClientConfiguration) { + public CurrencyCloudClient(String url, String loginId, String apiKey) { + this(url, loginId, apiKey, HttpClientConfiguration.builder().build()); + } + + public CurrencyCloudClient(String url, String loginId, String apiKey, HttpClientConfiguration httpClientConfiguration) { + String wellFormedUrl; + try { + wellFormedUrl = new URI(url).toURL().toString(); + } catch (URISyntaxException | MalformedURLException e) { + throw new IllegalArgumentException(e); + } + this.loginId = loginId; this.apiKey = apiKey; ClientConfig config = new ClientConfig(); @@ -144,7 +158,7 @@ public void configureObjectMapper(ObjectMapper objectMapper) { config.setHttpReadTimeout(httpClientConfiguration.getHttpReadTimeout()); api = RestProxyFactory.createProxy( - CurrencyCloud.class, url, config, + CurrencyCloud.class, wellFormedUrl, config, new AutoAuthenticator(this), new ExceptionTransformer(), new Reauthenticator(this) ); } diff --git a/src/test/java/com/currencycloud/client/CurrencyCloudClientSessionTest.java b/src/test/java/com/currencycloud/client/CurrencyCloudClientSessionTest.java index 9b395039..1268c18f 100644 --- a/src/test/java/com/currencycloud/client/CurrencyCloudClientSessionTest.java +++ b/src/test/java/com/currencycloud/client/CurrencyCloudClientSessionTest.java @@ -4,39 +4,40 @@ import org.junit.Test; import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertThrows; public class CurrencyCloudClientSessionTest { @Test - public void testRaisesAnErrorIfTheEnvironmentIsNotSet() throws Exception { - try { - CurrencyCloudClient client = new CurrencyCloudClient((CurrencyCloudClient.Environment) null, null, null); - client.authenticate(); - throw new AssertionError("Should have failed."); - } catch (NullPointerException ignored) { } + public void testRaisesAnErrorIfTheEnvironmentIsNotSet() { + assertThrows(NullPointerException.class, () -> new CurrencyCloudClient((CurrencyCloudClient.Environment) null, null, null)); } @Test - public void testRaisesAnErrorIfTheLoginIdIsNotSet() throws Exception { + public void testRaisesAnErrorIfTheLoginIdIsNotSet() { CurrencyCloudClient client = new CurrencyCloudClient(CurrencyCloudClient.Environment.demo, null, null); - try { - client.authenticate(); - throw new AssertionError("Should have failed."); - } catch (IllegalArgumentException e) { - assertThat(e.getMessage(), containsString("apiKey must be set")); - } + Exception exception = assertThrows(IllegalArgumentException.class, client::authenticate); + assertThat(exception.getMessage(), containsString("apiKey must be set")); } @Test - public void testRaisesAnErrorIfTheApiKeyIsNotSet() throws Exception { + public void testRaisesAnErrorIfTheApiKeyIsNotSet() { CurrencyCloudClient client = new CurrencyCloudClient(CurrencyCloudClient.Environment.demo, "development@currencycloud.com", null); - try { - client.authenticate(); - throw new AssertionError("Should have failed."); - } catch (IllegalArgumentException e) { - assertThat(e.getMessage(), containsString("apiKey")); - assertThat(e.getMessage(), containsString("must be set")); - } + Exception exception = assertThrows(IllegalArgumentException.class, client::authenticate); + assertThat(exception.getMessage(), containsString("apiKey must be set")); + } + + @Test + public void testRaisesAnErrorIfTheUrlIncorrect() { + Exception exception = assertThrows(IllegalArgumentException.class, () -> new CurrencyCloudClient("example.com", "development@currencycloud.com", "key")); + assertThat(exception.getMessage(), equalTo("URI is not absolute")); + } + + @Test + public void testRaisesAnErrorIfTheApiKeyIsNotSet2() { + Exception exception = assertThrows(IllegalArgumentException.class, () -> new CurrencyCloudClient("example com", "development@currencycloud.com", "key")); + assertThat(exception.getMessage(), containsString("Illegal character in path at index 7: example com")); } } From 60bc81aa0ebee4030e1264b10ec5c5fe26722ebf Mon Sep 17 00:00:00 2001 From: Ross Ackland Date: Thu, 7 Sep 2023 15:01:30 +0100 Subject: [PATCH 2/3] Added valid URL test --- .../client/CurrencyCloudClientSessionTest.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/test/java/com/currencycloud/client/CurrencyCloudClientSessionTest.java b/src/test/java/com/currencycloud/client/CurrencyCloudClientSessionTest.java index 1268c18f..dcfb46dc 100644 --- a/src/test/java/com/currencycloud/client/CurrencyCloudClientSessionTest.java +++ b/src/test/java/com/currencycloud/client/CurrencyCloudClientSessionTest.java @@ -30,14 +30,19 @@ public void testRaisesAnErrorIfTheApiKeyIsNotSet() { } @Test - public void testRaisesAnErrorIfTheUrlIncorrect() { + public void testRaisesAnErrorIfTheUrlNoAbsolute() { Exception exception = assertThrows(IllegalArgumentException.class, () -> new CurrencyCloudClient("example.com", "development@currencycloud.com", "key")); assertThat(exception.getMessage(), equalTo("URI is not absolute")); } @Test - public void testRaisesAnErrorIfTheApiKeyIsNotSet2() { + public void testRaisesAnErrorIfTheUrlIncorrect() { Exception exception = assertThrows(IllegalArgumentException.class, () -> new CurrencyCloudClient("example com", "development@currencycloud.com", "key")); assertThat(exception.getMessage(), containsString("Illegal character in path at index 7: example com")); } + + @Test + public void testValidUrlAndPortAccepted() { + new CurrencyCloudClient("http://localhost:8080", "development@currencycloud.com", "key"); + } } From a04979a9d42118acabf76f1950ac237025cec70b Mon Sep 17 00:00:00 2001 From: Ross Ackland Date: Fri, 8 Sep 2023 09:37:12 +0100 Subject: [PATCH 3/3] Made test intention clearer by specifying no exception in parameter --- .../currencycloud/client/CurrencyCloudClientSessionTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/com/currencycloud/client/CurrencyCloudClientSessionTest.java b/src/test/java/com/currencycloud/client/CurrencyCloudClientSessionTest.java index dcfb46dc..38b5f077 100644 --- a/src/test/java/com/currencycloud/client/CurrencyCloudClientSessionTest.java +++ b/src/test/java/com/currencycloud/client/CurrencyCloudClientSessionTest.java @@ -41,7 +41,7 @@ public void testRaisesAnErrorIfTheUrlIncorrect() { assertThat(exception.getMessage(), containsString("Illegal character in path at index 7: example com")); } - @Test + @Test(expected = Test.None.class) public void testValidUrlAndPortAccepted() { new CurrencyCloudClient("http://localhost:8080", "development@currencycloud.com", "key"); }