DATAJDBC-341 - Polishing.

Remove SpecialColumnValue in favor of JdbcPropertyValueProvider.hasProperty(…). Rename ResultSetWrapper to ResultSetAccessor. Replace ResultSetAccessor usage in JdbcConverter with ResultSet to avoid visibility issues.

Original pull request: #201.
This commit is contained in:
Mark Paluch
2020-03-31 11:45:53 +02:00
parent 95ca73df2f
commit c0803ddafe
10 changed files with 137 additions and 131 deletions

View File

@@ -24,18 +24,16 @@ import java.util.Optional;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.core.convert.ConverterNotFoundException;
import org.springframework.core.convert.converter.Converter;
import org.springframework.data.convert.CustomConversions;
import org.springframework.data.jdbc.core.convert.ResultSetWrapper.SpecialColumnValue;
import org.springframework.data.jdbc.core.mapping.AggregateReference;
import org.springframework.data.jdbc.support.JdbcUtil;
import org.springframework.data.mapping.MappingException;
import org.springframework.data.mapping.PersistentPropertyAccessor;
import org.springframework.data.mapping.PersistentPropertyPath;
import org.springframework.data.mapping.PreferredConstructor;
import org.springframework.data.mapping.context.MappingContext;
import org.springframework.data.mapping.model.PropertyValueProvider;
import org.springframework.data.mapping.model.SimpleTypeHolder;
import org.springframework.data.relational.core.conversion.BasicRelationalConverter;
import org.springframework.data.relational.core.conversion.RelationalConverter;
@@ -77,7 +75,8 @@ public class BasicJdbcConverter extends BasicRelationalConverter implements Jdbc
/**
* Creates a new {@link BasicRelationalConverter} given {@link MappingContext} and a
* {@link JdbcTypeFactory#unsupported() no-op type factory} throwing {@link UnsupportedOperationException} on type
* creation. Use {@link #BasicJdbcConverter(MappingContext, RelationResolver, CustomConversions, JdbcTypeFactory, IdentifierProcessing)}
* creation. Use
* {@link #BasicJdbcConverter(MappingContext, RelationResolver, CustomConversions, JdbcTypeFactory, IdentifierProcessing)}
* (MappingContext, RelationResolver, JdbcTypeFactory)} to convert arrays and large objects into JDBC-specific types.
*
* @param context must not be {@literal null}.
@@ -106,9 +105,9 @@ public class BasicJdbcConverter extends BasicRelationalConverter implements Jdbc
* @since 2.0
*/
public BasicJdbcConverter(
MappingContext<? extends RelationalPersistentEntity<?>, ? extends RelationalPersistentProperty> context,
RelationResolver relationResolver, CustomConversions conversions, JdbcTypeFactory typeFactory,
IdentifierProcessing identifierProcessing) {
MappingContext<? extends RelationalPersistentEntity<?>, ? extends RelationalPersistentProperty> context,
RelationResolver relationResolver, CustomConversions conversions, JdbcTypeFactory typeFactory,
IdentifierProcessing identifierProcessing) {
super(context, conversions);
@@ -199,7 +198,7 @@ public class BasicJdbcConverter extends BasicRelationalConverter implements Jdbc
@Nullable
public Object readValue(@Nullable Object value, TypeInformation<?> type) {
if (value == null || value == SpecialColumnValue.NO_SUCH_COLUMN) {
if (value == null) {
return value;
}
@@ -320,36 +319,31 @@ public class BasicJdbcConverter extends BasicRelationalConverter implements Jdbc
return null;
}
@SuppressWarnings("unchecked")
@Override
public <T> T mapRow(RelationalPersistentEntity<T> entity, ResultSetWrapper resultSet, Object key) {
return new ReadingContext<T>(
new PersistentPropertyPathExtension(
getMappingContext(), entity),
resultSet, Identifier.empty(), key).mapRow();
public <T> T mapRow(RelationalPersistentEntity<T> entity, ResultSet resultSet, Object key) {
return new ReadingContext<T>(new PersistentPropertyPathExtension(getMappingContext(), entity),
new ResultSetAccessor(resultSet), Identifier.empty(), key).mapRow();
}
@Override
public <T> T mapRow(PersistentPropertyPathExtension path, ResultSetWrapper resultSet, Identifier identifier,
Object key) {
return new ReadingContext<T>(path, resultSet, identifier, key).mapRow();
public <T> T mapRow(PersistentPropertyPathExtension path, ResultSet resultSet, Identifier identifier, Object key) {
return new ReadingContext<T>(path, new ResultSetAccessor(resultSet), identifier, key).mapRow();
}
private class ReadingContext<T> {
private final RelationalPersistentEntity<T> entity;
private final ResultSetWrapper resultSet;
private final PersistentPropertyPathExtension rootPath;
private final PersistentPropertyPathExtension path;
private final Identifier identifier;
private final Object key;
private final PropertyValueProvider<RelationalPersistentProperty> propertyValueProvider;
private final PropertyValueProvider<RelationalPersistentProperty> backReferencePropertyValueProvider;
private final JdbcPropertyValueProvider propertyValueProvider;
private final JdbcBackReferencePropertyValueProvider backReferencePropertyValueProvider;
@SuppressWarnings("unchecked")
private ReadingContext(PersistentPropertyPathExtension rootPath, ResultSetWrapper resultSet, Identifier identifier,
private ReadingContext(PersistentPropertyPathExtension rootPath, ResultSetAccessor accessor, Identifier identifier,
Object key) {
RelationalPersistentEntity<T> entity = (RelationalPersistentEntity<T>) rootPath.getLeafEntity();
@@ -357,38 +351,33 @@ public class BasicJdbcConverter extends BasicRelationalConverter implements Jdbc
Assert.notNull(entity, "The rootPath must point to an entity.");
this.entity = entity;
this.resultSet = resultSet;
this.rootPath = rootPath;
this.path = new PersistentPropertyPathExtension(
getMappingContext(),
this.entity);
this.path = new PersistentPropertyPathExtension(getMappingContext(), this.entity);
this.identifier = identifier;
this.key = key;
propertyValueProvider = new JdbcPropertyValueProvider(identifierProcessing, path, resultSet);
backReferencePropertyValueProvider = new JdbcBackReferencePropertyValueProvider(identifierProcessing, path,
resultSet);
this.propertyValueProvider = new JdbcPropertyValueProvider(identifierProcessing, path, accessor);
this.backReferencePropertyValueProvider = new JdbcBackReferencePropertyValueProvider(identifierProcessing, path,
accessor);
}
private ReadingContext(RelationalPersistentEntity<T> entity, ResultSetWrapper resultSet,
PersistentPropertyPathExtension rootPath, PersistentPropertyPathExtension path, Identifier identifier,
Object key) {
private ReadingContext(RelationalPersistentEntity<T> entity, PersistentPropertyPathExtension rootPath,
PersistentPropertyPathExtension path, Identifier identifier, Object key,
JdbcPropertyValueProvider propertyValueProvider,
JdbcBackReferencePropertyValueProvider backReferencePropertyValueProvider) {
this.entity = entity;
this.resultSet = resultSet;
this.rootPath = rootPath;
this.path = path;
this.identifier = identifier;
this.key = key;
propertyValueProvider = new JdbcPropertyValueProvider(identifierProcessing, path, resultSet);
backReferencePropertyValueProvider = new JdbcBackReferencePropertyValueProvider(identifierProcessing, path,
resultSet);
this.propertyValueProvider = propertyValueProvider;
this.backReferencePropertyValueProvider = backReferencePropertyValueProvider;
}
private <S> ReadingContext<S> extendBy(RelationalPersistentProperty property) {
return new ReadingContext<>(
(RelationalPersistentEntity<S>) getMappingContext().getRequiredPersistentEntity(property.getActualType()),
resultSet, rootPath.extendBy(property), path.extendBy(property), identifier, key);
rootPath.extendBy(property), path.extendBy(property), identifier, key,
propertyValueProvider.extendBy(property), backReferencePropertyValueProvider.extendBy(property));
}
T mapRow() {
@@ -412,11 +401,16 @@ public class BasicJdbcConverter extends BasicRelationalConverter implements Jdbc
continue;
}
Object value = readOrLoadProperty(idValue, property);
if (value != SpecialColumnValue.NO_SUCH_COLUMN) {
propertyAccessor.setProperty(property, value);
// skip absent simple properties
if (isSimpleProperty(property)) {
if (!propertyValueProvider.hasProperty(property)) {
continue;
}
}
Object value = readOrLoadProperty(idValue, property);
propertyAccessor.setProperty(property, value);
}
return propertyAccessor.getBean();
@@ -467,8 +461,7 @@ public class BasicJdbcConverter extends BasicRelationalConverter implements Jdbc
}
Object value = propertyValueProvider.getPropertyValue(property);
return value == SpecialColumnValue.NO_SUCH_COLUMN ? SpecialColumnValue.NO_SUCH_COLUMN
: readValue(value, property.getTypeInformation());
return value != null ? readValue(value, property.getTypeInformation()) : null;
}
@Nullable
@@ -501,7 +494,7 @@ public class BasicJdbcConverter extends BasicRelationalConverter implements Jdbc
}
Object value = readOrLoadProperty(idValue, embeddedProperty);
if (value != null && value != SpecialColumnValue.NO_SUCH_COLUMN) {
if (value != null) {
return true;
}
}
@@ -541,21 +534,15 @@ public class BasicJdbcConverter extends BasicRelationalConverter implements Jdbc
Assert.notNull(parameterName, "A constructor parameter name must not be null to be used with Spring Data JDBC");
RelationalPersistentProperty property = entity.getRequiredPersistentProperty(parameterName);
Object value = readOrLoadProperty(idValue, property);
if (value == SpecialColumnValue.NO_SUCH_COLUMN) {
throw new MappingException(
String.format("Couldn't create instance of type %s with id. No column found for argument named '%s'.",
entity.getType(), parameterName));
}
return value;
return readOrLoadProperty(idValue, property);
});
return populateProperties(instance, idValue);
}
}
private boolean isSimpleProperty(RelationalPersistentProperty property) {
return !property.isCollectionLike() && !property.isEntity() && !property.isMap() && !property.isEmbedded();
}
}

