codeconsole commented on PR #16019:
URL: https://github.com/apache/grails-core/pull/16019#issuecomment-5125817373
Every review comment above is answered inline, each naming the commit its
fix landed in. Four are
departures from what was suggested rather than straight fixes, flagged as
such in their threads:
the `isResolved()` guard (a `GroovyBugError` catch instead, since
`DecompiledClassNode` reports
itself resolved while having no loaded `Class`), `autoConfigurationName`
accepting an FQCN
(documented the four renames instead), `@GrailsBeans` retention (left
alone), and
`grails-test-examples/beans-dsl` keeping its own `grails-beans-dsl`
declaration (it deliberately has
no `grails-core`).
One change here did **not** come from the review and is worth a look:
`d8f2a7b78b` adds a shape to
the DSL. A bean closure with parameters but an **empty body** means
"construct the declared type
from these parameters".
```groovy
bean('validateableConstraintsEvaluator',
DefaultConstraintEvaluatorFactoryBean).lazy() {
List<MessageSource> messageSources,
@Qualifier('grailsDomainClassMappingContext') MappingContext
mappingContext,
GrailsApplication grailsApplication ->
}
```
The parameters stay at the declaration, which is the point — a closure
parameter list is the only
place in Groovy that can carry both a generic type and a parameter
annotation, so neither
`List<MessageSource>` nor the `@Qualifier` is expressible as an argument
anywhere else. The
generated body is the same `new Type(...)` that was written out before, so
the compiler selects the
constructor from the parameter types and nothing reads the declared type's
constructors — adding one
cannot change what an existing declaration injects.
Five declarations lose a redundant line: the constraint evaluator, the three
mail beans and the CORS
filter. Their generated `@Bean` signatures are unchanged, checked with
`javap`. A body is still
required wherever construction is more than a pass-through, such as
`new ConfigProperties(grailsApplication.config)` or anything followed by
`.tap { }`. The honest cost
is that an empty block reads as a stub at a glance; it is documented in both
the guide and the
annotation javadoc.
Two earlier attempts at this are off the branch: both inferred the
constructor from the declared
type, which made the declaration silent about what it injects and let a
constructor added elsewhere
change or break it.
On verification: `grails.util.GrailsUtilStackFiltererSpec` and
`org.apache.grails.core.GrailsBootstrapRegistryInitializerSpec` fail in
`grails-core`. Both
reproduce on a clean `upstream/8.0.x` worktree with none of this branch
applied, so they are not
from these changes.
--
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]
