[MENFORCER-229] Ban Distribution Management documentation example doesn't work
The problem was to use the getModel() of the Maven Project which contains the
interpolated inheritance hierarchy which results in having always an
distributionManagement if somewhere in the higher levels distributionManagement
has been defined. Using the getOriginalModel() shows the non interpolated model
(pom file) which is the correct way to check the distributionManagement.
Furthermore enhanced documentation.
git-svn-id: https://svn.apache.org/repos/asf/maven/enforcer/trunk@1694884 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/BanDistributionManagement.java b/enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/BanDistributionManagement.java
index fb71f38..ae7e58f 100644
--- a/enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/BanDistributionManagement.java
+++ b/enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/BanDistributionManagement.java
@@ -40,6 +40,7 @@
/**
* If we turn on the <code>ignoreParent</code> the parent will be ignored.
+ * @deprecated
*/
private boolean ignoreParent = true;
@@ -58,11 +59,6 @@
*/
private boolean allowSite = false;
- /**
- * Execute checking only in root of project.
- */
- private boolean executeRootOnly = false;
-
private Log logger;
/**
@@ -77,21 +73,33 @@
{
MavenProject project = (MavenProject) helper.evaluate( "${project}" );
- if ( !project.isExecutionRoot() && !executeRootOnly )
+ if ( project.isExecutionRoot() )
{
+ if ( project.getParent() == null )
+ {
+ // Does it make sense to check something? If yes please make a JIRA ticket for it.
+ logger.debug( "We have no parent and in the root of a build we don't check anything," );
+ logger.debug( "because that is the location where we defined maven-enforcer-plugin." );
+ }
+ else
+ {
+ logger.debug( "We are in the root of the execution and we have a parent." );
- logger.debug( "distributionManagement: " + project.getDistributionManagement() );
-
+ DistributionManagementCheck check = new DistributionManagementCheck( project );
+ check.execute( isAllowRepository(), isAllowSnapshotRepository(), isAllowSite() );
+ }
+ }
+ else
+ {
+ logger.debug( "We are in a deeper level." );
DistributionManagementCheck check = new DistributionManagementCheck( project );
-
check.execute( isAllowRepository(), isAllowSnapshotRepository(), isAllowSite() );
- if ( !isIgnoreParent() && ( project.getParent() != null ) )
+ if ( !isIgnoreParent() )
{
- DistributionManagementCheck checkParent = new DistributionManagementCheck( project.getParent() );
- checkParent.execute( isAllowRepository(), isAllowSnapshotRepository(), isAllowSite() );
+ logger.warn( "You have configured not to ignore the parent." );
+ logger.warn( "This configuration is deprecated and will be ignored." );
}
-
}
}
catch ( ExpressionEvaluationException e )
diff --git a/enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/utils/DistributionManagementCheck.java b/enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/utils/DistributionManagementCheck.java
index a52a8eb..e961222 100644
--- a/enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/utils/DistributionManagementCheck.java
+++ b/enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/utils/DistributionManagementCheck.java
@@ -25,7 +25,6 @@
/**
* @author Karl Heinz Marbaise <a href="mailto:khmarbaise@apache.org">khmarbaise@apache.org</a>
- *
*/
public class DistributionManagementCheck
{
@@ -33,7 +32,7 @@
public DistributionManagementCheck( MavenProject project )
{
- this.distributionManagement = project.getDistributionManagement();
+ this.distributionManagement = project.getOriginalModel().getDistributionManagement();
}
public void execute( boolean isAllowRepository, boolean isAllowSnapshotRepository, boolean isAllowSite )
diff --git a/enforcer-rules/src/site/apt/Images.odp b/enforcer-rules/src/site/apt/Images.odp
new file mode 100644
index 0000000..02653cd
--- /dev/null
+++ b/enforcer-rules/src/site/apt/Images.odp
Binary files differ
diff --git a/enforcer-rules/src/site/apt/banDistributionManagement.apt.vm b/enforcer-rules/src/site/apt/banDistributionManagement.apt.vm
index 4b9a1df..b928468 100644
--- a/enforcer-rules/src/site/apt/banDistributionManagement.apt.vm
+++ b/enforcer-rules/src/site/apt/banDistributionManagement.apt.vm
@@ -38,7 +38,7 @@
* allowSite - You can allow site entry (default: false).
- * ignoreParent - You can control if the parent will be checked or not (default: true).
+ * ignoreParent - You can control if the parent will be checked or not (default: true) (deprecated don't use it anymore).
[]
@@ -74,23 +74,101 @@
</project>
+---+
- The above configuration will prevent any declaration of distributionManagement
- in your pom files except in the parent.
-
- Let use take a look at the following project which is a typical multi-module project.
+* Best Practice
+
+ Usually you should define the <<distributionManagement>> only in a limited number of cases.
+ If you are in a corporate environment it makes usually only sense to define the <<distributionManagement>>
+ in the corporate pom and forbid the usage in any other pom's. Sometimes it makes sense to
+ allow for example the site repository definition in your other pom's which can be defined by using the
+ <<banDistributionManagement>> rule. For this use case the following has to be defined in your
+ corporate pom file:
-+-----+
- +-- root (pom.xml)
- +-- m1 (pom.xml)
- +-- m2 (pom.xml)
-+-----+
+
+ +---+
+<project>
+ [...]
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-enforcer-plugin</artifactId>
+ <version>${project.version}</version>
+ <executions>
+ <execution>
+ <id>no-distribution-management-at-all</id>
+ <goals>
+ <goal>enforce</goal>
+ </goals>
+ <configuration>
+ <rules>
+ <banDistributionManagement>
+ <allowSite>true</allowSite>
+ </banDistributionManagement>
+ </rules>
+ </configuration>
+ </execution>
+ </executions>
+ </plugin>
+ </plugins>
+ </build>
+ [...]
+</project>
++---+
+
- Usually you should define the distributionManagement only once in your hierarchy.
-
- company-parent (pom.xml)
-
-
- +-- root (pom.xml)
- +-- m1 (pom.xml)
- +-- m2 (pom.xml)
+* The Project Types
+ If we take a closer look to the possible project structures you will find the following cases where
+ the green arrow will show our current position.
+
+ * The Project without Parent.
+
+ We have no parent and no modules at all. This could be the situation where we are creating
+ a corporate pom file or another simple Maven project. So the definition of maven-enforcer-plugin
+ is within this pom file.
+
+[images/root.png] Root Project.
+
+ In consequence it does not really make sense to check if the pom contains distributionManagement
+ entries or not.
+
+ * Project with Parent.
+
+ We have a project with a parent. The parent is likely a kind of a corporate pom file which
+ contains the definition of maven-enforcer-plugin. So in this case it makes sense to check
+ if distributionManagement entries are made or not.
+
+ <<Note>>: At the moment it is not possible to
+ check if the parent does not contain the definition of maven-enforcer-plugin which would change
+ the situation.
+
+[images/root-with-parent.png] Root project.
+
+
+ * Project with Parent and Modules.
+
+ This situation is more or less the same as the one before. So banDistributionManagement rule will check
+ the distributionManagement entries.
+
+[images/root-with-parent-and-modules.png] Root project with parent and modules.
+
+
+ * Root Project With Modules.
+
+ If we don't have a parent this means the definition of maven-enforcer-plugin
+ is likely done in the current pom file which means it does not make really sense
+ to check the distributionManagement.
+
+[images/root-with-modules.png] Root with Modules.
+
+
+ * Module of a Multi Module Build
+
+ In this case we have the scenario that the module m1 has a parent <<p1>> which could contain
+ the definition of the maven-enforcer-plugin or the <<parent>> of which in consequence means to check the
+ distributionManagement entries.
+
+[images/module.png] Root with Modules.
+
+
+ []
diff --git a/enforcer-rules/src/site/resources/images/module.png b/enforcer-rules/src/site/resources/images/module.png
new file mode 100644
index 0000000..0bac49a
--- /dev/null
+++ b/enforcer-rules/src/site/resources/images/module.png
Binary files differ
diff --git a/enforcer-rules/src/site/resources/images/root-with-modules.png b/enforcer-rules/src/site/resources/images/root-with-modules.png
new file mode 100644
index 0000000..d0586bf
--- /dev/null
+++ b/enforcer-rules/src/site/resources/images/root-with-modules.png
Binary files differ
diff --git a/enforcer-rules/src/site/resources/images/root-with-parent-and-modules.png b/enforcer-rules/src/site/resources/images/root-with-parent-and-modules.png
new file mode 100644
index 0000000..eb254fb
--- /dev/null
+++ b/enforcer-rules/src/site/resources/images/root-with-parent-and-modules.png
Binary files differ
diff --git a/enforcer-rules/src/site/resources/images/root-with-parent.png b/enforcer-rules/src/site/resources/images/root-with-parent.png
new file mode 100644
index 0000000..4cb7041
--- /dev/null
+++ b/enforcer-rules/src/site/resources/images/root-with-parent.png
Binary files differ
diff --git a/enforcer-rules/src/site/resources/images/root.png b/enforcer-rules/src/site/resources/images/root.png
new file mode 100644
index 0000000..67597a5
--- /dev/null
+++ b/enforcer-rules/src/site/resources/images/root.png
Binary files differ
diff --git a/enforcer-rules/src/test/java/org/apache/maven/plugins/enforcer/BanDistributionManagementTest.java b/enforcer-rules/src/test/java/org/apache/maven/plugins/enforcer/BanDistributionManagementTest.java
index ca18152..fdc33f7 100644
--- a/enforcer-rules/src/test/java/org/apache/maven/plugins/enforcer/BanDistributionManagementTest.java
+++ b/enforcer-rules/src/test/java/org/apache/maven/plugins/enforcer/BanDistributionManagementTest.java
@@ -26,6 +26,7 @@
import org.apache.maven.enforcer.rule.api.EnforcerRuleHelper;
import org.apache.maven.model.DeploymentRepository;
import org.apache.maven.model.DistributionManagement;
+import org.apache.maven.model.Model;
import org.apache.maven.model.Site;
import org.apache.maven.plugin.logging.Log;
import org.apache.maven.project.MavenProject;
@@ -115,7 +116,8 @@
throws Exception
{
BanDistributionManagement rule =
- setupProjectWithDistributionManagement( new DeploymentRepository(), new DeploymentRepository(), new Site() );
+ setupProjectWithDistributionManagement( new DeploymentRepository(), new DeploymentRepository(),
+ new Site() );
rule.execute( helper );
// intentionally no assert cause we expect an exception.
}
@@ -184,7 +186,8 @@
throws Exception
{
BanDistributionManagement rule =
- setupProjectWithDistributionManagement( new DeploymentRepository(), new DeploymentRepository(), new Site() );
+ setupProjectWithDistributionManagement( new DeploymentRepository(), new DeploymentRepository(),
+ new Site() );
rule.setAllowRepository( true );
rule.setAllowSnapshotRepository( true );
rule.setAllowSite( true );
@@ -192,19 +195,6 @@
// intentionally no assert cause in case of an exception the test will be red.
}
- @Test( expected = EnforcerRuleException.class )
- public void shouldThrowExceptionCauseParentProjectHasDistributionManagement()
- throws Exception
- {
- BanDistributionManagement rule =
- setupProjectWithParentDistributionManagement( new DeploymentRepository(), null, null );
-
- rule.setIgnoreParent( false );
-
- rule.execute( helper );
- // intentionally no assert cause we expect an exception.
- }
-
@Test
public void shouldThrowExceptionCauseParentProjectHasDistributionManagementSnapshotRepository()
throws Exception
@@ -221,7 +211,7 @@
private BanDistributionManagement setupProjectWithParentDistributionManagement( DeploymentRepository repository,
DeploymentRepository snapshotRepository,
Site site )
- throws ExpressionEvaluationException
+ throws ExpressionEvaluationException
{
project = setupProject( null );
@@ -231,7 +221,9 @@
when( dmParent.getSite() ).thenReturn( site );
MavenProject parentProject = mock( MavenProject.class );
- when( parentProject.getDistributionManagement() ).thenReturn( dmParent );
+ Model model = mock( Model.class );
+ when( model.getDistributionManagement() ).thenReturn( dmParent );
+ when( parentProject.getOriginalModel() ).thenReturn( model );
when( project.getParent() ).thenReturn( parentProject );
BanDistributionManagement rule = setupEnforcerRule();
@@ -252,7 +244,7 @@
private BanDistributionManagement setupProjectWithDistributionManagement( DeploymentRepository repository,
DeploymentRepository snapshotRepository,
Site site )
- throws ExpressionEvaluationException
+ throws ExpressionEvaluationException
{
DistributionManagement dm = mock( DistributionManagement.class );
when( dm.getRepository() ).thenReturn( repository );
@@ -261,6 +253,9 @@
project = setupProject( dm );
+ when( project.getParent() ).thenReturn( mock( MavenProject.class ) );
+ when( project.isExecutionRoot() ).thenReturn( true );
+
BanDistributionManagement rule = setupEnforcerRule();
return rule;
@@ -270,7 +265,9 @@
{
MavenProject project = mock( MavenProject.class );
when( project.getPackaging() ).thenReturn( "jar" );
- when( project.getDistributionManagement() ).thenReturn( distributionManagement );
+ Model mavenModel = mock( Model.class );
+ when( project.getOriginalModel() ).thenReturn( mavenModel );
+ when( mavenModel.getDistributionManagement() ).thenReturn( distributionManagement );
return project;
}
diff --git a/maven-enforcer-plugin/src/it/projects/ban-distribution-management-multi-module-build/module1/pom.xml b/maven-enforcer-plugin/src/it/projects/ban-distribution-management-multi-module-build/module1/pom.xml
index 419896b..0271bd1 100644
--- a/maven-enforcer-plugin/src/it/projects/ban-distribution-management-multi-module-build/module1/pom.xml
+++ b/maven-enforcer-plugin/src/it/projects/ban-distribution-management-multi-module-build/module1/pom.xml
@@ -29,4 +29,14 @@
</parent>
<artifactId>module1</artifactId>
+ <!--
+ ! This entry will fail the build.
+ -->
+ <distributionManagement>
+ <repository>
+ <id>first</id>
+ <name>This is the name</name>
+ <url>file:///Test</url>
+ </repository>
+ </distributionManagement>
</project>
diff --git a/maven-enforcer-plugin/src/it/projects/ban-distribution-management-multi-module-build/pom.xml b/maven-enforcer-plugin/src/it/projects/ban-distribution-management-multi-module-build/pom.xml
index 88c6174..3c4a0a4 100644
--- a/maven-enforcer-plugin/src/it/projects/ban-distribution-management-multi-module-build/pom.xml
+++ b/maven-enforcer-plugin/src/it/projects/ban-distribution-management-multi-module-build/pom.xml
@@ -27,6 +27,9 @@
<version>1.0-SNAPSHOT</version>
<packaging>pom</packaging>
+ <!--
+ ! This entry is allowed
+ -->
<distributionManagement>
<repository>
<id>first</id>
