Skip to content

Commit ecb0de7

Browse files
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
2 parents 86ae3e4 + 4af8899 commit ecb0de7

File tree

4 files changed

+40
-22
lines changed

4 files changed

+40
-22
lines changed

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

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -174,20 +174,25 @@ export function objectCacheReducer(state = initialState, action: ObjectCacheActi
174174
* the new state, with the object added, or overwritten.
175175
*/
176176
function addToObjectCache(state: ObjectCacheState, action: AddToObjectCacheAction): ObjectCacheState {
177-
const existing = state[action.payload.objectToCache._links.self.href] || {} as any;
177+
const cacheLink = hasValue(action.payload.objectToCache?._links?.self) ? action.payload.objectToCache._links.self.href : action.payload.alternativeLink;
178+
const existing = state[cacheLink] || {} as any;
178179
const newAltLinks = hasValue(action.payload.alternativeLink) ? [action.payload.alternativeLink] : [];
179-
return Object.assign({}, state, {
180-
[action.payload.objectToCache._links.self.href]: {
181-
data: action.payload.objectToCache,
182-
timeCompleted: action.payload.timeCompleted,
183-
msToLive: action.payload.msToLive,
184-
requestUUIDs: [action.payload.requestUUID, ...(existing.requestUUIDs || [])],
185-
dependentRequestUUIDs: existing.dependentRequestUUIDs || [],
186-
isDirty: isNotEmpty(existing.patches),
187-
patches: existing.patches || [],
188-
alternativeLinks: [...(existing.alternativeLinks || []), ...newAltLinks],
189-
} as ObjectCacheEntry,
190-
});
180+
if (hasValue(cacheLink)) {
181+
return Object.assign({}, state, {
182+
[cacheLink]: {
183+
data: action.payload.objectToCache,
184+
timeCompleted: action.payload.timeCompleted,
185+
msToLive: action.payload.msToLive,
186+
requestUUIDs: [action.payload.requestUUID, ...(existing.requestUUIDs || [])],
187+
dependentRequestUUIDs: existing.dependentRequestUUIDs || [],
188+
isDirty: isNotEmpty(existing.patches),
189+
patches: existing.patches || [],
190+
alternativeLinks: [...(existing.alternativeLinks || []), ...newAltLinks],
191+
} as ObjectCacheEntry,
192+
});
193+
} else {
194+
return state;
195+
}
191196
}
192197

193198
/**

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

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,9 @@ export class ObjectCacheService {
9999
* An optional alternative link to this object
100100
*/
101101
add(object: CacheableObject, msToLive: number, requestUUID: string, alternativeLink?: string): void {
102-
object = this.linkService.removeResolvedLinks(object); // Ensure the object we're storing has no resolved links
102+
if (hasValue(object)) {
103+
object = this.linkService.removeResolvedLinks(object); // Ensure the object we're storing has no resolved links
104+
}
103105
this.store.dispatch(new AddToObjectCacheAction(object, new Date().getTime(), msToLive, requestUUID, alternativeLink));
104106
}
105107

@@ -175,11 +177,15 @@ export class ObjectCacheService {
175177
},
176178
),
177179
map((entry: ObjectCacheEntry) => {
178-
const type: GenericConstructor<T> = getClassForType((entry.data as any).type);
179-
if (typeof type !== 'function') {
180-
throw new Error(`${type} is not a valid constructor for ${JSON.stringify(entry.data)}`);
180+
if (hasValue(entry.data)) {
181+
const type: GenericConstructor<T> = getClassForType((entry.data as any).type);
182+
if (typeof type !== 'function') {
183+
throw new Error(`${type} is not a valid constructor for ${JSON.stringify(entry.data)}`);
184+
}
185+
return Object.assign(new type(), entry.data) as T;
186+
} else {
187+
return null;
181188
}
182-
return Object.assign(new type(), entry.data) as T;
183189
}),
184190
);
185191
}

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,13 @@ export class DspaceRestResponseParsingService implements ResponseParsingService
120120
if (hasValue(match)) {
121121
embedAltUrl = new URLCombiner(embedAltUrl, `?size=${match.size}`).toString();
122122
}
123+
if (data._embedded[property] == null) {
124+
// Embedded object is null, meaning it exists (not undefined), but had an empty response (204) -> cache it as null
125+
this.addToObjectCache(null, request, data, embedAltUrl);
126+
} else if (!isCacheableObject(data._embedded[property])) {
127+
// Embedded object exists, but doesn't contain a self link -> cache it using the alternative link instead
128+
this.objectCache.add(data._embedded[property], hasValue(request.responseMsToLive) ? request.responseMsToLive : environment.cache.msToLive.default, request.uuid, embedAltUrl);
129+
}
123130
this.process<ObjectDomain>(data._embedded[property], request, embedAltUrl);
124131
});
125132
}
@@ -237,7 +244,7 @@ export class DspaceRestResponseParsingService implements ResponseParsingService
237244
* @param alternativeURL an alternative url that can be used to retrieve the object
238245
*/
239246
addToObjectCache(co: CacheableObject, request: RestRequest, data: any, alternativeURL?: string): void {
240-
if (!isCacheableObject(co)) {
247+
if (hasValue(co) && !isCacheableObject(co)) {
241248
const type = hasValue(data) && hasValue(data.type) ? data.type : 'object';
242249
let dataJSON: string;
243250
if (hasValue(data._embedded)) {
@@ -251,7 +258,7 @@ export class DspaceRestResponseParsingService implements ResponseParsingService
251258
return;
252259
}
253260

254-
if (alternativeURL === co._links.self.href) {
261+
if (hasValue(co) && alternativeURL === co._links.self.href) {
255262
alternativeURL = undefined;
256263
}
257264

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ export class UUIDIndexEffects {
4545
addObject$ = createEffect(() => this.actions$
4646
.pipe(
4747
ofType(ObjectCacheActionTypes.ADD),
48-
filter((action: AddToObjectCacheAction) => hasValue(action.payload.objectToCache.uuid)),
48+
filter((action: AddToObjectCacheAction) => hasValue(action.payload.objectToCache) && hasValue(action.payload.objectToCache.uuid)),
4949
map((action: AddToObjectCacheAction) => {
5050
return new AddToIndexAction(
5151
IndexName.OBJECT,
@@ -64,7 +64,7 @@ export class UUIDIndexEffects {
6464
ofType(ObjectCacheActionTypes.ADD),
6565
map((action: AddToObjectCacheAction) => {
6666
const alternativeLink = action.payload.alternativeLink;
67-
const selfLink = action.payload.objectToCache._links.self.href;
67+
const selfLink = hasValue(action.payload.objectToCache?._links?.self) ? action.payload.objectToCache._links.self.href : alternativeLink;
6868
if (hasValue(alternativeLink) && alternativeLink !== selfLink) {
6969
return new AddToIndexAction(
7070
IndexName.ALTERNATIVE_OBJECT_LINK,

0 commit comments

Comments
 (0)