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 <koen.pauwels@atmire.com>
2025-10-07 01:54:15 +00:00 · 2025-01-07 22:23:48 +01:00
parent 9fce33f514
commit 76539dd8a1
15 changed files with 169 additions and 20 deletions
--- 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 { CacheableObject } from '../cache/cacheable-object.model';

 /**
@@ -9,6 +12,9 @@ export abstract class BrowseDefinition extends CacheableObject {
  @autoserialize
  id: string;

+  @autoserializeAs('metadata')
+  metadataKeys: string[];
+
  /**
   * Get the render type of the BrowseDefinition model
   */
--- a/src/app/core/shared/hierarchical-browse-definition.model.ts
+++ b/src/app/core/shared/hierarchical-browse-definition.model.ts
@@ -1,4 +1,4 @@
-import { autoserialize, autoserializeAs, deserialize, inheritSerialization } from 'cerialize';
+import { autoserialize, deserialize, inheritSerialization } from 'cerialize';
 import { typedObject } from '../cache/builders/build-decorators';
 import { excludeFromEquals } from '../utilities/equals.decorators';
 import { HIERARCHICAL_BROWSE_DEFINITION } from './hierarchical-browse-definition.resource-type';
@@ -26,9 +26,6 @@ export class HierarchicalBrowseDefinition extends BrowseDefinition {
  @autoserialize
  vocabulary: string;

-  @autoserializeAs('metadata')
-  metadataKeys: string[];
-
  get self(): string {
    return this._links.self.href;
  }
--- a/src/app/core/shared/non-hierarchical-browse-definition.ts
+++ b/src/app/core/shared/non-hierarchical-browse-definition.ts
@@ -16,9 +16,6 @@ export abstract class NonHierarchicalBrowseDefinition extends BrowseDefinition {
  @autoserializeAs('order')
  defaultSortOrder: string;

-  @autoserializeAs('metadata')
-  metadataKeys: string[];
-
  @autoserialize
  dataType: BrowseByDataType;
 }
--- a/src/app/entity-groups/journal-entities/item-pages/journal/journal.component.spec.ts
+++ b/src/app/entity-groups/journal-entities/item-pages/journal/journal.component.spec.ts
@@ -39,6 +39,8 @@ import {
  BrowseDefinitionDataServiceStub
 } from '../../../../shared/testing/browse-definition-data-service.stub';
 import { BrowseDefinitionDataService } from '../../../../core/browse/browse-definition-data.service';
+import { BrowseService } from '../../../../core/browse/browse.service';
+import { BrowseServiceStub } from '../../../../shared/testing/browse-service.stub';

 let comp: JournalComponent;
 let fixture: ComponentFixture<JournalComponent>;
@@ -105,7 +107,8 @@ describe('JournalComponent', () => {
        { provide: WorkspaceitemDataService, useValue: {} },
        { provide: SearchService, useValue: {} },
        { provide: RouteService, useValue: mockRouteService },
-        { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub }
+        { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub },
+        { provide: BrowseService, useValue: BrowseServiceStub },
      ],

      schemas: [NO_ERRORS_SCHEMA]
--- 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
@@ -6,6 +6,8 @@ import { TranslateLoaderMock } from '../../../../../shared/testing/translate-loa
 import { SharedModule } from '../../../../../shared/shared.module';
 import { APP_CONFIG } from '../../../../../../config/app-config.interface';
 import { environment } from '../../../../../../environments/environment';
+import { BrowseService } from '../../../../../core/browse/browse.service';
+import { BrowseServiceStub } from '../../../../../shared/testing/browse-service.stub';
 import { By } from '@angular/platform-browser';
 import { BrowseDefinitionDataService } from '../../../../../core/browse/browse-definition-data.service';
 import { BrowseDefinitionDataServiceStub } from '../../../../../shared/testing/browse-definition-data-service.stub';
@@ -27,7 +29,8 @@ describe('ItemPageAbstractFieldComponent', () => {
      ],
      providers: [
        { provide: APP_CONFIG, useValue: environment },
-        { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub }
+        { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub },
+        { provide: BrowseService, useValue: BrowseServiceStub },
      ],
      declarations: [ItemPageAbstractFieldComponent],
      schemas: [NO_ERRORS_SCHEMA]
--- 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
@@ -7,6 +7,8 @@ import { mockItemWithMetadataFieldsAndValue } from '../item-page-field.component
 import { ItemPageAuthorFieldComponent } from './item-page-author-field.component';
 import { APP_CONFIG } from '../../../../../../config/app-config.interface';
 import { environment } from '../../../../../../environments/environment';
+import { BrowseService } from '../../../../../core/browse/browse.service';
+import { BrowseServiceStub } from '../../../../../shared/testing/browse-service.stub';
 import { BrowseDefinitionDataService } from '../../../../../core/browse/browse-definition-data.service';
 import { BrowseDefinitionDataServiceStub } from '../../../../../shared/testing/browse-definition-data-service.stub';

@@ -27,7 +29,8 @@ describe('ItemPageAuthorFieldComponent', () => {
      })],
      providers: [
        { provide: APP_CONFIG, useValue: environment },
-        { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub }
+        { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub },
+        { provide: BrowseService, useValue: BrowseServiceStub },
      ],
      declarations: [ItemPageAuthorFieldComponent, MetadataValuesComponent],
      schemas: [NO_ERRORS_SCHEMA]
--- 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
@@ -7,6 +7,8 @@ import { mockItemWithMetadataFieldsAndValue } from '../item-page-field.component
 import { ItemPageDateFieldComponent } from './item-page-date-field.component';
 import { APP_CONFIG } from '../../../../../../config/app-config.interface';
 import { environment } from '../../../../../../environments/environment';
+import { BrowseService } from '../../../../../core/browse/browse.service';
+import { BrowseServiceStub } from '../../../../../shared/testing/browse-service.stub';
 import { BrowseDefinitionDataService } from '../../../../../core/browse/browse-definition-data.service';
 import { BrowseDefinitionDataServiceStub } from '../../../../../shared/testing/browse-definition-data-service.stub';

@@ -27,7 +29,8 @@ describe('ItemPageDateFieldComponent', () => {
      })],
      providers: [
        { provide: APP_CONFIG, useValue: environment },
-        { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub }
+        { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub },
+        { provide: BrowseService, useValue: BrowseServiceStub },
      ],
      declarations: [ItemPageDateFieldComponent, MetadataValuesComponent],
      schemas: [NO_ERRORS_SCHEMA]
--- 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
@@ -7,6 +7,8 @@ import { mockItemWithMetadataFieldsAndValue } from '../item-page-field.component
 import { GenericItemPageFieldComponent } from './generic-item-page-field.component';
 import { APP_CONFIG } from '../../../../../../config/app-config.interface';
 import { environment } from '../../../../../../environments/environment';
+import { BrowseService } from '../../../../../core/browse/browse.service';
+import { BrowseServiceStub } from '../../../../../shared/testing/browse-service.stub';
 import { BrowseDefinitionDataService } from '../../../../../core/browse/browse-definition-data.service';
 import { BrowseDefinitionDataServiceStub } from '../../../../../shared/testing/browse-definition-data-service.stub';

@@ -29,7 +31,8 @@ describe('GenericItemPageFieldComponent', () => {
      })],
      providers: [
        { provide: APP_CONFIG, useValue: environment },
-        { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub }
+        { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub },
+        { provide: BrowseService, useValue: BrowseServiceStub },
      ],
      declarations: [GenericItemPageFieldComponent, MetadataValuesComponent],
      schemas: [NO_ERRORS_SCHEMA]
