Use JavaSqlTimestampHelper for parsing timestamps in before/after

JGit's GitDateParser supports a confusingly limited set of formats,
and moreover silently drops any portion of the string that it can't
parse.

Instead, parse only a much more limited set of timestamps, but at
least support the same format as emitted in our JSON. This means that
scripts can simply pass the updated timestamp of the last result to
"before:" in their next query for accurate pagination.

Because timestamps emitted by the REST API are UTC, this may produce
slightly confusing behavior in the web UI, which converts to server
local time before displaying.

This loses some features, such as a few additional datetime formats
and relative dates. The latter is not particularly useful since we
already have "age:".

Change-Id: I646d07290f1888ebe91fa9415cc42fe4d7767de7

Documentation/user-search.txt

@@ -67,16 +67,16 @@ to include a unit suffix, for example `age:2d`:
 [[before_until]]
 before:'TIME'/until:'TIME'
 +
-Changes modified before the given 'TIME', inclusive. With no time,
-assumes 00:00:00 in the local time zone. Supports many formats supported
-by `git log`.
+Changes modified before the given 'TIME', inclusive. Must be in the
+format `2006-01-02[ 15:04:05[.890][ -0700]]`; omitting the time defaults
+to 00:00:00 and omitting the timezone defaults to UTC.
 
 [[after_since]]
 after:'TIME'/since:'TIME'
 +
-Changes modified before the given 'TIME', inclusive. With no time,
-assumes 00:00:00 in the local time zone. Supports many formats supported
-by `git log`.
+Changes modified after the given 'TIME', inclusive. Must be in the
+format `2006-01-02[ 15:04:05[.890][ -0700]]`; omitting the time defaults
+to 00:00:00 and omitting the timezone defaults to UTC.
 
 [[change]]
 change:'ID'::

gerrit-server/src/main/java/com/google/gerrit/server/index/TimestampRangePredicate.java

@@ -20,14 +20,10 @@ import static com.google.gerrit.server.index.ChangeField.UPDATED;
 
 import com.google.gerrit.server.query.QueryParseException;
 import com.google.gerrit.server.query.change.ChangeData;
-
-import org.eclipse.jgit.util.GitDateParser;
-import org.joda.time.DateTime;
+import com.google.gwtjsonrpc.common.JavaSqlTimestampHelper;
 
 import java.sql.Timestamp;
-import java.text.ParseException;
 import java.util.Date;
-import java.util.Locale;
 
 public abstract class TimestampRangePredicate<I> extends IndexPredicate<I> {
   @SuppressWarnings({"deprecation", "unchecked"})
@@ -46,11 +42,11 @@ public abstract class TimestampRangePredicate<I> extends IndexPredicate<I> {
     return (FieldDef<ChangeData, Timestamp>) f;
   }
 
-  protected static Date parse(String value) throws QueryParseException {
+  protected static Timestamp parse(String value) throws QueryParseException {
     try {
-      return GitDateParser.parse(value, DateTime.now().toCalendar(Locale.US));
-    } catch (ParseException e) {
-      // ParseException's message is specific and helpful, so preserve it.
+      return JavaSqlTimestampHelper.parseTimestamp(value);
+    } catch (IllegalArgumentException e) {
+      // parseTimestamp's errors are specific and helpful, so preserve them.
       throw new QueryParseException(e.getMessage(), e);
     }
   }

gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java

@@ -736,20 +736,24 @@ public abstract class AbstractQueryChangesTest {
   }
 
   @Test
-  public void byBeforeAbsolute() throws Exception {
+  public void byBefore() throws Exception {
     clockStepMs = MILLISECONDS.convert(30, HOURS);
     TestRepository<InMemoryRepository> repo = createProject("repo");
     Change change1 = newChange(repo, null, null, null, null).insert();
     Change change2 = newChange(repo, null, null, null, null).insert();
     clockStepMs = 0;
 
-    // GitDateParser drops unparsed portions of the string, so be very careful
-    // with formats.
     assertTrue(query("before:2009-09-29").isEmpty());
     assertTrue(query("before:2009-09-30").isEmpty());
     assertTrue(query("before:\"2009-09-30 16:59:00 -0400\"").isEmpty());
+    assertTrue(query("before:\"2009-09-30 20:59:00 -0000\"").isEmpty());
+    assertTrue(query("before:\"2009-09-30 20:59:00\"").isEmpty());
     assertResultEquals(change1,
-        queryOne("before:\"2009-09-30 21:02:00 -0400\""));
+        queryOne("before:\"2009-09-30 17:02:00 -0400\""));
+    assertResultEquals(change1,
+        queryOne("before:\"2009-10-01 21:02:00 -0000\""));
+    assertResultEquals(change1,
+        queryOne("before:\"2009-10-01 21:02:00\""));
     assertResultEquals(change1, queryOne("before:2009-10-01"));
 
     List<ChangeInfo> results;
@@ -760,41 +764,18 @@ public abstract class AbstractQueryChangesTest {
   }
 
   @Test
-  public void byBeforeRelative() throws Exception {
+  public void byAfter() throws Exception {
     clockStepMs = MILLISECONDS.convert(30, HOURS);
     TestRepository<InMemoryRepository> repo = createProject("repo");
     Change change1 = newChange(repo, null, null, null, null).insert();
     Change change2 = newChange(repo, null, null, null, null).insert();
     clockStepMs = 0;
 
-    assertTrue(query("before:\"3 days ago\"").isEmpty());
-    assertResultEquals(change1, queryOne("before:\"2 days ago\""));
-
-    List<ChangeInfo> results;
-    results = query("before:\"1 day ago\"");
-    assertEquals(2, results.size());
-    assertResultEquals(change2, results.get(0));
-    assertResultEquals(change1, results.get(1));
-
-    results = query("before:\"12 hours ago\"");
-    assertEquals(2, results.size());
-    assertResultEquals(change2, results.get(0));
-    assertResultEquals(change1, results.get(1));
-  }
-
-  @Test
-  public void byAfterAbsolute() throws Exception {
-    clockStepMs = MILLISECONDS.convert(30, HOURS);
-    TestRepository<InMemoryRepository> repo = createProject("repo");
-    Change change1 = newChange(repo, null, null, null, null).insert();
-    Change change2 = newChange(repo, null, null, null, null).insert();
-    clockStepMs = 0;
-
-    // GitDateParser drops unparsed portions of the string, so be very careful
-    // with formats.
-    assertTrue(query("after:2009-10-02").isEmpty());
+    assertTrue(query("after:2009-10-03").isEmpty());
     assertResultEquals(change2,
         queryOne("after:\"2009-10-01 20:59:59 -0400\""));
+    assertResultEquals(change2,
+        queryOne("after:\"2009-10-01 20:59:59 -0000\""));
     assertResultEquals(change2, queryOne("after:2009-10-01"));
 
     List<ChangeInfo> results;
@@ -804,29 +785,6 @@ public abstract class AbstractQueryChangesTest {
     assertResultEquals(change1, results.get(1));
   }
 
-  @Test
-  public void byAfterRelative() throws Exception {
-    clockStepMs = MILLISECONDS.convert(30, HOURS);
-    TestRepository<InMemoryRepository> repo = createProject("repo");
-    Change change1 = newChange(repo, null, null, null, null).insert();
-    Change change2 = newChange(repo, null, null, null, null).insert();
-    clockStepMs = 0;
-
-    assertTrue(query("after:\"1 days ago\"").isEmpty());
-    assertResultEquals(change2, queryOne("after:\"2 days ago\""));
-
-    List<ChangeInfo> results;
-    results = query("after:\"3 days ago\"");
-    assertEquals(2, results.size());
-    assertResultEquals(change2, results.get(0));
-    assertResultEquals(change1, results.get(1));
-
-    results = query("after:\"72 hours ago\"");
-    assertEquals(2, results.size());
-    assertResultEquals(change2, results.get(0));
-    assertResultEquals(change1, results.get(1));
-  }
-
   protected ChangeInserter newChange(
       TestRepository<InMemoryRepository> repo,
       @Nullable RevCommit commit, @Nullable String key, @Nullable Integer owner,