Add Session#restore_state for restoring previous state after block execution

[jeremyevans]

This type of API is very helpful for testing access control in
applications. I currently do something similar in the
capybara-restore_state gem, but that reaches into rack-test
internals, and I would like to have a public API for this so it
isn't necessary to do so.

[ioquatix]

I think this is a great feature to add. I'm not sold on the naming but the concept is great. I'd probably solicit input from a wider audience to see what typical users of rack-test might come up with.

It's too bad

rack-test/lib/rack/test.rb

Line 366 in 30b43cd

yield @last_response if block_given?

already exists, because it would be a logical place to use your proposed implementation/concept IMHO. Maybe we can add an option...

get '/', isolated: true do |response|

end

Or maybe we can make it a sub-class so people can opt into it for their tests. Or maybe it doesn't matter?

One more question I have, is how to handle

rack-test/lib/rack/test.rb

Line 119 in 30b43cd

@after_request << block

- should this be saved and restored too?

[jeremyevans]

In regards to naming, I agree restore_state is somewhat ambiguous (does it restore state before block execution or after?), but the same is true of preserve_state or keep_state. restore_state_after_block_to_state_before_block seems accurate but way too long.

My usage of this feature is typically not limited to a single request. It would be akin to:

session.restore_state do
  # get '/foo'
  # post '/foo/bar'
end
# get '/foo'

So adding this via an request-specific option would not be useful in my opinion, and I think having the block mean two different things (since it is already used for yielding the last response) would be confusing. For this reason, isolated_request doesn't seem like a good method name to me.

In terms of push_request_state as a name, this doesn't actually push a request state. Request states are updated automatically for each request. Also, push_* without pop_* sounds odd to me. restore_request_state could be used, but I'm not sure it is better than restore_state, since it could imply it only restores last_request, not last_response.

Restoring after_request does seem like a reasonable thing to do, I'll add that.

[ioquatix]

It helps to see an example, thanks.

What about with_isolated_state?

session.with_isolated_state do
  # get '/foo'
  # post '/foo/bar'
end
# get '/foo'

I cannot help but feel like restore_state is incredibly confusing name, but I also agree it's hard to find something better. Are you able to propose some other options?

[jeremyevans]

with_isolated_state implies the state inside the block is isolated from the state outside the block, which isn't true as the state is inherited. The method restores the state after block execution to the state before block execution. To me, restore_state seems like the best method name of reasonable length for this feature.

[ioquatix]

Would it make sense to have some kind of with_isolated_session which just creates a new session?

[jeremyevans]

We already have the equivalent of with_isolated_session: with_session(nil). That doesn't inherit the current state, so it doesn't work for the use case I have for restore_state.

[ioquatix]

I still think we should find a better name.

[jeremyevans]

I'll merge this now. We can always rename the method later if a better name is discovered.