Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deterministic and alphabetical orderding of JSON and YAML output #3613

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -138,6 +138,7 @@ public JsonSerializer<?> modifySerializer(
mapper.configure(SerializationFeature.WRITE_ENUMS_USING_TO_STRING, true);
mapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
mapper.configure(SerializationFeature.WRITE_NULL_MAP_VALUES, false);
mapper.configure(SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS, true);
mapper.setSerializationInclusion(JsonInclude.Include.NON_NULL);

return mapper;
Expand All @@ -150,6 +151,7 @@ public static ObjectMapper buildStrictGenericObjectMapper() {
mapper.configure(SerializationFeature.WRITE_ENUMS_USING_TO_STRING, true);
mapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
mapper.configure(SerializationFeature.WRITE_NULL_MAP_VALUES, false);
mapper.configure(SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS, true);
try {
mapper.configure(DeserializationFeature.valueOf("FAIL_ON_TRAILING_TOKENS"), true);
} catch (Throwable e) {
Expand Down
Expand Up @@ -16,6 +16,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashSet;
import java.util.List;
import java.util.Optional;
Expand Down Expand Up @@ -223,6 +224,9 @@ public static boolean isConstructorCompatible(Constructor<?> constructor) {
* excluding <code>Object</code> class. If the field from child class hides the field from superclass,
* the field from superclass won't be added to the result list.
*
* The list is sorted by name to make the output of this method deterministic.
* See https://docs.oracle.com/javase/8/docs/api/java/lang/Class.html#getFields--
*
* @param cls is the processing class
* @return list of Fields
*/
Expand All @@ -241,6 +245,10 @@ public static List<Field> getDeclaredFields(Class<?> cls) {
fields.add(field);
}
}

// Make sure the order is deterministic
fields.sort(Comparator.comparing(Field::getName));

return fields;
}

Expand Down
Expand Up @@ -195,7 +195,7 @@ public void deserializeModelWithObjectExample() throws IOException {
"}";

final Schema model = Json.mapper().readValue(json, Schema.class);
assertEquals(Json.mapper().writeValueAsString(model.getExample()), "{\"code\":1,\"message\":\"hello\",\"fields\":\"abc\"}");
assertEquals(Json.mapper().writeValueAsString(model.getExample()), "{\"code\":1,\"fields\":\"abc\",\"message\":\"hello\"}");
}

@Test(description = "it should deserialize a model with read-only property")
Expand Down Expand Up @@ -367,4 +367,4 @@ public void testEnumWithNull() throws Exception {
SerializationMatchers.assertEqualsToYaml(model, yaml);

}
}
}
Expand Up @@ -3,6 +3,7 @@
import io.swagger.v3.core.util.ReflectionUtils;
import io.swagger.v3.core.util.reflection.resources.Child;
import io.swagger.v3.core.util.reflection.resources.IParent;
import io.swagger.v3.core.util.reflection.resources.ObjectWithManyFields;
import io.swagger.v3.core.util.reflection.resources.Parent;
import io.swagger.v3.oas.annotations.media.Schema;
import io.swagger.v3.oas.annotations.responses.ApiResponse;
Expand All @@ -11,10 +12,13 @@
import org.testng.annotations.Test;

import javax.ws.rs.Path;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.lang.reflect.Type;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.stream.Collectors;

import static org.testng.Assert.assertNull;

Expand Down Expand Up @@ -134,6 +138,14 @@ public void getDeclaredFieldsFromInterfaceTest() throws NoSuchMethodException {
Assert.assertEquals(Collections.emptyList(), ReflectionUtils.getDeclaredFields(cls));
}

@Test
public void declaredFieldsShouldBeSorted() {
final Class cls = ObjectWithManyFields.class;
final List<Field> declaredFields = ReflectionUtils.getDeclaredFields(cls);
Assert.assertEquals(4, declaredFields.size());
Assert.assertEquals(Arrays.asList("a", "b", "c", "d"), declaredFields.stream().map(Field::getName).collect(Collectors.toList()));
}

