From dfc09820b1a525e9f46c85a8376a20b785702e0e Mon Sep 17 00:00:00 2001 From: Carsten Wickner <11309681+CarstenWickner@users.noreply.github.com> Date: Sun, 10 May 2020 14:49:44 +0200 Subject: [PATCH] feat: respect JsonPropertyOrder annotations (#99) * chore: parameterise tests for property sorting * feat: respect JsonPropertyOrder annotations --- CHANGELOG.md | 4 + .../generator/impl/PropertySortUtilsTest.java | 80 +++++------- jsonschema-module-jackson/README.md | 9 +- .../module/jackson/JacksonModule.java | 5 + .../module/jackson/JacksonOption.java | 6 + .../module/jackson/JsonPropertySorter.java | 114 ++++++++++++++++++ .../module/jackson/JacksonModuleTest.java | 11 ++ .../jackson/JsonPropertySorterTest.java | 111 +++++++++++++++++ 8 files changed, 283 insertions(+), 57 deletions(-) create mode 100644 jsonschema-module-jackson/src/main/java/com/github/victools/jsonschema/module/jackson/JsonPropertySorter.java create mode 100644 jsonschema-module-jackson/src/test/java/com/github/victools/jsonschema/module/jackson/JsonPropertySorterTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index d529b2c8..480e0e10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 #### Added - New `SchemaGeneratorGeneralConfigPart.withPropertySorter()` exposing the sorting logic of an object schema's properties +### `jsonschema-module-jackson` +#### Added +- New `JacksonOption.RESPECT_JSONPROPERTY_ORDER` to sort properties in an object's schema based on `@JsonPropertyOrder` annotations + ## [4.11.1] - 2020-04-30 ### `jsonschema-maven-plugin` #### Fixed diff --git a/jsonschema-generator/src/test/java/com/github/victools/jsonschema/generator/impl/PropertySortUtilsTest.java b/jsonschema-generator/src/test/java/com/github/victools/jsonschema/generator/impl/PropertySortUtilsTest.java index c3a259b0..2a34f217 100644 --- a/jsonschema-generator/src/test/java/com/github/victools/jsonschema/generator/impl/PropertySortUtilsTest.java +++ b/jsonschema-generator/src/test/java/com/github/victools/jsonschema/generator/impl/PropertySortUtilsTest.java @@ -19,40 +19,23 @@ import com.github.victools.jsonschema.generator.FieldScope; import com.github.victools.jsonschema.generator.MemberScope; import com.github.victools.jsonschema.generator.MethodScope; -import java.util.Arrays; -import java.util.List; +import java.util.Comparator; import java.util.stream.Collectors; +import java.util.stream.Stream; +import junitparams.JUnitParamsRunner; +import junitparams.Parameters; +import junitparams.naming.TestCaseName; import org.junit.Assert; -import org.junit.Before; import org.junit.Test; +import org.junit.runner.RunWith; import org.mockito.Mockito; /** * Test for the {@link PropertySortUtils} class. */ +@RunWith(JUnitParamsRunner.class) public class PropertySortUtilsTest { - private FieldScope instanceFieldA; - private FieldScope instanceFieldC; - private MethodScope instanceMethodB; - private FieldScope staticFieldE; - private MethodScope staticMethodD; - private MethodScope staticMethodF; - private List> memberList; - - @Before - public void setUp() { - this.instanceFieldA = this.createMemberMock(FieldScope.class, false, "a"); - this.instanceFieldC = this.createMemberMock(FieldScope.class, false, "c"); - this.staticFieldE = this.createMemberMock(FieldScope.class, true, "e"); - this.instanceMethodB = this.createMemberMock(MethodScope.class, false, "b()"); - this.staticMethodD = this.createMemberMock(MethodScope.class, true, "d()"); - this.staticMethodF = this.createMemberMock(MethodScope.class, true, "f()"); - - this.memberList = Arrays.asList( - this.instanceFieldC, this.staticMethodF, this.staticFieldE, this.instanceFieldA, this.instanceMethodB, this.staticMethodD); - } - private > S createMemberMock(Class scopeType, boolean isStatic, String name) { S mock = Mockito.mock(scopeType); Mockito.when(mock.isStatic()).thenReturn(isStatic); @@ -60,39 +43,30 @@ public void setUp() { return mock; } - /** - * Test the correct sorting based on the {@link PropertySortUtils#SORT_PROPERTIES_FIELDS_BEFORE_METHODS} {@code Comparator}. - */ - @Test - public void testSortPropertiesFieldsBeforeMethods() { - String sortingResult = this.memberList.stream() - .sorted(PropertySortUtils.SORT_PROPERTIES_FIELDS_BEFORE_METHODS) - .map(MemberScope::getSchemaPropertyName) - .collect(Collectors.joining(" ")); - Assert.assertEquals("c e a f() b() d()", sortingResult); - } - - /** - * Test the correct sorting based on the {@link PropertySortUtils#SORT_PROPERTIES_BY_NAME_ALPHABETICALLY} {@code Comparator}. - */ - @Test - public void testSortPropertiesByNameAlphabetically() { - String sortingResult = this.memberList.stream() - .sorted(PropertySortUtils.SORT_PROPERTIES_BY_NAME_ALPHABETICALLY) - .map(MemberScope::getSchemaPropertyName) - .collect(Collectors.joining(" ")); - Assert.assertEquals("a b() c d() e f()", sortingResult); + public Object[] parametersForTestSortProperties() { + Comparator> noSorting = (_first, _second) -> 0; + return new Object[][]{ + {"unsorted", "c f() e a b() d()", noSorting}, + {"fields-before-methods", "c e a f() b() d()", PropertySortUtils.SORT_PROPERTIES_FIELDS_BEFORE_METHODS}, + {"alphabetically-by-name", "a b() c d() e f()", PropertySortUtils.SORT_PROPERTIES_BY_NAME_ALPHABETICALLY}, + {"default-order", "a c e b() d() f()", PropertySortUtils.DEFAULT_PROPERTY_ORDER} + }; } - /** - * Test the correct sorting based on the {@link PropertySortUtils#DEFAULT_PROPERTY_ORDER} {@code Comparator}. - */ @Test - public void testDefaultPropertyOrder() { - String sortingResult = this.memberList.stream() - .sorted(PropertySortUtils.DEFAULT_PROPERTY_ORDER) + @Parameters + @TestCaseName(value = "{method}({0}) [{index}]") + public void testSortProperties(String _testCaseName, String expectedResult, Comparator> sortingLogic) { + Stream> properties = Stream.of( + this.createMemberMock(FieldScope.class, false, "c"), + this.createMemberMock(MethodScope.class, true, "f()"), + this.createMemberMock(FieldScope.class, true, "e"), + this.createMemberMock(FieldScope.class, false, "a"), + this.createMemberMock(MethodScope.class, false, "b()"), + this.createMemberMock(MethodScope.class, true, "d()")); + String sortingResult = properties.sorted(sortingLogic) .map(MemberScope::getSchemaPropertyName) .collect(Collectors.joining(" ")); - Assert.assertEquals("a c e b() d() f()", sortingResult); + Assert.assertEquals(expectedResult, sortingResult); } } diff --git a/jsonschema-module-jackson/README.md b/jsonschema-module-jackson/README.md index c23a8f4f..951700af 100644 --- a/jsonschema-module-jackson/README.md +++ b/jsonschema-module-jackson/README.md @@ -11,10 +11,11 @@ Module for the [jsonschema-generator](../jsonschema-generator) – deriving JSON 5. Ignore fields that are deemed to be ignored according to various other `jackson-annotations` (e.g. `@JsonIgnore`, `@JsonIgnoreType`, `@JsonIgnoreProperties`) or are otherwise supposed to be excluded. 6. Optionally: treat enum types as plain strings, serialized by `@JsonValue` annotated method 7. Optionally: treat enum types as plain strings, as per each enum constant's `@JsonProperty` annotation -8. Optionally: resolve subtypes according to `@JsonSubTypes` on a supertype in general or directly on specific fields/methods as an override of the per-type behavior. -9. Optionally: apply structural changes for subtypes according to `@JsonTypeInfo` on a supertype in general or directly on specific fields/methods as an override of the per-type behavior. - - Considering @JsonTypeInfo.include with values As.PROPERTY, As.EXISTING_PROPERTY, As.WRAPPER_ARRAY, As.WRAPPER_OBJECT - - Considering @JsonTypeInfo.use with values Id.CLASS, Id.NAME +8. Optionally: sort an object's properties according to its `@JsonPropertyOrder` annotation +9. Optionally: resolve subtypes according to `@JsonSubTypes` on a supertype in general or directly on specific fields/methods as an override of the per-type behavior. +10. Optionally: apply structural changes for subtypes according to `@JsonTypeInfo` on a supertype in general or directly on specific fields/methods as an override of the per-type behavior. + - Considering `@JsonTypeInfo.include` with values `As.PROPERTY`, `As.EXISTING_PROPERTY`, `As.WRAPPER_ARRAY`, `As.WRAPPER_OBJECT` + - Considering `@JsonTypeInfo.use` with values `Id.CLASS`, `Id.NAME` Schema attributes derived from validation annotations on getter methods are also applied to their associated fields. diff --git a/jsonschema-module-jackson/src/main/java/com/github/victools/jsonschema/module/jackson/JacksonModule.java b/jsonschema-module-jackson/src/main/java/com/github/victools/jsonschema/module/jackson/JacksonModule.java index 6df3532f..c64f22b8 100644 --- a/jsonschema-module-jackson/src/main/java/com/github/victools/jsonschema/module/jackson/JacksonModule.java +++ b/jsonschema-module-jackson/src/main/java/com/github/victools/jsonschema/module/jackson/JacksonModule.java @@ -85,6 +85,11 @@ public void applyToConfigBuilder(SchemaGeneratorConfigBuilder builder) { if (considerEnumJsonValue || considerEnumJsonProperty) { generalConfigPart.withCustomDefinitionProvider(new CustomEnumDefinitionProvider(considerEnumJsonValue, considerEnumJsonProperty)); } + + if (this.options.contains(JacksonOption.RESPECT_JSONPROPERTY_ORDER)) { + generalConfigPart.withPropertySorter(new JsonPropertySorter(true)); + } + boolean lookUpSubtypes = !this.options.contains(JacksonOption.SKIP_SUBTYPE_LOOKUP); boolean includeTypeInfoTransform = !this.options.contains(JacksonOption.IGNORE_TYPE_INFO_TRANSFORM); if (lookUpSubtypes || includeTypeInfoTransform) { diff --git a/jsonschema-module-jackson/src/main/java/com/github/victools/jsonschema/module/jackson/JacksonOption.java b/jsonschema-module-jackson/src/main/java/com/github/victools/jsonschema/module/jackson/JacksonOption.java index 803d61be..eaf72ae7 100644 --- a/jsonschema-module-jackson/src/main/java/com/github/victools/jsonschema/module/jackson/JacksonOption.java +++ b/jsonschema-module-jackson/src/main/java/com/github/victools/jsonschema/module/jackson/JacksonOption.java @@ -46,6 +46,12 @@ public enum JacksonOption { * @see com.github.victools.jsonschema.generator.Option#FLATTENED_ENUMS_FROM_TOSTRING */ FLATTENED_ENUMS_FROM_JSONPROPERTY, + /** + * Use this option to sort an object's properties according to associated + * {@link com.fasterxml.jackson.annotation.JsonPropertyOrder JsonPropertyOrder} annotations. Fields and methods without such an annotation are + * listed after annotated properties. + */ + RESPECT_JSONPROPERTY_ORDER, /** * Use this option to skip the automatic look-up of subtypes according to {@code @JsonSubTypes} annotations. */ 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 new file mode 100644 index 00000000..ffef04a3 --- /dev/null +++ b/jsonschema-module-jackson/src/main/java/com/github/victools/jsonschema/module/jackson/JsonPropertySorter.java @@ -0,0 +1,114 @@ +/* + * Copyright 2020 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.annotation.JsonPropertyOrder; +import com.github.victools.jsonschema.generator.MemberScope; +import com.github.victools.jsonschema.generator.MethodScope; +import com.github.victools.jsonschema.generator.impl.PropertySortUtils; +import java.util.Arrays; +import java.util.Collections; +import java.util.Comparator; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.stream.Stream; + +/** + * Implementation of the sorting logic for an object's properties based on a {@link JsonPropertyOrder} annotation on the declaring type. + */ +public class JsonPropertySorter implements Comparator> { + + private final boolean sortAlphabeticallyIfNotAnnotated; + private final Map, List> propertyOrderPerDeclaringType = new HashMap<>(); + private final Map, Boolean> enabledAlphabeticSorting = new HashMap<>(); + + /** + * Constructor. + * + * @param sortAlphabeticallyIfNotAnnotated whether properties of a type without {@link JsonPropertyOrder} should be sorted alphabetically + */ + public JsonPropertySorter(boolean sortAlphabeticallyIfNotAnnotated) { + this.sortAlphabeticallyIfNotAnnotated = sortAlphabeticallyIfNotAnnotated; + } + + @Override + public int compare(MemberScope first, MemberScope second) { + int result = PropertySortUtils.SORT_PROPERTIES_FIELDS_BEFORE_METHODS.compare(first, second); + if (result == 0) { + result = this.getPropertyIndex(first) - this.getPropertyIndex(second); + } + if (result == 0 && Stream.of(first, second) + .map(property -> property.getDeclaringType().getErasedType()) + .anyMatch(parentType -> this.enabledAlphabeticSorting.computeIfAbsent(parentType, this::shouldSortPropertiesAlphabetically))) { + result = PropertySortUtils.SORT_PROPERTIES_BY_NAME_ALPHABETICALLY.compare(first, second); + } + return result; + } + + /** + * Determine the given property's position in its declaring type's schema based on a {@link JsonPropertyOrder} annotation. If no such annotation + * is present, {@link Integer#MAX_VALUE} will be returned to append these at the end of the list of properties. + * + * @param property field/method for which the respective index should be determined + * @return specific property index or {@link Integer#MAX_VALUE} + */ + protected int getPropertyIndex(MemberScope property) { + List sortedProperties = this.propertyOrderPerDeclaringType + .computeIfAbsent(property.getDeclaringType().getErasedType(), this::getAnnotatedPropertyOrder); + String fieldName; + if (property instanceof MethodScope) { + fieldName = Optional.ofNullable(((MethodScope) property).findGetterField()).map(MemberScope::getSchemaPropertyName).orElse(null); + } else { + fieldName = property.getSchemaPropertyName(); + } + int propertyIndex = sortedProperties.indexOf(fieldName); + if (propertyIndex == -1) { + propertyIndex = Integer.MAX_VALUE; + } + return propertyIndex; + } + + /** + * Determine whether the given type's properties that are not specifically mentioned in a {@link JsonPropertyOrder} annotation should be sorted + * alphabetically, based on {@link JsonPropertyOrder#alphabetic()}. If no such annotation is present, the value given in the + * {@link #JsonPropertySorter(boolean)} constructor. + * + * @param declaringType type for which the properties' default sorting should be determined + * @return whether properties that are not specifically mentioned in a {@link JsonPropertyOrder} annotation should be sorted alphabetically + */ + protected boolean shouldSortPropertiesAlphabetically(Class declaringType) { + return Optional.ofNullable(declaringType.getAnnotation(JsonPropertyOrder.class)) + .map(JsonPropertyOrder::alphabetic) + .orElse(this.sortAlphabeticallyIfNotAnnotated); + } + + /** + * Lookup the list of specifically sorted property names in the given type based on its {@link JsonPropertyOrder} annotation. + * + * @param declaringType type for which to lookup the list of specifically sorted property names + * @return {@link JsonPropertyOrder#value()} or empty list + */ + private List getAnnotatedPropertyOrder(Class declaringType) { + return Optional.ofNullable(declaringType.getAnnotation(JsonPropertyOrder.class)) + .map(JsonPropertyOrder::value) + .filter(valueArray -> valueArray.length != 0) + .map(Arrays::asList) + .orElseGet(Collections::emptyList); + } +} diff --git a/jsonschema-module-jackson/src/test/java/com/github/victools/jsonschema/module/jackson/JacksonModuleTest.java b/jsonschema-module-jackson/src/test/java/com/github/victools/jsonschema/module/jackson/JacksonModuleTest.java index 03239ffb..3c23b0c7 100644 --- a/jsonschema-module-jackson/src/test/java/com/github/victools/jsonschema/module/jackson/JacksonModuleTest.java +++ b/jsonschema-module-jackson/src/test/java/com/github/victools/jsonschema/module/jackson/JacksonModuleTest.java @@ -74,6 +74,17 @@ public void testApplyToConfigBuilder() { Mockito.verifyNoMoreInteractions(this.configBuilder, this.fieldConfigPart, this.methodConfigPart, this.typesInGeneralConfigPart); } + @Test + public void testApplyToConfigBuilderWithRespectJsonPropertyOrderOption() { + new JacksonModule(JacksonOption.RESPECT_JSONPROPERTY_ORDER, JacksonOption.SKIP_SUBTYPE_LOOKUP, JacksonOption.IGNORE_TYPE_INFO_TRANSFORM) + .applyToConfigBuilder(this.configBuilder); + + this.verifyCommonConfigurations(); + + Mockito.verify(this.typesInGeneralConfigPart).withPropertySorter(Mockito.any(JsonPropertySorter.class)); + + Mockito.verifyNoMoreInteractions(this.configBuilder, this.fieldConfigPart, this.methodConfigPart, this.typesInGeneralConfigPart); + } @Test public void testApplyToConfigBuilderWithSkipSubtypeLookupAndIgnoreTypeInfoTranformOptions() { diff --git a/jsonschema-module-jackson/src/test/java/com/github/victools/jsonschema/module/jackson/JsonPropertySorterTest.java b/jsonschema-module-jackson/src/test/java/com/github/victools/jsonschema/module/jackson/JsonPropertySorterTest.java new file mode 100644 index 00000000..033f0374 --- /dev/null +++ b/jsonschema-module-jackson/src/test/java/com/github/victools/jsonschema/module/jackson/JsonPropertySorterTest.java @@ -0,0 +1,111 @@ +/* + * Copyright 2020 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.annotation.JsonPropertyOrder; +import com.github.victools.jsonschema.generator.MemberScope; +import com.github.victools.jsonschema.generator.SchemaVersion; +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; +import junitparams.JUnitParamsRunner; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; + +/** + * Test for the {@link JsonPropertySorter}. + */ +@RunWith(JUnitParamsRunner.class) +public class JsonPropertySorterTest extends AbstractTypeAwareTest { + + private List> memberList; + + public JsonPropertySorterTest() { + super(AnnotatedTestClass.class); + } + + @Before + public void setUp() { + this.prepareContextForVersion(SchemaVersion.DRAFT_2019_09); + this.memberList = Arrays.asList( + this.getTestClassField("x"), + this.getTestClassMethod("getX"), + this.getTestClassMethod("getA"), + this.getTestClassMethod("getB"), + this.getTestClassMethod("getC"), + this.getTestClassMethod("getD"), + this.getTestClassMethod("getE"), + this.getTestClassMethod("getF"), + this.getTestClassField("a"), + this.getTestClassField("b"), + this.getTestClassField("c"), + this.getTestClassField("d"), + this.getTestClassField("e"), + this.getTestClassField("f")); + } + + @Test + public void testPropertySortingAlphabeticallyByDefault() { + String sortingResult = this.memberList.stream() + .sorted(new JsonPropertySorter(true)) + .map(MemberScope::getSchemaPropertyName) + .collect(Collectors.joining(" ")); + Assert.assertEquals("a c e b f d x getA() getC() getE() getB() getF() getD() getX()", sortingResult); + } + + @JsonPropertyOrder(value = {"a", "c", "e", "b", "f"}, alphabetic = true) + private static class AnnotatedTestClass { + + private char x; + private String a; + private int b; + private long c; + private Boolean d; + private Object e; + private double f; + + public char getX() { + return this.x; + } + + public String getA() { + return this.a; + } + + public int getB() { + return this.b; + } + + public long getC() { + return this.c; + } + + public Boolean getD() { + return this.d; + } + + public Object getE() { + return this.e; + } + + public double getF() { + return this.f; + } + } +}