Merge branch 'stable-2.8'

* stable-2.8:
  PatchListCacheImpl: explicitly check for null revision
  ChangeField: skip PatchSets with null revisions
  Rename methods in PluginConfigFactory to be more explicit
  Explain limitation of storing plugin config in existing config files

Change-Id: Id34a619c8979e2d3048244a9fb3d2c03482ca952

Documentation/dev-plugins.txt

@@ -473,6 +473,17 @@ In Gerrit, global configuration is stored in the `gerrit.config` file.
 If a plugin needs global configuration, this configuration should be
 stored in a `plugin` subsection in the `gerrit.config` file.
 
+This approach of storing the plugin configuration is only suitable for
+plugins that have a simple configuration that only consists of
+key-value pairs. With this approach it is not possible to have
+subsections in the plugin configuration. Plugins that require a complex
+configuration need to store their configuration in their own
+configuration file where they can make use of subsections. On the other
+hand storing the plugin configuration in a 'plugin' subsection in the
+`gerrit.config` file has the advantage that administrators have all
+configuration parameters in one file, instead of having one
+configuration file per plugin.
+
 To avoid conflicts with other plugins, it is recommended that plugins
 only use the `plugin` subsection with their own name. For example the
 `helloworld` plugin should store its configuration in the
@@ -494,7 +505,7 @@ private com.google.gerrit.server.config.PluginConfigFactory cfg;
 
 [...]
 
-String language = cfg.get("helloworld")
+String language = cfg.getFromGerritConfig("helloworld")
                      .getString("language", "English");
 ----
 
@@ -508,6 +519,17 @@ needs configuration on project level (e.g. to enable its functionality
 only for certain projects), this configuration should be stored in a
 `plugin` subsection in the project's `project.config` file.
 
+This approach of storing the plugin configuration is only suitable for
+plugins that have a simple configuration that only consists of
+key-value pairs. With this approach it is not possible to have
+subsections in the plugin configuration. Plugins that require a complex
+configuration need to store their configuration in their own
+configuration file where they can make use of subsections. On the other
+hand storing the plugin configuration in a 'plugin' subsection in the
+`project.config` file has the advantage that project owners have all
+configuration parameters in one file, instead of having one
+configuration file per plugin.
+
 To avoid conflicts with other plugins, it is recommended that plugins
 only use the `plugin` subsection with their own name. For example the
 `helloworld` plugin should store its configuration in the
@@ -529,7 +551,7 @@ private com.google.gerrit.server.config.PluginConfigFactory cfg;
 
 [...]
 
-boolean enabled = cfg.get(project, "helloworld")
+boolean enabled = cfg.getFromProjectConfig(project, "helloworld")
                      .getBoolean("enabled", false);
 ----
 
@@ -543,7 +565,7 @@ private com.google.gerrit.server.config.PluginConfigFactory cfg;
 
 [...]
 
-boolean enabled = cfg.getWithInheritance(project, "helloworld")
+boolean enabled = cfg.getFromProjectConfigWithInheritance(project, "helloworld")
                      .getBoolean("enabled", false);
 ----
 

gerrit-server/src/main/java/com/google/gerrit/server/config/PluginConfigFactory.java

@@ -37,12 +37,49 @@ public class PluginConfigFactory {
     this.projectStateFactory = projectStateFactory;
   }
 
-  public PluginConfig get(String pluginName) {
+  /**
+   * Returns the configuration for the specified plugin that is stored in the
+   * 'gerrit.config' file.
+   *
+   * The returned plugin configuration provides access to all parameters of the
+   * 'gerrit.config' file that are set in the 'plugin' subsection of the
+   * specified plugin.
+   *
+   * E.g.:
+   *   [plugin "my-plugin"]
+   *     myKey = myValue
+   *
+   * @param pluginName the name of the plugin for which the configuration should
+   *        be returned
+   * @return the plugin configuration from the 'gerrit.config' file
+   */
+  public PluginConfig getFromGerritConfig(String pluginName) {
     return new PluginConfig(pluginName, cfg);
   }
 
-  public PluginConfig get(Project.NameKey projectName, String pluginName)
-      throws NoSuchProjectException {
+  /**
+   * Returns the configuration for the specified plugin that is stored in the
+   * 'project.config' file of the specified project.
+   *
+   * The returned plugin configuration provides access to all parameters of the
+   * 'project.config' file that are set in the 'plugin' subsection of the
+   * specified plugin.
+   *
+   * E.g.:
+   *   [plugin "my-plugin"]
+   *     myKey = myValue
+   *
+   * @param projectName the name of the project for which the plugin
+   *        configuration should be returned
+   * @param pluginName the name of the plugin for which the configuration should
+   *        be returned
+   * @return the plugin configuration from the 'project.config' file of the
+   *         specified project
+   * @throws NoSuchProjectException thrown if the specified project does not
+   *         exist
+   */
+  public PluginConfig getFromProjectConfig(Project.NameKey projectName,
+      String pluginName) throws NoSuchProjectException {
     ProjectState projectState = projectCache.get(projectName);
     if (projectState == null) {
       throw new NoSuchProjectException(projectName);
@@ -50,8 +87,45 @@ public class PluginConfigFactory {
     return projectState.getConfig().getPluginConfig(pluginName);
   }
 
-  public PluginConfig getWithInheritance(Project.NameKey projectName,
-      String pluginName) throws NoSuchProjectException {
-    return get(projectName, pluginName).withInheritance(projectStateFactory);
+  /**
+   * Returns the configuration for the specified plugin that is stored in the
+   * 'project.config' file of the specified project. Parameters which are not
+   * set in the 'project.config' of this project are inherited from the parent
+   * project's 'project.config' files.
+   *
+   * The returned plugin configuration provides access to all parameters of the
+   * 'project.config' file that are set in the 'plugin' subsection of the
+   * specified plugin.
+   *
+   * E.g.:
+   * child project:
+   *   [plugin "my-plugin"]
+   *     myKey = childValue
+   *
+   * parent project:
+   *   [plugin "my-plugin"]
+   *     myKey = parentValue
+   *     anotherKey = someValue
+   *
+   * return:
+   *   [plugin "my-plugin"]
+   *     myKey = childValue
+   *     anotherKey = someValue
+   *
+   * @param projectName the name of the project for which the plugin
+   *        configuration should be returned
+   * @param pluginName the name of the plugin for which the configuration should
+   *        be returned
+   * @return the plugin configuration from the 'project.config' file of the
+   *         specified project with inherited non-set parameters from the
+   *         parent projects
+   * @throws NoSuchProjectException thrown if the specified project does not
+   *         exist
+   */
+  public PluginConfig getFromProjectConfigWithInheritance(
+      Project.NameKey projectName, String pluginName)
+      throws NoSuchProjectException {
+    return getFromProjectConfig(projectName, pluginName).withInheritance(
+        projectStateFactory);
   }
 }