Replace joda-time with Java 8 API
joda-time is used to format dates to timestamp strings, but since change
Iccfdb1e20 in core Gerrit it is no longer exported in the plugin API.
So far we used an explicit dependency on joda-time, but a better long term
solution is to migrate to the Java 8 date/time API.
Factor the timestamp formatting out to a new utility class, replacing the
usage of joda-time with Java 8's DateTimeFormatter.
Timestamps previously generated by joda-time did not use a specific time
zone or locale, and were generated using the server's defaults. In the
new formatter, explicitly set the locale and zone to the server's defaults,
so that any previously generated timestamps will be parsed successfully.
In a future change, we may consider using UTC zone and US locale, and
migrating any existing timestamps.
Keep the dependency on joda-time, but only for a test which confirms
that the new implementation produces the same values for various time
zones (the formatting is different for UTC).
Change-Id: I49bffffbd171065c6b3c0803a5a2bb377bd27ef1
diff --git a/BUILD b/BUILD
index 7689c5e..bb1d8fe 100644
--- a/BUILD
+++ b/BUILD
@@ -9,7 +9,6 @@
gerrit_plugin(
name = "lfs",
srcs = glob(["src/main/java/**/*.java"]),
- resources = glob(["src/main/resources/**/*"]),
manifest_entries = [
"Gerrit-PluginName: lfs",
"Gerrit-Module: com.googlesource.gerrit.plugins.lfs.Module",
@@ -17,11 +16,11 @@
"Gerrit-SshModule: com.googlesource.gerrit.plugins.lfs.SshModule",
"Gerrit-InitStep: com.googlesource.gerrit.plugins.lfs.InitLfs",
],
+ resources = glob(["src/main/resources/**/*"]),
deps = [
"@jgit_http_apache//jar",
"@jgit_lfs//jar",
"@jgit_lfs_server//jar",
- "@joda_time//jar",
],
)
@@ -34,3 +33,13 @@
"@jgit_lfs//jar",
],
)
+
+java_library(
+ name = "lfs__plugin_test_deps",
+ testonly = 1,
+ visibility = ["//visibility:public"],
+ exports = PLUGIN_DEPS + PLUGIN_TEST_DEPS + [
+ ":lfs__plugin",
+ "@joda_time//jar",
+ ],
+)
diff --git a/src/main/java/com/googlesource/gerrit/plugins/lfs/LfsAuthToken.java b/src/main/java/com/googlesource/gerrit/plugins/lfs/LfsAuthToken.java
index 399dc21..0cadf7b 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/lfs/LfsAuthToken.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/lfs/LfsAuthToken.java
@@ -18,12 +18,10 @@
import com.google.common.base.Splitter;
import java.util.List;
import java.util.Optional;
-import org.joda.time.DateTime;
-import org.joda.time.DateTimeZone;
-import org.joda.time.format.DateTimeFormatter;
-import org.joda.time.format.ISODateTimeFormat;
public abstract class LfsAuthToken {
+ private static final LfsDateTime FORMAT = LfsDateTime.instance();
+
public abstract static class Processor<T extends LfsAuthToken> {
private static final char DELIMETER = '~';
@@ -65,12 +63,10 @@
protected abstract boolean verifyTokenValues();
static boolean onTime(String dateTime) {
- String now = LfsAuthToken.ISO.print(now());
- return now.compareTo(dateTime) <= 0;
+ return FORMAT.now().compareTo(dateTime) <= 0;
}
}
- static final DateTimeFormatter ISO = ISODateTimeFormat.dateTime();
public final String expiresAt;
protected LfsAuthToken(int expirationSeconds) {
@@ -82,10 +78,6 @@
}
static String timeout(int expirationSeconds) {
- return LfsAuthToken.ISO.print(now().plusSeconds(expirationSeconds));
- }
-
- static DateTime now() {
- return DateTime.now().toDateTime(DateTimeZone.UTC);
+ return FORMAT.now(expirationSeconds);
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/lfs/LfsDateTime.java b/src/main/java/com/googlesource/gerrit/plugins/lfs/LfsDateTime.java
new file mode 100644
index 0000000..b47eb98
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/lfs/LfsDateTime.java
@@ -0,0 +1,54 @@
+// Copyright (C) 2017 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.googlesource.gerrit.plugins.lfs;
+
+import java.time.Instant;
+import java.time.ZoneId;
+import java.time.ZoneOffset;
+import java.time.format.DateTimeFormatter;
+import java.util.Locale;
+
+public class LfsDateTime {
+ private final DateTimeFormatter format;
+
+ private LfsDateTime(ZoneId zone) {
+ format =
+ DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss.SSSZZZZZ")
+ .withZone(zone)
+ .withLocale(Locale.getDefault());
+ }
+
+ /* Create an instance with the system default time zone. */
+ public static LfsDateTime instance() {
+ return new LfsDateTime(ZoneOffset.systemDefault());
+ }
+
+ /* Create an instance with the specified time zone. */
+ public static LfsDateTime instance(ZoneId zone) {
+ return new LfsDateTime(zone);
+ }
+
+ public String now() {
+ return format.format(Instant.now());
+ }
+
+ public String now(int secondsToAdd) {
+ return format.format(Instant.now().plusSeconds(secondsToAdd));
+ }
+
+ public String format(Instant instant) {
+ return format.format(instant);
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/lfs/locks/LfsProjectLocks.java b/src/main/java/com/googlesource/gerrit/plugins/lfs/locks/LfsProjectLocks.java
index d0875c1..7dfa103 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/lfs/locks/LfsProjectLocks.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/lfs/locks/LfsProjectLocks.java
@@ -23,6 +23,7 @@
import com.google.gson.GsonBuilder;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
+import com.googlesource.gerrit.plugins.lfs.LfsDateTime;
import com.googlesource.gerrit.plugins.lfs.locks.LfsLocksHandler.LfsLockExistsException;
import java.io.IOException;
import java.io.OutputStreamWriter;
@@ -35,10 +36,6 @@
import java.util.stream.Stream;
import org.eclipse.jgit.internal.storage.file.LockFile;
import org.eclipse.jgit.lfs.errors.LfsException;
-import org.joda.time.DateTime;
-import org.joda.time.DateTimeZone;
-import org.joda.time.format.DateTimeFormatter;
-import org.joda.time.format.ISODateTimeFormat;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -48,12 +45,12 @@
}
private static final Logger log = LoggerFactory.getLogger(LfsProjectLocks.class);
- private static final DateTimeFormatter ISO = ISODateTimeFormat.dateTime();
private static final Gson gson =
new GsonBuilder()
.setFieldNamingPolicy(FieldNamingPolicy.LOWER_CASE_WITH_UNDERSCORES)
.disableHtmlEscaping()
.create();
+ private static final LfsDateTime FORMAT = LfsDateTime.instance();
private final PathToLockId toLockId;
private final String project;
@@ -107,7 +104,8 @@
throw new LfsLockExistsException(lock);
}
- lock = new LfsLock(lockId, input.path, now(), new LfsLockOwner(user.getUserName().get()));
+ lock =
+ new LfsLock(lockId, input.path, FORMAT.now(), new LfsLockOwner(user.getUserName().get()));
LockFile fileLock = new LockFile(locksPath.resolve(lockId).toFile());
try {
if (!fileLock.lock()) {
@@ -189,8 +187,4 @@
Collection<LfsLock> getLocks() {
return locks.asMap().values();
}
-
- private String now() {
- return ISO.print(DateTime.now().toDateTime(DateTimeZone.UTC));
- }
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/lfs/LfsAuthTokenTest.java b/src/test/java/com/googlesource/gerrit/plugins/lfs/LfsAuthTokenTest.java
index 185af35..c074151 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/lfs/LfsAuthTokenTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/lfs/LfsAuthTokenTest.java
@@ -15,31 +15,28 @@
package com.googlesource.gerrit.plugins.lfs;
import static com.google.common.truth.Truth.assertThat;
-import static com.googlesource.gerrit.plugins.lfs.LfsAuthToken.ISO;
import static com.googlesource.gerrit.plugins.lfs.LfsAuthToken.Verifier.onTime;
+import java.time.Instant;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
-import org.joda.time.DateTime;
-import org.joda.time.DateTimeZone;
import org.junit.Test;
public class LfsAuthTokenTest {
private final LfsCipher cipher = new LfsCipher();
+ private final LfsDateTime formatter = LfsDateTime.instance();
@Test
public void testExpiredTime() throws Exception {
- DateTime now = now();
// test that even 1ms expiration is enough
- assertThat(onTime(ISO.print(now.minusMillis(1)))).isFalse();
+ assertThat(onTime(formatter.format(now().minusMillis(1)))).isFalse();
}
@Test
public void testOnTime() throws Exception {
- DateTime now = now();
// if there is at least 1ms before there is no timeout
- assertThat(onTime(ISO.print(now.plusMillis(1)))).isTrue();
+ assertThat(onTime(formatter.format(now().plusMillis(1)))).isTrue();
}
@Test
@@ -69,8 +66,8 @@
assertThat(verifier.verify()).isFalse();
}
- private DateTime now() {
- return DateTime.now().toDateTime(DateTimeZone.UTC);
+ private Instant now() {
+ return Instant.now();
}
private class TestToken extends LfsAuthToken {
diff --git a/src/test/java/com/googlesource/gerrit/plugins/lfs/LfsDateTimeTest.java b/src/test/java/com/googlesource/gerrit/plugins/lfs/LfsDateTimeTest.java
new file mode 100644
index 0000000..99dcfe2
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/lfs/LfsDateTimeTest.java
@@ -0,0 +1,51 @@
+// Copyright (C) 2017 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.googlesource.gerrit.plugins.lfs;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import java.time.Instant;
+import java.util.TimeZone;
+import org.joda.time.DateTime;
+import org.joda.time.DateTimeZone;
+import org.joda.time.format.ISODateTimeFormat;
+import org.junit.Test;
+import org.junit.experimental.theories.DataPoints;
+import org.junit.experimental.theories.Theories;
+import org.junit.experimental.theories.Theory;
+import org.junit.runner.RunWith;
+
+@RunWith(Theories.class)
+public class LfsDateTimeTest {
+ @DataPoints public static String[] timeZones = {"US/Eastern", "Asia/Tokyo", "UTC"};
+
+ @Test
+ public void formatWithDefaultTimezone() throws Exception {
+ DateTime now = DateTime.now();
+ String jodaFormat = ISODateTimeFormat.dateTime().print(now);
+ LfsDateTime formatter = LfsDateTime.instance();
+ String javaFormat = formatter.format(Instant.ofEpochMilli(now.getMillis()));
+ assertThat(javaFormat).isEqualTo(jodaFormat);
+ }
+
+ @Theory
+ public void formatWithSpecifiedTimezone(String zone) throws Exception {
+ DateTime now = DateTime.now().withZone(DateTimeZone.forID(zone));
+ String jodaFormat = ISODateTimeFormat.dateTime().withZone(DateTimeZone.forID(zone)).print(now);
+ LfsDateTime formatter = LfsDateTime.instance(TimeZone.getTimeZone(zone).toZoneId());
+ String javaFormat = formatter.format(Instant.ofEpochMilli(now.getMillis()));
+ assertThat(javaFormat).isEqualTo(jodaFormat);
+ }
+}
