Skip to content

Commit 5467632

Browse files
abrooksvabrooksv
authored
Set correct content type on S3 upload (#2721)
1 parent fd836ec commit 5467632

File tree

7 files changed

+80
-59
lines changed

7 files changed

+80
-59
lines changed

‎.changes/next-release/feature-378a18b3-1289-4e13-bf6c-af2908325d4c.json‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"type" : "feature",
3+
"description" : "When uploading a file to S3, the content type is now set accoriding to the files extension"
4+
}

‎jetbrains-core/src/software/aws/toolkits/jetbrains/services/s3/TransferUtils.kt‎

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,9 @@ import com.intellij.openapi.progress.ProgressIndicator
77
import com.intellij.openapi.progress.ProgressManager
88
import com.intellij.openapi.progress.Task
99
import com.intellij.openapi.project.Project
10-
import com.intellij.util.io.inputStream
1110
import com.intellij.util.io.outputStream
12-
import com.intellij.util.io.size
1311
import software.amazon.awssdk.core.sync.RequestBody
12+
import software.amazon.awssdk.http.ContentStreamProvider
1413
import software.amazon.awssdk.services.s3.S3Client
1514
import software.amazon.awssdk.services.s3.model.GetObjectRequest
1615
import software.amazon.awssdk.services.s3.model.GetObjectResponse
@@ -37,12 +36,11 @@ fun S3Client.upload(
3736
key: String,
3837
message: String = message("s3.upload.object.progress", key),
3938
startInBackground: Boolean = true
40-
): CompletionStage<PutObjectResponse> = upload(project, source.inputStream(), source.size(), bucket, key, message, startInBackground)
39+
): CompletionStage<PutObjectResponse> = upload(project, RequestBody.fromFile(source), bucket, key, message, startInBackground)
4140

