From 3b46d03e1d07cc6b68ba86034c55fa5d89c1f799 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Wed, 27 Sep 2023 23:52:51 +0200 Subject: [PATCH 01/16] Create record inspector (for future use in bean mapping) --- .../internal/record/RecordComponent.java | 40 ++++++ .../internal/record/RecordInspector.java | 87 ++++++++++++ .../internal/record/ReflectionHelper.java | 58 ++++++++ .../internal/record/RecordInspectorTest.java | 134 ++++++++++++++++++ .../internal/record/ReflectionHelperTest.java | 86 +++++++++++ 5 files changed, 405 insertions(+) create mode 100644 src/main/java/ch/jalu/configme/internal/record/RecordComponent.java create mode 100644 src/main/java/ch/jalu/configme/internal/record/RecordInspector.java create mode 100644 src/main/java/ch/jalu/configme/internal/record/ReflectionHelper.java create mode 100644 src/test/java/ch/jalu/configme/internal/record/RecordInspectorTest.java create mode 100644 src/test/java/ch/jalu/configme/internal/record/ReflectionHelperTest.java diff --git a/src/main/java/ch/jalu/configme/internal/record/RecordComponent.java b/src/main/java/ch/jalu/configme/internal/record/RecordComponent.java new file mode 100644 index 00000000..233044a0 --- /dev/null +++ b/src/main/java/ch/jalu/configme/internal/record/RecordComponent.java @@ -0,0 +1,40 @@ +package ch.jalu.configme.internal.record; + +import org.jetbrains.annotations.NotNull; + +import java.lang.reflect.Type; + +/** + * Represents information about the component (a "property") of a Java record. + */ +public class RecordComponent { + + private final String name; + private final Class type; + private final Type genericType; + + /** + * Constructor. + * + * @param name the name of this component + * @param type the type of this component + * @param genericType type representing the generic type signature of this component + */ + public RecordComponent(@NotNull String name, @NotNull Class type, @NotNull Type genericType) { + this.name = name; + this.type = type; + this.genericType = genericType; + } + + public @NotNull String getName() { + return name; + } + + public @NotNull Class getType() { + return type; + } + + public @NotNull Type getGenericType() { + return genericType; + } +} diff --git a/src/main/java/ch/jalu/configme/internal/record/RecordInspector.java b/src/main/java/ch/jalu/configme/internal/record/RecordInspector.java new file mode 100644 index 00000000..e3597bb9 --- /dev/null +++ b/src/main/java/ch/jalu/configme/internal/record/RecordInspector.java @@ -0,0 +1,87 @@ +package ch.jalu.configme.internal.record; + +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.lang.reflect.Method; +import java.lang.reflect.Type; +import java.util.Arrays; + +/** + * Inspects classes and returns Record information (Java 14+). The inspection is performed by reflection + * because ConfigMe is still compiled against Java 8. + */ +public class RecordInspector { + + private final ReflectionHelper reflectionHelper; + private Method isRecordMethod; // Class#isRecord + private Method getRecordComponentsMethod; // Class#getRecordComponents + + private Method getComponentNameMethod; // RecordComponent#getName + private Method getComponentTypeMethod; // RecordComponent#getType + private Method getComponentGenericTypeMethod; // RecordComponent#getGenericType + + public RecordInspector(@NotNull ReflectionHelper reflectionHelper) { + this.reflectionHelper = reflectionHelper; + } + + /** + * Returns whether the given class is a record. + *

+ * This method uses {@code Class#isRecord} in a way that is compatible with Java 8 and above. + * + * @param clazz the class to inspect + * @return true if it's a record, false otherwise + */ + public boolean isRecord(@NotNull Class clazz) { + // Check superclass to make sure that Class#isRecord will exist, and to avoid redundant reflective + // calls to the method if we can rule out records anyway + if (hasRecordAsSuperclass(clazz)) { + if (isRecordMethod == null) { + isRecordMethod = reflectionHelper.getZeroArgMethod(Class.class, "isRecord"); + } + return reflectionHelper.invokeZeroArgMethod(isRecordMethod, clazz); + } + return false; + } + + /** + * Returns the components that make up the record. This method should only be called after checking that + * the class is a record ({@link #isRecord(Class)}). + *

+ * This calls {code Class#getRecordComponents} in a way that is compatible with Java 8 and above. + * + * @param clazz a record type whose components should be returned + * @return the record's components + */ + public RecordComponent @Nullable [] getRecordComponents(@NotNull Class clazz) { + if (getRecordComponentsMethod == null) { + getRecordComponentsMethod = reflectionHelper.getZeroArgMethod(Class.class, "getRecordComponents"); + } + + Object[] components = reflectionHelper.invokeZeroArgMethod(getRecordComponentsMethod, clazz); + if (getComponentGenericTypeMethod == null) { + Class recordComponentClass = reflectionHelper.getClassOrThrow("java.lang.reflect.RecordComponent"); + getComponentNameMethod = reflectionHelper.getZeroArgMethod(recordComponentClass, "getName"); + getComponentTypeMethod = reflectionHelper.getZeroArgMethod(recordComponentClass, "getType"); + getComponentGenericTypeMethod = reflectionHelper.getZeroArgMethod(recordComponentClass, "getGenericType"); + } + + return Arrays.stream(components) + .map(this::mapComponent) + .toArray(RecordComponent[]::new); + } + + boolean hasRecordAsSuperclass(@NotNull Class clazz) { + return clazz.getSuperclass() != null + && "java.lang.Record".equals(clazz.getSuperclass().getName()); + } + + private @NotNull RecordComponent mapComponent(@NotNull Object component) { + String name = reflectionHelper.invokeZeroArgMethod(getComponentNameMethod, component); + Class type = reflectionHelper.invokeZeroArgMethod(getComponentTypeMethod, component); + Type genericType = reflectionHelper.invokeZeroArgMethod(getComponentGenericTypeMethod, component); + + return new RecordComponent(name, type, genericType); + } +} diff --git a/src/main/java/ch/jalu/configme/internal/record/ReflectionHelper.java b/src/main/java/ch/jalu/configme/internal/record/ReflectionHelper.java new file mode 100644 index 00000000..a14fb00d --- /dev/null +++ b/src/main/java/ch/jalu/configme/internal/record/ReflectionHelper.java @@ -0,0 +1,58 @@ +package ch.jalu.configme.internal.record; + +import ch.jalu.configme.exception.ConfigMeException; +import ch.jalu.typeresolver.classutil.ClassUtils; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.lang.reflect.Method; + +/** + * Internal helper for reflective operations. + */ +public class ReflectionHelper { + + /** + * Loads the class by fully qualified name, throwing an exception if the class does not exist. + * + * @param name the name of the class to return (e.g. java.lang.Integer) + * @return the requested class + */ + public @NotNull Class getClassOrThrow(@NotNull String name) { + return ClassUtils.loadClassOrThrow(name); + } + + /** + * Returns the method with the given name on the given class. The method is assumed to have zero arguments; + * if it doesn't exist, a runtime exception is thrown. + * + * @param declarer the class declaring the method + * @param name the name of the method to retrieve + * @return the specified method + */ + public @NotNull Method getZeroArgMethod(@NotNull Class declarer, @NotNull String name) { + try { + return declarer.getDeclaredMethod(name); + } catch (NoSuchMethodException e) { + throw new ConfigMeException("Could not get " + declarer.getSimpleName() + "#" + name + " method", e); + } + } + + /** + * Invokes the given method (with zero arguments) on the given {@code instance} object. A runtime exception is + * thrown if the method invocation failed. + * + * @param method the method to invoke + * @param instance the object to invoke it on + * @param the return type (type is not statically checked) + * @return the + */ + @SuppressWarnings("unchecked") + public @NotNull T invokeZeroArgMethod(@NotNull Method method, @Nullable Object instance) { + try { + return (T) method.invoke(instance); + } catch (ReflectiveOperationException e) { + throw new ConfigMeException("Failed to call " + method + " for " + instance, e); + } + } +} diff --git a/src/test/java/ch/jalu/configme/internal/record/RecordInspectorTest.java b/src/test/java/ch/jalu/configme/internal/record/RecordInspectorTest.java new file mode 100644 index 00000000..b6f4d0d3 --- /dev/null +++ b/src/test/java/ch/jalu/configme/internal/record/RecordInspectorTest.java @@ -0,0 +1,134 @@ +package ch.jalu.configme.internal.record; + +import org.jetbrains.annotations.NotNull; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.lang.reflect.Method; +import java.lang.reflect.Type; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.arrayWithSize; +import static org.hamcrest.Matchers.equalTo; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verifyNoInteractions; + +/** + * Test for {@link RecordInspector}. + */ +@ExtendWith(MockitoExtension.class) +class RecordInspectorTest { + + @Test + void shouldCheckIfClassExtendsRecord() { + // given + ReflectionHelper reflectionHelper = mock(ReflectionHelper.class); + RecordInspector recordInspector = new RecordInspector(reflectionHelper); + + // when / then + assertFalse(recordInspector.isRecord(Object.class)); + assertFalse(recordInspector.isRecord(Integer.class)); + assertFalse(recordInspector.isRecord(int.class)); + verifyNoInteractions(reflectionHelper); + } + + @Test + void shouldReturnThatClassIsRecord() throws NoSuchMethodException { + // given + ReflectionHelper reflectionHelper = mock(ReflectionHelper.class); + Method isPrimitiveMethod = Class.class.getDeclaredMethod("isPrimitive"); + given(reflectionHelper.getZeroArgMethod(Class.class, "isRecord")).willReturn(isPrimitiveMethod); + given(reflectionHelper.invokeZeroArgMethod(any(Method.class), any(Class.class))).willCallRealMethod(); + + RecordInspector recordInspector = new RecordInspector(reflectionHelper) { + @Override + public boolean hasRecordAsSuperclass(@NotNull Class clazz) { + return true; + } + }; + + // when / then + assertThat(recordInspector.isRecord(int.class), equalTo(true)); + assertThat(recordInspector.isRecord(String.class), equalTo(false)); + } + + @Test + void shouldReturnComponents() throws NoSuchMethodException { + // given + ReflectionHelper reflectionHelper = mock(ReflectionHelper.class); + + // Instead of Class#getRecordComponents, return method for FakeRecord#getComponents + Method fakeGetComponentsMethod = FakeRecordType.class.getDeclaredMethod("getComponents"); + given(reflectionHelper.getZeroArgMethod(Class.class, "getRecordComponents")) + .willReturn(fakeGetComponentsMethod); + + // Return FakeRecordComponent.class when we want to load Java 14+ RecordComponent + Class fakeRecordComponentClass = FakeRecordComponent.class; + given(reflectionHelper.getClassOrThrow("java.lang.reflect.RecordComponent")) + .willReturn((Class) fakeRecordComponentClass); + + // All methods on FakeRecordComponent are named as on the RecordComponent, so pass the calls through + given(reflectionHelper.getZeroArgMethod(eq(fakeRecordComponentClass), anyString())) + .willCallRealMethod(); + given(reflectionHelper.invokeZeroArgMethod(any(Method.class), any(Object.class))).willCallRealMethod(); + + RecordInspector recordInspector = new RecordInspector(reflectionHelper) { + @Override + public boolean hasRecordAsSuperclass(@NotNull Class clazz) { + return true; + } + }; + + // when + RecordComponent[] components = recordInspector.getRecordComponents(FakeRecordType.class); + + // then + assertThat(components, arrayWithSize(2)); + assertThat(components[0].getName(), equalTo("age")); + assertThat(components[0].getType(), equalTo(int.class)); + assertThat(components[0].getGenericType(), equalTo(int.class)); + assertThat(components[1].getName(), equalTo("location")); + assertThat(components[1].getType(), equalTo(String.class)); + assertThat(components[1].getGenericType(), equalTo(Object.class)); + } + + private static final class FakeRecordType { + + public static FakeRecordComponent[] getComponents() { + FakeRecordComponent component1 = new FakeRecordComponent("age", int.class, int.class); + FakeRecordComponent component2 = new FakeRecordComponent("location", String.class, Object.class); + return new FakeRecordComponent[]{ component1, component2 }; + } + } + + private static final class FakeRecordComponent { + + private final String name; + private final Class type; + private final Type genericType; + + private FakeRecordComponent(String name, Class type, Type genericType) { + this.name = name; + this.type = type; + this.genericType = genericType; + } + + public String getName() { + return name; + } + + public Class getType() { + return type; + } + + public Type getGenericType() { + return genericType; + } + } +} diff --git a/src/test/java/ch/jalu/configme/internal/record/ReflectionHelperTest.java b/src/test/java/ch/jalu/configme/internal/record/ReflectionHelperTest.java new file mode 100644 index 00000000..8c368ffb --- /dev/null +++ b/src/test/java/ch/jalu/configme/internal/record/ReflectionHelperTest.java @@ -0,0 +1,86 @@ +package ch.jalu.configme.internal.record; + +import ch.jalu.configme.exception.ConfigMeException; +import ch.jalu.configme.properties.Property; +import org.junit.jupiter.api.Test; + +import java.lang.reflect.Method; +import java.util.HashMap; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; +import static org.junit.jupiter.api.Assertions.assertThrows; + +/** + * Test for {@link ReflectionHelper}. + */ +class ReflectionHelperTest { + + private final ReflectionHelper reflectionHelper = new ReflectionHelper(); + + @Test + void shouldReturnMethod() throws NoSuchMethodException { + // given / when + Method floatValueMethod = reflectionHelper.getZeroArgMethod(Integer.class, "floatValue"); + + // then + assertThat(floatValueMethod, equalTo(Integer.class.getDeclaredMethod("floatValue"))); + } + + @Test + void shouldThrowConfigMeExceptionForUnknownMethod() { + // given / when + ConfigMeException ex = assertThrows(ConfigMeException.class, + () -> reflectionHelper.getZeroArgMethod(Integer.class, "bogus")); + + // then + assertThat(ex.getMessage(), equalTo("Could not get Integer#bogus method")); + assertThat(ex.getCause(), instanceOf(NoSuchMethodException.class)); + } + + @Test + void shouldReturnClass() { + // given / when / then + assertThat(reflectionHelper.getClassOrThrow("java.lang.Integer"), equalTo(Integer.class)); + assertThat(reflectionHelper.getClassOrThrow("ch.jalu.configme.properties.Property"), equalTo(Property.class)); + } + + @Test + void shouldThrowForUnknownClass() { + // given / when + IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, + () -> reflectionHelper.getClassOrThrow("java.lang.Bogus")); + + // then + assertThat(ex.getMessage(), equalTo("Class 'java.lang.Bogus' could not be loaded")); + assertThat(ex.getCause(), instanceOf(ClassNotFoundException.class)); + } + + @Test + void shouldCallMethod() throws NoSuchMethodException { + // given + Integer number = 19; + Method toStringMethod = Integer.class.getDeclaredMethod("toString"); + + // when + String result = reflectionHelper.invokeZeroArgMethod(toStringMethod, number); + + // then + assertThat(result, equalTo("19")); + } + + @Test + void shouldWrapExceptionIfCallingMethodFails() throws NoSuchMethodException { + // given + Method toStringMethod = HashMap.class.getDeclaredMethod("resize"); + + // when + ConfigMeException ex = assertThrows(ConfigMeException.class, + () -> reflectionHelper.invokeZeroArgMethod(toStringMethod, new HashMap<>())); + + // then + assertThat(ex.getMessage(), equalTo("Failed to call final java.util.HashMap$Node[] java.util.HashMap.resize() for {}")); + assertThat(ex.getCause(), instanceOf(IllegalAccessException.class)); + } +} From e2f32cafadb0b3f9e4cff6c7a243aabcff8dcf17 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Tue, 14 Nov 2023 12:48:18 +0100 Subject: [PATCH 02/16] Rough POC - support different bean instantiations --- .../jalu/configme/beanmapper/MapperImpl.java | 40 ++--- .../instantiation/BeanInspector.java | 170 ++++++++++++++++++ .../instantiation/BeanInstantiation.java | 17 ++ .../BeanDescriptionFactory.java | 6 +- .../BeanDescriptionFactoryImpl.java | 46 ++++- .../BeanPropertyDescription.java | 9 - .../propertydescription/FieldProperty.java | 66 +++++++ .../internal/record/ReflectionHelper.java | 3 +- .../BeanExtensionTest.java | 4 +- .../BeanPropertyDescriptionImplTest.java | 21 +-- 10 files changed, 326 insertions(+), 56 deletions(-) create mode 100644 src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInspector.java create mode 100644 src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiation.java create mode 100644 src/main/java/ch/jalu/configme/beanmapper/propertydescription/FieldProperty.java diff --git a/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java b/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java index 239d66be..b57d30cd 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java +++ b/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java @@ -4,6 +4,8 @@ import ch.jalu.configme.beanmapper.context.ExportContextImpl; import ch.jalu.configme.beanmapper.context.MappingContext; import ch.jalu.configme.beanmapper.context.MappingContextImpl; +import ch.jalu.configme.beanmapper.instantiation.BeanInspector; +import ch.jalu.configme.beanmapper.instantiation.BeanInstantiation; import ch.jalu.configme.beanmapper.leafvaluehandler.LeafValueHandler; import ch.jalu.configme.beanmapper.leafvaluehandler.LeafValueHandlerImpl; import ch.jalu.configme.beanmapper.leafvaluehandler.MapperLeafType; @@ -25,6 +27,7 @@ import java.util.Map; import java.util.Optional; import java.util.TreeMap; +import java.util.stream.Collectors; import static ch.jalu.configme.internal.PathUtils.OPTIONAL_SPECIFIER; import static ch.jalu.configme.internal.PathUtils.pathSpecifierForIndex; @@ -70,6 +73,7 @@ public class MapperImpl implements Mapper { private final BeanDescriptionFactory beanDescriptionFactory; private final LeafValueHandler leafValueHandler; + private final BeanInspector beanInspector = new BeanInspector(); // TODO: Make configurable, add interface... public MapperImpl() { this(new BeanDescriptionFactoryImpl(), @@ -131,6 +135,7 @@ public MapperImpl(@NotNull BeanDescriptionFactory beanDescriptionFactory, // Step 3: treat as bean Map mappedBean = new LinkedHashMap<>(); + // TODO: Adapt me for (BeanPropertyDescription property : beanDescriptionFactory.getAllProperties(value.getClass())) { Object exportValueOfProperty = toExportValue(property.getValue(value), exportContext); if (exportValueOfProperty != null) { @@ -369,30 +374,21 @@ public MapperImpl(@NotNull BeanDescriptionFactory beanDescriptionFactory, if (!(value instanceof Map)) { return null; } - - Collection properties = - beanDescriptionFactory.getAllProperties(context.getTargetTypeAsClassOrThrow()); - // Check that we have properties (or else we don't have a bean) - if (properties.isEmpty()) { - return null; - } - Map entries = (Map) value; - Object bean = createBeanMatchingType(context); - for (BeanPropertyDescription property : properties) { - Object result = convertValueForType( - context.createChild(property.getName(), property.getTypeInformation()), - entries.get(property.getName())); - if (result == null) { - if (property.getValue(bean) == null) { - return null; // We do not support beans with a null value - } - context.registerError("No value found, fallback to field default value"); - } else { - property.setValue(bean, result); - } + + Optional instantiation = + beanInspector.findInstantiation(context.getTargetTypeAsClassOrThrow()); + if (instantiation.isPresent()) { + List propertyValues = instantiation.get().getProperties().stream() + .map(prop -> { + MappingContext childContext = context.createChild(prop.getName(), prop.getTypeInformation()); + return convertValueForType(childContext, entries.get(prop.getName())); + }) + .collect(Collectors.toList()); + + return instantiation.get().create(propertyValues, context.getErrorRecorder()); } - return bean; + return null; } /** diff --git a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInspector.java b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInspector.java new file mode 100644 index 00000000..84dfb530 --- /dev/null +++ b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInspector.java @@ -0,0 +1,170 @@ +package ch.jalu.configme.beanmapper.instantiation; + +import ch.jalu.configme.beanmapper.propertydescription.BeanDescriptionFactory; +import ch.jalu.configme.beanmapper.propertydescription.BeanDescriptionFactoryImpl; +import ch.jalu.configme.beanmapper.propertydescription.BeanPropertyComments; +import ch.jalu.configme.beanmapper.propertydescription.BeanPropertyDescription; +import ch.jalu.configme.beanmapper.propertydescription.FieldProperty; +import ch.jalu.configme.exception.ConfigMeException; +import ch.jalu.configme.internal.record.RecordComponent; +import ch.jalu.configme.internal.record.RecordInspector; +import ch.jalu.configme.internal.record.ReflectionHelper; +import ch.jalu.configme.properties.convertresult.ConvertErrorRecorder; +import ch.jalu.typeresolver.TypeInfo; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.lang.reflect.Constructor; +import java.util.Arrays; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; +import java.util.Objects; +import java.util.Optional; +import java.util.stream.Collectors; + +public class BeanInspector { + + private final RecordInspector recordInspector = new RecordInspector(new ReflectionHelper()); + private final BeanDescriptionFactory beanDescriptionFactory = new BeanDescriptionFactoryImpl(); + + public @NotNull Optional findInstantiation(@NotNull Class clazz) { + if (recordInspector.isRecord(clazz)) { + RecordComponent[] recordComponents = recordInspector.getRecordComponents(clazz); + return Optional.of(new RecordInstantiation(clazz, Arrays.asList(recordComponents))); + } + + Optional> zeroArgConstructor = getConstructor(clazz); + if (zeroArgConstructor.isPresent()) { + List properties = beanDescriptionFactory.getAllProperties2(clazz); + return Optional.of(new BeanNoArgConstructor(zeroArgConstructor.get(), properties)); + } + + return Optional.empty(); + } + + private static @NotNull Optional> getConstructor(@NotNull Class declarer, + Class @NotNull ... parameters) { + try { + return Optional.of(declarer.getDeclaredConstructor(parameters)); + } catch (NoSuchMethodException ignore) { + return Optional.empty(); + } + } + + private static final class BeanNoArgConstructor implements BeanInstantiation { + + private final Constructor zeroArgsConstructor; + private final List properties; + + private BeanNoArgConstructor(@NotNull Constructor zeroArgsConstructor, + @NotNull List properties) { + this.zeroArgsConstructor = zeroArgsConstructor; + this.properties = properties; + } + + @Override + public @NotNull List getProperties() { + return Collections.unmodifiableList(properties); + } + + @Override + public @Nullable Object create(@NotNull List propertyValues, + @NotNull ConvertErrorRecorder errorRecorder) { + final Object bean; + try { + bean = zeroArgsConstructor.newInstance(); + } catch (ReflectiveOperationException e) { + throw new ConfigMeException("Failed to call constructor for " + + zeroArgsConstructor.getDeclaringClass()); + } + + if (propertyValues.size() != properties.size()) { + throw new ConfigMeException("Invalid property values, " + propertyValues.size() + " were given, but " + + zeroArgsConstructor.getDeclaringClass() + " has " + properties.size() + " properties"); + } + + Iterator propIt = properties.iterator(); + Iterator valuesIt = propertyValues.iterator(); + while (propIt.hasNext() && valuesIt.hasNext()) { + FieldProperty property = propIt.next(); + Object value = valuesIt.next(); + if (value == null) { + if (property.getValue(bean) == null) { + return null; // No default value on field, return null -> no bean with a null value + } + errorRecorder.setHasError("Fallback to default value"); + } else { + property.setValue(bean, value); + } + } + return bean; + } + } + + private static final class RecordInstantiation implements BeanInstantiation { + + private final Constructor canonicalConstructor; + private final List properties; + + public RecordInstantiation(@NotNull Class clazz, @NotNull List components) { + this.properties = components.stream() + .map(RecordProperty::new) + .collect(Collectors.toList()); + Class[] recordTypes = components.stream().map(RecordComponent::getType).toArray(Class[]::new); + this.canonicalConstructor = getConstructor(clazz, recordTypes) + .orElseThrow(() -> new ConfigMeException("Could not get canonical constructor of " + clazz)); + } + + @Override + public @NotNull List getProperties() { + return properties; + } + + @Override + public @Nullable Object create(@NotNull List propertyValues, + @NotNull ConvertErrorRecorder errorRecorder) { + if (propertyValues.stream().anyMatch(Objects::isNull)) { + return null; // No support for null values in records + } + + Object[] properties = propertyValues.toArray(); + try { + return canonicalConstructor.newInstance(properties); + } catch (IllegalArgumentException | ReflectiveOperationException e) { + // TODO: Separate clause for InvocationTargetException? + throw new ConfigMeException("Error calling record constructor for " + + canonicalConstructor.getDeclaringClass(), e); + } + } + } + + private static final class RecordProperty implements BeanPropertyDescription { + + private final RecordComponent recordComponent; + + private RecordProperty(@NotNull RecordComponent recordComponent) { + this.recordComponent = recordComponent; + } + + @Override + public @NotNull String getName() { + return recordComponent.getName(); + } + + @Override + public @NotNull TypeInfo getTypeInformation() { + return TypeInfo.of(recordComponent.getGenericType()); + } + + @Override + public @Nullable Object getValue(@NotNull Object bean) { + throw new UnsupportedOperationException(); // TODO + } + + @Override + public @NotNull BeanPropertyComments getComments() { + return BeanPropertyComments.EMPTY; // TODO + } + } +} diff --git a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiation.java b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiation.java new file mode 100644 index 00000000..db4e7b79 --- /dev/null +++ b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiation.java @@ -0,0 +1,17 @@ +package ch.jalu.configme.beanmapper.instantiation; + +import ch.jalu.configme.beanmapper.propertydescription.BeanPropertyDescription; +import ch.jalu.configme.properties.convertresult.ConvertErrorRecorder; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.List; + +public interface BeanInstantiation { + + @NotNull List getProperties(); + + @Nullable Object create(@NotNull List propertyValues, + @NotNull ConvertErrorRecorder errorRecorder); + +} diff --git a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactory.java b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactory.java index 6551d2f5..ac5e0a07 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactory.java +++ b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactory.java @@ -2,7 +2,7 @@ import org.jetbrains.annotations.NotNull; -import java.util.Collection; +import java.util.List; /** * Factory which analyzes a class and returns all writable properties. @@ -18,6 +18,8 @@ public interface BeanDescriptionFactory { * @param clazz the class whose properties should be returned * @return the relevant properties on the class */ - @NotNull Collection getAllProperties(@NotNull Class clazz); + @NotNull List getAllProperties(@NotNull Class clazz); + + @NotNull List getAllProperties2(@NotNull Class clazz); } diff --git a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImpl.java b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImpl.java index 99433a27..3474ac21 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImpl.java +++ b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImpl.java @@ -11,6 +11,7 @@ import java.beans.Introspector; import java.beans.PropertyDescriptor; import java.lang.reflect.Field; +import java.lang.reflect.Modifier; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -45,10 +46,16 @@ public class BeanDescriptionFactoryImpl implements BeanDescriptionFactory { * @return the bean class's properties to handle */ @Override - public @NotNull Collection getAllProperties(@NotNull Class clazz) { + public @NotNull List getAllProperties(@NotNull Class clazz) { return classProperties.computeIfAbsent(clazz, this::collectAllProperties); } + @Override + public @NotNull List getAllProperties2(@NotNull Class clazz) { + // TODO: Caching. Here, or elsewhere? + return collectAllProperties2(clazz); + } + /** * Collects all properties available on the given class. * @@ -67,6 +74,27 @@ public class BeanDescriptionFactoryImpl implements BeanDescriptionFactory { return properties; } + /** + * Collects all properties available on the given class. + * + * @param clazz the class to process + * @return properties of the class + */ + protected @NotNull List collectAllProperties2(@NotNull Class clazz) { + List> parentsAndClass = collectClassAndAllParents(clazz); + + // TODO: Use field utils + List properties = parentsAndClass.stream() + .flatMap(clz -> Arrays.stream(clz.getDeclaredFields())) + .filter(field -> !Modifier.isStatic(field.getModifiers()) && !field.isSynthetic()) + .map(this::convert2) + .filter(Objects::nonNull) + .collect(Collectors.toList()); + + validateProperties(clazz, properties); + return properties; + } + /** * Converts a {@link PropertyDescriptor} to a {@link BeanPropertyDescription} object. * @@ -88,6 +116,14 @@ public class BeanDescriptionFactoryImpl implements BeanDescriptionFactory { comments); } + protected @Nullable FieldProperty convert2(@NotNull Field field) { + // TODO: Annotation to ignore field + + return new FieldProperty(field, + getCustomExportName(field), + getComments(field)); + } + /** * Returns the comments that are defined on the property. Comments are found by looking for an @{@link Comment} * annotation on a field with the same name as the property. @@ -126,7 +162,7 @@ public class BeanDescriptionFactoryImpl implements BeanDescriptionFactory { * @param properties the properties that will be used on the class */ protected void validateProperties(@NotNull Class clazz, - @NotNull Collection properties) { + @NotNull Collection properties) { Set names = new HashSet<>(properties.size()); properties.forEach(property -> { if (property.getName().isEmpty()) { @@ -153,6 +189,12 @@ protected void validateProperties(@NotNull Class clazz, return descriptor.getName(); } + protected @Nullable String getCustomExportName(@NotNull Field field) { + return field.isAnnotationPresent(ExportName.class) + ? field.getAnnotation(ExportName.class).value() + : null; + } + protected @NotNull TypeInfo createTypeInfo(@NotNull PropertyDescriptor descriptor) { return new TypeInfo(descriptor.getWriteMethod().getGenericParameterTypes()[0]); } diff --git a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescription.java b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescription.java index bc298682..56c8eff5 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescription.java +++ b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescription.java @@ -22,15 +22,6 @@ public interface BeanPropertyDescription { */ @NotNull TypeInfo getTypeInformation(); - /** - * Sets the given value on the provided bean for this property. The value should correspond - * to the {@link #getTypeInformation() property type}. - * - * @param bean the bean to set the property on - * @param value the value to set - */ - void setValue(@NotNull Object bean, @NotNull Object value); - /** * Returns the value of the property for the given bean. * diff --git a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/FieldProperty.java b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/FieldProperty.java new file mode 100644 index 00000000..3b632d31 --- /dev/null +++ b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/FieldProperty.java @@ -0,0 +1,66 @@ +package ch.jalu.configme.beanmapper.propertydescription; + +import ch.jalu.configme.beanmapper.ConfigMeMapperException; +import ch.jalu.configme.exception.ConfigMeException; +import ch.jalu.typeresolver.TypeInfo; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.lang.reflect.Field; + +public class FieldProperty implements BeanPropertyDescription { + + private final Field field; + private final String exportName; + private final BeanPropertyComments comments; + + public FieldProperty(@NotNull Field field, + @Nullable String exportName, + @NotNull BeanPropertyComments comments) { + this.field = field; + this.exportName = exportName; + this.comments = comments; + } + + @Override + public @NotNull String getName() { + return exportName == null + ? field.getName() + : exportName; + } + + @Override + public @NotNull TypeInfo getTypeInformation() { + return TypeInfo.of(field); + } + + public void setValue(@NotNull Object bean, @NotNull Object value) { + if (!field.isAccessible()) { + field.setAccessible(true); // todo: exception handling + } + try { + field.set(bean, value); + } catch (IllegalArgumentException | IllegalAccessException e) { + // todo: Use field utils for field name + throw new ConfigMeMapperException("Failed to set value to field " + field + ". Value: " + value, e); + } + } + + @Override + public @Nullable Object getValue(@NotNull Object bean) { + if (!field.isAccessible()) { + field.setAccessible(true); // todo: exception handling + } + try { + return field.get(bean); + } catch (IllegalArgumentException | IllegalAccessException e) { + // TODO: use field utils for field name + throw new ConfigMeException("Failed to get value for field " + field, e); + } + } + + @Override + public @NotNull BeanPropertyComments getComments() { + return comments; + } +} diff --git a/src/main/java/ch/jalu/configme/internal/record/ReflectionHelper.java b/src/main/java/ch/jalu/configme/internal/record/ReflectionHelper.java index a14fb00d..441a5fe4 100644 --- a/src/main/java/ch/jalu/configme/internal/record/ReflectionHelper.java +++ b/src/main/java/ch/jalu/configme/internal/record/ReflectionHelper.java @@ -45,9 +45,10 @@ public class ReflectionHelper { * @param method the method to invoke * @param instance the object to invoke it on * @param the return type (type is not statically checked) - * @return the + * @return the return value of the method */ @SuppressWarnings("unchecked") + // TODO: @NotNull on return value not generically valid - revise? public @NotNull T invokeZeroArgMethod(@NotNull Method method, @Nullable Object instance) { try { return (T) method.invoke(instance); diff --git a/src/test/java/ch/jalu/configme/beanmapper/migratingbeanmapper/BeanExtensionTest.java b/src/test/java/ch/jalu/configme/beanmapper/migratingbeanmapper/BeanExtensionTest.java index 7d5f20eb..c7aeb76f 100644 --- a/src/test/java/ch/jalu/configme/beanmapper/migratingbeanmapper/BeanExtensionTest.java +++ b/src/test/java/ch/jalu/configme/beanmapper/migratingbeanmapper/BeanExtensionTest.java @@ -14,12 +14,12 @@ /** * Test for {@link SingleValueToCollectionMapper}. */ -public class BeanExtensionTest { +class BeanExtensionTest { private static final String CONFIG_FILE = "/beanmapper/nested_chat_component.yml"; @Test - public void shouldLoadFileAndConvertSingleValuesToCollection() { + void shouldLoadFileAndConvertSingleValuesToCollection() { // given SettingsManager settingsManager = SettingsManagerBuilder .withYamlFile(TestUtils.getJarPath(CONFIG_FILE)) diff --git a/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescriptionImplTest.java b/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescriptionImplTest.java index dd114a6f..af7f07c3 100644 --- a/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescriptionImplTest.java +++ b/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescriptionImplTest.java @@ -13,10 +13,10 @@ /** * Test for {@link BeanPropertyDescriptionImpl}. */ -public class BeanPropertyDescriptionImplTest { +class BeanPropertyDescriptionImplTest { @Test - void shouldGetAndSetProperties() { + void shouldGetProperties() { // given BeanPropertyDescription sizeProperty = getDescriptor("size", SampleBean.class); SampleBean bean = new SampleBean(); @@ -24,24 +24,9 @@ void shouldGetAndSetProperties() { // when Object result1 = sizeProperty.getValue(bean); - sizeProperty.setValue(bean, -120); - Object result2 = sizeProperty.getValue(bean); // then - assertThat(bean.getSize(), equalTo(-120)); - assertThat(77, equalTo(result1)); - assertThat(-120, equalTo(result2)); - } - - @Test - void shouldHandlePropertySetError() { - // given - BeanPropertyDescription sizeProperty = getDescriptor("size", SampleBean.class); - SampleBean bean = new ThrowingBean(); - - // when / then - assertThrows(ConfigMeMapperException.class, - () -> sizeProperty.setValue(bean, -120)); + assertThat(result1, equalTo(77)); } @Test From 1eaa86fb3a88058351ded83de227be253b33b30d Mon Sep 17 00:00:00 2001 From: ljacqu Date: Tue, 14 Nov 2023 16:55:01 +0100 Subject: [PATCH 03/16] Add support for ignore annotation / shuffle things around -- WIP --- pom.xml | 2 +- .../ch/jalu/configme/beanmapper/Ignore.java | 11 + .../jalu/configme/beanmapper/MapperImpl.java | 26 ++- .../instantiation/BeanInspector.java | 140 +---------- .../BeanZeroArgConstrInstantiation.java | 63 +++++ .../instantiation/RecordInstantiation.java | 48 ++++ .../BeanDescriptionFactory.java | 13 +- .../BeanDescriptionFactoryImpl.java | 217 +++--------------- .../BeanPropertyDescriptionImpl.java | 2 +- .../propertydescription/FieldProperty.java | 12 +- .../BeanWithCustomLeafTypeTest.java | 6 +- .../configme/beanmapper/MapperImplTest.java | 10 +- .../MapperTypeInfoWithNoClassEquivTest.java | 4 +- .../BeanDescriptionFactoryImplTest.java | 178 ++------------ .../BeanPropertyDescriptionImplTest.java | 2 +- .../ch/jalu/configme/demo/beans/Country.java | 2 - .../ch/jalu/configme/demo/beans/UserBase.java | 7 +- .../configme/samples/inheritance/Child.java | 21 +- .../configme/samples/inheritance/Middle.java | 15 -- .../configme/samples/inheritance/Parent.java | 18 -- 20 files changed, 235 insertions(+), 562 deletions(-) create mode 100644 src/main/java/ch/jalu/configme/beanmapper/Ignore.java create mode 100644 src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstrInstantiation.java create mode 100644 src/main/java/ch/jalu/configme/beanmapper/instantiation/RecordInstantiation.java diff --git a/pom.xml b/pom.xml index 29e399b8..c43cf6e5 100644 --- a/pom.xml +++ b/pom.xml @@ -193,7 +193,7 @@ ch.jalu typeresolver - 0.1.0 + 0.2.0-SNAPSHOT diff --git a/src/main/java/ch/jalu/configme/beanmapper/Ignore.java b/src/main/java/ch/jalu/configme/beanmapper/Ignore.java new file mode 100644 index 00000000..ab2aef59 --- /dev/null +++ b/src/main/java/ch/jalu/configme/beanmapper/Ignore.java @@ -0,0 +1,11 @@ +package ch.jalu.configme.beanmapper; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Retention(RetentionPolicy.RUNTIME) +@Target(ElementType.FIELD) +public @interface Ignore { +} diff --git a/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java b/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java index b57d30cd..6404f708 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java +++ b/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java @@ -9,8 +9,6 @@ import ch.jalu.configme.beanmapper.leafvaluehandler.LeafValueHandler; import ch.jalu.configme.beanmapper.leafvaluehandler.LeafValueHandlerImpl; import ch.jalu.configme.beanmapper.leafvaluehandler.MapperLeafType; -import ch.jalu.configme.beanmapper.propertydescription.BeanDescriptionFactory; -import ch.jalu.configme.beanmapper.propertydescription.BeanDescriptionFactoryImpl; import ch.jalu.configme.beanmapper.propertydescription.BeanPropertyComments; import ch.jalu.configme.beanmapper.propertydescription.BeanPropertyDescription; import ch.jalu.configme.properties.convertresult.ConvertErrorRecorder; @@ -21,6 +19,7 @@ import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; @@ -71,23 +70,22 @@ public class MapperImpl implements Mapper { // Fields and general configurable methods // --------- - private final BeanDescriptionFactory beanDescriptionFactory; private final LeafValueHandler leafValueHandler; - private final BeanInspector beanInspector = new BeanInspector(); // TODO: Make configurable, add interface... + private final BeanInspector beanInspector; public MapperImpl() { - this(new BeanDescriptionFactoryImpl(), + this(new BeanInspector(), new LeafValueHandlerImpl(LeafValueHandlerImpl.createDefaultLeafTypes())); } - public MapperImpl(@NotNull BeanDescriptionFactory beanDescriptionFactory, + public MapperImpl(@NotNull BeanInspector beanInspector, @NotNull LeafValueHandler leafValueHandler) { - this.beanDescriptionFactory = beanDescriptionFactory; + this.beanInspector = beanInspector; this.leafValueHandler = leafValueHandler; } - protected final @NotNull BeanDescriptionFactory getBeanDescriptionFactory() { - return beanDescriptionFactory; + protected final @NotNull BeanInspector getBeanInspector() { + return beanInspector; } protected final @NotNull LeafValueHandler getLeafValueHandler() { @@ -135,8 +133,7 @@ public MapperImpl(@NotNull BeanDescriptionFactory beanDescriptionFactory, // Step 3: treat as bean Map mappedBean = new LinkedHashMap<>(); - // TODO: Adapt me - for (BeanPropertyDescription property : beanDescriptionFactory.getAllProperties(value.getClass())) { + for (BeanPropertyDescription property : getBeanProperties(value)) { Object exportValueOfProperty = toExportValue(property.getValue(value), exportContext); if (exportValueOfProperty != null) { BeanPropertyComments propComments = property.getComments(); @@ -151,6 +148,13 @@ public MapperImpl(@NotNull BeanDescriptionFactory beanDescriptionFactory, return mappedBean; } + @NotNull + private List getBeanProperties(@NotNull Object value) { + return beanInspector.findInstantiation(value.getClass()) + .map(BeanInstantiation::getProperties) + .orElse(Collections.emptyList()); + } + /** * Handles values of types which need special handling (such as Optional). Null means the value is not * a special type and that the export value should be built differently. Use {@link #RETURN_NULL} to diff --git a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInspector.java b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInspector.java index 84dfb530..5b97f814 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInspector.java +++ b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInspector.java @@ -2,26 +2,15 @@ import ch.jalu.configme.beanmapper.propertydescription.BeanDescriptionFactory; import ch.jalu.configme.beanmapper.propertydescription.BeanDescriptionFactoryImpl; -import ch.jalu.configme.beanmapper.propertydescription.BeanPropertyComments; -import ch.jalu.configme.beanmapper.propertydescription.BeanPropertyDescription; import ch.jalu.configme.beanmapper.propertydescription.FieldProperty; -import ch.jalu.configme.exception.ConfigMeException; import ch.jalu.configme.internal.record.RecordComponent; import ch.jalu.configme.internal.record.RecordInspector; import ch.jalu.configme.internal.record.ReflectionHelper; -import ch.jalu.configme.properties.convertresult.ConvertErrorRecorder; -import ch.jalu.typeresolver.TypeInfo; import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; import java.lang.reflect.Constructor; -import java.util.Arrays; -import java.util.Collections; -import java.util.Iterator; import java.util.List; -import java.util.Objects; import java.util.Optional; -import java.util.stream.Collectors; public class BeanInspector { @@ -31,140 +20,27 @@ public class BeanInspector { public @NotNull Optional findInstantiation(@NotNull Class clazz) { if (recordInspector.isRecord(clazz)) { RecordComponent[] recordComponents = recordInspector.getRecordComponents(clazz); - return Optional.of(new RecordInstantiation(clazz, Arrays.asList(recordComponents))); + List properties = + beanDescriptionFactory.createRecordProperties(clazz, recordComponents); + + return Optional.of(new RecordInstantiation(clazz, properties)); } Optional> zeroArgConstructor = getConstructor(clazz); if (zeroArgConstructor.isPresent()) { - List properties = beanDescriptionFactory.getAllProperties2(clazz); - return Optional.of(new BeanNoArgConstructor(zeroArgConstructor.get(), properties)); + List properties = beanDescriptionFactory.getAllProperties(clazz); + return Optional.of(new BeanZeroArgConstrInstantiation(zeroArgConstructor.get(), properties)); } return Optional.empty(); } - private static @NotNull Optional> getConstructor(@NotNull Class declarer, - Class @NotNull ... parameters) { + static @NotNull Optional> getConstructor(@NotNull Class declarer, + Class @NotNull ... parameters) { try { return Optional.of(declarer.getDeclaredConstructor(parameters)); } catch (NoSuchMethodException ignore) { return Optional.empty(); } } - - private static final class BeanNoArgConstructor implements BeanInstantiation { - - private final Constructor zeroArgsConstructor; - private final List properties; - - private BeanNoArgConstructor(@NotNull Constructor zeroArgsConstructor, - @NotNull List properties) { - this.zeroArgsConstructor = zeroArgsConstructor; - this.properties = properties; - } - - @Override - public @NotNull List getProperties() { - return Collections.unmodifiableList(properties); - } - - @Override - public @Nullable Object create(@NotNull List propertyValues, - @NotNull ConvertErrorRecorder errorRecorder) { - final Object bean; - try { - bean = zeroArgsConstructor.newInstance(); - } catch (ReflectiveOperationException e) { - throw new ConfigMeException("Failed to call constructor for " - + zeroArgsConstructor.getDeclaringClass()); - } - - if (propertyValues.size() != properties.size()) { - throw new ConfigMeException("Invalid property values, " + propertyValues.size() + " were given, but " - + zeroArgsConstructor.getDeclaringClass() + " has " + properties.size() + " properties"); - } - - Iterator propIt = properties.iterator(); - Iterator valuesIt = propertyValues.iterator(); - while (propIt.hasNext() && valuesIt.hasNext()) { - FieldProperty property = propIt.next(); - Object value = valuesIt.next(); - if (value == null) { - if (property.getValue(bean) == null) { - return null; // No default value on field, return null -> no bean with a null value - } - errorRecorder.setHasError("Fallback to default value"); - } else { - property.setValue(bean, value); - } - } - return bean; - } - } - - private static final class RecordInstantiation implements BeanInstantiation { - - private final Constructor canonicalConstructor; - private final List properties; - - public RecordInstantiation(@NotNull Class clazz, @NotNull List components) { - this.properties = components.stream() - .map(RecordProperty::new) - .collect(Collectors.toList()); - Class[] recordTypes = components.stream().map(RecordComponent::getType).toArray(Class[]::new); - this.canonicalConstructor = getConstructor(clazz, recordTypes) - .orElseThrow(() -> new ConfigMeException("Could not get canonical constructor of " + clazz)); - } - - @Override - public @NotNull List getProperties() { - return properties; - } - - @Override - public @Nullable Object create(@NotNull List propertyValues, - @NotNull ConvertErrorRecorder errorRecorder) { - if (propertyValues.stream().anyMatch(Objects::isNull)) { - return null; // No support for null values in records - } - - Object[] properties = propertyValues.toArray(); - try { - return canonicalConstructor.newInstance(properties); - } catch (IllegalArgumentException | ReflectiveOperationException e) { - // TODO: Separate clause for InvocationTargetException? - throw new ConfigMeException("Error calling record constructor for " - + canonicalConstructor.getDeclaringClass(), e); - } - } - } - - private static final class RecordProperty implements BeanPropertyDescription { - - private final RecordComponent recordComponent; - - private RecordProperty(@NotNull RecordComponent recordComponent) { - this.recordComponent = recordComponent; - } - - @Override - public @NotNull String getName() { - return recordComponent.getName(); - } - - @Override - public @NotNull TypeInfo getTypeInformation() { - return TypeInfo.of(recordComponent.getGenericType()); - } - - @Override - public @Nullable Object getValue(@NotNull Object bean) { - throw new UnsupportedOperationException(); // TODO - } - - @Override - public @NotNull BeanPropertyComments getComments() { - return BeanPropertyComments.EMPTY; // TODO - } - } } diff --git a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstrInstantiation.java b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstrInstantiation.java new file mode 100644 index 00000000..5c59847a --- /dev/null +++ b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstrInstantiation.java @@ -0,0 +1,63 @@ +package ch.jalu.configme.beanmapper.instantiation; + +import ch.jalu.configme.beanmapper.propertydescription.BeanPropertyDescription; +import ch.jalu.configme.beanmapper.propertydescription.FieldProperty; +import ch.jalu.configme.exception.ConfigMeException; +import ch.jalu.configme.properties.convertresult.ConvertErrorRecorder; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.lang.reflect.Constructor; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; + +final class BeanZeroArgConstrInstantiation implements BeanInstantiation { + + private final Constructor zeroArgsConstructor; + private final List properties; + + public BeanZeroArgConstrInstantiation(@NotNull Constructor zeroArgsConstructor, + @NotNull List properties) { + this.zeroArgsConstructor = zeroArgsConstructor; + this.properties = properties; + } + + @Override + public @NotNull List getProperties() { + return Collections.unmodifiableList(properties); + } + + @Override + public @Nullable Object create(@NotNull List propertyValues, + @NotNull ConvertErrorRecorder errorRecorder) { + final Object bean; + try { + bean = zeroArgsConstructor.newInstance(); + } catch (ReflectiveOperationException e) { + throw new ConfigMeException("Failed to call constructor for " + + zeroArgsConstructor.getDeclaringClass()); + } + + if (propertyValues.size() != properties.size()) { + throw new ConfigMeException("Invalid property values, " + propertyValues.size() + " were given, but " + + zeroArgsConstructor.getDeclaringClass() + " has " + properties.size() + " properties"); + } + + Iterator propIt = properties.iterator(); + Iterator valuesIt = propertyValues.iterator(); + while (propIt.hasNext() && valuesIt.hasNext()) { + FieldProperty property = propIt.next(); + Object value = valuesIt.next(); + if (value == null) { + if (property.getValue(bean) == null) { + return null; // No default value on field, return null -> no bean with a null value + } + errorRecorder.setHasError("Fallback to default value"); + } else { + property.setValue(bean, value); + } + } + return bean; + } +} diff --git a/src/main/java/ch/jalu/configme/beanmapper/instantiation/RecordInstantiation.java b/src/main/java/ch/jalu/configme/beanmapper/instantiation/RecordInstantiation.java new file mode 100644 index 00000000..4609b831 --- /dev/null +++ b/src/main/java/ch/jalu/configme/beanmapper/instantiation/RecordInstantiation.java @@ -0,0 +1,48 @@ +package ch.jalu.configme.beanmapper.instantiation; + +import ch.jalu.configme.beanmapper.propertydescription.BeanPropertyDescription; +import ch.jalu.configme.beanmapper.propertydescription.FieldProperty; +import ch.jalu.configme.exception.ConfigMeException; +import ch.jalu.configme.properties.convertresult.ConvertErrorRecorder; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.lang.reflect.Constructor; +import java.util.Collections; +import java.util.List; +import java.util.Objects; + +final class RecordInstantiation implements BeanInstantiation { + + private final Constructor canonicalConstructor; + private final List properties; + + public RecordInstantiation(@NotNull Class clazz, @NotNull List properties) { + this.properties = properties; + Class[] recordTypes = properties.stream().map(FieldProperty::getType).toArray(Class[]::new); + this.canonicalConstructor = BeanInspector.getConstructor(clazz, recordTypes) + .orElseThrow(() -> new ConfigMeException("Could not get canonical constructor of " + clazz)); + } + + @Override + public @NotNull List getProperties() { + return Collections.unmodifiableList(properties); + } + + @Override + public @Nullable Object create(@NotNull List propertyValues, + @NotNull ConvertErrorRecorder errorRecorder) { + if (propertyValues.stream().anyMatch(Objects::isNull)) { + return null; // No support for null values in records + } + + Object[] properties = propertyValues.toArray(); + try { + return canonicalConstructor.newInstance(properties); + } catch (IllegalArgumentException | ReflectiveOperationException e) { + // TODO: Separate clause for InvocationTargetException? + throw new ConfigMeException("Error calling record constructor for " + + canonicalConstructor.getDeclaringClass(), e); + } + } +} diff --git a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactory.java b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactory.java index ac5e0a07..5819aaf6 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactory.java +++ b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactory.java @@ -1,5 +1,6 @@ package ch.jalu.configme.beanmapper.propertydescription; +import ch.jalu.configme.internal.record.RecordComponent; import org.jetbrains.annotations.NotNull; import java.util.List; @@ -11,15 +12,9 @@ */ public interface BeanDescriptionFactory { - /** - * Returns all properties on the given class which should be considered while creating a bean of the - * given type. This is usually all properties which can be read from and written to. - * - * @param clazz the class whose properties should be returned - * @return the relevant properties on the class - */ - @NotNull List getAllProperties(@NotNull Class clazz); + @NotNull List getAllProperties(@NotNull Class clazz); - @NotNull List getAllProperties2(@NotNull Class clazz); + @NotNull List createRecordProperties(@NotNull Class clazz, + RecordComponent @NotNull [] components); } diff --git a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImpl.java b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImpl.java index 3474ac21..8e185123 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImpl.java +++ b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImpl.java @@ -3,22 +3,21 @@ import ch.jalu.configme.Comment; import ch.jalu.configme.beanmapper.ConfigMeMapperException; import ch.jalu.configme.beanmapper.ExportName; -import ch.jalu.typeresolver.TypeInfo; +import ch.jalu.configme.beanmapper.Ignore; +import ch.jalu.configme.exception.ConfigMeException; +import ch.jalu.configme.internal.record.RecordComponent; +import ch.jalu.typeresolver.FieldUtils; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import java.beans.IntrospectionException; -import java.beans.Introspector; -import java.beans.PropertyDescriptor; import java.lang.reflect.Field; import java.lang.reflect.Modifier; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; -import java.util.Collections; -import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Objects; @@ -32,46 +31,40 @@ * This description factory returns property descriptions for all properties on a class * for which a getter and setter is associated. Inherited properties are considered. *

- * This implementation supports {@link ExportName} and transient properties, declared either - * with the {@code transient} keyword or by adding the {@link java.beans.Transient} annotation. + * This implementation supports {@link ExportName} and transient properties, declared + * with the {@code transient} keyword or by adding the {@link Ignore} annotation. */ public class BeanDescriptionFactoryImpl implements BeanDescriptionFactory { - private final Map, List> classProperties = new HashMap<>(); + private final Map, List> classProperties = new HashMap<>(); - /** - * Returns all properties of the given bean class for which there exists a getter and setter. - * - * @param clazz the bean property to process - * @return the bean class's properties to handle - */ @Override - public @NotNull List getAllProperties(@NotNull Class clazz) { + public @NotNull List getAllProperties(@NotNull Class clazz) { return classProperties.computeIfAbsent(clazz, this::collectAllProperties); } - @Override - public @NotNull List getAllProperties2(@NotNull Class clazz) { - // TODO: Caching. Here, or elsewhere? - return collectAllProperties2(clazz); - } - - /** - * Collects all properties available on the given class. - * - * @param clazz the class to process - * @return properties of the class - */ - protected @NotNull List collectAllProperties(@NotNull Class clazz) { - List descriptors = getWritableProperties(clazz); - - List properties = descriptors.stream() - .map(this::convert) - .filter(Objects::nonNull) - .collect(Collectors.toList()); + public @NotNull List createRecordProperties(@NotNull Class clazz, + RecordComponent @NotNull [] components) { + // TODO: Caching here. + LinkedHashMap instanceFieldsByName = FieldUtils.getAllFields(clazz) + .filter(FieldUtils::isRegularInstanceField) + .collect(FieldUtils.collectByName(false)); - validateProperties(clazz, properties); - return properties; + List relevantFields = new ArrayList<>(); + for (RecordComponent component : components) { + Field field = instanceFieldsByName.get(component.getName()); + if (field == null) { + throw new ConfigMeException("Record component '" + component.getName() + "' for " + clazz.getName() + + " does not have a field with the same name"); + } + FieldProperty property = convert(field); + if (property == null) { + throw new ConfigMeException("Record component '" + component.getName() + "' for " + clazz.getName() + + " has a field defined to be ignored: this is not supported for records"); + } + relevantFields.add(property); + } + return relevantFields; } /** @@ -80,14 +73,13 @@ public class BeanDescriptionFactoryImpl implements BeanDescriptionFactory { * @param clazz the class to process * @return properties of the class */ - protected @NotNull List collectAllProperties2(@NotNull Class clazz) { - List> parentsAndClass = collectClassAndAllParents(clazz); + protected @NotNull List collectAllProperties(@NotNull Class clazz) { + LinkedHashMap instanceFieldsByName = FieldUtils.getAllFields(clazz) + .filter(FieldUtils::isRegularInstanceField) + .collect(FieldUtils.collectByName(false)); - // TODO: Use field utils - List properties = parentsAndClass.stream() - .flatMap(clz -> Arrays.stream(clz.getDeclaredFields())) - .filter(field -> !Modifier.isStatic(field.getModifiers()) && !field.isSynthetic()) - .map(this::convert2) + List properties = instanceFieldsByName.values().stream() + .map(this::convert) .filter(Objects::nonNull) .collect(Collectors.toList()); @@ -95,30 +87,11 @@ public class BeanDescriptionFactoryImpl implements BeanDescriptionFactory { return properties; } - /** - * Converts a {@link PropertyDescriptor} to a {@link BeanPropertyDescription} object. - * - * @param descriptor the descriptor to convert - * @return the converted object, or null if the property should be skipped - */ - protected @Nullable BeanPropertyDescription convert(@NotNull PropertyDescriptor descriptor) { - if (Boolean.TRUE.equals(descriptor.getValue("transient"))) { + protected @Nullable FieldProperty convert(@NotNull Field field) { + if (Modifier.isTransient(field.getModifiers()) || field.isAnnotationPresent(Ignore.class)) { return null; } - Field field = tryGetField(descriptor.getWriteMethod().getDeclaringClass(), descriptor.getName()); - BeanPropertyComments comments = getComments(field); - return new BeanPropertyDescriptionImpl( - getPropertyName(descriptor, field), - createTypeInfo(descriptor), - descriptor.getReadMethod(), - descriptor.getWriteMethod(), - comments); - } - - protected @Nullable FieldProperty convert2(@NotNull Field field) { - // TODO: Annotation to ignore field - return new FieldProperty(field, getCustomExportName(field), getComments(field)); @@ -140,21 +113,6 @@ public class BeanDescriptionFactoryImpl implements BeanDescriptionFactory { return BeanPropertyComments.EMPTY; } - /** - * Returns the field with the given name on the provided class, or null if it doesn't exist. - * - * @param clazz the class to search in - * @param name the field name to look for - * @return the field if matched, otherwise null - */ - protected @Nullable Field tryGetField(@NotNull Class clazz, @NotNull String name) { - try { - return clazz.getDeclaredField(name); - } catch (NoSuchFieldException ignore) { - } - return null; - } - /** * Validates the class's properties. * @@ -175,108 +133,9 @@ protected void validateProperties(@NotNull Class clazz, }); } - /** - * Returns the name which is used in the export files for the given property descriptor. - * - * @param descriptor the descriptor to get the name for - * @param field the field associated with the property (may be null) - * @return the property name - */ - protected @NotNull String getPropertyName(@NotNull PropertyDescriptor descriptor, @Nullable Field field) { - if (field != null && field.isAnnotationPresent(ExportName.class)) { - return field.getAnnotation(ExportName.class).value(); - } - return descriptor.getName(); - } - protected @Nullable String getCustomExportName(@NotNull Field field) { return field.isAnnotationPresent(ExportName.class) ? field.getAnnotation(ExportName.class).value() : null; } - - protected @NotNull TypeInfo createTypeInfo(@NotNull PropertyDescriptor descriptor) { - return new TypeInfo(descriptor.getWriteMethod().getGenericParameterTypes()[0]); - } - - /** - * Returns all properties of the given class that are writable - * (all bean properties with an associated read and write method). - * - * @param clazz the class to process - * @return all writable properties of the bean class - */ - protected @NotNull List getWritableProperties(@NotNull Class clazz) { - PropertyDescriptor[] descriptors; - try { - descriptors = Introspector.getBeanInfo(clazz).getPropertyDescriptors(); - } catch (IntrospectionException e) { - throw new IllegalStateException(e); - } - List writableProperties = new ArrayList<>(descriptors.length); - for (PropertyDescriptor descriptor : descriptors) { - if (descriptor.getWriteMethod() != null && descriptor.getReadMethod() != null) { - writableProperties.add(descriptor); - } - } - return sortPropertiesList(clazz, writableProperties); - } - - /** - * Returns a sorted list of the given properties which will be used for further processing and whose - * order will be maintained. May return the same list. - * - * @param clazz the class from which the properties come from - * @param properties the properties to sort - * @return sorted properties - */ - protected @NotNull List sortPropertiesList(@NotNull Class clazz, - @NotNull List properties) { - Map fieldNameByIndex = createFieldNameOrderMap(clazz); - int maxIndex = fieldNameByIndex.size(); - - properties.sort(Comparator.comparing(property -> { - Integer index = fieldNameByIndex.get(property.getName()); - return index == null ? maxIndex : index; - })); - return properties; - } - - /** - * Creates a map of index (encounter number) by field name for all fields of the given class, - * including its superclasses. Fields are sorted by declaration order in the classes; sorted - * by top-most class in the inheritance hierarchy to the lowest (the class provided as parameter). - * - * @param clazz the class to create the field index map for - * @return map with all field names as keys and its index as value - */ - protected @NotNull Map createFieldNameOrderMap(@NotNull Class clazz) { - Map nameByIndex = new HashMap<>(); - int i = 0; - for (Class currentClass : collectClassAndAllParents(clazz)) { - for (Field field : currentClass.getDeclaredFields()) { - nameByIndex.put(field.getName(), i); - ++i; - } - } - return nameByIndex; - } - - /** - * Returns a list of the class's parents, including the given class itself, with the top-most parent - * coming first. Does not include the Object class. - * - * @param clazz the class whose parents should be collected - * @return list with all of the class's parents, sorted by highest class in the hierarchy to lowest - */ - protected @NotNull List> collectClassAndAllParents(@NotNull Class clazz) { - List> parents = new ArrayList<>(); - Class curClass = clazz; - while (curClass != null && curClass != Object.class) { - parents.add(curClass); - curClass = curClass.getSuperclass(); - } - Collections.reverse(parents); - return parents; - } } diff --git a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescriptionImpl.java b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescriptionImpl.java index e7ef0e7c..69258b02 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescriptionImpl.java +++ b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescriptionImpl.java @@ -11,7 +11,7 @@ /** * Default implementation of {@link BeanPropertyDescription}. */ -public class BeanPropertyDescriptionImpl implements BeanPropertyDescription { +public class BeanPropertyDescriptionImpl implements BeanPropertyDescription { // TODO: Remove me private final String name; private final TypeInfo typeInformation; diff --git a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/FieldProperty.java b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/FieldProperty.java index 3b632d31..1373b6a7 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/FieldProperty.java +++ b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/FieldProperty.java @@ -2,6 +2,7 @@ import ch.jalu.configme.beanmapper.ConfigMeMapperException; import ch.jalu.configme.exception.ConfigMeException; +import ch.jalu.typeresolver.FieldUtils; import ch.jalu.typeresolver.TypeInfo; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -34,6 +35,10 @@ public FieldProperty(@NotNull Field field, return TypeInfo.of(field); } + public @NotNull Class getType() { + return field.getType(); + } + public void setValue(@NotNull Object bean, @NotNull Object value) { if (!field.isAccessible()) { field.setAccessible(true); // todo: exception handling @@ -41,8 +46,8 @@ public void setValue(@NotNull Object bean, @NotNull Object value) { try { field.set(bean, value); } catch (IllegalArgumentException | IllegalAccessException e) { - // todo: Use field utils for field name - throw new ConfigMeMapperException("Failed to set value to field " + field + ". Value: " + value, e); + String fieldName = FieldUtils.formatField(field); + throw new ConfigMeMapperException("Failed to set value to field " + fieldName + ". Value: " + value, e); } } @@ -54,8 +59,7 @@ public void setValue(@NotNull Object bean, @NotNull Object value) { try { return field.get(bean); } catch (IllegalArgumentException | IllegalAccessException e) { - // TODO: use field utils for field name - throw new ConfigMeException("Failed to get value for field " + field, e); + throw new ConfigMeException("Failed to get value for field " + FieldUtils.formatField(field), e); } } diff --git a/src/test/java/ch/jalu/configme/beanmapper/BeanWithCustomLeafTypeTest.java b/src/test/java/ch/jalu/configme/beanmapper/BeanWithCustomLeafTypeTest.java index cc5523b3..39fcedfc 100644 --- a/src/test/java/ch/jalu/configme/beanmapper/BeanWithCustomLeafTypeTest.java +++ b/src/test/java/ch/jalu/configme/beanmapper/BeanWithCustomLeafTypeTest.java @@ -4,9 +4,9 @@ import ch.jalu.configme.SettingsManager; import ch.jalu.configme.SettingsManagerBuilder; import ch.jalu.configme.TestUtils; -import ch.jalu.configme.beanmapper.leafvaluehandler.MapperLeafType; +import ch.jalu.configme.beanmapper.instantiation.BeanInspector; import ch.jalu.configme.beanmapper.leafvaluehandler.LeafValueHandlerImpl; -import ch.jalu.configme.beanmapper.propertydescription.BeanDescriptionFactoryImpl; +import ch.jalu.configme.beanmapper.leafvaluehandler.MapperLeafType; import ch.jalu.configme.properties.BeanProperty; import ch.jalu.configme.properties.Property; import ch.jalu.configme.properties.convertresult.ConvertErrorRecorder; @@ -84,7 +84,7 @@ private MyTestSettings() { public static final class MapperWithCustomIntSupport extends MapperImpl { MapperWithCustomIntSupport() { - super(new BeanDescriptionFactoryImpl(), + super(new BeanInspector(), LeafValueHandlerImpl.builder().addDefaults().addType(new CustomIntegerLeafType()).build()); } } diff --git a/src/test/java/ch/jalu/configme/beanmapper/MapperImplTest.java b/src/test/java/ch/jalu/configme/beanmapper/MapperImplTest.java index 356584d4..63f6fa29 100644 --- a/src/test/java/ch/jalu/configme/beanmapper/MapperImplTest.java +++ b/src/test/java/ch/jalu/configme/beanmapper/MapperImplTest.java @@ -10,9 +10,9 @@ import ch.jalu.configme.beanmapper.command.optionalproperties.ComplexOptionalTypeConfig; import ch.jalu.configme.beanmapper.context.MappingContext; import ch.jalu.configme.beanmapper.context.MappingContextImpl; +import ch.jalu.configme.beanmapper.instantiation.BeanInspector; import ch.jalu.configme.beanmapper.leafvaluehandler.LeafValueHandler; import ch.jalu.configme.beanmapper.leafvaluehandler.LeafValueHandlerImpl; -import ch.jalu.configme.beanmapper.propertydescription.BeanDescriptionFactory; import ch.jalu.configme.beanmapper.typeissues.GenericCollection; import ch.jalu.configme.beanmapper.typeissues.MapWithNonStringKeys; import ch.jalu.configme.beanmapper.typeissues.UnsupportedCollection; @@ -453,16 +453,16 @@ void shouldForwardException() { @Test void shouldReturnFields() { // given - BeanDescriptionFactory descriptionFactory = mock(BeanDescriptionFactory.class); + BeanInspector beanInspector = mock(BeanInspector.class); LeafValueHandlerImpl leafValueHandler = mock(LeafValueHandlerImpl.class); - MapperImpl mapper = new MapperImpl(descriptionFactory, leafValueHandler); + MapperImpl mapper = new MapperImpl(beanInspector, leafValueHandler); // when - BeanDescriptionFactory returnedDescriptionFactory = mapper.getBeanDescriptionFactory(); + BeanInspector returnedBeanInspector = mapper.getBeanInspector(); LeafValueHandler returnedLeafValueHandler = mapper.getLeafValueHandler(); // then - assertThat(returnedDescriptionFactory, sameInstance(descriptionFactory)); + assertThat(returnedBeanInspector, sameInstance(beanInspector)); assertThat(returnedLeafValueHandler, sameInstance(leafValueHandler)); } diff --git a/src/test/java/ch/jalu/configme/beanmapper/MapperTypeInfoWithNoClassEquivTest.java b/src/test/java/ch/jalu/configme/beanmapper/MapperTypeInfoWithNoClassEquivTest.java index 33011a83..bca4a4ca 100644 --- a/src/test/java/ch/jalu/configme/beanmapper/MapperTypeInfoWithNoClassEquivTest.java +++ b/src/test/java/ch/jalu/configme/beanmapper/MapperTypeInfoWithNoClassEquivTest.java @@ -2,9 +2,9 @@ import ch.jalu.configme.beanmapper.context.MappingContext; import ch.jalu.configme.beanmapper.context.MappingContextImpl; +import ch.jalu.configme.beanmapper.instantiation.BeanInspector; import ch.jalu.configme.beanmapper.leafvaluehandler.LeafValueHandlerImpl; import ch.jalu.configme.beanmapper.leafvaluehandler.MapperLeafType; -import ch.jalu.configme.beanmapper.propertydescription.BeanDescriptionFactoryImpl; import ch.jalu.configme.exception.ConfigMeException; import ch.jalu.configme.properties.convertresult.ConvertErrorRecorder; import ch.jalu.typeresolver.TypeInfo; @@ -43,7 +43,7 @@ void shouldNotThrowForTypeInfoWithNoClassEquivalentTooEarly() { .addDefaults() .addType(extNumberLeafType) .build(); - MapperImpl mapper = new MapperImpl(new BeanDescriptionFactoryImpl(), leafValueHandler); + MapperImpl mapper = new MapperImpl(new BeanInspector(), leafValueHandler); // when Object result = mapper.convertToBean(3.2, targetType, errorRecorder); diff --git a/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImplTest.java b/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImplTest.java index df496a3b..ff6956cc 100644 --- a/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImplTest.java +++ b/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImplTest.java @@ -14,7 +14,6 @@ import ch.jalu.typeresolver.TypeInfo; import org.junit.jupiter.api.Test; -import java.beans.Transient; import java.util.ArrayList; import java.util.Collection; import java.util.List; @@ -23,7 +22,6 @@ import static ch.jalu.configme.TestUtils.transform; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.contains; -import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; @@ -42,10 +40,10 @@ class BeanDescriptionFactoryImplTest { @Test void shouldReturnWritableProperties() { // given / when - Collection descriptions = factory.getAllProperties(SampleBean.class); + Collection descriptions = factory.getAllProperties(SampleBean.class); // then - assertThat(descriptions, hasSize(2)); + assertThat(descriptions, hasSize(4)); BeanPropertyDescription sizeProperty = getDescription("size", descriptions); assertThat(sizeProperty.getTypeInformation(), equalTo(new TypeInfo(int.class))); @@ -54,6 +52,14 @@ void shouldReturnWritableProperties() { BeanPropertyDescription nameProperty = getDescription("name", descriptions); assertThat(nameProperty.getTypeInformation(), equalTo(new TypeInfo(String.class))); assertThat(nameProperty.getComments(), sameInstance(BeanPropertyComments.EMPTY)); + + BeanPropertyDescription longFieldProperty = getDescription("longField", descriptions); + assertThat(longFieldProperty.getTypeInformation(), equalTo(new TypeInfo(long.class))); + assertThat(longFieldProperty.getComments(), sameInstance(BeanPropertyComments.EMPTY)); + + BeanPropertyDescription uuidProperty = getDescription("uuid", descriptions); + assertThat(uuidProperty.getTypeInformation(), equalTo(new TypeInfo(UUID.class))); + assertThat(uuidProperty.getComments(), sameInstance(BeanPropertyComments.EMPTY)); } @Test @@ -62,35 +68,20 @@ void shouldReturnEmptyListForNonBeanClass() { assertThat(factory.getAllProperties(List.class), empty()); } - @Test - void shouldHandleBooleanMethodsAndMatchWithFields() { - // given / when - List properties = new ArrayList<>(factory.getAllProperties(BooleanTestBean.class)); - - // then - assertThat(properties, hasSize(4)); - assertThat(transform(properties, BeanPropertyDescription::getName), - containsInAnyOrder("active", "isField", "empty", "isNotMatched")); - - // First two elements can be mapped to fields, so check their order. For the two unknown ones, we don't make any guarantees - assertThat(properties.get(0).getName(), equalTo("active")); - assertThat(properties.get(1).getName(), equalTo("isField")); - } - @Test void shouldNotConsiderTransientFields() { // given / when - Collection properties = factory.getAllProperties(BeanWithTransientFields.class); + Collection properties = factory.getAllProperties(BeanWithTransientFields.class); // then assertThat(properties, hasSize(2)); - assertThat(transform(properties, BeanPropertyDescription::getName), contains("name", "mandatory")); + assertThat(transform(properties, BeanPropertyDescription::getName), contains("name", "isMandatory")); } @Test void shouldBeAwareOfInheritanceAndRespectOrder() { // given / when - Collection properties = factory.getAllProperties(Middle.class); + Collection properties = factory.getAllProperties(Middle.class); // then assertThat(properties, hasSize(3)); @@ -100,7 +91,7 @@ void shouldBeAwareOfInheritanceAndRespectOrder() { @Test void shouldLetChildFieldsOverrideParentFields() { // given / when - Collection properties = factory.getAllProperties(Child.class); + Collection properties = factory.getAllProperties(Child.class); // then assertThat(properties, hasSize(5)); @@ -111,7 +102,7 @@ void shouldLetChildFieldsOverrideParentFields() { @Test void shouldUseExportName() { // given / when - Collection properties = factory.getAllProperties(AnnotatedEntry.class); + Collection properties = factory.getAllProperties(AnnotatedEntry.class); // then assertThat(properties, hasSize(2)); @@ -144,8 +135,8 @@ void shouldThrowForWhenExportNameIsNullForProperty() { @Test void shouldReturnCommentsWithUuidIfNotRepeatable() { // given / when - Collection sampleBeanProperties = factory.getAllProperties(SampleBean.class); - Collection sampleBeanProperties2 = factory.getAllProperties(SampleBean.class); + Collection sampleBeanProperties = factory.getAllProperties(SampleBean.class); + Collection sampleBeanProperties2 = factory.getAllProperties(SampleBean.class); // then BeanPropertyComments sizeComments = getDescription("size", sampleBeanProperties).getComments(); @@ -161,7 +152,7 @@ void shouldReturnCommentsWithUuidIfNotRepeatable() { @Test void shouldReturnCommentsWithoutUuid() { // given / when - Collection execDetailsProperties = factory.getAllProperties(ExecutionDetails.class); + Collection execDetailsProperties = factory.getAllProperties(ExecutionDetails.class); // then BeanPropertyComments executorComments = getDescription("executor", execDetailsProperties).getComments(); @@ -175,7 +166,7 @@ void shouldReturnCommentsWithoutUuid() { @Test void shouldPickUpCustomNameFromField() { // given / when - List properties = new ArrayList<>(factory.getAllProperties(BeanWithExportName.class)); + List properties = new ArrayList<>(factory.getAllProperties(BeanWithExportName.class)); // then assertThat(properties, hasSize(3)); @@ -190,7 +181,7 @@ void shouldPickUpCustomNameFromField() { @Test void shouldPickUpCustomNameFromFieldsIncludingInheritance() { // given / when - List properties = new ArrayList<>(factory.getAllProperties(BeanWithExportNameExtension.class)); + List properties = new ArrayList<>(factory.getAllProperties(BeanWithExportNameExtension.class)); // then assertThat(properties, hasSize(4)); @@ -205,7 +196,7 @@ void shouldPickUpCustomNameFromFieldsIncludingInheritance() { } private static BeanPropertyDescription getDescription(String name, - Collection descriptions) { + Collection descriptions) { for (BeanPropertyDescription description : descriptions) { if (name.equals(description.getName())) { return description; @@ -219,138 +210,17 @@ private static final class SampleBean { private String name; @Comment("Size of this entry (cm)") private int size; - private long longField; // static "getter" method - private UUID uuid = UUID.randomUUID(); // no setter - - public String getName() { - return name; - } - - public void setName(String name) { - this.name = name; - } - - public int getSize() { - return size; - } - - public void setSize(int size) { - this.size = size; - } - - public UUID getUuid() { - return uuid; - } - - public static long getLongField() { - // Method with normal getter name is static! - return 0; - } - - public void setLongField(long longField) { - this.longField = longField; - } - } - - private static final class BooleanTestBean { - private boolean isEmpty; - private Boolean isReference; - private boolean active; - private String isString; - private boolean isField; - private boolean notMatched; - - public boolean isEmpty() { - return isEmpty; - } - - public void setEmpty(boolean empty) { - isEmpty = empty; - } - - public Boolean isReference() { // "is" getter only supported for primitive boolean - return isReference; - } + private long longField; + private UUID uuid = UUID.randomUUID(); - public void setReference(Boolean isReference) { - this.isReference = isReference; - } - - public boolean isActive() { - return active; - } - - public void setActive(boolean active) { - this.active = active; - } - - public String isString() { // "is" only supported for boolean - return isString; - } - - public void setString(String isString) { - this.isString = isString; - } - - public boolean getIsField() { - return isField; - } - - public void setIsField(boolean field) { - this.isField = field; - } - - // ----------------- - // notMatched: creates a valid property "isNotMatched" picked up by the introspector, - // but we should not match this to the field `notMatched`. - // ----------------- - public boolean getIsNotMatched() { - return notMatched; - } - - public void setIsNotMatched(boolean notMatched) { - this.notMatched = notMatched; - } } private static final class BeanWithTransientFields { + private String name; private transient long tempId; private transient boolean isSaved; private boolean isMandatory; - public String getName() { - return name; - } - - public void setName(String name) { - this.name = name; - } - - public long getTempId() { - return tempId; - } - - @Transient - public void setTempId(long tempId) { - this.tempId = tempId; - } - - @Transient - public boolean isSaved() { - return isSaved; - } - - public void setSaved(boolean saved) { - isSaved = saved; - } - - public boolean isMandatory() { - return isMandatory; - } - - public void setMandatory(boolean mandatory) { - isMandatory = mandatory; - } } } diff --git a/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescriptionImplTest.java b/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescriptionImplTest.java index af7f07c3..17691ac2 100644 --- a/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescriptionImplTest.java +++ b/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescriptionImplTest.java @@ -43,7 +43,7 @@ void shouldHandlePropertyGetError() { @Test void shouldHaveAppropriateStringRepresentation() { // given - Collection properties = new BeanDescriptionFactoryImpl() + Collection properties = new BeanDescriptionFactoryImpl() .collectAllProperties(AnnotatedEntry.class); BeanPropertyDescription hasIdProperty = properties.stream() .filter(prop -> "has-id".equals(prop.getName())).findFirst().get(); diff --git a/src/test/java/ch/jalu/configme/demo/beans/Country.java b/src/test/java/ch/jalu/configme/demo/beans/Country.java index e041055c..e6239ca5 100644 --- a/src/test/java/ch/jalu/configme/demo/beans/Country.java +++ b/src/test/java/ch/jalu/configme/demo/beans/Country.java @@ -1,6 +1,5 @@ package ch.jalu.configme.demo.beans; -import java.beans.Transient; import java.util.List; /** @@ -37,7 +36,6 @@ public void setNeighbors(List neighbors) { this.neighbors = neighbors; } - @Transient public boolean isTemporary() { return isTemporary; } diff --git a/src/test/java/ch/jalu/configme/demo/beans/UserBase.java b/src/test/java/ch/jalu/configme/demo/beans/UserBase.java index fcdf537c..a5d32847 100644 --- a/src/test/java/ch/jalu/configme/demo/beans/UserBase.java +++ b/src/test/java/ch/jalu/configme/demo/beans/UserBase.java @@ -1,6 +1,6 @@ package ch.jalu.configme.demo.beans; -import java.beans.Transient; +import ch.jalu.configme.beanmapper.Ignore; /** * User base bean. @@ -11,6 +11,7 @@ public class UserBase { private User richie; private User lionel; private double version; + @Ignore private transient int build; public User getBobby() { @@ -45,10 +46,6 @@ public void setVersion(double version) { this.version = version; } - @Transient - public int getBuild() { - return build; - } public void setBuild(int build) { this.build = build; diff --git a/src/test/java/ch/jalu/configme/samples/inheritance/Child.java b/src/test/java/ch/jalu/configme/samples/inheritance/Child.java index 52010c90..f682e02a 100644 --- a/src/test/java/ch/jalu/configme/samples/inheritance/Child.java +++ b/src/test/java/ch/jalu/configme/samples/inheritance/Child.java @@ -1,30 +1,11 @@ package ch.jalu.configme.samples.inheritance; -import java.beans.Transient; - /** * Child class. */ public class Child extends Middle { private int importance; + private boolean temporary; - @Override - public boolean isTemporary() { - return super.isTemporary(); - } - - @Override - @Transient(false) - public void setTemporary(boolean temporary) { - super.setTemporary(temporary); - } - - public int getImportance() { - return importance; - } - - public void setImportance(int importance) { - this.importance = importance; - } } diff --git a/src/test/java/ch/jalu/configme/samples/inheritance/Middle.java b/src/test/java/ch/jalu/configme/samples/inheritance/Middle.java index c4a6688e..961bdccc 100644 --- a/src/test/java/ch/jalu/configme/samples/inheritance/Middle.java +++ b/src/test/java/ch/jalu/configme/samples/inheritance/Middle.java @@ -8,19 +8,4 @@ public class Middle extends Parent { private String name; private float ratio; - public String getName() { - return name; - } - - public void setName(String name) { - this.name = name; - } - - public float getRatio() { - return ratio; - } - - public void setRatio(float ratio) { - this.ratio = ratio; - } } diff --git a/src/test/java/ch/jalu/configme/samples/inheritance/Parent.java b/src/test/java/ch/jalu/configme/samples/inheritance/Parent.java index 1fcde387..3dc966a6 100644 --- a/src/test/java/ch/jalu/configme/samples/inheritance/Parent.java +++ b/src/test/java/ch/jalu/configme/samples/inheritance/Parent.java @@ -1,7 +1,5 @@ package ch.jalu.configme.samples.inheritance; -import java.beans.Transient; - /** * Parent class. */ @@ -10,20 +8,4 @@ public class Parent { private long id; private transient boolean temporary; - public long getId() { - return id; - } - - public void setId(long id) { - this.id = id; - } - - public boolean isTemporary() { - return temporary; - } - - @Transient - public void setTemporary(boolean temporary) { - this.temporary = temporary; - } } From 9094e23bc87fb71671d7a0c1fc4d354403b2d9e2 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Thu, 16 Nov 2023 13:30:01 +0100 Subject: [PATCH 04/16] Update -- --- .../BeanPropertyDescription.java | 5 +- .../BeanPropertyDescriptionImpl.java | 90 ------------------- .../propertydescription/FieldProperty.java | 16 ++-- .../ConfigurationDataBuilder.java | 26 +----- .../internal/record/ReflectionHelper.java | 17 ++++ .../BeanDescriptionFactoryImplTest.java | 2 +- ...onImplTest.java => FieldPropertyTest.java} | 20 +---- .../ConfigurationDataBuilderTest.java | 40 --------- .../internal/record/ReflectionHelperTest.java | 73 +++++++++++++++ 9 files changed, 110 insertions(+), 179 deletions(-) delete mode 100644 src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescriptionImpl.java rename src/test/java/ch/jalu/configme/beanmapper/propertydescription/{BeanPropertyDescriptionImplTest.java => FieldPropertyTest.java} (74%) diff --git a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescription.java b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescription.java index 56c8eff5..f5299c5d 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescription.java +++ b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescription.java @@ -5,10 +5,11 @@ import org.jetbrains.annotations.Nullable; /** - * Represents a property within a bean class, as used in {@link ch.jalu.configme.beanmapper.MapperImpl}. + * Represents a property within a bean class, as used in + * {@link ch.jalu.configme.beanmapper.instantiation.BeanInspector}. * There, for instance, there is a {@link BeanDescriptionFactory} field responsible for creating bean descriptions. *

- * Default implementation is {@link BeanPropertyDescriptionImpl}. + * Default implementation is {@link FieldProperty}. */ public interface BeanPropertyDescription { diff --git a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescriptionImpl.java b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescriptionImpl.java deleted file mode 100644 index 69258b02..00000000 --- a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescriptionImpl.java +++ /dev/null @@ -1,90 +0,0 @@ -package ch.jalu.configme.beanmapper.propertydescription; - -import ch.jalu.configme.beanmapper.ConfigMeMapperException; -import ch.jalu.typeresolver.TypeInfo; -import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; - -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; - -/** - * Default implementation of {@link BeanPropertyDescription}. - */ -public class BeanPropertyDescriptionImpl implements BeanPropertyDescription { // TODO: Remove me - - private final String name; - private final TypeInfo typeInformation; - private final Method getter; - private final Method setter; - private final BeanPropertyComments comments; - - /** - * Constructor. - * - * @param name name of the property in the export - * @param typeInformation type of the property - * @param getter getter for the property - * @param setter setter for the property - * @param comments the comments of the property - */ - public BeanPropertyDescriptionImpl(@NotNull String name, @NotNull TypeInfo typeInformation, - @NotNull Method getter, @NotNull Method setter, - @NotNull BeanPropertyComments comments) { - this.name = name; - this.typeInformation = typeInformation; - this.getter = getter; - this.setter = setter; - this.comments = comments; - } - - @Override - public @NotNull String getName() { - return name; - } - - @Override - public @NotNull TypeInfo getTypeInformation() { - return typeInformation; - } - - /** - * Returns the value of the property for the given bean. - * - * @param bean the bean to read the property from - * @return bean value - */ - public @Nullable Object getValue(@NotNull Object bean) { - try { - return getter.invoke(bean); - } catch (@NotNull IllegalAccessException | InvocationTargetException e) { - throw new ConfigMeMapperException( - "Could not get property '" + name + "' from instance '" + bean + "'", e); - } - } - - /** - * Sets the given property to the given value on the provided bean. - * - * @param bean the bean to modify - * @param value the value to set the property to - */ - public void setValue(@NotNull Object bean, @NotNull Object value) { - try { - setter.invoke(bean, value); - } catch (@NotNull IllegalAccessException | InvocationTargetException e) { - throw new ConfigMeMapperException( - "Could not set property '" + name + "' to value '" + value + "' on instance '" + bean + "'", e); - } - } - - @Override - public @NotNull BeanPropertyComments getComments() { - return comments; - } - - @Override - public @NotNull String toString() { - return "Bean property '" + name + "' with getter '" + getter + "'"; - } -} diff --git a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/FieldProperty.java b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/FieldProperty.java index 1373b6a7..431602e7 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/FieldProperty.java +++ b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/FieldProperty.java @@ -2,6 +2,7 @@ import ch.jalu.configme.beanmapper.ConfigMeMapperException; import ch.jalu.configme.exception.ConfigMeException; +import ch.jalu.configme.internal.record.ReflectionHelper; import ch.jalu.typeresolver.FieldUtils; import ch.jalu.typeresolver.TypeInfo; import org.jetbrains.annotations.NotNull; @@ -40,9 +41,8 @@ public FieldProperty(@NotNull Field field, } public void setValue(@NotNull Object bean, @NotNull Object value) { - if (!field.isAccessible()) { - field.setAccessible(true); // todo: exception handling - } + ReflectionHelper.setAccessibleIfNeeded(field); + try { field.set(bean, value); } catch (IllegalArgumentException | IllegalAccessException e) { @@ -53,9 +53,8 @@ public void setValue(@NotNull Object bean, @NotNull Object value) { @Override public @Nullable Object getValue(@NotNull Object bean) { - if (!field.isAccessible()) { - field.setAccessible(true); // todo: exception handling - } + ReflectionHelper.setAccessibleIfNeeded(field); + try { return field.get(bean); } catch (IllegalArgumentException | IllegalAccessException e) { @@ -67,4 +66,9 @@ public void setValue(@NotNull Object bean, @NotNull Object value) { public @NotNull BeanPropertyComments getComments() { return comments; } + + @Override + public @NotNull String toString() { + return "FieldProperty '" + getName() + "' for field '" + FieldUtils.formatField(field) + "'"; + } } diff --git a/src/main/java/ch/jalu/configme/configurationdata/ConfigurationDataBuilder.java b/src/main/java/ch/jalu/configme/configurationdata/ConfigurationDataBuilder.java index b1c52801..8074b197 100644 --- a/src/main/java/ch/jalu/configme/configurationdata/ConfigurationDataBuilder.java +++ b/src/main/java/ch/jalu/configme/configurationdata/ConfigurationDataBuilder.java @@ -3,6 +3,7 @@ import ch.jalu.configme.Comment; import ch.jalu.configme.SettingsHolder; import ch.jalu.configme.exception.ConfigMeException; +import ch.jalu.configme.internal.record.ReflectionHelper; import ch.jalu.configme.properties.Property; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -152,7 +153,7 @@ protected void setCommentForPropertyField(@NotNull Field field, @NotNull String protected @Nullable Property getPropertyField(@NotNull Field field) { if (Property.class.isAssignableFrom(field.getType()) && Modifier.isStatic(field.getModifiers())) { try { - setFieldAccessibleIfNeeded(field); + ReflectionHelper.setAccessibleIfNeeded(field); return (Property) field.get(null); } catch (IllegalAccessException e) { throw new ConfigMeException("Could not fetch field '" + field.getName() + "' from class '" @@ -162,27 +163,6 @@ protected void setCommentForPropertyField(@NotNull Field field, @NotNull String return null; } - /** - * Sets the Field object to be accessible if needed; this makes the code support constants in Kotlin without the - * {@code @JvmField} annotation. This method only calls {@link Field#setAccessible} if it is needed to avoid - * potential issues with a security manager. Within Java itself, only {@code public static final} property fields - * are expected. - * - * @param field the field to process - */ - protected void setFieldAccessibleIfNeeded(@NotNull Field field) { - try { - if (!Modifier.isPublic(field.getModifiers())) { - field.setAccessible(true); - } - // CHECKSTYLE:OFF - } catch (Exception e) { - // CHECKSTYLE: ON - throw new ConfigMeException("Failed to modify access for field '" + field.getName() + "' from class '" - + field.getDeclaringClass().getSimpleName() + "'", e); - } - } - protected void collectSectionComments(@NotNull Class clazz) { SettingsHolder settingsHolder = createSettingsHolderInstance(clazz); settingsHolder.registerComments(commentsConfiguration); @@ -198,7 +178,7 @@ protected void collectSectionComments(@NotNull Class c protected @NotNull T createSettingsHolderInstance(@NotNull Class clazz) { try { Constructor constructor = clazz.getDeclaredConstructor(); - constructor.setAccessible(true); + ReflectionHelper.setAccessibleIfNeeded(constructor); return constructor.newInstance(); } catch (NoSuchMethodException e) { throw new ConfigMeException("Expected no-args constructor to be available for " + clazz, e); diff --git a/src/main/java/ch/jalu/configme/internal/record/ReflectionHelper.java b/src/main/java/ch/jalu/configme/internal/record/ReflectionHelper.java index 441a5fe4..7e9f0d82 100644 --- a/src/main/java/ch/jalu/configme/internal/record/ReflectionHelper.java +++ b/src/main/java/ch/jalu/configme/internal/record/ReflectionHelper.java @@ -5,6 +5,7 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import java.lang.reflect.AccessibleObject; import java.lang.reflect.Method; /** @@ -56,4 +57,20 @@ public class ReflectionHelper { throw new ConfigMeException("Failed to call " + method + " for " + instance, e); } } + + /** + * Makes the given accessible object (e.g. a field) accessible if it isn't yet. + * + * @param accessibleObject the reflected object to make accessible (if needed) + */ + public static void setAccessibleIfNeeded(AccessibleObject accessibleObject) { + // #347: Catch InaccessibleObjectException, use #trySetAccessible? + if (!accessibleObject.isAccessible()) { + try { + accessibleObject.setAccessible(true); + } catch (SecurityException e) { + throw new ConfigMeException("Failed to make " + accessibleObject + " accessible", e); + } + } + } } diff --git a/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImplTest.java b/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImplTest.java index ff6956cc..4fd6cea4 100644 --- a/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImplTest.java +++ b/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImplTest.java @@ -129,7 +129,7 @@ void shouldThrowForWhenExportNameIsNullForProperty() { // then assertThat(ex.getMessage(), - equalTo("Custom name of Bean property '' with getter 'public java.lang.String ch.jalu.configme.samples.beanannotations.BeanWithEmptyName.getAuthor()' may not be empty")); + equalTo("Custom name of FieldProperty '' for field 'BeanWithEmptyName#author' may not be empty")); } @Test diff --git a/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescriptionImplTest.java b/src/test/java/ch/jalu/configme/beanmapper/propertydescription/FieldPropertyTest.java similarity index 74% rename from src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescriptionImplTest.java rename to src/test/java/ch/jalu/configme/beanmapper/propertydescription/FieldPropertyTest.java index 17691ac2..ca5cc9b0 100644 --- a/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescriptionImplTest.java +++ b/src/test/java/ch/jalu/configme/beanmapper/propertydescription/FieldPropertyTest.java @@ -1,6 +1,5 @@ package ch.jalu.configme.beanmapper.propertydescription; -import ch.jalu.configme.beanmapper.ConfigMeMapperException; import ch.jalu.configme.samples.beanannotations.AnnotatedEntry; import org.junit.jupiter.api.Test; @@ -8,12 +7,11 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; -import static org.junit.jupiter.api.Assertions.assertThrows; /** - * Test for {@link BeanPropertyDescriptionImpl}. + * Test for {@link FieldProperty}. */ -class BeanPropertyDescriptionImplTest { +class FieldPropertyTest { @Test void shouldGetProperties() { @@ -29,17 +27,6 @@ void shouldGetProperties() { assertThat(result1, equalTo(77)); } - @Test - void shouldHandlePropertyGetError() { - // given - BeanPropertyDescription sizeProperty = getDescriptor("size", SampleBean.class); - SampleBean bean = new ThrowingBean(); - - // when / then - assertThrows(ConfigMeMapperException.class, - () -> sizeProperty.getValue(bean)); - } - @Test void shouldHaveAppropriateStringRepresentation() { // given @@ -52,8 +39,7 @@ void shouldHaveAppropriateStringRepresentation() { String output = "Found " + hasIdProperty; // then - assertThat(output, equalTo("Found Bean property 'has-id' with getter " - + "'public boolean ch.jalu.configme.samples.beanannotations.AnnotatedEntry.getHasId()'")); + assertThat(output, equalTo("Found FieldProperty 'has-id' for field 'AnnotatedEntry#hasId'")); } private static BeanPropertyDescription getDescriptor(String name, Class clazz) { diff --git a/src/test/java/ch/jalu/configme/configurationdata/ConfigurationDataBuilderTest.java b/src/test/java/ch/jalu/configme/configurationdata/ConfigurationDataBuilderTest.java index fb0c813b..96388e3a 100644 --- a/src/test/java/ch/jalu/configme/configurationdata/ConfigurationDataBuilderTest.java +++ b/src/test/java/ch/jalu/configme/configurationdata/ConfigurationDataBuilderTest.java @@ -7,8 +7,6 @@ import ch.jalu.configme.properties.Property; import ch.jalu.configme.samples.ClassWithPrivatePropertyField; import ch.jalu.configme.samples.TestConfiguration; -import org.jetbrains.annotations.NotNull; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.junit.jupiter.MockitoExtension; @@ -24,8 +22,6 @@ import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; -import static org.hamcrest.Matchers.instanceOf; -import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.sameInstance; import static org.hamcrest.Matchers.startsWith; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -99,42 +95,6 @@ void shouldAccessPrivateField() throws NoSuchFieldException { assertThat(result, sameInstance(ClassWithPrivatePropertyField.getPrivatePropertyValue())); } - @Test - void shouldThrowWrappedExceptionIfFieldCannotBeAccessed() throws NoSuchFieldException { - // given - ConfigurationDataBuilder configurationDataBuilder = new ConfigurationDataBuilder() { - @Override - protected void setFieldAccessibleIfNeeded(@NotNull Field field) { - // do nothing - } - }; - - Field privateProperty = ClassWithPrivatePropertyField.class.getDeclaredField("PRIVATE_INT_PROPERTY"); - - // when - ConfigMeException ex = assertThrows(ConfigMeException.class, - () -> configurationDataBuilder.getPropertyField(privateProperty)); - - // then - assertThat(ex.getMessage(), equalTo("Could not fetch field 'PRIVATE_INT_PROPERTY' from class 'ClassWithPrivatePropertyField'. Is it maybe not public?")); - assertThat(ex.getCause(), instanceOf(IllegalAccessException.class)); - } - - @Test - @Disabled // #347: Enable once we move away from Java 8 - void shouldThrowIfFieldCannotBeMadeAccessible() { - // given - ConfigurationDataBuilder configDataBuilder = new ConfigurationDataBuilder(); - - // when - ConfigMeException ex = assertThrows(ConfigMeException.class, - () -> configDataBuilder.setFieldAccessibleIfNeeded(Integer.class.getDeclaredField("digits"))); - - // then - assertThat(ex.getMessage(), equalTo("Failed to modify access for field 'digits' from class 'Integer'")); - assertThat(ex.getCause(), notNullValue()); // instanceOf(InaccessibleObjectException.class) - } - @Test void shouldCreateConfigDataWithPropertiesList() { // given diff --git a/src/test/java/ch/jalu/configme/internal/record/ReflectionHelperTest.java b/src/test/java/ch/jalu/configme/internal/record/ReflectionHelperTest.java index 8c368ffb..7fda4f51 100644 --- a/src/test/java/ch/jalu/configme/internal/record/ReflectionHelperTest.java +++ b/src/test/java/ch/jalu/configme/internal/record/ReflectionHelperTest.java @@ -3,18 +3,29 @@ import ch.jalu.configme.exception.ConfigMeException; import ch.jalu.configme.properties.Property; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.junit.jupiter.MockitoExtension; +import java.lang.reflect.AccessibleObject; +import java.lang.reflect.Field; import java.lang.reflect.Method; import java.util.HashMap; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.sameInstance; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.BDDMockito.given; +import static org.mockito.BDDMockito.willThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.only; +import static org.mockito.Mockito.verify; /** * Test for {@link ReflectionHelper}. */ +@ExtendWith(MockitoExtension.class) class ReflectionHelperTest { private final ReflectionHelper reflectionHelper = new ReflectionHelper(); @@ -83,4 +94,66 @@ void shouldWrapExceptionIfCallingMethodFails() throws NoSuchMethodException { assertThat(ex.getMessage(), equalTo("Failed to call final java.util.HashMap$Node[] java.util.HashMap.resize() for {}")); assertThat(ex.getCause(), instanceOf(IllegalAccessException.class)); } + + @Test + void shouldMakeFieldAccessible() throws NoSuchFieldException { + // given + Field field = getClass().getDeclaredField("reflectionHelper"); + assertThat(field.isAccessible(), equalTo(false)); // Validate assumption + + // when + ReflectionHelper.setAccessibleIfNeeded(field); + + // then + assertThat(field.isAccessible(), equalTo(true)); + } + + @Test + void shouldNotSetAccessibleIfIsAccessible() { + // given + AccessibleObject accessibleObject = mock(AccessibleObject.class); + given(accessibleObject.isAccessible()).willReturn(true); + + // when + ReflectionHelper.setAccessibleIfNeeded(accessibleObject); + + // then + verify(accessibleObject, only()).isAccessible(); + } + + @Test + void shouldWrapSecurityException() { + // given + AccessibleObject accessibleObject = mock(AccessibleObject.class); + given(accessibleObject.isAccessible()).willReturn(false); + given(accessibleObject.toString()).willReturn("Shop#cashBox"); + + SecurityException securityException = new SecurityException("Bad credit score"); + willThrow(securityException).given(accessibleObject).setAccessible(true); + + // when + ConfigMeException ex = assertThrows(ConfigMeException.class, + () -> ReflectionHelper.setAccessibleIfNeeded(accessibleObject)); + + // then + assertThat(ex.getMessage(), equalTo("Failed to make Shop#cashBox accessible")); + assertThat(ex.getCause(), sameInstance(securityException)); + } + + @Test + void shouldPropagateException() { + // given + AccessibleObject accessibleObject = mock(AccessibleObject.class); + given(accessibleObject.isAccessible()).willReturn(false); + + IllegalArgumentException illegalArgEx = new IllegalArgumentException("Bad credit score"); + willThrow(illegalArgEx).given(accessibleObject).setAccessible(true); + + // when + IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, + () -> ReflectionHelper.setAccessibleIfNeeded(accessibleObject)); + + // then + assertThat(ex, sameInstance(illegalArgEx)); + } } From d4c0646882ccc2f00a761b0b36787872b34599c4 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Fri, 17 Nov 2023 18:27:48 +0100 Subject: [PATCH 05/16] Move & rename more --- .../ch/jalu/configme/beanmapper/Ignore.java | 7 ++ .../jalu/configme/beanmapper/MapperImpl.java | 11 ++- .../instantiation/BeanInspector.java | 46 --------- .../instantiation/BeanInstantiation.java | 22 ++++- .../BeanInstantiationService.java | 11 +++ .../BeanInstantiationServiceImpl.java | 68 ++++++++++++++ ...tion.java => BeanRecordInstantiation.java} | 24 +++-- .../BeanZeroArgConstrInstantiation.java | 12 +-- .../BeanDescriptionFactory.java | 6 +- .../BeanDescriptionFactoryImpl.java | 26 ++--- ...java => BeanFieldPropertyDescription.java} | 22 +++-- .../BeanPropertyDescription.java | 4 +- .../BeanWithCustomLeafTypeTest.java | 4 +- .../configme/beanmapper/MapperImplTest.java | 10 +- .../MapperTypeInfoWithNoClassEquivTest.java | 4 +- .../BeanDescriptionFactoryImplTest.java | 24 ++--- .../BeanFieldPropertyDescriptionTest.java | 94 +++++++++++++++++++ .../FieldPropertyTest.java | 81 ---------------- 18 files changed, 280 insertions(+), 196 deletions(-) delete mode 100644 src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInspector.java create mode 100644 src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationService.java create mode 100644 src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationServiceImpl.java rename src/main/java/ch/jalu/configme/beanmapper/instantiation/{RecordInstantiation.java => BeanRecordInstantiation.java} (66%) rename src/main/java/ch/jalu/configme/beanmapper/propertydescription/{FieldProperty.java => BeanFieldPropertyDescription.java} (70%) create mode 100644 src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanFieldPropertyDescriptionTest.java delete mode 100644 src/test/java/ch/jalu/configme/beanmapper/propertydescription/FieldPropertyTest.java diff --git a/src/main/java/ch/jalu/configme/beanmapper/Ignore.java b/src/main/java/ch/jalu/configme/beanmapper/Ignore.java index ab2aef59..666804f4 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/Ignore.java +++ b/src/main/java/ch/jalu/configme/beanmapper/Ignore.java @@ -5,6 +5,13 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; +/** + * In classes used for bean properties, annotating a field with this annotation tells ConfigMe to ignore the + * property, i.e. when a bean is created, the annotated field will not be set, and when a bean is exported, fields + * with this annotation will also be ignored. + *

+ * Alternatively, you can define fields as {@code transient} for them to be ignored by ConfigMe. + */ @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.FIELD) public @interface Ignore { diff --git a/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java b/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java index 6404f708..932e5265 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java +++ b/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java @@ -4,8 +4,9 @@ import ch.jalu.configme.beanmapper.context.ExportContextImpl; import ch.jalu.configme.beanmapper.context.MappingContext; import ch.jalu.configme.beanmapper.context.MappingContextImpl; -import ch.jalu.configme.beanmapper.instantiation.BeanInspector; +import ch.jalu.configme.beanmapper.instantiation.BeanInstantiationServiceImpl; import ch.jalu.configme.beanmapper.instantiation.BeanInstantiation; +import ch.jalu.configme.beanmapper.instantiation.BeanInstantiationService; import ch.jalu.configme.beanmapper.leafvaluehandler.LeafValueHandler; import ch.jalu.configme.beanmapper.leafvaluehandler.LeafValueHandlerImpl; import ch.jalu.configme.beanmapper.leafvaluehandler.MapperLeafType; @@ -71,20 +72,20 @@ public class MapperImpl implements Mapper { // --------- private final LeafValueHandler leafValueHandler; - private final BeanInspector beanInspector; + private final BeanInstantiationService beanInspector; public MapperImpl() { - this(new BeanInspector(), + this(new BeanInstantiationServiceImpl(), new LeafValueHandlerImpl(LeafValueHandlerImpl.createDefaultLeafTypes())); } - public MapperImpl(@NotNull BeanInspector beanInspector, + public MapperImpl(@NotNull BeanInstantiationService beanInspector, @NotNull LeafValueHandler leafValueHandler) { this.beanInspector = beanInspector; this.leafValueHandler = leafValueHandler; } - protected final @NotNull BeanInspector getBeanInspector() { + protected final @NotNull BeanInstantiationService getBeanInstantiationService() { return beanInspector; } diff --git a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInspector.java b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInspector.java deleted file mode 100644 index 5b97f814..00000000 --- a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInspector.java +++ /dev/null @@ -1,46 +0,0 @@ -package ch.jalu.configme.beanmapper.instantiation; - -import ch.jalu.configme.beanmapper.propertydescription.BeanDescriptionFactory; -import ch.jalu.configme.beanmapper.propertydescription.BeanDescriptionFactoryImpl; -import ch.jalu.configme.beanmapper.propertydescription.FieldProperty; -import ch.jalu.configme.internal.record.RecordComponent; -import ch.jalu.configme.internal.record.RecordInspector; -import ch.jalu.configme.internal.record.ReflectionHelper; -import org.jetbrains.annotations.NotNull; - -import java.lang.reflect.Constructor; -import java.util.List; -import java.util.Optional; - -public class BeanInspector { - - private final RecordInspector recordInspector = new RecordInspector(new ReflectionHelper()); - private final BeanDescriptionFactory beanDescriptionFactory = new BeanDescriptionFactoryImpl(); - - public @NotNull Optional findInstantiation(@NotNull Class clazz) { - if (recordInspector.isRecord(clazz)) { - RecordComponent[] recordComponents = recordInspector.getRecordComponents(clazz); - List properties = - beanDescriptionFactory.createRecordProperties(clazz, recordComponents); - - return Optional.of(new RecordInstantiation(clazz, properties)); - } - - Optional> zeroArgConstructor = getConstructor(clazz); - if (zeroArgConstructor.isPresent()) { - List properties = beanDescriptionFactory.getAllProperties(clazz); - return Optional.of(new BeanZeroArgConstrInstantiation(zeroArgConstructor.get(), properties)); - } - - return Optional.empty(); - } - - static @NotNull Optional> getConstructor(@NotNull Class declarer, - Class @NotNull ... parameters) { - try { - return Optional.of(declarer.getDeclaredConstructor(parameters)); - } catch (NoSuchMethodException ignore) { - return Optional.empty(); - } - } -} diff --git a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiation.java b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiation.java index db4e7b79..634f0b91 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiation.java +++ b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiation.java @@ -7,11 +7,29 @@ import java.util.List; +/** + * Creation method for a given bean type. A bean instantiation returns the properties that are needed to create it + * and allows to create beans of the given type. Objects implementing this interface are stateless. + */ public interface BeanInstantiation { + /** + * Returns the properties which the bean has. + * + * @return the bean's properties + */ @NotNull List getProperties(); - @Nullable Object create(@NotNull List propertyValues, - @NotNull ConvertErrorRecorder errorRecorder); + /** + * Creates a new bean with the given property values. The provided property values must be in the same order as + * returned by this instantiation in {@link #getProperties()}. + * Null is returned if the bean cannot be created, e.g. because a property value was null and it is not supported + * by this instantiation. + * + * @param propertyValues the values to set to the bean (can contain null entries) + * @param errorRecorder error recorder for errors if the bean can be created, but the values weren't fully valid + * @return the bean, if possible, otherwise null + */ + @Nullable Object create(@NotNull List propertyValues, @NotNull ConvertErrorRecorder errorRecorder); } diff --git a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationService.java b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationService.java new file mode 100644 index 00000000..929e26db --- /dev/null +++ b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationService.java @@ -0,0 +1,11 @@ +package ch.jalu.configme.beanmapper.instantiation; + +import org.jetbrains.annotations.NotNull; + +import java.util.Optional; + +public interface BeanInstantiationService { + + @NotNull Optional findInstantiation(@NotNull Class clazz); + +} diff --git a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationServiceImpl.java b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationServiceImpl.java new file mode 100644 index 00000000..9041d031 --- /dev/null +++ b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationServiceImpl.java @@ -0,0 +1,68 @@ +package ch.jalu.configme.beanmapper.instantiation; + +import ch.jalu.configme.beanmapper.propertydescription.BeanDescriptionFactory; +import ch.jalu.configme.beanmapper.propertydescription.BeanDescriptionFactoryImpl; +import ch.jalu.configme.beanmapper.propertydescription.BeanFieldPropertyDescription; +import ch.jalu.configme.internal.record.RecordComponent; +import ch.jalu.configme.internal.record.RecordInspector; +import ch.jalu.configme.internal.record.ReflectionHelper; +import org.jetbrains.annotations.NotNull; + +import java.lang.reflect.Constructor; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.ConcurrentHashMap; + +public class BeanInstantiationServiceImpl implements BeanInstantiationService { + + private final RecordInspector recordInspector = new RecordInspector(new ReflectionHelper()); + private final BeanDescriptionFactory beanDescriptionFactory = new BeanDescriptionFactoryImpl(); + private final Map, BeanInstantiation> cachedInstantiationByType = new ConcurrentHashMap<>(); + + @Override + public @NotNull Optional findInstantiation(@NotNull Class clazz) { + BeanInstantiation cachedInstantiation = cachedInstantiationByType.get(clazz); + if (cachedInstantiation != null) { + return Optional.of(cachedInstantiation); + } + + if (recordInspector.isRecord(clazz)) { + RecordComponent[] recordComponents = recordInspector.getRecordComponents(clazz); + List properties = + beanDescriptionFactory.createRecordProperties(clazz, recordComponents); + + BeanRecordInstantiation recordInstantiation = new BeanRecordInstantiation(clazz, properties); + cachedInstantiationByType.put(clazz, recordInstantiation); + return Optional.of(recordInstantiation); + } + + Optional> zeroArgConstructor = tryFindConstructor(clazz); + if (zeroArgConstructor.isPresent()) { + List properties = beanDescriptionFactory.getAllProperties(clazz); + BeanZeroArgConstrInstantiation zeroArgConstrInstantiation = + new BeanZeroArgConstrInstantiation(zeroArgConstructor.get(), properties); + cachedInstantiationByType.put(clazz, zeroArgConstrInstantiation); + return Optional.of(zeroArgConstrInstantiation); + } + + return Optional.empty(); + } + + /** + * Returns an optional with the constructor on the given {@code declarer} matching the parameter types, + * otherwise an empty optional. + * + * @param declarer the class to search for constructors + * @param parameterTypes the parameter types of the desired constructor + * @return optional with the constructor if found, empty optional otherwise + */ + protected static @NotNull Optional> tryFindConstructor(@NotNull Class declarer, + Class @NotNull ... parameterTypes) { + try { + return Optional.of(declarer.getDeclaredConstructor(parameterTypes)); + } catch (NoSuchMethodException ignore) { + return Optional.empty(); + } + } +} diff --git a/src/main/java/ch/jalu/configme/beanmapper/instantiation/RecordInstantiation.java b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanRecordInstantiation.java similarity index 66% rename from src/main/java/ch/jalu/configme/beanmapper/instantiation/RecordInstantiation.java rename to src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanRecordInstantiation.java index 4609b831..b11a1e13 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/instantiation/RecordInstantiation.java +++ b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanRecordInstantiation.java @@ -1,7 +1,7 @@ package ch.jalu.configme.beanmapper.instantiation; import ch.jalu.configme.beanmapper.propertydescription.BeanPropertyDescription; -import ch.jalu.configme.beanmapper.propertydescription.FieldProperty; +import ch.jalu.configme.beanmapper.propertydescription.BeanFieldPropertyDescription; import ch.jalu.configme.exception.ConfigMeException; import ch.jalu.configme.properties.convertresult.ConvertErrorRecorder; import org.jetbrains.annotations.NotNull; @@ -12,15 +12,24 @@ import java.util.List; import java.util.Objects; -final class RecordInstantiation implements BeanInstantiation { +/** + * Instantiates bean types that are records. + */ +public class BeanRecordInstantiation implements BeanInstantiation { private final Constructor canonicalConstructor; - private final List properties; - - public RecordInstantiation(@NotNull Class clazz, @NotNull List properties) { + private final List properties; + + /** + * Constructor. + * + * @param clazz the record type + * @param properties the properties of the record + */ + public BeanRecordInstantiation(@NotNull Class clazz, @NotNull List properties) { this.properties = properties; - Class[] recordTypes = properties.stream().map(FieldProperty::getType).toArray(Class[]::new); - this.canonicalConstructor = BeanInspector.getConstructor(clazz, recordTypes) + Class[] recordTypes = properties.stream().map(BeanFieldPropertyDescription::getType).toArray(Class[]::new); + this.canonicalConstructor = BeanInstantiationServiceImpl.tryFindConstructor(clazz, recordTypes) .orElseThrow(() -> new ConfigMeException("Could not get canonical constructor of " + clazz)); } @@ -40,7 +49,6 @@ public RecordInstantiation(@NotNull Class clazz, @NotNull List try { return canonicalConstructor.newInstance(properties); } catch (IllegalArgumentException | ReflectiveOperationException e) { - // TODO: Separate clause for InvocationTargetException? throw new ConfigMeException("Error calling record constructor for " + canonicalConstructor.getDeclaringClass(), e); } diff --git a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstrInstantiation.java b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstrInstantiation.java index 5c59847a..d984c61e 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstrInstantiation.java +++ b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstrInstantiation.java @@ -1,7 +1,7 @@ package ch.jalu.configme.beanmapper.instantiation; import ch.jalu.configme.beanmapper.propertydescription.BeanPropertyDescription; -import ch.jalu.configme.beanmapper.propertydescription.FieldProperty; +import ch.jalu.configme.beanmapper.propertydescription.BeanFieldPropertyDescription; import ch.jalu.configme.exception.ConfigMeException; import ch.jalu.configme.properties.convertresult.ConvertErrorRecorder; import org.jetbrains.annotations.NotNull; @@ -12,13 +12,13 @@ import java.util.Iterator; import java.util.List; -final class BeanZeroArgConstrInstantiation implements BeanInstantiation { +public class BeanZeroArgConstrInstantiation implements BeanInstantiation { private final Constructor zeroArgsConstructor; - private final List properties; + private final List properties; public BeanZeroArgConstrInstantiation(@NotNull Constructor zeroArgsConstructor, - @NotNull List properties) { + @NotNull List properties) { this.zeroArgsConstructor = zeroArgsConstructor; this.properties = properties; } @@ -44,10 +44,10 @@ public BeanZeroArgConstrInstantiation(@NotNull Constructor zeroArgsConstructo + zeroArgsConstructor.getDeclaringClass() + " has " + properties.size() + " properties"); } - Iterator propIt = properties.iterator(); + Iterator propIt = properties.iterator(); Iterator valuesIt = propertyValues.iterator(); while (propIt.hasNext() && valuesIt.hasNext()) { - FieldProperty property = propIt.next(); + BeanFieldPropertyDescription property = propIt.next(); Object value = valuesIt.next(); if (value == null) { if (property.getValue(bean) == null) { diff --git a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactory.java b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactory.java index 5819aaf6..07ace817 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactory.java +++ b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactory.java @@ -12,9 +12,9 @@ */ public interface BeanDescriptionFactory { - @NotNull List getAllProperties(@NotNull Class clazz); + @NotNull List getAllProperties(@NotNull Class clazz); - @NotNull List createRecordProperties(@NotNull Class clazz, - RecordComponent @NotNull [] components); + @NotNull List createRecordProperties(@NotNull Class clazz, + RecordComponent @NotNull [] components); } diff --git a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImpl.java b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImpl.java index 8e185123..d90033dc 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImpl.java +++ b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImpl.java @@ -15,7 +15,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; -import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; @@ -36,28 +35,21 @@ */ public class BeanDescriptionFactoryImpl implements BeanDescriptionFactory { - private final Map, List> classProperties = new HashMap<>(); - @Override - public @NotNull List getAllProperties(@NotNull Class clazz) { - return classProperties.computeIfAbsent(clazz, this::collectAllProperties); - } - - public @NotNull List createRecordProperties(@NotNull Class clazz, - RecordComponent @NotNull [] components) { - // TODO: Caching here. - LinkedHashMap instanceFieldsByName = FieldUtils.getAllFields(clazz) + public @NotNull List createRecordProperties(@NotNull Class clazz, + RecordComponent @NotNull [] components) { + Map instanceFieldsByName = FieldUtils.getAllFields(clazz) .filter(FieldUtils::isRegularInstanceField) .collect(FieldUtils.collectByName(false)); - List relevantFields = new ArrayList<>(); + List relevantFields = new ArrayList<>(); for (RecordComponent component : components) { Field field = instanceFieldsByName.get(component.getName()); if (field == null) { throw new ConfigMeException("Record component '" + component.getName() + "' for " + clazz.getName() + " does not have a field with the same name"); } - FieldProperty property = convert(field); + BeanFieldPropertyDescription property = convert(field); if (property == null) { throw new ConfigMeException("Record component '" + component.getName() + "' for " + clazz.getName() + " has a field defined to be ignored: this is not supported for records"); @@ -73,12 +65,12 @@ public class BeanDescriptionFactoryImpl implements BeanDescriptionFactory { * @param clazz the class to process * @return properties of the class */ - protected @NotNull List collectAllProperties(@NotNull Class clazz) { + public @NotNull List getAllProperties(@NotNull Class clazz) { LinkedHashMap instanceFieldsByName = FieldUtils.getAllFields(clazz) .filter(FieldUtils::isRegularInstanceField) .collect(FieldUtils.collectByName(false)); - List properties = instanceFieldsByName.values().stream() + List properties = instanceFieldsByName.values().stream() .map(this::convert) .filter(Objects::nonNull) .collect(Collectors.toList()); @@ -87,12 +79,12 @@ public class BeanDescriptionFactoryImpl implements BeanDescriptionFactory { return properties; } - protected @Nullable FieldProperty convert(@NotNull Field field) { + protected @Nullable BeanFieldPropertyDescription convert(@NotNull Field field) { if (Modifier.isTransient(field.getModifiers()) || field.isAnnotationPresent(Ignore.class)) { return null; } - return new FieldProperty(field, + return new BeanFieldPropertyDescription(field, getCustomExportName(field), getComments(field)); } diff --git a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/FieldProperty.java b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanFieldPropertyDescription.java similarity index 70% rename from src/main/java/ch/jalu/configme/beanmapper/propertydescription/FieldProperty.java rename to src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanFieldPropertyDescription.java index 431602e7..aa246a38 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/FieldProperty.java +++ b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanFieldPropertyDescription.java @@ -1,6 +1,5 @@ package ch.jalu.configme.beanmapper.propertydescription; -import ch.jalu.configme.beanmapper.ConfigMeMapperException; import ch.jalu.configme.exception.ConfigMeException; import ch.jalu.configme.internal.record.ReflectionHelper; import ch.jalu.typeresolver.FieldUtils; @@ -10,15 +9,18 @@ import java.lang.reflect.Field; -public class FieldProperty implements BeanPropertyDescription { +/** + * Bean property description based on a {@link Field}. + */ +public class BeanFieldPropertyDescription implements BeanPropertyDescription { private final Field field; private final String exportName; private final BeanPropertyComments comments; - public FieldProperty(@NotNull Field field, - @Nullable String exportName, - @NotNull BeanPropertyComments comments) { + public BeanFieldPropertyDescription(@NotNull Field field, + @Nullable String exportName, + @NotNull BeanPropertyComments comments) { this.field = field; this.exportName = exportName; this.comments = comments; @@ -40,6 +42,14 @@ public FieldProperty(@NotNull Field field, return field.getType(); } + /** + * Sets the provided value to the field wrapped by this instance on the given bean. This method does not + * check whether the field is final; in some contexts (e.g. instantiation a record type), this method cannot + * be called. + * + * @param bean the bean to set the value to + * @param value the value to set + */ public void setValue(@NotNull Object bean, @NotNull Object value) { ReflectionHelper.setAccessibleIfNeeded(field); @@ -47,7 +57,7 @@ public void setValue(@NotNull Object bean, @NotNull Object value) { field.set(bean, value); } catch (IllegalArgumentException | IllegalAccessException e) { String fieldName = FieldUtils.formatField(field); - throw new ConfigMeMapperException("Failed to set value to field " + fieldName + ". Value: " + value, e); + throw new ConfigMeException("Failed to set value to field " + fieldName + ". Value: " + value, e); } } diff --git a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescription.java b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescription.java index f5299c5d..bde49cb9 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescription.java +++ b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescription.java @@ -6,10 +6,10 @@ /** * Represents a property within a bean class, as used in - * {@link ch.jalu.configme.beanmapper.instantiation.BeanInspector}. + * {@link ch.jalu.configme.beanmapper.instantiation.BeanInstantiationService}. * There, for instance, there is a {@link BeanDescriptionFactory} field responsible for creating bean descriptions. *

- * Default implementation is {@link FieldProperty}. + * Default implementation is {@link BeanFieldPropertyDescription}. */ public interface BeanPropertyDescription { diff --git a/src/test/java/ch/jalu/configme/beanmapper/BeanWithCustomLeafTypeTest.java b/src/test/java/ch/jalu/configme/beanmapper/BeanWithCustomLeafTypeTest.java index 39fcedfc..5d317809 100644 --- a/src/test/java/ch/jalu/configme/beanmapper/BeanWithCustomLeafTypeTest.java +++ b/src/test/java/ch/jalu/configme/beanmapper/BeanWithCustomLeafTypeTest.java @@ -4,7 +4,7 @@ import ch.jalu.configme.SettingsManager; import ch.jalu.configme.SettingsManagerBuilder; import ch.jalu.configme.TestUtils; -import ch.jalu.configme.beanmapper.instantiation.BeanInspector; +import ch.jalu.configme.beanmapper.instantiation.BeanInstantiationServiceImpl; import ch.jalu.configme.beanmapper.leafvaluehandler.LeafValueHandlerImpl; import ch.jalu.configme.beanmapper.leafvaluehandler.MapperLeafType; import ch.jalu.configme.properties.BeanProperty; @@ -84,7 +84,7 @@ private MyTestSettings() { public static final class MapperWithCustomIntSupport extends MapperImpl { MapperWithCustomIntSupport() { - super(new BeanInspector(), + super(new BeanInstantiationServiceImpl(), LeafValueHandlerImpl.builder().addDefaults().addType(new CustomIntegerLeafType()).build()); } } diff --git a/src/test/java/ch/jalu/configme/beanmapper/MapperImplTest.java b/src/test/java/ch/jalu/configme/beanmapper/MapperImplTest.java index 63f6fa29..53b201a2 100644 --- a/src/test/java/ch/jalu/configme/beanmapper/MapperImplTest.java +++ b/src/test/java/ch/jalu/configme/beanmapper/MapperImplTest.java @@ -10,7 +10,7 @@ import ch.jalu.configme.beanmapper.command.optionalproperties.ComplexOptionalTypeConfig; import ch.jalu.configme.beanmapper.context.MappingContext; import ch.jalu.configme.beanmapper.context.MappingContextImpl; -import ch.jalu.configme.beanmapper.instantiation.BeanInspector; +import ch.jalu.configme.beanmapper.instantiation.BeanInstantiationService; import ch.jalu.configme.beanmapper.leafvaluehandler.LeafValueHandler; import ch.jalu.configme.beanmapper.leafvaluehandler.LeafValueHandlerImpl; import ch.jalu.configme.beanmapper.typeissues.GenericCollection; @@ -453,16 +453,16 @@ void shouldForwardException() { @Test void shouldReturnFields() { // given - BeanInspector beanInspector = mock(BeanInspector.class); + BeanInstantiationService beanInstantiationService = mock(BeanInstantiationService.class); LeafValueHandlerImpl leafValueHandler = mock(LeafValueHandlerImpl.class); - MapperImpl mapper = new MapperImpl(beanInspector, leafValueHandler); + MapperImpl mapper = new MapperImpl(beanInstantiationService, leafValueHandler); // when - BeanInspector returnedBeanInspector = mapper.getBeanInspector(); + BeanInstantiationService returnedBeanInstantiationService = mapper.getBeanInstantiationService(); LeafValueHandler returnedLeafValueHandler = mapper.getLeafValueHandler(); // then - assertThat(returnedBeanInspector, sameInstance(beanInspector)); + assertThat(returnedBeanInstantiationService, sameInstance(beanInstantiationService)); assertThat(returnedLeafValueHandler, sameInstance(leafValueHandler)); } diff --git a/src/test/java/ch/jalu/configme/beanmapper/MapperTypeInfoWithNoClassEquivTest.java b/src/test/java/ch/jalu/configme/beanmapper/MapperTypeInfoWithNoClassEquivTest.java index bca4a4ca..8a30b693 100644 --- a/src/test/java/ch/jalu/configme/beanmapper/MapperTypeInfoWithNoClassEquivTest.java +++ b/src/test/java/ch/jalu/configme/beanmapper/MapperTypeInfoWithNoClassEquivTest.java @@ -2,7 +2,7 @@ import ch.jalu.configme.beanmapper.context.MappingContext; import ch.jalu.configme.beanmapper.context.MappingContextImpl; -import ch.jalu.configme.beanmapper.instantiation.BeanInspector; +import ch.jalu.configme.beanmapper.instantiation.BeanInstantiationServiceImpl; import ch.jalu.configme.beanmapper.leafvaluehandler.LeafValueHandlerImpl; import ch.jalu.configme.beanmapper.leafvaluehandler.MapperLeafType; import ch.jalu.configme.exception.ConfigMeException; @@ -43,7 +43,7 @@ void shouldNotThrowForTypeInfoWithNoClassEquivalentTooEarly() { .addDefaults() .addType(extNumberLeafType) .build(); - MapperImpl mapper = new MapperImpl(new BeanInspector(), leafValueHandler); + MapperImpl mapper = new MapperImpl(new BeanInstantiationServiceImpl(), leafValueHandler); // when Object result = mapper.convertToBean(3.2, targetType, errorRecorder); diff --git a/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImplTest.java b/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImplTest.java index 4fd6cea4..c32f0b63 100644 --- a/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImplTest.java +++ b/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImplTest.java @@ -12,6 +12,7 @@ import ch.jalu.configme.samples.inheritance.Child; import ch.jalu.configme.samples.inheritance.Middle; import ch.jalu.typeresolver.TypeInfo; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import java.util.ArrayList; @@ -40,7 +41,7 @@ class BeanDescriptionFactoryImplTest { @Test void shouldReturnWritableProperties() { // given / when - Collection descriptions = factory.getAllProperties(SampleBean.class); + Collection descriptions = factory.getAllProperties(SampleBean.class); // then assertThat(descriptions, hasSize(4)); @@ -71,7 +72,7 @@ void shouldReturnEmptyListForNonBeanClass() { @Test void shouldNotConsiderTransientFields() { // given / when - Collection properties = factory.getAllProperties(BeanWithTransientFields.class); + Collection properties = factory.getAllProperties(BeanWithTransientFields.class); // then assertThat(properties, hasSize(2)); @@ -81,7 +82,7 @@ void shouldNotConsiderTransientFields() { @Test void shouldBeAwareOfInheritanceAndRespectOrder() { // given / when - Collection properties = factory.getAllProperties(Middle.class); + Collection properties = factory.getAllProperties(Middle.class); // then assertThat(properties, hasSize(3)); @@ -91,7 +92,7 @@ void shouldBeAwareOfInheritanceAndRespectOrder() { @Test void shouldLetChildFieldsOverrideParentFields() { // given / when - Collection properties = factory.getAllProperties(Child.class); + Collection properties = factory.getAllProperties(Child.class); // then assertThat(properties, hasSize(5)); @@ -102,7 +103,7 @@ void shouldLetChildFieldsOverrideParentFields() { @Test void shouldUseExportName() { // given / when - Collection properties = factory.getAllProperties(AnnotatedEntry.class); + Collection properties = factory.getAllProperties(AnnotatedEntry.class); // then assertThat(properties, hasSize(2)); @@ -133,10 +134,11 @@ void shouldThrowForWhenExportNameIsNullForProperty() { } @Test - void shouldReturnCommentsWithUuidIfNotRepeatable() { + @Disabled + void shouldReturnCommentsWithUuidIfNotRepeatable() { // TODO: Move me. // given / when - Collection sampleBeanProperties = factory.getAllProperties(SampleBean.class); - Collection sampleBeanProperties2 = factory.getAllProperties(SampleBean.class); + Collection sampleBeanProperties = factory.getAllProperties(SampleBean.class); + Collection sampleBeanProperties2 = factory.getAllProperties(SampleBean.class); // then BeanPropertyComments sizeComments = getDescription("size", sampleBeanProperties).getComments(); @@ -152,7 +154,7 @@ void shouldReturnCommentsWithUuidIfNotRepeatable() { @Test void shouldReturnCommentsWithoutUuid() { // given / when - Collection execDetailsProperties = factory.getAllProperties(ExecutionDetails.class); + Collection execDetailsProperties = factory.getAllProperties(ExecutionDetails.class); // then BeanPropertyComments executorComments = getDescription("executor", execDetailsProperties).getComments(); @@ -166,7 +168,7 @@ void shouldReturnCommentsWithoutUuid() { @Test void shouldPickUpCustomNameFromField() { // given / when - List properties = new ArrayList<>(factory.getAllProperties(BeanWithExportName.class)); + List properties = new ArrayList<>(factory.getAllProperties(BeanWithExportName.class)); // then assertThat(properties, hasSize(3)); @@ -181,7 +183,7 @@ void shouldPickUpCustomNameFromField() { @Test void shouldPickUpCustomNameFromFieldsIncludingInheritance() { // given / when - List properties = new ArrayList<>(factory.getAllProperties(BeanWithExportNameExtension.class)); + List properties = new ArrayList<>(factory.getAllProperties(BeanWithExportNameExtension.class)); // then assertThat(properties, hasSize(4)); diff --git a/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanFieldPropertyDescriptionTest.java b/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanFieldPropertyDescriptionTest.java new file mode 100644 index 00000000..3a214847 --- /dev/null +++ b/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanFieldPropertyDescriptionTest.java @@ -0,0 +1,94 @@ +package ch.jalu.configme.beanmapper.propertydescription; + +import ch.jalu.configme.exception.ConfigMeException; +import ch.jalu.configme.samples.beanannotations.AnnotatedEntry; +import org.junit.jupiter.api.Test; + +import java.util.Collection; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; +import static org.junit.jupiter.api.Assertions.assertThrows; + +/** + * Test for {@link BeanFieldPropertyDescription}. + */ +class BeanFieldPropertyDescriptionTest { + + @Test + void shouldGetProperties() { + // given + BeanFieldPropertyDescription sizeProperty = getDescriptor("size", SampleBean.class); + SampleBean bean = new SampleBean(); + bean.size = 77; + + // when + Object result1 = sizeProperty.getValue(bean); + sizeProperty.setValue(bean, -120); + Object result2 = sizeProperty.getValue(bean); + + // then + assertThat(result1, equalTo(77)); + assertThat(result2, equalTo(-120)); + } + + @Test + void shouldHandlePropertySetError() { + // given + BeanFieldPropertyDescription sizeProperty = getDescriptor("size", SampleBean.class); + String wrongObject = "test"; + + // when + ConfigMeException ex = assertThrows(ConfigMeException.class, + () -> sizeProperty.setValue(wrongObject, -120)); + + // then + assertThat(ex.getMessage(), equalTo("Failed to set value to field BeanFieldPropertyDescriptionTest$SampleBean#size. Value: -120")); + assertThat(ex.getCause(), instanceOf(IllegalArgumentException.class)); + } + + @Test + void shouldHandlePropertyGetError() { + // given + BeanPropertyDescription sizeProperty = getDescriptor("size", SampleBean.class); + String wrongObject = "test"; + + // when + ConfigMeException ex = assertThrows(ConfigMeException.class, + () -> sizeProperty.getValue(wrongObject)); + + // then + assertThat(ex.getMessage(), equalTo("Failed to get value for field BeanFieldPropertyDescriptionTest$SampleBean#size")); + assertThat(ex.getCause(), instanceOf(IllegalArgumentException.class)); + } + + @Test + void shouldHaveAppropriateStringRepresentation() { + // given + Collection properties = new BeanDescriptionFactoryImpl() + .getAllProperties(AnnotatedEntry.class); + BeanPropertyDescription hasIdProperty = properties.stream() + .filter(prop -> "has-id".equals(prop.getName())).findFirst().get(); + + // when + String output = "Found " + hasIdProperty; + + // then + assertThat(output, equalTo("Found FieldProperty 'has-id' for field 'AnnotatedEntry#hasId'")); + } + + private static BeanFieldPropertyDescription getDescriptor(String name, Class clazz) { + return new BeanDescriptionFactoryImpl().getAllProperties(clazz) + .stream() + .filter(prop -> name.equals(prop.getName())) + .findFirst() + .orElseThrow(IllegalArgumentException::new); + } + + private static class SampleBean { + + private int size; + + } +} diff --git a/src/test/java/ch/jalu/configme/beanmapper/propertydescription/FieldPropertyTest.java b/src/test/java/ch/jalu/configme/beanmapper/propertydescription/FieldPropertyTest.java deleted file mode 100644 index ca5cc9b0..00000000 --- a/src/test/java/ch/jalu/configme/beanmapper/propertydescription/FieldPropertyTest.java +++ /dev/null @@ -1,81 +0,0 @@ -package ch.jalu.configme.beanmapper.propertydescription; - -import ch.jalu.configme.samples.beanannotations.AnnotatedEntry; -import org.junit.jupiter.api.Test; - -import java.util.Collection; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.equalTo; - -/** - * Test for {@link FieldProperty}. - */ -class FieldPropertyTest { - - @Test - void shouldGetProperties() { - // given - BeanPropertyDescription sizeProperty = getDescriptor("size", SampleBean.class); - SampleBean bean = new SampleBean(); - bean.setSize(77); - - // when - Object result1 = sizeProperty.getValue(bean); - - // then - assertThat(result1, equalTo(77)); - } - - @Test - void shouldHaveAppropriateStringRepresentation() { - // given - Collection properties = new BeanDescriptionFactoryImpl() - .collectAllProperties(AnnotatedEntry.class); - BeanPropertyDescription hasIdProperty = properties.stream() - .filter(prop -> "has-id".equals(prop.getName())).findFirst().get(); - - // when - String output = "Found " + hasIdProperty; - - // then - assertThat(output, equalTo("Found FieldProperty 'has-id' for field 'AnnotatedEntry#hasId'")); - } - - private static BeanPropertyDescription getDescriptor(String name, Class clazz) { - return new BeanDescriptionFactoryImpl().collectAllProperties(clazz) - .stream() - .filter(prop -> name.equals(prop.getName())) - .findFirst() - .orElseThrow(IllegalArgumentException::new); - } - - private static class SampleBean { - private int size; - - // Need explicit default constructor so Java sees that it's visible - public SampleBean() { - } - - public int getSize() { - return size; - } - - public void setSize(int size) { - this.size = size; - } - } - - private static final class ThrowingBean extends SampleBean { - @Override - public void setSize(int size) { - throw new UnsupportedOperationException(); - } - - @Override - public int getSize() { - throw new UnsupportedOperationException(); - } - } - -} From 4f665224505cf0b023231b6a37d851781fb79223 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Fri, 17 Nov 2023 22:07:01 +0100 Subject: [PATCH 06/16] Add tests --- .../BeanInstantiationServiceImpl.java | 25 ++- ... BeanZeroArgConstructorInstantiation.java} | 14 +- .../BeanInstantiationServiceImplTest.java | 194 ++++++++++++++++++ .../BeanRecordInstantiationTest.java | 119 +++++++++++ ...anZeroArgConstructorInstantiationTest.java | 186 +++++++++++++++++ .../BeanDescriptionFactoryImplTest.java | 20 +- 6 files changed, 528 insertions(+), 30 deletions(-) rename src/main/java/ch/jalu/configme/beanmapper/instantiation/{BeanZeroArgConstrInstantiation.java => BeanZeroArgConstructorInstantiation.java} (82%) create mode 100644 src/test/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationServiceImplTest.java create mode 100644 src/test/java/ch/jalu/configme/beanmapper/instantiation/BeanRecordInstantiationTest.java create mode 100644 src/test/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstructorInstantiationTest.java diff --git a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationServiceImpl.java b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationServiceImpl.java index 9041d031..251fa958 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationServiceImpl.java +++ b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationServiceImpl.java @@ -16,10 +16,21 @@ public class BeanInstantiationServiceImpl implements BeanInstantiationService { - private final RecordInspector recordInspector = new RecordInspector(new ReflectionHelper()); - private final BeanDescriptionFactory beanDescriptionFactory = new BeanDescriptionFactoryImpl(); + private final RecordInspector recordInspector; + private final BeanDescriptionFactory beanDescriptionFactory; private final Map, BeanInstantiation> cachedInstantiationByType = new ConcurrentHashMap<>(); + public BeanInstantiationServiceImpl() { + this.recordInspector = new RecordInspector(new ReflectionHelper()); + this.beanDescriptionFactory = new BeanDescriptionFactoryImpl(); + } + + public BeanInstantiationServiceImpl(@NotNull RecordInspector recordInspector, + @NotNull BeanDescriptionFactory beanDescriptionFactory) { + this.recordInspector = recordInspector; + this.beanDescriptionFactory = beanDescriptionFactory; + } + @Override public @NotNull Optional findInstantiation(@NotNull Class clazz) { BeanInstantiation cachedInstantiation = cachedInstantiationByType.get(clazz); @@ -40,10 +51,12 @@ public class BeanInstantiationServiceImpl implements BeanInstantiationService { Optional> zeroArgConstructor = tryFindConstructor(clazz); if (zeroArgConstructor.isPresent()) { List properties = beanDescriptionFactory.getAllProperties(clazz); - BeanZeroArgConstrInstantiation zeroArgConstrInstantiation = - new BeanZeroArgConstrInstantiation(zeroArgConstructor.get(), properties); - cachedInstantiationByType.put(clazz, zeroArgConstrInstantiation); - return Optional.of(zeroArgConstrInstantiation); + if (!properties.isEmpty()) { + BeanZeroArgConstructorInstantiation zeroArgConstrInstantiation = + new BeanZeroArgConstructorInstantiation(zeroArgConstructor.get(), properties); + cachedInstantiationByType.put(clazz, zeroArgConstrInstantiation); + return Optional.of(zeroArgConstrInstantiation); + } } return Optional.empty(); diff --git a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstrInstantiation.java b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstructorInstantiation.java similarity index 82% rename from src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstrInstantiation.java rename to src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstructorInstantiation.java index d984c61e..09165ebf 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstrInstantiation.java +++ b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstructorInstantiation.java @@ -12,13 +12,17 @@ import java.util.Iterator; import java.util.List; -public class BeanZeroArgConstrInstantiation implements BeanInstantiation { +/** + * Instantiates bean types with a zero-arg constructor. This instantiation considers all instance fields that + * are not final. + */ +public class BeanZeroArgConstructorInstantiation implements BeanInstantiation { private final Constructor zeroArgsConstructor; private final List properties; - public BeanZeroArgConstrInstantiation(@NotNull Constructor zeroArgsConstructor, - @NotNull List properties) { + public BeanZeroArgConstructorInstantiation(@NotNull Constructor zeroArgsConstructor, + @NotNull List properties) { this.zeroArgsConstructor = zeroArgsConstructor; this.properties = properties; } @@ -36,7 +40,7 @@ public BeanZeroArgConstrInstantiation(@NotNull Constructor zeroArgsConstructo bean = zeroArgsConstructor.newInstance(); } catch (ReflectiveOperationException e) { throw new ConfigMeException("Failed to call constructor for " - + zeroArgsConstructor.getDeclaringClass()); + + zeroArgsConstructor.getDeclaringClass(), e); } if (propertyValues.size() != properties.size()) { @@ -53,7 +57,7 @@ public BeanZeroArgConstrInstantiation(@NotNull Constructor zeroArgsConstructo if (property.getValue(bean) == null) { return null; // No default value on field, return null -> no bean with a null value } - errorRecorder.setHasError("Fallback to default value"); + errorRecorder.setHasError("Fallback to default value for " + property); } else { property.setValue(bean, value); } diff --git a/src/test/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationServiceImplTest.java b/src/test/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationServiceImplTest.java new file mode 100644 index 00000000..c7a2f8e2 --- /dev/null +++ b/src/test/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationServiceImplTest.java @@ -0,0 +1,194 @@ +package ch.jalu.configme.beanmapper.instantiation; + +import ch.jalu.configme.beanmapper.propertydescription.BeanDescriptionFactory; +import ch.jalu.configme.beanmapper.propertydescription.BeanFieldPropertyDescription; +import ch.jalu.configme.beanmapper.propertydescription.BeanPropertyComments; +import ch.jalu.configme.internal.record.RecordComponent; +import ch.jalu.configme.internal.record.RecordInspector; +import ch.jalu.configme.properties.StringProperty; +import ch.jalu.configme.properties.convertresult.ConvertErrorRecorder; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.lang.reflect.Field; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Optional; +import java.util.UUID; +import java.util.concurrent.TimeUnit; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.sameInstance; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.BDDMockito.given; + +/** + * Test for {@link BeanInstantiationServiceImpl}. + */ +@ExtendWith(MockitoExtension.class) +class BeanInstantiationServiceImplTest { + + private BeanInstantiationServiceImpl beanInstantiationService; + + @Mock + private RecordInspector recordInspector; + + @Mock + private BeanDescriptionFactory beanDescriptionFactory; + + @BeforeEach + void createBeanInstantiationService() { + this.beanInstantiationService = new BeanInstantiationServiceImpl(recordInspector, beanDescriptionFactory); + } + + @Test + void shouldProvideInstantiationForRecord() throws NoSuchFieldException { + // given + given(recordInspector.isRecord(FakeRecord.class)).willReturn(true); + + RecordComponent nameComponent = new RecordComponent("name", String.class, String.class); + RecordComponent shoeSizeComponent = new RecordComponent("shoeSize", int.class, int.class); + RecordComponent ageComponent = new RecordComponent("age", double.class, double.class); + RecordComponent[] components = {nameComponent, shoeSizeComponent, ageComponent}; + given(recordInspector.getRecordComponents(FakeRecord.class)) + .willReturn(components); + + List beanProperties = Arrays.asList( + new BeanFieldPropertyDescription(FakeRecord.class.getDeclaredField("name"), null, BeanPropertyComments.EMPTY), + new BeanFieldPropertyDescription(FakeRecord.class.getDeclaredField("shoeSize"), null, BeanPropertyComments.EMPTY), + new BeanFieldPropertyDescription(FakeRecord.class.getDeclaredField("age"), null, BeanPropertyComments.EMPTY)); + given(beanDescriptionFactory.createRecordProperties(FakeRecord.class, components)).willReturn(beanProperties); + + // when + Optional instantiation = beanInstantiationService.findInstantiation(FakeRecord.class); + + // then + assertThat(instantiation.isPresent(), equalTo(true)); + assertThat(instantiation.get(), instanceOf(BeanRecordInstantiation.class)); + assertThat(instantiation.get().getProperties(), hasSize(3)); + + Object bean = instantiation.get().create(Arrays.asList("Test", 39, 40.25), new ConvertErrorRecorder()); + assertThat(bean, notNullValue()); + assertThat(((FakeRecord) bean).name, equalTo("Test")); + assertThat(((FakeRecord) bean).shoeSize, equalTo(39)); + assertThat(((FakeRecord) bean).age, equalTo(40.25)); + } + + @Test + void shouldProvideInstantiationForZeroArgConstructorClass() throws NoSuchFieldException { + // given + given(recordInspector.isRecord(SampleBean.class)).willReturn(false); + List beanProperties = Arrays.asList( + new BeanFieldPropertyDescription(SampleBean.class.getDeclaredField("name"), null, BeanPropertyComments.EMPTY), + new BeanFieldPropertyDescription(SampleBean.class.getDeclaredField("shoeSize"), null, BeanPropertyComments.EMPTY), + new BeanFieldPropertyDescription(SampleBean.class.getDeclaredField("age"), null, BeanPropertyComments.EMPTY)); + given(beanDescriptionFactory.getAllProperties(SampleBean.class)).willReturn(beanProperties); + + // when + Optional instantiation = beanInstantiationService.findInstantiation(SampleBean.class); + + // then + assertThat(instantiation.isPresent(), equalTo(true)); + assertThat(instantiation.get(), instanceOf(BeanZeroArgConstructorInstantiation.class)); + assertThat(instantiation.get().getProperties(), hasSize(3)); + + Object bean = instantiation.get().create(Arrays.asList("Test", 39, 40.25), new ConvertErrorRecorder()); + assertThat(bean, notNullValue()); + assertThat(((SampleBean) bean).name, equalTo("Test")); + assertThat(((SampleBean) bean).shoeSize, equalTo(39)); + assertThat(((SampleBean) bean).age, equalTo(40.25)); + } + + @Test + void shouldReturnEmptyOptionalForClassesWithNoInstantiationMethod() { + // given + given(recordInspector.isRecord(any(Class.class))).willReturn(false); + + // when + Optional result1 = beanInstantiationService.findInstantiation(String.class); + Optional result2 = beanInstantiationService.findInstantiation(TimeUnit.class); + Optional result3 = beanInstantiationService.findInstantiation(StringProperty.class); + + // then + assertThat(result1.isPresent(), equalTo(false)); + assertThat(result2.isPresent(), equalTo(false)); + assertThat(result3.isPresent(), equalTo(false)); + } + + @Test + void shouldCacheInstantiations() throws NoSuchFieldException { + // given + // Set up record instantiation + given(recordInspector.isRecord(FakeRecord.class)).willReturn(true); + + RecordComponent ageComponent = new RecordComponent("age", double.class, double.class); + RecordComponent[] components = {ageComponent}; + given(recordInspector.getRecordComponents(FakeRecord.class)).willReturn(components); + + Field recordAgeField = FakeRecord.class.getDeclaredField("age"); + BeanPropertyComments recordAgeComments = new BeanPropertyComments(Arrays.asList("some", "comment"), UUID.randomUUID()); + BeanFieldPropertyDescription recordAgeProperty = new BeanFieldPropertyDescription(recordAgeField, null, recordAgeComments); + given(beanDescriptionFactory.createRecordProperties(FakeRecord.class, components)).willReturn(Collections.singletonList(recordAgeProperty)); + + // Set up zero-args constructor instantiation + given(recordInspector.isRecord(SampleBean.class)).willReturn(false); + + Field beanNameField = SampleBean.class.getDeclaredField("name"); + BeanPropertyComments beanNameComments = new BeanPropertyComments(Collections.singletonList("comment"), UUID.randomUUID()); + BeanFieldPropertyDescription beanNameProperty = new BeanFieldPropertyDescription(beanNameField, null, beanNameComments); + given(beanDescriptionFactory.getAllProperties(SampleBean.class)).willReturn(Collections.singletonList(beanNameProperty)); + + // when + Optional recordInstantiation1 = beanInstantiationService.findInstantiation(FakeRecord.class); + Optional recordInstantiation2 = beanInstantiationService.findInstantiation(FakeRecord.class); + Optional zeroArgsInstantiation1 = beanInstantiationService.findInstantiation(SampleBean.class); + Optional zeroArgsInstantiation2 = beanInstantiationService.findInstantiation(SampleBean.class); + + // then + assertThat(recordInstantiation1.get(), sameInstance(recordInstantiation2.get())); + assertThat(recordInstantiation1.get().getProperties().get(0).getComments().getUuid(), equalTo(recordAgeComments.getUuid())); + assertThat(recordInstantiation2.get().getProperties().get(0).getComments().getUuid(), equalTo(recordAgeComments.getUuid())); + + assertThat(zeroArgsInstantiation1.get(), sameInstance(zeroArgsInstantiation2.get())); + assertThat(zeroArgsInstantiation1.get().getProperties().get(0).getComments().getUuid(), equalTo(beanNameComments.getUuid())); + assertThat(zeroArgsInstantiation2.get().getProperties().get(0).getComments().getUuid(), equalTo(beanNameComments.getUuid())); + } + + private static final class FakeRecord { // #347: Change to record when the Java version allows it + + private final String name; + private final int shoeSize; + private final double age; + + public FakeRecord(String name, int shoeSize, double age) { + this.name = name; + this.shoeSize = shoeSize; + this.age = age; + } + + // Simpler constructor to keep some tests shorter + public FakeRecord(double age) { + this.age = age; + this.shoeSize = -0; + this.name = "-"; + } + } + + private static final class SampleBean { + + private String name; + private int shoeSize; + private double age; + + public SampleBean() { + } + } +} diff --git a/src/test/java/ch/jalu/configme/beanmapper/instantiation/BeanRecordInstantiationTest.java b/src/test/java/ch/jalu/configme/beanmapper/instantiation/BeanRecordInstantiationTest.java new file mode 100644 index 00000000..fca9455e --- /dev/null +++ b/src/test/java/ch/jalu/configme/beanmapper/instantiation/BeanRecordInstantiationTest.java @@ -0,0 +1,119 @@ +package ch.jalu.configme.beanmapper.instantiation; + +import ch.jalu.configme.beanmapper.propertydescription.BeanFieldPropertyDescription; +import ch.jalu.configme.beanmapper.propertydescription.BeanPropertyComments; +import ch.jalu.configme.exception.ConfigMeException; +import ch.jalu.configme.properties.convertresult.ConvertErrorRecorder; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.util.Arrays; +import java.util.List; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.nullValue; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verifyNoInteractions; + +/** + * Test for {@link BeanRecordInstantiation}. + */ +@ExtendWith(MockitoExtension.class) +class BeanRecordInstantiationTest { + + @Test + void shouldThrowForMissingConstructor() throws NoSuchFieldException { + // given + // Size & name properties are in the wrong order + List properties = Arrays.asList( + new BeanFieldPropertyDescription(ExampleRecord.class.getDeclaredField("size"), null, BeanPropertyComments.EMPTY), + new BeanFieldPropertyDescription(ExampleRecord.class.getDeclaredField("name"), null, BeanPropertyComments.EMPTY)); + + // when + ConfigMeException ex = assertThrows(ConfigMeException.class, + () -> new BeanRecordInstantiation(ExampleRecord.class, properties)); + + // then + assertThat(ex.getMessage(), equalTo("Could not get canonical constructor of class ch.jalu.configme.beanmapper.instantiation.BeanRecordInstantiationTest$ExampleRecord")); + assertThat(ex.getCause(), nullValue()); + } + + @Test + void shouldInstantiateRecord() throws NoSuchFieldException { + // given + List properties = Arrays.asList( + new BeanFieldPropertyDescription(ExampleRecord.class.getDeclaredField("name"), null, BeanPropertyComments.EMPTY), + new BeanFieldPropertyDescription(ExampleRecord.class.getDeclaredField("size"), null, BeanPropertyComments.EMPTY)); + + BeanRecordInstantiation instantiation = new BeanRecordInstantiation(ExampleRecord.class, properties); + ConvertErrorRecorder errorRecorder = mock(ConvertErrorRecorder.class); + + // when + ExampleRecord result = (ExampleRecord) instantiation.create(Arrays.asList("Toast", 10), errorRecorder); + + // then + assertThat(result.name(), equalTo("Toast")); + assertThat(result.size(), equalTo(10)); + verifyNoInteractions(errorRecorder); + } + + @Test + void shouldReturnNullIfAnyPropertyIsNull() throws NoSuchFieldException { + // given + List properties = Arrays.asList( + new BeanFieldPropertyDescription(ExampleRecord.class.getDeclaredField("name"), null, BeanPropertyComments.EMPTY), + new BeanFieldPropertyDescription(ExampleRecord.class.getDeclaredField("size"), null, BeanPropertyComments.EMPTY)); + + BeanRecordInstantiation instantiation = new BeanRecordInstantiation(ExampleRecord.class, properties); + ConvertErrorRecorder errorRecorder = mock(ConvertErrorRecorder.class); + + // when / then + assertThat(instantiation.create(Arrays.asList("Toast", null), errorRecorder), nullValue()); + assertThat(instantiation.create(Arrays.asList(null, 33), errorRecorder), nullValue()); + + verifyNoInteractions(errorRecorder); + } + + @Test + void shouldHandleWrongPropertyValuesGracefully() throws NoSuchFieldException { + // given + List properties = Arrays.asList( + new BeanFieldPropertyDescription(ExampleRecord.class.getDeclaredField("name"), null, BeanPropertyComments.EMPTY), + new BeanFieldPropertyDescription(ExampleRecord.class.getDeclaredField("size"), null, BeanPropertyComments.EMPTY)); + + BeanRecordInstantiation instantiation = new BeanRecordInstantiation(ExampleRecord.class, properties); + ConvertErrorRecorder errorRecorder = mock(ConvertErrorRecorder.class); + + // when + ConfigMeException ex = assertThrows(ConfigMeException.class, + () -> instantiation.create(Arrays.asList(3, 3), errorRecorder)); + + // then + assertThat(ex.getMessage(), equalTo("Error calling record constructor for class ch.jalu.configme.beanmapper.instantiation.BeanRecordInstantiationTest$ExampleRecord")); + assertThat(ex.getCause(), instanceOf(IllegalArgumentException.class)); + assertThat(ex.getCause().getMessage(), equalTo("argument type mismatch")); + } + + private static class ExampleRecord { // #347: Change to an actual record :) + + private final String name; + private final int size; + + ExampleRecord(String name, int size) { + this.name = name; + this.size = size; + } + + String name() { + return name; + } + + int size() { + return size; + } + } +} diff --git a/src/test/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstructorInstantiationTest.java b/src/test/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstructorInstantiationTest.java new file mode 100644 index 00000000..e89c4204 --- /dev/null +++ b/src/test/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstructorInstantiationTest.java @@ -0,0 +1,186 @@ +package ch.jalu.configme.beanmapper.instantiation; + +import ch.jalu.configme.beanmapper.propertydescription.BeanFieldPropertyDescription; +import ch.jalu.configme.beanmapper.propertydescription.BeanPropertyComments; +import ch.jalu.configme.exception.ConfigMeException; +import ch.jalu.configme.properties.convertresult.ConvertErrorRecorder; +import ch.jalu.typeresolver.FieldUtils; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.stream.Collectors; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.verifyNoMoreInteractions; + +/** + * Test for {@link BeanZeroArgConstructorInstantiation}. + */ +@ExtendWith(MockitoExtension.class) +class BeanZeroArgConstructorInstantiationTest { + + @Test + void shouldCreateBeanWithProperties() { + // given + BeanZeroArgConstructorInstantiation instantiation = SampleBean.createInstantiation(); + ConvertErrorRecorder errorRecorder = mock(ConvertErrorRecorder.class); + + // when + SampleBean result = (SampleBean) instantiation.create(Arrays.asList("Toast", 12), errorRecorder); + + // then + assertThat(result.getName(), equalTo("Toast")); + assertThat(result.getSize(), equalTo(12)); + verifyNoInteractions(errorRecorder); + } + + @Test + void shouldNotCreateBeanForNullValue() { + // given + BeanZeroArgConstructorInstantiation instantiation = SampleBean.createInstantiation(); + ConvertErrorRecorder errorRecorder = mock(ConvertErrorRecorder.class); + + // when + Object result = instantiation.create(Arrays.asList(null, 534), errorRecorder); + + // then + assertThat(result, nullValue()); + verifyNoInteractions(errorRecorder); + } + + @Test + void shouldThrowForValueThatDoesNotMatchField() { + // given + BeanZeroArgConstructorInstantiation instantiation = SampleBean.createInstantiation(); + ConvertErrorRecorder errorRecorder = mock(ConvertErrorRecorder.class); + + // when + ConfigMeException ex = assertThrows(ConfigMeException.class, + () -> instantiation.create(Arrays.asList("Toast", "wrong"), errorRecorder)); + + // then + assertThat(ex.getMessage(), equalTo("Failed to set value to field BeanZeroArgConstructorInstantiationTest$SampleBean#size. Value: wrong")); + assertThat(ex.getCause(), instanceOf(IllegalArgumentException.class)); + } + + @Test + void shouldThrowForPrivateConstructor() throws NoSuchMethodException { + // given + BeanZeroArgConstructorInstantiation instantiation = new BeanZeroArgConstructorInstantiation( + BeanWithPrivateConstructor.class.getDeclaredConstructor(), + Collections.emptyList()); + ConvertErrorRecorder errorRecorder = new ConvertErrorRecorder(); + + // when + ConfigMeException ex = assertThrows(ConfigMeException.class, + () -> instantiation.create(Collections.emptyList(), errorRecorder)); + + // then + assertThat(ex.getMessage(), equalTo("Failed to call constructor for class ch.jalu.configme.beanmapper.instantiation.BeanZeroArgConstructorInstantiationTest$BeanWithPrivateConstructor")); + assertThat(ex.getCause(), instanceOf(IllegalAccessException.class)); + } + + @Test + void shouldAcceptNullValuesIfFieldHasDefault() throws NoSuchMethodException { + // given + List properties = Arrays.stream(BeanWithFieldDefaults.class.getDeclaredFields()) + .filter(FieldUtils::isRegularInstanceField) + .map(field -> new BeanFieldPropertyDescription(field, null, BeanPropertyComments.EMPTY)) + .collect(Collectors.toList()); + + BeanZeroArgConstructorInstantiation instantiation = new BeanZeroArgConstructorInstantiation( + BeanWithFieldDefaults.class.getDeclaredConstructor(), + properties); + + ConvertErrorRecorder errorRecorder = mock(ConvertErrorRecorder.class); + + // when + BeanWithFieldDefaults bean1 = (BeanWithFieldDefaults) instantiation.create(Arrays.asList(null, 3), errorRecorder); + BeanWithFieldDefaults bean2 = (BeanWithFieldDefaults) instantiation.create(Arrays.asList("test", null), errorRecorder); + + // then + assertThat(bean1, notNullValue()); + assertThat(bean1.name, equalTo("def")); + assertThat(bean1.size, equalTo(3)); + verify(errorRecorder).setHasError("Fallback to default value for FieldProperty 'name' for field 'BeanZeroArgConstructorInstantiationTest$BeanWithFieldDefaults#name'"); + + assertThat(bean2, notNullValue()); + assertThat(bean2.name, equalTo("test")); + assertThat(bean2.size, equalTo(0)); + verify(errorRecorder).setHasError("Fallback to default value for FieldProperty 'size' for field 'BeanZeroArgConstructorInstantiationTest$BeanWithFieldDefaults#size'"); + + verifyNoMoreInteractions(errorRecorder); + } + + @Test + void shouldThrowForPropertyValuesMismatch() { + // given + BeanZeroArgConstructorInstantiation instantiation = SampleBean.createInstantiation(); + + // when + ConfigMeException ex = assertThrows(ConfigMeException.class, + () -> instantiation.create(Arrays.asList(3, 4, 5), new ConvertErrorRecorder())); + + // then + assertThat(ex.getMessage(), equalTo("Invalid property values, 3 were given, but class " + + "ch.jalu.configme.beanmapper.instantiation.BeanZeroArgConstructorInstantiationTest$SampleBean has 2 properties")); + } + + private static final class SampleBean { + + private String name; + private int size; + + public SampleBean() { + } + + public String getName() { + return name; + } + + public int getSize() { + return size; + } + + static BeanZeroArgConstructorInstantiation createInstantiation() { + List properties = Arrays.stream(SampleBean.class.getDeclaredFields()) + .filter(FieldUtils::isRegularInstanceField) + .map(field -> new BeanFieldPropertyDescription(field, null, BeanPropertyComments.EMPTY)) + .collect(Collectors.toList()); + + try { + return new BeanZeroArgConstructorInstantiation(SampleBean.class.getDeclaredConstructor(), properties); + } catch (NoSuchMethodException e) { + throw new IllegalStateException(e); + } + } + } + + private static final class BeanWithPrivateConstructor { + + private BeanWithPrivateConstructor() { + } + } + + private static final class BeanWithFieldDefaults { + + private String name = "def"; + private int size; + + public BeanWithFieldDefaults() { + } + + } +} diff --git a/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImplTest.java b/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImplTest.java index c32f0b63..c8f62b3f 100644 --- a/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImplTest.java +++ b/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImplTest.java @@ -12,7 +12,6 @@ import ch.jalu.configme.samples.inheritance.Child; import ch.jalu.configme.samples.inheritance.Middle; import ch.jalu.typeresolver.TypeInfo; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import java.util.ArrayList; @@ -49,6 +48,7 @@ void shouldReturnWritableProperties() { BeanPropertyDescription sizeProperty = getDescription("size", descriptions); assertThat(sizeProperty.getTypeInformation(), equalTo(new TypeInfo(int.class))); assertThat(sizeProperty.getComments().getComments(), contains("Size of this entry (cm)")); + assertThat(sizeProperty.getComments().getUuid(), notNullValue()); BeanPropertyDescription nameProperty = getDescription("name", descriptions); assertThat(nameProperty.getTypeInformation(), equalTo(new TypeInfo(String.class))); @@ -133,24 +133,6 @@ void shouldThrowForWhenExportNameIsNullForProperty() { equalTo("Custom name of FieldProperty '' for field 'BeanWithEmptyName#author' may not be empty")); } - @Test - @Disabled - void shouldReturnCommentsWithUuidIfNotRepeatable() { // TODO: Move me. - // given / when - Collection sampleBeanProperties = factory.getAllProperties(SampleBean.class); - Collection sampleBeanProperties2 = factory.getAllProperties(SampleBean.class); - - // then - BeanPropertyComments sizeComments = getDescription("size", sampleBeanProperties).getComments(); - assertThat(sizeComments.getComments(), contains("Size of this entry (cm)")); - assertThat(sizeComments.getUuid(), notNullValue()); - - // Actually ensure that we have the same UUID if we fetch properties for the same class again - // -> there's no point in the UUID otherwise! - BeanPropertyComments sizeComments2 = getDescription("size", sampleBeanProperties2).getComments(); - assertThat(sizeComments2.getUuid(), equalTo(sizeComments.getUuid())); - } - @Test void shouldReturnCommentsWithoutUuid() { // given / when From f875c4910b5ef9dec43c52f118db12c5f48ed857 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sun, 26 Nov 2023 14:53:20 +0100 Subject: [PATCH 07/16] Extract methods / add javadoc / introduce interfaces --- .../jalu/configme/beanmapper/MapperImpl.java | 15 ++-- .../BeanInstantiationService.java | 12 +++ .../BeanInstantiationServiceImpl.java | 68 +++++++++++---- .../BeanRecordInstantiation.java | 2 +- .../BeanZeroArgConstructorInstantiation.java | 59 +++++++++---- .../BeanDescriptionFactory.java | 21 ++++- .../BeanDescriptionFactoryImpl.java | 74 ++++++++++------- .../BeanFieldPropertyDescription.java | 15 ++-- .../BeanPropertyDescription.java | 8 +- .../ConfigurationDataBuilder.java | 2 +- .../{record => }/ReflectionHelper.java | 11 ++- .../internal/record/RecordInspector.java | 80 ++---------------- .../internal/record/RecordInspectorImpl.java | 83 +++++++++++++++++++ .../BeanInstantiationServiceImplTest.java | 38 +++++---- ...anZeroArgConstructorInstantiationTest.java | 14 ++-- .../BeanDescriptionFactoryImplTest.java | 22 ++--- .../BeanFieldPropertyDescriptionTest.java | 4 +- .../ch/jalu/configme/demo/beans/UserBase.java | 3 + .../{record => }/ReflectionHelperTest.java | 21 ++++- ...Test.java => RecordInspectorImplTest.java} | 29 ++++--- 20 files changed, 371 insertions(+), 210 deletions(-) rename src/main/java/ch/jalu/configme/internal/{record => }/ReflectionHelper.java (85%) create mode 100644 src/main/java/ch/jalu/configme/internal/record/RecordInspectorImpl.java rename src/test/java/ch/jalu/configme/internal/{record => }/ReflectionHelperTest.java (86%) rename src/test/java/ch/jalu/configme/internal/record/{RecordInspectorTest.java => RecordInspectorImplTest.java} (80%) diff --git a/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java b/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java index 932e5265..cbf153dc 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java +++ b/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java @@ -72,21 +72,21 @@ public class MapperImpl implements Mapper { // --------- private final LeafValueHandler leafValueHandler; - private final BeanInstantiationService beanInspector; + private final BeanInstantiationService beanInstantiationService; public MapperImpl() { this(new BeanInstantiationServiceImpl(), new LeafValueHandlerImpl(LeafValueHandlerImpl.createDefaultLeafTypes())); } - public MapperImpl(@NotNull BeanInstantiationService beanInspector, + public MapperImpl(@NotNull BeanInstantiationService beanInstantiationService, @NotNull LeafValueHandler leafValueHandler) { - this.beanInspector = beanInspector; + this.beanInstantiationService = beanInstantiationService; this.leafValueHandler = leafValueHandler; } protected final @NotNull BeanInstantiationService getBeanInstantiationService() { - return beanInspector; + return beanInstantiationService; } protected final @NotNull LeafValueHandler getLeafValueHandler() { @@ -149,9 +149,8 @@ public MapperImpl(@NotNull BeanInstantiationService beanInspector, return mappedBean; } - @NotNull - private List getBeanProperties(@NotNull Object value) { - return beanInspector.findInstantiation(value.getClass()) + protected @NotNull List getBeanProperties(@NotNull Object value) { + return beanInstantiationService.findInstantiation(value.getClass()) .map(BeanInstantiation::getProperties) .orElse(Collections.emptyList()); } @@ -382,7 +381,7 @@ private List getBeanProperties(@NotNull Object value) { Map entries = (Map) value; Optional instantiation = - beanInspector.findInstantiation(context.getTargetTypeAsClassOrThrow()); + beanInstantiationService.findInstantiation(context.getTargetTypeAsClassOrThrow()); if (instantiation.isPresent()) { List propertyValues = instantiation.get().getProperties().stream() .map(prop -> { diff --git a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationService.java b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationService.java index 929e26db..cefc3d23 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationService.java +++ b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationService.java @@ -4,8 +4,20 @@ import java.util.Optional; +/** + * Service for the creation of beans. + * + * @see BeanInstantiationServiceImpl + */ public interface BeanInstantiationService { + /** + * Inspects the given class and returns an optional with an object defining how to instantiate the bean; + * an empty optional is returned if the class cannot be treated as a bean. + * + * @param clazz the class to inspect + * @return optional with the instantiation, empty optional if not possible + */ @NotNull Optional findInstantiation(@NotNull Class clazz); } diff --git a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationServiceImpl.java b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationServiceImpl.java index 251fa958..609dcebc 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationServiceImpl.java +++ b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationServiceImpl.java @@ -3,10 +3,12 @@ import ch.jalu.configme.beanmapper.propertydescription.BeanDescriptionFactory; import ch.jalu.configme.beanmapper.propertydescription.BeanDescriptionFactoryImpl; import ch.jalu.configme.beanmapper.propertydescription.BeanFieldPropertyDescription; +import ch.jalu.configme.internal.ReflectionHelper; import ch.jalu.configme.internal.record.RecordComponent; import ch.jalu.configme.internal.record.RecordInspector; -import ch.jalu.configme.internal.record.ReflectionHelper; +import ch.jalu.configme.internal.record.RecordInspectorImpl; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import java.lang.reflect.Constructor; import java.util.List; @@ -14,14 +16,25 @@ import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; +/** + * Default implementation of {@link BeanInstantiationService}: defines how bean classes can be created. + *

+ * This service can handle two different types of classes as beans:

    + *
  • Regular Java classes with a public no-args constructor: all non-static, non-transient fields + * will be considered as bean properties.
    • + *
  • Java records
    • + *
+ * + * See {@link BeanDescriptionFactory} for details on how properties of a bean class are determined. + */ public class BeanInstantiationServiceImpl implements BeanInstantiationService { private final RecordInspector recordInspector; private final BeanDescriptionFactory beanDescriptionFactory; - private final Map, BeanInstantiation> cachedInstantiationByType = new ConcurrentHashMap<>(); + private final Map, BeanInstantiation> cachedInstantiationsByType = new ConcurrentHashMap<>(); public BeanInstantiationServiceImpl() { - this.recordInspector = new RecordInspector(new ReflectionHelper()); + this.recordInspector = new RecordInspectorImpl(new ReflectionHelper()); this.beanDescriptionFactory = new BeanDescriptionFactoryImpl(); } @@ -33,33 +46,44 @@ public BeanInstantiationServiceImpl(@NotNull RecordInspector recordInspector, @Override public @NotNull Optional findInstantiation(@NotNull Class clazz) { - BeanInstantiation cachedInstantiation = cachedInstantiationByType.get(clazz); + BeanInstantiation cachedInstantiation = cachedInstantiationsByType.get(clazz); if (cachedInstantiation != null) { return Optional.of(cachedInstantiation); } - if (recordInspector.isRecord(clazz)) { - RecordComponent[] recordComponents = recordInspector.getRecordComponents(clazz); + BeanInstantiation instantiation = createInstantiation(clazz); + if (instantiation != null) { + cachedInstantiationsByType.put(clazz, instantiation); + return Optional.of(instantiation); + } + return Optional.empty(); + } + + /** + * Inspects the class and returns an appropriate instantiation for it, if available. Null is returned if the + * class cannot be treated as a bean. + * + * @param clazz the class to process + * @return bean instantiation for the class, or null if not applicable + */ + protected @Nullable BeanInstantiation createInstantiation(@NotNull Class clazz) { + RecordComponent[] recordComponents = recordInspector.getRecordComponents(clazz); + if (recordComponents != null) { List properties = - beanDescriptionFactory.createRecordProperties(clazz, recordComponents); + beanDescriptionFactory.collectPropertiesForRecord(clazz, recordComponents); - BeanRecordInstantiation recordInstantiation = new BeanRecordInstantiation(clazz, properties); - cachedInstantiationByType.put(clazz, recordInstantiation); - return Optional.of(recordInstantiation); + return new BeanRecordInstantiation(clazz, properties); } Optional> zeroArgConstructor = tryFindConstructor(clazz); if (zeroArgConstructor.isPresent()) { - List properties = beanDescriptionFactory.getAllProperties(clazz); + List properties = beanDescriptionFactory.collectProperties(clazz); if (!properties.isEmpty()) { - BeanZeroArgConstructorInstantiation zeroArgConstrInstantiation = - new BeanZeroArgConstructorInstantiation(zeroArgConstructor.get(), properties); - cachedInstantiationByType.put(clazz, zeroArgConstrInstantiation); - return Optional.of(zeroArgConstrInstantiation); + return new BeanZeroArgConstructorInstantiation(zeroArgConstructor.get(), properties); } } - return Optional.empty(); + return null; } /** @@ -78,4 +102,16 @@ public BeanInstantiationServiceImpl(@NotNull RecordInspector recordInspector, return Optional.empty(); } } + + protected final @NotNull RecordInspector getRecordInspector() { + return recordInspector; + } + + protected final @NotNull BeanDescriptionFactory getBeanDescriptionFactory() { + return beanDescriptionFactory; + } + + protected final @NotNull Map, BeanInstantiation> getCachedInstantiationsByType() { + return cachedInstantiationsByType; + } } diff --git a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanRecordInstantiation.java b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanRecordInstantiation.java index b11a1e13..ec1e979b 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanRecordInstantiation.java +++ b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanRecordInstantiation.java @@ -13,7 +13,7 @@ import java.util.Objects; /** - * Instantiates bean types that are records. + * Instantiates a bean type that is a Java record. */ public class BeanRecordInstantiation implements BeanInstantiation { diff --git a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstructorInstantiation.java b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstructorInstantiation.java index 09165ebf..3810328b 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstructorInstantiation.java +++ b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstructorInstantiation.java @@ -1,8 +1,9 @@ package ch.jalu.configme.beanmapper.instantiation; -import ch.jalu.configme.beanmapper.propertydescription.BeanPropertyDescription; import ch.jalu.configme.beanmapper.propertydescription.BeanFieldPropertyDescription; +import ch.jalu.configme.beanmapper.propertydescription.BeanPropertyDescription; import ch.jalu.configme.exception.ConfigMeException; +import ch.jalu.configme.internal.ReflectionHelper; import ch.jalu.configme.properties.convertresult.ConvertErrorRecorder; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -35,13 +36,7 @@ public BeanZeroArgConstructorInstantiation(@NotNull Constructor zeroArgsConst @Override public @Nullable Object create(@NotNull List propertyValues, @NotNull ConvertErrorRecorder errorRecorder) { - final Object bean; - try { - bean = zeroArgsConstructor.newInstance(); - } catch (ReflectiveOperationException e) { - throw new ConfigMeException("Failed to call constructor for " - + zeroArgsConstructor.getDeclaringClass(), e); - } + final Object bean = createNewInstance(); if (propertyValues.size() != properties.size()) { throw new ConfigMeException("Invalid property values, " + propertyValues.size() + " were given, but " @@ -53,15 +48,49 @@ public BeanZeroArgConstructorInstantiation(@NotNull Constructor zeroArgsConst while (propIt.hasNext() && valuesIt.hasNext()) { BeanFieldPropertyDescription property = propIt.next(); Object value = valuesIt.next(); - if (value == null) { - if (property.getValue(bean) == null) { - return null; // No default value on field, return null -> no bean with a null value - } - errorRecorder.setHasError("Fallback to default value for " + property); - } else { - property.setValue(bean, value); + + boolean isValid = handleProperty(bean, property, value, errorRecorder); + if (!isValid) { + return null; } } return bean; } + + /** + * Creates a new instance with the constructor. + * + * @return the new instance + */ + protected @NotNull Object createNewInstance() { + ReflectionHelper.setAccessibleIfNeeded(zeroArgsConstructor); + try { + return zeroArgsConstructor.newInstance(); + } catch (ReflectiveOperationException e) { + throw new ConfigMeException("Failed to call constructor for " + zeroArgsConstructor.getDeclaringClass(), e); + } + } + + /** + * Processes one property and its value, returning whether the bean is still valid, i.e. if this method returns + * false, the entire instantiation should be aborted. + * + * @param bean the bean to modify + * @param property the property to handle + * @param value the value given for the property + * @param errorRecorder error recorder for conversion errors + * @return false if the bean cannot be constructed, true otherwise (to continue) + */ + protected boolean handleProperty(@NotNull Object bean, @NotNull BeanFieldPropertyDescription property, + @Nullable Object value, @NotNull ConvertErrorRecorder errorRecorder) { + if (value == null) { + if (property.getValue(bean) == null) { + return false; // No default value on field, return null -> no bean with a null value + } + errorRecorder.setHasError("Fallback to default value for " + property); + } else { + property.setValue(bean, value); + } + return true; + } } diff --git a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactory.java b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactory.java index 07ace817..5f6c47ae 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactory.java +++ b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactory.java @@ -12,9 +12,24 @@ */ public interface BeanDescriptionFactory { - @NotNull List getAllProperties(@NotNull Class clazz); + /** + * Collects all properties for the given bean type; this is done based on the class's fields. + * An exception is thrown if the properties are not configured in a valid manner. + * + * @param clazz the bean type + * @return the properties of the given bean type + */ + @NotNull List collectProperties(@NotNull Class clazz); - @NotNull List createRecordProperties(@NotNull Class clazz, - RecordComponent @NotNull [] components); + /** + * Collects all properties for the given type that is a record; this is done based on the class's record components. + * An exception is thrown if the properties are not configured in a valid manner. + * + * @param clazz the bean type (must be a Java record) + * @param components the class's record components + * @return the properties of the given bean type + */ + @NotNull List collectPropertiesForRecord(@NotNull Class clazz, + RecordComponent @NotNull [] components); } diff --git a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImpl.java b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImpl.java index d90033dc..bba30f5d 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImpl.java +++ b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImpl.java @@ -19,7 +19,6 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Set; import java.util.UUID; import java.util.stream.Collectors; @@ -27,66 +26,74 @@ /** * Creates all {@link BeanPropertyDescription} objects for a given class. *

- * This description factory returns property descriptions for all properties on a class - * for which a getter and setter is associated. Inherited properties are considered. + * This description factory returns property descriptions for all instance fields on a class, + * including fields on its parent. If a class has a field of the same name as the parent, the parent's + * field is ignored. *

- * This implementation supports {@link ExportName} and transient properties, declared - * with the {@code transient} keyword or by adding the {@link Ignore} annotation. + * This implementation supports @{@link ExportName} and transient properties, declared + * with the {@code transient} keyword or by adding the @{@link Ignore} annotation. */ public class BeanDescriptionFactoryImpl implements BeanDescriptionFactory { @Override - public @NotNull List createRecordProperties(@NotNull Class clazz, - RecordComponent @NotNull [] components) { + public @NotNull List collectPropertiesForRecord(@NotNull Class clazz, + RecordComponent @NotNull [] components) { Map instanceFieldsByName = FieldUtils.getAllFields(clazz) .filter(FieldUtils::isRegularInstanceField) .collect(FieldUtils.collectByName(false)); - List relevantFields = new ArrayList<>(); + List properties = new ArrayList<>(); for (RecordComponent component : components) { Field field = instanceFieldsByName.get(component.getName()); - if (field == null) { - throw new ConfigMeException("Record component '" + component.getName() + "' for " + clazz.getName() - + " does not have a field with the same name"); - } + validateFieldForRecord(clazz, component, field); BeanFieldPropertyDescription property = convert(field); - if (property == null) { - throw new ConfigMeException("Record component '" + component.getName() + "' for " + clazz.getName() - + " has a field defined to be ignored: this is not supported for records"); - } - relevantFields.add(property); + properties.add(property); } - return relevantFields; + + validateProperties(clazz, properties); + return properties; } /** - * Collects all properties available on the given class. + * Validates the component and its associated field for a class that is a record. * - * @param clazz the class to process - * @return properties of the class + * @param clazz the record type the component belongs to + * @param component the record component to validate + * @param field the field associated with the record (nullable) */ - public @NotNull List getAllProperties(@NotNull Class clazz) { + protected void validateFieldForRecord(@NotNull Class clazz, @NotNull RecordComponent component, + @Nullable Field field) { + if (field == null) { + throw new ConfigMeException("Record component '" + component.getName() + "' for " + clazz.getName() + + " does not have a field with the same name"); + } else if (isFieldIgnored(field)) { + throw new ConfigMeException("Record component '" + component.getName() + "' for " + clazz.getName() + + " has a field defined to be ignored: this is not supported for records"); + } + } + + @Override + public @NotNull List collectProperties(@NotNull Class clazz) { + @SuppressWarnings("checkstyle:IllegalType") // LinkedHashMap indicates the values are ordered (important here) LinkedHashMap instanceFieldsByName = FieldUtils.getAllFields(clazz) .filter(FieldUtils::isRegularInstanceField) .collect(FieldUtils.collectByName(false)); List properties = instanceFieldsByName.values().stream() + .filter(field -> !isFieldIgnored(field)) .map(this::convert) - .filter(Objects::nonNull) .collect(Collectors.toList()); validateProperties(clazz, properties); return properties; } - protected @Nullable BeanFieldPropertyDescription convert(@NotNull Field field) { - if (Modifier.isTransient(field.getModifiers()) || field.isAnnotationPresent(Ignore.class)) { - return null; - } + protected @NotNull BeanFieldPropertyDescription convert(@NotNull Field field) { + return new BeanFieldPropertyDescription(field, getCustomExportName(field), getComments(field)); + } - return new BeanFieldPropertyDescription(field, - getCustomExportName(field), - getComments(field)); + protected boolean isFieldIgnored(@NotNull Field field) { + return Modifier.isTransient(field.getModifiers()) || field.isAnnotationPresent(Ignore.class); } /** @@ -125,6 +132,13 @@ protected void validateProperties(@NotNull Class clazz, }); } + /** + * Returns a custom name that the property should have in property resources for reading and writing. This method + * returns null if the field name should be used. + * + * @param field the field to process + * @return the custom name the property has in resources, null otherwise + */ protected @Nullable String getCustomExportName(@NotNull Field field) { return field.isAnnotationPresent(ExportName.class) ? field.getAnnotation(ExportName.class).value() diff --git a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanFieldPropertyDescription.java b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanFieldPropertyDescription.java index aa246a38..9adab268 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanFieldPropertyDescription.java +++ b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanFieldPropertyDescription.java @@ -1,7 +1,7 @@ package ch.jalu.configme.beanmapper.propertydescription; import ch.jalu.configme.exception.ConfigMeException; -import ch.jalu.configme.internal.record.ReflectionHelper; +import ch.jalu.configme.internal.ReflectionHelper; import ch.jalu.typeresolver.FieldUtils; import ch.jalu.typeresolver.TypeInfo; import org.jetbrains.annotations.NotNull; @@ -18,6 +18,13 @@ public class BeanFieldPropertyDescription implements BeanPropertyDescription { private final String exportName; private final BeanPropertyComments comments; + /** + * Constructor. + * + * @param field the field this description is for + * @param exportName the custom name of this property in the property resource, null for default + * @param comments the comments associated with this property + */ public BeanFieldPropertyDescription(@NotNull Field field, @Nullable String exportName, @NotNull BeanPropertyComments comments) { @@ -28,9 +35,7 @@ public BeanFieldPropertyDescription(@NotNull Field field, @Override public @NotNull String getName() { - return exportName == null - ? field.getName() - : exportName; + return exportName == null ? field.getName() : exportName; } @Override @@ -44,7 +49,7 @@ public BeanFieldPropertyDescription(@NotNull Field field, /** * Sets the provided value to the field wrapped by this instance on the given bean. This method does not - * check whether the field is final; in some contexts (e.g. instantiation a record type), this method cannot + * check whether the field is final; in some contexts (e.g. instantiating a record type), this method should not * be called. * * @param bean the bean to set the value to diff --git a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescription.java b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescription.java index bde49cb9..07c2d372 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescription.java +++ b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanPropertyDescription.java @@ -5,11 +5,11 @@ import org.jetbrains.annotations.Nullable; /** - * Represents a property within a bean class, as used in - * {@link ch.jalu.configme.beanmapper.instantiation.BeanInstantiationService}. - * There, for instance, there is a {@link BeanDescriptionFactory} field responsible for creating bean descriptions. + * Represents a property within a bean class, as used by + * {@link ch.jalu.configme.beanmapper.instantiation.BeanInstantiation BeanInstantiation}. Bean property descriptions + * are provided by {@link BeanDescriptionFactory}. *

- * Default implementation is {@link BeanFieldPropertyDescription}. + * Default implementation of this interface is {@link BeanFieldPropertyDescription}. */ public interface BeanPropertyDescription { diff --git a/src/main/java/ch/jalu/configme/configurationdata/ConfigurationDataBuilder.java b/src/main/java/ch/jalu/configme/configurationdata/ConfigurationDataBuilder.java index 8074b197..2c250344 100644 --- a/src/main/java/ch/jalu/configme/configurationdata/ConfigurationDataBuilder.java +++ b/src/main/java/ch/jalu/configme/configurationdata/ConfigurationDataBuilder.java @@ -3,7 +3,7 @@ import ch.jalu.configme.Comment; import ch.jalu.configme.SettingsHolder; import ch.jalu.configme.exception.ConfigMeException; -import ch.jalu.configme.internal.record.ReflectionHelper; +import ch.jalu.configme.internal.ReflectionHelper; import ch.jalu.configme.properties.Property; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; diff --git a/src/main/java/ch/jalu/configme/internal/record/ReflectionHelper.java b/src/main/java/ch/jalu/configme/internal/ReflectionHelper.java similarity index 85% rename from src/main/java/ch/jalu/configme/internal/record/ReflectionHelper.java rename to src/main/java/ch/jalu/configme/internal/ReflectionHelper.java index 7e9f0d82..5b59696a 100644 --- a/src/main/java/ch/jalu/configme/internal/record/ReflectionHelper.java +++ b/src/main/java/ch/jalu/configme/internal/ReflectionHelper.java @@ -1,4 +1,4 @@ -package ch.jalu.configme.internal.record; +package ch.jalu.configme.internal; import ch.jalu.configme.exception.ConfigMeException; import ch.jalu.typeresolver.classutil.ClassUtils; @@ -41,7 +41,7 @@ public class ReflectionHelper { /** * Invokes the given method (with zero arguments) on the given {@code instance} object. A runtime exception is - * thrown if the method invocation failed. + * thrown if the method invocation failed. An exception is thrown if the return value is null. * * @param method the method to invoke * @param instance the object to invoke it on @@ -49,10 +49,13 @@ public class ReflectionHelper { * @return the return value of the method */ @SuppressWarnings("unchecked") - // TODO: @NotNull on return value not generically valid - revise? public @NotNull T invokeZeroArgMethod(@NotNull Method method, @Nullable Object instance) { try { - return (T) method.invoke(instance); + T result = (T) method.invoke(instance); + if (result == null) { // Should never happen; used to guarantee @NotNull, as per the method declaration + throw new IllegalStateException("Method '" + method + "' unexpectedly returned null"); + } + return result; } catch (ReflectiveOperationException e) { throw new ConfigMeException("Failed to call " + method + " for " + instance, e); } diff --git a/src/main/java/ch/jalu/configme/internal/record/RecordInspector.java b/src/main/java/ch/jalu/configme/internal/record/RecordInspector.java index e3597bb9..5e420601 100644 --- a/src/main/java/ch/jalu/configme/internal/record/RecordInspector.java +++ b/src/main/java/ch/jalu/configme/internal/record/RecordInspector.java @@ -3,85 +3,19 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import java.lang.reflect.Method; -import java.lang.reflect.Type; -import java.util.Arrays; - /** - * Inspects classes and returns Record information (Java 14+). The inspection is performed by reflection - * because ConfigMe is still compiled against Java 8. + * Utilities for Java records. They are abstracted by this type because ConfigMe is compiled with Java 8. */ -public class RecordInspector { - - private final ReflectionHelper reflectionHelper; - private Method isRecordMethod; // Class#isRecord - private Method getRecordComponentsMethod; // Class#getRecordComponents - - private Method getComponentNameMethod; // RecordComponent#getName - private Method getComponentTypeMethod; // RecordComponent#getType - private Method getComponentGenericTypeMethod; // RecordComponent#getGenericType - - public RecordInspector(@NotNull ReflectionHelper reflectionHelper) { - this.reflectionHelper = reflectionHelper; - } +public interface RecordInspector { /** - * Returns whether the given class is a record. + * Returns the components that make up the record. *

- * This method uses {@code Class#isRecord} in a way that is compatible with Java 8 and above. + * This calls {@link Class#getRecordComponents} in a way that is compatible with Java 8 and above. * - * @param clazz the class to inspect - * @return true if it's a record, false otherwise + * @param clazz the class whose record components should be returned + * @return the record's components, null if the class is not a record */ - public boolean isRecord(@NotNull Class clazz) { - // Check superclass to make sure that Class#isRecord will exist, and to avoid redundant reflective - // calls to the method if we can rule out records anyway - if (hasRecordAsSuperclass(clazz)) { - if (isRecordMethod == null) { - isRecordMethod = reflectionHelper.getZeroArgMethod(Class.class, "isRecord"); - } - return reflectionHelper.invokeZeroArgMethod(isRecordMethod, clazz); - } - return false; - } - - /** - * Returns the components that make up the record. This method should only be called after checking that - * the class is a record ({@link #isRecord(Class)}). - *

- * This calls {code Class#getRecordComponents} in a way that is compatible with Java 8 and above. - * - * @param clazz a record type whose components should be returned - * @return the record's components - */ - public RecordComponent @Nullable [] getRecordComponents(@NotNull Class clazz) { - if (getRecordComponentsMethod == null) { - getRecordComponentsMethod = reflectionHelper.getZeroArgMethod(Class.class, "getRecordComponents"); - } - - Object[] components = reflectionHelper.invokeZeroArgMethod(getRecordComponentsMethod, clazz); - if (getComponentGenericTypeMethod == null) { - Class recordComponentClass = reflectionHelper.getClassOrThrow("java.lang.reflect.RecordComponent"); - getComponentNameMethod = reflectionHelper.getZeroArgMethod(recordComponentClass, "getName"); - getComponentTypeMethod = reflectionHelper.getZeroArgMethod(recordComponentClass, "getType"); - getComponentGenericTypeMethod = reflectionHelper.getZeroArgMethod(recordComponentClass, "getGenericType"); - } - - return Arrays.stream(components) - .map(this::mapComponent) - .toArray(RecordComponent[]::new); - } - - boolean hasRecordAsSuperclass(@NotNull Class clazz) { - return clazz.getSuperclass() != null - && "java.lang.Record".equals(clazz.getSuperclass().getName()); - } - - private @NotNull RecordComponent mapComponent(@NotNull Object component) { - String name = reflectionHelper.invokeZeroArgMethod(getComponentNameMethod, component); - Class type = reflectionHelper.invokeZeroArgMethod(getComponentTypeMethod, component); - Type genericType = reflectionHelper.invokeZeroArgMethod(getComponentGenericTypeMethod, component); + RecordComponent @Nullable [] getRecordComponents(@NotNull Class clazz); - return new RecordComponent(name, type, genericType); - } } diff --git a/src/main/java/ch/jalu/configme/internal/record/RecordInspectorImpl.java b/src/main/java/ch/jalu/configme/internal/record/RecordInspectorImpl.java new file mode 100644 index 00000000..7d44dd5e --- /dev/null +++ b/src/main/java/ch/jalu/configme/internal/record/RecordInspectorImpl.java @@ -0,0 +1,83 @@ +package ch.jalu.configme.internal.record; + +import ch.jalu.configme.internal.ReflectionHelper; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.lang.reflect.Method; +import java.lang.reflect.Type; +import java.util.Arrays; + +/** + * Inspects classes and returns Record information (Java 14+). The inspection is performed by reflection + * because ConfigMe is still compiled against Java 8. + */ +public class RecordInspectorImpl implements RecordInspector { + + private final ReflectionHelper reflectionHelper; + private Method isRecordMethod; // Class#isRecord + private Method getRecordComponentsMethod; // Class#getRecordComponents + + private Method getComponentNameMethod; // RecordComponent#getName + private Method getComponentTypeMethod; // RecordComponent#getType + private Method getComponentGenericTypeMethod; // RecordComponent#getGenericType + + public RecordInspectorImpl(@NotNull ReflectionHelper reflectionHelper) { + this.reflectionHelper = reflectionHelper; + } + + /** + * Returns whether the given class is a record. + *

+ * This method uses {@code Class#isRecord} in a way that is compatible with Java 8 and above. + * + * @param clazz the class to inspect + * @return true if it's a record, false otherwise + */ + boolean isRecord(@NotNull Class clazz) { + // Check superclass to make sure that Class#isRecord will exist, and to avoid redundant reflective + // calls to the method if we can rule out records anyway + if (hasRecordAsSuperclass(clazz)) { + if (isRecordMethod == null) { + isRecordMethod = reflectionHelper.getZeroArgMethod(Class.class, "isRecord"); + } + return reflectionHelper.invokeZeroArgMethod(isRecordMethod, clazz); + } + return false; + } + + @Override + public RecordComponent @Nullable [] getRecordComponents(@NotNull Class clazz) { + if (!isRecord(clazz)) { + return null; + } + if (getRecordComponentsMethod == null) { + getRecordComponentsMethod = reflectionHelper.getZeroArgMethod(Class.class, "getRecordComponents"); + } + + Object[] components = reflectionHelper.invokeZeroArgMethod(getRecordComponentsMethod, clazz); + if (getComponentGenericTypeMethod == null) { + Class recordComponentClass = reflectionHelper.getClassOrThrow("java.lang.reflect.RecordComponent"); + getComponentNameMethod = reflectionHelper.getZeroArgMethod(recordComponentClass, "getName"); + getComponentTypeMethod = reflectionHelper.getZeroArgMethod(recordComponentClass, "getType"); + getComponentGenericTypeMethod = reflectionHelper.getZeroArgMethod(recordComponentClass, "getGenericType"); + } + + return Arrays.stream(components) + .map(this::mapComponent) + .toArray(RecordComponent[]::new); + } + + boolean hasRecordAsSuperclass(@NotNull Class clazz) { + return clazz.getSuperclass() != null + && "java.lang.Record".equals(clazz.getSuperclass().getName()); + } + + private @NotNull RecordComponent mapComponent(@NotNull Object component) { + String name = reflectionHelper.invokeZeroArgMethod(getComponentNameMethod, component); + Class type = reflectionHelper.invokeZeroArgMethod(getComponentTypeMethod, component); + Type genericType = reflectionHelper.invokeZeroArgMethod(getComponentGenericTypeMethod, component); + + return new RecordComponent(name, type, genericType); + } +} diff --git a/src/test/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationServiceImplTest.java b/src/test/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationServiceImplTest.java index c7a2f8e2..82a16dce 100644 --- a/src/test/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationServiceImplTest.java +++ b/src/test/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationServiceImplTest.java @@ -22,12 +22,12 @@ import java.util.concurrent.TimeUnit; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.sameInstance; -import static org.mockito.ArgumentMatchers.any; import static org.mockito.BDDMockito.given; /** @@ -52,8 +52,6 @@ void createBeanInstantiationService() { @Test void shouldProvideInstantiationForRecord() throws NoSuchFieldException { // given - given(recordInspector.isRecord(FakeRecord.class)).willReturn(true); - RecordComponent nameComponent = new RecordComponent("name", String.class, String.class); RecordComponent shoeSizeComponent = new RecordComponent("shoeSize", int.class, int.class); RecordComponent ageComponent = new RecordComponent("age", double.class, double.class); @@ -65,7 +63,7 @@ void shouldProvideInstantiationForRecord() throws NoSuchFieldException { new BeanFieldPropertyDescription(FakeRecord.class.getDeclaredField("name"), null, BeanPropertyComments.EMPTY), new BeanFieldPropertyDescription(FakeRecord.class.getDeclaredField("shoeSize"), null, BeanPropertyComments.EMPTY), new BeanFieldPropertyDescription(FakeRecord.class.getDeclaredField("age"), null, BeanPropertyComments.EMPTY)); - given(beanDescriptionFactory.createRecordProperties(FakeRecord.class, components)).willReturn(beanProperties); + given(beanDescriptionFactory.collectPropertiesForRecord(FakeRecord.class, components)).willReturn(beanProperties); // when Optional instantiation = beanInstantiationService.findInstantiation(FakeRecord.class); @@ -85,12 +83,12 @@ void shouldProvideInstantiationForRecord() throws NoSuchFieldException { @Test void shouldProvideInstantiationForZeroArgConstructorClass() throws NoSuchFieldException { // given - given(recordInspector.isRecord(SampleBean.class)).willReturn(false); + given(recordInspector.getRecordComponents(SampleBean.class)).willReturn(null); List beanProperties = Arrays.asList( new BeanFieldPropertyDescription(SampleBean.class.getDeclaredField("name"), null, BeanPropertyComments.EMPTY), new BeanFieldPropertyDescription(SampleBean.class.getDeclaredField("shoeSize"), null, BeanPropertyComments.EMPTY), new BeanFieldPropertyDescription(SampleBean.class.getDeclaredField("age"), null, BeanPropertyComments.EMPTY)); - given(beanDescriptionFactory.getAllProperties(SampleBean.class)).willReturn(beanProperties); + given(beanDescriptionFactory.collectProperties(SampleBean.class)).willReturn(beanProperties); // when Optional instantiation = beanInstantiationService.findInstantiation(SampleBean.class); @@ -109,10 +107,7 @@ void shouldProvideInstantiationForZeroArgConstructorClass() throws NoSuchFieldEx @Test void shouldReturnEmptyOptionalForClassesWithNoInstantiationMethod() { - // given - given(recordInspector.isRecord(any(Class.class))).willReturn(false); - - // when + // given / when Optional result1 = beanInstantiationService.findInstantiation(String.class); Optional result2 = beanInstantiationService.findInstantiation(TimeUnit.class); Optional result3 = beanInstantiationService.findInstantiation(StringProperty.class); @@ -127,8 +122,6 @@ void shouldReturnEmptyOptionalForClassesWithNoInstantiationMethod() { void shouldCacheInstantiations() throws NoSuchFieldException { // given // Set up record instantiation - given(recordInspector.isRecord(FakeRecord.class)).willReturn(true); - RecordComponent ageComponent = new RecordComponent("age", double.class, double.class); RecordComponent[] components = {ageComponent}; given(recordInspector.getRecordComponents(FakeRecord.class)).willReturn(components); @@ -136,15 +129,15 @@ void shouldCacheInstantiations() throws NoSuchFieldException { Field recordAgeField = FakeRecord.class.getDeclaredField("age"); BeanPropertyComments recordAgeComments = new BeanPropertyComments(Arrays.asList("some", "comment"), UUID.randomUUID()); BeanFieldPropertyDescription recordAgeProperty = new BeanFieldPropertyDescription(recordAgeField, null, recordAgeComments); - given(beanDescriptionFactory.createRecordProperties(FakeRecord.class, components)).willReturn(Collections.singletonList(recordAgeProperty)); + given(beanDescriptionFactory.collectPropertiesForRecord(FakeRecord.class, components)).willReturn(Collections.singletonList(recordAgeProperty)); // Set up zero-args constructor instantiation - given(recordInspector.isRecord(SampleBean.class)).willReturn(false); + given(recordInspector.getRecordComponents(SampleBean.class)).willReturn(null); Field beanNameField = SampleBean.class.getDeclaredField("name"); BeanPropertyComments beanNameComments = new BeanPropertyComments(Collections.singletonList("comment"), UUID.randomUUID()); BeanFieldPropertyDescription beanNameProperty = new BeanFieldPropertyDescription(beanNameField, null, beanNameComments); - given(beanDescriptionFactory.getAllProperties(SampleBean.class)).willReturn(Collections.singletonList(beanNameProperty)); + given(beanDescriptionFactory.collectProperties(SampleBean.class)).willReturn(Collections.singletonList(beanNameProperty)); // when Optional recordInstantiation1 = beanInstantiationService.findInstantiation(FakeRecord.class); @@ -160,6 +153,19 @@ void shouldCacheInstantiations() throws NoSuchFieldException { assertThat(zeroArgsInstantiation1.get(), sameInstance(zeroArgsInstantiation2.get())); assertThat(zeroArgsInstantiation1.get().getProperties().get(0).getComments().getUuid(), equalTo(beanNameComments.getUuid())); assertThat(zeroArgsInstantiation2.get().getProperties().get(0).getComments().getUuid(), equalTo(beanNameComments.getUuid())); + + assertThat(beanInstantiationService.getCachedInstantiationsByType().keySet(), containsInAnyOrder(FakeRecord.class, SampleBean.class)); + } + + @Test + void shouldReturnFields() { + // given / when + RecordInspector returnedRecordInspector = beanInstantiationService.getRecordInspector(); + BeanDescriptionFactory returnedBeanDescriptionFactory = beanInstantiationService.getBeanDescriptionFactory(); + + // then + assertThat(returnedRecordInspector, sameInstance(recordInspector)); + assertThat(returnedBeanDescriptionFactory, sameInstance(beanDescriptionFactory)); } private static final class FakeRecord { // #347: Change to record when the Java version allows it @@ -188,7 +194,7 @@ private static final class SampleBean { private int shoeSize; private double age; - public SampleBean() { + private SampleBean() { } } } diff --git a/src/test/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstructorInstantiationTest.java b/src/test/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstructorInstantiationTest.java index e89c4204..074a3c7e 100644 --- a/src/test/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstructorInstantiationTest.java +++ b/src/test/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstructorInstantiationTest.java @@ -9,6 +9,7 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.junit.jupiter.MockitoExtension; +import java.lang.reflect.InvocationTargetException; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -76,10 +77,10 @@ void shouldThrowForValueThatDoesNotMatchField() { } @Test - void shouldThrowForPrivateConstructor() throws NoSuchMethodException { + void shouldPropagateExceptionInConstructor() throws NoSuchMethodException { // given BeanZeroArgConstructorInstantiation instantiation = new BeanZeroArgConstructorInstantiation( - BeanWithPrivateConstructor.class.getDeclaredConstructor(), + BeanWithThrowingConstructor.class.getDeclaredConstructor(), Collections.emptyList()); ConvertErrorRecorder errorRecorder = new ConvertErrorRecorder(); @@ -88,8 +89,8 @@ void shouldThrowForPrivateConstructor() throws NoSuchMethodException { () -> instantiation.create(Collections.emptyList(), errorRecorder)); // then - assertThat(ex.getMessage(), equalTo("Failed to call constructor for class ch.jalu.configme.beanmapper.instantiation.BeanZeroArgConstructorInstantiationTest$BeanWithPrivateConstructor")); - assertThat(ex.getCause(), instanceOf(IllegalAccessException.class)); + assertThat(ex.getMessage(), equalTo("Failed to call constructor for class ch.jalu.configme.beanmapper.instantiation.BeanZeroArgConstructorInstantiationTest$BeanWithThrowingConstructor")); + assertThat(ex.getCause(), instanceOf(InvocationTargetException.class)); } @Test @@ -168,9 +169,10 @@ static BeanZeroArgConstructorInstantiation createInstantiation() { } } - private static final class BeanWithPrivateConstructor { + private static final class BeanWithThrowingConstructor { - private BeanWithPrivateConstructor() { + private BeanWithThrowingConstructor() { + throw new IllegalStateException("Yikers"); } } diff --git a/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImplTest.java b/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImplTest.java index c8f62b3f..3611b4d5 100644 --- a/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImplTest.java +++ b/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImplTest.java @@ -40,7 +40,7 @@ class BeanDescriptionFactoryImplTest { @Test void shouldReturnWritableProperties() { // given / when - Collection descriptions = factory.getAllProperties(SampleBean.class); + Collection descriptions = factory.collectProperties(SampleBean.class); // then assertThat(descriptions, hasSize(4)); @@ -66,13 +66,13 @@ void shouldReturnWritableProperties() { @Test void shouldReturnEmptyListForNonBeanClass() { // given / when / then - assertThat(factory.getAllProperties(List.class), empty()); + assertThat(factory.collectProperties(List.class), empty()); } @Test void shouldNotConsiderTransientFields() { // given / when - Collection properties = factory.getAllProperties(BeanWithTransientFields.class); + Collection properties = factory.collectProperties(BeanWithTransientFields.class); // then assertThat(properties, hasSize(2)); @@ -82,7 +82,7 @@ void shouldNotConsiderTransientFields() { @Test void shouldBeAwareOfInheritanceAndRespectOrder() { // given / when - Collection properties = factory.getAllProperties(Middle.class); + Collection properties = factory.collectProperties(Middle.class); // then assertThat(properties, hasSize(3)); @@ -92,7 +92,7 @@ void shouldBeAwareOfInheritanceAndRespectOrder() { @Test void shouldLetChildFieldsOverrideParentFields() { // given / when - Collection properties = factory.getAllProperties(Child.class); + Collection properties = factory.collectProperties(Child.class); // then assertThat(properties, hasSize(5)); @@ -103,7 +103,7 @@ void shouldLetChildFieldsOverrideParentFields() { @Test void shouldUseExportName() { // given / when - Collection properties = factory.getAllProperties(AnnotatedEntry.class); + Collection properties = factory.collectProperties(AnnotatedEntry.class); // then assertThat(properties, hasSize(2)); @@ -115,7 +115,7 @@ void shouldUseExportName() { void shouldThrowForMultiplePropertiesWithSameName() { // given / when ConfigMeMapperException ex = assertThrows(ConfigMeMapperException.class, - () -> factory.getAllProperties(BeanWithNameClash.class)); + () -> factory.collectProperties(BeanWithNameClash.class)); // then assertThat(ex.getMessage(), @@ -126,7 +126,7 @@ void shouldThrowForMultiplePropertiesWithSameName() { void shouldThrowForWhenExportNameIsNullForProperty() { // given / when ConfigMeMapperException ex = assertThrows(ConfigMeMapperException.class, - () -> factory.getAllProperties(BeanWithEmptyName.class)); + () -> factory.collectProperties(BeanWithEmptyName.class)); // then assertThat(ex.getMessage(), @@ -136,7 +136,7 @@ void shouldThrowForWhenExportNameIsNullForProperty() { @Test void shouldReturnCommentsWithoutUuid() { // given / when - Collection execDetailsProperties = factory.getAllProperties(ExecutionDetails.class); + Collection execDetailsProperties = factory.collectProperties(ExecutionDetails.class); // then BeanPropertyComments executorComments = getDescription("executor", execDetailsProperties).getComments(); @@ -150,7 +150,7 @@ void shouldReturnCommentsWithoutUuid() { @Test void shouldPickUpCustomNameFromField() { // given / when - List properties = new ArrayList<>(factory.getAllProperties(BeanWithExportName.class)); + List properties = new ArrayList<>(factory.collectProperties(BeanWithExportName.class)); // then assertThat(properties, hasSize(3)); @@ -165,7 +165,7 @@ void shouldPickUpCustomNameFromField() { @Test void shouldPickUpCustomNameFromFieldsIncludingInheritance() { // given / when - List properties = new ArrayList<>(factory.getAllProperties(BeanWithExportNameExtension.class)); + List properties = new ArrayList<>(factory.collectProperties(BeanWithExportNameExtension.class)); // then assertThat(properties, hasSize(4)); diff --git a/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanFieldPropertyDescriptionTest.java b/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanFieldPropertyDescriptionTest.java index 3a214847..b29b3f11 100644 --- a/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanFieldPropertyDescriptionTest.java +++ b/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanFieldPropertyDescriptionTest.java @@ -67,7 +67,7 @@ void shouldHandlePropertyGetError() { void shouldHaveAppropriateStringRepresentation() { // given Collection properties = new BeanDescriptionFactoryImpl() - .getAllProperties(AnnotatedEntry.class); + .collectProperties(AnnotatedEntry.class); BeanPropertyDescription hasIdProperty = properties.stream() .filter(prop -> "has-id".equals(prop.getName())).findFirst().get(); @@ -79,7 +79,7 @@ void shouldHaveAppropriateStringRepresentation() { } private static BeanFieldPropertyDescription getDescriptor(String name, Class clazz) { - return new BeanDescriptionFactoryImpl().getAllProperties(clazz) + return new BeanDescriptionFactoryImpl().collectProperties(clazz) .stream() .filter(prop -> name.equals(prop.getName())) .findFirst() diff --git a/src/test/java/ch/jalu/configme/demo/beans/UserBase.java b/src/test/java/ch/jalu/configme/demo/beans/UserBase.java index a5d32847..ae47332f 100644 --- a/src/test/java/ch/jalu/configme/demo/beans/UserBase.java +++ b/src/test/java/ch/jalu/configme/demo/beans/UserBase.java @@ -46,6 +46,9 @@ public void setVersion(double version) { this.version = version; } + public int getBuild() { + return build; + } public void setBuild(int build) { this.build = build; diff --git a/src/test/java/ch/jalu/configme/internal/record/ReflectionHelperTest.java b/src/test/java/ch/jalu/configme/internal/ReflectionHelperTest.java similarity index 86% rename from src/test/java/ch/jalu/configme/internal/record/ReflectionHelperTest.java rename to src/test/java/ch/jalu/configme/internal/ReflectionHelperTest.java index 7fda4f51..c7056e0f 100644 --- a/src/test/java/ch/jalu/configme/internal/record/ReflectionHelperTest.java +++ b/src/test/java/ch/jalu/configme/internal/ReflectionHelperTest.java @@ -1,4 +1,4 @@ -package ch.jalu.configme.internal.record; +package ch.jalu.configme.internal; import ch.jalu.configme.exception.ConfigMeException; import ch.jalu.configme.properties.Property; @@ -10,6 +10,7 @@ import java.lang.reflect.Field; import java.lang.reflect.Method; import java.util.HashMap; +import java.util.function.Supplier; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; @@ -84,17 +85,31 @@ void shouldCallMethod() throws NoSuchMethodException { @Test void shouldWrapExceptionIfCallingMethodFails() throws NoSuchMethodException { // given - Method toStringMethod = HashMap.class.getDeclaredMethod("resize"); + Method privateResizeMethod = HashMap.class.getDeclaredMethod("resize"); // when ConfigMeException ex = assertThrows(ConfigMeException.class, - () -> reflectionHelper.invokeZeroArgMethod(toStringMethod, new HashMap<>())); + () -> reflectionHelper.invokeZeroArgMethod(privateResizeMethod, new HashMap<>())); // then assertThat(ex.getMessage(), equalTo("Failed to call final java.util.HashMap$Node[] java.util.HashMap.resize() for {}")); assertThat(ex.getCause(), instanceOf(IllegalAccessException.class)); } + @Test + void shouldThrowForNullReturnValue() throws NoSuchMethodException { + // given + Supplier supplier = () -> null; + Method supplierGetMethod = Supplier.class.getDeclaredMethod("get"); + + // when + IllegalStateException ex = assertThrows(IllegalStateException.class, + () -> reflectionHelper.invokeZeroArgMethod(supplierGetMethod, supplier)); + + // then + assertThat(ex.getMessage(), equalTo("Method 'public abstract java.lang.Object java.util.function.Supplier.get()' unexpectedly returned null")); + } + @Test void shouldMakeFieldAccessible() throws NoSuchFieldException { // given diff --git a/src/test/java/ch/jalu/configme/internal/record/RecordInspectorTest.java b/src/test/java/ch/jalu/configme/internal/record/RecordInspectorImplTest.java similarity index 80% rename from src/test/java/ch/jalu/configme/internal/record/RecordInspectorTest.java rename to src/test/java/ch/jalu/configme/internal/record/RecordInspectorImplTest.java index b6f4d0d3..aebc66d7 100644 --- a/src/test/java/ch/jalu/configme/internal/record/RecordInspectorTest.java +++ b/src/test/java/ch/jalu/configme/internal/record/RecordInspectorImplTest.java @@ -1,5 +1,6 @@ package ch.jalu.configme.internal.record; +import ch.jalu.configme.internal.ReflectionHelper; import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -11,7 +12,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.arrayWithSize; import static org.hamcrest.Matchers.equalTo; -import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; @@ -20,21 +21,21 @@ import static org.mockito.Mockito.verifyNoInteractions; /** - * Test for {@link RecordInspector}. + * Test for {@link RecordInspectorImpl}. */ @ExtendWith(MockitoExtension.class) -class RecordInspectorTest { +class RecordInspectorImplTest { @Test - void shouldCheckIfClassExtendsRecord() { + void shouldNotPerformReflectiveOperationIfClassDoesNotExtendRecord() { // given ReflectionHelper reflectionHelper = mock(ReflectionHelper.class); - RecordInspector recordInspector = new RecordInspector(reflectionHelper); + RecordInspectorImpl recordInspector = new RecordInspectorImpl(reflectionHelper); // when / then - assertFalse(recordInspector.isRecord(Object.class)); - assertFalse(recordInspector.isRecord(Integer.class)); - assertFalse(recordInspector.isRecord(int.class)); + assertNull(recordInspector.getRecordComponents(Object.class)); + assertNull(recordInspector.getRecordComponents(Integer.class)); + assertNull(recordInspector.getRecordComponents(int.class)); verifyNoInteractions(reflectionHelper); } @@ -46,7 +47,7 @@ void shouldReturnThatClassIsRecord() throws NoSuchMethodException { given(reflectionHelper.getZeroArgMethod(Class.class, "isRecord")).willReturn(isPrimitiveMethod); given(reflectionHelper.invokeZeroArgMethod(any(Method.class), any(Class.class))).willCallRealMethod(); - RecordInspector recordInspector = new RecordInspector(reflectionHelper) { + RecordInspectorImpl recordInspector = new RecordInspectorImpl(reflectionHelper) { @Override public boolean hasRecordAsSuperclass(@NotNull Class clazz) { return true; @@ -63,6 +64,10 @@ void shouldReturnComponents() throws NoSuchMethodException { // given ReflectionHelper reflectionHelper = mock(ReflectionHelper.class); + // Use Class#isMemberClass instead of Class#isRecord -> will be true for FakeRecordComponent.class + Method isMemberClassMethod = Class.class.getDeclaredMethod("isMemberClass"); + given(reflectionHelper.getZeroArgMethod(Class.class, "isRecord")).willReturn(isMemberClassMethod); + // Instead of Class#getRecordComponents, return method for FakeRecord#getComponents Method fakeGetComponentsMethod = FakeRecordType.class.getDeclaredMethod("getComponents"); given(reflectionHelper.getZeroArgMethod(Class.class, "getRecordComponents")) @@ -78,7 +83,7 @@ void shouldReturnComponents() throws NoSuchMethodException { .willCallRealMethod(); given(reflectionHelper.invokeZeroArgMethod(any(Method.class), any(Object.class))).willCallRealMethod(); - RecordInspector recordInspector = new RecordInspector(reflectionHelper) { + RecordInspectorImpl recordInspector = new RecordInspectorImpl(reflectionHelper) { @Override public boolean hasRecordAsSuperclass(@NotNull Class clazz) { return true; @@ -98,7 +103,7 @@ public boolean hasRecordAsSuperclass(@NotNull Class clazz) { assertThat(components[1].getGenericType(), equalTo(Object.class)); } - private static final class FakeRecordType { + public static final class FakeRecordType { public static FakeRecordComponent[] getComponents() { FakeRecordComponent component1 = new FakeRecordComponent("age", int.class, int.class); @@ -107,7 +112,7 @@ public static FakeRecordComponent[] getComponents() { } } - private static final class FakeRecordComponent { + public static final class FakeRecordComponent { private final String name; private final Class type; From c4af00ceb69ad1f4cbf06854f71445549b2328f0 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Mon, 8 Jan 2024 21:37:47 +0100 Subject: [PATCH 08/16] Small Javadoc revisions --- .../instantiation/BeanInstantiationServiceImpl.java | 4 ++-- .../propertydescription/BeanDescriptionFactory.java | 2 +- .../ch/jalu/configme/internal/record/RecordInspector.java | 1 + 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationServiceImpl.java b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationServiceImpl.java index 609dcebc..21ecbbd8 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationServiceImpl.java +++ b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationServiceImpl.java @@ -20,12 +20,12 @@ * Default implementation of {@link BeanInstantiationService}: defines how bean classes can be created. *

* This service can handle two different types of classes as beans:

    - *
  • Regular Java classes with a public no-args constructor: all non-static, non-transient fields + *
  • Regular Java classes with a no-args constructor: all fields that aren't static or transient * will be considered as bean properties.
    • *
  • Java records
    • *
* - * See {@link BeanDescriptionFactory} for details on how properties of a bean class are determined. + * See {@link BeanDescriptionFactory} for details on how the properties are determined for a bean class. */ public class BeanInstantiationServiceImpl implements BeanInstantiationService { diff --git a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactory.java b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactory.java index 5f6c47ae..73407404 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactory.java +++ b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactory.java @@ -6,7 +6,7 @@ import java.util.List; /** - * Factory which analyzes a class and returns all writable properties. + * Factory which analyzes a class and returns all properties which ConfigMe should consider. *

* Default implementation: {@link BeanDescriptionFactoryImpl}. */ diff --git a/src/main/java/ch/jalu/configme/internal/record/RecordInspector.java b/src/main/java/ch/jalu/configme/internal/record/RecordInspector.java index 5e420601..edb79097 100644 --- a/src/main/java/ch/jalu/configme/internal/record/RecordInspector.java +++ b/src/main/java/ch/jalu/configme/internal/record/RecordInspector.java @@ -5,6 +5,7 @@ /** * Utilities for Java records. They are abstracted by this type because ConfigMe is compiled with Java 8. + * This class will be removed once ConfigMe upgrades to Java {@code >= 14}. */ public interface RecordInspector { From c5f81dd2b63f4889affa0c6c820182ec9ce3d86e Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 5 Oct 2024 10:26:29 +0200 Subject: [PATCH 09/16] Update for typeresolver changes --- .../BeanInstantiationServiceImpl.java | 24 ++++--------------- .../BeanDescriptionFactoryImpl.java | 2 +- .../BeanFieldPropertyDescription.java | 2 +- 3 files changed, 6 insertions(+), 22 deletions(-) diff --git a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationServiceImpl.java b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationServiceImpl.java index 21ecbbd8..2b130fb1 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationServiceImpl.java +++ b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationServiceImpl.java @@ -7,6 +7,7 @@ import ch.jalu.configme.internal.record.RecordComponent; import ch.jalu.configme.internal.record.RecordInspector; import ch.jalu.configme.internal.record.RecordInspectorImpl; +import ch.jalu.typeresolver.reflect.ConstructorUtils; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -75,34 +76,17 @@ public BeanInstantiationServiceImpl(@NotNull RecordInspector recordInspector, return new BeanRecordInstantiation(clazz, properties); } - Optional> zeroArgConstructor = tryFindConstructor(clazz); - if (zeroArgConstructor.isPresent()) { + Constructor zeroArgConstructor = ConstructorUtils.getConstructorOrNull(clazz); + if (zeroArgConstructor != null) { List properties = beanDescriptionFactory.collectProperties(clazz); if (!properties.isEmpty()) { - return new BeanZeroArgConstructorInstantiation(zeroArgConstructor.get(), properties); + return new BeanZeroArgConstructorInstantiation(zeroArgConstructor, properties); } } return null; } - /** - * Returns an optional with the constructor on the given {@code declarer} matching the parameter types, - * otherwise an empty optional. - * - * @param declarer the class to search for constructors - * @param parameterTypes the parameter types of the desired constructor - * @return optional with the constructor if found, empty optional otherwise - */ - protected static @NotNull Optional> tryFindConstructor(@NotNull Class declarer, - Class @NotNull ... parameterTypes) { - try { - return Optional.of(declarer.getDeclaredConstructor(parameterTypes)); - } catch (NoSuchMethodException ignore) { - return Optional.empty(); - } - } - protected final @NotNull RecordInspector getRecordInspector() { return recordInspector; } diff --git a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImpl.java b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImpl.java index bba30f5d..8507d31f 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImpl.java +++ b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImpl.java @@ -6,7 +6,7 @@ import ch.jalu.configme.beanmapper.Ignore; import ch.jalu.configme.exception.ConfigMeException; import ch.jalu.configme.internal.record.RecordComponent; -import ch.jalu.typeresolver.FieldUtils; +import ch.jalu.typeresolver.reflect.FieldUtils; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; diff --git a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanFieldPropertyDescription.java b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanFieldPropertyDescription.java index 9adab268..0b6cb2c3 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanFieldPropertyDescription.java +++ b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanFieldPropertyDescription.java @@ -2,8 +2,8 @@ import ch.jalu.configme.exception.ConfigMeException; import ch.jalu.configme.internal.ReflectionHelper; -import ch.jalu.typeresolver.FieldUtils; import ch.jalu.typeresolver.TypeInfo; +import ch.jalu.typeresolver.reflect.FieldUtils; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; From bd5f2a7278a403f24e674c836b91a86b698a5f6d Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 5 Oct 2024 21:39:23 +0200 Subject: [PATCH 10/16] Remove snapshot version of JavaTypeResolver --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index c43cf6e5..3967283a 100644 --- a/pom.xml +++ b/pom.xml @@ -193,7 +193,7 @@ ch.jalu typeresolver - 0.2.0-SNAPSHOT + 0.2.0 From c033256f9a3a8e00f35abda061541fc235195f7d Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sun, 6 Oct 2024 11:14:21 +0200 Subject: [PATCH 11/16] Update JavaDocs (no longer mention JavaBeans) --- .../ch/jalu/configme/beanmapper/Mapper.java | 8 ++-- .../jalu/configme/beanmapper/MapperImpl.java | 40 +++++++++---------- .../BeanRecordInstantiation.java | 7 +++- .../configme/properties/BeanProperty.java | 3 +- .../properties/PropertyInitializer.java | 2 +- ...anZeroArgConstructorInstantiationTest.java | 2 +- .../ch/jalu/configme/demo/beans/README.md | 6 +-- 7 files changed, 36 insertions(+), 32 deletions(-) diff --git a/src/main/java/ch/jalu/configme/beanmapper/Mapper.java b/src/main/java/ch/jalu/configme/beanmapper/Mapper.java index 74624f4c..18b819e1 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/Mapper.java +++ b/src/main/java/ch/jalu/configme/beanmapper/Mapper.java @@ -6,8 +6,8 @@ import org.jetbrains.annotations.Nullable; /** - * Creates JavaBeans based on the values coming from a property reader. See the JavaDoc on the default implementation, - * {@link MapperImpl}, for more details. + * Maps values of a property reader to Java classes (referred to as beans). See the JavaDoc on the default + * implementation, {@link MapperImpl}, for more details. */ public interface Mapper { @@ -42,9 +42,9 @@ public interface Mapper { } /** - * Converts a complex type such as a JavaBean object to simple types suitable for exporting. This method + * Converts a complex type such as a bean to simple types suitable for exporting. This method * typically returns a Map of values, or simple types like String / Number for scalar values. - * Used in the {@link ch.jalu.configme.properties.BeanProperty#toExportValue} method. + * Used by {@link ch.jalu.configme.properties.types.BeanPropertyType#toExportValue}. * * @param object the object to convert to its export value * @return export value to use diff --git a/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java b/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java index cbf153dc..9aae02ba 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java +++ b/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java @@ -4,12 +4,11 @@ import ch.jalu.configme.beanmapper.context.ExportContextImpl; import ch.jalu.configme.beanmapper.context.MappingContext; import ch.jalu.configme.beanmapper.context.MappingContextImpl; -import ch.jalu.configme.beanmapper.instantiation.BeanInstantiationServiceImpl; import ch.jalu.configme.beanmapper.instantiation.BeanInstantiation; import ch.jalu.configme.beanmapper.instantiation.BeanInstantiationService; +import ch.jalu.configme.beanmapper.instantiation.BeanInstantiationServiceImpl; import ch.jalu.configme.beanmapper.leafvaluehandler.LeafValueHandler; import ch.jalu.configme.beanmapper.leafvaluehandler.LeafValueHandlerImpl; -import ch.jalu.configme.beanmapper.leafvaluehandler.MapperLeafType; import ch.jalu.configme.beanmapper.propertydescription.BeanPropertyComments; import ch.jalu.configme.beanmapper.propertydescription.BeanPropertyDescription; import ch.jalu.configme.properties.convertresult.ConvertErrorRecorder; @@ -34,33 +33,34 @@ import static ch.jalu.configme.internal.PathUtils.pathSpecifierForMapKey; /** - * Implementation of {@link Mapper}. + * Default implementation of {@link Mapper}. *

- * Maps a section of a property resource to the provided JavaBean class. The mapping is based on the bean's properties, - * whose names must correspond with the names in the property resource. For example, if a JavaBean class has a property - * {@code length} and should be mapped from the property resource's value at path {@code definition}, the mapper will - * look up {@code definition.length} to get the value of the JavaBean property. + * Maps a section of a property resource to the provided Java class (called a "bean" type). The mapping is based on the + * bean's properties, whose names must correspond with the names in the property resource. For example, if a bean class + * has a property {@code length} and should be mapped from the property resource's value at path {@code definition}, + * the mapper will look up {@code definition.length} to get the value of the bean property. *

- * Classes must be JavaBeans. These are simple classes with private fields, accompanied by getters and setters. - * The mapper only considers properties which have both a getter and a setter method. Any Java class without - * at least one property with both a getter and a setter is not considered as a JavaBean class. Such classes can - * be supported by implementing a custom {@link MapperLeafType} that performs the conversion from the value coming - * from the property reader to an object of the class's type. + * Classes are created by the {@link BeanInstantiationService}. The {@link BeanInstantiationServiceImpl + * default implementation} supports Java classes with a zero-args constructor, as well as Java records. The service can + * be extended to support more types of classes. + *
For Java classes with a zero-args constructor, the class's private fields are taken as properties. The perceived + * properties can be modified with {@link ExportName} and {@link Ignore}. *

- * Recursion: the mapping of values to a JavaBean is performed recursively, i.e. a JavaBean may have other - * JavaBeans as fields and generic types at any arbitrary "depth". + * Recursion: the mapping of values to a bean is performed recursively, i.e. a bean may have other beans + * as fields and generic types at any arbitrary "depth". *

- * Collections are only supported if they are explicitly typed, i.e. a field of {@code List} + * Collections are only supported if they have an explicit type argument, i.e. a field of {@code List} * is supported but {@code List} and {@code List} are not supported. Specifically, you may * only declare fields of type {@link java.util.List} or {@link java.util.Set}, or a parent type ({@link Collection} - * or {@link Iterable}). + * or {@link Iterable}) by default. * Fields of type Map are supported also, with similar limitations. Additionally, maps may only have * {@code String} as key type, but no restrictions are imposed on the value type. *

- * JavaBeans may have optional fields. If the mapper cannot map the property resource value to the corresponding + * Beans may have optional fields. If the mapper cannot map the property resource value to the corresponding * field, it only treats it as a failure if the field's value is {@code null}. If the field has a default value assigned - * to it on initialization, the default value remains and the mapping process continues. A JavaBean field whose value is - * {@code null} signifies a failure and stops the mapping process immediately. + * to it on initialization, the default value remains and the mapping process continues. If a bean is created with a + * null property, the mapping process is stopped immediately. + *
Optional properties can also be defined by declaring them with {@link Optional}. */ public class MapperImpl implements Mapper { @@ -367,7 +367,7 @@ public MapperImpl(@NotNull BeanInstantiationService beanInstantiationService, // -- Bean /** - * Converts the provided value to the requested JavaBeans class if possible. + * Converts the provided value to the requested bean class if possible. * * @param context mapping context (incl. desired type) * @param value the value from the property resource diff --git a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanRecordInstantiation.java b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanRecordInstantiation.java index ec1e979b..86ca8bfa 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanRecordInstantiation.java +++ b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanRecordInstantiation.java @@ -4,6 +4,7 @@ import ch.jalu.configme.beanmapper.propertydescription.BeanFieldPropertyDescription; import ch.jalu.configme.exception.ConfigMeException; import ch.jalu.configme.properties.convertresult.ConvertErrorRecorder; +import ch.jalu.typeresolver.reflect.ConstructorUtils; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -29,8 +30,10 @@ public class BeanRecordInstantiation implements BeanInstantiation { public BeanRecordInstantiation(@NotNull Class clazz, @NotNull List properties) { this.properties = properties; Class[] recordTypes = properties.stream().map(BeanFieldPropertyDescription::getType).toArray(Class[]::new); - this.canonicalConstructor = BeanInstantiationServiceImpl.tryFindConstructor(clazz, recordTypes) - .orElseThrow(() -> new ConfigMeException("Could not get canonical constructor of " + clazz)); + this.canonicalConstructor = ConstructorUtils.getConstructorOrNull(clazz, recordTypes); + if (this.canonicalConstructor == null) { + throw new ConfigMeException("Could not get canonical constructor of " + clazz); + } } @Override diff --git a/src/main/java/ch/jalu/configme/properties/BeanProperty.java b/src/main/java/ch/jalu/configme/properties/BeanProperty.java index e50d193a..b2fdec56 100644 --- a/src/main/java/ch/jalu/configme/properties/BeanProperty.java +++ b/src/main/java/ch/jalu/configme/properties/BeanProperty.java @@ -8,7 +8,8 @@ import org.jetbrains.annotations.NotNull; /** - * Property whose value is a bean class. + * Property whose value is a bean class: maps between a Java class and a section in a property resource + * using a {@link Mapper}. * * @param the bean type */ diff --git a/src/main/java/ch/jalu/configme/properties/PropertyInitializer.java b/src/main/java/ch/jalu/configme/properties/PropertyInitializer.java index a12a5e7e..49e21957 100644 --- a/src/main/java/ch/jalu/configme/properties/PropertyInitializer.java +++ b/src/main/java/ch/jalu/configme/properties/PropertyInitializer.java @@ -221,7 +221,7 @@ protected PropertyInitializer() { * Creates a new bean property. * * @param path the property's path - * @param beanClass the JavaBean class + * @param beanClass the bean class * @param defaultValue default value * @param the bean type * @return the created bean property diff --git a/src/test/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstructorInstantiationTest.java b/src/test/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstructorInstantiationTest.java index 074a3c7e..fa31175f 100644 --- a/src/test/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstructorInstantiationTest.java +++ b/src/test/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstructorInstantiationTest.java @@ -4,7 +4,7 @@ import ch.jalu.configme.beanmapper.propertydescription.BeanPropertyComments; import ch.jalu.configme.exception.ConfigMeException; import ch.jalu.configme.properties.convertresult.ConvertErrorRecorder; -import ch.jalu.typeresolver.FieldUtils; +import ch.jalu.typeresolver.reflect.FieldUtils; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.junit.jupiter.MockitoExtension; diff --git a/src/test/java/ch/jalu/configme/demo/beans/README.md b/src/test/java/ch/jalu/configme/demo/beans/README.md index 66f07e0e..1c415d97 100644 --- a/src/test/java/ch/jalu/configme/demo/beans/README.md +++ b/src/test/java/ch/jalu/configme/demo/beans/README.md @@ -1,11 +1,11 @@ ## Demo with beans -This is a demo showing how you can use custom JavaBeans with ConfigMe. The classes `Location`, -`User`, and `UserBase` are JavaBeans. ConfigMe can scan these classes to determine the properties +This is a demo showing how you can use a custom bean with ConfigMe. The classes `Location`, +`User`, and `UserBase` are beans. ConfigMe can scan these classes to determine the properties they have and will then use the config.yml file to create new objects with the data inside config.yml. `BeanPropertiesDemo` creates a settings manager and uses it to get the `DemoSettings.USER_BASE` setting. You can find the config YAML file [here](https://github.com/AuthMe/ConfigMe/blob/master/src/test/resources/demo/bean_demo_config.yml). -Detailed information about using JavaBeans as properties can be found on the +Detailed information about using beans as properties can be found on the [Bean properties page](https://github.com/AuthMe/ConfigMe/wiki/Bean-properties) of the Wiki. From f6bcb82d42953febe3ca2983e20692c4ea74bc68 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Tue, 15 Oct 2024 21:56:58 +0200 Subject: [PATCH 12/16] Add tests / revise final fields / remove unused method --- .../jalu/configme/beanmapper/MapperImpl.java | 17 -- .../BeanDescriptionFactory.java | 5 +- .../BeanDescriptionFactoryImpl.java | 48 +++-- .../BeanFieldPropertyDescription.java | 3 + .../beanmapper/BeanWithFinalFieldsTest.java | 116 ++++++++++ .../configme/beanmapper/MapperImplTest.java | 28 --- .../MapperWithBeanInheritanceTest.java | 198 ++++++++++++++++++ .../BeanDescriptionFactoryImplTest.java | 61 ++++-- .../ch/jalu/configme/demo/beans/UserBase.java | 3 - .../configme/samples/inheritance/Child.java | 16 ++ .../inheritance/ChildWithFieldOverrides.java | 31 +++ .../configme/samples/inheritance/Middle.java | 17 ++ .../configme/samples/inheritance/Parent.java | 18 ++ 13 files changed, 483 insertions(+), 78 deletions(-) create mode 100644 src/test/java/ch/jalu/configme/beanmapper/BeanWithFinalFieldsTest.java create mode 100644 src/test/java/ch/jalu/configme/beanmapper/MapperWithBeanInheritanceTest.java create mode 100644 src/test/java/ch/jalu/configme/samples/inheritance/ChildWithFieldOverrides.java diff --git a/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java b/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java index 9aae02ba..5a1b04c1 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java +++ b/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java @@ -394,21 +394,4 @@ public MapperImpl(@NotNull BeanInstantiationService beanInstantiationService, } return null; } - - /** - * Creates an object matching the given type information. - * - * @param mappingContext current mapping context - * @return new instance of the given type - */ - protected @NotNull Object createBeanMatchingType(@NotNull MappingContext mappingContext) { - // clazz is never null given the only path that leads to this method already performs that check - final Class clazz = mappingContext.getTargetTypeAsClassOrThrow(); - try { - return clazz.getDeclaredConstructor().newInstance(); - } catch (ReflectiveOperationException e) { - throw new ConfigMeMapperException(mappingContext, "Could not create object of type '" - + clazz.getName() + "'. It is required to have a default constructor", e); - } - } } diff --git a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactory.java b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactory.java index 73407404..06e20264 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactory.java +++ b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactory.java @@ -6,9 +6,10 @@ import java.util.List; /** - * Factory which analyzes a class and returns all properties which ConfigMe should consider. + * Factory which analyzes a bean class and returns all properties which ConfigMe should consider. *

- * Default implementation: {@link BeanDescriptionFactoryImpl}. + * Default implementation: {@link BeanDescriptionFactoryImpl}. This interface is used by + * {@link ch.jalu.configme.beanmapper.instantiation.BeanInstantiationServiceImpl BeanInstantiationServiceImpl}. */ public interface BeanDescriptionFactory { diff --git a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImpl.java b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImpl.java index 8507d31f..7718b64a 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImpl.java +++ b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImpl.java @@ -21,7 +21,6 @@ import java.util.Map; import java.util.Set; import java.util.UUID; -import java.util.stream.Collectors; /** * Creates all {@link BeanPropertyDescription} objects for a given class. @@ -42,7 +41,7 @@ public class BeanDescriptionFactoryImpl implements BeanDescriptionFactory { .filter(FieldUtils::isRegularInstanceField) .collect(FieldUtils.collectByName(false)); - List properties = new ArrayList<>(); + List properties = new ArrayList<>(components.length); for (RecordComponent component : components) { Field field = instanceFieldsByName.get(component.getName()); validateFieldForRecord(clazz, component, field); @@ -54,6 +53,26 @@ public class BeanDescriptionFactoryImpl implements BeanDescriptionFactory { return properties; } + @Override + public @NotNull List collectProperties(@NotNull Class clazz) { + @SuppressWarnings("checkstyle:IllegalType") // LinkedHashMap indicates the values are ordered (important here) + LinkedHashMap instanceFieldsByName = FieldUtils.getAllFields(clazz) + .filter(FieldUtils::isRegularInstanceField) + .collect(FieldUtils.collectByName(false)); + + List properties = new ArrayList<>(); + for (Field field : instanceFieldsByName.values()) { + if (!isFieldIgnored(field)) { + validateFieldForBean(clazz, field); + BeanFieldPropertyDescription property = convert(field); + properties.add(property); + } + } + + validateProperties(clazz, properties); + return properties; + } + /** * Validates the component and its associated field for a class that is a record. * @@ -72,20 +91,17 @@ protected void validateFieldForRecord(@NotNull Class clazz, @NotNull RecordCo } } - @Override - public @NotNull List collectProperties(@NotNull Class clazz) { - @SuppressWarnings("checkstyle:IllegalType") // LinkedHashMap indicates the values are ordered (important here) - LinkedHashMap instanceFieldsByName = FieldUtils.getAllFields(clazz) - .filter(FieldUtils::isRegularInstanceField) - .collect(FieldUtils.collectByName(false)); - - List properties = instanceFieldsByName.values().stream() - .filter(field -> !isFieldIgnored(field)) - .map(this::convert) - .collect(Collectors.toList()); - - validateProperties(clazz, properties); - return properties; + /** + * Validates the given field as valid for bean mapping. + * + * @param clazz the class the field belongs to (the bean type) + * @param field the field to validate + */ + protected void validateFieldForBean(@NotNull Class clazz, @NotNull Field field) { + if (Modifier.isFinal(field.getModifiers())) { + throw new ConfigMeException("Field '" + FieldUtils.formatField(field) + + "' is marked as final but not to be ignored. Final fields cannot be set by the mapper."); + } } protected @NotNull BeanFieldPropertyDescription convert(@NotNull Field field) { diff --git a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanFieldPropertyDescription.java b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanFieldPropertyDescription.java index 0b6cb2c3..e8a8a097 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanFieldPropertyDescription.java +++ b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanFieldPropertyDescription.java @@ -55,6 +55,9 @@ public BeanFieldPropertyDescription(@NotNull Field field, * @param bean the bean to set the value to * @param value the value to set */ + // Surprisingly, setting a value to a final field is allowed in some circumstances, but the value doesn't seem to + // actually be changed outside of the current context. For now, we keep this method free of any validation but + // note that a final field here might NOT cause any exception. public void setValue(@NotNull Object bean, @NotNull Object value) { ReflectionHelper.setAccessibleIfNeeded(field); diff --git a/src/test/java/ch/jalu/configme/beanmapper/BeanWithFinalFieldsTest.java b/src/test/java/ch/jalu/configme/beanmapper/BeanWithFinalFieldsTest.java new file mode 100644 index 00000000..9d50fd0c --- /dev/null +++ b/src/test/java/ch/jalu/configme/beanmapper/BeanWithFinalFieldsTest.java @@ -0,0 +1,116 @@ +package ch.jalu.configme.beanmapper; + +import ch.jalu.configme.exception.ConfigMeException; +import ch.jalu.configme.properties.BeanProperty; +import ch.jalu.configme.properties.convertresult.PropertyValue; +import ch.jalu.configme.resource.PropertyReader; +import org.junit.jupiter.api.Test; + +import java.util.HashMap; +import java.util.Map; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; + +/** + * Mapper integration test for beans with final fields. + */ +class BeanWithFinalFieldsTest { + + @Test + void shouldThrowForFinalField() { + // given + BeanProperty property = new BeanProperty<>("", BeanWithFinalField.class, new BeanWithFinalField()); + PropertyReader reader = mock(PropertyReader.class); + given(reader.getObject("")).willReturn(newMapWithName("t")); + + // when + ConfigMeException ex = assertThrows(ConfigMeException.class, () -> property.determineValue(reader)); + + // then + assertThat(ex.getMessage(), containsString("Final fields cannot be set by the mapper")); + } + + @Test + void shouldNotThrowForFinalTransientField() { + // given + BeanProperty property = + new BeanProperty<>("", BeanWithFinalTransientField.class, new BeanWithFinalTransientField()); + PropertyReader reader = mock(PropertyReader.class); + given(reader.getObject("")).willReturn(newMapWithName("Zoran")); + + // when + PropertyValue value = property.determineValue(reader); + + // then + assertThat(value.getValue().name, equalTo("Zoran")); + } + + @Test + void shouldNotThrowForFinalIgnoredField() { + // given + BeanProperty property = + new BeanProperty<>("", BeanWithFinalIgnoredField.class, new BeanWithFinalIgnoredField()); + PropertyReader reader = mock(PropertyReader.class); + given(reader.getObject("")).willReturn(newMapWithName("Goran")); + + // when + PropertyValue value = property.determineValue(reader); + + // then + assertThat(value.getValue().name, equalTo("Goran")); + } + + @Test + void shouldNotThrowForOverriddenField() { + // given + BeanProperty property = + new BeanProperty<>("", BeanWithFinalOverriddenField.class, new BeanWithFinalOverriddenField()); + PropertyReader reader = mock(PropertyReader.class); + given(reader.getObject("")).willReturn(newMapWithName("Bojan")); + + // when + PropertyValue value = property.determineValue(reader); + + // then + assertThat(value.getValue().name, equalTo("Bojan")); + } + + private static Map newMapWithName(String name) { + Map map = new HashMap<>(); + map.put("name", name); + return map; + } + + public static class BeanWithFinalField { + + String name; + final int version = 1; + + } + + public static class BeanWithFinalTransientField { + + String name; + final transient int version = 2; + + } + + public static class BeanWithFinalIgnoredField { + + String name; + @Ignore + final int version = 3; + + } + + public static class BeanWithFinalOverriddenField extends BeanWithFinalField { + + int version = 4; + + } +} diff --git a/src/test/java/ch/jalu/configme/beanmapper/MapperImplTest.java b/src/test/java/ch/jalu/configme/beanmapper/MapperImplTest.java index 53b201a2..713b17f4 100644 --- a/src/test/java/ch/jalu/configme/beanmapper/MapperImplTest.java +++ b/src/test/java/ch/jalu/configme/beanmapper/MapperImplTest.java @@ -21,7 +21,6 @@ import ch.jalu.configme.beanmapper.worldgroup.GameMode; import ch.jalu.configme.beanmapper.worldgroup.Group; import ch.jalu.configme.beanmapper.worldgroup.WorldGroupConfig; -import ch.jalu.configme.exception.ConfigMeException; import ch.jalu.configme.properties.convertresult.ConvertErrorRecorder; import ch.jalu.configme.resource.PropertyReader; import ch.jalu.configme.resource.YamlFileReader; @@ -50,7 +49,6 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsInAnyOrder; -import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; @@ -424,32 +422,6 @@ void shouldReturnEmptyOptionalForFileWithEmptyMap(@TempDir Path tempDir) throws assertThat(result.getCommandconfig(), equalTo(Optional.empty())); } - @Test - void shouldInvokeDefaultConstructor() { - // given - MapperImpl mapper = new MapperImpl(); - - // when - Object command = mapper.createBeanMatchingType(createContextWithTargetType(Command.class)); - - // then - assertThat(command, instanceOf(Command.class)); - } - - @Test - void shouldForwardException() { - // given - MapperImpl mapper = new MapperImpl(); - - // when - ConfigMeException ex = assertThrows(ConfigMeException.class, - () -> mapper.createBeanMatchingType(createContextWithTargetType(Iterable.class))); - - // then - assertThat(ex.getMessage(), containsString("It is required to have a default constructor")); - assertThat(ex.getCause(), instanceOf(NoSuchMethodException.class)); - } - @Test void shouldReturnFields() { // given diff --git a/src/test/java/ch/jalu/configme/beanmapper/MapperWithBeanInheritanceTest.java b/src/test/java/ch/jalu/configme/beanmapper/MapperWithBeanInheritanceTest.java new file mode 100644 index 00000000..4a1bc5f8 --- /dev/null +++ b/src/test/java/ch/jalu/configme/beanmapper/MapperWithBeanInheritanceTest.java @@ -0,0 +1,198 @@ +package ch.jalu.configme.beanmapper; + +import ch.jalu.configme.TestUtils; +import ch.jalu.configme.properties.BeanProperty; +import ch.jalu.configme.properties.Property; +import ch.jalu.configme.properties.convertresult.PropertyValue; +import ch.jalu.configme.resource.PropertyReader; +import ch.jalu.configme.resource.YamlFileReader; +import ch.jalu.configme.samples.inheritance.Child; +import ch.jalu.configme.samples.inheritance.ChildWithFieldOverrides; +import ch.jalu.configme.samples.inheritance.Middle; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Map; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.nullValue; + +/** + * Mapper integration test with various bean types that inherit from each other. + */ +class MapperWithBeanInheritanceTest { + + @TempDir + Path tempFolder; + + @Nested + class TestMiddle { + + private final Property BEAN_PROPERTY = + new BeanProperty<>("bean", Middle.class, new Middle()); + + @Test + void shouldLoadProperly() throws IOException { + // given + String yaml = "bean:" + + "\n id: 7" + + "\n temporary: true" + + "\n name: 'Foo'" + + "\n ratio: 0.25"; + Path file = TestUtils.createTemporaryFile(tempFolder); + Files.write(file, yaml.getBytes()); + PropertyReader reader = new YamlFileReader(file); + + // when + PropertyValue value = BEAN_PROPERTY.determineValue(reader); + + // then + assertThat(value.isValidInResource(), equalTo(true)); + assertThat(value.getValue().readId(), equalTo(7L)); + assertThat(value.getValue().readTemporary(), equalTo(false)); + assertThat(value.getValue().readName(), equalTo("Foo")); + assertThat(value.getValue().readRatio(), equalTo(0.25f)); + } + + @Test + void shouldExportProperly() { + // given + Middle bean = new Middle(); + bean.writeId(7L); + bean.writeTemporary(true); + bean.writeName("test"); + bean.writeRatio(0.45f); + + // when + Object value = BEAN_PROPERTY.toExportValue(bean); + + // then + assertThat(value, instanceOf(Map.class)); + Map exportValue = (Map) value; + assertThat(exportValue.keySet(), contains("id", "name", "ratio")); + assertThat(exportValue.get("id"), equalTo(7L)); + assertThat(exportValue.get("name"), equalTo("test")); + assertThat(exportValue.get("ratio"), equalTo(0.45f)); + } + } + + @Nested + class TestChild { + + private final Property BEAN_PROPERTY = + new BeanProperty<>("child", Child.class, new Child()); + + @Test + void shouldLoadProperly() throws IOException { + // given + String yaml = "child:" + + "\n id: 7" + + "\n temporary: true" + + "\n name: 'Foo'" + + "\n ratio: 0.25" + + "\n importance: 9"; + Path file = TestUtils.createTemporaryFile(tempFolder); + Files.write(file, yaml.getBytes()); + PropertyReader reader = new YamlFileReader(file); + + // when + PropertyValue value = BEAN_PROPERTY.determineValue(reader); + + // then + assertThat(value.isValidInResource(), equalTo(true)); + assertThat(value.getValue().readId(), equalTo(7L)); + assertThat(value.getValue().readTemporary(), equalTo(false)); + assertThat(value.getValue().readName(), equalTo("Foo")); + assertThat(value.getValue().readRatio(), equalTo(0.25f)); + assertThat(value.getValue().readImportance(), equalTo(9)); + assertThat(value.getValue().readChildTemporary(), equalTo(true)); + } + + @Test + void shouldExportProperly() { + // given + Child bean = new Child(); + bean.writeId(7L); + bean.writeTemporary(false); + bean.writeName("Mike"); + bean.writeRatio(0.33f); + bean.writeImportance(8); + bean.writeChildTemporary(true); + + // when + Object value = BEAN_PROPERTY.toExportValue(bean); + + // then + assertThat(value, instanceOf(Map.class)); + Map exportValue = (Map) value; + assertThat(exportValue.keySet(), contains("id", "temporary", "name", "ratio", "importance")); + assertThat(exportValue.get("id"), equalTo(7L)); + assertThat(exportValue.get("temporary"), equalTo(true)); + assertThat(exportValue.get("name"), equalTo("Mike")); + assertThat(exportValue.get("ratio"), equalTo(0.33f)); + assertThat(exportValue.get("importance"), equalTo(8)); + } + } + + @Nested + class TestChildWithFieldOverrides { + + private final Property BEAN_PROPERTY = + new BeanProperty<>("bean", ChildWithFieldOverrides.class, new ChildWithFieldOverrides()); + + @Test + void shouldLoadProperly() throws IOException { + // given + String yaml = "bean:" + + "\n id: 7" + + "\n temporary: true" + + "\n name: 'Foo'" + + "\n ratio: 99999.9" + + "\n o_ratio: 0.25"; + Path file = TestUtils.createTemporaryFile(tempFolder); + Files.write(file, yaml.getBytes()); + PropertyReader reader = new YamlFileReader(file); + + // when + PropertyValue value = BEAN_PROPERTY.determineValue(reader); + + // then + assertThat(value.isValidInResource(), equalTo(true)); + assertThat(value.getValue().readId(), equalTo(7L)); + assertThat(value.getValue().readTemporary(), equalTo(false)); + assertThat(value.getValue().readName(), nullValue()); + assertThat(value.getValue().readRatio(), equalTo(0f)); + assertThat(value.getValue().readChildName(), nullValue()); + assertThat(value.getValue().readChildRatio(), equalTo(0.25f)); + } + + @Test + void shouldExportProperly() { + // given + ChildWithFieldOverrides bean = new ChildWithFieldOverrides(); + bean.writeId(7L); + bean.writeTemporary(true); + bean.writeName("name (ignored)"); + bean.writeRatio(999); + bean.writeChildName("child name (ignored)"); + bean.writeChildRatio(6.5f); + + // when + Object value = BEAN_PROPERTY.toExportValue(bean); + + // then + assertThat(value, instanceOf(Map.class)); + Map exportValue = (Map) value; + assertThat(exportValue.keySet(), contains("id", "o_ratio")); + assertThat(exportValue.get("id"), equalTo(7L)); + assertThat(exportValue.get("o_ratio"), equalTo(6.5f)); + } + } +} diff --git a/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImplTest.java b/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImplTest.java index 3611b4d5..ca3a0ffb 100644 --- a/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImplTest.java +++ b/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImplTest.java @@ -4,17 +4,18 @@ import ch.jalu.configme.Comment; import ch.jalu.configme.beanmapper.ConfigMeMapperException; import ch.jalu.configme.beanmapper.command.ExecutionDetails; +import ch.jalu.configme.exception.ConfigMeException; import ch.jalu.configme.samples.beanannotations.AnnotatedEntry; import ch.jalu.configme.samples.beanannotations.BeanWithEmptyName; import ch.jalu.configme.samples.beanannotations.BeanWithExportName; import ch.jalu.configme.samples.beanannotations.BeanWithExportNameExtension; import ch.jalu.configme.samples.beanannotations.BeanWithNameClash; import ch.jalu.configme.samples.inheritance.Child; +import ch.jalu.configme.samples.inheritance.ChildWithFieldOverrides; import ch.jalu.configme.samples.inheritance.Middle; import ch.jalu.typeresolver.TypeInfo; import org.junit.jupiter.api.Test; -import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.UUID; @@ -40,25 +41,29 @@ class BeanDescriptionFactoryImplTest { @Test void shouldReturnWritableProperties() { // given / when - Collection descriptions = factory.collectProperties(SampleBean.class); + List descriptions = factory.collectProperties(SampleBean.class); // then assertThat(descriptions, hasSize(4)); - BeanPropertyDescription sizeProperty = getDescription("size", descriptions); + BeanPropertyDescription nameProperty = descriptions.get(0); + assertThat(nameProperty.getName(), equalTo("name")); + assertThat(nameProperty.getTypeInformation(), equalTo(new TypeInfo(String.class))); + assertThat(nameProperty.getComments(), sameInstance(BeanPropertyComments.EMPTY)); + + BeanPropertyDescription sizeProperty = descriptions.get(1); + assertThat(sizeProperty.getName(), equalTo("size")); assertThat(sizeProperty.getTypeInformation(), equalTo(new TypeInfo(int.class))); assertThat(sizeProperty.getComments().getComments(), contains("Size of this entry (cm)")); assertThat(sizeProperty.getComments().getUuid(), notNullValue()); - BeanPropertyDescription nameProperty = getDescription("name", descriptions); - assertThat(nameProperty.getTypeInformation(), equalTo(new TypeInfo(String.class))); - assertThat(nameProperty.getComments(), sameInstance(BeanPropertyComments.EMPTY)); - - BeanPropertyDescription longFieldProperty = getDescription("longField", descriptions); + BeanPropertyDescription longFieldProperty = descriptions.get(2); + assertThat(longFieldProperty.getName(), equalTo("longField")); assertThat(longFieldProperty.getTypeInformation(), equalTo(new TypeInfo(long.class))); assertThat(longFieldProperty.getComments(), sameInstance(BeanPropertyComments.EMPTY)); - BeanPropertyDescription uuidProperty = getDescription("uuid", descriptions); + BeanPropertyDescription uuidProperty = descriptions.get(3); + assertThat(uuidProperty.getName(), equalTo("uuid")); assertThat(uuidProperty.getTypeInformation(), equalTo(new TypeInfo(UUID.class))); assertThat(uuidProperty.getComments(), sameInstance(BeanPropertyComments.EMPTY)); } @@ -136,7 +141,7 @@ void shouldThrowForWhenExportNameIsNullForProperty() { @Test void shouldReturnCommentsWithoutUuid() { // given / when - Collection execDetailsProperties = factory.collectProperties(ExecutionDetails.class); + List execDetailsProperties = factory.collectProperties(ExecutionDetails.class); // then BeanPropertyComments executorComments = getDescription("executor", execDetailsProperties).getComments(); @@ -150,7 +155,7 @@ void shouldReturnCommentsWithoutUuid() { @Test void shouldPickUpCustomNameFromField() { // given / when - List properties = new ArrayList<>(factory.collectProperties(BeanWithExportName.class)); + List properties = factory.collectProperties(BeanWithExportName.class); // then assertThat(properties, hasSize(3)); @@ -165,7 +170,7 @@ void shouldPickUpCustomNameFromField() { @Test void shouldPickUpCustomNameFromFieldsIncludingInheritance() { // given / when - List properties = new ArrayList<>(factory.collectProperties(BeanWithExportNameExtension.class)); + List properties = factory.collectProperties(BeanWithExportNameExtension.class); // then assertThat(properties, hasSize(4)); @@ -179,6 +184,27 @@ void shouldPickUpCustomNameFromFieldsIncludingInheritance() { assertThat(properties.get(3).getComments().getComments(), contains("weight_com")); } + @Test + void shouldTakeOverFieldConfigsFromOverridingClass() { + // given / when + List properties = factory.collectProperties(ChildWithFieldOverrides.class); + + // then + assertThat(transform(properties, BeanPropertyDescription::getName), + contains("id", "o_ratio")); + } + + @Test + void shouldThrowForFinalField() { + // given / when + ConfigMeException ex = assertThrows(ConfigMeException.class, + () -> factory.collectProperties(BeanWithFinalField.class)); + + // then + assertThat(ex.getMessage(), equalTo( + "Field 'BeanDescriptionFactoryImplTest$BeanWithFinalField#version' is marked as final but not to be ignored. Final fields cannot be set by the mapper.")); + } + private static BeanPropertyDescription getDescription(String name, Collection descriptions) { for (BeanPropertyDescription description : descriptions) { @@ -201,10 +227,21 @@ private static final class SampleBean { private static final class BeanWithTransientFields { + private static final String CONSTANT = "This will be ignored"; + private static int counter = 3; // This will be ignored + private String name; private transient long tempId; private transient boolean isSaved; private boolean isMandatory; } + + private static final class BeanWithFinalField { + + private String name; + private final int version = 3; + private boolean isNew; + + } } diff --git a/src/test/java/ch/jalu/configme/demo/beans/UserBase.java b/src/test/java/ch/jalu/configme/demo/beans/UserBase.java index ae47332f..454d98c5 100644 --- a/src/test/java/ch/jalu/configme/demo/beans/UserBase.java +++ b/src/test/java/ch/jalu/configme/demo/beans/UserBase.java @@ -1,7 +1,5 @@ package ch.jalu.configme.demo.beans; -import ch.jalu.configme.beanmapper.Ignore; - /** * User base bean. */ @@ -11,7 +9,6 @@ public class UserBase { private User richie; private User lionel; private double version; - @Ignore private transient int build; public User getBobby() { diff --git a/src/test/java/ch/jalu/configme/samples/inheritance/Child.java b/src/test/java/ch/jalu/configme/samples/inheritance/Child.java index f682e02a..9ada45b0 100644 --- a/src/test/java/ch/jalu/configme/samples/inheritance/Child.java +++ b/src/test/java/ch/jalu/configme/samples/inheritance/Child.java @@ -8,4 +8,20 @@ public class Child extends Middle { private int importance; private boolean temporary; + public int readImportance() { + return importance; + } + + public void writeImportance(int importance) { + this.importance = importance; + } + + public boolean readChildTemporary() { + return temporary; + } + + public void writeChildTemporary(boolean temporary) { + this.temporary = temporary; + } + } diff --git a/src/test/java/ch/jalu/configme/samples/inheritance/ChildWithFieldOverrides.java b/src/test/java/ch/jalu/configme/samples/inheritance/ChildWithFieldOverrides.java new file mode 100644 index 00000000..d9c4f4a7 --- /dev/null +++ b/src/test/java/ch/jalu/configme/samples/inheritance/ChildWithFieldOverrides.java @@ -0,0 +1,31 @@ +package ch.jalu.configme.samples.inheritance; + +import ch.jalu.configme.beanmapper.ExportName; +import ch.jalu.configme.beanmapper.Ignore; + +public class ChildWithFieldOverrides extends Middle { + + @Ignore + private String name; // Ignore name of parent + + @ExportName("o_ratio") + private float ratio; // Override name of Middle#ratio + + + public String readChildName() { + return name; + } + + public void writeChildName(String childName) { + this.name = childName; + } + + public float readChildRatio() { + return ratio; + } + + public void writeChildRatio(float childRatio) { + this.ratio = childRatio; + } + +} diff --git a/src/test/java/ch/jalu/configme/samples/inheritance/Middle.java b/src/test/java/ch/jalu/configme/samples/inheritance/Middle.java index 961bdccc..8bba345b 100644 --- a/src/test/java/ch/jalu/configme/samples/inheritance/Middle.java +++ b/src/test/java/ch/jalu/configme/samples/inheritance/Middle.java @@ -8,4 +8,21 @@ public class Middle extends Parent { private String name; private float ratio; + + public String readName() { + return name; + } + + public void writeName(String name) { + this.name = name; + } + + public float readRatio() { + return ratio; + } + + public void writeRatio(float ratio) { + this.ratio = ratio; + } + } diff --git a/src/test/java/ch/jalu/configme/samples/inheritance/Parent.java b/src/test/java/ch/jalu/configme/samples/inheritance/Parent.java index 3dc966a6..449998d6 100644 --- a/src/test/java/ch/jalu/configme/samples/inheritance/Parent.java +++ b/src/test/java/ch/jalu/configme/samples/inheritance/Parent.java @@ -8,4 +8,22 @@ public class Parent { private long id; private transient boolean temporary; + + // Intentionally not named like getters because methods are ignored by the mapper + + public long readId() { + return id; + } + + public void writeId(long id) { + this.id = id; + } + + public boolean readTemporary() { + return temporary; + } + + public void writeTemporary(boolean temp) { + this.temporary = temp; + } } From 3dde8095733fbd64632d663e53455f23a704db06 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Wed, 16 Oct 2024 18:39:48 +0200 Subject: [PATCH 13/16] Rename Ignore annotation to IgnoreInMapping --- .../beanmapper/{Ignore.java => IgnoreInMapping.java} | 4 ++-- src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java | 2 +- .../propertydescription/BeanDescriptionFactoryImpl.java | 6 +++--- .../jalu/configme/beanmapper/BeanWithFinalFieldsTest.java | 2 +- .../samples/inheritance/ChildWithFieldOverrides.java | 4 ++-- 5 files changed, 9 insertions(+), 9 deletions(-) rename src/main/java/ch/jalu/configme/beanmapper/{Ignore.java => IgnoreInMapping.java} (79%) diff --git a/src/main/java/ch/jalu/configme/beanmapper/Ignore.java b/src/main/java/ch/jalu/configme/beanmapper/IgnoreInMapping.java similarity index 79% rename from src/main/java/ch/jalu/configme/beanmapper/Ignore.java rename to src/main/java/ch/jalu/configme/beanmapper/IgnoreInMapping.java index 666804f4..1e46e5f6 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/Ignore.java +++ b/src/main/java/ch/jalu/configme/beanmapper/IgnoreInMapping.java @@ -10,9 +10,9 @@ * property, i.e. when a bean is created, the annotated field will not be set, and when a bean is exported, fields * with this annotation will also be ignored. *

- * Alternatively, you can define fields as {@code transient} for them to be ignored by ConfigMe. + * Instead of this annotation, you can also declare fields as {@code transient} to have them skipped. */ @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.FIELD) -public @interface Ignore { +public @interface IgnoreInMapping { } diff --git a/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java b/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java index 5a1b04c1..2c234071 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java +++ b/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java @@ -44,7 +44,7 @@ * default implementation} supports Java classes with a zero-args constructor, as well as Java records. The service can * be extended to support more types of classes. *
For Java classes with a zero-args constructor, the class's private fields are taken as properties. The perceived - * properties can be modified with {@link ExportName} and {@link Ignore}. + * properties can be modified with {@link ExportName} and {@link IgnoreInMapping}. *

* Recursion: the mapping of values to a bean is performed recursively, i.e. a bean may have other beans * as fields and generic types at any arbitrary "depth". diff --git a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImpl.java b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImpl.java index 7718b64a..b18e91a5 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImpl.java +++ b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImpl.java @@ -3,7 +3,7 @@ import ch.jalu.configme.Comment; import ch.jalu.configme.beanmapper.ConfigMeMapperException; import ch.jalu.configme.beanmapper.ExportName; -import ch.jalu.configme.beanmapper.Ignore; +import ch.jalu.configme.beanmapper.IgnoreInMapping; import ch.jalu.configme.exception.ConfigMeException; import ch.jalu.configme.internal.record.RecordComponent; import ch.jalu.typeresolver.reflect.FieldUtils; @@ -30,7 +30,7 @@ * field is ignored. *

* This implementation supports @{@link ExportName} and transient properties, declared - * with the {@code transient} keyword or by adding the @{@link Ignore} annotation. + * with the {@code transient} keyword or by adding the @{@link IgnoreInMapping} annotation. */ public class BeanDescriptionFactoryImpl implements BeanDescriptionFactory { @@ -109,7 +109,7 @@ protected void validateFieldForBean(@NotNull Class clazz, @NotNull Field fiel } protected boolean isFieldIgnored(@NotNull Field field) { - return Modifier.isTransient(field.getModifiers()) || field.isAnnotationPresent(Ignore.class); + return Modifier.isTransient(field.getModifiers()) || field.isAnnotationPresent(IgnoreInMapping.class); } /** diff --git a/src/test/java/ch/jalu/configme/beanmapper/BeanWithFinalFieldsTest.java b/src/test/java/ch/jalu/configme/beanmapper/BeanWithFinalFieldsTest.java index 9d50fd0c..eaefc557 100644 --- a/src/test/java/ch/jalu/configme/beanmapper/BeanWithFinalFieldsTest.java +++ b/src/test/java/ch/jalu/configme/beanmapper/BeanWithFinalFieldsTest.java @@ -103,7 +103,7 @@ public static class BeanWithFinalTransientField { public static class BeanWithFinalIgnoredField { String name; - @Ignore + @IgnoreInMapping final int version = 3; } diff --git a/src/test/java/ch/jalu/configme/samples/inheritance/ChildWithFieldOverrides.java b/src/test/java/ch/jalu/configme/samples/inheritance/ChildWithFieldOverrides.java index d9c4f4a7..76c78d6a 100644 --- a/src/test/java/ch/jalu/configme/samples/inheritance/ChildWithFieldOverrides.java +++ b/src/test/java/ch/jalu/configme/samples/inheritance/ChildWithFieldOverrides.java @@ -1,11 +1,11 @@ package ch.jalu.configme.samples.inheritance; import ch.jalu.configme.beanmapper.ExportName; -import ch.jalu.configme.beanmapper.Ignore; +import ch.jalu.configme.beanmapper.IgnoreInMapping; public class ChildWithFieldOverrides extends Middle { - @Ignore + @IgnoreInMapping private String name; // Ignore name of parent @ExportName("o_ratio") From 0a8e9bd19e5b0bcd2fc2e30ede4032a2a6e5baaa Mon Sep 17 00:00:00 2001 From: ljacqu Date: Wed, 16 Oct 2024 18:53:10 +0200 Subject: [PATCH 14/16] Add tests for record properties --- .../BeanDescriptionFactoryImplTest.java | 109 ++++++++++++++++++ 1 file changed, 109 insertions(+) diff --git a/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImplTest.java b/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImplTest.java index ca3a0ffb..02fa9ccb 100644 --- a/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImplTest.java +++ b/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImplTest.java @@ -3,8 +3,11 @@ import ch.jalu.configme.Comment; import ch.jalu.configme.beanmapper.ConfigMeMapperException; +import ch.jalu.configme.beanmapper.ExportName; +import ch.jalu.configme.beanmapper.IgnoreInMapping; import ch.jalu.configme.beanmapper.command.ExecutionDetails; import ch.jalu.configme.exception.ConfigMeException; +import ch.jalu.configme.internal.record.RecordComponent; import ch.jalu.configme.samples.beanannotations.AnnotatedEntry; import ch.jalu.configme.samples.beanannotations.BeanWithEmptyName; import ch.jalu.configme.samples.beanannotations.BeanWithExportName; @@ -205,6 +208,82 @@ void shouldThrowForFinalField() { "Field 'BeanDescriptionFactoryImplTest$BeanWithFinalField#version' is marked as final but not to be ignored. Final fields cannot be set by the mapper.")); } + @Test + void shouldGetPropertiesForRecord() { + // given + RecordComponent component1 = new RecordComponent("name", String.class, String.class); + RecordComponent component2 = new RecordComponent("size", int.class, int.class); + + // when + List properties = + factory.collectPropertiesForRecord(SampleRecord.class, new RecordComponent[]{component1, component2}); + + // then + SampleRecord sampleRecord = new SampleRecord(); + assertThat(properties, hasSize(2)); + assertThat(properties.get(0).getName(), equalTo("name")); + assertThat(properties.get(0).getType(), equalTo(String.class)); + assertThat(properties.get(0).getValue(sampleRecord), equalTo("cur_name")); + assertThat(properties.get(1).getName(), equalTo("size")); + assertThat(properties.get(1).getType(), equalTo(int.class)); + assertThat(properties.get(1).getValue(sampleRecord), equalTo(20)); + } + + @Test + void shouldThrowForRecordWithDuplicatePropertyName() { + // given + RecordComponent component1 = new RecordComponent("name", String.class, String.class); + RecordComponent component2 = new RecordComponent("description", String.class, String.class); + + // when + ConfigMeException ex = assertThrows(ConfigMeException.class, + () -> factory.collectPropertiesForRecord(SampleRecordWithDuplicateName.class, new RecordComponent[]{component1, component2})); + + // then + assertThat(ex.getMessage(), equalTo("class ch.jalu.configme.beanmapper.propertydescription.BeanDescriptionFactoryImplTest$SampleRecordWithDuplicateName has multiple properties with name 'name'")); + } + + @Test + void shouldThrowForRecordWithEmptyCustomName() { + // given + RecordComponent component1 = new RecordComponent("location", String.class, String.class); + + // when + ConfigMeException ex = assertThrows(ConfigMeException.class, + () -> factory.collectPropertiesForRecord(SampleRecordWithEmptyName.class, new RecordComponent[]{component1})); + + // then + assertThat(ex.getMessage(), equalTo("Custom name of FieldProperty '' for field 'BeanDescriptionFactoryImplTest$SampleRecordWithEmptyName#location' may not be empty")); + } + + @Test + void shouldThrowForRecordComponentWithNoEquivalentField() { // This scenario should never happen, can probably be removed after using real records (#347) + // given + RecordComponent component1 = new RecordComponent("name", String.class, String.class); + RecordComponent component2 = new RecordComponent("bogus", String.class, String.class); + + // when + ConfigMeException ex = assertThrows(ConfigMeException.class, + () -> factory.collectPropertiesForRecord(SampleRecord.class, new RecordComponent[]{component1, component2})); + + // then + assertThat(ex.getMessage(), equalTo("Record component 'bogus' for ch.jalu.configme.beanmapper.propertydescription.BeanDescriptionFactoryImplTest$SampleRecord does not have a field with the same name")); + } + + @Test + void shouldThrowForRecordWithFieldToIgnore() { + // given + RecordComponent component1 = new RecordComponent("name", String.class, String.class); + RecordComponent component2 = new RecordComponent("desc", String.class, String.class); + + // when + ConfigMeException ex = assertThrows(ConfigMeException.class, + () -> factory.collectPropertiesForRecord(SampleRecordWithIgnoredField.class, new RecordComponent[]{component1, component2})); + + // then + assertThat(ex.getMessage(), equalTo("Record component 'desc' for ch.jalu.configme.beanmapper.propertydescription.BeanDescriptionFactoryImplTest$SampleRecordWithIgnoredField has a field defined to be ignored: this is not supported for records")); + } + private static BeanPropertyDescription getDescription(String name, Collection descriptions) { for (BeanPropertyDescription description : descriptions) { @@ -244,4 +323,34 @@ private static final class BeanWithFinalField { private boolean isNew; } + + private static final class SampleRecord { // #347: Change to an actual record + + private final String name = "cur_name"; + private final int size = 20; + + } + + private static final class SampleRecordWithDuplicateName { // #347: Change to an actual record + + private final String name = "cur_name"; + @ExportName("name") + private final String description = ""; + + } + + private static final class SampleRecordWithEmptyName { // #347: Change to an actual record + + @ExportName("") + private final String location = "W"; + + } + + private static final class SampleRecordWithIgnoredField { // #347: Change to an actual record + + private final String name = "n"; + @IgnoreInMapping + private final String desc = "d"; + + } } From e3a2686705e711a188d8c3115a5730170c019607 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sun, 20 Oct 2024 10:15:04 +0200 Subject: [PATCH 15/16] Change phrasing --- .../jalu/configme/beanmapper/IgnoreInMapping.java | 7 +++---- .../ch/jalu/configme/beanmapper/MapperImpl.java | 11 ++++++----- .../instantiation/BeanInstantiation.java | 4 ++-- .../BeanInstantiationServiceImpl.java | 6 +++--- .../instantiation/BeanRecordInstantiation.java | 4 ++-- .../BeanZeroArgConstructorInstantiation.java | 3 +-- .../BeanDescriptionFactoryImpl.java | 12 ++++++------ .../jalu/configme/internal/ReflectionHelper.java | 2 +- .../BeanDescriptionFactoryImplTest.java | 2 +- .../BeanFieldPropertyDescriptionTest.java | 15 +++++---------- 10 files changed, 30 insertions(+), 36 deletions(-) diff --git a/src/main/java/ch/jalu/configme/beanmapper/IgnoreInMapping.java b/src/main/java/ch/jalu/configme/beanmapper/IgnoreInMapping.java index 1e46e5f6..e09d7d76 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/IgnoreInMapping.java +++ b/src/main/java/ch/jalu/configme/beanmapper/IgnoreInMapping.java @@ -6,11 +6,10 @@ import java.lang.annotation.Target; /** - * In classes used for bean properties, annotating a field with this annotation tells ConfigMe to ignore the - * property, i.e. when a bean is created, the annotated field will not be set, and when a bean is exported, fields - * with this annotation will also be ignored. + * Annotation to tell ConfigMe to ignore the field during bean mapping. In other words, when a bean is created, + * a field with this annotation will not be written to or read. *

- * Instead of this annotation, you can also declare fields as {@code transient} to have them skipped. + * Fields declared as {@code transient} are also ignored by ConfigMe. */ @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.FIELD) diff --git a/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java b/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java index 2c234071..22e11045 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java +++ b/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java @@ -36,15 +36,16 @@ * Default implementation of {@link Mapper}. *

* Maps a section of a property resource to the provided Java class (called a "bean" type). The mapping is based on the - * bean's properties, whose names must correspond with the names in the property resource. For example, if a bean class - * has a property {@code length} and should be mapped from the property resource's value at path {@code definition}, - * the mapper will look up {@code definition.length} to get the value of the bean property. + * bean's properties, whose names must correspond to the names in the property resource. For example, if a bean + * has a property {@code length} and it should be mapped from the property resource's value at path {@code definition}, + * the mapper will look up {@code definition.length} in the resource to determine the value. *

* Classes are created by the {@link BeanInstantiationService}. The {@link BeanInstantiationServiceImpl * default implementation} supports Java classes with a zero-args constructor, as well as Java records. The service can * be extended to support more types of classes. - *
For Java classes with a zero-args constructor, the class's private fields are taken as properties. The perceived - * properties can be modified with {@link ExportName} and {@link IgnoreInMapping}. + *
For Java classes with a zero-args constructor, the class's instance fields are taken as properties. You can + * change the behavior of the fields with @{@link ExportName} and @{@link IgnoreInMapping}. There must be at + * least one property for the class to be treated as a bean. *

* Recursion: the mapping of values to a bean is performed recursively, i.e. a bean may have other beans * as fields and generic types at any arbitrary "depth". diff --git a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiation.java b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiation.java index 634f0b91..1515a250 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiation.java +++ b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiation.java @@ -14,7 +14,7 @@ public interface BeanInstantiation { /** - * Returns the properties which the bean has. + * Returns the properties of the bean. * * @return the bean's properties */ @@ -22,7 +22,7 @@ public interface BeanInstantiation { /** * Creates a new bean with the given property values. The provided property values must be in the same order as - * returned by this instantiation in {@link #getProperties()}. + * returned by this instantiation's {@link #getProperties()}. * Null is returned if the bean cannot be created, e.g. because a property value was null and it is not supported * by this instantiation. * diff --git a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationServiceImpl.java b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationServiceImpl.java index 2b130fb1..471880e1 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationServiceImpl.java +++ b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanInstantiationServiceImpl.java @@ -22,7 +22,7 @@ *

* This service can handle two different types of classes as beans:

    *
  • Regular Java classes with a no-args constructor: all fields that aren't static or transient - * will be considered as bean properties.
    • + * will be considered as bean properties. Must have at least one property. *
  • Java records
    • *
* @@ -61,8 +61,8 @@ public BeanInstantiationServiceImpl(@NotNull RecordInspector recordInspector, } /** - * Inspects the class and returns an appropriate instantiation for it, if available. Null is returned if the - * class cannot be treated as a bean. + * Inspects the class and returns an appropriate instantiation for it, if available. Null is returned if no + * instantiation could be found for the class. * * @param clazz the class to process * @return bean instantiation for the class, or null if not applicable diff --git a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanRecordInstantiation.java b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanRecordInstantiation.java index 86ca8bfa..ee638d8b 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanRecordInstantiation.java +++ b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanRecordInstantiation.java @@ -29,8 +29,8 @@ public class BeanRecordInstantiation implements BeanInstantiation { */ public BeanRecordInstantiation(@NotNull Class clazz, @NotNull List properties) { this.properties = properties; - Class[] recordTypes = properties.stream().map(BeanFieldPropertyDescription::getType).toArray(Class[]::new); - this.canonicalConstructor = ConstructorUtils.getConstructorOrNull(clazz, recordTypes); + Class[] paramTypes = properties.stream().map(BeanFieldPropertyDescription::getType).toArray(Class[]::new); + this.canonicalConstructor = ConstructorUtils.getConstructorOrNull(clazz, paramTypes); if (this.canonicalConstructor == null) { throw new ConfigMeException("Could not get canonical constructor of " + clazz); } diff --git a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstructorInstantiation.java b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstructorInstantiation.java index 3810328b..c3df7223 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstructorInstantiation.java +++ b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstructorInstantiation.java @@ -14,8 +14,7 @@ import java.util.List; /** - * Instantiates bean types with a zero-arg constructor. This instantiation considers all instance fields that - * are not final. + * Instantiates bean types via zero-arg constructor. */ public class BeanZeroArgConstructorInstantiation implements BeanInstantiation { diff --git a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImpl.java b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImpl.java index b18e91a5..b35e76ed 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImpl.java +++ b/src/main/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImpl.java @@ -26,11 +26,11 @@ * Creates all {@link BeanPropertyDescription} objects for a given class. *

* This description factory returns property descriptions for all instance fields on a class, - * including fields on its parent. If a class has a field of the same name as the parent, the parent's - * field is ignored. + * including the fields of its parents. If a field in a bean class has the same name as a field of a parent type, + * the parent field is ignored. *

- * This implementation supports @{@link ExportName} and transient properties, declared - * with the {@code transient} keyword or by adding the @{@link IgnoreInMapping} annotation. + * This implementation supports @{@link ExportName} and ignores fields that are {@code transient} or annotated + * with @{@link IgnoreInMapping}. */ public class BeanDescriptionFactoryImpl implements BeanDescriptionFactory { @@ -92,7 +92,7 @@ protected void validateFieldForRecord(@NotNull Class clazz, @NotNull RecordCo } /** - * Validates the given field as valid for bean mapping. + * Validates the given field that belongs to a bean class. * * @param clazz the class the field belongs to (the bean type) * @param field the field to validate @@ -100,7 +100,7 @@ protected void validateFieldForRecord(@NotNull Class clazz, @NotNull RecordCo protected void validateFieldForBean(@NotNull Class clazz, @NotNull Field field) { if (Modifier.isFinal(field.getModifiers())) { throw new ConfigMeException("Field '" + FieldUtils.formatField(field) - + "' is marked as final but not to be ignored. Final fields cannot be set by the mapper."); + + "' is final. Final fields cannot be set by the mapper. Remove final or mark it to be ignored."); } } diff --git a/src/main/java/ch/jalu/configme/internal/ReflectionHelper.java b/src/main/java/ch/jalu/configme/internal/ReflectionHelper.java index 5b59696a..1d5aca81 100644 --- a/src/main/java/ch/jalu/configme/internal/ReflectionHelper.java +++ b/src/main/java/ch/jalu/configme/internal/ReflectionHelper.java @@ -67,7 +67,7 @@ public class ReflectionHelper { * @param accessibleObject the reflected object to make accessible (if needed) */ public static void setAccessibleIfNeeded(AccessibleObject accessibleObject) { - // #347: Catch InaccessibleObjectException, use #trySetAccessible? + // #347: Handle InaccessibleObjectException, consider using #trySetAccessible if (!accessibleObject.isAccessible()) { try { accessibleObject.setAccessible(true); diff --git a/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImplTest.java b/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImplTest.java index 02fa9ccb..228c52ec 100644 --- a/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImplTest.java +++ b/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanDescriptionFactoryImplTest.java @@ -205,7 +205,7 @@ void shouldThrowForFinalField() { // then assertThat(ex.getMessage(), equalTo( - "Field 'BeanDescriptionFactoryImplTest$BeanWithFinalField#version' is marked as final but not to be ignored. Final fields cannot be set by the mapper.")); + "Field 'BeanDescriptionFactoryImplTest$BeanWithFinalField#version' is final. Final fields cannot be set by the mapper. Remove final or mark it to be ignored.")); } @Test diff --git a/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanFieldPropertyDescriptionTest.java b/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanFieldPropertyDescriptionTest.java index b29b3f11..5d7d0d3c 100644 --- a/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanFieldPropertyDescriptionTest.java +++ b/src/test/java/ch/jalu/configme/beanmapper/propertydescription/BeanFieldPropertyDescriptionTest.java @@ -4,8 +4,6 @@ import ch.jalu.configme.samples.beanannotations.AnnotatedEntry; import org.junit.jupiter.api.Test; -import java.util.Collection; - import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; @@ -19,7 +17,7 @@ class BeanFieldPropertyDescriptionTest { @Test void shouldGetProperties() { // given - BeanFieldPropertyDescription sizeProperty = getDescriptor("size", SampleBean.class); + BeanFieldPropertyDescription sizeProperty = getDescription("size", SampleBean.class); SampleBean bean = new SampleBean(); bean.size = 77; @@ -36,7 +34,7 @@ void shouldGetProperties() { @Test void shouldHandlePropertySetError() { // given - BeanFieldPropertyDescription sizeProperty = getDescriptor("size", SampleBean.class); + BeanFieldPropertyDescription sizeProperty = getDescription("size", SampleBean.class); String wrongObject = "test"; // when @@ -51,7 +49,7 @@ void shouldHandlePropertySetError() { @Test void shouldHandlePropertyGetError() { // given - BeanPropertyDescription sizeProperty = getDescriptor("size", SampleBean.class); + BeanPropertyDescription sizeProperty = getDescription("size", SampleBean.class); String wrongObject = "test"; // when @@ -66,10 +64,7 @@ void shouldHandlePropertyGetError() { @Test void shouldHaveAppropriateStringRepresentation() { // given - Collection properties = new BeanDescriptionFactoryImpl() - .collectProperties(AnnotatedEntry.class); - BeanPropertyDescription hasIdProperty = properties.stream() - .filter(prop -> "has-id".equals(prop.getName())).findFirst().get(); + BeanFieldPropertyDescription hasIdProperty = getDescription("has-id", AnnotatedEntry.class); // when String output = "Found " + hasIdProperty; @@ -78,7 +73,7 @@ void shouldHaveAppropriateStringRepresentation() { assertThat(output, equalTo("Found FieldProperty 'has-id' for field 'AnnotatedEntry#hasId'")); } - private static BeanFieldPropertyDescription getDescriptor(String name, Class clazz) { + private static BeanFieldPropertyDescription getDescription(String name, Class clazz) { return new BeanDescriptionFactoryImpl().collectProperties(clazz) .stream() .filter(prop -> name.equals(prop.getName())) From a7e1adae3ec698668605309c1f437864f0f18b32 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sun, 20 Oct 2024 10:26:04 +0200 Subject: [PATCH 16/16] Add getters for instantiation fields --- .../BeanRecordInstantiation.java | 8 +++ .../BeanZeroArgConstructorInstantiation.java | 8 +++ .../BeanRecordInstantiationTest.java | 51 ++++++++++++------- ...anZeroArgConstructorInstantiationTest.java | 16 ++++++ 4 files changed, 65 insertions(+), 18 deletions(-) diff --git a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanRecordInstantiation.java b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanRecordInstantiation.java index ee638d8b..55d1600e 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanRecordInstantiation.java +++ b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanRecordInstantiation.java @@ -36,6 +36,14 @@ public BeanRecordInstantiation(@NotNull Class clazz, @NotNull List getCanonicalConstructor() { + return canonicalConstructor; + } + + protected final @NotNull List getFieldProperties() { + return properties; + } + @Override public @NotNull List getProperties() { return Collections.unmodifiableList(properties); diff --git a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstructorInstantiation.java b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstructorInstantiation.java index c3df7223..a1a16daf 100644 --- a/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstructorInstantiation.java +++ b/src/main/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstructorInstantiation.java @@ -27,6 +27,14 @@ public BeanZeroArgConstructorInstantiation(@NotNull Constructor zeroArgsConst this.properties = properties; } + protected final @NotNull Constructor getZeroArgsConstructor() { + return zeroArgsConstructor; + } + + protected final @NotNull List getFieldProperties() { + return properties; + } + @Override public @NotNull List getProperties() { return Collections.unmodifiableList(properties); diff --git a/src/test/java/ch/jalu/configme/beanmapper/instantiation/BeanRecordInstantiationTest.java b/src/test/java/ch/jalu/configme/beanmapper/instantiation/BeanRecordInstantiationTest.java index fca9455e..c2a04270 100644 --- a/src/test/java/ch/jalu/configme/beanmapper/instantiation/BeanRecordInstantiationTest.java +++ b/src/test/java/ch/jalu/configme/beanmapper/instantiation/BeanRecordInstantiationTest.java @@ -4,12 +4,16 @@ import ch.jalu.configme.beanmapper.propertydescription.BeanPropertyComments; import ch.jalu.configme.exception.ConfigMeException; import ch.jalu.configme.properties.convertresult.ConvertErrorRecorder; +import ch.jalu.typeresolver.reflect.ConstructorUtils; +import ch.jalu.typeresolver.reflect.FieldUtils; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.junit.jupiter.MockitoExtension; +import java.lang.reflect.Constructor; import java.util.Arrays; import java.util.List; +import java.util.stream.Collectors; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; @@ -43,13 +47,9 @@ void shouldThrowForMissingConstructor() throws NoSuchFieldException { } @Test - void shouldInstantiateRecord() throws NoSuchFieldException { + void shouldInstantiateRecord() { // given - List properties = Arrays.asList( - new BeanFieldPropertyDescription(ExampleRecord.class.getDeclaredField("name"), null, BeanPropertyComments.EMPTY), - new BeanFieldPropertyDescription(ExampleRecord.class.getDeclaredField("size"), null, BeanPropertyComments.EMPTY)); - - BeanRecordInstantiation instantiation = new BeanRecordInstantiation(ExampleRecord.class, properties); + BeanRecordInstantiation instantiation = ExampleRecord.createInstantiation(); ConvertErrorRecorder errorRecorder = mock(ConvertErrorRecorder.class); // when @@ -62,13 +62,9 @@ void shouldInstantiateRecord() throws NoSuchFieldException { } @Test - void shouldReturnNullIfAnyPropertyIsNull() throws NoSuchFieldException { + void shouldReturnNullIfAnyPropertyIsNull() { // given - List properties = Arrays.asList( - new BeanFieldPropertyDescription(ExampleRecord.class.getDeclaredField("name"), null, BeanPropertyComments.EMPTY), - new BeanFieldPropertyDescription(ExampleRecord.class.getDeclaredField("size"), null, BeanPropertyComments.EMPTY)); - - BeanRecordInstantiation instantiation = new BeanRecordInstantiation(ExampleRecord.class, properties); + BeanRecordInstantiation instantiation = ExampleRecord.createInstantiation(); ConvertErrorRecorder errorRecorder = mock(ConvertErrorRecorder.class); // when / then @@ -79,13 +75,9 @@ void shouldReturnNullIfAnyPropertyIsNull() throws NoSuchFieldException { } @Test - void shouldHandleWrongPropertyValuesGracefully() throws NoSuchFieldException { + void shouldHandleWrongPropertyValuesGracefully() { // given - List properties = Arrays.asList( - new BeanFieldPropertyDescription(ExampleRecord.class.getDeclaredField("name"), null, BeanPropertyComments.EMPTY), - new BeanFieldPropertyDescription(ExampleRecord.class.getDeclaredField("size"), null, BeanPropertyComments.EMPTY)); - - BeanRecordInstantiation instantiation = new BeanRecordInstantiation(ExampleRecord.class, properties); + BeanRecordInstantiation instantiation = ExampleRecord.createInstantiation(); ConvertErrorRecorder errorRecorder = mock(ConvertErrorRecorder.class); // when @@ -98,6 +90,20 @@ void shouldHandleWrongPropertyValuesGracefully() throws NoSuchFieldException { assertThat(ex.getCause().getMessage(), equalTo("argument type mismatch")); } + @Test + void shouldReturnFieldsInGetters() { + // given + BeanRecordInstantiation instantiation = ExampleRecord.createInstantiation(); + + // when + Constructor zeroArgsConstructor = instantiation.getCanonicalConstructor(); + List fieldProperties = instantiation.getFieldProperties(); + + // then + assertThat(zeroArgsConstructor, equalTo(ConstructorUtils.getConstructorOrThrow(ExampleRecord.class, String.class, int.class))); + assertThat(fieldProperties, equalTo(instantiation.getProperties())); + } + private static class ExampleRecord { // #347: Change to an actual record :) private final String name; @@ -108,6 +114,15 @@ private static class ExampleRecord { // #347: Change to an actual record :) this.size = size; } + static BeanRecordInstantiation createInstantiation() { + List properties = Arrays.stream(ExampleRecord.class.getDeclaredFields()) + .filter(FieldUtils::isRegularInstanceField) + .map(field -> new BeanFieldPropertyDescription(field, null, BeanPropertyComments.EMPTY)) + .collect(Collectors.toList()); + + return new BeanRecordInstantiation(ExampleRecord.class, properties); + } + String name() { return name; } diff --git a/src/test/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstructorInstantiationTest.java b/src/test/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstructorInstantiationTest.java index fa31175f..89efb3b3 100644 --- a/src/test/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstructorInstantiationTest.java +++ b/src/test/java/ch/jalu/configme/beanmapper/instantiation/BeanZeroArgConstructorInstantiationTest.java @@ -4,11 +4,13 @@ import ch.jalu.configme.beanmapper.propertydescription.BeanPropertyComments; import ch.jalu.configme.exception.ConfigMeException; import ch.jalu.configme.properties.convertresult.ConvertErrorRecorder; +import ch.jalu.typeresolver.reflect.ConstructorUtils; import ch.jalu.typeresolver.reflect.FieldUtils; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.junit.jupiter.MockitoExtension; +import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; import java.util.Arrays; import java.util.Collections; @@ -139,6 +141,20 @@ void shouldThrowForPropertyValuesMismatch() { + "ch.jalu.configme.beanmapper.instantiation.BeanZeroArgConstructorInstantiationTest$SampleBean has 2 properties")); } + @Test + void shouldReturnFieldsInGetters() { + // given + BeanZeroArgConstructorInstantiation instantiation = SampleBean.createInstantiation(); + + // when + Constructor zeroArgsConstructor = instantiation.getZeroArgsConstructor(); + List fieldProperties = instantiation.getFieldProperties(); + + // then + assertThat(zeroArgsConstructor, equalTo(ConstructorUtils.getConstructorOrThrow(SampleBean.class))); + assertThat(fieldProperties, equalTo(instantiation.getProperties())); + } + private static final class SampleBean { private String name;