borinquenkid commented on code in PR #16048:
URL: https://github.com/apache/grails-core/pull/16048#discussion_r3887776700


##########
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/GroovyDslConfigurationMetadataParser.groovy:
##########
@@ -0,0 +1,207 @@
+/*
+ *  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 org.codehaus.groovy.ast.ModuleNode
+import org.codehaus.groovy.ast.expr.ArgumentListExpression
+import org.codehaus.groovy.ast.expr.BinaryExpression
+import org.codehaus.groovy.ast.expr.ClassExpression
+import org.codehaus.groovy.ast.expr.ClosureExpression
+import org.codehaus.groovy.ast.expr.ConstantExpression
+import org.codehaus.groovy.ast.expr.Expression
+import org.codehaus.groovy.ast.expr.ListExpression
+import org.codehaus.groovy.ast.expr.MapExpression
+import org.codehaus.groovy.ast.expr.MethodCallExpression
+import org.codehaus.groovy.ast.expr.PropertyExpression
+import org.codehaus.groovy.ast.expr.TernaryExpression
+import org.codehaus.groovy.ast.expr.VariableExpression
+import org.codehaus.groovy.ast.stmt.BlockStatement
+import org.codehaus.groovy.ast.stmt.ExpressionStatement
+import org.codehaus.groovy.ast.stmt.IfStatement
+import org.codehaus.groovy.ast.stmt.Statement
+import org.codehaus.groovy.control.CompilationFailedException
+import org.codehaus.groovy.control.SourceUnit
+import org.codehaus.groovy.syntax.Types
+
+import java.nio.charset.StandardCharsets
+
+/** Extracts configuration metadata from Groovy DSL source without evaluating 
source code. */
+final class GroovyDslConfigurationMetadataParser {
+
+    private GroovyDslConfigurationMetadataParser() {
+    }
+
+    static List<Map<String, Object>> parse(Collection<File> files, Map<String, 
String> rootPrefixes) {
+        List<Map<String, Object>> properties = []
+        files.findAll { File file -> file.isFile() }.sort { File file -> 
file.absolutePath }.each { File file ->
+            parseFile(file, rootPrefixes, properties)
+        }
+        properties.sort { Map<String, Object> property -> property.name as 
String }
+    }
+
+    private static void parseFile(File file, Map<String, String> rootPrefixes,
+                                  List<Map<String, Object>> properties) {
+        ModuleNode module = parseSource(file)
+        module.statementBlock.statements.each { Statement statement ->
+            MethodCallExpression call = methodCall(statement)
+            String root = call?.methodAsString
+            ClosureExpression closure = call == null ? null : 
closureArgument(call)
+            String prefix = root == null ? null : rootPrefixes[root]
+            if (prefix != null && closure != null) {
+                parseStatements(closure.code, prefix, false, properties)
+            }
+        }
+    }
+
+    private static ModuleNode parseSource(File file) {
+        try {
+            SourceUnit source = SourceUnit.create(file.absolutePath, 
file.getText(StandardCharsets.UTF_8.name()))
+            source.parse()
+            source.completePhase()
+            source.nextPhase()
+            source.convert()
+            source.errorCollector.failIfErrors()
+            source.AST
+        } catch (CompilationFailedException exception) {
+            throw new IllegalArgumentException("Failed to parse Groovy DSL 
source '${file.absolutePath}'", exception)
+        }
+    }
+
+    private static void parseStatements(Statement statement, String prefix, 
boolean conditional,
+                                        List<Map<String, Object>> properties) {
+        if (statement instanceof BlockStatement) {
+            statement.statements.each { Statement child -> 
parseStatements(child, prefix, conditional, properties) }
+        } else if (statement instanceof IfStatement) {
+            parseStatements(statement.ifBlock, prefix, true, properties)
+            parseStatements(statement.elseBlock, prefix, true, properties)
+        } else if (statement instanceof ExpressionStatement) {
+            Expression expression = statement.expression
+            if (expression instanceof BinaryExpression && 
expression.operation.type == Types.ASSIGN) {
+                addAssignment(expression, prefix, conditional, properties)
+            } else if (expression instanceof MethodCallExpression) {
+                ClosureExpression closure = closureArgument(expression)
+                String nestedName = expression.methodAsString
+                if (closure != null && nestedName != null) {
+                    parseStatements(closure.code, "${prefix}.${nestedName}", 
conditional, properties)

Review Comment:
   Fixed in 6a30571fa3 — an `environments{}` call is now recognized the way 
`IfStatement` already is: each named environment's closure is parsed under the 
unchanged prefix (not folded in as `<prefix>.environments.<name>.*`) and marked 
conditional, so it reconciles through the same merge logic as if/else branches. 
Added a spec covering `environments { production { ... } test { ... } }` 
asserting the property lands at the unprefixed path and no `.environments.` 
name is emitted.



##########
grails-spring-security/plugin/src/test/groovy/grails/plugin/springsecurity/ConfigurationMetadataSpec.groovy:
##########
@@ -0,0 +1,62 @@
+/*
+ *  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 grails.plugin.springsecurity
+
+import groovy.json.JsonSlurper
+import spock.lang.Specification
+
+class ConfigurationMetadataSpec extends Specification {
+
+    void 'generates merged configuration metadata from 
DefaultSecurityConfig'() {
+        given:
+        Map metadata = configurationMetadata()
+        Map<String, Map> properties = 
metadata.get('properties').collectEntries { Map property -> 
[(property.get('name')): property] }
+
+        expect: 'the complete curated metadata contract is retained'
+        metadata.get('groups').size() == 14
+        properties.size() == 145

Review Comment:
   Fixed in 013ba6c143 — replaced the hardcoded counts with an assertion that 
every group/property name in the curated overlay is present in the canonical 
generated metadata, reading the overlay off the classpath the same way the 
generated file already is (same disambiguation-by-declared-group pattern, since 
several modules now publish a same-named overlay resource). A missing entry now 
shows up by name in a `(list - list).empty` failure instead of a bare count 
mismatch. Verified two ways: it still passes against the real 
`DefaultSecurityConfig.groovy`, and it doesn't break when I temporarily added a 
146th curated property (which the old `properties.size() == 145` would have 
failed on) — reverted after confirming.



-- 
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]

Reply via email to