Skip to content

Commit

Permalink
Merged in task/dspace-cris-2023_02_x/DSC-1988 (pull request DSpace#2458)
Browse files Browse the repository at this point in the history
[DSC-1988] Limit the number of items requested by  search manager

Approved-by: Giuseppe Digilio
  • Loading branch information
alisaismailati authored and atarix83 committed Nov 29, 2024
2 parents 29a111a + bf28fa3 commit 327a5bd
Show file tree
Hide file tree
Showing 15 changed files with 178 additions and 31 deletions.
25 changes: 16 additions & 9 deletions config/config.example.yml
Original file line number Diff line number Diff line change
Expand Up @@ -327,15 +327,6 @@ item:
# The maximum number of values for repeatable metadata to show in the full item
metadataLimit: 20

# When the search results are retrieved, for each item type the metadata with a valid authority value are inspected.
# Referenced items will be fetched with a find all by id strategy to avoid individual rest requests
# to efficiently display the search results.
followAuthorityMetadata:
- type: Publication
metadata: dc.contributor.author
- type: Product
metadata: dc.contributor.author

# Collection Page Config
collection:
edit:
Expand Down Expand Up @@ -517,3 +508,19 @@ addToAnyPlugin:
title: DSpace CRIS 7 demo
# The link to be shown in the shared post, if different from document.location.origin (optional)
# link: https://dspacecris7.4science.cloud/

# When the search results are retrieved, for each item type the metadata with a valid authority value are inspected.
# Referenced items will be fetched with a find all by id strategy to avoid individual rest requests
# to efficiently display the search results.
followAuthorityMetadata:
- type: Publication
metadata: dc.contributor.author
- type: Product
metadata: dc.contributor.author

# The maximum number of item to process when following authority metadata values.
followAuthorityMaxItemLimit: 100

# The maximum number of metadata values to process for each metadata key
# when following authority metadata values.
followAuthorityMetadataValuesLimit: 5
15 changes: 10 additions & 5 deletions src/app/core/browse/search-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,8 @@ export class SearchManager {
})
.filter((item) => hasValue(item));

const uuidList = this.extractUUID(items, environment.followAuthorityMetadata);
const uuidList = this.extractUUID(items, environment.followAuthorityMetadata, environment.followAuthorityMaxItemLimit);

return uuidList.length > 0 ? this.itemService.findAllById(uuidList).pipe(
getFirstCompletedRemoteData(),
map(data => {
Expand All @@ -126,27 +127,31 @@ export class SearchManager {
) : of(null);
}

protected extractUUID(items: Item[], metadataToFollow: FollowAuthorityMetadata[]): string[] {
protected extractUUID(items: Item[], metadataToFollow: FollowAuthorityMetadata[], numberOfElementsToReturn?: number): string[] {
const uuidMap = {};

items.forEach((item) => {
metadataToFollow.forEach((followMetadata: FollowAuthorityMetadata) => {
if (item.entityType === followMetadata.type) {
if (isArray(followMetadata.metadata)) {
followMetadata.metadata.forEach((metadata) => {
Metadata.all(item.metadata, metadata)
followMetadata.metadata.forEach((metadata) => {
Metadata.all(item.metadata, metadata, null, environment.followAuthorityMetadataValuesLimit)
.filter((metadataValue: MetadataValue) => Metadata.hasValidItemAuthority(metadataValue.authority))
.forEach((metadataValue: MetadataValue) => uuidMap[metadataValue.authority] = metadataValue);
});
} else {
Metadata.all(item.metadata, followMetadata.metadata)
Metadata.all(item.metadata, followMetadata.metadata, null, environment.followAuthorityMetadataValuesLimit)
.filter((metadataValue: MetadataValue) => Metadata.hasValidItemAuthority(metadataValue.authority))
.forEach((metadataValue: MetadataValue) => uuidMap[metadataValue.authority] = metadataValue);
}
}
});
});

if (hasValue(numberOfElementsToReturn) && numberOfElementsToReturn > 0) {
return Object.keys(uuidMap).slice(0, numberOfElementsToReturn);
}

return Object.keys(uuidMap);
}
}
5 changes: 5 additions & 0 deletions src/app/core/browse/search.manager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,11 @@ describe('SearchManager', () => {
const uuidList = (service as any).extractUUID([firstPublication, firstPublication], [{type: 'Publication', metadata: ['dc.contributor.author']}]);
expect(uuidList).toEqual([validAuthority]);
});

it('should limit the number of extracted uuids', () => {
const uuidList = (service as any).extractUUID([firstPublication, secondPublication, invalidAuthorityPublication], [{type: 'Publication', metadata: ['dc.contributor.author']}], 2);
expect(uuidList.length).toBe(2);
});
});

});
12 changes: 12 additions & 0 deletions src/app/core/shared/dspace-object.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,18 @@ export class DSpaceObject extends ListableObject implements CacheableObject {
return Metadata.all(this.metadata, keyOrKeys, valueFilter);
}

