From 581a4aca917331c2ad7ea51c40c4452963f2eb4d Mon Sep 17 00:00:00 2001 From: Carsten Wickner <11309681+CarstenWickner@users.noreply.github.com> Date: Sun, 19 Nov 2023 00:45:25 +0100 Subject: [PATCH] chore: refactor for improved code health (#413) --- .../jsonschema/generator/SchemaBuilder.java | 234 +++++++++++------- 1 file changed, 142 insertions(+), 92 deletions(-) diff --git a/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/SchemaBuilder.java b/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/SchemaBuilder.java index c4d568c9..e51a2f87 100644 --- a/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/SchemaBuilder.java +++ b/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/SchemaBuilder.java @@ -30,11 +30,11 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.TreeMap; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Function; import java.util.function.Predicate; -import java.util.function.Supplier; import java.util.stream.Collectors; /** @@ -88,10 +88,8 @@ static SchemaBuilder forMultipleTypes(SchemaGeneratorConfig config, TypeContext this.generationContext = new SchemaGenerationContextImpl(this.config, this.typeContext); this.schemaNodes = new ArrayList<>(); - SchemaDefinitionNamingStrategy baseNamingStrategy = config.getDefinitionNamingStrategy(); - if (baseNamingStrategy == null) { - baseNamingStrategy = new DefaultSchemaDefinitionNamingStrategy(); - } + SchemaDefinitionNamingStrategy baseNamingStrategy = Optional.ofNullable(config.getDefinitionNamingStrategy()) + .orElseGet(DefaultSchemaDefinitionNamingStrategy::new); SchemaCleanUpUtils cleanupUtils = new SchemaCleanUpUtils(config); Function definitionCleanUpTask = config.shouldUsePlainDefinitionKeys() ? cleanupUtils::ensureDefinitionKeyIsPlain @@ -121,8 +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, this.generationContext); - if (definitionsNode.size() > 0) { + ObjectNode definitionsNode = this.buildDefinitionsAndResolveReferences(definitionsTagName, mainKey); + if (!definitionsNode.isEmpty()) { jsonSchemaResult.set(definitionsTagName, definitionsNode); } if (!createDefinitionForMainSchema) { @@ -169,7 +167,7 @@ public ObjectNode createSchemaReference(Type targetType, Type... typeParameters) * @see #createSchemaReference(Type, Type...) */ public ObjectNode collectDefinitions(String designatedDefinitionPath) { - ObjectNode definitionsNode = this.buildDefinitionsAndResolveReferences(designatedDefinitionPath, null, this.generationContext); + ObjectNode definitionsNode = this.buildDefinitionsAndResolveReferences(designatedDefinitionPath, null); this.performCleanup(); return definitionsNode; } @@ -199,61 +197,76 @@ private void performCleanup() { * * @param designatedDefinitionPath designated path to the returned definitions node (to be incorporated in {@link SchemaKeyword#TAG_REF} values) * @param mainSchemaKey definition key identifying the main type for which createSchemaReference() was invoked - * @param generationContext context containing all definitions of (sub) schemas and the list of references to them * @return node representing the main schema's "definitions" (may be empty) */ - private ObjectNode buildDefinitionsAndResolveReferences(String designatedDefinitionPath, DefinitionKey mainSchemaKey, - SchemaGenerationContextImpl generationContext) { + private ObjectNode buildDefinitionsAndResolveReferences(String designatedDefinitionPath, DefinitionKey mainSchemaKey) { + final String referenceKeyPrefix = this.config.getKeyword(SchemaKeyword.TAG_REF_MAIN) + '/' + designatedDefinitionPath + '/'; final ObjectNode definitionsNode = this.config.createObjectNode(); final AtomicBoolean considerOnlyDirectReferences = new AtomicBoolean(false); Predicate shouldProduceDefinition = this.getShouldProduceDefinitionCheck(mainSchemaKey, considerOnlyDirectReferences); - Map baseReferenceKeys = this.getReferenceKeys(mainSchemaKey, shouldProduceDefinition, generationContext); + DefinitionCollectionDetails definitionCollectionDetails = new DefinitionCollectionDetails(mainSchemaKey, referenceKeyPrefix, + shouldProduceDefinition, definitionsNode); + + Map baseReferenceKeys = this.getReferenceKeys(mainSchemaKey, shouldProduceDefinition); considerOnlyDirectReferences.set(true); + for (Map.Entry baseReferenceKey : baseReferenceKeys.entrySet()) { - String definitionName = baseReferenceKey.getValue(); DefinitionKey definitionKey = baseReferenceKey.getKey(); - List references = generationContext.getReferences(definitionKey); - List nullableReferences = generationContext.getNullableReferences(definitionKey); - final String referenceKey; - boolean referenceInline = !shouldProduceDefinition.test(definitionKey); - if (referenceInline) { - // it is a simple type, just in-line the sub-schema everywhere - ObjectNode definition = generationContext.getDefinition(definitionKey); - references.forEach(node -> AttributeCollector.mergeMissingAttributes(node, definition)); - referenceKey = null; - } else { - // the same sub-schema is referenced in multiple places - Supplier addDefinitionAndReturnReferenceKey = () -> { - definitionsNode.set(definitionName, this.generationContext.getDefinition(definitionKey)); - return this.config.getKeyword(SchemaKeyword.TAG_REF_MAIN) + '/' + designatedDefinitionPath + '/' + definitionName; - }; - referenceKey = getReferenceKey(mainSchemaKey, definitionKey, addDefinitionAndReturnReferenceKey); - references.forEach(node -> node.put(this.config.getKeyword(SchemaKeyword.TAG_REF), referenceKey)); - } + List references = this.generationContext.getReferences(definitionKey); + List nullableReferences = this.generationContext.getNullableReferences(definitionKey); + final String referenceKey = this.updateReferences(references, definitionCollectionDetails, baseReferenceKey); if (!nullableReferences.isEmpty()) { - ObjectNode definition; - if (referenceInline) { - definition = generationContext.getDefinition(definitionKey); - } else { - definition = this.config.createObjectNode().put(this.config.getKeyword(SchemaKeyword.TAG_REF), referenceKey); - } - generationContext.makeNullable(definition); - if (this.shouldCreateNullableDefinition(generationContext, definitionKey, nullableReferences)) { - String nullableDefinitionName = this.definitionNamingStrategy - .adjustNullableName(definitionKey, definitionName, generationContext); - definitionsNode.set(nullableDefinitionName, definition); - nullableReferences.forEach(node -> node.put(this.config.getKeyword(SchemaKeyword.TAG_REF), - this.config.getKeyword(SchemaKeyword.TAG_REF_MAIN) + '/' + designatedDefinitionPath + '/' + nullableDefinitionName)); - } else { - nullableReferences.forEach(node -> AttributeCollector.mergeMissingAttributes(node, definition)); - } + updateNullableReferences(nullableReferences, definitionCollectionDetails, referenceKey, baseReferenceKey); } } definitionsNode.forEach(node -> this.schemaNodes.add((ObjectNode) node)); return definitionsNode; } + private String updateReferences(List references, DefinitionCollectionDetails definitionCollectionDetails, + Map.Entry baseReferenceKey) { + if (definitionCollectionDetails.shouldProduceDefinition(baseReferenceKey.getKey())) { + final String referenceKey; + if (definitionCollectionDetails.isMainSchemaKey(baseReferenceKey.getKey()) && !this.config.shouldCreateDefinitionForMainSchema()) { + // no need to add the main schema into the definitions, unless explicitly configured to do so + referenceKey = this.config.getKeyword(SchemaKeyword.TAG_REF_MAIN); + } else { + // add it to the definitions + definitionCollectionDetails.getDefinitionsNode() + .set(baseReferenceKey.getValue(), this.generationContext.getDefinition(baseReferenceKey.getKey())); + referenceKey = definitionCollectionDetails.getReferenceKey(baseReferenceKey.getValue()); + } + references.forEach(node -> node.put(this.config.getKeyword(SchemaKeyword.TAG_REF), referenceKey)); + return referenceKey; + } + // in-line the sub-schema everywhere (assuming there is no complex hierarchy inside (especially no circular reference) + ObjectNode definition = this.generationContext.getDefinition(baseReferenceKey.getKey()); + references.forEach(node -> AttributeCollector.mergeMissingAttributes(node, definition)); + return null; + } + + private void updateNullableReferences(List nullableReferences, DefinitionCollectionDetails definitionCollectionDetails, + String nonNullableReferenceKey, Map.Entry baseReferenceKey) { + DefinitionKey definitionKey = baseReferenceKey.getKey(); + ObjectNode definition; + if (nonNullableReferenceKey == null) { + definition = this.generationContext.getDefinition(definitionKey); + } else { + definition = this.config.createObjectNode().put(this.config.getKeyword(SchemaKeyword.TAG_REF), nonNullableReferenceKey); + } + this.generationContext.makeNullable(definition); + if (this.shouldCreateNullableDefinition(this.generationContext, definitionKey, nullableReferences)) { + String nullableDefinitionName = this.definitionNamingStrategy + .adjustNullableName(definitionKey, baseReferenceKey.getValue(), this.generationContext); + definitionCollectionDetails.getDefinitionsNode().set(nullableDefinitionName, definition); + nullableReferences.forEach(node -> node.put(this.config.getKeyword(SchemaKeyword.TAG_REF), + definitionCollectionDetails.getReferenceKey(nullableDefinitionName))); + } else { + nullableReferences.forEach(node -> AttributeCollector.mergeMissingAttributes(node, definition)); + } + } + private boolean shouldCreateNullableDefinition(SchemaGenerationContextImpl generationContext, DefinitionKey definitionKey, List nullableReferences) { if (this.config.shouldInlineNullableSchemas()) { @@ -268,30 +281,18 @@ private boolean shouldCreateNullableDefinition(SchemaGenerationContextImpl gener return this.config.shouldCreateDefinitionsForAllObjects() || nullableReferences.size() > 1; } - private String getReferenceKey(DefinitionKey mainSchemaKey, DefinitionKey definitionKey, Supplier addDefinitionAndReturnReferenceKey) { - final String referenceKey; - if (definitionKey.equals(mainSchemaKey) && !this.config.shouldCreateDefinitionForMainSchema()) { - // no need to add the main schema into the definitions, unless explicitly configured to do so - referenceKey = this.config.getKeyword(SchemaKeyword.TAG_REF_MAIN); - } else { - // add it to the definitions - referenceKey = addDefinitionAndReturnReferenceKey.get(); - } - return referenceKey; - } - /** * Produce reusable predicate for checking whether a given type should produce an entry in the {@link SchemaKeyword#TAG_DEFINITIONS} or not. * * @param mainSchemaKey main type to consider - * @param considerOnlyDirectReferences whether to ignore nullable references when determing about definition vs. inlining + * @param considerOnlyDirectReferences whether to ignore nullable references when determining about definition vs. inlining * @return reusable predicate */ private Predicate getShouldProduceDefinitionCheck(DefinitionKey mainSchemaKey, AtomicBoolean considerOnlyDirectReferences) { final boolean createDefinitionsForAll = this.config.shouldCreateDefinitionsForAllObjects(); final boolean inlineAllSchemas = this.config.shouldInlineAllSchemas(); return definitionKey -> { - if (generationContext.shouldNeverInlineDefinition(definitionKey)) { + if (this.generationContext.shouldNeverInlineDefinition(definitionKey)) { // e.g. custom definition explicitly marked to always produce a definition return true; } @@ -302,15 +303,13 @@ private Predicate getShouldProduceDefinitionCheck(DefinitionKey m if (definitionKey.equals(mainSchemaKey)) { return true; } - List references = generationContext.getReferences(definitionKey); + List references = this.generationContext.getReferences(definitionKey); if (considerOnlyDirectReferences.get() && references.isEmpty()) { return false; } - if (createDefinitionsForAll || references.size() > 1) { - return true; - } - List nullableReferences = generationContext.getNullableReferences(definitionKey); - return (references.size() + nullableReferences.size()) > 1; + return createDefinitionsForAll + || references.size() > 1 + || (references.size() + this.generationContext.getNullableReferences(definitionKey).size()) > 1; }; } @@ -319,35 +318,16 @@ private Predicate getShouldProduceDefinitionCheck(DefinitionKey m * * @param mainSchemaKey special definition key for the main schema * @param shouldProduceDefinition filter to indicate whether a given key should be considered when determining definition names - * @param generationContext generation context in which all traversed types and their definitions have been collected * @return encountered types with their corresponding reference keys */ - private Map getReferenceKeys(DefinitionKey mainSchemaKey, Predicate shouldProduceDefinition, - SchemaGenerationContextImpl generationContext) { - boolean createDefinitionForMainSchema = this.config.shouldCreateDefinitionForMainSchema(); - Function definitionNamesForKey = key -> this.definitionNamingStrategy.getDefinitionNameForKey(key, generationContext); - Map> aliases = generationContext.getDefinedTypes().stream() - .collect(Collectors.groupingBy(definitionNamesForKey, TreeMap::new, Collectors.toList())); + private Map getReferenceKeys(DefinitionKey mainSchemaKey, Predicate shouldProduceDefinition) { + Function definitionNameForKey = key -> this.definitionNamingStrategy.getDefinitionNameForKey(key, + this.generationContext); + Map> aliases = this.generationContext.getDefinedTypes().stream() + .collect(Collectors.groupingBy(definitionNameForKey, TreeMap::new, Collectors.toList())); Map referenceKeys = new LinkedHashMap<>(); for (Map.Entry> group : aliases.entrySet()) { - group.getValue().forEach(key -> referenceKeys.put(key, "")); - List definitionKeys = group.getValue().stream() - .filter(shouldProduceDefinition) - .collect(Collectors.toList()); - if (definitionKeys.size() == 1 - || (definitionKeys.size() == 2 && !createDefinitionForMainSchema && definitionKeys.contains(mainSchemaKey))) { - definitionKeys.forEach(key -> referenceKeys.put(key, group.getKey())); - } else { - Map referenceKeyGroup = definitionKeys.stream() - .collect(Collectors.toMap(key -> key, _key -> group.getKey(), (val1, _val2) -> val1, LinkedHashMap::new)); - this.definitionNamingStrategy.adjustDuplicateNames(referenceKeyGroup, generationContext); - if (definitionKeys.size() != referenceKeyGroup.size()) { - throw new IllegalStateException(SchemaDefinitionNamingStrategy.class.getSimpleName() - + " of type " + this.definitionNamingStrategy.getClass().getSimpleName() - + " altered list of subschemas with duplicate names."); - } - referenceKeys.putAll(referenceKeyGroup); - } + this.collectReferenceKeysFromGroup(referenceKeys, group, mainSchemaKey, shouldProduceDefinition); } String remainingDuplicateKeys = referenceKeys.values().stream() .filter(value -> !value.isEmpty()) @@ -363,4 +343,74 @@ private Map getReferenceKeys(DefinitionKey mainSchemaKey, } return referenceKeys; } + + private void collectReferenceKeysFromGroup(Map referenceKeys, Map.Entry> group, + DefinitionKey mainSchemaKey, Predicate shouldProduceDefinition) { + String baseDefinitionName = group.getKey(); + List definitionKeys = group.getValue().stream() + .peek(key -> referenceKeys.put(key, "")) + .filter(shouldProduceDefinition) + .collect(Collectors.toList()); + if (this.areDefinitionKeysDistinct(mainSchemaKey, definitionKeys)) { + definitionKeys.forEach(key -> referenceKeys.put(key, baseDefinitionName)); + } else { + Map referenceKeyGroup = definitionKeys.stream() + .collect(Collectors.toMap(key -> key, _key -> baseDefinitionName, (val1, _val2) -> val1, LinkedHashMap::new)); + this.definitionNamingStrategy.adjustDuplicateNames(referenceKeyGroup, this.generationContext); + if (definitionKeys.size() != referenceKeyGroup.size()) { + throw new IllegalStateException(SchemaDefinitionNamingStrategy.class.getSimpleName() + + " of type " + this.definitionNamingStrategy.getClass().getSimpleName() + + " altered list of subschemas with duplicate names."); + } + referenceKeys.putAll(referenceKeyGroup); + } + } + + /** + * Check whether the given key can be included with its standard name. Otherwise, dedicated names need to be assigned for each definition key. + * + * @param mainSchemaKey the main schema's key to ignore in the list if it's being in-lined (unless otherwise configured) + * @param definitionKeys list of keys that have the same standard name in the "definitions"/"$defs" + * @return whether the given key can be included with its standard name + */ + private boolean areDefinitionKeysDistinct(DefinitionKey mainSchemaKey, List definitionKeys) { + return definitionKeys.size() == 1 + || (definitionKeys.size() == 2 + && !this.config.shouldCreateDefinitionForMainSchema() + && definitionKeys.contains(mainSchemaKey)); + } + + /** + * Helper type to group common method parameters in order to reduce overall parameter count and complexity in this class. + */ + private static class DefinitionCollectionDetails { + private final DefinitionKey mainSchemaKey; + private final String referenceKeyPrefix; + private final Predicate shouldProduceDefinition; + private final ObjectNode definitionsNode; + + DefinitionCollectionDetails(DefinitionKey mainSchemaKey, String referenceKeyPrefix, + Predicate shouldProduceDefinition, ObjectNode definitionsNode) { + this.mainSchemaKey = mainSchemaKey; + this.referenceKeyPrefix = referenceKeyPrefix; + this.shouldProduceDefinition = shouldProduceDefinition; + this.definitionsNode = definitionsNode; + } + + boolean isMainSchemaKey(DefinitionKey key) { + return key.equals(this.mainSchemaKey); + } + + String getReferenceKey(String definitionName) { + return this.referenceKeyPrefix + definitionName; + } + + boolean shouldProduceDefinition(DefinitionKey key) { + return this.shouldProduceDefinition.test(key); + } + + ObjectNode getDefinitionsNode() { + return this.definitionsNode; + } + } }