diff --git a/java/com/google/gerrit/extensions/client/ListOption.java b/java/com/google/gerrit/extensions/client/ListOption.java index e694c0eb6f..4dea42fe41 100644 --- a/java/com/google/gerrit/extensions/client/ListOption.java +++ b/java/com/google/gerrit/extensions/client/ListOption.java @@ -16,6 +16,7 @@ package com.google.gerrit.extensions.client; import java.lang.reflect.InvocationTargetException; import java.util.EnumSet; +import java.util.Set; /** Enum that can be expressed as a bitset in query parameters. */ public interface ListOption { @@ -46,4 +47,13 @@ public interface ListOption { } return r; } + + static String toHex(Set options) { + int v = 0; + for (ListChangesOption option : options) { + v |= 1 << option.getValue(); + } + + return Integer.toHexString(v); + } } diff --git a/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java b/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java index b1d4ac623e..6a66ba3ea6 100644 --- a/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java +++ b/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java @@ -18,15 +18,20 @@ import static com.google.template.soy.data.ordainers.GsonOrdainer.serializeObjec import com.google.common.base.Strings; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.common.flogger.FluentLogger; +import com.google.common.primitives.Ints; import com.google.gerrit.common.Nullable; import com.google.gerrit.common.UsedAt; import com.google.gerrit.common.UsedAt.Project; import com.google.gerrit.extensions.api.GerritApi; import com.google.gerrit.extensions.api.accounts.AccountApi; import com.google.gerrit.extensions.api.config.Server; +import com.google.gerrit.extensions.client.ListChangesOption; +import com.google.gerrit.extensions.client.ListOption; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.RestApiException; +import com.google.gerrit.extensions.restapi.Url; import com.google.gerrit.json.OutputFormat; import com.google.gson.Gson; import com.google.template.soy.data.SanitizedContent; @@ -34,11 +39,46 @@ import java.net.URI; import java.net.URISyntaxException; import java.util.HashMap; import java.util.Map; +import java.util.Set; import java.util.function.Function; +import java.util.regex.Matcher; +import java.util.regex.Pattern; /** Helper for generating parts of {@code index.html}. */ public class IndexHtmlUtil { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + static final String changeCanonicalUrl = ".*/c/(?.+)/\\+/(?\\d+)"; + static final String basePatchNumUrlPart = "(/(-?\\d+|edit)(\\.\\.(\\d+|edit))?)"; + static final Pattern changeUrlPattern = + Pattern.compile(changeCanonicalUrl + basePatchNumUrlPart + "?" + "/?$"); + static final Pattern diffUrlPattern = + Pattern.compile(changeCanonicalUrl + basePatchNumUrlPart + "(/(.+))" + "/?$"); + + public static String getDefaultChangeDetailHex() { + Set options = + ImmutableSet.of( + ListChangesOption.ALL_COMMITS, + ListChangesOption.ALL_REVISIONS, + ListChangesOption.CHANGE_ACTIONS, + ListChangesOption.DETAILED_LABELS, + ListChangesOption.DOWNLOAD_COMMANDS, + ListChangesOption.MESSAGES, + ListChangesOption.SUBMITTABLE, + ListChangesOption.WEB_LINKS, + ListChangesOption.SKIP_DIFFSTAT); + + return ListOption.toHex(options); + } + + public static String getDefaultDiffDetailHex() { + Set options = + ImmutableSet.of( + ListChangesOption.ALL_COMMITS, + ListChangesOption.ALL_REVISIONS, + ListChangesOption.SKIP_DIFFSTAT); + + return ListOption.toHex(options); + } /** * Returns both static and dynamic parameters of {@code index.html}. The result is to be used when @@ -50,12 +90,18 @@ public class IndexHtmlUtil { String cdnPath, String faviconPath, Map urlParameterMap, - Function urlInScriptTagOrdainer) + Function urlInScriptTagOrdainer, + String requestedURL) throws URISyntaxException, RestApiException { return ImmutableMap.builder() .putAll( staticTemplateData( - canonicalURL, cdnPath, faviconPath, urlParameterMap, urlInScriptTagOrdainer)) + canonicalURL, + cdnPath, + faviconPath, + urlParameterMap, + urlInScriptTagOrdainer, + requestedURL)) .putAll(dynamicTemplateData(gerritApi)) .build(); } @@ -98,7 +144,8 @@ public class IndexHtmlUtil { String cdnPath, String faviconPath, Map urlParameterMap, - Function urlInScriptTagOrdainer) + Function urlInScriptTagOrdainer, + String requestedURL) throws URISyntaxException { String canonicalPath = computeCanonicalPath(canonicalURL); @@ -133,9 +180,39 @@ public class IndexHtmlUtil { if (urlParameterMap.containsKey("gf")) { data.put("useGoogleFonts", "true"); } + + if (urlParameterMap.containsKey("pl") && requestedURL != null) { + data.put("defaultChangeDetailHex", getDefaultChangeDetailHex()); + data.put("defaultDiffDetailHex", getDefaultDiffDetailHex()); + + String changeRequestsPath = computeChangeRequestsPath(requestedURL, changeUrlPattern); + if (changeRequestsPath != null) { + data.put("preloadChangePage", "true"); + } else { + changeRequestsPath = computeChangeRequestsPath(requestedURL, diffUrlPattern); + data.put("preloadDiffPage", "true"); + } + + if (changeRequestsPath != null) { + data.put("changeRequestsPath", changeRequestsPath); + } + } + return data.build(); } + static String computeChangeRequestsPath(String requestedURL, Pattern pattern) { + Matcher matcher = pattern.matcher(requestedURL); + if (matcher.matches()) { + Integer changeId = Ints.tryParse(matcher.group("changeNum")); + if (changeId != null) { + return "changes/" + Url.encode(matcher.group("project")) + "~" + changeId; + } + } + + return null; + } + private static String computeCanonicalPath(@Nullable String canonicalURL) throws URISyntaxException { if (Strings.isNullOrEmpty(canonicalURL)) { diff --git a/java/com/google/gerrit/httpd/raw/IndexServlet.java b/java/com/google/gerrit/httpd/raw/IndexServlet.java index a0b41b2155..97d22701de 100644 --- a/java/com/google/gerrit/httpd/raw/IndexServlet.java +++ b/java/com/google/gerrit/httpd/raw/IndexServlet.java @@ -70,10 +70,11 @@ public class IndexServlet extends HttpServlet { SoySauce.Renderer renderer; try { Map parameterMap = req.getParameterMap(); + String requestUrl = req.getRequestURL() == null ? null : req.getRequestURL().toString(); // TODO(hiesel): Remove URL ordainer as parameter once Soy is consistent ImmutableMap templateData = IndexHtmlUtil.templateData( - gerritApi, canonicalUrl, cdnPath, faviconPath, parameterMap, urlOrdainer); + gerritApi, canonicalUrl, cdnPath, faviconPath, parameterMap, urlOrdainer, requestUrl); renderer = soySauce.renderTemplate("com.google.gerrit.httpd.raw.Index").setData(templateData); } catch (URISyntaxException | RestApiException e) { throw new IOException(e); diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java index ae5066bc58..0eefe02b32 100644 --- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java +++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java @@ -4637,7 +4637,6 @@ public class ChangeIT extends AbstractDaemonTest { ListChangesOption.ALL_COMMITS, ListChangesOption.ALL_REVISIONS, ListChangesOption.CHANGE_ACTIONS, - ListChangesOption.CURRENT_ACTIONS, ListChangesOption.DETAILED_LABELS, ListChangesOption.DOWNLOAD_COMMANDS, ListChangesOption.MESSAGES, diff --git a/javatests/com/google/gerrit/httpd/raw/IndexHtmlUtilTest.java b/javatests/com/google/gerrit/httpd/raw/IndexHtmlUtilTest.java index d9438f00e3..cad47961f2 100644 --- a/javatests/com/google/gerrit/httpd/raw/IndexHtmlUtilTest.java +++ b/javatests/com/google/gerrit/httpd/raw/IndexHtmlUtilTest.java @@ -15,6 +15,9 @@ package com.google.gerrit.httpd.raw; import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.httpd.raw.IndexHtmlUtil.changeUrlPattern; +import static com.google.gerrit.httpd.raw.IndexHtmlUtil.computeChangeRequestsPath; +import static com.google.gerrit.httpd.raw.IndexHtmlUtil.diffUrlPattern; import static com.google.gerrit.httpd.raw.IndexHtmlUtil.staticTemplateData; import com.google.template.soy.data.SanitizedContent; @@ -29,7 +32,12 @@ public class IndexHtmlUtilTest { public void noPathAndNoCDN() throws Exception { assertThat( staticTemplateData( - "http://example.com/", null, null, new HashMap<>(), IndexHtmlUtilTest::ordain)) + "http://example.com/", + null, + null, + new HashMap<>(), + IndexHtmlUtilTest::ordain, + null)) .containsExactly("canonicalPath", "", "staticResourcePath", ordain("")); } @@ -41,7 +49,8 @@ public class IndexHtmlUtilTest { null, null, new HashMap<>(), - IndexHtmlUtilTest::ordain)) + IndexHtmlUtilTest::ordain, + null)) .containsExactly("canonicalPath", "/gerrit", "staticResourcePath", ordain("/gerrit")); } @@ -53,7 +62,8 @@ public class IndexHtmlUtilTest { "http://my-cdn.com/foo/bar/", null, new HashMap<>(), - IndexHtmlUtilTest::ordain)) + IndexHtmlUtilTest::ordain, + null)) .containsExactly( "canonicalPath", "", "staticResourcePath", ordain("http://my-cdn.com/foo/bar/")); } @@ -66,7 +76,8 @@ public class IndexHtmlUtilTest { "http://my-cdn.com/foo/bar/", null, new HashMap<>(), - IndexHtmlUtilTest::ordain)) + IndexHtmlUtilTest::ordain, + null)) .containsExactly( "canonicalPath", "/gerrit", "staticResourcePath", ordain("http://my-cdn.com/foo/bar/")); } @@ -77,11 +88,51 @@ public class IndexHtmlUtilTest { urlParms.put("gf", new String[0]); assertThat( staticTemplateData( - "http://example.com/", null, null, urlParms, IndexHtmlUtilTest::ordain)) + "http://example.com/", null, null, urlParms, IndexHtmlUtilTest::ordain, null)) .containsExactly( "canonicalPath", "", "staticResourcePath", ordain(""), "useGoogleFonts", "true"); } + @Test + public void usePreloadRest() throws Exception { + Map urlParms = new HashMap<>(); + urlParms.put("pl", new String[0]); + assertThat( + staticTemplateData( + "http://example.com/", + null, + null, + urlParms, + IndexHtmlUtilTest::ordain, + "/c/project/+/123")) + .containsExactly( + "canonicalPath", "", + "staticResourcePath", ordain(""), + "defaultChangeDetailHex", "916314", + "defaultDiffDetailHex", "800014", + "preloadChangePage", "true", + "changeRequestsPath", "changes/project~123"); + } + + @Test + public void computeChangePath() throws Exception { + assertThat(computeChangeRequestsPath("/c/project/+/123", changeUrlPattern)) + .isEqualTo("changes/project~123"); + + assertThat(computeChangeRequestsPath("/c/project/+/124/2", changeUrlPattern)) + .isEqualTo("changes/project~124"); + + assertThat(computeChangeRequestsPath("/c/project/src/+/23", changeUrlPattern)) + .isEqualTo("changes/project%2Fsrc~23"); + + assertThat(computeChangeRequestsPath("/q/project/src/+/23", changeUrlPattern)).isEqualTo(null); + + assertThat(computeChangeRequestsPath("/c/Scripts/+/232/1//COMMIT_MSG", changeUrlPattern)) + .isEqualTo(null); + assertThat(computeChangeRequestsPath("/c/Scripts/+/232/1//COMMIT_MSG", diffUrlPattern)) + .isEqualTo("changes/Scripts~232"); + } + private static SanitizedContent ordain(String s) { return UnsafeSanitizedContentOrdainer.ordainAsSafe( s, SanitizedContent.ContentKind.TRUSTED_RESOURCE_URI); diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js index 4aefe46ae7..ce26bf63a3 100644 --- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js +++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js @@ -990,6 +990,20 @@ * @param {function()=} opt_cancelCondition */ getChangeDetail(changeNum, opt_errFn, opt_cancelCondition) { + return this.getConfig(false).then(config => { + const optionsHex = this._getChangeOptionsHex(config); + return this._getChangeDetail( + changeNum, optionsHex, opt_errFn, opt_cancelCondition) + .then(GrReviewerUpdatesParser.parse); + }); + } + + _getChangeOptionsHex(config) { + if (window.DEFAULT_DETAIL_HEXES && window.DEFAULT_DETAIL_HEXES.changePage + && !(config.receive && config.receive.enable_signed_push)) { + return window.DEFAULT_DETAIL_HEXES.changePage; + } + // This list MUST be kept in sync with // ChangeIT#changeDetailsDoesNotRequireIndex const options = [ @@ -1003,15 +1017,10 @@ this.ListChangesOption.WEB_LINKS, this.ListChangesOption.SKIP_DIFFSTAT, ]; - return this.getConfig(false).then(config => { - if (config.receive && config.receive.enable_signed_push) { - options.push(this.ListChangesOption.PUSH_CERTIFICATES); - } - const optionsHex = this.listChangesOptionsToHex(...options); - return this._getChangeDetail( - changeNum, optionsHex, opt_errFn, opt_cancelCondition) - .then(GrReviewerUpdatesParser.parse); - }); + if (config.receive && config.receive.enable_signed_push) { + options.push(this.ListChangesOption.PUSH_CERTIFICATES); + } + return this.listChangesOptionsToHex(...options); } /** @@ -1020,11 +1029,16 @@ * @param {function()=} opt_cancelCondition */ getDiffChangeDetail(changeNum, opt_errFn, opt_cancelCondition) { - const optionsHex = this.listChangesOptionsToHex( - this.ListChangesOption.ALL_COMMITS, - this.ListChangesOption.ALL_REVISIONS, - this.ListChangesOption.SKIP_DIFFSTAT - ); + let optionsHex = ''; + if (window.DEFAULT_DETAIL_HEXES && window.DEFAULT_DETAIL_HEXES.diffPage) { + optionsHex = window.DEFAULT_DETAIL_HEXES.diffPage; + } else { + optionsHex = this.listChangesOptionsToHex( + this.ListChangesOption.ALL_COMMITS, + this.ListChangesOption.ALL_REVISIONS, + this.ListChangesOption.SKIP_DIFFSTAT + ); + } return this._getChangeDetail(changeNum, optionsHex, opt_errFn, opt_cancelCondition); } @@ -1040,7 +1054,7 @@ const urlWithParams = this._restApiHelper .urlWithParams(url, optionsHex); const params = {O: optionsHex}; - let req = { + const req = { url, errFn: opt_errFn, cancelCondition: opt_cancelCondition, @@ -1048,7 +1062,6 @@ fetchOptions: this._etags.getOptions(urlWithParams), anonymizedUrl: '/changes/*~*/detail?O=' + optionsHex, }; - req = this._restApiHelper.addAcceptJsonHeader(req); return this._restApiHelper.fetchRawJSON(req).then(response => { if (response && response.status === 304) { return Promise.resolve(this._restApiHelper.parsePrefixedJSON( @@ -2039,12 +2052,15 @@ * @param {string|number=} opt_patchNum * @return {!Promise} Diff comments response. */ + // We don't want to add accept header, since preloading of comments is + // working only without accept header. + const noAcceptHeader = true; const fetchComments = opt_patchNum => this._getChangeURLAndFetch({ changeNum, endpoint, patchNum: opt_patchNum, reportEndpointAsIs: true, - }); + }, noAcceptHeader); if (!opt_basePatchNum && !opt_patchNum && !opt_path) { return fetchComments(); @@ -2609,7 +2625,7 @@ * @param {Gerrit.ChangeFetchRequest} req * @return {!Promise} */ - _getChangeURLAndFetch(req) { + _getChangeURLAndFetch(req, noAcceptHeader) { const anonymizedEndpoint = req.reportEndpointAsIs ? req.endpoint : req.anonymizedEndpoint; const anonymizedBaseUrl = req.patchNum ? @@ -2622,7 +2638,7 @@ fetchOptions: req.fetchOptions, anonymizedUrl: anonymizedEndpoint ? (anonymizedBaseUrl + anonymizedEndpoint) : undefined, - })); + }, noAcceptHeader)); } /** diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html index ca4b24672a..ed7d29f4bd 100644 --- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html +++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html @@ -1036,16 +1036,6 @@ limitations under the License. }); }); - test('_getChangeDetail accepts only json', () => { - const authFetchStub = sandbox.stub(element._auth, 'fetch') - .returns(Promise.resolve()); - const errFn = sinon.stub(); - element._getChangeDetail(123, '516714', errFn); - assert.isTrue(authFetchStub.called); - assert.equal(authFetchStub.lastCall.args[1].headers.get('Accept'), - 'application/json'); - }); - test('_getChangeDetail populates _projectLookup', () => { sandbox.stub(element, 'getChangeActionURL') .returns(Promise.resolve('')); diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.js index 91fef29afe..2c326df103 100644 --- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.js +++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.js @@ -204,9 +204,12 @@ * Same as {@link fetchRawJSON}, plus error handling. * * @param {Gerrit.FetchJSONRequest} req + * @param {boolean} noAcceptHeader - don't add default accept json header */ - fetchJSON(req) { - req = this.addAcceptJsonHeader(req); + fetchJSON(req, noAcceptHeader) { + if (!noAcceptHeader) { + req = this.addAcceptJsonHeader(req); + } return this.fetchRawJSON(req).then(response => { if (!response) { return; diff --git a/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy b/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy index 5046a2a846..b182309f03 100644 --- a/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy +++ b/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy @@ -28,6 +28,11 @@ {@param? polyfillSD: ?} {@param? polyfillSC: ?} {@param? useGoogleFonts: ?} + {@param? changeRequestsPath: ?} + {@param? defaultChangeDetailHex: ?} + {@param? defaultDiffDetailHex: ?} + {@param? preloadChangePage: ?} + {@param? preloadDiffPage: ?} {\n} {\n} {\n} @@ -43,6 +48,14 @@ // Disable extra font load from paper-styles window.polymerSkipLoadingFontRoboto = true; window.CLOSURE_NO_DEPS = true; + window.DEFAULT_DETAIL_HEXES = {lb} + {if $defaultChangeDetailHex} + changePage: '{$defaultChangeDetailHex}', + {/if} + {if $defaultDiffDetailHex} + diffPage: '{$defaultDiffDetailHex}', + {/if} + {rb}; {if $canonicalPath != ''}window.CANONICAL_PATH = '{$canonicalPath}';{/if} {if $versionInfo}window.VERSION_INFO = '{$versionInfo}';{/if} {if $staticResourcePath != ''}window.STATIC_RESOURCE_PATH = '{$staticResourcePath}';{/if} @@ -68,6 +81,16 @@ {else} {\n} {/if} + {if $changeRequestsPath} + {if $preloadChangePage and $defaultChangeDetailHex} + {\n} + {/if} + {if $preloadDiffPage and $defaultDiffDetailHex} + {\n} + {/if} + {\n} + {\n} + {/if} // RobotoMono fonts are used in styles/fonts.css {if $useGoogleFonts}