Merged in task/dspace-cris-2023_02_x/DSC-2176 (pull request DSpace#2801)

[DSC-2176] prevent redirect issue while fetching signposting links

Approved-by: Giuseppe Digilio

Author: FrancescoMolinaro

src/app/core/services/server-response.service.ts

@@ -73,7 +73,7 @@ export class ServerResponseService {
    * @param content
    */
   setHeader(header: string, content: string) {
-    if (this.response) {
+    if (this.response && !this.response.writableEnded) {
       this.response.setHeader(header, content);
     }
   }

src/app/item-page/full/full-item-page.component.spec.ts

@@ -153,6 +153,7 @@ describe('FullItemPageComponent', () => {
   beforeEach(waitForAsync(() => {
     routeData = {
       dso: createSuccessfulRemoteDataObject(mockItem),
+      links: [mocklink, mocklink2]
     };
 
     routeStub = Object.assign(new ActivatedRouteStub(), {

src/app/item-page/full/full-item-page.component.ts

@@ -16,7 +16,6 @@ import { hasValue } from '../../shared/empty.util';
 import { Location } from '@angular/common';
 import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service';
 import { ServerResponseService } from '../../core/services/server-response.service';
-import { SignpostingDataService } from '../../core/data/signposting-data.service';
 import { LinkHeadService } from '../../core/services/link-head.service';
 import { APP_CONFIG, AppConfig } from '../../../config/app-config.interface';
 
@@ -56,12 +55,11 @@ export class FullItemPageComponent extends ItemPageComponent implements OnInit,
     protected authorizationService: AuthorizationDataService,
     protected _location: Location,
     protected responseService: ServerResponseService,
-    protected signpostingDataService: SignpostingDataService,
     protected linkHeadService: LinkHeadService,
     @Inject(PLATFORM_ID) protected platformId: string,
     @Inject(APP_CONFIG) private appConfig: AppConfig,
   ) {
-    super(route, router, items, authorizationService, responseService, signpostingDataService, linkHeadService, platformId);
+    super(route, router, items, authorizationService, responseService, linkHeadService, platformId);
   }
 
   /*** AoT inheritance fix, will hopefully be resolved in the near future **/
@@ -89,6 +87,7 @@ export class FullItemPageComponent extends ItemPageComponent implements OnInit,
 
   ngOnDestroy() {
     this.subs.filter((sub) => hasValue(sub)).forEach((sub) => sub.unsubscribe());
+    super.ngOnDestroy();
   }
 
   protected increaseLimit(metadataKey: string) {

src/app/item-page/item-page-routing.module.ts

@@ -19,6 +19,8 @@ import { REQUEST_COPY_MODULE_PATH } from '../app-routing-paths';
 import { CrisItemPageTabResolver } from './cris-item-page-tab.resolver';
 import { OrcidPageComponent } from './orcid-page/orcid-page.component';
 import { OrcidPageGuard } from './orcid-page/orcid-page.guard';
+import { signpostingLinksResolver } from './simple/link-resolver/signposting-links.resolver';
+
 
 @NgModule({
   imports: [
@@ -28,6 +30,7 @@ import { OrcidPageGuard } from './orcid-page/orcid-page.guard';
         resolve: {
           dso: ItemPageResolver,
           breadcrumb: ItemBreadcrumbResolver,
+          links: signpostingLinksResolver,
         },
         runGuardsAndResolvers: 'always',
         children: [

src/app/item-page/simple/item-page.component.spec.ts

@@ -68,7 +68,7 @@ describe('ItemPageComponent', () => {
     /* eslint-enable no-empty, @typescript-eslint/no-empty-function */
   };
   const mockRoute = Object.assign(new ActivatedRouteStub(), {
-    data: observableOf({ dso: createSuccessfulRemoteDataObject(mockItem) })
+    data: observableOf({ dso: createSuccessfulRemoteDataObject(mockItem) , links: [mocklink, mocklink2]  })
   });
 
   beforeEach(waitForAsync(() => {

src/app/item-page/simple/item-page.component.ts

@@ -3,7 +3,7 @@ import { ActivatedRoute, Router } from '@angular/router';
 import { isPlatformServer } from '@angular/common';
 
 import { Observable } from 'rxjs';
-import { map, take } from 'rxjs/operators';
+import { map } from 'rxjs/operators';
 
 import { ItemDataService } from '../../core/data/item-data.service';
 import { RemoteData } from '../../core/data/remote-data';
@@ -15,7 +15,6 @@ import { getItemPageRoute } from '../item-page-routing-paths';
 import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service';
 import { FeatureID } from '../../core/data/feature-authorization/feature-id';
 import { ServerResponseService } from '../../core/services/server-response.service';
-import { SignpostingDataService } from '../../core/data/signposting-data.service';
 import { SignpostingLink } from '../../core/data/signposting-links.model';
 import { isNotEmpty } from '../../shared/empty.util';
 import { LinkDefinition, LinkHeadService } from '../../core/services/link-head.service';
@@ -79,7 +78,6 @@ export class ItemPageComponent implements OnInit, OnDestroy {
     protected items: ItemDataService,
     protected authorizationService: AuthorizationDataService,
     protected responseService: ServerResponseService,
-    protected signpostingDataService: SignpostingDataService,
     protected linkHeadService: LinkHeadService,
     @Inject(PLATFORM_ID) protected platformId: string
   ) {
@@ -111,29 +109,25 @@ export class ItemPageComponent implements OnInit, OnDestroy {
    * @private
    */
   private initPageLinks(): void {
-    this.route.params.subscribe(params => {
-      this.signpostingDataService.getLinks(params.id).pipe(take(1)).subscribe((signpostingLinks: SignpostingLink[]) => {
-        let links = '';
-        this.signpostingLinks = signpostingLinks;
-
-        signpostingLinks.forEach((link: SignpostingLink) => {
-          links = links + (isNotEmpty(links) ? ', ' : '') + `<${link.href}> ; rel="${link.rel}"` + (isNotEmpty(link.type) ? ` ; type="${link.type}" ` : ' ');
-          let tag: LinkDefinition = {
-            href: link.href,
-            rel: link.rel
-          };
-          if (isNotEmpty(link.type)) {
-            tag = Object.assign(tag, {
-              type: link.type
-            });
-          }
-          this.linkHeadService.addTag(tag);
-        });
-
-        if (isPlatformServer(this.platformId)) {
-          this.responseService.setHeader('Link', links);
+    this.route.data.subscribe(data => {
+      this.signpostingLinks = data.links ?? [];
+      let links = '';
+      this.signpostingLinks.forEach((link: SignpostingLink) => {
+        links = links + (isNotEmpty(links) ? ', ' : '') + `<${link.href}> ; rel="${link.rel}"` + (isNotEmpty(link.type) ? ` ; type="${link.type}" ` : ' ');
+        let tag: LinkDefinition = {
+          href: link.href,
+          rel: link.rel
+        };
+        if (isNotEmpty(link.type)) {
+          tag = Object.assign(tag, {
+            type: link.type
+          });
         }
+        this.linkHeadService.addTag(tag);
       });
+      if (isPlatformServer(this.platformId)) {
+        this.responseService.setHeader('Link', links);
+      }
     });
   }
 

src/app/item-page/simple/link-resolver/signposting-links.resolver.spec.ts

@@ -0,0 +1,61 @@
+import { ActivatedRouteSnapshot, RouterStateSnapshot } from '@angular/router';
+import { SignpostingDataService } from '../../../core/data/signposting-data.service';
+import { of } from 'rxjs';
+import { TestBed } from '@angular/core/testing';
+import { signpostingLinksResolver } from './signposting-links.resolver';
+describe('signpostingLinksResolver', () => {
+  let resolver: any;
+  let route: ActivatedRouteSnapshot;
+  let state = {};
+  let signpostingDataService: SignpostingDataService;
+  const testUuid = '1234567890';
+  const mocklink = {
+    href: 'http://test.org',
+    rel: 'rel1',
+    type: 'type1'
+  };
+  const mocklink2 = {
+    href: 'http://test2.org',
+    rel: 'rel2',
+    type: undefined
+  };
+  const resolvedLinks = `<${mocklink.href}> ; rel="${mocklink.rel}" ; type="${mocklink.type}" , <${mocklink2.href}> ; rel="${mocklink2.rel}" `;
+  const mockTag2 = {
+    href: 'http://test2.org',
+    rel: 'rel2',
+  };
+  function init() {
+    route = Object.assign(new ActivatedRouteSnapshot(), {
+      params: {
+        id: testUuid,
+      },
+    });
+    signpostingDataService = jasmine.createSpyObj('signpostingDataService', {
+      getLinks: of([mocklink, mocklink2]),
+      setLinks: () => null,
+    });
+    resolver = signpostingLinksResolver;
+  }
+  function initTestbed() {
+    TestBed.configureTestingModule({
+      providers: [
+        {provide: RouterStateSnapshot, useValue: state},
+        {provide: ActivatedRouteSnapshot, useValue: route},
+        {provide: SignpostingDataService, useValue: signpostingDataService},
+      ]
+    });
+  }
+  describe('when an item page is loaded', () => {
+    beforeEach(() => {
+      init();
+      initTestbed();
+    });
+    it('should retrieve links and set header and head tags', () => {
+      TestBed.runInInjectionContext(() => {
+        resolver(route, state).subscribe(() => {
+          expect(signpostingDataService.getLinks).toHaveBeenCalledWith(testUuid);
+        });
+      });
+    });
+  });
+});

src/app/item-page/simple/link-resolver/signposting-links.resolver.ts

@@ -0,0 +1,25 @@
+import { ActivatedRouteSnapshot, ResolveFn, RouterStateSnapshot } from '@angular/router';
+import { SignpostingLink } from '../../../core/data/signposting-links.model';
+import { Observable, of } from 'rxjs';
+import { inject } from '@angular/core';
+import { hasValue } from '../../../shared/empty.util';
+import { SignpostingDataService } from '../../../core/data/signposting-data.service';
+
+/**
+ * Resolver to retrieve signposting links before an eventual redirect of any route guard
+ *
+ * @param route
+ * @param state
+ * @param signpostingDataService
+ */
+export const signpostingLinksResolver: ResolveFn<Observable<SignpostingLink[]>> = (
+  route: ActivatedRouteSnapshot,
+  state: RouterStateSnapshot,
+  signpostingDataService: SignpostingDataService = inject(SignpostingDataService),
+): Observable<SignpostingLink[]> => {
+  const uuid = route.params.id;
+  if (!hasValue(uuid)) {
+    return of([]);
+  }
+  return signpostingDataService.getLinks(uuid);
+};