--- 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
@@ -13,6 +13,8 @@ import { MarkdownPipe } from '../../../../shared/utils/markdown.pipe';
 import { SharedModule } from '../../../../shared/shared.module';
 import { APP_CONFIG } from '../../../../../config/app-config.interface';
 import { By } from '@angular/platform-browser';
+import { BrowseService } from '../../../../core/browse/browse.service';
+import { BrowseServiceStub } from '../../../../shared/testing/browse-service.stub';
 import { BrowseDefinitionDataService } from '../../../../core/browse/browse-definition-data.service';
 import { BrowseDefinitionDataServiceStub } from '../../../../shared/testing/browse-definition-data-service.stub';
 import { RouterTestingModule } from '@angular/router/testing';
@@ -51,7 +53,8 @@ describe('ItemPageFieldComponent', () => {
      ],
      providers: [
        { provide: APP_CONFIG, useValue: appConfig },
-        { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub }
+        { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub },
+        { provide: BrowseService, useValue: BrowseServiceStub },
      ],
      declarations: [ItemPageFieldComponent, MetadataValuesComponent],
      schemas: [NO_ERRORS_SCHEMA]
--- 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
@@ -1,10 +1,20 @@
 import { Component, Input } from '@angular/core';
 import { Item } from '../../../../core/shared/item.model';
-import { map } from 'rxjs/operators';
+import intersectionWith from 'lodash/intersectionWith';
+import {
+  filter,
+  mergeAll,
+  take,
+} from 'rxjs/operators';
 import { Observable } from 'rxjs';
+import { BrowseService } from '../../../../core/browse/browse.service';
 import { BrowseDefinition } from '../../../../core/shared/browse-definition.model';
 import { BrowseDefinitionDataService } from '../../../../core/browse/browse-definition-data.service';
-import { getFirstCompletedRemoteData } from '../../../../core/shared/operators';
+import {
+  getFirstCompletedRemoteData,
+  getPaginatedListPayload,
+  getRemoteDataPayload,
+} from '../../../../core/shared/operators';

 /**
 * This component can be used to represent metadata on a simple item page.
@@ -17,7 +27,8 @@ import { getFirstCompletedRemoteData } from '../../../../core/shared/operators';
 })
 export class ItemPageFieldComponent {

-    constructor(protected browseDefinitionDataService: BrowseDefinitionDataService) {
+    constructor(protected browseDefinitionDataService: BrowseDefinitionDataService,
+                protected browseService: BrowseService) {
    }

    /**
@@ -56,9 +67,26 @@ export class ItemPageFieldComponent {
     * link in dspace.cfg (webui.browse.link.<n>)
     */
    get browseDefinition(): Observable<BrowseDefinition> {
-      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));
+    }
 }
