STI (Single Table Inheritance) Label Tags with Same Name Cause Type Mismatch and Comparison Errors

[iberianpig]

Summary

When using acts-as-taggable-on with STI labels that have the same name across different types, the gem incorrectly associates tags from different STI classes, leading to type mismatches and comparison failures.

Environment

• acts-as-taggable-on: v5.0.0 ~ v12.0.0
• Rails: 5-2-stable ~ 8.0-stable
• Ruby: 2.7.8 ~ 3.4.1

Steps to Reproduce

1. Create STI label classes:
   • Label < ActsAsTaggableOn::Tag
   • Label::Article < Label
   • Label::User < Label
2. Create labels with the same name in different STI types:
   • Label::Article.create(name: "label2")
   • Label::User.create(name: "label2")
3. Assign labels to an Article model and call label_list
4. Save the article
5. The wrong label type (Label::User instead of Label::Article) gets saved

Expected Behavior

• Articles should only be associated with Label::Article instances
• Calling article.labels.sort should work without errors

Actual Behavior

• After calling label_list and saving, Label::User instances are incorrectly associated with Article
• Attempting to sort labels raises: ArgumentError: comparison of Label::Article with Label::User failed

Root Cause

The default behavior of acts-as-taggable-on's find_or_create_tags_from_list_with_context method searches from ActsAsTaggableOn::Tag base class, which can return the wrong STI type when multiple records with the same name exist.

Reproduction Code

• save following code as issue_incorrect_tags_with_sti.rb for a minimal reproducible example.
• execute $ ruby issue_incorrect_tags_with_sti.rb in terminal.

# frozen_string_literal: true

begin
  require "bundler/inline"
rescue LoadError => e
  $stderr.puts "Bundler version 1.10 or later is required. Please update your Bundler"
  raise e
end
gemfile = File.read(File.expand_path("Gemfile", __dir__)).strip
gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }
  gem "logger"

  # gem "rails", github: "rails/rails", branch: "5-1-stable"
  # gem "rails", github: "rails/rails", branch: "5-2-stable"
  # gem "rails", github: "rails/rails", branch: "6-0-stable"
  # gem "rails", github: "rails/rails", branch: "6-1-stable"
  # gem "rails", github: "rails/rails", branch: "7-0-stable"
  # gem "rails", github: "rails/rails", branch: "7-1-stable"
  # gem "sqlite3", '~> 1.4.4'
  
  gem "rails", github: "rails/rails", branch: "8-0-stable"
  gem "sqlite3", '~> 2.1.0'
  gem 'debug'
  gem 'pry-byebug'

  gem 'stringio'

  gem 'acts-as-taggable-on', path: '.'
end

require "active_record"
require "minitest/autorun"
require "logger"

# Database connection setup
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

# Run acts-as-taggable-on migrations
ActiveRecord::Schema.define do
  # Create acts-as-taggable-on tables
  create_table :tags, force: true do |t|
    t.string :name
    t.integer :taggings_count, default: 0
    t.string :type
  end
  add_index :tags, ['name'], name: 'index_tags_on_name' # , unique: true

  create_table :taggings, force: true do |t|
    t.integer :tag_id

    # You should make sure that the column created is
    # long enough to store the required class names.
    t.string :taggable_type
    t.integer :taggable_id

    t.string :tagger_type
    t.integer :tagger_id

    # Limit is created to prevent MySQL error on index
    # length for MyISAM table type: http://bit.ly/vgW2Ql
    t.string :context, limit: 128

    t.string :tenant , limit: 128

    t.datetime :created_at
  end

  add_index :taggings,
            ['tag_id', 'taggable_id', 'taggable_type', 'context', 'tagger_id', 'tagger_type'],
            unique: true, name: 'taggings_idx'
  add_index :taggings, :tag_id , name: 'index_taggings_on_tag_id'

  # Test model tables
  create_table :articles, force: true do |t|
    t.string :title
    t.text :content
    t.timestamps

    t.text :cached_label_list
  end

  create_table :users, force: true do |t|
    t.string :name
    t.timestamps
  end
end

module ActsAsTaggableOnStiLabelContexts
  extend ActiveSupport::Concern

  private

  # Always search from the label class associated with the model
  #
  # This method overrides ActsAsTaggableOn's default behavior
  # ActsAsTaggableOn's default behavior SELECTs from ActsAsTaggableOn::Tag,
  # which can incorrectly select a different label class (with smaller record ID)
  # when multiple Label records with the same name exist.
  # Therefore, we override to search from the model-specific label class.
  #
  # see: https://github.com/mbleigh/acts-as-taggable-on/blob/v8.1.0/lib/acts_as_taggable_on/taggable/core.rb?plain=1#L329
  #
  # @param tag_list [Array<String>] List of tags, e.g. ["label1", "label2"]
  # @param context [Symbol] Context name, e.g. :labels (from acts_as_taggable_on :labels)
  def find_or_create_tags_from_list_with_context(tag_list, context)
    if context.to_sym == :labels
      label_klass = "Label::#{self.class}".constantize
      label_klass.find_or_create_all_with_like_by_name(tag_list)
    else
      super
    end
  end
end

# Model definitions
class Article < ActiveRecord::Base
  # following patch to fix the issue
  # prepend ActsAsTaggableOnStiLabelContexts

  acts_as_taggable_on :labels
end

class User < ActiveRecord::Base
  acts_as_taggable_on :labels
end

class Label < ActsAsTaggableOn::Tag
  # Monkey patch to allow labels with same name
  # => eval error: Validation failed: Name has already been taken
  def validates_name_uniqueness?
    false
  end
end

class Label::Article < Label
end

class Label::User < Label
end

# Test class
class ActsAsTaggableOnTest < Minitest::Test
  def setup
    @article = Article.find_or_create_by!(title: "Ruby on Rails", content: "Great framework")
    @user = User.find_or_create_by!(name: "Test User")
  end

  def test_same_name_labels_and_sort
    article_label1 = Label::Article.find_or_create_by!(name: "label1")
    # Add label with same name
    user_label2 = Label::User.find_or_create_by!(name: "label2")
    article_label2 = Label::Article.find_or_create_by!(name: "label2")

    @article.labels << article_label1
    @article.labels << article_label2

    @article.label_list # When label_list is called, subsequent save will incorrectly save user_label2 with same name: "label2"

    @article.save # Will be saved as "article_label1, user_label2"

    @article.reload

    assert_kind_of Label::Article, @article.labels.second
    # Expected #<Label::User id: 2, name: "label2", taggings_count: 1, type: "Label::User"> to be a kind of Label::Article, not Label::User.

    err = nil
    begin
      @article.labels.sort
    rescue ArgumentError => err
      assert_nil err
      # Expected #<ArgumentError: comparison of Label::Article with Label::User failed> to be nil.
    end
  end
end

Workaround

The issue can be fixed by prepending a module that overrides the tag finding behavior to search from the correct STI class:

module ActsAsTaggableOnStiLabelContexts
  private
  
  def find_or_create_tags_from_list_with_context(tag_list, context)
    if context.to_sym == :labels
      label_klass = "Label::#{self.class}".constantize
      label_klass.find_or_create_all_with_like_by_name(tag_list)
    else
      super
    end
  end
end

class Article < ActiveRecord::Base
  prepend ActsAsTaggableOnStiLabelContexts
  acts_as_taggable_on :labels
end

Additional Information

This issue affects applications using STI for tag organization where different models need separate tag namespaces but may share tag names.