From b38d7384f7ef6571895d6d0cf89791749c33f783 Mon Sep 17 00:00:00 2001 From: elinuxhenrik Date: Tue, 9 Feb 2021 13:01:42 +0100 Subject: [PATCH] Add error handling to HttpRequestInterceptor Change-Id: Ib250258eec2f2b4693215d79e918184f530fac64 Signed-off-by: elinuxhenrik Issue-ID: NONRTRIC-432 --- .../app/ei-coordinator/ei-job.datasource.spec.ts | 6 +- .../src/app/ei-coordinator/ei-job.datasource.ts | 14 +-- .../ei-coordinator/ei-producer.datasource.spec.ts | 4 +- .../app/ei-coordinator/ei-producer.datasource.ts | 13 +-- webapp-frontend/src/app/interceptor.spec.ts | 107 +++++++++++++++++++++ webapp-frontend/src/app/interceptor.ts | 23 +++-- .../policy-control/policy-type.datasource.spec.ts | 4 +- .../app/policy-control/policy-type.datasource.ts | 22 +---- 8 files changed, 131 insertions(+), 62 deletions(-) create mode 100644 webapp-frontend/src/app/interceptor.spec.ts diff --git a/webapp-frontend/src/app/ei-coordinator/ei-job.datasource.spec.ts b/webapp-frontend/src/app/ei-coordinator/ei-job.datasource.spec.ts index dc51387..e52ae6f 100644 --- a/webapp-frontend/src/app/ei-coordinator/ei-job.datasource.spec.ts +++ b/webapp-frontend/src/app/ei-coordinator/ei-job.datasource.spec.ts @@ -22,7 +22,6 @@ import { BehaviorSubject, of } from 'rxjs'; import { EIJobDataSource } from './ei-job.datasource'; import { EIService } from '../services/ei/ei.service'; -import { NotificationService } from '../services/ui/notification.service'; import { ToastrModule } from 'ngx-toastr'; import { EIJob } from '../interfaces/ei.types'; @@ -30,7 +29,7 @@ describe('EIJobDataSource', () => { let dataSource: EIJobDataSource; let eiServiceSpy: any; - let job = { ei_job_identity: '1', ei_job_data: 'data', ei_type_identity: 'Type ID 1', target_uri: 'hhtp://url', owner: 'owner'}; + const job = { ei_job_identity: '1', ei_job_data: 'data', ei_type_identity: 'Type ID 1', target_uri: 'hhtp://url', owner: 'owner'}; beforeEach(() => { eiServiceSpy = jasmine.createSpyObj('EIService', ['getProducerIds', 'getJobsForProducer']); @@ -40,8 +39,7 @@ describe('EIJobDataSource', () => { TestBed.configureTestingModule({ imports: [ToastrModule.forRoot()], providers: [ - { provide: EIService, useValue: eiServiceSpy }, - NotificationService + { provide: EIService, useValue: eiServiceSpy } ] }); }); diff --git a/webapp-frontend/src/app/ei-coordinator/ei-job.datasource.ts b/webapp-frontend/src/app/ei-coordinator/ei-job.datasource.ts index ef9e478..64c948c 100644 --- a/webapp-frontend/src/app/ei-coordinator/ei-job.datasource.ts +++ b/webapp-frontend/src/app/ei-coordinator/ei-job.datasource.ts @@ -18,17 +18,13 @@ * ========================LICENSE_END=================================== */ -import { HttpErrorResponse } from '@angular/common/http'; import { Injectable } from '@angular/core'; import { MatTableDataSource } from '@angular/material'; import { BehaviorSubject } from 'rxjs/BehaviorSubject'; -import { of } from 'rxjs/observable/of'; -import { catchError, finalize, map } from 'rxjs/operators'; import { EIJob } from '../interfaces/ei.types'; import { EIService } from '../services/ei/ei.service'; -import { NotificationService } from '../services/ui/notification.service'; @Injectable({ providedIn: 'root' @@ -45,21 +41,13 @@ export class EIJobDataSource extends MatTableDataSource { public rowCount = 1; // hide footer during intial load constructor( - private eiSvc: EIService, - private notificationService: NotificationService) { + private eiSvc: EIService) { super(); } getJobs() { this.loadingSubject.next(true); this.eiSvc.getProducerIds() - .pipe( - catchError((her: HttpErrorResponse) => { - this.notificationService.error('Failed to get EI jobs: ' + her.error); - return of([]); - }), - finalize(() => this.loadingSubject.next(false)) - ) .subscribe((producerIds: string[]) => { producerIds.forEach(id => { this.getJobsForProducer(id); diff --git a/webapp-frontend/src/app/ei-coordinator/ei-producer.datasource.spec.ts b/webapp-frontend/src/app/ei-coordinator/ei-producer.datasource.spec.ts index 107daae..d5ab614 100644 --- a/webapp-frontend/src/app/ei-coordinator/ei-producer.datasource.spec.ts +++ b/webapp-frontend/src/app/ei-coordinator/ei-producer.datasource.spec.ts @@ -21,7 +21,6 @@ import { TestBed } from '@angular/core/testing'; import { BehaviorSubject, of } from 'rxjs'; import { EIService } from '../services/ei/ei.service'; -import { NotificationService } from '../services/ui/notification.service'; import { ToastrModule } from 'ngx-toastr'; import { EIProducer, OperationalState, ProducerRegistrationInfo, ProducerStatus } from '../interfaces/ei.types'; import { EIProducerDataSource } from './ei-producer.datasource'; @@ -63,8 +62,7 @@ describe('EIProducerDataSource', () => { TestBed.configureTestingModule({ imports: [ToastrModule.forRoot()], providers: [ - { provide: EIService, useValue: eiServiceSpy }, - NotificationService + { provide: EIService, useValue: eiServiceSpy } ] }); }); diff --git a/webapp-frontend/src/app/ei-coordinator/ei-producer.datasource.ts b/webapp-frontend/src/app/ei-coordinator/ei-producer.datasource.ts index bd7f276..78ddd4e 100644 --- a/webapp-frontend/src/app/ei-coordinator/ei-producer.datasource.ts +++ b/webapp-frontend/src/app/ei-coordinator/ei-producer.datasource.ts @@ -18,18 +18,15 @@ * ========================LICENSE_END=================================== */ -import { HttpErrorResponse } from '@angular/common/http'; import { Injectable } from '@angular/core'; import { MatTableDataSource } from '@angular/material'; import { Observable } from 'rxjs/Observable'; import { BehaviorSubject } from 'rxjs/BehaviorSubject'; import { of } from 'rxjs/observable/of'; -import { catchError, finalize, tap } from 'rxjs/operators'; import { EIProducer } from '../interfaces/ei.types'; import { EIService } from '../services/ei/ei.service'; -import { NotificationService } from '../services/ui/notification.service'; @Injectable({ providedIn: 'root' @@ -46,8 +43,7 @@ export class EIProducerDataSource extends MatTableDataSource { public rowCount = 1; // hide footer during intial load constructor( - private eiSvc: EIService, - private notificationService: NotificationService) { + private eiSvc: EIService) { super(); } @@ -55,13 +51,6 @@ export class EIProducerDataSource extends MatTableDataSource { this.loadingSubject.next(true); let producers: Array = []; this.eiSvc.getProducerIds() - .pipe( - catchError((her: HttpErrorResponse) => { - this.notificationService.error('Failed to get producers: ' + her.error); - return of([]); - }), - finalize(() => this.loadingSubject.next(false)) - ) .subscribe((prodIds: string[]) => { console.log("ProducerIds: " + prodIds); prodIds.forEach(id => { diff --git a/webapp-frontend/src/app/interceptor.spec.ts b/webapp-frontend/src/app/interceptor.spec.ts new file mode 100644 index 0000000..3c35a71 --- /dev/null +++ b/webapp-frontend/src/app/interceptor.spec.ts @@ -0,0 +1,107 @@ +/*- + * ========================LICENSE_START================================= + * O-RAN-SC + * %% + * Copyright (C) 2021 Nordix Foundation + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * ========================LICENSE_END=================================== + */ + +import { TestBed } from '@angular/core/testing'; +import { + HttpClientTestingModule, + HttpTestingController, +} from '@angular/common/http/testing'; +import { HTTP_INTERCEPTORS } from '@angular/common/http'; +import { HttpRequestInterceptor } from './interceptor'; +import { NotificationService } from './services/ui/notification.service'; +import { EIService } from './services/ei/ei.service'; +import { of } from 'rxjs/observable/of'; + +describe(`HttpRequestInterceptor`, () => { + let service: EIService; + const notificationServiceSpy = jasmine.createSpyObj('NotificationService', [ 'error' ]); + let httpMock: HttpTestingController; + + beforeEach(() => { + + TestBed.configureTestingModule({ + imports: [HttpClientTestingModule], + providers: [ + { + provide: NotificationService, useValue: notificationServiceSpy + }, + { + provide: HTTP_INTERCEPTORS, + useClass: HttpRequestInterceptor, + multi: true, + }, + EIService + ] + }); + + httpMock = TestBed.get(HttpTestingController); + service = TestBed.get(EIService); + }); + + it('should create', () => { + const interceptors = TestBed.get(HTTP_INTERCEPTORS); + let found = false; + interceptors.forEach(interceptor => { + if (interceptor instanceof HttpRequestInterceptor) { + found = true; + }; + }); + expect(found).toBeTruthy(); + }) + + it('should pass through when ok', () => { + let response: [ 'roducer1' ]; + + spyOn(service, 'getProducerIds').and.returnValue(of(response)); + service.getProducerIds().subscribe(() => { + ids => expect(ids).toEqual(response); + }); + + httpMock.verify(); + }); + + it('should notify when error', () => { + let response: any; + let errResponse: any; + + service.getProducerIds().subscribe(res => response = res, err => errResponse = err); + + const data = 'Invalid request parameters'; + const mockErrorResponse = { status: 400, statusText: 'Bad Request' }; + httpMock.expectOne(`/ei-producer/v1/eiproducers`).flush(data, mockErrorResponse); + + httpMock.verify(); + + expect(notificationServiceSpy.error).toHaveBeenCalledWith(containsString('Bad Request')); + }); + + function containsString(msg: string) { + return { + asymmetricMatch: function(compareTo: string) { + return compareTo.includes(msg); + }, + + jasmineToString: function() { + return ''; + } + }; + } +}); + diff --git a/webapp-frontend/src/app/interceptor.ts b/webapp-frontend/src/app/interceptor.ts index a9253f8..b1b2aa5 100644 --- a/webapp-frontend/src/app/interceptor.ts +++ b/webapp-frontend/src/app/interceptor.ts @@ -18,16 +18,27 @@ * ========================LICENSE_END=================================== */ -import { Injectable, Injector } from '@angular/core'; -import { HttpEvent, HttpHandler, HttpInterceptor, HttpRequest, HttpResponse } from '@angular/common/http'; -import { Observable, of } from 'rxjs'; +import { Injectable } from '@angular/core'; +import { HttpErrorResponse, HttpEvent, HttpHandler, HttpInterceptor, HttpRequest } from '@angular/common/http'; +import { Observable } from 'rxjs'; +import { catchError } from 'rxjs/operators'; +import { throwError } from 'rxjs'; +import { NotificationService } from './services/ui/notification.service'; @Injectable() export class HttpRequestInterceptor implements HttpInterceptor { - constructor(private injector: Injector) {} + constructor(private notificationService: NotificationService) {} - intercept(request: HttpRequest, next: HttpHandler): Observable> { + intercept(request: HttpRequest, next: HttpHandler): Observable> { console.log('Interceptor Invoked' + request.url); - return next.handle(request); + return next.handle(request).pipe( + catchError((error: HttpErrorResponse) => { + console.error("Error from error interceptor", error); + + // show dialog for error message + this.notificationService.error(error.message); + return throwError(error); + }) + ) as Observable>; } } diff --git a/webapp-frontend/src/app/policy-control/policy-type.datasource.spec.ts b/webapp-frontend/src/app/policy-control/policy-type.datasource.spec.ts index 5b83368..aa759bc 100644 --- a/webapp-frontend/src/app/policy-control/policy-type.datasource.spec.ts +++ b/webapp-frontend/src/app/policy-control/policy-type.datasource.spec.ts @@ -20,7 +20,6 @@ import { TestBed } from '@angular/core/testing'; import { BehaviorSubject, of } from 'rxjs'; -import { NotificationService } from '../services/ui/notification.service'; import { ToastrModule } from 'ngx-toastr'; import { PolicyTypeDataSource } from './policy-type.datasource'; import { PolicyService } from '../services/policy/policy.service'; @@ -48,8 +47,7 @@ describe('PolicyTypeDataSource', () => { TestBed.configureTestingModule({ imports: [ToastrModule.forRoot()], providers: [ - { provide: PolicyService, useValue: policyServiceSpy }, - NotificationService + { provide: PolicyService, useValue: policyServiceSpy } ] }); }); diff --git a/webapp-frontend/src/app/policy-control/policy-type.datasource.ts b/webapp-frontend/src/app/policy-control/policy-type.datasource.ts index 2e10f55..a7aa98d 100644 --- a/webapp-frontend/src/app/policy-control/policy-type.datasource.ts +++ b/webapp-frontend/src/app/policy-control/policy-type.datasource.ts @@ -18,17 +18,14 @@ * ========================LICENSE_END=================================== */ -import { HttpErrorResponse } from '@angular/common/http'; import { CollectionViewer, DataSource } from '@angular/cdk/collections'; import { Injectable } from '@angular/core'; import { BehaviorSubject } from 'rxjs/BehaviorSubject'; import { of } from 'rxjs/observable/of'; import { Observable } from 'rxjs/Observable'; -import { catchError, finalize, map } from 'rxjs/operators'; import { PolicyType, PolicyTypes, PolicyTypeSchema } from '../interfaces/policy.types'; import { PolicyService } from '../services/policy/policy.service'; -import { NotificationService } from '../services/ui/notification.service'; @Injectable({ providedIn: 'root' @@ -40,25 +37,15 @@ export class PolicyTypeDataSource extends DataSource { policyTypeSubject = new BehaviorSubject([]); - private loadingSubject = new BehaviorSubject(false); - public rowCount = 1; // hide footer during intial load - constructor(public policySvc: PolicyService, - private notificationService: NotificationService) { + constructor(public policySvc: PolicyService) { super(); } public getPolicyTypes() { this.policyTypes = [] as PolicyTypeSchema[]; this.policySvc.getPolicyTypes() - .pipe( - catchError((httpError: HttpErrorResponse) => { - this.notificationService.error('Failed to get policy types: ' + httpError.error); - return of([]); - }), - finalize(() => this.loadingSubject.next(false)) - ) .subscribe((policyType: PolicyTypes) => { this.rowCount = policyType.policytype_ids.length; if (policyType.policytype_ids.length != 0) { @@ -72,13 +59,6 @@ export class PolicyTypeDataSource extends DataSource { } else { this.policySvc.getPolicyType(policyTypeId) - .pipe( - catchError((httpError: HttpErrorResponse) => { - this.notificationService.error('Failed to get policy type: ' + httpError.error); - return of([]); - }), - finalize(() => this.loadingSubject.next(false)) - ) .subscribe((policyType: PolicyType) => { policyTypeSchema.id = policyTypeId; policyTypeSchema.schemaObject = policyType.policy_schema; -- 2.16.6