Remove custom PeriodType formatting from TimeValue (#29433)

In order to decouple TimeValue from Joda, this removes the unused `format`
methods.

Relates to #28504
This commit is contained in:
Lee Hinman 2018-04-10 08:02:56 -06:00 committed by GitHub
parent 2574064e66
commit 0f40199d10
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 8 additions and 33 deletions

View File

@ -24,13 +24,8 @@ import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.ToXContentFragment; import org.elasticsearch.common.xcontent.ToXContentFragment;
import org.elasticsearch.common.xcontent.XContentBuilder; 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.io.IOException;
import java.util.Collections; import java.util.Collections;
@ -240,19 +235,6 @@ public class TimeValue implements Writeable, Comparable<TimeValue>, ToXContentFr
return daysFrac(); 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}. * Returns a {@link String} representation of the current {@link TimeValue}.
* *

View File

@ -210,7 +210,7 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv
if (defaultKeepAlive.millis() > maxKeepAlive.millis()) { if (defaultKeepAlive.millis() > maxKeepAlive.millis()) {
throw new IllegalArgumentException("Default keep alive setting for scroll [" + DEFAULT_KEEPALIVE_SETTING.getKey() + "]" + throw new IllegalArgumentException("Default keep alive setting for scroll [" + DEFAULT_KEEPALIVE_SETTING.getKey() + "]" +
" should be smaller than max keep alive [" + MAX_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 class SearchService extends AbstractLifecycleComponent implements IndexEv
private void contextScrollKeepAlive(SearchContext context, long keepAlive) throws IOException { private void contextScrollKeepAlive(SearchContext context, long keepAlive) throws IOException {
if (keepAlive > maxKeepAlive) { if (keepAlive > maxKeepAlive) {
throw new IllegalArgumentException( throw new IllegalArgumentException(
"Keep alive for scroll (" + TimeValue.timeValueMillis(keepAlive).format() + ") is too large. " + "Keep alive for scroll (" + TimeValue.timeValueMillis(keepAlive) + ") is too large. " +
"It must be less than (" + TimeValue.timeValueMillis(maxKeepAlive).format() + "). " + "It must be less than (" + TimeValue.timeValueMillis(maxKeepAlive) + "). " +
"This limit can be set by changing the [" + MAX_KEEPALIVE_SETTING.getKey() + "] cluster level setting."); "This limit can be set by changing the [" + MAX_KEEPALIVE_SETTING.getKey() + "] cluster level setting.");
} }
context.keepAlive(keepAlive); context.keepAlive(keepAlive);

View File

@ -57,13 +57,6 @@ public class TimeValueTests extends ESTestCase {
assertThat("1000d", equalTo(new TimeValue(1000, TimeUnit.DAYS).toString())); 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() { public void testMinusOne() {
assertThat(new TimeValue(-1).nanos(), lessThan(0L)); assertThat(new TimeValue(-1).nanos(), lessThan(0L));
} }

View File

@ -534,7 +534,7 @@ public class SearchScrollIT extends ESIntegTestCase {
client().admin().cluster().prepareUpdateSettings() client().admin().cluster().prepareUpdateSettings()
.setPersistentSettings(Settings.builder().put("search.max_keep_alive", "1m").put("search.default_keep_alive", "2m")).get .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() assertAcked(client().admin().cluster().prepareUpdateSettings()
.setPersistentSettings(Settings.builder().put("search.default_keep_alive", "5m").put("search.max_keep_alive", "5m")).get()); .setPersistentSettings(Settings.builder().put("search.default_keep_alive", "5m").put("search.max_keep_alive", "5m")).get());
@ -548,14 +548,14 @@ public class SearchScrollIT extends ESIntegTestCase {
exc = expectThrows(IllegalArgumentException.class, () -> client().admin().cluster().prepareUpdateSettings() exc = expectThrows(IllegalArgumentException.class, () -> client().admin().cluster().prepareUpdateSettings()
.setPersistentSettings(Settings.builder().put("search.default_keep_alive", "3m")).get()); .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() assertAcked(client().admin().cluster().prepareUpdateSettings()
.setPersistentSettings(Settings.builder().put("search.default_keep_alive", "1m")).get()); .setPersistentSettings(Settings.builder().put("search.default_keep_alive", "1m")).get());
exc = expectThrows(IllegalArgumentException.class, () -> client().admin().cluster().prepareUpdateSettings() exc = expectThrows(IllegalArgumentException.class, () -> client().admin().cluster().prepareUpdateSettings()
.setPersistentSettings(Settings.builder().put("search.max_keep_alive", "30s")).get()); .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 { public void testInvalidScrollKeepAlive() throws IOException {
@ -577,7 +577,7 @@ public class SearchScrollIT extends ESIntegTestCase {
IllegalArgumentException illegalArgumentException = IllegalArgumentException illegalArgumentException =
(IllegalArgumentException) ExceptionsHelper.unwrap(exc, IllegalArgumentException.class); (IllegalArgumentException) ExceptionsHelper.unwrap(exc, IllegalArgumentException.class);
assertNotNull(illegalArgumentException); 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() SearchResponse searchResponse = client().prepareSearch()
.setQuery(matchAllQuery()) .setQuery(matchAllQuery())
@ -594,7 +594,7 @@ public class SearchScrollIT extends ESIntegTestCase {
illegalArgumentException = illegalArgumentException =
(IllegalArgumentException) ExceptionsHelper.unwrap(exc, IllegalArgumentException.class); (IllegalArgumentException) ExceptionsHelper.unwrap(exc, IllegalArgumentException.class);
assertNotNull(illegalArgumentException); 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 { private void assertToXContentResponse(ClearScrollResponse response, boolean succeed, int numFreed) throws IOException {