diff --git a/spring-geode/src/main/java/org/springframework/geode/data/json/JsonCacheDataImporterExporter.java b/spring-geode/src/main/java/org/springframework/geode/data/json/JsonCacheDataImporterExporter.java index b5a77665..4da78232 100644 --- a/spring-geode/src/main/java/org/springframework/geode/data/json/JsonCacheDataImporterExporter.java +++ b/spring-geode/src/main/java/org/springframework/geode/data/json/JsonCacheDataImporterExporter.java @@ -24,6 +24,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.Writer; import java.util.Arrays; +import java.util.Objects; import java.util.Optional; import org.apache.geode.cache.Region; @@ -43,6 +44,7 @@ import org.springframework.geode.data.json.converter.ObjectToJsonConverter; import org.springframework.geode.data.json.converter.support.JSONFormatterJsonToPdxConverter; import org.springframework.geode.data.json.converter.support.JSONFormatterPdxToJsonConverter; import org.springframework.geode.data.json.converter.support.JacksonJsonToPdxConverter; +import org.springframework.geode.pdx.PdxInstanceWrapper; import org.springframework.geode.util.CacheUtils; import org.springframework.lang.NonNull; import org.springframework.lang.Nullable; @@ -66,6 +68,10 @@ import org.springframework.util.StringUtils; * @see org.springframework.geode.data.json.converter.JsonToPdxArrayConverter * @see org.springframework.geode.data.json.converter.JsonToPdxConverter * @see org.springframework.geode.data.json.converter.ObjectToJsonConverter + * @see org.springframework.geode.data.json.converter.support.JSONFormatterJsonToPdxConverter + * @see org.springframework.geode.data.json.converter.support.JSONFormatterPdxToJsonConverter + * @see org.springframework.geode.data.json.converter.support.JacksonJsonToPdxConverter + * @see org.springframework.geode.pdx.PdxInstanceWrapper * @see org.springframework.stereotype.Component * @since 1.3.0 */ @@ -78,9 +84,15 @@ public class JsonCacheDataImporterExporter extends AbstractCacheDataImporterExpo private static final int CONTENT_PREVIEW_LENGTH = 50; private static final int DEFAULT_BUFFER_SIZE = 32768; + protected static final String ARRAY_BEGIN = "["; + protected static final String ARRAY_END = "]"; + protected static final String AT_IDENTIFIER_FIELD_NAME = PdxInstanceWrapper.AT_IDENTIFIER_FIELD_NAME; protected static final String CLASSPATH_RESOURCE_PREFIX = ResourceLoader.CLASSPATH_URL_PREFIX; + protected static final String COMMA_SPACE = ", "; + protected static final String EMPTY_STRING = ""; protected static final String FILESYSTEM_RESOURCE_PREFIX = "file://"; - protected static final String ID_FIELD_NAME = "id"; + protected static final String ID_FIELD_NAME = PdxInstanceWrapper.ID_FIELD_NAME; + protected static final String NO_FIELD_NAME = ""; protected static final String RESOURCE_NAME_PATTERN = "data-%s.json"; private JsonToPdxConverter toPdxConverter = newJsonToPdxConverter(); @@ -208,8 +220,10 @@ public class JsonCacheDataImporterExporter extends AbstractCacheDataImporterExpo .map(this::getContent) .map(this::toPdxArray) .map(Arrays::stream) - .ifPresent(pdxInstances -> pdxInstances.forEach(pdxInstance -> - region.put(getIdentifier(pdxInstance), pdxInstance))); + .ifPresent(pdxInstances -> pdxInstances.forEach(pdxInstance -> { + pdxInstance = postProcess(pdxInstance); + region.put(getIdentifier(pdxInstance), pdxInstance); + })); return region; } @@ -251,14 +265,15 @@ public class JsonCacheDataImporterExporter extends AbstractCacheDataImporterExpo } /** - * Determines the {@link Object identifier} for the given {@link PdxInstance}. + * Determines the {@link Object identifier} (a.k.a. {@link PdxInstance#isIdentityField(String) identity}) + * for the given {@link PdxInstance}. * * @param pdxInstance {@link PdxInstance} to evaluate; must not be {@literal null}. - * @return the {@link Object identifier} for the given {@link PdxInstance}. + * @return the {@link Object identifier} for the given {@link PdxInstance}; never {@literal null}. * @throws IllegalArgumentException if {@link PdxInstance} is {@literal null}. - * @throws IllegalStateException if the {@link Object identifier} for the {@link PdxInstance} cannot be determined. + * @throws IllegalStateException if the {@link PdxInstance} does not have an id. * @see org.apache.geode.pdx.PdxInstance - * @see #getIdField(PdxInstance) + * @see #getId(PdxInstance) */ protected @NonNull Object getIdentifier(@NonNull PdxInstance pdxInstance) { @@ -271,28 +286,83 @@ public class JsonCacheDataImporterExporter extends AbstractCacheDataImporterExpo return idFieldName .map(pdxInstance::getField) - .orElseGet(() -> getIdField(pdxInstance)); + .orElseGet(() -> getId(pdxInstance)); } /** * Searches for a PDX {@link String field name} called {@literal id} on the given {@link PdxInstance} - * and returns its value as the {@link Object identifier} for the {@link PdxInstance}. + * and returns its {@link Object value} as the {@link Object identifier} for the {@link PdxInstance}. * * @param pdxInstance {@link PdxInstance} to evaluate; must not be {@literal null}. * @return the {@link Object value} of the {@literal id} {@link String field} on the {@link PdxInstance}. * @throws IllegalStateException if the {@link PdxInstance} does not have an id. * @see org.apache.geode.pdx.PdxInstance - * @see #getIdentifier(PdxInstance) + * @see #getAtIdentifier(PdxInstance) */ - protected @NonNull Object getIdField(@NonNull PdxInstance pdxInstance) { + protected @NonNull Object getId(@NonNull PdxInstance pdxInstance) { Assert.notNull(pdxInstance, "PdxInstance must not be null"); return Optional.of(pdxInstance) .filter(it -> it.hasField(ID_FIELD_NAME)) .map(it -> it.getField(ID_FIELD_NAME)) - .orElseThrow(() -> newIllegalStateException("PdxInstance for type [%1$s] has no %2$s", - pdxInstance.getClassName(), pdxInstance.hasField(ID_FIELD_NAME) ? "id" : "declared identity field")); + .orElseGet(() -> getAtIdentifier(pdxInstance)); + } + + /** + * Searches for a PDX {@link String field} declared by the {@literal @identifier} metadata {@link String field} + * on the given {@link PdxInstance} and returns the {@link Object value} of this {@link String field} + * as the {@link Object identifier} for the {@link PdxInstance}. + * + * @param pdxInstance {@link PdxInstance} to evaluate; most not be {@literal null}. + * @return the {@link Object value} of the {@link String field} declared in the {@literal @identifier} metadata + * {@link String field} on the {@link PdxInstance}. + * @throws IllegalArgumentException if {@link PdxInstance} is {@literal null}. + * @throws IllegalStateException if the {@link PdxInstance} does not have an id. + * @see org.apache.geode.pdx.PdxInstance + */ + protected @NonNull Object getAtIdentifier(@NonNull PdxInstance pdxInstance) { + + Assert.notNull(pdxInstance, "PdxInstance must not be null"); + + return Optional.of(pdxInstance) + .filter(pdx -> pdx.hasField(AT_IDENTIFIER_FIELD_NAME)) + .map(pdx -> pdx.getField(AT_IDENTIFIER_FIELD_NAME)) + .map(String::valueOf) + .filter(pdxInstance::hasField) + .map(pdxInstance::getField) + .orElseThrow(() -> newIllegalStateException(String.format("PdxInstance for type [%1$s] has no %2$s", + pdxInstance.getClassName(), resolveMessageForIdentifierError(pdxInstance)))); + } + + private @NonNull String resolveMessageForIdentifierError(@NonNull PdxInstance pdxInstance) { + + String message = "declared identifier"; + + if (pdxInstance.hasField(ID_FIELD_NAME)) { + message = "id"; + } + else if (pdxInstance.hasField(AT_IDENTIFIER_FIELD_NAME)) { + + Object atIdentifierFieldValue = pdxInstance.getField(AT_IDENTIFIER_FIELD_NAME); + + String resolvedIdentifierFieldName = Objects.nonNull(atIdentifierFieldValue) + ? atIdentifierFieldValue.toString().trim() + : NO_FIELD_NAME; + + boolean identifierFieldNameWasDeclaredAndIsValid = pdxInstance.hasField(resolvedIdentifierFieldName); + + Object identifier = identifierFieldNameWasDeclaredAndIsValid + ? pdxInstance.getField(resolvedIdentifierFieldName) + : null; + + message = identifierFieldNameWasDeclaredAndIsValid + ? String.format("value [%s] for field [%s] declared in [%s]", identifier, resolvedIdentifierFieldName, + AT_IDENTIFIER_FIELD_NAME) + : String.format("field [%s] declared in [%s]", resolvedIdentifierFieldName, AT_IDENTIFIER_FIELD_NAME); + } + + return message; } /** @@ -332,6 +402,17 @@ public class JsonCacheDataImporterExporter extends AbstractCacheDataImporterExpo return String.format("%1$s%2$s", FILESYSTEM_RESOURCE_PREFIX, System.getProperty("user.dir")); } + /** + * Post processes the given {{@link PdxInstance}. + * + * @param pdxInstance {@link PdxInstance} to process. + * @return the {@link PdxInstance}. + * @see org.apache.geode.pdx.PdxInstance + */ + protected PdxInstance postProcess(PdxInstance pdxInstance) { + return pdxInstance; + } + // TODO Replace implementation with AbstractObjectArrayToJsonConverter.convert(:Iterable) method. /** * Convert {@link Object values} contained in the {@link Region} to JSON. @@ -346,17 +427,17 @@ public class JsonCacheDataImporterExporter extends AbstractCacheDataImporterExpo Assert.notNull(region, "Region must not be null"); - StringBuilder json = new StringBuilder("["); + StringBuilder json = new StringBuilder(ARRAY_BEGIN); boolean addComma = false; for (Object value : CollectionUtils.nullSafeCollection(CacheUtils.collectValues(region))) { - json.append(addComma ? ", " : ""); + json.append(addComma ? COMMA_SPACE : EMPTY_STRING); json.append(toJson(value)); addComma = true; } - json.append("]"); + json.append(ARRAY_END); return json.toString(); } diff --git a/spring-geode/src/test/java/org/springframework/geode/data/json/JsonCacheDataImporterExporterIntegrationTests.java b/spring-geode/src/test/java/org/springframework/geode/data/json/JsonCacheDataImporterExporterIntegrationTests.java index 026ff473..09778fa3 100644 --- a/spring-geode/src/test/java/org/springframework/geode/data/json/JsonCacheDataImporterExporterIntegrationTests.java +++ b/spring-geode/src/test/java/org/springframework/geode/data/json/JsonCacheDataImporterExporterIntegrationTests.java @@ -25,6 +25,7 @@ import java.io.IOException; import java.io.StringWriter; import java.io.Writer; import java.math.BigDecimal; +import java.time.LocalDate; import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -37,6 +38,9 @@ import java.util.stream.Collectors; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.DeserializationFeature; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.exc.InvalidDefinitionException; +import com.fasterxml.jackson.databind.exc.MismatchedInputException; +import com.fasterxml.jackson.databind.node.ObjectNode; import org.junit.After; import org.junit.Before; @@ -51,6 +55,7 @@ import org.apache.geode.pdx.JSONFormatter; import org.apache.geode.pdx.JSONFormatterException; import org.apache.geode.pdx.PdxInstance; import org.apache.geode.pdx.PdxInstanceFactory; +import org.apache.geode.pdx.PdxSerializationException; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.annotation.AnnotationConfigApplicationContext; @@ -64,6 +69,7 @@ import org.springframework.data.gemfire.tests.integration.IntegrationTestsSuppor import org.springframework.geode.core.util.ObjectUtils; import org.springframework.geode.pdx.PdxInstanceBuilder; import org.springframework.lang.NonNull; +import org.springframework.util.FileCopyUtils; import org.springframework.util.StringUtils; import example.app.crm.model.Customer; @@ -351,18 +357,19 @@ public class JsonCacheDataImporterExporterIntegrationTests extends IntegrationTe assertExampleRegion(example); assertThat(example).isEmpty(); - Customer jonDoe = Customer.newCustomer(42L, "Play Doe"); + Customer playDoe = Customer.newCustomer(42L, "Play Doe"); - example.put(jonDoe.getId(), jonDoe); + example.put(playDoe.getId(), playDoe); assertThat(example).hasSize(1); - assertThat(example.get(42L)).isEqualTo(jonDoe); + assertThat(example.get(42L)).isEqualTo(playDoe); closeApplicationContext(); String actualJson = writer.toString(); + String expectedJson = String.format("[{\"@type\":\"%s\",\"id\":42,\"name\":\"Play Doe\"}]", - jonDoe.getClass().getName()); + playDoe.getClass().getName()); assertThat(actualJson).isEqualTo(expectedJson); } @@ -446,7 +453,7 @@ public class JsonCacheDataImporterExporterIntegrationTests extends IntegrationTe } @Test - public void exportImportWithRegionContainingMixOfObjectsAndPdxInstances() throws IOException { + public void exportImportWithRegionContainingObjectsAndPdxInstances() throws IOException { try { // EXPORT @@ -508,27 +515,36 @@ public class JsonCacheDataImporterExporterIntegrationTests extends IntegrationTe } } - // APACHE GEODE BUG!!! + // APACHE GEODE BUG 1!!! @Test(expected = JSONFormatterException.class) - public void geodeJsonFormatterCannotHandleTypedJsonObjects() throws JsonProcessingException { + public void geodeJsonFormatterFromJsonCannotParseArrays() throws IOException { - Customer sourDoe = Customer.newCustomer(1L, "Sour Doe"); + try { - ObjectMapper objectMapper = newObjectMapper() - .activateDefaultTypingAsProperty(null, ObjectMapper.DefaultTyping.EVERYTHING, "$class"); + byte[] json = FileCopyUtils.copyToByteArray( + new ClassPathResource("data-example-doefamily.json").getInputStream()); - String json = objectMapper.writeValueAsString(sourDoe); + assertThat(json).isNotNull(); + assertThat(json).isNotEmpty(); - assertThat(json).isNotEmpty(); - assertThat(json).describedAs("Actual JSON [%s]", json) - .contains(String.format("\"$class\":\"%s\"", sourDoe.getClass().getName())); + JSONFormatter.fromJSON(json); + } + catch (JSONFormatterException expected) { - JSONFormatter.fromJSON(json); + // Caused because the JSONFormatter.fromJSON(..) method's JsonParser is not configured correctly! + + assertThat(expected).hasMessageStartingWith("Could not parse JSON document"); + assertThat(expected).hasCauseInstanceOf(IllegalStateException.class); + assertThat(expected.getCause()).hasMessageStartingWith("Array start called when state is NONE"); + assertThat(expected.getCause()).hasNoCause(); + + throw expected; + } } - // APACHE GEODE BUG!!! + // APACHE GEODE BUG 2!!! @Test - public void geodeJsonFormatterCannotRememberPdxInstanceClass() { + public void geodeJsonFormatterToJsonDoesNotGenerateAtTypeJsonObjectPropertyFromPdxInstanceGetClassName() { Cache peerCache = newApplicationContext(TestGeodeConfiguration.class).getBean(Cache.class); @@ -556,6 +572,94 @@ public class JsonCacheDataImporterExporterIntegrationTests extends IntegrationTe assertThat(value).isInstanceOf(PdxInstance.class); assertThat(value).isNotEqualTo(doeDoe); + + // Causes ClassCastException! + // Bug caused by the JSONFormatter.toJSON(:PdxInstance) method not properly setting the '@type' JSON object + // property from the PdxInstance.getClassName() when the class name is a valid Java class! + //Customer jonDoeAgain = (Customer) value; + } + + // APACHE GEODE BUG 3!!! + @Test(expected = PdxSerializationException.class) + public void geodePdxInstanceObjectMapperCannotDeserializeJava8Types() { + + try { + + Cache peerCache = newApplicationContext(TestGeodeConfiguration.class).getBean(Cache.class); + + ObjectMapper objectMapper = newObjectMapper(); + + TimedType value = TimedType.create().with(LocalDate.now()); + + ObjectNode objectNode = objectMapper.valueToTree(value); + + objectNode.put("@type", value.getClass().getName()); + + String json = objectNode.toString(); + + PdxInstance pdx = JSONFormatter.fromJSON(json); + + // BOOM! + pdx.getObject(); + } + catch (PdxSerializationException expected) { + + // Caused because the PdxInstance ObjectMapper is not properly configured (to findAndRegisterModules() + // or Jackson Module Extensions on the classpath)! + + assertThat(expected).hasMessageStartingWith("Could not deserialize as java class '%s'", + TimedType.class.getName()); + assertThat(expected.getCause()).isInstanceOf(InvalidDefinitionException.class); + assertThat(expected.getCause()).hasMessageStartingWith("Cannot construct instance of `java.time.LocalDate`"); + assertThat(expected.getCause()).hasNoCause(); + + throw expected; + } + } + + // APACHE GEODE BUG 4!!! + @Test(expected = PdxSerializationException.class) + public void geodePdxInstanceObjectMapperCannotDeserializeTypedJsonObjects() + throws JsonProcessingException { + + try { + + Cache peerCache = newApplicationContext(TestGeodeConfiguration.class).getBean(Cache.class); + + PurchaseOrder purchaseOrder = new PurchaseOrder() + .add(LineItem.newLineItem(Product.newProduct("Test Product") + .havingPrice(BigDecimal.valueOf(39.99)) + .in(Product.Category.UNSOUGHT)) + .withQuantity(2)) + .identifiedAs(1L); + + ObjectMapper objectMapper = newObjectMapper() + .activateDefaultTypingAsProperty(null, ObjectMapper.DefaultTyping.EVERYTHING, "@type"); + + String json = objectMapper.writeValueAsString(purchaseOrder); + + assertThat(json).isNotEmpty(); + assertThat(json).describedAs("Actual JSON [%s]", json) + .contains(String.format("\"@type\":\"%s\"", purchaseOrder.getClass().getName())); + + PdxInstance pdx = JSONFormatter.fromJSON(json); + + // BOOM! + pdx.getObject(); + } + catch (PdxSerializationException expected) { + + // Caused because the PdxInstance.getObject() method's ObjectMapper is not properly configured! + + assertThat(expected).hasMessageStartingWith("Could not deserialize as java class '%s'", + PurchaseOrder.class.getName()); + assertThat(expected.getCause()).isInstanceOf(MismatchedInputException.class); + assertThat(expected.getCause()) + .hasMessageStartingWith("Cannot deserialize instance of `java.lang.Long` out of START_ARRAY token"); + assertThat(expected.getCause()).hasNoCause(); + + throw expected; + } } @PeerCacheApplication @@ -589,4 +693,22 @@ public class JsonCacheDataImporterExporterIntegrationTests extends IntegrationTe }; } } + + public static class TimedType { + + public static TimedType create() { + return new TimedType(); + } + + private LocalDate time; + + public LocalDate getTime() { + return this.time; + } + + public TimedType with(LocalDate time) { + this.time = time; + return this; + } + } } diff --git a/spring-geode/src/test/java/org/springframework/geode/data/json/JsonCacheDataImporterExporterUnitTests.java b/spring-geode/src/test/java/org/springframework/geode/data/json/JsonCacheDataImporterExporterUnitTests.java index da718a08..b8493442 100644 --- a/spring-geode/src/test/java/org/springframework/geode/data/json/JsonCacheDataImporterExporterUnitTests.java +++ b/spring-geode/src/test/java/org/springframework/geode/data/json/JsonCacheDataImporterExporterUnitTests.java @@ -70,8 +70,12 @@ import example.app.pos.model.LineItem; * @see org.apache.geode.cache.Region * @see org.apache.geode.pdx.PdxInstance * @see org.springframework.context.ApplicationContext + * @see org.springframework.core.io.ClassPathResource * @see org.springframework.core.io.Resource * @see org.springframework.geode.data.json.JsonCacheDataImporterExporter + * @see org.springframework.geode.data.json.converter.JsonToPdxArrayConverter + * @see org.springframework.geode.data.json.converter.JsonToPdxConverter + * @see org.springframework.geode.data.json.converter.ObjectToJsonConverter * @since 1.3.0 */ @RunWith(MockitoJUnitRunner.class) @@ -236,7 +240,9 @@ public class JsonCacheDataImporterExporterUnitTests { .getResource(eq(mockRegion), eq(JsonCacheDataImporterExporter.CLASSPATH_RESOURCE_PREFIX)); verify(this.importer, times(1)).getContent(eq(mockResource)); verify(this.importer, times(1)).toPdxArray(eq(json)); + verify(this.importer, times(1)).postProcess(eq(mockPdxInstanceOne)); verify(this.importer, times(1)).getIdentifier(eq(mockPdxInstanceOne)); + verify(this.importer, times(1)).postProcess(eq(mockPdxInstanceTwo)); verify(this.importer, times(1)).getIdentifier(eq(mockPdxInstanceTwo)); verify(mockResource, times(1)).exists(); verify(mockRegion, times(1)).put(eq(1), eq(mockPdxInstanceOne)); @@ -408,7 +414,7 @@ public class JsonCacheDataImporterExporterUnitTests { } @Test - public void getIdentifierFromPdxInstance() { + public void getIdentifierFromPdxInstanceHavingIdentity() { PdxInstance mockPdxInstance = mock(PdxInstance.class); @@ -418,7 +424,7 @@ public class JsonCacheDataImporterExporterUnitTests { assertThat(this.importer.getIdentifier(mockPdxInstance)).isEqualTo(42); - verify(this.importer, never()).getIdField(any(PdxInstance.class)); + verify(this.importer, never()).getId(any(PdxInstance.class)); verify(mockPdxInstance, times(1)).getFieldNames(); verify(mockPdxInstance, times(1)).isIdentityField(eq("age")); verify(mockPdxInstance, times(1)).isIdentityField(eq("id")); @@ -432,11 +438,11 @@ public class JsonCacheDataImporterExporterUnitTests { PdxInstance mockPdxInstance = mock(PdxInstance.class); doReturn(null).when(mockPdxInstance).getFieldNames(); - doReturn(69).when(this.importer).getIdField(eq(mockPdxInstance)); + doReturn(69).when(this.importer).getId(eq(mockPdxInstance)); assertThat(this.importer.getIdentifier(mockPdxInstance)).isEqualTo(69); - verify(this.importer, times(1)).getIdField(eq(mockPdxInstance)); + verify(this.importer, times(1)).getId(eq(mockPdxInstance)); verify(mockPdxInstance, times(1)).getFieldNames(); verify(mockPdxInstance, never()).isIdentityField(anyString()); verify(mockPdxInstance, never()).getField(anyString()); @@ -448,12 +454,12 @@ public class JsonCacheDataImporterExporterUnitTests { PdxInstance mockPdxInstance = mock(PdxInstance.class); doReturn(Arrays.asList("", "age", null, "name", " ")).when(mockPdxInstance).getFieldNames(); - doReturn(false).when(mockPdxInstance).isIdentityField(anyString()); - doReturn(99).when(this.importer).getIdField(eq(mockPdxInstance)); + doReturn(false).when(mockPdxInstance).isIdentityField(any()); + doReturn(99).when(this.importer).getId(eq(mockPdxInstance)); assertThat(this.importer.getIdentifier(mockPdxInstance)).isEqualTo(99); - verify(this.importer, times(1)).getIdField(eq(mockPdxInstance)); + verify(this.importer, times(1)).getId(eq(mockPdxInstance)); verify(mockPdxInstance, times(1)).getFieldNames(); verify(mockPdxInstance, times(1)).isIdentityField(eq("age")); verify(mockPdxInstance, times(1)).isIdentityField(eq("name")); @@ -470,14 +476,14 @@ public class JsonCacheDataImporterExporterUnitTests { doReturn(Collections.singletonList("name")).when(mockPdxInstance).getFieldNames(); doReturn(false).when(mockPdxInstance).isIdentityField(anyString()); - doThrow(new IllegalStateException("TEST")).when(this.importer).getIdField(eq(mockPdxInstance)); + doThrow(new IllegalStateException("NO ID")).when(this.importer).getId(eq(mockPdxInstance)); try { this.importer.getIdentifier(mockPdxInstance); } catch (IllegalStateException expected) { - assertThat(expected).hasMessage("TEST"); + assertThat(expected).hasMessage("NO ID"); assertThat(expected).hasNoCause(); throw expected; @@ -505,35 +511,122 @@ public class JsonCacheDataImporterExporterUnitTests { } @Test - public void getIdFieldFromPdxInstance() { + public void getIdFromPdxInstanceHavingIdField() { PdxInstance mockPdxInstance = mock(PdxInstance.class); doReturn(true).when(mockPdxInstance).hasField(eq(JsonCacheDataImporterExporter.ID_FIELD_NAME)); doReturn(42).when(mockPdxInstance).getField(eq(JsonCacheDataImporterExporter.ID_FIELD_NAME)); - assertThat(this.importer.getIdField(mockPdxInstance)).isEqualTo(42); + assertThat(this.importer.getId(mockPdxInstance)).isEqualTo(42); verify(mockPdxInstance, times(1)) .hasField(eq(JsonCacheDataImporterExporter.ID_FIELD_NAME)); verify(mockPdxInstance, times(1)) .getField(eq(JsonCacheDataImporterExporter.ID_FIELD_NAME)); + verify(this.importer, never()).getAtIdentifier(any()); + } + + @Test + public void getIdFromPdxInstanceHavingIdFieldWithNoValueCallsGetAtIdentifier() { + + PdxInstance mockPdxInstance = mock(PdxInstance.class); + + doReturn(true).when(mockPdxInstance).hasField(eq(JsonCacheDataImporterExporter.ID_FIELD_NAME)); + doReturn(null).when(mockPdxInstance).getField(eq(JsonCacheDataImporterExporter.ID_FIELD_NAME)); + doReturn(69).when(this.importer).getAtIdentifier(eq(mockPdxInstance)); + + assertThat(this.importer.getId(mockPdxInstance)).isEqualTo(69); + + verify(mockPdxInstance, times(1)) + .hasField(eq(JsonCacheDataImporterExporter.ID_FIELD_NAME)); + verify(mockPdxInstance, times(1)) + .getField(eq(JsonCacheDataImporterExporter.ID_FIELD_NAME)); + verify(this.importer, times(1)).getAtIdentifier(any()); + } + + @Test + public void getIdFromPdxInstanceWithNoIdFieldCallsGetAtIdentifier() { + + PdxInstance mockPdxInstance = mock(PdxInstance.class); + + doReturn(false).when(mockPdxInstance).hasField(any()); + doReturn(99).when(this.importer).getAtIdentifier(eq(mockPdxInstance)); + + assertThat(this.importer.getId(mockPdxInstance)).isEqualTo(99); + + verify(mockPdxInstance, times(1)) + .hasField(eq(JsonCacheDataImporterExporter.ID_FIELD_NAME)); + verify(mockPdxInstance, never()).getField(eq(JsonCacheDataImporterExporter.ID_FIELD_NAME)); + verify(this.importer, times(1)).getAtIdentifier(eq(mockPdxInstance)); + } + + @Test(expected = IllegalArgumentException.class) + public void getIdFromNullPdxInstance() { + + try { + this.importer.getId(null); + } + catch (IllegalArgumentException expected) { + + assertThat(expected).hasMessage("PdxInstance must not be null"); + assertThat(expected).hasNoCause(); + + throw expected; + } + } + + @Test + public void getAtIdentifierFromPdxInstance() { + + PdxInstance mockPdxInstance = mock(PdxInstance.class); + + doReturn(true).when(mockPdxInstance) + .hasField(eq(JsonCacheDataImporterExporter.AT_IDENTIFIER_FIELD_NAME)); + doReturn(true).when(mockPdxInstance).hasField(eq("isbn")); + doReturn("isbn").when(mockPdxInstance) + .getField(eq(JsonCacheDataImporterExporter.AT_IDENTIFIER_FIELD_NAME)); + doReturn("123456789").when(mockPdxInstance).getField(eq("isbn")); + + assertThat(this.importer.getAtIdentifier(mockPdxInstance)).isEqualTo("123456789"); + + verify(mockPdxInstance, times(1)) + .hasField(eq(JsonCacheDataImporterExporter.AT_IDENTIFIER_FIELD_NAME)); + verify(mockPdxInstance, times(1)) + .getField(eq(JsonCacheDataImporterExporter.AT_IDENTIFIER_FIELD_NAME)); + verify(mockPdxInstance, times(1)).hasField(eq("isbn")); + verify(mockPdxInstance, times(1)).getField(eq("isbn")); + } + + @Test(expected = IllegalArgumentException.class) + public void getAtIdentifierWithNullPdxInstance() { + + try { + this.importer.getAtIdentifier(null); + } + catch (IllegalArgumentException expected) { + + assertThat(expected).hasMessage("PdxInstance must not be null"); + assertThat(expected).hasNoCause(); + + throw expected; + } } @Test(expected = IllegalStateException.class) - public void getIdFieldFromPdxInstanceWithNoIdField() { + public void getAtIdentifierFromPdxInstanceWithNoDeclaredIdentity() { PdxInstance mockPdxInstance = mock(PdxInstance.class); doReturn(LineItem.class.getName()).when(mockPdxInstance).getClassName(); - doReturn(false).when(mockPdxInstance).hasField(anyString()); + doReturn(false).when(mockPdxInstance).hasField(any()); try { - this.importer.getIdField(mockPdxInstance); + this.importer.getAtIdentifier(mockPdxInstance); } catch (IllegalStateException expected) { - assertThat(expected).hasMessage("PdxInstance for type [%s] has no declared identity field", + assertThat(expected).hasMessage("PdxInstance for type [%s] has no declared identifier", LineItem.class.getName()); assertThat(expected).hasNoCause(); @@ -542,22 +635,23 @@ public class JsonCacheDataImporterExporterUnitTests { finally { verify(mockPdxInstance, times(1)).getClassName(); verify(mockPdxInstance, times(2)) + .hasField(eq(JsonCacheDataImporterExporter.AT_IDENTIFIER_FIELD_NAME)); + verify(mockPdxInstance, times(1)) .hasField(eq(JsonCacheDataImporterExporter.ID_FIELD_NAME)); verify(mockPdxInstance, never()).getField(anyString()); } } @Test(expected = IllegalStateException.class) - public void getIdFieldFromPdxInstanceWithNoId() { + public void getAtIdentifierFromPdxInstanceWithNoId() { PdxInstance mockPdxInstance = mock(PdxInstance.class); - doReturn(true).when(mockPdxInstance).hasField(anyString()); - doReturn(null).when(mockPdxInstance).getField(eq(JsonCacheDataImporterExporter.ID_FIELD_NAME)); doReturn(LineItem.class.getName()).when(mockPdxInstance).getClassName(); + doReturn(true).when(mockPdxInstance).hasField(eq(JsonCacheDataImporterExporter.ID_FIELD_NAME)); try { - this.importer.getIdField(mockPdxInstance); + this.importer.getAtIdentifier(mockPdxInstance); } catch (IllegalStateException expected) { @@ -570,25 +664,92 @@ public class JsonCacheDataImporterExporterUnitTests { finally { verify(mockPdxInstance, times(1)).getClassName(); verify(mockPdxInstance, times(1)) - .getField(eq(JsonCacheDataImporterExporter.ID_FIELD_NAME)); - verify(mockPdxInstance, times(2)) + .hasField(eq(JsonCacheDataImporterExporter.AT_IDENTIFIER_FIELD_NAME)); + verify(mockPdxInstance, times(1)) .hasField(eq(JsonCacheDataImporterExporter.ID_FIELD_NAME)); + verify(mockPdxInstance, never()).getField(any()); } } - @Test(expected = IllegalArgumentException.class) - public void getIdFieldFromNullPdxInstance() { + @Test(expected = IllegalStateException.class) + public void getAtIdentifierFromPdxInstanceWithValidAtIdentifierAndIdentifierFieldButNoId() { + + PdxInstance mockPdxInstance = mock(PdxInstance.class); + + doReturn(Customer.class.getName()).when(mockPdxInstance).getClassName(); + doReturn(true).when(mockPdxInstance) + .hasField(eq(JsonCacheDataImporterExporter.AT_IDENTIFIER_FIELD_NAME)); + doReturn(false).when(mockPdxInstance) + .hasField(eq(JsonCacheDataImporterExporter.ID_FIELD_NAME)); + doReturn(true).when(mockPdxInstance).hasField(eq("ssn")); + doReturn("ssn").when(mockPdxInstance) + .getField(eq(JsonCacheDataImporterExporter.AT_IDENTIFIER_FIELD_NAME)); + doReturn(null).when(mockPdxInstance).getField(eq("ssn")); try { - this.importer.getIdField(null); + this.importer.getAtIdentifier(mockPdxInstance); } - catch (IllegalArgumentException expected) { + catch (IllegalStateException expected) { - assertThat(expected).hasMessage("PdxInstance must not be null"); + assertThat(expected) + .hasMessage("PdxInstance for type [%s] has no value [null] for field [ssn] declared in [%s]", + Customer.class.getName(), JsonCacheDataImporterExporter.AT_IDENTIFIER_FIELD_NAME); assertThat(expected).hasNoCause(); throw expected; } + finally { + verify(mockPdxInstance, times(1)).getClassName(); + verify(mockPdxInstance, times(2)) + .hasField(eq(JsonCacheDataImporterExporter.AT_IDENTIFIER_FIELD_NAME)); + verify(mockPdxInstance, times(1)) + .hasField(eq(JsonCacheDataImporterExporter.ID_FIELD_NAME)); + verify(mockPdxInstance, times(2)).hasField(eq("ssn")); + verify(mockPdxInstance, times(2)) + .getField(eq(JsonCacheDataImporterExporter.AT_IDENTIFIER_FIELD_NAME)); + verify(mockPdxInstance, times(2)).getField(eq("ssn")); + verifyNoMoreInteractions(mockPdxInstance); + } + } + + @Test(expected = IllegalStateException.class) + public void getAtIdentifierFromPdxInstanceWithAtIdentifierReferringToInvalidIdentifierField() { + + PdxInstance mockPdxInstance = mock(PdxInstance.class); + + doReturn(Customer.class.getName()).when(mockPdxInstance).getClassName(); + doReturn(true).when(mockPdxInstance) + .hasField(eq(JsonCacheDataImporterExporter.AT_IDENTIFIER_FIELD_NAME)); + doReturn(false).when(mockPdxInstance) + .hasField(eq(JsonCacheDataImporterExporter.ID_FIELD_NAME)); + doReturn(false).when(mockPdxInstance).hasField(eq("ssn")); + doReturn("ssn").when(mockPdxInstance) + .getField(eq(JsonCacheDataImporterExporter.AT_IDENTIFIER_FIELD_NAME)); + + try { + this.importer.getAtIdentifier(mockPdxInstance); + } + catch (IllegalStateException expected) { + + assertThat(expected) + .hasMessage("PdxInstance for type [%s] has no field [ssn] declared in [%s]", + Customer.class.getName(), JsonCacheDataImporterExporter.AT_IDENTIFIER_FIELD_NAME); + assertThat(expected).hasNoCause(); + + throw expected; + } + finally { + verify(mockPdxInstance, times(1)).getClassName(); + verify(mockPdxInstance, times(2)) + .hasField(eq(JsonCacheDataImporterExporter.AT_IDENTIFIER_FIELD_NAME)); + verify(mockPdxInstance, times(1)) + .hasField(eq(JsonCacheDataImporterExporter.ID_FIELD_NAME)); + verify(mockPdxInstance, times(2)).hasField(eq("ssn")); + verify(mockPdxInstance, times(2)) + .getField(eq(JsonCacheDataImporterExporter.AT_IDENTIFIER_FIELD_NAME)); + verify(mockPdxInstance, never()).getField(eq("ssn")); + verifyNoMoreInteractions(mockPdxInstance); + } } @Test