Repository: victools/jsonschema-generator
Pull request #64 (2020-04-02 15:09:44)

Preserve Attributes from Custom Property Definitions


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1db1bab..5eddf7e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,11 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [Unreleased]
 ### `jsonschema-generator`
 #### Added
+- Convenience constructor on `SchemaGeneratorConfigBuilder` without explicit `ObjectMapper` instance being expected
 - Convenience methods on `FieldScope`/`MethodScope` for accessing (first level) container item annotations: `getContainerItemAnnotation()` and `getContainerItemAnnotationConsideringFieldAndGetter()`
 
 #### Changed
 - `MethodScope.getAnnotation()` now also considers annotations directly on return type (when no matching annotation was found on the method itself)
 
+#### Fixed
+- Attributes set in custom property definitions should not be overridden by other configurations (except for explicit attribute overrides)
+
 ### `jsonschema-module-javax-validation`
 #### Changed
 - Consider (first level) container item annotations (e.g. List<@Size(min = 3) String>`)
diff --git a/README.md b/README.md
index 8c90546..ba9e78a 100644
--- a/README.md
+++ b/README.md
@@ -36,7 +36,6 @@ It is recommended to use identical versions for all of them to ensure compatibil
 #### Complete/Minimal Example
 ```java
 import com.fasterxml.jackson.databind.JsonNode;
-import com.fasterxml.jackson.databind.ObjectMapper;
 import com.github.victools.jsonschema.generator.OptionPreset;
 import com.github.victools.jsonschema.generator.SchemaGenerator;
 import com.github.victools.jsonschema.generator.SchemaGeneratorConfig;
@@ -44,8 +43,7 @@ import com.github.victools.jsonschema.generator.SchemaGeneratorConfigBuilder;
 import com.github.victools.jsonschema.generator.SchemaVersion;
 ```
 ```java
-ObjectMapper objectMapper = new ObjectMapper();
-SchemaGeneratorConfigBuilder configBuilder = new SchemaGeneratorConfigBuilder(objectMapper, SchemaVersion.DRAFT_2019_09, OptionPreset.PLAIN_JSON);
+SchemaGeneratorConfigBuilder configBuilder = new SchemaGeneratorConfigBuilder(SchemaVersion.DRAFT_2019_09, OptionPreset.PLAIN_JSON);
 SchemaGeneratorConfig config = configBuilder.build();
 SchemaGenerator generator = new SchemaGenerator(config);
 JsonNode jsonSchema = generator.generateSchema(YourClass.class);
diff --git a/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/SchemaGeneratorConfigBuilder.java b/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/SchemaGeneratorConfigBuilder.java
index bf223e6..2aa6692 100644
--- a/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/SchemaGeneratorConfigBuilder.java
+++ b/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/SchemaGeneratorConfigBuilder.java
@@ -65,6 +65,17 @@ public class SchemaGeneratorConfigBuilder {
         this(objectMapper, schemaVersion, OptionPreset.FULL_DOCUMENTATION);
     }
 
+    /**
+     * Constructor of an empty configuration builder with a default {@link ObjectMapper} instance. This is equivalent to calling:<br>
+     * {@code new SchemaGeneratorConfigBuilder(schemaVersion, OptionPreset.FULL_DOCUMENTATION)}
+     *
+     * @param schemaVersion designated JSON Schema version
+     * @see #SchemaGeneratorConfigBuilder(ObjectMapper, SchemaVersion, OptionPreset)
+     */
+    public SchemaGeneratorConfigBuilder(SchemaVersion schemaVersion) {
+        this(new ObjectMapper(), schemaVersion, OptionPreset.FULL_DOCUMENTATION);
+    }
+
     /**
      * Constructor of an empty configuration builder for a {@link SchemaVersion#DRAFT_7 Draft 7} schema. This is equivalent to calling:<br>
      * {@code new SchemaGeneratorConfigBuilder(objectMapper, SchemaVersion.DRAFT_7, preset)}
@@ -78,6 +89,16 @@ public class SchemaGeneratorConfigBuilder {
         this(objectMapper, SchemaVersion.DRAFT_7, preset);
     }
 
+    /**
+     * Constructor of an empty configuration builder with a default {@link ObjectMapper} instance.
+     *
+     * @param schemaVersion designated JSON Schema version
+     * @param preset default settings for standard {@link Option} values
+     */
+    public SchemaGeneratorConfigBuilder(SchemaVersion schemaVersion, OptionPreset preset) {
+        this(new ObjectMapper(), schemaVersion, preset);
+    }
+
     /**
      * Constructor of an empty configuration builder.
      *
diff --git a/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/impl/SchemaGenerationContextImpl.java b/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/impl/SchemaGenerationContextImpl.java
index 054827b..4073453 100644
--- a/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/impl/SchemaGenerationContextImpl.java
+++ b/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/impl/SchemaGenerationContextImpl.java
@@ -41,6 +41,7 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -661,15 +662,10 @@ public class SchemaGenerationContextImpl implements SchemaGenerationContext {
         if (customDefinition != null && customDefinition.isMeantToBeInline()) {
             targetNode.setAll(customDefinition.getValue());
             if (customDefinition.shouldIncludeAttributes()) {
-                if (collectedAttributes != null && collectedAttributes.size() > 0) {
-                    targetNode.setAll(collectedAttributes);
-                }
+                this.addMissingAttributes(targetNode, collectedAttributes);
                 Set<String> allowedSchemaTypes = this.collectAllowedSchemaTypes(targetNode);
                 ObjectNode typeAttributes = AttributeCollector.collectTypeAttributes(scope, this, allowedSchemaTypes);
-                // ensure no existing attributes in the 'definition' are replaced, by way of first overriding any conflicts the other way around
-                typeAttributes.setAll(targetNode);
-                // apply merged attributes
-                targetNode.setAll(typeAttributes);
+                this.addMissingAttributes(targetNode, typeAttributes);
             }
             if (isNullable) {
                 this.makeNullable(targetNode);
@@ -684,7 +680,7 @@ public class SchemaGenerationContextImpl implements SchemaGenerationContext {
             } else if (customDefinition == null && scope.isContainerType()) {
                 // same as above, but the collected attributes should be applied also for containers/arrays
                 referenceContainer = targetNode;
-                referenceContainer.setAll(collectedAttributes);
+                this.addMissingAttributes(targetNode, collectedAttributes);
             } else {
                 // avoid mixing potential "$ref" element with contextual attributes by introducing an "allOf" wrapper
                 // this is only relevant for DRAFT_7 and is being cleaned-up afterwards for newer DRAFT versions
@@ -702,6 +698,25 @@ public class SchemaGenerationContextImpl implements SchemaGenerationContext {
         }
     }
 
+    /**
+     * Merge the second node's attributes into the first, skipping those attributes that are already contained in the first node.
+     *
+     * @param targetNode node to add non-existent attributes to
+     * @param attributeContainer container holding attributes to add to the first node
+     */
+    private void addMissingAttributes(ObjectNode targetNode, ObjectNode attributeContainer) {
+        if (attributeContainer == null) {
+            return;
+        }
+        Iterator<Map.Entry<String, JsonNode>> attributeIterator = attributeContainer.fields();
+        while (attributeIterator.hasNext()) {
+            Map.Entry<String, JsonNode> attribute = attributeIterator.next();
+            if (!targetNode.has(attribute.getKey())) {
+                targetNode.set(attribute.getKey(), attribute.getValue());
+            }
+        }
+    }
+
     @Override
     public ObjectNode makeNullable(ObjectNode node) {
         if (node.has(this.getKeyword(SchemaKeyword.TAG_REF))


Test patch:
diff --git a/jsonschema-generator/src/test/java/com/github/victools/jsonschema/generator/SchemaGeneratorCustomDefinitionsTest.java b/jsonschema-generator/src/test/java/com/github/victools/jsonschema/generator/SchemaGeneratorCustomDefinitionsTest.java
index 19bceae..525f9fc 100644
--- a/jsonschema-generator/src/test/java/com/github/victools/jsonschema/generator/SchemaGeneratorCustomDefinitionsTest.java
+++ b/jsonschema-generator/src/test/java/com/github/victools/jsonschema/generator/SchemaGeneratorCustomDefinitionsTest.java
@@ -99,20 +99,25 @@ public class SchemaGeneratorCustomDefinitionsTest {
                     // using SchemaGenerationContext.createStandardDefinition() to avoid endless loop with this custom definition
                     ObjectNode standardDefinition = context.createStandardDefinition(context.getTypeContext().resolve(Integer.class), this);
                     standardDefinition.put("$comment", "custom override of Integer");
+                    standardDefinition.put(context.getKeyword(SchemaKeyword.TAG_TITLE), "custom title");
                     return new CustomDefinition(standardDefinition);
                 }
                 return null;
             }
         };
-        SchemaGeneratorConfig config = new SchemaGeneratorConfigBuilder(new ObjectMapper(), schemaVersion)
-                .with(customDefinitionProvider)
-                .build();
-        SchemaGenerator generator = new SchemaGenerator(config);
+        SchemaGeneratorConfigBuilder configBuilder = new SchemaGeneratorConfigBuilder(new ObjectMapper(), schemaVersion);
+        configBuilder.forTypesInGeneral()
+                .withTitleResolver(_scope -> "type title")
+                .withDescriptionResolver(_scope -> "type description")
+                .withCustomDefinitionProvider(customDefinitionProvider);
+        SchemaGenerator generator = new SchemaGenerator(configBuilder.build());
         JsonNode result = generator.generateSchema(Integer.class);
-        Assert.assertEquals(2, result.size());
+        Assert.assertEquals(4, result.size());
         Assert.assertEquals(SchemaKeyword.TAG_TYPE_INTEGER.forVersion(schemaVersion),
                 result.get(SchemaKeyword.TAG_TYPE.forVersion(schemaVersion)).asText());
         Assert.assertEquals("custom override of Integer", result.get("$comment").asText());
+        Assert.assertEquals("custom title", result.get(SchemaKeyword.TAG_TITLE.forVersion(schemaVersion)).asText());
+        Assert.assertEquals("type description", result.get(SchemaKeyword.TAG_DESCRIPTION.forVersion(schemaVersion)).asText());
     }
 
     @Test
