Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MENFORCER-469] Fix banTransitiveDependencies and transitive dependen… #253

Merged
merged 1 commit into from
Mar 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public void execute() throws EnforcerRuleException {
}
} else {
StringBuilder messageBuilder = new StringBuilder();
DependencyNode rootNode = resolveUtil.resolveTransitiveDependencies();
DependencyNode rootNode = resolveUtil.resolveTransitiveDependenciesVerbose();
if (!validate(rootNode, 0, messageBuilder)) {
String message = "";
if (getMessage() != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public DependencyConvergence(ResolveUtil resolveUtil) {
@Override
public void execute() throws EnforcerRuleException {

DependencyNode node = resolveUtil.resolveTransitiveDependencies(
DependencyNode node = resolveUtil.resolveTransitiveDependenciesVerbose(
// TODO: use a modified version of ExclusionDependencySelector to process excludes and includes
new DependencySelector() {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public void setIncludes(List<String> includes) {

@Override
public void execute() throws EnforcerRuleException {
DependencyNode node = resolveUtil.resolveTransitiveDependencies();
DependencyNode node = resolveUtil.resolveTransitiveDependenciesVerbose();
upperBoundDepsVisitor = new RequireUpperBoundDepsVisitor()
.setUniqueVersions(uniqueVersions)
.setIncludes(includes);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,9 @@
import org.eclipse.aether.graph.DependencyNode;
import org.eclipse.aether.util.graph.manager.DependencyManagerUtils;
import org.eclipse.aether.util.graph.selector.AndDependencySelector;
import org.eclipse.aether.util.graph.selector.ExclusionDependencySelector;
import org.eclipse.aether.util.graph.selector.OptionalDependencySelector;
import org.eclipse.aether.util.graph.selector.ScopeDependencySelector;
import org.eclipse.aether.util.graph.transformer.ConflictResolver;

import static java.util.Optional.ofNullable;
import static org.apache.maven.artifact.Artifact.SCOPE_PROVIDED;
import static org.apache.maven.artifact.Artifact.SCOPE_TEST;

/**
* Resolver helper class.
Expand All @@ -55,6 +50,7 @@
class ResolveUtil {

private final RepositorySystem repositorySystem;

private final MavenSession session;

/**
Expand All @@ -66,6 +62,24 @@ class ResolveUtil {
this.session = Objects.requireNonNull(session);
}

/**
* Retrieves the {@link DependencyNode} instance containing the result of the transitive dependency
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what "the result of the transitive dependency" is exactly. Is this just the transitive dependencies of the current maven project? Consider rephrasing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it is for current maven project

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rewrite to clarify this

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please look at next lines - there is:

Retrieves the {@link DependencyNode} instance containing the result of the transitive dependency
for the current {@link MavenProject} in verbose mode.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"the result of the transitive dependency" is unclear. Transitive dependencies are not process and wouldn't normally be thought of as having results.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So ... can you propose description of this method?

* for the current {@link MavenProject} in verbose mode.
* <p>
* In verbose mode all nodes participating in a conflict are retained.
* </p>
* <p>
* Please consult {@link ConflictResolver} and {@link DependencyManagerUtils}>
* </p>
*
* @param selectors zero or more {@link DependencySelector} instances
* @return a Dependency Node which is the root of the project's dependency tree
* @throws EnforcerRuleException thrown if the lookup fails
*/
DependencyNode resolveTransitiveDependenciesVerbose(DependencySelector... selectors) throws EnforcerRuleException {
return resolveTransitiveDependencies(true, selectors);
}

/**
* Retrieves the {@link DependencyNode} instance containing the result of the transitive dependency
* for the current {@link MavenProject}.
Expand All @@ -75,23 +89,28 @@ class ResolveUtil {
* @throws EnforcerRuleException thrown if the lookup fails
*/
DependencyNode resolveTransitiveDependencies(DependencySelector... selectors) throws EnforcerRuleException {
if (selectors.length == 0) {
selectors = new DependencySelector[] {
new ScopeDependencySelector(SCOPE_TEST, SCOPE_PROVIDED),
new OptionalDependencySelector(),
new ExclusionDependencySelector()
};
}
return resolveTransitiveDependencies(false, selectors);
}

private DependencyNode resolveTransitiveDependencies(boolean verbose, DependencySelector... selectors)
throws EnforcerRuleException {

try {
MavenProject project = session.getCurrentProject();
ArtifactTypeRegistry artifactTypeRegistry =
session.getRepositorySession().getArtifactTypeRegistry();

DefaultRepositorySystemSession repositorySystemSession =
new DefaultRepositorySystemSession(session.getRepositorySession());
repositorySystemSession.setConfigProperty(ConflictResolver.CONFIG_PROP_VERBOSE, true);
repositorySystemSession.setConfigProperty(DependencyManagerUtils.CONFIG_PROP_VERBOSE, true);
repositorySystemSession.setDependencySelector(new AndDependencySelector(selectors));

if (selectors.length > 0) {
repositorySystemSession.setDependencySelector(new AndDependencySelector(selectors));
}

if (verbose) {
repositorySystemSession.setConfigProperty(ConflictResolver.CONFIG_PROP_VERBOSE, true);
repositorySystemSession.setConfigProperty(DependencyManagerUtils.CONFIG_PROP_VERBOSE, true);
}

CollectRequest collectRequest = new CollectRequest(
project.getDependencies().stream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ void excludesAndIncludesDoNotUseTransitiveDependencies() throws Exception {
@Test
void excludesUseTransitiveDependencies() throws Exception {

when(resolveUtil.resolveTransitiveDependencies())
when(resolveUtil.resolveTransitiveDependenciesVerbose())
.thenReturn(new DependencyNodeBuilder()
.withType(DependencyNodeBuilder.Type.POM)
.withChildNode(new DependencyNodeBuilder()
Expand Down Expand Up @@ -127,7 +127,7 @@ void excludesUseTransitiveDependencies() throws Exception {
@Test
void excludesAndIncludesUseTransitiveDependencies() throws Exception {

when(resolveUtil.resolveTransitiveDependencies())
when(resolveUtil.resolveTransitiveDependenciesVerbose())
.thenReturn(new DependencyNodeBuilder()
.withType(DependencyNodeBuilder.Type.POM)
.withChildNode(new DependencyNodeBuilder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ void testSearchNonTransitive() throws IOException {

@Test
void testSearchTransitiveMultipleFailures() throws Exception {
when(resolveUtil.resolveTransitiveDependencies()).thenReturn(getDependencyNodeWithMultipleSnapshots());
when(resolveUtil.resolveTransitiveDependenciesVerbose()).thenReturn(getDependencyNodeWithMultipleSnapshots());
rule.setSearchTransitive(true);

assertThatCode(rule::execute)
Expand All @@ -94,7 +94,7 @@ void testSearchTransitiveMultipleFailures() throws Exception {
@Test
void testSearchTransitiveNoFailures() throws Exception {
when(session.getCurrentProject()).thenReturn(project);
when(resolveUtil.resolveTransitiveDependencies()).thenReturn(new DependencyNodeBuilder().build());
when(resolveUtil.resolveTransitiveDependenciesVerbose()).thenReturn(new DependencyNodeBuilder().build());

rule.setSearchTransitive(true);
assertThatCode(rule::execute).doesNotThrowAnyException();
Expand All @@ -114,7 +114,8 @@ void testShouldFailOnlyWhenRelease() throws Exception {
@Test
void testWildcardExcludeTests() throws Exception {
when(session.getCurrentProject()).thenReturn(project);
when(resolveUtil.resolveTransitiveDependencies()).thenReturn(getDependencyNodeWithMultipleTestSnapshots());
when(resolveUtil.resolveTransitiveDependenciesVerbose())
.thenReturn(getDependencyNodeWithMultipleTestSnapshots());

rule.setExcludes(Collections.singletonList("*:*:*:*:test"));
rule.setSearchTransitive(true);
Expand All @@ -125,7 +126,8 @@ void testWildcardExcludeTests() throws Exception {
@Test
void testWildcardExcludeAll() throws Exception {
when(session.getCurrentProject()).thenReturn(project);
when(resolveUtil.resolveTransitiveDependencies()).thenReturn(getDependencyNodeWithMultipleTestSnapshots());
when(resolveUtil.resolveTransitiveDependenciesVerbose())
.thenReturn(getDependencyNodeWithMultipleTestSnapshots());

rule.setExcludes(Collections.singletonList("*"));
rule.setSearchTransitive(true);
Expand All @@ -135,7 +137,8 @@ void testWildcardExcludeAll() throws Exception {

@Test
void testExcludesAndIncludes() throws Exception {
when(resolveUtil.resolveTransitiveDependencies()).thenReturn(getDependencyNodeWithMultipleTestSnapshots());
when(resolveUtil.resolveTransitiveDependenciesVerbose())
.thenReturn(getDependencyNodeWithMultipleTestSnapshots());

rule.setExcludes(Collections.singletonList("*"));
rule.setIncludes(Collections.singletonList("*:*:*:*:test"));
Expand All @@ -161,7 +164,7 @@ void testId() {
void testFailWhenParentIsSnapshot() throws Exception {
when(session.getCurrentProject()).thenReturn(project);
when(project.getParentArtifact()).thenReturn(ARTIFACT_STUB_FACTORY.getSnapshotArtifact());
when(resolveUtil.resolveTransitiveDependencies()).thenReturn(new DependencyNodeBuilder().build());
when(resolveUtil.resolveTransitiveDependenciesVerbose()).thenReturn(new DependencyNodeBuilder().build());

rule.setFailWhenParentIsSnapshot(true);

Expand All @@ -174,7 +177,7 @@ void testFailWhenParentIsSnapshot() throws Exception {
void parentShouldBeExcluded() throws Exception {
when(session.getCurrentProject()).thenReturn(project);
when(project.getParentArtifact()).thenReturn(ARTIFACT_STUB_FACTORY.getSnapshotArtifact());
when(resolveUtil.resolveTransitiveDependencies()).thenReturn(new DependencyNodeBuilder().build());
when(resolveUtil.resolveTransitiveDependenciesVerbose()).thenReturn(new DependencyNodeBuilder().build());

rule.setFailWhenParentIsSnapshot(true);
rule.setExcludes(Collections.singletonList("testGroupId:*"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class RequireUpperBoundDepsTest {
@Test
void testRule() throws Exception {

when(resolveUtil.resolveTransitiveDependencies())
when(resolveUtil.resolveTransitiveDependenciesVerbose())
.thenReturn(new DependencyNodeBuilder()
.withType(DependencyNodeBuilder.Type.POM)
.withChildNode(new DependencyNodeBuilder()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
<?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>
<modelVersion>4.0.0</modelVersion>

<groupId>org.apache.maven.its.enforcer</groupId>
<artifactId>ban-transitive-test</artifactId>
<version>1.0</version>

<url>https://issues.apache.org/jira/browse/MENFORCER-469</url>

<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-enforcer-plugin</artifactId>
<version>@project.version@</version>
<executions>
<execution>
<id>test</id>
<goals>
<goal>enforce</goal>
</goals>
<configuration>
<rules>
<banTransitiveDependencies/>
</rules>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>

<dependencies>
<dependency>
<groupId>org.apache.maven.plugins.enforcer.its</groupId>
<artifactId>menforcer128_classic</artifactId>
<version>0.9.9</version>
</dependency>

<!-- menforcer128_api the same version as in menforcer128_classic -->

<dependency>
<groupId>org.apache.maven.plugins.enforcer.its</groupId>
<artifactId>menforcer128_api</artifactId>
<version>1.5.0</version>
</dependency>
</dependencies>

</project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
<?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>
<modelVersion>4.0.0</modelVersion>

<groupId>org.apache.maven.its.enforcer</groupId>
<artifactId>ban-transitive-test</artifactId>
<version>1.0</version>

<url>https://issues.apache.org/jira/browse/MENFORCER-469</url>

<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-enforcer-plugin</artifactId>
<version>@project.version@</version>
<executions>
<execution>
<id>test</id>
<goals>
<goal>enforce</goal>
</goals>
<configuration>
<rules>
<banTransitiveDependencies/>
</rules>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>

<dependencies>
<dependency>
<groupId>org.apache.maven.plugins.enforcer.its</groupId>
<artifactId>menforcer128_classic</artifactId>
<version>0.9.9</version>
</dependency>

<!-- menforcer128_api override version, in menforcer128_classic is 1.5.0 -->
<dependency>
<groupId>org.apache.maven.plugins.enforcer.its</groupId>
<artifactId>menforcer128_api</artifactId>
<version>1.6.0</version>
</dependency>
</dependencies>

</project>