From 49431a5412c8109ac6e65f86e047099206be4db6 Mon Sep 17 00:00:00 2001
From: Ajay Kannan <ajaykannan@google.com>
Date: Mon, 7 Mar 2016 08:54:22 -0800
Subject: [PATCH] Fix javadoc, handle potential null testPermissions response,
 don't use single letter variable names

---
 .../resourcemanager/ResourceManager.java      | 37 +++++++++++---
 .../resourcemanager/ResourceManagerImpl.java  | 50 +++++++++++--------
 .../testing/LocalResourceManagerHelper.java   |  6 +--
 .../gcloud/spi/DefaultResourceManagerRpc.java |  4 +-
 .../google/gcloud/spi/ResourceManagerRpc.java |  2 +-
 .../ResourceManagerImplTest.java              |  3 ++
 6 files changed, 69 insertions(+), 33 deletions(-)

diff --git a/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/ResourceManager.java b/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/ResourceManager.java
index 979de6e95580..6d79faf18862 100644
--- a/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/ResourceManager.java
+++ b/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/ResourceManager.java
@@ -18,6 +18,7 @@
 
 import com.google.common.base.Joiner;
 import com.google.common.collect.Sets;
+import com.google.gcloud.IamPolicy;
 import com.google.gcloud.Page;
 import com.google.gcloud.Service;
 import com.google.gcloud.spi.ResourceManagerRpc;
@@ -184,10 +185,12 @@ public enum Permission {
     SET_BILLING("setBillingAccount"),
     UNDELETE("undelete");
 
+    private static final String PREFIX = "resourcemanager.projects.";
+
     private final String strValue;
 
     Permission(String suffix) {
-      this.strValue = "resourcemanager.projects." + suffix;
+      this.strValue = PREFIX + suffix;
     }
 
     String strValue() {
@@ -196,7 +199,7 @@ String strValue() {
   }
 
   /**
-   * Create a new project.
+   * Creates a new project.
    *
    * <p>Initially, the project resource is owned by its creator exclusively. The creator can later
    * grant permission to others to read or update the project. Several APIs are activated
@@ -307,7 +310,7 @@ String strValue() {
    * Sets the IAM access control policy for the specified project. Replaces any existing policy. The
    * following constraints apply:
    * <ul>
-   * <li>Projects currently support only <I>user:{emailid}</I> and <I>serviceAccount:{emailid}</I>
+   * <li>Projects currently support only <i>user:{emailid}</i> and <i>serviceAccount:{emailid}</i>
    *     members in a binding of a policy.
    * <li>To be added as an owner, a user must be invited via Cloud Platform console and must accept
    *     the invitation.
@@ -324,11 +327,11 @@ String strValue() {
    * It is recommended that you use the read-modify-write pattern. This pattern entails reading the
    * project's current policy, updating it locally, and then sending the modified policy for
    * writing. Cloud IAM solves the problem of conflicting processes simultaneously attempting to
-   * modify a policy by using the etag property. This property is used to verify whether the
-   * policy has changed since the last request. When you make a request to Cloud IAM with an etag
-   * value, Cloud IAM compares the etag value in the request with the existing etag value associated
-   * with the policy. It writes the policy only if the etag values match. If an etag is not
-   * provided, the policy is overwritten blindly.
+   * modify a policy by using the {@link IamPolicy#etag etag} property. This property is used to
+   * verify whether the policy has changed since the last request. When you make a request to Cloud
+   * IAM with an etag value, Cloud IAM compares the etag value in the request with the existing etag
+   * value associated with the policy. It writes the policy only if the etag values match. If an
+   * etag is not provided, the policy is overwritten blindly.
    *
    * An example of using the read-write-modify pattern is as follows:
    * <pre> {@code
@@ -356,7 +359,25 @@ String strValue() {
    * @see <a href=
    * "https://cloud.google.com/resource-manager/reference/rest/v1beta1/projects/testIamPermissions">
    * Resource Manager testIamPermissions</a>
+   * @return A list of booleans representing whether the caller has the permissions specified. The
+   *     boolean responses are in the same order as the given list of permissions.
    * @throws ResourceManagerException upon failure
    */
   List<Boolean> testPermissions(String projectId, List<Permission> permissions);
+
+  /**
+   * Returns the permissions that a caller has on the specified project. You typically don't call
+   * this method if you're using Google Cloud Platform directly to manage permissions. This method
+   * is intended for integration with your proprietary software, such as a customized graphical user
+   * interface. For example, the Cloud Platform Console tests IAM permissions internally to
+   * determine which UI should be available to the logged-in user.
+   *
+   * @see <a href=
+   * "https://cloud.google.com/resource-manager/reference/rest/v1beta1/projects/testIamPermissions">
+   * Resource Manager testIamPermissions</a>
+   * @return A list of booleans representing whether the caller has the permissions specified. The
+   *     boolean responses are in the same order as the given permissions.
+   * @throws ResourceManagerException upon failure
+   */
+  List<Boolean> testPermissions(String projectId, Permission first, Permission... others);
 }
diff --git a/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/ResourceManagerImpl.java b/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/ResourceManagerImpl.java
index 09f8a9bb9229..b1a844361b85 100644
--- a/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/ResourceManagerImpl.java
+++ b/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/ResourceManagerImpl.java
@@ -33,6 +33,8 @@
 import com.google.gcloud.spi.ResourceManagerRpc;
 import com.google.gcloud.spi.ResourceManagerRpc.Tuple;
 
+import java.util.Arrays;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.Callable;
@@ -57,8 +59,8 @@ public com.google.api.services.cloudresourcemanager.model.Project call() {
               return resourceManagerRpc.create(project.toPb());
             }
           }, options().retryParams(), EXCEPTION_HANDLER));
