[MRM-1702] use the fileLockLManager in the code with possible race condition

git-svn-id: https://svn.apache.org/repos/asf/archiva/trunk@1550636 13f79535-47bb-0310-9956-ffa450edef68

6 files changed, 112 insertions, 39 deletions

diff --git a/archiva-modules/archiva-base/archiva-filelock/src/main/java/org/apache/archiva/common/filelock/DefaultFileLockManager.java b/archiva-modules/archiva-base/archiva-filelock/src/main/java/org/apache/archiva/common/filelock/DefaultFileLockManager.java
index 812ffe9a1..4adf69b4f 100644
--- a/archiva-modules/archiva-base/archiva-filelock/src/main/java/org/apache/archiva/common/filelock/DefaultFileLockManager.java
+++ b/archiva-modules/archiva-base/archiva-filelock/src/main/java/org/apache/archiva/common/filelock/DefaultFileLockManager.java
@@ -33,7 +33,7 @@ import java.util.concurrent.ConcurrentMap;
 /**
  * @author Olivier Lamy
  */
-@Service("fileLockManager#default")
+@Service( "fileLockManager#default" )
 public class DefaultFileLockManager
     implements FileLockManager
 {
@@ -45,6 +45,7 @@ public class DefaultFileLockManager
 
     private int timeout = 0;
 
+
     @Override
     public Lock readFileLock( File file )
         throws FileLockException, FileLockTimeoutException
@@ -56,7 +57,7 @@ public class DefaultFileLockManager
         }
         StopWatch stopWatch = new StopWatch();
         boolean acquired = false;
-
+        mkdirs( file.getParentFile() );
         try
         {
             Lock lock = new Lock( file, false );
@@ -108,6 +109,8 @@ public class DefaultFileLockManager
             return new Lock( file );
         }
 
+        mkdirs( file.getParentFile() );
+
         StopWatch stopWatch = new StopWatch();
         boolean acquired = false;
 
@@ -176,6 +179,36 @@ public class DefaultFileLockManager
         }
     }
 
