From a5f27495f8075ad2b5b715b136561c48932c3711 Mon Sep 17 00:00:00 2001 From: Julian Reschke Date: Mon, 6 Jul 2026 12:20:37 +0100 Subject: [PATCH 1/5] OAK-12298: SystemPropertySupplier: allow to signal that propery is not present (implies default value) --- .../properties/SystemPropertySupplier.java | 54 ++++++++++--------- .../oak/commons/properties/package-info.java | 2 +- .../SystemPropertySupplierTest.java | 10 ++++ 3 files changed, 40 insertions(+), 26 deletions(-) diff --git a/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/properties/SystemPropertySupplier.java b/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/properties/SystemPropertySupplier.java index 0521172842f..c1d16ae127b 100644 --- a/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/properties/SystemPropertySupplier.java +++ b/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/properties/SystemPropertySupplier.java @@ -23,6 +23,7 @@ import java.util.function.Supplier; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -49,26 +50,34 @@ public class SystemPropertySupplier implements Supplier { private final String propName; private final T defaultValue; private final Function parser; - private Logger log = LOG; private String successLogLevel = "INFO"; private boolean hideValue = false; - private Predicate validator = (a) -> true; + private Predicate validator = a -> true; private Function sysPropReader = System::getProperty; private BiFunction setMessageFormatter = (a, b) -> String.format("System property %s found to be '%s'", a, hideValue ? HIDDEN_REPLACEMENT : b); - private SystemPropertySupplier(@NotNull String propName, @NotNull T defaultValue) { + private SystemPropertySupplier(@NotNull String propName, @Nullable T defaultValue, @Nullable Class clazz) { this.propName = Objects.requireNonNull(propName, "propertyName must be non-null"); - this.defaultValue = Objects.requireNonNull(defaultValue, "defaultValue must be non-null"); - this.parser = getValueParser(defaultValue); + this.defaultValue = defaultValue; + this.parser = getValueParser(defaultValue, clazz); } /** * Create it for a given property name and default value. */ public static SystemPropertySupplier create(@NotNull String propName, @NotNull U defaultValue) { - return new SystemPropertySupplier<>(propName, defaultValue); + return new SystemPropertySupplier<>(propName, + Objects.requireNonNull(defaultValue, "defaultValue must not be null"), null); + } + + /** + * Create it for a given property name and no default value (but expected {@linkplain Class}). + */ + public static SystemPropertySupplier create(@NotNull String propName, @NotNull Class clazz) { + return new SystemPropertySupplier<>(propName, null, + Objects.requireNonNull(clazz, "clazz must not be null")); } /** @@ -100,18 +109,10 @@ public SystemPropertySupplier formatSetMessage(@NotNull BiFunction logSuccessAs(String successLogLevel) { - String newLevel; - switch (Objects.requireNonNull(successLogLevel)) { - case "DEBUG": - case "ERROR": - case "INFO": - case "TRACE": - case "WARN": - newLevel = successLogLevel; - break; - default: - throw new IllegalArgumentException("unsupported log level: " + successLogLevel); - } + String newLevel = switch (Objects.requireNonNull(successLogLevel)) { + case "DEBUG", "ERROR", "INFO", "TRACE", "WARN" -> successLogLevel; + default -> throw new IllegalArgumentException("unsupported log level: " + successLogLevel); + }; this.successLogLevel = newLevel; return this; } @@ -191,18 +192,21 @@ public T get() { } @SuppressWarnings("unchecked") - private static Function getValueParser(T defaultValue) { - if (defaultValue instanceof Boolean) { + private static Function getValueParser(T defaultValue, Class type) { + Class clazz = (defaultValue != null) ? defaultValue.getClass() : type; + + if (clazz == null) { + throw new IllegalArgumentException("Cannot determine type: defaultValue is null and no type provided."); + } else if (Boolean.class.isAssignableFrom(clazz)) { return v -> (T) Boolean.valueOf(v); - } else if (defaultValue instanceof Integer) { + } else if (Integer.class.isAssignableFrom(clazz)) { return v -> (T) Integer.valueOf(v); - } else if (defaultValue instanceof Long) { + } else if (Long.class.isAssignableFrom(clazz)) { return v -> (T) Long.valueOf(v); - } else if (defaultValue instanceof String) { + } else if (String.class.isAssignableFrom(clazz)) { return v -> (T) v; } else { - throw new IllegalArgumentException( - String.format("expects a defaultValue of Boolean, Integer, Long, or String, but got: %s", defaultValue.getClass())); + throw new IllegalArgumentException("Unsupported type: " + clazz.getName()); } } } diff --git a/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/properties/package-info.java b/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/properties/package-info.java index 474329ac53e..9c197aeb223 100644 --- a/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/properties/package-info.java +++ b/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/properties/package-info.java @@ -18,7 +18,7 @@ * For Oak internal use only. Do not use outside Oak components. */ @Internal(since = "1.1.0") -@Version("2.1.0") +@Version("2.2.0") package org.apache.jackrabbit.oak.commons.properties; import org.apache.jackrabbit.oak.commons.annotations.Internal; diff --git a/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/properties/SystemPropertySupplierTest.java b/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/properties/SystemPropertySupplierTest.java index 69ed32d9d49..fea034e08c1 100755 --- a/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/properties/SystemPropertySupplierTest.java +++ b/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/properties/SystemPropertySupplierTest.java @@ -17,6 +17,7 @@ package org.apache.jackrabbit.oak.commons.properties; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import org.apache.jackrabbit.oak.commons.junit.LogCustomizer; @@ -137,4 +138,13 @@ public void testUnsupportedType() { } catch (IllegalArgumentException expected) { } } + + @Test + public void testCheckNullDefault() { + try { + assertNull(SystemPropertySupplier.create("foo", Boolean.class). + usingSystemPropertyReader((n) -> null).get()); + } catch (IllegalArgumentException expected) { + } + } } From 276372043fd7c0ccd3dc90676a80f4c1800a34d5 Mon Sep 17 00:00:00 2001 From: Julian Reschke Date: Mon, 6 Jul 2026 12:52:56 +0100 Subject: [PATCH 2/5] OAK-12298: SystemPropertySupplier: allow to signal that propery is not present (implies default value) - add test for case of property set --- .../SystemPropertySupplierTest.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/properties/SystemPropertySupplierTest.java b/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/properties/SystemPropertySupplierTest.java index fea034e08c1..f4a8c8343f3 100755 --- a/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/properties/SystemPropertySupplierTest.java +++ b/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/properties/SystemPropertySupplierTest.java @@ -147,4 +147,23 @@ public void testCheckNullDefault() { } catch (IllegalArgumentException expected) { } } + + @Test + public void testCheckNoDefaultNotSet() { + try { + assertNull(SystemPropertySupplier.create("foo", Boolean.class). + usingSystemPropertyReader((n) -> null).get()); + } catch (IllegalArgumentException expected) { + } + } + + @Test + public void testCheckNoDefaultSet() { + try { + int value = SystemPropertySupplier.create("foo", Integer.class). + usingSystemPropertyReader((n) -> "4217").get(); + assertEquals(4217, value); + } catch (IllegalArgumentException expected) { + } + } } From d228309f5dfb7a13d53bee71e330d9d84bd04856 Mon Sep 17 00:00:00 2001 From: Julian Reschke Date: Mon, 6 Jul 2026 13:30:10 +0100 Subject: [PATCH 3/5] OAK-12298: SystemPropertySupplier: allow to signal that propery is not present (implies default value) - fix tests --- .../properties/SystemPropertySupplierTest.java | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/properties/SystemPropertySupplierTest.java b/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/properties/SystemPropertySupplierTest.java index f4a8c8343f3..fa8efd84fa0 100755 --- a/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/properties/SystemPropertySupplierTest.java +++ b/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/properties/SystemPropertySupplierTest.java @@ -150,20 +150,14 @@ public void testCheckNullDefault() { @Test public void testCheckNoDefaultNotSet() { - try { - assertNull(SystemPropertySupplier.create("foo", Boolean.class). - usingSystemPropertyReader((n) -> null).get()); - } catch (IllegalArgumentException expected) { - } + assertNull(SystemPropertySupplier.create("foo", Boolean.class). + usingSystemPropertyReader((n) -> null).get()); } @Test public void testCheckNoDefaultSet() { - try { - int value = SystemPropertySupplier.create("foo", Integer.class). - usingSystemPropertyReader((n) -> "4217").get(); - assertEquals(4217, value); - } catch (IllegalArgumentException expected) { - } + int value = SystemPropertySupplier.create("foo", Integer.class). + usingSystemPropertyReader((n) -> "4217").get(); + assertEquals(4217, value); } } From d575560608b643dade108ef8e51b4b2bdbd48bf4 Mon Sep 17 00:00:00 2001 From: Julian Reschke Date: Mon, 6 Jul 2026 13:37:15 +0100 Subject: [PATCH 4/5] OAK-12298: SystemPropertySupplier: allow to signal that propery is not present (implies default value) - fix value check when no default value --- .../oak/commons/properties/SystemPropertySupplier.java | 2 +- .../oak/commons/properties/SystemPropertySupplierTest.java | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/properties/SystemPropertySupplier.java b/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/properties/SystemPropertySupplier.java index c1d16ae127b..519fb635ad8 100644 --- a/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/properties/SystemPropertySupplier.java +++ b/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/properties/SystemPropertySupplier.java @@ -164,7 +164,7 @@ public T get() { log.error("Ignoring malformed value '{}' for system property {}", displayedValue, propName); } - if (!returnValue.equals(defaultValue)) { + if (!Objects.equals(returnValue, defaultValue)) { String msg = setMessageFormatter.apply(propName, returnValue); switch (successLogLevel) { case "INFO": diff --git a/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/properties/SystemPropertySupplierTest.java b/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/properties/SystemPropertySupplierTest.java index fa8efd84fa0..5b9e8dfa74f 100755 --- a/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/properties/SystemPropertySupplierTest.java +++ b/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/properties/SystemPropertySupplierTest.java @@ -160,4 +160,10 @@ public void testCheckNoDefaultSet() { usingSystemPropertyReader((n) -> "4217").get(); assertEquals(4217, value); } + + @Test + public void testCheckNoDefaultSetInvalid() { + assertNull(SystemPropertySupplier.create("foo", Integer.class). + usingSystemPropertyReader((n) -> "abcd").get()); + } } From 1ab1bb47a0274d06ab8f60d40035af07ce022d3b Mon Sep 17 00:00:00 2001 From: Julian Reschke Date: Mon, 6 Jul 2026 13:43:08 +0100 Subject: [PATCH 5/5] OAK-12298: SystemPropertySupplier: allow to signal that propery is not present (implies default value) - fix value check when no default and validator rejects --- .../oak/commons/properties/SystemPropertySupplierTest.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/properties/SystemPropertySupplierTest.java b/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/properties/SystemPropertySupplierTest.java index 5b9e8dfa74f..1d484ab94da 100755 --- a/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/properties/SystemPropertySupplierTest.java +++ b/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/properties/SystemPropertySupplierTest.java @@ -166,4 +166,10 @@ public void testCheckNoDefaultSetInvalid() { assertNull(SystemPropertySupplier.create("foo", Integer.class). usingSystemPropertyReader((n) -> "abcd").get()); } + + @Test + public void testCheckNoDefaultValidatorRejects() { + assertNull(SystemPropertySupplier.create("foo", String.class). + usingSystemPropertyReader((n) -> "abcd").validateWith(x-> false) .get()); + } }