diff --git a/src/app/core/data/request.models.ts b/src/app/core/data/request.models.ts index fbf394cc01..ee37f9c3d4 100644 --- a/src/app/core/data/request.models.ts +++ b/src/app/core/data/request.models.ts @@ -113,7 +113,7 @@ export class PatchRequest extends RestRequest { } } -export class FindByIDRequest extends RestRequest { +export class FindByIDRequest extends GetRequest { constructor( uuid: string, href: string, @@ -130,7 +130,7 @@ export class FindAllOptions { sort?: SortOptions; } -export class FindAllRequest extends RestRequest { +export class FindAllRequest extends GetRequest { constructor( uuid: string, href: string, @@ -140,7 +140,7 @@ export class FindAllRequest extends RestRequest { } } -export class RootEndpointRequest extends RestRequest { +export class RootEndpointRequest extends GetRequest { constructor(uuid: string, EnvConfig: GlobalConfig) { const href = new RESTURLCombiner(EnvConfig, '/').toString(); super(uuid, href); @@ -151,7 +151,7 @@ export class RootEndpointRequest extends RestRequest { } } -export class BrowseEndpointRequest extends RestRequest { +export class BrowseEndpointRequest extends GetRequest { constructor(uuid: string, href: string) { super(uuid, href); } @@ -161,7 +161,7 @@ export class BrowseEndpointRequest extends RestRequest { } } -export class ConfigRequest extends RestRequest { +export class ConfigRequest extends GetRequest { constructor(uuid: string, href: string) { super(uuid, href); } diff --git a/src/app/core/data/request.service.spec.ts b/src/app/core/data/request.service.spec.ts new file mode 100644 index 0000000000..afc4209d67 --- /dev/null +++ b/src/app/core/data/request.service.spec.ts @@ -0,0 +1,271 @@ +import { Store } from '@ngrx/store'; +import { cold, hot } from 'jasmine-marbles'; +import { Observable } from 'rxjs/Observable'; +import { initMockObjectCacheService } from '../../shared/mocks/mock-object-cache.service'; +import { initMockStore } from '../../shared/mocks/mock-store'; +import { defaultUUID, initMockUUIDService } from '../../shared/mocks/mock-uuid.service'; +import { ObjectCacheService } from '../cache/object-cache.service'; +import { ResponseCacheService } from '../cache/response-cache.service'; +import { CoreState } from '../core.reducers'; +import { UUIDService } from '../shared/uuid.service'; +import { + DeleteRequest, GetRequest, + HeadRequest, + OptionsRequest, + PatchRequest, + PostRequest, + PutRequest, RestRequest +} from './request.models'; +import { RequestService } from './request.service'; + +describe('RequestService', () => { + let service: RequestService; + let objectCache: ObjectCacheService; + let responseCache: ResponseCacheService; + let uuidService: UUIDService; + let store: Store; + + const testUUID = '5f2a0d2a-effa-4d54-bd54-5663b960f9eb'; + const testHref = 'https://rest.api/endpoint/selfLink'; + + function initMockResponseCacheService() { + return jasmine.createSpyObj('responseCache', { + has: true, + get: cold('b-', { + b: { + response: {} + } + }) + }); + } + + function initTestService() { + return new RequestService( + objectCache, + responseCache, + uuidService, + store + ); + } + + function initServices(selectResult: any) { + objectCache = initMockObjectCacheService(); + responseCache = initMockResponseCacheService(); + uuidService = initMockUUIDService(); + store = initMockStore(selectResult); + service = initTestService(); + } + + describe('generateRequestId', () => { + beforeEach(() => { + initServices(Observable.of(undefined)); + }); + + it('should generate a new request ID', () => { + const result = service.generateRequestId(); + const expected = `client/${defaultUUID}`; + + expect(result).toBe(expected); + }); + }); + + describe('isPending', () => { + let testRequest: GetRequest; + describe('before the request is configured', () => { + beforeEach(() => { + initServices(Observable.of(undefined)); + testRequest = new GetRequest(testUUID, testHref) + }); + + it('should return false', () => { + const result = service.isPending(testRequest); + const expected = false; + + expect(result).toBe(expected); + }); + }); + + describe('when the request has been configured but hasn\'t reached the store yet', () => { + beforeEach(() => { + initServices(Observable.of(undefined)); + }); + + it('should return true', () => { + (service as any).requestsOnTheirWayToTheStore = [testHref]; + const result = service.isPending(testRequest); + const expected = true; + + expect(result).toBe(expected); + }); + }); + + describe('when the request has reached the store, before the server responds', () => { + beforeEach(() => { + initServices(Observable.of({ + completed: false + })); + }); + + it('should return true', () => { + const result = service.isPending(testRequest); + const expected = true; + + expect(result).toBe(expected); + }); + }); + + describe('after the server responds', () => { + beforeEach(() => { + initServices(Observable.of({ + completed: true + })); + }); + + it('should return false', () => { + const result = service.isPending(testRequest); + const expected = false; + + expect(result).toBe(expected); + }); + }); + + describe('getByUUID', () => { + describe('if the request with the specified UUID exists in the store', () => { + it('should return an Observable of the RequestEntry', () => { + initServices(hot('a', { + a: { + completed: true + } + })); + + const result = service.getByUUID(testUUID); + const expected = cold('b', { + b: { + completed: true + } + }); + + expect(result).toBeObservable(expected); + }); + }); + + describe('if the request with the specified UUID doesn\'t exist in the store', () => { + it('should return an Observable of undefined', () => { + initServices(hot('a', { + a: undefined + })); + + const result = service.getByUUID(testUUID); + const expected = cold('b', { + b: undefined + }); + + expect(result).toBeObservable(expected); + }); + }); + + }); + + describe('getByHref', () => { + describe('if the request with the specified href exists in the store', () => { + it('should return an Observable of the RequestEntry', () => { + initServices(hot('a', { + a: testUUID + })); + spyOn(service, 'getByUUID').and.returnValue(cold('b', { + b: { + completed: true + } + })); + + const result = service.getByHref(testHref); + const expected = cold('c', { + c: { + completed: true + } + }); + + expect(result).toBeObservable(expected); + }); + }); + + describe('if the request with the specified href doesn\'t exist in the store', () => { + it('should return an Observable of undefined', () => { + initServices(hot('a', { + a: undefined + })); + spyOn(service, 'getByUUID').and.returnValue(cold('b', { + b: undefined + })); + + const result = service.getByHref(testHref); + const expected = cold('c', { + c: undefined + }); + + expect(result).toBeObservable(expected); + }); + }); + }); + + describe('configure', () => { + describe('if the request is a GET request', () => { + describe('and it isn\'t already cached', () => { + it('should dispatch the request', () => { + initServices(Observable.of(undefined)); + spyOn((service as any), 'dispatchRequest'); + spyOn((service as any), 'isCachedOrPending').and.returnValue(false); + + const request = new GetRequest(testUUID, testHref); + service.configure(request); + expect((service as any).dispatchRequest).toHaveBeenCalledWith(request); + }); + }); + describe('and it is already cached or pending', () => { + it('shouldn\'t dispatch the request', () => { + initServices(Observable.of(undefined)); + spyOn((service as any), 'dispatchRequest'); + spyOn((service as any), 'isCachedOrPending').and.returnValue(true); + + const request = new GetRequest(testUUID, testHref); + service.configure(request); + expect((service as any).dispatchRequest).not.toHaveBeenCalled(); + }); + }); + }); + + describe('if the request isn\'t a GET request', () => { + it('should dispatch the request', () => { + initServices(Observable.of(undefined)); + spyOn((service as any), 'dispatchRequest'); + + let request = new PostRequest(testUUID, testHref); + service.configure(request); + expect((service as any).dispatchRequest).toHaveBeenCalledWith(request); + + request = new PutRequest(testUUID, testHref); + service.configure(request); + expect((service as any).dispatchRequest).toHaveBeenCalledWith(request); + + request = new DeleteRequest(testUUID, testHref); + service.configure(request); + expect((service as any).dispatchRequest).toHaveBeenCalledWith(request); + + request = new OptionsRequest(testUUID, testHref); + service.configure(request); + expect((service as any).dispatchRequest).toHaveBeenCalledWith(request); + + request = new HeadRequest(testUUID, testHref); + service.configure(request); + expect((service as any).dispatchRequest).toHaveBeenCalledWith(request); + + request = new PatchRequest(testUUID, testHref); + service.configure(request); + expect((service as any).dispatchRequest).toHaveBeenCalledWith(request); + }); + }); + }); + + }); + +}); diff --git a/src/app/core/data/request.service.ts b/src/app/core/data/request.service.ts index 6cfbedb5cb..f589221e63 100644 --- a/src/app/core/data/request.service.ts +++ b/src/app/core/data/request.service.ts @@ -14,24 +14,10 @@ import { IndexName } from '../index/index.reducer'; import { pathSelector } from '../shared/selectors'; import { UUIDService } from '../shared/uuid.service'; import { RequestConfigureAction, RequestExecuteAction } from './request.actions'; -import { RestRequest, RestRequestMethod } from './request.models'; +import { GetRequest, RestRequest, RestRequestMethod } from './request.models'; import { RequestEntry, RequestState } from './request.reducer'; -function entryFromUUIDSelector(uuid: string): MemoizedSelector { - return pathSelector(coreSelector, 'data/request', uuid); -} - -function uuidFromHrefSelector(href: string): MemoizedSelector { - return pathSelector(coreSelector, 'index', IndexName.REQUEST, href); -} - -export function requestStateSelector(): MemoizedSelector { - return createSelector(coreSelector, (state: CoreState) => { - return state['data/request'] as RequestState; - }); -} - @Injectable() export class RequestService { private requestsOnTheirWayToTheStore: string[] = []; @@ -42,19 +28,27 @@ export class RequestService { private store: Store) { } + private entryFromUUIDSelector(uuid: string): MemoizedSelector { + return pathSelector(coreSelector, 'data/request', uuid); + } + + private uuidFromHrefSelector(href: string): MemoizedSelector { + return pathSelector(coreSelector, 'index', IndexName.REQUEST, href); + } + generateRequestId(): string { return `client/${this.uuidService.generate()}`; } - isPending(uuid: string): boolean { + isPending(request: GetRequest): boolean { // first check requests that haven't made it to the store yet - if (this.requestsOnTheirWayToTheStore.includes(uuid)) { + if (this.requestsOnTheirWayToTheStore.includes(request.href)) { return true; } // then check the store let isPending = false; - this.store.select(entryFromUUIDSelector(uuid)) + this.getByHref(request.href) .take(1) .subscribe((re: RequestEntry) => { isPending = (hasValue(re) && !re.completed) @@ -64,11 +58,11 @@ export class RequestService { } getByUUID(uuid: string): Observable { - return this.store.select(entryFromUUIDSelector(uuid)); + return this.store.select(this.entryFromUUIDSelector(uuid)); } getByHref(href: string): Observable { - return this.store.select(uuidFromHrefSelector(href)) + return this.store.select(this.uuidFromHrefSelector(href)) .flatMap((uuid: string) => this.getByUUID(uuid)); } @@ -78,7 +72,7 @@ export class RequestService { } } - private isCachedOrPending(request: RestRequest) { + private isCachedOrPending(request: GetRequest) { let isCached = this.objectCache.hasBySelfLink(request.href); if (!isCached && this.responseCache.has(request.href)) { const [successResponse, errorResponse] = this.responseCache.get(request.href) @@ -102,7 +96,7 @@ export class RequestService { ).subscribe((c) => isCached = c); } - const isPending = this.isPending(request.uuid); + const isPending = this.isPending(request); return isCached || isPending; } @@ -110,23 +104,25 @@ export class RequestService { private dispatchRequest(request: RestRequest) { this.store.dispatch(new RequestConfigureAction(request)); this.store.dispatch(new RequestExecuteAction(request.uuid)); - this.trackRequestsOnTheirWayToTheStore(request.uuid); + if (request.method === RestRequestMethod.Get) { + this.trackRequestsOnTheirWayToTheStore(request); + } } /** * ngrx action dispatches are asynchronous. But this.isPending needs to return true as soon as the - * configure method for a request has been executed, otherwise certain requests will happen multiple times. + * configure method for a GET request has been executed, otherwise certain requests will happen multiple times. * - * This method will store the href of every request that gets configured in a local variable, and + * This method will store the href of every GET request that gets configured in a local variable, and * remove it as soon as it can be found in the store. */ - private trackRequestsOnTheirWayToTheStore(uuid: string) { - this.requestsOnTheirWayToTheStore = [...this.requestsOnTheirWayToTheStore, uuid]; - this.store.select(entryFromUUIDSelector(uuid)) + private trackRequestsOnTheirWayToTheStore(request: GetRequest) { + this.requestsOnTheirWayToTheStore = [...this.requestsOnTheirWayToTheStore, request.href]; + this.store.select(this.entryFromUUIDSelector(request.href)) .filter((re: RequestEntry) => hasValue(re)) .take(1) .subscribe((re: RequestEntry) => { - this.requestsOnTheirWayToTheStore = this.requestsOnTheirWayToTheStore.filter((pendingUUID: string) => pendingUUID !== uuid) + this.requestsOnTheirWayToTheStore = this.requestsOnTheirWayToTheStore.filter((pendingHref: string) => pendingHref !== request.href) }); } } diff --git a/src/app/shared/mocks/mock-object-cache.service.ts b/src/app/shared/mocks/mock-object-cache.service.ts new file mode 100644 index 0000000000..80f7b102fa --- /dev/null +++ b/src/app/shared/mocks/mock-object-cache.service.ts @@ -0,0 +1,7 @@ +import { ObjectCacheService } from '../../core/cache/object-cache.service'; + +export function initMockObjectCacheService(): ObjectCacheService { + return jasmine.createSpyObj('objectCacheService', { + hasBySelfLink: true, + }); +} diff --git a/src/app/shared/mocks/mock-store.ts b/src/app/shared/mocks/mock-store.ts index c619b5aa77..54b521458d 100644 --- a/src/app/shared/mocks/mock-store.ts +++ b/src/app/shared/mocks/mock-store.ts @@ -1,23 +1,9 @@ -import { Action } from '@ngrx/store'; +import { Store } from '@ngrx/store'; import { Observable } from 'rxjs/Observable'; -import { BehaviorSubject } from 'rxjs/BehaviorSubject'; - -export class MockStore extends BehaviorSubject { - - constructor(private _initialState: T) { - super(_initialState); - } - - dispatch = (action: Action): void => { - console.info(); - } - - select = (pathOrMapFn: any): Observable => { - return Observable.of(this.getValue()); - } - - nextState(_newState: T) { - this.next(_newState); - } +export function initMockStore(selectResult: Observable): Store { + return jasmine.createSpyObj('store', { + dispatch: null, + select: selectResult, + }); } diff --git a/src/app/shared/mocks/mock-uuid.service.ts b/src/app/shared/mocks/mock-uuid.service.ts new file mode 100644 index 0000000000..b82e007ed2 --- /dev/null +++ b/src/app/shared/mocks/mock-uuid.service.ts @@ -0,0 +1,9 @@ +import { UUIDService } from '../../core/shared/uuid.service'; + +export const defaultUUID = 'c4ce6905-290b-478f-979d-a333bbd7820f'; + +export function initMockUUIDService(uuid = defaultUUID): UUIDService { + return jasmine.createSpyObj('uuidService', { + generate: uuid, + }); +}