Skip to content

Commit

Permalink
Fix edge cases in CompositeKeyExtractorTests (#30175)
Browse files Browse the repository at this point in the history
Currently the test picks random java.util.TimeZone ids in some places.
Internally we still need to convert back to joda DateTimeZone by id
occassionally (e.g. when serializing to pre 6.3 versions). There are
some deprecated "SystemV/*" time zones that Jodas DateTimeZone refuses
to convert. This change excludes those rare cases from the set of
allowed random time zones. It would be quiet odd for them to appear in
practice.

Closes #30156
  • Loading branch information
Christoph Büscher authored and costin committed Apr 26, 2018
1 parent c4fb372 commit 982f7d7
Showing 1 changed file with 13 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import org.joda.time.DateTimeZone;

import java.io.IOException;
import java.util.TimeZone;

import static java.util.Arrays.asList;
import static java.util.Collections.emptyList;
Expand All @@ -24,7 +25,7 @@
public class CompositeKeyExtractorTests extends AbstractWireSerializingTestCase<CompositeKeyExtractor> {

public static CompositeKeyExtractor randomCompositeKeyExtractor() {
return new CompositeKeyExtractor(randomAlphaOfLength(16), randomFrom(asList(Property.values())), randomTimeZone());
return new CompositeKeyExtractor(randomAlphaOfLength(16), randomFrom(asList(Property.values())), randomSafeTimeZone());
}

@Override
Expand Down Expand Up @@ -59,7 +60,7 @@ public void testExtractKey() {

@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/30156")
public void testExtractDate() {
CompositeKeyExtractor extractor = new CompositeKeyExtractor(randomAlphaOfLength(16), Property.VALUE, randomTimeZone());
CompositeKeyExtractor extractor = new CompositeKeyExtractor(randomAlphaOfLength(16), Property.VALUE, randomSafeTimeZone());

long millis = System.currentTimeMillis();
Bucket bucket = new TestBucket(singletonMap(extractor.key(), millis), randomLong(), new Aggregations(emptyList()));
Expand All @@ -74,4 +75,13 @@ public void testExtractIncorrectDateKey() {
SqlIllegalArgumentException exception = expectThrows(SqlIllegalArgumentException.class, () -> extractor.extract(bucket));
assertEquals("Invalid date key returned: " + value, exception.getMessage());
}
}

/**
* We need to exclude SystemV/* time zones because they cannot be converted
* back to DateTimeZone which we currently still need to do internally,
* e.g. in bwc serialization and in the extract() method
*/
private static TimeZone randomSafeTimeZone() {
return randomValueOtherThanMany(tz -> tz.getID().startsWith("SystemV"), () -> randomTimeZone());
}
}

0 comments on commit 982f7d7

Please sign in to comment.