DATAJDBC-370 - Fix read/write of nulled embedded objects.

We now allow read and write of Objects annotated with Embedded that are actually null.
When writing all contained fields will be nulled.
Reading back the entity considers an embedded object to be null itself if all contained properties are null within the backing result.

Relates to DATAJDBC-364

Original Pull Request: #151

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

@@ -52,6 +52,7 @@ import org.springframework.util.Assert;
  *
  * @author Mark Paluch
  * @author Jens Schauder
+ * @author Christoph Strobl
  * @see MappingContext
  * @see SimpleTypeHolder
  * @see CustomConversions
@@ -345,15 +346,40 @@ public class BasicJdbcConverter extends BasicRelationalConverter implements Jdbc
 
 			Object value = getObjectFromResultSet(path.extendBy(property).getColumnAlias());
 			return readValue(value, property.getTypeInformation());
-
 		}
 
-		@SuppressWarnings("unchecked")
+		@Nullable
 		private Object readEmbeddedEntityFrom(@Nullable Object idValue, RelationalPersistentProperty property) {
 
-			ReadingContext newContext = extendBy(property);
+			ReadingContext<?> ctx = extendBy(property);
+			return hasInstanceValues(property, ctx) ? ctx.createInstanceInternal(idValue) : null;
+		}
 
-			return newContext.createInstanceInternal(idValue);
+		private boolean hasInstanceValues(RelationalPersistentProperty property, ReadingContext<?> ctx) {
+
+			RelationalPersistentEntity<?> persistentEntity = getMappingContext()
+					.getPersistentEntity(property.getTypeInformation());
+
+			PersistentPropertyPathExtension extension = ctx.path;
+
+			for (RelationalPersistentProperty embeddedProperty : persistentEntity) {
+
+				if (embeddedProperty.isQualified() || embeddedProperty.isReference()) {
+					return true;
+				}
+
+				try {
+					if (ctx.getObjectFromResultSet(extension.extendBy(embeddedProperty).getColumnName()) != null) {
+						return true;
+					}
+				} catch (MappingException e) {
+					if (ctx.getObjectFromResultSet(extension.extendBy(embeddedProperty).getReverseColumnNameAlias()) != null) {
+						return true;
+					}
+				}
+			}
+
+			return false;
 		}
 
 		@Nullable

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

@@ -54,6 +54,7 @@ import org.springframework.util.Assert;
  * @author Mark Paluch
  * @author Thomas Lang
  * @author Bastian Wilhelm
