Merge pull request #1416 from FZJ-INM1-BDA/staging

v2.13.2

.github/workflows/ci.yml

@@ -58,16 +58,15 @@ jobs:
   backend:
     if: always()
     runs-on: ubuntu-latest
-
-    env:
-      NODE_ENV: test
-
     steps:
     - uses: actions/checkout@v3
-    - name: Use Node.js 16.x
-      uses: actions/setup-node@v1
+    - uses: actions/setup-python@v4
       with:
-        node-version: 16.x
+        python-version: '3.10'
     - run: |
-        echo "busybody"
-  
+        cd backend
+        echo "hello world" >> index.html
+        export PATH_TO_PUBLIC=$(pwd)
+        pip install -r requirements.txt
+        pip install pytest
+        pytest

backend/app/sane_url.py

@@ -4,7 +4,7 @@
 from authlib.integrations.requests_client import OAuth2Session
 import requests
 import json
-from typing import Union, Dict, Optional
+from typing import Union, Dict, Optional, Any
 import time
 from io import StringIO
 from pydantic import BaseModel
@@ -27,6 +27,7 @@
 
 class SaneUrlDPStore(DataproxyStore):
     class AlreadyExists(Exception): ...
+    class NotWritable(IOError): ...
 
     @staticmethod
     def GetTimeMs() -> int:
@@ -36,8 +37,12 @@ def GetTimeMs() -> int:
     def TransformKeyToObjName(key: str):
         return f"saneUrl/{key}.json"
 
+    writable = False
+
     def __init__(self, expiry_s=3 * 24 * 60 * 60):
-
+        if not (SXPLR_EBRAINS_IAM_SA_CLIENT_ID and SXPLR_EBRAINS_IAM_SA_CLIENT_SECRET):
+            super().__init__(None, SXPLR_BUCKET_NAME)
+            return
         resp = requests.get(f"{EBRAINS_IAM_DISCOVERY_URL}/.well-known/openid-configuration")
         resp.raise_for_status()
         resp_json = resp.json()
@@ -49,19 +54,21 @@ def __init__(self, expiry_s=3 * 24 * 60 * 60):
                                        SXPLR_EBRAINS_IAM_SA_CLIENT_SECRET,
                                        scope=" ".join(scopes))
         self.session = oauth2_session
-        self.expiry_s: float = None
+        self.expiry_s: float = expiry_s
+        self.token_expires_at: float = None
         self.token: str = None
         self._refresh_token()
 
         super().__init__(self.token, SXPLR_BUCKET_NAME)
+        self.writable = True
 
     def _refresh_token(self):
         token_dict = self.session.fetch_token(self._token_endpoint, grant_type="client_credentials")
-        self.expiry_s = token_dict.get("expires_at")
+        self.token_expires_at = token_dict.get("expires_at")
         self.token = token_dict.get("access_token")
 
     def _get_bucket(self):
-        token_expired = self.expiry_s - time.time() > 30
+        token_expired = (self.token_expires_at - time.time()) < 30
         if token_expired:
             self._refresh_token()
 
@@ -94,7 +101,8 @@ def get(self, key: str):
             raise SaneUrlDPStore.GenericException(str(e)) from e
 
     def set(self, key: str, value: Union[str, Dict], request: Optional[Request]=None):
-
+        if not self.writable:
+            raise SaneUrlDPStore.NotWritable
         object_name = SaneUrlDPStore.TransformKeyToObjName(key)
         try:
             super().get(object_name)
@@ -126,11 +134,19 @@ def delete(self, key: str):
 @router.get("/{short_id:str}")
 async def get_short(short_id:str, request: Request):
     try:
-        existing_json = data_proxy_store.get(short_id)
+        existing_json: Dict[str, Any] = data_proxy_store.get(short_id)
         accept = request.headers.get("Accept", "")
         if "text/html" in accept:
             hashed_path = existing_json.get("hashPath")
