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();
}
}