-    } catch (RetryHelperException e) {
-      throw ResourceManagerException.translateAndThrow(e);
+    } catch (RetryHelperException ex) {
+      throw ResourceManagerException.translateAndThrow(ex);
     }
   }
 
@@ -72,8 +74,8 @@ public Void call() {
           return null;
         }
       }, options().retryParams(), EXCEPTION_HANDLER);
-    } catch (RetryHelperException e) {
-      throw ResourceManagerException.translateAndThrow(e);
+    } catch (RetryHelperException ex) {
+      throw ResourceManagerException.translateAndThrow(ex);
     }
   }
 
@@ -89,8 +91,8 @@ public com.google.api.services.cloudresourcemanager.model.Project call() {
             }
           }, options().retryParams(), EXCEPTION_HANDLER);
       return answer == null ? null : Project.fromPb(this, answer);
-    } catch (RetryHelperException e) {
-      throw ResourceManagerException.translateAndThrow(e);
+    } catch (RetryHelperException ex) {
+      throw ResourceManagerException.translateAndThrow(ex);
     }
   }
 
@@ -148,8 +150,8 @@ public Project apply(
                   });
       return new PageImpl<>(
           new ProjectPageFetcher(serviceOptions, cursor, optionsMap), cursor, projects);
-    } catch (RetryHelperException e) {
-      throw ResourceManagerException.translateAndThrow(e);
+    } catch (RetryHelperException ex) {
+      throw ResourceManagerException.translateAndThrow(ex);
     }
   }
 
@@ -163,8 +165,8 @@ public com.google.api.services.cloudresourcemanager.model.Project call() {
               return resourceManagerRpc.replace(newProject.toPb());
             }
           }, options().retryParams(), EXCEPTION_HANDLER));
-    } catch (RetryHelperException e) {
-      throw ResourceManagerException.translateAndThrow(e);
+    } catch (RetryHelperException ex) {
+      throw ResourceManagerException.translateAndThrow(ex);
     }
   }
 
@@ -178,8 +180,8 @@ public Void call() {
           return null;
         }
       }, options().retryParams(), EXCEPTION_HANDLER);
-    } catch (RetryHelperException e) {
-      throw ResourceManagerException.translateAndThrow(e);
+    } catch (RetryHelperException ex) {
+      throw ResourceManagerException.translateAndThrow(ex);
     }
   }
 
@@ -197,8 +199,8 @@ public com.google.api.services.cloudresourcemanager.model.Policy call() {
               options().retryParams(),
               EXCEPTION_HANDLER);
       return answer == null ? null : Policy.fromPb(answer);
-    } catch (RetryHelperException e) {
-      throw ResourceManagerException.translateAndThrow(e);
+    } catch (RetryHelperException ex) {
+      throw ResourceManagerException.translateAndThrow(ex);
     }
   }
 
@@ -212,8 +214,8 @@ public com.google.api.services.cloudresourcemanager.model.Policy call() {
               return resourceManagerRpc.replacePolicy(projectId, newPolicy.toPb());
             }
           }, options().retryParams(), EXCEPTION_HANDLER));
-    } catch (RetryHelperException e) {
-      throw ResourceManagerException.translateAndThrow(e);
+    } catch (RetryHelperException ex) {
+      throw ResourceManagerException.translateAndThrow(ex);
     }
   }
 
@@ -227,17 +229,25 @@ public List<Boolean> call() {
               return resourceManagerRpc.testPermissions(projectId,
                   Lists.transform(permissions, new Function<Permission, String>() {
                       @Override
-                      public String apply(Permission p) {
-                        return p.strValue();
+                      public String apply(Permission permission) {
+                        return permission.strValue();
                       }
                   }));
             }
           }, options().retryParams(), EXCEPTION_HANDLER);
-    } catch (RetryHelperException e) {
-      throw ResourceManagerException.translateAndThrow(e);
+    } catch (RetryHelperException ex) {
+      throw ResourceManagerException.translateAndThrow(ex);
     }
   }
 
