Merge pull request DSpace#2299 from alexandrevryghem/fix-browse-by-pr…

…oxy-timeout_contribute-main Fix Proxy Timeouts on browse by pages
atmire · Jun 13, 2023 · c70e046 · c70e046
2 parents 187702d + 91b6357
commit c70e046
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 19 deletions.
diff --git a/src/app/browse-by/browse-by-guard.spec.ts b/src/app/browse-by/browse-by-guard.spec.ts
@@ -1,18 +1,20 @@
 import { first } from 'rxjs/operators';
 import { BrowseByGuard } from './browse-by-guard';
 import { of as observableOf } from 'rxjs';
-import { createSuccessfulRemoteDataObject$ } from '../shared/remote-data.utils';
+import { createFailedRemoteDataObject$, createSuccessfulRemoteDataObject$ } from '../shared/remote-data.utils';
 import { BrowseByDataType } from './browse-by-switcher/browse-by-decorator';
 import { ValueListBrowseDefinition } from '../core/shared/value-list-browse-definition.model';
 import { DSONameServiceMock } from '../shared/mocks/dso-name.service.mock';
 import { DSONameService } from '../core/breadcrumbs/dso-name.service';
+import { RouterStub } from '../shared/testing/router.stub';
 
 describe('BrowseByGuard', () => {
   describe('canActivate', () => {
     let guard: BrowseByGuard;
     let dsoService: any;
     let translateService: any;
     let browseDefinitionService: any;
+    let router: any;
 
     const name = 'An interesting DSO';
     const title = 'Author';
@@ -35,7 +37,9 @@ describe('BrowseByGuard', () => {
         findById: () => createSuccessfulRemoteDataObject$(browseDefinition)
       };
 
-      guard = new BrowseByGuard(dsoService, translateService, browseDefinitionService, new DSONameServiceMock() as DSONameService);
+      router = new RouterStub() as any;
+
+      guard = new BrowseByGuard(dsoService, translateService, browseDefinitionService, new DSONameServiceMock() as DSONameService, router);
     });
 
     it('should return true, and sets up the data correctly, with a scope and value', () => {
@@ -65,6 +69,7 @@ describe('BrowseByGuard', () => {
               value: '"' + value + '"'
             };
             expect(scopedRoute.data).toEqual(result);
+            expect(router.navigate).not.toHaveBeenCalled();
             expect(canActivate).toEqual(true);
           }
         );
@@ -97,6 +102,7 @@ describe('BrowseByGuard', () => {
               value: ''
             };
             expect(scopedNoValueRoute.data).toEqual(result);
+            expect(router.navigate).not.toHaveBeenCalled();
             expect(canActivate).toEqual(true);
           }
         );
@@ -128,9 +134,33 @@ describe('BrowseByGuard', () => {
               value: '"' + value + '"'
             };
             expect(route.data).toEqual(result);
+            expect(router.navigate).not.toHaveBeenCalled();
             expect(canActivate).toEqual(true);
           }
         );
     });
+
+    it('should return false, and sets up the data correctly, without a scope and with a value', () => {
+      jasmine.getEnv().allowRespy(true);
+      spyOn(browseDefinitionService, 'findById').and.returnValue(createFailedRemoteDataObject$());
+      const scopedRoute = {
+        data: {
+          title: field,
+        },
+        params: {
+          id,
+        },
+        queryParams: {
+          scope,
+          value
+        }
+      };
+      guard.canActivate(scopedRoute as any, undefined)
+        .pipe(first())
+        .subscribe((canActivate) => {
+          expect(router.navigate).toHaveBeenCalled();
+          expect(canActivate).toEqual(false);
+        });
+    });
   });
 });
