Skip to content

Commit

Permalink
Updates to #download() methods on Connection
Browse files Browse the repository at this point in the history
Changed to accept two strings instead of FtpFile. Turned out to be pointless
and also stopped users from downloading an already known file.
  • Loading branch information
Josh Greatrex committed May 8, 2014
1 parent b2ee893 commit f1389f2
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 119 deletions.
9 changes: 7 additions & 2 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@ apply plugin: 'eclipse'
apply plugin: 'application'

group 'jftp'
version '0.1-alpha'
version '0.2-alpha'

repositories { mavenCentral() }

dependencies {

compile 'com.jcraft:jsch:0.1.50'
compile 'joda-time:joda-time:2.3'
compile 'commons-net:commons-net:3.3'
Expand All @@ -26,5 +27,9 @@ task jarWithDependencies(type: Jar, dependsOn: 'build') {
from files(sourceSets.main.output.classesDir)
from files(sourceSets.main.output.resourcesDir)

from {configurations.compile.collect {zipTree(it)}}
from {
configurations.compile.collect {
zipTree(it)
}
}
}
73 changes: 4 additions & 69 deletions src/main/java/jftp/connection/Connection.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,80 +7,15 @@

public interface Connection {

/**
* Equivalent of a standard 'cd' on a directory. This will set the current working directory
* on the active connection to the given file path.
*
* @param
* directory - The directory to change to. Equivalent of a 'cd' command.
*
* @throws
* FtpException
*/
void changeDirectory(String directory) throws FtpException;

/**
* Reads out the current directory on the server.
*
* @return Absolute path purporting to the current working directory.
*/

String printWorkingDirectory() throws FtpException;

/**
* Lists all files and directories under the current working directory.
*
* @return
* A list of FtpFiles detailing what is in the current working directory.
*
* @throws
* FtpException
*/
List<FtpFile> listFiles() throws FtpException;

/**
* Lists all files in the given path. This will accept either relative or absolute paths.
*
* E.g. If currently in /foo - listFiles('bar') will list files in /foo/bar.
* E.g. If currently in /foo - listFiles('/bar') will list files in /bar.
*
* @param
* path
*
* @return
* A list of FtpFiles within relative path.
*
* @throws
* FtpException
*/

List<FtpFile> listFiles(String path) throws FtpException;

/**
* Downloads the given file to the given local directory.
*
* @param
* file file/directory to download
*
* @param
* localDirectory Local directory to download item to. This should NOT include the file name as this method
* will handle the creation of the file for the physical download.
*
* @throws
* FtpException
*/
void download(FtpFile file, String localDirectory) throws FtpException;

/**
* Uploads the given file/directory to the given local directory.
*
* @param
* localFilePath Local file/directory to upload.
*
* @param
* remoteDirectory This is the remote directory that the local file will be uploaded to. The path supplied must be
* a valid directory (not including file name).
*
* @throws
* FtpException
*/
void download(String remoteFilePath, String localDirectory) throws FtpException;

void upload(String localFilePath, String remoteDirectory) throws FtpException;
}
21 changes: 10 additions & 11 deletions src/main/java/jftp/connection/FtpConnection.java
Original file line number Diff line number Diff line change
Expand Up @@ -97,15 +97,15 @@ public List<FtpFile> listFiles(String remotePath) throws FtpException {
}

@Override
public void download(FtpFile file, String localDirectory) throws FtpException {
public void download(String remoteFilePath, String localDirectory) throws FtpException {

String localDestination = String.format("%s%s%s", localDirectory, FILE_SEPARATOR, file.getName());
String localDestination = determinePath(remoteFilePath, localDirectory);

try {

OutputStream outputStream = fileStreamFactory.createOutputStream(localDestination);

boolean hasDownloaded = client.retrieveFile(file.getFullPath(), outputStream);
boolean hasDownloaded = client.retrieveFile(remoteFilePath, outputStream);

outputStream.close();

Expand All @@ -117,7 +117,7 @@ public void download(FtpFile file, String localDirectory) throws FtpException {

} catch (IOException e) {

throw new FtpException(String.format(FILE_DOWNLOAD_FAILURE_MESSAGE, file.getName()), e);
throw new FtpException(String.format(FILE_DOWNLOAD_FAILURE_MESSAGE, remoteFilePath), e);
}
}

Expand All @@ -128,7 +128,7 @@ public void upload(String localFilePath, String remoteDirectory) throws FtpExcep

InputStream localFileInputStream = fileStreamFactory.createInputStream(localFilePath);

boolean hasUploaded = client.storeFile(determineRemotePath(localFilePath, remoteDirectory), localFileInputStream);
boolean hasUploaded = client.storeFile(determinePath(localFilePath, remoteDirectory), localFileInputStream);

localFileInputStream.close();

Expand All @@ -141,17 +141,16 @@ public void upload(String localFilePath, String remoteDirectory) throws FtpExcep

throw new FtpException("Upload may not have completed.", e);
}

}

private String determineRemotePath(String localFilePath, String remoteDirectory) {
private String determinePath(String sourcePathWithName, String targetPathWithoutName) {

Path remotePath = Paths.get(remoteDirectory);
Path targetPath = Paths.get(targetPathWithoutName);

String safeRemotePath = remotePath.toString();
String fileName = Paths.get(localFilePath).getFileName().toString();
String safePath = targetPath.toString();
String fileName = Paths.get(sourcePathWithName).getFileName().toString();

return safeRemotePath + remotePath.getFileSystem().getSeparator() + fileName;
return safePath + targetPath.getFileSystem().getSeparator() + fileName;
}

private void ensureFileHasSuccessfullyUploaded(boolean hasUploaded) {
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/jftp/connection/SftpConnection.java
Original file line number Diff line number Diff line change
Expand Up @@ -95,15 +95,15 @@ public List<FtpFile> listFiles(String remotePath) throws FtpException {
}

@Override
public void download(FtpFile file, String localDirectory) throws FtpException {
public void download(String remoteFilePath, String localDirectory) throws FtpException {

try {

channel.get(file.getName(), localDirectory);
channel.get(remoteFilePath, localDirectory);

} catch (SftpException e) {

throw new FtpException("Unable to download file " + file.getName(), e);
throw new FtpException("Unable to download file " + remoteFilePath, e);
}
}

Expand Down
43 changes: 18 additions & 25 deletions src/test/java/jftp/connection/FtpConnectionTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@

import org.apache.commons.net.ftp.FTPClient;
import org.apache.commons.net.ftp.FTPFile;
import org.joda.time.DateTime;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
Expand All @@ -36,9 +35,7 @@
public class FtpConnectionTest {

private static final String LOCAL_DIRECTORY = ".";
private static final String TEST_DOWNLOAD_FILE = "jUnit_Mock_File.txt";
private static final String DIRECTORY_PATH = "this/is/a/directory";
private static final String FILE_SEPARATOR = System.getProperty("file.separator");

@InjectMocks
private FtpConnection ftpConnection;
Expand Down Expand Up @@ -172,46 +169,45 @@ public void whenListingFilesAndGivingRelativePathThenThatPathShouldBeUsedAlongsi

verify(mockFtpClient).listFiles(DIRECTORY_PATH + "/relativePath");
}

@Test
public void downloadMethodShouldCreateLocalFileStreamFromCorrectPathBasedOnRemoteFileName() throws FileNotFoundException {

ftpConnection.download("path/to/remote.file", LOCAL_DIRECTORY);

verify(mockFileStreamFactory).createOutputStream(LOCAL_DIRECTORY + "/remote.file");
}

@Test
public void downloadMethodShouldCallOnFtpClientRetrieveFilesMethodWithRemoteFilename() throws IOException {

FtpFile file = new FtpFile(TEST_DOWNLOAD_FILE, 1000, "/full/path/to/FileToDownload.txt", new DateTime().getMillis(),
false);
ftpConnection.download("path/to/remote.file", LOCAL_DIRECTORY);

ftpConnection.download(file, LOCAL_DIRECTORY);

verify(mockFtpClient).retrieveFile(file.getFullPath(), mockFileOutputStream);
verify(mockFtpClient).retrieveFile("path/to/remote.file", mockFileOutputStream);
}

@Test
public void downloadMethodShouldThrowExceptionIfUnableToOpenStreamToLocalFile() throws IOException {

expectedException.expect(FtpException.class);
expectedException
.expectMessage(is(equalTo("Unable to write to local directory ." + FILE_SEPARATOR + TEST_DOWNLOAD_FILE)));

FtpFile file = new FtpFile(TEST_DOWNLOAD_FILE, 1000, "/full/path/to/FileToDownload.txt", new DateTime().getMillis(),
false);
.expectMessage(is(equalTo("Unable to write to local directory " + LOCAL_DIRECTORY + "/remote.file")));

when(mockFtpClient.retrieveFile(file.getFullPath(), mockFileOutputStream)).thenThrow(new FileNotFoundException());
when(mockFtpClient.retrieveFile("path/to/remote.file", mockFileOutputStream)).thenThrow(new FileNotFoundException());

ftpConnection.download(file, LOCAL_DIRECTORY);
ftpConnection.download("path/to/remote.file", LOCAL_DIRECTORY);
}

@Test
public void shouldDownloadFailForAnyReasonWhileInProgressThenCatchIOExceptionAndThrowNewDownloadFailedException()
throws IOException {

expectedException.expect(FtpException.class);
expectedException.expectMessage(is(equalTo("Unable to download file " + TEST_DOWNLOAD_FILE)));
expectedException.expectMessage(is(equalTo("Unable to download file path/to/remote.file")));

FtpFile file = new FtpFile(TEST_DOWNLOAD_FILE, 1000, "/full/path/to/FileToDownload.txt", new DateTime().getMillis(),
false);
when(mockFtpClient.retrieveFile("path/to/remote.file", mockFileOutputStream)).thenThrow(new IOException());

when(mockFtpClient.retrieveFile(file.getFullPath(), mockFileOutputStream)).thenThrow(new IOException());

ftpConnection.download(file, LOCAL_DIRECTORY);
ftpConnection.download("path/to/remote.file", LOCAL_DIRECTORY);
}

@Test
Expand All @@ -220,12 +216,9 @@ public void ifRetrieveFileMethodInClientReturnsFalseThenThrowDownloadFailedExcep
expectedException.expect(FtpException.class);
expectedException.expectMessage(is(equalTo("Server returned failure while downloading.")));

FtpFile file = new FtpFile(TEST_DOWNLOAD_FILE, 1000, "/full/path/to/FileToDownload.txt", new DateTime().getMillis(),
false);

when(mockFtpClient.retrieveFile(file.getFullPath(), mockFileOutputStream)).thenReturn(false);
when(mockFtpClient.retrieveFile("path/to/remote.file", mockFileOutputStream)).thenReturn(false);

ftpConnection.download(file, LOCAL_DIRECTORY);
ftpConnection.download("path/to/remote.file", LOCAL_DIRECTORY);
}

@Test
Expand Down
14 changes: 5 additions & 9 deletions src/test/java/jftp/connection/SftpConnectionTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -178,24 +178,20 @@ public void returnedFtpFilesShouldHaveCorrectModifiedDateTimesAgainstThem() {
@Test
public void downloadMethodShouldCallChannelGetMethodWithFtpFileNameAndDirectory() throws SftpException {

FtpFile file = new FtpFile("File Name.txt", 1000, "/remote/server/dir/File Name.txt", 123456789, false);
sftpConnection.download("path/to/file.txt", "some/directory");

sftpConnection.download(file, "some/directory");

verify(mockChannel).get("File Name.txt", "some/directory");
verify(mockChannel).get("path/to/file.txt", "some/directory");
}

@Test
public void downloadMethodShouldThrowDownloadFailedExceptionWhenChannelThrowsSftpConnection() throws SftpException {

expectedException.expect(FtpException.class);
expectedException.expectMessage(is(equalTo("Unable to download file File Name.txt")));

doThrow(new SftpException(999, "")).when(mockChannel).get(anyString(), anyString());
expectedException.expectMessage(is(equalTo("Unable to download file path/to/file.txt")));

FtpFile file = new FtpFile("File Name.txt", 1000, "remote/server/dir/File Name.txt", 123456789, false);
doThrow(new SftpException(999, "")).when(mockChannel).get("path/to/file.txt", "some/directory");

sftpConnection.download(file, "some/directory");
sftpConnection.download("path/to/file.txt", "some/directory");
}

@Test
Expand Down

0 comments on commit f1389f2

Please sign in to comment.