72699: JSDocs and Test cases

src/app/core/auth/auth.service.spec.ts

@@ -27,6 +27,7 @@ import { EPersonDataService } from '../eperson/eperson-data.service';
 import { createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils';
 import { authMethodsMock } from '../../shared/testing/auth-service.stub';
 import { AuthMethod } from './models/auth.method';
+import { HardRedirectService } from '../services/hard-redirect.service';
 
 describe('AuthService test', () => {
 
@@ -48,6 +49,7 @@ describe('AuthService test', () => {
   let authenticatedState;
   let unAuthenticatedState;
   let linkService;
+  let hardRedirectService;
 
   function init() {
     mockStore = jasmine.createSpyObj('store', {
@@ -77,6 +79,7 @@ describe('AuthService test', () => {
     linkService = {
       resolveLinks: {}
     };
+    hardRedirectService = jasmine.createSpyObj('hardRedirectService', ['redirect']);
     spyOn(linkService, 'resolveLinks').and.returnValue({ authenticated: true, eperson: observableOf({ payload: {} }) });
 
   }
@@ -104,6 +107,7 @@ describe('AuthService test', () => {
           { provide: ActivatedRoute, useValue: routeStub },
           { provide: Store, useValue: mockStore },
           { provide: EPersonDataService, useValue: mockEpersonDataService },
+          { provide: HardRedirectService, useValue: hardRedirectService },
           CookieService,
           AuthService
         ],
@@ -210,7 +214,7 @@ describe('AuthService test', () => {
           (state as any).core = Object.create({});
           (state as any).core.auth = authenticatedState;
         });
-      authService = new AuthService({}, window, undefined, authReqService, mockEpersonDataService, router, routeService, cookieService, store);
+      authService = new AuthService({}, window, undefined, authReqService, mockEpersonDataService, router, routeService, cookieService, store, hardRedirectService);
     }));
 
     it('should return true when user is logged in', () => {
@@ -289,7 +293,7 @@ describe('AuthService test', () => {
           (state as any).core = Object.create({});
           (state as any).core.auth = authenticatedState;
         });
-      authService = new AuthService({}, window, undefined, authReqService, mockEpersonDataService, router, routeService, cookieService, store);
+      authService = new AuthService({}, window, undefined, authReqService, mockEpersonDataService, router, routeService, cookieService, store, hardRedirectService);
       storage = (authService as any).storage;
       routeServiceMock = TestBed.get(RouteService);
       routerStub = TestBed.get(Router);
@@ -320,34 +324,34 @@ describe('AuthService test', () => {
 
     it('should set redirect url to previous page', () => {
       spyOn(routeServiceMock, 'getHistory').and.callThrough();
-      spyOn(routerStub, 'navigateByUrl');
       authService.redirectAfterLoginSuccess(true);
       expect(routeServiceMock.getHistory).toHaveBeenCalled();
-      expect(routerStub.navigateByUrl).toHaveBeenCalledWith('/collection/123');
+      // Reload with redirect URL set to /collection/123
+      expect(hardRedirectService.redirect).toHaveBeenCalledWith(jasmine.stringMatching(new RegExp('/reload/[0-9]*\\?redirect=' + encodeURIComponent('/collection/123'))));
     });
 
     it('should set redirect url to current page', () => {
       spyOn(routeServiceMock, 'getHistory').and.callThrough();
-      spyOn(routerStub, 'navigateByUrl');
       authService.redirectAfterLoginSuccess(false);
       expect(routeServiceMock.getHistory).toHaveBeenCalled();
-      expect(routerStub.navigateByUrl).toHaveBeenCalledWith('/home');
+      // Reload with redirect URL set to /home
+      expect(hardRedirectService.redirect).toHaveBeenCalledWith(jasmine.stringMatching(new RegExp('/reload/[0-9]*\\?redirect=' + encodeURIComponent('/home'))));
     });
 
     it('should redirect to / and not to /login', () => {
       spyOn(routeServiceMock, 'getHistory').and.returnValue(observableOf(['/login', '/login']));
-      spyOn(routerStub, 'navigateByUrl');
       authService.redirectAfterLoginSuccess(true);
       expect(routeServiceMock.getHistory).toHaveBeenCalled();
-      expect(routerStub.navigateByUrl).toHaveBeenCalledWith('/');
+      // Reload without a redirect URL
+      expect(hardRedirectService.redirect).toHaveBeenCalledWith(jasmine.stringMatching(new RegExp('/reload/[0-9]*(?!\\?)$')));
     });
 
     it('should redirect to / when no redirect url is found', () => {
       spyOn(routeServiceMock, 'getHistory').and.returnValue(observableOf(['']));
-      spyOn(routerStub, 'navigateByUrl');
       authService.redirectAfterLoginSuccess(true);
       expect(routeServiceMock.getHistory).toHaveBeenCalled();
-      expect(routerStub.navigateByUrl).toHaveBeenCalledWith('/');
+      // Reload without a redirect URL
+      expect(hardRedirectService.redirect).toHaveBeenCalledWith(jasmine.stringMatching(new RegExp('/reload/[0-9]*(?!\\?)$')));
     });
 
     describe('impersonate', () => {
@@ -464,6 +468,14 @@ describe('AuthService test', () => {
         });
       });
     });
+
+    describe('refreshAfterLogout', () => {
+      it('should call navigateToRedirectUrl with no url', () => {
+        spyOn(authService as any, 'navigateToRedirectUrl').and.stub();
+        authService.refreshAfterLogout();
+        expect((authService as any).navigateToRedirectUrl).toHaveBeenCalled();
+      });
+    });
   });
 
   describe('when user is not logged in', () => {
@@ -496,7 +508,7 @@ describe('AuthService test', () => {
           (state as any).core = Object.create({});
           (state as any).core.auth = unAuthenticatedState;
         });
-      authService = new AuthService({}, window, undefined, authReqService, mockEpersonDataService, router, routeService, cookieService, store);
+      authService = new AuthService({}, window, undefined, authReqService, mockEpersonDataService, router, routeService, cookieService, store, hardRedirectService);
     }));
 
     it('should return null for the shortlived token', () => {

src/app/core/reload/reload.guard.spec.ts

@@ -0,0 +1,47 @@
+import { ReloadGuard } from './reload.guard';
+import { Router } from '@angular/router';
+
+describe('ReloadGuard', () => {
+  let guard: ReloadGuard;
+  let router: Router;
+
+  beforeEach(() => {
+    router = jasmine.createSpyObj('router', ['parseUrl', 'createUrlTree']);
+    guard = new ReloadGuard(router);
+  });
+
+  describe('canActivate', () => {
+    let route;
+
+    describe('when the route\'s query params contain a redirect url', () => {
+      let redirectUrl;
+
+      beforeEach(() => {
+        redirectUrl = '/redirect/url?param=extra';
+        route = {
+          queryParams: {
+            redirect: redirectUrl
+          }
+        };
+      });
+
+      it('should create a UrlTree with the redirect URL', () => {
+        guard.canActivate(route, undefined);
+        expect(router.parseUrl).toHaveBeenCalledWith(redirectUrl);
+      });
+    });
+
+    describe('when the route\'s query params doesn\'t contain a redirect url', () => {
+      beforeEach(() => {
+        route = {
+          queryParams: {}
+        };
+      });
+
+      it('should create a UrlTree to home', () => {
+        guard.canActivate(route, undefined);
+        expect(router.createUrlTree).toHaveBeenCalledWith(['home']);
+      });
+    });
+  });
+});

src/app/core/reload/reload.guard.ts

@@ -2,11 +2,20 @@ import { ActivatedRouteSnapshot, CanActivate, Router, RouterStateSnapshot, UrlTr
 import { Injectable } from '@angular/core';
 import { isNotEmpty } from '../../shared/empty.util';
 
+/**
+ * A guard redirecting the user to the URL provided in the route's query params
+ * When no redirect url is found, the user is redirected to the homepage
+ */
 @Injectable()
 export class ReloadGuard implements CanActivate {
   constructor(private router: Router) {
   }
 
+  /**
+   * Get the UrlTree of the URL to redirect to
+   * @param route
+   * @param state
+   */
   canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): UrlTree {
     if (isNotEmpty(route.queryParams.redirect)) {
       return this.router.parseUrl(route.queryParams.redirect);

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

@@ -0,0 +1,40 @@
+import {async, TestBed} from '@angular/core/testing';
+import {BrowserHardRedirectService} from './browser-hard-redirect.service';
+
+describe('BrowserHardRedirectService', () => {
+
+  const mockLocation = {
+    href: undefined,
+    origin: 'test origin',
+  } as Location;
+
+  const service: BrowserHardRedirectService = new BrowserHardRedirectService(mockLocation);
+
+  beforeEach(() => {
+    TestBed.configureTestingModule({});
+  });
+
+  it('should be created', () => {
+    expect(service).toBeTruthy();
+  });
+
+  describe('when performing a redirect', () => {
+
+    const redirect = 'test redirect';
+
+    beforeEach(() => {
+      service.redirect(redirect);
+    });
+
+    it('should update the location', () => {
+      expect(mockLocation.href).toEqual(redirect);
+    })
+  });
+
+  describe('when requesting the origin', () => {
+
+    it('should return the location origin', () => {
+      expect(service.getOriginFromUrl()).toEqual('test origin');
+    });
+  });
+});

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

@@ -1,6 +1,9 @@
 import {Inject, Injectable} from '@angular/core';
 import {LocationToken} from '../../../modules/app/browser-app.module';
 
+/**
+ * Service for performing hard redirects within the browser app module
+ */
 @Injectable()
 export class BrowserHardRedirectService {
 
@@ -9,10 +12,17 @@ export class BrowserHardRedirectService {
   ) {
   }
 
+  /**
+   * Perform a hard redirect to URL
+   * @param url
+   */
   redirect(url: string) {
     this.location.href = url;
   }
 
+  /**
+   * Get the origin of a request
+   */
   getOriginFromUrl() {
     return this.location.origin;
   }

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

@@ -0,0 +1,48 @@
+import { TestBed } from '@angular/core/testing';
+import { ServerHardRedirectService } from './server-hard-redirect.service';
+
+describe('ServerHardRedirectService', () => {
+
+  const mockRequest = jasmine.createSpyObj(['get']);
+  const mockResponse = jasmine.createSpyObj(['redirect', 'end']);
+
+  const service: ServerHardRedirectService = new ServerHardRedirectService(mockRequest, mockResponse);
+
+  beforeEach(() => {
+    TestBed.configureTestingModule({});
+  });
+
+  it('should be created', () => {
+    expect(service).toBeTruthy();
+  });
+
+  describe('when performing a redirect', () => {
+
+    const redirect = 'test redirect';
+
+    beforeEach(() => {
+      service.redirect(redirect);
+    });
+
+    it('should update the response object', () => {
+      expect(mockResponse.redirect).toHaveBeenCalledWith(302, redirect);
+      expect(mockResponse.end).toHaveBeenCalled();
+    })
+  });
+
+  describe('when requesting the origin', () => {
+
+    beforeEach(() => {
+      mockRequest.protocol = 'test-protocol';
+      mockRequest.get.and.callFake((name) => {
+        if (name === 'hostname') {
+          return 'test-host';
+        }
+      });
+    });
+
+    it('should return the location origin', () => {
+      expect(service.getOriginFromUrl()).toEqual('test-protocol://test-host');
+    });
+  });
+});

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

@@ -2,6 +2,9 @@ import { Inject, Injectable } from '@angular/core';
 import { Request, Response } from 'express';
 import { REQUEST, RESPONSE } from '@nguniversal/express-engine/tokens';
 
+/**
+ * Service for performing hard redirects within the server app module
+ */
 @Injectable()
 export class ServerHardRedirectService {
 
@@ -11,6 +14,10 @@ export class ServerHardRedirectService {
   ) {
   }
 
+  /**
+   * Perform a hard redirect to URL
+   * @param url
+   */
   redirect(url: string) {
 
     if (url === this.req.url) {
@@ -45,6 +52,9 @@ export class ServerHardRedirectService {
     }
   }
 
+  /**
+   * Get the origin of a request
+   */
   getOriginFromUrl() {
 
     return new URL(`${this.req.protocol}://${this.req.get('hostname')}`).toString();