Fix regression of loading empty arrays.

Closes #1826
See #1812

Author: schauder

Diff for: spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/MappingJdbcConverter.java

@@ -26,7 +26,6 @@
 
 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 MappingJdbcConverter(RelationalMappingContext context, RelationResolver r
 	/**
 	 * 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 <R> R readAndResolve(TypeInformation<R> type, RowDocument source, Identif
 
 	@Override
 	protected RelationalPropertyValueProvider newValueProvider(RowDocumentAccessor documentAccessor,
-			ValueExpressionEvaluator evaluator, ConversionContext context) {
+															   ValueExpressionEvaluator evaluator, ConversionContext context) {
 
 		if (context instanceof ResolvingConversionContext rcc) {
 
@@ -330,7 +329,7 @@ class ResolvingRelationalPropertyValueProvider implements RelationalPropertyValu
 		private final Identifier identifier;
 
 		private ResolvingRelationalPropertyValueProvider(AggregatePathValueProvider delegate, RowDocumentAccessor accessor,
-				ResolvingConversionContext context, Identifier identifier) {
+														 ResolvingConversionContext context, Identifier identifier) {
 
 			AggregatePath path = context.aggregatePath();
 
@@ -339,15 +338,15 @@ private ResolvingRelationalPropertyValueProvider(AggregatePathValueProvider dele
 			this.context = context;
 			this.identifier = path.isEntity()
 					? potentiallyAppendIdentifier(identifier, path.getRequiredLeafEntity(),
-							property -> delegate.getValue(path.append(property)))
+					property -> delegate.getValue(path.append(property)))
 					: identifier;
 		}
 
 		/**
 		 * 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 <T> T getPropertyValue(RelationalPersistentProperty property) {
 		@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 boolean hasValue(RelationalPersistentProperty property) {
 			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 RelationalPropertyValueProvider withContext(ConversionContext context) {
 	 * @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) {

Diff for: spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AbstractJdbcAggregateTemplateIntegrationTests.java

@@ -681,6 +681,26 @@ void saveAndLoadAnEntityWithArray() {
 		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 @@ void saveAndLoadAnEntityWithList() {
 		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 for: spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/MappingRelationalConverter.java

@@ -490,6 +490,11 @@ public boolean hasValue(RelationalPersistentProperty property) {
 				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 @@ private void readProperties(ConversionContext context, RelationalPersistentEntit
 				continue;
 			}
 
+			// this hasValue should actually check against null
 			if (!valueProviderToUse.hasValue(property)) {
 				continue;
 			}
@@ -608,7 +614,7 @@ private boolean shouldReadEmbeddable(ConversionContext context, RelationalPersis
 					return true;
 				}
 
-			} else if (contextual.hasValue(persistentProperty)) {
+			} else if (contextual.hasNonEmptyValue(persistentProperty)) {
 				return true;
 			}
 		}
@@ -1051,6 +1057,13 @@ protected interface RelationalPropertyValueProvider extends PropertyValueProvide
 		 */
 		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 @@ protected interface AggregatePathValueProvider extends RelationalPropertyValuePr
 		 */
 		boolean hasValue(AggregatePath path);
 
+		boolean hasNonEmptyValue(AggregatePath aggregatePath);
+
 		/**
 		 * Determine whether there is a value for the given {@link SqlIdentifier}.
 		 *
@@ -1154,6 +1169,11 @@ public boolean hasValue(RelationalPersistentProperty property) {
 			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 Object getValue(AggregatePath path) {
 
 		@Override
 		public boolean hasValue(AggregatePath path) {
+
 			Object value = document.get(path.getColumnInfo().alias().getReference());
 
 			if (value == null) {
@@ -1179,6 +1200,18 @@ public boolean hasValue(AggregatePath path) {
 				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);
 			}