From: Stephan Müller Date: Thu, 22 Nov 2018 13:44:36 +0000 (+0100) Subject: mgr/dashboard: Settings service X-Git-Tag: v14.1.0~655^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=b32feea6f4a0b769b645fb506f5b9857a4e0fde6;p=ceph.git mgr/dashboard: Settings service 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 --- 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); }