From 67c3d492a26d08fb03b83669f2f871391c6ef9a3 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 5 Aug 2020 14:19:50 +0200 Subject: [PATCH] #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. --- .../reference/r2dbc-repositories.adoc | 10 ++-- .../r2dbc/convert/MappingR2dbcConverter.java | 46 +++++++++++++++---- .../MappingR2dbcConverterUnitTests.java | 25 ++++++++++ 3 files changed, 69 insertions(+), 12 deletions(-) diff --git a/src/main/asciidoc/reference/r2dbc-repositories.adoc b/src/main/asciidoc/reference/r2dbc-repositories.adoc index 59fddfe9..115133e3 100644 --- a/src/main/asciidoc/reference/r2dbc-repositories.adoc +++ b/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 diff --git a/src/main/java/org/springframework/data/r2dbc/convert/MappingR2dbcConverter.java b/src/main/java/org/springframework/data/r2dbc/convert/MappingR2dbcConverter.java index 80951fd9..6e1256e5 100644 --- a/src/main/java/org/springframework/data/r2dbc/convert/MappingR2dbcConverter.java +++ b/src/main/java/org/springframework/data/r2dbc/convert/MappingR2dbcConverter.java @@ -37,7 +37,6 @@ import org.springframework.data.mapping.context.MappingContext; 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 @@ public class MappingR2dbcConverter extends BasicRelationalConverter implements R 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 @@ public class MappingR2dbcConverter extends BasicRelationalConverter implements R } 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 void writePropertyInternal(OutboundRow sink, Object value, RelationalPersistentProperty property) { + 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, boolean isNew, + RelationalPersistentProperty property) { TypeInformation valueType = ClassTypeInformation.from(value.getClass()); @@ -370,7 +398,7 @@ public class MappingR2dbcConverter extends BasicRelationalConverter implements R if (valueType.getActualType() != null && valueType.getRequiredActualType().isCollectionLike()) { // pass-thru nested collections - writeSimpleInternal(sink, value, property); + writeSimpleInternal(sink, value, isNew, property); return; } diff --git a/src/test/java/org/springframework/data/r2dbc/convert/MappingR2dbcConverterUnitTests.java b/src/test/java/org/springframework/data/r2dbc/convert/MappingR2dbcConverterUnitTests.java index 61a9244a..e5f49bf8 100644 --- a/src/test/java/org/springframework/data/r2dbc/convert/MappingR2dbcConverterUnitTests.java +++ b/src/test/java/org/springframework/data/r2dbc/convert/MappingR2dbcConverterUnitTests.java @@ -20,6 +20,7 @@ import static org.mockito.Mockito.*; import io.r2dbc.spi.Row; import lombok.AllArgsConstructor; +import lombok.RequiredArgsConstructor; import java.time.Instant; import java.time.LocalDateTime; @@ -189,6 +190,24 @@ public class MappingR2dbcConverterUnitTests { 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 @@ public class MappingR2dbcConverterUnitTests { NonMappableEntity unsupported; } + @RequiredArgsConstructor + static class WithPrimitiveId { + + @Id final long id; + } + static class CustomConversionPerson { String foo;