Throw exception if legacy interval cannot be parsed (#41972)

Due to the fallthrough logic, DateIntervalWrapper assumed that an
otherwise unparsable interval was a legacy fixed millis interval. This
could then NPE if the interval was just illegal ("foobar").

This commit correctly checks if the legacy millis parsing fails too,
and throws an IllegalArgumentException at that point signaling the
provided interval is bad.

Author: Zachary Tong

server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateIntervalWrapper.java

@@ -288,11 +288,15 @@ public Rounding createRounding(ZoneId timeZone) {
         } else {
             // We're not sure what the interval was originally (legacy) so use old behavior of assuming
             // calendar first, then fixed.  Required because fixed/cal overlap in places ("1h")
-            DateTimeUnit intervalAsUnit = tryIntervalAsCalendarUnit();
-            if (intervalAsUnit != null) {
-                tzRoundingBuilder = Rounding.builder(tryIntervalAsCalendarUnit());
+            DateTimeUnit calInterval = tryIntervalAsCalendarUnit();
+            TimeValue fixedInterval = tryIntervalAsFixedUnit();
+            if (calInterval != null) {
+                tzRoundingBuilder = Rounding.builder(calInterval);
+            } else if (fixedInterval != null) {
+                tzRoundingBuilder = Rounding.builder(fixedInterval);
             } else {
-                tzRoundingBuilder = Rounding.builder(tryIntervalAsFixedUnit());
+                // If we get here we have exhausted our options and are not able to parse this interval
+                throw new IllegalArgumentException("Unable to parse interval [" + dateHistogramInterval + "]");
             }
         }
         if (timeZone != null) {

server/src/test/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregatorTests.java

@@ -1097,6 +1097,16 @@ public void testLegacyThenNew() throws IOException {
         assertWarnings("[interval] on [date_histogram] is deprecated, use [fixed_interval] or [calendar_interval] in the future.");
     }
 
+    public void testIllegalInterval() throws IOException {
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> testSearchCase(new MatchAllDocsQuery(),
+            Collections.emptyList(),
+            aggregation -> aggregation.dateHistogramInterval(new DateHistogramInterval("foobar")).field(DATE_FIELD),
+            histogram -> {}
+        ));
+        assertThat(e.getMessage(), equalTo("Unable to parse interval [foobar]"));
+        assertWarnings("[interval] on [date_histogram] is deprecated, use [fixed_interval] or [calendar_interval] in the future.");
+    }
+
     private void testSearchCase(Query query, List<String> dataset,
                                 Consumer<DateHistogramAggregationBuilder> configure,
                                 Consumer<InternalDateHistogram> verify) throws IOException {

x-pack/plugin/src/test/resources/rest-api-spec/test/rollup/rollup_search.yml

@@ -1246,9 +1246,7 @@ setup:
 
 ---
 "Search error against live index":
-  - skip:
-      version: "all"
-      reason:  "AwaitsFix: https://github.com/elastic/elasticsearch/issues/41970"
+
   - do:
       catch: bad_request
       rollup.rollup_search:
@@ -1264,9 +1262,7 @@ setup:
 
 ---
 "Search error against rollup and live index":
-  - skip:
-      version: "all"
-      reason:  "AwaitsFix: https://github.com/elastic/elasticsearch/issues/41970"
+
   - do:
       catch: bad_request
       rollup.rollup_search: