Use deterministic order for configuration properties metadata

This commit updates the annotation processor to write metadata in a consistent way. Groups, properties and hints are written and each item is ordered alphabetically based on its name. Also, deprecated items are written last. Closes gh-14347
spring-projects · Sep 8, 2018 · 0493355 · 0493355
1 parent d3ecd02
commit 0493355
Show file tree

Hide file tree

Showing 6 changed files with 118 additions and 61 deletions.
diff --git a/.../src/json-shade/java/org/springframework/boot/configurationprocessor/json/JSONObject.java b/.../src/json-shade/java/org/springframework/boot/configurationprocessor/json/JSONObject.java
@@ -18,8 +18,8 @@
 
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.HashMap;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
 import java.util.Map;
 
 // Note: this class was written without inspecting the non-free org.json source code.
@@ -111,7 +111,7 @@ public String toString() {
 	 * Creates a {@code JSONObject} with no name/value mappings.
 	 */
 	public JSONObject() {
-		this.nameValuePairs = new HashMap<>();
+		this.nameValuePairs = new LinkedHashMap<>();
 	}
 
 	/**

diff --git a/...main/java/org/springframework/boot/configurationprocessor/metadata/JSONOrderedObject.java b/...main/java/org/springframework/boot/configurationprocessor/metadata/JSONOrderedObject.java
diff --git a/...src/main/java/org/springframework/boot/configurationprocessor/metadata/JsonConverter.java b/...src/main/java/org/springframework/boot/configurationprocessor/metadata/JsonConverter.java
@@ -18,7 +18,10 @@
 
 import java.lang.reflect.Array;
 import java.util.Collection;
+import java.util.Comparator;
+import java.util.List;
 import java.util.Map;
+import java.util.stream.Collectors;
 
 import org.springframework.boot.configurationprocessor.json.JSONArray;
 import org.springframework.boot.configurationprocessor.json.JSONObject;
@@ -32,10 +35,15 @@
  */
 class JsonConverter {
 
+	private static final ItemMetadataComparator ITEM_COMPARATOR = new ItemMetadataComparator();
+
 	public JSONArray toJsonArray(ConfigurationMetadata metadata, ItemType itemType)
 			throws Exception {
 		JSONArray jsonArray = new JSONArray();
-		for (ItemMetadata item : metadata.getItems()) {
+		List<ItemMetadata> items = metadata.getItems().stream()
+				.filter((item) -> item.isOfItemType(itemType)).sorted(ITEM_COMPARATOR)
+				.collect(Collectors.toList());
+		for (ItemMetadata item : items) {
 			if (item.isOfItemType(itemType)) {
 				jsonArray.put(toJsonObject(item));
 			}
@@ -52,7 +60,7 @@ public JSONArray toJsonArray(Collection<ItemHint> hints) throws Exception {
 	}
 
 	public JSONObject toJsonObject(ItemMetadata item) throws Exception {
-		JSONObject jsonObject = new JSONOrderedObject();
+		JSONObject jsonObject = new JSONObject();
 		jsonObject.put("name", item.getName());
 		putIfPresent(jsonObject, "type", item.getType());
 		putIfPresent(jsonObject, "description", item.getDescription());
@@ -81,7 +89,7 @@ public JSONObject toJsonObject(ItemMetadata item) throws Exception {
 	}
 
 	private JSONObject toJsonObject(ItemHint hint) throws Exception {
-		JSONObject jsonObject = new JSONOrderedObject();
+		JSONObject jsonObject = new JSONObject();
 		jsonObject.put("name", hint.getName());
 		if (!hint.getValues().isEmpty()) {
 			jsonObject.put("values", getItemHintValues(hint));
@@ -101,7 +109,7 @@ private JSONArray getItemHintValues(ItemHint hint) throws Exception {
 	}
 
 	private JSONObject getItemHintValue(ItemHint.ValueHint value) throws Exception {
-		JSONObject result = new JSONOrderedObject();
+		JSONObject result = new JSONObject();
 		putHintValue(result, value.getValue());
 		putIfPresent(result, "description", value.getDescription());
 		return result;
@@ -117,10 +125,10 @@ private JSONArray getItemHintProviders(ItemHint hint) throws Exception {
 
 	private JSONObject getItemHintProvider(ItemHint.ValueProvider provider)
 			throws Exception {
-		JSONObject result = new JSONOrderedObject();
+		JSONObject result = new JSONObject();
 		result.put("name", provider.getName());
 		if (provider.getParameters() != null && !provider.getParameters().isEmpty()) {
-			JSONObject parameters = new JSONOrderedObject();
+			JSONObject parameters = new JSONObject();
 			for (Map.Entry<String, Object> entry : provider.getParameters().entrySet()) {
 				parameters.put(entry.getKey(), extractItemValue(entry.getValue()));
 			}
@@ -160,4 +168,34 @@ private Object extractItemValue(Object value) {
 		return defaultValue;
 	}
 
+	private static class ItemMetadataComparator implements Comparator<ItemMetadata> {
+
+		@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 o1.getName().compareTo(o2.getName());
+		}
+
+		private boolean isDeprecated(ItemMetadata item) {
+			return item.getDeprecation() != null;
+		}
+
+	}
+
 }
diff --git a/...rc/main/java/org/springframework/boot/configurationprocessor/metadata/JsonMarshaller.java b/...rc/main/java/org/springframework/boot/configurationprocessor/metadata/JsonMarshaller.java
@@ -45,7 +45,7 @@ public class JsonMarshaller {
 	public void write(ConfigurationMetadata metadata, OutputStream outputStream)
 			throws IOException {
 		try {
-			JSONObject object = new JSONOrderedObject();
+			JSONObject object = new JSONObject();
 			JsonConverter converter = new JsonConverter();
 			object.put("groups", converter.toJsonArray(metadata, ItemType.GROUP));
 			object.put("properties", converter.toJsonArray(metadata, ItemType.PROPERTY));

diff --git a/...gframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessorTests.java b/...gframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessorTests.java
@@ -22,6 +22,8 @@
 import java.time.Duration;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.List;
+import java.util.stream.Collectors;
 
 import org.junit.Before;
 import org.junit.Rule;
@@ -822,9 +824,26 @@ public void mergeExistingPropertyWithSeveralCandidates() throws Exception {
 		ConfigurationMetadata metadata = compile(SimpleProperties.class,
 				SimpleConflictingProperties.class);
 		assertThat(metadata.getItems()).hasSize(6);
-		assertThat(metadata).has(Metadata.withProperty("simple.flag", Boolean.class)
-				.fromSource(SimpleProperties.class).withDescription("A simple flag.")
-				.withDeprecation(null, null).withDefaultValue(true));
+		List<ItemMetadata> items = metadata.getItems().stream()
+				.filter((item) -> item.getName().equals("simple.flag"))
+				.collect(Collectors.toList());
+		assertThat(items).hasSize(2);
+		ItemMetadata matchingProperty = items.stream()
+				.filter((item) -> item.getType().equals(Boolean.class.getName()))
+				.findFirst().orElse(null);
+		assertThat(matchingProperty).isNotNull();
+		assertThat(matchingProperty.getDefaultValue()).isEqualTo(true);
+		assertThat(matchingProperty.getSourceType())
+				.isEqualTo(SimpleProperties.class.getName());
+		assertThat(matchingProperty.getDescription()).isEqualTo("A simple flag.");
+		ItemMetadata nonMatchingProperty = items.stream()
+				.filter((item) -> item.getType().equals(String.class.getName()))
+				.findFirst().orElse(null);
+		assertThat(nonMatchingProperty).isNotNull();
+		assertThat(nonMatchingProperty.getDefaultValue()).isEqualTo("hello");
+		assertThat(nonMatchingProperty.getSourceType())
+				.isEqualTo(SimpleConflictingProperties.class.getName());
+		assertThat(nonMatchingProperty.getDescription()).isNull();
 	}
 
 	@Test

diff --git a/...st/java/org/springframework/boot/configurationprocessor/metadata/JsonMarshallerTests.java b/...st/java/org/springframework/boot/configurationprocessor/metadata/JsonMarshallerTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2017 the original author or authors.
+ * Copyright 2012-2018 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.
@@ -18,6 +18,7 @@
 
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
+import java.io.IOException;
 import java.io.InputStream;
 import java.util.Arrays;
 import java.util.Collections;
@@ -82,4 +83,51 @@ public void marshallAndUnmarshal() throws Exception {
 				.withProvider("second"));
 	}
 
+	@Test
+	public void marshallOrderItems() throws IOException {
+		ConfigurationMetadata metadata = new ConfigurationMetadata();
+		metadata.add(ItemHint.newHint("fff"));
+		metadata.add(ItemHint.newHint("eee"));
+		metadata.add(ItemMetadata.newProperty("com.example.bravo", "bbb", null, null,
+				null, null, null, null));
+		metadata.add(ItemMetadata.newProperty("com.example.bravo", "aaa", null, null,
+				null, null, null, null));
+		metadata.add(ItemMetadata.newProperty("com.example.alpha", "ddd", null, null,
+				null, null, null, null));
+		metadata.add(ItemMetadata.newProperty("com.example.alpha", "ccc", null, null,
+				null, null, null, null));
+		metadata.add(ItemMetadata.newGroup("com.acme.bravo",
+				"com.example.AnotherTestProperties", null, null));
+		metadata.add(ItemMetadata.newGroup("com.acme.alpha", "com.example.TestProperties",
+				null, null));
+		ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
+		JsonMarshaller marshaller = new JsonMarshaller();
+		marshaller.write(metadata, outputStream);
+		String json = new String(outputStream.toByteArray());
+		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\"");
+	}
+
+	@Test
+	public void marshallPutDeprecatedItemsAtTheEnd() throws IOException {
+		ConfigurationMetadata metadata = new ConfigurationMetadata();
+		metadata.add(ItemMetadata.newProperty("com.example.bravo", "bbb", null, null,
+				null, null, null, null));
+		metadata.add(ItemMetadata.newProperty("com.example.bravo", "aaa", null, null,
+				null, null, null, new ItemDeprecation(null, null, "warning")));
+		metadata.add(ItemMetadata.newProperty("com.example.alpha", "ddd", null, null,
+				null, null, null, null));
+		metadata.add(ItemMetadata.newProperty("com.example.alpha", "ccc", null, null,
+				null, null, null, new ItemDeprecation(null, null, "warning")));
+		ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
+		JsonMarshaller marshaller = new JsonMarshaller();
+		marshaller.write(metadata, outputStream);
+		String json = new String(outputStream.toByteArray());
+		assertThat(json).containsSubsequence("\"properties\"",
+				"\"com.example.alpha.ddd\"", "\"com.example.bravo.bbb\"",
+				"\"com.example.alpha.ccc\"", "\"com.example.bravo.aaa\"");
+	}
+
 }