Reduce browse definition requests on simple item page (DSpace#3701)

KoenP · Koen Pauwels · tdonohue · commit 76539dd8a189 · 2025-01-07T17:00:53.000-06:00
* 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 &lt;koen.pauwels@atmire.com&gt;
diff --git 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
    */
diff --git 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;
   }
diff --git 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;
 }
diff --git 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]
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
@@ -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]
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
@@ -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]
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
@@ -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]
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
@@ -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]
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
@@ -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]
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
@@ -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));
+    }
 }
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
@@ -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]
diff --git 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]
diff --git 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]
diff --git 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, {
diff --git 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));
+  },
+};

Original file line number	Diff line number	Diff line change
`@@ -16,9 +16,6 @@ export abstract class NonHierarchicalBrowseDefinition extends BrowseDefinition {`
`16`	`16`	`@autoserializeAs('order')`
`17`	`17`	`defaultSortOrder: string;`
`18`	`18`
`19`		`- @autoserializeAs('metadata')`
`20`		`- metadataKeys: string[];`
`21`		`-`
`22`	`19`	`@autoserialize`
`23`	`20`	`dataType: BrowseByDataType;`
`24`	`21`	`}`