Skip to content

Commit

Permalink
Reduce browse definition requests on simple item page (#3701)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>
  • Loading branch information
2 people authored and tdonohue committed Jan 7, 2025
1 parent 9284763 commit 0120896
Show file tree
Hide file tree
Showing 12 changed files with 152 additions and 15 deletions.
8 changes: 7 additions & 1 deletion src/app/core/shared/browse-definition.model.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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
*/
Expand Down
4 changes: 0 additions & 4 deletions src/app/core/shared/hierarchical-browse-definition.model.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import {
autoserialize,
autoserializeAs,
deserialize,
inheritSerialization,
} from 'cerialize';
Expand Down Expand Up @@ -33,9 +32,6 @@ export class HierarchicalBrowseDefinition extends BrowseDefinition {
@autoserialize
vocabulary: string;

@autoserializeAs('metadata')
metadataKeys: string[];

get self(): string {
return this._links.self.href;
}
Expand Down
3 changes: 0 additions & 3 deletions src/app/core/shared/non-hierarchical-browse-definition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,6 @@ export abstract class NonHierarchicalBrowseDefinition extends BrowseDefinition {
@autoserializeAs('order')
defaultSortOrder: string;

@autoserializeAs('metadata')
metadataKeys: string[];

@autoserialize
dataType: BrowseByDataType;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -49,6 +51,7 @@ describe('ItemPageImgFieldComponent', () => {
providers: [
{ provide: APP_CONFIG, useValue: environment },
{ provide: BrowseDefinitionDataService, useValue: BrowseDefinitionDataServiceStub },
{ provide: BrowseService, useValue: BrowseServiceStub },
],
schemas: [NO_ERRORS_SCHEMA],
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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';
Expand Down Expand Up @@ -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],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -30,7 +39,8 @@ import { ImageField } from './image-field';
})
export class ItemPageFieldComponent {

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

/**
Expand Down Expand Up @@ -74,9 +84,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));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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, {
Expand Down
91 changes: 91 additions & 0 deletions src/app/shared/testing/browse-service.stub.ts
Original file line number Diff line number Diff line change
@@ -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));
},
};

0 comments on commit 0120896

Please sign in to comment.