From c1dd2b479dfd1a7e64882ff744b3494f5f691232 Mon Sep 17 00:00:00 2001
From: petergardfjall <peter.gardfjall.work@gmail.com>
Date: Mon, 10 Dec 2012 13:56:21 +0100
Subject: [PATCH] fixed review comments

---
 .../compute/GleSYSComputeServiceAdapter.java  |  20 ++-
 .../GleSYSComputeServiceContextModule.java    |  20 ---
 .../options/GleSYSTemplateOptions.java        | 126 ++++++++++++++++--
 .../BaseGleSYSComputeServiceExpectTest.java   |  20 +--
 .../options/GleSYSTemplateOptionsTest.java    |  66 +++++++++
 5 files changed, 192 insertions(+), 60 deletions(-)

diff --git a/providers/glesys/src/main/java/org/jclouds/glesys/compute/GleSYSComputeServiceAdapter.java b/providers/glesys/src/main/java/org/jclouds/glesys/compute/GleSYSComputeServiceAdapter.java
index 8cc61dfa9b..96dd438545 100644
--- a/providers/glesys/src/main/java/org/jclouds/glesys/compute/GleSYSComputeServiceAdapter.java
+++ b/providers/glesys/src/main/java/org/jclouds/glesys/compute/GleSYSComputeServiceAdapter.java
@@ -24,6 +24,7 @@ import static org.jclouds.compute.util.ComputeServiceUtils.metadataAndTagsAsComm
 import static org.jclouds.concurrent.FutureIterables.transformParallel;
 
 import java.util.Map;
+import java.util.UUID;
 import java.util.Map.Entry;
 import java.util.Set;
 import java.util.concurrent.ExecutorService;
@@ -32,7 +33,6 @@ import java.util.concurrent.Future;
 import javax.annotation.Resource;
 import javax.inject.Inject;
 import javax.inject.Named;
-import javax.inject.Provider;
 import javax.inject.Singleton;
 
 import org.jclouds.Constants;
@@ -92,18 +92,16 @@ public class GleSYSComputeServiceAdapter implements ComputeServiceAdapter<Server
    private final ExecutorService userThreads;
    private final Timeouts timeouts;
    private final Supplier<Set<? extends Location>> locations;
-   private final Provider<String> passwordProvider;
 
    @Inject
    public GleSYSComputeServiceAdapter(GleSYSApi api, GleSYSAsyncApi aapi,
          @Named(Constants.PROPERTY_USER_THREADS) ExecutorService userThreads, Timeouts timeouts,
-         @Memoized Supplier<Set<? extends Location>> locations, @Named("PASSWORD") Provider<String> passwordProvider) {
+         @Memoized Supplier<Set<? extends Location>> locations) {
       this.api = checkNotNull(api, "api");
       this.aapi = checkNotNull(aapi, "aapi");
       this.userThreads = checkNotNull(userThreads, "userThreads");
       this.timeouts = checkNotNull(timeouts, "timeouts");
       this.locations = checkNotNull(locations, "locations");
-      this.passwordProvider = checkNotNull(passwordProvider, "passwordProvider");
    }
 
    @Override
@@ -133,11 +131,12 @@ public class GleSYSComputeServiceAdapter implements ComputeServiceAdapter<Server
       builder.memorySizeMB(template.getHardware().getRam());
       builder.diskSizeGB(Math.round(template.getHardware().getVolumes().get(0).getSize()));
       builder.cpuCores((int) template.getHardware().getProcessors().get(0).getCores());
-      builder.transferGB(50);// TODO: add to template options with default value
+      builder.transferGB(templateOptions.getTransferGB());
       ServerSpec spec = builder.build();
 
-      String password = passwordProvider.get(); // TODO: add to templateOptions
-                                                // and set if present
+      
+      // use random root password unless one was provided via template options
+      String password = templateOptions.hasRootPassword() ? templateOptions.getRootPassword() : getRandomPassword();
 
       logger.debug(">> creating new Server spec(%s) name(%s) options(%s)", spec, name, createServerOptions);
       ServerDetails result = api.getServerApi().createWithHostnameAndRootPassword(spec, name, password,
@@ -148,6 +147,13 @@ public class GleSYSComputeServiceAdapter implements ComputeServiceAdapter<Server
             .password(password).build());
    }
 
