From f17eff7f444b5f3aaf9944d2f0d2cdd20359f0a0 Mon Sep 17 00:00:00 2001 From: Patrick Nawracay Date: Mon, 29 Oct 2018 09:59:59 +0100 Subject: [PATCH] mgr/dashboard: Cleanup - Make a method static instead of keeping it as classmethod - Fix return type of methods type hint - Reformat to fit 100 chars max width - Rename some variables, methods and argument names for a better understanding Examples: - activatedCompression() -> hasCompressionEnabled() - extendByItemsForSubmit() -> assignFormFields() - api -> fieldName - name -> formControlName - fn -> replaceFn - edit -> editable - Extract type declaration to interface and adapted the code to use that interface - Replace unused nearly-global variables with local ones - Add types where missing and appropriate Exmaples: - PoolService::getInfo() -> Observable - Moved model declaration to shared directory - Added documentation for a method which isn't that easy to understand Signed-off-by: Patrick Nawracay --- src/pybind/mgr/dashboard/controllers/pool.py | 11 +- .../pool/pool-form/pool-form.component.html | 2 +- .../pool-form/pool-form.component.spec.ts | 4 +- .../pool/pool-form/pool-form.component.ts | 153 +++++++++++------- .../api/erasure-code-profile.service.ts | 6 +- .../src/app/shared/api/pool.service.ts | 7 +- .../models}/pool-form-info.ts | 2 +- 7 files changed, 109 insertions(+), 76 deletions(-) rename src/pybind/mgr/dashboard/frontend/src/app/{ceph/pool/pool-form => shared/models}/pool-form-info.ts (82%) diff --git a/src/pybind/mgr/dashboard/controllers/pool.py b/src/pybind/mgr/dashboard/controllers/pool.py index 7f0254c6d81..1964204d88e 100644 --- a/src/pybind/mgr/dashboard/controllers/pool.py +++ b/src/pybind/mgr/dashboard/controllers/pool.py @@ -19,8 +19,8 @@ def pool_task(name, metadata, wait_for=2.0): @ApiController('/pool', Scope.POOL) class Pool(RESTController): - @classmethod - def _serialize_pool(cls, pool, attrs): + @staticmethod + def _serialize_pool(pool, attrs): if not attrs or not isinstance(attrs, list): attrs = pool.keys() @@ -65,9 +65,8 @@ class Pool(RESTController): raise cherrypy.NotFound('No such pool') return pool[0] - # '_get' will be wrapped into JSON through '_request_wrapper' def get(self, pool_name, attrs=None, stats=False): - # type: (str, str, bool) -> str + # type: (str, str, bool) -> dict return self._get(pool_name, attrs, stats) @pool_task('delete', ['{pool_name}']) @@ -145,10 +144,10 @@ class Pool(RESTController): for o in mgr.get('osd_metadata').values()) def compression_enum(conf_name): - return [[v for v in o['enum_values'] if len(v) > 0] for o in config_options + return [[v for v in o['enum_values'] if len(v) > 0] + for o in mgr.get('config_options')['options'] if o['name'] == conf_name][0] - config_options = mgr.get('config_options')['options'] return { "pool_names": [p['pool_name'] for p in self._pool_list()], "crush_rules_replicated": rules(1), diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.html b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.html index 4d99d8e84c1..96f9b8bb206 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.html +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.html @@ -375,7 +375,7 @@ -
+
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.spec.ts index 5770d240f5e..da5351aec17 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.spec.ts @@ -216,7 +216,7 @@ describe('PoolFormComponent', () => { ['crushRule', 'size', 'erasureProfile', 'ecOverwrites'].forEach((name) => formHelper.expectValid(name) ); - expect(component.compressionForm.valid).toBeTruthy(); + expect(component.form.get('compression').valid).toBeTruthy(); }); it('validates name', () => { @@ -297,7 +297,7 @@ describe('PoolFormComponent', () => { }); it('is valid', () => { - expect(component.compressionForm.valid).toBeTruthy(); + expect(component.form.get('compression').valid).toBeTruthy(); }); it('validates minBlobSize to be only valid between 0 and maxBlobSize', () => { diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.ts index a43198ff222..56ea0e5875f 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.ts @@ -16,6 +16,7 @@ import { CrushStep } from '../../../shared/models/crush-step'; import { ErasureCodeProfile } from '../../../shared/models/erasure-code-profile'; import { FinishedTask } from '../../../shared/models/finished-task'; import { Permission } from '../../../shared/models/permissions'; +import { PoolFormInfo } from '../../../shared/models/pool-form-info'; import { DimlessBinaryPipe } from '../../../shared/pipes/dimless-binary.pipe'; import { AuthStorageService } from '../../../shared/services/auth-storage.service'; import { FormatterService } from '../../../shared/services/formatter.service'; @@ -23,7 +24,15 @@ import { TaskWrapperService } from '../../../shared/services/task-wrapper.servic import { ErasureCodeProfileFormComponent } from '../erasure-code-profile-form/erasure-code-profile-form.component'; import { Pool } from '../pool'; import { PoolFormData } from './pool-form-data'; -import { PoolFormInfo } from './pool-form-info'; + +interface FormFieldDescription { + externalFieldName: string; + formControlName: string; + attr?: string; + replaceFn?: Function; + editable?: boolean; + resetValue?: any; +} @Component({ selector: 'cd-pool-form', @@ -33,7 +42,6 @@ import { PoolFormInfo } from './pool-form-info'; export class PoolFormComponent implements OnInit { permission: Permission; form: CdFormGroup; - compressionForm: CdFormGroup; ecProfiles: ErasureCodeProfile[]; info: PoolFormInfo; routeParamsSubscribe: any; @@ -59,7 +67,7 @@ export class PoolFormComponent implements OnInit { ) { this.editing = this.router.url.startsWith('/pool/edit'); this.authenticate(); - this.createForms(); + this.createForm(); } authenticate() { @@ -72,8 +80,8 @@ export class PoolFormComponent implements OnInit { } } - private createForms() { - this.compressionForm = new CdFormGroup({ + private createForm() { + const compressionForm = new CdFormGroup({ mode: new FormControl('none'), algorithm: new FormControl(''), minBlobSize: new FormControl('', { @@ -86,6 +94,7 @@ export class PoolFormComponent implements OnInit { updateOn: 'blur' }) }); + this.form = new CdFormGroup( { name: new FormControl('', { @@ -117,7 +126,7 @@ export class PoolFormComponent implements OnInit { validators: [Validators.required, Validators.min(1)] }), ecOverwrites: new FormControl(false), - compression: this.compressionForm + compression: compressionForm }, CdValidators.custom('form', () => null) ); @@ -389,19 +398,19 @@ export class PoolFormComponent implements OnInit { } private setCompressionValidators() { - CdValidators.validateIf(this.form.get('minBlobSize'), () => this.activatedCompression(), [ + CdValidators.validateIf(this.form.get('minBlobSize'), () => this.hasCompressionEnabled(), [ Validators.min(0), CdValidators.custom('maximum', (size) => this.oddBlobSize(size, this.form.getValue('maxBlobSize')) ) ]); - CdValidators.validateIf(this.form.get('maxBlobSize'), () => this.activatedCompression(), [ + CdValidators.validateIf(this.form.get('maxBlobSize'), () => this.hasCompressionEnabled(), [ Validators.min(0), CdValidators.custom('minimum', (size) => this.oddBlobSize(this.form.getValue('minBlobSize'), size) ) ]); - CdValidators.validateIf(this.form.get('ratio'), () => this.activatedCompression(), [ + CdValidators.validateIf(this.form.get('ratio'), () => this.hasCompressionEnabled(), [ Validators.min(0), Validators.max(1) ]); @@ -413,7 +422,7 @@ export class PoolFormComponent implements OnInit { return Boolean(minimum && maximum && minimum >= maximum); } - activatedCompression() { + hasCompressionEnabled() { return this.form.getValue('mode') && this.form.get('mode').value.toLowerCase() !== 'none'; } @@ -468,53 +477,70 @@ export class PoolFormComponent implements OnInit { this.form.setErrors({ cdSubmitButton: true }); return; } + const pool = { pool: this.form.getValue('name') }; - this.extendByItemsForSubmit(pool, [ - { api: 'pool_type', name: 'poolType' }, - { api: 'pg_num', name: 'pgNum', edit: true }, + + this.assignFormFields(pool, [ + { externalFieldName: 'pool_type', formControlName: 'poolType' }, + { externalFieldName: 'pg_num', formControlName: 'pgNum', editable: true }, this.form.getValue('poolType') === 'replicated' - ? { api: 'size', name: 'size' } - : { api: 'erasure_code_profile', name: 'erasureProfile', attr: 'name' }, - { api: 'rule_name', name: 'crushRule', attr: 'rule_name' } + ? { externalFieldName: 'size', formControlName: 'size' } + : { + externalFieldName: 'erasure_code_profile', + formControlName: 'erasureProfile', + attr: 'name' + }, + { externalFieldName: 'rule_name', formControlName: 'crushRule', attr: 'rule_name' } ]); + if (this.info.is_all_bluestore) { - this.extendByItemForSubmit(pool, { - api: 'flags', - name: 'ecOverwrites', - fn: () => ['ec_overwrites'] + this.assignFormField(pool, { + externalFieldName: 'flags', + formControlName: 'ecOverwrites', + replaceFn: () => ['ec_overwrites'] }); + if (this.form.getValue('mode') !== 'none') { - this.extendByItemsForSubmit(pool, [ + this.assignFormFields(pool, [ { - api: 'compression_mode', - name: 'mode', - edit: true, - fn: (value) => this.activatedCompression() && value + externalFieldName: 'compression_mode', + formControlName: 'mode', + editable: true, + replaceFn: (value) => this.hasCompressionEnabled() && value }, - { api: 'compression_algorithm', name: 'algorithm', edit: true }, { - api: 'compression_min_blob_size', - name: 'minBlobSize', - fn: this.formatter.toBytes, - edit: true, + externalFieldName: 'compression_algorithm', + formControlName: 'algorithm', + editable: true + }, + { + externalFieldName: 'compression_min_blob_size', + formControlName: 'minBlobSize', + replaceFn: this.formatter.toBytes, + editable: true, resetValue: 0 }, { - api: 'compression_max_blob_size', - name: 'maxBlobSize', - fn: this.formatter.toBytes, - edit: true, + externalFieldName: 'compression_max_blob_size', + formControlName: 'maxBlobSize', + replaceFn: this.formatter.toBytes, + editable: true, resetValue: 0 }, - { api: 'compression_required_ratio', name: 'ratio', edit: true, resetValue: 0 } + { + externalFieldName: 'compression_required_ratio', + formControlName: 'ratio', + editable: true, + resetValue: 0 + } ]); } else if (this.editing) { - this.extendByItemsForSubmit(pool, [ + this.assignFormFields(pool, [ { - api: 'compression_mode', - name: 'mode', - edit: true, - fn: () => 'unset' + externalFieldName: 'compression_mode', + formControlName: 'mode', + editable: true, + replaceFn: () => 'unset' } ]); } @@ -526,41 +552,44 @@ export class PoolFormComponent implements OnInit { this.triggerApiTask(pool); } - private extendByItemsForSubmit(pool, items: any[]) { - items.forEach((item) => this.extendByItemForSubmit(pool, item)); + /** + * Retrieves the values for the given form field descriptions and assigns the values to the given + * object. This method differentiates between `add` and `edit` mode and acts differently on one or + * the other. + */ + private assignFormFields(pool: object, formFieldDescription: FormFieldDescription[]): void { + formFieldDescription.forEach((item) => this.assignFormField(pool, item)); } - private extendByItemForSubmit( - pool, + /** + * Retrieves the value for the given form field description and assigns the values to the given + * object. This method differentiates between `add` and `edit` mode and acts differently on one or + * the other. + */ + private assignFormField( + pool: object, { - api, - name, + externalFieldName, + formControlName, attr, - fn, - edit, + replaceFn, + editable, resetValue - }: { - api: string; - name: string; - attr?: string; - fn?: Function; - edit?: boolean; - resetValue?: any; - } - ) { - if (this.editing && (!edit || this.form.get(name).pristine)) { + }: FormFieldDescription + ): void { + if (this.editing && (!editable || this.form.get(formControlName).pristine)) { return; } - const value = this.form.getValue(name); - let apiValue = fn ? fn(value) : attr ? _.get(value, attr) : value; + const value = this.form.getValue(formControlName); + let apiValue = replaceFn ? replaceFn(value) : attr ? _.get(value, attr) : value; if (!value || !apiValue) { - if (edit && !_.isUndefined(resetValue)) { + if (editable && !_.isUndefined(resetValue)) { apiValue = resetValue; } else { return; } } - pool[api] = apiValue; + pool[externalFieldName] = apiValue; } private triggerApiTask(pool) { diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/erasure-code-profile.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/erasure-code-profile.service.ts index 165a5d6a562..7599caaa60d 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/erasure-code-profile.service.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/erasure-code-profile.service.ts @@ -1,6 +1,8 @@ import { HttpClient } from '@angular/common/http'; import { Injectable } from '@angular/core'; +import { Observable } from 'rxjs'; + import { ErasureCodeProfile } from '../models/erasure-code-profile'; import { ApiModule } from './api.module'; @@ -12,8 +14,8 @@ export class ErasureCodeProfileService { constructor(private http: HttpClient) {} - list() { - return this.http.get(this.apiPath); + list(): Observable { + return this.http.get(this.apiPath); } create(ecp: ErasureCodeProfile) { diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/pool.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/pool.service.ts index 183b1c4081d..59f4eec6867 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/pool.service.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/pool.service.ts @@ -1,7 +1,10 @@ import { HttpClient } from '@angular/common/http'; import { Injectable } from '@angular/core'; +import { Observable } from 'rxjs'; + import { cdEncode } from '../decorators/cd-encode'; +import { PoolFormInfo } from '../models/pool-form-info'; import { ApiModule } from './api.module'; @cdEncode @@ -35,8 +38,8 @@ export class PoolService { return this.http.get(this.apiPath); } - getInfo() { - return this.http.get(`${this.apiPath}/_info`); + getInfo(): Observable { + return this.http.get(`${this.apiPath}/_info`); } list(attrs = []) { diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form-info.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/models/pool-form-info.ts similarity index 82% rename from src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form-info.ts rename to src/pybind/mgr/dashboard/frontend/src/app/shared/models/pool-form-info.ts index e1e56eadfce..7741087b9c3 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form-info.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/models/pool-form-info.ts @@ -1,4 +1,4 @@ -import { CrushRule } from '../../../shared/models/crush-rule'; +import { CrushRule } from './crush-rule'; export class PoolFormInfo { pool_names: string[]; -- 2.39.5