From 0e0e44e1c8dd4b16692c92ee36a4cec9b471180e Mon Sep 17 00:00:00 2001 From: Phil Shapiro Date: Wed, 3 Apr 2024 10:53:51 -0400 Subject: [PATCH] Proof-of-concept of using annotations for step inputs and outputs. A simple "data flow" can be generated at test time which can validate declared inputs and outputs for steps. --- .../workspace/common/stairway/BaseStep.java | 75 +++++++++++ .../workspace/common/stairway/StepInput.java | 14 +++ .../workspace/common/stairway/StepOutput.java | 14 +++ .../workspace/common/stairway/StepUtils.java | 119 ++++++++++++++++++ .../application/ApplicationAbleDaoStepV3.java | 55 ++++++++ .../application/ApplicationAbleFlightV3.java | 31 +++++ .../application/ApplicationAbleIamStepV3.java | 61 +++++++++ .../ApplicationAblePrecheckStepV3.java | 74 +++++++++++ .../ApplicationAbleFlightV3Test.java | 27 ++++ 9 files changed, 470 insertions(+) create mode 100644 service/src/main/java/bio/terra/workspace/common/stairway/BaseStep.java create mode 100644 service/src/main/java/bio/terra/workspace/common/stairway/StepInput.java create mode 100644 service/src/main/java/bio/terra/workspace/common/stairway/StepOutput.java create mode 100644 service/src/main/java/bio/terra/workspace/common/stairway/StepUtils.java create mode 100644 service/src/main/java/bio/terra/workspace/service/workspace/flight/application/ApplicationAbleDaoStepV3.java create mode 100644 service/src/main/java/bio/terra/workspace/service/workspace/flight/application/ApplicationAbleFlightV3.java create mode 100644 service/src/main/java/bio/terra/workspace/service/workspace/flight/application/ApplicationAbleIamStepV3.java create mode 100644 service/src/main/java/bio/terra/workspace/service/workspace/flight/application/ApplicationAblePrecheckStepV3.java create mode 100644 service/src/test/java/bio/terra/workspace/service/workspace/flight/application/ApplicationAbleFlightV3Test.java diff --git a/service/src/main/java/bio/terra/workspace/common/stairway/BaseStep.java b/service/src/main/java/bio/terra/workspace/common/stairway/BaseStep.java new file mode 100644 index 0000000000..c5de96010c --- /dev/null +++ b/service/src/main/java/bio/terra/workspace/common/stairway/BaseStep.java @@ -0,0 +1,75 @@ +package bio.terra.workspace.common.stairway; + +import bio.terra.buffer.model.ErrorReport; +import bio.terra.stairway.FlightContext; +import bio.terra.stairway.Step; +import bio.terra.stairway.StepResult; +import bio.terra.stairway.exception.RetryException; +import com.google.common.annotations.VisibleForTesting; +import org.springframework.http.HttpStatus; + +public abstract class BaseStep implements Step { + @StepOutput protected Object response; + @StepOutput protected HttpStatus statusCode; + + private FlightContext context; + + @Override + public final StepResult doStep(FlightContext context) + throws InterruptedException, RetryException { + this.context = context; + StepUtils.readInputs(this, context); + try { + return perform(); + } finally { + StepUtils.writeOutputs(this, context); + } + } + + @Override + public final StepResult undoStep(FlightContext context) throws InterruptedException { + this.context = context; + StepUtils.readInputs(this, context); + return undo(); + } + + public abstract StepResult perform() throws InterruptedException, RetryException; + + public StepResult undo() throws InterruptedException { + // Many steps aren't undoable. + return StepResult.getStepResultSuccess(); + } + + protected void setErrorResponse(String message, HttpStatus responseStatus) { + ErrorReport errorModel = new ErrorReport().message(message); + setResponse(errorModel, responseStatus); + } + + protected void setResponse(Object responseObject, HttpStatus responseStatus) { + response = responseObject; + statusCode = responseStatus; + } + + protected void setResponse(Object responseObject) { + response = responseObject; + statusCode = HttpStatus.OK; + } + + protected FlightContext getContext() { + return context; + } + + protected String getFlightId() { + return context.getFlightId(); + } + + @VisibleForTesting + public Object getResponse() { + return response; + } + + @VisibleForTesting + public HttpStatus getStatusCode() { + return statusCode; + } +} diff --git a/service/src/main/java/bio/terra/workspace/common/stairway/StepInput.java b/service/src/main/java/bio/terra/workspace/common/stairway/StepInput.java new file mode 100644 index 0000000000..98d1cb5301 --- /dev/null +++ b/service/src/main/java/bio/terra/workspace/common/stairway/StepInput.java @@ -0,0 +1,14 @@ +package bio.terra.workspace.common.stairway; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Retention(RetentionPolicy.RUNTIME) +@Target(ElementType.FIELD) +public @interface StepInput { + String USE_DEFAULT_NAME = ""; + + String value() default USE_DEFAULT_NAME; +} diff --git a/service/src/main/java/bio/terra/workspace/common/stairway/StepOutput.java b/service/src/main/java/bio/terra/workspace/common/stairway/StepOutput.java new file mode 100644 index 0000000000..0e7b99cb89 --- /dev/null +++ b/service/src/main/java/bio/terra/workspace/common/stairway/StepOutput.java @@ -0,0 +1,14 @@ +package bio.terra.workspace.common.stairway; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Retention(RetentionPolicy.RUNTIME) +@Target(ElementType.FIELD) +public @interface StepOutput { + String USE_DEFAULT_NAME = ""; + + String value() default USE_DEFAULT_NAME; +} diff --git a/service/src/main/java/bio/terra/workspace/common/stairway/StepUtils.java b/service/src/main/java/bio/terra/workspace/common/stairway/StepUtils.java new file mode 100644 index 0000000000..302024fa43 --- /dev/null +++ b/service/src/main/java/bio/terra/workspace/common/stairway/StepUtils.java @@ -0,0 +1,119 @@ +package bio.terra.workspace.common.stairway; + +import bio.terra.stairway.Flight; +import bio.terra.stairway.FlightContext; +import bio.terra.stairway.FlightMap; +import bio.terra.stairway.Step; + +import java.lang.annotation.Annotation; +import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.List; + +// Suppress sonar warnings about reflection API usage. Reflection APIs must be used to read and +// write fields in a Step. +@SuppressWarnings({"java:S3011"}) +public class StepUtils { + + public static class MissingStepInputException extends RuntimeException { + public MissingStepInputException(String key) { + super("No flight value found for StepInput key '" + key + "'"); + } + } + + public static class IllegalSetException extends RuntimeException { + public IllegalSetException(Throwable cause) { + super(cause); + } + } + + public static class IllegalGetException extends RuntimeException { + public IllegalGetException(Throwable cause) { + super(cause); + } + } + + private StepUtils() {} + + public static String keyFromField(Field field) { + var input = field.getAnnotation(StepInput.class); + if (input != null && !input.value().isEmpty()) { + return input.value(); + } + var output = field.getAnnotation(StepOutput.class); + if (output != null && !output.value().isEmpty()) { + return output.value(); + } + return field.getName(); + } + + private static List collect(Step step, Class annotation) { + List inputs = new ArrayList<>(); + for (Class clazz = step.getClass(); clazz != null; clazz = clazz.getSuperclass()) { + for (Field field : clazz.getDeclaredFields()) { + if (field.isAnnotationPresent(annotation)) { + inputs.add(field); + } + } + } + return inputs; + } + + public static void readInputs(Step step, FlightContext context) throws MissingStepInputException { + collect(step, StepInput.class) + .forEach( + field -> { + String key = keyFromField(field); + if (context.getInputParameters().containsKey(key)) { + setField(step, context.getInputParameters(), field, key); + } else if (context.getWorkingMap().containsKey(key)) { + setField(step, context.getWorkingMap(), field, key); + } else if (!field.isAnnotationPresent(StepOutput.class)) { + // If the field is only used as an input, report an error if there's no value for + // it. + throw new MissingStepInputException(key); + } + }); + } + + private static void setField(Step step, FlightMap map, Field field, String key) { + field.setAccessible(true); + try { + field.set(step, map.get(key, field.getType())); + } catch (IllegalAccessException e) { + throw new IllegalSetException(e); + } + } + + public static void writeOutputs(Step step, FlightContext context) { + collect(step, StepOutput.class) + .forEach( + field -> { + field.setAccessible(true); + final Object value; + try { + value = field.get(step); + } catch (IllegalAccessException e) { + throw new IllegalGetException(e); + } + // An unset output can occur if an exception is thrown inside the run() operation. + if (value != null) { + context.getWorkingMap().put(keyFromField(field), value); + } + }); + } + + public record StepData(String stepName, List inputs, List outputs) {} + + static StepData getStepData(Step step) { + return new StepData( + step.getClass().getSimpleName(), + collect(step, StepInput.class).stream().map(StepUtils::keyFromField).toList(), + collect(step, StepOutput.class).stream().map(StepUtils::keyFromField).toList()); + } + + // Using inputs and outputs, generate a flow analysis report + public static List generateFlowAnalysisReport(Flight flight) { + return flight.getSteps().stream().map(StepUtils::getStepData).toList(); + } +} diff --git a/service/src/main/java/bio/terra/workspace/service/workspace/flight/application/ApplicationAbleDaoStepV3.java b/service/src/main/java/bio/terra/workspace/service/workspace/flight/application/ApplicationAbleDaoStepV3.java new file mode 100644 index 0000000000..e659fa88e4 --- /dev/null +++ b/service/src/main/java/bio/terra/workspace/service/workspace/flight/application/ApplicationAbleDaoStepV3.java @@ -0,0 +1,55 @@ +package bio.terra.workspace.service.workspace.flight.application; + +import bio.terra.stairway.StepResult; +import bio.terra.stairway.exception.RetryException; +import bio.terra.workspace.common.stairway.BaseStep; +import bio.terra.workspace.common.stairway.StepInput; +import bio.terra.workspace.db.ApplicationDao; +import bio.terra.workspace.service.workspace.model.WsmWorkspaceApplication; +import java.util.UUID; + +public class ApplicationAbleDaoStepV3 extends BaseStep { + private final ApplicationDao applicationDao; + private final String applicationId; + @StepInput private UUID workspaceId; + @StepInput private AbleEnum applicationAbleEnum; + @StepInput private boolean applicationAbleDao; + + public ApplicationAbleDaoStepV3(ApplicationDao applicationDao, String applicationId) { + this.applicationDao = applicationDao; + this.applicationId = applicationId; + } + + @Override + public StepResult perform() throws InterruptedException, RetryException { + + // if the application was in the correct database state in precheck, we do nothing + if (applicationAbleDao) { + return StepResult.getStepResultSuccess(); + } + + WsmWorkspaceApplication wsmApp; + if (applicationAbleEnum == AbleEnum.ENABLE) { + wsmApp = applicationDao.enableWorkspaceApplication(workspaceId, applicationId); + } else { + wsmApp = applicationDao.disableWorkspaceApplication(workspaceId, applicationId); + } + setResponse(wsmApp); + return StepResult.getStepResultSuccess(); + } + + @Override + public StepResult undo() throws InterruptedException { + // if the application was in the correct database state in precheck, we do nothing + if (applicationAbleDao) { + return StepResult.getStepResultSuccess(); + } + + if (applicationAbleEnum == AbleEnum.ENABLE) { + applicationDao.disableWorkspaceApplication(workspaceId, applicationId); + } else { + applicationDao.enableWorkspaceApplication(workspaceId, applicationId); + } + return StepResult.getStepResultSuccess(); + } +} diff --git a/service/src/main/java/bio/terra/workspace/service/workspace/flight/application/ApplicationAbleFlightV3.java b/service/src/main/java/bio/terra/workspace/service/workspace/flight/application/ApplicationAbleFlightV3.java new file mode 100644 index 0000000000..a0cf87c677 --- /dev/null +++ b/service/src/main/java/bio/terra/workspace/service/workspace/flight/application/ApplicationAbleFlightV3.java @@ -0,0 +1,31 @@ +package bio.terra.workspace.service.workspace.flight.application; + +import bio.terra.stairway.Flight; +import bio.terra.stairway.FlightMap; +import bio.terra.workspace.common.utils.FlightBeanBag; +import bio.terra.workspace.service.workspace.flight.WorkspaceFlightMapKeys; +import com.fasterxml.jackson.core.type.TypeReference; +import java.util.List; + +public class ApplicationAbleFlightV3 extends Flight { + public ApplicationAbleFlightV3(FlightMap inputParameters, Object applicationContext) { + super(inputParameters, applicationContext); + + FlightBeanBag beanBag = FlightBeanBag.getFromObject(applicationContext); + + // get data from inputs that steps need + + List applicationIdList = + inputParameters.get(WorkspaceFlightMapKeys.APPLICATION_IDS, new TypeReference<>() {}); + + for (String applicationId : applicationIdList) { + addStep( + new ApplicationAblePrecheckStepV3( + beanBag.getApplicationDao(), beanBag.getSamService(), applicationId)); + + addStep(new ApplicationAbleIamStepV3(beanBag.getSamService())); + + addStep(new ApplicationAbleDaoStepV3(beanBag.getApplicationDao(), applicationId)); + } + } +} diff --git a/service/src/main/java/bio/terra/workspace/service/workspace/flight/application/ApplicationAbleIamStepV3.java b/service/src/main/java/bio/terra/workspace/service/workspace/flight/application/ApplicationAbleIamStepV3.java new file mode 100644 index 0000000000..135a0ab669 --- /dev/null +++ b/service/src/main/java/bio/terra/workspace/service/workspace/flight/application/ApplicationAbleIamStepV3.java @@ -0,0 +1,61 @@ +package bio.terra.workspace.service.workspace.flight.application; + +import bio.terra.stairway.StepResult; +import bio.terra.stairway.exception.RetryException; +import bio.terra.workspace.common.stairway.BaseStep; +import bio.terra.workspace.common.stairway.StepInput; +import bio.terra.workspace.service.iam.AuthenticatedUserRequest; +import bio.terra.workspace.service.iam.SamService; +import bio.terra.workspace.service.iam.model.WsmIamRole; +import bio.terra.workspace.service.workspace.model.WsmApplication; +import java.util.UUID; + +public class ApplicationAbleIamStepV3 extends BaseStep { + private final SamService samService; + @StepInput private AuthenticatedUserRequest authUserInfo; + @StepInput private UUID workspaceId; + @StepInput private AbleEnum applicationAbleEnum; + @StepInput private WsmApplication wsmApplication; + @StepInput private boolean applicationAbleSam; + + public ApplicationAbleIamStepV3(SamService samService) { + this.samService = samService; + } + + @Override + public StepResult perform() throws InterruptedException, RetryException { + // if the application was in the correct Sam state in precheck, then we do nothing + if (applicationAbleSam) { + return StepResult.getStepResultSuccess(); + } + + if (applicationAbleEnum == AbleEnum.ENABLE) { + samService.grantWorkspaceRole( + workspaceId, authUserInfo, WsmIamRole.APPLICATION, wsmApplication.getServiceAccount()); + } else { + samService.removeWorkspaceRole( + workspaceId, authUserInfo, WsmIamRole.APPLICATION, wsmApplication.getServiceAccount()); + } + + return StepResult.getStepResultSuccess(); + } + + @Override + public StepResult undo() throws InterruptedException { + + // if the application was not already enabled in Sam when we started, we do not undo it + if (applicationAbleSam) { + return StepResult.getStepResultSuccess(); + } + + if (applicationAbleEnum == AbleEnum.ENABLE) { + samService.removeWorkspaceRole( + workspaceId, authUserInfo, WsmIamRole.APPLICATION, wsmApplication.getServiceAccount()); + } else { + samService.grantWorkspaceRole( + workspaceId, authUserInfo, WsmIamRole.APPLICATION, wsmApplication.getServiceAccount()); + } + + return StepResult.getStepResultSuccess(); + } +} diff --git a/service/src/main/java/bio/terra/workspace/service/workspace/flight/application/ApplicationAblePrecheckStepV3.java b/service/src/main/java/bio/terra/workspace/service/workspace/flight/application/ApplicationAblePrecheckStepV3.java new file mode 100644 index 0000000000..b27cb0970b --- /dev/null +++ b/service/src/main/java/bio/terra/workspace/service/workspace/flight/application/ApplicationAblePrecheckStepV3.java @@ -0,0 +1,74 @@ +package bio.terra.workspace.service.workspace.flight.application; + +import bio.terra.stairway.StepResult; +import bio.terra.stairway.exception.RetryException; +import bio.terra.workspace.common.stairway.BaseStep; +import bio.terra.workspace.common.stairway.StepInput; +import bio.terra.workspace.common.stairway.StepOutput; +import bio.terra.workspace.db.ApplicationDao; +import bio.terra.workspace.db.exception.ApplicationNotFoundException; +import bio.terra.workspace.db.exception.InvalidApplicationStateException; +import bio.terra.workspace.service.iam.AuthenticatedUserRequest; +import bio.terra.workspace.service.iam.SamService; +import bio.terra.workspace.service.workspace.model.WsmApplication; +import bio.terra.workspace.service.workspace.model.WsmApplicationState; +import bio.terra.workspace.service.workspace.model.WsmWorkspaceApplication; +import java.util.UUID; + +// This step is shared by enable and disable to check the current enabled states of the +// application. +public class ApplicationAblePrecheckStepV3 extends BaseStep { + private final ApplicationDao applicationDao; + private final SamService samService; + private final String applicationId; + @StepInput private AuthenticatedUserRequest authUserInfo; + @StepInput private UUID workspaceId; + @StepInput private AbleEnum applicationAbleEnum; + @StepOutput private WsmApplication wsmApplication; + @StepOutput private boolean applicationAbleDao; + @StepOutput private boolean applicationAbleSam; + + public ApplicationAblePrecheckStepV3( + ApplicationDao applicationDao, SamService samService, String applicationId) { + this.applicationDao = applicationDao; + this.samService = samService; + this.applicationId = applicationId; + } + + @Override + public StepResult perform() throws InterruptedException, RetryException { + wsmApplication = applicationDao.getApplication(applicationId); + + // For enable, we require that the application be in the operating state + if (applicationAbleEnum == AbleEnum.ENABLE) { + if (wsmApplication.getState() != WsmApplicationState.OPERATING) { + throw new InvalidApplicationStateException( + "Applications is " + wsmApplication.getState().toApi() + " and cannot be enabled"); + } + } + + // See if the application is enabled + try { + WsmWorkspaceApplication workspaceApp = + applicationDao.getWorkspaceApplication(workspaceId, applicationId); + applicationAbleDao = computeCorrectState(applicationAbleEnum, workspaceApp.isEnabled()); + } catch (ApplicationNotFoundException e) { + applicationAbleDao = computeCorrectState(applicationAbleEnum, false); + } + + // See if the application already has APPLICATION role for the workspace + boolean enabledSam = + samService.isApplicationEnabledInSam( + workspaceId, wsmApplication.getServiceAccount(), authUserInfo); + applicationAbleSam = computeCorrectState(applicationAbleEnum, enabledSam); + + return StepResult.getStepResultSuccess(); + } + + private boolean computeCorrectState(AbleEnum ableEnum, boolean isEnabled) { + if (ableEnum == AbleEnum.ENABLE) { + return isEnabled; + } + return !isEnabled; + } +} diff --git a/service/src/test/java/bio/terra/workspace/service/workspace/flight/application/ApplicationAbleFlightV3Test.java b/service/src/test/java/bio/terra/workspace/service/workspace/flight/application/ApplicationAbleFlightV3Test.java new file mode 100644 index 0000000000..2083a95799 --- /dev/null +++ b/service/src/test/java/bio/terra/workspace/service/workspace/flight/application/ApplicationAbleFlightV3Test.java @@ -0,0 +1,27 @@ +package bio.terra.workspace.service.workspace.flight.application; + + +import bio.terra.stairway.Flight; +import bio.terra.stairway.FlightMap; +import bio.terra.workspace.common.stairway.StepUtils; +import bio.terra.workspace.common.utils.FlightBeanBag; +import bio.terra.workspace.service.workspace.flight.WorkspaceFlightMapKeys; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +import java.util.Arrays; + +import static org.mockito.Mockito.mock; + +@Tag("unit") +class ApplicationAbleFlightV3Test { + @Test + void testDataFlow() { + FlightMap inputParameters = new FlightMap(); + inputParameters.put( + WorkspaceFlightMapKeys.APPLICATION_IDS, + Arrays.asList("applicationId1")); + Flight flight = new ApplicationAbleFlightV3(inputParameters, mock(FlightBeanBag.class)); + StepUtils.generateFlowAnalysisReport(flight).forEach(System.out::println); + } +} \ No newline at end of file