Refactor Implementation and Design Smells

api/src/main/java/com/messagebird/MessageBirdServiceImpl.java

@@ -251,79 +251,77 @@ public <P, E> List<E> getJsonDataAsList(final String request, final P payload, f
         return getJsonDataAsList(request, payload, requestType, new HashMap<>(), elementClass);
     }
 
-    public <T, P> T getJsonData(final String request, final P payload, final String requestType, final Map<String, String> headers, final Class<T> clazz) throws UnauthorizedException, GeneralException, NotFoundException {
+    public <P> APIResponse executeRequest(final String request, final P payload, final String requestType, final Map<String, String> headers)
+            throws UnauthorizedException, GeneralException, NotFoundException {
         if (request == null) {
             throw new IllegalArgumentException(REQUEST_VALUE_MUST_BE_SPECIFIED);
         }
 
-        String url = request;
-        if (!isURLAbsolute(url)) {
-            url = serviceUrl + url;
-        }
-        final APIResponse apiResponse = doRequest(requestType, url, headers, payload);
+        String url = isURLAbsolute(request) ? request : serviceUrl + request;
+        return doRequest(requestType, url, headers, payload);
+    }
 
-        final String body = apiResponse.getBody();
-        final int status = apiResponse.getStatus();
+    private ObjectMapper configureObjectMapper() {
+        final ObjectMapper mapper = new ObjectMapper();
+        // If we as new properties, we don't want the system to fail, we rather want to ignore them
+        mapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
+        // Enable case insensitivity to avoid parsing errors if parameters' case in api response doesn't match sdk's
+        mapper.enable(MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES);
+        mapper.enable(MapperFeature.ACCEPT_CASE_INSENSITIVE_ENUMS);
+        return mapper;
+    }
 
-        if (status == HttpURLConnection.HTTP_OK || status == HttpURLConnection.HTTP_CREATED || status == HttpURLConnection.HTTP_ACCEPTED) {
-            try {
-                final ObjectMapper mapper = new ObjectMapper();
-                // If we as new properties, we don't want the system to fail, we rather want to ignore them
-                mapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
-                // Enable case insensitivity to avoid parsing errors if parameters' case in api response doesn't match sdk's
-                mapper.enable(MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES);
-                mapper.enable(MapperFeature.ACCEPT_CASE_INSENSITIVE_ENUMS);
-
-                // Prevents mismatched exception when clazz is null
-                return clazz == null
-                    ? null
-                    : this.readValue(mapper, body, clazz);
-            } catch (IOException ioe) {
-                throw new GeneralException(ioe);
-            }
-        } else if (status == HttpURLConnection.HTTP_NO_CONTENT) {
-            return null; // no content doesn't mean an error
+    private void handleResponseStatus(final int status, final String body) throws UnauthorizedException, GeneralException, NotFoundException {
+        if (status == HttpURLConnection.HTTP_OK ||
+                status == HttpURLConnection.HTTP_CREATED ||
+                status == HttpURLConnection.HTTP_ACCEPTED) {
+            return;
+        }
+
+        if (status == HttpURLConnection.HTTP_NO_CONTENT) {
+            return; // no content doesn't mean an error
         }
+
         handleHttpFailStatuses(status, body);
-        return null;
     }
 
-    // todo: need to refactor for duplicated code.
-    public <P, E> List<E> getJsonDataAsList(final String request,
-        final P payload, final String requestType, final Map<String, String> headers, final Class<E> elementClass)
-        throws UnauthorizedException, GeneralException, NotFoundException {
-        if (request == null) {
-            throw new IllegalArgumentException(REQUEST_VALUE_MUST_BE_SPECIFIED);
+    public <T, P> T getJsonData(final String request, final P payload, final String requestType, final Map<String, String> headers, final Class<T> clazz)
+            throws UnauthorizedException, GeneralException, NotFoundException {
+        final APIResponse apiResponse = executeRequest(request, payload, requestType, headers);
+        final int status = apiResponse.getStatus();
+        final String body = apiResponse.getBody();
+
+        handleResponseStatus(status, body);
+
+        // Prevents mismatched exception when clazz is null
+        if (clazz == null || status == HttpURLConnection.HTTP_NO_CONTENT) {
+            return null;
         }
 
-        String url = request;
-        if (!isURLAbsolute(url)) {
-            url = serviceUrl + url;
+        try {
+            return readValue(configureObjectMapper(), body, clazz);
+        } catch (IOException ioe) {
+            throw new GeneralException(ioe);
         }
-        final APIResponse apiResponse = doRequest(requestType, url, headers, payload);
+    }
 
-        final String body = apiResponse.getBody();
+    public <P, E> List<E> getJsonDataAsList(final String request, final P payload, final String requestType, final Map<String, String> headers, final Class<E> elementClass)
+            throws UnauthorizedException, GeneralException, NotFoundException {
+        final APIResponse apiResponse = executeRequest(request, payload, requestType, headers);
         final int status = apiResponse.getStatus();
+        final String body = apiResponse.getBody();
 
-        if (status == HttpURLConnection.HTTP_OK || status == HttpURLConnection.HTTP_CREATED || status == HttpURLConnection.HTTP_ACCEPTED) {
-            try {
-                final ObjectMapper mapper = new ObjectMapper();
-                // If we as new properties, we don't want the system to fail, we rather want to ignore them
-                mapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
-                // Enable case insensitivity to avoid parsing errors if parameters' case in api response doesn't match sdk's
-                mapper.enable(MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES);
-                mapper.enable(MapperFeature.ACCEPT_CASE_INSENSITIVE_ENUMS);
-
-                // Prevents mismatched exception when clazz is null
-                return this.readValueAsList(mapper, body, elementClass);
-            } catch (IOException ioe) {
-                throw new GeneralException(ioe);
-            }
-        } else if (status == HttpURLConnection.HTTP_NO_CONTENT) {
-            return Collections.emptyList(); // no content doesn't mean an error
+        handleResponseStatus(status, body);
+
+        if (status == HttpURLConnection.HTTP_NO_CONTENT) {
+            return Collections.emptyList();
+        }
+
+        try {
+            return readValueAsList(configureObjectMapper(), body, elementClass);
+        } catch (IOException ioe) {
+            throw new GeneralException(ioe);
         }
-        handleHttpFailStatuses(status, body);
-        return Collections.emptyList();
     }
 
     private <T> T readValue(ObjectMapper mapper, String content, Class<T> clazz)

api/src/main/java/com/messagebird/RequestValidator.java

@@ -193,12 +193,15 @@ private static String calculateSha256(byte[] bytes) {
     }
 
     private static String encodeHex(final byte[] data) {
-        final int l = data.length;
-        final char[] out = new char[l << 1];
-        for (int i = 0, j = 0; i < l; i++) {
-            out[j++] = HEX_DIGITS[(0xF0 & data[i]) >>> 4];
-            out[j++] = HEX_DIGITS[0x0F & data[i]];
+        final int length = data.length;
+        final char[] output = new char[length << 1];
+
+        for (int byteIndex = 0, charIndex = 0; byteIndex < length; byteIndex++) {
+            int highNibble = (data[byteIndex] & 0xF0) >>> 4;
+            int lowNibble = data[byteIndex] & 0x0F;
+            output[charIndex++] = HEX_DIGITS[highNibble];
+            output[charIndex++] = HEX_DIGITS[lowNibble];
         }
-        return new String(out);
+        return new String(output);
     }
 }

api/src/main/java/com/messagebird/objects/Hlr.java

@@ -14,9 +14,9 @@
 public class Hlr {
     private String id;
     private String href;
-    protected BigInteger msisdn;
+    private BigInteger msisdn;
     private String network;
-    protected String reference;
+    private String reference;
     private String status;
     private Date createdDatetime;
     private Date statusDatetime;
@@ -60,10 +60,14 @@ public String getHref() {
      * The telephone number.
      * @return
      */
-    public BigInteger getMsisdn() {
+    public BigInteger getPhoneNumber() {
         return msisdn;
     }
 
+    public void setPhoneNumber(BigInteger msisdn) {
+        this.msisdn = msisdn;
+    }
+
     /**
      * The MCCMNC code of the network provider
      * @return
@@ -80,6 +84,10 @@ public String getReference() {
         return reference;
     }
 
+    public void setReference(String reference) {
+        this.reference = reference;
+    }
+
     /**
      * The status of the msisdns. Possible values: sent, absent, active, unknown, and failed
      * @return
@@ -107,5 +115,9 @@ public Date getStatusDatetime() {
     public HlrDetails getDetails() {
         return details;
     }
+
+    public void setDetails(HlrDetails details) {
+        this.details = details;
+    }
 }
 

api/src/main/java/com/messagebird/objects/LookupHlr.java

@@ -13,15 +13,18 @@ public void setCountryCode(String countryCode) {
         this.countryCode = countryCode;
     }
 
+    @Override
     public BigInteger getPhoneNumber() {
-        return msisdn;
+        return super.getPhoneNumber();
     }
 
+    @Override
     public void setPhoneNumber(BigInteger phoneNumber) {
-        this.msisdn = phoneNumber;
+        super.setPhoneNumber(phoneNumber);
     }
 
+    @Override
     public void setReference(String reference) {
-        this.reference = reference;
+        super.setReference(reference);
     }
-}
+}

api/src/main/java/com/messagebird/objects/conversations/ConversationMessage.java

@@ -120,6 +120,9 @@ public String getPlatform() {
     }
 
     public void setPlatform(String platform) {
+        if (!ConversationPlatformConstants.isValidPlatform(platform)) {
+            throw new IllegalArgumentException("Invalid platform: " + platform);
+        }
         this.platform = platform;
     }
 

api/src/main/java/com/messagebird/objects/conversations/ConversationPlatformConstants.java

@@ -1,33 +1,18 @@
 package com.messagebird.objects.conversations;
 
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
 /**
  * Platforms are communication channels that a conversation can communicate through.
  */
 public class ConversationPlatformConstants {
-    // PlatformSMS identifies the MessageBird SMS platform.
-    public static final String SMS = "sms";
-
-    // PlatformWhatsApp identifies the WhatsApp platform.
-    public static final String WHATSAPP = "whatsapp";
-
-    // PlatformFacebook identifies the Facebook platform.
-    public static final String FACEBOOK = "facebook";
-
-    // PlatformTelegram identifies the Telegram platform.
-    public static final String TELEGRAM = "telegram";
-
-    // PlatformLine identifies the LINE platform.
-    public static final String LINE = "line";
-
-    // PlatformWeChat identifies the WeChat platform.
-    public static final String WECHAT = "wechat";
-
-    // PlatformEmail identifies the Email platform.
-    public static final String EMAIL = "email";
-
-    // PlatformEvents identifies the Events platform
-    public static final String EVENTS = "events";
+    private static final Set<String> VALID_PLATFORMS = new HashSet<>(Arrays.asList(
+            "sms", "whatsapp", "facebook", "telegram", "line", "wechat", "email", "events", "whatsapp_sandbox"
+    ));
 
-    // PlatformWhatsAppSandbox identified the WhatsApp sandbox platform.
-    public static final String WHATSAPP_SANDBOX = "whatsapp_sandbox";
+    public static boolean isValidPlatform(String platform) {
+        return VALID_PLATFORMS.contains(platform);
+    }
 }

api/src/main/java/com/messagebird/objects/integrations/HSMComponent.java

@@ -188,9 +188,19 @@ private void checkHeaderText() throws IllegalArgumentException {
    * @throws IllegalArgumentException Occurs when type is not {@code HEADER} and format is not {@code IMAGE}.
    */
   private void checkHeaderUrl() throws IllegalArgumentException {
-    if (!(HSMComponentType.HEADER.equals(type) &&
-            (HSMComponentFormat.IMAGE.equals(format) || HSMComponentFormat.VIDEO.equals(format) || HSMComponentFormat.DOCUMENT.equals(format)))) {
+    if (!isHeaderType() || !isValidFormat()) {
       throw new IllegalArgumentException("\"header_url\" is available for only HEADER type and IMAGE, VIDEO, or DOCUMENT formats.");
     }
   }
+
+  private boolean isHeaderType() {
+    return HSMComponentType.HEADER.equals(type);
+  }
+
+  private boolean isValidFormat() {
+    return HSMComponentFormat.IMAGE.equals(format) ||
+            HSMComponentFormat.VIDEO.equals(format) ||
+            HSMComponentFormat.DOCUMENT.equals(format);
+  }
+
 }