From 08a3cc0275254469639eca4547d24e95b55b20dd Mon Sep 17 00:00:00 2001
From: Adrian Cole <adrian@jclouds.org>
Date: Sun, 24 Apr 2011 16:00:37 -0700
Subject: [PATCH] Issue 540:fixed NPE when overriding aws-s3.endpoint with a
 regional one

---
 .../jclouds/s3/config/S3RestClientModule.java | 10 +++
 .../s3/functions/BindRegionToXmlPayload.java  | 34 +++++----
 .../s3/xml/LocationConstraintHandler.java     | 10 ++-
 ...stnameAndBindAsHostPrefixIfConfigured.java | 73 +++++++++++++++++++
 .../aws/s3/config/AWSS3RestClientModule.java  |  8 +-
 .../jclouds/aws/s3/AWSS3AsyncClientTest.java  | 63 +++++++++++++---
 6 files changed, 168 insertions(+), 30 deletions(-)
 create mode 100644 providers/aws-s3/src/main/java/org/jclouds/aws/s3/binders/AssignCorrectHostnameAndBindAsHostPrefixIfConfigured.java

diff --git a/apis/s3/src/main/java/org/jclouds/s3/config/S3RestClientModule.java b/apis/s3/src/main/java/org/jclouds/s3/config/S3RestClientModule.java
index 4efaf99ac4..9517e6c12e 100644
--- a/apis/s3/src/main/java/org/jclouds/s3/config/S3RestClientModule.java
+++ b/apis/s3/src/main/java/org/jclouds/s3/config/S3RestClientModule.java
@@ -21,6 +21,7 @@ package org.jclouds.s3.config;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
 
+import javax.annotation.Nullable;
 import javax.inject.Named;
 import javax.inject.Singleton;
 
@@ -33,6 +34,7 @@ import org.jclouds.http.RequiresHttp;
 import org.jclouds.http.annotation.ClientError;
 import org.jclouds.http.annotation.Redirection;
 import org.jclouds.http.annotation.ServerError;
+import org.jclouds.location.Region;
 import org.jclouds.rest.ConfiguresRestClient;
 import org.jclouds.rest.RequestSigner;
 import org.jclouds.s3.Bucket;
@@ -70,6 +72,14 @@ public class S3RestClientModule<S extends S3Client, A extends S3AsyncClient> ext
       return Maps.newConcurrentMap();
    }
 
