Path annotation ignored when endpoint not set on caller

core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java

@@ -380,53 +380,31 @@ public class RestAnnotationProcessor<T> {
    final Injector injector;
 
    private ClassMethodArgs caller;
-   private URI callerEndpoint;
 
    public void setCaller(ClassMethodArgs caller) {
       seedAnnotationCache.getUnchecked(caller.getMethod().getDeclaringClass());
       this.caller = caller;
-      try {
-         UriBuilder builder = uriBuilderProvider.get().uri(getEndpointFor(caller.getMethod(), caller.getArgs(), injector));
-         Multimap<String, String> tokenValues = addPathAndGetTokens(caller.getMethod().getDeclaringClass(), caller.getMethod(), caller.getArgs(), builder);
-         callerEndpoint = builder.buildFromEncodedMap(Maps2.convertUnsafe(tokenValues));
-      } catch (IllegalStateException e) {
-         // no endpoint annotation
-      }
    }
 
    public GeneratedHttpRequest createRequest(Method method, Object... args) {
       inputParamValidator.validateMethodParametersOrThrow(method, args);
-      ClassMethodArgs cma = logger.isTraceEnabled() ? new ClassMethodArgs(method.getDeclaringClass(), method, args)
-            : null;
 
-      URI endpoint = callerEndpoint;
+
-      try {
+      Optional<URI> endpoint = findEndpoint(method, args);
-         if (endpoint == null) {
+
-            endpoint = getEndpointFor(method, args, injector);
+      if (!endpoint.isPresent()) {
-            logger.trace("using endpoint %s for %s", endpoint, cma);
+         throw new NoSuchElementException(String.format("no endpoint found for %s",
-         } else {
+               new ClassMethodArgs(method.getDeclaringClass(), method, args)));
-            logger.trace("using endpoint %s from caller %s for %s", caller, endpoint, cma);
-         }
-      } catch (IllegalStateException e) {
-         logger.trace("looking up default endpoint for %s", cma);
-         endpoint = injector.getInstance(Key.get(uriSupplierLiteral, org.jclouds.location.Provider.class)).get();
-         logger.trace("using default endpoint %s for %s", endpoint, cma);
       }
-      GeneratedHttpRequest.Builder<?> requestBuilder;
+
+      GeneratedHttpRequest.Builder<?> requestBuilder = GeneratedHttpRequest.builder();
       HttpRequest r = RestAnnotationProcessor.findHttpRequestInArgs(args);
       if (r != null) {
-         requestBuilder = GeneratedHttpRequest.builder().fromHttpRequest(r);
+         requestBuilder.fromHttpRequest(r);
-         endpoint = r.getEndpoint();
       } else {
-         requestBuilder = GeneratedHttpRequest.builder();
          requestBuilder.method(getHttpMethodOrConstantOrThrowException(method));
       }
 
-      if (endpoint == null) {
-         throw new NoSuchElementException(String.format("no endpoint found for %s",
-                  new ClassMethodArgs(method.getDeclaringClass(), method, args)));
-      }
-
       requestBuilder.declaring(declaring)
                     .javaMethod(method)
                     .args(args)
@@ -434,15 +412,21 @@ public class RestAnnotationProcessor<T> {
                     .skips(skips)
                     .filters(getFiltersIfAnnotated(method));
 
-      UriBuilder builder = uriBuilderProvider.get().uri(endpoint);
+      UriBuilder builder = uriBuilderProvider.get().uri(endpoint.get());
-
+      
       Multimap<String, String> tokenValues = LinkedHashMultimap.create();
 
       tokenValues.put(Constants.PROPERTY_API_VERSION, apiVersion);
       tokenValues.put(Constants.PROPERTY_BUILD_VERSION, buildVersion);
 
+      // make sure any path from the caller is a prefix
+      if (caller != null) {
+         tokenValues.putAll(addPathAndGetTokens(caller.getMethod().getDeclaringClass(), caller.getMethod(),
+               caller.getArgs(), builder));
+      }
+      
       tokenValues.putAll(addPathAndGetTokens(declaring, method, args, builder));
-
+      
       Multimap<String, String> formParams = addFormParams(tokenValues.entries(), method, args);
       Multimap<String, String> queryParams = addQueryParams(tokenValues.entries(), method, args);
       Multimap<String, String> matrixParams = addMatrixParams(tokenValues.entries(), method, args);
@@ -451,9 +435,9 @@ public class RestAnnotationProcessor<T> {
          headers.putAll(r.getHeaders());
 
       if (shouldAddHostHeader(method)) {
-         StringBuilder hostHeader = new StringBuilder(endpoint.getHost());
+         StringBuilder hostHeader = new StringBuilder(endpoint.get().getHost());
-         if (endpoint.getPort() != -1)
+         if (endpoint.get().getPort() != -1)
-            hostHeader.append(":").append(endpoint.getPort());
+            hostHeader.append(":").append(endpoint.get().getPort());
          headers.put(HOST, hostHeader.toString());
       }
 
@@ -540,6 +524,39 @@ public class RestAnnotationProcessor<T> {
       return request;
    }
 
+   private Optional<URI> findEndpoint(Method method, Object... args) {
+      ClassMethodArgs cma = logger.isTraceEnabled() ? new ClassMethodArgs(method.getDeclaringClass(), method, args)
+            : null;
+      Optional<URI> endpoint = Optional.absent();
+
+      HttpRequest r = RestAnnotationProcessor.findHttpRequestInArgs(args);
+
+      if (r != null) {
+         endpoint = Optional.fromNullable(r.getEndpoint());
+         if (endpoint.isPresent())
+            logger.trace("using endpoint %s from args for %s", endpoint, cma);
+      }
+
+      if (!endpoint.isPresent() && caller != null) {
+         endpoint = getEndpointFor(caller.getMethod(), caller.getArgs());
+         if (endpoint.isPresent())
+            logger.trace("using endpoint %s from caller %s for %s", endpoint, caller, cma);
+      }
+      if (!endpoint.isPresent()) {
+         endpoint = getEndpointFor(method, args);
+         if (endpoint.isPresent())
+            logger.trace("using endpoint %s for %s", endpoint, cma);
+      }
+      if (!endpoint.isPresent()) {
+         logger.trace("looking up default endpoint for %s", cma);
+         endpoint = Optional.fromNullable(injector.getInstance(
+               Key.get(uriSupplierLiteral, org.jclouds.location.Provider.class)).get());
+         if (endpoint.isPresent())
+            logger.trace("using default endpoint %s for %s", endpoint, cma);
+      }
+      return endpoint;
+   }
+
    public static Multimap<String, String> filterOutContentHeaders(Multimap<String, String> headers) {
       // http message usually comes in as a null key header, let's filter it
       // out.
@@ -745,7 +762,7 @@ public class RestAnnotationProcessor<T> {
    };
 
    // TODO: change to LoadingCache<ClassMethodArgs, URI> and move this logic to the CacheLoader.
-   public static URI getEndpointFor(Method method, Object[] args, Injector injector) {
+   private Optional<URI> getEndpointFor(Method method, Object[] args) {
       URI endpoint = getEndpointInParametersOrNull(method, args, injector);
       if (endpoint == null) {
          Endpoint annotation;
@@ -754,16 +771,18 @@ public class RestAnnotationProcessor<T> {
          } else if (method.getDeclaringClass().isAnnotationPresent(Endpoint.class)) {
             annotation = method.getDeclaringClass().getAnnotation(Endpoint.class);
          } else {
-            throw new IllegalStateException("no annotations on class or method: " + method);
+            logger.trace("no annotations on class or method: %s", method);
+            return Optional.absent();
          }
          endpoint = injector.getInstance(Key.get(uriSupplierLiteral, annotation.value())).get();
       }
       URI providerEndpoint = injector.getInstance(Key.get(uriSupplierLiteral, org.jclouds.location.Provider.class))
                .get();
-      return addHostIfMissing(endpoint, providerEndpoint);
+      return Optional.fromNullable(addHostIfMissing(endpoint, providerEndpoint));
    }
 
-   public static URI addHostIfMissing(URI original, URI withHost) {
+   @VisibleForTesting
+   static URI addHostIfMissing(URI original, URI withHost) {
       checkNotNull(withHost, "URI withHost cannot be null");
       checkArgument(withHost.getHost() != null, "URI withHost must have host:" + withHost);
 

core/src/test/java/org/jclouds/rest/annotationparsing/DelegateAnnotationExpectTest.java

@@ -0,0 +1,116 @@
+/**
+ * Licensed to jclouds, Inc. (jclouds) under one or more
+ * contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  jclouds licenses this file
+ * to you 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.rest.annotationparsing;
+
+import static org.jclouds.providers.AnonymousProviderMetadata.forClientMappedToAsyncClientOnEndpoint;
+import static org.testng.Assert.assertTrue;
+
+import java.util.concurrent.TimeUnit;
+
+import javax.ws.rs.HEAD;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+
+import org.jclouds.concurrent.Timeout;
+import org.jclouds.http.HttpRequest;
+import org.jclouds.http.HttpResponse;
+import org.jclouds.providers.ProviderMetadata;
+import org.jclouds.rest.ConfiguresRestClient;
+import org.jclouds.rest.annotations.Delegate;
+import org.jclouds.rest.annotations.ExceptionParser;
+import org.jclouds.rest.config.RestClientModule;
+import org.jclouds.rest.functions.ReturnFalseOnNotFoundOr404;
+import org.jclouds.rest.internal.BaseRestClientExpectTest;
+import org.testng.annotations.Test;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.util.concurrent.ListenableFuture;
+import com.google.inject.Module;
+
+/**
+ * Tests the ways that {@link Delegate}
+ * 
+ * @author Adrian Cole
+ */
+@Test(groups = "unit", testName = "DelegateAnnotationExpectTest")
+public class DelegateAnnotationExpectTest extends BaseRestClientExpectTest<DelegateAnnotationExpectTest.DelegatingApi> {
+
+   @Timeout(duration = 60, timeUnit = TimeUnit.SECONDS)
+   static interface DelegatingApi {
+
+      @Delegate
+      @Path("/projects/{project}")
+      DiskApi getDiskApiForProject(@PathParam("project") String projectName);
+   }
+
+   static interface DelegatingAsyncApi {
+
+      @Delegate
+      @Path("/projects/{project}")
+      DiskAsyncApi getDiskApiForProject(@PathParam("project") String projectName);
+   }
+
+   @Timeout(duration = 1, timeUnit = TimeUnit.SECONDS)
+   static interface DiskApi {
+
+      boolean exists(@PathParam("disk") String diskName);
+   }
+
+   static interface DiskAsyncApi {
+
+      @HEAD
+      @Path("/disks/{disk}")
+      @ExceptionParser(ReturnFalseOnNotFoundOr404.class)
+      public ListenableFuture<Boolean> exists(@PathParam("disk") String diskName);
+   }
+
+   public void testDelegatingCallTakesIntoConsiderationCallerAndCalleePath() {
+
+      DelegatingApi client = requestSendsResponse(
+            HttpRequest.builder().method("HEAD").endpoint("http://mock/projects/prod/disks/disk1").build(),
+            HttpResponse.builder().statusCode(200).build());
+
+      assertTrue(client.getDiskApiForProject("prod").exists("disk1"));
+
+   }
+
+   // crufty junk until we inspect delegating api classes for all their client
+   // mappings and make a test helper for random classes.
+
+   @Override
+   public ProviderMetadata createProviderMetadata() {
+      return forClientMappedToAsyncClientOnEndpoint(DelegatingApi.class, DelegatingAsyncApi.class, "http://mock/");
+   }
+
+   @Override
+   protected Module createModule() {
+      return new DelegatingRestClientModule();
+   }
+
+   @ConfiguresRestClient
+   static class DelegatingRestClientModule extends RestClientModule<DelegatingApi, DelegatingAsyncApi> {
+
+      public DelegatingRestClientModule() {
+         // right now, we have to define the delegates by hand as opposed to
+         // reflection looking for coordinated annotations
+         super(ImmutableMap.<Class<?>, Class<?>> of(DiskApi.class, DiskAsyncApi.class));
+      }
+   }
+
+}