diff --git a/configlib-core/src/main/java/de/exlll/configlib/TypeSerializer.java b/configlib-core/src/main/java/de/exlll/configlib/TypeSerializer.java index 0844a30..370be62 100644 --- a/configlib-core/src/main/java/de/exlll/configlib/TypeSerializer.java +++ b/configlib-core/src/main/java/de/exlll/configlib/TypeSerializer.java @@ -67,7 +67,7 @@ sealed abstract class TypeSerializer> if ((elementValue == null) && !properties.outputNulls()) continue; - final Object serializedValue = serialize(element, elementValue); + final Object serializedValue = serializeElement(element, elementValue); final String formattedName = formatter.format(element.name()); result.put(formattedName, serializedValue); } @@ -75,13 +75,25 @@ sealed abstract class TypeSerializer> return result; } - protected final Object serialize(E element, Object value) { - // The following cast won't cause a ClassCastException because the serializers - // are selected based on the element type. + protected final Object serializeElement(E element, Object value) { + // This cast can lead to a ClassCastException if an element of type X is + // serialized by a custom serializer that expects a different type Y. @SuppressWarnings("unchecked") - final var serializer = (Serializer) - serializers.get(element.name()); - return (value != null) ? serializer.serialize(value) : null; + final var serializer = (Serializer) serializers.get(element.name()); + try { + return (value != null) ? serializer.serialize(value) : null; + } catch (ClassCastException e) { + String msg = ("Serialization of value '%s' for element '%s' of type '%s' failed.\n" + + "The type of the object to be serialized does not match the type " + + "the custom serializer of type '%s' expects.") + .formatted( + value, + element.element(), + element.declaringType(), + serializer.getClass() + ); + throw new ConfigurationException(msg, e); + } } protected final Object deserialize(E element, Object value) { @@ -119,12 +131,14 @@ sealed abstract class TypeSerializer> if (!serializedConfiguration.containsKey(formattedName)) { final Object defaultValue = getDefaultValueOf(element); result[i] = applyPostProcessorForElement(element, defaultValue); -// TODO: if (result[i] == null) requireNonPrimitiveType(element); continue; } final var serializedValue = serializedConfiguration.get(formattedName); + if ((serializedValue == null) && properties.inputNulls()) { + // This statement (and hence the whole block) could be removed, + // but in my opinion the code is clearer this way. result[i] = null; } else if (serializedValue == null) { result[i] = getDefaultValueOf(element); @@ -132,9 +146,7 @@ sealed abstract class TypeSerializer> result[i] = deserialize(element, serializedValue); } - if (result[i] == null) requireNonPrimitiveType(element); result[i] = applyPostProcessorForElement(element, result[i]); - // TODO: PostProcessor could return null, check should be done after } return result; @@ -145,34 +157,77 @@ sealed abstract class TypeSerializer> Object deserializeValue ) { Object result = deserializeValue; + + boolean postProcessed = false; for (final var entry : properties.getPostProcessorsByCondition().entrySet()) { final var condition = entry.getKey(); - if (condition.test(element)) { - final var postProcessor = entry.getValue(); - result = tryApplyPostProcessorForElement(postProcessor, result); - } + if (!condition.test(element)) continue; + + final var postProcessor = entry.getValue(); + result = tryApplyPostProcessorForElement(element, postProcessor, result); + postProcessed = true; } + + if ((result == null) && postProcessed) + requirePostProcessorDoesNotReturnNullForPrimitiveElement(element); + else if (result == null) + requireNonPrimitiveType(element); + return result; } private static Object tryApplyPostProcessorForElement( + ConfigurationElement element, UnaryOperator postProcessor, Object value ) { - // TODO: Properly throw a ClassCastException - // TODO: Add test: type of element does not match type postprocessor expects - @SuppressWarnings("unchecked") - final var pp = (UnaryOperator) postProcessor; - return pp.apply(value); + try { + // This cast can lead to a ClassCastException if an element of type X is + // annotated with a post-processor that takes values of some other type Y. + @SuppressWarnings("unchecked") + final var pp = (UnaryOperator) postProcessor; + return pp.apply(value); + } catch (ClassCastException e) { + String msg = ("Deserialization of value '%s' for element '%s' of type '%s' failed.\n" + + "The type of the object to be deserialized does not match the type " + + "post-processor '%s' expects.") + .formatted(value, element.element(), element.declaringType(), postProcessor); + throw new ConfigurationException(msg, e); + } } - private static void requireNonPrimitiveType(ConfigurationElement element) { + private static void requirePostProcessorDoesNotReturnNullForPrimitiveElement( + ConfigurationElement element + ) { + if (!element.type().isPrimitive()) return; + if (element instanceof RecordComponentElement recordComponentElement) { final RecordComponent component = recordComponentElement.element(); + String msg = """ + Post-processors must not return null for primitive record \ + components but some post-processor of component '%s' of \ + record type '%s' does.\ + """.formatted(component, component.getDeclaringRecord()); + throw new ConfigurationException(msg); + } - if (!component.getType().isPrimitive()) return; + if (element instanceof FieldElement fieldElement) { + final Field field = fieldElement.element(); + String msg = ("Post-processors must not return null for primitive fields " + + "but some post-processor of field '%s' does.") + .formatted(field); + throw new ConfigurationException(msg); + } + throw new ConfigurationException("Unhandled ConfigurationElement: " + element); + } + + private static void requireNonPrimitiveType(ConfigurationElement element) { + if (!element.type().isPrimitive()) return; + + if (element instanceof RecordComponentElement recordComponentElement) { + final RecordComponent component = recordComponentElement.element(); String msg = ("Cannot set component '%s' of record type '%s' to null. " + "Primitive types cannot be assigned null values.") .formatted(component, component.getDeclaringRecord()); @@ -181,9 +236,6 @@ sealed abstract class TypeSerializer> if (element instanceof FieldElement fieldElement) { final Field field = fieldElement.element(); - - if (!field.getType().isPrimitive()) return; - String msg = ("Cannot set field '%s' to null value. " + "Primitive types cannot be assigned null.") .formatted(field); @@ -197,7 +249,7 @@ sealed abstract class TypeSerializer> final List list = Arrays.stream(type.getDeclaredMethods()) .filter(method -> method.isAnnotationPresent(PostProcess.class)) .filter(Predicate.not(Method::isSynthetic)) - .filter(this::isNotAccessorMethod) + .filter(Predicate.not(this::isAccessorMethod)) .toList(); if (list.isEmpty()) @@ -242,19 +294,21 @@ sealed abstract class TypeSerializer> Reflect.invoke(method, object); return object; } - // The following cast won't fail because our last check above guarantees - // that the return type of the method equals T at this point. + // The following cast won't fail because our last two checks from above + // guarantee that the return type of the method equals T at this point. @SuppressWarnings("unchecked") T result = (T) Reflect.invoke(method, object); return result; }; } - private boolean isNotAccessorMethod(Method method) { - if (!type.isRecord()) return true; + final boolean isAccessorMethod(Method method) { + if (!type.isRecord()) return false; + if (!method.getDeclaringClass().equals(type)) return false; + if (method.getParameterCount() > 0) return false; return Arrays.stream(type.getRecordComponents()) .map(RecordComponent::getName) - .noneMatch(s -> s.equals(method.getName())); + .anyMatch(s -> s.equals(method.getName())); } protected abstract void requireSerializableElements(); diff --git a/configlib-core/src/test/java/de/exlll/configlib/ConfigurationSerializerTest.java b/configlib-core/src/test/java/de/exlll/configlib/ConfigurationSerializerTest.java index 89ad665..383660e 100644 --- a/configlib-core/src/test/java/de/exlll/configlib/ConfigurationSerializerTest.java +++ b/configlib-core/src/test/java/de/exlll/configlib/ConfigurationSerializerTest.java @@ -685,4 +685,52 @@ class ConfigurationSerializerTest { assertThat(deserialized.s3, is("empty")); assertThat(deserialized.s4, is("empty")); } + + @Configuration + static final class B15 { + // The order of fields is important for the test case below. + // No exception should be thrown for the Integer. + @PostProcess(key = "nullReturning") + private Integer refI; + @PostProcess(key = "nullReturning") + private int primI; + } + + @Test + void throwExceptionIfPostProcessorOfPrimitiveElementReturnsNullCls() { + final var serializer = newSerializer( + B15.class, + builder -> builder.inputNulls(true).addPostProcessor( + ConfigurationElementFilter.byPostProcessKey("nullReturning"), + object -> null + ) + ); + + assertThrowsConfigurationException( + () -> serializer.deserialize(Map.of()), + "Post-processors must not return null for primitive fields " + + "but some post-processor of field " + + "'private int de.exlll.configlib.ConfigurationSerializerTest$B15.primI' does." + ); + } + + @Configuration + static final class B16 { + @PostProcess(key = "nonNullReturning") + private int primI; + } + + @Test + void postProcessorCanPreventExceptionsThatHappenWhenTryingToSetPrimitiveFieldsToNull() { + final var serializer = newSerializer( + B16.class, + builder -> builder.inputNulls(true) + .addPostProcessor( + ConfigurationElementFilter.byPostProcessKey("nonNullReturning"), + (Integer value) -> 76 + ) + ); + B16 primI = serializer.deserialize(asMap("primI", null)); + assertThat(primI.primI, is(76)); + } } \ No newline at end of file diff --git a/configlib-core/src/test/java/de/exlll/configlib/RecordSerializerTest.java b/configlib-core/src/test/java/de/exlll/configlib/RecordSerializerTest.java index 1457a63..7cf5719 100644 --- a/configlib-core/src/test/java/de/exlll/configlib/RecordSerializerTest.java +++ b/configlib-core/src/test/java/de/exlll/configlib/RecordSerializerTest.java @@ -519,4 +519,45 @@ class RecordSerializerTest { assertThat(deserialized.s1, is("empty")); assertThat(deserialized.s2, nullValue()); } + + record R15( + @PostProcess(key = "nullReturning") + Integer refI, + @PostProcess(key = "nullReturning") + int primI + ) {} + + @Test + void throwExceptionIfPostProcessorOfPrimitiveElementReturnsNullCls() { + final var serializer = newSerializer( + R15.class, + builder -> builder.inputNulls(true).addPostProcessor( + ConfigurationElementFilter.byPostProcessKey("nullReturning"), + object -> null + ) + ); + + assertThrowsConfigurationException( + () -> serializer.deserialize(Map.of()), + "Post-processors must not return null for primitive record components " + + "but some post-processor of component 'int primI' of record type " + + "'class de.exlll.configlib.RecordSerializerTest$R15' does." + ); + } + + record R16(@PostProcess(key = "nonNullReturning") int primI) {} + + @Test + void postProcessorCanPreventExceptionsThatHappenWhenTryingToSetPrimitiveFieldsToNull() { + final var serializer = newSerializer( + R16.class, + builder -> builder.inputNulls(true) + .addPostProcessor( + ConfigurationElementFilter.byPostProcessKey("nonNullReturning"), + (Integer value) -> 76 + ) + ); + R16 primI = serializer.deserialize(asMap("primI", null)); + assertThat(primI.primI, is(76)); + } } \ No newline at end of file diff --git a/configlib-core/src/test/java/de/exlll/configlib/SerializersTest.java b/configlib-core/src/test/java/de/exlll/configlib/SerializersTest.java index 9fffa84..291e617 100644 --- a/configlib-core/src/test/java/de/exlll/configlib/SerializersTest.java +++ b/configlib-core/src/test/java/de/exlll/configlib/SerializersTest.java @@ -4,6 +4,7 @@ import de.exlll.configlib.Serializers.MapSerializer; import de.exlll.configlib.Serializers.NumberSerializer; import de.exlll.configlib.Serializers.SetAsListSerializer; import de.exlll.configlib.Serializers.SetSerializer; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.EnumSource; @@ -497,6 +498,8 @@ class SerializersTest { } @Test + @Disabled("This test is disabled because the URL constructor sends a network " + + "request that slows down the tests if the network is unresponsive.") void urlSerializer() throws Exception { Serializer serializer = new Serializers.UrlSerializer(); diff --git a/configlib-core/src/test/java/de/exlll/configlib/TypeSerializerTest.java b/configlib-core/src/test/java/de/exlll/configlib/TypeSerializerTest.java index 16cc1b5..0c94fc8 100644 --- a/configlib-core/src/test/java/de/exlll/configlib/TypeSerializerTest.java +++ b/configlib-core/src/test/java/de/exlll/configlib/TypeSerializerTest.java @@ -1,6 +1,7 @@ package de.exlll.configlib; import de.exlll.configlib.Serializers.*; +import de.exlll.configlib.TestUtils.DoubleIntSerializer; import de.exlll.configlib.configurations.ExampleConfigurationA2; import de.exlll.configlib.configurations.ExampleConfigurationB1; import de.exlll.configlib.configurations.ExampleConfigurationB2; @@ -8,16 +9,20 @@ import de.exlll.configlib.configurations.ExampleEnum; import org.junit.jupiter.api.Test; import java.awt.Point; +import java.lang.reflect.Method; import java.math.BigInteger; import java.util.List; import java.util.Map; import java.util.Set; import java.util.UUID; import java.util.function.Consumer; +import java.util.function.UnaryOperator; import static de.exlll.configlib.TestUtils.assertThrowsConfigurationException; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.*; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; class TypeSerializerTest { private static TypeSerializer newTypeSerializer( @@ -305,11 +310,13 @@ class TypeSerializerTest { @PostProcess E postProcess() {return null;} } + static class F extends E { @Override @PostProcess E postProcess() {return null;} } + static class G extends E { @Override @PostProcess @@ -460,4 +467,120 @@ class TypeSerializerTest { postProcessor.apply(b); assertThat(b.i, is(10)); } + + @Configuration + static final class ClsAccessorMethod { + private int a; + + public int a() {return a;} + + public int a(int a) {return a;} + + public int getA() {return a;} + } + + record RecAccessorMethodA(int a) { + public int a() {return a;} + + public int b() {return a;} + + public int a(int a) {return a;} + + public int getA() {return a;} + } + + record RecAccessorMethodB(int b) {} + + @Test + void classMethodsAreNoAccessorMethods() { + final var serializer = newTypeSerializer(ClsAccessorMethod.class); + + final Method getA = TestUtils.getMethod(ClsAccessorMethod.class, "getA"); + assertFalse(serializer.isAccessorMethod(getA)); + + final List as = TestUtils.getMethods(ClsAccessorMethod.class, "a"); + assertThat(as.size(), is(2)); + for (Method a : as) { + assertFalse(serializer.isAccessorMethod(a)); + } + } + + @Test + void recordMethodsCanBeAccessorMethodsA() { + final var serializerA = newTypeSerializer(RecAccessorMethodA.class); + + final Method getA = TestUtils.getMethod(RecAccessorMethodA.class, "getA"); + assertFalse(serializerA.isAccessorMethod(getA)); + + final List methods = TestUtils.getMethods(RecAccessorMethodA.class, "a"); + + final int accessMethodIndex = (methods.get(0).getParameterCount()) == 0 ? 0 : 1; + + Method method1 = methods.get(accessMethodIndex); + Method method2 = methods.get(1 - accessMethodIndex); + + assertTrue(serializerA.isAccessorMethod(method1)); + assertFalse(serializerA.isAccessorMethod(method2)); + } + + @Test + void recordMethodsCanBeAccessorMethodsB() { + final var serializerB = newTypeSerializer(RecAccessorMethodB.class); + + final Method a = TestUtils.getMethod(RecAccessorMethodA.class, "b"); + assertFalse(serializerB.isAccessorMethod(a)); + + final Method b = TestUtils.getMethod(RecAccessorMethodB.class, "b"); + assertTrue(serializerB.isAccessorMethod(b)); + } + + private static final class PostProcessorInteger implements UnaryOperator { + @Override + public Integer apply(Integer integer) { + return integer + 1; + } + + @Override + public String toString() { + return "PostProcessorInteger"; + } + } + + @Test + void postProcessorThrowsExceptionIfElementIsOfWrongType() { + record R(@PostProcess(key = "key1") String s) {} + + final var serializer = newTypeSerializer( + R.class, + b -> b.addPostProcessor( + ConfigurationElementFilter.byPostProcessKey("key1"), + new PostProcessorInteger() + ) + ); + + assertThrowsConfigurationException( + () -> serializer.deserialize(Map.of("s", "value")), + "Deserialization of value 'value' for element 'java.lang.String s' " + + "of type 'class de.exlll.configlib.TypeSerializerTest$1R' failed.\n" + + "The type of the object to be deserialized does not match the " + + "type post-processor 'PostProcessorInteger' expects." + ); + } + + @Test + void serializeThrowsExceptionIfCustomSerializerExpectsWrongType() { + record S(@SerializeWith(serializer = DoubleIntSerializer.class) String s) {} + + final var serializer = newTypeSerializer(S.class); + + assertThrowsConfigurationException( + () -> serializer.serialize(new S("value")), + "Serialization of value 'value' for element 'java.lang.String s' " + + "of type 'class de.exlll.configlib.TypeSerializerTest$1S' failed.\n" + + "The type of the object to be serialized does not match the type " + + "the custom serializer of type " + + "'class de.exlll.configlib.TestUtils$DoubleIntSerializer' expects." + ); + + } } diff --git a/configlib-core/src/testFixtures/java/de/exlll/configlib/TestUtils.java b/configlib-core/src/testFixtures/java/de/exlll/configlib/TestUtils.java index eb8524e..152d3f0 100644 --- a/configlib-core/src/testFixtures/java/de/exlll/configlib/TestUtils.java +++ b/configlib-core/src/testFixtures/java/de/exlll/configlib/TestUtils.java @@ -3,20 +3,24 @@ package de.exlll.configlib; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.function.Executable; -import java.awt.*; +import java.awt.Point; import java.io.File; import java.io.IOException; import java.lang.reflect.Field; +import java.lang.reflect.Method; import java.math.BigInteger; import java.nio.charset.Charset; import java.nio.file.Files; import java.nio.file.Path; -import java.util.List; import java.util.*; import java.util.function.Supplier; import java.util.stream.Collectors; import java.util.stream.Stream; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.hasSize; +import static org.junit.jupiter.api.Assertions.assertEquals; + public final class TestUtils { public static final PointSerializer POINT_SERIALIZER = new PointSerializer(); public static final PointIdentitySerializer POINT_IDENTITY_SERIALIZER = @@ -63,7 +67,7 @@ public final class TestUtils { String expectedExceptionMessage ) { T exception = Assertions.assertThrows(exceptionType, executable); - Assertions.assertEquals(expectedExceptionMessage, exception.getMessage()); + assertEquals(expectedExceptionMessage, exception.getMessage()); } public static final class CustomBigIntegerSerializer implements Serializer { @@ -160,6 +164,20 @@ public final class TestUtils { } } + public static final class DoubleIntSerializer + implements Serializer { + + @Override + public Integer serialize(Integer element) { + return element * 2; + } + + @Override + public Integer deserialize(Integer element) { + return element / 2; + } + } + @SafeVarargs public static List asList(E... elements) { return new ArrayList<>(Arrays.asList(elements)); @@ -295,6 +313,18 @@ public final class TestUtils { return new ConfigurationElements.FieldElement(field); } + public static Method getMethod(Class type, String methodName) { + final List methods = getMethods(type, methodName); + assertThat(methods, hasSize(1)); + return methods.get(0); + } + + public static List getMethods(Class type, String methodName) { + return Arrays.stream(type.getDeclaredMethods()) + .filter(method -> method.getName().equals(methodName)) + .toList(); + } + /* There were absolute path errors when trying to pass the unit tests on different platforms like Windows. Currently, Jimfs(1.3.0) lacks support