Skip to content

Commit

Permalink
feat: Support removal of duplicate attributes from member schemas (#428)
Browse files Browse the repository at this point in the history
* chore: Replace deprecated method usage
* feat: Introduce Option for duplicate member attribute removal
  • Loading branch information
CarstenWickner authored Jan 10, 2024
1 parent 8886eeb commit 6c07ac8
Show file tree
Hide file tree
Showing 18 changed files with 246 additions and 21 deletions.
7 changes: 6 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,12 @@ 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]
*-*
### `jsonschema-generator`
#### Added
- new `Option.DUPLICATE_MEMBER_ATTRIBUTE_CLEANUP_AT_THE_END` discard duplicate elements from member sub-schemas

#### Changed
- new `Option.DUPLICATE_MEMBER_ATTRIBUTE_CLEANUP_AT_THE_END` by default included in standard `OptionPreset`s

## [4.33.1] - 2023-12-19
### `jsonschema-module-jackson`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,13 @@ public enum Option {
* @since 4.6.0
*/
ALLOF_CLEANUP_AT_THE_END(null, null),
/**
* Whether at the end of the schema generation, all member sub-schemas referencing a common definition should be checked for any duplicated
* attributes, which should be removed from the inline member sub-schemas in favor of the equivalent in the single common definition.
*
* @since 4.34.0
*/
DUPLICATE_MEMBER_ATTRIBUTE_CLEANUP_AT_THE_END(null, null),
/**
* Whether at the end of the schema generation, all sub-schemas without an explicit "type" indication should be augmented by the implied "type"
* based on the other tags in the respective schema.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ public class OptionPreset {
Option.DEFINITIONS_FOR_ALL_OBJECTS,
Option.NULLABLE_FIELDS_BY_DEFAULT,
Option.NULLABLE_METHOD_RETURN_VALUES_BY_DEFAULT,
Option.ALLOF_CLEANUP_AT_THE_END
Option.ALLOF_CLEANUP_AT_THE_END,
Option.DUPLICATE_MEMBER_ATTRIBUTE_CLEANUP_AT_THE_END
);

/**
Expand All @@ -63,7 +64,8 @@ public class OptionPreset {
Option.PUBLIC_NONSTATIC_FIELDS,
Option.NONPUBLIC_NONSTATIC_FIELDS_WITH_GETTERS,
Option.NONPUBLIC_NONSTATIC_FIELDS_WITHOUT_GETTERS,
Option.ALLOF_CLEANUP_AT_THE_END
Option.ALLOF_CLEANUP_AT_THE_END,
Option.DUPLICATE_MEMBER_ATTRIBUTE_CLEANUP_AT_THE_END
);

/**
Expand All @@ -79,7 +81,8 @@ public class OptionPreset {
Option.NONSTATIC_NONVOID_NONGETTER_METHODS,
Option.SIMPLIFIED_ENUMS,
Option.SIMPLIFIED_OPTIONALS,
Option.ALLOF_CLEANUP_AT_THE_END
Option.ALLOF_CLEANUP_AT_THE_END,
Option.DUPLICATE_MEMBER_ATTRIBUTE_CLEANUP_AT_THE_END
);

private final Set<Option> defaultEnabledOptions;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,8 @@ private ObjectNode createSchemaForSingleType(Type mainTargetType, Type... typePa
this.generationContext.addReference(mainType, jsonSchemaResult, null, false);
}
String definitionsTagName = this.config.getKeyword(SchemaKeyword.TAG_DEFINITIONS);
ObjectNode definitionsNode = this.buildDefinitionsAndResolveReferences(definitionsTagName, mainKey);
String referenceKeyPrefix = this.getReferenceKeyPrefix(definitionsTagName);
ObjectNode definitionsNode = this.buildDefinitionsAndResolveReferences(referenceKeyPrefix, mainKey);
if (!definitionsNode.isEmpty()) {
jsonSchemaResult.set(definitionsTagName, definitionsNode);
}
Expand All @@ -128,7 +129,7 @@ private ObjectNode createSchemaForSingleType(Type mainTargetType, Type... typePa
jsonSchemaResult.setAll(mainSchemaNode);
this.schemaNodes.add(jsonSchemaResult);
}
this.performCleanup();
this.performCleanup(definitionsNode, referenceKeyPrefix);
this.config.resetAfterSchemaGenerationFinished();
return jsonSchemaResult;
}
Expand Down Expand Up @@ -167,25 +168,36 @@ public ObjectNode createSchemaReference(Type targetType, Type... typeParameters)
* @see #createSchemaReference(Type, Type...)
*/
public ObjectNode collectDefinitions(String designatedDefinitionPath) {
ObjectNode definitionsNode = this.buildDefinitionsAndResolveReferences(designatedDefinitionPath, null);
this.performCleanup();
String referenceKeyPrefix = this.getReferenceKeyPrefix(designatedDefinitionPath);
ObjectNode definitionsNode = this.buildDefinitionsAndResolveReferences(referenceKeyPrefix, null);
this.performCleanup(definitionsNode, referenceKeyPrefix);
return definitionsNode;
}

