From 1d79efd53157705b965657d60795e9334cddcfec Mon Sep 17 00:00:00 2001
From: Xiao Gui <xgui3783@gmail.com>
Date: Mon, 15 Feb 2021 16:40:42 +0100
Subject: [PATCH] bugfix: fragment parsing issue

---
 deploy/bkwdCompat/urlState.js      | 15 ++++++-----
 src/routerModule/router.service.ts | 35 +++++++------------------
 src/routerModule/util.ts           | 42 ++++++++++++++++++------------
 3 files changed, 43 insertions(+), 49 deletions(-)

diff --git a/deploy/bkwdCompat/urlState.js b/deploy/bkwdCompat/urlState.js
index 881e79cb6..0f7e97130 100644
--- a/deploy/bkwdCompat/urlState.js
+++ b/deploy/bkwdCompat/urlState.js
@@ -106,6 +106,8 @@ const templateMap = {
   'Allen Mouse': allenObj
 }
 
+const encodeId = id => id.replace(/\//g, '_')
+
 module.exports = query => {
   const {
     standaloneVolumes,
@@ -131,29 +133,28 @@ module.exports = query => {
   // to avoid potentially issues (e.g. url containing __, which is very possible)
 
   const plugins = pluginStates && pluginStates.split('__')
-
   let redirectUrl = '/#'
   if (standaloneVolumes) {
     redirectUrl += `/sv:${encodeURIComponent(standaloneVolumes)}`
     if (cNavigation) redirectUrl += `/@:${encodeURIComponent(cNavigation)}`
     if (previewingDatasetFiles) redirectUrl += `/dsp:${encodeURIComponent(previewingDatasetFiles)}`
-    if (plugins && plugins.length > 0) redirectUrl += `/pl:${encodeURIComponent(plugins)}`
+    if (plugins && plugins.length > 0) redirectUrl += `/pl:${encodeURIComponent(JSON.stringify(plugins))}`
 
-    if (niftiLayers) redirectUrl += `&niftiLayers=${encodeURIComponent(niftiLayers)}`
+    if (niftiLayers) redirectUrl += `?niftiLayers=${encodeURIComponent(niftiLayers)}`
     return redirectUrl
   }
 
   if (templateSelected && templateMap[templateSelected]) {
     const { id: t, aId: a, parc } = templateMap[templateSelected]
-    redirectUrl += `/a:${encodeURIComponent(a)}/t:${encodeURIComponent(t)}`
+    redirectUrl += `/a:${encodeId(a)}/t:${encodeId(t)}`
     const { id: p } = parc[parcellationSelected] || {}
-    if (p) redirectUrl += `/p:${encodeURIComponent(p)}`
+    if (p) redirectUrl += `/p:${encodeId(p)}`
     if (cRegionsSelected) redirectUrl += `/r:${encodeURIComponent(cRegionsSelected)}`
     if (cNavigation) redirectUrl += `/@:${encodeURIComponent(cNavigation)}`
     if (previewingDatasetFiles) redirectUrl += `/dsp:${encodeURIComponent(previewingDatasetFiles)}`
-    if (plugins && plugins.length > 0) redirectUrl += `/pl:${encodeURIComponent(plugins)}`
+    if (plugins && plugins.length > 0) redirectUrl += `/pl:${encodeURIComponent(JSON.stringify(plugins))}`
 
-    if (niftiLayers) redirectUrl += `&niftiLayers=${encodeURIComponent(niftiLayers)}`
+    if (niftiLayers) redirectUrl += `?niftiLayers=${encodeURIComponent(niftiLayers)}`
     return redirectUrl
   }
   return null
diff --git a/src/routerModule/router.service.ts b/src/routerModule/router.service.ts
index 566146194..df1027d06 100644
--- a/src/routerModule/router.service.ts
+++ b/src/routerModule/router.service.ts
@@ -4,7 +4,7 @@ import { Inject } from "@angular/core";
 import { NavigationEnd, Router } from '@angular/router'
 import { select, Store } from "@ngrx/store";
 import { combineLatest, Observable } from "rxjs";
-import { debounceTime, filter, map, mapTo, shareReplay, switchMapTo, take, tap, withLatestFrom } from "rxjs/operators";
+import { debounceTime, filter, map, mapTo, shareReplay, switchMapTo, take, withLatestFrom } from "rxjs/operators";
 import { viewerStateFetchedTemplatesSelector } from "src/services/state/viewerState.store.helper";
 import { viewerStateFetchedAtlasesSelector } from "src/services/state/viewerState/selectors";
 import { generalApplyState } from "src/services/stateStore.helper";
@@ -17,7 +17,6 @@ import { cvtStateToHashedRoutes, cvtFullRouteToState } from "./util";
 
 export class RouterService {
 
-  private firstRenderFlag = true
   private allFetchingReady$: Observable<boolean>
 
   constructor(
@@ -68,32 +67,13 @@ export class RouterService {
       )
     ).subscribe(([ev, state]: [NavigationEnd, any]) => {
       const fullPath = ev.urlAfterRedirects
-      const newState = cvtFullRouteToState(router.parseUrl(fullPath), state)
-      let newUrl: any[]
-      try {
-        newUrl = cvtStateToHashedRoutes(newState)
-      } catch (_e) {
-        console.error(`cvtStateToHashedRoutes error`, _e)
-      }
-
-      // NB this is required, as parseUrl seems to decode %3A back to :
-      // resulting in false positive
-      const newRoute = newUrl && router.parseUrl(
-        `/${newUrl.join('/')}`
-      ).toString()
-      const currentRoute = router.routerState.snapshot.url
+      const stateFromRoute = cvtFullRouteToState(router.parseUrl(fullPath), state, (...e: any[]) => console.log(...e))
+      const routeFromState = cvtStateToHashedRoutes(state)
 
-      // this fn would in principle be invoked every time path changes
-      // We are only interested in when path changes as a result of:
-      // - on open
-      // - on on history
-      // on way to do this is by checking the updated route === current route
-      // above scenarios would result in newRoute !== currentRoute
-      if ( this.firstRenderFlag || newRoute !== currentRoute) {
-        this.firstRenderFlag = false
+      if ( fullPath !== routeFromState.join('/')) {
         store$.dispatch(
           generalApplyState({
-            state: newState
+            state: stateFromRoute
           })
         )
       }
@@ -119,8 +99,11 @@ export class RouterService {
       if (routes.length === 0) {
         router.navigate([ baseHref ])
       } else {
+        const currUrl = router.routerState.snapshot.url
         const joinedRoutes = `/${routes.join('/')}`
-        router.navigateByUrl(joinedRoutes)
+        if (currUrl !== joinedRoutes) {
+          router.navigateByUrl(joinedRoutes)
+        }
       }
     })
   }
diff --git a/src/routerModule/util.ts b/src/routerModule/util.ts
index 80b15ca99..2deaaf1be 100644
--- a/src/routerModule/util.ts
+++ b/src/routerModule/util.ts
@@ -2,7 +2,7 @@ import { viewerStateGetSelectedAtlas, viewerStateSelectedParcellationSelector, v
 import { encodeNumber, decodeToNumber, separator } from './cipher'
 import { getGetRegionFromLabelIndexId } from 'src/util/fn'
 import { serialiseParcellationRegion } from "common/util"
-import { UrlTree } from "@angular/router"
+import { UrlSegment, UrlTree } from "@angular/router"
 import { getShader, PMAP_DEFAULT_CONFIG } from "src/util/constants"
 import { mixNgLayers } from "src/services/state/ngViewerState.store"
 import { PLUGINSTORE_CONSTANTS } from 'src/services/state/pluginState.store'
@@ -16,7 +16,8 @@ export const PARSE_ERROR = {
   PARCELLATION_NOT_UPDATED: 'PARCELLATION_NOT_UPDATED',
 }
 
-const encodeId = (id: string) => id.replace(/\//g, '_')
+const encodeId = (id: string) => id && id.replace(/\//g, '_')
+const decodeId = (codedId: string) => codedId && codedId.replace(/_/g, '/')
 const endcodePath = (key: string, val: string) => `${key}:${encodeURIComponent(val)}`
 const decodePath = (path: string) => {
   const re = /^(.*?):(.+)$/.exec(path)
@@ -58,7 +59,7 @@ type TConditional = Partial<
 
 type TUrlPathObj = (TUrlAtlas | TUrlStandaloneVolume) & TConditional
 
-function parseSearchParamForTemplateParcellationRegion(obj: TUrlPathObj, state: any) {
+function parseSearchParamForTemplateParcellationRegion(obj: TUrlPathObj, state: any, warnCb: Function) {
 
   /**
    * TODO if search param of either template or parcellation is incorrect, wrong things are searched
@@ -67,20 +68,21 @@ function parseSearchParamForTemplateParcellationRegion(obj: TUrlPathObj, state:
   const templateSelected = (() => {
     const { fetchedTemplates } = state.viewerState
 
-    const searchedId = obj['t']
+    const searchedId = decodeId(obj['t'])
 
-    if (!searchedId) { throw new Error(PARSE_ERROR.TEMPALTE_NOT_SET) }
+    if (!searchedId) return null
     const templateToLoad = fetchedTemplates.find(template => (template['@id'] || template['fullId']) === searchedId)
     if (!templateToLoad) { throw new Error(PARSE_ERROR.TEMPLATE_NOT_FOUND) }
     return templateToLoad
   })()
 
   const parcellationSelected = (() => {
-    const searchedId = obj['p']
+    if (!templateSelected) return null
+    const searchedId = decodeId(obj['p'])
 
     const parcellationToLoad = templateSelected.parcellations.find(parcellation => parcellation['@id'] || parcellation['fullId'] === searchedId)
     if (!parcellationToLoad) { 
-      // catch error
+      warnCb(`parcellation with id ${searchedId} not found... load the first parc instead`)
     }
     return parcellationToLoad || templateSelected.parcellations[0]
   })()
@@ -88,6 +90,7 @@ function parseSearchParamForTemplateParcellationRegion(obj: TUrlPathObj, state:
   /* selected regions */
 
   const regionsSelected = (() => {
+    if (!parcellationSelected) return []
 
     // TODO deprecate. Fallback (defaultNgId) (should) already exist
     // if (!viewerState.parcellationSelected.updated) throw new Error(PARCELLATION_NOT_UPDATED)
@@ -147,12 +150,13 @@ function parseSearchParamForTemplateParcellationRegion(obj: TUrlPathObj, state:
   }
 }
 
-export const cvtFullRouteToState = (fullPath: UrlTree, state) => {
+export const cvtFullRouteToState = (fullPath: UrlTree, state: any, _warnCb?: Function) => {
 
-  if (!fullPath.root.hasChildren()) return state
-  if (!fullPath.root.children['primary']) return state
+  let warnCb = _warnCb || ((...e: any[]) => console.warn(...e))
+  const pathFragments: UrlSegment[] = fullPath.root.hasChildren()
+    ? fullPath.root.children['primary'].segments
+    : []
 
-  const pathFragments = fullPath.root.children['primary'].segments
   const returnState = JSON.parse(JSON.stringify(state))
 
   const returnObj: Partial<TUrlPathObj> = {}
@@ -209,11 +213,12 @@ export const cvtFullRouteToState = (fullPath: UrlTree, state) => {
         // flag to allow for animation when enabled
         animation: {},
       }
-    } catch (_e) {
+    } catch (e) {
       /**
        * TODO Poisoned encoded char
        * send error message
        */
+      warnCb(`parse nav error`, e)
     }
   }
 
@@ -224,10 +229,11 @@ export const cvtFullRouteToState = (fullPath: UrlTree, state) => {
     try {
       const arrPluginStates = JSON.parse(pluginStates)
       pluginState.initManifests = arrPluginStates.map(url => [PLUGINSTORE_CONSTANTS.INIT_MANIFEST_SRC, url] as [string, string])
-    } catch (_e) {
+    } catch (e) {
       /**
        * parsing plugin error
        */
+      warnCb(`parse plugin states error`, e)
     }
   }
 
@@ -246,6 +252,7 @@ export const cvtFullRouteToState = (fullPath: UrlTree, state) => {
     }
   } catch (e) {
     // parsing previewingDatasetFiles
+    warnCb(`parse dsp error`, e)
   }
 
   // If sv (standaloneVolume is defined)
@@ -259,14 +266,16 @@ export const cvtFullRouteToState = (fullPath: UrlTree, state) => {
       returnState['viewerState']['standaloneVolumes'] = parsedArr
       returnState['viewerState']['navigation'] = parsedNavObj
       return returnState
-    } catch (_e) {
+    } catch (e) {
       // if any error occurs, parse rest per normal
+      warnCb(`parse standalone volume error`, e)
     }
+  } else {
+    returnState['viewerState']['standaloneVolumes'] = []
   }
 
   try {
-    const { parcellationSelected, regionsSelected, templateSelected } = parseSearchParamForTemplateParcellationRegion(returnObj as TUrlPathObj, state)
-
+    const { parcellationSelected, regionsSelected, templateSelected } = parseSearchParamForTemplateParcellationRegion(returnObj as TUrlPathObj, state, warnCb)
     returnState['viewerState']['parcellationSelected'] = parcellationSelected
     returnState['viewerState']['regionsSelected'] = regionsSelected
     returnState['viewerState']['templateSelected'] = templateSelected
@@ -274,6 +283,7 @@ export const cvtFullRouteToState = (fullPath: UrlTree, state) => {
     returnState['viewerState']['navigation'] = parsedNavObj
   } catch (e) {
     // if error, show error on UI?
+    warnCb(`parse template, parc, region error`, e)
   }
 
   /**
-- 
GitLab