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

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


The following commit(s) were added to refs/heads/master by this push:
     new c3d16b2164 [GOBBLIN-2155] Fix bug where empty delete activity result 
was not jackson deserializ… (#4054)
c3d16b2164 is described below

commit c3d16b216417178b944551a9d9689542f06aee31
Author: William Lo <[email protected]>
AuthorDate: Fri Sep 13 14:55:06 2024 -0400

    [GOBBLIN-2155] Fix bug where empty delete activity result was not jackson 
deserializ… (#4054)
    
    * Fix bug where empty delete activity result was not jackson deserializable
---
 .../activity/impl/DeleteWorkDirsActivityImpl.java  |  2 +-
 .../temporal/ddm/work/DirDeletionResult.java       | 13 ++++++
 .../impl/DeleteWorkDirsActivityImplTest.java       | 49 ++++++++++++++++++++++
 3 files changed, 63 insertions(+), 1 deletion(-)

diff --git 
a/gobblin-temporal/src/main/java/org/apache/gobblin/temporal/ddm/activity/impl/DeleteWorkDirsActivityImpl.java
 
b/gobblin-temporal/src/main/java/org/apache/gobblin/temporal/ddm/activity/impl/DeleteWorkDirsActivityImpl.java
index 6b14a70156..36db274bd6 100644
--- 
a/gobblin-temporal/src/main/java/org/apache/gobblin/temporal/ddm/activity/impl/DeleteWorkDirsActivityImpl.java
+++ 
b/gobblin-temporal/src/main/java/org/apache/gobblin/temporal/ddm/activity/impl/DeleteWorkDirsActivityImpl.java
@@ -50,7 +50,7 @@ public class DeleteWorkDirsActivityImpl implements 
DeleteWorkDirsActivity {
     // Ensure that non HDFS writers exit early as they rely on a different 
cleanup process, can consider consolidation in the future
     // through an abstracted cleanup method implemented at a writer level
     if (workDirPaths.isEmpty()) {
-      return new DirDeletionResult();
+      return DirDeletionResult.createEmpty();
     }
     //TODO: Emit timers to measure length of cleanup step
     Optional<String> optJobName = Optional.empty();
diff --git 
a/gobblin-temporal/src/main/java/org/apache/gobblin/temporal/ddm/work/DirDeletionResult.java
 
b/gobblin-temporal/src/main/java/org/apache/gobblin/temporal/ddm/work/DirDeletionResult.java
index 257e5b5772..33883432c8 100644
--- 
a/gobblin-temporal/src/main/java/org/apache/gobblin/temporal/ddm/work/DirDeletionResult.java
+++ 
b/gobblin-temporal/src/main/java/org/apache/gobblin/temporal/ddm/work/DirDeletionResult.java
@@ -17,6 +17,7 @@
 
 package org.apache.gobblin.temporal.ddm.work;
 
+import java.util.HashMap;
 import java.util.Map;
 
 import lombok.Data;
@@ -34,4 +35,16 @@ import lombok.RequiredArgsConstructor;
 public class DirDeletionResult {
 
   @NonNull private Map<String, Boolean> successesByDirPath;
+
+  /**
+   * Empty result that should be used instead of empty constructor and needed 
to support jackson (de)serialization, otherwise will face the following error
+   * Caused by: io.temporal.common.converter.DataConverterException: 
com.fasterxml.jackson.databind.JsonMappingException: successesByDirPath is 
marked non-null but is null
+   *  at [Source: (byte[])"{"successesByDirPath":null}"; line: 1, column: 23] 
(through reference chain: 
org.apache.gobblin.temporal.ddm.work.DirDeletionResult["successesByDirPath"])
+   *   at 
io.temporal.common.converter.JacksonJsonPayloadConverter.fromData(JacksonJsonPayloadConverter.java:101)
+   *   at 
io.temporal.common.converter.DefaultDataConverter.fromPayload(DefaultDataConverter.java:145)
+   * @return
+   */
+  public static DirDeletionResult createEmpty() {
+    return new DirDeletionResult(new HashMap<>());
+  }
 }
diff --git 
a/gobblin-temporal/src/test/java/org/apache/gobblin/temporal/ddm/activity/impl/DeleteWorkDirsActivityImplTest.java
 
b/gobblin-temporal/src/test/java/org/apache/gobblin/temporal/ddm/activity/impl/DeleteWorkDirsActivityImplTest.java
new file mode 100644
index 0000000000..78f81f7f4a
--- /dev/null
+++ 
b/gobblin-temporal/src/test/java/org/apache/gobblin/temporal/ddm/activity/impl/DeleteWorkDirsActivityImplTest.java
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.gobblin.temporal.ddm.activity.impl;
+
+import java.util.HashSet;
+import java.util.Optional;
+import java.util.Set;
+
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+import io.temporal.api.common.v1.Payload;
+import io.temporal.common.converter.JacksonJsonPayloadConverter;
+import io.temporal.common.converter.PayloadConverter;
+
+import org.apache.gobblin.temporal.ddm.activity.DeleteWorkDirsActivity;
+import org.apache.gobblin.temporal.ddm.work.DirDeletionResult;
+import org.apache.gobblin.temporal.ddm.work.WUProcessingSpec;
+
+
+public class DeleteWorkDirsActivityImplTest {
+
+  @Test
+  public void testEmptyDeleteSupportsSerde() {
+    DeleteWorkDirsActivity deleteWorkDirsActivity = new 
DeleteWorkDirsActivityImpl();
+    WUProcessingSpec workSpec = new WUProcessingSpec();
+    Set<String> workDirPaths = new HashSet<>();
+    DirDeletionResult result = deleteWorkDirsActivity.delete(workSpec, null, 
workDirPaths);
+    PayloadConverter converter = new JacksonJsonPayloadConverter();
+    Optional<Payload> payload = converter.toData(result);
+    DirDeletionResult result2 = converter.fromData(payload.get(), 
DirDeletionResult.class, DirDeletionResult.class);
+    Assert.assertEquals(result, result2);
+  }
+}

Reply via email to