Skip to content

Commit

Permalink
Merge pull request DSpace#2349 from alexandrevryghem/fix-setStaleByHr…
Browse files Browse the repository at this point in the history
…efSubtring-not-emitting-after-all-requests-were-stale_contribute-main

Fix `setStaleByHrefSubstring` not emitting true when all requests are stale
  • Loading branch information
tdonohue authored Sep 7, 2023
2 parents 1824507 + 273be5b commit 499463f
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 9 deletions.
46 changes: 45 additions & 1 deletion src/app/core/data/request.service.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Store, StoreModule } from '@ngrx/store';
import { cold, getTestScheduler } from 'jasmine-marbles';
import { EMPTY, of as observableOf } from 'rxjs';
import { EMPTY, Observable, of as observableOf } from 'rxjs';
import { TestScheduler } from 'rxjs/testing';

import { getMockObjectCacheService } from '../../shared/mocks/object-cache.service.mock';
Expand Down Expand Up @@ -638,4 +638,48 @@ describe('RequestService', () => {
expect(done$).toBeObservable(cold('-----(t|)', { t: true }));
}));
});

describe('setStaleByHrefSubstring', () => {
let dispatchSpy: jasmine.Spy;
let getByUUIDSpy: jasmine.Spy;

beforeEach(() => {
dispatchSpy = spyOn(store, 'dispatch');
getByUUIDSpy = spyOn(service, 'getByUUID').and.callThrough();
});

describe('with an empty/no matching requests in the state', () => {
it('should return true', () => {
const done$: Observable<boolean> = service.setStaleByHrefSubstring('https://rest.api/endpoint/selfLink');
expect(done$).toBeObservable(cold('(a|)', { a: true }));
});
});

describe('with a matching request in the state', () => {
beforeEach(() => {
const state = Object.assign({}, initialState, {
core: Object.assign({}, initialState.core, {
'index': {
'get-request/href-to-uuid': {
'https://rest.api/endpoint/selfLink': '5f2a0d2a-effa-4d54-bd54-5663b960f9eb'
}
}
})
});
mockStore.setState(state);
});

it('should return an Observable that emits true as soon as the request is stale', () => {
dispatchSpy.and.callFake(() => { /* empty */ }); // don't actually set as stale
getByUUIDSpy.and.returnValue(cold('a-b--c--d-', { // but fake the state in the cache
a: { state: RequestEntryState.ResponsePending },
b: { state: RequestEntryState.Success },
c: { state: RequestEntryState.SuccessStale },
d: { state: RequestEntryState.Error },
}));
const done$: Observable<boolean> = service.setStaleByHrefSubstring('https://rest.api/endpoint/selfLink');
expect(done$).toBeObservable(cold('-----(a|)', { a: true }));
});
});
});
});
36 changes: 28 additions & 8 deletions src/app/core/data/request.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ import { Injectable } from '@angular/core';
import { HttpHeaders } from '@angular/common/http';

import { createSelector, MemoizedSelector, select, Store } from '@ngrx/store';
import { Observable } from 'rxjs';
import { filter, map, take, tap } from 'rxjs/operators';
import { Observable, from as observableFrom } from 'rxjs';
import { filter, find, map, mergeMap, switchMap, take, tap, toArray } from 'rxjs/operators';
import cloneDeep from 'lodash/cloneDeep';
import { hasValue, isEmpty, isNotEmpty, hasNoValue } from '../../shared/empty.util';
import { ObjectCacheEntry } from '../cache/object-cache.reducer';
Expand Down Expand Up @@ -300,22 +300,42 @@ export class RequestService {
* Set all requests that match (part of) the href to stale
*
* @param href A substring of the request(s) href
* @return Returns an observable emitting whether or not the cache is removed
* @return Returns an observable emitting when those requests are all stale
*/
setStaleByHrefSubstring(href: string): Observable<boolean> {
this.store.pipe(
const requestUUIDs$ = this.store.pipe(
select(uuidsFromHrefSubstringSelector(requestIndexSelector, href)),
take(1)
).subscribe((uuids: string[]) => {
);
requestUUIDs$.subscribe((uuids: string[]) => {
for (const uuid of uuids) {
this.store.dispatch(new RequestStaleAction(uuid));
}
});
this.requestsOnTheirWayToTheStore = this.requestsOnTheirWayToTheStore.filter((reqHref: string) => reqHref.indexOf(href) < 0);

return this.store.pipe(
select(uuidsFromHrefSubstringSelector(requestIndexSelector, href)),
map((uuids) => isEmpty(uuids))
// emit true after all requests are stale
return requestUUIDs$.pipe(
switchMap((uuids: string[]) => {
if (isEmpty(uuids)) {
// if there were no matching requests, emit true immediately
return [true];
} else {
// otherwise emit all request uuids in order
return observableFrom(uuids).pipe(
// retrieve the RequestEntry for each uuid
mergeMap((uuid: string) => this.getByUUID(uuid)),
// check whether it is undefined or stale
map((request: RequestEntry) => hasNoValue(request) || isStale(request.state)),
// if it is, complete
find((stale: boolean) => stale === true),
// after all observables above are completed, emit them as a single array
toArray(),
// when the array comes in, emit true
map(() => true)
);
}
})
);
}

Expand Down

0 comments on commit 499463f

Please sign in to comment.