Json plugin: do json type resolution during prepare stage for argumen…

…ts and column mappers This allows type lookup and object reader / writer to be reused
jdbi · Jun 29, 2023 · 24f8f90 · 24f8f90
1 parent 6e6a612
commit 24f8f90
Show file tree

Hide file tree

Showing 10 changed files with 135 additions and 79 deletions.
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
@@ -2,6 +2,7 @@
 
   - fix `PreparedBatch` c'tor problem where the binding context was not set correctly.
   - Slight memory optimization on RowMappers and ColumnMappers findFor
+  - JsonMapper implementations now bind the Type earlier, which saves work in Jackson and Gson
 
 # 3.39.1
 

diff --git a/gson2/src/main/java/org/jdbi/v3/gson2/GsonJsonMapper.java b/gson2/src/main/java/org/jdbi/v3/gson2/GsonJsonMapper.java
@@ -13,19 +13,37 @@
  */
 package org.jdbi.v3.gson2;
 
+import java.io.IOException;
 import java.lang.reflect.Type;
 
+import com.google.gson.TypeAdapter;
+import com.google.gson.reflect.TypeToken;
 import org.jdbi.v3.core.config.ConfigRegistry;
+import org.jdbi.v3.core.result.UnableToProduceResultException;
 import org.jdbi.v3.json.JsonMapper;
 
 class GsonJsonMapper implements JsonMapper {
     @Override
-    public String toJson(Type type, Object value, ConfigRegistry config) {
-        return config.get(Gson2Config.class).getGson().toJson(value, type);
-    }
+    public TypedMapper forType(Type type, ConfigRegistry config) {
+        return new TypedMapper() {
+            @SuppressWarnings("rawtypes")
+            private final TypeAdapter adapter = config.get(Gson2Config.class)
+                    .getGson().getAdapter(TypeToken.get(type));
 
-    @Override
-    public Object fromJson(Type type, String json, ConfigRegistry config) {
-        return config.get(Gson2Config.class).getGson().fromJson(json, type);
+            @SuppressWarnings("unchecked")
+            @Override
+            public String toJson(Object value, ConfigRegistry config) {
+                return adapter.toJson(value);
+            }
+
+            @Override
+            public Object fromJson(String json, ConfigRegistry config) {
+                try {
+                    return adapter.fromJson(json);
+                } catch (IOException e) {
+                    throw new UnableToProduceResultException(e);
+                }
+            }
+        };
     }
 }
diff --git a/jackson2/src/main/java/org/jdbi/v3/jackson2/JacksonJsonMapper.java b/jackson2/src/main/java/org/jdbi/v3/jackson2/JacksonJsonMapper.java
@@ -17,6 +17,8 @@
 import java.lang.reflect.Type;
 
 import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.JavaType;
+import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.ObjectReader;
 import com.fasterxml.jackson.databind.ObjectWriter;
 import org.jdbi.v3.core.config.ConfigRegistry;
@@ -25,32 +27,40 @@
 
 class JacksonJsonMapper implements JsonMapper {
     @Override
-    public String toJson(Type type, Object value, ConfigRegistry config) {
-        try {
-            Jackson2Config cfg = config.get(Jackson2Config.class);
-            ObjectWriter writer = cfg.getMapper().writerFor(cfg.getMapper().constructType(type));
-            Class<?> view = cfg.getSerializationView();
-            if (view != null) {
-                writer = writer.withView(view);
+    public TypedMapper forType(Type type, ConfigRegistry config) {
+        return new TypedMapper() {
+            private final ObjectMapper mapper = config.get(Jackson2Config.class).getMapper();
+            private final JavaType mappedType = mapper.constructType(type);
+            private final ObjectReader reader = mapper.readerFor(mappedType);
+            private final ObjectWriter writer = mapper.writerFor(mappedType);
+
+            @Override
+            public String toJson(Object value, ConfigRegistry config) {
+                final Class<?> view = config.get(Jackson2Config.class).getSerializationView();
+                final ObjectWriter viewWriter =
+                          view == null
+                        ? writer
+                        : writer.withView(view);
+                try {
+                    return viewWriter.writeValueAsString(value);
+                } catch (JsonProcessingException e) {
+                    throw new UnableToProduceResultException(e);
+                }
             }
-            return writer.writeValueAsString(value);
-        } catch (JsonProcessingException e) {
-            throw new UnableToProduceResultException(e);
-        }
-    }
 
-    @Override
-    public Object fromJson(Type type, String json, ConfigRegistry config) {
-        try {
-            Jackson2Config cfg = config.get(Jackson2Config.class);
-            ObjectReader reader = cfg.getMapper().readerFor(cfg.getMapper().constructType(type));
-            Class<?> view = cfg.getDeserializationView();
-            if (view != null) {
-                reader = reader.withView(view);
+            @Override
+            public Object fromJson(String json, ConfigRegistry config) {
+                final Class<?> view = config.get(Jackson2Config.class).getDeserializationView();
+                final ObjectReader viewReader =
+                          view == null
+                        ? reader
+                        : reader.withView(view);
+                try {
+                    return viewReader.readValue(json);
+                } catch (IOException e) {
+                    throw new UnableToProduceResultException(e);
+                }
             }
-            return reader.readValue(json);
-        } catch (IOException e) {
-            throw new UnableToProduceResultException(e);
-        }
+        };
     }
 }
diff --git a/json/src/main/java/org/jdbi/v3/json/JsonMapper.java b/json/src/main/java/org/jdbi/v3/json/JsonMapper.java
@@ -26,6 +26,19 @@
  * jdbi3-jackson2 and jdbi3-gson2 are readily available for this.
  */
 public interface JsonMapper {
-    String toJson(Type type, Object value, ConfigRegistry config);
-    Object fromJson(Type type, String json, ConfigRegistry config);
+    @Deprecated // forRemoval=true
+    default String toJson(Type type, Object value, ConfigRegistry config) {
+        return forType(type, config).toJson(value, config);
+    }
+    @Deprecated // forRemoval=true
+    default Object fromJson(Type type, String json, ConfigRegistry config) {
+        return forType(type, config).fromJson(json, config);
+    }
+
+    TypedMapper forType(Type type, ConfigRegistry config);
+
+    interface TypedMapper {
+        String toJson(Object value, ConfigRegistry config);
+        Object fromJson(String json, ConfigRegistry config);
+    }
 }
diff --git a/json/src/main/java/org/jdbi/v3/json/internal/JsonArgumentFactory.java b/json/src/main/java/org/jdbi/v3/json/internal/JsonArgumentFactory.java
@@ -15,6 +15,7 @@
 
 import java.lang.reflect.Type;
 import java.util.Optional;
+import java.util.function.Function;
 
 import org.jdbi.v3.core.argument.Argument;
 import org.jdbi.v3.core.argument.ArgumentFactory;
@@ -26,26 +27,33 @@
 import org.jdbi.v3.json.EncodedJson;
 import org.jdbi.v3.json.Json;
 import org.jdbi.v3.json.JsonConfig;
+import org.jdbi.v3.json.JsonMapper.TypedMapper;
 
 /**
  * converts a value object to json text and delegates to another factory to perform the {@code (@Json) String} binding
  */
 @Json
-public class JsonArgumentFactory implements ArgumentFactory {
+public class JsonArgumentFactory implements ArgumentFactory.Preparable {
+    public static final QualifiedType<String> ENCODED_JSON = QualifiedType.of(String.class).with(EncodedJson.class);
+
     private static final String JSON_NOT_STORABLE = String.format(
         "No argument factory found for `@%s String` or 'String'",
         EncodedJson.class.getSimpleName()
     );
 
     @Override
-    public Optional<Argument> build(Type type, Object value, ConfigRegistry config) {
-        String nullableJson = value == null ? null : config.get(JsonConfig.class).getJsonMapper().toJson(type, value, config);
-        String json = "null".equals(nullableJson) ? null : nullableJson; // json null -> sql null
+    public Optional<Function<Object, Argument>> prepare(Type type, ConfigRegistry config) {
+        TypedMapper mapper = config.get(JsonConfig.class).getJsonMapper().forType(type, config);
         Arguments a = config.get(Arguments.class);
         // look for specialized json support first, revert to simple String binding if absent
-        return Optional.of(JdbiOptionals.findFirstPresent(
-                () -> a.findFor(QualifiedType.of(String.class).with(EncodedJson.class), json),
-                () -> a.findFor(String.class, json))
-                .orElseThrow(() -> new UnableToCreateStatementException(JSON_NOT_STORABLE)));
+        Function<Object, Argument> bindJson = JdbiOptionals.findFirstPresent(
+                () -> a.prepareFor(ENCODED_JSON),
+                () -> a.prepareFor(String.class))
+            .orElseThrow(() -> new UnableToCreateStatementException(JSON_NOT_STORABLE));
+        return Optional.of((Function<Object, Argument>) value -> {
+            String nullableJson = value == null ? null : mapper.toJson(value, config);
+            String json = "null".equals(nullableJson) ? null : nullableJson; // json null -> sql null
+            return bindJson.apply(json);
+        });
     }
 }
diff --git a/json/src/main/java/org/jdbi/v3/json/internal/JsonColumnMapperFactory.java b/json/src/main/java/org/jdbi/v3/json/internal/JsonColumnMapperFactory.java
@@ -26,7 +26,7 @@
 import org.jdbi.v3.json.EncodedJson;
 import org.jdbi.v3.json.Json;
 import org.jdbi.v3.json.JsonConfig;
-import org.jdbi.v3.json.JsonMapper;
+import org.jdbi.v3.json.JsonMapper.TypedMapper;
 
 /**
  * converts a {@code (@Json) String} fetched by another mapper into a value object
@@ -47,10 +47,9 @@ public Optional<ColumnMapper<?>> build(Type type, ConfigRegistry config) {
                 () -> cm.findFor(String.class))
                 .orElseThrow(() -> new UnableToProduceResultException(JSON_NOT_RETRIEVABLE));
 
-        final JsonMapper mapper = config.get(JsonConfig.class).getJsonMapper();
+        final TypedMapper mapper = config.get(JsonConfig.class).getJsonMapper().forType(type, config);
         return Optional.of((rs, i, ctx) ->
             mapper.fromJson(
-                    type,
                     Optional.ofNullable(jsonStringMapper.map(rs, i, ctx))
                             .orElse("null"), // sql null -> json null
                     config));

diff --git a/json/src/main/java/org/jdbi/v3/json/internal/UnimplementedJsonMapper.java b/json/src/main/java/org/jdbi/v3/json/internal/UnimplementedJsonMapper.java
@@ -16,7 +16,6 @@
 import java.lang.reflect.Type;
 
 import org.jdbi.v3.core.config.ConfigRegistry;
-import org.jdbi.v3.core.result.UnableToProduceResultException;
 import org.jdbi.v3.core.statement.UnableToCreateStatementException;
 import org.jdbi.v3.json.JsonConfig;
 import org.jdbi.v3.json.JsonMapper;
@@ -29,12 +28,7 @@ public class UnimplementedJsonMapper implements JsonMapper {
     );
 
     @Override
-    public String toJson(Type type, Object value, ConfigRegistry config) {
+    public TypedMapper forType(Type type, ConfigRegistry config) {
         throw new UnableToCreateStatementException(NO_IMPL_INSTALLED);
     }
-
-    @Override
-    public Object fromJson(Type type, String json, ConfigRegistry config) {
-        throw new UnableToProduceResultException(NO_IMPL_INSTALLED);
-    }
 }
diff --git a/json/src/test/java/org/jdbi/v3/json/JsonPluginTest.java b/json/src/test/java/org/jdbi/v3/json/JsonPluginTest.java
@@ -13,44 +13,52 @@
  */
 package org.jdbi.v3.json;
 
+import java.lang.reflect.Type;
+
+import org.jdbi.v3.core.Jdbi;
 import org.jdbi.v3.core.config.ConfigRegistry;
 import org.jdbi.v3.core.qualifier.QualifiedType;
 import org.jdbi.v3.testing.junit5.JdbiExtension;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.api.extension.RegisterExtension;
-import org.mockito.Mock;
-import org.mockito.junit.jupiter.MockitoExtension;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.eq;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
 
-@ExtendWith(MockitoExtension.class)
 public class JsonPluginTest {
-
     @RegisterExtension
     public JdbiExtension h2Extension = JdbiExtension.h2().withPlugin(new JsonPlugin());
 
-    @Mock
-    public JsonMapper jsonMapper;
-
     @BeforeEach
     public void before() {
-        h2Extension.getJdbi().getConfig(JsonConfig.class).setJsonMapper(jsonMapper);
         h2Extension.getJdbi().useHandle(h -> h.createUpdate("create table foo(bar varchar)").execute());
     }
 
     @Test
     public void factoryChainWorks() {
+        Jdbi jdbi = h2Extension.getJdbi();
         Object instance = new Foo();
         String json = "foo";
 
-        when(jsonMapper.toJson(eq(Foo.class), eq(instance), any(ConfigRegistry.class))).thenReturn(json);
-        when(jsonMapper.fromJson(eq(Foo.class), eq(json), any(ConfigRegistry.class))).thenReturn(instance);
+        jdbi.getConfig(JsonConfig.class).setJsonMapper(new JsonMapper() {
+            @Override
+            public TypedMapper forType(Type type, ConfigRegistry config) {
+                assertThat(type).isEqualTo(Foo.class);
+                return new TypedMapper() {
+                    @Override
+                    public String toJson(Object value, ConfigRegistry config) {
+                        assertThat(value).isEqualTo(instance);
+                        return json;
+                    }
+
+                    @Override
+                    public Object fromJson(String readJson, ConfigRegistry config) {
+                        assertThat(readJson).isEqualTo(json);
+                        return instance;
+                    }
+                };
+            }
+        });
 
         Object result = h2Extension.getJdbi().withHandle(h -> {
             h.createUpdate("insert into foo(bar) values(:foo)")
@@ -66,8 +74,6 @@ public void factoryChainWorks() {
         });
 
         assertThat(result).isSameAs(instance);
-        verify(jsonMapper).fromJson(eq(Foo.class), eq(json), any(ConfigRegistry.class));
-        verify(jsonMapper).toJson(eq(Foo.class), eq(instance), any(ConfigRegistry.class));
     }
 
     public static class Foo {

diff --git a/json/src/test/java/org/jdbi/v3/json/StubJsonMapperTest.java b/json/src/test/java/org/jdbi/v3/json/StubJsonMapperTest.java
@@ -13,7 +13,6 @@
  */
 package org.jdbi.v3.json;
 
-import org.jdbi.v3.core.result.UnableToProduceResultException;
 import org.jdbi.v3.core.statement.UnableToCreateStatementException;
 import org.jdbi.v3.sqlobject.SqlObjectPlugin;
 import org.jdbi.v3.sqlobject.statement.SqlQuery;
@@ -42,7 +41,7 @@ public void defaultFactoriesAreWorkingForSqlObject() {
                 .hasMessageContaining("a JsonMapper");
 
             assertThatThrownBy(dao::get)
-                .isInstanceOf(UnableToProduceResultException.class)
+                .isInstanceOf(UnableToCreateStatementException.class)
                 .hasMessageContaining("need to install")
                 .hasMessageContaining("a JsonMapper");
         });

diff --git a/moshi/src/main/java/org/jdbi/v3/moshi/MoshiJsonMapper.java b/moshi/src/main/java/org/jdbi/v3/moshi/MoshiJsonMapper.java
@@ -16,22 +16,30 @@
 import java.io.IOException;
 import java.lang.reflect.Type;
 
+import com.squareup.moshi.JsonAdapter;
 import org.jdbi.v3.core.config.ConfigRegistry;
 import org.jdbi.v3.core.result.UnableToProduceResultException;
 import org.jdbi.v3.json.JsonMapper;
 
 class MoshiJsonMapper implements JsonMapper {
     @Override
-    public String toJson(Type type, Object value, ConfigRegistry config) {
-        return config.get(MoshiConfig.class).getMoshi().adapter(type).toJson(value);
-    }
+    public TypedMapper forType(Type type, ConfigRegistry config) {
+        return new TypedMapper() {
+            private final JsonAdapter<Object> adapter = config.get(MoshiConfig.class).getMoshi().adapter(type);
 
-    @Override
-    public Object fromJson(Type type, String json, ConfigRegistry config) {
-        try {
-            return config.get(MoshiConfig.class).getMoshi().adapter(type).fromJson(json);
-        } catch (IOException e) {
-            throw new UnableToProduceResultException(e);
-        }
+            @Override
+            public String toJson(Object value, ConfigRegistry config) {
+                return adapter.toJson(value);
+            }
+
+            @Override
+            public Object fromJson(String json, ConfigRegistry config) {
+                try {
+                    return adapter.fromJson(json);
+                } catch (IOException e) {
+                    throw new UnableToProduceResultException(e);
+                }
+            }
+        };
     }
 }