Fixup return values and add more tests

Author: Tom Kirkpatrick

lib/index.js

@@ -17,6 +17,19 @@ module.exports = function loopbackComponentFsm(app) {
     return Model ? Model.__FSM_CONFIG__ : null
   }
 
+  function getModelName(instance) {
+    return instance.constructor.definition.name
+  }
+
+  function generateCacheKey(instance) {
+    if (!instance.id) {
+      return null
+    }
+    const modelName = getModelName(instance)
+
+    return `loopback-component-fsm.${modelName}.id:${instance.id}`
+  }
+
   /**
    * Fetch cached state machine for an instance.
    */
@@ -25,14 +38,14 @@ module.exports = function loopbackComponentFsm(app) {
       return null
     }
 
-    const modelName = instance.constructor.definition.name
-    const cacheKey = `loopback-component-fsm.${modelName}.id:${instance.id}`
+    const modelName = getModelName(instance)
+    const cacheKey = generateCacheKey(instance)
     let cache = _.get(app.locals, cacheKey)
 
     debug('Fetching cached state machine found for subscription %s (cache key=%s)', instance.id, cacheKey)
 
     if (cache) {
-      debug('Cached state machine found for subscription %s', instance.id, cache)
+      debug('Cached state machine found for subscription %s', instance.id)
       return cache
     }
 
@@ -45,10 +58,35 @@ module.exports = function loopbackComponentFsm(app) {
 
     config = Object.assign(config, { initial: instance.status })
     cache = new StateMachine(config)
+    debug('Caching state machine subscription %s: %o', instance.id, cache)
     _.set(app.locals, cacheKey, cache)
 
     return cache
   }
 
+
+  /**
+   * Fetch cached state machine for an instance.
+   */
+  function deleteStateMachine(instance) {
+    if (!instance.id) {
+      return null
+    }
+
+    const cacheKey = generateCacheKey(instance)
+
+    debug('Deleting cached state machine for subscription %s (cache key=%s)', instance.id, cacheKey)
+
+    if (!_.get(app.locals, cacheKey)) {
+      debug('Existing cached state machine not found for subscription %s (cache key=%s)', instance.id, cacheKey)
+      return false
+    }
+
+    _.unset(app.locals, cacheKey)
+    debug('Deleted cached state machine for subscription %s (cache key=%s)', instance.id, cacheKey)
+    return true
+  }
+
   app.getStateMachine = getStateMachine
+  app.deleteStateMachine = deleteStateMachine
 }

lib/mixins/state-machine.js

@@ -16,6 +16,7 @@ module.exports = function StateMachine(Model, settings) {
       onleave: function onleave(options) {
         debug(`Leaving state: ${options.from}`)
         options.instance = options.args[0]
+        delete options.args[0]
         return Model.notifyObserversOf('fsm:onleave', options)
           .then(result => Object.assign(options, result))
       },
@@ -25,7 +26,7 @@ module.exports = function StateMachine(Model, settings) {
         debug(`Finalizing state: ${options.to}`)
         return options.instance.updateAttribute(stateProperty, options.to)
           .then(result => {
-            options.res = result
+            options.instance = result
             return Model.notifyObserversOf('fsm:onenter', options)
           })
           .then(result => Object.assign(options, result))
@@ -35,7 +36,11 @@ module.exports = function StateMachine(Model, settings) {
       onentered: function onentered(options) {
         debug(`State is now: ${options.to}`)
         return Model.notifyObserversOf('fsm:onentered', options)
-          .then(result => Object.assign(options, result))
+          .then(result => {
+            // Return either the instance, or whatever was put in options.res
+            options.res = options.res || result.instance
+            return options
+          })
       },
     },
   }
@@ -103,7 +108,10 @@ module.exports = function StateMachine(Model, settings) {
     Model.__FSM_CONFIG__.callbacks[handler] = function runHandler(options) {
       debug('Calling %s: %o', handler, options)
       return Model.notifyObserversOf(`fsm:${handler}`, options)
-        .then(result => Object.assign(options, result))
+        .then(options => {
+          debug('Result from %s: %o', handler, options)
+          return options
+        })
     }
   })
 
@@ -113,7 +121,11 @@ module.exports = function StateMachine(Model, settings) {
   // Add event methods to the prototype.
   Model.getEventNames(settings.events).forEach(eventName => {
     Model.prototype[eventName] = function event(...args) {
-      return Model.app.getStateMachine(this)[eventName](this, ...args)
+      const fms = Model.app.getStateMachine(this)
+
+      return fms[eventName](this, ...args)
+        .then(result => result)
+        .finally(() => Model.app.deleteStateMachine(this))
     }
   })
 }

test/fullcube-state-machine/common/models/subscription.js