+  @Override
+  public List<Boolean> testPermissions(String projectId, Permission first, Permission... others) {
+    List<Permission> permissions = new LinkedList<>();
+    permissions.add(first);
+    permissions.addAll(Arrays.asList(others));
+    return testPermissions(projectId, permissions);
+  }
+
   private Map<ResourceManagerRpc.Option, ?> optionMap(Option... options) {
     Map<ResourceManagerRpc.Option, Object> temp = Maps.newEnumMap(ResourceManagerRpc.Option.class);
     for (Option option : options) {
diff --git a/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/testing/LocalResourceManagerHelper.java b/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/testing/LocalResourceManagerHelper.java
index b8bfcb77b76c..45e8a09b089b 100644
--- a/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/testing/LocalResourceManagerHelper.java
+++ b/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/testing/LocalResourceManagerHelper.java
@@ -59,14 +59,14 @@
  *
  * <ul>
  * <li>This mock assumes you have adequate permissions for any action. Related to this,
- *     <I>testIamPermissions</I> always indicates that the caller has all permissions listed in the
+ *     <i>testIamPermissions</i> always indicates that the caller has all permissions listed in the
  *     request.
  * <li>IAM policies are set to an empty policy with version 0 (only legacy roles supported) upon
  *     project creation. The actual service will not have an empty list of bindings and may also
  *     set your version to 1.
  * <li>There is no input validation for the policy provided when replacing a policy.
- * <li>In this mock, projects never move from the <I>DELETE_REQUESTED</I> lifecycle state to
- *     <I>DELETE_IN_PROGRESS</I> without an explicit call to the utility method
+ * <li>In this mock, projects never move from the <i>DELETE_REQUESTED</i> lifecycle state to
+ *     <i>DELETE_IN_PROGRESS</i> without an explicit call to the utility method
  *     {@link #changeLifecycleState}.  Similarly, a project is never completely removed without an
  *     explicit call to the utility method {@link #removeProject}.
  * <li>The messages in the error responses given by this mock do not necessarily match the messages
diff --git a/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/spi/DefaultResourceManagerRpc.java b/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/spi/DefaultResourceManagerRpc.java
index fab14061d433..6902f1ce1685 100644
--- a/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/spi/DefaultResourceManagerRpc.java
+++ b/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/spi/DefaultResourceManagerRpc.java
@@ -1,5 +1,6 @@
 package com.google.gcloud.spi;
 
+import static com.google.common.base.MoreObjects.firstNonNull;
 import static com.google.gcloud.spi.ResourceManagerRpc.Option.FIELDS;
 import static com.google.gcloud.spi.ResourceManagerRpc.Option.FILTER;
 import static com.google.gcloud.spi.ResourceManagerRpc.Option.PAGE_SIZE;
@@ -152,7 +153,8 @@ public List<Boolean> testPermissions(String projectId, List<String> permissions)
           .testIamPermissions(
               projectId, new TestIamPermissionsRequest().setPermissions(permissions))
           .execute();
-      Set<String> permissionsOwned = ImmutableSet.copyOf(response.getPermissions());
+      Set<String> permissionsOwned =
+          ImmutableSet.copyOf(firstNonNull(response.getPermissions(), ImmutableList.<String>of()));
       ImmutableList.Builder<Boolean> answer = ImmutableList.builder();
       for (String p : permissions) {
         answer.add(permissionsOwned.contains(p));
diff --git a/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/spi/ResourceManagerRpc.java b/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/spi/ResourceManagerRpc.java
index dd009c52b05e..c556ea684380 100644
--- a/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/spi/ResourceManagerRpc.java
+++ b/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/spi/ResourceManagerRpc.java
@@ -131,7 +131,7 @@ public Y y() {
   Policy getPolicy(String projectId);
 
   /**
-   * Replaces the IAM policy associated with the given project ID.
+   * Replaces the IAM policy associated with the given project.
    *
    * @throws ResourceManagerException upon failure
    */
diff --git a/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/ResourceManagerImplTest.java b/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/ResourceManagerImplTest.java
index 68a70f65114a..ada14ab4c977 100644
--- a/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/ResourceManagerImplTest.java
+++ b/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/ResourceManagerImplTest.java
@@ -378,6 +378,9 @@ public void testTestPermissions() {
     RESOURCE_MANAGER.create(PARTIAL_PROJECT);
     assertEquals(ImmutableList.of(true),
         RESOURCE_MANAGER.testPermissions(PARTIAL_PROJECT.projectId(), permissions));
+    assertEquals(ImmutableList.of(true, true),
+        RESOURCE_MANAGER.testPermissions(
+            PARTIAL_PROJECT.projectId(), Permission.DELETE, Permission.OWN));
   }
 
   @Test