Skip to content

Commit 4af8899

Browse files
Merge branch 'w2p-107155_Performance-re-request-embeds-7.4' into w2p-107155_Performance-re-request-embeds-7.6
2 parents 404ccd9 + 984c9bf commit 4af8899

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
@@ -166,20 +166,25 @@ export function objectCacheReducer(state = initialState, action: ObjectCacheActi
166166
* the new state, with the object added, or overwritten.
167167
*/
168168
function addToObjectCache(state: ObjectCacheState, action: AddToObjectCacheAction): ObjectCacheState {
169-
const existing = state[action.payload.objectToCache._links.self.href] || {} as any;
169+
const cacheLink = hasValue(action.payload.objectToCache?._links?.self) ? action.payload.objectToCache._links.self.href : action.payload.alternativeLink;
170+
const existing = state[cacheLink] || {} as any;
170171
const newAltLinks = hasValue(action.payload.alternativeLink) ? [action.payload.alternativeLink] : [];
171-
return Object.assign({}, state, {
172-
[action.payload.objectToCache._links.self.href]: {
173-
data: action.payload.objectToCache,
174-
timeCompleted: action.payload.timeCompleted,
175-
msToLive: action.payload.msToLive,
176-
requestUUIDs: [action.payload.requestUUID, ...(existing.requestUUIDs || [])],
177-
dependentRequestUUIDs: existing.dependentRequestUUIDs || [],
178-
isDirty: isNotEmpty(existing.patches),
179-
patches: existing.patches || [],
180-
alternativeLinks: [...(existing.alternativeLinks || []), ...newAltLinks]
181-
} as ObjectCacheEntry
182-
});
172+
if (hasValue(cacheLink)) {
173+
return Object.assign({}, state, {
174+
[cacheLink]: {
175+
data: action.payload.objectToCache,
176+
timeCompleted: action.payload.timeCompleted,
177+
msToLive: action.payload.msToLive,
178+
requestUUIDs: [action.payload.requestUUID, ...(existing.requestUUIDs || [])],
179+
dependentRequestUUIDs: existing.dependentRequestUUIDs || [],
180+
isDirty: isNotEmpty(existing.patches),
181+
patches: existing.patches || [],
182+
alternativeLinks: [...(existing.alternativeLinks || []), ...newAltLinks]
183+
} as ObjectCacheEntry
184+
});
185+
} else {
186+
return state;
187+
}
183188
}
184189

185190
/**

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

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,9 @@ export class ObjectCacheService {
6363
* An optional alternative link to this object
6464
*/
6565
add(object: CacheableObject, msToLive: number, requestUUID: string, alternativeLink?: string): void {
66-
object = this.linkService.removeResolvedLinks(object); // Ensure the object we're storing has no resolved links
66+
if (hasValue(object)) {
67+
object = this.linkService.removeResolvedLinks(object); // Ensure the object we're storing has no resolved links
68+
}
6769
this.store.dispatch(new AddToObjectCacheAction(object, new Date().getTime(), msToLive, requestUUID, alternativeLink));
6870
}
6971

@@ -139,11 +141,15 @@ export class ObjectCacheService {
139141
}
140142
),
141143
map((entry: ObjectCacheEntry) => {
142-
const type: GenericConstructor<T> = getClassForType((entry.data as any).type);
143-
if (typeof type !== 'function') {
144-
throw new Error(`${type} is not a valid constructor for ${JSON.stringify(entry.data)}`);
144+
if (hasValue(entry.data)) {
145+
const type: GenericConstructor<T> = getClassForType((entry.data as any).type);
146+
if (typeof type !== 'function') {
147+
throw new Error(`${type} is not a valid constructor for ${JSON.stringify(entry.data)}`);
148+
}
149+
return Object.assign(new type(), entry.data) as T;
150+
} else {
151+
return null;
145152
}
146-
return Object.assign(new type(), entry.data) as T;
147153
})
148154
);
149155
}

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,13 @@ export class DspaceRestResponseParsingService implements ResponseParsingService
109109
if (hasValue(match)) {
110110
embedAltUrl = new URLCombiner(embedAltUrl, `?size=${match.size}`).toString();
111111
}
112+
if (data._embedded[property] == null) {
113+
// Embedded object is null, meaning it exists (not undefined), but had an empty response (204) -> cache it as null
114+
this.addToObjectCache(null, request, data, embedAltUrl);
115+
} else if (!isCacheableObject(data._embedded[property])) {
116+
// Embedded object exists, but doesn't contain a self link -> cache it using the alternative link instead
117+
this.objectCache.add(data._embedded[property], hasValue(request.responseMsToLive) ? request.responseMsToLive : environment.cache.msToLive.default, request.uuid, embedAltUrl);
118+
}
112119
this.process<ObjectDomain>(data._embedded[property], request, embedAltUrl);
113120
});
114121
}
@@ -226,7 +233,7 @@ export class DspaceRestResponseParsingService implements ResponseParsingService
226233
* @param alternativeURL an alternative url that can be used to retrieve the object
227234
*/
228235
addToObjectCache(co: CacheableObject, request: RestRequest, data: any, alternativeURL?: string): void {
229-
if (!isCacheableObject(co)) {
236+
if (hasValue(co) && !isCacheableObject(co)) {
230237
const type = hasValue(data) && hasValue(data.type) ? data.type : 'object';
231238
let dataJSON: string;
232239
if (hasValue(data._embedded)) {
@@ -240,7 +247,7 @@ export class DspaceRestResponseParsingService implements ResponseParsingService
240247
return;
241248
}
242249

243-
if (alternativeURL === co._links.self.href) {
250+
if (hasValue(co) && alternativeURL === co._links.self.href) {
244251
alternativeURL = undefined;
245252
}
246253

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ export class UUIDIndexEffects {
2727
addObject$ = createEffect(() => this.actions$
2828
.pipe(
2929
ofType(ObjectCacheActionTypes.ADD),
30-
filter((action: AddToObjectCacheAction) => hasValue(action.payload.objectToCache.uuid)),
30+
filter((action: AddToObjectCacheAction) => hasValue(action.payload.objectToCache) && hasValue(action.payload.objectToCache.uuid)),
3131
map((action: AddToObjectCacheAction) => {
3232
return new AddToIndexAction(
3333
IndexName.OBJECT,
@@ -46,7 +46,7 @@ export class UUIDIndexEffects {
4646
ofType(ObjectCacheActionTypes.ADD),
4747
map((action: AddToObjectCacheAction) => {
4848
const alternativeLink = action.payload.alternativeLink;
49-
const selfLink = action.payload.objectToCache._links.self.href;
49+
const selfLink = hasValue(action.payload.objectToCache?._links?.self) ? action.payload.objectToCache._links.self.href : alternativeLink;
5050
if (hasValue(alternativeLink) && alternativeLink !== selfLink) {
5151
return new AddToIndexAction(
5252
IndexName.ALTERNATIVE_OBJECT_LINK,

0 commit comments

Comments
 (0)