-
Notifications
You must be signed in to change notification settings - Fork 41.2k
Log a fail/warn when config metadata default values are unable to be resolved #28062
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
b501f1f
ab95c45
19e4701
b54a4e1
0cf66c9
97dbe4c
1934985
11df4cc
91c3828
6eb73c8
2c0466d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -250,6 +250,10 @@ | |
"level": "error" | ||
} | ||
}, | ||
{ | ||
"name": "server.undertow.accesslog.dir", | ||
"defaultValue": "logs" | ||
}, | ||
{ | ||
"name": "server.undertow.buffers-per-region", | ||
"type": "java.lang.Integer", | ||
|
@@ -258,6 +262,14 @@ | |
"level": "error" | ||
} | ||
}, | ||
{ | ||
"name": "server.undertow.max-headers", | ||
"defaultValue": 200 | ||
}, | ||
{ | ||
"name": "server.undertow.max-parameters", | ||
onobc marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"defaultValue": 1000 | ||
}, | ||
{ | ||
"name": "server.use-forward-headers", | ||
"type": "java.lang.Boolean", | ||
|
@@ -336,6 +348,14 @@ | |
"level": "error" | ||
} | ||
}, | ||
{ | ||
"name": "spring.artemis.embedded.cluster-password", | ||
"defaultValue": "UUID.randomUUID().toString()" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, like you said, this is an interesting one. It is a random string value. We could instead do something like |
||
}, | ||
{ | ||
"name": "spring.artemis.embedded.server-id", | ||
"defaultValue": 0 | ||
}, | ||
{ | ||
"name": "spring.artemis.pool.maximum-active-session-per-connection", | ||
"deprecation": { | ||
|
@@ -1088,10 +1108,22 @@ | |
"name": "spring.info.git.location", | ||
"defaultValue": "classpath:git.properties" | ||
}, | ||
{ | ||
"name": "spring.integration.channel.max-broadcast-subscribers", | ||
"defaultValue": 2147483647 | ||
}, | ||
{ | ||
"name": "spring.integration.channel.max-unicast-subscribers", | ||
"defaultValue": 2147483647 | ||
}, | ||
{ | ||
"name": "spring.integration.jdbc.initialize-schema", | ||
"defaultValue": "embedded" | ||
}, | ||
{ | ||
"name": "spring.integration.poller.max-messages-per-poll", | ||
"defaultValue": "-2147483648" | ||
}, | ||
{ | ||
"name": "spring.jackson.constructor-detector", | ||
"defaultValue": "default" | ||
|
@@ -1459,10 +1491,20 @@ | |
"level": "error" | ||
} | ||
}, | ||
{ | ||
"name": "spring.kafka.bootstrap-servers", | ||
"defaultValue": [ | ||
"localhost:9092" | ||
] | ||
}, | ||
{ | ||
"name": "spring.kafka.consumer.isolation-level", | ||
"defaultValue": "read-uncommitted" | ||
}, | ||
{ | ||
"name": "spring.kafka.consumer.key-deserializer", | ||
"defaultValue": "org.apache.kafka.common.serialization.StringDeserializer" | ||
}, | ||
{ | ||
"name": "spring.kafka.consumer.ssl.keystore-location", | ||
"type": "org.springframework.core.io.Resource", | ||
|
@@ -1499,6 +1541,10 @@ | |
"level": "error" | ||
} | ||
}, | ||
{ | ||
"name": "spring.kafka.consumer.value-deserializer", | ||
"defaultValue": "org.apache.kafka.common.serialization.StringDeserializer" | ||
}, | ||
{ | ||
"name": "spring.kafka.jaas.control-flag", | ||
"defaultValue": "required" | ||
|
@@ -1507,6 +1553,10 @@ | |
"name": "spring.kafka.listener.type", | ||
"defaultValue": "single" | ||
}, | ||
{ | ||
"name": "spring.kafka.producer.key-serializer", | ||
"defaultValue": "org.apache.kafka.common.serialization.StringSerializer" | ||
}, | ||
{ | ||
"name": "spring.kafka.producer.ssl.keystore-location", | ||
"type": "org.springframework.core.io.Resource", | ||
|
@@ -1543,6 +1593,10 @@ | |
"level": "error" | ||
} | ||
}, | ||
{ | ||
"name": "spring.kafka.producer.value-serializer", | ||
"defaultValue": "org.apache.kafka.common.serialization.StringSerializer" | ||
}, | ||
{ | ||
"name": "spring.kafka.ssl.keystore-location", | ||
"type": "org.springframework.core.io.Resource", | ||
|
@@ -1736,6 +1790,10 @@ | |
"level": "error" | ||
} | ||
}, | ||
{ | ||
"name": "spring.rabbitmq.stream.port", | ||
"defaultValue": 5552 | ||
}, | ||
{ | ||
"name": "spring.rabbitmq.template.queue", | ||
"type": "java.lang.String", | ||
|
@@ -1967,6 +2025,10 @@ | |
"name": "spring.security.filter.order", | ||
"defaultValue": -100 | ||
}, | ||
{ | ||
"name": "spring.security.user.password", | ||
"defaultValue": "UUID.randomUUID().toString()" | ||
}, | ||
{ | ||
"name": "spring.session.hazelcast.flush-mode", | ||
"defaultValue": "on-save" | ||
|
@@ -2007,6 +2069,10 @@ | |
"request" | ||
] | ||
}, | ||
{ | ||
"name": "spring.session.servlet.filter-order", | ||
"defaultValue": -2147483598 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does not convey the same hint that |
||
}, | ||
{ | ||
"name": "spring.sql.init.enabled", | ||
"type": "java.lang.Boolean", | ||
|
@@ -2021,6 +2087,13 @@ | |
"name": "spring.sql.init.mode", | ||
"defaultValue": "embedded" | ||
}, | ||
{ | ||
"name": "spring.task.execution.pool.max-size", | ||
"defaultValue": 2147483647 | ||
}, { | ||
"name": "spring.task.execution.pool.queue-capacity", | ||
"defaultValue": 2147483647 | ||
}, | ||
{ | ||
"name": "spring.web.locale-resolver", | ||
"defaultValue": "accept-header" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,6 +45,7 @@ | |
import javax.lang.model.util.ElementFilter; | ||
import javax.tools.Diagnostic.Kind; | ||
|
||
import org.springframework.boot.configurationprocessor.fieldvalues.ValueWrapper; | ||
import org.springframework.boot.configurationprocessor.metadata.ConfigurationMetadata; | ||
import org.springframework.boot.configurationprocessor.metadata.InvalidConfigurationMetadataException; | ||
import org.springframework.boot.configurationprocessor.metadata.ItemMetadata; | ||
|
@@ -57,6 +58,7 @@ | |
* @author Phillip Webb | ||
* @author Kris De Volder | ||
* @author Jonas Keßler | ||
* @author Chris Bono | ||
* @since 1.2.0 | ||
*/ | ||
@SupportedAnnotationTypes({ ConfigurationMetadataAnnotationProcessor.CONFIGURATION_PROPERTIES_ANNOTATION, | ||
|
@@ -71,6 +73,8 @@ public class ConfigurationMetadataAnnotationProcessor extends AbstractProcessor | |
|
||
static final String ADDITIONAL_METADATA_LOCATIONS_OPTION = "org.springframework.boot.configurationprocessor.additionalMetadataLocations"; | ||
|
||
static final String FAIL_ON_UNDETERMINED_DEFAULT_VALUE_OPTION = "org.springframework.boot.configurationprocessor.failOnUndeterminedDefaultValue"; | ||
|
||
static final String CONFIGURATION_PROPERTIES_ANNOTATION = "org.springframework.boot.context.properties.ConfigurationProperties"; | ||
|
||
static final String NESTED_CONFIGURATION_PROPERTY_ANNOTATION = "org.springframework.boot.context.properties.NestedConfigurationProperty"; | ||
|
@@ -97,8 +101,8 @@ public class ConfigurationMetadataAnnotationProcessor extends AbstractProcessor | |
|
||
static final String NAME_ANNOTATION = "org.springframework.boot.context.properties.bind.Name"; | ||
|
||
private static final Set<String> SUPPORTED_OPTIONS = Collections | ||
.unmodifiableSet(Collections.singleton(ADDITIONAL_METADATA_LOCATIONS_OPTION)); | ||
private static final Set<String> SUPPORTED_OPTIONS = Collections.unmodifiableSet(new HashSet<>( | ||
Arrays.asList(ADDITIONAL_METADATA_LOCATIONS_OPTION, FAIL_ON_UNDETERMINED_DEFAULT_VALUE_OPTION))); | ||
|
||
private MetadataStore metadataStore; | ||
|
||
|
@@ -328,6 +332,7 @@ protected ConfigurationMetadata writeMetaData() throws Exception { | |
ConfigurationMetadata metadata = this.metadataCollector.getMetadata(); | ||
metadata = mergeAdditionalMetadata(metadata); | ||
if (!metadata.getItems().isEmpty()) { | ||
validateDefaultValuesResolved(metadata); | ||
this.metadataStore.writeMetadata(metadata); | ||
return metadata; | ||
} | ||
|
@@ -353,6 +358,26 @@ private ConfigurationMetadata mergeAdditionalMetadata(ConfigurationMetadata meta | |
return metadata; | ||
} | ||
|
||
private void validateDefaultValuesResolved(ConfigurationMetadata metadata) { | ||
|
||
boolean failOnBadDefaultValues = Boolean.valueOf(this.processingEnv.getOptions().getOrDefault( | ||
ConfigurationMetadataAnnotationProcessor.FAIL_ON_UNDETERMINED_DEFAULT_VALUE_OPTION, "true")); | ||
|
||
metadata.getItems().forEach((itemMetadata) -> { | ||
if (itemMetadata.getDefaultValue() instanceof ValueWrapper) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have not propagated the change from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One reason to keep it Object is to prevent the ripple around all areas that are accessing ItemMetadata#defaultValue (JsonConverter, MetadataStore, etc..). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah. I didn't even think of changing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately we have to wait until the additional metadata is merged to see if the user added a manual hint. |
||
ValueWrapper defaultValue = (ValueWrapper) itemMetadata.getDefaultValue(); | ||
if (!defaultValue.valueDetermined()) { | ||
String msg = String.format( | ||
"Could not resolve default value for property '%s' using expression '%s'. Add 'defaultValue' in additional-spring-configuration.metadata.json", | ||
itemMetadata.getName(), defaultValue.valueInitializerExpression()); | ||
log(failOnBadDefaultValues ? Kind.ERROR : Kind.WARNING, msg); | ||
} | ||
itemMetadata.setDefaultValue(defaultValue.value()); | ||
} | ||
|
||
}); | ||
} | ||
|
||
private String getStackTrace(Exception ex) { | ||
StringWriter writer = new StringWriter(); | ||
ex.printStackTrace(new PrintWriter(writer, true)); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,104 @@ | ||
/* | ||
* 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. | ||
* You may obtain a copy of the License at | ||
* | ||
* https://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
package org.springframework.boot.configurationprocessor.fieldvalues; | ||
|
||
import java.util.Objects; | ||
|
||
import javax.lang.model.element.TypeElement; | ||
|
||
/** | ||
* The value assigned to a field in a {@link TypeElement}, including information related | ||
* to value resolution. | ||
* | ||
* @author Chris Bono | ||
* @since 2.6 | ||
*/ | ||
public final class ValueWrapper { | ||
|
||
private final Object value; | ||
|
||
private final String valueInitializerExpression; | ||
|
||
private final boolean valueDetermined; | ||
|
||
private ValueWrapper(Object value, String valueInitializerExpression, boolean valueDetermined) { | ||
this.value = value; | ||
this.valueInitializerExpression = valueInitializerExpression; | ||
this.valueDetermined = valueDetermined; | ||
} | ||
|
||
public static ValueWrapper of(Object value, String valueInitializerExpression) { | ||
return new ValueWrapper(value, valueInitializerExpression, true); | ||
} | ||
|
||
public static ValueWrapper unresolvable(String valueInitializerExpression) { | ||
return new ValueWrapper(null, valueInitializerExpression, false); | ||
} | ||
|
||
/** | ||
* Return the wrapped value. | ||
* @return the wrapped value or {@code null} if the value was initialized with | ||
* {@code null} or the initializer expression was {@link #valueDetermined() unable to | ||
* be determined}. | ||
*/ | ||
public Object value() { | ||
return this.value; | ||
} | ||
|
||
/** | ||
* Return the expression used to initialize the value. | ||
* @return the expression used to initialize the value or {@code null} if no | ||
* initializer was used to determine the value | ||
*/ | ||
public String valueInitializerExpression() { | ||
return this.valueInitializerExpression; | ||
} | ||
|
||
/** | ||
* Return whether or not the value was able to be determined. | ||
* @return {@code true} if the value was able to be determined, {@code false} | ||
* otherwise. | ||
*/ | ||
public boolean valueDetermined() { | ||
return this.valueDetermined; | ||
} | ||
|
||
@Override | ||
public boolean equals(Object o) { | ||
if (this == o) { | ||
return true; | ||
} | ||
if (o == null || getClass() != o.getClass()) { | ||
return false; | ||
} | ||
ValueWrapper other = (ValueWrapper) o; | ||
return this.valueDetermined == other.valueDetermined && Objects.deepEquals(this.value, other.value) | ||
&& Objects.equals(this.valueInitializerExpression, other.valueInitializerExpression); | ||
} | ||
|
||
@Override | ||
public int hashCode() { | ||
return Objects.hash(this.value, this.valueInitializerExpression, this.valueDetermined); | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return "ValueWrapper{value=" + this.value + ", valueInitializerExpression=" + this.valueInitializerExpression | ||
+ ", valueDetermined=" + this.valueDetermined + "}"; | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another interesting one
DataSize.ofBytes(Integer.MAX_VALUE)
.