This is an automated email from the ASF dual-hosted git repository.
etudenhoefner pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg.git
The following commit(s) were added to refs/heads/main by this push:
new c469edf6cd Common: DynConstructors cleanup (#10542)
c469edf6cd is described below
commit c469edf6cd241e848d3926a07d978cc4b5994cc5
Author: Piotr Findeisen <[email protected]>
AuthorDate: Fri Jun 28 11:32:05 2024 +0200
Common: DynConstructors cleanup (#10542)
The `DynConstructors.Builder.hiddenImpl(Class<?>...)` is a varargs
method, but cannot be used as such, because `hiddenImpl(Class<T>,
Class<?>...)` exists and would be used instead. Since there are no
usages, deprecate it instead of fixing.
---
.../org/apache/iceberg/common/DynConstructors.java | 34 ++++++----
.../apache/iceberg/common/TestDynConstructors.java | 75 ++++++++++++++++++++++
2 files changed, 95 insertions(+), 14 deletions(-)
diff --git
a/common/src/main/java/org/apache/iceberg/common/DynConstructors.java
b/common/src/main/java/org/apache/iceberg/common/DynConstructors.java
index 4761be4f3e..7c77711287 100644
--- a/common/src/main/java/org/apache/iceberg/common/DynConstructors.java
+++ b/common/src/main/java/org/apache/iceberg/common/DynConstructors.java
@@ -43,6 +43,8 @@ public class DynConstructors {
this.constructed = constructed;
}
+ /** @deprecated since 1.6.0, will be removed in 1.7.0 */
+ @Deprecated
public Class<? extends C> getConstructedClass() {
return constructed;
}
@@ -57,9 +59,9 @@ public class DynConstructors {
} catch (InstantiationException | IllegalAccessException e) {
throw e;
} catch (InvocationTargetException e) {
- Throwables.propagateIfInstanceOf(e.getCause(), Exception.class);
- Throwables.propagateIfInstanceOf(e.getCause(), RuntimeException.class);
- throw Throwables.propagate(e.getCause());
+ Throwables.throwIfInstanceOf(e.getCause(), Exception.class);
+ Throwables.throwIfInstanceOf(e.getCause(), RuntimeException.class);
+ throw new RuntimeException(e.getCause());
}
}
@@ -67,8 +69,8 @@ public class DynConstructors {
try {
return newInstanceChecked(args);
} catch (Exception e) {
- Throwables.propagateIfInstanceOf(e, RuntimeException.class);
- throw Throwables.propagate(e);
+ Throwables.throwIfInstanceOf(e, RuntimeException.class);
+ throw new RuntimeException(e.getCause());
}
}
@@ -115,8 +117,8 @@ public class DynConstructors {
public static class Builder {
private final Class<?> baseClass;
private ClassLoader loader =
Thread.currentThread().getContextClassLoader();
- private Ctor ctor = null;
- private Map<String, Throwable> problems = Maps.newHashMap();
+ private Ctor<?> ctor = null;
+ private final Map<String, Throwable> problems = Maps.newHashMap();
public Builder(Class<?> baseClass) {
this.baseClass = baseClass;
@@ -162,7 +164,7 @@ public class DynConstructors {
}
try {
- ctor = new Ctor<T>(targetClass.getConstructor(types), targetClass);
+ ctor = new Ctor<>(targetClass.getConstructor(types), targetClass);
} catch (NoSuchMethodException e) {
// not the right implementation
problems.put(methodName(targetClass, types), e);
@@ -170,12 +172,16 @@ public class DynConstructors {
return this;
}
+ /**
+ * @deprecated since 1.6.0, will be removed in 1.7.0; This varargs method
conflicts with {@link
+ * #hiddenImpl(Class, Class...)}. Use {@link #builder(Class)} instead.
+ */
+ @Deprecated
public Builder hiddenImpl(Class<?>... types) {
hiddenImpl(baseClass, types);
return this;
}
- @SuppressWarnings("unchecked")
public Builder hiddenImpl(String className, Class<?>... types) {
// don't do any work if an implementation has been found
if (ctor != null) {
@@ -183,7 +189,7 @@ public class DynConstructors {
}
try {
- Class targetClass = Class.forName(className, true, loader);
+ Class<?> targetClass = Class.forName(className, true, loader);
hiddenImpl(targetClass, types);
} catch (NoClassDefFoundError | ClassNotFoundException e) {
// cannot load this implementation
@@ -201,7 +207,7 @@ public class DynConstructors {
try {
Constructor<T> hidden = targetClass.getDeclaredConstructor(types);
AccessController.doPrivileged(new MakeAccessible(hidden));
- ctor = new Ctor<T>(hidden, targetClass);
+ ctor = new Ctor<>(hidden, targetClass);
} catch (SecurityException e) {
// unusable
problems.put(methodName(targetClass, types), e);
@@ -215,7 +221,7 @@ public class DynConstructors {
@SuppressWarnings("unchecked")
public <C> Ctor<C> buildChecked() throws NoSuchMethodException {
if (ctor != null) {
- return ctor;
+ return (Ctor<C>) ctor;
}
throw buildCheckedException(baseClass, problems);
}
@@ -223,14 +229,14 @@ public class DynConstructors {
@SuppressWarnings("unchecked")
public <C> Ctor<C> build() {
if (ctor != null) {
- return ctor;
+ return (Ctor<C>) ctor;
}
throw buildRuntimeException(baseClass, problems);
}
}
private static class MakeAccessible implements PrivilegedAction<Void> {
- private Constructor<?> hidden;
+ private final Constructor<?> hidden;
MakeAccessible(Constructor<?> hidden) {
this.hidden = hidden;
diff --git
a/common/src/test/java/org/apache/iceberg/common/TestDynConstructors.java
b/common/src/test/java/org/apache/iceberg/common/TestDynConstructors.java
new file mode 100644
index 0000000000..baddcc8e2f
--- /dev/null
+++ b/common/src/test/java/org/apache/iceberg/common/TestDynConstructors.java
@@ -0,0 +1,75 @@
+/*
+ * 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.common;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import org.junit.jupiter.api.Test;
+
+public class TestDynConstructors {
+ @Test
+ public void testImplNewInstance() throws Exception {
+ DynConstructors.Ctor<MyClass> ctor =
+ DynConstructors.builder().impl(MyClass.class).buildChecked();
+ assertThat(ctor.newInstance()).isInstanceOf(MyClass.class);
+ }
+
+ @Test
+ public void testInterfaceImplNewInstance() throws Exception {
+ DynConstructors.Ctor<MyInterface> ctor =
+ DynConstructors.builder(MyInterface.class)
+ .impl("org.apache.iceberg.common.TestDynConstructors$MyClass")
+ .buildChecked();
+ assertThat(ctor.newInstance()).isInstanceOf(MyClass.class);
+ }
+
+ @Test
+ public void testInterfaceWrongImplString() throws Exception {
+ DynConstructors.Ctor<MyInterface> ctor =
+ DynConstructors.builder(MyInterface.class)
+ // TODO this should throw, since the MyUnrelatedClass does not
implement MyInterface
+
.impl("org.apache.iceberg.common.TestDynConstructors$MyUnrelatedClass")
+ .buildChecked();
+ assertThatThrownBy(ctor::newInstance)
+ .isInstanceOf(ClassCastException.class)
+ .hasMessage(
+ "org.apache.iceberg.common.TestDynConstructors$MyUnrelatedClass
cannot be cast to org.apache.iceberg.common.TestDynConstructors$MyInterface");
+ }
+
+ @Test
+ public void testInterfaceWrongImplClass() throws Exception {
+ DynConstructors.Ctor<MyInterface> ctor =
+ DynConstructors.builder(MyInterface.class)
+ // TODO this should throw or not compile at all, since the
MyUnrelatedClass does not
+ // implement MyInterface
+ .impl(MyUnrelatedClass.class)
+ .buildChecked();
+ assertThatThrownBy(ctor::newInstance)
+ .isInstanceOf(ClassCastException.class)
+ .hasMessage(
+ "org.apache.iceberg.common.TestDynConstructors$MyUnrelatedClass
cannot be cast to org.apache.iceberg.common.TestDynConstructors$MyInterface");
+ }
+
+ public interface MyInterface {}
+
+ public static class MyClass implements MyInterface {}
+
+ public static class MyUnrelatedClass {}
+}
