Improve exception handling and add more post-processor tests

configlib-core/src/main/java/de/exlll/configlib/TypeSerializer.java

@@ -67,7 +67,7 @@ sealed abstract class TypeSerializer<T, E extends ConfigurationElement<?>>
             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<T, E extends ConfigurationElement<?>>
         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<Object, Object>)
-                serializers.get(element.name());
+        final var serializer = (Serializer<Object, Object>) 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<T, E extends ConfigurationElement<?>>
             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<T, E extends ConfigurationElement<?>>
                 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<T, E extends ConfigurationElement<?>>
             Object deserializeValue
     ) {
         Object result = deserializeValue;
+
+        boolean postProcessed = false;
         for (final var entry : properties.getPostProcessorsByCondition().entrySet()) {
             final var condition = entry.getKey();
 
-            if (condition.test(element)) {
+            if (!condition.test(element)) continue;
+
             final var postProcessor = entry.getValue();
-                result = tryApplyPostProcessorForElement(postProcessor, result);
-            }
+            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
+        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<Object>) 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 (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);
+        }
 
-            if (!component.getType().isPrimitive()) return;
+        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<T, E extends ConfigurationElement<?>>
 
         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<T, E extends ConfigurationElement<?>>
         final List<Method> 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<T, E extends ConfigurationElement<?>>
                 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();

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));
+    }
 }

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));
+    }
 }

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<URL, String> serializer = new Serializers.UrlSerializer();
 

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 <T> TypeSerializer<T, ?> 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<Method> 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<Method> 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<Integer> {
+        @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."
+        );
+
+    }
 }

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<BigInteger, String> {
@@ -160,6 +164,20 @@ public final class TestUtils {
         }
     }
 
+    public static final class DoubleIntSerializer
+            implements Serializer<Integer, Integer> {
+
+        @Override
+        public Integer serialize(Integer element) {
+            return element * 2;
+        }
+
+        @Override
+        public Integer deserialize(Integer element) {
+            return element / 2;
+        }
+    }
+
     @SafeVarargs
     public static <E> List<E> 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<Method> methods = getMethods(type, methodName);
+        assertThat(methods, hasSize(1));
+        return methods.get(0);
+    }
+
+    public static List<Method> 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