--- 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
@@ -7,6 +7,8 @@ import { ItemPageUriFieldComponent } from './item-page-uri-field.component';
 import { MetadataUriValuesComponent } from '../../../../field-components/metadata-uri-values/metadata-uri-values.component';
 import { environment } from '../../../../../../environments/environment';
 import { APP_CONFIG } from '../../../../../../config/app-config.interface';
+import { BrowseService } from '../../../../../core/browse/browse.service';
+import { BrowseServiceStub } from '../../../../../shared/testing/browse-service.stub';
 import { BrowseDefinitionDataService } from '../../../../../core/browse/browse-definition-data.service';
 import { BrowseDefinitionDataServiceStub } from '../../../../../shared/testing/browse-definition-data-service.stub';

@@ -28,7 +30,8 @@ describe('ItemPageUriFieldComponent', () => {
      })],
      providers: [
        { provide: APP_CONFIG, useValue: environment },
-        { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub }
+        { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub },
+        { provide: BrowseService, useValue: BrowseServiceStub },
      ],
      declarations: [ItemPageUriFieldComponent, MetadataUriValuesComponent],
      schemas: [NO_ERRORS_SCHEMA]
--- a/src/app/item-page/simple/item-types/publication/publication.component.spec.ts
+++ b/src/app/item-page/simple/item-types/publication/publication.component.spec.ts
@@ -40,6 +40,8 @@ import { BrowseDefinitionDataService } from '../../../../core/browse/browse-defi
 import {
  BrowseDefinitionDataServiceStub
 } from '../../../../shared/testing/browse-definition-data-service.stub';
+import { BrowseService } from '../../../../core/browse/browse.service';
+import { BrowseServiceStub } from '../../../../shared/testing/browse-service.stub';

 const noMetadata = new MetadataMap();

@@ -93,6 +95,7 @@ describe('PublicationComponent', () => {
        { provide: SearchService, useValue: {} },
        { provide: RouteService, useValue: mockRouteService },
        { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub },
+        { provide: BrowseService, useValue: BrowseServiceStub },
      ],

      schemas: [NO_ERRORS_SCHEMA]
--- a/src/app/item-page/simple/item-types/shared/item.component.spec.ts
+++ b/src/app/item-page/simple/item-types/shared/item.component.spec.ts
@@ -44,6 +44,8 @@ import { BrowseDefinitionDataService } from '../../../../core/browse/browse-defi
 import {
  BrowseDefinitionDataServiceStub
 } from '../../../../shared/testing/browse-definition-data-service.stub';
+import { BrowseService } from '../../../../core/browse/browse.service';
+import { BrowseServiceStub } from '../../../../shared/testing/browse-service.stub';

 import { buildPaginatedList } from '../../../../core/data/paginated-list.model';
 import { PageInfo } from '../../../../core/shared/page-info.model';
@@ -133,6 +135,7 @@ export function getItemPageFieldsTest(mockItem: Item, component) {
          { provide: AuthorizationDataService, useValue: authorizationService },
          { provide: ResearcherProfileDataService, useValue: {} },
          { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub },
+          { provide: BrowseService, useValue: BrowseServiceStub },
        ],

        schemas: [NO_ERRORS_SCHEMA]
--- a/src/app/item-page/simple/item-types/untyped-item/untyped-item.component.spec.ts
+++ b/src/app/item-page/simple/item-types/untyped-item/untyped-item.component.spec.ts
@@ -41,6 +41,8 @@ import { BrowseDefinitionDataService } from '../../../../core/browse/browse-defi
 import {
  BrowseDefinitionDataServiceStub
 } from '../../../../shared/testing/browse-definition-data-service.stub';
+import { BrowseService } from '../../../../core/browse/browse.service';
+import { BrowseServiceStub } from '../../../../shared/testing/browse-service.stub';

 const noMetadata = new MetadataMap();

@@ -96,6 +98,7 @@ describe('UntypedItemComponent', () => {
        { provide: ItemVersionsSharedService, useValue: {} },
        { provide: RouteService, useValue: mockRouteService },
        { provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub },
+        { provide: BrowseService, useValue: BrowseServiceStub },
      ],
      schemas: [NO_ERRORS_SCHEMA]
    }).overrideComponent(UntypedItemComponent, {
--- a/src/app/shared/testing/browse-service.stub.ts
+++ 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<RemoteData<PaginatedList<BrowseDefinition>>> {
+    return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), mockData));
+  },
+};