Refinements

Cleaned up FtpConnection class. Removed unnecessary try/catch
JoshStark · May 6, 2014 · 6752056 · 6752056
1 parent 169e628
commit 6752056
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 29 deletions.
diff --git a/src/main/java/jftp/connection/FtpConnection.java b/src/main/java/jftp/connection/FtpConnection.java
@@ -51,32 +51,21 @@ public void changeDirectory(String directory) throws FtpException {
 
     @Override
     public String printWorkingDirectory() throws FtpException {
-        
+
         try {
-            
+
             return client.printWorkingDirectory();
-            
+
         } catch (IOException e) {
 
             throw new FtpException("Unable to print the working directory", e);
         }
     }
-    
+
     @Override
     public List<FtpFile> listFiles() throws FtpException {
 
-        String currentDirectory = "";
-
-        try {
-
-            currentDirectory = client.printWorkingDirectory();
-
-            return listFiles(currentDirectory);
-
-        } catch (IOException e) {
-
-            throw new FtpException(String.format(FILE_LISTING_ERROR_MESSAGE, currentDirectory), e);
-        }
+        return listFiles(printWorkingDirectory());
     }
 
     @Override
@@ -86,10 +75,18 @@ public List<FtpFile> listFiles(String remotePath) throws FtpException {
 
         try {
 
-            FTPFile[] ftpFiles = client.listFiles(remotePath);
+            String originalWorkingDirectory = printWorkingDirectory();
+
+            changeDirectory(remotePath);
+
+            String newWorkingDirectory = printWorkingDirectory();
+
+            FTPFile[] ftpFiles = client.listFiles(newWorkingDirectory);
 
             for (FTPFile file : ftpFiles)
-                files.add(toFtpFile(file));
+                files.add(toFtpFile(file, newWorkingDirectory));
+
+            changeDirectory(originalWorkingDirectory);
 
         } catch (IOException e) {
 
@@ -115,17 +112,17 @@ public void download(FtpFile file, String localDirectory) throws FtpException {
             ensureFileHasSuccessfullyDownloaded(hasDownloaded);
 
         } catch (FileNotFoundException e) {
-            
+
             throw new FtpException(String.format(FILE_STREAM_OPEN_FAIL_MESSAGE, localDestination), e);
 
         } catch (IOException e) {
-            
+
             throw new FtpException(String.format(FILE_DOWNLOAD_FAILURE_MESSAGE, file.getName()), e);
         }
     }
 
     @Override
-    public void upload(String localFilePath, String remoteDirectory)  throws FtpException {
+    public void upload(String localFilePath, String remoteDirectory) throws FtpException {
 
         try {
 
@@ -138,10 +135,10 @@ public void upload(String localFilePath, String remoteDirectory)  throws FtpExce
             ensureFileHasSuccessfullyUploaded(hasUploaded);
 
         } catch (FileNotFoundException e) {
-            
+
             throw new FtpException(String.format(COULD_NOT_FIND_FILE_MESSAGE, localFilePath), e);
         } catch (IOException e) {
-            
+
             throw new FtpException("Upload may not have completed.", e);
         }
 
@@ -169,11 +166,11 @@ private void ensureFileHasSuccessfullyDownloaded(boolean hasDownloaded) {
             throw new FtpException("Server returned failure while downloading.");
     }
 
-    private FtpFile toFtpFile(FTPFile ftpFile) throws IOException {
+    private FtpFile toFtpFile(FTPFile ftpFile, String filePath) throws IOException {
 
         String name = ftpFile.getName();
         long fileSize = ftpFile.getSize();
-        String fullPath = String.format("%s%s%s", client.printWorkingDirectory(), FILE_SEPARATOR, ftpFile.getName());
+        String fullPath = filePath + FILE_SEPARATOR + ftpFile.getName();
         long mTime = ftpFile.getTimestamp().getTime().getTime();
         boolean isDirectory = ftpFile.isDirectory();
 

diff --git a/src/test/java/jftp/connection/FtpConnectionTest.java b/src/test/java/jftp/connection/FtpConnectionTest.java
@@ -62,7 +62,7 @@ public void setUp() throws IOException {
 
         mockFtpClient = mock(FTPClient.class);
 
-        when(mockFtpClient.changeWorkingDirectory(DIRECTORY_PATH)).thenReturn(true);
+        when(mockFtpClient.changeWorkingDirectory(anyString())).thenReturn(true);
         when(mockFtpClient.printWorkingDirectory()).thenReturn(DIRECTORY_PATH);
         when(mockFtpClient.retrieveFile(anyString(), any(OutputStream.class))).thenReturn(true);
 
@@ -165,11 +165,12 @@ public void returnedFtpFilesShouldHaveCorrectModifiedDateTimesAgainstThem() {
 
     @Test
     public void whenListingFilesAndGivingRelativePathThenThatPathShouldBeUsedAlongsideCurrentWorkingDir() throws IOException {
-
-        ftpConnection.changeDirectory(DIRECTORY_PATH);
+
+        when(mockFtpClient.printWorkingDirectory()).thenReturn(DIRECTORY_PATH + "/relativePath");
+
         ftpConnection.listFiles("relativePath");
 
-        verify(mockFtpClient).listFiles("relativePath");
+        verify(mockFtpClient).listFiles(DIRECTORY_PATH + "/relativePath");
     }
 
     @Test
@@ -312,6 +313,23 @@ public void ifClientThrowsExceptionWhenTryingToGetWorkingDirectoryThenCatchExcep
         ftpConnection.printWorkingDirectory();
     }
 
+    @Test
+    public void whenListingFilesOnDifferentPathTheClientShouldCDToThatPathThenCDBackWhenFinished() throws IOException {
+
+        when(mockFtpClient.printWorkingDirectory()).thenReturn("initial/directory").thenReturn("another/path");
+
+        ftpConnection.changeDirectory("initial/directory");        
+        ftpConnection.listFiles("another/path");
+
+        InOrder inOrder = Mockito.inOrder(mockFtpClient);
+
+        inOrder.verify(mockFtpClient).printWorkingDirectory();
+        inOrder.verify(mockFtpClient).changeWorkingDirectory("another/path");
+        inOrder.verify(mockFtpClient).printWorkingDirectory();
+        inOrder.verify(mockFtpClient).listFiles("another/path");
+        inOrder.verify(mockFtpClient).changeWorkingDirectory("initial/directory");
+    }
+
     private FTPFile[] createRemoteFTPFiles() {
 
         Calendar calendar = Calendar.getInstance();