Repository: victools/jsonschema-generator
Pull request #34 (2020-02-28 22:23:09)

Improved object handling
The `Object.class` is currently represented by the following schema by default:
```javascript
{
    "type": "object"
}
```
However, that means any kind of element (e.g. "string", "number", "array") would fail the validation against this schema, even though from a Java perspective everything is an `Object`. Therefore changing this here now to simply omit the `"type"` for an otherwise unspecified `Object`.

----

Additionally, some collected attributes may not be valid in combination with the declared `"type"` in a sub-schema.
For now, only in the `SchemaGeneratorConfigBuilder.forTypesInGeneral()` the collection of some attributes is being skipped, if the existing `"type"` makes them invalid:
- `"string"` attributes are only collected if that `"type"` is declared (or no `"type"` at all)
- `"number"`/`"integer"` attributes are only collected if that `"type"` is declared (or no `"type"` at all)
- `"array"` attributes are only collected if that `"type"` is declared (or no `"type"` at all)
- `"object"` attributes are only collected if that `"type"` is declared (or no `"type"` at all)
This also affects the recently added `Option.FORBIDDEN_ADDITIONAL_PROPERTIES_BY_DEFAULT` which was adding the `"additionalProperties"` in too many places when non-`object` custom definitions were involved.


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index d9d8c9c..e91ce1e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+### Changed
+- limit collected type attributes by declared "type"
+
+### Fixed
+- not declare any "type" for `Object.class` by default
 
 ## [4.2.0] - 2020-02-27
 ### Added
