diff --git a/aliyun/src/main/java/org/apache/iceberg/aliyun/AliyunClientFactories.java b/aliyun/src/main/java/org/apache/iceberg/aliyun/AliyunClientFactories.java index d66a6783e2a2..c512d718da75 100644 --- a/aliyun/src/main/java/org/apache/iceberg/aliyun/AliyunClientFactories.java +++ b/aliyun/src/main/java/org/apache/iceberg/aliyun/AliyunClientFactories.java @@ -24,6 +24,7 @@ import java.util.Map; import org.apache.iceberg.common.DynConstructors; import org.apache.iceberg.relocated.com.google.common.base.Preconditions; +import org.apache.iceberg.util.PropertyUtil; public class AliyunClientFactories { @@ -36,12 +37,10 @@ public static AliyunClientFactory defaultFactory() { return ALIYUN_CLIENT_FACTORY_DEFAULT; } - public static AliyunClientFactory load(Map properties) { - if (properties.containsKey(AliyunProperties.CLIENT_FACTORY)) { - return load(properties.get(AliyunProperties.CLIENT_FACTORY), properties); - } else { - return defaultFactory(); - } + public static AliyunClientFactory from(Map properties) { + String factoryImpl = PropertyUtil.propertyAsString( + properties, AliyunProperties.CLIENT_FACTORY, DefaultAliyunClientFactory.class.getName()); + return loadClientFactory(factoryImpl, properties); } /** @@ -51,10 +50,10 @@ public static AliyunClientFactory load(Map properties) { * @param properties to initialize the factory. * @return an initialized {@link AliyunClientFactory}. */ - private static AliyunClientFactory load(String impl, Map properties) { + private static AliyunClientFactory loadClientFactory(String impl, Map properties) { DynConstructors.Ctor ctor; try { - ctor = DynConstructors.builder(AliyunClientFactory.class).impl(impl).buildChecked(); + ctor = DynConstructors.builder(AliyunClientFactory.class).hiddenImpl(impl).buildChecked(); } catch (NoSuchMethodException e) { throw new IllegalArgumentException(String.format( "Cannot initialize AliyunClientFactory, missing no-arg constructor: %s", impl), e); diff --git a/aliyun/src/main/java/org/apache/iceberg/aliyun/AliyunProperties.java b/aliyun/src/main/java/org/apache/iceberg/aliyun/AliyunProperties.java index 5a04f2a47ce3..7474e6a3b5a5 100644 --- a/aliyun/src/main/java/org/apache/iceberg/aliyun/AliyunProperties.java +++ b/aliyun/src/main/java/org/apache/iceberg/aliyun/AliyunProperties.java @@ -40,17 +40,17 @@ public class AliyunProperties implements Serializable { * For more information about how to obtain an AccessKey pair, see: * https://www.alibabacloud.com/help/doc-detail/53045.htm */ - public static final String ACCESS_KEY_ID = "access.key.id"; + public static final String CLIENT_ACCESS_KEY_ID = "client.access-key-id"; /** * Aliyun uses an AccessKey pair, which includes an AccessKey ID and an AccessKey secret to implement symmetric - * encryption and verify the identity of a requester. The AccessKey secret is used to encrypt and verify the + * encryption and verify the identity of a requester. The AccessKey secret is used to encrypt and verify the * signature string. *

* For more information about how to obtain an AccessKey pair, see: * https://www.alibabacloud.com/help/doc-detail/53045.htm */ - public static final String ACCESS_KEY_SECRET = "access.key.secret"; + public static final String CLIENT_ACCESS_KEY_SECRET = "client.access-key-secret"; /** * The implementation class of {@link AliyunClientFactory} to customize Aliyun client configurations. @@ -76,8 +76,8 @@ public AliyunProperties() { public AliyunProperties(Map properties) { // OSS endpoint, accessKeyId, accessKeySecret. this.ossEndpoint = properties.get(OSS_ENDPOINT); - this.accessKeyId = properties.get(ACCESS_KEY_ID); - this.accessKeySecret = properties.get(ACCESS_KEY_SECRET); + this.accessKeyId = properties.get(CLIENT_ACCESS_KEY_ID); + this.accessKeySecret = properties.get(CLIENT_ACCESS_KEY_SECRET); this.ossStagingDirectory = PropertyUtil.propertyAsString(properties, OSS_STAGING_DIRECTORY, System.getProperty("java.io.tmpdir")); diff --git a/aliyun/src/main/java/org/apache/iceberg/aliyun/oss/OSSFileIO.java b/aliyun/src/main/java/org/apache/iceberg/aliyun/oss/OSSFileIO.java index e45944c6e95c..00387617ac62 100644 --- a/aliyun/src/main/java/org/apache/iceberg/aliyun/oss/OSSFileIO.java +++ b/aliyun/src/main/java/org/apache/iceberg/aliyun/oss/OSSFileIO.java @@ -89,7 +89,7 @@ private OSS client() { @Override public void initialize(Map properties) { - AliyunClientFactory factory = AliyunClientFactories.load(properties); + AliyunClientFactory factory = AliyunClientFactories.from(properties); this.aliyunProperties = factory.aliyunProperties(); this.oss = factory::newOSSClient; } diff --git a/aliyun/src/test/java/org/apache/iceberg/aliyun/TestAliyunClientFactories.java b/aliyun/src/test/java/org/apache/iceberg/aliyun/TestAliyunClientFactories.java index 675ccb6090df..d5518a1ba870 100644 --- a/aliyun/src/test/java/org/apache/iceberg/aliyun/TestAliyunClientFactories.java +++ b/aliyun/src/test/java/org/apache/iceberg/aliyun/TestAliyunClientFactories.java @@ -21,6 +21,7 @@ import com.aliyun.oss.OSS; import java.util.Map; +import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.relocated.com.google.common.collect.Maps; import org.junit.Assert; import org.junit.Test; @@ -32,9 +33,20 @@ public void testLoadDefault() { Assert.assertEquals("Default client should be singleton", AliyunClientFactories.defaultFactory(), AliyunClientFactories.defaultFactory()); + AliyunClientFactory defaultFactory = AliyunClientFactories.from(Maps.newHashMap()); Assert.assertTrue( - "Should load default when not configured", - AliyunClientFactories.load(Maps.newHashMap()) instanceof AliyunClientFactories.DefaultAliyunClientFactory); + "Should load default when factory impl not configured", + defaultFactory instanceof AliyunClientFactories.DefaultAliyunClientFactory); + Assert.assertNull("Should have no Aliyun properties set", + defaultFactory.aliyunProperties().accessKeyId()); + + AliyunClientFactory defaultFactoryWithConfig = AliyunClientFactories.from( + ImmutableMap.of(AliyunProperties.CLIENT_ACCESS_KEY_ID, "key")); + Assert.assertTrue( + "Should load default when factory impl not configured", + defaultFactoryWithConfig instanceof AliyunClientFactories.DefaultAliyunClientFactory); + Assert.assertEquals("Should have access key set", + "key", defaultFactoryWithConfig.aliyunProperties().accessKeyId()); } @Test @@ -43,7 +55,7 @@ public void testLoadCustom() { properties.put(AliyunProperties.CLIENT_FACTORY, CustomFactory.class.getName()); Assert.assertTrue( "Should load custom class", - AliyunClientFactories.load(properties) instanceof CustomFactory); + AliyunClientFactories.from(properties) instanceof CustomFactory); } public static class CustomFactory implements AliyunClientFactory {