search

tarantool / cartridge-java

Tarantool Cartridge Java driver for Tarantool versions 1.10+ based on Netty framework
https://tarantool.io
Other
27 stars 11 forks source link

Add ability to obtain null value with SingleValueConverter #369

Open ArtDu opened 1 year ago

ArtDu commented 1 year ago

Now you can't obtain null if you use this converter https://github.com/tarantool/cartridge-java/blob/b961c6268be7c1fcffaea368208a7203a0177bc2/src/main/java/io/tarantool/driver/core/SingleValueCallResultImpl.java#L34-L36 What if I want to collect the first value and null it's acceptable

ArtDu commented 1 year ago

Proposal how it could be 

     public TarantoolResultMapper<TarantoolTuple> withArrayValueToTarantoolTupleResultConverter(
         ArrayValueToTarantoolTupleConverter tupleConverter) {
-        return withConverterWithoutTargetClass(
-            messagePackMapper.copy(),
-            ValueType.ARRAY,
-            new ArrayValueToTarantoolTupleResultConverter(tupleConverter)
-        );
+        return withConvertersWithoutTargetClass(messagePackMapper.copy(), Arrays.asList(
+            new ValueConverterWithInputTypeWrapper<>(
+                ValueType.ARRAY,
+                new ArrayValueToTarantoolTupleResultConverter(tupleConverter)
+            ),
+            new ValueConverterWithInputTypeWrapper<>(
+                ValueType.NIL,
+                new DefaultNilValueToNullConverter()
+            )
+        ));
     }

possible-solution.patch

