#448 Refactor OptionalProperty to be based on a property type

src/main/java/ch/jalu/configme/properties/OptionalProperty.java

@@ -1,70 +1,50 @@
 package ch.jalu.configme.properties;
 
-import ch.jalu.configme.properties.convertresult.PropertyValue;
-import ch.jalu.configme.resource.PropertyReader;
+import ch.jalu.configme.properties.types.OptionalPropertyType;
+import ch.jalu.configme.properties.types.PropertyType;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
 import java.util.Optional;
 
 /**
- * Property which may be empty.
- * <p>
- * Wraps another property with an {@link Optional}: if a property is not present in the property resource,
- * {@link Optional#empty} is returned.
+ * Optional property. Properties of this type may be absent from a property resource and it will still be considered
+ * valid.
  *
  * @param <T> the type of value
  */
-public class OptionalProperty<T> implements Property<Optional<T>> {
-
-    private final Property<T> baseProperty;
-    private final Optional<T> defaultValue;
-
-    public OptionalProperty(@NotNull Property<T> baseProperty) {
-        this.baseProperty = baseProperty;
-        this.defaultValue = Optional.empty();
-    }
-
-    public OptionalProperty(@NotNull Property<T> baseProperty, @NotNull T defaultValue) {
-        this.baseProperty = baseProperty;
-        this.defaultValue = Optional.of(defaultValue);
-    }
-
-    @Override
-    public @NotNull String getPath() {
-        return baseProperty.getPath();
-    }
-
-    @Override
-    public @NotNull PropertyValue<Optional<T>> determineValue(@NotNull PropertyReader reader) {
-        PropertyValue<T> basePropertyValue = baseProperty.determineValue(reader);
-        Optional<T> value = basePropertyValue.isValidInResource()
-            ? Optional.ofNullable(basePropertyValue.getValue())
-            : Optional.empty();
-
-        // Propagate the false "valid" property if the reader has a value at the base property's path
-        // and the base property says it's invalid -> triggers a rewrite to get rid of the invalid value.
-        boolean isWrongInResource = !basePropertyValue.isValidInResource() && reader.contains(baseProperty.getPath());
-        return isWrongInResource
-            ? PropertyValue.withValueRequiringRewrite(value)
-            : PropertyValue.withValidValue(value);
-    }
-
-    @Override
-    public @NotNull Optional<T> getDefaultValue() {
-        return defaultValue;
+public class OptionalProperty<T> extends TypeBasedProperty<Optional<T>> {
+
+    /**
+     * Constructor. Creates a new property with an empty Optional as default value.
+     *
+     * @param path the path of the property
+     * @param valueType the property type of the value inside the optional
+     */
+    public OptionalProperty(@NotNull String path, @NotNull PropertyType<T> valueType) {
+        this(path, new OptionalPropertyType<>(valueType), Optional.empty());
     }
 
-    @Override
-    public boolean isValidValue(@Nullable Optional<T> value) {
-        if (value == null) {
-            return false;
-        }
-        return value.map(baseProperty::isValidValue).orElse(true);
+    /**
+     * Constructor.
+     *
+     * @param path the path of the property
+     * @param valueType the property type of the value inside the optional
+     * @param defaultValue the default value of the property (will be wrapped in an Optional)
+     */
+    public OptionalProperty(@NotNull String path, @NotNull PropertyType<T> valueType, @Nullable T defaultValue) {
+        this(path, new OptionalPropertyType<>(valueType), Optional.ofNullable(defaultValue));
     }
 
-    @Override
-    public @Nullable Object toExportValue(@NotNull Optional<T> value) {
-        return value.map(baseProperty::toExportValue).orElse(null);
+    /**
+     * Constructor.
+     *
+     * @param path the path of the property
+     * @param type the type of this property
+     * @param defaultValue the default value of this property
+     */
+    public OptionalProperty(@NotNull String path, @NotNull PropertyType<Optional<T>> type,
+                            @NotNull Optional<T> defaultValue) {
+        super(path, type, defaultValue);
     }
 }

src/main/java/ch/jalu/configme/properties/PropertyInitializer.java

@@ -4,8 +4,15 @@
 import ch.jalu.configme.properties.builder.CollectionPropertyBuilder;
 import ch.jalu.configme.properties.builder.MapPropertyBuilder;
 import ch.jalu.configme.properties.types.ArrayPropertyType;
+import ch.jalu.configme.properties.types.BooleanType;
+import ch.jalu.configme.properties.types.EnumPropertyType;
 import ch.jalu.configme.properties.types.InlineArrayPropertyType;
+import ch.jalu.configme.properties.types.ListPropertyType;
+import ch.jalu.configme.properties.types.NumberType;
 import ch.jalu.configme.properties.types.PropertyType;
+import ch.jalu.configme.properties.types.RegexType;
+import ch.jalu.configme.properties.types.SetPropertyType;
+import ch.jalu.configme.properties.types.StringType;
 import org.jetbrains.annotations.NotNull;
 
 import java.time.LocalDate;
@@ -311,48 +318,47 @@ public static <E> ArrayPropertyBuilder<E, InlineArrayProperty<E>> inlineArrayPro
     // Optional flavors
     // --------------
     public static @NotNull OptionalProperty<Boolean> optionalBooleanProperty(@NotNull String path) {
-        return new OptionalProperty<>(new BooleanProperty(path, false));
+        return new OptionalProperty<>(path, BooleanType.BOOLEAN);
     }
 
     public static @NotNull OptionalProperty<Short> optionalShortProperty(@NotNull String path) {
-        return new OptionalProperty<>(new ShortProperty(path, (short) 0));
+        return new OptionalProperty<>(path, NumberType.SHORT);
     }
 
     public static @NotNull OptionalProperty<Integer> optionalIntegerProperty(@NotNull String path) {
-        return new OptionalProperty<>(new IntegerProperty(path, 0));
+        return new OptionalProperty<>(path, NumberType.INTEGER);
     }
 
     public static @NotNull OptionalProperty<Long> optionalLongProperty(@NotNull String path) {
-        return new OptionalProperty<>(new LongProperty(path, 0L));
+        return new OptionalProperty<>(path, NumberType.LONG);
     }
 
     public static @NotNull OptionalProperty<Float> optionalFloatProperty(@NotNull String path) {
-        return new OptionalProperty<>(new FloatProperty(path, 0f));
+        return new OptionalProperty<>(path, NumberType.FLOAT);
     }
 
     public static @NotNull OptionalProperty<Double> optionalDoubleProperty(@NotNull String path) {
-        return new OptionalProperty<>(new DoubleProperty(path, 0.0));
+        return new OptionalProperty<>(path, NumberType.DOUBLE);
     }
 
     public static @NotNull OptionalProperty<String> optionalStringProperty(@NotNull String path) {
-        return new OptionalProperty<>(new StringProperty(path, ""));
+        return new OptionalProperty<>(path, StringType.STRING);
     }
 
     public static <E extends Enum<E>> @NotNull OptionalProperty<E> optionalEnumProperty(@NotNull Class<E> clazz,
                                                                                         @NotNull String path) {
-        // default value may never be null, so get the first entry in the enum class
-        return new OptionalProperty<>(new EnumProperty<>(path, clazz, clazz.getEnumConstants()[0]));
+        return new OptionalProperty<>(path, new EnumPropertyType<>(clazz));
     }
 
     public static @NotNull OptionalProperty<Pattern> optionalRegexProperty(@NotNull String path) {
-        return new OptionalProperty<>(new RegexProperty(path, ""));
+        return new OptionalProperty<>(path, RegexType.REGEX);
     }
 
     public static @NotNull OptionalProperty<List<String>> optionalListProperty(@NotNull String path) {
-        return new OptionalProperty<>(new StringListProperty(path));
+        return new OptionalProperty<>(path, new ListPropertyType<>(StringType.STRING));
     }
 
     public static @NotNull OptionalProperty<Set<String>> optionalSetProperty(@NotNull String path) {
-        return new OptionalProperty<>(new StringSetProperty(path));
+        return new OptionalProperty<>(path, new SetPropertyType<>(StringType.STRING));
     }
 }

src/main/java/ch/jalu/configme/properties/types/OptionalPropertyType.java

@@ -0,0 +1,39 @@
+package ch.jalu.configme.properties.types;
+
+import ch.jalu.configme.properties.convertresult.ConvertErrorRecorder;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+
+import java.util.Optional;
+
+/**
+ * Property type for optionals. Wraps another property type.
+ *
+ * @param <T> the value type of the optional
+ */
+public class OptionalPropertyType<T> implements PropertyType<Optional<T>> {
+
+    private final PropertyType<T> valueType;
+
+    /**
+     * Constructor.
+     *
+     * @param valueType the property type to handle the value inside the optional
+     */
+    public OptionalPropertyType(PropertyType<T> valueType) {
+        this.valueType = valueType;
+    }
+
+    @Override
+    public @NotNull Optional<T> convert(@Nullable Object object, @NotNull ConvertErrorRecorder errorRecorder) {
+        if (object != null) {
+            return Optional.ofNullable(valueType.convert(object, errorRecorder));
+        }
+        return Optional.empty();
+    }
+
+    @Override
+    public @Nullable Object toExportValue(@NotNull Optional<T> value) {
+        return value.map(valueType::toExportValue).orElse(null);
+    }
+}

src/test/java/ch/jalu/configme/SettingsManagerImplTest.java

@@ -9,6 +9,7 @@
 import ch.jalu.configme.properties.BeanProperty;
 import ch.jalu.configme.properties.OptionalProperty;
 import ch.jalu.configme.properties.Property;
+import ch.jalu.configme.properties.types.NumberType;
 import ch.jalu.configme.resource.PropertyReader;
 import ch.jalu.configme.resource.PropertyResource;
 import ch.jalu.configme.resource.YamlFileResource;
@@ -203,7 +204,7 @@ void shouldSetOptionalPropertyCorrectly() {
         PropertyResource resource = new YamlFileResource(file);
         SettingsManager settingsManager =
             new SettingsManagerImpl(resource, createConfiguration(TestConfiguration.class), null);
-        OptionalProperty<Integer> intOptional = new OptionalProperty<>(newProperty("version", 65));
+        OptionalProperty<Integer> intOptional = new OptionalProperty<>("version", NumberType.INTEGER);
 
         // when
         settingsManager.setProperty(intOptional, Optional.empty());

src/test/java/ch/jalu/configme/properties/OptionalPropertyTest.java

@@ -1,6 +1,11 @@
 package ch.jalu.configme.properties;
 
 import ch.jalu.configme.properties.convertresult.PropertyValue;
+import ch.jalu.configme.properties.types.BooleanType;
+import ch.jalu.configme.properties.types.EnumPropertyType;
+import ch.jalu.configme.properties.types.NumberType;
+import ch.jalu.configme.properties.types.PropertyType;
+import ch.jalu.configme.properties.types.StringType;
 import ch.jalu.configme.resource.PropertyReader;
 import ch.jalu.configme.samples.TestEnum;
 import org.junit.jupiter.api.Test;
@@ -9,17 +14,16 @@
 import org.mockito.junit.jupiter.MockitoExtension;
 
 import java.util.Optional;
+import java.util.concurrent.TimeUnit;
 
 import static ch.jalu.configme.TestUtils.isErrorValueOf;
 import static ch.jalu.configme.TestUtils.isValidValueOf;
 import static java.util.Optional.of;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.nullValue;
 import static org.mockito.BDDMockito.given;
-import static org.mockito.Mockito.doReturn;
-import static org.mockito.Mockito.only;
-import static org.mockito.Mockito.spy;
-import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.mock;
 
 /**
  * Test for {@link OptionalProperty}.
@@ -33,9 +37,9 @@ class OptionalPropertyTest {
     @Test
     void shouldReturnPresentValues() {
         // given
-        OptionalProperty<Boolean> booleanProp = new OptionalProperty<>(new BooleanProperty("bool.path.test", false));
-        OptionalProperty<Integer> intProp = new OptionalProperty<>(new IntegerProperty("int.path.test", 0));
-        OptionalProperty<TestEnum> enumProp = new OptionalProperty<>(new EnumProperty<>("enum.path.test", TestEnum.class, TestEnum.SECOND));
+        OptionalProperty<Boolean> booleanProp = new OptionalProperty<>("bool.path.test", BooleanType.BOOLEAN);
+        OptionalProperty<Integer> intProp = new OptionalProperty<>("int.path.test", NumberType.INTEGER);
+        OptionalProperty<TestEnum> enumProp = new OptionalProperty<>("enum.path.test", new EnumPropertyType<>(TestEnum.class));
 
         given(reader.getObject("bool.path.test")).willReturn(true);
         given(reader.getObject("int.path.test")).willReturn(27);
@@ -55,9 +59,9 @@ void shouldReturnPresentValues() {
     @Test
     void shouldReturnEmptyOptional() {
         // given
-        OptionalProperty<Boolean> booleanProp = new OptionalProperty<>(new BooleanProperty("bool.path.wrong", false));
-        OptionalProperty<Integer> intProp = new OptionalProperty<>(new IntegerProperty("int.path.wrong", 0));
-        OptionalProperty<TestEnum> enumProp = new OptionalProperty<>(new EnumProperty<>("enum.path.wrong", TestEnum.class, TestEnum.SECOND));
+        OptionalProperty<Boolean> booleanProp = new OptionalProperty<>("bool.path.wrong", BooleanType.BOOLEAN);
+        OptionalProperty<Integer> intProp = new OptionalProperty<>("int.path.wrong", NumberType.INTEGER);
+        OptionalProperty<TestEnum> enumProp = new OptionalProperty<>("enum.path.wrong", new EnumPropertyType<>(TestEnum.class));
 
         // when
         PropertyValue<Optional<Boolean>> boolResult = booleanProp.determineValue(reader);
@@ -73,7 +77,7 @@ void shouldReturnEmptyOptional() {
     @Test
     void shouldAllowToDefineDefaultValue() {
         // given
-        OptionalProperty<Integer> integerProp = new OptionalProperty<>(new IntegerProperty("path", 0), 42);
+        OptionalProperty<Integer> integerProp = new OptionalProperty<>("int.path.wrong", NumberType.INTEGER, 42);
 
         // when
         Optional<Integer> defaultValue = integerProp.getDefaultValue();
@@ -85,48 +89,67 @@ void shouldAllowToDefineDefaultValue() {
     @Test
     void shouldReturnValueWithInvalidFlagIfReturnedFromReader() {
         // given
-        StringProperty baseProperty = spy(new StringProperty("the.path", "DEFAULT"));
-        doReturn(PropertyValue.withValueRequiringRewrite("this should be discarded")).when(baseProperty).determineValue(reader);
-        given(reader.contains("the.path")).willReturn(true);
-        OptionalProperty<String> optionalProperty = new OptionalProperty<>(baseProperty);
+        given(reader.getObject("the.path")).willReturn(400);
+        OptionalProperty<Byte> optionalProperty = new OptionalProperty<>("the.path", NumberType.BYTE);
 
         // when
-        PropertyValue<Optional<String>> value = optionalProperty.determineValue(reader);
+        PropertyValue<Optional<Byte>> value = optionalProperty.determineValue(reader);
 
         // then
-        assertThat(value, isErrorValueOf(Optional.empty()));
+        assertThat(value, isErrorValueOf(Optional.of(Byte.MAX_VALUE)));
     }
 
     @Test
-    void shouldDelegateToBasePropertyAndHaveEmptyOptionalAsDefault() {
+    void shouldValidateWithBasePropertyNullSafe() {
         // given
-        StringProperty baseProperty = new StringProperty("some.path", "Def");
-        OptionalProperty<String> property = new OptionalProperty<>(baseProperty);
+        OptionalProperty<String> property = new OptionalProperty<>("path", StringType.STRING);
 
         // when
-        Optional<String> defaultValue = property.getDefaultValue();
-        String path = property.getPath();
+        boolean isEmptyValid = property.isValidValue(Optional.empty());
+        boolean isValueValid = property.isValidValue(Optional.of("foo"));
+        boolean isNullValid = property.isValidValue(null);
 
         // then
-        assertThat(defaultValue, equalTo(Optional.empty()));
-        assertThat(path, equalTo("some.path"));
+        assertThat(isEmptyValid, equalTo(true));
+        assertThat(isValueValid, equalTo(true));
+        assertThat(isNullValid, equalTo(false));
     }
 
     @Test
-    void shouldValidateWithBasePropertyNullSafe() {
+    void shouldReturnNullAsExportValue() {
         // given
-        StringProperty baseProperty = spy(new StringProperty("some.path", "Def"));
-        OptionalProperty<String> property = new OptionalProperty<>(baseProperty);
+        OptionalProperty<Integer> property = new OptionalProperty<>("int.path", NumberType.INTEGER);
 
         // when
-        boolean isEmptyValid = property.isValidValue(Optional.empty());
-        boolean isValueValid = property.isValidValue(Optional.of("foo"));
-        boolean isNullValid = property.isValidValue(null);
+        Object exportValue = property.toExportValue(Optional.empty());
 
         // then
-        assertThat(isEmptyValid, equalTo(true));
-        assertThat(isValueValid, equalTo(true));
-        assertThat(isNullValid, equalTo(false));
-        verify(baseProperty, only()).isValidValue("foo");
+        assertThat(exportValue, nullValue());
+    }
+
+    @Test
+    void shouldReturnNullIfValuePropertyTypeReturnsNull() {
+        // given
+        PropertyType<String> valuePropertyType = mock(PropertyType.class);
+        given(valuePropertyType.toExportValue("demo")).willReturn(null);
+        OptionalProperty<String> optionalProperty = new OptionalProperty<>("int.path", valuePropertyType);
+
+        // when
+        Object exportValue = optionalProperty.toExportValue(Optional.of("demo"));
+
+        // then
+        assertThat(exportValue, nullValue());
+    }
+
+    @Test
+    void shouldConstructExportValue() {
+        // given
+        OptionalProperty<TimeUnit> optionalProperty = new OptionalProperty<>("duration.unit", EnumPropertyType.of(TimeUnit.class));
+
+        // when
+        Object exportValue = optionalProperty.toExportValue(Optional.of(TimeUnit.HOURS));
+
+        // then
+        assertThat(exportValue, equalTo("HOURS"));
     }
 }