roryqi commented on code in PR #13801:
URL: https://github.com/apache/gravitino/pull/13801#discussion_r4270716983
##########
common/src/main/java/org/apache/gravitino/dto/policy/PolicyContentDTO.java:
##########
@@ -203,6 +224,29 @@ public Map<String, String> rewriteOptions() {
: Collections.unmodifiableMap(new LinkedHashMap<>(rewriteOptions));
}
+ @Nullable
+ public String timeTrigger() {
+ return timeTrigger;
+ }
+
+ @Nullable
+ public Boolean commitTrigger() {
+ return commitTrigger;
+ }
+
+ @Nullable
+ public Long minIntervalMs() {
+ return minIntervalMs;
+ }
+
+ public Map<String, String> jobOptions() {
+ return jobOptions == null ? Collections.emptyMap() : jobOptions;
Review Comment:
`jobOptions()` returns the DTO's mutable backing map, unlike
`rewriteOptions()` above, which returns an unmodifiable defensive copy. A
caller can therefore mutate the DTO after validation and also change its
Lombok-generated equality/hash state. Please return an immutable defensive copy
here and for `updateOptions()` in all four DTOs. Centralizing these fields in
an immutable execution-spec DTO would also remove this repetition.
##########
common/src/main/java/org/apache/gravitino/dto/policy/PolicyContentDTO.java:
##########
@@ -203,6 +224,29 @@ public Map<String, String> rewriteOptions() {
: Collections.unmodifiableMap(new LinkedHashMap<>(rewriteOptions));
}
+ @Nullable
+ public String timeTrigger() {
Review Comment:
These five new public accessors have no Javadocs, and the same pattern is
repeated in the other execution-capable DTOs. Please document the new public
API methods (including null/default semantics), or centralize them behind the
proposed execution-spec abstraction.
--
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]
