Implement equals() and hashCode() on PermissionBackendCondition

We want to be able to deduplicate PermissionBackendConditions when given
a list of such objects. Therefore, this commit implements equals() and
hashCode().

This is not trivial since the individual implementations of
PermissionBackendCondition encapsulate a CurrentUser as well as a
PermissionBackend. Both of which do not implement equals() and
hashCode().

This is correct, since PermissionBackend is a service and comparing it
to other PermissionBackends makes no sense. CurrentUser could be
compared to other CurrentUser objects but the outcome depends on
wheather two annonymous users should be considered equal or not. This
depends on the use case.

Therefore, this commit adds user() and resourcePath() to
PermissionBackend to get the entities that a PermissionBackend object is
scoped to and evaluate the case at hand directly in
PermissionBackendCondition.

resourcePath() can come in handy in other places as well since it is an
accurate representation of the resource that we are performing checks
on. Concrete PermissionBackend implementations can use it to communicate
with their remove service if desired.

The implementation for resourcePath() picks /+ as the delimiter since it
is a forbidden character combination for project names. An alternative
would be to URL-encode the project name, but this is more expensive.

Change-Id: I7b357e61bfc6f14acc7b0d06830b615847798ec3

java/com/google/gerrit/server/permissions/ChangeControl.java

@@ -318,6 +318,13 @@ class ChangeControl {
       return user().equals(user) ? this : forUser(user).asForChange(cd, db);
     }
 
