jdaugherty opened a new pull request, #16795:
URL: https://github.com/apache/grails-core/pull/16795
## Description
Since #16552, a non-empty configuration list is presented to Spring's
`Environment` only under its indexed names, such as `app.names[0]`, so that
Spring Boot binds it element by element. A `${...}` placeholder that names the
list itself no longer finds a value:
```groovy
// application.yml: app.names: [first, second]
@Value('${app.names}')
List<String> names // 8.0.0: [first, second] 8.0.x: ['${app.names}']
```
Grails leaves a placeholder it cannot resolve as written, so the application
starts and the injection point silently receives the placeholder text. The
upgrade notes said the application would fail to start with `Could not resolve
placeholder`. An application that injects a list of trusted certificates this
way fails much later, when `${...}` is parsed as a certificate.
`GrailsPlaceholderConfigurer` now resolves such a placeholder, after the
environment and the local properties, to the elements of the list joined by
commas, which Spring converts to the `List` or array the placeholder is
injected into, as it did before #16552. The list is read with Spring Boot's
`Binder`, so it comes from the property source of the highest priority that
sets it, as `@ConfigurationProperties` binding reads it, and the placeholders
in its elements are resolved.
The resolution lives in the configurer only. The `Environment` still
presents the list by its indexed names, so #16550 stays fixed and
`environment.getProperty('app.names')` is still `null`. A placeholder naming a
list of objects, such as `springdoc.group-configs`, is still not resolved.
This reverses the part of #16552 that documented `@Value` as unable to read
a list. The Grails settings that #16552 moved to binding are unaffected. The
configuration guide and the Grails 8 upgrade notes now describe the placeholder
for both kinds of list, including that an unresolvable one is left as written
rather than failing startup.
The change from #16552 is on 8.0.x after the 8.0.0 tag, and on 8.1.x, 9.0.x
and 9.1.x by merge; no 7.x line has it. This PR targets 8.0.x so the fix merges
up with it, and it applies to 9.0.x without conflicts.
### Validation
- `GrailsPlaceholderConfigurerSpec`: five of the new features failed before
the change, each injecting `${app.names}` or `${app.origins}` literally; all 11
features pass after it, on 8.0.x and on 9.0.x.
- `./gradlew :grails-core:test :grails-core:codeStyle --rerun-tasks`: 1,053
tests, no failures, 3 skipped; Checkstyle and CodeNarc clean.
- `./gradlew :grails-testing-support-core:test`, which registers the same
configurer: 47 tests, no failures.
- A Grails 9 application that injects
`@Value('${trifleet.security.certificates}') List<String>`, a list set in a
plugin's Groovy configuration, received `['${trifleet.security.certificates}']`
and failed in `BootStrap` parsing it as a certificate. With this change applied
to 9.0.x it receives the certificate and installs its trust store.
- `git diff --check`.
- Not run: `./gradlew build --rerun-tasks` and the repository-wide
`./gradlew clean aggregateViolations :grails-test-report:check --continue`.
## Contributor Checklist
### Issue and Scope
- [x] This PR is linked to an existing issue that has been **acknowledged or
approved** by the project team. If no approved issue exists, please give
background on why this change is necessary. Tickets are preferred for release
change log history.
- No issue: the background is above. It follows up #16552 / #16550.
- [x] This PR addresses the **complete scope** of the linked issue. Partial
implementations or unfinished work should not be submitted for review.
- [x] This PR contains a **single, focused change**. Unrelated changes
should be submitted as separate pull requests.
- [x] This PR targets the **correct branch** for the type of change:
- **Patch release branches** (e.g., `7.0.x`): Bug fixes only. No new
features or API changes.
- **Minor release branches** (e.g., `7.1.x`): New features are welcome,
but breaking existing APIs must be avoided.
- **Major release branches** (e.g., `8.0.x`): Reserved for major
changes. Breaking API changes are permitted.
### Code Quality
- [x] I have **added or updated tests** that cover the changes introduced in
this PR. All code contributions are expected to include appropriate test
coverage.
- [ ] I have verified that all existing tests pass by running `./gradlew
build --rerun-tasks`.
- The affected modules' tests passed, as listed above. CI will run the
full build.
- [x] My code follows the project's **code style** guidelines. The affected
module's `codeStyle` task passed.
- [x] This PR does **not** include mass reformatting, style-only changes, or
large-scale refactoring unless it was **explicitly approved** in the linked
issue. Unsolicited reformatting will not be accepted.
### Licensing and Attribution
- [x] All contributed code is provided under the [Apache License
2.0](https://www.apache.org/licenses/LICENSE-2.0), and new source files include
the appropriate **Apache license header**.
- [x] I have the necessary rights to submit this contribution and confirm it
is my own original work (see [Legal
Notice](https://github.com/apache/grails-core/blob/8.0.x/CONTRIBUTING.md#i-want-to-contribute)).
### Documentation
- [x] If this PR introduces user-facing changes, I have included or updated
the relevant documentation.
- [x] If this PR adds a new feature, I have updated the **What's New**
section of the Grails Guide. — Not applicable: bug fix.
- [x] If this PR introduces breaking changes or changes that require user
action during an upgrade, I have updated the **Upgrade Notes** for the
corresponding version in the Grails Guide.
- [x] The PR description clearly explains **what** was changed and **why**.
--
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]
