From 2c5489b070e262875d35d17c62990b3ef4202e6d Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 18 Jul 2018 14:33:33 +0200 Subject: [PATCH] DATAJDBC-235 - Incorporate feedback from review. Refactor ResultSetParameterValueProvider into Function. Remove unnecessary assertions. --- .../data/jdbc/core/EntityRowMapper.java | 26 +++---------------- .../conversion/BasicRelationalConverter.java | 17 +++++------- .../core/conversion/RelationalConverter.java | 5 +++- .../DefaultDataAccessStrategyUnitTests.java | 2 -- .../BasicRelationalConverterUnitTests.java | 9 +------ 5 files changed, 15 insertions(+), 44 deletions(-) diff --git a/src/main/java/org/springframework/data/jdbc/core/EntityRowMapper.java b/src/main/java/org/springframework/data/jdbc/core/EntityRowMapper.java index b06878a8..9645ee8d 100644 --- a/src/main/java/org/springframework/data/jdbc/core/EntityRowMapper.java +++ b/src/main/java/org/springframework/data/jdbc/core/EntityRowMapper.java @@ -15,9 +15,6 @@ */ package org.springframework.data.jdbc.core; -import lombok.NonNull; -import lombok.RequiredArgsConstructor; - import java.sql.ResultSet; import java.sql.SQLException; import java.util.Map; @@ -26,8 +23,6 @@ import org.springframework.core.convert.converter.Converter; import org.springframework.data.mapping.MappingException; import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.PersistentPropertyAccessor; -import org.springframework.data.mapping.PreferredConstructor.Parameter; -import org.springframework.data.mapping.model.ParameterValueProvider; import org.springframework.data.relational.core.conversion.RelationalConverter; import org.springframework.data.relational.core.mapping.RelationalMappingContext; import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; @@ -144,33 +139,18 @@ public class EntityRowMapper implements RowMapper { } private S createInstance(RelationalPersistentEntity entity, ResultSet rs, String prefix) { - return converter.createInstance(entity, new ResultSetParameterValueProvider(rs, entity, prefix)); - } - @RequiredArgsConstructor - private static class ResultSetParameterValueProvider implements ParameterValueProvider { - - @NonNull private final ResultSet resultSet; - @NonNull private final RelationalPersistentEntity entity; - @NonNull private final String prefix; - - /* - * (non-Javadoc) - * @see org.springframework.data.mapping.model.ParameterValueProvider#getParameterValue(org.springframework.data.mapping.PreferredConstructor.Parameter) - */ - @SuppressWarnings("unchecked") - @Override - public T getParameterValue(Parameter parameter) { + return converter.createInstance(entity, parameter -> { String parameterName = parameter.getName(); Assert.notNull(parameterName, "A constructor parameter name must not be null to be used with Spring Data JDBC"); String column = prefix + entity.getRequiredPersistentProperty(parameterName).getColumnName(); try { - return (T) resultSet.getObject(column); + return rs.getObject(column); } catch (SQLException o_O) { throw new MappingException(String.format("Couldn't read column %s from ResultSet.", column), o_O); } - } + }); } } diff --git a/src/main/java/org/springframework/data/relational/core/conversion/BasicRelationalConverter.java b/src/main/java/org/springframework/data/relational/core/conversion/BasicRelationalConverter.java index 8b3fc951..a58f1091 100644 --- a/src/main/java/org/springframework/data/relational/core/conversion/BasicRelationalConverter.java +++ b/src/main/java/org/springframework/data/relational/core/conversion/BasicRelationalConverter.java @@ -19,6 +19,7 @@ import lombok.RequiredArgsConstructor; import java.util.Collections; import java.util.Optional; +import java.util.function.Function; import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.support.ConfigurableConversionService; @@ -130,11 +131,11 @@ public class BasicRelationalConverter implements RelationalConverter { /* * (non-Javadoc) - * @see org.springframework.data.relational.core.conversion.RelationalConverter#createInstance(org.springframework.data.mapping.PersistentEntity, org.springframework.data.mapping.model.ParameterValueProvider) + * @see org.springframework.data.relational.core.conversion.RelationalConverter#createInstance(org.springframework.data.mapping.PersistentEntity, java.util.function.Function) */ @Override public T createInstance(PersistentEntity entity, - ParameterValueProvider parameterValueProvider) { + Function, Object> parameterValueProvider) { return entityInstantiators.getInstantiatorFor(entity) // .createInstance(entity, new ConvertingParameterValueProvider<>(parameterValueProvider)); @@ -154,9 +155,9 @@ public class BasicRelationalConverter implements RelationalConverter { if (conversions.hasCustomReadTarget(value.getClass(), type.getType())) { return conversionService.convert(value, type.getType()); - } else { - return getPotentiallyConvertedSimpleRead(value, type.getType()); } + + return getPotentiallyConvertedSimpleRead(value, type.getType()); } /* @@ -221,10 +222,6 @@ public class BasicRelationalConverter implements RelationalConverter { return value; } - if (conversions.hasCustomReadTarget(value.getClass(), target)) { - return conversionService.convert(value, target); - } - if (Enum.class.isAssignableFrom(target)) { return Enum.valueOf((Class) target, value.toString()); } @@ -241,7 +238,7 @@ public class BasicRelationalConverter implements RelationalConverter { @RequiredArgsConstructor class ConvertingParameterValueProvider

> implements ParameterValueProvider

{ - private final ParameterValueProvider

delegate; + private final Function, Object> delegate; /* * (non-Javadoc) @@ -250,7 +247,7 @@ public class BasicRelationalConverter implements RelationalConverter { @Override @SuppressWarnings("unchecked") public T getParameterValue(Parameter parameter) { - return (T) readValue(delegate.getParameterValue(parameter), parameter.getType()); + return (T) readValue(delegate.apply(parameter), parameter.getType()); } } } diff --git a/src/main/java/org/springframework/data/relational/core/conversion/RelationalConverter.java b/src/main/java/org/springframework/data/relational/core/conversion/RelationalConverter.java index 19aa676d..09fc4df4 100644 --- a/src/main/java/org/springframework/data/relational/core/conversion/RelationalConverter.java +++ b/src/main/java/org/springframework/data/relational/core/conversion/RelationalConverter.java @@ -15,9 +15,12 @@ */ package org.springframework.data.relational.core.conversion; +import java.util.function.Function; + import org.springframework.core.convert.ConversionService; import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentPropertyAccessor; +import org.springframework.data.mapping.PreferredConstructor.Parameter; import org.springframework.data.mapping.context.MappingContext; import org.springframework.data.mapping.model.ParameterValueProvider; import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; @@ -57,7 +60,7 @@ public interface RelationalConverter { * @return */ T createInstance(PersistentEntity entity, - ParameterValueProvider parameterValueProvider); + Function, Object> parameterValueProvider); /** * Return a {@link PersistentPropertyAccessor} to access property values of the {@code instance}. diff --git a/src/test/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategyUnitTests.java b/src/test/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategyUnitTests.java index a420bd85..c9155825 100644 --- a/src/test/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategyUnitTests.java +++ b/src/test/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategyUnitTests.java @@ -111,8 +111,6 @@ public class DefaultDataAccessStrategyUnitTests { verify(jdbcOperations).update(sqlCaptor.capture(), paramSourceCaptor.capture(), any(KeyHolder.class)); - assertThat(sqlCaptor.getValue()) // - .contains("INSERT INTO entity_with_boolean (flag, id) VALUES (:flag, :id)"); assertThat(paramSourceCaptor.getValue().getValue("id")).isEqualTo(ORIGINAL_ID); assertThat(paramSourceCaptor.getValue().getValue("flag")).isEqualTo("T"); } diff --git a/src/test/java/org/springframework/data/relational/core/conversion/BasicRelationalConverterUnitTests.java b/src/test/java/org/springframework/data/relational/core/conversion/BasicRelationalConverterUnitTests.java index cf0e8b2c..13542941 100644 --- a/src/test/java/org/springframework/data/relational/core/conversion/BasicRelationalConverterUnitTests.java +++ b/src/test/java/org/springframework/data/relational/core/conversion/BasicRelationalConverterUnitTests.java @@ -22,8 +22,6 @@ import lombok.Value; import org.junit.Test; import org.springframework.data.mapping.PersistentPropertyAccessor; -import org.springframework.data.mapping.PreferredConstructor.Parameter; -import org.springframework.data.mapping.model.ParameterValueProvider; import org.springframework.data.relational.core.mapping.RelationalMappingContext; import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; @@ -78,12 +76,7 @@ public class BasicRelationalConverterUnitTests { RelationalPersistentEntity entity = (RelationalPersistentEntity) context .getRequiredPersistentEntity(MyValue.class); - MyValue result = converter.createInstance(entity, new ParameterValueProvider() { - @Override - public T getParameterValue(Parameter parameter) { - return (T) "bar"; - } - }); + MyValue result = converter.createInstance(entity, it -> "bar"); assertThat(result.getFoo()).isEqualTo("bar"); }