From 2e5441d6f7f8a6eaf0e91ad45da2b567e7387481 Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Wed, 15 Feb 2017 15:57:38 +0100 Subject: [PATCH] Moved all objects to a single data store: the cache. --- config/environment.default.js | 6 +- src/app/core/core.module.ts | 4 +- src/app/core/core.reducers.ts | 5 +- .../core/data-services/cache/cache.actions.ts | 33 +++++++++++ .../core/data-services/cache/cache.reducer.ts | 58 +++++++++++++++++++ .../core/data-services/cache/cache.service.ts | 58 +++++++++++++++++++ .../collection/collection-data.effects.ts | 41 +++++++++---- .../collection/collection-data.service.ts | 19 +++--- .../collection-find-multiple.actions.ts | 6 +- .../collection-find-multiple.reducer.ts | 8 +-- .../collection-find-single.actions.ts | 6 +- .../collection-find-single.reducer.ts | 12 ++-- src/app/core/shared/dspace-object.model.ts | 11 +++- ...cache.service.ts => demo-cache.service.ts} | 6 +- src/app/shared/model/model.service.ts | 4 +- src/backend/api.ts | 2 +- src/browser.module.ts | 8 +-- src/node.module.ts | 8 +-- 18 files changed, 240 insertions(+), 55 deletions(-) create mode 100644 src/app/core/data-services/cache/cache.actions.ts create mode 100644 src/app/core/data-services/cache/cache.reducer.ts create mode 100644 src/app/core/data-services/cache/cache.service.ts rename src/app/shared/{cache.service.ts => demo-cache.service.ts} (95%) diff --git a/config/environment.default.js b/config/environment.default.js index 17d6f5afdc..a39db0dff8 100644 --- a/config/environment.default.js +++ b/config/environment.default.js @@ -10,5 +10,9 @@ module.exports = { "ui": { "nameSpace": "/", "baseURL": "http://localhost:3000" + }, + "cache": { + // how long should objects be cached for by default + "msToLive": 15 * 60 * 1000 //15 minutes } -} +}; diff --git a/src/app/core/core.module.ts b/src/app/core/core.module.ts index 7fece0601a..e7fb604c34 100644 --- a/src/app/core/core.module.ts +++ b/src/app/core/core.module.ts @@ -4,6 +4,7 @@ import { SharedModule } from "../shared/shared.module"; import { isNotEmpty } from "../shared/empty.util"; import { DSpaceRESTv2Service } from "./dspace-rest-v2/dspace-rest-v2.service"; import { CollectionDataService } from "./data-services/collection/collection-data.service"; +import { CacheService } from "./data-services/cache/cache.service"; const IMPORTS = [ CommonModule, @@ -18,7 +19,8 @@ const EXPORTS = [ const PROVIDERS = [ CollectionDataService, - DSpaceRESTv2Service + DSpaceRESTv2Service, + CacheService ]; @NgModule({ diff --git a/src/app/core/core.reducers.ts b/src/app/core/core.reducers.ts index 47be0d13f3..9adea94095 100644 --- a/src/app/core/core.reducers.ts +++ b/src/app/core/core.reducers.ts @@ -3,13 +3,16 @@ import { CollectionDataState, collectionDataReducer } from "./data-services/collection/collection-data.reducer"; +import { CacheState, cacheReducer } from "./data-services/cache/cache.reducer"; export interface CoreState { - collectionData: CollectionDataState + collectionData: CollectionDataState, + cache: CacheState } export const reducers = { collectionData: collectionDataReducer, + cache: cacheReducer }; export function coreReducer(state: any, action: any) { diff --git a/src/app/core/data-services/cache/cache.actions.ts b/src/app/core/data-services/cache/cache.actions.ts new file mode 100644 index 0000000000..43146f5ce9 --- /dev/null +++ b/src/app/core/data-services/cache/cache.actions.ts @@ -0,0 +1,33 @@ +import { Action } from "@ngrx/store"; +import { type } from "../../../shared/ngrx/type"; +import { CacheableObject } from "./cache.reducer"; + +export const CacheActionTypes = { + ADD: type('dspace/core/data/cache/ADD'), + REMOVE: type('dspace/core/data/cache/REMOVE') +}; + +export class AddToCacheAction implements Action { + type = CacheActionTypes.ADD; + payload: { + objectToCache: CacheableObject; + msToLive: number; + }; + + constructor(objectToCache: CacheableObject, msToLive: number) { + this.payload = { objectToCache, msToLive }; + } +} + +export class RemoveFromCacheAction implements Action { + type = CacheActionTypes.REMOVE; + payload: string; + + constructor(uuid: string) { + this.payload = uuid; + } +} + +export type CacheAction + = AddToCacheAction + | RemoveFromCacheAction diff --git a/src/app/core/data-services/cache/cache.reducer.ts b/src/app/core/data-services/cache/cache.reducer.ts new file mode 100644 index 0000000000..59b567e939 --- /dev/null +++ b/src/app/core/data-services/cache/cache.reducer.ts @@ -0,0 +1,58 @@ +import { CacheAction, CacheActionTypes, AddToCacheAction, RemoveFromCacheAction } from "./cache.actions"; +import { hasValue } from "../../../shared/empty.util"; + +export interface CacheableObject { + uuid: string; +} + +export interface CacheEntry { + data: CacheableObject; + timeAdded: number; + msToLive: number; +} + +export interface CacheState { + [uuid: string]: CacheEntry +} + +// Object.create(null) ensures the object has no default js properties (e.g. `__proto__`) +const initialState: CacheState = Object.create(null); + +export const cacheReducer = (state = initialState, action: CacheAction): CacheState => { + switch (action.type) { + + case CacheActionTypes.ADD: { + return addToCache(state, action); + } + + case CacheActionTypes.REMOVE: { + return removeFromCache(state, action) + } + + default: { + return state; + } + } +}; + +function addToCache(state: CacheState, action: AddToCacheAction): CacheState { + return Object.assign({}, state, { + [action.payload.objectToCache.uuid]: { + data: action.payload.objectToCache, + timeAdded: new Date().getTime(), + msToLive: action.payload.msToLive + } + }); +} + +function removeFromCache(state: CacheState, action: RemoveFromCacheAction): CacheState { + if (hasValue(state[action.payload])) { + let newCache = Object.assign({}, state); + delete newCache[action.payload]; + + return newCache; + } + else { + return state; + } +} diff --git a/src/app/core/data-services/cache/cache.service.ts b/src/app/core/data-services/cache/cache.service.ts new file mode 100644 index 0000000000..d8c73ec1cb --- /dev/null +++ b/src/app/core/data-services/cache/cache.service.ts @@ -0,0 +1,58 @@ +import { Injectable } from "@angular/core"; +import { Store } from "@ngrx/store"; +import { CacheState, CacheEntry, CacheableObject } from "./cache.reducer"; +import { AddToCacheAction, RemoveFromCacheAction } from "./cache.actions"; +import { Observable } from "rxjs"; +import { hasNoValue } from "../../../shared/empty.util"; + +@Injectable() +export class CacheService { + constructor( + private store: Store + ) {} + + add(objectToCache: CacheableObject, msToLive: number): void { + this.store.dispatch(new AddToCacheAction(objectToCache, msToLive)); + } + + remove(uuid: string): void { + this.store.dispatch(new RemoveFromCacheAction(uuid)); + } + + get(uuid: string): Observable { + return this.store.select('core', 'cache', uuid) + .filter(entry => this.isValid(entry)) + .map((entry: CacheEntry) => entry.data); + } + + getList(uuids: Array): Observable> { + return Observable.combineLatest( + uuids.map((id: string) => this.get(id)) + ); + } + + has(uuid: string): boolean { + let result: boolean; + + this.store.select('core', 'cache', uuid) + .take(1) + .subscribe(entry => result = this.isValid(entry)); + + return result; + } + + private isValid(entry: CacheEntry): boolean { + if (hasNoValue(entry)) { + return false; + } + else { + const timeOutdated = entry.timeAdded + entry.msToLive; + const isOutDated = new Date().getTime() > timeOutdated; + if (isOutDated) { + this.store.dispatch(new RemoveFromCacheAction(entry.data.uuid)); + } + return !isOutDated; + } + } + +} diff --git a/src/app/core/data-services/collection/collection-data.effects.ts b/src/app/core/data-services/collection/collection-data.effects.ts index 7a5fdd4f6b..192b2e48b6 100644 --- a/src/app/core/data-services/collection/collection-data.effects.ts +++ b/src/app/core/data-services/collection/collection-data.effects.ts @@ -1,5 +1,5 @@ import { Injectable } from "@angular/core"; -import { Actions, Effect } from "@ngrx/effects"; +import { Actions, Effect, toPayload } from "@ngrx/effects"; import { Collection } from "../../shared/collection.model"; import { Observable } from "rxjs"; import { @@ -15,33 +15,52 @@ import { import { DSpaceRESTV2Response } from "../../dspace-rest-v2/dspace-rest-v2-response.model"; import { DSpaceRESTv2Serializer } from "../../dspace-rest-v2/dspace-rest-v2.serializer"; import { DSpaceRESTv2Service } from "../../dspace-rest-v2/dspace-rest-v2.service"; +import { CacheService } from "../cache/cache.service"; +import { GlobalConfig } from "../../../../config"; @Injectable() export class CollectionDataEffects { constructor( private actions$: Actions, - private restApiService: DSpaceRESTv2Service + private restApi: DSpaceRESTv2Service, + private cache: CacheService ) {} + // TODO, results of a findall aren't retrieved from cache for now, + // because currently the cache is more of an object store. We need to move + // more towards memoization for things like this. @Effect() findAll$ = this.actions$ .ofType(CollectionFindMultipleActionTypes.FIND_MULTI_REQUEST) .switchMap(() => { - return this.restApiService.get('/collections') + return this.restApi.get('/collections') .map((data: DSpaceRESTV2Response) => new DSpaceRESTv2Serializer(Collection).deserializeArray(data)) - .map((collections: Collection[]) => new CollectionFindMultipleSuccessAction(collections)) + .do((collections: Collection[]) => { + collections.forEach((collection) => { + this.cache.add(collection, GlobalConfig.cache.msToLive); + }); + }) + .map((collections: Array) => collections.map(collection => collection.id)) + .map((ids: Array) => new CollectionFindMultipleSuccessAction(ids)) .catch((errorMsg: string) => Observable.of(new CollectionFindMultipleErrorAction(errorMsg))); }); @Effect() findById$ = this.actions$ .ofType(CollectionFindSingleActionTypes.FIND_BY_ID_REQUEST) .switchMap(action => { - return this.restApiService.get(`/collections/${action.payload}`) - .map((data: DSpaceRESTV2Response) => { - const t = new DSpaceRESTv2Serializer(Collection).deserialize(data); - return t; - }) - .map((collection: Collection) => new CollectionFindByIdSuccessAction(collection)) - .catch((errorMsg: string) => Observable.of(new CollectionFindByIdErrorAction(errorMsg))); + if (this.cache.has(action.payload)) { + return this.cache.get(action.payload) + .map(collection => new CollectionFindByIdSuccessAction(collection.id)); + } + else { + return this.restApi.get(`/collections/${action.payload}`) + .map((data: DSpaceRESTV2Response) => new DSpaceRESTv2Serializer(Collection).deserialize(data)) + .do((collection: Collection) => { + this.cache.add(collection, GlobalConfig.cache.msToLive); + }) + .map((collection: Collection) => new CollectionFindByIdSuccessAction(collection.id)) + .catch((errorMsg: string) => Observable.of(new CollectionFindByIdErrorAction(errorMsg))); + } }); + } diff --git a/src/app/core/data-services/collection/collection-data.service.ts b/src/app/core/data-services/collection/collection-data.service.ts index b774b5cd0d..2468f8ad09 100644 --- a/src/app/core/data-services/collection/collection-data.service.ts +++ b/src/app/core/data-services/collection/collection-data.service.ts @@ -5,26 +5,29 @@ import { Store } from "@ngrx/store"; import { Collection } from "../../shared/collection.model"; import { CollectionFindMultipleRequestAction } from "./collection-find-multiple.actions"; import { CollectionFindByIdRequestAction } from "./collection-find-single.actions"; -import { isNotEmpty } from "../../../shared/empty.util"; -import 'rxjs/add/operator/filter'; +import { CacheService } from "../cache/cache.service"; +import 'rxjs/add/observable/forkJoin'; @Injectable() export class CollectionDataService { constructor( - private store: Store + private store: Store, + private cache: CacheService ) { } findAll(scope?: Collection): Observable { this.store.dispatch(new CollectionFindMultipleRequestAction(scope)); - return this.store.select('core', 'collectionData', 'findMultiple', 'collections'); + //get an observable of the IDs from the collectionData store + return this.store.select>('core', 'collectionData', 'findMultiple', 'collectionsIDs') + .flatMap((collectionIds: Array) => { + // use those IDs to fetch the actual collection objects from the cache + return this.cache.getList(collectionIds); + }); } findById(id: string): Observable { this.store.dispatch(new CollectionFindByIdRequestAction(id)); - return this.store.select('core', 'collectionData', 'findSingle', 'collection') - //this filter is necessary because the same collection - //object in the state is used for every findById call - .filter(collection => isNotEmpty(collection) && collection.id === id); + return this.cache.get(id); } } diff --git a/src/app/core/data-services/collection/collection-find-multiple.actions.ts b/src/app/core/data-services/collection/collection-find-multiple.actions.ts index 95009602ec..a68f7e2478 100644 --- a/src/app/core/data-services/collection/collection-find-multiple.actions.ts +++ b/src/app/core/data-services/collection/collection-find-multiple.actions.ts @@ -33,10 +33,10 @@ export class CollectionFindMultipleRequestAction implements Action { export class CollectionFindMultipleSuccessAction implements Action { type = CollectionFindMultipleActionTypes.FIND_MULTI_SUCCESS; - payload: Collection[]; + payload: Array; - constructor(collections: Collection[]) { - this.payload = collections; + constructor(collectionIDs: Array) { + this.payload = collectionIDs; } } diff --git a/src/app/core/data-services/collection/collection-find-multiple.reducer.ts b/src/app/core/data-services/collection/collection-find-multiple.reducer.ts index 9b4934317c..463155d0f5 100644 --- a/src/app/core/data-services/collection/collection-find-multiple.reducer.ts +++ b/src/app/core/data-services/collection/collection-find-multiple.reducer.ts @@ -8,7 +8,7 @@ import { export interface CollectionFindMultipleState { scope: Collection; - collections: Collection[]; + collectionsIDs: Array; isLoading: boolean; errorMessage: string; paginationOptions: PaginationOptions; @@ -17,7 +17,7 @@ export interface CollectionFindMultipleState { const initialState: CollectionFindMultipleState = { scope: undefined, - collections: [], + collectionsIDs: [], isLoading: false, errorMessage: undefined, paginationOptions: undefined, @@ -30,7 +30,7 @@ export const findMultipleReducer = (state = initialState, action: CollectionFind case CollectionFindMultipleActionTypes.FIND_MULTI_REQUEST: { return Object.assign({}, state, { scope: action.payload.scope, - collections: [], + collectionsIDs: [], isLoading: true, errorMessage: undefined, paginationOptions: action.payload.paginationOptions, @@ -41,7 +41,7 @@ export const findMultipleReducer = (state = initialState, action: CollectionFind case CollectionFindMultipleActionTypes.FIND_MULTI_SUCCESS: { return Object.assign({}, state, { isLoading: false, - collections: action.payload, + collectionsIDs: action.payload, errorMessage: undefined }); } diff --git a/src/app/core/data-services/collection/collection-find-single.actions.ts b/src/app/core/data-services/collection/collection-find-single.actions.ts index 37ccc8c328..92335cd49e 100644 --- a/src/app/core/data-services/collection/collection-find-single.actions.ts +++ b/src/app/core/data-services/collection/collection-find-single.actions.ts @@ -19,10 +19,10 @@ export class CollectionFindByIdRequestAction implements Action { export class CollectionFindByIdSuccessAction implements Action { type = CollectionFindSingleActionTypes.FIND_BY_ID_SUCCESS; - payload: Collection; + payload: string; - constructor(collection: Collection) { - this.payload = collection; + constructor(collectionID: string) { + this.payload = collectionID; } } diff --git a/src/app/core/data-services/collection/collection-find-single.reducer.ts b/src/app/core/data-services/collection/collection-find-single.reducer.ts index 0fc0bf65cf..720efdbca1 100644 --- a/src/app/core/data-services/collection/collection-find-single.reducer.ts +++ b/src/app/core/data-services/collection/collection-find-single.reducer.ts @@ -5,17 +5,15 @@ import { } from "./collection-find-single.actions"; export interface CollectionFindSingleState { - collection: Collection; isLoading: boolean; errorMessage: string; - id: string; + collectionID: string; } const initialState: CollectionFindSingleState = { - collection: undefined, isLoading: false, errorMessage: undefined, - id: undefined, + collectionID: undefined }; export const findSingleReducer = (state = initialState, action: CollectionFindSingleAction): CollectionFindSingleState => { @@ -24,17 +22,15 @@ export const findSingleReducer = (state = initialState, action: CollectionFindSi case CollectionFindSingleActionTypes.FIND_BY_ID_REQUEST: { return Object.assign({}, state, { isLoading: true, - id: action.payload, - collections: undefined, errorMessage: undefined, + collectionID: action.payload }); } case CollectionFindSingleActionTypes.FIND_BY_ID_SUCCESS: { return Object.assign({}, state, { isLoading: false, - collection: action.payload, - errorMessage: undefined + errorMessage: undefined, }); } diff --git a/src/app/core/shared/dspace-object.model.ts b/src/app/core/shared/dspace-object.model.ts index ecb449e709..9f48b7ca45 100644 --- a/src/app/core/shared/dspace-object.model.ts +++ b/src/app/core/shared/dspace-object.model.ts @@ -1,11 +1,12 @@ import { autoserialize, autoserializeAs } from "cerialize"; import { Metadatum } from "./metadatum.model" import { isEmpty, isNotEmpty } from "../../shared/empty.util"; +import { CacheableObject } from "../data-services/cache/cache.reducer"; /** * An abstract model class for a DSpaceObject. */ -export abstract class DSpaceObject { +export abstract class DSpaceObject implements CacheableObject { /** * The identifier of this DSpaceObject @@ -64,4 +65,12 @@ export abstract class DSpaceObject { return undefined; } } + + get uuid(): string { + return this.id; + } + + set uuid(val: string) { + this.id = val; + } } diff --git a/src/app/shared/cache.service.ts b/src/app/shared/demo-cache.service.ts similarity index 95% rename from src/app/shared/cache.service.ts rename to src/app/shared/demo-cache.service.ts index 1268c90410..3bdc5ecb59 100644 --- a/src/app/shared/cache.service.ts +++ b/src/app/shared/demo-cache.service.ts @@ -1,8 +1,8 @@ import { Inject, Injectable, isDevMode } from '@angular/core'; @Injectable() -export class CacheService { - static KEY = 'CacheService'; +export class DemoCacheService { + static KEY = 'DemoCacheService'; constructor( @Inject('LRU') public _cache: Map) { @@ -71,7 +71,7 @@ export class CacheService { */ normalizeKey(key: string | number): string { if (isDevMode() && this._isInvalidValue(key)) { - throw new Error('Please provide a valid key to save in the CacheService'); + throw new Error('Please provide a valid key to save in the DemoCacheService'); } return key + ''; diff --git a/src/app/shared/model/model.service.ts b/src/app/shared/model/model.service.ts index 251a31542f..734f931894 100644 --- a/src/app/shared/model/model.service.ts +++ b/src/app/shared/model/model.service.ts @@ -4,7 +4,7 @@ import 'rxjs/add/observable/of'; import 'rxjs/add/operator/do'; import 'rxjs/add/operator/share'; -import { CacheService } from '../cache.service'; +import { DemoCacheService } from '../demo-cache.service'; import { ApiService } from '../api.service'; export function hashCodeString(str: string): string { @@ -24,7 +24,7 @@ export function hashCodeString(str: string): string { @Injectable() export class ModelService { // This is only one example of one Model depending on your domain - constructor(public _api: ApiService, public _cache: CacheService) { + constructor(public _api: ApiService, public _cache: DemoCacheService) { } diff --git a/src/backend/api.ts b/src/backend/api.ts index edf7cc3f20..613388dd57 100644 --- a/src/backend/api.ts +++ b/src/backend/api.ts @@ -92,7 +92,7 @@ export function createMockApi() { router.route('/collections/:collection_id') .get(function(req, res) { - console.log('GET', util.inspect(req.collection, { colors: true })); + console.log('GET', util.inspect(req.collection.id, { colors: true })); res.json(toHALResponse(req, req.collection)); // }) // .put(function(req, res) { diff --git a/src/browser.module.ts b/src/browser.module.ts index 27a39e9b40..5ef364fea6 100755 --- a/src/browser.module.ts +++ b/src/browser.module.ts @@ -10,7 +10,7 @@ import { TranslateLoader, TranslateModule, TranslateStaticLoader } from 'ng2-tra import { AppModule, AppComponent } from './app/app.module'; import { SharedModule } from './app/shared/shared.module'; -import { CacheService } from './app/shared/cache.service'; +import { DemoCacheService } from './app/shared/demo-cache.service'; import { CoreModule } from "./app/core/core.module"; // Will be merged into @angular/platform-browser in a later release @@ -70,7 +70,7 @@ export const UNIVERSAL_KEY = 'UNIVERSAL_CACHE'; { provide: 'LRU', useFactory: getLRU, deps: [] }, - CacheService, + DemoCacheService, Meta, @@ -78,14 +78,14 @@ export const UNIVERSAL_KEY = 'UNIVERSAL_CACHE'; ] }) export class MainModule { - constructor(public cache: CacheService) { + constructor(public cache: DemoCacheService) { // TODO(gdi2290): refactor into a lifecycle hook this.doRehydrate(); } doRehydrate() { let defaultValue = {}; - let serverCache = this._getCacheValue(CacheService.KEY, defaultValue); + let serverCache = this._getCacheValue(DemoCacheService.KEY, defaultValue); this.cache.rehydrate(serverCache); } diff --git a/src/node.module.ts b/src/node.module.ts index 1dc29d164c..6a554e0e8c 100755 --- a/src/node.module.ts +++ b/src/node.module.ts @@ -9,7 +9,7 @@ import { TranslateLoader, TranslateModule, TranslateStaticLoader } from 'ng2-tra import { AppModule, AppComponent } from './app/app.module'; import { SharedModule } from './app/shared/shared.module'; -import { CacheService } from './app/shared/cache.service'; +import { DemoCacheService } from './app/shared/demo-cache.service'; import { CoreModule } from "./app/core/core.module"; // Will be merged into @angular/platform-browser in a later release @@ -61,13 +61,13 @@ export const UNIVERSAL_KEY = 'UNIVERSAL_CACHE'; { provide: 'LRU', useFactory: getLRU, deps: [] }, - CacheService, + DemoCacheService, Meta, ] }) export class MainModule { - constructor(public cache: CacheService) { + constructor(public cache: DemoCacheService) { } @@ -76,7 +76,7 @@ export class MainModule { * in Universal for now until it's fixed */ universalDoDehydrate = (universalCache) => { - universalCache[CacheService.KEY] = JSON.stringify(this.cache.dehydrate()); + universalCache[DemoCacheService.KEY] = JSON.stringify(this.cache.dehydrate()); } /**