more tests for request service

src/app/core/cache/builders/remote-data-build.service.ts

@@ -88,18 +88,18 @@ export class RemoteDataBuildService {
       (href: string, reqEntry: RequestEntry, resEntry: ResponseCacheEntry, payload: T) => {
         const requestPending = hasValue(reqEntry.requestPending) ? reqEntry.requestPending : true;
         const responsePending = hasValue(reqEntry.responsePending) ? reqEntry.responsePending : false;
-        let isSuccessFul: boolean;
+        let isSuccessful: boolean;
         let error: RemoteDataError;
         if (hasValue(resEntry) && hasValue(resEntry.response)) {
-          isSuccessFul = resEntry.response.isSuccessful;
+          isSuccessful = resEntry.response.isSuccessful;
-          const errorMessage = isSuccessFul === false ? (resEntry.response as ErrorResponse).errorMessage : undefined;
+          const errorMessage = isSuccessful === false ? (resEntry.response as ErrorResponse).errorMessage : undefined;
           error = new RemoteDataError(resEntry.response.statusCode, errorMessage);
         }
 
         return new RemoteData(
           requestPending,
           responsePending,
-          isSuccessFul,
+          isSuccessful,
           error,
           payload
         );
@@ -212,7 +212,7 @@ export class RemoteDataBuildService {
           .map((d: RemoteData<T>) => d.isResponsePending)
           .every((b: boolean) => b === true);
 
-        const isSuccessFul: boolean = arr
+        const isSuccessful: boolean = arr
           .map((d: RemoteData<T>) => d.hasSucceeded)
           .every((b: boolean) => b === true);
 
@@ -241,7 +241,7 @@ export class RemoteDataBuildService {
         return new RemoteData(
           requestPending,
           responsePending,
-          isSuccessFul,
+          isSuccessful,
           error,
           payload
         );

src/app/core/data/remote-data.ts

@@ -15,16 +15,16 @@ export class RemoteData<T> {
   constructor(
     private requestPending: boolean,
     private responsePending: boolean,
-    private isSuccessFul: boolean,
+    private isSuccessful: boolean,
     public error: RemoteDataError,
     public payload: T
   ) {
   }
 
   get state(): RemoteDataState {
-    if (this.isSuccessFul === true && hasValue(this.payload)) {
+    if (this.isSuccessful === true && hasValue(this.payload)) {
       return RemoteDataState.Success
-    } else if (this.isSuccessFul === false) {
+    } else if (this.isSuccessful === false) {
       return RemoteDataState.Failed
     } else if (this.requestPending === true) {
       return RemoteDataState.RequestPending

src/app/core/data/request.service.spec.ts

@@ -8,18 +8,21 @@ 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 { RequestConfigureAction, RequestExecuteAction } from './request.actions';
 import {
-  DeleteRequest, GetRequest,
+  DeleteRequest,
+  GetRequest,
   HeadRequest,
   OptionsRequest,
   PatchRequest,
   PostRequest,
-  PutRequest, RestRequest
+  PutRequest
 } from './request.models';
 import { RequestService } from './request.service';
 
 describe('RequestService', () => {
   let service: RequestService;
+  let serviceAsAny: any;
   let objectCache: ObjectCacheService;
   let responseCache: ResponseCacheService;
   let uuidService: UUIDService;
@@ -27,15 +30,23 @@ describe('RequestService', () => {
 
   const testUUID = '5f2a0d2a-effa-4d54-bd54-5663b960f9eb';
   const testHref = 'https://rest.api/endpoint/selfLink';
+  const testGetRequest = new GetRequest(testUUID, testHref);
+  const testPostRequest = new PostRequest(testUUID, testHref);
+  const testPutRequest = new PutRequest(testUUID, testHref);
+  const testDeleteRequest = new DeleteRequest(testUUID, testHref);
+  const testOptionsRequest = new OptionsRequest(testUUID, testHref);
+  const testHeadRequest = new HeadRequest(testUUID, testHref);
+  const testPatchRequest = new PatchRequest(testUUID, testHref);
 
-  function initMockResponseCacheService() {
+  const defaultSelectResult = Observable.of(undefined);
+  const defaultHasResponse = false;
+  const defaultGetResponse = Observable.of(undefined);
+  const defaultHasObjectCache = false;
+
+  function initMockResponseCacheService(hasResponse, getResponse) {
     return jasmine.createSpyObj('responseCache', {
-      has: true,
+      has: hasResponse,
-      get: cold('b-', {
+      get: getResponse
-        b: {
-          response: {}
-        }
-      })
     });
   }
 
@@ -48,17 +59,22 @@ describe('RequestService', () => {
     );
   }
 
-  function initServices(selectResult: any) {
+  function initServices(selectResult = defaultSelectResult, hasResponse = defaultHasResponse, getResponse = defaultGetResponse, hasObjectCache: boolean | boolean[] = defaultHasObjectCache) {
+    if (!Array.isArray(hasObjectCache)) {
+      hasObjectCache = [hasObjectCache];
+    }
     objectCache = initMockObjectCacheService();
-    responseCache = initMockResponseCacheService();
+    (objectCache.hasBySelfLink as any).and.returnValues(...hasObjectCache);
+    responseCache = initMockResponseCacheService(hasResponse, getResponse);
     uuidService = initMockUUIDService();
     store = initMockStore<CoreState>(selectResult);
     service = initTestService();
+    serviceAsAny = service as any;
   }
 
   describe('generateRequestId', () => {
     beforeEach(() => {
-      initServices(Observable.of(undefined));
+      initServices();
     });
 
     it('should generate a new request ID', () => {
@@ -70,15 +86,13 @@ describe('RequestService', () => {
   });
 
   describe('isPending', () => {
-    let testRequest: GetRequest;
     describe('before the request is configured', () => {
       beforeEach(() => {
-        initServices(Observable.of(undefined));
+        initServices();
-        testRequest = new GetRequest(testUUID, testHref)
       });
 
       it('should return false', () => {
-        const result = service.isPending(testRequest);
+        const result = service.isPending(testGetRequest);
         const expected = false;
 
         expect(result).toBe(expected);
@@ -87,12 +101,12 @@ describe('RequestService', () => {
 
     describe('when the request has been configured but hasn\'t reached the store yet', () => {
       beforeEach(() => {
-        initServices(Observable.of(undefined));
+        initServices();
       });
 
       it('should return true', () => {
-        (service as any).requestsOnTheirWayToTheStore = [testHref];
+        serviceAsAny.requestsOnTheirWayToTheStore = [testHref];
-        const result = service.isPending(testRequest);
+        const result = service.isPending(testGetRequest);
         const expected = true;
 
         expect(result).toBe(expected);
@@ -107,7 +121,7 @@ describe('RequestService', () => {
       });
 
       it('should return true', () => {
-        const result = service.isPending(testRequest);
+        const result = service.isPending(testGetRequest);
         const expected = true;
 
         expect(result).toBe(expected);
@@ -122,150 +136,309 @@ describe('RequestService', () => {
       });
 
       it('should return false', () => {
-        const result = service.isPending(testRequest);
+        const result = service.isPending(testGetRequest);
         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);
+  describe('getByUUID', () => {
-          const expected = cold('b', {
+    describe('if the request with the specified UUID exists in the store', () => {
-            b: {
+      it('should return an Observable of the RequestEntry', () => {
-              completed: true
+        initServices(hot('a', {
-            }
+          a: {
-          });
+            completed: true
+          }
+        }));
 
-          expect(result).toBeObservable(expected);
+        const result = service.getByUUID(testUUID);
+        const expected = cold('b', {
+          b: {
+            completed: true
+          }
         });
-      });
 
-      describe('if the request with the specified UUID doesn\'t exist in the store', () => {
+        expect(result).toBeObservable(expected);
-        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 with the specified UUID doesn\'t exist in the store', () => {
-      describe('if the request is a GET request', () => {
+      it('should return an Observable of undefined', () => {
-        describe('and it isn\'t already cached', () => {
+        initServices(hot('a', {
-          it('should dispatch the request', () => {
+          a: undefined
-            initServices(Observable.of(undefined));
+        }));
-            spyOn((service as any), 'dispatchRequest');
-            spyOn((service as any), 'isCachedOrPending').and.returnValue(false);
 
-            const request = new GetRequest(testUUID, testHref);
+        const result = service.getByUUID(testUUID);
-            service.configure(request);
+        const expected = cold('b', {
-            expect((service as any).dispatchRequest).toHaveBeenCalledWith(request);
+          b: undefined
-          });
         });
-        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);
+        expect(result).toBeObservable(expected);
-            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);
-        });
       });
     });
 
   });
 
+  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();
+          spyOn(serviceAsAny, 'dispatchRequest');
+          spyOn(serviceAsAny, 'isCachedOrPending').and.returnValue(false);
+
+          service.configure(testGetRequest);
+          expect(serviceAsAny.dispatchRequest).toHaveBeenCalledWith(testGetRequest);
+        });
+      });
+      describe('and it is already cached or pending', () => {
+        it('shouldn\'t dispatch the request', () => {
+          initServices();
+          spyOn(serviceAsAny, 'dispatchRequest');
+          spyOn(serviceAsAny, 'isCachedOrPending').and.returnValue(true);
+
+          service.configure(testGetRequest);
+          expect(serviceAsAny.dispatchRequest).not.toHaveBeenCalled();
+        });
+      });
+    });
+
+    describe('if the request isn\'t a GET request', () => {
+      it('should dispatch the request', () => {
+        initServices();
+        spyOn(serviceAsAny, 'dispatchRequest');
+
+        service.configure(testPostRequest);
+        expect(serviceAsAny.dispatchRequest).toHaveBeenCalledWith(testPostRequest);
+
+        service.configure(testPutRequest);
+        expect(serviceAsAny.dispatchRequest).toHaveBeenCalledWith(testPutRequest);
+
+        service.configure(testDeleteRequest);
+        expect(serviceAsAny.dispatchRequest).toHaveBeenCalledWith(testDeleteRequest);
+
+        service.configure(testOptionsRequest);
+        expect(serviceAsAny.dispatchRequest).toHaveBeenCalledWith(testOptionsRequest);
+
+        service.configure(testHeadRequest);
+        expect(serviceAsAny.dispatchRequest).toHaveBeenCalledWith(testHeadRequest);
+
+        service.configure(testPatchRequest);
+        expect(serviceAsAny.dispatchRequest).toHaveBeenCalledWith(testPatchRequest);
+      });
+    });
+  });
+
+  describe('isCachedOrPending', () => {
+    describe('when the request is cached', () => {
+      describe('in the ObjectCache', () => {
+        it('should return true', () => {
+          initServices(defaultSelectResult, true, Observable.of({
+              response: {
+                isSuccessful: true
+              }
+            }
+          ), true);
+
+          const result = serviceAsAny.isCachedOrPending(testGetRequest);
+          const expected = true;
+
+          expect(result).toEqual(expected);
+        });
+      });
+      describe('in the responseCache', () => {
+        describe('and it\'s a DSOSuccessResponse', () => {
+          it('should return true if all top level links in the response are cached in the object cache', () => {
+            initServices(defaultSelectResult, true, Observable.of({
+                response: {
+                  isSuccessful: true,
+                  resourceSelfLinks: [
+                    'https://rest.api/endpoint/selfLink1',
+                    'https://rest.api/endpoint/selfLink2'
+                  ]
+                }
+              }
+            ), [false, true, true]);
+
+            const result = serviceAsAny.isCachedOrPending(testGetRequest);
+            const expected = true;
+
+            expect(result).toEqual(expected);
+          });
+          it('should return false if not all top level links in the response are cached in the object cache', () => {
+            initServices(defaultSelectResult, true, Observable.of({
+                response: {
+                  isSuccessful: true,
+                  resourceSelfLinks: [
+                    'https://rest.api/endpoint/selfLink1',
+                    'https://rest.api/endpoint/selfLink2'
+                  ]
+                }
+              }
+            ), [false, true, false]);
+
+            const result = serviceAsAny.isCachedOrPending(testGetRequest);
+            const expected = false;
+
+            expect(result).toEqual(expected);
+          });
+        });
+        describe('and it isn\'t a DSOSuccessResponse', () => {
+          it('should return true', () => {
+            initServices(defaultSelectResult, true, Observable.of({
+                response: {
+                  isSuccessful: true
+                }
+              }
+            ), false);
+
+            const result = serviceAsAny.isCachedOrPending(testGetRequest);
+            const expected = true;
+
+            expect(result).toEqual(expected);
+          });
+        });
+      });
+    });
+
+    describe('when the request is pending', () => {
+      it('should return true', () => {
+        initServices();
+        spyOn(service, 'isPending').and.returnValue(true);
+
+        const result = serviceAsAny.isCachedOrPending(testGetRequest);
+        const expected = true;
+
+        expect(result).toEqual(expected);
+      });
+    });
+
+    describe('when the request is neither cached nor pending', () => {
+      it('should return false', () => {
+        initServices();
+
+        const result = serviceAsAny.isCachedOrPending(testGetRequest);
+        const expected = false;
+
+        expect(result).toEqual(expected);
+      });
+    });
+  });
+
+  describe('dispatchRequest', () => {
+    beforeEach(() => {
+      initServices();
+    });
+    it('should dispatch a RequestConfigureAction', () => {
+      const request = testGetRequest;
+      serviceAsAny.dispatchRequest(request);
+      expect(store.dispatch).toHaveBeenCalledWith(new RequestConfigureAction(request));
+    });
+    it('should dispatch a RequestExecuteAction', () => {
+      const request = testGetRequest;
+      serviceAsAny.dispatchRequest(request);
+      expect(store.dispatch).toHaveBeenCalledWith(new RequestExecuteAction(request.uuid));
+    });
+    describe('when it\'s a GET request', () => {
+      it('should track it on it\'s way to the store', () => {
+        spyOn(serviceAsAny, 'trackRequestsOnTheirWayToTheStore');
+        const request = testGetRequest;
+        serviceAsAny.dispatchRequest(request);
+        expect(serviceAsAny.trackRequestsOnTheirWayToTheStore).toHaveBeenCalledWith(request);
+      });
+    });
+    describe('when it\'s not a GET request', () => {
+      it('shouldn\'t track it', () => {
+        spyOn(serviceAsAny, 'trackRequestsOnTheirWayToTheStore');
+
+        serviceAsAny.dispatchRequest(testPostRequest);
+        expect(serviceAsAny.trackRequestsOnTheirWayToTheStore).not.toHaveBeenCalled();
+
+        serviceAsAny.dispatchRequest(testPutRequest);
+        expect(serviceAsAny.trackRequestsOnTheirWayToTheStore).not.toHaveBeenCalled();
+
+        serviceAsAny.dispatchRequest(testDeleteRequest);
+        expect(serviceAsAny.trackRequestsOnTheirWayToTheStore).not.toHaveBeenCalled();
+
+        serviceAsAny.dispatchRequest(testOptionsRequest);
+        expect(serviceAsAny.trackRequestsOnTheirWayToTheStore).not.toHaveBeenCalled();
+
+        serviceAsAny.dispatchRequest(testHeadRequest);
+        expect(serviceAsAny.trackRequestsOnTheirWayToTheStore).not.toHaveBeenCalled();
+
+        serviceAsAny.dispatchRequest(testPatchRequest);
+        expect(serviceAsAny.trackRequestsOnTheirWayToTheStore).not.toHaveBeenCalled();
+      });
+    });
+  });
+
+  describe('trackRequestsOnTheirWayToTheStore', () => {
+    let request: GetRequest;
+
+    beforeEach(() => {
+      initServices();
+      request = testGetRequest;
+    });
+
+    describe('when the method is called with a new request', () => {
+      it('should start tracking the request', () => {
+        expect(serviceAsAny.requestsOnTheirWayToTheStore.includes(request.href)).toBeFalsy();
+        serviceAsAny.trackRequestsOnTheirWayToTheStore(request);
+        expect(serviceAsAny.requestsOnTheirWayToTheStore.includes(request.href)).toBeTruthy();
+      });
+    });
+
+    describe('when the request is added to the store', () => {
+      it('should stop tracking the request', () => {
+        (store.select as any).and.returnValue(Observable.of({ request }));
+        serviceAsAny.trackRequestsOnTheirWayToTheStore(request);
+        expect(serviceAsAny.requestsOnTheirWayToTheStore.includes(request.href)).toBeFalsy();
+      });
+    });
+  });
 });

src/app/shared/mocks/mock-item.ts

@@ -13,7 +13,7 @@ export const MockItem: Item = Object.assign(new Item(), {
     self: 'dspace-angular://aggregated/object/1507836003548',
     requestPending: false,
     responsePending: false,
-    isSuccessFul: true,
+    isSuccessful: true,
     errorMessage: '',
     statusCode: '202',
     pageInfo: {},
@@ -25,7 +25,7 @@ export const MockItem: Item = Object.assign(new Item(), {
           self: 'https://dspace7.4science.it/dspace-spring-rest/api/core/bitstreamformats/10',
           requestPending: false,
           responsePending: false,
-          isSuccessFul: true,
+          isSuccessful: true,
           errorMessage: '',
           statusCode: '202',
           pageInfo: {},
@@ -60,7 +60,7 @@ export const MockItem: Item = Object.assign(new Item(), {
           self: 'https://dspace7.4science.it/dspace-spring-rest/api/core/bitstreamformats/4',
           requestPending: false,
           responsePending: false,
-          isSuccessFul: true,
+          isSuccessful: true,
           errorMessage: '',
           statusCode: '202',
           pageInfo: {},
@@ -191,7 +191,7 @@ export const MockItem: Item = Object.assign(new Item(), {
       self: 'https://dspace7.4science.it/dspace-spring-rest/api/core/collections/1c11f3f1-ba1f-4f36-908a-3f1ea9a557eb',
       requestPending: false,
       responsePending: false,
-      isSuccessFul: true,
+      isSuccessful: true,
       errorMessage: '',
       statusCode: '202',
       pageInfo: {},

src/app/shared/mocks/mock-object-cache.service.ts

@@ -1,7 +1,16 @@
 import { ObjectCacheService } from '../../core/cache/object-cache.service';
 
 export function initMockObjectCacheService(): ObjectCacheService {
-  return jasmine.createSpyObj('objectCacheService', {
+  return jasmine.createSpyObj('objectCacheService', [
-    hasBySelfLink: true,
+    'add',
-  });
+    'remove',
+    'getByUUID',
+    'getBySelfLink',
+    'getRequestHrefBySelfLink',
+    'getRequestHrefByUUID',
+    'getList',
+    'hasByUUID',
+    'hasBySelfLink'
+  ]);
+
 }