(druid) branch master updated: Bug fix: empty segment IDs cannot be both valid and invalid at the same time. (#16145)

[abhishekrb]

This is an automated email from the ASF dual-hosted git repository.

abhishekrb pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git


The following commit(s) were added to refs/heads/master by this push:
     new 3b35fb768c6 Bug fix: empty segment IDs cannot be both valid and 
invalid at the same time. (#16145)
3b35fb768c6 is described below

commit 3b35fb768c67c83b96802fde2d0d5b1c17d62633
Author: Abhishek Radhakrishnan <abhishek.r...@gmail.com>
AuthorDate: Mon Mar 18 00:47:32 2024 -0700

    Bug fix: empty segment IDs cannot be both valid and invalid at the same 
time. (#16145)
    
    Treat empty and null segment IDs as the same.
---
 .../druid/server/http/DataSourcesResource.java     |  8 +++-
 .../druid/server/http/DataSourcesResourceTest.java | 52 ++++++++++++++++------
 2 files changed, 45 insertions(+), 15 deletions(-)

diff --git 
a/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java 
b/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java
index d0458243fcf..a633640f0e2 100644
--- a/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java
+++ b/server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java
@@ -68,6 +68,7 @@ import org.apache.druid.timeline.TimelineLookup;
 import org.apache.druid.timeline.TimelineObjectHolder;
 import org.apache.druid.timeline.VersionedIntervalTimeline;
 import org.apache.druid.timeline.partition.PartitionChunk;
+import org.apache.druid.utils.CollectionUtils;
 import org.joda.time.DateTime;
 import org.joda.time.Interval;
 
@@ -1020,7 +1021,12 @@ public class DataSourcesResource
 
     public boolean isValid()
     {
-      return (interval == null ^ segmentIds == null) && (segmentIds == null || 
!segmentIds.isEmpty());
+      final boolean hasSegmentIds = !CollectionUtils.isNullOrEmpty(segmentIds);
+      if (interval == null) {
+        return hasSegmentIds;
+      } else {
+        return !hasSegmentIds;
+      }
     }
   }
 }
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
index ace68ad3c21..bd673f078b2 100644
--- 
a/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java
+++ 
b/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java
@@ -826,7 +826,7 @@ public class DataSourcesResourceTest
   }
 
   @Test
-  public void testMarkAsUsedNonOvershadowedSegmentsInvalidPayloadNoArguments()
+  public void 
testMarkAsUsedNonOvershadowedSegmentsWithNullIntervalAndSegmentIds()
   {
     DataSourcesResource dataSourcesResource = createResource();
 
@@ -838,7 +838,7 @@ public class DataSourcesResourceTest
   }
 
   @Test
-  public void 
testMarkAsUsedNonOvershadowedSegmentsInvalidPayloadBothArguments()
+  public void 
testMarkAsUsedNonOvershadowedSegmentsWithNonNullIntervalAndEmptySegmentIds()
   {
     DataSourcesResource dataSourcesResource = createResource();
 
@@ -846,11 +846,37 @@ public class DataSourcesResourceTest
         "datasource1",
         new 
DataSourcesResource.MarkDataSourceSegmentsPayload(Intervals.of("2010-01-22/P1D"),
 ImmutableSet.of())
     );
+    Assert.assertEquals(200, response.getStatus());
+    Assert.assertEquals(ImmutableMap.of("numChangedSegments", 0), 
response.getEntity());
+  }
+
+  @Test
+  public void 
testMarkAsUsedNonOvershadowedSegmentsWithNonNullIntervalAndNullSegmentIds()
+  {
+    DataSourcesResource dataSourcesResource = createResource();
+
+    Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments(
+        "datasource1",
+        new 
DataSourcesResource.MarkDataSourceSegmentsPayload(Intervals.of("2010-01-22/P1D"),
 null)
+    );
+    Assert.assertEquals(200, response.getStatus());
+    Assert.assertEquals(ImmutableMap.of("numChangedSegments", 0), 
response.getEntity());
+  }
+
+  @Test
+  public void 
testMarkAsUsedNonOvershadowedSegmentsWithNonNullIntervalAndSegmentIds()
+  {
+    DataSourcesResource dataSourcesResource = createResource();
+
+    Response response = dataSourcesResource.markAsUsedNonOvershadowedSegments(
+        "datasource1",
+        new 
DataSourcesResource.MarkDataSourceSegmentsPayload(Intervals.of("2010-01-22/P1D"),
 ImmutableSet.of("segment1"))
+    );
     Assert.assertEquals(400, response.getStatus());
   }
 
   @Test
-  public void testMarkAsUsedNonOvershadowedSegmentsInvalidPayloadEmptyArray()
+  public void 
testMarkAsUsedNonOvershadowedSegmentsWithNullIntervalAndEmptySegmentIds()
   {
     DataSourcesResource dataSourcesResource = createResource();
 
@@ -1170,8 +1196,7 @@ public class DataSourcesResourceTest
   @Test
   public void testMarkSegmentsAsUnusedNullPayload()
   {
-    DataSourcesResource dataSourcesResource =
-        createResource();
+    DataSourcesResource dataSourcesResource = createResource();
 
     Response response = 
dataSourcesResource.markSegmentsAsUnused("datasource1", null, request);
     Assert.assertEquals(400, response.getStatus());
@@ -1183,10 +1208,9 @@ public class DataSourcesResourceTest
   }
 
   @Test
-  public void testMarkSegmentsAsUnusedInvalidPayload()
+  public void testMarkSegmentsAsUnusedWithNullIntervalAndSegmentIds()
   {
-    DataSourcesResource dataSourcesResource =
-        createResource();
+    DataSourcesResource dataSourcesResource = createResource();
 
     final DataSourcesResource.MarkDataSourceSegmentsPayload payload =
         new DataSourcesResource.MarkDataSourceSegmentsPayload(null, null);
@@ -1197,17 +1221,17 @@ public class DataSourcesResourceTest
   }
 
   @Test
-  public void testMarkSegmentsAsUnusedInvalidPayloadBothArguments()
+  public void testMarkSegmentsAsUnusedWithNonNullIntervalAndEmptySegmentIds()
   {
-    DataSourcesResource dataSourcesResource =
-        createResource();
+    DataSourcesResource dataSourcesResource = createResource();
 
     final DataSourcesResource.MarkDataSourceSegmentsPayload payload =
         new 
DataSourcesResource.MarkDataSourceSegmentsPayload(Intervals.of("2010-01-01/P1D"),
 ImmutableSet.of());
-
+    prepareRequestForAudit();
     Response response = 
dataSourcesResource.markSegmentsAsUnused("datasource1", payload, request);
-    Assert.assertEquals(400, response.getStatus());
-    Assert.assertNotNull(response.getEntity());
+
+    Assert.assertEquals(200, response.getStatus());
+    Assert.assertEquals(ImmutableMap.of("numChangedSegments", 0), 
response.getEntity());
   }
 
   @Test


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org