Use JGit Config#getTimeUnit to parse durations
This simplifies the code and ensures more consistency with other
code using JGit. It involves some changes to semantics:
- "0 sec" is a duration now! Durations do not have to be nonzero.
- unfortunately, fractional units (like "5.2 sec") are not allowed any
more. You have to use integer expressions like "5200 ms".
- throws IllegalArgumentException instead of IllegalStateExpression
for bad units
If disallowing fractional units proves too much of a hardship, this
can be fixed in jgit.
Change-Id: I2b0053ed3f8a119cd06cb3f5dbd6a4ea5c24ac5f
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/ConfigUtil.java b/gitiles-servlet/src/main/java/com/google/gitiles/ConfigUtil.java
index 597a8e0..1915d73 100644
--- a/gitiles-servlet/src/main/java/com/google/gitiles/ConfigUtil.java
+++ b/gitiles-servlet/src/main/java/com/google/gitiles/ConfigUtil.java
@@ -14,6 +14,8 @@
package com.google.gitiles;
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+
import com.google.common.base.Optional;
import com.google.common.base.Predicates;
import com.google.common.cache.CacheBuilder;
@@ -28,6 +30,8 @@
import java.util.regex.Matcher;
import java.util.regex.Pattern;
+import javax.annotation.Nullable;
+
/** Utilities for working with {@link Config} objects. */
public class ConfigUtil {
/**
@@ -44,76 +48,10 @@
* @return a standard duration representing the time read, or defaultValue.
*/
public static Duration getDuration(
- Config config, String section, String subsection, String name, Duration defaultValue) {
- String valStr = config.getString(section, subsection, name);
- if (valStr == null) {
- return defaultValue;
- }
- valStr = valStr.trim();
- if (valStr.isEmpty()) {
- return defaultValue;
- }
- Duration val = parseDuration(valStr);
- if (val == null) {
- String key = section + (subsection != null ? "." + subsection : "") + "." + name;
- throw new IllegalStateException("Not time unit: " + key + " = " + valStr);
- }
- return val;
- }
-
- /**
- * Parse a duration value from a string.
- * <p>
- * Durations can be written with unit suffixes, for example {@code "1 s"} or
- * {@code "5 days"}. If units are not specified, milliseconds are assumed.
- *
- * @param valStr the value to parse.
- * @return a standard duration representing the time parsed, or null if not a
- * valid duration.
- */
- public static Duration parseDuration(String valStr) {
- if (valStr == null) {
- return null;
- }
- valStr = valStr.trim();
- if (valStr.isEmpty()) {
- return null;
- }
- Matcher m = matcher("^([1-9][0-9]*(?:\\.[0-9]*)?)\\s*(.*)$", valStr);
- if (!m.matches()) {
- return null;
- }
-
- String digits = m.group(1);
- String unitName = m.group(2).trim();
-
- TimeUnit unit;
- if ("".equals(unitName)) {
- unit = TimeUnit.MILLISECONDS;
- } else if (anyOf(unitName, "ms", "millis", "millisecond", "milliseconds")) {
- unit = TimeUnit.MILLISECONDS;
- } else if (anyOf(unitName, "s", "sec", "second", "seconds")) {
- unit = TimeUnit.SECONDS;
- } else if (anyOf(unitName, "m", "min", "minute", "minutes")) {
- unit = TimeUnit.MINUTES;
- } else if (anyOf(unitName, "h", "hr", "hour", "hours")) {
- unit = TimeUnit.HOURS;
- } else if (anyOf(unitName, "d", "day", "days")) {
- unit = TimeUnit.DAYS;
- } else {
- return null;
- }
-
- try {
- if (digits.indexOf('.') == -1) {
- long val = Long.parseLong(digits);
- return new Duration(val * TimeUnit.MILLISECONDS.convert(1, unit));
- }
- double val = Double.parseDouble(digits);
- return new Duration((long) (val * TimeUnit.MILLISECONDS.convert(1, unit)));
- } catch (NumberFormatException nfe) {
- return null;
- }
+ Config config, String section, String subsection, String name,
+ @Nullable Duration defaultValue) {
+ long m = config.getTimeUnit(section, subsection, name, -1, MILLISECONDS);
+ return m == -1 ? defaultValue : Duration.millis(m);
}
/**
diff --git a/gitiles-servlet/src/test/java/com/google/gitiles/ConfigUtilTest.java b/gitiles-servlet/src/test/java/com/google/gitiles/ConfigUtilTest.java
index e28b9fa..255d7b8 100644
--- a/gitiles-servlet/src/test/java/com/google/gitiles/ConfigUtilTest.java
+++ b/gitiles-servlet/src/test/java/com/google/gitiles/ConfigUtilTest.java
@@ -17,7 +17,7 @@
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gitiles.ConfigUtil.getDuration;
-import static com.google.gitiles.ConfigUtil.parseDuration;
+import static org.junit.Assert.fail;
import org.eclipse.jgit.lib.Config;
import org.joda.time.Duration;
@@ -39,8 +39,12 @@
assertThat(t.getMillis()).isEqualTo(500);
config.setString("core", "dht", "timeout", "5.2 sec");
- t = getDuration(config, "core", "dht", "timeout", def);
- assertThat(t.getMillis()).isEqualTo(5200);
+ try {
+ getDuration(config, "core", "dht", "timeout", def);
+ fail("expected IllegalArgumentException");
+ } catch (IllegalArgumentException e) {
+ assertThat(e).hasMessage("Invalid time unit value: core.dht.timeout=5.2 sec");
+ }
config.setString("core", "dht", "timeout", "1 min");
t = getDuration(config, "core", "dht", "timeout", def);
@@ -48,24 +52,37 @@
}
@Test
- public void parseDurationReturnsDuration() throws Exception {
- assertDoesNotParse(null);
- assertDoesNotParse("");
- assertDoesNotParse(" ");
- assertParses(500, "500 ms");
- assertParses(500, "500ms");
- assertParses(500, " 500 ms ");
- assertParses(5200, "5.2 sec");
- assertParses(60000, "1 min");
+ public void getDurationCanReturnDefault() throws Exception {
+ Duration def = Duration.standardSeconds(1);
+ Config config = new Config();
+ Duration t;
+
+ t = getDuration(config, "core", null, "blank", def);
+ assertThat(t.getMillis()).isEqualTo(1000);
+
+ config.setString("core", null, "blank", "");
+ t = getDuration(config, "core", null, "blank", def);
+ assertThat(t.getMillis()).isEqualTo(1000);
+
+ config.setString("core", null, "blank", " ");
+ t = getDuration(config, "core", null, "blank", def);
+ assertThat(t.getMillis()).isEqualTo(1000);
}
- private static void assertDoesNotParse(String val) {
- assertThat(parseDuration(val)).named(String.valueOf(val)).isNull();
- }
+ @Test
+ public void nullAsDefault() throws Exception {
+ Config config = new Config();
+ Duration t;
- private static void assertParses(long expectedMillis, String val) {
- Duration actual = parseDuration(checkNotNull(val));
- assertThat(actual).named(val).isNotNull();
- assertThat(actual.getMillis()).named(val).isEqualTo(expectedMillis);
+ t = getDuration(config, "core", null, "blank", null);
+ assertThat(t).isNull();
+
+ config.setString("core", null, "blank", "");
+ t = getDuration(config, "core", null, "blank", null);
+ assertThat(t).isNull();
+
+ config.setString("core", null, "blank", " ");
+ t = getDuration(config, "core", null, "blank", null);
+ assertThat(t).isNull();
}
}
