Merge pull request #1846 from atmire/w2p-94273_Track-dependencies-in-order-to-invalidate-them-when-an-object-is-invalidated

Track dependencies in order to invalidate them when an object is invalidated
This commit is contained in:
Tim Donohue
2022-09-28 10:05:37 -05:00
committed by GitHub
13 changed files with 570 additions and 66 deletions

View File

@@ -13,7 +13,9 @@ export const ObjectCacheActionTypes = {
REMOVE: type('dspace/core/cache/object/REMOVE'), REMOVE: type('dspace/core/cache/object/REMOVE'),
RESET_TIMESTAMPS: type('dspace/core/cache/object/RESET_TIMESTAMPS'), RESET_TIMESTAMPS: type('dspace/core/cache/object/RESET_TIMESTAMPS'),
ADD_PATCH: type('dspace/core/cache/object/ADD_PATCH'), ADD_PATCH: type('dspace/core/cache/object/ADD_PATCH'),
APPLY_PATCH: type('dspace/core/cache/object/APPLY_PATCH') APPLY_PATCH: type('dspace/core/cache/object/APPLY_PATCH'),
ADD_DEPENDENTS: type('dspace/core/cache/object/ADD_DEPENDENTS'),
REMOVE_DEPENDENTS: type('dspace/core/cache/object/REMOVE_DEPENDENTS')
}; };
/** /**
@@ -126,6 +128,46 @@ export class ApplyPatchObjectCacheAction implements Action {
} }
} }
/**
* An NgRx action to add dependent request UUIDs to a cached object
*/
export class AddDependentsObjectCacheAction implements Action {
type = ObjectCacheActionTypes.ADD_DEPENDENTS;
payload: {
href: string;
dependentRequestUUIDs: string[];
};
/**
* Create a new AddDependentsObjectCacheAction
*
* @param href the self link of a cached object
* @param dependentRequestUUIDs the UUID of the request that depends on this object
*/
constructor(href: string, dependentRequestUUIDs: string[]) {
this.payload = {
href,
dependentRequestUUIDs,
};
}
}
/**
* An NgRx action to remove all dependent request UUIDs from a cached object
*/
export class RemoveDependentsObjectCacheAction implements Action {
type = ObjectCacheActionTypes.REMOVE_DEPENDENTS;
payload: string;
/**
* Create a new RemoveDependentsObjectCacheAction
*
* @param href the self link of a cached object for which to remove all dependent request UUIDs
*/
constructor(href: string) {
this.payload = href;
}
}
/** /**
* A type to encompass all ObjectCacheActions * A type to encompass all ObjectCacheActions
@@ -135,4 +177,6 @@ export type ObjectCacheAction
| RemoveFromObjectCacheAction | RemoveFromObjectCacheAction
| ResetObjectCacheTimestampsAction | ResetObjectCacheTimestampsAction
| AddPatchObjectCacheAction | AddPatchObjectCacheAction
| ApplyPatchObjectCacheAction; | ApplyPatchObjectCacheAction
| AddDependentsObjectCacheAction
| RemoveDependentsObjectCacheAction;

View File

@@ -2,11 +2,13 @@ import * as deepFreeze from 'deep-freeze';
import { Operation } from 'fast-json-patch'; import { Operation } from 'fast-json-patch';
import { Item } from '../shared/item.model'; import { Item } from '../shared/item.model';
import { import {
AddDependentsObjectCacheAction,
AddPatchObjectCacheAction, AddPatchObjectCacheAction,
AddToObjectCacheAction, AddToObjectCacheAction,
ApplyPatchObjectCacheAction, ApplyPatchObjectCacheAction,
RemoveDependentsObjectCacheAction,
RemoveFromObjectCacheAction, RemoveFromObjectCacheAction,
ResetObjectCacheTimestampsAction ResetObjectCacheTimestampsAction,
} from './object-cache.actions'; } from './object-cache.actions';
import { objectCacheReducer } from './object-cache.reducer'; import { objectCacheReducer } from './object-cache.reducer';
@@ -42,20 +44,22 @@ describe('objectCacheReducer', () => {
timeCompleted: new Date().getTime(), timeCompleted: new Date().getTime(),
msToLive: 900000, msToLive: 900000,
requestUUIDs: [requestUUID1], requestUUIDs: [requestUUID1],
dependentRequestUUIDs: [],
patches: [], patches: [],
isDirty: false, isDirty: false,
}, },
[selfLink2]: { [selfLink2]: {
data: { data: {
type: Item.type, type: Item.type,
self: requestUUID2, self: selfLink2,
foo: 'baz', foo: 'baz',
_links: { self: { href: requestUUID2 } } _links: { self: { href: selfLink2 } }
}, },
alternativeLinks: [altLink3, altLink4], alternativeLinks: [altLink3, altLink4],
timeCompleted: new Date().getTime(), timeCompleted: new Date().getTime(),
msToLive: 900000, msToLive: 900000,
requestUUIDs: [selfLink2], requestUUIDs: [requestUUID2],
dependentRequestUUIDs: [requestUUID1],
patches: [], patches: [],
isDirty: false isDirty: false
} }
@@ -189,4 +193,20 @@ describe('objectCacheReducer', () => {
expect((newState[selfLink1].data as any).name).toEqual(newName); expect((newState[selfLink1].data as any).name).toEqual(newName);
}); });
it('should add dependent requests on ADD_DEPENDENTS', () => {
let newState = objectCacheReducer(testState, new AddDependentsObjectCacheAction(selfLink1, ['new', 'newer', 'newest']));
expect(newState[selfLink1].dependentRequestUUIDs).toEqual(['new', 'newer', 'newest']);
newState = objectCacheReducer(newState, new AddDependentsObjectCacheAction(selfLink2, ['more']));
expect(newState[selfLink2].dependentRequestUUIDs).toEqual([requestUUID1, 'more']);
});
it('should clear dependent requests on REMOVE_DEPENDENTS', () => {
let newState = objectCacheReducer(testState, new RemoveDependentsObjectCacheAction(selfLink1));
expect(newState[selfLink1].dependentRequestUUIDs).toEqual([]);
newState = objectCacheReducer(newState, new RemoveDependentsObjectCacheAction(selfLink2));
expect(newState[selfLink2].dependentRequestUUIDs).toEqual([]);
});
}); });

View File

@@ -1,12 +1,13 @@
/* eslint-disable max-classes-per-file */ /* eslint-disable max-classes-per-file */
import { import {
AddDependentsObjectCacheAction,
AddPatchObjectCacheAction, AddPatchObjectCacheAction,
AddToObjectCacheAction, AddToObjectCacheAction,
ApplyPatchObjectCacheAction, ApplyPatchObjectCacheAction,
ObjectCacheAction, ObjectCacheAction,
ObjectCacheActionTypes, ObjectCacheActionTypes, RemoveDependentsObjectCacheAction,
RemoveFromObjectCacheAction, RemoveFromObjectCacheAction,
ResetObjectCacheTimestampsAction ResetObjectCacheTimestampsAction,
} from './object-cache.actions'; } from './object-cache.actions';
import { hasValue, isNotEmpty } from '../../shared/empty.util'; import { hasValue, isNotEmpty } from '../../shared/empty.util';
import { CacheEntry } from './cache-entry'; import { CacheEntry } from './cache-entry';
@@ -69,6 +70,12 @@ export class ObjectCacheEntry implements CacheEntry {
*/ */
requestUUIDs: string[]; requestUUIDs: string[];
/**
* A list of UUIDs for the requests that depend on this object.
* When this object is invalidated, these requests will be invalidated as well.
*/
dependentRequestUUIDs: string[];
/** /**
* An array of patches that were made on the client side to this entry, but haven't been sent to the server yet * An array of patches that were made on the client side to this entry, but haven't been sent to the server yet
*/ */
@@ -134,6 +141,14 @@ export function objectCacheReducer(state = initialState, action: ObjectCacheActi
return applyPatchObjectCache(state, action as ApplyPatchObjectCacheAction); return applyPatchObjectCache(state, action as ApplyPatchObjectCacheAction);
} }
case ObjectCacheActionTypes.ADD_DEPENDENTS: {
return addDependentsObjectCacheState(state, action as AddDependentsObjectCacheAction);
}
case ObjectCacheActionTypes.REMOVE_DEPENDENTS: {
return removeDependentsObjectCacheState(state, action as RemoveDependentsObjectCacheAction);
}
default: { default: {
return state; return state;
} }
@@ -159,6 +174,7 @@ function addToObjectCache(state: ObjectCacheState, action: AddToObjectCacheActio
timeCompleted: action.payload.timeCompleted, timeCompleted: action.payload.timeCompleted,
msToLive: action.payload.msToLive, msToLive: action.payload.msToLive,
requestUUIDs: [action.payload.requestUUID, ...(existing.requestUUIDs || [])], requestUUIDs: [action.payload.requestUUID, ...(existing.requestUUIDs || [])],
dependentRequestUUIDs: existing.dependentRequestUUIDs || [],
isDirty: isNotEmpty(existing.patches), isDirty: isNotEmpty(existing.patches),
patches: existing.patches || [], patches: existing.patches || [],
alternativeLinks: [...(existing.alternativeLinks || []), ...newAltLinks] alternativeLinks: [...(existing.alternativeLinks || []), ...newAltLinks]
@@ -252,3 +268,49 @@ function applyPatchObjectCache(state: ObjectCacheState, action: ApplyPatchObject
} }
return newState; return newState;
} }
/**
* Add a list of dependent request UUIDs to a cached object, used when defining new dependencies
*
* @param state the current state
* @param action an AddDependentsObjectCacheAction
* @return the new state, with the dependent requests of the cached object updated
*/
function addDependentsObjectCacheState(state: ObjectCacheState, action: AddDependentsObjectCacheAction): ObjectCacheState {
const href = action.payload.href;
const newState = Object.assign({}, state);
if (hasValue(newState[href])) {
newState[href] = Object.assign({}, newState[href], {
dependentRequestUUIDs: [
...new Set([
...newState[href]?.dependentRequestUUIDs || [],
...action.payload.dependentRequestUUIDs,
])
]
});
}
return newState;
}
/**
* Remove all dependent request UUIDs from a cached object, used to clear out-of-date depedencies
*
* @param state the current state
* @param action an AddDependentsObjectCacheAction
* @return the new state, with the dependent requests of the cached object updated
*/
function removeDependentsObjectCacheState(state: ObjectCacheState, action: RemoveDependentsObjectCacheAction): ObjectCacheState {
const href = action.payload;
const newState = Object.assign({}, state);
if (hasValue(newState[href])) {
newState[href] = Object.assign({}, newState[href], {
dependentRequestUUIDs: []
});
}
return newState;
}

