Merge "ChangeScreen2: linkify change author"
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
index 9debd5d..c7f6d16 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
@@ -923,7 +923,7 @@
Boolean reviewed;
Boolean mergeable;
- String _sortkey;
+ public String _sortkey;
public int _number;
AccountInfo owner;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexRewriteImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexRewriteImpl.java
index 4d8c951..414715d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexRewriteImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexRewriteImpl.java
@@ -147,9 +147,8 @@
return sqlRewriter.rewrite(in);
}
in = basicRewrites.rewrite(in);
- // Add 1 to specified limit to match behavior of QueryProcessor.
int limit = ChangeQueryBuilder.hasLimit(in)
- ? ChangeQueryBuilder.getLimit(in) + 1
+ ? ChangeQueryBuilder.getLimit(in)
: MAX_LIMIT;
Predicate<ChangeData> out = rewriteImpl(in, index, limit);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryChanges.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryChanges.java
index 24186b8..44c71a9 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryChanges.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryChanges.java
@@ -101,6 +101,10 @@
queries.add(query);
}
+ public String getQuery(int i) {
+ return queries.get(i);
+ }
+
@Override
public Object apply(TopLevelResource rsrc)
throws BadRequestException, AuthException, OrmException {
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/index/IndexRewriteTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/index/IndexRewriteTest.java
index f9bb4f3..d9016e9 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/index/IndexRewriteTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/index/IndexRewriteTest.java
@@ -152,7 +152,7 @@
Predicate<ChangeData> out = rewrite(in);
assertSame(AndSource.class, out.getClass());
assertEquals(ImmutableList.of(
- query(in.getChild(0), 4),
+ query(in.getChild(0), 3),
in.getChild(1)),
out.getChildren());
}
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 87fdb18..1e8b87a 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
@@ -23,6 +23,7 @@
import com.google.common.base.Charsets;
import com.google.common.base.Objects;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Lists;
import com.google.common.hash.Hashing;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.restapi.TopLevelResource;
@@ -400,16 +401,171 @@
@Test
public void limit() throws Exception {
TestRepository<InMemoryRepository> repo = createProject("repo");
- Change change1 = newChange(repo, null, null, null, null).insert();
+ Change last = null;
+ int n = 5;
+ for (int i = 0; i < n; i++) {
+ last = newChange(repo, null, null, null, null).insert();
+ }
+
+ List<ChangeInfo> results;
+ for (int i = 1; i <= n + 2; i++) {
+ results = query("status:new limit:" + i);
+ assertEquals(Math.min(i, n), results.size());
+ assertResultEquals(last, results.get(0));
+ }
+ }
+
+ @Test
+ public void pagination() throws Exception {
+ TestRepository<InMemoryRepository> repo = createProject("repo");
+ List<Change> changes = Lists.newArrayList();
+ for (int i = 0; i < 5; i++) {
+ changes.add(newChange(repo, null, null, null, null).insert());
+ }
+
+ // Page forward and back through 3 pages of results.
+ QueryChanges q;
+ List<ChangeInfo> results;
+ results = query("status:new limit:2");
+ assertEquals(2, results.size());
+ assertResultEquals(changes.get(4), results.get(0));
+ assertResultEquals(changes.get(3), results.get(1));
+
+ q = newQuery("status:new limit:2");
+ q.setSortKeyBefore(results.get(1)._sortkey);
+ results = query(q);
+ assertEquals(2, results.size());
+ assertResultEquals(changes.get(2), results.get(0));
+ assertResultEquals(changes.get(1), results.get(1));
+
+ q = newQuery("status:new limit:2");
+ q.setSortKeyBefore(results.get(1)._sortkey);
+ results = query(q);
+ assertEquals(1, results.size());
+ assertResultEquals(changes.get(0), results.get(0));
+
+ q = newQuery("status:new limit:2");
+ q.setSortKeyAfter(results.get(0)._sortkey);
+ results = query(q);
+ assertEquals(2, results.size());
+ assertResultEquals(changes.get(2), results.get(0));
+ assertResultEquals(changes.get(1), results.get(1));
+
+ q = newQuery("status:new limit:2");
+ q.setSortKeyAfter(results.get(0)._sortkey);
+ results = query(q);
+ assertEquals(2, results.size());
+ assertResultEquals(changes.get(4), results.get(0));
+ assertResultEquals(changes.get(3), results.get(1));
+ }
+
+ @Test
+ public void sortKeyWithMinuteResolution() throws Exception {
+ clockStepMs = MILLISECONDS.convert(2, MINUTES);
+ TestRepository<InMemoryRepository> repo = createProject("repo");
+ ChangeInserter ins1 = newChange(repo, null, null, null, null);
+ Change change1 = ins1.insert();
+ ChangeControl ctl1 = changeControlFactory.controlFor(change1, user);
Change change2 = newChange(repo, null, null, null, null).insert();
- // TODO(dborowitz): Limit is broken in SQL (returns N+1 results) with some
- // kinds of predicates but not others.
- //assertResultEquals(change2, queryOne("status:new limit:1"));
- List<ChangeInfo> results = query("status:new limit:2");
+ assertTrue(lastUpdatedMs(change1) < lastUpdatedMs(change2));
+
+ List<ChangeInfo> results;
+ results = query("status:new");
assertEquals(2, results.size());
assertResultEquals(change2, results.get(0));
assertResultEquals(change1, results.get(1));
+
+ PostReview.Input input = new PostReview.Input();
+ input.message = "toplevel";
+ postReview.apply(new RevisionResource(
+ new ChangeResource(ctl1), ins1.getPatchSet()), input);
+ change1 = db.changes().get(change1.getId());
+
+ assertTrue(lastUpdatedMs(change1) > lastUpdatedMs(change2));
+ assertTrue(lastUpdatedMs(change1) - lastUpdatedMs(change2)
+ > MILLISECONDS.convert(1, MINUTES));
+
+ results = query("status:new");
+ assertEquals(2, results.size());
+ // change1 moved to the top.
+ assertResultEquals(change1, results.get(0));
+ assertResultEquals(change2, results.get(1));
+ }
+
+ @Test
+ public void sortKeyWithSubMinuteResolution() throws Exception {
+ TestRepository<InMemoryRepository> repo = createProject("repo");
+ ChangeInserter ins1 = newChange(repo, null, null, null, null);
+ Change change1 = ins1.insert();
+ ChangeControl ctl1 = changeControlFactory.controlFor(change1, user);
+ Change change2 = newChange(repo, null, null, null, null).insert();
+
+ assertTrue(lastUpdatedMs(change1) < lastUpdatedMs(change2));
+
+ List<ChangeInfo> results;
+ results = query("status:new");
+ assertEquals(2, results.size());
+ assertResultEquals(change2, results.get(0));
+ assertResultEquals(change1, results.get(1));
+
+ PostReview.Input input = new PostReview.Input();
+ input.message = "toplevel";
+ postReview.apply(new RevisionResource(
+ new ChangeResource(ctl1), ins1.getPatchSet()), input);
+ change1 = db.changes().get(change1.getId());
+
+ assertTrue(lastUpdatedMs(change1) > lastUpdatedMs(change2));
+ assertTrue(lastUpdatedMs(change1) - lastUpdatedMs(change2)
+ < MILLISECONDS.convert(1, MINUTES));
+
+ results = query("status:new");
+ assertEquals(2, results.size());
+ // Same order as before change1 was modified.
+ assertResultEquals(change2, results.get(0));
+ assertResultEquals(change1, results.get(1));
+ }
+
+ @Test
+ public void sortKeyBreaksTiesOnChangeId() throws Exception {
+ clockStepMs = 0;
+ TestRepository<InMemoryRepository> repo = createProject("repo");
+ Change change1 = newChange(repo, null, null, null, null).insert();
+ Change change2 = newChange(repo, null, null, null, null).insert();
+ assertEquals(change1.getLastUpdatedOn(), change2.getLastUpdatedOn());
+
+ List<ChangeInfo> results = query("status:new");
+ assertEquals(2, results.size());
+ assertResultEquals(change2, results.get(0));
+ assertResultEquals(change1, results.get(1));
+ }
+
+ @Test
+ public void filterOutMoreThanOnePageOfResults() throws Exception {
+ TestRepository<InMemoryRepository> repo = createProject("repo");
+ Change change = newChange(repo, null, null, userId.get(), null).insert();
+ int user2 = accountManager.authenticate(AuthRequest.forUser("anotheruser"))
+ .getAccountId().get();
+ for (int i = 0; i < 5; i++) {
+ newChange(repo, null, null, user2, null).insert();
+ }
+
+ assertResultEquals(change, queryOne("status:new ownerin:Administrators"));
+ assertResultEquals(change,
+ queryOne("status:new ownerin:Administrators limit:2"));
+ }
+
+ @Test
+ public void filterOutAllResults() throws Exception {
+ TestRepository<InMemoryRepository> repo = createProject("repo");
+ int user2 = accountManager.authenticate(AuthRequest.forUser("anotheruser"))
+ .getAccountId().get();
+ for (int i = 0; i < 5; i++) {
+ newChange(repo, null, null, user2, null).insert();
+ }
+
+ assertTrue(query("status:new ownerin:Administrators").isEmpty());
+ assertTrue(query("status:new ownerin:Administrators limit:2").isEmpty());
}
protected ChangeInserter newChange(
@@ -465,25 +621,33 @@
repoManager.openRepository(new Project.NameKey(name)));
}
- @SuppressWarnings({"rawtypes", "unchecked"})
- protected List<ChangeInfo> query(Object query) throws Exception {
+ protected QueryChanges newQuery(Object query) {
QueryChanges q = queryProvider.get();
q.addQuery(query.toString());
+ return q;
+ }
+
+ @SuppressWarnings({"rawtypes", "unchecked"})
+ protected List<ChangeInfo> query(QueryChanges q) throws Exception {
Object result = q.apply(TLR);
assertTrue(
String.format("expected List<ChangeInfo>, found %s for [%s]",
- result, query),
+ result, q.getQuery(0)),
result instanceof List);
List results = (List) result;
if (!results.isEmpty()) {
assertTrue(
String.format("expected ChangeInfo, found %s for [%s]",
- result, query),
+ result, q.getQuery(0)),
results.get(0) instanceof ChangeInfo);
}
return (List<ChangeInfo>) result;
}
+ protected List<ChangeInfo> query(Object query) throws Exception {
+ return query(newQuery(query));
+ }
+
protected ChangeInfo queryOne(Object query) throws Exception {
List<ChangeInfo> results = query(query);
assertTrue(
@@ -492,4 +656,8 @@
results.size() == 1);
return results.get(0);
}
+
+ private static long lastUpdatedMs(Change c) {
+ return c.getLastUpdatedOn().getTime();
+ }
}