@@ -1,9 +1,9 @@
 const FcSubscription = require('../providers/subscription')
 
 module.exports = function SubscriptionModel(Subscription) {
-  Subscription.observe('fsm:oncancel', ctx => new FcSubscription(ctx.instance).cancel().then(() => ctx))
-
-  Subscription.observe('fsm:onreactivate', ctx => new FcSubscription(ctx.instance).reactivate().then(() => ctx))
-
-  Subscription.observe('fsm:onexpire', ctx => new FcSubscription(ctx.instance).expire().then(() => ctx))
+  // Subscription.observe('fsm:oncancel', ctx => new FcSubscription(ctx.instance).cancel().then(() => ctx))
+  //
+  // Subscription.observe('fsm:onreactivate', ctx => new FcSubscription(ctx.instance).reactivate().then(() => ctx))
+  //
+  // Subscription.observe('fsm:onexpire', ctx => new FcSubscription(ctx.instance).expire().then(() => ctx))
 }

test/test.js

@@ -50,6 +50,7 @@ describe('State changes', function() {
     it('Should save the final state', function() {
       return this.subscription.cancel()
         .then(subscription => {
+          console.log('sdfsdfsdfsdd09-934-93409we09w09r0e9', subscription)
           expect(subscription).to.have.property('status', 'canceled')
         })
     })
@@ -148,8 +149,7 @@ describe('Validation', function() {
   })
 })
 
-describe('Get event states and names', function() {
-
+describe('Utility', function() {
   const testEvents = [
     { name: 'activate', from: [ 'none' ], to: 'active' },
     { name: 'cancel', from: 'active', to: 'canceled' },
@@ -158,20 +158,90 @@ describe('Get event states and names', function() {
     { name: 'expire', from: '*', to: 'expired' },
   ]
 
-  it('should extract the event states', function(done) {
-    const stateEvents = Subscription.getEventStates(testEvents)
+  describe('getEventStates', function() {
+    it('should extract the event states', function() {
+      const stateEvents = Subscription.getEventStates(testEvents)
 
-    expect(stateEvents.length).to.equal(4)
-    expect(stateEvents).to.include('none', 'active', 'canceled', 'expired')
-    done()
+      expect(stateEvents.length).to.equal(4)
+      expect(stateEvents).to.include('none', 'active', 'canceled', 'expired')
+    })
   })
 
-  it('should extract the event names', function(done) {
-    const stateNames = Subscription.getEventNames(testEvents)
+  describe('getEventNames', function() {
+    it('should extract the event names', function() {
+      const stateNames = Subscription.getEventNames(testEvents)
+
+      expect(stateNames.length).to.equal(4)
+      expect(stateNames).to.include('activate', 'cancel', 'reactivate', 'expire')
+    })
+  })
+})
+
+describe('Cache', function() {
+  beforeEach(function() {
+    delete app.locals['loopback-component-fsm']
+  })
 
-    expect(stateNames.length).to.equal(4)
-    expect(stateNames).to.include('activate', 'cancel', 'reactivate', 'expire')
-    done()
+  describe('Add to cache', function() {
+    const subscription = new Subscription({
+      id: 1,
+      status: 'active',
+    })
+
+    it('should create and cache a new state machine', function() {
+      const fsm = app.getStateMachine(subscription)
+      expect(app.locals).to.have.deep.property('loopback-component-fsm.Subscription.id:1', fsm)
+    })
+  })
+
+  describe('Remove from cache', function() {
+    const subscription = new Subscription({
+      id: 1,
+      status: 'active',
+    })
+
+    it('should delete an existing state machine from the cache', function() {
+      app.getStateMachine(subscription)
+      app.deleteStateMachine(subscription)
+      expect(app.locals).to.not.have.deep.property('loopback-component-fsm.Subscription.id:1')
+    })
   })
 
+  describe('Use cache through transition', function() {
+    beforeEach(function() {
+      return Subscription.create({ status: 'active' })
+        .then(subscription => {
+          this.subscription = subscription
+        })
+    })
+
+    it('should delete the state machine on completion', function() {
+      return this.subscription.cancel()
+        .then(() => {
+          expect(app.locals).to.not.have.deep.property(`loopback-component-fsm.Subscription.id:${this.subscription.id}`)
+        })
+    })
+
+    it('should reuse an existing state machine if one is in use', function(done) {
+
+      // Make the subscription.cancel method take 200ms second to run.
+      Subscription.observe('fsm:oncancel', ctx => {
+        return Promise.delay(200).return(ctx)
+      })
+
+      // Start a cancelation.
+      this.subscription.cancel()
+        .then(result => {
+          expect(app.locals).to.not.have.deep.property(`loopback-component-fsm.Subscription.id:${this.subscription.id}`)
+          done()
+        })
+
+        // Start another cancel in 100ms (whilst the other is still running).
+        Promise.delay(100).then(() => this.subscription.cancel())
+          .then(() => Promise.reject(new Error('Should not get this far')))
+          .catch(err => {
+            expect(err).to.have.property('message', 'Previous transition pending')
+          })
+    })
+  })
 })