diff --git a/config/config.example.yml b/config/config.example.yml index 3b46b8403f6..fbd9543c2d9 100644 --- a/config/config.example.yml +++ b/config/config.example.yml @@ -19,11 +19,32 @@ ui: # Angular Server Side Rendering (SSR) settings ssr: + # A boolean flag indicating whether the SSR configuration is enabled + # Defaults to true. + enabled: boolean; + + # Enable request performance profiling data collection and printing the results in the server console. + # Defaults to false. + enablePerformanceProfiler: boolean; + # Whether to tell Angular to inline "critical" styles into the server-side rendered HTML. # Determining which styles are critical is a relatively expensive operation; this option is # disabled (false) by default to boost server performance at the expense of loading smoothness. inlineCriticalCss: false + # Enable state transfer from the server-side application to the client-side application. + # Defaults to true. + # Note: When using an external application cache layer, it's recommended not to transfer the state to avoid caching it. + # Disabling it ensures that dynamic state information is not inadvertently cached, which can improve security and + # ensure that users always use the most up-to-date state. + transferState: boolean; + + # When a different REST base URL is used for the server-side application, the generated state contains references to + # REST resources with the internal URL configured, so it is not transferred to the client application, by default. + # Enabling this setting transfers the state to the client application and replaces internal URLs with the public + # URLs used by the client application. + replaceRestUrl: boolean; + # The REST API server settings # NOTE: these settings define which (publicly available) REST API to use. They are usually # 'synced' with the 'dspace.server.url' setting in your backend's local.cfg. @@ -33,6 +54,9 @@ rest: port: 443 # NOTE: Space is capitalized because 'namespace' is a reserved string in TypeScript nameSpace: /server + # Provide a different REST url to be used during SSR execution. It must contain the whole url including protocol, server port and + # server namespace (uncomment to use it). + #ssrBaseUrl: http://localhost:8080/server # Caching settings cache: diff --git a/server.ts b/server.ts index 07a05656d87..b4ddf2a99da 100644 --- a/server.ts +++ b/server.ts @@ -81,6 +81,9 @@ let anonymousCache: LRU; // extend environment with app config for server extendEnvironmentWithAppConfig(environment, appConfig); +// The REST server base URL +const REST_BASE_URL = environment.rest.ssrBaseUrl || environment.rest.baseUrl; + // The Express app is exported so that it can be used by serverless Functions. export function app() { @@ -156,7 +159,7 @@ export function app() { * Proxy the sitemaps */ router.use('/sitemap**', createProxyMiddleware({ - target: `${environment.rest.baseUrl}/sitemaps`, + target: `${REST_BASE_URL}/sitemaps`, pathRewrite: path => path.replace(environment.ui.nameSpace, '/'), changeOrigin: true, })); @@ -165,7 +168,7 @@ export function app() { * Proxy the linksets */ router.use('/signposting**', createProxyMiddleware({ - target: `${environment.rest.baseUrl}`, + target: `${REST_BASE_URL}`, pathRewrite: path => path.replace(environment.ui.nameSpace, '/'), changeOrigin: true, })); @@ -266,6 +269,11 @@ function serverSideRender(req, res, next, sendToUser: boolean = true) { }) .then((html) => { if (hasValue(html)) { + // Replace REST URL with UI URL + if (environment.ssr.replaceRestUrl && REST_BASE_URL !== environment.rest.baseUrl) { + html = html.replace(new RegExp(REST_BASE_URL, 'g'), environment.rest.baseUrl); + } + // save server side rendered page to cache (if any are enabled) saveToCache(req, html); if (sendToUser) { @@ -623,7 +631,7 @@ function start() { * The callback function to serve health check requests */ function healthCheck(req, res) { - const baseUrl = `${environment.rest.baseUrl}${environment.actuators.endpointPath}`; + const baseUrl = `${REST_BASE_URL}${environment.actuators.endpointPath}`; axios.get(baseUrl) .then((response) => { res.status(response.status).send(response.data); diff --git a/src/app/app.config.ts b/src/app/app.config.ts index 61a04c8adbb..77b29206cb5 100644 --- a/src/app/app.config.ts +++ b/src/app/app.config.ts @@ -54,6 +54,7 @@ import { } from './app-routes'; import { BROWSE_BY_DECORATOR_MAP } from './browse-by/browse-by-switcher/browse-by-decorator'; import { AuthInterceptor } from './core/auth/auth.interceptor'; +import { DspaceRestInterceptor } from './core/dspace-rest/dspace-rest.interceptor'; import { LocaleInterceptor } from './core/locale/locale.interceptor'; import { LogInterceptor } from './core/log/log.interceptor'; import { @@ -148,6 +149,11 @@ export const commonAppConfig: ApplicationConfig = { useClass: LogInterceptor, multi: true, }, + { + provide: HTTP_INTERCEPTORS, + useClass: DspaceRestInterceptor, + multi: true, + }, // register the dynamic matcher used by form. MUST be provided by the app module ...DYNAMIC_MATCHER_PROVIDERS, provideCore(), diff --git a/src/app/core/dspace-rest/dspace-rest.interceptor.spec.ts b/src/app/core/dspace-rest/dspace-rest.interceptor.spec.ts new file mode 100644 index 00000000000..4a47ffe9fd2 --- /dev/null +++ b/src/app/core/dspace-rest/dspace-rest.interceptor.spec.ts @@ -0,0 +1,194 @@ +import { + HTTP_INTERCEPTORS, + HttpClient, +} from '@angular/common/http'; +import { + HttpClientTestingModule, + HttpTestingController, +} from '@angular/common/http/testing'; +import { PLATFORM_ID } from '@angular/core'; +import { TestBed } from '@angular/core/testing'; + +import { + APP_CONFIG, + AppConfig, +} from '../../../config/app-config.interface'; +import { DspaceRestInterceptor } from './dspace-rest.interceptor'; +import { DspaceRestService } from './dspace-rest.service'; + +describe('DspaceRestInterceptor', () => { + let httpMock: HttpTestingController; + let httpClient: HttpClient; + const appConfig: Partial = { + rest: { + ssl: false, + host: 'localhost', + port: 8080, + nameSpace: '/server', + baseUrl: 'http://api.example.com/server', + }, + }; + const appConfigWithSSR: Partial = { + rest: { + ssl: false, + host: 'localhost', + port: 8080, + nameSpace: '/server', + baseUrl: 'http://api.example.com/server', + ssrBaseUrl: 'http://ssr.example.com/server', + }, + }; + + describe('When SSR base URL is not set ', () => { + describe('and it\'s in the browser', () => { + beforeEach(() => { + TestBed.configureTestingModule({ + imports: [HttpClientTestingModule], + providers: [ + DspaceRestService, + { + provide: HTTP_INTERCEPTORS, + useClass: DspaceRestInterceptor, + multi: true, + }, + { provide: APP_CONFIG, useValue: appConfig }, + { provide: PLATFORM_ID, useValue: 'browser' }, + ], + }); + + httpMock = TestBed.inject(HttpTestingController); + httpClient = TestBed.inject(HttpClient); + }); + + it('should not modify the request', () => { + const url = 'http://api.example.com/server/items'; + httpClient.get(url).subscribe((response) => { + expect(response).toBeTruthy(); + }); + + const req = httpMock.expectOne(url); + expect(req.request.url).toBe(url); + req.flush({}); + httpMock.verify(); + }); + }); + + describe('and it\'s in SSR mode', () => { + beforeEach(() => { + TestBed.configureTestingModule({ + imports: [HttpClientTestingModule], + providers: [ + DspaceRestService, + { + provide: HTTP_INTERCEPTORS, + useClass: DspaceRestInterceptor, + multi: true, + }, + { provide: APP_CONFIG, useValue: appConfig }, + { provide: PLATFORM_ID, useValue: 'server' }, + ], + }); + + httpMock = TestBed.inject(HttpTestingController); + httpClient = TestBed.inject(HttpClient); + }); + + it('should not replace the base URL', () => { + const url = 'http://api.example.com/server/items'; + + httpClient.get(url).subscribe((response) => { + expect(response).toBeTruthy(); + }); + + const req = httpMock.expectOne(url); + expect(req.request.url).toBe(url); + req.flush({}); + httpMock.verify(); + }); + }); + }); + + describe('When SSR base URL is set ', () => { + describe('and it\'s in the browser', () => { + beforeEach(() => { + TestBed.configureTestingModule({ + imports: [HttpClientTestingModule], + providers: [ + DspaceRestService, + { + provide: HTTP_INTERCEPTORS, + useClass: DspaceRestInterceptor, + multi: true, + }, + { provide: APP_CONFIG, useValue: appConfigWithSSR }, + { provide: PLATFORM_ID, useValue: 'browser' }, + ], + }); + + httpMock = TestBed.inject(HttpTestingController); + httpClient = TestBed.inject(HttpClient); + }); + + it('should not modify the request', () => { + const url = 'http://api.example.com/server/items'; + httpClient.get(url).subscribe((response) => { + expect(response).toBeTruthy(); + }); + + const req = httpMock.expectOne(url); + expect(req.request.url).toBe(url); + req.flush({}); + httpMock.verify(); + }); + }); + + describe('and it\'s in SSR mode', () => { + beforeEach(() => { + TestBed.configureTestingModule({ + imports: [HttpClientTestingModule], + providers: [ + DspaceRestService, + { + provide: HTTP_INTERCEPTORS, + useClass: DspaceRestInterceptor, + multi: true, + }, + { provide: APP_CONFIG, useValue: appConfigWithSSR }, + { provide: PLATFORM_ID, useValue: 'server' }, + ], + }); + + httpMock = TestBed.inject(HttpTestingController); + httpClient = TestBed.inject(HttpClient); + }); + + it('should replace the base URL', () => { + const url = 'http://api.example.com/server/items'; + const ssrBaseUrl = appConfigWithSSR.rest.ssrBaseUrl; + + httpClient.get(url).subscribe((response) => { + expect(response).toBeTruthy(); + }); + + const req = httpMock.expectOne(ssrBaseUrl + '/items'); + expect(req.request.url).toBe(ssrBaseUrl + '/items'); + req.flush({}); + httpMock.verify(); + }); + + it('should not replace any query param containing the base URL', () => { + const url = 'http://api.example.com/server/items?url=http://api.example.com/server/item/1'; + const ssrBaseUrl = appConfigWithSSR.rest.ssrBaseUrl; + + httpClient.get(url).subscribe((response) => { + expect(response).toBeTruthy(); + }); + + const req = httpMock.expectOne(ssrBaseUrl + '/items?url=http://api.example.com/server/item/1'); + expect(req.request.url).toBe(ssrBaseUrl + '/items?url=http://api.example.com/server/item/1'); + req.flush({}); + httpMock.verify(); + }); + }); + }); +}); diff --git a/src/app/core/dspace-rest/dspace-rest.interceptor.ts b/src/app/core/dspace-rest/dspace-rest.interceptor.ts new file mode 100644 index 00000000000..efd2c12b5d4 --- /dev/null +++ b/src/app/core/dspace-rest/dspace-rest.interceptor.ts @@ -0,0 +1,52 @@ +import { isPlatformBrowser } from '@angular/common'; +import { + HttpEvent, + HttpHandler, + HttpInterceptor, + HttpRequest, +} from '@angular/common/http'; +import { + Inject, + Injectable, + PLATFORM_ID, +} from '@angular/core'; +import { Observable } from 'rxjs'; + +import { + APP_CONFIG, + AppConfig, +} from '../../../config/app-config.interface'; +import { isEmpty } from '../../shared/empty.util'; + +@Injectable() +/** + * This Interceptor is used to use the configured base URL for the request made during SSR execution + */ +export class DspaceRestInterceptor implements HttpInterceptor { + + /** + * Contains the configured application base URL + * @protected + */ + protected baseUrl: string; + protected ssrBaseUrl: string; + + constructor( + @Inject(APP_CONFIG) protected appConfig: AppConfig, + @Inject(PLATFORM_ID) private platformId: string, + ) { + this.baseUrl = this.appConfig.rest.baseUrl; + this.ssrBaseUrl = this.appConfig.rest.ssrBaseUrl; + } + + intercept(request: HttpRequest, next: HttpHandler): Observable> { + if (isPlatformBrowser(this.platformId) || isEmpty(this.ssrBaseUrl) || this.baseUrl === this.ssrBaseUrl) { + return next.handle(request); + } + + // Different SSR Base URL specified so replace it in the current request url + const url = request.url.replace(this.baseUrl, this.ssrBaseUrl); + const newRequest: HttpRequest = request.clone({ url }); + return next.handle(newRequest); + } +} 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 27777ed7ba9..a904a8e66cf 100644 --- a/src/app/core/services/server-hard-redirect.service.spec.ts +++ b/src/app/core/services/server-hard-redirect.service.spec.ts @@ -1,5 +1,6 @@ import { TestBed } from '@angular/core/testing'; +import { environment } from '../../../environments/environment.test'; import { ServerHardRedirectService } from './server-hard-redirect.service'; describe('ServerHardRedirectService', () => { @@ -7,7 +8,7 @@ describe('ServerHardRedirectService', () => { const mockRequest = jasmine.createSpyObj(['get']); const mockResponse = jasmine.createSpyObj(['redirect', 'end']); - const service: ServerHardRedirectService = new ServerHardRedirectService(mockRequest, mockResponse); + let service: ServerHardRedirectService = new ServerHardRedirectService(environment, mockRequest, mockResponse); const origin = 'https://test-host.com:4000'; beforeEach(() => { @@ -68,4 +69,23 @@ describe('ServerHardRedirectService', () => { }); }); + describe('when SSR base url is set', () => { + const redirect = 'https://private-url:4000/server/api/bitstreams/uuid'; + const replacedUrl = 'https://public-url/server/api/bitstreams/uuid'; + const environmentWithSSRUrl: any = { ...environment, ...{ ...environment.rest, rest: { + ssrBaseUrl: 'https://private-url:4000/server', + baseUrl: 'https://public-url/server', + } } }; + service = new ServerHardRedirectService(environmentWithSSRUrl, mockRequest, mockResponse); + + beforeEach(() => { + service.redirect(redirect); + }); + + it('should perform a 302 redirect', () => { + expect(mockResponse.redirect).toHaveBeenCalledWith(302, replacedUrl); + expect(mockResponse.end).toHaveBeenCalled(); + }); + }); + }); diff --git a/src/app/core/services/server-hard-redirect.service.ts b/src/app/core/services/server-hard-redirect.service.ts index e1ded1568cc..1592d9bf1cf 100644 --- a/src/app/core/services/server-hard-redirect.service.ts +++ b/src/app/core/services/server-hard-redirect.service.ts @@ -7,10 +7,15 @@ import { Response, } from 'express'; +import { + APP_CONFIG, + AppConfig, +} from '../../../config/app-config.interface'; import { REQUEST, RESPONSE, } from '../../../express.tokens'; +import { isNotEmpty } from '../../shared/empty.util'; import { HardRedirectService } from './hard-redirect.service'; /** @@ -20,6 +25,7 @@ import { HardRedirectService } from './hard-redirect.service'; export class ServerHardRedirectService extends HardRedirectService { constructor( + @Inject(APP_CONFIG) protected appConfig: AppConfig, @Inject(REQUEST) protected req: Request, @Inject(RESPONSE) protected res: Response, ) { @@ -35,17 +41,22 @@ export class ServerHardRedirectService extends HardRedirectService { * optional HTTP status code to use for redirect (default = 302, which is a temporary redirect) */ redirect(url: string, statusCode?: number) { - if (url === this.req.url) { return; } + let redirectUrl = url; + // If redirect url contains SSR base url then replace with public base url + if (isNotEmpty(this.appConfig.rest.ssrBaseUrl) && this.appConfig.rest.baseUrl !== this.appConfig.rest.ssrBaseUrl) { + redirectUrl = url.replace(this.appConfig.rest.ssrBaseUrl, this.appConfig.rest.baseUrl); + } + if (this.res.finished) { const req: any = this.req; req._r_count = (req._r_count || 0) + 1; console.warn('Attempted to redirect on a finished response. From', - this.req.url, 'to', url); + this.req.url, 'to', redirectUrl); if (req._r_count > 10) { console.error('Detected a redirection loop. killing the nodejs process'); @@ -59,9 +70,9 @@ export class ServerHardRedirectService extends HardRedirectService { status = 302; } - console.log(`Redirecting from ${this.req.url} to ${url} with ${status}`); + console.info(`Redirecting from ${this.req.url} to ${redirectUrl} with ${status}`); - this.res.redirect(status, url); + this.res.redirect(status, redirectUrl); this.res.end(); // I haven't found a way to correctly stop Angular rendering. // So we just let it end its work, though we have already closed diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/models/lookup/dynamic-lookup.component.html b/src/app/shared/form/builder/ds-dynamic-form-ui/models/lookup/dynamic-lookup.component.html index 89e7af9aa67..79f2756b249 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/models/lookup/dynamic-lookup.component.html +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/models/lookup/dynamic-lookup.component.html @@ -94,11 +94,12 @@ (scrolled)="onScroll()" [scrollWindow]="false"> - - + diff --git a/src/app/shared/form/form.service.ts b/src/app/shared/form/form.service.ts index 1ca844d690d..11fe5c54de3 100644 --- a/src/app/shared/form/form.service.ts +++ b/src/app/shared/form/form.service.ts @@ -153,6 +153,9 @@ export class FormService { } public addControlErrors(field: AbstractControl, formId: string, fieldId: string, fieldIndex: number) { + if (field.errors === null) { + return; + } const errors: string[] = Object.keys(field.errors) .filter((errorKey) => field.errors[errorKey] === true) .map((errorKey) => `error.validation.${errorKey}`); diff --git a/src/app/thumbnail/thumbnail.component.spec.ts b/src/app/thumbnail/thumbnail.component.spec.ts index 10e461112bb..f81089e57c9 100644 --- a/src/app/thumbnail/thumbnail.component.spec.ts +++ b/src/app/thumbnail/thumbnail.component.spec.ts @@ -2,6 +2,7 @@ import { DebugElement, Pipe, PipeTransform, + PLATFORM_ID, } from '@angular/core'; import { ComponentFixture, @@ -48,298 +49,261 @@ describe('ThumbnailComponent', () => { let authService; let authorizationService; let fileService; + let spy; - beforeEach(waitForAsync(() => { - authService = jasmine.createSpyObj('AuthService', { - isAuthenticated: observableOf(true), - }); - authorizationService = jasmine.createSpyObj('AuthorizationService', { - isAuthorized: observableOf(true), - }); - fileService = jasmine.createSpyObj('FileService', { - retrieveFileDownloadLink: null, - }); - fileService.retrieveFileDownloadLink.and.callFake((url) => observableOf(`${url}?authentication-token=fake`)); - - TestBed.configureTestingModule({ - imports: [ - TranslateModule.forRoot(), - ThumbnailComponent, - SafeUrlPipe, - MockTranslatePipe, - VarDirective, - ], - providers: [ - { provide: AuthService, useValue: authService }, - { provide: AuthorizationDataService, useValue: authorizationService }, - { provide: FileService, useValue: fileService }, - { provide: ThemeService, useValue: getMockThemeService() }, - ], - }).overrideComponent(ThumbnailComponent, { - add: { - imports: [MockTranslatePipe], - }, - }) - .compileComponents(); - })); - - beforeEach(() => { - fixture = TestBed.createComponent(ThumbnailComponent); - fixture.detectChanges(); - - authService = TestBed.inject(AuthService); - - comp = fixture.componentInstance; // ThumbnailComponent test instance - de = fixture.debugElement.query(By.css('div.thumbnail')); - el = de.nativeElement; - }); + describe('when platform is browser', () => { + beforeEach(waitForAsync(() => { + authService = jasmine.createSpyObj('AuthService', { + isAuthenticated: observableOf(true), + }); + authorizationService = jasmine.createSpyObj('AuthorizationService', { + isAuthorized: observableOf(true), + }); + fileService = jasmine.createSpyObj('FileService', { + retrieveFileDownloadLink: null, + }); + fileService.retrieveFileDownloadLink.and.callFake((url) => observableOf(`${url}?authentication-token=fake`)); + + TestBed.configureTestingModule({ + imports: [ + TranslateModule.forRoot(), + ThumbnailComponent, + SafeUrlPipe, + MockTranslatePipe, + VarDirective, + ], + providers: [ + { provide: AuthService, useValue: authService }, + { provide: AuthorizationDataService, useValue: authorizationService }, + { provide: FileService, useValue: fileService }, + { provide: ThemeService, useValue: getMockThemeService() }, + { provide: PLATFORM_ID, useValue: 'browser' }, + ], + }).overrideComponent(ThumbnailComponent, { + add: { + imports: [MockTranslatePipe], + }, + }) + .compileComponents(); + })); - describe('loading', () => { - it('should start out with isLoading$ true', () => { - expect(comp.isLoading).toBeTrue(); - }); + beforeEach(() => { + fixture = TestBed.createComponent(ThumbnailComponent); + fixture.detectChanges(); - it('should set isLoading$ to false once an image is successfully loaded', () => { - comp.setSrc('http://bit.stream'); - fixture.debugElement.query(By.css('img.thumbnail-content')).triggerEventHandler('load', new Event('load')); - expect(comp.isLoading).toBeFalse(); - }); + authService = TestBed.inject(AuthService); - it('should set isLoading$ to false once the src is set to null', () => { - comp.setSrc(null); - expect(comp.isLoading).toBeFalse(); + comp = fixture.componentInstance; // ThumbnailComponent test instance + de = fixture.debugElement.query(By.css('div.thumbnail')); + el = de.nativeElement; }); - it('should show a loading animation while isLoading$ is true', () => { - expect(de.query(By.css('ds-loading'))).toBeTruthy(); + describe('loading', () => { + it('should start out with isLoading$ true', () => { + expect(comp.isLoading).toBeTrue(); + }); - comp.isLoading = false; - fixture.detectChanges(); - expect(fixture.debugElement.query(By.css('ds-loading'))).toBeFalsy(); - }); + it('should set isLoading$ to false once an image is successfully loaded', () => { + comp.setSrc('http://bit.stream'); + fixture.debugElement.query(By.css('img.thumbnail-content')).triggerEventHandler('load', new Event('load')); + expect(comp.isLoading).toBeFalse(); + }); - describe('with a thumbnail image', () => { - beforeEach(() => { - comp.src = 'https://bit.stream'; - fixture.detectChanges(); + it('should set isLoading$ to false once the src is set to null', () => { + comp.setSrc(null); + expect(comp.isLoading).toBeFalse(); }); - it('should render but hide the image while loading and show it once done', () => { - let img = fixture.debugElement.query(By.css('img.thumbnail-content')); - expect(img).toBeTruthy(); - expect(img.classes['d-none']).toBeTrue(); + it('should show a loading animation while isLoading$ is true', () => { + expect(de.query(By.css('ds-loading'))).toBeTruthy(); comp.isLoading = false; fixture.detectChanges(); - img = fixture.debugElement.query(By.css('img.thumbnail-content')); - expect(img).toBeTruthy(); - expect(img.classes['d-none']).toBeFalsy(); + expect(fixture.debugElement.query(By.css('ds-loading'))).toBeFalsy(); }); - }); + describe('with a thumbnail image', () => { + beforeEach(() => { + comp.src = 'https://bit.stream'; + fixture.detectChanges(); + }); - describe('without a thumbnail image', () => { - beforeEach(() => { - comp.src = null; - fixture.detectChanges(); - }); + it('should render but hide the image while loading and show it once done', () => { + let img = fixture.debugElement.query(By.css('img.thumbnail-content')); + expect(img).toBeTruthy(); + expect(img.classes['d-none']).toBeTrue(); - it('should only show the HTML placeholder once done loading', () => { - expect(fixture.debugElement.query(By.css('div.thumbnail-placeholder'))).toBeFalsy(); + comp.isLoading = false; + fixture.detectChanges(); + img = fixture.debugElement.query(By.css('img.thumbnail-content')); + expect(img).toBeTruthy(); + expect(img.classes['d-none']).toBeFalsy(); + }); - comp.isLoading = false; - fixture.detectChanges(); - expect(fixture.debugElement.query(By.css('div.thumbnail-placeholder'))).toBeTruthy(); }); - }); - }); + describe('without a thumbnail image', () => { + beforeEach(() => { + comp.src = null; + fixture.detectChanges(); + }); - const errorHandler = () => { - let setSrcSpy; + it('should only show the HTML placeholder once done loading', () => { + expect(fixture.debugElement.query(By.css('div.thumbnail-placeholder'))).toBeFalsy(); - beforeEach(() => { - // disconnect error handler to be sure it's only called once - const img = fixture.debugElement.query(By.css('img.thumbnail-content')); - img.nativeNode.onerror = null; + comp.isLoading = false; + fixture.detectChanges(); + expect(fixture.debugElement.query(By.css('div.thumbnail-placeholder'))).toBeTruthy(); + }); + }); - comp.ngOnChanges({}); - setSrcSpy = spyOn(comp, 'setSrc').and.callThrough(); }); - describe('retry with authentication token', () => { - it('should remember that it already retried once', () => { - expect(comp.retriedWithToken).toBeFalse(); - comp.errorHandler(); - expect(comp.retriedWithToken).toBeTrue(); - }); + const errorHandler = () => { + let setSrcSpy; - describe('if not logged in', () => { - beforeEach(() => { - authService.isAuthenticated.and.returnValue(observableOf(false)); - }); + beforeEach(() => { + // disconnect error handler to be sure it's only called once + const img = fixture.debugElement.query(By.css('img.thumbnail-content')); + img.nativeNode.onerror = null; - it('should fall back to default', () => { - comp.errorHandler(); - expect(setSrcSpy).toHaveBeenCalledWith(comp.defaultImage); - }); + comp.ngOnChanges({}); + setSrcSpy = spyOn(comp, 'setSrc').and.callThrough(); }); - describe('if logged in', () => { - beforeEach(() => { - authService.isAuthenticated.and.returnValue(observableOf(true)); + describe('retry with authentication token', () => { + it('should remember that it already retried once', () => { + expect(comp.retriedWithToken).toBeFalse(); + comp.errorHandler(); + expect(comp.retriedWithToken).toBeTrue(); }); - describe('and authorized to download the thumbnail', () => { + describe('if not logged in', () => { beforeEach(() => { - authorizationService.isAuthorized.and.returnValue(observableOf(true)); + authService.isAuthenticated.and.returnValue(observableOf(false)); }); - it('should add an authentication token to the thumbnail URL', () => { + it('should fall back to default', () => { comp.errorHandler(); - - if ((comp.thumbnail as RemoteData)?.hasFailed) { - // If we failed to retrieve the Bitstream in the first place, fall back to the default - expect(setSrcSpy).toHaveBeenCalledWith(comp.defaultImage); - } else { - expect(setSrcSpy).toHaveBeenCalledWith(CONTENT + '?authentication-token=fake'); - } + expect(setSrcSpy).toHaveBeenCalledWith(comp.defaultImage); }); }); - describe('but not authorized to download the thumbnail', () => { + describe('if logged in', () => { beforeEach(() => { - authorizationService.isAuthorized.and.returnValue(observableOf(false)); + authService.isAuthenticated.and.returnValue(observableOf(true)); }); - it('should fall back to default', () => { - comp.errorHandler(); - - expect(setSrcSpy).toHaveBeenCalledWith(comp.defaultImage); - - // We don't need to check authorization if we failed to retrieve the Bitstreamin the first place - if (!(comp.thumbnail as RemoteData)?.hasFailed) { - expect(authorizationService.isAuthorized).toHaveBeenCalled(); - } + describe('and authorized to download the thumbnail', () => { + beforeEach(() => { + authorizationService.isAuthorized.and.returnValue(observableOf(true)); + }); + + it('should add an authentication token to the thumbnail URL', () => { + comp.errorHandler(); + + if ((comp.thumbnail as RemoteData)?.hasFailed) { + // If we failed to retrieve the Bitstream in the first place, fall back to the default + expect(setSrcSpy).toHaveBeenCalledWith(comp.defaultImage); + } else { + expect(setSrcSpy).toHaveBeenCalledWith(CONTENT + '?authentication-token=fake'); + } + }); }); - }); - }); - }); - describe('after retrying with token', () => { - beforeEach(() => { - comp.retriedWithToken = true; - }); + describe('but not authorized to download the thumbnail', () => { + beforeEach(() => { + authorizationService.isAuthorized.and.returnValue(observableOf(false)); + }); - it('should fall back to default', () => { - comp.errorHandler(); - expect(authService.isAuthenticated).not.toHaveBeenCalled(); - expect(fileService.retrieveFileDownloadLink).not.toHaveBeenCalled(); - expect(setSrcSpy).toHaveBeenCalledWith(comp.defaultImage); - }); - }); - }; - - describe('fallback', () => { - describe('if there is a default image', () => { - it('should display the default image', () => { - comp.src = 'http://bit.stream'; - comp.defaultImage = 'http://default.img'; - comp.errorHandler(); - expect(comp.src).toBe(comp.defaultImage); - }); + it('should fall back to default', () => { + comp.errorHandler(); - it('should include the alt text', () => { - comp.src = 'http://bit.stream'; - comp.defaultImage = 'http://default.img'; - comp.errorHandler(); + expect(setSrcSpy).toHaveBeenCalledWith(comp.defaultImage); - fixture.detectChanges(); - const image: HTMLElement = fixture.debugElement.query(By.css('img')).nativeElement; - expect(image.getAttribute('alt')).toBe('TRANSLATED ' + comp.alt); + // We don't need to check authorization if we failed to retrieve the Bitstreamin the first place + if (!(comp.thumbnail as RemoteData)?.hasFailed) { + expect(authorizationService.isAuthorized).toHaveBeenCalled(); + } + }); + }); + }); }); - }); - describe('if there is no default image', () => { - it('should display the HTML placeholder', () => { - comp.src = 'http://default.img'; - comp.defaultImage = null; - comp.errorHandler(); - expect(comp.src).toBe(null); + describe('after retrying with token', () => { + beforeEach(() => { + comp.retriedWithToken = true; + }); - fixture.detectChanges(); - const placeholder = fixture.debugElement.query(By.css('div.thumbnail-placeholder')).nativeElement; - expect(placeholder.innerHTML).toContain('TRANSLATED ' + comp.placeholder); + it('should fall back to default', () => { + comp.errorHandler(); + expect(authService.isAuthenticated).not.toHaveBeenCalled(); + expect(fileService.retrieveFileDownloadLink).not.toHaveBeenCalled(); + expect(setSrcSpy).toHaveBeenCalledWith(comp.defaultImage); + }); }); - }); - }); + }; - describe('with thumbnail as Bitstream', () => { - let thumbnail; - beforeEach(() => { - thumbnail = new Bitstream(); - thumbnail._links = { - self: { href: 'self.url' }, - bundle: { href: 'bundle.url' }, - format: { href: 'format.url' }, - content: { href: CONTENT }, - thumbnail: undefined, - }; - comp.thumbnail = thumbnail; - }); + describe('fallback', () => { + describe('if there is a default image', () => { + it('should display the default image', () => { + comp.src = 'http://bit.stream'; + comp.defaultImage = 'http://default.img'; + comp.errorHandler(); + expect(comp.src).toBe(comp.defaultImage); + }); - describe('if content can be loaded', () => { - it('should display an image', () => { - comp.ngOnChanges({}); - fixture.detectChanges(); - const image: HTMLElement = fixture.debugElement.query(By.css('img')).nativeElement; - expect(image.getAttribute('src')).toBe(thumbnail._links.content.href); - }); + it('should include the alt text', () => { + comp.src = 'http://bit.stream'; + comp.defaultImage = 'http://default.img'; + comp.errorHandler(); - it('should include the alt text', () => { - comp.ngOnChanges({}); - fixture.detectChanges(); - const image: HTMLElement = fixture.debugElement.query(By.css('img')).nativeElement; - expect(image.getAttribute('alt')).toBe('TRANSLATED ' + comp.alt); + fixture.detectChanges(); + const image: HTMLElement = fixture.debugElement.query(By.css('img')).nativeElement; + expect(image.getAttribute('alt')).toBe('TRANSLATED ' + comp.alt); + }); }); - }); - - describe('if content can\'t be loaded', () => { - errorHandler(); - }); - }); - describe('with thumbnail as RemoteData', () => { - let thumbnail: Bitstream; + describe('if there is no default image', () => { + it('should display the HTML placeholder', () => { + comp.src = 'http://default.img'; + comp.defaultImage = null; + comp.errorHandler(); + expect(comp.src).toBe(null); - beforeEach(() => { - thumbnail = new Bitstream(); - thumbnail._links = { - self: { href: 'self.url' }, - bundle: { href: 'bundle.url' }, - format: { href: 'format.url' }, - content: { href: CONTENT }, - thumbnail: undefined, - }; + fixture.detectChanges(); + const placeholder = fixture.debugElement.query(By.css('div.thumbnail-placeholder')).nativeElement; + expect(placeholder.innerHTML).toContain('TRANSLATED ' + comp.placeholder); + }); + }); }); - describe('if RemoteData succeeded', () => { + describe('with thumbnail as Bitstream', () => { + let thumbnail; beforeEach(() => { - comp.thumbnail = createSuccessfulRemoteDataObject(thumbnail); + thumbnail = new Bitstream(); + thumbnail._links = { + self: { href: 'self.url' }, + bundle: { href: 'bundle.url' }, + format: { href: 'format.url' }, + content: { href: CONTENT }, + thumbnail: undefined, + }; + comp.thumbnail = thumbnail; }); describe('if content can be loaded', () => { it('should display an image', () => { comp.ngOnChanges({}); fixture.detectChanges(); - const image: HTMLElement = de.query(By.css('img')).nativeElement; + const image: HTMLElement = fixture.debugElement.query(By.css('img')).nativeElement; expect(image.getAttribute('src')).toBe(thumbnail._links.content.href); }); - it('should display the alt text', () => { + it('should include the alt text', () => { comp.ngOnChanges({}); fixture.detectChanges(); - const image: HTMLElement = de.query(By.css('img')).nativeElement; + const image: HTMLElement = fixture.debugElement.query(By.css('img')).nativeElement; expect(image.getAttribute('alt')).toBe('TRANSLATED ' + comp.alt); }); }); @@ -349,16 +313,117 @@ describe('ThumbnailComponent', () => { }); }); - describe('if RemoteData failed', () => { + describe('with thumbnail as RemoteData', () => { + let thumbnail: Bitstream; + beforeEach(() => { - comp.thumbnail = createFailedRemoteDataObject(); + thumbnail = new Bitstream(); + thumbnail._links = { + self: { href: 'self.url' }, + bundle: { href: 'bundle.url' }, + format: { href: 'format.url' }, + content: { href: CONTENT }, + thumbnail: undefined, + }; }); - it('should show the default image', () => { - comp.defaultImage = 'default/image.jpg'; - comp.ngOnChanges({}); - expect(comp.src).toBe('default/image.jpg'); + describe('if RemoteData succeeded', () => { + beforeEach(() => { + comp.thumbnail = createSuccessfulRemoteDataObject(thumbnail); + }); + + describe('if content can be loaded', () => { + it('should display an image', () => { + comp.ngOnChanges({}); + fixture.detectChanges(); + const image: HTMLElement = de.query(By.css('img')).nativeElement; + expect(image.getAttribute('src')).toBe(thumbnail._links.content.href); + }); + + it('should display the alt text', () => { + comp.ngOnChanges({}); + fixture.detectChanges(); + const image: HTMLElement = de.query(By.css('img')).nativeElement; + expect(image.getAttribute('alt')).toBe('TRANSLATED ' + comp.alt); + }); + }); + + describe('if content can\'t be loaded', () => { + errorHandler(); + }); + }); + + describe('if RemoteData failed', () => { + beforeEach(() => { + comp.thumbnail = createFailedRemoteDataObject(); + }); + + it('should show the default image', () => { + comp.defaultImage = 'default/image.jpg'; + comp.ngOnChanges({}); + expect(comp.src).toBe('default/image.jpg'); + }); }); }); }); + + describe('when platform is server', () => { + beforeEach(waitForAsync(() => { + + authService = jasmine.createSpyObj('AuthService', { + isAuthenticated: observableOf(true), + }); + authorizationService = jasmine.createSpyObj('AuthorizationService', { + isAuthorized: observableOf(true), + }); + fileService = jasmine.createSpyObj('FileService', { + retrieveFileDownloadLink: null, + }); + fileService.retrieveFileDownloadLink.and.callFake((url) => observableOf(`${url}?authentication-token=fake`)); + + TestBed.configureTestingModule({ + imports: [ + TranslateModule.forRoot(), + ThumbnailComponent, + SafeUrlPipe, + MockTranslatePipe, + VarDirective, + ], + providers: [ + { provide: AuthService, useValue: authService }, + { provide: AuthorizationDataService, useValue: authorizationService }, + { provide: FileService, useValue: fileService }, + { provide: ThemeService, useValue: getMockThemeService() }, + { provide: PLATFORM_ID, useValue: 'server' }, + ], + }).overrideComponent(ThumbnailComponent, { + add: { + imports: [MockTranslatePipe], + }, + }) + .compileComponents(); + })); + + beforeEach(() => { + fixture = TestBed.createComponent(ThumbnailComponent); + spyOn(fixture.componentInstance, 'setSrc').and.callThrough(); + fixture.detectChanges(); + + authService = TestBed.inject(AuthService); + + comp = fixture.componentInstance; // ThumbnailComponent test instance + de = fixture.debugElement.query(By.css('div.thumbnail')); + el = de.nativeElement; + }); + + it('should start out with isLoading$ true', () => { + expect(comp.isLoading).toBeTrue(); + expect(de.query(By.css('ds-loading'))).toBeTruthy(); + }); + + it('should not call setSrc', () => { + expect(comp.setSrc).not.toHaveBeenCalled(); + }); + + }); }); diff --git a/src/app/thumbnail/thumbnail.component.ts b/src/app/thumbnail/thumbnail.component.ts index cc583c3998f..7b22dde4cd7 100644 --- a/src/app/thumbnail/thumbnail.component.ts +++ b/src/app/thumbnail/thumbnail.component.ts @@ -1,8 +1,13 @@ -import { CommonModule } from '@angular/common'; +import { + CommonModule, + isPlatformBrowser, +} from '@angular/common'; import { Component, + Inject, Input, OnChanges, + PLATFORM_ID, SimpleChanges, } from '@angular/core'; import { TranslateModule } from '@ngx-translate/core'; @@ -76,6 +81,7 @@ export class ThumbnailComponent implements OnChanges { isLoading = true; constructor( + @Inject(PLATFORM_ID) private platformID: any, protected auth: AuthService, protected authorizationService: AuthorizationDataService, protected fileService: FileService, @@ -87,16 +93,18 @@ export class ThumbnailComponent implements OnChanges { * Use a default image if no actual image is available. */ ngOnChanges(changes: SimpleChanges): void { - if (hasNoValue(this.thumbnail)) { - this.setSrc(this.defaultImage); - return; - } + if (isPlatformBrowser(this.platformID)) { + if (hasNoValue(this.thumbnail)) { + this.setSrc(this.defaultImage); + return; + } - const src = this.contentHref; - if (hasValue(src)) { - this.setSrc(src); - } else { - this.setSrc(this.defaultImage); + const src = this.contentHref; + if (hasValue(src)) { + this.setSrc(src); + } else { + this.setSrc(this.defaultImage); + } } } diff --git a/src/config/server-config.interface.ts b/src/config/server-config.interface.ts index 8797ea3d600..cdf23cd1463 100644 --- a/src/config/server-config.interface.ts +++ b/src/config/server-config.interface.ts @@ -6,4 +6,6 @@ export class ServerConfig implements Config { public port: number; public nameSpace: string; public baseUrl?: string; + public ssrBaseUrl?: string; + public hasSsrBaseUrl?: boolean; } diff --git a/src/config/ssr-config.interface.ts b/src/config/ssr-config.interface.ts index 37b8b4a0d2b..83d149e3b3c 100644 --- a/src/config/ssr-config.interface.ts +++ b/src/config/ssr-config.interface.ts @@ -20,4 +20,22 @@ export interface SSRConfig extends Config { * For improved SSR performance, DSpace defaults this to false (disabled). */ inlineCriticalCss: boolean; + + /** + * Enable state transfer from the server-side application to the client-side application. + * Defaults to true. + * + * Note: When using an external application cache layer, it's recommended not to transfer the state to avoid caching it. + * Disabling it ensures that dynamic state information is not inadvertently cached, which can improve security and + * ensure that users always use the most up-to-date state. + */ + transferState: boolean; + + /** + * When a different REST base URL is used for the server-side application, the generated state contains references to + * REST resources with the internal URL configured, so it is not transferred to the client application, by default. + * Enabling this setting transfers the state to the client application and replaces internal URLs with the public + * URLs used by the client application. + */ + replaceRestUrl: boolean; } diff --git a/src/environments/environment.production.ts b/src/environments/environment.production.ts index 90b219cdbd7..8d46e5f614f 100644 --- a/src/environments/environment.production.ts +++ b/src/environments/environment.production.ts @@ -8,5 +8,7 @@ export const environment: Partial = { enabled: true, enablePerformanceProfiler: false, inlineCriticalCss: false, + transferState: true, + replaceRestUrl: false, }, }; diff --git a/src/environments/environment.test.ts b/src/environments/environment.test.ts index c7146dfb078..ac33f699b38 100644 --- a/src/environments/environment.test.ts +++ b/src/environments/environment.test.ts @@ -12,6 +12,8 @@ export const environment: BuildConfig = { enabled: true, enablePerformanceProfiler: false, inlineCriticalCss: false, + transferState: true, + replaceRestUrl: false, }, // Angular express server settings. diff --git a/src/environments/environment.ts b/src/environments/environment.ts index 0292c19200b..ca9be819458 100644 --- a/src/environments/environment.ts +++ b/src/environments/environment.ts @@ -13,6 +13,8 @@ export const environment: Partial = { enabled: false, enablePerformanceProfiler: false, inlineCriticalCss: false, + transferState: true, + replaceRestUrl: false, }, }; diff --git a/src/modules/app/browser-init.service.ts b/src/modules/app/browser-init.service.ts index 525067da3af..3bbdf12b914 100644 --- a/src/modules/app/browser-init.service.ts +++ b/src/modules/app/browser-init.service.ts @@ -54,6 +54,7 @@ import { APP_CONFIG_STATE, AppConfig, } from '../../config/app-config.interface'; +import { BuildConfig } from '../../config/build-config.interface'; import { extendEnvironmentWithAppConfig } from '../../config/config.util'; import { DefaultAppConfig } from '../../config/default-app-config'; import { environment } from '../../environments/environment'; @@ -70,7 +71,7 @@ export class BrowserInitService extends InitService { protected store: Store, protected correlationIdService: CorrelationIdService, protected transferState: TransferState, - @Inject(APP_CONFIG) protected appConfig: AppConfig, + @Inject(APP_CONFIG) protected appConfig: BuildConfig, protected translate: TranslateService, protected localeService: LocaleService, protected angulartics2DSpace: Angulartics2DSpace, @@ -144,15 +145,20 @@ export class BrowserInitService extends InitService { * @private */ private async loadAppState(): Promise { - const state = this.transferState.get(InitService.NGRX_STATE, null); - this.transferState.remove(InitService.NGRX_STATE); - this.store.dispatch(new StoreAction(StoreActionTypes.REHYDRATE, state)); - return lastValueFrom( - this.store.select(coreSelector).pipe( - find((core: any) => isNotEmpty(core)), - map(() => true), - ), - ); + // The app state can be transferred only when SSR and CSR are using the same base url for the REST API + if (this.appConfig.ssr.transferState) { + const state = this.transferState.get(InitService.NGRX_STATE, null); + this.transferState.remove(InitService.NGRX_STATE); + this.store.dispatch(new StoreAction(StoreActionTypes.REHYDRATE, state)); + return lastValueFrom( + this.store.select(coreSelector).pipe( + find((core: any) => isNotEmpty(core)), + map(() => true), + ), + ); + } else { + return Promise.resolve(true); + } } private trackAuthTokenExpiration(): void { diff --git a/src/modules/app/server-init.service.ts b/src/modules/app/server-init.service.ts index c4aa4ffba3a..60c6cbe4139 100644 --- a/src/modules/app/server-init.service.ts +++ b/src/modules/app/server-init.service.ts @@ -21,6 +21,10 @@ import { LocaleService } from '../../app/core/locale/locale.service'; import { HeadTagService } from '../../app/core/metadata/head-tag.service'; import { CorrelationIdService } from '../../app/correlation-id/correlation-id.service'; import { InitService } from '../../app/init.service'; +import { + isEmpty, + isNotEmpty, +} from '../../app/shared/empty.util'; import { MenuService } from '../../app/shared/menu/menu.service'; import { ThemeService } from '../../app/shared/theme-support/theme.service'; import { Angulartics2DSpace } from '../../app/statistics/angulartics/dspace-provider'; @@ -29,6 +33,7 @@ import { APP_CONFIG_STATE, AppConfig, } from '../../config/app-config.interface'; +import { BuildConfig } from '../../config/build-config.interface'; import { environment } from '../../environments/environment'; /** @@ -40,7 +45,7 @@ export class ServerInitService extends InitService { protected store: Store, protected correlationIdService: CorrelationIdService, protected transferState: TransferState, - @Inject(APP_CONFIG) protected appConfig: AppConfig, + @Inject(APP_CONFIG) protected appConfig: BuildConfig, protected translate: TranslateService, protected localeService: LocaleService, protected angulartics2DSpace: Angulartics2DSpace, @@ -89,17 +94,27 @@ export class ServerInitService extends InitService { * @private */ private saveAppState() { - this.transferState.onSerialize(InitService.NGRX_STATE, () => { - let state; - this.store.pipe(take(1)).subscribe((saveState: any) => { - state = saveState; - }); + if (this.appConfig.ssr.transferState && (isEmpty(this.appConfig.rest.ssrBaseUrl) || this.appConfig.ssr.replaceRestUrl)) { + this.transferState.onSerialize(InitService.NGRX_STATE, () => { + let state; + this.store.pipe(take(1)).subscribe((saveState: any) => { + state = saveState; + }); - return state; - }); + return state; + }); + } } private saveAppConfigForCSR(): void { - this.transferState.set(APP_CONFIG_STATE, environment as AppConfig); + if (isNotEmpty(environment.rest.ssrBaseUrl) && environment.rest.baseUrl !== environment.rest.ssrBaseUrl) { + // Avoid to transfer ssrBaseUrl in order to prevent security issues + const config: AppConfig = Object.assign({}, environment as AppConfig, { + rest: Object.assign({}, environment.rest, { ssrBaseUrl: '', hasSsrBaseUrl: true }), + }); + this.transferState.set(APP_CONFIG_STATE, config); + } else { + this.transferState.set(APP_CONFIG_STATE, environment as AppConfig); + } } }