@@ -147,12 +152,12 @@ public class SchemaGeneratorCustomDefinitionsTest {
                 return new CustomDefinition(customDefinition);
             }
         };
-        SchemaGeneratorConfig config = new SchemaGeneratorConfigBuilder(new ObjectMapper(), schemaVersion, OptionPreset.PLAIN_JSON)
-                .with(customDefinitionProviderOne)
-                .with(customDefinitionProviderTwo)
-                .with(Option.DEFINITIONS_FOR_ALL_OBJECTS)
-                .build();
-        SchemaGenerator generator = new SchemaGenerator(config);
+        SchemaGeneratorConfigBuilder configBuilder = new SchemaGeneratorConfigBuilder(new ObjectMapper(), schemaVersion, OptionPreset.PLAIN_JSON)
+                .with(Option.DEFINITIONS_FOR_ALL_OBJECTS);
+        configBuilder.forTypesInGeneral()
+                .withCustomDefinitionProvider(customDefinitionProviderOne)
+                .withCustomDefinitionProvider(customDefinitionProviderTwo);
+        SchemaGenerator generator = new SchemaGenerator(configBuilder.build());
         JsonNode result = generator.generateSchema(TestDirectCircularClass.class);
         JSONAssert.assertEquals('\n' + result.toString() + '\n',
                 loadResource("multiple-definitions-one-type-" + schemaVersion.name() + ".json"), result.toString(), JSONCompareMode.STRICT);
