Merge branch 'dspace-cris-2023_02_x' into ux-plus-2023_02_x

# Conflicts:
#	src/app/shared/browse-most-elements/abstract-browse-elements.component.ts

Author: atarix83

bitbucket-pipelines.yml

@@ -22,6 +22,8 @@ pipelines:
   branches:
     'ux-plus-2023_02_x':
       - step: *unittest-code-checks
+    'prod/**':
+      - step: *unittest-code-checks
   pull-requests:
     '**':
       - step: *unittest-code-checks

config/config.example.yml

@@ -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:
@@ -533,3 +524,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

src/app/collection-page/collection-page.component.spec.ts

@@ -14,14 +14,14 @@ import { RouterStub } from '../shared/testing/router.stub';
 import { environment } from 'src/environments/environment.test';
 import { createSuccessfulRemoteDataObject$ } from '../shared/remote-data.utils';
 import { Collection } from '../core/shared/collection.model';
-import { SearchService } from '../core/shared/search/search.service';
 import { By } from '@angular/platform-browser';
 import { FormsModule } from '@angular/forms';
 import { RouterTestingModule } from '@angular/router/testing';
 import { TranslateModule } from '@ngx-translate/core';
 import { VarDirective } from '../shared/utils/var.directive';
 import { BrowserAnimationsModule } from '@angular/platform-browser/animations';
 import { Bitstream } from '../core/shared/bitstream.model';