+   /**
+    * @return a generated random password string
+    */
+   private String getRandomPassword() {
+      return UUID.randomUUID().toString().replace("-","");
+   }
+
    @Singleton
    public static class FindLocationForServerSpec extends FindResourceInSet<ServerSpec, Location> {
 
diff --git a/providers/glesys/src/main/java/org/jclouds/glesys/compute/config/GleSYSComputeServiceContextModule.java b/providers/glesys/src/main/java/org/jclouds/glesys/compute/config/GleSYSComputeServiceContextModule.java
index e71bc1fc9b..4e44b024cb 100644
--- a/providers/glesys/src/main/java/org/jclouds/glesys/compute/config/GleSYSComputeServiceContextModule.java
+++ b/providers/glesys/src/main/java/org/jclouds/glesys/compute/config/GleSYSComputeServiceContextModule.java
@@ -18,12 +18,6 @@
  */
 package org.jclouds.glesys.compute.config;
 
-import java.util.UUID;
-
-import javax.inject.Named;
-import javax.inject.Provider;
-import javax.inject.Singleton;
-
 import org.jclouds.compute.ComputeServiceAdapter;
 import org.jclouds.compute.config.ComputeServiceAdapterContextModule;
 import org.jclouds.compute.domain.Hardware;
@@ -42,9 +36,7 @@ import org.jclouds.glesys.domain.OSTemplate;
 import org.jclouds.glesys.domain.ServerDetails;
 
 import com.google.common.base.Function;
-import com.google.inject.Scopes;
 import com.google.inject.TypeLiteral;
-import com.google.inject.name.Names;
 
 /**
  * 
@@ -70,19 +62,7 @@ public class GleSYSComputeServiceContextModule extends
       bind(new TypeLiteral<Function<String, OsFamilyVersion64Bit>>() {
       }).to(ParseOsFamilyVersion64BitFromImageName.class);
       bind(TemplateOptions.class).to(GleSYSTemplateOptions.class);
-      bind(String.class).annotatedWith(Names.named("PASSWORD")).toProvider(PasswordProvider.class).in(Scopes.SINGLETON);
-      // to have the compute service adapter override default locations
       install(new LocationsFromComputeServiceAdapterModule<ServerDetails, Hardware, OSTemplate, String>(){});
    }
 
-   @Named("PASSWORD")
-   @Singleton
-   public static class PasswordProvider implements Provider<String> {
-
-      @Override
-      public String get() {
-         return UUID.randomUUID().toString().replace("-","");
-      }
-      
-   }
 }
diff --git a/providers/glesys/src/main/java/org/jclouds/glesys/compute/options/GleSYSTemplateOptions.java b/providers/glesys/src/main/java/org/jclouds/glesys/compute/options/GleSYSTemplateOptions.java
index d377b5218c..5b1c1f295a 100644
--- a/providers/glesys/src/main/java/org/jclouds/glesys/compute/options/GleSYSTemplateOptions.java
+++ b/providers/glesys/src/main/java/org/jclouds/glesys/compute/options/GleSYSTemplateOptions.java
@@ -19,28 +19,32 @@
 package org.jclouds.glesys.compute.options;
 
 import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
 
 import java.util.Map;
 
 import org.jclouds.compute.options.TemplateOptions;
+import org.jclouds.glesys.domain.ServerSpec;
 
+import com.google.common.base.Objects.ToStringHelper;
 import com.google.common.net.InetAddresses;
 
 /**
  * Contains options supported by the
  * {@link ComputeService#createNodesInGroup(String, int, TemplateOptions)} and
- * {@link ComputeService#createNodesInGroup(String, int, TemplateOptions)} operations on the
- * <em>glesys</em> provider.
+ * {@link ComputeService#createNodesInGroup(String, int, TemplateOptions)}
+ * operations on the <em>glesys</em> provider.
  * 
- * <h2>Usage</h2> The recommended way to instantiate a {@link GleSYSTemplateOptions} object is to
- * statically import {@code GleSYSTemplateOptions.*} and invoke a static creation method followed by
- * an instance mutator (if needed):
+ * <h2>Usage</h2> The recommended way to instantiate a
+ * {@link GleSYSTemplateOptions} object is to statically import
+ * {@code GleSYSTemplateOptions.*} and invoke a static creation method followed
+ * by an instance mutator (if needed):
  * <p>
  * 
  * <pre>
  * import static org.jclouds.compute.options.GleSYSTemplateOptions.Builder.*;
  * ComputeService api = // get connection
- * templateBuilder.options(inboundPorts(22, 80, 8080, 443));
+ * templateBuilder.options(rootPassword("caQu5rou"));
  * Set&lt;? extends NodeMetadata&gt; set = api.createNodesInGroup(tag, 2, templateBuilder.build());
  * </pre>
  * 
@@ -48,7 +52,20 @@ import com.google.common.net.InetAddresses;
  */
 public class GleSYSTemplateOptions extends TemplateOptions implements Cloneable {
 
+   /**
+    * The IP address to assign to the new node instance. If set to "
+    * <code>any</code>" the node will be automatically assigned a free IP
+    * address.
+    */
    protected String ip = "any";
+   /**
+    * The password to set for the root user on the created server instance. If
+    * left unspecified, a random password will be assigned.
+    */
+   protected String rootPassword = null;
+
+   /** The monthly data transfer limit (in GB) for the server. */
+   protected int transferGB = 50;
 
    @Override
    public GleSYSTemplateOptions clone() {
@@ -61,39 +78,106 @@ public class GleSYSTemplateOptions extends TemplateOptions implements Cloneable
    public void copyTo(TemplateOptions to) {
       super.copyTo(to);
       if (to instanceof GleSYSTemplateOptions) {
-         GleSYSTemplateOptions eTo = GleSYSTemplateOptions.class.cast(to);
-         eTo.ip(ip);
+         GleSYSTemplateOptions copy = GleSYSTemplateOptions.class.cast(to);
+         copy.ip(ip);
+         copy.rootPassword(rootPassword);
+         copy.transferGB(transferGB);
       }
    }
 
    /**
+    * Sets the IP address to assign to the new server instance. If set to "
+    * <code>any</code>" the server will be automatically assigned a free IP
+    * address.
     * 
     * @see ServerApi#createWithHostnameAndRootPassword
     * @see InetAddresses#isInetAddress
     */
-   public TemplateOptions ip(String ip) {
-      if (ip != null)
-         checkArgument("any".equals(ip) || InetAddresses.isInetAddress(ip), "ip %s is not valid", ip);
+   public GleSYSTemplateOptions ip(String ip) {
+      checkNotNull(ip);
+      checkArgument("any".equals(ip) || InetAddresses.isInetAddress(ip), "ip %s is not valid", ip);      
       this.ip = ip;
       return this;
    }
 
+   /**
+    * @return the IP address to assign to the new server instance.
+    */
    public String getIp() {
       return ip;
    }
 
-   public static final GleSYSTemplateOptions NONE = new GleSYSTemplateOptions();
+   /**
+    * Sets the password for the root user on the created server instance. If
+    * left unspecified, a random password will be assigned.
+    * 
+    * @see ServerApi#createWithHostnameAndRootPassword
+    */
+   public GleSYSTemplateOptions rootPassword(String rootPassword) {
+      checkNotNull(rootPassword, "root password cannot be null");
+      this.rootPassword = rootPassword;
+      return this;
+   }
+
+   /**
+    * @return the password set for the root user or <code>null</code> if none is
+    *         set (and a random password will be assigned).
+    */
+   public String getRootPassword() {
+      return rootPassword;
+   }
+
+   /**
+    * @return <code>true</code> if a root password has been specified.
+    */
+   public boolean hasRootPassword() {
+      return rootPassword != null;
+   }
+
+   /**
+    * Sets the monthly data transfer limit (in GB) for the server.
+    * 
+    * @see ServerSpec#getTransferGB()
+    */
+   public GleSYSTemplateOptions transferGB(int transferGB) {
+      checkArgument(transferGB >= 0, "transferGB value must be >= 0", transferGB);
+      this.transferGB = transferGB;
+      return this;
+   }
+
+   /**
+    * @return the monthly data transfer limit (in GB) for the server.
+    */
+   public int getTransferGB() {
+      return transferGB;
+   }
 
    public static class Builder {
 
       /**
-       * @see #ip
+       * @see GleSYSTemplateOptions#ip
        */
       public static GleSYSTemplateOptions ip(String ip) {
          GleSYSTemplateOptions options = new GleSYSTemplateOptions();
          return GleSYSTemplateOptions.class.cast(options.ip(ip));
       }
 
+      /**
+       * @see GleSYSTemplateOptions#rootPassword
+       */
+      public static GleSYSTemplateOptions rootPassword(String rootPassword) {
+         GleSYSTemplateOptions options = new GleSYSTemplateOptions();
+         return GleSYSTemplateOptions.class.cast(options.rootPassword(rootPassword));
+      }
+
+      /**
+       * @see GleSYSTemplateOptions#transferGB
+       */
+      public static GleSYSTemplateOptions transferGB(int transferGB) {
+         GleSYSTemplateOptions options = new GleSYSTemplateOptions();
+         return GleSYSTemplateOptions.class.cast(options.transferGB(transferGB));
+      }
+
       // methods that only facilitate returning the correct object type
 
       /**
@@ -111,7 +195,7 @@ public class GleSYSTemplateOptions extends TemplateOptions implements Cloneable
          GleSYSTemplateOptions options = new GleSYSTemplateOptions();
          return GleSYSTemplateOptions.class.cast(options.blockOnPort(port, seconds));
       }
-      
+
       /**
        * @see TemplateOptions#userMetadata(Map)
        */
@@ -178,4 +262,18 @@ public class GleSYSTemplateOptions extends TemplateOptions implements Cloneable
    public GleSYSTemplateOptions userMetadata(String key, String value) {
       return GleSYSTemplateOptions.class.cast(super.userMetadata(key, value));
    }
+
+   @Override
+   public ToStringHelper string() {
+      ToStringHelper stringHelper = super.string();
+
+      stringHelper.add("transferGB", this.transferGB);
+      stringHelper.add("ip", this.ip);
+      if (this.hasRootPassword()) {
+         stringHelper.add("rootPasswordPresent", true);
+      }
+
+      return stringHelper;
+   }
+
 }
diff --git a/providers/glesys/src/test/java/org/jclouds/glesys/compute/internal/BaseGleSYSComputeServiceExpectTest.java b/providers/glesys/src/test/java/org/jclouds/glesys/compute/internal/BaseGleSYSComputeServiceExpectTest.java
index 102939532e..9fe26d7135 100644
--- a/providers/glesys/src/test/java/org/jclouds/glesys/compute/internal/BaseGleSYSComputeServiceExpectTest.java
+++ b/providers/glesys/src/test/java/org/jclouds/glesys/compute/internal/BaseGleSYSComputeServiceExpectTest.java
@@ -25,14 +25,12 @@ import org.jclouds.apis.ApiMetadata;
 import org.jclouds.compute.ComputeService;
 import org.jclouds.compute.ComputeServiceContext;
 import org.jclouds.glesys.GleSYSApiMetadata;
-import org.jclouds.glesys.compute.config.GleSYSComputeServiceContextModule.PasswordProvider;
 import org.jclouds.http.HttpRequest;
 import org.jclouds.http.HttpResponse;
 import org.jclouds.rest.internal.BaseRestApiExpectTest;
 
 import com.google.common.base.Function;
 import com.google.common.collect.ImmutableMap;
-import com.google.inject.AbstractModule;
 import com.google.inject.Injector;
 import com.google.inject.Module;
 
@@ -57,15 +55,6 @@ public abstract class BaseGleSYSComputeServiceExpectTest extends BaseRestApiExpe
       return createInjector(fn, module, props).getInstance(ComputeService.class);
    }
 
-   protected PasswordProvider passwordGenerator() {
-      // make sure we can predict passwords generated for createServer requests
-      return new PasswordProvider() {
-         public String get() {
-            return "foo";
-         }
-      };
-   }
-
    protected Injector injectorForKnownArgumentsAndConstantPassword() {
       return injectorForKnownArgumentsAndConstantPassword(ImmutableMap.<HttpRequest, HttpResponse> of());
    }
@@ -95,14 +84,7 @@ public abstract class BaseGleSYSComputeServiceExpectTest extends BaseRestApiExpe
                         .addHeader("Authorization", "Basic aWRlbnRpdHk6Y3JlZGVudGlhbA==").build(),
                         HttpResponse.builder().statusCode(204)
                               .payload(payloadFromResource("/server_allowed_arguments.json")).build())
-                  .putAll(requestsResponses).build(), new AbstractModule() {
-
-               @Override
-               protected void configure() {
-                  bind(PasswordProvider.class).toInstance(passwordGenerator());
-               }
-
-            }).getContext();
+                  .putAll(requestsResponses).build()).getContext();
    }
 
    protected ComputeServiceContext computeContextForKnownArgumentsAndConstantPassword() {
diff --git a/providers/glesys/src/test/java/org/jclouds/glesys/compute/options/GleSYSTemplateOptionsTest.java b/providers/glesys/src/test/java/org/jclouds/glesys/compute/options/GleSYSTemplateOptionsTest.java
index b74c90e249..894973eba5 100644
--- a/providers/glesys/src/test/java/org/jclouds/glesys/compute/options/GleSYSTemplateOptionsTest.java
+++ b/providers/glesys/src/test/java/org/jclouds/glesys/compute/options/GleSYSTemplateOptionsTest.java
@@ -19,6 +19,8 @@
 package org.jclouds.glesys.compute.options;
 
 import static org.jclouds.glesys.compute.options.GleSYSTemplateOptions.Builder.ip;
+import static org.jclouds.glesys.compute.options.GleSYSTemplateOptions.Builder.rootPassword;
+import static org.jclouds.glesys.compute.options.GleSYSTemplateOptions.Builder.transferGB;
 import static org.testng.Assert.assertEquals;
 
 import org.jclouds.compute.options.TemplateOptions;
@@ -57,8 +59,72 @@ public class GleSYSTemplateOptionsTest {
       assertEquals(options.as(GleSYSTemplateOptions.class).getIp(), "1.1.1.1");
    }
 
+   @Test(expectedExceptions = NullPointerException.class)
+   public void testNullIpThrowsNPE() {
+      new GleSYSTemplateOptions().ip(null);
+   }
+
+   @Test(expectedExceptions = IllegalArgumentException.class)
+   public void testInvalidIpThrowsIllegalArgument() {
+      new GleSYSTemplateOptions().ip("1.1.1");
+   }
+
    @Test(expectedExceptions = IllegalArgumentException.class)
    public void testipIsInvalidThrowsIllegalArgument() {
       new GleSYSTemplateOptions().ip("foo");
    }
+
+   @Test
+   public void testDefaultRootPassword() {
+      TemplateOptions options = new GleSYSTemplateOptions();
+      assertEquals(options.as(GleSYSTemplateOptions.class).getRootPassword(), null);
+   }
+
+   @Test
+   public void testRootPassword() {
+      TemplateOptions options = new GleSYSTemplateOptions().rootPassword("secret");
+      assertEquals(options.as(GleSYSTemplateOptions.class).getRootPassword(), "secret");
+   }   
+
+   @Test
+   public void testRootPasswordStatic() {
+      TemplateOptions options = rootPassword("secret");
+      assertEquals(options.as(GleSYSTemplateOptions.class).getRootPassword(), "secret");
+   }   
+
+   @Test(expectedExceptions = NullPointerException.class)
+   public void testNullRootPasswordThrowsNPE() {
+      new GleSYSTemplateOptions().rootPassword(null);
+   }   
+
+   @Test
+   public void testDefaultTranferGB() {
+      TemplateOptions options = new GleSYSTemplateOptions();
+      assertEquals(options.as(GleSYSTemplateOptions.class).getTransferGB(), 50);
+   }
+
+   @Test
+   public void testTransferGB() {
+      TemplateOptions options = new GleSYSTemplateOptions().transferGB(75);
+      assertEquals(options.as(GleSYSTemplateOptions.class).getTransferGB(), 75);
+   }   
+
+   @Test
+   public void testTransferGBStatic() {
+      TemplateOptions options = transferGB(75);
+      assertEquals(options.as(GleSYSTemplateOptions.class).getTransferGB(), 75);
+   }
+
+   @Test(expectedExceptions = IllegalArgumentException.class)
+   public void testNegativeTransferGBThrowsException() {
+      new GleSYSTemplateOptions().transferGB(-1);
+   }
+   
+   @Test
+   public void testClone() {
+      GleSYSTemplateOptions clone = transferGB(75).rootPassword("root").ip("1.1.1.1").clone();
+      assertEquals(clone.getTransferGB(), 75);
+      assertEquals(clone.getRootPassword(), "root");
+      assertEquals(clone.getIp(), "1.1.1.1");
+   }
 }