Skip to content

Commit

Permalink
Add Javadoc to constructors parameters for documented properties
Browse files Browse the repository at this point in the history 
Closes #1463
  • Loading branch information
@unkish
unkish committed Feb 10, 2023
1 parent 25bfc76 commit 876bc7e
Show file tree
Hide file tree
Showing 14 changed files with 333 additions and 80 deletions.
85 changes: 49 additions & 36 deletions jsonschema2pojo-core/src/main/java/org/jsonschema2pojo/rules/ConstructorRule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,14 @@
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedHashSet;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;
import java.util.Set;

import org.apache.commons.lang3.StringUtils;
import org.jsonschema2pojo.GenerationConfig;
import org.jsonschema2pojo.Schema;
import org.jsonschema2pojo.util.NameHelper;
Expand Down Expand Up @@ -69,10 +70,10 @@ public JDefinedClass apply(String nodeName, JsonNode node, JsonNode parent, JDef
return instanceClass;
}

private JDefinedClass handleLegacyConfiguration(JsonNode node, JDefinedClass instanceClass, Schema currentSchema) {
private void handleLegacyConfiguration(JsonNode node, JDefinedClass instanceClass, Schema currentSchema) {
// Determine which properties belong to that class (or its superType/parent)
LinkedHashSet<String> requiredClassProperties = getConstructorProperties(node, true);
LinkedHashSet<String> requiredCombinedSuperProperties = getSuperTypeConstructorPropertiesRecursive(node, currentSchema, true);
Map<String, String> requiredClassProperties = getConstructorProperties(node, true);
Map<String, String> requiredCombinedSuperProperties = getSuperTypeConstructorPropertiesRecursive(node, currentSchema, true);

// Only generate the constructors if there are actually properties to put in them
if (!requiredClassProperties.isEmpty() || !requiredCombinedSuperProperties.isEmpty()) {
Expand All @@ -84,20 +85,17 @@ private JDefinedClass handleLegacyConfiguration(JsonNode node, JDefinedClass ins
// Generate the actual constructor taking in only the required properties
addFieldsConstructor(instanceClass, requiredClassProperties, requiredCombinedSuperProperties);
}

// Return the original class we modified
return instanceClass;
}

private void handleMultiChoiceConstructorConfiguration(JsonNode node, JDefinedClass instanceClass, Schema currentSchema) {
// Use this flag to keep track of whether or not we'll actually need to generate any constructors
boolean requiresConstructors = false;

// Use these lists to keep track of the properties on the class, but we'll only populate them if we need to
LinkedHashSet<String> requiredClassProperties = null;
LinkedHashSet<String> classProperties = null;
LinkedHashSet<String> requiredCombinedSuperProperties = null;
LinkedHashSet<String> combinedSuperProperties = null;
Map<String, String> requiredClassProperties = null;
Map<String, String> classProperties = null;
Map<String, String> requiredCombinedSuperProperties = null;
Map<String, String> combinedSuperProperties = null;

GenerationConfig generationConfig = ruleFactory.getGenerationConfig();
boolean includeCopyConstructor = generationConfig.isIncludeCopyConstructor();
Expand Down Expand Up @@ -139,7 +137,7 @@ private void handleMultiChoiceConstructorConfiguration(JsonNode node, JDefinedCl
}
}

private void addFieldsConstructor(JDefinedClass instanceClass, Set<String> classProperties, Set<String> combinedSuperProperties) {
private void addFieldsConstructor(JDefinedClass instanceClass, Map<String, String> classProperties, Map<String, String> combinedSuperProperties) {
GenerationConfig generationConfig = ruleFactory.getGenerationConfig();

// Generate the constructor with the properties which were located
Expand All @@ -154,7 +152,7 @@ private void addFieldsConstructor(JDefinedClass instanceClass, Set<String> class
}
}

private void addCopyConstructor(JDefinedClass instanceClass, Set<String> classProperties, Set<String> combinedSuperProperties) {
private void addCopyConstructor(JDefinedClass instanceClass, Map<String, String> classProperties, Map<String, String> combinedSuperProperties) {
GenerationConfig generationConfig = ruleFactory.getGenerationConfig();

// Generate the constructor with the properties which were located
Expand All @@ -173,13 +171,13 @@ private void addCopyConstructor(JDefinedClass instanceClass, Set<String> classPr
* Retrieve the list of properties to go in the constructor from node. This is all properties listed in node["properties"] if ! onlyRequired, and
* only required properties if onlyRequired.
*/
private LinkedHashSet<String> getConstructorProperties(JsonNode node, boolean onlyRequired) {
private Map<String, String> getConstructorProperties(JsonNode node, boolean onlyRequired) {

if (!node.has("properties")) {
return new LinkedHashSet<>();
return new LinkedHashMap<>();
}

LinkedHashSet<String> rtn = new LinkedHashSet<>();
LinkedHashMap<String, String> rtn = new LinkedHashMap<>();
Set<String> draft4RequiredProperties = new HashSet<>();

// setup the set of required properties for draft4 style "required"
Expand All @@ -200,38 +198,50 @@ private LinkedHashSet<String> getConstructorProperties(JsonNode node, boolean on
Map.Entry<String, JsonNode> property = properties.next();

JsonNode propertyObj = property.getValue();
final String javadoc = getJavadocForProperty(propertyObj);
if (onlyRequired) {
// draft3 style
if (propertyObj.has("required") && propertyObj.get("required")
.asBoolean()) {
rtn.add(nameHelper.getPropertyName(property.getKey(), property.getValue()));
if (propertyObj.has("required") && propertyObj.get("required").asBoolean()) {
rtn.put(nameHelper.getPropertyName(property.getKey(), property.getValue()), javadoc);
}

// draft4 style
if (draft4RequiredProperties.contains(property.getKey())) {
rtn.add(nameHelper.getPropertyName(property.getKey(), property.getValue()));
rtn.put(nameHelper.getPropertyName(property.getKey(), property.getValue()), javadoc);
}
} else {
rtn.add(nameHelper.getPropertyName(property.getKey(), property.getValue()));
rtn.put(nameHelper.getPropertyName(property.getKey(), property.getValue()), javadoc);
}
}
return rtn;
}

private String getJavadocForProperty(JsonNode propertyObj) {
final boolean hasDescription = propertyObj.has("description") && !propertyObj.get("description").isNull();
final String title;
if (propertyObj.has("title") && !propertyObj.get("title").isNull()) {
title = StringUtils.appendIfMissing(propertyObj.get("title").asText(), ".") + " ";
} else {
title = "";
}
final String description = hasDescription ? StringUtils.appendIfMissing(propertyObj.get("description").asText(), ".") : "";
return title + description;
}

/**
* Recursive, walks the schema tree and assembles a list of all properties of this schema's super schemas
*/
private LinkedHashSet<String> getSuperTypeConstructorPropertiesRecursive(JsonNode node, Schema schema, boolean onlyRequired) {
private Map<String, String> getSuperTypeConstructorPropertiesRecursive(JsonNode node, Schema schema, boolean onlyRequired) {
Schema superTypeSchema = reflectionHelper.getSuperSchema(node, schema, true);

if (superTypeSchema == null) {
return new LinkedHashSet<>();
return new LinkedHashMap<>();
}

JsonNode superSchemaNode = superTypeSchema.getContent();

LinkedHashSet<String> rtn = getConstructorProperties(superSchemaNode, onlyRequired);
rtn.addAll(getSuperTypeConstructorPropertiesRecursive(superSchemaNode, superTypeSchema, onlyRequired));
Map<String, String> rtn = getConstructorProperties(superSchemaNode, onlyRequired);
rtn.putAll(getSuperTypeConstructorPropertiesRecursive(superSchemaNode, superTypeSchema, onlyRequired));

return rtn;
}
Expand Down Expand Up @@ -301,14 +311,13 @@ private void generateFieldsConcreteBuilderConstructor(JDefinedClass baseBuilderC
}
}

private JMethod generateCopyConstructor(JDefinedClass jclass, Set<String> classProperties, Set<String> combinedSuperProperties) {
private JMethod generateCopyConstructor(JDefinedClass jclass, Map<String, String> classProperties, Map<String, String> combinedSuperProperties) {

// Create the JMethod for the copy constructor
JMethod copyConstructorResult = jclass.constructor(JMod.PUBLIC);

// Add a parameter to the copyConstructor for the actual object being copied
copyConstructorResult.javadoc()
.addParam("source");
copyConstructorResult.javadoc().addParam("source").append("an actual object being copied");
JVar copyConstructorParam = copyConstructorResult.param(jclass, "source");

// Create the method block for the copyConstructor
Expand All @@ -325,7 +334,7 @@ private JMethod generateCopyConstructor(JDefinedClass jclass, Set<String> classP
// For each of the class properties being set we then need to do something like the following
// this.property = source.property
Map<String, JFieldVar> fields = jclass.fields();
for (String property : classProperties) {
for (String property : classProperties.keySet()) {
JFieldVar field = fields.get(property);

if (field == null) {
Expand All @@ -339,7 +348,7 @@ private JMethod generateCopyConstructor(JDefinedClass jclass, Set<String> classP
return copyConstructorResult;
}

private JMethod generateFieldsConstructor(JDefinedClass jclass, Set<String> classProperties, Set<String> combinedSuperProperties) {
private JMethod generateFieldsConstructor(JDefinedClass jclass, Map<String, String> classProperties, Map<String, String> combinedSuperProperties) {
// add the public constructor with property parameters
JMethod fieldsConstructor = jclass.constructor(JMod.PUBLIC);

Expand All @@ -359,15 +368,17 @@ private JMethod generateFieldsConstructor(JDefinedClass jclass, Set<String> clas
Map<String, JFieldVar> fields = jclass.fields();
Map<String, JVar> classFieldParams = new HashMap<>();

for (String property : classProperties) {
for (Entry<String, String> entry : classProperties.entrySet()) {
String property = entry.getKey();
JFieldVar field = fields.get(property);

if (field == null) {
throw new IllegalStateException("Property " + property + " hasn't been added to JDefinedClass before calling addConstructors");
}

fieldsConstructor.javadoc()
.addParam(property);
if (StringUtils.isNotEmpty(entry.getValue())) {
fieldsConstructor.javadoc().addParam(property).append(entry.getValue());
}
if (generationConfig.isIncludeConstructorPropertiesAnnotation() && constructorPropertiesAnnotation != null) {
constructorPropertiesAnnotation.param(field.name());
}
Expand All @@ -380,7 +391,8 @@ private JMethod generateFieldsConstructor(JDefinedClass jclass, Set<String> clas

List<JVar> superConstructorParams = new ArrayList<>();

for (String property : combinedSuperProperties) {
for (Entry<String, String> entry : combinedSuperProperties.entrySet()) {
String property = entry.getKey();
JFieldVar field = reflectionHelper.searchSuperClassesForField(property, jclass);

if (field == null) {
Expand All @@ -393,8 +405,9 @@ private JMethod generateFieldsConstructor(JDefinedClass jclass, Set<String> clas
param = fieldsConstructor.param(field.type(), field.name());
}

fieldsConstructor.javadoc()
.addParam(property);
if (StringUtils.isNotEmpty(entry.getValue())) {
fieldsConstructor.javadoc().addParam(property).append(entry.getValue());
}
if (generationConfig.isIncludeConstructorPropertiesAnnotation() && constructorPropertiesAnnotation != null) {
constructorPropertiesAnnotation.param(param.name());
}
Expand Down
2 changes: 1 addition & 1 deletion jsonschema2pojo-integration-tests/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@
<artifactId>robolectric</artifactId>
</dependency>
<dependency>
<groupId>qdox</groupId>
<groupId>com.thoughtworks.qdox</groupId>
<artifactId>qdox</artifactId>
</dependency>
<dependency>
Expand Down
12 changes: 5 additions & 7 deletions ...2pojo-integration-tests/src/test/java/org/jsonschema2pojo/integration/ConstructorsIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import org.jsonschema2pojo.integration.util.Jsonschema2PojoRule;
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Ignore;
import org.junit.Test;
import org.junit.experimental.runners.Enclosed;
import org.junit.runner.RunWith;
Expand Down Expand Up @@ -74,8 +73,7 @@ public static Object getValue(Object instance, String methodName) throws NoSuchM
}


@Ignore
public static class ConstructorTestClasses {
private static class ConstructorTestClasses {

protected Class<?> typeWithoutProperties;
protected Class<?> typeWithoutRequiredProperties;
Expand Down Expand Up @@ -104,7 +102,7 @@ public ConstructorTestClasses(Jsonschema2PojoRule classSchemaRule, Map<String, O
/**
* Tests what happens when includeConstructors is set to true
*/
public static class DefaultInlcudeConstructorsIT {
public static class DefaultIncludeConstructorsIT {

@ClassRule
public static Jsonschema2PojoRule classSchemaRule = new Jsonschema2PojoRule();
Expand All @@ -126,7 +124,7 @@ public void testGeneratesConstructorWithAllProperties() throws Exception {
}

@Test
public void testGeneratesCosntructorWithAllPropertiesArrayStyle() throws Exception {
public void testGeneratesConstructorWithAllPropertiesArrayStyle() throws Exception {
assertHasModifier(JMod.PUBLIC, getAllPropertiesConstructor(testClasses.typeWithRequiredArray).getModifiers(), "public");
}

Expand Down Expand Up @@ -225,7 +223,7 @@ public void testGeneratesConstructorWithAllProperties() throws Exception {
}

@Test
public void testGeneratesCosntructorWithAllPropertiesArrayStyle() throws Exception {
public void testGeneratesConstructorWithAllPropertiesArrayStyle() throws Exception {
assertHasModifier(JMod.PUBLIC, getAllPropertiesConstructor(testClasses.typeWithRequiredArray).getModifiers(), "public");
}

Expand Down Expand Up @@ -297,7 +295,7 @@ public void testGeneratesConstructorWithAllProperties() throws Exception {
}

@Test
public void testGeneratesCosntructorWithAllPropertiesArrayStyle() throws Exception {
public void testGeneratesConstructorWithAllPropertiesArrayStyle() throws Exception {
assertHasModifier(JMod.PUBLIC, getAllPropertiesConstructor(testClasses.typeWithRequiredArray).getModifiers(), "public");
}

Expand Down
Loading

0 comments on commit 876bc7e

Please sign in to comment.