Merge changes I92746327,I9516b07c,I56b0f01c,If3477ba6,I8db9e1ed

* changes:
  Change default submit type for new projects to INHERIT
  CreateProjectIT: Assert that API creates refs/meta/config
  RepositoryConfig: Extract a constant for default submit type
  RepositoryConfig: Change return types to ImmutableList
  Streamify RepositoryConfig

Documentation/config-gerrit.txt

@@ -3765,10 +3765,14 @@ are `INHERIT`, `MERGE_IF_NECESSARY`, `FAST_FORWARD_ONLY`, `REBASE_IF_NECESSARY`,
 +
 For more details see link:project-configuration.html#submit_type[Submit Types].
 +
+Default is link:project-configuration.html#submit_type_inherit[`INHERIT`].
++
 This submit type is only applied at project creation time if a submit type is
 omitted from the link:rest-api-projects.html#project-input[ProjectInput]. If the
-submit type is unset in the project config at runtime, it defaults to
-link:project-configuration.html#merge_if_necessary[`MERGE_IF_NECESSARY`].
+submit type is unset in the project config at runtime, for backwards
+compatibility purposes, it defaults to
+link:project-configuration.html#merge_if_necessary[`MERGE_IF_NECESSARY`] rather
+than `INHERIT`.
 
 [[repository.name.ownerGroup]]repository.<name>.ownerGroup::
 +

Documentation/project-configuration.txt

@@ -60,6 +60,9 @@ The following submit types are supported:
 [[submit_type_inherit]]
 * Inherit
 +
+This is the default for new projects, unless overridden by a global
+link:config-gerrit.html#repository.name.defaultSubmitType[`defaultSubmitType` option].
++
 Inherit the submit type from the parent project. In `All-Projects`, this
 is equivalent to link:#merge_if_necessary[Merge If Necessary].
 
@@ -76,9 +79,6 @@ tip of the destination branch at submit time.
 [[merge_if_necessary]]
 * Merge If Necessary
 +
-This is the default for new projects, unless overridden by a global
-link:config-gerrit.html#repository.name.defaultSubmitType[`defaultSubmitType` option].
-+
 If the change being submitted is a strict superset of the destination
 branch, then the branch is fast-forwarded to the change.  If not,
 then a merge commit is automatically created.  This is identical

java/com/google/gerrit/server/config/RepositoryConfig.java

@@ -14,15 +14,18 @@
 
 package com.google.gerrit.server.config;
 
+import static com.google.common.collect.ImmutableList.toImmutableList;
+import static java.util.Comparator.comparing;
+
 import com.google.common.collect.ImmutableList;
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.extensions.client.SubmitType;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.util.ArrayList;
-import java.util.List;
+import java.util.Objects;
 import org.eclipse.jgit.lib.Config;
 
 @Singleton
