Merge branch 'stable-3.5' into stable-3.6

* stable-3.5:
  Allow uploading changes to group refs
  project-configuration: Fix old UI references
  Setup operator aliases with submit requirement expressions

Change-Id: I58c960319d39ae1b4f570619530a2bf2a9dfc08b
Release-Notes: skip

5 files changed, 59 insertions, 20 deletions

diff --git a/Documentation/project-configuration.txt b/Documentation/project-configuration.txt
index e583f457e0..3c88c2e400 100644
--- a/Documentation/project-configuration.txt
+++ b/Documentation/project-configuration.txt
@@ -5,7 +5,7 @@
 
 There are several ways to create a new project in Gerrit:
 
-- in the Web UI under 'Projects' > 'Create Project'
+- click 'CREATE NEW' in the Web UI under 'BROWSE' > 'Repositories'
 - via the link:rest-api-projects.html#create-project[Create Project]
   REST endpoint
 - via the link:cmd-create-project.html[create-project] SSH command
@@ -58,7 +58,7 @@ See details at link:config-project-config.html#project-section[project section].
 
 There are several ways to create a new branch in a project:
 
-- in the Web UI under 'Projects' > 'List' > <project> > 'Branches'
+- in the Web UI under 'BROWSE' > 'Repositories' > <project> > 'Branches'
 - via the link:rest-api-projects.html#create-branch[Create Branch]
   REST endpoint
 - via the link:cmd-create-branch.html[create-branch] SSH command
@@ -84,7 +84,7 @@ are not supported.
 
 There are several ways to delete a branch:
 
-- in the Web UI under 'Projects' > 'List' > <project> > 'Branches'
+- in the Web UI under 'BROWSE' > 'Repositories' > <project> > 'Branches'
 - via the link:rest-api-projects.html#delete-branch[Delete Branch]
   REST endpoint
 - by using a git client
@@ -114,10 +114,11 @@ if the project was created with empty branches.
 For convenience reasons, when the repository is cloned Git creates a
 local branch for this default branch and checks it out.
 
-Project owners can set `HEAD`
+Project owners can set `HEAD` several ways:
 
-- in the Web UI under 'Projects' > 'List' > <project> > 'Branches' or
+- in the Web UI under 'BROWSE' > 'Repositories' > <project> > 'Branches'
 - via the link:rest-api-projects.html#set-head[Set HEAD] REST endpoint
+- via the link:cmd-set-head.html[Set HEAD] SSH command
 
 
 GERRIT
diff --git a/java/com/google/gerrit/server/git/validators/MergeValidators.java b/java/com/google/gerrit/server/git/validators/MergeValidators.java
index 40ce671a36..811e960a34 100644
--- a/java/com/google/gerrit/server/git/validators/MergeValidators.java
+++ b/java/com/google/gerrit/server/git/validators/MergeValidators.java
@@ -36,6 +36,7 @@ import com.google.gerrit.server.config.PluginConfig;
 import com.google.gerrit.server.config.ProjectConfigEntry;
 import com.google.gerrit.server.git.CodeReviewCommit;
 import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
+import com.google.gerrit.server.group.db.GroupConfig;
 import com.google.gerrit.server.permissions.GlobalPermission;
 import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -343,10 +344,12 @@ public class MergeValidators {
     }
 
     private final AllUsersName allUsersName;
+    private final ChangeData.Factory changeDataFactory;
 
     @Inject
-    public GroupMergeValidator(AllUsersName allUsersName) {
+    public GroupMergeValidator(AllUsersName allUsersName, ChangeData.Factory changeDataFactory) {
       this.allUsersName = allUsersName;
+      this.changeDataFactory = changeDataFactory;
     }
 
     @Override
@@ -365,7 +368,30 @@ public class MergeValidators {
         return;
       }
 
-      throw new MergeValidationException("group update not allowed");
+      // Update to group files is not supported because there are no validations
+      // on the changes being done to these files, without which the group data
+      // might get corrupted. Thus don't allow merges into All-Users group refs
+      // which updates group files (i.e., group.config, members and subgroups).
+      // But it is still useful to allow users to update files apart from group
+      // files. For example, users can maintain task config in group refs which
+      // allows users to collaborate and review changes on group specific task configs.
+      ChangeData cd =
+          changeDataFactory.create(destProject.getProject().getNameKey(), patchSetId.changeId());
+      try {
+        if (cd.currentFilePaths().contains(GroupConfig.GROUP_CONFIG_FILE)
+            || cd.currentFilePaths().contains(GroupConfig.MEMBERS_FILE)
+            || cd.currentFilePaths().contains(GroupConfig.SUBGROUPS_FILE)) {
+          throw new MergeValidationException(
+              String.format(
+                  "update to group files (%s, %s, %s) not allowed",
+                  GroupConfig.GROUP_CONFIG_FILE,
+                  GroupConfig.MEMBERS_FILE,
+                  GroupConfig.SUBGROUPS_FILE));
+        }
+      } catch (StorageException e) {
+        logger.atSevere().withCause(e).log("Cannot validate group update");
+        throw new MergeValidationException("group validation unavailable", e);
+      }
     }
   }
 
