consistency_fail icon indicating copy to clipboard operation
consistency_fail copied to clipboard

Rails 5.2 ActiveStorage Introduces Unique Index Violation on active_storage_attachments

Open olliebennett opened this issue 6 years ago • 7 comments

Hi! This isn't really a problem caused by this gem, but thought I'd post here as it's the first place people might look!

After upgrading Rails to 5.2, I see this output from consistency_fail

Hooray! All calls to validates_uniqueness_of are correctly backed by a unique index.

Hooray! All calls to has_one are correctly backed by a unique index.


There are calls to has_one_with_polymorphic that aren't backed by unique indexes.
--------------------------------------------------------------------------------
Model                Table Columns
--------------------------------------------------------------------------------
ActiveStorage::Blob  active_storage_attachments (record_type, record_id)
--------------------------------------------------------------------------------

I'd love to know a way to fix and/or skip this problem with a seemingly internal Rails problem?

For reference, this Rails issue appears to be highlighting the issue on their side.

I'm using consistency_fail (0.3.5).

olliebennett avatar Jun 18 '18 09:06 olliebennett

The same table is used for has_many_attached so adding a unique index is probably out of the question.

hidde-jan avatar Jun 19 '18 08:06 hidde-jan

Hey @trptcolin I wonder if you've got any thoughts on this.

Is the solution a whitelist we can configure somewhere? Happy to have a stab at it if you could give us a nudge in the right direction.

olliebennett avatar Jul 24 '18 06:07 olliebennett

Yeah a whitelist of issues to skip would be my initial idea for a workaround here, and would help other use cases too (e.g. https://github.com/trptcolin/consistency_fail/issues/34)

trptcolin avatar Jul 24 '18 11:07 trptcolin

is there a workaround for this? I am getting the same problem

KaisHaddadin avatar Mar 14 '19 13:03 KaisHaddadin

I have a monkey-patch that will exclude ActiveStorage::Attachment from the polymorphic checks. I'll try and make time to turn it into a PR as this is something that should always be skipped.

Stick this in an initialiser:

  class ConsistencyFail::Introspectors::Polymorphic
    def instances(model)
      model.reflect_on_all_associations.select do |a|
        a.macro == :has_one &&
          a.options[:as].to_s.length > 0 &&
          a.options[:class_name] != "ActiveStorage::Attachment"
      end
    end
  end

paulleader avatar Mar 28 '19 12:03 paulleader

Awesome, thanks for that! Works for us. I wrapped the initializer code with

# config/initializers/consistency_fail_workaround.rb
if Object.const_defined?(:ConsistencyFail)
  # ... code from above
end

to avoid uninitialized constant issues outside of the environments where the gem is used;

# Gemfile.lock
gem 'consistency_fail', group: %i[development test]

olliebennett avatar Mar 28 '19 14:03 olliebennett

@olliebennett Ah yes, good suggestion.

My colleague (@styrmis) came up with a slightly safer version, with the option to extend it to other models if necessary:

class ConsistencyFail::Introspectors::Polymorphic
  IGNORED_MODEL_NAMES = ['ActiveStorage::Attachment']
  alias_method :unfiltered_desired_indexes, :desired_indexes

  def desired_indexes(model)
    indexes = unfiltered_desired_indexes(model)
    indexes.reject { |index| IGNORED_MODEL_NAMES.include?(index.model.name) }
  end
end

This approach should allow the base implementation to be updated reasonably safely.

paulleader avatar Mar 28 '19 21:03 paulleader