Merge branch 'stable-2.14'

* stable-2.14:
  Introduce GlobalPluginConfig{s} annotations
  Clarify usage of @GerritConfig#value() and #values()
  Clarify usage of GerritConfig#name()
  GerritServer: Split annotation sanity check to a separate method

Change-Id: I8c7e145ec4ddacd049f7d908fd74dcf0046c677e

gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java

@@ -83,6 +83,7 @@ import com.google.gerrit.server.change.Revisions;
 import com.google.gerrit.server.config.AllProjectsName;
 import com.google.gerrit.server.config.CanonicalWebUrl;
 import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.config.PluginConfigFactory;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.git.MetaDataUpdate;
 import com.google.gerrit.server.git.ProjectConfig;
@@ -216,6 +217,7 @@ public abstract class AbstractDaemonTest {
   @Inject protected ProjectCache projectCache;
   @Inject protected Provider<InternalChangeQuery> queryProvider;
   @Inject protected PushOneCommit.Factory pushFactory;
+  @Inject protected PluginConfigFactory pluginConfig;
   @Inject protected Revisions revisions;
   @Inject protected SystemGroupBackend systemGroupBackend;
   @Inject protected TestNotesMigration notesMigration;

gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/ConfigAnnotationParser.java

@@ -17,8 +17,11 @@ package com.google.gerrit.acceptance;
 import com.google.common.base.Splitter;
 import com.google.common.base.Strings;
 import com.google.common.collect.Lists;
+import java.lang.annotation.Annotation;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
 import org.eclipse.jgit.lib.Config;
 
 class ConfigAnnotationParser {
@@ -42,6 +45,67 @@ class ConfigAnnotationParser {
     return cfg;
   }
 
+  static class GlobalPluginConfigToGerritConfig implements GerritConfig {
+    private final GlobalPluginConfig delegate;
+
+    GlobalPluginConfigToGerritConfig(GlobalPluginConfig delegate) {
+      this.delegate = delegate;
+    }
+
+    @Override
+    public Class<? extends Annotation> annotationType() {
+      return delegate.annotationType();
+    }
+
+    @Override
+    public String name() {
+      return delegate.name();
+    }
+
+    @Override
+    public String value() {
+      return delegate.value();
+    }
+
+    @Override
+    public String[] values() {
+      return delegate.values();
+    }
+  }
+
+  static Map<String, Config> parse(GlobalPluginConfig annotation) {
+    if (annotation == null) {
+      return null;
+    }
+    Map<String, Config> result = new HashMap<>();
+    Config cfg = new Config();
+    parseAnnotation(cfg, new GlobalPluginConfigToGerritConfig(annotation));
+    result.put(annotation.pluginName(), cfg);
+    return result;
+  }
+
+  static Map<String, Config> parse(GlobalPluginConfigs annotation) {
+    if (annotation == null || annotation.value().length < 1) {
+      return null;
+    }
+
+    HashMap<String, Config> result = new HashMap<>();
+
+    for (GlobalPluginConfig c : annotation.value()) {
+      String pluginName = c.pluginName();
+      Config config;
+      if (result.containsKey(pluginName)) {
+        config = result.get(pluginName);
+      } else {
+        config = new Config();
+        result.put(pluginName, config);
+      }
+      parseAnnotation(config, new GlobalPluginConfigToGerritConfig(c));
+    }
+
+    return result;
+  }
+
   private static void parseAnnotation(Config cfg, GerritConfig c) {
     ArrayList<String> l = Lists.newArrayList(splitter.split(c.name()));
     if (l.size() == 2) {

gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GerritConfig.java

@@ -25,9 +25,22 @@ import java.lang.annotation.Target;
 @Retention(RUNTIME)
 @Repeatable(GerritConfigs.class)
 public @interface GerritConfig {
+  /**
+   * Setting name in the form {@code "section.name"} or {@code "section.subsection.name"} where
+   * {@code section}, {@code subsection} and {@code name} correspond to the parameters of the same
+   * names in JGit's {@code Config#getString} method.
+   *
+   * @see org.eclipse.jgit.lib.Config#getString(String, String, String)
+   */
   String name();
 
+  /**
+   * Single value. Takes precedence over values specified in {@code values}.
+   */
   String value() default "";
 
+  /**
+   * Multiple values (list). Ignored if {@code value} is specified.
+   */
   String[] values() default "";
 }

gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GerritServer.java

@@ -44,6 +44,8 @@ import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.URI;
 import java.nio.file.Paths;
+import java.util.HashMap;
+import java.util.Map;
 import java.util.concurrent.BrokenBarrierException;
 import java.util.concurrent.CyclicBarrier;
 import java.util.concurrent.ExecutorService;
@@ -68,7 +70,9 @@ public class GerritServer {
           has(Sandboxed.class, testDesc.getTestClass()),
           has(UseSsh.class, testDesc.getTestClass()),
           null, // @GerritConfig is only valid on methods.
-          null); // @GerritConfigs is only valid on methods.
+          null, // @GerritConfigs is only valid on methods.
+          null, // @GlobalPluginConfig is only valid on methods.
+          null); // @GlobalPluginConfigs is only valid on methods.
     }
 
     static Description forTestMethod(org.junit.runner.Description testDesc, String configName) {
@@ -83,7 +87,9 @@ public class GerritServer {
           testDesc.getAnnotation(UseSsh.class) != null
               || has(UseSsh.class, testDesc.getTestClass()),
           testDesc.getAnnotation(GerritConfig.class),
-          testDesc.getAnnotation(GerritConfigs.class));
+          testDesc.getAnnotation(GerritConfigs.class),
+          testDesc.getAnnotation(GlobalPluginConfig.class),
+          testDesc.getAnnotation(GlobalPluginConfigs.class));
     }
 
     private static boolean has(Class<? extends Annotation> annotation, Class<?> clazz) {
@@ -114,10 +120,26 @@ public class GerritServer {
     @Nullable
     abstract GerritConfigs configs();
 
-    private Config buildConfig(Config baseConfig) {
+    @Nullable
+    abstract GlobalPluginConfig pluginConfig();
+
+    @Nullable
+    abstract GlobalPluginConfigs pluginConfigs();
+
+    private void checkValidAnnotations() {
       if (configs() != null && config() != null) {
         throw new IllegalStateException("Use either @GerritConfigs or @GerritConfig not both");
       }
+      if (pluginConfigs() != null && pluginConfig() != null) {
+        throw new IllegalStateException(
+            "Use either @GlobalPluginConfig or @GlobalPluginConfigs not both");
+      }
+      if ((pluginConfigs() != null || pluginConfig() != null) && memory()) {
+        throw new IllegalStateException("Must use @UseLocalDisk with @GlobalPluginConfig(s)");
+      }
+    }
+
+    private Config buildConfig(Config baseConfig) {
       if (configs() != null) {
         return ConfigAnnotationParser.parse(baseConfig, configs());
       } else if (config() != null) {
@@ -126,10 +148,20 @@ public class GerritServer {
         return baseConfig;
       }
     }
+
+    private Map<String, Config> buildPluginConfigs() {
+      if (pluginConfigs() != null) {
+        return ConfigAnnotationParser.parse(pluginConfigs());
+      } else if (pluginConfig() != null) {
+        return ConfigAnnotationParser.parse(pluginConfig());
+      }
+      return new HashMap<>();
+    }
   }
 
   /** Returns fully started Gerrit server */
   static GerritServer start(Description desc, Config baseConfig) throws Exception {
+    desc.checkValidAnnotations();
     Config cfg = desc.buildConfig(baseConfig);
     Logger.getLogger("com.google.gerrit").setLevel(Level.DEBUG);
     CyclicBarrier serverStarted = new CyclicBarrier(2);
@@ -164,7 +196,7 @@ public class GerritServer {
           ImmutableList.<Module>of(new InMemoryTestingDatabaseModule(cfg)));
       daemon.start();
     } else {
-      site = initSite(cfg);
+      site = initSite(cfg, desc.buildPluginConfigs());
       daemonService = Executors.newSingleThreadExecutor();
       @SuppressWarnings("unused")
       Future<?> possiblyIgnoredError =
@@ -189,7 +221,7 @@ public class GerritServer {
     return new GerritServer(desc, i, daemon, daemonService);
   }
 
-  private static File initSite(Config base) throws Exception {
+  private static File initSite(Config base, Map<String, Config> pluginConfigs) throws Exception {
     File tmp = TempFileUtil.createTempDirectory();
     Init init = new Init();
     int rc =
@@ -207,6 +239,16 @@ public class GerritServer {
     cfg.merge(base);
     mergeTestConfig(cfg);
     cfg.save();
+
+    for (String pluginName : pluginConfigs.keySet()) {
+      MergeableFileBasedConfig pluginCfg =
+          new MergeableFileBasedConfig(
+              new File(new File(tmp, "etc"), pluginName + ".config"), FS.DETECTED);
+      pluginCfg.load();
+      pluginCfg.merge(pluginConfigs.get(pluginName));
+      pluginCfg.save();
+    }
+
     return tmp;
   }
 

gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GlobalPluginConfig.java

@@ -0,0 +1,47 @@
+// Copyright (C) 2017 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;
+
+import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+import java.lang.annotation.Repeatable;
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+@Target({METHOD})
+@Retention(RUNTIME)
+@Repeatable(GlobalPluginConfigs.class)
+public @interface GlobalPluginConfig {
+  /**
+   * Name of the plugin, corresponding to {@code $site/etc/@pluginName.comfig}.
+   */
+  String pluginName();
+
+  /**
+   * @see GerritConfig#name()
+   */
+  String name();
+
+  /**
+   * @see GerritConfig#value()
+   */
+  String value() default "";
+
+  /**
+   * @see GerritConfig#values()
+   */
+  String[] values() default "";
+}

gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GlobalPluginConfigs.java

@@ -0,0 +1,27 @@
+// Copyright (C) 2017 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;
+
+import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+@Target({METHOD})
+@Retention(RUNTIME)
+public @interface GlobalPluginConfigs {
+  GlobalPluginConfig[] value();
+}

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/annotation/UseGerritConfigAnnotationTest.java

@@ -22,25 +22,60 @@ import org.junit.Test;
 
 public class UseGerritConfigAnnotationTest extends AbstractDaemonTest {
   @Test
-  @GerritConfig(name = "x.y", value = "z")
+  @GerritConfig(name = "section.name", value = "value")
   public void testOne() {
-    assertThat(cfg.getString("x", null, "y")).isEqualTo("z");
+    assertThat(cfg.getString("section", null, "name")).isEqualTo("value");
   }
 
   @Test
-  @GerritConfig(name = "x.y", value = "z")
-  @GerritConfig(name = "a.b", value = "c")
+  @GerritConfig(name = "section.subsection.name", value = "value")
+  public void testOneWithSubsection() {
+    assertThat(cfg.getString("section", "subsection", "name")).isEqualTo("value");
+  }
+
+  @Test
+  @GerritConfig(name = "section.name", value = "value")
+  @GerritConfig(name = "section1.name", value = "value1")
+  @GerritConfig(name = "section.subsection.name", value = "value")
+  @GerritConfig(name = "section.subsection1.name", value = "value1")
   public void testMultiple() {
-    assertThat(cfg.getString("x", null, "y")).isEqualTo("z");
-    assertThat(cfg.getString("a", null, "b")).isEqualTo("c");
+    assertThat(cfg.getString("section", null, "name")).isEqualTo("value");
+    assertThat(cfg.getString("section1", null, "name")).isEqualTo("value1");
+    assertThat(cfg.getString("section", "subsection", "name")).isEqualTo("value");
+    assertThat(cfg.getString("section", "subsection1", "name")).isEqualTo("value1");
   }
 
   @Test
   @GerritConfig(
-    name = "x.y",
-    values = {"a", "b"}
+    name = "section.name",
+    values = {"value-1", "value-2"}
   )
   public void testList() {
-    assertThat(cfg.getStringList("x", null, "y")).asList().containsExactly("a", "b");
+    assertThat(cfg.getStringList("section", null, "name"))
+        .asList()
+        .containsExactly("value-1", "value-2");
+  }
+
+  @Test
+  @GerritConfig(
+    name = "section.subsection.name",
+    values = {"value-1", "value-2"}
+  )
+  public void testListWithSubsection() {
+    assertThat(cfg.getStringList("section", "subsection", "name"))
+        .asList()
+        .containsExactly("value-1", "value-2");
+  }
+
+  @Test
+  @GerritConfig(
+    name = "section.name",
+    value = "value-1",
+    values = {"value-2", "value-3"}
+  )
+  public void valueHasPrecedenceOverValues() {
+    assertThat(cfg.getStringList("section", null, "name"))
+        .asList()
+        .containsExactly("value-1");
   }
 }

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/annotation/UseGlobalPluginConfigAnnotationTest.java

@@ -0,0 +1,94 @@
+// Copyright (C) 2017 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.annotation;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.GlobalPluginConfig;
+import com.google.gerrit.acceptance.UseLocalDisk;
+import org.eclipse.jgit.lib.Config;
+import org.junit.Test;
+
+public class UseGlobalPluginConfigAnnotationTest extends AbstractDaemonTest {
+  private Config cfg() {
+    return pluginConfig.getGlobalPluginConfig("test");
+  }
+
+  @Test
+  @UseLocalDisk
+  @GlobalPluginConfig(pluginName = "test", name = "section.name", value = "value")
+  public void testOne() {
+    assertThat(cfg().getString("section", null, "name")).isEqualTo("value");
+  }
+
+  @Test
+  @UseLocalDisk
+  @GlobalPluginConfig(pluginName = "test", name = "section.subsection.name", value = "value")
+  public void testOneWithSubsection() {
+    assertThat(cfg().getString("section", "subsection", "name")).isEqualTo("value");
+  }
+
+  @Test
+  @UseLocalDisk
+  @GlobalPluginConfig(pluginName = "test", name = "section.name", value = "value")
+  @GlobalPluginConfig(pluginName = "test", name = "section1.name", value = "value1")
+  @GlobalPluginConfig(pluginName = "test", name = "section.subsection.name", value = "value")
+  @GlobalPluginConfig(pluginName = "test", name = "section.subsection1.name", value = "value1")
+  public void testMultiple() {
+    assertThat(cfg().getString("section", null, "name")).isEqualTo("value");
+    assertThat(cfg().getString("section1", null, "name")).isEqualTo("value1");
+    assertThat(cfg().getString("section", "subsection", "name")).isEqualTo("value");
+    assertThat(cfg().getString("section", "subsection1", "name")).isEqualTo("value1");
+  }
+
+  @Test
+  @UseLocalDisk
+  @GlobalPluginConfig(
+    pluginName = "test",
+    name = "section.name",
+    values = {"value-1", "value-2"}
+  )
+  public void testList() {
+    assertThat(cfg().getStringList("section", null, "name"))
+        .asList()
+        .containsExactly("value-1", "value-2");
+  }
+
+  @Test
+  @UseLocalDisk
+  @GlobalPluginConfig(
+    pluginName = "test",
+    name = "section.subsection.name",
+    values = {"value-1", "value-2"}
+  )
+  public void testListWithSubsection() {
+    assertThat(cfg().getStringList("section", "subsection", "name"))
+        .asList()
+        .containsExactly("value-1", "value-2");
+  }
+
+  @Test
+  @UseLocalDisk
+  @GlobalPluginConfig(
+    pluginName = "test",
+    name = "section.name",
+    value = "value-1",
+    values = {"value-2", "value-3"}
+  )
+  public void valueHasPrecedenceOverValues() {
+    assertThat(cfg().getStringList("section", null, "name")).asList().containsExactly("value-1");
+  }
+}