+ * @author Christoph Strobl
  * @since 1.1
  */
 public class DefaultDataAccessStrategy implements DataAccessStrategy {
@@ -313,7 +314,8 @@ public class DefaultDataAccessStrategy implements DataAccessStrategy {
 
 		MapSqlParameterSource parameters = new MapSqlParameterSource();
 
-		PersistentPropertyAccessor<S> propertyAccessor = persistentEntity.getPropertyAccessor(instance);
+		PersistentPropertyAccessor<S> propertyAccessor = instance != null ? persistentEntity.getPropertyAccessor(instance)
+				: NoValuePropertyAccessor.instance();
 
 		persistentEntity.doWithProperties((PropertyHandler<RelationalPersistentProperty>) property -> {
 
@@ -480,4 +482,33 @@ public class DefaultDataAccessStrategy implements DataAccessStrategy {
 			return it -> false;
 		}
 	}
+
+	/**
+	 * A {@link PersistentPropertyAccessor} implementation always returning null
+	 * 
+	 * @param <T>
+	 */
+	static class NoValuePropertyAccessor<T> implements PersistentPropertyAccessor<T> {
+
+		private static final NoValuePropertyAccessor INSTANCE = new NoValuePropertyAccessor();
+
+		static <T> NoValuePropertyAccessor<T> instance() {
+			return INSTANCE;
+		}
+
+		@Override
+		public void setProperty(PersistentProperty<?> property, Object value) {
+			throw new UnsupportedOperationException("Cannot set value on 'null' target object.");
+		}
+
+		@Override
+		public Object getProperty(PersistentProperty<?> property) {
+			return null;
+		}
+
+		@Override
+		public T getBean() {
+			return null;
+		}
+	}
 }

spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/EntityRowMapperUnitTests.java

@@ -18,11 +18,15 @@ package org.springframework.data.jdbc.core.convert;
 import static java.util.Arrays.*;
 import static java.util.Collections.*;
 import static org.assertj.core.api.Assertions.*;
-import static org.mockito.ArgumentMatchers.*;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.*;
 
 import lombok.AllArgsConstructor;
+import lombok.EqualsAndHashCode;
+import lombok.NoArgsConstructor;
 import lombok.RequiredArgsConstructor;
+import lombok.SneakyThrows;
 import lombok.Value;
 import lombok.experimental.Wither;
 
@@ -301,6 +305,101 @@ public class EntityRowMapperUnitTests {
 				.containsExactly(ID_FOR_ENTITY_NOT_REFERENCING_MAP, new ImmutableValue("ru'Ha'"));
 	}
 
+	@Test // DATAJDBC-370
+	@SneakyThrows
+	public void simplePrimitiveImmutableEmbeddedGetsProperlyExtracted() {
+
+		ResultSet rs = mockResultSet(asList("id", "value"), //
+				ID_FOR_ENTITY_NOT_REFERENCING_MAP, 24);
+		rs.next();
+
+		WithPrimitiveImmutableValue extracted = createRowMapper(WithPrimitiveImmutableValue.class).mapRow(rs, 1);
+
+		assertThat(extracted) //
+				.isNotNull() //
+				.extracting(e -> e.id, e -> e.embeddedImmutablePrimitiveValue) //
+				.containsExactly(ID_FOR_ENTITY_NOT_REFERENCING_MAP, new ImmutablePrimitiveValue(24));
+	}
+
+	@Test // DATAJDBC-370
+	public void simpleImmutableEmbeddedShouldBeNullIfAllOfTheEmbeddableAreNull() throws SQLException {
+
+		ResultSet rs = mockResultSet(asList("id", "value"), //
+				ID_FOR_ENTITY_NOT_REFERENCING_MAP, null);
+		rs.next();
+
+		WithImmutableValue extracted = createRowMapper(WithImmutableValue.class).mapRow(rs, 1);
+
+		assertThat(extracted) //
+				.isNotNull() //
+				.extracting(e -> e.id, e -> e.embeddedImmutableValue) //
+				.containsExactly(ID_FOR_ENTITY_NOT_REFERENCING_MAP, null);
+	}
+
+	@Test // DATAJDBC-370
+	@SneakyThrows
+	public void embeddedShouldBeNullWhenFieldsAreNull() {
+
+		ResultSet rs = mockResultSet(asList("id", "name", "prefix_id", "prefix_name"), //
+				ID_FOR_ENTITY_NOT_REFERENCING_MAP, "alpha", null, null);
+		rs.next();
+
+		EmbeddedEntity extracted = createRowMapper(EmbeddedEntity.class).mapRow(rs, 1);
+
+		assertThat(extracted) //
+				.isNotNull() //
+				.extracting(e -> e.id, e -> e.name, e -> e.children) //
+				.containsExactly(ID_FOR_ENTITY_NOT_REFERENCING_MAP, "alpha", null);
+	}
+
+	@Test // DATAJDBC-370
+	@SneakyThrows
+	public void embeddedShouldNotBeNullWhenAtLeastOneFieldIsNotNull() {
+
+		ResultSet rs = mockResultSet(asList("id", "name", "prefix_id", "prefix_name"), //
+				ID_FOR_ENTITY_NOT_REFERENCING_MAP, "alpha", 24, null);
+		rs.next();
+
+		EmbeddedEntity extracted = createRowMapper(EmbeddedEntity.class).mapRow(rs, 1);
+
+		assertThat(extracted) //
+				.isNotNull() //
+				.extracting(e -> e.id, e -> e.name, e -> e.children) //
+				.containsExactly(ID_FOR_ENTITY_NOT_REFERENCING_MAP, "alpha", new Trivial(24L, null));
+	}
+
+	@Test // DATAJDBC-370
+	@SneakyThrows
+	public void primitiveEmbeddedShouldBeNullWhenNoValuePresent() {
+
+		ResultSet rs = mockResultSet(asList("id", "value"), //
+				ID_FOR_ENTITY_NOT_REFERENCING_MAP, null);
+		rs.next();
+
+		WithPrimitiveImmutableValue extracted = createRowMapper(WithPrimitiveImmutableValue.class).mapRow(rs, 1);
+
+		assertThat(extracted) //
+				.isNotNull() //
+				.extracting(e -> e.id, e -> e.embeddedImmutablePrimitiveValue) //
+				.containsExactly(ID_FOR_ENTITY_NOT_REFERENCING_MAP, null);
+	}
+
+	@Test // DATAJDBC-370
+	@SneakyThrows
+	public void deepNestedEmbeddable() {
+
+		ResultSet rs = mockResultSet(asList("id", "level0", "level1_value", "level1_level2_value"), //
+				ID_FOR_ENTITY_NOT_REFERENCING_MAP, "0", "1", "2");
+		rs.next();
+
+		WithDeepNestedEmbeddable extracted = createRowMapper(WithDeepNestedEmbeddable.class).mapRow(rs, 1);
+
+		assertThat(extracted) //
+				.isNotNull() //
+				.extracting(e -> e.id, e -> extracted.level0, e -> e.level1.value, e -> e.level1.level2.value) //
+				.containsExactly(ID_FOR_ENTITY_NOT_REFERENCING_MAP, "0", "1", "2");
+	}
+
 	// Model classes to be used in tests
 
 	@Wither
@@ -311,6 +410,9 @@ public class EntityRowMapperUnitTests {
 		private final String name;
 	}
 
+	@EqualsAndHashCode
+	@NoArgsConstructor
+	@AllArgsConstructor
 	static class Trivial {
 
 		@Id Long id;
@@ -432,11 +534,35 @@ public class EntityRowMapperUnitTests {
 		@Embedded ImmutableValue embeddedImmutableValue;
 	}
 
+	static class WithPrimitiveImmutableValue {
+
+		@Id Long id;
+		@Embedded ImmutablePrimitiveValue embeddedImmutablePrimitiveValue;
+	}
+
 	@Value
 	static class ImmutableValue {
 		Object value;
 	}
 
+	@Value
+	static class ImmutablePrimitiveValue {
+		int value;
+	}
+
+	static class WithDeepNestedEmbeddable {
+
+		@Id Long id;
+		String level0;
+		@Embedded("level1_") EmbeddedWithEmbedded level1;
+	}
+
+	static class EmbeddedWithEmbedded {
+
+		Object value;
+		@Embedded("level2_") ImmutableValue level2;
+	}
+
 	// Infrastructure for assertions and constructing mocks
 
 	private <T> FixtureBuilder<T> buildFixture() {
@@ -454,22 +580,23 @@ public class EntityRowMapperUnitTests {
 		DataAccessStrategy accessStrategy = mock(DataAccessStrategy.class);
 
 		// the ID of the entity is used to determine what kind of ResultSet is needed for subsequent selects.
-		doReturn(new HashSet<>(asList(new Trivial(), new Trivial()))).when(accessStrategy)
+		doReturn(new HashSet<>(asList(new Trivial(1L, "one"), new Trivial(2L, "two")))).when(accessStrategy)
 				.findAllByProperty(eq(ID_FOR_ENTITY_NOT_REFERENCING_MAP), any(RelationalPersistentProperty.class));
 
 		doReturn(new HashSet<>(asList( //
-				new SimpleEntry<>("one", new Trivial()), //
-				new SimpleEntry<>("two", new Trivial()) //
+				new SimpleEntry<>("one", new Trivial(1L, "one")), //
+				new SimpleEntry<>("two", new Trivial(2L, "two")) //
 		))).when(accessStrategy).findAllByProperty(eq(ID_FOR_ENTITY_REFERENCING_MAP),
 				any(RelationalPersistentProperty.class));
 
 		doReturn(new HashSet<>(asList( //
-				new SimpleEntry<>(1, new Trivial()), //
-				new SimpleEntry<>(2, new Trivial()) //
+				new SimpleEntry<>(1, new Trivial(1L, "one")), //
+				new SimpleEntry<>(2, new Trivial(2L, "tow")) //
 		))).when(accessStrategy).findAllByProperty(eq(ID_FOR_ENTITY_REFERENCING_LIST),
 				any(RelationalPersistentProperty.class));
 
-		JdbcConverter converter = new BasicJdbcConverter(context, new JdbcCustomConversions());
+		JdbcConverter converter = new BasicJdbcConverter(context, new JdbcCustomConversions(),
+				JdbcTypeFactory.unsupported());
 
 		return new EntityRowMapper<>( //
 				(RelationalPersistentEntity<T>) context.getRequiredPersistentEntity(type), //

spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryEmbeddedIntegrationTests.java

@@ -44,6 +44,7 @@ import org.springframework.transaction.annotation.Transactional;
  * Very simple use cases for creation and usage of JdbcRepositories with test {@link Embedded} annotation in Entities.
  *
  * @author Bastian Wilhelm
+ * @author Christoph Strobl
  */
 @ContextConfiguration
 @Transactional
@@ -209,6 +210,14 @@ public class JdbcRepositoryEmbeddedIntegrationTests {
 		assertThat(repository.findAll()).isEmpty();
 	}
 
+	@Test // DATAJDBC-370
+	public void saveWithNullValueEmbeddable() {
+
+		DummyEntity entity = repository.save(new DummyEntity());
+
+		assertThat(JdbcTestUtils.countRowsInTableWhere((JdbcTemplate) template.getJdbcOperations(), "dummy_entity",
+				"id = " + entity.getId())).isEqualTo(1);
+	}
 
 	private static DummyEntity createDummyEntity() {
 		DummyEntity entity = new DummyEntity();
@@ -222,7 +231,6 @@ public class JdbcRepositoryEmbeddedIntegrationTests {
 
 		entity.setPrefixedEmbeddable(prefixedCascadedEmbeddable);
 
-
 		final CascadedEmbeddable cascadedEmbeddable = new CascadedEmbeddable();
 		cascadedEmbeddable.setTest("c2");