feat(core): remove redundant read/writes to flow files (#4715)

* refactor scoped forBot

* reorder

* fix

* array cache

* avoid updating cache multiple times

* fix services being intialized twice

* diag use createApp

* use correct typing

* add todo

* avoid responding to own file changes

* make renaming work

* remove outdated todo

* fix double start

* throw exceptions

* move arraycache to its own file

* remove todo

* remove other todo

* todo

* handle nlu

* fix

* dont go below 0

* added unit tests for array-cache

* remove ghost changes

* fix

* todos

* fix

* fix delete

Co-authored-by: Laurent Leclerc-Poulin <laurentleclercpoulin@gmail.com>

src/bp/common/array-cache.test.ts

@@ -0,0 +1,254 @@
+import _ from 'lodash'
+import { ArrayCache } from './array-cache'
+
+const RANDOM = () => Math.random().toString(36)
+
+class TestObject {
+  constructor(public someKey: string, public someOtherKey: string, public aNumber: number) {}
+
+  static default(): TestObject {
+    return new TestObject(RANDOM(), RANDOM(), Math.random())
+  }
+
+  setSomeKey(val: string) {
+    this.someKey = val
+  }
+}
+
+const getKeySpy = jest.fn()
+const getKey = (val: TestObject) => {
+  getKeySpy()
+  return val.someKey
+}
+const renameValSpy = jest.fn()
+const renameVal = (value: TestObject, prevKey: string, newKey: string) => {
+  renameValSpy()
+  value.setSomeKey(newKey)
+
+  return value
+}
+
+const createObjects = (number: number) => {
+  const objects: TestObject[] = []
+  for (let i = 0; i < number; i++) {
+    objects.push(TestObject.default())
+  }
+
+  return objects
+}
+
+const numberOfObjects = 10
+const emptyArray = new Array()
+
+let cache: ArrayCache<string, TestObject>
+let objects: TestObject[]
+
+describe('ArrayCache', () => {
+  beforeEach(() => {
+    cache = new ArrayCache(getKey, renameVal)
+    objects = createObjects(numberOfObjects)
+
+    jest.resetAllMocks()
+  })
+
+  describe('Initialize', () => {
+    it('Set the inner array of the cache with the given values', () => {
+      cache.initialize(_.cloneDeep(objects))
+
+      const innerArray = cache['array']
+
+      expect(innerArray.length).toEqual(numberOfObjects)
+      expect(innerArray.length).toEqual(cache.values().length)
+      expect(_.isEqual(innerArray, objects)).toEqual(true)
+    })
+
+    it('Sets an empty cache', () => {
+      cache.initialize(emptyArray)
+
+      const innerArray = cache['array']
+
+      expect(innerArray.length).toEqual(0)
+      expect(innerArray.length).toEqual(cache.values().length)
+      expect(_.isEqual(innerArray, emptyArray)).toEqual(true)
+    })
+
+    it('Overrides the current cache values', () => {
+      cache.initialize(objects)
+
+      expect(cache.values().length).toEqual(numberOfObjects)
+
+      cache.initialize(emptyArray)
+
+      const innerArray = cache['array']
+
+      expect(innerArray.length).toEqual(0)
+      expect(innerArray.length).toEqual(cache.values().length)
+      expect(_.isEqual(innerArray, emptyArray)).toEqual(true)
+    })
+  })
+
+  describe('Values', () => {
+    it('Returns the inner array of the cache', () => {
+      cache.initialize(_.cloneDeep(objects))
+
+      expect(cache.values().length).toEqual(numberOfObjects)
+      expect(_.isEqual(cache.values(), objects)).toEqual(true)
+    })
+
+    it('Returns an empty cache', () => {
+      cache.initialize(emptyArray)
+
+      expect(cache.values().length).toEqual(0)
+      expect(_.isEqual(cache.values(), emptyArray)).toEqual(true)
+    })
+  })
+
+  describe('Reset', () => {
+    it('Clear the cache', () => {
+      cache.initialize(objects)
+      cache.reset()
+
+      const innerArray = cache['array']
+
+      expect(innerArray.length).toEqual(0)
+      expect(innerArray.length).toEqual(cache.values().length)
+      expect(_.isEqual(innerArray, objects)).toEqual(false)
+    })
+  })
+
+  describe('Get', () => {
+    it('Returns the corresponding value', () => {
+      cache.initialize(objects)
+
+      const selectedObject = _.cloneDeep(objects[0])
+      const returnedObject = cache.get(selectedObject.someKey)
+
+      expect(getKeySpy).toHaveBeenCalled()
+      expect(returnedObject).not.toBeUndefined()
+      expect(_.isEqual(selectedObject, returnedObject)).toEqual(true)
+    })
+
+    it('Returns no value when the key does not exist in the cache', () => {
+      cache.initialize(objects)
+
+      const returnedObject = cache.get(RANDOM())
+
+      expect(getKeySpy).toHaveBeenCalledTimes(numberOfObjects)
+      expect(returnedObject).toBeUndefined()
+    })
+  })
+
+  describe('Update', () => {
+    it('Updates the value in the cache', () => {
+      cache.initialize(objects)
+
+      const selectedObject = _.cloneDeep(objects[0])
+      selectedObject.aNumber = Math.random()
+      selectedObject.someOtherKey = RANDOM()
+
+      let returnedObject = cache.get(selectedObject.someKey)
+
+      expect(returnedObject).not.toBeUndefined()
+      expect(_.isEqual(selectedObject, returnedObject)).toEqual(false)
+
+      cache.update(selectedObject.someKey, selectedObject)
+      returnedObject = cache.get(selectedObject.someKey)
+
+      expect(returnedObject).not.toBeUndefined()
+      expect(_.isEqual(selectedObject, returnedObject)).toEqual(true)
+    })
+
+    it('Inserts the value when the key does not exist in the cache', () => {
+      cache.initialize(objects)
+
+      const newObject = createObjects(1)[0]
+      let returnedObject = cache.get(newObject.someKey)
+      expect(returnedObject).toBeUndefined()
+
+      cache.update(newObject.someKey, newObject)
+
+      returnedObject = cache.get(newObject.someKey)
+      expect(returnedObject).not.toBeUndefined()
+      expect(_.isEqual(newObject, returnedObject)).toEqual(true)
+    })
+  })
+
+  describe('Rename', () => {
+    it('Renames the key in the cache', () => {
+      cache.initialize(objects)
+
+      const selectedObject = _.cloneDeep(objects[0])
+      const newKey = RANDOM()
+      const oldKey = selectedObject.someKey
+
+      cache.rename(selectedObject.someKey, newKey)
+      const returnedObject = cache.get(newKey)
+      const oldObject = cache.get(oldKey)
+
+      expect(renameValSpy).toHaveBeenCalledTimes(1)
+      expect(returnedObject).not.toBeUndefined()
+      expect(oldObject).toBeUndefined()
+      expect(getKey(returnedObject!)).toEqual(newKey)
+    })
+
+    it('Throws an error when the key does not exist in the cache', () => {
+      cache.initialize(objects)
+
+      const someKey = RANDOM()
+      const returnedObject = cache.get(someKey)
+
+      expect(renameValSpy).toHaveBeenCalledTimes(0)
+      expect(returnedObject).toBeUndefined()
+      expect(() => cache.rename(someKey, RANDOM())).toThrow()
+    })
+  })
+
+  describe('Remove', () => {
+    it('Removes the value from the cache', () => {
+      cache.initialize(objects)
+
+      const selectedObject = _.cloneDeep(objects[0])
+
+      cache.remove(selectedObject.someKey)
+      const returnedObject = cache.get(selectedObject.someKey)
+
+      expect(returnedObject).toBeUndefined()
+      expect(cache.values().length).toEqual(numberOfObjects - 1)
+    })
+
+    it('Throws an error when the key does not exist in the cache', () => {
+      cache.initialize(objects)
+
+      const someKey = RANDOM()
+      const returnedObject = cache.get(someKey)
+
+      expect(returnedObject).toBeUndefined()
+      expect(() => cache.remove(someKey)).toThrow()
+    })
+  })
+
+  describe('indexOf', () => {
+    it('Returns the index of the key in the cache', () => {
+      cache.initialize(objects)
+
+      const index = 0
+      const selectedObject = objects[index]
+
+      const returnedIndex = cache['indexOf'](selectedObject.someKey)
+
+      expect(index).toEqual(returnedIndex)
+    })
+
+    it('Returns a negative index if the key is not found in the cache', () => {
+      cache.initialize(objects)
+
+      const someKey = RANDOM()
+
+      const returnedObject = cache.get(someKey)
+      const returnedIndex = cache['indexOf'](someKey)
+
+      expect(returnedObject).toBeUndefined()
+      expect(returnedIndex).toBeLessThan(0)
+    })
+  })
+})

src/bp/common/array-cache.ts

@@ -0,0 +1,89 @@
+export class ArrayCache<K, V> {
+  private array: V[] = []
+
+  /**
+   * Array cache
+   * @param getKey Function allowing to specify which key to use when fetching a value from the inner cache
+   * @param renameVal Function called when a key is renamed
+   */
+  constructor(private getKey: (value: V) => K, private renameVal: (value: V, prevKey: K, newKey: K) => V) {}
+
+  /**
+   * Returns all the content of the cache
+   * @returns The content of the cache
+   */
+  values() {
+    return this.array
+  }
+
+  /**
+   * Initializes the cache with some values
+   * @param values Some values of type V to initialize the cache with
+   */
+  initialize(values: V[]) {
+    this.array = values
+  }
+
+  /**
+   * Clears the cache of all its content
+   */
+  reset() {
+    this.array = []
+  }
+
+  /**
+   * Returns the value in the cache associated with the given key
+   * @param key The key used to retrieve the object
+   * @returns Returns an object of type V or undefined when the is no hit.
+   */
+  get(key: K) {
+    return this.array.find(x => this.getKey(x) === key)
+  }
+
+  /**
+   * Updates the value of a cached object.
+   * **Will insert the object if it is not found in the cache.**
+   * @param key The key associated with the object to update
+   * @param value The new value to put in cache
+   */
+  update(key: K, value: V) {
+    const index = this.indexOf(key)
+    if (index >= 0) {
+      this.array[index] = value
+    } else {
+      this.array.push(value)
+    }
+  }
+
+  /**
+   * Renames the key of a cache object
+   * @param prevKey The old key
+   * @param newKey The new key
+   * @throws an Error if the key is not found
+   */
+  rename(prevKey: K, newKey: K) {
+    const index = this.indexOf(prevKey)
+    if (index >= 0) {
+      this.array[index] = this.renameVal(this.array[index], prevKey, newKey)
+    } else {
+      throw new Error('Cannot rename a key that does not exist')
+    }
+  }
+
+  /**
+   * Removes an object from the cache
+   * @param key The key used to retrieve the object
+   */
+  remove(key: K) {
+    const index = this.indexOf(key)
+    if (index >= 0) {
+      this.array.splice(index, 1)
+    } else {
+      throw new Error('Cannot remove a key that does not exist')
+    }
+  }
+
+  private indexOf(key: K) {
+    return this.array.findIndex(x => this.getKey(x) === key)
+  }
+}