fix: Jackson JsonPropertySorter handling non-getter methods

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`

jsonschema-generator-parent/pom.xml

@@ -118,6 +118,7 @@
             <url>https://github.com/magicDGS</url>
             <roles>
                 <role>Provided PR #300 (introducing support for standard "format" values via Option)</role>
+                <role>Provided PR #423 (fixing Jackson property order handling)</role>
             </roles>
         </contributor>
     </contributors>

jsonschema-generator/src/main/java/com/github/victools/jsonschema/generator/impl/SchemaCleanUpUtils.java

@@ -345,7 +345,7 @@ private Set<String> 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<ObjectNode> mergeSchemas(ObjectNode mainNodeIncludingAllOf, List<JsonNode> nodes, Map<String, SchemaKeyword> 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<ObjectNode> parts = nodes.stream()
@@ -354,9 +354,7 @@ private Supplier<ObjectNode> mergeSchemas(ObjectNode mainNodeIncludingAllOf, Lis
                 .collect(Collectors.toList());
 
         // collect all defined attributes from the separate parts and check whether there are incompatible differences
-        Map<String, List<JsonNode>> fieldsFromAllParts = parts.stream()
-                .flatMap(part -> StreamSupport.stream(((Iterable<Map.Entry<String, JsonNode>>) part::fields).spliterator(), false))
-                .collect(Collectors.groupingBy(Map.Entry::getKey, LinkedHashMap::new, Collectors.mapping(Map.Entry::getValue, Collectors.toList())));
+        Map<String, List<JsonNode>> fieldsFromAllParts = this.getFieldsFromAllParts(parts);
         if (this.shouldSkipMergingAllOf(mainNodeIncludingAllOf, parts, fieldsFromAllParts)) {
             return null;
         }
@@ -384,13 +382,25 @@ private Supplier<ObjectNode> 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<String, List<JsonNode>> getFieldsFromAllParts(List<ObjectNode> parts) {
+        return parts.stream()
+                .flatMap(part -> StreamSupport.stream(((Iterable<Map.Entry<String, JsonNode>>) 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<ObjectNode> parts, Map<String, List<JsonNode>> fieldsFromAllParts) {

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.<MemberScope>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();
         }

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")

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<String> 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;
+        }
+    }
+
+}