Clearer error when handling fractional time values

In 2f638b5a23, support for fractional time
values was removed. While this change is documented, the error message
presented does not give an indication that fractional inputs are not
supported. This commit fixes this by detecting when the input is a time
value that would successfully parse as a double but will not parse as a
long and presenting a clear error message that fractional time values
are not supported.

Relates #19158
This commit is contained in:
Jason Tedor 2016-06-29 13:36:11 -04:00 committed by GitHub
parent 0d81dee013
commit fc38e503e0
2 changed files with 68 additions and 31 deletions

View File

@ -317,25 +317,24 @@ public class TimeValue implements Writeable {
if (sValue == null) {
return defaultValue;
}
try {
String lowerSValue = sValue.toLowerCase(Locale.ROOT).trim();
if (lowerSValue.endsWith("nanos")) {
return new TimeValue(parse(lowerSValue, 5), TimeUnit.NANOSECONDS);
} else if (lowerSValue.endsWith("micros")) {
return new TimeValue(parse(lowerSValue, 6), TimeUnit.MICROSECONDS);
} else if (lowerSValue.endsWith("ms")) {
return new TimeValue(parse(lowerSValue, 2), TimeUnit.MILLISECONDS);
} else if (lowerSValue.endsWith("s")) {
return new TimeValue(parse(lowerSValue, 1), TimeUnit.SECONDS);
} else if (lowerSValue.endsWith("m")) {
return new TimeValue(parse(lowerSValue, 1), TimeUnit.MINUTES);
} else if (lowerSValue.endsWith("h")) {
return new TimeValue(parse(lowerSValue, 1), TimeUnit.HOURS);
} else if (lowerSValue.endsWith("d")) {
return new TimeValue(parse(lowerSValue, 1), TimeUnit.DAYS);
} else if (lowerSValue.matches("-0*1")) {
final String normalized = sValue.toLowerCase(Locale.ROOT).trim();
if (normalized.endsWith("nanos")) {
return new TimeValue(parse(sValue, normalized, 5), TimeUnit.NANOSECONDS);
} else if (normalized.endsWith("micros")) {
return new TimeValue(parse(sValue, normalized, 6), TimeUnit.MICROSECONDS);
} else if (normalized.endsWith("ms")) {
return new TimeValue(parse(sValue, normalized, 2), TimeUnit.MILLISECONDS);
} else if (normalized.endsWith("s")) {
return new TimeValue(parse(sValue, normalized, 1), TimeUnit.SECONDS);
} else if (normalized.endsWith("m")) {
return new TimeValue(parse(sValue, normalized, 1), TimeUnit.MINUTES);
} else if (normalized.endsWith("h")) {
return new TimeValue(parse(sValue, normalized, 1), TimeUnit.HOURS);
} else if (normalized.endsWith("d")) {
return new TimeValue(parse(sValue, normalized, 1), TimeUnit.DAYS);
} else if (normalized.matches("-0*1")) {
return TimeValue.MINUS_ONE;
} else if (lowerSValue.matches("0+")) {
} else if (normalized.matches("0+")) {
return TimeValue.ZERO;
} else {
// Missing units:
@ -344,13 +343,20 @@ public class TimeValue implements Writeable {
settingName,
sValue);
}
} catch (NumberFormatException e) {
throw new ElasticsearchParseException("failed to parse [{}]", e, sValue);
}
}
private static long parse(String s, int suffixLength) {
return Long.parseLong(s.substring(0, s.length() - suffixLength).trim());
private static long parse(final String initialInput, final String normalized, final int suffixLength) {
final String s = normalized.substring(0, normalized.length() - suffixLength).trim();
try {
return Long.parseLong(s);
} catch (final NumberFormatException e) {
try {
@SuppressWarnings("unused") final double ignored = Double.parseDouble(s);
throw new ElasticsearchParseException("failed to parse [{}], fractional time values are not supported", e, initialInput);
} catch (final NumberFormatException ignored) {
throw new ElasticsearchParseException("failed to parse [{}]", e, initialInput);
}
}
}
private static final long C0 = 1L;

View File

@ -30,9 +30,12 @@ import java.util.concurrent.TimeUnit;
import static org.elasticsearch.common.unit.TimeValue.timeValueNanos;
import static org.elasticsearch.common.unit.TimeValue.timeValueSeconds;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.lessThan;
import static org.hamcrest.object.HasToString.hasToString;
public class TimeValueTests extends ESTestCase {
@ -125,6 +128,34 @@ public class TimeValueTests extends ESTestCase {
assertThat(TimeValue.parseTimeValue(t.getStringRep(), null, "test"), equalTo(t));
}
private static final String FRACTIONAL_TIME_VALUES_ARE_NOT_SUPPORTED = "fractional time values are not supported";
public void testNonFractionalTimeValues() {
final String s = randomAsciiOfLength(10) + randomTimeUnit();
final ElasticsearchParseException e =
expectThrows(ElasticsearchParseException.class, () -> TimeValue.parseTimeValue(s, null, "test"));
assertThat(e, hasToString(containsString("failed to parse [" + s + "]")));
assertThat(e, not(hasToString(containsString(FRACTIONAL_TIME_VALUES_ARE_NOT_SUPPORTED))));
assertThat(e.getCause(), instanceOf(NumberFormatException.class));
}
public void testFractionalTimeValues() {
double value;
do {
value = randomDouble();
} while (value == 0);
final String s = Double.toString(randomIntBetween(0, 128) + value) + randomTimeUnit();
final ElasticsearchParseException e =
expectThrows(ElasticsearchParseException.class, () -> TimeValue.parseTimeValue(s, null, "test"));
assertThat(e, hasToString(containsString("failed to parse [" + s + "]")));
assertThat(e, hasToString(containsString(FRACTIONAL_TIME_VALUES_ARE_NOT_SUPPORTED)));
assertThat(e.getCause(), instanceOf(NumberFormatException.class));
}
private String randomTimeUnit() {
return randomFrom("nanos", "micros", "ms", "s", "m", "h", "d");
}
private void assertEqualityAfterSerialize(TimeValue value, int expectedSize) throws IOException {
BytesStreamOutput out = new BytesStreamOutput();
value.writeTo(out);