From e99deb95f7669f298cd3a879c926b50db6552993 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Mon, 15 Apr 2019 16:17:11 +0200 Subject: [PATCH] Fix source detection in case of multiple candidates This commit improves the detection of a property source when more than one group with the same type exist. Closes gh-16549 --- ...gurationMetadataRepositoryJsonBuilder.java | 12 +---- .../RawConfigurationMetadata.java | 23 +++++----- ...ionMetadataRepositoryJsonBuilderTests.java | 27 ++++++++++- .../JsonReaderTests.java | 21 ++++++++- .../configuration-metadata-multi-groups.json | 45 +++++++++++++++++++ 5 files changed, 105 insertions(+), 23 deletions(-) create mode 100644 spring-boot-project/spring-boot-tools/spring-boot-configuration-metadata/src/test/resources/metadata/configuration-metadata-multi-groups.json diff --git a/spring-boot-project/spring-boot-tools/spring-boot-configuration-metadata/src/main/java/org/springframework/boot/configurationmetadata/ConfigurationMetadataRepositoryJsonBuilder.java b/spring-boot-project/spring-boot-tools/spring-boot-configuration-metadata/src/main/java/org/springframework/boot/configurationmetadata/ConfigurationMetadataRepositoryJsonBuilder.java index 8912b0612e..9560a30825 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-configuration-metadata/src/main/java/org/springframework/boot/configurationmetadata/ConfigurationMetadataRepositoryJsonBuilder.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-configuration-metadata/src/main/java/org/springframework/boot/configurationmetadata/ConfigurationMetadataRepositoryJsonBuilder.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2018 the original author or authors. + * Copyright 2012-2019 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. @@ -108,7 +108,7 @@ public final class ConfigurationMetadataRepositoryJsonBuilder { SimpleConfigurationMetadataRepository repository = new SimpleConfigurationMetadataRepository(); repository.add(metadata.getSources()); for (ConfigurationMetadataItem item : metadata.getItems()) { - ConfigurationMetadataSource source = getSource(metadata, item); + ConfigurationMetadataSource source = metadata.getSource(item); repository.add(item, source); } Map allProperties = repository @@ -146,14 +146,6 @@ public final class ConfigurationMetadataRepositoryJsonBuilder { property.getHints().getKeyProviders().addAll(hint.getValueProviders()); } - private ConfigurationMetadataSource getSource(RawConfigurationMetadata metadata, - ConfigurationMetadataItem item) { - if (item.getSourceType() != null) { - return metadata.getSource(item.getSourceType()); - } - return null; - } - /** * Create a new builder instance using {@link StandardCharsets#UTF_8} as the default * charset and the specified json resource. diff --git a/spring-boot-project/spring-boot-tools/spring-boot-configuration-metadata/src/main/java/org/springframework/boot/configurationmetadata/RawConfigurationMetadata.java b/spring-boot-project/spring-boot-tools/spring-boot-configuration-metadata/src/main/java/org/springframework/boot/configurationmetadata/RawConfigurationMetadata.java index 0cab69d8d6..8317a09bab 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-configuration-metadata/src/main/java/org/springframework/boot/configurationmetadata/RawConfigurationMetadata.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-configuration-metadata/src/main/java/org/springframework/boot/configurationmetadata/RawConfigurationMetadata.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2019 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. @@ -17,6 +17,7 @@ package org.springframework.boot.configurationmetadata; import java.util.ArrayList; +import java.util.Comparator; import java.util.List; /** @@ -48,13 +49,16 @@ class RawConfigurationMetadata { return this.sources; } - public ConfigurationMetadataSource getSource(String type) { - for (ConfigurationMetadataSource source : this.sources) { - if (type.equals(source.getType())) { - return source; - } + public ConfigurationMetadataSource getSource(ConfigurationMetadataItem item) { + if (item.getSourceType() == null) { + return null; } - return null; + return this.sources.stream() + .filter((candidate) -> item.getSourceType().equals(candidate.getType()) + && item.getId().startsWith(candidate.getGroupId())) + .max(Comparator + .comparingInt((candidate) -> candidate.getGroupId().length())) + .orElse(null); } public List getItems() { @@ -72,10 +76,7 @@ class RawConfigurationMetadata { */ private void resolveName(ConfigurationMetadataItem item) { item.setName(item.getId()); // fallback - if (item.getSourceType() == null) { - return; - } - ConfigurationMetadataSource source = getSource(item.getSourceType()); + ConfigurationMetadataSource source = getSource(item); if (source != null) { String groupId = source.getGroupId(); String dottedPrefix = groupId + "."; diff --git a/spring-boot-project/spring-boot-tools/spring-boot-configuration-metadata/src/test/java/org/springframework/boot/configurationmetadata/ConfigurationMetadataRepositoryJsonBuilderTests.java b/spring-boot-project/spring-boot-tools/spring-boot-configuration-metadata/src/test/java/org/springframework/boot/configurationmetadata/ConfigurationMetadataRepositoryJsonBuilderTests.java index c98984a401..c53298f093 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-configuration-metadata/src/test/java/org/springframework/boot/configurationmetadata/ConfigurationMetadataRepositoryJsonBuilderTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-configuration-metadata/src/test/java/org/springframework/boot/configurationmetadata/ConfigurationMetadataRepositoryJsonBuilderTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2018 the original author or authors. + * Copyright 2012-2019 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. @@ -129,6 +129,31 @@ public class ConfigurationMetadataRepositoryJsonBuilderTests } } + @Test + public void multiGroups() throws IOException { + try (InputStream in = getInputStreamFor("multi-groups")) { + ConfigurationMetadataRepository repo = ConfigurationMetadataRepositoryJsonBuilder + .create(in).build(); + assertThat(repo.getAllGroups()).containsOnlyKeys("test.group.one.retry", + "test.group.two.retry", "test.group.one.retry.specific"); + ConfigurationMetadataGroup one = repo.getAllGroups() + .get("test.group.one.retry"); + assertThat(one.getSources()).containsOnlyKeys("com.example.Retry"); + assertThat(one.getProperties()) + .containsOnlyKeys("test.group.one.retry.enabled"); + ConfigurationMetadataGroup two = repo.getAllGroups() + .get("test.group.two.retry"); + assertThat(two.getSources()).containsOnlyKeys("com.example.Retry"); + assertThat(two.getProperties()) + .containsOnlyKeys("test.group.two.retry.enabled"); + ConfigurationMetadataGroup oneSpecific = repo.getAllGroups() + .get("test.group.one.retry.specific"); + assertThat(oneSpecific.getSources()).containsOnlyKeys("com.example.Retry"); + assertThat(oneSpecific.getProperties()) + .containsOnlyKeys("test.group.one.retry.specific.enabled"); + } + } + @Test public void builderInstancesAreIsolated() throws IOException { try (InputStream foo = getInputStreamFor("foo"); diff --git a/spring-boot-project/spring-boot-tools/spring-boot-configuration-metadata/src/test/java/org/springframework/boot/configurationmetadata/JsonReaderTests.java b/spring-boot-project/spring-boot-tools/spring-boot-configuration-metadata/src/test/java/org/springframework/boot/configurationmetadata/JsonReaderTests.java index 7deb8a331f..7d9343f76f 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-configuration-metadata/src/test/java/org/springframework/boot/configurationmetadata/JsonReaderTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-configuration-metadata/src/test/java/org/springframework/boot/configurationmetadata/JsonReaderTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2018 the original author or authors. + * Copyright 2012-2019 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. @@ -199,6 +199,25 @@ public class JsonReaderTests extends AbstractConfigurationMetadataTests { .isEqualTo(Deprecation.Level.WARNING); } + @Test + public void multiGroupsMetadata() throws IOException { + RawConfigurationMetadata rawMetadata = readFor("multi-groups"); + List items = rawMetadata.getItems(); + assertThat(items).hasSize(3); + + ConfigurationMetadataItem item = items.get(0); + assertThat(item.getName()).isEqualTo("enabled"); + assertThat(item.getSourceType()).isEqualTo("com.example.Retry"); + + ConfigurationMetadataItem item2 = items.get(1); + assertThat(item2.getName()).isEqualTo("enabled"); + assertThat(item2.getSourceType()).isEqualTo("com.example.Retry"); + + ConfigurationMetadataItem item3 = items.get(2); + assertThat(item3.getName()).isEqualTo("enabled"); + assertThat(item3.getSourceType()).isEqualTo("com.example.Retry"); + } + RawConfigurationMetadata readFor(String path) throws IOException { return this.reader.read(getInputStreamFor(path), DEFAULT_CHARSET); } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-configuration-metadata/src/test/resources/metadata/configuration-metadata-multi-groups.json b/spring-boot-project/spring-boot-tools/spring-boot-configuration-metadata/src/test/resources/metadata/configuration-metadata-multi-groups.json new file mode 100644 index 0000000000..61c3f971a3 --- /dev/null +++ b/spring-boot-project/spring-boot-tools/spring-boot-configuration-metadata/src/test/resources/metadata/configuration-metadata-multi-groups.json @@ -0,0 +1,45 @@ +{ + "groups": [ + { + "name": "test.group.one.retry", + "type": "com.example.Retry", + "sourceType": "org.acme.config.TestApp", + "sourceMethod": "one()" + }, + { + "name": "test.group.two.retry", + "type": "com.example.Retry", + "sourceType": "org.acme.config.TestApp", + "sourceMethod": "two()" + }, + { + "name": "test.group.one.retry.specific", + "type": "com.example.Retry", + "sourceType": "org.acme.config.TestApp", + "sourceMethod": "two()" + } + ], + "properties": [ + { + "name": "test.group.one.retry.enabled", + "type": "java.lang.Boolean", + "description": "Whether publishing retries are enabled.", + "sourceType": "com.example.Retry", + "defaultValue": false + }, + { + "name": "test.group.two.retry.enabled", + "type": "java.lang.Boolean", + "description": "Whether publishing retries are enabled.", + "sourceType": "com.example.Retry", + "defaultValue": false + }, + { + "name": "test.group.one.retry.specific.enabled", + "type": "java.lang.Boolean", + "description": "Whether publishing retries are enabled.", + "sourceType": "com.example.Retry", + "defaultValue": false + } + ] +} \ No newline at end of file