Skip to content

Commit

Permalink
Raise a descriptive error from store_accessor if the column is not …
Browse files Browse the repository at this point in the history
…a Store

If a developer has neglected to use a structured column type (hstore
or json) or to declare a serializer with `ActiveRecord.store`:

```ruby
class User < ActiveRecord::Base
  store_accessor :settings, :notifications
end
```

then a `ConfigurationError` will now be raised by `store_accessor`
with a descriptive error message:

    ActiveRecord::ConfigurationError: the column 'settings' has not
    been configured as a store.  Please make sure the column is
    declared serializable via 'ActiveRecord.store' or, if your
    database supports it, use a structured column type like hstore or
    json.

Previously, in this situation, a `NoMethodError` was raised when the
accessor was read or written:

```ruby
puts user.notifications
```

Raising an exception earlier, with a helpful message, should help
developers understand more quickly what's wrong and how to fix it.

Note that this change makes `store_accessor` call `type_for_attribute`
which will potentially load the model's schema where it was not being
loaded before. As a result, some small changes were needed to the test
suite where the `json_data_type` table was not created early enough,
and where an anonymous class needed to be bound to a real table. I
don't think this should affect the majority of Store users.

Closes rails#51699
  • Loading branch information
flavorjones committed May 23, 2024
1 parent 43e4916 commit aeedd35
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 0 deletions.
16 changes: 16 additions & 0 deletions activerecord/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,19 @@
* Improve `ActiveRecord.store_accessor` to raise an exception if the column is not either
structured (e.g., PostgreSQL +hstore+/+json+, or MySQL +json+) or declared serializable via
`ActiveRecord.store`.

Previously, an exception would be raised late when the accessor was read or written:

NoMethodError: undefined method `accessor' for an instance of ActiveRecord::Type::Text

Now, an exception is raised early when `store_accessor` is called:

ActiveRecord::ConfigurationError: the column 'metadata' has not been configured as a store.
Please make sure the column is declared serializable via 'ActiveRecord.store' or, if your
database supports it, use a structured column type like hstore or json.

*Mike Dalessio*

* Fix inference of association model on nested models with the same demodularized name.

E.g. with the following setup:
Expand Down
4 changes: 4 additions & 0 deletions activerecord/lib/active_record/store.rb
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,10 @@ def store(store_attribute, options = {})
end

def store_accessor(store_attribute, *keys, prefix: nil, suffix: nil)
unless type_for_attribute(store_attribute).respond_to?(:accessor)
raise ConfigurationError, "the column '#{store_attribute}' has not been configured as a store. Please make sure the column is declared serializable via 'ActiveRecord.store' or, if your database supports it, use a structured column type like hstore or json."
end

keys = keys.flatten

accessor_prefix =
Expand Down
6 changes: 6 additions & 0 deletions activerecord/test/cases/json_shared_test_cases.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@
require "support/schema_dumping_helper"
require "pp"

# ensure we have a "json_data_type" table when JsonDataType is defined,
# because store_accessor checks the column for a store accessor.
ActiveRecord::Base.lease_connection.create_table("json_data_type") do |t|
t.json "settings"
end

module JSONSharedTestCases
include SchemaDumpingHelper

Expand Down
14 changes: 14 additions & 0 deletions activerecord/test/cases/store_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -285,9 +285,13 @@ def test_convert_store_attributes_from_Hash_to_HashWithIndifferentAccess_saving_

test "stored_attributes are tracked per class" do
first_model = Class.new(ActiveRecord::Base) do
self.table_name = "admin_users"
store :data
store_accessor :data, :color
end
second_model = Class.new(ActiveRecord::Base) do
self.table_name = "admin_users"
store :data
store_accessor :data, :width, :height
end

Expand All @@ -297,6 +301,8 @@ def test_convert_store_attributes_from_Hash_to_HashWithIndifferentAccess_saving_

test "stored_attributes are tracked per subclass" do
first_model = Class.new(ActiveRecord::Base) do
self.table_name = "admin_users"
store :data
store_accessor :data, :color
end
second_model = Class.new(first_model) do
Expand Down Expand Up @@ -359,4 +365,12 @@ def test_convert_store_attributes_from_Hash_to_HashWithIndifferentAccess_saving_
test "prefix/suffix do not affect stored attributes" do
assert_equal [:secret_question, :two_factor_auth, :login_retry], Admin::User.stored_attributes[:configs]
end

test "store_accessor raises an exception if the column is not either serializable or a structured type" do
assert_raises ActiveRecord::ConfigurationError do
Class.new(Admin::User) do
store_accessor :name, :color
end
end
end
end

0 comments on commit aeedd35

Please sign in to comment.