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.
This commit is contained in:
Abhishek Radhakrishnan 2024-03-18 00:47:32 -07:00 committed by GitHub
parent 86a24012a6
commit 3b35fb768c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 45 additions and 15 deletions

View File

@ -68,6 +68,7 @@ import org.apache.druid.timeline.TimelineLookup;
import org.apache.druid.timeline.TimelineObjectHolder; import org.apache.druid.timeline.TimelineObjectHolder;
import org.apache.druid.timeline.VersionedIntervalTimeline; import org.apache.druid.timeline.VersionedIntervalTimeline;
import org.apache.druid.timeline.partition.PartitionChunk; import org.apache.druid.timeline.partition.PartitionChunk;
import org.apache.druid.utils.CollectionUtils;
import org.joda.time.DateTime; import org.joda.time.DateTime;
import org.joda.time.Interval; import org.joda.time.Interval;
@ -1020,7 +1021,12 @@ public class DataSourcesResource
public boolean isValid() 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;
}
} }
} }
} }

View File

@ -826,7 +826,7 @@ public class DataSourcesResourceTest
} }
@Test @Test
public void testMarkAsUsedNonOvershadowedSegmentsInvalidPayloadNoArguments() public void testMarkAsUsedNonOvershadowedSegmentsWithNullIntervalAndSegmentIds()
{ {
DataSourcesResource dataSourcesResource = createResource(); DataSourcesResource dataSourcesResource = createResource();
@ -838,7 +838,7 @@ public class DataSourcesResourceTest
} }
@Test @Test
public void testMarkAsUsedNonOvershadowedSegmentsInvalidPayloadBothArguments() public void testMarkAsUsedNonOvershadowedSegmentsWithNonNullIntervalAndEmptySegmentIds()
{ {
DataSourcesResource dataSourcesResource = createResource(); DataSourcesResource dataSourcesResource = createResource();
@ -846,11 +846,37 @@ public class DataSourcesResourceTest
"datasource1", "datasource1",
new DataSourcesResource.MarkDataSourceSegmentsPayload(Intervals.of("2010-01-22/P1D"), ImmutableSet.of()) 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()); Assert.assertEquals(400, response.getStatus());
} }
@Test @Test
public void testMarkAsUsedNonOvershadowedSegmentsInvalidPayloadEmptyArray() public void testMarkAsUsedNonOvershadowedSegmentsWithNullIntervalAndEmptySegmentIds()
{ {
DataSourcesResource dataSourcesResource = createResource(); DataSourcesResource dataSourcesResource = createResource();
@ -1170,8 +1196,7 @@ public class DataSourcesResourceTest
@Test @Test
public void testMarkSegmentsAsUnusedNullPayload() public void testMarkSegmentsAsUnusedNullPayload()
{ {
DataSourcesResource dataSourcesResource = DataSourcesResource dataSourcesResource = createResource();
createResource();
Response response = dataSourcesResource.markSegmentsAsUnused("datasource1", null, request); Response response = dataSourcesResource.markSegmentsAsUnused("datasource1", null, request);
Assert.assertEquals(400, response.getStatus()); Assert.assertEquals(400, response.getStatus());
@ -1183,10 +1208,9 @@ public class DataSourcesResourceTest
} }
@Test @Test
public void testMarkSegmentsAsUnusedInvalidPayload() public void testMarkSegmentsAsUnusedWithNullIntervalAndSegmentIds()
{ {
DataSourcesResource dataSourcesResource = DataSourcesResource dataSourcesResource = createResource();
createResource();
final DataSourcesResource.MarkDataSourceSegmentsPayload payload = final DataSourcesResource.MarkDataSourceSegmentsPayload payload =
new DataSourcesResource.MarkDataSourceSegmentsPayload(null, null); new DataSourcesResource.MarkDataSourceSegmentsPayload(null, null);
@ -1197,17 +1221,17 @@ public class DataSourcesResourceTest
} }
@Test @Test
public void testMarkSegmentsAsUnusedInvalidPayloadBothArguments() public void testMarkSegmentsAsUnusedWithNonNullIntervalAndEmptySegmentIds()
{ {
DataSourcesResource dataSourcesResource = DataSourcesResource dataSourcesResource = createResource();
createResource();
final DataSourcesResource.MarkDataSourceSegmentsPayload payload = final DataSourcesResource.MarkDataSourceSegmentsPayload payload =
new DataSourcesResource.MarkDataSourceSegmentsPayload(Intervals.of("2010-01-01/P1D"), ImmutableSet.of()); new DataSourcesResource.MarkDataSourceSegmentsPayload(Intervals.of("2010-01-01/P1D"), ImmutableSet.of());
prepareRequestForAudit();
Response response = dataSourcesResource.markSegmentsAsUnused("datasource1", payload, request); 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 @Test