Ticket 758: Use of temporary file during FTP file upload master matsFeatureBranch jenkins-Beast-382
authorMats Vernersson <mats.vernersson@smhi.se>
Tue, 11 Dec 2018 13:56:10 +0000 (14:56 +0100)
committerMats Vernersson <mats.vernersson@smhi.se>
Tue, 11 Dec 2018 13:56:10 +0000 (14:56 +0100)
FTP upload handler updated to first write to a temporary file named .<target_filename>. When writing is completed, the file is renamed to <target_filename>.

src/eu/baltrad/beast/net/ftp/FTPFileUploadHandler.java
src/eu/baltrad/beast/net/scp/SCPOnlyFileUploadHandler.java
test/eu/baltrad/beast/net/ftp/FTPFileUploadHandlerTest.java

index 3687f32..46a3cbc 100644 (file)
@@ -28,10 +28,14 @@ import org.apache.commons.net.ftp.FTP;
 import org.apache.commons.net.ftp.FTPClient;
 import org.apache.commons.net.ftp.FTPFile;
 import org.apache.commons.net.ftp.FTPReply;
+import org.apache.log4j.LogManager;
+import org.apache.log4j.Logger;
 
 import eu.baltrad.beast.net.FileUploadHandlerBase;
 
 public class FTPFileUploadHandler extends FileUploadHandlerBase {
+  private final static Logger logger = LogManager.getLogger(FTPFileUploadHandler.class);
+  
   protected static final int DEFAULT_CONNECT_TIMEOUT = 10000;
   protected static final int DEFAULT_SOCKET_TIMEOUT = 60000;
 
@@ -76,18 +80,33 @@ public class FTPFileUploadHandler extends FileUploadHandlerBase {
   protected void store(File src, URI dst) throws IOException {
     File dstPath = getPath(dst);
     String dstFilename = src.getName();
+    
     if (!isDirectory(dstPath)) {
       dstFilename = dstPath.getName();
       dstPath = dstPath.getParentFile();
     }
-    if (!client.changeWorkingDirectory(dstPath.toString()))
-      throw new IOException("Failed to cwd: " + dstPath);
-    if (!client.setFileType(FTP.BINARY_FILE_TYPE))
-      throw new IOException("Failed to set binary transfer mode");
+    
+    if (!client.changeWorkingDirectory(dstPath.toString())) {
+      throw new IOException("Failed to cwd: " + dstPath);      
+    }
+    
+    if (!client.setFileType(FTP.BINARY_FILE_TYPE)) {
+      throw new IOException("Failed to set binary transfer mode");      
+    }
+    
     InputStream is = openStream(src);
+    String tempDstFileName = "." + dstFilename;
     try {
-      if (!client.storeFile(dstFilename, is))
-        throw new IOException("Failed to store " + src.toString());
+      if (client.storeFile(tempDstFileName, is)) {
+        logger.debug("Storing temporary file: " + tempDstFileName);
+        if (!client.rename(tempDstFileName, dstFilename)) {
+          throw new IOException("Failed to rename temporary file " + tempDstFileName + " to " + dstFilename);
+        } else {
+          logger.debug("Renamed temporary file from " + tempDstFileName + " to target name " + dstFilename);          
+        }
+      } else {
+        throw new IOException("Failed to store " + src.toString());                
+      }
     } finally {
       if (is != null) {
         is.close();
index ac8eff8..a592adb 100644 (file)
@@ -9,11 +9,7 @@ import net.schmizz.sshj.xfer.FileSystemFile;
 import net.schmizz.sshj.xfer.scp.SCPFileTransfer;
 import net.schmizz.sshj.xfer.scp.ScpCommandLine;
 
-import org.apache.log4j.LogManager;
-import org.apache.log4j.Logger;
-
 public class SCPOnlyFileUploadHandler extends SCPFileUploadHandler {
-  private final static Logger logger = LogManager.getLogger(SCPOnlyFileUploadHandler.class);
 
   /**
    * @see eu.baltrad.beast.net.FileUploadHandler#upload(java.io.File, java.net.URI)
index b1091ab..c4899f5 100644 (file)
@@ -254,7 +254,8 @@ public class FTPFileUploadHandlerTest extends EasyMockSupport {
     expect(client.changeWorkingDirectory("/remote/path")).andReturn(true);
     expect(client.setFileType(FTPClient.BINARY_FILE_TYPE)).andReturn(true);
     expect(methods.openStream(src)).andReturn(stream);
-    expect(client.storeFile("file", stream)).andReturn(true);
+    expect(client.storeFile(".file", stream)).andReturn(true);
+    expect(client.rename(".file", "file")).andReturn(true);
 
     replayAll();
 
@@ -285,7 +286,8 @@ public class FTPFileUploadHandlerTest extends EasyMockSupport {
     expect(client.changeWorkingDirectory("/remote/path")).andReturn(true);
     expect(client.setFileType(FTPClient.BINARY_FILE_TYPE)).andReturn(true);
     expect(methods.openStream(src)).andReturn(stream);
-    expect(client.storeFile("newfilename", stream)).andReturn(true);
+    expect(client.storeFile(".newfilename", stream)).andReturn(true);
+    expect(client.rename(".newfilename", "newfilename")).andReturn(true);
 
     replayAll();
 
@@ -365,7 +367,7 @@ public class FTPFileUploadHandlerTest extends EasyMockSupport {
     expect(client.changeWorkingDirectory("/remote/path")).andReturn(true);
     expect(client.setFileType(FTPClient.BINARY_FILE_TYPE)).andReturn(true);
     expect(methods.openStream(src)).andReturn(stream);
-    expect(client.storeFile("file", stream)).andReturn(false);
+    expect(client.storeFile(".file", stream)).andReturn(false);
 
     replayAll();
 
@@ -376,6 +378,43 @@ public class FTPFileUploadHandlerTest extends EasyMockSupport {
     
     verifyAll();
   }
+  
+  @Test
+  public void testStore_renameFailure() throws Exception {
+    URI dst = URI.create("ftp://user:passwd@host/remote/path");
+    File src = new File("/path/to/target.tst");
+    InputStream stream = new InputStream() {
+      @Override public int read() { return -1; } 
+    };
+
+    classUnderTest = new FTPFileUploadHandler(client) {
+      protected InputStream openStream(File f) throws IOException {
+        return methods.openStream(f);
+      }
+
+      protected boolean isDirectory(File path) throws IOException {
+        return methods.isDirectory(path);
+      }
+    };
+
+    expect(methods.isDirectory(new File("/remote/path"))).andReturn(true);
+    expect(client.changeWorkingDirectory("/remote/path")).andReturn(true);
+    expect(client.setFileType(FTPClient.BINARY_FILE_TYPE)).andReturn(true);
+    expect(methods.openStream(src)).andReturn(stream);
+    expect(client.storeFile(".target.tst", stream)).andReturn(true);
+    expect(client.rename(".target.tst", "target.tst")).andReturn(false);
+
+    replayAll();
+
+    try {
+      classUnderTest.store(src, dst);
+      fail("expected IOException");
+    } catch (IOException e) { 
+      assertTrue(e.getMessage().equals("Failed to rename temporary file .target.tst to target.tst"));
+    }
+    
+    verifyAll();
+  }
 
   @Test
   public void testIsDirectory() throws Exception {