From 1017ac7f2c8a9e44fa7adaf864de1e2c8a0699cf Mon Sep 17 00:00:00 2001 From: Tiago Melo Date: Mon, 28 Sep 2020 08:45:35 +0000 Subject: [PATCH] mgr/dashboard: Improve Change Detection on RBD Snapshot We are now using OnPush for the RbdSnapshotList component. RbdSnapshotActionsModel is now only created once. Some data is only updated when it changes. Fixes: https://tracker.ceph.com/issues/47685 Signed-off-by: Tiago Melo --- .../rbd-snapshot-form-modal.component.ts | 10 +-- .../rbd-snapshot-actions.model.ts | 9 +- .../rbd-snapshot-list.component.spec.ts | 4 +- .../rbd-snapshot-list.component.ts | 86 +++++++++++++------ .../src/app/shared/classes/cd-helper.class.ts | 5 ++ 5 files changed, 78 insertions(+), 36 deletions(-) diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-form/rbd-snapshot-form-modal.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-form/rbd-snapshot-form-modal.component.ts index b908a7ca35fab..cb744d6056fc3 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-form/rbd-snapshot-form-modal.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-form/rbd-snapshot-form-modal.component.ts @@ -1,4 +1,4 @@ -import { Component, OnInit } from '@angular/core'; +import { Component } from '@angular/core'; import { FormControl, Validators } from '@angular/forms'; import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap'; @@ -17,7 +17,7 @@ import { TaskManagerService } from '../../../shared/services/task-manager.servic templateUrl: './rbd-snapshot-form-modal.component.html', styleUrls: ['./rbd-snapshot-form-modal.component.scss'] }) -export class RbdSnapshotFormModalComponent implements OnInit { +export class RbdSnapshotFormModalComponent { poolName: string; namespace: string; imageName: string; @@ -29,7 +29,7 @@ export class RbdSnapshotFormModalComponent implements OnInit { action: string; resource: string; - public onSubmit: Subject; + public onSubmit: Subject = new Subject(); constructor( public activeModal: NgbActiveModal, @@ -51,10 +51,6 @@ export class RbdSnapshotFormModalComponent implements OnInit { }); } - ngOnInit() { - this.onSubmit = new Subject(); - } - setSnapName(snapName: string) { this.snapName = snapName; this.snapshotForm.get('snapshotName').setValue(snapName); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-list/rbd-snapshot-actions.model.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-list/rbd-snapshot-actions.model.ts index 2b22da09aab65..0bf61061e40e1 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-list/rbd-snapshot-actions.model.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-list/rbd-snapshot-actions.model.ts @@ -18,7 +18,11 @@ export class RbdSnapshotActionsModel { cloneFormatVersion = 1; - constructor(actionLabels: ActionLabelsI18n, featuresName: string[], rbdService: RbdService) { + constructor( + actionLabels: ActionLabelsI18n, + public featuresName: string[], + rbdService: RbdService + ) { rbdService.cloneFormatVersion().subscribe((version: number) => { this.cloneFormatVersion = version; }); @@ -50,7 +54,8 @@ export class RbdSnapshotActionsModel { this.clone = { permission: 'create', canBePrimary: (selection: CdTableSelection) => selection.hasSingleSelection, - disable: (selection: CdTableSelection) => this.getCloneDisableDesc(selection, featuresName), + disable: (selection: CdTableSelection) => + this.getCloneDisableDesc(selection, this.featuresName), icon: Icons.clone, name: actionLabels.CLONE }; diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-list/rbd-snapshot-list.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-list/rbd-snapshot-list.component.spec.ts index 44f4130183f0c..fec0995ca1fb0 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-list/rbd-snapshot-list.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-list/rbd-snapshot-list.component.spec.ts @@ -111,7 +111,8 @@ describe('RbdSnapshotListComponent', () => { notificationService, null, null, - actionLabelsI18n + actionLabelsI18n, + null ); spyOn(rbdService, 'deleteSnapshot').and.returnValue(observableThrowError({ status: 500 })); spyOn(notificationService, 'notifyTask').and.stub(); @@ -227,6 +228,7 @@ describe('RbdSnapshotListComponent', () => { }); it('should test all TableActions combinations', () => { + component.ngOnInit(); const permissionHelper: PermissionHelper = new PermissionHelper(component.permission); const tableActions: TableActionsComponent = permissionHelper.setPermissionsAndGetActions( component.tableActions diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-list/rbd-snapshot-list.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-list/rbd-snapshot-list.component.ts index d14136c7c888d..26542b55321dd 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-list/rbd-snapshot-list.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-snapshot-list/rbd-snapshot-list.component.ts @@ -1,10 +1,20 @@ -import { Component, Input, OnChanges, OnInit, TemplateRef, ViewChild } from '@angular/core'; +import { + ChangeDetectionStrategy, + ChangeDetectorRef, + Component, + Input, + OnChanges, + OnInit, + TemplateRef, + ViewChild +} from '@angular/core'; import { NgbModalRef } from '@ng-bootstrap/ng-bootstrap'; import moment from 'moment'; import { of } from 'rxjs'; import { RbdService } from '../../../shared/api/rbd.service'; +import { CdHelperClass } from '../../../shared/classes/cd-helper.class'; import { ConfirmationModalComponent } from '../../../shared/components/confirmation-modal/confirmation-modal.component'; import { CriticalConfirmationModalComponent } from '../../../shared/components/critical-confirmation-modal/critical-confirmation-modal.component'; import { ActionLabelsI18n } from '../../../shared/constants/app.constants'; @@ -33,7 +43,8 @@ import { RbdSnapshotModel } from './rbd-snapshot.model'; selector: 'cd-rbd-snapshot-list', templateUrl: './rbd-snapshot-list.component.html', styleUrls: ['./rbd-snapshot-list.component.scss'], - providers: [TaskListService] + providers: [TaskListService], + changeDetection: ChangeDetectionStrategy.OnPush }) export class RbdSnapshotListComponent implements OnInit, OnChanges { @Input() @@ -54,6 +65,8 @@ export class RbdSnapshotListComponent implements OnInit, OnChanges { permission: Permission; selection = new CdTableSelection(); tableActions: CdTableAction[]; + rbdTableActions: RbdSnapshotActionsModel; + imageSpec: ImageSpec; data: RbdSnapshotModel[]; @@ -79,7 +92,8 @@ export class RbdSnapshotListComponent implements OnInit, OnChanges { private notificationService: NotificationService, private summaryService: SummaryService, private taskListService: TaskListService, - private actionLabels: ActionLabelsI18n + private actionLabels: ActionLabelsI18n, + private cdr: ChangeDetectorRef ) { this.permission = this.authStorageService.getPermissions().rbdImage; } @@ -125,28 +139,26 @@ export class RbdSnapshotListComponent implements OnInit, OnChanges { pipe: this.cdDatePipe } ]; - } - - ngOnChanges() { - const imageSpec = new ImageSpec(this.poolName, this.namespace, this.rbdName); - const actions = new RbdSnapshotActionsModel( + this.imageSpec = new ImageSpec(this.poolName, this.namespace, this.rbdName); + this.rbdTableActions = new RbdSnapshotActionsModel( this.actionLabels, this.featuresName, this.rbdService ); - actions.create.click = () => this.openCreateSnapshotModal(); - actions.rename.click = () => this.openEditSnapshotModal(); - actions.protect.click = () => this.toggleProtection(); - actions.unprotect.click = () => this.toggleProtection(); + this.rbdTableActions.create.click = () => this.openCreateSnapshotModal(); + this.rbdTableActions.rename.click = () => this.openEditSnapshotModal(); + this.rbdTableActions.protect.click = () => this.toggleProtection(); + this.rbdTableActions.unprotect.click = () => this.toggleProtection(); const getImageUri = () => this.selection.first() && - `${imageSpec.toStringEncoded()}/${encodeURIComponent(this.selection.first().name)}`; - actions.clone.routerLink = () => `/block/rbd/clone/${getImageUri()}`; - actions.copy.routerLink = () => `/block/rbd/copy/${getImageUri()}`; - actions.rollback.click = () => this.rollbackModal(); - actions.deleteSnap.click = () => this.deleteSnapshotModal(); - this.tableActions = actions.ordering; + `${this.imageSpec.toStringEncoded()}/${encodeURIComponent(this.selection.first().name)}`; + this.rbdTableActions.clone.routerLink = () => `/block/rbd/clone/${getImageUri()}`; + this.rbdTableActions.copy.routerLink = () => `/block/rbd/copy/${getImageUri()}`; + this.rbdTableActions.rollback.click = () => this.rollbackModal(); + this.rbdTableActions.deleteSnap.click = () => this.deleteSnapshotModal(); + + this.tableActions = this.rbdTableActions.ordering; const itemFilter = (entry: any, task: Task) => { return entry.name === task.metadata['snapshot_name']; @@ -156,21 +168,47 @@ export class RbdSnapshotListComponent implements OnInit, OnChanges { return ( ['rbd/snap/create', 'rbd/snap/delete', 'rbd/snap/edit', 'rbd/snap/rollback'].includes( task.name - ) && imageSpec.toString() === task.metadata['image_spec'] + ) && this.imageSpec.toString() === task.metadata['image_spec'] ); }; this.taskListService.init( () => of(this.snapshots), null, - (items) => (this.data = items), - () => (this.data = this.snapshots), + (items) => { + const hasChanges = CdHelperClass.updateChanged(this, { + data: items + }); + if (hasChanges) { + this.cdr.detectChanges(); + this.data = [...this.data]; + } + }, + () => { + const hasChanges = CdHelperClass.updateChanged(this, { + data: this.snapshots + }); + if (hasChanges) { + this.cdr.detectChanges(); + this.data = [...this.data]; + } + }, taskFilter, itemFilter, this.builders ); } + ngOnChanges() { + if (this.columns) { + this.imageSpec = new ImageSpec(this.poolName, this.namespace, this.rbdName); + if (this.rbdTableActions) { + this.rbdTableActions.featuresName = this.featuresName; + } + this.taskListService.fetch(); + } + } + private openSnapshotModal(taskName: string, snapName: string = null) { this.modalRef = this.modalService.show(RbdSnapshotFormModalComponent); this.modalRef.componentInstance.poolName = this.poolName; @@ -188,12 +226,10 @@ export class RbdSnapshotListComponent implements OnInit, OnChanges { const executingTask = new ExecutingTask(); executingTask.name = taskName; executingTask.metadata = { - image_name: this.rbdName, - pool_name: this.poolName, + image_spec: this.imageSpec.toString(), snapshot_name: snapshotName }; this.summaryService.addRunningTask(executingTask); - this.ngOnChanges(); }); } @@ -223,7 +259,6 @@ export class RbdSnapshotListComponent implements OnInit, OnChanges { executingTask.name = finishedTask.name; executingTask.metadata = finishedTask.metadata; this.summaryService.addRunningTask(executingTask); - this.ngOnChanges(); this.taskManagerService.subscribe( finishedTask.name, finishedTask.metadata, @@ -250,7 +285,6 @@ export class RbdSnapshotListComponent implements OnInit, OnChanges { executingTask.metadata = finishedTask.metadata; this.summaryService.addRunningTask(executingTask); this.modalRef.close(); - this.ngOnChanges(); this.taskManagerService.subscribe( executingTask.name, executingTask.metadata, diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/classes/cd-helper.class.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/classes/cd-helper.class.ts index 7a31d34583a45..92e955ee4bd64 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/classes/cd-helper.class.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/classes/cd-helper.class.ts @@ -9,11 +9,16 @@ export class CdHelperClass { * it would update even if it equals */ static updateChanged(componentThis: any, change: { [publicVarName: string]: any }) { + let hasChanges = false; + Object.keys(change).forEach((publicVarName) => { const data = change[publicVarName]; if (!_.isEqual(data, componentThis[publicVarName])) { componentThis[publicVarName] = data; + hasChanges = true; } }); + + return hasChanges; } } -- 2.39.5