Merge branch 'response-cache-refactoring' into w2p-54472_Create-community-and-collection-pages

Conflicts:
	src/app/core/metadata/metadata.service.spec.ts

src/app/core/data/base-response-parsing.service.ts

@@ -6,7 +6,6 @@ import { ObjectCacheService } from '../cache/object-cache.service';
 import { GlobalConfig } from '../../../config/global-config.interface';
 import { GenericConstructor } from '../shared/generic-constructor';
 import { PaginatedList } from './paginated-list';
-import { NormalizedObject } from '../cache/models/normalized-object.model';
 import { ResourceType } from '../shared/resource-type';
 import { RESTURLCombiner } from '../url-combiner/rest-url-combiner';
 
@@ -15,7 +14,7 @@ function isObjectLevel(halObj: any) {
 }
 
 function isPaginatedResponse(halObj: any) {
-  return isNotEmpty(halObj.page) && hasValue(halObj._embedded);
+  return hasValue(halObj.page) && hasValue(halObj._embedded);
 }
 
 /* tslint:disable:max-classes-per-file */
@@ -26,14 +25,14 @@ export abstract class BaseResponseParsingService {
   protected abstract objectFactory: any;
   protected abstract toCache: boolean;
 
-  protected process<ObjectDomain, ObjectType>(data: any, requestHref: string): any {
+  protected process<ObjectDomain, ObjectType>(data: any, requestUUID: string): any {
     if (isNotEmpty(data)) {
       if (hasNoValue(data) || (typeof data !== 'object')) {
         return data;
       } else if (isPaginatedResponse(data)) {
-        return this.processPaginatedList(data, requestHref);
+        return this.processPaginatedList(data, requestUUID);
       } else if (Array.isArray(data)) {
-        return this.processArray(data, requestHref);
+        return this.processArray(data, requestUUID);
       } else if (isObjectLevel(data)) {
         data = this.fixBadEPersonRestResponse(data);
         const object = this.deserialize(data);
@@ -42,7 +41,7 @@ export abstract class BaseResponseParsingService {
             .keys(data._embedded)
             .filter((property) => data._embedded.hasOwnProperty(property))
             .forEach((property) => {
-              const parsedObj = this.process<ObjectDomain, ObjectType>(data._embedded[property], requestHref);
+              const parsedObj = this.process<ObjectDomain, ObjectType>(data._embedded[property], requestUUID);
               if (isNotEmpty(parsedObj)) {
                 if (isPaginatedResponse(data._embedded[property])) {
                   object[property] = parsedObj;
@@ -56,7 +55,7 @@ export abstract class BaseResponseParsingService {
             });
         }
 
-        this.cache(object, requestHref);
+        this.cache(object, requestUUID);
         return object;
       }
       const result = {};
@@ -64,7 +63,7 @@ export abstract class BaseResponseParsingService {
         .filter((property) => data.hasOwnProperty(property))
         .filter((property) => hasValue(data[property]))
         .forEach((property) => {
-          const obj = this.process(data[property], requestHref);
+          const obj = this.process(data[property], requestUUID);
           result[property] = obj;
         });
       return result;
@@ -72,7 +71,7 @@ export abstract class BaseResponseParsingService {
     }
   }
 
-  protected processPaginatedList<ObjectDomain, ObjectType>(data: any, requestHref: string): PaginatedList<ObjectDomain> {
+  protected processPaginatedList<ObjectDomain, ObjectType>(data: any, requestUUID: string): PaginatedList<ObjectDomain> {
     const pageInfo: PageInfo = this.processPageInfo(data);
     let list = data._embedded;
 
@@ -80,14 +79,14 @@ export abstract class BaseResponseParsingService {
     if (!Array.isArray(list)) {
       list = this.flattenSingleKeyObject(list);
     }
-    const page: ObjectDomain[] = this.processArray(list, requestHref);
+    const page: ObjectDomain[] = this.processArray(list, requestUUID);
     return new PaginatedList<ObjectDomain>(pageInfo, page);
   }
 
-  protected processArray<ObjectDomain, ObjectType>(data: any, requestHref: string): ObjectDomain[] {
+  protected processArray<ObjectDomain, ObjectType>(data: any, requestUUID: string): ObjectDomain[] {
     let array: ObjectDomain[] = [];
     data.forEach((datum) => {
-        array = [...array, this.process(datum, requestHref)];
+        array = [...array, this.process(datum, requestUUID)];
       }
     );
     return array;
@@ -115,21 +114,21 @@ export abstract class BaseResponseParsingService {
     }
   }
 
-  protected cache<ObjectDomain, ObjectType>(obj, requestHref) {
+  protected cache<ObjectDomain, ObjectType>(obj, requestUUID) {
     if (this.toCache) {
-      this.addToObjectCache(obj, requestHref);
+      this.addToObjectCache(obj, requestUUID);
     }
   }
 
-  protected addToObjectCache(co: CacheableObject, requestHref: string): void {
+  protected addToObjectCache(co: CacheableObject, requestUUID: string): void {
     if (hasNoValue(co) || hasNoValue(co.self)) {
       throw new Error('The server returned an invalid object');
     }
-    this.objectCache.add(co, this.EnvConfig.cache.msToLive.default, requestHref);
+    this.objectCache.add(co, this.EnvConfig.cache.msToLive.default, requestUUID);
   }
 
   processPageInfo(payload: any): PageInfo {
-    if (isNotEmpty(payload.page)) {
+    if (hasValue(payload.page)) {
       const pageObj = Object.assign({}, payload.page, { _links: payload._links });
       const pageInfoObject = new DSpaceRESTv2Serializer(PageInfo).deserialize(pageObj);
       if (pageInfoObject.currentPage >= 0) {

src/app/core/data/comcol-data.service.ts

@@ -49,23 +49,6 @@ export abstract class ComColDataService<TNormalized extends NormalizedObject, TD
           this.requestService.configure(request);
         }));
 
-      // return scopeCommunityHrefObs.pipe(
-      //   mergeMap((href: string) => this.responseCache.get(href)),
-      //   map((entry: ResponseCacheEntry) => entry.response),
-      //   mergeMap((response) => {
-      //     if (response.isSuccessful) {
-      //       const community$: Observable<NormalizedCommunity> = this.objectCache.getByUUID(scopeID);
-      //       return community$.pipe(
-      //         map((community) => community._links[linkPath]),
-      //         filter((href) => isNotEmpty(href)),
-      //         distinctUntilChanged()
-      //       );
-      //     } else if (!response.isSuccessful) {
-      //       return observableThrowError(new Error(`The Community with scope ${scopeID} couldn't be retrieved`))
-      //     }
-      //   }),
-      //   distinctUntilChanged()
-      // );
       const responses = scopeCommunityHrefObs.pipe(
         mergeMap((href: string) => this.requestService.getByHref(href)),
         getResponseFromEntry()

src/app/core/data/config-response-parsing.service.ts

@@ -28,7 +28,7 @@ export class ConfigResponseParsingService extends BaseResponseParsingService imp
 
   parse(request: RestRequest, data: DSpaceRESTV2Response): RestResponse {
     if (isNotEmpty(data.payload) && isNotEmpty(data.payload._links) && (data.statusCode === '201' || data.statusCode === '200' || data.statusCode === 'OK')) {
-      const configDefinition = this.process<ConfigObject,ConfigType>(data.payload, request.href);
+      const configDefinition = this.process<ConfigObject,ConfigType>(data.payload, request.uuid);
       return new ConfigSuccessResponse(configDefinition, data.statusCode, this.processPageInfo(data.payload));
     } else {
       return new ErrorResponse(

src/app/core/data/dso-response-parsing.service.ts

@@ -28,7 +28,7 @@ export class DSOResponseParsingService extends BaseResponseParsingService implem
   }
 
   parse(request: RestRequest, data: DSpaceRESTV2Response): RestResponse {
-    const processRequestDTO = this.process<NormalizedObject, ResourceType>(data.payload, request.href);
+    const processRequestDTO = this.process<NormalizedObject, ResourceType>(data.payload, request.uuid);
     let objectList = processRequestDTO;
 
     if (hasNoValue(processRequestDTO)) {

src/app/core/data/facet-value-map-response-parsing.service.ts

@@ -9,8 +9,6 @@ import { ResponseParsingService } from './parsing.service';
 import { RestRequest } from './request.models';
 import { DSpaceRESTV2Response } from '../dspace-rest-v2/dspace-rest-v2-response.model';
 import { DSpaceRESTv2Serializer } from '../dspace-rest-v2/dspace-rest-v2.serializer';
-import { PageInfo } from '../shared/page-info.model';
-import { isNotEmpty } from '../../shared/empty.util';
 import { FacetValue } from '../../+search-page/search-service/facet-value.model';
 import { BaseResponseParsingService } from './base-response-parsing.service';
 import { ObjectCacheService } from '../cache/object-cache.service';

src/app/core/data/facet-value-response-parsing.service.ts

@@ -1,16 +1,9 @@
 import { Inject, Injectable } from '@angular/core';
-import {
-  FacetValueMap,
-  FacetValueMapSuccessResponse,
-  FacetValueSuccessResponse,
-  RestResponse
-} from '../cache/response.models';
+import { FacetValueSuccessResponse, RestResponse } from '../cache/response.models';
 import { ResponseParsingService } from './parsing.service';
 import { RestRequest } from './request.models';
 import { DSpaceRESTV2Response } from '../dspace-rest-v2/dspace-rest-v2-response.model';
 import { DSpaceRESTv2Serializer } from '../dspace-rest-v2/dspace-rest-v2.serializer';
-import { PageInfo } from '../shared/page-info.model';
-import { isNotEmpty } from '../../shared/empty.util';
 import { FacetValue } from '../../+search-page/search-service/facet-value.model';
 import { BaseResponseParsingService } from './base-response-parsing.service';
 import { ObjectCacheService } from '../cache/object-cache.service';

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

@@ -174,7 +174,7 @@ describe('RequestService', () => {
         //   b: undefined
         // });
 
-        scheduler.expectObservable(result).toBe('b', {b: undefined});
+        scheduler.expectObservable(result).toBe('b', { b: undefined });
       });
     });
 
@@ -458,4 +458,105 @@ describe('RequestService', () => {
       });
     });
   });
+
+  describe('isReusable', () => {
+    describe('when the given UUID is has no value', () => {
+      let reusable;
+      beforeEach(() => {
+        const uuid = undefined;
+        reusable = serviceAsAny.isReusable(uuid);
+      });
+      it('return an observable emitting false', () => {
+        reusable.subscribe((isReusable) => expect(isReusable).toBe(false));
+      })
+    });
+
+    describe('when the given UUID has a value, but no cached entry is found', () => {
+      let reusable;
+      beforeEach(() => {
+        spyOn(service, 'getByUUID').and.returnValue(observableOf(undefined));
+        const uuid = 'a45bb291-1adb-40d9-b2fc-7ad9080607be';
+        reusable = serviceAsAny.isReusable(uuid);
+      });
+      it('return an observable emitting false', () => {
+        reusable.subscribe((isReusable) => expect(isReusable).toBe(false));
+      })
+    });
+
+    describe('when the given UUID has a value, a cached entry is found, but it has no response', () => {
+      let reusable;
+      beforeEach(() => {
+        spyOn(service, 'getByUUID').and.returnValue(observableOf({ response: undefined }));
+        const uuid = '53c9b814-ad8b-4567-9bc1-d9bb6cfba6c8';
+        reusable = serviceAsAny.isReusable(uuid);
+      });
+      it('return an observable emitting false', () => {
+        reusable.subscribe((isReusable) => expect(isReusable).toBe(false));
+      })
+    });
+
+    describe('when the given UUID has a value, a cached entry is found, but its response was not successful', () => {
+      let reusable;
+      beforeEach(() => {
+        spyOn(service, 'getByUUID').and.returnValue(observableOf({ response: { isSuccessful: false } }));
+        const uuid = '694c9b32-7b2e-4788-835b-ef3fc2252e6c';
+        reusable = serviceAsAny.isReusable(uuid);
+      });
+      it('return an observable emitting false', () => {
+        reusable.subscribe((isReusable) => expect(isReusable).toBe(false));
+      })
+    });
+
+    describe('when the given UUID has a value, a cached entry is found, its response was successful, but the response is outdated', () => {
+      let reusable;
+      const now = 100000;
+      const timeAdded = 99899;
+      const msToLive = 100;
+
+      beforeEach(() => {
+        spyOn(Date.prototype, 'getTime').and.returnValue(now);
+        spyOn(service, 'getByUUID').and.returnValue(observableOf({
+          response: {
+            isSuccessful: true,
+            timeAdded: timeAdded
+          },
+          request: {
+            responseMsToLive: msToLive
+          }
+        }));
+        const uuid = 'f9b85788-881c-4994-86b6-bae8dad024d2';
+        reusable = serviceAsAny.isReusable(uuid);
+      });
+
+      it('return an observable emitting false', () => {
+        reusable.subscribe((isReusable) => expect(isReusable).toBe(false));
+      })
+    });
+
+    describe('when the given UUID has a value, a cached entry is found, its response was successful, and the response is not outdated', () => {
+      let reusable;
+      const now = 100000;
+      const timeAdded = 99999;
+      const msToLive = 100;
+
+      beforeEach(() => {
+        spyOn(Date.prototype, 'getTime').and.returnValue(now);
+        spyOn(service, 'getByUUID').and.returnValue(observableOf({
+          response: {
+            isSuccessful: true,
+            timeAdded: timeAdded
+          },
+          request: {
+            responseMsToLive: msToLive
+          }
+        }));
+        const uuid = 'f9b85788-881c-4994-86b6-bae8dad024d2';
+        reusable = serviceAsAny.isReusable(uuid);
+      });
+
+      it('return an observable emitting true', () => {
+        reusable.subscribe((isReusable) => expect(isReusable).toBe(true));
+      })
+    })
+  })
 });

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

@@ -76,7 +76,7 @@ export class RequestService {
   }
 
   getByUUID(uuid: string): Observable<RequestEntry> {
-     return observableRace(
+    return observableRace(
       this.store.pipe(select(this.entryFromUUIDSelector(uuid))),
       this.store.pipe(
         select(this.originalUUIDFromUUIDSelector(uuid)),
@@ -94,6 +94,12 @@ export class RequestService {
     );
   }
 
+  /**
+   * Configure a certain request
+   * Used to make sure a request is in the cache
+   * @param {RestRequest} request The request to send out
+   * @param {boolean} forceBypassCache When true, a new request is always dispatched
+   */
   // TODO to review "overrideRequest" param when https://github.com/DSpace/dspace-angular/issues/217 will be fixed
   configure<T extends CacheableObject>(request: RestRequest, forceBypassCache: boolean = false): void {
     const isGetRequest = request.method === RestRequestMethod.GET;
@@ -113,6 +119,11 @@ export class RequestService {
     }
   }
 
+  /**
+   * Check if a request is in the cache or if it's still pending
+   * @param {GetRequest} request The request to check
+   * @returns {boolean} True if the request is cached or still pending
+   */
   private isCachedOrPending(request: GetRequest) {
     let isCached = this.objectCache.hasBySelfLink(request.href);
     if (isCached) {
@@ -142,6 +153,10 @@ export class RequestService {
     return isCached || isPending;
   }
 
+  /**
+   * Configure and execute the request
+   * @param {RestRequest} request to dispatch
+   */
   private dispatchRequest(request: RestRequest) {
     this.store.dispatch(new RequestConfigureAction(request));
     this.store.dispatch(new RequestExecuteAction(request.uuid));
@@ -164,6 +179,10 @@ export class RequestService {
     });
   }
 
+  /**
+   * Dispatch commit action to send all changes (for a certain method) to the server (buffer)
+   * @param {RestRequestMethod} method RestRequestMethod for which the changes should be committed
+   */
   commit(method?: RestRequestMethod) {
     this.store.dispatch(new CommitSSBAction(method))
   }
@@ -171,11 +190,11 @@ export class RequestService {
   /**
    * Check whether a Response should still be cached
    *
-   * @param entry
-   *    the entry to check
+   * @param uuid
+   *    the uuid of the entry to check
    * @return boolean
-   *    false if the entry is null, undefined, or its time to
-   *    live has been exceeded, true otherwise
+   *    false if the uuid has no value, no entry could be found, the response was nog successful or its time to
+   *    live has exceeded, true otherwise
    */
   private isReusable(uuid: string): Observable<boolean> {
     if (hasNoValue(uuid)) {
@@ -194,7 +213,6 @@ export class RequestService {
           }
         })
       );
-      return observableOf(false);
     }
   }
 }