adnanhemani commented on code in PR #1586:
URL: https://github.com/apache/polaris/pull/1586#discussion_r2094201688
##########
polaris-core/src/test/java/org/apache/polaris/service/storage/StorageLocationTest.java:
##########
@@ -19,17 +19,41 @@
package org.apache.polaris.service.storage;
import org.apache.polaris.core.storage.StorageLocation;
-import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
public class StorageLocationTest {
@Test
- public void testOfDifferentPrefixes() {
- StorageLocation StandardLocation =
StorageLocation.of("file:///path/to/file");
+ public void testAwsLocations() {
+ StorageLocation baseStorageLocation =
StorageLocation.of("s3://path/to/file");
+ StorageLocation testStorageLocation =
StorageLocation.of("s3://path/to/////file");
+ Assertions.assertEquals(testStorageLocation, baseStorageLocation);
+ testStorageLocation = StorageLocation.of("s3://////path/////to///file");
+ Assertions.assertEquals(testStorageLocation, baseStorageLocation);
+
+ testStorageLocation =
StorageLocation.of("s3://////path'/////*to===///\"file");
+ Assertions.assertEquals("s3://path'/*to===/\"file",
testStorageLocation.toString());
+ }
+
+ @Test
+ public void testGcpLocations() {
+ StorageLocation baseStorageLocation =
StorageLocation.of("gcs://path/to/file");
+ StorageLocation testStorageLocation =
StorageLocation.of("gcs://path/to/////file");
+ Assertions.assertEquals(testStorageLocation, baseStorageLocation);
+ testStorageLocation = StorageLocation.of("gcs://////path/////to///file");
+ Assertions.assertEquals(testStorageLocation, baseStorageLocation);
+
+ testStorageLocation =
StorageLocation.of("gcs://////path'/////*to===///\"file");
+ Assertions.assertEquals("gcs://path'/*to===/\"file",
testStorageLocation.toString());
Review Comment:
It's not correct. Actually when looking through Java URI code - we are not
currently applying _any_ normalization to URLs...
And I'm not sure that normalizing URLs is entirely necessary either. We've
worked through the comments above that double slashes shouldn't be modified and
similarly, "." and ".." also shouldn't be normalized for blob storage.
As a result, I think this might be trying to do too much. I'm going to stop
work on this and push a new branch that simply just takes the URI create off
for the Blob Storage locations and adds additional tests. Will link that PR
here when I push it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
