codeconsole commented on code in PR #16428: URL: https://github.com/apache/grails-core/pull/16428#discussion_r4134583504
########## build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/ParentBomVersions.groovy: ########## @@ -0,0 +1,142 @@ +/* + * 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 java.util.function.Function + +import groovy.transform.CompileStatic + +import org.apache.maven.model.Dependency +import org.apache.maven.model.Model +import org.apache.maven.model.Parent + +/** + * Everything a set of parent BOMs manages, including what they manage through the BOMs they + * import, together with the parent-level property that controls each version. + * + * <p>A version a parent manages through an imported BOM is attributed to the property the parent + * imports that BOM with - {@code jackson-2-bom.version} for everything + * {@code com.fasterxml.jackson:jackson-bom} manages, not the property that BOM uses internally - + * because that is the property a consumer sets to move the whole family. A version the parent + * writes literally has no property, and is recorded with an empty one.</p> + * + * <p>When two entries manage the same module, the first one wins, with a BOM's own entries ahead + * of the entries it imports, matching Maven's {@code <dependencyManagement>} resolution.</p> + * + * @since 8.0 + */ +@CompileStatic +class ParentBomVersions { + + private static final int MAX_PARENT_DEPTH = 10 + + /** {@code group:artifact} to the version the parents manage it at. */ + final Map<String, String> versions = new LinkedHashMap<>() + + /** {@code group:artifact} to the parent-level property controlling its version, or empty. */ + final Map<String, String> properties = new LinkedHashMap<>() + + private final Function<String, File> pomResolver + private final Set<String> visited = new HashSet<>() + private final Map<String, Map<String, String>> propertiesByPom = [:] + + private ParentBomVersions(Function<String, File> pomResolver) { + this.pomResolver = pomResolver + } + + /** + * @param parentBoms the parent BOMs as {@code group:artifact:version}, in precedence order + * @param pomResolver returns the POM file for a {@code group:artifact:version} + */ + static ParentBomVersions resolve(Collection<String> parentBoms, Function<String, File> pomResolver) { + ParentBomVersions result = new ParentBomVersions(pomResolver) + for (String parentBom : parentBoms) { + result.collect(parentBom, null) + } + result + } + + /** + * @param controllingProperty the parent-level property the BOM was imported with, empty when it + * was imported at a literal version, or {@code null} for a parent BOM itself + */ + private void collect(String coordinates, String controllingProperty) { + if (!visited.add(coordinates)) { + return + } + Model bom = PomVersions.read(pomResolver.apply(coordinates)) + Map<String, String> bomProperties = effectiveProperties(bom, 0) + + List<Dependency> imports = [] + for (Dependency dependency : PomVersions.managedDependencies(bom)) { + if (!dependency.groupId || !dependency.artifactId) { + continue + } + if (PomVersions.isImport(dependency)) { + imports.add(dependency) + continue + } + record(dependency, bomProperties, controllingProperty) + } + + for (Dependency imported : imports) { + String version = record(imported, bomProperties, controllingProperty) + if (version) { + collect("${PomVersions.key(imported)}:${version}" as String, controllingPropertyOf(imported, controllingProperty)) + } + } + } + + private String record(Dependency dependency, Map<String, String> bomProperties, String controllingProperty) { + String version = PomVersions.interpolate(dependency.version, bomProperties) + String key = PomVersions.key(dependency) Review Comment: [P2] Interpolate dependency coordinates before indexing parent versions `MavenXpp3Reader` leaves expressions in `groupId` and `artifactId` unresolved, but this key is stored without interpolation. This already affects BOMs imported by Spring Boot 4.1.1: Kotlin, Brave, Zipkin Reporter, and Querydsl declare managed dependencies with `<groupId>${project.groupId}</groupId>`. Their entries are indexed as `${project.groupId}:kotlin-stdlib`, for example, so a Grails pin for `org.jetbrains.kotlin:kotlin-stdlib` never matches `parentVersions` and silently bypasses both the property-name and redundant-version checks. I reproduced this through `validateBomProperties` using a local parent BOM managing `${project.groupId}:lib` at `${lib.version}=1.0` and a published constraint for `test:lib:1.0` under `${wrong.version}`: validation succeeds. Replacing only the parent's group ID expression with literal `test` makes it correctly report both violations. Please interpolate the group/artifact coordinates against the effective POM properties before constructing the lookup key (and imported-BOM resolution coordinates), and cover this case in the functional spec. -- 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]
