From 1cc60949d32fa989cbcd72b5f8baf6ff15970527 Mon Sep 17 00:00:00 2001 From: Sandhya S Date: Fri, 27 Jun 2025 12:43:37 -0700 Subject: [PATCH 1/3] -Additional unit test for Etags -Added a few corner case IT tests for testing etags with schema changes. --- .../PolarisRestCatalogIntegrationTest.java | 128 ++++++++++++ .../polaris/service/http/IfNoneMatchTest.java | 196 ++++++++++++++++++ 2 files changed, 324 insertions(+) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java index a999ab9fe6..0366f28203 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java @@ -67,6 +67,7 @@ import org.apache.iceberg.rest.RESTUtil; import org.apache.iceberg.rest.requests.CreateTableRequest; import org.apache.iceberg.rest.responses.ErrorResponse; +import org.apache.iceberg.rest.responses.LoadTableResponse; import org.apache.iceberg.types.Types; import org.apache.polaris.core.admin.model.AwsStorageConfigInfo; import org.apache.polaris.core.admin.model.Catalog; @@ -1549,4 +1550,131 @@ public void testUpdateTableWithReservedProperty() { .hasMessageContaining("reserved prefix"); genericTableApi.purge(currentCatalogName, namespace); } + + @Test + public void testLoadTableWithNonMatchingIfNoneMatchHeader() { + // Create a table first + Namespace ns1 = Namespace.of("ns1"); + restCatalog.createNamespace(ns1); + restCatalog.buildTable(TableIdentifier.of(ns1, "test_table"), + new Schema(List.of(Types.NestedField.required(1, "col1", Types.StringType.get())))).create(); + + // Load table with a non-matching If-None-Match header + String nonMatchingETag = "W/\"non-matching-etag-value\""; + Invocation invocation = + catalogApi + .request("v1/" + currentCatalogName + "/namespaces/ns1/tables/test_table") + .header(HttpHeaders.IF_NONE_MATCH, nonMatchingETag) + .build("GET"); + + try (Response response = invocation.invoke()) { + assertThat(response.getStatus()).isEqualTo(Response.Status.OK.getStatusCode()); + assertThat(response.getHeaders()).containsKey(HttpHeaders.ETAG); + + LoadTableResponse loadTableResponse = response.readEntity(LoadTableResponse.class); + assertThat(loadTableResponse).isNotNull(); + assertThat(loadTableResponse.metadataLocation()).isNotNull(); + } + } + + @Test + public void testLoadTableWithMultipleIfNoneMatchETags() { + // Create a table first + Namespace ns1 = Namespace.of("ns1"); + restCatalog.createNamespace(ns1); + restCatalog.buildTable(TableIdentifier.of(ns1, "test_table"), + new Schema(List.of(Types.NestedField.required(1, "col1", Types.StringType.get())))).create(); + + // First, load the table to get the ETag + Invocation initialInvocation = + catalogApi + .request("v1/" + currentCatalogName + "/namespaces/ns1/tables/test_table") + .build("GET"); + + String correctETag; + try (Response initialResponse = initialInvocation.invoke()) { + assertThat(initialResponse.getStatus()).isEqualTo(Response.Status.OK.getStatusCode()); + assertThat(initialResponse.getHeaders()).containsKey(HttpHeaders.ETAG); + correctETag = initialResponse.getHeaders().getFirst(HttpHeaders.ETAG).toString(); + } + + // Create multiple ETags, one of which matches + String wrongETag1 = "W/\"wrong-etag-1\""; + String wrongETag2 = "W/\"wrong-etag-2\""; + String multipleETags = wrongETag1 + ", " + correctETag + ", " + wrongETag2; + + // Load the table with multiple ETags + Invocation etaggedInvocation = + catalogApi + .request("v1/" + currentCatalogName + "/namespaces/ns1/tables/test_table") + .header(HttpHeaders.IF_NONE_MATCH, multipleETags) + .build("GET"); + + try (Response etaggedResponse = etaggedInvocation.invoke()) { + assertThat(etaggedResponse.getStatus()).isEqualTo(Response.Status.NOT_MODIFIED.getStatusCode()); + assertThat(etaggedResponse.hasEntity()).isFalse(); + } + } + + @Test + public void testLoadTableWithWildcardIfNoneMatchReturns400() { + // Create a table first + Namespace ns1 = Namespace.of("ns1"); + restCatalog.createNamespace(ns1); + restCatalog.buildTable(TableIdentifier.of(ns1, "test_table"), + new Schema(List.of(Types.NestedField.required(1, "col1", Types.StringType.get())))).create(); + + // Load table with wildcard If-None-Match header (should be rejected) + Invocation invocation = + catalogApi + .request("v1/" + currentCatalogName + "/namespaces/ns1/tables/test_table") + .header(HttpHeaders.IF_NONE_MATCH, "*") + .build("GET"); + + try (Response response = invocation.invoke()) { + assertThat(response.getStatus()).isEqualTo(Response.Status.BAD_REQUEST.getStatusCode()); + } + } + + @Test + public void testLoadTableWithInvalidIfNoneMatchFormat() { + // Create a table first + Namespace ns1 = Namespace.of("ns1"); + restCatalog.createNamespace(ns1); + restCatalog.buildTable(TableIdentifier.of(ns1, "test_table"), + new Schema(List.of(Types.NestedField.required(1, "col1", Types.StringType.get())))).create(); + + // Load table with invalid If-None-Match header format + String invalidETag = "invalid-etag-format"; + Invocation invocation = + catalogApi + .request("v1/" + currentCatalogName + "/namespaces/ns1/tables/test_table") + .header(HttpHeaders.IF_NONE_MATCH, invalidETag) + .build("GET"); + + try (Response response = invocation.invoke()) { + // Should return 400 Bad Request for invalid ETag format + assertThat(response.getStatus()).isEqualTo(Response.Status.BAD_REQUEST.getStatusCode()); + } + } + + @Test + public void testLoadNonExistentTableWithIfNoneMatch() { + // Create namespace but not the table + Namespace ns1 = Namespace.of("ns1"); + restCatalog.createNamespace(ns1); + + // Try to load a non-existent table with If-None-Match header + String etag = "W/\"some-etag\""; + Invocation invocation = + catalogApi + .request("v1/" + currentCatalogName + "/namespaces/ns1/tables/non_existent_table") + .header(HttpHeaders.IF_NONE_MATCH, etag) + .build("GET"); + + try (Response response = invocation.invoke()) { + // Should return 404 Not Found regardless of If-None-Match header + assertThat(response.getStatus()).isEqualTo(Response.Status.NOT_FOUND.getStatusCode()); + } + } } diff --git a/service/common/src/test/java/org/apache/polaris/service/http/IfNoneMatchTest.java b/service/common/src/test/java/org/apache/polaris/service/http/IfNoneMatchTest.java index 8e57d02da0..c865d9a696 100644 --- a/service/common/src/test/java/org/apache/polaris/service/http/IfNoneMatchTest.java +++ b/service/common/src/test/java/org/apache/polaris/service/http/IfNoneMatchTest.java @@ -18,10 +18,17 @@ */ package org.apache.polaris.service.http; +import static org.assertj.core.api.Assertions.assertThat; + import java.util.List; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +/** + * Tests for If-None-Match header processing and ETag interaction scenarios. + * This includes both HTTP header parsing and tests that verify how ETag generation + * works together with If-None-Match processing for metadata location changes. + */ public class IfNoneMatchTest { @Test @@ -129,4 +136,193 @@ public void invalidHeaderWithOnlyWhitespacesBetween() { Assertions.assertThrows( IllegalArgumentException.class, () -> IfNoneMatch.fromHeader("W/\"etag\" \"valid-etag\"")); } + + + @Test + public void testETagGenerationConsistency() { + // Test that ETag generation is consistent for the same metadata location + String metadataLocation = "s3://bucket/path/metadata.json"; + + String etag1 = IcebergHttpUtil.generateETagForMetadataFileLocation(metadataLocation); + String etag2 = IcebergHttpUtil.generateETagForMetadataFileLocation(metadataLocation); + + assertThat(etag1).isEqualTo(etag2); + assertThat(etag1).startsWith("W/\""); + assertThat(etag1).endsWith("\""); + } + + @Test + public void testETagGenerationDifferentLocations() { + // Test that different metadata locations generate different ETags + String location1 = "s3://bucket/path/metadata1.json"; + String location2 = "s3://bucket/path/metadata2.json"; + + String etag1 = IcebergHttpUtil.generateETagForMetadataFileLocation(location1); + String etag2 = IcebergHttpUtil.generateETagForMetadataFileLocation(location2); + + assertThat(etag1).isNotEqualTo(etag2); + assertThat(etag1).startsWith("W/\"").endsWith("\""); + assertThat(etag2).startsWith("W/\"").endsWith("\""); + } + + @Test + public void testETagChangeAfterMetadataLocationChange() { + // Test that ETags change when metadata location changes (simulating schema updates) + String originalMetadataLocation = "s3://bucket/path/metadata/v1.metadata.json"; + String updatedMetadataLocation = "s3://bucket/path/metadata/v2.metadata.json"; + + String originalETag = IcebergHttpUtil.generateETagForMetadataFileLocation(originalMetadataLocation); + String updatedETag = IcebergHttpUtil.generateETagForMetadataFileLocation(updatedMetadataLocation); + + // ETags should be different for different metadata locations + assertThat(originalETag).isNotEqualTo(updatedETag); + + // Both should be valid weak ETags + assertThat(originalETag).startsWith("W/\"").endsWith("\""); + assertThat(updatedETag).startsWith("W/\"").endsWith("\""); + + // Test If-None-Match behavior with changed metadata + IfNoneMatch ifNoneMatch = IfNoneMatch.fromHeader(originalETag); + + // Original ETag should match itself + assertThat(ifNoneMatch.anyMatch(originalETag)).isTrue(); + + // Original ETag should NOT match the updated ETag (indicating table has changed) + assertThat(ifNoneMatch.anyMatch(updatedETag)).isFalse(); + } + + @Test + public void testETagBehaviorForTableSchemaChanges() { + // Simulate a table schema change scenario + String baseLocation = "s3://warehouse/db/table/metadata/"; + + // Original table metadata + String v1MetadataLocation = baseLocation + "v1.metadata.json"; + String v1ETag = IcebergHttpUtil.generateETagForMetadataFileLocation(v1MetadataLocation); + + // After adding a column (new metadata version) + String v2MetadataLocation = baseLocation + "v2.metadata.json"; + String v2ETag = IcebergHttpUtil.generateETagForMetadataFileLocation(v2MetadataLocation); + + // After adding another column (another metadata version) + String v3MetadataLocation = baseLocation + "v3.metadata.json"; + String v3ETag = IcebergHttpUtil.generateETagForMetadataFileLocation(v3MetadataLocation); + + // All ETags should be different + assertThat(v1ETag).isNotEqualTo(v2ETag); + assertThat(v1ETag).isNotEqualTo(v3ETag); + assertThat(v2ETag).isNotEqualTo(v3ETag); + + // Test If-None-Match with original ETag after schema changes + IfNoneMatch originalIfNoneMatch = IfNoneMatch.fromHeader(v1ETag); + + // Should match the original version + assertThat(originalIfNoneMatch.anyMatch(v1ETag)).isTrue(); + + // Should NOT match newer versions (indicating table has changed) + assertThat(originalIfNoneMatch.anyMatch(v2ETag)).isFalse(); + assertThat(originalIfNoneMatch.anyMatch(v3ETag)).isFalse(); + + // Test with multiple ETags including the current one + String multipleETags = "W/\"some-old-etag\", " + v1ETag + ", W/\"another-old-etag\""; + IfNoneMatch multipleIfNoneMatch = IfNoneMatch.fromHeader(multipleETags); + + // Should match v1 (one of the ETags in the list) + assertThat(multipleIfNoneMatch.anyMatch(v1ETag)).isTrue(); + + // Should NOT match v2 or v3 (not in the list) + assertThat(multipleIfNoneMatch.anyMatch(v2ETag)).isFalse(); + assertThat(multipleIfNoneMatch.anyMatch(v3ETag)).isFalse(); + } + + @Test + public void testETagBehaviorForTableDropAndRecreate() { + // Simulate a table drop and recreate scenario + String baseLocation = "s3://warehouse/db/table/metadata/"; + + // Original table metadata location + String originalMetadataLocation = baseLocation + "original-v1.metadata.json"; + String originalETag = IcebergHttpUtil.generateETagForMetadataFileLocation(originalMetadataLocation); + + // After dropping and recreating table (completely new metadata location) + String recreatedMetadataLocation = "s3://warehouse/db/table/metadata/recreated-v1.metadata.json"; + String recreatedETag = IcebergHttpUtil.generateETagForMetadataFileLocation(recreatedMetadataLocation); + + // ETags should be completely different for different metadata locations + assertThat(originalETag).isNotEqualTo(recreatedETag); + + // Both should be valid weak ETags + assertThat(originalETag).startsWith("W/\"").endsWith("\""); + assertThat(recreatedETag).startsWith("W/\"").endsWith("\""); + + // Test If-None-Match with original ETag after table recreation + IfNoneMatch originalIfNoneMatch = IfNoneMatch.fromHeader(originalETag); + + // Should match the original ETag + assertThat(originalIfNoneMatch.anyMatch(originalETag)).isTrue(); + + // Should NOT match the recreated table's ETag (completely different table) + assertThat(originalIfNoneMatch.anyMatch(recreatedETag)).isFalse(); + } + + @Test + public void testETagUniquenessAcrossTableLifecycle() { + // Test ETag uniqueness across the complete table lifecycle + String baseLocation = "s3://warehouse/db/users/metadata/"; + + // Original table creation + String v1MetadataLocation = baseLocation + "v1.metadata.json"; + String v1ETag = IcebergHttpUtil.generateETagForMetadataFileLocation(v1MetadataLocation); + + // Schema evolution + String v2MetadataLocation = baseLocation + "v2.metadata.json"; + String v2ETag = IcebergHttpUtil.generateETagForMetadataFileLocation(v2MetadataLocation); + + // More schema changes + String v3MetadataLocation = baseLocation + "v3.metadata.json"; + String v3ETag = IcebergHttpUtil.generateETagForMetadataFileLocation(v3MetadataLocation); + + // Table dropped and recreated with different schema (new metadata path) + String recreatedV1MetadataLocation = baseLocation + "recreated-v1.metadata.json"; + String recreatedV1ETag = IcebergHttpUtil.generateETagForMetadataFileLocation(recreatedV1MetadataLocation); + + // Further evolution of recreated table + String recreatedV2MetadataLocation = baseLocation + "recreated-v2.metadata.json"; + String recreatedV2ETag = IcebergHttpUtil.generateETagForMetadataFileLocation(recreatedV2MetadataLocation); + + // All ETags should be unique + List allETags = List.of(v1ETag, v2ETag, v3ETag, recreatedV1ETag, recreatedV2ETag); + + // Verify all ETags are different from each other + for (int i = 0; i < allETags.size(); i++) { + for (int j = i + 1; j < allETags.size(); j++) { + assertThat(allETags.get(i)).isNotEqualTo(allETags.get(j)); + } + } + + // Test If-None-Match behavior across lifecycle + IfNoneMatch originalV1IfNoneMatch = IfNoneMatch.fromHeader(v1ETag); + + // Should match original v1 + assertThat(originalV1IfNoneMatch.anyMatch(v1ETag)).isTrue(); + + // Should NOT match any other version (evolution or recreation) + assertThat(originalV1IfNoneMatch.anyMatch(v2ETag)).isFalse(); + assertThat(originalV1IfNoneMatch.anyMatch(v3ETag)).isFalse(); + assertThat(originalV1IfNoneMatch.anyMatch(recreatedV1ETag)).isFalse(); + assertThat(originalV1IfNoneMatch.anyMatch(recreatedV2ETag)).isFalse(); + + // Test with multiple ETags from original table lifecycle + String multipleOriginalETags = v1ETag + ", " + v2ETag + ", " + v3ETag; + IfNoneMatch multipleOriginalIfNoneMatch = IfNoneMatch.fromHeader(multipleOriginalETags); + + // Should match any of the original table versions + assertThat(multipleOriginalIfNoneMatch.anyMatch(v1ETag)).isTrue(); + assertThat(multipleOriginalIfNoneMatch.anyMatch(v2ETag)).isTrue(); + assertThat(multipleOriginalIfNoneMatch.anyMatch(v3ETag)).isTrue(); + + // Should NOT match recreated table versions + assertThat(multipleOriginalIfNoneMatch.anyMatch(recreatedV1ETag)).isFalse(); + assertThat(multipleOriginalIfNoneMatch.anyMatch(recreatedV2ETag)).isFalse(); + } } From dffd2639c41011ea68a2da60363f8aec3dd878a3 Mon Sep 17 00:00:00 2001 From: Sandhya S Date: Fri, 27 Jun 2025 13:36:06 -0700 Subject: [PATCH 2/3] Added IT tests to test changes after DDL and DML --- .../PolarisRestCatalogIntegrationTest.java | 325 ++++++++++++++++-- 1 file changed, 305 insertions(+), 20 deletions(-) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java index 0366f28203..01def246b2 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java @@ -69,6 +69,10 @@ import org.apache.iceberg.rest.responses.ErrorResponse; import org.apache.iceberg.rest.responses.LoadTableResponse; import org.apache.iceberg.types.Types; +import org.apache.iceberg.AppendFiles; +import org.apache.iceberg.DataFile; +import org.apache.iceberg.DataFiles; +import org.apache.iceberg.Table; import org.apache.polaris.core.admin.model.AwsStorageConfigInfo; import org.apache.polaris.core.admin.model.Catalog; import org.apache.polaris.core.admin.model.CatalogGrant; @@ -1636,45 +1640,326 @@ public void testLoadTableWithWildcardIfNoneMatchReturns400() { } } + @Test - public void testLoadTableWithInvalidIfNoneMatchFormat() { - // Create a table first + public void testLoadNonExistentTableWithIfNoneMatch() { + // Create namespace but not the table Namespace ns1 = Namespace.of("ns1"); restCatalog.createNamespace(ns1); - restCatalog.buildTable(TableIdentifier.of(ns1, "test_table"), - new Schema(List.of(Types.NestedField.required(1, "col1", Types.StringType.get())))).create(); - // Load table with invalid If-None-Match header format - String invalidETag = "invalid-etag-format"; + // Try to load a non-existent table with If-None-Match header + String etag = "W/\"some-etag\""; Invocation invocation = catalogApi - .request("v1/" + currentCatalogName + "/namespaces/ns1/tables/test_table") - .header(HttpHeaders.IF_NONE_MATCH, invalidETag) + .request("v1/" + currentCatalogName + "/namespaces/ns1/tables/non_existent_table") + .header(HttpHeaders.IF_NONE_MATCH, etag) .build("GET"); try (Response response = invocation.invoke()) { - // Should return 400 Bad Request for invalid ETag format - assertThat(response.getStatus()).isEqualTo(Response.Status.BAD_REQUEST.getStatusCode()); + // Should return 404 Not Found regardless of If-None-Match header + assertThat(response.getStatus()).isEqualTo(Response.Status.NOT_FOUND.getStatusCode()); } } @Test - public void testLoadNonExistentTableWithIfNoneMatch() { - // Create namespace but not the table + public void testETagBehaviorForTableSchemaChangesIntegration() { + // Integration test equivalent of testETagBehaviorForTableSchemaChanges unit test Namespace ns1 = Namespace.of("ns1"); restCatalog.createNamespace(ns1); + TableIdentifier tableId = TableIdentifier.of(ns1, "test_schema_evolution_table"); - // Try to load a non-existent table with If-None-Match header - String etag = "W/\"some-etag\""; - Invocation invocation = + // Create initial table with v1 schema + Schema v1Schema = new Schema(List.of( + Types.NestedField.required(1, "id", Types.LongType.get()), + Types.NestedField.optional(2, "name", Types.StringType.get()) + )); + restCatalog.buildTable(tableId, v1Schema).create(); + + // Load table and get v1 ETag + Invocation v1Invocation = catalogApi - .request("v1/" + currentCatalogName + "/namespaces/ns1/tables/non_existent_table") - .header(HttpHeaders.IF_NONE_MATCH, etag) + .request("v1/" + currentCatalogName + "/namespaces/ns1/tables/test_schema_evolution_table") .build("GET"); - try (Response response = invocation.invoke()) { - // Should return 404 Not Found regardless of If-None-Match header - assertThat(response.getStatus()).isEqualTo(Response.Status.NOT_FOUND.getStatusCode()); + String v1ETag; + try (Response v1Response = v1Invocation.invoke()) { + assertThat(v1Response.getStatus()).isEqualTo(Response.Status.OK.getStatusCode()); + assertThat(v1Response.getHeaders()).containsKey(HttpHeaders.ETAG); + v1ETag = v1Response.getHeaders().getFirst(HttpHeaders.ETAG).toString(); + } + + // Evolve schema to v2 (add email column) + restCatalog.loadTable(tableId).updateSchema() + .addColumn("email", Types.StringType.get()) + .commit(); + + // Load table and get v2 ETag + Invocation v2Invocation = + catalogApi + .request("v1/" + currentCatalogName + "/namespaces/ns1/tables/test_schema_evolution_table") + .build("GET"); + + String v2ETag; + try (Response v2Response = v2Invocation.invoke()) { + assertThat(v2Response.getStatus()).isEqualTo(Response.Status.OK.getStatusCode()); + assertThat(v2Response.getHeaders()).containsKey(HttpHeaders.ETAG); + v2ETag = v2Response.getHeaders().getFirst(HttpHeaders.ETAG).toString(); + } + + // Evolve schema to v3 (add age column) + restCatalog.loadTable(tableId).updateSchema() + .addColumn("age", Types.IntegerType.get()) + .commit(); + + // Load table and get v3 ETag + Invocation v3Invocation = + catalogApi + .request("v1/" + currentCatalogName + "/namespaces/ns1/tables/test_schema_evolution_table") + .build("GET"); + + String v3ETag; + try (Response v3Response = v3Invocation.invoke()) { + assertThat(v3Response.getStatus()).isEqualTo(Response.Status.OK.getStatusCode()); + assertThat(v3Response.getHeaders()).containsKey(HttpHeaders.ETAG); + v3ETag = v3Response.getHeaders().getFirst(HttpHeaders.ETAG).toString(); + } + + // Verify all ETags are different + assertThat(v1ETag).isNotEqualTo(v2ETag); + assertThat(v1ETag).isNotEqualTo(v3ETag); + assertThat(v2ETag).isNotEqualTo(v3ETag); + + // Test If-None-Match with v1 ETag against current v3 table + Invocation v1EtagTestInvocation = + catalogApi + .request("v1/" + currentCatalogName + "/namespaces/ns1/tables/test_schema_evolution_table") + .header(HttpHeaders.IF_NONE_MATCH, v1ETag) + .build("GET"); + + try (Response v1EtagTestResponse = v1EtagTestInvocation.invoke()) { + // Should return 200 OK because table has evolved since v1 + assertThat(v1EtagTestResponse.getStatus()).isEqualTo(Response.Status.OK.getStatusCode()); + assertThat(v1EtagTestResponse.getHeaders()).containsKey(HttpHeaders.ETAG); + + String currentETag = v1EtagTestResponse.getHeaders().getFirst(HttpHeaders.ETAG).toString(); + assertThat(currentETag).isEqualTo(v3ETag); // Should match current v3 ETag + } + + // Test with multiple ETags including v1 and v2 + String multipleETags = v1ETag + ", " + v2ETag; + Invocation multipleEtagsInvocation = + catalogApi + .request("v1/" + currentCatalogName + "/namespaces/ns1/tables/test_schema_evolution_table") + .header(HttpHeaders.IF_NONE_MATCH, multipleETags) + .build("GET"); + + try (Response multipleEtagsResponse = multipleEtagsInvocation.invoke()) { + // Should return 200 OK because current v3 ETag doesn't match v1 or v2 + assertThat(multipleEtagsResponse.getStatus()).isEqualTo(Response.Status.OK.getStatusCode()); + } + + // Test with current v3 ETag + Invocation currentEtagInvocation = + catalogApi + .request("v1/" + currentCatalogName + "/namespaces/ns1/tables/test_schema_evolution_table") + .header(HttpHeaders.IF_NONE_MATCH, v3ETag) + .build("GET"); + + try (Response currentEtagResponse = currentEtagInvocation.invoke()) { + // Should return 304 Not Modified because ETag matches current version + assertThat(currentEtagResponse.getStatus()).isEqualTo(Response.Status.NOT_MODIFIED.getStatusCode()); + assertThat(currentEtagResponse.hasEntity()).isFalse(); + } + } + + @Test + public void testETagBehaviorForTableDropAndRecreateIntegration() { + // Integration test equivalent of testETagBehaviorForTableDropAndRecreate unit test + Namespace ns1 = Namespace.of("ns1"); + restCatalog.createNamespace(ns1); + TableIdentifier tableId = TableIdentifier.of(ns1, "test_drop_recreate_behavior_table"); + + // Create original table + Schema originalSchema = new Schema(List.of( + Types.NestedField.required(1, "original_id", Types.LongType.get()), + Types.NestedField.optional(2, "original_name", Types.StringType.get()) + )); + restCatalog.buildTable(tableId, originalSchema).create(); + + // Load original table and get ETag + Invocation originalInvocation = + catalogApi + .request("v1/" + currentCatalogName + "/namespaces/ns1/tables/test_drop_recreate_behavior_table") + .build("GET"); + + String originalETag; + String originalMetadataLocation; + try (Response originalResponse = originalInvocation.invoke()) { + assertThat(originalResponse.getStatus()).isEqualTo(Response.Status.OK.getStatusCode()); + assertThat(originalResponse.getHeaders()).containsKey(HttpHeaders.ETAG); + originalETag = originalResponse.getHeaders().getFirst(HttpHeaders.ETAG).toString(); + + LoadTableResponse originalLoadResponse = originalResponse.readEntity(LoadTableResponse.class); + originalMetadataLocation = originalLoadResponse.metadataLocation(); + } + + // Drop the table + restCatalog.dropTable(tableId); + + // Recreate table with completely different schema + Schema recreatedSchema = new Schema(List.of( + Types.NestedField.required(1, "recreated_uuid", Types.StringType.get()), + Types.NestedField.optional(2, "recreated_data", Types.StringType.get()), + Types.NestedField.optional(3, "recreated_timestamp", Types.TimestampType.withoutZone()) + )); + restCatalog.buildTable(tableId, recreatedSchema).create(); + + // Load recreated table and get ETag + Invocation recreatedInvocation = + catalogApi + .request("v1/" + currentCatalogName + "/namespaces/ns1/tables/test_drop_recreate_behavior_table") + .build("GET"); + + String recreatedETag; + String recreatedMetadataLocation; + try (Response recreatedResponse = recreatedInvocation.invoke()) { + assertThat(recreatedResponse.getStatus()).isEqualTo(Response.Status.OK.getStatusCode()); + assertThat(recreatedResponse.getHeaders()).containsKey(HttpHeaders.ETAG); + recreatedETag = recreatedResponse.getHeaders().getFirst(HttpHeaders.ETAG).toString(); + + LoadTableResponse recreatedLoadResponse = recreatedResponse.readEntity(LoadTableResponse.class); + recreatedMetadataLocation = recreatedLoadResponse.metadataLocation(); + } + + // Verify ETags and metadata locations are completely different + assertThat(originalETag).isNotEqualTo(recreatedETag); + assertThat(originalMetadataLocation).isNotEqualTo(recreatedMetadataLocation); + + // Test If-None-Match with original ETag against recreated table + Invocation originalEtagTestInvocation = + catalogApi + .request("v1/" + currentCatalogName + "/namespaces/ns1/tables/test_drop_recreate_behavior_table") + .header(HttpHeaders.IF_NONE_MATCH, originalETag) + .build("GET"); + + try (Response originalEtagTestResponse = originalEtagTestInvocation.invoke()) { + // Should return 200 OK because it's a completely different table + assertThat(originalEtagTestResponse.getStatus()).isEqualTo(Response.Status.OK.getStatusCode()); + assertThat(originalEtagTestResponse.getHeaders()).containsKey(HttpHeaders.ETAG); + + String currentETag = originalEtagTestResponse.getHeaders().getFirst(HttpHeaders.ETAG).toString(); + assertThat(currentETag).isEqualTo(recreatedETag); // Should match recreated table ETag + + LoadTableResponse currentLoadResponse = originalEtagTestResponse.readEntity(LoadTableResponse.class); + + // Verify we get the recreated table schema (not the original) + assertThat(currentLoadResponse.tableMetadata().schema().columns()).hasSize(3); + assertThat(currentLoadResponse.tableMetadata().schema().findField("recreated_uuid")).isNotNull(); + assertThat(currentLoadResponse.tableMetadata().schema().findField("recreated_data")).isNotNull(); + assertThat(currentLoadResponse.tableMetadata().schema().findField("recreated_timestamp")).isNotNull(); + + // Verify original schema fields are NOT present + assertThat(currentLoadResponse.tableMetadata().schema().findField("original_id")).isNull(); + assertThat(currentLoadResponse.tableMetadata().schema().findField("original_name")).isNull(); + } + + // Test with current recreated ETag + Invocation currentEtagInvocation = + catalogApi + .request("v1/" + currentCatalogName + "/namespaces/ns1/tables/test_drop_recreate_behavior_table") + .header(HttpHeaders.IF_NONE_MATCH, recreatedETag) + .build("GET"); + + try (Response currentEtagResponse = currentEtagInvocation.invoke()) { + // Should return 304 Not Modified because ETag matches current recreated table + assertThat(currentEtagResponse.getStatus()).isEqualTo(Response.Status.NOT_MODIFIED.getStatusCode()); + assertThat(currentEtagResponse.hasEntity()).isFalse(); + } + } + + @Test + public void testETagChangeAfterDMLOperations() { + // Test that ETags change after DML operations (INSERT, UPDATE, DELETE) + Namespace ns1 = Namespace.of("ns1"); + restCatalog.createNamespace(ns1); + TableIdentifier tableId = TableIdentifier.of(ns1, "test_dml_etag_table"); + + // Create table with initial schema + Schema schema = new Schema(List.of( + Types.NestedField.required(1, "id", Types.LongType.get()), + Types.NestedField.optional(2, "name", Types.StringType.get()), + Types.NestedField.optional(3, "value", Types.IntegerType.get()) + )); + restCatalog.buildTable(tableId, schema).create(); + + // Load table and get initial ETag (before any data) + Invocation initialInvocation = + catalogApi + .request("v1/" + currentCatalogName + "/namespaces/ns1/tables/test_dml_etag_table") + .build("GET"); + + String initialETag; + String initialMetadataLocation; + try (Response initialResponse = initialInvocation.invoke()) { + assertThat(initialResponse.getStatus()).isEqualTo(Response.Status.OK.getStatusCode()); + assertThat(initialResponse.getHeaders()).containsKey(HttpHeaders.ETAG); + initialETag = initialResponse.getHeaders().getFirst(HttpHeaders.ETAG).toString(); + + LoadTableResponse initialLoadResponse = initialResponse.readEntity(LoadTableResponse.class); + initialMetadataLocation = initialLoadResponse.metadataLocation(); + } + + // Simulate DML operation by creating a new snapshot (append operation) + Table table = restCatalog.loadTable(tableId); + + // Create a data file and append it (simulating INSERT operation) + AppendFiles append = table.newAppend(); + + // Create a mock data file entry + DataFile dataFile = DataFiles.builder(table.spec()) + .withPath("/path/to/data/file1.parquet") + .withFileSizeInBytes(1024) + .withRecordCount(100) + .build(); + + append.appendFile(dataFile); + append.commit(); // This creates a new snapshot and should change the ETag + + // Load table after DML operation and get new ETag + Invocation afterDMLInvocation = + catalogApi + .request("v1/" + currentCatalogName + "/namespaces/ns1/tables/test_dml_etag_table") + .build("GET"); + + String afterDMLETag; + String afterDMLMetadataLocation; + try (Response afterDMLResponse = afterDMLInvocation.invoke()) { + assertThat(afterDMLResponse.getStatus()).isEqualTo(Response.Status.OK.getStatusCode()); + assertThat(afterDMLResponse.getHeaders()).containsKey(HttpHeaders.ETAG); + afterDMLETag = afterDMLResponse.getHeaders().getFirst(HttpHeaders.ETAG).toString(); + + LoadTableResponse afterDMLLoadResponse = afterDMLResponse.readEntity(LoadTableResponse.class); + afterDMLMetadataLocation = afterDMLLoadResponse.metadataLocation(); + } + + // Verify ETag and metadata location changed after DML operation + assertThat(initialETag).isNotEqualTo(afterDMLETag); + assertThat(initialMetadataLocation).isNotEqualTo(afterDMLMetadataLocation); + + // Test If-None-Match with initial ETag after DML operation + Invocation initialEtagTestInvocation = + catalogApi + .request("v1/" + currentCatalogName + "/namespaces/ns1/tables/test_dml_etag_table") + .header(HttpHeaders.IF_NONE_MATCH, initialETag) + .build("GET"); + + try (Response initialEtagTestResponse = initialEtagTestInvocation.invoke()) { + // Should return 200 OK because table has new snapshot after DML + assertThat(initialEtagTestResponse.getStatus()).isEqualTo(Response.Status.OK.getStatusCode()); + assertThat(initialEtagTestResponse.getHeaders()).containsKey(HttpHeaders.ETAG); + + String currentETag = initialEtagTestResponse.getHeaders().getFirst(HttpHeaders.ETAG).toString(); + assertThat(currentETag).isEqualTo(afterDMLETag); // Should match post-DML ETag } } } From 770b6bce05803f4bec45e61b38f437050986b023 Mon Sep 17 00:00:00 2001 From: Sandhya S Date: Fri, 27 Jun 2025 16:43:15 -0700 Subject: [PATCH 3/3] Consolidated some duplicate tests and addressed review comments. --- .../PolarisRestCatalogIntegrationBase.java | 21 +++++- .../polaris/service/http/IfNoneMatchTest.java | 71 +++++-------------- 2 files changed, 34 insertions(+), 58 deletions(-) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationBase.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationBase.java index 9a2e46ebd8..8ebee36f0a 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationBase.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationBase.java @@ -1670,8 +1670,7 @@ public void testLoadNonExistentTableWithIfNoneMatch() { } @Test - public void testETagBehaviorForTableSchemaChangesIntegration() { - // Integration test equivalent of testETagBehaviorForTableSchemaChanges unit test + public void testETagBehaviorForTableSchemaChanges() { Namespace ns1 = Namespace.of("ns1"); restCatalog.createNamespace(ns1); TableIdentifier tableId = TableIdentifier.of(ns1, "test_schema_evolution_table"); @@ -1776,6 +1775,22 @@ public void testETagBehaviorForTableSchemaChangesIntegration() { assertThat(multipleEtagsResponse.getStatus()).isEqualTo(Response.Status.OK.getStatusCode()); } + // Test with multiple ETags including v1 and v3 + multipleETags = v1ETag + ", " + v3ETag; + multipleEtagsInvocation = + catalogApi + .request( + "v1/" + currentCatalogName + "/namespaces/ns1/tables/test_schema_evolution_table") + .header(HttpHeaders.IF_NONE_MATCH, multipleETags) + .build("GET"); + + try (Response multipleEtagsResponse = multipleEtagsInvocation.invoke()) { + // Should return 304 Not Modified because ETag matches current v3 + assertThat(multipleEtagsResponse.getStatus()) + .isEqualTo(Response.Status.NOT_MODIFIED.getStatusCode()); + assertThat(multipleEtagsResponse.hasEntity()).isFalse(); + } + // Test with current v3 ETag Invocation currentEtagInvocation = catalogApi @@ -1962,7 +1977,7 @@ public void testETagChangeAfterDMLOperations() { // Create a mock data file entry DataFile dataFile = DataFiles.builder(table.spec()) - .withPath("/path/to/data/file1.parquet") + .withPath(table.locationProvider().newDataLocation("file1.parquet")) .withFileSizeInBytes(1024) .withRecordCount(100) .build(); diff --git a/service/common/src/test/java/org/apache/polaris/service/http/IfNoneMatchTest.java b/service/common/src/test/java/org/apache/polaris/service/http/IfNoneMatchTest.java index c865d9a696..14eb25bfda 100644 --- a/service/common/src/test/java/org/apache/polaris/service/http/IfNoneMatchTest.java +++ b/service/common/src/test/java/org/apache/polaris/service/http/IfNoneMatchTest.java @@ -21,13 +21,14 @@ import static org.assertj.core.api.Assertions.assertThat; import java.util.List; +import java.util.Set; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; /** - * Tests for If-None-Match header processing and ETag interaction scenarios. - * This includes both HTTP header parsing and tests that verify how ETag generation - * works together with If-None-Match processing for metadata location changes. + * Tests for If-None-Match header processing and ETag interaction scenarios. This includes both HTTP + * header parsing and tests that verify how ETag generation works together with If-None-Match + * processing for metadata location changes. */ public class IfNoneMatchTest { @@ -137,7 +138,6 @@ public void invalidHeaderWithOnlyWhitespacesBetween() { IllegalArgumentException.class, () -> IfNoneMatch.fromHeader("W/\"etag\" \"valid-etag\"")); } - @Test public void testETagGenerationConsistency() { // Test that ETag generation is consistent for the same metadata location @@ -151,28 +151,16 @@ public void testETagGenerationConsistency() { assertThat(etag1).endsWith("\""); } - @Test - public void testETagGenerationDifferentLocations() { - // Test that different metadata locations generate different ETags - String location1 = "s3://bucket/path/metadata1.json"; - String location2 = "s3://bucket/path/metadata2.json"; - - String etag1 = IcebergHttpUtil.generateETagForMetadataFileLocation(location1); - String etag2 = IcebergHttpUtil.generateETagForMetadataFileLocation(location2); - - assertThat(etag1).isNotEqualTo(etag2); - assertThat(etag1).startsWith("W/\"").endsWith("\""); - assertThat(etag2).startsWith("W/\"").endsWith("\""); - } - @Test public void testETagChangeAfterMetadataLocationChange() { // Test that ETags change when metadata location changes (simulating schema updates) String originalMetadataLocation = "s3://bucket/path/metadata/v1.metadata.json"; String updatedMetadataLocation = "s3://bucket/path/metadata/v2.metadata.json"; - String originalETag = IcebergHttpUtil.generateETagForMetadataFileLocation(originalMetadataLocation); - String updatedETag = IcebergHttpUtil.generateETagForMetadataFileLocation(updatedMetadataLocation); + String originalETag = + IcebergHttpUtil.generateETagForMetadataFileLocation(originalMetadataLocation); + String updatedETag = + IcebergHttpUtil.generateETagForMetadataFileLocation(updatedMetadataLocation); // ETags should be different for different metadata locations assertThat(originalETag).isNotEqualTo(updatedETag); @@ -209,9 +197,10 @@ public void testETagBehaviorForTableSchemaChanges() { String v3ETag = IcebergHttpUtil.generateETagForMetadataFileLocation(v3MetadataLocation); // All ETags should be different - assertThat(v1ETag).isNotEqualTo(v2ETag); - assertThat(v1ETag).isNotEqualTo(v3ETag); - assertThat(v2ETag).isNotEqualTo(v3ETag); + Set etagSet = Set.of(v1ETag, v2ETag, v3ETag); + assertThat(etagSet) + .as("Schema evolution should generate unique ETags for each version (v1, v2, v3)") + .hasSize(3); // Test If-None-Match with original ETag after schema changes IfNoneMatch originalIfNoneMatch = IfNoneMatch.fromHeader(v1ETag); @@ -235,36 +224,6 @@ public void testETagBehaviorForTableSchemaChanges() { assertThat(multipleIfNoneMatch.anyMatch(v3ETag)).isFalse(); } - @Test - public void testETagBehaviorForTableDropAndRecreate() { - // Simulate a table drop and recreate scenario - String baseLocation = "s3://warehouse/db/table/metadata/"; - - // Original table metadata location - String originalMetadataLocation = baseLocation + "original-v1.metadata.json"; - String originalETag = IcebergHttpUtil.generateETagForMetadataFileLocation(originalMetadataLocation); - - // After dropping and recreating table (completely new metadata location) - String recreatedMetadataLocation = "s3://warehouse/db/table/metadata/recreated-v1.metadata.json"; - String recreatedETag = IcebergHttpUtil.generateETagForMetadataFileLocation(recreatedMetadataLocation); - - // ETags should be completely different for different metadata locations - assertThat(originalETag).isNotEqualTo(recreatedETag); - - // Both should be valid weak ETags - assertThat(originalETag).startsWith("W/\"").endsWith("\""); - assertThat(recreatedETag).startsWith("W/\"").endsWith("\""); - - // Test If-None-Match with original ETag after table recreation - IfNoneMatch originalIfNoneMatch = IfNoneMatch.fromHeader(originalETag); - - // Should match the original ETag - assertThat(originalIfNoneMatch.anyMatch(originalETag)).isTrue(); - - // Should NOT match the recreated table's ETag (completely different table) - assertThat(originalIfNoneMatch.anyMatch(recreatedETag)).isFalse(); - } - @Test public void testETagUniquenessAcrossTableLifecycle() { // Test ETag uniqueness across the complete table lifecycle @@ -284,11 +243,13 @@ public void testETagUniquenessAcrossTableLifecycle() { // Table dropped and recreated with different schema (new metadata path) String recreatedV1MetadataLocation = baseLocation + "recreated-v1.metadata.json"; - String recreatedV1ETag = IcebergHttpUtil.generateETagForMetadataFileLocation(recreatedV1MetadataLocation); + String recreatedV1ETag = + IcebergHttpUtil.generateETagForMetadataFileLocation(recreatedV1MetadataLocation); // Further evolution of recreated table String recreatedV2MetadataLocation = baseLocation + "recreated-v2.metadata.json"; - String recreatedV2ETag = IcebergHttpUtil.generateETagForMetadataFileLocation(recreatedV2MetadataLocation); + String recreatedV2ETag = + IcebergHttpUtil.generateETagForMetadataFileLocation(recreatedV2MetadataLocation); // All ETags should be unique List allETags = List.of(v1ETag, v2ETag, v3ETag, recreatedV1ETag, recreatedV2ETag);