diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/MappingJdbcConverter.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/MappingJdbcConverter.java index 82b62c4d6..3810efd12 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/MappingJdbcConverter.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/MappingJdbcConverter.java @@ -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 * {@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 /** * 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 @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 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 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 * Conditionally append the identifier if the entity has an identifier property. */ static Identifier potentiallyAppendIdentifier(Identifier base, RelationalPersistentEntity entity, - Function getter) { + Function getter) { if (entity.hasIdProperty()) { @@ -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 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 * @param identifier */ private record ResolvingConversionContext(ConversionContext delegate, AggregatePath aggregatePath, - Identifier identifier) implements ConversionContext { + Identifier identifier) implements ConversionContext { @Override public S convert(Object source, TypeInformation typeHint) { diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AbstractJdbcAggregateTemplateIntegrationTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AbstractJdbcAggregateTemplateIntegrationTests.java index 253577a23..d1a085bd2 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AbstractJdbcAggregateTemplateIntegrationTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AbstractJdbcAggregateTemplateIntegrationTests.java @@ -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 { 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() { diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/MappingRelationalConverter.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/MappingRelationalConverter.java index 8ce549693..e97b3d733 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/MappingRelationalConverter.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/MappingRelationalConverter.java @@ -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 continue; } + // this hasValue should actually check against null if (!valueProviderToUse.hasValue(property)) { continue; } @@ -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 */ 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 */ 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 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 @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 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); }