114624: Made the legacyBitstreamURLRedirectGuard return false for val…

…id bitstream urls in combination with a HardRedirectService#redirect, this will make ensure the redirect is visible for curl instead of being performed by Angular
DSpace · May 24, 2024 · 23644e9 · 23644e9
1 parent 369bd69
commit 23644e9
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 39 deletions.
diff --git a/src/app/bitstream-page/legacy-bitstream-url-redirect.guard.spec.ts b/src/app/bitstream-page/legacy-bitstream-url-redirect.guard.spec.ts
@@ -1,36 +1,42 @@
+import { cold } from 'jasmine-marbles';
 import { EMPTY } from 'rxjs';
+
+import { PAGE_NOT_FOUND_PATH } from '../app-routing-paths';
 import { BitstreamDataService } from '../core/data/bitstream-data.service';
 import { RemoteData } from '../core/data/remote-data';
 import { RequestEntryState } from '../core/data/request-entry-state.model';
-import { legacyBitstreamURLRedirectGuard } from './legacy-bitstream-url-redirect.guard';
-import { RouterStub } from '../shared/testing/router.stub';
-import { ServerResponseServiceStub } from '../shared/testing/server-response-service.stub';
-import { fakeAsync } from '@angular/core/testing';
-import { cold } from 'jasmine-marbles';
-import { PAGE_NOT_FOUND_PATH } from '../app-routing-paths';
+import { BrowserHardRedirectService } from '../core/services/browser-hard-redirect.service';
+import { HardRedirectService } from '../core/services/hard-redirect.service';
 import { Bitstream } from '../core/shared/bitstream.model';
