From 168858c1b0ec5163cc11c569e8f1ee60f2a44c1f Mon Sep 17 00:00:00 2001 From: Jack Ye Date: Thu, 2 Dec 2021 22:46:28 -0800 Subject: [PATCH 1/2] Aliyun: align property names with AWS module --- .../iceberg/aliyun/AliyunClientFactories.java | 17 ++++++++--------- .../iceberg/aliyun/AliyunProperties.java | 14 +++++++------- .../apache/iceberg/aliyun/oss/OSSFileIO.java | 2 +- .../aliyun/TestAliyunClientFactories.java | 18 +++++++++++++++--- 4 files changed, 31 insertions(+), 20 deletions(-) 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..9f53fac8ddd7 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); @@ -84,7 +83,7 @@ public OSS newOSSClient() { aliyunProperties, "Cannot create aliyun oss client before initializing the AliyunClientFactory."); return new OSSClientBuilder().build( - aliyunProperties.ossEndpoint(), aliyunProperties.accessKeyId(), aliyunProperties.accessKeySecret()); + aliyunProperties.ossEndpoint(), aliyunProperties.accessKeyId(), aliyunProperties.secretAccessKey()); } @Override 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..48f38df2ce21 100644 --- a/aliyun/src/main/java/org/apache/iceberg/aliyun/AliyunProperties.java +++ b/aliyun/src/main/java/org/apache/iceberg/aliyun/AliyunProperties.java @@ -40,7 +40,7 @@ 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 @@ -50,7 +50,7 @@ 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_SECRET = "access.key.secret"; + public static final String CLIENT_SECRET_ACCESS_KEY = "client.secret-access-key"; /** * The implementation class of {@link AliyunClientFactory} to customize Aliyun client configurations. @@ -66,7 +66,7 @@ public class AliyunProperties implements Serializable { private final String ossEndpoint; private final String accessKeyId; - private final String accessKeySecret; + private final String secretAccessKey; private final String ossStagingDirectory; public AliyunProperties() { @@ -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.secretAccessKey = properties.get(CLIENT_SECRET_ACCESS_KEY); this.ossStagingDirectory = PropertyUtil.propertyAsString(properties, OSS_STAGING_DIRECTORY, System.getProperty("java.io.tmpdir")); @@ -91,8 +91,8 @@ public String accessKeyId() { return accessKeyId; } - public String accessKeySecret() { - return accessKeySecret; + public String secretAccessKey() { + return secretAccessKey; } public String ossStagingDirectory() { 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 { From 58f8bd1273070fbd46fee6605c20c8151f2be440 Mon Sep 17 00:00:00 2001 From: Jack Ye Date: Mon, 6 Dec 2021 15:05:21 -0800 Subject: [PATCH 2/2] rename secret-access-key to access-key-secret --- .../apache/iceberg/aliyun/AliyunClientFactories.java | 2 +- .../org/apache/iceberg/aliyun/AliyunProperties.java | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) 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 9f53fac8ddd7..c512d718da75 100644 --- a/aliyun/src/main/java/org/apache/iceberg/aliyun/AliyunClientFactories.java +++ b/aliyun/src/main/java/org/apache/iceberg/aliyun/AliyunClientFactories.java @@ -83,7 +83,7 @@ public OSS newOSSClient() { aliyunProperties, "Cannot create aliyun oss client before initializing the AliyunClientFactory."); return new OSSClientBuilder().build( - aliyunProperties.ossEndpoint(), aliyunProperties.accessKeyId(), aliyunProperties.secretAccessKey()); + aliyunProperties.ossEndpoint(), aliyunProperties.accessKeyId(), aliyunProperties.accessKeySecret()); } @Override 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 48f38df2ce21..7474e6a3b5a5 100644 --- a/aliyun/src/main/java/org/apache/iceberg/aliyun/AliyunProperties.java +++ b/aliyun/src/main/java/org/apache/iceberg/aliyun/AliyunProperties.java @@ -44,13 +44,13 @@ public class AliyunProperties implements Serializable { /** * 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 CLIENT_SECRET_ACCESS_KEY = "client.secret-access-key"; + public static final String CLIENT_ACCESS_KEY_SECRET = "client.access-key-secret"; /** * The implementation class of {@link AliyunClientFactory} to customize Aliyun client configurations. @@ -66,7 +66,7 @@ public class AliyunProperties implements Serializable { private final String ossEndpoint; private final String accessKeyId; - private final String secretAccessKey; + private final String accessKeySecret; private final String ossStagingDirectory; public AliyunProperties() { @@ -77,7 +77,7 @@ public AliyunProperties(Map properties) { // OSS endpoint, accessKeyId, accessKeySecret. this.ossEndpoint = properties.get(OSS_ENDPOINT); this.accessKeyId = properties.get(CLIENT_ACCESS_KEY_ID); - this.secretAccessKey = properties.get(CLIENT_SECRET_ACCESS_KEY); + this.accessKeySecret = properties.get(CLIENT_ACCESS_KEY_SECRET); this.ossStagingDirectory = PropertyUtil.propertyAsString(properties, OSS_STAGING_DIRECTORY, System.getProperty("java.io.tmpdir")); @@ -91,8 +91,8 @@ public String accessKeyId() { return accessKeyId; } - public String secretAccessKey() { - return secretAccessKey; + public String accessKeySecret() { + return accessKeySecret; } public String ossStagingDirectory() {