-            return RedirectResponse(f"{HOST_PATHNAME}/#{hashed_path}")
+            extra_routes = []
+            for key in existing_json:
+                if key.startswith("x-"):
+                    extra_routes.append(f"{key}:{short_id}")
+                    continue
+
+            extra_routes_str = "" if len(extra_routes) == 0 else ("/" + "/".join(extra_routes))
+
+            return RedirectResponse(f"{HOST_PATHNAME}/#{hashed_path}{extra_routes_str}")
         return JSONResponse(existing_json)
     except DataproxyStore.NotFound as e:
         raise HTTPException(404, str(e))

backend/pytest.ini

@@ -0,0 +1,4 @@
+[pytest]
+pythonpath = .
+testpaths = 
+    test_app

backend/test_app/test_sane_url.py

@@ -0,0 +1,13 @@
+from app.app import app
+from fastapi.testclient import TestClient
+
+client = TestClient(app)
+
+def test_annotation_redirect():
+    resp = client.get("/go/stnr", headers={
+        "Accept": "text/html"
+    }, follow_redirects=False)
+    loc = resp.headers.get("Location")
+    assert loc, "Expected location header to be present, but was not"
+    assert "x-user-anntn:stnr" in loc, f"Expected the string 'x-user-anntn:stnr' in {loc!r}, but was not"
+

docs/releases/v2.13.2.md

@@ -0,0 +1,11 @@
+# v2.13.2
+
+## Features
+
+- added visual display of errors relating to siibra-api
+
+## Bugfixes
+
+- fixed displaying annotation in `saneurl`
+- fixed feature fetching spinner
+- fixed feature fetching logic

e2e/checklist.md

