feat: Introduce Option for duplicate member attribute removal

victools · Jan 9, 2024 · c305d1b · c305d1b
1 parent f85cc1d
commit c305d1b
Show file tree

Hide file tree

Showing 9 changed files with 100 additions and 13 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -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`

diff --git a/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/Option.java b/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/Option.java
@@ -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.

diff --git a/...schema-generator/src/main/java/com/github/victools/jsonschema/generator/OptionPreset.java b/...schema-generator/src/main/java/com/github/victools/jsonschema/generator/OptionPreset.java
@@ -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
     );
 
     /**
@@ -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
     );
 
     /**
@@ -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;

diff --git a/...chema-generator/src/main/java/com/github/victools/jsonschema/generator/SchemaBuilder.java b/...chema-generator/src/main/java/com/github/victools/jsonschema/generator/SchemaBuilder.java
@@ -195,7 +195,9 @@ private void performCleanup(ObjectNode definitionsNode, String referenceKeyPrefi
             cleanUpUtils.reduceAllOfNodes(this.schemaNodes);
         }
         cleanUpUtils.reduceAnyOfNodes(this.schemaNodes);
-        cleanUpUtils.reduceRedundantMemberAttributes(this.schemaNodes, definitionsNode, referenceKeyPrefix);
+        if (this.config.shouldDiscardDuplicateMemberAttributes()) {
+            cleanUpUtils.reduceRedundantMemberAttributes(this.schemaNodes, definitionsNode, referenceKeyPrefix);
+        }
         if (this.config.shouldIncludeStrictTypeInfo()) {
             cleanUpUtils.setStrictTypeInfo(this.schemaNodes, true);
         }

diff --git a/...nerator/src/main/java/com/github/victools/jsonschema/generator/SchemaGeneratorConfig.java b/...nerator/src/main/java/com/github/victools/jsonschema/generator/SchemaGeneratorConfig.java
@@ -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.
      *

diff --git a/...rator/src/main/java/com/github/victools/jsonschema/generator/impl/SchemaCleanUpUtils.java b/...rator/src/main/java/com/github/victools/jsonschema/generator/impl/SchemaCleanUpUtils.java
@@ -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;
@@ -607,9 +608,22 @@ private void reduceRedundantMemberAttributesIfPossible(ObjectNode schemaNode,
      * @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();
-            if (memberAttribute.getValue().equals(referencedDefinition.get(memberAttribute.getKey()))) {
+            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();
             }

diff --git a/...rc/main/java/com/github/victools/jsonschema/generator/impl/SchemaGeneratorConfigImpl.java b/...rc/main/java/com/github/victools/jsonschema/generator/impl/SchemaGeneratorConfigImpl.java
@@ -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);

diff --git a/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/impl/Util.java b/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/impl/Util.java
@@ -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);
+    }
 }
diff --git a/.../test/java/com/github/victools/jsonschema/generator/SchemaGeneratorMemberCleanUpTest.java b/.../test/java/com/github/victools/jsonschema/generator/SchemaGeneratorMemberCleanUpTest.java
@@ -17,23 +17,46 @@
 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 {
 
-    @Test
-    public void testMemberCleanUp() {
-        SchemaGeneratorConfig generatorConfig = new SchemaGeneratorConfigBuilder(SchemaVersion.DRAFT_2019_09, OptionPreset.PLAIN_JSON)
-                .with(Option.MAP_VALUES_AS_ADDITIONAL_PROPERTIES)
-                .with(Option.DEFINITIONS_FOR_ALL_OBJECTS)
-                .build();
+    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);
-        System.out.println(schema.toPrettyString());
+        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 {