diff --git a/java/com/google/gerrit/server/group/db/GroupConfig.java b/java/com/google/gerrit/server/group/db/GroupConfig.java
index 4f2c04972b..682fd15f27 100644
--- a/java/com/google/gerrit/server/group/db/GroupConfig.java
+++ b/java/com/google/gerrit/server/group/db/GroupConfig.java
@@ -19,7 +19,6 @@ import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static java.util.Objects.requireNonNull;
 import static java.util.stream.Collectors.joining;
 
-import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Splitter;
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableSet;
@@ -89,9 +88,9 @@ import org.eclipse.jgit.revwalk.RevSort;
  * doesn't have any members or subgroups.
  */
 public class GroupConfig extends VersionedMetaData {
-  @VisibleForTesting public static final String GROUP_CONFIG_FILE = "group.config";
-  @VisibleForTesting static final String MEMBERS_FILE = "members";
-  @VisibleForTesting static final String SUBGROUPS_FILE = "subgroups";
+  public static final String GROUP_CONFIG_FILE = "group.config";
+  public static final String MEMBERS_FILE = "members";
+  public static final String SUBGROUPS_FILE = "subgroups";
   private static final Pattern LINE_SEPARATOR_PATTERN = Pattern.compile("\\R");
 
   /**
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index d6b26b9f0c..6932b0c2dc 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -492,18 +492,22 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil
   @Inject
   ChangeQueryBuilder(Arguments args) {
     this(mydef, args);
-    setupAliases();
   }
 
   @VisibleForTesting
   protected ChangeQueryBuilder(Definition<ChangeData, ChangeQueryBuilder> def, Arguments args) {
     super(def, args.opFactories);
     this.args = args;
+    setupAliases();
   }
 
   private void setupAliases() {
-    setOperatorAliases(args.operatorAliasConfig.getChangeQueryOperatorAliases());
-    hasOperandAliases = args.hasOperandAliasConfig.getChangeQueryHasOperandAliases();
+    if (args.operatorAliasConfig != null) {
+      setOperatorAliases(args.operatorAliasConfig.getChangeQueryOperatorAliases());
+    }
+    if (args.hasOperandAliasConfig != null) {
+      hasOperandAliases = args.hasOperandAliasConfig.getChangeQueryHasOperandAliases();
+    }
   }
 
   public ChangeQueryBuilder asUser(CurrentUser user) {
diff --git a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
index d6302960bd..12f8506b51 100644
--- a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
@@ -1269,16 +1269,24 @@ public class GroupsIT extends AbstractDaemonTest {
   }
 
   @Test
-  public void pushToGroupBranchForReviewForAllUsersRepoIsRejectedOnSubmit() throws Throwable {
+  public void pushToGroupBranchForReviewForAllUsersRepoIsRejectedOnSubmitForGroupFiles()
+      throws Throwable {
+    String error = "update to group files (group.config, members, subgroups) not allowed";
     pushToGroupBranchForReviewAndSubmit(
-        allUsers, RefNames.refsGroups(adminGroupUuid()), "group update not allowed");
+        allUsers, RefNames.refsGroups(adminGroupUuid()), "group.config", error);
+    pushToGroupBranchForReviewAndSubmit(
+        allUsers, RefNames.refsGroups(adminGroupUuid()), "members", error);
+    pushToGroupBranchForReviewAndSubmit(
+        allUsers, RefNames.refsGroups(adminGroupUuid()), "subgroups", error);
+    pushToGroupBranchForReviewAndSubmit(
+        allUsers, RefNames.refsGroups(adminGroupUuid()), "destinations/myreviews", null);
   }
 
   @Test
   public void pushToGroupBranchForReviewForNonAllUsersRepoAndSubmit() throws Throwable {
     String groupRef = RefNames.refsGroups(adminGroupUuid());
     createBranch(project, groupRef);
-    pushToGroupBranchForReviewAndSubmit(project, groupRef, null);
+    pushToGroupBranchForReviewAndSubmit(project, groupRef, "group.config", null);
   }
 
   @Test
@@ -1558,7 +1566,8 @@ public class GroupsIT extends AbstractDaemonTest {
   }
 
   private void pushToGroupBranchForReviewAndSubmit(
-      Project.NameKey project, String groupRef, String expectedError) throws Throwable {
+      Project.NameKey project, String groupRef, String fileName, String expectedError)
+      throws Throwable {
     projectOperations
         .project(project)
         .forUpdate()
@@ -1576,7 +1585,7 @@ public class GroupsIT extends AbstractDaemonTest {
 
     PushOneCommit.Result r =
         pushFactory
-            .create(admin.newIdent(), repo, "Update group config", "group.config", "some content")
+            .create(admin.newIdent(), repo, "Update group config", fileName, "some content")
             .to(MagicBranch.NEW_CHANGE + groupRef);
     r.assertOkStatus();
     assertThat(r.getChange().change().getDest().branch()).isEqualTo(groupRef);
@@ -1585,7 +1594,7 @@ public class GroupsIT extends AbstractDaemonTest {
     ThrowingRunnable submit = () -> gApi.changes().id(r.getChangeId()).current().submit();
     if (expectedError != null) {
       Throwable thrown = assertThrows(ResourceConflictException.class, submit);
-      assertThat(thrown).hasMessageThat().contains("group update not allowed");
+      assertThat(thrown).hasMessageThat().contains(expectedError);
     } else {
       submit.run();
     }