From f45d9131d5d2bbb2cc4cec67ebe82054fda58f55 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Tue, 10 Mar 2020 11:46:10 -0700 Subject: [PATCH] fix: verify correctness of map -> list equality (#174) * fix: verify correctness of map -> list equality * address comments * format --- .../main/java/com/google/cloud/Policy.java | 16 ++++++++-- .../java/com/google/cloud/PolicyTest.java | 30 +++++++++++++++++++ 2 files changed, 43 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 a6ecbae11e..1c5b78c9ea 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 @@ -260,7 +260,7 @@ public final Builder addIdentity(Role role, Identity first, Identity... others) Binding binding = bindingsList.get(i); if (binding.getRole().equals(role.getValue())) { Binding.Builder bindingBuilder = binding.toBuilder(); - ImmutableList.Builder membersBuilder = ImmutableList.builder(); + ImmutableSet.Builder membersBuilder = ImmutableSet.builder(); membersBuilder.addAll(binding.getMembers()); membersBuilder.add(first.strValue()); for (Identity identity : others) { @@ -273,7 +273,7 @@ public final Builder addIdentity(Role role, Identity first, Identity... others) } // Binding does not yet exist. Binding.Builder bindingBuilder = Binding.newBuilder().setRole(role.getValue()); - ImmutableList.Builder membersBuilder = ImmutableList.builder(); + ImmutableSet.Builder membersBuilder = ImmutableSet.builder(); membersBuilder.add(first.strValue()); for (Identity identity : others) { membersBuilder.add(identity.strValue()); @@ -432,9 +432,19 @@ public boolean equals(Object obj) { return false; } Policy other = (Policy) obj; - if (!bindingsList.equals(other.getBindingsList())) { + if (bindingsList == null && other.getBindingsList() == null) { + return true; + } + if ((bindingsList == null && other.getBindingsList() != null) + || bindingsList != null && other.getBindingsList() == null + || bindingsList.size() != other.getBindingsList().size()) { return false; } + for (Binding binding : bindingsList) { + if (!other.getBindingsList().contains(binding)) { + return false; + } + } return Objects.equals(etag, other.getEtag()) && 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 eb03e33726..98695c2a4c 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 @@ -104,6 +104,36 @@ public void testBuilder() { assertEquals(0, policy.getVersion()); } + @Test + public void testPolicyOrderShouldNotMatter() { + Role role1 = Role.of("role1"); + Identity identity1 = Identity.user("user1@example.com"); + Role role2 = Role.of("role2"); + Identity identity2 = Identity.user("user2@example.com"); + Policy policy1 = + Policy.newBuilder().addIdentity(role1, identity1).addIdentity(role2, identity2).build(); + Policy policy2 = + Policy.newBuilder().addIdentity(role2, identity2).addIdentity(role1, identity1).build(); + assertEquals(policy1, policy2); + } + + @Test + public void testPolicyMultipleAddIdentitiesShouldNotMatter() { + Role role1 = Role.of("role1"); + Identity identity1 = Identity.user("user1@example.com"); + Role role2 = Role.of("role2"); + Identity identity2 = Identity.user("user2@example.com"); + Policy policy1 = + Policy.newBuilder() + .addIdentity(role1, identity1) + .addIdentity(role2, identity2) + .addIdentity(role2, identity2) + .build(); + Policy policy2 = + Policy.newBuilder().addIdentity(role2, identity2).addIdentity(role1, identity1).build(); + assertEquals(policy1, policy2); + } + @Test public void testIllegalPolicies() { try {