replace pathSelector by ngrx memoized selectors

src/app/core/cache/object-cache.service.ts

@@ -1,34 +1,37 @@
+import { Injectable } from '@angular/core';
+import { createSelector, MemoizedSelector, select, Store } from '@ngrx/store';
+import { applyPatch, Operation } from 'fast-json-patch';
 import { combineLatest as observableCombineLatest, Observable } from 'rxjs';
 
 import { distinctUntilChanged, filter, map, mergeMap, take, } from 'rxjs/operators';
-import { Injectable } from '@angular/core';
-import { MemoizedSelector, select, Store } from '@ngrx/store';
-import { IndexName } from '../index/index.reducer';
-
-import { CacheableObject, ObjectCacheEntry } from './object-cache.reducer';
+import { hasNoValue, hasValue, isNotEmpty } from '../../shared/empty.util';
+import { CoreState } from '../core.reducers';
+import { coreSelector } from '../core.selectors';
+import { RestRequestMethod } from '../data/rest-request-method';
+import { selfLinkFromUuidSelector } from '../index/index.selectors';
+import { GenericConstructor } from '../shared/generic-constructor';
+import { NormalizedObjectFactory } from './models/normalized-object-factory';
+import { NormalizedObject } from './models/normalized-object.model';
 import {
   AddPatchObjectCacheAction,
   AddToObjectCacheAction,
   ApplyPatchObjectCacheAction,
   RemoveFromObjectCacheAction
 } from './object-cache.actions';
-import { hasNoValue, isNotEmpty } from '../../shared/empty.util';
-import { GenericConstructor } from '../shared/generic-constructor';
-import { coreSelector, CoreState } from '../core.reducers';
-import { pathSelector } from '../shared/selectors';
-import { NormalizedObjectFactory } from './models/normalized-object-factory';
-import { NormalizedObject } from './models/normalized-object.model';
-import { applyPatch, Operation } from 'fast-json-patch';
+
+import { CacheableObject, ObjectCacheEntry, ObjectCacheState } from './object-cache.reducer';
 import { AddToSSBAction } from './server-sync-buffer.actions';
-import { RestRequestMethod } from '../data/rest-request-method';
 
-function selfLinkFromUuidSelector(uuid: string): MemoizedSelector<CoreState, string> {
-  return pathSelector<CoreState, string>(coreSelector, 'index', IndexName.OBJECT, uuid);
-}
+const objectCacheSelector = createSelector(
+  coreSelector,
+  (state: CoreState) => state['cache/object']
+);
 
