Fix bug in UserCreateRequest (#40)

* Fix bug in UserCreateRequest

* general cleanup

Author: Crim

CHANGELOG.md

@@ -2,6 +2,9 @@
 The format is based on [Keep a Changelog](http://keepachangelog.com/)
 and this project adheres to [Semantic Versioning](http://semver.org/).
 
+## 1.1.1 (12/19/2019)
+- Fixed bug in User create endpoint where setting a new user's role could only be set via a RoleId.  Now you can set the role by name or id.
+
 ## 1.1.0 (12/17/2019)
 
 - Removed `org.apache.logging.log4j` dependency, instead relying on the org.slf4j logging interface/facade dependency explicitly.

pom.xml

@@ -6,7 +6,7 @@
 
     <groupId>com.darksci</groupId>
     <artifactId>pardot-api-client</artifactId>
-    <version>1.1.0</version>
+    <version>1.1.1</version>
     <packaging>jar</packaging>
 
     <!-- Require Maven 3.5.0 -->

src/main/java/com/darksci/pardot/api/ConnectionFailedException.java

@@ -0,0 +1,32 @@
+/**
+ * Copyright 2017, 2018, 2019 Stephen Powis https://github.com/Crim/pardot-java-client
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated
+ * documentation files (the "Software"), to deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit
+ * persons to whom the Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE
+ * WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
+ * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+ * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+package com.darksci.pardot.api;
+
+/**
+ * An error occurred while attempting to connect to remote server.
+ * Typically, the connection was refused remotely.
+ */
+public class ConnectionFailedException extends InvalidRequestException {
+    public ConnectionFailedException(final String message, final int errorCode) {
+        super(message, errorCode);
+    }
+
+    public ConnectionFailedException(final String message, final Throwable cause) {
+        super(message, cause);
+    }
+}

src/main/java/com/darksci/pardot/api/request/user/UserCreateRequest.java

@@ -45,11 +45,19 @@ public UserCreateRequest withUser(final NewUser user) {
             .withFirstName(user.getFirstName())
             .withLastName(user.getLastName())
             .withJobTitle(user.getJobTitle())
-            .withRoleId(user.getRoleId())
             .withPhone(user.getPhone())
             .withUrl(user.getUrl())
             .withPasswordExpireable(user.isPasswordExpirable());
 
+        // Determine if role is being passed as a name or id.
+        if (user.getRoleId() != null) {
+            // RoleId is not null, so let's use that.
+            withRole(user.getRoleId());
+        } else if (user.getRole() != null) {
+            // Role Name is not null, so lets use that.
+            withRole(user.getRole());
+        }
+
         // Optional Timezone
         if (user.getTimezone() != null) {
             withTimezone(user.getTimezone());
@@ -114,13 +122,44 @@ public UserCreateRequest withJobTitle(final String jobTitle) {
     }
 
     /**
-     * Define the RoleId field on the user.
+     * Define the Role field on the user by the role's id.
      *
      * @param roleId roleId for new user.
      * @return UserCreateRequest builder.
+     * @deprecated see withRole(Long)
      */
     public UserCreateRequest withRoleId(final Long roleId) {
-        return setParam("role", roleId);
+        return withRole(roleId);
+    }
+
+    /**
+     * Define the Role field on the user by name of the role.
+     *
+     * @param roleName name of the role for new user.
+     * @return UserCreateRequest builder.
+     */
+    public UserCreateRequest withRole(final String roleName) {
+        // If roleName was passed
+        if (roleName != null ) {
+            // Clear out role_id property.
+            setParam("role_id", null);
+        }
+        return setParam("role_name", roleName);
+    }
+
+    /**
+     * Define the Role field on the user by name of the role.
+     *
+     * @param roleId id of the role for new user.
+     * @return UserCreateRequest builder.
+     */
+    public UserCreateRequest withRole(final Long roleId) {
+        // If roleId was passed
+        if (roleId != null) {
+            // Clear out role_name property.
+            setParam("role_name", null);
+        }
+        return setParam("role_id", roleId);
     }
 
     /**

src/main/java/com/darksci/pardot/api/response/user/NewUser.java

@@ -43,32 +43,36 @@ public String getEmail() {
         return email;
     }
 
-    public void setEmail(final String email) {
+    public NewUser setEmail(final String email) {
         this.email = email;
+        return this;
     }
 
     public String getFirstName() {
         return firstName;
     }
 
-    public void setFirstName(final String firstName) {
+    public NewUser setFirstName(final String firstName) {
         this.firstName = firstName;
+        return this;
     }
 
     public String getLastName() {
         return lastName;
     }
 
-    public void setLastName(final String lastName) {
+    public NewUser setLastName(final String lastName) {
         this.lastName = lastName;
+        return this;
     }
 
     public String getJobTitle() {
         return jobTitle;
     }
 
-    public void setJobTitle(final String jobTitle) {
+    public NewUser setJobTitle(final String jobTitle) {
         this.jobTitle = jobTitle;
+        return this;
     }
 
     public Long getRoleId() {
@@ -79,13 +83,14 @@ public Long getRoleId() {
      * You can set the user's role by Id OR Name.
      * @param roleId Id of the role to set.
      */
-    public void setRoleId(final Long roleId) {
+    public NewUser setRoleId(final Long roleId) {
         this.roleId = roleId;
 
         // Null out role name.
         if (roleId != null) {
             this.role = null;
         }
+        return this;
     }
 
     public String getRole() {
@@ -96,53 +101,59 @@ public String getRole() {
      * You can set the user's role by Name or Id.
      * @param role Name of the role to set.
      */
-    public void setRole(final String role) {
+    public NewUser setRole(final String role) {
         this.role = role;
 
         // Null out roleId
         if (role != null) {
             roleId = null;
         }
+        return this;
     }
 
     public String getPhone() {
         return phone;
     }
 
-    public void setPhone(final String phone) {
+    public NewUser setPhone(final String phone) {
         this.phone = phone;
+        return this;
     }
 
     public String getUrl() {
         return url;
     }
 
-    public void setUrl(final String url) {
+    public NewUser setUrl(final String url) {
         this.url = url;
+        return this;
     }
 
     public DateTimeZone getTimezone() {
         return timezone;
     }
 
-    public void setTimezone(final DateTimeZone timezone) {
+    public NewUser setTimezone(final DateTimeZone timezone) {
         this.timezone = timezone;
+        return this;
     }
 
     public boolean isPasswordExpirable() {
         return isPasswordExpirable;
     }
 
-    public void setPasswordExpirable(final boolean passwordExpirable) {
+    public NewUser setPasswordExpirable(final boolean passwordExpirable) {
         isPasswordExpirable = passwordExpirable;
+        return this;
     }
 
     public String getCrmUsername() {
         return crmUsername;
     }
 
-    public void setCrmUsername(final String crmUsername) {
+    public NewUser setCrmUsername(final String crmUsername) {
         this.crmUsername = crmUsername;
+        return this;
     }
 
     @Override

src/main/java/com/darksci/pardot/api/response/user/UserAbilitiesResponse.java

@@ -69,7 +69,10 @@ public List<String> getCredentials() {
 
             @Override
             public String toString() {
-                return credentials.toString();
+                if (credentials != null) {
+                    return credentials.toString();
+                }
+                return "[]";
             }
         }
     }

src/main/java/com/darksci/pardot/api/rest/HttpClientRestClient.java

@@ -18,6 +18,7 @@
 package com.darksci.pardot.api.rest;
 
 import com.darksci.pardot.api.Configuration;
+import com.darksci.pardot.api.ConnectionFailedException;
 import com.darksci.pardot.api.request.Request;
 import com.darksci.pardot.api.rest.handlers.RestResponseHandler;
 import com.darksci.pardot.api.rest.interceptor.RequestInterceptor;
@@ -48,6 +49,7 @@
 import javax.net.ssl.TrustManager;
 import javax.net.ssl.TrustManagerFactory;
 import java.io.IOException;
+import java.net.ConnectException;
 import java.nio.charset.StandardCharsets;
 import java.security.KeyManagementException;
 import java.security.KeyStore;
@@ -245,6 +247,7 @@ private <T> T submitRequest(final Request request, final ResponseHandler<T> resp
      * @param responseHandler The response Handler to use to parse the response
      * @param <T> The type that ResponseHandler returns.
      * @return Parsed response.
+     * @throws ConnectionFailedException if remote server does not accept connection.
      */
     private <T> T submitRequest(final String url, final Map<String, String> postParams, final ResponseHandler<T> responseHandler) throws IOException {
         try {
@@ -261,7 +264,7 @@ private <T> T submitRequest(final String url, final Map<String, String> postPara
             }
 
             // Attach submitRequest params
-            for (Map.Entry<String, String> entry : postParams.entrySet()) {
+            for (final Map.Entry<String, String> entry : postParams.entrySet()) {
                 params.add(new BasicNameValuePair(entry.getKey(), entry.getValue()));
             }
             post.setEntity(new UrlEncodedFormEntity(params, StandardCharsets.UTF_8));
@@ -278,6 +281,12 @@ private <T> T submitRequest(final String url, final Map<String, String> postPara
             return httpClient.execute(post, responseHandler);
         } catch (final ClientProtocolException exception) {
             logger.error("Caught ClientProtocolException: {}", exception.getMessage(), exception);
+        } catch (final ConnectException connectException) {
+            // Signals that an error occurred while attempting to connect a
+            // socket to a remote address and port.  Typically, the connection
+            // was refused remotely (e.g., no process is listening on the
+            // remote address/port).
+            throw new ConnectionFailedException(connectException.getMessage(), connectException);
         } catch (final IOException exception) {
             // Typically this is a parse error.
             logger.error("Caught IOException: {}", exception.getMessage(), exception);

src/test/java/com/darksci/pardot/api/PardotClientTest.java

@@ -272,7 +272,7 @@ public void userCreateTest() {
             .withPhone("123-123-1234")
             .withUrl("http:/www.example.com")
             .withPasswordExpireable(false)
-            .withRoleId(4L)
+            .withRole(4L)
             .withTimezone(DateTimeZone.UTC);
 
         final User response = client.userCreate(createRequest);
@@ -295,7 +295,7 @@ public void userDeleteByEmailTest() {
             .withPhone("123-123-1234")
             .withUrl("http:/www.example.com")
             .withPasswordExpireable(false)
-            .withRoleId(4L)
+            .withRole(4L)
             .withTimezone(DateTimeZone.UTC);
 
         final User response = client.userCreate(createRequest);
@@ -322,7 +322,7 @@ public void userDeleteByIdTest() {
             .withPhone("123-123-1234")
             .withUrl("http:/www.example.com")
             .withPasswordExpireable(false)
-            .withRoleId(4L)
+            .withRole(4L)
             .withTimezone(DateTimeZone.UTC);
 
         final User response = client.userCreate(createRequest);
@@ -355,7 +355,7 @@ public void userUpdateRoleByIdTest() {
             .withPhone("123-123-1234")
             .withUrl("http:/www.example.com")
             .withPasswordExpireable(false)
-            .withRoleId(originalRoleId)
+            .withRole(originalRoleId)
             .withTimezone(DateTimeZone.UTC);
 
         final User response = client.userCreate(createRequest);
@@ -391,7 +391,7 @@ public void campaignQueryTest() {
      * Attempt to read campaign.
      */
     @Test
-    public void campaignReadTest() throws IOException {
+    public void campaignReadTest() {
         CampaignReadRequest request = new CampaignReadRequest()
             .selectById(14885L);
 
@@ -746,7 +746,7 @@ public void formQueryTest() {
      * Attempt to read campaign.
      */
     @Test
-    public void formReadTest() throws IOException {
+    public void formReadTest() {
         final FormReadRequest request = new FormReadRequest()
             .selectById(1L);
 
@@ -1111,8 +1111,6 @@ public void prospectDeleteTest() {
 
         // Issue request
         final boolean response = client.prospectDelete(request);
-
-        assertNotNull("Should not be null", response);
         logger.info("Response: {}", response);
     }
 
@@ -1247,8 +1245,8 @@ public void tagObjectReadQuery() {
 
         final TagObject response = client.tagObjectRead(request);
         assertNotNull("Should not be null", response);
-        response.getType();
-        response.getTypeName();
+        assertNotNull(response.getType());
+        assertNotNull(response.getTypeName());
         logger.info("Response: {}", response);
     }