validates_uniqueness_of with custom setter fails with "should require case sensitive unique value for email"

[dgmstuart]

On upgrading from 2.x to 3.0.1 I get the following error:

User when the record has a password_digest should require should require case sensitive unique value for email
     Failure/Error: it { should validate_uniqueness_of(:email).with_message(/already been used by you/) }
     Shoulda::Matchers::ActiveModel::AllowValueMatcher::CouldNotSetAttributeError:
       The allow_value matcher attempted to set :email on User to
       "TEST1@FOO.COM", but when the attribute was read back, it had stored
       "test1@foo.com" instead.

       This creates a problem because it means that the model is behaving in a
       way that is interfering with the test -- there's a mismatch between the
       test that was written and test that was actually run.

       There are a couple of reasons why this could be happening:

       * The writer method for :email has been overridden and contains custom
         logic to prevent certain values from being set or change which values
         are stored.
       * ActiveRecord is typecasting the incoming value.

       Regardless, the fact you're seeing this message usually indicates a
       larger problem. Please file an issue on the GitHub repo for
       shoulda-matchers, including details about your model and the test
       you've written, and we can point you in the right direction:

       https://github.com/thoughtbot/shoulda-matchers/issues

This seems to be because shoulda-matchers now cares about case-sensitivity when checking for uniqueness, and we have a custom setter which is downcasing values before saving:

# models/user.rb
validates :email, uniqueness: true

def email=(string)
  return super string unless string
  super string.strip.downcase
end

# models/user_spec.rb
subject(:user) { create(:user) }
it { should validate_uniqueness_of(:email) }

I don't know if this custom setter is a good or bad idea - happy to take advice on that. My guess is that shoulda-matchers possibly can't deal with this sort of thing because of the way it works under the hood (iirc it uses the model's getters and setters, right?).

I've tried the example with .case_insensitive added to the matcher, and I get basically the same message.

Would it make sense for shoulda-matchers to ignore this scenario unless :case_insensitive is true? Or provide an option to skip this part of the checks? I'm happy to work on a pull request if we can agree a sensible approach.

[mcmire]

Yup, you hit the nail on the head -- it's due to the behavior of email=.

I've tried the example with .case_insensitive added to the matcher, and I get basically the same message.

The case_insensitive qualifier tests that if two records have the same email where one email is uppercase and the second is lowercase, then the second record should be invalid. The reason why it doesn't work here is that since the email gets downcased, that's the same test that gets run if you don't use case_insensitive.

Would it make sense for shoulda-matchers to ignore this scenario unless :case_insensitive is true? Or provide an option to skip this part of the checks?

This issue has been raised before (#786), and I'm working on a fix here: #820. Basically I'm considering adding a qualifier that would look like the following:

it do
  should validate_uniqueness_of(:email).
    converting_values("TEST1@FOO.COM" => "test1@foo.com")
end

Now that I look at this, I'm not entirely happy with it, because it means that your test has to care about values that aren't mentioned anywhere else.

Hmm... does your factory have to create emails that are uppercased?

[dgmstuart]

Ah thanks so much - sorry I didn't find the previous issue. Glad it's being looked into 😄

Our factory is creating lowercase emails:

sequence(:email) { |n| "test#{n}@foo.com" }

Based on what you've described, the default case (where @options[:case_insensitive] is nil or false) is actively doing a check that the uniqueness isn't case sensitive right? (i.e. that "TEST1@FOO.COM" and "test1@foo.com" can both be persisted).

Would a solution be to add an option to skip_case_sensitivity_checks: to not actively check whether it's case sensitive or not? In our use case we genuinely don't care, because we're doing our best to ensure that only lowercase emails get into the data store.

To be honest I don't quite understand why email= is used instead of accessing the underlying instance[:email] (?), but I'm sure there are good reasons for that 😄

[mcmire]

Based on what you've described, the default case (where @options[:case_insensitive] is nil or false) is actively doing a check that the uniqueness isn't case sensitive right? (i.e. that "TEST1@FOO.COM" and "test1@foo.com" can both be persisted).

Yes, that's what what happens. Basically it says, if you have a record with "TEST1@FOO.COM", then a second record with "TEST1@FOO.COM" isn't okay, but a record with "test1@foo.com" is okay. I think in your case that's actually being reversed ("test1@foo.com" + "test1@foo.com", then "test1@foo.com" + "TEST1@FOO.COM"). So the matcher is producing a record with "TEST1@FOO.COM", which ends up getting converted to "test1@foo.com".

Would a solution be to add an option to skip_case_sensitivity_checks?

Yeah, now that you mention it, I think that would be a good qualifier to have. This is where that case sensitivity check is happening.

[mcmire]

A colleague at work just ran into this same issue, except the scenario is different. Basically, Devise adds a before_validation on whatever field you're using for authentication (let's say username) so that the value this is set to will be downcased. So this screws up the uniqueness matcher in a similar manner to what was posted above.

[harshs08]

I have also got the same issue while writing test for a model created using FFaker gem. I guess as already mentioned Devise before_validation is downcasing the email id created using FFaker.
So until this patch is released how can I fix this issue.

[dgmstuart]

@harshs08 I don't think it's possible to test this scenario through shoulda-matchers until this PR is merged and released. Either:

1. Use an older version of the gem
2. Use the gem from the PR branch on my fork (probably not advisable!!)
3. Add your own specs for uniqueness (maybe by copying some of the internals of the gem?)
4. Don't test this for now, e.g:

it do
  pending("a fix for https://github.com/thoughtbot/shoulda-matchers/issues/836") do
    should validate_uniqueness_of(:email_id)
  end
end

As I understand it, this will be shown as pending until the issue is fixed (I guess when you upgrade to a patched version), at which point this spec will fail with a message informing you that it has been fixed.

[harshs08]

@dgmstuart, great suggestions. I guess right now I am gonna go with some custom validations (2).

[weppos]

Same issue here and the reason is exactly the same. We override email= to force lower case.

[lfzawacki]

Affected by this issue as well.

[mcmire]

I'm going to close this in favor of #840 to clean things up a bit.