[validation] Better number range and general validation

tweed5-default-extensions/src/main/java/de/siphalor/tweed5/defaultextensions/validation/api/ValidationExtension.java

@@ -3,11 +3,10 @@ package de.siphalor.tweed5.defaultextensions.validation.api;
 import de.siphalor.tweed5.core.api.entry.ConfigEntry;
 import de.siphalor.tweed5.core.api.extension.TweedExtension;
 import de.siphalor.tweed5.core.api.middleware.Middleware;
-import de.siphalor.tweed5.defaultextensions.validation.api.result.ValidationIssue;
 import de.siphalor.tweed5.defaultextensions.validation.api.result.ValidationIssues;
 import de.siphalor.tweed5.defaultextensions.validation.api.validators.SimpleValidatorMiddleware;
 import de.siphalor.tweed5.defaultextensions.validation.impl.ValidationExtensionImpl;
-import org.jetbrains.annotations.ApiStatus;
+import de.siphalor.tweed5.patchwork.api.Patchwork;
 import org.jspecify.annotations.Nullable;
 
 import java.util.*;
@@ -57,5 +56,7 @@ public interface ValidationExtension extends TweedExtension {
 	}
 	<T> void addValidatorMiddleware(ConfigEntry<T> entry, Middleware<ConfigEntryValidator> validator);
 
+	ValidationIssues captureValidationIssues(Patchwork readContextExtensionsData);
+
 	<T extends @Nullable Object> ValidationIssues validate(ConfigEntry<T> entry, T value);
 }

tweed5-default-extensions/src/main/java/de/siphalor/tweed5/defaultextensions/validation/api/validators/NumberRangeValidator.java

@@ -5,54 +5,80 @@ import de.siphalor.tweed5.defaultextensions.validation.api.ConfigEntryValidator;
 import de.siphalor.tweed5.defaultextensions.validation.api.result.ValidationIssue;
 import de.siphalor.tweed5.defaultextensions.validation.api.result.ValidationIssueLevel;
 import de.siphalor.tweed5.defaultextensions.validation.api.result.ValidationResult;
-import lombok.AllArgsConstructor;
+import lombok.*;
-import lombok.Value;
-import org.jspecify.annotations.NonNull;
 import org.jspecify.annotations.Nullable;
 
 import java.util.Collections;
 
 @Value
