From 11058afd8f4ea8fe63b1bef11aa2d78b987a38b3 Mon Sep 17 00:00:00 2001 From: John Blum Date: Thu, 16 Jan 2020 11:12:25 -0800 Subject: [PATCH] DATAGEODE-291 - Refactor MappingPdxSerializer Unit and Integration Tests for resolving the EntityInstantiator. --- .../gemfire/mapping/MappingPdxSerializer.java | 22 +++-- .../MappingPdxSerializerIntegrationTests.java | 94 +++++++++++++++---- .../MappingPdxSerializerUnitTests.java | 68 +++++++++++--- 3 files changed, 148 insertions(+), 36 deletions(-) diff --git a/spring-data-geode/src/main/java/org/springframework/data/gemfire/mapping/MappingPdxSerializer.java b/spring-data-geode/src/main/java/org/springframework/data/gemfire/mapping/MappingPdxSerializer.java index 9faf89e0..53f197f1 100644 --- a/spring-data-geode/src/main/java/org/springframework/data/gemfire/mapping/MappingPdxSerializer.java +++ b/spring-data-geode/src/main/java/org/springframework/data/gemfire/mapping/MappingPdxSerializer.java @@ -475,7 +475,7 @@ public class MappingPdxSerializer implements PdxSerializer, ApplicationContextAw * @see java.lang.Object * @see java.lang.Class */ - @SuppressWarnings("unchecked") + @SuppressWarnings({ "rawtypes", "unchecked" }) Object doFromData(Class type, PdxReader reader) { GemfirePersistentEntity entity = getPersistentEntity(type); @@ -493,6 +493,8 @@ public class MappingPdxSerializer implements PdxSerializer, ApplicationContextAw PdxSerializer customPdxSerializer = resolveCustomPdxSerializer(persistentProperty); + boolean isCustomPdxSerializerPresent = customPdxSerializer != null; + Object value = null; try { @@ -502,9 +504,9 @@ public class MappingPdxSerializer implements PdxSerializer, ApplicationContextAw ? String.format(" using custom PdxSerializer [%s]", customPdxSerializer) : ""))); } - value = (customPdxSerializer != null + value = isCustomPdxSerializerPresent ? customPdxSerializer.fromData(persistentProperty.getType(), reader) - : reader.readField(persistentProperty.getName())); + : reader.readField(persistentProperty.getName()); if (getLogger().isDebugEnabled()) { getLogger().debug(String.format("... with value [%s]", value)); @@ -513,10 +515,16 @@ public class MappingPdxSerializer implements PdxSerializer, ApplicationContextAw propertyAccessor.setProperty(persistentProperty, value); } catch (Exception cause) { - throw new MappingException( - String.format("While setting value [%1$s] of property [%2$s] for entity of type [%3$s] from PDX%4$s", - value, persistentProperty.getName(), type, (customPdxSerializer != null - ? String.format(" using custom PdxSerializer [%s]", customPdxSerializer) : "")), cause); + + String MAPPING_ERROR_MESSAGE = + "While setting value [%1$s] of property [%2$s] for entity of type [%3$s] from PDX%4$s"; + + String CUSTOM_PDX_SERIALIZER_MESSAGE = isCustomPdxSerializerPresent + ? String.format(" using custom PdxSerializer [%s]", customPdxSerializer) + : ""; + + throw new MappingException(String.format(MAPPING_ERROR_MESSAGE, + value, persistentProperty.getName(), type, CUSTOM_PDX_SERIALIZER_MESSAGE), cause); } } }); diff --git a/spring-data-geode/src/test/java/org/springframework/data/gemfire/mapping/MappingPdxSerializerIntegrationTests.java b/spring-data-geode/src/test/java/org/springframework/data/gemfire/mapping/MappingPdxSerializerIntegrationTests.java index 0b1a3a86..3067b4c9 100644 --- a/spring-data-geode/src/test/java/org/springframework/data/gemfire/mapping/MappingPdxSerializerIntegrationTests.java +++ b/spring-data-geode/src/test/java/org/springframework/data/gemfire/mapping/MappingPdxSerializerIntegrationTests.java @@ -13,17 +13,18 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.springframework.data.gemfire.mapping; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.isA; +import static org.mockito.Mockito.atLeast; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import java.io.DataInput; @@ -34,14 +35,13 @@ import java.time.LocalDateTime; import java.time.Month; import java.util.Base64; import java.util.Collections; +import java.util.Map; import java.util.Optional; -import lombok.AllArgsConstructor; -import lombok.Data; -import lombok.Getter; -import lombok.NonNull; -import lombok.RequiredArgsConstructor; -import lombok.Setter; +import org.junit.After; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; import org.apache.geode.DataSerializable; import org.apache.geode.Instantiator; @@ -53,19 +53,23 @@ import org.apache.geode.pdx.PdxReader; import org.apache.geode.pdx.PdxSerializer; import org.apache.geode.pdx.PdxWriter; -import org.junit.After; -import org.junit.AfterClass; -import org.junit.BeforeClass; -import org.junit.Test; - import org.springframework.data.annotation.ReadOnlyProperty; import org.springframework.data.annotation.Transient; +import org.springframework.data.convert.EntityInstantiator; import org.springframework.data.gemfire.GemfireUtils; import org.springframework.data.gemfire.repository.sample.Address; import org.springframework.data.gemfire.repository.sample.Person; +import org.springframework.data.mapping.PersistentEntity; + +import lombok.AllArgsConstructor; +import lombok.Data; +import lombok.Getter; +import lombok.NonNull; +import lombok.RequiredArgsConstructor; +import lombok.Setter; /** - * Integration tests for {@link MappingPdxSerializer}. + * Integration Tests for {@link MappingPdxSerializer}. * * @author Oliver Gierke * @author John Blum @@ -97,7 +101,6 @@ public class MappingPdxSerializerIntegrationTests { } @AfterClass - @SuppressWarnings("all") public static void tearDown() { GemfireUtils.close(cache); } @@ -154,7 +157,66 @@ public class MappingPdxSerializerIntegrationTests { } @Test - @SuppressWarnings("unchecked") + @SuppressWarnings("rawtypes") + public void resolveEntityInstantiatorForManagedPersistentEntityWithEntityInstantiator() { + + EntityInstantiator mockEntityInstantiator = mock(EntityInstantiator.class); + + Map, EntityInstantiator> entityInstantiators = + Collections.singletonMap(Person.class, mockEntityInstantiator); + + PersistentEntity mockEntity = mock(PersistentEntity.class); + + when(mockEntity.getType()).thenReturn(Person.class); + + assertThat(cache.getPdxSerializer()).isInstanceOf(MappingPdxSerializer.class); + + MappingPdxSerializer serializer = ((MappingPdxSerializer) cache.getPdxSerializer()); + + try { + serializer.setEntityInstantiators(entityInstantiators); + + assertThat(serializer.resolveEntityInstantiator(mockEntity)).isEqualTo(mockEntityInstantiator); + } + finally { + serializer.setEntityInstantiators(Collections.emptyMap()); + } + + verify(mockEntity, atLeast(1)).getType(); + verifyNoInteractions(mockEntityInstantiator); + } + + @Test + @SuppressWarnings("rawtypes") + public void resolveEntityInstantiatorForNonManagedPersistentEntityWithNoEntityInstantiator() { + + EntityInstantiator mockEntityInstantiator = mock(EntityInstantiator.class); + + Map, EntityInstantiator> entityInstantiators = + Collections.singletonMap(Person.class, mockEntityInstantiator); + + PersistentEntity mockEntity = mock(PersistentEntity.class); + + when(mockEntity.getType()).thenReturn(Address.class); + + assertThat(cache.getPdxSerializer()).isInstanceOf(MappingPdxSerializer.class); + + MappingPdxSerializer serializer = ((MappingPdxSerializer) cache.getPdxSerializer()); + + try { + serializer.setEntityInstantiators(entityInstantiators); + + assertThat(serializer.resolveEntityInstantiator(mockEntity)).isNotEqualTo(mockEntityInstantiator); + } + finally { + serializer.setEntityInstantiators(Collections.emptyMap()); + } + + verify(mockEntity, atLeast(1)).getType(); + verifyNoInteractions(mockEntityInstantiator); + } + + @Test public void serializesAndDeserializesEntity() { Address address = new Address(); @@ -384,7 +446,7 @@ public class MappingPdxSerializerIntegrationTests { @Override - public void fromData(DataInput dataInput) throws IOException, ClassNotFoundException { + public void fromData(DataInput dataInput) throws IOException { this.value = dataInput.readUTF(); } diff --git a/spring-data-geode/src/test/java/org/springframework/data/gemfire/mapping/MappingPdxSerializerUnitTests.java b/spring-data-geode/src/test/java/org/springframework/data/gemfire/mapping/MappingPdxSerializerUnitTests.java index a990a141..509bab99 100644 --- a/spring-data-geode/src/test/java/org/springframework/data/gemfire/mapping/MappingPdxSerializerUnitTests.java +++ b/spring-data-geode/src/test/java/org/springframework/data/gemfire/mapping/MappingPdxSerializerUnitTests.java @@ -13,16 +13,14 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.springframework.data.gemfire.mapping; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.isA; import static org.mockito.ArgumentMatchers.isNull; -import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anyString; -import static org.mockito.Matchers.eq; -import static org.mockito.Mockito.atLeast; import static org.mockito.Mockito.atMost; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; @@ -30,7 +28,7 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import static org.springframework.data.gemfire.util.RuntimeExceptionFactory.newIllegalArgumentException; @@ -42,16 +40,16 @@ import java.util.Optional; import com.gemstone.gemfire.TestGemStoneGemFireType; -import org.apache.geode.pdx.PdxReader; -import org.apache.geode.pdx.PdxSerializer; -import org.apache.geode.pdx.PdxWriter; - import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; +import org.apache.geode.pdx.PdxReader; +import org.apache.geode.pdx.PdxSerializer; +import org.apache.geode.pdx.PdxWriter; + import org.springframework.core.Ordered; import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.support.DefaultConversionService; @@ -79,11 +77,10 @@ import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.model.ParameterValueProvider; /** - * Unit tests for {@link MappingPdxSerializer}. + * Unit Tests for {@link MappingPdxSerializer}. * * @author Oliver Gierke * @author John Blum - * @see org.junit.Rule * @see org.junit.Test * @see org.junit.runner.RunWith * @see org.mockito.Mock @@ -260,6 +257,7 @@ public class MappingPdxSerializerUnitTests { assertThat(this.pdxSerializer.getEntityInstantiators()).isSameAs(mockEntityInstantiators); } + @SuppressWarnings("all") @Test(expected = IllegalArgumentException.class) public void setEntityInstantiatorsWithNullEntityInstantiators() { @@ -286,6 +284,7 @@ public class MappingPdxSerializerUnitTests { assertThat(this.pdxSerializer.getEntityInstantiators()).isInstanceOf(EntityInstantiators.class); } + @SuppressWarnings("all") @Test(expected = IllegalArgumentException.class) public void setEntityInstantiatorsWithNullMap() { @@ -467,6 +466,47 @@ public class MappingPdxSerializerUnitTests { assertThat(this.pdxSerializer.resolveCustomPdxSerializer(addressProperty)).isEqualTo(mockTypedSerializer); } + @Test + @SuppressWarnings("rawtypes") + public void resolveEntityInstantiatorForManagedPersistentEntityWithEntityInstantiator() { + + EntityInstantiator mockEntityInstantiator = mock(EntityInstantiator.class); + + EntityInstantiators mockEntityInstantiators = mock(EntityInstantiators.class); + + PersistentEntity mockEntity = mock(PersistentEntity.class); + + doReturn(mockEntityInstantiators).when(this.pdxSerializer).getEntityInstantiators(); + doReturn(mockEntityInstantiator).when(mockEntityInstantiators).getInstantiatorFor(eq(mockEntity)); + + assertThat(this.pdxSerializer.resolveEntityInstantiator(mockEntity)).isEqualTo(mockEntityInstantiator); + + verify(this.pdxSerializer, times(1)).getEntityInstantiators(); + verify(mockEntityInstantiators, times(1)).getInstantiatorFor(eq(mockEntity)); + verifyNoInteractions(mockEntityInstantiator); + verifyNoInteractions(mockEntity); + } + + @Test + @SuppressWarnings("rawtypes") + public void resolveEntityInstantiatorForNonManagedPersistentEntityWithNoEntityInstantiator() { + + EntityInstantiator mockEntityInstantiator = mock(EntityInstantiator.class); + + EntityInstantiators mockEntityInstantiators = mock(EntityInstantiators.class); + + PersistentEntity mockEntity = mock(PersistentEntity.class); + + doReturn(mockEntityInstantiators).when(this.pdxSerializer).getEntityInstantiators(); + + assertThat(this.pdxSerializer.resolveEntityInstantiator(mockEntity)).isNotEqualTo(mockEntityInstantiator); + + verify(this.pdxSerializer, times(1)).getEntityInstantiators(); + verify(mockEntityInstantiators, times(1)).getInstantiatorFor(eq(mockEntity)); + verifyNoInteractions(mockEntityInstantiator); + verifyNoInteractions(mockEntity); + } + @Test public void resolveTypeWithNonNullType() { assertThat(this.pdxSerializer.resolveType("test")).isEqualTo(String.class); @@ -478,9 +518,11 @@ public class MappingPdxSerializerUnitTests { } @Test + @SuppressWarnings("rawtypes") public void toFullyQualifiedPropertyName() { PersistentEntity mockEntity = mock(PersistentEntity.class); + PersistentProperty mockProperty = mock(PersistentProperty.class); when(mockProperty.getName()).thenReturn("mockProperty"); @@ -571,7 +613,7 @@ public class MappingPdxSerializerUnitTests { } @Test - @SuppressWarnings("unchecked") + @SuppressWarnings("all") public void fromDataUsesRegisteredEntityInstantiator() { Address address = new Address();