Merge changes Ia439a16c,I5d4af103
* changes:
Support > and < in LabelPredicate, with refactorings
Update index tests
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
index af9a07a..338e9c6 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
@@ -42,20 +42,17 @@
private final Account.Id account;
private final AccountGroup.UUID group;
- EqualsLabelPredicate(ProjectCache projectCache,
- ChangeControl.GenericFactory ccFactory,
- IdentifiedUser.GenericFactory userFactory, Provider<ReviewDb> dbProvider,
- String label, int expVal, Account.Id account,
- AccountGroup.UUID group) {
+ EqualsLabelPredicate(LabelPredicate.Args args, String label, int expVal,
+ Account.Id account) {
super(ChangeField.LABEL, ChangeField.formatLabel(label, expVal, account));
- this.ccFactory = ccFactory;
- this.projectCache = projectCache;
- this.userFactory = userFactory;
- this.dbProvider = dbProvider;
+ this.ccFactory = args.ccFactory;
+ this.projectCache = args.projectCache;
+ this.userFactory = args.userFactory;
+ this.dbProvider = args.dbProvider;
+ this.group = args.group;
this.label = label;
this.expVal = expVal;
this.account = account;
- this.group = group;
}
@Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/LabelPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/LabelPredicate.java
index 4674cf4..60f7ffa 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/LabelPredicate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/LabelPredicate.java
@@ -34,30 +34,42 @@
public class LabelPredicate extends OrPredicate<ChangeData> {
private static final int MAX_LABEL_VALUE = 4;
- private static enum Test {
- EQ, GT_EQ, LT_EQ;
+ static class Args {
+ final ProjectCache projectCache;
+ final ChangeControl.GenericFactory ccFactory;
+ final IdentifiedUser.GenericFactory userFactory;
+ final Provider<ReviewDb> dbProvider;
+ final String value;
+ final Set<Account.Id> accounts;
+ final AccountGroup.UUID group;
- boolean isEq() {
- return EQ.equals(this);
+ private Args(
+ ProjectCache projectCache,
+ ChangeControl.GenericFactory ccFactory,
+ IdentifiedUser.GenericFactory userFactory,
+ Provider<ReviewDb> dbProvider,
+ String value,
+ Set<Account.Id> accounts,
+ AccountGroup.UUID group) {
+ this.projectCache = projectCache;
+ this.ccFactory = ccFactory;
+ this.userFactory = userFactory;
+ this.dbProvider = dbProvider;
+ this.value = value;
+ this.accounts = accounts;
+ this.group = group;
}
+ }
- boolean isGtEq() {
- return GT_EQ.equals(this);
- }
+ private static class Parsed {
+ private final String label;
+ private final String test;
+ private final int expVal;
- static Test op(String op) {
- if ("=".equals(op)) {
- return EQ;
-
- } else if (">=".equals(op)) {
- return GT_EQ;
-
- } else if ("<=".equals(op)) {
- return LT_EQ;
-
- } else {
- throw new IllegalArgumentException("Unsupported operation " + op);
- }
+ private Parsed(String label, String test, int expVal) {
+ this.label = label;
+ this.test = test;
+ this.expVal = expVal;
}
}
@@ -67,73 +79,79 @@
ChangeControl.GenericFactory ccFactory,
IdentifiedUser.GenericFactory userFactory, Provider<ReviewDb> dbProvider,
String value, Set<Account.Id> accounts, AccountGroup.UUID group) {
- super(predicates(projectCache, ccFactory, userFactory, dbProvider, value,
- accounts, group));
+ super(predicates(new Args(projectCache, ccFactory, userFactory, dbProvider,
+ value, accounts, group)));
this.value = value;
}
- private static List<Predicate<ChangeData>> predicates(
- ProjectCache projectCache, ChangeControl.GenericFactory ccFactory,
- IdentifiedUser.GenericFactory userFactory, Provider<ReviewDb> dbProvider,
- String value, Set<Account.Id> accounts, AccountGroup.UUID group) {
- String label = null;
- Test test = null;
- int expVal = 0;
+ private static List<Predicate<ChangeData>> predicates(Args args) {
+ String v = args.value;
+ Parsed parsed = null;
try {
- LabelVote v = LabelVote.parse(value);
- test = Test.EQ;
- label = v.getLabel();
- expVal = v.getValue();
+ LabelVote lv = LabelVote.parse(v);
+ parsed = new Parsed(lv.getLabel(), "=", lv.getValue());
} catch (IllegalArgumentException e) {
// Try next format.
}
try {
- LabelVote v = LabelVote.parseWithEquals(value);
- test = Test.EQ;
- label = v.getLabel();
- expVal = v.getValue();
+ LabelVote lv = LabelVote.parseWithEquals(v);
+ parsed = new Parsed(lv.getLabel(), "=", lv.getValue());
} catch (IllegalArgumentException e) {
// Try next format.
}
- if (label == null) {
- Matcher m = Pattern.compile("(>=|<=)([+-]?\\d+)$").matcher(value);
+ if (parsed == null) {
+ Matcher m = Pattern.compile("(>|>=|=|<|<=)([+-]?\\d+)$").matcher(v);
if (m.find()) {
- label = value.substring(0, m.start());
- test = Test.op(m.group(1));
- expVal = value(m.group(2));
+ parsed = new Parsed(v.substring(0, m.start()), m.group(1),
+ value(m.group(2)));
} else {
- label = value;
- test = Test.EQ;
- expVal = 1;
+ parsed = new Parsed(v, "=", 1);
}
}
- List<Predicate<ChangeData>> r = Lists.newArrayListWithCapacity(2 * MAX_LABEL_VALUE);
- if (test.isEq()) {
- if (expVal != 0) {
- r.add(equalsLabelPredicate(projectCache, ccFactory, userFactory,
- dbProvider, label, expVal, accounts, group));
- } else {
- r.add(noLabelQuery(projectCache, ccFactory, userFactory,
- dbProvider, label, accounts, group));
- }
- } else {
- for (int i = test.isGtEq() ? expVal : neg(expVal); i <= MAX_LABEL_VALUE; i++) {
- if (i != 0) {
- r.add(equalsLabelPredicate(projectCache, ccFactory, userFactory,
- dbProvider, label, test.isGtEq() ? i : neg(i), accounts, group));
- } else {
- r.add(noLabelQuery(projectCache, ccFactory, userFactory,
- dbProvider, label, accounts, group));
- }
- }
+ int min, max;
+ switch (parsed.test) {
+ case "=":
+ default:
+ min = max = parsed.expVal;
+ break;
+ case ">":
+ min = parsed.expVal + 1;
+ max = MAX_LABEL_VALUE;
+ break;
+ case ">=":
+ min = parsed.expVal;
+ max = MAX_LABEL_VALUE;
+ break;
+ case "<":
+ min = -MAX_LABEL_VALUE;
+ max = parsed.expVal - 1;
+ break;
+ case "<=":
+ min = -MAX_LABEL_VALUE;
+ max = parsed.expVal;
+ break;
+ }
+ List<Predicate<ChangeData>> r =
+ Lists.newArrayListWithCapacity(max - min + 1);
+ for (int i = min; i <= max; i++) {
+ r.add(onePredicate(args, parsed.label, i));
}
return r;
}
+ private static Predicate<ChangeData> onePredicate(Args args, String label,
+ int expVal) {
+ if (expVal != 0) {
+ return equalsLabelPredicate(args, label, expVal);
+ } else {
+ return noLabelQuery(args, label);
+ }
+ }
+
private static int value(String value) {
if (value.startsWith("+")) {
value = value.substring(1);
@@ -141,38 +159,24 @@
return Integer.parseInt(value);
}
- private static int neg(int value) {
- return -1 * value;
- }
-
- private static Predicate<ChangeData> noLabelQuery(ProjectCache projectCache,
- ChangeControl.GenericFactory ccFactory,
- IdentifiedUser.GenericFactory userFactory, Provider<ReviewDb> dbProvider,
- String label, Set<Account.Id> accounts, AccountGroup.UUID group) {
+ private static Predicate<ChangeData> noLabelQuery(Args args, String label) {
List<Predicate<ChangeData>> r =
Lists.newArrayListWithCapacity(2 * MAX_LABEL_VALUE);
for (int i = 1; i <= MAX_LABEL_VALUE; i++) {
- r.add(not(equalsLabelPredicate(projectCache, ccFactory, userFactory,
- dbProvider, label, i, accounts, group)));
- r.add(not(equalsLabelPredicate(projectCache, ccFactory, userFactory,
- dbProvider, label, neg(i), accounts, group)));
+ r.add(not(equalsLabelPredicate(args, label, i)));
+ r.add(not(equalsLabelPredicate(args, label, -i)));
}
return and(r);
}
- private static Predicate<ChangeData> equalsLabelPredicate(
- ProjectCache projectCache, ChangeControl.GenericFactory ccFactory,
- IdentifiedUser.GenericFactory userFactory, Provider<ReviewDb> dbProvider,
- String label, int expVal, Set<Account.Id> accounts,
- AccountGroup.UUID group) {
- if (accounts == null || accounts.isEmpty()) {
- return new EqualsLabelPredicate(projectCache, ccFactory, userFactory,
- dbProvider, label, expVal, null, group);
+ private static Predicate<ChangeData> equalsLabelPredicate(Args args,
+ String label, int expVal) {
+ if (args.accounts == null || args.accounts.isEmpty()) {
+ return new EqualsLabelPredicate(args, label, expVal, null);
} else {
List<Predicate<ChangeData>> r = Lists.newArrayList();
- for (Account.Id a : accounts) {
- r.add(new EqualsLabelPredicate(projectCache, ccFactory, userFactory,
- dbProvider, label, expVal, a, group));
+ for (Account.Id a : args.accounts) {
+ r.add(new EqualsLabelPredicate(args, label, expVal, a));
}
return or(r);
}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractIndexQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractIndexQueryChangesTest.java
deleted file mode 100644
index 5099d10..0000000
--- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractIndexQueryChangesTest.java
+++ /dev/null
@@ -1,87 +0,0 @@
-// Copyright (C) 2013 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.server.query.change;
-
-import static org.junit.Assert.assertTrue;
-
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
-import com.google.gerrit.extensions.api.changes.ReviewInput;
-import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.server.change.ChangeInserter;
-import com.google.gerrit.server.change.ChangeResource;
-import com.google.gerrit.server.change.RevisionResource;
-import com.google.gerrit.server.project.ChangeControl;
-
-import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
-import org.eclipse.jgit.junit.TestRepository;
-import org.eclipse.jgit.revwalk.RevCommit;
-import org.junit.Ignore;
-import org.junit.Test;
-
-import java.util.List;
-
-@Ignore
-public abstract class AbstractIndexQueryChangesTest
- extends AbstractQueryChangesTest {
- @Test
- public void byFileExact() throws Exception {
- TestRepository<InMemoryRepository> repo = createProject("repo");
- RevCommit commit = repo.parseBody(
- repo.commit().message("one")
- .add("file1", "contents1").add("file2", "contents2")
- .create());
- Change change = newChange(repo, commit, null, null, null).insert();
-
- assertTrue(query("file:file").isEmpty());
- assertResultEquals(change, queryOne("file:file1"));
- assertResultEquals(change, queryOne("file:file2"));
- }
-
- @Test
- public void byFileRegex() throws Exception {
- TestRepository<InMemoryRepository> repo = createProject("repo");
- RevCommit commit = repo.parseBody(
- repo.commit().message("one")
- .add("file1", "contents1").add("file2", "contents2")
- .create());
- Change change = newChange(repo, commit, null, null, null).insert();
-
- assertTrue(query("file:file.*").isEmpty());
- assertResultEquals(change, queryOne("file:^file.*"));
- }
-
- @Test
- public void byComment() throws Exception {
- TestRepository<InMemoryRepository> repo = createProject("repo");
- ChangeInserter ins = newChange(repo, null, null, null, null);
- Change change = ins.insert();
- ChangeControl ctl = changeControlFactory.controlFor(change, user);
-
- ReviewInput input = new ReviewInput();
- input.message = "toplevel";
- ReviewInput.Comment comment = new ReviewInput.Comment();
- comment.line = 1;
- comment.message = "inline";
- input.comments = ImmutableMap.<String, List<ReviewInput.Comment>> of(
- "Foo.java", ImmutableList.<ReviewInput.Comment> of(comment));
- postReview.apply(new RevisionResource(
- new ChangeResource(ctl), ins.getPatchSet()), input);
-
- assertTrue(query("comment:foo").isEmpty());
- assertResultEquals(change, queryOne("comment:toplevel"));
- assertResultEquals(change, queryOne("comment:inline"));
- }
-}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 2edb955..14a8637 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -22,6 +22,7 @@
import com.google.common.base.Charsets;
import com.google.common.base.Objects;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists;
import com.google.common.hash.Hashing;
@@ -378,19 +379,17 @@
assertTrue(query("label:Code-Review=2").isEmpty());
assertTrue(query("label:Code-Review+2").isEmpty());
- // TODO(dborowitz): > and < are broken at head.
-
assertResultEquals(change, queryOne("label:Code-Review>=0"));
- //assertResultEquals(change, queryOne("label:Code-Review>0"));
+ assertResultEquals(change, queryOne("label:Code-Review>0"));
assertResultEquals(change, queryOne("label:Code-Review>=1"));
- //assertTrue(query("label:Code-Review>1").isEmpty());
+ assertTrue(query("label:Code-Review>1").isEmpty());
assertTrue(query("label:Code-Review>=2").isEmpty());
assertResultEquals(change, queryOne("label: Code-Review<=2"));
- //assertResultEquals(change, queryOne("label: Code-Review<2"));
+ assertResultEquals(change, queryOne("label: Code-Review<2"));
assertResultEquals(change, queryOne("label: Code-Review<=1"));
- //assertTrue(query("label: Code-Review<1").isEmpty());
- assertTrue(query("label: Code-Review<=0").isEmpty());
+ assertTrue(query("label:Code-Review<1").isEmpty());
+ assertTrue(query("label:Code-Review<=0").isEmpty());
assertTrue(query("label:Code-Review=+1,anotheruser").isEmpty());
assertResultEquals(change, queryOne("label:Code-Review=+1,user"));
@@ -569,6 +568,55 @@
assertTrue(query("status:new ownerin:Administrators limit:2").isEmpty());
}
+ @Test
+ public void byFileExact() throws Exception {
+ TestRepository<InMemoryRepository> repo = createProject("repo");
+ RevCommit commit = repo.parseBody(
+ repo.commit().message("one")
+ .add("file1", "contents1").add("file2", "contents2")
+ .create());
+ Change change = newChange(repo, commit, null, null, null).insert();
+
+ assertTrue(query("file:file").isEmpty());
+ assertResultEquals(change, queryOne("file:file1"));
+ assertResultEquals(change, queryOne("file:file2"));
+ }
+
+ @Test
+ public void byFileRegex() throws Exception {
+ TestRepository<InMemoryRepository> repo = createProject("repo");
+ RevCommit commit = repo.parseBody(
+ repo.commit().message("one")
+ .add("file1", "contents1").add("file2", "contents2")
+ .create());
+ Change change = newChange(repo, commit, null, null, null).insert();
+
+ assertTrue(query("file:file.*").isEmpty());
+ assertResultEquals(change, queryOne("file:^file.*"));
+ }
+
+ @Test
+ public void byComment() throws Exception {
+ TestRepository<InMemoryRepository> repo = createProject("repo");
+ ChangeInserter ins = newChange(repo, null, null, null, null);
+ Change change = ins.insert();
+ ChangeControl ctl = changeControlFactory.controlFor(change, user);
+
+ ReviewInput input = new ReviewInput();
+ input.message = "toplevel";
+ ReviewInput.Comment comment = new ReviewInput.Comment();
+ comment.line = 1;
+ comment.message = "inline";
+ input.comments = ImmutableMap.<String, List<ReviewInput.Comment>> of(
+ "Foo.java", ImmutableList.<ReviewInput.Comment> of(comment));
+ postReview.apply(new RevisionResource(
+ new ChangeResource(ctl), ins.getPatchSet()), input);
+
+ assertTrue(query("comment:foo").isEmpty());
+ assertResultEquals(change, queryOne("comment:toplevel"));
+ assertResultEquals(change, queryOne("comment:inline"));
+ }
+
protected ChangeInserter newChange(
TestRepository<InMemoryRepository> repo,
@Nullable RevCommit commit, @Nullable String key, @Nullable Integer owner,
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java
index ab12986..370dd9d 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java
@@ -18,14 +18,8 @@
import com.google.inject.Guice;
import com.google.inject.Injector;
-import org.eclipse.jgit.lib.Config;
-
-public class LuceneQueryChangesTest extends AbstractIndexQueryChangesTest {
+public class LuceneQueryChangesTest extends AbstractQueryChangesTest {
protected Injector createInjector() {
- Config cfg = InMemoryModule.newDefaultConfig();
- cfg.setString("index", null, "type", "lucene");
- cfg.setBoolean("index", "lucene", "testInmemory", true);
- cfg.setInt("index", "lucene", "testVersion", 4);
- return Guice.createInjector(new InMemoryModule(cfg));
+ return Guice.createInjector(new InMemoryModule());
}
}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryModule.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryModule.java
index 77a0346..40c79b0 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryModule.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryModule.java
@@ -84,7 +84,8 @@
cfg.setString("cache", null, "directory", null);
cfg.setString("index", null, "type", "lucene");
cfg.setBoolean("index", "lucene", "testInmemory", true);
- cfg.setInt("index", "lucene", "testVersion", 4);
+ cfg.setInt("index", "lucene", "testVersion",
+ ChangeSchemas.getLatest().getVersion());
return cfg;
}