42-
fun S3Client.upload(
41+
private fun S3Client.upload(
4342
project: Project,
44-
source: InputStream,
45-
length: Long,
43+
source: RequestBody,
4644
bucket: String,
4745
key: String,
4846
message: String = message("s3.upload.object.progress", key),
@@ -53,12 +51,18 @@ fun S3Client.upload(
5351
ProgressManager.getInstance().run(
5452
object : Task.Backgroundable(project, message, true, if (startInBackground) ALWAYS_BACKGROUND else null) {
5553
override fun run(indicator: ProgressIndicator) {
56-
indicator.isIndeterminate = false
54+
indicator.isIndeterminate = source.optionalContentLength().isEmpty
55+
val requestSource = if (source.optionalContentLength().isPresent) {
56+
val length = source.optionalContentLength().get()
57+
val newProvider = ProgressTrackingContentProvider(indicator, source.contentStreamProvider(), length)
58+
59+
RequestBody.fromContentProvider(newProvider, length, source.contentType())
60+
} else {
61+
source
62+
}
63+
5764
try {
58-
val result = ProgressMonitorInputStream(indicator, source, length = length).use {
59-
this@upload.putObject(request, RequestBody.fromInputStream(it, length))
60-
}
61-
future.complete(result)
65+
future.complete(this@upload.putObject(request, requestSource))
6266
} catch (e: Exception) {
6367
future.completeExceptionally(e)
6468
}
@@ -68,6 +72,26 @@ fun S3Client.upload(
6872
return future
6973
}
7074

75+
private class ProgressTrackingContentProvider(
76+
private val progressIndicator: ProgressIndicator,
77+
private val underlyingInputStreamProvider: ContentStreamProvider,
78+
private val length: Long
79+
) : ContentStreamProvider {
80+
private var currentStream: InputStream? = null
81+
82+
override fun newStream(): InputStream {
83+
currentStream?.let {
84+
runCatching {
85+
it.close()
86+
}
87+
}
88+
89+
return ProgressMonitorInputStream(progressIndicator, underlyingInputStreamProvider.newStream(), length).also {
90+
currentStream = it
91+
}
92+
}
93+
}
94+
7195
fun S3Client.download(
7296
project: Project,
7397
bucket: String,

‎jetbrains-core/src/software/aws/toolkits/jetbrains/services/s3/editor/S3TreeTable.kt‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import com.intellij.openapi.project.Project
1010
import com.intellij.openapi.util.io.FileUtil
1111
import com.intellij.openapi.util.io.FileUtilRt.getUserContentLoadLimit
1212
import com.intellij.openapi.util.text.StringUtil
13-
import com.intellij.openapi.vfs.LocalFileSystem
1413
import com.intellij.openapi.vfs.VirtualFileWrapper
1514
import com.intellij.ui.DoubleClickListener
1615
import com.intellij.ui.TreeTableSpeedSearch
@@ -63,12 +62,7 @@ class S3TreeTable(
6362
return
6463
}
6564

66-
val lfs = LocalFileSystem.getInstance()
67-
val virtualFiles = data.mapNotNull {
68-
lfs.findFileByIoFile(it)
69-
}
70-
71-
uploadObjects(project, this@S3TreeTable, virtualFiles, node)
65+
uploadObjects(project, this@S3TreeTable, data.map { it.toPath() }, node)
7266
}
7367
}
7468

‎jetbrains-core/src/software/aws/toolkits/jetbrains/services/s3/editor/S3VirtualBucket.kt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,9 @@ import software.aws.toolkits.jetbrains.utils.getCoroutineBgContext
2727
import software.aws.toolkits.jetbrains.utils.getCoroutineUiContext
2828
import software.aws.toolkits.jetbrains.utils.notifyError
2929
import software.aws.toolkits.resources.message
30-
import java.io.InputStream
3130
import java.io.OutputStream
3231
import java.net.URL
32+
import java.nio.file.Path
3333

3434
class S3VirtualBucket(val s3Bucket: Bucket, prefix: String, val client: S3Client, val project: Project) :
3535
LightVirtualFile(vfsName(s3Bucket, prefix)),
@@ -93,9 +93,9 @@ class S3VirtualBucket(val s3Bucket: Bucket, prefix: String, val client: S3Client
9393
}
9494
}
9595

96-
suspend fun upload(project: Project, source: InputStream, length: Long, key: String) {
96+
suspend fun upload(project: Project, source: Path, key: String) {
9797
withContext(getCoroutineBgContext()) {
98-
client.upload(project, source, length, s3Bucket.name(), key).await()
98+
client.upload(project, source, s3Bucket.name(), key).await()
9999
}
100100
}
101101

‎jetbrains-core/src/software/aws/toolkits/jetbrains/services/s3/objectActions/UploadObjectAction.kt‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import com.intellij.openapi.actionSystem.DataContext
88
import com.intellij.openapi.fileChooser.FileChooserDescriptorFactory
99
import com.intellij.openapi.fileChooser.FileChooserFactory
1010
import com.intellij.openapi.project.Project
11-
import com.intellij.openapi.vfs.VirtualFile
11+
import com.intellij.util.io.isDirectory
1212
import kotlinx.coroutines.launch
1313
import software.aws.toolkits.jetbrains.core.utils.getRequiredData
1414
import software.aws.toolkits.jetbrains.services.s3.editor.S3EditorDataKeys
@@ -20,15 +20,16 @@ import software.aws.toolkits.jetbrains.utils.notifyError
2020
import software.aws.toolkits.resources.message
2121
import software.aws.toolkits.telemetry.Result
2222
import software.aws.toolkits.telemetry.S3Telemetry
23+
import java.nio.file.Path
2324

2425
class UploadObjectAction : S3ObjectAction(message("s3.upload.object.action"), AllIcons.Actions.Upload) {
2526
override fun performAction(dataContext: DataContext, nodes: List<S3TreeNode>) {
2627
val project = dataContext.getRequiredData(CommonDataKeys.PROJECT)
2728
val treeTable = dataContext.getRequiredData(S3EditorDataKeys.BUCKET_TABLE)
2829
val node = nodes.firstOrNull() ?: treeTable.rootNode
2930

30-
val descriptor =
31-
FileChooserDescriptorFactory.createAllButJarContentsDescriptor().withDescription(message("s3.upload.object.action", treeTable.bucket.name))
31+
val descriptor = FileChooserDescriptorFactory.createAllButJarContentsDescriptor()
32+
.withDescription(message("s3.upload.object.action", treeTable.bucket.name))
3233
val chooserDialog = FileChooserFactory.getInstance().createFileChooser(descriptor, project, null)
3334
val filesChosen = chooserDialog.choose(project).toList()
3435

@@ -38,33 +39,33 @@ class UploadObjectAction : S3ObjectAction(message("s3.upload.object.action"), Al
3839
return
3940
}
4041

41-
uploadObjects(project, treeTable, filesChosen, node)
42+
uploadObjects(project, treeTable, filesChosen.map { it.toNioPath() }, node)
4243
}
4344

4445
override fun enabled(nodes: List<S3TreeNode>): Boolean = nodes.isEmpty() ||
4546
(nodes.size == 1 && nodes.first().let { it is S3TreeDirectoryNode })
4647
}
4748

48-
fun uploadObjects(project: Project, treeTable: S3TreeTable, files: List<VirtualFile>, parentNode: S3TreeNode) {
49+
fun uploadObjects(project: Project, treeTable: S3TreeTable, files: List<Path>, parentNode: S3TreeNode) {
4950
if (files.isEmpty()) {
5051
return
5152
}
5253
ApplicationThreadPoolScope("UploadObjectAction").launch {
5354
var changeMade = false
5455
try {
5556
files.forEach {
56-
if (it.isDirectory) {
57+
if (it.isDirectory()) {
5758
notifyError(
58-
title = message("s3.upload.object.failed", it.name),
59-
content = message("s3.upload.directory.impossible", it.name),
59+
title = message("s3.upload.object.failed", it.fileName),
60+
content = message("s3.upload.directory.impossible", it.fileName),
6061
project = project
6162
)
6263
} else {
6364
try {
64-
treeTable.bucket.upload(project, it.inputStream, it.length, parentNode.directoryPath() + it.name)
65+
treeTable.bucket.upload(project, it, parentNode.directoryPath() + it.fileName)
6566
changeMade = true
6667
} catch (e: Exception) {
67-
e.notifyError(message("s3.upload.object.failed", it.path), project)
68+
e.notifyError(message("s3.upload.object.failed", it.fileName), project)
6869
throw e
6970
}
7071
}

‎jetbrains-core/tst/software/aws/toolkits/jetbrains/services/s3/S3VirtualBucketTest.kt‎

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,19 +7,19 @@ import com.intellij.openapi.fileEditor.FileEditorManager
77
import com.intellij.openapi.fileTypes.FileTypes
88
import com.intellij.openapi.fileTypes.ex.FileTypeManagerEx
99
import com.intellij.openapi.util.io.FileUtil
10-
import com.intellij.openapi.vfs.VirtualFile
1110
import com.intellij.testFramework.ProjectRule
11+
import com.intellij.testFramework.RuleChain
1212
import com.intellij.testFramework.runInEdtAndWait
1313
import kotlinx.coroutines.runBlocking
1414
import org.assertj.core.api.Assertions.assertThat
1515
import org.assertj.core.api.Assertions.assertThatThrownBy
1616
import org.junit.Rule
1717
import org.junit.Test
18+
import org.junit.rules.TemporaryFolder
1819
import org.mockito.kotlin.any
1920
import org.mockito.kotlin.argumentCaptor
2021
import org.mockito.kotlin.doAnswer
2122
import org.mockito.kotlin.doReturn
22-
import org.mockito.kotlin.mock
2323
import org.mockito.kotlin.stub
2424
import org.mockito.kotlin.verify
2525
import software.amazon.awssdk.core.sync.RequestBody
@@ -46,23 +46,21 @@ import software.aws.toolkits.jetbrains.core.AwsClientManager
4646
import software.aws.toolkits.jetbrains.core.MockClientManagerRule
4747
import software.aws.toolkits.jetbrains.core.credentials.MockAwsConnectionManager.ProjectAccountSettingsManagerRule
4848
import software.aws.toolkits.jetbrains.services.s3.editor.S3VirtualBucket
49-
import java.io.ByteArrayInputStream
5049
import java.net.URL
5150
import java.time.Instant
5251

5352
class S3VirtualBucketTest {
53+
private val projectRule = ProjectRule()
54+
private val mockClientManager = MockClientManagerRule()
55+
private val settingsManagerRule = ProjectAccountSettingsManagerRule(projectRule)
5456

5557
@Rule
5658
@JvmField
57-
val projectRule = ProjectRule()
58-
59-
@JvmField
60-
@Rule
61-
val mockClientManager = MockClientManagerRule()
59+
val chain = RuleChain(projectRule, mockClientManager, settingsManagerRule)
6260

6361
@Rule
6462
@JvmField
65-
val settingsManagerRule = ProjectAccountSettingsManagerRule(projectRule)
63+
val tempFolder = TemporaryFolder()
6664

6765
@Test
6866
fun deleteObjects() {
@@ -131,30 +129,30 @@ class S3VirtualBucketTest {
131129
@Test
132130
fun uploadObject() {
133131
val s3Client = mockClientManager.create<S3Client>()
134-
val uploadCaptor = argumentCaptor<PutObjectRequest>()
135132

136133
s3Client.stub {
137134
on {
138-
putObject(uploadCaptor.capture(), any<RequestBody>())
135+
putObject(any<PutObjectRequest>(), any<RequestBody>())
139136
} doReturn PutObjectResponse.builder()
140137
.versionId("VersionFoo")
141138
.build()
142139
}
143140

144-
val testFile = mock<VirtualFile> { on { name } doReturn "TestFile" }
145-
testFile.stub { on { length } doReturn 341 }
146-
testFile.stub { on { inputStream } doReturn ByteArrayInputStream("Hello".toByteArray()) }
141+
val testFile = tempFolder.newFile("someFile.html").toPath()
147142

148143
val sut = S3VirtualBucket(Bucket.builder().name("TestBucket").build(), "", s3Client, projectRule.project)
149144
runBlocking {
150-
sut.upload(projectRule.project, testFile.inputStream, testFile.length, "TestFile")
145+
sut.upload(projectRule.project, testFile, "TestFile")
151146
}
152147

153-
verify(s3Client).putObject(any<PutObjectRequest>(), any<RequestBody>())
148+
argumentCaptor<PutObjectRequest, RequestBody>().let { (request, body) ->
149+
verify(s3Client).putObject(request.capture(), body.capture())
154150

155-
val uploadRequestCapture = uploadCaptor.firstValue
156-
assertThat(uploadRequestCapture.bucket()).isEqualTo("TestBucket")
157-
assertThat(uploadRequestCapture.key()).isEqualTo("TestFile")
151+
assertThat(request.firstValue.bucket()).isEqualTo("TestBucket")
152+
assertThat(request.firstValue.key()).isEqualTo("TestFile")
153+
154+
assertThat(body.firstValue.contentType()).isEqualTo("text/html")
155+
}
158156
}
159157

160158
@Test

‎jetbrains-core/tst/software/aws/toolkits/jetbrains/services/s3/objectActions/UploadObjectActionTest.kt‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ class UploadObjectActionTest : ObjectActionTestBase() {
122122

123123
retryableAssert {
124124
verifyBlocking(s3Bucket, never()) {
125-
upload(any(), any(), any(), any())
125+
upload(any(), any(), any())
126126
}
127127
verify(treeTable, never()).invalidateLevel(any())
128128
verify(treeTable, never()).refresh()
@@ -141,7 +141,7 @@ class UploadObjectActionTest : ObjectActionTestBase() {
141141

142142
retryableAssert {
143143
argumentCaptor<String>().apply {
144-
verifyBlocking(s3Bucket) { upload(any(), any(), any(), capture()) }
144+
verifyBlocking(s3Bucket) { upload(any(), any(), capture()) }
145145

146146
assertThat(allValues).hasSize(1)
147147
assertThat(firstValue).isEqualTo("path1/${newFile.name}")
@@ -165,7 +165,7 @@ class UploadObjectActionTest : ObjectActionTestBase() {
165165

166166
retryableAssert {
167167
argumentCaptor<String>().apply {
168-
verifyBlocking(s3Bucket, times(2)) { upload(any(), any(), any(), capture()) }
168+
verifyBlocking(s3Bucket, times(2)) { upload(any(), any(), capture()) }
169169

170170
assertThat(firstValue).isEqualTo("path1/${newFile.name}")
171171
assertThat(secondValue).isEqualTo("path1/${newFile2.name}")
@@ -190,7 +190,7 @@ class UploadObjectActionTest : ObjectActionTestBase() {
190190

191191
retryableAssert {
192192
argumentCaptor<String>().apply {
193-
verifyBlocking(s3Bucket, times(2)) { upload(any(), any(), any(), capture()) }
193+
verifyBlocking(s3Bucket, times(2)) { upload(any(), any(), capture()) }
194194

195195
assertThat(firstValue).isEqualTo("path1/${newFile.name}")
196196
assertThat(secondValue).isEqualTo("path1/${newFile2.name}")
@@ -209,7 +209,7 @@ class UploadObjectActionTest : ObjectActionTestBase() {
209209

210210
s3Bucket.stub {
211211
onBlocking {
212-
upload(any(), any(), any(), any())
212+
upload(any(), any(), any())
213213
}.thenReturn(null).thenThrow(S3Exception.builder().message("Test exception").build())
214214
}
215215

@@ -220,7 +220,7 @@ class UploadObjectActionTest : ObjectActionTestBase() {
220220

221221
retryableAssert {
222222
argumentCaptor<String>().apply {
223-
verifyBlocking(s3Bucket, times(2)) { upload(any(), any(), any(), capture()) }
223+
verifyBlocking(s3Bucket, times(2)) { upload(any(), any(), capture()) }
224224

225225
assertThat(firstValue).isEqualTo("path1/${newFile.name}")
226226
}
@@ -236,7 +236,7 @@ class UploadObjectActionTest : ObjectActionTestBase() {
236236

237237
s3Bucket.stub {
238238
onBlocking {
239-
upload(any(), any(), any(), any())
239+
upload(any(), any(), any())
240240
}.thenReturn(null).thenThrow(S3Exception.builder().message("Test exception").build())
241241
}
242242

@@ -246,7 +246,7 @@ class UploadObjectActionTest : ObjectActionTestBase() {
246246
sut.executeAction(nodes)
247247

248248
retryableAssert {
249-
verifyBlocking(s3Bucket, never()) { upload(any(), any(), any(), any()) }
249+
verifyBlocking(s3Bucket, never()) { upload(any(), any(), any()) }
250250
verify(treeTable, never()).invalidateLevel(dir)
251251
verify(treeTable, never()).refresh()
252252
}
@@ -261,7 +261,7 @@ class UploadObjectActionTest : ObjectActionTestBase() {
261261

262262
retryableAssert {
263263
argumentCaptor<String>().apply {
264-
verifyBlocking(s3Bucket) { upload(any(), any(), any(), capture()) }
264+
verifyBlocking(s3Bucket) { upload(any(), any(), capture()) }
265265

266266
assertThat(allValues).hasSize(1)
267267
assertThat(firstValue).isEqualTo(newFile.name)

0 commit comments

Comments
 (0)