diff --git a/CHANGELOG.md b/CHANGELOG.md index 79db5aab..12e6fe40 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,9 @@ 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-module-jackson` +#### Fixed +- Respect `@JsonPropertyOrder` also for properties derived from non-getter methods ## [4.33.0] - 2023-11-23 ### `jsonschema-generator` diff --git a/jsonschema-generator-parent/pom.xml b/jsonschema-generator-parent/pom.xml index 165141c5..79f11c3a 100644 --- a/jsonschema-generator-parent/pom.xml +++ b/jsonschema-generator-parent/pom.xml @@ -118,6 +118,7 @@ https://github.com/magicDGS Provided PR #300 (introducing support for standard "format" values via Option) + Provided PR #423 (fixing Jackson property order handling) diff --git a/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/impl/SchemaCleanUpUtils.java b/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/impl/SchemaCleanUpUtils.java index 9248049d..c877e2b9 100644 --- a/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/impl/SchemaCleanUpUtils.java +++ b/jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/impl/SchemaCleanUpUtils.java @@ -345,7 +345,7 @@ private Set collectTextItemsFromArrayNode(JsonNode arrayNode) { * @return supplier of the successfully merged schemas (into a new node) or {@code null} if merging the given nodes is not easily possible */ private Supplier mergeSchemas(ObjectNode mainNodeIncludingAllOf, List nodes, Map reverseKeywordMap) { - if (nodes.stream().anyMatch(part -> !(part instanceof ObjectNode) && !(part.isBoolean() && part.asBoolean()))) { + if (nodes.stream().anyMatch(part -> part.isBoolean() && !part.asBoolean())) { return null; } List parts = nodes.stream() @@ -354,9 +354,7 @@ private Supplier mergeSchemas(ObjectNode mainNodeIncludingAllOf, Lis .collect(Collectors.toList()); // collect all defined attributes from the separate parts and check whether there are incompatible differences - Map> fieldsFromAllParts = parts.stream() - .flatMap(part -> StreamSupport.stream(((Iterable>) part::fields).spliterator(), false)) - .collect(Collectors.groupingBy(Map.Entry::getKey, LinkedHashMap::new, Collectors.mapping(Map.Entry::getValue, Collectors.toList()))); + Map> fieldsFromAllParts = this.getFieldsFromAllParts(parts); if (this.shouldSkipMergingAllOf(mainNodeIncludingAllOf, parts, fieldsFromAllParts)) { return null; } @@ -384,13 +382,25 @@ private Supplier mergeSchemas(ObjectNode mainNodeIncludingAllOf, Lis }; } + /** + * Collect all defined attributes from the separate parts. + * + * @param parts entries of the {@link SchemaKeyword#TAG_ALLOF} array to consider + * @return flattened collection of all attributes in the given parts + */ + private Map> getFieldsFromAllParts(List parts) { + return parts.stream() + .flatMap(part -> StreamSupport.stream(((Iterable>) part::fields).spliterator(), false)) + .collect(Collectors.groupingBy(Map.Entry::getKey, LinkedHashMap::new, Collectors.mapping(Map.Entry::getValue, Collectors.toList()))); + } + /** * Check whether the merging of the given node and it's allOf entries should be skipped due to a {@link SchemaKeyword#TAG_REF} being present. * Drafts 6 and 7 would ignore any other attributes besides the {@link SchemaKeyword#TAG_REF}. * * @param mainNode the main node containing an {@link SchemaKeyword#TAG_ALLOF} array (maybe {@code null}) * @param parts entries of the {@link SchemaKeyword#TAG_ALLOF} array to consider - * @param fieldsFromAllParts flatten collection of all attributes in the given parts + * @param fieldsFromAllParts flattened collection of all attributes in the given parts * @return whether to block merging of the given {@link SchemaKeyword#TAG_ALLOF} candidate */ private boolean shouldSkipMergingAllOf(ObjectNode mainNode, List parts, Map> fieldsFromAllParts) { diff --git a/jsonschema-module-jackson/src/main/java/com/github/victools/jsonschema/module/jackson/JsonPropertySorter.java b/jsonschema-module-jackson/src/main/java/com/github/victools/jsonschema/module/jackson/JsonPropertySorter.java index 2da2a4b4..3d6c79e5 100644 --- a/jsonschema-module-jackson/src/main/java/com/github/victools/jsonschema/module/jackson/JsonPropertySorter.java +++ b/jsonschema-module-jackson/src/main/java/com/github/victools/jsonschema/module/jackson/JsonPropertySorter.java @@ -75,7 +75,10 @@ protected int getPropertyIndex(MemberScope property) { .computeIfAbsent(topMostHierarchyType.getErasedType(), this::getAnnotatedPropertyOrder); String fieldName; if (property instanceof MethodScope) { - fieldName = Optional.ofNullable(((MethodScope) property).findGetterField()).map(MemberScope::getSchemaPropertyName).orElse(null); + fieldName = Optional.ofNullable(((MethodScope) property).findGetterField()) + // since 4.33.1: fall-back on method's property name if no getter can be found + .orElse(property) + .getSchemaPropertyName(); } else { fieldName = property.getSchemaPropertyName(); } diff --git a/jsonschema-module-jackson/src/test/java/com/github/victools/jsonschema/module/jackson/IntegrationTest.java b/jsonschema-module-jackson/src/test/java/com/github/victools/jsonschema/module/jackson/IntegrationTest.java index d58d730e..c446ad7d 100644 --- a/jsonschema-module-jackson/src/test/java/com/github/victools/jsonschema/module/jackson/IntegrationTest.java +++ b/jsonschema-module-jackson/src/test/java/com/github/victools/jsonschema/module/jackson/IntegrationTest.java @@ -75,7 +75,6 @@ private static String loadResource(String resourcePath) throws IOException { } } return stringBuilder.toString(); - } @JsonClassDescription("test description") diff --git a/jsonschema-module-jackson/src/test/java/com/github/victools/jsonschema/module/jackson/JsonPropertySorterIntegrationTest.java b/jsonschema-module-jackson/src/test/java/com/github/victools/jsonschema/module/jackson/JsonPropertySorterIntegrationTest.java new file mode 100644 index 00000000..e0cfdbfe --- /dev/null +++ b/jsonschema-module-jackson/src/test/java/com/github/victools/jsonschema/module/jackson/JsonPropertySorterIntegrationTest.java @@ -0,0 +1,107 @@ +/* + * Copyright 2023 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.module.jackson; + +import com.fasterxml.jackson.databind.node.ObjectNode; +import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonPropertyOrder; +import com.fasterxml.jackson.databind.JsonNode; +import com.github.victools.jsonschema.generator.Option; +import com.github.victools.jsonschema.generator.OptionPreset; +import com.github.victools.jsonschema.generator.SchemaGenerator; +import com.github.victools.jsonschema.generator.SchemaGeneratorConfig; +import com.github.victools.jsonschema.generator.SchemaGeneratorConfigBuilder; +import com.github.victools.jsonschema.generator.SchemaKeyword; +import com.github.victools.jsonschema.generator.SchemaVersion; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.stream.Stream; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; + +public class JsonPropertySorterIntegrationTest { + + @ParameterizedTest + @CsvSource({ + "TestObject, one two three", + "TestContainer, one two three" + }) + public void testJsonPropertyOrderWithChildAnnotations(String targetTypeName, String expectedFieldOrder) throws Exception { + JacksonModule module = new JacksonModule(JacksonOption.RESPECT_JSONPROPERTY_ORDER, + JacksonOption.INCLUDE_ONLY_JSONPROPERTY_ANNOTATED_METHODS); + SchemaGeneratorConfig config = new SchemaGeneratorConfigBuilder(SchemaVersion.DRAFT_2019_09, OptionPreset.PLAIN_JSON) + .with(Option.NONSTATIC_NONVOID_NONGETTER_METHODS, Option.FIELDS_DERIVED_FROM_ARGUMENTFREE_METHODS) + .with(module) + .build(); + Class targetType = Stream.of(JsonPropertySorterIntegrationTest.class.getDeclaredClasses()) + .filter(testType -> testType.getSimpleName().equals(targetTypeName)) + .findFirst() + .orElseThrow(IllegalArgumentException::new); + SchemaGenerator generator = new SchemaGenerator(config); + JsonNode result = generator.generateSchema(targetType); + + ObjectNode properties = (ObjectNode) result.get(config.getKeyword(SchemaKeyword.TAG_PROPERTIES)); + List resultPropertyNames = new ArrayList<>(); + properties.fieldNames().forEachRemaining(resultPropertyNames::add); + Assertions.assertEquals(Arrays.asList(expectedFieldOrder.split(" ")), resultPropertyNames); + } + + @JsonPropertyOrder({"one", "two", "three"}) + public static class TestContainer { + + @JsonProperty("three") + private Integer thirdInteger; + @JsonProperty("one") + private String firstString; + + @JsonProperty("two") + public boolean getSecondValue() { + return true; + } + } + + @JsonPropertyOrder({"one", "two", "three"}) + public static class TestObject { + + private TestContainer container; + private String secondString; + + @JsonIgnore + public TestContainer getContainer() { + return this.container; + } + + @JsonProperty("three") + public Integer getInteger() { + return this.container.thirdInteger; + } + + @JsonProperty("two") + public String getSecondString() { + return this.secondString; + } + + @JsonProperty("one") + public String getString() { + return this.container.firstString; + } + } + +}