Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AsyncRequestBody: ability to set content-type #5143

Open
1 of 2 tasks
chaykin opened this issue Apr 24, 2024 · 2 comments
Open
1 of 2 tasks

AsyncRequestBody: ability to set content-type #5143

chaykin opened this issue Apr 24, 2024 · 2 comments
Assignees
Labels
feature-request A feature should be added or improved.

Comments

@chaykin
Copy link

chaykin commented Apr 24, 2024

Describe the feature

There is special AsyncRequestBody implementation for uploading content to S3 by InputStream. It is BlockingInputStreamAsyncRequestBody. This class is useful and simple, but it has no way to set custom content-type:

  • class is final (so, no way to inherit from it)
  • no contentType method overriding (so, default implementation from AsyncRequestBody used instead. That implementation simply always returns application/octet-stream)

Use Case

Any time I need to upload my existing InputStream, I want to use this BlockingInputStreamAsyncRequestBody. But, then my content type is differ from application/octet-stream (almost always), I need to have own copy-past implementation with my custom content-type...

Proposed Solution

  • Add private final field contentType to BlockingInputStreamAsyncRequestBody
  • Add new constructor with additional argument contentType
  • Override contentType method to return corresponding field

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

AWS Java SDK version used

2.25.37

JDK version used

java 17.0.6 2023-01-17 LTS Java(TM) SE Runtime Environment (build 17.0.6+9-LTS-190) Java HotSpot(TM) 64-Bit Server VM (build 17.0.6+9-LTS-190, mixed mode, sharing)

Operating System and version

Ubuntu 22.04.3 LTS

@chaykin chaykin added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Apr 24, 2024
@debora-ito
Copy link
Member

Providing content-type to PutObjectRequest (not the request body) should work fine, I believe.

Can you share a repro code showing how you're making the upload?

@debora-ito debora-ito added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. and removed needs-triage This issue or PR still needs to be triaged. labels Apr 25, 2024
@debora-ito debora-ito self-assigned this Apr 25, 2024
@chaykin
Copy link
Author

chaykin commented Apr 25, 2024

I do it like this:

public CompletableFuture<PutObjectResponse> uploadContentAsync(ContentId contentId, AsyncRequestBody body) {
	var requestBuilder = PutObjectRequest.builder()
	        .bucket(contentId.bucket())
	        .key(contentId.key()
	        .contentType(body.contentType()); // Use contentType from body here
	
	return client.putObject(requestBuilder.build(), body);
}

So, content-type can be set in requestBuilder, but use AsyncRequestBody for that seems more preferrable to me...

My point here is: why do we need to add extra-argument contentType to uploadContentAsync method, if AsyncRequestBody has quite similar data already?

It is far more obvious and easy to use single source-of-truth instead of always aware, which source is more thuth here...

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. label Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved.
Projects
None yet
Development

No branches or pull requests

2 participants