Skip to content

Commit cbcfb57

Browse files
author
Ajay Kannan
committed
Fix javadoc, handle potential null testPermissions response, don't use single letter variable names
1 parent bea5d1d commit cbcfb57

File tree

6 files changed

+69
-33
lines changed

6 files changed

+69
-33
lines changed

gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/ResourceManager.java

+29-8
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import com.google.common.base.Joiner;
2020
import com.google.common.collect.Sets;
21+
import com.google.gcloud.IamPolicy;
2122
import com.google.gcloud.Page;
2223
import com.google.gcloud.Service;
2324
import com.google.gcloud.spi.ResourceManagerRpc;
@@ -184,10 +185,12 @@ public enum Permission {
184185
SET_BILLING("setBillingAccount"),
185186
UNDELETE("undelete");
186187

188+
private static final String PREFIX = "resourcemanager.projects.";
189+
187190
private final String strValue;
188191

189192
Permission(String suffix) {
190-
this.strValue = "resourcemanager.projects." + suffix;
193+
this.strValue = PREFIX + suffix;
191194
}
192195

193196
String strValue() {
@@ -196,7 +199,7 @@ String strValue() {
196199
}
197200

198201
/**
199-
* Create a new project.
202+
* Creates a new project.
200203
*
201204
* <p>Initially, the project resource is owned by its creator exclusively. The creator can later
202205
* grant permission to others to read or update the project. Several APIs are activated
@@ -307,7 +310,7 @@ String strValue() {
307310
* Sets the IAM access control policy for the specified project. Replaces any existing policy. The
308311
* following constraints apply:
309312
* <ul>
310-
* <li>Projects currently support only <I>user:{emailid}</I> and <I>serviceAccount:{emailid}</I>
313+
* <li>Projects currently support only <i>user:{emailid}</i> and <i>serviceAccount:{emailid}</i>
311314
* members in a binding of a policy.
312315
* <li>To be added as an owner, a user must be invited via Cloud Platform console and must accept
313316
* the invitation.
@@ -324,11 +327,11 @@ String strValue() {
324327
* It is recommended that you use the read-modify-write pattern. This pattern entails reading the
325328
* project's current policy, updating it locally, and then sending the modified policy for
326329
* writing. Cloud IAM solves the problem of conflicting processes simultaneously attempting to
327-
* modify a policy by using the etag property. This property is used to verify whether the
328-
* policy has changed since the last request. When you make a request to Cloud IAM with an etag
329-
* value, Cloud IAM compares the etag value in the request with the existing etag value associated
330-
* with the policy. It writes the policy only if the etag values match. If an etag is not
331-
* provided, the policy is overwritten blindly.
330+
* modify a policy by using the {@link IamPolicy#etag etag} property. This property is used to
331+
* verify whether the policy has changed since the last request. When you make a request to Cloud
332+
* IAM with an etag value, Cloud IAM compares the etag value in the request with the existing etag
333+
* value associated with the policy. It writes the policy only if the etag values match. If an
334+
* etag is not provided, the policy is overwritten blindly.
332335
*
333336
* An example of using the read-write-modify pattern is as follows:
334337
* <pre> {@code
@@ -356,7 +359,25 @@ String strValue() {
356359
* @see <a href=
357360
* "https://cloud.google.com/resource-manager/reference/rest/v1beta1/projects/testIamPermissions">
358361
* Resource Manager testIamPermissions</a>
362+
* @returns A list of booleans representing whether the caller has the permissions specified. The
363+
* boolean responses are in the same order as the given list of permissions.
359364
* @throws ResourceManagerException upon failure
360365
*/
361366
List<Boolean> testPermissions(String projectId, List<Permission> permissions);
367+
368+
/**
369+
* Returns the permissions that a caller has on the specified project. You typically don't call
370+
* this method if you're using Google Cloud Platform directly to manage permissions. This method
371+
* is intended for integration with your proprietary software, such as a customized graphical user
372+
* interface. For example, the Cloud Platform Console tests IAM permissions internally to
373+
* determine which UI should be available to the logged-in user.
374+
*
375+
* @see <a href=
376+
* "https://cloud.google.com/resource-manager/reference/rest/v1beta1/projects/testIamPermissions">
377+
* Resource Manager testIamPermissions</a>
378+
* @returns A list of booleans representing whether the caller has the permissions specified. The
379+
* boolean responses are in the same order as the given permissions.
380+
* @throws ResourceManagerException upon failure
381+
*/
382+
List<Boolean> testPermissions(String projectId, Permission first, Permission... others);
362383
}

gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/ResourceManagerImpl.java

+30-20
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@
3333
import com.google.gcloud.spi.ResourceManagerRpc;
3434
import com.google.gcloud.spi.ResourceManagerRpc.Tuple;
3535

36+
import java.util.Arrays;
37+
import java.util.LinkedList;
3638
import java.util.List;
3739
import java.util.Map;
3840
import java.util.concurrent.Callable;
@@ -57,8 +59,8 @@ public com.google.api.services.cloudresourcemanager.model.Project call() {
5759
return resourceManagerRpc.create(project.toPb());
5860
}
5961
}, options().retryParams(), EXCEPTION_HANDLER));
60-
} catch (RetryHelperException e) {
61-
throw ResourceManagerException.translateAndThrow(e);
62+
} catch (RetryHelperException ex) {
63+
throw ResourceManagerException.translateAndThrow(ex);
6264
}
6365
}
6466

