slawekjaranowski commented on code in PR #362: URL: https://github.com/apache/maven-dependency-plugin/pull/362#discussion_r1536078311
########## src/it/projects/analyze-invalid-exclude/pom.xml: ########## @@ -0,0 +1,95 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + ~ 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. + --> + +<project xmlns="http://maven.apache.org/POM/4.0.0"; + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd";> + <modelVersion>4.0.0</modelVersion> + + <groupId>org.apache.maven.its.dependency</groupId> + <artifactId>test</artifactId> + <version>1.0-SNAPSHOT</version> + + <name>Test</name> + <description> + Test dependency:analyze-exclusion + </description> + + <properties> + <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> + </properties> + + <dependencies> + <dependency> + <groupId>org.apache.maven</groupId> + <artifactId>maven-project</artifactId> + <version>2.0.6</version> Review Comment: even in test please use more actual versions ########## src/test/java/org/apache/maven/plugins/dependency/exclusion/AnalyzeExclusionsMojoTest.java: ########## @@ -0,0 +1,365 @@ +/* + * 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.apache.maven.plugins.dependency.exclusion; + +import java.io.File; +import java.io.PrintWriter; +import java.io.StringWriter; +import java.util.ArrayList; +import java.util.List; + +import org.apache.maven.artifact.Artifact; +import org.apache.maven.execution.MavenSession; +import org.apache.maven.model.Dependency; +import org.apache.maven.model.Exclusion; +import org.apache.maven.plugin.LegacySupport; +import org.apache.maven.plugin.MojoExecutionException; +import org.apache.maven.plugin.logging.Log; +import org.apache.maven.plugins.dependency.AbstractDependencyMojoTestCase; +import org.apache.maven.project.MavenProject; +import org.apache.maven.project.ProjectBuilder; +import org.apache.maven.project.ProjectBuildingResult; + +import static com.google.common.collect.Lists.newArrayList; +import static com.google.common.collect.Sets.newHashSet; Review Comment: Please don't use guava, technically it is available by transitive dependencies ########## src/main/java/org/apache/maven/plugins/dependency/exclusion/Coordinates.java: ########## @@ -0,0 +1,101 @@ +/* + * 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.apache.maven.plugins.dependency.exclusion; + +import java.lang.reflect.Proxy; +import java.nio.file.FileSystems; +import java.nio.file.Path; +import java.nio.file.PathMatcher; +import java.util.Objects; +import java.util.function.Predicate; + +/** + * Simple "record" to hold the coordinates of the dependency which is excluded. + * <p> + * When dealing with exclusions the version is not important. Only groupId and artifactId is used. + * </p> + */ +class Coordinates { + private final String groupId; + private final String artifactId; + + private Coordinates(String groupId, String artifactId) { + this.groupId = groupId; + this.artifactId = artifactId; + } + + public static Coordinates coordinates(String groupId, String artifactId) { + return new Coordinates(groupId, artifactId); + } + + public String getGroupId() { + return groupId; + } + + public String getArtifactId() { + return artifactId; + } + + Predicate<Coordinates> getExclusionPattern() { + PathMatcher groupId = FileSystems.getDefault().getPathMatcher("glob:" + getGroupId()); + PathMatcher artifactId = FileSystems.getDefault().getPathMatcher("glob:" + getArtifactId()); + Predicate<Coordinates> predGroupId = a -> groupId.matches(createPathProxy(a.getGroupId())); + Predicate<Coordinates> predArtifactId = a -> artifactId.matches(createPathProxy(a.getArtifactId())); + + return predGroupId.and(predArtifactId); + } + + /** + * In order to reuse the glob matcher from the filesystem, we need + * to create Path instances. Those are only used with the toString method. + * This hack works because the only system-dependent thing is the path + * separator which should not be part of the groupId or artifactId. + */ Review Comment: To think if we really need glob patterns for FS. To check how it is implemented in other mojos. ########## src/main/java/org/apache/maven/plugins/dependency/exclusion/ExclusionChecker.java: ########## @@ -0,0 +1,45 @@ +/* + * 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.apache.maven.plugins.dependency.exclusion; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import static java.util.stream.Collectors.toList; + +class ExclusionChecker { + + private final Map<Coordinates, List<Coordinates>> violations = new HashMap<>(); + + Map<Coordinates, List<Coordinates>> getViolations() { + return violations; + } + + void check(Coordinates artifact, final Set<Coordinates> excludes, final Set<Coordinates> actualDependencies) { Review Comment: `final` is not needed ########## src/test/java/org/apache/maven/plugins/dependency/exclusion/AnalyzeExclusionsMojoTest.java: ########## @@ -0,0 +1,365 @@ +/* + * 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.apache.maven.plugins.dependency.exclusion; + +import java.io.File; +import java.io.PrintWriter; +import java.io.StringWriter; +import java.util.ArrayList; +import java.util.List; + +import org.apache.maven.artifact.Artifact; +import org.apache.maven.execution.MavenSession; +import org.apache.maven.model.Dependency; +import org.apache.maven.model.Exclusion; +import org.apache.maven.plugin.LegacySupport; +import org.apache.maven.plugin.MojoExecutionException; +import org.apache.maven.plugin.logging.Log; +import org.apache.maven.plugins.dependency.AbstractDependencyMojoTestCase; +import org.apache.maven.project.MavenProject; +import org.apache.maven.project.ProjectBuilder; +import org.apache.maven.project.ProjectBuildingResult; + +import static com.google.common.collect.Lists.newArrayList; +import static com.google.common.collect.Sets.newHashSet; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.anyBoolean; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class AnalyzeExclusionsMojoTest extends AbstractDependencyMojoTestCase { + + AnalyzeExclusionsMojo mojo; + MavenProject project; + private TestLog testLog; + + @Override + public void setUp() throws Exception { + super.setUp("analyze-exclusions", true, false); + File testPom = new File(getBasedir(), "target/test-classes/unit/analyze-exclusions/plugin-config.xml"); + mojo = (AnalyzeExclusionsMojo) lookupMojo("analyze-exclusions", testPom); + assertNotNull(mojo); + project = (MavenProject) getVariableValueFromObject(mojo, "project"); + MavenSession session = newMavenSession(project); + setVariableValueToObject(mojo, "session", session); + + LegacySupport legacySupport = lookup(LegacySupport.class); + legacySupport.setSession(session); + installLocalRepository(legacySupport); + + testLog = new TestLog(); + mojo.setLog(testLog); + } + + public void test_shall_throw_exception_when_fail_on_warning() throws Exception { Review Comment: Please preserve convention of methods name like in other tests in project -- 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: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
