From f1139d6d7adf27dbbc868f432f8447e709faba53 Mon Sep 17 00:00:00 2001 From: przemekd <1896041+przemekd@users.noreply.github.com> Date: Tue, 18 Mar 2025 13:07:48 +0100 Subject: [PATCH 1/7] Core: fix locationProvider() --- .../org/apache/iceberg/SerializableTable.java | 17 +- .../java/org/apache/iceberg/util/TryUtil.java | 121 ++++++++++++++ .../org/apache/iceberg/util/TestTryUtil.java | 147 ++++++++++++++++++ 3 files changed, 278 insertions(+), 7 deletions(-) create mode 100644 core/src/main/java/org/apache/iceberg/util/TryUtil.java create mode 100644 core/src/test/java/org/apache/iceberg/util/TestTryUtil.java diff --git a/core/src/main/java/org/apache/iceberg/SerializableTable.java b/core/src/main/java/org/apache/iceberg/SerializableTable.java index 57e2e1b730c1..eefb08fea390 100644 --- a/core/src/main/java/org/apache/iceberg/SerializableTable.java +++ b/core/src/main/java/org/apache/iceberg/SerializableTable.java @@ -30,6 +30,7 @@ import org.apache.iceberg.relocated.com.google.common.collect.Maps; import org.apache.iceberg.util.SerializableMap; import org.apache.iceberg.util.SerializableSupplier; +import org.apache.iceberg.util.TryUtil; /** * A read-only serializable table that can be sent to other nodes in a cluster. @@ -65,8 +66,8 @@ public class SerializableTable implements Table, HasTableOperations, Serializabl private final Map refs; private final UUID uuid; private final int formatVersion; + private final TryUtil.Try locationProviderTry; - private transient volatile LocationProvider lazyLocationProvider = null; private transient volatile Table lazyTable = null; private transient volatile Schema lazySchema = null; private transient volatile Map lazySpecs = null; @@ -85,6 +86,7 @@ protected SerializableTable(Table table) { this.sortOrderAsJson = SortOrderParser.toJson(table.sortOrder()); this.io = fileIO(table); this.encryption = table.encryption(); + this.locationProviderTry = TryUtil.run(() -> table.locationProvider()); this.refs = SerializableMap.copyOf(table.refs()); this.uuid = table.uuid(); this.formatVersion = formatVersion(table); @@ -265,14 +267,15 @@ public EncryptionManager encryption() { @Override public LocationProvider locationProvider() { - if (lazyLocationProvider == null) { - synchronized (this) { - if (lazyLocationProvider == null) { - this.lazyLocationProvider = LocationProviders.locationsFor(location, properties); - } + try { + return locationProviderTry.get(); + } catch (Exception e) { + if (e instanceof RuntimeException) { + throw (RuntimeException) e; + } else { + throw new RuntimeException(e); } } - return lazyLocationProvider; } @Override diff --git a/core/src/main/java/org/apache/iceberg/util/TryUtil.java b/core/src/main/java/org/apache/iceberg/util/TryUtil.java new file mode 100644 index 000000000000..f017313d2e00 --- /dev/null +++ b/core/src/main/java/org/apache/iceberg/util/TryUtil.java @@ -0,0 +1,121 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.util; + +import java.io.Serializable; +import org.apache.iceberg.relocated.com.google.common.base.Preconditions; + +/** + * Utility for executing code that may throw exceptions and deferring exception handling. + */ +public class TryUtil { + private TryUtil() {} + + /** + * Functional interface for operations that may throw exceptions. + * + * @param return type of the operation + */ + @FunctionalInterface + public interface ThrowingSupplier { + T get() throws Exception; + } + + /** + * Executes the given operation and returns a Try object containing either the result + * or the exception. + * + * @param operation the operation to execute + * @param the type of the result + * @return a Try object containing either the result or the exception + */ + public static Try run(ThrowingSupplier operation) { + try { + return Try.success(operation.get()); + } catch (Exception e) { + return Try.failure(e); + } + } + + /** + * Container for the result of an operation that might throw an exception. + * + * @param the type of the result + */ + public static class Try implements Serializable { + private static final long serialVersionUID = 1L; + + private final T value; + private final Exception exception; + + private Try(T value, Exception exception) { + this.value = value; + this.exception = exception; + } + + /** + * Creates a successful Try with the given value. + */ + public static Try success(T value) { + return new Try<>(value, null); + } + + /** + * Creates a failed Try with the given exception. + */ + public static Try failure(Exception exception) { + Preconditions.checkNotNull(exception, "Exception cannot be null"); + return new Try<>(null, exception); + } + + /** + * Checks if the operation was successful. + */ + public boolean isSuccess() { + return exception == null; + } + + /** + * Checks if the operation failed. + */ + public boolean isFailure() { + return exception != null; + } + + /** + * Gets the value if the operation was successful, or throws the original exception if it failed. + * + * @return the result value + * @throws Exception the original exception if the operation failed + */ + public T get() throws Exception { + if (exception != null) { + throw exception; + } + return value; + } + + /** + * Gets the value if the operation was successful, or returns the provided default value if it failed. + */ + public T orElse(T defaultValue) { + return isSuccess() ? value : defaultValue; + } + } +} \ No newline at end of file diff --git a/core/src/test/java/org/apache/iceberg/util/TestTryUtil.java b/core/src/test/java/org/apache/iceberg/util/TestTryUtil.java new file mode 100644 index 000000000000..2dc6b5499999 --- /dev/null +++ b/core/src/test/java/org/apache/iceberg/util/TestTryUtil.java @@ -0,0 +1,147 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.util; + +import org.assertj.core.api.Assertions; +import org.junit.Test; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.ObjectInputStream; +import java.io.ObjectOutputStream; + +public class TestTryUtil { + + @Test + public void testSuccessfulOperation() throws Exception { + TryUtil.Try result = TryUtil.run(() -> "success"); + + Assertions.assertThat(result.isSuccess()).isTrue(); + Assertions.assertThat(result.isFailure()).isFalse(); + Assertions.assertThat(result.get()).isEqualTo("success"); + Assertions.assertThat(result.orElse("default")).isEqualTo("success"); + } + + @Test + public void testFailedOperation() { + Exception testException = new IllegalArgumentException("test exception"); + TryUtil.Try result = TryUtil.run(() -> { + throw testException; + }); + + Assertions.assertThat(result.isSuccess()).isFalse(); + Assertions.assertThat(result.isFailure()).isTrue(); + Assertions.assertThat(result.orElse("default")).isEqualTo("default"); + } + + @Test + public void testGetWithFailure() { + Exception testException = new IllegalArgumentException("test exception"); + TryUtil.Try result = TryUtil.run(() -> { + throw testException; + }); + + Assertions.assertThatThrownBy(result::get) + .isSameAs(testException); + } + + @Test + public void testCheckedException() { + TryUtil.Try result = TryUtil.run(() -> { + throw new Exception("checked exception"); + }); + + Assertions.assertThat(result.isFailure()).isTrue(); + Assertions.assertThatThrownBy(result::get) + .isInstanceOf(Exception.class) + .hasMessage("checked exception"); + } + + @Test + public void testRuntimeException() { + RuntimeException runtimeException = new RuntimeException("runtime exception"); + TryUtil.Try result = TryUtil.run(() -> { + throw runtimeException; + }); + + Assertions.assertThat(result.isFailure()).isTrue(); + Assertions.assertThatThrownBy(result::get) + .isSameAs(runtimeException); + } + + @Test + public void testNullValue() throws Exception { + TryUtil.Try result = TryUtil.run(() -> null); + + Assertions.assertThat(result.isSuccess()).isTrue(); + Assertions.assertThat(result.get()).isNull(); + Assertions.assertThat(result.orElse("default")).isNull(); + } + + @Test + public void testOrElseWithDefaultValue() { + TryUtil.Try result = TryUtil.run(() -> { + throw new RuntimeException("test exception"); + }); + + Assertions.assertThat(result.orElse("default value")).isEqualTo("default value"); + } + + @Test + public void testSerialization() throws Exception { + TryUtil.Try original = TryUtil.run(() -> "serialized value"); + + // Serialize to byte array + ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + ObjectOutputStream out = new ObjectOutputStream(bytes); + out.writeObject(original); + out.close(); + + // Deserialize from byte array + ObjectInputStream in = new ObjectInputStream(new ByteArrayInputStream(bytes.toByteArray())); + TryUtil.Try deserialized = (TryUtil.Try) in.readObject(); + in.close(); + + // Verify state is preserved + Assertions.assertThat(deserialized.isSuccess()).isTrue(); + Assertions.assertThat(deserialized.get()).isEqualTo("serialized value"); + } + + @Test + public void testSerializationWithException() throws Exception { + Exception original_exception = new IllegalArgumentException("test exception"); + TryUtil.Try original = TryUtil.run(() -> { throw original_exception; }); + // Serialize to byte array + ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + ObjectOutputStream out = new ObjectOutputStream(bytes); + out.writeObject(original); + out.close(); + + // Deserialize from byte array + ObjectInputStream in = new ObjectInputStream(new ByteArrayInputStream(bytes.toByteArray())); + TryUtil.Try deserialized = (TryUtil.Try) in.readObject(); + in.close(); + + // Verify state is preserved + Assertions.assertThat(deserialized.isFailure()).isTrue(); + Assertions.assertThatThrownBy(deserialized::get) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("test exception"); + } +} \ No newline at end of file From 3c534614fdc1ef06f0d872e4a5fd4985de6459d2 Mon Sep 17 00:00:00 2001 From: przemekd <1896041+przemekd@users.noreply.github.com> Date: Tue, 18 Mar 2025 13:12:43 +0100 Subject: [PATCH 2/7] Add info about LocationProvider that needs to be serializable --- .../main/java/org/apache/iceberg/SerializableTable.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/apache/iceberg/SerializableTable.java b/core/src/main/java/org/apache/iceberg/SerializableTable.java index eefb08fea390..2da9c5971005 100644 --- a/core/src/main/java/org/apache/iceberg/SerializableTable.java +++ b/core/src/main/java/org/apache/iceberg/SerializableTable.java @@ -42,10 +42,10 @@ * table metadata, it directly persists the current schema, spec, sort order, table properties to * avoid reading the metadata file from other nodes for frequently needed metadata. * - *

The implementation assumes the passed instances of {@link FileIO}, {@link EncryptionManager} - * are serializable. If you are serializing the table using a custom serialization framework like - * Kryo, those instances of {@link FileIO}, {@link EncryptionManager} must be supported by that - * particular serialization framework. + *

The implementation assumes the passed instances of {@link FileIO}, {@link EncryptionManager}, + * {@link LocationProvider} are serializable. If you are serializing the table using a custom + * serialization framework like Kryo, those instances of {@link FileIO}, {@link EncryptionManager}, + * {@link LocationProvider} must be supported by that particular serialization framework. * *

Note: loading the complete metadata from a large number of nodes can overwhelm the * storage. From 941399014c1fec63dbbc613089d14a1cc5d03ae5 Mon Sep 17 00:00:00 2001 From: przemekd <1896041+przemekd@users.noreply.github.com> Date: Tue, 18 Mar 2025 13:20:09 +0100 Subject: [PATCH 3/7] Fix style --- core/src/main/java/org/apache/iceberg/SerializableTable.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/iceberg/SerializableTable.java b/core/src/main/java/org/apache/iceberg/SerializableTable.java index 2da9c5971005..9143b44e49d5 100644 --- a/core/src/main/java/org/apache/iceberg/SerializableTable.java +++ b/core/src/main/java/org/apache/iceberg/SerializableTable.java @@ -268,7 +268,7 @@ public EncryptionManager encryption() { @Override public LocationProvider locationProvider() { try { - return locationProviderTry.get(); + return this.locationProviderTry.get(); } catch (Exception e) { if (e instanceof RuntimeException) { throw (RuntimeException) e; From 99ee19bdb5c8e969e2e682d5a97bbf4a5b57e8cf Mon Sep 17 00:00:00 2001 From: przemekd <1896041+przemekd@users.noreply.github.com> Date: Tue, 18 Mar 2025 14:28:54 +0100 Subject: [PATCH 4/7] Fix style --- .../java/org/apache/iceberg/util/TryUtil.java | 32 ++++------ .../org/apache/iceberg/util/TestTryUtil.java | 59 +++++++++++-------- 2 files changed, 47 insertions(+), 44 deletions(-) diff --git a/core/src/main/java/org/apache/iceberg/util/TryUtil.java b/core/src/main/java/org/apache/iceberg/util/TryUtil.java index f017313d2e00..e117f9300d2b 100644 --- a/core/src/main/java/org/apache/iceberg/util/TryUtil.java +++ b/core/src/main/java/org/apache/iceberg/util/TryUtil.java @@ -21,9 +21,7 @@ import java.io.Serializable; import org.apache.iceberg.relocated.com.google.common.base.Preconditions; -/** - * Utility for executing code that may throw exceptions and deferring exception handling. - */ +/** Utility for executing code that may throw exceptions and deferring exception handling. */ public class TryUtil { private TryUtil() {} @@ -38,8 +36,8 @@ public interface ThrowingSupplier { } /** - * Executes the given operation and returns a Try object containing either the result - * or the exception. + * Executes the given operation and returns a Try object containing either the result or the + * exception. * * @param operation the operation to execute * @param the type of the result @@ -69,37 +67,30 @@ private Try(T value, Exception exception) { this.exception = exception; } - /** - * Creates a successful Try with the given value. - */ + /** Creates a successful Try with the given value. */ public static Try success(T value) { return new Try<>(value, null); } - /** - * Creates a failed Try with the given exception. - */ + /** Creates a failed Try with the given exception. */ public static Try failure(Exception exception) { Preconditions.checkNotNull(exception, "Exception cannot be null"); return new Try<>(null, exception); } - /** - * Checks if the operation was successful. - */ + /** Checks if the operation was successful. */ public boolean isSuccess() { return exception == null; } - /** - * Checks if the operation failed. - */ + /** Checks if the operation failed. */ public boolean isFailure() { return exception != null; } /** - * Gets the value if the operation was successful, or throws the original exception if it failed. + * Gets the value if the operation was successful, or throws the original exception if it + * failed. * * @return the result value * @throws Exception the original exception if the operation failed @@ -112,10 +103,11 @@ public T get() throws Exception { } /** - * Gets the value if the operation was successful, or returns the provided default value if it failed. + * Gets the value if the operation was successful, or returns the provided default value if it + * failed. */ public T orElse(T defaultValue) { return isSuccess() ? value : defaultValue; } } -} \ No newline at end of file +} diff --git a/core/src/test/java/org/apache/iceberg/util/TestTryUtil.java b/core/src/test/java/org/apache/iceberg/util/TestTryUtil.java index 2dc6b5499999..d2a362d7b7f7 100644 --- a/core/src/test/java/org/apache/iceberg/util/TestTryUtil.java +++ b/core/src/test/java/org/apache/iceberg/util/TestTryUtil.java @@ -18,13 +18,12 @@ */ package org.apache.iceberg.util; -import org.assertj.core.api.Assertions; -import org.junit.Test; - import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.ObjectInputStream; import java.io.ObjectOutputStream; +import org.assertj.core.api.Assertions; +import org.junit.Test; public class TestTryUtil { @@ -41,9 +40,11 @@ public void testSuccessfulOperation() throws Exception { @Test public void testFailedOperation() { Exception testException = new IllegalArgumentException("test exception"); - TryUtil.Try result = TryUtil.run(() -> { - throw testException; - }); + TryUtil.Try result = + TryUtil.run( + () -> { + throw testException; + }); Assertions.assertThat(result.isSuccess()).isFalse(); Assertions.assertThat(result.isFailure()).isTrue(); @@ -53,19 +54,22 @@ public void testFailedOperation() { @Test public void testGetWithFailure() { Exception testException = new IllegalArgumentException("test exception"); - TryUtil.Try result = TryUtil.run(() -> { - throw testException; - }); + TryUtil.Try result = + TryUtil.run( + () -> { + throw testException; + }); - Assertions.assertThatThrownBy(result::get) - .isSameAs(testException); + Assertions.assertThatThrownBy(result::get).isSameAs(testException); } @Test public void testCheckedException() { - TryUtil.Try result = TryUtil.run(() -> { - throw new Exception("checked exception"); - }); + TryUtil.Try result = + TryUtil.run( + () -> { + throw new Exception("checked exception"); + }); Assertions.assertThat(result.isFailure()).isTrue(); Assertions.assertThatThrownBy(result::get) @@ -76,13 +80,14 @@ public void testCheckedException() { @Test public void testRuntimeException() { RuntimeException runtimeException = new RuntimeException("runtime exception"); - TryUtil.Try result = TryUtil.run(() -> { - throw runtimeException; - }); + TryUtil.Try result = + TryUtil.run( + () -> { + throw runtimeException; + }); Assertions.assertThat(result.isFailure()).isTrue(); - Assertions.assertThatThrownBy(result::get) - .isSameAs(runtimeException); + Assertions.assertThatThrownBy(result::get).isSameAs(runtimeException); } @Test @@ -96,9 +101,11 @@ public void testNullValue() throws Exception { @Test public void testOrElseWithDefaultValue() { - TryUtil.Try result = TryUtil.run(() -> { - throw new RuntimeException("test exception"); - }); + TryUtil.Try result = + TryUtil.run( + () -> { + throw new RuntimeException("test exception"); + }); Assertions.assertThat(result.orElse("default value")).isEqualTo("default value"); } @@ -126,7 +133,11 @@ public void testSerialization() throws Exception { @Test public void testSerializationWithException() throws Exception { Exception original_exception = new IllegalArgumentException("test exception"); - TryUtil.Try original = TryUtil.run(() -> { throw original_exception; }); + TryUtil.Try original = + TryUtil.run( + () -> { + throw original_exception; + }); // Serialize to byte array ByteArrayOutputStream bytes = new ByteArrayOutputStream(); ObjectOutputStream out = new ObjectOutputStream(bytes); @@ -144,4 +155,4 @@ public void testSerializationWithException() throws Exception { .isInstanceOf(IllegalArgumentException.class) .hasMessage("test exception"); } -} \ No newline at end of file +} From 01ee2e38804aa4260416570250f8a01b1ec513d5 Mon Sep 17 00:00:00 2001 From: przemekd <1896041+przemekd@users.noreply.github.com> Date: Tue, 18 Mar 2025 14:45:55 +0100 Subject: [PATCH 5/7] Fix style --- core/src/main/java/org/apache/iceberg/SerializableTable.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/iceberg/SerializableTable.java b/core/src/main/java/org/apache/iceberg/SerializableTable.java index 9143b44e49d5..c65e1ee74813 100644 --- a/core/src/main/java/org/apache/iceberg/SerializableTable.java +++ b/core/src/main/java/org/apache/iceberg/SerializableTable.java @@ -86,7 +86,7 @@ protected SerializableTable(Table table) { this.sortOrderAsJson = SortOrderParser.toJson(table.sortOrder()); this.io = fileIO(table); this.encryption = table.encryption(); - this.locationProviderTry = TryUtil.run(() -> table.locationProvider()); + this.locationProviderTry = TryUtil.run(table::locationProvider); this.refs = SerializableMap.copyOf(table.refs()); this.uuid = table.uuid(); this.formatVersion = formatVersion(table); From 3c0507e4e39bab1a8c3a0f077bc2f927228adc0c Mon Sep 17 00:00:00 2001 From: przemekd <1896041+przemekd@users.noreply.github.com> Date: Tue, 18 Mar 2025 16:17:40 +0100 Subject: [PATCH 6/7] Fix style --- .../org/apache/iceberg/util/TestTryUtil.java | 48 ++++++++++--------- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/core/src/test/java/org/apache/iceberg/util/TestTryUtil.java b/core/src/test/java/org/apache/iceberg/util/TestTryUtil.java index d2a362d7b7f7..cd66e87d9d06 100644 --- a/core/src/test/java/org/apache/iceberg/util/TestTryUtil.java +++ b/core/src/test/java/org/apache/iceberg/util/TestTryUtil.java @@ -18,11 +18,13 @@ */ package org.apache.iceberg.util; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.ObjectInputStream; import java.io.ObjectOutputStream; -import org.assertj.core.api.Assertions; import org.junit.Test; public class TestTryUtil { @@ -31,10 +33,10 @@ public class TestTryUtil { public void testSuccessfulOperation() throws Exception { TryUtil.Try result = TryUtil.run(() -> "success"); - Assertions.assertThat(result.isSuccess()).isTrue(); - Assertions.assertThat(result.isFailure()).isFalse(); - Assertions.assertThat(result.get()).isEqualTo("success"); - Assertions.assertThat(result.orElse("default")).isEqualTo("success"); + assertThat(result.isSuccess()).isTrue(); + assertThat(result.isFailure()).isFalse(); + assertThat(result.get()).isEqualTo("success"); + assertThat(result.orElse("default")).isEqualTo("success"); } @Test @@ -46,9 +48,9 @@ public void testFailedOperation() { throw testException; }); - Assertions.assertThat(result.isSuccess()).isFalse(); - Assertions.assertThat(result.isFailure()).isTrue(); - Assertions.assertThat(result.orElse("default")).isEqualTo("default"); + assertThat(result.isSuccess()).isFalse(); + assertThat(result.isFailure()).isTrue(); + assertThat(result.orElse("default")).isEqualTo("default"); } @Test @@ -60,7 +62,7 @@ public void testGetWithFailure() { throw testException; }); - Assertions.assertThatThrownBy(result::get).isSameAs(testException); + assertThatThrownBy(result::get).isSameAs(testException); } @Test @@ -71,8 +73,8 @@ public void testCheckedException() { throw new Exception("checked exception"); }); - Assertions.assertThat(result.isFailure()).isTrue(); - Assertions.assertThatThrownBy(result::get) + assertThat(result.isFailure()).isTrue(); + assertThatThrownBy(result::get) .isInstanceOf(Exception.class) .hasMessage("checked exception"); } @@ -86,17 +88,17 @@ public void testRuntimeException() { throw runtimeException; }); - Assertions.assertThat(result.isFailure()).isTrue(); - Assertions.assertThatThrownBy(result::get).isSameAs(runtimeException); + assertThat(result.isFailure()).isTrue(); + assertThatThrownBy(result::get).isSameAs(runtimeException); } @Test public void testNullValue() throws Exception { TryUtil.Try result = TryUtil.run(() -> null); - Assertions.assertThat(result.isSuccess()).isTrue(); - Assertions.assertThat(result.get()).isNull(); - Assertions.assertThat(result.orElse("default")).isNull(); + assertThat(result.isSuccess()).isTrue(); + assertThat(result.get()).isNull(); + assertThat(result.orElse("default")).isNull(); } @Test @@ -107,7 +109,7 @@ public void testOrElseWithDefaultValue() { throw new RuntimeException("test exception"); }); - Assertions.assertThat(result.orElse("default value")).isEqualTo("default value"); + assertThat(result.orElse("default value")).isEqualTo("default value"); } @Test @@ -126,17 +128,17 @@ public void testSerialization() throws Exception { in.close(); // Verify state is preserved - Assertions.assertThat(deserialized.isSuccess()).isTrue(); - Assertions.assertThat(deserialized.get()).isEqualTo("serialized value"); + assertThat(deserialized.isSuccess()).isTrue(); + assertThat(deserialized.get()).isEqualTo("serialized value"); } @Test public void testSerializationWithException() throws Exception { - Exception original_exception = new IllegalArgumentException("test exception"); + Exception originalException = new IllegalArgumentException("test exception"); TryUtil.Try original = TryUtil.run( () -> { - throw original_exception; + throw originalException; }); // Serialize to byte array ByteArrayOutputStream bytes = new ByteArrayOutputStream(); @@ -150,8 +152,8 @@ public void testSerializationWithException() throws Exception { in.close(); // Verify state is preserved - Assertions.assertThat(deserialized.isFailure()).isTrue(); - Assertions.assertThatThrownBy(deserialized::get) + assertThat(deserialized.isFailure()).isTrue(); + assertThatThrownBy(deserialized::get) .isInstanceOf(IllegalArgumentException.class) .hasMessage("test exception"); } From d0f51c01b8188678b7725231ef6fa832849fc617 Mon Sep 17 00:00:00 2001 From: przemekd <1896041+przemekd@users.noreply.github.com> Date: Tue, 18 Mar 2025 21:55:58 +0100 Subject: [PATCH 7/7] Fix style --- core/src/test/java/org/apache/iceberg/util/TestTryUtil.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/core/src/test/java/org/apache/iceberg/util/TestTryUtil.java b/core/src/test/java/org/apache/iceberg/util/TestTryUtil.java index cd66e87d9d06..56c702e6ac52 100644 --- a/core/src/test/java/org/apache/iceberg/util/TestTryUtil.java +++ b/core/src/test/java/org/apache/iceberg/util/TestTryUtil.java @@ -74,9 +74,7 @@ public void testCheckedException() { }); assertThat(result.isFailure()).isTrue(); - assertThatThrownBy(result::get) - .isInstanceOf(Exception.class) - .hasMessage("checked exception"); + assertThatThrownBy(result::get).isInstanceOf(Exception.class).hasMessage("checked exception"); } @Test