Cleanup per Adrian's comments & fixed broken unit test

apis/cloudstack/src/main/java/org/jclouds/cloudstack/handlers/CloudStackErrorHandler.java

@@ -18,11 +18,8 @@
  */
 package org.jclouds.cloudstack.handlers;
 
-import java.io.IOException;
+import com.google.common.base.Throwables;
-
+import com.google.common.io.Closeables;
-import javax.annotation.Resource;
-import javax.inject.Singleton;
-
 import org.jclouds.http.HttpCommand;
 import org.jclouds.http.HttpErrorHandler;
 import org.jclouds.http.HttpResponse;
@@ -32,8 +29,9 @@ import org.jclouds.rest.AuthorizationException;
 import org.jclouds.rest.ResourceNotFoundException;
 import org.jclouds.util.Strings2;
 
-import com.google.common.base.Throwables;
+import javax.annotation.Resource;
-import com.google.common.io.Closeables;
+import javax.inject.Singleton;
+import java.io.IOException;
 
 /**
  * 
@@ -57,6 +55,7 @@ public class CloudStackErrorHandler implements HttpErrorHandler {
          case 400:
             exception = new IllegalArgumentException(message, exception);
             break;
+         case 531:
          case 401:
             exception = new AuthorizationException(message, exception);
             break;

apis/cloudstack/src/main/java/org/jclouds/cloudstack/util/ApiKeyPairs.java

@@ -29,6 +29,8 @@ import org.jclouds.compute.ComputeServiceContext;
 import org.jclouds.compute.ComputeServiceContextFactory;
 import org.jclouds.rest.RestContextFactory;
 
+import java.net.URI;
+import java.util.NoSuchElementException;
 import java.util.Properties;
 import java.util.Set;
 
@@ -52,9 +54,11 @@ public class ApiKeyPairs {
     *          User password
     * @param domain
     *          Domain name. If empty defaults to ROOT
+    * @throws NoSuchElementException, AuthorizationException
     * @return
     */
