Merge "Add a flag for the new Lit based diff rendering"
diff --git a/java/com/google/gerrit/server/project/BranchResource.java b/java/com/google/gerrit/server/project/BranchResource.java
index f071fbe..fb1fa57 100644
--- a/java/com/google/gerrit/server/project/BranchResource.java
+++ b/java/com/google/gerrit/server/project/BranchResource.java
@@ -18,18 +18,20 @@
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.server.CurrentUser;
import com.google.inject.TypeLiteral;
+import java.util.Optional;
+import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
public class BranchResource extends RefResource {
public static final TypeLiteral<RestView<BranchResource>> BRANCH_KIND = new TypeLiteral<>() {};
private final String refName;
- private final String revision;
+ private final Optional<String> revision;
public BranchResource(ProjectState projectState, CurrentUser user, Ref ref) {
super(projectState, user);
this.refName = ref.getName();
- this.revision = ref.getObjectId() != null ? ref.getObjectId().name() : null;
+ this.revision = Optional.ofNullable(ref.getObjectId()).map(ObjectId::name);
}
public BranchNameKey getBranchKey() {
@@ -42,7 +44,7 @@
}
@Override
- public String getRevision() {
+ public Optional<String> getRevision() {
return revision;
}
}
diff --git a/java/com/google/gerrit/server/project/RefResource.java b/java/com/google/gerrit/server/project/RefResource.java
index fcf6048..c67e10d 100644
--- a/java/com/google/gerrit/server/project/RefResource.java
+++ b/java/com/google/gerrit/server/project/RefResource.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.project;
import com.google.gerrit.server.CurrentUser;
+import java.util.Optional;
public abstract class RefResource extends ProjectResource {
@@ -25,6 +26,10 @@
/** Returns the ref's name */
public abstract String getRef();
- /** Returns the ref's revision */
- public abstract String getRevision();
+ /**
+ * Returns the ref's revision.
+ *
+ * @return the ref's revision, {@link Optional#empty()} if the ref doesn't exist (yet)
+ */
+ public abstract Optional<String> getRevision();
}
diff --git a/java/com/google/gerrit/server/project/TagResource.java b/java/com/google/gerrit/server/project/TagResource.java
index c837557..d46b9ab 100644
--- a/java/com/google/gerrit/server/project/TagResource.java
+++ b/java/com/google/gerrit/server/project/TagResource.java
@@ -18,6 +18,7 @@
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.server.CurrentUser;
import com.google.inject.TypeLiteral;
+import java.util.Optional;
public class TagResource extends RefResource {
public static final TypeLiteral<RestView<TagResource>> TAG_KIND = new TypeLiteral<>() {};
@@ -39,7 +40,7 @@
}
@Override
- public String getRevision() {
- return tagInfo.revision;
+ public Optional<String> getRevision() {
+ return Optional.ofNullable(tagInfo.revision);
}
}
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index cb90251..c828d4d 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -571,7 +571,7 @@
}
@Operator
- public Predicate<ChangeData> status(String statusName) {
+ public Predicate<ChangeData> status(String statusName) throws QueryParseException {
if ("reviewed".equalsIgnoreCase(statusName)) {
return ChangePredicates.unreviewed();
}
diff --git a/java/com/google/gerrit/server/query/change/ChangeStatusPredicate.java b/java/com/google/gerrit/server/query/change/ChangeStatusPredicate.java
index 0721433..9aff0c5 100644
--- a/java/com/google/gerrit/server/query/change/ChangeStatusPredicate.java
+++ b/java/com/google/gerrit/server/query/change/ChangeStatusPredicate.java
@@ -21,6 +21,7 @@
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Change.Status;
import com.google.gerrit.index.query.Predicate;
+import com.google.gerrit.index.query.QueryParseException;
import com.google.gerrit.server.index.change.ChangeField;
import java.util.ArrayList;
import java.util.List;
@@ -75,7 +76,7 @@
return status.name().toLowerCase();
}
- public static Predicate<ChangeData> parse(String value) {
+ public static Predicate<ChangeData> parse(String value) throws QueryParseException {
String lower = value.toLowerCase();
NavigableMap<String, Predicate<ChangeData>> head = PREDICATES.tailMap(lower, true);
if (!head.isEmpty()) {
@@ -85,7 +86,7 @@
return e.getValue();
}
}
- return NONE;
+ throw new QueryParseException("Unrecognized value: " + value);
}
public static Predicate<ChangeData> open() {
diff --git a/java/com/google/gerrit/server/restapi/project/FilesCollection.java b/java/com/google/gerrit/server/restapi/project/FilesCollection.java
index 888ecf2..ca5284f 100644
--- a/java/com/google/gerrit/server/restapi/project/FilesCollection.java
+++ b/java/com/google/gerrit/server/restapi/project/FilesCollection.java
@@ -46,8 +46,14 @@
@Override
public FileResource parse(BranchResource parent, IdString id)
throws ResourceNotFoundException, IOException {
+ if (parent.getRevision().isEmpty()) {
+ throw new ResourceNotFoundException(id);
+ }
return FileResource.create(
- repoManager, parent.getProjectState(), ObjectId.fromString(parent.getRevision()), id.get());
+ repoManager,
+ parent.getProjectState(),
+ ObjectId.fromString(parent.getRevision().get()),
+ id.get());
}
@Override
diff --git a/javatests/com/google/gerrit/acceptance/api/change/PluginOperatorsIT.java b/javatests/com/google/gerrit/acceptance/api/change/PluginOperatorsIT.java
index f4cf96d..3ffbda1 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/PluginOperatorsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/PluginOperatorsIT.java
@@ -15,6 +15,7 @@
package com.google.gerrit.acceptance.api.change;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.extensions.annotations.Exports;
@@ -46,7 +47,11 @@
String oddChangeId = createChange().getChangeId();
String evenChangeId = createChange().getChangeId();
- assertThat(getChanges(queryChanges)).hasSize(0);
+ BadRequestException exception =
+ assertThrows(BadRequestException.class, () -> getChanges(queryChanges));
+ assertThat(exception)
+ .hasMessageThat()
+ .isEqualTo("Unrecognized value: changeNumberEven_myplugin");
try (AutoCloseable ignored = installPlugin("myplugin", IsOperatorModule.class)) {
List<?> changes = getChanges(queryChanges);
@@ -57,8 +62,6 @@
assertThat(outputChangeId).isEqualTo(evenChangeId);
assertThat(outputChangeId).isNotEqualTo(oddChangeId);
}
-
- assertThat(getChanges(queryChanges)).hasSize(0);
}
protected static class IsOperatorModule extends AbstractModule {
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/FileBranchIT.java b/javatests/com/google/gerrit/acceptance/rest/project/FileBranchIT.java
index a7f3174..8dce9c3 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/FileBranchIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/FileBranchIT.java
@@ -18,16 +18,17 @@
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import com.google.gerrit.acceptance.AbstractDaemonTest;
-import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.extensions.api.projects.BranchApi;
import com.google.gerrit.extensions.restapi.BinaryResult;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.Repository;
import org.junit.Before;
import org.junit.Test;
-@NoHttpd
public class FileBranchIT extends AbstractDaemonTest {
private BranchNameKey branch;
@@ -47,6 +48,24 @@
}
@Test
+ public void getFileFromNonExistingBranch() throws Exception {
+ RestResponse response =
+ adminRestSession.get(
+ String.format("/projects/%s/branches/non-existing/files/path", project.get()));
+ response.assertNotFound();
+ }
+
+ @Test
+ public void getFileFromSymbolicRefPointingToAnUnbornBranch() throws Exception {
+ try (Repository repo = repoManager.openRepository(project)) {
+ repo.updateRef(Constants.HEAD, true).link("refs/heads/non-existing");
+ }
+ RestResponse response =
+ adminRestSession.get(String.format("/projects/%s/branches/HEAD/files/path", project.get()));
+ response.assertNotFound();
+ }
+
+ @Test
public void getNonExistingFile() throws Exception {
assertThrows(ResourceNotFoundException.class, () -> branch().file("does-not-exist"));
}
diff --git a/javatests/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java b/javatests/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java
index e5b2ffb..3a47ad8 100644
--- a/javatests/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java
+++ b/javatests/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java
@@ -207,12 +207,10 @@
assertThat(status("file:a")).isEqualTo(all);
assertThat(status("is:new")).containsExactly(NEW);
assertThat(status("is:new OR is:merged")).containsExactly(NEW, MERGED);
- assertThat(status("is:new OR is:x")).isEqualTo(all);
assertThat(status("is:new is:merged")).isEmpty();
assertThat(status("(is:new) (is:merged)")).isEmpty();
assertThat(status("(is:new) (is:merged)")).isEmpty();
- assertThat(status("is:new is:x")).containsExactly(NEW);
}
@Test
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 902d620..e916147 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -347,8 +347,10 @@
assertQuery("is:new", change1);
assertQuery("status:merged", change2);
assertQuery("is:merged", change2);
- assertQuery("status:draft");
- assertQuery("is:draft");
+ Exception thrown = assertThrows(BadRequestException.class, () -> assertQuery("is:draft"));
+ assertThat(thrown).hasMessageThat().isEqualTo("Unrecognized value: draft");
+ thrown = assertThrows(BadRequestException.class, () -> assertQuery("status:draft"));
+ assertThat(thrown).hasMessageThat().isEqualTo("Unrecognized value: draft");
}
@Test
@@ -421,8 +423,10 @@
assertQuery("status:N", change1);
assertQuery("status:nE", change1);
assertQuery("status:neW", change1);
- assertQuery("status:nx");
- assertQuery("status:newx");
+ Exception thrown = assertThrows(BadRequestException.class, () -> assertQuery("status:newx"));
+ assertThat(thrown).hasMessageThat().isEqualTo("Unrecognized value: newx");
+ thrown = assertThrows(BadRequestException.class, () -> assertQuery("status:nx"));
+ assertThat(thrown).hasMessageThat().isEqualTo("Unrecognized value: nx");
}
@Test
