Skip to content

Commit 885e553

Browse files
dotasekGrahame Grieve
dotasek
and
Grahame Grieve
authored
FTPClient: upload file path creation, logging, and improved default path (#1078)
* More logging of errors. * WIP create path for file to upload. * WIP Tests * More tests * Improved logging and path fixes * Clean up imports * Rename test * add testing code Co-authored-by: dotasek <david.otasek@smilecdr.com> Co-authored-by: Grahame Grieve <grahameg@gmail.ccom>
1 parent d707131 commit 885e553

File tree

2 files changed

+191
-36
lines changed

2 files changed

+191
-36
lines changed

‎org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/FTPClient.java‎

Lines changed: 126 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,39 @@
11
package org.hl7.fhir.utilities;
22

3+
import lombok.Getter;
34
import org.apache.commons.net.ftp.FTP;
45
import org.apache.commons.net.ftp.FTPReply;
6+
import org.hl7.fhir.exceptions.FHIRException;
7+
import org.slf4j.Logger;
8+
import org.slf4j.LoggerFactory;
59

6-
7-
import java.io.File;
810
import java.io.FileInputStream;
911
import java.io.IOException;
1012

1113
public class FTPClient {
1214

15+
private static final Logger logger = LoggerFactory.getLogger(FTPClient.class);
16+
1317
private final org.apache.commons.net.ftp.FTPClient clientImpl;
1418

15-
final String server;
19+
@Getter
20+
private final String server;
21+
22+
@Getter
23+
private final String path;
1624

17-
final String path;
25+
private String resolvedPath = null;
1826

19-
final String user;
27+
@Getter
28+
private final String user;
2029

21-
final String password;
30+
@Getter
31+
private final String password;
2232

23-
final int port;
33+
@Getter
34+
private final int port;
35+
36+
private final String remoteSeparator;
2437

2538
/**
2639
* Connect to an FTP server
@@ -36,17 +49,26 @@ public FTPClient(String server, String path, String user, String password) {
3649
protected FTPClient(String server, int port, String path, String user, String password) {
3750
this.server = server;
3851
this.port = port;
39-
if (path.endsWith("/")) {
40-
this.path = path;
41-
} else {
42-
this.path = path + "/";
43-
}
52+
this.remoteSeparator = "/";
53+
this.path = buildPath(path);
54+
4455
this.user = user;
4556
this.password = password;
4657

4758
clientImpl = new org.apache.commons.net.ftp.FTPClient();
4859
}
4960

61+
private String buildPath(String path) {
62+
if (path.length() == 0) {
63+
return "";
64+
}
65+
if (path.endsWith(remoteSeparator))
66+
{
67+
return path;
68+
}
69+
return path + remoteSeparator;
70+
}
71+
5072
/**
5173
* Connect to the server, throw an exception if it fails
5274
*/
@@ -60,13 +82,17 @@ public void connect() throws IOException {
6082

6183
clientImpl.login(user, password);
6284

85+
checkForPositiveCompletionAndLogErrors("FTP server could not connect.", true);
6386

64-
int reply = clientImpl.getReplyCode();
87+
logger.debug("Initial Working directory: " + clientImpl.printWorkingDirectory());
6588

66-
if(!FTPReply.isPositiveCompletion(reply)) {
67-
clientImpl.disconnect();
68-
throw new IOException("FTP server refused connection. Reply code: " + reply);
69-
}
89+
clientImpl.changeWorkingDirectory(path);
90+
91+
checkForPositiveCompletionAndLogErrors("FTP server could not establish default working directory", true);
92+
93+
resolvedPath = clientImpl.printWorkingDirectory();
94+
95+
logger.debug("Resolved working directory: " + resolvedPath);
7096
}
7197

7298
/**
@@ -76,11 +102,53 @@ public void connect() throws IOException {
76102
*/
77103
public void delete(String path) throws IOException {
78104
String resolvedPath = resolveRemotePath(path);
105+
logger.debug("Deleting remote file: " + resolvedPath);
79106
clientImpl.deleteFile(resolvedPath);
107+
checkForPositiveCompletionAndLogErrors("Error deleting file.", false);
108+
logger.debug("Remote file deleted: " + resolvedPath);
109+
}
110+
111+
/**
112+
* Takes a file path and creates all intermediate directories if they do not yet exist.
113+
* @param filePath relative to the path provided in the constructor and including the file name
114+
* @throws IOException
115+
*/
116+
protected void createRemotePathIfNotExists(String filePath) throws IOException {
117+
String[] subPath = filePath.split(remoteSeparator);
118+
try {
119+
for (int i = 0 ; i < subPath.length - 1; i++){
120+
if (subPath[i].isEmpty() ) {
121+
continue;
122+
}
123+
boolean exists = clientImpl.changeWorkingDirectory(subPath[i]);
124+
if (!exists) {
125+
logger.debug("Remote directory does not exist: " + clientImpl.printWorkingDirectory() + remoteSeparator + subPath[i]);
126+
clientImpl.makeDirectory(subPath[i]);
127+
clientImpl.changeWorkingDirectory(subPath[i]);
128+
logger.debug("Made remote directory: " + clientImpl.printWorkingDirectory());
129+
}
130+
}} catch (IOException e) {
131+
throw new IOException("Error creating remote path: " + filePath, e);
132+
} finally {
133+
clientImpl.changeWorkingDirectory(this.resolvedPath);
134+
}
135+
}
136+
137+
protected boolean remotePathExists(String path) throws IOException {
138+
boolean output;
139+
try {
140+
output = clientImpl.changeWorkingDirectory(path);
141+
} finally {
142+
clientImpl.changeWorkingDirectory(this.resolvedPath);
143+
}
144+
return output;
80145
}
81146

82147
private String resolveRemotePath(String path) {
83-
return String.join("", this.path, path);
148+
if (path.startsWith(remoteSeparator)) {
149+
throw new IllegalArgumentException("Absolute remote path is not permitted. Path: " + path);
150+
}
151+
return String.join(remoteSeparator, path);
84152
}
85153

86154
/**
@@ -90,21 +158,59 @@ private String resolveRemotePath(String path) {
90158
*/
91159
public void upload(String source, String path) throws IOException {
92160
String resolvedPath = resolveRemotePath(path);
161+
logger.debug("Uploading file to remote path: " + resolvedPath);
162+
createRemotePathIfNotExists(path);
163+
93164
FileInputStream localStream = new FileInputStream(source);
94165
clientImpl.setFileType(FTP.BINARY_FILE_TYPE);
95166
clientImpl.enterLocalPassiveMode();
96167
clientImpl.storeFile( resolvedPath, localStream);
97168
localStream.close();
98169

170+
checkForPositiveCompletionAndLogErrors("Error uploading file.", false);
171+
logger.debug("Remote file uploaded: " + resolvedPath);
172+
}
173+
174+
private void checkForPositiveCompletionAndLogErrors(String localErrorMessage, boolean disconnectOnError) throws IOException {
99175
int reply = clientImpl.getReplyCode();
100176

101-
if(!FTPReply.isPositiveCompletion(reply)) {
102-
throw new IOException("Error uploading file. Reply code: " + reply);
177+
if (FTPReply.isPositiveCompletion(reply)) {
178+
return;
103179
}
180+
181+
String remoteErrorMessage = clientImpl.getReplyString();
182+
if (disconnectOnError) {
183+
clientImpl.disconnect();
184+
}
185+
throw new IOException(localErrorMessage + " Reply code: " + reply + " Message: " + remoteErrorMessage);
186+
187+
104188
}
105189

106190
public void disconnect() throws IOException {
107191
clientImpl.disconnect();
108192
}
109193

194+
195+
public static void main(String[] args) throws IOException, FHIRException {
196+
FTPClient ftp = new FTPClient(getNamedParam(args, "-upload-server"), getNamedParam(args, "-upload-path"), getNamedParam(args, "-upload-user"), getNamedParam(args, "-upload-password"));
197+
ftp.connect();
198+
ftp.upload("/Users/grahamegrieve/temp/test.xml", "testing/test.xml");
199+
ftp.delete("testing/test.xml");
200+
ftp.disconnect();
201+
}
202+
203+
private static String getNamedParam(String[] args, String param) {
204+
boolean found = false;
205+
for (String a : args) {
206+
if (found)
207+
return a;
208+
if (a.equals(param)) {
209+
found = true;
210+
}
211+
}
212+
return null;
213+
}
214+
215+
110216
}

‎org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/FTPClientTest.java‎

Lines changed: 65 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,14 @@
33

44
import org.apache.commons.io.IOUtils;
55
import org.hl7.fhir.utilities.tests.ResourceLoaderTests;
6-
import org.jetbrains.annotations.NotNull;
6+
77
import org.junit.jupiter.api.*;
8+
import org.junit.jupiter.params.ParameterizedTest;
9+
import org.junit.jupiter.params.provider.CsvSource;
810
import org.mockftpserver.fake.FakeFtpServer;
911
import org.mockftpserver.fake.UserAccount;
1012
import org.mockftpserver.fake.filesystem.*;
1113

12-
import java.io.File;
1314
import java.io.IOException;
1415
import java.io.InputStream;
1516
import java.nio.charset.StandardCharsets;
@@ -31,6 +32,7 @@ public class FTPClientTest implements ResourceLoaderTests {
3132
public static final String DUMMY_FILE_TO_UPLOAD = "dummyFileToUpload";
3233
public static final int FAKE_FTP_PORT = 8021;
3334
public static final String DUMMY_FILE_CONTENT = "Dummy file content\nMore content\n";
35+
public static final String LOCALHOST = "localhost";
3436

3537

3638
FakeFtpServer fakeFtpServer;
@@ -69,8 +71,6 @@ private String createDummyFileContent() {
6971
}
7072

7173
public void setupFakeFtpServer() throws IOException {
72-
73-
7474
fakeFtpServer = new FakeFtpServer();
7575
fakeFtpServer.setServerControlPort(FAKE_FTP_PORT);
7676
fakeFtpServer.addUserAccount(new UserAccount(DUMMY_USER, DUMMY_PASSWORD, fakeFtpDirectory.toFile().getAbsolutePath()));
@@ -108,6 +108,21 @@ public void tearDownFakeFtpServer() {
108108
fakeFtpServer.stop();
109109
}
110110

111+
@ParameterizedTest
112+
@CsvSource({"/", "/test", "/test", "/test/", "/test1/test2", "/test1/test2", "test", "test/", "test1/test2"})
113+
public void testValidRelativePaths(String path) {
114+
FTPClient client = new FTPClient("localhost", path, DUMMY_USER, DUMMY_PASSWORD);
115+
assertTrue(path.length() == client.getPath().length() || path.length() + 1 == client.getPath().length());
116+
assertTrue(client.getPath().startsWith(path));
117+
assertTrue(client.getPath().endsWith("/"));
118+
}
119+
120+
@Test
121+
public void testEmptyRelativePath() {
122+
FTPClient client = new FTPClient("localhost", "", DUMMY_USER, DUMMY_PASSWORD);
123+
assertEquals("", client.getPath());
124+
}
125+
111126
@Test
112127
public void testDelete() throws IOException {
113128

@@ -120,17 +135,16 @@ public void testDelete() throws IOException {
120135
assertFalse(fakeFtpServer.getFileSystem().exists(deleteFilePath));
121136
}
122137

123-
@NotNull
124138
private static FTPClient connectToFTPClient() throws IOException {
125-
FTPClient client = new FTPClient("localhost", FAKE_FTP_PORT, RELATIVE_PATH_1 + "/", DUMMY_USER, DUMMY_PASSWORD);
139+
FTPClient client = new FTPClient(LOCALHOST, FAKE_FTP_PORT, RELATIVE_PATH_1, DUMMY_USER, DUMMY_PASSWORD);
126140
client.connect();
127141
return client;
128142
}
129143

130144
@Test
131145
public void testDelete2() throws IOException {
132146

133-
FTPClient client = connectToFTPClient2();
147+
FTPClient client = connectToFTPClient();
134148

135149
String deleteFilePath = dummyFileToDeletePath.toFile().getAbsolutePath();
136150
assertTrue(fakeFtpServer.getFileSystem().exists(deleteFilePath));
@@ -140,30 +154,65 @@ public void testDelete2() throws IOException {
140154
client.disconnect();
141155
}
142156

143-
@NotNull
144-
private static FTPClient connectToFTPClient2() throws IOException {
145-
FTPClient client = new FTPClient("localhost", FAKE_FTP_PORT, RELATIVE_PATH_1, DUMMY_USER, DUMMY_PASSWORD);
146-
client.connect();
147-
return client;
148-
}
149157

150158
@Test
151159
public void testUpload() throws IOException {
152160

153161
FTPClient client = connectToFTPClient();
154162

155163
String uploadFilePath = dummyUploadedFilePath.toFile().getAbsolutePath();
156-
157164
assertFalse(fakeFtpServer.getFileSystem().exists(uploadFilePath));
158165

159166
client.upload(dummyFileToUploadPath.toFile().getAbsolutePath(), RELATIVE_PATH_2 + "/" + DUMMY_FILE_TO_UPLOAD);
160167

161-
assertTrue(fakeFtpServer.getFileSystem().exists(uploadFilePath));
168+
assertUploadedFileCorrect(uploadFilePath);
169+
}
162170

163-
FileEntry fileEntry= (FileEntry)fakeFtpServer.getFileSystem().getEntry(uploadFilePath);
171+
private void assertUploadedFileCorrect(String uploadedFilePath) throws IOException {
172+
assertTrue(fakeFtpServer.getFileSystem().exists(uploadedFilePath));
173+
FileEntry fileEntry = (FileEntry)fakeFtpServer.getFileSystem().getEntry(uploadedFilePath);
174+
assertNotNull(fileEntry);
164175
InputStream inputStream = fileEntry.createInputStream();
165176
byte[] bytes = IOUtils.toByteArray(inputStream);
166177
String actualContent = new String(bytes, StandardCharsets.UTF_8);
167178
assertEquals(DUMMY_FILE_CONTENT,actualContent);
168179
}
180+
181+
@Test
182+
public void testCreateRemotePathDoesntExist() throws IOException {
183+
FTPClient client = connectToFTPClient();
184+
185+
Path newPath1 = relativePath2.resolve("newPath1");
186+
Path newPath2 = newPath1.resolve("newPath2");
187+
188+
assertFalse(fakeFtpServer.getFileSystem().exists(newPath1.toFile().getAbsolutePath()));
189+
assertFalse(fakeFtpServer.getFileSystem().exists(newPath2.toFile().getAbsolutePath()));
190+
191+
client.createRemotePathIfNotExists(RELATIVE_PATH_2 + "/newPath1/newPath2/newFile.txt");
192+
193+
assertTrue(fakeFtpServer.getFileSystem().exists(newPath1.toFile().getAbsolutePath()));
194+
assertTrue(fakeFtpServer.getFileSystem().exists(newPath2.toFile().getAbsolutePath()));
195+
196+
}
197+
198+
@Test
199+
public void testUploadWherePathDoesntExist() throws IOException {
200+
Path newPath1 = relativePath2.resolve("newPath1");
201+
Path newPath2 = newPath1.resolve("newPath2");
202+
203+
FTPClient client = connectToFTPClient();
204+
205+
Path uploadFilePath = newPath2.resolve(DUMMY_FILE_TO_UPLOAD);
206+
assertFalse(fakeFtpServer.getFileSystem().exists(uploadFilePath.toFile().getAbsolutePath()));
207+
208+
assertFalse(fakeFtpServer.getFileSystem().exists(newPath1.toFile().getAbsolutePath()));
209+
assertFalse(fakeFtpServer.getFileSystem().exists(newPath2.toFile().getAbsolutePath()));
210+
211+
client.upload(dummyFileToUploadPath.toFile().getAbsolutePath(),RELATIVE_PATH_2 + "/newPath1/newPath2/" + DUMMY_FILE_TO_UPLOAD);
212+
213+
assertTrue(fakeFtpServer.getFileSystem().exists(newPath1.toFile().getAbsolutePath()));
214+
assertTrue(fakeFtpServer.getFileSystem().exists(newPath2.toFile().getAbsolutePath()));
215+
216+
assertUploadedFileCorrect(uploadFilePath.toFile().getAbsolutePath());
217+
}
169218
}

0 commit comments

Comments
 (0)