djezzzl/database_consistency

Idea: checker for unique index for has_one associations

tdeo opened this issue · 5 comments

tdeo commented

Currently there exists 2 things in database_consistency:

  • a MissingUniqueIndexChecker that checks that uniqueness validations have a unique index in the DB
  • a MissingIndexChecker that checks that an index exists for has_one and has_many associations

I think in the has_one case, we should enforce that there should be a unique index on exactly the columns of the associations, so that the DB guarantees that this has_one remains a has_one and doesn't become a has_many.

I believe it would be hard to integrate this feature in MissingUniqueIndexChecker as this one inherits ValidatorChecker, and I'm unsure if it should be part of MissingIndexChecker or split this one into two checkers for has_many (which the current behavior) and for has_one (which would be even stricter).

CC @frantisekrokusek

Let me know what you think and which path you'd rather have this implemented, thanks

djezzzl commented

Hi @tdeo,

Thank you for using the gem and contributing to it! And sorry for me taking too long to response.

I must say that I really love your suggestion. Actually, we even had an issue recently related to that. So I'm fully up to the idea!

I think it's okay to have as many checks as needed keeping their logic and purpose easy to follow and isolated.
However, if in this case, extending MissingIndexChecker looks good and doesn't break it responsibility, we may adjust just that.

Do you want to contribute with the PR or you want me to do that? I think I may have some time this weekend to do that. But please let me know about your decision before this weekend so we don't do same PR together.

djezzzl commented

Hi @tdeo,

I've released 1.1.13 with this feature: PR. For now, it only supports unscoped has_one associations. Please have a look and let me know if it doesn't work for you.

Also, please feel free to share more ideas, I'd be happy to add them.

I will close this issue but free to reopen it if needed.

Have a great day!

djezzzl commented

Hey @tdeo,

Version 1.1.13 had an issue that was already fixed. Please try 1.1.14 instead.

tdeo commented

Hello,

Thanks for the quick release ! Seems to be working pretty well on our end.

On a side-note, I believe you already implemented support for polymorphic associations with this line: https://github.com/djezzzl/database_consistency/blob/master/lib/database_consistency/checkers/association_checkers/missing_index_checker.rb#L71, so I don't see any reason why to skip if association[:as].

Sorry for not being more helpful, but setting up the dev environment is a bit cumbersome with the rest of my local dev environment 😅

djezzzl commented

Hello @tdeo,

Thanks for the quick release ! Seems to be working pretty well on our end.

Glad to hear that!

On a side-note, I believe you already implemented support for polymorphic associations with this line: https://github.com/djezzzl/database_consistency/blob/master/lib/database_consistency/checkers/association_checkers/missing_index_checker.rb#L71, so I don't see any reason why to skip if association[:as].

Unfortunately, we can't enforce unique indexes for polymorphic associations that simple as when some object may have has_one association with polymoprhic, another may have has_many.

For example,

class Payment
  belongs_to :subject, polymorphic: true
end

class User
  has_many :payments, as: :subject
end

class ReferralBonus
  has_one :payment
end

However, it is still doable to achieve but with some workarounds I plan to add later.

Sorry for not being more helpful, but setting up the dev environment is a bit cumbersome with the rest of my local dev environment 😅

No worries at all. You already helped a lot with great suggestion!

BTW, I have some suggestions for you to try (I believe they might be good additions to your projects):

  • https://github.com/djezzzl/n1_loader in case you build GraphQL API in Ruby. This is a must have in case you won't to get rid of N+1 issues easily. Have a look and check it yourself. It has proven itself as very efficient and stable in production code.
  • https://github.com/djezzzl/factory_trace in case you're using FactoryBot. This helped me to clean 10% of unused factories in one of projects easily.

Have a great weekend! Feel free to ping me in case of any questions!