+    private boolean mkdirs( File directory )
+    {
+        if ( directory == null )
+        {
+            return false;
+        }
+
+        if ( directory.exists() )
+        {
+            return false;
+        }
+        if ( directory.mkdir() )
+        {
+            return true;
+        }
+
+        File canonDir = null;
+        try
+        {
+            canonDir = directory.getCanonicalFile();
+        }
+        catch ( IOException e )
+        {
+            return false;
+        }
+
+        File parentDir = canonDir.getParentFile();
+        return ( parentDir != null && ( mkdirs( parentDir ) || parentDir.exists() ) && canonDir.mkdir() );
+    }
+
     public int getTimeout()
     {
         return timeout;
diff --git a/archiva-modules/archiva-base/archiva-filelock/src/main/java/org/apache/archiva/common/filelock/FileLockManager.java b/archiva-modules/archiva-base/archiva-filelock/src/main/java/org/apache/archiva/common/filelock/FileLockManager.java
index bf18292ae..5b7eeed38 100644
--- a/archiva-modules/archiva-base/archiva-filelock/src/main/java/org/apache/archiva/common/filelock/FileLockManager.java
+++ b/archiva-modules/archiva-base/archiva-filelock/src/main/java/org/apache/archiva/common/filelock/FileLockManager.java
@@ -27,9 +27,23 @@ import java.io.FileNotFoundException;
  */
 public interface FileLockManager
 {
+    /**
+     *
+     * @param file
+     * @return
+     * @throws FileLockException
+     * @throws FileLockTimeoutException
+     */
     Lock writeFileLock( File file )
         throws FileLockException, FileLockTimeoutException;
 
+    /**
+     *
+     * @param file
+     * @return
+     * @throws FileLockException
+     * @throws FileLockTimeoutException
+     */
     Lock readFileLock( File file )
         throws FileLockException, FileLockTimeoutException;
 
diff --git a/archiva-modules/archiva-base/archiva-filelock/src/test/java/org/apache/archiva/common/filelock/DefaultFileLockManagerTest.java b/archiva-modules/archiva-base/archiva-filelock/src/test/java/org/apache/archiva/common/filelock/DefaultFileLockManagerTest.java
index bf756afdf..3f17725e7 100644
--- a/archiva-modules/archiva-base/archiva-filelock/src/test/java/org/apache/archiva/common/filelock/DefaultFileLockManagerTest.java
+++ b/archiva-modules/archiva-base/archiva-filelock/src/test/java/org/apache/archiva/common/filelock/DefaultFileLockManagerTest.java
@@ -70,7 +70,7 @@ public class DefaultFileLockManagerTest
             throws IOException
         {
             this.fileLockManager = fileLockManager;
-            file.createNewFile();
+            //file.createNewFile();
 
         }
 
diff --git a/archiva-modules/archiva-base/archiva-policies/src/main/java/org/apache/archiva/policies/PreDownloadPolicy.java b/archiva-modules/archiva-base/archiva-policies/src/main/java/org/apache/archiva/policies/PreDownloadPolicy.java
index 25d5f531a..426d5e192 100644
--- a/archiva-modules/archiva-base/archiva-policies/src/main/java/org/apache/archiva/policies/PreDownloadPolicy.java
+++ b/archiva-modules/archiva-base/archiva-policies/src/main/java/org/apache/archiva/policies/PreDownloadPolicy.java
@@ -22,9 +22,9 @@ package org.apache.archiva.policies;
 
 /**
  * Policy to apply before the download is attempted.
- *
- *
  */
-public interface PreDownloadPolicy extends DownloadPolicy
+public interface PreDownloadPolicy
+    extends DownloadPolicy
 {
+    // no op
 }
diff --git a/archiva-modules/archiva-base/archiva-proxy/pom.xml b/archiva-modules/archiva-base/archiva-proxy/pom.xml
index 2585c417f..b0c7fe6b9 100644
--- a/archiva-modules/archiva-base/archiva-proxy/pom.xml
+++ b/archiva-modules/archiva-base/archiva-proxy/pom.xml
@@ -53,6 +53,10 @@
     </dependency>
     <dependency>
       <groupId>org.apache.archiva</groupId>
+      <artifactId>archiva-filelock</artifactId>
+    </dependency>
+    <dependency>
+      <groupId>org.apache.archiva</groupId>
       <artifactId>archiva-repository-scanner</artifactId>
     </dependency>
     <dependency>
@@ -225,6 +229,7 @@
               org.apache.maven.wagon.proxy,
               org.apache.maven.wagon.repository,
               com.google.common.io,
+              org.apache.archiva.common.filelock,
               org.slf4j;resolution:=optional
             </Import-Package>
           </instructions>
diff --git a/archiva-modules/archiva-base/archiva-proxy/src/main/java/org/apache/archiva/proxy/DefaultRepositoryProxyConnectors.java b/archiva-modules/archiva-base/archiva-proxy/src/main/java/org/apache/archiva/proxy/DefaultRepositoryProxyConnectors.java
index 72eb22e8e..278c8f76f 100644
--- a/archiva-modules/archiva-base/archiva-proxy/src/main/java/org/apache/archiva/proxy/DefaultRepositoryProxyConnectors.java
+++ b/archiva-modules/archiva-base/archiva-proxy/src/main/java/org/apache/archiva/proxy/DefaultRepositoryProxyConnectors.java
@@ -25,6 +25,10 @@ import org.apache.archiva.admin.model.beans.NetworkProxy;
 import org.apache.archiva.admin.model.beans.ProxyConnectorRuleType;
 import org.apache.archiva.admin.model.beans.RemoteRepository;
 import org.apache.archiva.admin.model.networkproxy.NetworkProxyAdmin;
+import org.apache.archiva.common.filelock.FileLockException;
+import org.apache.archiva.common.filelock.FileLockManager;
+import org.apache.archiva.common.filelock.FileLockTimeoutException;
+import org.apache.archiva.common.filelock.Lock;
 import org.apache.archiva.configuration.ArchivaConfiguration;
 import org.apache.archiva.configuration.Configuration;
 import org.apache.archiva.configuration.ConfigurationNames;
@@ -144,6 +148,10 @@ public class DefaultRepositoryProxyConnectors
     @Inject
     private NetworkProxyAdmin networkProxyAdmin;
 
+    @Inject
+    @Named( value = "fileLockManager#default" )
+    private FileLockManager fileLockManager;
+
     @PostConstruct
     public void initialize()
     {
@@ -431,8 +439,8 @@ public class DefaultRepositoryProxyConnectors
             catch ( RepositoryAdminException e )
             {
                 log.debug( MarkerFactory.getDetachedMarker( "transfer.error" ),
-                           "Transfer error from repository {} for resource {}, continuing to next repository. Error message: {}",targetRepository.getRepository().getId(), path,
-                           e.getMessage(), e );
+                           "Transfer error from repository {} for resource {}, continuing to next repository. Error message: {}",
+                           targetRepository.getRepository().getId(), path, e.getMessage(), e );
                 log.debug( MarkerFactory.getDetachedMarker( "transfer.error" ), "Full stack trace", e );
             }
         }
@@ -490,14 +498,16 @@ public class DefaultRepositoryProxyConnectors
             }
             catch ( ProxyException e )
             {
-                log.warn( "Transfer error from repository {} for versioned Metadata {}, continuing to next repository. Error message: {}",
-                          targetRepository.getRepository().getId(), logicalPath, e.getMessage() );
+                log.warn(
+                    "Transfer error from repository {} for versioned Metadata {}, continuing to next repository. Error message: {}",
+                    targetRepository.getRepository().getId(), logicalPath, e.getMessage() );
                 log.debug( "Full stack trace", e );
             }
             catch ( RepositoryAdminException e )
             {
-                log.warn( "Transfer error from repository {} for versioned Metadata {}, continuing to next repository. Error message: {}",
-                          targetRepository.getRepository().getId(), logicalPath, e.getMessage() );
+                log.warn(
+                    "Transfer error from repository {} for versioned Metadata {}, continuing to next repository. Error message: {}",
+                    targetRepository.getRepository().getId(), logicalPath, e.getMessage() );
                 log.debug( "Full stack trace", e );
             }
         }