View File

@@ -11,10 +11,12 @@ import { coreReducers} from '../core.reducers';
import { RestRequestMethod } from '../data/rest-request-method'; import { RestRequestMethod } from '../data/rest-request-method';
import { Item } from '../shared/item.model'; import { Item } from '../shared/item.model';
import { import {
AddDependentsObjectCacheAction,
RemoveDependentsObjectCacheAction,
AddPatchObjectCacheAction, AddPatchObjectCacheAction,
AddToObjectCacheAction, AddToObjectCacheAction,
ApplyPatchObjectCacheAction, ApplyPatchObjectCacheAction,
RemoveFromObjectCacheAction RemoveFromObjectCacheAction,
} from './object-cache.actions'; } from './object-cache.actions';
import { Patch } from './object-cache.reducer'; import { Patch } from './object-cache.reducer';
import { ObjectCacheService } from './object-cache.service'; import { ObjectCacheService } from './object-cache.service';
@@ -25,6 +27,7 @@ import { storeModuleConfig } from '../../app.reducer';
import { TestColdObservable } from 'jasmine-marbles/src/test-observables'; import { TestColdObservable } from 'jasmine-marbles/src/test-observables';
import { IndexName } from '../index/index-name.model'; import { IndexName } from '../index/index-name.model';
import { CoreState } from '../core-state.model'; import { CoreState } from '../core-state.model';
import { TestScheduler } from 'rxjs/testing';
describe('ObjectCacheService', () => { describe('ObjectCacheService', () => {
let service: ObjectCacheService; let service: ObjectCacheService;
@@ -38,6 +41,7 @@ describe('ObjectCacheService', () => {
let altLink1; let altLink1;
let altLink2; let altLink2;
let requestUUID; let requestUUID;
let requestUUID2;
let alternativeLink; let alternativeLink;
let timestamp; let timestamp;
let timestamp2; let timestamp2;
@@ -55,6 +59,7 @@ describe('ObjectCacheService', () => {
altLink1 = 'https://alternative.link/endpoint/1234'; altLink1 = 'https://alternative.link/endpoint/1234';
altLink2 = 'https://alternative.link/endpoint/5678'; altLink2 = 'https://alternative.link/endpoint/5678';
requestUUID = '4d3a4ce8-a375-4b98-859b-39f0a014d736'; requestUUID = '4d3a4ce8-a375-4b98-859b-39f0a014d736';
requestUUID2 = 'c0f486c1-c4d3-4a03-b293-ca5b71ff0054';
alternativeLink = 'https://rest.api/endpoint/5e4f8a5-be98-4c51-9fd8-6bfedcbd59b7/item'; alternativeLink = 'https://rest.api/endpoint/5e4f8a5-be98-4c51-9fd8-6bfedcbd59b7/item';
timestamp = new Date().getTime(); timestamp = new Date().getTime();
timestamp2 = new Date().getTime() - 200; timestamp2 = new Date().getTime() - 200;
@@ -71,13 +76,17 @@ describe('ObjectCacheService', () => {
data: objectToCache, data: objectToCache,
timeCompleted: timestamp, timeCompleted: timestamp,
msToLive: msToLive, msToLive: msToLive,
alternativeLinks: [altLink1, altLink2] alternativeLinks: [altLink1, altLink2],
requestUUIDs: [requestUUID],
dependentRequestUUIDs: [],
}; };
cacheEntry2 = { cacheEntry2 = {
data: objectToCache, data: objectToCache,
timeCompleted: timestamp2, timeCompleted: timestamp2,
msToLive: msToLive2, msToLive: msToLive2,
alternativeLinks: [altLink2] alternativeLinks: [altLink2],
requestUUIDs: [requestUUID2],
dependentRequestUUIDs: [],
}; };
invalidCacheEntry = Object.assign({}, cacheEntry, { msToLive: -1 }); invalidCacheEntry = Object.assign({}, cacheEntry, { msToLive: -1 });
operations = [{ op: 'replace', path: '/name', value: 'random string' } as Operation]; operations = [{ op: 'replace', path: '/name', value: 'random string' } as Operation];
@@ -343,4 +352,122 @@ describe('ObjectCacheService', () => {
expect(store.dispatch).toHaveBeenCalledWith(new ApplyPatchObjectCacheAction(selfLink)); expect(store.dispatch).toHaveBeenCalledWith(new ApplyPatchObjectCacheAction(selfLink));
}); });
}); });
describe('request dependencies', () => {
beforeEach(() => {
const state = Object.assign({}, initialState, {
core: Object.assign({}, initialState.core, {
'cache/object': {
['objectWithoutDependents']: {
dependentRequestUUIDs: [],
},
['objectWithDependents']: {
dependentRequestUUIDs: [requestUUID],
},
[selfLink]: cacheEntry,
},
'index': {
'object/alt-link-to-self-link': {
[anotherLink]: selfLink,
['objectWithoutDependentsAlt']: 'objectWithoutDependents',
['objectWithDependentsAlt']: 'objectWithDependents',
}
}
})
});
mockStore.setState(state);
});
describe('addDependency', () => {
it('should dispatch an ADD_DEPENDENTS action', () => {
service.addDependency(selfLink, 'objectWithoutDependents');
expect(store.dispatch).toHaveBeenCalledOnceWith(new AddDependentsObjectCacheAction('objectWithoutDependents', [requestUUID]));
});
it('should resolve alt links', () => {
service.addDependency(anotherLink, 'objectWithoutDependentsAlt');
expect(store.dispatch).toHaveBeenCalledOnceWith(new AddDependentsObjectCacheAction('objectWithoutDependents', [requestUUID]));
});
it('should not dispatch if either href cannot be resolved to a cached self link', () => {
service.addDependency(selfLink, 'unknown');
service.addDependency('unknown', 'objectWithoutDependents');
service.addDependency('nothing', 'matches');
expect(store.dispatch).not.toHaveBeenCalled();
});
it('should not dispatch if either href is undefined', () => {
service.addDependency(selfLink, undefined);
service.addDependency(undefined, 'objectWithoutDependents');
service.addDependency(undefined, undefined);
expect(store.dispatch).not.toHaveBeenCalled();
});
it('should not dispatch if the dependency exists already', () => {
service.addDependency(selfLink, 'objectWithDependents');
expect(store.dispatch).not.toHaveBeenCalled();
});
it('should work with observable hrefs', () => {
service.addDependency(observableOf(selfLink), observableOf('objectWithoutDependents'));
expect(store.dispatch).toHaveBeenCalledOnceWith(new AddDependentsObjectCacheAction('objectWithoutDependents', [requestUUID]));
});
it('should only dispatch once for the first value of either observable href', () => {
const testScheduler = new TestScheduler((actual, expected) => {
expect(actual).toEqual(expected);
});
testScheduler.run(({ cold: tsCold, flush }) => {
const href$ = tsCold('--y-n-n', {
y: selfLink,
n: 'NOPE'
});
const dependsOnHref$ = tsCold('-y-n-n', {
y: 'objectWithoutDependents',
n: 'NOPE'
});
service.addDependency(href$, dependsOnHref$);
flush();
expect(store.dispatch).toHaveBeenCalledOnceWith(new AddDependentsObjectCacheAction('objectWithoutDependents', [requestUUID]));
});
});
it('should not dispatch if either of the hrefs emits undefined', () => {
const testScheduler = new TestScheduler((actual, expected) => {
expect(actual).toEqual(expected);
});
testScheduler.run(({ cold: tsCold, flush }) => {
const undefined$ = tsCold('--u');
service.addDependency(selfLink, undefined$);
service.addDependency(undefined$, 'objectWithoutDependents');
service.addDependency(undefined$, undefined$);
flush();
expect(store.dispatch).not.toHaveBeenCalled();
});
});
});
describe('removeDependents', () => {
it('should dispatch a REMOVE_DEPENDENTS action', () => {
service.removeDependents('objectWithDependents');
expect(store.dispatch).toHaveBeenCalledOnceWith(new RemoveDependentsObjectCacheAction('objectWithDependents'));
});
it('should resolve alt links', () => {
service.removeDependents('objectWithDependentsAlt');
expect(store.dispatch).toHaveBeenCalledOnceWith(new RemoveDependentsObjectCacheAction('objectWithDependents'));
});
it('should not dispatch if the href cannot be resolved to a cached self link', () => {
service.removeDependents('unknown');
expect(store.dispatch).not.toHaveBeenCalled();
});
});
});
}); });

