fixed an issue where certain GET requests would be performed more often then needed and added tests for the requestservice (not everything tested yet)

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);
   }

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<CoreState>;
+
+  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<CoreState>(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);
+        });
+      });
+    });
+
+  });
+
+});

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<CoreState, RequestEntry> {
-  return pathSelector<CoreState, RequestEntry>(coreSelector, 'data/request', uuid);
-}
-
-function uuidFromHrefSelector(href: string): MemoizedSelector<CoreState, string> {
-  return pathSelector<CoreState, string>(coreSelector, 'index', IndexName.REQUEST, href);
-}
-
-export function requestStateSelector(): MemoizedSelector<CoreState, RequestState> {
-  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<CoreState>) {
   }
 
+  private entryFromUUIDSelector(uuid: string): MemoizedSelector<CoreState, RequestEntry> {
+    return pathSelector<CoreState, RequestEntry>(coreSelector, 'data/request', uuid);
+  }
+
+  private uuidFromHrefSelector(href: string): MemoizedSelector<CoreState, string> {
+    return pathSelector<CoreState, string>(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<RequestEntry> {
-    return this.store.select(entryFromUUIDSelector(uuid));
+    return this.store.select(this.entryFromUUIDSelector(uuid));
   }
 
   getByHref(href: string): Observable<RequestEntry> {
-    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) {
+  private trackRequestsOnTheirWayToTheStore(request: GetRequest) {
-    this.requestsOnTheirWayToTheStore = [...this.requestsOnTheirWayToTheStore, uuid];
+    this.requestsOnTheirWayToTheStore = [...this.requestsOnTheirWayToTheStore, request.href];
-    this.store.select(entryFromUUIDSelector(uuid))
+    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)
       });
   }
 }

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,
+  });
+}

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<T> extends BehaviorSubject<T> {
-
-  constructor(private _initialState: T) {
-    super(_initialState);
-  }
-
-  dispatch = (action: Action): void => {
-    console.info();
-  }
-
-  select = <R>(pathOrMapFn: any): Observable<T> => {
-    return Observable.of(this.getValue());
-  }
-
-  nextState(_newState: T) {
-    this.next(_newState);
-  }
 
+export function initMockStore<T>(selectResult: Observable<T>): Store<T> {
+  return jasmine.createSpyObj('store', {
+    dispatch: null,
+    select: selectResult,
+  });
 }

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,
+  });
+}