From 4cfff33301460217e9140562fd0a19114281a811 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Mon, 24 Jan 2022 22:04:02 +0100 Subject: [PATCH 1/2] [CST-4880] Add ServerCheckGuard in order to show internal server error page when rest server is not available --- src/app/app-routing-paths.ts | 6 ++ src/app/app-routing.module.ts | 39 +++++++---- src/app/app.module.ts | 8 ++- src/app/core/data/root-data.service.spec.ts | 43 +++++++++++- src/app/core/data/root-data.service.ts | 27 +++++++- .../server-check/server-check.guard.spec.ts | 68 +++++++++++++++++++ .../core/server-check/server-check.guard.ts | 39 +++++++++++ .../core/services/server-response.service.ts | 4 ++ .../page-internal-server-error.component.html | 10 +++ .../page-internal-server-error.component.scss | 0 .../page-internal-server-error.component.ts | 23 +++++++ ...ed-page-internal-server-error.component.ts | 26 +++++++ src/app/root/root.component.ts | 9 ++- src/app/shared/menu/menu.component.ts | 4 +- src/assets/i18n/en.json5 | 5 ++ 15 files changed, 290 insertions(+), 21 deletions(-) create mode 100644 src/app/core/server-check/server-check.guard.spec.ts create mode 100644 src/app/core/server-check/server-check.guard.ts create mode 100644 src/app/page-internal-server-error/page-internal-server-error.component.html create mode 100644 src/app/page-internal-server-error/page-internal-server-error.component.scss create mode 100644 src/app/page-internal-server-error/page-internal-server-error.component.ts create mode 100644 src/app/page-internal-server-error/themed-page-internal-server-error.component.ts diff --git a/src/app/app-routing-paths.ts b/src/app/app-routing-paths.ts index db6b22a023..d9c3410931 100644 --- a/src/app/app-routing-paths.ts +++ b/src/app/app-routing-paths.ts @@ -89,6 +89,12 @@ export function getPageNotFoundRoute() { return `/${PAGE_NOT_FOUND_PATH}`; } +export const INTERNAL_SERVER_ERROR = '500'; + +export function getPageInternalServerErrorRoute() { + return `/${INTERNAL_SERVER_ERROR}`; +} + export const INFO_MODULE_PATH = 'info'; export function getInfoModulePath() { return `/${INFO_MODULE_PATH}`; diff --git a/src/app/app-routing.module.ts b/src/app/app-routing.module.ts index 04d2c55bdd..88f7791b1b 100644 --- a/src/app/app-routing.module.ts +++ b/src/app/app-routing.module.ts @@ -11,10 +11,12 @@ import { FORBIDDEN_PATH, FORGOT_PASSWORD_PATH, INFO_MODULE_PATH, + INTERNAL_SERVER_ERROR, + LEGACY_BITSTREAM_MODULE_PATH, PROFILE_MODULE_PATH, REGISTER_PATH, + REQUEST_COPY_MODULE_PATH, WORKFLOW_ITEM_MODULE_PATH, - LEGACY_BITSTREAM_MODULE_PATH, REQUEST_COPY_MODULE_PATH, } from './app-routing-paths'; import { COLLECTION_MODULE_PATH } from './collection-page/collection-page-routing-paths'; import { COMMUNITY_MODULE_PATH } from './community-page/community-page-routing-paths'; @@ -26,14 +28,25 @@ import { SiteRegisterGuard } from './core/data/feature-authorization/feature-aut import { ThemedPageNotFoundComponent } from './pagenotfound/themed-pagenotfound.component'; import { ThemedForbiddenComponent } from './forbidden/themed-forbidden.component'; import { GroupAdministratorGuard } from './core/data/feature-authorization/feature-authorization-guard/group-administrator.guard'; +import { ThemedPageInternalServerErrorComponent } from './page-internal-server-error/themed-page-internal-server-error.component'; +import { ServerCheckGuard } from './core/server-check/server-check.guard'; @NgModule({ imports: [ - RouterModule.forRoot([{ - path: '', canActivate: [AuthBlockingGuard], + RouterModule.forRoot([ + { path: INTERNAL_SERVER_ERROR, component: ThemedPageInternalServerErrorComponent }, + { + path: '', + canActivate: [AuthBlockingGuard], + canActivateChild: [ServerCheckGuard], children: [ { path: '', redirectTo: '/home', pathMatch: 'full' }, - { path: 'reload/:rnd', component: ThemedPageNotFoundComponent, pathMatch: 'full', canActivate: [ReloadGuard] }, + { + path: 'reload/:rnd', + component: ThemedPageNotFoundComponent, + pathMatch: 'full', + canActivate: [ReloadGuard] + }, { path: 'home', loadChildren: () => import('./home-page/home-page.module') @@ -89,7 +102,8 @@ import { GroupAdministratorGuard } from './core/data/feature-authorization/featu .then((m) => m.ItemPageModule), canActivate: [EndUserAgreementCurrentUserGuard] }, - { path: 'entities/:entity-type', + { + path: 'entities/:entity-type', loadChildren: () => import('./item-page/item-page.module') .then((m) => m.ItemPageModule), canActivate: [EndUserAgreementCurrentUserGuard] @@ -133,12 +147,12 @@ import { GroupAdministratorGuard } from './core/data/feature-authorization/featu { path: 'login', loadChildren: () => import('./login-page/login-page.module') - .then((m) => m.LoginPageModule), + .then((m) => m.LoginPageModule) }, { path: 'logout', loadChildren: () => import('./logout-page/logout-page.module') - .then((m) => m.LogoutPageModule), + .then((m) => m.LogoutPageModule) }, { path: 'submit', @@ -178,7 +192,7 @@ import { GroupAdministratorGuard } from './core/data/feature-authorization/featu }, { path: INFO_MODULE_PATH, - loadChildren: () => import('./info/info.module').then((m) => m.InfoModule), + loadChildren: () => import('./info/info.module').then((m) => m.InfoModule) }, { path: REQUEST_COPY_MODULE_PATH, @@ -192,7 +206,7 @@ import { GroupAdministratorGuard } from './core/data/feature-authorization/featu { path: 'statistics', loadChildren: () => import('./statistics-page/statistics-page-routing.module') - .then((m) => m.StatisticsPageRoutingModule), + .then((m) => m.StatisticsPageRoutingModule) }, { path: ACCESS_CONTROL_MODULE_PATH, @@ -200,9 +214,10 @@ import { GroupAdministratorGuard } from './core/data/feature-authorization/featu canActivate: [GroupAdministratorGuard], }, { path: '**', pathMatch: 'full', component: ThemedPageNotFoundComponent }, - ]} - ],{ - onSameUrlNavigation: 'reload', + ] + } + ], { + onSameUrlNavigation: 'reload', }) ], exports: [RouterModule], diff --git a/src/app/app.module.ts b/src/app/app.module.ts index 32c3c78348..67bccd9105 100755 --- a/src/app/app.module.ts +++ b/src/app/app.module.ts @@ -54,8 +54,10 @@ import { ThemedFooterComponent } from './footer/themed-footer.component'; import { ThemedBreadcrumbsComponent } from './breadcrumbs/themed-breadcrumbs.component'; import { ThemedHeaderNavbarWrapperComponent } from './header-nav-wrapper/themed-header-navbar-wrapper.component'; import { IdleModalComponent } from './shared/idle-modal/idle-modal.component'; +import { ThemedPageInternalServerErrorComponent } from './page-internal-server-error/themed-page-internal-server-error.component'; +import { PageInternalServerErrorComponent } from './page-internal-server-error/page-internal-server-error.component'; -import { AppConfig, APP_CONFIG } from '../config/app-config.interface'; +import { APP_CONFIG, AppConfig } from '../config/app-config.interface'; export function getConfig() { return environment; @@ -181,7 +183,9 @@ const DECLARATIONS = [ ThemedBreadcrumbsComponent, ForbiddenComponent, ThemedForbiddenComponent, - IdleModalComponent + IdleModalComponent, + ThemedPageInternalServerErrorComponent, + PageInternalServerErrorComponent ]; const EXPORTS = [ diff --git a/src/app/core/data/root-data.service.spec.ts b/src/app/core/data/root-data.service.spec.ts index 9e5bb9a68f..e3e23dac12 100644 --- a/src/app/core/data/root-data.service.spec.ts +++ b/src/app/core/data/root-data.service.spec.ts @@ -1,13 +1,16 @@ import { RootDataService } from './root-data.service'; import { HALEndpointService } from '../shared/hal-endpoint.service'; import { createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; -import { Observable } from 'rxjs'; +import { Observable, of } from 'rxjs'; import { RemoteData } from './remote-data'; import { Root } from './root.model'; +import { RawRestResponse } from '../dspace-rest/raw-rest-response.model'; +import { cold } from 'jasmine-marbles'; describe('RootDataService', () => { let service: RootDataService; let halService: HALEndpointService; + let restService; let rootEndpoint; beforeEach(() => { @@ -15,7 +18,10 @@ describe('RootDataService', () => { halService = jasmine.createSpyObj('halService', { getRootHref: rootEndpoint }); - service = new RootDataService(null, null, null, null, halService, null, null, null); + restService = jasmine.createSpyObj('halService', { + get: jasmine.createSpy('get') + }); + service = new RootDataService(null, null, null, null, halService, null, null, null, restService); (service as any).dataService = jasmine.createSpyObj('dataService', { findByHref: createSuccessfulRemoteDataObject$({}) }); @@ -35,4 +41,37 @@ describe('RootDataService', () => { }); }); }); + + describe('checkServerAvailability', () => { + let result$: Observable; + + it('should return observable of true when root endpoint is available', () => { + const mockResponse = { + statusCode: 200, + statusText: 'OK' + } as RawRestResponse; + + restService.get.and.returnValue(of(mockResponse)); + result$ = service.checkServerAvailability(); + + expect(result$).toBeObservable(cold('(a|)', { + a: true + })); + }); + + it('should return observable of false when root endpoint is not available', () => { + const mockResponse = { + statusCode: 500, + statusText: 'Internal Server Error' + } as RawRestResponse; + + restService.get.and.returnValue(of(mockResponse)); + result$ = service.checkServerAvailability(); + + expect(result$).toBeObservable(cold('(a|)', { + a: false + })); + }); + + }); }); diff --git a/src/app/core/data/root-data.service.ts b/src/app/core/data/root-data.service.ts index 8b4e836671..07d2de32ef 100644 --- a/src/app/core/data/root-data.service.ts +++ b/src/app/core/data/root-data.service.ts @@ -17,6 +17,10 @@ import { RemoteData } from './remote-data'; import { FollowLinkConfig } from '../../shared/utils/follow-link-config.model'; import { FindListOptions } from './request.models'; import { PaginatedList } from './paginated-list.model'; +import { DspaceRestService } from '../dspace-rest/dspace-rest.service'; +import { RawRestResponse } from '../dspace-rest/raw-rest-response.model'; +import { catchError, map } from 'rxjs/operators'; +import { of } from 'rxjs/internal/observable/of'; /* tslint:disable:max-classes-per-file */ @@ -59,10 +63,24 @@ export class RootDataService { protected halService: HALEndpointService, protected notificationsService: NotificationsService, protected http: HttpClient, - protected comparator: DefaultChangeAnalyzer) { + protected comparator: DefaultChangeAnalyzer, + protected restService: DspaceRestService) { this.dataService = new DataServiceImpl(requestService, rdbService, null, objectCache, halService, notificationsService, http, comparator); } + /** + * Check if root endpoint is available + */ + checkServerAvailability(): Observable { + return this.restService.get(this.halService.getRootHref()).pipe( + catchError((err ) => { + console.error(err); + return of(false); + }), + map((res: RawRestResponse) => res.statusCode === 200) + ); + } + /** * Find the {@link Root} object of the REST API * @param useCachedVersionIfAvailable If this is true, the request will only be sent if there's @@ -106,5 +124,12 @@ export class RootDataService { findAllByHref(href: string | Observable, findListOptions: FindListOptions = {}, useCachedVersionIfAvailable = true, reRequestOnStale = true, ...linksToFollow: FollowLinkConfig[]): Observable>> { return this.dataService.findAllByHref(href, findListOptions, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow); } + + /** + * Set to sale the root endpoint cache hit + */ + invalidateRootCache() { + this.requestService.setStaleByHrefSubstring('server/api'); + } } /* tslint:enable:max-classes-per-file */ diff --git a/src/app/core/server-check/server-check.guard.spec.ts b/src/app/core/server-check/server-check.guard.spec.ts new file mode 100644 index 0000000000..1f126be5e5 --- /dev/null +++ b/src/app/core/server-check/server-check.guard.spec.ts @@ -0,0 +1,68 @@ +import { ServerCheckGuard } from './server-check.guard'; +import { Router } from '@angular/router'; + +import { of } from 'rxjs'; +import { take } from 'rxjs/operators'; + +import { getPageInternalServerErrorRoute } from '../../app-routing-paths'; +import { RootDataService } from '../data/root-data.service'; +import SpyObj = jasmine.SpyObj; + +describe('ServerCheckGuard', () => { + let guard: ServerCheckGuard; + let router: SpyObj; + let rootDataServiceStub: SpyObj; + + rootDataServiceStub = jasmine.createSpyObj('RootDataService', { + checkServerAvailability: jasmine.createSpy('checkServerAvailability'), + invalidateRootCache: jasmine.createSpy('invalidateRootCache') + }); + router = jasmine.createSpyObj('Router', { + navigateByUrl: jasmine.createSpy('navigateByUrl') + }); + + beforeEach(() => { + guard = new ServerCheckGuard(router, rootDataServiceStub); + }); + + afterEach(() => { + router.navigateByUrl.calls.reset(); + rootDataServiceStub.invalidateRootCache.calls.reset(); + }); + + it('should be created', () => { + expect(guard).toBeTruthy(); + }); + + describe('when root endpoint has succeeded', () => { + beforeEach(() => { + rootDataServiceStub.checkServerAvailability.and.returnValue(of(true)); + }); + + it('should not redirect to error page', () => { + guard.canActivateChild({} as any, {} as any).pipe( + take(1) + ).subscribe((canActivate: boolean) => { + expect(canActivate).toEqual(true); + expect(rootDataServiceStub.invalidateRootCache).not.toHaveBeenCalled(); + expect(router.navigateByUrl).not.toHaveBeenCalled(); + }); + }); + }); + + describe('when root endpoint has not succeeded', () => { + beforeEach(() => { + rootDataServiceStub.checkServerAvailability.and.returnValue(of(false)); + }); + + it('should redirect to error page', () => { + guard.canActivateChild({} as any, {} as any).pipe( + take(1) + ).subscribe((canActivate: boolean) => { + expect(canActivate).toEqual(false); + expect(rootDataServiceStub.invalidateRootCache).toHaveBeenCalled(); + expect(router.navigateByUrl).toHaveBeenCalledWith(getPageInternalServerErrorRoute()); + }); + }); + }); +}); diff --git a/src/app/core/server-check/server-check.guard.ts b/src/app/core/server-check/server-check.guard.ts new file mode 100644 index 0000000000..8a0e26c01d --- /dev/null +++ b/src/app/core/server-check/server-check.guard.ts @@ -0,0 +1,39 @@ +import { Injectable } from '@angular/core'; +import { ActivatedRouteSnapshot, CanActivateChild, Router, RouterStateSnapshot } from '@angular/router'; + +import { Observable } from 'rxjs'; +import { take, tap } from 'rxjs/operators'; + +import { RootDataService } from '../data/root-data.service'; +import { getPageInternalServerErrorRoute } from '../../app-routing-paths'; + +@Injectable({ + providedIn: 'root' +}) +/** + * A guard that checks if root api endpoint is reachable. + * If not redirect to 500 error page + */ +export class ServerCheckGuard implements CanActivateChild { + constructor(private router: Router, private rootDataService: RootDataService) { + } + + /** + * True when root api endpoint is reachable. + */ + canActivateChild( + route: ActivatedRouteSnapshot, + state: RouterStateSnapshot): Observable { + + return this.rootDataService.checkServerAvailability().pipe( + take(1), + tap((isAvailable: boolean) => { + if (!isAvailable) { + this.rootDataService.invalidateRootCache(); + this.router.navigateByUrl(getPageInternalServerErrorRoute()); + } + }) + ); + + } +} diff --git a/src/app/core/services/server-response.service.ts b/src/app/core/services/server-response.service.ts index df5662991d..02e00446bc 100644 --- a/src/app/core/services/server-response.service.ts +++ b/src/app/core/services/server-response.service.ts @@ -31,4 +31,8 @@ export class ServerResponseService { setNotFound(message = 'Not found'): this { return this.setStatus(404, message); } + + setInternalServerError(message = 'Internal Server Error'): this { + return this.setStatus(500, message); + } } diff --git a/src/app/page-internal-server-error/page-internal-server-error.component.html b/src/app/page-internal-server-error/page-internal-server-error.component.html new file mode 100644 index 0000000000..4995afc80b --- /dev/null +++ b/src/app/page-internal-server-error/page-internal-server-error.component.html @@ -0,0 +1,10 @@ +
+

500

+

{{"500.page-internal-server-error" | translate}}

+
+

{{"500.help" | translate}}

+
+

+ {{"500.link.home-page" | translate}} +

+
diff --git a/src/app/page-internal-server-error/page-internal-server-error.component.scss b/src/app/page-internal-server-error/page-internal-server-error.component.scss new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/app/page-internal-server-error/page-internal-server-error.component.ts b/src/app/page-internal-server-error/page-internal-server-error.component.ts new file mode 100644 index 0000000000..7ecb0a7609 --- /dev/null +++ b/src/app/page-internal-server-error/page-internal-server-error.component.ts @@ -0,0 +1,23 @@ +import { ChangeDetectionStrategy, Component } from '@angular/core'; +import { ServerResponseService } from '../core/services/server-response.service'; + +/** + * This component representing the `PageInternalServer` DSpace page. + */ +@Component({ + selector: 'ds-page-internal-server-error', + styleUrls: ['./page-internal-server-error.component.scss'], + templateUrl: './page-internal-server-error.component.html', + changeDetection: ChangeDetectionStrategy.Default +}) +export class PageInternalServerErrorComponent { + + /** + * Initialize instance variables + * + * @param {ServerResponseService} responseService + */ + constructor(private responseService: ServerResponseService) { + this.responseService.setInternalServerError(); + } +} diff --git a/src/app/page-internal-server-error/themed-page-internal-server-error.component.ts b/src/app/page-internal-server-error/themed-page-internal-server-error.component.ts new file mode 100644 index 0000000000..e8792c4789 --- /dev/null +++ b/src/app/page-internal-server-error/themed-page-internal-server-error.component.ts @@ -0,0 +1,26 @@ +import { Component } from '@angular/core'; +import { ThemedComponent } from '../shared/theme-support/themed.component'; +import { PageInternalServerErrorComponent } from './page-internal-server-error.component'; + +/** + * Themed wrapper for PageInternalServerErrorComponent + */ +@Component({ + selector: 'ds-themed-search-page', + styleUrls: [], + templateUrl: '../shared/theme-support/themed.component.html', +}) +export class ThemedPageInternalServerErrorComponent extends ThemedComponent { + + protected getComponentName(): string { + return 'PageInternalServerErrorComponent'; + } + + protected importThemedComponent(themeName: string): Promise { + return import(`../../themes/${themeName}/app/page-internal-server-error/page-internal-server-error.component`); + } + + protected importUnthemedComponent(): Promise { + return import(`./page-internal-server-error.component`); + } +} diff --git a/src/app/root/root.component.ts b/src/app/root/root.component.ts index 6ba859ef23..dc44095573 100644 --- a/src/app/root/root.component.ts +++ b/src/app/root/root.component.ts @@ -1,5 +1,5 @@ import { map } from 'rxjs/operators'; -import { Component, Inject, OnInit, Input } from '@angular/core'; +import { Component, Inject, Input, OnInit } from '@angular/core'; import { Router } from '@angular/router'; import { combineLatest as combineLatestObservable, Observable, of } from 'rxjs'; @@ -19,6 +19,7 @@ import { ThemeConfig } from '../../config/theme.model'; import { Angulartics2DSpace } from '../statistics/angulartics/dspace-provider'; import { environment } from '../../environments/environment'; import { slideSidebarPadding } from '../shared/animations/slide'; +import { getPageInternalServerErrorRoute } from '../app-routing-paths'; @Component({ selector: 'ds-root', @@ -68,9 +69,13 @@ export class RootComponent implements OnInit { this.totalSidebarWidth = this.cssService.getVariable('totalSidebarWidth'); const sidebarCollapsed = this.menuService.isMenuCollapsed(MenuID.ADMIN); - this.slideSidebarOver = combineLatestObservable(sidebarCollapsed, this.windowService.isXsOrSm()) + this.slideSidebarOver = combineLatestObservable([sidebarCollapsed, this.windowService.isXsOrSm()]) .pipe( map(([collapsed, mobile]) => collapsed || mobile) ); + + if (this.router.url === getPageInternalServerErrorRoute()) { + this.shouldShowRouteLoader = false; + } } } diff --git a/src/app/shared/menu/menu.component.ts b/src/app/shared/menu/menu.component.ts index caf613a33f..e5e899805b 100644 --- a/src/app/shared/menu/menu.component.ts +++ b/src/app/shared/menu/menu.component.ts @@ -145,8 +145,8 @@ export class MenuComponent implements OnInit, OnDestroy { * Get statistics route dso data */ getObjectUrl(data) { - const object = data.site ? data.site : data.dso.payload; - return object._links.self.href; + const object = data.site ? data.site : data.dso?.payload; + return object?._links?.self?.href; } /** diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 6b47238f5b..f82835401d 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -14,6 +14,11 @@ "403.forbidden": "forbidden", + "500.page-internal-server-error": "Service Unavailable", + + "500.help": "The server is temporarily unable to service your request due to maintenance downtime or capacity problems. Please try again later.", + + "500.link.home-page": "Take me to the home page", "404.help": "We can't find the page you're looking for. The page may have been moved or deleted. You can use the button below to get back to the home page. ", From d9df7336df2b5310eff20dd82507f8b61fda3382 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Tue, 25 Jan 2022 10:57:08 +0100 Subject: [PATCH 2/2] [CST-4880] Fix invalidateRootCache method --- src/app/core/data/root-data.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/core/data/root-data.service.ts b/src/app/core/data/root-data.service.ts index 07d2de32ef..ef542199dd 100644 --- a/src/app/core/data/root-data.service.ts +++ b/src/app/core/data/root-data.service.ts @@ -129,7 +129,7 @@ export class RootDataService { * Set to sale the root endpoint cache hit */ invalidateRootCache() { - this.requestService.setStaleByHrefSubstring('server/api'); + this.requestService.setStaleByHrefSubstring(this.halService.getRootHref()); } } /* tslint:enable:max-classes-per-file */