-function entryFromSelfLinkSelector(selfLink: string): MemoizedSelector<CoreState, ObjectCacheEntry> {
-  return pathSelector<CoreState, ObjectCacheEntry>(coreSelector, 'cache/object', selfLink);
-}
+const entryFromSelfLinkSelector =
+  (selfLink: string): MemoizedSelector<CoreState, ObjectCacheEntry> => createSelector(
+    objectCacheSelector,
+    (state: ObjectCacheState) => state[selfLink],
+  );
 
   /**
  * A service to interact with the object cache

src/app/core/cache/server-sync-buffer.effects.ts

@@ -1,6 +1,7 @@
 import { delay, exhaustMap, map, switchMap, take } from 'rxjs/operators';
 import { Inject, Injectable } from '@angular/core';
 import { Actions, Effect, ofType } from '@ngrx/effects';
+import { coreSelector } from '../core.selectors';
 import {
   AddToSSBAction,
   CommitSSBAction,
@@ -9,7 +10,7 @@ import {
 } from './server-sync-buffer.actions';
 import { GLOBAL_CONFIG } from '../../../config';
 import { GlobalConfig } from '../../../config/global-config.interface';
-import { coreSelector, CoreState } from '../core.reducers';
+import { CoreState } from '../core.reducers';
 import { Action, createSelector, MemoizedSelector, select, Store } from '@ngrx/store';
 import { ServerSyncBufferEntry, ServerSyncBufferState } from './server-sync-buffer.reducer';
 import { combineLatest as observableCombineLatest, of as observableOf } from 'rxjs';

src/app/core/core.reducers.ts

@@ -4,7 +4,7 @@ import {
 } from '@ngrx/store';
 
 import { objectCacheReducer, ObjectCacheState } from './cache/object-cache.reducer';
-import { indexReducer, IndexState } from './index/index.reducer';
+import { indexReducer, MetaIndexState } from './index/index.reducer';
 import { requestReducer, RequestState } from './data/request.reducer';
 import { authReducer, AuthState } from './auth/auth.reducer';
 import { serverSyncBufferReducer, ServerSyncBufferState } from './cache/server-sync-buffer.reducer';
@@ -18,7 +18,7 @@ export interface CoreState {
   'cache/syncbuffer': ServerSyncBufferState,
   'cache/object-updates': ObjectUpdatesState
   'data/request': RequestState,
-  'index': IndexState,
+  'index': MetaIndexState,
   'auth': AuthState,
 }
 
@@ -30,5 +30,3 @@ export const coreReducers: ActionReducerMap<CoreState> = {
   'index': indexReducer,
   'auth': authReducer,
 };
-
-export const coreSelector = createFeatureSelector<CoreState>('core');

src/app/core/core.selectors.ts

@@ -0,0 +1,4 @@
+import { createFeatureSelector } from '@ngrx/store';
+import { CoreState } from './core.reducers';
+
+export const coreSelector = createFeatureSelector<CoreState>('core');

src/app/core/data/object-updates/object-updates.service.ts

@@ -1,6 +1,7 @@
 import { Injectable } from '@angular/core';
 import { createSelector, MemoizedSelector, select, Store } from '@ngrx/store';
-import { coreSelector, CoreState } from '../../core.reducers';
+import { CoreState } from '../../core.reducers';
+import { coreSelector } from '../../core.selectors';
 import {
   FieldState,
   FieldUpdates,

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

@@ -22,7 +22,7 @@ import { ActionsSubject, Store } from '@ngrx/store';
 import { TestScheduler } from 'rxjs/testing';
 import { BehaviorSubject } from 'rxjs/internal/BehaviorSubject';
 import { MockStore } from '../../shared/testing/mock-store';
-import { IndexState } from '../index/index.reducer';
+import { MetaIndexState } from '../index/index.reducer';
 
 describe('RequestService', () => {
   let scheduler: TestScheduler;

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

@@ -1,59 +1,51 @@
 import { merge as observableMerge, Observable, of as observableOf } from 'rxjs';
 import {
-  distinctUntilChanged,
   filter,
-  find,
-  first,
   map,
   mergeMap,
-  reduce,
-  startWith,
   switchMap,
   take,
-  tap
 } from 'rxjs/operators';
 import { race as observableRace } from 'rxjs';
 import { Injectable } from '@angular/core';
 
 import { createSelector, MemoizedSelector, select, Store } from '@ngrx/store';
-import { hasNoValue, hasValue, isNotEmpty, isNotUndefined } from '../../shared/empty.util';
+import { AppState } from '../../app.reducer';
+import { hasNoValue, hasValue, isNotEmpty } from '../../shared/empty.util';
 import { CacheableObject } from '../cache/object-cache.reducer';
 import { ObjectCacheService } from '../cache/object-cache.service';
 import { DSOSuccessResponse, RestResponse } from '../cache/response.models';
-import { coreSelector, CoreState } from '../core.reducers';
-import { IndexName, IndexState } from '../index/index.reducer';
-import { pathSelector } from '../shared/selectors';
+import { CoreState } from '../core.reducers';
+import { coreSelector } from '../core.selectors';
+import {
+  IndexName, IndexState,
+  MetaIndexState
+} from '../index/index.reducer';
+import {
+  originalRequestUUIDFromRequestUUIDSelector, requestIndexSelector,
+  uuidFromHrefSelector
+} from '../index/index.selectors';
 import { UUIDService } from '../shared/uuid.service';
 import { RequestConfigureAction, RequestExecuteAction, RequestRemoveAction } from './request.actions';
 import { GetRequest, RestRequest } from './request.models';
 
-import { RequestEntry } from './request.reducer';
+import { RequestEntry, RequestState } from './request.reducer';
 import { CommitSSBAction } from '../cache/server-sync-buffer.actions';
 import { RestRequestMethod } from './rest-request-method';
 import { getResponseFromEntry } from '../shared/operators';
 import { AddToIndexAction, RemoveFromIndexBySubstringAction } from '../index/index.actions';
 
-@Injectable()
-export class RequestService {
-  private requestsOnTheirWayToTheStore: string[] = [];
+const requestCacheSelector = createSelector(
+  coreSelector,
+  (state: CoreState) => state['data/request']
+);
 
-  constructor(private objectCache: ObjectCacheService,
-              private uuidService: UUIDService,
-              private store: Store<CoreState>,
-              private indexStore: Store<IndexState>) {
-  }
-
-  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);
-  }
-
-  private originalUUIDFromUUIDSelector(uuid: string): MemoizedSelector<CoreState, string> {
-    return pathSelector<CoreState, string>(coreSelector, 'index', IndexName.UUID_MAPPING, uuid);
+const entryFromUUIDSelector = (uuid: string): MemoizedSelector<CoreState, RequestEntry> => createSelector(
+  requestCacheSelector,
+  (state: RequestState) => {
+    return hasValue(state) ? state[uuid] : undefined;
   }
+);
 
 /**
  * Create a selector that fetches a list of request UUIDs from a given index substate of which the request href
@@ -62,29 +54,34 @@ export class RequestService {
  * @param name        The name of the index substate we're fetching request UUIDs from
  * @param href        Substring that the request's href should contain
  */
