rails icon indicating copy to clipboard operation
rails copied to clipboard

Rails 7.1: `ActiveRecord::Base#query_constraints` and composite keys are incompatible with ActiveStorage

Open Slotos opened this issue 3 months ago • 1 comments

Steps to reproduce

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

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

  gem "rails"
  # If you want to test against edge Rails replace the previous line with this:
  # gem "rails", github: "rails/rails", branch: "main"

  gem "sqlite3"
end

require "active_record/railtie"
require "active_storage/engine"
require "tmpdir"

class TestApp < Rails::Application
  config.load_defaults Rails::VERSION::STRING.to_f

  config.root = __dir__
  config.hosts << "example.org"
  config.eager_load = false
  config.session_store :cookie_store, key: "cookie_store_key"
  config.secret_key_base = "secret_key_base"

  config.logger = Logger.new($stdout)
  Rails.logger  = config.logger

  config.active_storage.service = :local
  config.active_storage.service_configurations = {
    local: {
      root: Dir.tmpdir,
      service: "Disk"
    }
  }
end

ENV["DATABASE_URL"] = "sqlite3::memory:"

Rails.application.initialize!

require ActiveStorage::Engine.root.join("db/migrate/20170806125915_create_active_storage_tables.rb").to_s

ActiveRecord::Schema.define do
  CreateActiveStorageTables.new.change

  create_table :users, force: true do |t|
    t.bigint :tenant_id, null: false
    t.index [:tenant_id, :id], unique: true
  end

  create_table :cats, primary_key: [:breed, :color], force: true do |t|
    t.string :color, null: false
    t.string :breed, null: false
  end
end

class User < ActiveRecord::Base
  query_constraints :tenant_id, :id

  has_one_attached :profile
end

class Cat < ActiveRecord::Base
  has_one_attached :profile
end

require "minitest/autorun"

class BugTest < Minitest::Test
  def test_upload_and_download_query_constraint
    user = User.create!(
      tenant_id: 42,
      profile: {
        content_type: "text/plain",
        filename: "dummy.txt",
        io: ::StringIO.new("dummy"),
      }
    )

    assert_equal "dummy", user.profile.download
  end

  def test_upload_and_download_primary_key
    cat = Cat.create!(
      breed: "cat",
      color: "orange",
      profile: {
        content_type: "text/plain",
        filename: "dummy.txt",
        io: ::StringIO.new("dummy"),
      }
    )

    assert_equal "dummy", cat.profile.download
  end
end

Expected behavior

Since ActiveStorage storage tables are not configurable, it should simply work. Attaching a file should result in an attached file.

Actual behavior

query_constraints route results in an attempt to query non-existing columns on active_storage_attachments. Redefining relations on a model doesn't help, since inverse belong_to association then attempts to compare two columns to one in ActiveRecord::AutosaveAssociation#compute_primary_key.

Composite primary key route doesn't work, unless the key contains :id column.

Notes

This is kind-of solvable by adding exttra columns to active_storage_attachments, but that can get out of hand real fast.

Incidentally, allowing belongs_to to accept an array primary_key, would solve the problem by letting the record have a regular id for use by ActiveSupport, but configure relations between models using composite keys. Right now, this route doesn't work due to ActiveRecord::Reflection::BelongsToReflection#association_primary_key transforming primary_key option with to_s.

System configuration

Rails version: 7.1.3

Ruby version: 3.3.0

Slotos avatar Feb 19 '24 16:02 Slotos