Merge remote-tracking branch 'contributions/w2p-107155_Performance-re…

…-request-embeds-7.6' into w2p-107155_Performance-re-request-embeds_contribute-7_x

# Conflicts:
#	src/app/core/browse/browse.service.ts
#	src/app/item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.spec.ts

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

@@ -28,7 +28,7 @@ import { AuthorizationDataService } from '../core/data/feature-authorization/aut
 import { FeatureID } from '../core/data/feature-authorization/feature-id';
 import { getCollectionPageRoute } from './collection-page-routing-paths';
 import { redirectOn4xx } from '../core/shared/authorized.operators';
-import { BROWSE_LINKS_TO_FOLLOW } from '../core/browse/browse.service';
+import { getBrowseLinksToFollow } from '../core/browse/browse.service';
 import { DSONameService } from '../core/breadcrumbs/dso-name.service';
 import { APP_CONFIG, AppConfig } from '../../../src/config/app-config.interface';
 
@@ -115,7 +115,7 @@ export class CollectionPageComponent implements OnInit {
               pagination: currentPagination,
               sort: currentSort,
               dsoTypes: [DSpaceObjectType.ITEM]
-            }), null, true, true, ...BROWSE_LINKS_TO_FOLLOW)
+            }), null, true, true, ...getBrowseLinksToFollow())
             .pipe(toDSpaceObjectListRD()) as Observable<RemoteData<PaginatedList<Item>>>;
         }),
         startWith(undefined) // Make sure switching pages shows loading component

src/app/core/breadcrumbs/item-breadcrumb.resolver.ts

@@ -4,7 +4,7 @@ import { ItemDataService } from '../data/item-data.service';
 import { Item } from '../shared/item.model';
 import { DSOBreadcrumbResolver } from './dso-breadcrumb.resolver';
 import { FollowLinkConfig } from '../../shared/utils/follow-link-config.model';
-import { ITEM_PAGE_LINKS_TO_FOLLOW } from '../../item-page/item.resolver';
+import { getItemPageLinksToFollow } from '../../item-page/item.resolver';
 
 /**
  * The class that resolves the BreadcrumbConfig object for an Item
@@ -23,6 +23,6 @@ export class ItemBreadcrumbResolver extends DSOBreadcrumbResolver<Item> {
    * Requesting them as embeds will limit the number of requests
    */
   get followLinks(): FollowLinkConfig<Item>[] {
-    return ITEM_PAGE_LINKS_TO_FOLLOW;
+    return getItemPageLinksToFollow();
   }
 }

src/app/core/browse/browse.service.spec.ts

@@ -1,10 +1,8 @@
 import { cold, getTestScheduler, hot } from 'jasmine-marbles';
 import { of as observableOf } from 'rxjs';
 import { TestScheduler } from 'rxjs/testing';
-import { getMockRemoteDataBuildService } from '../../shared/mocks/remote-data-build.service.mock';
 import { getMockRequestService } from '../../shared/mocks/request.service.mock';
 import { HALEndpointServiceStub } from '../../shared/testing/hal-endpoint-service.stub';
-import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service';
 import { RequestService } from '../data/request.service';
 import { BrowseEntrySearchOptions } from './browse-entry-search-options.model';
 import { BrowseService } from './browse.service';