@@ -72,8 +74,8 @@ public Void call() {
7274
return null;
7375
}
7476
}, options().retryParams(), EXCEPTION_HANDLER);
75-
} catch (RetryHelperException e) {
76-
throw ResourceManagerException.translateAndThrow(e);
77+
} catch (RetryHelperException ex) {
78+
throw ResourceManagerException.translateAndThrow(ex);
7779
}
7880
}
7981

@@ -89,8 +91,8 @@ public com.google.api.services.cloudresourcemanager.model.Project call() {
8991
}
9092
}, options().retryParams(), EXCEPTION_HANDLER);
9193
return answer == null ? null : Project.fromPb(this, answer);
92-
} catch (RetryHelperException e) {
93-
throw ResourceManagerException.translateAndThrow(e);
94+
} catch (RetryHelperException ex) {
95+
throw ResourceManagerException.translateAndThrow(ex);
9496
}
9597
}
9698

@@ -148,8 +150,8 @@ public Project apply(
148150
});
149151
return new PageImpl<>(
150152
new ProjectPageFetcher(serviceOptions, cursor, optionsMap), cursor, projects);
151-
} catch (RetryHelperException e) {
152-
throw ResourceManagerException.translateAndThrow(e);
153+
} catch (RetryHelperException ex) {
154+
throw ResourceManagerException.translateAndThrow(ex);
153155
}
154156
}
155157

@@ -163,8 +165,8 @@ public com.google.api.services.cloudresourcemanager.model.Project call() {
163165
return resourceManagerRpc.replace(newProject.toPb());
164166
}
165167
}, options().retryParams(), EXCEPTION_HANDLER));
166-
} catch (RetryHelperException e) {
167-
throw ResourceManagerException.translateAndThrow(e);
168+
} catch (RetryHelperException ex) {
169+
throw ResourceManagerException.translateAndThrow(ex);
168170
}
169171
}
170172

@@ -178,8 +180,8 @@ public Void call() {
178180
return null;
179181
}
180182
}, options().retryParams(), EXCEPTION_HANDLER);
181-
} catch (RetryHelperException e) {
182-
throw ResourceManagerException.translateAndThrow(e);
183+
} catch (RetryHelperException ex) {
184+
throw ResourceManagerException.translateAndThrow(ex);
183185
}
184186
}
185187

@@ -197,8 +199,8 @@ public com.google.api.services.cloudresourcemanager.model.Policy call() {
197199
options().retryParams(),
198200
EXCEPTION_HANDLER);
199201
return answer == null ? null : Policy.fromPb(answer);
200-
} catch (RetryHelperException e) {
201-
throw ResourceManagerException.translateAndThrow(e);
202+
} catch (RetryHelperException ex) {
203+
throw ResourceManagerException.translateAndThrow(ex);
202204
}
203205
}
204206

@@ -212,8 +214,8 @@ public com.google.api.services.cloudresourcemanager.model.Policy call() {
212214
return resourceManagerRpc.replacePolicy(projectId, newPolicy.toPb());
213215
}
214216
}, options().retryParams(), EXCEPTION_HANDLER));
215-
} catch (RetryHelperException e) {
216-
throw ResourceManagerException.translateAndThrow(e);
217+
} catch (RetryHelperException ex) {
218+
throw ResourceManagerException.translateAndThrow(ex);
217219
}
218220
}
219221

