Copilot commented on code in PR #7955:
URL: https://github.com/apache/incubator-seata/pull/7955#discussion_r2711708094
##########
seata-spring-autoconfigure/seata-spring-autoconfigure-core/src/main/java/org/apache/seata/spring/boot/autoconfigure/provider/SpringBootConfigurationProvider.java:
##########
@@ -231,6 +231,10 @@ private Object getConfigFromEnvironment(String dataId,
Class<?> dataType) {
if (value == null) {
value =
environment.getProperty(org.apache.seata.common.util.StringUtils.hump2Line(dataId),
dataType);
}
+ // ensures that values are converted to their actual values
+ if (value instanceof String) {
+ value = environment.resolvePlaceholders((String) value);
+ }
Review Comment:
This bug fix adds placeholder resolution functionality but lacks test
coverage. Consider adding a unit test that verifies placeholder resolution
works correctly, for example by setting up a test with a property value like
"${some.property}" and verifying it gets resolved to the actual value. This
would prevent regression and document the expected behavior.
##########
seata-spring-autoconfigure/seata-spring-autoconfigure-core/src/main/java/org/apache/seata/spring/boot/autoconfigure/provider/SpringBootConfigurationProvider.java:
##########
@@ -231,6 +231,10 @@ private Object getConfigFromEnvironment(String dataId,
Class<?> dataType) {
if (value == null) {
value =
environment.getProperty(org.apache.seata.common.util.StringUtils.hump2Line(dataId),
dataType);
}
+ // ensures that values are converted to their actual values
+ if (value instanceof String) {
+ value = environment.resolvePlaceholders((String) value);
+ }
Review Comment:
Consider moving the placeholder resolution logic to just before the return
statement (after line 245, before line 246). This would make the code cleaner
and handle all return paths consistently in one place, rather than having it
between two different null-checking blocks. The current placement works but is
less maintainable as it's easy to miss that the recursive call at line 243
needs to handle its own placeholder resolution.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
