Skip to content

Commit d4622ca

Browse files
Cheos137zimeg
andauthored
Fix #1345 filename & title getting improperly defaulted in filesUploadV2 (#1346)
Co-authored-by: Eden Zimbelman <zim@o526.net>
1 parent 7859986 commit d4622ca

2 files changed

Lines changed: 97 additions & 11 deletions

File tree

slack-api-client/src/main/java/com/slack/api/methods/impl/MethodsClientImpl.java

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2333,20 +2333,19 @@ public FilesUploadV2Response filesUploadV2(FilesUploadV2Request req) throws IOEx
23332333
if (req.getUploadFiles() != null && req.getUploadFiles().size() > 0) {
23342334
// upload multiple files
23352335
for (FilesUploadV2Request.UploadFile uploadFile : req.getUploadFiles()) {
2336-
if (uploadFile.getTitle() != null) {
2337-
uploadFile.setTitle(uploadFile.getTitle());
2338-
} else {
2339-
String filename = req.getFilename();
2340-
if (filename == null) {
2341-
if (uploadFile.getFile() != null && uploadFile.getFile().getName() != null) {
2342-
filename = uploadFile.getFile().getName();
2343-
} else {
2344-
filename = "Uploaded file";
2345-
}
2336+
String filename = uploadFile.getFilename();
2337+
if (filename == null) {
2338+
if (uploadFile.getFile() != null && uploadFile.getFile().getName() != null) {
2339+
filename = uploadFile.getFile().getName();
2340+
} else {
2341+
filename = "Uploaded file";
23462342
}
2347-
uploadFile.setFilename(filename);
2343+
}
2344+
uploadFile.setFilename(filename);
2345+
if (uploadFile.getTitle() == null) {
23482346
uploadFile.setTitle(filename);
23492347
}
2348+
23502349
String fileId = helper.uploadFile(req, uploadFile);
23512350

23522351
FilesCompleteUploadExternalRequest.FileDetails file = new FilesCompleteUploadExternalRequest.FileDetails();

slack-api-client/src/test/java/test_with_remote_apis/methods/files_Test.java

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1167,4 +1167,91 @@ public void filesUploadV2_manual_two_files() throws Exception {
11671167
FilesInfoResponse file1info = client.filesInfo(r -> r.file(file1Upload.getFileId()));
11681168
assertThat(file1info.getFile().getShares().getPublicChannels().get(randomChannelId), is(notNullValue()));
11691169
}
1170+
1171+
@Test
1172+
public void issue1345_filesUploadV2_multiple() throws IOException, SlackApiException {
1173+
loadRandomChannelId();
1174+
MethodsClient slackMethods = slack.methods(userToken);
1175+
1176+
byte[] fileData = Files.readAllBytes(Paths.get("src/test/resources/sample.txt"));
1177+
1178+
// note: this test can be done using only one file as everything tested applies to each file individually - the only important thing is to use "uploadFiles" instead of direct values
1179+
FilesUploadV2Request.UploadFile uploadFile = FilesUploadV2Request.UploadFile.builder().filename("issue1345_test.txt").title("issue1345 test").fileData(fileData).build();
1180+
FilesUploadV2Response response = slackMethods.filesUploadV2(r -> r.uploadFiles(Arrays.asList(uploadFile)));
1181+
1182+
assertThat(response.getError(), is(nullValue()));
1183+
assertThat(response.getFiles().get(0).getName(), is("issue1345_test.txt"));
1184+
assertThat(response.getFiles().get(0).getTitle(), is("issue1345 test"));
1185+
}
1186+
1187+
@Test
1188+
public void issue1345_filesUploadV2_multiple_no_title() throws IOException, SlackApiException {
1189+
loadRandomChannelId();
1190+
MethodsClient slackMethods = slack.methods(userToken);
1191+
1192+
byte[] fileData = Files.readAllBytes(Paths.get("src/test/resources/sample.txt"));
1193+
1194+
// note: this test can be done using only one file as everything tested applies to each file individually - the only important thing is to use "uploadFiles" instead of direct values
1195+
FilesUploadV2Request.UploadFile uploadFile = FilesUploadV2Request.UploadFile.builder().filename("issue1345_test.txt").fileData(fileData).build();
1196+
FilesUploadV2Response response = slackMethods.filesUploadV2(r -> r.uploadFiles(Arrays.asList(uploadFile)));
1197+
1198+
assertThat(response.getError(), is(nullValue()));
1199+
assertThat(response.getFiles().get(0).getName(), is("issue1345_test.txt"));
1200+
// title should default to filename
1201+
assertThat(response.getFiles().get(0).getTitle(), is("issue1345_test.txt"));
1202+
}
1203+
1204+
@Test
1205+
public void issue1345_filesUploadV2_multiple_no_filename() throws IOException, SlackApiException {
1206+
loadRandomChannelId();
1207+
MethodsClient slackMethods = slack.methods(userToken);
1208+
1209+
byte[] fileData = Files.readAllBytes(Paths.get("src/test/resources/sample.txt"));
1210+
1211+
// note: this test can be done using only one file as everything tested applies to each file individually - the only important thing is to use "uploadFiles" instead of direct values
1212+
FilesUploadV2Request.UploadFile uploadFile = FilesUploadV2Request.UploadFile.builder().title("issue1345 test").fileData(fileData).build();
1213+
FilesUploadV2Response response = slackMethods.filesUploadV2(r -> r.uploadFiles(Arrays.asList(uploadFile)));
1214+
1215+
assertThat(response.getError(), is(nullValue()));
1216+
// filename defaults to "Uploaded file", which will be converted by slack servers to be lowercase only, contain no special characters but underscores, dots and dashes, etc.
1217+
assertThat(response.getFiles().get(0).getName(), is("uploaded_file"));
1218+
// title should be unaffected if it has a value set
1219+
assertThat(response.getFiles().get(0).getTitle(), is("issue1345 test"));
1220+
}
1221+
1222+
@Test
1223+
public void issue1345_filesUploadV2_multiple_no_filename_no_title_file() throws IOException, SlackApiException {
1224+
loadRandomChannelId();
1225+
MethodsClient slackMethods = slack.methods(userToken);
1226+
1227+
File file = new File("src/test/resources/sample.txt");
1228+
1229+
// note: this test can be done using only one file as everything tested applies to each file individually - the only important thing is to use "uploadFiles" instead of direct values
1230+
FilesUploadV2Request.UploadFile uploadFile = FilesUploadV2Request.UploadFile.builder().file(file).build();
1231+
FilesUploadV2Response response = slackMethods.filesUploadV2(r -> r.uploadFiles(Arrays.asList(uploadFile)));
1232+
1233+
assertThat(response.getError(), is(nullValue()));
1234+
// filename defaults to "Uploaded file", which will be converted by slack servers to be lowercase only, contain no special characters but underscores, dots and dashes, etc.
1235+
assertThat(response.getFiles().get(0).getName(), is("sample.txt"));
1236+
// title should be unaffected if it has a value set
1237+
assertThat(response.getFiles().get(0).getTitle(), is("sample.txt"));
1238+
}
1239+
1240+
@Test
1241+
public void issue1345_filesUploadV2_multiple_no_filename_no_title() throws IOException, SlackApiException {
1242+
loadRandomChannelId();
1243+
MethodsClient slackMethods = slack.methods(userToken);
1244+
1245+
byte[] fileData = Files.readAllBytes(Paths.get("src/test/resources/sample.txt"));
1246+
1247+
// note: this test can be done using only one file as everything tested applies to each file individually - the only important thing is to use "uploadFiles" instead of direct values
1248+
FilesUploadV2Request.UploadFile uploadFile = FilesUploadV2Request.UploadFile.builder().fileData(fileData).build();
1249+
FilesUploadV2Response response = slackMethods.filesUploadV2(r -> r.uploadFiles(Arrays.asList(uploadFile)));
1250+
1251+
assertThat(response.getError(), is(nullValue()));
1252+
// filename defaults to "Uploaded file", which will be converted by slack servers to be lowercase only, contain no special characters but underscores, dots and dashes, etc.
1253+
assertThat(response.getFiles().get(0).getName(), is("uploaded_file"));
1254+
// title defaults to filename, which is (as this is defaulted on the client side) "Uploaded file"
1255+
assertThat(response.getFiles().get(0).getTitle(), is("Uploaded file"));
1256+
}
11701257
}

0 commit comments

Comments
 (0)