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 45e88e7..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. 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 e3f4fe4..88b6e1a 100644 --- a/lib/errgonomic/rails/active_record_optional.rb +++ b/lib/errgonomic/rails/active_record_optional.rb @@ -30,58 +30,135 @@ module ActiveRecordOptional extend ActiveSupport::Concern included do - # ::Rails.logger.debug('ActiveRecordOptional') - 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 + reflect_on_all_associations(:belongs_to) + .select { |r| r.options[:optional] } + .each { |r| errgonomic_wrap_optional(r.name) } 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 - @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_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 + 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 + # that wrapped it again would nest it. + def errgonomic_optional?(name) + return true if errgonomic_optional_names.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) + # 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 + + # 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 # 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, 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 { errgonomic_unwrap_optionals(*names) } end 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 end + + 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 + 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..fb9da94 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 @@ -99,6 +105,64 @@ 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 + +# 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 + +# 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') @@ -221,6 +285,99 @@ 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 + + # 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. + 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 + + # 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 + 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')