jdaugherty commented on code in PR #16047:
URL: https://github.com/apache/grails-core/pull/16047#discussion_r3683039239
##########
grails-doc/build.gradle:
##########
@@ -292,7 +292,9 @@ generateConfigReference.configure { Task it ->
project(':grails-cache').tasks.named('jar'),
project(':grails-data-hibernate5-dbmigration').tasks.named('jar'),
project(':grails-data-mongodb').tasks.named('jar'),
- project(':grails-views-gson').tasks.named('jar')
+ project(':grails-views-gson').tasks.named('jar'),
+ project(':grails-views-markup').tasks.named('jar'),
Review Comment:
Wiring these two jars into `generateConfigReference` publishes the generated
metadata straight into the Application Properties reference, and
`grails-views-markup` has no curated overlay. Generating the reference from
this branch:
- 33 of 187 rows now have an empty Description **and** an empty Default: all
24 `grails.views.markup.*`, plus the 9 newly inferred `grails.views.json.*`
entries (`baseTemplateClass`, `cache`, `enableReloading`, `extension`,
`packageImports`, `packageName`, `staticImports`, `templatePath`,
`useAbsoluteLinks`). Every pre-existing row in that file has a description, so
all 33 blanks are new.
- The markup rows land inside the existing "Views & GSP" section,
interleaved with fully documented `grails.views.gsp.*` rows, so they read as
holes in an otherwise complete table.
Every one of these is settable, so every one needs documenting. Can we add
`grails-views-markup/src/main/resources/META-INF/additional-spring-configuration-metadata.json`
with a group description plus a description and default for each of the 24,
and descriptions for the 9 new JSON Views entries, before these jars feed the
reference?
##########
grails-configuration-metadata/src/main/groovy/org/apache/grails/configuration/metadata/ConfigurationMetadataTransformation.groovy:
##########
@@ -0,0 +1,277 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.apache.grails.configuration.metadata
+
+import groovy.transform.CompileStatic
+import org.codehaus.groovy.ast.ASTNode
+import org.codehaus.groovy.ast.ClassHelper
+import org.codehaus.groovy.ast.ClassNode
+import org.codehaus.groovy.ast.FieldNode
+import org.codehaus.groovy.ast.GenericsType
+import org.codehaus.groovy.ast.PropertyNode
+import org.codehaus.groovy.ast.expr.ConstantExpression
+import org.codehaus.groovy.ast.expr.Expression
+import org.codehaus.groovy.ast.MethodNode
+import org.codehaus.groovy.control.CompilePhase
+import org.codehaus.groovy.control.SourceUnit
+import org.codehaus.groovy.syntax.SyntaxException
+import org.codehaus.groovy.transform.ASTTransformation
+import org.codehaus.groovy.transform.GroovyASTTransformation
+
+import static java.lang.reflect.Modifier.PRIVATE
+import static java.lang.reflect.Modifier.FINAL
+import static java.lang.reflect.Modifier.STATIC
+
+/**
+ * Embeds configuration metadata in each annotated Groovy class. Aggregation
is deliberately
+ * deferred to the Gradle task so incremental Groovy compilation never writes
shared output.
+ */
+@CompileStatic
+@GroovyASTTransformation(phase = CompilePhase.SEMANTIC_ANALYSIS)
Review Comment:
`SEMANTIC_ANALYSIS` runs before `@Delegate` composes
(`DelegateASTTransformation` is `CANONICALIZATION`), so delegate-generated
setters are invisible to `collectSetterProperties`. CORS is exactly that case:
running `:grails-web-url-mappings:generateConfigurationMetadata` on this
branch, the generated half contributes only `grails.cors.enabled` and
`grails.cors.mappings`. `allowedOrigins`, `allowedMethods`, `allowedHeaders`,
`exposedHeaders`, `maxAge` and `allowCredentials` — all bindable through the
`@Delegate GrailsDefaultCorsConfiguration` and Spring's `CorsConfiguration`
setters — survive only because the new overlay lists them by hand.
That is a reasonable limitation to accept, but today it fails silently: add
a delegated property and the metadata simply will not have it, with nothing in
the build to say so. Can we state the limitation in the class documentation,
and ideally have the transform warn (or the task fail) when a
`@ConfigurationProperties` class carries a `@Delegate` field, so whoever adds
one is told to extend the overlay?
##########
grails-configuration-metadata/build.gradle:
##########
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+plugins {
+ id 'groovy'
+ id 'java-library'
+ id 'org.apache.grails.buildsrc.properties'
+ id 'org.apache.grails.buildsrc.dependency-validator'
+ id 'org.apache.grails.buildsrc.compile'
+ id 'org.apache.grails.buildsrc.publish'
Review Comment:
Applying `buildsrc.publish` (with the matching `publish-root-config.gradle`
entry) puts this module on Maven Central, and because `grails-bom/base` adds
every published subproject as a constraint it lands in `grails-bom` too. It is
also the only new module that deliberately skips `gradle/docs-config.gradle`,
so it ships as a published artifact with no API docs. It is `compileOnly` on
the five framework modules, so it never reaches their POMs, and nothing outside
this build can use it without the unpublished `build-logic` plugin.
It also registers a **global** transform via
`META-INF/services/org.codehaus.groovy.transform.ASTTransformation`. Once
published, anyone who ends up with it on a compile classpath silently gets
`__grailsConfigurationMetadata` injected into their own
`@ConfigurationProperties` classes, and a hard compile error if they happen to
declare a field with that name. Is there a consumer that needs the artifact? If
not, dropping the publish/sbom/vulnerability-scan plugins and the
`publishedProjects` entry keeps it internal to the build.
Separately, `compileOnly 'org.springframework.boot:spring-boot'` on line 39
looks unused — the transform matches `ConfigurationProperties` and
`ConstructorBinding` by name string and imports nothing from Spring.
##########
grails-web-core/src/main/resources/META-INF/spring-configuration-metadata.json:
##########
@@ -20,10 +20,6 @@
"name": "grails.logging",
"description": "Web & Controllers"
},
- {
- "name": "grails.cors",
- "description": "CORS"
- },
{
"name": "grails.mime",
Review Comment:
With CORS moved out, this file still carries 10 groups and 24 properties for
prefixes spanning URL mappings, content negotiation, static resources and
scaffolding. Not a blocker for this PR, but is the intent to keep relocating
those to the modules that own them as they gain `@ConfigurationProperties`
classes? Worth capturing in #15469 so this file does not stay the central
fallback by default.
##########
grails-configuration-metadata/src/test/groovy/org/apache/grails/configuration/metadata/ConfigurationMetadataTransformationSpec.groovy:
##########
@@ -0,0 +1,117 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.apache.grails.configuration.metadata
+
+import groovy.json.JsonSlurper
+import org.codehaus.groovy.control.MultipleCompilationErrorsException
+import spock.lang.Specification
+
+import java.lang.reflect.Field
+import java.lang.reflect.Modifier
+
+class ConfigurationMetadataTransformationSpec extends Specification {
Review Comment:
Two feature methods for a 277-line transform leaves most of the logic that
decides what gets published unexercised. The TestKit spec covers the Java/ASM
half thoroughly, but the Groovy half is only tested through one happy-path
class, so the branches below can change behaviour without any test noticing:
- `@Delegate` field exclusion, and the `setGrailsApplication`/`setMetaClass`
exclusions.
- `collectSetterProperties` walking superclasses and interfaces — a config
class extending a base class and implementing a trait is the shape both view
configurations actually have, and it is the source of most generated properties.
- `constructorBoundProperties` selection: `@ConstructorBinding` on one of
several constructors, the "no no-arg constructor plus exactly one candidate"
rule, and private/synthetic constructors being ignored.
- A non-constant `prefix` expression, which makes `addPayload` bail out
entirely and silently hands the class to the bytecode bean-scan path instead —
worth pinning, since the two paths do not infer the same set.
- `typeName`/`genericTypeName` for arrays, wildcards (`? extends` / `?
super`) and type variables.
- The hand-rolled `toJson`/`escapeJson`. A constant default containing a
quote, backslash, newline or control character is written into a class-file
string constant and parsed back by `JsonSlurper` in the Gradle task; if the
escaping is ever wrong the failure surfaces as a JSON parse error during
someone else's module build.
- An interface annotated `@ConfigurationProperties` (skipped by
`!node.interface`), and a prefix-less `@ConfigurationProperties`.
Each is a few lines of `parseClass` in this spec, and they are what makes
the mechanism safe to change later.
##########
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/ConfigurationMetadataPlugin.groovy:
##########
@@ -0,0 +1,557 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.apache.grails.buildsrc
+
+import groovy.json.JsonOutput
+import groovy.json.JsonSlurper
+import org.gradle.api.DefaultTask
+import org.gradle.api.Plugin
+import org.gradle.api.Project
+import org.gradle.api.file.ConfigurableFileCollection
+import org.gradle.api.file.DirectoryProperty
+import org.gradle.api.file.FileSystemOperations
+import org.gradle.api.file.RegularFileProperty
+import org.gradle.api.plugins.JavaPluginExtension
+import org.gradle.api.tasks.CacheableTask
+import org.gradle.api.tasks.InputFile
+import org.gradle.api.tasks.InputFiles
+import org.gradle.api.tasks.Optional
+import org.gradle.api.tasks.OutputDirectory
+import org.gradle.api.tasks.PathSensitive
+import org.gradle.api.tasks.PathSensitivity
+import org.gradle.api.tasks.TaskAction
+import org.gradle.api.tasks.compile.JavaCompile
+import org.objectweb.asm.AnnotationVisitor
+import org.objectweb.asm.ClassReader
+import org.objectweb.asm.ClassVisitor
+import org.objectweb.asm.FieldVisitor
+import org.objectweb.asm.MethodVisitor
+import org.objectweb.asm.Opcodes
+import org.objectweb.asm.RecordComponentVisitor
+import org.objectweb.asm.Type
+
+import java.nio.charset.StandardCharsets
+import java.nio.file.Files
+import java.util.stream.Stream
+
+import javax.inject.Inject
+
+/** Generates standard Spring Boot configuration metadata from compiled
classes without classloading them. */
+class ConfigurationMetadataPlugin implements Plugin<Project> {
+
+ @Override
+ void apply(Project project) {
+ project.plugins.withId('java') {
+ JavaPluginExtension java =
project.extensions.getByType(JavaPluginExtension)
+ project.tasks.withType(JavaCompile).configureEach { JavaCompile
task ->
+ if (!task.options.compilerArgs.contains('-parameters')) {
+ task.options.compilerArgs.add('-parameters')
+ }
+ }
+ Project compilerProject =
project.rootProject.findProject(':grails-configuration-metadata')
+ if (compilerProject == null) {
+ throw new IllegalStateException(
+ 'The configuration metadata plugin requires the
:grails-configuration-metadata compiler project')
+ }
+ project.dependencies.add('compileOnly', compilerProject)
+ def main = java.sourceSets.named('main')
+ main.configure { sourceSet ->
+
sourceSet.resources.exclude('META-INF/spring-configuration-metadata.json')
+ }
+ def generate =
project.tasks.register('generateConfigurationMetadata',
GenerateConfigurationMetadataTask) {
+ it.classesDirs.from(main.map { sourceSet ->
sourceSet.output.classesDirs })
+ it.dependsOn(main.map { sourceSet ->
sourceSet.output.classesDirs })
+ it.dependsOn(project.tasks.matching { task -> task.name ==
'copyAstClasses' })
+ def overlay = project.layout.projectDirectory.file(
+
'src/main/resources/META-INF/additional-spring-configuration-metadata.json')
+ if (overlay.asFile.isFile()) {
+ it.additionalMetadata.set(overlay)
+ }
+
it.outputDirectory.set(project.layout.buildDirectory.dir('generated/configurationMetadata'))
+ }
+ project.tasks.named(main.get().processResourcesTaskName) {
+ it.dependsOn(generate)
+ it.from(generate)
+ }
+ }
+ }
+}
+
+@CacheableTask
+abstract class GenerateConfigurationMetadataTask extends DefaultTask {
+
+ static final String CONFIGURATION_PROPERTIES =
+
'Lorg/springframework/boot/context/properties/ConfigurationProperties;'
+ static final String CONSTRUCTOR_BINDING =
+
'Lorg/springframework/boot/context/properties/bind/ConstructorBinding;'
+ static final String PAYLOAD_FIELD = '__grailsConfigurationMetadata'
+
+ @InputFiles
+ @PathSensitive(PathSensitivity.RELATIVE)
+ abstract ConfigurableFileCollection getClassesDirs()
+
+ @InputFile
+ @Optional
+ @PathSensitive(PathSensitivity.RELATIVE)
+ abstract RegularFileProperty getAdditionalMetadata()
+
+ @OutputDirectory
+ abstract DirectoryProperty getOutputDirectory()
+
+ @Inject
+ abstract FileSystemOperations getFileSystemOperations()
+
+ @TaskAction
+ void generate() {
+ Map<String, ClassModel> models = readModels()
+ List<Map<String, Object>> groups = []
+ List<Map<String, Object>> properties = []
+ models.values().findAll { ClassModel model -> model.prefix != null
}.sort { ClassModel model -> model.name }.each {
+ ClassModel model ->
+ if (model.prefix) {
+ groups << [name: model.prefix, type: model.name, sourceType:
model.name]
+ }
+ if (model.payloadProperties != null) {
+ model.payloadGroups.each { Map<String, Object> group ->
+ Map<String, Object> entry = new LinkedHashMap<>(group)
+ entry.sourceType = model.name
+ groups << entry
+ }
+ model.payloadProperties.each { Map<String, Object> property ->
+ Map<String, Object> entry = new LinkedHashMap<>(property)
+ entry.sourceType = model.name
+ properties << entry
+ }
+ } else {
+ GenerateConfigurationMetadataTask.addProperties(
+ model, model.prefix, model.name, models, groups,
properties, new LinkedHashSet<String>())
+ }
+ }
+
+ Map<String, Object> metadata = merge(groups, properties, readOverlay())
+ File output = outputDirectory.get().asFile
+ fileSystemOperations.delete { it.delete(output) }
+ File target = new File(output,
'META-INF/spring-configuration-metadata.json')
+ target.parentFile.mkdirs()
+
target.setText(JsonOutput.prettyPrint(JsonOutput.toJson(canonical(metadata))) +
'\n', StandardCharsets.UTF_8.name())
+ }
+
+ private Map<String, ClassModel> readModels() {
+ Map<String, ClassModel> models = [:]
+ classesDirs.files.findAll { File file -> file.isDirectory() }.sort {
File file -> file.absolutePath }.each {
+ File directory ->
+ Stream<java.nio.file.Path> paths = Files.walk(directory.toPath())
+ try {
+ paths.filter { java.nio.file.Path path ->
Files.isRegularFile(path) && path.fileName.toString().endsWith('.class') }
+ .sorted()
+ .forEach { java.nio.file.Path path ->
+ ClassModel model =
GenerateConfigurationMetadataTask.readClass(Files.readAllBytes(path))
+ ClassModel previous = models.put(model.name, model)
+ if (previous != null && previous != model) {
+ throw new IllegalArgumentException(
+ "Duplicate compiled class
'${model.name}' in configuration metadata inputs")
+ }
+ }
+ } finally {
+ paths.close()
+ }
+ }
+ models
+ }
+
+ static ClassModel readClass(byte[] bytes) {
+ ClassModel model = new ClassModel()
+ new ClassReader(bytes).accept(new ClassVisitor(Opcodes.ASM9) {
+ @Override
+ void visit(int version, int access, String name, String signature,
String superName, String[] interfaces) {
+ model.name = name.replace('/', '.')
+ model.superName = superName?.replace('/', '.')
+ model.interfaces = interfaces.collect { String interfaceName
-> interfaceName.replace('/', '.') }
+ }
+
+ @Override
+ AnnotationVisitor visitAnnotation(String descriptor, boolean
visible) {
+ if (descriptor != CONFIGURATION_PROPERTIES) {
+ return null
+ }
+ model.prefix = ''
+ new AnnotationVisitor(Opcodes.ASM9) {
+ @Override
+ void visit(String name, Object value) {
+ if (name == 'prefix' || name == 'value') {
+ model.prefix = String.valueOf(value)
+ }
+ }
+ }
+ }
+
+ @Override
+ FieldVisitor visitField(int access, String name, String
descriptor, String signature, Object value) {
+ int payloadAccess = Opcodes.ACC_PRIVATE | Opcodes.ACC_STATIC |
Opcodes.ACC_FINAL | Opcodes.ACC_SYNTHETIC
+ if (name == PAYLOAD_FIELD && value instanceof String &&
(access & payloadAccess) == payloadAccess) {
+ model.payload = value as String
+ }
+ null
+ }
+
+ @Override
+ RecordComponentVisitor visitRecordComponent(String name, String
descriptor, String signature) {
+ model.properties[name] = new PropertyModel(
+ name: name,
+ type: fieldType(descriptor, signature),
+ constructorBound: true,
+ readable: true)
+ null
+ }
+
+ @Override
+ MethodVisitor visitMethod(int access, String name, String
descriptor, String signature, String[] exceptions) {
+ Type method = Type.getMethodType(descriptor)
+ if (name == '<init>' && (access & (Opcodes.ACC_PRIVATE |
Opcodes.ACC_SYNTHETIC)) == 0) {
+ ConstructorModel constructor = new ConstructorModel()
+ model.constructors << constructor
+ Type[] argumentTypes = method.argumentTypes
+ List<String> argumentTypeNames =
methodArgumentTypes(descriptor, signature)
+ return new MethodVisitor(Opcodes.ASM9) {
+ private int parameterIndex
+
+ @Override
+ void visitParameter(String parameterName, int
parameterAccess) {
+ if (parameterName && parameterIndex <
argumentTypes.length &&
+ (parameterAccess & (Opcodes.ACC_SYNTHETIC
| Opcodes.ACC_MANDATED)) == 0) {
+ constructor.properties[parameterName] = new
PropertyModel(
+ name: parameterName,
+ type:
argumentTypeNames[parameterIndex],
+ constructorBound: true)
+ }
+ parameterIndex++
+ }
+
+ @Override
+ AnnotationVisitor visitAnnotation(String
annotationDescriptor, boolean visible) {
+ constructor.selected |= annotationDescriptor ==
CONSTRUCTOR_BINDING
+ null
+ }
+ }
+ }
+ if ((access & Opcodes.ACC_PUBLIC) == 0 ||
+ (access & (Opcodes.ACC_STATIC |
Opcodes.ACC_SYNTHETIC)) != 0 || name.contains('$')) {
+ return null
+ }
+ if (name.startsWith('get') && name.length() > 3 &&
method.argumentTypes.length == 0 &&
+ method.returnType.sort != Type.VOID) {
+ addAccessor(model, decapitalize(name.substring(3)),
method.returnType.descriptor,
+ methodReturnSignature(signature), false)
+ } else if (name.startsWith('is') && name.length() > 2 &&
method.argumentTypes.length == 0 &&
+ method.returnType.sort == Type.BOOLEAN) {
+ addAccessor(model, decapitalize(name.substring(2)),
method.returnType.descriptor,
+ methodReturnSignature(signature), false)
+ } else if (name.startsWith('set') && name.length() > 3 &&
method.argumentTypes.length == 1) {
+ addAccessor(model, decapitalize(name.substring(3)),
method.argumentTypes[0].descriptor,
+ methodFirstArgumentSignature(signature), true)
+ }
+ null
+ }
+ }, ClassReader.SKIP_CODE | ClassReader.SKIP_FRAMES)
+
+ if (model.payload != null) {
+ Map payload = new JsonSlurper().parseText(model.payload) as Map
+ model.prefix = payload.get('prefix') as String
+ model.name = payload.get('sourceType') as String
+ model.payloadGroups = ((payload.get('groups') ?: []) as
List).collect { Map group ->
+ new LinkedHashMap<String, Object>(group)
+ }
+ model.payloadProperties = ((payload.get('properties') ?: []) as
List).collect { Map property ->
+ new LinkedHashMap<String, Object>(property)
+ }
+ } else {
+ List<ConstructorModel> selectedConstructors =
model.constructors.findAll { ConstructorModel constructor ->
+ constructor.selected
+ }
+ ConstructorModel bindingConstructor = selectedConstructors.size()
== 1 ? selectedConstructors[0] :
+ (model.constructors.size() == 1 &&
!model.constructors[0].properties.isEmpty() ?
+ model.constructors[0] : null)
+ bindingConstructor?.properties?.each { String name, PropertyModel
constructorProperty ->
+ PropertyModel property =
model.properties.computeIfAbsent(name) { new PropertyModel(name: name) }
+ property.type = property.type ?: constructorProperty.type
+ property.constructorBound = true
+ }
+ model.properties = model.properties.findAll { String name,
PropertyModel property ->
+ property.writable || property.collectionOrMap() ||
property.constructorBound
+ }
+ }
+ model
+ }
+
+ private static void addAccessor(ClassModel model, String name, String
descriptor, String signature, boolean writable) {
+ PropertyModel property = model.properties.computeIfAbsent(name) { new
PropertyModel(name: name) }
+ if (property.type == null || signature != null) {
+ property.type = fieldType(descriptor, signature)
+ }
+ property.writable |= writable
+ property.readable |= !writable
+ }
+
+ static void addProperties(ClassModel model, String prefix, String
sourceType,
+ Map<String, ClassModel> models,
List<Map<String, Object>> groups,
+ List<Map<String, Object>> properties,
+ Set<String> visiting) {
+ if (!visiting.add(model.name)) {
+ return
+ }
+ propertiesFor(model, models, new LinkedHashSet<String>()).values()
+ .sort { PropertyModel property -> property.name }.each {
PropertyModel property ->
+ String name = prefix ? "${prefix}.${property.name}" : property.name
+ ClassModel nested = models[property.rawType()]
+ if (nested != null && !propertiesFor(nested, models, new
LinkedHashSet<String>()).isEmpty()) {
+ groups << [name: name, type: property.type, sourceType:
sourceType]
+ addProperties(nested, name, sourceType, models, groups,
properties, visiting)
+ } else {
+ properties << [name: name, type: property.type, sourceType:
sourceType]
+ }
+ }
+ visiting.remove(model.name)
+ }
+
+ private static Map<String, PropertyModel> propertiesFor(ClassModel model,
Map<String, ClassModel> models,
+ Set<String>
visited) {
+ if (model == null || !visited.add(model.name)) {
+ return [:]
+ }
+ Map<String, PropertyModel> properties = [:]
+ properties.putAll(propertiesFor(models[model.superName], models,
visited))
+ model.interfaces.each { String interfaceName ->
+ properties.putAll(propertiesFor(models[interfaceName], models,
visited))
+ }
+ properties.putAll(model.properties)
+ properties
+ }
+
+ private Map readOverlay() {
+ File file = additionalMetadata.asFile.orNull
+ file?.isFile() ? new JsonSlurper().parse(file,
StandardCharsets.UTF_8.name()) as Map : [:]
+ }
+
+ private static Map<String, Object> merge(List<Map<String, Object>> groups,
+ List<Map<String, Object>>
properties, Map overlay) {
+ Map<String, Object> result = [:]
+ result['groups'] = mergeNamed(groups, (overlay.get('groups') ?: []) as
List, 'groups')
+ result['properties'] = mergeNamed(properties,
(overlay.get('properties') ?: []) as List, 'properties')
+ if (overlay.containsKey('hints')) {
+ result['hints'] = mergeNamed([], overlay.get('hints') as List,
'hints')
+ }
+ overlay.each { Object keyValue, Object value ->
+ String key = keyValue.toString()
+ if (!(key in ['groups', 'properties', 'hints', 'ignored'])) {
+ result[key] = value
+ }
+ }
+ if (overlay.containsKey('ignored')) {
+ Map ignored = new LinkedHashMap((overlay.get('ignored') ?: [:]) as
Map)
+ if (ignored.containsKey('properties')) {
+ ignored['properties'] = mergeNamed([],
ignored.get('properties') as List, 'ignored.properties')
+ }
+ result['ignored'] = ignored
+ }
+ result
+ }
+
+ private static List<Object> mergeNamed(List generated, List overlay,
String category) {
+ Map<String, Object> generatedByName = indexByName(generated, category,
'generated')
+ Map<String, Object> overlayByName = indexByName(overlay, category,
'overlay')
+ Map<String, Object> merged = new LinkedHashMap<>(generatedByName)
+ overlayByName.each { String name, Object value ->
+ if (merged[name] instanceof Map && value instanceof Map) {
+ merged[name] = new LinkedHashMap((Map) merged[name]) + (Map)
value
+ } else {
+ merged[name] = value
+ }
+ }
+ merged.keySet().sort().collect { String name -> merged[name] }
+ }
+
+ private static Map<String, Object> indexByName(List source, String
category, String sourceName) {
Review Comment:
`indexByName` enforces name uniqueness per category. That is right for
`properties`, but Boot's format allows repeated group names distinguished by
`sourceType`/`sourceMethod`, and `generate()` emits one group per
`@ConfigurationProperties` class (line 128) plus one per nested holder. Two
config classes sharing a prefix in the same module, or a nested prefix
reachable from two classes, would fail the build with `Conflicting generated
groups metadata for '...'` on metadata that is legal. Worth keying groups on
name + `sourceType`, or merging instead of rejecting.
##########
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/ConfigurationMetadataPlugin.groovy:
##########
@@ -0,0 +1,557 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.apache.grails.buildsrc
+
+import groovy.json.JsonOutput
+import groovy.json.JsonSlurper
+import org.gradle.api.DefaultTask
+import org.gradle.api.Plugin
+import org.gradle.api.Project
+import org.gradle.api.file.ConfigurableFileCollection
+import org.gradle.api.file.DirectoryProperty
+import org.gradle.api.file.FileSystemOperations
+import org.gradle.api.file.RegularFileProperty
+import org.gradle.api.plugins.JavaPluginExtension
+import org.gradle.api.tasks.CacheableTask
+import org.gradle.api.tasks.InputFile
+import org.gradle.api.tasks.InputFiles
+import org.gradle.api.tasks.Optional
+import org.gradle.api.tasks.OutputDirectory
+import org.gradle.api.tasks.PathSensitive
+import org.gradle.api.tasks.PathSensitivity
+import org.gradle.api.tasks.TaskAction
+import org.gradle.api.tasks.compile.JavaCompile
+import org.objectweb.asm.AnnotationVisitor
+import org.objectweb.asm.ClassReader
+import org.objectweb.asm.ClassVisitor
+import org.objectweb.asm.FieldVisitor
+import org.objectweb.asm.MethodVisitor
+import org.objectweb.asm.Opcodes
+import org.objectweb.asm.RecordComponentVisitor
+import org.objectweb.asm.Type
+
+import java.nio.charset.StandardCharsets
+import java.nio.file.Files
+import java.util.stream.Stream
+
+import javax.inject.Inject
+
+/** Generates standard Spring Boot configuration metadata from compiled
classes without classloading them. */
+class ConfigurationMetadataPlugin implements Plugin<Project> {
+
+ @Override
+ void apply(Project project) {
+ project.plugins.withId('java') {
+ JavaPluginExtension java =
project.extensions.getByType(JavaPluginExtension)
+ project.tasks.withType(JavaCompile).configureEach { JavaCompile
task ->
+ if (!task.options.compilerArgs.contains('-parameters')) {
+ task.options.compilerArgs.add('-parameters')
+ }
+ }
+ Project compilerProject =
project.rootProject.findProject(':grails-configuration-metadata')
+ if (compilerProject == null) {
+ throw new IllegalStateException(
+ 'The configuration metadata plugin requires the
:grails-configuration-metadata compiler project')
+ }
+ project.dependencies.add('compileOnly', compilerProject)
+ def main = java.sourceSets.named('main')
+ main.configure { sourceSet ->
+
sourceSet.resources.exclude('META-INF/spring-configuration-metadata.json')
+ }
+ def generate =
project.tasks.register('generateConfigurationMetadata',
GenerateConfigurationMetadataTask) {
+ it.classesDirs.from(main.map { sourceSet ->
sourceSet.output.classesDirs })
+ it.dependsOn(main.map { sourceSet ->
sourceSet.output.classesDirs })
+ it.dependsOn(project.tasks.matching { task -> task.name ==
'copyAstClasses' })
+ def overlay = project.layout.projectDirectory.file(
+
'src/main/resources/META-INF/additional-spring-configuration-metadata.json')
+ if (overlay.asFile.isFile()) {
+ it.additionalMetadata.set(overlay)
+ }
+
it.outputDirectory.set(project.layout.buildDirectory.dir('generated/configurationMetadata'))
+ }
+ project.tasks.named(main.get().processResourcesTaskName) {
+ it.dependsOn(generate)
+ it.from(generate)
+ }
+ }
+ }
+}
+
+@CacheableTask
+abstract class GenerateConfigurationMetadataTask extends DefaultTask {
+
+ static final String CONFIGURATION_PROPERTIES =
+
'Lorg/springframework/boot/context/properties/ConfigurationProperties;'
+ static final String CONSTRUCTOR_BINDING =
+
'Lorg/springframework/boot/context/properties/bind/ConstructorBinding;'
+ static final String PAYLOAD_FIELD = '__grailsConfigurationMetadata'
+
+ @InputFiles
+ @PathSensitive(PathSensitivity.RELATIVE)
+ abstract ConfigurableFileCollection getClassesDirs()
+
+ @InputFile
+ @Optional
+ @PathSensitive(PathSensitivity.RELATIVE)
+ abstract RegularFileProperty getAdditionalMetadata()
+
+ @OutputDirectory
+ abstract DirectoryProperty getOutputDirectory()
+
+ @Inject
+ abstract FileSystemOperations getFileSystemOperations()
+
+ @TaskAction
+ void generate() {
+ Map<String, ClassModel> models = readModels()
+ List<Map<String, Object>> groups = []
+ List<Map<String, Object>> properties = []
+ models.values().findAll { ClassModel model -> model.prefix != null
}.sort { ClassModel model -> model.name }.each {
+ ClassModel model ->
+ if (model.prefix) {
+ groups << [name: model.prefix, type: model.name, sourceType:
model.name]
+ }
+ if (model.payloadProperties != null) {
+ model.payloadGroups.each { Map<String, Object> group ->
+ Map<String, Object> entry = new LinkedHashMap<>(group)
+ entry.sourceType = model.name
+ groups << entry
+ }
+ model.payloadProperties.each { Map<String, Object> property ->
+ Map<String, Object> entry = new LinkedHashMap<>(property)
+ entry.sourceType = model.name
+ properties << entry
+ }
+ } else {
+ GenerateConfigurationMetadataTask.addProperties(
+ model, model.prefix, model.name, models, groups,
properties, new LinkedHashSet<String>())
+ }
+ }
+
+ Map<String, Object> metadata = merge(groups, properties, readOverlay())
+ File output = outputDirectory.get().asFile
+ fileSystemOperations.delete { it.delete(output) }
+ File target = new File(output,
'META-INF/spring-configuration-metadata.json')
+ target.parentFile.mkdirs()
+
target.setText(JsonOutput.prettyPrint(JsonOutput.toJson(canonical(metadata))) +
'\n', StandardCharsets.UTF_8.name())
+ }
+
+ private Map<String, ClassModel> readModels() {
+ Map<String, ClassModel> models = [:]
+ classesDirs.files.findAll { File file -> file.isDirectory() }.sort {
File file -> file.absolutePath }.each {
+ File directory ->
+ Stream<java.nio.file.Path> paths = Files.walk(directory.toPath())
+ try {
+ paths.filter { java.nio.file.Path path ->
Files.isRegularFile(path) && path.fileName.toString().endsWith('.class') }
+ .sorted()
+ .forEach { java.nio.file.Path path ->
+ ClassModel model =
GenerateConfigurationMetadataTask.readClass(Files.readAllBytes(path))
+ ClassModel previous = models.put(model.name, model)
+ if (previous != null && previous != model) {
Review Comment:
`ClassModel` does not implement `equals`, so `previous != model` is identity
comparison and is always true for two distinct instances. This reduces to "fail
on any duplicate class name across the input directories", which is not what
the guard reads as. Either give `ClassModel` value equality (or compare
prefix/payload) so a genuinely identical duplicate is tolerated as intended, or
drop the second condition so the code says what it does.
##########
grails-configuration-metadata/src/main/groovy/org/apache/grails/configuration/metadata/ConfigurationMetadataTransformation.groovy:
##########
@@ -0,0 +1,277 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.apache.grails.configuration.metadata
+
+import groovy.transform.CompileStatic
+import org.codehaus.groovy.ast.ASTNode
+import org.codehaus.groovy.ast.ClassHelper
+import org.codehaus.groovy.ast.ClassNode
+import org.codehaus.groovy.ast.FieldNode
+import org.codehaus.groovy.ast.GenericsType
+import org.codehaus.groovy.ast.PropertyNode
+import org.codehaus.groovy.ast.expr.ConstantExpression
+import org.codehaus.groovy.ast.expr.Expression
+import org.codehaus.groovy.ast.MethodNode
+import org.codehaus.groovy.control.CompilePhase
+import org.codehaus.groovy.control.SourceUnit
+import org.codehaus.groovy.syntax.SyntaxException
+import org.codehaus.groovy.transform.ASTTransformation
+import org.codehaus.groovy.transform.GroovyASTTransformation
+
+import static java.lang.reflect.Modifier.PRIVATE
+import static java.lang.reflect.Modifier.FINAL
+import static java.lang.reflect.Modifier.STATIC
+
+/**
+ * Embeds configuration metadata in each annotated Groovy class. Aggregation
is deliberately
+ * deferred to the Gradle task so incremental Groovy compilation never writes
shared output.
+ */
+@CompileStatic
+@GroovyASTTransformation(phase = CompilePhase.SEMANTIC_ANALYSIS)
+class ConfigurationMetadataTransformation implements ASTTransformation {
+
+ static final String PAYLOAD_FIELD = '__grailsConfigurationMetadata'
+ private static final String CONSTRUCTOR_BINDING =
+
'org.springframework.boot.context.properties.bind.ConstructorBinding'
+ private static final int SYNTHETIC = 0x00001000
+ private static final String CONFIGURATION_PROPERTIES =
'org.springframework.boot.context.properties.ConfigurationProperties'
+
+ @Override
+ void visit(ASTNode[] nodes, SourceUnit source) {
+ source.AST.classes.findAll { ClassNode node ->
+ !node.interface &&
node.getAnnotations(ClassHelper.make(CONFIGURATION_PROPERTIES))
+ }.each { ClassNode node -> addPayload(node, source) }
+ }
+
+ private static void addPayload(ClassNode node, SourceUnit source) {
+ FieldNode existingField = node.getDeclaredField(PAYLOAD_FIELD)
+ if (existingField != null) {
+ source.addError(new SyntaxException(
+ "Configuration properties classes cannot declare reserved
field '${PAYLOAD_FIELD}'",
+ existingField.lineNumber, existingField.columnNumber))
+ return
+ }
+ def annotation =
node.getAnnotations(ClassHelper.make(CONFIGURATION_PROPERTIES))[0]
+ Expression prefixExpression = annotation.getMember('prefix') ?:
annotation.getMember('value')
+ if (prefixExpression != null && !(prefixExpression instanceof
ConstantExpression)) {
+ return
+ }
+ String prefix = prefixExpression == null ? '' :
String.valueOf(((ConstantExpression) prefixExpression).value)
+ Map<String, List<Map<String, Object>>> metadata = metadata(
+ node, prefix, node.name, new LinkedHashSet<String>())
+ String payload = toJson([
+ prefix: prefix,
+ sourceType: node.name,
+ groups: metadata.get('groups'),
+ properties: metadata.get('properties')
+ ])
+ FieldNode field = node.addField(PAYLOAD_FIELD, PRIVATE | STATIC |
FINAL | SYNTHETIC,
+ ClassHelper.STRING_TYPE, new ConstantExpression(payload))
+ field.synthetic = true
+ }
+
+ private static Map<String, List<Map<String, Object>>> metadata(ClassNode
node, String prefix,
+ String
sourceType, Set<String> visiting) {
+ if (!visiting.add(node.name)) {
+ return [groups: [], properties: []]
+ }
+ Map<String, Map<String, Object>> bindable = [:]
+ node.properties.findAll { PropertyNode property ->
isBindableProperty(property) }.each {
+ PropertyNode propertyNode ->
+ FieldNode field = propertyNode.field
+ Map<String, Object> propertyMetadata = [
+ propertyName: field.name, type: typeName(field.type),
classNode: field.type, nested: true]
+ Expression initialExpression = field.initialExpression
+ if (initialExpression instanceof ConstantExpression &&
!initialExpression.isNullExpression()) {
+ propertyMetadata.put('defaultValue', ((ConstantExpression)
initialExpression).value)
+ }
+ bindable.put(field.name, propertyMetadata)
+ }
+ collectSetterProperties(node, bindable, new LinkedHashSet<String>())
+
+ List<Map<String, Object>> groups = []
+ List<Map<String, Object>> properties = []
+ bindable.values().sort { Map<String, Object> property ->
property.propertyName as String }.each {
+ Map<String, Object> property ->
+ String propertyName = property.propertyName as String
+ String name = prefix ? "${prefix}.${propertyName}" : propertyName
+ ClassNode propertyType = property.classNode as ClassNode
+ if (property.nested && isNested(propertyType)) {
+ Map<String, List<Map<String, Object>>> nested =
metadata(propertyType, name, sourceType, visiting)
+ if (nested.get('groups') || nested.get('properties')) {
+ groups << [name: name, type: property.type, sourceType:
sourceType]
+ groups.addAll(nested.get('groups'))
+ properties.addAll(nested.get('properties'))
+ } else {
+ properties << scalarProperty(name, property)
+ }
+ } else {
+ properties << scalarProperty(name, property)
+ }
+ }
+ visiting.remove(node.name)
+ [
+ groups: groups.sort { Map<String, Object> group -> group.name
as String },
+ properties: properties.sort { Map<String, Object> property ->
property.name as String }
+ ]
+ }
+
+ private static Map<String, Object> scalarProperty(String name, Map<String,
Object> property) {
+ Map<String, Object> entry = [name: name, type: property.type]
+ if (property.containsKey('defaultValue')) {
+ entry.put('defaultValue', property.get('defaultValue'))
+ }
+ entry
+ }
+
+ private static boolean isBindableProperty(PropertyNode property) {
+ FieldNode field = property.field
+ boolean constructorBound = !field.final ||
constructorBoundProperties(field.owner).contains(field.name)
+ !field.static && constructorBound && !field.name.startsWith('$') &&
+ field.name != 'metaClass' && field.name != PAYLOAD_FIELD &&
+ !field.annotations.any { annotation ->
+ annotation.classNode.name in ['groovy.lang.Delegate',
'groovy.transform.Delegate']
+ }
+ }
+
+ private static Set<String> constructorBoundProperties(ClassNode owner) {
+ List constructors = owner.declaredConstructors.findAll { constructor ->
+ !constructor.synthetic && !constructor.private
+ }
+ List candidates = constructors.findAll { constructor ->
constructor.parameters.length > 0 }
+ List selected = candidates.findAll { constructor ->
+ constructor.annotations.any { annotation ->
annotation.classNode.name == CONSTRUCTOR_BINDING }
+ }
+ def bindingConstructor = selected.size() == 1 ? selected[0] :
+ (!constructors.any { constructor ->
constructor.parameters.length == 0 } && candidates.size() == 1 ?
+ candidates[0] : null)
+ bindingConstructor ? bindingConstructor.parameters*.name as
Set<String> : Collections.emptySet()
+ }
+
+ private static void collectSetterProperties(ClassNode node, Map<String,
Map<String, Object>> bindable,
+ Set<String> visited) {
+ if (node == null || node.name == Object.name ||
!visited.add(node.name)) {
+ return
+ }
+ node.methods.findAll { MethodNode method ->
+ method.public && !method.static && method.name.startsWith('set')
&& method.name.length() > 3 &&
+ !(method.name in ['setGrailsApplication', 'setMetaClass'])
&& method.parameters.length == 1
+ }.each { MethodNode method ->
+ String propertyName = decapitalize(method.name.substring(3))
+ ClassNode propertyType = method.parameters[0].type
+ bindable.putIfAbsent(propertyName,
+ [propertyName: propertyName, type: typeName(propertyType),
classNode: propertyType, nested: true])
+ }
+ collectSetterProperties(node.superClass, bindable, visited)
+ node.interfaces.each { ClassNode interfaceNode ->
collectSetterProperties(interfaceNode, bindable, visited) }
+ }
+
+ private static boolean isNested(ClassNode type) {
Review Comment:
`isNested` treats every type outside `java.*`/`groovy.*` as a nested
configuration group, and `collectSetterProperties` walks superclasses and
interfaces without applying that filter at all — it only stops at `Object`. The
two hardcoded names on line 174 are what keeps that from exploding today:
`JsonViewConfiguration` and `MarkupViewConfiguration` implement the
`GenericViewConfiguration` trait, which extends `GrailsApplicationAware`, so
without the `setGrailsApplication` exclusion the transform would recurse into
`GrailsApplication`.
Boot's own processor keys nesting off inner classes and
`@NestedConfigurationProperty` rather than a package-name heuristic, and the
ASM path in this PR is already narrower — it nests only into types found in the
same module's output (`models[property.rawType()]`). Could the Groovy path use
the same restriction, and honour `@NestedConfigurationProperty`? As written,
the next config class with a setter taking a framework or third-party bean type
silently emits a nested group tree, and the remedy will be another name added
to that blocklist.
--
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]
