Merge branch 'w2p-107155_Performance-re-request-embeds-7.6'

# Conflicts: # src/app/core/cache/object-cache.reducer.ts # src/app/core/cache/object-cache.service.ts
alexandrevryghem · Oct 11, 2024 · ecb0de7 · ecb0de7
2 parents 86ae3e4 + 4af8899
commit ecb0de7
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 22 deletions.
diff --git a/src/app/core/cache/object-cache.reducer.ts b/src/app/core/cache/object-cache.reducer.ts
@@ -174,20 +174,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;
+  }
 }
 
 /**

diff --git a/src/app/core/cache/object-cache.service.ts b/src/app/core/cache/object-cache.service.ts
@@ -99,7 +99,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));
   }
 
@@ -175,11 +177,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;
       }),
     );
   }

diff --git a/src/app/core/data/dspace-rest-response-parsing.service.ts b/src/app/core/data/dspace-rest-response-parsing.service.ts
@@ -120,6 +120,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);
             });
         }
@@ -237,7 +244,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)) {
@@ -251,7 +258,7 @@ export class DspaceRestResponseParsingService implements ResponseParsingService
       return;
     }
 
-    if (alternativeURL === co._links.self.href) {
+    if (hasValue(co) && alternativeURL === co._links.self.href) {
       alternativeURL = undefined;
     }
 

diff --git a/src/app/core/index/index.effects.ts b/src/app/core/index/index.effects.ts
@@ -45,7 +45,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,
@@ -64,7 +64,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,