[iceberg] branch master updated: Core: Add round-trip serialization to RESTCatalog tests (#4881)

[bl...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

blue pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/iceberg.git


The following commit(s) were added to refs/heads/master by this push:
     new 22e3b45f9 Core: Add round-trip serialization to RESTCatalog tests (#4881)
22e3b45f9 is described below

commit 22e3b45f970d54d9e5cfbb156c4e492b019beccd
Author: Ryan Blue <bl...@apache.org>
AuthorDate: Fri May 27 10:04:51 2022 -0700

    Core: Add round-trip serialization to RESTCatalog tests (#4881)
---
 .../java/org/apache/iceberg/TableMetadata.java     |  8 ++++++-
 .../org/apache/iceberg/rest/CatalogHandlers.java   | 15 +++++++++++-
 .../org/apache/iceberg/rest/TestRESTCatalog.java   | 27 +++++++++++++++++++++-
 3 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/core/src/main/java/org/apache/iceberg/TableMetadata.java b/core/src/main/java/org/apache/iceberg/TableMetadata.java
index 123f4fb30..f49566cf3 100644
--- a/core/src/main/java/org/apache/iceberg/TableMetadata.java
+++ b/core/src/main/java/org/apache/iceberg/TableMetadata.java
@@ -1158,8 +1158,14 @@ public class TableMetadata implements Serializable {
       return this;
     }
 
+    private boolean hasChanges() {
+      return changes.size() != startingChangeCount ||
+          (discardChanges && changes.size() > 0) ||
+          metadataLocation != null;
+    }
+
     public TableMetadata build() {
-      if (changes.size() == startingChangeCount && !(discardChanges && changes.size() > 0)) {
+      if (!hasChanges()) {
         return base;
       }
 
diff --git a/core/src/main/java/org/apache/iceberg/rest/CatalogHandlers.java b/core/src/main/java/org/apache/iceberg/rest/CatalogHandlers.java
index bf5885baa..53e905bd3 100644
--- a/core/src/main/java/org/apache/iceberg/rest/CatalogHandlers.java
+++ b/core/src/main/java/org/apache/iceberg/rest/CatalogHandlers.java
@@ -168,11 +168,24 @@ public class CatalogHandlers {
     properties.put("created-at", OffsetDateTime.now().toString());
     properties.putAll(request.properties());
 
+    String location;
+    if (request.location() != null) {
+      location = request.location();
+    } else {
+      location = catalog.buildTable(ident, request.schema())
+          .withPartitionSpec(request.spec())
+          .withSortOrder(request.writeOrder())
+          .withProperties(properties)
+          .createTransaction()
+          .table()
+          .location();
+    }
+
     TableMetadata metadata = TableMetadata.newTableMetadata(
         request.schema(),
         request.spec() != null ? request.spec() : PartitionSpec.unpartitioned(),
         request.writeOrder() != null ? request.writeOrder() : SortOrder.unsorted(),
-        request.location(),
+        location,
         properties);
 
     return LoadTableResponse.builder()
diff --git a/core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java b/core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java
index d10bead64..941b23646 100644
--- a/core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java
+++ b/core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java
@@ -19,6 +19,8 @@
 
 package org.apache.iceberg.rest;
 
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.ObjectMapper;
 import java.io.File;
 import java.io.IOException;
 import java.nio.file.Path;
@@ -56,6 +58,8 @@ import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.times;
 
 public class TestRESTCatalog extends CatalogTests<RESTCatalog> {
+  private static final ObjectMapper MAPPER = RESTObjectMapper.mapper();
+
   @TempDir
   public Path temp;
 
@@ -94,7 +98,10 @@ public class TestRESTCatalog extends CatalogTests<RESTCatalog> {
             Assertions.assertEquals(contextHeaders, headers, "Headers did not match for path: " + path);
           }
         }
-        return super.execute(method, path, body, responseType, headers, errorHandler);
+        Object request = roundTripSerialize(body, "request");
+        T response = super.execute(method, path, request, responseType, headers, errorHandler);
+        T responseAfterSerialization = roundTripSerialize(response, "response");
+        return responseAfterSerialization;
       }
     };
 
@@ -106,6 +113,24 @@ public class TestRESTCatalog extends CatalogTests<RESTCatalog> {
     restCatalog.initialize("prod", ImmutableMap.of(CatalogProperties.URI, "ignored", "credential", "catalog:12345"));
   }
 
+  @SuppressWarnings("unchecked")
+  public static <T> T roundTripSerialize(T payload, String description) {
+    if (payload != null) {
+      try {
+        if (payload instanceof RESTMessage) {
+          return (T) MAPPER.readValue(MAPPER.writeValueAsString(payload), payload.getClass());
+        } else {
+          // use Map so that Jackson doesn't try to instantiate ImmutableMap from payload.getClass()
+          return (T) MAPPER.readValue(MAPPER.writeValueAsString(payload), Map.class);
+        }
+      } catch (JsonProcessingException e) {
+        throw new RuntimeException(
+            String.format("Failed to serialize and deserialize %s: %s", description, payload), e);
+      }
+    }
+    return null;
+  }
+
   @AfterEach
   public void closeCatalog() throws IOException {
     if (restCatalog != null) {