Skip to content

Commit a5f823a

Browse files
chriscollins3456chriscollins3456
authored
fix(files) Remove bucket name prefix to presigned urls for files (#15082)
1 parent 4d0f2b7 commit a5f823a

File tree

4 files changed

+22
-24
lines changed

4 files changed

+22
-24
lines changed

‎datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/resolvers/files/GetPresignedUploadUrlResolver.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,7 @@ private String getS3Key(
115115
UploadDownloadScenario scenario = input.getScenario();
116116

117117
if (scenario == UploadDownloadScenario.ASSET_DOCUMENTATION) {
118-
return String.format(
119-
"%s/%s/%s",
120-
s3Configuration.getBucketName(), s3Configuration.getAssetPathPrefix(), fileId);
118+
return String.format("%s/%s", s3Configuration.getAssetPathPrefix(), fileId);
121119
} else {
122120
throw new IllegalArgumentException("Unsupported upload scenario: " + scenario);
123121
}

‎datahub-graphql-core/src/test/java/com/linkedin/datahub/graphql/resolvers/files/GetPresignedUploadUrlResolverTest.java‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -138,10 +138,10 @@ public void testGetPresignedUploadUrlReturnsFileId() throws Exception {
138138
assertNotNull(result.getFileId());
139139

140140
String capturedS3Key = s3KeyCaptor.getValue();
141-
assertTrue(capturedS3Key.startsWith(TEST_BUCKET_NAME + "/" + TEST_ASSET_PATH_PREFIX + "/"));
141+
assertTrue(capturedS3Key.startsWith(TEST_ASSET_PATH_PREFIX + "/"));
142142

143143
// Extract fileId from s3Key
144-
String expectedFileIdPrefix = TEST_BUCKET_NAME + "/" + TEST_ASSET_PATH_PREFIX + "/";
144+
String expectedFileIdPrefix = TEST_ASSET_PATH_PREFIX + "/";
145145
String extractedFileId = capturedS3Key.substring(expectedFileIdPrefix.length());
146146

147147
assertEquals(result.getFileId(), extractedFileId);
@@ -181,7 +181,7 @@ public void testGetPresignedUploadUrlGeneratesCorrectS3Key() throws Exception {
181181
future.get(); // Execute the resolver to capture the argument
182182

183183
String capturedS3Key = s3KeyCaptor.getValue();
184-
assertTrue(capturedS3Key.startsWith(TEST_BUCKET_NAME + "/" + TEST_ASSET_PATH_PREFIX + "/"));
184+
assertTrue(capturedS3Key.startsWith(TEST_ASSET_PATH_PREFIX + "/"));
185185
assertTrue(capturedS3Key.contains(testFileName));
186186
}
187187

@@ -596,10 +596,10 @@ public void testGetPresignedUploadUrlWithSchemaFieldUrn() throws Exception {
596596
assertNotNull(result.getFileId());
597597

598598
String capturedS3Key = s3KeyCaptor.getValue();
599-
assertTrue(capturedS3Key.startsWith(TEST_BUCKET_NAME + "/" + TEST_ASSET_PATH_PREFIX + "/"));
599+
assertTrue(capturedS3Key.startsWith(TEST_ASSET_PATH_PREFIX + "/"));
600600

601601
// Extract fileId from s3Key
602-
String expectedFileIdPrefix = TEST_BUCKET_NAME + "/" + TEST_ASSET_PATH_PREFIX + "/";
602+
String expectedFileIdPrefix = TEST_ASSET_PATH_PREFIX + "/";
603603
String extractedFileId = capturedS3Key.substring(expectedFileIdPrefix.length());
604604

605605
assertEquals(result.getFileId(), extractedFileId);
@@ -686,10 +686,10 @@ public void testGetPresignedUploadUrlWithAssetUrnAndNullSchemaFieldUrn() throws
686686
assertNotNull(result.getFileId());
687687

688688
String capturedS3Key = s3KeyCaptor.getValue();
689-
assertTrue(capturedS3Key.startsWith(TEST_BUCKET_NAME + "/" + TEST_ASSET_PATH_PREFIX + "/"));
689+
assertTrue(capturedS3Key.startsWith(TEST_ASSET_PATH_PREFIX + "/"));
690690

691691
// Extract fileId from s3Key
692-
String expectedFileIdPrefix = TEST_BUCKET_NAME + "/" + TEST_ASSET_PATH_PREFIX + "/";
692+
String expectedFileIdPrefix = TEST_ASSET_PATH_PREFIX + "/";
693693
String extractedFileId = capturedS3Key.substring(expectedFileIdPrefix.length());
694694

695695
assertEquals(result.getFileId(), extractedFileId);
@@ -759,10 +759,10 @@ public void testGetPresignedUploadUrlWhenBothAssetUrnAndSchemaFieldUrnAreProvide
759759
assertNotNull(result.getFileId());
760760

761761
String capturedS3Key = s3KeyCaptor.getValue();
762-
assertTrue(capturedS3Key.startsWith(TEST_BUCKET_NAME + "/" + TEST_ASSET_PATH_PREFIX + "/"));
762+
assertTrue(capturedS3Key.startsWith(TEST_ASSET_PATH_PREFIX + "/"));
763763

764764
// Extract fileId from s3Key
765-
String expectedFileIdPrefix = TEST_BUCKET_NAME + "/" + TEST_ASSET_PATH_PREFIX + "/";
765+
String expectedFileIdPrefix = TEST_ASSET_PATH_PREFIX + "/";
766766
String extractedFileId = capturedS3Key.substring(expectedFileIdPrefix.length());
767767

768768
assertEquals(result.getFileId(), extractedFileId);

‎metadata-service/openapi-servlet/src/main/java/io/datahubproject/openapi/v1/files/FilesController.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ public ResponseEntity<Void> getFile(
106106
}
107107

108108
// Prefix file ID with bucket name
109-
String key = String.format("%s/%s/%s", bucket, folder, fileId);
109+
String key = String.format("%s/%s", folder, fileId);
110110

111111
// Generate presigned URL using the existing S3Util
112112
String presignedUrl = s3Util.generatePresignedDownloadUrl(bucket, key, expiration);

‎metadata-service/openapi-servlet/src/test/java/io/datahubproject/openapi/v1/files/FilesControllerTest.java‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ public void initTest() {
173173
@Test
174174
public void testGetFileWithDefaultExpiration() throws Exception {
175175
// Mock S3Util to return presigned URL
176-
String expectedKey = String.format("%s/%s/%s", TEST_BUCKET, TEST_FOLDER, TEST_FILE_ID);
176+
String expectedKey = String.format("%s/%s", TEST_FOLDER, TEST_FILE_ID);
177177
when(mockS3Util.generatePresignedDownloadUrl(
178178
eq(TEST_BUCKET), eq(expectedKey), eq(DEFAULT_EXPIRATION)))
179179
.thenReturn(TEST_PRESIGNED_URL);
@@ -190,7 +190,7 @@ public void testGetFileWithDefaultExpiration() throws Exception {
190190
@Test
191191
public void testGetFileWithCustomExpiration() throws Exception {
192192
int customExpiration = 7200;
193-
String expectedKey = String.format("%s/%s/%s", TEST_BUCKET, TEST_FOLDER, TEST_FILE_ID);
193+
String expectedKey = String.format("%s/%s", TEST_FOLDER, TEST_FILE_ID);
194194

195195
when(mockS3Util.generatePresignedDownloadUrl(
196196
eq(TEST_BUCKET), eq(expectedKey), eq(customExpiration)))
@@ -208,7 +208,7 @@ public void testGetFileWithCustomExpiration() throws Exception {
208208
@Test
209209
public void testGetFileWithMinimumValidExpiration() throws Exception {
210210
int minExpiration = 1;
211-
String expectedKey = String.format("%s/%s/%s", TEST_BUCKET, TEST_FOLDER, TEST_FILE_ID);
211+
String expectedKey = String.format("%s/%s", TEST_FOLDER, TEST_FILE_ID);
212212

213213
when(mockS3Util.generatePresignedDownloadUrl(
214214
eq(TEST_BUCKET), eq(expectedKey), eq(minExpiration)))
@@ -225,7 +225,7 @@ public void testGetFileWithMinimumValidExpiration() throws Exception {
225225

226226
@Test
227227
public void testGetFileWithMaximumValidExpiration() throws Exception {
228-
String expectedKey = String.format("%s/%s/%s", TEST_BUCKET, TEST_FOLDER, TEST_FILE_ID);
228+
String expectedKey = String.format("%s/%s", TEST_FOLDER, TEST_FILE_ID);
229229

230230
when(mockS3Util.generatePresignedDownloadUrl(
231231
eq(TEST_BUCKET), eq(expectedKey), eq(MAX_EXPIRATION)))
@@ -291,7 +291,7 @@ public void testGetFileWithNullBucket() throws Exception {
291291

292292
@Test
293293
public void testGetFileWithS3UtilException() throws Exception {
294-
String expectedKey = String.format("%s/%s/%s", TEST_BUCKET, TEST_FOLDER, TEST_FILE_ID);
294+
String expectedKey = String.format("%s/%s", TEST_FOLDER, TEST_FILE_ID);
295295

296296
// Mock S3Util to throw an exception
297297
when(mockS3Util.generatePresignedDownloadUrl(
@@ -308,7 +308,7 @@ public void testGetFileWithS3UtilException() throws Exception {
308308
@Test
309309
public void testGetFileWithDifferentFolder() throws Exception {
310310
String differentFolder = "images";
311-
String expectedKey = String.format("%s/%s/%s", TEST_BUCKET, differentFolder, TEST_FILE_ID);
311+
String expectedKey = String.format("%s/%s", differentFolder, TEST_FILE_ID);
312312

313313
when(mockS3Util.generatePresignedDownloadUrl(
314314
eq(TEST_BUCKET), eq(expectedKey), eq(DEFAULT_EXPIRATION)))
@@ -325,7 +325,7 @@ public void testGetFileWithDifferentFolder() throws Exception {
325325
@Test
326326
public void testGetFileWithSpecialCharactersInFileId() throws Exception {
327327
String specialFileId = "file-with_special.chars-123";
328-
String expectedKey = String.format("%s/%s/%s", TEST_BUCKET, TEST_FOLDER, specialFileId);
328+
String expectedKey = String.format("%s/%s", TEST_FOLDER, specialFileId);
329329

330330
when(mockS3Util.generatePresignedDownloadUrl(
331331
eq(TEST_BUCKET), eq(expectedKey), eq(DEFAULT_EXPIRATION)))
@@ -345,7 +345,7 @@ public void testGetFileWithSpecialCharactersInFileId() throws Exception {
345345
public void testGetFileWithValidAssetDocumentationPermissions() throws Exception {
346346
// Setup file ID with separator to test UUID extraction
347347
String fileIdWithSeparator = "abc123__filename.pdf";
348-
String expectedKey = String.format("%s/%s/%s", TEST_BUCKET, TEST_FOLDER, fileIdWithSeparator);
348+
String expectedKey = String.format("%s/%s", TEST_FOLDER, fileIdWithSeparator);
349349

350350
when(mockS3Util.generatePresignedDownloadUrl(
351351
eq(TEST_BUCKET), eq(expectedKey), eq(DEFAULT_EXPIRATION)))
@@ -437,7 +437,7 @@ public void testGetFileWithEntityServiceException() throws Exception {
437437
public void testGetFileWithUUIDExtraction() throws Exception {
438438
// Test that UUID is correctly extracted from file ID with separator
439439
String fileIdWithSeparator = "abc123__some-filename-with-special-chars.pdf";
440-
String expectedKey = String.format("%s/%s/%s", TEST_BUCKET, TEST_FOLDER, fileIdWithSeparator);
440+
String expectedKey = String.format("%s/%s", TEST_FOLDER, fileIdWithSeparator);
441441

442442
// The UUID "abc123" should be extracted
443443
Urn expectedFileUrn = UrnUtils.getUrn("urn:li:dataHubFile:abc123");
@@ -462,7 +462,7 @@ public void testGetFileWithDifferentAssetUrn() throws Exception {
462462
setupDefaultFileEntity(
463463
TEST_FILE_URN, differentAssetUrn, FileUploadScenario.ASSET_DOCUMENTATION);
464464

465-
String expectedKey = String.format("%s/%s/%s", TEST_BUCKET, TEST_FOLDER, TEST_FILE_ID);
465+
String expectedKey = String.format("%s/%s", TEST_FOLDER, TEST_FILE_ID);
466466

467467
when(mockS3Util.generatePresignedDownloadUrl(
468468
eq(TEST_BUCKET), eq(expectedKey), eq(DEFAULT_EXPIRATION)))
@@ -480,7 +480,7 @@ public void testGetFileWithDifferentAssetUrn() throws Exception {
480480
public void testBuildOperationContextWithCorrectParameters() throws Exception {
481481
// This test verifies that the endpoint successfully builds an operation context and processes
482482
// the request
483-
String expectedKey = String.format("%s/%s/%s", TEST_BUCKET, TEST_FOLDER, TEST_FILE_ID);
483+
String expectedKey = String.format("%s/%s", TEST_FOLDER, TEST_FILE_ID);
484484

485485
when(mockS3Util.generatePresignedDownloadUrl(
486486
eq(TEST_BUCKET), eq(expectedKey), eq(DEFAULT_EXPIRATION)))

0 commit comments

Comments
 (0)