Merge "Implement equals() and hashCode() on PermissionBackendCondition"

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
@@ -77,6 +79,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 {
@@ -105,6 +121,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 {
@@ -133,6 +165,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 {
@@ -161,5 +209,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);
+  }
+}