From: Stephan Müller Date: Mon, 31 Aug 2020 16:13:02 +0000 (+0200) Subject: mgr/dashboard: Refactor of RBD form X-Git-Tag: v16.1.0~801^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=4cf1bc62edaa3eab7c08b87bf5d57c1e1b7684b2;p=ceph.git mgr/dashboard: Refactor of RBD form Refactors unit tests and component. Fixes: https://tracker.ceph.com/issues/37408 Signed-off-by: Stephan Müller --- diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-form/rbd-form.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-form/rbd-form.component.spec.ts index f9bbd4c112b..dad872435d0 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-form/rbd-form.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-form/rbd-form.component.spec.ts @@ -11,18 +11,40 @@ import { delay } from 'rxjs/operators'; import { ActivatedRouteStub } from '../../../../testing/activated-route-stub'; import { configureTestBed } from '../../../../testing/unit-test-helper'; +import { PoolService } from '../../../shared/api/pool.service'; import { RbdService } from '../../../shared/api/rbd.service'; import { ImageSpec } from '../../../shared/models/image-spec'; import { SharedModule } from '../../../shared/shared.module'; +import { Pool } from '../../pool/pool'; import { RbdConfigurationFormComponent } from '../rbd-configuration-form/rbd-configuration-form.component'; import { RbdImageFeature } from './rbd-feature.interface'; import { RbdFormMode } from './rbd-form-mode.enum'; +import { RbdFormResponseModel } from './rbd-form-response.model'; import { RbdFormComponent } from './rbd-form.component'; describe('RbdFormComponent', () => { + const urlPrefix = { + create: '/block/rbd/create', + edit: '/block/rbd/edit', + clone: '/block/rbd/clone', + copy: '/block/rbd/copy' + }; let component: RbdFormComponent; let fixture: ComponentFixture; let activatedRoute: ActivatedRouteStub; + const mock: { rbd: RbdFormResponseModel; pools: Pool[]; defaultFeatures: string[] } = { + rbd: {} as RbdFormResponseModel, + pools: [], + defaultFeatures: [] + }; + + const setRouterUrl = ( + action: 'create' | 'edit' | 'clone' | 'copy', + poolName?: string, + imageName?: string + ) => { + component['routerUrl'] = [urlPrefix[action], poolName, imageName].filter((x) => x).join('/'); + }; const queryNativeElement = (cssSelector: string) => fixture.debugElement.query(By.css(cssSelector)).nativeElement; @@ -67,6 +89,19 @@ describe('RbdFormComponent', () => { const DELAY = 100; + const getPool = ( + pool_name: string, + type: 'replicated' | 'erasure', + flags_names: string, + application_metadata: string[] + ): Pool => + ({ + pool_name, + flags_names, + application_metadata, + type + } as Pool); + beforeEach(() => { createAction = spyOn(component, 'createAction').and.returnValue(of(null)); editAction = spyOn(component, 'editAction'); @@ -75,8 +110,17 @@ describe('RbdFormComponent', () => { copyAction = spyOn(component, 'copyAction').and.returnValue(of(null)); spyOn(component, 'setResponse').and.stub(); routerNavigate = spyOn(TestBed.inject(Router), 'navigate').and.stub(); + mock.pools = [ + getPool('one', 'replicated', '', []), + getPool('two', 'replicated', '', ['rbd']), + getPool('three', 'replicated', '', ['rbd']), + getPool('four', 'erasure', '', ['rbd']), + getPool('four', 'erasure', 'ec_overwrites', ['rbd']) + ]; + spyOn(TestBed.inject(PoolService), 'list').and.callFake(() => of(mock.pools)); rbdServiceGetSpy = spyOn(TestBed.inject(RbdService), 'get'); - rbdServiceGetSpy.and.returnValue(of({ pool_name: 'foo', pool_image: 'bar' })); + mock.rbd = ({ pool_name: 'foo', pool_image: 'bar' } as any) as RbdFormResponseModel; + rbdServiceGetSpy.and.returnValue(of(mock.rbd)); component.mode = undefined; }); @@ -92,13 +136,13 @@ describe('RbdFormComponent', () => { }); it('should unsubscribe right after image data is received', () => { - component.mode = RbdFormMode.editing; - rbdServiceGetSpy.and.returnValue(of({ pool_name: 'foo', pool_image: 'bar' })); + setRouterUrl('edit', 'foo', 'bar'); + rbdServiceGetSpy.and.returnValue(of(mock.rbd)); editAction.and.returnValue(NEVER); - component.ngOnInit(); - component.submit(); - expect(component['rbdImage'].observers.length).toEqual(0); + component.ngOnInit(); // Subscribes to image once during init + component.submit(); + expect(component['rbdImage'].observers.length).toEqual(1); expect(createAction).toHaveBeenCalledTimes(0); expect(editAction).toHaveBeenCalledTimes(1); expect(cloneAction).toHaveBeenCalledTimes(0); @@ -107,10 +151,8 @@ describe('RbdFormComponent', () => { }); it('should not edit image if no image data is received', fakeAsync(() => { - component.mode = RbdFormMode.editing; - rbdServiceGetSpy.and.returnValue( - of({ pool_name: 'foo', pool_image: 'bar' }).pipe(delay(DELAY)) - ); + setRouterUrl('edit', 'foo', 'bar'); + rbdServiceGetSpy.and.returnValue(of(mock.rbd).pipe(delay(DELAY))); component.ngOnInit(); component.submit(); @@ -154,7 +196,7 @@ describe('RbdFormComponent', () => { }); it('should edit image after image data is received', () => { - component.mode = RbdFormMode.editing; + setRouterUrl('edit', 'foo', 'bar'); component.ngOnInit(); component.submit(); @@ -166,10 +208,8 @@ describe('RbdFormComponent', () => { }); it('should not clone image if no image data is received', fakeAsync(() => { - component.mode = RbdFormMode.cloning; - rbdServiceGetSpy.and.returnValue( - of({ pool_name: 'foo', pool_image: 'bar' }).pipe(delay(DELAY)) - ); + setRouterUrl('clone', 'foo', 'bar'); + rbdServiceGetSpy.and.returnValue(of(mock.rbd).pipe(delay(DELAY))); component.ngOnInit(); component.submit(); @@ -183,7 +223,7 @@ describe('RbdFormComponent', () => { })); it('should clone image after image data is received', () => { - component.mode = RbdFormMode.cloning; + setRouterUrl('clone', 'foo', 'bar'); component.ngOnInit(); component.submit(); @@ -195,10 +235,8 @@ describe('RbdFormComponent', () => { }); it('should not copy image if no image data is received', fakeAsync(() => { - component.mode = RbdFormMode.copying; - rbdServiceGetSpy.and.returnValue( - of({ pool_name: 'foo', pool_image: 'bar' }).pipe(delay(DELAY)) - ); + setRouterUrl('copy', 'foo', 'bar'); + rbdServiceGetSpy.and.returnValue(of(mock.rbd).pipe(delay(DELAY))); component.ngOnInit(); component.submit(); @@ -212,7 +250,7 @@ describe('RbdFormComponent', () => { })); it('should copy image after image data is received', () => { - component.mode = RbdFormMode.copying; + setRouterUrl('copy', 'foo', 'bar'); component.ngOnInit(); component.submit(); @@ -319,7 +357,7 @@ describe('RbdFormComponent', () => { }) ); spyOn(rbdService, 'defaultFeatures').and.returnValue(of(defaultFeatures)); - component.router = { url: `/block/rbd/edit/${pool}/${image}` } as Router; + setRouterUrl('edit', pool, image); fixture.detectChanges(); [ deepFlatten, @@ -369,7 +407,7 @@ describe('RbdFormComponent', () => { beforeEach(() => { const rbdService = TestBed.inject(RbdService); spyOn(rbdService, 'defaultFeatures').and.returnValue(of(defaultFeatures)); - component.router = { url: '/block/rbd/create' } as Router; + setRouterUrl('create'); fixture.detectChanges(); [ deepFlatten, diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-form/rbd-form.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-form/rbd-form.component.ts index 676922c43d7..eec514bec79 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-form/rbd-form.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-form/rbd-form.component.ts @@ -96,6 +96,7 @@ export class RbdFormComponent extends CdForm implements OnInit { action: string; resource: string; private rbdImage = new ReplaySubject(1); + private routerUrl: string; icons = Icons; @@ -108,9 +109,10 @@ export class RbdFormComponent extends CdForm implements OnInit { private taskWrapper: TaskWrapperService, private dimlessBinaryPipe: DimlessBinaryPipe, public actionLabels: ActionLabelsI18n, - public router: Router + private router: Router ) { super(); + this.routerUrl = this.router.url; this.poolPermission = this.authStorageService.getPermissions().pool; this.resource = $localize`RBD`; this.features = { @@ -231,7 +233,7 @@ export class RbdFormComponent extends CdForm implements OnInit { } private prepareFormForAction() { - const url = this.router.url; + const url = this.routerUrl; if (url.startsWith('/block/rbd/edit')) { this.mode = this.rbdFormMode.editing; this.action = this.actionLabels.EDIT; @@ -331,15 +333,15 @@ export class RbdFormComponent extends CdForm implements OnInit { } onPoolChange(selectedPoolName: string) { - const newDataPools = this.allDataPools + const dataPoolControl = this.rbdForm.get('dataPool'); + if (dataPoolControl.value === selectedPoolName) { + dataPoolControl.setValue(null); + } + this.dataPools = this.allDataPools ? this.allDataPools.filter((dataPool: any) => { return dataPool.pool_name !== selectedPoolName; }) : []; - if (this.rbdForm.getValue('dataPool') === selectedPoolName) { - this.rbdForm.get('dataPool').setValue(null); - } - this.dataPools = newDataPools; this.namespaces = null; if (selectedPoolName in this.namespacesByPoolCache) { this.namespaces = this.namespacesByPoolCache[selectedPoolName]; @@ -422,12 +424,8 @@ export class RbdFormComponent extends CdForm implements OnInit { }; } - protected getDependendChildFeatures(featureKey: string) { - return _.filter(this.features, (f) => f.requires === featureKey) || []; - } - deepBoxCheck(key: string, checked: boolean) { - const childFeatures = this.getDependendChildFeatures(key); + const childFeatures = this.getDependentChildFeatures(key); childFeatures.forEach((feature) => { const featureControl = this.rbdForm.get(feature.key); if (checked) { @@ -452,6 +450,10 @@ export class RbdFormComponent extends CdForm implements OnInit { }); } + protected getDependentChildFeatures(featureKey: string) { + return _.filter(this.features, (f) => f.requires === featureKey) || []; + } + interlockCheck(key: string, checked: boolean) { // Adds a compatibility layer for Ceph cluster where the feature interlock of features hasn't // been implemented yet. It disables the feature interlock for images which only have one of @@ -556,7 +558,6 @@ export class RbdFormComponent extends CdForm implements OnInit { .get('stripingUnit') .setValue(this.dimlessBinaryPipe.transform(response.stripe_unit)); this.rbdForm.get('stripingCount').setValue(response.stripe_count); - /* Configuration */ this.initializeConfigData.emit({ initialData: this.response.configuration, @@ -570,6 +571,14 @@ export class RbdFormComponent extends CdForm implements OnInit { request.namespace = this.rbdForm.getValue('namespace'); request.name = this.rbdForm.getValue('name'); request.size = this.formatter.toBytes(this.rbdForm.getValue('size')); + this.addObjectSizeAndStripingToRequest(request); + request.configuration = this.getDirtyConfigurationValues(); + return request; + } + + private addObjectSizeAndStripingToRequest( + request: RbdFormCreateRequestModel | RbdFormCloneRequestModel | RbdFormCopyRequestModel + ) { request.obj_size = this.formatter.toBytes(this.rbdForm.getValue('obj_size')); _.forIn(this.features, (feature) => { if (this.rbdForm.getValue(feature.key)) { @@ -581,11 +590,6 @@ export class RbdFormComponent extends CdForm implements OnInit { request.stripe_unit = this.formatter.toBytes(this.rbdForm.getValue('stripingUnit')); request.stripe_count = this.rbdForm.getValue('stripingCount'); request.data_pool = this.rbdForm.getValue('dataPool'); - - /* Configuration */ - request.configuration = this.getDirtyConfigurationValues(); - - return request; } createAction(): Observable { @@ -609,9 +613,7 @@ export class RbdFormComponent extends CdForm implements OnInit { request.features.push(feature.key); } }); - request.configuration = this.getDirtyConfigurationValues(); - return request; } @@ -620,24 +622,11 @@ export class RbdFormComponent extends CdForm implements OnInit { request.child_pool_name = this.rbdForm.getValue('pool'); request.child_namespace = this.rbdForm.getValue('namespace'); request.child_image_name = this.rbdForm.getValue('name'); - request.obj_size = this.formatter.toBytes(this.rbdForm.getValue('obj_size')); - _.forIn(this.features, (feature) => { - if (this.rbdForm.getValue(feature.key)) { - request.features.push(feature.key); - } - }); - - /* Striping */ - request.stripe_unit = this.formatter.toBytes(this.rbdForm.getValue('stripingUnit')); - request.stripe_count = this.rbdForm.getValue('stripingCount'); - request.data_pool = this.rbdForm.getValue('dataPool'); - - /* Configuration */ + this.addObjectSizeAndStripingToRequest(request); request.configuration = this.getDirtyConfigurationValues( true, RbdConfigurationSourceField.image ); - return request; } @@ -682,24 +671,11 @@ export class RbdFormComponent extends CdForm implements OnInit { request.dest_pool_name = this.rbdForm.getValue('pool'); request.dest_namespace = this.rbdForm.getValue('namespace'); request.dest_image_name = this.rbdForm.getValue('name'); - request.obj_size = this.formatter.toBytes(this.rbdForm.getValue('obj_size')); - _.forIn(this.features, (feature) => { - if (this.rbdForm.getValue(feature.key)) { - request.features.push(feature.key); - } - }); - - /* Striping */ - request.stripe_unit = this.formatter.toBytes(this.rbdForm.getValue('stripingUnit')); - request.stripe_count = this.rbdForm.getValue('stripingCount'); - request.data_pool = this.rbdForm.getValue('dataPool'); - - /* Configuration */ + this.addObjectSizeAndStripingToRequest(request); request.configuration = this.getDirtyConfigurationValues( true, RbdConfigurationSourceField.image ); - return request; }