Non-polymorphic owners configurable?

[mediafinger]

First of all: thx for releasing this gem! 🔐

I had a first look and saw these lines in the generated migration:

      t.index :owner_id if foreign_key_type # Index owner_id only if it's a separate column
      t.index :owner_type if foreign_key_type # Index owner_type only if it's a separate column

which kinda make it look like you planned to make it configurable? But the migration also contains:

      # Who owns this key? Can be null for ownerless keys.
      t.references :owner, polymorphic: true, null: true, type: foreign_key_type

and the ApiKey class:

belongs_to :owner, polymorphic: true, optional: true

In my use case I would only allow my Organization model to be an owner of api_keys - and expect every ApiKey to have an owner. I would prefer to have a non-polymorphic association to take advantage of a foreign_key.

I assume in the gem's code base the owner is usually accessed over the activerecord association - meaning it would behave the same way for a polymorphic and non-polymorphic relationship in all(?) use cases. So probably having this configurable would be a smaller code change?

I'm not 100% sure what would be the best way to define the belongs_to relationship depending if the implementation uses polymorphic or not. What is your take on this?

[rameerez]

Hi @mediafinger, thanks a lot for checking out my gem!

So I decided to go for a polymorphic relationship to minimize setup and config. Anything can own API keys easily through the model concern, just drop the has_api_keys and it's done. That's kinda the Rails magic I was aiming for, works for pretty much any use case.

Sure, this could be done differently / non-polymorphic (for example, if we define a config option like ApiKeys::Configuration.key_owner_class = Organizaition or something, but then the migration and the model mixin become much more complex with little added benefit for the average gem user I think.

I do see your point though, you may wanna enforce your API keys always belong to a valid organization etc. I think you can do that easily by monkey-patching the ApiKey class, something like:

Rails.application.config.to_prepare do
  ApiKeys::ApiKey.class_eval do
    validates :owner, presence: true
    end
  end
end

You could make an initializer for this for example. Then you could also add more checks like ensuring the owner is always an Organization etc. like:

validate :owner_must_be_organization

def owner_must_be_organization
  unless owner.is_a?(Organization)
    errors.add(:owner_type, "must be an Organization")
  end
end

Not sure if this answers your question! My tl;dr would be that I want the gem to work as smoothly as possible for the median case and that means being a bit opinionated and providing defaults I think are sensible, adding configuration options like this would probably cause many people not to get the gem working first try due to misconfig