Improve code-containment and efficiency of etag-aware loading

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();
         });
   }

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

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);