+    @Override
+    public String resourcePath() {
+      return String.format(
+          "/projects/%s/+changes/%s",
+          getProjectControl().getProjectState().getName(), changeData().getId().get());
+    }
+
     @Override
     public void check(ChangePermissionOrLabel perm)
         throws AuthException, PermissionBackendException {

java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java

@@ -70,6 +70,11 @@ public class DefaultPermissionBackend extends PermissionBackend {
       this.user = checkNotNull(user, "user");
     }
 
+    @Override
+    public CurrentUser user() {
+      return user;
+    }
+
     @Override
     public ForProject project(Project.NameKey project) {
       try {

java/com/google/gerrit/server/permissions/FailedPermissionBackend.java

@@ -72,11 +72,22 @@ public class FailedPermissionBackend {
       return this;
     }
 
+    @Override
+    public CurrentUser user() {
+      throw new UnsupportedOperationException("FailedPermissionBackend is not scoped to user");
+    }
+
     @Override
     public ForProject user(CurrentUser user) {
       return this;
     }
 
+    @Override
+    public String resourcePath() {
+      throw new UnsupportedOperationException(
+          "FailedPermissionBackend is not scoped to a resource");
+    }
+
     @Override
     public ForRef ref(String ref) {
       return new FailedRef(message, cause);
@@ -108,11 +119,22 @@ public class FailedPermissionBackend {
       return this;
     }
 
+    @Override
+    public CurrentUser user() {
+      throw new UnsupportedOperationException("FailedPermissionBackend is not scoped to user");
+    }
+
     @Override
     public ForRef user(CurrentUser user) {
       return this;
     }
 
+    @Override
+    public String resourcePath() {
+      throw new UnsupportedOperationException(
+          "FailedPermissionBackend is not scoped to a resource");
+    }
+
     @Override
     public ForChange change(ChangeData cd) {
       return new FailedChange(message, cause);
@@ -159,6 +181,12 @@ public class FailedPermissionBackend {
       return this;
     }
 
+    @Override
+    public String resourcePath() {
+      throw new UnsupportedOperationException(
+          "FailedPermissionBackend is not scoped to a resource");
+    }
+
     @Override
     public void check(ChangePermissionOrLabel perm) throws PermissionBackendException {
       throw new PermissionBackendException(message, cause);

java/com/google/gerrit/server/permissions/PermissionBackend.java

@@ -135,6 +135,9 @@ public abstract class PermissionBackend {
 
   /** PermissionBackend scoped to a specific user. */
   public abstract static class WithUser extends AcceptsReviewDb<WithUser> {
+    /** @return user this instance is scoped to. */
+    public abstract CurrentUser user();
+
     /** @return instance scoped for the specified project. */
     public abstract ForProject project(Project.NameKey project);
 
@@ -247,6 +250,12 @@ public abstract class PermissionBackend {
 
   /** PermissionBackend scoped to a user and project. */
   public abstract static class ForProject extends AcceptsReviewDb<ForProject> {
+    /** @return user this instance is scoped to. */
+    public abstract CurrentUser user();
+
+    /** @return fully qualified resource path that this instance is scoped to. */
+    public abstract String resourcePath();
+
     /** @return new instance rescoped to same project, but different {@code user}. */
     public abstract ForProject user(CurrentUser user);
 
@@ -304,6 +313,12 @@ public abstract class PermissionBackend {
 
   /** PermissionBackend scoped to a user, project and reference. */
   public abstract static class ForRef extends AcceptsReviewDb<ForRef> {
+    /** @return user this instance is scoped to. */
+    public abstract CurrentUser user();
+
+    /** @return fully qualified resource path that this instance is scoped to. */
+    public abstract String resourcePath();
+
     /** @return new instance rescoped to same reference, but different {@code user}. */
     public abstract ForRef user(CurrentUser user);
 
@@ -359,6 +374,9 @@ public abstract class PermissionBackend {
     /** @return user this instance is scoped to. */
     public abstract CurrentUser user();
 
+    /** @return fully qualified resource path that this instance is scoped to. */
+    public abstract String resourcePath();
+
     /** @return new instance rescoped to same change, but different {@code user}. */
     public abstract ForChange user(CurrentUser user);
 

java/com/google/gerrit/server/permissions/PermissionBackendCondition.java

@@ -17,6 +17,8 @@ package com.google.gerrit.server.permissions;
 import com.google.gerrit.extensions.api.access.GlobalOrPluginPermission;
 import com.google.gerrit.extensions.conditions.BooleanCondition;
 import com.google.gerrit.extensions.conditions.PrivateInternals_BooleanCondition;
+import com.google.gerrit.server.CurrentUser;
+import java.util.Objects;
 
 /** {@link BooleanCondition} to evaluate a permission. */
 public abstract class PermissionBackendCondition
@@ -64,6 +66,20 @@ public abstract class PermissionBackendCondition
     public String toString() {
       return "PermissionBackendCondition.WithUser(" + perm + ")";
     }
+
+    @Override
+    public int hashCode() {
+      return Objects.hash(perm, hashForUser(impl.user()));
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+      if (!(obj instanceof WithUser)) {
+        return false;
+      }
+      WithUser other = (WithUser) obj;
+      return Objects.equals(perm, other.perm) && usersAreEqual(impl.user(), other.impl.user());
+    }
   }
 
   public static class ForProject extends PermissionBackendCondition {
@@ -92,6 +108,22 @@ public abstract class PermissionBackendCondition
     public String toString() {
       return "PermissionBackendCondition.ForProject(" + perm + ")";
     }
+
+    @Override
+    public int hashCode() {
+      return Objects.hash(perm, impl.resourcePath(), hashForUser(impl.user()));
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+      if (!(obj instanceof ForProject)) {
+        return false;
+      }
+      ForProject other = (ForProject) obj;
+      return Objects.equals(perm, other.perm)
+          && Objects.equals(impl.resourcePath(), other.impl.resourcePath())
+          && usersAreEqual(impl.user(), other.impl.user());
+    }
   }
 
   public static class ForRef extends PermissionBackendCondition {
@@ -120,6 +152,22 @@ public abstract class PermissionBackendCondition
     public String toString() {
       return "PermissionBackendCondition.ForRef(" + perm + ")";
     }
+
+    @Override
+    public int hashCode() {
+      return Objects.hash(perm, impl.resourcePath(), hashForUser(impl.user()));
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+      if (!(obj instanceof ForRef)) {
+        return false;
+      }
+      ForRef other = (ForRef) obj;
+      return Objects.equals(perm, other.perm)
+          && Objects.equals(impl.resourcePath(), other.impl.resourcePath())
+          && usersAreEqual(impl.user(), other.impl.user());
+    }
   }
 
   public static class ForChange extends PermissionBackendCondition {
@@ -148,5 +196,35 @@ public abstract class PermissionBackendCondition
     public String toString() {
       return "PermissionBackendCondition.ForChange(" + perm + ")";
     }
+
+    @Override
+    public int hashCode() {
+      return Objects.hash(perm, impl.resourcePath(), hashForUser(impl.user()));
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+      if (!(obj instanceof ForChange)) {
+        return false;
+      }
+      ForChange other = (ForChange) obj;
+      return Objects.equals(perm, other.perm)
+          && Objects.equals(impl.resourcePath(), other.impl.resourcePath())
+          && usersAreEqual(impl.user(), other.impl.user());
+    }
+  }
+
+  private static int hashForUser(CurrentUser user) {
+    if (!user.isIdentifiedUser()) {
+      return 0;
+    }
+    return user.getAccountId().get();
+  }
+
+  private static boolean usersAreEqual(CurrentUser user1, CurrentUser user2) {
+    if (user1.isIdentifiedUser() && user2.isIdentifiedUser()) {
+      return user1.getAccountId().equals(user2.getAccountId());
+    }
+    return false;
   }
 }

java/com/google/gerrit/server/permissions/ProjectControl.java

@@ -312,11 +312,21 @@ class ProjectControl {
   }
 
   private class ForProjectImpl extends ForProject {
+    @Override
+    public CurrentUser user() {
+      return getUser();
+    }
+
     @Override
     public ForProject user(CurrentUser user) {
       return forUser(user).asForProject().database(db);
     }
 
+    @Override
+    public String resourcePath() {
+      return "/projects/" + getProjectState().getName();
+    }
+
     @Override
     public ForRef ref(String ref) {
       return controlForRef(ref).asForRef().database(db);

java/com/google/gerrit/server/permissions/RefControl.java

@@ -431,11 +431,22 @@ class RefControl {
   }
 
   private class ForRefImpl extends ForRef {
+    @Override
+    public CurrentUser user() {
+      return getUser();
+    }
+
     @Override
     public ForRef user(CurrentUser user) {
       return forUser(user).asForRef().database(db);
     }
 
+    @Override
+    public String resourcePath() {
+      return String.format(
+          "/projects/%s/+refs/%s", getProjectControl().getProjectState().getName(), refName);
+    }
+
     @Override
     public ForChange change(ChangeData cd) {
       try {

javatests/com/google/gerrit/acceptance/server/permissions/PermissionBackendConditionIT.java

@@ -0,0 +1,170 @@
+// Copyright (C) 2018 The Android Open Source Project
+//
+// 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.gerrit.acceptance.server.permissions;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.extensions.conditions.BooleanCondition;
+import com.google.gerrit.reviewdb.client.Branch;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.permissions.ChangePermission;
+import com.google.gerrit.server.permissions.GlobalPermission;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.ProjectPermission;
+import com.google.gerrit.server.permissions.RefPermission;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.inject.Inject;
+import org.junit.Test;
+
+public class PermissionBackendConditionIT extends AbstractDaemonTest {
+
+  @Inject PermissionBackend pb;
+
+  @Test
+  public void globalPermissions_sameUserAndPermissionEquals() throws Exception {
+    BooleanCondition cond1 = pb.user(user()).testCond(GlobalPermission.CREATE_GROUP);
+    BooleanCondition cond2 = pb.user(user()).testCond(GlobalPermission.CREATE_GROUP);
+    assertEquals(cond1, cond2);
+    assertEquals(cond1.hashCode(), cond2.hashCode());
+  }
+
+  @Test
+  public void globalPermissions_differentPermissionDoesNotEquals() throws Exception {
+    BooleanCondition cond1 = pb.user(user()).testCond(GlobalPermission.CREATE_GROUP);
+    BooleanCondition cond2 = pb.user(user()).testCond(GlobalPermission.ACCESS_DATABASE);
+    assertNotEquals(cond1, cond2);
+    assertNotEquals(cond1.hashCode(), cond2.hashCode());
+  }
+
+  @Test
+  public void globalPermissions_differentUserDoesNotEqual() throws Exception {
+    BooleanCondition cond1 = pb.user(user()).testCond(GlobalPermission.CREATE_GROUP);
+    BooleanCondition cond2 = pb.user(admin()).testCond(GlobalPermission.CREATE_GROUP);
+    assertNotEquals(cond1, cond2);
+    assertNotEquals(cond1.hashCode(), cond2.hashCode());
+  }
+
+  @Test
+  public void changePermissions_sameResourceAndUserEquals() throws Exception {
+    ChangeData change = createChange().getChange();
+    BooleanCondition cond1 = pb.user(user()).change(change).testCond(ChangePermission.READ);
+    BooleanCondition cond2 = pb.user(user()).change(change).testCond(ChangePermission.READ);
+
+    assertEquals(cond1, cond2);
+    assertEquals(cond1.hashCode(), cond2.hashCode());
+  }
+
+  @Test
+  public void changePermissions_sameResourceDifferentUserDoesNotEqual() throws Exception {
+    ChangeData change = createChange().getChange();
+    BooleanCondition cond1 = pb.user(user()).change(change).testCond(ChangePermission.READ);
+    BooleanCondition cond2 = pb.user(admin()).change(change).testCond(ChangePermission.READ);
+
+    assertNotEquals(cond1, cond2);
+    assertNotEquals(cond1.hashCode(), cond2.hashCode());
+  }
+
+  @Test
+  public void changePermissions_differentResourceSameUserDoesNotEqual() throws Exception {
+    ChangeData change1 = createChange().getChange();
+    ChangeData change2 = createChange().getChange();
+    BooleanCondition cond1 = pb.user(user()).change(change1).testCond(ChangePermission.READ);
+    BooleanCondition cond2 = pb.user(user()).change(change2).testCond(ChangePermission.READ);
+
+    assertNotEquals(cond1, cond2);
+    assertNotEquals(cond1.hashCode(), cond2.hashCode());
+  }
+
+  @Test
+  public void projectPermissions_sameResourceAndUserEquals() throws Exception {
+    BooleanCondition cond1 = pb.user(user()).project(project).testCond(ProjectPermission.READ);
+    BooleanCondition cond2 = pb.user(user()).project(project).testCond(ProjectPermission.READ);
+
+    assertEquals(cond1, cond2);
+    assertEquals(cond1.hashCode(), cond2.hashCode());
+  }
+
+  @Test
+  public void projectPermissions_sameResourceDifferentUserDoesNotEqual() throws Exception {
+    BooleanCondition cond1 = pb.user(user()).project(project).testCond(ProjectPermission.READ);
+    BooleanCondition cond2 = pb.user(admin()).project(project).testCond(ProjectPermission.READ);
+
+    assertNotEquals(cond1, cond2);
+    assertNotEquals(cond1.hashCode(), cond2.hashCode());
+  }
+
+  @Test
+  public void projectPermissions_differentResourceSameUserDoesNotEqual() throws Exception {
+    Project.NameKey project2 = createProject("p2");
+    BooleanCondition cond1 = pb.user(user()).project(project).testCond(ProjectPermission.READ);
+    BooleanCondition cond2 = pb.user(user()).project(project2).testCond(ProjectPermission.READ);
+
+    assertNotEquals(cond1, cond2);
+    assertNotEquals(cond1.hashCode(), cond2.hashCode());
+  }
+
+  @Test
+  public void refPermissions_sameResourceAndUserEquals() throws Exception {
+    Branch.NameKey branch = new Branch.NameKey(project, "branch");
+    BooleanCondition cond1 = pb.user(user()).ref(branch).testCond(RefPermission.READ);
+    BooleanCondition cond2 = pb.user(user()).ref(branch).testCond(RefPermission.READ);
+
+    assertEquals(cond1, cond2);
+    assertEquals(cond1.hashCode(), cond2.hashCode());
+  }
+
+  @Test
+  public void refPermissions_sameResourceAndDifferentUserDoesNotEqual() throws Exception {
+    Branch.NameKey branch = new Branch.NameKey(project, "branch");
+    BooleanCondition cond1 = pb.user(user()).ref(branch).testCond(RefPermission.READ);
+    BooleanCondition cond2 = pb.user(admin()).ref(branch).testCond(RefPermission.READ);
+
+    assertNotEquals(cond1, cond2);
+    assertNotEquals(cond1.hashCode(), cond2.hashCode());
+  }
+
+  @Test
+  public void refPermissions_differentResourceAndSameUserDoesNotEqual() throws Exception {
+    Branch.NameKey branch1 = new Branch.NameKey(project, "branch");
+    Branch.NameKey branch2 = new Branch.NameKey(project, "branch2");
+    BooleanCondition cond1 = pb.user(user()).ref(branch1).testCond(RefPermission.READ);
+    BooleanCondition cond2 = pb.user(user()).ref(branch2).testCond(RefPermission.READ);
+
+    assertNotEquals(cond1, cond2);
+    assertNotEquals(cond1.hashCode(), cond2.hashCode());
+  }
+
+  @Test
+  public void refPermissions_differentResourceAndSameUserDoesNotEqual2() throws Exception {
+    Branch.NameKey branch1 = new Branch.NameKey(project, "branch");
+    Branch.NameKey branch2 = new Branch.NameKey(createProject("p2"), "branch");
+    BooleanCondition cond1 = pb.user(user()).ref(branch1).testCond(RefPermission.READ);
+    BooleanCondition cond2 = pb.user(user()).ref(branch2).testCond(RefPermission.READ);
+
+    assertNotEquals(cond1, cond2);
+    assertNotEquals(cond1.hashCode(), cond2.hashCode());
+  }
+
+  private CurrentUser user() {
+    return identifiedUserFactory.create(user.id);
+  }
+
+  private CurrentUser admin() {
+    return identifiedUserFactory.create(admin.id);
+  }
+}