diff --git a/src/app/dso-shared/dso-edit-metadata/dso-edit-metadata-value-field/dso-edit-metadata-field.service.ts b/src/app/dso-shared/dso-edit-metadata/dso-edit-metadata-value-field/dso-edit-metadata-field.service.ts index 630ceca848f..6ef73a83720 100644 --- a/src/app/dso-shared/dso-edit-metadata/dso-edit-metadata-value-field/dso-edit-metadata-field.service.ts +++ b/src/app/dso-shared/dso-edit-metadata/dso-edit-metadata-value-field/dso-edit-metadata-field.service.ts @@ -23,6 +23,9 @@ import { switchMap } from 'rxjs/operators'; }) export class DsoEditMetadataFieldService { + private static readonly VALID_METADATA_FIELD_PATTERN = + /^[a-zA-Z][a-zA-Z0-9_-]*\.[a-zA-Z]{2,}[a-zA-Z0-9_-]*(\.[a-zA-Z]{2,}[a-zA-Z0-9_-]*)?$/; + constructor( protected itemService: ItemDataService, protected vocabularyService: VocabularyService, @@ -36,7 +39,7 @@ export class DsoEditMetadataFieldService { * @param mdField The metadata field */ findDsoFieldVocabulary(dso: DSpaceObject, mdField: string): Observable { - if (isNotEmpty(mdField)) { + if (isNotEmpty(mdField) && DsoEditMetadataFieldService.VALID_METADATA_FIELD_PATTERN.test(mdField)) { const owningCollection$: Observable = this.itemService.findByHref(dso._links.self.href, true, true, followLink('owningCollection')).pipe( getFirstSucceededRemoteDataPayload(), switchMap((item: Item) => item.owningCollection), diff --git a/src/app/dso-shared/dso-edit-metadata/dso-edit-metadata-value/dso-edit-metadata-value.component.spec.ts b/src/app/dso-shared/dso-edit-metadata/dso-edit-metadata-value/dso-edit-metadata-value.component.spec.ts index cd6d1877e52..660d2e7b889 100644 --- a/src/app/dso-shared/dso-edit-metadata/dso-edit-metadata-value/dso-edit-metadata-value.component.spec.ts +++ b/src/app/dso-shared/dso-edit-metadata/dso-edit-metadata-value/dso-edit-metadata-value.component.spec.ts @@ -4,7 +4,9 @@ import { } from '@angular/core'; import { ComponentFixture, + fakeAsync, TestBed, + tick, waitForAsync, } from '@angular/core/testing'; import { By } from '@angular/platform-browser'; @@ -22,6 +24,7 @@ import { VIRTUAL_METADATA_PREFIX, } from '@dspace/core/shared/metadata.models'; import { ItemMetadataRepresentation } from '@dspace/core/shared/metadata-representation/item/item-metadata-representation.model'; +import { Vocabulary } from '@dspace/core/submission/vocabularies/models/vocabulary.model'; import { DsoEditMetadataFieldServiceStub } from '@dspace/core/testing/dso-edit-metadata-field.service.stub'; import { createPaginatedList } from '@dspace/core/testing/utils.test'; import { createSuccessfulRemoteDataObject$ } from '@dspace/core/utilities/remote-data.utils'; @@ -38,6 +41,7 @@ import { DsoEditMetadataValue, } from '../dso-edit-metadata-form'; import { DsoEditMetadataFieldService } from '../dso-edit-metadata-value-field/dso-edit-metadata-field.service'; +import { EditMetadataValueFieldType } from '../dso-edit-metadata-value-field/dso-edit-metadata-field-type.enum'; import { DsoEditMetadataValueFieldLoaderComponent } from '../dso-edit-metadata-value-field/dso-edit-metadata-value-field-loader/dso-edit-metadata-value-field-loader.component'; import { DsoEditMetadataValueComponent } from './dso-edit-metadata-value.component'; @@ -60,7 +64,7 @@ describe('DsoEditMetadataValueComponent', () => { let metadataValue: MetadataValue; let dso: DSpaceObject; - const collection = Object.assign(new Collection(), { + const collection = Object.assign(new Collection(), { uuid: 'fake-uuid', }); @@ -241,6 +245,147 @@ describe('DsoEditMetadataValueComponent', () => { assertButton(DRAG_BTN, true, false); }); + describe('fieldType$', () => { + + let dcMetadataSchema: MetadataSchema; + let metadataFieldDcTitle: MetadataField; + + beforeEach(() => { + dcMetadataSchema = Object.assign(new MetadataSchema(), { + prefix: 'dc', + }); + + metadataFieldDcTitle = Object.assign(new MetadataField(), { + element: 'title', + qualifier: null, + schema: createSuccessfulRemoteDataObject$(dcMetadataSchema), + }); + }); + + describe('when the metadata field is incomplete or invalid', () => { + const invalidFields = ['dc', 'dc.', 'dc.c', 'dc.re']; + + invalidFields.forEach((field) => { + it(`should not call findDsoFieldVocabulary for incomplete field "${field}"`, fakeAsync(() => { + spyOn(dsoEditMetadataFieldService, 'findDsoFieldVocabulary').and.callThrough(); + component.mdField = field; + tick(300); + expect(dsoEditMetadataFieldService.findDsoFieldVocabulary).not.toHaveBeenCalled(); + })); + }); + }); + + describe('when the metadata field does not exist on the server', () => { + beforeEach(() => { + (registryService.queryMetadataFields as jasmine.Spy).and.returnValue( + createSuccessfulRemoteDataObject$(createPaginatedList([])), + ); + }); + + it('should not call findDsoFieldVocabulary if field does not exist', fakeAsync(() => { + spyOn(dsoEditMetadataFieldService, 'findDsoFieldVocabulary').and.callThrough(); + component.mdField = 'dc.nonexistent'; + tick(300); + expect(dsoEditMetadataFieldService.findDsoFieldVocabulary).not.toHaveBeenCalled(); + })); + + it('should emit PLAIN_TEXT if field does not exist on server', fakeAsync(() => { + let result: EditMetadataValueFieldType; + component.fieldType$.subscribe((fieldType) => result = fieldType); + component.mdField = 'dc.nonexistent'; + tick(300); + expect(result).toBe(EditMetadataValueFieldType.PLAIN_TEXT); + })); + }); + + describe('when the metadata field is valid and exists on the server', () => { + beforeEach(() => { + (registryService.queryMetadataFields as jasmine.Spy).and.returnValue( + createSuccessfulRemoteDataObject$(createPaginatedList([metadataFieldDcTitle])), + ); + }); + + it('should emit a field type when field is valid and exists', fakeAsync(() => { + let result: EditMetadataValueFieldType; + const freshFixture = TestBed.createComponent(DsoEditMetadataValueComponent); + const freshComponent = freshFixture.componentInstance; + freshComponent.mdValue = editMetadataValue; + freshComponent.dso = dso; + freshComponent.metadataSecurityConfiguration = mockSecurityConfig; + freshComponent.saving$ = of(false); + spyOn(freshComponent, 'validateMetadataField').and.returnValue(of(true)); + freshFixture.detectChanges(); + freshComponent.fieldType$.subscribe((fieldType) => result = fieldType); + freshComponent.mdField = 'dc.title'; + tick(300); + expect([ + EditMetadataValueFieldType.PLAIN_TEXT, + EditMetadataValueFieldType.AUTHORITY, + EditMetadataValueFieldType.ENTITY_TYPE, + ]).toContain(result); + })); + + it('should emit PLAIN_TEXT when no vocabulary is associated', fakeAsync(() => { + let result: EditMetadataValueFieldType; + const freshFixture = TestBed.createComponent(DsoEditMetadataValueComponent); + const freshComponent = freshFixture.componentInstance; + freshComponent.mdValue = editMetadataValue; + freshComponent.dso = dso; + freshComponent.metadataSecurityConfiguration = mockSecurityConfig; + freshComponent.saving$ = of(false); + spyOn(freshComponent, 'validateMetadataField').and.returnValue(of(true)); + freshFixture.detectChanges(); + freshComponent.fieldType$.subscribe((fieldType) => result = fieldType); + freshComponent.mdField = 'dc.title'; + tick(300); + expect(result).toBe(EditMetadataValueFieldType.PLAIN_TEXT); + })); + + it('should emit AUTHORITY when a vocabulary is associated', fakeAsync(() => { + let result: EditMetadataValueFieldType; + spyOn(dsoEditMetadataFieldService, 'findDsoFieldVocabulary').and.returnValue(of(new Vocabulary())); + const freshFixture = TestBed.createComponent(DsoEditMetadataValueComponent); + const freshComponent = freshFixture.componentInstance; + freshComponent.mdValue = editMetadataValue; + freshComponent.dso = dso; + freshComponent.metadataSecurityConfiguration = mockSecurityConfig; + freshComponent.saving$ = of(false); + spyOn(freshComponent, 'validateMetadataField').and.returnValue(of(true)); + freshFixture.detectChanges(); + freshComponent.fieldType$.subscribe((fieldType) => result = fieldType); + freshComponent.mdField = 'dc.title'; + tick(300); + expect(result).toBe(EditMetadataValueFieldType.AUTHORITY); + })); + + it('should emit ENTITY_TYPE for dspace.entity.type field', fakeAsync(() => { + let result: EditMetadataValueFieldType; + const dspaceSchema = Object.assign(new MetadataSchema(), { prefix: 'dspace' }); + const entityTypeField = Object.assign(new MetadataField(), { + element: 'entity', + qualifier: 'type', + schema: createSuccessfulRemoteDataObject$(dspaceSchema), + }); + (registryService.queryMetadataFields as jasmine.Spy).and.returnValue( + createSuccessfulRemoteDataObject$(createPaginatedList([entityTypeField])), + ); + const freshFixture = TestBed.createComponent(DsoEditMetadataValueComponent); + const freshComponent = freshFixture.componentInstance; + freshComponent.mdValue = editMetadataValue; + freshComponent.dso = dso; + freshComponent.metadataSecurityConfiguration = mockSecurityConfig; + freshComponent.saving$ = of(false); + spyOn(freshComponent, 'validateMetadataField').and.returnValue(of(true)); + freshFixture.detectChanges(); + freshComponent.fieldType$.subscribe((fieldType) => result = fieldType); + freshComponent.mdField = 'dspace.entity.type'; + tick(300); + expect(result).toBe(EditMetadataValueFieldType.ENTITY_TYPE); + })); + }); + + }); + function assertButton(name: string, exists: boolean, disabled: boolean = false): void { describe(`${name} button`, () => { let btn: DebugElement; diff --git a/src/app/dso-shared/dso-edit-metadata/dso-edit-metadata-value/dso-edit-metadata-value.component.ts b/src/app/dso-shared/dso-edit-metadata/dso-edit-metadata-value/dso-edit-metadata-value.component.ts index 7bc9ce77bf8..f8b73a521c3 100644 --- a/src/app/dso-shared/dso-edit-metadata/dso-edit-metadata-value/dso-edit-metadata-value.component.ts +++ b/src/app/dso-shared/dso-edit-metadata/dso-edit-metadata-value/dso-edit-metadata-value.component.ts @@ -38,6 +38,7 @@ import { MetadataRepresentationType, } from '@dspace/core/shared/metadata-representation/metadata-representation.model'; import { + debounceTimeWorkaround as debounceTime, getFirstCompletedRemoteData, metadataFieldsToString, } from '@dspace/core/shared/operators'; @@ -109,6 +110,15 @@ import { DsoEditMetadataValueFieldLoaderComponent } from '../dso-edit-metadata-v */ export class DsoEditMetadataValueComponent implements OnInit, OnChanges, OnDestroy { + + /** + * Minimum valid metadata field pattern: schema.element or schema.element.qualifier. + * Used to skip vocabulary and field-type lookups for incomplete inputs, preventing + * unnecessary backend requests that would result in 422 errors. + */ + private static readonly VALID_METADATA_FIELD_PATTERN = + /^[a-zA-Z][a-zA-Z0-9_-]*\.[a-zA-Z][a-zA-Z0-9_-]*(\.[a-zA-Z][a-zA-Z0-9_-]*)?$/; + @Input() context: Context; /** @@ -116,6 +126,7 @@ export class DsoEditMetadataValueComponent implements OnInit, OnChanges, OnDestr * Also used to determine metadata-representations in case of virtual metadata */ @Input() dso: DSpaceObject; + /** * Editable metadata value to show */ @@ -228,6 +239,7 @@ export class DsoEditMetadataValueComponent implements OnInit, OnChanges, OnDestr * The name of the item represented by this virtual metadata value (otherwise null) */ mdRepresentationName$: Observable; + readonly mdSecurityConfigLevel$: BehaviorSubject = new BehaviorSubject([]); canShowMetadataSecurity$: Observable; @@ -239,7 +251,6 @@ export class DsoEditMetadataValueComponent implements OnInit, OnChanges, OnDestr */ public editingAuthority = false; - /** * Whether or not the free-text editing is enabled when scrollable dropdown or hierarchical vocabulary is used */ @@ -249,7 +260,7 @@ export class DsoEditMetadataValueComponent implements OnInit, OnChanges, OnDestr * Field group used by authority field * @type {UntypedFormGroup} */ - group = new UntypedFormGroup({ authorityField : new UntypedFormControl() }); + group = new UntypedFormGroup({ authorityField: new UntypedFormControl() }); /** * The type of edit field that should be displayed @@ -273,19 +284,60 @@ export class DsoEditMetadataValueComponent implements OnInit, OnChanges, OnDestr ngOnInit(): void { this.initVirtualProperties(); + /** + * Reactively determine the field type as the user types, with debouncing to avoid + * unnecessary backend requests on every keystroke. + * + * The pipeline: + * 1. debounceTime: waits 300ms after the last keystroke before proceeding. + * 2. distinctUntilChanged: skips if the value hasn't changed. + * 3. VALID_METADATA_FIELD_PATTERN: early exit for inputs that don't match the + * minimum schema.element format, avoiding requests with partial inputs. + * 4. validateMetadataField: confirms the field exists on the server via byFieldName + * before calling byMetadataAndCollection, preventing 422 errors from incomplete inputs. + * 5. findDsoFieldVocabulary: only called once the field is confirmed valid. + */ + this.fieldType$ = this._mdField$.pipe( + debounceTime(300), + distinctUntilChanged(), + switchMap((field: string) => { + if (!field || !DsoEditMetadataValueComponent.VALID_METADATA_FIELD_PATTERN.test(field)) { + return of(EditMetadataValueFieldType.PLAIN_TEXT); + } + return this.validateMetadataField().pipe( + switchMap((isValid: boolean) => { + if (!isValid) { + return of(EditMetadataValueFieldType.PLAIN_TEXT); + } + return this.dsoEditMetadataFieldService.findDsoFieldVocabulary(this.dso, field).pipe( + map((vocabulary: Vocabulary) => { + if (hasValue(vocabulary)) { + return EditMetadataValueFieldType.AUTHORITY; + } + if (field === 'dspace.entity.type') { + return EditMetadataValueFieldType.ENTITY_TYPE; + } + return EditMetadataValueFieldType.PLAIN_TEXT; + }), + ); + }), + ); + }), + ); + this.sub = combineLatest([ this._mdField$, this._metadataSecurityConfiguration$.pipe(filter(config => !!config)), ]).subscribe(([mdField, metadataSecurityConfig]) => this.initSecurityLevel(mdField, metadataSecurityConfig)); this.canShowMetadataSecurity$ = - combineLatest([ - this._mdField$.pipe(distinctUntilChanged()), - this.mdSecurityConfigLevel$, - ]).pipe( - map(([mdField, securityConfigLevel]) => hasValue(mdField) && this.hasSecurityChoice(securityConfigLevel)), - shareReplay({ refCount: false, bufferSize: 1 }), - ); + combineLatest([ + this._mdField$.pipe(distinctUntilChanged()), + this.mdSecurityConfigLevel$, + ]).pipe( + map(([mdField, securityConfigLevel]) => hasValue(mdField) && this.hasSecurityChoice(securityConfigLevel)), + shareReplay({ refCount: false, bufferSize: 1 }), + ); } /** @@ -338,37 +390,15 @@ export class DsoEditMetadataValueComponent implements OnInit, OnChanges, OnDestr return securityConfigLevel?.length > 1; } - - /** - * Retrieves the {@link EditMetadataValueFieldType} to be displayed for the current field while in edit mode. - */ - getFieldType(): Observable { - return this.dsoEditMetadataFieldService.findDsoFieldVocabulary(this.dso, this.mdField).pipe( - map((vocabulary: Vocabulary) => { - if (hasValue(vocabulary)) { - return EditMetadataValueFieldType.AUTHORITY; - } - if (this.mdField === 'dspace.entity.type') { - return EditMetadataValueFieldType.ENTITY_TYPE; - } - return EditMetadataValueFieldType.PLAIN_TEXT; - }), - ); - } - /** * Change callback for the component. Check if the mdField has changed to retrieve whether it is metadata - * that uses a controlled vocabulary and update the related properties + * that uses a controlled vocabulary and update the related properties. * * @param {SimpleChanges} changes */ ngOnChanges(changes: SimpleChanges): void { - if (changes.mdField) { - this.fieldType$ = this.getFieldType(); - } - if (isNotEmpty(changes.mdField) && !changes.mdField.firstChange) { - if (isNotEmpty(changes.mdField.currentValue) ) { + if (isNotEmpty(changes.mdField.currentValue)) { if (isNotEmpty(changes.mdField.previousValue) && changes.mdField.previousValue !== changes.mdField.currentValue) { // Clear authority value in case it has been assigned with the previous metadataField used @@ -376,8 +406,8 @@ export class DsoEditMetadataValueComponent implements OnInit, OnChanges, OnDestr this.mdValue.newValue.confidence = ConfidenceType.CF_UNSET; } - // Only ask if the current mdField have a period character to reduce request - if (changes.mdField.currentValue.includes('.')) { + // Only validate if the field matches the minimum valid format to reduce unnecessary requests + if (DsoEditMetadataValueComponent.VALID_METADATA_FIELD_PATTERN.test(changes.mdField.currentValue)) { this.validateMetadataField().subscribe((isValid: boolean) => { if (isValid) { this.cdr.detectChanges(); @@ -409,7 +439,6 @@ export class DsoEditMetadataValueComponent implements OnInit, OnChanges, OnDestr ); } - ngOnDestroy(): void { if (hasValue(this.sub)) { this.sub.unsubscribe();