From c8d7659860af3b12d87ef29478bc1b825a9b3d2f Mon Sep 17 00:00:00 2001 From: Stephen Smith Date: Mon, 24 Oct 2022 19:31:45 -0400 Subject: [PATCH] 16073: Refactor process io loading into actions and reducers to eliminate infinite loops Arvados-DCO-1.1-Signed-off-by: Stephen Smith --- .../process-panel/process-panel-actions.ts | 77 ++++++++-- .../process-panel/process-panel-reducer.ts | 40 +++++- src/store/process-panel/process-panel.ts | 15 +- src/store/processes/processes-actions.ts | 1 + src/views/process-panel/process-io-card.tsx | 6 +- .../process-panel/process-panel-root.tsx | 133 ++++++------------ src/views/process-panel/process-panel.tsx | 13 +- 7 files changed, 181 insertions(+), 104 deletions(-) diff --git a/src/store/process-panel/process-panel-actions.ts b/src/store/process-panel/process-panel-actions.ts index 7d7bd426..64fc493f 100644 --- a/src/store/process-panel/process-panel-actions.ts +++ b/src/store/process-panel/process-panel-actions.ts @@ -3,7 +3,7 @@ // SPDX-License-Identifier: AGPL-3.0 import { unionize, ofType, UnionOf } from "common/unionize"; -import { getRawOutputs, loadProcess } from 'store/processes/processes-actions'; +import { getInputs, getOutputParameters, getRawInputs, getRawOutputs, loadProcess } from 'store/processes/processes-actions'; import { Dispatch } from 'redux'; import { ProcessStatus } from 'store/processes/process'; import { RootState } from 'store/store'; @@ -16,11 +16,22 @@ import { loadSubprocessPanel } from "../subprocess-panel/subprocess-panel-action import { initProcessLogsPanel, processLogsPanelActions } from "store/process-logs-panel/process-logs-panel-actions"; import { CollectionFile } from "models/collection-file"; import { ContainerRequestResource } from "models/container-request"; +import { CommandOutputParameter } from 'cwlts/mappings/v1.0/CommandOutputParameter'; +import { CommandInputParameter, getIOParamId } from 'models/workflow'; +import { getIOParamDisplayValue, ProcessIOParameter } from "views/process-panel/process-io-card"; +import { OutputDetails } from "./process-panel"; +import { AuthState } from "store/auth/auth-reducer"; export const processPanelActions = unionize({ + RESET_PROCESS_PANEL: ofType<{}>(), SET_PROCESS_PANEL_CONTAINER_REQUEST_UUID: ofType(), SET_PROCESS_PANEL_FILTERS: ofType(), TOGGLE_PROCESS_PANEL_FILTER: ofType(), + SET_INPUT_RAW: ofType(), + SET_INPUT_PARAMS: ofType(), + SET_OUTPUT_RAW: ofType(), + SET_OUTPUT_DEFINITIONS: ofType(), + SET_OUTPUT_PARAMS: ofType(), }); export type ProcessPanelAction = UnionOf; @@ -29,6 +40,7 @@ export const toggleProcessPanelFilter = processPanelActions.TOGGLE_PROCESS_PANEL export const loadProcessPanel = (uuid: string) => async (dispatch: Dispatch) => { + dispatch(processPanelActions.RESET_PROCESS_PANEL()); dispatch(processLogsPanelActions.RESET_PROCESS_LOGS_PANEL()); dispatch(processPanelActions.SET_PROCESS_PANEL_CONTAINER_REQUEST_UUID(uuid)); await dispatch(loadProcess(uuid)); @@ -47,10 +59,19 @@ export const navigateToOutput = (uuid: string) => } }; -export const loadOutputs = (containerRequest: ContainerRequestResource, setOutputs) => +export const loadInputs = (containerRequest: ContainerRequestResource) => + async (dispatch: Dispatch, getState: () => RootState, services: ServiceRepository) => { + dispatch(processPanelActions.SET_INPUT_RAW(getRawInputs(containerRequest))); + dispatch(processPanelActions.SET_INPUT_PARAMS(formatInputData(getInputs(containerRequest), getState().auth))); + }; + +export const loadOutputs = (containerRequest: ContainerRequestResource) => async (dispatch: Dispatch, getState: () => RootState, services: ServiceRepository) => { const noOutputs = {rawOutputs: {}}; - if (!containerRequest.outputUuid) {setOutputs(noOutputs); return;}; + if (!containerRequest.outputUuid) { + dispatch(processPanelActions.SET_OUTPUT_RAW(noOutputs)); + return; + }; try { const propsOutputs = getRawOutputs(containerRequest); const filesPromise = services.collectionService.files(containerRequest.outputUuid); @@ -59,22 +80,42 @@ export const loadOutputs = (containerRequest: ContainerRequestResource, setOutpu // If has propsOutput, skip fetching cwl.output.json if (propsOutputs !== undefined) { - setOutputs({rawOutputs: propsOutputs, pdh: collection.portableDataHash}); + dispatch(processPanelActions.SET_OUTPUT_RAW({ + rawOutputs: propsOutputs, + pdh: collection.portableDataHash + })); } else { // Fetch outputs from keep const outputFile = files.find((file) => file.name === 'cwl.output.json') as CollectionFile | undefined; let outputData = outputFile ? await services.collectionService.getFileContents(outputFile) : undefined; if (outputData && (outputData = JSON.parse(outputData)) && collection.portableDataHash) { - setOutputs({ + dispatch(processPanelActions.SET_OUTPUT_RAW({ rawOutputs: outputData, pdh: collection.portableDataHash, - }); + })); } else { - setOutputs(noOutputs); + dispatch(processPanelActions.SET_OUTPUT_RAW(noOutputs)); } } } catch { - setOutputs(noOutputs); + dispatch(processPanelActions.SET_OUTPUT_RAW(noOutputs)); + } + }; + +export const loadOutputDefinitions = (containerRequest: ContainerRequestResource) => + async (dispatch: Dispatch, getState: () => RootState, services: ServiceRepository) => { + if (containerRequest && containerRequest.mounts) { + dispatch(processPanelActions.SET_OUTPUT_DEFINITIONS(getOutputParameters(containerRequest))); + } + }; + +export const updateOutputParams = () => + async (dispatch: Dispatch, getState: () => RootState, services: ServiceRepository) => { + const outputDefinitions = getState().processPanel.outputDefinitions; + const outputRaw = getState().processPanel.outputRaw; + + if (outputRaw !== null && outputRaw.rawOutputs) { + dispatch(processPanelActions.SET_OUTPUT_PARAMS(formatOutputData(outputDefinitions, outputRaw.rawOutputs, outputRaw.pdh, getState().auth))); } }; @@ -94,3 +135,23 @@ export const initProcessPanelFilters = processPanelActions.SET_PROCESS_PANEL_FIL ProcessStatus.WARNING, ProcessStatus.CANCELLED ]); + +const formatInputData = (inputs: CommandInputParameter[], auth: AuthState): ProcessIOParameter[] => { + return inputs.map(input => { + return { + id: getIOParamId(input), + label: input.label || "", + value: getIOParamDisplayValue(auth, input) + }; + }); +}; + +const formatOutputData = (definitions: CommandOutputParameter[], values: any, pdh: string | undefined, auth: AuthState): ProcessIOParameter[] => { + return definitions.map(output => { + return { + id: getIOParamId(output), + label: output.label || "", + value: getIOParamDisplayValue(auth, Object.assign(output, { value: values[getIOParamId(output)] || [] }), pdh) + }; + }); +}; diff --git a/src/store/process-panel/process-panel-reducer.ts b/src/store/process-panel/process-panel-reducer.ts index d26e7693..48cdb39f 100644 --- a/src/store/process-panel/process-panel-reducer.ts +++ b/src/store/process-panel/process-panel-reducer.ts @@ -7,11 +7,17 @@ import { ProcessPanelAction, processPanelActions } from 'store/process-panel/pro const initialState: ProcessPanel = { containerRequestUuid: "", - filters: {} + filters: {}, + inputRaw: null, + inputParams: null, + outputRaw: null, + outputDefinitions: [], + outputParams: null, }; export const processPanelReducer = (state = initialState, action: ProcessPanelAction): ProcessPanel => processPanelActions.match(action, { + RESET_PROCESS_PANEL: () => initialState, SET_PROCESS_PANEL_CONTAINER_REQUEST_UUID: containerRequestUuid => ({ ...state, containerRequestUuid }), @@ -23,5 +29,37 @@ export const processPanelReducer = (state = initialState, action: ProcessPanelAc const filters = { ...state.filters, [status]: !state.filters[status] }; return { ...state, filters }; }, + SET_INPUT_RAW: inputRaw => { + // Since mounts can disappear and reappear, only set inputs + // if current state is null or new inputs has content + if (state.inputRaw === null || (inputRaw && inputRaw.length)) { + return { ...state, inputRaw }; + } else { + return state; + } + }, + SET_INPUT_PARAMS: inputParams => { + // Since mounts can disappear and reappear, only set inputs + // if current state is null or new inputs has content + if (state.inputParams === null || (inputParams && inputParams.length)) { + return { ...state, inputParams }; + } else { + return state; + } + }, + SET_OUTPUT_RAW: outputRaw => { + return { ...state, outputRaw }; + }, + SET_OUTPUT_DEFINITIONS: outputDefinitions => { + // Set output definitions is only additive to avoid clearing when mounts go temporarily missing + if (outputDefinitions.length) { + return { ...state, outputDefinitions } + } else { + return state; + } + }, + SET_OUTPUT_PARAMS: outputParams => { + return { ...state, outputParams }; + }, default: () => state, }); diff --git a/src/store/process-panel/process-panel.ts b/src/store/process-panel/process-panel.ts index 49c2691d..d0d5edeb 100644 --- a/src/store/process-panel/process-panel.ts +++ b/src/store/process-panel/process-panel.ts @@ -2,16 +2,29 @@ // // SPDX-License-Identifier: AGPL-3.0 +import { CommandInputParameter } from 'models/workflow'; import { RouterState } from "react-router-redux"; import { matchProcessRoute } from "routes/routes"; +import { ProcessIOParameter } from "views/process-panel/process-io-card"; +import { CommandOutputParameter } from 'cwlts/mappings/v1.0/CommandOutputParameter'; + +export type OutputDetails = { + rawOutputs?: any; + pdh?: string; +} export interface ProcessPanel { containerRequestUuid: string; filters: { [status: string]: boolean }; + inputRaw: CommandInputParameter[] | null; + inputParams: ProcessIOParameter[] | null; + outputRaw: OutputDetails | null; + outputDefinitions: CommandOutputParameter[]; + outputParams: ProcessIOParameter[] | null; } export const getProcessPanelCurrentUuid = (router: RouterState) => { const pathname = router.location ? router.location.pathname : ''; const match = matchProcessRoute(pathname); return match ? match.params.id : undefined; -}; \ No newline at end of file +}; diff --git a/src/store/processes/processes-actions.ts b/src/store/processes/processes-actions.ts index 1deb4cb8..458efa20 100644 --- a/src/store/processes/processes-actions.ts +++ b/src/store/processes/processes-actions.ts @@ -147,6 +147,7 @@ export const getRawInputs = (data: any): CommandInputParameter[] | undefined => } export const getInputs = (data: any): CommandInputParameter[] => { + // Definitions from mounts are needed so we return early if missing if (!data || !data.mounts || !data.mounts[MOUNT_PATH_CWL_WORKFLOW]) { return []; } const content = getRawInputs(data) as any; if (!content) { return []; } diff --git a/src/views/process-panel/process-io-card.tsx b/src/views/process-panel/process-io-card.tsx index 79a7799d..5fd444b5 100644 --- a/src/views/process-panel/process-io-card.tsx +++ b/src/views/process-panel/process-io-card.tsx @@ -210,8 +210,8 @@ export enum ProcessIOCardType { export interface ProcessIOCardDataProps { process: Process; label: ProcessIOCardType; - params?: ProcessIOParameter[]; - raw?: any; + params: ProcessIOParameter[] | null; + raw: any; mounts?: InputCollectionMount[]; outputUuid?: string; } @@ -238,7 +238,7 @@ export const ProcessIOCard = withStyles(styles)(connect(null, mapDispatchToProps const PanelIcon = label === ProcessIOCardType.INPUT ? InputIcon : OutputIcon; const mainProcess = !process.containerRequest.requestingContainerUuid; - const loading = raw === undefined || params === undefined; + const loading = raw === null || raw === undefined || params === null; const hasRaw = !!(raw && Object.keys(raw).length > 0); const hasParams = !!(params && params.length > 0); diff --git a/src/views/process-panel/process-panel-root.tsx b/src/views/process-panel/process-panel-root.tsx index c2267ec0..bc485d9f 100644 --- a/src/views/process-panel/process-panel-root.tsx +++ b/src/views/process-panel/process-panel-root.tsx @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: AGPL-3.0 -import React, { useState } from 'react'; +import React from 'react'; import { Grid, StyleRulesCallback, WithStyles, withStyles } from '@material-ui/core'; import { DefaultView } from 'components/default-view/default-view'; import { ProcessIcon } from 'components/icon/icon'; @@ -12,17 +12,18 @@ import { SubprocessFilterDataProps } from 'components/subprocess-filter/subproce import { MPVContainer, MPVPanelContent, MPVPanelState } from 'components/multi-panel-view/multi-panel-view'; import { ArvadosTheme } from 'common/custom-theme'; import { ProcessDetailsCard } from './process-details-card'; -import { getIOParamDisplayValue, ProcessIOCard, ProcessIOCardType, ProcessIOParameter } from './process-io-card'; +import { ProcessIOCard, ProcessIOCardType, ProcessIOParameter } from './process-io-card'; import { getProcessPanelLogs, ProcessLogsPanel } from 'store/process-logs-panel/process-logs-panel'; import { ProcessLogsCard } from './process-log-card'; import { FilterOption } from 'views/process-panel/process-log-form'; -import { getInputs, getInputCollectionMounts, getOutputParameters, getRawInputs } from 'store/processes/processes-actions'; -import { CommandInputParameter, getIOParamId } from 'models/workflow'; +import { getInputCollectionMounts } from 'store/processes/processes-actions'; +import { CommandInputParameter } from 'models/workflow'; import { CommandOutputParameter } from 'cwlts/mappings/v1.0/CommandOutputParameter'; import { AuthState } from 'store/auth/auth-reducer'; import { ProcessCmdCard } from './process-cmd-card'; import { ContainerRequestResource } from 'models/container-request'; +import { OutputDetails } from 'store/process-panel/process-panel'; type CssRules = 'root'; @@ -38,6 +39,11 @@ export interface ProcessPanelRootDataProps { filters: Array; processLogsPanel: ProcessLogsPanel; auth: AuthState; + inputRaw: CommandInputParameter[] | null; + inputParams: ProcessIOParameter[] | null; + outputRaw: OutputDetails | null; + outputDefinitions: CommandOutputParameter[]; + outputParams: ProcessIOParameter[] | null; } export interface ProcessPanelRootActionProps { @@ -47,16 +53,14 @@ export interface ProcessPanelRootActionProps { onLogFilterChange: (filter: FilterOption) => void; navigateToLog: (uuid: string) => void; onCopyToClipboard: (uuid: string) => void; - fetchOutputs: (containerRequest: ContainerRequestResource, fetchOutputs) => void; + loadInputs: (containerRequest: ContainerRequestResource) => void; + loadOutputs: (containerRequest: ContainerRequestResource) => void; + loadOutputDefinitions: (containerRequest: ContainerRequestResource) => void; + updateOutputParams: () => void; } export type ProcessPanelRootProps = ProcessPanelRootDataProps & ProcessPanelRootActionProps & WithStyles; -type OutputDetails = { - rawOutputs?: any; - pdh?: string; -} - const panelsData: MPVPanelState[] = [ {name: "Details"}, {name: "Command"}, @@ -67,72 +71,41 @@ const panelsData: MPVPanelState[] = [ ]; export const ProcessPanelRoot = withStyles(styles)( - ({ process, auth, processLogsPanel, fetchOutputs, ...props }: ProcessPanelRootProps) => { - - const [outputDetails, setOutputs] = useState(undefined); - const [outputDefinitions, setOutputDefinitions] = useState([]); - const [rawInputs, setInputs] = useState(undefined); - - const [processedOutputs, setProcessedOutputs] = useState(undefined); - const [processedInputs, setProcessedInputs] = useState(undefined); + ({ + process, + auth, + processLogsPanel, + inputRaw, + inputParams, + outputRaw, + outputDefinitions, + outputParams, + loadInputs, + loadOutputs, + loadOutputDefinitions, + updateOutputParams, + ...props + }: ProcessPanelRootProps) => { const outputUuid = process?.containerRequest.outputUuid; - const requestUuid = process?.containerRequest.uuid; - const containerRequest = process?.containerRequest; - const inputMounts = getInputCollectionMounts(process?.containerRequest); - // Resets state when changing processes - React.useEffect(() => { - setOutputs(undefined); - setOutputDefinitions([]); - setInputs(undefined); - setProcessedOutputs(undefined); - setProcessedInputs(undefined); - }, [requestUuid]); - - // Fetch raw output (async for fetching from keep) React.useEffect(() => { if (containerRequest) { - fetchOutputs(containerRequest, setOutputs); - } - }, [containerRequest, fetchOutputs]); - - // Fetch outputDefinitons from mounts whenever containerRequest is updated - React.useEffect(() => { - if (containerRequest && containerRequest.mounts) { - const newOutputDefinitions = getOutputParameters(containerRequest); - // Avoid setting output definitions to [] when mounts briefly go missing - if (newOutputDefinitions.length) { - setOutputDefinitions(newOutputDefinitions); - } + // Load inputs from mounts or props + loadInputs(containerRequest); + // Fetch raw output (loads from props or keep) + loadOutputs(containerRequest); + // Loads output definitions from mounts into store + loadOutputDefinitions(containerRequest); } - }, [containerRequest]); + }, [containerRequest, loadInputs, loadOutputs, loadOutputDefinitions]); - // Format raw output into ProcessIOParameter[] when it changes + // Trigger processing output params when raw or definitions change React.useEffect(() => { - if (outputDetails !== undefined && outputDetails.rawOutputs) { - // Update processed outputs as long as outputDetails is loaded (or failed to load with {} rawOutputs) - setProcessedOutputs(formatOutputData(outputDefinitions, outputDetails.rawOutputs, outputDetails.pdh, auth)); - } - }, [outputDetails, auth, outputDefinitions]); - - // Fetch raw inputs and format into ProcessIOParameter[] - // Can be sync because inputs are either already in containerRequest mounts or props - React.useEffect(() => { - if (containerRequest) { - // Since mounts can disappear and reappear, only set inputs if raw / processed inputs is undefined or new inputs has content - const newRawInputs = getRawInputs(containerRequest); - if (rawInputs === undefined || (newRawInputs && newRawInputs.length)) { - setInputs(newRawInputs); - } - const newInputs = getInputs(containerRequest); - if (processedInputs === undefined || (newInputs && newInputs.length)) { - setProcessedInputs(formatInputData(newInputs, auth)); - } - } - }, [requestUuid, auth, containerRequest, processedInputs, rawInputs]); + updateOutputParams(); + }, [outputRaw, outputDefinitions, updateOutputParams]); return process ? @@ -168,8 +141,8 @@ export const ProcessPanelRoot = withStyles(styles)( @@ -177,8 +150,8 @@ export const ProcessPanelRoot = withStyles(styles)( @@ -196,23 +169,3 @@ export const ProcessPanelRoot = withStyles(styles)( ; } ); - -const formatInputData = (inputs: CommandInputParameter[], auth: AuthState): ProcessIOParameter[] => { - return inputs.map(input => { - return { - id: getIOParamId(input), - label: input.label || "", - value: getIOParamDisplayValue(auth, input) - }; - }); -}; - -const formatOutputData = (definitions: CommandOutputParameter[], values: any, pdh: string | undefined, auth: AuthState): ProcessIOParameter[] => { - return definitions.map(output => { - return { - id: getIOParamId(output), - label: output.label || "", - value: getIOParamDisplayValue(auth, Object.assign(output, { value: values[getIOParamId(output)] || [] }), pdh) - }; - }); -}; diff --git a/src/views/process-panel/process-panel.tsx b/src/views/process-panel/process-panel.tsx index 75e934ab..6e2d75c6 100644 --- a/src/views/process-panel/process-panel.tsx +++ b/src/views/process-panel/process-panel.tsx @@ -18,8 +18,11 @@ import { } from 'store/process-panel/process-panel'; import { groupBy } from 'lodash'; import { + loadInputs, + loadOutputDefinitions, loadOutputs, toggleProcessPanelFilter, + updateOutputParams, } from 'store/process-panel/process-panel-actions'; import { cancelRunningWorkflow } from 'store/processes/processes-actions'; import { navigateToLogCollection, setProcessLogsPanelFilter } from 'store/process-logs-panel/process-logs-panel-actions'; @@ -34,6 +37,11 @@ const mapStateToProps = ({ router, auth, resources, processPanel, processLogsPan filters: getFilters(processPanel, subprocesses), processLogsPanel: processLogsPanel, auth: auth, + inputRaw: processPanel.inputRaw, + inputParams: processPanel.inputParams, + outputRaw: processPanel.outputRaw, + outputDefinitions: processPanel.outputDefinitions, + outputParams: processPanel.outputParams, }; }; @@ -54,7 +62,10 @@ const mapDispatchToProps = (dispatch: Dispatch): ProcessPanelRootActionProps => cancelProcess: (uuid) => dispatch(cancelRunningWorkflow(uuid)), onLogFilterChange: (filter) => dispatch(setProcessLogsPanelFilter(filter.value)), navigateToLog: (uuid) => dispatch(navigateToLogCollection(uuid)), - fetchOutputs: (containerRequest, setOutputs) => dispatch(loadOutputs(containerRequest, setOutputs)), + loadInputs: (containerRequest) => dispatch(loadInputs(containerRequest)), + loadOutputs: (containerRequest) => dispatch(loadOutputs(containerRequest)), + loadOutputDefinitions: (containerRequest) => dispatch(loadOutputDefinitions(containerRequest)), + updateOutputParams: () => dispatch(updateOutputParams()) }); const getFilters = (processPanel: ProcessPanelState, processes: Process[]) => { -- 2.30.2