From 35875c7f086f17ac38214bbfe5d6933302bfe3e8 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Thu, 20 Aug 2015 17:08:54 +0200 Subject: [PATCH] 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 --- .../appendix-configuration-metadata.adoc | 16 ++- ...figurationMetadataAnnotationProcessor.java | 2 +- .../metadata/ConfigurationMetadata.java | 114 ++++++++++++++---- .../metadata/ItemMetadata.java | 60 ++++++--- ...ationMetadataAnnotationProcessorTests.java | 79 ++++++++++-- 5 files changed, 220 insertions(+), 51 deletions(-) diff --git a/spring-boot-docs/src/main/asciidoc/appendix-configuration-metadata.adoc b/spring-boot-docs/src/main/asciidoc/appendix-configuration-metadata.adoc index efd77301a4..f2aa6e4507 100644 --- a/spring-boot-docs/src/main/asciidoc/appendix-configuration-metadata.adoc +++ b/spring-boot-docs/src/main/asciidoc/appendix-configuration-metadata.adoc @@ -752,11 +752,17 @@ if it were nested. [[configuration-metadata-additional-metadata]] ==== Adding additional meta-data -Spring Boot's configuration file handling is quite flexible; and it often the case that -properties may exist that are not bound to a `@ConfigurationProperties` bean. To support -such cases and allow you to provide custom "hints", the annotation processor will -automatically merge items from `META-INF/additional-spring-configuration-metadata.json` -into the main meta-data file. +Spring Boot's configuration file handling is quite flexible; and it is often the case +that properties may exist that are not bound to a `@ConfigurationProperties` bean. You +may also need to tune some attributes of an existing key. To support such cases and allow +you to provide custom "hints", the annotation processor will automatically merge items +from `META-INF/additional-spring-configuration-metadata.json` into the main meta-data +file. + +If you refer to a property that has been detected automatically, the description, +default value and deprecation information are overridden if specified. If the manual +property declaration is not identified in the current module, it is added as a brand new +property. The format of the `additional-spring-configuration-metadata.json` file is exactly the same as the regular `spring-configuration-metadata.json`. The additional properties file is diff --git a/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessor.java b/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessor.java index 9c2673bcf3..f15e7b15d1 100644 --- a/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessor.java +++ b/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) { diff --git a/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/metadata/ConfigurationMetadata.java b/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/metadata/ConfigurationMetadata.java index ac87835994..899baa14be 100644 --- a/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/metadata/ConfigurationMetadata.java +++ b/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 items; + private final MultiValueMap items; - private final List hints; + private final MultiValueMap hints; public ConfigurationMetadata() { - this.items = new ArrayList(); - this.hints = new ArrayList(); + this.items = new LinkedMultiValueMap(); + this.hints = new LinkedMultiValueMap(); } public ConfigurationMetadata(ConfigurationMetadata metadata) { - this.items = new ArrayList(metadata.getItems()); - this.hints = new ArrayList(metadata.getHints()); + this.items = new LinkedMultiValueMap(metadata.items); + this.hints = new LinkedMultiValueMap(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 getItems() { - return Collections.unmodifiableList(this.items); + return flattenValues(this.items); } /** * @return the meta-data hints. */ public List 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 candidates = this.items.get(metadata.getName()); + if (CollectionUtils.isEmpty(candidates)) { + return null; + } + ListIterator 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 List flattenValues(MultiValueMap map) { + List content = new ArrayList(); + for (List values : map.values()) { + content.addAll(values); + } + Collections.sort(content); + return content; + } + } diff --git a/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/metadata/ItemMetadata.java b/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/metadata/ItemMetadata.java index f8e852beee..08fd9f734d 100644 --- a/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/metadata/ItemMetadata.java +++ b/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 { - 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 { 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); diff --git a/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessorTests.java b/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessorTests.java index f5955d5031..739c303137 100644 --- a/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessorTests.java +++ b/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();