Merge "Make project state check in READ explicit"

java/com/google/gerrit/server/git/LocalMergeSuperSetComputation.java

@@ -33,6 +33,8 @@ import com.google.gerrit.server.permissions.ChangePermission;
 import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.project.NoSuchProjectException;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.ProjectState;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gerrit.server.query.change.InternalChangeQuery;
 import com.google.gwtorm.server.OrmException;
@@ -87,10 +89,14 @@ public class LocalMergeSuperSetComputation implements MergeSuperSetComputation {
   private final Provider<InternalChangeQuery> queryProvider;
   private final Map<QueryKey, List<ChangeData>> queryCache;
   private final Map<Branch.NameKey, Optional<RevCommit>> heads;
+  private final ProjectCache projectCache;
 
   @Inject
   LocalMergeSuperSetComputation(
-      PermissionBackend permissionBackend, Provider<InternalChangeQuery> queryProvider) {
+      PermissionBackend permissionBackend,
+      Provider<InternalChangeQuery> queryProvider,
+      ProjectCache projectCache) {
+    this.projectCache = projectCache;
     this.permissionBackend = permissionBackend;
     this.queryProvider = queryProvider;
     this.queryCache = new HashMap<>();
@@ -171,8 +177,12 @@ public class LocalMergeSuperSetComputation implements MergeSuperSetComputation {
   }
 
   private boolean isVisible(ReviewDb db, ChangeSet changeSet, ChangeData cd, CurrentUser user)
-      throws PermissionBackendException {
-    boolean visible = changeSet.ids().contains(cd.getId());
+      throws PermissionBackendException, IOException {
+    ProjectState projectState = projectCache.checkedGet(cd.project());
+    boolean visible =
+        changeSet.ids().contains(cd.getId())
+            && (projectState != null)
+            && projectState.statePermitsRead();
     if (visible
         && !permissionBackend.user(user).change(cd).database(db).test(ChangePermission.READ)) {
       // We thought the change was visible, but it isn't.

java/com/google/gerrit/server/git/MergeSuperSet.java

@@ -27,6 +27,8 @@ import com.google.gerrit.server.index.change.ChangeField;
 import com.google.gerrit.server.permissions.ChangePermission;
 import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.ProjectState;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gerrit.server.query.change.InternalChangeQuery;
 import com.google.gwtorm.server.OrmException;
@@ -75,6 +77,7 @@ public class MergeSuperSet {
   private final DynamicItem<MergeSuperSetComputation> mergeSuperSetComputation;
   private final PermissionBackend permissionBackend;
   private final Config cfg;
+  private final ProjectCache projectCache;
 
   private MergeOpRepoManager orm;
   private boolean closeOrm;
@@ -86,13 +89,15 @@ public class MergeSuperSet {
       Provider<InternalChangeQuery> queryProvider,
       Provider<MergeOpRepoManager> repoManagerProvider,
       DynamicItem<MergeSuperSetComputation> mergeSuperSetComputation,
-      PermissionBackend permissionBackend) {
+      PermissionBackend permissionBackend,
+      ProjectCache projectCache) {
     this.cfg = cfg;
     this.changeDataFactory = changeDataFactory;
     this.queryProvider = queryProvider;
     this.repoManagerProvider = repoManagerProvider;
     this.mergeSuperSetComputation = mergeSuperSetComputation;
     this.permissionBackend = permissionBackend;
+    this.projectCache = projectCache;
   }
 
   public static boolean wholeTopicEnabled(Config config) {
@@ -115,9 +120,17 @@ public class MergeSuperSet {
       }
 
       ChangeData cd = changeDataFactory.create(db, change.getProject(), change.getId());
+      ProjectState projectState = projectCache.checkedGet(cd.project());
       ChangeSet changeSet =
           new ChangeSet(
-              cd, permissionBackend.user(user).change(cd).database(db).test(ChangePermission.READ));
+              cd,
+              projectState != null
+                  && projectState.statePermitsRead()
+                  && permissionBackend
+                      .user(user)
+                      .change(cd)
+                      .database(db)
+                      .test(ChangePermission.READ));
       if (wholeTopicEnabled(cfg)) {
         return completeChangeSetIncludingTopics(db, changeSet, user);
       }
@@ -149,7 +162,7 @@ public class MergeSuperSet {
       CurrentUser user,
       Set<String> topicsSeen,
       Set<String> visibleTopicsSeen)
-      throws OrmException, PermissionBackendException {
+      throws OrmException, PermissionBackendException, IOException {
     List<ChangeData> visibleChanges = new ArrayList<>();
     List<ChangeData> nonVisibleChanges = new ArrayList<>();
 
@@ -208,7 +221,10 @@ public class MergeSuperSet {
   }
 
   private boolean canRead(ReviewDb db, CurrentUser user, ChangeData cd)
-      throws PermissionBackendException {
-    return permissionBackend.user(user).change(cd).database(db).test(ChangePermission.READ);
+      throws PermissionBackendException, IOException {
+    ProjectState projectState = projectCache.checkedGet(cd.project());
+    return projectState != null
+        && projectState.statePermitsRead()
+        && permissionBackend.user(user).change(cd).database(db).test(ChangePermission.READ);
   }
 }

java/com/google/gerrit/server/git/VisibleRefFilter.java

@@ -316,7 +316,8 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
       Map<Change.Id, Branch.NameKey> visibleChanges = new HashMap<>();
       for (ChangeData cd : changeCache.getChangeData(db.get(), project)) {
         ChangeNotes notes = changeNotesFactory.createFromIndexedChange(cd.change());
-        if (perm.indexedChange(cd, notes).test(ChangePermission.READ)) {
+        if (projectState.statePermitsRead()
+            && perm.indexedChange(cd, notes).test(ChangePermission.READ)) {
           visibleChanges.put(cd.getId(), cd.change().getDest());
         }
       }
@@ -349,7 +350,7 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
       return null;
     }
     try {
-      if (perm.change(r.notes()).test(ChangePermission.READ)) {
+      if (projectState.statePermitsRead() && perm.change(r.notes()).test(ChangePermission.READ)) {
         return r.notes();
       }
     } catch (PermissionBackendException e) {