From 18782d70c98adbd33314f470673b739c5ff2173c Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Sat, 19 May 2018 00:04:33 +0200 Subject: [PATCH] DATACMNS-1326 - PersistentPropertyInspectingIsNewStrategy now considers primitive type's defaults. PersistentPropertyInspectingIsNewStrategy now considers entities with primitive default identifier values new. --- .../MappingContextIsNewStrategyFactory.java | 29 +------------------ ...gContextIsNewStrategyFactoryUnitTests.java | 26 ++++++++++++----- 2 files changed, 20 insertions(+), 35 deletions(-) diff --git a/src/main/java/org/springframework/data/mapping/context/MappingContextIsNewStrategyFactory.java b/src/main/java/org/springframework/data/mapping/context/MappingContextIsNewStrategyFactory.java index 658edceaf..756b54e6c 100644 --- a/src/main/java/org/springframework/data/mapping/context/MappingContextIsNewStrategyFactory.java +++ b/src/main/java/org/springframework/data/mapping/context/MappingContextIsNewStrategyFactory.java @@ -78,7 +78,7 @@ public class MappingContextIsNewStrategyFactory extends IsNewStrategyFactorySupp if (entity.hasVersionProperty()) { return new PropertyIsNullOrZeroNumberIsNewStrategy(entity.getVersionProperty()); } else if (entity.hasIdProperty()) { - return new PropertyIsNullIsNewStrategy(entity.getIdProperty()); + return new PropertyIsNullOrZeroNumberIsNewStrategy(entity.getIdProperty()); } else { throw new MappingException(String.format("Cannot determine IsNewStrategy for type %s!", type)); } @@ -119,33 +119,6 @@ public class MappingContextIsNewStrategyFactory extends IsNewStrategyFactorySupp protected abstract boolean decideIsNew(Object property); } - /** - * {@link IsNewStrategy} that does a check against {@literal null} for the given value and considers the object new if - * the value given is {@literal null}. - * - * @author Oliver Gierke - */ - static class PropertyIsNullIsNewStrategy extends PersistentPropertyInspectingIsNewStrategy { - - /** - * Creates a new {@link PropertyIsNullIsNewStrategy} using the given {@link PersistentProperty}. - * - * @param property must not be {@literal null}. - */ - public PropertyIsNullIsNewStrategy(PersistentProperty property) { - super(property); - } - - /* - * (non-Javadoc) - * @see org.springframework.data.mapping.model.MappingContextIsNewStrategyFactory.PersistentPropertyInspectingIsNewStrategy#decideIsNew(java.lang.Object) - */ - @Override - protected boolean decideIsNew(Object property) { - return property == null; - } - } - /** * {@link IsNewStrategy} that considers property values of {@literal null} or 0 (in case of a {@link Number}) * implementation as indicators for the new state. diff --git a/src/test/java/org/springframework/data/mapping/context/MappingContextIsNewStrategyFactoryUnitTests.java b/src/test/java/org/springframework/data/mapping/context/MappingContextIsNewStrategyFactoryUnitTests.java index f3872d665..366999206 100644 --- a/src/test/java/org/springframework/data/mapping/context/MappingContextIsNewStrategyFactoryUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/context/MappingContextIsNewStrategyFactoryUnitTests.java @@ -27,8 +27,6 @@ import org.junit.Test; import org.springframework.data.annotation.Id; import org.springframework.data.annotation.Version; import org.springframework.data.domain.Persistable; -import org.springframework.data.mapping.context.MappingContextIsNewStrategyFactory.PropertyIsNullIsNewStrategy; -import org.springframework.data.mapping.context.MappingContextIsNewStrategyFactory.PropertyIsNullOrZeroNumberIsNewStrategy; import org.springframework.data.support.IsNewStrategy; import org.springframework.data.support.IsNewStrategyFactory; @@ -45,7 +43,8 @@ public class MappingContextIsNewStrategyFactoryUnitTests { public void setUp() { SampleMappingContext context = new SampleMappingContext(); - context.setInitialEntitySet(new HashSet>(Arrays.> asList(Entity.class, VersionedEntity.class))); + context.setInitialEntitySet( + new HashSet>(Arrays.> asList(Entity.class, VersionedEntity.class, PrimitiveIdEntity.class))); context.afterPropertiesSet(); factory = new MappingContextIsNewStrategyFactory(new PersistentEntities(Collections.singleton(context))); @@ -55,7 +54,6 @@ public class MappingContextIsNewStrategyFactoryUnitTests { public void returnsPropertyIsNullOrZeroIsNewStrategyForVersionedEntity() { IsNewStrategy strategy = factory.getIsNewStrategy(VersionedEntity.class); - assertThat(strategy, is(instanceOf(PropertyIsNullOrZeroNumberIsNewStrategy.class))); VersionedEntity entity = new VersionedEntity(); assertThat(strategy.isNew(entity), is(true)); @@ -74,7 +72,6 @@ public class MappingContextIsNewStrategyFactoryUnitTests { public void returnsPropertyIsNullOrZeroIsNewStrategyForPrimitiveVersionedEntity() { IsNewStrategy strategy = factory.getIsNewStrategy(VersionedEntity.class); - assertThat(strategy, is(instanceOf(PropertyIsNullOrZeroNumberIsNewStrategy.class))); VersionedEntity entity = new VersionedEntity(); assertThat(strategy.isNew(entity), is(true)); @@ -90,7 +87,6 @@ public class MappingContextIsNewStrategyFactoryUnitTests { public void returnsPropertyIsNullIsNewStrategyForEntity() { IsNewStrategy strategy = factory.getIsNewStrategy(Entity.class); - assertThat(strategy, is(instanceOf(PropertyIsNullIsNewStrategy.class))); Entity entity = new Entity(); assertThat(strategy.isNew(entity), is(true)); @@ -99,6 +95,18 @@ public class MappingContextIsNewStrategyFactoryUnitTests { assertThat(strategy.isNew(entity), is(false)); } + @Test // DATACMNS-1326 + public void entityWithPrimitiveDefaultIsNotConsideredNew() { + + IsNewStrategy strategy = factory.getIsNewStrategy(PrimitiveIdEntity.class); + + PrimitiveIdEntity entity = new PrimitiveIdEntity(); + assertThat(strategy.isNew(entity), is(true)); + + entity.id = 1L; + assertThat(strategy.isNew(entity), is(false)); + } + @SuppressWarnings("serial") static class PersistableEntity implements Persistable { @@ -124,7 +132,7 @@ public class MappingContextIsNewStrategyFactoryUnitTests { @Id Long id; } - static class PrimitveVersionedEntity { + static class PrimitiveVersionedEntity { @Version long version = 0; @@ -135,4 +143,8 @@ public class MappingContextIsNewStrategyFactoryUnitTests { @Id Long id; } + + static class PrimitiveIdEntity { + @Id long id; + } }