snazy commented on code in PR #2149:
URL: https://github.com/apache/polaris/pull/2149#discussion_r2250891043


##########
polaris-core/src/main/java/org/apache/polaris/core/storage/StorageUtil.java:
##########
@@ -62,4 +70,46 @@ public class StorageUtil {
   public static @Nonnull String getBucket(URI uri) {
     return uri.getAuthority();
   }
+
+  /** Given a TableMetadata, extracts the locations where the table's 
[meta]data might be found. */
+  public static @Nonnull Set<String> 
getLocationsAllowedToBeAccessed(TableMetadata tableMetadata) {
+    return getLocationsAllowedToBeAccessed(tableMetadata.location(), 
tableMetadata.properties());
+  }
+
+  /** Given a baseLocation and entity (table?) properties, extracts the 
relevant locations */
+  public static @Nonnull Set<String> getLocationsAllowedToBeAccessed(
+      String baseLocation, Map<String, String> properties) {
+    Set<String> locations = new HashSet<>();
+    locations.add(baseLocation);
+    
locations.add(properties.get(IcebergTableLikeEntity.USER_SPECIFIED_WRITE_DATA_LOCATION_KEY));
+    locations.add(
+        
properties.get(IcebergTableLikeEntity.USER_SPECIFIED_WRITE_METADATA_LOCATION_KEY));
+    locations.remove(null);
+    return removeRedundantLocations(locations);
+  }
+
+  /** Given a ViewMetadata, extracts the locations where the view's [meta]data 
might be found. */
+  public static @Nonnull Set<String> 
getLocationsAllowedToBeAccessed(ViewMetadata viewMetadata) {
+    return Set.of(viewMetadata.location());
+  }
+
+  /** Removes "redundant" locations, so {/a/b/, /a/b/c, /a/b/d} will be 
reduced to just {/a/b/} */
+  private static @Nonnull Set<String> removeRedundantLocations(Set<String> 
locationStrings) {
+    Set<StorageLocation> locations =
+        locationStrings.stream()
+            .filter(Objects::nonNull)
+            .map(StorageLocation::of)
+            .collect(Collectors.toSet());
+
+    for (StorageLocation potentialParent : locations) {
+      for (StorageLocation potentialChild : locations) {
+        if (!potentialParent.equals(potentialChild)) {
+          if (potentialChild.isChildOf(potentialParent)) {
+            locations.remove(potentialChild);

Review Comment:
   Looks like this can run into multiple error conditions: the `Set` being not 
mutable (`There are no guarantees on the type, mutability, ... of the Set 
returned` from `Collectors.toSet()`), a `ConcurrentModificationException` in 
the `Iterator`s of the nested loops, or one of the iterators (depending on the 
`Set` implementation) skipping an element after a `.remove()`



-- 
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]

Reply via email to