matrei commented on code in PR #16047: URL: https://github.com/apache/grails-core/pull/16047#discussion_r4059787093
########## build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/ConfigurationMetadataPlugin.groovy: ########## @@ -0,0 +1,760 @@ +/* + * 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.bundling.Jar +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.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) + } + // the per-class payloads are build-time input of generateConfigurationMetadata only + project.tasks.withType(Jar).configureEach { Jar task -> + task.exclude("${GenerateConfigurationMetadataTask.PAYLOAD_DIRECTORY}/**") + } + } + } +} + +@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_DIRECTORY = 'META-INF/grails-configuration-metadata' + private static final List<String> FRAMEWORK_ACCESSORS = ['getMetaClass', 'setMetaClass', 'setGrailsApplication'] + private static final List<String> ROOT_TYPES = ['java.lang.Object', 'java.lang.Record', 'groovy.lang.GroovyObject'] + private static final Map<String, String> WRAPPERS = [ + 'boolean': 'java.lang.Boolean', 'byte': 'java.lang.Byte', 'char': 'java.lang.Character', + 'double': 'java.lang.Double', 'float': 'java.lang.Float', 'int': 'java.lang.Integer', + 'long': 'java.lang.Long', 'short': 'java.lang.Short' + ] + + @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() + Map overlay = readOverlay() + 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 + } + addDelegatedProperties(model, models, overlay, groups, properties) + } else { + GenerateConfigurationMetadataTask.addProperties( + model, model.prefix, model.name, models, groups, properties, new LinkedHashSet<String>()) + } + } + + Map<String, Object> metadata = merge(groups, properties, overlay) + 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 = [:] + Map<String, File> origins = [:] + 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) { + throw new IllegalArgumentException( + "Duplicate compiled class '${model.name}' in configuration metadata inputs " + + "(first seen in '${origins[model.name]}', also in '${directory}')") + } + origins[model.name] = directory + } + } finally { + paths.close() + } + } + readPayloads(models) + models + } + + /** + * Payloads are honoured only for a compiled class that still carries the annotation, so a payload left + * behind by a deleted or no longer annotated source never leaks into the metadata. + */ + private void readPayloads(Map<String, ClassModel> models) { + classesDirs.files.collect { File directory -> new File(directory, PAYLOAD_DIRECTORY) } + .findAll { File directory -> directory.isDirectory() } + .sort { File directory -> directory.absolutePath }.each { File directory -> + directory.listFiles().findAll { File file -> file.isFile() && file.name.endsWith('.json') } + .sort { File file -> file.name }.each { File file -> + ClassModel model = models[file.name - '.json'] Review Comment: Good catch, confirmed. `String.minus` ate the `.json` package segment, so the `JsonViewConfiguration` payload was written but never applied, and the ASM fallback masked it. Fixed by stripping only the extension: ```groovy ClassModel model = models[file.name.replaceFirst(/\.json$/, '')] ``` Added the fixture you suggested: `"applies the payload of a Groovy class whose package contains a json segment"` compiles `fixture.json.view.SegmentConfiguration` and `fixture.view.SegmentConfiguration` in the same module and asserts that each one gets its own payload, using the compile-time `defaultValue` as the evidence since the bytecode fallback cannot produce it. That covers both the silently dropped payload and the cross-application hazard. The spec fails against the old lookup. The generated metadata for all five migrated modules is byte-identical before and after, which matches what you saw — but gson now gets there through the payload, so the `@NestedConfigurationProperty` on `JsonViewConfiguration.generator` and the `@Delegate` handling are actually exercised. ########## build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/ConfigurationMetadataPlugin.groovy: ########## @@ -0,0 +1,760 @@ +/* + * 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.bundling.Jar +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.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') + } Review Comment: Confirmed, I reproduced the `0` for `DataBindingConfigurationProperties`. `groovyOptions.parameters` only covers the Groovy sources; the javac side of joint compilation reads `GroovyCompile.options.compilerArgs`. `CompilePlugin` now adds `-parameters` there, next to the existing `groovyOptions.parameters`, as you suggested. After the change that class carries 6 `MethodParameters` entries. I kept the block in the metadata plugin rather than removing it, and extended it to `GroovyCompile` as well. It is redundant wherever `org.apache.grails.buildsrc.compile` is applied, but the metadata plugin reads constructor binding from `MethodParameters`, so I would rather it not depend silently on another plugin being present — its own TestKit fixture applies it standalone. Both sides are guarded with a `contains` check, so the flag is never passed twice regardless of plugin order. New coverage: `"binds the constructor of a Java class that is joint-compiled from the Groovy source directory"` (a constructor-bound `.java` under `src/main/groovy`), and a `CompilePluginSpec` case asserting the `GroovyCompile` compiler args. ########## build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/ConfigurationMetadataPlugin.groovy: ########## @@ -0,0 +1,760 @@ +/* + * 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.bundling.Jar +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.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) + } + // the per-class payloads are build-time input of generateConfigurationMetadata only + project.tasks.withType(Jar).configureEach { Jar task -> + task.exclude("${GenerateConfigurationMetadataTask.PAYLOAD_DIRECTORY}/**") + } + } + } +} + +@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_DIRECTORY = 'META-INF/grails-configuration-metadata' + private static final List<String> FRAMEWORK_ACCESSORS = ['getMetaClass', 'setMetaClass', 'setGrailsApplication'] + private static final List<String> ROOT_TYPES = ['java.lang.Object', 'java.lang.Record', 'groovy.lang.GroovyObject'] + private static final Map<String, String> WRAPPERS = [ + 'boolean': 'java.lang.Boolean', 'byte': 'java.lang.Byte', 'char': 'java.lang.Character', + 'double': 'java.lang.Double', 'float': 'java.lang.Float', 'int': 'java.lang.Integer', + 'long': 'java.lang.Long', 'short': 'java.lang.Short' + ] + + @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() + Map overlay = readOverlay() + 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 + } + addDelegatedProperties(model, models, overlay, groups, properties) + } else { + GenerateConfigurationMetadataTask.addProperties( + model, model.prefix, model.name, models, groups, properties, new LinkedHashSet<String>()) + } + } + + Map<String, Object> metadata = merge(groups, properties, overlay) + 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 = [:] + Map<String, File> origins = [:] + 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) { + throw new IllegalArgumentException( + "Duplicate compiled class '${model.name}' in configuration metadata inputs " + + "(first seen in '${origins[model.name]}', also in '${directory}')") + } + origins[model.name] = directory + } + } finally { + paths.close() + } + } + readPayloads(models) + models + } + + /** + * Payloads are honoured only for a compiled class that still carries the annotation, so a payload left + * behind by a deleted or no longer annotated source never leaks into the metadata. + */ + private void readPayloads(Map<String, ClassModel> models) { + classesDirs.files.collect { File directory -> new File(directory, PAYLOAD_DIRECTORY) } + .findAll { File directory -> directory.isDirectory() } + .sort { File directory -> directory.absolutePath }.each { File directory -> + directory.listFiles().findAll { File file -> file.isFile() && file.name.endsWith('.json') } + .sort { File file -> file.name }.each { File file -> + ClassModel model = models[file.name - '.json'] + if (model?.prefix != null) { + GenerateConfigurationMetadataTask.applyPayload(model, file.getText(StandardCharsets.UTF_8.name())) + } + } + } + } + + /** + * The compiler cannot see what a {@code @Delegate} field contributes, so its properties are taken from + * the delegate's compiled class. Whatever is not compiled by this project has to come from the overlay. + */ + protected void addDelegatedProperties(ClassModel model, Map<String, ClassModel> models, Map overlay, + List<Map<String, Object>> groups, List<Map<String, Object>> properties) { + model.payloadDelegates.each { Map<String, Object> delegate -> + ClassModel delegateModel = models[delegate.type as String] + if (delegateModel != null) { + List<Map<String, Object>> delegatedGroups = [] + List<Map<String, Object>> delegatedProperties = [] + GenerateConfigurationMetadataTask.addProperties(delegateModel, model.prefix, model.name, models, + delegatedGroups, delegatedProperties, new LinkedHashSet<String>()) + Set<String> known = (groups + properties).findAll { Map<String, Object> entry -> + entry.sourceType == model.name + }*.name as Set<String> + groups.addAll(delegatedGroups.findAll { Map<String, Object> entry -> !(entry.name in known) }) + properties.addAll(delegatedProperties.findAll { Map<String, Object> entry -> !(entry.name in known) }) + } + if (!GenerateConfigurationMetadataTask.fullyCompiledHere(delegateModel, models) && + !GenerateConfigurationMetadataTask.overlayDocuments(overlay, model.prefix, properties)) { + logger.warn("Configuration properties class '${model.name}' uses @Delegate field '${delegate.field}' " + + "of type '${delegate.type}', which is not entirely compiled by this project. " + + 'Add metadata for the delegated properties to additional-spring-configuration-metadata.json.') + } + } + } + + private static boolean fullyCompiledHere(ClassModel model, Map<String, ClassModel> models) { + ClassModel current = model + while (current != null) { + if (current.superName == null || current.superName in ROOT_TYPES) { + return true + } + current = models[current.superName] + } + false + } + + private static boolean overlayDocuments(Map overlay, String prefix, List<Map<String, Object>> generated) { + Set<String> generatedNames = generated*.name as Set<String> + String start = prefix ? "${prefix}." : '' + ((overlay.get('properties') ?: []) as List).any { Object entry -> + String name = entry instanceof Map ? ((Map) entry).name as String : null + name != null && name.startsWith(start) && !(name in generatedNames) + } + } + + 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 + 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) + // a generic Signature attribute leaves out the synthetic and mandated parameters that the + // descriptor and MethodParameters include, so those must not advance the type index + boolean implicitParametersOmitted = argumentTypeNames.size() != argumentTypes.length + return new MethodVisitor(Opcodes.ASM9) { + private int parameterIndex + private int implicitParameters + + @Override + void visitParameter(String parameterName, int parameterAccess) { + boolean implicit = (parameterAccess & (Opcodes.ACC_SYNTHETIC | Opcodes.ACC_MANDATED)) != 0 + int typeIndex = implicitParametersOmitted ? parameterIndex - implicitParameters : parameterIndex + if (parameterName && !implicit && typeIndex < argumentTypeNames.size()) { + constructor.properties[parameterName] = new PropertyModel( + name: parameterName, + type: argumentTypeNames[typeIndex], + constructorBound: true) + } + if (implicit) { + implicitParameters++ + } + 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('$') || + name in FRAMEWORK_ACCESSORS) { + 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) + + model.properties.values().each { PropertyModel property -> property.resolveAccessorType() } + 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 + } + + static void applyPayload(ClassModel model, String json) { + Map payload = new JsonSlurper().parseText(json) as Map + model.prefix = payload.get('prefix') 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) + } + model.payloadDelegates = ((payload.get('delegates') ?: []) as List).collect { Map delegate -> + new LinkedHashMap<String, Object>(delegate) + } + } + + private static void addAccessor(ClassModel model, String name, String descriptor, String signature, boolean writable) { + PropertyModel property = model.properties.computeIfAbsent(name) { new PropertyModel(name: name) } + String type = fieldType(descriptor, signature) + if (writable) { + property.setterTypes << type + } else { + property.getterType = property.getterType ?: type + } + 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 + } + bindableProperties(model, models, new LinkedHashSet<String>(visiting - model.name)).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 && !bindableProperties(nested, models, new LinkedHashSet<String>(visiting)).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) + } + + /** + * A property binds when it can be written, is a mutable container, or is a getter-only nested object that + * itself has something to bind. + */ + private static Map<String, PropertyModel> bindableProperties(ClassModel model, Map<String, ClassModel> models, + Set<String> visiting) { + if (!visiting.add(model.name)) { + return [:] + } + Map<String, PropertyModel> bindable = propertiesFor(model, models, new LinkedHashSet<String>()).findAll { + String name, PropertyModel property -> + if (property.writable || property.constructorBound || property.collectionOrMap()) { + return true + } + ClassModel nested = models[property.rawType()] + nested != null && !bindableProperties(nested, models, visiting).isEmpty() + } + visiting.remove(model.name) + bindable + } + + 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'] = mergeGroups(groups, (overlay.get('groups') ?: []) as List) + 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) { + Map<String, Object> indexed = [:] + source.each { Object entry -> + String name = entry instanceof Map ? ((Map) entry).name as String : entry as String + if (!name) { + throw new IllegalArgumentException("${category} entry has no name") + } + if (indexed.containsKey(name) && indexed[name] != entry) { Review Comment: Agreed, and the inconsistency with `mergeGroups` was the real problem. I went with your first option: properties are now identified the same way groups are. `mergeGroups` became `mergeSourced` and handles both, keyed by name + `sourceType` + `sourceMethod`. The metadata format explicitly allows repeated names, so both entries are kept. Overlay semantics carry over unchanged: a name-only entry augments every generated entry of that name, a source-qualified one targets its own. A conflict within the same identity is still fatal, and the message now names the source type: ``` Conflicting generated properties metadata for 'grails.foo.enabled' from source type 'A' ``` Both shapes you described are covered: `"preserves a property name repeated by two configuration classes sharing a prefix"` (also asserts the name-only vs. source-qualified overlay behaviour) and `"preserves a nested property that another configuration class binds by its own prefix"`. Both failed with the old code. One note: the docs generator in `grails-doc/build.gradle` and `ConfigReportCommand` index properties by name only, so a repeated name would render as two rows. No migrated module has one today, so I left those consumers alone for this PR. -- 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]
