Skip to content

Commit

Permalink
Revert "updated additional properties rule to respect includeAccessor…
Browse files Browse the repository at this point in the history 
…s flag"
  • Loading branch information
@ctrimble
ctrimble committed Nov 25, 2015
1 parent 6b157e5 commit 97f01ca
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 33 deletions.
13 changes: 4 additions & 9 deletions jsonschema2pojo-core/src/main/java/org/jsonschema2pojo/rules/AdditionalPropertiesRule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,9 @@ public JDefinedClass apply(String nodeName, JsonNode node, JDefinedClass jclass,

JFieldVar field = addAdditionalPropertiesField(jclass, propertyType);

if (ruleFactory.getGenerationConfig().isIncludeAccessors()) {
addGetter(jclass, field);
addSetter(jclass, propertyType, field);
}
addGetter(jclass, field);

addSetter(jclass, propertyType, field);

if (ruleFactory.getGenerationConfig().isGenerateBuilders()) {
addBuilder(jclass, propertyType, field);
Expand All @@ -126,7 +125,7 @@ private JFieldVar addAdditionalPropertiesField(JDefinedClass jclass, JType prope
JClass propertiesMapImplType = jclass.owner().ref(HashMap.class);
propertiesMapImplType = propertiesMapImplType.narrow(jclass.owner().ref(String.class), propertyType.boxify());

JFieldVar field = jclass.field(fieldScopeModifier(), propertiesMapType, "additionalProperties");
JFieldVar field = jclass.field(JMod.PRIVATE, propertiesMapType, "additionalProperties");

ruleFactory.getAnnotator().additionalPropertiesField(field, jclass, "additionalProperties");

Expand All @@ -135,10 +134,6 @@ private JFieldVar addAdditionalPropertiesField(JDefinedClass jclass, JType prope
return field;
}

private int fieldScopeModifier() {
return ruleFactory.getGenerationConfig().isIncludeAccessors() ? JMod.PRIVATE : JMod.PUBLIC;
}

private void addSetter(JDefinedClass jclass, JType propertyType, JFieldVar field) {
JMethod setter = jclass.method(JMod.PUBLIC, void.class, "setAdditionalProperty");

Expand Down
98 changes: 74 additions & 24 deletions ...ts/src/test/java/org/jsonschema2pojo/integration/config/IncludeAccessorsPropertiesIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,19 @@
import static org.hamcrest.Matchers.*;
import static org.jsonschema2pojo.integration.util.CodeGenerationHelper.*;
import static org.junit.Assert.*;
import static org.fest.util.Lists.newArrayList;

import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.lang.reflect.Member;
import java.lang.reflect.Modifier;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import org.hamcrest.Description;
import org.hamcrest.Matcher;
import org.hamcrest.TypeSafeDiagnosingMatcher;
import org.hamcrest.TypeSafeMatcher;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized.Parameters;
Expand Down Expand Up @@ -74,52 +76,100 @@ public void noGettersOrSettersWhenFalse() throws ClassNotFoundException, Securit
ClassLoader resultsClassLoader = generateAndCompile(path, PACKAGE, includeAccessorsFalse);
Class generatedType = resultsClassLoader.loadClass(typeName);

for (Method method : generatedType.getDeclaredMethods()) {
assertThat("getters and setters should not exist", method.getName(), not(anyOf(startsWith("get"), startsWith("set"))));
}
assertThat("getters and setters should not exist", generatedType.getDeclaredMethods(), everyItemInArray(anyOf(methodWhitelist(), not(fieldGetterOrSetter()))));
}

@Test
public void hasGettersOrSettersWhenTrue() throws ClassNotFoundException, SecurityException, NoSuchMethodException, NoSuchFieldException {
ClassLoader resultsClassLoader = generateAndCompile(path, PACKAGE, includeAccessorsTrue);
Class generatedType = resultsClassLoader.loadClass(typeName);

List<String> methodNames = new ArrayList<String>();
for (Method method : generatedType.getDeclaredMethods()) {
methodNames.add(method.getName());
}
assertThat("a getter or setter should be found.", methodNames, hasItem(anyOf(startsWith("get"), startsWith("set"))));
assertThat("a getter or setter should be found.", generatedType.getDeclaredMethods(), hasItemInArray(allOf(not(methodWhitelist()), fieldGetterOrSetter())));
}

@Test
public void onlyHasPublicInstanceFieldsWhenFalse() throws ClassNotFoundException, SecurityException, NoSuchMethodException, NoSuchFieldException {
ClassLoader resultsClassLoader = generateAndCompile(path, PACKAGE, includeAccessorsFalse);
Class generatedType = resultsClassLoader.loadClass(typeName);

for (Field field : generatedType.getDeclaredFields()) {
if (Modifier.isStatic(field.getModifiers())) {
continue;
}
assertThat("only public instance fields exist", Modifier.isPublic(field.getModifiers()), equalTo(true));
}
assertThat("only public instance fields exist", generatedType.getDeclaredFields(), everyItemInArray(anyOf(hasModifiers(Modifier.STATIC), fieldWhitelist(), hasModifiers(Modifier.PUBLIC))));
}

@Test
public void noPublicInstanceFieldsWhenTrue() throws ClassNotFoundException, SecurityException, NoSuchMethodException, NoSuchFieldException {
ClassLoader resultsClassLoader = generateAndCompile(path, PACKAGE, includeAccessorsTrue);
Class generatedType = resultsClassLoader.loadClass(typeName);

for (Field field : generatedType.getDeclaredFields()) {
if (Modifier.isStatic(field.getModifiers())) {
continue;
}
assertThat("only public instance fields exist", Modifier.isPublic(field.getModifiers()), not(equalTo(true)));
}
assertThat("only public instance fields exist", generatedType.getDeclaredFields(), everyItemInArray(anyOf(not(hasModifiers(Modifier.PUBLIC)), fieldWhitelist())));
}

private static Map<String, Object> configWithIncludeAccessors(Map<String, Object> template, boolean includeAccessors) {
Map<String, Object> config = new HashMap<String, Object>(template);
config.put("includeAccessors", includeAccessors);
return config;
}

private static <M extends Member> Matcher<M> hasModifiers(final int modifiers) {
return new TypeSafeMatcher<M>() {
@Override
public void describeTo(Description description) {
description.appendText("has modifier ").appendValue(Modifier.toString(modifiers));
}

@Override
protected boolean matchesSafely(M item) {
int masked = item.getModifiers() & modifiers;
return masked == modifiers;
}
};
}

private static <M extends Member> Matcher<M> nameMatches(final Matcher<String> nameMatcher) {
return new TypeSafeMatcher<M>() {
@Override
public void describeTo(Description description) {
description.appendText("name ").appendDescriptionOf(nameMatcher);
}

@Override
protected boolean matchesSafely(M item) {
return nameMatcher.matches(item.getName());
}
};
}

private static <T> Matcher<T[]> everyItemInArray(final Matcher<T> itemMatcher) {
return new TypeSafeDiagnosingMatcher<T[]>() {

@Override
public void describeTo(Description description) {
description.appendText("every item in array is ").appendDescriptionOf(itemMatcher);
}

@Override
protected boolean matchesSafely(T[] items, Description mismatchDescription) {
for (T item : items) {
if (!itemMatcher.matches(item)) {
mismatchDescription.appendText("an item ");
itemMatcher.describeMismatch(item, mismatchDescription);
return false;
}
}
return true;
}

};
}

private static <M extends Member> Matcher<M> methodWhitelist() {
return nameMatches(isIn(newArrayList("setAdditionalProperty", "getAdditionalProperties")));
}

private static <M extends Member> Matcher<M> fieldWhitelist() {
return nameMatches(isIn(newArrayList("additionalProperties")));
}

private static <M extends Member> Matcher<M> fieldGetterOrSetter() {
return nameMatches(anyOf(startsWith("get"), startsWith("set")));
}
}

2 comments on commit 97f01ca

@ctrimble
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joelittlejohn I have corrected the problem you noticed with additional properties and the includeAccessors flag. I didn't bother with a PR, since I ended up reverting the changes in functionality. I added tests in 6b157e5 to make sure this flag combination will stay working.

@joelittlejohn
Copy link
Owner

@joelittlejohn joelittlejohn commented on 97f01ca Nov 25, 2015 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.