diff --git a/src/main/java/io/tarantool/driver/core/SingleValueCallResultImpl.java b/src/main/java/io/tarantool/driver/core/SingleValueCallResultImpl.java
index d44ea092..74b19f45 100644
--- a/src/main/java/io/tarantool/driver/core/SingleValueCallResultImpl.java
+++ b/src/main/java/io/tarantool/driver/core/SingleValueCallResultImpl.java
@@ -39,7 +39,7 @@ public class SingleValueCallResultImpl<T> implements SingleValueCallResult<T> {
         Value errorsValue = result.getOrNilValue(1);
         if (callResultSize == 0 || callResultSize == 1 && resultValue.isNilValue()) {
             // [nil] or []
-            return null;
+            return valueGetter.apply(resultValue);
         } else if (callResultSize == 2 && (resultValue.isNilValue() && !errorsValue.isNilValue())) {
             // [nil, "Error msg..."] or [nil, {str="Error msg...", stack="..."}]
             throw TarantoolErrorsParser.parse(errorsValue);
diff --git a/src/main/java/io/tarantool/driver/core/metadata/DDLTarantoolSpaceMetadataConverter.java b/src/main/java/io/tarantool/driver/core/metadata/DDLTarantoolSpaceMetadataConverter.java
index 7a504f0f..6bf1c11a 100644
--- a/src/main/java/io/tarantool/driver/core/metadata/DDLTarantoolSpaceMetadataConverter.java
+++ b/src/main/java/io/tarantool/driver/core/metadata/DDLTarantoolSpaceMetadataConverter.java
@@ -57,6 +57,11 @@ public final class DDLTarantoolSpaceMetadataConverter implements ValueConverter<

     @Override
     public TarantoolMetadataContainer fromValue(Value value) {
+        ProxyTarantoolMetadataContainer proxyMetadata = new ProxyTarantoolMetadataContainer();
+
+        if (value.isNilValue()) {
+            return proxyMetadata;
+        }

         if (!value.isMapValue()) {
             throw new TarantoolClientException("Unsupported space metadata format: expected map");
@@ -71,8 +76,6 @@ public final class DDLTarantoolSpaceMetadataConverter implements ValueConverter<
         }

         spacesMap = spacesMap.get(SPACES_KEY).asMapValue().map();
-
-        ProxyTarantoolMetadataContainer proxyMetadata = new ProxyTarantoolMetadataContainer();
         for (Value nameValue : spacesMap.keySet()) {
             if (!nameValue.isStringValue()) {
                 throw new TarantoolClientException(
diff --git a/src/main/java/io/tarantool/driver/mappers/factories/AbstractResultMapperFactory.java b/src/main/java/io/tarantool/driver/mappers/factories/AbstractResultMapperFactory.java
index dad58775..4e0ee5aa 100644
--- a/src/main/java/io/tarantool/driver/mappers/factories/AbstractResultMapperFactory.java
+++ b/src/main/java/io/tarantool/driver/mappers/factories/AbstractResultMapperFactory.java
@@ -56,6 +56,10 @@ public abstract class AbstractResultMapperFactory<O, T extends AbstractResultMap
         MessagePackValueMapper valueMapper,
         List<ValueConverterWithInputTypeWrapper<O>> converters);

+    protected abstract T createMapperWithoutTargetClass(
+        MessagePackValueMapper valueMapper,
+        List<ValueConverterWithInputTypeWrapper<Object>> converters);
+
     /**
      * Create {@link AbstractResultMapper} instance with the passed converter.
      *
@@ -131,10 +135,10 @@ public abstract class AbstractResultMapperFactory<O, T extends AbstractResultMap
         return createMapper(valueMapper, converters, resultClass);
     }

-    public T withConverterWithoutTargetClass(
+    public T withConvertersWithoutTargetClass(
         MessagePackValueMapper valueMapper,
-        List<ValueConverterWithInputTypeWrapper<O>> converters) {
-        return createMapper(valueMapper, converters);
+        List<ValueConverterWithInputTypeWrapper<Object>> converters) {
+        return createMapperWithoutTargetClass(valueMapper, converters);
     }

 }
diff --git a/src/main/java/io/tarantool/driver/mappers/factories/ArrayValueToTarantoolResultMapperFactory.java b/src/main/java/io/tarantool/driver/mappers/factories/ArrayValueToTarantoolResultMapperFactory.java
index dd1338fc..58e2ba21 100644
--- a/src/main/java/io/tarantool/driver/mappers/factories/ArrayValueToTarantoolResultMapperFactory.java
+++ b/src/main/java/io/tarantool/driver/mappers/factories/ArrayValueToTarantoolResultMapperFactory.java
@@ -48,7 +48,7 @@ public class ArrayValueToTarantoolResultMapperFactory<T> extends TarantoolResult
             messagePackMapper.copy(),
             ValueType.ARRAY,
             new ArrayValueToTarantoolResultConverter<>(valueConverter)
-        );
+                                              );
     }

     /**
diff --git a/src/main/java/io/tarantool/driver/mappers/factories/ArrayValueToTarantoolTupleResultMapperFactory.java b/src/main/java/io/tarantool/driver/mappers/factories/ArrayValueToTarantoolTupleResultMapperFactory.java
index 4202f683..73f53bd7 100644
--- a/src/main/java/io/tarantool/driver/mappers/factories/ArrayValueToTarantoolTupleResultMapperFactory.java
+++ b/src/main/java/io/tarantool/driver/mappers/factories/ArrayValueToTarantoolTupleResultMapperFactory.java
@@ -1,11 +1,14 @@
 package io.tarantool.driver.mappers.factories;

+import java.util.Arrays;
+
 import io.tarantool.driver.api.metadata.TarantoolSpaceMetadata;
 import io.tarantool.driver.api.tuple.TarantoolTuple;
 import io.tarantool.driver.mappers.MessagePackMapper;
 import io.tarantool.driver.mappers.MessagePackValueMapper;
 import io.tarantool.driver.mappers.TarantoolResultMapper;
 import io.tarantool.driver.mappers.converters.ValueConverterWithInputTypeWrapper;
+import io.tarantool.driver.mappers.converters.object.DefaultNilValueToNullConverter;
 import io.tarantool.driver.mappers.converters.value.ArrayValueToTarantoolTupleConverter;
 import io.tarantool.driver.mappers.converters.value.ArrayValueToTarantoolTupleResultConverter;
 import org.msgpack.value.ValueType;
@@ -58,11 +61,16 @@ public class ArrayValueToTarantoolTupleResultMapperFactory

     public TarantoolResultMapper<TarantoolTuple> withArrayValueToTarantoolTupleResultConverter(
         ArrayValueToTarantoolTupleConverter tupleConverter) {
-        return withConverterWithoutTargetClass(
-            messagePackMapper.copy(),
-            ValueType.ARRAY,
-            new ArrayValueToTarantoolTupleResultConverter(tupleConverter)
-        );
+        return withConvertersWithoutTargetClass(messagePackMapper.copy(), Arrays.asList(
+            new ValueConverterWithInputTypeWrapper<>(
+                ValueType.ARRAY,
+                new ArrayValueToTarantoolTupleResultConverter(tupleConverter)
+            ),
+            new ValueConverterWithInputTypeWrapper<>(
+                ValueType.NIL,
+                new DefaultNilValueToNullConverter()
+            )
+        ));
     }

     public ValueConverterWithInputTypeWrapper<Object> getArrayValueToTarantoolTupleResultConverter(
diff --git a/src/main/java/io/tarantool/driver/mappers/factories/RowsMetadataToTarantoolTupleResultMapperFactory.java b/src/main/java/io/tarantool/driver/mappers/factories/RowsMetadataToTarantoolTupleResultMapperFactory.java
index 18f2f481..b4f1df16 100644
--- a/src/main/java/io/tarantool/driver/mappers/factories/RowsMetadataToTarantoolTupleResultMapperFactory.java
+++ b/src/main/java/io/tarantool/driver/mappers/factories/RowsMetadataToTarantoolTupleResultMapperFactory.java
@@ -62,7 +62,7 @@ public class RowsMetadataToTarantoolTupleResultMapperFactory
             messagePackMapper.copy(),
             ValueType.MAP,
             new RowsMetadataToTarantoolTupleResultConverter(tupleConverter)
-        );
+                                              );
     }

     public ValueConverterWithInputTypeWrapper<Object> getRowsMetadataToTarantoolTupleResultConverter(
@@ -80,6 +80,6 @@ public class RowsMetadataToTarantoolTupleResultMapperFactory
             valueMapper,
             ValueType.MAP,
             new RowsMetadataToTarantoolTupleResultConverter(tupleConverter)
-        );
+                                              );
     }
 }
diff --git a/src/main/java/io/tarantool/driver/mappers/factories/TarantoolCallResultMapperFactory.java b/src/main/java/io/tarantool/driver/mappers/factories/TarantoolCallResultMapperFactory.java
index b46bbd88..909e897b 100644
--- a/src/main/java/io/tarantool/driver/mappers/factories/TarantoolCallResultMapperFactory.java
+++ b/src/main/java/io/tarantool/driver/mappers/factories/TarantoolCallResultMapperFactory.java
@@ -54,4 +54,10 @@ public class TarantoolCallResultMapperFactory<T, R extends CallResult<T>> extend
         MessagePackValueMapper valueMapper, List<ValueConverterWithInputTypeWrapper<R>> converters) {
         return new CallResultMapper<>(valueMapper, converters);
     }
+
+    @Override
+    protected CallResultMapper<T, R> createMapperWithoutTargetClass(
+        MessagePackValueMapper valueMapper, List<ValueConverterWithInputTypeWrapper<Object>> converters) {
+        return new CallResultMapper(valueMapper, converters);
+    }
 }
diff --git a/src/main/java/io/tarantool/driver/mappers/factories/TarantoolResultMapperFactory.java b/src/main/java/io/tarantool/driver/mappers/factories/TarantoolResultMapperFactory.java
index f517affe..cd564f62 100644
--- a/src/main/java/io/tarantool/driver/mappers/factories/TarantoolResultMapperFactory.java
+++ b/src/main/java/io/tarantool/driver/mappers/factories/TarantoolResultMapperFactory.java
@@ -57,4 +57,11 @@ public class TarantoolResultMapperFactory<T> extends
         List<ValueConverterWithInputTypeWrapper<TarantoolResult<T>>> converters) {
         return new TarantoolResultMapper<>(valueMapper, converters);
     }
+
+    @Override
+    protected TarantoolResultMapper<T> createMapperWithoutTargetClass(
+        MessagePackValueMapper valueMapper,
+        List<ValueConverterWithInputTypeWrapper<Object>> converters) {
+        return new TarantoolResultMapper(valueMapper, converters);
+    }
 }
diff --git a/src/test/java/io/tarantool/driver/integration/TarantoolErrorsIT.java b/src/test/java/io/tarantool/driver/integration/TarantoolErrorsIT.java
index 63e61ecd..f144f5b2 100644
--- a/src/test/java/io/tarantool/driver/integration/TarantoolErrorsIT.java
+++ b/src/test/java/io/tarantool/driver/integration/TarantoolErrorsIT.java
@@ -10,6 +10,8 @@ import io.tarantool.driver.exceptions.TarantoolClientException;
 import io.tarantool.driver.exceptions.TarantoolInternalException;
 import io.tarantool.driver.exceptions.TarantoolInternalNetworkException;
 import io.tarantool.driver.exceptions.TarantoolNoSuchProcedureException;
+import io.tarantool.driver.exceptions.TarantoolSpaceNotFoundException;
+
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;

@@ -213,4 +215,18 @@ public class TarantoolErrorsIT extends SharedCartridgeContainer {
         assertTrue(cause.getMessage()
             .contains("Execute access to function 'ddl.get_schema' is denied for user 'restricted_user'"));
     }
+
+    @Test
+    void test_failedToRefreshMetadata_shouldHasReasonOfException() {
+        ProxyTarantoolTupleClient adminClient = setupAdminClient();
+
+        adminClient.eval("true_ddl = ddl; " +
+                             "ddl = {get_schema = function() return nil end}").join();
+
+        TarantoolSpaceNotFoundException exception = assertThrows(TarantoolSpaceNotFoundException.class,
+            () -> adminClient.space("test_space").select(Conditions.any()));
+        assertEquals("Space with name 'test_space' not found", exception.getMessage());
+
+        adminClient.eval("ddl = true_ddl").join();
+    }
 }
akudiyar commented 1 year ago

Although I don't like the idea of passing a list of converters simultaneously (that gives some ambiguity about the order of these converters in the stack), I agree with this bugfix in general.