From 2b392d1c0b31f0ffd5caad833f9d8b326086b019 Mon Sep 17 00:00:00 2001 From: Kevin Hellemun Date: Wed, 11 Jul 2018 10:49:27 +0200 Subject: [PATCH] Some minor refactoring of the header code. (bunq/sdk_java#93) --- .../java/com/bunq/sdk/http/ApiClient.java | 6 +- .../com/bunq/sdk/http/BunqBasicHeader.java | 5 +- .../java/com/bunq/sdk/http/BunqHeader.java | 22 +-- .../com/bunq/sdk/http/BunqRequestBuilder.java | 2 +- .../com/bunq/sdk/security/SecurityUtils.java | 2 +- .../java/com/bunq/sdk/BunqSdkTestBase.java | 10 +- .../com/bunq/sdk/http/BunqHeaderTest.java | 136 +++++++++++------- 7 files changed, 109 insertions(+), 74 deletions(-) diff --git a/src/main/java/com/bunq/sdk/http/ApiClient.java b/src/main/java/com/bunq/sdk/http/ApiClient.java index bd88da51..7d00ba54 100644 --- a/src/main/java/com/bunq/sdk/http/ApiClient.java +++ b/src/main/java/com/bunq/sdk/http/ApiClient.java @@ -157,9 +157,9 @@ public BunqResponseRaw post( requestBodyBytes ); - if (customHeaders.containsKey(BunqHeader.CONTENT_TYPE.getHeader())) { + if (customHeaders.containsKey(BunqHeader.CONTENT_TYPE.getHeaderName())) { bunqRequestBody = BunqRequestBody.create( - MediaType.parse(customHeaders.get(BunqHeader.CONTENT_TYPE.getHeader())), + MediaType.parse(customHeaders.get(BunqHeader.CONTENT_TYPE.getHeaderName())), requestBodyBytes ); } @@ -279,7 +279,7 @@ private BunqResponseRaw createBunqResponseRaw(Response response) private static String getResponseId(Response response) { Map headerMap = getHeadersMap(response); - return BunqHeader.CLIENT_RESPONSE_ID.getOrDefault(headerMap); + return BunqHeader.CLIENT_RESPONSE_ID.getHeaderValueOrDefault(headerMap); } /** diff --git a/src/main/java/com/bunq/sdk/http/BunqBasicHeader.java b/src/main/java/com/bunq/sdk/http/BunqBasicHeader.java index d48dae32..1267fe43 100644 --- a/src/main/java/com/bunq/sdk/http/BunqBasicHeader.java +++ b/src/main/java/com/bunq/sdk/http/BunqBasicHeader.java @@ -1,6 +1,5 @@ package com.bunq.sdk.http; -import com.sun.org.apache.xpath.internal.functions.FuncBoolean; import okhttp3.Response; import java.util.ArrayList; @@ -19,7 +18,7 @@ public class BunqBasicHeader implements Comparable { private final String value; public static BunqBasicHeader get(BunqHeader header, Response response) { - return new BunqBasicHeader(header, response.header(header.getHeader())); + return new BunqBasicHeader(header, response.header(header.getHeaderName())); } public BunqBasicHeader(BunqHeader name, String value) { @@ -36,7 +35,7 @@ public String getValue() { } private String forSigning() { - return getName().getHeader() + DELIMITER_HEADER_NAME_AND_VALUE + getValue(); + return getName().getHeaderName() + DELIMITER_HEADER_NAME_AND_VALUE + getValue(); } public static String collectForSigning( diff --git a/src/main/java/com/bunq/sdk/http/BunqHeader.java b/src/main/java/com/bunq/sdk/http/BunqHeader.java index 728d194f..34833a80 100644 --- a/src/main/java/com/bunq/sdk/http/BunqHeader.java +++ b/src/main/java/com/bunq/sdk/http/BunqHeader.java @@ -34,7 +34,7 @@ public enum BunqHeader { this.defaultValue = defaultValue; } - public static BunqHeader parse(String value) { + public static BunqHeader parseHeaderOrNull(String value) { for (BunqHeader header:values()) { if (header.equals(value)) { return header; @@ -44,7 +44,7 @@ public static BunqHeader parse(String value) { return null; } - public String getHeader() { + public String getHeaderName() { return header; } @@ -52,7 +52,7 @@ public String getDefaultValue() { return defaultValue; } - private String getOrDefault(String value) { + private String getHeaderValueOrDefault(String value) { if (value != null) { return value; } @@ -61,7 +61,7 @@ private String getOrDefault(String value) { } public void addTo(Map headers, String value) { - headers.put(getHeader(), getOrDefault(value)); + headers.put(getHeaderName(), getHeaderValueOrDefault(value)); } public void addTo(BunqRequestBuilder requestBuilder) { @@ -69,18 +69,18 @@ public void addTo(BunqRequestBuilder requestBuilder) { } public void addTo(BunqRequestBuilder requestBuilder, String value) { - requestBuilder.addHeader(getHeader(), getOrDefault(value)); + requestBuilder.addHeader(getHeaderName(), getHeaderValueOrDefault(value)); } public boolean equals(String header) { - return getHeader().equalsIgnoreCase(header); + return getHeaderName().equalsIgnoreCase(header); } public boolean isBunq() { - return getHeader().startsWith(PREFIX); + return getHeaderName().startsWith(PREFIX); } - private String findKey(Collection keys) { + private String findKeyOrNull(Collection keys) { for (String key:keys) { if (this.equals(key)) { return key; @@ -90,11 +90,13 @@ private String findKey(Collection keys) { return null; } - public String getOrDefault(Map headers) { - String key = findKey(headers.keySet()); + public String getHeaderValueOrDefault(Map headers) { + String key = findKeyOrNull(headers.keySet()); + if (key != null && headers.get(key) != null) { return headers.get(key); } + return getDefaultValue(); } } diff --git a/src/main/java/com/bunq/sdk/http/BunqRequestBuilder.java b/src/main/java/com/bunq/sdk/http/BunqRequestBuilder.java index a52dd82c..5f245819 100644 --- a/src/main/java/com/bunq/sdk/http/BunqRequestBuilder.java +++ b/src/main/java/com/bunq/sdk/http/BunqRequestBuilder.java @@ -76,7 +76,7 @@ public BunqRequestBuilder url(URL url) { } private void addToAllHeader(String name, String value) { - BunqHeader header = BunqHeader.parse(name); + BunqHeader header = BunqHeader.parseHeaderOrNull(name); if (header != null) { this.allHeader.add(new BunqBasicHeader(header, value)); diff --git a/src/main/java/com/bunq/sdk/security/SecurityUtils.java b/src/main/java/com/bunq/sdk/security/SecurityUtils.java index 5e206e41..805ab8b3 100644 --- a/src/main/java/com/bunq/sdk/security/SecurityUtils.java +++ b/src/main/java/com/bunq/sdk/security/SecurityUtils.java @@ -489,7 +489,7 @@ private static byte[] getResponseBytes( List allResponseHeader = new ArrayList<>(); for (int i = INDEX_FIRST; i < allHeader.names().size(); i++) { - BunqHeader header = BunqHeader.parse(allHeader.name(i)); + BunqHeader header = BunqHeader.parseHeaderOrNull(allHeader.name(i)); if (header != null && !BunqHeader.SERVER_SIGNATURE.equals(header)) { allResponseHeader.add(new BunqBasicHeader(header, allHeader.get(allHeader.name(i)))); diff --git a/src/test/java/com/bunq/sdk/BunqSdkTestBase.java b/src/test/java/com/bunq/sdk/BunqSdkTestBase.java index afc76197..7f86c900 100644 --- a/src/test/java/com/bunq/sdk/BunqSdkTestBase.java +++ b/src/test/java/com/bunq/sdk/BunqSdkTestBase.java @@ -121,11 +121,11 @@ private static SandboxUser generateNewSandboxUser() { URL_PATH_SANDBOX_USER ) .post(RequestBody.create(null, new byte[INDEX_FIRST])) - .addHeader(BunqHeader.CLIENT_REQUEST_ID.getHeader(), UUID.randomUUID().toString()) - .addHeader(BunqHeader.CACHE_CONTROL.getHeader(), BunqHeader.CACHE_CONTROL.getDefaultValue()) - .addHeader(BunqHeader.GEOLOCATION.getHeader(), BunqHeader.GEOLOCATION.getDefaultValue()) - .addHeader(BunqHeader.LANGUAGE.getHeader(), BunqHeader.LANGUAGE.getDefaultValue()) - .addHeader(BunqHeader.REGION.getHeader(), BunqHeader.REGION.getDefaultValue()) + .addHeader(BunqHeader.CLIENT_REQUEST_ID.getHeaderName(), UUID.randomUUID().toString()) + .addHeader(BunqHeader.CACHE_CONTROL.getHeaderName(), BunqHeader.CACHE_CONTROL.getDefaultValue()) + .addHeader(BunqHeader.GEOLOCATION.getHeaderName(), BunqHeader.GEOLOCATION.getDefaultValue()) + .addHeader(BunqHeader.LANGUAGE.getHeaderName(), BunqHeader.LANGUAGE.getDefaultValue()) + .addHeader(BunqHeader.REGION.getHeaderName(), BunqHeader.REGION.getDefaultValue()) .build(); try { diff --git a/src/test/java/com/bunq/sdk/http/BunqHeaderTest.java b/src/test/java/com/bunq/sdk/http/BunqHeaderTest.java index 4ba67efd..7ebfe4af 100644 --- a/src/test/java/com/bunq/sdk/http/BunqHeaderTest.java +++ b/src/test/java/com/bunq/sdk/http/BunqHeaderTest.java @@ -1,69 +1,103 @@ package com.bunq.sdk.http; +import org.junit.Test; + import java.util.Collections; -import java.util.Map; import java.util.HashMap; - -import org.junit.Test; +import java.util.Map; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; public class BunqHeaderTest { - @Test - public void parse() { - // parse works case-insensitive - assertEquals(BunqHeader.CLIENT_RESPONSE_ID, BunqHeader.parse("X-Bunq-Client-Response-Id")); - assertEquals(BunqHeader.CLIENT_RESPONSE_ID, BunqHeader.parse("x-bunq-client-response-id")); - } - @Test - public void isBunq() { - assertFalse(BunqHeader.CACHE_CONTROL.isBunq()); - assertFalse(BunqHeader.CONTENT_TYPE.isBunq()); - assertFalse(BunqHeader.USER_AGENT.isBunq()); + /** + * Expected error messages constatns. + */ + private static final String ERROR_STRING_COULD_NOT_DETERMINE_RESPONSE_ID = "Could not determine response id."; + + /** + * Header name constants. + */ + private static final String HEADER_X_BUNQ_CLIENT_RESPONSE_ID = "X-Bunq-Client-Response-Id"; + private static final String HEADER_LOWER_X_BUNQ_CLIENT_RESPONSE_ID = "x-bunq-client-response-id"; + private static final String HEADER_X_BUNQ_SOME_OTHER_HEADER = "x-bunq-some-other-header"; + + /** + * Header value constatns. + */ + private static final String HEADER_VALUE_USER_AGENT = "test-agent"; + private static final String HEADER_VALUE_LANGUAGE = "en_US"; + private static final String HEADER_VALUE_GEOLOCATION = "0 0 0 0 000"; + private static final String RESPONSE_ID = "test-id"; + + @Test + public void testParseHeaderCaseInsensitive() { + assertEquals( + BunqHeader.CLIENT_RESPONSE_ID, + BunqHeader.parseHeaderOrNull(HEADER_X_BUNQ_CLIENT_RESPONSE_ID) + ); + assertEquals( + BunqHeader.CLIENT_RESPONSE_ID, + BunqHeader.parseHeaderOrNull(HEADER_LOWER_X_BUNQ_CLIENT_RESPONSE_ID) + ); + } + + @Test + public void testIsBunq() { + assertFalse(BunqHeader.CACHE_CONTROL.isBunq()); + assertFalse(BunqHeader.CONTENT_TYPE.isBunq()); + assertFalse(BunqHeader.USER_AGENT.isBunq()); - assertTrue(BunqHeader.ATTACHMENT_DESCRIPTION.isBunq()); - assertTrue(BunqHeader.CLIENT_AUTHENTICATION.isBunq()); - assertTrue(BunqHeader.CLIENT_ENCRYPTION_HMAC.isBunq()); - assertTrue(BunqHeader.CLIENT_ENCRYPTION_IV.isBunq()); - assertTrue(BunqHeader.CLIENT_ENCRYPTION_IV.isBunq()); - assertTrue(BunqHeader.CLIENT_ENCRYPTION_KEY.isBunq()); - assertTrue(BunqHeader.CLIENT_REQUEST_ID.isBunq()); - assertTrue(BunqHeader.CLIENT_SIGNATURE.isBunq()); - assertTrue(BunqHeader.CLIENT_RESPONSE_ID.isBunq()); - assertTrue(BunqHeader.GEOLOCATION.isBunq()); - assertTrue(BunqHeader.REGION.isBunq()); - assertTrue(BunqHeader.SERVER_SIGNATURE.isBunq()); - assertTrue(BunqHeader.CLIENT_ENCRYPTION_KEY.isBunq()); - assertTrue(BunqHeader.CLIENT_REQUEST_ID.isBunq()); - } + assertTrue(BunqHeader.ATTACHMENT_DESCRIPTION.isBunq()); + assertTrue(BunqHeader.CLIENT_AUTHENTICATION.isBunq()); + assertTrue(BunqHeader.CLIENT_ENCRYPTION_HMAC.isBunq()); + assertTrue(BunqHeader.CLIENT_ENCRYPTION_IV.isBunq()); + assertTrue(BunqHeader.CLIENT_ENCRYPTION_IV.isBunq()); + assertTrue(BunqHeader.CLIENT_ENCRYPTION_KEY.isBunq()); + assertTrue(BunqHeader.CLIENT_REQUEST_ID.isBunq()); + assertTrue(BunqHeader.CLIENT_SIGNATURE.isBunq()); + assertTrue(BunqHeader.CLIENT_RESPONSE_ID.isBunq()); + assertTrue(BunqHeader.GEOLOCATION.isBunq()); + assertTrue(BunqHeader.REGION.isBunq()); + assertTrue(BunqHeader.SERVER_SIGNATURE.isBunq()); + assertTrue(BunqHeader.CLIENT_ENCRYPTION_KEY.isBunq()); + assertTrue(BunqHeader.CLIENT_REQUEST_ID.isBunq()); + } - @Test - public void getOrDefault() { - BunqHeader h = BunqHeader.CLIENT_RESPONSE_ID; + @Test + public void getOrDefault() { + BunqHeader header = BunqHeader.CLIENT_RESPONSE_ID; - assertEquals("test-id", - h.getOrDefault(Collections.singletonMap("x-bunq-client-response-id", "test-id"))); - assertEquals("Could not determine response id.", - h.getOrDefault(Collections.singletonMap("x-bunq-some-other-header", "test-id"))); - assertEquals("Could not determine response id.", - h.getOrDefault(Collections.emptyMap())); - } + assertEquals( + RESPONSE_ID, + header.getHeaderValueOrDefault( + Collections.singletonMap(HEADER_LOWER_X_BUNQ_CLIENT_RESPONSE_ID, RESPONSE_ID) + ) + ); + assertEquals( + ERROR_STRING_COULD_NOT_DETERMINE_RESPONSE_ID, + header.getHeaderValueOrDefault( + Collections.singletonMap(HEADER_X_BUNQ_SOME_OTHER_HEADER, RESPONSE_ID) + ) + ); + assertEquals( + ERROR_STRING_COULD_NOT_DETERMINE_RESPONSE_ID, + header.getHeaderValueOrDefault(Collections.emptyMap()) + ); + } - @Test - public void addToMap() { - Map headers = new HashMap<>(); + @Test + public void addToMap() { + Map headers = new HashMap<>(); - //sut - BunqHeader.LANGUAGE.addTo(headers, null); - BunqHeader.GEOLOCATION.addTo(headers, null); - BunqHeader.USER_AGENT.addTo(headers, "test-agent"); + BunqHeader.LANGUAGE.addTo(headers, null); + BunqHeader.GEOLOCATION.addTo(headers, null); + BunqHeader.USER_AGENT.addTo(headers, HEADER_VALUE_USER_AGENT); - // verify - assertEquals("en_US", headers.get("X-Bunq-Language")); - assertEquals("0 0 0 0 000", headers.get("X-Bunq-Geolocation")); - assertEquals("test-agent", headers.get("User-Agent")); - } + assertEquals(HEADER_VALUE_LANGUAGE, headers.get(BunqHeader.LANGUAGE.getHeaderName())); + assertEquals(HEADER_VALUE_GEOLOCATION, headers.get(BunqHeader.GEOLOCATION.getHeaderName())); + assertEquals(HEADER_VALUE_USER_AGENT, headers.get(BunqHeader.USER_AGENT.getHeaderName())); + } }