From 1b10ec63d6b4bab038fd3029569c257f25fbc38e Mon Sep 17 00:00:00 2001 From: lotte Date: Wed, 16 Sep 2020 17:55:59 +0200 Subject: [PATCH] fixed google scholar links, todo: fix tests --- scripts/set-env.ts | 4 +- .../core/metadata/metadata.service.spec.ts | 2 + src/app/core/metadata/metadata.service.ts | 7 ++- .../browser-hard-redirect.service.spec.ts | 11 +++- .../services/browser-hard-redirect.service.ts | 10 +++- .../services/hard-redirect.service.spec.ts | 53 +++++++++++++++++++ .../core/services/hard-redirect.service.ts | 18 +++++++ .../server-hard-redirect.service.spec.ts | 13 +++++ .../services/server-hard-redirect.service.ts | 10 +++- .../file-download-link.component.spec.ts | 8 +-- .../file-download-link.component.ts | 10 +++- src/config/global-config.interface.ts | 1 + src/environments/environment.common.ts | 1 + 13 files changed, 136 insertions(+), 12 deletions(-) create mode 100644 src/app/core/services/hard-redirect.service.spec.ts diff --git a/scripts/set-env.ts b/scripts/set-env.ts index 5570b77218..0aa106538c 100644 --- a/scripts/set-env.ts +++ b/scripts/set-env.ts @@ -57,8 +57,8 @@ function generateEnvironmentFile(file: GlobalConfig): void { // TODO remove workaround in beta 5 if (file.rest.nameSpace.match("(.*)/api/?$") !== null) { - const newValue = getNameSpace(file.rest.nameSpace); - console.log(colors.white.bgMagenta.bold(`The rest.nameSpace property in your environment file or in your DSPACE_REST_NAMESPACE environment variable ends with '/api'.\nThis is deprecated. As '/api' isn't configurable on the rest side, it shouldn't be repeated in every environment file.\nPlease change the rest nameSpace to '${newValue}'`)); + file.rest.nameSpace = getNameSpace(file.rest.nameSpace); + console.log(colors.white.bgMagenta.bold(`The rest.nameSpace property in your environment file or in your DSPACE_REST_NAMESPACE environment variable ends with '/api'.\nThis is deprecated. As '/api' isn't configurable on the rest side, it shouldn't be repeated in every environment file.\nPlease change the rest nameSpace to '${file.rest.nameSpace}'`)); } const contents = `export const environment = ` + JSON.stringify(file); diff --git a/src/app/core/metadata/metadata.service.spec.ts b/src/app/core/metadata/metadata.service.spec.ts index 74c4522bbd..c560f62d67 100644 --- a/src/app/core/metadata/metadata.service.spec.ts +++ b/src/app/core/metadata/metadata.service.spec.ts @@ -52,6 +52,7 @@ 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'; /* tslint:disable:max-classes-per-file */ @Component({ @@ -170,6 +171,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 ], diff --git a/src/app/core/metadata/metadata.service.ts b/src/app/core/metadata/metadata.service.ts index 02d2b0c86b..d2b755b7b5 100644 --- a/src/app/core/metadata/metadata.service.ts +++ b/src/app/core/metadata/metadata.service.ts @@ -21,6 +21,7 @@ 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'; @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 = [this.redirectService.getRequestOrigin(), this.router.url].join(''); 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); } }); } diff --git a/src/app/core/services/browser-hard-redirect.service.spec.ts b/src/app/core/services/browser-hard-redirect.service.spec.ts index 9d4c5df9a2..64518579aa 100644 --- a/src/app/core/services/browser-hard-redirect.service.spec.ts +++ b/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); + }); + }); + }); diff --git a/src/app/core/services/browser-hard-redirect.service.ts b/src/app/core/services/browser-hard-redirect.service.ts index 0d14b6b834..4b7424bee2 100644 --- a/src/app/core/services/browser-hard-redirect.service.ts +++ b/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; + } } diff --git a/src/app/core/services/hard-redirect.service.spec.ts b/src/app/core/services/hard-redirect.service.spec.ts new file mode 100644 index 0000000000..f90e666023 --- /dev/null +++ b/src/app/core/services/hard-redirect.service.spec.ts @@ -0,0 +1,53 @@ +import { HardRedirectService } from './hard-redirect.service'; +import { environment } from '../../../environments/environment'; +import { TestBed } from '@angular/core/testing'; + +const requestOrigin = 'http://dspace-angular-ui.dspace.com'; + +fdescribe('HardRedirectService', () => { + const service: TestHardRedirectService = new TestHardRedirectService(); + + beforeEach(() => { + TestBed.configureTestingModule({}); + }); + + 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 + relativePath); + }); + + afterEach(() => { + environment.rewriteDownloadUrls = originalValue; + }) + }); + +}); + +class TestHardRedirectService extends HardRedirectService { + constructor() { + + } + redirect(url: string) { + } + + getCurrentRoute() { + return undefined; + } + + getRequestOrigin() { + return requestOrigin; + } +} diff --git a/src/app/core/services/hard-redirect.service.ts b/src/app/core/services/hard-redirect.service.ts index 09757a1250..4602b9fb5f 100644 --- a/src/app/core/services/hard-redirect.service.ts +++ b/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) { + let hostName = this.getRequestOrigin(); + let namespace = environment.rest.nameSpace; + let rewrittenUrl = new URLCombiner(hostName, namespace).toString(); + return originalUrl.replace(environment.rest.baseUrl, rewrittenUrl); + } else { + return originalUrl; + } + } } diff --git a/src/app/core/services/server-hard-redirect.service.spec.ts b/src/app/core/services/server-hard-redirect.service.spec.ts index 2d09c21eb9..8bda7a5f56 100644 --- a/src/app/core/services/server-hard-redirect.service.spec.ts +++ b/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 = { + origin: '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); + }); + }); + }); diff --git a/src/app/core/services/server-hard-redirect.service.ts b/src/app/core/services/server-hard-redirect.service.ts index 79755d2dc9..d3b678cc71 100644 --- a/src/app/core/services/server-hard-redirect.service.ts +++ b/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.origin; + } } diff --git a/src/app/shared/file-download-link/file-download-link.component.spec.ts b/src/app/shared/file-download-link/file-download-link.component.spec.ts index ac1751d43d..56702787d3 100644 --- a/src/app/shared/file-download-link/file-download-link.component.spec.ts +++ b/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(() => { diff --git a/src/app/shared/file-download-link/file-download-link.component.ts b/src/app/shared/file-download-link/file-download-link.component.ts index 9df7c191ff..96732aef87 100644 --- a/src/app/shared/file-download-link/file-download-link.component.ts +++ b/src/app/shared/file-download-link/file-download-link.component.ts @@ -2,6 +2,10 @@ 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 { environment } from '../../../environments/environment'; +import { hasValue } from '../empty.util'; +import { URLCombiner } from '../../core/url-combiner/url-combiner'; +import { HardRedirectService } from '../../core/services/hard-redirect.service'; @Component({ selector: 'ds-file-download-link', @@ -30,10 +34,13 @@ export class FileDownloadLinkComponent implements OnInit { isAuthenticated$: Observable; 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 +51,4 @@ export class FileDownloadLinkComponent implements OnInit { this.fileService.downloadFile(this.href); return false; } - } diff --git a/src/config/global-config.interface.ts b/src/config/global-config.interface.ts index acef3404eb..07ee4ca444 100644 --- a/src/config/global-config.interface.ts +++ b/src/config/global-config.interface.ts @@ -31,4 +31,5 @@ export interface GlobalConfig extends Config { item: ItemPageConfig; collection: CollectionPageConfig; theme: Theme; + rewriteDownloadUrls: boolean; } diff --git a/src/environments/environment.common.ts b/src/environments/environment.common.ts index 32ae2f54b0..75c2210d33 100644 --- a/src/environments/environment.common.ts +++ b/src/environments/environment.common.ts @@ -216,4 +216,5 @@ export const environment: GlobalConfig = { theme: { name: 'default', }, + rewriteDownloadUrls: false, };