Skip to content

Commit

Permalink
added fix to handle references to types that extend other types
Browse files Browse the repository at this point in the history
  • Loading branch information
jacek99 committed Jul 3, 2015
1 parent eac997c commit 35c459b
Show file tree
Hide file tree
Showing 7 changed files with 90 additions and 18 deletions.
3 changes: 0 additions & 3 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,6 @@ bintray {
user = "$System.env.BINTRAY_USER"
key = "$System.env.BINTRAY_API_KEY"

print user
print key

configurations = ['published', 'archives']
dryRun = false //Whether to run this as dry-run, without deploying
publish = true //If version should be auto published after an upload
Expand Down
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# speed up Gradle
#org.gradle.daemon=true

version = 0.1.0
version = 0.2.0
group = org.immutizer4j
25 changes: 12 additions & 13 deletions src/main/java/org/immutizer4j/Immutizer.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import java.lang.reflect.*;
import java.util.Collection;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ConcurrentMap;

/**
Expand Down Expand Up @@ -111,7 +111,7 @@ public ValidationResult getValidationResult(Class<?> clazz) {
// all good, we verified this type before
return validationCache.get(clazz);
} else {
ValidationResult result = performValidation(clazz);
ValidationResult result = validateType(clazz);
//remember that it was fine
validationCache.putIfAbsent(clazz, result);
return result;
Expand All @@ -123,15 +123,19 @@ public ValidationResult getValidationResult(Class<?> clazz) {

}

// performs actual walk down the graph hierarchy
private ValidationResult performValidation(Class<?> entity) {
// performs actual walk down the graph hierarchy starting from the root object
private ValidationResult validateType(Class<?> entity) {
return validateType(entity, new ValidationResult(ImmutableSet.<ValidationError>of()));
}

// performs actual walk down the graph hierarchy within an existing validation process
private ValidationResult validateType(Class<?> entity, ValidationResult result) {
Class<?> current = entity;
ValidationResult result = new ValidationResult(ImmutableSet.<ValidationError>of());
while (current != null && !current.equals(Object.class)) {

Field[] fields = current.getDeclaredFields();
for(Field field : fields) {
result = validateField(field, result, Optional.<Field>empty());
result = validateField(field, result);
}

// move up the class hierarchy level
Expand All @@ -142,7 +146,7 @@ private ValidationResult performValidation(Class<?> entity) {
}

// performs all the validations for a single field
private ValidationResult validateField(Field field, ValidationResult result, Optional<Field> parent) {
private ValidationResult validateField(Field field, ValidationResult result) {

if (!Modifier.isStatic(field.getModifiers())) {

Expand All @@ -159,13 +163,8 @@ private ValidationResult validateField(Field field, ValidationResult result, Opt
Class<?> actualType = (field.getType().isArray()) ? field.getType().getComponentType() : field.getType();

if (!isSafeType(actualType)) {

for(Field childField : actualType.getDeclaredFields()) {
log.debug("Validating {}.{}", childField.getDeclaringClass().getSimpleName(), childField.getName());
result = validateField(childField, result, Optional.of(field));
}
result = validateType(actualType, result);
}

}

return result;
Expand Down
37 changes: 36 additions & 1 deletion src/test/java/org/immutizer4j/test/BaseTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,41 @@ public void testGetValidationResultForNull() {
@Test
public void testImmutablePojo() {
ValidationResult result = defaultImmutizer.getValidationResult(ImmutablePojo.class);
assertEquals(true,result.isValid());
assertEquals(true, result.isValid());
}

/**
* We should not just test a particular object's field, but all of its ancestors as well
*/
@Test
public void testAllFieldsInAnObjectHierarchy() {
ValidationResult result = defaultImmutizer.getValidationResult(ChildPojo.class);

assertEquals(false,result.isValid());
// we should get 1 error from the ParentPojo, one from the ChildPojo
assertEquals(2, result.getErrors().size());

assertTrue(result.toString(),
result.toString().contains("org.immutizer4j.test.sample.ChildPojo.childMutableInt : NON_FINAL_FIELD"));
assertTrue(result.toString(),
result.toString().contains("org.immutizer4j.test.sample.ParentPojo.paretMutableInt : NON_FINAL_FIELD"));
}


/**
* When validating fields we should not just walk up the inheritance hierarchy for the root type, but for all
* the referenced types as well
*/
@Test
public void testReferencesToAllFieldsInAnObjectHierarchy() {
ValidationResult result = defaultImmutizer.getValidationResult(ChildPojoReferencePojo.class);

assertEquals(false,result.isValid());
// we should get 1 error from the ParentPojo, one from the ChildPojo
assertEquals(2, result.getErrors().size());
assertTrue(result.toString(),
result.toString().contains("org.immutizer4j.test.sample.ChildPojo.childMutableInt : NON_FINAL_FIELD"));
assertTrue(result.toString(),
result.toString().contains("org.immutizer4j.test.sample.ParentPojo.paretMutableInt : NON_FINAL_FIELD"));
}
}
14 changes: 14 additions & 0 deletions src/test/java/org/immutizer4j/test/sample/ChildPojo.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package org.immutizer4j.test.sample;

import lombok.Data;
import lombok.EqualsAndHashCode;

/**
* A POJO that extends the parent POJO
* Used for testing whether we check all fields in an object's inheritance hierarchy
*/
@Data @EqualsAndHashCode(callSuper = true)
public class ChildPojo extends ParentPojo {
private final int childImmutableInt = 0;
private int childMutableInt = 0;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package org.immutizer4j.test.sample;

import lombok.Value;

/**
* A POJO that references ChildPojo
* We want to ensure that when it walks down the object graph, it goes up the inheritance hierarchy not just for the
* root object but for every referenced type as well
*/
@Value
public class ChildPojoReferencePojo {
// final refernce to a mutable POJO with mutable fields in both parent and concrete type
private ChildPojo childPojo;
}
13 changes: 13 additions & 0 deletions src/test/java/org/immutizer4j/test/sample/ParentPojo.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package org.immutizer4j.test.sample;

import lombok.Data;

/**
* A POJO that will be used as an ancestor for other POJOs
* Contains a mix of mutable and non-mutable fields
*/
@Data
public class ParentPojo {
private final int parentImmutableInt = 0;
private int paretMutableInt = 0;
}

0 comments on commit 35c459b

Please sign in to comment.