From c8f8fef6a6655f1fc8f1bc394941ff1f505ed165 Mon Sep 17 00:00:00 2001 From: Carlos Sanchez Gonzalez Date: Tue, 15 May 2007 02:14:25 +0000 Subject: [PATCH] =?UTF-8?q?[MNG-2985]=20DefaultWagonManager=20does=20not?= =?UTF-8?q?=20safely=20remove=20TransferListeners=20from=20the=20wagon=20S?= =?UTF-8?q?ubmitted=20By:=20Abel=20Mui=C3=B1o?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit git-svn-id: https://svn.apache.org/repos/asf/maven/components/trunk@538041 13f79535-47bb-0310-9956-ffa450edef68 --- .../artifact/manager/DefaultWagonManager.java | 108 +++++++++++------- .../manager/DefaultWagonManagerTest.java | 74 ++++++++++++ .../maven/artifact/manager/WagonNoOp.java | 73 ++++++++++++ .../manager/DefaultWagonManagerTest.xml | 5 + 4 files changed, 219 insertions(+), 41 deletions(-) create mode 100644 maven-artifact/src/test/java/org/apache/maven/artifact/manager/WagonNoOp.java diff --git a/maven-artifact/src/main/java/org/apache/maven/artifact/manager/DefaultWagonManager.java b/maven-artifact/src/main/java/org/apache/maven/artifact/manager/DefaultWagonManager.java index 67e0dcf0d7..2205095494 100644 --- a/maven-artifact/src/main/java/org/apache/maven/artifact/manager/DefaultWagonManager.java +++ b/maven-artifact/src/main/java/org/apache/maven/artifact/manager/DefaultWagonManager.java @@ -1,4 +1,4 @@ - package org.apache.maven.artifact.manager; +package org.apache.maven.artifact.manager; /* * Licensed to the Apache Software Foundation (ASF) under one @@ -69,6 +69,11 @@ public class DefaultWagonManager { private static final String WILDCARD = "*"; + private static final String[] CHECKSUM_IDS = { "md5", "sha1" }; + + /** have to match the CHECKSUM_IDS */ + private static final String[] CHECKSUM_ALGORITHMS = { "MD5", "SHA-1" }; + private PlexusContainer container; // TODO: proxies, authentication and mirrors are via settings, and should come in via an alternate method - perhaps @@ -190,44 +195,43 @@ public class DefaultWagonManager Map sums = new HashMap( 2 ); // TODO: configure these on the repository - try + for ( int i = 0; i < CHECKSUM_IDS.length; i++ ) { - ChecksumObserver checksumObserver = new ChecksumObserver( "MD5" ); - wagon.addTransferListener( checksumObserver ); - checksums.put( "md5", checksumObserver ); - checksumObserver = new ChecksumObserver( "SHA-1" ); - wagon.addTransferListener( checksumObserver ); - checksums.put( "sha1", checksumObserver ); - } - catch ( NoSuchAlgorithmException e ) - { - throw new TransferFailedException( "Unable to add checksum methods: " + e.getMessage(), e ); + checksums.put( CHECKSUM_IDS[i], addChecksumObserver( wagon, CHECKSUM_ALGORITHMS[i] ) ); } try { - Repository artifactRepository = new Repository( repository.getId(), repository.getUrl() ); - - if ( serverPermissionsMap.containsKey( repository.getId() ) ) + try { - RepositoryPermissions perms = (RepositoryPermissions) serverPermissionsMap.get( repository.getId() ); + Repository artifactRepository = new Repository( repository.getId(), repository.getUrl() ); - getLogger().debug( - "adding permissions to wagon connection: " + perms.getFileMode() + " " + perms.getDirectoryMode() ); + if ( serverPermissionsMap.containsKey( repository.getId() ) ) + { + RepositoryPermissions perms = (RepositoryPermissions) serverPermissionsMap.get( repository.getId() ); - artifactRepository.setPermissions( perms ); + getLogger().debug( + "adding permissions to wagon connection: " + perms.getFileMode() + " " + perms.getDirectoryMode() ); + + artifactRepository.setPermissions( perms ); + } + else + { + getLogger().debug( "not adding permissions to wagon connection" ); + } + + wagon.connect( artifactRepository, getAuthenticationInfo( repository.getId() ), getProxy( protocol ) ); + + wagon.put( source, remotePath ); } - else + finally { - getLogger().debug( "not adding permissions to wagon connection" ); + if ( downloadMonitor != null ) + { + wagon.removeTransferListener( downloadMonitor ); + } } - wagon.connect( artifactRepository, getAuthenticationInfo( repository.getId() ), getProxy( protocol ) ); - - wagon.put( source, remotePath ); - - wagon.removeTransferListener( downloadMonitor ); - // Pre-store the checksums as any future puts will overwrite them for ( Iterator i = checksums.keySet().iterator(); i.hasNext(); ) { @@ -271,12 +275,37 @@ public class DefaultWagonManager } finally { + // Remove every checksum listener + for ( int i = 0; i < CHECKSUM_IDS.length; i++ ) + { + TransferListener checksumListener = (TransferListener) checksums.get( CHECKSUM_IDS[i] ); + if ( checksumListener != null ) + { + wagon.removeTransferListener( checksumListener ); + } + } + disconnectWagon( wagon ); releaseWagon( protocol, wagon ); } } + private ChecksumObserver addChecksumObserver( Wagon wagon, String algorithm ) + throws TransferFailedException + { + try + { + ChecksumObserver checksumObserver = new ChecksumObserver( algorithm ); + wagon.addTransferListener( checksumObserver ); + return checksumObserver; + } + catch ( NoSuchAlgorithmException e ) + { + throw new TransferFailedException( "Unable to add checksum for unsupported algorithm " + algorithm, e ); + } + } + public void getArtifact( Artifact artifact, List remoteRepositories ) throws TransferFailedException, ResourceDoesNotExistException { @@ -385,20 +414,9 @@ public class DefaultWagonManager } // TODO: configure on repository - ChecksumObserver md5ChecksumObserver; - ChecksumObserver sha1ChecksumObserver; - try - { - md5ChecksumObserver = new ChecksumObserver( "MD5" ); - wagon.addTransferListener( md5ChecksumObserver ); - - sha1ChecksumObserver = new ChecksumObserver( "SHA-1" ); - wagon.addTransferListener( sha1ChecksumObserver ); - } - catch ( NoSuchAlgorithmException e ) - { - throw new TransferFailedException( "Unable to add checksum methods: " + e.getMessage(), e ); - } + int i = 0; + ChecksumObserver md5ChecksumObserver = addChecksumObserver( wagon, CHECKSUM_ALGORITHMS[i++] ); + ChecksumObserver sha1ChecksumObserver = addChecksumObserver( wagon, CHECKSUM_ALGORITHMS[i++] ); File temp = new File( destination + ".tmp" ); temp.deleteOnExit(); @@ -531,6 +549,14 @@ public class DefaultWagonManager } finally { + // Remove every TransferListener + wagon.removeTransferListener( md5ChecksumObserver ); + wagon.removeTransferListener( sha1ChecksumObserver ); + if ( downloadMonitor != null ) + { + wagon.removeTransferListener( downloadMonitor ); + } + disconnectWagon( wagon ); releaseWagon( protocol, wagon ); diff --git a/maven-artifact/src/test/java/org/apache/maven/artifact/manager/DefaultWagonManagerTest.java b/maven-artifact/src/test/java/org/apache/maven/artifact/manager/DefaultWagonManagerTest.java index a6dcfcf2d4..2f289b2bea 100644 --- a/maven-artifact/src/test/java/org/apache/maven/artifact/manager/DefaultWagonManagerTest.java +++ b/maven-artifact/src/test/java/org/apache/maven/artifact/manager/DefaultWagonManagerTest.java @@ -19,8 +19,19 @@ package org.apache.maven.artifact.manager; * under the License. */ +import java.io.File; + +import org.apache.maven.artifact.Artifact; +import org.apache.maven.artifact.DefaultArtifact; +import org.apache.maven.artifact.metadata.ArtifactMetadata; +import org.apache.maven.artifact.repository.ArtifactRepository; +import org.apache.maven.artifact.repository.DefaultArtifactRepository; +import org.apache.maven.artifact.repository.layout.ArtifactRepositoryLayout; +import org.apache.maven.artifact.versioning.VersionRange; import org.apache.maven.wagon.UnsupportedProtocolException; import org.apache.maven.wagon.Wagon; +import org.apache.maven.wagon.events.TransferListener; +import org.apache.maven.wagon.observers.Debug; import org.apache.maven.wagon.repository.Repository; import org.codehaus.plexus.PlexusTestCase; import org.codehaus.plexus.util.xml.Xpp3Dom; @@ -35,6 +46,8 @@ public class DefaultWagonManagerTest private WagonManager wagonManager; + private TransferListener transferListener = new Debug(); + protected void setUp() throws Exception { @@ -54,6 +67,8 @@ public class DefaultWagonManagerTest assertWagon( "c" ); + assertWagon( "noop" ); + try { assertWagon( "d" ); @@ -111,6 +126,46 @@ public class DefaultWagonManagerTest } } + /** + * Check that transfer listeners are properly removed after getArtifact and putArtifact + */ + public void testWagonTransferListenerRemovedAfterGetArtifactAndPutArtifact() + throws Exception + { + File tmpFile = File.createTempFile( "mvn-test", ".temp" ); + + try + { + tmpFile.deleteOnExit(); + Artifact artifact = new DefaultArtifact( "sample.group", "sample-art", VersionRange + .createFromVersion( "1.0" ), "scope", "type", "classifier", null ); + artifact.setFile( tmpFile ); + ArtifactRepository repo = new DefaultArtifactRepository( "id", "noop://url", + new ArtifactRepositoryLayoutStub() ); + WagonNoOp wagon = (WagonNoOp) wagonManager.getWagon( "noop" ); + + /* getArtifact */ + assertFalse( "Transfer listener is registered before test", wagon.getTransferEventSupport() + .hasTransferListener( transferListener ) ); + wagonManager.setDownloadMonitor( transferListener ); + wagonManager.getArtifact( artifact, repo ); + assertFalse( "Transfer listener still registered after getArtifact", wagon.getTransferEventSupport() + .hasTransferListener( transferListener ) ); + + /* putArtifact */ + assertFalse( "Transfer listener is registered before test", wagon.getTransferEventSupport() + .hasTransferListener( transferListener ) ); + wagonManager.setDownloadMonitor( transferListener ); + wagonManager.putArtifact( new File( "sample file" ), artifact, repo ); + assertFalse( "Transfer listener still registered after putArtifact", wagon.getTransferEventSupport() + .hasTransferListener( transferListener ) ); + } + finally + { + tmpFile.delete(); + } + } + private void assertWagon( String protocol ) throws Exception { @@ -147,4 +202,23 @@ public class DefaultWagonManagerTest assertEquals( "Check configuration for wagon, protocol=" + protocol, s, wagon.getConfigurableField() ); } + private final class ArtifactRepositoryLayoutStub + implements ArtifactRepositoryLayout + { + public String pathOfRemoteRepositoryMetadata( ArtifactMetadata metadata ) + { + return "path"; + } + + public String pathOfLocalRepositoryMetadata( ArtifactMetadata metadata, ArtifactRepository repository ) + { + return "path"; + } + + public String pathOf( Artifact artifact ) + { + return "path"; + } + } + } diff --git a/maven-artifact/src/test/java/org/apache/maven/artifact/manager/WagonNoOp.java b/maven-artifact/src/test/java/org/apache/maven/artifact/manager/WagonNoOp.java new file mode 100644 index 0000000000..d5f4eb65b7 --- /dev/null +++ b/maven-artifact/src/test/java/org/apache/maven/artifact/manager/WagonNoOp.java @@ -0,0 +1,73 @@ +package org.apache.maven.artifact.manager; + +/* + * 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. + */ + +import java.io.File; + +import org.apache.maven.wagon.AbstractWagon; +import org.apache.maven.wagon.ResourceDoesNotExistException; +import org.apache.maven.wagon.TransferFailedException; +import org.apache.maven.wagon.authorization.AuthorizationException; +import org.apache.maven.wagon.resource.Resource; + +public class WagonNoOp + extends AbstractWagon +{ + + public void closeConnection() + { + // NO-OP + } + + public void get( String resourceName, File destination ) + throws TransferFailedException, ResourceDoesNotExistException, AuthorizationException + { + Resource resource = new Resource( resourceName ); + fireGetInitiated( resource, destination ); + fireGetStarted( resource, destination ); + fireGetCompleted( resource, destination ); + } + + public boolean getIfNewer( String resourceName, File destination, long timestamp ) + throws TransferFailedException, ResourceDoesNotExistException, AuthorizationException + { + // NO-OP + return false; + } + + public void openConnection() + { + // NO-OP + } + + public void put( File source, String destination ) + throws TransferFailedException, ResourceDoesNotExistException, AuthorizationException + { + Resource resource = new Resource( destination ); + firePutInitiated( resource, source ); + firePutStarted( resource, source ); + firePutCompleted( resource, source ); + } + + public String[] getSupportedProtocols() + { + return new String[] { "noop" }; + } +} diff --git a/maven-artifact/src/test/resources/org/apache/maven/artifact/manager/DefaultWagonManagerTest.xml b/maven-artifact/src/test/resources/org/apache/maven/artifact/manager/DefaultWagonManagerTest.xml index 8f44003fe8..d0c034c344 100644 --- a/maven-artifact/src/test/resources/org/apache/maven/artifact/manager/DefaultWagonManagerTest.xml +++ b/maven-artifact/src/test/resources/org/apache/maven/artifact/manager/DefaultWagonManagerTest.xml @@ -39,6 +39,11 @@ under the License. c org.apache.maven.artifact.manager.WagonC + + org.apache.maven.wagon.Wagon + noop + org.apache.maven.artifact.manager.WagonNoOp + org.apache.maven.artifact.repository.authentication.AuthenticationInfoProvider org.apache.maven.artifact.repository.authentication.DummyAuthenticationInfoProvider