View File

@@ -4,23 +4,15 @@ import { applyPatch, Operation } from 'fast-json-patch';
import { combineLatest as observableCombineLatest, Observable, of as observableOf } from 'rxjs'; import { combineLatest as observableCombineLatest, Observable, of as observableOf } from 'rxjs';
import { distinctUntilChanged, filter, map, mergeMap, switchMap, take } from 'rxjs/operators'; import { distinctUntilChanged, filter, map, mergeMap, switchMap, take } from 'rxjs/operators';
import { hasValue, isNotEmpty, isEmpty } from '../../shared/empty.util'; import { hasNoValue, hasValue, isEmpty, isNotEmpty } from '../../shared/empty.util';
import { CoreState } from '../core-state.model'; import { CoreState } from '../core-state.model';
import { coreSelector } from '../core.selectors'; import { coreSelector } from '../core.selectors';
import { RestRequestMethod } from '../data/rest-request-method'; import { RestRequestMethod } from '../data/rest-request-method';
import { import { selfLinkFromAlternativeLinkSelector, selfLinkFromUuidSelector } from '../index/index.selectors';
selfLinkFromAlternativeLinkSelector,
selfLinkFromUuidSelector
} from '../index/index.selectors';
import { GenericConstructor } from '../shared/generic-constructor'; import { GenericConstructor } from '../shared/generic-constructor';
import { getClassForType } from './builders/build-decorators'; import { getClassForType } from './builders/build-decorators';
import { LinkService } from './builders/link.service'; import { LinkService } from './builders/link.service';
import { import { AddDependentsObjectCacheAction, AddPatchObjectCacheAction, AddToObjectCacheAction, ApplyPatchObjectCacheAction, RemoveDependentsObjectCacheAction, RemoveFromObjectCacheAction } from './object-cache.actions';
AddPatchObjectCacheAction,
AddToObjectCacheAction,
ApplyPatchObjectCacheAction,
RemoveFromObjectCacheAction
} from './object-cache.actions';
import { ObjectCacheEntry, ObjectCacheState } from './object-cache.reducer'; import { ObjectCacheEntry, ObjectCacheState } from './object-cache.reducer';
import { AddToSSBAction } from './server-sync-buffer.actions'; import { AddToSSBAction } from './server-sync-buffer.actions';
@@ -339,4 +331,97 @@ export class ObjectCacheService {
this.store.dispatch(new ApplyPatchObjectCacheAction(selfLink)); this.store.dispatch(new ApplyPatchObjectCacheAction(selfLink));
} }
/**
* Add a new dependency between two cached objects.
* When {@link dependsOnHref$} is invalidated, {@link href$} will be invalidated as well.
*
* This method should be called _after_ requests have been sent;
* it will only work if both objects are already present in the cache.
*
* If either object is undefined, the dependency will not be added
*
* @param href$ the href of an object to add a dependency to
* @param dependsOnHref$ the href of the new dependency
*/
addDependency(href$: string | Observable<string>, dependsOnHref$: string | Observable<string>) {
if (hasNoValue(href$) || hasNoValue(dependsOnHref$)) {
return;
}
if (typeof href$ === 'string') {
href$ = observableOf(href$);
}
if (typeof dependsOnHref$ === 'string') {
dependsOnHref$ = observableOf(dependsOnHref$);
}
observableCombineLatest([
href$,
dependsOnHref$.pipe(
switchMap(dependsOnHref => this.resolveSelfLink(dependsOnHref))
),
]).pipe(
switchMap(([href, dependsOnSelfLink]: [string, string]) => {
const dependsOnSelfLink$ = observableOf(dependsOnSelfLink);
return observableCombineLatest([
dependsOnSelfLink$,
dependsOnSelfLink$.pipe(
switchMap(selfLink => this.getBySelfLink(selfLink)),
map(oce => oce?.dependentRequestUUIDs || []),
),
this.getByHref(href).pipe(
// only add the latest request to keep dependency index from growing indefinitely
map((entry: ObjectCacheEntry) => entry?.requestUUIDs?.[0]),
)
]);
}),
take(1),
).subscribe(([dependsOnSelfLink, currentDependents, newDependent]: [string, string[], string]) => {
// don't dispatch if either href is invalid or if the new dependency already exists
if (hasValue(dependsOnSelfLink) && hasValue(newDependent) && !currentDependents.includes(newDependent)) {
this.store.dispatch(new AddDependentsObjectCacheAction(dependsOnSelfLink, [newDependent]));
}
});
}
/**
* Clear all dependent requests associated with a cache entry.
*
* @href the href of a cached object
*/
removeDependents(href: string) {
this.resolveSelfLink(href).pipe(
take(1),
).subscribe((selfLink: string) => {
if (hasValue(selfLink)) {
this.store.dispatch(new RemoveDependentsObjectCacheAction(selfLink));
}
});
}
/**
* Resolve the self link of an existing cached object from an arbitrary href
*
* @param href any href
* @return an observable of the self link corresponding to the given href.
* Will emit the given href if it was a self link, another href
* if the given href was an alt link, or undefined if there is no
* cached object for this href.
*/
private resolveSelfLink(href: string): Observable<string> {
return this.getBySelfLink(href).pipe(
switchMap((oce: ObjectCacheEntry) => {
if (isNotEmpty(oce)) {
return [href];
} else {
return this.store.pipe(
select(selfLinkFromAlternativeLinkSelector(href)),
);
}
}),
);
}
} }

