Merge manual item meta-data

Previously, manual meta-data were added to the existing set of entries
which could lead to duplicates if a manual entry is meant to complement
a property that is detected via the processor.

We now match the name and type of the item against the auto-detected
entries. If no match is found, we add the extra entry as we did before.
If a match is found we override the description, default value and
deprecation information.

Closes gh-3562

spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessor.java

@@ -374,7 +374,7 @@ public class ConfigurationMetadataAnnotationProcessor extends AbstractProcessor
 	private ConfigurationMetadata mergeAdditionalMetadata(ConfigurationMetadata metadata) {
 		try {
 			ConfigurationMetadata merged = new ConfigurationMetadata(metadata);
-			merged.addAll(this.metadataStore.readAdditionalMetadata());
+			merged.merge(this.metadataStore.readAdditionalMetadata());
 			return merged;
 		}
 		catch (FileNotFoundException ex) {

spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/metadata/ConfigurationMetadata.java

@@ -19,9 +19,15 @@ package org.springframework.boot.configurationprocessor.metadata;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.ListIterator;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import org.springframework.util.CollectionUtils;
+import org.springframework.util.LinkedMultiValueMap;
+import org.springframework.util.MultiValueMap;
+import org.springframework.util.ObjectUtils;
+
 /**
  * Configuration meta-data.
  *
@@ -34,18 +40,18 @@ public class ConfigurationMetadata {
 
 	private static final Pattern CAMEL_CASE_PATTERN = Pattern.compile("([^A-Z-])([A-Z])");
 
-	private final List<ItemMetadata> items;
+	private final MultiValueMap<String, ItemMetadata> items;
 
-	private final List<ItemHint> hints;
+	private final MultiValueMap<String, ItemHint> hints;
 
 	public ConfigurationMetadata() {
-		this.items = new ArrayList<ItemMetadata>();
-		this.hints = new ArrayList<ItemHint>();
+		this.items = new LinkedMultiValueMap<String, ItemMetadata>();
+		this.hints = new LinkedMultiValueMap<String, ItemHint>();
 	}
 
 	public ConfigurationMetadata(ConfigurationMetadata metadata) {
-		this.items = new ArrayList<ItemMetadata>(metadata.getItems());
-		this.hints = new ArrayList<ItemHint>(metadata.getHints());
+		this.items = new LinkedMultiValueMap<String, ItemMetadata>(metadata.items);
+		this.hints = new LinkedMultiValueMap<String, ItemHint>(metadata.hints);
 	}
 
 	/**
@@ -53,40 +59,97 @@ public class ConfigurationMetadata {
 	 * @param itemMetadata the meta-data to add
 	 */
 	public void add(ItemMetadata itemMetadata) {
-		this.items.add(itemMetadata);
-		Collections.sort(this.items);
-	}
-
-	public void add(ItemHint itemHint) {
-		this.hints.add(itemHint);
-		Collections.sort(this.hints);
+		this.items.add(itemMetadata.getName(), itemMetadata);
 	}
 
 	/**
-	 * Add all properties from another {@link ConfigurationMetadata}.
+	 * Add item hint.
+	 * @param itemHint the item hint to add
+	 */
+	public void add(ItemHint itemHint) {
+		this.hints.add(itemHint.getName(), itemHint);
+	}
+
+	/**
+	 * Merge the content from another {@link ConfigurationMetadata}.
 	 * @param metadata the {@link ConfigurationMetadata} instance to merge
 	 */
-	public void addAll(ConfigurationMetadata metadata) {
-		this.items.addAll(metadata.getItems());
-		Collections.sort(this.items);
-		this.hints.addAll(metadata.getHints());
-		Collections.sort(this.hints);
+	public void merge(ConfigurationMetadata metadata) {
+		for (ItemMetadata additionalItem : metadata.getItems()) {
+			mergeItemMetadata(additionalItem);
+		}
+		for (ItemHint itemHint : metadata.getHints()) {
+			add(itemHint);
+		}
 	}
 
 	/**
 	 * @return the meta-data properties.
 	 */
 	public List<ItemMetadata> getItems() {
-		return Collections.unmodifiableList(this.items);
+		return flattenValues(this.items);
 	}
 
 	/**
 	 * @return the meta-data hints.
 	 */
 	public List<ItemHint> getHints() {
-		return Collections.unmodifiableList(this.hints);
+		return flattenValues(this.hints);
 	}
 
+	protected void mergeItemMetadata(ItemMetadata metadata) {
+		ItemMetadata matching = findMatchingItemMetadata(metadata);
+		if (matching != null) {
+			if (metadata.getDescription() != null) {
+				matching.setDescription(metadata.getDescription());
+			}
+			if (metadata.getDefaultValue() != null) {
+				matching.setDefaultValue(metadata.getDefaultValue());
+			}
+			ItemDeprecation deprecation = metadata.getDeprecation();
+			ItemDeprecation matchingDeprecation = matching.getDeprecation();
+			if (deprecation != null) {
+				if (matchingDeprecation == null) {
+					matching.setDeprecation(deprecation);
+				}
+				else {
+					if (deprecation.getReason() != null) {
+						matchingDeprecation.setReason(deprecation.getReason());
+					}
+					if (deprecation.getReplacement() != null) {
+						matchingDeprecation.setReplacement(deprecation.getReplacement());
+					}
+				}
+			}
+		}
+		else {
+			this.items.add(metadata.getName(), metadata);
+		}
+	}
+
+	private ItemMetadata findMatchingItemMetadata(ItemMetadata metadata) {
+		List<ItemMetadata> candidates = this.items.get(metadata.getName());
+		if (CollectionUtils.isEmpty(candidates)) {
+			return null;
+		}
+		ListIterator<ItemMetadata> it = candidates.listIterator();
+		while (it.hasNext()) {
+			if (!it.next().hasSameType(metadata)) {
+				it.remove();
+			}
+		}
+		if (candidates.size() == 1)  {
+			return candidates.get(0);
+		}
+		for (ItemMetadata candidate : candidates) {
+			if (ObjectUtils.nullSafeEquals(candidate.getSourceType(), metadata.getSourceType())) {
+				return candidate;
+			}
+		}
+		return null;
+	}
+
+
 	public static String nestedPrefix(String prefix, String name) {
 		String nestedPrefix = (prefix == null ? "" : prefix);
 		String dashedName = toDashedCase(name);
@@ -114,4 +177,13 @@ public class ConfigurationMetadata {
 		return first + "-" + second;
 	}
 
+	private static <T extends Comparable> List<T> flattenValues(MultiValueMap<?, T> map) {
+		List<T> content = new ArrayList<T>();
+		for (List<T> values : map.values()) {
+			content.addAll(values);
+		}
+		Collections.sort(content);
+		return content;
+	}
+
 }

spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/metadata/ItemMetadata.java

@@ -26,21 +26,21 @@ package org.springframework.boot.configurationprocessor.metadata;
  */
 public class ItemMetadata implements Comparable<ItemMetadata> {
 
-	private final ItemType itemType;
+	private ItemType itemType;
 
-	private final String name;
+	private String name;
 
-	private final String type;
+	private String type;
 
-	private final String description;
+	private String description;
 
-	private final String sourceType;
+	private String sourceType;
 
-	private final String sourceMethod;
+	private String sourceMethod;
 
-	private final Object defaultValue;
+	private Object defaultValue;
 
-	private final ItemDeprecation deprecation;
+	private ItemDeprecation deprecation;
 
 	ItemMetadata(ItemType itemType, String prefix, String name, String type,
 			String sourceType, String sourceMethod, String description,
@@ -72,34 +72,66 @@ public class ItemMetadata implements Comparable<ItemMetadata> {
 		return this.itemType == itemType;
 	}
 
+	public boolean hasSameType(ItemMetadata metadata) {
+		return this.itemType == metadata.itemType;
+	}
+
 	public String getName() {
 		return this.name;
 	}
 
+	public void setName(String name) {
+		this.name = name;
+	}
+
 	public String getType() {
 		return this.type;
 	}
 
-	public String getSourceType() {
-		return this.sourceType;
-	}
-
-	public String getSourceMethod() {
-		return this.sourceMethod;
+	public void setType(String type) {
+		this.type = type;
 	}
 
 	public String getDescription() {
 		return this.description;
 	}
 
+	public void setDescription(String description) {
+		this.description = description;
+	}
+
+	public String getSourceType() {
+		return this.sourceType;
+	}
+
+	public void setSourceType(String sourceType) {
+		this.sourceType = sourceType;
+	}
+
+	public String getSourceMethod() {
+		return this.sourceMethod;
+	}
+
+	public void setSourceMethod(String sourceMethod) {
+		this.sourceMethod = sourceMethod;
+	}
+
 	public Object getDefaultValue() {
 		return this.defaultValue;
 	}
 
+	public void setDefaultValue(Object defaultValue) {
+		this.defaultValue = defaultValue;
+	}
+
 	public ItemDeprecation getDeprecation() {
 		return this.deprecation;
 	}
 
+	public void setDeprecation(ItemDeprecation deprecation) {
+		this.deprecation = deprecation;
+	}
+
 	@Override
 	public String toString() {
 		StringBuilder string = new StringBuilder(this.name);

spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessorTests.java

@@ -348,16 +348,9 @@ public class ConfigurationMetadataAnnotationProcessorTests {
 
 	@Test
 	public void mergingOfAdditionalProperty() throws Exception {
-		File additionalMetadataFile = createAdditionalMetadataFile();
-		JSONObject property = new JSONObject();
-		property.put("name", "foo");
-		property.put("type", "java.lang.String");
-		property.put("sourceType", AdditionalMetadata.class.getName());
-		JSONArray properties = new JSONArray();
-		properties.put(property);
-		JSONObject additionalMetadata = new JSONObject();
-		additionalMetadata.put("properties", properties);
-		writeMetadata(additionalMetadataFile, additionalMetadata);
+		ItemMetadata property = ItemMetadata.newProperty(null, "foo",
+				"java.lang.String", AdditionalMetadata.class.getName(), null, null, null,null);
+		writeAdditionalMetadata(property);
 		ConfigurationMetadata metadata = compile(SimpleProperties.class);
 		assertThat(metadata, containsProperty("simple.comparator"));
 		assertThat(metadata,
@@ -365,6 +358,65 @@ public class ConfigurationMetadataAnnotationProcessorTests {
 						.fromSource(AdditionalMetadata.class));
 	}
 
+	@Test
+	public void mergeExistingPropertyDefaultValue() throws Exception {
+		ItemMetadata property = ItemMetadata.newProperty("simple", "flag", null,
+				null, null, null, true, null);
+		writeAdditionalMetadata(property);
+		ConfigurationMetadata metadata = compile(SimpleProperties.class);
+		assertThat(
+				metadata,
+				containsProperty("simple.flag", Boolean.class)
+						.fromSource(SimpleProperties.class)
+						.withDescription("A simple flag.")
+						.withDefaultValue(is(true)));
+		assertThat(metadata.getItems().size(), is(4));
+	}
+
+	@Test
+	public void mergeExistingPropertyDescription() throws Exception {
+		ItemMetadata property = ItemMetadata.newProperty("simple", "comparator", null,
+				null, null, "A nice comparator.", null, null);
+		writeAdditionalMetadata(property);
+		ConfigurationMetadata metadata = compile(SimpleProperties.class);
+		assertThat(
+				metadata,
+				containsProperty("simple.comparator", "java.util.Comparator<?>")
+						.fromSource(SimpleProperties.class)
+						.withDescription("A nice comparator."));
+		assertThat(metadata.getItems().size(), is(4));
+	}
+
+	@Test
+	public void mergeExistingPropertyDeprecation() throws Exception {
+		ItemMetadata property = ItemMetadata.newProperty("simple", "comparator", null,
+				null, null, null, null, new ItemDeprecation("Don't use this.",
+						"simple.complex-comparator"));
+		writeAdditionalMetadata(property);
+		ConfigurationMetadata metadata = compile(SimpleProperties.class);
+		assertThat(
+				metadata,
+				containsProperty("simple.comparator", "java.util.Comparator<?>")
+						.fromSource(SimpleProperties.class)
+						.withDeprecation("Don't use this.", "simple.complex-comparator"));
+		assertThat(metadata.getItems().size(), is(4));
+	}
+
+	@Test
+	public void mergeExistingPropertyDeprecationOverride() throws Exception {
+		ItemMetadata property = ItemMetadata.newProperty("singledeprecated", "name", null,
+				null, null, null, null, new ItemDeprecation("Don't use this.",
+						"single.name"));
+		writeAdditionalMetadata(property);
+		ConfigurationMetadata metadata = compile(DeprecatedSingleProperty.class);
+		assertThat(
+				metadata,
+				containsProperty("singledeprecated.name", String.class.getName())
+						.fromSource(DeprecatedSingleProperty.class)
+						.withDeprecation("Don't use this.", "single.name"));
+		assertThat(metadata.getItems().size(), is(3));
+	}
+
 	@Test
 	public void mergeOfInvalidAdditionalMetadata() throws IOException {
 		File additionalMetadataFile = createAdditionalMetadataFile();
@@ -532,6 +584,13 @@ public class ConfigurationMetadataAnnotationProcessorTests {
 		return processor.getMetadata();
 	}
 
+	private void writeAdditionalMetadata(ItemMetadata... metadata) throws IOException {
+		File additionalMetadataFile = createAdditionalMetadataFile();
+		JSONObject additionalMetadata = new JSONObject();
+		additionalMetadata.put("properties", metadata);
+		writeMetadata(additionalMetadataFile, additionalMetadata);
+	}
+
 	private void writeAdditionalHints(ItemHint... hints) throws IOException {
 		File additionalMetadataFile = createAdditionalMetadataFile();
 		JSONObject additionalMetadata = new JSONObject();