Copilot commented on code in PR #6633: URL: https://github.com/apache/incubator-kie-drools/pull/6633#discussion_r3239869211
########## kie-no-external-managed-dependency-enforcer-rule/src/main/java/org/kie/noexternalmanageddependencyrule/NoExternalManagedDependencyRule.java: ########## @@ -0,0 +1,211 @@ +/* + * 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 + * + * http://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.kie.noexternalmanageddependencyrule; + +import java.io.File; +import java.io.PrintWriter; +import java.io.StringWriter; +import java.util.HashSet; +import java.util.Objects; + +import javax.inject.Inject; +import javax.inject.Named; + +import java.util.Set; +import org.apache.maven.enforcer.rule.api.AbstractEnforcerRule; +import org.apache.maven.enforcer.rule.api.EnforcerRuleException; +import org.apache.maven.execution.MavenSession; +import org.apache.maven.model.Dependency; +import org.apache.maven.model.DependencyManagement; +import org.apache.maven.model.Profile; +import org.apache.maven.project.MavenProject; +import org.apache.maven.project.ProjectBuilder; + +/** + * No External Managed Dependency Enforcer Rule + * This rule is meant to forbid declaration of managed dependencies that are not part of the current multi-module maven project. + */ +@Named("noExternalManagedDependencyRule") +public class NoExternalManagedDependencyRule extends AbstractEnforcerRule { + + // Inject needed Maven components + private final MavenProject project; + private final ProjectBuilder projectBuilder; + private final MavenSession mavenSession; + + /** + * Comma-separated set of group ids to check for + */ + private Set<String> filteredGroupIds; + + /** + * Comma-separated set of group:artifact that are allowed + */ + private Set<String> allowedGA; + /** + * The relative path to the root of the current multimodule project. + */ + private String rootPom; + + @Inject + public NoExternalManagedDependencyRule(MavenProject project, ProjectBuilder projectBuilder, MavenSession mavenSession) { + this.project = Objects.requireNonNull(project); + this.projectBuilder = Objects.requireNonNull(projectBuilder); + this.mavenSession = Objects.requireNonNull(mavenSession); + } + + public void execute() throws EnforcerRuleException { + if (rootPom == null || rootPom.isEmpty()) { + throw new EnforcerRuleException("The rootPom parameter is required and cannot be empty."); + } + if (filteredGroupIds == null || filteredGroupIds.isEmpty()) { + throw new EnforcerRuleException("The filteredGroupIds parameter is required and cannot be empty."); + } + if (project.getOriginalModel().getDependencyManagement() == null && (project.getOriginalModel().getProfiles() == null || project.getOriginalModel().getProfiles().stream().allMatch(profile -> profile.getDependencyManagement() == null))) { + // If there is no dependency management there is no need to check for managed dependencies + return; + } + checkForManagedDependencies(); + } + + private void checkForManagedDependencies() throws EnforcerRuleException { + MavenProject rootMavenProject = getRootMavenProject(); + Set<Dependency> implementedDependencies = getImplementedDependencies(rootMavenProject); + checkForManagedDependenciesInProject(implementedDependencies); + checkForManagedDependenciesInProfiles(implementedDependencies); + } + + private void checkForManagedDependenciesInProject(Set<Dependency> implementedDependencies) throws EnforcerRuleException { + Set<String> invalidManagedDependencies = invalidManagedDependencies(project.getOriginalModel().getDependencyManagement(), implementedDependencies); + if (!invalidManagedDependencies.isEmpty()) { + String invalidDependencies = String.join(",\r\n", invalidManagedDependencies); + throw new EnforcerRuleException(String.format("The current pom %s:%s:%s has the following invalid managed dependencies:\n" + + "%s", project.getGroupId(), project.getArtifactId(), project.getVersion(), invalidDependencies)); + } + } + + private void checkForManagedDependenciesInProfiles(Set<Dependency> implementedDependencies) throws EnforcerRuleException { + if (project.getOriginalModel().getProfiles() != null) { + for (Profile profile : project.getModel().getProfiles()) { + checkForManagedDependenciesInProfile(profile, implementedDependencies); + } + } Review Comment: In checkForManagedDependenciesInProfiles(), the loop iterates over project.getModel().getProfiles() even though the rule is meant to validate only dependencyManagement declared in the current POM (you already use getOriginalModel() elsewhere). This can end up validating inherited/effective profiles (and their dependencyManagement) and failing a module even when the module itself didn’t declare dependencyManagement in any profile. Iterate over project.getOriginalModel().getProfiles() (or alternatively check location/inheritance similarly to NoDependencyManagementRule) so only locally-declared profile dependencyManagement is validated. ########## kie-no-external-managed-dependency-enforcer-rule/src/main/java/org/kie/noexternalmanageddependencyrule/NoExternalManagedDependencyRule.java: ########## @@ -0,0 +1,211 @@ +/* + * 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 + * + * http://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.kie.noexternalmanageddependencyrule; + +import java.io.File; +import java.io.PrintWriter; +import java.io.StringWriter; +import java.util.HashSet; +import java.util.Objects; + +import javax.inject.Inject; +import javax.inject.Named; + +import java.util.Set; +import org.apache.maven.enforcer.rule.api.AbstractEnforcerRule; +import org.apache.maven.enforcer.rule.api.EnforcerRuleException; +import org.apache.maven.execution.MavenSession; +import org.apache.maven.model.Dependency; +import org.apache.maven.model.DependencyManagement; +import org.apache.maven.model.Profile; +import org.apache.maven.project.MavenProject; +import org.apache.maven.project.ProjectBuilder; + +/** + * No External Managed Dependency Enforcer Rule + * This rule is meant to forbid declaration of managed dependencies that are not part of the current multi-module maven project. + */ +@Named("noExternalManagedDependencyRule") +public class NoExternalManagedDependencyRule extends AbstractEnforcerRule { + + // Inject needed Maven components + private final MavenProject project; + private final ProjectBuilder projectBuilder; + private final MavenSession mavenSession; + + /** + * Comma-separated set of group ids to check for + */ + private Set<String> filteredGroupIds; + + /** + * Comma-separated set of group:artifact that are allowed + */ + private Set<String> allowedGA; + /** + * The relative path to the root of the current multimodule project. + */ + private String rootPom; + + @Inject + public NoExternalManagedDependencyRule(MavenProject project, ProjectBuilder projectBuilder, MavenSession mavenSession) { + this.project = Objects.requireNonNull(project); + this.projectBuilder = Objects.requireNonNull(projectBuilder); + this.mavenSession = Objects.requireNonNull(mavenSession); + } + + public void execute() throws EnforcerRuleException { + if (rootPom == null || rootPom.isEmpty()) { + throw new EnforcerRuleException("The rootPom parameter is required and cannot be empty."); + } + if (filteredGroupIds == null || filteredGroupIds.isEmpty()) { + throw new EnforcerRuleException("The filteredGroupIds parameter is required and cannot be empty."); + } + if (project.getOriginalModel().getDependencyManagement() == null && (project.getOriginalModel().getProfiles() == null || project.getOriginalModel().getProfiles().stream().allMatch(profile -> profile.getDependencyManagement() == null))) { + // If there is no dependency management there is no need to check for managed dependencies + return; + } + checkForManagedDependencies(); + } + + private void checkForManagedDependencies() throws EnforcerRuleException { + MavenProject rootMavenProject = getRootMavenProject(); + Set<Dependency> implementedDependencies = getImplementedDependencies(rootMavenProject); + checkForManagedDependenciesInProject(implementedDependencies); + checkForManagedDependenciesInProfiles(implementedDependencies); + } + + private void checkForManagedDependenciesInProject(Set<Dependency> implementedDependencies) throws EnforcerRuleException { + Set<String> invalidManagedDependencies = invalidManagedDependencies(project.getOriginalModel().getDependencyManagement(), implementedDependencies); + if (!invalidManagedDependencies.isEmpty()) { + String invalidDependencies = String.join(",\r\n", invalidManagedDependencies); + throw new EnforcerRuleException(String.format("The current pom %s:%s:%s has the following invalid managed dependencies:\n" + + "%s", project.getGroupId(), project.getArtifactId(), project.getVersion(), invalidDependencies)); + } + } + + private void checkForManagedDependenciesInProfiles(Set<Dependency> implementedDependencies) throws EnforcerRuleException { + if (project.getOriginalModel().getProfiles() != null) { + for (Profile profile : project.getModel().getProfiles()) { + checkForManagedDependenciesInProfile(profile, implementedDependencies); + } + } + } + + private void checkForManagedDependenciesInProfile(Profile profile, Set<Dependency> implementedDependencies) throws EnforcerRuleException { + Set<String> invalidManagedDependencies = invalidManagedDependencies(profile.getDependencyManagement(), implementedDependencies); + if (!invalidManagedDependencies.isEmpty()) { + String invalidDependencies = String.join(",\r\n", invalidManagedDependencies); + throw new EnforcerRuleException(String.format("The profile %s in the current pom %s:%s:%s has the following invalid managed dependencies:\r\n%s", profile.getId(), project.getGroupId(), + project.getArtifactId(), project.getVersion(), invalidDependencies)); + } + } + + private Set<String> invalidManagedDependencies(DependencyManagement dependencyManagement, Set<Dependency> implementedDependencies) { + Set<String> toReturn = new HashSet<>(); + if (dependencyManagement != null) { + for (Dependency dependency : dependencyManagement.getDependencies()) { + if (!isAllowedGA(dependency) && isFiltered(dependency) && isNotAllowed(dependency, implementedDependencies)) { + toReturn.add(String.format("%s:%s:%s", dependency.getGroupId(), dependency.getArtifactId(), dependency.getVersion())); + } + } + } + return toReturn; + } + + private boolean isAllowedGA(Dependency dependency) { + return allowedGA.stream() + .anyMatch(allowedGA -> allowedGA.equals(dependency.getGroupId() + ":" + dependency.getArtifactId())); + } + + private boolean isFiltered(Dependency dependency) { + return filteredGroupIds.contains(dependency.getGroupId()); + } Review Comment: isAllowedGA() calls allowedGA.stream() without null/emptiness handling. In the enforcer configuration, <allowedGA>${allowed_GA}</allowedGA> can resolve to an empty value, which may inject as null and cause a NullPointerException before any filtering logic runs. Treat null as an empty set (or guard in isAllowedGA / invalidManagedDependencies) so the rule behaves correctly when no exceptions are configured. ########## kie-maven-plugin/src/it/kie-maven-plugin-test-kjar-setup/kie-maven-plugin-test-kjar-parent/pom.xml: ########## @@ -40,6 +40,7 @@ <compiler.plugin.version>3.13.0</compiler.plugin.version> <surefire.plugin.version>3.2.5</surefire.plugin.version> <failsafe.plugin.version>3.2.5</failsafe.plugin.version> + <buildlog.directory>@path.to.buildlog.directory@</buildlog.directory> </properties> Review Comment: This property introduces an invoker-filter token (@path.to.buildlog.directory@). Ensure the invoking build actually filters/provides this value; otherwise buildlog.directory will be set to the literal token string and tests that read build logs will fail. Either pass path.to.buildlog.directory via maven-invoker-plugin <filterProperties> or avoid using an unfiltered @...@ placeholder here. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
