fix: bug for UriResult deserialization #1575

ICIJ · Sep 18, 2024 · 976c96b · 976c96b
1 parent 6faadb6
commit 976c96b
Show file tree

Hide file tree

Showing 7 changed files with 52 additions and 39 deletions.
diff --git a/datashare-app/src/main/java/org/icij/datashare/web/TaskResource.java b/datashare-app/src/main/java/org/icij/datashare/web/TaskResource.java
@@ -122,9 +122,8 @@ public <V> Payload createTask(@Parameter(name = "id", description = "task id", r
     public Payload getTaskResult(@Parameter(name = "id", description = "task id", in = ParameterIn.PATH) String id, Context context) throws IOException {
         Task<?> task = forbiddenIfNotSameUser(context, notFoundIfNull(taskManager.getTask(id)));
         Object result = task.getResult();
-        if (result instanceof UriResult) {
-            UriResult uriResult = (UriResult) result;
-            Path filePath = Path.of(uriResult.uri.getPath());
+        if (result instanceof UriResult uriResult) {
+            Path filePath = Path.of(uriResult.uri().getPath());
             String fileName = filePath.getFileName().toString();
             String contentDisposition = "attachment;filename=\"" + fileName + "\"";
             InputStream fileInputStream = Files.newInputStream(filePath);

diff --git a/datashare-app/src/test/java/org/icij/datashare/tasks/BatchDownloadRunnerTest.java b/datashare-app/src/test/java/org/icij/datashare/tasks/BatchDownloadRunnerTest.java
@@ -57,7 +57,7 @@ public void test_max_default_results() throws Exception {
                     put(SCROLL_SIZE_OPT, "3");
                 }}), taskView, taskView.progress(updater::progress)).call();
 
-        assertThat(new ZipFile(new File(result.uri)).size()).isEqualTo(3);
+        assertThat(new ZipFile(new File(result.uri())).size()).isEqualTo(3);
     }
 
     @Test
@@ -70,7 +70,7 @@ public void test_max_zip_size() throws Exception {
             put(SCROLL_SIZE_OPT, "3");
         }}), taskView, taskView.progress(updater::progress)).call();
 
-        assertThat(new ZipFile(new File(result.uri)).size()).isEqualTo(3); // the 4th doc must have been skipped
+        assertThat(new ZipFile(new File(result.uri())).size()).isEqualTo(3); // the 4th doc must have been skipped
     }
 
     @Test(expected = ElasticsearchException.class)

diff --git a/datashare-app/src/test/java/org/icij/datashare/tasks/TaskWorkerIntTest.java b/datashare-app/src/test/java/org/icij/datashare/tasks/TaskWorkerIntTest.java
@@ -111,7 +111,7 @@ public void test_update_batch_download_zip_size() throws Exception {
         Task<File> taskView = createTaskView(bd);
         UriResult result = new BatchDownloadRunner(indexer, createProvider(), taskView, taskView.progress(taskModifier::progress)).call();
 
-        assertThat(result.size).isGreaterThan(0);
+        assertThat(result.size()).isGreaterThan(0);
     }
 
     @Test

diff --git a/datashare-tasks/src/main/java/org/icij/datashare/asynctasks/Task.java b/datashare-tasks/src/main/java/org/icij/datashare/asynctasks/Task.java
@@ -4,10 +4,12 @@
 import com.fasterxml.jackson.annotation.JsonIgnore;
 import com.fasterxml.jackson.annotation.JsonInclude;
 import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.annotation.JsonSubTypes;
 import com.fasterxml.jackson.annotation.JsonTypeInfo;
 import org.icij.datashare.Entity;
 import org.icij.datashare.asynctasks.bus.amqp.Event;
 import org.icij.datashare.asynctasks.bus.amqp.TaskError;
+import org.icij.datashare.asynctasks.bus.amqp.UriResult;
 import org.icij.datashare.user.User;
 
 import java.io.Serializable;
@@ -39,6 +41,12 @@ public enum State {CREATED, QUEUED, RUNNING, CANCELLED, ERROR, DONE}
     volatile TaskError error;
     private volatile State state;
     private volatile double progress;
+
+    @JsonSubTypes({
+        @JsonSubTypes.Type(value = UriResult.class),
+        @JsonSubTypes.Type(value = Long.class)
+    })
+    @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "@type")
     private volatile V result;
 
     public Task(String name, User user, Map<String, Object> args) {

diff --git a/datashare-tasks/src/main/java/org/icij/datashare/asynctasks/bus/amqp/UriResult.java b/datashare-tasks/src/main/java/org/icij/datashare/asynctasks/bus/amqp/UriResult.java
@@ -6,37 +6,14 @@
 
 import java.io.Serializable;
 import java.net.URI;
-import java.util.Objects;
 
 import static java.lang.String.format;
 
 @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "@type")
