mgr/dashboard: Refactor of RBD form

Refactors unit tests and component.

Fixes: https://tracker.ceph.com/issues/37408
Signed-off-by: Stephan Müller <smueller@suse.com>

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 f9bbd4c112bdb8ec71fdcd5ad33efe8515a5a121..dad872435d0d57940dd890d57bc81d6e1f4ea7a3 100644 (file)
--- 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<RbdFormComponent>;
   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 676922c43d7b57e6d71ce59d64bda2199ab44cbd..eec514bec79cc4f000f1b4e502e53274bacb2ee8 100644 (file)
--- 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<any> {
@@ -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;
   }