From 90b89d1dfe9af271534636ed8e8d8feda2b54f27 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Tue, 9 Sep 2014 15:23:44 +0200 Subject: [PATCH] ChangeStatusPredicate: Prefix search for open/closed Change-Id: I6d0bfbf3db1263b68d85359ad11f2f2f948c6db9 --- .../query/change/BasicChangeRewrites.java | 4 +- .../query/change/ChangeStatusPredicate.java | 55 ++++++++++--------- .../change/AbstractQueryChangesTest.java | 18 ++++++ 3 files changed, 49 insertions(+), 28 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/BasicChangeRewrites.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/BasicChangeRewrites.java index 50a8d5535b..e85365610d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/BasicChangeRewrites.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/BasicChangeRewrites.java @@ -59,7 +59,7 @@ public class BasicChangeRewrites extends QueryRewriter { @Rewrite("-status:merged") public Predicate r00_notMerged() { return or(ChangeStatusPredicate.open(), - new ChangeStatusPredicate(Change.Status.ABANDONED)); + ChangeStatusPredicate.forStatus(Change.Status.ABANDONED)); } @SuppressWarnings("unchecked") @@ -67,7 +67,7 @@ public class BasicChangeRewrites extends QueryRewriter { @Rewrite("-status:abandoned") public Predicate r00_notAbandoned() { return or(ChangeStatusPredicate.open(), - new ChangeStatusPredicate(Change.Status.MERGED)); + ChangeStatusPredicate.forStatus(Change.Status.MERGED)); } @NoCostComputation diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeStatusPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeStatusPredicate.java index 7f35bcd289..7d1cdd1882 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeStatusPredicate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeStatusPredicate.java @@ -37,13 +37,27 @@ import java.util.TreeMap; * Status names are looked up by prefix case-insensitively. */ public final class ChangeStatusPredicate extends IndexPredicate { - private static final TreeMap VALUES; + private static final TreeMap> PREDICATES; + private static final Predicate CLOSED; + private static final Predicate OPEN; static { - VALUES = new TreeMap<>(); + PREDICATES = new TreeMap<>(); + List> open = new ArrayList<>(); + List> closed = new ArrayList<>(); + for (Change.Status s : Change.Status.values()) { - VALUES.put(canonicalize(s), s); + ChangeStatusPredicate p = new ChangeStatusPredicate(s); + PREDICATES.put(canonicalize(s), p); + (s.isOpen() ? open : closed).add(p); } + + CLOSED = Predicate.or(closed); + OPEN = Predicate.or(open); + + PREDICATES.put("closed", CLOSED); + PREDICATES.put("open", OPEN); + PREDICATES.put("pending", OPEN); } public static String canonicalize(Change.Status status) { @@ -51,46 +65,35 @@ public final class ChangeStatusPredicate extends IndexPredicate { } public static Predicate parse(String value) { - if ("open".equalsIgnoreCase(value) || "pending".equalsIgnoreCase(value)) { - return open(); - } else if ("closed".equalsIgnoreCase(value)) { - return closed(); - } String lower = value.toLowerCase(); - NavigableMap head = VALUES.tailMap(lower, true); + NavigableMap> head = + PREDICATES.tailMap(lower, true); if (!head.isEmpty()) { // Assume no statuses share a common prefix so we can only walk one entry. - Map.Entry e = head.entrySet().iterator().next(); + Map.Entry> e = + head.entrySet().iterator().next(); if (e.getKey().startsWith(lower)) { - return new ChangeStatusPredicate(e.getValue()); + return e.getValue(); } } throw new IllegalArgumentException("invalid change status: " + value); } + public static Predicate forStatus(Change.Status status) { + return parse(status.name()); + } + public static Predicate open() { - List> r = new ArrayList<>(4); - for (final Change.Status e : Change.Status.values()) { - if (e.isOpen()) { - r.add(new ChangeStatusPredicate(e)); - } - } - return r.size() == 1 ? r.get(0) : or(r); + return OPEN; } public static Predicate closed() { - List> r = new ArrayList<>(4); - for (final Change.Status e : Change.Status.values()) { - if (e.isClosed()) { - r.add(new ChangeStatusPredicate(e)); - } - } - return r.size() == 1 ? r.get(0) : or(r); + return CLOSED; } private final Change.Status status; - ChangeStatusPredicate(Change.Status status) { + private ChangeStatusPredicate(Change.Status status) { super(ChangeField.STATUS, canonicalize(status)); this.status = status; } 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 827860141b..c9fdfbd9bd 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 @@ -229,7 +229,17 @@ public abstract class AbstractQueryChangesTest { assertEquals(2, results.size()); assertResultEquals(change2, results.get(0)); assertResultEquals(change1, results.get(1)); + assertEquals(2, query("status:OPEN").size()); + assertEquals(2, query("status:o").size()); + assertEquals(2, query("status:op").size()); + assertEquals(2, query("status:ope").size()); + assertEquals(2, query("status:pending").size()); + assertEquals(2, query("status:PENDING").size()); + assertEquals(2, query("status:p").size()); + assertEquals(2, query("status:pe").size()); + assertEquals(2, query("status:pen").size()); + results = query("is:open"); assertEquals(2, results.size()); assertResultEquals(change2, results.get(0)); @@ -257,7 +267,15 @@ public abstract class AbstractQueryChangesTest { assertEquals(2, results.size()); assertResultEquals(change2, results.get(0)); assertResultEquals(change1, results.get(1)); + assertEquals(2, query("status:CLOSED").size()); + assertEquals(2, query("status:c").size()); + assertEquals(2, query("status:cl").size()); + assertEquals(2, query("status:clo").size()); + assertEquals(2, query("status:clos").size()); + assertEquals(2, query("status:close").size()); + assertEquals(2, query("status:closed").size()); + results = query("is:closed"); assertEquals(2, results.size()); assertResultEquals(change2, results.get(0));