ruby-on-railstestingrspecmulti-tenantdefault-scope

RSpec with multi tenancy - Why is this simple test failing?


What I'm doing

I recently implemented multi-tenancy (using scopes) following Multitenancy with Scopes (subscription required) as a guide. NOTE: I am using the dreaded "default_scope" for tenant scoping (as shown in Ryan's Railscast). Everything is working in browser just fine, but many (not all) of my tests are failing and I can't figure out why.

I built authentication from scratch (based on this Railscast: Authentication from Scratch (revised) - subscription required) and using an auth_token for "Remember me" functionality (based on this Railscast: Remember Me & Reset Password).

My question

Why is this test failing, and why do the two workarounds work? I've been stumped for a couple days now and just can't figure it out.

What I think is happening

I'm calling the Jobs#create action, and the Job.count is reducing by 1 instead of increasing by 1. I think what's happening is the job is being created, then the app is losing the 'tenant' assignment (tenant is dropping to nil), and the test is counting Jobs for the wrong tenant.

What's odd is that it's expecting "1" and getting "-1" (and not "0"), which implies it's getting a count (note that there's already a 'seed' job created in the before block, so it's probably counting "1" before calling #create), calling the create action (which should increase the count by 1 to 2 total), then losing the tenant and switching to a nil tenant where there are 0 jobs. So it:

...resulting in a -1 change in the Job.count.

You can see below that I've semi-confirmed this by adding ".unscoped" to my Job.count line in the test. This implies that the expected number of jobs is there, but the jobs just aren't in the tenant the app is testing under.

What I don't understand is how it's losing the tenant.

Code

I've tried to grab the relevant parts of my code, and I've created a dedicated single-test spec to make this as easy to dissect as possible. If I can do anything else to make this easy on possible answerers, just let me know what to do!

# application_controller.rb
class ApplicationController < ActionController::Base
  protect_from_forgery
  include SessionsHelper

  around_filter :scope_current_tenant

  private

  def current_user
    @current_user ||= User.unscoped.find_by_auth_token!(cookies[:auth_token]) if cookies[:auth_token]
  end
  helper_method :current_user

  def current_tenant
    @current_tenant ||= Tenant.find_by_id!(session[:tenant_id]) if session[:tenant_id]
  end
  helper_method :current_tenant

  def update_current_tenant
    Tenant.current_id = current_tenant.id if current_tenant
  end
  helper_method :set_current_tenant

  def scope_current_tenant
    update_current_tenant
    yield
  ensure
    Tenant.current_id = nil
  end
end

# sessions_controller.rb
class SessionsController < ApplicationController
  def create
    user = User.unscoped.authenticate(params[:session][:email], params[:session][:password])

    if user && user.active? && user.active_tenants.any?
      if params[:remember_me]
        cookies.permanent[:auth_token] = user.auth_token
      else
        cookies[:auth_token] = user.auth_token
      end

      if !user.default_tenant_id.nil? && (default_tenant = Tenant.find(user.default_tenant_id)) && default_tenant.active
        # The user has a default tenant set, and that tenant is active
        session[:tenant_id] = default_tenant.id
      else
        # The user doesn't have a default
        session[:tenant_id] = user.active_tenants.first.id
      end
      redirect_back_or  root_path
    else
      flash.now[:error] = "Invalid email/password combination."
      @title = "Sign in"
      render 'new'
    end  
  end

  def destroy
    cookies.delete(:auth_token)
    session[:tenant_id] = nil
    redirect_to root_path
  end
end

# jobs_controller.rb
class JobsController < ApplicationController
  before_filter :authenticate_admin

  # POST /jobs
  # POST /jobs.json
  def create    
    @job = Job.new(params[:job])
    @job.creator = current_user

    respond_to do |format|
      if @job.save
        format.html { redirect_to @job, notice: 'Job successfully created.' }
        format.json { render json: @job, status: :created, location: @job }
      else
        flash.now[:error] = 'There was a problem creating the Job.'
        format.html { render action: "new" }
        format.json { render json: @job.errors, status: :unprocessable_entity }
      end
    end
  end
end

# job.rb
class Job < ActiveRecord::Base
  has_ancestry

  default_scope { where(tenant_id: Tenant.current_id) }
  .
  .
  .

end

# sessions_helper.rb
module SessionsHelper

require 'bcrypt'

  def authenticate_admin
    deny_access unless admin_signed_in?
  end

  def deny_access
    store_location
    redirect_to signin_path, :notice => "Please sign in to access this page."
  end

  private

  def store_location
    session[:return_to] = request.fullpath
  end
end

# spec_test_helper.rb
module SpecTestHelper 
  def test_sign_in(user)
    request.cookies[:auth_token] = user.auth_token
    session[:tenant_id] = user.default_tenant_id
    current_user = user
    @current_user = user
  end

  def current_tenant
    @current_tenant ||= Tenant.find_by_id!(session[:tenant_id]) if session[:tenant_id]
  end
end


# test_jobs_controller_spec.rb
require 'spec_helper'

describe JobsController do
  before do
    # This is all just setup to support requirements that the admin is an "Admin" (role)
    # That there's a tenant for him to use
    # That there are some workdays - a basic requirement for the app - jobs, checklist
    # All of this is to satisfy assocations and 
    @role = FactoryGirl.create(:role)
    @role.name = "Admin"
    @role.save
    @tenant1 = FactoryGirl.create(:tenant)
    @tenant2 = FactoryGirl.create(:tenant)
    @tenant3 = FactoryGirl.create(:tenant)

    Tenant.current_id = @tenant1.id
    @user = FactoryGirl.create(:user)
    @workday1 = FactoryGirl.create(:workday)
    @workday1.name = Time.now.to_date.strftime("%A")
    @workday1.save
    @checklist1 = FactoryGirl.create(:checklist)
    @job = FactoryGirl.create(:job)
    @checklist1.jobs << @job
    @workday1.checklists << @checklist1
    @admin1 = FactoryGirl.create(:user)
    @admin1.tenants << @tenant1
    @admin1.roles << @role
    @admin1.default_tenant_id = @tenant1.id
    @admin1.pin = ""
    @admin1.save!
    # This is above in the spec_test_helper.rb code
    test_sign_in(@admin1)
  end

  describe "POST create" do
    context "with valid attributes" do      
      it "creates a new job" do
        expect{ # <-- This is line 33 that's mentioned in the failure below
          post :create, job: FactoryGirl.attributes_for(:job)
        # This will pass if I change the below to Job.unscoped
        # OR it will pass if I add Tenant.current_id = @tenant1.id right here.
        # But I shouldn't need to do either of those because
        # The tenant should be set by the around_filter in application_controller.rb
        # And the default_scope for Job should handle scoping
        }.to change(Job,:count).by(1)
      end
    end
  end
end

Here is the failure from rspec:

Failures:

  1) JobsController POST create with valid attributes creates a new job
     Failure/Error: expect{
       count should have been changed by 1, but was changed by -1
     # ./spec/controllers/test_jobs_controller_spec.rb:33:in `block (4 levels) in <top (required)>'

Finished in 0.66481 seconds
1 example, 1 failure

Failed examples:

rspec ./spec/controllers/test_jobs_controller_spec.rb:32 # JobsController POST create with valid attributes creates a new job

If I add some 'puts' lines to see who the current_tenant is directly and by inspecting the session hash, I see the same tenant ID all the way:

describe "POST create" do
  context "with valid attributes" do      
    it "creates a new job" do
      expect{
        puts current_tenant.id.to_s
        puts session[:tenant_id]
        post :create, job: FactoryGirl.attributes_for(:job)
        puts current_tenant.id.to_s
        puts session[:tenant_id]
      }.to change(Job,:count).by(1)
    end
  end
end

Yields...

87
87
87
87
F

Failures:

  1) JobsController POST create with valid attributes creates a new job
     Failure/Error: expect{
       count should have been changed by 1, but was changed by -1
     # ./spec/controllers/test_jobs_controller_spec.rb:33:in `block (4 levels) in <top (required)>'

Finished in 0.66581 seconds
1 example, 1 failure

Failed examples:

rspec ./spec/controllers/test_jobs_controller_spec.rb:32 # JobsController POST create with valid attributes creates a new job

Solution

  • I managed to get my tests to pass, although I'm still not sure why they were failing to begin with. Here's what I did:

      describe "POST create" do
        context "with valid attributes" do      
          it "creates a new job" do
            expect{ # <-- This is line 33 that's mentioned in the failure below
              post :create, job: FactoryGirl.attributes_for(:job)
            }.to change(Job.where(tenant_id: @tenant1.id),:count).by(1)
          end
        end
      end
    

    I changed:

    change(Job,:count).by(1)
    

    ...to:

    change(Job.where(tenant_id: @tenant1.id),:count).by(1)
    

    NOTE: @tenant1 is the logged-in admin's tenant.

    I assumed default_scopes would be applied in RSpec, but it seems they aren't (or at least not in the ":change" portion of an "expect" block). In this case, the default_scope for Job is:

    default_scope { where(tenant_id: Tenant.current_id) }
    

    In fact, if I change that line to:

    change(Job.where(tenant_id: Tenant.current_id),:count).by(1)
    

    ...it will also pass. So if I explicitly mimic the default_scope for Job within the spec, it'll pass. This seems like confirmation that RSpec is ignoring my default_scope on Jobs.

    In a way, I think my new test is a better way to make sure tenant data stays segregated because I'm explicitly checking counts within a particular tenant rather than implicitly checking the counts for a tenant (by assuming the count is in the "current tenant").

    I'm marking my answer is correct because it's the only answer, and if someone else encounters this, I think my answer will help them get past the issue. That said, I really haven't answered my original question regarding why the test was failing. If anyone has any insight into why RSpec seems to be ignoring default_scope in "expect" blocks, that might help making this question useful for others.