From ebcea509b88e2f75c2f32541d27a2c1e87cb563a Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 14 Jul 2020 09:38:06 +0200 Subject: [PATCH] DATACMNS-1768 - Reuse bean state in InstantiationAwarePropertyAccessor when setting multiple properties. We now reuse the new bean in InstantiationAwarePropertyAccessor when setting properties. Previously, we used the initial bean state as the bean was held by a delegate PersistentPropertyAccessor which caused only the last set property to be visible. --- .../InstantiationAwarePropertyAccessor.java | 39 ++++++++++++++++--- ...antiationAwarePropertyAccessorFactory.java | 8 ++-- ...rePersistentPropertyAccessorUnitTests.java | 28 +++++++++++-- .../model/BasicPersistentEntityUnitTests.java | 9 +++-- 4 files changed, 69 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/springframework/data/mapping/model/InstantiationAwarePropertyAccessor.java b/src/main/java/org/springframework/data/mapping/model/InstantiationAwarePropertyAccessor.java index 3035053a3..32012de9d 100644 --- a/src/main/java/org/springframework/data/mapping/model/InstantiationAwarePropertyAccessor.java +++ b/src/main/java/org/springframework/data/mapping/model/InstantiationAwarePropertyAccessor.java @@ -1,5 +1,5 @@ /* - * Copyright 2019 the original author or authors. + * Copyright 2019-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,6 +15,8 @@ */ package org.springframework.data.mapping.model; +import java.util.function.Function; + import org.springframework.data.annotation.PersistenceConstructor; import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentProperty; @@ -31,13 +33,15 @@ import org.springframework.util.Assert; * {@link PersistentProperty} is to be applied on a completely immutable entity type exposing a persistence constructor. * * @author Oliver Drotbohm + * @author Mark Paluch + * @since 2.3 */ public class InstantiationAwarePropertyAccessor implements PersistentPropertyAccessor { private static final String NO_SETTER_OR_CONSTRUCTOR = "Cannot set property %s because no setter, wither or copy constructor exists for %s!"; private static final String NO_CONSTRUCTOR_PARAMETER = "Cannot set property %s because no setter, no wither and it's not part of the persistence constructor %s!"; - private final PersistentPropertyAccessor delegate; + private final Function> delegateFunction; private final EntityInstantiators instantiators; private T bean; @@ -48,17 +52,41 @@ public class InstantiationAwarePropertyAccessor implements PersistentProperty * * @param delegate must not be {@literal null}. * @param instantiators must not be {@literal null}. + * @deprecated since 2.4. Using this constructor allows only setting a single property as + * {@link PersistentPropertyAccessor} holds a reference to the initial bean state. */ + @Deprecated public InstantiationAwarePropertyAccessor(PersistentPropertyAccessor delegate, EntityInstantiators instantiators) { - Assert.notNull(delegate, "Delegate PersistenPropertyAccessor must not be null!"); + Assert.notNull(delegate, "Delegate PersistentPropertyAccessor must not be null!"); Assert.notNull(instantiators, "EntityInstantiators must not be null!"); - this.delegate = delegate; this.instantiators = instantiators; + this.delegateFunction = t -> delegate; this.bean = delegate.getBean(); } + /** + * Creates an {@link InstantiationAwarePropertyAccessor} using the given delegate {@code accessorFunction} and + * {@link EntityInstantiators}. {@code accessorFunction} is used to obtain a new {@link PersistentPropertyAccessor} + * for each property to set. + * + * @param bean must not be {@literal null}. + * @param accessorFunction must not be {@literal null}. + * @param instantiators must not be {@literal null}. + */ + public InstantiationAwarePropertyAccessor(T bean, Function> accessorFunction, + EntityInstantiators instantiators) { + + Assert.notNull(bean, "Bean must not be null!"); + Assert.notNull(accessorFunction, "PersistentPropertyAccessor function must not be null!"); + Assert.notNull(instantiators, "EntityInstantiators must not be null!"); + + this.delegateFunction = accessorFunction; + this.instantiators = instantiators; + this.bean = bean; + } + /* * (non-Javadoc) * @see org.springframework.data.mapping.PersistentPropertyAccessor#setProperty(org.springframework.data.mapping.PersistentProperty, java.lang.Object) @@ -68,6 +96,7 @@ public class InstantiationAwarePropertyAccessor implements PersistentProperty public void setProperty(PersistentProperty property, @Nullable Object value) { PersistentEntity owner = property.getOwner(); + PersistentPropertyAccessor delegate = delegateFunction.apply(this.bean); if (!property.isImmutable() || property.getWither() != null || ReflectionUtils.isKotlinClass(owner.getType())) { @@ -123,7 +152,7 @@ public class InstantiationAwarePropertyAccessor implements PersistentProperty @Nullable @Override public Object getProperty(PersistentProperty property) { - return delegate.getProperty(property); + return delegateFunction.apply(bean).getProperty(property); } /* diff --git a/src/main/java/org/springframework/data/mapping/model/InstantiationAwarePropertyAccessorFactory.java b/src/main/java/org/springframework/data/mapping/model/InstantiationAwarePropertyAccessorFactory.java index 29dbb1820..728fd2179 100644 --- a/src/main/java/org/springframework/data/mapping/model/InstantiationAwarePropertyAccessorFactory.java +++ b/src/main/java/org/springframework/data/mapping/model/InstantiationAwarePropertyAccessorFactory.java @@ -23,6 +23,8 @@ import org.springframework.data.mapping.PersistentPropertyAccessor; * an {@link InstantiationAwarePropertyAccessor} to allow the handling of purely immutable types. * * @author Oliver Drotbohm + * @author Mark Paluch + * @since 2.3 */ public class InstantiationAwarePropertyAccessorFactory implements PersistentPropertyAccessorFactory { @@ -41,10 +43,8 @@ public class InstantiationAwarePropertyAccessorFactory implements PersistentProp */ @Override public PersistentPropertyAccessor getPropertyAccessor(PersistentEntity entity, T bean) { - - PersistentPropertyAccessor accessor = delegate.getPropertyAccessor(entity, bean); - - return new InstantiationAwarePropertyAccessor<>(accessor, instantiators); + return new InstantiationAwarePropertyAccessor<>(bean, it -> delegate.getPropertyAccessor(entity, it), + instantiators); } /* diff --git a/src/test/java/org/springframework/data/mapping/InstantiationAwarePersistentPropertyAccessorUnitTests.java b/src/test/java/org/springframework/data/mapping/InstantiationAwarePersistentPropertyAccessorUnitTests.java index 24e2123bf..b7da2b4af 100644 --- a/src/test/java/org/springframework/data/mapping/InstantiationAwarePersistentPropertyAccessorUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/InstantiationAwarePersistentPropertyAccessorUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2019 the original author or authors. + * Copyright 2019-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -26,12 +26,15 @@ import org.springframework.data.mapping.context.SamplePersistentProperty; import org.springframework.data.mapping.model.InstantiationAwarePropertyAccessor; /** + * Unit tests for {@link InstantiationAwarePropertyAccessor}. + * * @author Oliver Drotbohm + * @author Mark Paluch */ public class InstantiationAwarePersistentPropertyAccessorUnitTests { - @Test - public void testname() { + @Test // DATACMNS-1639 + public void shouldCreateNewInstance() { EntityInstantiators instantiators = new EntityInstantiators(); SampleMappingContext context = new SampleMappingContext(); @@ -48,6 +51,25 @@ public class InstantiationAwarePersistentPropertyAccessorUnitTests { assertThat(wrapper.getBean()).isEqualTo(new Sample("Oliver August", "Matthews", 42)); } + @Test // DATACMNS-1768 + public void shouldSetMultipleProperties() { + + EntityInstantiators instantiators = new EntityInstantiators(); + SampleMappingContext context = new SampleMappingContext(); + + PersistentEntity entity = context.getRequiredPersistentEntity(Sample.class); + + Sample bean = new Sample("Dave", "Matthews", 42); + + PersistentPropertyAccessor wrapper = new InstantiationAwarePropertyAccessor<>(bean, + entity::getPropertyAccessor, instantiators); + + wrapper.setProperty(entity.getRequiredPersistentProperty("firstname"), "Oliver August"); + wrapper.setProperty(entity.getRequiredPersistentProperty("lastname"), "Heisenberg"); + + assertThat(wrapper.getBean()).isEqualTo(new Sample("Oliver August", "Heisenberg", 42)); + } + @Value static class Sample { diff --git a/src/test/java/org/springframework/data/mapping/model/BasicPersistentEntityUnitTests.java b/src/test/java/org/springframework/data/mapping/model/BasicPersistentEntityUnitTests.java index 22114f45f..aa63a1b69 100755 --- a/src/test/java/org/springframework/data/mapping/model/BasicPersistentEntityUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/model/BasicPersistentEntityUnitTests.java @@ -28,6 +28,7 @@ import java.util.Iterator; import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Function; import java.util.stream.Stream; import org.junit.jupiter.api.Test; @@ -186,7 +187,8 @@ class BasicPersistentEntityUnitTests> { .hasMessageContaining("Required property foo not found"); } - @Test // DATACMNS-809 + @Test // DATACMNS-809, DATACMNS-1768 + @SuppressWarnings("rawtypes") void returnsGeneratedPropertyAccessorForPropertyAccessor() { SampleMappingContext context = new SampleMappingContext(); @@ -196,11 +198,12 @@ class BasicPersistentEntityUnitTests> { PersistentPropertyAccessor accessor = entity.getPropertyAccessor(value); assertThat(accessor).isNotInstanceOf(BeanWrapper.class); - assertThat(accessor).isInstanceOfSatisfying(InstantiationAwarePropertyAccessor.class, it -> { - PersistentPropertyAccessor delegate = (PersistentPropertyAccessor) ReflectionTestUtils.getField(it, "delegate"); + Function> delegateFunction = (Function>) ReflectionTestUtils + .getField(it, "delegateFunction"); + PersistentPropertyAccessor delegate = delegateFunction.apply(value); assertThat(delegate.getClass().getName()).contains("_Accessor_"); assertThat(delegate.getBean()).isEqualTo(value); });