From f60b872b14ce50b96ec7993d2b3b7c96adf27ded Mon Sep 17 00:00:00 2001 From: Xiao Gui <xgui3783@gmail.com> Date: Thu, 19 Nov 2020 10:53:34 +0100 Subject: [PATCH] bugfix: race con bw atlas and tmpl --- common/constants.js | 6 +- .../atlasViewer.apiService.service.ts | 5 +- src/main.module.ts | 3 +- src/services/state/uiState/ui.effects.ts | 11 +- .../state/viewerState.store.helper.ts | 10 -- src/services/state/viewerState.store.ts | 26 ---- src/services/state/viewerState/selectors.ts | 4 + .../viewerState.useEffect.spec.ts | 121 +++++++++++++++++- .../viewerState.useEffect.ts | 50 +++++++- 9 files changed, 189 insertions(+), 47 deletions(-) diff --git a/common/constants.js b/common/constants.js index 8c0565a7d..69ffc91ca 100644 --- a/common/constants.js +++ b/common/constants.js @@ -62,6 +62,10 @@ exports.CONST = { MULTI_REGION_SELECTION: `Multi region selection`, REGIONAL_FEATURES: 'Regional features', - NO_ADDIONTAL_INFO_AVAIL: `Currently, no additional information is linked to this region.` + NO_ADDIONTAL_INFO_AVAIL: `Currently, no additional information is linked to this region.`, + + ATLAS_NOT_FOUND: `Atlas not found. Maybe it is still loading. Try again in a few seconds?`, + TEMPLATE_NOT_FOUND: `Template not found. Maybe it is still loading. Try again in a few seconds?`, + PARC_NOT_FOUND: `` } })(typeof exports === 'undefined' ? module.exports : exports) diff --git a/src/atlasViewer/atlasViewer.apiService.service.ts b/src/atlasViewer/atlasViewer.apiService.service.ts index 7277a3ebb..d4872bb82 100644 --- a/src/atlasViewer/atlasViewer.apiService.service.ts +++ b/src/atlasViewer/atlasViewer.apiService.service.ts @@ -5,6 +5,7 @@ import { Observable, Subject, Subscription, from, race, of, } from "rxjs"; import { distinctUntilChanged, map, filter, startWith, switchMap, catchError, mapTo, take } from "rxjs/operators"; import { DialogService } from "src/services/dialogService.service"; import { uiStateMouseOverSegmentsSelector } from "src/services/state/uiState/selectors"; +import { viewerStateFetchedTemplatesSelector } from "src/services/state/viewerState/selectors"; import { getLabelIndexMap, getMultiNgIdsRegionsLabelIndexMap, @@ -197,9 +198,7 @@ export class AtlasViewerAPIServices implements OnDestroy{ } this.loadedTemplates$ = this.store.pipe( - select('viewerState'), - safeFilter('fetchedTemplates'), - map(state => state.fetchedTemplates), + select(viewerStateFetchedTemplatesSelector) ) this.selectParcellation$ = this.store.pipe( diff --git a/src/main.module.ts b/src/main.module.ts index 75ecc9773..7dbd23fa5 100644 --- a/src/main.module.ts +++ b/src/main.module.ts @@ -54,7 +54,7 @@ import 'src/res/css/extra_styles.css' import 'src/res/css/version.css' import 'src/theme.scss' import { DatasetPreviewGlue, datasetPreviewMetaReducer, IDatasetPreviewGlue, GlueEffects, ClickInterceptorService } from './glue'; -import { viewerStateHelperReducer, viewerStateFleshOutDetail, viewerStateMetaReducers, ViewerStateHelperEffect } from './services/state/viewerState.store.helper'; +import { viewerStateHelperReducer, viewerStateMetaReducers, ViewerStateHelperEffect } from './services/state/viewerState.store.helper'; import { TOS_OBS_INJECTION_TOKEN } from './ui/kgtos/kgtos.component'; import { UiEffects } from './services/state/uiState/ui.effects'; @@ -113,7 +113,6 @@ export function debug(reducer: ActionReducer<any>): ActionReducer<any> { // debug, ...viewerStateMetaReducers, datasetPreviewMetaReducer, - viewerStateFleshOutDetail ] }), HttpClientModule, diff --git a/src/services/state/uiState/ui.effects.ts b/src/services/state/uiState/ui.effects.ts index 2f9287652..128655ad5 100644 --- a/src/services/state/uiState/ui.effects.ts +++ b/src/services/state/uiState/ui.effects.ts @@ -1,4 +1,5 @@ import { Injectable, OnDestroy } from "@angular/core"; +import { MatSnackBar } from "@angular/material/snack-bar"; import { Actions, ofType } from "@ngrx/effects"; import { Subscription } from "rxjs"; import { generalActionError } from "src/services/stateStore.helper"; @@ -11,11 +12,17 @@ export class UiEffects implements OnDestroy{ private subscriptions: Subscription[] = [] - constructor(private actions$: Actions){ + constructor( + private actions$: Actions, + snackBar: MatSnackBar + ){ this.subscriptions.push( this.actions$.pipe( ofType(generalActionError.type) - ).subscribe(console.log) + ).subscribe((payload: any) => { + if (!payload.message) console.log(payload) + snackBar.open(payload.message || `Error: cannot complete your action.`, 'Dismiss', { duration: 5000 }) + }) ) } diff --git a/src/services/state/viewerState.store.helper.ts b/src/services/state/viewerState.store.helper.ts index 307ea047f..8bfe2d676 100644 --- a/src/services/state/viewerState.store.helper.ts +++ b/src/services/state/viewerState.store.helper.ts @@ -153,16 +153,6 @@ export const viewerStateHelperReducer = createReducer( export const viewerStateHelperStoreName = 'viewerStateHelper' -export function viewerStateFleshOutDetail(reducer: ActionReducer<any>): ActionReducer<any> { - return (state, action) => { - if (action.type === viewerStateSelectAtlas.type) { - const reconstitutedAtlas = state[viewerStateHelperStoreName].fetchedAtlases.find(a => a['@id'] === (action as any).atlas['@id']) - return reducer(state, { type: action.type, atlas: reconstitutedAtlas } as any) - } - return reducer(state, action) - } -} - export const defaultState = initialState interface IVersion{ diff --git a/src/services/state/viewerState.store.ts b/src/services/state/viewerState.store.ts index b28168b9e..36435f358 100644 --- a/src/services/state/viewerState.store.ts +++ b/src/services/state/viewerState.store.ts @@ -87,32 +87,6 @@ export const defaultState: StateInterface = { export const getStateStore = ({ state = defaultState } = {}) => (prevState: Partial<StateInterface> = state, action: ActionInterface) => { switch (action.type) { - /** - * glue code. in future, viewerStateSelectAtlas should load templates and parcellations by it self - */ - case viewerStateSelectAtlas.type: { - const { fetchedTemplates } = prevState - /** - * selecting atlas means selecting the first available templateSpace - */ - const atlas = (action as any).atlas - const templateTobeSelected = atlas.templateSpaces[0] - const templateSpaceId = templateTobeSelected['@id'] - - const parcellationId = ( - templateTobeSelected.availableIn.find(p => !!p.baseLayer) || - templateTobeSelected.availableIn[0] - )['@id'] - - const templateSelected = fetchedTemplates.find(t => templateSpaceId === t['@id']) - const parcellationSelected = templateSelected.parcellations.find(p => p['@id'] === parcellationId) - - return { - ...prevState, - templateSelected, - parcellationSelected - } - } /** * TODO may be obsolete. test when nifti become available */ diff --git a/src/services/state/viewerState/selectors.ts b/src/services/state/viewerState/selectors.ts index 875901dd4..bb2bd592e 100644 --- a/src/services/state/viewerState/selectors.ts +++ b/src/services/state/viewerState/selectors.ts @@ -99,6 +99,10 @@ export const viewerStateGetOverlayingAdditionalParcellations = createSelector( } ) +export const viewerStateFetchedAtlasesSelector = createSelector( + state => state[viewerStateHelperStoreName], + helperState => helperState['fetchedAtlases'] +) export const viewerStateGetSelectedAtlas = createSelector( state => state[viewerStateHelperStoreName], diff --git a/src/ui/viewerStateController/viewerState.useEffect.spec.ts b/src/ui/viewerStateController/viewerState.useEffect.spec.ts index d720dfa2c..b38a7db90 100644 --- a/src/ui/viewerStateController/viewerState.useEffect.spec.ts +++ b/src/ui/viewerStateController/viewerState.useEffect.spec.ts @@ -3,7 +3,7 @@ import { Observable, of } from 'rxjs' import { TestBed, async } from '@angular/core/testing' import { provideMockActions } from '@ngrx/effects/testing' import { MockStore, provideMockStore } from '@ngrx/store/testing' -import { defaultRootState, generalActionError, NEWVIEWER } from 'src/services/stateStore.service' +import { defaultRootState, generalActionError } from 'src/services/stateStore.service' import { Injectable } from '@angular/core' import { TemplateCoordinatesTransformation, ITemplateCoordXformResp } from 'src/services/templateCoordinatesTransformation.service' import { hot } from 'jasmine-marbles' @@ -11,7 +11,9 @@ import { AngularMaterialModule } from '../sharedModules/angularMaterial.module' import { HttpClientModule } from '@angular/common/http' import { WidgetModule } from 'src/widget' import { PluginModule } from 'src/atlasViewer/pluginUnit/plugin.module' -import { viewerStateNavigateToRegion, viewerStateNavigationStateSelector, viewerStateNewViewer, viewerStateSelectTemplateWithName } from 'src/services/state/viewerState.store.helper' +import { viewerStateFetchedTemplatesSelector, viewerStateNavigateToRegion, viewerStateNavigationStateSelector, viewerStateNewViewer, viewerStateSelectAtlas, viewerStateSelectTemplateWithName } from 'src/services/state/viewerState.store.helper' +import { viewerStateFetchedAtlasesSelector } from 'src/services/state/viewerState/selectors' +import { CONST } from 'common/constants' const bigbrainJson = require('!json-loader!src/res/ext/bigbrain.json') const bigBrainNehubaConfig = require('!json-loader!src/res/ext/bigbrainNehubaConfig.json') @@ -346,6 +348,121 @@ describe('> viewerState.useEffect.ts', () => { }) }) }) + + describe('> onSelectAtlasSelectTmplParc$', () => { + let mockStore: MockStore + beforeEach(() => { + mockStore = TestBed.inject(MockStore) + }) + + it('> if atlas not found, return general error', () => { + mockStore.overrideSelector(viewerStateFetchedTemplatesSelector, []) + mockStore.overrideSelector(viewerStateFetchedAtlasesSelector, []) + actions$ = hot('a', { + a: viewerStateSelectAtlas({ + atlas: { + ['@id']: 'foo-bar', + } + }) + }) + + const viewerSTateCtrlEffect = TestBed.inject(ViewerStateControllerUseEffect) + expect( + viewerSTateCtrlEffect.onSelectAtlasSelectTmplParc$ + ).toBeObservable( + hot('a', { + a: generalActionError({ + message: CONST.ATLAS_NOT_FOUND + }) + }) + ) + }) + + describe('> if atlas found, will try to find id of first template', () => { + const mockParc1 = { + ['@id']: 'parc-1', + availableIn: [{ + ['@id']: 'test-1' + }] + } + const mockParc0 = { + ['@id']: 'parc-0', + availableIn: [{ + ['@id']: 'hello world' + }] + } + const mockTmplSpc = { + ['@id']: 'hello world', + availableIn: [ mockParc0 ] + } + const mockTmplSpc1 = { + ['@id']: 'test-1', + availableIn: [ mockParc1 ] + } + it('> if fails, will return general error', () => { + + mockStore.overrideSelector(viewerStateFetchedTemplatesSelector, [ + mockTmplSpc1 + ]) + mockStore.overrideSelector(viewerStateFetchedAtlasesSelector, [{ + ['@id']: 'foo-bar', + templateSpaces: [ mockTmplSpc ] + }]) + actions$ = hot('a', { + a: viewerStateSelectAtlas({ + atlas: { + ['@id']: 'foo-bar', + } + }) + }) + + const viewerSTateCtrlEffect = TestBed.inject(ViewerStateControllerUseEffect) + expect( + viewerSTateCtrlEffect.onSelectAtlasSelectTmplParc$ + ).toBeObservable( + hot('a', { + a: generalActionError({ + message: CONST.TEMPLATE_NOT_FOUND + }) + }) + ) + }) + + it('> if succeeds, will dispatch new viewer', () => { + const completeMocktmpl = { + ...mockTmplSpc1, + parcellations: [ mockParc1 ] + } + mockStore.overrideSelector(viewerStateFetchedTemplatesSelector, [ + completeMocktmpl + ]) + mockStore.overrideSelector(viewerStateFetchedAtlasesSelector, [{ + ['@id']: 'foo-bar', + templateSpaces: [ mockTmplSpc1 ] + }]) + actions$ = hot('a', { + a: viewerStateSelectAtlas({ + atlas: { + ['@id']: 'foo-bar', + } + }) + }) + + const viewerSTateCtrlEffect = TestBed.inject(ViewerStateControllerUseEffect) + expect( + viewerSTateCtrlEffect.onSelectAtlasSelectTmplParc$ + ).toBeObservable( + hot('a', { + a: viewerStateNewViewer({ + selectTemplate: completeMocktmpl, + selectParcellation: mockParc1 + }) + }) + ) + }) + }) + }) + }) describe('> cvtNehubaConfigToNavigationObj', () => { diff --git a/src/ui/viewerStateController/viewerState.useEffect.ts b/src/ui/viewerStateController/viewerState.useEffect.ts index f5ed55c04..18eef3fea 100644 --- a/src/ui/viewerStateController/viewerState.useEffect.ts +++ b/src/ui/viewerStateController/viewerState.useEffect.ts @@ -7,12 +7,14 @@ import { CHANGE_NAVIGATION, FETCHED_TEMPLATE, IavRootStoreInterface, SELECT_PARC import { VIEWERSTATE_CONTROLLER_ACTION_TYPES } from "./viewerState.base"; import { TemplateCoordinatesTransformation } from "src/services/templateCoordinatesTransformation.service"; import { CLEAR_STANDALONE_VOLUMES } from "src/services/state/viewerState.store"; -import { viewerStateToggleRegionSelect, viewerStateHelperSelectParcellationWithId, viewerStateSelectTemplateWithId, viewerStateNavigateToRegion, viewerStateSelectedTemplateSelector, viewerStateFetchedTemplatesSelector, viewerStateNewViewer, viewerStateSelectedParcellationSelector, viewerStateNavigationStateSelector, viewerStateSelectTemplateWithName, viewerStateSelectedRegionsSelector } from "src/services/state/viewerState.store.helper"; +import { viewerStateToggleRegionSelect, viewerStateHelperSelectParcellationWithId, viewerStateSelectTemplateWithId, viewerStateNavigateToRegion, viewerStateSelectedTemplateSelector, viewerStateFetchedTemplatesSelector, viewerStateNewViewer, viewerStateSelectedParcellationSelector, viewerStateNavigationStateSelector, viewerStateSelectTemplateWithName, viewerStateSelectedRegionsSelector, viewerStateSelectAtlas } from "src/services/state/viewerState.store.helper"; import { ngViewerSelectorClearViewEntries } from "src/services/state/ngViewerState/selectors"; import { ngViewerActionClearView } from "src/services/state/ngViewerState/actions"; import { PureContantService } from "src/util"; import { verifyPositionArg } from 'common/util' +import { CONST } from 'common/constants' import { uiActionHideAllDatasets } from "src/services/state/uiState/actions"; +import { viewerStateFetchedAtlasesSelector } from "src/services/state/viewerState/selectors"; const defaultPerspectiveZoom = 1e6 const defaultZoom = 1e6 @@ -115,6 +117,52 @@ export class ViewerStateControllerUseEffect implements OnDestroy { }), ) + @Effect() + public onSelectAtlasSelectTmplParc$ = this.actions$.pipe( + ofType(viewerStateSelectAtlas.type), + withLatestFrom( + this.store$.pipe( + select(viewerStateFetchedTemplatesSelector), + startWith([]) + ), + this.store$.pipe( + select(viewerStateFetchedAtlasesSelector), + startWith([]) + ) + ), + map(([action, fetchedTemplates, fetchedAtlases ])=> { + + const atlas = fetchedAtlases.find(a => a['@id'] === (action as any).atlas['@id']) + if (!atlas) { + return generalActionError({ + message: CONST.ATLAS_NOT_FOUND + }) + } + /** + * selecting atlas means selecting the first available templateSpace + */ + const templateTobeSelected = atlas.templateSpaces[0] + const templateSpaceId = templateTobeSelected['@id'] + + const parcellationId = ( + templateTobeSelected.availableIn.find(p => !!p.baseLayer) || + templateTobeSelected.availableIn[0] + )['@id'] + + const templateSelected = fetchedTemplates.find(t => templateSpaceId === t['@id']) + if (!templateSelected) { + return generalActionError({ + message: CONST.TEMPLATE_NOT_FOUND + }) + } + const parcellationSelected = templateSelected.parcellations.find(p => p['@id'] === parcellationId) + return viewerStateNewViewer({ + selectTemplate: templateSelected, + selectParcellation: parcellationSelected + }) + }) + ) + /** * on region selected change (clear, select, or change selection), clear selected dataset ids */ -- GitLab