From 3e34b0a60ac6f7fa956d4024277857d1f552d34c Mon Sep 17 00:00:00 2001 From: Dmitry Pavlov Date: Sun, 25 Apr 2021 13:00:27 +0300 Subject: [PATCH 1/2] Fix ordering of metadata entries This commit provides a consistent ordering for groups that share the same configuration property prefix. See gh-26230 --- .../metadata/JsonConverter.java | 24 ++++-------- .../metadata/JsonMarshallerTests.java | 38 ++++++++++++++++++- 2 files changed, 44 insertions(+), 18 deletions(-) diff --git a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/metadata/JsonConverter.java b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/metadata/JsonConverter.java index df0f0f0061..3991227020 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/metadata/JsonConverter.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/metadata/JsonConverter.java @@ -160,26 +160,18 @@ class JsonConverter { private static class ItemMetadataComparator implements Comparator { + private final Comparator itemComparator = Comparator.comparing(this::isDeprecated) + .thenComparing(ItemMetadata::getName).thenComparing(ItemMetadata::getSourceType); + + private final Comparator groupComparator = Comparator.comparing(ItemMetadata::getName) + .thenComparing(ItemMetadata::getSourceType); + @Override public int compare(ItemMetadata o1, ItemMetadata o2) { if (o1.isOfItemType(ItemType.GROUP)) { - return compareGroup(o1, o2); - } - return compareProperty(o1, o2); - } - - private int compareGroup(ItemMetadata o1, ItemMetadata o2) { - return o1.getName().compareTo(o2.getName()); - } - - private int compareProperty(ItemMetadata o1, ItemMetadata o2) { - if (isDeprecated(o1) && !isDeprecated(o2)) { - return 1; - } - if (isDeprecated(o2) && !isDeprecated(o1)) { - return -1; + return this.groupComparator.compare(o1, o2); } - return o1.getName().compareTo(o2.getName()); + return this.itemComparator.compare(o1, o2); } private boolean isDeprecated(ItemMetadata item) { diff --git a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/metadata/JsonMarshallerTests.java b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/metadata/JsonMarshallerTests.java index 4f3aa76175..a4baa758fd 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/metadata/JsonMarshallerTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/metadata/JsonMarshallerTests.java @@ -82,7 +82,7 @@ class JsonMarshallerTests { ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); JsonMarshaller marshaller = new JsonMarshaller(); marshaller.write(metadata, outputStream); - String json = new String(outputStream.toByteArray()); + String json = outputStream.toString(); assertThat(json).containsSubsequence("\"groups\"", "\"com.acme.alpha\"", "\"com.acme.bravo\"", "\"properties\"", "\"com.example.alpha.ccc\"", "\"com.example.alpha.ddd\"", "\"com.example.bravo.aaa\"", "\"com.example.bravo.bbb\"", "\"hints\"", "\"eee\"", "\"fff\""); @@ -100,9 +100,43 @@ class JsonMarshallerTests { ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); JsonMarshaller marshaller = new JsonMarshaller(); marshaller.write(metadata, outputStream); - String json = new String(outputStream.toByteArray()); + String json = outputStream.toString(); assertThat(json).containsSubsequence("\"properties\"", "\"com.example.alpha.ddd\"", "\"com.example.bravo.bbb\"", "\"com.example.alpha.ccc\"", "\"com.example.bravo.aaa\""); } + @Test + void orderingForSameGroupNames() throws IOException { + ConfigurationMetadata metadata = new ConfigurationMetadata(); + metadata.add(ItemMetadata.newGroup("com.acme.alpha", null, "com.example.Foo", null)); + metadata.add(ItemMetadata.newGroup("com.acme.alpha", null, "com.example.Bar", null)); + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + JsonMarshaller marshaller = new JsonMarshaller(); + marshaller.write(metadata, outputStream); + String json = outputStream.toString(); + assertThat(json).containsSubsequence("\"groups\"", "\"name\": \"com.acme.alpha\"", + "\"sourceType\": \"com.example.Bar\"", "\"name\": \"com.acme.alpha\"", + "\"sourceType\": \"com.example.Foo\""); + } + + @Test + void orderingForSamePropertyNames() throws IOException { + ConfigurationMetadata metadata = new ConfigurationMetadata(); + metadata.add(ItemMetadata.newProperty("com.example.bravo", "aaa", "java.lang.Boolean", "com.example.Foo", null, + null, null, null)); + metadata.add(ItemMetadata.newProperty("com.example.bravo", "aaa", "java.lang.Integer", "com.example.Bar", null, + null, null, null)); + metadata.add( + ItemMetadata.newProperty("com.example.alpha", "ddd", null, "com.example.Bar", null, null, null, null)); + metadata.add( + ItemMetadata.newProperty("com.example.alpha", "ccc", null, "com.example.Foo", null, null, null, null)); + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + JsonMarshaller marshaller = new JsonMarshaller(); + marshaller.write(metadata, outputStream); + String json = outputStream.toString(); + assertThat(json).containsSubsequence("\"groups\"", "\"properties\"", "\"com.example.alpha.ccc\"", + "com.example.Foo", "\"com.example.alpha.ddd\"", "com.example.Bar", "\"com.example.bravo.aaa\"", + "com.example.Bar", "\"com.example.bravo.aaa\"", "com.example.Foo"); + } + } From d610f3c2ffb51fe539087e971fbcd98c02e0f58c Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Wed, 28 Apr 2021 11:17:17 +0200 Subject: [PATCH 2/2] Polish "Fix ordering of metadata entries" See gh-26230 --- .../metadata/JsonConverter.java | 17 ++++++----- .../metadata/JsonMarshallerTests.java | 30 ++++++++++++++++++- 2 files changed, 38 insertions(+), 9 deletions(-) diff --git a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/metadata/JsonConverter.java b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/metadata/JsonConverter.java index 3991227020..de97b71f32 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/metadata/JsonConverter.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/metadata/JsonConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -160,21 +160,22 @@ class JsonConverter { private static class ItemMetadataComparator implements Comparator { - private final Comparator itemComparator = Comparator.comparing(this::isDeprecated) - .thenComparing(ItemMetadata::getName).thenComparing(ItemMetadata::getSourceType); + private static final Comparator GROUP = Comparator.comparing(ItemMetadata::getName) + .thenComparing(ItemMetadata::getSourceType, Comparator.nullsFirst(Comparator.naturalOrder())); - private final Comparator groupComparator = Comparator.comparing(ItemMetadata::getName) - .thenComparing(ItemMetadata::getSourceType); + private static final Comparator ITEM = Comparator.comparing(ItemMetadataComparator::isDeprecated) + .thenComparing(ItemMetadata::getName) + .thenComparing(ItemMetadata::getSourceType, Comparator.nullsFirst(Comparator.naturalOrder())); @Override public int compare(ItemMetadata o1, ItemMetadata o2) { if (o1.isOfItemType(ItemType.GROUP)) { - return this.groupComparator.compare(o1, o2); + return GROUP.compare(o1, o2); } - return this.itemComparator.compare(o1, o2); + return ITEM.compare(o1, o2); } - private boolean isDeprecated(ItemMetadata item) { + private static boolean isDeprecated(ItemMetadata item) { return item.getDeprecation() != null; } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/metadata/JsonMarshallerTests.java b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/metadata/JsonMarshallerTests.java index a4baa758fd..1f9c8d241e 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/metadata/JsonMarshallerTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/metadata/JsonMarshallerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -139,4 +139,32 @@ class JsonMarshallerTests { "com.example.Bar", "\"com.example.bravo.aaa\"", "com.example.Foo"); } + @Test + void orderingForSameGroupWithNullSourceType() throws IOException { + ConfigurationMetadata metadata = new ConfigurationMetadata(); + metadata.add(ItemMetadata.newGroup("com.acme.alpha", null, "com.example.Foo", null)); + metadata.add(ItemMetadata.newGroup("com.acme.alpha", null, null, null)); + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + JsonMarshaller marshaller = new JsonMarshaller(); + marshaller.write(metadata, outputStream); + String json = outputStream.toString(); + assertThat(json).containsSubsequence("\"groups\"", "\"name\": \"com.acme.alpha\"", + "\"name\": \"com.acme.alpha\"", "\"sourceType\": \"com.example.Foo\""); + } + + @Test + void orderingForSamePropertyNamesWithNullSourceType() throws IOException { + ConfigurationMetadata metadata = new ConfigurationMetadata(); + metadata.add(ItemMetadata.newProperty("com.example.bravo", "aaa", "java.lang.Boolean", null, null, null, null, + null)); + metadata.add(ItemMetadata.newProperty("com.example.bravo", "aaa", "java.lang.Integer", "com.example.Bar", null, + null, null, null)); + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + JsonMarshaller marshaller = new JsonMarshaller(); + marshaller.write(metadata, outputStream); + String json = outputStream.toString(); + assertThat(json).containsSubsequence("\"groups\"", "\"properties\"", "\"com.example.bravo.aaa\"", + "\"java.lang.Boolean\"", "\"com.example.bravo.aaa\"", "\"java.lang.Integer\"", "\"com.example.Bar"); + } + }