@@ -20,7 +18,6 @@ describe('BrowseService', () => {
   let scheduler: TestScheduler;
   let service: BrowseService;
   let requestService: RequestService;
-  let rdbService: RemoteDataBuildService;
 
   const browsesEndpointURL = 'https://rest.api/browses';
   const halService: any = new HALEndpointServiceStub(browsesEndpointURL);
@@ -118,7 +115,6 @@ describe('BrowseService', () => {
       halService,
       browseDefinitionDataService,
       hrefOnlyDataService,
-      rdbService
     );
   }
 
@@ -130,11 +126,9 @@ describe('BrowseService', () => {
 
     beforeEach(() => {
       requestService = getMockRequestService(getRequestEntry$(true));
-      rdbService = getMockRemoteDataBuildService();
       service = initTestService();
       spyOn(halService, 'getEndpoint').and
         .returnValue(hot('--a-', { a: browsesEndpointURL }));
-      spyOn(rdbService, 'buildList').and.callThrough();
     });
 
     it('should call BrowseDefinitionDataService to create the RemoteData Observable', () => {
@@ -151,9 +145,7 @@ describe('BrowseService', () => {
 
     beforeEach(() => {
       requestService = getMockRequestService(getRequestEntry$(true));
-      rdbService = getMockRemoteDataBuildService();
       service = initTestService();
-      spyOn(rdbService, 'buildList').and.callThrough();
     });
 
     describe('when getBrowseEntriesFor is called with a valid browse definition id', () => {
@@ -204,7 +196,6 @@ describe('BrowseService', () => {
     describe('if getBrowseDefinitions fires', () => {
       beforeEach(() => {
         requestService = getMockRequestService(getRequestEntry$(true));
-        rdbService = getMockRemoteDataBuildService();
         service = initTestService();
         spyOn(service, 'getBrowseDefinitions').and
           .returnValue(hot('--a-', {
@@ -259,7 +250,6 @@ describe('BrowseService', () => {
     describe('if getBrowseDefinitions doesn\'t fire', () => {
       it('should return undefined', () => {
         requestService = getMockRequestService(getRequestEntry$(true));
-        rdbService = getMockRemoteDataBuildService();
         service = initTestService();
         spyOn(service, 'getBrowseDefinitions').and
           .returnValue(hot('----'));
@@ -277,9 +267,7 @@ describe('BrowseService', () => {
   describe('getFirstItemFor', () => {
     beforeEach(() => {
       requestService = getMockRequestService();
-      rdbService = getMockRemoteDataBuildService();
       service = initTestService();
-      spyOn(rdbService, 'buildList').and.callThrough();
     });
 
     describe('when getFirstItemFor is called with a valid browse definition id', () => {

src/app/core/browse/browse.service.ts

@@ -2,7 +2,6 @@ import { Injectable } from '@angular/core';
 import { Observable } from 'rxjs';
 import { distinctUntilChanged, map, startWith } from 'rxjs/operators';
 import { hasValue, hasValueOperator, isEmpty, isNotEmpty } from '../../shared/empty.util';
-import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service';
 import { PaginatedList } from '../data/paginated-list.model';
 import { RemoteData } from '../data/remote-data';
 import { RequestService } from '../data/request.service';
@@ -24,11 +23,18 @@ import { HrefOnlyDataService } from '../data/href-only-data.service';
 import { followLink, FollowLinkConfig } from '../../shared/utils/follow-link-config.model';
 import { BrowseDefinitionDataService } from './browse-definition-data.service';
 import { SortDirection } from '../cache/models/sort-options.model';
+import { environment } from '../../../environments/environment';
 
 
-export const BROWSE_LINKS_TO_FOLLOW: FollowLinkConfig<BrowseEntry | Item>[] = [
-  followLink('thumbnail')
-];
+export function getBrowseLinksToFollow(): FollowLinkConfig<BrowseEntry | Item>[] {
+  const followLinks = [
+    followLink('thumbnail'),
+  ];
+  if (environment.item.showAccessStatuses) {
+    followLinks.push(followLink('accessStatus'));
+  }
+  return followLinks;
+}
 
 /**
  * The service handling all browse requests
@@ -55,7 +61,6 @@ export class BrowseService {
     protected halService: HALEndpointService,
     private browseDefinitionDataService: BrowseDefinitionDataService,
     private hrefOnlyDataService: HrefOnlyDataService,
-    private rdb: RemoteDataBuildService,
   ) {
   }
 
@@ -105,7 +110,7 @@ export class BrowseService {
       })
     );
     if (options.fetchThumbnail ) {
-      return this.hrefOnlyDataService.findListByHref<BrowseEntry>(href$, {}, undefined, undefined, ...BROWSE_LINKS_TO_FOLLOW);
+      return this.hrefOnlyDataService.findListByHref<BrowseEntry>(href$, {}, undefined, undefined, ...getBrowseLinksToFollow());
     }
     return this.hrefOnlyDataService.findListByHref<BrowseEntry>(href$);
   }
@@ -153,7 +158,7 @@ export class BrowseService {
       }),
     );
     if (options.fetchThumbnail) {
-      return this.hrefOnlyDataService.findListByHref<Item>(href$, {}, undefined, undefined, ...BROWSE_LINKS_TO_FOLLOW);
+      return this.hrefOnlyDataService.findListByHref<Item>(href$, {}, undefined, undefined, ...getBrowseLinksToFollow());
     }
     return this.hrefOnlyDataService.findListByHref<Item>(href$);
   }

src/app/core/cache/object-cache.reducer.ts

@@ -166,20 +166,25 @@ export function objectCacheReducer(state = initialState, action: ObjectCacheActi
  *    the new state, with the object added, or overwritten.
  */
 function addToObjectCache(state: ObjectCacheState, action: AddToObjectCacheAction): ObjectCacheState {
-  const existing = state[action.payload.objectToCache._links.self.href] || {} as any;
+  const cacheLink = hasValue(action.payload.objectToCache?._links?.self) ? action.payload.objectToCache._links.self.href : action.payload.alternativeLink;
+  const existing = state[cacheLink] || {} as any;
   const newAltLinks = hasValue(action.payload.alternativeLink) ? [action.payload.alternativeLink] : [];
-  return Object.assign({}, state, {
-    [action.payload.objectToCache._links.self.href]: {
-      data: action.payload.objectToCache,
-      timeCompleted: action.payload.timeCompleted,
-      msToLive: action.payload.msToLive,
-      requestUUIDs: [action.payload.requestUUID, ...(existing.requestUUIDs || [])],
-      dependentRequestUUIDs: existing.dependentRequestUUIDs || [],
-      isDirty: isNotEmpty(existing.patches),
-      patches: existing.patches || [],
-      alternativeLinks: [...(existing.alternativeLinks || []), ...newAltLinks]
-    } as ObjectCacheEntry
-  });
+  if (hasValue(cacheLink)) {
+    return Object.assign({}, state, {
+      [cacheLink]: {
+        data: action.payload.objectToCache,
+        timeCompleted: action.payload.timeCompleted,
+        msToLive: action.payload.msToLive,
+        requestUUIDs: [action.payload.requestUUID, ...(existing.requestUUIDs || [])],
+        dependentRequestUUIDs: existing.dependentRequestUUIDs || [],
+        isDirty: isNotEmpty(existing.patches),
+        patches: existing.patches || [],
+        alternativeLinks: [...(existing.alternativeLinks || []), ...newAltLinks]
+      } as ObjectCacheEntry
+    });
+  } else {
+    return state;
+  }
 }
 
 /**

src/app/core/cache/object-cache.service.ts

@@ -63,7 +63,9 @@ export class ObjectCacheService {
    *    An optional alternative link to this object
    */
   add(object: CacheableObject, msToLive: number, requestUUID: string, alternativeLink?: string): void {
-    object = this.linkService.removeResolvedLinks(object); // Ensure the object we're storing has no resolved links
+    if (hasValue(object)) {
+      object = this.linkService.removeResolvedLinks(object); // Ensure the object we're storing has no resolved links
+    }
     this.store.dispatch(new AddToObjectCacheAction(object, new Date().getTime(), msToLive, requestUUID, alternativeLink));
   }
 
@@ -139,11 +141,15 @@ export class ObjectCacheService {
         }
       ),
       map((entry: ObjectCacheEntry) => {
-        const type: GenericConstructor<T> = getClassForType((entry.data as any).type);
-        if (typeof type !== 'function') {
-          throw new Error(`${type} is not a valid constructor for ${JSON.stringify(entry.data)}`);
+        if (hasValue(entry.data)) {
+          const type: GenericConstructor<T> = getClassForType((entry.data as any).type);
+          if (typeof type !== 'function') {
+            throw new Error(`${type} is not a valid constructor for ${JSON.stringify(entry.data)}`);
+          }
+          return Object.assign(new type(), entry.data) as T;
+        } else {
+          return null;
         }
-        return Object.assign(new type(), entry.data) as T;
       })
     );
   }

src/app/core/data/dspace-rest-response-parsing.service.ts

@@ -109,6 +109,13 @@ export class DspaceRestResponseParsingService implements ResponseParsingService
               if (hasValue(match)) {
                 embedAltUrl = new URLCombiner(embedAltUrl, `?size=${match.size}`).toString();
               }
+              if (data._embedded[property] == null) {
+                // Embedded object is null, meaning it exists (not undefined), but had an empty response (204) -> cache it as null
+                this.addToObjectCache(null, request, data, embedAltUrl);
+              } else if (!isCacheableObject(data._embedded[property])) {
+                // Embedded object exists, but doesn't contain a self link -> cache it using the alternative link instead
+                this.objectCache.add(data._embedded[property], hasValue(request.responseMsToLive) ? request.responseMsToLive : environment.cache.msToLive.default, request.uuid, embedAltUrl);
+              }
               this.process<ObjectDomain>(data._embedded[property], request, embedAltUrl);
             });
         }
@@ -226,7 +233,7 @@ export class DspaceRestResponseParsingService implements ResponseParsingService
    * @param alternativeURL  an alternative url that can be used to retrieve the object
    */
   addToObjectCache(co: CacheableObject, request: RestRequest, data: any, alternativeURL?: string): void {
-    if (!isCacheableObject(co)) {
+    if (hasValue(co) && !isCacheableObject(co)) {
       const type = hasValue(data) && hasValue(data.type) ? data.type : 'object';
       let dataJSON: string;
       if (hasValue(data._embedded)) {
@@ -240,7 +247,7 @@ export class DspaceRestResponseParsingService implements ResponseParsingService
       return;
     }
 
-    if (alternativeURL === co._links.self.href) {
+    if (hasValue(co) && alternativeURL === co._links.self.href) {
       alternativeURL = undefined;
     }
 

src/app/core/data/relationship-data.service.spec.ts

@@ -23,6 +23,7 @@ import { FindListOptions } from './find-list-options.model';
 import { testSearchDataImplementation } from './base/search-data.spec';
 import { MetadataValue } from '../shared/metadata.models';
 import { MetadataRepresentationType } from '../shared/metadata-representation/metadata-representation.model';
+import { environment } from '../../../environments/environment.test';
 
 describe('RelationshipDataService', () => {
   let service: RelationshipDataService;
@@ -138,6 +139,7 @@ describe('RelationshipDataService', () => {
       itemService,
       null,
       jasmine.createSpy('paginatedRelationsToItems').and.returnValue((v) => v),
+      environment,
     );
   }
 
@@ -153,7 +155,7 @@ describe('RelationshipDataService', () => {
   });
 
   describe('composition', () => {
-    const initService = () => new RelationshipDataService(null, null, null, null, null, null, null);
+    const initService = () => new RelationshipDataService(null, null, null, null, null, null, null, environment);
 
     testSearchDataImplementation(initService);
   });

src/app/core/data/relationship-data.service.ts

@@ -51,6 +51,7 @@ import { MetadataRepresentation } from '../shared/metadata-representation/metada
 import { MetadatumRepresentation } from '../shared/metadata-representation/metadatum/metadatum-representation.model';
 import { ItemMetadataRepresentation } from '../shared/metadata-representation/item/item-metadata-representation.model';
 import { DSpaceObject } from '../shared/dspace-object.model';
+import { APP_CONFIG, AppConfig } from '../../../config/app-config.interface';
 
 const relationshipListsStateSelector = (state: AppState) => state.relationshipLists;
 
@@ -92,6 +93,7 @@ export class RelationshipDataService extends IdentifiableDataService<Relationshi
     protected itemService: ItemDataService,
     protected appStore: Store<AppState>,
     @Inject(PAGINATED_RELATIONS_TO_ITEMS_OPERATOR) private paginatedRelationsToItems: (thisId: string) => (source: Observable<RemoteData<PaginatedList<Relationship>>>) => Observable<RemoteData<PaginatedList<Item>>>,
+    @Inject(APP_CONFIG) private appConfig: AppConfig,
   ) {
     super('relationships', requestService, rdbService, objectCache, halService, 15 * 60 * 1000);
 
@@ -278,7 +280,7 @@ export class RelationshipDataService extends IdentifiableDataService<Relationshi
    * @param options
    */
   getRelatedItemsByLabel(item: Item, label: string, options?: FindListOptions): Observable<RemoteData<PaginatedList<Item>>> {
-    let linksToFollow: FollowLinkConfig<Relationship>[] = itemLinksToFollow(options.fetchThumbnail);
+    let linksToFollow: FollowLinkConfig<Relationship>[] = itemLinksToFollow(options.fetchThumbnail, this.appConfig.item.showAccessStatuses);
     linksToFollow.push(followLink('relationshipType'));
 
     return this.getItemRelationshipsByLabel(item, label, options, true, true, ...linksToFollow).pipe(this.paginatedRelationsToItems(item.uuid));

src/app/core/index/index.effects.ts

@@ -27,7 +27,7 @@ export class UUIDIndexEffects {
    addObject$ = createEffect(() => this.actions$
     .pipe(
       ofType(ObjectCacheActionTypes.ADD),
-      filter((action: AddToObjectCacheAction) => hasValue(action.payload.objectToCache.uuid)),
+      filter((action: AddToObjectCacheAction) => hasValue(action.payload.objectToCache) && hasValue(action.payload.objectToCache.uuid)),
       map((action: AddToObjectCacheAction) => {
         return new AddToIndexAction(
           IndexName.OBJECT,
@@ -46,7 +46,7 @@ export class UUIDIndexEffects {
       ofType(ObjectCacheActionTypes.ADD),
       map((action: AddToObjectCacheAction) => {
         const alternativeLink = action.payload.alternativeLink;
-        const selfLink = action.payload.objectToCache._links.self.href;
+        const selfLink = hasValue(action.payload.objectToCache?._links?.self) ? action.payload.objectToCache._links.self.href : alternativeLink;
         if (hasValue(alternativeLink) && alternativeLink !== selfLink) {
           return new AddToIndexAction(
             IndexName.ALTERNATIVE_OBJECT_LINK,

src/app/home-page/recent-item-list/recent-item-list.component.ts

@@ -64,6 +64,9 @@ export class RecentItemListComponent implements OnInit {
     if (this.appConfig.browseBy.showThumbnails) {
       linksToFollow.push(followLink('thumbnail'));
     }
+    if (this.appConfig.item.showAccessStatuses) {
+      linksToFollow.push(followLink('accessStatus'));
+    }
 
     this.itemRD$ = this.searchService.search(
       new PaginatedSearchOptions({

src/app/item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts

@@ -13,7 +13,7 @@ import { environment } from '../../../../environments/environment';
 import { getItemPageRoute } from '../../item-page-routing-paths';
 import { getAllSucceededRemoteData } from '../../../core/shared/operators';
 import { hasValue } from '../../../shared/empty.util';
-import { ITEM_PAGE_LINKS_TO_FOLLOW } from '../../item.resolver';
+import { getItemPageLinksToFollow } from '../../item.resolver';
 import { FieldUpdate } from '../../../core/data/object-updates/field-update.model';
 import { FieldUpdates } from '../../../core/data/object-updates/field-updates.model';
 
@@ -72,7 +72,7 @@ export class AbstractItemUpdateComponent extends AbstractTrackableComponent impl
           this.item = rd.payload;
         }),
         switchMap((rd: RemoteData<Item>) => {
-          return this.itemService.findByHref(rd.payload._links.self.href, true, true, ...ITEM_PAGE_LINKS_TO_FOLLOW);
+          return this.itemService.findByHref(rd.payload._links.self.href, true, true, ...getItemPageLinksToFollow());
         }),
         getAllSucceededRemoteData()
       ).subscribe((rd: RemoteData<Item>) => {

src/app/item-page/edit-item-page/item-delete/item-delete.component.ts

@@ -278,8 +278,8 @@ export class ItemDeleteComponent
     this.linkService.resolveLinks(
       relationship,
       followLink('relationshipType'),
-      followLink('leftItem'),
-      followLink('rightItem'),
+      followLink('leftItem', undefined, followLink<Item>('accessStatus')),
+      followLink('rightItem', undefined, followLink<Item>('accessStatus')),
     );
     return relationship.relationshipType.pipe(
       getFirstSucceededRemoteData(),

src/app/item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.spec.ts

@@ -36,6 +36,7 @@ import { APP_CONFIG } from '../../../../../config/app-config.interface';
 import { EditItemRelationshipsServiceStub } from '../../../../shared/testing/edit-item-relationships.service.stub';
 import { EditItemRelationshipsService } from '../edit-item-relationships.service';
 import { cold } from 'jasmine-marbles';
+import { environment } from '../../../../../environments/environment.test';
 
 describe('EditRelationshipListComponent', () => {
 
@@ -213,11 +214,11 @@ describe('EditRelationshipListComponent', () => {
 
     editItemRelationshipsService = new EditItemRelationshipsServiceStub();
 
-    const environmentUseThumbs = {
+    const environmentUseThumbs = Object.assign({}, environment, {
       browseBy: {
         showThumbnails: true
       }
-    };
+    });
 
     TestBed.configureTestingModule({
       imports: [SharedModule, TranslateModule.forRoot()],