From 369bd693d4bdcbb3a289e5ee0984206207aa6bfc Mon Sep 17 00:00:00 2001 From: Kuno Vercammen Date: Wed, 24 Apr 2024 11:41:35 +0200 Subject: [PATCH 1/2] 114624: Refactored legacyBitstreamURL resolver into a guard to set the redirect status code to 301 Moved Permanently --- .../bitstream-page-routing.module.ts | 10 +- ...egacy-bitstream-url-redirect.guard.spec.ts | 153 ++++++++++++++++++ .../legacy-bitstream-url-redirect.guard.ts | 53 ++++++ .../legacy-bitstream-url.resolver.spec.ts | 145 ----------------- .../legacy-bitstream-url.resolver.ts | 48 ------ .../testing/server-response-service.stub.ts | 30 ++++ 6 files changed, 239 insertions(+), 200 deletions(-) create mode 100644 src/app/bitstream-page/legacy-bitstream-url-redirect.guard.spec.ts create mode 100644 src/app/bitstream-page/legacy-bitstream-url-redirect.guard.ts delete mode 100644 src/app/bitstream-page/legacy-bitstream-url.resolver.spec.ts delete mode 100644 src/app/bitstream-page/legacy-bitstream-url.resolver.ts create mode 100644 src/app/shared/testing/server-response-service.stub.ts diff --git a/src/app/bitstream-page/bitstream-page-routing.module.ts b/src/app/bitstream-page/bitstream-page-routing.module.ts index 3960ccb7436..f378b301aa8 100644 --- a/src/app/bitstream-page/bitstream-page-routing.module.ts +++ b/src/app/bitstream-page/bitstream-page-routing.module.ts @@ -8,7 +8,7 @@ import { ResourcePolicyCreateComponent } from '../shared/resource-policies/creat import { ResourcePolicyResolver } from '../shared/resource-policies/resolvers/resource-policy.resolver'; import { ResourcePolicyEditComponent } from '../shared/resource-policies/edit/resource-policy-edit.component'; import { BitstreamAuthorizationsComponent } from './bitstream-authorizations/bitstream-authorizations.component'; -import { LegacyBitstreamUrlResolver } from './legacy-bitstream-url.resolver'; +import { legacyBitstreamURLRedirectGuard } from './legacy-bitstream-url-redirect.guard'; import { BitstreamBreadcrumbResolver } from '../core/breadcrumbs/bitstream-breadcrumb.resolver'; import { BitstreamBreadcrumbsService } from '../core/breadcrumbs/bitstream-breadcrumbs.service'; import { I18nBreadcrumbResolver } from '../core/breadcrumbs/i18n-breadcrumb.resolver'; @@ -27,17 +27,13 @@ const EDIT_BITSTREAM_AUTHORIZATIONS_PATH = ':id/authorizations'; // Resolve XMLUI bitstream download URLs path: 'handle/:prefix/:suffix/:filename', component: BitstreamDownloadPageComponent, - resolve: { - bitstream: LegacyBitstreamUrlResolver - }, + canActivate: [legacyBitstreamURLRedirectGuard], }, { // Resolve JSPUI bitstream download URLs path: ':prefix/:suffix/:sequence_id/:filename', component: BitstreamDownloadPageComponent, - resolve: { - bitstream: LegacyBitstreamUrlResolver - }, + canActivate: [legacyBitstreamURLRedirectGuard], }, { // Resolve angular bitstream download URLs 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 new file mode 100644 index 00000000000..8ca14231e89 --- /dev/null +++ b/src/app/bitstream-page/legacy-bitstream-url-redirect.guard.spec.ts @@ -0,0 +1,153 @@ +import { EMPTY } from 'rxjs'; +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 { Bitstream } from '../core/shared/bitstream.model'; + +describe('legacyBitstreamURLRedirectGuard', () => { + let resolver: any; + let bitstreamDataService: BitstreamDataService; + let remoteDataMocks: { [type: string]: RemoteData }; + let route; + let state; + let serverResponseService: ServerResponseServiceStub; + let router: RouterStub; + + beforeEach(() => { + route = { + params: {}, + queryParams: {} + }; + router = new RouterStub(); + serverResponseService = new ServerResponseServiceStub(); + state = {}; + 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), + NoContent: new RemoteData(0, 0, 0, RequestEntryState.Success, undefined, undefined, 204), + Error: new RemoteData(0, 0, 0, RequestEntryState.Error, 'Internal server error', undefined, 500), + }; + bitstreamDataService = { + findByItemHandle: () => undefined + } as any; + resolver = legacyBitstreamURLRedirectGuard; + }); + + describe(`resolve`, () => { + describe(`For JSPUI-style URLs`, () => { + beforeEach(() => { + spyOn(bitstreamDataService, 'findByItemHandle').and.returnValue(EMPTY); + route = Object.assign({}, route, { + params: { + prefix: '123456789', + suffix: '1234', + filename: 'some-file.pdf', + sequence_id: '5' + } + }); + }); + it(`should call findByItemHandle with the handle, sequence id, and filename from the route`, () => { + resolver(route, state, bitstreamDataService, serverResponseService, router); + expect(bitstreamDataService.findByItemHandle).toHaveBeenCalledWith( + `${route.params.prefix}/${route.params.suffix}`, + route.params.sequence_id, + route.params.filename + ); + }); + }); + + describe(`For XMLUI-style URLs`, () => { + describe(`when there is a sequenceId query parameter`, () => { + beforeEach(() => { + spyOn(bitstreamDataService, 'findByItemHandle').and.returnValue(EMPTY); + route = Object.assign({}, route, { + params: { + prefix: '123456789', + suffix: '1234', + filename: 'some-file.pdf', + }, + queryParams: { + sequenceId: '5' + } + }); + }); + 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); + expect(bitstreamDataService.findByItemHandle).toHaveBeenCalledWith( + `${route.params.prefix}/${route.params.suffix}`, + route.queryParams.sequenceId, + route.params.filename + ); + }); + }); + describe(`when there's no sequenceId query parameter`, () => { + beforeEach(() => { + spyOn(bitstreamDataService, 'findByItemHandle').and.returnValue(EMPTY); + route = Object.assign({}, route, { + params: { + prefix: '123456789', + suffix: '1234', + filename: 'some-file.pdf', + }, + }); + }); + it(`should call findByItemHandle with the handle, and filename from the route`, () => { + resolver(route, state, bitstreamDataService, serverResponseService, router); + expect(bitstreamDataService.findByItemHandle).toHaveBeenCalledWith( + `${route.params.prefix}/${route.params.suffix}`, + undefined, + route.params.filename + ); + }); + }); + }); + describe('should return and complete after the RemoteData has...', () => { + it('...failed', fakeAsync(() => { + 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(() => { + expect(bitstreamDataService.findByItemHandle).toHaveBeenCalled(); + expect(router.createUrlTree).toHaveBeenCalledWith([PAGE_NOT_FOUND_PATH]); + }); + })); + + it('...succeeded without content', fakeAsync(() => { + 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(() => { + expect(bitstreamDataService.findByItemHandle).toHaveBeenCalled(); + expect(router.createUrlTree).toHaveBeenCalledWith([PAGE_NOT_FOUND_PATH]); + }); + })); + + it('...succeeded', fakeAsync(() => { + spyOn(serverResponseService, 'setStatus').and.callThrough(); + 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(() => { + expect(bitstreamDataService.findByItemHandle).toHaveBeenCalled(); + expect(serverResponseService.setStatus).toHaveBeenCalledWith(301); + expect(router.parseUrl).toHaveBeenCalled(); + }); + })); + }); + }); +}); diff --git a/src/app/bitstream-page/legacy-bitstream-url-redirect.guard.ts b/src/app/bitstream-page/legacy-bitstream-url-redirect.guard.ts new file mode 100644 index 00000000000..6c6357b5a54 --- /dev/null +++ b/src/app/bitstream-page/legacy-bitstream-url-redirect.guard.ts @@ -0,0 +1,53 @@ +import { inject } from '@angular/core'; +import { ActivatedRouteSnapshot, CanActivateFn, UrlTree, Router, RouterStateSnapshot } from '@angular/router'; +import { Observable } from 'rxjs'; +import { RemoteData } from '../core/data/remote-data'; +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'; + +/** + * Redirects to a bitstream based on the handle of the item, and the sequence id or the filename of the + * bitstream. In production mode the status code will also be set the status code to 301 marking it as a permanent URL + * redirect for bots. + * + * @returns Observable Returns a URL to redirect the user to the new URL format + */ +export const legacyBitstreamURLRedirectGuard: CanActivateFn = ( + route: ActivatedRouteSnapshot, + state: RouterStateSnapshot, + bitstreamDataService: BitstreamDataService = inject(BitstreamDataService), + serverResponseService: ServerResponseService = inject(ServerResponseService), + router: Router = inject(Router), +): Observable => { + const prefix = route.params.prefix; + const suffix = route.params.suffix; + const filename = route.params.filename; + let sequenceId = route.params.sequence_id; + if (hasNoValue(sequenceId)) { + sequenceId = route.queryParams.sequenceId; + } + return bitstreamDataService.findByItemHandle( + `${prefix}/${suffix}`, + sequenceId, + filename, + ).pipe( + getFirstCompletedRemoteData(), + tap((rd: RemoteData) => { + if (rd.hasSucceeded && !rd.hasNoContent) { + serverResponseService.setStatus(301); + } + }), + map((rd: RemoteData) => { + if (rd.hasSucceeded && !rd.hasNoContent) { + return router.parseUrl(`/bitstreams/${rd.payload.uuid}/download`); + } else { + return router.createUrlTree([PAGE_NOT_FOUND_PATH]); + } + }) + ); +}; diff --git a/src/app/bitstream-page/legacy-bitstream-url.resolver.spec.ts b/src/app/bitstream-page/legacy-bitstream-url.resolver.spec.ts deleted file mode 100644 index aec8cd22f44..00000000000 --- a/src/app/bitstream-page/legacy-bitstream-url.resolver.spec.ts +++ /dev/null @@ -1,145 +0,0 @@ -import { LegacyBitstreamUrlResolver } from './legacy-bitstream-url.resolver'; -import { EMPTY } from 'rxjs'; -import { BitstreamDataService } from '../core/data/bitstream-data.service'; -import { RemoteData } from '../core/data/remote-data'; -import { TestScheduler } from 'rxjs/testing'; -import { RequestEntryState } from '../core/data/request-entry-state.model'; - -describe(`LegacyBitstreamUrlResolver`, () => { - let resolver: LegacyBitstreamUrlResolver; - let bitstreamDataService: BitstreamDataService; - let testScheduler; - let remoteDataMocks; - let route; - let state; - - beforeEach(() => { - testScheduler = new TestScheduler((actual, expected) => { - expect(actual).toEqual(expected); - }); - - route = { - params: {}, - queryParams: {} - }; - state = {}; - 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, {}, 200), - Error: new RemoteData(0, 0, 0, RequestEntryState.Error, 'Internal server error', undefined, 500), - }; - bitstreamDataService = { - findByItemHandle: () => undefined - } as any; - resolver = new LegacyBitstreamUrlResolver(bitstreamDataService); - }); - - describe(`resolve`, () => { - describe(`For JSPUI-style URLs`, () => { - beforeEach(() => { - spyOn(bitstreamDataService, 'findByItemHandle').and.returnValue(EMPTY); - route = Object.assign({}, route, { - params: { - prefix: '123456789', - suffix: '1234', - filename: 'some-file.pdf', - sequence_id: '5' - } - }); - }); - it(`should call findByItemHandle with the handle, sequence id, and filename from the route`, () => { - testScheduler.run(() => { - resolver.resolve(route, state); - expect(bitstreamDataService.findByItemHandle).toHaveBeenCalledWith( - `${route.params.prefix}/${route.params.suffix}`, - route.params.sequence_id, - route.params.filename - ); - }); - }); - }); - - describe(`For XMLUI-style URLs`, () => { - describe(`when there is a sequenceId query parameter`, () => { - beforeEach(() => { - spyOn(bitstreamDataService, 'findByItemHandle').and.returnValue(EMPTY); - route = Object.assign({}, route, { - params: { - prefix: '123456789', - suffix: '1234', - filename: 'some-file.pdf', - }, - queryParams: { - sequenceId: '5' - } - }); - }); - it(`should call findByItemHandle with the handle and filename from the route, and the sequence ID from the queryParams`, () => { - testScheduler.run(() => { - resolver.resolve(route, state); - expect(bitstreamDataService.findByItemHandle).toHaveBeenCalledWith( - `${route.params.prefix}/${route.params.suffix}`, - route.queryParams.sequenceId, - route.params.filename - ); - }); - }); - }); - describe(`when there's no sequenceId query parameter`, () => { - beforeEach(() => { - spyOn(bitstreamDataService, 'findByItemHandle').and.returnValue(EMPTY); - route = Object.assign({}, route, { - params: { - prefix: '123456789', - suffix: '1234', - filename: 'some-file.pdf', - }, - }); - }); - it(`should call findByItemHandle with the handle, and filename from the route`, () => { - testScheduler.run(() => { - resolver.resolve(route, state); - expect(bitstreamDataService.findByItemHandle).toHaveBeenCalledWith( - `${route.params.prefix}/${route.params.suffix}`, - undefined, - route.params.filename - ); - }); - }); - }); - }); - describe(`should return and complete after the remotedata has...`, () => { - it(`...failed`, () => { - testScheduler.run(({ cold, expectObservable }) => { - spyOn(bitstreamDataService, 'findByItemHandle').and.returnValue(cold('a-b-c', { - a: remoteDataMocks.RequestPending, - b: remoteDataMocks.ResponsePending, - c: remoteDataMocks.Error, - })); - const expected = '----(c|)'; - const values = { - c: remoteDataMocks.Error, - }; - - expectObservable(resolver.resolve(route, state)).toBe(expected, values); - }); - }); - it(`...succeeded`, () => { - testScheduler.run(({ cold, expectObservable }) => { - spyOn(bitstreamDataService, 'findByItemHandle').and.returnValue(cold('a-b-c', { - a: remoteDataMocks.RequestPending, - b: remoteDataMocks.ResponsePending, - c: remoteDataMocks.Success, - })); - const expected = '----(c|)'; - const values = { - c: remoteDataMocks.Success, - }; - - expectObservable(resolver.resolve(route, state)).toBe(expected, values); - }); - }); - }); - }); -}); diff --git a/src/app/bitstream-page/legacy-bitstream-url.resolver.ts b/src/app/bitstream-page/legacy-bitstream-url.resolver.ts deleted file mode 100644 index 948bec24731..00000000000 --- a/src/app/bitstream-page/legacy-bitstream-url.resolver.ts +++ /dev/null @@ -1,48 +0,0 @@ -import { Injectable } from '@angular/core'; -import { ActivatedRouteSnapshot, Resolve, RouterStateSnapshot } from '@angular/router'; -import { Observable } from 'rxjs'; -import { RemoteData } from '../core/data/remote-data'; -import { Bitstream } from '../core/shared/bitstream.model'; -import { getFirstCompletedRemoteData } from '../core/shared/operators'; -import { hasNoValue } from '../shared/empty.util'; -import { BitstreamDataService } from '../core/data/bitstream-data.service'; - -/** - * This class resolves a bitstream based on the DSpace 6 XMLUI or JSPUI bitstream download URLs - */ -@Injectable({ - providedIn: 'root' -}) -export class LegacyBitstreamUrlResolver implements Resolve> { - constructor(protected bitstreamDataService: BitstreamDataService) { - } - - /** - * Resolve a bitstream based on the handle of the item, and the sequence id or the filename of the - * bitstream - * - * @param {ActivatedRouteSnapshot} route The current ActivatedRouteSnapshot - * @param {RouterStateSnapshot} state The current RouterStateSnapshot - * @returns Observable<> Emits the found bitstream based on the parameters in - * current route, or an error if something went wrong - */ - resolve(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): - Observable> { - const prefix = route.params.prefix; - const suffix = route.params.suffix; - const filename = route.params.filename; - - let sequenceId = route.params.sequence_id; - if (hasNoValue(sequenceId)) { - sequenceId = route.queryParams.sequenceId; - } - - return this.bitstreamDataService.findByItemHandle( - `${prefix}/${suffix}`, - sequenceId, - filename, - ).pipe( - getFirstCompletedRemoteData() - ); - } -} diff --git a/src/app/shared/testing/server-response-service.stub.ts b/src/app/shared/testing/server-response-service.stub.ts new file mode 100644 index 00000000000..fd6e4a0d240 --- /dev/null +++ b/src/app/shared/testing/server-response-service.stub.ts @@ -0,0 +1,30 @@ +/* eslint-disable no-empty,@typescript-eslint/no-empty-function */ +/** + * Stub class of {@link ServerResponseService} + */ +export class ServerResponseServiceStub { + + setStatus(_code: number, _message?: string): this { + return this; + } + + setUnauthorized(_message = 'Unauthorized'): this { + return this; + } + + setForbidden(_message = 'Forbidden'): this { + return this; + } + + setNotFound(_message = 'Not found'): this { + return this; + } + + setInternalServerError(_message = 'Internal Server Error'): this { + return this; + } + + setHeader(_header: string, _content: string): void { + } + +} From 23644e9ec70bef8bfbb3159412c56314e89c2106 Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Fri, 24 May 2024 14:21:57 +0200 Subject: [PATCH 2/2] 114624: Made the legacyBitstreamURLRedirectGuard return false for valid bitstream urls in combination with a HardRedirectService#redirect, this will make ensure the redirect is visible for curl instead of being performed by Angular --- ...egacy-bitstream-url-redirect.guard.spec.ts | 53 ++++++++++--------- .../legacy-bitstream-url-redirect.guard.ts | 31 ++++++----- src/app/core/data/bitstream-data.service.ts | 2 +- 3 files changed, 47 insertions(+), 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 index 8ca14231e89..38be4dbb389 100644 --- 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,14 +1,15 @@ +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; @@ -16,21 +17,26 @@ describe('legacyBitstreamURLRedirectGuard', () => { let remoteDataMocks: { [type: string]: RemoteData }; 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 index 6c6357b5a54..d8d8932c30d 100644 --- 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 => { +): Observable => { 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) => { - if (rd.hasSucceeded && !rd.hasNoContent) { - serverResponseService.setStatus(301); - } - }), map((rd: RemoteData) => { 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 index bb4ec281665..bf8e3585df4 100644 --- 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 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(