View File

@@ -10,7 +10,7 @@ import { RemoteDataBuildService } from '../../cache/builders/remote-data-build.s
import { HALEndpointService } from '../../shared/hal-endpoint.service'; import { HALEndpointService } from '../../shared/hal-endpoint.service';
import { ObjectCacheService } from '../../cache/object-cache.service'; import { ObjectCacheService } from '../../cache/object-cache.service';
import { FindListOptions } from '../find-list-options.model'; import { FindListOptions } from '../find-list-options.model';
import { Observable, of as observableOf } from 'rxjs'; import { Observable, of as observableOf, combineLatest as observableCombineLatest } from 'rxjs';
import { getMockRequestService } from '../../../shared/mocks/request.service.mock'; import { getMockRequestService } from '../../../shared/mocks/request.service.mock';
import { HALEndpointServiceStub } from '../../../shared/testing/hal-endpoint-service.stub'; import { HALEndpointServiceStub } from '../../../shared/testing/hal-endpoint-service.stub';
import { getMockRemoteDataBuildService } from '../../../shared/mocks/remote-data-build.service.mock'; import { getMockRemoteDataBuildService } from '../../../shared/mocks/remote-data-build.service.mock';
@@ -20,6 +20,7 @@ import { RemoteData } from '../remote-data';
import { RequestEntryState } from '../request-entry-state.model'; import { RequestEntryState } from '../request-entry-state.model';
import { fakeAsync, tick } from '@angular/core/testing'; import { fakeAsync, tick } from '@angular/core/testing';
import { BaseDataService } from './base-data.service'; import { BaseDataService } from './base-data.service';
import { createFailedRemoteDataObject$, createSuccessfulRemoteDataObject$ } from '../../../shared/remote-data.utils';
const endpoint = 'https://rest.api/core'; const endpoint = 'https://rest.api/core';
@@ -65,7 +66,13 @@ describe('BaseDataService', () => {
}, },
getByHref: () => { getByHref: () => {
/* empty */ /* empty */
} },
addDependency: () => {
/* empty */
},
removeDependents: () => {
/* empty */
},
} as any; } as any;
selfLink = 'https://rest.api/endpoint/1698f1d3-be98-4c51-9fd8-6bfedcbd59b7'; selfLink = 'https://rest.api/endpoint/1698f1d3-be98-4c51-9fd8-6bfedcbd59b7';
linksToFollow = [ linksToFollow = [
@@ -558,7 +565,8 @@ describe('BaseDataService', () => {
beforeEach(() => { beforeEach(() => {
getByHrefSpy = spyOn(objectCache, 'getByHref').and.returnValue(observableOf({ getByHrefSpy = spyOn(objectCache, 'getByHref').and.returnValue(observableOf({
requestUUIDs: ['request1', 'request2', 'request3'] requestUUIDs: ['request1', 'request2', 'request3'],
dependentRequestUUIDs: ['request4', 'request5']
})); }));
}); });
@@ -570,6 +578,8 @@ describe('BaseDataService', () => {
expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request1'); expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request1');
expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request2'); expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request2');
expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request3'); expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request3');
expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request4');
expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request5');
done(); done();
}); });
}); });
@@ -582,6 +592,8 @@ describe('BaseDataService', () => {
expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request1'); expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request1');
expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request2'); expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request2');
expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request3'); expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request3');
expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request4');
expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request5');
})); }));
it('should return an Observable that only emits true once all requests are stale', () => { it('should return an Observable that only emits true once all requests are stale', () => {
@@ -591,9 +603,13 @@ describe('BaseDataService', () => {
case 'request1': case 'request1':
return cold('--(t|)', BOOLEAN); return cold('--(t|)', BOOLEAN);
case 'request2': case 'request2':
return cold('----(t|)', BOOLEAN);
case 'request3':
return cold('------(t|)', BOOLEAN); return cold('------(t|)', BOOLEAN);
case 'request3':
return cold('---(t|)', BOOLEAN);
case 'request4':
return cold('-(t|)', BOOLEAN);
case 'request5':
return cold('----(t|)', BOOLEAN);
} }
}); });
@@ -607,9 +623,9 @@ describe('BaseDataService', () => {
it('should only fire for the current state of the object (instead of tracking it)', () => { it('should only fire for the current state of the object (instead of tracking it)', () => {
testScheduler.run(({ cold, flush }) => { testScheduler.run(({ cold, flush }) => {
getByHrefSpy.and.returnValue(cold('a---b---c---', { getByHrefSpy.and.returnValue(cold('a---b---c---', {
a: { requestUUIDs: ['request1'] }, // this is the state at the moment we're invalidating the cache a: { requestUUIDs: ['request1'], dependentRequestUUIDs: [] }, // this is the state at the moment we're invalidating the cache
b: { requestUUIDs: ['request2'] }, // we shouldn't keep tracking the state b: { requestUUIDs: ['request2'], dependentRequestUUIDs: [] }, // we shouldn't keep tracking the state
c: { requestUUIDs: ['request3'] }, // because we may invalidate when we shouldn't c: { requestUUIDs: ['request3'], dependentRequestUUIDs: [] }, // because we may invalidate when we shouldn't
})); }));
service.invalidateByHref('some-href'); service.invalidateByHref('some-href');
@@ -624,4 +640,42 @@ describe('BaseDataService', () => {
}); });
}); });
}); });
describe('addDependency', () => {
let addDependencySpy;
beforeEach(() => {
addDependencySpy = spyOn(objectCache, 'addDependency');
});
it('should call objectCache.addDependency with the object\'s self link', () => {
addDependencySpy.and.callFake((href$: Observable<string>, dependsOn$: Observable<string>) => {
observableCombineLatest([href$, dependsOn$]).subscribe(([href, dependsOn]) => {
expect(href).toBe('object-href');
expect(dependsOn).toBe('dependsOnHref');
});
});
(service as any).addDependency(
createSuccessfulRemoteDataObject$({ _links: { self: { href: 'object-href' } } }),
observableOf('dependsOnHref')
);
expect(addDependencySpy).toHaveBeenCalled();
});
it('should call objectCache.addDependency without an href if request failed', () => {
addDependencySpy.and.callFake((href$: Observable<string>, dependsOn$: Observable<string>) => {
observableCombineLatest([href$, dependsOn$]).subscribe(([href, dependsOn]) => {
expect(href).toBe(undefined);
expect(dependsOn).toBe('dependsOnHref');
});
});
(service as any).addDependency(
createFailedRemoteDataObject$('something went wrong'),
observableOf('dependsOnHref')
);
expect(addDependencySpy).toHaveBeenCalled();
});
});
}); });

