Merge pull request #868 from atmire/google-scholar-fixes

Google scholar fixes

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

@@ -52,10 +52,13 @@ import { UUIDService } from '../shared/uuid.service';
 import { MetadataService } from './metadata.service';
 import { environment } from '../../../environments/environment';
 import { storeModuleConfig } from '../../app.reducer';
+import { HardRedirectService } from '../services/hard-redirect.service';
+import { URLCombiner } from '../url-combiner/url-combiner';
 
 /* tslint:disable:max-classes-per-file */
 @Component({
-  template: `<router-outlet></router-outlet>`
+  template: `
+      <router-outlet></router-outlet>`
 })
 class TestComponent {
   constructor(private metadata: MetadataService) {
@@ -170,6 +173,7 @@ describe('MetadataService', () => {
         Title,
         // tslint:disable-next-line:no-empty
         { provide: ItemDataService, useValue: { findById: () => {} } },
+        { provide: HardRedirectService, useValue: { rewriteDownloadURL: (a) => a, getRequestOrigin: () => environment.ui.baseUrl }},
         BrowseService,
         MetadataService
       ],
@@ -208,7 +212,7 @@ describe('MetadataService', () => {
     tick();
     expect(tagStore.get('citation_dissertation_name')[0].content).toEqual('Test PowerPoint Document');
     expect(tagStore.get('citation_dissertation_institution')[0].content).toEqual('Mock Publisher');
-    expect(tagStore.get('citation_abstract_html_url')[0].content).toEqual([environment.ui.baseUrl, router.url].join(''));
+    expect(tagStore.get('citation_abstract_html_url')[0].content).toEqual(new URLCombiner(environment.ui.baseUrl, router.url).toString());
     expect(tagStore.get('citation_pdf_url')[0].content).toEqual('https://dspace7.4science.it/dspace-spring-rest/api/core/bitstreams/99b00f3c-1cc6-4689-8158-91965bee6b28/content');
   }));
 

src/app/core/metadata/metadata.service.ts

@@ -1,4 +1,4 @@
-import { Inject, Injectable } from '@angular/core';
+import { Injectable } from '@angular/core';
 
 import { Meta, MetaDefinition, Title } from '@angular/platform-browser';
 import { ActivatedRoute, NavigationEnd, Router } from '@angular/router';
@@ -20,7 +20,8 @@ import { Bitstream } from '../shared/bitstream.model';
 import { DSpaceObject } from '../shared/dspace-object.model';
 import { Item } from '../shared/item.model';
 import { getFirstSucceededRemoteDataPayload, getFirstSucceededRemoteListPayload } from '../shared/operators';
-import { environment } from '../../../environments/environment';
+import { HardRedirectService } from '../services/hard-redirect.service';
+import { URLCombiner } from '../url-combiner/url-combiner';
 
 @Injectable()
 export class MetadataService {
@@ -39,6 +40,7 @@ export class MetadataService {
     private dsoNameService: DSONameService,
     private bitstreamDataService: BitstreamDataService,
     private bitstreamFormatDataService: BitstreamFormatDataService,
+    private redirectService: HardRedirectService
   ) {
     // TODO: determine what open graph meta tags are needed and whether
     // the differ per route. potentially add image based on DSpaceObject
@@ -254,7 +256,7 @@ export class MetadataService {
    */
   private setCitationAbstractUrlTag(): void {
     if (this.currentObject.value instanceof Item) {
-      const value = [environment.ui.baseUrl, this.router.url].join('');
+      const value = new URLCombiner(this.redirectService.getRequestOrigin(), this.router.url).toString();
       this.addMetaTag('citation_abstract_html_url', value);
     }
   }
@@ -279,7 +281,8 @@ export class MetadataService {
               getFirstSucceededRemoteDataPayload()
             ).subscribe((format: BitstreamFormat) => {
               if (format.mimetype === 'application/pdf') {
-                this.addMetaTag('citation_pdf_url', bitstream._links.content.href);
+                const rewrittenURL= this.redirectService.rewriteDownloadURL(bitstream._links.content.href);
+                this.addMetaTag('citation_pdf_url', rewrittenURL);
               }
             });
           }

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

@@ -2,11 +2,12 @@ import {TestBed} from '@angular/core/testing';
 import {BrowserHardRedirectService} from './browser-hard-redirect.service';
 
 describe('BrowserHardRedirectService', () => {
-
+  const origin = 'test origin';
   const mockLocation = {
     href: undefined,
     pathname: '/pathname',
     search: '/search',
+    origin
   } as Location;
 
   const service: BrowserHardRedirectService = new BrowserHardRedirectService(mockLocation);
@@ -38,4 +39,12 @@ describe('BrowserHardRedirectService', () => {
       expect(service.getCurrentRoute()).toEqual(mockLocation.pathname + mockLocation.search);
     });
   });
+
+  describe('when requesting the origin', () => {
+
+    it('should return the location origin', () => {
+      expect(service.getRequestOrigin()).toEqual(origin);
+    });
+  });
+
 });

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

@@ -11,11 +11,12 @@ export function locationProvider(): Location {
  * Service for performing hard redirects within the browser app module
  */
 @Injectable()
-export class BrowserHardRedirectService implements HardRedirectService {
+export class BrowserHardRedirectService extends HardRedirectService {
 
   constructor(
     @Inject(LocationToken) protected location: Location,
   ) {
+    super();
   }
 
   /**
@@ -32,4 +33,11 @@ export class BrowserHardRedirectService implements HardRedirectService {
   getCurrentRoute() {
     return this.location.pathname + this.location.search;
   }
+
+  /**
+   * Get the hostname of the request
+   */
+  getRequestOrigin() {
+    return this.location.origin;
+  }
 }

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

@@ -0,0 +1,57 @@
+import { HardRedirectService } from './hard-redirect.service';
+import { environment } from '../../../environments/environment';
+import { TestBed } from '@angular/core/testing';
+import { Injectable } from '@angular/core';
+
+const requestOrigin = 'http://dspace-angular-ui.dspace.com';
+
+describe('HardRedirectService', () => {
+  let service: TestHardRedirectService;
+
+  beforeEach(() => {
+    TestBed.configureTestingModule({ providers: [TestHardRedirectService] });
+    service = TestBed.get(TestHardRedirectService);
+  });
+
+  describe('when calling rewriteDownloadURL', () => {
+    let originalValue;
+    const relativePath = '/test/url/path';
+    const testURL = environment.rest.baseUrl + relativePath;
+    beforeEach(() => {
+      originalValue = environment.rewriteDownloadUrls;
+    });
+
+    it('it should return the same url when rewriteDownloadURL is false', () => {
+      environment.rewriteDownloadUrls = false;
+      expect(service.rewriteDownloadURL(testURL)).toEqual(testURL);
+    });
+
+    it('it should replace part of the url when rewriteDownloadURL is true', () => {
+      environment.rewriteDownloadUrls = true;
+      expect(service.rewriteDownloadURL(testURL)).toEqual(requestOrigin + environment.rest.nameSpace + relativePath);
+    });
+
+    afterEach(() => {
+      environment.rewriteDownloadUrls = originalValue;
+    })
+  });
+});
+
+@Injectable()
+class TestHardRedirectService extends HardRedirectService {
+  constructor() {
+    super();
+  }
+
+  redirect(url: string) {
+    return undefined;
+  }
+
+  getCurrentRoute() {
+    return undefined;
+  }
+
+  getRequestOrigin() {
+    return requestOrigin;
+  }
+}

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

@@ -1,4 +1,6 @@
 import { Injectable } from '@angular/core';
+import { environment } from '../../../environments/environment';
+import { URLCombiner } from '../url-combiner/url-combiner';
 
 /**
  * Service to take care of hard redirects
@@ -19,4 +21,20 @@ export abstract class HardRedirectService {
    * e.g. /search?page=1&query=open%20access&f.dateIssued.min=1980&f.dateIssued.max=2020
    */
   abstract getCurrentRoute();
+
+  /**
+   * Get the hostname of the request
+   */
+  abstract getRequestOrigin();
+
+  public rewriteDownloadURL(originalUrl: string): string {
+    if (environment.rewriteDownloadUrls) {
+      const hostName = this.getRequestOrigin();
+      const namespace = environment.rest.nameSpace;
+      const rewrittenUrl = new URLCombiner(hostName, namespace).toString();
+      return originalUrl.replace(environment.rest.baseUrl, rewrittenUrl);
+    } else {
+      return originalUrl;
+    }
+  }
 }

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

@@ -7,8 +7,13 @@ describe('ServerHardRedirectService', () => {
   const mockResponse = jasmine.createSpyObj(['redirect', 'end']);
 
   const service: ServerHardRedirectService = new ServerHardRedirectService(mockRequest, mockResponse);
+  const origin = 'test-host';
 
   beforeEach(() => {
+    mockRequest.headers = {
+      host: 'test-host',
+    };
+
     TestBed.configureTestingModule({});
   });
 
@@ -40,4 +45,12 @@ describe('ServerHardRedirectService', () => {
       expect(service.getCurrentRoute()).toEqual(mockRequest.originalUrl);
     });
   });
+
+  describe('when requesting the origin', () => {
+
+    it('should return the location origin', () => {
+      expect(service.getRequestOrigin()).toEqual(origin);
+    });
+  });
+
 });

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

@@ -7,12 +7,13 @@ import { HardRedirectService } from './hard-redirect.service';
  * Service for performing hard redirects within the server app module
  */
 @Injectable()
-export class ServerHardRedirectService implements HardRedirectService {
+export class ServerHardRedirectService extends HardRedirectService {
 
   constructor(
     @Inject(REQUEST) protected req: Request,
     @Inject(RESPONSE) protected res: Response,
   ) {
+    super();
   }
 
   /**
@@ -59,4 +60,11 @@ export class ServerHardRedirectService implements HardRedirectService {
   getCurrentRoute() {
     return this.req.originalUrl;
   }
+
+  /**
+   * Get the hostname of the request
+   */
+  getRequestOrigin() {
+    return this.req.headers.host;
+  }
 }

src/app/shared/file-download-link/file-download-link.component.spec.ts

@@ -3,6 +3,7 @@ import { FileDownloadLinkComponent } from './file-download-link.component';
 import { AuthService } from '../../core/auth/auth.service';
 import { FileService } from '../../core/shared/file.service';
 import { of as observableOf } from 'rxjs';
+import { HardRedirectService } from '../../core/services/hard-redirect.service';
 
 describe('FileDownloadLinkComponent', () => {
   let component: FileDownloadLinkComponent;
@@ -23,13 +24,14 @@ describe('FileDownloadLinkComponent', () => {
   beforeEach(async(() => {
     init();
     TestBed.configureTestingModule({
-      declarations: [ FileDownloadLinkComponent ],
+      declarations: [FileDownloadLinkComponent],
       providers: [
         { provide: AuthService, useValue: authService },
-        { provide: FileService, useValue: fileService }
+        { provide: FileService, useValue: fileService },
+        { provide: HardRedirectService, useValue: { rewriteDownloadURL: (a) => a } },
       ]
     })
-    .compileComponents();
+      .compileComponents();
   }));
 
   beforeEach(() => {

src/app/shared/file-download-link/file-download-link.component.ts

@@ -2,6 +2,7 @@ import { Component, Input, OnInit } from '@angular/core';
 import { FileService } from '../../core/shared/file.service';
 import { Observable } from 'rxjs/internal/Observable';
 import { AuthService } from '../../core/auth/auth.service';
+import { HardRedirectService } from '../../core/services/hard-redirect.service';
 
 @Component({
   selector: 'ds-file-download-link',
@@ -30,10 +31,13 @@ export class FileDownloadLinkComponent implements OnInit {
   isAuthenticated$: Observable<boolean>;
 
   constructor(private fileService: FileService,
-              private authService: AuthService) { }
+              private authService: AuthService,
+              private redirectService: HardRedirectService) {
+  }
 
   ngOnInit() {
     this.isAuthenticated$ = this.authService.isAuthenticated();
+    this.href = this.redirectService.rewriteDownloadURL(this.href);
   }
 
   /**
@@ -44,5 +48,4 @@ export class FileDownloadLinkComponent implements OnInit {
     this.fileService.downloadFile(this.href);
     return false;
   }
-
 }

src/config/global-config.interface.ts

@@ -31,4 +31,5 @@ export interface GlobalConfig extends Config {
   item: ItemPageConfig;
   collection: CollectionPageConfig;
   theme: Theme;
+  rewriteDownloadUrls: boolean;
 }

src/environments/environment.common.ts

@@ -216,4 +216,6 @@ export const environment: GlobalConfig = {
   theme: {
     name: 'default',
   },
+  // Whether the UI should rewrite file download URLs to match its domain. Only necessary to enable when running UI and REST API on separate domains
+  rewriteDownloadUrls: false,
 };