Skip to content

Commit

Permalink
Fix #1345 filename & title getting improperly defaulted in filesUploa…
Browse files Browse the repository at this point in the history 
…dV2 (#1346)

Co-authored-by: Eden Zimbelman <zim@o526.net>
  • Loading branch information
@Cheos137 @zimeg
Cheos137 and zimeg committed Aug 19, 2024
1 parent 7859986 commit d4622ca
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 11 deletions.
21 changes: 10 additions & 11 deletions slack-api-client/src/main/java/com/slack/api/methods/impl/MethodsClientImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2333,20 +2333,19 @@ public FilesUploadV2Response filesUploadV2(FilesUploadV2Request req) throws IOEx
if (req.getUploadFiles() != null && req.getUploadFiles().size() > 0) {
// upload multiple files
for (FilesUploadV2Request.UploadFile uploadFile : req.getUploadFiles()) {
if (uploadFile.getTitle() != null) {
uploadFile.setTitle(uploadFile.getTitle());
} else {
String filename = req.getFilename();
if (filename == null) {
if (uploadFile.getFile() != null && uploadFile.getFile().getName() != null) {
filename = uploadFile.getFile().getName();
} else {
filename = "Uploaded file";
}
String filename = uploadFile.getFilename();
if (filename == null) {
if (uploadFile.getFile() != null && uploadFile.getFile().getName() != null) {
filename = uploadFile.getFile().getName();
} else {
filename = "Uploaded file";
}
uploadFile.setFilename(filename);
}
uploadFile.setFilename(filename);
if (uploadFile.getTitle() == null) {
uploadFile.setTitle(filename);
}

String fileId = helper.uploadFile(req, uploadFile);

FilesCompleteUploadExternalRequest.FileDetails file = new FilesCompleteUploadExternalRequest.FileDetails();
Expand Down
87 changes: 87 additions & 0 deletions slack-api-client/src/test/java/test_with_remote_apis/methods/files_Test.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1167,4 +1167,91 @@ public void filesUploadV2_manual_two_files() throws Exception {
FilesInfoResponse file1info = client.filesInfo(r -> r.file(file1Upload.getFileId()));
assertThat(file1info.getFile().getShares().getPublicChannels().get(randomChannelId), is(notNullValue()));
}

@Test
public void issue1345_filesUploadV2_multiple() throws IOException, SlackApiException {
loadRandomChannelId();
MethodsClient slackMethods = slack.methods(userToken);

byte[] fileData = Files.readAllBytes(Paths.get("src/test/resources/sample.txt"));

// 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
FilesUploadV2Request.UploadFile uploadFile = FilesUploadV2Request.UploadFile.builder().filename("issue1345_test.txt").title("issue1345 test").fileData(fileData).build();
FilesUploadV2Response response = slackMethods.filesUploadV2(r -> r.uploadFiles(Arrays.asList(uploadFile)));

assertThat(response.getError(), is(nullValue()));
assertThat(response.getFiles().get(0).getName(), is("issue1345_test.txt"));
assertThat(response.getFiles().get(0).getTitle(), is("issue1345 test"));
}

@Test
public void issue1345_filesUploadV2_multiple_no_title() throws IOException, SlackApiException {
loadRandomChannelId();
MethodsClient slackMethods = slack.methods(userToken);

byte[] fileData = Files.readAllBytes(Paths.get("src/test/resources/sample.txt"));

// 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
FilesUploadV2Request.UploadFile uploadFile = FilesUploadV2Request.UploadFile.builder().filename("issue1345_test.txt").fileData(fileData).build();
FilesUploadV2Response response = slackMethods.filesUploadV2(r -> r.uploadFiles(Arrays.asList(uploadFile)));

assertThat(response.getError(), is(nullValue()));
assertThat(response.getFiles().get(0).getName(), is("issue1345_test.txt"));
// title should default to filename
assertThat(response.getFiles().get(0).getTitle(), is("issue1345_test.txt"));
}

@Test
public void issue1345_filesUploadV2_multiple_no_filename() throws IOException, SlackApiException {
loadRandomChannelId();
MethodsClient slackMethods = slack.methods(userToken);

byte[] fileData = Files.readAllBytes(Paths.get("src/test/resources/sample.txt"));

// 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
FilesUploadV2Request.UploadFile uploadFile = FilesUploadV2Request.UploadFile.builder().title("issue1345 test").fileData(fileData).build();
FilesUploadV2Response response = slackMethods.filesUploadV2(r -> r.uploadFiles(Arrays.asList(uploadFile)));

assertThat(response.getError(), is(nullValue()));
// 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.
assertThat(response.getFiles().get(0).getName(), is("uploaded_file"));
// title should be unaffected if it has a value set
assertThat(response.getFiles().get(0).getTitle(), is("issue1345 test"));
}

@Test
public void issue1345_filesUploadV2_multiple_no_filename_no_title_file() throws IOException, SlackApiException {
loadRandomChannelId();
MethodsClient slackMethods = slack.methods(userToken);

File file = new File("src/test/resources/sample.txt");

// 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
FilesUploadV2Request.UploadFile uploadFile = FilesUploadV2Request.UploadFile.builder().file(file).build();
FilesUploadV2Response response = slackMethods.filesUploadV2(r -> r.uploadFiles(Arrays.asList(uploadFile)));

assertThat(response.getError(), is(nullValue()));
// 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.
assertThat(response.getFiles().get(0).getName(), is("sample.txt"));
// title should be unaffected if it has a value set
assertThat(response.getFiles().get(0).getTitle(), is("sample.txt"));
}

@Test
public void issue1345_filesUploadV2_multiple_no_filename_no_title() throws IOException, SlackApiException {
loadRandomChannelId();
MethodsClient slackMethods = slack.methods(userToken);

byte[] fileData = Files.readAllBytes(Paths.get("src/test/resources/sample.txt"));

// 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
FilesUploadV2Request.UploadFile uploadFile = FilesUploadV2Request.UploadFile.builder().fileData(fileData).build();
FilesUploadV2Response response = slackMethods.filesUploadV2(r -> r.uploadFiles(Arrays.asList(uploadFile)));

assertThat(response.getError(), is(nullValue()));
// 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.
assertThat(response.getFiles().get(0).getName(), is("uploaded_file"));
// title defaults to filename, which is (as this is defaulted on the client side) "Uploaded file"
assertThat(response.getFiles().get(0).getTitle(), is("Uploaded file"));
}
}

0 comments on commit d4622ca

Please sign in to comment.