diff --git a/docs/package.json b/docs/package.json index de8b5a8..8da5885 100644 --- a/docs/package.json +++ b/docs/package.json @@ -23,8 +23,6 @@ "nuclear-js": "^1.0.5", "webpack": "^1.9.11", "webpack-dev-server": "^1.9.0", - "grunt-concurrent": "^1.0.0", - "grunt-contrib-connect": "^0.10.1", "remarkable": "^1.6.0", "front-matter": "^1.0.0", "glob": "^5.0.10", diff --git a/src/console-polyfill.js b/src/console-polyfill.js index 124dae8..33c531b 100644 --- a/src/console-polyfill.js +++ b/src/console-polyfill.js @@ -1,11 +1,11 @@ try { - if (!(window.console && console.log)) { + if (!(window.console && console.log)) { // eslint-disable-line console = { - log: function(){}, - debug: function(){}, - info: function(){}, - warn: function(){}, - error: function(){} - }; + log: function() {}, + debug: function() {}, + info: function() {}, + warn: function() {}, + error: function() {}, + } } -} catch(e) {} +} catch(e) {} // eslint-disable-line diff --git a/src/create-react-mixin.js b/src/create-react-mixin.js index 9195f72..3965451 100644 --- a/src/create-react-mixin.js +++ b/src/create-react-mixin.js @@ -26,7 +26,7 @@ export default function(reactor) { each(this.getDataBindings(), (getter, key) => { const unwatchFn = reactor.observe(getter, (val) => { this.setState({ - [key]: val + [key]: val, }) }) diff --git a/src/getter.js b/src/getter.js index aece455..4123675 100644 --- a/src/getter.js +++ b/src/getter.js @@ -2,6 +2,8 @@ import Immutable, { List } from 'immutable' import { isFunction, isArray } from './utils' import { isKeyPath } from './key-path' +const CACHE_OPTIONS = ['default', 'always', 'never'] + /** * Getter helper functions * A getter is an array with the form: @@ -9,6 +11,41 @@ import { isKeyPath } from './key-path' */ const identity = (x) => x +/** + * Add override options to a getter + * @param {getter} getter + * @param {object} options + * @param {boolean} options.cache + * @param {*} options.cacheKey + * @returns {getter} + */ +function Getter(getter, options={}) { + if (!isKeyPath(getter) && !isGetter(getter)) { + throw new Error('createGetter must be passed a keyPath or Getter') + } + + if (getter.hasOwnProperty('__options')) { + throw new Error('Cannot reassign options to getter') + } + + getter.__options = {} + getter.__options.cache = CACHE_OPTIONS.indexOf(options.cache) > -1 ? options.cache : 'default' + getter.__options.cacheKey = options.cacheKey !== undefined ? options.cacheKey : null + return getter +} + +/** + * Retrieve an option from getter options + * @param {getter} getter + * @param {string} Name of option to retrieve + * @returns {*} + */ +function getGetterOption(getter, option) { + if (getter.__options) { + return getter.__options[option] + } +} + /** * Checks if something is a getter literal, ex: ['dep1', 'dep2', function(dep1, dep2) {...}] * @param {*} toTest @@ -106,7 +143,9 @@ export default { isGetter, getComputeFn, getFlattenedDeps, + getGetterOption, getStoreDeps, getDeps, fromKeyPath, + Getter, } diff --git a/src/main.js b/src/main.js index 3e0ebc1..784f5f7 100644 --- a/src/main.js +++ b/src/main.js @@ -4,7 +4,7 @@ import Reactor from './reactor' import Immutable from 'immutable' import { toJS, toImmutable, isImmutable } from './immutable-helpers' import { isKeyPath } from './key-path' -import { isGetter } from './getter' +import { isGetter, Getter } from './getter' import createReactMixin from './create-react-mixin' export default { @@ -17,4 +17,5 @@ export default { toImmutable, isImmutable, createReactMixin, + Getter, } diff --git a/src/reactor.js b/src/reactor.js index ce46ecb..759528c 100644 --- a/src/reactor.js +++ b/src/reactor.js @@ -1,6 +1,7 @@ import Immutable from 'immutable' import createReactMixin from './create-react-mixin' import * as fns from './reactor/fns' +import { BasicCache, LRUCache } from './reactor/cache' import { isKeyPath } from './key-path' import { isGetter } from './getter' import { toJS } from './immutable-helpers' @@ -24,11 +25,33 @@ import { class Reactor { constructor(config = {}) { const debug = !!config.debug + const useCache = config.useCache === undefined ? true : !!config.useCache + + let Cache = LRUCache + + if (config.cache !== undefined) { + Cache = config.cache + } else if (config.maxItemsToCache !== undefined) { + const maxItemsToCache = Number(config.maxItemsToCache) + if (maxItemsToCache > 0) { + Cache = LRUCache.bind(LRUCache, maxItemsToCache) + } else { + Cache = BasicCache + } + } + + const cacheFactory = () => { + return new Cache() + } + const baseOptions = debug ? DEBUG_OPTIONS : PROD_OPTIONS const initialReactorState = new ReactorState({ debug: debug, + cacheFactory: cacheFactory, + cache: cacheFactory(), // merge config options with the defaults options: baseOptions.merge(config.options || {}), + useCache: useCache, }) this.prevReactorState = initialReactorState @@ -88,7 +111,9 @@ class Reactor { let { observerState, entry } = fns.addObserver(this.observerState, getter, handler) this.observerState = observerState return () => { - this.observerState = fns.removeObserverByEntry(this.observerState, entry) + let { observerState, reactorState } = fns.removeObserverByEntry(this.observerState, this.reactorState, entry) + this.observerState = observerState + this.reactorState = reactorState } } @@ -100,7 +125,9 @@ class Reactor { throw new Error('Must call unobserve with a Getter') } - this.observerState = fns.removeObserver(this.observerState, getter, handler) + const { observerState, reactorState } = fns.removeObserver(this.observerState, this.reactorState, getter, handler) + this.observerState = observerState + this.reactorState = reactorState } /** diff --git a/src/reactor/cache.js b/src/reactor/cache.js new file mode 100644 index 0000000..d9a2b73 --- /dev/null +++ b/src/reactor/cache.js @@ -0,0 +1,220 @@ +import { Map, OrderedSet, Record } from 'immutable' + +export const CacheEntry = Record({ + value: null, + storeStates: Map(), + dispatchId: null, +}) + +/******************************************************************************* + * interface PersistentCache { + * has(item) + * lookup(item, notFoundValue) + * hit(item) + * miss(item, entry) + * evict(item) + * asMap() + * } + * + * Inspired by clojure.core.cache/CacheProtocol + *******************************************************************************/ + +/** + * Plain map-based cache + */ +export class BasicCache { + + /** + * @param {Immutable.Map} cache + */ + constructor(cache = Map()) { + this.cache = cache + } + + /** + * Retrieve the associated value, if it exists in this cache, otherwise + * returns notFoundValue (or undefined if not provided) + * @param {Object} item + * @param {Object?} notFoundValue + * @return {CacheEntry?} + */ + lookup(item, notFoundValue) { + return this.cache.get(item, notFoundValue) + } + + /** + * Checks if this cache contains an associated value + * @param {Object} item + * @return {boolean} + */ + has(item) { + return this.cache.has(item) + } + + /** + * Return cached items as map + * @return {Immutable.Map} + */ + asMap() { + return this.cache + } + + /** + * Updates this cache when it is determined to contain the associated value + * @param {Object} item + * @return {BasicCache} + */ + hit(item) { + return this + } + + /** + * Updates this cache when it is determined to **not** contain the associated value + * @param {Object} item + * @param {CacheEntry} entry + * @return {BasicCache} + */ + miss(item, entry) { + return new BasicCache( + this.cache.update(item, existingEntry => { + if (existingEntry && existingEntry.dispatchId > entry.dispatchId) { + throw new Error('Refusing to cache older value') + } + return entry + }) + ) + } + + /** + * Removes entry from cache + * @param {Object} item + * @return {BasicCache} + */ + evict(item) { + return new BasicCache(this.cache.remove(item)) + } +} + +export const DEFAULT_LRU_LIMIT = 1000 +export const DEFAULT_LRU_EVICT_COUNT = 1 + +/** + * Implements caching strategy that evicts least-recently-used items in cache + * when an item is being added to a cache that has reached a configured size + * limit. + */ +export class LRUCache { + + constructor(limit = DEFAULT_LRU_LIMIT, evictCount = DEFAULT_LRU_EVICT_COUNT, cache = new BasicCache(), lru = OrderedSet()) { + this.limit = limit + this.evictCount = evictCount + this.cache = cache + this.lru = lru + } + + /** + * Retrieve the associated value, if it exists in this cache, otherwise + * returns notFoundValue (or undefined if not provided) + * @param {Object} item + * @param {Object?} notFoundValue + * @return {CacheEntry} + */ + lookup(item, notFoundValue) { + return this.cache.lookup(item, notFoundValue) + } + + /** + * Checks if this cache contains an associated value + * @param {Object} item + * @return {boolean} + */ + has(item) { + return this.cache.has(item) + } + + /** + * Return cached items as map + * @return {Immutable.Map} + */ + asMap() { + return this.cache.asMap() + } + + /** + * Updates this cache when it is determined to contain the associated value + * @param {Object} item + * @return {LRUCache} + */ + hit(item) { + if (!this.cache.has(item)) { + return this + } + + // remove it first to reorder in lru OrderedSet + return new LRUCache(this.limit, this.evictCount, this.cache, this.lru.remove(item).add(item)) + } + + /** + * Updates this cache when it is determined to **not** contain the associated value + * If cache has reached size limit, the LRU item is evicted. + * @param {Object} item + * @param {CacheEntry} entry + * @return {LRUCache} + */ + miss(item, entry) { + if (this.lru.size >= this.limit) { + if (this.has(item)) { + return new LRUCache( + this.limit, + this.evictCount, + this.cache.miss(item, entry), + this.lru.remove(item).add(item) + ) + } + + const cache = (this.lru + .take(this.evictCount) + .reduce((c, evictItem) => c.evict(evictItem), this.cache) + .miss(item, entry)) + + return new LRUCache( + this.limit, + this.evictCount, + cache, + this.lru.skip(this.evictCount).add(item) + ) + } + return new LRUCache( + this.limit, + this.evictCount, + this.cache.miss(item, entry), + this.lru.add(item) + ) + } + + /** + * Removes entry from cache + * @param {Object} item + * @return {LRUCache} + */ + evict(item) { + if (!this.cache.has(item)) { + return this + } + + return new LRUCache( + this.limit, + this.evictCount, + this.cache.evict(item), + this.lru.remove(item) + ) + } +} + +/** + * Returns default cache strategy + * @return {BasicCache} + */ +export function DefaultCache() { + return new LRUCache() +} diff --git a/src/reactor/fns.js b/src/reactor/fns.js index 7f47527..d8c4919 100644 --- a/src/reactor/fns.js +++ b/src/reactor/fns.js @@ -1,8 +1,9 @@ import Immutable from 'immutable' import logging from '../logging' +import { CacheEntry } from './cache' import { isImmutableValue } from '../immutable-helpers' import { toImmutable } from '../immutable-helpers' -import { fromKeyPath, getStoreDeps, getComputeFn, getDeps, isGetter } from '../getter' +import { fromKeyPath, getStoreDeps, getComputeFn, getDeps, isGetter, getGetterOption } from '../getter' import { isEqual, isKeyPath } from '../key-path' import { each } from '../utils' @@ -74,7 +75,7 @@ export function replaceStores(reactorState, stores) { */ export function dispatch(reactorState, actionType, payload) { if (actionType === undefined && getOption(reactorState, 'throwOnUndefinedActionType')) { - throw new Error('`dispatch` cannot be called with an `undefined` action type.'); + throw new Error('`dispatch` cannot be called with an `undefined` action type.') } const currState = reactorState.get('state') @@ -168,6 +169,7 @@ export function loadState(reactorState, state) { export function addObserver(observerState, getter, handler) { // use the passed in getter as the key so we can rely on a byreference call for unobserve const getterKey = getter + if (isKeyPath(getter)) { getter = fromKeyPath(getter) } @@ -223,25 +225,57 @@ export function getOption(reactorState, option) { /** * Use cases - * removeObserver(observerState, []) - * removeObserver(observerState, [], handler) - * removeObserver(observerState, ['keyPath']) - * removeObserver(observerState, ['keyPath'], handler) - * removeObserver(observerState, getter) - * removeObserver(observerState, getter, handler) + * removeObserver(observerState, reactorState, []) + * removeObserver(observerState, reactorState, [], handler) + * removeObserver(observerState, reactorState, ['keyPath']) + * removeObserver(observerState, reactorState, ['keyPath'], handler) + * removeObserver(observerState, reactorState, getter) + * removeObserver(observerState, reactorState, getter, handler) * @param {ObserverState} observerState + * @param {ReactorState} reactorState * @param {KeyPath|Getter} getter * @param {Function} handler - * @return {ObserverState} + * @return {Array} */ -export function removeObserver(observerState, getter, handler) { - const entriesToRemove = observerState.get('observersMap').filter(entry => { +export function removeObserver(observerState, reactorState, getter, handler) { + let entriesToRemove = getAllObserversForGetter(observerState, getter) + const originalEntriesToRemoveCount = entriesToRemove.size + if (handler) { + entriesToRemove = entriesToRemove.filter(entry => { + return entry.get('handler') === handler + }) + } + + // If a handler was specified, only clear cache if ALL entries for a getter have that handler + const shouldClearCache = originalEntriesToRemoveCount === entriesToRemove.size + + // Update both observer and reactor state + observerState = observerState.withMutations(oState => { + reactorState = reactorState.withMutations(rState => { + + entriesToRemove.forEach(entry => { removeObserverByEntry(oState, rState, entry, false) }) + if (shouldClearCache) { + removeCacheValue(rState, getter) + } + }) + }) + return { + observerState, + reactorState, + } +} + +/** + * Retrieve all observer entries that match a given getter + * @param {ObserverState} observerState + * @param {Getter} getter + * @returns {Immutable.Map} + */ +export function getAllObserversForGetter(observerState, getter) { + return observerState.get('observersMap').filter(entry => { // use the getterKey in the case of a keyPath is transformed to a getter in addObserver - let entryGetter = entry.get('getterKey') - let handlersMatch = (!handler || entry.get('handler') === handler) - if (!handlersMatch) { - return false - } + const entryGetter = entry.get('getterKey') + // check for a by-value equality of keypaths if (isKeyPath(getter) && isKeyPath(entryGetter)) { return isEqual(getter, entryGetter) @@ -249,20 +283,23 @@ export function removeObserver(observerState, getter, handler) { // we are comparing two getters do it by reference return (getter === entryGetter) }) - - return observerState.withMutations(map => { - entriesToRemove.forEach(entry => removeObserverByEntry(map, entry)) - }) } /** * Removes an observer entry by id from the observerState * @param {ObserverState} observerState + * @param {ReactorState} reactorState * @param {Immutable.Map} entry - * @return {ObserverState} + * @return {Array} */ -export function removeObserverByEntry(observerState, entry) { - return observerState.withMutations(map => { +export function removeObserverByEntry(observerState, reactorState, entry, attemptToClearCache=true) { + + // Only clear cache values for a getter if there is only one entry for it + if (attemptToClearCache && getAllObserversForGetter(observerState, entry.get('getterKey')).size === 1) { + reactorState = removeCacheValue(reactorState, entry.get('getterKey')) + } + + observerState = observerState.withMutations(map => { const id = entry.get('id') const storeDeps = entry.get('storeDeps') @@ -282,6 +319,11 @@ export function removeObserverByEntry(observerState, entry) { map.removeIn(['observersMap', id]) }) + + return { + observerState, + reactorState, + } } /** @@ -308,6 +350,8 @@ export function reset(reactorState) { reactorState.update('storeStates', storeStates => incrementStoreStates(storeStates, storeIds)) resetDirtyStores(reactorState) + + reactorState.set('cache', reactorState.cacheFactory()) }) } @@ -330,23 +374,16 @@ export function evaluate(reactorState, keyPathOrGetter) { } // Must be a Getter - // if the value is cached for this dispatch cycle, return the cached value - if (isCached(reactorState, keyPathOrGetter)) { - // Cache hit - return evaluateResult( - getCachedValue(reactorState, keyPathOrGetter), - reactorState - ) + var cacheEntry = reactorState.cache.lookup(getCacheKey(keyPathOrGetter)) + const isCacheMiss = !cacheEntry || isDirtyCacheEntry(reactorState, cacheEntry) + if (isCacheMiss) { + cacheEntry = createCacheEntry(reactorState, keyPathOrGetter) } - - // evaluate dependencies - const args = getDeps(keyPathOrGetter).map(dep => evaluate(reactorState, dep).result) - const evaluatedValue = getComputeFn(keyPathOrGetter).apply(null, args) - return evaluateResult( - evaluatedValue, - cacheValue(reactorState, keyPathOrGetter, evaluatedValue) + cacheEntry.get('value'), + cacheValue(reactorState, keyPathOrGetter, cacheEntry, isCacheMiss) ) + } /** @@ -381,51 +418,69 @@ export function resetDirtyStores(reactorState) { * @return {Getter} */ function getCacheKey(getter) { - return getter + const getterOption = getGetterOption(getter, 'cacheKey') + return getterOption ? getterOption : getter } /** * @param {ReactorState} reactorState - * @param {Getter|KeyPath} keyPathOrGetter - * @return {Immutable.Map} + * @param {CacheEntry} cacheEntry + * @return {boolean} */ -function getCacheEntry(reactorState, keyPathOrGetter) { - const key = getCacheKey(keyPathOrGetter) - return reactorState.getIn(['cache', key]) +function isDirtyCacheEntry(reactorState, cacheEntry) { + const storeStates = cacheEntry.get('storeStates') + + // if there are no store states for this entry then it was never cached before + return !storeStates.size || storeStates.some((stateId, storeId) => { + return reactorState.getIn(['storeStates', storeId]) !== stateId + }) } /** + * Evaluates getter for given reactorState and returns CacheEntry * @param {ReactorState} reactorState - * @param {Getter} getter - * @return {Boolean} + * @param {getter} getter + * @param {cacheEntry} CacheEntry + * @param {boolean} isCacheMiss + * @return {ReactorState} */ -function isCached(reactorState, keyPathOrGetter) { - const entry = getCacheEntry(reactorState, keyPathOrGetter) - if (!entry) { - return false +function cacheValue(reactorState, getter, cacheEntry, isCacheMiss) { + + // Check global cache settings + const globalCacheEnabled = !!reactorState.get('useCache') + let useCache = globalCacheEnabled + + // Check cache settings on a getter basis + const getterCacheOption = getGetterOption(getter, 'cache') + if (getterCacheOption === 'always') { + useCache = true + } else if (getterCacheOption === 'never') { + useCache = false } - const storeStates = entry.get('storeStates') - if (storeStates.size === 0) { - // if there are no store states for this entry then it was never cached before - return false + if (!useCache) { + return reactorState } - return storeStates.every((stateId, storeId) => { - return reactorState.getIn(['storeStates', storeId]) === stateId + const cacheKey = getCacheKey(getter) + return reactorState.update('cache', cache => { + return isCacheMiss ? + cache.miss(cacheKey, cacheEntry) : + cache.hit(cacheKey) }) } /** - * Caches the value of a getter given state, getter, args, value + * Evaluates getter for given reactorState and returns CacheEntry * @param {ReactorState} reactorState * @param {Getter} getter - * @param {*} value - * @return {ReactorState} + * @return {CacheEntry} */ -function cacheValue(reactorState, getter, value) { - const cacheKey = getCacheKey(getter) - const dispatchId = reactorState.get('dispatchId') +function createCacheEntry(reactorState, getter) { + // evaluate dependencies + const args = getDeps(getter).map(dep => evaluate(reactorState, dep).result) + const value = getComputeFn(getter).apply(null, args) + const storeDeps = getStoreDeps(getter) const storeStates = toImmutable({}).withMutations(map => { storeDeps.forEach(storeId => { @@ -434,19 +489,24 @@ function cacheValue(reactorState, getter, value) { }) }) - return reactorState.setIn(['cache', cacheKey], Immutable.Map({ + return CacheEntry({ value: value, storeStates: storeStates, - dispatchId: dispatchId, - })) + dispatchId: reactorState.get('dispatchId'), + }) } /** - * Pulls out the cached value for a getter + * Remove getter from cache + * @param {ReactorState} reactorState + * @param {getter} getter + * @return {ReactorState} */ -function getCachedValue(reactorState, getter) { - const key = getCacheKey(getter) - return reactorState.getIn(['cache', key, 'value']) +function removeCacheValue(reactorState, getter) { + const cacheKey = getCacheKey(getter) + return reactorState.update('cache', (cache) => { + return cache.evict(cacheKey) + }) } /** @@ -457,7 +517,6 @@ function incrementId(reactorState) { return reactorState.update('dispatchId', id => id + 1) } - /** * @param {Immutable.Map} storeStates * @param {Array} storeIds diff --git a/src/reactor/records.js b/src/reactor/records.js index a59314f..49ddc2a 100644 --- a/src/reactor/records.js +++ b/src/reactor/records.js @@ -1,4 +1,5 @@ import { Map, Set, Record } from 'immutable' +import { DefaultCache } from './cache' export const PROD_OPTIONS = Map({ // logs information for each dispatch @@ -38,13 +39,15 @@ export const ReactorState = Record({ dispatchId: 0, state: Map(), stores: Map(), - cache: Map(), + cache: DefaultCache(), + cacheFactory: DefaultCache, // maintains a mapping of storeId => state id (monotomically increasing integer whenever store state changes) storeStates: Map(), dirtyStores: Set(), debug: false, // production defaults options: PROD_OPTIONS, + useCache: true, }) export const ObserverState = Record({ diff --git a/tests/cache-tests.js b/tests/cache-tests.js new file mode 100644 index 0000000..01ccbc8 --- /dev/null +++ b/tests/cache-tests.js @@ -0,0 +1,137 @@ +import Immutable, { Map } from 'immutable' +import { LRUCache } from '../src/reactor/cache' + +describe('Cache', () => { + describe('LRUCache', () => { + var cache + + beforeEach(() => { + jasmine.addCustomEqualityTester(Immutable.is) + }) + + it('should evict least recently used', () => { + cache = new LRUCache(3) + + expect(cache.asMap().isEmpty()).toBe(true) + + var a = {foo: 'bar'} + var b = {bar: 'baz'} + var c = {baz: 'foo'} + + cache = cache.miss('a', a) + + expect(cache.asMap()).toEqual(Map({a: a})) + expect(cache.has('a')).toBe(true) + expect(cache.lookup('a')).toBe(a) + + cache = cache.miss('b', b) + expect(cache.asMap()).toEqual(Map({a: a, b: b})) + expect(cache.has('b')).toBe(true) + expect(cache.lookup('b')).toBe(b) + + cache = cache.miss('c', c) + expect(cache.asMap()).toEqual(Map({a: a, b: b, c: c})) + expect(cache.has('c')).toBe(true) + expect(cache.lookup('c')).toBe(c) + + expect(cache.has('d')).toBe(false) + expect(cache.lookup('d')).not.toBeDefined() + + var notFound = {found: false} + expect(cache.lookup('d', notFound)).toBe(notFound) + + cache = cache.miss('d', 4) + + expect(cache.asMap()).toEqual(Map({b: b, c: c, d: 4}), 'a should have been evicted') + + cache = cache.hit('b') // Touch b so its not LRU + + expect(cache.asMap()).toEqual(Map({b: b, c: c, d: 4}), 'should not have have changed') + + cache = cache.miss('e', 5) + + expect(cache.asMap()).toEqual(Map({b: b, d: 4, e: 5}), 'should have changed') + + cache = cache.hit('b') + .hit('e') + .hit('d') + .miss('a', 1) + + expect(cache.asMap()).toEqual(Map({a: 1, d: 4, e: 5}), 'should have changed') + }) + + it('should maintain LRU after manual evict', () => { + cache = new LRUCache(3) + .miss('a', 'A') + .miss('b', 'B') + .miss('c', 'C') + + expect(cache.asMap()).toEqual(Map({a: 'A', b: 'B', c: 'C'})) + + cache = cache.evict('a') + expect(cache.asMap()).toEqual(Map({b: 'B', c: 'C'})) + + cache = cache.miss('d', 'D') + expect(cache.asMap()).toEqual(Map({b: 'B', c: 'C', d: 'D'})) + }) + + it('should not evict if under limit', () => { + cache = new LRUCache(2) + .miss('a', 1) + .miss('b', 2) + .miss('b', 3) + + expect(cache.asMap()).toEqual(Map({a: 1, b: 3})) + cache = cache.miss('a', 4) + expect(cache.asMap()).toEqual(Map({a: 4, b: 3})) + + cache = new LRUCache(3) + .miss('a', 1) + .miss('b', 2) + .miss('b', 3) + .miss('c', 4) + .miss('d', 5) + .miss('e', 6) + + expect(cache.asMap()).toEqual(Map({e: 6, d: 5, c: 4})) + }) + + it('should not evict if hitting unknown items', () => { + cache = new LRUCache(2) + .hit('x') + .hit('y') + .hit('z') + .miss('a', 1) + .miss('b', 2) + .miss('c', 3) + .miss('d', 4) + .miss('e', 5) + + expect(cache.asMap()).toEqual(Map({d: 4, e: 5})) + + cache = new LRUCache(2) + .hit('x') + .hit('y') + .miss('a', 1) + .miss('b', 2) + .miss('y', 3) + .miss('x', 4) + .miss('e', 5) + + expect(cache.asMap()).toEqual(Map({x: 4, e: 5})) + }) + + it('should be able to evict multiple LRU items at once', () => { + cache = new LRUCache(4, 2) + .miss('a', 1) + .miss('b', 2) + .miss('c', 3) + .miss('d', 4) + .miss('e', 5) + + expect(cache.asMap()).toEqual(Map({c: 3, d: 4, e: 5})) + expect(cache.miss('f', 6).asMap()).toEqual(Map({c: 3, d: 4, e: 5, f: 6})) + expect(cache.miss('f', 6).miss('g', 7).asMap()).toEqual(Map({e: 5, f: 6, g: 7})) + }) + }) +}) diff --git a/tests/getter-tests.js b/tests/getter-tests.js index 93d89f6..aedd8b7 100644 --- a/tests/getter-tests.js +++ b/tests/getter-tests.js @@ -1,4 +1,4 @@ -import { isGetter, getFlattenedDeps, fromKeyPath } from '../src/getter' +import { isGetter, getFlattenedDeps, fromKeyPath, getGetterOption, Getter } from '../src/getter' import { Set, List, is } from 'immutable' describe('Getter', () => { @@ -32,6 +32,58 @@ describe('Getter', () => { }) }) + describe('#getGetterOption', () => { + it('should return undefined if options are not set, or an unrecognized option is requested', () => { + expect(getGetterOption(['test'], 'cache')).toBe(undefined) + expect(getGetterOption(Getter(['test'], { cache: 'always'}), 'fakeOption')).toBe(undefined) + }) + it('should return the value of the requested option', () => { + expect(getGetterOption(Getter(['test'], { cache: 'always'}), 'cache')).toBe('always') + }) + }) + + describe('#Getter', () => { + it('should throw an error if not passed a getter', () => { + expect(() => { Getter(false) }).toThrow() + }) + + it('should accept a keyPath as a getter argument', () => { + const keyPath = ['test'] + expect(Getter(keyPath)).toBe(keyPath) + }) + + it('should accept a getter as a getter argument', () => { + const getter = ['test', () => 1] + expect(Getter(getter)).toBe(getter) + }) + + + it('should use "default" as the default cache option', () => { + const getterObject = Getter(['test'], {}) + expect(getGetterOption(getterObject, 'cache')).toBe('default') + const getterObject1 = Getter(['test'], { cache: 'fakeOption' }) + expect(getGetterOption(getterObject1, 'cache')).toBe('default') + }) + + it('should set "always" and "never" as cache options', () => { + const getterObject = Getter(['test'], { cache: 'never' }) + expect(getGetterOption(getterObject, 'cache')).toBe('never') + const getterObject1 = Getter(['test'], { cache: 'always' }) + expect(getGetterOption(getterObject1, 'cache')).toBe('always') + }) + + it('should default cacheKey to null', () => { + const getterObject = Getter(['test'], {}) + expect(getGetterOption(getterObject, 'cacheKey')).toBe(null) + }) + + it('should set cacheKey to supplied value', () => { + const getter = ['test'] + const getterObject = Getter(getter, { cacheKey: 'test' }) + expect(getGetterOption(getterObject, 'cacheKey')).toBe('test') + }) + }) + describe('getFlattenedDeps', function() { describe('when passed the identity getter', () => { it('should return a set with only an empty list', () => { diff --git a/tests/reactor-fns-tests.js b/tests/reactor-fns-tests.js index dfdd4c3..6a4072a 100644 --- a/tests/reactor-fns-tests.js +++ b/tests/reactor-fns-tests.js @@ -2,8 +2,9 @@ import { Map, Set, is } from 'immutable' import { Store } from '../src/main' import * as fns from '../src/reactor/fns' -import { ReactorState, ObserverState, DEBUG_OPTIONS } from '../src/reactor/records' +import { ReactorState, ObserverState } from '../src/reactor/records' import { toImmutable } from '../src/immutable-helpers' +import { Getter } from '../src/getter' describe('reactor fns', () => { describe('#registerStores', () => { @@ -355,6 +356,11 @@ describe('reactor fns', () => { }) expect(is(expected, result)).toBe(true) }) + + it('should clear cache', () => { + const resultCache = nextReactorState.cache.asMap() + expect(resultCache.toJS()).toEqual({}) + }) }) describe('#addObserver', () => { @@ -471,7 +477,7 @@ describe('reactor fns', () => { }) describe('#removeObserver', () => { - let initialObserverState, nextObserverState, getter1, getter2, handler1, handler2, handler3 + let initialObserverState, initialReactorState, getter1, getter2, handler1, handler2, handler3 beforeEach(() => { handler1 = () => 1 @@ -483,7 +489,15 @@ describe('reactor fns', () => { ['store2'], (a, b) => a + b ] - getter2 = [[], x => x] + getter2 = Getter([[], x => x]) + + initialReactorState = new ReactorState() + initialReactorState = initialReactorState.update('cache', cache => { + return cache.miss(getter1, 'test1') + }) + initialReactorState = initialReactorState.update('cache', cache => { + return cache.miss(getter2, 'test2') + }) const initialObserverState1 = new ObserverState() const result1 = fns.addObserver(initialObserverState1, getter1, handler1) @@ -496,8 +510,9 @@ describe('reactor fns', () => { describe('when removing by getter', () => { it('should return a new ObserverState with all entries containing the getter removed', () => { - nextObserverState = fns.removeObserver(initialObserverState, getter1) - const expected = Map({ + expect(initialReactorState.cache.lookup(getter1)).toBe('test1') + let { observerState, reactorState } = fns.removeObserver(initialObserverState, initialReactorState, getter1) + const expectedObserverState = Map({ any: Set.of(3), stores: Map({ store1: Set(), @@ -514,14 +529,15 @@ describe('reactor fns', () => { })] ]) }) - const result = nextObserverState - expect(is(expected, result)).toBe(true) + expect(is(expectedObserverState, observerState)).toBe(true) + expect(reactorState.cache.lookup(getter1)).toBe(undefined) + expect(reactorState.cache.lookup(getter2)).toBe('test2') }) }) describe('when removing by getter / handler', () => { it('should return a new ObserverState with all entries containing the getter removed', () => { - nextObserverState = fns.removeObserver(initialObserverState, getter2, handler3) + let { observerState, reactorState } = fns.removeObserver(initialObserverState, initialReactorState, getter2, handler3) const expected = Map({ any: Set(), stores: Map({ @@ -546,11 +562,60 @@ describe('reactor fns', () => { })] ]) }) - const result = nextObserverState - expect(is(expected, result)).toBe(true) + expect(is(expected, observerState)).toBe(true) + expect(reactorState.cache.lookup(getter2)).toBe(undefined) + expect(reactorState.cache.lookup(getter1)).toBe('test1') + }) + it('should not clear cache if more there are multiple handlers for the same getter', () => { + let { reactorState } = fns.removeObserver(initialObserverState, initialReactorState, getter1, handler2) + expect(reactorState.cache.lookup(getter2)).toBe('test2') + expect(reactorState.cache.lookup(getter1)).toBe('test1') }) }) }) + + + describe('#removeObserverByEntry', () => { + let initialObserverState, initialReactorState, getter1, handler1, handler2, entry1, entry2 + + beforeEach(() => { + handler1 = () => 1 + handler2 = () => 2 + + getter1 = [ + ['store1'], + ['store2'], + (a, b) => a + b + ] + + initialReactorState = new ReactorState() + initialReactorState = initialReactorState.update('cache', cache => { + return cache.miss(getter1, 'test1') + }) + + const initialObserverState1 = new ObserverState() + const result1 = fns.addObserver(initialObserverState1, getter1, handler1) + const initialObserverState2 = result1.observerState + const result2 = fns.addObserver(initialObserverState2, getter1, handler2) + initialObserverState = result2.observerState + entry1 = result1.entry + entry2 = result2.entry + }) + + it('should should not clear cache if there is more than one entry associated with a getter', () => { + expect(initialReactorState.cache.lookup(getter1)).toBe('test1') + let { reactorState } = fns.removeObserverByEntry(initialObserverState, initialReactorState, entry1, true) + expect(reactorState.cache.lookup(getter1)).toBe('test1') + }) + + it('should should clear cache if there is only one entry associated with a getter', () => { + expect(initialReactorState.cache.lookup(getter1)).toBe('test1') + let { observerState, reactorState } = fns.removeObserverByEntry(initialObserverState, initialReactorState, entry1, true) + let { reactorState: reactorState2 } = fns.removeObserverByEntry(observerState, reactorState, entry2, true) + expect(reactorState2.cache.lookup(getter1)).toBe(undefined) + }) + }) + describe('#getDebugOption', () => { it('should parse the option value in a reactorState', () => { const reactorState = new ReactorState({ @@ -571,7 +636,7 @@ describe('reactor fns', () => { }) expect(function() { - const result = fns.getOption(reactorState, 'unknownOption') + fns.getOption(reactorState, 'unknownOption') }).toThrow() }) }) diff --git a/tests/reactor-tests.js b/tests/reactor-tests.js index bf095db..8a5f3e8 100644 --- a/tests/reactor-tests.js +++ b/tests/reactor-tests.js @@ -4,6 +4,8 @@ import { getOption } from '../src/reactor/fns' import { toImmutable } from '../src/immutable-helpers' import { PROD_OPTIONS, DEBUG_OPTIONS } from '../src/reactor/records' import logging from '../src/logging' +import { Getter } from '../src/getter' +import { DEFAULT_LRU_LIMIT, DEFAULT_LRU_EVICT_COUNT, BasicCache, LRUCache } from '../src/reactor/cache' describe('Reactor', () => { it('should construct without \'new\'', () => { @@ -11,11 +13,13 @@ describe('Reactor', () => { expect(reactor instanceof Reactor).toBe(true) }) - describe('debug and options flags', () => { + describe('debug, cache, and options flags', () => { it('should create a reactor with PROD_OPTIONS', () => { var reactor = new Reactor() expect(reactor.reactorState.get('debug')).toBe(false) expect(is(reactor.reactorState.get('options'), PROD_OPTIONS)).toBe(true) + expect(reactor.reactorState.cache instanceof LRUCache).toBe(true) + expect(reactor.reactorState.cache.limit).toBe(DEFAULT_LRU_LIMIT) }) it('should create a reactor with DEBUG_OPTIONS', () => { var reactor = new Reactor({ @@ -23,12 +27,14 @@ describe('Reactor', () => { }) expect(reactor.reactorState.get('debug')).toBe(true) expect(is(reactor.reactorState.get('options'), DEBUG_OPTIONS)).toBe(true) + expect(reactor.reactorState.cache instanceof LRUCache).toBe(true) + expect(reactor.reactorState.cache.limit).toBe(DEFAULT_LRU_LIMIT) }) it('should override PROD options', () => { var reactor = new Reactor({ options: { logDispatches: true, - } + }, }) expect(getOption(reactor.reactorState, 'logDispatches')).toBe(true) expect(getOption(reactor.reactorState, 'logAppState')).toBe(false) @@ -44,7 +50,7 @@ describe('Reactor', () => { options: { logDispatches: false, throwOnDispatchInDispatch: false, - } + }, }) expect(getOption(reactor.reactorState, 'logDispatches')).toBe(false) expect(getOption(reactor.reactorState, 'logAppState')).toBe(true) @@ -54,6 +60,35 @@ describe('Reactor', () => { expect(getOption(reactor.reactorState, 'throwOnNonImmutableStore')).toBe(true) expect(getOption(reactor.reactorState, 'throwOnDispatchInDispatch')).toBe(false) }) + it('should create a reactor that overrides maxItemsToCache', () => { + var reactor = new Reactor({ + maxItemsToCache: 20, + }) + expect(reactor.reactorState.cache instanceof LRUCache).toBe(true) + expect(reactor.reactorState.cache.limit).toBe(20) + reactor = new Reactor({ + maxItemsToCache: -20, + }) + expect(reactor.reactorState.cache instanceof BasicCache).toBe(true) + reactor = new Reactor({ + maxItemsToCache: 'abc', + }) + expect(reactor.reactorState.cache instanceof BasicCache).toBe(true) + }) + it('should override caching if set globally', () => { + let reactor = new Reactor({}) + expect(reactor.reactorState.get('useCache')).toBe(true) + reactor = new Reactor({ + useCache: false, + }) + expect(reactor.reactorState.get('useCache')).toBe(false) + }) + it('should allow users to pass in a cache constructor', () => { + let reactor = new Reactor({ + cache: BasicCache, + }) + expect(reactor.reactorState.cache instanceof BasicCache).toBe(true) + }) }) describe('options', () => { @@ -100,7 +135,7 @@ describe('Reactor', () => { initialize() { this.on('action', () => undefined) }, - }) + }), }) reactor.dispatch('action') reactor.reset() @@ -123,7 +158,7 @@ describe('Reactor', () => { initialize() { this.on('action', () => undefined) }, - }) + }), }) }).toThrow() }) @@ -144,7 +179,7 @@ describe('Reactor', () => { initialize() { this.on('action', () => undefined) }, - }) + }), }) }).toThrow() }) @@ -164,8 +199,8 @@ describe('Reactor', () => { }, handleReset() { return undefined - } - }) + }, + }), }) reactor.reset() }).toThrow() @@ -189,7 +224,7 @@ describe('Reactor', () => { handleReset() { return { foo: 'baz' } }, - }) + }), }) reactor.reset() }).not.toThrow() @@ -208,7 +243,7 @@ describe('Reactor', () => { getInitialState() { return { foo: 'bar' } }, - }) + }), }) }).toThrow() }) @@ -229,7 +264,7 @@ describe('Reactor', () => { handleReset() { return { foo: 'baz' } }, - }) + }), }) reactor.reset() }).toThrow() @@ -252,8 +287,8 @@ describe('Reactor', () => { }, initialize() { this.on('increment', curr => curr + 1) - } - }) + }, + }), }) reactor.observe(['count'], (val) => { @@ -281,8 +316,8 @@ describe('Reactor', () => { }, initialize() { this.on('increment', curr => curr + 1) - } - }) + }, + }), }) reactor.observe(['count'], (val) => { @@ -595,6 +630,120 @@ describe('Reactor', () => { expect(taxPercentSpy.calls.count()).toEqual(2) expect(subtotalSpy.calls.count()).toEqual(1) }) + + it('should not cache if useCache is set to false', () => { + let cacheReactor = new Reactor({ + useCache: true, + }) + cacheReactor.evaluate([['test'], () => 1]) + expect(cacheReactor.reactorState.cache.asMap().size).toBe(1) + cacheReactor = new Reactor({ + useCache: false, + }) + cacheReactor.evaluate([['test'], () => 1]) + expect(cacheReactor.reactorState.cache.asMap().size).toBe(0) + }) + + it('should respect individual getter cache settings over global settings', () => { + let cacheReactor = new Reactor({ + useCache: true, + }) + + let getter = Getter([['test2'], () => 1], { cache: 'never' }) + cacheReactor.evaluate(getter) + expect(cacheReactor.reactorState.cache.asMap().size).toBe(0) + + cacheReactor = new Reactor({ + useCache: false, + }) + getter = Getter([['test2'], () => 1], { cache: 'always' }) + cacheReactor.evaluate(getter) + expect(cacheReactor.reactorState.cache.asMap().size).toBe(1) + }) + + it('should use cache key supplied by getter if it is present in options', () => { + let cacheReactor = new Reactor({ + useCache: true, + }) + + let getter = Getter([['test'], () => 1], { cacheKey: 'test' }) + cacheReactor.evaluate(getter) + expect(Object.keys(cacheReactor.reactorState.cache.asMap().toJS())).toEqual(['test']) + }) + + it('should not cache keyPaths', () => { + let cacheReactor = new Reactor({ + useCache: true, + }) + let getter = ['test'] + cacheReactor.evaluate(getter) + expect(Object.keys(cacheReactor.reactorState.cache.asMap().toJS())).toEqual([]) + getter = Getter(['test']) + cacheReactor.evaluate(getter) + expect(Object.keys(cacheReactor.reactorState.cache.asMap().toJS())).toEqual([]) + }) + + describe('lru caching', () => { + var getters = Array.apply(null, new Array(100)).map(() => { + return [['price'], () => 1] + }) + + var cacheReactor + const maxItemsToCache = 50 + + beforeEach(() => { + cacheReactor = new Reactor({ + maxItemsToCache: maxItemsToCache, + }) + }) + + it('should add getters to the lru set', () => { + for (let x = 0; x < 50; x++) { + cacheReactor.evaluate(getters[x]) + } + expect(cacheReactor.reactorState.cache.lru.size).toBe(50) + expect(cacheReactor.reactorState.cache.lru.first()).toBe(getters[0]) + expect(cacheReactor.reactorState.cache.lru.last()).toBe(getters[49]) + }) + + it('should prioritize the lru cache by when getters were added', () => { + for (let x = 0; x < 10; x++) { + cacheReactor.evaluate(getters[x]) + } + cacheReactor.evaluate(getters[0]) + expect(cacheReactor.reactorState.cache.lru.size).toBe(10) + expect(cacheReactor.reactorState.cache.lru.last()).toBe(getters[0]) + expect(cacheReactor.reactorState.cache.lru.first()).toBe(getters[1]) + }) + + it('should clear the appropriate number of items from cache when the maxItemsToCache limit is exceeded', () => { + for (let x = 0; x <= maxItemsToCache + 1; x++) { + cacheReactor.evaluate(getters[x]) + } + expect(cacheReactor.reactorState.cache.lru.size).toBe(cacheReactor.reactorState.cache.asMap().size) + expect(cacheReactor.reactorState.cache.lru.size).toBe(maxItemsToCache + 1 - DEFAULT_LRU_EVICT_COUNT) + expect(cacheReactor.reactorState.cache.lookup(getters[0])).toBe(undefined) + expect(cacheReactor.reactorState.cache.lookup(getters[maxItemsToCache + 1])).toBeTruthy() + }) + }) + + it('should update cache with updated item after action', () => { + const lastItemGetter = [['items', 'all'], (items) => items.last()] + + // ensure its in cache + const lastItemBefore = reactor.evaluate(lastItemGetter) + const cacheEntryBefore = reactor.reactorState.cache.lookup(lastItemGetter) + expect(lastItemBefore === cacheEntryBefore.value).toBe(true) + + checkoutActions.addItem('potato', 0.80) + + const lastItemAfter = reactor.evaluate(lastItemGetter) + const cacheEntryAfter = reactor.reactorState.cache.lookup(lastItemGetter) + expect(lastItemAfter === cacheEntryAfter.value).toBe(true) + + // sanity check that lastItem actually changed for completeness + expect(lastItemAfter !== lastItemBefore).toBe(true) + }) }) describe('#observe', () => { @@ -607,6 +756,15 @@ describe('Reactor', () => { expect(mockFn.calls.count()).toEqual(1) expect(mockFn.calls.argsFor(0)).toEqual([5]) }) + it('should observe a Getter object', () => { + var mockFn = jasmine.createSpy() + reactor.observe(Getter(['taxPercent']), mockFn) + + checkoutActions.setTaxPercent(5) + + expect(mockFn.calls.count()).toEqual(1) + expect(mockFn.calls.argsFor(0)).toEqual([5]) + }) it('should not invoke a change handler if another keyPath changes', () => { var mockFn = jasmine.createSpy() reactor.observe(['taxPercent'], mockFn) @@ -695,8 +853,8 @@ describe('Reactor', () => { test: Store({ getInitialState() { return 1 - } - }) + }, + }), }) expect(mockFn.calls.count()).toEqual(1) @@ -717,24 +875,24 @@ describe('Reactor', () => { }, initialize() { this.on('increment', (state) => state + 1) - } - }) + }, + }), }) // it should call the observer after the store has been registered expect(mockFn.calls.count()).toEqual(1) var observedValue = mockFn.calls.argsFor(0)[0] var expectedHandlerValue = Map({ - test: 1 + test: 1, }) expect(is(observedValue, expectedHandlerValue)).toBe(true) // it should call the observer again when the store handles an action reactor.dispatch('increment') expect(mockFn.calls.count()).toEqual(2) - var observedValue = mockFn.calls.argsFor(1)[0] - var expectedHandlerValue = Map({ - test: 2 + observedValue = mockFn.calls.argsFor(1)[0] + expectedHandlerValue = Map({ + test: 2, }) expect(is(observedValue, expectedHandlerValue)).toBe(true) }) @@ -828,14 +986,12 @@ describe('Reactor', () => { var mockFn1 = jasmine.createSpy() var mockFn2 = jasmine.createSpy() var unwatchFn2 - var unwatchFn1 = reactor.observe(subtotalGetter, (val) => { - console.log('hanlder1', unwatchFn2) + reactor.observe(subtotalGetter, (val) => { unwatchFn2() mockFn1(val) }) unwatchFn2 = reactor.observe(subtotalGetter, (val) => { - console.log('handler2') mockFn2(val) }) @@ -1777,7 +1933,6 @@ describe('Reactor', () => { describe('#replaceStores', () => { let counter1Store let counter2Store - let counter1StoreReplaced let reactor beforeEach(() => { @@ -1786,13 +1941,13 @@ describe('Reactor', () => { getInitialState: () => 1, initialize() { this.on('increment1', (state) => state + 1) - } + }, }) counter2Store = new Store({ getInitialState: () => 1, initialize() { this.on('increment2', (state) => state + 1) - } + }, }) reactor.registerStores({ @@ -1806,7 +1961,7 @@ describe('Reactor', () => { getInitialState: () => 1, initialize() { this.on('increment1', (state) => state + 10) - } + }, }) expect(reactor.evaluate(['counter1'])).toBe(1) @@ -1828,13 +1983,13 @@ describe('Reactor', () => { getInitialState: () => 1, initialize() { this.on('increment1', (state) => state + 10) - } + }, }) let newStore2 = new Store({ getInitialState: () => 1, initialize() { this.on('increment2', (state) => state + 20) - } + }, }) expect(reactor.evaluate(['counter1'])).toBe(1)