Merge branch 'stable-3.9' into stable-3.10 * stable-3.9: Manage change number prefixed URL to project/+/changeNum Release-Notes: skip Change-Id: I21ca67f2027f974a142fc4e91fa4551a5db607b5
diff --git a/java/com/google/gerrit/acceptance/GerritServerRestSession.java b/java/com/google/gerrit/acceptance/GerritServerRestSession.java index c2c77fe..9605f50 100644 --- a/java/com/google/gerrit/acceptance/GerritServerRestSession.java +++ b/java/com/google/gerrit/acceptance/GerritServerRestSession.java
@@ -142,7 +142,8 @@ return execute(delete); } - private String getUrl(String endPoint) { + @Override + public String getUrl(String endPoint) { return url + (account != null ? "/a" : "") + endPoint; } }
diff --git a/java/com/google/gerrit/acceptance/RestResponse.java b/java/com/google/gerrit/acceptance/RestResponse.java index a9c14aa..a53c015 100644 --- a/java/com/google/gerrit/acceptance/RestResponse.java +++ b/java/com/google/gerrit/acceptance/RestResponse.java
@@ -103,4 +103,9 @@ assertStatus(SC_MOVED_TEMPORARILY); assertThat(URI.create(getHeader("Location")).getPath()).isEqualTo(path); } + + public void assertTemporaryRedirectUri(String uri) throws Exception { + assertStatus(SC_MOVED_TEMPORARILY); + assertThat(getHeader("Location")).isEqualTo(uri); + } }
diff --git a/java/com/google/gerrit/acceptance/RestSession.java b/java/com/google/gerrit/acceptance/RestSession.java index 0865e31..3fefd5b 100644 --- a/java/com/google/gerrit/acceptance/RestSession.java +++ b/java/com/google/gerrit/acceptance/RestSession.java
@@ -52,4 +52,6 @@ RestResponse delete(String endPoint) throws Exception; RestResponse deleteWithHeaders(String endPoint, Header... headers) throws Exception; + + String getUrl(String endPoint); }
diff --git a/java/com/google/gerrit/httpd/NumericChangeIdRedirectServlet.java b/java/com/google/gerrit/httpd/NumericChangeIdRedirectServlet.java index 2c80c9b..ca937fd 100644 --- a/java/com/google/gerrit/httpd/NumericChangeIdRedirectServlet.java +++ b/java/com/google/gerrit/httpd/NumericChangeIdRedirectServlet.java
@@ -16,6 +16,7 @@ import static com.google.common.collect.ImmutableList.toImmutableList; +import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; import com.google.gerrit.common.PageLinks; import com.google.gerrit.entities.Change; @@ -85,6 +86,7 @@ if (finalSegment != null) { path += finalSegment; } - UrlModule.toGerrit(path, req, rsp); + String queryString = Strings.emptyToNull(req.getQueryString()); + UrlModule.toGerrit(path + (queryString != null ? "?" + queryString : ""), req, rsp); } }
diff --git a/java/com/google/gerrit/httpd/UrlModule.java b/java/com/google/gerrit/httpd/UrlModule.java index aedf8e9..f62fbe8 100644 --- a/java/com/google/gerrit/httpd/UrlModule.java +++ b/java/com/google/gerrit/httpd/UrlModule.java
@@ -45,6 +45,9 @@ class UrlModule extends ServletModule { private final AuthConfig authConfig; + private static final String CHANGE_NUMBER_REGEX = "(?:/c)?/([1-9][0-9]*)"; + private static final String PATCH_SET_REGEX = "([1-9][0-9]*(\\.\\.[1-9][0-9]*)?)"; + UrlModule(AuthConfig authConfig) { this.authConfig = authConfig; } @@ -72,7 +75,12 @@ serveRegex("^/settings/?$").with(screen(PageLinks.SETTINGS)); serveRegex("^/register$").with(registerScreen(false)); serveRegex("^/register/(.+)$").with(registerScreen(true)); - serveRegex("^(?:/c)?/([1-9][0-9]*)/?.*$").with(NumericChangeIdRedirectServlet.class); + serveRegex("^" + CHANGE_NUMBER_REGEX + "(/" + PATCH_SET_REGEX + ")?/?$") + .with(NumericChangeIdRedirectServlet.class); + serveRegex("^" + CHANGE_NUMBER_REGEX + "/" + PATCH_SET_REGEX + "?/[^+]+$") + .with(NumericChangeIdRedirectServlet.class); + serveRegex("^" + CHANGE_NUMBER_REGEX + "/comment/\\w+/?$") + .with(NumericChangeIdRedirectServlet.class); serveRegex("^/p/(.*)$").with(queryProjectNew()); serveRegex("^/r/(.+)/?$").with(DirectChangeByCommit.class);
diff --git a/java/com/google/gerrit/httpd/raw/StaticModule.java b/java/com/google/gerrit/httpd/raw/StaticModule.java index b00294f..42e10d8 100644 --- a/java/com/google/gerrit/httpd/raw/StaticModule.java +++ b/java/com/google/gerrit/httpd/raw/StaticModule.java
@@ -63,7 +63,17 @@ public class StaticModule extends ServletModule { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - public static final String CHANGE_NUMBER_URI_REGEX = "^(?:/c)?/([1-9][0-9]*)/?.*"; + // This constant is copied and NOT reused from UrlModule because of the need for + // StaticModule and UrlModule to be used in isolation. The requirement comes + // from the way Google includes these two classes in their setup. + private static final String CHANGE_NUMBER_REGEX = "(?:/c)?/([1-9][0-9]*)"; + // Regex matching the direct links to comments using only the change number + // 1234/comment/abc_def + public static final String CHANGE_NUMBER_URI_REGEX = + "^" + + CHANGE_NUMBER_REGEX + + "(/[1-9][0-9]*(\\.\\.[1-9][0-9]*)?(/[^+]*)?)?(/comment/[^+]+)?/?$"; + private static final Pattern CHANGE_NUMBER_URI_PATTERN = Pattern.compile(CHANGE_NUMBER_URI_REGEX); /**
diff --git a/javatests/com/google/gerrit/acceptance/rest/RestApiServletIT.java b/javatests/com/google/gerrit/acceptance/rest/RestApiServletIT.java index 8011fb0..a54c7ca 100644 --- a/javatests/com/google/gerrit/acceptance/rest/RestApiServletIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/RestApiServletIT.java
@@ -454,12 +454,21 @@ ChangeData changeData = createChange().getChange(); int changeNumber = changeData.getId().get(); - String finalSegment = "any/Thing"; + assertChangeNumberWithSuffixRedirected(changeNumber, "1..2"); + assertChangeNumberWithSuffixRedirected(changeNumber, "2"); + assertChangeNumberWithSuffixRedirected(changeNumber, "2/COMMIT_MSG"); + assertChangeNumberWithSuffixRedirected(changeNumber, "2?foo=bar"); + assertChangeNumberWithSuffixRedirected(changeNumber, "2/path/to/source/file/MyClass.java"); + } - String redirectUri = String.format("/c/%s/+/%d/%s", project.get(), changeNumber, finalSegment); + private void assertChangeNumberWithSuffixRedirected(int changeNumber, String suffix) + throws Exception { + String redirectUri = + anonymousRestSession.getUrl( + String.format("/c/%s/+/%d/%s", project.get(), changeNumber, suffix)); anonymousRestSession - .get(String.format("/c/%d/%s", changeNumber, finalSegment)) - .assertTemporaryRedirect(redirectUri); + .get(String.format("/c/%d/%s", changeNumber, suffix)) + .assertTemporaryRedirectUri(redirectUri); } @Test @@ -467,12 +476,12 @@ int changeNumber = createChange().getChange().getId().get(); String commentId = "ff3303fd_8341647b"; - String redirectUri = + String redirectPath = String.format("/c/%s/+/%d/comment/%s", project.get(), changeNumber, commentId); anonymousRestSession .get(String.format("/%s/comment/%s", changeNumber, commentId)) - .assertTemporaryRedirect(redirectUri); + .assertTemporaryRedirect(redirectPath); } @Test
diff --git a/javatests/com/google/gerrit/httpd/raw/StaticModuleTest.java b/javatests/com/google/gerrit/httpd/raw/StaticModuleTest.java index 28ec30d..e973a26 100644 --- a/javatests/com/google/gerrit/httpd/raw/StaticModuleTest.java +++ b/javatests/com/google/gerrit/httpd/raw/StaticModuleTest.java
@@ -15,19 +15,33 @@ package com.google.gerrit.httpd.raw; import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.httpd.raw.StaticModule.PolyGerritFilter.isPolyGerritIndex; -import com.google.common.collect.ImmutableList; import org.junit.Test; public class StaticModuleTest { @Test public void doNotMatchPolyGerritIndex() { - ImmutableList.of( - "/c/123456/anyString", - "/123456/anyString", - "/c/123456/comment/9ab75172_67d798e1", - "/123456/comment/9ab75172_67d798e1") - .forEach(url -> assertThat(StaticModule.PolyGerritFilter.isPolyGerritIndex(url)).isFalse()); + assertThat(isPolyGerritIndex("/123456")).isFalse(); + assertThat(isPolyGerritIndex("/123456/")).isFalse(); + assertThat(isPolyGerritIndex("/123456/1")).isFalse(); + assertThat(isPolyGerritIndex("/123456/1/")).isFalse(); + assertThat(isPolyGerritIndex("/c/123456/comment/9ab75172_67d798e1")).isFalse(); + assertThat(isPolyGerritIndex("/123456/comment/9ab75172_67d798e1")).isFalse(); + assertThat(isPolyGerritIndex("/123456/comment/9ab75172_67d798e1/")).isFalse(); + assertThat(isPolyGerritIndex("/123456/1..2")).isFalse(); + assertThat(isPolyGerritIndex("/c/123456/1..2")).isFalse(); + assertThat(isPolyGerritIndex("/c/2/1/COMMIT_MSG")).isFalse(); + assertThat(isPolyGerritIndex("/c/2/1/path/to/source/file/MyClass.java")).isFalse(); + } + + @Test + public void matchPolyGerritIndex() { + assertThat(isPolyGerritIndex("/c/test/+/123456/anyString")).isTrue(); + assertThat(isPolyGerritIndex("/c/test/+/123456/comment/9ab75172_67d798e1")).isTrue(); + assertThat(isPolyGerritIndex("/c/321/+/123456/anyString")).isTrue(); + assertThat(isPolyGerritIndex("/c/321/+/123456/comment/9ab75172_67d798e1")).isTrue(); + assertThat(isPolyGerritIndex("/c/321/anyString")).isTrue(); } }