Skip to content
Permalink
Browse files
fix: verify correctness of map -> list equality (#174)
* fix: verify correctness of map -> list equality

* address comments

* format
  • Loading branch information
frankyn committed Mar 10, 2020
1 parent d28f885 commit f45d9131d5d2bbb2cc4cec67ebe82054fda58f55
Showing with 43 additions and 3 deletions.
  1. +13 −3 google-cloud-core/src/main/java/com/google/cloud/Policy.java
  2. +30 −0 google-cloud-core/src/test/java/com/google/cloud/PolicyTest.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<String> membersBuilder = ImmutableList.builder();
ImmutableSet.Builder<String> 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<String> membersBuilder = ImmutableList.builder();
ImmutableSet.Builder<String> 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();
}

@@ -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 {

0 comments on commit f45d913

Please sign in to comment.