Added new rules to SftpConnection#listFiles()

Ensured that it is CDing to the new directory when listing files then CDing back when finished. This ensures that the FULL PATH of the file is correct in relation to the FTP user currently connected. A bit nasty but current libs don't seem to have a built-in #getPath() method.
JoshStark · May 6, 2014 · 7cc1ca7 · 7cc1ca7
1 parent 6752056
commit 7cc1ca7
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 28 deletions.
diff --git a/src/main/java/jftp/connection/SftpConnection.java b/src/main/java/jftp/connection/SftpConnection.java
@@ -20,12 +20,12 @@ public class SftpConnection implements Connection {
 
     private static final String COULD_NOT_FIND_FILE_MESSAGE = "Could not find file: %s";
     private static final String DIRECTORY_DOES_NOT_EXIST_MESSAGE = "Directory %s does not exist.";
+    private static final String FILE_LISTING_ERROR_MESSAGE = "Unable to list files in directory %s";
     private static final String FILE_SEPARATOR = System.getProperty("file.separator");
 
     private static final int MILLIS = 1000;
 
     private ChannelSftp channel;
-    private String currentDirectory = ".";
 
     private FileStreamFactory fileStreamFactory = new FileStreamFactory();
 
@@ -34,20 +34,18 @@ public SftpConnection(ChannelSftp channel) {
     }
 
     @Override
-    public void changeDirectory(String directory) {
+    public void changeDirectory(String directory) throws FtpException {
 
         try {
 
             channel.cd(directory);
-            currentDirectory = channel.pwd();
 
         } catch (SftpException e) {
 
             throw new FtpException(String.format(DIRECTORY_DOES_NOT_EXIST_MESSAGE, directory), e);
         }
     }
 
-
     @Override
     public String printWorkingDirectory() throws FtpException {
 
@@ -64,7 +62,7 @@ public String printWorkingDirectory() throws FtpException {
     @Override
     public List<FtpFile> listFiles() throws FtpException {
 
-        return listFiles(".");
+        return listFiles(printWorkingDirectory());
     }
 
     @Override
@@ -75,16 +73,24 @@ public List<FtpFile> listFiles(String remotePath) throws FtpException {
 
             List<FtpFile> files = new ArrayList<FtpFile>();
 
-            Vector<LsEntry> lsEntries = channel.ls(remotePath);
+            String originalWorkingDirectory = printWorkingDirectory();
+
+            changeDirectory(remotePath);
+
+            String newWorkingDirectory = printWorkingDirectory();
 
+            Vector<LsEntry> lsEntries = channel.ls(newWorkingDirectory);
+
             for (LsEntry entry : lsEntries)
-                files.add(toFtpFile(entry));
+                files.add(toFtpFile(entry, newWorkingDirectory));
 
+            changeDirectory(originalWorkingDirectory);
+
             return files;
 
         } catch (SftpException e) {
 
-            throw new FtpException("Unable to list files in directory " + currentDirectory, e);
+            throw new FtpException(String.format(FILE_LISTING_ERROR_MESSAGE, remotePath), e);
         }
     }
 
@@ -134,11 +140,11 @@ private String determineRemotePath(String localFilePath, String remoteDirectory)
         return safeRemotePath + remotePath.getFileSystem().getSeparator() + uploadAs;
     }
 
-    private FtpFile toFtpFile(LsEntry lsEntry) throws SftpException {
+    private FtpFile toFtpFile(LsEntry lsEntry, String filePath) throws SftpException {
 
         String name = lsEntry.getFilename();
         long fileSize = lsEntry.getAttrs().getSize();
-        String fullPath = String.format("%s%s%s", channel.pwd(), FILE_SEPARATOR, lsEntry.getFilename());
+        String fullPath = String.format("%s%s%s", filePath, FILE_SEPARATOR, lsEntry.getFilename());
         int mTime = lsEntry.getAttrs().getMTime();
         boolean directory = lsEntry.getAttrs().isDir();
 

diff --git a/src/test/java/jftp/connection/SftpConnectionTest.java b/src/test/java/jftp/connection/SftpConnectionTest.java
@@ -6,7 +6,6 @@
 import static org.mockito.Matchers.anyString;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 import static org.mockito.MockitoAnnotations.initMocks;
@@ -102,35 +101,46 @@ public void listFilesMethodShouldCallOnChannelLsMethodForPresentDirectory() thro
 
         sftpConnection.listFiles();
 
-        verify(mockChannel).ls(".");
+        verify(mockChannel).ls(DIRECTORY);
     }
 
     @Test
     public void whenListingFilesGivingRelativePathThenChannelLsMethodShouldUseGivenPath() throws SftpException {
 
+        when(mockChannel.pwd()).thenReturn(DIRECTORY + "/some/other/path");
+
         sftpConnection.listFiles("some/other/path");
 
-        verify(mockChannel).ls("some/other/path");
+        verify(mockChannel).ls(DIRECTORY + "/some/other/path");
     }
-
-    @Test
-    public void setDirectoryMethodShouldCallOnChannelPwdMethodToGetCurrentDirectory() throws SftpException {
-
-        sftpConnection.changeDirectory(DIRECTORY);
-
-        verify(mockChannel, times(1)).pwd();
-    }
-
+
     @Test
-    public void whenLsCommandThrowsExceptionThenItShouldBeCaughtAndWrappedInFileListingExcepion() throws SftpException {
-
+    public void ifUnderlyingChannelIsUnableToListFilesInPWDThenExceptionShouldBeCaughtAndRethrown() throws SftpException {
+        
         expectedException.expect(FtpException.class);
-        expectedException.expectMessage(is(equalTo("Unable to list files in directory .")));
-
-        when(mockChannel.ls(".")).thenThrow(new SftpException(999, ""));
-
+        expectedException.expectMessage(is(equalTo("Unable to list files in directory " + DIRECTORY)));
+        
+        when(mockChannel.ls(DIRECTORY)).thenThrow(new SftpException(0, ""));
+        
         sftpConnection.listFiles();
     }
+
+    @Test
+    public void whenListingFilesInNewDirectoryThenChannelShouldCDThenListThenCDBackToRetainPWD() throws SftpException {
+
+        when(mockChannel.pwd()).thenReturn("initial/directory").thenReturn("another/path");
+
+        sftpConnection.changeDirectory("initial/directory");        
+        sftpConnection.listFiles("another/path");
+
+        InOrder inOrder = Mockito.inOrder(mockChannel);
+
+        inOrder.verify(mockChannel).pwd();
+        inOrder.verify(mockChannel).cd("another/path");
+        inOrder.verify(mockChannel).pwd();
+        inOrder.verify(mockChannel).ls("another/path");
+        inOrder.verify(mockChannel).cd("initial/directory");
+    }
 
     @Test
     public void lsEntriesReturnedFromChannelShouldBeParsedIntoFtpFileAndReturnedInList() {