API: Provide better error message for invalid FileFormat enum (apache…

…#5918)
rafoid · Oct 11, 2022 · 8c6adf6 · 8c6adf6
1 parent b71c37d
commit 8c6adf6
Show file tree

Hide file tree

Showing 89 changed files with 115 additions and 195 deletions.
diff --git a/api/src/main/java/org/apache/iceberg/FileFormat.java b/api/src/main/java/org/apache/iceberg/FileFormat.java
@@ -18,6 +18,8 @@
  */
 package org.apache.iceberg;
 
+import java.util.Locale;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
 import org.apache.iceberg.types.Comparators;
 
 /** Enum of supported file formats. */
@@ -64,4 +66,13 @@ public static FileFormat fromFileName(CharSequence filename) {
 
     return null;
   }
+
+  public static FileFormat fromString(String fileFormat) {
+    Preconditions.checkArgument(null != fileFormat, "Invalid file format: null");
+    try {
+      return FileFormat.valueOf(fileFormat.toUpperCase(Locale.ENGLISH));
+    } catch (IllegalArgumentException e) {
+      throw new IllegalArgumentException(String.format("Invalid file format: %s", fileFormat), e);
+    }
+  }
 }
diff --git a/core/src/main/java/org/apache/iceberg/BaseFile.java b/core/src/main/java/org/apache/iceberg/BaseFile.java
@@ -249,7 +249,7 @@ public void put(int i, Object value) {
         this.filePath = value.toString();
         return;
       case 2:
-        this.format = FileFormat.valueOf(value.toString());
+        this.format = FileFormat.fromString(value.toString());
         return;
       case 3:
         this.partitionSpecId = (value != null) ? (Integer) value : -1;

diff --git a/core/src/main/java/org/apache/iceberg/DataFiles.java b/core/src/main/java/org/apache/iceberg/DataFiles.java
@@ -20,7 +20,6 @@
 
 import java.nio.ByteBuffer;
 import java.util.List;
-import java.util.Locale;
 import java.util.Map;
 import org.apache.hadoop.fs.FileStatus;
 import org.apache.iceberg.encryption.EncryptedOutputFile;
@@ -210,7 +209,7 @@ public Builder withPath(String newFilePath) {
     }
 
     public Builder withFormat(String newFormat) {
-      this.format = FileFormat.valueOf(newFormat.toUpperCase(Locale.ENGLISH));
+      this.format = FileFormat.fromString(newFormat);
       return this;
     }
 

diff --git a/core/src/main/java/org/apache/iceberg/FileMetadata.java b/core/src/main/java/org/apache/iceberg/FileMetadata.java
@@ -19,7 +19,6 @@
 package org.apache.iceberg;
 
 import java.nio.ByteBuffer;
-import java.util.Locale;
 import java.util.Map;
 import org.apache.hadoop.fs.FileStatus;
 import org.apache.iceberg.encryption.EncryptedOutputFile;
@@ -145,7 +144,7 @@ public Builder withPath(String newFilePath) {
     }
 
     public Builder withFormat(String newFormat) {
-      this.format = FileFormat.valueOf(newFormat.toUpperCase(Locale.ENGLISH));
+      this.format = FileFormat.fromString(newFormat);
       return this;
     }
 

diff --git a/core/src/main/java/org/apache/iceberg/io/OutputFileFactory.java b/core/src/main/java/org/apache/iceberg/io/OutputFileFactory.java
@@ -21,7 +21,6 @@
 import static org.apache.iceberg.TableProperties.DEFAULT_FILE_FORMAT;
 import static org.apache.iceberg.TableProperties.DEFAULT_FILE_FORMAT_DEFAULT;
 
-import java.util.Locale;
 import java.util.UUID;
 import java.util.concurrent.atomic.AtomicInteger;
 import org.apache.iceberg.FileFormat;
@@ -126,7 +125,7 @@ private Builder(Table table, int partitionId, long taskId) {
 
       String formatAsString =
           table.properties().getOrDefault(DEFAULT_FILE_FORMAT, DEFAULT_FILE_FORMAT_DEFAULT);
-      this.format = FileFormat.valueOf(formatAsString.toUpperCase(Locale.ROOT));
+      this.format = FileFormat.fromString(formatAsString);
     }
 
     public Builder defaultSpec(PartitionSpec newDefaultSpec) {

diff --git a/data/src/test/java/org/apache/iceberg/TestSplitScan.java b/data/src/test/java/org/apache/iceberg/TestSplitScan.java
@@ -23,7 +23,6 @@
 import java.io.File;
 import java.io.IOException;
 import java.util.List;
-import java.util.Locale;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.iceberg.data.GenericAppenderFactory;
 import org.apache.iceberg.data.IcebergGenerics;
@@ -66,7 +65,7 @@ public static Object[] parameters() {
   private final FileFormat format;
 
   public TestSplitScan(String format) {
-    this.format = FileFormat.valueOf(format.toUpperCase(Locale.ENGLISH));
+    this.format = FileFormat.fromString(format);
   }
 
   @Before

diff --git a/data/src/test/java/org/apache/iceberg/data/FileHelpers.java b/data/src/test/java/org/apache/iceberg/data/FileHelpers.java
@@ -24,7 +24,6 @@
 import java.io.Closeable;
 import java.io.IOException;
 import java.util.List;
-import java.util.Locale;
 import java.util.Map;
 import org.apache.iceberg.DataFile;
 import org.apache.iceberg.DataFiles;
@@ -146,6 +145,6 @@ private static EncryptedOutputFile encrypt(OutputFile out) {
 
   private static FileFormat defaultFormat(Map<String, String> properties) {
     String formatString = properties.getOrDefault(DEFAULT_FILE_FORMAT, DEFAULT_FILE_FORMAT_DEFAULT);
-    return FileFormat.valueOf(formatString.toUpperCase(Locale.ENGLISH));
+    return FileFormat.fromString(formatString);
   }
 }
diff --git a/data/src/test/java/org/apache/iceberg/data/TestLocalScan.java b/data/src/test/java/org/apache/iceberg/data/TestLocalScan.java
@@ -34,7 +34,6 @@
 import java.nio.ByteOrder;
 import java.util.Iterator;
 import java.util.List;
-import java.util.Locale;
 import java.util.Set;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.Path;
@@ -87,7 +86,7 @@ public static Object[] parameters() {
   private final FileFormat format;
 
   public TestLocalScan(String format) {
-    this.format = FileFormat.valueOf(format.toUpperCase(Locale.ENGLISH));
+    this.format = FileFormat.fromString(format);
   }
 
   private String sharedTableLocation = null;

diff --git a/data/src/test/java/org/apache/iceberg/data/TestMetricsRowGroupFilter.java b/data/src/test/java/org/apache/iceberg/data/TestMetricsRowGroupFilter.java
@@ -43,7 +43,6 @@
 import java.io.IOException;
 import java.io.UncheckedIOException;
 import java.util.List;
-import java.util.Locale;
 import java.util.UUID;
 import org.apache.avro.generic.GenericData.Record;
 import org.apache.avro.generic.GenericRecordBuilder;
@@ -98,7 +97,7 @@ public static Object[] parameters() {
   private final FileFormat format;
 
   public TestMetricsRowGroupFilter(String format) {
-    this.format = FileFormat.valueOf(format.toUpperCase(Locale.ENGLISH));
+    this.format = FileFormat.fromString(format);
   }
 
   private static final Types.StructType structFieldType =

diff --git a/data/src/test/java/org/apache/iceberg/data/TestMetricsRowGroupFilterTypes.java b/data/src/test/java/org/apache/iceberg/data/TestMetricsRowGroupFilterTypes.java
@@ -33,7 +33,6 @@
 import java.time.OffsetDateTime;
 import java.time.format.DateTimeFormatter;
 import java.util.List;
-import java.util.Locale;
 import java.util.UUID;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.Path;
@@ -277,7 +276,7 @@ public static Object[][] parameters() {
 
   public TestMetricsRowGroupFilterTypes(
       String format, String column, Object readValue, Object skipValue) {
-    this.format = FileFormat.valueOf(format.toUpperCase(Locale.ENGLISH));
+    this.format = FileFormat.fromString(format);
     this.column = column;
     this.readValue = readValue;
     this.skipValue = skipValue;

diff --git a/data/src/test/java/org/apache/iceberg/io/TestAppenderFactory.java b/data/src/test/java/org/apache/iceberg/io/TestAppenderFactory.java
@@ -21,7 +21,6 @@
 import java.io.File;
 import java.io.IOException;
 import java.util.List;
-import java.util.Locale;
 import java.util.Set;
 import org.apache.iceberg.DataFile;
 import org.apache.iceberg.FileFormat;
@@ -77,7 +76,7 @@ public static Object[] parameters() {
 
   public TestAppenderFactory(String fileFormat, boolean partitioned) {
     super(FORMAT_V2);
-    this.format = FileFormat.valueOf(fileFormat.toUpperCase(Locale.ENGLISH));
+    this.format = FileFormat.fromString(fileFormat);
     this.partitioned = partitioned;
   }
 

diff --git a/data/src/test/java/org/apache/iceberg/io/TestBaseTaskWriter.java b/data/src/test/java/org/apache/iceberg/io/TestBaseTaskWriter.java
@@ -25,7 +25,6 @@
 import java.nio.file.Paths;
 import java.util.Arrays;
 import java.util.List;
-import java.util.Locale;
 import java.util.stream.Collectors;
 import org.apache.iceberg.FileFormat;
 import org.apache.iceberg.PartitionSpec;
@@ -60,7 +59,7 @@ public static Object[][] parameters() {
 
   public TestBaseTaskWriter(String fileFormat) {
     super(FORMAT_V2);
-    this.format = FileFormat.valueOf(fileFormat.toUpperCase(Locale.ENGLISH));
+    this.format = FileFormat.fromString(fileFormat);
   }
 
   @Override

diff --git a/data/src/test/java/org/apache/iceberg/io/TestGenericSortedPosDeleteWriter.java b/data/src/test/java/org/apache/iceberg/io/TestGenericSortedPosDeleteWriter.java
@@ -21,7 +21,6 @@
 import java.io.File;
 import java.io.IOException;
 import java.util.List;
-import java.util.Locale;
 import org.apache.iceberg.DataFile;
 import org.apache.iceberg.DeleteFile;
 import org.apache.iceberg.FileFormat;
@@ -66,7 +65,7 @@ public static Object[] parameters() {
 
   public TestGenericSortedPosDeleteWriter(String fileFormat) {
     super(FORMAT_V2);
-    this.format = FileFormat.valueOf(fileFormat.toUpperCase(Locale.ENGLISH));
+    this.format = FileFormat.fromString(fileFormat);
   }
 
   @Override

diff --git a/data/src/test/java/org/apache/iceberg/io/TestTaskEqualityDeltaWriter.java b/data/src/test/java/org/apache/iceberg/io/TestTaskEqualityDeltaWriter.java
@@ -22,7 +22,6 @@
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.List;
-import java.util.Locale;
 import java.util.function.Function;
 import org.apache.iceberg.DataFile;
 import org.apache.iceberg.DeleteFile;
@@ -75,7 +74,7 @@ public static Object[][] parameters() {
 
   public TestTaskEqualityDeltaWriter(String fileFormat) {
     super(FORMAT_V2);
-    this.format = FileFormat.valueOf(fileFormat.toUpperCase(Locale.ENGLISH));
+    this.format = FileFormat.fromString(fileFormat);
   }
 
   @Override

diff --git a/flink/v1.13/flink/src/main/java/org/apache/iceberg/flink/FlinkWriteConf.java b/flink/v1.13/flink/src/main/java/org/apache/iceberg/flink/FlinkWriteConf.java
@@ -18,7 +18,6 @@
  */
 package org.apache.iceberg.flink;
 
-import java.util.Locale;
 import java.util.Map;
 import org.apache.flink.configuration.ReadableConfig;
 import org.apache.iceberg.DistributionMode;
@@ -81,7 +80,7 @@ public FileFormat dataFileFormat() {
             .tableProperty(TableProperties.DEFAULT_FILE_FORMAT)
             .defaultValue(TableProperties.DEFAULT_FILE_FORMAT_DEFAULT)
             .parse();
-    return FileFormat.valueOf(valueAsString.toUpperCase(Locale.ENGLISH));
+    return FileFormat.fromString(valueAsString);
   }
 
   public long targetDataFileSize() {

diff --git a/flink/v1.13/flink/src/main/java/org/apache/iceberg/flink/sink/FlinkFileWriterFactory.java b/flink/v1.13/flink/src/main/java/org/apache/iceberg/flink/sink/FlinkFileWriterFactory.java
@@ -24,7 +24,6 @@
 import static org.apache.iceberg.TableProperties.DELETE_DEFAULT_FILE_FORMAT;
 
 import java.io.Serializable;
-import java.util.Locale;
 import java.util.Map;
 import org.apache.flink.table.data.RowData;
 import org.apache.flink.table.data.StringData;
@@ -192,11 +191,11 @@ static class Builder {
 
       String dataFileFormatName =
           properties.getOrDefault(DEFAULT_FILE_FORMAT, DEFAULT_FILE_FORMAT_DEFAULT);
-      this.dataFileFormat = FileFormat.valueOf(dataFileFormatName.toUpperCase(Locale.ENGLISH));
+      this.dataFileFormat = FileFormat.fromString(dataFileFormatName);
 
       String deleteFileFormatName =
           properties.getOrDefault(DELETE_DEFAULT_FILE_FORMAT, dataFileFormatName);
-      this.deleteFileFormat = FileFormat.valueOf(deleteFileFormatName.toUpperCase(Locale.ENGLISH));
+      this.deleteFileFormat = FileFormat.fromString(deleteFileFormatName);
     }
 
     Builder dataFileFormat(FileFormat newDataFileFormat) {

diff --git a/flink/v1.13/flink/src/main/java/org/apache/iceberg/flink/source/RowDataRewriter.java b/flink/v1.13/flink/src/main/java/org/apache/iceberg/flink/source/RowDataRewriter.java
@@ -22,7 +22,6 @@
 
 import java.util.Collection;
 import java.util.List;
-import java.util.Locale;
 import java.util.stream.Collectors;
 import org.apache.flink.api.common.functions.RichMapFunction;
 import org.apache.flink.configuration.Configuration;
@@ -74,7 +73,7 @@ public RowDataRewriter(
             table.properties(),
             TableProperties.DEFAULT_FILE_FORMAT,
             TableProperties.DEFAULT_FILE_FORMAT_DEFAULT);
-    FileFormat format = FileFormat.valueOf(formatString.toUpperCase(Locale.ENGLISH));
+    FileFormat format = FileFormat.fromString(formatString);
     RowType flinkSchema = FlinkSchemaUtil.convert(table.schema());
     this.taskWriterFactory =
         new RowDataTaskWriterFactory(

diff --git a/flink/v1.13/flink/src/test/java/org/apache/iceberg/flink/sink/TestDeltaTaskWriter.java b/flink/v1.13/flink/src/test/java/org/apache/iceberg/flink/sink/TestDeltaTaskWriter.java
@@ -31,7 +31,6 @@
 import java.nio.file.Paths;
 import java.util.Arrays;
 import java.util.List;
-import java.util.Locale;
 import java.util.stream.Collectors;
 import org.apache.flink.table.data.RowData;
 import org.apache.iceberg.FileContent;
@@ -68,7 +67,7 @@ public static Object[][] parameters() {
 
   public TestDeltaTaskWriter(String fileFormat) {
     super(FORMAT_V2);
-    this.format = FileFormat.valueOf(fileFormat.toUpperCase(Locale.ENGLISH));
+    this.format = FileFormat.fromString(fileFormat);
   }
 
   @Override

diff --git a/flink/v1.13/flink/src/test/java/org/apache/iceberg/flink/sink/TestFlinkIcebergSink.java b/flink/v1.13/flink/src/test/java/org/apache/iceberg/flink/sink/TestFlinkIcebergSink.java
@@ -21,7 +21,6 @@
 import java.io.File;
 import java.io.IOException;
 import java.util.List;
-import java.util.Locale;
 import java.util.Map;
 import java.util.stream.Collectors;
 import org.apache.flink.api.common.typeinfo.TypeInformation;
@@ -97,7 +96,7 @@ public static Object[][] parameters() {
   }
 
   public TestFlinkIcebergSink(String format, int parallelism, boolean partitioned) {
-    this.format = FileFormat.valueOf(format.toUpperCase(Locale.ENGLISH));
+    this.format = FileFormat.fromString(format);
     this.parallelism = parallelism;
     this.partitioned = partitioned;
   }
@@ -378,7 +377,7 @@ public void testOverrideWriteConfigWithUnknownFileFormat() {
     AssertHelpers.assertThrows(
         "Should fail with invalid file format.",
         IllegalArgumentException.class,
-        "No enum constant org.apache.iceberg.FileFormat.UNRECOGNIZED",
+        "Invalid file format: UNRECOGNIZED",
         () -> {
           builder.append();
 

diff --git a/flink/v1.13/flink/src/test/java/org/apache/iceberg/flink/sink/TestFlinkIcebergSinkV2.java b/flink/v1.13/flink/src/test/java/org/apache/iceberg/flink/sink/TestFlinkIcebergSinkV2.java
@@ -21,7 +21,6 @@
 import java.io.File;
 import java.io.IOException;
 import java.util.List;
-import java.util.Locale;
 import java.util.Map;
 import org.apache.flink.api.common.typeinfo.TypeInformation;
 import org.apache.flink.api.java.functions.KeySelector;
@@ -112,7 +111,7 @@ public static Object[][] parameters() {
   public TestFlinkIcebergSinkV2(
       String format, int parallelism, boolean partitioned, String writeDistributionMode) {
     super(FORMAT_V2);
-    this.format = FileFormat.valueOf(format.toUpperCase(Locale.ENGLISH));
+    this.format = FileFormat.fromString(format);
     this.parallelism = parallelism;
     this.partitioned = partitioned;
     this.writeDistributionMode = writeDistributionMode;

diff --git a/flink/v1.13/flink/src/test/java/org/apache/iceberg/flink/sink/TestIcebergFilesCommitter.java b/flink/v1.13/flink/src/test/java/org/apache/iceberg/flink/sink/TestIcebergFilesCommitter.java
@@ -27,7 +27,6 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.List;
-import java.util.Locale;
 import java.util.stream.Collectors;
 import org.apache.flink.api.common.ExecutionConfig;
 import org.apache.flink.api.common.JobID;
@@ -90,7 +89,7 @@ public static Object[][] parameters() {
 
   public TestIcebergFilesCommitter(String format, int formatVersion) {
     super(formatVersion);
-    this.format = FileFormat.valueOf(format.toUpperCase(Locale.ENGLISH));
+    this.format = FileFormat.fromString(format);
   }
 
   @Override

diff --git a/flink/v1.13/flink/src/test/java/org/apache/iceberg/flink/sink/TestIcebergStreamWriter.java b/flink/v1.13/flink/src/test/java/org/apache/iceberg/flink/sink/TestIcebergStreamWriter.java
@@ -22,7 +22,6 @@
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.List;
-import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
 import org.apache.flink.streaming.api.operators.BoundedOneInput;
@@ -88,7 +87,7 @@ public static Object[][] parameters() {
   }
 
   public TestIcebergStreamWriter(String format, boolean partitioned) {
-    this.format = FileFormat.valueOf(format.toUpperCase(Locale.ENGLISH));
+    this.format = FileFormat.fromString(format);
     this.partitioned = partitioned;
   }