-@AllArgsConstructor
+@AllArgsConstructor(access = AccessLevel.PRIVATE)
-public class NumberRangeValidator<N extends @NonNull Number> implements ConfigEntryValidator {
+public class NumberRangeValidator<N extends Number> implements ConfigEntryValidator {
 	Class<N> numberClass;
 	@Nullable N minimum;
+	boolean minimumExclusive;
 	@Nullable N maximum;
+	boolean maximumExclusive;
+	String description;
+
+	public static <N extends Number> Builder<N> builder(Class<N> numberClass) {
+		return new Builder<>(numberClass);
+	}
 
 	@Override
 	public <T extends @Nullable Object> ValidationResult<T> validate(ConfigEntry<T> configEntry, T value) {
 		if (!(value instanceof Number)) {
 			return ValidationResult.withIssues(value, Collections.singleton(
-					new ValidationIssue("Value must be numeric", ValidationIssueLevel.ERROR)
+					new ValidationIssue("Value must be numeric, got" + getClassName(value), ValidationIssueLevel.ERROR)
 			));
 		}
 		if (value.getClass() != numberClass) {
 			return ValidationResult.withIssues(value, Collections.singleton(
 					new ValidationIssue(
-							"Value is of wrong type, expected " + numberClass.getSimpleName() +
+							"Value is of wrong type, expected " + numberClass.getName() +
-									", got " + value.getClass().getSimpleName(),
+									", got " + getClassName(value),
 							ValidationIssueLevel.ERROR
 					)
 			));
 		}
 
 		Number numberValue = (Number) value;
-		if (minimum != null && compare(numberValue, minimum) < 0) {
+		if (minimum != null) {
-			//noinspection unchecked
+			int minCmp = compare(numberValue, minimum);
-			return ValidationResult.withIssues((T) minimum, Collections.singleton(
+			if (minimumExclusive ? minCmp <= 0 : minCmp < 0) {
-					new ValidationIssue("Value must be at least " + minimum, ValidationIssueLevel.WARN)
+				//noinspection unchecked
-			));
+				return ValidationResult.withIssues(
+						(T) minimum,
+						Collections.singleton(new ValidationIssue(
+								description + ", got: " + value,
+								ValidationIssueLevel.WARN
+						))
+				);
+			}
 		}
-		if (maximum != null && compare(numberValue, maximum) > 0) {
+		if (maximum != null) {
-			//noinspection unchecked
+			int maxCmp = compare(numberValue, maximum);
-			return ValidationResult.withIssues((T) maximum, Collections.singleton(
+			if (maximumExclusive ? maxCmp >= 0 : maxCmp > 0) {
-					new ValidationIssue("Value must be at most " + maximum, ValidationIssueLevel.WARN)
+				//noinspection unchecked
-			));
+				return ValidationResult.withIssues(
+						(T) maximum,
+						Collections.singleton(new ValidationIssue(
+								description + " Got: " + value,
+								ValidationIssueLevel.WARN
+						))
+				);
+			}
 		}
 
 		return ValidationResult.ok(value);
 	}
 
+	private static String getClassName(@Nullable Object value) {
+		if (value == null) {
+			return "<null>";
+		}
+		return value.getClass().getName();
+	}
+
 	private int compare(Number a, Number b) {
 		if (numberClass == Byte.class) {
 			return Byte.compare(a.byteValue(), b.byteValue());
@@ -71,18 +97,86 @@ public class NumberRangeValidator<N extends @NonNull Number> implements ConfigEn
 
 	@Override
 	public <T> String description(ConfigEntry<T> configEntry) {
-		if (minimum == null) {
+		return description;
-			if (maximum == null) {
+	}
-				return "";
+
-			} else {
+	@RequiredArgsConstructor(access = AccessLevel.PRIVATE)
-				return "Must be smaller or equal to " + maximum + ".";
+	public static class Builder<N extends Number> {
+		private final Class<N> numberClass;
+		private @Nullable N minimum;
+		private boolean minimumExclusive;
+		private @Nullable N maximum;
+		private boolean maximumExclusive;
+
+		public Builder<N> greaterThan(N minimum) {
+			this.minimumExclusive = true;
+			this.minimum = minimum;
+			return this;
+		}
+
+		public Builder<N> greaterThanOrEqualTo(N minimum) {
+			this.minimumExclusive = false;
+			this.minimum = minimum;
+			return this;
+		}
+
+		public Builder<N> lessThan(N maximum) {
+			this.maximumExclusive = true;
+			this.maximum = maximum;
+			return this;
+		}
+
+		public Builder<N> lessThanOrEqualTo(N maximum) {
+			this.maximumExclusive = false;
+			this.maximum = maximum;
+			return this;
+		}
+
+		public NumberRangeValidator<N> build() {
+			return new NumberRangeValidator<>(
+					numberClass,
+					minimum, minimumExclusive,
+					maximum, maximumExclusive,
+					createDescription()
+			);
+		}
+
+		private String createDescription() {
+			if (minimum != null) {
+				if (maximum != null) {
+					if (minimumExclusive == maximumExclusive) {
+						if (minimumExclusive) {
+							return "Must be exclusively between " + minimum + " and " + maximum + ".";
+						} else {
+							return "Must be inclusively between " + minimum + " and " + maximum + ".";
+						}
+					}
+
+					StringBuilder description = new StringBuilder(40);
+					description.append("Must be greater than ");
+					if (!minimumExclusive) {
+						description.append("or equal to ");
+					}
+					description.append(minimum).append(" and less than ");
+					if (!maximumExclusive) {
+						description.append("or equal to ");
+					}
+					description.append(maximum).append('.');
+					return description.toString();
+				} else if (minimumExclusive) {
+					return "Must be greater than " + minimum + ".";
+				} else {
+					return "Must be greater than or equal to " + minimum + ".";
+				}
 			}
-		} else {
+			if (maximum != null) {
-			if (maximum == null) {
+				if (maximumExclusive) {
-				return "Must be greater or equal to " + minimum + ".";
+					return "Must be less than " + maximum + ".";
-			} else {
+				} else {
-				return "Must be inclusively between " + minimum + " and " + maximum + ".";
+					return "Must be less than or equal to " + maximum + ".";
+				}
 			}
+			return "";
 		}
 	}
 }

tweed5-default-extensions/src/main/java/de/siphalor/tweed5/defaultextensions/validation/impl/ValidationExtensionImpl.java

@@ -27,6 +27,7 @@ import de.siphalor.tweed5.defaultextensions.validation.api.result.ValidationIssu
 import de.siphalor.tweed5.defaultextensions.validation.api.result.ValidationIssueLevel;
 import de.siphalor.tweed5.defaultextensions.validation.api.result.ValidationIssues;
 import de.siphalor.tweed5.defaultextensions.validation.api.result.ValidationResult;
+import de.siphalor.tweed5.patchwork.api.Patchwork;
 import de.siphalor.tweed5.patchwork.api.PatchworkPartAccess;
 import lombok.*;
 import org.jspecify.annotations.Nullable;
@@ -169,6 +170,11 @@ public class ValidationExtensionImpl implements ReadWriteRelatedExtension, Valid
 		return entryData;
 	}
 
+	@Override
+	public ValidationIssues captureValidationIssues(Patchwork readContextExtensionsData) {
+		return getOrCreateValidationIssues(readContextExtensionsData);
+	}
+
 	@Override
 	public <T> ValidationIssues validate(ConfigEntry<T> entry, @Nullable T value) {
 		PathTracking pathTracking = new PathTracking();
@@ -215,12 +221,7 @@ public class ValidationExtensionImpl implements ReadWriteRelatedExtension, Valid
 			//noinspection unchecked
 			TweedEntryReader<Object, ConfigEntry<Object>> castedInner = (TweedEntryReader<Object, ConfigEntry<Object>>) inner;
 			return (TweedDataReader reader, ConfigEntry<Object> entry, TweedReadContext context) -> {
-				ValidationIssues validationIssues = context.extensionsData().get(readContextValidationIssuesAccess);
+				ValidationIssues validationIssues = getOrCreateValidationIssues(context.extensionsData());
-				if (validationIssues == null) {
-					validationIssues = new ValidationIssuesImpl();
-				} else {
-					validationIssues = (ValidationIssues) context.extensionsData();
-				}
 
 				Object value = castedInner.read(reader, entry, context);
 
@@ -251,6 +252,16 @@ public class ValidationExtensionImpl implements ReadWriteRelatedExtension, Valid
 		}
 	}
 
+	private ValidationIssues getOrCreateValidationIssues(Patchwork readContextExtensionsData) {
+		assert readContextValidationIssuesAccess != null;
+		ValidationIssues validationIssues = readContextExtensionsData.get(readContextValidationIssuesAccess);
+		if (validationIssues == null) {
+			validationIssues = new ValidationIssuesImpl();
+			readContextExtensionsData.set(readContextValidationIssuesAccess, validationIssues);
+		}
+		return validationIssues;
+	}
+
 	@Getter
 	@RequiredArgsConstructor
 	private class ValidatingConfigEntryVisitor implements ConfigEntryValueVisitor {

tweed5-default-extensions/src/test/java/de/siphalor/tweed5/defaultextensions/validation/impl/ValidationExtensionImplTest.java

@@ -6,26 +6,36 @@ import de.siphalor.tweed5.core.api.entry.SimpleConfigEntry;
 import de.siphalor.tweed5.core.impl.DefaultConfigContainer;
 import de.siphalor.tweed5.core.impl.entry.SimpleConfigEntryImpl;
 import de.siphalor.tweed5.core.impl.entry.StaticMapCompoundConfigEntryImpl;
+import de.siphalor.tweed5.data.extension.api.ReadWriteExtension;
+import de.siphalor.tweed5.data.hjson.HjsonLexer;
+import de.siphalor.tweed5.data.hjson.HjsonReader;
 import de.siphalor.tweed5.defaultextensions.comment.api.CommentExtension;
 import de.siphalor.tweed5.defaultextensions.validation.api.ValidationExtension;
-import de.siphalor.tweed5.defaultextensions.validation.api.result.ValidationIssue;
 import de.siphalor.tweed5.defaultextensions.validation.api.result.ValidationIssueLevel;
 import de.siphalor.tweed5.defaultextensions.validation.api.result.ValidationIssues;
 import de.siphalor.tweed5.defaultextensions.validation.api.validators.NumberRangeValidator;
+import org.jspecify.annotations.Nullable;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.CsvSource;
 
+import java.io.StringReader;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Consumer;
 
+import static de.siphalor.tweed5.data.extension.api.ReadWriteExtension.entryReaderWriter;
+import static de.siphalor.tweed5.data.extension.api.ReadWriteExtension.read;
+import static de.siphalor.tweed5.data.extension.api.readwrite.TweedEntryReaderWriters.*;
 import static de.siphalor.tweed5.defaultextensions.comment.api.CommentExtension.baseComment;
 import static de.siphalor.tweed5.defaultextensions.validation.api.ValidationExtension.validators;
 import static de.siphalor.tweed5.testutils.MapTestUtils.sequencedMap;
 import static java.util.Map.entry;
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.*;
 
 class ValidationExtensionImplTest {
@@ -39,17 +49,26 @@ class ValidationExtensionImplTest {
 	void setUp() {
 		configContainer = new DefaultConfigContainer<>();
 
-		configContainer.registerExtension(CommentExtension.DEFAULT);
+		configContainer.registerExtension(CommentExtension.class);
-		configContainer.registerExtension(ValidationExtension.DEFAULT);
+		configContainer.registerExtension(ReadWriteExtension.class);
+		configContainer.registerExtension(ValidationExtension.class);
 		configContainer.finishExtensionSetup();
 
 		byteEntry = new SimpleConfigEntryImpl<>(configContainer, Byte.class)
-				.apply(validators(new NumberRangeValidator<>(Byte.class, (byte) 10, (byte) 100)));
+				.apply(entryReaderWriter(byteReaderWriter()))
+				.apply(validators(
+						NumberRangeValidator.builder(Byte.class)
+								.greaterThanOrEqualTo((byte) 11)
+								.lessThanOrEqualTo((byte) 100)
+								.build()
+				));
 		intEntry = new SimpleConfigEntryImpl<>(configContainer, Integer.class)
-				.apply(validators(new NumberRangeValidator<>(Integer.class, null, 123)))
+				.apply(entryReaderWriter(intReaderWriter()))
+				.apply(validators(NumberRangeValidator.builder(Integer.class).lessThanOrEqualTo(123).build()))
 				.apply(baseComment("This is the main comment!"));
 		doubleEntry = new SimpleConfigEntryImpl<>(configContainer, Double.class)
-				.apply(validators(new NumberRangeValidator<>(Double.class, 0.5, null)));
+				.apply(entryReaderWriter(doubleReaderWriter()))
+				.apply(validators(NumberRangeValidator.builder(Double.class).greaterThanOrEqualTo(0.5).build()));
 
 		//noinspection unchecked
 		rootEntry = new StaticMapCompoundConfigEntryImpl<>(
@@ -61,7 +80,8 @@ class ValidationExtensionImplTest {
 						entry("int", intEntry),
 						entry("double", doubleEntry)
 				))
-		);
+		)
+				.apply(entryReaderWriter(compoundReaderWriter()));
 
 
 		configContainer.attachTree(rootEntry);
@@ -98,9 +118,71 @@ class ValidationExtensionImplTest {
 		assertNotNull(result.issuesByPath());
 
 		assertAll(
-				() -> assertValidationIssue(result, ".byte", byteEntry, new ValidationIssue("Value must be at least 10", ValidationIssueLevel.WARN)),
+				() -> assertValidationIssue(
-				() -> assertValidationIssue(result, ".int", intEntry, new ValidationIssue("Value must be at most 123", ValidationIssueLevel.WARN)),
+						result,
-				() -> assertValidationIssue(result, ".double", doubleEntry, new ValidationIssue("Value must be at least 0.5", ValidationIssueLevel.WARN))
+						".byte",
+						byteEntry,
+						ValidationIssueLevel.WARN,
+						message -> assertThat(message).contains("11", "100", "9")
+				),
+				() -> assertValidationIssue(
+						result,
+						".int",
+						intEntry,
+						ValidationIssueLevel.WARN,
+						message -> assertThat(message).contains("123", "124")
+				),
+				() -> assertValidationIssue(
+						result,
+						".double",
+						doubleEntry,
+						ValidationIssueLevel.WARN,
+						message -> assertThat(message).contains("0.5", "0.2")
+				)
+		);
+	}
+
+	@Test
+	void readInvalid() {
+		ValidationExtension validationExtension = configContainer.extension(ValidationExtension.class).orElseThrow();
+
+		var reader = new HjsonReader(new HjsonLexer(new StringReader("""
+				{
+					byte: 9
+					int: 124
+					double: 0.2
+				}
+				""")));
+		var validationIssues = new AtomicReference<@Nullable ValidationIssues>();
+		Map<String, Object> value = configContainer.rootEntry().call(read(
+				reader,
+				extensionsData -> validationIssues.set(validationExtension.captureValidationIssues(extensionsData))
+		));
+
+		assertThat(value).isEqualTo(Map.of("byte", (byte) 11, "int", 123, "double", 0.5));
+		//noinspection DataFlowIssue
+		assertThat(validationIssues.get()).isNotNull().satisfies(
+				vi -> assertValidationIssue(
+						vi,
+						".byte",
+						byteEntry,
+						ValidationIssueLevel.WARN,
+						message -> assertThat(message).contains("11", "100", "9")
+				),
+				vi -> assertValidationIssue(
+						vi,
+						".int",
+						intEntry,
+						ValidationIssueLevel.WARN,
+						message -> assertThat(message).contains("123", "124")
+				 ),
+				vi -> assertValidationIssue(
+						vi,
+						".double",
+						doubleEntry,
+						ValidationIssueLevel.WARN,
+						message -> assertThat(message).contains("0.5", "0.2")
+				)
 		);
 	}
 
@@ -108,13 +190,15 @@ class ValidationExtensionImplTest {
 			ValidationIssues issues,
 			String expectedPath,
 			ConfigEntry<?> expectedEntry,
-			ValidationIssue expectedIssue
+			ValidationIssueLevel expectedLevel,
+			Consumer<String> issueMessageConsumer
 	) {
-		assertTrue(issues.issuesByPath().containsKey(expectedPath), "Must have issues for path " + expectedPath);
+		assertThat(issues.issuesByPath()).hasEntrySatisfying(expectedPath, entryIssues -> assertThat(entryIssues).satisfies(
-		ValidationIssues.EntryIssues entryIssues = issues.issuesByPath().get(expectedPath);
+				eis -> assertThat(eis.entry()).isSameAs(expectedEntry),
-		assertSame(expectedEntry, entryIssues.entry(), "Entry must match");
+				eis -> assertThat(eis.issues()).singleElement().satisfies(
-		assertEquals(1, entryIssues.issues().size(), "Entry must have exactly one issue");
+						ei -> assertThat(ei.level()).isEqualTo(expectedLevel),
-		assertEquals(expectedIssue, entryIssues.issues().iterator().next(), "Issue must match");
+						ei -> assertThat(ei.message()).satisfies(issueMessageConsumer)
+				)
+		));
 	}
-
 }