From 71e63f9e5275711582e2ad41e77291ee1c1e1ec3 Mon Sep 17 00:00:00 2001 From: Michael Simons Date: Wed, 3 Jul 2019 11:23:17 +0200 Subject: [PATCH] GH-643 - Make sure all kinds of enums are correctly identified. This fixes #643. --- .../org/neo4j/ogm/support/ClassUtils.java | 26 +++++++ .../org/neo4j/ogm/metadata/ClassInfo.java | 2 +- .../org/neo4j/ogm/metadata/DomainInfo.java | 13 ++-- .../metadata/reflect/EntityAccessManager.java | 4 +- .../typeconversion/MapCompositeConverter.java | 5 +- .../domain/properties/UserWithEnumMap.java | 11 ++- .../properties/EnumMapPropertiesTest.java | 7 +- .../org/neo4j/ogm/support/ClassUtilsTest.java | 72 +++++++++++++++++++ 8 files changed, 125 insertions(+), 15 deletions(-) create mode 100644 test/src/test/java/org/neo4j/ogm/support/ClassUtilsTest.java diff --git a/api/src/main/java/org/neo4j/ogm/support/ClassUtils.java b/api/src/main/java/org/neo4j/ogm/support/ClassUtils.java index 8937cfbc25..49ab3d8b36 100644 --- a/api/src/main/java/org/neo4j/ogm/support/ClassUtils.java +++ b/api/src/main/java/org/neo4j/ogm/support/ClassUtils.java @@ -64,6 +64,32 @@ public static ClassLoader getDefaultClassLoader() { return cl; } + /** + * See https://github.com/neo4j/neo4j-ogm/issues/643. An enum instance that overrides methods of the enum itself + * is realized as an anonymous inner class for which {@link Class#isEnum()} returns false. + * + * @param clazz The class to check whether it is an enum or not. + * @return True, if {@code clazz} is an enum. + */ + public static boolean isEnum(Class clazz) { + + return clazz.isEnum() || Enum.class.isAssignableFrom(clazz); + } + + /** + * @param object + * @return True, if the object is an enum instance. + * @see #isEnum(Class) + */ + public static boolean isEnum(Object object) { + + if (object == null) { + return false; + } + + return isEnum(object.getClass()); + } + private ClassUtils() { } } diff --git a/core/src/main/java/org/neo4j/ogm/metadata/ClassInfo.java b/core/src/main/java/org/neo4j/ogm/metadata/ClassInfo.java index d881864914..2e5d0bebc7 100644 --- a/core/src/main/java/org/neo4j/ogm/metadata/ClassInfo.java +++ b/core/src/main/java/org/neo4j/ogm/metadata/ClassInfo.java @@ -116,7 +116,7 @@ public ClassInfo(Class cls) { final int modifiers = cls.getModifiers(); this.isInterface = Modifier.isInterface(modifiers); this.isAbstract = Modifier.isAbstract(modifiers); - this.isEnum = cls.isEnum(); + this.isEnum = org.neo4j.ogm.support.ClassUtils.isEnum(cls); this.className = cls.getName(); if (cls.getSuperclass() != null) { diff --git a/core/src/main/java/org/neo4j/ogm/metadata/DomainInfo.java b/core/src/main/java/org/neo4j/ogm/metadata/DomainInfo.java index 2435842a1e..d59b8285cc 100644 --- a/core/src/main/java/org/neo4j/ogm/metadata/DomainInfo.java +++ b/core/src/main/java/org/neo4j/ogm/metadata/DomainInfo.java @@ -19,6 +19,7 @@ package org.neo4j.ogm.metadata; import static java.util.Comparator.*; +import static org.neo4j.ogm.support.ClassUtils.*; import io.github.lukehutch.fastclasspathscanner.FastClasspathScanner; import io.github.lukehutch.fastclasspathscanner.scanner.ScanResult; @@ -379,13 +380,11 @@ private void registerDefaultFieldConverters(ClassInfo classInfo, FieldInfo field } } - if (!enumConverterSet) { - if (fieldType.isEnum()) { - LOGGER.debug( - "Setting default enum converter for unscanned class " + classInfo.name() + ", field: " - + fieldInfo.getName()); - setEnumFieldConverter(fieldInfo, fieldType); - } + if (!enumConverterSet && isEnum(fieldType)) { + LOGGER.debug( + "Setting default enum converter for unscanned class " + classInfo.name() + ", field: " + + fieldInfo.getName()); + setEnumFieldConverter(fieldInfo, fieldType); } } } diff --git a/core/src/main/java/org/neo4j/ogm/metadata/reflect/EntityAccessManager.java b/core/src/main/java/org/neo4j/ogm/metadata/reflect/EntityAccessManager.java index 2ba0f101c5..e7296b12e9 100644 --- a/core/src/main/java/org/neo4j/ogm/metadata/reflect/EntityAccessManager.java +++ b/core/src/main/java/org/neo4j/ogm/metadata/reflect/EntityAccessManager.java @@ -18,6 +18,8 @@ */ package org.neo4j.ogm.metadata.reflect; +import static org.neo4j.ogm.support.ClassUtils.*; + import java.lang.reflect.Array; import java.util.*; import java.util.concurrent.atomic.AtomicInteger; @@ -204,7 +206,7 @@ private static Object stringToCharacterIterable(Object value, Class parameterTyp return characters; } - if (value.getClass().isArray() && (elementType == String.class || elementType.isEnum())) { + if (value.getClass().isArray() && (elementType == String.class || isEnum(elementType))) { String[] strings = (String[]) value; return Arrays.asList(strings); } diff --git a/core/src/main/java/org/neo4j/ogm/typeconversion/MapCompositeConverter.java b/core/src/main/java/org/neo4j/ogm/typeconversion/MapCompositeConverter.java index b8a14a57fd..2b6f9ef1ce 100644 --- a/core/src/main/java/org/neo4j/ogm/typeconversion/MapCompositeConverter.java +++ b/core/src/main/java/org/neo4j/ogm/typeconversion/MapCompositeConverter.java @@ -20,6 +20,7 @@ import static java.util.Collections.*; import static java.util.stream.Collectors.*; +import static org.neo4j.ogm.support.ClassUtils.*; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.ParameterizedType; @@ -192,7 +193,7 @@ private String keyInstanceToString(Object propertyKey) { if (propertyKey instanceof String) { return (String) propertyKey; - } else if (propertyKey.getClass().isEnum()) { + } else if (isEnum(propertyKey)) { return ((Enum) propertyKey).name(); } @@ -206,7 +207,7 @@ private Object keyInstanceFromString(String propertyKey, Class keyType) { return propertyKey; } else if (keyType.equals(String.class)) { return propertyKey; - } else if (keyType.isEnum()) { + } else if (isEnum(keyType)) { try { return keyType.getDeclaredMethod("valueOf", String.class).invoke(keyType, propertyKey); } catch (IllegalAccessException | InvocationTargetException | NoSuchMethodException e) { diff --git a/test/src/test/java/org/neo4j/ogm/domain/properties/UserWithEnumMap.java b/test/src/test/java/org/neo4j/ogm/domain/properties/UserWithEnumMap.java index d63e36234c..278540c2a0 100644 --- a/test/src/test/java/org/neo4j/ogm/domain/properties/UserWithEnumMap.java +++ b/test/src/test/java/org/neo4j/ogm/domain/properties/UserWithEnumMap.java @@ -26,6 +26,7 @@ /** * @author Frantisek Hartman + * @author Michael J. Simons */ @NodeEntity(label = "User") public class UserWithEnumMap { @@ -70,7 +71,15 @@ public void putMyProperty(UserProperties key, Object value) { public enum UserProperties { CITY, - ZIP_CODE, + ZIP_CODE { + @Override + void doSomething() { + } + }; + + void doSomething() { + + } } } diff --git a/test/src/test/java/org/neo4j/ogm/persistence/types/properties/EnumMapPropertiesTest.java b/test/src/test/java/org/neo4j/ogm/persistence/types/properties/EnumMapPropertiesTest.java index f65a8f531e..eae2fcd6e4 100644 --- a/test/src/test/java/org/neo4j/ogm/persistence/types/properties/EnumMapPropertiesTest.java +++ b/test/src/test/java/org/neo4j/ogm/persistence/types/properties/EnumMapPropertiesTest.java @@ -36,20 +36,21 @@ /** * @author Frantisek Hartman + * @author Michael J. Simons */ public class EnumMapPropertiesTest extends MultiDriverTestClass { private static Session session; @BeforeClass - public static void init() throws IOException { + public static void init() { session = new SessionFactory(driver, UserWithEnumMap.class.getName()) .openSession(); } @Before - public void setUp() throws Exception { + public void setUp() { session.purgeDatabase(); } @@ -74,7 +75,7 @@ public void shouldMapMapAttributeToProperties() throws Exception { } @Test - public void shouldMapNodePropertiesToPropertiesAttribute() throws Exception { + public void shouldMapNodePropertiesToPropertiesAttribute() { session.query( "CREATE (u:User {`name`:'Frantisek', `myProperties.CITY`:'London', `myProperties.ZIP_CODE`:'SW1A 1AA'})", emptyMap()); diff --git a/test/src/test/java/org/neo4j/ogm/support/ClassUtilsTest.java b/test/src/test/java/org/neo4j/ogm/support/ClassUtilsTest.java new file mode 100644 index 0000000000..004026467b --- /dev/null +++ b/test/src/test/java/org/neo4j/ogm/support/ClassUtilsTest.java @@ -0,0 +1,72 @@ +/* + * Copyright (c) 2002-2019 "Neo4j," + * Neo4j Sweden AB [http://neo4j.com] + * + * This file is part of Neo4j. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.neo4j.ogm.support; + +import static org.assertj.core.api.Assertions.*; + +import org.junit.Test; + +/** + * @author Michael J. Simons + */ +public class ClassUtilsTest { + + enum YourFriendlyEnumMostPeopleUse { + THING1, THING2 + } + + enum YesEnumsCanBeMuchMore { + THING1, THING2 { + @Override + public void something() { + } + }; + + @SuppressWarnings({ "unused" }) + void something() { + } + } + + interface SomeInterface { + } + + enum Singleton implements SomeInterface { + THE_INSTANCE + } + + @Test + public void shouldWorkWithPlainEnum() { + assertThat(ClassUtils.isEnum(YourFriendlyEnumMostPeopleUse.class)).isTrue(); + assertThat(ClassUtils.isEnum(YourFriendlyEnumMostPeopleUse.THING1)).isTrue(); + assertThat(ClassUtils.isEnum(YourFriendlyEnumMostPeopleUse.THING2)).isTrue(); + } + + @Test + public void shouldWorkWithExtendedEnum() { + assertThat(ClassUtils.isEnum(YesEnumsCanBeMuchMore.class)).isTrue(); + assertThat(ClassUtils.isEnum(YesEnumsCanBeMuchMore.THING1)).isTrue(); + assertThat(ClassUtils.isEnum(YesEnumsCanBeMuchMore.THING2)).isTrue(); + } + + @Test + public void shouldWorkWithInterfaceEnum() { + assertThat(ClassUtils.isEnum(Singleton.class)).isTrue(); + assertThat(ClassUtils.isEnum(Singleton.THE_INSTANCE)).isTrue(); + } +}