@Test
public void testFindMethodForNullClass() throws Exception {
Method method = ReflectionUtilsTest.class.getMethod("testFindMethodForNullClass", (Class<?>[]) null);
Expand Down
@@ -0,0 +1,10 @@
package io.swagger.v3.core.util.reflection.resources;

public class ObjectWithManyFields {

public String a;
public boolean d;
public Integer c;
public Object b;

}
Expand Up @@ -58,6 +58,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
Expand All @@ -68,6 +69,8 @@
import java.util.Optional;
import java.util.Set;
import java.util.TreeSet;
import java.util.stream.Collectors;
import java.util.stream.Stream;

public class Reader implements OpenApiReader {
private static final Logger LOGGER = LoggerFactory.getLogger(Reader.class);
Expand Down Expand Up @@ -373,8 +376,13 @@ public OpenAPI read(Class<?> cls,
// look for field-level annotated properties
globalParameters.addAll(ReaderUtils.collectFieldParameters(cls, components, classConsumes, null));

// Make sure that the class methods are sorted for deterministic order
// See https://docs.oracle.com/javase/8/docs/api/java/lang/Class.html#getMethods--
final List<Method> methods = Arrays.stream(cls.getMethods())
.sorted(new MethodComparator())
.collect(Collectors.toList());

// iterate class methods
Method[] methods = cls.getMethods();
for (Method method : methods) {
if (isOperationHidden(method)) {
continue;
Expand Down Expand Up @@ -1471,4 +1479,36 @@ private static Class<?> getClassArgument(Type cls) {
return null;
}
}

/**
* Comparator for uniquely sorting a collection of Method objects.
* Supports overloaded methods (with the same name).
*
* @see Method
*/
private static class MethodComparator implements Comparator<Method> {

@Override
public int compare(Method m1, Method m2) {
// First compare the names of the method
int val = m1.getName().compareTo(m2.getName());

// If the names are equal, compare each argument type
if (val == 0) {
val = m1.getParameterTypes().length - m2.getParameterTypes().length;
if (val == 0) {
Class<?>[] types1 = m1.getParameterTypes();
Class<?>[] types2 = m2.getParameterTypes();
for (int i = 0; i < types1.length; i++) {
val = types1[i].getName().compareTo(types2[i].getName());

if (val != 0) {
break;
}
}
}
}
return val;
}
}
}
Expand Up @@ -2214,37 +2214,37 @@ public void testTicket3587() {
Reader reader = new Reader(new OpenAPI());

OpenAPI openAPI = reader.read(Ticket3587Resource.class);
String yaml = "openapi: 3.0.1\n"
+ "paths:\n"
+ " /test/test:\n"
+ " get:\n"
+ " operationId: parameterExamplesOrderingTest\n"
+ " parameters:\n"
+ " - in: query\n"
+ " schema:\n"
+ " type: string\n"
+ " examples:\n"
+ " Example One:\n"
+ " description: Example One\n"
+ " Example Two:\n"
+ " description: Example Two\n"
+ " Example Three:\n"
+ " description: Example Three\n"
+ " - in: query\n"
+ " schema:\n"
+ " type: string\n"
+ " examples:\n"
+ " Example Three:\n"
+ " description: Example Three\n"
+ " Example Two:\n"
+ " description: Example Two\n"
+ " Example One:\n"
+ " description: Example One\n"
+ " responses:\n"
+ " default:\n"
+ " description: default response\n"
+ " content:\n"
+ " '*/*': {}";
String yaml = "openapi: 3.0.1\n" +
"paths:\n" +
" /test/test:\n" +
" get:\n" +
" operationId: parameterExamplesOrderingTest\n" +
" parameters:\n" +
" - in: query\n" +
" schema:\n" +
" type: string\n" +
" examples:\n" +
" Example One:\n" +
" description: Example One\n" +
" Example Three:\n" +
" description: Example Three\n" +
" Example Two:\n" +
" description: Example Two\n" +
" - in: query\n" +
" schema:\n" +
" type: string\n" +
" examples:\n" +
" Example One:\n" +
" description: Example One\n" +
" Example Three:\n" +
" description: Example Three\n" +
" Example Two:\n" +
" description: Example Two\n" +
" responses:\n" +
" default:\n" +
" description: default response\n" +
" content:\n" +
" '*/*': {}\n";
SerializationMatchers.assertEqualsToYamlExact(openAPI, yaml);
}

Expand Down
Expand Up @@ -17,6 +17,10 @@
import javax.ws.rs.POST;
import javax.ws.rs.Path;

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Paths;

import static org.testng.Assert.assertEquals;

public class ExamplesTest extends AbstractAnnotationTest {
Expand Down Expand Up @@ -416,17 +420,15 @@ public void testFullExample() {
" User:\n" +
" type: object\n" +
" properties:\n" +
" id:\n" +
" type: integer\n" +
" format: int64\n" +
" username:\n" +
" email:\n" +
" type: string\n" +
" firstName:\n" +
" type: string\n" +
" id:\n" +
" type: integer\n" +
" format: int64\n" +
" lastName:\n" +
" type: string\n" +
" email:\n" +
" type: string\n" +
" password:\n" +
" type: string\n" +
" phone:\n" +
Expand All @@ -435,6 +437,8 @@ public void testFullExample() {
" type: integer\n" +
" description: User Status\n" +
" format: int32\n" +
" username:\n" +
" type: string\n" +
" xml:\n" +
" name: User";
assertEquals(extractedYAML, expectedYAML);
Expand Down
Expand Up @@ -373,16 +373,16 @@ public void testOperationWithResponseMultipleHeaders() {
" \"200\":\n" +
" description: voila!\n" +
" headers:\n" +
" X-Rate-Limit-Desc:\n" +
" description: The description of rate limit\n" +
" style: simple\n" +
" schema:\n" +
" type: string\n" +
" Rate-Limit-Limit:\n" +
" description: The number of allowed requests in the current period\n" +
" style: simple\n" +
" schema:\n" +
" type: integer\n" +
" X-Rate-Limit-Desc:\n" +
" description: The description of rate limit\n" +
" style: simple\n" +
" schema:\n" +
" type: string\n" +
" deprecated: true\n";
assertEquals(expectedYAML, extractedYAML);
}
Expand Down
Expand Up @@ -19,4 +19,4 @@ public Response test(
@Parameter(required = true) ModelWithJsonIdentityCyclic model) {
return Response.ok().entity("SUCCESS").build();
}
}
}