diff --git a/src/main/java/com/github/victools/jsonschema/generator/impl/AttributeCollector.java b/src/main/java/com/github/victools/jsonschema/generator/impl/AttributeCollector.java
index c9d6453..f92e180 100644
--- a/src/main/java/com/github/victools/jsonschema/generator/impl/AttributeCollector.java
+++ b/src/main/java/com/github/victools/jsonschema/generator/impl/AttributeCollector.java
@@ -32,6 +32,7 @@ import java.math.BigInteger;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.stream.Collectors;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -127,9 +128,11 @@ public class AttributeCollector {
      *
      * @param scope the scope/type representation for which to collect JSON schema attributes
      * @param generationContext generation context, including configuration to apply when looking-up attribute values
+     * @param allowedSchemaTypes declared schema types determining which attributes are meaningful to be included
      * @return node holding all collected attributes (possibly empty)
      */
-    public static ObjectNode collectTypeAttributes(TypeScope scope, SchemaGenerationContextImpl generationContext) {
+    public static ObjectNode collectTypeAttributes(TypeScope scope, SchemaGenerationContextImpl generationContext,
+            Set<String> allowedSchemaTypes) {
         SchemaGeneratorConfig config = generationContext.getGeneratorConfig();
         ObjectNode node = config.createObjectNode();
         AttributeCollector collector = new AttributeCollector(config.getObjectMapper());
@@ -137,20 +140,29 @@ public class AttributeCollector {
         collector.setDescription(node, config.resolveDescriptionForType(scope));
         collector.setDefault(node, config.resolveDefaultForType(scope));
         collector.setEnum(node, config.resolveEnumForType(scope));
-        collector.setAdditionalProperties(node, config.resolveAdditionalPropertiesForType(scope), generationContext);
-        collector.setPatternProperties(node, config.resolvePatternPropertiesForType(scope), generationContext);
-        collector.setStringMinLength(node, config.resolveStringMinLengthForType(scope));
-        collector.setStringMaxLength(node, config.resolveStringMaxLengthForType(scope));
-        collector.setStringFormat(node, config.resolveStringFormatForType(scope));
-        collector.setStringPattern(node, config.resolveStringPatternForType(scope));
-        collector.setNumberInclusiveMinimum(node, config.resolveNumberInclusiveMinimumForType(scope));
-        collector.setNumberExclusiveMinimum(node, config.resolveNumberExclusiveMinimumForType(scope));
-        collector.setNumberInclusiveMaximum(node, config.resolveNumberInclusiveMaximumForType(scope));
-        collector.setNumberExclusiveMaximum(node, config.resolveNumberExclusiveMaximumForType(scope));
-        collector.setNumberMultipleOf(node, config.resolveNumberMultipleOfForType(scope));
-        collector.setArrayMinItems(node, config.resolveArrayMinItemsForType(scope));
-        collector.setArrayMaxItems(node, config.resolveArrayMaxItemsForType(scope));
-        collector.setArrayUniqueItems(node, config.resolveArrayUniqueItemsForType(scope));
+        if (allowedSchemaTypes.isEmpty() || allowedSchemaTypes.contains(SchemaConstants.TAG_TYPE_OBJECT)) {
+            collector.setAdditionalProperties(node, config.resolveAdditionalPropertiesForType(scope), generationContext);
+            collector.setPatternProperties(node, config.resolvePatternPropertiesForType(scope), generationContext);
+        }
+        if (allowedSchemaTypes.isEmpty() || allowedSchemaTypes.contains(SchemaConstants.TAG_TYPE_STRING)) {
+            collector.setStringMinLength(node, config.resolveStringMinLengthForType(scope));
+            collector.setStringMaxLength(node, config.resolveStringMaxLengthForType(scope));
+            collector.setStringFormat(node, config.resolveStringFormatForType(scope));
+            collector.setStringPattern(node, config.resolveStringPatternForType(scope));
+        }
+        if (allowedSchemaTypes.isEmpty() || allowedSchemaTypes.contains(SchemaConstants.TAG_TYPE_INTEGER)
+                || allowedSchemaTypes.contains(SchemaConstants.TAG_TYPE_NUMBER)) {
+            collector.setNumberInclusiveMinimum(node, config.resolveNumberInclusiveMinimumForType(scope));
+            collector.setNumberExclusiveMinimum(node, config.resolveNumberExclusiveMinimumForType(scope));
+            collector.setNumberInclusiveMaximum(node, config.resolveNumberInclusiveMaximumForType(scope));
+            collector.setNumberExclusiveMaximum(node, config.resolveNumberExclusiveMaximumForType(scope));
+            collector.setNumberMultipleOf(node, config.resolveNumberMultipleOfForType(scope));
+        }
+        if (allowedSchemaTypes.isEmpty() || allowedSchemaTypes.contains(SchemaConstants.TAG_TYPE_ARRAY)) {
+            collector.setArrayMinItems(node, config.resolveArrayMinItemsForType(scope));
+            collector.setArrayMaxItems(node, config.resolveArrayMaxItemsForType(scope));
+            collector.setArrayUniqueItems(node, config.resolveArrayUniqueItemsForType(scope));
+        }
         return node;
     }
 
@@ -318,9 +330,7 @@ public class AttributeCollector {
             for (Map.Entry<String, Type> entry : patternProperties.entrySet()) {
                 ObjectNode singlePatternSchema = this.objectMapper.createObjectNode();
                 ResolvedType targetType = generationContext.getTypeContext().resolve(entry.getValue());
-                if (targetType.getErasedType() != Object.class) {
-                    generationContext.traverseGenericType(targetType, singlePatternSchema, false);
-                }
+                generationContext.traverseGenericType(targetType, singlePatternSchema, false);
                 patternPropertiesNode.set(entry.getKey(), singlePatternSchema);
             }
             node.set(SchemaConstants.TAG_PATTERN_PROPERTIES, patternPropertiesNode);
diff --git a/src/main/java/com/github/victools/jsonschema/generator/impl/SchemaGenerationContextImpl.java b/src/main/java/com/github/victools/jsonschema/generator/impl/SchemaGenerationContextImpl.java
index cbfcda4..87c5dc0 100644
--- a/src/main/java/com/github/victools/jsonschema/generator/impl/SchemaGenerationContextImpl.java
+++ b/src/main/java/com/github/victools/jsonschema/generator/impl/SchemaGenerationContextImpl.java
@@ -44,7 +44,9 @@ import java.util.Map;
 import java.util.Set;
 import java.util.TreeMap;
 import java.util.function.Function;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
+import java.util.stream.StreamSupport;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -262,7 +264,8 @@ public class SchemaGenerationContextImpl implements SchemaGenerationContext {
             }
         }
         TypeScope scope = this.typeContext.createTypeScope(targetType);
-        ObjectNode typeAttributes = AttributeCollector.collectTypeAttributes(scope, this);
+        Set<String> allowedSchemaTypes = this.collectAllowedSchemaTypes(definition);
+        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(definition);
         // apply merged attributes
@@ -272,6 +275,27 @@ public class SchemaGenerationContextImpl implements SchemaGenerationContext {
                 .forEach(override -> override.overrideTypeAttributes(definition, scope, this.generatorConfig));
     }
 
+    /**
+     * Collect the specified value(s) from the given definition's "{@value SchemaConstants#TAG_TYPE}" attribute.
+     *
+     * @param definition type definition to extract specified "{@value SchemaConstants#TAG_TYPE}" values from
+     * @return extracted "{@value SchemaConstants#TAG_TYPE}" values (may be empty)
+     */
+    private Set<String> collectAllowedSchemaTypes(ObjectNode definition) {
+        JsonNode declaredTypes = definition.get(SchemaConstants.TAG_TYPE);
+        final Set<String> allowedSchemaTypes;
+        if (declaredTypes == null) {
+            allowedSchemaTypes = Collections.emptySet();
+        } else if (declaredTypes.isTextual()) {
+            allowedSchemaTypes = Collections.singleton(declaredTypes.textValue());
+        } else {
+            allowedSchemaTypes = StreamSupport.stream(declaredTypes.spliterator(), false)
+                    .map(JsonNode::textValue)
+                    .collect(Collectors.toSet());
+        }
+        return allowedSchemaTypes;
+    }
+
     /**
      * Preparation Step: add the given targetType (which was previously determined to be an array type).
      *
diff --git a/src/main/java/com/github/victools/jsonschema/generator/impl/module/EnumModule.java b/src/main/java/com/github/victools/jsonschema/generator/impl/module/EnumModule.java
index 7b7e08f..605fa47 100644
--- a/src/main/java/com/github/victools/jsonschema/generator/impl/module/EnumModule.java
+++ b/src/main/java/com/github/victools/jsonschema/generator/impl/module/EnumModule.java
@@ -20,7 +20,6 @@ import com.fasterxml.classmate.ResolvedType;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.github.victools.jsonschema.generator.CustomDefinition;
-import com.github.victools.jsonschema.generator.CustomDefinitionProvider;
 import com.github.victools.jsonschema.generator.CustomDefinitionProviderV2;
 import com.github.victools.jsonschema.generator.MethodScope;
 import com.github.victools.jsonschema.generator.Module;
@@ -128,7 +127,7 @@ public class EnumModule implements Module {
     }
 
     /**
-     * Implementation of the {@link CustomDefinitionProvider} interface for treating enum types as plain strings.
+     * Implementation of the {@link CustomDefinitionProviderV2} interface for treating enum types as plain strings.
      */
     private static class EnumAsStringDefinitionProvider implements CustomDefinitionProviderV2 {
 
diff --git a/src/main/java/com/github/victools/jsonschema/generator/impl/module/SimpleTypeModule.java b/src/main/java/com/github/victools/jsonschema/generator/impl/module/SimpleTypeModule.java
index 043e158..ada7c00 100644
--- a/src/main/java/com/github/victools/jsonschema/generator/impl/module/SimpleTypeModule.java
+++ b/src/main/java/com/github/victools/jsonschema/generator/impl/module/SimpleTypeModule.java
@@ -61,7 +61,7 @@ public class SimpleTypeModule implements Module {
     public static SimpleTypeModule forPrimitiveTypes() {
         SimpleTypeModule module = new SimpleTypeModule();
 
-        module.withObjectType(Object.class);
+        module.withEmptySchema(Object.class);
 
         Stream.of(String.class, Character.class, char.class, CharSequence.class)
                 .forEach(module::withStringType);
@@ -115,12 +115,24 @@ public class SimpleTypeModule implements Module {
         return this;
     }
 
+    /**
+     * Add the given mapping for a (simple) java class that should be represented by an empty schema.
+     *
+     * @param javaType java class to map to a fixed JSON schema definition
+     * @return this module instance (for chaining)
+     */
+    public final SimpleTypeModule withEmptySchema(Class<?> javaType) {
+        return this.with(javaType, "");
+    }
+
     /**
      * Add the given mapping for a (simple) java class to its JSON schema equivalent "type" attribute: "{@value SchemaConstants#TAG_TYPE_OBJECT}".
      *
      * @param javaType java class to map to a fixed JSON schema definition
      * @return this module instance (for chaining)
+     * @deprecated rather use {@link SimpleTypeModule#withEmptySchema(Class)} instead to really allow any value
      */
+    @Deprecated
     public final SimpleTypeModule withObjectType(Class<?> javaType) {
         return this.with(javaType, SchemaConstants.TAG_TYPE_OBJECT);
     }
@@ -196,7 +208,7 @@ public class SimpleTypeModule implements Module {
      * @return either Object.class to cause omission of the "additonalProperties" keyword or null to leave it up to following configurations
      */
     private Type resolveAdditionalProperties(TypeScope scope) {
-        if (scope.getType().getTypeParameters().isEmpty() && this.fixedJsonSchemaTypes.containsKey(scope.getType().getErasedType())) {
+        if (scope.getType().getTypeParameters().isEmpty() && "".equals(this.fixedJsonSchemaTypes.get(scope.getType().getErasedType()))) {
             // indicate no specific additionalProperties type - thereby causing it to be omitted from the generated schema
             return Object.class;
         }
@@ -210,7 +222,7 @@ public class SimpleTypeModule implements Module {
      * @return either an empty map to cause omission of the "patternProperties" keyword or null to leave it up to following configurations
      */
     private Map<String, Type> resolvePatternProperties(TypeScope scope) {
-        if (scope.getType().getTypeParameters().isEmpty() && this.fixedJsonSchemaTypes.containsKey(scope.getType().getErasedType())) {
+        if (scope.getType().getTypeParameters().isEmpty() && "".equals(this.fixedJsonSchemaTypes.get(scope.getType().getErasedType()))) {
             // indicate no specific patternProperties - thereby causing it to be omitted from the generated schema
             return Collections.emptyMap();
         }
@@ -243,8 +255,10 @@ public class SimpleTypeModule implements Module {
                 return null;
             }
             // create fixed JSON schema definition, containing only the corresponding "type" attribute
-            ObjectNode customSchema = this.objectMapper.createObjectNode()
-                    .put(SchemaConstants.TAG_TYPE, jsonSchemaTypeValue);
+            ObjectNode customSchema = this.objectMapper.createObjectNode();
+            if (!jsonSchemaTypeValue.isEmpty()) {
+                customSchema.put(SchemaConstants.TAG_TYPE, jsonSchemaTypeValue);
+            }
             // set true as second parameter to indicate simple types to be always in-lined (i.e. not put into definitions)
             return new CustomDefinition(customSchema, true);
         }


Test patch:
diff --git a/src/test/java/com/github/victools/jsonschema/generator/SchemaGeneratorTest.java b/src/test/java/com/github/victools/jsonschema/generator/SchemaGeneratorTest.java
index 9ebbae4..1700f05 100644
--- a/src/test/java/com/github/victools/jsonschema/generator/SchemaGeneratorTest.java
+++ b/src/test/java/com/github/victools/jsonschema/generator/SchemaGeneratorTest.java
@@ -53,7 +53,7 @@ public class SchemaGeneratorTest {
 
     Object parametersForTestGenerateSchema_SimpleType() {
         return new Object[][]{
-            {Object.class, SchemaConstants.TAG_TYPE_OBJECT},
+            {Object.class, null},
             {String.class, SchemaConstants.TAG_TYPE_STRING},
             {Character.class, SchemaConstants.TAG_TYPE_STRING},
             {char.class, SchemaConstants.TAG_TYPE_STRING},
@@ -81,8 +81,12 @@ public class SchemaGeneratorTest {
         SchemaGeneratorConfig config = new SchemaGeneratorConfigBuilder(new ObjectMapper()).build();
         SchemaGenerator generator = new SchemaGenerator(config);
         JsonNode result = generator.generateSchema(targetType);
-        Assert.assertEquals(1, result.size());
-        Assert.assertEquals(expectedJsonSchemaType, result.get(SchemaConstants.TAG_TYPE).asText());
+        if (expectedJsonSchemaType == null) {
+            Assert.assertTrue(result.isEmpty());
+        } else {
+            Assert.assertEquals(1, result.size());
+            Assert.assertEquals(expectedJsonSchemaType, result.get(SchemaConstants.TAG_TYPE).asText());
+        }
     }
 
     @Test
@@ -93,8 +97,12 @@ public class SchemaGeneratorTest {
                 .build();
         SchemaGenerator generator = new SchemaGenerator(config);
         JsonNode result = generator.generateSchema(targetType);
-        Assert.assertEquals(1, result.size());
-        Assert.assertEquals(expectedJsonSchemaType, result.get(SchemaConstants.TAG_TYPE).asText());
+        if (expectedJsonSchemaType == null) {
+            Assert.assertTrue(result.isEmpty());
+        } else {
+            Assert.assertEquals(1, result.size());
+            Assert.assertEquals(expectedJsonSchemaType, result.get(SchemaConstants.TAG_TYPE).asText());
+        }
     }
 
     @Test
diff --git a/src/test/java/com/github/victools/jsonschema/generator/impl/AttributeCollectorTest.java b/src/test/java/com/github/victools/jsonschema/generator/impl/AttributeCollectorTest.java
index 3a5ddc7..6029db6 100644
--- a/src/test/java/com/github/victools/jsonschema/generator/impl/AttributeCollectorTest.java
+++ b/src/test/java/com/github/victools/jsonschema/generator/impl/AttributeCollectorTest.java
@@ -128,7 +128,7 @@ public class AttributeCollectorTest {
         Assert.assertTrue(patternPropertiesNode.isObject());
         Assert.assertEquals(1, patternPropertiesNode.size());
         Assert.assertTrue(patternPropertiesNode.get("^objectClass.*$").isObject());
-        Assert.assertFalse(this.generationContext.containsDefinition(this.generationContext.getTypeContext().resolve(Object.class)));
+        Assert.assertTrue(this.generationContext.containsDefinition(this.generationContext.getTypeContext().resolve(Object.class)));
     }
 
     @Test
@@ -142,7 +142,7 @@ public class AttributeCollectorTest {
         Assert.assertTrue(patternPropertiesNode.isObject());
         Assert.assertEquals(1, patternPropertiesNode.size());
         Assert.assertTrue(patternPropertiesNode.get("^resolvedObjectClass.*$").isObject());
-        Assert.assertFalse(this.generationContext.containsDefinition(this.generationContext.getTypeContext().resolve(Object.class)));
+        Assert.assertTrue(this.generationContext.containsDefinition(this.generationContext.getTypeContext().resolve(Object.class)));
     }
 
     @Test
diff --git a/src/test/resources/com/github/victools/jsonschema/generator/testclass2-array.json b/src/test/resources/com/github/victools/jsonschema/generator/testclass2-array.json
index 9e903e8..533f6fc 100644
--- a/src/test/resources/com/github/victools/jsonschema/generator/testclass2-array.json
+++ b/src/test/resources/com/github/victools/jsonschema/generator/testclass2-array.json
@@ -5,16 +5,10 @@
             "properties": {
                 "genericArray": {
                     "type": ["array", "null"],
-                    "items": {
-                        "type": "object"
-                    }
+                    "items": {}
                 },
-                "genericValue": {
-                    "type": ["object", "null"]
-                },
-                "getGenericValue()": {
-                    "type": ["object", "null"]
-                }
+                "genericValue": {},
+                "getGenericValue()": {}
             }
         }
     },