+import { SearchManager } from '../core/browse/search-manager';
 
 describe('CollectionPageComponent', () => {
   let component: CollectionPageComponent;
@@ -33,7 +33,7 @@ describe('CollectionPageComponent', () => {
   let paginationServiceSpy: jasmine.SpyObj<PaginationService>;
   let authorizationDataServiceSpy: jasmine.SpyObj<AuthorizationDataService>;
   let dsoNameServiceSpy: jasmine.SpyObj<DSONameService>;
-  let searchServiceSpy: jasmine.SpyObj<SearchService>;
+  let searchServiceSpy: jasmine.SpyObj<SearchManager>;
   let aroute = new ActivatedRouteStub();
   let router = new RouterStub();
 
@@ -44,7 +44,7 @@ describe('CollectionPageComponent', () => {
     paginationServiceSpy = jasmine.createSpyObj('PaginationService', ['getCurrentPagination', 'getCurrentSort', 'clearPagination']);
     authorizationDataServiceSpy = jasmine.createSpyObj('AuthorizationDataService', ['isAuthorized']);
     collectionDataServiceSpy = jasmine.createSpyObj('CollectionDataService', ['findById', 'getAuthorizedCollection']);
-    searchServiceSpy = jasmine.createSpyObj('SearchService', ['search']);
+    searchServiceSpy = jasmine.createSpyObj('SearchManager', ['search']);
     dsoNameServiceSpy = jasmine.createSpyObj('DSONameService', ['getName']);
 
     await TestBed.configureTestingModule({
@@ -58,7 +58,7 @@ describe('CollectionPageComponent', () => {
         { provide: PaginationService, useValue: paginationServiceSpy },
         { provide: AuthorizationDataService, useValue: authorizationDataServiceSpy },
         { provide: DSONameService, useValue: dsoNameServiceSpy },
-        { provide: SearchService, useValue: searchServiceSpy },
+        { provide: SearchManager, useValue: searchServiceSpy },
         { provide: APP_CONFIG, useValue: environment },
         { provide: PLATFORM_ID, useValue: 'browser' },
       ]

src/app/collection-page/collection-page.component.ts

@@ -4,9 +4,7 @@ import { ActivatedRoute, Router } from '@angular/router';
 import { BehaviorSubject, combineLatest as observableCombineLatest, Observable, Subject } from 'rxjs';
 import { filter, map, mergeMap, startWith, switchMap, take } from 'rxjs/operators';
 import { PaginatedSearchOptions } from '../shared/search/models/paginated-search-options.model';
-import { SearchService } from '../core/shared/search/search.service';
 import { SortDirection, SortOptions } from '../core/cache/models/sort-options.model';
-import { CollectionDataService } from '../core/data/collection-data.service';
 import { PaginatedList } from '../core/data/paginated-list.model';
 import { RemoteData } from '../core/data/remote-data';
 import { Bitstream } from '../core/shared/bitstream.model';
@@ -30,6 +28,7 @@ import { redirectOn4xx } from '../core/shared/authorized.operators';
 import { BROWSE_LINKS_TO_FOLLOW } from '../core/browse/browse.service';
 import { DSONameService } from '../core/breadcrumbs/dso-name.service';
 import { APP_CONFIG, AppConfig } from '../../../src/config/app-config.interface';
+import { SearchManager } from '../core/browse/search-manager';
 
 @Component({
   selector: 'ds-collection-page',
@@ -64,8 +63,7 @@ export class CollectionPageComponent implements OnInit {
 
   constructor(
     @Inject(PLATFORM_ID) private platformId: Object,
-    private collectionDataService: CollectionDataService,
-    private searchService: SearchService,
+    private searchManager: SearchManager,
     private route: ActivatedRoute,
     private router: Router,
     private authService: AuthService,
@@ -113,14 +111,13 @@ export class CollectionPageComponent implements OnInit {
         getFirstSucceededRemoteData(),
         map((rd) => rd.payload.id),
         switchMap((id: string) => {
-          return this.searchService.search<Item>(
+          return this.searchManager.search<Item>(
             new PaginatedSearchOptions({
               scope: id,
               pagination: currentPagination,
               sort: currentSort,
               dsoTypes: [DSpaceObjectType.ITEM],
               forcedEmbeddedKeys: ['metrics'],
-              projection: 'preventMetadataSecurity'
             }), null, true, true, ...BROWSE_LINKS_TO_FOLLOW)
             .pipe(toDSpaceObjectListRD()) as Observable<RemoteData<PaginatedList<Item>>>;
         }),

src/app/core/browse/search-manager.ts

@@ -45,7 +45,7 @@ export class SearchManager {
    * @returns {Observable<RemoteData<PaginatedList<Item>>>}
    */
   getBrowseItemsFor(filterValue: string, filterAuthority: string, options: BrowseEntrySearchOptions, ...linksToFollow: FollowLinkConfig<any>[]): Observable<RemoteData<PaginatedList<Item>>> {
-    const browseOptions = Object.assign({}, options, { projection: 'preventMetadataSecurity' });
+    const browseOptions = Object.assign({}, options, { projection: options.projection ?? 'preventMetadataSecurity' });
     return this.browseService.getBrowseItemsFor(filterValue, filterAuthority, browseOptions, ...linksToFollow)
       .pipe(this.completeWithExtraData());
   }
@@ -67,7 +67,8 @@ export class SearchManager {
     useCachedVersionIfAvailable = true,
     reRequestOnStale = true,
     ...linksToFollow: FollowLinkConfig<T>[]): Observable<RemoteData<SearchObjects<T>>> {
-    return this.searchService.search(searchOptions, responseMsToLive, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow)
+    const optionsWithDefaultProjection = Object.assign(new PaginatedSearchOptions({}), searchOptions, { projection: searchOptions.projection ?? 'preventMetadataSecurity' });
+    return this.searchService.search(optionsWithDefaultProjection, responseMsToLive, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow)
       .pipe(this.completeSearchObjectsWithExtraData());
   }
 
@@ -112,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 => {
@@ -125,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);
   }
 }

src/app/core/browse/search.manager.spec.ts

@@ -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);
+    });
   });
 
 });

src/app/core/shared/dspace-object.model.ts

@@ -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.
    *

src/app/core/shared/metadata.utils.spec.ts

@@ -1,7 +1,7 @@
 import { isUndefined } from '../../shared/empty.util';
 import { v4 as uuidv4 } from 'uuid';
 import { MetadataMap, MetadataValue, MetadataValueFilter, MetadatumViewModel } from './metadata.models';
-import { Metadata } from './metadata.utils';
+import { Metadata, PLACEHOLDER_VALUE } from './metadata.utils';
 
 const mdValue = (value: string, language?: string, authority?: string): MetadataValue => {
   return Object.assign(new MetadataValue(), {
@@ -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;
     }
@@ -297,4 +298,29 @@ 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);
+    });
+  });
+
+  describe('Placeholder values', () => {
+    it('should ignore placeholder values in get methods', () => {
+      const placeholderMd = mdValue(PLACEHOLDER_VALUE);
+      const key = 'dc.test.placeholder';
+      const map = { 'dc.test.placeholder': [placeholderMd] };
+
+      expect(Metadata.all(map, key).length).toEqual(0);
+      expect(Metadata.allValues(map, key).length).toEqual(0);
+      expect(Metadata.has(map, key)).toBeFalsy();
+      expect(Metadata.first(map, key)).toBeUndefined();
+      expect(Metadata.firstValue(map, key)).toBeUndefined();
+      expect(Metadata.hasValue(placeholderMd)).toBeFalsy();
+      expect(Metadata.valueMatches(placeholderMd, null)).toBeFalsy();
+    });
+  });
+
 });

src/app/core/shared/metadata.utils.ts

@@ -14,6 +14,8 @@ import { validate as uuidValidate } from 'uuid';
 
 export const AUTHORITY_GENERATE = 'will be generated::';
 export const AUTHORITY_REFERENCE = 'will be referenced::';
+export const PLACEHOLDER_VALUE = '#PLACEHOLDER_PARENT_METADATA_VALUE#';
+
 
 /**
  * Utility class for working with DSpace object metadata.
@@ -29,18 +31,18 @@ export const AUTHORITY_REFERENCE = 'will be referenced::';
  * followed by any other (non-dc) metadata values.
  */
 export class Metadata {
-
   /**
    * Gets all matching metadata in the map(s).
    *
    * @param {MetadataMapInterface|MetadataMapInterface[]} mapOrMaps The source map(s). When multiple maps are given, they will be
    * 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) {
@@ -50,6 +52,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;
+              }
             }
           }
         }
@@ -148,11 +153,11 @@ export class Metadata {
    * Returns true if this Metadatum's value is defined
    */
   public static hasValue(value: MetadataValue|string): boolean {
-    if (isEmpty(value)) {
+    if (isEmpty(value) || value === PLACEHOLDER_VALUE) {
       return false;
     }
     if (isObject(value) && value.hasOwnProperty('value')) {
-      return isNotEmpty(value.value);
+      return isNotEmpty(value.value) && value.value !== PLACEHOLDER_VALUE;
     }
     return true;
   }
@@ -165,7 +170,9 @@ export class Metadata {
    * @returns {boolean} whether the filter matches, or true if no filter is given.
    */
   public static valueMatches(mdValue: MetadataValue, filter: MetadataValueFilter) {
-    if (!filter) {
+    if (mdValue.value === PLACEHOLDER_VALUE) {
+      return false;
+    } else if (!filter) {
       return true;
     } else if (filter.language && filter.language !== mdValue.language) {
       return false;

src/app/cris-layout/cris-layout-matrix/cris-layout-box-container/boxes/metadata/rendering-types/crisref/crisref.component.spec.ts

@@ -20,7 +20,7 @@ describe('CrisrefComponent', () => {
   let fixture: ComponentFixture<CrisrefComponent>;
 
   const itemService = jasmine.createSpyObj('ItemDataService', {
-    findById: jasmine.createSpy('findById')
+    findByIdWithProjections: jasmine.createSpy('findByIdWithProjections')
   });
   const metadataValue = Object.assign(new MetadataValue(), {
     'value': 'test item title',
@@ -106,7 +106,7 @@ describe('CrisrefComponent', () => {
   beforeEach(() => {
     fixture = TestBed.createComponent(CrisrefComponent);
     component = fixture.componentInstance;
-    itemService.findById.and.returnValue(createSuccessfulRemoteDataObject$(testPerson));
+    itemService.findByIdWithProjections.and.returnValue(createSuccessfulRemoteDataObject$(testPerson));
     fixture.detectChanges();
   });