@@ -168,20 +173,47 @@ public class SchemaGeneratorCustomDefinitionsTest {
             }
             ResolvedType generic = context.getTypeContext().getContainerItemType(javaType);
             SchemaGeneratorConfig config = context.getGeneratorConfig();
-            return new CustomDefinition(context.getGeneratorConfig().createObjectNode()
+            return new CustomDefinition(config.createObjectNode()
                     .put(config.getKeyword(SchemaKeyword.TAG_TYPE), config.getKeyword(SchemaKeyword.TAG_TYPE_OBJECT))
-                    .set(config.getKeyword(SchemaKeyword.TAG_PROPERTIES), context.getGeneratorConfig().createObjectNode()
+                    .set(config.getKeyword(SchemaKeyword.TAG_PROPERTIES), config.createObjectNode()
                             .set(accessProperty, context.createDefinitionReference(generic))));
         };
-        SchemaGeneratorConfig config = new SchemaGeneratorConfigBuilder(new ObjectMapper(), schemaVersion)
-                .with(customDefinitionProvider)
-                .build();
-        SchemaGenerator generator = new SchemaGenerator(config);
+        SchemaGeneratorConfigBuilder configBuilder = new SchemaGeneratorConfigBuilder(new ObjectMapper(), schemaVersion);
+        configBuilder.forTypesInGeneral()
+                .withCustomDefinitionProvider(customDefinitionProvider);
+        SchemaGenerator generator = new SchemaGenerator(configBuilder.build());
         JsonNode result = generator.generateSchema(TestCircularClass1.class);
         JSONAssert.assertEquals('\n' + result.toString() + '\n',
                 loadResource("circular-custom-definition-" + schemaVersion.name() + ".json"), result.toString(), JSONCompareMode.STRICT);
     }
 
