From b281b01f780364a217439fc60bc4931da4265150 Mon Sep 17 00:00:00 2001 From: Nick Zadrozny Date: Wed, 12 Aug 2026 12:09:05 -0500 Subject: [PATCH 1/4] Wrap an optional belongs_to declared after the include MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The concern only wrapped the associations a class had already declared, so the conventional placement of a concern — at the top of the model, above its associations — silently produced a half-converted model: nullable columns wrapped, associations not, and no signal that it had happened. Hooking belongs_to the way encrypts is already hooked makes placement irrelevant. Wrapping now runs through one path, which also lets errgonomic_optional_except name an association and not just an attribute. --- README.md | 2 +- .../rails/active_record_optional.rb | 69 +++++++++++-------- test/rails_test.rb | 35 ++++++++++ 3 files changed, 78 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index 45e88e7..b0747af 100644 --- a/README.md +++ b/README.md @@ -201,7 +201,7 @@ end When `Rails::Railtie` is defined, Errgonomic installs a Railtie with two opt-in integrations for ActiveRecord: -- `include Errgonomic::Rails::ActiveRecordOptional` in a model makes its nullable attributes and `optional: true` associations return `Some(value)` or `None()` instead of a value-or-nil. Every nullable column and optional association is wrapped, with no per-attribute opt-in. Two kinds of attribute stay unwrapped: those declared with `encrypts`, whose surrounding machinery reads the raw value, and those named by `errgonomic_optional_except`, which must appear before the include. +- `include Errgonomic::Rails::ActiveRecordOptional` in a model makes its nullable attributes and `optional: true` associations return `Some(value)` or `None()` instead of a value-or-nil. Every nullable column and optional association is wrapped, with no per-attribute opt-in. An `optional: true` association declared after the include is wrapped as it is declared, so the include can sit at the top of the model with the other concerns. Two kinds of attribute stay unwrapped: those declared with `encrypts`, whose surrounding machinery reads the raw value, and those named by `errgonomic_optional_except`, which must appear before the include. ```ruby class Credential < ApplicationRecord diff --git a/lib/errgonomic/rails/active_record_optional.rb b/lib/errgonomic/rails/active_record_optional.rb index e3f4fe4..d7bbe03 100644 --- a/lib/errgonomic/rails/active_record_optional.rb +++ b/lib/errgonomic/rails/active_record_optional.rb @@ -31,39 +31,29 @@ module ActiveRecordOptional included do # ::Rails.logger.debug('ActiveRecordOptional') + @errgonomic_optional_exclusions = + Array(encrypted_attributes).map(&:to_s) + Array(try(:errgonomic_optional_exceptions)).map(&:to_s) optional_associations = reflect_on_all_associations(:belongs_to) .select { |r| r.options[:optional] } .map(&:name) - excluded = Array(encrypted_attributes).map(&:to_s) + Array(try(:errgonomic_optional_exceptions)) - optional_attributes = column_names - .select { |n| column_for_attribute(n).null } - .reject { |n| excluded.include?(n) } - @errgonomic_optionals = (optional_attributes + optional_associations) - @errgonomic_optionals.each do |name| - class_eval <<-RUBY, __FILE__, __LINE__ + 1 - def #{name} - reads = Thread.current[:errgonomic_optional_reads] ||= {} - key = [object_id, :#{name}] - if reads[key] - raise Errgonomic::RecursiveOptionalReadError, - "\#{self.class}##{name} re-entered itself; something beneath this reader reads it again" - end - - reads[key] = true - begin - val = super - ensure - reads.delete(key) - end - val.nil? ? Errgonomic::Option::None.new : Errgonomic::Option::Some.new(val) - end - RUBY - end + optional_attributes = column_names.select { |n| column_for_attribute(n).null } + (optional_attributes + optional_associations).each { |name| errgonomic_wrap_optional(name) } end class_methods do def errgonomic_optionals - @errgonomic_optionals + @errgonomic_optionals ||= [] + end + + def errgonomic_optional_exclusions + @errgonomic_optional_exclusions ||= [] + end + + # A concern belongs at the top of a model, above its associations, so + # an optional belongs_to is routinely declared after the include. + # Wrap it when it arrives, or the conversion is silently partial. + def belongs_to(name, scope = nil, **options) + super.tap { errgonomic_wrap_optional(name) if options[:optional] } end # Encryption surrounds an attribute with machinery that reads the raw @@ -77,11 +67,36 @@ def encrypts(*names, **options) def errgonomic_unwrap_optionals(*names) names.map(&:to_s).each do |name| - next unless @errgonomic_optionals&.delete(name) + next unless errgonomic_optionals.delete(name) remove_method(name) end end + + def errgonomic_wrap_optional(name) + name = name.to_s + return if errgonomic_optional_exclusions.include?(name) || errgonomic_optionals.include?(name) + + errgonomic_optionals << name + class_eval <<-RUBY, __FILE__, __LINE__ + 1 + def #{name} + reads = Thread.current[:errgonomic_optional_reads] ||= {} + key = [object_id, :#{name}] + if reads[key] + raise Errgonomic::RecursiveOptionalReadError, + "\#{self.class}##{name} re-entered itself; something beneath this reader reads it again" + end + + reads[key] = true + begin + val = super + ensure + reads.delete(key) + end + val.nil? ? Errgonomic::Option::None.new : Errgonomic::Option::Some.new(val) + end + RUBY + end end end end diff --git a/test/rails_test.rb b/test/rails_test.rb index fa056b7..a23b8de 100644 --- a/test/rails_test.rb +++ b/test/rails_test.rb @@ -99,6 +99,23 @@ class OptedOutCredential < ActiveRecord::Base include Errgonomic::Rails::ActiveRecordOptional end +# Rails convention puts a concern at the top of a model, above its +# associations, so an optional belongs_to is routinely declared after the +# include. +class LateAssociationBook < ActiveRecord::Base + self.table_name = 'books' + include Errgonomic::Rails::ActiveRecordOptional + belongs_to :author, optional: true +end + +# An opt-out names an association the same way it names an attribute. +class OptedOutBook < ActiveRecord::Base + self.table_name = 'books' + errgonomic_optional_except :author + belongs_to :author, optional: true + include Errgonomic::Rails::ActiveRecordOptional +end + class BugTest < Minitest::Test def test_optional_attributes author = Author.create!(name: 'Cixin Liu') @@ -221,6 +238,24 @@ def test_encrypted_attributes_may_be_absent assert credential.reload.access_secret.nil? end + # Wrapping only what the class already declared makes the include's + # position load-bearing, and a partial conversion is silent. + def test_optional_belongs_to_declared_after_the_include_is_wrapped + author = Author.create!(name: 'Cixin Liu') + book = LateAssociationBook.create!(title: 'The Dark Forest', author_id: author.id) + + assert book.author.some? + assert_equal author.name, book.author.unwrap!.name + assert LateAssociationBook.create!(title: 'The Wandering Earth').author.none? + end + + def test_errgonomic_optional_except_skips_named_associations + author = Author.create!(name: 'Cixin Liu') + book = OptedOutBook.create!(title: 'The Dark Forest', author_id: author.id) + + assert_equal author.name, book.author.name + end + def test_errgonomic_optional_except_skips_named_attributes credential = OptedOutCredential.create!(access_key: 'abc123', access_secret: 'shhh') From a85ae094d94831728cef5b7f74c1e0377f4798b3 Mon Sep 17 00:00:00 2001 From: Nick Zadrozny Date: Wed, 12 Aug 2026 12:16:52 -0500 Subject: [PATCH 2/4] Wrap nullable columns when the schema loads, not at include MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reading column_names in the included block made a model's class body require a live database connection: a class that includes the concern raises DatabaseConnectionError on load, where the same class without it loads fine. Any boot that loads models without a reachable database — an asset build, an image build, a schema check — fails on the include. ActiveRecord already has a seam for this. It defines attribute methods the first time a model needs its schema, and load_schema! is where that happens, so wrapping from there restores ordinary lazy loading. Two consequences follow: encrypts must record its exclusion for a schema that has not arrived yet rather than only reclaiming a reader, and a subclass now reaches the seam a second time, so wrapping has to see the readers an ancestor already wrapped or it would nest them. --- .../rails/active_record_optional.rb | 53 +++++++++++++++---- test/rails_test.rb | 44 +++++++++++++++ 2 files changed, 87 insertions(+), 10 deletions(-) diff --git a/lib/errgonomic/rails/active_record_optional.rb b/lib/errgonomic/rails/active_record_optional.rb index d7bbe03..ddad78e 100644 --- a/lib/errgonomic/rails/active_record_optional.rb +++ b/lib/errgonomic/rails/active_record_optional.rb @@ -33,11 +33,9 @@ module ActiveRecordOptional # ::Rails.logger.debug('ActiveRecordOptional') @errgonomic_optional_exclusions = Array(encrypted_attributes).map(&:to_s) + Array(try(:errgonomic_optional_exceptions)).map(&:to_s) - optional_associations = reflect_on_all_associations(:belongs_to) - .select { |r| r.options[:optional] } - .map(&:name) - optional_attributes = column_names.select { |n| column_for_attribute(n).null } - (optional_attributes + optional_associations).each { |name| errgonomic_wrap_optional(name) } + reflect_on_all_associations(:belongs_to) + .select { |r| r.options[:optional] } + .each { |r| errgonomic_wrap_optional(r.name) } end class_methods do @@ -46,7 +44,38 @@ def errgonomic_optionals end def errgonomic_optional_exclusions - @errgonomic_optional_exclusions ||= [] + @errgonomic_optional_exclusions ||= + if superclass.respond_to?(:errgonomic_optional_exclusions) + superclass.errgonomic_optional_exclusions.dup + else + [] + end + end + + # A reader wrapped by an ancestor is already an Option; a subclass + # that wrapped it again would nest it. + def errgonomic_optional?(name) + return true if errgonomic_optionals.include?(name) + + superclass.respond_to?(:errgonomic_optional?) && superclass.errgonomic_optional?(name) + end + + # ActiveRecord defines its attribute methods the first time a model + # needs its schema, not when the class body runs. Wrapping nullable + # columns from the same seam keeps a database out of class loading. + def load_schema! + super + errgonomic_wrap_nullable_columns + end + + # A subclass loads its own schema, so whichever of the two is touched + # first wraps the shared columns first, and a subclass that got there + # first would wrap its parent's readers a second time. Walk the chain + # from the top down instead, so an ancestor's readers always exist + # before a subclass considers the same name. + def errgonomic_wrap_nullable_columns + superclass.errgonomic_wrap_nullable_columns if superclass.respond_to?(:errgonomic_wrap_nullable_columns) + column_names.each { |name| errgonomic_wrap_optional(name) if column_for_attribute(name).null } end # A concern belongs at the top of a model, above its associations, so @@ -59,10 +88,14 @@ def belongs_to(name, scope = nil, **options) # Encryption surrounds an attribute with machinery that reads the raw # value, including a length validator that calls to_s on it, so a # wrapped encrypted attribute cannot be saved. Declaring encrypts - # after the include is the ordinary spelling, so catch it here too and - # give the attribute its plain reader back. + # after the include is the ordinary spelling, so record the exclusion + # for whenever the schema arrives, and take back any reader already + # wrapped. def encrypts(*names, **options) - super.tap { errgonomic_unwrap_optionals(*names) } + super.tap do + errgonomic_optional_exclusions.concat(names.map(&:to_s)) + errgonomic_unwrap_optionals(*names) + end end def errgonomic_unwrap_optionals(*names) @@ -75,7 +108,7 @@ def errgonomic_unwrap_optionals(*names) def errgonomic_wrap_optional(name) name = name.to_s - return if errgonomic_optional_exclusions.include?(name) || errgonomic_optionals.include?(name) + return if errgonomic_optional_exclusions.include?(name) || errgonomic_optional?(name) errgonomic_optionals << name class_eval <<-RUBY, __FILE__, __LINE__ + 1 diff --git a/test/rails_test.rb b/test/rails_test.rb index a23b8de..6622dc9 100644 --- a/test/rails_test.rb +++ b/test/rails_test.rb @@ -32,6 +32,12 @@ t.timestamps end + create_table 'magazines', force: :cascade do |t| + t.string :title, null: false + t.string :issn + t.timestamps + end + create_table 'credentials', force: :cascade do |t| t.string :access_key, limit: 255 t.string :access_secret, limit: 255 @@ -116,6 +122,10 @@ class OptedOutBook < ActiveRecord::Base include Errgonomic::Rails::ActiveRecordOptional end +# A subclass has its own schema state, so it reaches the wrapping seam a +# second time for columns its parent already wrapped. +class Novel < Book; end + class BugTest < Minitest::Test def test_optional_attributes author = Author.create!(name: 'Cixin Liu') @@ -238,6 +248,40 @@ def test_encrypted_attributes_may_be_absent assert credential.reload.access_secret.nil? end + # ActiveRecord loads a model's schema on first use, not at definition, so a + # class body must not need a database. Wrapping at include time did, and + # any boot that loads models without a reachable database — an asset build, + # an image build, a schema check — then fails on the include. + def test_including_the_concern_does_not_reach_for_the_schema + statements = [] + subscription = ActiveSupport::Notifications.subscribe('sql.active_record') do |*, payload| + statements << payload[:sql] + end + + magazine = Class.new(ActiveRecord::Base) do + def self.name = 'Magazine' + self.table_name = 'magazines' + include Errgonomic::Rails::ActiveRecordOptional + end + + ActiveSupport::Notifications.unsubscribe(subscription) + + assert_empty statements + assert magazine.create!(title: 'Nature').issn.none? + end + + # An inherited reader is already wrapped, and a second wrap nests: Some of + # a Some for a present value, while an absent one collapses back to None + # because None answers nil?. Half of it is silent. + def test_a_subclass_reads_its_inherited_wrapped_attributes_once + author = Author.create!(name: 'Cixin Liu') + novel = Novel.create!(title: 'Death\'s End', author_id: author.id, isbn: '9780765377104') + + assert_equal '9780765377104', novel.isbn.unwrap! + assert_equal author.id, novel.author.unwrap!.id + assert Novel.create!(title: 'Supernova Era').isbn.none? + end + # Wrapping only what the class already declared makes the include's # position load-bearing, and a partial conversion is silent. def test_optional_belongs_to_declared_after_the_include_is_wrapped From 281145581687f575ce50edfb77a18874bc2b99bb Mon Sep 17 00:00:00 2001 From: Nick Zadrozny Date: Wed, 12 Aug 2026 12:35:52 -0500 Subject: [PATCH 3/4] Report the wrapped set for the columns too Wrapping the columns at schema load left errgonomic_optionals answering with the associations alone until something else happened to touch the model, which is exactly backwards: the set is how a conversion gets checked, and it was empty right after the include. Asking now loads the schema, and the wrapping itself reads the raw list so it does not ask the schema to load while it is loading. --- lib/errgonomic/rails/active_record_optional.rb | 16 +++++++++++++--- test/rails_test.rb | 12 ++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/lib/errgonomic/rails/active_record_optional.rb b/lib/errgonomic/rails/active_record_optional.rb index ddad78e..bad8629 100644 --- a/lib/errgonomic/rails/active_record_optional.rb +++ b/lib/errgonomic/rails/active_record_optional.rb @@ -39,7 +39,17 @@ module ActiveRecordOptional end class_methods do + # What a model wrapped is the signal that a conversion did what it + # meant to, and the columns are not wrapped until the schema loads, so + # asking loads it. def errgonomic_optionals + load_schema + errgonomic_optional_names + end + + # The set as it stands, for the wrapping itself: reaching for the + # schema from here would ask the schema to load while it is loading. + def errgonomic_optional_names @errgonomic_optionals ||= [] end @@ -55,7 +65,7 @@ def errgonomic_optional_exclusions # A reader wrapped by an ancestor is already an Option; a subclass # that wrapped it again would nest it. def errgonomic_optional?(name) - return true if errgonomic_optionals.include?(name) + return true if errgonomic_optional_names.include?(name) superclass.respond_to?(:errgonomic_optional?) && superclass.errgonomic_optional?(name) end @@ -100,7 +110,7 @@ def encrypts(*names, **options) def errgonomic_unwrap_optionals(*names) names.map(&:to_s).each do |name| - next unless errgonomic_optionals.delete(name) + next unless errgonomic_optional_names.delete(name) remove_method(name) end @@ -110,7 +120,7 @@ def errgonomic_wrap_optional(name) name = name.to_s return if errgonomic_optional_exclusions.include?(name) || errgonomic_optional?(name) - errgonomic_optionals << name + errgonomic_optional_names << name class_eval <<-RUBY, __FILE__, __LINE__ + 1 def #{name} reads = Thread.current[:errgonomic_optional_reads] ||= {} diff --git a/test/rails_test.rb b/test/rails_test.rb index 6622dc9..2aae76a 100644 --- a/test/rails_test.rb +++ b/test/rails_test.rb @@ -270,6 +270,18 @@ def self.name = 'Magazine' assert magazine.create!(title: 'Nature').issn.none? end + # What a model wrapped is how a conversion is checked, so it has to answer + # for the columns too, before anything else has touched the model. + def test_the_wrapped_set_is_reported_before_the_schema_is_used + quarterly = Class.new(ActiveRecord::Base) do + def self.name = 'Quarterly' + self.table_name = 'magazines' + include Errgonomic::Rails::ActiveRecordOptional + end + + assert_equal %w[issn], quarterly.errgonomic_optionals + end + # An inherited reader is already wrapped, and a second wrap nests: Some of # a Some for a present value, while an absent one collapses back to None # because None answers nil?. Half of it is silent. From d7f0ac939543f4aa34264bedd8b45090f7e7fd0e Mon Sep 17 00:00:00 2001 From: Nick Zadrozny Date: Wed, 12 Aug 2026 15:07:35 -0500 Subject: [PATCH 4/4] Reach every model by including the concern on a base class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wrapping happens in per-model hooks now — the association macros and the schema seam — so where the include goes decides how far it reaches. On a model, that model converts; on an application's base class, every model below it does, and no model mentions errgonomic again. Converting one model or the whole application is placement rather than a setting. An application's own base class is the useful place for it. Engine and gem models descend straight from ActiveRecord::Base, and their code reads their attributes knowing nothing about an Option, so they stay out of it by construction rather than by a list of namespaces this gem would maintain. Three things had to change for that include to work. An abstract class has no table, and the walk up the chain asked one for its columns, which raised before any model loaded. Exclusions were snapshotted at include time, but a base class include leaves a model no "before" to declare anything in, so they are read when a reader is about to be wrapped, and errgonomic_optional_except also takes back a reader already wrapped. And a model needs a way out that does not point at an include of its own: errgonomic_optional_off. Rubocop stops applying the size cops to test/, where splitting a case to satisfy one hides the behaviour it was written to name. --- .rubocop.yml | 21 ++++++ README.md | 40 ++++++++++- .../rails/active_record_delegate_optional.rb | 13 ++-- .../rails/active_record_optional.rb | 53 ++++++++++----- test/rails_test.rb | 66 +++++++++++++++++++ 5 files changed, 171 insertions(+), 22 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 5b31ecb..55ddda0 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -18,6 +18,7 @@ Metrics/ClassLength: Exclude: - lib/errgonomic/option.rb - lib/errgonomic/result.rb + - test/**/* # core_ext vendors ActiveSupport's blank?/present? patches; the reopened core # classes there are explained by the file header, not per class. Test @@ -26,3 +27,23 @@ Style/Documentation: Exclude: - lib/errgonomic/core_ext/**/* - test/**/* + +# A test states one behavior end to end, and splitting one to satisfy a size +# metric hides the behavior it was written to name. A cop that names its own +# Exclude replaces this one, so Metrics/ClassLength repeats the path above. +Metrics: + Exclude: + - test/**/* + +# The ActiveRecord hooks in the optional concern are one subject — where a +# reader may come from and what leaves it alone — and reading them together is +# the point. The generated reader is a heredoc, which the length cops count as +# if it were code. +Metrics/BlockLength: + Exclude: + - lib/errgonomic/rails/active_record_optional.rb + - test/**/* +Metrics/MethodLength: + Exclude: + - lib/errgonomic/rails/active_record_optional.rb + - test/**/* diff --git a/README.md b/README.md index b0747af..0924096 100644 --- a/README.md +++ b/README.md @@ -201,7 +201,7 @@ end When `Rails::Railtie` is defined, Errgonomic installs a Railtie with two opt-in integrations for ActiveRecord: -- `include Errgonomic::Rails::ActiveRecordOptional` in a model makes its nullable attributes and `optional: true` associations return `Some(value)` or `None()` instead of a value-or-nil. Every nullable column and optional association is wrapped, with no per-attribute opt-in. An `optional: true` association declared after the include is wrapped as it is declared, so the include can sit at the top of the model with the other concerns. Two kinds of attribute stay unwrapped: those declared with `encrypts`, whose surrounding machinery reads the raw value, and those named by `errgonomic_optional_except`, which must appear before the include. +- `include Errgonomic::Rails::ActiveRecordOptional` in a model makes its nullable attributes and `optional: true` associations return `Some(value)` or `None()` instead of a value-or-nil. Every nullable column and optional association is wrapped, with no per-attribute opt-in. Two kinds of attribute stay unwrapped: those declared with `encrypts`, whose surrounding machinery reads the raw value, and those named by `errgonomic_optional_except`. ```ruby class Credential < ApplicationRecord @@ -211,6 +211,44 @@ class Credential < ApplicationRecord encrypts :access_secret # also left unwrapped, declared either side of the include end ``` + +**Where the include goes.** A model that includes the concern converts itself, and only itself. The include may sit at the top of the model with the other concerns, which is where Rails convention puts one. An `optional: true` association declared below it is wrapped as it is declared, rather than only the associations the class happened to declare above it. + +```ruby +class Book < ApplicationRecord + include Errgonomic::Rails::ActiveRecordOptional + + belongs_to :author, optional: true # Some(author) or None() +end +``` + +On an application's own base class, the same include reaches every model below it, and no model mentions errgonomic again: + +```ruby +class ApplicationRecord < ActiveRecord::Base + primary_abstract_class + include Errgonomic::Rails::ActiveRecordOptional +end +``` + +Converting one model or all of them is therefore where the include goes, not a setting to choose. The association macros wrap as each model declares them, and a model's nullable columns are wrapped when ActiveRecord loads its schema, so no class body needs a database while it loads. + +An application's own base class is the useful place for it. Engine and gem models such as `ActiveStorage::Blob` and `PaperTrail::Version` descend straight from `ActiveRecord::Base`, and their own code reads their attributes knowing nothing about an Option. Including it on `ActiveRecord::Base` reaches those too, which is rarely what anyone wants. + +Two ways out, both readable in a model with no include of its own to point at: + +```ruby +class LegacyImport < ApplicationRecord + errgonomic_optional_off # this model keeps value-or-nil throughout +end + +class Credential < ApplicationRecord + errgonomic_optional_except :legacy_token # this attribute does +end +``` + +`Model.errgonomic_optionals` reports which readers a model wrapped, which is how to check that a conversion did what it meant to. + - `delegate_optional :name, to: :association` (available on all models) delegates through an optional association, returning an Option instead of raising on nil. `Object#to_option` is also available in Rails to lift any value into an Option (`nil.to_option # => None()`). diff --git a/lib/errgonomic/rails/active_record_delegate_optional.rb b/lib/errgonomic/rails/active_record_delegate_optional.rb index bc02052..4d590d3 100644 --- a/lib/errgonomic/rails/active_record_delegate_optional.rb +++ b/lib/errgonomic/rails/active_record_delegate_optional.rb @@ -9,15 +9,20 @@ module ActiveRecordDelegateOptional extend ActiveSupport::Concern class_methods do - # Names attributes that ActiveRecordOptional must leave alone. It has - # to be callable before the include, which is what computes the - # wrapped set, so it lives here rather than in the concern itself. + # Names attributes that ActiveRecordOptional must leave alone. It has to + # be callable before the include, which is what starts the wrapping for + # a model that converts itself, so it lives here rather than in the + # concern. Where the concern is included on a base class there is no + # before, so it also takes back a reader already wrapped. def errgonomic_optional_except(*names) @errgonomic_optional_exceptions = errgonomic_optional_exceptions + names.map(&:to_s) + errgonomic_unwrap_optionals(*names) if respond_to?(:errgonomic_unwrap_optionals) + @errgonomic_optional_exceptions end def errgonomic_optional_exceptions - @errgonomic_optional_exceptions ||= [] + @errgonomic_optional_exceptions ||= + superclass.respond_to?(:errgonomic_optional_exceptions) ? superclass.errgonomic_optional_exceptions.dup : [] end def delegate_optional(*methods, to: nil, prefix: nil, private: nil) diff --git a/lib/errgonomic/rails/active_record_optional.rb b/lib/errgonomic/rails/active_record_optional.rb index bad8629..88b6e1a 100644 --- a/lib/errgonomic/rails/active_record_optional.rb +++ b/lib/errgonomic/rails/active_record_optional.rb @@ -30,9 +30,6 @@ module ActiveRecordOptional extend ActiveSupport::Concern included do - # ::Rails.logger.debug('ActiveRecordOptional') - @errgonomic_optional_exclusions = - Array(encrypted_attributes).map(&:to_s) + Array(try(:errgonomic_optional_exceptions)).map(&:to_s) reflect_on_all_associations(:belongs_to) .select { |r| r.options[:optional] } .each { |r| errgonomic_wrap_optional(r.name) } @@ -50,16 +47,35 @@ def errgonomic_optionals # The set as it stands, for the wrapping itself: reaching for the # schema from here would ask the schema to load while it is loading. def errgonomic_optional_names - @errgonomic_optionals ||= [] + @errgonomic_optional_names ||= [] end + # Read when a reader is about to be wrapped rather than snapshotted at + # include time, so an exclusion works on either side of the include. + # That is what an include on a base class needs: there is no "before" + # for a model to declare anything in. def errgonomic_optional_exclusions - @errgonomic_optional_exclusions ||= - if superclass.respond_to?(:errgonomic_optional_exclusions) - superclass.errgonomic_optional_exclusions.dup - else - [] - end + inherited = if superclass.respond_to?(:errgonomic_optional_exclusions) + superclass.errgonomic_optional_exclusions + else + [] + end + + inherited | Array(encrypted_attributes).map(&:to_s) | Array(try(:errgonomic_optional_exceptions)).map(&:to_s) + end + + # A model that keeps value-or-nil throughout, for whatever the + # application knows about it that the concern does not. Where the + # concern is included on a base class, this is how a model leaves. + def errgonomic_optional_off + @errgonomic_optional_off = true + errgonomic_unwrap_optionals(*errgonomic_optional_names.dup) + end + + def errgonomic_optional_off? + return true if defined?(@errgonomic_optional_off) && @errgonomic_optional_off + + superclass.respond_to?(:errgonomic_optional_off?) && superclass.errgonomic_optional_off? end # A reader wrapped by an ancestor is already an Option; a subclass @@ -85,6 +101,11 @@ def load_schema! # before a subclass considers the same name. def errgonomic_wrap_nullable_columns superclass.errgonomic_wrap_nullable_columns if superclass.respond_to?(:errgonomic_wrap_nullable_columns) + # An abstract class has no table, and asking one for its columns + # raises. The concern belongs on an abstract class all the same: that + # is where an application puts behaviour every model should have. + return if abstract_class? || table_name.nil? + column_names.each { |name| errgonomic_wrap_optional(name) if column_for_attribute(name).null } end @@ -98,14 +119,11 @@ def belongs_to(name, scope = nil, **options) # Encryption surrounds an attribute with machinery that reads the raw # value, including a length validator that calls to_s on it, so a # wrapped encrypted attribute cannot be saved. Declaring encrypts - # after the include is the ordinary spelling, so record the exclusion - # for whenever the schema arrives, and take back any reader already - # wrapped. + # after the include is the ordinary spelling, and the exclusion is read + # from ActiveRecord's own register when a reader is about to be + # wrapped, so this only has to take back a reader already wrapped. def encrypts(*names, **options) - super.tap do - errgonomic_optional_exclusions.concat(names.map(&:to_s)) - errgonomic_unwrap_optionals(*names) - end + super.tap { errgonomic_unwrap_optionals(*names) } end def errgonomic_unwrap_optionals(*names) @@ -118,6 +136,7 @@ def errgonomic_unwrap_optionals(*names) def errgonomic_wrap_optional(name) name = name.to_s + return if errgonomic_optional_off? return if errgonomic_optional_exclusions.include?(name) || errgonomic_optional?(name) errgonomic_optional_names << name diff --git a/test/rails_test.rb b/test/rails_test.rb index 2aae76a..fb9da94 100644 --- a/test/rails_test.rb +++ b/test/rails_test.rb @@ -126,6 +126,43 @@ class OptedOutBook < ActiveRecord::Base # second time for columns its parent already wrapped. class Novel < Book; end +# Where the include goes decides how far it reaches. On an application's own +# base class it reaches every model below, so a model converts without naming +# errgonomic at all. +class HouseRecord < ActiveRecord::Base + self.abstract_class = true + include Errgonomic::Rails::ActiveRecordOptional +end + +# Nothing in these two mentions the concern. +class Zine < HouseRecord + self.table_name = 'magazines' +end + +class Chapbook < HouseRecord + self.table_name = 'books' + belongs_to :author, optional: true +end + +# A model that keeps value-or-nil throughout. +class PlainZine < HouseRecord + self.table_name = 'magazines' + errgonomic_optional_off +end + +# One attribute back to value-or-nil, in a model with no include to declare it +# before. +class PartlyPlainZine < HouseRecord + self.table_name = 'magazines' + errgonomic_optional_except :issn +end + +# A model from a gem descends straight from ActiveRecord::Base, as engine +# models do, so an application's base class does not reach it. +class VendorLedger < ActiveRecord::Base + self.table_name = 'magazines' +end + class BugTest < Minitest::Test def test_optional_attributes author = Author.create!(name: 'Cixin Liu') @@ -294,6 +331,35 @@ def test_a_subclass_reads_its_inherited_wrapped_attributes_once assert Novel.create!(title: 'Supernova Era').isbn.none? end + # An include on a base class reaches every model below it, columns and + # associations alike, which is how an application converts all at once. + def test_a_base_class_include_reaches_the_models_below_it + assert Zine.create!(title: 'Nature').issn.none? + assert_equal %w[issn], Zine.errgonomic_optionals + + author = Author.create!(name: 'Cixin Liu') + + assert_equal author.id, Chapbook.create!(title: 'The Wandering Earth II', author_id: author.id).author.unwrap!.id + assert Chapbook.create!(title: 'Supernova Era').author.none? + end + + # A model whose own code reads its attributes raw has to be able to say so, + # in a model body with no include to point at. + def test_a_model_below_the_base_class_can_opt_out_entirely + assert_nil PlainZine.create!(title: 'Asimovs').issn + assert_empty PlainZine.errgonomic_optionals + end + + def test_a_model_below_the_base_class_can_opt_out_one_attribute + assert_equal '1937-7843', PartlyPlainZine.create!(title: 'Clarkesworld', issn: '1937-7843').issn + end + + # Engine and gem models descend straight from ActiveRecord::Base, and their + # own code knows nothing about an Option. + def test_a_model_outside_the_base_class_is_untouched + assert_nil VendorLedger.create!(title: 'Ledger').issn + end + # Wrapping only what the class already declared makes the include's # position load-bearing, and a partial conversion is silent. def test_optional_belongs_to_declared_after_the_include_is_wrapped