-   public static ApiKeyPair getApiKeyPairForUser(String endpoint, String username, String password, String domain) {
+   public static ApiKeyPair loginToEndpointAsUsernameInDomainWithPasswordAndReturnApiKeyPair(
+         URI endpoint, String username, String password, String domain) {
       ComputeServiceContext context = null;
       try {
          context = new ComputeServiceContextFactory(setupRestProperties()).
@@ -72,7 +76,7 @@ public class ApiKeyPairs {
                }
             }
          }
-         return null;
+         throw new NoSuchElementException("Unable to find API keypair for user " + username);
 
       } finally {
          if (context != null)
@@ -84,7 +88,7 @@ public class ApiKeyPairs {
       return RestContextFactory.getPropertiesFromResource("/rest.properties");
    }
 
-   private static Properties setupProperties(String endpoint, String username, String password, String domain) {
+   private static Properties setupProperties(URI endpoint, String username, String password, String domain) {
       Properties overrides = new Properties();
 
       overrides.put(Constants.PROPERTY_TRUST_ALL_CERTS, "true");
@@ -92,7 +96,7 @@ public class ApiKeyPairs {
 
       overrides.put("jclouds.cloudstack.credential-type", "passwordCredentials");
 
-      overrides.put(PROVIDER + ".endpoint", checkNotNull(endpoint, "endpoint"));
+      overrides.put(PROVIDER + ".endpoint", checkNotNull(endpoint, "endpoint").toASCIIString());
       overrides.put(PROVIDER + ".identity",
          String.format("%s/%s", checkNotNull(domain, "domain"), checkNotNull(username, "username")));
       overrides.put(PROVIDER + ".credential", checkNotNull(password, "password"));

apis/cloudstack/src/test/java/org/jclouds/cloudstack/PasswordAuthenticationExpectTest.java

@@ -35,7 +35,7 @@ import com.google.common.net.HttpHeaders;
 
 /**
  * 
- * @see KeystoneProperties#CREDENTIAL_TYPE
+ * @see CloudStackProperties#CREDENTIAL_TYPE
  * @author Adrian Cole
  */
 @Test(groups = "unit", testName = "PasswordAuthenticationExpectTest")

apis/cloudstack/src/test/java/org/jclouds/cloudstack/features/BaseCloudStackRestClientExpectTest.java

@@ -18,12 +18,10 @@
  */
 package org.jclouds.cloudstack.features;
 
-import static org.jclouds.crypto.CryptoStreams.md5Hex;
+import com.google.common.base.Function;
-
+import com.google.common.collect.ImmutableMultimap;
-import java.net.URI;
+import com.google.common.collect.ImmutableSet;
-import java.net.URLEncoder;
+import com.google.inject.Module;
-import java.util.Properties;
-
 import org.jclouds.cloudstack.CloudStackContext;
 import org.jclouds.compute.ComputeServiceContextFactory;
 import org.jclouds.http.HttpRequest;
@@ -31,10 +29,11 @@ import org.jclouds.http.HttpResponse;
 import org.jclouds.logging.config.NullLoggingModule;
 import org.jclouds.rest.BaseRestClientExpectTest;
 
-import com.google.common.base.Function;
+import java.net.URI;
-import com.google.common.collect.ImmutableMultimap;
+import java.net.URLEncoder;
-import com.google.common.collect.ImmutableSet;
+import java.util.Properties;
-import com.google.inject.Module;
+
+import static org.jclouds.crypto.CryptoStreams.md5Hex;
 
 /**
  * Base class for writing CloudStack Rest Client Expect tests
@@ -60,7 +59,7 @@ public abstract class BaseCloudStackRestClientExpectTest<S> extends BaseRestClie
      .method("GET")
      .endpoint(
         URI.create("http://localhost:8080/client/api?response=json&command=login&" +
-           "username=identity&password=" + md5Hex("credential")+ "&domain=ROOT"))
+           "username=identity&password=" + md5Hex("credential")+ "&domain="))
      .headers(
         ImmutableMultimap.<String, String>builder()
            .put("Accept", "application/json")

apis/cloudstack/src/test/java/org/jclouds/cloudstack/features/SessionClientLiveTest.java

@@ -19,12 +19,16 @@
 package org.jclouds.cloudstack.features;
 
 import org.jclouds.cloudstack.domain.Account;
+import org.jclouds.cloudstack.domain.ApiKeyPair;
 import org.jclouds.cloudstack.domain.LoginResponse;
 import org.jclouds.cloudstack.domain.User;
 import org.jclouds.cloudstack.util.ApiKeyPairs;
 import org.jclouds.crypto.CryptoStreams;
+import org.jclouds.rest.AuthorizationException;
 import org.testng.annotations.Test;
 
+import java.net.URI;
+
 import static org.jclouds.cloudstack.features.GlobalAccountClientLiveTest.createTestAccount;
 import static org.jclouds.cloudstack.features.GlobalUserClientLiveTest.createTestUser;
 import static org.testng.Assert.assertEquals;
@@ -54,10 +58,11 @@ public class SessionClientLiveTest extends BaseCloudStackClientLiveTest {
 
          checkLoginAsTheNewUser(expectedUsername);
 
-         assertEquals(
+         ApiKeyPair expected = globalAdminClient.getUserClient().registerUserKeys(testUser.getId());
-            globalAdminClient.getUserClient().registerUserKeys(testUser.getId()),
+         ApiKeyPair actual = ApiKeyPairs.loginToEndpointAsUsernameInDomainWithPasswordAndReturnApiKeyPair(
-            ApiKeyPairs.getApiKeyPairForUser(endpoint, prefix + "-user", "password", "")
+            URI.create(endpoint), prefix + "-user", "password", "");
-         );
+
+         assertEquals(actual, expected);
 
       } finally {
          if (testUser != null)
@@ -67,6 +72,12 @@ public class SessionClientLiveTest extends BaseCloudStackClientLiveTest {
       }
    }
 
+   @Test(expectedExceptions = AuthorizationException.class)
+   public void testTryToGetApiKeypairWithWrongCredentials() {
+      ApiKeyPairs.loginToEndpointAsUsernameInDomainWithPasswordAndReturnApiKeyPair(
+         URI.create(endpoint), "dummy-missing-user", "with-a-wrong-password", "");
+   }
+
    private void checkLoginAsTheNewUser(String expectedUsername) {
       LoginResponse response = globalAdminClient.getSessionClient()
          .loginUserInDomainWithHashOfPassword(expectedUsername, "", CryptoStreams.md5Hex("password"));