-  private uuidsFromHrefSubstringSelector(selector: MemoizedSelector<any, IndexState>, name: string, href: string): MemoizedSelector<any, string[]> {
-    return createSelector(selector, (state: IndexState) => this.getUuidsFromHrefSubstring(state, name, href));
-  }
+const uuidsFromHrefSubstringSelector =
+  (selector: MemoizedSelector<AppState, IndexState>, href: string): MemoizedSelector<AppState, string[]> => createSelector(
+    selector,
+    (state: IndexState) => getUuidsFromHrefSubstring(state, href)
+  );
 
 /**
  * Fetch a list of request UUIDs from a given index substate of which the request href contains a given substring
  * @param state   The IndexState
-   * @param name    The name of the index substate we're fetching request UUIDs from
  * @param href    Substring that the request's href should contain
  */
-  private getUuidsFromHrefSubstring(state: IndexState, name: string, href: string): string[] {
+const getUuidsFromHrefSubstring = (state: IndexState, href: string): string[] => {
   let result = [];
   if (isNotEmpty(state)) {
-      const subState = state[name];
-      if (isNotEmpty(subState)) {
-        for (const value in subState) {
-          if (value.indexOf(href) > -1) {
-            result = [...result, subState[value]];
-          }
-        }
-      }
+    result = Object.values(state)
+      .filter((value: string) => value.startsWith(href));
   }
   return result;
+};
+
+@Injectable()
+export class RequestService {
+  private requestsOnTheirWayToTheStore: string[] = [];
+
+  constructor(private objectCache: ObjectCacheService,
+              private uuidService: UUIDService,
+              private store: Store<CoreState>,
+              private indexStore: Store<MetaIndexState>) {
   }
 
   generateRequestId(): string {
@@ -110,11 +107,11 @@ export class RequestService {
 
   getByUUID(uuid: string): Observable<RequestEntry> {
     return observableRace(
-      this.store.pipe(select(this.entryFromUUIDSelector(uuid))),
+      this.store.pipe(select(entryFromUUIDSelector(uuid))),
       this.store.pipe(
-        select(this.originalUUIDFromUUIDSelector(uuid)),
+        select(originalRequestUUIDFromRequestUUIDSelector(uuid)),
         mergeMap((originalUUID) => {
-            return this.store.pipe(select(this.entryFromUUIDSelector(originalUUID)))
+            return this.store.pipe(select(entryFromUUIDSelector(originalUUID)))
           },
         ))
     );
@@ -122,7 +119,7 @@ export class RequestService {
 
   getByHref(href: string): Observable<RequestEntry> {
     return this.store.pipe(
-      select(this.uuidFromHrefSelector(href)),
+      select(uuidFromHrefSelector(href)),
       mergeMap((uuid: string) => this.getByUUID(uuid))
     );
   }
@@ -159,7 +156,7 @@ export class RequestService {
    */
   removeByHrefSubstring(href: string) {
     this.store.pipe(
-      select(this.uuidsFromHrefSubstringSelector(pathSelector<CoreState, IndexState>(coreSelector, 'index'), IndexName.REQUEST, href)),
+      select(uuidsFromHrefSubstringSelector(requestIndexSelector, href)),
       take(1)
     ).subscribe((uuids: string[]) => {
       for (const uuid of uuids) {
@@ -230,7 +227,7 @@ export class RequestService {
    */
   private trackRequestsOnTheirWayToTheStore(request: GetRequest) {
     this.requestsOnTheirWayToTheStore = [...this.requestsOnTheirWayToTheStore, request.href];
-    this.store.pipe(select(this.entryFromUUIDSelector(request.href)),
+    this.store.pipe(select(entryFromUUIDSelector(request.href)),
       filter((re: RequestEntry) => hasValue(re)),
       take(1)
     ).subscribe((re: RequestEntry) => {

src/app/core/index/index.reducer.spec.ts

@@ -1,6 +1,6 @@
 import * as deepFreeze from 'deep-freeze';
 
-import { IndexName, indexReducer, IndexState } from './index.reducer';
+import { IndexName, indexReducer, MetaIndexState } from './index.reducer';
 import { AddToIndexAction, RemoveFromIndexBySubstringAction, RemoveFromIndexByValueAction } from './index.actions';
 
 class NullAction extends AddToIndexAction {
@@ -17,7 +17,7 @@ describe('requestReducer', () => {
   const key2 = '1911e8a4-6939-490c-b58b-a5d70f8d91fb';
   const val1 = 'https://dspace7.4science.it/dspace-spring-rest/api/core/items/567a639f-f5ff-4126-807c-b7d0910808c8';
   const val2 = 'https://dspace7.4science.it/dspace-spring-rest/api/core/items/1911e8a4-6939-490c-b58b-a5d70f8d91fb';
-  const testState: IndexState = {
+  const testState: MetaIndexState = {
     [IndexName.OBJECT]: {
       [key1]: val1
     },[IndexName.REQUEST]: {

src/app/core/index/index.reducer.ts

@@ -1,8 +1,9 @@
 import {
+  AddToIndexAction,
   IndexAction,
   IndexActionTypes,
-  AddToIndexAction,
-  RemoveFromIndexByValueAction, RemoveFromIndexBySubstringAction
+  RemoveFromIndexBySubstringAction,
+  RemoveFromIndexByValueAction
 } from './index.actions';
 
 export enum IndexName {
@@ -11,16 +12,18 @@ export enum IndexName {
   UUID_MAPPING = 'get-request/configured-to-cache-uuid'
 }
 
-export type IndexState = {
-  [name in IndexName]: {
+export interface IndexState {
   [key: string]: string
 }
+
+export type MetaIndexState = {
+  [name in IndexName]: IndexState
 }
 
 // Object.create(null) ensures the object has no default js properties (e.g. `__proto__`)
-const initialState: IndexState = Object.create(null);
+const initialState: MetaIndexState = Object.create(null);
 
-export function indexReducer(state = initialState, action: IndexAction): IndexState {
+export function indexReducer(state = initialState, action: IndexAction): MetaIndexState {
   switch (action.type) {
 
     case IndexActionTypes.ADD: {
@@ -41,7 +44,7 @@ export function indexReducer(state = initialState, action: IndexAction): IndexSt
   }
 }
 
-function addToIndex(state: IndexState, action: AddToIndexAction): IndexState {
+function addToIndex(state: MetaIndexState, action: AddToIndexAction): MetaIndexState {
   const subState = state[action.payload.name];
   const newSubState = Object.assign({}, subState, {
     [action.payload.key]: action.payload.value
@@ -52,7 +55,7 @@ function addToIndex(state: IndexState, action: AddToIndexAction): IndexState {
   return obs;
 }
 
-function removeFromIndexByValue(state: IndexState, action: RemoveFromIndexByValueAction): IndexState {
+function removeFromIndexByValue(state: MetaIndexState, action: RemoveFromIndexByValueAction): MetaIndexState {
   const subState = state[action.payload.name];
   const newSubState = Object.create(null);
   for (const value in subState) {
@@ -70,7 +73,7 @@ function removeFromIndexByValue(state: IndexState, action: RemoveFromIndexByValu
  * @param state     The IndexState to remove values from
  * @param action    The RemoveFromIndexByValueAction containing the necessary information to remove the values
  */
-function removeFromIndexBySubstring(state: IndexState, action: RemoveFromIndexByValueAction): IndexState {
+function removeFromIndexBySubstring(state: MetaIndexState, action: RemoveFromIndexByValueAction): MetaIndexState {
   const subState = state[action.payload.name];
   const newSubState = Object.create(null);
   for (const value in subState) {

src/app/core/index/index.selectors.ts

@@ -0,0 +1,45 @@
+import { createSelector, MemoizedSelector } from '@ngrx/store';
+import { AppState } from '../../app.reducer';
+import { hasValue } from '../../shared/empty.util';
+import { CoreState } from '../core.reducers';
+import { coreSelector } from '../core.selectors';
+import { IndexName, IndexState, MetaIndexState } from './index.reducer';
+
+export const metaIndexSelector: MemoizedSelector<AppState, MetaIndexState> = createSelector(
+  coreSelector,
+  (state: CoreState) => state.index
+);
+export const objectIndexSelector: MemoizedSelector<AppState, IndexState> = createSelector(
+  metaIndexSelector,
+  (state: MetaIndexState) => state[IndexName.OBJECT]
+);
+export const requestIndexSelector: MemoizedSelector<AppState, IndexState> = createSelector(
+  metaIndexSelector,
+  (state: MetaIndexState) => state[IndexName.REQUEST]
+);
+export const requestUUIDIndexSelector: MemoizedSelector<AppState, IndexState> = createSelector(
+  metaIndexSelector,
+  (state: MetaIndexState) => state[IndexName.UUID_MAPPING]
+);
+export const selfLinkFromUuidSelector =
+  (uuid: string): MemoizedSelector<AppState, string> => createSelector(
+    objectIndexSelector,
+    (state: IndexState) => hasValue(state) ? state[uuid] : undefined
+  );
+export const uuidFromHrefSelector =
+  (href: string): MemoizedSelector<AppState, string> => createSelector(
+    requestIndexSelector,
+    (state: IndexState) => hasValue(state) ? state[href] : undefined
+  );
+/**
+ * If a request wasn't sent to the server because the result was already cached,
+ * this selector allows you to find the UUID of the cached request based on the
+ * UUID of the new request
+ *
+ * @param uuid    The uuid of the new request
+ */
+export const originalRequestUUIDFromRequestUUIDSelector =
+  (uuid: string): MemoizedSelector<AppState, string> => createSelector(
+    requestUUIDIndexSelector,
+    (state: IndexState) => hasValue(state) ? state[uuid] : undefined
+  );

src/app/core/shared/selectors.ts

@@ -1,17 +0,0 @@
-import { createSelector, MemoizedSelector } from '@ngrx/store';
-import { hasNoValue, isEmpty } from '../../shared/empty.util';
-
-export function pathSelector<From, To>(selector: MemoizedSelector<any, From>, ...path: string[]): MemoizedSelector<any, To> {
-  return createSelector(selector, (state: any) => getSubState(state, path));
-}
-
-function getSubState(state: any, path: string[]) {
-  const current = path[0];
-  const remainingPath = path.slice(1);
-  const subState = state[current];
-  if (hasNoValue(subState) || isEmpty(remainingPath)) {
-    return subState;
-  } else {
-    return getSubState(subState, remainingPath);
-  }
-}