diff --git a/src/app/+bitstream-page/bitstream-page-routing.module.ts b/src/app/+bitstream-page/bitstream-page-routing.module.ts index 284f29f7b4..27b9db9a05 100644 --- a/src/app/+bitstream-page/bitstream-page-routing.module.ts +++ b/src/app/+bitstream-page/bitstream-page-routing.module.ts @@ -9,6 +9,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'; const EDIT_BITSTREAM_PATH = ':id/edit'; const EDIT_BITSTREAM_AUTHORIZATIONS_PATH = ':id/authorizations'; @@ -20,7 +21,24 @@ const EDIT_BITSTREAM_AUTHORIZATIONS_PATH = ':id/authorizations'; imports: [ RouterModule.forChild([ { - path:':id/download', + // Resolve XMLUI bitstream download URLs + path: 'handle/:prefix/:suffix/:filename', + component: BitstreamDownloadPageComponent, + resolve: { + bitstream: LegacyBitstreamUrlResolver + }, + }, + { + // Resolve JSPUI bitstream download URLs + path: ':prefix/:suffix/:sequence_id/:filename', + component: BitstreamDownloadPageComponent, + resolve: { + bitstream: LegacyBitstreamUrlResolver + }, + }, + { + // Resolve angular bitstream download URLs + path: ':id/download', component: BitstreamDownloadPageComponent, resolve: { bitstream: BitstreamPageResolver diff --git a/src/app/+bitstream-page/legacy-bitstream-url.resolver.spec.ts b/src/app/+bitstream-page/legacy-bitstream-url.resolver.spec.ts new file mode 100644 index 0000000000..25e245c5b7 --- /dev/null +++ b/src/app/+bitstream-page/legacy-bitstream-url.resolver.spec.ts @@ -0,0 +1,145 @@ +import { LegacyBitstreamUrlResolver } from './legacy-bitstream-url.resolver'; +import { of as observableOf, EMPTY } from 'rxjs'; +import { BitstreamDataService } from '../core/data/bitstream-data.service'; +import { RemoteData } from '../core/data/remote-data'; +import { RequestEntryState } from '../core/data/request.reducer'; +import { TestScheduler } from 'rxjs/testing'; + +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 new file mode 100644 index 0000000000..948bec2473 --- /dev/null +++ b/src/app/+bitstream-page/legacy-bitstream-url.resolver.ts @@ -0,0 +1,48 @@ +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/app-routing-paths.ts b/src/app/app-routing-paths.ts index 7dfdbd2c49..6ec8463f41 100644 --- a/src/app/app-routing-paths.ts +++ b/src/app/app-routing-paths.ts @@ -10,6 +10,11 @@ import { URLCombiner } from './core/url-combiner/url-combiner'; export const BITSTREAM_MODULE_PATH = 'bitstreams'; +/** + * The bitstream module path to resolve XMLUI and JSPUI bitstream download URLs + */ +export const LEGACY_BITSTREAM_MODULE_PATH = 'bitstream'; + export function getBitstreamModuleRoute() { return `/${BITSTREAM_MODULE_PATH}`; } diff --git a/src/app/app-routing.module.ts b/src/app/app-routing.module.ts index ffbd993e8c..56672482f1 100644 --- a/src/app/app-routing.module.ts +++ b/src/app/app-routing.module.ts @@ -14,6 +14,7 @@ import { PROFILE_MODULE_PATH, REGISTER_PATH, WORKFLOW_ITEM_MODULE_PATH, + LEGACY_BITSTREAM_MODULE_PATH, } from './app-routing-paths'; import { COLLECTION_MODULE_PATH } from './+collection-page/collection-page-routing-paths'; import { COMMUNITY_MODULE_PATH } from './+community-page/community-page-routing-paths'; @@ -93,6 +94,12 @@ import { GroupAdministratorGuard } from './core/data/feature-authorization/featu .then((m) => m.ItemPageModule), canActivate: [EndUserAgreementCurrentUserGuard] }, + { + path: LEGACY_BITSTREAM_MODULE_PATH, + loadChildren: () => import('./+bitstream-page/bitstream-page.module') + .then((m) => m.BitstreamPageModule), + canActivate: [EndUserAgreementCurrentUserGuard] + }, { path: BITSTREAM_MODULE_PATH, loadChildren: () => import('./+bitstream-page/bitstream-page.module') diff --git a/src/app/core/data/bitstream-data.service.ts b/src/app/core/data/bitstream-data.service.ts index 3890f4e663..23aec80ff2 100644 --- a/src/app/core/data/bitstream-data.service.ts +++ b/src/app/core/data/bitstream-data.service.ts @@ -28,6 +28,7 @@ import { HttpOptions } from '../dspace-rest/dspace-rest.service'; import { sendRequest } from '../shared/operators'; import { createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; import { PageInfo } from '../shared/page-info.model'; +import { RequestParam } from '../cache/models/request-param.model'; /** * A service to retrieve {@link Bitstream}s from the REST API @@ -136,4 +137,50 @@ export class BitstreamDataService extends DataService { return this.rdbService.buildFromRequestUUID(requestId); } + /** + * Returns an observable of {@link RemoteData} of a {@link Bitstream}, based on a handle and an + * optional sequenceId or filename, with a list of {@link FollowLinkConfig}, to automatically + * resolve {@link HALLink}s of the object + * + * @param handle The handle of the bitstream we want to retrieve + * @param sequenceId The sequence id of the bitstream we want to retrieve + * @param filename The filename of the bitstream we want to retrieve + * @param useCachedVersionIfAvailable If this is true, the request will only be sent if there's + * no valid cached version. Defaults to true + * @param reRequestOnStale Whether or not the request should automatically be re- + * requested after the response becomes stale + * @param linksToFollow List of {@link FollowLinkConfig} that indicate which + * {@link HALLink}s should be automatically resolved + */ + findByItemHandle( + handle: string, + sequenceId?: string, + filename?: string, + useCachedVersionIfAvailable = true, + reRequestOnStale = true, + ...linksToFollow: FollowLinkConfig[] + ): Observable> { + const searchParams = []; + searchParams.push(new RequestParam('handle', handle)); + if (hasValue(sequenceId)) { + searchParams.push(new RequestParam('sequenceId', sequenceId)); + } + if (hasValue(filename)) { + searchParams.push(new RequestParam('filename', filename)); + } + + const hrefObs = this.getSearchByHref( + 'byItemHandle', + { searchParams }, + ...linksToFollow + ); + + return this.findByHref( + hrefObs, + useCachedVersionIfAvailable, + reRequestOnStale, + ...linksToFollow + ); + } + } diff --git a/src/app/core/shared/operators.spec.ts b/src/app/core/shared/operators.spec.ts index 780487eade..6fd15ceacc 100644 --- a/src/app/core/shared/operators.spec.ts +++ b/src/app/core/shared/operators.spec.ts @@ -15,7 +15,12 @@ import { redirectOn4xx } from './operators'; import { of as observableOf } from 'rxjs'; -import { createFailedRemoteDataObject, createSuccessfulRemoteDataObject } from '../../shared/remote-data.utils'; +import { + createFailedRemoteDataObject, + createSuccessfulRemoteDataObject +} from '../../shared/remote-data.utils'; + +// tslint:disable:no-shadowed-variable describe('Core Module - RxJS Operators', () => { let scheduler: TestScheduler; @@ -172,8 +177,12 @@ describe('Core Module - RxJS Operators', () => { describe('redirectOn4xx', () => { let router; let authService; + let testScheduler; beforeEach(() => { + testScheduler = new TestScheduler((actual, expected) => { + expect(actual).toEqual(expected); + }); router = jasmine.createSpyObj('router', ['navigateByUrl']); authService = jasmine.createSpyObj('authService', { isAuthenticated: observableOf(true), @@ -181,32 +190,69 @@ describe('Core Module - RxJS Operators', () => { }); }); - it('should call navigateByUrl to a 404 page, when the remote data contains a 404 error', () => { - const testRD = createFailedRemoteDataObject('Object was not found', 404); + it('should call navigateByUrl to a 404 page, when the remote data contains a 404 error, and not emit anything', () => { + testScheduler.run(({ cold, expectObservable, flush }) => { + const testRD = createFailedRemoteDataObject('Object was not found', 404); + const source = cold('a', { a: testRD }); + const expected = '-'; + const values = {}; - observableOf(testRD).pipe(redirectOn4xx(router, authService)).subscribe(); - expect(router.navigateByUrl).toHaveBeenCalledWith('/404', { skipLocationChange: true }); + expectObservable(source.pipe(redirectOn4xx(router, authService))).toBe(expected, values); + flush(); + expect(router.navigateByUrl).toHaveBeenCalledWith('/404', { skipLocationChange: true }); + }); }); - it('should call navigateByUrl to a 401 page, when the remote data contains a 403 error', () => { - const testRD = createFailedRemoteDataObject('Forbidden', 403); + it('should call navigateByUrl to a 404 page, when the remote data contains a 422 error, and not emit anything', () => { + testScheduler.run(({ cold, expectObservable, flush }) => { + const testRD = createFailedRemoteDataObject('Unprocessable Entity', 422); + const source = cold('a', { a: testRD }); + const expected = '-'; + const values = {}; - observableOf(testRD).pipe(redirectOn4xx(router, authService)).subscribe(); - expect(router.navigateByUrl).toHaveBeenCalledWith('/403', { skipLocationChange: true }); + expectObservable(source.pipe(redirectOn4xx(router, authService))).toBe(expected, values); + flush(); + expect(router.navigateByUrl).toHaveBeenCalledWith('/404', { skipLocationChange: true }); + }); }); - it('should not call navigateByUrl to a 404, 403 or 401 page, when the remote data contains another error than a 404, 403 or 401', () => { - const testRD = createFailedRemoteDataObject('Something went wrong', 500); + it('should call navigateByUrl to a 401 page, when the remote data contains a 403 error, and not emit anything', () => { + testScheduler.run(({ cold, expectObservable, flush }) => { + const testRD = createFailedRemoteDataObject('Forbidden', 403); + const source = cold('a', { a: testRD }); + const expected = '-'; + const values = {}; - observableOf(testRD).pipe(redirectOn4xx(router, authService)).subscribe(); - expect(router.navigateByUrl).not.toHaveBeenCalled(); + expectObservable(source.pipe(redirectOn4xx(router, authService))).toBe(expected, values); + flush(); + expect(router.navigateByUrl).toHaveBeenCalledWith('/403', { skipLocationChange: true }); + }); }); - it('should not call navigateByUrl to a 404, 403 or 401 page, when the remote data contains no error', () => { - const testRD = createSuccessfulRemoteDataObject(undefined); + it('should not call navigateByUrl to a 404, 403 or 401 page, when the remote data contains another error than a 404, 422, 403 or 401, and emit the source rd', () => { + testScheduler.run(({ cold, expectObservable, flush }) => { + const testRD = createFailedRemoteDataObject('Something went wrong', 500); + const source = cold('a', { a: testRD }); + const expected = 'a'; + const values = { a: testRD }; - observableOf(testRD).pipe(redirectOn4xx(router, authService)).subscribe(); - expect(router.navigateByUrl).not.toHaveBeenCalled(); + expectObservable(source.pipe(redirectOn4xx(router, authService))).toBe(expected, values); + flush(); + expect(router.navigateByUrl).not.toHaveBeenCalled(); + }); + }); + + it('should not call navigateByUrl to a 404, 403 or 401 page, when the remote data contains no error, and emit the source rd', () => { + testScheduler.run(({ cold, expectObservable, flush }) => { + const testRD = createSuccessfulRemoteDataObject(undefined); + const source = cold('a', { a: testRD }); + const expected = 'a'; + const values = { a: testRD }; + + expectObservable(source.pipe(redirectOn4xx(router, authService))).toBe(expected, values); + flush(); + expect(router.navigateByUrl).not.toHaveBeenCalled(); + }); }); describe('when the user is not authenticated', () => { @@ -214,20 +260,32 @@ describe('Core Module - RxJS Operators', () => { (authService.isAuthenticated as jasmine.Spy).and.returnValue(observableOf(false)); }); - it('should set the redirect url and navigate to login when the remote data contains a 401 error', () => { - const testRD = createFailedRemoteDataObject('The current user is unauthorized', 401); + it('should set the redirect url and navigate to login when the remote data contains a 401 error, and not emit anything', () => { + testScheduler.run(({ cold, expectObservable, flush }) => { + const testRD = createFailedRemoteDataObject('The current user is unauthorized', 401); + const source = cold('a', { a: testRD }); + const expected = '-'; + const values = {}; - observableOf(testRD).pipe(redirectOn4xx(router, authService)).subscribe(); - expect(authService.setRedirectUrl).toHaveBeenCalled(); - expect(router.navigateByUrl).toHaveBeenCalledWith('login'); + expectObservable(source.pipe(redirectOn4xx(router, authService))).toBe(expected, values); + flush(); + expect(authService.setRedirectUrl).toHaveBeenCalled(); + expect(router.navigateByUrl).toHaveBeenCalledWith('login'); + }); }); - it('should set the redirect url and navigate to login when the remote data contains a 403 error', () => { - const testRD = createFailedRemoteDataObject('Forbidden', 403); + it('should set the redirect url and navigate to login when the remote data contains a 403 error, and not emit anything', () => { + testScheduler.run(({ cold, expectObservable, flush }) => { + const testRD = createFailedRemoteDataObject('Forbidden', 403); + const source = cold('a', { a: testRD }); + const expected = '-'; + const values = {}; - observableOf(testRD).pipe(redirectOn4xx(router, authService)).subscribe(); - expect(authService.setRedirectUrl).toHaveBeenCalled(); - expect(router.navigateByUrl).toHaveBeenCalledWith('login'); + expectObservable(source.pipe(redirectOn4xx(router, authService))).toBe(expected, values); + flush(); + expect(authService.setRedirectUrl).toHaveBeenCalled(); + expect(router.navigateByUrl).toHaveBeenCalledWith('login'); + }); }); }); }); diff --git a/src/app/core/shared/operators.ts b/src/app/core/shared/operators.ts index 20aaf23ba8..3be04447ab 100644 --- a/src/app/core/shared/operators.ts +++ b/src/app/core/shared/operators.ts @@ -1,6 +1,17 @@ import { Router, UrlTree } from '@angular/router'; import { combineLatest as observableCombineLatest, Observable } from 'rxjs'; -import { debounceTime, filter, find, map, mergeMap, switchMap, take, takeWhile, tap } from 'rxjs/operators'; +import { + debounceTime, + filter, + find, + map, + mergeMap, + switchMap, + take, + takeWhile, + tap, + withLatestFrom +} from 'rxjs/operators'; import { hasNoValue, hasValue, hasValueOperator, isNotEmpty } from '../../shared/empty.util'; import { SearchResult } from '../../shared/search/search-result.model'; import { PaginatedList } from '../data/paginated-list.model'; @@ -22,6 +33,11 @@ export const DEBOUNCE_TIME_OPERATOR = new InjectionToken<(dueTime: number) => factory: () => debounceTime }); +export const REDIRECT_ON_4XX = new InjectionToken<(router: Router, authService: AuthService) => (source: Observable>) => Observable>>('redirectOn4xx', { + providedIn: 'root', + factory: () => redirectOn4xx +}); + /** * This file contains custom RxJS operators that can be used in multiple places */ @@ -175,29 +191,37 @@ export const getAllSucceededRemoteListPayload = () => ); /** - * Operator that checks if a remote data object returned a 401 or 404 error - * When it does contain such an error, it will redirect the user to the related error page, without altering the current URL + * Operator that checks if a remote data object returned a 4xx error + * When it does contain such an error, it will redirect the user to the related error page, without + * altering the current URL + * * @param router The router used to navigate to a new page * @param authService Service to check if the user is authenticated */ export const redirectOn4xx = (router: Router, authService: AuthService) => (source: Observable>): Observable> => - observableCombineLatest(source, authService.isAuthenticated()).pipe( - map(([rd, isAuthenticated]: [RemoteData, boolean]) => { + source.pipe( + withLatestFrom(authService.isAuthenticated()), + filter(([rd, isAuthenticated]: [RemoteData, boolean]) => { if (rd.hasFailed) { - if (rd.statusCode === 404) { - router.navigateByUrl(getPageNotFoundRoute(), {skipLocationChange: true}); + if (rd.statusCode === 404 || rd.statusCode === 422) { + router.navigateByUrl(getPageNotFoundRoute(), { skipLocationChange: true }); + return false; } else if (rd.statusCode === 403 || rd.statusCode === 401) { if (isAuthenticated) { - router.navigateByUrl(getForbiddenRoute(), {skipLocationChange: true}); + router.navigateByUrl(getForbiddenRoute(), { skipLocationChange: true }); + return false; } else { authService.setRedirectUrl(router.url); router.navigateByUrl('login'); + return false; } } } - return rd; - })); + return true; + }), + map(([rd,]: [RemoteData, boolean]) => rd) + ); /** * Operator that returns a UrlTree to a forbidden page or the login page when the boolean received is false