diff --git a/azure/src/test/java/org/apache/iceberg/azure/TestAdlsTokenCredentialProviders.java b/azure/src/test/java/org/apache/iceberg/azure/TestAdlsTokenCredentialProviders.java index 5060a9bc4455..80474463f11c 100644 --- a/azure/src/test/java/org/apache/iceberg/azure/TestAdlsTokenCredentialProviders.java +++ b/azure/src/test/java/org/apache/iceberg/azure/TestAdlsTokenCredentialProviders.java @@ -97,7 +97,8 @@ public void nonImplementingClassAsCredentialProvider() { ImmutableMap.of(AzureProperties.ADLS_TOKEN_CREDENTIAL_PROVIDER, "java.lang.String"); assertThatIllegalArgumentException() .isThrownBy(() -> AdlsTokenCredentialProviders.from(properties)) - .withMessageContaining("java.lang.String does not implement AdlsTokenCredentialProvider"); + .withMessage( + "Cannot use java.lang.String as an implementation of org.apache.iceberg.azure.AdlsTokenCredentialProvider"); } @Test 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 bf857a15ab89..06eceb45bdf8 100644 --- a/common/src/main/java/org/apache/iceberg/common/DynConstructors.java +++ b/common/src/main/java/org/apache/iceberg/common/DynConstructors.java @@ -157,6 +157,7 @@ public Builder impl(Class targetClass, Class... types) { return this; } + checkBaseClassAssignableFrom(targetClass); try { ctor = new Ctor<>(targetClass.getConstructor(types), targetClass); } catch (NoSuchMethodException e) { @@ -189,6 +190,7 @@ public Builder hiddenImpl(Class targetClass, Class... types) { return this; } + checkBaseClassAssignableFrom(targetClass); try { Constructor hidden = targetClass.getDeclaredConstructor(types); AccessController.doPrivileged(new MakeAccessible(hidden)); @@ -230,6 +232,16 @@ private Class classForName(String className) throws ClassNotFoundException { } } } + + private void checkBaseClassAssignableFrom(Class targetClass) { + if (baseClass != null) { + Preconditions.checkArgument( + baseClass.isAssignableFrom(targetClass), + "Cannot use %s as an implementation of %s", + targetClass.getName(), + baseClass.getName()); + } + } } private static class MakeAccessible implements PrivilegedAction { diff --git a/common/src/test/java/org/apache/iceberg/common/TestDynConstructors.java b/common/src/test/java/org/apache/iceberg/common/TestDynConstructors.java index 1edf70a7fbba..61e5e9d627e2 100644 --- a/common/src/test/java/org/apache/iceberg/common/TestDynConstructors.java +++ b/common/src/test/java/org/apache/iceberg/common/TestDynConstructors.java @@ -42,29 +42,38 @@ public void testInterfaceImplNewInstance() throws Exception { @Test public void testInterfaceWrongImplString() throws Exception { - DynConstructors.Ctor 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) + assertThatThrownBy( + () -> + DynConstructors.builder(MyInterface.class) + .impl("org.apache.iceberg.common.TestDynConstructors$MyUnrelatedClass") + .buildChecked()) + .isInstanceOf(IllegalArgumentException.class) .hasMessageStartingWith( - "class org.apache.iceberg.common.TestDynConstructors$MyUnrelatedClass cannot be cast to class org.apache.iceberg.common.TestDynConstructors$MyInterface"); + "Cannot use org.apache.iceberg.common.TestDynConstructors$MyUnrelatedClass as an implementation of org.apache.iceberg.common.TestDynConstructors$MyInterface"); } @Test public void testInterfaceWrongImplClass() throws Exception { - DynConstructors.Ctor 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) + assertThatThrownBy( + () -> + DynConstructors.builder(MyInterface.class) + .impl(MyUnrelatedClass.class) + .buildChecked()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith( + "Cannot use org.apache.iceberg.common.TestDynConstructors$MyUnrelatedClass as an implementation of org.apache.iceberg.common.TestDynConstructors$MyInterface"); + } + + @Test + public void testInterfaceWrongHiddenImplClass() throws Exception { + assertThatThrownBy( + () -> + DynConstructors.builder(MyInterface.class) + .hiddenImpl(MyUnrelatedClass.class) + .buildChecked()) + .isInstanceOf(IllegalArgumentException.class) .hasMessageStartingWith( - "class org.apache.iceberg.common.TestDynConstructors$MyUnrelatedClass cannot be cast to class org.apache.iceberg.common.TestDynConstructors$MyInterface"); + "Cannot use org.apache.iceberg.common.TestDynConstructors$MyUnrelatedClass as an implementation of org.apache.iceberg.common.TestDynConstructors$MyInterface"); } @Test diff --git a/core/src/main/java/org/apache/iceberg/CatalogUtil.java b/core/src/main/java/org/apache/iceberg/CatalogUtil.java index 2b400ccebc8b..863150d45d0a 100644 --- a/core/src/main/java/org/apache/iceberg/CatalogUtil.java +++ b/core/src/main/java/org/apache/iceberg/CatalogUtil.java @@ -407,14 +407,7 @@ public static FileIO loadFileIO( String.format("Cannot initialize FileIO implementation %s: %s", impl, e.getMessage()), e); } - FileIO fileIO; - try { - fileIO = ctor.newInstance(); - } catch (ClassCastException e) { - throw new IllegalArgumentException( - String.format("Cannot initialize FileIO, %s does not implement FileIO.", impl), e); - } - + FileIO fileIO = ctor.newInstance(); configureHadoopConf(fileIO, hadoopConf); if (fileIO instanceof SupportsStorageCredentials) { ((SupportsStorageCredentials) fileIO).setCredentials(storageCredentials); diff --git a/core/src/test/java/org/apache/iceberg/TestCatalogUtil.java b/core/src/test/java/org/apache/iceberg/TestCatalogUtil.java index 84e79e35c9b5..c13071985e9b 100644 --- a/core/src/test/java/org/apache/iceberg/TestCatalogUtil.java +++ b/core/src/test/java/org/apache/iceberg/TestCatalogUtil.java @@ -108,8 +108,8 @@ public void loadCustomCatalog_NotImplementCatalog() { CatalogUtil.loadCatalog( TestCatalogNoInterface.class.getName(), name, options, hadoopConf)) .isInstanceOf(IllegalArgumentException.class) - .hasMessageStartingWith("Cannot initialize Catalog") - .hasMessageContaining("does not implement Catalog"); + .hasMessage( + "Cannot use org.apache.iceberg.TestCatalogUtil$TestCatalogNoInterface as an implementation of org.apache.iceberg.catalog.Catalog"); } @Test @@ -214,8 +214,8 @@ public void loadCustomFileIO_badClass() { () -> CatalogUtil.loadFileIO(TestFileIONotImpl.class.getName(), Maps.newHashMap(), null)) .isInstanceOf(IllegalArgumentException.class) - .hasMessageStartingWith("Cannot initialize FileIO") - .hasMessageContaining("does not implement FileIO"); + .hasMessage( + "Cannot use org.apache.iceberg.TestCatalogUtil$TestFileIONotImpl as an implementation of org.apache.iceberg.io.FileIO"); } @Test @@ -263,7 +263,8 @@ public void loadCustomMetricsReporter_badClass() { CatalogProperties.METRICS_REPORTER_IMPL, TestFileIONotImpl.class.getName()))) .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("does not implement MetricsReporter"); + .hasMessage( + "Cannot use org.apache.iceberg.TestCatalogUtil$TestFileIONotImpl as an implementation of org.apache.iceberg.metrics.MetricsReporter"); } @Test diff --git a/core/src/test/java/org/apache/iceberg/TestLocationProvider.java b/core/src/test/java/org/apache/iceberg/TestLocationProvider.java index 146f2c8da5e7..6bfeaa75fb51 100644 --- a/core/src/test/java/org/apache/iceberg/TestLocationProvider.java +++ b/core/src/test/java/org/apache/iceberg/TestLocationProvider.java @@ -183,8 +183,7 @@ public void testInvalidNoInterfaceDynamicallyLoadedLocationProvider() { assertThatThrownBy(() -> table.locationProvider()) .isInstanceOf(IllegalArgumentException.class) .hasMessage( - "Provided implementation for dynamic instantiation should implement %s.", - LocationProvider.class); + "Cannot use org.apache.iceberg.TestLocationProvider$InvalidNoInterfaceDynamicallyLoadedLocationProvider as an implementation of org.apache.iceberg.io.LocationProvider"); } @TestTemplate diff --git a/core/src/test/java/org/apache/iceberg/rest/TestHTTPClient.java b/core/src/test/java/org/apache/iceberg/rest/TestHTTPClient.java index 8cf97bca32ef..dadd61112bfd 100644 --- a/core/src/test/java/org/apache/iceberg/rest/TestHTTPClient.java +++ b/core/src/test/java/org/apache/iceberg/rest/TestHTTPClient.java @@ -391,8 +391,8 @@ public void testLoadTLSConfigurerNotImplementTLSConfigurer() { ImmutableMap.of(HTTPClient.REST_TLS_CONFIGURER, Object.class.getName()); assertThatThrownBy(() -> HTTPClient.configureConnectionManager(properties)) .isInstanceOf(IllegalArgumentException.class) - .hasMessageStartingWith("Cannot initialize TLSConfigurer") - .hasMessageContaining("does not implement TLSConfigurer"); + .hasMessage( + "Cannot use java.lang.Object as an implementation of org.apache.iceberg.rest.auth.TLSConfigurer"); } @Test