abhishekrb19 commented on code in PR #16141:
URL: https://github.com/apache/druid/pull/16141#discussion_r1527787367
##########
server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java:
##########
@@ -990,37 +993,59 @@ static boolean
isSegmentLoaded(Iterable<ImmutableSegmentLoadInfo> servedSegments
return false;
}
+ /**
+ * Either {@code interval} or {@code segmentIds} array must be specified,
but not both.
+ * {@code versions} may be optionally specified only when {@code interval}
is provided.
+ */
@VisibleForTesting
- protected static class MarkDataSourceSegmentsPayload
+ static class MarkDataSourceSegmentsPayload
{
private final Interval interval;
private final Set<String> segmentIds;
+ private final List<String> versions;
@JsonCreator
public MarkDataSourceSegmentsPayload(
- @JsonProperty("interval") Interval interval,
- @JsonProperty("segmentIds") Set<String> segmentIds
+ @JsonProperty("interval") @Nullable Interval interval,
+ @JsonProperty("segmentIds") @Nullable Set<String> segmentIds,
+ @JsonProperty("versions") @Nullable List<String> versions
)
{
this.interval = interval;
this.segmentIds = segmentIds;
+ this.versions = versions;
}
+ @Nullable
@JsonProperty
public Interval getInterval()
{
return interval;
}
+ @Nullable
@JsonProperty
public Set<String> getSegmentIds()
{
return segmentIds;
}
- public boolean isValid()
+ @Nullable
+ @JsonProperty
+ public List<String> getVersions()
+ {
+ return versions;
+ }
+
+ private boolean isValid()
{
- return (interval == null ^ segmentIds == null) && (segmentIds == null ||
!segmentIds.isEmpty());
+ if (interval == null && CollectionUtils.isNullOrEmpty(segmentIds)) {
+ return false;
+ }
+ if (interval != null && segmentIds != null) {
+ return false;
+ }
+ return CollectionUtils.isNullOrEmpty(versions) || interval != null;
Review Comment:
Yeah, one thing I noticed with the existing XOR logic in `isValid()` is that
empty segment IDs are both treated as both presence and absence of segment IDs,
which is confusing and seems like a bug to me. Let me open a PR to fix that
separately as this change would cause a couple of existing unit tests to fail.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
