Skip to content

Commit e9336a8

Browse files
author
Ajay Kannan
committed
---
yaml --- r: 7069 b: refs/heads/tswast-patch-1 c: c05e738 h: refs/heads/master i: 7067: 6335e70
1 parent f528805 commit e9336a8

9 files changed

Lines changed: 74 additions & 89 deletions

File tree

‎[refs]‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,5 +57,5 @@ refs/tags/v0.18.0: 9d193c4c4b9d1c6f21515dd8e50836b9194ec9bb
5757
refs/tags/v0.19.0: e67b56e4d8dad5f9a7b38c9b2107c23c828f2ed5
5858
refs/tags/v0.20.0: 839f7fb7156535146aa1cb2c5aadd8d375d854e8
5959
refs/tags/v0.20.1: 370471f437f1f4f68a11e068df5cd6bf39edb1fa
60-
refs/heads/tswast-patch-1: 3e9e1a0e8e034248c090e9c9b9918d3a783412be
60+
refs/heads/tswast-patch-1: c05e738b1f17048560b781f4445ca1ccff7f0759
6161
refs/heads/pubsub-streaming-pull: 19262b752ee874eb2ca3b950eb2aef44d5a5267b

‎branches/tswast-patch-1/gcloud-java-core/src/main/java/com/google/gcloud/IamPolicy.java‎

Lines changed: 16 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -83,39 +83,8 @@ public final B bindings(Map<R, Set<Identity>> bindings) {
8383
return self();
8484
}
8585

86-
/**
87-
* Adds a binding to the policy.
88-
*
89-
* @throws IllegalArgumentException if the policy already contains a binding with the same role
90-
* or if the role or any identities are null
91-
*/
92-
public final B addBinding(R role, Set<Identity> identities) {
93-
verifyBinding(role, identities);
94-
checkArgument(!bindings.containsKey(role),
95-
"The policy already contains a binding with the role " + role.toString() + ".");
96-
bindings.put(role, new HashSet<Identity>(identities));
97-
return self();
98-
}
99-
100-
/**
101-
* Adds a binding to the policy.
102-
*
103-
* @throws IllegalArgumentException if the policy already contains a binding with the same role
104-
* or if the role or any identities are null
105-
*/
106-
public final B addBinding(R role, Identity first, Identity... others) {
107-
HashSet<Identity> identities = new HashSet<>();
108-
identities.add(first);
109-
identities.addAll(Arrays.asList(others));
110-
return addBinding(role, identities);
111-
}
112-
11386
private void verifyBinding(R role, Collection<Identity> identities) {
11487
checkArgument(role != null, "The role cannot be null.");
115-
verifyIdentities(identities);
116-
}
117-
118-
private void verifyIdentities(Collection<Identity> identities) {
11988
checkArgument(identities != null, "A role cannot be assigned to a null set of identities.");
12089
checkArgument(!identities.contains(null), "Null identities are not permitted.");
12190
}
@@ -129,33 +98,33 @@ public final B removeBinding(R role) {
12998
}
13099

131100
/**
132-
* Adds one or more identities to an existing binding.
133-
*
134-
* @throws IllegalArgumentException if the policy doesn't contain a binding with the specified
135-
* role or any identities are null
101+
* Adds one or more identities to the policy under the role specified. Creates a new role
102+
* binding if the binding corresponding to the given role did not previously exist.
136103
*/
137104
public final B addIdentity(R role, Identity first, Identity... others) {
138-
checkArgument(bindings.containsKey(role),
139-
"The policy doesn't contain the role " + role.toString() + ".");
140105
List<Identity> toAdd = new LinkedList<>();
141106
toAdd.add(first);
142107
toAdd.addAll(Arrays.asList(others));
143-
verifyIdentities(toAdd);
144-
bindings.get(role).addAll(toAdd);
108+
verifyBinding(role, toAdd);
109+
Set<Identity> identities = bindings.get(role);
110+
if (identities == null) {
111+
identities = new HashSet<Identity>();
112+
bindings.put(role, identities);
113+
}
114+
identities.addAll(toAdd);
145115
return self();
146116
}
147117

148118
/**
149-
* Removes one or more identities from an existing binding.
150-
*
151-
* @throws IllegalArgumentException if the policy doesn't contain a binding with the specified
152-
* role
119+
* Removes one or more identities from an existing binding. Does nothing if the binding
120+
* associated with the provided role doesn't exist.
153121
*/
154122
public final B removeIdentity(R role, Identity first, Identity... others) {
155-
checkArgument(bindings.containsKey(role),
156-
"The policy doesn't contain the role " + role.toString() + ".");
157-
bindings.get(role).remove(first);
158-
bindings.get(role).removeAll(Arrays.asList(others));
123+
Set<Identity> identities = bindings.get(role);
124+
if (identities != null) {
125+
identities.remove(first);
126+
identities.removeAll(Arrays.asList(others));
127+
}
159128
return self();
160129
}
161130

‎branches/tswast-patch-1/gcloud-java-core/src/test/java/com/google/gcloud/IamPolicyTest.java‎

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828

2929
import org.junit.Test;
3030

31+
import java.util.HashMap;
3132
import java.util.Map;
3233
import java.util.Set;
3334

@@ -46,8 +47,8 @@ public class IamPolicyTest {
4647
"editor",
4748
ImmutableSet.of(ALL_AUTH_USERS, GROUP, DOMAIN));
4849
private static final PolicyImpl SIMPLE_POLICY = PolicyImpl.builder()
49-
.addBinding("viewer", ImmutableSet.of(USER, SERVICE_ACCOUNT, ALL_USERS))
50-
.addBinding("editor", ImmutableSet.of(ALL_AUTH_USERS, GROUP, DOMAIN))
50+
.addIdentity("viewer", USER, SERVICE_ACCOUNT, ALL_USERS)
51+
.addIdentity("editor", ALL_AUTH_USERS, GROUP, DOMAIN)
5152
.build();
5253
private static final PolicyImpl FULL_POLICY =
5354
new PolicyImpl.Builder(SIMPLE_POLICY.bindings(), "etag", 1).build();
@@ -105,22 +106,43 @@ public void testBuilder() {
105106
policy.bindings());
106107
assertNull(policy.etag());
107108
assertNull(policy.version());
108-
policy = PolicyImpl.builder().addBinding("owner", USER, SERVICE_ACCOUNT).build();
109+
policy = PolicyImpl.builder()
110+
.removeIdentity("viewer", USER)
111+
.addIdentity("owner", USER, SERVICE_ACCOUNT)
112+
.build();
109113
assertEquals(
110114
ImmutableMap.of("owner", ImmutableSet.of(USER, SERVICE_ACCOUNT)), policy.bindings());
111115
assertNull(policy.etag());
112116
assertNull(policy.version());
117+
}
118+
119+
@Test
120+
public void testIllegalPolicies() {
121+
try {
122+
PolicyImpl.builder().addIdentity(null, USER);
123+
fail("Null role should cause exception.");
124+
} catch (IllegalArgumentException ex) {
125+
assertEquals("The role cannot be null.", ex.getMessage());
126+
}
127+
try {
128+
PolicyImpl.builder().addIdentity("viewer", null, USER);
129+
fail("Null identity should cause exception.");
130+
} catch (IllegalArgumentException ex) {
131+
assertEquals("Null identities are not permitted.", ex.getMessage());
132+
}
113133
try {
114-
SIMPLE_POLICY.toBuilder().addBinding("viewer", USER);
115-
fail("Should have failed due to duplicate role.");
116-
} catch (IllegalArgumentException e) {
117-
assertEquals("The policy already contains a binding with the role viewer.", e.getMessage());
134+
PolicyImpl.builder().bindings(null);
135+
fail("Null bindings map should cause exception.");
136+
} catch (IllegalArgumentException ex) {
137+
assertEquals("The provided map of bindings cannot be null.", ex.getMessage());
118138
}
119139
try {
120-
SIMPLE_POLICY.toBuilder().addBinding("editor", ImmutableSet.of(USER));
121-
fail("Should have failed due to duplicate role.");
122-
} catch (IllegalArgumentException e) {
123-
assertEquals("The policy already contains a binding with the role editor.", e.getMessage());
140+
Map<String, Set<Identity>> bindings = new HashMap<>();
141+
bindings.put("viewer", null);
142+
PolicyImpl.builder().bindings(bindings);
143+
fail("Null set of identities should cause exception.");
144+
} catch (IllegalArgumentException ex) {
145+
assertEquals("A role cannot be assigned to a null set of identities.", ex.getMessage());
124146
}
125147
}
126148

