From b32feea6f4a0b769b645fb506f5b9857a4e0fde6 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Stephan=20M=C3=BCller?= Date: Thu, 22 Nov 2018 14:44:36 +0100 Subject: [PATCH] mgr/dashboard: Settings service MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The settings service handles settings that are set within the dashboard. The service has a helpful function and a good memory. The function will allow to check if a certain option is set, if it's set the callback will be triggered. If the API request was made before for this particular option, no API request is made anymore, as the service will remember if the state is set or not. Fixes: https://tracker.ceph.com/issues/37471 Signed-off-by: Stephan Müller --- .../app/shared/api/grafana.service.spec.ts | 34 ------ .../src/app/shared/api/grafana.service.ts | 15 --- .../app/shared/api/settings.service.spec.ts | 109 ++++++++++++++++++ .../src/app/shared/api/settings.service.ts | 30 +++++ .../grafana/grafana.component.spec.ts | 33 +++++- .../components/grafana/grafana.component.ts | 21 ++-- 6 files changed, 175 insertions(+), 67 deletions(-) delete mode 100644 src/pybind/mgr/dashboard/frontend/src/app/shared/api/grafana.service.spec.ts delete mode 100755 src/pybind/mgr/dashboard/frontend/src/app/shared/api/grafana.service.ts create mode 100644 src/pybind/mgr/dashboard/frontend/src/app/shared/api/settings.service.spec.ts create mode 100755 src/pybind/mgr/dashboard/frontend/src/app/shared/api/settings.service.ts diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/grafana.service.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/grafana.service.spec.ts deleted file mode 100644 index 4deecf42177a..000000000000 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/grafana.service.spec.ts +++ /dev/null @@ -1,34 +0,0 @@ -import { HttpClientTestingModule, HttpTestingController } from '@angular/common/http/testing'; -import { TestBed } from '@angular/core/testing'; - -import { configureTestBed } from '../../../testing/unit-test-helper'; -import { GrafanaService } from './grafana.service'; - -describe('GrafanaService', () => { - let service: GrafanaService; - let httpTesting: HttpTestingController; - - configureTestBed({ - providers: [GrafanaService], - imports: [HttpClientTestingModule] - }); - - beforeEach(() => { - service = TestBed.get(GrafanaService); - httpTesting = TestBed.get(HttpTestingController); - }); - - afterEach(() => { - httpTesting.verify(); - }); - - it('should be created', () => { - expect(service).toBeTruthy(); - }); - - it('should get protocol', () => { - service.getGrafanaApiUrl().subscribe(); - const req = httpTesting.expectOne('api/grafana/url'); - expect(req.request.method).toBe('GET'); - }); -}); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/grafana.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/grafana.service.ts deleted file mode 100755 index 6a37c2179ed1..000000000000 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/grafana.service.ts +++ /dev/null @@ -1,15 +0,0 @@ -import { HttpClient } from '@angular/common/http'; -import { Injectable } from '@angular/core'; - -import { ApiModule } from './api.module'; - -@Injectable({ - providedIn: ApiModule -}) -export class GrafanaService { - constructor(private http: HttpClient) {} - - getGrafanaApiUrl() { - return this.http.get('api/grafana/url'); - } -} diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/settings.service.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/settings.service.spec.ts new file mode 100644 index 000000000000..10dee3506e14 --- /dev/null +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/settings.service.spec.ts @@ -0,0 +1,109 @@ +import { HttpClientTestingModule, HttpTestingController } from '@angular/common/http/testing'; +import { fakeAsync, TestBed, tick } from '@angular/core/testing'; + +import { configureTestBed } from '../../../testing/unit-test-helper'; +import { SettingsService } from './settings.service'; + +describe('SettingsService', () => { + let service: SettingsService; + let httpTesting: HttpTestingController; + + configureTestBed( + { + providers: [SettingsService], + imports: [HttpClientTestingModule] + }, + true + ); + + beforeEach(() => { + service = TestBed.get(SettingsService); + httpTesting = TestBed.get(HttpTestingController); + }); + + afterEach(() => { + httpTesting.verify(); + }); + + it('should be created', () => { + expect(service).toBeTruthy(); + }); + + describe('getSettingsValue', () => { + const testMethod = (data, expected: string) => { + expect(service['getSettingsValue'](data)).toBe(expected); + }; + + it('should explain the logic of the method', () => { + expect('' || undefined).toBe(undefined); + expect(undefined || '').toBe(''); + expect('test' || undefined || '').toBe('test'); + }); + + it('should test the method for empty string values', () => { + testMethod({}, ''); + testMethod({ wrongAttribute: 'test' }, ''); + testMethod({ value: '' }, ''); + testMethod({ instance: '' }, ''); + }); + + it('should test the method for non empty string values', () => { + testMethod({ value: 'test' }, 'test'); + testMethod({ instance: 'test' }, 'test'); + }); + }); + + describe('isSettingConfigured', () => { + const exampleUrl = 'api/settings/something'; + const exampleValue = 'http://localhost:3000'; + let increment: number; + + const testConfig = (url, value) => { + service.ifSettingConfigured(url, (setValue) => { + expect(setValue).toBe(value); + increment++; + }); + }; + + const expectSettingsApiCall = (url: string, value: object, isSet: string) => { + testConfig(url, isSet); + const req = httpTesting.expectOne(url); + expect(req.request.method).toBe('GET'); + req.flush(value); + tick(); + expect(increment).toBe(isSet !== '' ? 1 : 0); + expect(service['settings'][url]).toBe(isSet); + }; + + beforeEach(() => { + increment = 0; + }); + + it(`should return true if 'value' does not contain an empty string`, fakeAsync(() => { + expectSettingsApiCall(exampleUrl, { value: exampleValue }, exampleValue); + })); + + it(`should return false if 'value' does contain an empty string`, fakeAsync(() => { + expectSettingsApiCall(exampleUrl, { value: '' }, ''); + })); + + it(`should return true if 'instance' does not contain an empty string`, fakeAsync(() => { + expectSettingsApiCall(exampleUrl, { value: exampleValue }, exampleValue); + })); + + it(`should return false if 'instance' does contain an empty string`, fakeAsync(() => { + expectSettingsApiCall(exampleUrl, { instance: '' }, ''); + })); + + it(`should return false if the api object is empty`, fakeAsync(() => { + expectSettingsApiCall(exampleUrl, {}, ''); + })); + + it(`should call the API once even if it is called multiple times`, fakeAsync(() => { + expectSettingsApiCall(exampleUrl, { value: exampleValue }, exampleValue); + testConfig(exampleUrl, exampleValue); + httpTesting.expectNone(exampleUrl); + expect(increment).toBe(2); + })); + }); +}); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/settings.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/settings.service.ts new file mode 100755 index 000000000000..9385c05f1111 --- /dev/null +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/settings.service.ts @@ -0,0 +1,30 @@ +import { HttpClient } from '@angular/common/http'; +import { Injectable } from '@angular/core'; + +import * as _ from 'lodash'; +import { ApiModule } from './api.module'; + +@Injectable({ + providedIn: ApiModule +}) +export class SettingsService { + constructor(private http: HttpClient) {} + + private settings: { [url: string]: string } = {}; + + ifSettingConfigured(url: string, fn: (value?: string) => void): void { + const setting = this.settings[url]; + if (setting === undefined) { + this.http.get(url).subscribe((data: any) => { + this.settings[url] = this.getSettingsValue(data); + this.ifSettingConfigured(url, fn); + }); + } else if (setting !== '') { + fn(setting); + } + } + + private getSettingsValue(data: any): string { + return data.value || data.instance || ''; + } +} diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/components/grafana/grafana.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/components/grafana/grafana.component.spec.ts index a295049e02f0..ea227cb1f14d 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/components/grafana/grafana.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/components/grafana/grafana.component.spec.ts @@ -5,9 +5,8 @@ import { RouterTestingModule } from '@angular/router/testing'; import { AlertModule } from 'ngx-bootstrap/alert'; import { configureTestBed, i18nProviders } from '../../../../testing/unit-test-helper'; -import { GrafanaService } from '../../../shared/api/grafana.service'; - import { SummaryService } from '../../../shared/services/summary.service'; +import { SettingsService } from '../../api/settings.service'; import { CephReleaseNamePipe } from '../../pipes/ceph-release-name.pipe'; import { InfoPanelComponent } from '../info-panel/info-panel.component'; import { LoadingPanelComponent } from '../loading-panel/loading-panel.component'; @@ -20,16 +19,42 @@ describe('GrafanaComponent', () => { configureTestBed({ declarations: [GrafanaComponent, InfoPanelComponent, LoadingPanelComponent], imports: [AlertModule.forRoot(), HttpClientTestingModule, RouterTestingModule], - providers: [CephReleaseNamePipe, GrafanaService, SummaryService, i18nProviders] + providers: [CephReleaseNamePipe, SettingsService, SummaryService, i18nProviders] }); beforeEach(() => { fixture = TestBed.createComponent(GrafanaComponent); component = fixture.componentInstance; - fixture.detectChanges(); + component.grafanaPath = 'somePath'; }); it('should create', () => { expect(component).toBeTruthy(); }); + + it('should have found out that grafana does not exist', () => { + fixture.detectChanges(); + expect(component.grafanaExist).toBe(false); + expect(component.baseUrl).toBe(undefined); + expect(component.loading).toBe(true); + expect(component.url).toBe(undefined); + expect(component.grafanaSrc).toEqual(undefined); + }); + + describe('with grafana initialized', () => { + beforeEach(() => { + TestBed.get(SettingsService).settings = { 'api/grafana/url': 'http:localhost:3000' }; + fixture.detectChanges(); + }); + + it('should have found out that grafana exists', () => { + expect(component.grafanaExist).toBe(true); + expect(component.baseUrl).toBe('http:localhost:3000/d/'); + expect(component.loading).toBe(false); + expect(component.url).toBe('http:localhost:3000/d/somePath&refresh=2s&kiosk'); + expect(component.grafanaSrc).toEqual({ + changingThisBreaksApplicationSecurity: 'http:localhost:3000/d/somePath&refresh=2s&kiosk' + }); + }); + }); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/components/grafana/grafana.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/components/grafana/grafana.component.ts index 77dcfa232eea..6bbfa47cbdd3 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/components/grafana/grafana.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/components/grafana/grafana.component.ts @@ -3,9 +3,9 @@ import { Component, Input, OnChanges, OnInit } from '@angular/core'; import { DomSanitizer } from '@angular/platform-browser'; import { SafeUrl } from '@angular/platform-browser'; -import { GrafanaService } from '../../../shared/api/grafana.service'; import { CephReleaseNamePipe } from '../../../shared/pipes/ceph-release-name.pipe'; import { SummaryService } from '../../../shared/services/summary.service'; +import { SettingsService } from '../../api/settings.service'; @Component({ selector: 'cd-grafana', @@ -32,13 +32,12 @@ export class GrafanaComponent implements OnInit, OnChanges { grafanaPath: string; @Input() grafanaStyle: string; - grafanaUrl: any; docsUrl: string; constructor( private summaryService: SummaryService, private sanitizer: DomSanitizer, - private grafanaService: GrafanaService, + private settingsService: SettingsService, private cephReleaseNamePipe: CephReleaseNamePipe ) {} @@ -64,22 +63,16 @@ export class GrafanaComponent implements OnInit, OnChanges { }, 0); }); - this.grafanaService.getGrafanaApiUrl().subscribe((data: any) => { - this.grafanaUrl = data.instance; - if (this.grafanaUrl === '') { - this.grafanaExist = false; - return; - } else { - this.getFrame(); - } + this.settingsService.ifSettingConfigured('api/grafana/url', (url) => { + this.grafanaExist = true; + this.loading = false; + this.baseUrl = url + '/d/'; + this.getFrame(); }); this.panelStyle = this.styles[this.grafanaStyle]; } getFrame() { - this.baseUrl = this.grafanaUrl + '/d/'; - this.grafanaExist = true; - this.loading = false; this.url = this.baseUrl + this.grafanaPath + '&refresh=2s' + this.mode; this.grafanaSrc = this.sanitizer.bypassSecurityTrustResourceUrl(this.url); } -- 2.47.3