codope commented on code in PR #18988:
URL: https://github.com/apache/hudi/pull/18988#discussion_r3687534826


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/callback/common/HoodieWriteCommitCallbackMessage.java:
##########
@@ -69,10 +73,91 @@ public class HoodieWriteCommitCallbackMessage implements 
Serializable {
    */
   private final Option<Map<String, String>> extraMetadata;
 
+  /**
+   * Previous base file paths keyed by fileId, derived lazily from {@link 
#hoodieWriteStat} and
+   * the {@link BaseFileOnlyView} handed over by the write client, so that 
callback
+   * implementations don't have to rebuild a view themselves. Empty for 
inserts and for
+   * callers that don't supply a view.
+   *
+   * <p>Resolution is deferred until the first {@link #getPrevFilePaths()} 
call: a callback
+   * that never reads the previous paths pays nothing (no FileSystemView 
access). Transient
+   * because it captures a FileSystemView supplier, which is not serializable 
- these paths
+   * are JVM-local derived state, so a message restored from Java 
serialization reports none.
+   * Excluded from the generated getters so the {@link Lazy} wrapper never 
leaks into JSON.
+   */
+  @Getter(AccessLevel.NONE)
+  private final transient Lazy<Map<String, PrevFilePaths>> prevFilePaths;
+
+  /**
+   * Free-form context that producers can attach for downstream callback 
consumers.
+   * The OSS write client populates this as empty; specialized callsites or 
wrappers
+   * may populate it with whatever context their callbacks need.
+   */
+  private final Map<String, String> extraContext;
+
+  public HoodieWriteCommitCallbackMessage(String commitTime,
+                                          String tableName,
+                                          String basePath,
+                                          List<HoodieWriteStat> 
hoodieWriteStat,
+                                          Option<String> commitActionType,
+                                          Option<Map<String, String>> 
extraMetadata,
+                                          Supplier<BaseFileOnlyView> 
fsViewSupplier,
+                                          Map<String, String> extraContext) {
+    this.commitTime = commitTime;
+    this.tableName = tableName;
+    this.basePath = basePath;
+    this.hoodieWriteStat = hoodieWriteStat;
+    this.commitActionType = commitActionType;
+    this.extraMetadata = extraMetadata;
+    this.prevFilePaths = Lazy.lazily(() -> 
HoodieWriteCommitCallbackUtil.resolvePrevFilePaths(
+        hoodieWriteStat, fsViewSupplier == null ? null : 
fsViewSupplier.get()));
+    this.extraContext = extraContext;
+  }
+
   public HoodieWriteCommitCallbackMessage(String commitTime,
                                           String tableName,
                                           String basePath,
                                           List<HoodieWriteStat> 
hoodieWriteStat) {
-    this(commitTime, tableName, basePath, hoodieWriteStat, Option.empty(), 
Option.empty());
+    this(commitTime, tableName, basePath, hoodieWriteStat, Option.empty(), 
Option.empty(),
+        null, Collections.emptyMap());
+  }
+
+  public HoodieWriteCommitCallbackMessage(String commitTime,
+                                          String tableName,
+                                          String basePath,
+                                          List<HoodieWriteStat> 
hoodieWriteStat,
+                                          Option<String> commitActionType,
+                                          Option<Map<String, String>> 
extraMetadata) {
+    this(commitTime, tableName, basePath, hoodieWriteStat, commitActionType, 
extraMetadata,
+        null, Collections.emptyMap());
+  }
+
+  /**
+   * Returns the previous base file paths keyed by fileId, resolving them from 
the file-system
+   * view on first access. A consumer that never calls this triggers no 
FileSystemView lookup.
+   * Never null: empty when no view was supplied, when the commit only 
inserted, or when this
+   * message was restored from Java serialization.
+   */
+  public Map<String, PrevFilePaths> getPrevFilePaths() {

Review Comment:
   Thanks @danny0405. I'll  take this and push the two-field version. One thing 
to correct though:
   > it seems the Ser/De customization is unavoidable. The PR description 
already claims this behavior, but the current implementation does not provide 
it...
   
   That PR description was outdated. Nothing actually forces the behaviour. All 
three built-in callbacks (HTTP, Kafka, Pulsar) go through convertToJsonString, 
and Jackson invokes getPrevFilePaths() while generating the payload, so the 
JSON path resolves the paths either way. Nothing in the repo Java-serializes 
the message, and prevFilePaths is new in this PR, so no consumer can be 
depending on it yet.
   
   But, i agree with your choice. The class is a `@PublicAPIClass` and 
implements `Serializable`, so a custom callback that ships the message across a 
JVM boundary would silently get an empty map rather than an error.



-- 
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]

Reply via email to