@@ -898,7 +908,7 @@ public class DefaultRepositoryProxyConnectors
         catch ( ProxyException e )
         {
             urlFailureCache.cacheFailure( url );
-            log.warn( "Transfer failed on checksum: {} : {}",url ,e.getMessage(), e );
+            log.warn( "Transfer failed on checksum: {} : {}", url, e.getMessage(), e );
             // Critical issue, pass it on.
             throw e;
         }
@@ -1059,7 +1069,7 @@ public class DefaultRepositoryProxyConnectors
 
         log.warn(
             "Transfer error from repository {} for artifact {} , continuing to next repository. Error message: {}",
-            content.getRepository().getId(), Keys.toKey( artifact), exception.getMessage() );
+            content.getRepository().getId(), Keys.toKey( artifact ), exception.getMessage() );
         log.debug( "Full stack trace", exception );
     }
 
@@ -1086,40 +1096,51 @@ public class DefaultRepositoryProxyConnectors
     private void moveTempToTarget( File temp, File target )
         throws ProxyException
     {
-        if ( target.exists() && !target.delete() )
-        {
-            throw new ProxyException( "Unable to overwrite existing target file: " + target.getAbsolutePath() );
-        }
 
-        target.getParentFile().mkdirs();
         // TODO file lock library
-        RandomAccessFile raf;
-
-        if ( !temp.renameTo( target ) )
+        Lock lock = null;
+        try
         {
-            log.warn( "Unable to rename tmp file to its final name... resorting to copy command." );
-
-            try
+            lock = fileLockManager.writeFileLock( target );
+            if ( lock.getFile().exists() && !lock.getFile().delete() )
             {
-                FileUtils.copyFile( temp, target );
+                throw new ProxyException( "Unable to overwrite existing target file: " + target.getAbsolutePath() );
             }
-            catch ( IOException e )
+
+            lock.getFile().getParentFile().mkdirs();
+
+            if ( !temp.renameTo( lock.getFile() ) )
             {
-                if ( target.exists() )
+                log.warn( "Unable to rename tmp file to its final name... resorting to copy command." );
+
+                try
                 {
-                    log.debug( "Tried to copy file {} to {} but file with this name already exists.", temp.getName(),
-                               target.getAbsolutePath() );
+                    FileUtils.copyFile( temp, lock.getFile() );
                 }
-                else
+                catch ( IOException e )
                 {
-                    throw new ProxyException(
-                        "Cannot copy tmp file " + temp.getAbsolutePath() + " to its final location", e );
+                    if ( lock.getFile().exists() )
+                    {
+                        log.debug( "Tried to copy file {} to {} but file with this name already exists.",
+                                   temp.getName(), lock.getFile().getAbsolutePath() );
+                    }
+                    else
+                    {
+                        throw new ProxyException(
+                            "Cannot copy tmp file " + temp.getAbsolutePath() + " to its final location", e );
+                    }
+                }
+                finally
+                {
+                    FileUtils.deleteQuietly( temp );
                 }
             }
-            finally
-            {
-                FileUtils.deleteQuietly( temp );
-            }
+        } catch( FileLockException e)
+        {
+            throw new ProxyException( e.getMessage(), e );
+        } catch (FileLockTimeoutException e)
+        {
+            throw new ProxyException( e.getMessage(), e );
         }
     }
 
@@ -1187,12 +1208,12 @@ public class DefaultRepositoryProxyConnectors
         }
         catch ( ConnectionException e )
         {
-            log.warn( "Could not connect to {}: {}", remoteRepository.getRepository().getName(),  e.getMessage() );
+            log.warn( "Could not connect to {}: {}", remoteRepository.getRepository().getName(), e.getMessage() );
             connected = false;
         }
         catch ( AuthenticationException e )
         {
-            log.warn( "Could not connect to {}: {}", remoteRepository.getRepository().getName(),  e.getMessage() );
+            log.warn( "Could not connect to {}: {}", remoteRepository.getRepository().getName(), e.getMessage() );
             connected = false;
         }