PR feedback - remove making public last assigned partition id as we'r…

…e not testing PartitionSpec anyway
apache · Jun 24, 2022 · 9ac577e · 9ac577e
1 parent d709524
commit 9ac577e
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 16 deletions.
diff --git a/api/src/main/java/org/apache/iceberg/PartitionSpec.java b/api/src/main/java/org/apache/iceberg/PartitionSpec.java
@@ -102,8 +102,7 @@ public boolean isUnpartitioned() {
     return !isPartitioned();
   }
 
-  // visible for testing
-  public int lastAssignedFieldId() {
+  int lastAssignedFieldId() {
     return lastAssignedFieldId;
   }
 

diff --git a/core/src/test/java/org/apache/iceberg/rest/RequestResponseTestBase.java b/core/src/test/java/org/apache/iceberg/rest/RequestResponseTestBase.java
@@ -22,6 +22,7 @@
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import org.assertj.core.api.Assertions;
+import org.junit.Assert;
 import org.junit.Test;
 
 public abstract class RequestResponseTestBase<T extends RESTMessage> {
@@ -86,7 +87,7 @@ protected void assertRoundTripSerializesEquallyFrom(String json, T expected) thr
     T actual = deserialize(json);
     assertEquals(actual, expected);
 
-    // Check that the deserialized value serializes back into the original JSON
-    Assertions.assertThat(serialize(expected)).isEqualTo(json);
+    Assert.assertEquals("The serialized version of the Java record should match the original JSON",
+        serialize(expected), json);
   }
 }
diff --git a/core/src/test/java/org/apache/iceberg/rest/responses/TestLoadTableResponse.java b/core/src/test/java/org/apache/iceberg/rest/responses/TestLoadTableResponse.java
@@ -58,15 +58,15 @@ public class TestLoadTableResponse extends RequestResponseTestBase<LoadTableResp
 
   private static final String TEST_TABLE_LOCATION = "s3://bucket/test/location";
 
-  private static final Schema TEST_SCHEMA = new Schema(7,
+  private static final Schema SCHEMA_7 = new Schema(7,
       Types.NestedField.required(1, "x", Types.LongType.get()),
       Types.NestedField.required(2, "y", Types.LongType.get(), "comment"),
       Types.NestedField.required(3, "z", Types.LongType.get())
   );
 
-  private static final PartitionSpec SPEC_5 = PartitionSpec.builderFor(TEST_SCHEMA).withSpecId(5).build();
+  private static final PartitionSpec SPEC_5 = PartitionSpec.builderFor(SCHEMA_7).withSpecId(5).build();
 
-  private static final SortOrder SORT_ORDER_3 = SortOrder.builderFor(TEST_SCHEMA)
+  private static final SortOrder SORT_ORDER_3 = SortOrder.builderFor(SCHEMA_7)
       .withOrderId(3)
       .asc("y", NullOrder.NULLS_FIRST)
       .desc(Expressions.bucket("z", 4), NullOrder.NULLS_LAST)
@@ -77,8 +77,6 @@ public class TestLoadTableResponse extends RequestResponseTestBase<LoadTableResp
 
   private static final long PREVIOUS_SNAPSHOT_ID = System.currentTimeMillis() - new Random(1234).nextInt(3600);
 
-  private static final String TEST_UUIID = "01073077-d2fd-4132-b86c-09d5107e4747";
-
   private static final Map<String, String> TABLE_PROPS = ImmutableMap.of(
       "format-version", "1",
       "owner", "hank");
@@ -98,7 +96,7 @@ public String[] allFieldsFromSpec() {
   @Override
   public LoadTableResponse createExampleInstance() {
     TableMetadata metadata = TableMetadata.newTableMetadata(
-        TEST_SCHEMA, SPEC_5, SORT_ORDER_3, TEST_TABLE_LOCATION, TABLE_PROPS);
+        SCHEMA_7, SPEC_5, SORT_ORDER_3, TEST_TABLE_LOCATION, TABLE_PROPS);
     // Add location
     TableMetadata metadataWithLocation = TableMetadata
         .buildFrom(metadata)
@@ -132,7 +130,7 @@ public void testRoundTripSerDe() throws JsonProcessingException {
     // Build table metadata so that its uuid and last updated millis can be reused.
     TableMetadata metaV1 = TableMetadata.buildFrom(
         TableMetadata.newTableMetadata(
-            TEST_SCHEMA, SPEC_5, SORT_ORDER_3, TEST_TABLE_LOCATION, TABLE_PROPS))
+            SCHEMA_7, SPEC_5, SORT_ORDER_3, TEST_TABLE_LOCATION, TABLE_PROPS))
         .withMetadataLocation(TEST_METADATA_LOCATION)
         .build();
 
@@ -283,9 +281,9 @@ private TableMetadata buildV2TableMetadataWithTagsAndSnapshotHistory() {
     //  there.
     TableMetadata expected = new TableMetadata(TEST_METADATA_LOCATION, 2, UUID.randomUUID().toString(),
         TEST_TABLE_LOCATION,
-        SEQ_NO, System.currentTimeMillis(), 3,
-        7, ImmutableList.of(TEST_SCHEMA, schema),
-        5, ImmutableList.of(SPEC_5), SPEC_5.lastAssignedFieldId(),
+        SEQ_NO, System.currentTimeMillis(), LAST_ASSIGNED_COLUMN_ID,
+        7, ImmutableList.of(SCHEMA_7, schema),
+        5, ImmutableList.of(SPEC_5), 999,
         3, ImmutableList.of(SORT_ORDER_3), ImmutableMap.of("property", "value"), currentSnapshotId,
         Arrays.asList(previousSnapshot, currentSnapshot), snapshotLog, ImmutableList.of(), refs,
         ImmutableList.of());
@@ -330,8 +328,6 @@ void assertEqualTableMetadata(TableMetadata actual, TableMetadata expected) {
         expected.defaultSpecId(), actual.defaultSpecId());
     Assert.assertEquals("PartitionSpec map should match",
         expected.specs(), actual.specs());
-    Assert.assertEquals("lastAssignedFieldId across all PartitionSpecs should match",
-        expected.spec().lastAssignedFieldId(), actual.lastAssignedPartitionId());
     Assert.assertEquals("Default Sort ID should match",
         expected.defaultSortOrderId(), actual.defaultSortOrderId());
     Assert.assertEquals("Sort order should match",