Skip to content

Commit

Permalink
#318 Mark property values as not null
Browse files Browse the repository at this point in the history
- Undo most of 4c35833: consider property values to always be not-null as to benefit from type safety. Correct usage of ConfigMe should never return null as property value; instead, a suitable wrapper like Optional should be used.
  • Loading branch information
ljacqu committed Jun 21, 2023
1 parent f41d4ff commit 844da6b
Show file tree
Hide file tree
Showing 11 changed files with 39 additions and 46 deletions.
16 changes: 8 additions & 8 deletions src/main/java/ch/jalu/configme/SettingsManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,20 @@ public interface SettingsManager {
/**
* Gets the given property from the configuration.
*
* @param property The property to retrieve
* @param <T> The property's type
* @return The property's value
* @param property the property to retrieve
* @param <T> the property's type
* @return the property's value
*/
<T> /* PV */ T getProperty(@NotNull Property<T> property);
<T> @NotNull T getProperty(@NotNull Property<T> property);

/**
* Sets a new value for the given property.
*
* @param property The property to modify
* @param value The new value to assign to the property
* @param <T> The property's type
* @param property the property to modify
* @param value the new value to assign to the property
* @param <T> the property's type
*/
<T> void setProperty(@NotNull Property<T> property, /* PV */ T value);
<T> void setProperty(@NotNull Property<T> property, @NotNull T value);

/**
* Reloads the configuration.
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/ch/jalu/configme/SettingsManagerImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ protected SettingsManagerImpl(@NotNull PropertyResource resource, @NotNull Confi
* @param <T> the property's type
*/
@Override
public <T> void setProperty(@NotNull Property<T> property, /* PV */ T value) {
public <T> void setProperty(@NotNull Property<T> property, @NotNull T value) {
configurationData.setValue(property, value);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,24 +57,22 @@ public interface ConfigurationData {

/**
* Returns the value associated with the given property. Only to be used with properties contained in
* {@link #getProperties()}. By default, never returns null (since base properties always resolve to a non-null
* value) and throws an exception if the property is unknown.
* {@link #getProperties()}. Throws an exception if the property is unknown.
*
* @param property the property to look up
* @param <T> property type
* @return value associated with the property, or null if not present
* @return value associated with the property
*/
<T> /* PV */ T getValue(@NotNull Property<T> property);
<T> @NotNull T getValue(@NotNull Property<T> property);

/**
* Sets the given value for the given property. May throw an exception
* if the value is not valid.
* Sets the given value for the given property. May throw an exception if the value is not valid.
*
* @param property the property to change the value for
* @param value the value to set
* @param <T> the property type
*/
<T> void setValue(@NotNull Property<T> property, /* PV */ T value);
<T> void setValue(@NotNull Property<T> property, @NotNull T value);

/**
* Returns if the last call of {@link #initializeValues} had fully valid values in the resource
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ protected ConfigurationDataImpl(@NotNull List<? extends Property<?>> allProperti
}

@Override
public <T> void setValue(@NotNull Property<T> property, /* PV */ T value) {
public <T> void setValue(@NotNull Property<T> property, @NotNull T value) {
if (property.isValidValue(value)) {
values.put(property.getPath(), value);
} else {
Expand Down
8 changes: 0 additions & 8 deletions src/main/java/ch/jalu/configme/properties/BaseProperty.java
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,6 @@ public boolean isValidValue(@Nullable T value) {
return value != null;
}

/*
* Overridden to set the param as @NotNull. If the value from #determineValue is not null and #isValidValue rejects
* nulls, it is guaranteed that this method will never be called with a null value.
*/
@Override
@SuppressWarnings("NullableProblems")
public abstract @Nullable Object toExportValue(@NotNull T value);

/**
* Constructs the value of the property from the property reader. Returns null if no value is
* available in the reader or if it cannot be used to construct a value for this property.
Expand Down
11 changes: 5 additions & 6 deletions src/main/java/ch/jalu/configme/properties/Property.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,9 @@ public interface Property<T> {
@NotNull String getPath();

/**
* Returns the value, based on the given reader, which should be used for this property. By default
* this is the value as constructed from the reader, and otherwise the default value. Implementations
* of {@link BaseProperty} never return null. The return value must be in sync with
* {@link #isValidValue(Object)}.
* Returns the value, based on the given reader, which should be used for this property. By default,
* this is the value as constructed from the reader, and otherwise the default value (never null).
* The return value must be in sync with {@link #isValidValue(Object)}.
*
* @param propertyReader the reader to construct the value from (if possible)
* @return the value to associate to this property
Expand All @@ -52,7 +51,7 @@ default boolean isValidInResource(@NotNull PropertyReader propertyReader) {
*
* @return the default value
*/
@Nullable T getDefaultValue();
@NotNull T getDefaultValue();

/**
* Returns whether the value can be associated to the given property, i.e. whether it fulfills all
Expand Down Expand Up @@ -82,6 +81,6 @@ default boolean isValidInResource(@NotNull PropertyReader propertyReader) {
* @param value the value to convert to an export value
* @return value to use for export, null to skip the property
*/
@Nullable Object toExportValue(@Nullable T value);
@Nullable Object toExportValue(@NotNull T value);

}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public class PropertyValue<T> {
* @param value the value associated with the property
* @param isValidInResource true if the value in the resource was fully valid
*/
public PropertyValue(/* PV */ T value, boolean isValidInResource) {
public PropertyValue(@NotNull T value, boolean isValidInResource) {
this.value = value;
this.isValidInResource = isValidInResource;
}
Expand All @@ -39,7 +39,7 @@ public PropertyValue(/* PV */ T value, boolean isValidInResource) {
* @param <T> the value type
* @return property value with the given value and the valid flag set to true
*/
public static <T> @NotNull PropertyValue<T> withValidValue(/* PV */ T value) {
public static <T> @NotNull PropertyValue<T> withValidValue(@NotNull T value) {
return new PropertyValue<>(value, true);
}

Expand All @@ -50,14 +50,14 @@ public PropertyValue(/* PV */ T value, boolean isValidInResource) {
* @param <T> the value type
* @return property value with the given value and the valid flag set to false
*/
public static <T> @NotNull PropertyValue<T> withValueRequiringRewrite(/* PV */ T value) {
public static <T> @NotNull PropertyValue<T> withValueRequiringRewrite(@NotNull T value) {
return new PropertyValue<>(value, false);
}

/**
* @return the value to associate with the property
*/
public /* PV */ T getValue() {
public @NotNull T getValue() {
return value;
}

Expand Down
12 changes: 5 additions & 7 deletions src/test/java/ch/jalu/configme/TestUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -160,15 +160,13 @@ public static Class<? extends Exception> getExceptionTypeForNullArg() {
}

/**
* Returns the expected exception type when a null value (from a field) is supplied where it is not allowed.
* See {@link #getExceptionTypeForNullArg()} for the background.
* Returns whether the code was compiled such that {@link NotNull} parameters are checked to ensure they are not
* null, which happens when the code is locally built in IntelliJ.
*
* @return the expected exception type for a null state where it is not allowed (NPE or IllegalStateException)
* @return true if NotNull is checked in methods (= local IntelliJ builds), false otherwise
*/
public static Class<? extends Exception> getExceptionTypeForNullField() {
return getOrCaptureNullExceptionType().equals(NullPointerException.class)
? NullPointerException.class
: IllegalStateException.class;
public static boolean hasBytecodeCheckForNotNullAnnotation() {
return !getOrCaptureNullExceptionType().equals(NullPointerException.class);
}

// -------------
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package ch.jalu.configme.configurationdata;

import ch.jalu.configme.TestUtils;
import ch.jalu.configme.exception.ConfigMeException;
import ch.jalu.configme.properties.Property;
import ch.jalu.configme.properties.convertresult.PropertyValue;
Expand Down Expand Up @@ -65,8 +66,13 @@ void shouldThrowForInvalidValue() {
ConfigurationData configData = new ConfigurationDataImpl(properties, Collections.emptyMap());

// when / then
verifyException(() -> configData.setValue(properties.get(0), null),
ConfigMeException.class, "Invalid value");
if (TestUtils.hasBytecodeCheckForNotNullAnnotation()) {
verifyException(() -> configData.setValue(properties.get(0), null),
IllegalArgumentException.class, "Argument for @NotNull parameter 'value'");
} else {
verifyException(() -> configData.setValue(properties.get(0), null),
ConfigMeException.class, "Invalid value");
}
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ protected Byte getFromReader(@NotNull PropertyReader reader, @NotNull ConvertErr
}

@Override
public Object toExportValue(Byte value) {
public Object toExportValue(@NotNull Byte value) {
return value;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ protected Set<TestEnum> getFromReader(@NotNull PropertyReader reader,
}

@Override
public List<String> toExportValue(Set<TestEnum> value) {
public List<String> toExportValue(@NotNull Set<TestEnum> value) {
return transform(value, Enum::name);
}
}
Expand Down

0 comments on commit 844da6b

Please sign in to comment.