private String getReferenceKeyPrefix(String designatedDefinitionPath) {
return this.config.getKeyword(SchemaKeyword.TAG_REF_MAIN) + '/' + designatedDefinitionPath + '/';
}

/**
* Reduce unnecessary structures in the generated schema definitions. Assumption being that this method is being invoked as the very last action
* of the schema generation.
*
* @param definitionsNode object node containing common schema definitions
* @param referenceKeyPrefix designated prefix to the entries in the returned definitions node (i.e., on {@link SchemaKeyword#TAG_REF} values)
*
* @see SchemaGeneratorConfig#shouldCleanupUnnecessaryAllOfElements()
* @see SchemaCleanUpUtils#reduceAllOfNodes(List)
* @see SchemaCleanUpUtils#reduceAnyOfNodes(List)
*/
private void performCleanup() {
private void performCleanup(ObjectNode definitionsNode, String referenceKeyPrefix) {
SchemaCleanUpUtils cleanUpUtils = new SchemaCleanUpUtils(this.config);
if (this.config.shouldCleanupUnnecessaryAllOfElements()) {
cleanUpUtils.reduceAllOfNodes(this.schemaNodes);
}
cleanUpUtils.reduceAnyOfNodes(this.schemaNodes);
if (this.config.shouldDiscardDuplicateMemberAttributes()) {
cleanUpUtils.reduceRedundantMemberAttributes(this.schemaNodes, definitionsNode, referenceKeyPrefix);
}
if (this.config.shouldIncludeStrictTypeInfo()) {
cleanUpUtils.setStrictTypeInfo(this.schemaNodes, true);
}
Expand All @@ -195,12 +207,11 @@ private void performCleanup() {
* Finalisation Step: collect the entries for the generated schema's "definitions" and ensure that all references are either pointing to the
* appropriate definition or contain the respective (sub) schema directly inline.
*
* @param designatedDefinitionPath designated path to the returned definitions node (to be incorporated in {@link SchemaKeyword#TAG_REF} values)
* @param referenceKeyPrefix designated prefix to the entries in the returned definitions node (i.e., on {@link SchemaKeyword#TAG_REF} values)
* @param mainSchemaKey definition key identifying the main type for which createSchemaReference() was invoked
* @return node representing the main schema's "definitions" (may be empty)
*/
private ObjectNode buildDefinitionsAndResolveReferences(String designatedDefinitionPath, DefinitionKey mainSchemaKey) {
final String referenceKeyPrefix = this.config.getKeyword(SchemaKeyword.TAG_REF_MAIN) + '/' + designatedDefinitionPath + '/';
private ObjectNode buildDefinitionsAndResolveReferences(String referenceKeyPrefix, DefinitionKey mainSchemaKey) {
final ObjectNode definitionsNode = this.config.createObjectNode();

final AtomicBoolean considerOnlyDirectReferences = new AtomicBoolean(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,16 @@ public interface SchemaGeneratorConfig extends StatefulConfig {
*/
boolean shouldCleanupUnnecessaryAllOfElements();

/**
* Determine whether duplicate elements should be removed from member/property sub-schemas if equal elements are contained in the referenced
* common definition.
*
* @return whether to discard duplicate elements from property schemas during the last schema generation step
*
* @since 4.34.0
*/
boolean shouldDiscardDuplicateMemberAttributes();

/**
* Determine whether sub schemas should get the {@link SchemaKeyword#TAG_TYPE} added implicitly based on other contained tags, if it is missing.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import com.github.victools.jsonschema.generator.SchemaVersion;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
Expand Down Expand Up @@ -127,6 +128,30 @@ public void reduceAnyOfNodes(List<ObjectNode> jsonSchemas) {
this.finaliseSchemaParts(jsonSchemas, nodeToCheck -> this.reduceAnyOfWrappersIfPossible(nodeToCheck, anyOfTagName));
}

/**
* Discard attributes on member schemas, that also reference an entry in the common definitions, which contains those exact same attributes.
*
* @param jsonSchemas generated schemas that may contain redundant attributes in nested object member schemas
* @param definitionsNode object node containing common schema definitions
* @param referenceKeyPrefix designated prefix to the entries in the returned definitions node (i.e., on {@link SchemaKeyword#TAG_REF} values)
*/
public void reduceRedundantMemberAttributes(List<ObjectNode> jsonSchemas, ObjectNode definitionsNode, String referenceKeyPrefix) {
final Map<String, Map<String, JsonNode>> definitions = new HashMap<>();
for (Iterator<Map.Entry<String, JsonNode>> defIt = definitionsNode.fields(); defIt.hasNext(); ) {
Map.Entry<String, JsonNode> definition = defIt.next();
Map<String, JsonNode> attributes = new HashMap<>();
for (Iterator<Map.Entry<String, JsonNode>> attIt = definition.getValue().fields(); attIt.hasNext(); ) {
Map.Entry<String, JsonNode> attriibute = attIt.next();
attributes.put(attriibute.getKey(), attriibute.getValue());
}
definitions.put(referenceKeyPrefix + definition.getKey(), attributes);
}
final String propertiesKeyword = this.config.getKeyword(SchemaKeyword.TAG_PROPERTIES);
final String refKeyWord = this.config.getKeyword(SchemaKeyword.TAG_REF);
this.finaliseSchemaParts(jsonSchemas,
nodeToCheck -> this.reduceRedundantMemberAttributesIfPossible(nodeToCheck, propertiesKeyword, refKeyWord, definitions));
}

/**
* Go through all sub-schemas and look for those without a {@link SchemaKeyword#TAG_TYPE} indication. Then try to derive the appropriate type
* indication from the other present tags (e.g., "properties" implies it is an "object").
Expand Down Expand Up @@ -553,6 +578,62 @@ private void reduceAnyOfWrappersIfPossible(JsonNode schemaNode, String anyOfTagN
}
}

/**
* Discard attributes on member schemas, that also reference an entry in the common definitions, which contains those exact same attributes.
*
* @param schemaNode single schema to check for properties, which in turn contain redundant attributes to be removed
* @param propertiesKeyword keyword under which to find an object schema's properties
* @param refKeyword keyword containing the reference to a common schema definition
* @param definitions object node containing common schema definitions
*/
private void reduceRedundantMemberAttributesIfPossible(ObjectNode schemaNode,
String propertiesKeyword, String refKeyword, Map<String, Map<String, JsonNode>> definitions) {
JsonNode propertiesNode = schemaNode.get(propertiesKeyword);
if (propertiesNode == null || !propertiesNode.isObject()) {
return;
}
for (Iterator<JsonNode> it = propertiesNode.elements(); it.hasNext(); ) {
JsonNode memberSchema = it.next();
JsonNode reference = memberSchema.get(refKeyword);
if (reference == null || !(memberSchema instanceof ObjectNode)) {
// only considering a property/member schema containing a direct reference to a common definition
continue;
}
if (definitions.containsKey(reference.asText())) {
this.reduceRedundantAttributesIfPossible((ObjectNode) memberSchema, definitions.get(reference.asText()));
}
}
}

/**
* Discard attributes on the given member schema, if those same attributes are already contained in the common definition being referenced.
*
* @param memberSchema single schema to discard redundant attributes from
* @param referencedDefinition attribute names and values in the common definition, which don't need to be repeated
*/
private void reduceRedundantAttributesIfPossible(ObjectNode memberSchema, Map<String, JsonNode> referencedDefinition) {
Set<String> skippedKeywords = new HashSet<>();
String ifKeyword = this.config.getKeyword(SchemaKeyword.TAG_IF);
String thenKeyword = this.config.getKeyword(SchemaKeyword.TAG_THEN);
String elseKeyword = this.config.getKeyword(SchemaKeyword.TAG_ELSE);
boolean shouldSkipConditionals = !Util.nullSafeEquals(memberSchema.get(ifKeyword), referencedDefinition.get(ifKeyword))
|| !Util.nullSafeEquals(memberSchema.get(thenKeyword), referencedDefinition.get(thenKeyword))
|| !Util.nullSafeEquals(memberSchema.get(elseKeyword), referencedDefinition.get(elseKeyword));
if (shouldSkipConditionals) {
skippedKeywords.add(ifKeyword);
skippedKeywords.add(thenKeyword);
skippedKeywords.add(elseKeyword);
}
for (Iterator<Map.Entry<String, JsonNode>> it = memberSchema.fields(); it.hasNext(); ) {
Map.Entry<String, JsonNode> memberAttribute = it.next();
String keyword = memberAttribute.getKey();
if (!skippedKeywords.contains(keyword) && memberAttribute.getValue().equals(referencedDefinition.get(keyword))) {
// remove member attribute, that also exists on the referenced definition
it.remove();
}
}
}

/**
* Add the {@link SchemaKeyword#TAG_TYPE} where it is missing and it can be implied from other present tags.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,11 @@ public boolean shouldCleanupUnnecessaryAllOfElements() {
return this.isOptionEnabled(Option.ALLOF_CLEANUP_AT_THE_END);
}

@Override
public boolean shouldDiscardDuplicateMemberAttributes() {
return this.isOptionEnabled(Option.DUPLICATE_MEMBER_ATTRIBUTE_CLEANUP_AT_THE_END);
}

@Override
public boolean shouldIncludeStrictTypeInfo() {
return this.isOptionEnabled(Option.STRICT_TYPE_INFO);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,22 @@ public static <T> List<T> nullSafe(List<T> list) {
}
return list;
}

/**
* Ensure the two given values are either both {@code null} or equal to each other.
*
* @param one first value to check
* @param other second value to check
* @return whether the two given values are equal
*/
public static boolean nullSafeEquals(Object one, Object other) {
if (one == null) {
return other == null;
}
if (other == null) {
return false;
}
return one.hashCode() == other.hashCode()
&& one.equals(other);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ protected AbstractTypeAwareTest(Class<?> testClass) {
* @param schemaVersion designated JSON Schema version
*/
protected void prepareContextForVersion(SchemaVersion schemaVersion) {
TypeContext typeContext = Mockito.spy(TypeContextFactory.createDefaultTypeContext());
TypeContext typeContext = Mockito.spy(TypeContextFactory.createDefaultTypeContext(Mockito.mock(SchemaGeneratorConfig.class)));
ResolvedType resolvedTestClass = typeContext.resolve(this.testClass);
this.declarationDetails = new MemberScope.DeclarationDetails(resolvedTestClass, typeContext.resolveWithMembers(resolvedTestClass));
this.context = Mockito.mock(SchemaGenerationContext.class, Mockito.RETURNS_DEEP_STUBS);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/*
* Copyright 2024 VicTools.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.github.victools.jsonschema.generator;

import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.stream.Stream;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

/**
* Test for {@link SchemaGenerator} class.
*/
public class SchemaGeneratorMemberCleanUpTest {

private static Stream<Arguments> testMemberCleanup() {
return Stream.of(
Arguments.of(true, Arrays.asList("$ref")),
Arguments.of(false, Arrays.asList("$ref", "additionalProperties"))
);
}

@ParameterizedTest
@MethodSource
public void testMemberCleanup(boolean enableCleanup, List<String> expectedMemberAttributes) {
SchemaGeneratorConfigBuilder configBuilder = new SchemaGeneratorConfigBuilder(SchemaVersion.DRAFT_2019_09, OptionPreset.PLAIN_JSON)
.with(Option.MAP_VALUES_AS_ADDITIONAL_PROPERTIES, Option.DEFINITIONS_FOR_ALL_OBJECTS);
if (enableCleanup) {
configBuilder.with(Option.DUPLICATE_MEMBER_ATTRIBUTE_CLEANUP_AT_THE_END);
} else {
configBuilder.without(Option.DUPLICATE_MEMBER_ATTRIBUTE_CLEANUP_AT_THE_END);
}
SchemaGeneratorConfig generatorConfig = configBuilder.build();
ObjectNode schema = new SchemaGenerator(generatorConfig).generateSchema(TestClass.class);
JsonNode memberSchema = schema.get(generatorConfig.getKeyword(SchemaKeyword.TAG_PROPERTIES)).get("mapValue");
Assertions.assertEquals(expectedMemberAttributes.size(), memberSchema.size());
memberSchema.fieldNames()
.forEachRemaining(fieldName -> Assertions.assertTrue(expectedMemberAttributes.contains(fieldName)));
}

private static class TestClass {
@JsonProperty("map_value")
public Map<String, ValueClass> mapValue;
}

private static class ValueClass {
@JsonProperty("int_value")
public Integer intValue;
@JsonProperty("string_value")
public String stringValue;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.EnumSource;
import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.Mockito;

/**
* Test for the {@link AttributeCollector} class.
Expand Down Expand Up @@ -69,7 +70,8 @@ private SchemaGenerationContextImpl generateContext(SchemaVersion schemaVersion,
new SchemaGeneratorConfigPart<>(),
new SchemaGeneratorConfigPart<>(),
Collections.emptyMap());
return new SchemaGenerationContextImpl(generatorConfig, TypeContextFactory.createDefaultTypeContext());
return new SchemaGenerationContextImpl(generatorConfig,
TypeContextFactory.createDefaultTypeContext(Mockito.mock(SchemaGeneratorConfig.class)));
}

@ParameterizedTest
Expand Down
Loading

0 comments on commit 6c07ac8

Please sign in to comment.