#402 - Exclude id property using initial value when inserting objects.

We now exclude Id properties from being used in the INSERT field list if the Id value is zero and of a primitive type or if the value is null using a numeric wrapper type.

Author: mp911de

src/main/asciidoc/reference/r2dbc-repositories.adoc

@@ -332,6 +332,9 @@ The ID of an entity must be annotated with Spring Data's https://docs.spring.io/
 
 When your database has an auto-increment column for the ID column, the generated value gets set in the entity after inserting it into the database.
 
+Spring Data R2DBC does not attempt to insert values of identifier columns when the entity is new and the identifier value defaults to its initial value.
+That is `0` for primitive types and `null` if the identifier property uses a numeric wrapper type such as `Long`.
+
 One important constraint is that, after saving an entity, the entity must not be new anymore.
 Note that whether an entity is new is part of the entity's state.
 With auto-increment columns, this happens automatically, because the ID gets set by Spring Data with the value from the ID column.
@@ -340,7 +343,8 @@ With auto-increment columns, this happens automatically, because the ID gets set
 === Optimistic Locking
 
 The `@Version` annotation provides syntax similar to that of JPA in the context of R2DBC and makes sure updates are only applied to documents with a matching version.
-Therefore, the actual value of the version property is added to the update query in such a way that the update does not have any effect if another operation altered the document in the meantime. In that case, an `OptimisticLockingFailureException` is thrown.
+Therefore, the actual value of the version property is added to the update query in such a way that the update does not have any effect if another operation altered the document in the meantime.
+In that case, an `OptimisticLockingFailureException` is thrown.
 The following example shows these features:
 
 ====
@@ -370,8 +374,8 @@ template.save(other).subscribe(); // emits OptimisticLockingFailureException
 ----
 <1> Initially insert row. `version` is set to `0`.
 <2> Load the just inserted row. `version` is still `0`.
-<3> Update the row with `version = 0`. Set the `lastname` and bump `version` to `1`.
-<4> Try to update the previously loaded document that still has `version = 0`. The operation fails with an `OptimisticLockingFailureException`, as the current `version` is `1`.
+<3> Update the row with `version = 0`.Set the `lastname` and bump `version` to `1`.
+<4> Try to update the previously loaded document that still has `version = 0`.The operation fails with an `OptimisticLockingFailureException`, as the current `version` is `1`.
 ====
 
 :projection-collection: Flux

src/main/java/org/springframework/data/r2dbc/convert/MappingR2dbcConverter.java

@@ -37,7 +37,6 @@
 import org.springframework.data.mapping.model.ConvertingPropertyAccessor;
 import org.springframework.data.mapping.model.ParameterValueProvider;
 import org.springframework.data.r2dbc.mapping.OutboundRow;
-import org.springframework.data.r2dbc.mapping.SettableValue;
 import org.springframework.data.r2dbc.support.ArrayUtils;
 import org.springframework.data.relational.core.conversion.BasicRelationalConverter;
 import org.springframework.data.relational.core.conversion.RelationalConverter;
@@ -330,11 +329,11 @@ private void writeInternal(Object source, OutboundRow sink, Class<?> userClass)
 		RelationalPersistentEntity<?> entity = getRequiredPersistentEntity(userClass);
 		PersistentPropertyAccessor<?> propertyAccessor = entity.getPropertyAccessor(source);
 
-		writeProperties(sink, entity, propertyAccessor);
+		writeProperties(sink, entity, propertyAccessor, entity.isNew(source));
 	}
 
 	private void writeProperties(OutboundRow sink, RelationalPersistentEntity<?> entity,
-			PersistentPropertyAccessor<?> accessor) {
+			PersistentPropertyAccessor<?> accessor, boolean isNew) {
 
 		for (RelationalPersistentProperty property : entity) {
 
@@ -350,18 +349,47 @@ private void writeProperties(OutboundRow sink, RelationalPersistentEntity<?> ent
 			}
 
 			if (getConversions().isSimpleType(value.getClass())) {
-				writeSimpleInternal(sink, value, property);
+				writeSimpleInternal(sink, value, isNew, property);
 			} else {
-				writePropertyInternal(sink, value, property);
+				writePropertyInternal(sink, value, isNew, property);
 			}
 		}
 	}
 
-	private void writeSimpleInternal(OutboundRow sink, Object value, RelationalPersistentProperty property) {
-		sink.put(property.getColumnName(), Parameter.from(getPotentiallyConvertedSimpleWrite(value)));
+	private void writeSimpleInternal(OutboundRow sink, Object value, boolean isNew,
+			RelationalPersistentProperty property) {
+
+		Object result = getPotentiallyConvertedSimpleWrite(value);
+
+		if (property.isIdProperty() && isNew) {
+			if (shouldSkipIdValue(result, property)) {
+				return;
+			}
+		}
+
+		sink.put(property.getColumnName(),
+				Parameter.fromOrEmpty(result, getPotentiallyConvertedSimpleNullType(property.getType())));
+	}
+
+	private boolean shouldSkipIdValue(@Nullable Object value, RelationalPersistentProperty property) {
+
+		if (value == null) {
+			return true;
+		}
+
+		if (!property.getType().isPrimitive()) {
+			return value == null;
+		}
+
+		if (Number.class.isInstance(value)) {
+			return ((Number) value).longValue() == 0L;
+		}
+
+		return false;
 	}
 
-	private void writePropertyInternal(OutboundRow sink, Object value, RelationalPersistentProperty property) {
+	private void writePropertyInternal(OutboundRow sink, Object value, boolean isNew,
+			RelationalPersistentProperty property) {
 
 		TypeInformation<?> valueType = ClassTypeInformation.from(value.getClass());
 
@@ -370,7 +398,7 @@ private void writePropertyInternal(OutboundRow sink, Object value, RelationalPer
 			if (valueType.getActualType() != null && valueType.getRequiredActualType().isCollectionLike()) {
 
 				// pass-thru nested collections
-				writeSimpleInternal(sink, value, property);
+				writeSimpleInternal(sink, value, isNew, property);
 				return;
 			}
 

src/test/java/org/springframework/data/r2dbc/convert/MappingR2dbcConverterUnitTests.java

@@ -20,6 +20,7 @@
 
 import io.r2dbc.spi.Row;
 import lombok.AllArgsConstructor;
+import lombok.RequiredArgsConstructor;
 
 import java.time.Instant;
 import java.time.LocalDateTime;
@@ -189,6 +190,24 @@ public void shouldReadTopLevelEntity() {
 		assertThat(result.entity).isNotNull();
 	}
 
+	@Test // gh-402
+	public void writeShouldSkipPrimitiveIdIfValueIsZero() {
+
+		OutboundRow row = new OutboundRow();
+		converter.write(new WithPrimitiveId(0), row);
+
+		assertThat(row).isEmpty();
+	}
+
+	@Test // gh-402
+	public void writeShouldWritePrimitiveIdIfValueIsNonZero() {
+
+		OutboundRow row = new OutboundRow();
+		converter.write(new WithPrimitiveId(1), row);
+
+		assertThat(row).containsEntry(SqlIdentifier.unquoted("id"), Parameter.fromOrEmpty(1L, Long.TYPE));
+	}
+
 	@AllArgsConstructor
 	static class Person {
 		@Id String id;
@@ -214,6 +233,12 @@ static class PersonWithConversions {
 		NonMappableEntity unsupported;
 	}
 
+	@RequiredArgsConstructor
+	static class WithPrimitiveId {
+
+		@Id final long id;
+	}
+
 	static class CustomConversionPerson {
 
 		String foo;