From 67e52cef4c9901d9059b1f9bec6769ca4fa49f11 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Tue, 17 Dec 2019 12:15:24 -0800 Subject: [PATCH 01/31] Base implementation --- .../main/java/com/google/cloud/Binding.java | 154 +++++++++++++ .../main/java/com/google/cloud/Condition.java | 118 ++++++++++ .../main/java/com/google/cloud/Policy.java | 211 ++++++++++++------ 3 files changed, 412 insertions(+), 71 deletions(-) create mode 100644 google-cloud-core/src/main/java/com/google/cloud/Binding.java create mode 100644 google-cloud-core/src/main/java/com/google/cloud/Condition.java diff --git a/google-cloud-core/src/main/java/com/google/cloud/Binding.java b/google-cloud-core/src/main/java/com/google/cloud/Binding.java new file mode 100644 index 0000000000..19f026a264 --- /dev/null +++ b/google-cloud-core/src/main/java/com/google/cloud/Binding.java @@ -0,0 +1,154 @@ +/* + * Copyright 2019 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.cloud; + +import static com.google.common.base.Preconditions.checkNotNull; + +import com.google.api.core.InternalApi; +import com.google.common.base.MoreObjects; +import java.util.*; + +public class Binding { + private Role role; + private Set identities; + private Condition condition; + + public static class Builder { + private Role role; + private Set identities; + private Condition condition; + + @InternalApi("This class should only be extended within google-cloud-java") + protected Builder() {} + + @InternalApi("This class should only be extended within google-cloud-java") + protected Builder(Binding binding) { + setRole(binding.role); + setIdentities(binding.identities); + setCondition(binding.condition); + } + + public final Binding.Builder setRole(Role role) { + this.role = role; + return this; + } + + public final Binding.Builder setIdentities(Set identities) { + this.identities = identities; + return this; + } + + public final Binding.Builder setCondition(Condition condition) { + this.condition = condition; + return this; + } + + /** + * Adds one or more identities to the binding + * + * @throws NullPointerException if any of the identities is null. + */ + public final Builder addIdentity(Identity first, Identity... others) { + String nullIdentityMessage = "Null identities are not permitted."; + checkNotNull(first, nullIdentityMessage); + checkNotNull(others, nullIdentityMessage); + for (Identity identity : others) { + checkNotNull(identity, nullIdentityMessage); + } + Set toAdd = new LinkedHashSet<>(); + toAdd.add(first); + toAdd.addAll(Arrays.asList(others)); + if (identities == null) { + identities = new HashSet<>(); + } + identities.addAll(toAdd); + return this; + } + + /** + * Removes one or more identities from an existing binding. Does nothing if the binding + * associated with the provided role doesn't exist. + */ + public final Builder removeIdentity(Identity first, Identity... others) { + if (identities != null) { + identities.remove(first); + identities.removeAll(Arrays.asList(others)); + } + return this; + } + + /** Creates a {@code Policy} object. */ + public final Binding build() { + return new Binding(this); + } + } + + private Binding(Binding.Builder builder) { + this.role = builder.role; + this.identities = builder.identities; + this.condition = builder.condition; + } + + public Binding.Builder toBuilder() { + return new Binding.Builder(this); + } + + public Role getRole() { + return this.role; + } + + public Set getIdentities() { + return this.identities; + } + + public Condition getCondition() { + return this.condition; + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("role", role) + .add("identities", identities) + .add("condition", condition) + .toString(); + } + + @Override + public int hashCode() { + return Objects.hash(getClass(), role, identities, condition); + } + + @Override + public boolean equals(Object obj) { + if (obj == this) { + return true; + } + if (!(obj instanceof Binding)) { + return false; + } + Binding other = (Binding) obj; + return Objects.equals(role, other.getRole()) + && Objects.equals(identities, other.getIdentities()) + && Objects.equals(condition, other.getCondition()); + } + + /** Returns a builder for {@code Policy} objects. */ + public static Binding.Builder newBuilder() { + return new Binding.Builder(); + } +} diff --git a/google-cloud-core/src/main/java/com/google/cloud/Condition.java b/google-cloud-core/src/main/java/com/google/cloud/Condition.java new file mode 100644 index 0000000000..566fef142c --- /dev/null +++ b/google-cloud-core/src/main/java/com/google/cloud/Condition.java @@ -0,0 +1,118 @@ +/* + * Copyright 2019 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.cloud; + +import com.google.api.core.InternalApi; +import com.google.common.base.MoreObjects; +import java.util.Objects; + +public class Condition { + private String title; + private String description; + private String expression; + + public static class Builder { + private String title; + private String description; + private String expression; + + @InternalApi("This class should only be extended within google-cloud-java") + protected Builder() {} + + @InternalApi("This class should only be extended within google-cloud-java") + protected Builder(Condition condition) { + this.title = condition.title; + this.description = condition.description; + this.expression = condition.expression; + } + + public final Condition.Builder setTitle(String title) { + this.title = title; + return this; + } + + public final Condition.Builder setDescription(String description) { + this.description = description; + return this; + } + + public final Condition.Builder setExpression(String expression) { + this.expression = expression; + return this; + } + + /** Creates a {@code Condition} object. */ + public final Condition build() { + return new Condition(this); + } + } + + private Condition(Condition.Builder builder) { + this.title = builder.title; + this.description = builder.description; + this.expression = builder.expression; + } + + public Condition.Builder toBuilder() { + return new Condition.Builder(this); + } + + public String getTitle() { + return title; + } + + public String getDescription() { + return description; + } + + public String getExpression() { + return expression; + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("title", title) + .add("description", description) + .add("expression", expression) + .toString(); + } + + @Override + public int hashCode() { + return Objects.hash(getClass(), title, description, expression); + } + + @Override + public boolean equals(Object obj) { + if (obj == this) { + return true; + } + if (!(obj instanceof Condition)) { + return false; + } + Condition other = (Condition) obj; + return Objects.equals(title, other.getTitle()) + && Objects.equals(description, other.getDescription()) + && Objects.equals(expression, other.getExpression()); + } + + /** Returns a builder for {@code Policy} objects. */ + public static Condition.Builder newBuilder() { + return new Condition.Builder(); + } +} diff --git a/google-cloud-core/src/main/java/com/google/cloud/Policy.java b/google-cloud-core/src/main/java/com/google/cloud/Policy.java index 84067ec87e..658a4ac43d 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Policy.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Policy.java @@ -23,6 +23,7 @@ import com.google.api.core.InternalApi; import com.google.common.base.Function; import com.google.common.base.MoreObjects; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Lists; @@ -30,10 +31,7 @@ import com.google.protobuf.ByteString; import java.io.Serializable; import java.util.ArrayList; -import java.util.Arrays; -import java.util.HashMap; import java.util.HashSet; -import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -51,8 +49,7 @@ public final class Policy implements Serializable { private static final long serialVersionUID = -3348914530232544290L; - - private final Map> bindings; + private final List bindingsV3; private final String etag; private final int version; @@ -85,46 +82,51 @@ public static class DefaultMarshaller extends Marshaller> bindings = new HashMap<>(); - for (com.google.iam.v1.Binding bindingPb : policyPb.getBindingsList()) { - bindings.put( - Role.of(bindingPb.getRole()), - ImmutableSet.copyOf( - Lists.transform( - bindingPb.getMembersList(), - new Function() { - @Override - public Identity apply(String s) { - return IDENTITY_VALUE_OF_FUNCTION.apply(s); - } - }))); - } - return newBuilder() - .setBindings(bindings) - .setEtag( - policyPb.getEtag().isEmpty() - ? null - : BaseEncoding.base64().encode(policyPb.getEtag().toByteArray())) - .setVersion(policyPb.getVersion()) - .build(); + List bindingsV3 = new ArrayList(); + for (com.google.iam.v1.Binding bindingPb : policyPb.getBindingsList()) { + bindingsV3.add( + Binding.newBuilder().setRole(Role.of(bindingPb.getRole())) + .setIdentities(ImmutableSet.copyOf( + Lists.transform( + bindingPb.getMembersList(), + new Function() { + @Override + public Identity apply(String s) { + return IDENTITY_VALUE_OF_FUNCTION.apply(s); + } + }))) + .setCondition(null) + .build()); + // TODO(frankyn): Add support for bindingBuilder.setCondition after com.google.iam.v1 is regenerated. + + } + return newBuilder() + .setBindingsV3(bindingsV3) + .setEtag( + policyPb.getEtag().isEmpty() + ? null + : BaseEncoding.base64().encode(policyPb.getEtag().toByteArray())) + .setVersion(policyPb.getVersion()) + .build(); } @Override protected com.google.iam.v1.Policy toPb(Policy policy) { com.google.iam.v1.Policy.Builder policyBuilder = com.google.iam.v1.Policy.newBuilder(); List bindingPbList = new LinkedList<>(); - for (Map.Entry> binding : policy.getBindings().entrySet()) { + for (Binding binding : policy.getBindingsV3()) { com.google.iam.v1.Binding.Builder bindingBuilder = com.google.iam.v1.Binding.newBuilder(); - bindingBuilder.setRole(binding.getKey().getValue()); + bindingBuilder.setRole(binding.getRole().getValue()); bindingBuilder.addAllMembers( - Lists.transform( - new ArrayList<>(binding.getValue()), - new Function() { - @Override - public String apply(Identity identity) { - return IDENTITY_STR_VALUE_FUNCTION.apply(identity); - } - })); + Lists.transform( + new ArrayList<>(binding.getIdentities()), + new Function() { + @Override + public String apply(Identity identity) { + return IDENTITY_STR_VALUE_FUNCTION.apply(identity); + } + })); + // TODO(frankyn): Add support for bindingBuilder.setCondition after com.google.iam.v1 is regenerated. bindingPbList.add(bindingBuilder.build()); } policyBuilder.addAllBindings(bindingPbList); @@ -138,8 +140,7 @@ public String apply(Identity identity) { /** A builder for {@code Policy} objects. */ public static class Builder { - - private final Map> bindings = new HashMap<>(); + private final List bindingsV3 = new ArrayList(); private String etag; private int version; @@ -148,7 +149,7 @@ protected Builder() {} @InternalApi("This class should only be extended within google-cloud-java") protected Builder(Policy policy) { - setBindings(policy.bindings); + setBindingsV3(policy.bindingsV3); setEtag(policy.etag); setVersion(policy.version); } @@ -158,6 +159,7 @@ protected Builder(Policy policy) { * * @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 + * @throws IllegalArgumentException if policy version is equal to 3. */ public final Builder setBindings(Map> bindings) { checkNotNull(bindings, "The provided map of bindings cannot be null."); @@ -167,16 +169,52 @@ public final Builder setBindings(Map> bindings) { 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(); + // convert into v3 format + this.bindingsV3.clear(); for (Map.Entry> binding : bindings.entrySet()) { - this.bindings.put(binding.getKey(), new HashSet<>(binding.getValue())); + Binding.Builder bindingBuilder = Binding.newBuilder(); + bindingBuilder.setIdentities(new HashSet<>(binding.getValue())); + bindingBuilder.setRole(binding.getKey()); + this.bindingsV3.add(bindingBuilder.build()); } return this; } + /** + * Replaces the builder's map of bindings with the given map of bindingsV3. + * + * @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 Builder setBindingsV3(List bindings) { + for (Binding binding : bindings) { + checkNotNull(binding.getRole().getValue(), "The role cannot be null."); + Set identities = binding.getIdentities(); + checkNotNull(identities, "A role cannot be assigned to a null set of identities."); + checkArgument(!identities.contains(null), "Null identities are not permitted."); + } + // Set version to 3. + this.bindingsV3.clear(); + for (Binding binding : bindings) { + Binding.Builder bindingBuilder = Binding.newBuilder(); + bindingBuilder.setIdentities(new HashSet<>(binding.getIdentities())); + bindingBuilder.setRole(binding.getRole()); + bindingBuilder.setCondition(binding.getCondition()); + this.bindingsV3.add(bindingBuilder.build()); + } + return this; + } + + /** Removes the role (and all identities associated with that role) from the policy. */ public final Builder removeRole(Role role) { - bindings.remove(role); + checkArgument(this.version != 3, "removeRole is not supported with version 3 policies."); + for (Binding binding : bindingsV3) { + if (binding.getRole().equals(role)) { + bindingsV3.remove(binding); + } + } + return this; } @@ -184,38 +222,49 @@ public final Builder removeRole(Role role) { * Adds one or more identities to the policy under the role specified. * * @throws NullPointerException if the role or any of the identities is null. + * @throws IllegalArgumentException if policy version is equal to 3. */ public final Builder addIdentity(Role role, Identity first, Identity... others) { + checkArgument(this.version != 3, "removeRole is not supported with version 3 policies."); String nullIdentityMessage = "Null identities are not permitted."; checkNotNull(first, nullIdentityMessage); checkNotNull(others, nullIdentityMessage); - for (Identity identity : others) { - checkNotNull(identity, nullIdentityMessage); + for (Binding binding : bindingsV3) { + if (binding.getRole().equals(checkNotNull(role, "The role cannot be null."))) { + Binding.Builder bindingBuilder = binding.toBuilder(); + bindingBuilder.addIdentity(first, others); + bindingsV3.remove(binding); + bindingsV3.add(bindingBuilder.build()); + return this; + } } - Set toAdd = new LinkedHashSet<>(); - toAdd.add(first); - toAdd.addAll(Arrays.asList(others)); - Set identities = bindings.get(checkNotNull(role, "The role cannot be null.")); - if (identities == null) { - identities = new HashSet<>(); - bindings.put(role, identities); - } - identities.addAll(toAdd); + + Binding.Builder bindingBuilder = Binding.newBuilder(); + bindingBuilder.setRole(role); + bindingBuilder.addIdentity(first, others); + bindingsV3.add(bindingBuilder.build()); + return this; } /** * Removes one or more identities from an existing binding. Does nothing if the binding * associated with the provided role doesn't exist. + * @throws IllegalArgumentException if policy version is equal to 3. */ public final Builder removeIdentity(Role role, Identity first, Identity... others) { - Set identities = bindings.get(role); - if (identities != null) { - identities.remove(first); - identities.removeAll(Arrays.asList(others)); - } - if (identities != null && identities.isEmpty()) { - bindings.remove(role); + checkArgument(this.version != 3, "removeRole is not supported with version 3 policies."); + for (Binding binding : bindingsV3) { + if (binding.getRole().equals(checkNotNull(role, "The role cannot be null."))) { + Binding.Builder bindingBuilder = binding.toBuilder(); + bindingBuilder.removeIdentity(first, others); + bindingsV3.remove(binding); + Binding builtBinding = bindingBuilder.build(); + if (builtBinding.getIdentities() != null && !builtBinding.getIdentities().isEmpty()) { + bindingsV3.add(builtBinding); + } + break; + } } return this; } @@ -240,7 +289,7 @@ public final Builder setEtag(String etag) { * Sets the version of the policy. The default version is 0, meaning only the "owner", "editor", * and "viewer" roles are permitted. If the version is 1, you may also use other roles. */ - protected final Builder setVersion(int version) { + public final Builder setVersion(int version) { this.version = version; return this; } @@ -252,11 +301,15 @@ public final Policy build() { } private Policy(Builder builder) { - ImmutableMap.Builder> bindingsBuilder = ImmutableMap.builder(); - for (Map.Entry> binding : builder.bindings.entrySet()) { - bindingsBuilder.put(binding.getKey(), ImmutableSet.copyOf(binding.getValue())); + ImmutableList.Builder bindingsV3Builder = ImmutableList.builder(); + for (Binding binding : builder.bindingsV3) { + Binding.Builder bindingBuilder = Binding.newBuilder(); + bindingBuilder.setRole(binding.getRole()) + .setIdentities(ImmutableSet.copyOf(binding.getIdentities())) + .setCondition(binding.getCondition()); + bindingsV3Builder.add(bindingBuilder.build()); } - this.bindings = bindingsBuilder.build(); + this.bindingsV3 = bindingsV3Builder.build(); this.etag = builder.etag; this.version = builder.version; } @@ -266,9 +319,25 @@ public Builder toBuilder() { return new Builder(this); } - /** Returns the map of bindings that comprises the policy. */ + /** Returns the map of bindings that comprises the policy. + * + * @throws IllegalArgumentException if policy version is equal to 3. + * */ + public Map> getBindings() { - return bindings; + checkArgument(this.version != 3, "removeRole is not supported with version 3 policies."); + // Convert to V1 IAM Policy if version is not 3. + ImmutableMap.Builder> bindingsV1Builder = ImmutableMap.builder(); + for (Binding binding : bindingsV3) { + bindingsV1Builder.put(binding.getRole(), binding.getIdentities()); + } + return bindingsV1Builder.build(); + } + + /** Returns the map of bindings that comprises the policy for version 3. */ + + public List getBindingsV3() { + return bindingsV3; } /** @@ -297,7 +366,7 @@ public int getVersion() { @Override public String toString() { return MoreObjects.toStringHelper(this) - .add("bindings", bindings) + .add("bindings", bindingsV3) .add("etag", etag) .add("version", version) .toString(); @@ -305,7 +374,7 @@ public String toString() { @Override public int hashCode() { - return Objects.hash(getClass(), bindings, etag, version); + return Objects.hash(getClass(), bindingsV3, etag, version); } @Override @@ -317,7 +386,7 @@ public boolean equals(Object obj) { return false; } Policy other = (Policy) obj; - return Objects.equals(bindings, other.getBindings()) + return Objects.equals(bindingsV3, other.getBindings()) && Objects.equals(etag, other.getEtag()) && Objects.equals(version, other.getVersion()); } From 84fcfea4d040bc14939de7e4bdf070f45183ec71 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Thu, 19 Dec 2019 22:55:26 -0800 Subject: [PATCH 02/31] Update with unit tests --- .../main/java/com/google/cloud/Binding.java | 83 +++--- .../main/java/com/google/cloud/Policy.java | 250 +++++++++-------- .../java/com/google/cloud/PolicyTest.java | 2 +- .../java/com/google/cloud/PolicyV3Test.java | 259 ++++++++++++++++++ 4 files changed, 439 insertions(+), 155 deletions(-) create mode 100644 google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java diff --git a/google-cloud-core/src/main/java/com/google/cloud/Binding.java b/google-cloud-core/src/main/java/com/google/cloud/Binding.java index 19f026a264..e7ab048ed3 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Binding.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Binding.java @@ -16,20 +16,23 @@ package com.google.cloud; +import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import com.google.api.core.InternalApi; import com.google.common.base.MoreObjects; +import com.google.common.collect.ImmutableList; + import java.util.*; public class Binding { - private Role role; - private Set identities; + private String role; + private List members; private Condition condition; public static class Builder { - private Role role; - private Set identities; + private List members = new ArrayList(); + private String role; private Condition condition; @InternalApi("This class should only be extended within google-cloud-java") @@ -38,56 +41,54 @@ protected Builder() {} @InternalApi("This class should only be extended within google-cloud-java") protected Builder(Binding binding) { setRole(binding.role); - setIdentities(binding.identities); + setMembers(binding.members); setCondition(binding.condition); } - public final Binding.Builder setRole(Role role) { + public final Binding.Builder setRole(String role) { + String nullIdentityMessage = "The role cannot be null."; + checkNotNull(role, nullIdentityMessage); this.role = role; return this; } - public final Binding.Builder setIdentities(Set identities) { - this.identities = identities; + public final Binding.Builder setMembers(List members) { + String nullIdentityMessage = "Null members are not permitted."; + checkNotNull(members, nullIdentityMessage); + this.members.clear(); + for (String member : members) { + // Check member not null + this.members.add(member); + } return this; } - public final Binding.Builder setCondition(Condition condition) { - this.condition = condition; + public final Binding.Builder removeMembers(String first, String... others) { + String nullIdentityMessage = "Null members are not permitted."; + checkNotNull(first, nullIdentityMessage); + checkNotNull(others, nullIdentityMessage); + this.members.remove(first); + for (String member : others) { + if (member != null) + this.members.remove(member); + } return this; } - /** - * Adds one or more identities to the binding - * - * @throws NullPointerException if any of the identities is null. - */ - public final Builder addIdentity(Identity first, Identity... others) { + public final Binding.Builder addMembers(String first, String... others) { String nullIdentityMessage = "Null identities are not permitted."; checkNotNull(first, nullIdentityMessage); checkNotNull(others, nullIdentityMessage); - for (Identity identity : others) { - checkNotNull(identity, nullIdentityMessage); - } - Set toAdd = new LinkedHashSet<>(); - toAdd.add(first); - toAdd.addAll(Arrays.asList(others)); - if (identities == null) { - identities = new HashSet<>(); + this.members.add(first); + for (String member : others) { + if (member != null) + this.members.add(member); } - identities.addAll(toAdd); return this; } - /** - * Removes one or more identities from an existing binding. Does nothing if the binding - * associated with the provided role doesn't exist. - */ - public final Builder removeIdentity(Identity first, Identity... others) { - if (identities != null) { - identities.remove(first); - identities.removeAll(Arrays.asList(others)); - } + public final Binding.Builder setCondition(Condition condition) { + this.condition = condition; return this; } @@ -99,7 +100,7 @@ public final Binding build() { private Binding(Binding.Builder builder) { this.role = builder.role; - this.identities = builder.identities; + this.members = ImmutableList.copyOf(builder.members); this.condition = builder.condition; } @@ -107,12 +108,12 @@ public Binding.Builder toBuilder() { return new Binding.Builder(this); } - public Role getRole() { + public String getRole() { return this.role; } - public Set getIdentities() { - return this.identities; + public List getMembers() { + return this.members; } public Condition getCondition() { @@ -123,14 +124,14 @@ public Condition getCondition() { public String toString() { return MoreObjects.toStringHelper(this) .add("role", role) - .add("identities", identities) + .add("members", members) .add("condition", condition) .toString(); } @Override public int hashCode() { - return Objects.hash(getClass(), role, identities, condition); + return Objects.hash(getClass(), role, members, condition); } @Override @@ -143,7 +144,7 @@ public boolean equals(Object obj) { } Binding other = (Binding) obj; return Objects.equals(role, other.getRole()) - && Objects.equals(identities, other.getIdentities()) + && Objects.equals(members, other.getMembers()) && Objects.equals(condition, other.getCondition()); } diff --git a/google-cloud-core/src/main/java/com/google/cloud/Policy.java b/google-cloud-core/src/main/java/com/google/cloud/Policy.java index 658a4ac43d..7be4f6f520 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Policy.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Policy.java @@ -29,6 +29,8 @@ import com.google.common.collect.Lists; import com.google.common.io.BaseEncoding; import com.google.protobuf.ByteString; +import com.google.type.Expr; + import java.io.Serializable; import java.util.ArrayList; import java.util.HashSet; @@ -49,29 +51,31 @@ public final class Policy implements Serializable { private static final long serialVersionUID = -3348914530232544290L; - private final List bindingsV3; + private final List bindingsList; private final String etag; private final int version; + /* + * Return false if Policy is version 3 OR bindings has a conditional binding. + * Return true if Policy is version 1 AND bindings does not have a conditional binding. + */ + private static boolean checkVersion(int version, List bindingsList) { + for (Binding binding : bindingsList) { + if (binding.getCondition() != null) { + return false; + } + } + if (version > 1) { + return false; + } + return true; + } + public abstract static class Marshaller { @InternalApi("This class should only be extended within google-cloud-java") protected Marshaller() {} - protected static final ApiFunction IDENTITY_VALUE_OF_FUNCTION = - new ApiFunction() { - @Override - public Identity apply(String identityPb) { - return Identity.valueOf(identityPb); - } - }; - protected static final ApiFunction IDENTITY_STR_VALUE_FUNCTION = - new ApiFunction() { - @Override - public String apply(Identity identity) { - return identity.strValue(); - } - }; protected abstract Policy fromPb(T policyPb); @@ -82,26 +86,22 @@ public static class DefaultMarshaller extends Marshaller bindingsV3 = new ArrayList(); + List bindingsList = new ArrayList(); for (com.google.iam.v1.Binding bindingPb : policyPb.getBindingsList()) { - bindingsV3.add( - Binding.newBuilder().setRole(Role.of(bindingPb.getRole())) - .setIdentities(ImmutableSet.copyOf( - Lists.transform( - bindingPb.getMembersList(), - new Function() { - @Override - public Identity apply(String s) { - return IDENTITY_VALUE_OF_FUNCTION.apply(s); - } - }))) - .setCondition(null) - .build()); - // TODO(frankyn): Add support for bindingBuilder.setCondition after com.google.iam.v1 is regenerated. - + Binding.Builder convertedBinding = Binding.newBuilder().setRole(bindingPb.getRole()) + .setMembers(ImmutableList.copyOf(bindingPb.getMembersList())); + if (bindingPb.hasCondition()) { + Expr expr = bindingPb.getCondition(); + convertedBinding.setCondition(Condition.newBuilder() + .setTitle(expr.getTitle()) + .setDescription(expr.getDescription()) + .setExpression(expr.getExpression()) + .build()); + } + bindingsList.add(convertedBinding.build()); } return newBuilder() - .setBindingsV3(bindingsV3) + .setBindings(bindingsList) .setEtag( policyPb.getEtag().isEmpty() ? null @@ -114,19 +114,18 @@ public Identity apply(String s) { protected com.google.iam.v1.Policy toPb(Policy policy) { com.google.iam.v1.Policy.Builder policyBuilder = com.google.iam.v1.Policy.newBuilder(); List bindingPbList = new LinkedList<>(); - for (Binding binding : policy.getBindingsV3()) { + for (Binding binding : policy.getBindingsList()) { com.google.iam.v1.Binding.Builder bindingBuilder = com.google.iam.v1.Binding.newBuilder(); - bindingBuilder.setRole(binding.getRole().getValue()); - bindingBuilder.addAllMembers( - Lists.transform( - new ArrayList<>(binding.getIdentities()), - new Function() { - @Override - public String apply(Identity identity) { - return IDENTITY_STR_VALUE_FUNCTION.apply(identity); - } - })); - // TODO(frankyn): Add support for bindingBuilder.setCondition after com.google.iam.v1 is regenerated. + bindingBuilder.setRole(binding.getRole()); + bindingBuilder.addAllMembers(ImmutableList.copyOf(binding.getMembers())); + if (binding.getCondition() != null) { + Condition condition = binding.getCondition(); + bindingBuilder.setCondition(Expr.newBuilder() + .setTitle(condition.getTitle()) + .setDescription(condition.getDescription()) + .setExpression(condition.getExpression()) + .build()); + } bindingPbList.add(bindingBuilder.build()); } policyBuilder.addAllBindings(bindingPbList); @@ -140,7 +139,7 @@ public String apply(Identity identity) { /** A builder for {@code Policy} objects. */ public static class Builder { - private final List bindingsV3 = new ArrayList(); + private final List bindingsList = new ArrayList(); private String etag; private int version; @@ -149,7 +148,7 @@ protected Builder() {} @InternalApi("This class should only be extended within google-cloud-java") protected Builder(Policy policy) { - setBindingsV3(policy.bindingsV3); + setBindings(policy.bindingsList); setEtag(policy.etag); setVersion(policy.version); } @@ -159,48 +158,51 @@ protected Builder(Policy policy) { * * @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 - * @throws IllegalArgumentException if policy version is equal to 3. + * @throws IllegalArgumentException if policy version is equal to 3 or has conditional bindings. */ public final Builder setBindings(Map> bindings) { checkNotNull(bindings, "The provided map of bindings cannot be null."); + checkArgument(checkVersion(this.version, this.bindingsList), + "setBindings() is only supported with version 1 policies and non-conditional policies"); for (Map.Entry> binding : bindings.entrySet()) { checkNotNull(binding.getKey(), "The role cannot be null."); Set 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."); } - // convert into v3 format - this.bindingsV3.clear(); + // convert into List format + this.bindingsList.clear(); for (Map.Entry> binding : bindings.entrySet()) { Binding.Builder bindingBuilder = Binding.newBuilder(); - bindingBuilder.setIdentities(new HashSet<>(binding.getValue())); - bindingBuilder.setRole(binding.getKey()); - this.bindingsV3.add(bindingBuilder.build()); + bindingBuilder.setRole(binding.getKey().getValue()); + for (Identity identity : binding.getValue()) { + bindingBuilder.addMembers(identity.strValue()); + } + this.bindingsList.add(bindingBuilder.build()); } return this; } /** - * Replaces the builder's map of bindings with the given map of bindingsV3. + * Replaces the builder's List of bindings with the given List of Bindings. * * @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 Builder setBindingsV3(List bindings) { + public final Builder setBindings(List bindings) { for (Binding binding : bindings) { - checkNotNull(binding.getRole().getValue(), "The role cannot be null."); - Set identities = binding.getIdentities(); - checkNotNull(identities, "A role cannot be assigned to a null set of identities."); - checkArgument(!identities.contains(null), "Null identities are not permitted."); + checkNotNull(binding.getRole(), "The role cannot be null."); + List members = binding.getMembers(); + checkNotNull(members, "A role cannot be assigned to a null set of identities."); + checkArgument(!members.contains(null), "Null identities are not permitted."); } - // Set version to 3. - this.bindingsV3.clear(); + this.bindingsList.clear(); for (Binding binding : bindings) { Binding.Builder bindingBuilder = Binding.newBuilder(); - bindingBuilder.setIdentities(new HashSet<>(binding.getIdentities())); + bindingBuilder.setMembers(new ArrayList(binding.getMembers())); bindingBuilder.setRole(binding.getRole()); bindingBuilder.setCondition(binding.getCondition()); - this.bindingsV3.add(bindingBuilder.build()); + this.bindingsList.add(bindingBuilder.build()); } return this; } @@ -208,13 +210,16 @@ public final Builder setBindingsV3(List bindings) { /** Removes the role (and all identities associated with that role) from the policy. */ public final Builder removeRole(Role role) { - checkArgument(this.version != 3, "removeRole is not supported with version 3 policies."); - for (Binding binding : bindingsV3) { - if (binding.getRole().equals(role)) { - bindingsV3.remove(binding); + checkArgument(checkVersion(this.version, this.bindingsList), + "removeRole() is only supported with version 1 policies and non-conditional policies"); + for (int i = 0; i < bindingsList.size(); ++i) { + Binding binding = bindingsList.get(i); + if (binding.getRole().equals(role.getValue())) { + System.out.println(role.getValue() + " " + binding.getRole()); + bindingsList.remove(i); + return this; } } - return this; } @@ -222,48 +227,65 @@ public final Builder removeRole(Role role) { * Adds one or more identities to the policy under the role specified. * * @throws NullPointerException if the role or any of the identities is null. - * @throws IllegalArgumentException if policy version is equal to 3. + * @throws IllegalArgumentException if policy version is equal to 3 or has conditional bindings. */ public final Builder addIdentity(Role role, Identity first, Identity... others) { - checkArgument(this.version != 3, "removeRole is not supported with version 3 policies."); + checkArgument(checkVersion(this.version, this.bindingsList), + "addIdentity() is only supported with version 1 policies and non-conditional policies"); String nullIdentityMessage = "Null identities are not permitted."; checkNotNull(first, nullIdentityMessage); checkNotNull(others, nullIdentityMessage); - for (Binding binding : bindingsV3) { - if (binding.getRole().equals(checkNotNull(role, "The role cannot be null."))) { - Binding.Builder bindingBuilder = binding.toBuilder(); - bindingBuilder.addIdentity(first, others); - bindingsV3.remove(binding); - bindingsV3.add(bindingBuilder.build()); - return this; + checkNotNull(role, "The role cannot be null."); + for (int i = 0; i < bindingsList.size(); ++i) { + Binding binding = bindingsList.get(i); + if (binding.getRole().equals(role.getValue())) { + Binding.Builder bindingBuilder = binding.toBuilder().addMembers(first.strValue()); + for (Identity identity : others) { + bindingBuilder.addMembers(identity.strValue()); + } + bindingsList.set(i, bindingBuilder.build()); + return this; + } } + List members = new ArrayList<>(); + members.add(first.strValue()); + for (Identity identity : others) { + if (identity != null) + members.add(identity.strValue()); } - - Binding.Builder bindingBuilder = Binding.newBuilder(); - bindingBuilder.setRole(role); - bindingBuilder.addIdentity(first, others); - bindingsV3.add(bindingBuilder.build()); - + bindingsList.add(Binding.newBuilder() + .setRole(role.getValue()) + .setMembers(members) + .build()); return this; } /** * Removes one or more identities from an existing binding. Does nothing if the binding * associated with the provided role doesn't exist. - * @throws IllegalArgumentException if policy version is equal to 3. + * @throws IllegalArgumentException if policy version is equal to 3 or has conditional bindings. */ public final Builder removeIdentity(Role role, Identity first, Identity... others) { - checkArgument(this.version != 3, "removeRole is not supported with version 3 policies."); - for (Binding binding : bindingsV3) { - if (binding.getRole().equals(checkNotNull(role, "The role cannot be null."))) { - Binding.Builder bindingBuilder = binding.toBuilder(); - bindingBuilder.removeIdentity(first, others); - bindingsV3.remove(binding); - Binding builtBinding = bindingBuilder.build(); - if (builtBinding.getIdentities() != null && !builtBinding.getIdentities().isEmpty()) { - bindingsV3.add(builtBinding); + checkArgument(checkVersion(this.version, this.bindingsList), + "removeIdentity() is only supported with version 1 policies and non-conditional policies"); + String nullIdentityMessage = "Null identities are not permitted."; + checkNotNull(first, nullIdentityMessage); + checkNotNull(others, nullIdentityMessage); + checkNotNull(role, "The role cannot be null."); + for (int i = 0; i < bindingsList.size(); ++i) { + Binding binding = bindingsList.get(i); + if (binding.getRole().equals(role.getValue())) { + Binding.Builder bindingBuilder = binding.toBuilder().removeMembers(first.strValue()); + for (Identity identity : others) { + bindingBuilder.removeMembers(identity.strValue()); + } + Binding updatedBindings = bindingBuilder.build(); + if (updatedBindings.getMembers().size() == 0) { + bindingsList.remove(i); + } else { + bindingsList.set(i, updatedBindings); } - break; + return this; } } return this; @@ -301,15 +323,8 @@ public final Policy build() { } private Policy(Builder builder) { - ImmutableList.Builder bindingsV3Builder = ImmutableList.builder(); - for (Binding binding : builder.bindingsV3) { - Binding.Builder bindingBuilder = Binding.newBuilder(); - bindingBuilder.setRole(binding.getRole()) - .setIdentities(ImmutableSet.copyOf(binding.getIdentities())) - .setCondition(binding.getCondition()); - bindingsV3Builder.add(bindingBuilder.build()); - } - this.bindingsV3 = bindingsV3Builder.build(); + + this.bindingsList = ImmutableList.copyOf(builder.bindingsList); this.etag = builder.etag; this.version = builder.version; } @@ -321,23 +336,27 @@ public Builder toBuilder() { /** Returns the map of bindings that comprises the policy. * - * @throws IllegalArgumentException if policy version is equal to 3. + * @throws IllegalArgumentException if policy version is equal to 3 or has conditional bindings. * */ public Map> getBindings() { - checkArgument(this.version != 3, "removeRole is not supported with version 3 policies."); - // Convert to V1 IAM Policy if version is not 3. + checkArgument(checkVersion(this.version, this.bindingsList), + "getBindings() is only supported with version 1 policies and non-conditional policies"); ImmutableMap.Builder> bindingsV1Builder = ImmutableMap.builder(); - for (Binding binding : bindingsV3) { - bindingsV1Builder.put(binding.getRole(), binding.getIdentities()); + for (Binding binding : bindingsList) { + ImmutableSet.Builder identities = ImmutableSet.builder(); + for (String member : binding.getMembers()) { + identities.add(Identity.valueOf(member)); + } + bindingsV1Builder.put(Role.of(binding.getRole()), identities.build()); } return bindingsV1Builder.build(); } /** Returns the map of bindings that comprises the policy for version 3. */ - public List getBindingsV3() { - return bindingsV3; + public List getBindingsList() { + return bindingsList; } /** @@ -366,7 +385,7 @@ public int getVersion() { @Override public String toString() { return MoreObjects.toStringHelper(this) - .add("bindings", bindingsV3) + .add("bindings", bindingsList) .add("etag", etag) .add("version", version) .toString(); @@ -374,7 +393,7 @@ public String toString() { @Override public int hashCode() { - return Objects.hash(getClass(), bindingsV3, etag, version); + return Objects.hash(getClass(), bindingsList, etag, version); } @Override @@ -386,8 +405,13 @@ public boolean equals(Object obj) { return false; } Policy other = (Policy) obj; - return Objects.equals(bindingsV3, other.getBindings()) - && Objects.equals(etag, other.getEtag()) + if (bindingsList.size() != other.getBindingsList().size()) { + return false; + } + for (int i = 0; i < bindingsList.size(); ++i) { + bindingsList.get(i).equals(other.getBindingsList().get(i)); + } + return Objects.equals(etag, other.getEtag()) && Objects.equals(version, other.getVersion()); } diff --git a/google-cloud-core/src/test/java/com/google/cloud/PolicyTest.java b/google-cloud-core/src/test/java/com/google/cloud/PolicyTest.java index 61d99223e4..33505a80b1 100644 --- a/google-cloud-core/src/test/java/com/google/cloud/PolicyTest.java +++ b/google-cloud-core/src/test/java/com/google/cloud/PolicyTest.java @@ -125,7 +125,7 @@ public void testIllegalPolicies() { assertEquals("Null identities are not permitted.", ex.getMessage()); } try { - Policy.newBuilder().setBindings(null); + Policy.newBuilder().setBindings((Map>)null); fail("Null bindings map should cause exception."); } catch (NullPointerException ex) { assertEquals("The provided map of bindings cannot be null.", ex.getMessage()); diff --git a/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java b/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java new file mode 100644 index 0000000000..9305bd7d3e --- /dev/null +++ b/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java @@ -0,0 +1,259 @@ +/* + * Copyright 2016 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.cloud; + +import com.google.cloud.Policy.DefaultMarshaller; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import org.junit.Test; + +import java.util.*; + +import static org.junit.Assert.*; + +public class PolicyV3Test { + + private static final String ALL_USERS = "allUsers"; + private static final String ALL_AUTH_USERS = "allAuthenticatedUsers"; + private static final String USER = "user:abc@gmail.com"; + private static final String SERVICE_ACCOUNT = "serviceAccount:service-account@gmail.com"; + private static final String GROUP = "group:group@gmail.com"; + private static final String DOMAIN = "domain:google.com"; + private static final String VIEWER = "roles/viewer"; + private static final String EDITOR = "roles/editor"; + private static final String OWNER = "roles/owner"; + private static final List MEMBERS_LIST_1 = ImmutableList.of(USER, SERVICE_ACCOUNT, ALL_USERS); + private static final List MEMBERS_LIST_2 = ImmutableList.of(ALL_AUTH_USERS, GROUP, DOMAIN); + private static final List BINDINGS_NO_CONDITIONS = + ImmutableList.of( + Binding.newBuilder() + .setRole(VIEWER) + .setMembers(MEMBERS_LIST_1).build(), + Binding.newBuilder() + .setRole(EDITOR) + .setMembers(MEMBERS_LIST_2).build()); + private static final List BINDINGS_WITH_CONDITIONS = + ImmutableList.copyOf(BINDINGS_NO_CONDITIONS).of( + Binding.newBuilder() + .setRole(VIEWER) + .setMembers(MEMBERS_LIST_1) + .setCondition(Condition.newBuilder() + .setTitle("Condition") + .setDescription("Condition") + .setExpression("Expr") + .build()) + .build(), + Binding.newBuilder() + .setRole(EDITOR) + .setMembers(MEMBERS_LIST_2).build()); + private static final Policy FULL_POLICY_V1 = + Policy.newBuilder() + .setBindings(BINDINGS_NO_CONDITIONS) + .setEtag("etag") + .setVersion(1) + .build(); + + private static final Policy FULL_POLICY_V3 = + Policy.newBuilder() + .setBindings(BINDINGS_WITH_CONDITIONS) + .setEtag("etag") + .setVersion(3) + .build(); + + private static final Policy FULL_POLICY_V3_WITH_VERSION_1 = + Policy.newBuilder() + .setBindings(BINDINGS_WITH_CONDITIONS) + .setEtag("etag") + .setVersion(1) + .build(); + + @Test + public void testBuilder() { + assertEquals(BINDINGS_NO_CONDITIONS, FULL_POLICY_V1.getBindingsList()); + assertEquals(1, FULL_POLICY_V1.getVersion()); + assertEquals("etag", FULL_POLICY_V1.getEtag()); + Policy policy = FULL_POLICY_V1.toBuilder().setBindings(BINDINGS_NO_CONDITIONS).build(); + assertEquals(BINDINGS_NO_CONDITIONS, policy.getBindingsList()); + assertEquals("etag", policy.getEtag()); + assertEquals(1, policy.getVersion()); + + assertEquals(BINDINGS_WITH_CONDITIONS, FULL_POLICY_V3.getBindingsList()); + assertEquals(3, FULL_POLICY_V3.getVersion()); + assertEquals("etag", FULL_POLICY_V3.getEtag()); + policy = FULL_POLICY_V3.toBuilder().setBindings(BINDINGS_WITH_CONDITIONS).build(); + assertEquals(BINDINGS_WITH_CONDITIONS, policy.getBindingsList()); + assertEquals("etag", policy.getEtag()); + assertEquals(3, policy.getVersion()); + + assertEquals(BINDINGS_WITH_CONDITIONS, FULL_POLICY_V3_WITH_VERSION_1.getBindingsList()); + assertEquals(1, FULL_POLICY_V3_WITH_VERSION_1.getVersion()); + assertEquals("etag", FULL_POLICY_V3_WITH_VERSION_1.getEtag()); + policy = FULL_POLICY_V3_WITH_VERSION_1.toBuilder().setBindings(BINDINGS_WITH_CONDITIONS).setVersion(3).build(); + assertEquals(BINDINGS_WITH_CONDITIONS, policy.getBindingsList()); + assertEquals("etag", policy.getEtag()); + assertEquals(3, policy.getVersion()); + } + + + @Test + public void removeMemberFromPolicy() { + assertEquals(3, FULL_POLICY_V3.getBindingsList().get(0).getMembers().size()); + List bindings = new ArrayList<>(FULL_POLICY_V3.getBindingsList()); + + for (int i = 0; i < bindings.size(); ++i) { + Binding binding = bindings.get(i); + if (binding.getRole().equals(VIEWER)) { + bindings.set(i, binding.toBuilder().removeMembers(ALL_USERS).build()); + break; + } + } + + Policy updatedPolicy = FULL_POLICY_V3.toBuilder().setBindings(bindings).build(); + assertEquals(2, updatedPolicy.getBindingsList().get(0).getMembers().size()); + } + + @Test + public void addMemberFromPolicy() { + assertEquals(3, FULL_POLICY_V3.getBindingsList().get(0).getMembers().size()); + List bindings = new ArrayList<>(FULL_POLICY_V3.getBindingsList()); + + for (int i = 0; i < bindings.size(); ++i) { + Binding binding = bindings.get(i); + if (binding.getRole().equals(VIEWER)) { + bindings.set(i, binding.toBuilder().addMembers("user:example@example.com").build()); + } + } + + Policy updatedPolicy = FULL_POLICY_V3.toBuilder().setBindings(bindings).build(); + assertEquals(4, updatedPolicy.getBindingsList().get(0).getMembers().size()); + } + + @Test + public void removeBindingFromPolicy() { + assertEquals(2, FULL_POLICY_V3.getBindingsList().size()); + List bindings = new ArrayList<>(FULL_POLICY_V3.getBindingsList()); + + for (int i = 0; i < bindings.size(); ++i) { + Binding binding = bindings.get(i); + if (binding.getRole().equals(EDITOR) && binding.getCondition() == null) { + bindings.remove(i); + break; + } + } + + Policy updatedPolicy = FULL_POLICY_V3.toBuilder().setBindings(bindings).build(); + assertEquals(1, updatedPolicy.getBindingsList().size()); + } + + @Test + public void addBindingToPolicy() { + assertEquals(2, FULL_POLICY_V3.getBindingsList().size()); + List bindings = new ArrayList<>(FULL_POLICY_V3.getBindingsList()); + bindings.add(Binding.newBuilder().setRole(OWNER).addMembers(USER).build()); + Policy updatedPolicy = FULL_POLICY_V3.toBuilder().setBindings(bindings).build(); + assertEquals(3, updatedPolicy.getBindingsList().size()); + } + + + @Test + public void testIllegalPolicies() { + try { + Binding.newBuilder().setRole(null); + fail("Null role should cause exception."); + } catch (NullPointerException ex) { + assertEquals("The role cannot be null.", ex.getMessage()); + } + try { + Binding.newBuilder().addMembers(null); + fail("Null member should cause exception."); + } catch (NullPointerException ex) { + assertEquals("Null identities are not permitted.", ex.getMessage()); + } + try { + FULL_POLICY_V3.getBindings(); + fail("getBindings() should cause exception with Policy V3."); + } catch (IllegalArgumentException ex) { + assertEquals("getBindings() is only supported with version 1 policies and non-conditional policies", ex.getMessage()); + } + try { + FULL_POLICY_V3.toBuilder().addIdentity(Role.editor(), Identity.allUsers()); + fail("getBindings() should cause exception with Policy V3."); + } catch (IllegalArgumentException ex) { + assertEquals("addIdentity() is only supported with version 1 policies and non-conditional policies", ex.getMessage()); + } + try { + FULL_POLICY_V3.toBuilder().removeIdentity(Role.editor(), Identity.allUsers()); + fail("getBindings() should cause exception with Policy V3."); + } catch (IllegalArgumentException ex) { + assertEquals("removeIdentity() is only supported with version 1 policies and non-conditional policies", ex.getMessage()); + } + try { + FULL_POLICY_V3.toBuilder().setBindings(FULL_POLICY_V1.getBindings()); + fail("getBindings() should cause exception with Policy V3."); + } catch (IllegalArgumentException ex) { + assertEquals("setBindings() is only supported with version 1 policies and non-conditional policies", ex.getMessage()); + } + } + + @Test + public void testEqualsHashCode() { + assertNotNull(FULL_POLICY_V3); + Policy emptyPolicy = Policy.newBuilder().build(); + Policy anotherPolicy = Policy.newBuilder().build(); + assertEquals(emptyPolicy, anotherPolicy); + assertEquals(emptyPolicy.hashCode(), anotherPolicy.hashCode()); + assertNotEquals(FULL_POLICY_V3, FULL_POLICY_V1); + assertNotEquals(FULL_POLICY_V3.hashCode(), FULL_POLICY_V1.hashCode()); + Policy copy = FULL_POLICY_V1.toBuilder().build(); + assertEquals(FULL_POLICY_V1, copy); + assertEquals(FULL_POLICY_V1.hashCode(), copy.hashCode()); + } + + @Test + public void testBindings() { + assertTrue(Policy.newBuilder().build().getBindingsList().isEmpty()); + assertEquals(BINDINGS_WITH_CONDITIONS, FULL_POLICY_V3.getBindingsList()); + } + + @Test + public void testEtag() { + assertNotNull(FULL_POLICY_V3.getEtag()); + assertEquals("etag", FULL_POLICY_V3.getEtag()); + } + + @Test + public void testVersion() { + assertEquals(1, FULL_POLICY_V1.getVersion()); + assertEquals(3, FULL_POLICY_V3.getVersion()); + assertEquals(1, FULL_POLICY_V3_WITH_VERSION_1.getVersion()); + } + + @Test + public void testDefaultMarshaller() { + DefaultMarshaller marshaller = new DefaultMarshaller(); + Policy emptyPolicy = Policy.newBuilder().build(); + assertEquals(emptyPolicy, marshaller.fromPb(marshaller.toPb(emptyPolicy))); + assertEquals(FULL_POLICY_V3, marshaller.fromPb(marshaller.toPb(FULL_POLICY_V3))); + assertEquals(FULL_POLICY_V1, marshaller.fromPb(marshaller.toPb(FULL_POLICY_V1))); + com.google.iam.v1.Policy policyPb = com.google.iam.v1.Policy.getDefaultInstance(); + Policy policy = marshaller.fromPb(policyPb); + assertTrue(policy.getBindingsList().isEmpty()); + assertNull(policy.getEtag()); + assertEquals(0, policy.getVersion()); + } +} From 145f7a082654eb24798b94c5ea184f37039a60b0 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Thu, 19 Dec 2019 23:07:20 -0800 Subject: [PATCH 03/31] lint --- .../main/java/com/google/cloud/Binding.java | 218 +++--- .../main/java/com/google/cloud/Condition.java | 162 ++--- .../main/java/com/google/cloud/Policy.java | 659 +++++++++--------- .../java/com/google/cloud/PolicyTest.java | 308 ++++---- .../java/com/google/cloud/PolicyV3Test.java | 404 +++++------ 5 files changed, 888 insertions(+), 863 deletions(-) diff --git a/google-cloud-core/src/main/java/com/google/cloud/Binding.java b/google-cloud-core/src/main/java/com/google/cloud/Binding.java index e7ab048ed3..827c30012d 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Binding.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Binding.java @@ -16,7 +16,6 @@ package com.google.cloud; -import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import com.google.api.core.InternalApi; @@ -26,130 +25,133 @@ import java.util.*; public class Binding { - private String role; - private List members; - private Condition condition; - - public static class Builder { - private List members = new ArrayList(); private String role; + private List members; private Condition condition; - @InternalApi("This class should only be extended within google-cloud-java") - protected Builder() {} + public static class Builder { + private List members = new ArrayList(); + private String role; + private Condition condition; + + @InternalApi("This class should only be extended within google-cloud-java") + protected Builder() { + } + + @InternalApi("This class should only be extended within google-cloud-java") + protected Builder(Binding binding) { + setRole(binding.role); + setMembers(binding.members); + setCondition(binding.condition); + } + + public final Binding.Builder setRole(String role) { + String nullIdentityMessage = "The role cannot be null."; + checkNotNull(role, nullIdentityMessage); + this.role = role; + return this; + } + + public final Binding.Builder setMembers(List members) { + String nullIdentityMessage = "Null members are not permitted."; + checkNotNull(members, nullIdentityMessage); + this.members.clear(); + for (String member : members) { + // Check member not null + this.members.add(member); + } + return this; + } + + public final Binding.Builder removeMembers(String first, String... others) { + String nullIdentityMessage = "Null members are not permitted."; + checkNotNull(first, nullIdentityMessage); + checkNotNull(others, nullIdentityMessage); + this.members.remove(first); + for (String member : others) { + this.members.remove(member); + } + return this; + } + + public final Binding.Builder addMembers(String first, String... others) { + String nullIdentityMessage = "Null identities are not permitted."; + checkNotNull(first, nullIdentityMessage); + checkNotNull(others, nullIdentityMessage); + this.members.add(first); + for (String member : others) { + this.members.add(member); + } + return this; + } + + public final Binding.Builder setCondition(Condition condition) { + this.condition = condition; + return this; + } + + /** + * Creates a {@code Policy} object. + */ + public final Binding build() { + return new Binding(this); + } + } - @InternalApi("This class should only be extended within google-cloud-java") - protected Builder(Binding binding) { - setRole(binding.role); - setMembers(binding.members); - setCondition(binding.condition); + private Binding(Binding.Builder builder) { + this.role = builder.role; + this.members = ImmutableList.copyOf(builder.members); + this.condition = builder.condition; } - public final Binding.Builder setRole(String role) { - String nullIdentityMessage = "The role cannot be null."; - checkNotNull(role, nullIdentityMessage); - this.role = role; - return this; + public Binding.Builder toBuilder() { + return new Binding.Builder(this); } - public final Binding.Builder setMembers(List members) { - String nullIdentityMessage = "Null members are not permitted."; - checkNotNull(members, nullIdentityMessage); - this.members.clear(); - for (String member : members) { - // Check member not null - this.members.add(member); - } - return this; + public String getRole() { + return this.role; } - public final Binding.Builder removeMembers(String first, String... others) { - String nullIdentityMessage = "Null members are not permitted."; - checkNotNull(first, nullIdentityMessage); - checkNotNull(others, nullIdentityMessage); - this.members.remove(first); - for (String member : others) { - if (member != null) - this.members.remove(member); - } - return this; + public List getMembers() { + return this.members; } - public final Binding.Builder addMembers(String first, String... others) { - String nullIdentityMessage = "Null identities are not permitted."; - checkNotNull(first, nullIdentityMessage); - checkNotNull(others, nullIdentityMessage); - this.members.add(first); - for (String member : others) { - if (member != null) - this.members.add(member); - } - return this; + public Condition getCondition() { + return this.condition; } - public final Binding.Builder setCondition(Condition condition) { - this.condition = condition; - return this; + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("role", role) + .add("members", members) + .add("condition", condition) + .toString(); } - /** Creates a {@code Policy} object. */ - public final Binding build() { - return new Binding(this); + @Override + public int hashCode() { + return Objects.hash(getClass(), role, members, condition); } - } - - private Binding(Binding.Builder builder) { - this.role = builder.role; - this.members = ImmutableList.copyOf(builder.members); - this.condition = builder.condition; - } - - public Binding.Builder toBuilder() { - return new Binding.Builder(this); - } - - public String getRole() { - return this.role; - } - - public List getMembers() { - return this.members; - } - - public Condition getCondition() { - return this.condition; - } - - @Override - public String toString() { - return MoreObjects.toStringHelper(this) - .add("role", role) - .add("members", members) - .add("condition", condition) - .toString(); - } - - @Override - public int hashCode() { - return Objects.hash(getClass(), role, members, condition); - } - - @Override - public boolean equals(Object obj) { - if (obj == this) { - return true; + + @Override + public boolean equals(Object obj) { + if (obj == this) { + return true; + } + if (!(obj instanceof Binding)) { + return false; + } + Binding other = (Binding) obj; + return Objects.equals(role, other.getRole()) + && Objects.equals(members, other.getMembers()) + && Objects.equals(condition, other.getCondition()); } - if (!(obj instanceof Binding)) { - return false; + + /** + * Returns a builder for {@code Policy} objects. + */ + public static Binding.Builder newBuilder() { + return new Binding.Builder(); } - Binding other = (Binding) obj; - return Objects.equals(role, other.getRole()) - && Objects.equals(members, other.getMembers()) - && Objects.equals(condition, other.getCondition()); - } - - /** Returns a builder for {@code Policy} objects. */ - public static Binding.Builder newBuilder() { - return new Binding.Builder(); - } } diff --git a/google-cloud-core/src/main/java/com/google/cloud/Condition.java b/google-cloud-core/src/main/java/com/google/cloud/Condition.java index 566fef142c..4f4a8b8afb 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Condition.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Condition.java @@ -18,101 +18,107 @@ import com.google.api.core.InternalApi; import com.google.common.base.MoreObjects; + import java.util.Objects; public class Condition { - private String title; - private String description; - private String expression; - - public static class Builder { private String title; private String description; private String expression; - @InternalApi("This class should only be extended within google-cloud-java") - protected Builder() {} + public static class Builder { + private String title; + private String description; + private String expression; + + @InternalApi("This class should only be extended within google-cloud-java") + protected Builder() { + } + + @InternalApi("This class should only be extended within google-cloud-java") + protected Builder(Condition condition) { + this.title = condition.title; + this.description = condition.description; + this.expression = condition.expression; + } + + public final Condition.Builder setTitle(String title) { + this.title = title; + return this; + } + + public final Condition.Builder setDescription(String description) { + this.description = description; + return this; + } + + public final Condition.Builder setExpression(String expression) { + this.expression = expression; + return this; + } + + /** + * Creates a {@code Condition} object. + */ + public final Condition build() { + return new Condition(this); + } + } - @InternalApi("This class should only be extended within google-cloud-java") - protected Builder(Condition condition) { - this.title = condition.title; - this.description = condition.description; - this.expression = condition.expression; + private Condition(Condition.Builder builder) { + this.title = builder.title; + this.description = builder.description; + this.expression = builder.expression; } - public final Condition.Builder setTitle(String title) { - this.title = title; - return this; + public Condition.Builder toBuilder() { + return new Condition.Builder(this); } - public final Condition.Builder setDescription(String description) { - this.description = description; - return this; + public String getTitle() { + return title; } - public final Condition.Builder setExpression(String expression) { - this.expression = expression; - return this; + public String getDescription() { + return description; } - /** Creates a {@code Condition} object. */ - public final Condition build() { - return new Condition(this); + public String getExpression() { + return expression; } - } - - private Condition(Condition.Builder builder) { - this.title = builder.title; - this.description = builder.description; - this.expression = builder.expression; - } - - public Condition.Builder toBuilder() { - return new Condition.Builder(this); - } - - public String getTitle() { - return title; - } - - public String getDescription() { - return description; - } - - public String getExpression() { - return expression; - } - - @Override - public String toString() { - return MoreObjects.toStringHelper(this) - .add("title", title) - .add("description", description) - .add("expression", expression) - .toString(); - } - - @Override - public int hashCode() { - return Objects.hash(getClass(), title, description, expression); - } - - @Override - public boolean equals(Object obj) { - if (obj == this) { - return true; + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("title", title) + .add("description", description) + .add("expression", expression) + .toString(); } - if (!(obj instanceof Condition)) { - return false; + + @Override + public int hashCode() { + return Objects.hash(getClass(), title, description, expression); + } + + @Override + public boolean equals(Object obj) { + if (obj == this) { + return true; + } + if (!(obj instanceof Condition)) { + return false; + } + Condition other = (Condition) obj; + return Objects.equals(title, other.getTitle()) + && Objects.equals(description, other.getDescription()) + && Objects.equals(expression, other.getExpression()); + } + + /** + * Returns a builder for {@code Policy} objects. + */ + public static Condition.Builder newBuilder() { + return new Condition.Builder(); } - Condition other = (Condition) obj; - return Objects.equals(title, other.getTitle()) - && Objects.equals(description, other.getDescription()) - && Objects.equals(expression, other.getExpression()); - } - - /** Returns a builder for {@code Policy} objects. */ - public static Condition.Builder newBuilder() { - return new Condition.Builder(); - } } diff --git a/google-cloud-core/src/main/java/com/google/cloud/Policy.java b/google-cloud-core/src/main/java/com/google/cloud/Policy.java index 7be4f6f520..9de35bfc97 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Policy.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Policy.java @@ -50,373 +50,388 @@ */ public final class Policy implements Serializable { - private static final long serialVersionUID = -3348914530232544290L; - private final List bindingsList; - private final String etag; - private final int version; - - /* - * Return false if Policy is version 3 OR bindings has a conditional binding. - * Return true if Policy is version 1 AND bindings does not have a conditional binding. - */ - private static boolean checkVersion(int version, List bindingsList) { - for (Binding binding : bindingsList) { - if (binding.getCondition() != null) { - return false; - } - } - if (version > 1) { - return false; + private static final long serialVersionUID = -3348914530232544290L; + private final List bindingsList; + private final String etag; + private final int version; + + /* + * Return false if Policy is version 3 OR bindings has a conditional binding. + * Return true if Policy is version 1 AND bindings does not have a conditional binding. + */ + private static boolean checkVersion(int version, List bindingsList) { + for (Binding binding : bindingsList) { + if (binding.getCondition() != null) { + return false; + } + } + if (version > 1) { + return false; + } + return true; } - return true; - } - - public abstract static class Marshaller { - @InternalApi("This class should only be extended within google-cloud-java") - protected Marshaller() {} + public abstract static class Marshaller { + @InternalApi("This class should only be extended within google-cloud-java") + protected Marshaller() { + } - protected abstract Policy fromPb(T policyPb); - protected abstract T toPb(Policy policy); - } + protected abstract Policy fromPb(T policyPb); - public static class DefaultMarshaller extends Marshaller { + protected abstract T toPb(Policy policy); + } - @Override - protected Policy fromPb(com.google.iam.v1.Policy policyPb) { - List bindingsList = new ArrayList(); - for (com.google.iam.v1.Binding bindingPb : policyPb.getBindingsList()) { - Binding.Builder convertedBinding = Binding.newBuilder().setRole(bindingPb.getRole()) - .setMembers(ImmutableList.copyOf(bindingPb.getMembersList())); - if (bindingPb.hasCondition()) { - Expr expr = bindingPb.getCondition(); - convertedBinding.setCondition(Condition.newBuilder() - .setTitle(expr.getTitle()) - .setDescription(expr.getDescription()) - .setExpression(expr.getExpression()) - .build()); - } - bindingsList.add(convertedBinding.build()); + public static class DefaultMarshaller extends Marshaller { + + @Override + protected Policy fromPb(com.google.iam.v1.Policy policyPb) { + List bindingsList = new ArrayList(); + for (com.google.iam.v1.Binding bindingPb : policyPb.getBindingsList()) { + Binding.Builder convertedBinding = Binding.newBuilder().setRole(bindingPb.getRole()) + .setMembers(ImmutableList.copyOf(bindingPb.getMembersList())); + if (bindingPb.hasCondition()) { + Expr expr = bindingPb.getCondition(); + convertedBinding.setCondition(Condition.newBuilder() + .setTitle(expr.getTitle()) + .setDescription(expr.getDescription()) + .setExpression(expr.getExpression()) + .build()); + } + bindingsList.add(convertedBinding.build()); + } + return newBuilder() + .setBindings(bindingsList) + .setEtag( + policyPb.getEtag().isEmpty() + ? null + : BaseEncoding.base64().encode(policyPb.getEtag().toByteArray())) + .setVersion(policyPb.getVersion()) + .build(); } - return newBuilder() - .setBindings(bindingsList) - .setEtag( - policyPb.getEtag().isEmpty() - ? null - : BaseEncoding.base64().encode(policyPb.getEtag().toByteArray())) - .setVersion(policyPb.getVersion()) - .build(); - } - @Override - protected com.google.iam.v1.Policy toPb(Policy policy) { - com.google.iam.v1.Policy.Builder policyBuilder = com.google.iam.v1.Policy.newBuilder(); - List bindingPbList = new LinkedList<>(); - for (Binding binding : policy.getBindingsList()) { - com.google.iam.v1.Binding.Builder bindingBuilder = com.google.iam.v1.Binding.newBuilder(); - bindingBuilder.setRole(binding.getRole()); - bindingBuilder.addAllMembers(ImmutableList.copyOf(binding.getMembers())); - if (binding.getCondition() != null) { - Condition condition = binding.getCondition(); - bindingBuilder.setCondition(Expr.newBuilder() - .setTitle(condition.getTitle()) - .setDescription(condition.getDescription()) - .setExpression(condition.getExpression()) - .build()); + @Override + protected com.google.iam.v1.Policy toPb(Policy policy) { + com.google.iam.v1.Policy.Builder policyBuilder = com.google.iam.v1.Policy.newBuilder(); + List bindingPbList = new LinkedList<>(); + for (Binding binding : policy.getBindingsList()) { + com.google.iam.v1.Binding.Builder bindingBuilder = com.google.iam.v1.Binding.newBuilder(); + bindingBuilder.setRole(binding.getRole()); + bindingBuilder.addAllMembers(ImmutableList.copyOf(binding.getMembers())); + if (binding.getCondition() != null) { + Condition condition = binding.getCondition(); + bindingBuilder.setCondition(Expr.newBuilder() + .setTitle(condition.getTitle()) + .setDescription(condition.getDescription()) + .setExpression(condition.getExpression()) + .build()); + } + bindingPbList.add(bindingBuilder.build()); + } + policyBuilder.addAllBindings(bindingPbList); + if (policy.etag != null) { + policyBuilder.setEtag(ByteString.copyFrom(BaseEncoding.base64().decode(policy.etag))); + } + policyBuilder.setVersion(policy.version); + return policyBuilder.build(); } - bindingPbList.add(bindingBuilder.build()); - } - policyBuilder.addAllBindings(bindingPbList); - if (policy.etag != null) { - policyBuilder.setEtag(ByteString.copyFrom(BaseEncoding.base64().decode(policy.etag))); - } - policyBuilder.setVersion(policy.version); - return policyBuilder.build(); - } - } - - /** A builder for {@code Policy} objects. */ - public static class Builder { - private final List bindingsList = new ArrayList(); - private String etag; - private int version; - - @InternalApi("This class should only be extended within google-cloud-java") - protected Builder() {} - - @InternalApi("This class should only be extended within google-cloud-java") - protected Builder(Policy policy) { - setBindings(policy.bindingsList); - setEtag(policy.etag); - setVersion(policy.version); } /** - * Replaces the builder's map of bindings with the given map of bindings. - * - * @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 - * @throws IllegalArgumentException if policy version is equal to 3 or has conditional bindings. + * A builder for {@code Policy} objects. */ - public final Builder setBindings(Map> bindings) { - checkNotNull(bindings, "The provided map of bindings cannot be null."); - checkArgument(checkVersion(this.version, this.bindingsList), - "setBindings() is only supported with version 1 policies and non-conditional policies"); - for (Map.Entry> binding : bindings.entrySet()) { - checkNotNull(binding.getKey(), "The role cannot be null."); - Set 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."); - } - // convert into List format - this.bindingsList.clear(); - for (Map.Entry> binding : bindings.entrySet()) { - Binding.Builder bindingBuilder = Binding.newBuilder(); - bindingBuilder.setRole(binding.getKey().getValue()); - for (Identity identity : binding.getValue()) { - bindingBuilder.addMembers(identity.strValue()); + public static class Builder { + private final List bindingsList = new ArrayList(); + private String etag; + private int version; + + @InternalApi("This class should only be extended within google-cloud-java") + protected Builder() { } - this.bindingsList.add(bindingBuilder.build()); - } - return this; - } - /** - * Replaces the builder's List of bindings with the given List of Bindings. - * - * @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 Builder setBindings(List bindings) { - for (Binding binding : bindings) { - checkNotNull(binding.getRole(), "The role cannot be null."); - List members = binding.getMembers(); - checkNotNull(members, "A role cannot be assigned to a null set of identities."); - checkArgument(!members.contains(null), "Null identities are not permitted."); - } - this.bindingsList.clear(); - for (Binding binding : bindings) { - Binding.Builder bindingBuilder = Binding.newBuilder(); - bindingBuilder.setMembers(new ArrayList(binding.getMembers())); - bindingBuilder.setRole(binding.getRole()); - bindingBuilder.setCondition(binding.getCondition()); - this.bindingsList.add(bindingBuilder.build()); - } - return this; - } + @InternalApi("This class should only be extended within google-cloud-java") + protected Builder(Policy policy) { + setBindings(policy.bindingsList); + setEtag(policy.etag); + setVersion(policy.version); + } + /** + * Replaces the builder's map of bindings with the given map of bindings. + * + * @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 + * @throws IllegalArgumentException if policy version is equal to 3 or has conditional bindings. + */ + public final Builder setBindings(Map> bindings) { + checkNotNull(bindings, "The provided map of bindings cannot be null."); + checkArgument(checkVersion(this.version, this.bindingsList), + "setBindings() is only supported with version 1 policies and non-conditional policies"); + for (Map.Entry> binding : bindings.entrySet()) { + checkNotNull(binding.getKey(), "The role cannot be null."); + Set 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."); + } + // convert into List format + this.bindingsList.clear(); + for (Map.Entry> binding : bindings.entrySet()) { + Binding.Builder bindingBuilder = Binding.newBuilder(); + bindingBuilder.setRole(binding.getKey().getValue()); + for (Identity identity : binding.getValue()) { + bindingBuilder.addMembers(identity.strValue()); + } + this.bindingsList.add(bindingBuilder.build()); + } + return this; + } - /** Removes the role (and all identities associated with that role) from the policy. */ - public final Builder removeRole(Role role) { - checkArgument(checkVersion(this.version, this.bindingsList), - "removeRole() is only supported with version 1 policies and non-conditional policies"); - for (int i = 0; i < bindingsList.size(); ++i) { - Binding binding = bindingsList.get(i); - if (binding.getRole().equals(role.getValue())) { - System.out.println(role.getValue() + " " + binding.getRole()); - bindingsList.remove(i); - return this; + /** + * Replaces the builder's List of bindings with the given List of Bindings. + * + * @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 Builder setBindings(List bindings) { + for (Binding binding : bindings) { + checkNotNull(binding.getRole(), "The role cannot be null."); + List members = binding.getMembers(); + checkNotNull(members, "A role cannot be assigned to a null set of identities."); + checkArgument(!members.contains(null), "Null identities are not permitted."); + } + this.bindingsList.clear(); + for (Binding binding : bindings) { + Binding.Builder bindingBuilder = Binding.newBuilder(); + bindingBuilder.setMembers(new ArrayList(binding.getMembers())); + bindingBuilder.setRole(binding.getRole()); + bindingBuilder.setCondition(binding.getCondition()); + this.bindingsList.add(bindingBuilder.build()); + } + return this; } - } - return this; - } - /** - * Adds one or more identities to the policy under the role specified. - * - * @throws NullPointerException if the role or any of the identities is null. - * @throws IllegalArgumentException if policy version is equal to 3 or has conditional bindings. - */ - public final Builder addIdentity(Role role, Identity first, Identity... others) { - checkArgument(checkVersion(this.version, this.bindingsList), - "addIdentity() is only supported with version 1 policies and non-conditional policies"); - String nullIdentityMessage = "Null identities are not permitted."; - checkNotNull(first, nullIdentityMessage); - checkNotNull(others, nullIdentityMessage); - checkNotNull(role, "The role cannot be null."); - for (int i = 0; i < bindingsList.size(); ++i) { - Binding binding = bindingsList.get(i); - if (binding.getRole().equals(role.getValue())) { - Binding.Builder bindingBuilder = binding.toBuilder().addMembers(first.strValue()); + + /** + * Removes the role (and all identities associated with that role) from the policy. + */ + public final Builder removeRole(Role role) { + checkArgument(checkVersion(this.version, this.bindingsList), + "removeRole() is only supported with version 1 policies and non-conditional policies"); + for (int i = 0; i < bindingsList.size(); ++i) { + Binding binding = bindingsList.get(i); + if (binding.getRole().equals(role.getValue())) { + System.out.println(role.getValue() + " " + binding.getRole()); + bindingsList.remove(i); + return this; + } + } + return this; + } + + /** + * Adds one or more identities to the policy under the role specified. + * + * @throws NullPointerException if the role or any of the identities is null. + * @throws IllegalArgumentException if policy version is equal to 3 or has conditional bindings. + */ + public final Builder addIdentity(Role role, Identity first, Identity... others) { + checkArgument(checkVersion(this.version, this.bindingsList), + "addIdentity() is only supported with version 1 policies and non-conditional policies"); + String nullIdentityMessage = "Null identities are not permitted."; + checkNotNull(first, nullIdentityMessage); + checkNotNull(others, nullIdentityMessage); + checkNotNull(role, "The role cannot be null."); + for (int i = 0; i < bindingsList.size(); ++i) { + Binding binding = bindingsList.get(i); + if (binding.getRole().equals(role.getValue())) { + Binding.Builder bindingBuilder = binding.toBuilder().addMembers(first.strValue()); + for (Identity identity : others) { + bindingBuilder.addMembers(identity.strValue()); + } + bindingsList.set(i, bindingBuilder.build()); + return this; + } + } + List members = new ArrayList<>(); + members.add(first.strValue()); for (Identity identity : others) { - bindingBuilder.addMembers(identity.strValue()); + members.add(identity.strValue()); + } + bindingsList.add(Binding.newBuilder() + .setRole(role.getValue()) + .setMembers(members) + .build()); + return this; + } + + /** + * Removes one or more identities from an existing binding. Does nothing if the binding + * associated with the provided role doesn't exist. + * + * @throws IllegalArgumentException if policy version is equal to 3 or has conditional bindings. + */ + public final Builder removeIdentity(Role role, Identity first, Identity... others) { + checkArgument(checkVersion(this.version, this.bindingsList), + "removeIdentity() is only supported with version 1 policies and non-conditional policies"); + String nullIdentityMessage = "Null identities are not permitted."; + checkNotNull(first, nullIdentityMessage); + checkNotNull(others, nullIdentityMessage); + checkNotNull(role, "The role cannot be null."); + for (int i = 0; i < bindingsList.size(); ++i) { + Binding binding = bindingsList.get(i); + if (binding.getRole().equals(role.getValue())) { + Binding.Builder bindingBuilder = binding.toBuilder().removeMembers(first.strValue()); + for (Identity identity : others) { + bindingBuilder.removeMembers(identity.strValue()); + } + Binding updatedBindings = bindingBuilder.build(); + if (updatedBindings.getMembers().size() == 0) { + bindingsList.remove(i); + } else { + bindingsList.set(i, updatedBindings); + } + return this; + } } - bindingsList.set(i, bindingBuilder.build()); return this; } + + /** + * Sets the policy's etag. + * + *

Etags are used for optimistic concurrency control as a way to help prevent simultaneous + * updates of a policy from overwriting each other. It is strongly suggested that systems make + * use of the etag in the read-modify-write cycle to perform policy updates in order to avoid + * race conditions. An etag is returned in the response to getIamPolicy, and systems are + * expected to put that etag in the request to setIamPolicy to ensure that their change will be + * applied to the same version of the policy. If no etag is provided in the call to + * setIamPolicy, then the existing policy is overwritten blindly. + */ + public final Builder setEtag(String etag) { + this.etag = etag; + return this; + } + + /** + * Sets the version of the policy. The default version is 0, meaning only the "owner", "editor", + * and "viewer" roles are permitted. If the version is 1, you may also use other roles. + */ + public final Builder setVersion(int version) { + this.version = version; + return this; } - List members = new ArrayList<>(); - members.add(first.strValue()); - for (Identity identity : others) { - if (identity != null) - members.add(identity.strValue()); - } - bindingsList.add(Binding.newBuilder() - .setRole(role.getValue()) - .setMembers(members) - .build()); - return this; + + /** + * Creates a {@code Policy} object. + */ + public final Policy build() { + return new Policy(this); + } + } + + private Policy(Builder builder) { + + this.bindingsList = ImmutableList.copyOf(builder.bindingsList); + this.etag = builder.etag; + this.version = builder.version; } /** - * Removes one or more identities from an existing binding. Does nothing if the binding - * associated with the provided role doesn't exist. + * Returns a builder containing the properties of this IAM Policy. + */ + public Builder toBuilder() { + return new Builder(this); + } + + /** + * Returns the map of bindings that comprises the policy. + * * @throws IllegalArgumentException if policy version is equal to 3 or has conditional bindings. */ - public final Builder removeIdentity(Role role, Identity first, Identity... others) { - checkArgument(checkVersion(this.version, this.bindingsList), - "removeIdentity() is only supported with version 1 policies and non-conditional policies"); - String nullIdentityMessage = "Null identities are not permitted."; - checkNotNull(first, nullIdentityMessage); - checkNotNull(others, nullIdentityMessage); - checkNotNull(role, "The role cannot be null."); - for (int i = 0; i < bindingsList.size(); ++i) { - Binding binding = bindingsList.get(i); - if (binding.getRole().equals(role.getValue())) { - Binding.Builder bindingBuilder = binding.toBuilder().removeMembers(first.strValue()); - for (Identity identity : others) { - bindingBuilder.removeMembers(identity.strValue()); - } - Binding updatedBindings = bindingBuilder.build(); - if (updatedBindings.getMembers().size() == 0) { - bindingsList.remove(i); - } else { - bindingsList.set(i, updatedBindings); - } - return this; + + public Map> getBindings() { + checkArgument(checkVersion(this.version, this.bindingsList), + "getBindings() is only supported with version 1 policies and non-conditional policies"); + ImmutableMap.Builder> bindingsV1Builder = ImmutableMap.builder(); + for (Binding binding : bindingsList) { + ImmutableSet.Builder identities = ImmutableSet.builder(); + for (String member : binding.getMembers()) { + identities.add(Identity.valueOf(member)); + } + bindingsV1Builder.put(Role.of(binding.getRole()), identities.build()); } - } - return this; + return bindingsV1Builder.build(); } /** - * Sets the policy's etag. + * Returns the map of bindings that comprises the policy for version 3. + */ + + public List getBindingsList() { + return bindingsList; + } + + /** + * Returns the policy's etag. * *

Etags are used for optimistic concurrency control as a way to help prevent simultaneous - * updates of a policy from overwriting each other. It is strongly suggested that systems make - * use of the etag in the read-modify-write cycle to perform policy updates in order to avoid - * race conditions. An etag is returned in the response to getIamPolicy, and systems are - * expected to put that etag in the request to setIamPolicy to ensure that their change will be - * applied to the same version of the policy. If no etag is provided in the call to - * setIamPolicy, then the existing policy is overwritten blindly. + * updates of a policy from overwriting each other. It is strongly suggested that systems make use + * of the etag in the read-modify-write cycle to perform policy updates in order to avoid race + * conditions. An etag is returned in the response to getIamPolicy, and systems are expected to + * put that etag in the request to setIamPolicy to ensure that their change will be applied to the + * same version of the policy. If no etag is provided in the call to setIamPolicy, then the + * existing policy is overwritten blindly. */ - public final Builder setEtag(String etag) { - this.etag = etag; - return this; + public String getEtag() { + return etag; } /** - * Sets the version of the policy. The default version is 0, meaning only the "owner", "editor", - * and "viewer" roles are permitted. If the version is 1, you may also use other roles. + * Returns the version of the policy. The default version is 0, meaning only the "owner", + * "editor", and "viewer" roles are permitted. If the version is 1, you may also use other roles. */ - public final Builder setVersion(int version) { - this.version = version; - return this; + public int getVersion() { + return version; } - /** Creates a {@code Policy} object. */ - public final Policy build() { - return new Policy(this); - } - } - - private Policy(Builder builder) { - - this.bindingsList = ImmutableList.copyOf(builder.bindingsList); - this.etag = builder.etag; - this.version = builder.version; - } - - /** Returns a builder containing the properties of this IAM Policy. */ - public Builder toBuilder() { - return new Builder(this); - } - - /** Returns the map of bindings that comprises the policy. - * - * @throws IllegalArgumentException if policy version is equal to 3 or has conditional bindings. - * */ - - public Map> getBindings() { - checkArgument(checkVersion(this.version, this.bindingsList), - "getBindings() is only supported with version 1 policies and non-conditional policies"); - ImmutableMap.Builder> bindingsV1Builder = ImmutableMap.builder(); - for (Binding binding : bindingsList) { - ImmutableSet.Builder identities = ImmutableSet.builder(); - for (String member : binding.getMembers()) { - identities.add(Identity.valueOf(member)); - } - bindingsV1Builder.put(Role.of(binding.getRole()), identities.build()); - } - return bindingsV1Builder.build(); - } - - /** Returns the map of bindings that comprises the policy for version 3. */ - - public List getBindingsList() { - return bindingsList; - } - - /** - * Returns the policy's etag. - * - *

Etags are used for optimistic concurrency control as a way to help prevent simultaneous - * updates of a policy from overwriting each other. It is strongly suggested that systems make use - * of the etag in the read-modify-write cycle to perform policy updates in order to avoid race - * conditions. An etag is returned in the response to getIamPolicy, and systems are expected to - * put that etag in the request to setIamPolicy to ensure that their change will be applied to the - * same version of the policy. If no etag is provided in the call to setIamPolicy, then the - * existing policy is overwritten blindly. - */ - public String getEtag() { - return etag; - } - - /** - * Returns the version of the policy. The default version is 0, meaning only the "owner", - * "editor", and "viewer" roles are permitted. If the version is 1, you may also use other roles. - */ - public int getVersion() { - return version; - } - - @Override - public String toString() { - return MoreObjects.toStringHelper(this) - .add("bindings", bindingsList) - .add("etag", etag) - .add("version", version) - .toString(); - } - - @Override - public int hashCode() { - return Objects.hash(getClass(), bindingsList, etag, version); - } - - @Override - public boolean equals(Object obj) { - if (obj == this) { - return true; + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("bindings", bindingsList) + .add("etag", etag) + .add("version", version) + .toString(); } - if (!(obj instanceof Policy)) { - return false; + + @Override + public int hashCode() { + return Objects.hash(getClass(), bindingsList, etag, version); } - Policy other = (Policy) obj; - if (bindingsList.size() != other.getBindingsList().size()) { - return false; + + @Override + public boolean equals(Object obj) { + if (obj == this) { + return true; + } + if (!(obj instanceof Policy)) { + return false; + } + Policy other = (Policy) obj; + if (bindingsList.size() != other.getBindingsList().size()) { + return false; + } + for (int i = 0; i < bindingsList.size(); ++i) { + bindingsList.get(i).equals(other.getBindingsList().get(i)); + } + return Objects.equals(etag, other.getEtag()) + && Objects.equals(version, other.getVersion()); } - for (int i = 0; i < bindingsList.size(); ++i) { - bindingsList.get(i).equals(other.getBindingsList().get(i)); + + /** + * Returns a builder for {@code Policy} objects. + */ + public static Builder newBuilder() { + return new Builder(); } - return Objects.equals(etag, other.getEtag()) - && Objects.equals(version, other.getVersion()); - } - - /** Returns a builder for {@code Policy} objects. */ - public static Builder newBuilder() { - return new Builder(); - } } diff --git a/google-cloud-core/src/test/java/com/google/cloud/PolicyTest.java b/google-cloud-core/src/test/java/com/google/cloud/PolicyTest.java index 33505a80b1..0e12ae7ce3 100644 --- a/google-cloud-core/src/test/java/com/google/cloud/PolicyTest.java +++ b/google-cloud-core/src/test/java/com/google/cloud/PolicyTest.java @@ -26,173 +26,175 @@ import com.google.cloud.Policy.DefaultMarshaller; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; + import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Set; + import org.junit.Test; public class PolicyTest { - private static final Identity ALL_USERS = Identity.allUsers(); - private static final Identity ALL_AUTH_USERS = Identity.allAuthenticatedUsers(); - private static final Identity USER = Identity.user("abc@gmail.com"); - private static final Identity SERVICE_ACCOUNT = - Identity.serviceAccount("service-account@gmail.com"); - private static final Identity GROUP = Identity.group("group@gmail.com"); - private static final Identity DOMAIN = Identity.domain("google.com"); - private static final Role VIEWER = Role.viewer(); - private static final Role EDITOR = Role.editor(); - private static final Role OWNER = Role.owner(); - private static final Map> BINDINGS = - ImmutableMap.of( - VIEWER, - ImmutableSet.of(USER, SERVICE_ACCOUNT, ALL_USERS), - EDITOR, - ImmutableSet.of(ALL_AUTH_USERS, GROUP, DOMAIN)); - private static final Policy SIMPLE_POLICY = - Policy.newBuilder() - .addIdentity(VIEWER, USER, SERVICE_ACCOUNT, ALL_USERS) - .addIdentity(EDITOR, ALL_AUTH_USERS, GROUP, DOMAIN) - .build(); - private static final Policy FULL_POLICY = - Policy.newBuilder() - .setBindings(SIMPLE_POLICY.getBindings()) - .setEtag("etag") - .setVersion(1) - .build(); + private static final Identity ALL_USERS = Identity.allUsers(); + private static final Identity ALL_AUTH_USERS = Identity.allAuthenticatedUsers(); + private static final Identity USER = Identity.user("abc@gmail.com"); + private static final Identity SERVICE_ACCOUNT = + Identity.serviceAccount("service-account@gmail.com"); + private static final Identity GROUP = Identity.group("group@gmail.com"); + private static final Identity DOMAIN = Identity.domain("google.com"); + private static final Role VIEWER = Role.viewer(); + private static final Role EDITOR = Role.editor(); + private static final Role OWNER = Role.owner(); + private static final Map> BINDINGS = + ImmutableMap.of( + VIEWER, + ImmutableSet.of(USER, SERVICE_ACCOUNT, ALL_USERS), + EDITOR, + ImmutableSet.of(ALL_AUTH_USERS, GROUP, DOMAIN)); + private static final Policy SIMPLE_POLICY = + Policy.newBuilder() + .addIdentity(VIEWER, USER, SERVICE_ACCOUNT, ALL_USERS) + .addIdentity(EDITOR, ALL_AUTH_USERS, GROUP, DOMAIN) + .build(); + private static final Policy FULL_POLICY = + Policy.newBuilder() + .setBindings(SIMPLE_POLICY.getBindings()) + .setEtag("etag") + .setVersion(1) + .build(); - @Test - public void testBuilder() { - assertEquals(BINDINGS, SIMPLE_POLICY.getBindings()); - assertEquals(null, SIMPLE_POLICY.getEtag()); - assertEquals(0, SIMPLE_POLICY.getVersion()); - assertEquals(BINDINGS, FULL_POLICY.getBindings()); - assertEquals("etag", FULL_POLICY.getEtag()); - assertEquals(1, FULL_POLICY.getVersion()); - Map> editorBinding = - ImmutableMap.>builder().put(EDITOR, BINDINGS.get(EDITOR)).build(); - Policy policy = FULL_POLICY.toBuilder().setBindings(editorBinding).build(); - assertEquals(editorBinding, policy.getBindings()); - assertEquals("etag", policy.getEtag()); - assertEquals(1, policy.getVersion()); - policy = SIMPLE_POLICY.toBuilder().removeRole(EDITOR).build(); - assertEquals(ImmutableMap.of(VIEWER, BINDINGS.get(VIEWER)), policy.getBindings()); - assertNull(policy.getEtag()); - assertEquals(0, policy.getVersion()); - policy = - policy - .toBuilder() - .removeIdentity(VIEWER, USER, ALL_USERS) - .addIdentity(VIEWER, DOMAIN, GROUP) - .build(); - assertEquals( - ImmutableMap.of(VIEWER, ImmutableSet.of(SERVICE_ACCOUNT, DOMAIN, GROUP)), - policy.getBindings()); - assertNull(policy.getEtag()); - assertEquals(0, policy.getVersion()); - policy = - Policy.newBuilder() - .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.getBindings()); - assertNull(policy.getEtag()); - assertEquals(0, policy.getVersion()); - } - - @Test - public void testIllegalPolicies() { - try { - Policy.newBuilder().addIdentity(null, USER); - fail("Null role should cause exception."); - } catch (NullPointerException ex) { - assertEquals("The role cannot be null.", ex.getMessage()); - } - try { - Policy.newBuilder().addIdentity(VIEWER, null, USER); - fail("Null identity should cause exception."); - } catch (NullPointerException ex) { - assertEquals("Null identities are not permitted.", ex.getMessage()); - } - try { - Policy.newBuilder().addIdentity(VIEWER, USER, (Identity[]) null); - fail("Null identity should cause exception."); - } catch (NullPointerException ex) { - assertEquals("Null identities are not permitted.", ex.getMessage()); - } - try { - Policy.newBuilder().setBindings((Map>)null); - fail("Null bindings map should cause exception."); - } catch (NullPointerException ex) { - assertEquals("The provided map of bindings cannot be null.", ex.getMessage()); - } - try { - Map> bindings = new HashMap<>(); - bindings.put(VIEWER, null); - Policy.newBuilder().setBindings(bindings); - fail("Null set of identities should cause exception."); - } catch (NullPointerException ex) { - assertEquals("A role cannot be assigned to a null set of identities.", ex.getMessage()); + @Test + public void testBuilder() { + assertEquals(BINDINGS, SIMPLE_POLICY.getBindings()); + assertEquals(null, SIMPLE_POLICY.getEtag()); + assertEquals(0, SIMPLE_POLICY.getVersion()); + assertEquals(BINDINGS, FULL_POLICY.getBindings()); + assertEquals("etag", FULL_POLICY.getEtag()); + assertEquals(1, FULL_POLICY.getVersion()); + Map> editorBinding = + ImmutableMap.>builder().put(EDITOR, BINDINGS.get(EDITOR)).build(); + Policy policy = FULL_POLICY.toBuilder().setBindings(editorBinding).build(); + assertEquals(editorBinding, policy.getBindings()); + assertEquals("etag", policy.getEtag()); + assertEquals(1, policy.getVersion()); + policy = SIMPLE_POLICY.toBuilder().removeRole(EDITOR).build(); + assertEquals(ImmutableMap.of(VIEWER, BINDINGS.get(VIEWER)), policy.getBindings()); + assertNull(policy.getEtag()); + assertEquals(0, policy.getVersion()); + policy = + policy + .toBuilder() + .removeIdentity(VIEWER, USER, ALL_USERS) + .addIdentity(VIEWER, DOMAIN, GROUP) + .build(); + assertEquals( + ImmutableMap.of(VIEWER, ImmutableSet.of(SERVICE_ACCOUNT, DOMAIN, GROUP)), + policy.getBindings()); + assertNull(policy.getEtag()); + assertEquals(0, policy.getVersion()); + policy = + Policy.newBuilder() + .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.getBindings()); + assertNull(policy.getEtag()); + assertEquals(0, policy.getVersion()); } - try { - Map> bindings = new HashMap<>(); - Set identities = new HashSet<>(); - identities.add(null); - bindings.put(VIEWER, identities); - Policy.newBuilder().setBindings(bindings); - fail("Null identity should cause exception."); - } catch (IllegalArgumentException ex) { - assertEquals("Null identities are not permitted.", ex.getMessage()); + + @Test + public void testIllegalPolicies() { + try { + Policy.newBuilder().addIdentity(null, USER); + fail("Null role should cause exception."); + } catch (NullPointerException ex) { + assertEquals("The role cannot be null.", ex.getMessage()); + } + try { + Policy.newBuilder().addIdentity(VIEWER, null, USER); + fail("Null identity should cause exception."); + } catch (NullPointerException ex) { + assertEquals("Null identities are not permitted.", ex.getMessage()); + } + try { + Policy.newBuilder().addIdentity(VIEWER, USER, (Identity[]) null); + fail("Null identity should cause exception."); + } catch (NullPointerException ex) { + assertEquals("Null identities are not permitted.", ex.getMessage()); + } + try { + Policy.newBuilder().setBindings((Map>) null); + fail("Null bindings map should cause exception."); + } catch (NullPointerException ex) { + assertEquals("The provided map of bindings cannot be null.", ex.getMessage()); + } + try { + Map> bindings = new HashMap<>(); + bindings.put(VIEWER, null); + Policy.newBuilder().setBindings(bindings); + fail("Null set of identities should cause exception."); + } catch (NullPointerException ex) { + assertEquals("A role cannot be assigned to a null set of identities.", ex.getMessage()); + } + try { + Map> bindings = new HashMap<>(); + Set identities = new HashSet<>(); + identities.add(null); + bindings.put(VIEWER, identities); + Policy.newBuilder().setBindings(bindings); + fail("Null identity should cause exception."); + } catch (IllegalArgumentException ex) { + assertEquals("Null identities are not permitted.", ex.getMessage()); + } } - } - @Test - public void testEqualsHashCode() { - assertNotNull(FULL_POLICY); - Policy emptyPolicy = Policy.newBuilder().build(); - Policy anotherPolicy = Policy.newBuilder().build(); - assertEquals(emptyPolicy, anotherPolicy); - assertEquals(emptyPolicy.hashCode(), anotherPolicy.hashCode()); - assertNotEquals(FULL_POLICY, SIMPLE_POLICY); - assertNotEquals(FULL_POLICY.hashCode(), SIMPLE_POLICY.hashCode()); - Policy copy = SIMPLE_POLICY.toBuilder().build(); - assertEquals(SIMPLE_POLICY, copy); - assertEquals(SIMPLE_POLICY.hashCode(), copy.hashCode()); - } + @Test + public void testEqualsHashCode() { + assertNotNull(FULL_POLICY); + Policy emptyPolicy = Policy.newBuilder().build(); + Policy anotherPolicy = Policy.newBuilder().build(); + assertEquals(emptyPolicy, anotherPolicy); + assertEquals(emptyPolicy.hashCode(), anotherPolicy.hashCode()); + assertNotEquals(FULL_POLICY, SIMPLE_POLICY); + assertNotEquals(FULL_POLICY.hashCode(), SIMPLE_POLICY.hashCode()); + Policy copy = SIMPLE_POLICY.toBuilder().build(); + assertEquals(SIMPLE_POLICY, copy); + assertEquals(SIMPLE_POLICY.hashCode(), copy.hashCode()); + } - @Test - public void testBindings() { - assertTrue(Policy.newBuilder().build().getBindings().isEmpty()); - assertEquals(BINDINGS, SIMPLE_POLICY.getBindings()); - } + @Test + public void testBindings() { + assertTrue(Policy.newBuilder().build().getBindings().isEmpty()); + assertEquals(BINDINGS, SIMPLE_POLICY.getBindings()); + } - @Test - public void testEtag() { - assertNull(SIMPLE_POLICY.getEtag()); - assertEquals("etag", FULL_POLICY.getEtag()); - } + @Test + public void testEtag() { + assertNull(SIMPLE_POLICY.getEtag()); + assertEquals("etag", FULL_POLICY.getEtag()); + } - @Test - public void testVersion() { - assertEquals(0, SIMPLE_POLICY.getVersion()); - assertEquals(1, FULL_POLICY.getVersion()); - } + @Test + public void testVersion() { + assertEquals(0, SIMPLE_POLICY.getVersion()); + assertEquals(1, FULL_POLICY.getVersion()); + } - @Test - public void testDefaultMarshaller() { - DefaultMarshaller marshaller = new DefaultMarshaller(); - Policy emptyPolicy = Policy.newBuilder().build(); - assertEquals(emptyPolicy, marshaller.fromPb(marshaller.toPb(emptyPolicy))); - assertEquals(SIMPLE_POLICY, marshaller.fromPb(marshaller.toPb(SIMPLE_POLICY))); - assertEquals(FULL_POLICY, marshaller.fromPb(marshaller.toPb(FULL_POLICY))); - com.google.iam.v1.Policy policyPb = com.google.iam.v1.Policy.getDefaultInstance(); - Policy policy = marshaller.fromPb(policyPb); - assertTrue(policy.getBindings().isEmpty()); - assertNull(policy.getEtag()); - assertEquals(0, policy.getVersion()); - } + @Test + public void testDefaultMarshaller() { + DefaultMarshaller marshaller = new DefaultMarshaller(); + Policy emptyPolicy = Policy.newBuilder().build(); + assertEquals(emptyPolicy, marshaller.fromPb(marshaller.toPb(emptyPolicy))); + assertEquals(SIMPLE_POLICY, marshaller.fromPb(marshaller.toPb(SIMPLE_POLICY))); + assertEquals(FULL_POLICY, marshaller.fromPb(marshaller.toPb(FULL_POLICY))); + com.google.iam.v1.Policy policyPb = com.google.iam.v1.Policy.getDefaultInstance(); + Policy policy = marshaller.fromPb(policyPb); + assertTrue(policy.getBindings().isEmpty()); + assertNull(policy.getEtag()); + assertEquals(0, policy.getVersion()); + } } diff --git a/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java b/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java index 9305bd7d3e..5ebab48a2e 100644 --- a/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java +++ b/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java @@ -28,232 +28,232 @@ public class PolicyV3Test { - private static final String ALL_USERS = "allUsers"; - private static final String ALL_AUTH_USERS = "allAuthenticatedUsers"; - private static final String USER = "user:abc@gmail.com"; - private static final String SERVICE_ACCOUNT = "serviceAccount:service-account@gmail.com"; - private static final String GROUP = "group:group@gmail.com"; - private static final String DOMAIN = "domain:google.com"; - private static final String VIEWER = "roles/viewer"; - private static final String EDITOR = "roles/editor"; - private static final String OWNER = "roles/owner"; - private static final List MEMBERS_LIST_1 = ImmutableList.of(USER, SERVICE_ACCOUNT, ALL_USERS); - private static final List MEMBERS_LIST_2 = ImmutableList.of(ALL_AUTH_USERS, GROUP, DOMAIN); - private static final List BINDINGS_NO_CONDITIONS = - ImmutableList.of( - Binding.newBuilder() - .setRole(VIEWER) - .setMembers(MEMBERS_LIST_1).build(), - Binding.newBuilder() - .setRole(EDITOR) - .setMembers(MEMBERS_LIST_2).build()); - private static final List BINDINGS_WITH_CONDITIONS = - ImmutableList.copyOf(BINDINGS_NO_CONDITIONS).of( - Binding.newBuilder() - .setRole(VIEWER) - .setMembers(MEMBERS_LIST_1) - .setCondition(Condition.newBuilder() - .setTitle("Condition") - .setDescription("Condition") - .setExpression("Expr") - .build()) - .build(), - Binding.newBuilder() - .setRole(EDITOR) - .setMembers(MEMBERS_LIST_2).build()); - private static final Policy FULL_POLICY_V1 = - Policy.newBuilder() - .setBindings(BINDINGS_NO_CONDITIONS) - .setEtag("etag") - .setVersion(1) - .build(); + private static final String ALL_USERS = "allUsers"; + private static final String ALL_AUTH_USERS = "allAuthenticatedUsers"; + private static final String USER = "user:abc@gmail.com"; + private static final String SERVICE_ACCOUNT = "serviceAccount:service-account@gmail.com"; + private static final String GROUP = "group:group@gmail.com"; + private static final String DOMAIN = "domain:google.com"; + private static final String VIEWER = "roles/viewer"; + private static final String EDITOR = "roles/editor"; + private static final String OWNER = "roles/owner"; + private static final List MEMBERS_LIST_1 = ImmutableList.of(USER, SERVICE_ACCOUNT, ALL_USERS); + private static final List MEMBERS_LIST_2 = ImmutableList.of(ALL_AUTH_USERS, GROUP, DOMAIN); + private static final List BINDINGS_NO_CONDITIONS = + ImmutableList.of( + Binding.newBuilder() + .setRole(VIEWER) + .setMembers(MEMBERS_LIST_1).build(), + Binding.newBuilder() + .setRole(EDITOR) + .setMembers(MEMBERS_LIST_2).build()); + private static final List BINDINGS_WITH_CONDITIONS = + ImmutableList.copyOf(BINDINGS_NO_CONDITIONS).of( + Binding.newBuilder() + .setRole(VIEWER) + .setMembers(MEMBERS_LIST_1) + .setCondition(Condition.newBuilder() + .setTitle("Condition") + .setDescription("Condition") + .setExpression("Expr") + .build()) + .build(), + Binding.newBuilder() + .setRole(EDITOR) + .setMembers(MEMBERS_LIST_2).build()); + private static final Policy FULL_POLICY_V1 = + Policy.newBuilder() + .setBindings(BINDINGS_NO_CONDITIONS) + .setEtag("etag") + .setVersion(1) + .build(); - private static final Policy FULL_POLICY_V3 = - Policy.newBuilder() - .setBindings(BINDINGS_WITH_CONDITIONS) - .setEtag("etag") - .setVersion(3) - .build(); + private static final Policy FULL_POLICY_V3 = + Policy.newBuilder() + .setBindings(BINDINGS_WITH_CONDITIONS) + .setEtag("etag") + .setVersion(3) + .build(); - private static final Policy FULL_POLICY_V3_WITH_VERSION_1 = - Policy.newBuilder() - .setBindings(BINDINGS_WITH_CONDITIONS) - .setEtag("etag") - .setVersion(1) - .build(); + private static final Policy FULL_POLICY_V3_WITH_VERSION_1 = + Policy.newBuilder() + .setBindings(BINDINGS_WITH_CONDITIONS) + .setEtag("etag") + .setVersion(1) + .build(); - @Test - public void testBuilder() { - assertEquals(BINDINGS_NO_CONDITIONS, FULL_POLICY_V1.getBindingsList()); - assertEquals(1, FULL_POLICY_V1.getVersion()); - assertEquals("etag", FULL_POLICY_V1.getEtag()); - Policy policy = FULL_POLICY_V1.toBuilder().setBindings(BINDINGS_NO_CONDITIONS).build(); - assertEquals(BINDINGS_NO_CONDITIONS, policy.getBindingsList()); - assertEquals("etag", policy.getEtag()); - assertEquals(1, policy.getVersion()); + @Test + public void testBuilder() { + assertEquals(BINDINGS_NO_CONDITIONS, FULL_POLICY_V1.getBindingsList()); + assertEquals(1, FULL_POLICY_V1.getVersion()); + assertEquals("etag", FULL_POLICY_V1.getEtag()); + Policy policy = FULL_POLICY_V1.toBuilder().setBindings(BINDINGS_NO_CONDITIONS).build(); + assertEquals(BINDINGS_NO_CONDITIONS, policy.getBindingsList()); + assertEquals("etag", policy.getEtag()); + assertEquals(1, policy.getVersion()); - assertEquals(BINDINGS_WITH_CONDITIONS, FULL_POLICY_V3.getBindingsList()); - assertEquals(3, FULL_POLICY_V3.getVersion()); - assertEquals("etag", FULL_POLICY_V3.getEtag()); - policy = FULL_POLICY_V3.toBuilder().setBindings(BINDINGS_WITH_CONDITIONS).build(); - assertEquals(BINDINGS_WITH_CONDITIONS, policy.getBindingsList()); - assertEquals("etag", policy.getEtag()); - assertEquals(3, policy.getVersion()); + assertEquals(BINDINGS_WITH_CONDITIONS, FULL_POLICY_V3.getBindingsList()); + assertEquals(3, FULL_POLICY_V3.getVersion()); + assertEquals("etag", FULL_POLICY_V3.getEtag()); + policy = FULL_POLICY_V3.toBuilder().setBindings(BINDINGS_WITH_CONDITIONS).build(); + assertEquals(BINDINGS_WITH_CONDITIONS, policy.getBindingsList()); + assertEquals("etag", policy.getEtag()); + assertEquals(3, policy.getVersion()); + + assertEquals(BINDINGS_WITH_CONDITIONS, FULL_POLICY_V3_WITH_VERSION_1.getBindingsList()); + assertEquals(1, FULL_POLICY_V3_WITH_VERSION_1.getVersion()); + assertEquals("etag", FULL_POLICY_V3_WITH_VERSION_1.getEtag()); + policy = FULL_POLICY_V3_WITH_VERSION_1.toBuilder().setBindings(BINDINGS_WITH_CONDITIONS).setVersion(3).build(); + assertEquals(BINDINGS_WITH_CONDITIONS, policy.getBindingsList()); + assertEquals("etag", policy.getEtag()); + assertEquals(3, policy.getVersion()); + } - assertEquals(BINDINGS_WITH_CONDITIONS, FULL_POLICY_V3_WITH_VERSION_1.getBindingsList()); - assertEquals(1, FULL_POLICY_V3_WITH_VERSION_1.getVersion()); - assertEquals("etag", FULL_POLICY_V3_WITH_VERSION_1.getEtag()); - policy = FULL_POLICY_V3_WITH_VERSION_1.toBuilder().setBindings(BINDINGS_WITH_CONDITIONS).setVersion(3).build(); - assertEquals(BINDINGS_WITH_CONDITIONS, policy.getBindingsList()); - assertEquals("etag", policy.getEtag()); - assertEquals(3, policy.getVersion()); - } + @Test + public void removeMemberFromPolicy() { + assertEquals(3, FULL_POLICY_V3.getBindingsList().get(0).getMembers().size()); + List bindings = new ArrayList<>(FULL_POLICY_V3.getBindingsList()); - @Test - public void removeMemberFromPolicy() { - assertEquals(3, FULL_POLICY_V3.getBindingsList().get(0).getMembers().size()); - List bindings = new ArrayList<>(FULL_POLICY_V3.getBindingsList()); + for (int i = 0; i < bindings.size(); ++i) { + Binding binding = bindings.get(i); + if (binding.getRole().equals(VIEWER)) { + bindings.set(i, binding.toBuilder().removeMembers(ALL_USERS).build()); + break; + } + } - for (int i = 0; i < bindings.size(); ++i) { - Binding binding = bindings.get(i); - if (binding.getRole().equals(VIEWER)) { - bindings.set(i, binding.toBuilder().removeMembers(ALL_USERS).build()); - break; - } + Policy updatedPolicy = FULL_POLICY_V3.toBuilder().setBindings(bindings).build(); + assertEquals(2, updatedPolicy.getBindingsList().get(0).getMembers().size()); } - Policy updatedPolicy = FULL_POLICY_V3.toBuilder().setBindings(bindings).build(); - assertEquals(2, updatedPolicy.getBindingsList().get(0).getMembers().size()); - } + @Test + public void addMemberFromPolicy() { + assertEquals(3, FULL_POLICY_V3.getBindingsList().get(0).getMembers().size()); + List bindings = new ArrayList<>(FULL_POLICY_V3.getBindingsList()); - @Test - public void addMemberFromPolicy() { - assertEquals(3, FULL_POLICY_V3.getBindingsList().get(0).getMembers().size()); - List bindings = new ArrayList<>(FULL_POLICY_V3.getBindingsList()); + for (int i = 0; i < bindings.size(); ++i) { + Binding binding = bindings.get(i); + if (binding.getRole().equals(VIEWER)) { + bindings.set(i, binding.toBuilder().addMembers("user:example@example.com").build()); + } + } - for (int i = 0; i < bindings.size(); ++i) { - Binding binding = bindings.get(i); - if (binding.getRole().equals(VIEWER)) { - bindings.set(i, binding.toBuilder().addMembers("user:example@example.com").build()); - } + Policy updatedPolicy = FULL_POLICY_V3.toBuilder().setBindings(bindings).build(); + assertEquals(4, updatedPolicy.getBindingsList().get(0).getMembers().size()); } - Policy updatedPolicy = FULL_POLICY_V3.toBuilder().setBindings(bindings).build(); - assertEquals(4, updatedPolicy.getBindingsList().get(0).getMembers().size()); - } + @Test + public void removeBindingFromPolicy() { + assertEquals(2, FULL_POLICY_V3.getBindingsList().size()); + List bindings = new ArrayList<>(FULL_POLICY_V3.getBindingsList()); - @Test - public void removeBindingFromPolicy() { - assertEquals(2, FULL_POLICY_V3.getBindingsList().size()); - List bindings = new ArrayList<>(FULL_POLICY_V3.getBindingsList()); + for (int i = 0; i < bindings.size(); ++i) { + Binding binding = bindings.get(i); + if (binding.getRole().equals(EDITOR) && binding.getCondition() == null) { + bindings.remove(i); + break; + } + } - for (int i = 0; i < bindings.size(); ++i) { - Binding binding = bindings.get(i); - if (binding.getRole().equals(EDITOR) && binding.getCondition() == null) { - bindings.remove(i); - break; - } + Policy updatedPolicy = FULL_POLICY_V3.toBuilder().setBindings(bindings).build(); + assertEquals(1, updatedPolicy.getBindingsList().size()); } - Policy updatedPolicy = FULL_POLICY_V3.toBuilder().setBindings(bindings).build(); - assertEquals(1, updatedPolicy.getBindingsList().size()); - } - - @Test - public void addBindingToPolicy() { - assertEquals(2, FULL_POLICY_V3.getBindingsList().size()); - List bindings = new ArrayList<>(FULL_POLICY_V3.getBindingsList()); - bindings.add(Binding.newBuilder().setRole(OWNER).addMembers(USER).build()); - Policy updatedPolicy = FULL_POLICY_V3.toBuilder().setBindings(bindings).build(); - assertEquals(3, updatedPolicy.getBindingsList().size()); - } + @Test + public void addBindingToPolicy() { + assertEquals(2, FULL_POLICY_V3.getBindingsList().size()); + List bindings = new ArrayList<>(FULL_POLICY_V3.getBindingsList()); + bindings.add(Binding.newBuilder().setRole(OWNER).addMembers(USER).build()); + Policy updatedPolicy = FULL_POLICY_V3.toBuilder().setBindings(bindings).build(); + assertEquals(3, updatedPolicy.getBindingsList().size()); + } - @Test - public void testIllegalPolicies() { - try { - Binding.newBuilder().setRole(null); - fail("Null role should cause exception."); - } catch (NullPointerException ex) { - assertEquals("The role cannot be null.", ex.getMessage()); - } - try { - Binding.newBuilder().addMembers(null); - fail("Null member should cause exception."); - } catch (NullPointerException ex) { - assertEquals("Null identities are not permitted.", ex.getMessage()); - } - try { - FULL_POLICY_V3.getBindings(); - fail("getBindings() should cause exception with Policy V3."); - } catch (IllegalArgumentException ex) { - assertEquals("getBindings() is only supported with version 1 policies and non-conditional policies", ex.getMessage()); - } - try { - FULL_POLICY_V3.toBuilder().addIdentity(Role.editor(), Identity.allUsers()); - fail("getBindings() should cause exception with Policy V3."); - } catch (IllegalArgumentException ex) { - assertEquals("addIdentity() is only supported with version 1 policies and non-conditional policies", ex.getMessage()); + @Test + public void testIllegalPolicies() { + try { + Binding.newBuilder().setRole(null); + fail("Null role should cause exception."); + } catch (NullPointerException ex) { + assertEquals("The role cannot be null.", ex.getMessage()); + } + try { + Binding.newBuilder().addMembers(null); + fail("Null member should cause exception."); + } catch (NullPointerException ex) { + assertEquals("Null identities are not permitted.", ex.getMessage()); + } + try { + FULL_POLICY_V3.getBindings(); + fail("getBindings() should cause exception with Policy V3."); + } catch (IllegalArgumentException ex) { + assertEquals("getBindings() is only supported with version 1 policies and non-conditional policies", ex.getMessage()); + } + try { + FULL_POLICY_V3.toBuilder().addIdentity(Role.editor(), Identity.allUsers()); + fail("getBindings() should cause exception with Policy V3."); + } catch (IllegalArgumentException ex) { + assertEquals("addIdentity() is only supported with version 1 policies and non-conditional policies", ex.getMessage()); + } + try { + FULL_POLICY_V3.toBuilder().removeIdentity(Role.editor(), Identity.allUsers()); + fail("getBindings() should cause exception with Policy V3."); + } catch (IllegalArgumentException ex) { + assertEquals("removeIdentity() is only supported with version 1 policies and non-conditional policies", ex.getMessage()); + } + try { + FULL_POLICY_V3.toBuilder().setBindings(FULL_POLICY_V1.getBindings()); + fail("getBindings() should cause exception with Policy V3."); + } catch (IllegalArgumentException ex) { + assertEquals("setBindings() is only supported with version 1 policies and non-conditional policies", ex.getMessage()); + } } - try { - FULL_POLICY_V3.toBuilder().removeIdentity(Role.editor(), Identity.allUsers()); - fail("getBindings() should cause exception with Policy V3."); - } catch (IllegalArgumentException ex) { - assertEquals("removeIdentity() is only supported with version 1 policies and non-conditional policies", ex.getMessage()); - } - try { - FULL_POLICY_V3.toBuilder().setBindings(FULL_POLICY_V1.getBindings()); - fail("getBindings() should cause exception with Policy V3."); - } catch (IllegalArgumentException ex) { - assertEquals("setBindings() is only supported with version 1 policies and non-conditional policies", ex.getMessage()); - } - } - @Test - public void testEqualsHashCode() { - assertNotNull(FULL_POLICY_V3); - Policy emptyPolicy = Policy.newBuilder().build(); - Policy anotherPolicy = Policy.newBuilder().build(); - assertEquals(emptyPolicy, anotherPolicy); - assertEquals(emptyPolicy.hashCode(), anotherPolicy.hashCode()); - assertNotEquals(FULL_POLICY_V3, FULL_POLICY_V1); - assertNotEquals(FULL_POLICY_V3.hashCode(), FULL_POLICY_V1.hashCode()); - Policy copy = FULL_POLICY_V1.toBuilder().build(); - assertEquals(FULL_POLICY_V1, copy); - assertEquals(FULL_POLICY_V1.hashCode(), copy.hashCode()); - } + @Test + public void testEqualsHashCode() { + assertNotNull(FULL_POLICY_V3); + Policy emptyPolicy = Policy.newBuilder().build(); + Policy anotherPolicy = Policy.newBuilder().build(); + assertEquals(emptyPolicy, anotherPolicy); + assertEquals(emptyPolicy.hashCode(), anotherPolicy.hashCode()); + assertNotEquals(FULL_POLICY_V3, FULL_POLICY_V1); + assertNotEquals(FULL_POLICY_V3.hashCode(), FULL_POLICY_V1.hashCode()); + Policy copy = FULL_POLICY_V1.toBuilder().build(); + assertEquals(FULL_POLICY_V1, copy); + assertEquals(FULL_POLICY_V1.hashCode(), copy.hashCode()); + } - @Test - public void testBindings() { - assertTrue(Policy.newBuilder().build().getBindingsList().isEmpty()); - assertEquals(BINDINGS_WITH_CONDITIONS, FULL_POLICY_V3.getBindingsList()); - } + @Test + public void testBindings() { + assertTrue(Policy.newBuilder().build().getBindingsList().isEmpty()); + assertEquals(BINDINGS_WITH_CONDITIONS, FULL_POLICY_V3.getBindingsList()); + } - @Test - public void testEtag() { - assertNotNull(FULL_POLICY_V3.getEtag()); - assertEquals("etag", FULL_POLICY_V3.getEtag()); - } + @Test + public void testEtag() { + assertNotNull(FULL_POLICY_V3.getEtag()); + assertEquals("etag", FULL_POLICY_V3.getEtag()); + } - @Test - public void testVersion() { - assertEquals(1, FULL_POLICY_V1.getVersion()); - assertEquals(3, FULL_POLICY_V3.getVersion()); - assertEquals(1, FULL_POLICY_V3_WITH_VERSION_1.getVersion()); - } + @Test + public void testVersion() { + assertEquals(1, FULL_POLICY_V1.getVersion()); + assertEquals(3, FULL_POLICY_V3.getVersion()); + assertEquals(1, FULL_POLICY_V3_WITH_VERSION_1.getVersion()); + } - @Test - public void testDefaultMarshaller() { - DefaultMarshaller marshaller = new DefaultMarshaller(); - Policy emptyPolicy = Policy.newBuilder().build(); - assertEquals(emptyPolicy, marshaller.fromPb(marshaller.toPb(emptyPolicy))); - assertEquals(FULL_POLICY_V3, marshaller.fromPb(marshaller.toPb(FULL_POLICY_V3))); - assertEquals(FULL_POLICY_V1, marshaller.fromPb(marshaller.toPb(FULL_POLICY_V1))); - com.google.iam.v1.Policy policyPb = com.google.iam.v1.Policy.getDefaultInstance(); - Policy policy = marshaller.fromPb(policyPb); - assertTrue(policy.getBindingsList().isEmpty()); - assertNull(policy.getEtag()); - assertEquals(0, policy.getVersion()); - } + @Test + public void testDefaultMarshaller() { + DefaultMarshaller marshaller = new DefaultMarshaller(); + Policy emptyPolicy = Policy.newBuilder().build(); + assertEquals(emptyPolicy, marshaller.fromPb(marshaller.toPb(emptyPolicy))); + assertEquals(FULL_POLICY_V3, marshaller.fromPb(marshaller.toPb(FULL_POLICY_V3))); + assertEquals(FULL_POLICY_V1, marshaller.fromPb(marshaller.toPb(FULL_POLICY_V1))); + com.google.iam.v1.Policy policyPb = com.google.iam.v1.Policy.getDefaultInstance(); + Policy policy = marshaller.fromPb(policyPb); + assertTrue(policy.getBindingsList().isEmpty()); + assertNull(policy.getEtag()); + assertEquals(0, policy.getVersion()); + } } From ac1fd6ee2482aa42e23571a2457d46f3385658f1 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Tue, 7 Jan 2020 10:37:55 -0800 Subject: [PATCH 04/31] correct copyright date --- google-cloud-core/src/main/java/com/google/cloud/Binding.java | 2 +- google-cloud-core/src/main/java/com/google/cloud/Condition.java | 2 +- .../src/test/java/com/google/cloud/PolicyV3Test.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/google-cloud-core/src/main/java/com/google/cloud/Binding.java b/google-cloud-core/src/main/java/com/google/cloud/Binding.java index 827c30012d..d9a28f89f6 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Binding.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Binding.java @@ -1,5 +1,5 @@ /* - * Copyright 2019 Google LLC + * Copyright 2020 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/google-cloud-core/src/main/java/com/google/cloud/Condition.java b/google-cloud-core/src/main/java/com/google/cloud/Condition.java index 4f4a8b8afb..550b001dc5 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Condition.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Condition.java @@ -1,5 +1,5 @@ /* - * Copyright 2019 Google LLC + * Copyright 2020 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java b/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java index 5ebab48a2e..e4f10b5e72 100644 --- a/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java +++ b/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java @@ -1,5 +1,5 @@ /* - * Copyright 2016 Google LLC + * Copyright 2020 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. From 2c8724bfb7ff813e956ab8c22c027986dbf8aacf Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Tue, 7 Jan 2020 10:50:10 -0800 Subject: [PATCH 05/31] lint --- .../main/java/com/google/cloud/Binding.java | 220 +++--- .../main/java/com/google/cloud/Condition.java | 162 ++--- .../main/java/com/google/cloud/Policy.java | 668 +++++++++--------- .../java/com/google/cloud/PolicyTest.java | 308 ++++---- .../java/com/google/cloud/PolicyV3Test.java | 419 +++++------ 5 files changed, 875 insertions(+), 902 deletions(-) diff --git a/google-cloud-core/src/main/java/com/google/cloud/Binding.java b/google-cloud-core/src/main/java/com/google/cloud/Binding.java index d9a28f89f6..91325c3228 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Binding.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Binding.java @@ -21,137 +21,133 @@ import com.google.api.core.InternalApi; import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableList; - -import java.util.*; +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; public class Binding { + private String role; + private List members; + private Condition condition; + + public static class Builder { + private List members = new ArrayList(); private String role; - private List members; private Condition condition; - public static class Builder { - private List members = new ArrayList(); - private String role; - private Condition condition; - - @InternalApi("This class should only be extended within google-cloud-java") - protected Builder() { - } - - @InternalApi("This class should only be extended within google-cloud-java") - protected Builder(Binding binding) { - setRole(binding.role); - setMembers(binding.members); - setCondition(binding.condition); - } - - public final Binding.Builder setRole(String role) { - String nullIdentityMessage = "The role cannot be null."; - checkNotNull(role, nullIdentityMessage); - this.role = role; - return this; - } - - public final Binding.Builder setMembers(List members) { - String nullIdentityMessage = "Null members are not permitted."; - checkNotNull(members, nullIdentityMessage); - this.members.clear(); - for (String member : members) { - // Check member not null - this.members.add(member); - } - return this; - } - - public final Binding.Builder removeMembers(String first, String... others) { - String nullIdentityMessage = "Null members are not permitted."; - checkNotNull(first, nullIdentityMessage); - checkNotNull(others, nullIdentityMessage); - this.members.remove(first); - for (String member : others) { - this.members.remove(member); - } - return this; - } - - public final Binding.Builder addMembers(String first, String... others) { - String nullIdentityMessage = "Null identities are not permitted."; - checkNotNull(first, nullIdentityMessage); - checkNotNull(others, nullIdentityMessage); - this.members.add(first); - for (String member : others) { - this.members.add(member); - } - return this; - } - - public final Binding.Builder setCondition(Condition condition) { - this.condition = condition; - return this; - } - - /** - * Creates a {@code Policy} object. - */ - public final Binding build() { - return new Binding(this); - } - } + @InternalApi("This class should only be extended within google-cloud-java") + protected Builder() {} - private Binding(Binding.Builder builder) { - this.role = builder.role; - this.members = ImmutableList.copyOf(builder.members); - this.condition = builder.condition; + @InternalApi("This class should only be extended within google-cloud-java") + protected Builder(Binding binding) { + setRole(binding.role); + setMembers(binding.members); + setCondition(binding.condition); } - public Binding.Builder toBuilder() { - return new Binding.Builder(this); + public final Binding.Builder setRole(String role) { + String nullIdentityMessage = "The role cannot be null."; + checkNotNull(role, nullIdentityMessage); + this.role = role; + return this; } - public String getRole() { - return this.role; + public final Binding.Builder setMembers(List members) { + String nullIdentityMessage = "Null members are not permitted."; + checkNotNull(members, nullIdentityMessage); + this.members.clear(); + for (String member : members) { + // Check member not null + this.members.add(member); + } + return this; } - public List getMembers() { - return this.members; + public final Binding.Builder removeMembers(String first, String... others) { + String nullIdentityMessage = "Null members are not permitted."; + checkNotNull(first, nullIdentityMessage); + checkNotNull(others, nullIdentityMessage); + this.members.remove(first); + for (String member : others) { + this.members.remove(member); + } + return this; } - public Condition getCondition() { - return this.condition; + public final Binding.Builder addMembers(String first, String... others) { + String nullIdentityMessage = "Null identities are not permitted."; + checkNotNull(first, nullIdentityMessage); + checkNotNull(others, nullIdentityMessage); + this.members.add(first); + for (String member : others) { + this.members.add(member); + } + return this; } - @Override - public String toString() { - return MoreObjects.toStringHelper(this) - .add("role", role) - .add("members", members) - .add("condition", condition) - .toString(); + public final Binding.Builder setCondition(Condition condition) { + this.condition = condition; + return this; } - @Override - public int hashCode() { - return Objects.hash(getClass(), role, members, condition); + /** Creates a {@code Policy} object. */ + public final Binding build() { + return new Binding(this); } - - @Override - public boolean equals(Object obj) { - if (obj == this) { - return true; - } - if (!(obj instanceof Binding)) { - return false; - } - Binding other = (Binding) obj; - return Objects.equals(role, other.getRole()) - && Objects.equals(members, other.getMembers()) - && Objects.equals(condition, other.getCondition()); + } + + private Binding(Binding.Builder builder) { + this.role = builder.role; + this.members = ImmutableList.copyOf(builder.members); + this.condition = builder.condition; + } + + public Binding.Builder toBuilder() { + return new Binding.Builder(this); + } + + public String getRole() { + return this.role; + } + + public List getMembers() { + return this.members; + } + + public Condition getCondition() { + return this.condition; + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("role", role) + .add("members", members) + .add("condition", condition) + .toString(); + } + + @Override + public int hashCode() { + return Objects.hash(getClass(), role, members, condition); + } + + @Override + public boolean equals(Object obj) { + if (obj == this) { + return true; } - - /** - * Returns a builder for {@code Policy} objects. - */ - public static Binding.Builder newBuilder() { - return new Binding.Builder(); + if (!(obj instanceof Binding)) { + return false; } + Binding other = (Binding) obj; + return Objects.equals(role, other.getRole()) + && Objects.equals(members, other.getMembers()) + && Objects.equals(condition, other.getCondition()); + } + + /** Returns a builder for {@code Policy} objects. */ + public static Binding.Builder newBuilder() { + return new Binding.Builder(); + } } diff --git a/google-cloud-core/src/main/java/com/google/cloud/Condition.java b/google-cloud-core/src/main/java/com/google/cloud/Condition.java index 550b001dc5..854ea64e63 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Condition.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Condition.java @@ -18,107 +18,101 @@ import com.google.api.core.InternalApi; import com.google.common.base.MoreObjects; - import java.util.Objects; public class Condition { + private String title; + private String description; + private String expression; + + public static class Builder { private String title; private String description; private String expression; - public static class Builder { - private String title; - private String description; - private String expression; - - @InternalApi("This class should only be extended within google-cloud-java") - protected Builder() { - } - - @InternalApi("This class should only be extended within google-cloud-java") - protected Builder(Condition condition) { - this.title = condition.title; - this.description = condition.description; - this.expression = condition.expression; - } - - public final Condition.Builder setTitle(String title) { - this.title = title; - return this; - } - - public final Condition.Builder setDescription(String description) { - this.description = description; - return this; - } - - public final Condition.Builder setExpression(String expression) { - this.expression = expression; - return this; - } - - /** - * Creates a {@code Condition} object. - */ - public final Condition build() { - return new Condition(this); - } - } + @InternalApi("This class should only be extended within google-cloud-java") + protected Builder() {} - private Condition(Condition.Builder builder) { - this.title = builder.title; - this.description = builder.description; - this.expression = builder.expression; + @InternalApi("This class should only be extended within google-cloud-java") + protected Builder(Condition condition) { + this.title = condition.title; + this.description = condition.description; + this.expression = condition.expression; } - public Condition.Builder toBuilder() { - return new Condition.Builder(this); + public final Condition.Builder setTitle(String title) { + this.title = title; + return this; } - public String getTitle() { - return title; + public final Condition.Builder setDescription(String description) { + this.description = description; + return this; } - public String getDescription() { - return description; + public final Condition.Builder setExpression(String expression) { + this.expression = expression; + return this; } - public String getExpression() { - return expression; + /** Creates a {@code Condition} object. */ + public final Condition build() { + return new Condition(this); } - - @Override - public String toString() { - return MoreObjects.toStringHelper(this) - .add("title", title) - .add("description", description) - .add("expression", expression) - .toString(); + } + + private Condition(Condition.Builder builder) { + this.title = builder.title; + this.description = builder.description; + this.expression = builder.expression; + } + + public Condition.Builder toBuilder() { + return new Condition.Builder(this); + } + + public String getTitle() { + return title; + } + + public String getDescription() { + return description; + } + + public String getExpression() { + return expression; + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("title", title) + .add("description", description) + .add("expression", expression) + .toString(); + } + + @Override + public int hashCode() { + return Objects.hash(getClass(), title, description, expression); + } + + @Override + public boolean equals(Object obj) { + if (obj == this) { + return true; } - - @Override - public int hashCode() { - return Objects.hash(getClass(), title, description, expression); - } - - @Override - public boolean equals(Object obj) { - if (obj == this) { - return true; - } - if (!(obj instanceof Condition)) { - return false; - } - Condition other = (Condition) obj; - return Objects.equals(title, other.getTitle()) - && Objects.equals(description, other.getDescription()) - && Objects.equals(expression, other.getExpression()); - } - - /** - * Returns a builder for {@code Policy} objects. - */ - public static Condition.Builder newBuilder() { - return new Condition.Builder(); + if (!(obj instanceof Condition)) { + return false; } + Condition other = (Condition) obj; + return Objects.equals(title, other.getTitle()) + && Objects.equals(description, other.getDescription()) + && Objects.equals(expression, other.getExpression()); + } + + /** Returns a builder for {@code Policy} objects. */ + public static Condition.Builder newBuilder() { + return new Condition.Builder(); + } } diff --git a/google-cloud-core/src/main/java/com/google/cloud/Policy.java b/google-cloud-core/src/main/java/com/google/cloud/Policy.java index 9de35bfc97..1214ead85a 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Policy.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Policy.java @@ -19,21 +19,16 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; -import com.google.api.core.ApiFunction; import com.google.api.core.InternalApi; -import com.google.common.base.Function; import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Lists; import com.google.common.io.BaseEncoding; import com.google.protobuf.ByteString; import com.google.type.Expr; - import java.io.Serializable; import java.util.ArrayList; -import java.util.HashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -50,388 +45,375 @@ */ public final class Policy implements Serializable { - private static final long serialVersionUID = -3348914530232544290L; - private final List bindingsList; - private final String etag; - private final int version; - - /* - * Return false if Policy is version 3 OR bindings has a conditional binding. - * Return true if Policy is version 1 AND bindings does not have a conditional binding. - */ - private static boolean checkVersion(int version, List bindingsList) { - for (Binding binding : bindingsList) { - if (binding.getCondition() != null) { - return false; - } - } - if (version > 1) { - return false; - } - return true; + private static final long serialVersionUID = -3348914530232544290L; + private final List bindingsList; + private final String etag; + private final int version; + + /* + * Return false if Policy is version 3 OR bindings has a conditional binding. + * Return true if Policy is version 1 AND bindings does not have a conditional binding. + */ + private static boolean checkVersion(int version, List bindingsList) { + for (Binding binding : bindingsList) { + if (binding.getCondition() != null) { + return false; + } + } + if (version > 1) { + return false; } + return true; + } - public abstract static class Marshaller { + public abstract static class Marshaller { - @InternalApi("This class should only be extended within google-cloud-java") - protected Marshaller() { - } + @InternalApi("This class should only be extended within google-cloud-java") + protected Marshaller() {} + protected abstract Policy fromPb(T policyPb); - protected abstract Policy fromPb(T policyPb); + protected abstract T toPb(Policy policy); + } - protected abstract T toPb(Policy policy); - } + public static class DefaultMarshaller extends Marshaller { - public static class DefaultMarshaller extends Marshaller { - - @Override - protected Policy fromPb(com.google.iam.v1.Policy policyPb) { - List bindingsList = new ArrayList(); - for (com.google.iam.v1.Binding bindingPb : policyPb.getBindingsList()) { - Binding.Builder convertedBinding = Binding.newBuilder().setRole(bindingPb.getRole()) - .setMembers(ImmutableList.copyOf(bindingPb.getMembersList())); - if (bindingPb.hasCondition()) { - Expr expr = bindingPb.getCondition(); - convertedBinding.setCondition(Condition.newBuilder() - .setTitle(expr.getTitle()) - .setDescription(expr.getDescription()) - .setExpression(expr.getExpression()) - .build()); - } - bindingsList.add(convertedBinding.build()); - } - return newBuilder() - .setBindings(bindingsList) - .setEtag( - policyPb.getEtag().isEmpty() - ? null - : BaseEncoding.base64().encode(policyPb.getEtag().toByteArray())) - .setVersion(policyPb.getVersion()) - .build(); + @Override + protected Policy fromPb(com.google.iam.v1.Policy policyPb) { + List bindingsList = new ArrayList(); + for (com.google.iam.v1.Binding bindingPb : policyPb.getBindingsList()) { + Binding.Builder convertedBinding = + Binding.newBuilder() + .setRole(bindingPb.getRole()) + .setMembers(ImmutableList.copyOf(bindingPb.getMembersList())); + if (bindingPb.hasCondition()) { + Expr expr = bindingPb.getCondition(); + convertedBinding.setCondition( + Condition.newBuilder() + .setTitle(expr.getTitle()) + .setDescription(expr.getDescription()) + .setExpression(expr.getExpression()) + .build()); } + bindingsList.add(convertedBinding.build()); + } + return newBuilder() + .setBindings(bindingsList) + .setEtag( + policyPb.getEtag().isEmpty() + ? null + : BaseEncoding.base64().encode(policyPb.getEtag().toByteArray())) + .setVersion(policyPb.getVersion()) + .build(); + } - @Override - protected com.google.iam.v1.Policy toPb(Policy policy) { - com.google.iam.v1.Policy.Builder policyBuilder = com.google.iam.v1.Policy.newBuilder(); - List bindingPbList = new LinkedList<>(); - for (Binding binding : policy.getBindingsList()) { - com.google.iam.v1.Binding.Builder bindingBuilder = com.google.iam.v1.Binding.newBuilder(); - bindingBuilder.setRole(binding.getRole()); - bindingBuilder.addAllMembers(ImmutableList.copyOf(binding.getMembers())); - if (binding.getCondition() != null) { - Condition condition = binding.getCondition(); - bindingBuilder.setCondition(Expr.newBuilder() - .setTitle(condition.getTitle()) - .setDescription(condition.getDescription()) - .setExpression(condition.getExpression()) - .build()); - } - bindingPbList.add(bindingBuilder.build()); - } - policyBuilder.addAllBindings(bindingPbList); - if (policy.etag != null) { - policyBuilder.setEtag(ByteString.copyFrom(BaseEncoding.base64().decode(policy.etag))); - } - policyBuilder.setVersion(policy.version); - return policyBuilder.build(); + @Override + protected com.google.iam.v1.Policy toPb(Policy policy) { + com.google.iam.v1.Policy.Builder policyBuilder = com.google.iam.v1.Policy.newBuilder(); + List bindingPbList = new LinkedList<>(); + for (Binding binding : policy.getBindingsList()) { + com.google.iam.v1.Binding.Builder bindingBuilder = com.google.iam.v1.Binding.newBuilder(); + bindingBuilder.setRole(binding.getRole()); + bindingBuilder.addAllMembers(ImmutableList.copyOf(binding.getMembers())); + if (binding.getCondition() != null) { + Condition condition = binding.getCondition(); + bindingBuilder.setCondition( + Expr.newBuilder() + .setTitle(condition.getTitle()) + .setDescription(condition.getDescription()) + .setExpression(condition.getExpression()) + .build()); } + bindingPbList.add(bindingBuilder.build()); + } + policyBuilder.addAllBindings(bindingPbList); + if (policy.etag != null) { + policyBuilder.setEtag(ByteString.copyFrom(BaseEncoding.base64().decode(policy.etag))); + } + policyBuilder.setVersion(policy.version); + return policyBuilder.build(); + } + } + + /** A builder for {@code Policy} objects. */ + public static class Builder { + private final List bindingsList = new ArrayList(); + private String etag; + private int version; + + @InternalApi("This class should only be extended within google-cloud-java") + protected Builder() {} + + @InternalApi("This class should only be extended within google-cloud-java") + protected Builder(Policy policy) { + setBindings(policy.bindingsList); + setEtag(policy.etag); + setVersion(policy.version); } /** - * A builder for {@code Policy} objects. + * Replaces the builder's map of bindings with the given map of bindings. + * + * @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 + * @throws IllegalArgumentException if policy version is equal to 3 or has conditional bindings. */ - public static class Builder { - private final List bindingsList = new ArrayList(); - private String etag; - private int version; - - @InternalApi("This class should only be extended within google-cloud-java") - protected Builder() { + public final Builder setBindings(Map> bindings) { + checkNotNull(bindings, "The provided map of bindings cannot be null."); + checkArgument( + checkVersion(this.version, this.bindingsList), + "setBindings() is only supported with version 1 policies and non-conditional policies"); + for (Map.Entry> binding : bindings.entrySet()) { + checkNotNull(binding.getKey(), "The role cannot be null."); + Set 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."); + } + // convert into List format + this.bindingsList.clear(); + for (Map.Entry> binding : bindings.entrySet()) { + Binding.Builder bindingBuilder = Binding.newBuilder(); + bindingBuilder.setRole(binding.getKey().getValue()); + for (Identity identity : binding.getValue()) { + bindingBuilder.addMembers(identity.strValue()); } - - @InternalApi("This class should only be extended within google-cloud-java") - protected Builder(Policy policy) { - setBindings(policy.bindingsList); - setEtag(policy.etag); - setVersion(policy.version); - } - - /** - * Replaces the builder's map of bindings with the given map of bindings. - * - * @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 - * @throws IllegalArgumentException if policy version is equal to 3 or has conditional bindings. - */ - public final Builder setBindings(Map> bindings) { - checkNotNull(bindings, "The provided map of bindings cannot be null."); - checkArgument(checkVersion(this.version, this.bindingsList), - "setBindings() is only supported with version 1 policies and non-conditional policies"); - for (Map.Entry> binding : bindings.entrySet()) { - checkNotNull(binding.getKey(), "The role cannot be null."); - Set 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."); - } - // convert into List format - this.bindingsList.clear(); - for (Map.Entry> binding : bindings.entrySet()) { - Binding.Builder bindingBuilder = Binding.newBuilder(); - bindingBuilder.setRole(binding.getKey().getValue()); - for (Identity identity : binding.getValue()) { - bindingBuilder.addMembers(identity.strValue()); - } - this.bindingsList.add(bindingBuilder.build()); - } - return this; - } - - /** - * Replaces the builder's List of bindings with the given List of Bindings. - * - * @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 Builder setBindings(List bindings) { - for (Binding binding : bindings) { - checkNotNull(binding.getRole(), "The role cannot be null."); - List members = binding.getMembers(); - checkNotNull(members, "A role cannot be assigned to a null set of identities."); - checkArgument(!members.contains(null), "Null identities are not permitted."); - } - this.bindingsList.clear(); - for (Binding binding : bindings) { - Binding.Builder bindingBuilder = Binding.newBuilder(); - bindingBuilder.setMembers(new ArrayList(binding.getMembers())); - bindingBuilder.setRole(binding.getRole()); - bindingBuilder.setCondition(binding.getCondition()); - this.bindingsList.add(bindingBuilder.build()); - } - return this; - } - - - /** - * Removes the role (and all identities associated with that role) from the policy. - */ - public final Builder removeRole(Role role) { - checkArgument(checkVersion(this.version, this.bindingsList), - "removeRole() is only supported with version 1 policies and non-conditional policies"); - for (int i = 0; i < bindingsList.size(); ++i) { - Binding binding = bindingsList.get(i); - if (binding.getRole().equals(role.getValue())) { - System.out.println(role.getValue() + " " + binding.getRole()); - bindingsList.remove(i); - return this; - } - } - return this; - } - - /** - * Adds one or more identities to the policy under the role specified. - * - * @throws NullPointerException if the role or any of the identities is null. - * @throws IllegalArgumentException if policy version is equal to 3 or has conditional bindings. - */ - public final Builder addIdentity(Role role, Identity first, Identity... others) { - checkArgument(checkVersion(this.version, this.bindingsList), - "addIdentity() is only supported with version 1 policies and non-conditional policies"); - String nullIdentityMessage = "Null identities are not permitted."; - checkNotNull(first, nullIdentityMessage); - checkNotNull(others, nullIdentityMessage); - checkNotNull(role, "The role cannot be null."); - for (int i = 0; i < bindingsList.size(); ++i) { - Binding binding = bindingsList.get(i); - if (binding.getRole().equals(role.getValue())) { - Binding.Builder bindingBuilder = binding.toBuilder().addMembers(first.strValue()); - for (Identity identity : others) { - bindingBuilder.addMembers(identity.strValue()); - } - bindingsList.set(i, bindingBuilder.build()); - return this; - } - } - List members = new ArrayList<>(); - members.add(first.strValue()); - for (Identity identity : others) { - members.add(identity.strValue()); - } - bindingsList.add(Binding.newBuilder() - .setRole(role.getValue()) - .setMembers(members) - .build()); - return this; - } - - /** - * Removes one or more identities from an existing binding. Does nothing if the binding - * associated with the provided role doesn't exist. - * - * @throws IllegalArgumentException if policy version is equal to 3 or has conditional bindings. - */ - public final Builder removeIdentity(Role role, Identity first, Identity... others) { - checkArgument(checkVersion(this.version, this.bindingsList), - "removeIdentity() is only supported with version 1 policies and non-conditional policies"); - String nullIdentityMessage = "Null identities are not permitted."; - checkNotNull(first, nullIdentityMessage); - checkNotNull(others, nullIdentityMessage); - checkNotNull(role, "The role cannot be null."); - for (int i = 0; i < bindingsList.size(); ++i) { - Binding binding = bindingsList.get(i); - if (binding.getRole().equals(role.getValue())) { - Binding.Builder bindingBuilder = binding.toBuilder().removeMembers(first.strValue()); - for (Identity identity : others) { - bindingBuilder.removeMembers(identity.strValue()); - } - Binding updatedBindings = bindingBuilder.build(); - if (updatedBindings.getMembers().size() == 0) { - bindingsList.remove(i); - } else { - bindingsList.set(i, updatedBindings); - } - return this; - } - } - return this; - } - - /** - * Sets the policy's etag. - * - *

Etags are used for optimistic concurrency control as a way to help prevent simultaneous - * updates of a policy from overwriting each other. It is strongly suggested that systems make - * use of the etag in the read-modify-write cycle to perform policy updates in order to avoid - * race conditions. An etag is returned in the response to getIamPolicy, and systems are - * expected to put that etag in the request to setIamPolicy to ensure that their change will be - * applied to the same version of the policy. If no etag is provided in the call to - * setIamPolicy, then the existing policy is overwritten blindly. - */ - public final Builder setEtag(String etag) { - this.etag = etag; - return this; - } - - /** - * Sets the version of the policy. The default version is 0, meaning only the "owner", "editor", - * and "viewer" roles are permitted. If the version is 1, you may also use other roles. - */ - public final Builder setVersion(int version) { - this.version = version; - return this; - } - - /** - * Creates a {@code Policy} object. - */ - public final Policy build() { - return new Policy(this); - } - } - - private Policy(Builder builder) { - - this.bindingsList = ImmutableList.copyOf(builder.bindingsList); - this.etag = builder.etag; - this.version = builder.version; + this.bindingsList.add(bindingBuilder.build()); + } + return this; } /** - * Returns a builder containing the properties of this IAM Policy. + * Replaces the builder's List of bindings with the given List of Bindings. + * + * @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 Builder toBuilder() { - return new Builder(this); + public final Builder setBindings(List bindings) { + for (Binding binding : bindings) { + checkNotNull(binding.getRole(), "The role cannot be null."); + List members = binding.getMembers(); + checkNotNull(members, "A role cannot be assigned to a null set of identities."); + checkArgument(!members.contains(null), "Null identities are not permitted."); + } + this.bindingsList.clear(); + for (Binding binding : bindings) { + Binding.Builder bindingBuilder = Binding.newBuilder(); + bindingBuilder.setMembers(new ArrayList(binding.getMembers())); + bindingBuilder.setRole(binding.getRole()); + bindingBuilder.setCondition(binding.getCondition()); + this.bindingsList.add(bindingBuilder.build()); + } + return this; + } + + /** Removes the role (and all identities associated with that role) from the policy. */ + public final Builder removeRole(Role role) { + checkArgument( + checkVersion(this.version, this.bindingsList), + "removeRole() is only supported with version 1 policies and non-conditional policies"); + for (int i = 0; i < bindingsList.size(); ++i) { + Binding binding = bindingsList.get(i); + if (binding.getRole().equals(role.getValue())) { + System.out.println(role.getValue() + " " + binding.getRole()); + bindingsList.remove(i); + return this; + } + } + return this; } /** - * Returns the map of bindings that comprises the policy. + * Adds one or more identities to the policy under the role specified. * + * @throws NullPointerException if the role or any of the identities is null. * @throws IllegalArgumentException if policy version is equal to 3 or has conditional bindings. */ - - public Map> getBindings() { - checkArgument(checkVersion(this.version, this.bindingsList), - "getBindings() is only supported with version 1 policies and non-conditional policies"); - ImmutableMap.Builder> bindingsV1Builder = ImmutableMap.builder(); - for (Binding binding : bindingsList) { - ImmutableSet.Builder identities = ImmutableSet.builder(); - for (String member : binding.getMembers()) { - identities.add(Identity.valueOf(member)); - } - bindingsV1Builder.put(Role.of(binding.getRole()), identities.build()); + public final Builder addIdentity(Role role, Identity first, Identity... others) { + checkArgument( + checkVersion(this.version, this.bindingsList), + "addIdentity() is only supported with version 1 policies and non-conditional policies"); + String nullIdentityMessage = "Null identities are not permitted."; + checkNotNull(first, nullIdentityMessage); + checkNotNull(others, nullIdentityMessage); + checkNotNull(role, "The role cannot be null."); + for (int i = 0; i < bindingsList.size(); ++i) { + Binding binding = bindingsList.get(i); + if (binding.getRole().equals(role.getValue())) { + Binding.Builder bindingBuilder = binding.toBuilder().addMembers(first.strValue()); + for (Identity identity : others) { + bindingBuilder.addMembers(identity.strValue()); + } + bindingsList.set(i, bindingBuilder.build()); + return this; } - return bindingsV1Builder.build(); + } + List members = new ArrayList<>(); + members.add(first.strValue()); + for (Identity identity : others) { + members.add(identity.strValue()); + } + bindingsList.add(Binding.newBuilder().setRole(role.getValue()).setMembers(members).build()); + return this; } /** - * Returns the map of bindings that comprises the policy for version 3. + * Removes one or more identities from an existing binding. Does nothing if the binding + * associated with the provided role doesn't exist. + * + * @throws IllegalArgumentException if policy version is equal to 3 or has conditional bindings. */ - - public List getBindingsList() { - return bindingsList; + public final Builder removeIdentity(Role role, Identity first, Identity... others) { + checkArgument( + checkVersion(this.version, this.bindingsList), + "removeIdentity() is only supported with version 1 policies and non-conditional policies"); + String nullIdentityMessage = "Null identities are not permitted."; + checkNotNull(first, nullIdentityMessage); + checkNotNull(others, nullIdentityMessage); + checkNotNull(role, "The role cannot be null."); + for (int i = 0; i < bindingsList.size(); ++i) { + Binding binding = bindingsList.get(i); + if (binding.getRole().equals(role.getValue())) { + Binding.Builder bindingBuilder = binding.toBuilder().removeMembers(first.strValue()); + for (Identity identity : others) { + bindingBuilder.removeMembers(identity.strValue()); + } + Binding updatedBindings = bindingBuilder.build(); + if (updatedBindings.getMembers().size() == 0) { + bindingsList.remove(i); + } else { + bindingsList.set(i, updatedBindings); + } + return this; + } + } + return this; } /** - * Returns the policy's etag. + * Sets the policy's etag. * *

Etags are used for optimistic concurrency control as a way to help prevent simultaneous - * updates of a policy from overwriting each other. It is strongly suggested that systems make use - * of the etag in the read-modify-write cycle to perform policy updates in order to avoid race - * conditions. An etag is returned in the response to getIamPolicy, and systems are expected to - * put that etag in the request to setIamPolicy to ensure that their change will be applied to the - * same version of the policy. If no etag is provided in the call to setIamPolicy, then the - * existing policy is overwritten blindly. + * updates of a policy from overwriting each other. It is strongly suggested that systems make + * use of the etag in the read-modify-write cycle to perform policy updates in order to avoid + * race conditions. An etag is returned in the response to getIamPolicy, and systems are + * expected to put that etag in the request to setIamPolicy to ensure that their change will be + * applied to the same version of the policy. If no etag is provided in the call to + * setIamPolicy, then the existing policy is overwritten blindly. */ - public String getEtag() { - return etag; + public final Builder setEtag(String etag) { + this.etag = etag; + return this; } /** - * Returns the version of the policy. The default version is 0, meaning only the "owner", - * "editor", and "viewer" roles are permitted. If the version is 1, you may also use other roles. + * Sets the version of the policy. The default version is 0, meaning only the "owner", "editor", + * and "viewer" roles are permitted. If the version is 1, you may also use other roles. */ - public int getVersion() { - return version; + public final Builder setVersion(int version) { + this.version = version; + return this; } - @Override - public String toString() { - return MoreObjects.toStringHelper(this) - .add("bindings", bindingsList) - .add("etag", etag) - .add("version", version) - .toString(); + /** Creates a {@code Policy} object. */ + public final Policy build() { + return new Policy(this); } - - @Override - public int hashCode() { - return Objects.hash(getClass(), bindingsList, etag, version); + } + + private Policy(Builder builder) { + + this.bindingsList = ImmutableList.copyOf(builder.bindingsList); + this.etag = builder.etag; + this.version = builder.version; + } + + /** Returns a builder containing the properties of this IAM Policy. */ + public Builder toBuilder() { + return new Builder(this); + } + + /** + * Returns the map of bindings that comprises the policy. + * + * @throws IllegalArgumentException if policy version is equal to 3 or has conditional bindings. + */ + public Map> getBindings() { + checkArgument( + checkVersion(this.version, this.bindingsList), + "getBindings() is only supported with version 1 policies and non-conditional policies"); + ImmutableMap.Builder> bindingsV1Builder = ImmutableMap.builder(); + for (Binding binding : bindingsList) { + ImmutableSet.Builder identities = ImmutableSet.builder(); + for (String member : binding.getMembers()) { + identities.add(Identity.valueOf(member)); + } + bindingsV1Builder.put(Role.of(binding.getRole()), identities.build()); } - - @Override - public boolean equals(Object obj) { - if (obj == this) { - return true; - } - if (!(obj instanceof Policy)) { - return false; - } - Policy other = (Policy) obj; - if (bindingsList.size() != other.getBindingsList().size()) { - return false; - } - for (int i = 0; i < bindingsList.size(); ++i) { - bindingsList.get(i).equals(other.getBindingsList().get(i)); - } - return Objects.equals(etag, other.getEtag()) - && Objects.equals(version, other.getVersion()); + return bindingsV1Builder.build(); + } + + /** Returns the map of bindings that comprises the policy for version 3. */ + public List getBindingsList() { + return bindingsList; + } + + /** + * Returns the policy's etag. + * + *

Etags are used for optimistic concurrency control as a way to help prevent simultaneous + * updates of a policy from overwriting each other. It is strongly suggested that systems make use + * of the etag in the read-modify-write cycle to perform policy updates in order to avoid race + * conditions. An etag is returned in the response to getIamPolicy, and systems are expected to + * put that etag in the request to setIamPolicy to ensure that their change will be applied to the + * same version of the policy. If no etag is provided in the call to setIamPolicy, then the + * existing policy is overwritten blindly. + */ + public String getEtag() { + return etag; + } + + /** + * Returns the version of the policy. The default version is 0, meaning only the "owner", + * "editor", and "viewer" roles are permitted. If the version is 1, you may also use other roles. + */ + public int getVersion() { + return version; + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("bindings", bindingsList) + .add("etag", etag) + .add("version", version) + .toString(); + } + + @Override + public int hashCode() { + return Objects.hash(getClass(), bindingsList, etag, version); + } + + @Override + public boolean equals(Object obj) { + if (obj == this) { + return true; } - - /** - * Returns a builder for {@code Policy} objects. - */ - public static Builder newBuilder() { - return new Builder(); + if (!(obj instanceof Policy)) { + return false; } + Policy other = (Policy) obj; + if (bindingsList.size() != other.getBindingsList().size()) { + return false; + } + for (int i = 0; i < bindingsList.size(); ++i) { + bindingsList.get(i).equals(other.getBindingsList().get(i)); + } + return Objects.equals(etag, other.getEtag()) && Objects.equals(version, other.getVersion()); + } + + /** Returns a builder for {@code Policy} objects. */ + public static Builder newBuilder() { + return new Builder(); + } } diff --git a/google-cloud-core/src/test/java/com/google/cloud/PolicyTest.java b/google-cloud-core/src/test/java/com/google/cloud/PolicyTest.java index 0e12ae7ce3..eb03e33726 100644 --- a/google-cloud-core/src/test/java/com/google/cloud/PolicyTest.java +++ b/google-cloud-core/src/test/java/com/google/cloud/PolicyTest.java @@ -26,175 +26,173 @@ import com.google.cloud.Policy.DefaultMarshaller; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; - import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Set; - import org.junit.Test; public class PolicyTest { - private static final Identity ALL_USERS = Identity.allUsers(); - private static final Identity ALL_AUTH_USERS = Identity.allAuthenticatedUsers(); - private static final Identity USER = Identity.user("abc@gmail.com"); - private static final Identity SERVICE_ACCOUNT = - Identity.serviceAccount("service-account@gmail.com"); - private static final Identity GROUP = Identity.group("group@gmail.com"); - private static final Identity DOMAIN = Identity.domain("google.com"); - private static final Role VIEWER = Role.viewer(); - private static final Role EDITOR = Role.editor(); - private static final Role OWNER = Role.owner(); - private static final Map> BINDINGS = - ImmutableMap.of( - VIEWER, - ImmutableSet.of(USER, SERVICE_ACCOUNT, ALL_USERS), - EDITOR, - ImmutableSet.of(ALL_AUTH_USERS, GROUP, DOMAIN)); - private static final Policy SIMPLE_POLICY = - Policy.newBuilder() - .addIdentity(VIEWER, USER, SERVICE_ACCOUNT, ALL_USERS) - .addIdentity(EDITOR, ALL_AUTH_USERS, GROUP, DOMAIN) - .build(); - private static final Policy FULL_POLICY = - Policy.newBuilder() - .setBindings(SIMPLE_POLICY.getBindings()) - .setEtag("etag") - .setVersion(1) - .build(); + private static final Identity ALL_USERS = Identity.allUsers(); + private static final Identity ALL_AUTH_USERS = Identity.allAuthenticatedUsers(); + private static final Identity USER = Identity.user("abc@gmail.com"); + private static final Identity SERVICE_ACCOUNT = + Identity.serviceAccount("service-account@gmail.com"); + private static final Identity GROUP = Identity.group("group@gmail.com"); + private static final Identity DOMAIN = Identity.domain("google.com"); + private static final Role VIEWER = Role.viewer(); + private static final Role EDITOR = Role.editor(); + private static final Role OWNER = Role.owner(); + private static final Map> BINDINGS = + ImmutableMap.of( + VIEWER, + ImmutableSet.of(USER, SERVICE_ACCOUNT, ALL_USERS), + EDITOR, + ImmutableSet.of(ALL_AUTH_USERS, GROUP, DOMAIN)); + private static final Policy SIMPLE_POLICY = + Policy.newBuilder() + .addIdentity(VIEWER, USER, SERVICE_ACCOUNT, ALL_USERS) + .addIdentity(EDITOR, ALL_AUTH_USERS, GROUP, DOMAIN) + .build(); + private static final Policy FULL_POLICY = + Policy.newBuilder() + .setBindings(SIMPLE_POLICY.getBindings()) + .setEtag("etag") + .setVersion(1) + .build(); - @Test - public void testBuilder() { - assertEquals(BINDINGS, SIMPLE_POLICY.getBindings()); - assertEquals(null, SIMPLE_POLICY.getEtag()); - assertEquals(0, SIMPLE_POLICY.getVersion()); - assertEquals(BINDINGS, FULL_POLICY.getBindings()); - assertEquals("etag", FULL_POLICY.getEtag()); - assertEquals(1, FULL_POLICY.getVersion()); - Map> editorBinding = - ImmutableMap.>builder().put(EDITOR, BINDINGS.get(EDITOR)).build(); - Policy policy = FULL_POLICY.toBuilder().setBindings(editorBinding).build(); - assertEquals(editorBinding, policy.getBindings()); - assertEquals("etag", policy.getEtag()); - assertEquals(1, policy.getVersion()); - policy = SIMPLE_POLICY.toBuilder().removeRole(EDITOR).build(); - assertEquals(ImmutableMap.of(VIEWER, BINDINGS.get(VIEWER)), policy.getBindings()); - assertNull(policy.getEtag()); - assertEquals(0, policy.getVersion()); - policy = - policy - .toBuilder() - .removeIdentity(VIEWER, USER, ALL_USERS) - .addIdentity(VIEWER, DOMAIN, GROUP) - .build(); - assertEquals( - ImmutableMap.of(VIEWER, ImmutableSet.of(SERVICE_ACCOUNT, DOMAIN, GROUP)), - policy.getBindings()); - assertNull(policy.getEtag()); - assertEquals(0, policy.getVersion()); - policy = - Policy.newBuilder() - .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.getBindings()); - assertNull(policy.getEtag()); - assertEquals(0, policy.getVersion()); - } + @Test + public void testBuilder() { + assertEquals(BINDINGS, SIMPLE_POLICY.getBindings()); + assertEquals(null, SIMPLE_POLICY.getEtag()); + assertEquals(0, SIMPLE_POLICY.getVersion()); + assertEquals(BINDINGS, FULL_POLICY.getBindings()); + assertEquals("etag", FULL_POLICY.getEtag()); + assertEquals(1, FULL_POLICY.getVersion()); + Map> editorBinding = + ImmutableMap.>builder().put(EDITOR, BINDINGS.get(EDITOR)).build(); + Policy policy = FULL_POLICY.toBuilder().setBindings(editorBinding).build(); + assertEquals(editorBinding, policy.getBindings()); + assertEquals("etag", policy.getEtag()); + assertEquals(1, policy.getVersion()); + policy = SIMPLE_POLICY.toBuilder().removeRole(EDITOR).build(); + assertEquals(ImmutableMap.of(VIEWER, BINDINGS.get(VIEWER)), policy.getBindings()); + assertNull(policy.getEtag()); + assertEquals(0, policy.getVersion()); + policy = + policy + .toBuilder() + .removeIdentity(VIEWER, USER, ALL_USERS) + .addIdentity(VIEWER, DOMAIN, GROUP) + .build(); + assertEquals( + ImmutableMap.of(VIEWER, ImmutableSet.of(SERVICE_ACCOUNT, DOMAIN, GROUP)), + policy.getBindings()); + assertNull(policy.getEtag()); + assertEquals(0, policy.getVersion()); + policy = + Policy.newBuilder() + .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.getBindings()); + assertNull(policy.getEtag()); + assertEquals(0, policy.getVersion()); + } - @Test - public void testIllegalPolicies() { - try { - Policy.newBuilder().addIdentity(null, USER); - fail("Null role should cause exception."); - } catch (NullPointerException ex) { - assertEquals("The role cannot be null.", ex.getMessage()); - } - try { - Policy.newBuilder().addIdentity(VIEWER, null, USER); - fail("Null identity should cause exception."); - } catch (NullPointerException ex) { - assertEquals("Null identities are not permitted.", ex.getMessage()); - } - try { - Policy.newBuilder().addIdentity(VIEWER, USER, (Identity[]) null); - fail("Null identity should cause exception."); - } catch (NullPointerException ex) { - assertEquals("Null identities are not permitted.", ex.getMessage()); - } - try { - Policy.newBuilder().setBindings((Map>) null); - fail("Null bindings map should cause exception."); - } catch (NullPointerException ex) { - assertEquals("The provided map of bindings cannot be null.", ex.getMessage()); - } - try { - Map> bindings = new HashMap<>(); - bindings.put(VIEWER, null); - Policy.newBuilder().setBindings(bindings); - fail("Null set of identities should cause exception."); - } catch (NullPointerException ex) { - assertEquals("A role cannot be assigned to a null set of identities.", ex.getMessage()); - } - try { - Map> bindings = new HashMap<>(); - Set identities = new HashSet<>(); - identities.add(null); - bindings.put(VIEWER, identities); - Policy.newBuilder().setBindings(bindings); - fail("Null identity should cause exception."); - } catch (IllegalArgumentException ex) { - assertEquals("Null identities are not permitted.", ex.getMessage()); - } + @Test + public void testIllegalPolicies() { + try { + Policy.newBuilder().addIdentity(null, USER); + fail("Null role should cause exception."); + } catch (NullPointerException ex) { + assertEquals("The role cannot be null.", ex.getMessage()); } - - @Test - public void testEqualsHashCode() { - assertNotNull(FULL_POLICY); - Policy emptyPolicy = Policy.newBuilder().build(); - Policy anotherPolicy = Policy.newBuilder().build(); - assertEquals(emptyPolicy, anotherPolicy); - assertEquals(emptyPolicy.hashCode(), anotherPolicy.hashCode()); - assertNotEquals(FULL_POLICY, SIMPLE_POLICY); - assertNotEquals(FULL_POLICY.hashCode(), SIMPLE_POLICY.hashCode()); - Policy copy = SIMPLE_POLICY.toBuilder().build(); - assertEquals(SIMPLE_POLICY, copy); - assertEquals(SIMPLE_POLICY.hashCode(), copy.hashCode()); + try { + Policy.newBuilder().addIdentity(VIEWER, null, USER); + fail("Null identity should cause exception."); + } catch (NullPointerException ex) { + assertEquals("Null identities are not permitted.", ex.getMessage()); } - - @Test - public void testBindings() { - assertTrue(Policy.newBuilder().build().getBindings().isEmpty()); - assertEquals(BINDINGS, SIMPLE_POLICY.getBindings()); + try { + Policy.newBuilder().addIdentity(VIEWER, USER, (Identity[]) null); + fail("Null identity should cause exception."); + } catch (NullPointerException ex) { + assertEquals("Null identities are not permitted.", ex.getMessage()); } - - @Test - public void testEtag() { - assertNull(SIMPLE_POLICY.getEtag()); - assertEquals("etag", FULL_POLICY.getEtag()); + try { + Policy.newBuilder().setBindings((Map>) null); + fail("Null bindings map should cause exception."); + } catch (NullPointerException ex) { + assertEquals("The provided map of bindings cannot be null.", ex.getMessage()); } - - @Test - public void testVersion() { - assertEquals(0, SIMPLE_POLICY.getVersion()); - assertEquals(1, FULL_POLICY.getVersion()); + try { + Map> bindings = new HashMap<>(); + bindings.put(VIEWER, null); + Policy.newBuilder().setBindings(bindings); + fail("Null set of identities should cause exception."); + } catch (NullPointerException ex) { + assertEquals("A role cannot be assigned to a null set of identities.", ex.getMessage()); } - - @Test - public void testDefaultMarshaller() { - DefaultMarshaller marshaller = new DefaultMarshaller(); - Policy emptyPolicy = Policy.newBuilder().build(); - assertEquals(emptyPolicy, marshaller.fromPb(marshaller.toPb(emptyPolicy))); - assertEquals(SIMPLE_POLICY, marshaller.fromPb(marshaller.toPb(SIMPLE_POLICY))); - assertEquals(FULL_POLICY, marshaller.fromPb(marshaller.toPb(FULL_POLICY))); - com.google.iam.v1.Policy policyPb = com.google.iam.v1.Policy.getDefaultInstance(); - Policy policy = marshaller.fromPb(policyPb); - assertTrue(policy.getBindings().isEmpty()); - assertNull(policy.getEtag()); - assertEquals(0, policy.getVersion()); + try { + Map> bindings = new HashMap<>(); + Set identities = new HashSet<>(); + identities.add(null); + bindings.put(VIEWER, identities); + Policy.newBuilder().setBindings(bindings); + fail("Null identity should cause exception."); + } catch (IllegalArgumentException ex) { + assertEquals("Null identities are not permitted.", ex.getMessage()); } + } + + @Test + public void testEqualsHashCode() { + assertNotNull(FULL_POLICY); + Policy emptyPolicy = Policy.newBuilder().build(); + Policy anotherPolicy = Policy.newBuilder().build(); + assertEquals(emptyPolicy, anotherPolicy); + assertEquals(emptyPolicy.hashCode(), anotherPolicy.hashCode()); + assertNotEquals(FULL_POLICY, SIMPLE_POLICY); + assertNotEquals(FULL_POLICY.hashCode(), SIMPLE_POLICY.hashCode()); + Policy copy = SIMPLE_POLICY.toBuilder().build(); + assertEquals(SIMPLE_POLICY, copy); + assertEquals(SIMPLE_POLICY.hashCode(), copy.hashCode()); + } + + @Test + public void testBindings() { + assertTrue(Policy.newBuilder().build().getBindings().isEmpty()); + assertEquals(BINDINGS, SIMPLE_POLICY.getBindings()); + } + + @Test + public void testEtag() { + assertNull(SIMPLE_POLICY.getEtag()); + assertEquals("etag", FULL_POLICY.getEtag()); + } + + @Test + public void testVersion() { + assertEquals(0, SIMPLE_POLICY.getVersion()); + assertEquals(1, FULL_POLICY.getVersion()); + } + + @Test + public void testDefaultMarshaller() { + DefaultMarshaller marshaller = new DefaultMarshaller(); + Policy emptyPolicy = Policy.newBuilder().build(); + assertEquals(emptyPolicy, marshaller.fromPb(marshaller.toPb(emptyPolicy))); + assertEquals(SIMPLE_POLICY, marshaller.fromPb(marshaller.toPb(SIMPLE_POLICY))); + assertEquals(FULL_POLICY, marshaller.fromPb(marshaller.toPb(FULL_POLICY))); + com.google.iam.v1.Policy policyPb = com.google.iam.v1.Policy.getDefaultInstance(); + Policy policy = marshaller.fromPb(policyPb); + assertTrue(policy.getBindings().isEmpty()); + assertNull(policy.getEtag()); + assertEquals(0, policy.getVersion()); + } } diff --git a/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java b/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java index e4f10b5e72..5188fff7ae 100644 --- a/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java +++ b/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java @@ -16,244 +16,247 @@ package com.google.cloud; +import static org.junit.Assert.*; + import com.google.cloud.Policy.DefaultMarshaller; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; +import java.util.ArrayList; +import java.util.List; import org.junit.Test; -import java.util.*; +public class PolicyV3Test { -import static org.junit.Assert.*; + private static final String ALL_USERS = "allUsers"; + private static final String ALL_AUTH_USERS = "allAuthenticatedUsers"; + private static final String USER = "user:abc@gmail.com"; + private static final String SERVICE_ACCOUNT = "serviceAccount:service-account@gmail.com"; + private static final String GROUP = "group:group@gmail.com"; + private static final String DOMAIN = "domain:google.com"; + private static final String VIEWER = "roles/viewer"; + private static final String EDITOR = "roles/editor"; + private static final String OWNER = "roles/owner"; + private static final List MEMBERS_LIST_1 = + ImmutableList.of(USER, SERVICE_ACCOUNT, ALL_USERS); + private static final List MEMBERS_LIST_2 = + ImmutableList.of(ALL_AUTH_USERS, GROUP, DOMAIN); + private static final List BINDINGS_NO_CONDITIONS = + ImmutableList.of( + Binding.newBuilder().setRole(VIEWER).setMembers(MEMBERS_LIST_1).build(), + Binding.newBuilder().setRole(EDITOR).setMembers(MEMBERS_LIST_2).build()); + private static final List BINDINGS_WITH_CONDITIONS = + ImmutableList.copyOf(BINDINGS_NO_CONDITIONS) + .of( + Binding.newBuilder() + .setRole(VIEWER) + .setMembers(MEMBERS_LIST_1) + .setCondition( + Condition.newBuilder() + .setTitle("Condition") + .setDescription("Condition") + .setExpression("Expr") + .build()) + .build(), + Binding.newBuilder().setRole(EDITOR).setMembers(MEMBERS_LIST_2).build()); + private static final Policy FULL_POLICY_V1 = + Policy.newBuilder().setBindings(BINDINGS_NO_CONDITIONS).setEtag("etag").setVersion(1).build(); -public class PolicyV3Test { + private static final Policy FULL_POLICY_V3 = + Policy.newBuilder() + .setBindings(BINDINGS_WITH_CONDITIONS) + .setEtag("etag") + .setVersion(3) + .build(); - private static final String ALL_USERS = "allUsers"; - private static final String ALL_AUTH_USERS = "allAuthenticatedUsers"; - private static final String USER = "user:abc@gmail.com"; - private static final String SERVICE_ACCOUNT = "serviceAccount:service-account@gmail.com"; - private static final String GROUP = "group:group@gmail.com"; - private static final String DOMAIN = "domain:google.com"; - private static final String VIEWER = "roles/viewer"; - private static final String EDITOR = "roles/editor"; - private static final String OWNER = "roles/owner"; - private static final List MEMBERS_LIST_1 = ImmutableList.of(USER, SERVICE_ACCOUNT, ALL_USERS); - private static final List MEMBERS_LIST_2 = ImmutableList.of(ALL_AUTH_USERS, GROUP, DOMAIN); - private static final List BINDINGS_NO_CONDITIONS = - ImmutableList.of( - Binding.newBuilder() - .setRole(VIEWER) - .setMembers(MEMBERS_LIST_1).build(), - Binding.newBuilder() - .setRole(EDITOR) - .setMembers(MEMBERS_LIST_2).build()); - private static final List BINDINGS_WITH_CONDITIONS = - ImmutableList.copyOf(BINDINGS_NO_CONDITIONS).of( - Binding.newBuilder() - .setRole(VIEWER) - .setMembers(MEMBERS_LIST_1) - .setCondition(Condition.newBuilder() - .setTitle("Condition") - .setDescription("Condition") - .setExpression("Expr") - .build()) - .build(), - Binding.newBuilder() - .setRole(EDITOR) - .setMembers(MEMBERS_LIST_2).build()); - private static final Policy FULL_POLICY_V1 = - Policy.newBuilder() - .setBindings(BINDINGS_NO_CONDITIONS) - .setEtag("etag") - .setVersion(1) - .build(); + private static final Policy FULL_POLICY_V3_WITH_VERSION_1 = + Policy.newBuilder() + .setBindings(BINDINGS_WITH_CONDITIONS) + .setEtag("etag") + .setVersion(1) + .build(); - private static final Policy FULL_POLICY_V3 = - Policy.newBuilder() - .setBindings(BINDINGS_WITH_CONDITIONS) - .setEtag("etag") - .setVersion(3) - .build(); + @Test + public void testBuilder() { + assertEquals(BINDINGS_NO_CONDITIONS, FULL_POLICY_V1.getBindingsList()); + assertEquals(1, FULL_POLICY_V1.getVersion()); + assertEquals("etag", FULL_POLICY_V1.getEtag()); + Policy policy = FULL_POLICY_V1.toBuilder().setBindings(BINDINGS_NO_CONDITIONS).build(); + assertEquals(BINDINGS_NO_CONDITIONS, policy.getBindingsList()); + assertEquals("etag", policy.getEtag()); + assertEquals(1, policy.getVersion()); - private static final Policy FULL_POLICY_V3_WITH_VERSION_1 = - Policy.newBuilder() - .setBindings(BINDINGS_WITH_CONDITIONS) - .setEtag("etag") - .setVersion(1) - .build(); + assertEquals(BINDINGS_WITH_CONDITIONS, FULL_POLICY_V3.getBindingsList()); + assertEquals(3, FULL_POLICY_V3.getVersion()); + assertEquals("etag", FULL_POLICY_V3.getEtag()); + policy = FULL_POLICY_V3.toBuilder().setBindings(BINDINGS_WITH_CONDITIONS).build(); + assertEquals(BINDINGS_WITH_CONDITIONS, policy.getBindingsList()); + assertEquals("etag", policy.getEtag()); + assertEquals(3, policy.getVersion()); - @Test - public void testBuilder() { - assertEquals(BINDINGS_NO_CONDITIONS, FULL_POLICY_V1.getBindingsList()); - assertEquals(1, FULL_POLICY_V1.getVersion()); - assertEquals("etag", FULL_POLICY_V1.getEtag()); - Policy policy = FULL_POLICY_V1.toBuilder().setBindings(BINDINGS_NO_CONDITIONS).build(); - assertEquals(BINDINGS_NO_CONDITIONS, policy.getBindingsList()); - assertEquals("etag", policy.getEtag()); - assertEquals(1, policy.getVersion()); + assertEquals(BINDINGS_WITH_CONDITIONS, FULL_POLICY_V3_WITH_VERSION_1.getBindingsList()); + assertEquals(1, FULL_POLICY_V3_WITH_VERSION_1.getVersion()); + assertEquals("etag", FULL_POLICY_V3_WITH_VERSION_1.getEtag()); + policy = + FULL_POLICY_V3_WITH_VERSION_1 + .toBuilder() + .setBindings(BINDINGS_WITH_CONDITIONS) + .setVersion(3) + .build(); + assertEquals(BINDINGS_WITH_CONDITIONS, policy.getBindingsList()); + assertEquals("etag", policy.getEtag()); + assertEquals(3, policy.getVersion()); + } - assertEquals(BINDINGS_WITH_CONDITIONS, FULL_POLICY_V3.getBindingsList()); - assertEquals(3, FULL_POLICY_V3.getVersion()); - assertEquals("etag", FULL_POLICY_V3.getEtag()); - policy = FULL_POLICY_V3.toBuilder().setBindings(BINDINGS_WITH_CONDITIONS).build(); - assertEquals(BINDINGS_WITH_CONDITIONS, policy.getBindingsList()); - assertEquals("etag", policy.getEtag()); - assertEquals(3, policy.getVersion()); + @Test + public void removeMemberFromPolicy() { + assertEquals(3, FULL_POLICY_V3.getBindingsList().get(0).getMembers().size()); + List bindings = new ArrayList<>(FULL_POLICY_V3.getBindingsList()); - assertEquals(BINDINGS_WITH_CONDITIONS, FULL_POLICY_V3_WITH_VERSION_1.getBindingsList()); - assertEquals(1, FULL_POLICY_V3_WITH_VERSION_1.getVersion()); - assertEquals("etag", FULL_POLICY_V3_WITH_VERSION_1.getEtag()); - policy = FULL_POLICY_V3_WITH_VERSION_1.toBuilder().setBindings(BINDINGS_WITH_CONDITIONS).setVersion(3).build(); - assertEquals(BINDINGS_WITH_CONDITIONS, policy.getBindingsList()); - assertEquals("etag", policy.getEtag()); - assertEquals(3, policy.getVersion()); + for (int i = 0; i < bindings.size(); ++i) { + Binding binding = bindings.get(i); + if (binding.getRole().equals(VIEWER)) { + bindings.set(i, binding.toBuilder().removeMembers(ALL_USERS).build()); + break; + } } + Policy updatedPolicy = FULL_POLICY_V3.toBuilder().setBindings(bindings).build(); + assertEquals(2, updatedPolicy.getBindingsList().get(0).getMembers().size()); + } - @Test - public void removeMemberFromPolicy() { - assertEquals(3, FULL_POLICY_V3.getBindingsList().get(0).getMembers().size()); - List bindings = new ArrayList<>(FULL_POLICY_V3.getBindingsList()); + @Test + public void addMemberFromPolicy() { + assertEquals(3, FULL_POLICY_V3.getBindingsList().get(0).getMembers().size()); + List bindings = new ArrayList<>(FULL_POLICY_V3.getBindingsList()); - for (int i = 0; i < bindings.size(); ++i) { - Binding binding = bindings.get(i); - if (binding.getRole().equals(VIEWER)) { - bindings.set(i, binding.toBuilder().removeMembers(ALL_USERS).build()); - break; - } - } - - Policy updatedPolicy = FULL_POLICY_V3.toBuilder().setBindings(bindings).build(); - assertEquals(2, updatedPolicy.getBindingsList().get(0).getMembers().size()); + for (int i = 0; i < bindings.size(); ++i) { + Binding binding = bindings.get(i); + if (binding.getRole().equals(VIEWER)) { + bindings.set(i, binding.toBuilder().addMembers("user:example@example.com").build()); + } } - @Test - public void addMemberFromPolicy() { - assertEquals(3, FULL_POLICY_V3.getBindingsList().get(0).getMembers().size()); - List bindings = new ArrayList<>(FULL_POLICY_V3.getBindingsList()); + Policy updatedPolicy = FULL_POLICY_V3.toBuilder().setBindings(bindings).build(); + assertEquals(4, updatedPolicy.getBindingsList().get(0).getMembers().size()); + } - for (int i = 0; i < bindings.size(); ++i) { - Binding binding = bindings.get(i); - if (binding.getRole().equals(VIEWER)) { - bindings.set(i, binding.toBuilder().addMembers("user:example@example.com").build()); - } - } + @Test + public void removeBindingFromPolicy() { + assertEquals(2, FULL_POLICY_V3.getBindingsList().size()); + List bindings = new ArrayList<>(FULL_POLICY_V3.getBindingsList()); - Policy updatedPolicy = FULL_POLICY_V3.toBuilder().setBindings(bindings).build(); - assertEquals(4, updatedPolicy.getBindingsList().get(0).getMembers().size()); + for (int i = 0; i < bindings.size(); ++i) { + Binding binding = bindings.get(i); + if (binding.getRole().equals(EDITOR) && binding.getCondition() == null) { + bindings.remove(i); + break; + } } - @Test - public void removeBindingFromPolicy() { - assertEquals(2, FULL_POLICY_V3.getBindingsList().size()); - List bindings = new ArrayList<>(FULL_POLICY_V3.getBindingsList()); + Policy updatedPolicy = FULL_POLICY_V3.toBuilder().setBindings(bindings).build(); + assertEquals(1, updatedPolicy.getBindingsList().size()); + } - for (int i = 0; i < bindings.size(); ++i) { - Binding binding = bindings.get(i); - if (binding.getRole().equals(EDITOR) && binding.getCondition() == null) { - bindings.remove(i); - break; - } - } + @Test + public void addBindingToPolicy() { + assertEquals(2, FULL_POLICY_V3.getBindingsList().size()); + List bindings = new ArrayList<>(FULL_POLICY_V3.getBindingsList()); + bindings.add(Binding.newBuilder().setRole(OWNER).addMembers(USER).build()); + Policy updatedPolicy = FULL_POLICY_V3.toBuilder().setBindings(bindings).build(); + assertEquals(3, updatedPolicy.getBindingsList().size()); + } - Policy updatedPolicy = FULL_POLICY_V3.toBuilder().setBindings(bindings).build(); - assertEquals(1, updatedPolicy.getBindingsList().size()); + @Test + public void testIllegalPolicies() { + try { + Binding.newBuilder().setRole(null); + fail("Null role should cause exception."); + } catch (NullPointerException ex) { + assertEquals("The role cannot be null.", ex.getMessage()); } - - @Test - public void addBindingToPolicy() { - assertEquals(2, FULL_POLICY_V3.getBindingsList().size()); - List bindings = new ArrayList<>(FULL_POLICY_V3.getBindingsList()); - bindings.add(Binding.newBuilder().setRole(OWNER).addMembers(USER).build()); - Policy updatedPolicy = FULL_POLICY_V3.toBuilder().setBindings(bindings).build(); - assertEquals(3, updatedPolicy.getBindingsList().size()); + try { + Binding.newBuilder().addMembers(null); + fail("Null member should cause exception."); + } catch (NullPointerException ex) { + assertEquals("Null identities are not permitted.", ex.getMessage()); } - - - @Test - public void testIllegalPolicies() { - try { - Binding.newBuilder().setRole(null); - fail("Null role should cause exception."); - } catch (NullPointerException ex) { - assertEquals("The role cannot be null.", ex.getMessage()); - } - try { - Binding.newBuilder().addMembers(null); - fail("Null member should cause exception."); - } catch (NullPointerException ex) { - assertEquals("Null identities are not permitted.", ex.getMessage()); - } - try { - FULL_POLICY_V3.getBindings(); - fail("getBindings() should cause exception with Policy V3."); - } catch (IllegalArgumentException ex) { - assertEquals("getBindings() is only supported with version 1 policies and non-conditional policies", ex.getMessage()); - } - try { - FULL_POLICY_V3.toBuilder().addIdentity(Role.editor(), Identity.allUsers()); - fail("getBindings() should cause exception with Policy V3."); - } catch (IllegalArgumentException ex) { - assertEquals("addIdentity() is only supported with version 1 policies and non-conditional policies", ex.getMessage()); - } - try { - FULL_POLICY_V3.toBuilder().removeIdentity(Role.editor(), Identity.allUsers()); - fail("getBindings() should cause exception with Policy V3."); - } catch (IllegalArgumentException ex) { - assertEquals("removeIdentity() is only supported with version 1 policies and non-conditional policies", ex.getMessage()); - } - try { - FULL_POLICY_V3.toBuilder().setBindings(FULL_POLICY_V1.getBindings()); - fail("getBindings() should cause exception with Policy V3."); - } catch (IllegalArgumentException ex) { - assertEquals("setBindings() is only supported with version 1 policies and non-conditional policies", ex.getMessage()); - } + try { + FULL_POLICY_V3.getBindings(); + fail("getBindings() should cause exception with Policy V3."); + } catch (IllegalArgumentException ex) { + assertEquals( + "getBindings() is only supported with version 1 policies and non-conditional policies", + ex.getMessage()); } - - @Test - public void testEqualsHashCode() { - assertNotNull(FULL_POLICY_V3); - Policy emptyPolicy = Policy.newBuilder().build(); - Policy anotherPolicy = Policy.newBuilder().build(); - assertEquals(emptyPolicy, anotherPolicy); - assertEquals(emptyPolicy.hashCode(), anotherPolicy.hashCode()); - assertNotEquals(FULL_POLICY_V3, FULL_POLICY_V1); - assertNotEquals(FULL_POLICY_V3.hashCode(), FULL_POLICY_V1.hashCode()); - Policy copy = FULL_POLICY_V1.toBuilder().build(); - assertEquals(FULL_POLICY_V1, copy); - assertEquals(FULL_POLICY_V1.hashCode(), copy.hashCode()); + try { + FULL_POLICY_V3.toBuilder().addIdentity(Role.editor(), Identity.allUsers()); + fail("getBindings() should cause exception with Policy V3."); + } catch (IllegalArgumentException ex) { + assertEquals( + "addIdentity() is only supported with version 1 policies and non-conditional policies", + ex.getMessage()); } - - @Test - public void testBindings() { - assertTrue(Policy.newBuilder().build().getBindingsList().isEmpty()); - assertEquals(BINDINGS_WITH_CONDITIONS, FULL_POLICY_V3.getBindingsList()); + try { + FULL_POLICY_V3.toBuilder().removeIdentity(Role.editor(), Identity.allUsers()); + fail("getBindings() should cause exception with Policy V3."); + } catch (IllegalArgumentException ex) { + assertEquals( + "removeIdentity() is only supported with version 1 policies and non-conditional policies", + ex.getMessage()); } - - @Test - public void testEtag() { - assertNotNull(FULL_POLICY_V3.getEtag()); - assertEquals("etag", FULL_POLICY_V3.getEtag()); + try { + FULL_POLICY_V3.toBuilder().setBindings(FULL_POLICY_V1.getBindings()); + fail("getBindings() should cause exception with Policy V3."); + } catch (IllegalArgumentException ex) { + assertEquals( + "setBindings() is only supported with version 1 policies and non-conditional policies", + ex.getMessage()); } + } - @Test - public void testVersion() { - assertEquals(1, FULL_POLICY_V1.getVersion()); - assertEquals(3, FULL_POLICY_V3.getVersion()); - assertEquals(1, FULL_POLICY_V3_WITH_VERSION_1.getVersion()); - } + @Test + public void testEqualsHashCode() { + assertNotNull(FULL_POLICY_V3); + Policy emptyPolicy = Policy.newBuilder().build(); + Policy anotherPolicy = Policy.newBuilder().build(); + assertEquals(emptyPolicy, anotherPolicy); + assertEquals(emptyPolicy.hashCode(), anotherPolicy.hashCode()); + assertNotEquals(FULL_POLICY_V3, FULL_POLICY_V1); + assertNotEquals(FULL_POLICY_V3.hashCode(), FULL_POLICY_V1.hashCode()); + Policy copy = FULL_POLICY_V1.toBuilder().build(); + assertEquals(FULL_POLICY_V1, copy); + assertEquals(FULL_POLICY_V1.hashCode(), copy.hashCode()); + } - @Test - public void testDefaultMarshaller() { - DefaultMarshaller marshaller = new DefaultMarshaller(); - Policy emptyPolicy = Policy.newBuilder().build(); - assertEquals(emptyPolicy, marshaller.fromPb(marshaller.toPb(emptyPolicy))); - assertEquals(FULL_POLICY_V3, marshaller.fromPb(marshaller.toPb(FULL_POLICY_V3))); - assertEquals(FULL_POLICY_V1, marshaller.fromPb(marshaller.toPb(FULL_POLICY_V1))); - com.google.iam.v1.Policy policyPb = com.google.iam.v1.Policy.getDefaultInstance(); - Policy policy = marshaller.fromPb(policyPb); - assertTrue(policy.getBindingsList().isEmpty()); - assertNull(policy.getEtag()); - assertEquals(0, policy.getVersion()); - } + @Test + public void testBindings() { + assertTrue(Policy.newBuilder().build().getBindingsList().isEmpty()); + assertEquals(BINDINGS_WITH_CONDITIONS, FULL_POLICY_V3.getBindingsList()); + } + + @Test + public void testEtag() { + assertNotNull(FULL_POLICY_V3.getEtag()); + assertEquals("etag", FULL_POLICY_V3.getEtag()); + } + + @Test + public void testVersion() { + assertEquals(1, FULL_POLICY_V1.getVersion()); + assertEquals(3, FULL_POLICY_V3.getVersion()); + assertEquals(1, FULL_POLICY_V3_WITH_VERSION_1.getVersion()); + } + + @Test + public void testDefaultMarshaller() { + DefaultMarshaller marshaller = new DefaultMarshaller(); + Policy emptyPolicy = Policy.newBuilder().build(); + assertEquals(emptyPolicy, marshaller.fromPb(marshaller.toPb(emptyPolicy))); + assertEquals(FULL_POLICY_V3, marshaller.fromPb(marshaller.toPb(FULL_POLICY_V3))); + assertEquals(FULL_POLICY_V1, marshaller.fromPb(marshaller.toPb(FULL_POLICY_V1))); + com.google.iam.v1.Policy policyPb = com.google.iam.v1.Policy.getDefaultInstance(); + Policy policy = marshaller.fromPb(policyPb); + assertTrue(policy.getBindingsList().isEmpty()); + assertNull(policy.getEtag()); + assertEquals(0, policy.getVersion()); + } } From c90dbb026d14f0f3422ee352d1d4fccafa1e11e1 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Wed, 8 Jan 2020 10:58:43 -0800 Subject: [PATCH 06/31] Revert removal of helper functions --- .../src/main/java/com/google/cloud/Policy.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/google-cloud-core/src/main/java/com/google/cloud/Policy.java b/google-cloud-core/src/main/java/com/google/cloud/Policy.java index 1214ead85a..16712644d1 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Policy.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Policy.java @@ -19,6 +19,7 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; +import com.google.api.core.ApiFunction; import com.google.api.core.InternalApi; import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableList; @@ -71,6 +72,21 @@ public abstract static class Marshaller { @InternalApi("This class should only be extended within google-cloud-java") protected Marshaller() {} + protected static final ApiFunction IDENTITY_VALUE_OF_FUNCTION = + new ApiFunction() { + @Override + public Identity apply(String identityPb) { + return Identity.valueOf(identityPb); + } + }; + protected static final ApiFunction IDENTITY_STR_VALUE_FUNCTION = + new ApiFunction() { + @Override + public String apply(Identity identity) { + return identity.strValue(); + } + }; + protected abstract Policy fromPb(T policyPb); protected abstract T toPb(Policy policy); From b0a617f6397524ad2836b86d8b4b6d97c1e6a4cc Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Tue, 21 Jan 2020 18:30:48 -0800 Subject: [PATCH 07/31] use auto-value --- google-cloud-core/pom.xml | 10 ++ .../main/java/com/google/cloud/Binding.java | 152 ++++-------------- .../main/java/com/google/cloud/Condition.java | 112 ++----------- .../main/java/com/google/cloud/Policy.java | 17 +- .../java/com/google/cloud/PolicyV3Test.java | 10 +- pom.xml | 29 +++- 6 files changed, 100 insertions(+), 230 deletions(-) diff --git a/google-cloud-core/pom.xml b/google-cloud-core/pom.xml index d3f2f690a8..510e1b1a33 100644 --- a/google-cloud-core/pom.xml +++ b/google-cloud-core/pom.xml @@ -39,6 +39,16 @@ gax + com.google.auto.value + auto-value-annotations + true + + + com.google.auto.value + auto-value + true + + com.google.protobuf protobuf-java-util diff --git a/google-cloud-core/src/main/java/com/google/cloud/Binding.java b/google-cloud-core/src/main/java/com/google/cloud/Binding.java index 91325c3228..a457ae1ebf 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Binding.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Binding.java @@ -16,138 +16,56 @@ package com.google.cloud; -import static com.google.common.base.Preconditions.checkNotNull; - -import com.google.api.core.InternalApi; -import com.google.common.base.MoreObjects; -import com.google.common.collect.ImmutableList; +import java.lang.reflect.Array; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; -import java.util.Objects; -public class Binding { - private String role; - private List members; - private Condition condition; +import com.google.auto.value.AutoValue; +import com.google.common.base.Preconditions; +import com.google.common.base.Predicates; +import com.google.common.collect.Collections2; +import com.google.common.collect.ImmutableList; - public static class Builder { - private List members = new ArrayList(); - private String role; - private Condition condition; +import javax.annotation.Nullable; - @InternalApi("This class should only be extended within google-cloud-java") - protected Builder() {} +@AutoValue +abstract class Binding { + abstract String getRole(); + abstract ImmutableList getMembers(); + @Nullable abstract Condition getCondition(); + public abstract Builder toBuilder(); - @InternalApi("This class should only be extended within google-cloud-java") - protected Builder(Binding binding) { - setRole(binding.role); - setMembers(binding.members); - setCondition(binding.condition); - } + static Builder newBuilder() { + return new AutoValue_Binding.Builder(); + } - public final Binding.Builder setRole(String role) { - String nullIdentityMessage = "The role cannot be null."; - checkNotNull(role, nullIdentityMessage); - this.role = role; - return this; - } + @AutoValue.Builder + abstract static class Builder { + abstract Builder setRole(String role); + abstract Builder setMembers(List members); + abstract Builder setCondition(Condition condition); - public final Binding.Builder setMembers(List members) { - String nullIdentityMessage = "Null members are not permitted."; - checkNotNull(members, nullIdentityMessage); - this.members.clear(); - for (String member : members) { - // Check member not null - this.members.add(member); - } - return this; - } + abstract String getRole(); + abstract ImmutableList getMembers(); + abstract Condition getCondition(); - public final Binding.Builder removeMembers(String first, String... others) { - String nullIdentityMessage = "Null members are not permitted."; - checkNotNull(first, nullIdentityMessage); - checkNotNull(others, nullIdentityMessage); - this.members.remove(first); - for (String member : others) { - this.members.remove(member); - } - return this; - } + public abstract ImmutableList.Builder membersBuilder(); + //public abstract Condition.Builder conditionBuilder(); - public final Binding.Builder addMembers(String first, String... others) { - String nullIdentityMessage = "Null identities are not permitted."; - checkNotNull(first, nullIdentityMessage); - checkNotNull(others, nullIdentityMessage); - this.members.add(first); - for (String member : others) { - this.members.add(member); + public Builder addMembers(String... members){ + for (String member : members) { + membersBuilder().add(member); } return this; } - public final Binding.Builder setCondition(Condition condition) { - this.condition = condition; + public Builder removeMembers(String... members) { + setMembers(ImmutableList.copyOf(Collections2.filter(getMembers(), + Predicates.not(Predicates.in(Arrays.asList(members)))))); return this; } - /** Creates a {@code Policy} object. */ - public final Binding build() { - return new Binding(this); - } - } - - private Binding(Binding.Builder builder) { - this.role = builder.role; - this.members = ImmutableList.copyOf(builder.members); - this.condition = builder.condition; - } - - public Binding.Builder toBuilder() { - return new Binding.Builder(this); - } - - public String getRole() { - return this.role; - } - - public List getMembers() { - return this.members; - } - - public Condition getCondition() { - return this.condition; - } - - @Override - public String toString() { - return MoreObjects.toStringHelper(this) - .add("role", role) - .add("members", members) - .add("condition", condition) - .toString(); - } - - @Override - public int hashCode() { - return Objects.hash(getClass(), role, members, condition); - } - - @Override - public boolean equals(Object obj) { - if (obj == this) { - return true; - } - if (!(obj instanceof Binding)) { - return false; - } - Binding other = (Binding) obj; - return Objects.equals(role, other.getRole()) - && Objects.equals(members, other.getMembers()) - && Objects.equals(condition, other.getCondition()); - } - - /** Returns a builder for {@code Policy} objects. */ - public static Binding.Builder newBuilder() { - return new Binding.Builder(); + abstract Binding build(); } -} +} \ No newline at end of file diff --git a/google-cloud-core/src/main/java/com/google/cloud/Condition.java b/google-cloud-core/src/main/java/com/google/cloud/Condition.java index 854ea64e63..744686ab08 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Condition.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Condition.java @@ -16,103 +16,25 @@ package com.google.cloud; -import com.google.api.core.InternalApi; -import com.google.common.base.MoreObjects; -import java.util.Objects; +import com.google.auto.value.AutoValue; +import javax.annotation.Nullable; -public class Condition { - private String title; - private String description; - private String expression; +@AutoValue +abstract class Condition { + @Nullable abstract String getTitle(); + @Nullable abstract String getDescription(); + @Nullable abstract String getExpression(); + public abstract Builder toBuilder(); - public static class Builder { - private String title; - private String description; - private String expression; - - @InternalApi("This class should only be extended within google-cloud-java") - protected Builder() {} - - @InternalApi("This class should only be extended within google-cloud-java") - protected Builder(Condition condition) { - this.title = condition.title; - this.description = condition.description; - this.expression = condition.expression; - } - - public final Condition.Builder setTitle(String title) { - this.title = title; - return this; - } - - public final Condition.Builder setDescription(String description) { - this.description = description; - return this; - } - - public final Condition.Builder setExpression(String expression) { - this.expression = expression; - return this; - } - - /** Creates a {@code Condition} object. */ - public final Condition build() { - return new Condition(this); - } - } - - private Condition(Condition.Builder builder) { - this.title = builder.title; - this.description = builder.description; - this.expression = builder.expression; - } - - public Condition.Builder toBuilder() { - return new Condition.Builder(this); - } - - public String getTitle() { - return title; - } - - public String getDescription() { - return description; - } - - public String getExpression() { - return expression; - } - - @Override - public String toString() { - return MoreObjects.toStringHelper(this) - .add("title", title) - .add("description", description) - .add("expression", expression) - .toString(); - } - - @Override - public int hashCode() { - return Objects.hash(getClass(), title, description, expression); - } - - @Override - public boolean equals(Object obj) { - if (obj == this) { - return true; - } - if (!(obj instanceof Condition)) { - return false; - } - Condition other = (Condition) obj; - return Objects.equals(title, other.getTitle()) - && Objects.equals(description, other.getDescription()) - && Objects.equals(expression, other.getExpression()); + static Builder newBuilder() { + return new AutoValue_Condition.Builder(); } - /** Returns a builder for {@code Policy} objects. */ - public static Condition.Builder newBuilder() { - return new Condition.Builder(); + @AutoValue.Builder + abstract static class Builder { + abstract Builder setTitle(String title); + abstract Builder setDescription(String description); + abstract Builder setExpression(String expression); + abstract Condition build(); } -} +} \ No newline at end of file diff --git a/google-cloud-core/src/main/java/com/google/cloud/Policy.java b/google-cloud-core/src/main/java/com/google/cloud/Policy.java index 16712644d1..bd426a8fa7 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Policy.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Policy.java @@ -153,7 +153,7 @@ protected com.google.iam.v1.Policy toPb(Policy policy) { /** A builder for {@code Policy} objects. */ public static class Builder { - private final List bindingsList = new ArrayList(); + private final List bindingsList = new ArrayList(); private String etag; private int version; @@ -162,7 +162,9 @@ protected Builder() {} @InternalApi("This class should only be extended within google-cloud-java") protected Builder(Policy policy) { - setBindings(policy.bindingsList); + for (Binding binding : policy.bindingsList) { + bindingsList.add(binding.toBuilder().build()); + } setEtag(policy.etag); setVersion(policy.version); } @@ -214,7 +216,7 @@ public final Builder setBindings(List bindings) { this.bindingsList.clear(); for (Binding binding : bindings) { Binding.Builder bindingBuilder = Binding.newBuilder(); - bindingBuilder.setMembers(new ArrayList(binding.getMembers())); + bindingBuilder.setMembers(ImmutableList.copyOf(binding.getMembers())); bindingBuilder.setRole(binding.getRole()); bindingBuilder.setCondition(binding.getCondition()); this.bindingsList.add(bindingBuilder.build()); @@ -230,7 +232,6 @@ public final Builder removeRole(Role role) { for (int i = 0; i < bindingsList.size(); ++i) { Binding binding = bindingsList.get(i); if (binding.getRole().equals(role.getValue())) { - System.out.println(role.getValue() + " " + binding.getRole()); bindingsList.remove(i); return this; } @@ -263,12 +264,12 @@ public final Builder addIdentity(Role role, Identity first, Identity... others) return this; } } - List members = new ArrayList<>(); - members.add(first.strValue()); + Binding.Builder bindingBuilder = Binding.newBuilder().setRole(role.getValue()); + bindingBuilder.addMembers(first.strValue()); for (Identity identity : others) { - members.add(identity.strValue()); + bindingBuilder.addMembers(identity.strValue()); } - bindingsList.add(Binding.newBuilder().setRole(role.getValue()).setMembers(members).build()); + bindingsList.add(bindingBuilder.build()); return this; } diff --git a/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java b/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java index 5188fff7ae..6a311803cd 100644 --- a/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java +++ b/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java @@ -168,16 +168,10 @@ public void addBindingToPolicy() { @Test public void testIllegalPolicies() { try { - Binding.newBuilder().setRole(null); + Binding.newBuilder().setRole(null).build(); fail("Null role should cause exception."); } catch (NullPointerException ex) { - assertEquals("The role cannot be null.", ex.getMessage()); - } - try { - Binding.newBuilder().addMembers(null); - fail("Null member should cause exception."); - } catch (NullPointerException ex) { - assertEquals("Null identities are not permitted.", ex.getMessage()); + assertEquals("Null role", ex.getMessage()); } try { FULL_POLICY_V3.getBindings(); diff --git a/pom.xml b/pom.xml index 2718decfb9..cc2bfccb21 100644 --- a/pom.xml +++ b/pom.xml @@ -152,6 +152,8 @@ github google-cloud-core-parent + 1.7 + 1.7 1.52.0 1.8.1 1.17.0 @@ -231,7 +233,18 @@ pom import - + + com.google.auto.value + auto-value-annotations + ${auto-value-annotations.version} + true + + + com.google.auto.value + auto-value + ${auto-value.version} + true + com.google.api api-common @@ -247,7 +260,6 @@ proto-google-iam-v1 ${google.iam.version} - io.opencensus @@ -324,7 +336,20 @@ org.objenesis:objenesis + + maven-compiler-plugin + + + + com.google.auto.value + auto-value + ${auto-value.version} + + + + + From 54ad0764bc4c758b771772b0595796dd5c6a35bd Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Tue, 21 Jan 2020 18:40:52 -0800 Subject: [PATCH 08/31] reformat Binding.java and Condition.java --- .../main/java/com/google/cloud/Binding.java | 31 +++++++++++-------- .../main/java/com/google/cloud/Condition.java | 17 +++++++--- 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/google-cloud-core/src/main/java/com/google/cloud/Binding.java b/google-cloud-core/src/main/java/com/google/cloud/Binding.java index a457ae1ebf..a4a6889f35 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Binding.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Binding.java @@ -16,24 +16,23 @@ package com.google.cloud; -import java.lang.reflect.Array; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; - import com.google.auto.value.AutoValue; -import com.google.common.base.Preconditions; import com.google.common.base.Predicates; import com.google.common.collect.Collections2; import com.google.common.collect.ImmutableList; - +import java.util.Arrays; +import java.util.List; import javax.annotation.Nullable; @AutoValue abstract class Binding { abstract String getRole(); + abstract ImmutableList getMembers(); - @Nullable abstract Condition getCondition(); + + @Nullable + abstract Condition getCondition(); + public abstract Builder toBuilder(); static Builder newBuilder() { @@ -43,17 +42,21 @@ static Builder newBuilder() { @AutoValue.Builder abstract static class Builder { abstract Builder setRole(String role); + abstract Builder setMembers(List members); + abstract Builder setCondition(Condition condition); abstract String getRole(); + abstract ImmutableList getMembers(); + abstract Condition getCondition(); public abstract ImmutableList.Builder membersBuilder(); - //public abstract Condition.Builder conditionBuilder(); + // public abstract Condition.Builder conditionBuilder(); - public Builder addMembers(String... members){ + public Builder addMembers(String... members) { for (String member : members) { membersBuilder().add(member); } @@ -61,11 +64,13 @@ public Builder addMembers(String... members){ } public Builder removeMembers(String... members) { - setMembers(ImmutableList.copyOf(Collections2.filter(getMembers(), - Predicates.not(Predicates.in(Arrays.asList(members)))))); + setMembers( + ImmutableList.copyOf( + Collections2.filter( + getMembers(), Predicates.not(Predicates.in(Arrays.asList(members)))))); return this; } abstract Binding build(); } -} \ No newline at end of file +} diff --git a/google-cloud-core/src/main/java/com/google/cloud/Condition.java b/google-cloud-core/src/main/java/com/google/cloud/Condition.java index 744686ab08..02225dceaf 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Condition.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Condition.java @@ -21,9 +21,15 @@ @AutoValue abstract class Condition { - @Nullable abstract String getTitle(); - @Nullable abstract String getDescription(); - @Nullable abstract String getExpression(); + @Nullable + abstract String getTitle(); + + @Nullable + abstract String getDescription(); + + @Nullable + abstract String getExpression(); + public abstract Builder toBuilder(); static Builder newBuilder() { @@ -33,8 +39,11 @@ static Builder newBuilder() { @AutoValue.Builder abstract static class Builder { abstract Builder setTitle(String title); + abstract Builder setDescription(String description); + abstract Builder setExpression(String expression); + abstract Condition build(); } -} \ No newline at end of file +} From ff496207608c756463fa3a4610e3c4731c917e21 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Tue, 21 Jan 2020 21:04:41 -0800 Subject: [PATCH 09/31] remove unnecessary dep --- google-cloud-core/pom.xml | 5 ----- .../src/main/java/com/google/cloud/Binding.java | 2 ++ pom.xml | 6 ------ 3 files changed, 2 insertions(+), 11 deletions(-) diff --git a/google-cloud-core/pom.xml b/google-cloud-core/pom.xml index 05c2d4f8a4..390b78176c 100644 --- a/google-cloud-core/pom.xml +++ b/google-cloud-core/pom.xml @@ -44,11 +44,6 @@ true - com.google.auto.value - auto-value - true - - com.google.protobuf protobuf-java-util diff --git a/google-cloud-core/src/main/java/com/google/cloud/Binding.java b/google-cloud-core/src/main/java/com/google/cloud/Binding.java index a4a6889f35..f10a27117c 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Binding.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Binding.java @@ -18,8 +18,10 @@ import com.google.auto.value.AutoValue; import com.google.common.base.Predicates; + import com.google.common.collect.Collections2; import com.google.common.collect.ImmutableList; + import java.util.Arrays; import java.util.List; import javax.annotation.Nullable; diff --git a/pom.xml b/pom.xml index 921b031a5b..85d5848ed4 100644 --- a/pom.xml +++ b/pom.xml @@ -239,12 +239,6 @@ ${auto-value-annotations.version} true - - com.google.auto.value - auto-value - ${auto-value.version} - true - com.google.api api-common From 298b2be051e7a3a899f68fc76ee140a9e05eeb19 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Tue, 21 Jan 2020 21:07:50 -0800 Subject: [PATCH 10/31] code format --- google-cloud-core/src/main/java/com/google/cloud/Binding.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/google-cloud-core/src/main/java/com/google/cloud/Binding.java b/google-cloud-core/src/main/java/com/google/cloud/Binding.java index f10a27117c..a4a6889f35 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Binding.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Binding.java @@ -18,10 +18,8 @@ import com.google.auto.value.AutoValue; import com.google.common.base.Predicates; - import com.google.common.collect.Collections2; import com.google.common.collect.ImmutableList; - import java.util.Arrays; import java.util.List; import javax.annotation.Nullable; From aaebba430c28e8714d836792354602ec5971ac23 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Tue, 21 Jan 2020 21:11:56 -0800 Subject: [PATCH 11/31] add dep on com.google.code.findbugs in google-cloud-core --- google-cloud-core/pom.xml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/google-cloud-core/pom.xml b/google-cloud-core/pom.xml index 390b78176c..de68fc0081 100644 --- a/google-cloud-core/pom.xml +++ b/google-cloud-core/pom.xml @@ -88,6 +88,10 @@ guava + + com.google.code.findbugs + jsr305 + com.google.truth truth From a5f63eaa97fac0545578d836a40ab0bf1fb17f71 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Thu, 6 Feb 2020 10:39:57 -0800 Subject: [PATCH 12/31] address comments --- .../main/java/com/google/cloud/Binding.java | 27 ++++++++------- .../main/java/com/google/cloud/Condition.java | 20 +++++------ .../main/java/com/google/cloud/Policy.java | 33 ++++++++----------- .../java/com/google/cloud/PolicyV3Test.java | 10 ++++-- 4 files changed, 45 insertions(+), 45 deletions(-) diff --git a/google-cloud-core/src/main/java/com/google/cloud/Binding.java b/google-cloud-core/src/main/java/com/google/cloud/Binding.java index a4a6889f35..695b545c50 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Binding.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Binding.java @@ -25,36 +25,35 @@ import javax.annotation.Nullable; @AutoValue -abstract class Binding { - abstract String getRole(); +public abstract class Binding { + public abstract String getRole(); - abstract ImmutableList getMembers(); + public abstract ImmutableList getMembers(); @Nullable - abstract Condition getCondition(); + public abstract Condition getCondition(); public abstract Builder toBuilder(); - static Builder newBuilder() { + public static Builder newBuilder() { return new AutoValue_Binding.Builder(); } @AutoValue.Builder - abstract static class Builder { - abstract Builder setRole(String role); + public abstract static class Builder { + public abstract Builder setRole(String role); - abstract Builder setMembers(List members); + public abstract Builder setMembers(List members); - abstract Builder setCondition(Condition condition); + public abstract Builder setCondition(Condition condition); - abstract String getRole(); + public abstract String getRole(); - abstract ImmutableList getMembers(); + public abstract ImmutableList getMembers(); - abstract Condition getCondition(); + public abstract Condition getCondition(); public abstract ImmutableList.Builder membersBuilder(); - // public abstract Condition.Builder conditionBuilder(); public Builder addMembers(String... members) { for (String member : members) { @@ -71,6 +70,6 @@ public Builder removeMembers(String... members) { return this; } - abstract Binding build(); + public abstract Binding build(); } } diff --git a/google-cloud-core/src/main/java/com/google/cloud/Condition.java b/google-cloud-core/src/main/java/com/google/cloud/Condition.java index 02225dceaf..3de5d67271 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Condition.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Condition.java @@ -20,30 +20,30 @@ import javax.annotation.Nullable; @AutoValue -abstract class Condition { +public abstract class Condition { @Nullable - abstract String getTitle(); + public abstract String getTitle(); @Nullable - abstract String getDescription(); + public abstract String getDescription(); @Nullable - abstract String getExpression(); + public abstract String getExpression(); public abstract Builder toBuilder(); - static Builder newBuilder() { + public static Builder newBuilder() { return new AutoValue_Condition.Builder(); } @AutoValue.Builder - abstract static class Builder { - abstract Builder setTitle(String title); + public abstract static class Builder { + public abstract Builder setTitle(String title); - abstract Builder setDescription(String description); + public abstract Builder setDescription(String description); - abstract Builder setExpression(String expression); + public abstract Builder setExpression(String expression); - abstract Condition build(); + public abstract Condition build(); } } diff --git a/google-cloud-core/src/main/java/com/google/cloud/Policy.java b/google-cloud-core/src/main/java/com/google/cloud/Policy.java index bd426a8fa7..ad746da71b 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Policy.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Policy.java @@ -52,19 +52,19 @@ public final class Policy implements Serializable { private final int version; /* - * Return false if Policy is version 3 OR bindings has a conditional binding. - * Return true if Policy is version 1 AND bindings does not have a conditional binding. + * Return true if Policy is version 3 OR bindings has a conditional binding. + * Return false if Policy is version 1 AND bindings does not have a conditional binding. */ - private static boolean checkVersion(int version, List bindingsList) { + private static boolean isConditional(int version, List bindingsList) { for (Binding binding : bindingsList) { if (binding.getCondition() != null) { - return false; + return true; } } if (version > 1) { - return false; + return true; } - return true; + return false; } public abstract static class Marshaller { @@ -178,8 +178,7 @@ protected Builder(Policy policy) { */ public final Builder setBindings(Map> bindings) { checkNotNull(bindings, "The provided map of bindings cannot be null."); - checkArgument( - checkVersion(this.version, this.bindingsList), + checkArgument(!isConditional(this.version, this.bindingsList), "setBindings() is only supported with version 1 policies and non-conditional policies"); for (Map.Entry> binding : bindings.entrySet()) { checkNotNull(binding.getKey(), "The role cannot be null."); @@ -226,8 +225,7 @@ public final Builder setBindings(List bindings) { /** Removes the role (and all identities associated with that role) from the policy. */ public final Builder removeRole(Role role) { - checkArgument( - checkVersion(this.version, this.bindingsList), + checkArgument(!isConditional(this.version, this.bindingsList), "removeRole() is only supported with version 1 policies and non-conditional policies"); for (int i = 0; i < bindingsList.size(); ++i) { Binding binding = bindingsList.get(i); @@ -246,8 +244,7 @@ public final Builder removeRole(Role role) { * @throws IllegalArgumentException if policy version is equal to 3 or has conditional bindings. */ public final Builder addIdentity(Role role, Identity first, Identity... others) { - checkArgument( - checkVersion(this.version, this.bindingsList), + checkArgument(!isConditional(this.version, this.bindingsList), "addIdentity() is only supported with version 1 policies and non-conditional policies"); String nullIdentityMessage = "Null identities are not permitted."; checkNotNull(first, nullIdentityMessage); @@ -280,8 +277,7 @@ public final Builder addIdentity(Role role, Identity first, Identity... others) * @throws IllegalArgumentException if policy version is equal to 3 or has conditional bindings. */ public final Builder removeIdentity(Role role, Identity first, Identity... others) { - checkArgument( - checkVersion(this.version, this.bindingsList), + checkArgument(!isConditional(this.version, this.bindingsList), "removeIdentity() is only supported with version 1 policies and non-conditional policies"); String nullIdentityMessage = "Null identities are not permitted."; checkNotNull(first, nullIdentityMessage); @@ -323,10 +319,11 @@ public final Builder setEtag(String etag) { } /** - * Sets the version of the policy. The default version is 0, meaning only the "owner", "editor", - * and "viewer" roles are permitted. If the version is 1, you may also use other roles. + * Sets the version of the policy. */ public final Builder setVersion(int version) { + //checkArgument(version==1 && !isConditional(version, this.bindingsList), + // "Version 3 is required for conditional policies."); this.version = version; return this; } @@ -338,7 +335,6 @@ public final Policy build() { } private Policy(Builder builder) { - this.bindingsList = ImmutableList.copyOf(builder.bindingsList); this.etag = builder.etag; this.version = builder.version; @@ -355,8 +351,7 @@ public Builder toBuilder() { * @throws IllegalArgumentException if policy version is equal to 3 or has conditional bindings. */ public Map> getBindings() { - checkArgument( - checkVersion(this.version, this.bindingsList), + checkArgument(!isConditional(this.version, this.bindingsList), "getBindings() is only supported with version 1 policies and non-conditional policies"); ImmutableMap.Builder> bindingsV1Builder = ImmutableMap.builder(); for (Binding binding : bindingsList) { diff --git a/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java b/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java index 6a311803cd..bfb89fc2e1 100644 --- a/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java +++ b/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java @@ -16,14 +16,20 @@ package com.google.cloud; -import static org.junit.Assert.*; - import com.google.cloud.Policy.DefaultMarshaller; import com.google.common.collect.ImmutableList; import java.util.ArrayList; import java.util.List; import org.junit.Test; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + + public class PolicyV3Test { private static final String ALL_USERS = "allUsers"; From c853a84a21d066e8761250ae8d2e1b78b74fc649 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Mon, 10 Feb 2020 21:11:54 -0800 Subject: [PATCH 13/31] Clean up --- .../main/java/com/google/cloud/Policy.java | 21 ++++++++++--------- .../java/com/google/cloud/PolicyV3Test.java | 11 +++++----- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/google-cloud-core/src/main/java/com/google/cloud/Policy.java b/google-cloud-core/src/main/java/com/google/cloud/Policy.java index ad746da71b..e1647a0c3d 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Policy.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Policy.java @@ -178,7 +178,8 @@ protected Builder(Policy policy) { */ public final Builder setBindings(Map> bindings) { checkNotNull(bindings, "The provided map of bindings cannot be null."); - checkArgument(!isConditional(this.version, this.bindingsList), + checkArgument( + !isConditional(this.version, this.bindingsList), "setBindings() is only supported with version 1 policies and non-conditional policies"); for (Map.Entry> binding : bindings.entrySet()) { checkNotNull(binding.getKey(), "The role cannot be null."); @@ -225,7 +226,8 @@ public final Builder setBindings(List bindings) { /** Removes the role (and all identities associated with that role) from the policy. */ public final Builder removeRole(Role role) { - checkArgument(!isConditional(this.version, this.bindingsList), + checkArgument( + !isConditional(this.version, this.bindingsList), "removeRole() is only supported with version 1 policies and non-conditional policies"); for (int i = 0; i < bindingsList.size(); ++i) { Binding binding = bindingsList.get(i); @@ -244,7 +246,8 @@ public final Builder removeRole(Role role) { * @throws IllegalArgumentException if policy version is equal to 3 or has conditional bindings. */ public final Builder addIdentity(Role role, Identity first, Identity... others) { - checkArgument(!isConditional(this.version, this.bindingsList), + checkArgument( + !isConditional(this.version, this.bindingsList), "addIdentity() is only supported with version 1 policies and non-conditional policies"); String nullIdentityMessage = "Null identities are not permitted."; checkNotNull(first, nullIdentityMessage); @@ -277,7 +280,8 @@ public final Builder addIdentity(Role role, Identity first, Identity... others) * @throws IllegalArgumentException if policy version is equal to 3 or has conditional bindings. */ public final Builder removeIdentity(Role role, Identity first, Identity... others) { - checkArgument(!isConditional(this.version, this.bindingsList), + checkArgument( + !isConditional(this.version, this.bindingsList), "removeIdentity() is only supported with version 1 policies and non-conditional policies"); String nullIdentityMessage = "Null identities are not permitted."; checkNotNull(first, nullIdentityMessage); @@ -318,12 +322,8 @@ public final Builder setEtag(String etag) { return this; } - /** - * Sets the version of the policy. - */ + /** Sets the version of the policy. */ public final Builder setVersion(int version) { - //checkArgument(version==1 && !isConditional(version, this.bindingsList), - // "Version 3 is required for conditional policies."); this.version = version; return this; } @@ -351,7 +351,8 @@ public Builder toBuilder() { * @throws IllegalArgumentException if policy version is equal to 3 or has conditional bindings. */ public Map> getBindings() { - checkArgument(!isConditional(this.version, this.bindingsList), + checkArgument( + !isConditional(this.version, this.bindingsList), "getBindings() is only supported with version 1 policies and non-conditional policies"); ImmutableMap.Builder> bindingsV1Builder = ImmutableMap.builder(); for (Binding binding : bindingsList) { diff --git a/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java b/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java index bfb89fc2e1..fdfae16180 100644 --- a/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java +++ b/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java @@ -16,12 +16,6 @@ package com.google.cloud; -import com.google.cloud.Policy.DefaultMarshaller; -import com.google.common.collect.ImmutableList; -import java.util.ArrayList; -import java.util.List; -import org.junit.Test; - import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; @@ -29,6 +23,11 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import com.google.cloud.Policy.DefaultMarshaller; +import com.google.common.collect.ImmutableList; +import java.util.ArrayList; +import java.util.List; +import org.junit.Test; public class PolicyV3Test { From d2fab21ceedfc3a1c3f5f3c5e02d333f9322d01a Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Tue, 18 Feb 2020 21:35:09 -0800 Subject: [PATCH 14/31] respond to comments --- .../src/main/java/com/google/cloud/Policy.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/google-cloud-core/src/main/java/com/google/cloud/Policy.java b/google-cloud-core/src/main/java/com/google/cloud/Policy.java index e1647a0c3d..0bfc5eabae 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Policy.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Policy.java @@ -96,7 +96,7 @@ public static class DefaultMarshaller extends Marshaller bindingsList = new ArrayList(); + ImmutableList.Builder bindingsListBuilder = ImmutableList.builder(); for (com.google.iam.v1.Binding bindingPb : policyPb.getBindingsList()) { Binding.Builder convertedBinding = Binding.newBuilder() @@ -111,10 +111,10 @@ protected Policy fromPb(com.google.iam.v1.Policy policyPb) { .setExpression(expr.getExpression()) .build()); } - bindingsList.add(convertedBinding.build()); + bindingsListBuilder.add(convertedBinding.build()); } return newBuilder() - .setBindings(bindingsList) + .setBindings(bindingsListBuilder.build()) .setEtag( policyPb.getEtag().isEmpty() ? null From 085959d388f9f265a46fbdf49dc3c6a1020a9d60 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Wed, 19 Feb 2020 16:19:36 -0800 Subject: [PATCH 15/31] address comments --- .../main/java/com/google/cloud/Binding.java | 9 +++--- .../main/java/com/google/cloud/Policy.java | 32 +++++++++++++------ 2 files changed, 27 insertions(+), 14 deletions(-) diff --git a/google-cloud-core/src/main/java/com/google/cloud/Binding.java b/google-cloud-core/src/main/java/com/google/cloud/Binding.java index 695b545c50..b5f75708e9 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Binding.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Binding.java @@ -17,10 +17,12 @@ package com.google.cloud; import com.google.auto.value.AutoValue; +import com.google.common.base.Predicate; import com.google.common.base.Predicates; import com.google.common.collect.Collections2; import com.google.common.collect.ImmutableList; import java.util.Arrays; +import java.util.Collection; import java.util.List; import javax.annotation.Nullable; @@ -63,10 +65,9 @@ public Builder addMembers(String... members) { } public Builder removeMembers(String... members) { - setMembers( - ImmutableList.copyOf( - Collections2.filter( - getMembers(), Predicates.not(Predicates.in(Arrays.asList(members)))))); + Predicate selectMembersNotInList = Predicates.not(Predicates.in(Arrays.asList(members))); + Collection filter = Collections2.filter(getMembers(), selectMembersNotInList); + setMembers(ImmutableList.copyOf(filter)); return this; } diff --git a/google-cloud-core/src/main/java/com/google/cloud/Policy.java b/google-cloud-core/src/main/java/com/google/cloud/Policy.java index 0bfc5eabae..521e825f1c 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Policy.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Policy.java @@ -21,6 +21,7 @@ import com.google.api.core.ApiFunction; import com.google.api.core.InternalApi; +import com.google.api.gax.retrying.RetrySettings; import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -30,6 +31,7 @@ import com.google.type.Expr; import java.io.Serializable; import java.util.ArrayList; +import java.util.Iterator; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -61,7 +63,7 @@ private static boolean isConditional(int version, List bindingsList) { return true; } } - if (version > 1) { + if (version == 3) { return true; } return false; @@ -229,10 +231,13 @@ public final Builder removeRole(Role role) { checkArgument( !isConditional(this.version, this.bindingsList), "removeRole() is only supported with version 1 policies and non-conditional policies"); - for (int i = 0; i < bindingsList.size(); ++i) { - Binding binding = bindingsList.get(i); + Iterator iterator = bindingsList.iterator(); + + while (iterator.hasNext()) + { + Binding binding = (Binding)iterator.next(); if (binding.getRole().equals(role.getValue())) { - bindingsList.remove(i); + iterator.remove(); return this; } } @@ -295,14 +300,21 @@ public final Builder removeIdentity(Role role, Identity first, Identity... other bindingBuilder.removeMembers(identity.strValue()); } Binding updatedBindings = bindingBuilder.build(); - if (updatedBindings.getMembers().size() == 0) { - bindingsList.remove(i); - } else { - bindingsList.set(i, updatedBindings); - } - return this; + bindingsList.set(i, updatedBindings); + break; } } + + Iterator iterator = bindingsList.iterator(); + while (iterator.hasNext()) + { + Binding binding = (Binding)iterator.next(); + if (binding.getRole().equals(role.getValue()) && binding.getMembers().size() == 0) { + iterator.remove(); + break; + } + } + return this; } From 174e8c4a9bbedf9532bd0313ee4566eb269e44b5 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Wed, 19 Feb 2020 18:28:42 -0800 Subject: [PATCH 16/31] format --- .../src/main/java/com/google/cloud/Binding.java | 3 ++- .../src/main/java/com/google/cloud/Policy.java | 11 ++++------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/google-cloud-core/src/main/java/com/google/cloud/Binding.java b/google-cloud-core/src/main/java/com/google/cloud/Binding.java index b5f75708e9..359be69e72 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Binding.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Binding.java @@ -65,7 +65,8 @@ public Builder addMembers(String... members) { } public Builder removeMembers(String... members) { - Predicate selectMembersNotInList = Predicates.not(Predicates.in(Arrays.asList(members))); + Predicate selectMembersNotInList = + Predicates.not(Predicates.in(Arrays.asList(members))); Collection filter = Collections2.filter(getMembers(), selectMembersNotInList); setMembers(ImmutableList.copyOf(filter)); return this; diff --git a/google-cloud-core/src/main/java/com/google/cloud/Policy.java b/google-cloud-core/src/main/java/com/google/cloud/Policy.java index 521e825f1c..51ab954291 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Policy.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Policy.java @@ -21,7 +21,6 @@ import com.google.api.core.ApiFunction; import com.google.api.core.InternalApi; -import com.google.api.gax.retrying.RetrySettings; import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -233,9 +232,8 @@ public final Builder removeRole(Role role) { "removeRole() is only supported with version 1 policies and non-conditional policies"); Iterator iterator = bindingsList.iterator(); - while (iterator.hasNext()) - { - Binding binding = (Binding)iterator.next(); + while (iterator.hasNext()) { + Binding binding = (Binding) iterator.next(); if (binding.getRole().equals(role.getValue())) { iterator.remove(); return this; @@ -306,9 +304,8 @@ public final Builder removeIdentity(Role role, Identity first, Identity... other } Iterator iterator = bindingsList.iterator(); - while (iterator.hasNext()) - { - Binding binding = (Binding)iterator.next(); + while (iterator.hasNext()) { + Binding binding = (Binding) iterator.next(); if (binding.getRole().equals(role.getValue()) && binding.getMembers().size() == 0) { iterator.remove(); break; From 14e1aaca23afd9af1f7f8149653ed8a195b489fa Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Thu, 20 Feb 2020 21:41:09 -0800 Subject: [PATCH 17/31] address feedback --- google-cloud-core/pom.xml | 1 - .../src/main/java/com/google/cloud/Binding.java | 1 + .../src/main/java/com/google/cloud/Policy.java | 17 +++++++++-------- .../java/com/google/cloud/PolicyV3Test.java | 12 +++++++----- 4 files changed, 17 insertions(+), 14 deletions(-) diff --git a/google-cloud-core/pom.xml b/google-cloud-core/pom.xml index a0b74f071b..6789692116 100644 --- a/google-cloud-core/pom.xml +++ b/google-cloud-core/pom.xml @@ -30,7 +30,6 @@ com.google.auto.value auto-value-annotations - true com.google.protobuf diff --git a/google-cloud-core/src/main/java/com/google/cloud/Binding.java b/google-cloud-core/src/main/java/com/google/cloud/Binding.java index 359be69e72..7f06da8e56 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Binding.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Binding.java @@ -59,6 +59,7 @@ public abstract static class Builder { public Builder addMembers(String... members) { for (String member : members) { + membersBuilder().add(member); } return this; diff --git a/google-cloud-core/src/main/java/com/google/cloud/Policy.java b/google-cloud-core/src/main/java/com/google/cloud/Policy.java index 51ab954291..3e71a29a06 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Policy.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Policy.java @@ -39,7 +39,7 @@ /** * Class for Identity and Access Management (IAM) policies. IAM policies are used to specify access - * settings for Cloud Platform resources. A policy is a map of bindings. A binding assigns a set of + * settings for Cloud Platform resources. A policy is a list of bindings. A binding assigns a set of * identities to a role, where the identities can be user accounts, Google groups, Google domains, * and service accounts. A role is a named list of permissions defined by IAM. * @@ -174,8 +174,8 @@ protected Builder(Policy policy) { * Replaces the builder's map of bindings with the given map of bindings. * * @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 - * @throws IllegalArgumentException if policy version is equal to 3 or has conditional bindings. + * @throws IllegalArgumentException if any identities in the given map are null or if policy + * version is equal to 3 or has conditional bindings. */ public final Builder setBindings(Map> bindings) { checkNotNull(bindings, "The provided map of bindings cannot be null."); @@ -204,7 +204,8 @@ public final Builder setBindings(Map> bindings) { /** * Replaces the builder's List of bindings with the given List of Bindings. * - * @throws NullPointerException if the given map is null or contains any null keys or values + * @throws NullPointerException if the given list is null or contains any null members in + * bindings. * @throws IllegalArgumentException if any identities in the given map are null */ public final Builder setBindings(List bindings) { @@ -256,7 +257,7 @@ public final Builder addIdentity(Role role, Identity first, Identity... others) checkNotNull(first, nullIdentityMessage); checkNotNull(others, nullIdentityMessage); checkNotNull(role, "The role cannot be null."); - for (int i = 0; i < bindingsList.size(); ++i) { + for (int i = 0; i < bindingsList.size(); i++) { Binding binding = bindingsList.get(i); if (binding.getRole().equals(role.getValue())) { Binding.Builder bindingBuilder = binding.toBuilder().addMembers(first.strValue()); @@ -290,7 +291,7 @@ public final Builder removeIdentity(Role role, Identity first, Identity... other checkNotNull(first, nullIdentityMessage); checkNotNull(others, nullIdentityMessage); checkNotNull(role, "The role cannot be null."); - for (int i = 0; i < bindingsList.size(); ++i) { + for (int i = 0; i < bindingsList.size(); i++) { Binding binding = bindingsList.get(i); if (binding.getRole().equals(role.getValue())) { Binding.Builder bindingBuilder = binding.toBuilder().removeMembers(first.strValue()); @@ -374,7 +375,7 @@ public Map> getBindings() { return bindingsV1Builder.build(); } - /** Returns the map of bindings that comprises the policy for version 3. */ + /** Returns the list of bindings that comprises the policy for version 3. */ public List getBindingsList() { return bindingsList; } @@ -428,7 +429,7 @@ public boolean equals(Object obj) { if (bindingsList.size() != other.getBindingsList().size()) { return false; } - for (int i = 0; i < bindingsList.size(); ++i) { + for (int i = 0; i < bindingsList.size(); i++) { bindingsList.get(i).equals(other.getBindingsList().get(i)); } return Objects.equals(etag, other.getEtag()) && Objects.equals(version, other.getVersion()); diff --git a/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java b/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java index fdfae16180..2f68883a58 100644 --- a/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java +++ b/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java @@ -26,6 +26,7 @@ import com.google.cloud.Policy.DefaultMarshaller; import com.google.common.collect.ImmutableList; import java.util.ArrayList; +import java.util.Iterator; import java.util.List; import org.junit.Test; @@ -116,7 +117,7 @@ public void removeMemberFromPolicy() { assertEquals(3, FULL_POLICY_V3.getBindingsList().get(0).getMembers().size()); List bindings = new ArrayList<>(FULL_POLICY_V3.getBindingsList()); - for (int i = 0; i < bindings.size(); ++i) { + for (int i = 0; i < bindings.size(); i++) { Binding binding = bindings.get(i); if (binding.getRole().equals(VIEWER)) { bindings.set(i, binding.toBuilder().removeMembers(ALL_USERS).build()); @@ -133,7 +134,7 @@ public void addMemberFromPolicy() { assertEquals(3, FULL_POLICY_V3.getBindingsList().get(0).getMembers().size()); List bindings = new ArrayList<>(FULL_POLICY_V3.getBindingsList()); - for (int i = 0; i < bindings.size(); ++i) { + for (int i = 0; i < bindings.size(); i++) { Binding binding = bindings.get(i); if (binding.getRole().equals(VIEWER)) { bindings.set(i, binding.toBuilder().addMembers("user:example@example.com").build()); @@ -149,10 +150,11 @@ public void removeBindingFromPolicy() { assertEquals(2, FULL_POLICY_V3.getBindingsList().size()); List bindings = new ArrayList<>(FULL_POLICY_V3.getBindingsList()); - for (int i = 0; i < bindings.size(); ++i) { - Binding binding = bindings.get(i); + Iterator iterator = bindings.iterator(); + while (iterator.hasNext()) { + Binding binding = (Binding) iterator.next(); if (binding.getRole().equals(EDITOR) && binding.getCondition() == null) { - bindings.remove(i); + iterator.remove(); break; } } From fdb040a13e19a2d699453ec5a85970c9c82167ab Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Fri, 21 Feb 2020 00:16:30 -0700 Subject: [PATCH 18/31] remove unnecessary null check --- .../src/main/java/com/google/cloud/Binding.java | 3 ++- .../src/main/java/com/google/cloud/Policy.java | 12 +++--------- .../test/java/com/google/cloud/PolicyV3Test.java | 15 +++++++++++++++ 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/google-cloud-core/src/main/java/com/google/cloud/Binding.java b/google-cloud-core/src/main/java/com/google/cloud/Binding.java index 7f06da8e56..cd78d904ee 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Binding.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Binding.java @@ -26,6 +26,8 @@ import java.util.List; import javax.annotation.Nullable; +import static com.google.common.base.Preconditions.checkArgument; + @AutoValue public abstract class Binding { public abstract String getRole(); @@ -59,7 +61,6 @@ public abstract static class Builder { public Builder addMembers(String... members) { for (String member : members) { - membersBuilder().add(member); } return this; diff --git a/google-cloud-core/src/main/java/com/google/cloud/Policy.java b/google-cloud-core/src/main/java/com/google/cloud/Policy.java index 3e71a29a06..effccaac3c 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Policy.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Policy.java @@ -204,21 +204,15 @@ public final Builder setBindings(Map> bindings) { /** * Replaces the builder's List of bindings with the given List of Bindings. * - * @throws NullPointerException if the given list is null or contains any null members in + * @throws NullPointerException if the given list is null, role is null, or contains any null members in * bindings. - * @throws IllegalArgumentException if any identities in the given map are null */ public final Builder setBindings(List bindings) { - for (Binding binding : bindings) { - checkNotNull(binding.getRole(), "The role cannot be null."); - List members = binding.getMembers(); - checkNotNull(members, "A role cannot be assigned to a null set of identities."); - checkArgument(!members.contains(null), "Null identities are not permitted."); - } this.bindingsList.clear(); for (Binding binding : bindings) { + checkNotNull(binding.getRole(), "The role cannot be null."); Binding.Builder bindingBuilder = Binding.newBuilder(); - bindingBuilder.setMembers(ImmutableList.copyOf(binding.getMembers())); + bindingBuilder.addMembers(binding.getMembers().toArray(new String[binding.getMembers().size()])); bindingBuilder.setRole(binding.getRole()); bindingBuilder.setCondition(binding.getCondition()); this.bindingsList.add(bindingBuilder.build()); diff --git a/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java b/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java index 2f68883a58..c9da2198fe 100644 --- a/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java +++ b/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java @@ -25,7 +25,10 @@ import com.google.cloud.Policy.DefaultMarshaller; import com.google.common.collect.ImmutableList; + +import java.lang.reflect.Array; import java.util.ArrayList; +import java.util.Arrays; import java.util.Iterator; import java.util.List; import org.junit.Test; @@ -180,6 +183,18 @@ public void testIllegalPolicies() { } catch (NullPointerException ex) { assertEquals("Null role", ex.getMessage()); } + try { + FULL_POLICY_V3.toBuilder().setBindings(Arrays.asList(Binding.newBuilder().setRole("test").setMembers(Arrays.asList(null, "user")).build())).build(); + fail("Null member should cause exception."); + } catch (NullPointerException ex) { + assertEquals("at index 0", ex.getMessage()); + } + try { + FULL_POLICY_V3.toBuilder().setBindings(Arrays.asList(Binding.newBuilder().setRole("test").addMembers(null, "user").build())).build(); + fail("Null member should cause exception."); + } catch (NullPointerException ex) { + assertNull(ex.getMessage()); + } try { FULL_POLICY_V3.getBindings(); fail("getBindings() should cause exception with Policy V3."); From 42199d181d5396d986fe90babde31ef9a8106477 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Fri, 21 Feb 2020 00:22:46 -0700 Subject: [PATCH 19/31] lint --- .../main/java/com/google/cloud/Binding.java | 2 -- .../src/main/java/com/google/cloud/Policy.java | 7 ++++--- .../java/com/google/cloud/PolicyV3Test.java | 18 ++++++++++++++---- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/google-cloud-core/src/main/java/com/google/cloud/Binding.java b/google-cloud-core/src/main/java/com/google/cloud/Binding.java index cd78d904ee..359be69e72 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Binding.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Binding.java @@ -26,8 +26,6 @@ import java.util.List; import javax.annotation.Nullable; -import static com.google.common.base.Preconditions.checkArgument; - @AutoValue public abstract class Binding { public abstract String getRole(); diff --git a/google-cloud-core/src/main/java/com/google/cloud/Policy.java b/google-cloud-core/src/main/java/com/google/cloud/Policy.java index effccaac3c..dadf1cdb35 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Policy.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Policy.java @@ -204,15 +204,16 @@ public final Builder setBindings(Map> bindings) { /** * Replaces the builder's List of bindings with the given List of Bindings. * - * @throws NullPointerException if the given list is null, role is null, or contains any null members in - * bindings. + * @throws NullPointerException if the given list is null, role is null, or contains any null + * members in bindings. */ public final Builder setBindings(List bindings) { this.bindingsList.clear(); for (Binding binding : bindings) { checkNotNull(binding.getRole(), "The role cannot be null."); Binding.Builder bindingBuilder = Binding.newBuilder(); - bindingBuilder.addMembers(binding.getMembers().toArray(new String[binding.getMembers().size()])); + bindingBuilder.addMembers( + binding.getMembers().toArray(new String[binding.getMembers().size()])); bindingBuilder.setRole(binding.getRole()); bindingBuilder.setCondition(binding.getCondition()); this.bindingsList.add(bindingBuilder.build()); diff --git a/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java b/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java index c9da2198fe..d700526076 100644 --- a/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java +++ b/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java @@ -25,8 +25,6 @@ import com.google.cloud.Policy.DefaultMarshaller; import com.google.common.collect.ImmutableList; - -import java.lang.reflect.Array; import java.util.ArrayList; import java.util.Arrays; import java.util.Iterator; @@ -184,13 +182,25 @@ public void testIllegalPolicies() { assertEquals("Null role", ex.getMessage()); } try { - FULL_POLICY_V3.toBuilder().setBindings(Arrays.asList(Binding.newBuilder().setRole("test").setMembers(Arrays.asList(null, "user")).build())).build(); + FULL_POLICY_V3 + .toBuilder() + .setBindings( + Arrays.asList( + Binding.newBuilder() + .setRole("test") + .setMembers(Arrays.asList(null, "user")) + .build())) + .build(); fail("Null member should cause exception."); } catch (NullPointerException ex) { assertEquals("at index 0", ex.getMessage()); } try { - FULL_POLICY_V3.toBuilder().setBindings(Arrays.asList(Binding.newBuilder().setRole("test").addMembers(null, "user").build())).build(); + FULL_POLICY_V3 + .toBuilder() + .setBindings( + Arrays.asList(Binding.newBuilder().setRole("test").addMembers(null, "user").build())) + .build(); fail("Null member should cause exception."); } catch (NullPointerException ex) { assertNull(ex.getMessage()); From bbb708a8d95d0ea526e9338a719309b217006bfe Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Fri, 21 Feb 2020 00:27:43 -0700 Subject: [PATCH 20/31] address feedback --- google-cloud-core/src/main/java/com/google/cloud/Policy.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/google-cloud-core/src/main/java/com/google/cloud/Policy.java b/google-cloud-core/src/main/java/com/google/cloud/Policy.java index dadf1cdb35..dd3e86e625 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Policy.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Policy.java @@ -210,10 +210,8 @@ public final Builder setBindings(Map> bindings) { public final Builder setBindings(List bindings) { this.bindingsList.clear(); for (Binding binding : bindings) { - checkNotNull(binding.getRole(), "The role cannot be null."); Binding.Builder bindingBuilder = Binding.newBuilder(); - bindingBuilder.addMembers( - binding.getMembers().toArray(new String[binding.getMembers().size()])); + bindingBuilder.setMembers(ImmutableList.copyOf(binding.getMembers())); bindingBuilder.setRole(binding.getRole()); bindingBuilder.setCondition(binding.getCondition()); this.bindingsList.add(bindingBuilder.build()); From 7f0e33e7982e99b808bb4b820e4491c73c5f10c9 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Fri, 21 Feb 2020 01:07:02 -0700 Subject: [PATCH 21/31] remove ImmutableList from Binding AutoValue surface --- .../main/java/com/google/cloud/Binding.java | 13 ++++++++----- .../main/java/com/google/cloud/Policy.java | 19 ++++++++++++++----- .../java/com/google/cloud/PolicyV3Test.java | 13 ++----------- 3 files changed, 24 insertions(+), 21 deletions(-) diff --git a/google-cloud-core/src/main/java/com/google/cloud/Binding.java b/google-cloud-core/src/main/java/com/google/cloud/Binding.java index 359be69e72..d0427081aa 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Binding.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Binding.java @@ -30,7 +30,7 @@ public abstract class Binding { public abstract String getRole(); - public abstract ImmutableList getMembers(); + public abstract List getMembers(); @Nullable public abstract Condition getCondition(); @@ -51,19 +51,22 @@ public abstract static class Builder { public abstract String getRole(); - public abstract ImmutableList getMembers(); + public abstract List getMembers(); public abstract Condition getCondition(); - public abstract ImmutableList.Builder membersBuilder(); - + // Members property must be initialized before this method can be used. public Builder addMembers(String... members) { + ImmutableList.Builder membersBuilder = ImmutableList.builder(); + membersBuilder.addAll(getMembers()); for (String member : members) { - membersBuilder().add(member); + membersBuilder.add(member); } + setMembers(membersBuilder.build()); return this; } + // Members property must be initialized before this method can be used. public Builder removeMembers(String... members) { Predicate selectMembersNotInList = Predicates.not(Predicates.in(Arrays.asList(members))); diff --git a/google-cloud-core/src/main/java/com/google/cloud/Policy.java b/google-cloud-core/src/main/java/com/google/cloud/Policy.java index dd3e86e625..8a4a3fde96 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Policy.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Policy.java @@ -193,9 +193,11 @@ public final Builder setBindings(Map> bindings) { for (Map.Entry> binding : bindings.entrySet()) { Binding.Builder bindingBuilder = Binding.newBuilder(); bindingBuilder.setRole(binding.getKey().getValue()); + ImmutableList.Builder membersBuilder = ImmutableList.builder(); for (Identity identity : binding.getValue()) { - bindingBuilder.addMembers(identity.strValue()); + membersBuilder.add(identity.strValue()); } + bindingBuilder.setMembers(membersBuilder.build()); this.bindingsList.add(bindingBuilder.build()); } return this; @@ -253,19 +255,26 @@ public final Builder addIdentity(Role role, Identity first, Identity... others) for (int i = 0; i < bindingsList.size(); i++) { Binding binding = bindingsList.get(i); if (binding.getRole().equals(role.getValue())) { - Binding.Builder bindingBuilder = binding.toBuilder().addMembers(first.strValue()); + Binding.Builder bindingBuilder = binding.toBuilder(); + ImmutableList.Builder membersBuilder = ImmutableList.builder(); + membersBuilder.addAll(binding.getMembers()); + membersBuilder.add(first.strValue()); for (Identity identity : others) { - bindingBuilder.addMembers(identity.strValue()); + membersBuilder.add(identity.strValue()); } + bindingBuilder.setMembers(membersBuilder.build()); bindingsList.set(i, bindingBuilder.build()); return this; } } + // Binding does not yet exist. Binding.Builder bindingBuilder = Binding.newBuilder().setRole(role.getValue()); - bindingBuilder.addMembers(first.strValue()); + ImmutableList.Builder membersBuilder = ImmutableList.builder(); + membersBuilder.add(first.strValue()); for (Identity identity : others) { - bindingBuilder.addMembers(identity.strValue()); + membersBuilder.add(identity.strValue()); } + bindingBuilder.setMembers(membersBuilder.build()); bindingsList.add(bindingBuilder.build()); return this; } diff --git a/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java b/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java index d700526076..2b2a2bb299 100644 --- a/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java +++ b/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java @@ -143,6 +143,7 @@ public void addMemberFromPolicy() { } Policy updatedPolicy = FULL_POLICY_V3.toBuilder().setBindings(bindings).build(); + System.out.println(updatedPolicy.getBindingsList()); assertEquals(4, updatedPolicy.getBindingsList().get(0).getMembers().size()); } @@ -168,7 +169,7 @@ public void removeBindingFromPolicy() { public void addBindingToPolicy() { assertEquals(2, FULL_POLICY_V3.getBindingsList().size()); List bindings = new ArrayList<>(FULL_POLICY_V3.getBindingsList()); - bindings.add(Binding.newBuilder().setRole(OWNER).addMembers(USER).build()); + bindings.add(Binding.newBuilder().setRole(OWNER).setMembers(ImmutableList.of(USER)).build()); Policy updatedPolicy = FULL_POLICY_V3.toBuilder().setBindings(bindings).build(); assertEquals(3, updatedPolicy.getBindingsList().size()); } @@ -195,16 +196,6 @@ public void testIllegalPolicies() { } catch (NullPointerException ex) { assertEquals("at index 0", ex.getMessage()); } - try { - FULL_POLICY_V3 - .toBuilder() - .setBindings( - Arrays.asList(Binding.newBuilder().setRole("test").addMembers(null, "user").build())) - .build(); - fail("Null member should cause exception."); - } catch (NullPointerException ex) { - assertNull(ex.getMessage()); - } try { FULL_POLICY_V3.getBindings(); fail("getBindings() should cause exception with Policy V3."); From 108faecf34c3583bd1b0f03a653406c9e8307cde Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Mon, 24 Feb 2020 20:33:36 -0800 Subject: [PATCH 22/31] address feedback --- google-cloud-core/src/main/java/com/google/cloud/Binding.java | 2 ++ google-cloud-core/src/main/java/com/google/cloud/Condition.java | 2 ++ .../src/test/java/com/google/cloud/PolicyV3Test.java | 1 - 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/google-cloud-core/src/main/java/com/google/cloud/Binding.java b/google-cloud-core/src/main/java/com/google/cloud/Binding.java index d0427081aa..baf6000e1f 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Binding.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Binding.java @@ -16,6 +16,7 @@ package com.google.cloud; +import com.google.api.core.BetaApi; import com.google.auto.value.AutoValue; import com.google.common.base.Predicate; import com.google.common.base.Predicates; @@ -26,6 +27,7 @@ import java.util.List; import javax.annotation.Nullable; +@BetaApi @AutoValue public abstract class Binding { public abstract String getRole(); diff --git a/google-cloud-core/src/main/java/com/google/cloud/Condition.java b/google-cloud-core/src/main/java/com/google/cloud/Condition.java index 3de5d67271..b1ef80869c 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Condition.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Condition.java @@ -16,9 +16,11 @@ package com.google.cloud; +import com.google.api.core.BetaApi; import com.google.auto.value.AutoValue; import javax.annotation.Nullable; +@BetaApi @AutoValue public abstract class Condition { @Nullable diff --git a/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java b/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java index 2b2a2bb299..2c25ecb023 100644 --- a/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java +++ b/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java @@ -143,7 +143,6 @@ public void addMemberFromPolicy() { } Policy updatedPolicy = FULL_POLICY_V3.toBuilder().setBindings(bindings).build(); - System.out.println(updatedPolicy.getBindingsList()); assertEquals(4, updatedPolicy.getBindingsList().get(0).getMembers().size()); } From 79126b5ec57f482c96bac645301b4e79b46c343a Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Mon, 24 Feb 2020 21:40:27 -0800 Subject: [PATCH 23/31] split up unit test --- .../src/test/java/com/google/cloud/PolicyV3Test.java | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java b/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java index 2c25ecb023..13cab4332c 100644 --- a/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java +++ b/google-cloud-core/src/test/java/com/google/cloud/PolicyV3Test.java @@ -82,7 +82,7 @@ public class PolicyV3Test { .build(); @Test - public void testBuilder() { + public void testBuilderV1() { assertEquals(BINDINGS_NO_CONDITIONS, FULL_POLICY_V1.getBindingsList()); assertEquals(1, FULL_POLICY_V1.getVersion()); assertEquals("etag", FULL_POLICY_V1.getEtag()); @@ -90,19 +90,25 @@ public void testBuilder() { assertEquals(BINDINGS_NO_CONDITIONS, policy.getBindingsList()); assertEquals("etag", policy.getEtag()); assertEquals(1, policy.getVersion()); + } + @Test + public void testBuilderV3WithConditions() { assertEquals(BINDINGS_WITH_CONDITIONS, FULL_POLICY_V3.getBindingsList()); assertEquals(3, FULL_POLICY_V3.getVersion()); assertEquals("etag", FULL_POLICY_V3.getEtag()); - policy = FULL_POLICY_V3.toBuilder().setBindings(BINDINGS_WITH_CONDITIONS).build(); + Policy policy = FULL_POLICY_V3.toBuilder().setBindings(BINDINGS_WITH_CONDITIONS).build(); assertEquals(BINDINGS_WITH_CONDITIONS, policy.getBindingsList()); assertEquals("etag", policy.getEtag()); assertEquals(3, policy.getVersion()); + } + @Test + public void testBuilderV1ToV3Compatability() { assertEquals(BINDINGS_WITH_CONDITIONS, FULL_POLICY_V3_WITH_VERSION_1.getBindingsList()); assertEquals(1, FULL_POLICY_V3_WITH_VERSION_1.getVersion()); assertEquals("etag", FULL_POLICY_V3_WITH_VERSION_1.getEtag()); - policy = + Policy policy = FULL_POLICY_V3_WITH_VERSION_1 .toBuilder() .setBindings(BINDINGS_WITH_CONDITIONS) From 505f9bc9937311ca7aa72cdbfa65ab376548aa95 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Tue, 25 Feb 2020 09:18:21 -0800 Subject: [PATCH 24/31] use guava beta annotation --- google-cloud-core/src/main/java/com/google/cloud/Binding.java | 4 ++-- .../src/main/java/com/google/cloud/Condition.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/google-cloud-core/src/main/java/com/google/cloud/Binding.java b/google-cloud-core/src/main/java/com/google/cloud/Binding.java index baf6000e1f..5f27e70fb7 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Binding.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Binding.java @@ -16,8 +16,8 @@ package com.google.cloud; -import com.google.api.core.BetaApi; import com.google.auto.value.AutoValue; +import com.google.common.annotations.Beta; import com.google.common.base.Predicate; import com.google.common.base.Predicates; import com.google.common.collect.Collections2; @@ -27,7 +27,7 @@ import java.util.List; import javax.annotation.Nullable; -@BetaApi +@Beta @AutoValue public abstract class Binding { public abstract String getRole(); diff --git a/google-cloud-core/src/main/java/com/google/cloud/Condition.java b/google-cloud-core/src/main/java/com/google/cloud/Condition.java index b1ef80869c..6a1f6c9437 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Condition.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Condition.java @@ -16,11 +16,11 @@ package com.google.cloud; -import com.google.api.core.BetaApi; import com.google.auto.value.AutoValue; +import com.google.common.annotations.Beta; import javax.annotation.Nullable; -@BetaApi +@Beta @AutoValue public abstract class Condition { @Nullable From a89ef0c98006f2c4c58cad2b857e7a05c208459e Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Tue, 25 Feb 2020 15:15:22 -0800 Subject: [PATCH 25/31] surface ImmutableList<> for Binding class. --- google-cloud-core/src/main/java/com/google/cloud/Binding.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/google-cloud-core/src/main/java/com/google/cloud/Binding.java b/google-cloud-core/src/main/java/com/google/cloud/Binding.java index 5f27e70fb7..1c2779be52 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Binding.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Binding.java @@ -32,7 +32,7 @@ public abstract class Binding { public abstract String getRole(); - public abstract List getMembers(); + public abstract ImmutableList getMembers(); @Nullable public abstract Condition getCondition(); @@ -53,7 +53,7 @@ public abstract static class Builder { public abstract String getRole(); - public abstract List getMembers(); + public abstract ImmutableList getMembers(); public abstract Condition getCondition(); From f0b508534483ae391f83a15b6db1b77bb5533a28 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Tue, 25 Feb 2020 15:22:47 -0800 Subject: [PATCH 26/31] use BetaApi from api.core --- google-cloud-core/src/main/java/com/google/cloud/Binding.java | 4 ++-- .../src/main/java/com/google/cloud/Condition.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/google-cloud-core/src/main/java/com/google/cloud/Binding.java b/google-cloud-core/src/main/java/com/google/cloud/Binding.java index 1c2779be52..fbe91a3932 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Binding.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Binding.java @@ -16,8 +16,8 @@ package com.google.cloud; +import com.google.api.core.BetaApi; import com.google.auto.value.AutoValue; -import com.google.common.annotations.Beta; import com.google.common.base.Predicate; import com.google.common.base.Predicates; import com.google.common.collect.Collections2; @@ -27,7 +27,7 @@ import java.util.List; import javax.annotation.Nullable; -@Beta +@BetaApi("This is a Beta API is not stable yet and may change in the future.") @AutoValue public abstract class Binding { public abstract String getRole(); diff --git a/google-cloud-core/src/main/java/com/google/cloud/Condition.java b/google-cloud-core/src/main/java/com/google/cloud/Condition.java index 6a1f6c9437..f1fb50c30d 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Condition.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Condition.java @@ -16,11 +16,11 @@ package com.google.cloud; +import com.google.api.core.BetaApi; import com.google.auto.value.AutoValue; -import com.google.common.annotations.Beta; import javax.annotation.Nullable; -@Beta +@BetaApi("This is a Beta API is not stable yet and may change in the future.") @AutoValue public abstract class Condition { @Nullable From 9f5e6004f441b1e5bd4e189d9759d64154fec35b Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Wed, 26 Feb 2020 12:12:31 -0800 Subject: [PATCH 27/31] return as expected --- google-cloud-core/src/main/java/com/google/cloud/Policy.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/google-cloud-core/src/main/java/com/google/cloud/Policy.java b/google-cloud-core/src/main/java/com/google/cloud/Policy.java index 8a4a3fde96..c6c1218c83 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Policy.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Policy.java @@ -432,7 +432,9 @@ public boolean equals(Object obj) { return false; } for (int i = 0; i < bindingsList.size(); i++) { - bindingsList.get(i).equals(other.getBindingsList().get(i)); + if (!bindingsList.get(i).equals(other.getBindingsList().get(i))) { + return false; + } } return Objects.equals(etag, other.getEtag()) && Objects.equals(version, other.getVersion()); } From 8580f5a8e545b6467b02b213a2584823bcebd353 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Wed, 26 Feb 2020 12:44:36 -0800 Subject: [PATCH 28/31] partial addressing of feedback --- .../main/java/com/google/cloud/Binding.java | 23 ++++++------ .../main/java/com/google/cloud/Condition.java | 4 --- .../main/java/com/google/cloud/Policy.java | 35 +++++++++---------- 3 files changed, 27 insertions(+), 35 deletions(-) diff --git a/google-cloud-core/src/main/java/com/google/cloud/Binding.java b/google-cloud-core/src/main/java/com/google/cloud/Binding.java index fbe91a3932..5da55c3471 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Binding.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Binding.java @@ -16,12 +16,15 @@ package com.google.cloud; +import static com.google.common.base.Predicates.in; +import static com.google.common.base.Predicates.not; + import com.google.api.core.BetaApi; import com.google.auto.value.AutoValue; import com.google.common.base.Predicate; -import com.google.common.base.Predicates; import com.google.common.collect.Collections2; import com.google.common.collect.ImmutableList; +import com.google.common.collect.Lists; import java.util.Arrays; import java.util.Collection; import java.util.List; @@ -40,7 +43,8 @@ public abstract class Binding { public abstract Builder toBuilder(); public static Builder newBuilder() { - return new AutoValue_Binding.Builder(); + List emptyMembers = ImmutableList.of(); + return new AutoValue_Binding.Builder().setMembers(emptyMembers); } @AutoValue.Builder @@ -51,27 +55,20 @@ public abstract static class Builder { public abstract Builder setCondition(Condition condition); - public abstract String getRole(); - - public abstract ImmutableList getMembers(); - - public abstract Condition getCondition(); + abstract ImmutableList getMembers(); // Members property must be initialized before this method can be used. - public Builder addMembers(String... members) { + public Builder addMembers(String member, String... moreMembers) { ImmutableList.Builder membersBuilder = ImmutableList.builder(); membersBuilder.addAll(getMembers()); - for (String member : members) { - membersBuilder.add(member); - } + membersBuilder.addAll(Lists.asList(member, moreMembers)); setMembers(membersBuilder.build()); return this; } // Members property must be initialized before this method can be used. public Builder removeMembers(String... members) { - Predicate selectMembersNotInList = - Predicates.not(Predicates.in(Arrays.asList(members))); + Predicate selectMembersNotInList = not(in(Arrays.asList(members))); Collection filter = Collections2.filter(getMembers(), selectMembersNotInList); setMembers(ImmutableList.copyOf(filter)); return this; diff --git a/google-cloud-core/src/main/java/com/google/cloud/Condition.java b/google-cloud-core/src/main/java/com/google/cloud/Condition.java index f1fb50c30d..3338b4638e 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Condition.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Condition.java @@ -18,18 +18,14 @@ import com.google.api.core.BetaApi; import com.google.auto.value.AutoValue; -import javax.annotation.Nullable; @BetaApi("This is a Beta API is not stable yet and may change in the future.") @AutoValue public abstract class Condition { - @Nullable public abstract String getTitle(); - @Nullable public abstract String getDescription(); - @Nullable public abstract String getExpression(); public abstract Builder toBuilder(); diff --git a/google-cloud-core/src/main/java/com/google/cloud/Policy.java b/google-cloud-core/src/main/java/com/google/cloud/Policy.java index c6c1218c83..834539dff2 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Policy.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Policy.java @@ -48,7 +48,7 @@ public final class Policy implements Serializable { private static final long serialVersionUID = -3348914530232544290L; - private final List bindingsList; + private final ImmutableList bindingsList; private final String etag; private final int version; @@ -102,7 +102,7 @@ protected Policy fromPb(com.google.iam.v1.Policy policyPb) { Binding.Builder convertedBinding = Binding.newBuilder() .setRole(bindingPb.getRole()) - .setMembers(ImmutableList.copyOf(bindingPb.getMembersList())); + .setMembers(bindingPb.getMembersList()); if (bindingPb.hasCondition()) { Expr expr = bindingPb.getCondition(); convertedBinding.setCondition( @@ -131,7 +131,7 @@ protected com.google.iam.v1.Policy toPb(Policy policy) { for (Binding binding : policy.getBindingsList()) { com.google.iam.v1.Binding.Builder bindingBuilder = com.google.iam.v1.Binding.newBuilder(); bindingBuilder.setRole(binding.getRole()); - bindingBuilder.addAllMembers(ImmutableList.copyOf(binding.getMembers())); + bindingBuilder.addAllMembers(binding.getMembers()); if (binding.getCondition() != null) { Condition condition = binding.getCondition(); bindingBuilder.setCondition( @@ -163,9 +163,7 @@ protected Builder() {} @InternalApi("This class should only be extended within google-cloud-java") protected Builder(Policy policy) { - for (Binding binding : policy.bindingsList) { - bindingsList.add(binding.toBuilder().build()); - } + bindingsList.addAll(policy.bindingsList); setEtag(policy.etag); setVersion(policy.version); } @@ -175,7 +173,8 @@ protected Builder(Policy policy) { * * @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 or if policy - * version is equal to 3 or has conditional bindings. + * version is equal to 3 or has conditional bindings because conditional policies are not + * supported */ public final Builder setBindings(Map> bindings) { checkNotNull(bindings, "The provided map of bindings cannot be null."); @@ -207,7 +206,7 @@ public final Builder setBindings(Map> bindings) { * Replaces the builder's List of bindings with the given List of Bindings. * * @throws NullPointerException if the given list is null, role is null, or contains any null - * members in bindings. + * members in bindings */ public final Builder setBindings(List bindings) { this.bindingsList.clear(); @@ -221,7 +220,12 @@ public final Builder setBindings(List bindings) { return this; } - /** Removes the role (and all identities associated with that role) from the policy. */ + /** + * Removes the role (and all identities associated with that role) from the policy. + * + * @throws IllegalArgumentException if policy version is equal to 3 or has conditional bindings + * because conditional policies are not supported + */ public final Builder removeRole(Role role) { checkArgument( !isConditional(this.version, this.bindingsList), @@ -283,7 +287,7 @@ public final Builder addIdentity(Role role, Identity first, Identity... others) * Removes one or more identities from an existing binding. Does nothing if the binding * associated with the provided role doesn't exist. * - * @throws IllegalArgumentException if policy version is equal to 3 or has conditional bindings. + * @throws IllegalArgumentException if policy version is equal to 3 or has conditional bindings */ public final Builder removeIdentity(Role role, Identity first, Identity... others) { checkArgument( @@ -360,7 +364,7 @@ public Builder toBuilder() { /** * Returns the map of bindings that comprises the policy. * - * @throws IllegalArgumentException if policy version is equal to 3 or has conditional bindings. + * @throws IllegalArgumentException if policy version is equal to 3 or has conditional bindings */ public Map> getBindings() { checkArgument( @@ -378,7 +382,7 @@ public Map> getBindings() { } /** Returns the list of bindings that comprises the policy for version 3. */ - public List getBindingsList() { + public ImmutableList getBindingsList() { return bindingsList; } @@ -428,14 +432,9 @@ public boolean equals(Object obj) { return false; } Policy other = (Policy) obj; - if (bindingsList.size() != other.getBindingsList().size()) { + if (!bindingsList.equals(other.getBindingsList())) { return false; } - for (int i = 0; i < bindingsList.size(); i++) { - if (!bindingsList.get(i).equals(other.getBindingsList().get(i))) { - return false; - } - } return Objects.equals(etag, other.getEtag()) && Objects.equals(version, other.getVersion()); } From 9fe4358d9c361e9bab8cfc3ed690e5fb82a0c2ad Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Wed, 26 Feb 2020 12:47:37 -0800 Subject: [PATCH 29/31] address feedback pt2 --- google-cloud-core/src/main/java/com/google/cloud/Binding.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/google-cloud-core/src/main/java/com/google/cloud/Binding.java b/google-cloud-core/src/main/java/com/google/cloud/Binding.java index 5da55c3471..10bf068a63 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Binding.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Binding.java @@ -51,7 +51,7 @@ public static Builder newBuilder() { public abstract static class Builder { public abstract Builder setRole(String role); - public abstract Builder setMembers(List members); + public abstract Builder setMembers(Iterable members); public abstract Builder setCondition(Condition condition); @@ -70,7 +70,7 @@ public Builder addMembers(String member, String... moreMembers) { public Builder removeMembers(String... members) { Predicate selectMembersNotInList = not(in(Arrays.asList(members))); Collection filter = Collections2.filter(getMembers(), selectMembersNotInList); - setMembers(ImmutableList.copyOf(filter)); + setMembers(filter); return this; } From 8f48a15ad20c8170539295493ee2a2be06f5bb22 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Wed, 26 Feb 2020 13:00:31 -0800 Subject: [PATCH 30/31] address remaining feedback --- .../main/java/com/google/cloud/Binding.java | 37 ++++++++++++++++++- .../main/java/com/google/cloud/Condition.java | 18 +++++++++ .../main/java/com/google/cloud/Policy.java | 2 +- 3 files changed, 54 insertions(+), 3 deletions(-) diff --git a/google-cloud-core/src/main/java/com/google/cloud/Binding.java b/google-cloud-core/src/main/java/com/google/cloud/Binding.java index 10bf068a63..be16f87421 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Binding.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Binding.java @@ -30,18 +30,31 @@ import java.util.List; import javax.annotation.Nullable; +/** + * Class for Identity and Access Management (IAM) policies. IAM policies are used to specify access + * settings for Cloud Platform resources. A policy is a list of bindings. A binding assigns a set of + * identities to a role, where the identities can be user accounts, Google groups, Google domains, + * and service accounts. A role is a named list of permissions defined by IAM. + * + * @see Policy + */ @BetaApi("This is a Beta API is not stable yet and may change in the future.") @AutoValue public abstract class Binding { + /** Get IAM Policy Binding Role */ public abstract String getRole(); + /** Get IAM Policy Binding Members */ public abstract ImmutableList getMembers(); + /** Get IAM Policy Binding Condition */ @Nullable public abstract Condition getCondition(); + /** Create a Binding.Builder from an existing Binding */ public abstract Builder toBuilder(); + /** Create a new Binding.Builder */ public static Builder newBuilder() { List emptyMembers = ImmutableList.of(); return new AutoValue_Binding.Builder().setMembers(emptyMembers); @@ -49,15 +62,31 @@ public static Builder newBuilder() { @AutoValue.Builder public abstract static class Builder { + /** + * Set IAM Role for Policy Binding + * + * @throws NullPointerException if the role is null. + */ public abstract Builder setRole(String role); + /** + * Set IAM Members for Policy Binding + * + * @throws NullPointerException if a member is null. + */ public abstract Builder setMembers(Iterable members); + /** Set IAM Condition for Policy Binding */ public abstract Builder setCondition(Condition condition); + /** Internal use to getMembers() in addMembers() and removeMembers() */ abstract ImmutableList getMembers(); - // Members property must be initialized before this method can be used. + /** + * Add members to Policy Binding. + * + * @throws NullPointerException if a member is null. + */ public Builder addMembers(String member, String... moreMembers) { ImmutableList.Builder membersBuilder = ImmutableList.builder(); membersBuilder.addAll(getMembers()); @@ -66,7 +95,11 @@ public Builder addMembers(String member, String... moreMembers) { return this; } - // Members property must be initialized before this method can be used. + /** + * Remove members to Policy Binding. + * + * @throws NullPointerException if a member is null. + */ public Builder removeMembers(String... members) { Predicate selectMembersNotInList = not(in(Arrays.asList(members))); Collection filter = Collections2.filter(getMembers(), selectMembersNotInList); diff --git a/google-cloud-core/src/main/java/com/google/cloud/Condition.java b/google-cloud-core/src/main/java/com/google/cloud/Condition.java index 3338b4638e..3854883796 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Condition.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Condition.java @@ -19,29 +19,47 @@ import com.google.api.core.BetaApi; import com.google.auto.value.AutoValue; +/** + * Class for Identity and Access Management (IAM) policies. IAM policies are used to specify access + * settings for Cloud Platform resources. A policy is a list of bindings. A binding assigns a set of + * identities to a role, where the identities can be user accounts, Google groups, Google domains, + * and service accounts. A role is a named list of permissions defined by IAM. + * + * @see Policy + * @see IAM Conditions + */ @BetaApi("This is a Beta API is not stable yet and may change in the future.") @AutoValue public abstract class Condition { + /** Get IAM Policy Binding Condition Title */ public abstract String getTitle(); + /** Get IAM Policy Binding Condition Description */ public abstract String getDescription(); + /** Get IAM Policy Binding Condition Expression */ public abstract String getExpression(); + /** Create a new Condition.Builder from an existing Condition */ public abstract Builder toBuilder(); + /** Create a new Condition.Builder */ public static Builder newBuilder() { return new AutoValue_Condition.Builder(); } @AutoValue.Builder public abstract static class Builder { + /** Set IAM Policy Binding Condition Title */ public abstract Builder setTitle(String title); + /** Set IAM Policy Binding Condition Description */ public abstract Builder setDescription(String description); + /** Set IAM Policy Binding Condition Expression */ public abstract Builder setExpression(String expression); + /** Build Builder which creates a Condition instance */ public abstract Condition build(); } } diff --git a/google-cloud-core/src/main/java/com/google/cloud/Policy.java b/google-cloud-core/src/main/java/com/google/cloud/Policy.java index 834539dff2..eaf5f67aaf 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Policy.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Policy.java @@ -43,7 +43,7 @@ * identities to a role, where the identities can be user accounts, Google groups, Google domains, * and service accounts. A role is a named list of permissions defined by IAM. * - * @see Policy + * @see Policy */ public final class Policy implements Serializable { From 2b566410f4bfe55e083818c21aaf46c21d9c74ac Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Wed, 26 Feb 2020 13:08:42 -0800 Subject: [PATCH 31/31] address one last feedback --- google-cloud-core/src/main/java/com/google/cloud/Policy.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google-cloud-core/src/main/java/com/google/cloud/Policy.java b/google-cloud-core/src/main/java/com/google/cloud/Policy.java index eaf5f67aaf..a6ecbae11e 100644 --- a/google-cloud-core/src/main/java/com/google/cloud/Policy.java +++ b/google-cloud-core/src/main/java/com/google/cloud/Policy.java @@ -435,7 +435,7 @@ public boolean equals(Object obj) { if (!bindingsList.equals(other.getBindingsList())) { return false; } - return Objects.equals(etag, other.getEtag()) && Objects.equals(version, other.getVersion()); + return Objects.equals(etag, other.getEtag()) && version == other.getVersion(); } /** Returns a builder for {@code Policy} objects. */