@@ -33,6 +36,8 @@ public class RepositoryConfig {
   static final String DEFAULT_SUBMIT_TYPE_NAME = "defaultSubmitType";
   static final String BASE_PATH_NAME = "basePath";
 
+  static final SubmitType DEFAULT_SUBMIT_TYPE = SubmitType.INHERIT;
+
   private final Config cfg;
 
   @Inject
@@ -42,13 +47,10 @@ public class RepositoryConfig {
 
   public SubmitType getDefaultSubmitType(Project.NameKey project) {
     return cfg.getEnum(
-        SECTION_NAME,
-        findSubSection(project.get()),
-        DEFAULT_SUBMIT_TYPE_NAME,
-        SubmitType.MERGE_IF_NECESSARY);
+        SECTION_NAME, findSubSection(project.get()), DEFAULT_SUBMIT_TYPE_NAME, DEFAULT_SUBMIT_TYPE);
   }
 
-  public List<String> getOwnerGroups(Project.NameKey project) {
+  public ImmutableList<String> getOwnerGroups(Project.NameKey project) {
     return ImmutableList.copyOf(
         cfg.getStringList(SECTION_NAME, findSubSection(project.get()), OWNER_GROUP_NAME));
   }
@@ -58,22 +60,20 @@ public class RepositoryConfig {
     return basePath != null ? Paths.get(basePath) : null;
   }
 
-  public List<Path> getAllBasePaths() {
-    List<Path> basePaths = new ArrayList<>();
-    for (String subSection : cfg.getSubsections(SECTION_NAME)) {
-      String basePath = cfg.getString(SECTION_NAME, subSection, BASE_PATH_NAME);
-      if (basePath != null) {
-        basePaths.add(Paths.get(basePath));
-      }
-    }
-    return basePaths;
+  public ImmutableList<Path> getAllBasePaths() {
+    return cfg.getSubsections(SECTION_NAME)
+        .stream()
+        .map(sub -> cfg.getString(SECTION_NAME, sub, BASE_PATH_NAME))
+        .filter(Objects::nonNull)
+        .map(Paths::get)
+        .collect(toImmutableList());
   }
 
   /**
-   * Find the subSection to get repository configuration from.
+   * Find the subsection to get repository configuration from.
    *
-   * <p>SubSection can use the * pattern so if project name matches more than one section, return
-   * the more precise one. E.g if the following subSections are defined:
+   * <p>Subsection can use the * pattern so if project name matches more than one section, return
+   * the more precise one. E.g if the following subsections are defined:
    *
    * <pre>
    * [repository "somePath/*"]
@@ -83,20 +83,18 @@ public class RepositoryConfig {
    * </pre>
    *
    * and this method is called with "somePath/somePath/someProject" as project name, it will return
-   * the subSection "somePath/somePath/*"
+   * the subsection "somePath/somePath/*"
    *
    * @param project Name of the project
-   * @return the name of the subSection, null if none is found
+   * @return the name of the subsection, null if none is found
    */
+  @Nullable
   private String findSubSection(String project) {
-    String subSectionFound = null;
-    for (String subSection : cfg.getSubsections(SECTION_NAME)) {
-      if (isMatch(subSection, project)
-          && (subSectionFound == null || subSectionFound.length() < subSection.length())) {
-        subSectionFound = subSection;
-      }
-    }
-    return subSectionFound;
+    return cfg.getSubsections(SECTION_NAME)
+        .stream()
+        .filter(ss -> isMatch(ss, project))
+        .max(comparing(String::length))
+        .orElse(null);
   }
 
   private boolean isMatch(String subSection, String project) {

javatests/com/google/gerrit/acceptance/rest/project/CreateProjectIT.java

@@ -16,8 +16,11 @@ package com.google.gerrit.acceptance.rest.project;
 
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth.assertWithMessage;
+import static com.google.common.truth.Truth8.assertThat;
 import static com.google.gerrit.acceptance.rest.project.ProjectAssert.assertProjectInfo;
 import static com.google.gerrit.acceptance.rest.project.ProjectAssert.assertProjectOwners;
+import static com.google.gerrit.server.git.ProjectConfig.PROJECT_CONFIG;
+import static java.nio.charset.StandardCharsets.UTF_8;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
@@ -46,6 +49,7 @@ import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.server.group.SystemGroupBackend;
 import com.google.gerrit.server.project.ProjectState;
 import java.util.Collections;
+import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.Callable;
 import java.util.concurrent.CyclicBarrier;
@@ -55,7 +59,10 @@ import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
 import org.apache.http.HttpStatus;
 import org.apache.http.message.BasicHeader;
+import org.eclipse.jgit.junit.TestRepository;
 import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.ObjectLoader;
+import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevWalk;
@@ -155,6 +162,8 @@ public class CreateProjectIT extends AbstractDaemonTest {
     assertThat(projectState).isNotNull();
     assertProjectInfo(projectState.getProject(), p);
     assertHead(newProjectName, "refs/heads/master");
+    assertThat(readProjectConfig(newProjectName))
+        .hasValue("[access]\n\tinheritFrom = All-Projects\n[submit]\n\taction = inherit\n");
   }
 
   @Test
@@ -343,7 +352,7 @@ public class CreateProjectIT extends AbstractDaemonTest {
     ConfigInfo cfg = gApi.projects().create(pin).config();
     assertThat(cfg.submitType).isEqualTo(SubmitType.MERGE_IF_NECESSARY);
     assertThat(cfg.defaultSubmitType.value).isEqualTo(SubmitType.MERGE_IF_NECESSARY);
-    assertThat(cfg.defaultSubmitType.configuredValue).isEqualTo(SubmitType.MERGE_IF_NECESSARY);
+    assertThat(cfg.defaultSubmitType.configuredValue).isEqualTo(SubmitType.INHERIT);
     assertThat(cfg.defaultSubmitType.inheritedValue).isEqualTo(SubmitType.MERGE_IF_NECESSARY);
 
     ConfigInput cin = new ConfigInput();
@@ -438,4 +447,19 @@ public class CreateProjectIT extends AbstractDaemonTest {
     exception.expect(errType);
     gApi.projects().create(in);
   }
+
+  private Optional<String> readProjectConfig(String projectName) throws Exception {
+    try (Repository repo = repoManager.openRepository(new Project.NameKey(projectName))) {
+      TestRepository<?> tr = new TestRepository<>(repo);
+      RevWalk rw = tr.getRevWalk();
+      Ref ref = repo.exactRef(RefNames.REFS_CONFIG);
+      if (ref == null) {
+        return Optional.empty();
+      }
+      ObjectLoader obj =
+          rw.getObjectReader()
+              .open(tr.get(rw.parseTree(ref.getObjectId()), PROJECT_CONFIG), Constants.OBJ_BLOB);
+      return Optional.of(new String(obj.getCachedBytes(Integer.MAX_VALUE), UTF_8));
+    }
+  }
 }

javatests/com/google/gerrit/server/config/RepositoryConfigTest.java

@@ -40,8 +40,9 @@ public class RepositoryConfigTest {
 
   @Test
   public void defaultSubmitTypeWhenNotConfigured() {
+    // Check expected value explicitly rather than depending on constant.
     assertThat(repoCfg.getDefaultSubmitType(new NameKey("someProject")))
-        .isEqualTo(SubmitType.MERGE_IF_NECESSARY);
+        .isEqualTo(SubmitType.INHERIT);
   }
 
   @Test
@@ -67,7 +68,7 @@ public class RepositoryConfigTest {
   public void defaultSubmitTypeForSpecificFilter() {
     configureDefaultSubmitType("someProject", SubmitType.CHERRY_PICK);
     assertThat(repoCfg.getDefaultSubmitType(new NameKey("someOtherProject")))
-        .isEqualTo(SubmitType.MERGE_IF_NECESSARY);
+        .isEqualTo(RepositoryConfig.DEFAULT_SUBMIT_TYPE);
     assertThat(repoCfg.getDefaultSubmitType(new NameKey("someProject")))
         .isEqualTo(SubmitType.CHERRY_PICK);
   }

javatests/com/google/gerrit/server/git/MultiBaseLocalDiskRepositoryManagerTest.java

@@ -20,6 +20,7 @@ import static org.easymock.EasyMock.expect;
 import static org.easymock.EasyMock.replay;
 import static org.easymock.EasyMock.reset;
 
+import com.google.common.collect.ImmutableList;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.server.config.RepositoryConfig;
 import com.google.gerrit.server.config.SitePaths;
@@ -28,8 +29,6 @@ import com.google.gerrit.testing.TempFileUtil;
 import java.io.IOException;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.SortedSet;
 import org.eclipse.jgit.errors.RepositoryNotFoundException;
 import org.eclipse.jgit.lib.Config;
@@ -55,7 +54,7 @@ public class MultiBaseLocalDiskRepositoryManagerTest extends GerritBaseTests {
     cfg = new Config();
     cfg.setString("gerrit", null, "basePath", "git");
     configMock = createNiceMock(RepositoryConfig.class);
-    expect(configMock.getAllBasePaths()).andReturn(new ArrayList<Path>()).anyTimes();
+    expect(configMock.getAllBasePaths()).andReturn(ImmutableList.of()).anyTimes();
     replay(configMock);
     repoManager = new MultiBaseLocalDiskRepositoryManager(site, cfg, configMock);
   }
@@ -96,7 +95,7 @@ public class MultiBaseLocalDiskRepositoryManagerTest extends GerritBaseTests {
     Project.NameKey someProjectKey = new Project.NameKey("someProject");
     reset(configMock);
     expect(configMock.getBasePath(someProjectKey)).andReturn(alternateBasePath).anyTimes();
-    expect(configMock.getAllBasePaths()).andReturn(Arrays.asList(alternateBasePath)).anyTimes();
+    expect(configMock.getAllBasePaths()).andReturn(ImmutableList.of(alternateBasePath)).anyTimes();
     replay(configMock);
 
     Repository repo = repoManager.createRepository(someProjectKey);
@@ -130,7 +129,7 @@ public class MultiBaseLocalDiskRepositoryManagerTest extends GerritBaseTests {
     reset(configMock);
     expect(configMock.getBasePath(altPathProject)).andReturn(alternateBasePath).anyTimes();
     expect(configMock.getBasePath(misplacedProject2)).andReturn(alternateBasePath).anyTimes();
-    expect(configMock.getAllBasePaths()).andReturn(Arrays.asList(alternateBasePath)).anyTimes();
+    expect(configMock.getAllBasePaths()).andReturn(ImmutableList.of(alternateBasePath)).anyTimes();
     replay(configMock);
 
     repoManager.createRepository(basePathProject);
@@ -157,7 +156,7 @@ public class MultiBaseLocalDiskRepositoryManagerTest extends GerritBaseTests {
   @Test(expected = IllegalStateException.class)
   public void testRelativeAlternateLocation() {
     configMock = createNiceMock(RepositoryConfig.class);
-    expect(configMock.getAllBasePaths()).andReturn(Arrays.asList(Paths.get("repos"))).anyTimes();
+    expect(configMock.getAllBasePaths()).andReturn(ImmutableList.of(Paths.get("repos"))).anyTimes();
     replay(configMock);
     repoManager = new MultiBaseLocalDiskRepositoryManager(site, cfg, configMock);
   }