View File

@@ -64,11 +64,9 @@ public class EntityRowMapper<T> implements RowMapper<T> {
@Override
public T mapRow(ResultSet resultSet, int rowNumber) {
ResultSetWrapper wrappedResultSet = new ResultSetWrapper(resultSet);
return path == null //
? converter.mapRow(entity, wrappedResultSet, rowNumber) //
: converter.mapRow(path, wrappedResultSet, identifier, rowNumber);
? converter.mapRow(entity, resultSet, rowNumber) //
: converter.mapRow(path, resultSet, identifier, rowNumber);
}
}

View File

@@ -21,9 +21,8 @@ import org.springframework.data.relational.core.mapping.RelationalPersistentProp
import org.springframework.data.relational.core.sql.IdentifierProcessing;
/**
* {@link PropertyValueProvider} obtaining values from a {@link ResultSetWrapper}.
* For a given id property it provides the value in the resultset under which other entities refer back to it.
*
* {@link PropertyValueProvider} obtaining values from a {@link ResultSetAccessor}. For a given id property it provides
* the value in the resultset under which other entities refer back to it.
*
* @author Jens Schauder
* @since 2.0
@@ -32,15 +31,16 @@ class JdbcBackReferencePropertyValueProvider implements PropertyValueProvider<Re
private final IdentifierProcessing identifierProcessing;
private final PersistentPropertyPathExtension basePath;
private final ResultSetWrapper resultSet;
private final ResultSetAccessor resultSet;
/**
* @param identifierProcessing used for converting the {@link org.springframework.data.relational.core.sql.SqlIdentifier} from a property to a column label
* @param identifierProcessing used for converting the
* {@link org.springframework.data.relational.core.sql.SqlIdentifier} from a property to a column label
* @param basePath path from the aggregate root relative to which all properties get resolved.
* @param resultSet the {@link ResultSetWrapper} from which to obtain the actual values.
* @param resultSet the {@link ResultSetAccessor} from which to obtain the actual values.
*/
JdbcBackReferencePropertyValueProvider(IdentifierProcessing identifierProcessing, PersistentPropertyPathExtension basePath, ResultSetWrapper resultSet) {
JdbcBackReferencePropertyValueProvider(IdentifierProcessing identifierProcessing,
PersistentPropertyPathExtension basePath, ResultSetAccessor resultSet) {
this.resultSet = resultSet;
this.basePath = basePath;
@@ -49,6 +49,11 @@ class JdbcBackReferencePropertyValueProvider implements PropertyValueProvider<Re
@Override
public <T> T getPropertyValue(RelationalPersistentProperty property) {
return (T)resultSet.getObject(basePath.extendBy(property).getReverseColumnNameAlias().getReference(identifierProcessing));
return (T) resultSet
.getObject(basePath.extendBy(property).getReverseColumnNameAlias().getReference(identifierProcessing));
}
public JdbcBackReferencePropertyValueProvider extendBy(RelationalPersistentProperty property) {
return new JdbcBackReferencePropertyValueProvider(identifierProcessing, basePath.extendBy(property), resultSet);
}
}

View File

@@ -54,7 +54,7 @@ public interface JdbcConverter extends RelationalConverter {
* @param <T>
* @return
*/
<T> T mapRow(RelationalPersistentEntity<T> entity, ResultSetWrapper resultSet, Object key);
<T> T mapRow(RelationalPersistentEntity<T> entity, ResultSet resultSet, Object key);
/**
* Read the current row from {@link ResultSet} to an {@link PersistentPropertyPathExtension#getActualType() entity}.
@@ -66,7 +66,7 @@ public interface JdbcConverter extends RelationalConverter {
* @param <T>
* @return
*/
<T> T mapRow(PersistentPropertyPathExtension path, ResultSetWrapper resultSet, Identifier identifier, Object key);
<T> T mapRow(PersistentPropertyPathExtension path, ResultSet resultSet, Identifier identifier, Object key);
/**
* The type to be used to store this property in the database. Multidimensional arrays are unwrapped to reflect a

View File

@@ -21,7 +21,7 @@ import org.springframework.data.relational.core.mapping.RelationalPersistentProp
import org.springframework.data.relational.core.sql.IdentifierProcessing;
/**
* {@link PropertyValueProvider} obtaining values from a {@link ResultSetWrapper}.
* {@link PropertyValueProvider} obtaining values from a {@link ResultSetAccessor}.
*
* @author Jens Schauder
* @since 2.0
@@ -30,15 +30,16 @@ class JdbcPropertyValueProvider implements PropertyValueProvider<RelationalPersi
private final IdentifierProcessing identifierProcessing;
private final PersistentPropertyPathExtension basePath;
private final ResultSetWrapper resultSet;
private final ResultSetAccessor resultSet;
/**
* @param identifierProcessing used for converting the {@link org.springframework.data.relational.core.sql.SqlIdentifier} from a property to a column label
* @param identifierProcessing used for converting the
* {@link org.springframework.data.relational.core.sql.SqlIdentifier} from a property to a column label
* @param basePath path from the aggregate root relative to which all properties get resolved.
* @param resultSet the {@link ResultSetWrapper} from which to obtain the actual values.
* @param resultSet the {@link ResultSetAccessor} from which to obtain the actual values.
*/
JdbcPropertyValueProvider(IdentifierProcessing identifierProcessing, PersistentPropertyPathExtension basePath,
ResultSetWrapper resultSet) {
ResultSetAccessor resultSet) {
this.resultSet = resultSet;
this.basePath = basePath;
@@ -47,8 +48,24 @@ class JdbcPropertyValueProvider implements PropertyValueProvider<RelationalPersi
@Override
public <T> T getPropertyValue(RelationalPersistentProperty property) {
return (T) resultSet.getObject(getColumnName(property));
}
String columnName = basePath.extendBy(property).getColumnAlias().getReference(identifierProcessing);
return (T) resultSet.getObject(columnName);
/**
* Returns whether the underlying source contains a data source for the given {@link RelationalPersistentProperty}.
*
* @param property
* @return
*/
public boolean hasProperty(RelationalPersistentProperty property) {
return resultSet.hasValue(getColumnName(property));
}
private String getColumnName(RelationalPersistentProperty property) {
return basePath.extendBy(property).getColumnAlias().getReference(identifierProcessing);
}
public JdbcPropertyValueProvider extendBy(RelationalPersistentProperty property) {
return new JdbcPropertyValueProvider(identifierProcessing, basePath.extendBy(property), resultSet);
}
}

View File

@@ -54,6 +54,6 @@ class MapEntityRowMapper<T> implements RowMapper<Map.Entry<Object, T>> {
}
private T mapEntity(ResultSet resultSet, Object key) {
return converter.mapRow(path, new ResultSetWrapper(resultSet), identifier, key);
return converter.mapRow(path, resultSet, identifier, key);
}
}

View File

@@ -27,25 +27,29 @@ import org.springframework.lang.Nullable;
import org.springframework.util.LinkedCaseInsensitiveMap;
/**
* Wraps a {@link java.sql.ResultSet} in order to provide fast lookup of columns by name, including for missing columns.
* Wrapper value object for a {@link java.sql.ResultSet} to be able to access raw values by
* {@link org.springframework.data.relational.core.mapping.RelationalPersistentProperty} references. Provides fast
* lookup of columns by name, including for absent columns.
*
* @author Jens Schauder
* @author Mark Paluch
* @since 2.0
*/
class ResultSetWrapper {
class ResultSetAccessor {
private static final Logger LOG = LoggerFactory.getLogger(ResultSetWrapper.class);
private static final Logger LOG = LoggerFactory.getLogger(ResultSetAccessor.class);
private final ResultSet resultSet;
private final Map<String, Integer> indexLookUp;
ResultSetWrapper(ResultSet resultSet) {
ResultSetAccessor(ResultSet resultSet) {
this.resultSet = resultSet;
indexLookUp = indexColumns();
this.indexLookUp = indexColumns(resultSet);
}
private Map<String, Integer> indexColumns() {
private static Map<String, Integer> indexColumns(ResultSet resultSet) {
try {
@@ -57,29 +61,35 @@ class ResultSetWrapper {
for (int i = 1; i <= columnCount; i++) {
String label = metaData.getColumnLabel(i);
if (index.containsKey(label)) {
LOG.warn("We encountered a ResutSet with multiple columns associated with the same label {}.", label);
LOG.warn("ResultSet contains {} multiple times", label);
continue;
}
index.put(label, i);
index.put(label, i);
}
return index;
} catch (SQLException se) {
throw new MappingException("Failure while accessing metadata.");
throw new MappingException("Cannot obtain result metadata", se);
}
}
/**
* Returns the value if the result set contains the {@code columnName}.
*
* @param columnName the column name (label).
* @return
* @see ResultSet#getObject(int)
*/
@Nullable
Object getObject(String columnName) {
public Object getObject(String columnName) {
try {
int column = findColumnIndex(columnName);
return column > 0 ? resultSet.getObject(column) : SpecialColumnValue.NO_SUCH_COLUMN;
int index = findColumnIndex(columnName);
return index > 0 ? resultSet.getObject(index) : null;
} catch (SQLException o_O) {
throw new MappingException(String.format("Could not read value %s from result set!", columnName), o_O);
}
@@ -89,7 +99,13 @@ class ResultSetWrapper {
return indexLookUp.getOrDefault(columnName, -1);
}
enum SpecialColumnValue {
NO_SUCH_COLUMN
/**
* Returns {@literal true} if the result set contains the {@code columnName}.
*
* @param columnName the column name (label).
* @return
*/
public boolean hasValue(String columnName) {
return indexLookUp.containsKey(columnName);
}
}

View File

@@ -43,17 +43,16 @@ import java.util.stream.Stream;
import javax.naming.OperationNotSupportedException;
import org.assertj.core.api.Assertions;
import org.assertj.core.api.SoftAssertions;
import org.junit.Test;
import org.mockito.ArgumentMatchers;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
import org.springframework.data.annotation.Id;
import org.springframework.data.annotation.PersistenceConstructor;
import org.springframework.data.jdbc.core.mapping.AggregateReference;
import org.springframework.data.jdbc.core.mapping.JdbcMappingContext;
import org.springframework.data.mapping.MappingException;
import org.springframework.data.mapping.PersistentPropertyPath;
import org.springframework.data.relational.core.mapping.Embedded;
import org.springframework.data.relational.core.mapping.Embedded.OnEmpty;
@@ -476,10 +475,10 @@ public class EntityRowMapperUnitTests {
ID_FOR_ENTITY_NOT_REFERENCING_MAP);
rs.next();
assertThatThrownBy(() -> createRowMapper(TrivialImmutable.class).mapRow(rs, 1)) //
.hasMessageContaining("TrivialImmutable") //
.hasMessageContaining("name");
TrivialImmutable trivialImmutable = createRowMapper(TrivialImmutable.class).mapRow(rs, 1);
assertThat(trivialImmutable.id).isEqualTo(23L);
assertThat(trivialImmutable.name).isNull();
}
@Test // DATAJDBC-341
@@ -530,16 +529,15 @@ public class EntityRowMapperUnitTests {
}
@Test // DATAJDBC-341
public void immutableEmbeddedWithSomeColumnsMissingShouldThrowAnException() throws SQLException {
public void immutableEmbeddedWithSomeColumnsMissingShouldNotBeEmpty() throws SQLException {
ResultSet rs = mockResultSet(asList("ID", "VALUE"), //
ID_FOR_ENTITY_NOT_REFERENCING_MAP, "some value");
rs.next();
Assertions.assertThatThrownBy( //
() -> createRowMapper(WithNullableEmbeddedImmutableValue.class).mapRow(rs, 1) //
).isInstanceOf(MappingException.class) //
.hasMessageContaining("'name'");
WithNullableEmbeddedImmutableValue result = createRowMapper(WithNullableEmbeddedImmutableValue.class).mapRow(rs, 1);
assertThat(result.embeddedImmutableValue).isNotNull();
}
@Test // DATAJDBC-341
@@ -619,19 +617,6 @@ public class EntityRowMapperUnitTests {
.containsExactly(ID_FOR_ENTITY_NOT_REFERENCING_MAP, "alpha", 24L, null);
}
@Test // DATAJDBC-341
public void immutableOneToOneWithMissingColumnResultsInException() throws SQLException {
ResultSet rs = mockResultSet(asList("ID", "NAME", "CHILD_ID"), //
ID_FOR_ENTITY_NOT_REFERENCING_MAP, "alpha", 24L);
rs.next();
Assertions.assertThatThrownBy( //
() -> createRowMapper(OneToOneImmutable.class).mapRow(rs, 1) //
).isInstanceOf(MappingException.class) //
.hasMessageContaining("'name'");
}
@Test // DATAJDBC-341
public void oneToOneWithMissingIdColumnResultsInNullProperty() throws SQLException {
@@ -641,10 +626,7 @@ public class EntityRowMapperUnitTests {
OneToOne extracted = createRowMapper(OneToOne.class).mapRow(rs, 1);
assertThat(extracted) //
.isNotNull() //
.extracting(e -> e.id, e -> e.name, e -> e.child.id, e -> e.child.name) //
.containsExactly(ID_FOR_ENTITY_NOT_REFERENCING_MAP, "alpha", null, "Alfred");
assertThat(extracted.child).isNull();
}
@Test // DATAJDBC-341
@@ -654,10 +636,11 @@ public class EntityRowMapperUnitTests {
ID_FOR_ENTITY_NOT_REFERENCING_MAP, "alpha", "Alfred");
rs.next();
Assertions.assertThatThrownBy( //
() -> createRowMapper(OneToOneImmutable.class).mapRow(rs, 1) //
).isInstanceOf(MappingException.class) //
.hasMessageContaining("'id'");
OneToOneImmutable result = createRowMapper(OneToOneImmutable.class).mapRow(rs, 1);
assertThat(result.id).isEqualTo(23);
assertThat(result.name).isEqualTo("alpha");
assertThat(result.child).isNull();
}
// Model classes to be used in tests