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); } 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..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,162 +1,100 @@ package com.algolia.search.util; +import com.algolia.search.Defaults; import com.algolia.search.exceptions.AlgoliaRuntimeException; -import com.fasterxml.jackson.annotation.JsonProperty; -import java.lang.reflect.Field; +import com.fasterxml.jackson.databind.*; +import com.fasterxml.jackson.databind.introspect.BeanPropertyDefinition; + 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); + /** + * 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() + .filter(d -> d.getPrimaryType().isTypeOrSubTypeOf(String.class)) + .anyMatch(d -> PROPERTY_OBJECT_ID.equals(d.getName())); + } - return null; - } + /** + * Introspection of the class using Jackson + */ + protected static BeanDescription introspectClass(Class clazz) { + ObjectMapper mapper = getMapper(); + JavaType type = mapper.getTypeFactory().constructType(clazz); + return mapper.getSerializationConfig().introspect(type); + } - /** - * 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; + private static ObjectMapper getMapper() { + return Defaults.getObjectMapper(); + } - 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(getMapper().valueToTree(data) + .get(PROPERTY_OBJECT_ID)) + .filter(JsonNode::isTextual) + .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..279aa0acd --- /dev/null +++ b/algoliasearch-core/src/test/java/com/algolia/search/util/AlgoliaUtilsTest.java @@ -0,0 +1,150 @@ +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.*; + +class AlgoliaUtilsTest { + + protected interface SetObjectId { + void set(String objectId); + } + + /** + * 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; + + @Override + public void set(String objectId) { + this.objectID = objectId; + } + } + + protected static class SomeClassWithGetter extends BaseClass { + + private String objectID; + + public String getObjectID() { + return objectID; + } + + @Override + public void set(String objectId) { + this.objectID = objectId; + } + } + + + protected static class SomeClassWithAnnotation extends BaseClass { + + @JsonProperty(AlgoliaUtils.PROPERTY_OBJECT_ID) + private String objectID; + + @Override + public void set(String objectId) { + this.objectID = objectId; + } + + } + + protected static class SomeClassWithGetterAndAnnotation extends BaseClass { + + private String objectID; + + @JsonProperty(AlgoliaUtils.PROPERTY_OBJECT_ID) + public String getWithCustomName() { + return objectID; + } + + @Override + public void set(String objectId) { + this.objectID = objectId; + } + } + + /** + * To test if {@link ObjectMapper} fails because of missing type converters + */ + protected static class SomeClassWithInvalidObjectIDType extends BaseClass { + public SomeNonTextualObject objectID = new SomeNonTextualObject(); + + protected static class SomeNonTextualObject { + } + + @Override + 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, + 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 getObjectId(Class clazz) throws Exception { + SetObjectId instance = clazz.getDeclaredConstructor().newInstance(); + instance.set("12345"); + assertEquals("12345", AlgoliaUtils.getObjectID(instance)); + } + + @Test + void containsObjectID_WithInvalidType_ReturnsFalse() { + BeanDescription introspection = AlgoliaUtils.introspectClass(SomeClassWithInvalidObjectIDType.class); + assertFalse(AlgoliaUtils.containsObjectID(introspection)); + } + + @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)); + } +}