danielcweeks commented on a change in pull request #1786:
URL: https://github.com/apache/iceberg/pull/1786#discussion_r529034755



##########
File path: aws/src/main/java/org/apache/iceberg/aws/s3/S3RequestUtil.java
##########
@@ -33,87 +36,66 @@ private S3RequestUtil() {
   }
 
   static void configureEncryption(AwsProperties awsProperties, 
PutObjectRequest.Builder requestBuilder) {
-    switch (awsProperties.s3FileIoSseType().toLowerCase(Locale.ENGLISH)) {
-      case AwsProperties.S3FILEIO_SSE_TYPE_NONE:
-        break;
+    configureEncryption(awsProperties, requestBuilder::serverSideEncryption, 
requestBuilder::ssekmsKeyId,
+        requestBuilder::sseCustomerAlgorithm, requestBuilder::sseCustomerKey, 
requestBuilder::sseCustomerKeyMD5);
+  }
 
-      case AwsProperties.S3FILEIO_SSE_TYPE_KMS:
-        requestBuilder.serverSideEncryption(ServerSideEncryption.AWS_KMS);
-        requestBuilder.ssekmsKeyId(awsProperties.s3FileIoSseKey());
-        break;
+  static void configureEncryption(AwsProperties awsProperties, 
CreateMultipartUploadRequest.Builder requestBuilder) {
+    configureEncryption(awsProperties, requestBuilder::serverSideEncryption, 
requestBuilder::ssekmsKeyId,
+        requestBuilder::sseCustomerAlgorithm, requestBuilder::sseCustomerKey, 
requestBuilder::sseCustomerKeyMD5);
+  }
 
-      case AwsProperties.S3FILEIO_SSE_TYPE_S3:
-        requestBuilder.serverSideEncryption(ServerSideEncryption.AES256);
-        break;
+  static void configureEncryption(AwsProperties awsProperties, 
UploadPartRequest.Builder requestBuilder) {
+    configureEncryption(awsProperties, null, null,
+        requestBuilder::sseCustomerAlgorithm, requestBuilder::sseCustomerKey, 
requestBuilder::sseCustomerKeyMD5);
+  }
 
-      case AwsProperties.S3FILEIO_SSE_TYPE_CUSTOM:
-        
requestBuilder.sseCustomerAlgorithm(ServerSideEncryption.AES256.name());
-        requestBuilder.sseCustomerKey(awsProperties.s3FileIoSseKey());
-        requestBuilder.sseCustomerKeyMD5(awsProperties.s3FileIoSseMd5());
-        break;
+  static void configureEncryption(AwsProperties awsProperties, 
GetObjectRequest.Builder requestBuilder) {
+    configureEncryption(awsProperties, null, null,
+        requestBuilder::sseCustomerAlgorithm, requestBuilder::sseCustomerKey, 
requestBuilder::sseCustomerKeyMD5);
+  }
 
-      default:
-        throw new IllegalArgumentException(
-            "Cannot support given S3 encryption type: " + 
awsProperties.s3FileIoSseType());
-    }
+  static void configureEncryption(AwsProperties awsProperties, 
HeadObjectRequest.Builder requestBuilder) {
+    configureEncryption(awsProperties, null, null,
+        requestBuilder::sseCustomerAlgorithm, requestBuilder::sseCustomerKey, 
requestBuilder::sseCustomerKeyMD5);
   }
 
-  static void configureEncryption(AwsProperties awsProperties, 
CreateMultipartUploadRequest.Builder requestBuilder) {
+  @SuppressWarnings("ReturnValueIgnored")
+  static void configureEncryption(
+      AwsProperties awsProperties,
+      Function<ServerSideEncryption, S3Request.Builder> encryptionSetter,
+      Function<String, S3Request.Builder> kmsKeySetter,
+      Function<String, S3Request.Builder> customAlgorithmSetter,
+      Function<String, S3Request.Builder> customKeySetter,
+      Function<String, S3Request.Builder> customMd5Setter) {
+
     switch (awsProperties.s3FileIoSseType().toLowerCase(Locale.ENGLISH)) {
       case AwsProperties.S3FILEIO_SSE_TYPE_NONE:
         break;
 
       case AwsProperties.S3FILEIO_SSE_TYPE_KMS:
-        requestBuilder.serverSideEncryption(ServerSideEncryption.AWS_KMS);
-        requestBuilder.ssekmsKeyId(awsProperties.s3FileIoSseKey());
-        break;
+        if (encryptionSetter != null) {

Review comment:
       The null checks as controls feel a little awkward.  Would it make sense 
to switch them out for noop functions like:




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to