Skip to content

Commit 04a85db

Browse files
committed
Remove custom PeriodType formatting from TimeValue (#29433)
In order to decouple TimeValue from Joda, this removes the unused `format` methods. Relates to #28504
1 parent 2dcf183 commit 04a85db

File tree

4 files changed

+8
-33
lines changed

4 files changed

+8
-33
lines changed

server/src/main/java/org/elasticsearch/common/unit/TimeValue.java

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,8 @@
2424
import org.elasticsearch.common.io.stream.StreamInput;
2525
import org.elasticsearch.common.io.stream.StreamOutput;
2626
import org.elasticsearch.common.io.stream.Writeable;
27-
import org.elasticsearch.common.xcontent.ToXContent;
2827
import org.elasticsearch.common.xcontent.ToXContentFragment;
2928
import org.elasticsearch.common.xcontent.XContentBuilder;
30-
import org.joda.time.Period;
31-
import org.joda.time.PeriodType;
32-
import org.joda.time.format.PeriodFormat;
33-
import org.joda.time.format.PeriodFormatter;
3429

3530
import java.io.IOException;
3631
import java.util.Collections;
@@ -240,19 +235,6 @@ public double getDaysFrac() {
240235
return daysFrac();
241236
}
242237

243-
private final PeriodFormatter defaultFormatter = PeriodFormat.getDefault()
244-
.withParseType(PeriodType.standard());
245-
246-
public String format() {
247-
Period period = new Period(millis());
248-
return defaultFormatter.print(period);
249-
}
250-
251-
public String format(PeriodType type) {
252-
Period period = new Period(millis());
253-
return PeriodFormat.getDefault().withParseType(type).print(period);
254-
}
255-
256238
/**
257239
* Returns a {@link String} representation of the current {@link TimeValue}.
258240
*

server/src/main/java/org/elasticsearch/search/SearchService.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ private void validateKeepAlives(TimeValue defaultKeepAlive, TimeValue maxKeepAli
211211
if (defaultKeepAlive.millis() > maxKeepAlive.millis()) {
212212
throw new IllegalArgumentException("Default keep alive setting for scroll [" + DEFAULT_KEEPALIVE_SETTING.getKey() + "]" +
213213
" should be smaller than max keep alive [" + MAX_KEEPALIVE_SETTING.getKey() + "], " +
214-
"was (" + defaultKeepAlive.format() + " > " + maxKeepAlive.format() + ")");
214+
"was (" + defaultKeepAlive + " > " + maxKeepAlive + ")");
215215
}
216216
}
217217

@@ -673,8 +673,8 @@ public void freeAllScrollContexts() {
673673
private void contextScrollKeepAlive(SearchContext context, long keepAlive) throws IOException {
674674
if (keepAlive > maxKeepAlive) {
675675
throw new QueryPhaseExecutionException(context,
676-
"Keep alive for scroll (" + TimeValue.timeValueMillis(keepAlive).format() + ") is too large. " +
677-
"It must be less than (" + TimeValue.timeValueMillis(maxKeepAlive).format() + "). " +
676+
"Keep alive for scroll (" + TimeValue.timeValueMillis(keepAlive) + ") is too large. " +
677+
"It must be less than (" + TimeValue.timeValueMillis(maxKeepAlive) + "). " +
678678
"This limit can be set by changing the [" + MAX_KEEPALIVE_SETTING.getKey() + "] cluster level setting.");
679679
}
680680
context.keepAlive(keepAlive);

server/src/test/java/org/elasticsearch/common/unit/TimeValueTests.java

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,13 +57,6 @@ public void testToString() {
5757
assertThat("1000d", equalTo(new TimeValue(1000, TimeUnit.DAYS).toString()));
5858
}
5959

60-
public void testFormat() {
61-
assertThat(new TimeValue(1025, TimeUnit.MILLISECONDS).format(PeriodType.dayTime()), equalTo("1 second and 25 milliseconds"));
62-
assertThat(new TimeValue(1, TimeUnit.MINUTES).format(PeriodType.dayTime()), equalTo("1 minute"));
63-
assertThat(new TimeValue(65, TimeUnit.MINUTES).format(PeriodType.dayTime()), equalTo("1 hour and 5 minutes"));
64-
assertThat(new TimeValue(24 * 600 + 85, TimeUnit.MINUTES).format(PeriodType.dayTime()), equalTo("241 hours and 25 minutes"));
65-
}
66-
6760
public void testMinusOne() {
6861
assertThat(new TimeValue(-1).nanos(), lessThan(0L));
6962
}

server/src/test/java/org/elasticsearch/search/scroll/SearchScrollIT.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -536,7 +536,7 @@ public void testScrollInvalidDefaultKeepAlive() throws IOException {
536536
client().admin().cluster().prepareUpdateSettings()
537537
.setPersistentSettings(Settings.builder().put("search.max_keep_alive", "1m").put("search.default_keep_alive", "2m")).get
538538
());
539-
assertThat(exc.getMessage(), containsString("was (2 minutes > 1 minute)"));
539+
assertThat(exc.getMessage(), containsString("was (2m > 1m)"));
540540

541541
assertAcked(client().admin().cluster().prepareUpdateSettings()
542542
.setPersistentSettings(Settings.builder().put("search.default_keep_alive", "5m").put("search.max_keep_alive", "5m")).get());
@@ -550,14 +550,14 @@ public void testScrollInvalidDefaultKeepAlive() throws IOException {
550550

551551
exc = expectThrows(IllegalArgumentException.class, () -> client().admin().cluster().prepareUpdateSettings()
552552
.setPersistentSettings(Settings.builder().put("search.default_keep_alive", "3m")).get());
553-
assertThat(exc.getMessage(), containsString("was (3 minutes > 2 minutes)"));
553+
assertThat(exc.getMessage(), containsString("was (3m > 2m)"));
554554

555555
assertAcked(client().admin().cluster().prepareUpdateSettings()
556556
.setPersistentSettings(Settings.builder().put("search.default_keep_alive", "1m")).get());
557557

558558
exc = expectThrows(IllegalArgumentException.class, () -> client().admin().cluster().prepareUpdateSettings()
559559
.setPersistentSettings(Settings.builder().put("search.max_keep_alive", "30s")).get());
560-
assertThat(exc.getMessage(), containsString("was (1 minute > 30 seconds)"));
560+
assertThat(exc.getMessage(), containsString("was (1m > 30s)"));
561561
}
562562

563563
public void testInvalidScrollKeepAlive() throws IOException {
@@ -579,7 +579,7 @@ public void testInvalidScrollKeepAlive() throws IOException {
579579
QueryPhaseExecutionException queryPhaseExecutionException =
580580
(QueryPhaseExecutionException) ExceptionsHelper.unwrap(exc, QueryPhaseExecutionException.class);
581581
assertNotNull(queryPhaseExecutionException);
582-
assertThat(queryPhaseExecutionException.getMessage(), containsString("Keep alive for scroll (2 hours) is too large"));
582+
assertThat(queryPhaseExecutionException.getMessage(), containsString("Keep alive for scroll (2h) is too large"));
583583

584584
SearchResponse searchResponse = client().prepareSearch()
585585
.setQuery(matchAllQuery())
@@ -596,7 +596,7 @@ public void testInvalidScrollKeepAlive() throws IOException {
596596
queryPhaseExecutionException =
597597
(QueryPhaseExecutionException) ExceptionsHelper.unwrap(exc, QueryPhaseExecutionException.class);
598598
assertNotNull(queryPhaseExecutionException);
599-
assertThat(queryPhaseExecutionException.getMessage(), containsString("Keep alive for scroll (3 hours) is too large"));
599+
assertThat(queryPhaseExecutionException.getMessage(), containsString("Keep alive for scroll (3h) is too large"));
600600
}
601601

602602
private void assertToXContentResponse(ClearScrollResponse response, boolean succeed, int numFreed) throws IOException {

0 commit comments

Comments
 (0)