Skip to content

Commit

Permalink
Addressed most of the comments - exceptions, unit tests, formatting, …
Browse files Browse the repository at this point in the history
…safeInvokeApi
  • Loading branch information
mirelap-amazon committed May 23, 2020
1 parent ab7580d commit 1713e9d
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 76 deletions.
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package software.amazon.codeguruprofiler.profilinggroup;

import software.amazon.awssdk.services.codeguruprofiler.CodeGuruProfilerClient;
import software.amazon.awssdk.services.codeguruprofiler.model.ActionGroup;
import software.amazon.awssdk.services.codeguruprofiler.model.CodeGuruProfilerException;
import software.amazon.awssdk.services.codeguruprofiler.model.ConflictException;
import software.amazon.awssdk.services.codeguruprofiler.model.CreateProfilingGroupRequest;
Expand All @@ -25,86 +24,100 @@
import java.util.Optional;

import static java.lang.String.format;
import static software.amazon.awssdk.services.codeguruprofiler.model.ActionGroup.AGENT_PERMISSIONS;

public class CreateHandler extends BaseHandler<CallbackContext> {

private final CodeGuruProfilerClient profilerClient = CodeGuruProfilerClientBuilder.create();

@Override
public ProgressEvent<ResourceModel, CallbackContext> handleRequest(
final AmazonWebServicesClientProxy proxy,
final ResourceHandlerRequest<ResourceModel> request,
final CallbackContext callbackContext,
final Logger logger) {
final AmazonWebServicesClientProxy proxy,
final ResourceHandlerRequest<ResourceModel> request,
final CallbackContext callbackContext,
final Logger logger) {

final String awsAccountId = request.getAwsAccountId();
final ResourceModel model = request.getDesiredResourceState();
final String pgName = model.getProfilingGroupName();

CreateProfilingGroupRequest createProfilingGroupRequest = CreateProfilingGroupRequest.builder()
.profilingGroupName(pgName)
.clientToken(request.getClientRequestToken())
.build();
try {
.profilingGroupName(pgName)
.clientToken(request.getClientRequestToken())
.build();
safelyInvokeApi(() -> {
proxy.injectCredentialsAndInvokeV2(createProfilingGroupRequest, profilerClient::createProfilingGroup);
} catch (CodeGuruProfilerException e) {
wrapException(e);
}
});
logger.log(format("%s [%s] for accountId [%s] has been successfully created!", ResourceModel.TYPE_NAME, pgName, awsAccountId));

Optional<List<String>> principals = principalsForAgentPermissionsFrom(model);
if (principals.isPresent()) {
putAgentPermissions(proxy, pgName, principals.get());
putAgentPermissions(proxy, logger, pgName, principals.get(), awsAccountId);
logger.log(format("%s [%s] for accountId [%s] has been successfully updated with agent permissions!",
ResourceModel.TYPE_NAME, pgName, awsAccountId));
ResourceModel.TYPE_NAME, pgName, awsAccountId));
}

return ProgressEvent.defaultSuccessHandler(model);
}

