Skip to content

Counter cache columns are not marked as readonly [skip ci] #51450

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 6, 2024

Conversation

fatkodima
Copy link
Member

@fatkodima fatkodima commented Mar 29, 2024

Counter caches were marked as readonly a very long time ago (rails 2),

module_eval(
"#{reflection.class_name}.send(:attr_readonly,\"#{cache_column}\".intern) if defined?(#{reflection.class_name}) && #{reflection.class_name}.respond_to?(:attr_readonly)"
)
but this is no longer true.

I was confused by this before, but I do not remember what I tested last time that it looked correct for me 🤦 😅 #44950

Reproducible test

# 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"
require "minitest/autorun"
require "logger"

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord.raise_on_assign_to_attr_readonly = true

ActiveRecord::Schema.define do
  create_table :posts, force: true do |t|
    t.integer :comments_count
  end

  create_table :comments, force: true do |t|
    t.integer :post_id
  end
end

class Post < ActiveRecord::Base
  has_many :comments
end

class Comment < ActiveRecord::Base
  belongs_to :post, counter_cache: true
end

class BugTest < Minitest::Test
  def test_counter_cache
    post = Post.create!
    post.comments.create!

    assert Post.readonly_attributes.any?  # <---- fails

    assert_raises do  # <---- will also fail
      post.update!(comments_count: 10)
    end
  end
end

cc @jonathanhefner

@skipkayhil
Copy link
Member

Changed here: 7b6720d#diff-084ec489ba3b9f2e388734a99876f0d94776111278acfe4901235dca570040c5R40 so that assigning values to a counter_cache column would not raise (I wonder if they should raise though 🤔)

@fatkodima fatkodima force-pushed the remove-stale-comment branch 2 times, most recently from 8222308 to 7e3e830 Compare March 30, 2024 19:47
@rails-bot rails-bot bot added the docs label Mar 30, 2024
@fatkodima fatkodima force-pushed the remove-stale-comment branch from 7e3e830 to bb779f9 Compare April 6, 2024 09:39
@fatkodima fatkodima merged commit bad7ff1 into rails:main Apr 6, 2024
@fatkodima fatkodima deleted the remove-stale-comment branch April 6, 2024 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants