Skip to content

Commit

Permalink
Take care of minor changes
Browse files Browse the repository at this point in the history
  • Loading branch information
Ajay Kannan committed Mar 21, 2016
1 parent c05e738 commit 7bbe67f
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 24 deletions.
44 changes: 25 additions & 19 deletions gcloud-java-core/src/main/java/com/google/gcloud/IamPolicy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,16 @@
package com.google.gcloud;

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;

import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;

import java.io.Serializable;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.LinkedHashSet;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
Expand Down Expand Up @@ -69,12 +68,16 @@ protected Builder() {}
/**
* Replaces the builder's map of bindings with the given map of bindings.
*
* @throws IllegalArgumentException if the provided map is null or contain any null values
* @throws NullPointerException if the given map is null or contains any null keys or values
* @throws IllegalArgumentException if any identities in the given map are null
*/
public final B bindings(Map<R, Set<Identity>> bindings) {
checkArgument(bindings != null, "The provided map of bindings cannot be null.");
checkNotNull(bindings, "The provided map of bindings cannot be null.");
for (Map.Entry<R, Set<Identity>> binding : bindings.entrySet()) {
verifyBinding(binding.getKey(), binding.getValue());
checkNotNull(binding.getKey(), "The role cannot be null.");
Set<Identity> identities = binding.getValue();
checkNotNull(identities, "A role cannot be assigned to a null set of identities.");
checkArgument(!identities.contains(null), "Null identities are not permitted.");
}
this.bindings.clear();
for (Map.Entry<R, Set<Identity>> binding : bindings.entrySet()) {
Expand All @@ -83,30 +86,30 @@ public final B bindings(Map<R, Set<Identity>> bindings) {
return self();
}

private void verifyBinding(R role, Collection<Identity> identities) {
checkArgument(role != null, "The role cannot be null.");
checkArgument(identities != null, "A role cannot be assigned to a null set of identities.");
checkArgument(!identities.contains(null), "Null identities are not permitted.");
}

/**
* Removes the binding associated with the specified role.
* Removes the role (and all identities associated with that role) from the policy.
*/
public final B removeBinding(R role) {
public final B removeRole(R role) {
bindings.remove(role);
return self();
}

/**
* Adds one or more identities to the policy under the role specified. Creates a new role
* binding if the binding corresponding to the given role did not previously exist.
* Adds one or more identities to the policy under the role specified.
*
* @throws NullPointerException if the role or any of the identities is null.
*/
public final B addIdentity(R role, Identity first, Identity... others) {
List<Identity> toAdd = new LinkedList<>();
String nullIdentityMessage = "Null identities are not permitted.";
checkNotNull(first, nullIdentityMessage);
checkNotNull(others, nullIdentityMessage);
for (Identity identity : others) {
checkNotNull(identity, nullIdentityMessage);
}
Set<Identity> toAdd = new LinkedHashSet<>();
toAdd.add(first);
toAdd.addAll(Arrays.asList(others));
verifyBinding(role, toAdd);
Set<Identity> identities = bindings.get(role);
Set<Identity> identities = bindings.get(checkNotNull(role, "The role cannot be null."));
if (identities == null) {
identities = new HashSet<Identity>();
bindings.put(role, identities);
Expand All @@ -125,6 +128,9 @@ public final B removeIdentity(R role, Identity first, Identity... others) {
identities.remove(first);
identities.removeAll(Arrays.asList(others));
}
if (identities != null && identities.isEmpty()) {
bindings.remove(role);
}
return self();
}

Expand Down
29 changes: 24 additions & 5 deletions gcloud-java-core/src/test/java/com/google/gcloud/IamPolicyTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.junit.Test;

import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;

Expand Down Expand Up @@ -94,7 +95,7 @@ public void testBuilder() {
assertEquals(editorBinding, policy.bindings());
assertEquals("etag", policy.etag());
assertEquals(1, policy.version().intValue());
policy = SIMPLE_POLICY.toBuilder().removeBinding("editor").build();
policy = SIMPLE_POLICY.toBuilder().removeRole("editor").build();
assertEquals(ImmutableMap.of("viewer", BINDINGS.get("viewer")), policy.bindings());
assertNull(policy.etag());
assertNull(policy.version());
Expand All @@ -109,6 +110,8 @@ public void testBuilder() {
policy = PolicyImpl.builder()
.removeIdentity("viewer", USER)
.addIdentity("owner", USER, SERVICE_ACCOUNT)
.addIdentity("editor", GROUP)
.removeIdentity("editor", GROUP)
.build();
assertEquals(
ImmutableMap.of("owner", ImmutableSet.of(USER, SERVICE_ACCOUNT)), policy.bindings());
Expand All @@ -121,29 +124,45 @@ public void testIllegalPolicies() {
try {
PolicyImpl.builder().addIdentity(null, USER);
fail("Null role should cause exception.");
} catch (IllegalArgumentException ex) {
} catch (NullPointerException ex) {
assertEquals("The role cannot be null.", ex.getMessage());
}
try {
PolicyImpl.builder().addIdentity("viewer", null, USER);
fail("Null identity should cause exception.");
} catch (IllegalArgumentException ex) {
} catch (NullPointerException ex) {
assertEquals("Null identities are not permitted.", ex.getMessage());
}
try {
PolicyImpl.builder().addIdentity("viewer", USER, (Identity[]) null);
fail("Null identity should cause exception.");
} catch (NullPointerException ex) {
assertEquals("Null identities are not permitted.", ex.getMessage());
}
try {
PolicyImpl.builder().bindings(null);
fail("Null bindings map should cause exception.");
} catch (IllegalArgumentException ex) {
} catch (NullPointerException ex) {
assertEquals("The provided map of bindings cannot be null.", ex.getMessage());
}
try {
Map<String, Set<Identity>> bindings = new HashMap<>();
bindings.put("viewer", null);
PolicyImpl.builder().bindings(bindings);
fail("Null set of identities should cause exception.");
} catch (IllegalArgumentException ex) {
} catch (NullPointerException ex) {
assertEquals("A role cannot be assigned to a null set of identities.", ex.getMessage());
}
try {
Map<String, Set<Identity>> bindings = new HashMap<>();
Set<Identity> identities = new HashSet<>();
identities.add(null);
bindings.put("viewer", identities);
PolicyImpl.builder().bindings(bindings);
fail("Null identity should cause exception.");
} catch (IllegalArgumentException ex) {
assertEquals("Null identities are not permitted.", ex.getMessage());
}
}

@Test
Expand Down

0 comments on commit 7bbe67f

Please sign in to comment.