sandhyasun commented on code in PR #1972:
URL: https://github.com/apache/polaris/pull/1972#discussion_r2175647752
##########
service/common/src/test/java/org/apache/polaris/service/http/IfNoneMatchTest.java:
##########
@@ -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<String> 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));
Review Comment:
This 4th commit should show the change ?
[https://github.com/apache/polaris/pull/1972/commits/770b6bce05803f4bec45e61b38f437050986b023](url)
--
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
