From 24f8f90da715e81aebb6ad26049f6ba62280104c Mon Sep 17 00:00:00 2001 From: Steven Schlansker Date: Thu, 29 Jun 2023 14:50:25 -0700 Subject: [PATCH] Json plugin: do json type resolution during prepare stage for arguments and column mappers This allows type lookup and object reader / writer to be reused --- RELEASE_NOTES.md | 1 + .../org/jdbi/v3/gson2/GsonJsonMapper.java | 30 ++++++++-- .../jdbi/v3/jackson2/JacksonJsonMapper.java | 58 +++++++++++-------- .../java/org/jdbi/v3/json/JsonMapper.java | 17 +++++- .../v3/json/internal/JsonArgumentFactory.java | 24 +++++--- .../internal/JsonColumnMapperFactory.java | 5 +- .../internal/UnimplementedJsonMapper.java | 8 +-- .../java/org/jdbi/v3/json/JsonPluginTest.java | 40 +++++++------ .../org/jdbi/v3/json/StubJsonMapperTest.java | 3 +- .../org/jdbi/v3/moshi/MoshiJsonMapper.java | 28 +++++---- 10 files changed, 135 insertions(+), 79 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index c17544c182..f32d2fbb35 100644 --- 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 index 8e124518fd..b565239977 100644 --- 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 index 07ca4a8294..8a867cd6d1 100644 --- 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 index ec45a78d93..e0cb5857e0 100644 --- 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 index b06be87deb..2ccb9d9d4e 100644 --- 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 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 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> 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 bindJson = JdbiOptionals.findFirstPresent( + () -> a.prepareFor(ENCODED_JSON), + () -> a.prepareFor(String.class)) + .orElseThrow(() -> new UnableToCreateStatementException(JSON_NOT_STORABLE)); + return Optional.of((Function) 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 index d12c05f627..243a7b93fe 100644 --- 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> 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 index 021f272a8a..db6fb1c115 100644 --- 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 index 1536666f35..570d6a621f 100644 --- 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 index 6a299ea059..da312546a2 100644 --- 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 index 6267ef87bc..43b5e722a0 100644 --- 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 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); + } + } + }; } }