diff --git a/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.html b/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.html new file mode 100644 index 0000000000..3e360cc55e --- /dev/null +++ b/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.html @@ -0,0 +1,27 @@ +
+
+
+ + + {{'system-wide-alert-banner.countdown.prefix' | translate }} + + + {{'system-wide-alert-banner.countdown.days' | translate: { + days: countDownDays|async + } }} + + + {{'system-wide-alert-banner.countdown.hours' | translate: { + hours: countDownHours| async + } }} + + + {{'system-wide-alert-banner.countdown.minutes' | translate: { + minutes: countDownMinutes|async + } }} + + + {{(systemWideAlert$ |async)?.message}} +
+
+
diff --git a/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.scss b/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.scss new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.spec.ts b/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.spec.ts new file mode 100644 index 0000000000..d27e5379e9 --- /dev/null +++ b/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.spec.ts @@ -0,0 +1,111 @@ +import { ComponentFixture, discardPeriodicTasks, fakeAsync, TestBed, tick, waitForAsync } from '@angular/core/testing'; +import { SystemWideAlertBannerComponent } from './system-wide-alert-banner.component'; +import { SystemWideAlertDataService } from '../../core/data/system-wide-alert-data.service'; +import { SystemWideAlert } from '../system-wide-alert.model'; +import { createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; +import { utcToZonedTime } from 'date-fns-tz'; +import { createPaginatedList } from '../../shared/testing/utils.test'; +import { TestScheduler } from 'rxjs/testing'; +import { getTestScheduler } from 'jasmine-marbles'; +import { By } from '@angular/platform-browser'; +import { TranslateModule } from '@ngx-translate/core'; + + +describe('SystemWideAlertBannerComponent', () => { + let comp: SystemWideAlertBannerComponent; + let fixture: ComponentFixture; + let systemWideAlertDataService: SystemWideAlertDataService; + + let systemWideAlert: SystemWideAlert; + let scheduler: TestScheduler; + + beforeEach(waitForAsync(() => { + scheduler = getTestScheduler(); + + const countDownDate = new Date(); + countDownDate.setDate(countDownDate.getDate() + 1); + countDownDate.setHours(countDownDate.getHours() + 1); + countDownDate.setMinutes(countDownDate.getMinutes() + 1); + + systemWideAlert = Object.assign(new SystemWideAlert(), { + alertId: 1, + message: 'Test alert message', + active: true, + countdownTo: utcToZonedTime(countDownDate, 'UTC').toISOString() + }); + + systemWideAlertDataService = jasmine.createSpyObj('systemWideAlertDataService', { + findAll: createSuccessfulRemoteDataObject$(createPaginatedList([systemWideAlert])), + }); + + TestBed.configureTestingModule({ + imports: [TranslateModule.forRoot()], + declarations: [SystemWideAlertBannerComponent], + providers: [ + {provide: SystemWideAlertDataService, useValue: systemWideAlertDataService}, + ] + }).compileComponents(); + })); + + beforeEach(() => { + fixture = TestBed.createComponent(SystemWideAlertBannerComponent); + comp = fixture.componentInstance; + fixture.detectChanges(); + }); + + describe('init', () => { + it('should init the comp', () => { + expect(comp).toBeTruthy(); + }); + it('should set the time countdown parts in their respective behaviour subjects', fakeAsync(() => { + spyOn(comp.countDownDays, 'next'); + spyOn(comp.countDownHours, 'next'); + spyOn(comp.countDownMinutes, 'next'); + comp.ngOnInit(); + tick(2000); + expect(comp.countDownDays.next).toHaveBeenCalled(); + expect(comp.countDownHours.next).toHaveBeenCalled(); + expect(comp.countDownMinutes.next).toHaveBeenCalled(); + discardPeriodicTasks(); + + })); + }); + + describe('banner', () => { + it('should display the alert message and the timer', () => { + comp.countDownDays.next(1); + comp.countDownHours.next(1); + comp.countDownMinutes.next(1); + fixture.detectChanges(); + + const banner = fixture.debugElement.queryAll(By.css('span')); + expect(banner.length).toEqual(6); + + expect(banner[0].nativeElement.innerHTML).toContain('system-wide-alert-banner.countdown.prefix'); + expect(banner[0].nativeElement.innerHTML).toContain('system-wide-alert-banner.countdown.days'); + expect(banner[0].nativeElement.innerHTML).toContain('system-wide-alert-banner.countdown.hours'); + expect(banner[0].nativeElement.innerHTML).toContain('system-wide-alert-banner.countdown.minutes'); + + expect(banner[5].nativeElement.innerHTML).toContain(systemWideAlert.message); + }); + + it('should display the alert message but no timer when no timer is present', () => { + comp.countDownDays.next(0); + comp.countDownHours.next(0); + comp.countDownMinutes.next(0); + fixture.detectChanges(); + + const banner = fixture.debugElement.queryAll(By.css('span')); + expect(banner.length).toEqual(2); + expect(banner[1].nativeElement.innerHTML).toContain(systemWideAlert.message); + }); + + it('should not display an alert when none is present', () => { + comp.systemWideAlert$.next(null); + fixture.detectChanges(); + + const banner = fixture.debugElement.queryAll(By.css('span')); + expect(banner.length).toEqual(0); + }); + }); +}); diff --git a/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.ts b/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.ts new file mode 100644 index 0000000000..b405957c54 --- /dev/null +++ b/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.ts @@ -0,0 +1,113 @@ +import { Component, OnDestroy, OnInit } from '@angular/core'; +import { SystemWideAlertDataService } from '../../core/data/system-wide-alert-data.service'; +import { getAllSucceededRemoteDataPayload } from '../../core/shared/operators'; +import { filter, map, switchMap } from 'rxjs/operators'; +import { PaginatedList } from '../../core/data/paginated-list.model'; +import { SystemWideAlert } from '../system-wide-alert.model'; +import { hasValue, isNotEmpty } from '../../shared/empty.util'; +import { BehaviorSubject, EMPTY, interval, Subscription } from 'rxjs'; +import { zonedTimeToUtc } from 'date-fns-tz'; + +/** + * Component responsible for rendering a banner and the countdown for an active system-wide alert + */ +@Component({ + selector: 'ds-system-wide-alert-banner', + styleUrls: ['./system-wide-alert-banner.component.scss'], + templateUrl: './system-wide-alert-banner.component.html' +}) +export class SystemWideAlertBannerComponent implements OnInit, OnDestroy { + + /** + * BehaviorSubject that keeps track of the currently configured system-wide alert + */ + systemWideAlert$ = new BehaviorSubject(undefined); + + /** + * BehaviorSubject that keeps track of the amount of minutes left to count down to + */ + countDownMinutes = new BehaviorSubject(0); + + /** + * BehaviorSubject that keeps track of the amount of hours left to count down to + */ + countDownHours = new BehaviorSubject(0); + + /** + * BehaviorSubject that keeps track of the amount of days left to count down to + */ + countDownDays = new BehaviorSubject(0); + + /** + * List of subscriptions + */ + subscriptions: Subscription[] = []; + + constructor( + protected systemWideAlertDataService: SystemWideAlertDataService + ) { + } + + ngOnInit() { + this.subscriptions.push(this.systemWideAlertDataService.findAll().pipe( + getAllSucceededRemoteDataPayload(), + map((payload: PaginatedList) => payload.page), + filter((page) => isNotEmpty(page)), + map((page) => page[0]) + ).subscribe((alert: SystemWideAlert) => { + this.systemWideAlert$.next(alert); + })); + + this.subscriptions.push(this.systemWideAlert$.pipe( + switchMap((alert: SystemWideAlert) => { + if (hasValue(alert) && hasValue(alert.countdownTo)) { + const date = zonedTimeToUtc(alert.countdownTo, 'UTC'); + const timeDifference = date.getTime() - new Date().getTime(); + if (timeDifference > 0) { + return interval(1000); + } + } + // Reset the countDown times to 0 and return EMPTY to prevent unnecessary countdown calculations + this.countDownDays.next(0); + this.countDownHours.next(0); + this.countDownMinutes.next(0); + return EMPTY; + }) + ).subscribe(() => { + this.setTimeDifference(this.systemWideAlert$.getValue().countdownTo); + })); + } + + /** + * Helper method to calculate the time difference between the countdown date from the system-wide alert and "now" + * @param countdownTo - The date to count down to + */ + private setTimeDifference(countdownTo: string) { + const date = zonedTimeToUtc(countdownTo, 'UTC'); + + const timeDifference = date.getTime() - new Date().getTime(); + this.allocateTimeUnits(timeDifference); + } + + /** + * Helper method to push how many days, hours and minutes are left in the countdown to their respective behaviour subject + * @param timeDifference - The time difference to calculate and push the time units for + */ + private allocateTimeUnits(timeDifference) { + const minutes = Math.floor((timeDifference) / (1000 * 60) % 60); + const hours = Math.floor((timeDifference) / (1000 * 60 * 60) % 24); + const days = Math.floor((timeDifference) / (1000 * 60 * 60 * 24)); + + this.countDownMinutes.next(minutes); + this.countDownHours.next(hours); + this.countDownDays.next(days); + } + + ngOnDestroy(): void { + this.subscriptions.forEach((sub: Subscription) => { + if (hasValue(sub)) { + sub.unsubscribe(); + } + }); + } +} diff --git a/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.html b/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.html new file mode 100644 index 0000000000..2821a14bf8 --- /dev/null +++ b/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.html @@ -0,0 +1,74 @@ +
+ +
+
+
+
+ +
+
+
+
+ + +
+ + {{ 'system-wide-alert.form.error.message' | translate }} + +
+
+
+
+
+
+
+ +
+
+
+
+
+ + +
+
+
+
+ +
+
+
+
+ +
+
+
+
+ {{'system-wide-alert.form.label.countdownTo.hint' | translate}} +
+ +
+ + +
+ +
diff --git a/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.scss b/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.scss new file mode 100644 index 0000000000..ce3e065caf --- /dev/null +++ b/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.scss @@ -0,0 +1,3 @@ +.timepicker-margin { + margin-top: -38px; +} diff --git a/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.spec.ts b/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.spec.ts new file mode 100644 index 0000000000..435aef3a7d --- /dev/null +++ b/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.spec.ts @@ -0,0 +1,219 @@ +import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing'; +import { SystemWideAlertDataService } from '../../core/data/system-wide-alert-data.service'; +import { SystemWideAlert } from '../system-wide-alert.model'; +import { utcToZonedTime, zonedTimeToUtc } from 'date-fns-tz'; +import { createFailedRemoteDataObject$, createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; +import { createPaginatedList } from '../../shared/testing/utils.test'; +import { TranslateModule } from '@ngx-translate/core'; +import { SystemWideAlertFormComponent } from './system-wide-alert-form.component'; +import { RequestService } from '../../core/data/request.service'; +import { NotificationsServiceStub } from '../../shared/testing/notifications-service.stub'; +import { RouterStub } from '../../shared/testing/router.stub'; +import { NotificationsService } from '../../shared/notifications/notifications.service'; +import { Router } from '@angular/router'; +import { FormsModule } from '@angular/forms'; +import { SharedModule } from '../../shared/shared.module'; +import { UiSwitchModule } from 'ngx-ui-switch'; + +describe('SystemWideAlertFormComponent', () => { + let comp: SystemWideAlertFormComponent; + let fixture: ComponentFixture; + let systemWideAlertDataService: SystemWideAlertDataService; + + let systemWideAlert: SystemWideAlert; + let requestService: RequestService; + let notificationsService; + let router; + + + beforeEach(waitForAsync(() => { + + const countDownDate = new Date(); + countDownDate.setDate(countDownDate.getDate() + 1); + countDownDate.setHours(countDownDate.getHours() + 1); + countDownDate.setMinutes(countDownDate.getMinutes() + 1); + + systemWideAlert = Object.assign(new SystemWideAlert(), { + alertId: 1, + message: 'Test alert message', + active: true, + countdownTo: utcToZonedTime(countDownDate, 'UTC').toISOString() + }); + + systemWideAlertDataService = jasmine.createSpyObj('systemWideAlertDataService', { + findAll: createSuccessfulRemoteDataObject$(createPaginatedList([systemWideAlert])), + put: createSuccessfulRemoteDataObject$(systemWideAlert), + create: createSuccessfulRemoteDataObject$(systemWideAlert) + }); + + requestService = jasmine.createSpyObj('requestService', ['setStaleByHrefSubstring']); + + notificationsService = new NotificationsServiceStub(); + router = new RouterStub(); + + TestBed.configureTestingModule({ + imports: [FormsModule, SharedModule, UiSwitchModule, TranslateModule.forRoot()], + declarations: [SystemWideAlertFormComponent], + providers: [ + {provide: SystemWideAlertDataService, useValue: systemWideAlertDataService}, + {provide: NotificationsService, useValue: notificationsService}, + {provide: Router, useValue: router}, + {provide: RequestService, useValue: requestService}, + ] + }).compileComponents(); + })); + + beforeEach(() => { + fixture = TestBed.createComponent(SystemWideAlertFormComponent); + comp = fixture.componentInstance; + + spyOn(comp, 'createForm').and.callThrough(); + spyOn(comp, 'initFormValues').and.callThrough(); + + fixture.detectChanges(); + }); + + describe('init', () => { + it('should init the comp', () => { + expect(comp).toBeTruthy(); + }); + it('should create the form and init the values based on an existing alert', () => { + expect(comp.createForm).toHaveBeenCalled(); + expect(comp.initFormValues).toHaveBeenCalledWith(systemWideAlert); + }); + }); + + describe('createForm', () => { + it('should create the form', () => { + const now = new Date(); + + comp.createForm(); + expect(comp.formMessage.value).toEqual(''); + expect(comp.formActive.value).toEqual(false); + expect(comp.time).toEqual({hour: now.getHours(), minute: now.getMinutes()}); + expect(comp.date).toEqual({year: now.getFullYear(), month: now.getMonth() + 1, day: now.getDate()}); + }); + }); + + describe('initFormValues', () => { + it('should fill in the form based on the provided system-wide alert', () => { + comp.initFormValues(systemWideAlert); + + const countDownTo = zonedTimeToUtc(systemWideAlert.countdownTo, 'UTC'); + + expect(comp.formMessage.value).toEqual(systemWideAlert.message); + expect(comp.formActive.value).toEqual(true); + expect(comp.time).toEqual({hour: countDownTo.getHours(), minute: countDownTo.getMinutes()}); + expect(comp.date).toEqual({ + year: countDownTo.getFullYear(), + month: countDownTo.getMonth() + 1, + day: countDownTo.getDate() + }); + }); + }); + describe('save', () => { + it('should update the exising alert with the form values and show a success notification on success', () => { + spyOn(comp, 'back'); + comp.currentAlert = systemWideAlert; + + comp.formMessage.patchValue('New message'); + comp.formActive.patchValue(true); + comp.time = {hour: 4, minute: 26}; + comp.date = {year: 2023, month: 1, day: 25}; + + const expectedAlert = new SystemWideAlert(); + expectedAlert.alertId = systemWideAlert.alertId; + expectedAlert.message = 'New message'; + expectedAlert.active = true; + const countDownTo = new Date(2023, 0, 25, 4, 26); + expectedAlert.countdownTo = utcToZonedTime(countDownTo, 'UTC').toUTCString(); + + comp.save(); + + expect(systemWideAlertDataService.put).toHaveBeenCalledWith(expectedAlert); + expect(notificationsService.success).toHaveBeenCalled(); + expect(requestService.setStaleByHrefSubstring).toHaveBeenCalledWith('systemwidealerts'); + expect(comp.back).toHaveBeenCalled(); + }); + it('should update the exising alert with the form values and show a error notification on error', () => { + spyOn(comp, 'back'); + (systemWideAlertDataService.put as jasmine.Spy).and.returnValue(createFailedRemoteDataObject$()); + comp.currentAlert = systemWideAlert; + + comp.formMessage.patchValue('New message'); + comp.formActive.patchValue(true); + comp.time = {hour: 4, minute: 26}; + comp.date = {year: 2023, month: 1, day: 25}; + + const expectedAlert = new SystemWideAlert(); + expectedAlert.alertId = systemWideAlert.alertId; + expectedAlert.message = 'New message'; + expectedAlert.active = true; + const countDownTo = new Date(2023, 0, 25, 4, 26); + expectedAlert.countdownTo = utcToZonedTime(countDownTo, 'UTC').toUTCString(); + + comp.save(); + + expect(systemWideAlertDataService.put).toHaveBeenCalledWith(expectedAlert); + expect(notificationsService.error).toHaveBeenCalled(); + expect(requestService.setStaleByHrefSubstring).not.toHaveBeenCalledWith('systemwidealerts'); + expect(comp.back).not.toHaveBeenCalled(); + }); + it('should create a new alert with the form values and show a success notification on success', () => { + spyOn(comp, 'back'); + comp.currentAlert = undefined; + + comp.formMessage.patchValue('New message'); + comp.formActive.patchValue(true); + comp.time = {hour: 4, minute: 26}; + comp.date = {year: 2023, month: 1, day: 25}; + + const expectedAlert = new SystemWideAlert(); + expectedAlert.message = 'New message'; + expectedAlert.active = true; + const countDownTo = new Date(2023, 0, 25, 4, 26); + expectedAlert.countdownTo = utcToZonedTime(countDownTo, 'UTC').toUTCString(); + + comp.save(); + + expect(systemWideAlertDataService.create).toHaveBeenCalledWith(expectedAlert); + expect(notificationsService.success).toHaveBeenCalled(); + expect(requestService.setStaleByHrefSubstring).toHaveBeenCalledWith('systemwidealerts'); + expect(comp.back).toHaveBeenCalled(); + + }); + it('should create a new alert with the form values and show a error notification on error', () => { + spyOn(comp, 'back'); + (systemWideAlertDataService.create as jasmine.Spy).and.returnValue(createFailedRemoteDataObject$()); + + comp.currentAlert = undefined; + + comp.formMessage.patchValue('New message'); + comp.formActive.patchValue(true); + comp.time = {hour: 4, minute: 26}; + comp.date = {year: 2023, month: 1, day: 25}; + + const expectedAlert = new SystemWideAlert(); + expectedAlert.message = 'New message'; + expectedAlert.active = true; + const countDownTo = new Date(2023, 0, 25, 4, 26); + expectedAlert.countdownTo = utcToZonedTime(countDownTo, 'UTC').toUTCString(); + + comp.save(); + + expect(systemWideAlertDataService.create).toHaveBeenCalledWith(expectedAlert); + expect(notificationsService.error).toHaveBeenCalled(); + expect(requestService.setStaleByHrefSubstring).not.toHaveBeenCalledWith('systemwidealerts'); + expect(comp.back).not.toHaveBeenCalled(); + + }); + }); + describe('back', () => { + it('should navigate back to the home page', () => { + comp.back(); + expect(router.navigate).toHaveBeenCalledWith(['/home']); + }); + }); + + +}); diff --git a/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.ts b/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.ts new file mode 100644 index 0000000000..59aa9166ce --- /dev/null +++ b/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.ts @@ -0,0 +1,163 @@ +import { Component, OnInit } from '@angular/core'; +import { SystemWideAlertDataService } from '../../core/data/system-wide-alert-data.service'; +import { getFirstCompletedRemoteData, getFirstSucceededRemoteDataPayload } from '../../core/shared/operators'; +import { filter, map } from 'rxjs/operators'; +import { PaginatedList } from '../../core/data/paginated-list.model'; +import { SystemWideAlert } from '../system-wide-alert.model'; +import { hasValue, isNotEmpty } from '../../shared/empty.util'; +import { Observable } from 'rxjs'; +import { FormBuilder, FormControl, FormGroup, Validators } from '@angular/forms'; +import { NgbDateStruct } from '@ng-bootstrap/ng-bootstrap'; +import { utcToZonedTime, zonedTimeToUtc } from 'date-fns-tz'; +import { RemoteData } from '../../core/data/remote-data'; +import { NotificationsService } from '../../shared/notifications/notifications.service'; +import { Router } from '@angular/router'; +import { RequestService } from '../../core/data/request.service'; +import { TranslateService } from '@ngx-translate/core'; + + +/** + * Component responsible for rendering the form to update a system-wide alert + */ +@Component({ + selector: 'ds-system-wide-alert-form', + styleUrls: ['./system-wide-alert-form.component.scss'], + templateUrl: './system-wide-alert-form.component.html' +}) +export class SystemWideAlertFormComponent implements OnInit { + + /** + * Observable to track an existing system-wide alert + */ + systemWideAlert$: Observable; + + /** + * The currently configured system-wide alert + */ + currentAlert: SystemWideAlert; + + /** + * The form group representing the system-wide alert + */ + alertForm: FormGroup; + + /** + * Date object to store the countdown date part + */ + date: NgbDateStruct; + + /** + * Object to store the countdown time part + */ + time; + + + constructor( + protected systemWideAlertDataService: SystemWideAlertDataService, + protected notificationsService: NotificationsService, + protected router: Router, + protected requestService: RequestService, + protected translateService: TranslateService + ) { + } + + ngOnInit() { + this.systemWideAlert$ = this.systemWideAlertDataService.findAll().pipe( + getFirstSucceededRemoteDataPayload(), + map((payload: PaginatedList) => payload.page), + filter((page) => isNotEmpty(page)), + map((page) => page[0]) + ); + this.createForm(); + + this.systemWideAlert$.subscribe((alert) => { + this.currentAlert = alert; + this.initFormValues(alert); + }); + } + + /** + * Creates the form with empty values + */ + createForm() { + this.alertForm = new FormBuilder().group({ + formMessage: new FormControl('', { + validators: [Validators.required], + }), + formActive: new FormControl(false), + } + ); + this.setDateTime(new Date()); + } + + /** + * Sets the form values based on the values retrieve from the provided system-wide alert + * @param alert - System-wide alert to use to init the form + */ + initFormValues(alert: SystemWideAlert) { + this.formMessage.patchValue(alert.message); + this.formActive.patchValue(alert.active); + const countDownTo = zonedTimeToUtc(alert.countdownTo, 'UTC'); + if (countDownTo.getTime() - new Date().getTime() > 0) { + this.setDateTime(countDownTo); + } + + } + + private setDateTime(dateToSet) { + this.time = {hour: dateToSet.getHours(), minute: dateToSet.getMinutes()}; + this.date = {year: dateToSet.getFullYear(), month: dateToSet.getMonth() + 1, day: dateToSet.getDate()}; + + } + + get formMessage() { + return this.alertForm.get('formMessage'); + } + + get formActive() { + return this.alertForm.get('formActive'); + } + + /** + * Save the system-wide alert present in the form + * When no alert is present yet on the server, a new one will be created + * When one already exists, the existing one will be updated + */ + save() { + const alert = new SystemWideAlert(); + alert.message = this.formMessage.value; + alert.active = this.formActive.value; + const countDownTo = new Date(this.date.year, this.date.month - 1, this.date.day, this.time.hour, this.time.minute); + alert.countdownTo = utcToZonedTime(countDownTo, 'UTC').toUTCString(); + + if (hasValue(this.currentAlert)) { + const updatedAlert = Object.assign(new SystemWideAlert(), this.currentAlert, alert); + this.handleResponse(this.systemWideAlertDataService.put(updatedAlert), 'system-wide-alert.form.update'); + } else { + this.handleResponse(this.systemWideAlertDataService.create(alert), 'system-wide-alert.form.create'); + } + } + + private handleResponse(response$: Observable>, messagePrefix) { + response$.pipe( + getFirstCompletedRemoteData() + ).subscribe((response: RemoteData) => { + if (response.hasSucceeded) { + this.notificationsService.success(this.translateService.get(`${messagePrefix}.success`)); + this.requestService.setStaleByHrefSubstring('systemwidealerts'); + this.back(); + } else { + this.notificationsService.error(this.translateService.get(`${messagePrefix}.error`, response.errorMessage)); + } + }); + } + + /** + * Navigate back to the homepage + */ + back() { + this.router.navigate(['/home']); + } + + +} diff --git a/src/app/system-wide-alert/system-wide-alert-routing.module.ts b/src/app/system-wide-alert/system-wide-alert-routing.module.ts new file mode 100644 index 0000000000..beb1b32187 --- /dev/null +++ b/src/app/system-wide-alert/system-wide-alert-routing.module.ts @@ -0,0 +1,22 @@ +import { NgModule } from '@angular/core'; +import { RouterModule } from '@angular/router'; +import { + SiteAdministratorGuard +} from '../core/data/feature-authorization/feature-authorization-guard/site-administrator.guard'; +import { SystemWideAlertFormComponent } from './alert-form/system-wide-alert-form.component'; + +@NgModule({ + imports: [ + RouterModule.forChild([ + { + path: '', + canActivate: [SiteAdministratorGuard], + component: SystemWideAlertFormComponent, + }, + + ]) + ] +}) +export class SystemWideAlertRoutingModule { + +} diff --git a/src/app/system-wide-alert/system-wide-alert.model.ts b/src/app/system-wide-alert/system-wide-alert.model.ts new file mode 100644 index 0000000000..158deb2603 --- /dev/null +++ b/src/app/system-wide-alert/system-wide-alert.model.ts @@ -0,0 +1,55 @@ +import { autoserialize, deserialize } from 'cerialize'; +import { typedObject } from '../core/cache/builders/build-decorators'; +import { CacheableObject } from '../core/cache/cacheable-object.model'; +import { HALLink } from '../core/shared/hal-link.model'; +import { ResourceType } from '../core/shared/resource-type'; +import { excludeFromEquals } from '../core/utilities/equals.decorators'; +import { SYSTEMWIDEALERT } from './system-wide-alert.resource-type'; + +/** + * Object representing a system-wide alert + */ +@typedObject +export class SystemWideAlert implements CacheableObject { + static type = SYSTEMWIDEALERT; + + /** + * The object type + */ + @excludeFromEquals + @autoserialize + type: ResourceType; + + /** + * The identifier for this system-wide alert + */ + @autoserialize + alertId: string; + + /** + * The message for this system-wide alert + */ + @autoserialize + message: string; + + /** + * A string representation of the date to which this system-wide alert will count down when active + */ + @autoserialize + countdownTo: string; + + /** + * Whether the system-wide alert is active + */ + @autoserialize + active: boolean; + + + /** + * The {@link HALLink}s for this system-wide alert + */ + @deserialize + _links: { + self: HALLink, + }; +} diff --git a/src/app/system-wide-alert/system-wide-alert.module.ts b/src/app/system-wide-alert/system-wide-alert.module.ts new file mode 100644 index 0000000000..ce2a87f982 --- /dev/null +++ b/src/app/system-wide-alert/system-wide-alert.module.ts @@ -0,0 +1,30 @@ +import { NgModule } from '@angular/core'; +import { FormsModule } from '@angular/forms'; +import { SystemWideAlertBannerComponent } from './alert-banner/system-wide-alert-banner.component'; +import { SystemWideAlertFormComponent } from './alert-form/system-wide-alert-form.component'; +import { SharedModule } from '../shared/shared.module'; +import { SystemWideAlertDataService } from '../core/data/system-wide-alert-data.service'; +import { SystemWideAlertRoutingModule } from './system-wide-alert-routing.module'; +import { UiSwitchModule } from 'ngx-ui-switch'; + +@NgModule({ + imports: [ + FormsModule, + SharedModule, + UiSwitchModule, + SystemWideAlertRoutingModule, + ], + exports: [ + SystemWideAlertBannerComponent + ], + declarations: [ + SystemWideAlertBannerComponent, + SystemWideAlertFormComponent + ], + providers: [ + SystemWideAlertDataService + ] +}) +export class SystemWideAlertModule { + +} diff --git a/src/app/system-wide-alert/system-wide-alert.resource-type.ts b/src/app/system-wide-alert/system-wide-alert.resource-type.ts new file mode 100644 index 0000000000..f67f00719b --- /dev/null +++ b/src/app/system-wide-alert/system-wide-alert.resource-type.ts @@ -0,0 +1,10 @@ +/** + * The resource type for SystemWideAlert + * + * Needs to be in a separate file to prevent circular + * dependencies in webpack. + */ + +import { ResourceType } from '../core/shared/resource-type'; + +export const SYSTEMWIDEALERT = new ResourceType('systemwidealert'); diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 597f226cc7..dd52b7dd9d 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -4811,4 +4811,47 @@ "person.orcid.registry.auth": "ORCID Authorizations", "home.recent-submissions.head": "Recent Submissions", + + + "system-wide-alert-banner.countdown.prefix": "In", + + "system-wide-alert-banner.countdown.days": "{{days}} day(s),", + + "system-wide-alert-banner.countdown.hours": "{{hours}} hour(s) and", + + "system-wide-alert-banner.countdown.minutes": "{{minutes}} minute(s):", + + + + "menu.section.system-wide-alert": "System-wide Alerts", + + "system-wide-alert.form.header": "System-wide Alerts", + + "system-wide-alert.form.cancel": "Cancel", + + "system-wide-alert.form.save": "Save", + + "system-wide-alert.form.label.active": "ACTIVE", + + "system-wide-alert.form.label.inactive": "INACTIVE", + + "system-wide-alert.form.error.message": "The system wide alert must have a message", + + "system-wide-alert.form.label.message": "Alert message", + + "system-wide-alert.form.label.countdownTo": "Count down", + + "system-wide-alert.form.label.countdownTo.hint": "Hint: When a date in the future is set, the system wide alert banner will perform a countdown to the set date. Setting the date to the current time or in the past will disable the countdown.", + + "system-wide-alert.form.update.success": "The system-wide alert was successfully updated", + + "system-wide-alert.form.update.error": "Something went wrong when updating the system-wide alert", + + "system-wide-alert.form.create.success": "The system-wide alert was successfully created", + + "system-wide-alert.form.create.error": "Something went wrong when creating the system-wide alert", + + "admin.system-wide-alert.breadcrumbs": "System-wide Alerts", + + "admin.system-wide-alert.title": "System-wide Alerts", } diff --git a/src/themes/custom/lazy-theme.module.ts b/src/themes/custom/lazy-theme.module.ts index d2ac0ae787..3aacac9c73 100644 --- a/src/themes/custom/lazy-theme.module.ts +++ b/src/themes/custom/lazy-theme.module.ts @@ -114,6 +114,7 @@ import { ObjectListComponent } from './app/shared/object-list/object-list.compon import { BrowseByMetadataPageComponent } from './app/browse-by/browse-by-metadata-page/browse-by-metadata-page.component'; import { BrowseByDatePageComponent } from './app/browse-by/browse-by-date-page/browse-by-date-page.component'; import { BrowseByTitlePageComponent } from './app/browse-by/browse-by-title-page/browse-by-title-page.component'; +import { SystemWideAlertModule } from '../../app/system-wide-alert/system-wide-alert.module'; const DECLARATIONS = [ FileSectionComponent, @@ -220,6 +221,7 @@ const DECLARATIONS = [ FormsModule, ResourcePoliciesModule, ComcolModule, + SystemWideAlertModule ], declarations: DECLARATIONS, exports: [ diff --git a/yarn.lock b/yarn.lock index 9542bfdbe1..2e2ce3140e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4759,6 +4759,16 @@ data-urls@^3.0.1: whatwg-mimetype "^3.0.0" whatwg-url "^10.0.0" +date-fns-tz@^1.3.7: + version "1.3.7" + resolved "https://registry.yarnpkg.com/date-fns-tz/-/date-fns-tz-1.3.7.tgz#e8e9d2aaceba5f1cc0e677631563081fdcb0e69a" + integrity sha512-1t1b8zyJo+UI8aR+g3iqr5fkUHWpd58VBx8J/ZSQ+w7YrGlw80Ag4sA86qkfCXRBLmMc4I2US+aPMd4uKvwj5g== + +date-fns@^2.29.3: + version "2.29.3" + resolved "https://registry.yarnpkg.com/date-fns/-/date-fns-2.29.3.tgz#27402d2fc67eb442b511b70bbdf98e6411cd68a8" + integrity sha512-dDCnyH2WnnKusqvZZ6+jA1O51Ibt8ZMRNkDZdyAyK4YfbDwa/cEmuztzG5pk6hqlp9aSBPYcjOlktquahGwGeA== + date-format@^4.0.4: version "4.0.4" resolved "https://registry.yarnpkg.com/date-format/-/date-format-4.0.4.tgz#b58036e29e74121fca3e1b3e0dc4a62c65faa233" From b6218494e548a4bdf11689ce39056df2007a2b1b Mon Sep 17 00:00:00 2001 From: Yana De Pauw Date: Fri, 23 Dec 2022 12:15:35 +0100 Subject: [PATCH 02/22] 97425: Fix countdown timer intial display and remove rounded banner corner --- .../alert-banner/system-wide-alert-banner.component.html | 2 +- .../alert-banner/system-wide-alert-banner.component.ts | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.html b/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.html index 3e360cc55e..5f741091f1 100644 --- a/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.html +++ b/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.html @@ -1,5 +1,5 @@
-
+
diff --git a/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.ts b/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.ts index b405957c54..57a9604f90 100644 --- a/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.ts +++ b/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.ts @@ -64,6 +64,7 @@ export class SystemWideAlertBannerComponent implements OnInit, OnDestroy { const date = zonedTimeToUtc(alert.countdownTo, 'UTC'); const timeDifference = date.getTime() - new Date().getTime(); if (timeDifference > 0) { + this.allocateTimeUnits(timeDifference); return interval(1000); } } From 827ab4d49ead977f23b4241bd7cb996269f4fe50 Mon Sep 17 00:00:00 2001 From: Yana De Pauw Date: Fri, 6 Jan 2023 10:07:23 +0100 Subject: [PATCH 03/22] Yarn lock change --- yarn.lock | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/yarn.lock b/yarn.lock index 68f37ebf57..e92c5bf954 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4333,20 +4333,15 @@ date-fns-tz@^1.3.7: resolved "https://registry.yarnpkg.com/date-fns-tz/-/date-fns-tz-1.3.7.tgz#e8e9d2aaceba5f1cc0e677631563081fdcb0e69a" integrity sha512-1t1b8zyJo+UI8aR+g3iqr5fkUHWpd58VBx8J/ZSQ+w7YrGlw80Ag4sA86qkfCXRBLmMc4I2US+aPMd4uKvwj5g== -date-fns-tz@^1.3.7: - version "1.3.7" - resolved "https://registry.yarnpkg.com/date-fns-tz/-/date-fns-tz-1.3.7.tgz#e8e9d2aaceba5f1cc0e677631563081fdcb0e69a" - integrity sha512-1t1b8zyJo+UI8aR+g3iqr5fkUHWpd58VBx8J/ZSQ+w7YrGlw80Ag4sA86qkfCXRBLmMc4I2US+aPMd4uKvwj5g== - date-fns@^2.29.3: version "2.29.3" resolved "https://registry.yarnpkg.com/date-fns/-/date-fns-2.29.3.tgz#27402d2fc67eb442b511b70bbdf98e6411cd68a8" integrity sha512-dDCnyH2WnnKusqvZZ6+jA1O51Ibt8ZMRNkDZdyAyK4YfbDwa/cEmuztzG5pk6hqlp9aSBPYcjOlktquahGwGeA== -date-format@^4.0.4: - version "4.0.4" - resolved "https://registry.yarnpkg.com/date-format/-/date-format-4.0.4.tgz#b58036e29e74121fca3e1b3e0dc4a62c65faa233" - integrity sha512-/jyf4rhB17ge328HJuJjAcmRtCsGd+NDeAtahRBTaK6vSPR6MO5HlrAit3Nn7dVjaa6sowW0WXt8yQtLyZQFRg== +date-format@^4.0.14: + version "4.0.14" + resolved "https://registry.yarnpkg.com/date-format/-/date-format-4.0.14.tgz#7a8e584434fb169a521c8b7aa481f355810d9400" + integrity sha512-39BOQLs9ZjKh0/patS9nrT8wc3ioX3/eA/zgbKNopnF2wCqJEoxywwwElATYvRsXdnOxA/OQeQoFZ3rFjVajhg== dayjs@^1.10.4: version "1.11.7" From 3e8cb4f3d94f4e9a6cde2a224754a76081deb270 Mon Sep 17 00:00:00 2001 From: Yana De Pauw Date: Mon, 23 Jan 2023 16:48:47 +0100 Subject: [PATCH 04/22] 97425: Implement feedback --- .../system-wide-alert-form.component.html | 91 +++++++++++++------ .../system-wide-alert-form.component.scss | 1 + .../system-wide-alert-form.component.spec.ts | 61 +++++++++++++ .../system-wide-alert-form.component.ts | 69 +++++++++++++- src/assets/i18n/en.json5 | 10 +- 5 files changed, 198 insertions(+), 34 deletions(-) diff --git a/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.html b/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.html index 2821a14bf8..062b06c294 100644 --- a/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.html +++ b/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.html @@ -17,50 +17,89 @@ [className]="(formMessage.invalid) && (formMessage.dirty || formMessage.touched) ? 'form-control is-invalid' :'form-control'" formControlName="formMessage"> -
+
{{ 'system-wide-alert.form.error.message' | translate }} -
+
-
- +
+ + {{ 'system-wide-alert.form.label.countdownTo.enable' | translate }}
-
-
-
- - +
+
+
+
+ + +
-
-
-
- +
+
+ +
-
-
-
- +
+
+ +
-
+
{{'system-wide-alert.form.label.countdownTo.hint' | translate}}
+ +
+
+
+ +
+
+
+ + + {{'system-wide-alert-banner.countdown.prefix' | translate }} + + + {{'system-wide-alert-banner.countdown.days' | translate: { + days: previewDays + } }} + + + {{'system-wide-alert-banner.countdown.hours' | translate: { + hours: previewHours + } }} + + + {{'system-wide-alert-banner.countdown.minutes' | translate: { + minutes: previewMinutes + } }} + + + {{formMessage.value}} +
+
+
-
+
From 715d3ae014a74f510be41a7eaedae1b9e986382f Mon Sep 17 00:00:00 2001 From: Yana De Pauw Date: Tue, 24 Jan 2023 11:24:13 +0100 Subject: [PATCH 06/22] Fix issues with module changes --- .../alert-form/system-wide-alert-form.component.spec.ts | 4 ++-- src/app/system-wide-alert/system-wide-alert.module.ts | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.spec.ts b/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.spec.ts index 608170a094..4fc79c1caa 100644 --- a/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.spec.ts +++ b/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.spec.ts @@ -12,8 +12,8 @@ import { RouterStub } from '../../shared/testing/router.stub'; import { NotificationsService } from '../../shared/notifications/notifications.service'; import { Router } from '@angular/router'; import { FormsModule } from '@angular/forms'; -import { SharedModule } from '../../shared/shared.module'; import { UiSwitchModule } from 'ngx-ui-switch'; +import { SystemWideAlertModule } from '../system-wide-alert.module'; describe('SystemWideAlertFormComponent', () => { let comp: SystemWideAlertFormComponent; @@ -52,7 +52,7 @@ describe('SystemWideAlertFormComponent', () => { router = new RouterStub(); TestBed.configureTestingModule({ - imports: [FormsModule, SharedModule, UiSwitchModule, TranslateModule.forRoot()], + imports: [FormsModule, SystemWideAlertModule, UiSwitchModule, TranslateModule.forRoot()], declarations: [SystemWideAlertFormComponent], providers: [ {provide: SystemWideAlertDataService, useValue: systemWideAlertDataService}, diff --git a/src/app/system-wide-alert/system-wide-alert.module.ts b/src/app/system-wide-alert/system-wide-alert.module.ts index ce2a87f982..ca200fa4f1 100644 --- a/src/app/system-wide-alert/system-wide-alert.module.ts +++ b/src/app/system-wide-alert/system-wide-alert.module.ts @@ -6,6 +6,7 @@ import { SharedModule } from '../shared/shared.module'; import { SystemWideAlertDataService } from '../core/data/system-wide-alert-data.service'; import { SystemWideAlertRoutingModule } from './system-wide-alert-routing.module'; import { UiSwitchModule } from 'ngx-ui-switch'; +import { NgbDatepickerModule, NgbTimepickerModule } from '@ng-bootstrap/ng-bootstrap'; @NgModule({ imports: [ @@ -13,6 +14,8 @@ import { UiSwitchModule } from 'ngx-ui-switch'; SharedModule, UiSwitchModule, SystemWideAlertRoutingModule, + NgbTimepickerModule, + NgbDatepickerModule, ], exports: [ SystemWideAlertBannerComponent From b4273fa734983b9b8c82748fefd62a5fd5793dfe Mon Sep 17 00:00:00 2001 From: Yana De Pauw Date: Thu, 26 Jan 2023 15:41:45 +0100 Subject: [PATCH 07/22] 97425: Implement feedback --- .../system-wide-alert-banner.component.ts | 11 ++++-- .../system-wide-alert-form.component.html | 3 +- .../system-wide-alert-form.component.spec.ts | 36 ++++++++++++++++++- .../system-wide-alert-form.component.ts | 33 ++++++++++++++--- 4 files changed, 74 insertions(+), 9 deletions(-) diff --git a/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.ts b/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.ts index 57a9604f90..a19d2a7e41 100644 --- a/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.ts +++ b/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.ts @@ -1,4 +1,4 @@ -import { Component, OnDestroy, OnInit } from '@angular/core'; +import { Component, Inject, OnDestroy, OnInit, PLATFORM_ID } from '@angular/core'; import { SystemWideAlertDataService } from '../../core/data/system-wide-alert-data.service'; import { getAllSucceededRemoteDataPayload } from '../../core/shared/operators'; import { filter, map, switchMap } from 'rxjs/operators'; @@ -7,6 +7,7 @@ import { SystemWideAlert } from '../system-wide-alert.model'; import { hasValue, isNotEmpty } from '../../shared/empty.util'; import { BehaviorSubject, EMPTY, interval, Subscription } from 'rxjs'; import { zonedTimeToUtc } from 'date-fns-tz'; +import { isPlatformBrowser } from '@angular/common'; /** * Component responsible for rendering a banner and the countdown for an active system-wide alert @@ -44,6 +45,7 @@ export class SystemWideAlertBannerComponent implements OnInit, OnDestroy { subscriptions: Subscription[] = []; constructor( + @Inject(PLATFORM_ID) protected platformId: Object, protected systemWideAlertDataService: SystemWideAlertDataService ) { } @@ -65,7 +67,12 @@ export class SystemWideAlertBannerComponent implements OnInit, OnDestroy { const timeDifference = date.getTime() - new Date().getTime(); if (timeDifference > 0) { this.allocateTimeUnits(timeDifference); - return interval(1000); + if (isPlatformBrowser(this.platformId)) { + return interval(1000); + } else { + return EMPTY; + } + } } // Reset the countDown times to 0 and return EMPTY to prevent unnecessary countdown calculations diff --git a/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.html b/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.html index d6eeaf5046..169081e277 100644 --- a/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.html +++ b/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.html @@ -7,7 +7,7 @@ + (change)="setActive($event)">
@@ -44,6 +44,7 @@ placeholder="yyyy-mm-dd" name="dp" [(ngModel)]="date" + [minDate]="minDate" ngbDatepicker #d="ngbDatepicker" (ngModelChange)="updatePreviewTime()" diff --git a/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.spec.ts b/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.spec.ts index 4fc79c1caa..505990b445 100644 --- a/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.spec.ts +++ b/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.spec.ts @@ -149,8 +149,19 @@ describe('SystemWideAlertFormComponent', () => { }); }); + describe('setActive', () => { + it('should set whether the alert is active and save the current alert', () => { + spyOn(comp, 'save'); + spyOn(comp.formActive, 'patchValue'); + comp.setActive(true); + + expect(comp.formActive.patchValue).toHaveBeenCalledWith(true); + expect(comp.save).toHaveBeenCalledWith(false); + }); + }); + describe('save', () => { - it('should update the exising alert with the form values and show a success notification on success', () => { + it('should update the exising alert with the form values and show a success notification on success and navigate back', () => { spyOn(comp, 'back'); comp.currentAlert = systemWideAlert; @@ -173,6 +184,29 @@ describe('SystemWideAlertFormComponent', () => { expect(requestService.setStaleByHrefSubstring).toHaveBeenCalledWith('systemwidealerts'); expect(comp.back).toHaveBeenCalled(); }); + it('should update the exising alert with the form values and show a success notification on success and not navigate back when false is provided to the save method', () => { + spyOn(comp, 'back'); + comp.currentAlert = systemWideAlert; + + comp.formMessage.patchValue('New message'); + comp.formActive.patchValue(true); + comp.time = {hour: 4, minute: 26}; + comp.date = {year: 2023, month: 1, day: 25}; + + const expectedAlert = new SystemWideAlert(); + expectedAlert.alertId = systemWideAlert.alertId; + expectedAlert.message = 'New message'; + expectedAlert.active = true; + const countDownTo = new Date(2023, 0, 25, 4, 26); + expectedAlert.countdownTo = utcToZonedTime(countDownTo, 'UTC').toUTCString(); + + comp.save(false); + + expect(systemWideAlertDataService.put).toHaveBeenCalledWith(expectedAlert); + expect(notificationsService.success).toHaveBeenCalled(); + expect(requestService.setStaleByHrefSubstring).toHaveBeenCalledWith('systemwidealerts'); + expect(comp.back).not.toHaveBeenCalled(); + }); it('should update the exising alert with the form values but add an empty countdown date when disabled and show a success notification on success', () => { spyOn(comp, 'back'); comp.currentAlert = systemWideAlert; diff --git a/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.ts b/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.ts index 6e0d2030fd..db517ef8cd 100644 --- a/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.ts +++ b/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.ts @@ -46,6 +46,11 @@ export class SystemWideAlertFormComponent implements OnInit { */ date: NgbDateStruct; + /** + * The minimum date for the countdown timer + */ + minDate: NgbDateStruct; + /** * Object to store the countdown time part */ @@ -90,6 +95,10 @@ export class SystemWideAlertFormComponent implements OnInit { ); this.createForm(); + const currentDate = new Date(); + this.minDate = {year: currentDate.getFullYear(), month: currentDate.getMonth() + 1, day: currentDate.getDate()}; + + this.systemWideAlert$.subscribe((alert) => { this.currentAlert = alert; this.initFormValues(alert); @@ -125,6 +134,16 @@ export class SystemWideAlertFormComponent implements OnInit { } + /** + * Set whether the system-wide alert is active + * Will also save the info in the current system-wide alert + * @param active + */ + setActive(active: boolean) { + this.formActive.patchValue(active); + this.save(false); + } + /** * Set whether the countdown timer is enabled or disabled. This will also update the counter in the preview * @param enabled - Whether the countdown timer is enabled or disabled. @@ -180,8 +199,10 @@ export class SystemWideAlertFormComponent implements OnInit { * Save the system-wide alert present in the form * When no alert is present yet on the server, a new one will be created * When one already exists, the existing one will be updated + * + * @param navigateToHomePage - Whether the user should be navigated back on successful save or not */ - save() { + save(navigateToHomePage = true) { const alert = new SystemWideAlert(); alert.message = this.formMessage.value; alert.active = this.formActive.value; @@ -193,20 +214,22 @@ export class SystemWideAlertFormComponent implements OnInit { } if (hasValue(this.currentAlert)) { const updatedAlert = Object.assign(new SystemWideAlert(), this.currentAlert, alert); - this.handleResponse(this.systemWideAlertDataService.put(updatedAlert), 'system-wide-alert.form.update'); + this.handleResponse(this.systemWideAlertDataService.put(updatedAlert), 'system-wide-alert.form.update', navigateToHomePage); } else { - this.handleResponse(this.systemWideAlertDataService.create(alert), 'system-wide-alert.form.create'); + this.handleResponse(this.systemWideAlertDataService.create(alert), 'system-wide-alert.form.create', navigateToHomePage); } } - private handleResponse(response$: Observable>, messagePrefix) { + private handleResponse(response$: Observable>, messagePrefix, navigateToHomePage: boolean) { response$.pipe( getFirstCompletedRemoteData() ).subscribe((response: RemoteData) => { if (response.hasSucceeded) { this.notificationsService.success(this.translateService.get(`${messagePrefix}.success`)); this.requestService.setStaleByHrefSubstring('systemwidealerts'); - this.back(); + if (navigateToHomePage) { + this.back(); + } } else { this.notificationsService.error(this.translateService.get(`${messagePrefix}.error`, response.errorMessage)); } From 38063d617e465db958efa6b96a3c35d826b4bc57 Mon Sep 17 00:00:00 2001 From: Yana De Pauw Date: Tue, 31 Jan 2023 13:06:03 +0100 Subject: [PATCH 08/22] 97425: Implement feedback --- .../data/system-wide-alert-data.service.ts | 23 ++++++++++++++++++- ...system-wide-alert-banner.component.spec.ts | 5 +++- .../system-wide-alert-banner.component.ts | 17 ++++++++++---- src/assets/i18n/en.json5 | 2 ++ 4 files changed, 41 insertions(+), 6 deletions(-) diff --git a/src/app/core/data/system-wide-alert-data.service.ts b/src/app/core/data/system-wide-alert-data.service.ts index 029ab76693..51a88b907a 100644 --- a/src/app/core/data/system-wide-alert-data.service.ts +++ b/src/app/core/data/system-wide-alert-data.service.ts @@ -17,16 +17,18 @@ import { SYSTEMWIDEALERT } from '../../system-wide-alert/system-wide-alert.resou import { SystemWideAlert } from '../../system-wide-alert/system-wide-alert.model'; import { PutData, PutDataImpl } from './base/put-data'; import { RequestParam } from '../cache/models/request-param.model'; +import { SearchData, SearchDataImpl } from './base/search-data'; /** * Dataservice representing a system-wide alert */ @Injectable() @dataService(SYSTEMWIDEALERT) -export class SystemWideAlertDataService extends IdentifiableDataService implements FindAllData, CreateData, PutData { +export class SystemWideAlertDataService extends IdentifiableDataService implements FindAllData, CreateData, PutData, SearchData { private findAllData: FindAllDataImpl; private createData: CreateDataImpl; private putData: PutDataImpl; + private searchData: SearchData; constructor( protected requestService: RequestService, @@ -40,6 +42,7 @@ export class SystemWideAlertDataService extends IdentifiableDataService>} + * Return an observable that emits response from the server + */ + searchBy(searchMethod: string, options?: FindListOptions, useCachedVersionIfAvailable?: boolean, reRequestOnStale?: boolean, ...linksToFollow: FollowLinkConfig[]): Observable>> { + return this.searchData.searchBy(searchMethod, options, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow); + } + } diff --git a/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.spec.ts b/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.spec.ts index d27e5379e9..f767d0f196 100644 --- a/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.spec.ts +++ b/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.spec.ts @@ -9,6 +9,8 @@ import { TestScheduler } from 'rxjs/testing'; import { getTestScheduler } from 'jasmine-marbles'; import { By } from '@angular/platform-browser'; import { TranslateModule } from '@ngx-translate/core'; +import { NotificationsService } from '../../shared/notifications/notifications.service'; +import { NotificationsServiceStub } from '../../shared/testing/notifications-service.stub'; describe('SystemWideAlertBannerComponent', () => { @@ -35,7 +37,7 @@ describe('SystemWideAlertBannerComponent', () => { }); systemWideAlertDataService = jasmine.createSpyObj('systemWideAlertDataService', { - findAll: createSuccessfulRemoteDataObject$(createPaginatedList([systemWideAlert])), + searchBy: createSuccessfulRemoteDataObject$(createPaginatedList([systemWideAlert])), }); TestBed.configureTestingModule({ @@ -43,6 +45,7 @@ describe('SystemWideAlertBannerComponent', () => { declarations: [SystemWideAlertBannerComponent], providers: [ {provide: SystemWideAlertDataService, useValue: systemWideAlertDataService}, + {provide: NotificationsService, useValue: new NotificationsServiceStub()}, ] }).compileComponents(); })); diff --git a/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.ts b/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.ts index a19d2a7e41..320e42db85 100644 --- a/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.ts +++ b/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.ts @@ -1,6 +1,6 @@ import { Component, Inject, OnDestroy, OnInit, PLATFORM_ID } from '@angular/core'; import { SystemWideAlertDataService } from '../../core/data/system-wide-alert-data.service'; -import { getAllSucceededRemoteDataPayload } from '../../core/shared/operators'; +import { getAllCompletedRemoteData } from '../../core/shared/operators'; import { filter, map, switchMap } from 'rxjs/operators'; import { PaginatedList } from '../../core/data/paginated-list.model'; import { SystemWideAlert } from '../system-wide-alert.model'; @@ -8,6 +8,7 @@ import { hasValue, isNotEmpty } from '../../shared/empty.util'; import { BehaviorSubject, EMPTY, interval, Subscription } from 'rxjs'; import { zonedTimeToUtc } from 'date-fns-tz'; import { isPlatformBrowser } from '@angular/common'; +import { NotificationsService } from '../../shared/notifications/notifications.service'; /** * Component responsible for rendering a banner and the countdown for an active system-wide alert @@ -46,13 +47,21 @@ export class SystemWideAlertBannerComponent implements OnInit, OnDestroy { constructor( @Inject(PLATFORM_ID) protected platformId: Object, - protected systemWideAlertDataService: SystemWideAlertDataService + protected systemWideAlertDataService: SystemWideAlertDataService, + protected notificationsService: NotificationsService, ) { } ngOnInit() { - this.subscriptions.push(this.systemWideAlertDataService.findAll().pipe( - getAllSucceededRemoteDataPayload(), + this.subscriptions.push(this.systemWideAlertDataService.searchBy('active').pipe( + getAllCompletedRemoteData(), + map((rd) => { + if (rd.hasSucceeded) { + return rd.payload; + } else { + this.notificationsService.error('system-wide-alert-banner.retrieval.error'); + } + }), map((payload: PaginatedList) => payload.page), filter((page) => isNotEmpty(page)), map((page) => page[0]) diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 26844bb1d6..056fc593d8 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -4844,6 +4844,8 @@ "listable-notification-object.default-message": "This object couldn't be retrieved", + "system-wide-alert-banner.retrieval.error": "Something went wrong retrieving the system-wide alert banner", + "system-wide-alert-banner.countdown.prefix": "In", "system-wide-alert-banner.countdown.days": "{{days}} day(s),", From 36e10f87b3e29c6f3a67abf3314b2d413292ee14 Mon Sep 17 00:00:00 2001 From: Yana De Pauw Date: Thu, 2 Feb 2023 16:22:59 +0100 Subject: [PATCH 09/22] 97425: Fix minor issues --- .../system-wide-alert-banner.component.ts | 13 ++++--------- .../alert-form/system-wide-alert-form.component.ts | 11 +++++++++-- src/assets/i18n/en.json5 | 2 ++ 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.ts b/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.ts index 320e42db85..27bdb14f1d 100644 --- a/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.ts +++ b/src/app/system-wide-alert/alert-banner/system-wide-alert-banner.component.ts @@ -1,6 +1,8 @@ import { Component, Inject, OnDestroy, OnInit, PLATFORM_ID } from '@angular/core'; import { SystemWideAlertDataService } from '../../core/data/system-wide-alert-data.service'; -import { getAllCompletedRemoteData } from '../../core/shared/operators'; +import { + getAllSucceededRemoteDataPayload +} from '../../core/shared/operators'; import { filter, map, switchMap } from 'rxjs/operators'; import { PaginatedList } from '../../core/data/paginated-list.model'; import { SystemWideAlert } from '../system-wide-alert.model'; @@ -54,14 +56,7 @@ export class SystemWideAlertBannerComponent implements OnInit, OnDestroy { ngOnInit() { this.subscriptions.push(this.systemWideAlertDataService.searchBy('active').pipe( - getAllCompletedRemoteData(), - map((rd) => { - if (rd.hasSucceeded) { - return rd.payload; - } else { - this.notificationsService.error('system-wide-alert-banner.retrieval.error'); - } - }), + getAllSucceededRemoteDataPayload(), map((payload: PaginatedList) => payload.page), filter((page) => isNotEmpty(page)), map((page) => page[0]) diff --git a/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.ts b/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.ts index db517ef8cd..23c9fb75f9 100644 --- a/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.ts +++ b/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.ts @@ -1,6 +1,6 @@ import { Component, OnInit } from '@angular/core'; import { SystemWideAlertDataService } from '../../core/data/system-wide-alert-data.service'; -import { getFirstCompletedRemoteData, getFirstSucceededRemoteDataPayload } from '../../core/shared/operators'; +import { getFirstCompletedRemoteData } from '../../core/shared/operators'; import { filter, map } from 'rxjs/operators'; import { PaginatedList } from '../../core/data/paginated-list.model'; import { SystemWideAlert } from '../system-wide-alert.model'; @@ -88,7 +88,14 @@ export class SystemWideAlertFormComponent implements OnInit { ngOnInit() { this.systemWideAlert$ = this.systemWideAlertDataService.findAll().pipe( - getFirstSucceededRemoteDataPayload(), + getFirstCompletedRemoteData(), + map((rd) => { + if (rd.hasSucceeded) { + return rd.payload; + } else { + this.notificationsService.error('system-wide-alert-form.retrieval.error'); + } + }), map((payload: PaginatedList) => payload.page), filter((page) => isNotEmpty(page)), map((page) => page[0]) diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 056fc593d8..60bbb9e860 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -4860,6 +4860,8 @@ "system-wide-alert.form.header": "System-wide Alert", + "system-wide-alert-form.retrieval.error": "Something went wrong retrieving the system-wide alert", + "system-wide-alert.form.cancel": "Cancel", "system-wide-alert.form.save": "Save", From bb7eef263196e6cb5d3c578d6687b39b3871bd53 Mon Sep 17 00:00:00 2001 From: Yana De Pauw Date: Thu, 2 Feb 2023 17:58:55 +0100 Subject: [PATCH 10/22] Fix typo --- .../alert-form/system-wide-alert-form.component.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.scss b/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.scss index 6e881e4807..cdcfd8b0cd 100644 --- a/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.scss +++ b/src/app/system-wide-alert/alert-form/system-wide-alert-form.component.scss @@ -1,4 +1,4 @@ .timepicker-margin { - // Negative margin to offset the time picker arrowsand ensure the date and time are correctly aligned + // Negative margin to offset the time picker arrows and ensure the date and time are correctly aligned margin-top: -38px; } From b0696a404d3abf37c35c72898133cb06d6bb87ee Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Mon, 9 Jan 2023 13:59:02 -0600 Subject: [PATCH 11/22] Add SSR caching via lru-cache. Update Cache-Control header to 1 week, but tell browsers not to cache index.html --- config/config.example.yml | 7 + package.json | 1 + server.ts | 241 +++++++++++++++++++++------ src/config/cache-config.interface.ts | 8 + src/config/default-app-config.ts | 8 + src/index.html | 1 + yarn.lock | 2 +- 7 files changed, 217 insertions(+), 51 deletions(-) diff --git a/config/config.example.yml b/config/config.example.yml index af04859201..28ba095098 100644 --- a/config/config.example.yml +++ b/config/config.example.yml @@ -32,12 +32,19 @@ cache: # NOTE: how long should objects be cached for by default msToLive: default: 900000 # 15 minutes + # Cache-Control HTTP Header control: max-age=60 # revalidate browser autoSync: defaultTime: 0 maxBufferSize: 100 timePerMethod: PATCH: 3 # time in seconds + # In-memory cache of server-side rendered content + serverSide: + # Maximum number of pages (rendered via SSR) to cache. Set to zero to disable server side caching. + max: 100 + # Amount of time after which cached pages are considered stale (in ms) + timeToLive: 900000 # 15 minutes # Authentication settings auth: diff --git a/package.json b/package.json index dcb629a331..945bb1f158 100644 --- a/package.json +++ b/package.json @@ -106,6 +106,7 @@ "jwt-decode": "^3.1.2", "klaro": "^0.7.18", "lodash": "^4.17.21", + "lru-cache": "^7.14.1", "markdown-it": "^13.0.1", "markdown-it-mathjax3": "^4.3.1", "mirador": "^3.3.0", diff --git a/server.ts b/server.ts index ecbbb982d4..c5905e0ccf 100644 --- a/server.ts +++ b/server.ts @@ -28,6 +28,7 @@ import * as expressStaticGzip from 'express-static-gzip'; /* eslint-enable import/no-namespace */ import axios from 'axios'; +import LRU from 'lru-cache'; import { createCertificate } from 'pem'; import { createServer } from 'https'; import { json } from 'body-parser'; @@ -54,6 +55,7 @@ import { APP_CONFIG, AppConfig } from './src/config/app-config.interface'; import { extendEnvironmentWithAppConfig } from './src/config/config.util'; import { logStartupMessage } from './startup-message'; + /* * Set path for the browser application's dist folder */ @@ -67,6 +69,9 @@ const cookieParser = require('cookie-parser'); const appConfig: AppConfig = buildAppConfig(join(DIST_FOLDER, 'assets/config.json')); +// cache of SSR pages, only enabled in production mode +let cache: LRU; + // extend environment with app config for server extendEnvironmentWithAppConfig(environment, appConfig); @@ -87,10 +92,12 @@ export function app() { /* * If production mode is enabled in the environment file: * - Enable Angular's production mode + * - Enable caching of SSR rendered pages (if enabled in config.yml) * - Enable compression for SSR reponses. See [compression](https://github.com/expressjs/compression) */ if (environment.production) { enableProdMode(); + enableCache(); server.use(compression({ // only compress responses we've marked as SSR // otherwise, this middleware may compress files we've chosen not to compress via compression-webpack-plugin @@ -186,7 +193,7 @@ export function app() { * Serve static resources (images, i18n messages, …) * Handle pre-compressed files with [express-static-gzip](https://github.com/tkoenig89/express-static-gzip) */ - router.get('*.*', cacheControl, expressStaticGzip(DIST_FOLDER, { + router.get('*.*', addCacheControl, expressStaticGzip(DIST_FOLDER, { index: false, enableBrotli: true, orderPreference: ['br', 'gzip'], @@ -202,8 +209,11 @@ export function app() { */ server.get('/app/health', healthCheck); - // Register the ngApp callback function to handle incoming requests - router.get('*', ngApp); + /** + * Default sending all incoming requests to ngApp() function, after first checking for a cached + * copy of the page (see cacheCheck()) + */ + router.get('*', cacheCheck, ngApp); server.use(environment.ui.nameSpace, router); @@ -215,60 +225,191 @@ export function app() { */ function ngApp(req, res) { if (environment.universal.preboot) { - res.render(indexHtml, { - req, - res, - preboot: environment.universal.preboot, - async: environment.universal.async, - time: environment.universal.time, - baseUrl: environment.ui.nameSpace, - originUrl: environment.ui.baseUrl, - requestUrl: req.originalUrl, - providers: [{ provide: APP_BASE_HREF, useValue: req.baseUrl }] - }, (err, data) => { - if (hasNoValue(err) && hasValue(data)) { - res.locals.ssr = true; // mark response as SSR - res.send(data); - } else if (hasValue(err) && err.code === 'ERR_HTTP_HEADERS_SENT') { - // When this error occurs we can't fall back to CSR because the response has already been - // sent. These errors occur for various reasons in universal, not all of which are in our - // control to solve. - console.warn('Warning [ERR_HTTP_HEADERS_SENT]: Tried to set headers after they were sent to the client'); - } else { - console.warn('Error in SSR, serving for direct CSR.'); - if (hasValue(err)) { - console.warn('Error details : ', err); - } - res.render(indexHtml, { - req, - providers: [{ - provide: APP_BASE_HREF, - useValue: req.baseUrl - }] - }); - } - }); + // Render the page to user via SSR (server side rendering) + serverSideRender(req, res); } else { // If preboot is disabled, just serve the client - console.log('Universal off, serving for direct CSR'); - res.render(indexHtml, { - req, - providers: [{ - provide: APP_BASE_HREF, - useValue: req.baseUrl - }] + console.log('Universal off, serving for direct client-side rendering (CSR)'); + clientSideRender(req, res); + } +} + +/** + * Render page content on server side using Angular SSR. By default this page content is + * returned to the user. + * @param req current request + * @param res current response + * @param sendToUser if true (default), send the rendered content to the user. + * If false, then only save this rendered content to the in-memory cache (to refresh cache). + */ +function serverSideRender(req, res, sendToUser: boolean = true) { + // Render the page via SSR (server side rendering) + res.render(indexHtml, { + req, + res, + preboot: environment.universal.preboot, + async: environment.universal.async, + time: environment.universal.time, + baseUrl: environment.ui.nameSpace, + originUrl: environment.ui.baseUrl, + requestUrl: req.originalUrl, + providers: [{ provide: APP_BASE_HREF, useValue: req.baseUrl }] + }, (err, data) => { + if (hasNoValue(err) && hasValue(data)) { + res.locals.ssr = true; // mark response as SSR (enables text compression) + // save server side rendered data to cache + saveToCache(getCacheKey(req), data); + if (sendToUser) { + // send rendered page to user + res.send(data); + } + } else if (hasValue(err) && err.code === 'ERR_HTTP_HEADERS_SENT') { + // When this error occurs we can't fall back to CSR because the response has already been + // sent. These errors occur for various reasons in universal, not all of which are in our + // control to solve. + console.warn('Warning [ERR_HTTP_HEADERS_SENT]: Tried to set headers after they were sent to the client'); + } else { + console.warn('Error in server-side rendering (SSR)'); + if (hasValue(err)) { + console.warn('Error details : ', err); + } + if (sendToUser) { + console.warn('Falling back to serving direct client-side rendering (CSR).'); + clientSideRender(req, res); + } + } + }); +} + +/** + * Send back response to user to trigger direct client-side rendering (CSR) + * @param req current request + * @param res current response + */ +function clientSideRender(req, res) { + res.render(indexHtml, { + req, + providers: [{ + provide: APP_BASE_HREF, + useValue: req.baseUrl + }] + }); +} + + +/* + * Adds a Cache-Control HTTP header to the response. + * The cache control value can be configured in the config.*.yml file + * Defaults to max-age=604,800 seconds (1 week) + */ +function addCacheControl(req, res, next) { + // instruct browser to revalidate + res.header('Cache-Control', environment.cache.control || 'max-age=604800'); + next(); +} + +/* + * Enable server-side caching of pages rendered via SSR. + */ +function enableCache() { + if (cacheEnabled()) { + // Initialize a new "least-recently-used" item cache. + // See https://www.npmjs.com/package/lru-cache + cache = new LRU( { + max: environment.cache.serverSide.max || 100, // 100 items in cache maximum + ttl: environment.cache.serverSide.timeToLive || 15 * 60 * 1000, // 15 minute cache + allowStale: true // If object is found to be stale, return stale value before deleting }); } } -/* - * Adds a cache control header to the response - * The cache control value can be configured in the environments file and defaults to max-age=60 +/** + * Return whether server side caching is enabled in configuration. */ -function cacheControl(req, res, next) { - // instruct browser to revalidate - res.header('Cache-Control', environment.cache.control || 'max-age=60'); - next(); +function cacheEnabled(): boolean { + // Caching is only enabled is SSR is enabled AND + // "serverSide.max" setting is greater than zero + return environment.universal.preboot && environment.cache.serverSide.max && (environment.cache.serverSide.max > 0); +} + +/** + * Check if the currently requested page is in our server-side, in-memory cache. + * Caching is ONLY done for SSR requests. Pages are cached base on their path (e.g. /home or /search?query=test) + */ +function cacheCheck(req, res, next) { + let cacheHit = false; + let debug = false; // Enable to see cache hits & re-rendering logs + + // Only check cache if cache enabled & SSR is enabled + if (cacheEnabled()) { + const key = getCacheKey(req); + + // Check if this page is in our cache + let cachedCopy = cache.get(key); + if (cachedCopy) { + cacheHit = true; + res.locals.ssr = true; // mark response as SSR (enables text compression) + if (debug) { console.log(`CACHE HIT FOR ${key}`); } + // return page from cache to user + res.send(cachedCopy); + + // Check if cached copy is expired (in this sitution key will now be gone from cache) + if (!cache.has(key)) { + if (debug) { console.log(`CACHE EXPIRED FOR ${key} Re-rendering...`); } + // Update cached copy by rerendering server-side + // NOTE: Cached copy was already returned to user above. So, this re-render is just to prepare for next user. + serverSideRender(req, res, false); + } + + // Tell Express to skip all other handlers for this path + // This ensures we don't try to re-render the page since we've already returned the cached copy + next('router'); + } + } + + // If nothing found in cache, just continue with next handler + // (This should send the request on to the handler that rerenders the page via SSR) + if (!cacheHit) { + next(); + } +} + +/** + * Create a cache key from the current request. + * The cache key is the URL path (NOTE: this key will also include any querystring params). + * E.g. "/home" or "/search?query=test" + * @param req current request + * @returns cache key to use for this page + */ +function getCacheKey(req): string { + // NOTE: this will return the URL path *without* any baseUrl + return req.url; +} + +/** + * Save data to server side cache, if enabled. If caching is not enabled, this is a noop + * @param key page key + * @param data page data to save to cache + */ +function saveToCache(key: string, data: any) { + // Only cache if caching is enabled and this path is allowed to be cached + if (cacheEnabled() && canCachePage(key)) { + cache.set(key, data); + } +} + +/** + * Whether this path is allowed to be cached. Only public paths can be cached as the cache is shared across all users. + * @param key page key (corresponds to path of page) + * @returns true if allowed to be cached, false otherwise. + */ +function canCachePage(key: string) { + // Only these publicly accessible pages can be cached. + // NOTE: Caching pages which require authentication is NOT ALLOWED. The same cache is used by all users & user-specific data must NEVER appear in cache. + const allowedPages = [/^\/$/, /^\/home$/, /^\/items\//, /^\/entities\//, /^\/collections\//, /^\/communities\//, /^\/search[\/?]?/, /\/browse\//, /^\/community-list$/]; + + // Check whether any of these regexs match with the passed in key + return allowedPages.some(regex => regex.test(key)); } /* diff --git a/src/config/cache-config.interface.ts b/src/config/cache-config.interface.ts index c535a96bb5..d0dfc677d9 100644 --- a/src/config/cache-config.interface.ts +++ b/src/config/cache-config.interface.ts @@ -5,6 +5,14 @@ export interface CacheConfig extends Config { msToLive: { default: number; }; + // Cache-Control HTTP Header control: string; autoSync: AutoSyncConfig; + // In-memory cache of server-side rendered content + serverSide: { + // Maximum number of pages (rendered via SSR) to cache. + max: number; + // Amount of time after which cached pages are considered stale (in ms) + timeToLive: number; + } } diff --git a/src/config/default-app-config.ts b/src/config/default-app-config.ts index 205ea8acc0..516d0eca2e 100644 --- a/src/config/default-app-config.ts +++ b/src/config/default-app-config.ts @@ -67,11 +67,19 @@ export class DefaultAppConfig implements AppConfig { msToLive: { default: 15 * 60 * 1000 // 15 minutes }, + // Cache-Control HTTP Header control: 'max-age=60', // revalidate browser autoSync: { defaultTime: 0, maxBufferSize: 100, timePerMethod: { [RestRequestMethod.PATCH]: 3 } as any // time in seconds + }, + // In-memory cache of server-side rendered content + serverSide: { + // Maximum number of pages (rendered via SSR) to cache. Set to zero to disable server side caching. + max: 100, + // Amount of time after which cached pages are considered stale (in ms) + timeToLive: 15 * 60 * 1000 // 15 minutes } }; diff --git a/src/index.html b/src/index.html index ddd448f289..565fc0439d 100644 --- a/src/index.html +++ b/src/index.html @@ -6,6 +6,7 @@ DSpace + diff --git a/yarn.lock b/yarn.lock index 2bbabcd654..96693299dc 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7468,7 +7468,7 @@ lru-cache@^6.0.0: dependencies: yallist "^4.0.0" -lru-cache@^7.7.1: +lru-cache@^7.14.1, lru-cache@^7.7.1: version "7.14.1" resolved "https://registry.yarnpkg.com/lru-cache/-/lru-cache-7.14.1.tgz#8da8d2f5f59827edb388e63e459ac23d6d408fea" integrity sha512-ysxwsnTKdAx96aTRdhDOCQfDgbHnt8SK0KY8SEjO0wHinhWOFTESbjVCMPbU1uGXg/ch4lifqx0wfjOawU2+WA== From 0d4cf5e4689af0916b2ba53ec0b33a76e44a8b21 Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Mon, 9 Jan 2023 16:20:06 -0600 Subject: [PATCH 12/22] Update SSR caching to only work when unauthenticated. Enhance config comments --- config/config.example.yml | 11 +++++++--- server.ts | 46 +++++++++++++++++++-------------------- 2 files changed, 31 insertions(+), 26 deletions(-) diff --git a/config/config.example.yml b/config/config.example.yml index 28ba095098..ec1087525c 100644 --- a/config/config.example.yml +++ b/config/config.example.yml @@ -39,11 +39,16 @@ cache: maxBufferSize: 100 timePerMethod: PATCH: 3 # time in seconds - # In-memory cache of server-side rendered content + # In-memory cache of server-side rendered pages. This cache stores the most recently accessed public pages. + # Pages are automatically added/dropped from this cache based on how recently they have been used. serverSide: - # Maximum number of pages (rendered via SSR) to cache. Set to zero to disable server side caching. + # Maximum number of pages to cache. Set to zero (0) to disable server side caching. Default is 100, which means + # the 100 most recently accessed public pages will be cached. As all pages are cached in server memory, + # increasing this value will increase memory needs. Individual cached pages are usually small (<100KB), + # so max=100 should only require a maximum of 9-10MB of memory. Restarting the app clears this page cache. max: 100 - # Amount of time after which cached pages are considered stale (in ms) + # Amount of time after which cached pages are considered stale (in ms). After becoming stale, the cached + # copy is automatically refreshed on the next request. timeToLive: 900000 # 15 minutes # Authentication settings diff --git a/server.ts b/server.ts index c5905e0ccf..3417cb3b96 100644 --- a/server.ts +++ b/server.ts @@ -54,6 +54,8 @@ import { buildAppConfig } from './src/config/config.server'; import { APP_CONFIG, AppConfig } from './src/config/app-config.interface'; import { extendEnvironmentWithAppConfig } from './src/config/config.util'; import { logStartupMessage } from './startup-message'; +import { TOKENITEM } from 'src/app/core/auth/models/auth-token-info.model'; +import { isAuthenticated } from 'src/app/core/auth/selectors'; /* @@ -113,13 +115,13 @@ export function app() { /* * Add cookie parser middleware - * See [morgan](https://github.com/expressjs/cookie-parser) + * See [cookie-parser](https://github.com/expressjs/cookie-parser) */ server.use(cookieParser()); /* - * Add parser for request bodies - * See [morgan](https://github.com/expressjs/body-parser) + * Add JSON parser for request bodies + * See [body-parser](https://github.com/expressjs/body-parser) */ server.use(json()); @@ -258,7 +260,7 @@ function serverSideRender(req, res, sendToUser: boolean = true) { if (hasNoValue(err) && hasValue(data)) { res.locals.ssr = true; // mark response as SSR (enables text compression) // save server side rendered data to cache - saveToCache(getCacheKey(req), data); + saveToCache(req, data); if (sendToUser) { // send rendered page to user res.send(data); @@ -313,7 +315,7 @@ function addCacheControl(req, res, next) { */ function enableCache() { if (cacheEnabled()) { - // Initialize a new "least-recently-used" item cache. + // Initialize a new "least-recently-used" item cache (where least recently used items are removed first) // See https://www.npmjs.com/package/lru-cache cache = new LRU( { max: environment.cache.serverSide.max || 100, // 100 items in cache maximum @@ -340,8 +342,10 @@ function cacheCheck(req, res, next) { let cacheHit = false; let debug = false; // Enable to see cache hits & re-rendering logs - // Only check cache if cache enabled & SSR is enabled - if (cacheEnabled()) { + // Only check cache if cache enabled & NOT authenticated. + // NOTE: Authenticated users cannot use the SSR cache. Cached pages only show data available to anonymous users. + // Only public pages can currently be cached, as the cached data is not user-specific. + if (cacheEnabled() && !isUserAuthenticated(req)) { const key = getCacheKey(req); // Check if this page is in our cache @@ -387,29 +391,25 @@ function getCacheKey(req): string { } /** - * Save data to server side cache, if enabled. If caching is not enabled, this is a noop - * @param key page key + * Save data to server side cache, if enabled. If caching is not enabled or user is authenticated, this is a noop + * @param req current page request * @param data page data to save to cache */ -function saveToCache(key: string, data: any) { - // Only cache if caching is enabled and this path is allowed to be cached - if (cacheEnabled() && canCachePage(key)) { - cache.set(key, data); +function saveToCache(req, data: any) { + // Only cache if caching is enabled and no one is currently authenticated. This means ONLY public pages can be cached. + // NOTE: It's not safe to save page data to the cache when a user is authenticated. In that situation, + // the page may include sensitive or user-specific materials. As the cache is shared across all users, it can only contain public info. + if (cacheEnabled() && !isUserAuthenticated(req)) { + cache.set(getCacheKey(req), data); } } /** - * Whether this path is allowed to be cached. Only public paths can be cached as the cache is shared across all users. - * @param key page key (corresponds to path of page) - * @returns true if allowed to be cached, false otherwise. + * Whether a user is authenticated or not */ -function canCachePage(key: string) { - // Only these publicly accessible pages can be cached. - // NOTE: Caching pages which require authentication is NOT ALLOWED. The same cache is used by all users & user-specific data must NEVER appear in cache. - const allowedPages = [/^\/$/, /^\/home$/, /^\/items\//, /^\/entities\//, /^\/collections\//, /^\/communities\//, /^\/search[\/?]?/, /\/browse\//, /^\/community-list$/]; - - // Check whether any of these regexs match with the passed in key - return allowedPages.some(regex => regex.test(key)); +function isUserAuthenticated(req): boolean { + // Check whether our authentication Cookie exists or not + return req.cookies[TOKENITEM]; } /* From ee0ebebf14f9c0d4cf176ba520eab67511b5c513 Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Mon, 9 Jan 2023 16:24:12 -0600 Subject: [PATCH 13/22] Add missing test configs --- src/environments/environment.test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/environments/environment.test.ts b/src/environments/environment.test.ts index b323fa464d..88bc3e2223 100644 --- a/src/environments/environment.test.ts +++ b/src/environments/environment.test.ts @@ -55,6 +55,11 @@ export const environment: BuildConfig = { defaultTime: 0, maxBufferSize: 100, timePerMethod: { [RestRequestMethod.PATCH]: 3 } as any // time in seconds + }, + // In-memory cache of server-side rendered pages. Disabled in test environment (max=0) + serverSide: { + max: 0, + timeToLive: 15 * 60 * 1000, // 15 minutes } }, From 2a7aa2b5c12e106ea8210f0c478644eb12672f48 Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Mon, 9 Jan 2023 16:40:35 -0600 Subject: [PATCH 14/22] Updates to defaults of cache.control setting and enhanced comments --- config/config.example.yml | 8 ++++++-- src/config/default-app-config.ts | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/config/config.example.yml b/config/config.example.yml index ec1087525c..27155f74c1 100644 --- a/config/config.example.yml +++ b/config/config.example.yml @@ -32,8 +32,12 @@ cache: # NOTE: how long should objects be cached for by default msToLive: default: 900000 # 15 minutes - # Cache-Control HTTP Header - control: max-age=60 # revalidate browser + # Default 'Cache-Control' HTTP Header to set for all static content (including compiled *.js files) + # Defaults to max-age=604,800 seconds (one week). This lets a user's browser know that it can cache these + # files for one week, after which they will be "stale" and need to be redownloaded. + # NOTE: When updates are made to compiled *.js files, it will automatically bypass this browser cache, because + # all compiled *.js files include a unique hash in their name which updates when content is modified. + control: max-age=604800 # revalidate browser autoSync: defaultTime: 0 maxBufferSize: 100 diff --git a/src/config/default-app-config.ts b/src/config/default-app-config.ts index 516d0eca2e..a1ac29e8de 100644 --- a/src/config/default-app-config.ts +++ b/src/config/default-app-config.ts @@ -68,7 +68,7 @@ export class DefaultAppConfig implements AppConfig { default: 15 * 60 * 1000 // 15 minutes }, // Cache-Control HTTP Header - control: 'max-age=60', // revalidate browser + control: 'max-age=604800', // revalidate browser autoSync: { defaultTime: 0, maxBufferSize: 100, From f9810493c3023aed4a556791fea99172eda8ccfc Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Mon, 9 Jan 2023 17:02:13 -0600 Subject: [PATCH 15/22] Remove unused import --- server.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/server.ts b/server.ts index 3417cb3b96..bf6816d1c9 100644 --- a/server.ts +++ b/server.ts @@ -55,7 +55,6 @@ import { APP_CONFIG, AppConfig } from './src/config/app-config.interface'; import { extendEnvironmentWithAppConfig } from './src/config/config.util'; import { logStartupMessage } from './startup-message'; import { TOKENITEM } from 'src/app/core/auth/models/auth-token-info.model'; -import { isAuthenticated } from 'src/app/core/auth/selectors'; /* From 048e5b012d8669bb0c4d0f30f25fd602b5c2dc42 Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Tue, 10 Jan 2023 08:53:46 -0600 Subject: [PATCH 16/22] Rename enableCache() to initCache() to make code easier to understand --- server.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server.ts b/server.ts index bf6816d1c9..8f9617834d 100644 --- a/server.ts +++ b/server.ts @@ -93,12 +93,12 @@ export function app() { /* * If production mode is enabled in the environment file: * - Enable Angular's production mode - * - Enable caching of SSR rendered pages (if enabled in config.yml) + * - Initialize caching of SSR rendered pages (if enabled in config.yml) * - Enable compression for SSR reponses. See [compression](https://github.com/expressjs/compression) */ if (environment.production) { enableProdMode(); - enableCache(); + initCache(); server.use(compression({ // only compress responses we've marked as SSR // otherwise, this middleware may compress files we've chosen not to compress via compression-webpack-plugin @@ -310,9 +310,9 @@ function addCacheControl(req, res, next) { } /* - * Enable server-side caching of pages rendered via SSR. + * Initialize server-side caching of pages rendered via SSR. */ -function enableCache() { +function initCache() { if (cacheEnabled()) { // Initialize a new "least-recently-used" item cache (where least recently used items are removed first) // See https://www.npmjs.com/package/lru-cache From 36085e4854a28930a182b81b8ba5e476ed16e76f Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Fri, 20 Jan 2023 16:50:42 -0600 Subject: [PATCH 17/22] Avoid caching a page twice in a row --- server.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/server.ts b/server.ts index 8f9617834d..04a0e36670 100644 --- a/server.ts +++ b/server.ts @@ -399,7 +399,12 @@ function saveToCache(req, data: any) { // NOTE: It's not safe to save page data to the cache when a user is authenticated. In that situation, // the page may include sensitive or user-specific materials. As the cache is shared across all users, it can only contain public info. if (cacheEnabled() && !isUserAuthenticated(req)) { - cache.set(getCacheKey(req), data); + const key = getCacheKey(req); + // Make sure this key is not already in our cache. If "has()" returns true, + // then it's in the cache already and *not* expired. + if (!cache.has(key)) { + cache.set(key, data); + } } } From e92c2209e91d7c38fa991fff94bdcafacc5336a9 Mon Sep 17 00:00:00 2001 From: Agustina Martinez Date: Thu, 2 Feb 2023 23:39:50 +0000 Subject: [PATCH 18/22] Update recent-item-list.component.ts Update PaginatedSearchOptions to only retrieve items --- .../home-page/recent-item-list/recent-item-list.component.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/app/home-page/recent-item-list/recent-item-list.component.ts b/src/app/home-page/recent-item-list/recent-item-list.component.ts index f0e5803b71..9a8535d970 100644 --- a/src/app/home-page/recent-item-list/recent-item-list.component.ts +++ b/src/app/home-page/recent-item-list/recent-item-list.component.ts @@ -17,6 +17,7 @@ import { followLink, FollowLinkConfig } from '../../shared/utils/follow-link-con import { APP_CONFIG, AppConfig } from '../../../config/app-config.interface'; import { isPlatformBrowser } from '@angular/common'; import { setPlaceHolderAttributes } from '../../shared/utils/object-list-utils'; +import { DSpaceObjectType } from '../../core/shared/dspace-object-type.model'; @Component({ selector: 'ds-recent-item-list', @@ -67,6 +68,7 @@ export class RecentItemListComponent implements OnInit { this.itemRD$ = this.searchService.search( new PaginatedSearchOptions({ pagination: this.paginationConfig, + dsoTypes: [DSpaceObjectType.ITEM], sort: this.sortConfig, }), undefined, From bae63111e7a4ed8d4c9aec210a168b9aa2c1493c Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Fri, 3 Feb 2023 12:55:23 -0600 Subject: [PATCH 19/22] Refactor to two caches. One for bots and one for anonymous users --- config/config.example.yml | 40 +++++-- package.json | 1 + server.ts | 164 ++++++++++++++++++--------- src/config/cache-config.interface.ts | 21 +++- src/config/default-app-config.ts | 20 +++- yarn.lock | 5 + 6 files changed, 178 insertions(+), 73 deletions(-) diff --git a/config/config.example.yml b/config/config.example.yml index 27155f74c1..52b06b1b08 100644 --- a/config/config.example.yml +++ b/config/config.example.yml @@ -43,17 +43,37 @@ cache: maxBufferSize: 100 timePerMethod: PATCH: 3 # time in seconds - # In-memory cache of server-side rendered pages. This cache stores the most recently accessed public pages. - # Pages are automatically added/dropped from this cache based on how recently they have been used. + # In-memory cache(s) of server-side rendered pages. These caches will store the most recently accessed public pages. + # Pages are automatically added/dropped from these caches based on how recently they have been used. + # Restarting the app clears all page caches. + # NOTE: To control the cache size, use the "max" setting. Keep in mind, individual cached pages are usually small (<100KB). + # Enabling *both* caches will mean that a page may be cached twice, once in each cache (but may expire at different times via timeToLive). serverSide: - # Maximum number of pages to cache. Set to zero (0) to disable server side caching. Default is 100, which means - # the 100 most recently accessed public pages will be cached. As all pages are cached in server memory, - # increasing this value will increase memory needs. Individual cached pages are usually small (<100KB), - # so max=100 should only require a maximum of 9-10MB of memory. Restarting the app clears this page cache. - max: 100 - # Amount of time after which cached pages are considered stale (in ms). After becoming stale, the cached - # copy is automatically refreshed on the next request. - timeToLive: 900000 # 15 minutes + # When enabled (i.e. max > 0), known bots will be sent pages from a server side cache specific for bots. + # (Keep in mind, bot detection cannot be guarranteed. It is possible some bots will bypass this cache.) + botCache: + # Maximum number of pages to cache for known bots. Set to zero (0) to disable server side caching for bots. + # Default is 1000, which means the 1000 most recently accessed public pages will be cached. + # As all pages are cached in server memory, increasing this value will increase memory needs. + # Individual cached pages are usually small (<100KB), so max=1000 should only require ~100MB of memory. + max: 1000 + # Amount of time after which cached pages are considered stale (in ms). After becoming stale, the cached + # copy is automatically refreshed on the next request. + # NOTE: For the bot cache, this setting may impact how quickly search engine bots will index new content on your site. + # For example, setting this to one week may mean that search engine bots may not find all new content for one week. + timeToLive: 86400000 # 1 day + # When enabled (i.e. max > 0), all anonymous users will be sent pages from a server side cache. + # This allows anonymous users to interact more quickly with the site, but also means they may see slightly + # outdated content (based on timeToLive) + anonymousCache: + # Maximum number of pages to cache. Default is zero (0) which means anonymous user cache is disabled. + # As all pages are cached in server memory, increasing this value will increase memory needs. + # Individual cached pages are usually small (<100KB), so a value of max=1000 would only require ~100MB of memory. + max: 0 + # Amount of time after which cached pages are considered stale (in ms). After becoming stale, the cached + # copy is automatically refreshed on the next request. + # NOTE: For the anonymous cache, it is recommended to keep this value low to avoid anonymous users seeing outdated content. + timeToLive: 10000 # 10 seconds # Authentication settings auth: diff --git a/package.json b/package.json index 945bb1f158..184f1a5647 100644 --- a/package.json +++ b/package.json @@ -99,6 +99,7 @@ "fast-json-patch": "^3.0.0-1", "filesize": "^6.1.0", "http-proxy-middleware": "^1.0.5", + "isbot": "^3.6.5", "js-cookie": "2.2.1", "js-yaml": "^4.1.0", "json5": "^2.2.2", diff --git a/server.ts b/server.ts index 04a0e36670..8c9835cf16 100644 --- a/server.ts +++ b/server.ts @@ -29,6 +29,7 @@ import * as expressStaticGzip from 'express-static-gzip'; import axios from 'axios'; import LRU from 'lru-cache'; +import isbot from 'isbot'; import { createCertificate } from 'pem'; import { createServer } from 'https'; import { json } from 'body-parser'; @@ -70,8 +71,11 @@ const cookieParser = require('cookie-parser'); const appConfig: AppConfig = buildAppConfig(join(DIST_FOLDER, 'assets/config.json')); -// cache of SSR pages, only enabled in production mode -let cache: LRU; +// cache of SSR pages for known bots, only enabled in production mode +let botCache: LRU; + +// cache of SSR pages for anonymous users. Disabled by default, and only available in production mode +let anonymousCache: LRU; // extend environment with app config for server extendEnvironmentWithAppConfig(environment, appConfig); @@ -257,10 +261,10 @@ function serverSideRender(req, res, sendToUser: boolean = true) { providers: [{ provide: APP_BASE_HREF, useValue: req.baseUrl }] }, (err, data) => { if (hasNoValue(err) && hasValue(data)) { - res.locals.ssr = true; // mark response as SSR (enables text compression) - // save server side rendered data to cache + // save server side rendered page to cache (if any are enabled) saveToCache(req, data); if (sendToUser) { + res.locals.ssr = true; // mark response as SSR (enables text compression) // send rendered page to user res.send(data); } @@ -313,24 +317,45 @@ function addCacheControl(req, res, next) { * Initialize server-side caching of pages rendered via SSR. */ function initCache() { - if (cacheEnabled()) { - // Initialize a new "least-recently-used" item cache (where least recently used items are removed first) + if (botCacheEnabled()) { + // Initialize a new "least-recently-used" item cache (where least recently used pages are removed first) // See https://www.npmjs.com/package/lru-cache - cache = new LRU( { - max: environment.cache.serverSide.max || 100, // 100 items in cache maximum - ttl: environment.cache.serverSide.timeToLive || 15 * 60 * 1000, // 15 minute cache + // When enabled, each page defaults to expiring after 1 day + botCache = new LRU( { + max: environment.cache.serverSide.botCache.max, + ttl: environment.cache.serverSide.botCache.timeToLive || 24 * 60 * 60 * 1000, // 1 day + allowStale: true // If object is found to be stale, return stale value before deleting + }); + } + + if (anonymousCacheEnabled()) { + // NOTE: While caches may share SSR pages, this cache must be kept separately because the timeToLive + // may expire pages more frequently. + // When enabled, each page defaults to expiring after 10 seconds (to minimize anonymous users seeing out-of-date content) + anonymousCache = new LRU( { + max: environment.cache.serverSide.anonymousCache.max, + ttl: environment.cache.serverSide.anonymousCache.timeToLive || 10 * 1000, // 10 seconds allowStale: true // If object is found to be stale, return stale value before deleting }); } } /** - * Return whether server side caching is enabled in configuration. + * Return whether bot-specific server side caching is enabled in configuration. */ -function cacheEnabled(): boolean { - // Caching is only enabled is SSR is enabled AND - // "serverSide.max" setting is greater than zero - return environment.universal.preboot && environment.cache.serverSide.max && (environment.cache.serverSide.max > 0); +function botCacheEnabled(): boolean { + // Caching is only enabled if SSR is enabled AND + // "max" pages to cache is greater than zero + return environment.universal.preboot && environment.cache.serverSide.botCache.max && (environment.cache.serverSide.botCache.max > 0); +} + +/** + * Return whether anonymous user server side caching is enabled in configuration. + */ +function anonymousCacheEnabled(): boolean { + // Caching is only enabled if SSR is enabled AND + // "max" pages to cache is greater than zero + return environment.universal.preboot && environment.cache.serverSide.anonymousCache.max && (environment.cache.serverSide.anonymousCache.max > 0); } /** @@ -338,43 +363,64 @@ function cacheEnabled(): boolean { * Caching is ONLY done for SSR requests. Pages are cached base on their path (e.g. /home or /search?query=test) */ function cacheCheck(req, res, next) { - let cacheHit = false; - let debug = false; // Enable to see cache hits & re-rendering logs + // Cached copy of page (if found) + let cachedCopy; - // Only check cache if cache enabled & NOT authenticated. - // NOTE: Authenticated users cannot use the SSR cache. Cached pages only show data available to anonymous users. - // Only public pages can currently be cached, as the cached data is not user-specific. - if (cacheEnabled() && !isUserAuthenticated(req)) { - const key = getCacheKey(req); + // If the bot cache is enabled and this request looks like a bot, check the bot cache for a cached page. + if (botCacheEnabled() && isbot(req.get('user-agent'))) { + cachedCopy = checkCacheForRequest('bot', botCache, req, res); + } else if (anonymousCacheEnabled() && !isUserAuthenticated(req)) { + cachedCopy = checkCacheForRequest('anonymous', anonymousCache, req, res); + } - // Check if this page is in our cache - let cachedCopy = cache.get(key); - if (cachedCopy) { - cacheHit = true; - res.locals.ssr = true; // mark response as SSR (enables text compression) - if (debug) { console.log(`CACHE HIT FOR ${key}`); } - // return page from cache to user - res.send(cachedCopy); + // If cached copy exists, return it to the user. + if (cachedCopy) { + res.locals.ssr = true; // mark response as SSR-generated (enables text compression) + res.send(cachedCopy); - // Check if cached copy is expired (in this sitution key will now be gone from cache) - if (!cache.has(key)) { - if (debug) { console.log(`CACHE EXPIRED FOR ${key} Re-rendering...`); } - // Update cached copy by rerendering server-side - // NOTE: Cached copy was already returned to user above. So, this re-render is just to prepare for next user. - serverSideRender(req, res, false); - } + // Tell Express to skip all other handlers for this path + // This ensures we don't try to re-render the page since we've already returned the cached copy + next('router'); + } else { + // If nothing found in cache, just continue with next handler + // (This should send the request on to the handler that rerenders the page via SSR + next(); + } +} - // Tell Express to skip all other handlers for this path - // This ensures we don't try to re-render the page since we've already returned the cached copy - next('router'); +/** + * Checks if the current request (i.e. page) is found in the given cache. If it is found, + * the cached copy is returned. When found, this method also triggers a re-render via + * SSR if the cached copy is now expired (i.e. timeToLive has passed for this cached copy). + * @param cacheName name of cache (just useful for debug logging) + * @param cache LRU cache to check + * @param req current request to look for in the cache + * @param res current response + * @returns cached copy (if found) or undefined (if not found) + */ +function checkCacheForRequest(cacheName: string, cache: LRU, req, res): any { + let debug = false; // Enable to see cache hits & re-rendering in logs + + // Get the cache key for this request + const key = getCacheKey(req); + + // Check if this page is in our cache + let cachedCopy = cache.get(key); + if (cachedCopy) { + if (debug) { console.log(`CACHE HIT FOR ${key} in ${cacheName} cache`); } + + // Check if cached copy is expired (If expired, the key will now be gone from cache) + if (!cache.has(key)) { + if (debug) { console.log(`CACHE EXPIRED FOR ${key} in ${cacheName} cache. Re-rendering...`); } + // Update cached copy by rerendering server-side + // NOTE: In this scenario the currently cached copy will be returned to the current user. + // This re-render is peformed behind the scenes to update cached copy for next user. + serverSideRender(req, res, false); } } - // If nothing found in cache, just continue with next handler - // (This should send the request on to the handler that rerenders the page via SSR) - if (!cacheHit) { - next(); - } + // return page from cache + return cachedCopy; } /** @@ -390,20 +436,30 @@ function getCacheKey(req): string { } /** - * Save data to server side cache, if enabled. If caching is not enabled or user is authenticated, this is a noop + * Save page to server side cache(s), if enabled. If caching is not enabled or a user is authenticated, this is a noop + * If multiple caches are enabled, the page will be saved to any caches where it does not yet exist (or is expired). + * (This minimizes the number of times we need to run SSR on the same page.) * @param req current page request - * @param data page data to save to cache + * @param page page data to save to cache */ -function saveToCache(req, data: any) { - // Only cache if caching is enabled and no one is currently authenticated. This means ONLY public pages can be cached. +function saveToCache(req, page: any) { + // Only cache if no one is currently authenticated. This means ONLY public pages can be cached. // NOTE: It's not safe to save page data to the cache when a user is authenticated. In that situation, // the page may include sensitive or user-specific materials. As the cache is shared across all users, it can only contain public info. - if (cacheEnabled() && !isUserAuthenticated(req)) { + if (!isUserAuthenticated(req)) { const key = getCacheKey(req); - // Make sure this key is not already in our cache. If "has()" returns true, - // then it's in the cache already and *not* expired. - if (!cache.has(key)) { - cache.set(key, data); + // Avoid caching "/reload/[random]" paths (these are hard refreshes after logout) + if (key.startsWith('/reload')) { return; } + + // If bot cache is enabled, save it to that cache if it doesn't exist or is expired + // (NOTE: has() will return false if page is expired in cache) + if (botCacheEnabled() && !botCache.has(key)) { + botCache.set(key, page); + } + + // If anonymous cache is enabled, save it to that cache if it doesn't exist or is expired + if (anonymousCacheEnabled() && !anonymousCache.has(key)) { + anonymousCache.set(key, page); } } } @@ -412,7 +468,7 @@ function saveToCache(req, data: any) { * Whether a user is authenticated or not */ function isUserAuthenticated(req): boolean { - // Check whether our authentication Cookie exists or not + // Check whether our DSpace authentication Cookie exists or not return req.cookies[TOKENITEM]; } diff --git a/src/config/cache-config.interface.ts b/src/config/cache-config.interface.ts index d0dfc677d9..1826bd0d30 100644 --- a/src/config/cache-config.interface.ts +++ b/src/config/cache-config.interface.ts @@ -8,11 +8,22 @@ export interface CacheConfig extends Config { // Cache-Control HTTP Header control: string; autoSync: AutoSyncConfig; - // In-memory cache of server-side rendered content + // In-memory caches of server-side rendered (SSR) content. These caches can be used to limit the frequency + // of re-generating SSR pages to improve performance. serverSide: { - // Maximum number of pages (rendered via SSR) to cache. - max: number; - // Amount of time after which cached pages are considered stale (in ms) - timeToLive: number; + // Cache specific to known bots. Allows you to serve cached contents to bots only. + botCache: { + // Maximum number of pages (rendered via SSR) to cache. Setting max=0 disables the cache. + max: number; + // Amount of time after which cached pages are considered stale (in ms) + timeToLive: number; + }, + // Cache specific to anonymous users. Allows you to serve cached content to non-authenticated users. + anonymousCache: { + // Maximum number of pages (rendered via SSR) to cache. Setting max=0 disables the cache. + max: number; + // Amount of time after which cached pages are considered stale (in ms) + timeToLive: number; + } } } diff --git a/src/config/default-app-config.ts b/src/config/default-app-config.ts index a1ac29e8de..9e5b535872 100644 --- a/src/config/default-app-config.ts +++ b/src/config/default-app-config.ts @@ -76,10 +76,22 @@ export class DefaultAppConfig implements AppConfig { }, // In-memory cache of server-side rendered content serverSide: { - // Maximum number of pages (rendered via SSR) to cache. Set to zero to disable server side caching. - max: 100, - // Amount of time after which cached pages are considered stale (in ms) - timeToLive: 15 * 60 * 1000 // 15 minutes + // Cache specific to known bots. Allows you to serve cached contents to bots only. + // Defaults to caching 1,000 pages. Each page expires after 1 day + botCache: { + // Maximum number of pages (rendered via SSR) to cache. Setting max=0 disables the cache. + max: 1000, + // Amount of time after which cached pages are considered stale (in ms) + timeToLive: 24 * 60 * 60 * 1000, // 1 day + }, + // Cache specific to anonymous users. Allows you to serve cached content to non-authenticated users. + // Defaults to caching 0 pages. But, when enabled, each page expires after 10 seconds (to minimize anonymous users seeing out-of-date content) + anonymousCache: { + // Maximum number of pages (rendered via SSR) to cache. Setting max=0 disables the cache. + max: 0, // disabled by default + // Amount of time after which cached pages are considered stale (in ms) + timeToLive: 10 * 1000, // 10 seconds + } } }; diff --git a/yarn.lock b/yarn.lock index 96693299dc..3843ac4dab 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6749,6 +6749,11 @@ isbinaryfile@^4.0.8: resolved "https://registry.yarnpkg.com/isbinaryfile/-/isbinaryfile-4.0.10.tgz#0c5b5e30c2557a2f06febd37b7322946aaee42b3" integrity sha512-iHrqe5shvBUcFbmZq9zOQHBoeOhZJu6RQGrDpBgenUm/Am+F3JM2MgQj+rK3Z601fzrL5gLZWtAPH2OBaSVcyw== +isbot@^3.6.5: + version "3.6.5" + resolved "https://registry.yarnpkg.com/isbot/-/isbot-3.6.5.tgz#a749980d9dfba9ebcc03ee7b548d1f24dd8c9f1e" + integrity sha512-BchONELXt6yMad++BwGpa0oQxo/uD0keL7N15cYVf0A1oMIoNQ79OqeYdPMFWDrNhCqCbRuw9Y9F3QBjvAxZ5g== + isexe@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/isexe/-/isexe-2.0.0.tgz#e8fbf374dc556ff8947a10dcb0572d633f2cfa10" From a150c64e2f4ea1b475129a3f30f0dc25387b0dc4 Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Fri, 3 Feb 2023 13:28:35 -0600 Subject: [PATCH 20/22] Fix test configuration --- src/environments/environment.test.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/environments/environment.test.ts b/src/environments/environment.test.ts index 88bc3e2223..613024f798 100644 --- a/src/environments/environment.test.ts +++ b/src/environments/environment.test.ts @@ -58,8 +58,14 @@ export const environment: BuildConfig = { }, // In-memory cache of server-side rendered pages. Disabled in test environment (max=0) serverSide: { - max: 0, - timeToLive: 15 * 60 * 1000, // 15 minutes + botCache: { + max: 0, + timeToLive: 24 * 60 * 60 * 1000, // 1 day + }, + anonymousCache: { + max: 0, + timeToLive: 10 * 1000, // 10 seconds + } } }, From c099bc468d752fb357ac6ad786701c12728ef6c4 Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Tue, 7 Feb 2023 12:22:32 -0600 Subject: [PATCH 21/22] Add "debug" config and "allowStale" configs --- config/config.example.yml | 12 ++++++++++++ server.ts | 15 +++++++++------ src/config/cache-config.interface.ts | 6 ++++++ src/config/default-app-config.ts | 3 +++ src/environments/environment.test.ts | 3 +++ 5 files changed, 33 insertions(+), 6 deletions(-) diff --git a/config/config.example.yml b/config/config.example.yml index 52b06b1b08..500c2c476a 100644 --- a/config/config.example.yml +++ b/config/config.example.yml @@ -49,6 +49,8 @@ cache: # NOTE: To control the cache size, use the "max" setting. Keep in mind, individual cached pages are usually small (<100KB). # Enabling *both* caches will mean that a page may be cached twice, once in each cache (but may expire at different times via timeToLive). serverSide: + # Set to true to see all cache hits/misses/refreshes in your console logs. Useful for debugging SSR caching issues. + debug: false # When enabled (i.e. max > 0), known bots will be sent pages from a server side cache specific for bots. # (Keep in mind, bot detection cannot be guarranteed. It is possible some bots will bypass this cache.) botCache: @@ -62,6 +64,11 @@ cache: # NOTE: For the bot cache, this setting may impact how quickly search engine bots will index new content on your site. # For example, setting this to one week may mean that search engine bots may not find all new content for one week. timeToLive: 86400000 # 1 day + # When set to true, after timeToLive expires, the next request will receive the *cached* page & then re-render the page + # behind the scenes to update the cache. This ensures users primarily interact with the cache, but may receive stale pages (older than timeToLive). + # When set to false, after timeToLive expires, the next request will wait on SSR to complete & receive a fresh page (which is then saved to cache). + # This ensures stale pages (older than timeToLive) are never returned from the cache, but some users will wait on SSR. + allowStale: true # When enabled (i.e. max > 0), all anonymous users will be sent pages from a server side cache. # This allows anonymous users to interact more quickly with the site, but also means they may see slightly # outdated content (based on timeToLive) @@ -74,6 +81,11 @@ cache: # copy is automatically refreshed on the next request. # NOTE: For the anonymous cache, it is recommended to keep this value low to avoid anonymous users seeing outdated content. timeToLive: 10000 # 10 seconds + # When set to true, after timeToLive expires, the next request will receive the *cached* page & then re-render the page + # behind the scenes to update the cache. This ensures users primarily interact with the cache, but may receive stale pages (older than timeToLive). + # When set to false, after timeToLive expires, the next request will wait on SSR to complete & receive a fresh page (which is then saved to cache). + # This ensures stale pages (older than timeToLive) are never returned from the cache, but some users will wait on SSR. + allowStale: true # Authentication settings auth: diff --git a/server.ts b/server.ts index 8c9835cf16..478a4c0ec1 100644 --- a/server.ts +++ b/server.ts @@ -324,7 +324,7 @@ function initCache() { botCache = new LRU( { max: environment.cache.serverSide.botCache.max, ttl: environment.cache.serverSide.botCache.timeToLive || 24 * 60 * 60 * 1000, // 1 day - allowStale: true // If object is found to be stale, return stale value before deleting + allowStale: environment.cache.serverSide.botCache.allowStale || true // if object is stale, return stale value before deleting }); } @@ -335,7 +335,7 @@ function initCache() { anonymousCache = new LRU( { max: environment.cache.serverSide.anonymousCache.max, ttl: environment.cache.serverSide.anonymousCache.timeToLive || 10 * 1000, // 10 seconds - allowStale: true // If object is found to be stale, return stale value before deleting + allowStale: environment.cache.serverSide.anonymousCache.allowStale || true // if object is stale, return stale value before deleting }); } } @@ -399,24 +399,25 @@ function cacheCheck(req, res, next) { * @returns cached copy (if found) or undefined (if not found) */ function checkCacheForRequest(cacheName: string, cache: LRU, req, res): any { - let debug = false; // Enable to see cache hits & re-rendering in logs - // Get the cache key for this request const key = getCacheKey(req); // Check if this page is in our cache let cachedCopy = cache.get(key); if (cachedCopy) { - if (debug) { console.log(`CACHE HIT FOR ${key} in ${cacheName} cache`); } + if (environment.cache.serverSide.debug) { console.log(`CACHE HIT FOR ${key} in ${cacheName} cache`); } // Check if cached copy is expired (If expired, the key will now be gone from cache) + // NOTE: This will only occur when "allowStale=true", as it means the "get(key)" above returned a stale value. if (!cache.has(key)) { - if (debug) { console.log(`CACHE EXPIRED FOR ${key} in ${cacheName} cache. Re-rendering...`); } + if (environment.cache.serverSide.debug) { console.log(`CACHE EXPIRED FOR ${key} in ${cacheName} cache. Re-rendering...`); } // Update cached copy by rerendering server-side // NOTE: In this scenario the currently cached copy will be returned to the current user. // This re-render is peformed behind the scenes to update cached copy for next user. serverSideRender(req, res, false); } + } else { + if (environment.cache.serverSide.debug) { console.log(`CACHE MISS FOR ${key} in ${cacheName} cache.`); } } // return page from cache @@ -455,11 +456,13 @@ function saveToCache(req, page: any) { // (NOTE: has() will return false if page is expired in cache) if (botCacheEnabled() && !botCache.has(key)) { botCache.set(key, page); + if (environment.cache.serverSide.debug) { console.log(`CACHE SAVE FOR ${key} in bot cache.`); } } // If anonymous cache is enabled, save it to that cache if it doesn't exist or is expired if (anonymousCacheEnabled() && !anonymousCache.has(key)) { anonymousCache.set(key, page); + if (environment.cache.serverSide.debug) { console.log(`CACHE SAVE FOR ${key} in anonymous cache.`); } } } } diff --git a/src/config/cache-config.interface.ts b/src/config/cache-config.interface.ts index 1826bd0d30..9560fe46a5 100644 --- a/src/config/cache-config.interface.ts +++ b/src/config/cache-config.interface.ts @@ -11,12 +11,16 @@ export interface CacheConfig extends Config { // In-memory caches of server-side rendered (SSR) content. These caches can be used to limit the frequency // of re-generating SSR pages to improve performance. serverSide: { + // Debug server-side caching. Set to true to see cache hits/misses/refreshes in console logs. + debug: boolean, // Cache specific to known bots. Allows you to serve cached contents to bots only. botCache: { // Maximum number of pages (rendered via SSR) to cache. Setting max=0 disables the cache. max: number; // Amount of time after which cached pages are considered stale (in ms) timeToLive: number; + // true = return page from cache after timeToLive expires. false = return a fresh page after timeToLive expires + allowStale: boolean; }, // Cache specific to anonymous users. Allows you to serve cached content to non-authenticated users. anonymousCache: { @@ -24,6 +28,8 @@ export interface CacheConfig extends Config { max: number; // Amount of time after which cached pages are considered stale (in ms) timeToLive: number; + // true = return page from cache after timeToLive expires. false = return a fresh page after timeToLive expires + allowStale: boolean; } } } diff --git a/src/config/default-app-config.ts b/src/config/default-app-config.ts index 9e5b535872..e7851d4b34 100644 --- a/src/config/default-app-config.ts +++ b/src/config/default-app-config.ts @@ -76,6 +76,7 @@ export class DefaultAppConfig implements AppConfig { }, // In-memory cache of server-side rendered content serverSide: { + debug: false, // Cache specific to known bots. Allows you to serve cached contents to bots only. // Defaults to caching 1,000 pages. Each page expires after 1 day botCache: { @@ -83,6 +84,7 @@ export class DefaultAppConfig implements AppConfig { max: 1000, // Amount of time after which cached pages are considered stale (in ms) timeToLive: 24 * 60 * 60 * 1000, // 1 day + allowStale: true, }, // Cache specific to anonymous users. Allows you to serve cached content to non-authenticated users. // Defaults to caching 0 pages. But, when enabled, each page expires after 10 seconds (to minimize anonymous users seeing out-of-date content) @@ -91,6 +93,7 @@ export class DefaultAppConfig implements AppConfig { max: 0, // disabled by default // Amount of time after which cached pages are considered stale (in ms) timeToLive: 10 * 1000, // 10 seconds + allowStale: true, } } }; diff --git a/src/environments/environment.test.ts b/src/environments/environment.test.ts index 613024f798..0bb36da61f 100644 --- a/src/environments/environment.test.ts +++ b/src/environments/environment.test.ts @@ -58,13 +58,16 @@ export const environment: BuildConfig = { }, // In-memory cache of server-side rendered pages. Disabled in test environment (max=0) serverSide: { + debug: false, botCache: { max: 0, timeToLive: 24 * 60 * 60 * 1000, // 1 day + allowStale: true, }, anonymousCache: { max: 0, timeToLive: 10 * 1000, // 10 seconds + allowStale: true, } } }, From 9e11d69a8ade6fae07676d49f6a2cb4697705107 Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Thu, 9 Feb 2023 08:48:50 -0600 Subject: [PATCH 22/22] Fix bug where allowStale couldn't be disabled --- server.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server.ts b/server.ts index 478a4c0ec1..ba0c8fd7b2 100644 --- a/server.ts +++ b/server.ts @@ -324,7 +324,7 @@ function initCache() { botCache = new LRU( { max: environment.cache.serverSide.botCache.max, ttl: environment.cache.serverSide.botCache.timeToLive || 24 * 60 * 60 * 1000, // 1 day - allowStale: environment.cache.serverSide.botCache.allowStale || true // if object is stale, return stale value before deleting + allowStale: environment.cache.serverSide.botCache.allowStale ?? true // if object is stale, return stale value before deleting }); } @@ -335,7 +335,7 @@ function initCache() { anonymousCache = new LRU( { max: environment.cache.serverSide.anonymousCache.max, ttl: environment.cache.serverSide.anonymousCache.timeToLive || 10 * 1000, // 10 seconds - allowStale: environment.cache.serverSide.anonymousCache.allowStale || true // if object is stale, return stale value before deleting + allowStale: environment.cache.serverSide.anonymousCache.allowStale ?? true // if object is stale, return stale value before deleting }); } }