@@ -72,10 +72,13 @@
 - [ ] [saneUrl](https://atlases.ebrains.eu/viewer-staging/saneUrl/whs4) redirects to waxholm v4
 - [ ] [saneUrl](https://atlases.ebrains.eu/viewer-staging/saneUrl/allen2017) redirects to allen 2017
 - [ ] [saneUrl](https://atlases.ebrains.eu/viewer-staging/saneUrl/mebrains) redirects to monkey
+- [ ] [saneUrl](https://atlases.ebrains.eu/viewer-staging/saneUrl/stnr) redirects to URL that contains annotations
+
 ## VIP URL
 - [ ] [vipUrl](https://atlases.ebrains.eu/viewer-staging/human) redirects to human mni152
 - [ ] [vipUrl](https://atlases.ebrains.eu/viewer-staging/monkey) redirects mebrains
 - [ ] [vipUrl](https://atlases.ebrains.eu/viewer-staging/rat) redirects to waxholm v4
 - [ ] [vipUrl](https://atlases.ebrains.eu/viewer-staging/mouse) redirects allen mouse 2017
+
 ## plugins
 - [ ] jugex plugin works

mkdocs.yml

@@ -33,6 +33,7 @@ nav:
     - Fetching datasets: 'advanced/datasets.md'
     - Display non-atlas volumes: 'advanced/otherVolumes.md'
   - Release notes:
+    - v2.13.2: 'releases/v2.13.2.md'
     - v2.13.1: 'releases/v2.13.1.md'
     - v2.13.0: 'releases/v2.13.0.md'
     - v2.12.5: 'releases/v2.12.5.md'

package.json

@@ -1,6 +1,6 @@
 {
   "name": "siibra-explorer",
-  "version": "2.13.1",
+  "version": "2.13.2",
   "description": "siibra-explorer - explore brain atlases. Based on humanbrainproject/nehuba & google/neuroglancer. Built with angular",
   "scripts": {
     "lint": "eslint src --ext .ts",

src/atlasComponents/annotations/annotation.service.ts

@@ -2,6 +2,7 @@ import { BehaviorSubject, Observable } from "rxjs";
 import { distinctUntilChanged } from "rxjs/operators";
 import { getUuid, waitFor } from "src/util/fn";
 import { PeriodicSvc } from "src/util/periodic.service";
+import { NehubaLayerControlService } from "src/viewerModule/nehuba/layerCtrl.service";
 
 export type TNgAnnotationEv = {
   pickedAnnotationId: string
@@ -54,6 +55,10 @@ interface NgAnnotationLayer {
     registerDisposer(fn: () => void): void
   }
   setVisible(flag: boolean): void
+  layerChanged: {
+    add(cb: () => void): void
+  }
+  visible: boolean
 }
 
 export class AnnotationLayer {
@@ -105,15 +110,18 @@ export class AnnotationLayer {
       this._onHover.next(payload)
     })
     this.onDestroyCb.push(res)
-    
+
     this.nglayer.layer.registerDisposer(() => {
-      this.dispose()
+      // TODO registerdisposer seems to fire without the layer been removed
+      // Thus it cannot be relied upon for cleanup
     })
+    NehubaLayerControlService.RegisterLayerName(this.name)
   }
   setVisible(flag: boolean){
     this.nglayer && this.nglayer.setVisible(flag)
   }
   dispose() {
+    NehubaLayerControlService.DeregisterLayerName(this.name)
     AnnotationLayer.Map.delete(this.name)
     this._onHover.complete()
     while(this.onDestroyCb.length > 0) this.onDestroyCb.pop()()

src/atlasComponents/sapi/sapi.service.spec.ts

@@ -53,12 +53,11 @@ describe("> sapi.service.ts", () => {
           })
         })
         it("> should call fetch twice", async () => {
-          expect(fetchSpy).toHaveBeenCalledTimes(2)
+          expect(fetchSpy).toHaveBeenCalledTimes(1)
 
           const allArgs = fetchSpy.calls.allArgs()
-          expect(allArgs.length).toEqual(2)
+          expect(allArgs.length).toEqual(1)
           expect(atlasEndpts).toContain(allArgs[0][0])
-          expect(atlasEndpts).toContain(allArgs[1][0])
         })
 
         it("> endpoint should be set", async () => {
@@ -67,11 +66,11 @@ describe("> sapi.service.ts", () => {
 
         it("> additional calls should return cached observable", () => {
 
-          expect(fetchSpy).toHaveBeenCalledTimes(2)
+          expect(fetchSpy).toHaveBeenCalledTimes(1)
           SAPI.BsEndpoint$.subscribe()
           SAPI.BsEndpoint$.subscribe()
 
-          expect(fetchSpy).toHaveBeenCalledTimes(2)
+          expect(fetchSpy).toHaveBeenCalledTimes(1)
         })
       })
 

src/atlasComponents/sapi/sapi.service.ts

@@ -5,13 +5,13 @@ import { getExportNehuba, noop } from "src/util/fn";
 import { MatSnackBar } from "@angular/material/snack-bar";
 import { AtlasWorkerService } from "src/atlasViewer/atlasViewer.workerService.service";
 import { EnumColorMapName } from "src/util/colorMaps";
-import { forkJoin, from, NEVER, Observable, of, throwError } from "rxjs";
+import { BehaviorSubject, forkJoin, from, NEVER, Observable, of, throwError } from "rxjs";
 import { environment } from "src/environments/environment"
 import {
   translateV3Entities
 } from "./translateV3"
 import { FeatureType, PathReturn, RouteParam, SapiRoute } from "./typeV3";
-import { BoundingBox, SxplrAtlas, SxplrParcellation, SxplrRegion, SxplrTemplate, VoiFeature, Feature } from "./sxplrTypes";
+import { BoundingBox, SxplrAtlas, SxplrParcellation, SxplrRegion, SxplrTemplate, VoiFeature } from "./sxplrTypes";
 import { parcBanList, speciesOrder } from "src/util/constants";
 import { CONST } from "common/constants"
 
@@ -110,15 +110,6 @@ export class SAPI{
 
     BS_ENDPOINT_CACHED_VALUE = new Observable<string>(obs => {
       (async () => {
-        const backupPr = new Promise<string>(rs => {
-          for (const endpt of backupEndpoints) {
-            SAPI.VerifyEndpoint(endpt)
-              .then(flag => {
-                if (flag) rs(endpt)
-              })
-              .catch(noop)
-          }
-        })
         try {
           const url = await Promise.race([
             SAPI.VerifyEndpoint(mainEndpoint),
@@ -129,12 +120,23 @@ export class SAPI{
 
           try {
             const url = await Promise.race([
-              backupPr,
+              /**
+               * find the first endpoint of the backup endpoints
+               */
+              new Promise<string>(rs => {
+                for (const endpt of backupEndpoints) {
+                  SAPI.VerifyEndpoint(endpt)
+                    .then(flag => {
+                      if (flag) rs(endpt)
+                    })
+                    .catch(noop)
+                }
+              }),
               new Promise<string>((_, rj) => setTimeout(() => rj(`5s timeout`), 5000))
             ])
             obs.next(url)
           } catch (e) {
-            SAPI.ErrorMessage = `No usabe mirror found`
+            SAPI.ErrorMessage = `No usabe siibra-api endpoints found. Tried: ${mainEndpoint}, ${backupEndpoints.join(",")}`
           }
         } finally {
           obs.complete()
@@ -147,7 +149,10 @@ export class SAPI{
     return BS_ENDPOINT_CACHED_VALUE
   }
 
-  static ErrorMessage = null
+  static ErrorMessage$ = new BehaviorSubject<string>(null)
+  static set ErrorMessage(val: string){
+    this.ErrorMessage$.next(val)
+  }
 
   getParcRegions(parcId: string) {
     const param = {
@@ -187,40 +192,6 @@ export class SAPI{
     if (!!resp.total || resp.total === 0) return true
     return false
   }
-  getV3Features<T extends FeatureType>(featureType: T, sapiParam: RouteParam<`/feature/${T}`>): Observable<Feature[]> {
-    const query = structuredClone(sapiParam)
-    return this.v3Get<`/feature/${T}`>(`/feature/${featureType}`, {
-      ...query
-    }).pipe(
-      switchMap(resp => {
-        if (!this.#isPaged(resp)) return throwError(`endpoint not returning paginated response`)
-        return this.iteratePages(
-          resp,
-          page => {
-            const query = structuredClone(sapiParam)
-            query.query.page = page
-            return this.v3Get(`/feature/${featureType}`, {
-              ...query,
-            }).pipe(
-              map(val => {
-                if (this.#isPaged(val)) return val
-                return { items: [], total: 0, page: 0, size: 0 }
-              })
-            )
-          }
-        )
-      }),
-      switchMap(features => features.length === 0
-        ? of([])
-        : forkJoin(
-          features.map(feat => translateV3Entities.translateFeature(feat) )
-        )
-      ),
-      catchError((err) => {
-        console.error("Error fetching features", err)
-        return of([])}),
-    )
-  }
 
   getV3FeatureDetail<T extends FeatureType>(featureType: T, sapiParam: RouteParam<`/feature/${T}/{feature_id}`>): Observable<PathReturn<`/feature/${T}/{feature_id}`>> {
     return this.v3Get<`/feature/${T}/{feature_id}`>(`/feature/${featureType}/{feature_id}`, {
@@ -248,12 +219,6 @@ export class SAPI{
     )
   }
 
-  getModalities() {
-    return this.v3Get("/feature/_types", { query: {} }).pipe(
-      map(v => v.items)
-    )
-  }
-
   v3GetRoute<T extends SapiRoute>(route: T, sapiParam: RouteParam<T>) {
     const params: Record<string, string|number> = "query" in sapiParam ? sapiParam["query"] : {}
     const _path: Record<string, string|number> = "path" in sapiParam ? sapiParam["path"] : {}
@@ -593,9 +558,6 @@ export class SAPI{
     private snackbar: MatSnackBar,
     private workerSvc: AtlasWorkerService,
   ){
-    if (SAPI.ErrorMessage) {
-      this.snackbar.open(SAPI.ErrorMessage, 'Dismiss', { duration: 5000 })
-    }
   }
 
   /**