nickfargo / state

Composable, heritable first-class states
statejs.org
Other
93 stars 8 forks source link

Only the first guard is called #21

Open AdrianRossouw opened 10 years ago

AdrianRossouw commented 10 years ago

i just spent the last hour rewriting this logic, but I finally had to make my session object downgrade automatically as credentials get added.

I already know that the guards really wanted the query: fn hash, so what i was expecting to do was..

class Session
 initState: ->
    state @,
      offline: state 'initial'

      online: state 'abstract',
        session: state
        socket: state
            release:
              sip: -> !!@sip
              session: -> !@socket
        sip: state
        voice: state

But what I found is that the it will only ever call the first guard, and not consider the keys. I found that if my first guards query string (include ',' separated queries) didn't match, other guards would never be called.

So the following would never match, for either release or admit.

  release:
    sip: -> false
    '**': -> true

The code was also using the emit() calls from before still, but this seemed like an issue around the guards specifically, since i had run into it before.

nickfargo commented 10 years ago

Can you give any details?

Guards should short-circuit, like a logical AND, i.e. the first false kills the evaluation process. But, I expect you’d have expected this, and something else is afoot.

AdrianRossouw commented 10 years ago

god. i hit submit on this? editing my issue post.

nickfargo commented 10 years ago

heh … ⌘⏎ … damn near gets me every time

AdrianRossouw commented 10 years ago

for posterity, here is my rewritten session model, including 30 tests.

there's minimal app-specific code still in there, so it should be pretty easy to get running.

nickfargo commented 10 years ago

Is this gist then showing a successful manual implementation of the desired guard-like behavior … which I assume is failing using the proper State guards?

If so, do you have a failing case? If not, what have I missed …

AdrianRossouw commented 10 years ago

the gist is the rewrite to not use guards. but you are in luck, i had an intermediate commit too.

updated gist

The order of the release guards in the middle states matters. only the first will fire.

If you want to check out what I've been writing, come chat on https://werewolves.io quickly. I'll be online for the next 20 minutes or so. I would have invited you in private but i am unable to find an email or anything for you. (the voip connection kicks in after the intro music)

nickfargo commented 10 years ago

(same name at twitter and gmail)

I see. I’ll have a look …

nickfargo commented 10 years ago

So … and this isn’t quite runnable I don’t think, on account of require('../app') and App.module "Modules", unless I’m whiffing on something again … anyway, question was, then, for these state expressions:

        socket:
          downgrade: 'session'
          upgrade: 'sip'
          release:
            'sip': -> true if @owner.socket
            'session': -> true if !@owner.socket
          admit:
            'sip,offline,session': -> true if @owner.socket
        sip:
          downgrade: 'socket'
          upgrade: 'voice'
          release:
            'voice': -> true if @owner.sip
            'socket': -> true if !@owner.sip
          admit:
            'socket,offline,voice': -> true if (@owner.sip && @owner.socket)

release was being evaluated, but was apparently precluding evaluation of the admit that followed it. Do I have that right?

AdrianRossouw commented 10 years ago

Those can be removed pretty easily.

and this was the problem :

in the socket state, only the sip release guard was ever called. in the sip state, only the voice release guard is ever called.

the admit guard was always called, and only on the matched states.

if i switched around the order of the socket.release.sip and socket.release.session, only the first defined one would ever be called.

I had experienced this problem before when attempting to have more than one admit guard, although the code wasn't nearly as compartmentalised, and I was forced to work around it in another way.

AdrianRossouw commented 10 years ago

i updated the code without the App stuff. https://gist.github.com/Vertice/b21ccc40fee226da87b2

nickfargo commented 10 years ago

I see. And I take it, in these cases, it is confirmed/assumable that the first key-value (selector–predicate) pair was indeed evaluating to true, meaning we should be expecting it to iterate on to the next predicate, rather than evaluating to false and short-circuiting…?

Because if those aren’t returning truthily, then I think we’d expect to see the observed behavior.

Also, I’m forgetting or unclear on where this @attribute class function here comes from.… (Backbone it is not — and running session.tests.coffee in the cloned gist still errors here for me.)

class Session extends Backbone.Model
  # session identifiers
  urlRoot: 'session'
  @attribute 'session'
  @attribute 'socket'
  @attribute 'sip'
  @attribute 'voice'

I must assume there’s something elided here that is supposed to cause a session (et al) property to be expandoed onto instances after invoking the constructor with a new Session …? Which, then, must be the origin of the

-> true if @owner.session

bits and so forth that these predicates are, um … predicated upon?

AdrianRossouw commented 10 years ago

sorry. the attribute thing is from here :

http://srackham.wordpress.com/2011/10/16/getters-and-setters-for-backbone-model-attributes/

i'm actually really surprised at how well that stuff all works.

nickfargo commented 10 years ago

Ah, ok, should have guessed. Anyway … will be good to have this on the radar for the new Guards’ tests. Thanks.