Merge branch 'stable-2.16'

* stable-2.16: Fix missing `</section>` in gr-settings-view Suggest --no-edit when Change-Id is missing PG: Assume weblinks have correct direct or relative urls PG: Make commitlink selection configurable and consistent Add config gerrit.primaryWeblinkName gerrit.sh: Improve error message when JRE cannot be found PrologEnvironment#setPredicate: Reduce log level to DEBUG ElasticContainer: Bump V7_0 test server to 7.0.0-beta1 Change-Id: I8bef4ae02e935809de6d7abf59e1a8de6800432c
2019-02-19 10:25:32 +09:00
parent 707e61a75a a172ef1aa3
commit b8969585ec
15 changed files with 140 additions and 54 deletions
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -1967,6 +1967,22 @@ Example:
  installModule = com.example.abc.OurSpecialSauceModule
 ----

+[[gerrit.primaryWeblinkName]]gerrit.primaryWeblinkName::
+
+Name of the link:dev-plugins.html#links-to-external-tools[Weblink] that should
+be chosen in cases where only one Weblink can be used in the UI, for example in
+inline links.
+
+By default unset, meaning that the UI is responsible for trying to identify
+a weblink to be used in these cases, most likely weblinks that links to code
+browsers with known integrations with Gerrit (like Gitiles and Gitweb).
+
+Example:
+----
+[gerrit]
+  primaryWeblinkName = gitiles
+----
+
 [[gerrit.reportBugUrl]]gerrit.reportBugUrl::
 +
 URL to direct users to when they need to report a bug.
