Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rails 5.2 ActiveStorage Introduces Unique Index Violation on active_storage_attachments #42

Open
olliebennett opened this issue Jun 18, 2018 · 7 comments

Comments

@olliebennett
Copy link

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).

@hidde-jan
Copy link

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

@olliebennett
Copy link
Author

olliebennett commented Jul 24, 2018

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.

@trptcolin
Copy link
Owner

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. #34)

@KaisHaddadin
Copy link

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

@paulleader
Copy link

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

@olliebennett
Copy link
Author

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]

@paulleader
Copy link

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants