[iceberg] branch master updated: Core: Use unknown report type for forward-compatibility (#7145)

[blue]

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 49fb12e66e Core: Use unknown report type for forward-compatibility 
(#7145)
49fb12e66e is described below

commit 49fb12e66e865c0b0e1dcabb68cd9ebf1e9989b1
Author: Eduard Tudenhöfner <etudenhoef...@gmail.com>
AuthorDate: Mon Mar 20 16:36:44 2023 +0100

    Core: Use unknown report type for forward-compatibility (#7145)
---
 .../rest/requests/ReportMetricsRequest.java        | 15 ++++++---
 .../rest/requests/ReportMetricsRequestParser.java  |  2 +-
 .../requests/TestReportMetricsRequestParser.java   | 37 +++++++++++++++-------
 3 files changed, 37 insertions(+), 17 deletions(-)

diff --git 
a/core/src/main/java/org/apache/iceberg/rest/requests/ReportMetricsRequest.java 
b/core/src/main/java/org/apache/iceberg/rest/requests/ReportMetricsRequest.java
index c85a6e9add..01f04f0e20 100644
--- 
a/core/src/main/java/org/apache/iceberg/rest/requests/ReportMetricsRequest.java
+++ 
b/core/src/main/java/org/apache/iceberg/rest/requests/ReportMetricsRequest.java
@@ -30,6 +30,7 @@ import org.immutables.value.Value;
 public interface ReportMetricsRequest extends RESTRequest {
 
   enum ReportType {
+    UNKNOWN,
     SCAN_REPORT,
     COMMIT_REPORT;
 
@@ -38,7 +39,7 @@ public interface ReportMetricsRequest extends RESTRequest {
       try {
         return ReportType.valueOf(reportType.toUpperCase(Locale.ENGLISH));
       } catch (IllegalArgumentException e) {
-        throw new IllegalArgumentException(String.format("Invalid report type: 
%s", reportType), e);
+        return UNKNOWN;
       }
     }
   }
@@ -54,16 +55,20 @@ public interface ReportMetricsRequest extends RESTRequest {
   }
 
   static ReportMetricsRequest of(MetricsReport report) {
-    ReportType reportType = null;
+    ReportType reportType = ReportType.UNKNOWN;
     if (report instanceof ScanReport) {
       reportType = ReportType.SCAN_REPORT;
     } else if (report instanceof CommitReport) {
       reportType = ReportType.COMMIT_REPORT;
     }
 
-    Preconditions.checkArgument(
-        null != reportType, "Unsupported report type: %s", 
report.getClass().getName());
-
     return 
ImmutableReportMetricsRequest.builder().reportType(reportType).report(report).build();
   }
+
+  static ReportMetricsRequest unknown() {
+    return ImmutableReportMetricsRequest.builder()
+        .reportType(ReportType.UNKNOWN)
+        .report(new MetricsReport() {})
+        .build();
+  }
 }
diff --git 
a/core/src/main/java/org/apache/iceberg/rest/requests/ReportMetricsRequestParser.java
 
b/core/src/main/java/org/apache/iceberg/rest/requests/ReportMetricsRequestParser.java
index e6d98cdac3..9bfdc4a8e0 100644
--- 
a/core/src/main/java/org/apache/iceberg/rest/requests/ReportMetricsRequestParser.java
+++ 
b/core/src/main/java/org/apache/iceberg/rest/requests/ReportMetricsRequestParser.java
@@ -94,6 +94,6 @@ public class ReportMetricsRequestParser {
           .build();
     }
 
-    throw new IllegalArgumentException(String.format("Cannot build metrics 
request from %s", json));
+    return ReportMetricsRequest.unknown();
   }
 }
diff --git 
a/core/src/test/java/org/apache/iceberg/rest/requests/TestReportMetricsRequestParser.java
 
b/core/src/test/java/org/apache/iceberg/rest/requests/TestReportMetricsRequestParser.java
index 70a219e791..465714b173 100644
--- 
a/core/src/test/java/org/apache/iceberg/rest/requests/TestReportMetricsRequestParser.java
+++ 
b/core/src/test/java/org/apache/iceberg/rest/requests/TestReportMetricsRequestParser.java
@@ -67,19 +67,34 @@ public class TestReportMetricsRequestParser {
 
   @Test
   public void invalidReportType() {
-    Assertions.assertThatThrownBy(
-            () -> 
ReportMetricsRequestParser.fromJson("{\"report-type\":\"invalid\"}"))
-        .isInstanceOf(IllegalArgumentException.class)
-        .hasMessage("Invalid report type: invalid");
+    Assertions.assertThat(
+            
ReportMetricsRequestParser.fromJson("{\"report-type\":\"invalid\"}").reportType())
+        .isEqualTo(ReportMetricsRequest.unknown().reportType());
 
-    Assertions.assertThatThrownBy(
-            () ->
-                ReportMetricsRequestParser.fromJson(
+    Assertions.assertThat(
+            ReportMetricsRequestParser.fromJson(
                     ReportMetricsRequestParser.toJson(
-                        ReportMetricsRequest.of(new MetricsReport() {}))))
-        .isInstanceOf(IllegalArgumentException.class)
-        .hasMessage(
-            "Unsupported report type: 
org.apache.iceberg.rest.requests.TestReportMetricsRequestParser$1");
+                        ReportMetricsRequest.of(new MetricsReport() {})))
+                .reportType())
+        .isEqualTo(ReportMetricsRequest.unknown().reportType());
+
+    // this is simulating a newer client sending a request to server running 
an older version (not
+    // knowing the new report type). this should not fail parsing on the server
+    String json =
+        "{\n"
+            + "  \"report-type\" : \"new-report-type\",\n"
+            + "  \"table-name\" : \"roundTripTableName\",\n"
+            + "  \"snapshot-id\" : 23,\n"
+            + "  \"filter\" : true,\n"
+            + "  \"schema-id\" : 4,\n"
+            + "  \"projected-field-ids\" : [ 1, 2, 3 ],\n"
+            + "  \"projected-field-names\" : [ \"c1\", \"c2\", \"c3\" ],\n"
+            + "  \"metrics\" : { }\n"
+            + "}";
+
+    ReportMetricsRequest request = ReportMetricsRequestParser.fromJson(json);
+    Assertions.assertThat(request.reportType())
+        .isEqualTo(ReportMetricsRequest.unknown().reportType());
   }
 
   @Test