Skip to content

Commit

Permalink
#5 Add validation for SettingsHolder classes: final and hidden constr…
Browse files Browse the repository at this point in the history
…uctor
  • Loading branch information
ljacqu committed Dec 28, 2019
1 parent ebcb509 commit b3f6613
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import ch.jalu.configme.properties.Property;

import javax.annotation.Nullable;
import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.Modifier;
import java.util.ArrayList;
Expand All @@ -30,26 +31,28 @@ public class SettingsHolderClassValidator {
* Runs all validations of this class with the given settings holder classes.
* More details at {@link #validate(Iterable)}.
*
* @param settingsHolderClasses settings holder classes that make up the configuration data of the project
* @param settingHolders settings holder classes that make up the configuration data of the project
*/
@SafeVarargs
public final void validate(Class<? extends SettingsHolder>... settingsHolderClasses) {
validate(Arrays.asList(settingsHolderClasses));
public final void validate(Class<? extends SettingsHolder>... settingHolders) {
validate(Arrays.asList(settingHolders));
}

/**
* Runs all validations of this class with the given settings holder classes. Some of the validations
* are not needed from a technical point of view and may be undesired in your project. They can all be
* run individually and can be customized (method parameters, or overridable methods).
*
* @param settingsHolderClasses settings holder classes that make up the configuration data of the project
* @param settingHolders settings holder classes that make up the configuration data of the project
*/
public void validate(Iterable<Class<? extends SettingsHolder>> settingsHolderClasses) {
public void validate(Iterable<Class<? extends SettingsHolder>> settingHolders) {
validateAllPropertiesAreConstants(settingHolders);
validateSettingsHolderClassesFinal(settingHolders);
validateClassesHaveHiddenNoArgConstructor(settingHolders);

// Note: creating the ConfigurationData with the default builder validates that
// no properties have overlapping paths
ConfigurationData configurationData = createConfigurationData(settingsHolderClasses);

validateAllPropertiesAreConstants(settingsHolderClasses);
ConfigurationData configurationData = createConfigurationData(settingHolders);
validateHasCommentOnEveryProperty(configurationData, null);
validateCommentLengthsAreWithinBounds(configurationData, null, 90);
validateHasAllEnumEntriesInComment(configurationData, null);
Expand All @@ -61,12 +64,12 @@ public void validate(Iterable<Class<? extends SettingsHolder>> settingsHolderCla
/**
* Throws an exception if any Property field of the given classes is not public, static, or final.
*
* @param settingsHolderClasses the classes to check
* @param settingHolders the classes to check
*/
public void validateAllPropertiesAreConstants(Iterable<Class<? extends SettingsHolder>> settingsHolderClasses) {
public void validateAllPropertiesAreConstants(Iterable<Class<? extends SettingsHolder>> settingHolders) {
List<String> invalidFields = new ArrayList<>();

for (Class<? extends SettingsHolder> clazz : settingsHolderClasses) {
for (Class<? extends SettingsHolder> clazz : settingHolders) {
List<String> invalidFieldsForClazz = getAllFields(clazz)
.filter(field -> Property.class.isAssignableFrom(field.getType()))
.filter(field -> !isValidConstantField(field))
Expand All @@ -81,6 +84,47 @@ public void validateAllPropertiesAreConstants(Iterable<Class<? extends SettingsH
}
}

/**
* Throws an exception if any of the provided settings holder classes is not final.
*
* @param settingHolders the classes to check
*/
public void validateSettingsHolderClassesFinal(Iterable<Class<? extends SettingsHolder>> settingHolders) {
List<String> invalidClasses = new ArrayList<>();

for (Class<? extends SettingsHolder> clazz : settingHolders) {
if (!Modifier.isFinal(clazz.getModifiers())) {
invalidClasses.add(clazz.getCanonicalName());
}
}

if (!invalidClasses.isEmpty()) {
throw new IllegalStateException("The following classes are not final:\n- "
+ String.join("\n- ", invalidClasses));
}
}

/**
* Throws an exception if any of the provided setting holder classes does not have a single private
* no-args constructor.
*
* @param settingHolders the classes to check
*/
public void validateClassesHaveHiddenNoArgConstructor(Iterable<Class<? extends SettingsHolder>> settingHolders) {
List<String> invalidClasses = new ArrayList<>();

for (Class<? extends SettingsHolder> clazz : settingHolders) {
if (!hasValidConstructorSetup(clazz)) {
invalidClasses.add(clazz.getCanonicalName());
}
}

if (!invalidClasses.isEmpty()) {
throw new IllegalStateException("The following classes do not have a single no-args private constructor:"
+ "\n- " + String.join("\n- ", invalidClasses));
}
}

/**
* Throws an exception if there isn't a non-empty comment for every property in the configuration data.
*
Expand Down Expand Up @@ -232,6 +276,13 @@ protected List<String> gatherExpectedEnumNames(Class<? extends Enum<?>> enumClas
.collect(Collectors.toList());
}

protected boolean hasValidConstructorSetup(Class<? extends SettingsHolder> clazz) {
Constructor<?>[] constructors = clazz.getDeclaredConstructors();
return constructors.length == 1
&& constructors[0].getParameterCount() == 0
&& Modifier.isPrivate(constructors[0].getModifiers());
}

/**
* Returns all fields of the class, including all fields of parent classes, recursively.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ public class SettingsHolderWithInvalidConstants implements SettingsHolder {
private SettingsHolderWithInvalidConstants() {
}

// used for test ensuring classes only have a sole no-args private constructor
private SettingsHolderWithInvalidConstants(boolean other) {
}

public static class Child extends SettingsHolderWithInvalidConstants {

private final Property<String> strProp = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ void shouldValidateSuccessfully() {
// then - no exception
Matcher<Iterable<Class<? extends SettingsHolder>>> matcher = (Matcher) contains(FullyValidSettingsHolder1.class, FullyValidSettingsHolder2.class);
verify(validatorSpy).validateAllPropertiesAreConstants(argThat(matcher));
verify(validatorSpy).validateSettingsHolderClassesFinal(argThat(matcher));
verify(validatorSpy).validateClassesHaveHiddenNoArgConstructor(argThat(matcher));
verify(validatorSpy).validateHasCommentOnEveryProperty(any(ConfigurationData.class), isNull());
verify(validatorSpy).validateCommentLengthsAreWithinBounds(any(ConfigurationData.class), isNull(), eq(90));
verify(validatorSpy).validateHasAllEnumEntriesInComment(any(ConfigurationData.class), isNull());
Expand Down Expand Up @@ -165,4 +167,36 @@ void shouldVerifyAllEnumEntriesInComment() {
assertThat(e2.getMessage(), equalTo("The following enum properties do not list all enum values:"
+ "\n- For Property 'sample.gameMode': missing CREATIVE, SURVIVAL"));
}

@Test
void shouldThrowForNonFinalClasses() {
// given
List<Class<? extends SettingsHolder>> classes = Arrays.asList(
SettingsHolderWithEnumPropertyComments.class, FullyValidSettingsHolder1.class, SettingsHolderWithInvalidConstants.class);

// when
IllegalStateException e = assertThrows(IllegalStateException.class,
() -> validator.validateSettingsHolderClassesFinal(classes));

// then
assertThat(e.getMessage(), equalTo("The following classes are not final:"
+ "\n- ch.jalu.configme.samples.settingsholders.SettingsHolderWithEnumPropertyComments"
+ "\n- ch.jalu.configme.samples.settingsholders.SettingsHolderWithInvalidConstants"));
}

@Test
void shouldThrowForClassWithoutPrivateConstructor() {
// given
List<Class<? extends SettingsHolder>> classes = Arrays.asList(
SettingsHolderWithEnumPropertyComments.class, FullyValidSettingsHolder1.class, SettingsHolderWithInvalidConstants.class);

// when
IllegalStateException e = assertThrows(IllegalStateException.class,
() -> validator.validateClassesHaveHiddenNoArgConstructor(classes));

// then
assertThat(e.getMessage(), equalTo("The following classes do not have a single no-args private constructor:"
+ "\n- ch.jalu.configme.samples.settingsholders.SettingsHolderWithEnumPropertyComments"
+ "\n- ch.jalu.configme.samples.settingsholders.SettingsHolderWithInvalidConstants"));
}
}

0 comments on commit b3f6613

Please # to comment.