[DE-821] fix deserialize empty response (#560)

rashtao · web-flow · commit 9a0506ae5b96 · 2024-06-12T21:13:26.000+02:00
* fixed deserialization of empty response body

* test async execution and later result retrieval

* fixed check entity class

* fix concurrent modification exceptions in resilience tests logs
diff --git a/core/src/main/java/com/arangodb/internal/config/ArangoConfig.java b/core/src/main/java/com/arangodb/internal/config/ArangoConfig.java
@@ -1,8 +1,6 @@
 package com.arangodb.internal.config;
 
-import com.arangodb.ArangoDBException;
 import com.arangodb.Compression;
-import com.arangodb.ContentType;
 import com.arangodb.Protocol;
 import com.arangodb.arch.UsedInApi;
 import com.arangodb.config.ArangoConfigProperties;
@@ -16,8 +14,6 @@
 import com.arangodb.serde.ArangoSerde;
 import com.arangodb.serde.ArangoSerdeProvider;
 import com.fasterxml.jackson.databind.Module;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import javax.net.ssl.SSLContext;
 import java.util.*;
@@ -52,35 +48,6 @@ public class ArangoConfig {
     private Integer compressionLevel;
     private ProtocolConfig protocolConfig;
 
-    private static final Logger LOG = LoggerFactory.getLogger(ArangoConfig.class);
-
-    private static ArangoSerdeProvider serdeProvider(ContentType contentType) {
-        ServiceLoader<ArangoSerdeProvider> loader = ServiceLoader.load(ArangoSerdeProvider.class);
-        ArangoSerdeProvider serdeProvider = null;
-        Iterator<ArangoSerdeProvider> iterator = loader.iterator();
-        while (iterator.hasNext()) {
-            ArangoSerdeProvider p;
-            try {
-                p = iterator.next();
-            } catch (ServiceConfigurationError e) {
-                LOG.warn("ServiceLoader failed to load ArangoSerdeProvider", e);
-                continue;
-            }
-            if (contentType.equals(p.getContentType())) {
-                if (serdeProvider != null) {
-                    throw new ArangoDBException("Found multiple serde providers! Please set explicitly the one to use.");
-                }
-                serdeProvider = p;
-            }
-        }
-        if (serdeProvider == null) {
-            LOG.warn("No ArangoSerdeProvider found, using InternalSerdeProvider. Please consider registering a custom " +
-                    "ArangoSerdeProvider to avoid depending on internal classes which are not part of the public API.");
-            serdeProvider = new InternalSerdeProvider(contentType);
-        }
-        return serdeProvider;
-    }
-
     public ArangoConfig() {
         // load default properties
         loadProperties(new ArangoConfigProperties() {
@@ -272,7 +239,7 @@ public void setLoadBalancingStrategy(LoadBalancingStrategy loadBalancingStrategy
 
     public ArangoSerde getUserDataSerde() {
         if (userDataSerde == null) {
-            userDataSerde = serdeProvider(ContentTypeFactory.of(getProtocol())).create();
+            userDataSerde = ArangoSerdeProvider.of(ContentTypeFactory.of(getProtocol())).create();
         }
         return userDataSerde;
     }
diff --git a/core/src/main/java/com/arangodb/internal/serde/InternalSerdeImpl.java b/core/src/main/java/com/arangodb/internal/serde/InternalSerdeImpl.java
@@ -160,7 +160,7 @@ public <T> T deserialize(final JsonNode node, final Type type) {
 
     @Override
     public <T> T deserialize(final byte[] content, final Type type) {
-        if (content == null) {
+        if (content == null || content.length == 0) {
             return null;
         }
         try {
@@ -175,6 +175,15 @@ private boolean isManagedClass(Class<?> clazz) {
                 RawJson.class.equals(clazz) ||
                 RawBytes.class.equals(clazz) ||
                 BaseDocument.class.equals(clazz) ||
-                BaseEdgeDocument.class.equals(clazz);
+                BaseEdgeDocument.class.equals(clazz) ||
+                isEntityClass(clazz);
+    }
+
+    private boolean isEntityClass(Class<?> clazz) {
+        Package pkg = clazz.getPackage();
+        if (pkg == null) {
+            return false;
+        }
+        return pkg.getName().startsWith("com.arangodb.entity");
     }
 }
diff --git a/core/src/main/java/com/arangodb/serde/ArangoSerdeProvider.java b/core/src/main/java/com/arangodb/serde/ArangoSerdeProvider.java
@@ -1,9 +1,46 @@
 package com.arangodb.serde;
 
+import com.arangodb.ArangoDBException;
 import com.arangodb.ContentType;
+import com.arangodb.internal.serde.InternalSerdeProvider;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Iterator;
+import java.util.ServiceConfigurationError;
+import java.util.ServiceLoader;
 
 public interface ArangoSerdeProvider {
 
+    static ArangoSerdeProvider of(ContentType contentType) {
+        Logger LOG = LoggerFactory.getLogger(ArangoSerdeProvider.class);
+
+        ServiceLoader<ArangoSerdeProvider> loader = ServiceLoader.load(ArangoSerdeProvider.class);
+        ArangoSerdeProvider serdeProvider = null;
+        Iterator<ArangoSerdeProvider> iterator = loader.iterator();
+        while (iterator.hasNext()) {
+            ArangoSerdeProvider p;
+            try {
+                p = iterator.next();
+            } catch (ServiceConfigurationError e) {
+                LOG.warn("ServiceLoader failed to load ArangoSerdeProvider", e);
+                continue;
+            }
+            if (contentType.equals(p.getContentType())) {
+                if (serdeProvider != null) {
+                    throw new ArangoDBException("Found multiple serde providers! Please set explicitly the one to use.");
+                }
+                serdeProvider = p;
+            }
+        }
+        if (serdeProvider == null) {
+            LOG.warn("No ArangoSerdeProvider found, using InternalSerdeProvider. Please consider registering a custom " +
+                    "ArangoSerdeProvider to avoid depending on internal classes which are not part of the public API.");
+            serdeProvider = new InternalSerdeProvider(contentType);
+        }
+        return serdeProvider;
+    }
+
     /**
      * @return a new serde instance
      */
diff --git a/driver/src/test/java/com/arangodb/ArangoDBTest.java b/driver/src/test/java/com/arangodb/ArangoDBTest.java
@@ -31,6 +31,7 @@
 import com.arangodb.util.UnicodeUtils;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.node.JsonNodeFactory;
+import com.fasterxml.jackson.databind.node.ObjectNode;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Disabled;
@@ -691,6 +692,36 @@ void queueTime(ArangoDB arangoDB) throws InterruptedException, ExecutionExceptio
             assertThat(avg).isEqualTo(0.0);
             assertThat(values).isEmpty();
         }
+    }
+
+    @ParameterizedTest
+    @MethodSource("arangos")
+    void asyncAndLaterResultRetrieval(ArangoDB arangoDB) throws InterruptedException {
+        Request<RawJson> request = Request.<RawJson>builder()
+                .db(ArangoRequestParam.SYSTEM)
+                .method(Request.Method.POST)
+                .path("/_api/cursor")
+                .header("x-arango-async", "store")
+                .body(RawJson.of("{\"query\":\"RETURN SLEEP(0.1) || 5\"}"))
+                .build();
+
+        Response<?> response = arangoDB.execute(request, Void.class);
+        String jobId = response.getHeaders().get("x-arango-async-id");
+        System.out.println(jobId);
+
+        Request<?> request2 = Request.builder()
+                .db(ArangoRequestParam.SYSTEM)
+                .method(Request.Method.PUT)
+                .path("/_api/job/" + jobId)
+                .build();
+
+        Response<ObjectNode> response2 = arangoDB.execute(request2, ObjectNode.class);
+        while (response2.getResponseCode() == 204) {
+            Thread.sleep(50);
+            response2 = arangoDB.execute(request2, ObjectNode.class);
+        }
 
+        assertThat(response2.getResponseCode()).isEqualTo(201);
+        assertThat(response2.getBody().get("result").get(0).numberValue()).isEqualTo(5);
     }
 }
diff --git a/driver/src/test/java/com/arangodb/serde/SerdeTest.java b/driver/src/test/java/com/arangodb/serde/SerdeTest.java
@@ -13,8 +13,7 @@
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.EnumSource;
 
-import java.util.Collections;
-import java.util.Map;
+import java.util.*;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -69,4 +68,35 @@ void serializeBaseDocumentWithNestedProperties(ContentType type) {
         assertThat(on.get("properties").get("foo").textValue()).isEqualTo("bbb");
     }
 
+    @ParameterizedTest
+    @EnumSource(ContentType.class)
+    void deserializeNull(ContentType type) {
+        InternalSerde s = new InternalSerdeProvider(type).create();
+        Void deser = s.deserialize((byte[]) null, Void.class);
+        assertThat(deser).isNull();
+    }
+
+    @ParameterizedTest
+    @EnumSource(ContentType.class)
+    void deserializeNullUserSerde(ContentType type) {
+        ArangoSerde s = ArangoSerdeProvider.of(type).create();
+        Void deser = s.deserialize(null, Void.class);
+        assertThat(deser).isNull();
+    }
+
+    @ParameterizedTest
+    @EnumSource(ContentType.class)
+    void deserializeEmpty(ContentType type) {
+        InternalSerde s = new InternalSerdeProvider(type).create();
+        Void deser = s.deserialize(new byte[0], Void.class);
+        assertThat(deser).isNull();
+    }
+
+    @ParameterizedTest
+    @EnumSource(ContentType.class)
+    void deserializeEmptyUserSerde(ContentType type) {
+        ArangoSerde s = ArangoSerdeProvider.of(type).create();
+        Void deser = s.deserialize(new byte[0], Void.class);
+        assertThat(deser).isNull();
+    }
 }
diff --git a/jackson-serde-json/src/main/java/com/arangodb/serde/jackson/internal/JacksonSerdeImpl.java b/jackson-serde-json/src/main/java/com/arangodb/serde/jackson/internal/JacksonSerdeImpl.java
@@ -43,6 +43,9 @@ public <T> T deserialize(final byte[] content, final Class<T> type) {
     @Override
     public <T> T deserialize(byte[] content, Class<T> type, RequestContext ctx) {
         Objects.requireNonNull(ctx);
+        if (content == null || content.length == 0) {
+            return null;
+        }
         try {
             return mapper.readerFor(mapper.constructType(type))
                     .with(ContextAttributes.getEmpty().withPerCallAttribute(SERDE_CONTEXT_ATTRIBUTE_NAME, ctx))
diff --git a/resilience-tests/src/test/java/resilience/utils/MemoryAppender.java b/resilience-tests/src/test/java/resilience/utils/MemoryAppender.java
@@ -6,6 +6,7 @@
 import ch.qos.logback.core.read.ListAppender;
 import org.slf4j.LoggerFactory;
 
+import java.util.ArrayList;
 import java.util.stream.Stream;
 
 public class MemoryAppender extends ListAppender<ILoggingEvent> {
@@ -22,6 +23,7 @@ public void reset() {
     }
 
     public Stream<ILoggingEvent> getLogs() {
-        return list.stream();
+        // avoid concurrent modification exceptions
+        return new ArrayList<>(list).stream();
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -6,6 +6,7 @@`
`6`	`6`	`import ch.qos.logback.core.read.ListAppender;`
`7`	`7`	`import org.slf4j.LoggerFactory;`
`8`	`8`
	`9`	`+import java.util.ArrayList;`
`9`	`10`	`import java.util.stream.Stream;`
`10`	`11`
`11`	`12`	`public class MemoryAppender extends ListAppender<ILoggingEvent> {`
`@@ -22,6 +23,7 @@ public void reset() {`
`22`	`23`	`}`
`23`	`24`
`24`	`25`	`public Stream<ILoggingEvent> getLogs() {`
`25`		`- return list.stream();`
	`26`	`+ // avoid concurrent modification exceptions`
	`27`	`+ return new ArrayList<>(list).stream();`
`26`	`28`	`}`
`27`	`29`	`}`