From 4eab93f94027f0fae82a9e703f0e7f9c5d0c31d5 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Thu, 7 Oct 2021 10:42:06 +0200 Subject: [PATCH 1/8] Deploy At End feature (no extension) This PR makes deployAtEnd work as expected even if maven-deploy-plugin is not used as extension. How it works: it uses mojo Context to store "markers" (and params): * presence of marker means project was "processed" * value of marker tells what should be done (true = deploy, false = skipped) * if needed, other params are stored as well UTs adjusted to provide plugin context (was null before). --- pom.xml | 76 ++++++++----- .../maven/plugins/deploy/DeployMojo.java | 105 ++++++++++++++---- .../plugins/deploy/DeployFileMojoTest.java | 17 +-- .../maven/plugins/deploy/DeployMojoTest.java | 35 ++++-- .../apache/maven/plugins/deploy/Utils.java | 2 +- 5 files changed, 166 insertions(+), 69 deletions(-) diff --git a/pom.xml b/pom.xml index 3c590c66..fcba2f1a 100644 --- a/pom.xml +++ b/pom.xml @@ -63,8 +63,10 @@ under the License. - 3.0 - 7 + 3.2.5 + 1.7.32 + 1.1.0 + 8 2020-04-07T21:04:00Z @@ -73,35 +75,34 @@ under the License. org.apache.maven maven-plugin-api ${mavenVersion} + provided org.apache.maven - maven-core + maven-model ${mavenVersion} + provided org.apache.maven - maven-model + maven-artifact ${mavenVersion} + provided + + org.apache.maven - maven-artifact + maven-core ${mavenVersion} + provided - org.apache.maven.shared maven-artifact-transfer 0.13.1 - - - commons-io - commons-io - 2.5 - - + org.apache.maven.plugin-tools @@ -111,12 +112,12 @@ under the License. org.codehaus.plexus plexus-utils - 3.2.0 + 3.4.1 org.apache.maven.plugin-testing maven-plugin-testing-harness - 2.1 + 3.3.0 test @@ -134,28 +135,52 @@ under the License. junit junit - 4.13.1 - test - - - org.sonatype.aether - aether-connector-file - 1.7 + 4.13.2 test org.slf4j slf4j-api - 1.7.5 + ${slf4jVersion} provided org.slf4j slf4j-nop - 1.7.5 + ${slf4jVersion} test - + + org.eclipse.aether + aether-api + ${aetherVersion} + test + + + org.eclipse.aether + aether-util + ${aetherVersion} + test + + + org.eclipse.aether + aether-impl + ${aetherVersion} + test + + + org.eclipse.aether + aether-connector-basic + ${aetherVersion} + test + + + org.eclipse.aether + aether-transport-file + ${aetherVersion} + test + + @@ -188,6 +213,7 @@ under the License. org.apache.maven.plugins maven-invoker-plugin + 3.2.2 true true diff --git a/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java b/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java index ae26134b..ef5372a8 100644 --- a/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java +++ b/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java @@ -19,10 +19,8 @@ * under the License. */ -import java.util.ArrayList; -import java.util.Collections; import java.util.List; -import java.util.concurrent.atomic.AtomicInteger; +import java.util.Map; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -30,6 +28,7 @@ import org.apache.maven.artifact.repository.ArtifactRepository; import org.apache.maven.plugin.MojoExecutionException; import org.apache.maven.plugin.MojoFailureException; +import org.apache.maven.plugin.descriptor.PluginDescriptor; import org.apache.maven.plugins.annotations.Component; import org.apache.maven.plugins.annotations.LifecyclePhase; import org.apache.maven.plugins.annotations.Mojo; @@ -51,27 +50,31 @@ public class DeployMojo extends AbstractDeployMojo { - private static final Pattern ALT_LEGACY_REPO_SYNTAX_PATTERN = Pattern.compile( "(.+?)::(.+?)::(.+)" ); + private static final String DEPLOY_PROCESSED_MARKER = + DeployMojo.class.getName() + ".processed"; - private static final Pattern ALT_REPO_SYNTAX_PATTERN = Pattern.compile( "(.+?)::(.+)" ); + private static final String DEPLOY_ALT_RELEASE_DEPLOYMENT_REPOSITORY = + DeployMojo.class.getName() + ".altReleaseDeploymentRepository"; - /** - * When building with multiple threads, reaching the last project doesn't have to mean that all projects are ready - * to be deployed - */ - private static final AtomicInteger READYPROJECTSCOUNTER = new AtomicInteger(); + private static final String DEPLOY_ALT_SNAPSHOT_DEPLOYMENT_REPOSITORY = + DeployMojo.class.getName() + ".altSnapshotDeploymentRepository"; - private static final List DEPLOYREQUESTS = - Collections.synchronizedList( new ArrayList() ); + private static final String DEPLOY_ALT_DEPLOYMENT_REPOSITORY = + DeployMojo.class.getName() + ".altDeploymentRepository"; + + private static final Pattern ALT_LEGACY_REPO_SYNTAX_PATTERN = Pattern.compile( "(.+?)::(.+?)::(.+)" ); + + private static final Pattern ALT_REPO_SYNTAX_PATTERN = Pattern.compile( "(.+?)::(.+)" ); - /** - */ @Parameter( defaultValue = "${project}", readonly = true, required = true ) private MavenProject project; @Parameter( defaultValue = "${reactorProjects}", required = true, readonly = true ) private List reactorProjects; + @Parameter( defaultValue = "${plugin}", required = true, readonly = true ) + private PluginDescriptor pluginDescriptor; + /** * Whether every project should be deployed during its own deploy-phase or at the end of the multimodule build. If * set to {@code true} and the build fails, none of the reactor projects is deployed. @@ -146,12 +149,14 @@ public class DeployMojo public void execute() throws MojoExecutionException, MojoFailureException { + final String projectKey = project.getGroupId() + ":" + project.getArtifactId() + ":" + project.getVersion(); boolean addedDeployRequest = false; if ( Boolean.parseBoolean( skip ) || ( "releases".equals( skip ) && !ArtifactUtils.isSnapshot( project.getVersion() ) ) || ( "snapshots".equals( skip ) && ArtifactUtils.isSnapshot( project.getVersion() ) ) ) { + getPluginContext().put( DEPLOY_PROCESSED_MARKER, Boolean.FALSE ); getLog().info( "Skipping artifact deployment" ); } else @@ -177,29 +182,81 @@ public void execute() } else { - DEPLOYREQUESTS.add( pdr ); + if ( altReleaseDeploymentRepository != null ) + { + getPluginContext().put( + DEPLOY_ALT_RELEASE_DEPLOYMENT_REPOSITORY, + altReleaseDeploymentRepository + ); + } + if ( altSnapshotDeploymentRepository != null ) + { + getPluginContext().put( + DEPLOY_ALT_SNAPSHOT_DEPLOYMENT_REPOSITORY, + altSnapshotDeploymentRepository + ); + } + if ( altDeploymentRepository != null ) + { + getPluginContext().put( + DEPLOY_ALT_DEPLOYMENT_REPOSITORY, + altDeploymentRepository + ); + } + getPluginContext().put( DEPLOY_PROCESSED_MARKER, Boolean.TRUE ); addedDeployRequest = true; } } - boolean projectsReady = READYPROJECTSCOUNTER.incrementAndGet() == reactorProjects.size(); - if ( projectsReady ) + if ( allProjectsMarked() ) { - synchronized ( DEPLOYREQUESTS ) + for ( MavenProject reactorProject : reactorProjects ) { - while ( !DEPLOYREQUESTS.isEmpty() ) + Map pluginContext = getSession().getPluginContext( pluginDescriptor, reactorProject ); + Boolean install = (Boolean) pluginContext.get( DEPLOY_PROCESSED_MARKER ); + if ( !install ) { - ArtifactRepository repo = getDeploymentRepository( DEPLOYREQUESTS.get( 0 ) ); - - deployProject( getSession().getProjectBuildingRequest(), DEPLOYREQUESTS.remove( 0 ), repo ); + getLog().info( "Project " + projectKey + " skipped install" ); + } + else + { + String altReleaseDeploymentRepository = + (String) pluginContext.get( DEPLOY_ALT_RELEASE_DEPLOYMENT_REPOSITORY ); + String altSnapshotDeploymentRepository = + (String) pluginContext.get( DEPLOY_ALT_SNAPSHOT_DEPLOYMENT_REPOSITORY ); + String altDeploymentRepository = + (String) pluginContext.get( DEPLOY_ALT_DEPLOYMENT_REPOSITORY ); + + ProjectDeployerRequest pdr = new ProjectDeployerRequest() + .setProject( reactorProject ) + .setRetryFailedDeploymentCount( getRetryFailedDeploymentCount() ) + .setAltReleaseDeploymentRepository( altReleaseDeploymentRepository ) + .setAltSnapshotDeploymentRepository( altSnapshotDeploymentRepository ) + .setAltDeploymentRepository( altDeploymentRepository ); + + ArtifactRepository repo = getDeploymentRepository( pdr ); + + deployProject( getSession().getProjectBuildingRequest(), pdr, repo ); } } } else if ( addedDeployRequest ) { - getLog().info( "Deploying " + project.getGroupId() + ":" + project.getArtifactId() + ":" - + project.getVersion() + " at end" ); + getLog().info( "Deploying " + projectKey + " at end" ); + } + } + + private boolean allProjectsMarked() + { + for ( MavenProject reactorProject : reactorProjects ) + { + Map pluginContext = getSession().getPluginContext( pluginDescriptor, reactorProject ); + if ( !pluginContext.containsKey( DEPLOY_PROCESSED_MARKER ) ) + { + return false; + } } + return true; } private void deployProject( ProjectBuildingRequest pbr, ProjectDeployerRequest pir, ArtifactRepository repo ) diff --git a/src/test/java/org/apache/maven/plugins/deploy/DeployFileMojoTest.java b/src/test/java/org/apache/maven/plugins/deploy/DeployFileMojoTest.java index 6c7064bb..b07b5503 100644 --- a/src/test/java/org/apache/maven/plugins/deploy/DeployFileMojoTest.java +++ b/src/test/java/org/apache/maven/plugins/deploy/DeployFileMojoTest.java @@ -30,11 +30,12 @@ import org.apache.maven.model.Model; import org.apache.maven.plugin.testing.AbstractMojoTestCase; import org.apache.maven.project.ProjectBuildingRequest; -import org.apache.maven.repository.internal.MavenRepositorySystemSession; +import org.eclipse.aether.DefaultRepositorySystemSession; +import org.eclipse.aether.internal.impl.EnhancedLocalRepositoryManagerFactory; +import org.eclipse.aether.repository.LocalRepository; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.MockitoAnnotations; -import org.sonatype.aether.impl.internal.SimpleLocalRepositoryManager; /** * @author Allan Ramirez @@ -92,8 +93,8 @@ public void testBasicDeployFile() ProjectBuildingRequest buildingRequest = mock ( ProjectBuildingRequest.class ); when( session.getProjectBuildingRequest() ).thenReturn( buildingRequest ); - MavenRepositorySystemSession repositorySession = new MavenRepositorySystemSession(); - repositorySession.setLocalRepositoryManager( new SimpleLocalRepositoryManager( LOCAL_REPO ) ); + DefaultRepositorySystemSession repositorySession = new DefaultRepositorySystemSession(); + repositorySession.setLocalRepositoryManager( new EnhancedLocalRepositoryManagerFactory().newInstance( repositorySession, new LocalRepository( LOCAL_REPO )) ); when( buildingRequest.getRepositorySession() ).thenReturn( repositorySession ); String groupId = (String) getVariableValueFromObject( mojo, "groupId" ); @@ -193,8 +194,8 @@ public void testDeployIfClassifierIsSet() ProjectBuildingRequest buildingRequest = mock ( ProjectBuildingRequest.class ); when( session.getProjectBuildingRequest() ).thenReturn( buildingRequest ); - MavenRepositorySystemSession repositorySession = new MavenRepositorySystemSession(); - repositorySession.setLocalRepositoryManager( new SimpleLocalRepositoryManager( LOCAL_REPO ) ); + DefaultRepositorySystemSession repositorySession = new DefaultRepositorySystemSession(); + repositorySession.setLocalRepositoryManager( new EnhancedLocalRepositoryManagerFactory().newInstance( repositorySession, new LocalRepository( LOCAL_REPO )) ); when( buildingRequest.getRepositorySession() ).thenReturn( repositorySession ); String classifier = ( String ) getVariableValueFromObject( mojo, "classifier" ); @@ -241,8 +242,8 @@ public void testDeployIfArtifactIsNotJar() ProjectBuildingRequest buildingRequest = mock ( ProjectBuildingRequest.class ); when( session.getProjectBuildingRequest() ).thenReturn( buildingRequest ); - MavenRepositorySystemSession repositorySession = new MavenRepositorySystemSession(); - repositorySession.setLocalRepositoryManager( new SimpleLocalRepositoryManager( LOCAL_REPO ) ); + DefaultRepositorySystemSession repositorySession = new DefaultRepositorySystemSession(); + repositorySession.setLocalRepositoryManager( new EnhancedLocalRepositoryManagerFactory().newInstance( repositorySession, new LocalRepository( LOCAL_REPO )) ); when( buildingRequest.getRepositorySession() ).thenReturn( repositorySession ); String groupId = (String) getVariableValueFromObject( mojo, "groupId" ); diff --git a/src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java b/src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java index 63fe2dee..e032bd7b 100644 --- a/src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java +++ b/src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java @@ -19,6 +19,7 @@ * under the License. */ +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; @@ -28,11 +29,13 @@ import java.util.Collections; import java.util.List; import java.util.Properties; +import java.util.concurrent.ConcurrentHashMap; import org.apache.maven.artifact.repository.ArtifactRepository; import org.apache.maven.execution.MavenSession; import org.apache.maven.plugin.MojoExecutionException; import org.apache.maven.plugin.MojoFailureException; +import org.apache.maven.plugin.descriptor.PluginDescriptor; import org.apache.maven.plugin.testing.AbstractMojoTestCase; import org.apache.maven.plugin.testing.stubs.MavenProjectStub; import org.apache.maven.plugins.deploy.stubs.ArtifactDeployerStub; @@ -40,14 +43,15 @@ import org.apache.maven.plugins.deploy.stubs.DeployArtifactStub; import org.apache.maven.project.MavenProject; import org.apache.maven.project.ProjectBuildingRequest; -import org.apache.maven.repository.internal.MavenRepositorySystemSession; import org.apache.maven.shared.transfer.project.deploy.ProjectDeployerRequest; import org.codehaus.plexus.util.FileUtils; +import org.eclipse.aether.DefaultRepositorySystemSession; +import org.eclipse.aether.internal.impl.EnhancedLocalRepositoryManagerFactory; +import org.eclipse.aether.repository.LocalRepository; import org.junit.Ignore; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.MockitoAnnotations; -import org.sonatype.aether.impl.internal.SimpleLocalRepositoryManager; /** * @author Allan Ramirez @@ -77,7 +81,11 @@ public void setUp() throws Exception { super.setUp(); - + + MockitoAnnotations.initMocks( this ); + when( session.getPluginContext( any(PluginDescriptor.class ), any( MavenProject.class ) ) ) + .thenReturn( new ConcurrentHashMap<>( )); + remoteRepo = new File( REMOTE_REPO ); remoteRepo.mkdirs(); @@ -132,8 +140,8 @@ public void testBasicDeploy() ProjectBuildingRequest buildingRequest = mock ( ProjectBuildingRequest.class ); when( session.getProjectBuildingRequest() ).thenReturn( buildingRequest ); - MavenRepositorySystemSession repositorySession = new MavenRepositorySystemSession(); - repositorySession.setLocalRepositoryManager( new SimpleLocalRepositoryManager( LOCAL_REPO ) ); + DefaultRepositorySystemSession repositorySession = new DefaultRepositorySystemSession(); + repositorySession.setLocalRepositoryManager( new EnhancedLocalRepositoryManagerFactory().newInstance( repositorySession, new LocalRepository( LOCAL_REPO )) ); when( buildingRequest.getRepositorySession() ).thenReturn( repositorySession ); File file = new File( getBasedir(), @@ -143,7 +151,8 @@ public void testBasicDeploy() assertTrue( file.exists() ); MavenProject project = (MavenProject) getVariableValueFromObject( mojo, "project" ); - + + setVariableValueToObject( mojo, "pluginContext", new ConcurrentHashMap<>() ); setVariableValueToObject( mojo, "reactorProjects", Collections.singletonList( project ) ); artifact = ( DeployArtifactStub ) project.getArtifact(); @@ -250,7 +259,9 @@ public void testSkippingDeploy() assertTrue( file.exists() ); MavenProject project = (MavenProject) getVariableValueFromObject( mojo, "project" ); - + + setVariableValueToObject( mojo, "session", session ); + setVariableValueToObject( mojo, "pluginContext", new ConcurrentHashMap<>() ); setVariableValueToObject( mojo, "reactorProjects", Collections.singletonList( project ) ); artifact = (DeployArtifactStub) project.getArtifact(); @@ -303,8 +314,8 @@ public void testBasicDeployWithPackagingAsPom() ProjectBuildingRequest buildingRequest = mock ( ProjectBuildingRequest.class ); when( session.getProjectBuildingRequest() ).thenReturn( buildingRequest ); - MavenRepositorySystemSession repositorySession = new MavenRepositorySystemSession(); - repositorySession.setLocalRepositoryManager( new SimpleLocalRepositoryManager( LOCAL_REPO ) ); + DefaultRepositorySystemSession repositorySession = new DefaultRepositorySystemSession(); + repositorySession.setLocalRepositoryManager( new EnhancedLocalRepositoryManagerFactory().newInstance( repositorySession, new LocalRepository( LOCAL_REPO )) ); when( buildingRequest.getRepositorySession() ).thenReturn( repositorySession ); File pomFile = new File( getBasedir(), @@ -315,6 +326,7 @@ public void testBasicDeployWithPackagingAsPom() MavenProject project = (MavenProject) getVariableValueFromObject( mojo, "project" ); + setVariableValueToObject( mojo, "pluginContext", new ConcurrentHashMap<>() ); setVariableValueToObject( mojo, "reactorProjects", Collections.singletonList( project ) ); artifact = (DeployArtifactStub) project.getArtifact(); @@ -380,6 +392,7 @@ public void testDeployIfArtifactFileIsNull() MavenProject project = (MavenProject) getVariableValueFromObject( mojo, "project" ); + setVariableValueToObject( mojo, "pluginContext", new ConcurrentHashMap<>() ); setVariableValueToObject( mojo, "reactorProjects", Collections.singletonList( project ) ); artifact = (DeployArtifactStub) project.getArtifact(); @@ -415,8 +428,8 @@ public void testDeployWithAttachedArtifacts() ProjectBuildingRequest buildingRequest = mock ( ProjectBuildingRequest.class ); when( session.getProjectBuildingRequest() ).thenReturn( buildingRequest ); - MavenRepositorySystemSession repositorySession = new MavenRepositorySystemSession(); - repositorySession.setLocalRepositoryManager( new SimpleLocalRepositoryManager( LOCAL_REPO ) ); + DefaultRepositorySystemSession repositorySession = new DefaultRepositorySystemSession(); + repositorySession.setLocalRepositoryManager( new EnhancedLocalRepositoryManagerFactory().newInstance( repositorySession, new LocalRepository( LOCAL_REPO )) ); when( buildingRequest.getRepositorySession() ).thenReturn( repositorySession ); MavenProject project = (MavenProject) getVariableValueFromObject( mojo, "project" ); diff --git a/src/test/java/org/apache/maven/plugins/deploy/Utils.java b/src/test/java/org/apache/maven/plugins/deploy/Utils.java index 4df74424..f4acbc9d 100644 --- a/src/test/java/org/apache/maven/plugins/deploy/Utils.java +++ b/src/test/java/org/apache/maven/plugins/deploy/Utils.java @@ -26,7 +26,7 @@ import java.util.Map; import org.apache.maven.plugin.MojoExecutionException; -import org.sonatype.aether.util.ChecksumUtils; +import org.eclipse.aether.util.ChecksumUtils; /** * A utility class to assist testing. From fc30c356e3fc3ad613bd3d6b46cdac07c7c8f7bc Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Thu, 7 Oct 2021 14:41:53 +0200 Subject: [PATCH 2/8] Go back to Java7 --- pom.xml | 5 +++-- .../java/org/apache/maven/plugins/deploy/DeployMojoTest.java | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/pom.xml b/pom.xml index fcba2f1a..0721ae30 100644 --- a/pom.xml +++ b/pom.xml @@ -66,7 +66,8 @@ under the License. 3.2.5 1.7.32 1.1.0 - 8 + 2.22.2 + 7 2020-04-07T21:04:00Z @@ -112,7 +113,7 @@ under the License. org.codehaus.plexus plexus-utils - 3.4.1 + 3.3.0 org.apache.maven.plugin-testing diff --git a/src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java b/src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java index e032bd7b..63b2e6ea 100644 --- a/src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java +++ b/src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java @@ -84,7 +84,7 @@ public void setUp() MockitoAnnotations.initMocks( this ); when( session.getPluginContext( any(PluginDescriptor.class ), any( MavenProject.class ) ) ) - .thenReturn( new ConcurrentHashMap<>( )); + .thenReturn( new ConcurrentHashMap() ); remoteRepo = new File( REMOTE_REPO ); From 2074d62e7e342f798ecd69fdc1f4766cbe5a2d4a Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Fri, 19 Nov 2021 10:35:43 +0100 Subject: [PATCH 3/8] Fix keying and log message. --- .../apache/maven/plugins/deploy/DeployMojo.java | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java b/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java index ef5372a8..62f42401 100644 --- a/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java +++ b/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java @@ -149,7 +149,6 @@ public class DeployMojo public void execute() throws MojoExecutionException, MojoFailureException { - final String projectKey = project.getGroupId() + ":" + project.getArtifactId() + ":" + project.getVersion(); boolean addedDeployRequest = false; if ( Boolean.parseBoolean( skip ) || ( "releases".equals( skip ) && !ArtifactUtils.isSnapshot( project.getVersion() ) ) @@ -213,10 +212,12 @@ public void execute() for ( MavenProject reactorProject : reactorProjects ) { Map pluginContext = getSession().getPluginContext( pluginDescriptor, reactorProject ); - Boolean install = (Boolean) pluginContext.get( DEPLOY_PROCESSED_MARKER ); - if ( !install ) + Boolean deploy = (Boolean) pluginContext.get( DEPLOY_PROCESSED_MARKER ); + if ( !deploy ) { - getLog().info( "Project " + projectKey + " skipped install" ); + getLog().info( + "Project " + getProjectReferenceId( reactorProject ) + " skipped deploy" + ); } else { @@ -242,10 +243,15 @@ public void execute() } else if ( addedDeployRequest ) { - getLog().info( "Deploying " + projectKey + " at end" ); + getLog().info( "Deploying " + getProjectReferenceId( project ) + " at end" ); } } + private String getProjectReferenceId( MavenProject mavenProject ) + { + return mavenProject.getGroupId() + ":" + mavenProject.getArtifactId() + ":" + mavenProject.getVersion(); + } + private boolean allProjectsMarked() { for ( MavenProject reactorProject : reactorProjects ) From d40b8d49a0c4c7866983ce9440be0376c455d391 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Sun, 29 May 2022 20:58:57 +0200 Subject: [PATCH 4/8] Fix UT --- .../maven/plugins/deploy/DeployMojoTest.java | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java b/src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java index 6404f5ad..29dafdba 100644 --- a/src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java +++ b/src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java @@ -19,6 +19,7 @@ * under the License. */ +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; @@ -28,11 +29,13 @@ import java.util.Collections; import java.util.List; import java.util.Properties; +import java.util.concurrent.ConcurrentHashMap; import org.apache.maven.artifact.repository.ArtifactRepository; import org.apache.maven.execution.MavenSession; import org.apache.maven.plugin.MojoExecutionException; import org.apache.maven.plugin.MojoFailureException; +import org.apache.maven.plugin.descriptor.PluginDescriptor; import org.apache.maven.plugin.testing.AbstractMojoTestCase; import org.apache.maven.plugin.testing.stubs.MavenProjectStub; import org.apache.maven.plugins.deploy.stubs.ArtifactDeployerStub; @@ -68,7 +71,6 @@ public class DeployMojoTest MavenProjectStub project = new MavenProjectStub(); - @Mock private MavenSession session; @InjectMocks @@ -78,7 +80,11 @@ public void setUp() throws Exception { super.setUp(); - + + session = mock( MavenSession.class ); + when( session.getPluginContext(any(PluginDescriptor.class), any(MavenProject.class))) + .thenReturn( new ConcurrentHashMap() ); + remoteRepo = new File( REMOTE_REPO ); remoteRepo.mkdirs(); @@ -144,7 +150,8 @@ public void testBasicDeploy() assertTrue( file.exists() ); MavenProject project = (MavenProject) getVariableValueFromObject( mojo, "project" ); - + + setVariableValueToObject( mojo, "pluginContext", new ConcurrentHashMap<>() ); setVariableValueToObject( mojo, "reactorProjects", Collections.singletonList( project ) ); artifact = ( DeployArtifactStub ) project.getArtifact(); @@ -251,8 +258,11 @@ public void testSkippingDeploy() assertTrue( file.exists() ); MavenProject project = (MavenProject) getVariableValueFromObject( mojo, "project" ); - + + setVariableValueToObject( mojo, "pluginDescriptor", new PluginDescriptor() ); + setVariableValueToObject( mojo, "pluginContext", new ConcurrentHashMap<>() ); setVariableValueToObject( mojo, "reactorProjects", Collections.singletonList( project ) ); + setVariableValueToObject( mojo, "session", session ); artifact = (DeployArtifactStub) project.getArtifact(); From 198de55d3128d742de3e7a8e7c1a78c728ea5877 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Sun, 29 May 2022 21:22:04 +0200 Subject: [PATCH 5/8] Simplify --- .../java/org/apache/maven/plugins/deploy/DeployMojo.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java b/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java index c027fec9..73f81530 100644 --- a/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java +++ b/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java @@ -149,7 +149,6 @@ public class DeployMojo public void execute() throws MojoExecutionException, MojoFailureException { - boolean addedDeployRequest = false; if ( Boolean.parseBoolean( skip ) || ( "releases".equals( skip ) && !ArtifactUtils.isSnapshot( project.getVersion() ) ) || ( "snapshots".equals( skip ) && ArtifactUtils.isSnapshot( project.getVersion() ) ) @@ -203,7 +202,7 @@ public void execute() ); } getPluginContext().put( DEPLOY_PROCESSED_MARKER, Boolean.TRUE ); - addedDeployRequest = true; + getLog().info( "Deploying " + getProjectReferenceId( project ) + " at end" ); } } @@ -241,10 +240,6 @@ public void execute() } } } - else if ( addedDeployRequest ) - { - getLog().info( "Deploying " + getProjectReferenceId( project ) + " at end" ); - } } private String getProjectReferenceId( MavenProject mavenProject ) From d1fb040c0a1296d909bc3b7a1798a54e416af864 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Wed, 29 Jun 2022 15:02:35 +0200 Subject: [PATCH 6/8] Better handling of all cases (skipped one module). Also, be clearer re intent. Still, cannot use enum directly, as due classloading, there is no single enum. --- .../maven/plugins/deploy/DeployMojo.java | 46 +++++++++---------- .../maven/plugins/deploy/DeployMojoTest.java | 4 ++ 2 files changed, 27 insertions(+), 23 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java b/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java index 73f81530..08d977ee 100644 --- a/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java +++ b/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java @@ -146,6 +146,11 @@ public class DeployMojo @Component private ProjectDeployer projectDeployer; + private enum State + { + SKIPPED, DEPLOYED, TO_BE_DEPLOYED + } + public void execute() throws MojoExecutionException, MojoFailureException { @@ -154,29 +159,30 @@ public void execute() || ( "snapshots".equals( skip ) && ArtifactUtils.isSnapshot( project.getVersion() ) ) ) { - getPluginContext().put( DEPLOY_PROCESSED_MARKER, Boolean.FALSE ); getLog().info( "Skipping artifact deployment" ); + getPluginContext().put( DEPLOY_PROCESSED_MARKER, State.SKIPPED.name() ); } else { failIfOffline(); - // CHECKSTYLE_OFF: LineLength - // @formatter:off - ProjectDeployerRequest pdr = new ProjectDeployerRequest() - .setProject( project ) - .setRetryFailedDeploymentCount( getRetryFailedDeploymentCount() ) - .setAltReleaseDeploymentRepository( altReleaseDeploymentRepository ) - .setAltSnapshotDeploymentRepository( altSnapshotDeploymentRepository ) - .setAltDeploymentRepository( altDeploymentRepository ); - // @formatter:on - // CHECKSTYLE_ON: LineLength - - ArtifactRepository repo = getDeploymentRepository( pdr ); - if ( !deployAtEnd ) { + // CHECKSTYLE_OFF: LineLength + // @formatter:off + ProjectDeployerRequest pdr = new ProjectDeployerRequest() + .setProject( project ) + .setRetryFailedDeploymentCount( getRetryFailedDeploymentCount() ) + .setAltReleaseDeploymentRepository( altReleaseDeploymentRepository ) + .setAltSnapshotDeploymentRepository( altSnapshotDeploymentRepository ) + .setAltDeploymentRepository( altDeploymentRepository ); + // @formatter:on + // CHECKSTYLE_ON: LineLength + + ArtifactRepository repo = getDeploymentRepository( pdr ); + deployProject( getSession().getProjectBuildingRequest(), pdr, repo ); + getPluginContext().put( DEPLOY_PROCESSED_MARKER, State.DEPLOYED.name() ); } else { @@ -201,7 +207,7 @@ public void execute() altDeploymentRepository ); } - getPluginContext().put( DEPLOY_PROCESSED_MARKER, Boolean.TRUE ); + getPluginContext().put( DEPLOY_PROCESSED_MARKER, State.TO_BE_DEPLOYED.name() ); getLog().info( "Deploying " + getProjectReferenceId( project ) + " at end" ); } } @@ -211,14 +217,8 @@ public void execute() for ( MavenProject reactorProject : reactorProjects ) { Map pluginContext = getSession().getPluginContext( pluginDescriptor, reactorProject ); - Boolean deploy = (Boolean) pluginContext.get( DEPLOY_PROCESSED_MARKER ); - if ( !deploy ) - { - getLog().info( - "Project " + getProjectReferenceId( reactorProject ) + " skipped deploy" - ); - } - else + State state = State.valueOf( (String) pluginContext.get( DEPLOY_PROCESSED_MARKER ) ); + if ( state == State.TO_BE_DEPLOYED ) { String altReleaseDeploymentRepository = (String) pluginContext.get( DEPLOY_ALT_RELEASE_DEPLOYMENT_REPOSITORY ); diff --git a/src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java b/src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java index 29dafdba..c8350c47 100644 --- a/src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java +++ b/src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java @@ -326,6 +326,7 @@ public void testBasicDeployWithPackagingAsPom() MavenProject project = (MavenProject) getVariableValueFromObject( mojo, "project" ); + setVariableValueToObject( mojo, "pluginContext", new ConcurrentHashMap<>() ); setVariableValueToObject( mojo, "reactorProjects", Collections.singletonList( project ) ); artifact = (DeployArtifactStub) project.getArtifact(); @@ -391,6 +392,7 @@ public void testDeployIfArtifactFileIsNull() MavenProject project = (MavenProject) getVariableValueFromObject( mojo, "project" ); + setVariableValueToObject( mojo, "pluginContext", new ConcurrentHashMap<>() ); setVariableValueToObject( mojo, "reactorProjects", Collections.singletonList( project ) ); artifact = (DeployArtifactStub) project.getArtifact(); @@ -432,6 +434,7 @@ public void testDeployWithAttachedArtifacts() MavenProject project = (MavenProject) getVariableValueFromObject( mojo, "project" ); + setVariableValueToObject( mojo, "pluginContext", new ConcurrentHashMap<>() ); setVariableValueToObject( mojo, "reactorProjects", Collections.singletonList( project ) ); artifact = (DeployArtifactStub) project.getArtifact(); @@ -533,6 +536,7 @@ public void _testBasicDeployWithScpAsProtocol() MavenProject project = (MavenProject) getVariableValueFromObject( mojo, "project" ); + setVariableValueToObject( mojo, "pluginContext", new ConcurrentHashMap<>() ); setVariableValueToObject( mojo, "reactorProjects", Collections.singletonList( project ) ); artifact = (DeployArtifactStub) project.getArtifact(); From dc837d8650ecb75d44d24c4cc918429c650a1450 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Boutemy?= Date: Sat, 2 Jul 2022 15:59:00 +0200 Subject: [PATCH 7/8] create state management methods to clarify --- .../maven/plugins/deploy/DeployMojo.java | 95 ++++++++++--------- 1 file changed, 52 insertions(+), 43 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java b/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java index 08d977ee..115d66c6 100644 --- a/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java +++ b/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java @@ -50,18 +50,6 @@ public class DeployMojo extends AbstractDeployMojo { - private static final String DEPLOY_PROCESSED_MARKER = - DeployMojo.class.getName() + ".processed"; - - private static final String DEPLOY_ALT_RELEASE_DEPLOYMENT_REPOSITORY = - DeployMojo.class.getName() + ".altReleaseDeploymentRepository"; - - private static final String DEPLOY_ALT_SNAPSHOT_DEPLOYMENT_REPOSITORY = - DeployMojo.class.getName() + ".altSnapshotDeploymentRepository"; - - private static final String DEPLOY_ALT_DEPLOYMENT_REPOSITORY = - DeployMojo.class.getName() + ".altDeploymentRepository"; - private static final Pattern ALT_LEGACY_REPO_SYNTAX_PATTERN = Pattern.compile( "(.+?)::(.+?)::(.+)" ); private static final Pattern ALT_REPO_SYNTAX_PATTERN = Pattern.compile( "(.+?)::(.+)" ); @@ -151,6 +139,46 @@ private enum State SKIPPED, DEPLOYED, TO_BE_DEPLOYED } + private static final String DEPLOY_PROCESSED_MARKER = DeployMojo.class.getName() + ".processed"; + + private static final String DEPLOY_ALT_RELEASE_DEPLOYMENT_REPOSITORY = + DeployMojo.class.getName() + ".altReleaseDeploymentRepository"; + + private static final String DEPLOY_ALT_SNAPSHOT_DEPLOYMENT_REPOSITORY = + DeployMojo.class.getName() + ".altSnapshotDeploymentRepository"; + + private static final String DEPLOY_ALT_DEPLOYMENT_REPOSITORY = + DeployMojo.class.getName() + ".altDeploymentRepository"; + + private void putState( State state ) + { + getPluginContext().put( DEPLOY_PROCESSED_MARKER, state.name() ); + } + + private void putPluginContextValue( String key, String value ) + { + if ( value != null ) + { + getPluginContext().put( key, value ); + } + } + + private String getPluginContextValue( Map pluginContext, String key ) + { + return (String) pluginContext.get( key ); + } + + private State getState( Map pluginContext ) + { + return State.valueOf( getPluginContextValue( pluginContext, DEPLOY_PROCESSED_MARKER ) ); + } + + private boolean hasState( MavenProject project ) + { + Map pluginContext = getSession().getPluginContext( pluginDescriptor, project ); + return pluginContext.containsKey( DEPLOY_PROCESSED_MARKER ); + } + public void execute() throws MojoExecutionException, MojoFailureException { @@ -160,7 +188,7 @@ public void execute() ) { getLog().info( "Skipping artifact deployment" ); - getPluginContext().put( DEPLOY_PROCESSED_MARKER, State.SKIPPED.name() ); + putState( State.SKIPPED ); } else { @@ -182,33 +210,15 @@ public void execute() ArtifactRepository repo = getDeploymentRepository( pdr ); deployProject( getSession().getProjectBuildingRequest(), pdr, repo ); - getPluginContext().put( DEPLOY_PROCESSED_MARKER, State.DEPLOYED.name() ); + putState( State.DEPLOYED ); } else { - if ( altReleaseDeploymentRepository != null ) - { - getPluginContext().put( - DEPLOY_ALT_RELEASE_DEPLOYMENT_REPOSITORY, - altReleaseDeploymentRepository - ); - } - if ( altSnapshotDeploymentRepository != null ) - { - getPluginContext().put( - DEPLOY_ALT_SNAPSHOT_DEPLOYMENT_REPOSITORY, - altSnapshotDeploymentRepository - ); - } - if ( altDeploymentRepository != null ) - { - getPluginContext().put( - DEPLOY_ALT_DEPLOYMENT_REPOSITORY, - altDeploymentRepository - ); - } - getPluginContext().put( DEPLOY_PROCESSED_MARKER, State.TO_BE_DEPLOYED.name() ); - getLog().info( "Deploying " + getProjectReferenceId( project ) + " at end" ); + putState( State.TO_BE_DEPLOYED ); + putPluginContextValue( DEPLOY_ALT_RELEASE_DEPLOYMENT_REPOSITORY, altReleaseDeploymentRepository ); + putPluginContextValue( DEPLOY_ALT_SNAPSHOT_DEPLOYMENT_REPOSITORY, altSnapshotDeploymentRepository ); + putPluginContextValue( DEPLOY_ALT_DEPLOYMENT_REPOSITORY, altDeploymentRepository ); + getLog().info( "Deferring deploy for " + getProjectReferenceId( project ) + " at end" ); } } @@ -217,15 +227,15 @@ public void execute() for ( MavenProject reactorProject : reactorProjects ) { Map pluginContext = getSession().getPluginContext( pluginDescriptor, reactorProject ); - State state = State.valueOf( (String) pluginContext.get( DEPLOY_PROCESSED_MARKER ) ); + State state = getState( pluginContext ); if ( state == State.TO_BE_DEPLOYED ) { String altReleaseDeploymentRepository = - (String) pluginContext.get( DEPLOY_ALT_RELEASE_DEPLOYMENT_REPOSITORY ); + getPluginContextValue( pluginContext, DEPLOY_ALT_RELEASE_DEPLOYMENT_REPOSITORY ); String altSnapshotDeploymentRepository = - (String) pluginContext.get( DEPLOY_ALT_SNAPSHOT_DEPLOYMENT_REPOSITORY ); + getPluginContextValue( pluginContext, DEPLOY_ALT_SNAPSHOT_DEPLOYMENT_REPOSITORY ); String altDeploymentRepository = - (String) pluginContext.get( DEPLOY_ALT_DEPLOYMENT_REPOSITORY ); + getPluginContextValue( pluginContext, DEPLOY_ALT_DEPLOYMENT_REPOSITORY ); ProjectDeployerRequest pdr = new ProjectDeployerRequest() .setProject( reactorProject ) @@ -251,8 +261,7 @@ private boolean allProjectsMarked() { for ( MavenProject reactorProject : reactorProjects ) { - Map pluginContext = getSession().getPluginContext( pluginDescriptor, reactorProject ); - if ( !pluginContext.containsKey( DEPLOY_PROCESSED_MARKER ) ) + if ( !hasState( reactorProject ) ) { return false; } From 8ec56880a9978635b0c4a32c6fd61b842926fe88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Boutemy?= Date: Sat, 2 Jul 2022 16:31:32 +0200 Subject: [PATCH 8/8] fix ITs --- src/it/deploy-at-end-fail/verify.groovy | 2 +- src/it/deploy-at-end-pass/verify.groovy | 2 +- src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/it/deploy-at-end-fail/verify.groovy b/src/it/deploy-at-end-fail/verify.groovy index 03398e8b..27d678a6 100644 --- a/src/it/deploy-at-end-fail/verify.groovy +++ b/src/it/deploy-at-end-fail/verify.groovy @@ -22,5 +22,5 @@ assert !( new File( basedir, "module1/target/repo/org/apache/maven/its/deploy/da File buildLog = new File( basedir, 'build.log' ) assert buildLog.exists() -assert buildLog.text.contains( "[INFO] Deploying org.apache.maven.its.deploy.dae.fail:dae:1.0 at end" ) +assert buildLog.text.contains( "[INFO] Deferring deploy for org.apache.maven.its.deploy.dae.fail:dae:1.0 at end" ) diff --git a/src/it/deploy-at-end-pass/verify.groovy b/src/it/deploy-at-end-pass/verify.groovy index d776935a..634808a5 100644 --- a/src/it/deploy-at-end-pass/verify.groovy +++ b/src/it/deploy-at-end-pass/verify.groovy @@ -22,5 +22,5 @@ assert new File( basedir, "module1/target/repo/org/apache/maven/its/deploy/dae/p File buildLog = new File( basedir, 'build.log' ) assert buildLog.exists() -assert buildLog.text.contains( "[INFO] Deploying org.apache.maven.its.deploy.dae.pass:dae:1.0 at end" ) +assert buildLog.text.contains( "[INFO] Deferring deploy for org.apache.maven.its.deploy.dae.pass:dae:1.0 at end" ) diff --git a/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java b/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java index 115d66c6..bec6b384 100644 --- a/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java +++ b/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java @@ -218,7 +218,7 @@ public void execute() putPluginContextValue( DEPLOY_ALT_RELEASE_DEPLOYMENT_REPOSITORY, altReleaseDeploymentRepository ); putPluginContextValue( DEPLOY_ALT_SNAPSHOT_DEPLOYMENT_REPOSITORY, altSnapshotDeploymentRepository ); putPluginContextValue( DEPLOY_ALT_DEPLOYMENT_REPOSITORY, altDeploymentRepository ); - getLog().info( "Deferring deploy for " + getProjectReferenceId( project ) + " at end" ); + getLog().info( "Deferring deploy for " + getProjectReferenceId( project ) + " at end" ); } }