Multitenancy

[airblade]

Hi Pat,

I have patched Gutentag to support multitenancy, i.e. a self-contained set of tags for each account in my Rails app, and I thought you might like to see how. It's been running happily in production for a few weeks.

First I added a scoping column to Gutentag::Tag which I called :tenant_id. This involved modifying the migration:

create_table :gutentag_tags do |t|
  t.integer :tenant_id, :null => false  # added this line

  t.string :name, :null => false
  t.timestamps :null => false
end

add_index :gutentag_tags, [:name, :tenant_id], :unique => true  # changed this line

Then I patched the class to take :tenant_id into account:

# config/initializers/gutentag.rb
Gutentag::Tag.class_eval do
  attr_accessible :tenant_id if ActiveRecord::VERSION::MAJOR == 3

  scope :by_tenant_id, ->(tenant_id) { where tenant_id: tenant_id }

  # Change uniqueness validation to act within a scope.
  #
  # We cannot modify the existing one in place so we:
  #
  #1) remove it (works on Rails 3.2; not tested on Rails 4)
  _validators[:name].reject! { |v| v.is_a?(ActiveRecord::Validations::UniquenessValidator) }

  _validate_callbacks.reject! do |callback|
    callback.raw_filter.is_a?(ActiveRecord::Validations::UniquenessValidator) &&
      callback.raw_filter.attributes == [:name]
  end

  #2) add a new one
  validates :name, uniqueness: {case_sensitive: false, scope: :tenant_id}

  def self.find_by_name_and_tenant_id(name, tenant_id)
    where(tenant_id: tenant_id).find_by_name(name)
  end

  def self.find_or_create_by_name_and_tenant_id(name, tenant_id)
    find_by_name_and_tenant_id(name, tenant_id) || create(name: name, tenant_id: tenant_id)
  end
end

The only tricky part was updating the uniqueness validator to scope by :tenant_id.

Then I needed to tell Gutentag::Persistence to use a custom tagger which is aware of a tenant's scope. Unfortunately this was also a little tricky because it's only ever instantiated inside the after_save callback set up by the has_many_tags method – which is difficult to reach inside.

I created a module which a taggable model can include instead of calling has_many_tags:

# The taggable must respond to `#tenant_id`.
#
# Alternatively we could change the signature of `#has_many_tags` to take a block, which would
# be a lambda version of a taggable's current `#tenant_id` method.  Then the `after_save` callback
# could get the `tenant_id` by calling the block we passed in.
module Taggable
  def self.included(base)
    base.extend ClassMethods
    base.has_many_tags  # bootstrap
  end

  module ClassMethods
    # I need to inject my own tagger into Gutentag::Persistence so I can use the tenant_id when
    # finding or creating tags.  Although Gutentag::Persistence supports an injectable tagger,
    # the way it is instantiated in the after_save callback doesn't give any opportunity to inject
    # a tagger.
    #
    # I can't find a way to skip the after_save callback on Gutentag::ActiveRecord because it is
    # defined inline as a lambda.  Were it a symbol/instance method, it ought to be skippable so:
    #
    #   skip_callback :save, :after, :persist
    #
    # Instead I copy-paste the non-callback parts of Gutentag::ActiveRecord::ClassMethods#has_many_tags
    # and add my own after_save callback.
    def has_many_tags
      has_many :taggings, :class_name => 'Gutentag::Tagging', :as => :taggable,
        :dependent => :destroy
      has_many :tags,     :class_name => 'Gutentag::Tag',
        :through => :taggings

      after_save do |instance|
        persister = Gutentag::Persistence.new(instance)
        persister.tagger = TenantTagger.new(instance.tenant_id)
        persister.persist
      end
    end
  end

  def tenant_id
    raise NotImplementedError, 'taggable must implement'
  end
end

Finally, here is the custom tagger:

class TenantTagger
  def initialize(tenant_id)
    @tenant_id = tenant_id
  end

  def find_by_name(name)
    Gutentag::Tag.find_by_name_and_tenant_id(name, tenant_id)
  end

  def find_or_create(name)
    Gutentag::Tag.find_or_create_by_name_and_tenant_id(name, tenant_id)
  end

  private

  attr_reader :tenant_id
end

Client code can use it like this:

class Foo < ActiveRecord::Base
  belongs_to :account  # for example

  include Taggable

  def tenant_id
    account.id
  end
