acts_as_tenant
acts_as_tenant copied to clipboard
Fix test_tenant leaking into current_tenant
Using test_tenant
in conjunction with with_tenant {}
and without_tenant {}
in some cases leads to test_tenant
being assign to current_tenant
which leads to subtle bugs, that are difficult to pin down.
This happens because the value of test_tenant
is restored as current_tenant
in the ensure
part of those methods. The test cases in this pr illustrate the issue.
I'm pretty sure that's the expected behavior: https://github.com/ErwinM/acts_as_tenant#testing
Hi @excid3
Just to give a bit more explanation to what's going on here. We start with test_tenant
set to test_account
, this behaves like this:
# start (set test_tenant = test_account)
RequestStore.store[:current_tenant] == nil
ActsAsTenant.test_tenant == test_account
ActsAsTenant.current_tenant == test_account
Then we might do a request, using ActsAsTenant::TestTenantMiddleware
, which sets test_tenant
to nil
during the request. So far, everything is still fine.
# start request with ActsAsTenant::TestTenantMiddleware (set test_tenant = nil)
RequestStore.store[:current_tenant] == nil
ActsAsTenant.test_tenant == nil
ActsAsTenant.current_tenant == nil
# finish request with ActsAsTenant::TestTenantMiddleware (reset test_tenant = test_account)
Now we do a with_tenant
block. This is where things start to go wrong.
ActsAsTenant.with_tenant(account1) do
RequestStore.store[:current_tenant] == account1
ActsAsTenant.test_tenant == test_account
ActsAsTenant.current_tenant == account1
end
At the end of with_tenant
, the RequestStore.store[:current_tenant]
is set to the test_tenant
... but, it still behaves correctly, for now.
RequestStore.store[:current_tenant] == test_account # <--- not correct!
ActsAsTenant.test_tenant == test_account
ActsAsTenant.current_tenant == test_account # <--- but still ok...
We do another request, again test_tenant
is set to nil
, BUT this time RequestStore.store[:current_tenant]
is set, so current_tenant
looks like it's set!
# start request with ActsAsTenant::TestTenantMiddleware (set test_tenant = nil)
RequestStore.store[:current_tenant] == test_account
ActsAsTenant.test_tenant == nil
ActsAsTenant.current_tenant == test_account # wrong!
# finish request with ActsAsTenant::TestTenantMiddleware (reset test_tenant = test_account)
In short, what we're seeing is that with_tenant
sets the RequestStore.store[:current_tenant]
and doesn't reset it afterwards, which affects any further requests that rely on ActsAsTenant::TestTenantMiddleware
. In particular, requesting routes that don't expect a tenant (i.e. routes that show multiple tenants) now have a tenant set.