Browse Source

Fix regression of loading empty arrays.

Closes #1826
See #1812
pull/1830/head
Jens Schauder 1 year ago
parent
commit
7bd907c5e6
No known key found for this signature in database
GPG Key ID: 74F6C554AE971567
  1. 49
      spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/MappingJdbcConverter.java
  2. 37
      spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AbstractJdbcAggregateTemplateIntegrationTests.java
  3. 35
      spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/MappingRelationalConverter.java

49
spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/MappingJdbcConverter.java

@ -26,7 +26,6 @@ import java.util.function.Function; @@ -26,7 +26,6 @@ import java.util.function.Function;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.context.ApplicationContextAware;
import org.springframework.core.convert.ConverterNotFoundException;
import org.springframework.core.convert.converter.Converter;
@ -81,7 +80,7 @@ public class MappingJdbcConverter extends MappingRelationalConverter implements @@ -81,7 +80,7 @@ public class MappingJdbcConverter extends MappingRelationalConverter implements
* {@link #MappingJdbcConverter(RelationalMappingContext, RelationResolver, CustomConversions, JdbcTypeFactory)}
* (MappingContext, RelationResolver, JdbcTypeFactory)} to convert arrays and large objects into JDBC-specific types.
*
* @param context must not be {@literal null}.
* @param context must not be {@literal null}.
* @param relationResolver used to fetch additional relations from the database. Must not be {@literal null}.
*/
public MappingJdbcConverter(RelationalMappingContext context, RelationResolver relationResolver) {
@ -99,12 +98,12 @@ public class MappingJdbcConverter extends MappingRelationalConverter implements @@ -99,12 +98,12 @@ public class MappingJdbcConverter extends MappingRelationalConverter implements
/**
* Creates a new {@link MappingJdbcConverter} given {@link MappingContext}.
*
* @param context must not be {@literal null}.
* @param context must not be {@literal null}.
* @param relationResolver used to fetch additional relations from the database. Must not be {@literal null}.
* @param typeFactory must not be {@literal null}
* @param typeFactory must not be {@literal null}
*/
public MappingJdbcConverter(RelationalMappingContext context, RelationResolver relationResolver,
CustomConversions conversions, JdbcTypeFactory typeFactory) {
CustomConversions conversions, JdbcTypeFactory typeFactory) {
super(context, conversions);
@ -301,7 +300,7 @@ public class MappingJdbcConverter extends MappingRelationalConverter implements @@ -301,7 +300,7 @@ public class MappingJdbcConverter extends MappingRelationalConverter implements
@Override
protected RelationalPropertyValueProvider newValueProvider(RowDocumentAccessor documentAccessor,
ValueExpressionEvaluator evaluator, ConversionContext context) {
ValueExpressionEvaluator evaluator, ConversionContext context) {
if (context instanceof ResolvingConversionContext rcc) {
@ -330,7 +329,7 @@ public class MappingJdbcConverter extends MappingRelationalConverter implements @@ -330,7 +329,7 @@ public class MappingJdbcConverter extends MappingRelationalConverter implements
private final Identifier identifier;
private ResolvingRelationalPropertyValueProvider(AggregatePathValueProvider delegate, RowDocumentAccessor accessor,
ResolvingConversionContext context, Identifier identifier) {
ResolvingConversionContext context, Identifier identifier) {
AggregatePath path = context.aggregatePath();
@ -339,7 +338,7 @@ public class MappingJdbcConverter extends MappingRelationalConverter implements @@ -339,7 +338,7 @@ public class MappingJdbcConverter extends MappingRelationalConverter implements
this.context = context;
this.identifier = path.isEntity()
? potentiallyAppendIdentifier(identifier, path.getRequiredLeafEntity(),
property -> delegate.getValue(path.append(property)))
property -> delegate.getValue(path.append(property)))
: identifier;
}
@ -347,7 +346,7 @@ public class MappingJdbcConverter extends MappingRelationalConverter implements @@ -347,7 +346,7 @@ public class MappingJdbcConverter extends MappingRelationalConverter implements
* Conditionally append the identifier if the entity has an identifier property.
*/
static Identifier potentiallyAppendIdentifier(Identifier base, RelationalPersistentEntity<?> entity,
Function<RelationalPersistentProperty, Object> getter) {
Function<RelationalPersistentProperty, Object> getter) {
if (entity.hasIdProperty()) {
@ -422,7 +421,7 @@ public class MappingJdbcConverter extends MappingRelationalConverter implements @@ -422,7 +421,7 @@ public class MappingJdbcConverter extends MappingRelationalConverter implements
@Override
public boolean hasValue(RelationalPersistentProperty property) {
if ((property.isCollectionLike() && property.isEntity())|| property.isMap()) {
if ((property.isCollectionLike() && property.isEntity()) || property.isMap()) {
// attempt relation fetch
return true;
}
@ -445,12 +444,38 @@ public class MappingJdbcConverter extends MappingRelationalConverter implements @@ -445,12 +444,38 @@ public class MappingJdbcConverter extends MappingRelationalConverter implements
return delegate.hasValue(aggregatePath);
}
@Override
public boolean hasNonEmptyValue(RelationalPersistentProperty property) {
if ((property.isCollectionLike() && property.isEntity()) || property.isMap()) {
// attempt relation fetch
return true;
}
AggregatePath aggregatePath = context.aggregatePath();
if (property.isEntity()) {
RelationalPersistentEntity<?> entity = getMappingContext().getRequiredPersistentEntity(property);
if (entity.hasIdProperty()) {
RelationalPersistentProperty referenceId = entity.getRequiredIdProperty();
AggregatePath toUse = aggregatePath.append(referenceId);
return delegate.hasValue(toUse);
}
return delegate.hasValue(aggregatePath.getTableInfo().reverseColumnInfo().alias());
}
return delegate.hasNonEmptyValue(aggregatePath);
}
@Override
public RelationalPropertyValueProvider withContext(ConversionContext context) {
return context == this.context ? this
: new ResolvingRelationalPropertyValueProvider(delegate.withContext(context), accessor,
(ResolvingConversionContext) context, identifier);
(ResolvingConversionContext) context, identifier);
}
}
@ -462,7 +487,7 @@ public class MappingJdbcConverter extends MappingRelationalConverter implements @@ -462,7 +487,7 @@ public class MappingJdbcConverter extends MappingRelationalConverter implements
* @param identifier
*/
private record ResolvingConversionContext(ConversionContext delegate, AggregatePath aggregatePath,
Identifier identifier) implements ConversionContext {
Identifier identifier) implements ConversionContext {
@Override
public <S> S convert(Object source, TypeInformation<? extends S> typeHint) {

37
spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AbstractJdbcAggregateTemplateIntegrationTests.java

@ -681,6 +681,26 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests { @@ -681,6 +681,26 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests {
assertThat(reloaded.digits).isEqualTo(new String[] { "one", "two", "three" });
}
@Test // GH-1826
@EnabledOnFeature(SUPPORTS_ARRAYS)
void saveAndLoadAnEntityWithEmptyArray() {
ArrayOwner arrayOwner = new ArrayOwner();
arrayOwner.digits = new String[] { };
ArrayOwner saved = template.save(arrayOwner);
assertThat(saved.id).isNotNull();
ArrayOwner reloaded = template.findById(saved.id, ArrayOwner.class);
assertThat(reloaded).isNotNull();
assertThat(reloaded.id).isEqualTo(saved.id);
assertThat(reloaded.digits) //
.isNotNull() //
.isEmpty();
}
@Test // DATAJDBC-259, DATAJDBC-512
@EnabledOnFeature(SUPPORTS_MULTIDIMENSIONAL_ARRAYS)
void saveAndLoadAnEntityWithMultidimensionalArray() {
@ -718,6 +738,23 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests { @@ -718,6 +738,23 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests {
assertThat(reloaded.digits).isEqualTo(asList("one", "two", "three"));
}
@Test // DATAJDBC-1826
@EnabledOnFeature(SUPPORTS_ARRAYS)
void saveAndLoadAnEntityWithEmptyList() {
ListOwner arrayOwner = new ListOwner();
ListOwner saved = template.save(arrayOwner);
assertThat(saved.id).isNotNull();
ListOwner reloaded = template.findById(saved.id, ListOwner.class);
assertThat(reloaded).isNotNull();
assertThat(reloaded.id).isEqualTo(saved.id);
assertThat(reloaded.digits).isNotNull().isEmpty();
}
@Test // GH-1033
@EnabledOnFeature(SUPPORTS_ARRAYS)
void saveAndLoadAnEntityWithListOfDouble() {

35
spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/MappingRelationalConverter.java

@ -490,6 +490,11 @@ public class MappingRelationalConverter extends AbstractRelationalConverter @@ -490,6 +490,11 @@ public class MappingRelationalConverter extends AbstractRelationalConverter
return withContext(context.forProperty(property)).hasValue(property);
}
@Override
public boolean hasNonEmptyValue(RelationalPersistentProperty property) {
return withContext(context.forProperty(property)).hasNonEmptyValue(property);
}
@SuppressWarnings("unchecked")
@Nullable
@Override
@ -565,6 +570,7 @@ public class MappingRelationalConverter extends AbstractRelationalConverter @@ -565,6 +570,7 @@ public class MappingRelationalConverter extends AbstractRelationalConverter
continue;
}
// this hasValue should actually check against null
if (!valueProviderToUse.hasValue(property)) {
continue;
}
@ -608,7 +614,7 @@ public class MappingRelationalConverter extends AbstractRelationalConverter @@ -608,7 +614,7 @@ public class MappingRelationalConverter extends AbstractRelationalConverter
return true;
}
} else if (contextual.hasValue(persistentProperty)) {
} else if (contextual.hasNonEmptyValue(persistentProperty)) {
return true;
}
}
@ -1051,6 +1057,13 @@ public class MappingRelationalConverter extends AbstractRelationalConverter @@ -1051,6 +1057,13 @@ public class MappingRelationalConverter extends AbstractRelationalConverter
*/
boolean hasValue(RelationalPersistentProperty property);
/**
* Determine whether there is a non empty value for the given {@link RelationalPersistentProperty}.
*
* @param property the property to check for whether a value is present.
*/
boolean hasNonEmptyValue(RelationalPersistentProperty property);
/**
* Contextualize this property value provider.
*
@ -1072,6 +1085,8 @@ public class MappingRelationalConverter extends AbstractRelationalConverter @@ -1072,6 +1085,8 @@ public class MappingRelationalConverter extends AbstractRelationalConverter
*/
boolean hasValue(AggregatePath path);
boolean hasNonEmptyValue(AggregatePath aggregatePath);
/**
* Determine whether there is a value for the given {@link SqlIdentifier}.
*
@ -1154,6 +1169,11 @@ public class MappingRelationalConverter extends AbstractRelationalConverter @@ -1154,6 +1169,11 @@ public class MappingRelationalConverter extends AbstractRelationalConverter
return accessor.hasValue(property);
}
@Override
public boolean hasNonEmptyValue(RelationalPersistentProperty property) {
return hasValue(property);
}
@Nullable
@Override
public Object getValue(AggregatePath path) {
@ -1169,6 +1189,7 @@ public class MappingRelationalConverter extends AbstractRelationalConverter @@ -1169,6 +1189,7 @@ public class MappingRelationalConverter extends AbstractRelationalConverter
@Override
public boolean hasValue(AggregatePath path) {
Object value = document.get(path.getColumnInfo().alias().getReference());
if (value == null) {
@ -1179,6 +1200,18 @@ public class MappingRelationalConverter extends AbstractRelationalConverter @@ -1179,6 +1200,18 @@ public class MappingRelationalConverter extends AbstractRelationalConverter
return true;
}
return true;
}
@Override
public boolean hasNonEmptyValue(AggregatePath path) {
if (!hasValue(path)) {
return false;
}
Object value = document.get(path.getColumnInfo().alias().getReference());
if (value instanceof Collection<?> || value.getClass().isArray()) {
return !ObjectUtils.isEmpty(value);
}

Loading…
Cancel
Save