end

Overall I was pleased by how little I had to change conceptually. The implementation was a little messy because it isn't straightforward to modify validators or after-save callbacks.

I know you don't necessarily want to support multitenancy, but I wonder whether we could rearrange Gutentag's code at all, wihtout changing its function, to make extending for multitenancy easier?

Thanks for such a cleanly and clearly written gem!

Cheers,
Andy

Cross-ref: #9.

[pat]

Hey Andy

Been mulling over this on and off, and finally got a chance to do something about it. I've not taken your code wholesale into the gem - am still loath to have official support for multi-tenancy in Gutentag itself - but there are two changes which should help your modifications:

Via 0cbc601 the after_save callback in a taggable model has changed from a Proc to a private method (persist_tags), so you can customise that behaviour far more easily.

Via 51bd8f2 you can customise the validations being added to the Gutentag::Tag model by setting Gutentag.tag_validations to something that responds to call (a Proc, a class, an instance) and takes a single argument, which is the Gutentag::Tag class. This will remove the need for you to remove the default validations.

Gutentag.tag_validations = lambda { |klass|
  klass.validates :name, presence: true,
    uniqueness: {case_sensitive: false, scope: :tenant_id}
}

That second change is only Rails-friendly, due to Rails' lazy loading of models. It will not work with Sinatra or other frameworks (though I'm certainly welcome to feedback on how it could be better structured to do so).

These should help clean up your code a bit… are there other aspects that you feel could be managed better to allow your modifications to happen more cleanly?

[airblade]

Hey Pat,

Those two changes greatly clean up my code – thanks! I was able to delete 36 lines of ugly customisation :)

The only other modification I wonder about is how best to add scopes to Gutentag::Tag. Currently I class eval them like this:

Gutentag::Tag.class_eval do
  scope :by_tenant_id, ->(tenant_id) { where tenant_id: tenant_id }
end

Would you do it this way or another way?

[pat]

scope is a public method (at least, it is in ActiveRecord 4.1), so it's possible to write that scope like so:

Gutentag::Tag.scope :by_tenant_id, ->(tenant_id) { Gutentag::Tag.where tenant_id: tenant_id }

The catch with that is that I'm not sure it would work appropriately if chained on top of another scope. Not the end of the world, but given it then becomes less obvious/normal, I'd probably go with the class_eval call instead. At this point, I can't think of any way to abstract this behaviour out neatly so it's easier to inject.

[airblade]

That's all good to know, thanks.

I can't see any further way to simplify my multitenancy changes. For the sake of completeness, here they are:

Gutentag.tag_validations = lambda { |klass|
  klass.validates :name,
    presence: true,
    uniqueness: {case_sensitive: false, scope: :tenant_id}
}

Gutentag::Tag.class_eval do
  attr_accessible :tenant_id if ActiveRecord::VERSION::MAJOR == 3

  scope :by_tenant_id, ->(tenant_id) { where tenant_id: tenant_id }

  def self.find_by_name_and_tenant_id(name, tenant_id)
    where(tenant_id: tenant_id).find_by_name(name)
  end

  def self.find_or_create_by_name_and_tenant_id(name, tenant_id)
    find_by_name_and_tenant_id(name, tenant_id) || create(name: name, tenant_id: tenant_id)
  end
end

module Taggable
  def self.included(base)
    base.has_many_tags
  end

  def tenant_id
    raise NotImplementedError, 'taggable must implement'
  end

  private

  # Override Gutentag::ActiveRecord#persist_tags so I can use a `tenant_id`-aware tagger.
  def persist_tags
    persister = Gutentag::Persistence.new(self)
    persister.tagger = TenantTagger.new(tenant_id)
    persister.persist
  end

  class TenantTagger
    def initialize(tenant_id)
      @tenant_id = tenant_id
    end

    def find_by_name(name)
      Gutentag::Tag.find_by_name_and_tenant_id(name, tenant_id)
    end

    def find_or_create(name)
      Gutentag::Tag.find_or_create_by_name_and_tenant_id(name, tenant_id)
    end

    private

    attr_reader :tenant_id
  end
end

Thanks for all the help!

[airblade]

I extracted this into a gem: gutentag-multitenancy.

Thanks again for the help!

[pat]

Great stuff! :)