Factoring out describe calls and reducing the over engineered api ver…

…sion selection behavior. Note the V12 API code paths are marked for removal.
aws-cloudformation · Nov 5, 2024 · c26541c · c26541c
1 parent 82a9ef3
commit c26541c
Show file tree

Hide file tree

Showing 4 changed files with 119 additions and 140 deletions.
diff --git a/...-dbinstance/src/main/java/software/amazon/rds/dbinstance/client/ApiVersionDispatcher.java b/...-dbinstance/src/main/java/software/amazon/rds/dbinstance/client/ApiVersionDispatcher.java
@@ -1,5 +1,6 @@
 package software.amazon.rds.dbinstance.client;
 
+import javax.annotation.Nonnull;
 import java.util.Comparator;
 import java.util.Map;
 import java.util.TreeMap;
@@ -24,6 +25,7 @@ public ApiVersionDispatcher<M, C> register(final ApiVersion version, final BiPre
      * @param context An instance of Context.
      * @return Returns the highest {@code ApiVersion} matching the input model and context. If no matchers triggered, DEFAULT version is returned (hence no need to register a match-all tester for DEFAULT version).
      */
+    @Nonnull
     public ApiVersion dispatch(final M model, final C context) {
         for (final Map.Entry<ApiVersion, BiPredicate<M, C>> entry : versionTesters.entrySet()) {
             if (entry.getValue().test(model, context)) {

diff --git a/aws-rds-dbinstance/src/main/java/software/amazon/rds/dbinstance/common/Fetch.java b/aws-rds-dbinstance/src/main/java/software/amazon/rds/dbinstance/common/Fetch.java
@@ -0,0 +1,32 @@
+package software.amazon.rds.dbinstance.common;
+
+import lombok.AllArgsConstructor;
+import software.amazon.awssdk.services.rds.RdsClient;
+import software.amazon.awssdk.services.rds.model.DBInstance;
+import software.amazon.awssdk.services.rds.model.DBSnapshot;
+import software.amazon.awssdk.services.rds.model.DescribeDbInstancesResponse;
+import software.amazon.awssdk.services.rds.model.DescribeDbSnapshotsResponse;
+import software.amazon.cloudformation.proxy.ProxyClient;
+import software.amazon.rds.dbinstance.ResourceModel;
+import software.amazon.rds.dbinstance.Translator;
+
+@AllArgsConstructor
+public class Fetch {
+    private final ProxyClient<RdsClient> rdsProxyClient;
+
+    public DBInstance dbInstance(final ResourceModel model) {
+        final DescribeDbInstancesResponse response = rdsProxyClient.injectCredentialsAndInvokeV2(
+                Translator.describeDbInstancesRequest(model),
+                rdsProxyClient.client()::describeDBInstances
+        );
+        return response.dbInstances().get(0);
+    }
+
+    public DBSnapshot dbSnapshot(final ResourceModel model) {
+        final DescribeDbSnapshotsResponse response = rdsProxyClient.injectCredentialsAndInvokeV2(
+                Translator.describeDbSnapshotsRequest(model),
+                rdsProxyClient.client()::describeDBSnapshots
+        );
+        return response.dbSnapshots().get(0);
+    }
+}
diff --git a/...-dbinstance/src/main/java/software/amazon/rds/dbinstance/common/create/FreshInstance.java b/...-dbinstance/src/main/java/software/amazon/rds/dbinstance/common/create/FreshInstance.java
@@ -3,8 +3,6 @@
 import lombok.AllArgsConstructor;
 import software.amazon.awssdk.services.rds.RdsClient;
 import software.amazon.awssdk.services.rds.model.DBInstance;
-import software.amazon.awssdk.services.rds.model.DescribeDbInstancesResponse;
-import software.amazon.awssdk.utils.ImmutableMap;
 import software.amazon.cloudformation.proxy.AmazonWebServicesClientProxy;
 import software.amazon.cloudformation.proxy.ProgressEvent;
 import software.amazon.cloudformation.proxy.ProxyClient;
@@ -21,8 +19,7 @@
 import software.amazon.rds.dbinstance.client.ApiVersionDispatcher;
 import software.amazon.rds.dbinstance.client.VersionedProxyClient;
 import software.amazon.rds.dbinstance.common.Errors;
-
-import java.util.Map;
+import software.amazon.rds.dbinstance.common.Fetch;
 
 @AllArgsConstructor
 public class FreshInstance implements DBInstanceFactory {
@@ -35,111 +32,89 @@ public class FreshInstance implements DBInstanceFactory {
     private final ApiVersionDispatcher<ResourceModel, CallbackContext> apiVersionDispatcher;
 
     public ProgressEvent<ResourceModel, CallbackContext> create(ProgressEvent<ResourceModel, CallbackContext> progress) {
-        return versioned(proxy, rdsProxyClient, progress, allTags, ImmutableMap.of(
-            ApiVersion.V12, this::createDbInstanceV12,
-            ApiVersion.DEFAULT, safeAddTags(this::createDbInstance)
-        ));
+        final ResourceModel model = progress.getResourceModel();
+        final CallbackContext callbackContext = progress.getCallbackContext();
+        final ApiVersion apiVersion = selectVersion(model, callbackContext);
+
+        if (apiVersion == ApiVersion.V12) {
+            return createDbInstanceV12(proxy, rdsProxyClient.forVersion(apiVersion), progress, allTags);
+        }
+
+        return safeAddTags(this::createDbInstance).invoke(proxy, rdsProxyClient.forVersion(apiVersion), progress, allTags);
     }
 
     private ProgressEvent<ResourceModel, CallbackContext> createDbInstance(
-        final AmazonWebServicesClientProxy proxy,
-        final ProxyClient<RdsClient> rdsProxyClient,
-        final ProgressEvent<ResourceModel, CallbackContext> progress,
-        final Tagging.TagSet tagSet
+            final AmazonWebServicesClientProxy proxy,
+            final ProxyClient<RdsClient> rdsProxyClient,
+            final ProgressEvent<ResourceModel, CallbackContext> progress,
+            final Tagging.TagSet tagSet
     ) {
+        final Fetch fetch = new Fetch(rdsProxyClient);
         return proxy.initiate(
-                "rds::create-db-instance",
-                rdsProxyClient,
-                progress.getResourceModel(),
-                progress.getCallbackContext()
-            ).translateToServiceRequest(model -> Translator.createDbInstanceRequest(model, tagSet))
-            .backoffDelay(config.getBackoff())
-            .makeServiceCall((createRequest, proxyInvocation) -> proxyInvocation.injectCredentialsAndInvokeV2(
-                createRequest,
-                proxyInvocation.client()::createDBInstance
-            ))
-            .stabilize((request, response, proxyInvocation, model, context) ->
-            {
-                final DBInstance dbInstance = fetchDBInstance(rdsProxyClient, model);
-                return DBInstancePredicates.isDBInstanceStabilizedAfterMutate(dbInstance, model, context, requestLogger) ;
-            })
-            .handleError((request, exception, client, model, context) -> Commons.handleException(
-                ProgressEvent.progress(model, context),
-                exception,
-                Errors.CREATE_DB_INSTANCE_ERROR_RULE_SET,
-                requestLogger
-            ))
-            .progress();
-    }
-
-    private DBInstance fetchDBInstance(
-        final ProxyClient<RdsClient> rdsProxyClient,
-        final ResourceModel model
-    ) {
-        final DescribeDbInstancesResponse response = rdsProxyClient.injectCredentialsAndInvokeV2(
-            Translator.describeDbInstancesRequest(model),
-            rdsProxyClient.client()::describeDBInstances
-        );
-        return response.dbInstances().get(0);
+                        "rds::create-db-instance",
+                        rdsProxyClient,
+                        progress.getResourceModel(),
+                        progress.getCallbackContext()
+                ).translateToServiceRequest(model -> Translator.createDbInstanceRequest(model, tagSet))
+                .backoffDelay(config.getBackoff())
+                .makeServiceCall((createRequest, proxyInvocation) -> proxyInvocation.injectCredentialsAndInvokeV2(
+                        createRequest,
+                        proxyInvocation.client()::createDBInstance
+                ))
+                .stabilize((request, response, proxyInvocation, model, context) ->
+                {
+                    final DBInstance dbInstance = fetch.dbInstance(model);
+                    return DBInstancePredicates.isDBInstanceStabilizedAfterMutate(dbInstance, model, context, requestLogger);
+                })
+                .handleError((request, exception, client, model, context) -> Commons.handleException(
+                        ProgressEvent.progress(model, context),
+                        exception,
+                        Errors.CREATE_DB_INSTANCE_ERROR_RULE_SET,
+                        requestLogger
+                ))
+                .progress();
     }
 
     private ProgressEvent<ResourceModel, CallbackContext> createDbInstanceV12(
-        final AmazonWebServicesClientProxy proxy,
-        final ProxyClient<RdsClient> rdsProxyClient,
-        final ProgressEvent<ResourceModel, CallbackContext> progress,
-        final Tagging.TagSet tagSet
+            final AmazonWebServicesClientProxy proxy,
+            final ProxyClient<RdsClient> rdsProxyClient,
+            final ProgressEvent<ResourceModel, CallbackContext> progress,
+            final Tagging.TagSet tagSet
     ) {
         requestLogger.log("CreateDbInstanceAPIv12Invoked");
         requestLogger.log("API version 12 create detected",
-            "This indicates that the customer is using DBSecurityGroup, which may result in certain features not" +
-                " functioning properly. Please refer to the API model for supported parameters");
+                "This indicates that the customer is using DBSecurityGroup, which may result in certain features not" +
+                        " functioning properly. Please refer to the API model for supported parameters");
+        final Fetch fetch = new Fetch(rdsProxyClient);
         return proxy.initiate(
-                "rds::create-db-instance-v12",
-                rdsProxyClient,
-                progress.getResourceModel(),
-                progress.getCallbackContext()
-            ).translateToServiceRequest(Translator::createDbInstanceRequestV12)
-            .backoffDelay(config.getBackoff())
-            .makeServiceCall((createRequest, proxyInvocation) -> proxyInvocation.injectCredentialsAndInvokeV2(
-                createRequest,
-                proxyInvocation.client()::createDBInstance
-            ))
-            .stabilize((request, response, proxyInvocation, model, context) -> {
-                final DBInstance dbInstance = fetchDBInstance(rdsProxyClient, model);
-                return DBInstancePredicates.isDBInstanceStabilizedAfterMutate(dbInstance, model, context, requestLogger) ;
-            })
-            .handleError((request, exception, client, model, context) -> Commons.handleException(
-                ProgressEvent.progress(model, context),
-                exception,
-                Errors.CREATE_DB_INSTANCE_ERROR_RULE_SET,
-                requestLogger
-            ))
-            .progress();
+                        "rds::create-db-instance-v12",
+                        rdsProxyClient,
+                        progress.getResourceModel(),
+                        progress.getCallbackContext()
+                ).translateToServiceRequest(Translator::createDbInstanceRequestV12)
+                .backoffDelay(config.getBackoff())
+                .makeServiceCall((createRequest, proxyInvocation) -> proxyInvocation.injectCredentialsAndInvokeV2(
+                        createRequest,
+                        proxyInvocation.client()::createDBInstance
+                ))
+                .stabilize((request, response, proxyInvocation, model, context) -> {
+                    final DBInstance dbInstance = fetch.dbInstance(model);
+                    return DBInstancePredicates.isDBInstanceStabilizedAfterMutate(dbInstance, model, context, requestLogger);
+                })
+                .handleError((request, exception, client, model, context) -> Commons.handleException(
+                        ProgressEvent.progress(model, context),
+                        exception,
+                        Errors.CREATE_DB_INSTANCE_ERROR_RULE_SET,
+                        requestLogger
+                ))
+                .progress();
     }
 
-    protected ProgressEvent<ResourceModel, CallbackContext> versioned(
-        final AmazonWebServicesClientProxy proxy,
-        final VersionedProxyClient<RdsClient> rdsProxyClient,
-        final ProgressEvent<ResourceModel, CallbackContext> progress,
-        final Tagging.TagSet allTags,
-        final Map<ApiVersion, HandlerMethod<ResourceModel, CallbackContext>> methodVersions
-    ) {
-        final ResourceModel model = progress.getResourceModel();
-        final CallbackContext callbackContext = progress.getCallbackContext();
-        final ApiVersion apiVersion = getApiVersionDispatcher().dispatch(model, callbackContext);
-        if (!methodVersions.containsKey(apiVersion)) {
-            throw new RuntimeException("Missing method version");
-        }
-        return methodVersions.get(apiVersion).invoke(proxy, rdsProxyClient.forVersion(apiVersion), progress, allTags);
-    }
-
-    protected ApiVersionDispatcher<ResourceModel, CallbackContext> getApiVersionDispatcher() {
-        return apiVersionDispatcher;
+    private ApiVersion selectVersion(ResourceModel model, CallbackContext callbackContext) {
+        return apiVersionDispatcher.dispatch(model, callbackContext);
     }
 
     private HandlerMethod<ResourceModel, CallbackContext> safeAddTags(final HandlerMethod<ResourceModel, CallbackContext> handlerMethod) {
         return (proxy, rdsProxyClient, progress, tagSet) -> progress.then(p -> Tagging.createWithTaggingFallback(proxy, rdsProxyClient, handlerMethod, progress, tagSet));
     }
-
-
 }
diff --git a/...s-dbinstance/src/main/java/software/amazon/rds/dbinstance/common/create/FromSnapshot.java b/...s-dbinstance/src/main/java/software/amazon/rds/dbinstance/common/create/FromSnapshot.java
@@ -25,6 +25,7 @@
 import software.amazon.rds.dbinstance.client.ApiVersionDispatcher;
 import software.amazon.rds.dbinstance.client.VersionedProxyClient;
 import software.amazon.rds.dbinstance.common.Errors;
+import software.amazon.rds.dbinstance.common.Fetch;
 import software.amazon.rds.dbinstance.util.ResourceModelHelper;
 
 import java.util.Map;
@@ -42,10 +43,14 @@ public class FromSnapshot implements DBInstanceFactory {
     @Override
     public ProgressEvent<ResourceModel, CallbackContext> create(ProgressEvent<ResourceModel, CallbackContext> progress) {
         final ResourceModel model = progress.getResourceModel();
+        final CallbackContext callbackContext = progress.getCallbackContext();
+        final ApiVersion apiVersion = selectVersion(model, callbackContext);
+
+        final Fetch fetch = new Fetch(rdsProxyClient.defaultClient());
         final boolean isMultiAZ = BooleanUtils.isTrue(model.getMultiAZ());
         if (ResourceModelHelper.isRestoreFromSnapshot(model) && !isMultiAZ) {
             try {
-                final DBSnapshot snapshot = fetchDBSnapshot(rdsProxyClient.defaultClient(), model);
+                final DBSnapshot snapshot = fetch.dbSnapshot(model);
                 final String engine = snapshot.engine();
                 if (StringUtils.isNullOrEmpty(model.getEngine())) {
                     model.setEngine(engine);
@@ -57,10 +62,12 @@ public ProgressEvent<ResourceModel, CallbackContext> create(ProgressEvent<Resour
                 return Commons.handleException(progress, e, Errors.RESTORE_DB_INSTANCE_ERROR_RULE_SET, requestLogger);
             }
         }
-        return versioned(proxy, rdsProxyClient, progress, allTags, ImmutableMap.of(
-                ApiVersion.V12, this::restoreDbInstanceFromSnapshotV12,
-                ApiVersion.DEFAULT, safeAddTags(this::restoreDbInstanceFromSnapshot)
-        ));
+
+        if (apiVersion == ApiVersion.V12) {
+            return restoreDbInstanceFromSnapshotV12(proxy, rdsProxyClient.forVersion(apiVersion), progress, allTags);
+        }
+
+        return safeAddTags(this::restoreDbInstanceFromSnapshot).invoke(proxy, rdsProxyClient.forVersion(apiVersion), progress, allTags);
     }
 
     private ProgressEvent<ResourceModel, CallbackContext> restoreDbInstanceFromSnapshotV12(
@@ -73,6 +80,7 @@ private ProgressEvent<ResourceModel, CallbackContext> restoreDbInstanceFromSnaps
         requestLogger.log("API version 12 restore detected",
                 "This indicates that the customer is using DBSecurityGroup, which may result in certain features not" +
                         " functioning properly. Please refer to the API model for supported parameters");
+        final Fetch fetch = new Fetch(rdsProxyClient);
         return proxy.initiate(
                         "rds::restore-db-instance-from-snapshot-v12",
                         rdsProxyClient,
@@ -85,7 +93,7 @@ private ProgressEvent<ResourceModel, CallbackContext> restoreDbInstanceFromSnaps
                         proxyInvocation.client()::restoreDBInstanceFromDBSnapshot
                 ))
                 .stabilize((request, response, proxyInvocation, model, context) -> {
-                    final DBInstance dbInstance = fetchDBInstance(rdsProxyClient, model);
+                    final DBInstance dbInstance = fetch.dbInstance(model);
                     return DBInstancePredicates.isDBInstanceStabilizedAfterMutate(dbInstance, model, context, requestLogger);
                 })
                 .handleError((request, exception, client, model, context) -> Commons.handleException(
@@ -103,6 +111,7 @@ private ProgressEvent<ResourceModel, CallbackContext> restoreDbInstanceFromSnaps
             final ProgressEvent<ResourceModel, CallbackContext> progress,
             final Tagging.TagSet tagSet
     ) {
+        final Fetch fetch = new Fetch(rdsProxyClient);
         return proxy.initiate(
                         "rds::restore-db-instance-from-snapshot",
                         rdsProxyClient,
@@ -115,7 +124,7 @@ private ProgressEvent<ResourceModel, CallbackContext> restoreDbInstanceFromSnaps
                         proxyInvocation.client()::restoreDBInstanceFromDBSnapshot
                 ))
                 .stabilize((request, response, proxyInvocation, model, context) -> {
-                    final DBInstance dbInstance = fetchDBInstance(rdsProxyClient, model);
+                    final DBInstance dbInstance = fetch.dbInstance(model);
                     return DBInstancePredicates.isDBInstanceStabilizedAfterMutate(dbInstance, model, context, requestLogger);
                 })
                 .handleError((request, exception, client, model, context) -> Commons.handleException(
@@ -127,50 +136,11 @@ private ProgressEvent<ResourceModel, CallbackContext> restoreDbInstanceFromSnaps
                 .progress();
     }
 
-    private DBInstance fetchDBInstance(
-            final ProxyClient<RdsClient> rdsProxyClient,
-            final ResourceModel model
-    ) {
-        final DescribeDbInstancesResponse response = rdsProxyClient.injectCredentialsAndInvokeV2(
-                Translator.describeDbInstancesRequest(model),
-                rdsProxyClient.client()::describeDBInstances
-        );
-        return response.dbInstances().get(0);
-    }
-
-    protected ProgressEvent<ResourceModel, CallbackContext> versioned(
-            final AmazonWebServicesClientProxy proxy,
-            final VersionedProxyClient<RdsClient> rdsProxyClient,
-            final ProgressEvent<ResourceModel, CallbackContext> progress,
-            final Tagging.TagSet allTags,
-            final Map<ApiVersion, HandlerMethod<ResourceModel, CallbackContext>> methodVersions
-    ) {
-        final ResourceModel model = progress.getResourceModel();
-        final CallbackContext callbackContext = progress.getCallbackContext();
-        final ApiVersion apiVersion = getApiVersionDispatcher().dispatch(model, callbackContext);
-        if (!methodVersions.containsKey(apiVersion)) {
-            throw new RuntimeException("Missing method version");
-        }
-        return methodVersions.get(apiVersion).invoke(proxy, rdsProxyClient.forVersion(apiVersion), progress, allTags);
-    }
-
-    protected ApiVersionDispatcher<ResourceModel, CallbackContext> getApiVersionDispatcher() {
-        return apiVersionDispatcher;
+    private ApiVersion selectVersion(ResourceModel model, CallbackContext callbackContext) {
+        return apiVersionDispatcher.dispatch(model, callbackContext);
     }
 
     private HandlerMethod<ResourceModel, CallbackContext> safeAddTags(final HandlerMethod<ResourceModel, CallbackContext> handlerMethod) {
         return (proxy, rdsProxyClient, progress, tagSet) -> progress.then(p -> Tagging.createWithTaggingFallback(proxy, rdsProxyClient, handlerMethod, progress, tagSet));
     }
-
-    protected DBSnapshot fetchDBSnapshot(
-            final ProxyClient<RdsClient> rdsProxyClient,
-            final ResourceModel model
-    ) {
-        final DescribeDbSnapshotsResponse response = rdsProxyClient.injectCredentialsAndInvokeV2(
-                Translator.describeDbSnapshotsRequest(model),
-                rdsProxyClient.client()::describeDBSnapshots
-        );
-        return response.dbSnapshots().get(0);
-    }
-
 }