From 8d84c5f3bb1820fb9766bc9d3d46795ae93b27ef Mon Sep 17 00:00:00 2001 From: Xiao Gui <xgui3783@gmail.com> Date: Mon, 20 Apr 2020 14:30:49 +0200 Subject: [PATCH] bugfix: selected region not persistent via URL --- .gitignore | 2 + check_all.sh | 47 ++++++++ deploy/datasets/query.js | 15 ++- docs/releases/v2.1.2.md | 9 ++ e2e/setup.sh | 6 + e2e/src/advanced/urlParsing.e2e-spec.js | 99 +++++++++++++++- e2e/src/util.js | 3 +- mkdocs.yml | 1 + src/atlasViewer/atlasViewer.urlUtil.spec.ts | 106 +++++++++++++++--- src/atlasViewer/atlasViewer.urlUtil.ts | 6 +- .../nehubaContainer.component.ts | 41 ++++--- .../nehubaViewerInterface.directive.ts | 46 +++++++- 12 files changed, 331 insertions(+), 50 deletions(-) create mode 100755 check_all.sh create mode 100644 docs/releases/v2.1.2.md create mode 100755 e2e/setup.sh diff --git a/.gitignore b/.gitignore index b2a1cc502..037bee1f5 100644 --- a/.gitignore +++ b/.gitignore @@ -9,3 +9,5 @@ deploy/datasets/data/ .DS_Store venv/ site + +*.log diff --git a/check_all.sh b/check_all.sh new file mode 100755 index 000000000..108ea83b2 --- /dev/null +++ b/check_all.sh @@ -0,0 +1,47 @@ +#! /bin/bash + +echo 'Running all tests ...' + +echo 'Running linter ...' + +# check lint +npm run lint &> check_all_lint.log || exit + +echo 'Linter complete.' + +echo 'Running unit tests ...' +# unit test +NODE_ENV=test npm run test &> check_all_test.log || exit +echo 'Unit tests complete.' + +echo 'Running server tests ...' +cd deploy +npm run test &> check_all_deploy_test.log || exit +echo 'Server test complete.' + +cd .. + +echo 'Building and running e2e tests ...' +echo 'Building ...' +# e2e tests +npm run build-aot &> check_all_build_aot.log || exit +echo 'Build complete.' + +echo 'Spinning up node server ...' +# run node server +cd deploy +node server.js &> check_all_deploy_server.log & +NDOE_SERVER_PID=$! + +# run e2e test + +echo 'Running e2e tests ...' +cd .. +npm run e2e &> check_all_e2e.log || kill "$NDOE_SERVER_PID" && exit +echo 'e2e tests complete.' + +echo 'Killing server.js pid $NDOE_SERVER_PID' + +kill "$NDOE_SERVER_PID" + +echo 'All runs complete.' diff --git a/deploy/datasets/query.js b/deploy/datasets/query.js index fc97e10e7..7e556ad5e 100644 --- a/deploy/datasets/query.js +++ b/deploy/datasets/query.js @@ -38,12 +38,15 @@ const fetchDatasetFromKg = async ({ user } = {}) => { return await new Promise((resolve, reject) => { request(`${KG_QUERY_DATASETS_URL}${releasedOnly ? '&databaseScope=RELEASED' : ''}`, option, (err, resp, body) => { - if (err) - return reject(err) - if (resp.statusCode >= 400) - return reject(resp.statusCode) - const json = JSON.parse(body) - return resolve(json) + if (err) return reject(err) + if (resp.statusCode >= 400) return reject(resp.statusCode) + try { + const json = JSON.parse(body) + return resolve(json) + }catch (e) { + console.warn(`parsing json obj error`, body) + reject(e) + } }) }) } diff --git a/docs/releases/v2.1.2.md b/docs/releases/v2.1.2.md new file mode 100644 index 000000000..c268f7470 --- /dev/null +++ b/docs/releases/v2.1.2.md @@ -0,0 +1,9 @@ +# v2.1.2 + +## Bugfixes + +- Fixed a bug where selected region is not persisting via URL. (#501) + +## Under the hood stuff + +- added `check_all.sh`, allowing for quick git-push check \ No newline at end of file diff --git a/e2e/setup.sh b/e2e/setup.sh new file mode 100755 index 000000000..08597d425 --- /dev/null +++ b/e2e/setup.sh @@ -0,0 +1,6 @@ +#! /bin/bash + +export CHROMIUM_VERSION=80.0.3987.106 +export PPTR_VERSION=2.1.0 +npm run wd -- update --versions.chrome=${CHROMIUM_VERSION} +npm i --no-save puppeteer@${PPTR_VERSION} \ No newline at end of file diff --git a/e2e/src/advanced/urlParsing.e2e-spec.js b/e2e/src/advanced/urlParsing.e2e-spec.js index 93d26e685..944dce4fa 100644 --- a/e2e/src/advanced/urlParsing.e2e-spec.js +++ b/e2e/src/advanced/urlParsing.e2e-spec.js @@ -1,7 +1,7 @@ const { AtlasPage } = require("../util") const proxy = require('selenium-webdriver/proxy') -describe('url parsing', () => { +describe('> url parsing', () => { let iavPage beforeEach(async () => { @@ -22,7 +22,7 @@ describe('url parsing', () => { // expect(searchParamObj.get('templateSelected')).toBeNull() // }) - it('navigation state should be perserved', async () => { + it('> navigation state should be perserved', async () => { const searchParam = `/?templateSelected=Big+Brain+%28Histology%29&parcellationSelected=Cytoarchitectonic+Maps&cNavigation=zvyba.z0UJ7._WMxv.-TTch..2_cJ0e.2-OUQG._a9qP._QPHw..7LIx..2CQ3O.1FYC.259Wu..2r6` const expectedNav = { @@ -62,7 +62,100 @@ describe('url parsing', () => { }) - it('pluginStates should result in xhr to get pluginManifest', async () => { + it('> if cRegionSelected is defined, should select region in viewer', async () => { + const url = '/?templateSelected=MNI+152+ICBM+2009c+Nonlinear+Asymmetric&parcellationSelected=JuBrain+Cytoarchitectonic+Atlas&cRegionsSelected=%7B"jubrain+mni152+v18+left"%3A"8"%7D' + await iavPage.goto(url) + await iavPage.clearAlerts() + + // TODO use screenshot API when merg v2.3.0 + const screenshotData = await iavPage._driver.takeScreenshot() + const [ red, green, blue ] = await iavPage._driver.executeAsyncScript(() => { + + const dataUri = arguments[0] + const pos = arguments[1] + const dim = arguments[2] + const cb = arguments[arguments.length - 1] + + const img = new Image() + img.onload = () => { + const canvas = document.createElement('canvas') + canvas.width = dim[0] + canvas.height = dim[1] + + const ctx = canvas.getContext('2d') + ctx.drawImage(img, 0, 0) + const imgData = ctx.getImageData(0, 0, dim[0], dim[1]) + + const idx = (dim[0] * pos[1] + pos[0]) * 4 + const red = imgData.data[idx] + const green = imgData.data[idx + 1] + const blue = imgData.data[idx + 2] + cb([red, green, blue]) + } + img.src = dataUri + + }, `data:image/png;base64,${screenshotData}`, [600, 490], [800, 796]) + + expect(red).toBeGreaterThan(0) + expect(red).toEqual(green) + expect(red).toEqual(blue) + }) + + it('> if niftiLayers are defined, parcellation layer should be hidden', async () => { + const url = `/?templateSelected=MNI+152+ICBM+2009c+Nonlinear+Asymmetric&parcellationSelected=JuBrain+Cytoarchitectonic+Atlas&niftiLayers=https%3A%2F%2Fneuroglancer.humanbrainproject.eu%2Fprecomputed%2FJuBrain%2F17%2Ficbm152casym%2Fpmaps%2FVisual_hOc1_r_N10_nlin2MNI152ASYM2009C_2.4_publicP_a48ca5d938781ebaf1eaa25f59df74d0.nii.gz` + await iavPage.goto(url) + await iavPage.clearAlerts() + + // TODO use screenshot API when merg v2.3.0 + const screenshotData = await iavPage._driver.takeScreenshot() + const [ red, green, blue ] = await iavPage._driver.executeAsyncScript(() => { + + const dataUri = arguments[0] + const pos = arguments[1] + const dim = arguments[2] + const cb = arguments[arguments.length - 1] + + const img = new Image() + img.onload = () => { + const canvas = document.createElement('canvas') + canvas.width = dim[0] + canvas.height = dim[1] + + const ctx = canvas.getContext('2d') + ctx.drawImage(img, 0, 0) + const imgData = ctx.getImageData(0, 0, dim[0], dim[1]) + + const idx = (dim[0] * pos[1] + pos[0]) * 4 + const red = imgData.data[idx] + const green = imgData.data[idx + 1] + const blue = imgData.data[idx + 2] + cb([red, green, blue]) + } + img.src = dataUri + + }, `data:image/png;base64,${screenshotData}`, [600, 490], [800, 796]) + + expect(red).toBeGreaterThan(0) + expect(red).toEqual(green) + expect(red).toEqual(blue) + }) + + it('> if niftiLayers is defined, after parsing, it should persist', async () => { + const url = `/?templateSelected=MNI+152+ICBM+2009c+Nonlinear+Asymmetric&parcellationSelected=JuBrain+Cytoarchitectonic+Atlas&niftiLayers=https%3A%2F%2Fneuroglancer.humanbrainproject.eu%2Fprecomputed%2FJuBrain%2F17%2Ficbm152casym%2Fpmaps%2FVisual_hOc1_r_N10_nlin2MNI152ASYM2009C_2.4_publicP_a48ca5d938781ebaf1eaa25f59df74d0.nii.gz` + await iavPage.goto(url) + await iavPage.clearAlerts() + + // TODO use execScript from iavPage API + const niftiLayers = await iavPage._driver.executeScript(() => { + const search = new URLSearchParams(window.location.search) + return search.get('niftiLayers') + }) + const expected = `https://neuroglancer.humanbrainproject.eu/precomputed/JuBrain/17/icbm152casym/pmaps/Visual_hOc1_r_N10_nlin2MNI152ASYM2009C_2.4_publicP_a48ca5d938781ebaf1eaa25f59df74d0.nii.gz` + expect(niftiLayers).toEqual(expected) + }) + + + it('> pluginStates should result in xhr to get pluginManifest', async () => { const searchParam = new URLSearchParams() searchParam.set('templateSelected', 'MNI 152 ICBM 2009c Nonlinear Asymmetric') diff --git a/e2e/src/util.js b/e2e/src/util.js index 679cbffe2..a2c5e5653 100644 --- a/e2e/src/util.js +++ b/e2e/src/util.js @@ -182,7 +182,6 @@ class WdBase{ await this.dismissModal() await this.wait(200) await this.waitForAsync() - } } @@ -255,6 +254,8 @@ class WdBase{ Key.ESCAPE ) .perform() + + await this.wait(500) } async execScript(fn, ...arg){ diff --git a/mkdocs.yml b/mkdocs.yml index d9004a659..e4224574e 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -29,6 +29,7 @@ pages: - Fetching datasets: 'advanced/datasets.md' - Display non-atlas volumes: 'advanced/otherVolumes.md' - Release notes: + - v2.1.2: 'releases/v2.1.2.md' - v2.1.1: 'releases/v2.1.1.md' - v2.1.0: 'releases/v2.1.0.md' - v2.0.2: 'releases/v2.0.2.md' diff --git a/src/atlasViewer/atlasViewer.urlUtil.spec.ts b/src/atlasViewer/atlasViewer.urlUtil.spec.ts index 14b7bfa31..430dc39b9 100644 --- a/src/atlasViewer/atlasViewer.urlUtil.spec.ts +++ b/src/atlasViewer/atlasViewer.urlUtil.spec.ts @@ -7,6 +7,7 @@ import { cvtSearchParamToState, PARSING_SEARCHPARAM_ERROR, cvtStateToSearchParam const bigbrainJson = require('!json-loader!src/res/ext/bigbrain.json') const colin = require('!json-loader!src/res/ext/colin.json') const mni152 = require('!json-loader!src/res/ext/MNI152.json') +const mni152Nehubaconfig = require('!json-loader!src/res/ext/MNI152NehubaConfig.json') const allen = require('!json-loader!src/res/ext/allenMouse.json') const waxholm = require('!json-loader!src/res/ext/waxholmRatV2_0.json') @@ -57,8 +58,47 @@ describe('atlasViewer.urlService.service.ts', () => { }) - it('parses niftiLayers correctly', () => { + describe('niftiLayers', () => { + let searchparam = new URLSearchParams() + beforeEach(() => { + searchparam = new URLSearchParams() + searchparam.set('templateSelected', mni152.name) + searchparam.set('parcellationSelected', mni152.parcellations[1].name) + }) + + + it('parses niftiLayers correctly', () => { + const uri1 = `https://neuroglancer.humanbrainproject.eu/precomputed/JuBrain/17/icbm152casym/pmaps/Visual_hOc1_r_N10_nlin2MNI152ASYM2009C_2.4_publicP_a48ca5d938781ebaf1eaa25f59df74d0.nii.gz` + const uri2 = `https://neuroglancer.humanbrainproject.eu/precomputed/JuBrain/17/icbm152casym/pmaps/Visual_hOc1_r_N10_nlin2MNI152ASYM2009C_2000.4_publicP_a48ca5d938781ebaf1eaa25f59df74d0.nii.gz` + searchparam.set('niftiLayers', [uri1, uri2].join('__')) + + const newState = cvtSearchParamToState(searchparam, fetchedTemplateRootState) + expect(newState.ngViewerState.layers.length).toEqual(2) + + const layer1 = newState.ngViewerState.layers[0] + expect(layer1.name).toEqual(uri1) + expect(layer1.source).toEqual(`nifti://${uri1}`) + expect(layer1.mixability).toEqual('nonmixable') + + const layer2 = newState.ngViewerState.layers[1] + expect(layer2.name).toEqual(uri2) + expect(layer2.source).toEqual(`nifti://${uri2}`) + expect(layer2.mixability).toEqual('nonmixable') + }) + + it('parses multiple niftiLayers correctly', () => { + const uri = `https://neuroglancer.humanbrainproject.eu/precomputed/JuBrain/17/icbm152casym/pmaps/Visual_hOc1_r_N10_nlin2MNI152ASYM2009C_2.4_publicP_a48ca5d938781ebaf1eaa25f59df74d0.nii.gz` + searchparam.set('niftiLayers', uri) + + const newState = cvtSearchParamToState(searchparam, fetchedTemplateRootState) + expect(newState.ngViewerState.layers.length).toEqual(1) + + const layer = newState.ngViewerState.layers[0] + expect(layer.name).toEqual(uri) + expect(layer.source).toEqual(`nifti://${uri}`) + expect(layer.mixability).toEqual('nonmixable') + }) }) it('parses pluginStates correctly', () => { @@ -102,21 +142,59 @@ describe('atlasViewer.urlService.service.ts', () => { const stringified = searchParam.toString() expect(stringified).toBe('templateSelected=Big+Brain+%28Histology%29') }) - }) + it('should convert template selected and parcellation selected', () => { + + const { viewerState } = defaultRootState + const searchParam = cvtStateToSearchParam({ + ...defaultRootState, + viewerState: { + ...viewerState, + templateSelected: bigbrainJson, + parcellationSelected: bigbrainJson.parcellations[0] + } + }) + + const stringified = searchParam.toString() + expect(stringified).toBe('templateSelected=Big+Brain+%28Histology%29&parcellationSelected=Cytoarchitectonic+Maps') + }) - it('should convert template selected and parcellation selected', () => { + describe('niftiLayers', () => { + it('should convert multiple nifti layers', () => { - const { viewerState } = defaultRootState - const searchParam = cvtStateToSearchParam({ - ...defaultRootState, - viewerState: { - ...viewerState, - templateSelected: bigbrainJson, - parcellationSelected: bigbrainJson.parcellations[0] - } - }) + const uri1 = `http://localhost:1111/test1.nii.gz` + const uri2 = `http://localhost:2222/test2.nii.gz` + + const layer1 = { + mixability: 'nonmixable', + name: 'foo', + source: `nifti://${uri1}`, + } - const stringified = searchParam.toString() - expect(stringified).toBe('templateSelected=Big+Brain+%28Histology%29&parcellationSelected=Cytoarchitectonic+Maps') + const layer2 = { + mixability: 'nonmixable', + name: 'bar', + source: `nifti://${uri2}` + } + const { ngViewerState, viewerState } = defaultRootState + const searchParam = cvtStateToSearchParam({ + ...defaultRootState, + viewerState: { + ...viewerState, + templateSelected: { + ...mni152, + nehubaConfig: mni152Nehubaconfig + }, + parcellationSelected: mni152.parcellations[0] + }, + ngViewerState: { + ...ngViewerState, + layers: [ layer1, layer2 ] + } + }) + const str = searchParam.get('niftiLayers') + expect(str).toBeTruthy() + expect( encodeURIComponent(str) ).toEqual(`http%3A%2F%2Flocalhost%3A1111%2Ftest1.nii.gz__http%3A%2F%2Flocalhost%3A2222%2Ftest2.nii.gz`) + }) + }) }) }) diff --git a/src/atlasViewer/atlasViewer.urlUtil.ts b/src/atlasViewer/atlasViewer.urlUtil.ts index 19e5720fc..025980709 100644 --- a/src/atlasViewer/atlasViewer.urlUtil.ts +++ b/src/atlasViewer/atlasViewer.urlUtil.ts @@ -69,11 +69,13 @@ export const cvtStateToSearchParam = (state: IavRootStoreInterface): URLSearchPa const initialNgState = templateSelected.nehubaConfig.dataset.initialNgState const { layers } = ngViewerState const additionalLayers = layers.filter(layer => - /^blob:/.test(layer.name) && + !/^blob:/.test(layer.name) && Object.keys(initialNgState.layers).findIndex(layerName => layerName === layer.name) < 0, ) const niftiLayers = additionalLayers.filter(layer => /^nifti:\/\//.test(layer.source)) - if (niftiLayers.length > 0) { searchParam.set('niftiLayers', niftiLayers.join('__')) } + if (niftiLayers.length > 0) { + searchParam.set('niftiLayers', niftiLayers.map(layer => layer.source.replace(/^nifti:\/\//, '')).join('__')) + } } // plugin state diff --git a/src/ui/nehubaContainer/nehubaContainer.component.ts b/src/ui/nehubaContainer/nehubaContainer.component.ts index 0ddb3de68..3122e45d0 100644 --- a/src/ui/nehubaContainer/nehubaContainer.component.ts +++ b/src/ui/nehubaContainer/nehubaContainer.component.ts @@ -1,6 +1,6 @@ import { Component, ElementRef, Input, OnChanges, OnDestroy, OnInit, ViewChild, ChangeDetectorRef, Output, EventEmitter } from "@angular/core"; import { select, Store } from "@ngrx/store"; -import { combineLatest, fromEvent, merge, Observable, of, Subscription } from "rxjs"; +import { combineLatest, fromEvent, merge, Observable, of, Subscription, timer } from "rxjs"; import { pipeFromArray } from "rxjs/internal/util/pipe"; import { buffer, @@ -18,10 +18,11 @@ import { take, takeUntil, tap, - withLatestFrom + withLatestFrom, + delayWhen, } from "rxjs/operators"; import { LoggingService } from "src/logging"; -import { FOUR_PANEL, H_ONE_THREE, NEHUBA_READY, NG_VIEWER_ACTION_TYPES, SINGLE_PANEL, V_ONE_THREE } from "src/services/state/ngViewerState.store"; +import { FOUR_PANEL, H_ONE_THREE, NG_VIEWER_ACTION_TYPES, SINGLE_PANEL, V_ONE_THREE } from "src/services/state/ngViewerState.store"; import { SELECT_REGIONS_WITH_ID, VIEWERSTATE_ACTION_TYPES } from "src/services/state/viewerState.store"; import { ADD_NG_LAYER, generateLabelIndexId, getMultiNgIdsRegionsLabelIndexMap, getNgIds, ILandmark, IOtherLandmarkGeometry, IPlaneLandmarkGeometry, IPointLandmarkGeometry, isDefined, MOUSE_OVER_LANDMARK, NgViewerStateInterface, REMOVE_NG_LAYER, safeFilter, ViewerStateInterface } from "src/services/stateStore.service"; import { getExportNehuba, isSame } from "src/util/fn"; @@ -579,10 +580,6 @@ export class NehubaContainer implements OnInit, OnChanges, OnDestroy { ) ), ).subscribe(([templateSelected, parcellationSelected]) => { - this.store.dispatch({ - type: NEHUBA_READY, - nehubaReady: false, - }) this.selectedTemplate = templateSelected this.createNewNehuba(templateSelected) @@ -637,25 +634,25 @@ export class NehubaContainer implements OnInit, OnChanges, OnDestroy { distinctUntilChanged(), ), this.selectedParcellation$, - ) - .subscribe(([regions, hideSegmentFlag, forceShowSegment, selectedParcellation]) => { - if (!this.nehubaViewer) { return } + ).pipe( + delayWhen(() => timer()) + ).subscribe(([regions, hideSegmentFlag, forceShowSegment, selectedParcellation]) => { + if (!this.nehubaViewer) { return } - const { ngId: defaultNgId } = selectedParcellation + const { ngId: defaultNgId } = selectedParcellation - /* selectedregionindexset needs to be updated regardless of forceshowsegment */ - this.selectedRegionIndexSet = new Set(regions.map(({ngId = defaultNgId, labelIndex}) => generateLabelIndexId({ ngId, labelIndex }))) + /* selectedregionindexset needs to be updated regardless of forceshowsegment */ + this.selectedRegionIndexSet = new Set(regions.map(({ngId = defaultNgId, labelIndex}) => generateLabelIndexId({ ngId, labelIndex }))) - if ( forceShowSegment === false || (forceShowSegment === null && hideSegmentFlag) ) { - this.nehubaViewer.hideAllSeg() - return - } + if ( forceShowSegment === false || (forceShowSegment === null && hideSegmentFlag) ) { + this.nehubaViewer.hideAllSeg() + return + } - this.selectedRegionIndexSet.size > 0 ? - this.nehubaViewer.showSegs([...this.selectedRegionIndexSet]) : - this.nehubaViewer.showAllSeg() - }, - ), + this.selectedRegionIndexSet.size > 0 + ? this.nehubaViewer.showSegs([...this.selectedRegionIndexSet]) + : this.nehubaViewer.showAllSeg() + }), ) this.subscriptions.push( diff --git a/src/ui/nehubaContainer/nehubaViewerInterface/nehubaViewerInterface.directive.ts b/src/ui/nehubaContainer/nehubaViewerInterface/nehubaViewerInterface.directive.ts index 7e771d624..3080792cb 100644 --- a/src/ui/nehubaContainer/nehubaViewerInterface/nehubaViewerInterface.directive.ts +++ b/src/ui/nehubaContainer/nehubaViewerInterface/nehubaViewerInterface.directive.ts @@ -191,7 +191,43 @@ const accumulatorFn: ( return newMap } -// TODO port viewer related functionalities (input/outputs) from nehubacontainer to here! +// methods +// +// new viewer +// change state (layer visibliity) +// change state (segment visibility) +// change state (color map) +// change state (add/remove layer) +// changeNavigation +// setLayout (2x2 or max screen) + +// emitters +// +// mouseoverSegments +// mouseoverLandmarks +// selectSegment +// navigationChanged + +/** + * This directive should only deal with non-navigational interface between + * - viewer (nehuba) + * - state store (ngrx) + * + * + * public prop + * + * - newViewer (new template / null for destroying current instance) + * - segmentVisibility change + * - setColorMap for segmentation map + * - add/remove layer (image/segmentation/mesh) + * + * emitters + * + * - mouseoverSegments + * - mouseoverLandmark + * - selectSegment + * - loadingStatus + */ @Directive({ selector: '[iav-nehuba-viewer-container]', @@ -208,7 +244,7 @@ export class NehubaViewerContainerDirective implements OnInit, OnDestroy{ constructor( private el: ViewContainerRef, private cfr: ComponentFactoryResolver, - private store$: Store<IavRootStoreInterface> + private store$: Store<IavRootStoreInterface>, ){ this.nehubaViewerFactory = this.cfr.resolveComponentFactory(NehubaViewerUnit) @@ -293,6 +329,12 @@ export class NehubaViewerContainerDirective implements OnInit, OnDestroy{ } createNehubaInstance(template: any){ + + this.store$.dispatch({ + type: NEHUBA_READY, + nehubaReady: false, + }) + this.clear() this.iavNehubaViewerContainerViewerLoading.emit(true) this.cr = this.el.createComponent(this.nehubaViewerFactory) -- GitLab