114624: Refactored legacyBitstreamURL resolver into a guard to set the redirect status code to 301 Moved Permanently

(cherry picked from commit 369bd693d4)

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: {
+        canActivate: [legacyBitstreamURLRedirectGuard],
-          bitstream: LegacyBitstreamUrlResolver
-        },
       },
       {
         // Resolve JSPUI bitstream download URLs
         path: ':prefix/:suffix/:sequence_id/:filename',
         component: BitstreamDownloadPageComponent,
-        resolve: {
+        canActivate: [legacyBitstreamURLRedirectGuard],
-          bitstream: LegacyBitstreamUrlResolver
-        },
       },
       {
         // Resolve angular bitstream download URLs

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<any> };
+  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();
+        });
+      }));
+    });
+  });
+});

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<UrlTree> 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<UrlTree> => {
+  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<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`);
+      } else {
+        return router.createUrlTree([PAGE_NOT_FOUND_PATH]);
+      }
+    })
+  );
+};

src/app/bitstream-page/legacy-bitstream-url.resolver.spec.ts

@@ -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);
-        });
-      });
-    });
-  });
-});

src/app/bitstream-page/legacy-bitstream-url.resolver.ts

@@ -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<RemoteData<Bitstream>> {
-  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<<RemoteData<Item>> Emits the found bitstream based on the parameters in
-   * current route, or an error if something went wrong
-   */
-  resolve(route: ActivatedRouteSnapshot, state: RouterStateSnapshot):
-    Observable<RemoteData<Bitstream>> {
-      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()
-      );
-  }
-}

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 {
+  }
+
+}