--- a/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java
+++ b/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java
@@ -199,7 +199,9 @@ abstract class AbstractElasticIndex<K, V> implements Index<K, V> {

    // Recreate the index.
    String indexCreationFields = concatJsonString(getSettings(), getMappings());
-    response = performRequest("PUT", indexName, indexCreationFields);
+    response =
+        performRequest(
+            "PUT", indexName + client.adapter().includeTypeNameParam(), indexCreationFields);
    statusCode = response.getStatusLine().getStatusCode();
    if (statusCode != HttpStatus.SC_OK) {
      String error = String.format("Failed to create index %s: %s", indexName, statusCode);
--- a/java/com/google/gerrit/elasticsearch/ElasticQueryAdapter.java
+++ b/java/com/google/gerrit/elasticsearch/ElasticQueryAdapter.java
@@ -30,6 +30,7 @@ public class ElasticQueryAdapter {
  private final String indexProperty;
  private final String rawFieldsKey;
  private final String versionDiscoveryUrl;
+  private final String includeTypeNameParam;

  ElasticQueryAdapter(ElasticVersion version) {
    this.ignoreUnmapped = false;
@@ -42,6 +43,7 @@ public class ElasticQueryAdapter {
    this.stringFieldType = "text";
    this.indexProperty = "true";
    this.rawFieldsKey = "_source";
+    this.includeTypeNameParam = version.isV7OrLater() ? "?include_type_name=true" : "";
  }

  void setIgnoreUnmapped(JsonObject properties) {
@@ -95,4 +97,8 @@ public class ElasticQueryAdapter {
  String getVersionDiscoveryUrl(String name) {
    return String.format(versionDiscoveryUrl, name);
  }
+
+  String includeTypeNameParam() {
+    return includeTypeNameParam;
+  }
 }
--- a/java/com/google/gerrit/extensions/common/GerritInfo.java
+++ b/java/com/google/gerrit/extensions/common/GerritInfo.java
@@ -22,4 +22,5 @@ public class GerritInfo {
  public Boolean editGpgKeys;
  public String reportBugUrl;
  public String reportBugText;
+  public String primaryWeblinkName;
 }
--- a/java/com/google/gerrit/server/git/validators/CommitValidators.java
+++ b/java/com/google/gerrit/server/git/validators/CommitValidators.java
@@ -333,7 +333,7 @@ public class CommitValidators {
            .append(getCommitMessageHookInstallationHint())
            .append("\n")
            .append("and then amend the commit:\n")
-            .append("  git commit --amend\n")
+            .append("  git commit --amend --no-edit\n")
            .append("Finally, push your changes again\n");
      }
      return new CommitValidationMessage(sb.toString(), Type.ERROR);
--- a/java/com/google/gerrit/server/restapi/config/GetServerInfo.java
+++ b/java/com/google/gerrit/server/restapi/config/GetServerInfo.java
@@ -298,6 +298,7 @@ public class GetServerInfo implements RestReadView<ConfigResource> {
    info.docSearch = docSearcher.isAvailable();
    info.editGpgKeys =
        toBoolean(enableSignedPush && config.getBoolean("gerrit", null, "editGpgKeys", true));
+    info.primaryWeblinkName = config.getString("gerrit", null, "primaryWeblinkName");
    return info;
  }

--- a/java/com/google/gerrit/server/rules/PrologEnvironment.java
+++ b/java/com/google/gerrit/server/rules/PrologEnvironment.java
@@ -84,7 +84,7 @@ public class PrologEnvironment extends BufferingPrologControl {
  public void setPredicate(Predicate goal) {
    super.setPredicate(goal);
    int reductionLimit = args.reductionLimit(goal);
-    logger.atInfo().log("setting reductionLimit %d", reductionLimit);
+    logger.atFine().log("setting reductionLimit %d", reductionLimit);
    setReductionLimit(reductionLimit);
  }

--- a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
@@ -49,7 +49,7 @@ public class ElasticContainer extends ElasticsearchContainer {
      case V6_6:
        return "docker.elastic.co/elasticsearch/elasticsearch-oss:6.6.0";
      case V7_0:
-        return "docker.elastic.co/elasticsearch/elasticsearch-oss:7.0.0-alpha2";
+        return "docker.elastic.co/elasticsearch/elasticsearch-oss:7.0.0-beta1";
    }
    throw new IllegalStateException("No tests for version: " + version.name());
  }
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
@@ -330,11 +330,11 @@ limitations under the License.
            account="[[account]]"
            mutable="[[_mutable]]"></gr-change-requirements>
      </div>
-      <section id="webLinks" hidden$="[[!_computeWebLinks(commitInfo)]]">
+      <section id="webLinks" hidden$="[[!_computeWebLinks(commitInfo, serverConfig)]]">
        <span class="title">Links</span>
        <span class="value">
          <template is="dom-repeat"
-              items="[[_computeWebLinks(commitInfo)]]" as="link">
+              items="[[_computeWebLinks(commitInfo, serverConfig)]]" as="link">
            <a href="[[link.url]]" class="webLink" rel="noopener" target="_blank">
              [[link.name]]
            </a>
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
@@ -191,12 +191,15 @@
     * an existential check can be used to hide or show the webLinks
     * section.
     */
-    _computeWebLinks(commitInfo) {
+    _computeWebLinks(commitInfo, serverConfig) {
      if (!commitInfo) { return null; }
      const weblinks = Gerrit.Nav.getChangeWeblinks(
          this.change ? this.change.repo : '',
          commitInfo.commit,
-          {weblinks: commitInfo.web_links});
+          {
+            weblinks: commitInfo.web_links,
+            config: serverConfig,
+          });
      return weblinks.length ? weblinks : null;
    },

--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html
@@ -133,6 +133,7 @@ limitations under the License.
      const weblinksStub = sandbox.stub(Gerrit.Nav, '_generateWeblinks')
          .returns([{name: 'stubb', url: '#s'}]);
      element.commitInfo = {};
+      element.serverConfig = {};
      flushAsynchronousOperations();
      const webLinks = element.$.webLinks;
      assert.isTrue(weblinksStub.called);
@@ -142,6 +143,7 @@ limitations under the License.

    test('weblinks hidden when no weblinks', () => {
      element.commitInfo = {};
+      element.serverConfig = {};
      flushAsynchronousOperations();
      const webLinks = element.$.webLinks;
      assert.isTrue(webLinks.hasAttribute('hidden'));
@@ -149,12 +151,26 @@ limitations under the License.

    test('weblinks hidden when only gitiles weblink', () => {
      element.commitInfo = {web_links: [{name: 'gitiles', url: '#'}]};
+      element.serverConfig = {};
      flushAsynchronousOperations();
      const webLinks = element.$.webLinks;
      assert.isTrue(webLinks.hasAttribute('hidden'));
      assert.equal(element._computeWebLinks(element.commitInfo), null);
    });

+    test('weblinks hidden when sole weblink is set as primary', () => {
+      const browser = 'browser';
+      element.commitInfo = {web_links: [{name: browser, url: '#'}]};
+      element.serverConfig = {
+        gerrit: {
+          primary_weblink_name: browser,
+        },
+      };
+      flushAsynchronousOperations();
+      const webLinks = element.$.webLinks;
+      assert.isTrue(webLinks.hasAttribute('hidden'));
+    });
+
    test('weblinks are visible when other weblinks', () => {
      const router = document.createElement('gr-router');
      sandbox.stub(Gerrit.Nav, '_generateWeblinks',
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.js
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.js
@@ -281,38 +281,52 @@

    _getPatchSetWeblink(params) {
      const {commit, options} = params;
-      const {weblinks} = options || {};
+      const {weblinks, config} = options || {};
      const name = commit && commit.slice(0, 7);
-      const url = this._getSupportedWeblinkUrl(weblinks);
-      if (!url) {
+      const weblink = this._getBrowseCommitWeblink(weblinks, config);
+      if (!weblink || !weblink.url) {
        return {name};
      } else {
-        return {name, url};
+        return {name, url: weblink.url};
      }
    },

-    _isDirectCommit(link) {
-      // This is a whitelist of web link types that provide direct links to
-      // the commit in the url property.
-      return link.name === 'gitiles' || link.name === 'gitweb';
+    _firstCodeBrowserWeblink(weblinks) {
+      // This is an ordered whitelist of web link types that provide direct
+      // links to the commit in the url property.
+      const codeBrowserLinks = ['gitiles', 'browse', 'gitweb'];
+      for (let i = 0; i < codeBrowserLinks.length; i++) {
+        const weblink =
+          weblinks.find(weblink => weblink.name === codeBrowserLinks[i]);
+        if (weblink) { return weblink; }
+      }
+      return null;
    },

-    _getSupportedWeblinkUrl(weblinks) {
+
+    _getBrowseCommitWeblink(weblinks, config) {
      if (!weblinks) { return null; }
-      const weblink = weblinks.find(this._isDirectCommit);
+      let weblink;
+      // Use primary weblink if configured and exists.
+      if (config && config.gerrit && config.gerrit.primary_weblink_name) {
+        weblink = weblinks.find(
+            weblink => weblink.name === config.gerrit.primary_weblink_name
+        );
+      }
+      if (!weblink) {
+        weblink = this._firstCodeBrowserWeblink(weblinks);
+      }
      if (!weblink) { return null; }
-      return weblink.url;
+      return weblink;
    },

-    _getChangeWeblinks({repo, commit, options: {weblinks}}) {
+    _getChangeWeblinks({repo, commit, options: {weblinks, config}}) {
      if (!weblinks || !weblinks.length) return [];
-      return weblinks.filter(weblink => !this._isDirectCommit(weblink)).map(
-          ({name, url}) => {
-            if (!url.startsWith('https:') && !url.startsWith('http:')) {
-              url = this.getBaseUrl() + (url.startsWith('/') ? '' : '/') + url;
-            }
-            return {name, url};
-          });
+      const commitWeblink = this._getBrowseCommitWeblink(weblinks, config);
+      return weblinks.filter(weblink =>
+        !commitWeblink ||
+        !commitWeblink.name ||
+        weblink.name !== commitWeblink.name);
    },

    _getFileWebLinks({repo, commit, file, options: {weblinks}}) {
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html
@@ -44,20 +44,46 @@ limitations under the License.

    teardown(() => { sandbox.restore(); });

-    test('_getChangeWeblinks', () => {
-      sandbox.stub(element, '_isDirectCommit').returns(false);
-      sandbox.stub(element, 'getBaseUrl').returns('base');
+    test('_firstCodeBrowserWeblink', () => {
+      assert.deepEqual(element._firstCodeBrowserWeblink([
+        {name: 'gitweb'},
+        {name: 'gitiles'},
+        {name: 'browse'},
+        {name: 'test'}]), {name: 'gitiles'});
+
+      assert.deepEqual(element._firstCodeBrowserWeblink([
+        {name: 'gitweb'},
+        {name: 'test'}]), {name: 'gitweb'});
+    });
+
+    test('_getBrowseCommitWeblink', () => {
+      const browserLink = {name: 'browser', url: 'browser/url'};
      const link = {name: 'test', url: 'test/url'};
-      const mapLinksToConfig = weblink => ({options: {weblinks: [weblink]}});
-      assert.deepEqual(element._getChangeWeblinks(mapLinksToConfig(link))[0],
-          {name: 'test', url: 'base/test/url'});
+      const weblinks = [browserLink, link];
+      const config = {gerrit: {primary_weblink_name: browserLink.name}};
+      sandbox.stub(element, '_firstCodeBrowserWeblink').returns(link);

-      link.url = '/' + link.url;
-      assert.deepEqual(element._getChangeWeblinks(mapLinksToConfig(link))[0],
-          {name: 'test', url: 'base/test/url'});
+      assert.deepEqual(element._getBrowseCommitWeblink(weblinks, config),
+          browserLink);

-      link.url = 'https:/' + link.url;
-      assert.deepEqual(element._getChangeWeblinks(mapLinksToConfig(link))[0],
+      assert.deepEqual(element._getBrowseCommitWeblink(weblinks, {}), link);
+    });
+
+    test('_getChangeWeblinks', () => {
+      const link = {name: 'test', url: 'test/url'};
+      const browserLink = {name: 'browser', url: 'browser/url'};
+      const mapLinksToConfig = weblinks => ({options: {weblinks}});
+      sandbox.stub(element, '_getBrowseCommitWeblink').returns(browserLink);
+
+      assert.deepEqual(
+          element._getChangeWeblinks(mapLinksToConfig([link, browserLink]))[0],
+          {name: 'test', url: 'test/url'});
+
+      assert.deepEqual(element._getChangeWeblinks(mapLinksToConfig([link]))[0],
+          {name: 'test', url: 'test/url'});
+
+      link.url = 'https://' + link.url;
+      assert.deepEqual(element._getChangeWeblinks(mapLinksToConfig([link]))[0],
          {name: 'test', url: 'https://test/url'});
    });

--- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html
+++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html
@@ -349,19 +349,20 @@ limitations under the License.
            </span>
          </section>
          <section>
-          <div class="pref">
-            <span class="title">Ignore Whitespace</span>
-            <span class="value">
-              <gr-select bind-value="{{_diffPrefs.ignore_whitespace}}">
-                <select>
-                  <option value="IGNORE_NONE">None</option>
-                  <option value="IGNORE_TRAILING">Trailing</option>
-                  <option value="IGNORE_LEADING_AND_TRAILING">Leading & trailing</option>
-                  <option value="IGNORE_ALL">All</option>
-                </select>
-              </gr-select>
-            </span>
-          </div>
+            <div class="pref">
+              <span class="title">Ignore Whitespace</span>
+              <span class="value">
+                <gr-select bind-value="{{_diffPrefs.ignore_whitespace}}">
+                  <select>
+                    <option value="IGNORE_NONE">None</option>
+                    <option value="IGNORE_TRAILING">Trailing</option>
+                    <option value="IGNORE_LEADING_AND_TRAILING">Leading & trailing</option>
+                    <option value="IGNORE_ALL">All</option>
+                  </select>
+                </gr-select>
+              </span>
+            </div>
+          </section>
          <gr-button
              id="saveDiffPrefs"
              on-tap="_handleSaveDiffPreferences"
--- a/resources/com/google/gerrit/pgm/init/gerrit.sh
+++ b/resources/com/google/gerrit/pgm/init/gerrit.sh
@@ -263,9 +263,9 @@ if test -z "$JAVA" \
 fi

 if test -z "$JAVA" ; then
-  echo >&2 "Cannot find a JRE or JDK. Please set JAVA_HOME or"
-  echo >&2 "container.javaHome in $GERRIT_SITE/etc/gerrit.config"
-  echo >&2 "to a >=1.7 JRE"
+  echo >&2 "Cannot find a JRE or JDK. Please ensure that the JAVA_HOME environment"
+  echo >&2 "variable or container.javaHome in $GERRIT_SITE/etc/gerrit.config is"
+  echo >&2 "set to a valid >=1.7 JRE location"
  exit 1
 fi