From c597e079d5e8826523237cf044c969f38d9174d9 Mon Sep 17 00:00:00 2001
From: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Date: Fri, 30 Sep 2011 11:26:53 +0100
Subject: [PATCH] fix to check for nullable in those places where we test that
 nullable is supported, and to test that leaving out nullable disallows null
 parameters

---
 .../internal/RestAnnotationProcessor.java     | 15 +++++++
 .../internal/RestAnnotationProcessorTest.java | 40 +++++++++----------
 2 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java b/core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java
index 75f96b24c9..19321a8228 100644
--- a/core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java
+++ b/core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java
@@ -960,6 +960,21 @@ public class RestAnnotationProcessor<T> {
             }
             if (shouldBreak)
                break OUTER;
+         } else {
+            // either arg is null, or request.getArgs().size() < entry.getKey() + 1
+            // in either case, we require that null be allowed
+            // (first, however, let's make sure we have enough args on the actual method)
+            if (entry.getKey() >= request.getJavaMethod().getParameterAnnotations().length) {
+               // not known whether this happens
+               throw new IllegalArgumentException("Argument index "+(entry.getKey()+1)+" is out of bounds for method "+request.getJavaMethod());
+            }
+            
+            Annotation[] annotations = request.getJavaMethod().getParameterAnnotations()[entry.getKey()];
+            for (Annotation a: annotations) {
+               if (Nullable.class.isAssignableFrom(a.annotationType()))
+                  continue OUTER;
+            }
+            Preconditions.checkNotNull(null, request.getJavaMethod().getName()+" parameter "+(entry.getKey()+1));
          }
       }
 
diff --git a/core/src/test/java/org/jclouds/rest/internal/RestAnnotationProcessorTest.java b/core/src/test/java/org/jclouds/rest/internal/RestAnnotationProcessorTest.java
index f943e2ee98..ad225d1884 100644
--- a/core/src/test/java/org/jclouds/rest/internal/RestAnnotationProcessorTest.java
+++ b/core/src/test/java/org/jclouds/rest/internal/RestAnnotationProcessorTest.java
@@ -609,27 +609,25 @@ public class RestAnnotationProcessorTest extends BaseRestClientTest {
       assertPayloadEquals(request, null, "application/unknown", false);
    }
 
-   //FIXME the following two tests fail, suggesting that the tests above aren't actually testing Nullable
-   //but rather indicates that RestAnnotationProcessor.decorateRequest silently ignores nulls
-//   public void testCreatePostRequestNullNotOk1() throws SecurityException, NoSuchMethodException, IOException {
-//      Method method = TestPost.class.getMethod("postNonnull", String.class);
-//      try {
-//         HttpRequest request = factory(TestPost.class).createRequest(method);
-//         Assert.fail("call should have failed with illegal null parameter, not permitted "+request+" to be created");
-//      } catch (NullPointerException e) {
-////         Assert.assertTrue(e.toString().indexOf("???")>=0, "Error message should have referred to parameter 'fooble': "+e);
-//      }
-//   }
-//
-//   public void testCreatePostRequestNullNotOk2() throws SecurityException, NoSuchMethodException, IOException {
-//      Method method = TestPost.class.getMethod("postNonnull", String.class);
-//      try {
-//         HttpRequest request = factory(TestPost.class).createRequest(method, (String)null);
-//         Assert.fail("call should have failed with illegal null parameter, not permitted "+request+" to be created");
-//      } catch (NullPointerException e) {
-//         //      Assert.assertTrue(e.toString().indexOf("???")>=0, "Error message should have referred to parameter 'fooble': "+e);
-//      }
-//   }
+   public void testCreatePostRequestNullNotOk1() throws SecurityException, NoSuchMethodException, IOException {
+      Method method = TestPost.class.getMethod("postNonnull", String.class);
+      try {
+         HttpRequest request = factory(TestPost.class).createRequest(method);
+         Assert.fail("call should have failed with illegal null parameter, not permitted "+request+" to be created");
+      } catch (NullPointerException e) {
+         Assert.assertTrue(e.toString().indexOf("postNonnull parameter 1")>=0, "Error message should have referred to 'parameter 1': "+e);
+      }
+   }
+
+   public void testCreatePostRequestNullNotOk2() throws SecurityException, NoSuchMethodException, IOException {
+      Method method = TestPost.class.getMethod("postNonnull", String.class);
+      try {
+         HttpRequest request = factory(TestPost.class).createRequest(method, (String)null);
+         Assert.fail("call should have failed with illegal null parameter, not permitted "+request+" to be created");
+      } catch (NullPointerException e) {
+               Assert.assertTrue(e.toString().indexOf("postNonnull parameter 1")>=0, "Error message should have referred to parameter 'parameter 1': "+e);
+      }
+   }
 
    public void testCreatePostJsonRequest() throws SecurityException, NoSuchMethodException, IOException {
       Method method = TestPost.class.getMethod("postAsJson", String.class);