Skip to content

Commit b1a69ec

Browse files
sandeepvinayaksandeepvinayak
authored
blobstore: handle the gcp blobstore endpoint with empty path / (#263)
1 parent f0dec6b commit b1a69ec

File tree

5 files changed

+81
-32
lines changed

5 files changed

+81
-32
lines changed

‎blob/blob-client/src/main/java/com/salesforce/multicloudj/blob/driver/BlobStoreValidator.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -268,13 +268,13 @@ public void validateEndpoint(URI endpoint, boolean requirePort) {
268268
if (!isProtocolValid) {
269269
throw new IllegalArgumentException(String.format(INVALID_ENDPOINT_PROTOCOL_MSG, protocol));
270270
}
271-
if(endpoint.getHost() == null || endpoint.getHost().isBlank()) {
271+
if(StringUtils.isBlank(endpoint.getHost())) {
272272
throw new IllegalArgumentException(String.format(INVALID_ENDPOINT_HOSTNAME_MSG, endpoint.getHost()));
273273
}
274274
if(requirePort && endpoint.getPort() < 0) {
275275
throw new IllegalArgumentException(String.format(INVALID_ENDPOINT_PORT_MSG, endpoint.getPort()));
276276
}
277-
if(endpoint.getPath() != null && !endpoint.getPath().isBlank()) {
277+
if(!StringUtils.isBlank(endpoint.getPath()) && !endpoint.getPath().equals("/")) {
278278
throw new IllegalArgumentException(String.format(INVALID_ENDPOINT_NO_PATH_ALLOWED_MSG, endpoint.getPath()));
279279
}
280280
if(endpoint.getQuery() != null) {

‎blob/blob-client/src/test/java/com/salesforce/multicloudj/blob/driver/BlobStoreValidatorTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ void testValidateEndpoint() {
259259
boolean portValid = !requirePort || port >= 0;
260260

261261
for (String path : Arrays.asList(null, " ", "/", "/path")) {
262-
boolean pathValid = path==null;
262+
boolean pathValid = path==null || path.equals("/");
263263

264264
for (String query : Arrays.asList(null, "?", "?key=value")) {
265265
boolean queryValid = query==null;

‎blob/blob-gcp/src/main/java/com/salesforce/multicloudj/blob/gcp/GcpBlobStore.java‎

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@
8585
import java.io.InputStream;
8686
import java.io.OutputStream;
8787
import java.lang.reflect.Method;
88+
import java.net.URI;
8889
import java.net.URL;
8990
import java.nio.ByteBuffer;
9091
import java.nio.channels.Channels;
@@ -940,21 +941,42 @@ public Builder copyFrom(BlobStoreBuilder<?> source) {
940941
}
941942

942943
/**
943-
* Helper function for generating the Storage client
944+
* Normalizes endpoint to ensure it ends with "/"
944945
*/
945-
private static Storage buildStorage(Builder builder) {
946-
CloseableHttpClient httpClient = buildHttpClient(builder);
946+
private static String normalizeEndpoint(URI endpoint) {
947+
if (endpoint == null) {
948+
return null;
949+
}
950+
String endpointStr = endpoint.toString();
951+
if (!endpointStr.endsWith("/")) {
952+
endpointStr = endpointStr + "/";
953+
}
954+
return endpointStr;
955+
}
947956

957+
/**
958+
* Creates HttpTransportOptions with ApacheHttpTransport
959+
*/
960+
private static HttpTransportOptions buildTransportOptions(Builder builder) {
961+
CloseableHttpClient httpClient = buildHttpClient(builder);
948962
ApacheHttpTransport transport = new ApacheHttpTransport(httpClient);
949-
HttpTransportOptions transportOptions = HttpTransportOptions.newBuilder()
963+
return HttpTransportOptions.newBuilder()
950964
.setHttpTransportFactory(() -> transport)
951965
.build();
966+
}
967+
968+
/**
969+
* Helper function for generating the Storage client
970+
*/
971+
private static Storage buildStorage(Builder builder) {
972+
HttpTransportOptions transportOptions = buildTransportOptions(builder);
952973

953974
StorageOptions.Builder storageOptionsBuilder = StorageOptions.newBuilder();
954975
storageOptionsBuilder.setTransportOptions(transportOptions);
955976

956-
if (builder.getEndpoint() != null) {
957-
storageOptionsBuilder.setHost(builder.getEndpoint().toString());
977+
String endpoint = normalizeEndpoint(builder.getEndpoint());
978+
if (endpoint != null) {
979+
storageOptionsBuilder.setHost(endpoint);
958980
}
959981

960982
if (builder.getCredentialsOverrider() != null) {
@@ -969,17 +991,13 @@ private static Storage buildStorage(Builder builder) {
969991
* Helper function for generating the MultipartUpload client
970992
*/
971993
private static MultipartUploadClient buildMultipartUploadClient(Builder builder) {
972-
CloseableHttpClient httpClient = buildHttpClient(builder);
973-
974-
ApacheHttpTransport transport = new ApacheHttpTransport(httpClient);
975-
HttpTransportOptions transportOptions = HttpTransportOptions.newBuilder()
976-
.setHttpTransportFactory(() -> transport)
977-
.build();
994+
HttpTransportOptions transportOptions = buildTransportOptions(builder);
978995

979996
HttpStorageOptions.Builder storageOptionsBuilder = HttpStorageOptions.http().setTransportOptions(transportOptions);
980997

981-
if (builder.getEndpoint() != null) {
982-
storageOptionsBuilder.setHost(builder.getEndpoint().toString());
998+
String endpoint = normalizeEndpoint(builder.getEndpoint());
999+
if (endpoint != null) {
1000+
storageOptionsBuilder.setHost(endpoint);
9831001
}
9841002

9851003
if (builder.getCredentialsOverrider() != null) {

‎blob/blob-gcp/src/test/java/com/salesforce/multicloudj/blob/gcp/GcpBlobStoreTest.java‎

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,17 +47,16 @@
4747
import com.salesforce.multicloudj.blob.driver.MultipartUpload;
4848
import com.salesforce.multicloudj.blob.driver.MultipartUploadRequest;
4949
import com.salesforce.multicloudj.blob.driver.MultipartUploadResponse;
50+
import com.salesforce.multicloudj.blob.driver.ObjectLockInfo;
5051
import com.salesforce.multicloudj.blob.driver.PresignedOperation;
5152
import com.salesforce.multicloudj.blob.driver.PresignedUrlRequest;
53+
import com.salesforce.multicloudj.blob.driver.RetentionMode;
5254
import com.salesforce.multicloudj.blob.driver.UploadRequest;
5355
import com.salesforce.multicloudj.blob.driver.UploadResponse;
54-
import com.salesforce.multicloudj.blob.driver.ObjectLockInfo;
55-
import com.salesforce.multicloudj.blob.driver.RetentionMode;
56-
import com.salesforce.multicloudj.common.exceptions.FailedPreconditionException;
57-
import com.salesforce.multicloudj.common.exceptions.ResourceNotFoundException;
58-
import com.salesforce.multicloudj.common.exceptions.UnSupportedOperationException;
5956
import com.salesforce.multicloudj.blob.gcp.async.GcpAsyncBlobStore;
6057
import com.salesforce.multicloudj.blob.gcp.async.GcpAsyncBlobStoreProvider;
58+
import com.salesforce.multicloudj.common.exceptions.FailedPreconditionException;
59+
import com.salesforce.multicloudj.common.exceptions.ResourceNotFoundException;
6160
import com.salesforce.multicloudj.common.exceptions.SubstrateSdkException;
6261
import com.salesforce.multicloudj.common.exceptions.UnknownException;
6362
import com.salesforce.multicloudj.common.gcp.GcpConstants;
@@ -112,13 +111,9 @@
112111
import static org.mockito.Mockito.doReturn;
113112
import static org.mockito.Mockito.doThrow;
114113
import static org.mockito.Mockito.lenient;
115-
import static org.mockito.ArgumentMatchers.any;
116-
import static org.mockito.ArgumentMatchers.eq;
117-
import static org.mockito.Mockito.lenient;
118114
import static org.mockito.Mockito.mock;
119115
import static org.mockito.Mockito.never;
120116
import static org.mockito.Mockito.verify;
121-
import static org.mockito.Mockito.times;
122117
import static org.mockito.Mockito.when;
123118

124119
@ExtendWith(MockitoExtension.class)
@@ -3025,7 +3020,7 @@ void testUpdateObjectRetention_ObjectNotFound() {
30253020
// Given
30263021
String key = "non-existent-key";
30273022
java.time.Instant newRetainUntil = java.time.Instant.now().plusSeconds(7200);
3028-
3023+
30293024
when(mockTransformer.toBlobId(eq(TEST_BUCKET), eq(key), any())).thenReturn(mockBlobId);
30303025
when(mockStorage.get(mockBlobId)).thenReturn(null);
30313026

@@ -3035,4 +3030,38 @@ void testUpdateObjectRetention_ObjectNotFound() {
30353030
});
30363031
}
30373032

3033+
@Test
3034+
void testBuild_EndpointWithoutTrailingSlash() {
3035+
// Given - endpoint without trailing slash
3036+
URI endpointWithoutSlash = URI.create("https://storage.googleapis.com");
3037+
3038+
GcpBlobStore.Builder builder = (GcpBlobStore.Builder) new GcpBlobStore.Builder()
3039+
.withBucket(TEST_BUCKET)
3040+
.withEndpoint(endpointWithoutSlash);
3041+
3042+
// When - build the GcpBlobStore (this will call buildStorage and buildMultipartUploadClient)
3043+
GcpBlobStore store = builder.build();
3044+
3045+
// Then - verify the store was created successfully
3046+
assertNotNull(store);
3047+
assertEquals(TEST_BUCKET, store.getBucket());
3048+
}
3049+
3050+
@Test
3051+
void testBuild_EndpointWithTrailingSlash() {
3052+
// Given - endpoint with trailing slash
3053+
URI endpointWithSlash = URI.create("https://storage.googleapis.com/");
3054+
3055+
GcpBlobStore.Builder builder = (GcpBlobStore.Builder) new GcpBlobStore.Builder()
3056+
.withBucket(TEST_BUCKET)
3057+
.withEndpoint(endpointWithSlash);
3058+
3059+
// When - build the GcpBlobStore (this will call buildStorage and buildMultipartUploadClient)
3060+
GcpBlobStore store = builder.build();
3061+
3062+
// Then - verify the store was created successfully
3063+
assertNotNull(store);
3064+
assertEquals(TEST_BUCKET, store.getBucket());
3065+
}
3066+
30383067
}

‎examples/src/main/java/com/salesforce/multicloudj/blob/Main.java‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -511,7 +511,7 @@ private static BucketClient getBucketClient(String provider) {
511511
if (region != null && !region.trim().isEmpty()) {
512512
builder.withRegion(region);
513513
}
514-
514+
515515
if (endpoint != null && !endpoint.trim().isEmpty()) {
516516
builder.withEndpoint(URI.create(endpoint));
517517
}
@@ -601,21 +601,23 @@ public static void main(String[] args) {
601601
System.out.println("Provider: " + getProvider());
602602

603603
try {
604+
// single operation upload
605+
upload();
604606
// Create test directory first
605607
System.out.println("=== Creating Test Directory ===");
606-
//createTestDirectory();
608+
createTestDirectory();
607609

608610
// Test directory upload only
609611
System.out.println("=== Testing Directory Upload ===");
610-
//uploadDirectory();
612+
uploadDirectory();
611613

612614
// Test directory download
613615
getLogger().info("=== Testing Directory Download ===");
614-
//downloadDirectory();
616+
downloadDirectory();
615617

616618
// Test directory delete
617619
getLogger().info("=== Testing Directory Delete ===");
618-
//deleteDirectory();
620+
deleteDirectory();
619621

620622
System.out.println("Directory operations test completed!");
621623

0 commit comments

Comments
 (0)