Rails/UniqueValidationWithoutIndex should not fail when using scope

[nickserv]

A uniqueness validation over a scope should not fail Rails/UniqueValidationWithoutIndex, as a unique index would not necessarily be unique for the entire table when it's unique under the scope.

---

Expected behavior

Cop should not fail when a scope is used to prevent false positives and not encourage the user to change their schema in a way that could break scoped uniqueness functionality.

Actual behavior

Rails/UniqueValidationWithoutIndex: Uniqueness validation should be with a unique index.

Steps to reproduce the problem

validates :field_1, uniqueness: { scope: :field_2 }

RuboCop version

rubocop (0.81.0)
rubocop-rails (2.5.2)

[koic]

RuboCop Rails 2.5.2 has an expected tests for uniqueness: { scope: :field_2 }. https://github.com/rubocop-hq/rubocop-rails/blob/v2.5.2/spec/rubocop/cop/rails/unique_validation_without_index_spec.rb#L117-L135

Rails/UniqueValidationWithoutIndex cop inspects a model and db/schema.rb as a set. Can you show me the model and db/schema.rb as a set?

[nickserv]

Model validation: https://github.com/nickmccurdy/market/blob/master/app/models/player.rb#L3 Schema: https://github.com/nickmccurdy/market/blob/master/db/schema.rb

[koic]

Thank you showing code.

This issue seems to be the expected behavior, not a bug. Here is a quote from #197 where Rails/UniqueValidationWithoutIndex cop were introduced:

The target is the ActiveRecord's uniqueness validation.

When you define uniqueness validation in a model, you also should add a unique index. It has two reasons.

First, duplicated records may occur even if ActiveRecord's validation is defined. The Rails Guide mentions this problem.

This helper validates that the attribute's value is unique right before the object gets saved. It does not create a uniqueness constraint in the database, so it may happen that two different database connections create two records with the same value for a column that you intend to be unique. To avoid that, you must create a unique index on that column in your database. https://guides.rubyonrails.org/active_record_validations.html#uniqueness

Second, it will cause slow queries. The validation executes a SELECT statement with the target column when inserting/updating a record. If the column does not have an index and the table is large, the query will be heavy.

In this case, it is expected that the following unique index is generated in db/schema.rb.

t.index ["field_1", "field_2"], name: "idx_field_1_field_2", unique: true

[nickserv]

Thanks for the explanation. Would it be possible for the cop to give a more specific message? I thought I already had an index, but didn’t realize it was incorrect.

[koic]

Yeah, the offense message can be changed. On the other hand, offense messages are preferably simple, concise and short explanation. I'm not getting a good message yet. Do you have any good explanation?

[nickserv]

I don’t really know how to describe that syntax. Perhaps the error could just have a one-line code example of what’s expected in the model.

[kortirso]

@koic , I have same problem for scope with polymorphic

belongs_to :fieldable, polymorphic: true validates :type, uniqueness: { scope: :fieldable }

I tried to add index with these migrations (with and without name options) add_index :fields, [:type, :fieldable_type, :fieldable_id], name: "index_fields_on_type_and_fieldable", unique: true, algorithm: :concurrently add_index :fields, [:fieldable_type, :fieldable_id, :type], name: "index_fields_on_type_and_fieldable", unique: true, algorithm: :concurrently

last version of schema t.index ["type", "fieldable_type", "fieldable_id"], name: "index_fields_on_type_and_fieldable", unique: true

[tubbo]

this is happening for me as well.

model:

  validates :name, presence: true, uniqueness: { scope: :author }

schema:

  create_table "apps", force: :cascade do |t|
    t.string "name", null: false
    t.string "author_type", null: false
    t.bigint "author_id", null: false
    t.citext "authentication_token", null: false
    t.integer "failed_attempts", default: 0, null: false
    t.citext "unlock_token"
    t.datetime "locked_at"
    t.integer "sign_in_count", default: 0, null: false
    t.datetime "current_sign_in_at"
    t.datetime "last_sign_in_at"
    t.inet "current_sign_in_ip"
    t.inet "last_sign_in_ip"
    t.datetime "created_at", precision: 6, null: false
    t.datetime "updated_at", precision: 6, null: false
    t.index ["author_id", "author_type", "name"], name: "index_apps_on_author_id_and_author_type_and_name", unique: true
    t.index ["author_type", "author_id"], name: "index_apps_on_author_type_and_author_id"
  end

It seems like maybe the cop isn't checking for _type as well as _id in the list of indexes? Makes sense as this doesn't fail for me on a regular belongs_to:

model:

  validates :name, presence: true, uniqueness: { scope: :version }

schema:

  create_table "docs", force: :cascade do |t|
    t.bigint "version_id"
    t.string "name", null: false
    t.datetime "created_at", precision: 6, null: false
    t.datetime "updated_at", precision: 6, null: false
    t.index ["name", "version_id"], name: "index_docs_on_name_and_version_id", unique: true
    t.index ["name"], name: "index_docs_on_name"
    t.index ["version_id"], name: "index_docs_on_version_id"
  end

[alec-c4]

The same problem :(

model:

  validates :employee, uniqueness: { scope: :employable, conditions: -> { where(active: true) } }

migration:

    add_index :employments, [:id, :employable_id, :employee_id, :active], unique: true, name: "active_employments_index"

[schmijos]

I had to disable this cop for the following code:

Schema:

  create_table "time_slot_bookings", force: :cascade do |t|
    t.string "state", default: "pending"
    t.index ["state", "published_time_slot_id"], name: "index_time_slot_bookings_on_state_and_published_time_slot_id"
    …

Validation:

  validates :state, uniqueness: { scope: :published_time_slot_id,
                                  conditions: -> { processed },
                                  message: 'should not be double booked' }

While I see that an index makes sense in general, uniqueness on the database is not possible in this case (without manually writing SQL).

[TorionJ]

Hi! I'm still running into this issue...

Gems: rubocop-rails: 2.15.2 rubocop: 1.34.1 ruby: 3.0.4 rails: 6.1.7

Model: validates :name, :version, presence: true, uniqueness: { scope: :product_id }

Schema: t.index ["name", "version", "product_id"], name: "index_ota_releases_on_name_and_version_and_product_id", unique: true

Is there an underlying fix for this issue besides silencing Rubocop?

[fatkodima]

We can probably improve the handling of scopes/polymorphic associations/etc in the rubocop itself (unfortunately by introducing more and more complexity into it), but there is already a tool better suited for such checks and more - https://github.com/gregnavis/active_record_doctor. It has a check for missing unique indexes - https://github.com/gregnavis/active_record_doctor#detecting-uniqueness-validations-not-backed-by-an-index and handles all the mentioned problematic cases.