diff --git a/src/app/browse-by/browse-by-guard.ts b/src/app/browse-by/browse-by-guard.ts
@@ -1,15 +1,17 @@
-import { ActivatedRouteSnapshot, CanActivate, RouterStateSnapshot } from '@angular/router';
+import { ActivatedRouteSnapshot, CanActivate, Router, RouterStateSnapshot } from '@angular/router';
 import { Injectable } from '@angular/core';
 import { DSpaceObjectDataService } from '../core/data/dspace-object-data.service';
 import { hasNoValue, hasValue } from '../shared/empty.util';
 import { map, switchMap } from 'rxjs/operators';
-import { getFirstSucceededRemoteDataPayload } from '../core/shared/operators';
+import { getFirstCompletedRemoteData, getFirstSucceededRemoteDataPayload } from '../core/shared/operators';
 import { TranslateService } from '@ngx-translate/core';
 import { Observable, of as observableOf } from 'rxjs';
 import { BrowseDefinitionDataService } from '../core/browse/browse-definition-data.service';
 import { BrowseDefinition } from '../core/shared/browse-definition.model';
 import { DSONameService } from '../core/breadcrumbs/dso-name.service';
 import { DSpaceObject } from '../core/shared/dspace-object.model';
+import { RemoteData } from '../core/data/remote-data';
+import { PAGE_NOT_FOUND_PATH } from '../app-routing-paths';
 
 @Injectable()
 /**
@@ -21,35 +23,44 @@ export class BrowseByGuard implements CanActivate {
               protected translate: TranslateService,
               protected browseDefinitionService: BrowseDefinitionDataService,
               protected dsoNameService: DSONameService,
+              protected router: Router,
   ) {
   }
 
-  canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot) {
+  canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable<boolean> {
     const title = route.data.title;
     const id = route.params.id || route.queryParams.id || route.data.id;
-    let browseDefinition$: Observable<BrowseDefinition>;
+    let browseDefinition$: Observable<BrowseDefinition | undefined>;
     if (hasNoValue(route.data.browseDefinition) && hasValue(id)) {
-      browseDefinition$ = this.browseDefinitionService.findById(id).pipe(getFirstSucceededRemoteDataPayload());
+      browseDefinition$ = this.browseDefinitionService.findById(id).pipe(
+        getFirstCompletedRemoteData(),
+        map((browseDefinitionRD: RemoteData<BrowseDefinition>) => browseDefinitionRD.payload),
+      );
     } else {
       browseDefinition$ = observableOf(route.data.browseDefinition);
     }
     const scope = route.queryParams.scope;
     const value = route.queryParams.value;
     const metadataTranslated = this.translate.instant(`browse.metadata.${id}`);
     return browseDefinition$.pipe(
-      switchMap((browseDefinition: BrowseDefinition) => {
-        if (hasValue(scope)) {
-          const dso$: Observable<DSpaceObject> = this.dsoService.findById(scope).pipe(getFirstSucceededRemoteDataPayload());
-          return dso$.pipe(
-            map((dso: DSpaceObject) => {
-              const name = this.dsoNameService.getName(dso);
-              route.data = this.createData(title, id, browseDefinition, name, metadataTranslated, value, route);
-              return true;
-            })
-          );
+      switchMap((browseDefinition: BrowseDefinition | undefined) => {
+        if (hasValue(browseDefinition)) {
+          if (hasValue(scope)) {
+            const dso$: Observable<DSpaceObject> = this.dsoService.findById(scope).pipe(getFirstSucceededRemoteDataPayload());
+            return dso$.pipe(
+              map((dso: DSpaceObject) => {
+                const name = this.dsoNameService.getName(dso);
+                route.data = this.createData(title, id, browseDefinition, name, metadataTranslated, value, route);
+                return true;
+              })
+            );
+          } else {
+            route.data = this.createData(title, id, browseDefinition, '', metadataTranslated, value, route);
+            return observableOf(true);
+          }
         } else {
-          route.data = this.createData(title, id, browseDefinition, '', metadataTranslated, value, route);
-          return observableOf(true);
+          void this.router.navigate([PAGE_NOT_FOUND_PATH]);
+          return observableOf(false);
         }
       })
     );