Skip to content

Conversation

@seratch
Copy link
Contributor

@seratch seratch commented Sep 30, 2022

This pull request adds a new way to upload files to Slack.

The legacy files.upload API endpoint now has severe performance issues as described in the following reports:

Slack platform team decided to unlock a new way to upload files using the following endpoints:

This pull request adds supports for the new endpoints, so that now the low-level APIs are available for developers. Having said that, going through the above process for uploading files requires many lines of code on 3rd party app side. Also, following all the steps can be confusing for developers.

For this reason, in addition to the low-level API supports, I propose to add MethodsClient#filesUploadV2 method as a wrapper of the whole file-upload operation. Here are the code examples demonstrating how it works:

import com.slack.api.Slack;
import com.slack.api.methods.request.files.FilesUploadV2Request;
import com.slack.api.methods.response.files.FilesUploadResponse;
import com.slack.api.methods.response.files.FilesUploadV2Response;

import java.io.File;
import java.util.Arrays;
import java.util.List;

public class Sample {
  public static void main(String[] args) throws Exception {

    String token = System.getenv("SLACK_BOT_TOKEN");
    Slack slack = Slack.getInstance();

    // Legacy files.upload code example
    FilesUploadResponse legacyResponse = slack.methods(token).filesUpload(r -> r
        .file(new File("./logo.png"))
        .filename("logo.png")
        .title("Company Logo")
        .initialComment("Here is the latest version of our company logo!")
        .channels(Arrays.asList("C123456"))
    );
    com.slack.api.model.File legacyFile = legacyResponse.getFile();

    // V2 wrapper is mostly the same!
    FilesUploadV2Response v2Response = slack.methods(token).filesUploadV2(r -> r
        .file(new File("./logo.png"))
        .filename("logo.png")
        .title("Company Logo")
        .initialComment("Here is the latest version of our company logo!")
        .channel("C123456") // sharing a file in multiple channels is no longer supported
    );
    com.slack.api.model.File v2File = v2Response.getFile();

    // Now you can upload multiple files at a time!
    FilesUploadV2Response v2MultiFilesResponse = slack.methods(token).filesUploadV2(r -> r
        .fileUploads(Arrays.asList(
            FilesUploadV2Request.FileUpload.builder()
                .file(new File("./logo.png"))
                .filename("logo.png")
                .title("Company Logo")
                .build(),
            FilesUploadV2Request.FileUpload.builder()
                .file(new File("./2022-03-01-meeting-minutes.md"))
                .filename("2022-03-01-meeting-minutes.md")
                .title("2022-03-01 Meeting Minutes")
                .snippetType("text")
                .build()
        ))
        .initialComment("Here are the files I mentioned in the meeting :wave:")
        .channel("C123456")
    );
    List<com.slack.api.model.File> v2Files = v2MultiFilesResponse.getFiles();
  }
}

Once we agree that we can go with this design, I will add the same one to other SDKs (Node, Python).

Category (place an x in each of the [ ])

  • bolt (Bolt for Java)
  • bolt-{sub modules} (Bolt for Java - optional modules)
  • slack-api-client (Slack API Clients)
  • slack-api-model (Slack API Data Models)
  • slack-api-*-kotlin-extension (Kotlin Extensions for Slack API Clients)
  • slack-app-backend (The primitive layer of Bolt for Java)

Requirements

Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you agree to those rules.

@seratch seratch added enhancement M-T: A feature request for new functionality project:slack-api-client project:slack-api-client labels Sep 30, 2022
@seratch seratch added this to the 1.26.0 milestone Sep 30, 2022
@seratch seratch self-assigned this Sep 30, 2022
@codecov
Copy link

codecov bot commented Sep 30, 2022

Codecov Report

Merging #1065 (30732b7) into main (612d9a7) will increase coverage by 0.07%.
The diff coverage is 81.34%.

❗ Current head 30732b7 differs from pull request most recent head 6265e15. Consider uploading reports for the commit 6265e15 to get more accurate results

@@             Coverage Diff              @@
##               main    #1065      +/-   ##
============================================
+ Coverage     76.80%   76.87%   +0.07%     
- Complexity     3733     3765      +32     
============================================
  Files           407      409       +2     
  Lines         11250    11384     +134     
  Branches       1128     1141      +13     
============================================
+ Hits           8640     8752     +112     
- Misses         1936     1948      +12     
- Partials        674      684      +10     
Impacted Files Coverage Δ
.../com/slack/api/methods/impl/MethodsClientImpl.java 84.62% <73.33%> (-0.45%) ⬇️
...om/slack/api/methods/impl/FilesUploadV2Helper.java 80.48% <80.48%> (ø)
...java/com/slack/api/methods/RequestFormBuilder.java 88.43% <92.30%> (+0.03%) ⬆️
.../java/com/slack/api/methods/MethodsRateLimits.java 99.06% <100.00%> (+<0.01%) ⬆️
...slack/api/methods/SlackFilesUploadV2Exception.java 100.00% <100.00%> (ø)
...slack/api/methods/impl/AsyncMethodsClientImpl.java 92.95% <100.00%> (+0.09%) ⬆️
...imits/metrics/impl/BaseMemoryMetricsDatastore.java 80.08% <0.00%> (+0.43%) ⬆️
...va/com/slack/api/socket_mode/SocketModeClient.java 64.33% <0.00%> (+0.63%) ⬆️
.../java/com/slack/api/util/http/SlackHttpClient.java 87.68% <0.00%> (+0.72%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@WilliamBergamin WilliamBergamin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, slick work 💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement M-T: A feature request for new functionality project:slack-api-client project:slack-api-client

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants