From 8f19e91b80a2667ca1644931a42c29c14323ab27 Mon Sep 17 00:00:00 2001 From: Victor Rocheleau Date: Thu, 31 Aug 2023 14:58:23 -0400 Subject: [PATCH 1/7] perf: delete variants on project/dataset deletion --- src/modules/metadata/actions.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/modules/metadata/actions.js b/src/modules/metadata/actions.js index 76f545fb0..4b2bbf20e 100644 --- a/src/modules/metadata/actions.js +++ b/src/modules/metadata/actions.js @@ -33,7 +33,6 @@ export const clearDatasetDataType = networkAction((datasetId, dataType) => (disp const serviceUrl = dataType === "variant" ? getState().services.itemsByKind.gohan.url : getState().services.itemsByKind.metadata.url; - console.log(serviceUrl); return { types: DELETE_DATASET_DATA_TYPE, url: `${serviceUrl}/datasets/${datasetId}/data-types/${dataType}`, @@ -122,7 +121,11 @@ export const deleteProject = networkAction(project => (dispatch, getState) => ({ url: `${getState().services.metadataService.url}/api/projects/${project.identifier}`, req: {method: "DELETE"}, err: `Error deleting project '${project.title}'`, // TODO: More user-friendly, detailed error - onSuccess: () => message.success(`Project '${project.title}' deleted!`), + onSuccess: () => { + project.datasets.map(ds => ds.identifier) + .forEach(async (id) => await dispatch(clearDatasetDataType(id, "variant"))); + message.success(`Project '${project.title}' deleted!`); + }, })); export const deleteProjectIfPossible = project => (dispatch, getState) => { @@ -179,6 +182,10 @@ export const deleteProjectDataset = networkAction((project, dataset) => (dispatc url: `${getState().services.metadataService.url}/api/datasets/${dataset.identifier}`, req: {method: "DELETE"}, err: `Error deleting dataset '${dataset.title}'`, + onSuccess: async () => { + // Only delete variants if katsu dataset deletion is a success + await dispatch(clearDatasetDataType(dataset.identifier, "variant")); + } })); export const deleteProjectDatasetIfPossible = (project, dataset) => (dispatch, getState) => { From d6fe54542c63ad77276b3350d43b7d886c9c4448 Mon Sep 17 00:00:00 2001 From: Victor Rocheleau Date: Thu, 31 Aug 2023 19:05:30 +0000 Subject: [PATCH 2/7] lint --- src/modules/metadata/actions.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/modules/metadata/actions.js b/src/modules/metadata/actions.js index 4b2bbf20e..287969306 100644 --- a/src/modules/metadata/actions.js +++ b/src/modules/metadata/actions.js @@ -122,6 +122,7 @@ export const deleteProject = networkAction(project => (dispatch, getState) => ({ req: {method: "DELETE"}, err: `Error deleting project '${project.title}'`, // TODO: More user-friendly, detailed error onSuccess: () => { + // Iter all deleted project's datasets and delete variants in gohan project.datasets.map(ds => ds.identifier) .forEach(async (id) => await dispatch(clearDatasetDataType(id, "variant"))); message.success(`Project '${project.title}' deleted!`); @@ -182,10 +183,8 @@ export const deleteProjectDataset = networkAction((project, dataset) => (dispatc url: `${getState().services.metadataService.url}/api/datasets/${dataset.identifier}`, req: {method: "DELETE"}, err: `Error deleting dataset '${dataset.title}'`, - onSuccess: async () => { - // Only delete variants if katsu dataset deletion is a success - await dispatch(clearDatasetDataType(dataset.identifier, "variant")); - } + // Only delete variants if katsu dataset deletion is a success + onSuccess: async () => await dispatch(clearDatasetDataType(dataset.identifier, "variant")), })); export const deleteProjectDatasetIfPossible = (project, dataset) => (dispatch, getState) => { From 4d422a8816f1905439fe75b91f4fdde5d152be91 Mon Sep 17 00:00:00 2001 From: Victor Rocheleau Date: Thu, 31 Aug 2023 21:36:55 +0000 Subject: [PATCH 3/7] dataset/project deletion actions refact --- src/modules/metadata/actions.js | 41 +++++++++++++++++++++++---------- 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/src/modules/metadata/actions.js b/src/modules/metadata/actions.js index 287969306..0edaadde1 100644 --- a/src/modules/metadata/actions.js +++ b/src/modules/metadata/actions.js @@ -39,6 +39,10 @@ export const clearDatasetDataType = networkAction((datasetId, dataType) => (disp req: { method: "DELETE", }, + onError: (error) => { + // Needs to re throw for project/dataset deletion error handling + throw error; + }, }; }); @@ -121,19 +125,28 @@ export const deleteProject = networkAction(project => (dispatch, getState) => ({ url: `${getState().services.metadataService.url}/api/projects/${project.identifier}`, req: {method: "DELETE"}, err: `Error deleting project '${project.title}'`, // TODO: More user-friendly, detailed error - onSuccess: () => { - // Iter all deleted project's datasets and delete variants in gohan - project.datasets.map(ds => ds.identifier) - .forEach(async (id) => await dispatch(clearDatasetDataType(id, "variant"))); - message.success(`Project '${project.title}' deleted!`); - }, + onSuccess: () => message.success(`Project '${project.title}' deleted!`), })); -export const deleteProjectIfPossible = project => (dispatch, getState) => { +export const deleteProjectIfPossible = project => async (dispatch, getState) => { if (getState().projects.isDeleting) return; - return dispatch(deleteProject(project)); + + // Remove data without destroying project/datasets first + try { + await Promise.all(project.datasets.map(ds => dispatch(clearDatasetDataTypes(ds.identifier)))); + await dispatch(deleteProject(project)); + } catch (err) { + console.error(err); + message.error(`Error deleting project '${project.title}'`); + } }; +export const clearDatasetDataTypes = datasetId => async (dispatch, getState) => { + const dataTypes = Object.values(getState().datasetDataTypes.itemsById[datasetId].itemsById) + .filter(dtDetails => dtDetails.queryable) + .map(dtDetails => dtDetails.id); + await Promise.all(dataTypes.map(async dt => await dispatch(clearDatasetDataType(datasetId, dt)))); +}; const saveProject = networkAction(project => (dispatch, getState) => ({ types: SAVE_PROJECT, @@ -183,15 +196,19 @@ export const deleteProjectDataset = networkAction((project, dataset) => (dispatc url: `${getState().services.metadataService.url}/api/datasets/${dataset.identifier}`, req: {method: "DELETE"}, err: `Error deleting dataset '${dataset.title}'`, - // Only delete variants if katsu dataset deletion is a success - onSuccess: async () => await dispatch(clearDatasetDataType(dataset.identifier, "variant")), })); -export const deleteProjectDatasetIfPossible = (project, dataset) => (dispatch, getState) => { +export const deleteProjectDatasetIfPossible = (project, dataset) => async (dispatch, getState) => { if (getState().projects.isAddingDataset || getState().projects.isSavingDataset || getState().projects.isDeletingDataset) return; - return dispatch(deleteProjectDataset(project, dataset)); + try { + await dispatch(clearDatasetDataTypes(dataset.identifier)); + await dispatch(deleteProjectDataset(project, dataset)); + } catch (err) { + console.error(err); + message.error(`Error deleting dataset '${dataset.title}'`); + } }; From bbdf93bb3c2376d2d7e7ea14c176d91c31a884e0 Mon Sep 17 00:00:00 2001 From: David Lougheed Date: Fri, 1 Sep 2023 11:13:08 -0400 Subject: [PATCH 4/7] refact!: load data types from new service registry & clean up dependents remove hard-coding of service artifacts in some places --- src/components/datasets/Dataset.js | 8 +- src/components/datasets/DatasetDataTypes.js | 6 +- .../discovery/DiscoveryQueryBuilder.js | 5 +- src/components/manager/DataTypeSelect.js | 16 +- .../manager/projects/RoutedProject.js | 8 - src/modules/datasets/actions.js | 31 ++-- src/modules/datasets/reducers.js | 16 +- src/modules/metadata/actions.js | 8 +- src/modules/services/actions.js | 79 ++++------ src/modules/services/reducers.js | 148 +++--------------- src/modules/services/utils.js | 2 + src/propTypes.js | 29 ++-- 12 files changed, 105 insertions(+), 251 deletions(-) create mode 100644 src/modules/services/utils.js diff --git a/src/components/datasets/Dataset.js b/src/components/datasets/Dataset.js index 75fe1ebc9..227f71a47 100644 --- a/src/components/datasets/Dataset.js +++ b/src/components/datasets/Dataset.js @@ -13,8 +13,8 @@ import { } from "../../modules/metadata/actions"; import { - fetchDatasetDataTypesSummaryIfPossible, - fetchDatasetSummaryIfPossible, + fetchDatasetDataTypesSummariesIfPossible, + fetchDatasetSummariesIfPossible, } from "../../modules/datasets/actions"; import {INITIAL_DATA_USE_VALUE} from "../../duo"; @@ -315,8 +315,8 @@ const mapDispatchToProps = (dispatch, ownProps) => ({ deleteProjectDataset: dataset => dispatch(deleteProjectDatasetIfPossible(ownProps.project, dataset)), deleteLinkedFieldSet: (dataset, linkedFieldSet, linkedFieldSetIndex) => dispatch(deleteDatasetLinkedFieldSetIfPossible(dataset, linkedFieldSet, linkedFieldSetIndex)), - fetchDatasetSummary: (datasetId) => dispatch(fetchDatasetSummaryIfPossible(datasetId)), - fetchDatasetDataTypesSummary: (datasetId) => dispatch(fetchDatasetDataTypesSummaryIfPossible(datasetId)), + fetchDatasetSummary: (datasetId) => dispatch(fetchDatasetSummariesIfPossible(datasetId)), + fetchDatasetDataTypesSummary: (datasetId) => dispatch(fetchDatasetDataTypesSummariesIfPossible(datasetId)), }); export default connect(mapStateToProps, mapDispatchToProps)(Dataset); diff --git a/src/components/datasets/DatasetDataTypes.js b/src/components/datasets/DatasetDataTypes.js index 4aecd931a..450604574 100644 --- a/src/components/datasets/DatasetDataTypes.js +++ b/src/components/datasets/DatasetDataTypes.js @@ -5,7 +5,7 @@ import { Button, Col, Row, Table, Typography } from "antd"; import PropTypes from "prop-types"; import { datasetPropTypesShape, projectPropTypesShape } from "../../propTypes"; import { clearDatasetDataType } from "../../modules/metadata/actions"; -import { fetchDatasetDataTypesSummaryIfPossible } from "../../modules/datasets/actions"; +import { fetchDatasetDataTypesSummariesIfPossible } from "../../modules/datasets/actions"; import genericConfirm from "../ConfirmationModal"; import DataTypeSummaryModal from "./datatype/DataTypeSummaryModal"; import { nop } from "../../utils/misc"; @@ -16,7 +16,7 @@ const DatasetDataTypes = React.memo( ({isPrivate, project, dataset, onDatasetIngest}) => { const dispatch = useDispatch(); const datasetDataTypes = useSelector((state) => Object.values( - state.datasetDataTypes.itemsById[dataset.identifier]?.itemsById)); + state.datasetDataTypes.itemsById[dataset.identifier]?.itemsById ?? {})); const datasetSummaries = useSelector((state) => state.datasetSummaries.itemsById[dataset.identifier]); const isFetchingDataset = useSelector( (state) => state.datasetDataTypes.itemsById[dataset.identifier]?.isFetching); @@ -35,7 +35,7 @@ const DatasetDataTypes = React.memo( "will be deleted permanently, and will no longer be available for exploration.", onOk: async () => { await dispatch(clearDatasetDataType(dataset.identifier, dataType.id)); - await dispatch(fetchDatasetDataTypesSummaryIfPossible(dataset.identifier)); + await dispatch(fetchDatasetDataTypesSummariesIfPossible(dataset.identifier)); }, }); }, [dispatch, dataset]); diff --git a/src/components/discovery/DiscoveryQueryBuilder.js b/src/components/discovery/DiscoveryQueryBuilder.js index 38b7067b6..a0fb07c8b 100644 --- a/src/components/discovery/DiscoveryQueryBuilder.js +++ b/src/components/discovery/DiscoveryQueryBuilder.js @@ -254,14 +254,15 @@ DiscoveryQueryBuilder.propTypes = { const mapStateToProps = state => ({ servicesInfo: state.services.items, - dataTypes: state.serviceDataTypes.dataTypesByServiceID, dataTypesByID: state.serviceDataTypes.itemsByID, dataTypesByDataset: state.datasetDataTypes, autoQuery: state.explorer.autoQuery, isFetchingTextSearch: state.explorer.fetchingTextSearch || false, - dataTypesLoading: state.services.isFetching || state.datasetDataTypes.isFetchingAll, + dataTypesLoading: state.services.isFetching + || state.serviceDataTypes.isFetching + || state.datasetDataTypes.isFetchingAll, }); const mapDispatchToProps = (dispatch) => ({ diff --git a/src/components/manager/DataTypeSelect.js b/src/components/manager/DataTypeSelect.js index 3962e34f1..531e3c06f 100644 --- a/src/components/manager/DataTypeSelect.js +++ b/src/components/manager/DataTypeSelect.js @@ -6,17 +6,15 @@ import { useSelector } from "react-redux"; const DataTypeSelect = ({value, workflows, onChange}) => { const [selected, setSelected] = useState(value ?? undefined); const servicesFetching = useSelector((state) => state.services.isFetchingAll); - const workflowsFetching = useSelector((state) => state.serviceWorkflows.isFetchingAll); + const workflowsFetching = useSelector((state) => state.serviceWorkflows.isFetching); const { - itemsByID: dataTypes, - isFetchingAll: isFetchingDataTypes, + items: dataTypes, + isFetching: isFetchingDataTypes, } = useSelector((state) => state.serviceDataTypes); const labels = useMemo(() => { if (!dataTypes) return {}; - return Object.fromEntries( - Object.values(dataTypes).map(dt => [dt.id, dt.label]), - ); + return Object.fromEntries(dataTypes.map(dt => [dt.id, dt.label])); }, dataTypes); useEffect(() => { @@ -34,8 +32,8 @@ const DataTypeSelect = ({value, workflows, onChange}) => { if (!Array.isArray(workflows)) { return []; } - const dataTypes = new Set(workflows.map((w) => w.data_type)); - return Array.from(dataTypes) + const workflowDataTypes = new Set(workflows.map((w) => w.data_type)); + return Array.from(workflowDataTypes) // filter out workflow types for which we have no labels (mcode) .filter(dt => dt in labels) .map((dt) => @@ -43,7 +41,7 @@ const DataTypeSelect = ({value, workflows, onChange}) => { {labels[dt]} ({{dt}}) , ); - }, [workflows, dataTypes, labels]); + }, [workflows, labels]); return ( diff --git a/src/components/manager/projects/RoutedProject.js b/src/components/manager/projects/RoutedProject.js index 22a2536ab..73c686ca7 100644 --- a/src/components/manager/projects/RoutedProject.js +++ b/src/components/manager/projects/RoutedProject.js @@ -150,12 +150,6 @@ RoutedProject.propTypes = { services: PropTypes.arrayOf(serviceInfoPropTypesShape), servicesByID: PropTypes.objectOf(serviceInfoPropTypesShape), - serviceDataTypesByServiceID: PropTypes.objectOf(PropTypes.shape({ - items: PropTypes.array, // TODO: Shape - itemsByID: PropTypes.object, // TODO: Shape - isFetching: PropTypes.bool, - })), - projects: PropTypes.arrayOf(projectPropTypesShape), projectsByID: PropTypes.objectOf(projectPropTypesShape), @@ -178,8 +172,6 @@ const mapStateToProps = state => ({ services: state.services.items, servicesByID: state.services.itemsByID, - serviceDataTypesByServiceID: state.serviceDataTypes.dataTypesByServiceID, - projects: state.projects.items, projectsByID: state.projects.itemsByID, diff --git a/src/modules/datasets/actions.js b/src/modules/datasets/actions.js index 265922b6c..655d486c7 100644 --- a/src/modules/datasets/actions.js +++ b/src/modules/datasets/actions.js @@ -1,28 +1,30 @@ import {beginFlow, createFlowActionTypes, createNetworkActionTypes, endFlow, networkAction} from "../../utils/actions"; +import {getDataServices} from "../services/utils"; -export const FETCHING_DATASETS_DATATYPE = createFlowActionTypes("FETCHING_DATASETS_DATATYPE"); -export const FETCH_DATASET_DATATYPE = createNetworkActionTypes("FETCH_DATASET_DATATYPE"); +export const FETCHING_DATASETS_DATA_TYPES = createFlowActionTypes("FETCHING_DATASETS_DATA_TYPES"); +export const FETCH_DATASET_DATA_TYPES_SUMMARY = createNetworkActionTypes("FETCH_DATASET_DATA_TYPES_SUMMARY"); export const FETCH_DATASET_SUMMARY = createNetworkActionTypes("FETCH_DATASET_SUMMARY"); -const fetchDatasetDataTypeSummary = networkAction((serviceInfo, datasetID) => ({ - types: FETCH_DATASET_DATATYPE, +const fetchDatasetDataTypesSummary = networkAction((serviceInfo, datasetID) => ({ + types: FETCH_DATASET_DATA_TYPES_SUMMARY, params: {serviceInfo, datasetID}, url: `${serviceInfo.url}/datasets/${datasetID}/data-types`, })); -export const fetchDatasetDataTypesSummaryIfPossible = (datasetID) => async (dispatch, getState) => { - if (getState().datasetDataTypes.isFetching) return; - await dispatch(fetchDatasetDataTypeSummary(getState().services.itemsByArtifact.metadata, datasetID)); - await dispatch(fetchDatasetDataTypeSummary(getState().services.itemsByArtifact.gohan, datasetID)); +export const fetchDatasetDataTypesSummariesIfPossible = (datasetID) => async (dispatch, getState) => { + if (getState().datasetDataTypes.isFetchingAll) return; + await Promise.all( + getDataServices(getState()).map(serviceInfo => dispatch(fetchDatasetDataTypesSummary(serviceInfo, datasetID))) + ); }; export const fetchDatasetsDataTypes = () => async (dispatch, getState) => { - dispatch(beginFlow(FETCHING_DATASETS_DATATYPE)); + dispatch(beginFlow(FETCHING_DATASETS_DATA_TYPES)); await Promise.all( Object.keys(getState().projects.datasetsByID).map(datasetID => - dispatch(fetchDatasetDataTypesSummaryIfPossible(datasetID))), + dispatch(fetchDatasetDataTypesSummariesIfPossible(datasetID))), ); - dispatch(endFlow(FETCHING_DATASETS_DATATYPE)); + dispatch(endFlow(FETCHING_DATASETS_DATA_TYPES)); }; const fetchDatasetSummary = networkAction((serviceInfo, datasetID) => ({ @@ -31,8 +33,9 @@ const fetchDatasetSummary = networkAction((serviceInfo, datasetID) => ({ url: `${serviceInfo.url}/datasets/${datasetID}/summary`, })); -export const fetchDatasetSummaryIfPossible = (datasetID) => async (dispatch, getState) => { +export const fetchDatasetSummariesIfPossible = (datasetID) => async (dispatch, getState) => { if (getState().datasetSummaries.isFetching) return; - await dispatch(fetchDatasetSummary(getState().services.itemsByArtifact.metadata, datasetID)); - await dispatch(fetchDatasetSummary(getState().services.itemsByArtifact.gohan, datasetID)); + await Promise.all( + getDataServices(getState()).map(serviceInfo => fetchDatasetSummary(serviceInfo, datasetID)) + ); }; diff --git a/src/modules/datasets/reducers.js b/src/modules/datasets/reducers.js index 614dd82cf..e2af914ed 100644 --- a/src/modules/datasets/reducers.js +++ b/src/modules/datasets/reducers.js @@ -1,4 +1,4 @@ -import {FETCHING_DATASETS_DATATYPE, FETCH_DATASET_DATATYPE, FETCH_DATASET_SUMMARY} from "./actions"; +import {FETCHING_DATASETS_DATA_TYPES, FETCH_DATASET_DATA_TYPES_SUMMARY, FETCH_DATASET_SUMMARY} from "./actions"; export const datasetDataTypes = ( state = { @@ -8,24 +8,24 @@ export const datasetDataTypes = ( action, ) => { switch (action.type) { - case FETCHING_DATASETS_DATATYPE.BEGIN: + case FETCHING_DATASETS_DATA_TYPES.BEGIN: return {...state, isFetchingAll: true}; - case FETCHING_DATASETS_DATATYPE.END: + case FETCHING_DATASETS_DATA_TYPES.END: return {...state, isFetchingAll: false}; - case FETCH_DATASET_DATATYPE.REQUEST:{ + case FETCH_DATASET_DATA_TYPES_SUMMARY.REQUEST:{ const {datasetID} = action; return { ...state, itemsById: { ...state.itemsById, [datasetID]: { - itemsById: {...(state.itemsById[datasetID]?.itemsById ?? {})}, + itemsById: state.itemsById[datasetID]?.itemsById ?? {}, isFetching: true, }, }, }; } - case FETCH_DATASET_DATATYPE.RECEIVE:{ + case FETCH_DATASET_DATA_TYPES_SUMMARY.RECEIVE:{ const {datasetID} = action; const itemsByID = Object.fromEntries(action.data.map(d => [d.id, d])); return { @@ -41,8 +41,8 @@ export const datasetDataTypes = ( }, }; } - case FETCH_DATASET_DATATYPE.FINISH: - case FETCH_DATASET_DATATYPE.ERROR:{ + case FETCH_DATASET_DATA_TYPES_SUMMARY.FINISH: + case FETCH_DATASET_DATA_TYPES_SUMMARY.ERROR:{ const {datasetID} = action; return { ...state, diff --git a/src/modules/metadata/actions.js b/src/modules/metadata/actions.js index 0edaadde1..3047f2261 100644 --- a/src/modules/metadata/actions.js +++ b/src/modules/metadata/actions.js @@ -35,7 +35,7 @@ export const clearDatasetDataType = networkAction((datasetId, dataType) => (disp : getState().services.itemsByKind.metadata.url; return { types: DELETE_DATASET_DATA_TYPE, - url: `${serviceUrl}/datasets/${datasetId}/data-types/${dataType}`, + url: `${serviceUrl}/datasets/${datasetId}/data-types/${dataType.identifier}`, req: { method: "DELETE", }, @@ -142,10 +142,10 @@ export const deleteProjectIfPossible = project => async (dispatch, getState) => }; export const clearDatasetDataTypes = datasetId => async (dispatch, getState) => { + // only clear data types which can yield counts - `queryable` is a proxy for this const dataTypes = Object.values(getState().datasetDataTypes.itemsById[datasetId].itemsById) - .filter(dtDetails => dtDetails.queryable) - .map(dtDetails => dtDetails.id); - await Promise.all(dataTypes.map(async dt => await dispatch(clearDatasetDataType(datasetId, dt)))); + .filter(dtDetails => dtDetails.queryable); + return await Promise.all(dataTypes.map(dt => dispatch(clearDatasetDataType(datasetId, dt)))); }; const saveProject = networkAction(project => (dispatch, getState) => ({ diff --git a/src/modules/services/actions.js b/src/modules/services/actions.js index 556a0e336..0af025c1f 100644 --- a/src/modules/services/actions.js +++ b/src/modules/services/actions.js @@ -21,12 +21,8 @@ export const LOADING_ALL_SERVICE_DATA = createFlowActionTypes("LOADING_ALL_SERVI export const FETCH_BENTO_SERVICES = createNetworkActionTypes("FETCH_BENTO_SERVICES"); export const FETCH_SERVICES = createNetworkActionTypes("FETCH_SERVICES"); - -export const FETCH_SERVICE_DATA_TYPES = createNetworkActionTypes("FETCH_SERVICE_DATA_TYPES"); -export const LOADING_SERVICE_DATA_TYPES = createFlowActionTypes("LOADING_SERVICE_DATA_TYPES"); - -export const FETCH_SERVICE_WORKFLOWS = createNetworkActionTypes("FETCH_SERVICE_WORKFLOWS"); -export const LOADING_SERVICE_WORKFLOWS = createFlowActionTypes("LOADING_SERVICE_WORKFLOWS"); +export const FETCH_DATA_TYPES = createNetworkActionTypes("FETCH_DATA_TYPES"); +export const FETCH_WORKFLOWS = createNetworkActionTypes("FETCH_WORKFLOWS"); export const fetchBentoServices = networkAction(() => ({ @@ -41,64 +37,44 @@ export const fetchServices = networkAction(() => ({ err: "Error fetching services", })); -export const fetchDataServiceDataTypes = networkAction((serviceInfo) => ({ - types: FETCH_SERVICE_DATA_TYPES, - params: {serviceInfo}, - url: `${serviceInfo.url}/data-types`, - err: `Error fetching data types from service '${serviceInfo.name}'`, +export const fetchDataTypes = networkAction(() => ({ + types: FETCH_DATA_TYPES, + url: `${BENTO_PUBLIC_URL}/api/service-registry/data-types`, + err: "Error fetching data types", })); -export const fetchDataServiceWorkflows = networkAction((serviceInfo) => ({ - types: FETCH_SERVICE_WORKFLOWS, - params: {serviceInfo}, - url: `${serviceInfo.url}/workflows`, -})); +export const fetchWorkflows = networkAction(() => ({ + types: FETCH_WORKFLOWS, + url: `${BENTO_PUBLIC_URL}/api/service-registry/workflows`, + err: "Error fetching workflows", +})) export const fetchServicesWithMetadataAndDataTypes = (onServiceFetchFinish) => async (dispatch, getState) => { dispatch(beginFlow(LOADING_ALL_SERVICE_DATA)); // Fetch Services - await Promise.all([dispatch(fetchBentoServices()), dispatch(fetchServices())]); - if (!getState().services.items) { - // Something went wrong, terminate early - dispatch(terminateFlow(LOADING_ALL_SERVICE_DATA)); - return; - } - - // Fetch other data (need metadata first): - - // - Skip services that don't provide data (i.e. no data types/workflows/etc.) - - const dataServicesInfo = getState().services.items.filter(s => s?.type).map(s => { - // Backwards compatibility for: - // - old type ("group:artifact:version") - // - and new ({"group": "...", "artifact": "...", "version": "..."}) - const serviceKind = s.bento?.serviceKind ?? s.type.artifact; - return { - ...s, - bentoService: getState().bentoServices.itemsByKind[serviceKind] ?? null, - }; - }).filter(s => s.bento?.dataService ?? false); - - // - Custom stuff to start - explicitly don't wait for this promise to finish since it runs parallel to this flow. - if (onServiceFetchFinish) onServiceFetchFinish(); - - // - Fetch Data Service Data Types and Workflows await Promise.all([ (async () => { - dispatch(beginFlow(LOADING_SERVICE_DATA_TYPES)); - await Promise.all(dataServicesInfo.map(s => dispatch(fetchDataServiceDataTypes(s)))); - dispatch(endFlow(LOADING_SERVICE_DATA_TYPES)); + await Promise.all([ + dispatch(fetchBentoServices()), + dispatch(fetchServices()), + ]); + // - Custom stuff to start + // - explicitly don't wait for this promise to finish since it runs parallel to this flow. + if (onServiceFetchFinish) onServiceFetchFinish(); })(), - (async () => { - dispatch(beginFlow(LOADING_SERVICE_WORKFLOWS)); - await Promise.all(dataServicesInfo.map(s => dispatch(fetchDataServiceWorkflows(s)))); - dispatch(endFlow(LOADING_SERVICE_WORKFLOWS)); - })(), + dispatch(fetchDataTypes()), + dispatch(fetchWorkflows()), ]); + if (!getState().services.items) { + // Something went wrong, terminate early + dispatch(terminateFlow(LOADING_ALL_SERVICE_DATA)); + return; + } + dispatch(endFlow(LOADING_ALL_SERVICE_DATA)); }; @@ -106,8 +82,7 @@ export const fetchServicesWithMetadataAndDataTypesIfNeeded = (onServiceFetchFini (dispatch, getState) => { const state = getState(); if ((Object.keys(state.bentoServices.itemsByArtifact).length === 0 || state.services.items.length === 0 || - Object.keys(state.serviceDataTypes.dataTypesByServiceID).length === 0) && - !state.services.isFetchingAll) { + state.serviceDataTypes.items.length === 0) && !state.services.isFetchingAll) { return dispatch(fetchServicesWithMetadataAndDataTypes(onServiceFetchFinish)); } }; diff --git a/src/modules/services/reducers.js b/src/modules/services/reducers.js index ed1f7c810..06bb8c9f6 100644 --- a/src/modules/services/reducers.js +++ b/src/modules/services/reducers.js @@ -3,12 +3,8 @@ import { FETCH_BENTO_SERVICES, FETCH_SERVICES, - - FETCH_SERVICE_DATA_TYPES, - LOADING_SERVICE_DATA_TYPES, - - FETCH_SERVICE_WORKFLOWS, - LOADING_SERVICE_WORKFLOWS, + FETCH_DATA_TYPES, + FETCH_WORKFLOWS, } from "./actions"; import {normalizeServiceInfo} from "../../utils/serviceInfo"; @@ -125,108 +121,23 @@ export const services = ( export const serviceDataTypes = ( state = { - isFetchingAll: false, + isFetching: false, itemsByID: {}, - dataTypesByServiceID: {}, - dataTypesByServiceArtifact: {}, - dataTypesByServiceKind: {}, + items: [], }, action, ) => { switch (action.type) { - case LOADING_SERVICE_DATA_TYPES.BEGIN: - return {...state, isFetchingAll: true}; - - case LOADING_SERVICE_DATA_TYPES.END: - case LOADING_SERVICE_DATA_TYPES.TERMINATE: - return {...state, isFetchingAll: false}; - - case FETCH_SERVICE_DATA_TYPES.REQUEST: { - const {serviceInfo} = action; - const kind = serviceInfo.bento?.serviceKind ?? serviceInfo.type.artifact; - return { - ...state, - dataTypesByServiceID: { - ...state.dataTypesByServiceID, - [serviceInfo.id]: { - ...(state.dataTypesByServiceID[serviceInfo.id] ?? {items: null, itemsByID: null}), - isFetching: true, - }, - }, - dataTypesByServiceArtifact: { - ...state.dataTypesByServiceArtifact, - [serviceInfo.type.artifact]: { - ...(state.dataTypesByServiceArtifact[serviceInfo.type.artifact] ?? - {items: null, itemsByID: null}), - isFetching: true, - }, - }, - dataTypesByServiceKind: { - ...state.dataTypesByServiceKind, - [kind]: { - ...(state.dataTypesByServiceKind[kind] ?? {items: null, itemsByID: null}), - isFetching: true, - }, - }, - }; - } - - case FETCH_SERVICE_DATA_TYPES.RECEIVE: { - const {serviceInfo} = action; - const artifact = serviceInfo.type.artifact; - const kind = serviceInfo.bento?.serviceKind ?? artifact; - const itemsByID = Object.fromEntries(action.data.map(d => [d.id, d])); - return { - ...state, - itemsByID: { - ...state.itemsByID, - ...itemsByID, - }, - dataTypesByServiceID: { - ...state.dataTypesByServiceID, - [serviceInfo.id]: {items: action.data, itemsByID, isFetching: false}, - }, - dataTypesByServiceArtifact: { - ...state.dataTypesByServiceArtifact, - [artifact]: {items: action.data, itemsByID, isFetching: false}, - }, - dataTypesByServiceKind: { - ...state.dataTypesByServiceKind, - [kind]: {items: action.data, itemsByID, isFetching: false}, - }, - lastUpdated: action.receivedAt, - }; - } - - case FETCH_SERVICE_DATA_TYPES.ERROR: { - const {serviceInfo} = action; - const artifact = serviceInfo.type.artifact; - const kind = serviceInfo.bento?.serviceKind ?? artifact; + case FETCH_DATA_TYPES.REQUEST: + return {...state, isFetching: true}; + case FETCH_DATA_TYPES.RECEIVE: return { ...state, - dataTypesByServiceID: { - ...state.dataTypesByServiceID, - [action.serviceID]: { - ...(state.dataTypesByServiceID[serviceInfo.id] ?? {items: null, itemsByID: null}), - isFetching: false, - }, - }, - dataTypesByServiceArtifact: { - ...state.dataTypesByServiceArtifact, - [artifact]: { - ...(state.dataTypesByServiceArtifact[artifact] ?? {items: null, itemsByID: null}), - isFetching: false, - }, - }, - dataTypesByServiceKind: { - ...state.dataTypesByServiceArtifact, - [kind]: { - ...(state.dataTypesByServiceArtifact[kind] ?? {items: null, itemsByID: null}), - isFetching: false, - }, - }, + items: action.data, + itemsByID: Object.fromEntries(action.data.map(dt => [dt.id, dt])), }; - } + case FETCH_DATA_TYPES.FINISH: + return {...state, isFetching: false}; default: return state; @@ -235,47 +146,24 @@ export const serviceDataTypes = ( export const serviceWorkflows = ( state = { - isFetchingAll: false, - workflowsByServiceID: {}, + isFetching: false, + items: {}, // by purpose and then by workflow ID }, action, ) => { switch (action.type) { - case LOADING_SERVICE_WORKFLOWS.BEGIN: - return {...state, isFetchingAll: true}; - - case LOADING_SERVICE_WORKFLOWS.END: - case LOADING_SERVICE_WORKFLOWS.TERMINATE: - return {...state, isFetchingAll: false}; - - case FETCH_SERVICE_WORKFLOWS.REQUEST: { - const {serviceInfo: {id: serviceID}} = action; + case FETCH_WORKFLOWS.REQUEST: + return {...state, isFetching: true}; + case FETCH_WORKFLOWS.RECEIVE: return { ...state, - workflowsByServiceID: { - ...state.workflowsByServiceID, - [serviceID]: { - isFetching: true, - ...(state.workflowsByServiceID[serviceID] ?? {workflows: null}), - }, - }, + items: action.data, }; - } - - case FETCH_SERVICE_WORKFLOWS.RECEIVE: { - const {serviceInfo: {id: serviceID}, data} = action; + case FETCH_WORKFLOWS.FINISH: return { ...state, isFetching: false, - workflowsByServiceID: { - ...state.workflowsByServiceID, - [serviceID]: {isFetching: false, workflows: data}, - }, }; - } - - case FETCH_SERVICE_WORKFLOWS.ERROR: - return {...state, isFetching: false}; default: return state; diff --git a/src/modules/services/utils.js b/src/modules/services/utils.js new file mode 100644 index 000000000..5b8777661 --- /dev/null +++ b/src/modules/services/utils.js @@ -0,0 +1,2 @@ +export const getDataServices = (state) => + state.services.items.filter(serviceInfo => serviceInfo.bento?.dataService ?? false); diff --git a/src/propTypes.js b/src/propTypes.js index 45f33c717..39c8a4a63 100644 --- a/src/propTypes.js +++ b/src/propTypes.js @@ -161,26 +161,21 @@ export const workflowsStateToPropsMixin = state => { export: [], }; - Object.entries(state.serviceWorkflows.workflowsByServiceID) - .filter(([_, s]) => !s.isFetching) - .forEach(([serviceID, s]) => { - Object.entries(s.workflows).forEach(([workflowType, workflowTypeWorkflows]) => { - if (!(workflowType in workflowsByType)) return; - - // noinspection JSCheckFunctionSignatures - workflowsByType[workflowType].push( - ...Object.entries(workflowTypeWorkflows).map(([k, v]) => ({ - ...v, - id: k, - serviceID, - })), - ); - }); - }); + Object.entries(state.serviceWorkflows.items).forEach(([workflowType, workflowTypeWorkflows]) => { + if (!(workflowType in workflowsByType)) return; + + // noinspection JSCheckFunctionSignatures + workflowsByType[workflowType].push( + ...Object.entries(workflowTypeWorkflows).map(([k, v]) => ({ + ...v, + id: k, + })), + ); + }); return { workflows: workflowsByType, - workflowsLoading: state.services.isFetchingAll || state.serviceWorkflows.isFetchingAll, + workflowsLoading: state.services.isFetchingAll || state.serviceWorkflows.isFetching, }; }; From bb800be10e79854d997312068b5b04c5acc6f178 Mon Sep 17 00:00:00 2001 From: David Lougheed Date: Fri, 1 Sep 2023 11:15:18 -0400 Subject: [PATCH 5/7] lint --- src/modules/datasets/actions.js | 4 ++-- src/modules/services/actions.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/modules/datasets/actions.js b/src/modules/datasets/actions.js index 655d486c7..b6dcfdb77 100644 --- a/src/modules/datasets/actions.js +++ b/src/modules/datasets/actions.js @@ -14,7 +14,7 @@ const fetchDatasetDataTypesSummary = networkAction((serviceInfo, datasetID) => ( export const fetchDatasetDataTypesSummariesIfPossible = (datasetID) => async (dispatch, getState) => { if (getState().datasetDataTypes.isFetchingAll) return; await Promise.all( - getDataServices(getState()).map(serviceInfo => dispatch(fetchDatasetDataTypesSummary(serviceInfo, datasetID))) + getDataServices(getState()).map(serviceInfo => dispatch(fetchDatasetDataTypesSummary(serviceInfo, datasetID))), ); }; @@ -36,6 +36,6 @@ const fetchDatasetSummary = networkAction((serviceInfo, datasetID) => ({ export const fetchDatasetSummariesIfPossible = (datasetID) => async (dispatch, getState) => { if (getState().datasetSummaries.isFetching) return; await Promise.all( - getDataServices(getState()).map(serviceInfo => fetchDatasetSummary(serviceInfo, datasetID)) + getDataServices(getState()).map(serviceInfo => fetchDatasetSummary(serviceInfo, datasetID)), ); }; diff --git a/src/modules/services/actions.js b/src/modules/services/actions.js index 0af025c1f..c7f265aeb 100644 --- a/src/modules/services/actions.js +++ b/src/modules/services/actions.js @@ -47,7 +47,7 @@ export const fetchWorkflows = networkAction(() => ({ types: FETCH_WORKFLOWS, url: `${BENTO_PUBLIC_URL}/api/service-registry/workflows`, err: "Error fetching workflows", -})) +})); export const fetchServicesWithMetadataAndDataTypes = (onServiceFetchFinish) => async (dispatch, getState) => { From 7f3cbe88872c03f13ed0a36603585e7989553548 Mon Sep 17 00:00:00 2001 From: David Lougheed Date: Fri, 1 Sep 2023 11:54:19 -0400 Subject: [PATCH 6/7] fix: data type clearing --- src/modules/metadata/actions.js | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/modules/metadata/actions.js b/src/modules/metadata/actions.js index 3047f2261..9e692b9a3 100644 --- a/src/modules/metadata/actions.js +++ b/src/modules/metadata/actions.js @@ -28,14 +28,11 @@ export const FETCH_OVERVIEW_SUMMARY = createNetworkActionTypes("FETCH_OVERVIEW_S export const DELETE_DATASET_DATA_TYPE = createNetworkActionTypes("DELETE_DATASET_DATA_TYPE"); -export const clearDatasetDataType = networkAction((datasetId, dataType) => (dispatch, getState) => { - // TODO: more robust mapping from dataType to url. - const serviceUrl = dataType === "variant" - ? getState().services.itemsByKind.gohan.url - : getState().services.itemsByKind.metadata.url; +export const clearDatasetDataType = networkAction((datasetId, dataTypeID) => (dispatch, getState) => { + const {service_base_url: serviceBaseUrl} = getState().serviceDataTypes.itemsByID[dataTypeID]; return { types: DELETE_DATASET_DATA_TYPE, - url: `${serviceUrl}/datasets/${datasetId}/data-types/${dataType.identifier}`, + url: `${serviceBaseUrl}datasets/${datasetId}/data-types/${dataTypeID}`, req: { method: "DELETE", }, @@ -144,8 +141,8 @@ export const deleteProjectIfPossible = project => async (dispatch, getState) => export const clearDatasetDataTypes = datasetId => async (dispatch, getState) => { // only clear data types which can yield counts - `queryable` is a proxy for this const dataTypes = Object.values(getState().datasetDataTypes.itemsById[datasetId].itemsById) - .filter(dtDetails => dtDetails.queryable); - return await Promise.all(dataTypes.map(dt => dispatch(clearDatasetDataType(datasetId, dt)))); + .filter(dt => dt.queryable); + return await Promise.all(dataTypes.map(dt => dispatch(clearDatasetDataType(datasetId, dt.id)))); }; const saveProject = networkAction(project => (dispatch, getState) => ({ From f7c61a216ef0ae50d1ace435a42cb99cb88629c1 Mon Sep 17 00:00:00 2001 From: David Lougheed Date: Fri, 1 Sep 2023 11:58:43 -0400 Subject: [PATCH 7/7] fix: missing ADD_PROJECT_DATASET case in reducer --- src/modules/metadata/reducers.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/modules/metadata/reducers.js b/src/modules/metadata/reducers.js index 26e2653fb..0614fcdf7 100644 --- a/src/modules/metadata/reducers.js +++ b/src/modules/metadata/reducers.js @@ -146,6 +146,9 @@ export const projects = ( }; } + case ADD_PROJECT_DATASET.FINISH: + return {...state, isAddingDataset: false}; + // DELETE_PROJECT_DATASET case DELETE_PROJECT_DATASET.REQUEST: