From a725113a6ec6e10b2966603227bb520da64dc28e Mon Sep 17 00:00:00 2001 From: Bernard Jollans Date: Tue, 12 May 2020 16:51:45 +0200 Subject: [PATCH] Move duplicate handler code to super class (#13) * Add ListHandler with UnitTests * Move duplicate handler code into super class --- .../server/BaseOpsWorksCMHandler.java | 74 +++++++++++++++++++ .../opsworkscm/server/CreateHandler.java | 53 +------------ .../opsworkscm/server/DeleteHandler.java | 53 +------------ .../amazon/opsworkscm/server/ReadHandler.java | 57 +------------- .../opsworkscm/server/UpdateHandler.java | 58 +-------------- .../opsworkscm/server/ReadHandlerTest.java | 13 ++-- 6 files changed, 93 insertions(+), 215 deletions(-) create mode 100644 aws-opsworkscm-server/src/main/java/software/amazon/opsworkscm/server/BaseOpsWorksCMHandler.java diff --git a/aws-opsworkscm-server/src/main/java/software/amazon/opsworkscm/server/BaseOpsWorksCMHandler.java b/aws-opsworkscm-server/src/main/java/software/amazon/opsworkscm/server/BaseOpsWorksCMHandler.java new file mode 100644 index 0000000..0b8f18e --- /dev/null +++ b/aws-opsworkscm-server/src/main/java/software/amazon/opsworkscm/server/BaseOpsWorksCMHandler.java @@ -0,0 +1,74 @@ +package software.amazon.opsworkscm.server; + +import com.amazonaws.util.StringUtils; +import software.amazon.awssdk.services.opsworkscm.OpsWorksCmClient; +import software.amazon.cloudformation.proxy.AmazonWebServicesClientProxy; +import software.amazon.cloudformation.proxy.Logger; +import software.amazon.cloudformation.proxy.ProgressEvent; +import software.amazon.cloudformation.proxy.ResourceHandlerRequest; +import software.amazon.cloudformation.resource.IdentifierUtils; + +abstract public class BaseOpsWorksCMHandler extends BaseHandler { + + + protected ResourceModel model; + protected ResourceModel oldModel; + protected CallbackContext callbackContext; + protected Logger logger; + protected ResourceHandlerRequest request; + protected ClientWrapper client; + + protected static int NO_CALLBACK_DELAY = 0; + protected static int CALLBACK_DELAY_SECONDS = 60; + private static final int MAX_LENGTH_CONFIGURATION_SET_NAME = 40; + + @Override + abstract public ProgressEvent handleRequest( + final AmazonWebServicesClientProxy proxy, + final ResourceHandlerRequest request, + final CallbackContext callbackContext, + final Logger logger); + + protected void initialize(final AmazonWebServicesClientProxy proxy, + final ResourceHandlerRequest request, + final CallbackContext callbackContext, + final Logger logger) { + this.request = request; + this.model = request.getDesiredResourceState(); + this.oldModel = request.getPreviousResourceState(); + this.callbackContext = callbackContext; + this.logger = logger; + + setModelServerName(); + setModelId(); + + final OpsWorksCmClient opsWorksCmClientclient = ClientBuilder.getClient(); + this.client = new ClientWrapper(opsWorksCmClientclient, model, oldModel, proxy, logger); + } + + private void setModelServerName() { + if (StringUtils.isNullOrEmpty(model.getServerName())) { + logger.log("RequestModel doesn't have the server name. Setting it using request identifier and client token"); + model.setServerName( + IdentifierUtils.generateResourceIdentifier( + request.getLogicalResourceIdentifier(), + request.getClientRequestToken(), + MAX_LENGTH_CONFIGURATION_SET_NAME + ) + ); + } else if (model.getServerName().length() > MAX_LENGTH_CONFIGURATION_SET_NAME) { + logger.log(String.format("ServerName length was greater than %d characters. Truncating the ServerName", MAX_LENGTH_CONFIGURATION_SET_NAME)); + model.setServerName(model.getServerName().substring(0, MAX_LENGTH_CONFIGURATION_SET_NAME)); + } + } + + private void setModelId() { + if (model.getId() == null) { + logger.log("RequestModel doesn't have the model id. Setting it using request identifier and client token"); + model.setId(IdentifierUtils.generateResourceIdentifier( + request.getLogicalResourceIdentifier(), + request.getClientRequestToken() + )); + } + } +} diff --git a/aws-opsworkscm-server/src/main/java/software/amazon/opsworkscm/server/CreateHandler.java b/aws-opsworkscm-server/src/main/java/software/amazon/opsworkscm/server/CreateHandler.java index 657dcf6..b7ac12f 100644 --- a/aws-opsworkscm-server/src/main/java/software/amazon/opsworkscm/server/CreateHandler.java +++ b/aws-opsworkscm-server/src/main/java/software/amazon/opsworkscm/server/CreateHandler.java @@ -1,7 +1,5 @@ package software.amazon.opsworkscm.server; -import com.amazonaws.util.StringUtils; -import software.amazon.awssdk.services.opsworkscm.OpsWorksCmClient; import software.amazon.awssdk.services.opsworkscm.model.DescribeServersResponse; import software.amazon.awssdk.services.opsworkscm.model.InvalidStateException; import software.amazon.awssdk.services.opsworkscm.model.ResourceAlreadyExistsException; @@ -13,20 +11,8 @@ import software.amazon.cloudformation.proxy.Logger; import software.amazon.cloudformation.proxy.ProgressEvent; import software.amazon.cloudformation.proxy.ResourceHandlerRequest; -import software.amazon.cloudformation.resource.IdentifierUtils; -public class CreateHandler extends BaseHandler { - - ResourceModel model; - ResourceModel oldModel; - CallbackContext callbackContext; - Logger logger; - ResourceHandlerRequest request; - ClientWrapper client; - - private static int NO_CALLBACK_DELAY = 0; - private static int CALLBACK_DELAY_SECONDS = 60; - private static final int MAX_LENGTH_CONFIGURATION_SET_NAME = 40; +public class CreateHandler extends BaseOpsWorksCMHandler { @Override public ProgressEvent handleRequest( @@ -35,17 +21,7 @@ public ProgressEvent handleRequest( final CallbackContext callbackContext, final Logger logger) { - this.request = request; - this.model = request.getDesiredResourceState(); - this.oldModel = request.getPreviousResourceState(); - this.callbackContext = callbackContext; - this.logger = logger; - - setModelServerName(); - setModelId(); - - final OpsWorksCmClient opsWorksCmClientclient = ClientBuilder.getClient(); - this.client = new ClientWrapper(opsWorksCmClientclient, model, oldModel, proxy, logger); + initialize(proxy, request, callbackContext, logger); try { if (callbackContext.isStabilizationStarted()) { @@ -118,31 +94,6 @@ private ProgressEvent handleStabilize() { } } - private void setModelServerName() { - if (StringUtils.isNullOrEmpty(model.getServerName())) { - logger.log("RequestModel doesn't have the server name. Setting it using request identifier and client token"); - model.setServerName( - IdentifierUtils.generateResourceIdentifier( - request.getLogicalResourceIdentifier(), - request.getClientRequestToken(), - MAX_LENGTH_CONFIGURATION_SET_NAME - ) - ); - } else if (model.getServerName().length() > MAX_LENGTH_CONFIGURATION_SET_NAME) { - logger.log(String.format("ServerName length was greater than %d characters. Truncating the ServerName", MAX_LENGTH_CONFIGURATION_SET_NAME)); - model.setServerName(model.getServerName().substring(0, MAX_LENGTH_CONFIGURATION_SET_NAME)); - } - } - - private void setModelId() { - if (model.getId() == null) { - logger.log("RequestModel doesn't have the model id. Setting it using request identifier and client token"); - model.setId(IdentifierUtils.generateResourceIdentifier( - request.getLogicalResourceIdentifier(), - request.getClientRequestToken() - )); - } - } private ProgressEvent handleServerNotFound(final String serverName) { logger.log(String.format("Server %s failed to CREATE because it was not found.", serverName)); diff --git a/aws-opsworkscm-server/src/main/java/software/amazon/opsworkscm/server/DeleteHandler.java b/aws-opsworkscm-server/src/main/java/software/amazon/opsworkscm/server/DeleteHandler.java index 73259ec..388d1c6 100644 --- a/aws-opsworkscm-server/src/main/java/software/amazon/opsworkscm/server/DeleteHandler.java +++ b/aws-opsworkscm-server/src/main/java/software/amazon/opsworkscm/server/DeleteHandler.java @@ -1,39 +1,23 @@ package software.amazon.opsworkscm.server; -import com.amazonaws.util.StringUtils; -import software.amazon.awssdk.services.opsworkscm.OpsWorksCmClient; import software.amazon.awssdk.services.opsworkscm.model.DescribeServersResponse; import software.amazon.awssdk.services.opsworkscm.model.InvalidStateException; import software.amazon.awssdk.services.opsworkscm.model.ResourceNotFoundException; import software.amazon.awssdk.services.opsworkscm.model.Server; import software.amazon.awssdk.services.opsworkscm.model.ServerStatus; import software.amazon.awssdk.services.opsworkscm.model.ValidationException; -import software.amazon.cloudformation.exceptions.CfnInvalidRequestException; import software.amazon.cloudformation.proxy.AmazonWebServicesClientProxy; import software.amazon.cloudformation.proxy.HandlerErrorCode; import software.amazon.cloudformation.proxy.Logger; import software.amazon.cloudformation.proxy.ProgressEvent; import software.amazon.cloudformation.proxy.ResourceHandlerRequest; -import software.amazon.cloudformation.resource.IdentifierUtils; -public class DeleteHandler extends BaseHandler { +public class DeleteHandler extends BaseOpsWorksCMHandler { public static final String SERVER_DELETION_FAILED_MESSAGE = "Server %s deletion has failed with reason: %s"; public static final String SERVER_OPERATION_STILL_IN_PROGRESS_MESSAGE = "Cannot delete the server '%s'. The current operation on the server is still in progress\\." + " \\(Service: AWSOpsWorksCM; Status Code: 400; Error Code: ValidationException; Request ID: .*\\)"; - AmazonWebServicesClientProxy proxy; - ResourceHandlerRequest request; - CallbackContext callbackContext; - Logger logger; - ResourceModel model; - ResourceModel oldModel; - ClientWrapper client; - - private static int NO_CALLBACK_DELAY = 0; - private static int CALLBACK_DELAY_SECONDS = 60; - private static final int MAX_LENGTH_CONFIGURATION_SET_NAME = 40; - @Override public ProgressEvent handleRequest( final AmazonWebServicesClientProxy proxy, @@ -41,17 +25,7 @@ public ProgressEvent handleRequest( final CallbackContext callbackContext, final Logger logger) { - this.request = request; - this.model = request.getDesiredResourceState(); - this.oldModel = request.getPreviousResourceState(); - this.callbackContext = callbackContext; - this.logger = logger; - - setModelServerName(); - setModelId(); - - final OpsWorksCmClient opsWorksCmClientclient = ClientBuilder.getClient(); - this.client = new ClientWrapper(opsWorksCmClientclient, model, oldModel, proxy, logger); + initialize(proxy, request, callbackContext, logger); try { if (callbackContext.isStabilizationStarted()) { @@ -116,29 +90,6 @@ private ProgressEvent handleStabilize() { } } - private void setModelServerName() { - if (StringUtils.isNullOrEmpty(model.getServerName())) { - model.setServerName( - IdentifierUtils.generateResourceIdentifier( - request.getLogicalResourceIdentifier(), - request.getClientRequestToken(), - MAX_LENGTH_CONFIGURATION_SET_NAME - ) - ); - } else if (model.getServerName().length() > MAX_LENGTH_CONFIGURATION_SET_NAME) { - model.setServerName(model.getServerName().substring(0, MAX_LENGTH_CONFIGURATION_SET_NAME)); - } - } - - private void setModelId() { - if (model.getId() == null) { - model.setId(IdentifierUtils.generateResourceIdentifier( - request.getLogicalResourceIdentifier(), - request.getClientRequestToken() - )); - } - } - private ProgressEvent handleServerNotFound(final String serverName) { logger.log(String.format("Server %s deleted successfully.", serverName)); return ProgressEvent.defaultSuccessHandler(model); diff --git a/aws-opsworkscm-server/src/main/java/software/amazon/opsworkscm/server/ReadHandler.java b/aws-opsworkscm-server/src/main/java/software/amazon/opsworkscm/server/ReadHandler.java index aedd825..0e04342 100644 --- a/aws-opsworkscm-server/src/main/java/software/amazon/opsworkscm/server/ReadHandler.java +++ b/aws-opsworkscm-server/src/main/java/software/amazon/opsworkscm/server/ReadHandler.java @@ -1,27 +1,14 @@ package software.amazon.opsworkscm.server; -import com.amazonaws.util.StringUtils; -import software.amazon.awssdk.services.opsworkscm.OpsWorksCmClient; import software.amazon.awssdk.services.opsworkscm.model.DescribeServersResponse; import software.amazon.awssdk.services.opsworkscm.model.Server; -import software.amazon.cloudformation.exceptions.CfnInvalidRequestException; +import software.amazon.cloudformation.exceptions.ResourceNotFoundException; import software.amazon.cloudformation.proxy.AmazonWebServicesClientProxy; import software.amazon.cloudformation.proxy.Logger; import software.amazon.cloudformation.proxy.ProgressEvent; import software.amazon.cloudformation.proxy.ResourceHandlerRequest; -import software.amazon.cloudformation.resource.IdentifierUtils; -import software.amazon.cloudformation.exceptions.ResourceNotFoundException; - -public class ReadHandler extends BaseHandler { - ResourceModel model; - ResourceModel oldModel; - CallbackContext callbackContext; - Logger logger; - ResourceHandlerRequest request; - ClientWrapper client; - - private static final int MAX_LENGTH_CONFIGURATION_SET_NAME = 40; +public class ReadHandler extends BaseOpsWorksCMHandler { @Override public ProgressEvent handleRequest( @@ -30,17 +17,7 @@ public ProgressEvent handleRequest( final CallbackContext callbackContext, final Logger logger) { - this.request = request; - this.model = request.getDesiredResourceState(); - this.oldModel = request.getPreviousResourceState(); - this.callbackContext = callbackContext; - this.logger = logger; - - setModelServerName(); - setModelId(); - - final OpsWorksCmClient opsWorksCmClientclient = ClientBuilder.getClient(); - this.client = new ClientWrapper(opsWorksCmClientclient, model, oldModel, proxy, logger); + initialize(proxy, request, callbackContext, logger); final DescribeServersResponse result; final String serverName = model.getServerName(); @@ -59,34 +36,6 @@ public ProgressEvent handleRequest( } } - private void setModelServerName() { - if (StringUtils.isNullOrEmpty(model.getServerName())) { - logger.log("RequestModel doesn't have the server name. Setting it using request identifier and client token"); - model.setServerName( - IdentifierUtils.generateResourceIdentifier( - request.getLogicalResourceIdentifier(), - request.getClientRequestToken(), - MAX_LENGTH_CONFIGURATION_SET_NAME - ) - ); - } else if (model.getServerName().length() > MAX_LENGTH_CONFIGURATION_SET_NAME) { - logger.log(String.format("ServerName %s was greater than %d characters", model.getServerName(), MAX_LENGTH_CONFIGURATION_SET_NAME)); - throw new CfnInvalidRequestException(String.format("ServerName %s must have less than or equal to %d characters", model.getServerName(), MAX_LENGTH_CONFIGURATION_SET_NAME)); - } else { - model.setServerName(model.getServerName()); - } - } - - private void setModelId() { - if (model.getId() == null) { - logger.log("RequestModel doesn't have the model id. Setting it using request identifier and client token"); - model.setId(IdentifierUtils.generateResourceIdentifier( - request.getLogicalResourceIdentifier(), - request.getClientRequestToken() - )); - } - } - private void addDescribeServerResponseAttributes(final Server server) { model.setEndpoint(server.endpoint()); model.setArn(server.serverArn()); diff --git a/aws-opsworkscm-server/src/main/java/software/amazon/opsworkscm/server/UpdateHandler.java b/aws-opsworkscm-server/src/main/java/software/amazon/opsworkscm/server/UpdateHandler.java index c2c4217..01b72a1 100644 --- a/aws-opsworkscm-server/src/main/java/software/amazon/opsworkscm/server/UpdateHandler.java +++ b/aws-opsworkscm-server/src/main/java/software/amazon/opsworkscm/server/UpdateHandler.java @@ -1,7 +1,5 @@ package software.amazon.opsworkscm.server; -import com.amazonaws.util.StringUtils; -import software.amazon.awssdk.services.opsworkscm.OpsWorksCmClient; import software.amazon.awssdk.services.opsworkscm.model.InvalidStateException; import software.amazon.awssdk.services.opsworkscm.model.ResourceNotFoundException; import software.amazon.awssdk.services.opsworkscm.model.ValidationException; @@ -10,19 +8,8 @@ import software.amazon.cloudformation.proxy.Logger; import software.amazon.cloudformation.proxy.ProgressEvent; import software.amazon.cloudformation.proxy.ResourceHandlerRequest; -import software.amazon.cloudformation.resource.IdentifierUtils; -public class UpdateHandler extends BaseHandler { - - ResourceModel model; - ResourceModel oldModel; - CallbackContext callbackContext; - Logger logger; - ResourceHandlerRequest request; - ClientWrapper client; - - private static int NO_CALLBACK_DELAY = 0; - private static final int MAX_LENGTH_CONFIGURATION_SET_NAME = 40; +public class UpdateHandler extends BaseOpsWorksCMHandler { @Override public ProgressEvent handleRequest( @@ -31,17 +18,7 @@ public ProgressEvent handleRequest( final CallbackContext callbackContext, final Logger logger) { - this.request = request; - this.model = request.getDesiredResourceState(); - this.oldModel = request.getPreviousResourceState(); - this.callbackContext = callbackContext; - this.logger = logger; - - setModelServerName(); - setModelId(); - - final OpsWorksCmClient opsWorksCmClientclient = ClientBuilder.getClient(); - this.client = new ClientWrapper(opsWorksCmClientclient, model, oldModel, proxy, logger); + initialize(proxy, request, callbackContext, logger); try { if (!callbackContext.isUpdateTagComplete()) { @@ -63,43 +40,16 @@ public ProgressEvent handleRequest( } } - public ProgressEvent updateTags() { + private ProgressEvent updateTags() { client.untagServer(); client.tagServer(); callbackContext.setUpdateTagComplete(true); return ProgressEvent.defaultInProgressHandler(callbackContext, NO_CALLBACK_DELAY, model); } - public ProgressEvent updateServer() { + private ProgressEvent updateServer() { client.updateServer(); callbackContext.setUpdateServerComplete(true); return ProgressEvent.defaultInProgressHandler(callbackContext, NO_CALLBACK_DELAY, model); } - - private void setModelServerName() { - if (StringUtils.isNullOrEmpty(model.getServerName())) { - logger.log("RequestModel doesn't have the server name. Setting it using request identifier and client token"); - model.setServerName( - IdentifierUtils.generateResourceIdentifier( - request.getLogicalResourceIdentifier(), - request.getClientRequestToken(), - MAX_LENGTH_CONFIGURATION_SET_NAME - ) - ); - } else if (model.getServerName().length() > MAX_LENGTH_CONFIGURATION_SET_NAME) { - logger.log(String.format("ServerName length was greater than %d characters. Truncating the ServerName", MAX_LENGTH_CONFIGURATION_SET_NAME)); - model.setServerName(model.getServerName().substring(0, MAX_LENGTH_CONFIGURATION_SET_NAME)); - } - } - - private void setModelId() { - if (model.getId() == null) { - logger.log("RequestModel doesn't have the model id. Setting it using request identifier and client token"); - model.setId(IdentifierUtils.generateResourceIdentifier( - request.getLogicalResourceIdentifier(), - request.getClientRequestToken() - )); - } - } - } diff --git a/aws-opsworkscm-server/src/test/java/software/amazon/opsworkscm/server/ReadHandlerTest.java b/aws-opsworkscm-server/src/test/java/software/amazon/opsworkscm/server/ReadHandlerTest.java index 94b0572..623ead1 100644 --- a/aws-opsworkscm-server/src/test/java/software/amazon/opsworkscm/server/ReadHandlerTest.java +++ b/aws-opsworkscm-server/src/test/java/software/amazon/opsworkscm/server/ReadHandlerTest.java @@ -22,6 +22,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; @ExtendWith(MockitoExtension.class) @@ -63,6 +64,9 @@ public void setup() { .clientRequestToken(UUID.randomUUID().toString()) .region(REGION) .build(); + + lenient().doReturn(DescribeServersResponse.builder().servers(Server.builder().serverName(SERVER_NAME).status("HEALTHY").build()).build()) + .when(proxy).injectCredentialsAndInvokeV2(any(), any()); } @Test @@ -101,10 +105,10 @@ public void testDescribeSimpleServerLongName() { .region(REGION) .build(); - assertThrows(CfnInvalidRequestException.class, - ()->{ - handler.handleRequest(proxy, request, callbackContext, logger); - }); + ProgressEvent response = handler.handleRequest(proxy, request, callbackContext, logger); + String actualServerName = response.getResourceModel().getServerName(); + assertThat(actualServerName).isEqualTo("OpsWorksCMServerHasAVeryLongServerNameIn"); + assertThat(actualServerName.length()).isEqualTo(40); } @Test @@ -120,7 +124,6 @@ public void testDescribeServerNotFound() { } private ProgressEvent assertDescribeSuccess(ResourceHandlerRequest request) { - doReturn(DescribeServersResponse.builder().servers(Server.builder().serverName(SERVER_NAME).status("HEALTHY").build()).build()).when(proxy).injectCredentialsAndInvokeV2(any(), any()); final ProgressEvent response = handler.handleRequest(proxy, request, callbackContext, logger); assertThat(response).isNotNull();