‎branches/tswast-patch-1/gcloud-java-examples/src/main/java/com/google/gcloud/examples/resourcemanager/snippets/ModifyPolicy.java‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,7 @@ public static void main(String... args) {
4949
// Add a viewer
5050
Policy.Builder modifiedPolicy = policy.toBuilder();
5151
Identity newViewer = Identity.user("<insert user's email address here>");
52-
if (policy.bindings().containsKey(Role.viewer())) {
53-
modifiedPolicy.addIdentity(Role.viewer(), newViewer);
54-
} else {
55-
modifiedPolicy.addBinding(Role.viewer(), newViewer);
56-
}
52+
modifiedPolicy.addIdentity(Role.viewer(), newViewer);
5753

5854
// Write policy
5955
Policy updatedPolicy = project.replacePolicy(modifiedPolicy.build());

‎branches/tswast-patch-1/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/Project.java‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -202,10 +202,10 @@ public Project replace() {
202202
}
203203

204204
/**
205-
* Returns the IAM access control policy for the specified project. Returns {@code null} if the
206-
* resource does not exist or if you do not have adequate permission to view the project or get
207-
* the policy.
205+
* Returns the IAM access control policy for this project. Returns {@code null} if the resource
206+
* does not exist or if you do not have adequate permission to view the project or get the policy.
208207
*
208+
* @return the IAM policy for the project
209209
* @throws ResourceManagerException upon failure
210210
* @see <a href=
211211
* "https://cloud.google.com/resource-manager/reference/rest/v1beta1/projects/getIamPolicy">
@@ -216,12 +216,12 @@ public Policy getPolicy() {
216216
}
217217

218218
/**
219-
* Sets the IAM access control policy for the specified project. Replaces any existing policy.
220-
* It is recommended that you use the read-modify-write pattern. See code samples and important
221-
* details of replacing policies in the documentation for {@link ResourceManager#replacePolicy}.
219+
* Sets the IAM access control policy for this project. Replaces any existing policy. It is
220+
* recommended that you use the read-modify-write pattern. See code samples and important details
221+
* of replacing policies in the documentation for {@link ResourceManager#replacePolicy}.
222222
*
223+
* @return the newly set IAM policy for this project
223224
* @throws ResourceManagerException upon failure
224-
* @see ResourceManager#replacePolicy
225225
* @see <a href=
226226
* "https://cloud.google.com/resource-manager/reference/rest/v1beta1/projects/setIamPolicy">
227227
* Resource Manager setIamPolicy</a>
@@ -237,7 +237,7 @@ public Policy replacePolicy(Policy newPolicy) {
237237
* For example, the Cloud Platform Console tests IAM permissions internally to determine which UI
238238
* should be available to the logged-in user.
239239
*
240-
* @return A list of booleans representing whether the caller has the permissions specified (in
240+
* @return a list of booleans representing whether the caller has the permissions specified (in
241241
* the order of the given permissions)
242242
* @throws ResourceManagerException upon failure
243243
* @see <a href=
@@ -255,7 +255,7 @@ List<Boolean> testPermissions(List<Permission> permissions) {
255255
* For example, the Cloud Platform Console tests IAM permissions internally to determine which UI
256256
* should be available to the logged-in user.
257257
*
258-
* @return A list of booleans representing whether the caller has the permissions specified (in
258+
* @return a list of booleans representing whether the caller has the permissions specified (in
259259
* the order of the given permissions)
260260
* @throws ResourceManagerException upon failure
261261
* @see <a href=

‎branches/tswast-patch-1/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/PolicyTest.java‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,10 @@ public class PolicyTest {
3737
private static final Identity GROUP = Identity.group("[email protected]");
3838
private static final Identity DOMAIN = Identity.domain("google.com");
3939
private static final Policy SIMPLE_POLICY = Policy.builder()
40-
.addBinding(Role.owner(), ImmutableSet.of(USER))
41-
.addBinding(Role.viewer(), ImmutableSet.of(ALL_USERS))
42-
.addBinding(Role.editor(), ImmutableSet.of(ALL_AUTH_USERS, DOMAIN))
43-
.addBinding(Role.rawRole("some-role"), ImmutableSet.of(SERVICE_ACCOUNT, GROUP))
40+
.addIdentity(Role.owner(), USER)
41+
.addIdentity(Role.viewer(), ALL_USERS)
42+
.addIdentity(Role.editor(), ALL_AUTH_USERS, DOMAIN)
43+
.addIdentity(Role.rawRole("some-role"), SERVICE_ACCOUNT, GROUP)
4444
.build();
4545
private static final Policy FULL_POLICY =
4646
new Policy.Builder(SIMPLE_POLICY.bindings(), "etag", 1).build();
@@ -68,10 +68,10 @@ public void testRoleType() {
6868
@Test
6969
public void testEquals() {
7070
Policy copy = Policy.builder()
71-
.addBinding(Role.owner(), ImmutableSet.of(USER))
72-
.addBinding(Role.viewer(), ImmutableSet.of(ALL_USERS))
73-
.addBinding(Role.editor(), ImmutableSet.of(ALL_AUTH_USERS, DOMAIN))
74-
.addBinding(Role.rawRole("some-role"), ImmutableSet.of(SERVICE_ACCOUNT, GROUP))
71+
.addIdentity(Role.owner(), USER)
72+
.addIdentity(Role.viewer(), ALL_USERS)
73+
.addIdentity(Role.editor(), ALL_AUTH_USERS, DOMAIN)
74+
.addIdentity(Role.rawRole("some-role"), SERVICE_ACCOUNT, GROUP)
7575
.build();
7676
assertEquals(SIMPLE_POLICY, copy);
7777
assertNotEquals(SIMPLE_POLICY, FULL_POLICY);

‎branches/tswast-patch-1/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/ProjectTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,8 @@ public class ProjectTest {
5858
private static final Identity SERVICE_ACCOUNT =
5959
Identity.serviceAccount("[email protected]");
6060
private static final Policy POLICY = Policy.builder()
61-
.addBinding(Role.owner(), ImmutableSet.of(USER))
62-
.addBinding(Role.editor(), ImmutableSet.of(SERVICE_ACCOUNT))
61+
.addIdentity(Role.owner(), USER)
62+
.addIdentity(Role.editor(), SERVICE_ACCOUNT)
6363
.build();
6464

6565
private ResourceManager serviceMockReturnsOptions = createStrictMock(ResourceManager.class);

‎branches/tswast-patch-1/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/ResourceManagerImplTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,8 @@ public class ResourceManagerImplTest {
7272
.build();
7373
private static final Map<ResourceManagerRpc.Option, ?> EMPTY_RPC_OPTIONS = ImmutableMap.of();
7474
private static final Policy POLICY = Policy.builder()
75-
.addBinding(Role.owner(), Identity.user("[email protected]"))
76-
.addBinding(Role.editor(), Identity.serviceAccount("[email protected]"))
75+
.addIdentity(Role.owner(), Identity.user("[email protected]"))
76+
.addIdentity(Role.editor(), Identity.serviceAccount("[email protected]"))
7777
.build();
7878

7979
@Rule

‎branches/tswast-patch-1/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/SerializationTest.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
package com.google.gcloud.resourcemanager;
1818

1919
import com.google.common.collect.ImmutableMap;
20-
import com.google.common.collect.ImmutableSet;
2120
import com.google.gcloud.BaseSerializationTest;
2221
import com.google.gcloud.Identity;
2322
import com.google.gcloud.PageImpl;
@@ -46,9 +45,8 @@ public class SerializationTest extends BaseSerializationTest {
4645
ResourceManager.ProjectGetOption.fields(ResourceManager.ProjectField.NAME);
4746
private static final ResourceManager.ProjectListOption PROJECT_LIST_OPTION =
4847
ResourceManager.ProjectListOption.filter("name:*");
49-
private static final Policy POLICY = Policy.builder()
50-
.addBinding(Policy.Role.viewer(), ImmutableSet.of(Identity.user("[email protected]")))
51-
.build();
48+
private static final Policy POLICY =
49+
Policy.builder().addIdentity(Policy.Role.viewer(), Identity.user("[email protected]")).build();
5250
private static final ResourceManagerException RESOURCE_MANAGER_EXCEPTION =
5351
new ResourceManagerException(42, "message");
5452

0 commit comments

Comments
 (0)