eric-maynard commented on code in PR #2149: URL: https://github.com/apache/polaris/pull/2149#discussion_r2261183514
########## 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: I went with a revert, because it seems that the logic to do this is actually much more complicated than originally thought. The logic becomes more stateful, more imperative, and just generally longer. It seems that the extra 6 calls to `StorageLocation.of` could be preferable. -- 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: issues-unsubscr...@polaris.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org