+   @Provides
+   @Bucket
+   @Singleton
+   @Nullable
+   protected String defaultRegionForBucket(@Nullable @Region String defaultRegion) {
+      return defaultRegion;
+   }
+
    @Override
    protected void configure() {
       install(new S3ObjectModule());
diff --git a/apis/s3/src/main/java/org/jclouds/s3/functions/BindRegionToXmlPayload.java b/apis/s3/src/main/java/org/jclouds/s3/functions/BindRegionToXmlPayload.java
index 4668ec06f9..c634bf5388 100644
--- a/apis/s3/src/main/java/org/jclouds/s3/functions/BindRegionToXmlPayload.java
+++ b/apis/s3/src/main/java/org/jclouds/s3/functions/BindRegionToXmlPayload.java
@@ -19,9 +19,8 @@
 package org.jclouds.s3.functions;
 
 import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
 
-import java.net.URI;
-import java.util.Map;
 import java.util.Set;
 
 import javax.annotation.Nullable;
@@ -33,11 +32,12 @@ import javax.ws.rs.core.MediaType;
 import org.jclouds.http.HttpRequest;
 import org.jclouds.logging.Logger;
 import org.jclouds.rest.binders.BindToStringPayload;
+import org.jclouds.s3.Bucket;
 
 /**
  * 
- * Depending on your latency and legal requirements, you can specify a location constraint that will
- * affect where your data physically resides.
+ * Depending on your latency and legal requirements, you can specify a location
+ * constraint that will affect where your data physically resides.
  * 
  * @author Adrian Cole
  * 
@@ -47,25 +47,28 @@ public class BindRegionToXmlPayload extends BindToStringPayload {
    @Resource
    protected Logger logger = Logger.NULL;
 
-   private final String defaultRegion;
+   private final String defaultRegionForEndpoint;
+   private final String defaultRegionForService;
    private final Set<String> regions;
 
    @Inject
-   BindRegionToXmlPayload(@org.jclouds.location.Region @Nullable String defaultRegion,
-            @org.jclouds.location.Region Set<String> regions) {
-      this.defaultRegion = defaultRegion;
-      this.regions = regions;
+   public BindRegionToXmlPayload(@org.jclouds.location.Region @Nullable String defaultRegionForEndpoint,
+         @Nullable @Bucket String defaultRegionForService, @org.jclouds.location.Region Set<String> regions) {
+      this.defaultRegionForEndpoint = defaultRegionForEndpoint;
+      this.defaultRegionForService = defaultRegionForService;
+      this.regions = checkNotNull(regions, "regions");
    }
 
    @Override
    public <R extends HttpRequest> R bindToRequest(R request, Object input) {
-      if (defaultRegion == null)
+      if (defaultRegionForEndpoint == null)
          return request;
-      input = input == null ? defaultRegion : input;
+      input = input == null ? defaultRegionForEndpoint : input;
       checkArgument(input instanceof String, "this binder is only valid for Region!");
       String constraint = (String) input;
       String value = null;
-      if (defaultRegion.equals(constraint)) {
+      if ((defaultRegionForService == null && constraint == null)
+            || (defaultRegionForService != null && defaultRegionForService.equals(constraint))) {
          // nothing to bind as this is default.
          return request;
       } else if (regions.contains(constraint)) {
@@ -74,9 +77,10 @@ public class BindRegionToXmlPayload extends BindToStringPayload {
          logger.warn("region %s not in %s ", constraint, regions);
          value = constraint;
       }
-      String payload = String.format(
-               "<CreateBucketConfiguration><LocationConstraint>%s</LocationConstraint></CreateBucketConfiguration>",
-               value);
+      String payload = String
+            .format(
+                  "<CreateBucketConfiguration><LocationConstraint>%s</LocationConstraint></CreateBucketConfiguration>",
+                  value);
       request = super.bindToRequest(request, payload);
       request.getPayload().getContentMetadata().setContentType(MediaType.TEXT_XML);
       return request;
diff --git a/apis/s3/src/main/java/org/jclouds/s3/xml/LocationConstraintHandler.java b/apis/s3/src/main/java/org/jclouds/s3/xml/LocationConstraintHandler.java
index 6d9f1c25f6..e3ffabc7ef 100644
--- a/apis/s3/src/main/java/org/jclouds/s3/xml/LocationConstraintHandler.java
+++ b/apis/s3/src/main/java/org/jclouds/s3/xml/LocationConstraintHandler.java
@@ -20,7 +20,7 @@ package org.jclouds.s3.xml;
 
 import static org.jclouds.util.SaxUtils.currentOrNull;
 
-import java.util.concurrent.ConcurrentMap;
+import java.util.Map;
 
 import javax.inject.Inject;
 
@@ -35,17 +35,19 @@ import org.jclouds.s3.Bucket;
  * <p/>
  * Region is the document we expect to parse.
  * 
- * @see <a href= "http://docs.amazonwebservices.com/AmazonS3/latest/RESTBucketLocationGET.html" />
+ * @see <a href=
+ *      "http://docs.amazonwebservices.com/AmazonS3/latest/RESTBucketLocationGET.html"
+ *      />
  * @author Adrian Cole
  */
 public class LocationConstraintHandler extends ParseSax.HandlerWithResult<String> {
-   private final ConcurrentMap<String, String> bucketToRegion;
+   private final Map<String, String> bucketToRegion;
    private StringBuilder currentText = new StringBuilder();
    private String region;
    private String bucket;
 
    @Inject
-   public LocationConstraintHandler(@Bucket ConcurrentMap<String, String> bucketToRegion) {
+   public LocationConstraintHandler(@Bucket Map<String, String> bucketToRegion) {
       this.bucketToRegion = bucketToRegion;
    }
 
diff --git a/providers/aws-s3/src/main/java/org/jclouds/aws/s3/binders/AssignCorrectHostnameAndBindAsHostPrefixIfConfigured.java b/providers/aws-s3/src/main/java/org/jclouds/aws/s3/binders/AssignCorrectHostnameAndBindAsHostPrefixIfConfigured.java
new file mode 100644
index 0000000000..20c5abd042
--- /dev/null
+++ b/providers/aws-s3/src/main/java/org/jclouds/aws/s3/binders/AssignCorrectHostnameAndBindAsHostPrefixIfConfigured.java
@@ -0,0 +1,73 @@
+/**
+ *
+ * Copyright (C) 2011 Cloud Conscious, LLC. <info@cloudconscious.com>
+ *
+ * ====================================================================
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * ====================================================================
+ */
+
+package org.jclouds.aws.s3.binders;
+
+import static org.jclouds.s3.reference.S3Constants.PROPERTY_S3_SERVICE_PATH;
+import static org.jclouds.s3.reference.S3Constants.PROPERTY_S3_VIRTUAL_HOST_BUCKETS;
+
+import java.net.URI;
+import java.util.Map;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Provider;
+import javax.inject.Singleton;
+import javax.ws.rs.core.UriBuilder;
+
+import org.jclouds.http.HttpRequest;
+import org.jclouds.http.utils.ModifyRequest;
+import org.jclouds.location.functions.RegionToEndpointOrProviderIfNull;
+import org.jclouds.rest.binders.BindAsHostPrefix;
+import org.jclouds.s3.Bucket;
+import org.jclouds.s3.binders.BindAsHostPrefixIfConfigured;
+
+/**
+ * 
+ * @author Adrian Cole
+ */
+@Singleton
+public class AssignCorrectHostnameAndBindAsHostPrefixIfConfigured extends BindAsHostPrefixIfConfigured {
+   private final Map<String, String> bucketToRegion;
+   private final RegionToEndpointOrProviderIfNull r2;
+
+   @Inject
+   public AssignCorrectHostnameAndBindAsHostPrefixIfConfigured(BindAsHostPrefix bindAsHostPrefix,
+         @Named(PROPERTY_S3_VIRTUAL_HOST_BUCKETS) boolean isVhostStyle,
+         @Named(PROPERTY_S3_SERVICE_PATH) String servicePath, RegionToEndpointOrProviderIfNull r2,
+         Provider<UriBuilder> uriBuilderProvider, @Bucket Map<String, String> bucketToRegion) {
+      super(bindAsHostPrefix, isVhostStyle, servicePath, uriBuilderProvider);
+      this.bucketToRegion = bucketToRegion;
+      this.r2 = r2;
+   }
+
+   @Override
+   public <R extends HttpRequest> R bindToRequest(R request, Object payload) {
+      String bucket = payload.toString();
+      String region = bucketToRegion.get(bucket);
+      if (region != null) {
+         URI endpoint = r2.apply(region);
+         request = ModifyRequest.endpoint(
+               request,
+               uriBuilderProvider.get().uri(endpoint).path(request.getEndpoint().getPath())
+                     .replaceQuery(request.getEndpoint().getQuery()).build());
+      }
+      return super.bindToRequest(request, payload);
+   }
+}
diff --git a/providers/aws-s3/src/main/java/org/jclouds/aws/s3/config/AWSS3RestClientModule.java b/providers/aws-s3/src/main/java/org/jclouds/aws/s3/config/AWSS3RestClientModule.java
index b2b8465fed..daccd2d7ed 100644
--- a/providers/aws-s3/src/main/java/org/jclouds/aws/s3/config/AWSS3RestClientModule.java
+++ b/providers/aws-s3/src/main/java/org/jclouds/aws/s3/config/AWSS3RestClientModule.java
@@ -31,6 +31,7 @@ import org.jclouds.aws.s3.AWSS3AsyncClient;
 import org.jclouds.aws.s3.AWSS3Client;
 import org.jclouds.aws.s3.binders.AssignCorrectHostnameAndBindAsHostPrefixIfConfigured;
 import org.jclouds.http.RequiresHttp;
+import org.jclouds.location.Region;
 import org.jclouds.rest.ConfiguresRestClient;
 import org.jclouds.s3.Bucket;
 import org.jclouds.s3.S3AsyncClient;
@@ -52,10 +53,15 @@ public class AWSS3RestClientModule extends S3RestClientModule<AWSS3Client, AWSS3
    @Provides
    @Singleton
    @Bucket
-   protected URI provideLocationURI(@Named(PROPERTY_REGION + "." + US_STANDARD + "." + ENDPOINT) String endpoint) {
+   protected URI provideBucketURI(@Named(PROPERTY_REGION + "." + US_STANDARD + "." + ENDPOINT) String endpoint) {
       return URI.create(endpoint);
    }
 
+   @Override
+   protected String defaultRegionForBucket(@Region String defaultRegion) {
+      return US_STANDARD;
+   }
+
    @Override
    protected void configure() {
       bind(BindAsHostPrefixIfConfigured.class).to(AssignCorrectHostnameAndBindAsHostPrefixIfConfigured.class);
diff --git a/providers/aws-s3/src/test/java/org/jclouds/aws/s3/AWSS3AsyncClientTest.java b/providers/aws-s3/src/test/java/org/jclouds/aws/s3/AWSS3AsyncClientTest.java
index 0ee634a309..137777a4f2 100644
--- a/providers/aws-s3/src/test/java/org/jclouds/aws/s3/AWSS3AsyncClientTest.java
+++ b/providers/aws-s3/src/test/java/org/jclouds/aws/s3/AWSS3AsyncClientTest.java
@@ -43,7 +43,6 @@ import org.jclouds.rest.RestContextFactory;
 import org.jclouds.rest.functions.MapHttp4xxCodesToExceptions;
 import org.jclouds.rest.functions.ReturnVoidOnNotFoundOr404;
 import org.jclouds.rest.internal.RestAnnotationProcessor;
-import org.jclouds.s3.S3AsyncClient;
 import org.jclouds.s3.domain.ObjectMetadata;
 import org.jclouds.s3.domain.ObjectMetadataBuilder;
 import org.jclouds.s3.functions.ReturnFalseIfBucketAlreadyOwnedByYouOrIllegalState;
@@ -61,7 +60,8 @@ import com.google.inject.TypeLiteral;
 /**
  * @author Adrian Cole
  */
-// NOTE:without testName, this will not call @Before* and fail w/NPE during surefire
+// NOTE:without testName, this will not call @Before* and fail w/NPE during
+// surefire
 @Test(groups = "unit", testName = "AWSS3AsyncClientTest")
 public class AWSS3AsyncClientTest extends org.jclouds.s3.S3AsyncClientTest<AWSS3AsyncClient> {
 
@@ -70,11 +70,11 @@ public class AWSS3AsyncClientTest extends org.jclouds.s3.S3AsyncClientTest<AWSS3
    }
 
    public void testGetBucketLocationEU() throws SecurityException, NoSuchMethodException, IOException {
-      Method method = S3AsyncClient.class.getMethod("getBucketLocation", String.class);
+      Method method = AWSS3AsyncClient.class.getMethod("getBucketLocation", String.class);
       HttpRequest request = processor.createRequest(method, "eubucket");
 
-      assertRequestLineEquals(request, "GET https://eubucket.bucket/?location HTTP/1.1");
-      assertNonPayloadHeadersEqual(request, "Host: bucket.s3.amazonaws.com\n");
+      assertRequestLineEquals(request, "GET https://eubucket.s3-eu-west-1.amazonaws.com/?location HTTP/1.1");
+      assertNonPayloadHeadersEqual(request, "Host: eubucket.s3-eu-west-1.amazonaws.com\n");
       assertPayloadEquals(request, null, null, false);
 
       assertResponseParserClassEquals(method, request, ParseSax.class);
@@ -84,6 +84,48 @@ public class AWSS3AsyncClientTest extends org.jclouds.s3.S3AsyncClientTest<AWSS3
       checkFilters(request);
    }
 
+   @Override
+   public void testGetBucketLocation() throws SecurityException, NoSuchMethodException, IOException {
+      Method method = AWSS3AsyncClient.class.getMethod("getBucketLocation", String.class);
+      HttpRequest request = processor.createRequest(method, "bucket");
+
+      assertRequestLineEquals(request, "GET https://bucket.bucketendpoint/?location HTTP/1.1");
+      assertNonPayloadHeadersEqual(request, "Host: bucket.bucketendpoint\n");
+      assertPayloadEquals(request, null, null, false);
+
+      request = filter.filter(request);
+
+      assertRequestLineEquals(request, "GET https://bucket.bucketendpoint/?location HTTP/1.1");
+      assertNonPayloadHeadersEqual(
+            request,
+            "Authorization: AWS identity:2fFTeYJTDwiJmaAkKj732RjNbOg=\nDate: 2009-11-08T15:54:08.897Z\nHost: bucket.bucketendpoint\n");
+      assertPayloadEquals(request, null, null, false);
+
+      assertResponseParserClassEquals(method, request, ParseSax.class);
+      assertSaxResponseParserClassEquals(method, LocationConstraintHandler.class);
+      assertExceptionParserClassEquals(method, null);
+
+      checkFilters(request);
+   }
+
+   @Override
+   public void testPutBucketDefault() throws ArrayIndexOutOfBoundsException, SecurityException,
+         IllegalArgumentException, NoSuchMethodException, IOException {
+      Method method = AWSS3AsyncClient.class.getMethod("putBucketInRegion", String.class, String.class, Array
+            .newInstance(PutBucketOptions.class, 0).getClass());
+      HttpRequest request = processor.createRequest(method, (String) null, "bucket");
+
+      assertRequestLineEquals(request, "PUT https://bucket.bucketendpoint/ HTTP/1.1");
+      assertNonPayloadHeadersEqual(request, "Host: bucket.bucketendpoint\n");
+      assertPayloadEquals(request, null, null, false);
+
+      assertResponseParserClassEquals(method, request, ReturnTrueIf2xx.class);
+      assertSaxResponseParserClassEquals(method, null);
+      assertExceptionParserClassEquals(method, ReturnFalseIfBucketAlreadyOwnedByYouOrIllegalState.class);
+
+      checkFilters(request);
+   }
+
    @Override
    protected TypeLiteral<RestAnnotationProcessor<AWSS3AsyncClient>> createTypeLiteral() {
       return new TypeLiteral<RestAnnotationProcessor<AWSS3AsyncClient>>() {
@@ -107,7 +149,8 @@ public class AWSS3AsyncClientTest extends org.jclouds.s3.S3AsyncClientTest<AWSS3
             + url + "\n");
       assertPayloadEquals(request, null, null, false);
 
-      // as this is a payload-related command, but with no payload, be careful that we check
+      // as this is a payload-related command, but with no payload, be careful
+      // that we check
       // filtering and do not ignore if this fails later.
       request = request.getFilters().get(0).filter(request);
 
@@ -185,8 +228,8 @@ public class AWSS3AsyncClientTest extends org.jclouds.s3.S3AsyncClientTest<AWSS3
             .newInstance(PutBucketOptions.class, 0).getClass());
       HttpRequest request = processor.createRequest(method, "EU", "bucket");
 
-      assertRequestLineEquals(request, "PUT https://bucket." + url + "/ HTTP/1.1");
-      assertNonPayloadHeadersEqual(request, "Host: bucket." + url + "\n");
+      assertRequestLineEquals(request, "PUT https://bucket.bucketendpoint/ HTTP/1.1");
+      assertNonPayloadHeadersEqual(request, "Host: bucket.bucketendpoint\n");
       assertPayloadEquals(request,
             "<CreateBucketConfiguration><LocationConstraint>EU</LocationConstraint></CreateBucketConfiguration>",
             "text/xml", false);
@@ -214,8 +257,8 @@ public class AWSS3AsyncClientTest extends org.jclouds.s3.S3AsyncClientTest<AWSS3
       }
 
       @Override
-      protected URI provideLocationURI(String endpoint) {
-         return URI.create("https://bucket");
+      protected URI provideBucketURI(String endpoint) {
+         return URI.create("https://bucketendpoint");
       }
 
       @Override