diff --git a/zeppelin-web-angular/src/app/interfaces/interpreter.ts b/zeppelin-web-angular/src/app/interfaces/interpreter.ts index 76a6e00ad0a..d2558bc74b6 100644 --- a/zeppelin-web-angular/src/app/interfaces/interpreter.ts +++ b/zeppelin-web-angular/src/app/interfaces/interpreter.ts @@ -65,13 +65,48 @@ interface SnapshotPolicy { interface Properties { [key: string]: { name: string; - value: boolean; - type: string; + value: string | number | boolean | null; + type: InterpreterPropertyTypes; defaultValue?: string; description?: string; }; } +export interface InterpreterPropertyValue { + name: string; + value: string | boolean; + type: InterpreterPropertyTypes; +} + +/** + * Request shape for creating/updating an interpreter setting. + * Mirrors the fields the server actually reads (InterpreterOption.java) — + * UI-only state such as `session`/`process` must not be sent. + */ +export interface InterpreterSettingOption { + isExistingProcess: boolean; + isUserImpersonate: boolean; + owners: string[]; + perNote: string; + perUser: string; + /** null is accepted by the server and kept as the default -1 (unset) */ + port: number | null; + host: string; + remote: boolean; + setPermission: boolean; +} + +export interface InterpreterSettingRequest { + name: string; + group: string; + option: InterpreterSettingOption; + properties: Record; + dependencies: Array<{ + groupArtifactVersion: string; + exclusions: string[]; + }>; +} + interface InterpreterGroupItem { name: string; class: string; @@ -91,14 +126,19 @@ interface DependenciesItem { exclusions: string[]; } +/** + * Response shape of InterpreterOption.java serialized by Gson. + * Primitive boolean/int fields are always present; String/List fields + * are omitted when null (e.g. fresh option templates from GET /interpreter). + */ interface Option { remote: boolean; port: number; isExistingProcess: boolean; setPermission: boolean; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - owners: any[]; isUserImpersonate: boolean; + host?: string; + owners?: string[]; perNote?: string; perUser?: string; } diff --git a/zeppelin-web-angular/src/app/pages/workspace/interpreter/interpreter.component.ts b/zeppelin-web-angular/src/app/pages/workspace/interpreter/interpreter.component.ts index 1c6a7a3e861..0c31e2a752c 100644 --- a/zeppelin-web-angular/src/app/pages/workspace/interpreter/interpreter.component.ts +++ b/zeppelin-web-angular/src/app/pages/workspace/interpreter/interpreter.component.ts @@ -17,7 +17,12 @@ import { debounceTime } from 'rxjs/operators'; import { NzMessageService } from 'ng-zorro-antd/message'; import { NzModalService } from 'ng-zorro-antd/modal'; -import { Interpreter, InterpreterPropertyTypes, InterpreterRepository } from '@zeppelin/interfaces'; +import { + Interpreter, + InterpreterPropertyTypes, + InterpreterRepository, + InterpreterSettingRequest +} from '@zeppelin/interfaces'; import { InterpreterService } from '@zeppelin/services'; import { InterpreterCreateRepositoryModalComponent } from './create-repository-modal/create-repository-modal.component'; @@ -75,7 +80,7 @@ export class InterpreterComponent implements OnInit, OnDestroy { }); } - addInterpreterSetting(data: Interpreter): void { + addInterpreterSetting(data: InterpreterSettingRequest): void { this.interpreterService.addInterpreterSetting(data).subscribe(res => { this.interpreterSettings.push(res); this.showCreateSetting = false; @@ -84,7 +89,7 @@ export class InterpreterComponent implements OnInit, OnDestroy { }); } - updateInterpreter(data: Interpreter): void { + updateInterpreter(data: InterpreterSettingRequest): void { this.interpreterService.updateInterpreter(data).subscribe(res => { const current = this.interpreterSettings.find(e => e.name === res.name); if (current) { diff --git a/zeppelin-web-angular/src/app/pages/workspace/interpreter/item/item.component.html b/zeppelin-web-angular/src/app/pages/workspace/interpreter/item/item.component.html index 5d8b81b914a..f38c9363426 100644 --- a/zeppelin-web-angular/src/app/pages/workspace/interpreter/item/item.component.html +++ b/zeppelin-web-angular/src/app/pages/workspace/interpreter/item/item.component.html @@ -257,6 +257,7 @@

Option

Properties ****** } @case ('url') { - + {{ control.get('value')?.value || '' }} } diff --git a/zeppelin-web-angular/src/app/pages/workspace/interpreter/item/item.component.ts b/zeppelin-web-angular/src/app/pages/workspace/interpreter/item/item.component.ts index 15e10e7e12d..313936cf860 100644 --- a/zeppelin-web-angular/src/app/pages/workspace/interpreter/item/item.component.ts +++ b/zeppelin-web-angular/src/app/pages/workspace/interpreter/item/item.component.ts @@ -11,22 +11,58 @@ */ import { ChangeDetectionStrategy, ChangeDetectorRef, Component, Input, OnDestroy, OnInit } from '@angular/core'; -import { - AbstractControl, - UntypedFormArray, - UntypedFormBuilder, - UntypedFormGroup, - ValidationErrors, - Validators, - ValidatorFn -} from '@angular/forms'; +import { AbstractControl, FormArray, FormControl, FormGroup, ValidationErrors, Validators } from '@angular/forms'; import { DestroyHookComponent } from '@zeppelin/core'; -import { Interpreter } from '@zeppelin/interfaces'; +import { + Interpreter, + InterpreterPropertyTypes, + InterpreterPropertyValue, + InterpreterSettingRequest +} from '@zeppelin/interfaces'; import { InterpreterService, SecurityService, TicketService } from '@zeppelin/services'; import { BehaviorSubject, Observable } from 'rxjs'; import { debounceTime, filter, map, switchMap, takeUntil, tap } from 'rxjs/operators'; import { InterpreterComponent } from '../interpreter.component'; +type PropertyValue = string | number | boolean | null; + +interface PropertyFormGroup { + key: FormControl; + value: FormControl; + description: FormControl; + type: FormControl; +} + +interface DependencyFormGroup { + groupArtifactVersion: FormControl; + exclusions: FormControl; +} + +interface OptionFormGroup { + isExistingProcess: FormControl; + isUserImpersonate: FormControl; + owners: FormControl; + perNote: FormControl; + perUser: FormControl; + port: FormControl; + host: FormControl; + remote: FormControl; + setPermission: FormControl; + // TODO: `session`/`process` are write-only leftovers from the pre-0.7 boolean isolation + // model, superseded by the perNote/perUser modes in ZEPPELIN-1210. They are never read + // by the template, never sent to the server, and should be removed in a follow-up. + session: FormControl; + process: FormControl; +} + +interface InterpreterFormGroup { + name: FormControl; + group: FormControl; + option: FormGroup; + properties: FormArray>; + dependencies: FormArray>; +} + @Component({ selector: 'zeppelin-interpreter-item', templateUrl: './item.component.html', @@ -38,12 +74,12 @@ export class InterpreterItemComponent extends DestroyHookComponent implements On @Input() mode: 'create' | 'view' | 'edit' = 'view'; @Input() interpreter?: Interpreter; - formGroup!: UntypedFormGroup; - optionFormGroup!: UntypedFormGroup; - editingPropertiesFormGroup?: UntypedFormGroup; - editingDependenceFormGroup?: UntypedFormGroup; - propertiesFormArray!: UntypedFormArray; - dependenciesFormArray!: UntypedFormArray; + formGroup!: FormGroup; + optionFormGroup!: FormGroup; + editingPropertiesFormGroup?: FormGroup; + editingDependenceFormGroup?: FormGroup; + propertiesFormArray!: FormArray>; + dependenciesFormArray!: FormArray>; userList$?: Observable; userSearchChange$: BehaviorSubject | null = new BehaviorSubject(''); runningOptionMap = { @@ -86,31 +122,38 @@ export class InterpreterItemComponent extends DestroyHookComponent implements On this.addProperties(); this.addDependence(); const formData = this.formGroup.getRawValue(); - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const properties: Record = {}; - - formData.properties - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .sort((e: any) => e.key) - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .forEach((e: any) => { - const { key, value, type } = e; - properties[key] = { - value, - type, - name: key - }; - }); - formData.properties = properties; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - formData.dependencies.forEach((e: any) => { - e.exclusions = e.exclusions.split(',').filter((s: string) => s !== ''); + const properties: Record = {}; + + formData.properties.forEach(({ key, value, type }) => { + properties[key] = { + // Numeric inputs hold JS numbers, which Gson would deserialize as Double + // (e.g. 60000 -> "60000.0") and break Long/Integer parsing (ZEPPELIN-6395), + // so send them as strings. Checkboxes stay real booleans, as in the classic UI. + value: type === 'checkbox' ? value === true || value === 'true' : String(value ?? ''), + type, + name: key + }; }); + // session/process are UI-only state derived from perNote/perUser; + // the server-side InterpreterOption has no such fields, so they are not sent. + const { isExistingProcess, isUserImpersonate, owners, perNote, perUser, port, host, remote, setPermission } = + formData.option; + const setting: InterpreterSettingRequest = { + name: formData.name, + group: formData.group, + option: { isExistingProcess, isUserImpersonate, owners, perNote, perUser, port, host, remote, setPermission }, + properties, + dependencies: formData.dependencies.map(({ groupArtifactVersion, exclusions }) => ({ + groupArtifactVersion, + exclusions: exclusions.split(',').filter(s => s !== '') + })) + }; + if (this.mode === 'create') { - this.parent.addInterpreterSetting(formData); + this.parent.addInterpreterSetting(setting); } else { - this.parent.updateInterpreter(formData); + this.parent.updateInterpreter(setting); this.mode = 'view'; } } @@ -146,11 +189,11 @@ export class InterpreterItemComponent extends DestroyHookComponent implements On this.cdr.markForCheck(); } - onTypeChange(type: string) { + onTypeChange(type: InterpreterPropertyTypes) { if (!this.editingPropertiesFormGroup) { throw new Error("'editingPropertiesFormGroup' is not defined. Please check if it is initialized properly."); } - let valueSet: string | boolean | number; + let valueSet: PropertyValue; switch (type) { case 'number': valueSet = 0; @@ -161,7 +204,7 @@ export class InterpreterItemComponent extends DestroyHookComponent implements On default: valueSet = ''; } - this.editingPropertiesFormGroup.get('value')!.setValue(valueSet); + this.editingPropertiesFormGroup.controls.value.setValue(valueSet); } addDependence(): void { @@ -172,17 +215,12 @@ export class InterpreterItemComponent extends DestroyHookComponent implements On if (this.editingDependenceFormGroup.valid) { const data = this.editingDependenceFormGroup.getRawValue(); const current = this.dependenciesFormArray.controls.find( - control => control.get('groupArtifactVersion')!.value === data.groupArtifactVersion + control => control.controls.groupArtifactVersion.value === data.groupArtifactVersion ); if (current) { - current.get('exclusions')!.setValue(data.exclusions); + current.controls.exclusions.setValue(data.exclusions); } else { - this.dependenciesFormArray.push( - this.formBuilder.group({ - groupArtifactVersion: [data.groupArtifactVersion, [Validators.required]], - exclusions: data.exclusions - }) - ); + this.dependenciesFormArray.push(this.createDependencyFormGroup(data)); } this.editingDependenceFormGroup.reset({ exclusions: '', @@ -199,19 +237,12 @@ export class InterpreterItemComponent extends DestroyHookComponent implements On if (this.editingPropertiesFormGroup.valid) { const data = this.editingPropertiesFormGroup.getRawValue(); - const current = this.propertiesFormArray.controls.find(control => control.get('key')!.value === data.key); + const current = this.propertiesFormArray.controls.find(control => control.controls.key.value === data.key); if (current) { - current.get('value')!.setValue(data.value); - current.get('type')!.setValue(data.type); + current.controls.value.setValue(data.value); + current.controls.type.setValue(data.type); } else { - this.propertiesFormArray.push( - this.formBuilder.group({ - key: [data.key, [Validators.required]], - value: data.value || '', - description: null, - type: data.type - }) - ); + this.propertiesFormArray.push(this.createPropertyFormGroup({ ...data, value: data.value ?? '' })); } this.editingPropertiesFormGroup.reset({ key: '', @@ -225,8 +256,8 @@ export class InterpreterItemComponent extends DestroyHookComponent implements On setInterpreterRunningOption(perNote: string, perUser: string) { const { sharedModeName, globallyModeName, perNoteModeName, perUserModeName } = this.runningOptionMap; - this.optionFormGroup.get('perNote')!.setValue(perNote); - this.optionFormGroup.get('perUser')!.setValue(perUser); + this.optionFormGroup.controls.perNote.setValue(perNote); + this.optionFormGroup.controls.perUser.setValue(perUser); // Globally == shared_perNote + shared_perUser if (perNote === sharedModeName && perUser === sharedModeName) { @@ -252,34 +283,34 @@ export class InterpreterItemComponent extends DestroyHookComponent implements On } } - this.optionFormGroup.get('perNote')!.setValue(sharedModeName); - this.optionFormGroup.get('perUser')!.setValue(sharedModeName); + this.optionFormGroup.controls.perNote.setValue(sharedModeName); + this.optionFormGroup.controls.perUser.setValue(sharedModeName); this.interpreterRunningOption = globallyModeName; } setPerNoteOrUserOption(type: 'perNote' | 'perUser', value: string) { - this.optionFormGroup.get(type)!.setValue(value); + this.optionFormGroup.controls[type].setValue(value); switch (value) { case this.sessionOptionMap.isolated: - this.optionFormGroup.get('session')!.setValue(false); - this.optionFormGroup.get('process')!.setValue(true); + this.optionFormGroup.controls.session.setValue(false); + this.optionFormGroup.controls.process.setValue(true); break; case this.sessionOptionMap.scoped: - this.optionFormGroup.get('session')!.setValue(true); - this.optionFormGroup.get('process')!.setValue(false); + this.optionFormGroup.controls.session.setValue(true); + this.optionFormGroup.controls.process.setValue(false); break; case this.sessionOptionMap.shared: - this.optionFormGroup.get('session')!.setValue(false); - this.optionFormGroup.get('process')!.setValue(false); + this.optionFormGroup.controls.session.setValue(false); + this.optionFormGroup.controls.process.setValue(false); break; } } - nameValidator(control: AbstractControl): ValidationErrors | null { + nameValidator(control: AbstractControl): ValidationErrors | null { if (this.mode !== 'create') { return null; } - const name = (control.value as string).trim(); + const name = control.value.trim(); const exist = this.parent.interpreterSettings.find(e => e.name === name); if (exist) { return { exist: true, message: `Name '${name}' already exists` }; @@ -291,25 +322,24 @@ export class InterpreterItemComponent extends DestroyHookComponent implements On buildForm(): void { let name = ''; let group = ''; - this.optionFormGroup = this.formBuilder.group({ - isExistingProcess: false, - isUserImpersonate: false, - owners: [[]], - perNote: '', - perUser: '', - port: [ - null, - [Validators.pattern('^()([1-9]|[1-5]?[0-9]{2,4}|6[1-4][0-9]{3}|65[1-4][0-9]{2}|655[1-2][0-9]|6553[1-5])$')] - ], - host: '', - remote: true, - setPermission: false, - session: false, - process: false + this.optionFormGroup = new FormGroup({ + isExistingProcess: new FormControl(false, { nonNullable: true }), + isUserImpersonate: new FormControl(false, { nonNullable: true }), + owners: new FormControl([], { nonNullable: true }), + perNote: new FormControl('', { nonNullable: true }), + perUser: new FormControl('', { nonNullable: true }), + port: new FormControl(null, [ + Validators.pattern('^()([1-9]|[1-5]?[0-9]{2,4}|6[1-4][0-9]{3}|65[1-4][0-9]{2}|655[1-2][0-9]|6553[1-5])$') + ]), + host: new FormControl('', { nonNullable: true }), + remote: new FormControl(true, { nonNullable: true }), + setPermission: new FormControl(false, { nonNullable: true }), + session: new FormControl(false, { nonNullable: true }), + process: new FormControl(false, { nonNullable: true }) }); - this.propertiesFormArray = this.formBuilder.array([]); - this.dependenciesFormArray = this.formBuilder.array([]); + this.propertiesFormArray = new FormArray>([]); + this.dependenciesFormArray = new FormArray>([]); if (this.mode === 'view' && this.interpreter) { name = this.interpreter.name; @@ -324,18 +354,18 @@ export class InterpreterItemComponent extends DestroyHookComponent implements On // set dependencies fields this.interpreter.dependencies.forEach(e => { const exclusions = Array.isArray(e.exclusions) ? e.exclusions : []; - this.dependenciesFormArray!.push( - this.formBuilder.group({ - exclusions: [exclusions.join(',')], - groupArtifactVersion: [e.groupArtifactVersion, [Validators.required]] + this.dependenciesFormArray.push( + this.createDependencyFormGroup({ + exclusions: exclusions.join(','), + groupArtifactVersion: e.groupArtifactVersion }) ); }); // set properties fields Object.entries(this.interpreter.properties).forEach(([key, item]) => { - this.propertiesFormArray!.push( - this.formBuilder.group({ + this.propertiesFormArray.push( + this.createPropertyFormGroup({ key, value: item.value, description: null, @@ -345,9 +375,12 @@ export class InterpreterItemComponent extends DestroyHookComponent implements On }); } - this.formGroup = this.formBuilder.group({ - name: [name, [Validators.required, (c: Parameters[0]) => this.nameValidator(c)]], - group: [group, [Validators.required]], + this.formGroup = new FormGroup({ + name: new FormControl(name, { + nonNullable: true, + validators: [Validators.required, control => this.nameValidator(control)] + }), + group: new FormControl(group, { nonNullable: true, validators: [Validators.required] }), option: this.optionFormGroup, properties: this.propertiesFormArray, dependencies: this.dependenciesFormArray @@ -368,43 +401,40 @@ export class InterpreterItemComponent extends DestroyHookComponent implements On }) ); - this.editingPropertiesFormGroup = this.formBuilder.group({ - key: ['', [Validators.required]], + this.editingPropertiesFormGroup = this.createPropertyFormGroup({ + key: '', value: '', description: null, type: 'string' }); - this.editingDependenceFormGroup = this.formBuilder.group({ - groupArtifactVersion: ['', [Validators.required]], - exclusions: [''] + this.editingDependenceFormGroup = this.createDependencyFormGroup({ + groupArtifactVersion: '', + exclusions: '' }); if (this.mode === 'create') { - this.formGroup - .get('group')! - .valueChanges.pipe(takeUntil(this.destroy$)) - .subscribe(value => { - // remove all controls - while (this.propertiesFormArray!.length) { - this.propertiesFormArray!.removeAt(0); - } - - const interpreters = this.parent.availableInterpreters.filter(e => e.group === value); - interpreters.forEach(interpreter => { - Object.entries(interpreter.properties).forEach(([key, item]) => { - this.propertiesFormArray!.push( - this.formBuilder.group({ - key: [key, [Validators.required]], - value: item.defaultValue, - description: item.description, - type: item.type - }) - ); - }); + this.formGroup.controls.group.valueChanges.pipe(takeUntil(this.destroy$)).subscribe(value => { + // remove all controls + while (this.propertiesFormArray.length) { + this.propertiesFormArray.removeAt(0); + } + + const interpreters = this.parent.availableInterpreters.filter(e => e.group === value); + interpreters.forEach(interpreter => { + Object.entries(interpreter.properties).forEach(([key, item]) => { + this.propertiesFormArray.push( + this.createPropertyFormGroup({ + key, + value: item.defaultValue ?? '', + description: item.description ?? null, + type: item.type + }) + ); }); - this.cdr.markForCheck(); }); + this.cdr.markForCheck(); + }); } } @@ -413,7 +443,6 @@ export class InterpreterItemComponent extends DestroyHookComponent implements On public ticketService: TicketService, private securityService: SecurityService, private interpreterService: InterpreterService, - private formBuilder: UntypedFormBuilder, private cdr: ChangeDetectorRef ) { super(); @@ -421,7 +450,7 @@ export class InterpreterItemComponent extends DestroyHookComponent implements On ngOnInit() { this.buildForm(); - const option = this.optionFormGroup!.getRawValue(); + const option = this.optionFormGroup.getRawValue(); this.setInterpreterRunningOption(option.perNote, option.perUser); if (this.mode !== 'view') { @@ -437,4 +466,31 @@ export class InterpreterItemComponent extends DestroyHookComponent implements On this.userSearchChange$ = null; super.ngOnDestroy(); } + + private createPropertyFormGroup(property: { + key: string; + value: PropertyValue; + description: string | null; + type: InterpreterPropertyTypes; + }): FormGroup { + return new FormGroup({ + key: new FormControl(property.key, { nonNullable: true, validators: [Validators.required] }), + value: new FormControl(property.value), + description: new FormControl(property.description), + type: new FormControl(property.type, { nonNullable: true }) + }); + } + + private createDependencyFormGroup(dependency: { + groupArtifactVersion: string; + exclusions: string; + }): FormGroup { + return new FormGroup({ + groupArtifactVersion: new FormControl(dependency.groupArtifactVersion, { + nonNullable: true, + validators: [Validators.required] + }), + exclusions: new FormControl(dependency.exclusions, { nonNullable: true }) + }); + } } diff --git a/zeppelin-web-angular/src/app/services/interpreter.service.ts b/zeppelin-web-angular/src/app/services/interpreter.service.ts index e0f60bf8361..0deef5b7e5b 100644 --- a/zeppelin-web-angular/src/app/services/interpreter.service.ts +++ b/zeppelin-web-angular/src/app/services/interpreter.service.ts @@ -18,7 +18,8 @@ import { Interpreter, InterpreterMap, InterpreterPropertyTypes, - InterpreterRepository + InterpreterRepository, + InterpreterSettingRequest } from '@zeppelin/interfaces'; import { InterpreterItem } from '@zeppelin/sdk'; @@ -60,13 +61,15 @@ export class InterpreterService extends BaseRest { return this.http.get(this.restUrl`/interpreter/property/types`); } - addInterpreterSetting(interpreter: Interpreter) { - return this.http.post(this.restUrl`/interpreter/setting`, interpreter); + addInterpreterSetting(setting: InterpreterSettingRequest) { + return this.http.post(this.restUrl`/interpreter/setting`, setting); } - updateInterpreter(interpreter: Interpreter) { - const { option, properties, dependencies } = interpreter; - return this.http.put(this.restUrl`/interpreter/setting/${interpreter.name}`, { + updateInterpreter(setting: InterpreterSettingRequest) { + // PUT only accepts option/properties/dependencies; name is used as the path id + // and group is immutable after creation (see UpdateInterpreterSettingRequest.java). + const { option, properties, dependencies } = setting; + return this.http.put(this.restUrl`/interpreter/setting/${setting.name}`, { option, properties, dependencies