private void putAgentPermissions(final AmazonWebServicesClientProxy proxy, final String pgName, final List<String> principals) {
private void putAgentPermissions(final AmazonWebServicesClientProxy proxy, final Logger logger,
final String pgName, final List<String> principals, final String awsAccountId) {
PutPermissionRequest putPermissionRequest = PutPermissionRequest.builder()
.profilingGroupName(pgName)
.actionGroup(ActionGroup.AGENT_PERMISSIONS)
.principals(principals)
.build();
try {
proxy.injectCredentialsAndInvokeV2(putPermissionRequest, profilerClient::putPermission);
} catch (CodeGuruProfilerException e) {
.profilingGroupName(pgName)
.actionGroup(AGENT_PERMISSIONS)
.principals(principals)
.build();

safelyInvokeApi(() -> {
try {
DeleteProfilingGroupRequest deletePgRequest = DeleteProfilingGroupRequest.builder().profilingGroupName(pgName).build();
proxy.injectCredentialsAndInvokeV2(deletePgRequest, profilerClient::deleteProfilingGroup);
} catch (Throwable deleteException) {
e.addSuppressed(deleteException);
} finally {
wrapException(e);
proxy.injectCredentialsAndInvokeV2(putPermissionRequest, profilerClient::putPermission);
} catch (CodeGuruProfilerException putPermissionException) {
logger.log(format("%s [%s] for accountId [%s] has failed when updating the agent permissions, trying to delete the profiling group!",
ResourceModel.TYPE_NAME, pgName, awsAccountId));
deleteProfilingGroup(proxy, logger, pgName, awsAccountId, putPermissionException);
throw putPermissionException;
}
});
}

private void deleteProfilingGroup(AmazonWebServicesClientProxy proxy, Logger logger,
String pgName, String awsAccountId, CodeGuruProfilerException putPermissionException) {
DeleteProfilingGroupRequest deletePgRequest = DeleteProfilingGroupRequest.builder().profilingGroupName(pgName).build();
try {
proxy.injectCredentialsAndInvokeV2(deletePgRequest, profilerClient::deleteProfilingGroup);
logger.log(format("%s [%s] for accountId [%s] has succeeded when deleting the profiling group!",
ResourceModel.TYPE_NAME, pgName, awsAccountId));
} catch (CodeGuruProfilerException deleteException) {
logger.log(format("%s [%s] for accountId [%s] has failed when deleting the profiling group!",
ResourceModel.TYPE_NAME, pgName, awsAccountId));
putPermissionException.addSuppressed(deleteException);
throw putPermissionException;
}
}

private static void wrapException(final CodeGuruProfilerException e) {
if (e instanceof ConflictException) {
private static void safelyInvokeApi(final Runnable lambda) {
try {
lambda.run();
} catch (ConflictException e) {
throw new CfnAlreadyExistsException(e);
}
if (e instanceof InternalServerException) {
} catch (InternalServerException e) {
throw new CfnServiceInternalErrorException(e);
}
if (e instanceof ServiceQuotaExceededException) {
} catch (ServiceQuotaExceededException e) {
throw new CfnServiceLimitExceededException(e);
}
if (e instanceof ThrottlingException) {
} catch (ThrottlingException e) {
throw new CfnThrottlingException(e);
}
if (e instanceof ValidationException) {
} catch (ValidationException e) {
throw new CfnInvalidRequestException(ResourceModel.TYPE_NAME + e.getMessage(), e);
}
throw e;
}

private static Optional<List<String>> principalsForAgentPermissionsFrom(final ResourceModel model) {
if (model.getAgentPermissions() == null) {
return Optional.empty();
}
return Optional.ofNullable(model.getAgentPermissions().getPrincipals());
if (model.getAgentPermissions().getPrincipals() == null) {
return Optional.empty();
}
return Optional.of(model.getAgentPermissions().getPrincipals());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
Expand Down Expand Up @@ -30,6 +31,7 @@
import software.amazon.cloudformation.proxy.ResourceHandlerRequest;

import java.util.Arrays;
import java.util.List;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertThrows;
Expand All @@ -41,38 +43,44 @@
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static software.amazon.awssdk.services.codeguruprofiler.model.ActionGroup.AGENT_PERMISSIONS;
import static software.amazon.codeguruprofiler.profilinggroup.RequestBuilder.makeRequest;
import static software.amazon.codeguruprofiler.profilinggroup.RequestBuilder.makeValidRequest;

@ExtendWith(MockitoExtension.class)
public class CreateHandlerTest {

@Mock
private AmazonWebServicesClientProxy proxy;
private AmazonWebServicesClientProxy proxy = mock(AmazonWebServicesClientProxy.class);

@Mock
private Logger logger;
private Logger logger = mock(Logger.class);

private CreateHandler subject;
private CreateHandler subject = new CreateHandler();

private ResourceHandlerRequest<ResourceModel> request;

private final String profilingGroupName = "Silver-2020";
private final String clientToken = "clientTokenXXX";
private final List<String> principals = Arrays.asList("a", "bc");

private final CreateProfilingGroupRequest createPgRequest = CreateProfilingGroupRequest.builder()
.profilingGroupName(profilingGroupName)
.clientToken(clientToken)
.build();

@BeforeEach
public void setup() {
proxy = mock(AmazonWebServicesClientProxy.class);
logger = mock(Logger.class);
subject = new CreateHandler();
}
private final PutPermissionRequest putPermissionsRequest = PutPermissionRequest.builder()
.profilingGroupName(profilingGroupName)
.actionGroup(AGENT_PERMISSIONS)
.principals(principals)
.build();

private final DeleteProfilingGroupRequest deleteProfilingGroupRequest = DeleteProfilingGroupRequest.builder()
.profilingGroupName(profilingGroupName)
.build();

@Nested
class WithNoPermissions {
class WhenPermissionsAreNotSet {

@BeforeEach
public void setup() {
Expand All @@ -90,7 +98,7 @@ public void setup() {
public void testSuccess() {
final ProgressEvent<ResourceModel, CallbackContext> response = subject.handleRequest(proxy, request, null, logger);

assertResponse(response);
assertSuccessfulResponse(response);
}

@Test
Expand All @@ -103,29 +111,29 @@ public void testCorrectOperationIsCalled() {
}

@Nested
class WithPermissions {
class WhenPermissionsAreSet {

@BeforeEach
public void setup() {
doReturn(CreateProfilingGroupResponse.builder().build())
.when(proxy).injectCredentialsAndInvokeV2(any(CreateProfilingGroupRequest.class), any());
.when(proxy).injectCredentialsAndInvokeV2(eq(createPgRequest), any());
}

@Nested
class NoPrincipals {
class WhenPrincipalsAreNotSet {

@Test
public void testNullPermissions() {
ResourceModel model = newResourceModel(null);
request = makeRequest(model);
assertResponse(subject.handleRequest(proxy, request, null, logger));
assertSuccessfulResponse(subject.handleRequest(proxy, request, null, logger));
}

@Test
public void testNullPrincipals() {
ResourceModel model = newResourceModel(AgentPermissions.builder().principals(null).build());
request = makeRequest(model);
assertResponse(subject.handleRequest(proxy, request, null, logger));
assertSuccessfulResponse(subject.handleRequest(proxy, request, null, logger));
}

@AfterEach
Expand All @@ -136,57 +144,47 @@ public void testCorrectOperationIsCalled() {
}

@Nested
class WithPrincipals {
class WhenPrincipalsAreSet {

@BeforeEach
public void setup() {
ResourceModel model = newResourceModel(AgentPermissions.builder().principals(Arrays.asList("a", "bc")).build());
ResourceModel model = newResourceModel(AgentPermissions.builder().principals(principals).build());
request = makeRequest(model);
}

@Test
public void testSuccess() {
assertResponse(subject.handleRequest(proxy, request, null, logger));
assertSuccessfulResponse(subject.handleRequest(proxy, request, null, logger));
verify(proxy, times(1)).injectCredentialsAndInvokeV2(eq(createPgRequest), any());
verify(proxy, times(1)).injectCredentialsAndInvokeV2(any(PutPermissionRequest.class), any());
verify(proxy, times(1)).injectCredentialsAndInvokeV2(eq(putPermissionsRequest), any());
verifyNoMoreInteractions(proxy);
}

@Test
public void testPutPermissionsFails() {
public void testPutPermissionsFailsAssertExceptionType() {
doThrow(ConflictException.builder().build())
.when(proxy).injectCredentialsAndInvokeV2(any(PutPermissionRequest.class), any());
.when(proxy).injectCredentialsAndInvokeV2(eq(putPermissionsRequest), any());

CfnAlreadyExistsException exception = assertThrows(CfnAlreadyExistsException.class,
() -> subject.handleRequest(proxy, request, null, logger));

assertThat(exception).hasCauseExactlyInstanceOf(ConflictException.class);
assertThat(exception).hasNoSuppressedExceptions();

verify(proxy, times(1)).injectCredentialsAndInvokeV2(eq(createPgRequest), any());
verify(proxy, times(1)).injectCredentialsAndInvokeV2(any(PutPermissionRequest.class), any());
verify(proxy).injectCredentialsAndInvokeV2(any(DeleteProfilingGroupRequest.class), any());
verifyNoMoreInteractions(proxy);
}

@Test
public void testPutPermissionsFailsAndDeleteProfilingGroupFails() {
public void testPutPermissionsFailsAndDeleteProfilingGroupFailsAssertExceptionType() {
Throwable deleteException = InternalServerException.builder().build();
doThrow(ConflictException.builder().build())
.when(proxy).injectCredentialsAndInvokeV2(any(PutPermissionRequest.class), any());
.when(proxy).injectCredentialsAndInvokeV2(eq(putPermissionsRequest), any());
doThrow(deleteException)
.when(proxy).injectCredentialsAndInvokeV2(any(DeleteProfilingGroupRequest.class), any());
.when(proxy).injectCredentialsAndInvokeV2(eq(deleteProfilingGroupRequest), any());

CfnAlreadyExistsException exception = assertThrows(CfnAlreadyExistsException.class,
() -> subject.handleRequest(proxy, request, null, logger));

assertThat(exception).hasCauseExactlyInstanceOf(ConflictException.class);
assertThat(exception.getCause()).hasSuppressedException(deleteException);

verify(proxy, times(1)).injectCredentialsAndInvokeV2(eq(createPgRequest), any());
verify(proxy, times(1)).injectCredentialsAndInvokeV2(any(PutPermissionRequest.class), any());
verify(proxy, times(1)).injectCredentialsAndInvokeV2(any(DeleteProfilingGroupRequest.class), any());
verifyNoMoreInteractions(proxy);
}
}
}
Expand Down Expand Up @@ -266,7 +264,7 @@ private ResourceModel newResourceModel(final AgentPermissions permissions) {
.build();
}

private void assertResponse(ProgressEvent<ResourceModel, CallbackContext> response) {
private void assertSuccessfulResponse(ProgressEvent<ResourceModel, CallbackContext> response) {
assertThat(response).isNotNull();
assertThat(response.getStatus()).isEqualTo(OperationStatus.SUCCESS);
assertThat(response.getCallbackContext()).isNull();
Expand Down

0 comments on commit 1713e9d

Please sign in to comment.