@@ -227,17 +229,25 @@ public List<Boolean> call() {
227229
return resourceManagerRpc.testPermissions(projectId,
228230
Lists.transform(permissions, new Function<Permission, String>() {
229231
@Override
230-
public String apply(Permission p) {
231-
return p.strValue();
232+
public String apply(Permission permission) {
233+
return permission.strValue();
232234
}
233235
}));
234236
}
235237
}, options().retryParams(), EXCEPTION_HANDLER);
236-
} catch (RetryHelperException e) {
237-
throw ResourceManagerException.translateAndThrow(e);
238+
} catch (RetryHelperException ex) {
239+
throw ResourceManagerException.translateAndThrow(ex);
238240
}
239241
}
240242

243+
@Override
244+
public List<Boolean> testPermissions(String projectId, Permission first, Permission... others) {
245+
List<Permission> permissions = new LinkedList<>();
246+
permissions.add(first);
247+
permissions.addAll(Arrays.asList(others));
248+
return testPermissions(projectId, permissions);
249+
}
250+
241251
private Map<ResourceManagerRpc.Option, ?> optionMap(Option... options) {
242252
Map<ResourceManagerRpc.Option, Object> temp = Maps.newEnumMap(ResourceManagerRpc.Option.class);
243253
for (Option option : options) {

gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/testing/LocalResourceManagerHelper.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -59,14 +59,14 @@
5959
*
6060
* <ul>
6161
* <li>This mock assumes you have adequate permissions for any action. Related to this,
62-
* <I>testIamPermissions</I> always indicates that the caller has all permissions listed in the
62+
* <i>testIamPermissions</i> always indicates that the caller has all permissions listed in the
6363
* request.
6464
* <li>IAM policies are set to an empty policy with version 0 (only legacy roles supported) upon
6565
* project creation. The actual service will not have an empty list of bindings and may also
6666
* set your version to 1.
6767
* <li>There is no input validation for the policy provided when replacing a policy.
68-
* <li>In this mock, projects never move from the <I>DELETE_REQUESTED</I> lifecycle state to
69-
* <I>DELETE_IN_PROGRESS</I> without an explicit call to the utility method
68+
* <li>In this mock, projects never move from the <i>DELETE_REQUESTED</i> lifecycle state to
69+
* <i>DELETE_IN_PROGRESS</i> without an explicit call to the utility method
7070
* {@link #changeLifecycleState}. Similarly, a project is never completely removed without an
7171
* explicit call to the utility method {@link #removeProject}.
7272
* <li>The messages in the error responses given by this mock do not necessarily match the messages

gcloud-java-resourcemanager/src/main/java/com/google/gcloud/spi/DefaultResourceManagerRpc.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.google.gcloud.spi;
22

3+
import static com.google.common.base.MoreObjects.firstNonNull;
34
import static com.google.gcloud.spi.ResourceManagerRpc.Option.FIELDS;
45
import static com.google.gcloud.spi.ResourceManagerRpc.Option.FILTER;
56
import static com.google.gcloud.spi.ResourceManagerRpc.Option.PAGE_SIZE;
@@ -152,7 +153,8 @@ public List<Boolean> testPermissions(String projectId, List<String> permissions)
152153
.testIamPermissions(
153154
projectId, new TestIamPermissionsRequest().setPermissions(permissions))
154155
.execute();
155-
Set<String> permissionsOwned = ImmutableSet.copyOf(response.getPermissions());
156+
Set<String> permissionsOwned =
157+
ImmutableSet.copyOf(firstNonNull(response.getPermissions(), ImmutableList.<String>of()));
156158
ImmutableList.Builder<Boolean> answer = ImmutableList.builder();
157159
for (String p : permissions) {
158160
answer.add(permissionsOwned.contains(p));

gcloud-java-resourcemanager/src/main/java/com/google/gcloud/spi/ResourceManagerRpc.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ public Y y() {
131131
Policy getPolicy(String projectId);
132132

133133
/**
134-
* Replaces the IAM policy associated with the given project ID.
134+
* Replaces the IAM policy associated with the given project.
135135
*
136136
* @throws ResourceManagerException upon failure
137137
*/

gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/ResourceManagerImplTest.java

+3
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,9 @@ public void testTestPermissions() {
378378
RESOURCE_MANAGER.create(PARTIAL_PROJECT);
379379
assertEquals(ImmutableList.of(true),
380380
RESOURCE_MANAGER.testPermissions(PARTIAL_PROJECT.projectId(), permissions));
381+
assertEquals(ImmutableList.of(true, true),
382+
RESOURCE_MANAGER.testPermissions(
383+
PARTIAL_PROJECT.projectId(), Permission.DELETE, Permission.OWN));
381384
}
382385

383386
@Test

0 commit comments

Comments
 (0)