diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java index 6db06cb01a..ccb6b44f57 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java @@ -235,6 +235,31 @@ private static Namespace decodeNamespace(String namespace) { return RESTUtil.decodeNamespace(URLEncoder.encode(namespace, Charset.defaultCharset())); } + /** + * For situations where we typically expect a metadataLocation to be present in the response and + * so expect to insert an etag header, this helper gracefully falls back to omitting the header if + * unable to get metadata location and logs a warning. + */ + private Response.ResponseBuilder tryInsertETagHeader( + Response.ResponseBuilder builder, + LoadTableResponse response, + String namespace, + String tableName) { + if (response.metadataLocation() != null) { + builder = + builder.header( + HttpHeaders.ETAG, + IcebergHttpUtil.generateETagForMetadataFileLocation(response.metadataLocation())); + } else { + LOGGER + .atWarn() + .addKeyValue("namespace", namespace) + .addKeyValue("tableName", tableName) + .log("Response has null metadataLocation; omitting etag"); + } + return builder; + } + @Override public Response namespaceExists( String prefix, String namespace, RealmContext realmContext, SecurityContext securityContext) { @@ -312,20 +337,14 @@ public Response createTable( } } else if (delegationModes.isEmpty()) { LoadTableResponse response = catalog.createTableDirect(ns, createTableRequest); - return Response.ok(response) - .header( - HttpHeaders.ETAG, - IcebergHttpUtil.generateETagForMetadataFileLocation( - response.metadataLocation())) + return tryInsertETagHeader( + Response.ok(response), response, namespace, createTableRequest.name()) .build(); } else { LoadTableResponse response = catalog.createTableDirectWithWriteDelegation(ns, createTableRequest); - return Response.ok(response) - .header( - HttpHeaders.ETAG, - IcebergHttpUtil.generateETagForMetadataFileLocation( - response.metadataLocation())) + return tryInsertETagHeader( + Response.ok(response), response, namespace, createTableRequest.name()) .build(); } }); @@ -384,11 +403,7 @@ public Response loadTable( .orElseThrow(() -> new WebApplicationException(Response.Status.NOT_MODIFIED)); } - return Response.ok(response) - .header( - HttpHeaders.ETAG, - IcebergHttpUtil.generateETagForMetadataFileLocation(response.metadataLocation())) - .build(); + return tryInsertETagHeader(Response.ok(response), response, namespace, table).build(); }); } @@ -446,11 +461,8 @@ public Response registerTable( prefix, catalog -> { LoadTableResponse response = catalog.registerTable(ns, registerTableRequest); - - return Response.ok(response) - .header( - HttpHeaders.ETAG, - IcebergHttpUtil.generateETagForMetadataFileLocation(response.metadataLocation())) + return tryInsertETagHeader( + Response.ok(response), response, namespace, registerTableRequest.name()) .build(); }); } diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java index 78fecf4797..b5e1a0edb9 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java @@ -503,8 +503,7 @@ public boolean sendNotification(TableIdentifier identifier, NotificationRequest * @return the Polaris table entity for the table */ private IcebergTableLikeEntity getTableEntity(TableIdentifier tableIdentifier) { - PolarisResolvedPathWrapper target = - resolutionManifest.getPassthroughResolvedPath(tableIdentifier); + PolarisResolvedPathWrapper target = resolutionManifest.getResolvedPath(tableIdentifier); return IcebergTableLikeEntity.of(target.getRawLeafEntity()); } @@ -529,12 +528,24 @@ public Optional loadTableIfStale( authorizeBasicTableLikeOperationOrThrow( op, PolarisEntitySubType.ICEBERG_TABLE, tableIdentifier); - IcebergTableLikeEntity tableEntity = getTableEntity(tableIdentifier); - String tableEntityTag = - IcebergHttpUtil.generateETagForMetadataFileLocation(tableEntity.getMetadataLocation()); - - if (ifNoneMatch != null && ifNoneMatch.anyMatch(tableEntityTag)) { - return Optional.empty(); + if (ifNoneMatch != null) { + // Perform freshness-aware table loading if caller specified ifNoneMatch. + IcebergTableLikeEntity tableEntity = getTableEntity(tableIdentifier); + if (tableEntity == null || tableEntity.getMetadataLocation() == null) { + LOGGER + .atWarn() + .addKeyValue("tableIdentifier", tableIdentifier) + .addKeyValue("tableEntity", tableEntity) + .log("Failed to getMetadataLocation to generate ETag when loading table"); + } else { + // TODO: Refactor null-checking into the helper method once we create a more canonical + // interface for associate etags with entities. + String tableEntityTag = + IcebergHttpUtil.generateETagForMetadataFileLocation(tableEntity.getMetadataLocation()); + if (ifNoneMatch.anyMatch(tableEntityTag)) { + return Optional.empty(); + } + } } return Optional.of(CatalogHandlers.loadTable(baseCatalog, tableIdentifier)); @@ -601,12 +612,24 @@ public Optional loadTableWithAccessDelegationIfStale( FeatureConfiguration.ALLOW_EXTERNAL_CATALOG_CREDENTIAL_VENDING.catalogConfig()); } - IcebergTableLikeEntity tableEntity = getTableEntity(tableIdentifier); - String tableETag = - IcebergHttpUtil.generateETagForMetadataFileLocation(tableEntity.getMetadataLocation()); - - if (ifNoneMatch != null && ifNoneMatch.anyMatch(tableETag)) { - return Optional.empty(); + if (ifNoneMatch != null) { + // Perform freshness-aware table loading if caller specified ifNoneMatch. + IcebergTableLikeEntity tableEntity = getTableEntity(tableIdentifier); + if (tableEntity == null || tableEntity.getMetadataLocation() == null) { + LOGGER + .atWarn() + .addKeyValue("tableIdentifier", tableIdentifier) + .addKeyValue("tableEntity", tableEntity) + .log("Failed to getMetadataLocation to generate ETag when loading table"); + } else { + // TODO: Refactor null-checking into the helper method once we create a more canonical + // interface for associate etags with entities. + String tableETag = + IcebergHttpUtil.generateETagForMetadataFileLocation(tableEntity.getMetadataLocation()); + if (ifNoneMatch.anyMatch(tableETag)) { + return Optional.empty(); + } + } } // TODO: Find a way for the configuration or caller to better express whether to fail or omit diff --git a/service/common/src/main/java/org/apache/polaris/service/http/IcebergHttpUtil.java b/service/common/src/main/java/org/apache/polaris/service/http/IcebergHttpUtil.java index 88c056aa70..2fd0445d93 100644 --- a/service/common/src/main/java/org/apache/polaris/service/http/IcebergHttpUtil.java +++ b/service/common/src/main/java/org/apache/polaris/service/http/IcebergHttpUtil.java @@ -32,6 +32,11 @@ private IcebergHttpUtil() {} * @return the generated ETag */ public static String generateETagForMetadataFileLocation(String metadataFileLocation) { + if (metadataFileLocation == null) { + // Throw a more appropriate exception than letting DigestUtils die randomly. + throw new IllegalArgumentException("Unable to generate etag for null metadataFileLocation"); + } + // Use hash of metadata location since we don't want clients to use the ETag to try to extract // the metadata file location String hashedMetadataFileLocation = DigestUtils.sha256Hex(metadataFileLocation);