-public class UriResult implements Serializable {
-    public final URI uri;
-    public final long size;
-
+public record UriResult(URI uri, long size) implements Serializable {
     @JsonCreator
     public UriResult(@JsonProperty("uri") URI uri, @JsonProperty("size") long size) {
         this.uri = uri;
         this.size = size;
     }
-
-    @Override
-    public boolean equals(Object o) {
-        if (this == o) return true;
-        if (o == null || getClass() != o.getClass()) return false;
-        UriResult uriResult = (UriResult) o;
-        return Objects.equals(uri, uriResult.uri)
-                && Objects.equals(size, uriResult.size);
-    }
-
-    @Override
-    public int hashCode() {
-        return Objects.hash(uri, size);
-    }
-
-    @Override
-    public String toString() {
-        return format("%s (%d bytes)", uri, size);
-    }
 }
diff --git a/datashare-tasks/src/test/java/org/icij/datashare/asynctasks/TaskManagerAmqpTest.java b/datashare-tasks/src/test/java/org/icij/datashare/asynctasks/TaskManagerAmqpTest.java
@@ -2,9 +2,9 @@
 
 import org.fest.assertions.Assertions;
 import org.icij.datashare.PropertiesProvider;
-import org.icij.datashare.asynctasks.bus.amqp.AmqpServerRule;
 import org.icij.datashare.asynctasks.bus.amqp.AmqpInterlocutor;
 import org.icij.datashare.asynctasks.bus.amqp.AmqpQueue;
+import org.icij.datashare.asynctasks.bus.amqp.AmqpServerRule;
 import org.icij.datashare.asynctasks.bus.amqp.TaskError;
 import org.icij.datashare.user.User;
 import org.icij.extract.redis.RedissonClientFactory;
@@ -15,6 +15,11 @@
 import org.junit.BeforeClass;
 import org.junit.ClassRule;
 import org.junit.Test;
+import org.redisson.Redisson;
+import org.redisson.RedissonMap;
+import org.redisson.api.RedissonClient;
+import org.redisson.command.CommandSyncService;
+import org.redisson.liveobject.core.RedissonObjectBuilder;
 
 import java.io.IOException;
 import java.io.Serializable;
@@ -24,11 +29,6 @@
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.TimeUnit;
-import org.redisson.Redisson;
-import org.redisson.RedissonMap;
-import org.redisson.api.RedissonClient;
-import org.redisson.command.CommandSyncService;
-import org.redisson.liveobject.core.RedissonObjectBuilder;
 
 import static org.fest.assertions.Assertions.assertThat;
 

diff --git a/datashare-tasks/src/test/java/org/icij/datashare/asynctasks/TaskManagerRedisCodecTest.java b/datashare-tasks/src/test/java/org/icij/datashare/asynctasks/TaskManagerRedisCodecTest.java
@@ -2,24 +2,27 @@
 
 import io.netty.buffer.Unpooled;
 import org.fest.assertions.Assertions;
+import org.icij.datashare.asynctasks.bus.amqp.UriResult;
 import org.icij.datashare.user.User;
 import org.junit.Test;
 import org.redisson.client.handler.State;
 
+import java.io.IOException;
+import java.net.URI;
 import java.nio.charset.Charset;
 import java.util.HashMap;
+import java.util.Map;
 
 import static org.fest.assertions.Assertions.assertThat;
 import static org.fest.assertions.MapAssert.entry;
+import static org.icij.datashare.json.JsonObjectMapper.MAPPER;
 
 public class TaskManagerRedisCodecTest {
     TaskManagerRedis.TaskViewCodec codec = new TaskManagerRedis.TaskViewCodec();
 
     @Test
     public void test_json_serialize_deserialize_with_inline_properties_map() throws Exception {
-        Task<?> taskView = new Task<>("name", User.local(), new HashMap<>() {{
-            put("key", "value");
-        }});
+        Task<?> taskView = new Task<>("name", User.local(), Map.of("key", "value"));
         String json = codec.getValueEncoder().encode(taskView).toString(Charset.defaultCharset());
 
         assertThat(json).contains("\"key\":\"value\"");
@@ -31,4 +34,30 @@ public void test_json_serialize_deserialize_with_inline_properties_map() throws
         Assertions.assertThat(actualTask.args).includes(entry("key", "value"));
         Assertions.assertThat(actualTask.getUser()).isEqualTo(User.local());
     }
+
+    @Test
+    public void test_uri_result() throws Exception {
+        Task<?> task = new Task<>("name", User.local(), new HashMap<>());
+        task.setResult(new UriResult(new URI("file://uri"), 123L));
+
+        assertThat(encodeDecode(task).getResult()).isInstanceOf(UriResult.class);
+    }
+
+    @Test
+    public void test_simple_results() throws Exception {
+        Task<?> task = new Task<>("name", User.local(), new HashMap<>());
+        task.setResult(123L);
+        assertThat(encodeDecode(task).getResult()).isInstanceOf(Long.class);
+
+        task.setResult("string");
+        assertThat(encodeDecode(task).getResult()).isInstanceOf(String.class);
+
+        task.setResult(123);
+        assertThat(encodeDecode(task).getResult()).isInstanceOf(Integer.class);
+    }
+
+    private Task<?> encodeDecode(Task<?> task) throws IOException {
+        String json = codec.getValueEncoder().encode(task).toString(Charset.defaultCharset());
+        return (Task<?>) codec.getValueDecoder().decode(Unpooled.wrappedBuffer(json.getBytes()), new State());
+    }
 }