Merge pull request #2331 from tdonohue/ssr_redirect_301

Implement basic 301 Redirect for DSpace 6.x URLs when SSR is used.

src/app/core/data/dso-redirect.service.spec.ts

@@ -10,6 +10,7 @@ import { RequestService } from './request.service';
 import { createSuccessfulRemoteDataObject } from '../../shared/remote-data.utils';
 import { Item } from '../shared/item.model';
 import { EMBED_SEPARATOR } from './base/base-data.service';
+import { HardRedirectService } from '../services/hard-redirect.service';
 
 describe('DsoRedirectService', () => {
   let scheduler: TestScheduler;
@@ -17,7 +18,7 @@ describe('DsoRedirectService', () => {
   let halService: HALEndpointService;
   let requestService: RequestService;
   let rdbService: RemoteDataBuildService;
-  let router;
+  let redirectService: HardRedirectService;
   let remoteData;
   const dsoUUID = '9b4f22f4-164a-49db-8817-3316b6ee5746';
   const dsoHandle = '1234567789/22';
@@ -38,9 +39,6 @@ describe('DsoRedirectService', () => {
       generateRequestId: requestUUID,
       send: true
     });
-    router = {
-      navigate: jasmine.createSpy('navigate')
-    };
 
     remoteData = createSuccessfulRemoteDataObject(Object.assign(new Item(), {
       type: 'item',
@@ -52,12 +50,17 @@ describe('DsoRedirectService', () => {
         a: remoteData
       })
     });
+
+    redirectService = jasmine.createSpyObj('redirectService', {
+      redirect: {}
+    });
+
     service = new DsoRedirectService(
       requestService,
       rdbService,
       objectCache,
       halService,
-      router,
+      redirectService
     );
   });
 
@@ -104,7 +107,7 @@ describe('DsoRedirectService', () => {
       redir.subscribe();
       scheduler.schedule(() => redir);
       scheduler.flush();
-      expect(router.navigate).toHaveBeenCalledWith(['/items/' + remoteData.payload.uuid]);
+      expect(redirectService.redirect).toHaveBeenCalledWith('/items/' + remoteData.payload.uuid, 301);
     });
     it('should navigate to entities route with the corresponding entity type', () => {
       remoteData.payload.type = 'item';
@@ -121,7 +124,7 @@ describe('DsoRedirectService', () => {
       redir.subscribe();
       scheduler.schedule(() => redir);
       scheduler.flush();
-      expect(router.navigate).toHaveBeenCalledWith(['/entities/publication/' + remoteData.payload.uuid]);
+      expect(redirectService.redirect).toHaveBeenCalledWith('/entities/publication/' + remoteData.payload.uuid, 301);
     });
 
     it('should navigate to collections route', () => {
@@ -130,7 +133,7 @@ describe('DsoRedirectService', () => {
       redir.subscribe();
       scheduler.schedule(() => redir);
       scheduler.flush();
-      expect(router.navigate).toHaveBeenCalledWith(['/collections/' + remoteData.payload.uuid]);
+      expect(redirectService.redirect).toHaveBeenCalledWith('/collections/' + remoteData.payload.uuid, 301);
     });
 
     it('should navigate to communities route', () => {
@@ -139,7 +142,7 @@ describe('DsoRedirectService', () => {
       redir.subscribe();
       scheduler.schedule(() => redir);
       scheduler.flush();
-      expect(router.navigate).toHaveBeenCalledWith(['/communities/' + remoteData.payload.uuid]);
+      expect(redirectService.redirect).toHaveBeenCalledWith('/communities/' + remoteData.payload.uuid, 301);
     });
   });
 

src/app/core/data/dso-redirect.service.ts

@@ -7,7 +7,6 @@
  */
 /* eslint-disable max-classes-per-file */
 import { Injectable } from '@angular/core';
-import { Router } from '@angular/router';
 import { Observable } from 'rxjs';
 import { tap } from 'rxjs/operators';
 import { hasValue } from '../../shared/empty.util';
@@ -21,6 +20,7 @@ import { getFirstCompletedRemoteData } from '../shared/operators';
 import { DSpaceObject } from '../shared/dspace-object.model';
 import { IdentifiableDataService } from './base/identifiable-data.service';
 import { getDSORoute } from '../../app-routing-paths';
+import { HardRedirectService } from '../services/hard-redirect.service';
 
 const ID_ENDPOINT = 'pid';
 const UUID_ENDPOINT = 'dso';
@@ -74,13 +74,16 @@ export class DsoRedirectService {
     protected rdbService: RemoteDataBuildService,
     protected objectCache: ObjectCacheService,
     protected halService: HALEndpointService,
-    private router: Router,
+    private hardRedirectService: HardRedirectService
   ) {
     this.dataService = new DsoByIdOrUUIDDataService(requestService, rdbService, objectCache, halService);
   }
 
   /**
-   * Retrieve a DSpaceObject by
+   * Redirect to a DSpaceObject's path using the given identifier type and ID.
+   * This is used to redirect paths like "/handle/[prefix]/[suffix]" to the object's path (e.g. /items/[uuid]).
+   * See LookupGuard for more examples.
+   *
    * @param id              the identifier of the object to retrieve
    * @param identifierType  the type of the given identifier (defaults to UUID)
    */
@@ -94,7 +97,8 @@ export class DsoRedirectService {
           if (hasValue(dso.uuid)) {
             let newRoute = getDSORoute(dso);
             if (hasValue(newRoute)) {
-              this.router.navigate([newRoute]);
+              // Use a "301 Moved Permanently" redirect for SEO purposes
+              this.hardRedirectService.redirect(newRoute, 301);
             }
           }
         }

src/app/core/services/hard-redirect.service.ts

@@ -11,8 +11,10 @@ export abstract class HardRedirectService {
    *
    * @param url
    *    the page to redirect to
+   * @param statusCode
+   *    optional HTTP status code to use for redirect (default = 302, which is a temporary redirect)
    */
-  abstract redirect(url: string);
+  abstract redirect(url: string, statusCode?: number);
 
   /**
    * Get the current route, with query params included

src/app/core/services/server-hard-redirect.service.spec.ts

@@ -22,20 +22,33 @@ describe('ServerHardRedirectService', () => {
     expect(service).toBeTruthy();
   });
 
-  describe('when performing a redirect', () => {
-
+  describe('when performing a default redirect', () => {
     const redirect = 'test redirect';
 
     beforeEach(() => {
       service.redirect(redirect);
     });
 
-    it('should update the response object', () => {
+    it('should perform a 302 redirect', () => {
       expect(mockResponse.redirect).toHaveBeenCalledWith(302, redirect);
       expect(mockResponse.end).toHaveBeenCalled();
     });
   });
 
+  describe('when performing a 301 redirect', () => {
+    const redirect = 'test 301 redirect';
+    const redirectStatusCode = 301;
+
+    beforeEach(() => {
+      service.redirect(redirect, redirectStatusCode);
+    });
+
+    it('should redirect with passed in status code', () => {
+      expect(mockResponse.redirect).toHaveBeenCalledWith(redirectStatusCode, redirect);
+      expect(mockResponse.end).toHaveBeenCalled();
+    });
+  });
+
   describe('when requesting the current route', () => {
 
     beforeEach(() => {

src/app/core/services/server-hard-redirect.service.ts

@@ -17,10 +17,14 @@ export class ServerHardRedirectService extends HardRedirectService {
   }
 
   /**
-   * Perform a hard redirect to URL
+   * Perform a hard redirect to a given location.
+   *
    * @param url
+   *    the page to redirect to
+   * @param statusCode
+   *    optional HTTP status code to use for redirect (default = 302, which is a temporary redirect)
    */
-  redirect(url: string) {
+  redirect(url: string, statusCode?: number) {
 
     if (url === this.req.url) {
       return;
@@ -38,8 +42,8 @@ export class ServerHardRedirectService extends HardRedirectService {
         process.exit(1);
       }
     } else {
-      // attempt to use the already set status
-      let status = this.res.statusCode || 0;
+      // attempt to use passed in statusCode or the already set status (in request)
+      let status = statusCode || this.res.statusCode || 0;
       if (status < 300 || status >= 400) {
         // temporary redirect
         status = 302;