/**
* Gets all matching metadata in this DSpaceObject, up to a limit.
*
* @param {string|string[]} keyOrKeys The metadata key(s) in scope. Wildcards are supported; see [[Metadata]].
* @param {number} limit The maximum number of results to return.
* @param {MetadataValueFilter} valueFilter The value filter to use. If unspecified, no filtering will be done.
* @returns {MetadataValue[]} the matching values or an empty array.
*/
limitedMetadata(keyOrKeys: string | string[], limit: number, valueFilter?: MetadataValueFilter): MetadataValue[] {
return Metadata.all(this.metadata, keyOrKeys, valueFilter, limit);
}

/**
* Like [[allMetadata]], but only returns string values.
*
Expand Down
15 changes: 12 additions & 3 deletions src/app/core/shared/metadata.utils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,19 +44,20 @@ const multiViewModelList = [
{ key: 'foo', ...bar, order: 0 }
];

const testMethod = (fn, resultKind, mapOrMaps, keyOrKeys, expected, filter?) => {
const testMethod = (fn, resultKind, mapOrMaps, keyOrKeys, expected, filter?, limit?: number) => {
const keys = keyOrKeys instanceof Array ? keyOrKeys : [keyOrKeys];
describe('and key' + (keys.length === 1 ? (' ' + keys[0]) : ('s ' + JSON.stringify(keys)))
+ ' with ' + (isUndefined(filter) ? 'no filter' : 'filter ' + JSON.stringify(filter)), () => {
const result = fn(mapOrMaps, keys, filter);
const result = fn(mapOrMaps, keys, filter, limit);
let shouldReturn;
if (resultKind === 'boolean') {
shouldReturn = expected;
} else if (isUndefined(expected)) {
shouldReturn = 'undefined';
} else if (expected instanceof Array) {
shouldReturn = 'an array with ' + expected.length + ' ' + (expected.length > 1 ? 'ordered ' : '')
+ resultKind + (expected.length !== 1 ? 's' : '');
+ resultKind + (expected.length !== 1 ? 's' : '')
+ (isUndefined(limit) ? '' : ' (limited to ' + limit + ')');
} else {
shouldReturn = 'a ' + resultKind;
}
Expand Down Expand Up @@ -297,4 +298,12 @@ describe('Metadata', () => {

});

describe('all method with limit', () => {
const testAllWithLimit = (mapOrMaps, keyOrKeys, expected, limit) =>
testMethod(Metadata.all, 'value', mapOrMaps, keyOrKeys, expected, undefined, limit);

describe('with multiMap and limit', () => {
testAllWithLimit(multiMap, 'dc.title', [dcTitle1], 1);
});
});
});
6 changes: 5 additions & 1 deletion src/app/core/shared/metadata.utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,11 @@ export class Metadata {
* checked in order, and only values from the first with at least one match will be returned.
* @param {string|string[]} keyOrKeys The metadata key(s) in scope. Wildcards are supported; see above.
* @param {MetadataValueFilter} filter The value filter to use. If unspecified, no filtering will be done.
* @param {number} limit The maximum number of values to return. If unspecified, all matching values will be returned.
* @returns {MetadataValue[]} the matching values or an empty array.
*/
public static all(mapOrMaps: MetadataMapInterface | MetadataMapInterface[], keyOrKeys: string | string[],
filter?: MetadataValueFilter): MetadataValue[] {
filter?: MetadataValueFilter, limit?: number): MetadataValue[] {
const mdMaps: MetadataMapInterface[] = mapOrMaps instanceof Array ? mapOrMaps : [mapOrMaps];
const matches: MetadataValue[] = [];
for (const mdMap of mdMaps) {
Expand All @@ -50,6 +51,9 @@ export class Metadata {
for (const candidate of candidates) {
if (Metadata.valueMatches(candidate as MetadataValue, filter)) {
matches.push(candidate as MetadataValue);
if (hasValue(limit) && matches.length >= limit) {
return matches;
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,20 @@ <h3 [innerHTML]="dsoTitle" [ngClass]="{'lead': true,'text-muted': !item.firstMet
<span class="item-list-date" [innerHTML]="item.firstMetadataValue('dc.date.issued') || ('mydspace.results.no-date' | translate)"></span>)
<span *ngIf="item.hasMetadata(authorMetadata);" class="item-list-authors">
<span *ngIf="item.allMetadataValues(authorMetadata).length === 0">{{'mydspace.results.no-authors' | translate}}</span>
<span *ngFor="let author of item.allMetadata(authorMetadata); let i=index; let last=last;">
<ds-metadata-link-view *ngIf="!!item && !!author" [item]="item" [metadataName]="authorMetadata"
[metadata]="author"></ds-metadata-link-view><span
*ngIf="!last">; </span>
</span>
<span *ngIf="!(isCollapsed$ | async)">
<span *ngFor="let author of item.allMetadata(authorMetadata); let i=index; let last=last;">
<ds-metadata-link-view *ngIf="!!item && !!author" [item]="item" [metadataName]="authorMetadata"
[metadata]="author"></ds-metadata-link-view><span
*ngIf="!last">; </span>
</span>
</span>
<span *ngIf="isCollapsed$ | async">
<span *ngFor="let author of item.limitedMetadata(authorMetadata, authorMetadataLimit); let i=index; let last=last;">
<ds-metadata-link-view *ngIf="!!item && !!author" [item]="item" [metadataName]="authorMetadata"
[metadata]="author"></ds-metadata-link-view><span
*ngIf="!last">; </span>
</span>
</span>
</span>
</ds-truncatable-part>
</span>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { TranslateLoaderMock } from '../../../mocks/translate-loader.mock';
import { NoopAnimationsModule } from '@angular/platform-browser/animations';
import { VarDirective } from '../../../utils/var.directive';
import { APP_CONFIG } from '../../../../../config/app-config.interface';
import { TruncatableService } from '../../../truncatable/truncatable.service';

let component: ItemListPreviewComponent;
let fixture: ComponentFixture<ItemListPreviewComponent>;
Expand Down Expand Up @@ -80,6 +81,10 @@ const enviromentNoThumbs = {
}
};

const truncatableServiceStub: any = {
isCollapsed: (id: number) => observableOf(true),
};

describe('ItemListPreviewComponent', () => {
beforeEach(waitForAsync(() => {
TestBed.configureTestingModule({
Expand All @@ -95,7 +100,8 @@ describe('ItemListPreviewComponent', () => {
declarations: [ItemListPreviewComponent, TruncatePipe, VarDirective],
providers: [
{ provide: 'objectElementProvider', useValue: { mockItemWithAuthorAndDate }},
{ provide: APP_CONFIG, useValue: environmentUseThumbs }
{ provide: APP_CONFIG, useValue: environmentUseThumbs },
{ provide: TruncatableService, useValue: truncatableServiceStub },
],

schemas: [NO_ERRORS_SCHEMA]
Expand Down Expand Up @@ -198,6 +204,33 @@ describe('ItemListPreviewComponent', () => {
expect(entityField).toBeNull();
});
});


describe('When truncatable section is collapsed', () => {
beforeEach(() => {
component.isCollapsed$ = observableOf(true);
component.item = mockItemWithAuthorAndDate;
fixture.detectChanges();
});

it('should show limitedMetadata', () => {
const authorElements = fixture.debugElement.queryAll(By.css('span.item-list-authors ds-metadata-link-view'));
expect(authorElements.length).toBe(mockItemWithAuthorAndDate.limitedMetadata(component.authorMetadata, component.authorMetadataLimit).length);
});
});

describe('When truncatable section is expanded', () => {
beforeEach(() => {
component.isCollapsed$ = observableOf(false);
component.item = mockItemWithAuthorAndDate;
fixture.detectChanges();
});

it('should show allMetadata', () => {
const authorElements = fixture.debugElement.queryAll(By.css('span.item-list-authors ds-metadata-link-view'));
expect(authorElements.length).toBe(mockItemWithAuthorAndDate.allMetadata(component.authorMetadata).length);
});
});
});

describe('ItemListPreviewComponent', () => {
Expand All @@ -215,7 +248,8 @@ describe('ItemListPreviewComponent', () => {
declarations: [ItemListPreviewComponent, TruncatePipe],
providers: [
{provide: 'objectElementProvider', useValue: {mockItemWithAuthorAndDate}},
{provide: APP_CONFIG, useValue: enviromentNoThumbs}
{provide: APP_CONFIG, useValue: enviromentNoThumbs},
{ provide: TruncatableService, useValue: truncatableServiceStub },
],

schemas: [NO_ERRORS_SCHEMA]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import {
} from '../../../../submission/sections/detect-duplicate/models/duplicate-detail-metadata.model';
import { parseISO, differenceInDays, differenceInMilliseconds } from 'date-fns';
import { environment } from '../../../../../environments/environment';
import { TruncatableService } from '../../../truncatable/truncatable.service';
import { Observable } from 'rxjs';

/**
* This component show metadata for the given item object in the list view.
Expand Down Expand Up @@ -78,9 +80,14 @@ export class ItemListPreviewComponent implements OnInit {

authorMetadata = environment.searchResult.authorMetadata;

authorMetadataLimit = environment.followAuthorityMetadataValuesLimit;

isCollapsed$: Observable<boolean>;

constructor(
@Inject(APP_CONFIG) protected appConfig: AppConfig,
public dsoNameService: DSONameService,
public truncateService: TruncatableService
) {
}

Expand All @@ -96,6 +103,6 @@ export class ItemListPreviewComponent implements OnInit {
ngOnInit(): void {
this.showThumbnails = this.showThumbnails ?? this.appConfig.browseBy.showThumbnails;
this.dsoTitle = this.dsoNameService.getHitHighlights(this.object, this.item);
this.isCollapsed$ = this.truncateService.isCollapsed(this.item.uuid);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,18 @@
[innerHTML]="firstMetadataValue('dc.date.issued')"></span>)
</ng-container>
<span *ngIf="dso.allMetadataValues(authorMetadata).length > 0" class="item-list-authors">
<span *ngFor="let author of dso.allMetadata(authorMetadata); let i=index; let last=last;">
<ds-metadata-link-view [item]="dso" [metadata]="author" [metadataName]="authorMetadata"></ds-metadata-link-view><span *ngIf="!last">; </span>
</span>
<ng-container *ngVar="(isCollapsed() | async) as isCollapsed">
<ng-container *ngIf="isCollapsed">
<span *ngFor="let author of dso.limitedMetadata(authorMetadata, additionalMetadataLimit); let i=index; let last=last;">
<ds-metadata-link-view [item]="dso" [metadata]="author" [metadataName]="authorMetadata"></ds-metadata-link-view><span *ngIf="!last">; </span>
</span>
</ng-container>
<ng-container *ngIf="!isCollapsed">
<span *ngFor="let author of dso.allMetadata(authorMetadata); let i=index; let last=last;">
<ds-metadata-link-view [item]="dso" [metadata]="author" [metadataName]="authorMetadata"></ds-metadata-link-view><span *ngIf="!last">; </span>
</span>
</ng-container>
</ng-container>
</span>
</ds-truncatable-part>
</span>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,13 +186,17 @@ const enviromentNoThumbs = {
}
};

const truncatableServiceStub = {
isCollapsed: (id: number) => observableOf(true),
};

describe('ItemSearchResultListElementComponent', () => {
beforeEach(waitForAsync(() => {
TestBed.configureTestingModule({
imports: [TranslateModule.forRoot()],
declarations: [ItemSearchResultListElementComponent, TruncatePipe, VarDirective],
providers: [
{ provide: TruncatableService, useValue: {} },
{ provide: TruncatableService, useValue: truncatableServiceStub },
{ provide: DSONameService, useClass: DSONameServiceMock },
{ provide: APP_CONFIG, useValue: environmentUseThumbs }
],
Expand Down Expand Up @@ -247,6 +251,32 @@ describe('ItemSearchResultListElementComponent', () => {
});
});

describe('When the item has authors and isCollapsed is true', () => {
beforeEach(() => {
spyOn(publicationListElementComponent, 'isCollapsed').and.returnValue(observableOf(true));
publicationListElementComponent.object = mockItemWithMetadata;
fixture.detectChanges();
});

it('should show limitedMetadata', () => {
const authorElements = fixture.debugElement.queryAll(By.css('span.item-list-authors ds-metadata-link-view'));
expect(authorElements.length).toBe(mockItemWithMetadata.indexableObject.limitedMetadata(publicationListElementComponent.authorMetadata, publicationListElementComponent.additionalMetadataLimit).length);
});
});

describe('When the item has authors and isCollapsed is false', () => {
beforeEach(() => {
spyOn(publicationListElementComponent, 'isCollapsed').and.returnValue(observableOf(false));
publicationListElementComponent.object = mockItemWithMetadata;
fixture.detectChanges();
});

it('should show allMetadata', () => {
const authorElements = fixture.debugElement.queryAll(By.css('span.item-list-authors ds-metadata-link-view'));
expect(authorElements.length).toBe(mockItemWithMetadata.indexableObject.allMetadata(publicationListElementComponent.authorMetadata).length);
});
});

describe('When the item has a publisher', () => {
beforeEach(() => {
publicationListElementComponent.object = mockItemWithMetadata;
Expand Down Expand Up @@ -375,7 +405,7 @@ describe('ItemSearchResultListElementComponent', () => {
imports: [TranslateModule.forRoot()],
declarations: [ItemSearchResultListElementComponent, TruncatePipe],
providers: [
{provide: TruncatableService, useValue: {}},
{provide: TruncatableService, useValue: truncatableServiceStub},
{provide: DSONameService, useClass: DSONameServiceMock},
{ provide: APP_CONFIG, useValue: enviromentNoThumbs }
],
Expand Down
Loading

0 comments on commit 327a5bd

Please sign in to comment.