sbglasius commented on code in PR #16047: URL: https://github.com/apache/grails-core/pull/16047#discussion_r4057736350
########## 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: **`-parameters` never reaches the sources that need it, and is redundant where it does.** All five migrated modules keep their `.java` sources under `src/main/groovy` (none of them has a `src/main/java`), so those files are joint-compiled by `GroovyCompile`, not `JavaCompile`. `groovyOptions.parameters = true` only covers Groovy sources; the javac side of joint compilation takes its flags from `GroovyCompile.options.compilerArgs`. Measured on this branch: ``` $ javap -v .../groovy/main/org/grails/plugins/databinding/DataBindingConfigurationProperties.class | grep -c MethodParameters 0 $ javap -v .../groovy/main/grails/plugin/cache/CachePluginConfiguration.class | grep -c MethodParameters 7 ``` So for a Java `@ConfigurationProperties` class in these modules, `visitParameter` never fires, `constructor.properties` stays empty, `bindingConstructor` resolves to `null`, and every constructor-bound property is silently omitted — which is exactly the "immutable constructor-bound properties are supported" case in the PR description. Separately, this block is a no-op even where `JavaCompile` does run: `CompilePlugin.groovy:101` already adds `-parameters` to every `JavaCompile` in the build, and every one of these modules applies `org.apache.grails.buildsrc.compile`. If constructor binding is meant to work for joint-compiled Java, this needs to configure `GroovyCompile.options.compilerArgs` instead (ideally in `CompilePlugin`, next to the existing `groovyOptions.parameters`). ########## 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: **The payload lookup strips the first `.json`, not the extension.** Groovy's `String.minus(String)` removes the *first* occurrence, so for `grails.plugin.json.view.JsonViewConfiguration.json` this evaluates to `grails.plugin.view.JsonViewConfiguration` — the `.json` package segment is eaten. `models[...]` is then `null` and `applyPayload` is never called, so the payload is silently discarded. This is happening right now on this branch: the payload file is written correctly to `grails-views-gson/build/classes/groovy/main/META-INF/grails-configuration-metadata/grails.plugin.json.view.JsonViewConfiguration.json`, but it never reaches the model. The gson output only looks right because the ASM fallback happens to cover that class — I confirmed the generated file is byte-identical after fixing the lookup. What is being skipped for any such class: - the payload's compile-time `defaultValue`s, - the `@NestedConfigurationProperty` this PR adds to `JsonViewConfiguration.generator` (the nesting currently comes from the ASM heuristic instead, so the annotation is untested here), - all `@Delegate` handling — `addDelegatedProperties` and the "not entirely compiled by this project" warning never run. There is also a correctness hazard rather than just a gap: with both `a.b.json.C` and `a.b.C` compiled in one module, the payload of the former is applied to the model of the latter, overwriting its prefix, groups and properties. Suggested fix: ```groovy ClassModel model = models[file.name.substring(0, file.name.length() - '.json'.length())] ``` Worth adding a `ConfigurationMetadataPluginSpec` fixture in a package containing a `json` segment — none of the current cases would catch this. ########## 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: **Two config classes sharing a prefix abort the build on the properties path.** `mergeGroups` deliberately supports a shared prefix — there is a spec for it, `"preserves repeated group names from two configuration classes sharing a prefix"`. The properties path does not: entries carry `sourceType`, so two generated properties with the same `name` but different `sourceType` are unequal and this throws. Concretely, if classes `A` and `B` both use prefix `grails.foo` and both expose `enabled`, `generate()` emits ```groovy [name: 'grails.foo.enabled', type: ..., sourceType: 'A'] [name: 'grails.foo.enabled', type: ..., sourceType: 'B'] ``` and the build fails with `Conflicting generated properties metadata for 'grails.foo.enabled'` — a message that reads like an authoring error in `additional-spring-configuration-metadata.json`, pointing the reader at the wrong file. The same shape arises without a literal duplicate prefix: class `A` with prefix `grails.x` and a nested property `b`, alongside class `B` annotated `@ConfigurationProperties('grails.x.b')`. Either de-duplicate generated properties by identity the way `mergeGroups` does (name + sourceType), or keep it fatal but say which two source types collided. -- 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]
