Remove custom PeriodType formatting from TimeValue

In order to decouple TimeValue from Joda, this removes the unused `format` methods. Relates to elastic#28504
dakrone · Apr 9, 2018 · 2376309 · 2376309
1 parent dfcce2d
commit 2376309
Show file tree

Hide file tree

Showing 4 changed files with 8 additions and 33 deletions.
diff --git a/server/src/main/java/org/elasticsearch/common/unit/TimeValue.java b/server/src/main/java/org/elasticsearch/common/unit/TimeValue.java
@@ -24,13 +24,8 @@
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Writeable;
-import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.common.xcontent.ToXContentFragment;
 import org.elasticsearch.common.xcontent.XContentBuilder;
-import org.joda.time.Period;
-import org.joda.time.PeriodType;
-import org.joda.time.format.PeriodFormat;
-import org.joda.time.format.PeriodFormatter;
 
 import java.io.IOException;
 import java.util.Collections;
@@ -240,19 +235,6 @@ public double getDaysFrac() {
         return daysFrac();
     }
 
-    private final PeriodFormatter defaultFormatter = PeriodFormat.getDefault()
-            .withParseType(PeriodType.standard());
-
-    public String format() {
-        Period period = new Period(millis());
-        return defaultFormatter.print(period);
-    }
-
-    public String format(PeriodType type) {
-        Period period = new Period(millis());
-        return PeriodFormat.getDefault().withParseType(type).print(period);
-    }
-
     /**
      * Returns a {@link String} representation of the current {@link TimeValue}.
      *

diff --git a/server/src/main/java/org/elasticsearch/search/SearchService.java b/server/src/main/java/org/elasticsearch/search/SearchService.java
@@ -210,7 +210,7 @@ private void validateKeepAlives(TimeValue defaultKeepAlive, TimeValue maxKeepAli
         if (defaultKeepAlive.millis() > maxKeepAlive.millis()) {
             throw new IllegalArgumentException("Default keep alive setting for scroll [" + DEFAULT_KEEPALIVE_SETTING.getKey() + "]" +
                 " should be smaller than max keep alive [" + MAX_KEEPALIVE_SETTING.getKey() + "], " +
-                "was (" + defaultKeepAlive.format() + " > " + maxKeepAlive.format() + ")");
+                "was (" + defaultKeepAlive + " > " + maxKeepAlive + ")");
         }
     }
 
@@ -673,8 +673,8 @@ public void freeAllScrollContexts() {
     private void contextScrollKeepAlive(SearchContext context, long keepAlive) throws IOException {
         if (keepAlive > maxKeepAlive) {
             throw new IllegalArgumentException(
-                "Keep alive for scroll (" + TimeValue.timeValueMillis(keepAlive).format() + ") is too large. " +
-                    "It must be less than (" + TimeValue.timeValueMillis(maxKeepAlive).format() + "). " +
+                "Keep alive for scroll (" + TimeValue.timeValueMillis(keepAlive) + ") is too large. " +
+                    "It must be less than (" + TimeValue.timeValueMillis(maxKeepAlive) + "). " +
                     "This limit can be set by changing the [" + MAX_KEEPALIVE_SETTING.getKey() + "] cluster level setting.");
         }
         context.keepAlive(keepAlive);

diff --git a/server/src/test/java/org/elasticsearch/common/unit/TimeValueTests.java b/server/src/test/java/org/elasticsearch/common/unit/TimeValueTests.java
@@ -57,13 +57,6 @@ public void testToString() {
         assertThat("1000d", equalTo(new TimeValue(1000, TimeUnit.DAYS).toString()));
     }
 
-    public void testFormat() {
-        assertThat(new TimeValue(1025, TimeUnit.MILLISECONDS).format(PeriodType.dayTime()), equalTo("1 second and 25 milliseconds"));
-        assertThat(new TimeValue(1, TimeUnit.MINUTES).format(PeriodType.dayTime()), equalTo("1 minute"));
-        assertThat(new TimeValue(65, TimeUnit.MINUTES).format(PeriodType.dayTime()), equalTo("1 hour and 5 minutes"));
-        assertThat(new TimeValue(24 * 600 + 85, TimeUnit.MINUTES).format(PeriodType.dayTime()), equalTo("241 hours and 25 minutes"));
-    }
-
     public void testMinusOne() {
         assertThat(new TimeValue(-1).nanos(), lessThan(0L));
     }

diff --git a/server/src/test/java/org/elasticsearch/search/scroll/SearchScrollIT.java b/server/src/test/java/org/elasticsearch/search/scroll/SearchScrollIT.java
@@ -534,7 +534,7 @@ public void testScrollInvalidDefaultKeepAlive() throws IOException {
             client().admin().cluster().prepareUpdateSettings()
                 .setPersistentSettings(Settings.builder().put("search.max_keep_alive", "1m").put("search.default_keep_alive", "2m")).get
             ());
-        assertThat(exc.getMessage(), containsString("was (2 minutes > 1 minute)"));
+        assertThat(exc.getMessage(), containsString("was (2m > 1m)"));
 
         assertAcked(client().admin().cluster().prepareUpdateSettings()
             .setPersistentSettings(Settings.builder().put("search.default_keep_alive", "5m").put("search.max_keep_alive", "5m")).get());
@@ -548,14 +548,14 @@ public void testScrollInvalidDefaultKeepAlive() throws IOException {
 
         exc = expectThrows(IllegalArgumentException.class, () -> client().admin().cluster().prepareUpdateSettings()
             .setPersistentSettings(Settings.builder().put("search.default_keep_alive", "3m")).get());
-        assertThat(exc.getMessage(), containsString("was (3 minutes > 2 minutes)"));
+        assertThat(exc.getMessage(), containsString("was (3m > 2m)"));
 
         assertAcked(client().admin().cluster().prepareUpdateSettings()
             .setPersistentSettings(Settings.builder().put("search.default_keep_alive", "1m")).get());
 
         exc = expectThrows(IllegalArgumentException.class, () -> client().admin().cluster().prepareUpdateSettings()
             .setPersistentSettings(Settings.builder().put("search.max_keep_alive", "30s")).get());
-        assertThat(exc.getMessage(), containsString("was (1 minute > 30 seconds)"));
+        assertThat(exc.getMessage(), containsString("was (1m > 30s)"));
     }
 
     public void testInvalidScrollKeepAlive() throws IOException {
@@ -577,7 +577,7 @@ public void testInvalidScrollKeepAlive() throws IOException {
         IllegalArgumentException illegalArgumentException =
             (IllegalArgumentException) ExceptionsHelper.unwrap(exc, IllegalArgumentException.class);
         assertNotNull(illegalArgumentException);
-        assertThat(illegalArgumentException.getMessage(), containsString("Keep alive for scroll (2 hours) is too large"));
+        assertThat(illegalArgumentException.getMessage(), containsString("Keep alive for scroll (2h) is too large"));
 
         SearchResponse searchResponse = client().prepareSearch()
             .setQuery(matchAllQuery())
@@ -594,7 +594,7 @@ public void testInvalidScrollKeepAlive() throws IOException {
         illegalArgumentException =
             (IllegalArgumentException) ExceptionsHelper.unwrap(exc, IllegalArgumentException.class);
         assertNotNull(illegalArgumentException);
-        assertThat(illegalArgumentException.getMessage(), containsString("Keep alive for scroll (3 hours) is too large"));
+        assertThat(illegalArgumentException.getMessage(), containsString("Keep alive for scroll (3h) is too large"));
     }
 
     private void assertToXContentResponse(ClearScrollResponse response, boolean succeed, int numFreed) throws IOException {