From 0120896135ed0d3c4eb6b7115f9183ce5c2acc23 Mon Sep 17 00:00:00 2001 From: Koen Pauwels Date: Tue, 7 Jan 2025 22:23:48 +0100 Subject: [PATCH] Reduce browse definition requests on simple item page (#3701) * 121561 Reduce the number of browse definition requests on Item pages by reusing the navbar request for all browse indexes * Fix test issues. --------- Co-authored-by: Koen Pauwels --- .../core/shared/browse-definition.model.ts | 8 +- .../hierarchical-browse-definition.model.ts | 4 - .../non-hierarchical-browse-definition.ts | 3 - ...item-page-abstract-field.component.spec.ts | 3 + .../item-page-author-field.component.spec.ts | 3 + .../item-page-date-field.component.spec.ts | 4 +- .../generic-item-page-field.component.spec.ts | 3 + .../img/item-page-img-field.component.spec.ts | 3 + .../item-page-field.component.spec.ts | 3 + .../item-page-field.component.ts | 39 ++++++-- .../uri/item-page-uri-field.component.spec.ts | 3 + src/app/shared/testing/browse-service.stub.ts | 91 +++++++++++++++++++ 12 files changed, 152 insertions(+), 15 deletions(-) create mode 100644 src/app/shared/testing/browse-service.stub.ts diff --git a/src/app/core/shared/browse-definition.model.ts b/src/app/core/shared/browse-definition.model.ts index 6de81727fae..5fe5d02ecb4 100644 --- a/src/app/core/shared/browse-definition.model.ts +++ b/src/app/core/shared/browse-definition.model.ts @@ -1,4 +1,7 @@ -import { autoserialize } from 'cerialize'; +import { + autoserialize, + autoserializeAs, +} from 'cerialize'; import { BrowseByDataType } from '../../browse-by/browse-by-switcher/browse-by-data-type'; import { CacheableObject } from '../cache/cacheable-object.model'; @@ -11,6 +14,9 @@ export abstract class BrowseDefinition extends CacheableObject { @autoserialize id: string; + @autoserializeAs('metadata') + metadataKeys: string[]; + /** * Get the render type of the BrowseDefinition model */ diff --git a/src/app/core/shared/hierarchical-browse-definition.model.ts b/src/app/core/shared/hierarchical-browse-definition.model.ts index e7c06a5372f..eb606b7bbeb 100644 --- a/src/app/core/shared/hierarchical-browse-definition.model.ts +++ b/src/app/core/shared/hierarchical-browse-definition.model.ts @@ -1,6 +1,5 @@ import { autoserialize, - autoserializeAs, deserialize, inheritSerialization, } from 'cerialize'; @@ -33,9 +32,6 @@ export class HierarchicalBrowseDefinition extends BrowseDefinition { @autoserialize vocabulary: string; - @autoserializeAs('metadata') - metadataKeys: string[]; - get self(): string { return this._links.self.href; } diff --git a/src/app/core/shared/non-hierarchical-browse-definition.ts b/src/app/core/shared/non-hierarchical-browse-definition.ts index 7f8c6d0e5f0..07083f7a8a8 100644 --- a/src/app/core/shared/non-hierarchical-browse-definition.ts +++ b/src/app/core/shared/non-hierarchical-browse-definition.ts @@ -21,9 +21,6 @@ export abstract class NonHierarchicalBrowseDefinition extends BrowseDefinition { @autoserializeAs('order') defaultSortOrder: string; - @autoserializeAs('metadata') - metadataKeys: string[]; - @autoserialize dataType: BrowseByDataType; } diff --git a/src/app/item-page/simple/field-components/specific-field/abstract/item-page-abstract-field.component.spec.ts b/src/app/item-page/simple/field-components/specific-field/abstract/item-page-abstract-field.component.spec.ts index 979b79eafc4..2e17d09fc1c 100644 --- a/src/app/item-page/simple/field-components/specific-field/abstract/item-page-abstract-field.component.spec.ts +++ b/src/app/item-page/simple/field-components/specific-field/abstract/item-page-abstract-field.component.spec.ts @@ -15,8 +15,10 @@ import { import { APP_CONFIG } from '../../../../../../config/app-config.interface'; import { environment } from '../../../../../../environments/environment'; +import { BrowseService } from '../../../../../core/browse/browse.service'; import { BrowseDefinitionDataService } from '../../../../../core/browse/browse-definition-data.service'; import { BrowseDefinitionDataServiceStub } from '../../../../../shared/testing/browse-definition-data-service.stub'; +import { BrowseServiceStub } from '../../../../../shared/testing/browse-service.stub'; import { TranslateLoaderMock } from '../../../../../shared/testing/translate-loader.mock'; import { ItemPageAbstractFieldComponent } from './item-page-abstract-field.component'; @@ -38,6 +40,7 @@ describe('ItemPageAbstractFieldComponent', () => { providers: [ { provide: APP_CONFIG, useValue: environment }, { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub }, + { provide: BrowseService, useValue: BrowseServiceStub }, ], schemas: [NO_ERRORS_SCHEMA], }).overrideComponent(ItemPageAbstractFieldComponent, { diff --git a/src/app/item-page/simple/field-components/specific-field/author/item-page-author-field.component.spec.ts b/src/app/item-page/simple/field-components/specific-field/author/item-page-author-field.component.spec.ts index 1f38317acaf..a9dfe998bc6 100644 --- a/src/app/item-page/simple/field-components/specific-field/author/item-page-author-field.component.spec.ts +++ b/src/app/item-page/simple/field-components/specific-field/author/item-page-author-field.component.spec.ts @@ -15,9 +15,11 @@ import { import { APP_CONFIG } from '../../../../../../config/app-config.interface'; import { environment } from '../../../../../../environments/environment'; +import { BrowseService } from '../../../../../core/browse/browse.service'; import { BrowseDefinitionDataService } from '../../../../../core/browse/browse-definition-data.service'; import { ActivatedRouteStub } from '../../../../../shared/testing/active-router.stub'; import { BrowseDefinitionDataServiceStub } from '../../../../../shared/testing/browse-definition-data-service.stub'; +import { BrowseServiceStub } from '../../../../../shared/testing/browse-service.stub'; import { TranslateLoaderMock } from '../../../../../shared/testing/translate-loader.mock'; import { MetadataValuesComponent } from '../../../../field-components/metadata-values/metadata-values.component'; import { mockItemWithMetadataFieldsAndValue } from '../item-page-field.component.spec'; @@ -41,6 +43,7 @@ describe('ItemPageAuthorFieldComponent', () => { providers: [ { provide: APP_CONFIG, useValue: environment }, { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub }, + { provide: BrowseService, useValue: BrowseServiceStub }, { provide: ActivatedRoute, useValue: new ActivatedRouteStub() }, ], schemas: [NO_ERRORS_SCHEMA], diff --git a/src/app/item-page/simple/field-components/specific-field/date/item-page-date-field.component.spec.ts b/src/app/item-page/simple/field-components/specific-field/date/item-page-date-field.component.spec.ts index 23e4492602e..264ae5ddbc9 100644 --- a/src/app/item-page/simple/field-components/specific-field/date/item-page-date-field.component.spec.ts +++ b/src/app/item-page/simple/field-components/specific-field/date/item-page-date-field.component.spec.ts @@ -15,9 +15,11 @@ import { import { APP_CONFIG } from '../../../../../../config/app-config.interface'; import { environment } from '../../../../../../environments/environment'; +import { BrowseService } from '../../../../../core/browse/browse.service'; import { BrowseDefinitionDataService } from '../../../../../core/browse/browse-definition-data.service'; import { ActivatedRouteStub } from '../../../../../shared/testing/active-router.stub'; import { BrowseDefinitionDataServiceStub } from '../../../../../shared/testing/browse-definition-data-service.stub'; +import { BrowseServiceStub } from '../../../../../shared/testing/browse-service.stub'; import { TranslateLoaderMock } from '../../../../../shared/testing/translate-loader.mock'; import { MetadataValuesComponent } from '../../../../field-components/metadata-values/metadata-values.component'; import { mockItemWithMetadataFieldsAndValue } from '../item-page-field.component.spec'; @@ -41,8 +43,8 @@ describe('ItemPageDateFieldComponent', () => { providers: [ { provide: APP_CONFIG, useValue: environment }, { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub }, + { provide: BrowseService, useValue: BrowseServiceStub }, { provide: ActivatedRoute, useValue: new ActivatedRouteStub() }, - ], schemas: [NO_ERRORS_SCHEMA], }).overrideComponent(ItemPageDateFieldComponent, { diff --git a/src/app/item-page/simple/field-components/specific-field/generic/generic-item-page-field.component.spec.ts b/src/app/item-page/simple/field-components/specific-field/generic/generic-item-page-field.component.spec.ts index f46b8fb0f88..a1ac655660a 100644 --- a/src/app/item-page/simple/field-components/specific-field/generic/generic-item-page-field.component.spec.ts +++ b/src/app/item-page/simple/field-components/specific-field/generic/generic-item-page-field.component.spec.ts @@ -15,9 +15,11 @@ import { import { APP_CONFIG } from '../../../../../../config/app-config.interface'; import { environment } from '../../../../../../environments/environment'; +import { BrowseService } from '../../../../../core/browse/browse.service'; import { BrowseDefinitionDataService } from '../../../../../core/browse/browse-definition-data.service'; import { ActivatedRouteStub } from '../../../../../shared/testing/active-router.stub'; import { BrowseDefinitionDataServiceStub } from '../../../../../shared/testing/browse-definition-data-service.stub'; +import { BrowseServiceStub } from '../../../../../shared/testing/browse-service.stub'; import { TranslateLoaderMock } from '../../../../../shared/testing/translate-loader.mock'; import { MetadataValuesComponent } from '../../../../field-components/metadata-values/metadata-values.component'; import { mockItemWithMetadataFieldsAndValue } from '../item-page-field.component.spec'; @@ -43,6 +45,7 @@ describe('GenericItemPageFieldComponent', () => { providers: [ { provide: APP_CONFIG, useValue: environment }, { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub }, + { provide: BrowseService, useValue: BrowseServiceStub }, { provide: ActivatedRoute, useValue: new ActivatedRouteStub() }, ], schemas: [NO_ERRORS_SCHEMA], diff --git a/src/app/item-page/simple/field-components/specific-field/img/item-page-img-field.component.spec.ts b/src/app/item-page/simple/field-components/specific-field/img/item-page-img-field.component.spec.ts index da1320370a8..18c02d95f65 100644 --- a/src/app/item-page/simple/field-components/specific-field/img/item-page-img-field.component.spec.ts +++ b/src/app/item-page/simple/field-components/specific-field/img/item-page-img-field.component.spec.ts @@ -14,8 +14,10 @@ import { import { APP_CONFIG } from '../../../../../../config/app-config.interface'; import { environment } from '../../../../../../environments/environment'; +import { BrowseService } from '../../../../../core/browse/browse.service'; import { BrowseDefinitionDataService } from '../../../../../core/browse/browse-definition-data.service'; import { BrowseDefinitionDataServiceStub } from '../../../../../shared/testing/browse-definition-data-service.stub'; +import { BrowseServiceStub } from '../../../../../shared/testing/browse-service.stub'; import { TranslateLoaderMock } from '../../../../../shared/testing/translate-loader.mock'; import { MetadataValuesComponent } from '../../../../field-components/metadata-values/metadata-values.component'; import { GenericItemPageFieldComponent } from '../generic/generic-item-page-field.component'; @@ -49,6 +51,7 @@ describe('ItemPageImgFieldComponent', () => { providers: [ { provide: APP_CONFIG, useValue: environment }, { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub }, + { provide: BrowseService, useValue: BrowseServiceStub }, ], schemas: [NO_ERRORS_SCHEMA], }) diff --git a/src/app/item-page/simple/field-components/specific-field/item-page-field.component.spec.ts b/src/app/item-page/simple/field-components/specific-field/item-page-field.component.spec.ts index 5815f18180f..6d654effe74 100644 --- a/src/app/item-page/simple/field-components/specific-field/item-page-field.component.spec.ts +++ b/src/app/item-page/simple/field-components/specific-field/item-page-field.component.spec.ts @@ -16,6 +16,7 @@ import { import { APP_CONFIG } from '../../../../../config/app-config.interface'; import { environment } from '../../../../../environments/environment'; +import { BrowseService } from '../../../../core/browse/browse.service'; import { BrowseDefinitionDataService } from '../../../../core/browse/browse-definition-data.service'; import { Item } from '../../../../core/shared/item.model'; import { MathService } from '../../../../core/shared/math.service'; @@ -26,6 +27,7 @@ import { import { TranslateLoaderMock } from '../../../../shared/mocks/translate-loader.mock'; import { createSuccessfulRemoteDataObject$ } from '../../../../shared/remote-data.utils'; import { BrowseDefinitionDataServiceStub } from '../../../../shared/testing/browse-definition-data-service.stub'; +import { BrowseServiceStub } from '../../../../shared/testing/browse-service.stub'; import { createPaginatedList } from '../../../../shared/testing/utils.test'; import { MarkdownDirective } from '../../../../shared/utils/markdown.directive'; import { MetadataValuesComponent } from '../../../field-components/metadata-values/metadata-values.component'; @@ -66,6 +68,7 @@ describe('ItemPageFieldComponent', () => { providers: [ { provide: APP_CONFIG, useValue: appConfig }, { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub }, + { provide: BrowseService, useValue: BrowseServiceStub }, { provide: MathService, useValue: {} }, ], schemas: [NO_ERRORS_SCHEMA], diff --git a/src/app/item-page/simple/field-components/specific-field/item-page-field.component.ts b/src/app/item-page/simple/field-components/specific-field/item-page-field.component.ts index 58521569f15..fd3506c91ee 100644 --- a/src/app/item-page/simple/field-components/specific-field/item-page-field.component.ts +++ b/src/app/item-page/simple/field-components/specific-field/item-page-field.component.ts @@ -3,17 +3,26 @@ import { Component, Input, } from '@angular/core'; +import intersectionWith from 'lodash/intersectionWith'; import { Observable } from 'rxjs'; -import { map } from 'rxjs/operators'; +import { + filter, + mergeAll, + take, +} from 'rxjs/operators'; +import { BrowseService } from '../../../../core/browse/browse.service'; import { BrowseDefinitionDataService } from '../../../../core/browse/browse-definition-data.service'; import { BrowseDefinition } from '../../../../core/shared/browse-definition.model'; import { Item } from '../../../../core/shared/item.model'; -import { getFirstCompletedRemoteData } from '../../../../core/shared/operators'; +import { + getFirstCompletedRemoteData, + getPaginatedListPayload, + getRemoteDataPayload, +} from '../../../../core/shared/operators'; import { MetadataValuesComponent } from '../../../field-components/metadata-values/metadata-values.component'; import { ImageField } from './image-field'; - /** * This component can be used to represent metadata on a simple item page. * It expects one input parameter of type Item to which the metadata belongs. @@ -30,7 +39,8 @@ import { ImageField } from './image-field'; }) export class ItemPageFieldComponent { - constructor(protected browseDefinitionDataService: BrowseDefinitionDataService) { + constructor(protected browseDefinitionDataService: BrowseDefinitionDataService, + protected browseService: BrowseService) { } /** @@ -74,9 +84,26 @@ export class ItemPageFieldComponent { * link in dspace.cfg (webui.browse.link.) */ get browseDefinition(): Observable { - return this.browseDefinitionDataService.findByFields(this.fields).pipe( + return this.browseService.getBrowseDefinitions().pipe( getFirstCompletedRemoteData(), - map((def) => def.payload), + getRemoteDataPayload(), + getPaginatedListPayload(), + mergeAll(), + filter((def: BrowseDefinition) => + intersectionWith(def.metadataKeys, this.fields, ItemPageFieldComponent.fieldMatch).length > 0, + ), + take(1), ); } + + /** + * Returns true iff the spec and field match. + * @param spec Specification of a metadata field name: either a metadata field, or a prefix ending in ".*". + * @param field A metadata field name. + * @private + */ + private static fieldMatch(spec: string, field: string): boolean { + return field === spec + || (spec.endsWith('.*') && field.substring(0, spec.length - 1) === spec.substring(0, spec.length - 1)); + } } diff --git a/src/app/item-page/simple/field-components/specific-field/uri/item-page-uri-field.component.spec.ts b/src/app/item-page/simple/field-components/specific-field/uri/item-page-uri-field.component.spec.ts index 4b865047896..53edcab28ee 100644 --- a/src/app/item-page/simple/field-components/specific-field/uri/item-page-uri-field.component.spec.ts +++ b/src/app/item-page/simple/field-components/specific-field/uri/item-page-uri-field.component.spec.ts @@ -14,8 +14,10 @@ import { import { APP_CONFIG } from '../../../../../../config/app-config.interface'; import { environment } from '../../../../../../environments/environment'; +import { BrowseService } from '../../../../../core/browse/browse.service'; import { BrowseDefinitionDataService } from '../../../../../core/browse/browse-definition-data.service'; import { BrowseDefinitionDataServiceStub } from '../../../../../shared/testing/browse-definition-data-service.stub'; +import { BrowseServiceStub } from '../../../../../shared/testing/browse-service.stub'; import { TranslateLoaderMock } from '../../../../../shared/testing/translate-loader.mock'; import { MetadataUriValuesComponent } from '../../../../field-components/metadata-uri-values/metadata-uri-values.component'; import { mockItemWithMetadataFieldsAndValue } from '../item-page-field.component.spec'; @@ -40,6 +42,7 @@ describe('ItemPageUriFieldComponent', () => { providers: [ { provide: APP_CONFIG, useValue: environment }, { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub }, + { provide: BrowseService, useValue: BrowseServiceStub }, ], schemas: [NO_ERRORS_SCHEMA], }).overrideComponent(ItemPageUriFieldComponent, { diff --git a/src/app/shared/testing/browse-service.stub.ts b/src/app/shared/testing/browse-service.stub.ts new file mode 100644 index 00000000000..85ca717cb03 --- /dev/null +++ b/src/app/shared/testing/browse-service.stub.ts @@ -0,0 +1,91 @@ +import { + EMPTY, + Observable, +} from 'rxjs'; + +import { + buildPaginatedList, + PaginatedList, +} from '../../core/data/paginated-list.model'; +import { RemoteData } from '../../core/data/remote-data'; +import { BrowseDefinition } from '../../core/shared/browse-definition.model'; +import { FlatBrowseDefinition } from '../../core/shared/flat-browse-definition.model'; +import { HierarchicalBrowseDefinition } from '../../core/shared/hierarchical-browse-definition.model'; +import { PageInfo } from '../../core/shared/page-info.model'; +import { ValueListBrowseDefinition } from '../../core/shared/value-list-browse-definition.model'; +import { createSuccessfulRemoteDataObject$ } from '../remote-data.utils'; + +const mockData = [ + Object.assign(new FlatBrowseDefinition(), { + 'id': 'dateissued', + 'browseType': 'flatBrowse', + 'dataType': 'date', + 'sortOptions': EMPTY, + 'order': 'ASC', + 'type': 'browse', + 'metadataKeys': [ + 'dc.date.issued', + ], + '_links': EMPTY, + }), + + Object.assign(new ValueListBrowseDefinition(), { + 'id': 'author', + 'browseType': 'valueList', + 'dataType': 'text', + 'sortOptions': EMPTY, + 'order': 'ASC', + 'type': 'browse', + 'metadataKeys': [ + 'dc.contributor.*', + 'dc.creator', + ], + '_links': EMPTY, + }), + + Object.assign(new FlatBrowseDefinition(), { + 'id': 'title', + 'browseType': 'flatBrowse', + 'dataType': 'title', + 'sortOptions': EMPTY, + 'order': 'ASC', + 'type': 'browse', + 'metadataKeys': [ + 'dc.title', + ], + '_links': EMPTY, + }), + + Object.assign(new ValueListBrowseDefinition(), { + 'id': 'subject', + 'browseType': 'valueList', + 'dataType': 'text', + 'sortOptions': EMPTY, + 'order': 'ASC', + 'type': 'browse', + 'metadataKeys': [ + 'dc.subject.*', + ], + '_links': EMPTY, + }), + + Object.assign(new HierarchicalBrowseDefinition(), { + 'id': 'srsc', + 'browseType': 'hierarchicalBrowse', + 'facetType': 'subject', + 'vocabulary': 'srsc', + 'type': 'browse', + 'metadataKeys': [ + 'dc.subject', + ], + '_links': EMPTY, + }), +]; +export const BrowseServiceStub: any = { + /** + * Get all browse definitions. + */ + getBrowseDefinitions(): Observable>> { + return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), mockData)); + }, +};