+    @Test
+    @Parameters(source = SchemaVersion.class)
+    public void testGenerateSchema_CustomPropertyDefinition(SchemaVersion schemaVersion) throws Exception {
+        CustomPropertyDefinitionProvider<FieldScope> customPropertyDefinitionProvider = (field, context) -> {
+            if (field.getType().getErasedType() == int.class) {
+                return new CustomPropertyDefinition(context.createDefinition(field.getType())
+                        .put(context.getKeyword(SchemaKeyword.TAG_TITLE), "custom title"));
+            }
+            if (field.getType().getErasedType() == String.class) {
+                return new CustomPropertyDefinition(context.createDefinition(field.getType())
+                        .put(context.getKeyword(SchemaKeyword.TAG_DESCRIPTION), "custom description"));
+            }
+            // the circular reference would result in an endless loop here; need to be careful!
+            return null;
+        };
+        SchemaGeneratorConfigBuilder configBuilder = new SchemaGeneratorConfigBuilder(schemaVersion, OptionPreset.PLAIN_JSON);
+        configBuilder.forTypesInGeneral()
+                .withTitleResolver(_scope -> "type title");
+        configBuilder.forFields()
+                .withDescriptionResolver(_field -> "field description")
+                .withCustomDefinitionProvider(customPropertyDefinitionProvider);
+        SchemaGenerator generator = new SchemaGenerator(configBuilder.build());
+        JsonNode result = generator.generateSchema(TestDirectCircularClass.class);
+        JSONAssert.assertEquals('\n' + result.toString() + '\n',
+                loadResource("custom-property-definition-" + schemaVersion.name() + ".json"), result.toString(), JSONCompareMode.STRICT);
+    }
+
     private static String loadResource(String resourcePath) throws IOException {
         StringBuilder stringBuilder = new StringBuilder();
         try (InputStream inputStream = SchemaGeneratorComplexTypesTest.class
diff --git a/jsonschema-generator/src/test/resources/com/github/victools/jsonschema/generator/custom-property-definition-DRAFT_2019_09.json b/jsonschema-generator/src/test/resources/com/github/victools/jsonschema/generator/custom-property-definition-DRAFT_2019_09.json
new file mode 100644
index 0000000..efd8b2a
--- /dev/null
+++ b/jsonschema-generator/src/test/resources/com/github/victools/jsonschema/generator/custom-property-definition-DRAFT_2019_09.json
@@ -0,0 +1,21 @@
+{
+    "$schema": "https://json-schema.org/draft/2019-09/schema",
+    "type": "object",
+    "properties": {
+        "number": {
+            "type": "integer",
+            "title": "custom title",
+            "description": "field description"
+        },
+        "self": {
+            "$ref": "#",
+            "description": "field description"
+        },
+        "text": {
+            "type": "string",
+            "title": "type title",
+            "description": "custom description"
+        }
+    },
+    "title": "type title"
+}
diff --git a/jsonschema-generator/src/test/resources/com/github/victools/jsonschema/generator/custom-property-definition-DRAFT_7.json b/jsonschema-generator/src/test/resources/com/github/victools/jsonschema/generator/custom-property-definition-DRAFT_7.json
new file mode 100644
index 0000000..99ffa44
--- /dev/null
+++ b/jsonschema-generator/src/test/resources/com/github/victools/jsonschema/generator/custom-property-definition-DRAFT_7.json
@@ -0,0 +1,24 @@
+{
+    "$schema": "http://json-schema.org/draft-07/schema#",
+    "type": "object",
+    "properties": {
+        "number": {
+            "type": "integer",
+            "title": "custom title",
+            "description": "field description"
+        },
+        "self": {
+            "allOf": [{
+                    "$ref": "#"
+                }, {
+                    "description": "field description"
+                }]
+        },
+        "text": {
+            "type": "string",
+            "title": "type title",
+            "description": "custom description"
+        }
+    },
+    "title": "type title"
+}