View File

@@ -23,6 +23,7 @@ import { PaginatedList } from '../paginated-list.model';
import { ObjectCacheEntry } from '../../cache/object-cache.reducer'; import { ObjectCacheEntry } from '../../cache/object-cache.reducer';
import { ObjectCacheService } from '../../cache/object-cache.service'; import { ObjectCacheService } from '../../cache/object-cache.service';
import { HALDataService } from './hal-data-service.interface'; import { HALDataService } from './hal-data-service.interface';
import { getFirstCompletedRemoteData } from '../../shared/operators';
/** /**
* Common functionality for data services. * Common functionality for data services.
@@ -352,19 +353,55 @@ export class BaseDataService<T extends CacheableObject> implements HALDataServic
} }
/** /**
* Invalidate a cached object by its href * Shorthand method to add a dependency to a cached object
* @param href the href to invalidate * ```
* const out$ = this.findByHref(...); // or another method that sends a request
* this.addDependency(out$, dependsOnHref);
* ```
* When {@link dependsOnHref$} is invalidated, {@link object$} will be invalidated as well.
*
*
* @param object$ the cached object
* @param dependsOnHref$ the href of the object it should depend on
*/ */
public invalidateByHref(href: string): Observable<boolean> { protected addDependency(object$: Observable<RemoteData<T | PaginatedList<T>>>, dependsOnHref$: string | Observable<string>) {
this.objectCache.addDependency(
object$.pipe(
getFirstCompletedRemoteData(),
switchMap((rd: RemoteData<T>) => {
if (rd.hasSucceeded) {
return [rd.payload._links.self.href];
} else {
// undefined href will be skipped in objectCache.addDependency
return [undefined];
}
}),
),
dependsOnHref$
);
}
/**
* Invalidate an existing DSpaceObject by marking all requests it is included in as stale
* @param href The self link of the object to be invalidated
* @return An Observable that will emit `true` once all requests are stale
*/
invalidateByHref(href: string): Observable<boolean> {
const done$ = new AsyncSubject<boolean>(); const done$ = new AsyncSubject<boolean>();
this.objectCache.getByHref(href).pipe( this.objectCache.getByHref(href).pipe(
take(1), take(1),
switchMap((oce: ObjectCacheEntry) => observableFrom(oce.requestUUIDs).pipe( switchMap((oce: ObjectCacheEntry) => {
return observableFrom([
...oce.requestUUIDs,
...oce.dependentRequestUUIDs
]).pipe(
mergeMap((requestUUID: string) => this.requestService.setStaleByUUID(requestUUID)), mergeMap((requestUUID: string) => this.requestService.setStaleByUUID(requestUUID)),
toArray(), toArray(),
)), );
}),
).subscribe(() => { ).subscribe(() => {
this.objectCache.removeDependents(href);
done$.next(true); done$.next(true);
done$.complete(); done$.complete();
}); });

View File

@@ -178,7 +178,12 @@ describe('PatchDataImpl', () => {
describe('patch', () => { describe('patch', () => {
const dso = { const dso = {
uuid: 'dso-uuid' uuid: 'dso-uuid',
_links: {
self: {
href: 'dso-href',
}
}
}; };
const operations = [ const operations = [
Object.assign({ Object.assign({
@@ -188,14 +193,23 @@ describe('PatchDataImpl', () => {
}) as Operation }) as Operation
]; ];
beforeEach((done) => { it('should send a PatchRequest', () => {
service.patch(dso, operations).subscribe(() => { service.patch(dso, operations);
done(); expect(requestService.send).toHaveBeenCalledWith(jasmine.any(PatchRequest));
});
}); });
it('should send a PatchRequest', () => { it('should invalidate the cached object if successfully patched', () => {
expect(requestService.send).toHaveBeenCalledWith(jasmine.any(PatchRequest)); spyOn(rdbService, 'buildFromRequestUUIDAndAwait');
spyOn(service, 'invalidateByHref');
service.patch(dso, operations);
expect(rdbService.buildFromRequestUUIDAndAwait).toHaveBeenCalled();
expect((rdbService.buildFromRequestUUIDAndAwait as jasmine.Spy).calls.argsFor(0)[0]).toBe(requestService.generateRequestId());
const callback = (rdbService.buildFromRequestUUIDAndAwait as jasmine.Spy).calls.argsFor(0)[1];
callback();
expect(service.invalidateByHref).toHaveBeenCalledWith('dso-href');
}); });
}); });

View File

@@ -101,7 +101,7 @@ export class PatchDataImpl<T extends CacheableObject> extends IdentifiableDataSe
this.requestService.send(request); this.requestService.send(request);
}); });
return this.rdbService.buildFromRequestUUID(requestId); return this.rdbService.buildFromRequestUUIDAndAwait(requestId, () => this.invalidateByHref(object._links.self.href));
} }
/** /**

View File

@@ -2,7 +2,7 @@ import { AuthorizationDataService } from './authorization-data.service';
import { SiteDataService } from '../site-data.service'; import { SiteDataService } from '../site-data.service';
import { Site } from '../../shared/site.model'; import { Site } from '../../shared/site.model';
import { EPerson } from '../../eperson/models/eperson.model'; import { EPerson } from '../../eperson/models/eperson.model';
import { of as observableOf } from 'rxjs'; import { of as observableOf, combineLatest as observableCombineLatest, Observable } from 'rxjs';
import { FeatureID } from './feature-id'; import { FeatureID } from './feature-id';
import { hasValue } from '../../../shared/empty.util'; import { hasValue } from '../../../shared/empty.util';
import { RequestParam } from '../../cache/models/request-param.model'; import { RequestParam } from '../../cache/models/request-param.model';
@@ -12,10 +12,12 @@ import { createPaginatedList } from '../../../shared/testing/utils.test';
import { Feature } from '../../shared/feature.model'; import { Feature } from '../../shared/feature.model';
import { FindListOptions } from '../find-list-options.model'; import { FindListOptions } from '../find-list-options.model';
import { testSearchDataImplementation } from '../base/search-data.spec'; import { testSearchDataImplementation } from '../base/search-data.spec';
import { getMockObjectCacheService } from '../../../shared/mocks/object-cache.service.mock';
describe('AuthorizationDataService', () => { describe('AuthorizationDataService', () => {
let service: AuthorizationDataService; let service: AuthorizationDataService;
let siteService: SiteDataService; let siteService: SiteDataService;
let objectCache;
let site: Site; let site: Site;
let ePerson: EPerson; let ePerson: EPerson;
@@ -38,7 +40,8 @@ describe('AuthorizationDataService', () => {
siteService = jasmine.createSpyObj('siteService', { siteService = jasmine.createSpyObj('siteService', {
find: observableOf(site), find: observableOf(site),
}); });
service = new AuthorizationDataService(requestService, undefined, undefined, undefined, siteService); objectCache = getMockObjectCacheService();
service = new AuthorizationDataService(requestService, undefined, objectCache, undefined, siteService);
} }
beforeEach(() => { beforeEach(() => {
@@ -110,6 +113,43 @@ describe('AuthorizationDataService', () => {
expect(service.searchBy).toHaveBeenCalledWith('object', createExpected(objectUrl, ePersonUuid, FeatureID.LoginOnBehalfOf), true, true); expect(service.searchBy).toHaveBeenCalledWith('object', createExpected(objectUrl, ePersonUuid, FeatureID.LoginOnBehalfOf), true, true);
}); });
}); });
describe('dependencies', () => {
let addDependencySpy;
beforeEach(() => {
(service.searchBy as any).and.returnValue(observableOf('searchBy RD$'));
addDependencySpy = spyOn(service as any, 'addDependency');
});
it('should add a dependency on the objectUrl', (done) => {
addDependencySpy.and.callFake((href$: Observable<string>, dependsOn$: Observable<string>) => {
observableCombineLatest([href$, dependsOn$]).subscribe(([href, dependsOn]) => {
expect(href).toBe('searchBy RD$');
expect(dependsOn).toBe('object-href');
});
});
service.searchByObject(FeatureID.AdministratorOf, 'object-href').subscribe(() => {
expect(addDependencySpy).toHaveBeenCalled();
done();
});
});
it('should add a dependency on the Site object if no objectUrl is given', (done) => {
addDependencySpy.and.callFake((object$: Observable<any>, dependsOn$: Observable<string>) => {
observableCombineLatest([object$, dependsOn$]).subscribe(([object, dependsOn]) => {
expect(object).toBe('searchBy RD$');
expect(dependsOn).toBe('test-site-href');
});
});
service.searchByObject(FeatureID.AdministratorOf).subscribe(() => {
expect(addDependencySpy).toHaveBeenCalled();
done();
});
});
});
}); });
describe('isAuthorized', () => { describe('isAuthorized', () => {

View File

@@ -11,10 +11,10 @@ import { followLink, FollowLinkConfig } from '../../../shared/utils/follow-link-
import { RemoteData } from '../remote-data'; import { RemoteData } from '../remote-data';
import { PaginatedList } from '../paginated-list.model'; import { PaginatedList } from '../paginated-list.model';
import { catchError, map, switchMap } from 'rxjs/operators'; import { catchError, map, switchMap } from 'rxjs/operators';
import { hasValue, isNotEmpty } from '../../../shared/empty.util'; import { hasNoValue, hasValue, isNotEmpty } from '../../../shared/empty.util';
import { RequestParam } from '../../cache/models/request-param.model'; import { RequestParam } from '../../cache/models/request-param.model';
import { AuthorizationSearchParams } from './authorization-search-params'; import { AuthorizationSearchParams } from './authorization-search-params';
import { addSiteObjectUrlIfEmpty, oneAuthorizationMatchesFeature } from './authorization-utils'; import { oneAuthorizationMatchesFeature } from './authorization-utils';
import { FeatureID } from './feature-id'; import { FeatureID } from './feature-id';
import { getFirstCompletedRemoteData } from '../../shared/operators'; import { getFirstCompletedRemoteData } from '../../shared/operators';
import { FindListOptions } from '../find-list-options.model'; import { FindListOptions } from '../find-list-options.model';
@@ -96,12 +96,28 @@ export class AuthorizationDataService extends BaseDataService<Authorization> imp
* {@link HALLink}s should be automatically resolved * {@link HALLink}s should be automatically resolved
*/ */
searchByObject(featureId?: FeatureID, objectUrl?: string, ePersonUuid?: string, options: FindListOptions = {}, useCachedVersionIfAvailable = true, reRequestOnStale = true, ...linksToFollow: FollowLinkConfig<Authorization>[]): Observable<RemoteData<PaginatedList<Authorization>>> { searchByObject(featureId?: FeatureID, objectUrl?: string, ePersonUuid?: string, options: FindListOptions = {}, useCachedVersionIfAvailable = true, reRequestOnStale = true, ...linksToFollow: FollowLinkConfig<Authorization>[]): Observable<RemoteData<PaginatedList<Authorization>>> {
return observableOf(new AuthorizationSearchParams(objectUrl, ePersonUuid, featureId)).pipe( const objectUrl$ = observableOf(objectUrl).pipe(
addSiteObjectUrlIfEmpty(this.siteService), switchMap((url) => {
if (hasNoValue(url)) {
return this.siteService.find().pipe(
map((site) => site.self)
);
} else {
return observableOf(url);
}
}),
);
const out$ = objectUrl$.pipe(
map((url: string) => new AuthorizationSearchParams(url, ePersonUuid, featureId)),
switchMap((params: AuthorizationSearchParams) => { switchMap((params: AuthorizationSearchParams) => {
return this.searchBy(this.searchByObjectPath, this.createSearchOptions(params.objectUrl, options, params.ePersonUuid, params.featureId), useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow); return this.searchBy(this.searchByObjectPath, this.createSearchOptions(params.objectUrl, options, params.ePersonUuid, params.featureId), useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow);
}) })
); );
this.addDependency(out$, objectUrl$);
return out$;
} }
/** /**

View File

@@ -26,10 +26,9 @@ import { EPersonMock, EPersonMock2 } from '../../shared/testing/eperson.mock';
import { createPaginatedList, createRequestEntry$ } from '../../shared/testing/utils.test'; import { createPaginatedList, createRequestEntry$ } from '../../shared/testing/utils.test';
import { CoreState } from '../core-state.model'; import { CoreState } from '../core-state.model';
import { FindListOptions } from '../data/find-list-options.model'; import { FindListOptions } from '../data/find-list-options.model';
import { ObjectCacheService } from '../cache/object-cache.service';
import { getMockLinkService } from '../../shared/mocks/link-service.mock';
import { of as observableOf } from 'rxjs'; import { of as observableOf } from 'rxjs';
import { ObjectCacheEntry } from '../cache/object-cache.reducer'; import { ObjectCacheEntry } from '../cache/object-cache.reducer';
import { getMockObjectCacheService } from '../../shared/mocks/object-cache.service.mock';
describe('GroupDataService', () => { describe('GroupDataService', () => {
let service: GroupDataService; let service: GroupDataService;
@@ -42,7 +41,7 @@ describe('GroupDataService', () => {
let groups$; let groups$;
let halService; let halService;
let rdbService; let rdbService;
let objectCache: ObjectCacheService; let objectCache;
function init() { function init() {
restEndpointURL = 'https://dspace.4science.it/dspace-spring-rest/api/eperson'; restEndpointURL = 'https://dspace.4science.it/dspace-spring-rest/api/eperson';
groupsEndpoint = `${restEndpointURL}/groups`; groupsEndpoint = `${restEndpointURL}/groups`;
@@ -50,7 +49,7 @@ describe('GroupDataService', () => {
groups$ = createSuccessfulRemoteDataObject$(createPaginatedList(groups)); groups$ = createSuccessfulRemoteDataObject$(createPaginatedList(groups));
rdbService = getMockRemoteDataBuildServiceHrefMap(undefined, { 'https://dspace.4science.it/dspace-spring-rest/api/eperson/groups': groups$ }); rdbService = getMockRemoteDataBuildServiceHrefMap(undefined, { 'https://dspace.4science.it/dspace-spring-rest/api/eperson/groups': groups$ });
halService = new HALEndpointServiceStub(restEndpointURL); halService = new HALEndpointServiceStub(restEndpointURL);
objectCache = new ObjectCacheService(store, getMockLinkService()); objectCache = getMockObjectCacheService();
TestBed.configureTestingModule({ TestBed.configureTestingModule({
imports: [ imports: [
CommonModule, CommonModule,
@@ -114,8 +113,9 @@ describe('GroupDataService', () => {
describe('addSubGroupToGroup', () => { describe('addSubGroupToGroup', () => {
beforeEach(() => { beforeEach(() => {
spyOn(objectCache, 'getByHref').and.returnValue(observableOf({ objectCache.getByHref.and.returnValue(observableOf({
requestUUIDs: ['request1', 'request2'] requestUUIDs: ['request1', 'request2'],
dependentRequestUUIDs: [],
} as ObjectCacheEntry)); } as ObjectCacheEntry));
spyOn((service as any).deleteData, 'invalidateByHref'); spyOn((service as any).deleteData, 'invalidateByHref');
service.addSubGroupToGroup(GroupMock, GroupMock2).subscribe(); service.addSubGroupToGroup(GroupMock, GroupMock2).subscribe();
@@ -143,8 +143,9 @@ describe('GroupDataService', () => {
describe('deleteSubGroupFromGroup', () => { describe('deleteSubGroupFromGroup', () => {
beforeEach(() => { beforeEach(() => {
spyOn(objectCache, 'getByHref').and.returnValue(observableOf({ objectCache.getByHref.and.returnValue(observableOf({
requestUUIDs: ['request1', 'request2'] requestUUIDs: ['request1', 'request2'],
dependentRequestUUIDs: [],
} as ObjectCacheEntry)); } as ObjectCacheEntry));
spyOn((service as any).deleteData, 'invalidateByHref'); spyOn((service as any).deleteData, 'invalidateByHref');
service.deleteSubGroupFromGroup(GroupMock, GroupMock2).subscribe(); service.deleteSubGroupFromGroup(GroupMock, GroupMock2).subscribe();
@@ -168,8 +169,9 @@ describe('GroupDataService', () => {
describe('addMemberToGroup', () => { describe('addMemberToGroup', () => {
beforeEach(() => { beforeEach(() => {
spyOn(objectCache, 'getByHref').and.returnValue(observableOf({ objectCache.getByHref.and.returnValue(observableOf({
requestUUIDs: ['request1', 'request2'] requestUUIDs: ['request1', 'request2'],
dependentRequestUUIDs: [],
} as ObjectCacheEntry)); } as ObjectCacheEntry));
spyOn((service as any).deleteData, 'invalidateByHref'); spyOn((service as any).deleteData, 'invalidateByHref');
service.addMemberToGroup(GroupMock, EPersonMock2).subscribe(); service.addMemberToGroup(GroupMock, EPersonMock2).subscribe();
@@ -198,8 +200,9 @@ describe('GroupDataService', () => {
describe('deleteMemberFromGroup', () => { describe('deleteMemberFromGroup', () => {
beforeEach(() => { beforeEach(() => {
spyOn(objectCache, 'getByHref').and.returnValue(observableOf({ objectCache.getByHref.and.returnValue(observableOf({
requestUUIDs: ['request1', 'request2'] requestUUIDs: ['request1', 'request2'],
dependentRequestUUIDs: [],
} as ObjectCacheEntry)); } as ObjectCacheEntry));
spyOn((service as any).deleteData, 'invalidateByHref'); spyOn((service as any).deleteData, 'invalidateByHref');
service.deleteMemberFromGroup(GroupMock, EPersonMock).subscribe(); service.deleteMemberFromGroup(GroupMock, EPersonMock).subscribe();

View File

@@ -13,6 +13,8 @@ export function getMockObjectCacheService(): ObjectCacheService {
'hasByUUID', 'hasByUUID',
'hasByHref', 'hasByHref',
'getRequestUUIDBySelfLink', 'getRequestUUIDBySelfLink',
'addDependency',
'removeDependents',
]); ]);
} }