From 2b9a654fcd74122a0b1833814f0ce0e833f118ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marian=20Kl=C3=BChspies?= Date: Sun, 17 Dec 2023 18:16:45 +0100 Subject: [PATCH 1/4] feat: AlgoliaUtils now supports Getter annotations and is based on Jackson instead of custom reflection logic. --- .../java/com/algolia/search/SearchIndex.java | 2 +- .../search/models/indexing/SearchResult.java | 4 +- .../com/algolia/search/util/AlgoliaUtils.java | 200 ++++++------------ .../algolia/search/EnsureObjectIDTest.java | 22 +- .../search/integration/index/SearchTest.java | 6 +- .../algolia/search/util/AlgoliaUtilsTest.java | 93 ++++++++ 6 files changed, 176 insertions(+), 151 deletions(-) create mode 100644 algoliasearch-core/src/test/java/com/algolia/search/util/AlgoliaUtilsTest.java diff --git a/algoliasearch-core/src/main/java/com/algolia/search/SearchIndex.java b/algoliasearch-core/src/main/java/com/algolia/search/SearchIndex.java index 28dec9475..67d205d20 100644 --- a/algoliasearch-core/src/main/java/com/algolia/search/SearchIndex.java +++ b/algoliasearch-core/src/main/java/com/algolia/search/SearchIndex.java @@ -480,7 +480,7 @@ public CompletableFuture partialUpdateObjectAsync( Objects.requireNonNull(data, "Data is required."); Objects.requireNonNull(createIfNotExists, "createIfNotExists is required."); - String objectID = AlgoliaUtils.getObjectID(data, clazz); + String objectID = AlgoliaUtils.getObjectID(data); if (requestOptions == null) { requestOptions = new RequestOptions(); diff --git a/algoliasearch-core/src/main/java/com/algolia/search/models/indexing/SearchResult.java b/algoliasearch-core/src/main/java/com/algolia/search/models/indexing/SearchResult.java index f1db283c7..185de578b 100644 --- a/algoliasearch-core/src/main/java/com/algolia/search/models/indexing/SearchResult.java +++ b/algoliasearch-core/src/main/java/com/algolia/search/models/indexing/SearchResult.java @@ -330,9 +330,9 @@ public SearchResult setFacets_stats(Map facets_stats) { return this; } - public int getObjectPosition(@Nonnull String objectID, @Nonnull Class clazz) { + public int getObjectPosition(@Nonnull String objectID) { return IntStream.range(0, hits.size()) - .filter(i -> objectID.equals(AlgoliaUtils.getObjectID(hits.get(i), clazz))) + .filter(i -> objectID.equals(AlgoliaUtils.getObjectID(hits.get(i)))) .findFirst() .orElse(-1); } diff --git a/algoliasearch-core/src/main/java/com/algolia/search/util/AlgoliaUtils.java b/algoliasearch-core/src/main/java/com/algolia/search/util/AlgoliaUtils.java index f7b1702dc..7f3f90e14 100644 --- a/algoliasearch-core/src/main/java/com/algolia/search/util/AlgoliaUtils.java +++ b/algoliasearch-core/src/main/java/com/algolia/search/util/AlgoliaUtils.java @@ -1,162 +1,94 @@ package com.algolia.search.util; import com.algolia.search.exceptions.AlgoliaRuntimeException; -import com.fasterxml.jackson.annotation.JsonProperty; -import java.lang.reflect.Field; +import com.fasterxml.jackson.databind.BeanDescription; +import com.fasterxml.jackson.databind.JavaType; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; + import java.time.Clock; import java.time.Instant; import java.time.OffsetDateTime; import java.time.ZoneOffset; import java.time.zone.ZoneRules; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.Optional; +import java.util.*; import javax.annotation.Nonnull; public class AlgoliaUtils { - /** Checks if the given string is empty or white spaces */ - public static Boolean isEmptyWhiteSpace(final String stringToCheck) { - return stringToCheck.trim().length() == 0; - } - - /** Checks if the given string is null, empty or white spaces */ - public static Boolean isNullOrEmptyWhiteSpace(final String stringToCheck) { - return stringToCheck == null || stringToCheck.trim().length() == 0; - } - - private static final ZoneRules ZONE_RULES_UTC = ZoneOffset.UTC.getRules(); - - /** - * Memory optimization for getZoneRules with the same ZoneOffset (UTC). ZoneRules is immutable and - * threadsafe, but getRules method consumes a lot of memory during load testing. - */ - public static OffsetDateTime nowUTC() { - final Instant now = Clock.system(ZoneOffset.UTC).instant(); - return OffsetDateTime.ofInstant(now, ZONE_RULES_UTC.getOffset(now)); - } + public static final String PROPERTY_OBJECT_ID = "objectID"; - /** - * Ensure that the objectID field or the @JsonProperty(\"objectID\")" is present in the given - * class - * - * @param clazz The class to scan - * @throws AlgoliaRuntimeException When the class doesn't have an objectID field or a Jackson - * annotation @JsonProperty(\"objectID\"") - */ - public static void ensureObjectID(@Nonnull Class clazz) { - // Try to find the objectID field - Field objectIDField = getField(clazz, "objectID"); - - // If objectID field doesn't exist, let's check for Jackson annotations in all the fields - Optional optObjectIDField = findObjectIDInAnnotation(clazz); - - if (objectIDField == null && !optObjectIDField.isPresent()) { - throw new AlgoliaRuntimeException( - "The " - + clazz - + " must have an objectID property or a Jackson annotation @JsonProperty(\"objectID\")"); + /** + * Checks if the given string is empty or white spaces + */ + public static Boolean isEmptyWhiteSpace(final String stringToCheck) { + return stringToCheck.trim().isEmpty(); } - } - /** - * Get the objectID of the given class at runtime - * - * @param clazz The class to scan - * @throws AlgoliaRuntimeException When the class doesn't have an objectID field or a Jackson - * annotation @JsonProperty(\"objectID\"") - */ - public static String getObjectID(@Nonnull T data, @Nonnull Class clazz) { + /** + * Checks if the given string is null, empty or white spaces + */ + public static Boolean isNullOrEmptyWhiteSpace(final String stringToCheck) { + return stringToCheck == null || stringToCheck.trim().isEmpty(); + } - String objectID = null; + private static final ZoneRules ZONE_RULES_UTC = ZoneOffset.UTC.getRules(); - // Try to find the objectID field - try { - Field objectIDField = getField(clazz, "objectID"); - if (objectIDField != null) { - objectID = (String) objectIDField.get(data); - } - } catch ( - IllegalAccessException - ignored) { // Ignored because if it fails we want to move forward on annotations + /** + * Memory optimization for getZoneRules with the same ZoneOffset (UTC). ZoneRules is immutable and + * thread-safe, but getRules method consumes a lot of memory during load testing. + */ + public static OffsetDateTime nowUTC() { + final Instant now = Clock.system(ZoneOffset.UTC).instant(); + return OffsetDateTime.ofInstant(now, ZONE_RULES_UTC.getOffset(now)); } - if (objectID != null) { - return objectID; + /** + * Ensure that the objectID field or the @JsonProperty(\"objectID\")" is present in the given + * class + * + * @param clazz The class to scan + * @throws AlgoliaRuntimeException When the class doesn't have an objectID field or a Jackson + * annotation @JsonProperty(\"objectID\"") + */ + public static void ensureObjectID(@Nonnull Class clazz) { + BeanDescription introspection = introspectClass(clazz); + if (!containsObjectID(introspection)) throw objectIDNotFoundException(clazz); } - // If objectID field doesn't exist, let's check for Jackson annotations in all the fields - Optional optObjectIDField = findObjectIDInAnnotation(clazz); - - if (optObjectIDField.isPresent()) { - Field objectIDField = optObjectIDField.get(); - try { - objectIDField.setAccessible(true); - - objectID = (String) objectIDField.get(data); - - if (objectID != null) { - return objectID; - } - - } catch (IllegalAccessException ignored) { - throw new AlgoliaRuntimeException("Can't access the ObjectID field."); - } + private static AlgoliaRuntimeException objectIDNotFoundException(Class clazz) { + return new AlgoliaRuntimeException( + "The " + clazz + " must have an objectID property or a Jackson annotation @JsonProperty(\"objectID\")"); } - // If non of the both above the method fails - throw new AlgoliaRuntimeException( - "The " - + clazz - + " must have an objectID property or a Jackson annotation @JsonProperty(\"objectID\")"); - } - - private static Optional findObjectIDInAnnotation(@Nonnull Class clazz) { - List fields = getFields(clazz); - return fields.stream() - .filter( - f -> - f.getAnnotation(JsonProperty.class) != null - && f.getAnnotation(JsonProperty.class).value().equals("objectID")) - .findFirst(); - } - - /** - * Recursively search for the given field in the given class - * - * @param clazz The class to reflect on - * @param fieldName The field to reach - */ - private static Field getField(@Nonnull Class clazz, @Nonnull String fieldName) { - Class tmpClass = clazz; - do { - try { - Field f = tmpClass.getDeclaredField(fieldName); - f.setAccessible(true); - return f; - } catch (NoSuchFieldException e) { - tmpClass = tmpClass.getSuperclass(); - } - } while (tmpClass != null); - - return null; - } + /** + * Checks if the {@value PROPERTY_OBJECT_ID} is present in the classes public fields, getter methods or + * annotations using Jackson's {@link BeanDescription} + */ + protected static boolean containsObjectID(BeanDescription introspection) { + return introspection.findProperties().stream().anyMatch(d -> PROPERTY_OBJECT_ID.equals(d.getName())); + } - /** - * Recursively search for all fields in the given class - * - * @param clazz The class to reflect on - */ - private static List getFields(@Nonnull Class clazz) { - List result = new ArrayList<>(); - Class i = clazz; + /** + * Introspection of the class using Jackson + */ + protected static BeanDescription introspectClass(Class clazz) { + ObjectMapper mapper = new ObjectMapper(); + JavaType type = mapper.getTypeFactory().constructType(clazz); + return mapper.getSerializationConfig().introspect(type); + } - while (i != null && i != Object.class) { - Collections.addAll(result, i.getDeclaredFields()); - i = i.getSuperclass(); + /** + * Get the objectID of the given class at runtime + * + * @throws AlgoliaRuntimeException When the class doesn't have an objectID field or a Jackson + * annotation @JsonProperty(\"objectID\"") + */ + public static String getObjectID(@Nonnull T data) { + return Optional.ofNullable(new ObjectMapper().valueToTree(data) + .get(PROPERTY_OBJECT_ID)) + .map(JsonNode::asText) + .orElseThrow(() -> objectIDNotFoundException(data.getClass())); } - return result; - } } diff --git a/algoliasearch-core/src/test/java/com/algolia/search/EnsureObjectIDTest.java b/algoliasearch-core/src/test/java/com/algolia/search/EnsureObjectIDTest.java index e781af8b1..2b86328cd 100644 --- a/algoliasearch-core/src/test/java/com/algolia/search/EnsureObjectIDTest.java +++ b/algoliasearch-core/src/test/java/com/algolia/search/EnsureObjectIDTest.java @@ -65,7 +65,7 @@ void testGetObjectIDWithoutObjectID() { assertThatThrownBy( () -> AlgoliaUtils.getObjectID( - new DummyObjectWithoutObjectId(), DummyObjectWithoutObjectId.class)) + new DummyObjectWithoutObjectId())) .isInstanceOf(AlgoliaRuntimeException.class) .hasMessageContaining( "must have an objectID property or a Jackson annotation @JsonProperty(\"objectID\")"); @@ -73,7 +73,7 @@ void testGetObjectIDWithoutObjectID() { assertThatThrownBy( () -> AlgoliaUtils.getObjectID( - new DummyChildWithoutObjectID(), DummyChildWithoutObjectID.class)) + new DummyChildWithoutObjectID())) .isInstanceOf(AlgoliaRuntimeException.class) .hasMessageContaining( "must have an objectID property or a Jackson annotation @JsonProperty(\"objectID\")"); @@ -85,7 +85,7 @@ void testGetObjectIDWithWrongAnnotation() { assertThatThrownBy( () -> AlgoliaUtils.getObjectID( - new DummyObjectWithWrongAnnotation(), DummyObjectWithWrongAnnotation.class)) + new DummyObjectWithWrongAnnotation())) .isInstanceOf(AlgoliaRuntimeException.class) .hasMessageContaining( "must have an objectID property or a Jackson annotation @JsonProperty(\"objectID\")"); @@ -93,7 +93,7 @@ void testGetObjectIDWithWrongAnnotation() { assertThatThrownBy( () -> AlgoliaUtils.getObjectID( - new DummyChildWithWrongAnnotation(), DummyChildWithWrongAnnotation.class)) + new DummyChildWithWrongAnnotation())) .isInstanceOf(AlgoliaRuntimeException.class) .hasMessageContaining( "must have an objectID property or a Jackson annotation @JsonProperty(\"objectID\")"); @@ -105,15 +105,15 @@ void testGetObjectIDWithObjectID() { assertThatCode( () -> AlgoliaUtils.getObjectID( - new DummyObjectWithObjectID().setObjectID("foo"), - DummyObjectWithObjectID.class)) + new DummyObjectWithObjectID().setObjectID("foo") + )) .doesNotThrowAnyException(); assertThatCode( () -> AlgoliaUtils.getObjectID( - (DummyChildWithObjectID) new DummyChildWithObjectID().setObjectID("foo"), - DummyChildWithObjectID.class)) + (DummyChildWithObjectID) new DummyChildWithObjectID().setObjectID("foo") + )) .doesNotThrowAnyException(); } @@ -123,14 +123,14 @@ void testGetObjectIDWithAnnotation() { assertThatCode( () -> AlgoliaUtils.getObjectID( - new DummyObjectWithAnnotation().setId("foo"), DummyObjectWithAnnotation.class)) + new DummyObjectWithAnnotation().setId("foo"))) .doesNotThrowAnyException(); assertThatCode( () -> AlgoliaUtils.getObjectID( - (DummyChildWithAnnotation) new DummyChildWithAnnotation().setId("foo"), - DummyChildWithAnnotation.class)) + (DummyChildWithAnnotation) new DummyChildWithAnnotation().setId("foo") + )) .doesNotThrowAnyException(); } } diff --git a/algoliasearch-core/src/test/java/com/algolia/search/integration/index/SearchTest.java b/algoliasearch-core/src/test/java/com/algolia/search/integration/index/SearchTest.java index d60e7e697..9154bde0a 100644 --- a/algoliasearch-core/src/test/java/com/algolia/search/integration/index/SearchTest.java +++ b/algoliasearch-core/src/test/java/com/algolia/search/integration/index/SearchTest.java @@ -88,11 +88,11 @@ void testSearch() throws ExecutionException, InterruptedException { searchFacetFuture); assertThat(searchAlgoliaFuture.get().getHits()).hasSize(2); - assertThat(searchAlgoliaFuture.get().getObjectPosition("nicolas-dessaigne", Employee.class)) + assertThat(searchAlgoliaFuture.get().getObjectPosition("nicolas-dessaigne")) .isEqualTo(0); - assertThat(searchAlgoliaFuture.get().getObjectPosition("julien-lemoine", Employee.class)) + assertThat(searchAlgoliaFuture.get().getObjectPosition("julien-lemoine")) .isEqualTo(1); - assertThat(searchAlgoliaFuture.get().getObjectPosition("unknown", Employee.class)) + assertThat(searchAlgoliaFuture.get().getObjectPosition("unknown")) .isEqualTo(-1); assertTrue(searchAlgoliaFuture.get().getExhaustiveNbHits()); assertThat(searchElonFuture.get().getQueryID()).isNotNull(); diff --git a/algoliasearch-core/src/test/java/com/algolia/search/util/AlgoliaUtilsTest.java b/algoliasearch-core/src/test/java/com/algolia/search/util/AlgoliaUtilsTest.java new file mode 100644 index 000000000..60e4997e7 --- /dev/null +++ b/algoliasearch-core/src/test/java/com/algolia/search/util/AlgoliaUtilsTest.java @@ -0,0 +1,93 @@ +package com.algolia.search.util; + +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.databind.BeanDescription; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * Created by Marian at 17.12.2023 + */ +class AlgoliaUtilsTest { + + protected interface SetObjectId { + void set(String objectId); + } + + protected static class SomeClassWithPublicField implements SetObjectId { + + public String objectID; + + @Override + public void set(String objectId) { + this.objectID = objectId; + } + } + + protected static class SomeClassWithGetter implements SetObjectId { + + private String objectID; + + public String getObjectID() { + return objectID; + } + + @Override + public void set(String objectId) { + this.objectID = objectId; + } + } + + + protected static class SomeClassWithAnnotation implements SetObjectId { + + @JsonProperty(AlgoliaUtils.PROPERTY_OBJECT_ID) + private String objectID; + + @Override + public void set(String objectId) { + this.objectID = objectId; + } + + } + + protected static class SomeClassWithGetterAndAnnotation implements SetObjectId { + + private String objectID; + + @JsonProperty(AlgoliaUtils.PROPERTY_OBJECT_ID) + public String getWithCustomName() { + return objectID; + } + + @Override + public void set(String objectId) { + this.objectID = objectId; + } + } + + @ParameterizedTest + @ValueSource(classes = { + SomeClassWithPublicField.class, + SomeClassWithGetter.class, + SomeClassWithAnnotation.class, + SomeClassWithGetterAndAnnotation.class}) + void containsObjectID(Class clazz) { + BeanDescription introspection = AlgoliaUtils.introspectClass(clazz); + assertTrue(AlgoliaUtils.containsObjectID(introspection)); + } + + @ParameterizedTest + @ValueSource(classes = { + SomeClassWithPublicField.class, + SomeClassWithGetter.class, + SomeClassWithAnnotation.class, + SomeClassWithGetterAndAnnotation.class}) + void extractObjectID(Class clazz) throws Exception { + SetObjectId instance = clazz.getDeclaredConstructor().newInstance(); + instance.set("12345"); + assertEquals("12345", AlgoliaUtils.getObjectID(instance)); + } +} From 5a3a9cf60887338193e5a9e64ca5b97dd4f4ce16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marian=20Kl=C3=BChspies?= Date: Sun, 17 Dec 2023 20:42:17 +0100 Subject: [PATCH 2/4] test: AlgoliaUtilsTest enhanced with two error cases --- .../algolia/search/util/AlgoliaUtilsTest.java | 57 ++++++++++++++++--- 1 file changed, 49 insertions(+), 8 deletions(-) diff --git a/algoliasearch-core/src/test/java/com/algolia/search/util/AlgoliaUtilsTest.java b/algoliasearch-core/src/test/java/com/algolia/search/util/AlgoliaUtilsTest.java index 60e4997e7..d23a662c9 100644 --- a/algoliasearch-core/src/test/java/com/algolia/search/util/AlgoliaUtilsTest.java +++ b/algoliasearch-core/src/test/java/com/algolia/search/util/AlgoliaUtilsTest.java @@ -1,22 +1,38 @@ package com.algolia.search.util; +import com.algolia.search.exceptions.AlgoliaRuntimeException; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.databind.BeanDescription; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; +import java.math.BigDecimal; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.util.Calendar; + import static org.junit.jupiter.api.Assertions.*; -/** - * Created by Marian at 17.12.2023 - */ class AlgoliaUtilsTest { protected interface SetObjectId { void set(String objectId); } - protected static class SomeClassWithPublicField implements SetObjectId { + /** + * Additional fields to ensure Jackson calls won't fail because of missing type converters + */ + protected abstract static class BaseClass implements SetObjectId { + + public LocalDate localDate = LocalDate.now(); + public Calendar calendar = Calendar.getInstance(); + public BigDecimal bigDecimal = new BigDecimal(10); + public LocalDateTime localDateTime = LocalDateTime.now(); + } + + protected static class SomeClassWithPublicField extends BaseClass { public String objectID; @@ -26,7 +42,7 @@ public void set(String objectId) { } } - protected static class SomeClassWithGetter implements SetObjectId { + protected static class SomeClassWithGetter extends BaseClass { private String objectID; @@ -41,7 +57,7 @@ public void set(String objectId) { } - protected static class SomeClassWithAnnotation implements SetObjectId { + protected static class SomeClassWithAnnotation extends BaseClass { @JsonProperty(AlgoliaUtils.PROPERTY_OBJECT_ID) private String objectID; @@ -53,7 +69,7 @@ public void set(String objectId) { } - protected static class SomeClassWithGetterAndAnnotation implements SetObjectId { + protected static class SomeClassWithGetterAndAnnotation extends BaseClass { private String objectID; @@ -68,6 +84,20 @@ public void set(String objectId) { } } + /** + * To test if {@link ObjectMapper} fails because of missing type converters + */ + protected static class SomeClassWithInvalidObjectIDType extends BaseClass { + public SomeNonTextualObject someNonTextualObject = new SomeNonTextualObject(); + + protected static class SomeNonTextualObject { + } + + @Override + public void set(String objectId) { + } + } + @ParameterizedTest @ValueSource(classes = { SomeClassWithPublicField.class, @@ -85,9 +115,20 @@ void containsObjectID(Class clazz) { SomeClassWithGetter.class, SomeClassWithAnnotation.class, SomeClassWithGetterAndAnnotation.class}) - void extractObjectID(Class clazz) throws Exception { + void getObjectId(Class clazz) throws Exception { SetObjectId instance = clazz.getDeclaredConstructor().newInstance(); instance.set("12345"); assertEquals("12345", AlgoliaUtils.getObjectID(instance)); } + + @Test + void containsObjectID_WithInvalidType_ThrowsError() { + BeanDescription introspection = AlgoliaUtils.introspectClass(SomeClassWithInvalidObjectIDType.class); + assertFalse(AlgoliaUtils.containsObjectID(introspection)); + } + + @Test + void getObjectID_WithInvalidType_ThrowsError() { + assertThrows(AlgoliaRuntimeException.class, () -> AlgoliaUtils.getObjectID(new SomeClassWithInvalidObjectIDType())); + } } From 137e5a62e3133355e3b4e1b6a4759eb85ebf3743 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marian=20Kl=C3=BChspies?= Date: Sun, 17 Dec 2023 20:42:42 +0100 Subject: [PATCH 3/4] feat: Disabling FAIL_ON_EMPTY_BEANS --- .../src/main/java/com/algolia/search/Defaults.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/algoliasearch-core/src/main/java/com/algolia/search/Defaults.java b/algoliasearch-core/src/main/java/com/algolia/search/Defaults.java index 65bdf8660..a6f20818f 100644 --- a/algoliasearch-core/src/main/java/com/algolia/search/Defaults.java +++ b/algoliasearch-core/src/main/java/com/algolia/search/Defaults.java @@ -35,6 +35,10 @@ private static class Holder { .disable( DeserializationFeature .READ_DATE_TIMESTAMPS_AS_NANOSECONDS) // Nano seconds not supported by the + + .disable( + SerializationFeature + .FAIL_ON_EMPTY_BEANS) // engine .disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES); } From b0eda12cef1f3b2d64bf8947caf1a13bb679ba40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marian=20Kl=C3=BChspies?= Date: Sun, 17 Dec 2023 21:31:13 +0100 Subject: [PATCH 4/4] test: ensuring objectID allows only Strings --- .../com/algolia/search/util/AlgoliaUtils.java | 22 ++++++++++------ .../algolia/search/util/AlgoliaUtilsTest.java | 26 +++++++++++++++---- 2 files changed, 35 insertions(+), 13 deletions(-) diff --git a/algoliasearch-core/src/main/java/com/algolia/search/util/AlgoliaUtils.java b/algoliasearch-core/src/main/java/com/algolia/search/util/AlgoliaUtils.java index 7f3f90e14..b37d7dd69 100644 --- a/algoliasearch-core/src/main/java/com/algolia/search/util/AlgoliaUtils.java +++ b/algoliasearch-core/src/main/java/com/algolia/search/util/AlgoliaUtils.java @@ -1,10 +1,9 @@ package com.algolia.search.util; +import com.algolia.search.Defaults; import com.algolia.search.exceptions.AlgoliaRuntimeException; -import com.fasterxml.jackson.databind.BeanDescription; -import com.fasterxml.jackson.databind.JavaType; -import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.*; +import com.fasterxml.jackson.databind.introspect.BeanPropertyDefinition; import java.time.Clock; import java.time.Instant; @@ -66,18 +65,24 @@ private static AlgoliaRuntimeException objectIDNotFoundException(Class cl * annotations using Jackson's {@link BeanDescription} */ protected static boolean containsObjectID(BeanDescription introspection) { - return introspection.findProperties().stream().anyMatch(d -> PROPERTY_OBJECT_ID.equals(d.getName())); + return introspection.findProperties().stream() + .filter(d -> d.getPrimaryType().isTypeOrSubTypeOf(String.class)) + .anyMatch(d -> PROPERTY_OBJECT_ID.equals(d.getName())); } /** * Introspection of the class using Jackson */ protected static BeanDescription introspectClass(Class clazz) { - ObjectMapper mapper = new ObjectMapper(); + ObjectMapper mapper = getMapper(); JavaType type = mapper.getTypeFactory().constructType(clazz); return mapper.getSerializationConfig().introspect(type); } + private static ObjectMapper getMapper() { + return Defaults.getObjectMapper(); + } + /** * Get the objectID of the given class at runtime * @@ -85,8 +90,9 @@ protected static BeanDescription introspectClass(Class clazz) { * annotation @JsonProperty(\"objectID\"") */ public static String getObjectID(@Nonnull T data) { - return Optional.ofNullable(new ObjectMapper().valueToTree(data) - .get(PROPERTY_OBJECT_ID)) + return Optional.ofNullable(getMapper().valueToTree(data) + .get(PROPERTY_OBJECT_ID)) + .filter(JsonNode::isTextual) .map(JsonNode::asText) .orElseThrow(() -> objectIDNotFoundException(data.getClass())); } diff --git a/algoliasearch-core/src/test/java/com/algolia/search/util/AlgoliaUtilsTest.java b/algoliasearch-core/src/test/java/com/algolia/search/util/AlgoliaUtilsTest.java index d23a662c9..279aa0acd 100644 --- a/algoliasearch-core/src/test/java/com/algolia/search/util/AlgoliaUtilsTest.java +++ b/algoliasearch-core/src/test/java/com/algolia/search/util/AlgoliaUtilsTest.java @@ -88,7 +88,7 @@ public void set(String objectId) { * To test if {@link ObjectMapper} fails because of missing type converters */ protected static class SomeClassWithInvalidObjectIDType extends BaseClass { - public SomeNonTextualObject someNonTextualObject = new SomeNonTextualObject(); + public SomeNonTextualObject objectID = new SomeNonTextualObject(); protected static class SomeNonTextualObject { } @@ -98,6 +98,19 @@ public void set(String objectId) { } } + /** + * Although this would theoretically work, we only allowing Strings + */ + protected static class SomeClassWithPublicFieldOfTypeInt extends BaseClass { + + public int objectID; + + @Override + public void set(String objectId) { + this.objectID = Integer.parseInt(objectId); + } + } + @ParameterizedTest @ValueSource(classes = { SomeClassWithPublicField.class, @@ -122,13 +135,16 @@ void getObjectId(Class clazz) throws Exception { } @Test - void containsObjectID_WithInvalidType_ThrowsError() { + void containsObjectID_WithInvalidType_ReturnsFalse() { BeanDescription introspection = AlgoliaUtils.introspectClass(SomeClassWithInvalidObjectIDType.class); assertFalse(AlgoliaUtils.containsObjectID(introspection)); } - @Test - void getObjectID_WithInvalidType_ThrowsError() { - assertThrows(AlgoliaRuntimeException.class, () -> AlgoliaUtils.getObjectID(new SomeClassWithInvalidObjectIDType())); + @ParameterizedTest + @ValueSource(classes = {SomeClassWithInvalidObjectIDType.class, SomeClassWithPublicFieldOfTypeInt.class}) + void getObjectID_WithInvalidType_ThrowsError(Class clazz) throws Exception { + SetObjectId instance = clazz.getDeclaredConstructor().newInstance(); + instance.set("1234"); + assertThrows(AlgoliaRuntimeException.class, () -> AlgoliaUtils.getObjectID(instance)); } }