+import { RouterStub } from '../shared/testing/router.stub';
+import { legacyBitstreamURLRedirectGuard } from './legacy-bitstream-url-redirect.guard';
 
 describe('legacyBitstreamURLRedirectGuard', () => {
   let resolver: any;
   let bitstreamDataService: BitstreamDataService;
   let remoteDataMocks: { [type: string]: RemoteData<any> };
   let route;
   let state;
-  let serverResponseService: ServerResponseServiceStub;
+  let hardRedirectService: HardRedirectService;
   let router: RouterStub;
 
+  let bitstream: Bitstream;
+
   beforeEach(() => {
     route = {
       params: {},
       queryParams: {}
     };
     router = new RouterStub();
-    serverResponseService = new ServerResponseServiceStub();
+    hardRedirectService = new BrowserHardRedirectService(window.location);
     state = {};
+    bitstream = Object.assign(new Bitstream(), {
+      uuid: 'bitstream-id',
+    });
     remoteDataMocks = {
       RequestPending: new RemoteData(undefined, 0, 0, RequestEntryState.RequestPending, undefined, undefined, undefined),
       ResponsePending: new RemoteData(undefined, 0, 0, RequestEntryState.ResponsePending, undefined, undefined, undefined),
-      Success: new RemoteData(0, 0, 0, RequestEntryState.Success, undefined, new Bitstream(), 200),
+      Success: new RemoteData(0, 0, 0, RequestEntryState.Success, undefined, bitstream, 200),
       NoContent: new RemoteData(0, 0, 0, RequestEntryState.Success, undefined, undefined, 204),
       Error: new RemoteData(0, 0, 0, RequestEntryState.Error, 'Internal server error', undefined, 500),
     };
@@ -54,7 +60,7 @@ describe('legacyBitstreamURLRedirectGuard', () => {
         });
       });
       it(`should call findByItemHandle with the handle, sequence id, and filename from the route`, () => {
-        resolver(route, state, bitstreamDataService, serverResponseService, router);
+        resolver(route, state, bitstreamDataService, hardRedirectService, router);
         expect(bitstreamDataService.findByItemHandle).toHaveBeenCalledWith(
           `${route.params.prefix}/${route.params.suffix}`,
           route.params.sequence_id,
@@ -79,7 +85,7 @@ describe('legacyBitstreamURLRedirectGuard', () => {
           });
         });
         it(`should call findByItemHandle with the handle and filename from the route, and the sequence ID from the queryParams`, () => {
-          resolver(route, state, bitstreamDataService, serverResponseService, router);
+          resolver(route, state, bitstreamDataService, hardRedirectService, router);
           expect(bitstreamDataService.findByItemHandle).toHaveBeenCalledWith(
             `${route.params.prefix}/${route.params.suffix}`,
             route.queryParams.sequenceId,
@@ -99,7 +105,7 @@ describe('legacyBitstreamURLRedirectGuard', () => {
           });
         });
         it(`should call findByItemHandle with the handle, and filename from the route`, () => {
-          resolver(route, state, bitstreamDataService, serverResponseService, router);
+          resolver(route, state, bitstreamDataService, hardRedirectService, router);
           expect(bitstreamDataService.findByItemHandle).toHaveBeenCalledWith(
             `${route.params.prefix}/${route.params.suffix}`,
             undefined,
@@ -109,45 +115,44 @@ describe('legacyBitstreamURLRedirectGuard', () => {
       });
     });
     describe('should return and complete after the RemoteData has...', () => {
-      it('...failed', fakeAsync(() => {
+      it('...failed', () => {
         spyOn(router, 'createUrlTree').and.callThrough();
         spyOn(bitstreamDataService, 'findByItemHandle').and.returnValue(cold('a-b-c', {
           a: remoteDataMocks.RequestPending,
           b: remoteDataMocks.ResponsePending,
           c: remoteDataMocks.Error,
         }));
-        resolver(route, state, bitstreamDataService, serverResponseService, router).subscribe(() => {
+        resolver(route, state, bitstreamDataService, hardRedirectService, router).subscribe(() => {
           expect(bitstreamDataService.findByItemHandle).toHaveBeenCalled();
           expect(router.createUrlTree).toHaveBeenCalledWith([PAGE_NOT_FOUND_PATH]);
         });
-      }));
+      });
 
-      it('...succeeded without content', fakeAsync(() => {
+      it('...succeeded without content', () => {
         spyOn(router, 'createUrlTree').and.callThrough();
         spyOn(bitstreamDataService, 'findByItemHandle').and.returnValue(cold('a-b-c', {
           a: remoteDataMocks.RequestPending,
           b: remoteDataMocks.ResponsePending,
           c: remoteDataMocks.NoContent,
         }));
-        resolver(route, state, bitstreamDataService, serverResponseService, router).subscribe(() => {
+        resolver(route, state, bitstreamDataService, hardRedirectService, router).subscribe(() => {
           expect(bitstreamDataService.findByItemHandle).toHaveBeenCalled();
           expect(router.createUrlTree).toHaveBeenCalledWith([PAGE_NOT_FOUND_PATH]);
         });
-      }));
+      });
 
-      it('...succeeded', fakeAsync(() => {
-        spyOn(serverResponseService, 'setStatus').and.callThrough();
+      it('...succeeded', () => {
+        spyOn(hardRedirectService, 'redirect');
         spyOn(bitstreamDataService, 'findByItemHandle').and.returnValue(cold('a-b-c', {
           a: remoteDataMocks.RequestPending,
           b: remoteDataMocks.ResponsePending,
           c: remoteDataMocks.Success,
         }));
-        resolver(route, state, bitstreamDataService, serverResponseService, router).subscribe(() => {
+        resolver(route, state, bitstreamDataService, hardRedirectService, router).subscribe(() => {
           expect(bitstreamDataService.findByItemHandle).toHaveBeenCalled();
-          expect(serverResponseService.setStatus).toHaveBeenCalledWith(301);
-          expect(router.parseUrl).toHaveBeenCalled();
+          expect(hardRedirectService.redirect).toHaveBeenCalledWith(new URL(`/bitstreams/${bitstream.uuid}/download`, window.location.origin).href, 301);
         });
-      }));
+      });
     });
   });
 });
diff --git a/src/app/bitstream-page/legacy-bitstream-url-redirect.guard.ts b/src/app/bitstream-page/legacy-bitstream-url-redirect.guard.ts
@@ -1,14 +1,21 @@
 import { inject } from '@angular/core';
-import { ActivatedRouteSnapshot, CanActivateFn, UrlTree, Router, RouterStateSnapshot } from '@angular/router';
+import {
+  ActivatedRouteSnapshot,
+  CanActivateFn,
+  Router,
+  RouterStateSnapshot,
+  UrlTree,
+} from '@angular/router';
 import { Observable } from 'rxjs';
+import { map } from 'rxjs/operators';
+
+import { PAGE_NOT_FOUND_PATH } from '../app-routing-paths';
+import { BitstreamDataService } from '../core/data/bitstream-data.service';
 import { RemoteData } from '../core/data/remote-data';
+import { HardRedirectService } from '../core/services/hard-redirect.service';
 import { Bitstream } from '../core/shared/bitstream.model';
-import { hasNoValue } from '../shared/empty.util';
-import { BitstreamDataService } from '../core/data/bitstream-data.service';
-import { ServerResponseService } from '../core/services/server-response.service';
-import { map, tap } from 'rxjs/operators';
-import { PAGE_NOT_FOUND_PATH } from '../app-routing-paths';
 import { getFirstCompletedRemoteData } from '../core/shared/operators';
+import { hasNoValue } from '../shared/empty.util';
 
 /**
  * Redirects to a bitstream based on the handle of the item, and the sequence id or the filename of the
@@ -21,9 +28,9 @@ export const legacyBitstreamURLRedirectGuard: CanActivateFn = (
   route: ActivatedRouteSnapshot,
   state: RouterStateSnapshot,
   bitstreamDataService: BitstreamDataService = inject(BitstreamDataService),
-  serverResponseService: ServerResponseService = inject(ServerResponseService),
+  serverHardRedirectService: HardRedirectService = inject(HardRedirectService),
   router: Router = inject(Router),
-): Observable<UrlTree> => {
+): Observable<UrlTree | boolean> => {
   const prefix = route.params.prefix;
   const suffix = route.params.suffix;
   const filename = route.params.filename;
@@ -37,14 +44,10 @@ export const legacyBitstreamURLRedirectGuard: CanActivateFn = (
     filename,
   ).pipe(
     getFirstCompletedRemoteData(),
-    tap((rd: RemoteData<Bitstream>) => {
-      if (rd.hasSucceeded && !rd.hasNoContent) {
-        serverResponseService.setStatus(301);
-      }
-    }),
     map((rd: RemoteData<Bitstream>) => {
       if (rd.hasSucceeded && !rd.hasNoContent) {
-        return router.parseUrl(`/bitstreams/${rd.payload.uuid}/download`);
+        serverHardRedirectService.redirect(new URL(`/bitstreams/${rd.payload.uuid}/download`, serverHardRedirectService.getCurrentOrigin()).href, 301);
+        return false;
       } else {
         return router.createUrlTree([PAGE_NOT_FOUND_PATH]);
       }

diff --git a/src/app/core/data/bitstream-data.service.ts b/src/app/core/data/bitstream-data.service.ts
@@ -171,7 +171,7 @@ export class BitstreamDataService extends IdentifiableDataService<Bitstream> imp
       searchParams.push(new RequestParam('sequenceId', sequenceId));
     }
     if (hasValue(filename)) {
-      searchParams.push(new RequestParam('filename', filename));
+      searchParams.push(new RequestParam('filename', encodeURIComponent(filename)));
     }
 
     const hrefObs = this.getSearchByHref(