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 aab6c64f38..bbc4773db1 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 @@ -154,7 +154,7 @@ public class IndexRewriteImpl implements ChangeQueryRewriter { Predicate out = rewriteImpl(in, index, limit); if (in == out || out instanceof IndexPredicate) { - return new IndexedChangeQuery(index, out, limit); + return new IndexedChangeQuery(db, index, out, limit); } else if (out == null /* cannot rewrite */) { return in; } else { @@ -231,7 +231,7 @@ public class IndexRewriteImpl implements ChangeQueryRewriter { if (isIndexed.cardinality() == 1) { int i = isIndexed.nextSetBit(0); newChildren.add( - 0, new IndexedChangeQuery(index, newChildren.remove(i), limit)); + 0, new IndexedChangeQuery(db, index, newChildren.remove(i), limit)); return copy(in, newChildren); } @@ -251,7 +251,7 @@ public class IndexRewriteImpl implements ChangeQueryRewriter { all.add(c); } } - all.add(0, new IndexedChangeQuery(index, in.copy(indexed), limit)); + all.add(0, new IndexedChangeQuery(db, index, in.copy(indexed), limit)); return copy(in, all); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexedChangeQuery.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexedChangeQuery.java index 974e29112d..2185af3917 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexedChangeQuery.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexedChangeQuery.java @@ -14,17 +14,22 @@ package com.google.gerrit.server.index; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Function; - import com.google.common.base.Objects; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; +import com.google.common.collect.Lists; +import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.query.Predicate; import com.google.gerrit.server.query.QueryParseException; import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.ChangeDataSource; +import com.google.gerrit.server.query.change.Paginated; +import com.google.gerrit.server.query.change.SortKeyPredicate; import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.ResultSet; +import com.google.inject.Provider; import java.util.Collection; import java.util.Iterator; @@ -39,18 +44,55 @@ import java.util.List; * {@link ChangeDataSource} to be chosen by the query processor. */ public class IndexedChangeQuery extends Predicate - implements ChangeDataSource { - private final ChangeIndex index; - private final Predicate pred; - private final int limit; - private final ChangeDataSource source; + implements ChangeDataSource, Paginated { - public IndexedChangeQuery(ChangeIndex index, Predicate pred, int limit) - throws QueryParseException { + /** + * Replace all {@link SortKeyPredicate}s in a tree. + *

+ * Strictly speaking this should replace only the {@link SortKeyPredicate} at + * the top-level AND node, but this implementation is simpler, and the + * behavior of having multiple sortkey operators is undefined anyway. + * + * @param p predicate to replace in. + * @param newValue new cut value to replace all sortkey operators with. + * @return a copy of {@code p} with all sortkey predicates replaced; or p + * itself. + */ + @VisibleForTesting + static Predicate replaceSortKeyPredicates( + Predicate p, String newValue) { + if (p instanceof SortKeyPredicate) { + return ((SortKeyPredicate) p).copy(newValue); + } else if (p.getChildCount() > 0) { + List> newChildren = + Lists.newArrayListWithCapacity(p.getChildCount()); + boolean replaced = false; + for (Predicate c : p.getChildren()) { + Predicate nc = replaceSortKeyPredicates(c, newValue); + newChildren.add(nc); + if (nc != c) { + replaced = true; + } + } + return replaced ? p.copy(newChildren) : p; + } else { + return p; + } + } + + private final Provider db; + private final ChangeIndex index; + private final int limit; + + private Predicate pred; + private ChangeDataSource source; + + public IndexedChangeQuery(Provider db, ChangeIndex index, + Predicate pred, int limit) throws QueryParseException { + this.db = db; this.index = index; this.pred = pred; this.limit = limit; - this.source = index.getSource(pred, limit); } @Override @@ -71,9 +113,14 @@ public class IndexedChangeQuery extends Predicate return ImmutableList.of(pred); } + @Override + public int limit() { + return limit; + } + @Override public int getCardinality() { - return source.getCardinality(); + return source != null ? source.getCardinality() : limit(); } @Override @@ -84,7 +131,15 @@ public class IndexedChangeQuery extends Predicate @Override public ResultSet read() throws OrmException { + final ChangeDataSource currSource; + try { + currSource = index.getSource(pred, limit); + } catch (QueryParseException e) { + throw new OrmException(e); + } + source = currSource; final ResultSet rs = source.read(); + return new ResultSet() { @Override public Iterator iterator() { @@ -94,7 +149,7 @@ public class IndexedChangeQuery extends Predicate @Override public ChangeData apply(ChangeData input) { - input.cacheFromSource(source); + input.cacheFromSource(currSource); return input; } }).iterator(); @@ -104,7 +159,7 @@ public class IndexedChangeQuery extends Predicate public List toList() { List r = rs.toList(); for (ChangeData cd : r) { - cd.cacheFromSource(source); + cd.cacheFromSource(currSource); } return r; } @@ -116,6 +171,12 @@ public class IndexedChangeQuery extends Predicate }; } + @Override + public ResultSet restart(ChangeData last) throws OrmException { + pred = replaceSortKeyPredicates(pred, last.change(db).getSortKey()); + return read(); + } + @Override public Predicate copy( Collection> children) { @@ -124,7 +185,7 @@ public class IndexedChangeQuery extends Predicate @Override public boolean match(ChangeData cd) throws OrmException { - return cd.isFromSource(source) || pred.match(cd); + return (source != null && cd.isFromSource(source)) || pred.match(cd); } @Override @@ -153,7 +214,7 @@ public class IndexedChangeQuery extends Predicate @Override public String toString() { return Objects.toStringHelper("index") - .add("p", source) + .add("p", pred) .add("limit", limit) .toString(); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/Paginated.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/Paginated.java index 2b3edf7a2f..d9ff80ca23 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/Paginated.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/Paginated.java @@ -17,7 +17,7 @@ package com.google.gerrit.server.query.change; import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.ResultSet; -interface Paginated { +public interface Paginated { int limit(); ResultSet restart(ChangeData last) throws OrmException; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/SortKeyPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/SortKeyPredicate.java index 35a42f6e0b..5b4881d721 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/SortKeyPredicate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/SortKeyPredicate.java @@ -38,6 +38,7 @@ public abstract class SortKeyPredicate extends IndexPredicate { public abstract long getMinValue(); public abstract long getMaxValue(); + public abstract SortKeyPredicate copy(String newValue); public static class Before extends SortKeyPredicate { Before(Provider dbProvider, String value) { @@ -59,6 +60,11 @@ public abstract class SortKeyPredicate extends IndexPredicate { Change change = cd.change(dbProvider); return change != null && change.getSortKey().compareTo(getValue()) < 0; } + + @Override + public Before copy(String newValue) { + return new Before(dbProvider, newValue); + } } public static class After extends SortKeyPredicate { @@ -81,5 +87,10 @@ public abstract class SortKeyPredicate extends IndexPredicate { Change change = cd.change(dbProvider); return change != null && change.getSortKey().compareTo(getValue()) > 0; } + + @Override + public After copy(String newValue) { + return new After(dbProvider, newValue); + } } } 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 fb3f9bad2d..08617b6850 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 @@ -214,7 +214,7 @@ public class IndexRewriteTest extends TestCase { private IndexedChangeQuery query(Predicate p, int limit) throws QueryParseException { - return new IndexedChangeQuery(index, p, limit); + return new IndexedChangeQuery(null, index, p, limit); } private Set status(String query) throws QueryParseException { diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/index/IndexedChangeQueryTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/index/IndexedChangeQueryTest.java new file mode 100644 index 0000000000..ba1f53d6e8 --- /dev/null +++ b/gerrit-server/src/test/java/com/google/gerrit/server/index/IndexedChangeQueryTest.java @@ -0,0 +1,67 @@ +// 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.git; + +package com.google.gerrit.server.index; + +import static com.google.gerrit.server.index.IndexedChangeQuery.replaceSortKeyPredicates; + +import com.google.gerrit.server.query.Predicate; +import com.google.gerrit.server.query.QueryParseException; +import com.google.gerrit.server.query.change.ChangeData; +import com.google.gerrit.server.query.change.ChangeQueryBuilder; + +import junit.framework.TestCase; + +public class IndexedChangeQueryTest extends TestCase { + private FakeIndex index; + private ChangeQueryBuilder queryBuilder; + + @Override + public void setUp() throws Exception { + super.setUp(); + index = new FakeIndex(FakeIndex.V2); + IndexCollection indexes = new IndexCollection(); + indexes.setSearchIndex(index); + queryBuilder = new FakeQueryBuilder(indexes); + } + + public void testReplaceSortKeyPredicate_NoSortKey() throws Exception { + Predicate p = parse("foo:a bar:b OR (foo:b bar:a)"); + assertSame(p, replaceSortKeyPredicates(p, "1234")); + } + + public void testReplaceSortKeyPredicate_TopLevelSortKey() throws Exception { + Predicate p; + p = parse("foo:a bar:b sortkey_before:1234 OR (foo:b bar:a)"); + assertEquals(parse("foo:a bar:b sortkey_before:5678 OR (foo:b bar:a)"), + replaceSortKeyPredicates(p, "5678")); + p = parse("foo:a bar:b sortkey_after:1234 OR (foo:b bar:a)"); + assertEquals(parse("foo:a bar:b sortkey_after:5678 OR (foo:b bar:a)"), + replaceSortKeyPredicates(p, "5678")); + } + + public void testReplaceSortKeyPredicate_NestedSortKey() throws Exception { + Predicate p; + p = parse("foo:a bar:b OR (foo:b bar:a AND sortkey_before:1234)"); + assertEquals(parse("foo:a bar:b OR (foo:b bar:a sortkey_before:5678)"), + replaceSortKeyPredicates(p, "5678")); + p = parse("foo:a bar:b OR (foo:b bar:a AND sortkey_after:1234)"); + assertEquals(parse("foo:a bar:b OR (foo:b bar:a sortkey_after:5678)"), + replaceSortKeyPredicates(p, "5678")); + } + + private Predicate parse(String query) throws QueryParseException { + return queryBuilder.parse(query); + } +}