Fix SQL Innterval.of() error message

apache · Jan 16, 2024 · ae27630 · ae27630
1 parent 18d2a89
commit ae27630
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 52 deletions.
diff --git a/processing/src/main/java/org/apache/druid/java/util/common/Intervals.java b/processing/src/main/java/org/apache/druid/java/util/common/Intervals.java
@@ -20,6 +20,7 @@
 package org.apache.druid.java.util.common;
 
 import com.google.common.collect.ImmutableList;
+import org.apache.druid.error.InvalidInput;
 import org.apache.druid.java.util.common.guava.Comparators;
 import org.joda.time.DateTime;
 import org.joda.time.Interval;
@@ -39,7 +40,12 @@ public static Interval utc(long startInstant, long endInstant)
 
   public static Interval of(String interval)
   {
-    return new Interval(interval, ISOChronology.getInstanceUTC());
+    try {
+      return new Interval(interval, ISOChronology.getInstanceUTC());
+    }
+    catch (IllegalArgumentException e) {
+      throw InvalidInput.exception(e, "Bad Interval[%s]: [%s]", interval, e.getMessage());
+    }
   }
 
   public static Interval of(String format, Object... formatArgs)

diff --git a/processing/src/test/java/org/apache/druid/java/util/common/IntervalsTest.java b/processing/src/test/java/org/apache/druid/java/util/common/IntervalsTest.java
@@ -19,6 +19,7 @@
 
 package org.apache.druid.java.util.common;
 
+import org.apache.druid.error.DruidExceptionMatcher;
 import org.apache.druid.java.util.common.guava.Comparators;
 import org.joda.time.Interval;
 import org.junit.Assert;
@@ -78,4 +79,11 @@ public void testFindOverlappingInterval()
     );
   }
 
+  @Test
+  public void testInvalidInterval()
+  {
+    DruidExceptionMatcher.invalidInput().assertThrowsAndMatches(
+        () -> Intervals.of("invalid string")
+    );
+  }
 }
diff --git a/server/src/test/java/org/apache/druid/metadata/SqlSegmentsMetadataManagerTest.java b/server/src/test/java/org/apache/druid/metadata/SqlSegmentsMetadataManagerTest.java
@@ -697,25 +697,6 @@ public void testMarkAsUsedNonOvershadowedSegmentsInInterval() throws IOException
     );
   }
 
-  @Test(expected = IllegalArgumentException.class)
-  public void testMarkAsUsedNonOvershadowedSegmentsInIntervalWithInvalidInterval() throws IOException
-  {
-    sqlSegmentsMetadataManager.startPollingDatabasePeriodically();
-    sqlSegmentsMetadataManager.poll();
-    Assert.assertTrue(sqlSegmentsMetadataManager.isPollingDatabasePeriodically());
-
-    final String newDataSource = "wikipedia2";
-    final DataSegment newSegment1 = createNewSegment1(newDataSource);
-
-    final DataSegment newSegment2 = createNewSegment2(newDataSource);
-
-    publish(newSegment1, false);
-    publish(newSegment2, false);
-    // invalid interval start > end
-    final Interval theInterval = Intervals.of("2017-10-22T00:00:00.000/2017-10-02T00:00:00.000");
-    sqlSegmentsMetadataManager.markAsUsedNonOvershadowedSegmentsInInterval(newDataSource, theInterval);
-  }
-
   @Test
   public void testMarkAsUsedNonOvershadowedSegmentsInIntervalWithOverlappingInterval() throws IOException
   {
@@ -833,25 +814,6 @@ public void testMarkAsUnusedSegmentsInInterval() throws IOException
     );
   }
 
-  @Test(expected = IllegalArgumentException.class)
-  public void testMarkAsUnusedSegmentsInIntervalWithInvalidInterval() throws IOException
-  {
-    sqlSegmentsMetadataManager.startPollingDatabasePeriodically();
-    sqlSegmentsMetadataManager.poll();
-    Assert.assertTrue(sqlSegmentsMetadataManager.isPollingDatabasePeriodically());
-
-    final String newDataSource = "wikipedia2";
-    final DataSegment newSegment1 = createNewSegment1(newDataSource);
-
-    final DataSegment newSegment2 = createNewSegment2(newDataSource);
-
-    publisher.publishSegment(newSegment1);
-    publisher.publishSegment(newSegment2);
-    // invalid interval start > end
-    final Interval theInterval = Intervals.of("2017-10-22T00:00:00.000/2017-10-02T00:00:00.000");
-    sqlSegmentsMetadataManager.markAsUnusedSegmentsInInterval(newDataSource, theInterval);
-  }
-
   @Test
   public void testMarkAsUnusedSegmentsInIntervalWithOverlappingInterval() throws IOException
   {

diff --git a/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java b/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java
@@ -35,6 +35,7 @@
 import org.apache.druid.client.ImmutableDruidDataSource;
 import org.apache.druid.client.ImmutableSegmentLoadInfo;
 import org.apache.druid.client.SegmentLoadInfo;
+import org.apache.druid.error.DruidExceptionMatcher;
 import org.apache.druid.java.util.common.Intervals;
 import org.apache.druid.metadata.MetadataRuleManager;
 import org.apache.druid.metadata.SegmentsMetadataManager;
@@ -611,19 +612,9 @@ public void testMarkAsUnusedAllSegmentsInDataSourceBadRequest()
     EasyMock.replay(overlordClient, server);
     DataSourcesResource dataSourcesResource =
         new DataSourcesResource(inventoryView, null, null, overlordClient, null, null, auditManager);
-    try {
-      Response response =
-          dataSourcesResource.markAsUnusedAllSegmentsOrKillUnusedSegmentsInInterval("datasource", "true", "???", request);
-      // 400 (Bad Request) or an IllegalArgumentException is expected.
-      Assert.assertEquals(400, response.getStatus());
-      Assert.assertNotNull(response.getEntity());
-      Assert.assertTrue(response.getEntity().toString().contains("java.lang.IllegalArgumentException"));
-    }
-    catch (IllegalArgumentException ignore) {
-      // expected
-    }
-
-    EasyMock.verify(overlordClient, server);
+    DruidExceptionMatcher.invalidInput().assertThrowsAndMatches(
+        () -> dataSourcesResource.markAsUnusedAllSegmentsOrKillUnusedSegmentsInInterval("datasource", "true", "???", request)
+    );
   }
 
   @Test