John M Naglick
John M Naglick

Reputation: 1881

FactoryGirl attribute set in after(:create) doesnt persist until referenced?

Sorry for the vague title, there are a lot of moving parts to this problem so I think it will only be clear after seeing my code. I'm fairly sure I know what's going on here and am looking for feedback on how to do it differently:

I have a User model that sets a uuid attr via an ActiveRecord callback (this is actually in a "SetsUuid" concern, but the effect is this):

class User < ActiveRecord::Base
    before_validation     :set_uuid, on: :create
    validates             :uuid, presence: true, uniqueness: true

    private

    def set_uuid
      self.uuid = SecureRandom.uuid
    end
end

I am writing a functional rspec controller test for a "foo/add_user" endpoint. The controller code looks like this (there's some other stuff like error-handling and @foo and @params being set by filters, but you get the point. I know this is all working.)

class FoosController < ApplicationController
    def add_user
        @foo.users << User.find_by_uuid!(@params[:user_id])
        render json: {
            status: 'awesome controller great job'
        }
    end
end

I am writing a functional rspec controller test for the case "foo/add_user adds user to foo". My test looks roughly this (again, leaving stuff out here, but the point should be obvious, and I know it's all working as intended. Also, just to preempt the comments: no, I'm not actually 'hardcoding' the "user-uuid" string value in the test, this is just for the example)

RSpec.describe FoosController, type: :controller do
    describe '#add_user' do
        it_behaves_like 'has @foo' do
            it_behaves_like 'has @params', {user_id: 'user-uuid'} do
                context 'user with uuid exists' do
                    let(:user) { create(:user_with_uuid, uuid: params[:user_id]) } # params is set by the 'has @params' shared_context

                    it 'adds user with uuid to @foo' do
                        route.call() # route is defined by a previous let that I truncated from this example code
                        expect(foo.users).to include(user) # foo is set by the 'has @foo' shared_context
                    end
                end
            end
        end
    end
end

And here is my user factory (I've tried setting the uuid in several different ways, but my problem (that I go into below) is always the same. I think this way (with traits) is the most elegant, so that's what I'm putting here):

FactoryGirl.define do
  factory :user do
    email         { |n| "user-#{n}@example.com" }
    first_name    'john'
    last_name     'naglick'
    phone         '718-555-1234'

    trait :with_uuid do
      after(:create) do |user, eval|
        user.update!(uuid: eval.uuid)
      end
    end

    factory :user_with_uuid, traits: [:with_uuid]
  end
end

Finally, The problem:

This only works if I reference user.uuid before route.call() in the spec.

As in, if I simply add the line "user.uuid" before route.call(), everything works as intended.

If I don't have that line, the spec fails because the user's uuid doesn't actually get updated by the after(:create) callback in the trait in the factory, and thus the User.find_by_uuid! line in the controller does not find the user.

And just to preempt another comment: I'm NOT asking how to re-write this spec so that it works like I want. I already know a myriad of ways to do this (the easiest and most obvious being to manually update user.uuid in the spec itself and forget about setting the uuid in the factory altogether). The thing I'm asking here is why is factorygirl behaving like this?

I know it has something to do with lazy-attributes (obvious by the fact it magically works if I have a line evaluating user.uuid), but why? And, even better: is there some way I can do what I want here (setting the uuid in the factory) and have everything work like I intend? I think it's a rather elegant looking use of rspec/factorygirl, so I'd really like it to work like this.

Thanks for reading my long question! Very much appreciate any insight

Upvotes: 1

Views: 454

Answers (2)

max
max

Reputation: 102368

Instead of using the before_validation callback you should be using after_initialize. That way the callback is fired even before .valid? is called in the model lifecycle.

class User < ActiveRecord::Base
    before_initialization :set_uuid!, on: :create, if: :set_uuid?
    validates             :uuid, presence: true, uniqueness: true

    private

    def set_uuid!
        # we should also check that the UUID 
        # does not actually previously exist in the DB 
        begin
           self.uuid = SecureRandom.uuid
        end while User.where(uuid: self.uuid).any?
    end

    def set_uuid?
        self.uuid.nil?
    end
end

Although the chance of generating the same hash twice with SecureRandom.uuid is extremely slim it is possible due to the pigeonhole principle. If you maxed out in the bad luck lottery this would simply generate a new UUID.

Since the callback fires before validation occurs the actual logic here should be completely self contained in the model. Therefore there is no need to setup a callback in FactoryGirl.

Instead you would setup your spec like so:

let!(:user) { create(:user) }

it 'adds user with uuid to @foo' do
   post :add_user, user_id: user.uuid, { baz: 3 }
end

Upvotes: 0

Anthony
Anthony

Reputation: 15977

Your issue has less to do with FactoryGirl and more to do with let being lazily evaluated.

From the docs:

Use let to define a memoized helper method. The value will be cached across multiple calls in the same example but not across examples.

Note that let is lazy-evaluated: it is not evaluated until the first time the method it defines is invoked. You can use let! to force the method's invocation before each example.

Since your test doesn't invoke the user object until the expectation there is nothing created. To force rspec to load object, you can use let!.

Upvotes: 2

Related Questions