diff --git a/CHANGELOG.md b/CHANGELOG.md index cc1e349..08c1fda 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,19 @@ +### Unreleased + +- Add `table(false)` to create no table, so that a model whose superclass is a + concrete Active Record class uses single table inheritance. +- Accept a String, a Symbol, or a callable for `superclass:`, resolved for each + example, so that another `with_model` model can be the superclass. +- Raise `WithModel::InvalidSuperclass` for a `superclass:` that cannot be used, + and `WithModel::MissingSuperclass` for a name that resolves to nothing. Both + are `ArgumentError`s, and the latter is an `InvalidSuperclass`, so either can + be rescued as narrowly as needed. +- Deprecate omitting `table`, which currently creates a table with only an id + column. In 3.0 it will create no table. +- Create minitest models in the order they are declared, so that a model can + refer to another model declared above it. +- Require Ruby 3.2 or later. + ### 2.2.0 - Fix dependency tracking issue when `cache_classes: true` is set in Rails 7+. diff --git a/README.md b/README.md index d3ff49a..54d1f36 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ Install as usual: `gem install with_model` or add `gem 'with_model'` to your Gem Extend `WithModel` into RSpec: ```ruby -require 'with_model' +require "with_model" RSpec.configure do |config| config.extend WithModel @@ -31,7 +31,7 @@ end Extend `WithModel` into minitest/spec and set the test runner explicitly: ```ruby -require 'with_model' +require "with_model" WithModel.runner = :minitest @@ -45,11 +45,16 @@ end After setting up as above, call `with_model` and inside its block pass it a `table` block and a `model` block. ```ruby -require 'spec_helper' +require "spec_helper" -describe "A blog post" do - module MyModule; end +module MyModule; end + +# A pre-existing model +class Car < ActiveRecord::Base + self.abstract_class = true +end +describe "A blog post" do with_model :BlogPost do # The table block (and an options hash) is passed to Active Record migration’s `create_table`. table do |t| @@ -60,15 +65,16 @@ describe "A blog post" do # The model block is the Active Record model’s class body. model do include MyModule + has_many :comments validates_presence_of :title def self.some_class_method - 'chunky' + "chunky" end def some_instance_method - 'bacon' + "bacon" end end end @@ -91,15 +97,15 @@ describe "A blog post" do end it "has the module" do - expect(BlogPost.include?(MyModule)).to eq true + expect(BlogPost.include?(MyModule)).to be true end it "has the class method" do - expect(BlogPost.some_class_method).to eq 'chunky' + expect(BlogPost.some_class_method).to eq "chunky" end it "has the instance method" do - expect(BlogPost.new.some_instance_method).to eq 'bacon' + expect(BlogPost.new.some_instance_method).to eq "bacon" end it "can do all the things a regular model can" do @@ -107,22 +113,21 @@ describe "A blog post" do expect(record).not_to be_valid record.title = "foo" expect(record).to be_valid - expect(record.save).to eq true + expect(record.save).to be true expect(record.reload).to eq record - record.comments.create!(:text => "Lorem ipsum") + record.comments.create!(text: "Lorem ipsum") expect(record.comments.count).to eq 1 end - # with_model classes can have inheritance. - class Car < ActiveRecord::Base - self.abstract_class = true - end - + # with_model classes can have inheritance. Car is abstract, so it has no table + # and Ford gets one of its own. To inherit a concrete superclass's table + # instead, see "Single table inheritance" below. with_model :Ford, superclass: Car do + table end it "has a specified superclass" do - expect(Ford < Car).to eq true + expect(Ford.new).to be_a(Car) end end @@ -150,8 +155,8 @@ end describe "with table options" do with_model :WithOptions do - table :id => false do |t| - t.string 'foo' + table id: false do |t| + t.string "foo" t.timestamps null: false end end @@ -162,6 +167,89 @@ describe "with table options" do end ``` +## Single table inheritance + +Pass `table(false)` to create no table at all. Active Record's own inheritance +then supplies the superclass's table, which is what single table inheritance +needs. + +The superclass can be another `with_model` model. Its constant does not exist yet +when the `superclass:` argument is read, so name it with a String or a Symbol, or +pass a callable returning it, and it will be resolved once per example. + +```ruby +describe "with_model supports Single Table Inheritance" do + with_model :Sandwich do + table do |t| + t.string "type" + t.string "bread" + end + end + + with_model :ChunkyBacon, superclass: :Sandwich do + table(false) + end + + it "shares the superclass's table" do + expect(ChunkyBacon.table_name).to eq Sandwich.table_name + end + + it "stores its own type" do + sandwich = ChunkyBacon.create!(bread: "rye") + + expect(sandwich.reload.type).to eq "ChunkyBacon" + expect(Sandwich.first).to be_a ChunkyBacon + end +end +``` + +A `superclass:` that cannot be used raises `WithModel::InvalidSuperclass`: one +that is not an Active Record class, one with no table of its own +(`ActiveRecord::Base`, or an abstract class such as a Rails app's +`ApplicationRecord`), or one whose table has no inheritance column to tell a +subclass's rows apart. A name that resolves to nothing raises +`WithModel::MissingSuperclass`, a kind of `InvalidSuperclass`, quoting the +constant Ruby could not find — for a namespaced name, that is the segment which +is actually missing. Both are `ArgumentError`s. + +A superclass whose table does not exist *yet* is allowed, since the table may be +created later in the example, and Active Record reports its absence clearly +enough on its own. Rows the model wrote are deleted when it goes away, since they +name a class that is about to stop existing and would make the superclass +unloadable. + +## Foreign keys + +`foreign_key: true` asks Active Record to infer the table a foreign key points +at, and it infers `authors` from `author_id`. Generated table names are unique +rather than conventional, so name the table instead: + +```ruby +describe "with_model supports foreign keys" do + with_model :Author do + table + end + + with_model :Book do + table do |t| + t.references :author, foreign_key: {to_table: Author.table_name} + end + + model do + belongs_to :author + end + end + + it "has a foreign key" do + expect { Book.create!(author_id: 0) } + .to raise_error ActiveRecord::InvalidForeignKey + end +end +``` + +This reads `Author.table_name`, so declare `Author` first: models are created in +the order they are declared, and destroyed in the reverse order. + ## Requirements See the [gemspec metadata](https://rubygems.org/gems/with_model) for dependency requirements. RSpec and minitest are indirect dependencies, and `with_model` should support any maintained version of both. diff --git a/lib/with_model.rb b/lib/with_model.rb index 88c0ed7..224f09a 100644 --- a/lib/with_model.rb +++ b/lib/with_model.rb @@ -1,7 +1,11 @@ # frozen_string_literal: true +require "active_support/deprecation" +require "with_model/invalid_superclass" +require "with_model/missing_superclass" require "with_model/model" require "with_model/model/dsl" +require "with_model/null_table" require "with_model/table" require "with_model/version" @@ -36,6 +40,14 @@ def self.runner @runner ||= :rspec end + # The deprecator used for with_model's own deprecation warnings. Callers can + # silence it (`WithModel.deprecator.silenced = true`) or escalate it + # (`behavior = :raise`) while migrating, and Rails applications can register + # it in `Rails.application.deprecators`. + def self.deprecator + @deprecator ||= ActiveSupport::Deprecation.new("3.0", "with_model") + end + # @param [Symbol] name The constant name to assign the model class to. # @param scope Passed to `before`/`after` in the test context. RSpec only. # @param options Passed to {WithModel::Model#initialize}. @@ -45,6 +57,8 @@ def with_model(name, scope: nil, **options, &block) model = Model.new name, **options dsl = Model::DSL.new model dsl.instance_exec(&block) if block + # caller_locations(1) is this method's caller: the `with_model` line itself. + WithModel.warn_omitted_table(name, caller_locations(1)) unless model.table_specified? setup_object(model, scope: scope, runner: runner) end @@ -60,6 +74,26 @@ def with_table(name, scope: nil, **options, &block) setup_object(table, scope: scope, runner: runner) end + # Warns once per call site, at definition time, rather than once per example. + # The horizon is stated in the message because `Deprecation#warn` does not + # interpolate the deprecator's `deprecation_horizon`. + # + # The callstack has to be handed in. `ActiveSupport::Deprecation` skips Rails' + # own frames and the standard library when working out where a warning came + # from, but with_model's frames look like anyone else's to it, so left to itself + # it reports this file for every omission in a suite. + # + # @param callstack Frames to blame, beginning with the caller to report. + def self.warn_omitted_table(name, callstack) + deprecator.warn( + "with_model #{name.inspect} was called without a `table`, which creates a table with only " \ + "an id column. In with_model 3.0 no table will be created. Call `table` (with no " \ + "arguments or an empty block) to keep a table, or `table(false)` to inherit the " \ + "superclass's table (single table inheritance).", + callstack + ) + end + private # @param [Object] object The new model object instance to create diff --git a/lib/with_model/invalid_superclass.rb b/lib/with_model/invalid_superclass.rb new file mode 100644 index 0000000..1f30d09 --- /dev/null +++ b/lib/with_model/invalid_superclass.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module WithModel + # Raised when the `superclass:` a model was given cannot be used: it is not an + # Active Record class at all, or it is one that cannot supply a table to a model + # which has none of its own - because it has no table itself + # (`ActiveRecord::Base`, or an abstract class), or because its table has no + # inheritance column and so cannot tell a subclass's rows apart. + # + # An `ArgumentError`, because each is a fact about the argument rather than about + # when it was looked at, and the superclass is needed the moment the model is + # built - unlike a table, which an example is free to create later. + class InvalidSuperclass < ArgumentError + end +end diff --git a/lib/with_model/missing_superclass.rb b/lib/with_model/missing_superclass.rb new file mode 100644 index 0000000..75aff14 --- /dev/null +++ b/lib/with_model/missing_superclass.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +require "with_model/invalid_superclass" + +module WithModel + # Raised when the name given for a `superclass:` resolves to nothing at all. + # + # A kind of {WithModel::InvalidSuperclass}, so every superclass that cannot be + # used is catchable in one place, while a name that is simply not there - a + # typo, or a with_model superclass declared after the models inheriting it - + # can be told apart from a class that exists but cannot supply a table. + class MissingSuperclass < InvalidSuperclass + end +end diff --git a/lib/with_model/model.rb b/lib/with_model/model.rb index 27d2166..9e68471 100644 --- a/lib/with_model/model.rb +++ b/lib/with_model/model.rb @@ -7,6 +7,9 @@ require "with_model/constant_stubber" require "with_model/descendants_tracker" require "with_model/methods" +require "with_model/invalid_superclass" +require "with_model/missing_superclass" +require "with_model/null_table" require "with_model/table" module WithModel @@ -16,17 +19,43 @@ class Model attr_writer :model_block, :table_block, :table_options # @param [Symbol] name The constant name to assign the model class to. - # @param [Class] superclass The superclass for the created class. Should - # have `ActiveRecord::Base` as an ancestor. + # @param superclass The superclass for the created class. Either a Class + # having `ActiveRecord::Base` as an ancestor, a String naming one, or a + # callable returning one. A String or callable is resolved afresh for + # every example, which is what allows another `with_model` model - whose + # constant does not exist when this line is read - to be the superclass. def initialize(name, superclass: ActiveRecord::Base) @name = name.to_sym @model_block = nil @table_block = nil @table_options = {} - @superclass = superclass + @table_specified = false + @skip_table = false + @superclass_spec = superclass end + # Records what {WithModel::Model::DSL#table} was asked for, including the + # fact that it was asked for at all. + def specify_table(options, block) + @table_specified = true + + if options == false + raise ArgumentError, "table does not take a block when its first argument is falsy" if block + + @skip_table = true + else + @table_options = options + @table_block = block + end + end + + # Whether a table was specified at all. A `table` call with no arguments + # counts, so this cannot be inferred from the options and block alone. + def table_specified? = @table_specified + def create + @superclass = resolve_superclass + @table = nil table.create @model = Class.new(@superclass) do extend WithModel::Methods @@ -36,22 +65,69 @@ def create end def destroy + # Test runners tear down even when setup raised, so `create` may not have + # reached the point of building the model. Nothing was stubbed and nothing + # wrote rows, so there is nothing to undo. + return unless @model + + # Before `unstub_const`: a teardown that identifies this model's own rows + # can only do so while the class still has its name. + table.teardown(@model) stubber.unstub_const cleanup_descendants_tracking reset_dependencies_cache - table.destroy WithModel::DescendantsTracker.clear([@model]) @model = nil end private + def resolve_superclass + spec = @superclass_spec + spec = spec.call if spec.respond_to?(:call) + spec = constantize_superclass(spec) if class_name?(spec) + + unless spec.is_a?(Class) && spec <= ActiveRecord::Base + raise InvalidSuperclass, + "superclass must be a Class descending from ActiveRecord::Base, but was #{spec.inspect}. " \ + "To refer to another with_model class, or anything else that is only defined once the " \ + "test is running, name it with a String or a Symbol, or pass a callable returning it." + end + + spec + end + + # A Symbol reads naturally here, since with_model names its own models with + # them. Anything else has to answer to `to_str`, which only real strings do: + # every object has a `to_s`, so accepting that would quietly look up a + # constant named "42". + def class_name?(spec) + spec.is_a?(Symbol) || spec.respond_to?(:to_str) + end + + # `to_s` is safe here where `class_name?` has already vouched for the value, + # and it keeps the Symbol intact for the message below, which reports what was + # passed in rather than what it was converted to. + # + # The NameError is worth quoting rather than replacing: for a namespaced name + # it reports which segment was missing, which this message cannot work out. + # Raising inside the rescue leaves it as the `cause` for anything that wants + # the backtrace. + def constantize_superclass(name) + name.to_s.constantize + rescue NameError => e + raise MissingSuperclass, + "superclass #{name.inspect} could not be resolved: #{e.message}. Names are resolved while " \ + "the test is running, so a with_model superclass has to be declared before the models " \ + "that inherit it." + end + def const_name @name.to_s.camelize.to_sym end def setup_model - @model.table_name = table_name + table.configure(@model) @model.class_eval(&@model_block) if @model_block @model.reset_column_information end @@ -72,7 +148,11 @@ def stubber end def table - @table ||= Table.new table_name, @table_options, connection: @superclass.connection, &@table_block + @table ||= if @skip_table + NullTable.new(@superclass, @name) + else + Table.new table_name, @table_options, connection: @superclass.connection, &@table_block + end end def table_name diff --git a/lib/with_model/model/dsl.rb b/lib/with_model/model/dsl.rb index d7b93d6..da8cc0e 100644 --- a/lib/with_model/model/dsl.rb +++ b/lib/with_model/model/dsl.rb @@ -11,10 +11,13 @@ def initialize(model) # Provide a schema definition for the table, passed to ActiveRecord's `create_table`. # The table name will be auto-generated. # + # Pass `false` instead of options to create no table at all, so that the + # model inherits its superclass's table as Rails Single Table Inheritance + # requires. `table(false)` takes no block. + # # @see https://api.rubyonrails.org/classes/ActiveRecord/ConnectionAdapters/SchemaStatements.html#method-i-create_table def table(options = {}, &block) - @model.table_options = options - @model.table_block = block + @model.specify_table(options, block) end # Provide a class body for the ActiveRecord model. diff --git a/lib/with_model/null_table.rb b/lib/with_model/null_table.rb new file mode 100644 index 0000000..db00bca --- /dev/null +++ b/lib/with_model/null_table.rb @@ -0,0 +1,98 @@ +# frozen_string_literal: true + +require "active_record" +require "with_model/invalid_superclass" + +module WithModel + # Stands in for a {WithModel::Table} when a model should inherit its + # superclass's table instead of getting one of its own, as Rails Single Table + # Inheritance requires. Selected by `table(false)`. + # + # In general, direct use of this class should be avoided. Instead use + # either the {WithModel high-level API} or {WithModel::Model::DSL low-level API}. + class NullTable + # @param [Class] superclass The resolved superclass whose table will be + # inherited. + # @param [Symbol, String] name The model's name, so that a refusal can say + # which model it is talking about. + def initialize(superclass, name) + @superclass = superclass + @name = name + end + + # Creates nothing, but refuses a superclass that cannot support single + # table inheritance. + # + # Refusals describe the model's situation rather than the call that produced + # it. Any expression that evaluates to false selects a NullTable, so there + # is no call spelling to quote - and in with_model 3.0, omitting `table` + # will arrive here too. + # + # A superclass whose table does not exist *yet* is deliberately allowed: the + # table may be created later in the example, and a test may legitimately + # want a model whose table is missing. Active Record raises a clear + # `StatementInvalid` naming the table if it never appears, so refusing here + # would only forbid working setups. + # + # What is left is {WithModel::InvalidSuperclass}: both refusals are permanent + # facts about the superclass passed in, not about when it was looked at, so no + # amount of waiting makes them work. + def create + refuse "#{@superclass} has none to inherit" unless table_name? + + # Nothing more can be checked until there is a table to look at. + return unless table_exists? + return if inheritance_column? + + refuse "#{@superclass}'s table #{@superclass.table_name.inspect} has no " \ + "#{inheritance_column.inspect} column, so Active Record cannot tell its rows " \ + "apart from a subclass's" + end + + # Deliberately does nothing: leaving `table_name` unassigned is what lets + # Active Record's own inheritance supply the superclass's table. + def configure(klass) + end + + # Removes the rows this model wrote, which would otherwise outlive the + # constant that names them and make the superclass unloadable + # (`ActiveRecord::SubclassNotFound`). + # + # `unscoped` is required because a `default_scope` on the superclass would + # otherwise hide rows from the delete; the inheritance-column condition is + # then reapplied explicitly, since `unscoped` also discards the type + # condition that keeps this from touching the superclass's own rows. + # + # A table that does not exist holds no rows to remove, and failing here + # would fail an example whose body had already passed. + def teardown(klass) + return unless klass.table_exists? + + klass.unscoped.where(klass.inheritance_column => klass.sti_name).delete_all + end + + # Drops nothing; there is no table of our own to drop. + def destroy + end + + private + + def inheritance_column = @superclass.inheritance_column + + # `ActiveRecord::Base` and abstract classes alike have no `table_name`, so + # this is what "nothing to inherit" actually looks like - `abstract_class?` + # is false for `ActiveRecord::Base` and so does not describe both. + def table_name? = @superclass.table_name.present? + + def table_exists? = @superclass.table_exists? + + def inheritance_column? + inheritance_column.present? && @superclass.columns_hash.key?(inheritance_column) + end + + def refuse(problem) + raise InvalidSuperclass, + "with_model #{@name.inspect} has no table of its own, but #{problem}" + end + end +end diff --git a/lib/with_model/table.rb b/lib/with_model/table.rb index dabae1d..457998a 100644 --- a/lib/with_model/table.rb +++ b/lib/with_model/table.rb @@ -25,6 +25,18 @@ def create connection.create_table(@name, **@options, &@block) end + # Points the model at this table. + def configure(klass) + klass.table_name = @name + end + + # Removes everything this table holds by dropping it. The model is not + # needed, but is accepted so that {WithModel::NullTable} - which does need + # it - can stand in here. + def teardown(_klass) + destroy + end + def destroy connection.drop_table(@name) end diff --git a/spec/active_record_behaviors_spec.rb b/spec/active_record_behaviors_spec.rb index a340011..ab8d48f 100644 --- a/spec/active_record_behaviors_spec.rb +++ b/spec/active_record_behaviors_spec.rb @@ -69,6 +69,7 @@ with_table :animals with_model :StuffedAnimal do + table model do has_many :tea_cups, as: :pet end @@ -101,7 +102,9 @@ end end - with_model :Country + with_model :Country do + table + end context "in earlier examples" do it "works as normal" do diff --git a/spec/descendants_tracking_spec.rb b/spec/descendants_tracking_spec.rb index 58bbc24..0ffdef8 100644 --- a/spec/descendants_tracking_spec.rb +++ b/spec/descendants_tracking_spec.rb @@ -4,6 +4,7 @@ describe "Descendants tracking" do with_model :BlogPost do + table model do def self.inspect "BlogPost class #{object_id}" diff --git a/spec/readme_spec.rb b/spec/readme_spec.rb index 12468bf..4388026 100644 --- a/spec/readme_spec.rb +++ b/spec/readme_spec.rb @@ -2,19 +2,22 @@ require "spec_helper" -describe "A blog post" do - before do - stub_const("MyModule", Module.new) - end +module MyModule; end + +# A pre-existing model +class Car < ActiveRecord::Base + self.abstract_class = true +end +describe "A blog post" do with_model :BlogPost do - # The table block works just like a migration. + # The table block (and an options hash) is passed to Active Record migration’s `create_table`. table do |t| t.string :title t.timestamps null: false end - # The model block works just like the class definition. + # The model block is the Active Record model’s class body. model do include MyModule @@ -71,15 +74,15 @@ def some_instance_method expect(record.comments.count).to eq 1 end - # with_model classes can have inheritance. - class Car < ActiveRecord::Base # standard:disable Lint/ConstantDefinitionInBlock - self.abstract_class = true + # with_model classes can have inheritance. Car is abstract, so it has no table + # and Ford gets one of its own. To inherit a concrete superclass's table + # instead, see "Single table inheritance" below. + with_model :Ford, superclass: Car do + table end - with_model :Ford, superclass: Car - it "has a specified superclass" do - expect(Ford < Car).to be true + expect(Ford.new).to be_a(Car) end end @@ -117,3 +120,48 @@ class Car < ActiveRecord::Base # standard:disable Lint/ConstantDefinitionInBlock expect(WithOptions.columns.map(&:name)).not_to include("id") end end + +describe "with_model supports Single Table Inheritance" do + with_model :Sandwich do + table do |t| + t.string "type" + t.string "bread" + end + end + + with_model :ChunkyBacon, superclass: :Sandwich do + table(false) + end + + it "shares the superclass's table" do + expect(ChunkyBacon.table_name).to eq Sandwich.table_name + end + + it "stores its own type" do + sandwich = ChunkyBacon.create!(bread: "rye") + + expect(sandwich.reload.type).to eq "ChunkyBacon" + expect(Sandwich.first).to be_a ChunkyBacon + end +end + +describe "with_model supports foreign keys" do + with_model :Author do + table + end + + with_model :Book do + table do |t| + t.references :author, foreign_key: {to_table: Author.table_name} + end + + model do + belongs_to :author + end + end + + it "has a foreign key" do + expect { Book.create!(author_id: 0) } + .to raise_error ActiveRecord::InvalidForeignKey + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index b184223..e0ab7f1 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -3,6 +3,8 @@ require "bundler/setup" require "with_model" +WithModel.deprecator.behavior = :raise + RSpec.configure do |config| config.extend WithModel diff --git a/spec/table_false_spec.rb b/spec/table_false_spec.rb new file mode 100644 index 0000000..ea3945e --- /dev/null +++ b/spec/table_false_spec.rb @@ -0,0 +1,516 @@ +# frozen_string_literal: true + +require "spec_helper" + +# Wrapping the file in a module keeps these parents out of the global namespace +# while still defining them at file scope, where Standard's +# Lint/ConstantDefinitionInBlock does not fire. Blocks resolve constants from +# where they are written, so the examples below name them without qualifying. +# Their tables are supplied per-example by `with_table`, so no schema persists +# between spec files. +module TableFalseSpec + # The ordinary shape single table inheritance expects. + class HasTypeColumn < ActiveRecord::Base + end + + class HasCustomInheritanceColumn < ActiveRecord::Base + self.inheritance_column = "kind" + end + + # Both a custom inheritance column and a `type` column, so a test can tell + # which one with_model actually reads. + class HasCustomInheritanceColumnAndTypeColumn < ActiveRecord::Base + self.inheritance_column = "kind" + end + + class HasDefaultScope < ActiveRecord::Base + default_scope { where(archived: false) } + end + + class HasNoTypeColumn < ActiveRecord::Base + end + + class HasNilInheritanceColumn < ActiveRecord::Base + self.inheritance_column = nil + end + + class IsAbstract < ActiveRecord::Base + self.abstract_class = true + end + + # The shape a Rails app's ApplicationRecord takes: abstract, and inherited from + # rather than ActiveRecord::Base directly. + class ApplicationRecord < ActiveRecord::Base + self.abstract_class = true + end + + # Its table is never created. + class HasMissingTable < ApplicationRecord + end + + # Its table is created partway through an example. + class HasLateTable < ActiveRecord::Base + end + + RSpec.describe "table(false)" do + describe "with a concrete superclass" do + with_table(HasTypeColumn.table_name) do |t| + t.string :type + t.string :name + end + + before { HasTypeColumn.reset_column_information } + + with_model :Truck, superclass: HasTypeColumn do + table(false) + model do + def honk = "beep" + end + end + + it "inherits the superclass table rather than creating its own" do + expect(Truck.table_name).to eq HasTypeColumn.table_name + end + + it "creates no table of its own" do + expect(ActiveRecord::Base.connection.tables.grep(/with_model_truck/)).to be_empty + end + + it "reports the superclass as the STI base class" do + expect(Truck.base_class).to eq HasTypeColumn + end + + it "still evaluates the model block" do + expect(Truck.new.honk).to eq "beep" + end + + it "writes its own name to the inheritance column" do + Truck.create!(name: "big") + expect(HasTypeColumn.first.type).to eq "Truck" + end + + it "is found through the superclass as an instance of itself" do + Truck.create!(name: "big") + expect(HasTypeColumn.first).to be_a Truck + end + + # Deliberately a second example asserting the same thing: a superclass + # resolved once and cached on the Model instance would go stale here, + # because the parent is the same class object but the child is rebuilt. + it "works in a second example too" do + Truck.create!(name: "also big") + expect(HasTypeColumn.first).to be_a Truck + end + end + + describe "teardown" do + with_table(HasTypeColumn.table_name) do |t| + t.string :type + t.string :name + end + + before { HasTypeColumn.reset_column_information } + + def build(name, superclass:, &dsl) + WithModel::Model.new(name, superclass: superclass).tap do |model| + WithModel::Model::DSL.new(model).instance_eval(&dsl) + end + end + + it "deletes its own rows and leaves the superclass's rows alone" do + model = build(:Truck, superclass: HasTypeColumn) { table(false) } + model.create + Truck.create!(name: "child row") + HasTypeColumn.create!(name: "parent row") + + model.destroy + + expect(HasTypeColumn.where(type: "Truck").count).to eq 0 + expect(HasTypeColumn.where(type: nil).count).to eq 1 + end + + it "leaves the superclass queryable" do + model = build(:Truck, superclass: HasTypeColumn) { table(false) } + model.create + Truck.create!(name: "child row") + + model.destroy + + expect { HasTypeColumn.first }.not_to raise_error + end + + it "does not delete rows when create refused the superclass" do + HasTypeColumn.create!(name: "parent row") + model = build(:Truck, superclass: IsAbstract) { table(false) } + + expect { model.create }.to raise_error(WithModel::InvalidSuperclass) + expect { model.destroy }.not_to raise_error + + expect(HasTypeColumn.count).to eq 1 + end + end + + describe "with a superclass that has a default_scope" do + with_table(HasDefaultScope.table_name) do |t| + t.string :type + t.boolean :archived, default: false + end + + before { HasDefaultScope.reset_column_information } + + it "deletes rows the default scope would have hidden" do + model = WithModel::Model.new(:ScopedChild, superclass: HasDefaultScope) + WithModel::Model::DSL.new(model).table(false) + model.create + ScopedChild.create!(archived: false) + ScopedChild.create!(archived: true) + + model.destroy + + expect(HasDefaultScope.unscoped.where(type: "ScopedChild").count).to eq 0 + end + end + + describe "with a non-standard inheritance_column" do + with_table(HasCustomInheritanceColumn.table_name) do |t| + t.string :kind + t.string :name + end + + before { HasCustomInheritanceColumn.reset_column_information } + + with_model :KindedChild, superclass: HasCustomInheritanceColumn do + table(false) + end + + it "writes to the custom column" do + KindedChild.create!(name: "x") + expect(HasCustomInheritanceColumn.first.kind).to eq "KindedChild" + end + + it "accepts a Symbol inheritance_column" do + expect(HasCustomInheritanceColumn.inheritance_column).to eq "kind" + end + end + + # The anchor case: a validation or teardown that hard-codes "type" passes + # every other example here and still scopes on the wrong column. + describe "with both a type column and a custom inheritance_column" do + with_table(HasCustomInheritanceColumnAndTypeColumn.table_name) do |t| + t.string :kind + t.string :type + t.string :name + end + + before { HasCustomInheritanceColumnAndTypeColumn.reset_column_information } + + with_model :KindedTypeChild, superclass: HasCustomInheritanceColumnAndTypeColumn do + table(false) + end + + it "writes the custom column and leaves type null" do + KindedTypeChild.create!(name: "x") + row = HasCustomInheritanceColumnAndTypeColumn.first + expect(row.kind).to eq "KindedTypeChild" + expect(row.type).to be_nil + end + end + + describe "with a with_model superclass" do + with_model :Parent do + table do |t| + t.string :type + t.string :name + end + end + + with_model :StringChild, superclass: "Parent" do + table(false) + end + + with_model :SymbolChild, superclass: :Parent do + table(false) + end + + with_model :CallableChild, superclass: -> { Parent } do + table(false) + end + + it "resolves a String superclass" do + expect(StringChild.superclass).to eq Parent + expect(StringChild.table_name).to eq Parent.table_name + end + + # The same spelling with_model names its own models with. + it "resolves a Symbol superclass" do + expect(SymbolChild.superclass).to eq Parent + expect(SymbolChild.table_name).to eq Parent.table_name + end + + it "resolves a callable superclass" do + expect(CallableChild.superclass).to eq Parent + end + + it "round-trips through the parent" do + StringChild.create!(name: "x") + expect(Parent.first).to be_a StringChild + end + + # Second example: the parent class object is rebuilt every example, so a + # superclass memoized on the first resolution refers to a dead class here. + it "resolves against the current parent in a later example" do + expect(StringChild.superclass).to eq Parent + expect(StringChild.superclass).to be Parent + end + end + + describe "namespaced constants" do + with_table(HasTypeColumn.table_name) do |t| + t.string :type + t.string :name + end + + before { HasTypeColumn.reset_column_information } + + # WithModel::ConstantStubber resolves a namespaced name with `const_get` and + # stubs only the last segment, so the namespace has to exist already. This + # file's own module supplies one. + with_model "TableFalseSpec::Step", superclass: HasTypeColumn do + table(false) + end + + it "stores and round-trips the fully qualified name" do + Step.create!(name: "x") + expect(HasTypeColumn.first.type).to eq "TableFalseSpec::Step" + expect(HasTypeColumn.first).to be_a Step + end + end + + # A missing table is a fact about the schema at that moment, not about the + # model, so it is allowed: the table may arrive later in the example, or its + # absence may be the very thing under test. + describe "when the superclass's table does not exist" do + with_model :Orphan, superclass: HasMissingTable do + table(false) + end + + it "still defines the model, which inherits the missing table's name" do + # Not `new`: building an instance reads column information, which is + # itself a use of the absent table. + expect(Orphan).to be < HasMissingTable + expect(Orphan.table_name).to eq HasMissingTable.table_name + end + + it "lets Active Record raise its own error, naming the table" do + expect { Orphan.create!(name: "x") } + .to raise_error(ActiveRecord::StatementInvalid, /#{HasMissingTable.table_name}/) + end + + it "tears down without failing an example that otherwise passed" do + # Reaching the end of this example at all is the assertion: teardown has + # no rows to delete and must not go looking for them. + expect(Orphan.name).to eq "Orphan" + end + end + + describe "when the superclass's table is created during the example" do + with_model :LateChild, superclass: HasLateTable do + table(false) + end + + after do + ActiveRecord::Base.connection.drop_table HasLateTable.table_name, if_exists: true + end + + it "inherits the table once it exists" do + ActiveRecord::Base.connection.create_table HasLateTable.table_name, force: true do |t| + t.string :type + t.string :name + end + HasLateTable.reset_column_information + + LateChild.create!(name: "late") + + expect(HasLateTable.count).to eq 1 + expect(HasLateTable.first).to be_a LateChild + end + end + + describe "refusals" do + def create_model(superclass:, &dsl) + model = WithModel::Model.new(:Refused, superclass: superclass) + WithModel::Model::DSL.new(model).instance_eval(&dsl || proc { table(false) }) + model.create + end + + it "names the model, which the superclass's name does not identify" do + expect { create_model(superclass: ActiveRecord::Base) } + .to raise_error(WithModel::InvalidSuperclass, /with_model :Refused has no table of its own/) + end + + it "refuses ActiveRecord::Base as the superclass" do + expect { create_model(superclass: ActiveRecord::Base) } + .to raise_error(WithModel::InvalidSuperclass, /ActiveRecord::Base has none to inherit/) + end + + it "refuses an abstract superclass" do + expect { create_model(superclass: IsAbstract) } + .to raise_error(WithModel::InvalidSuperclass, /IsAbstract has none to inherit/) + end + + it "refuses an abstract superclass that a Rails app would call ApplicationRecord" do + expect { create_model(superclass: ApplicationRecord) } + .to raise_error(WithModel::InvalidSuperclass, /ApplicationRecord has none to inherit/) + end + + context "when the superclass table lacks the inheritance column" do + with_table(HasNoTypeColumn.table_name) do |t| + t.string :name + end + + before { HasNoTypeColumn.reset_column_information } + + it "refuses and names the column it looked for" do + expect { create_model(superclass: HasNoTypeColumn) } + .to raise_error(WithModel::InvalidSuperclass, /"type" column/) + end + end + + context "when the superclass disables inheritance" do + with_table(HasNilInheritanceColumn.table_name) do |t| + t.string :name + end + + it "refuses" do + expect { create_model(superclass: HasNilInheritanceColumn) } + .to raise_error(WithModel::InvalidSuperclass) + end + end + + it "refuses a table(false) call that also passes a block" do + expect do + create_model(superclass: HasTypeColumn) { table(false) { |t| t.string :nope } } + end.to raise_error(ArgumentError, /table does not take a block when its first argument is falsy/) + end + + describe "a superclass that cannot be found" do + it "quotes the reason it could not be resolved" do + expect { create_model(superclass: "NoSuchParent") } + .to raise_error(WithModel::MissingSuperclass, + /superclass "NoSuchParent" could not be resolved: uninitialized constant NoSuchParent/) + end + + it "names the segment of a namespaced superclass that is missing" do + expect { create_model(superclass: "NoSuchNamespace::Deep::Parent") } + .to raise_error(WithModel::MissingSuperclass, /uninitialized constant NoSuchNamespace\./) + end + + it "reports an unresolvable Symbol as the Symbol that was passed" do + expect { create_model(superclass: :NoSuchParent) } + .to raise_error(WithModel::MissingSuperclass, + /superclass :NoSuchParent could not be resolved: uninitialized constant NoSuchParent/) + end + + it "keeps the NameError as its cause" do + # Ruby sets this for a raise inside a rescue, so the original backtrace + # stays reachable. + expect { create_model(superclass: "NoSuchParent") } + .to raise_error(WithModel::MissingSuperclass) { |error| + expect(error.cause).to be_a NameError + } + end + end + + it "tells a missing superclass apart from an unusable one, but catches both" do + expect(WithModel::MissingSuperclass).to be < WithModel::InvalidSuperclass + expect(WithModel::InvalidSuperclass).to be < ArgumentError + end + + it "says how to name a class that is not defined yet" do + expect { create_model(superclass: 42) } + .to raise_error(WithModel::InvalidSuperclass, + /name it with a String or a Symbol, or pass a callable returning it/) + end + + it "refuses a value that only has a to_s" do + # Every object has one, so honoring it would look up the constant "42". + expect { create_model(superclass: 42) } + .to raise_error(WithModel::InvalidSuperclass, /but was 42/) + end + + it "refuses a callable returning a non-ActiveRecord value" do + expect { create_model(superclass: -> { 42 }) } + .to raise_error(WithModel::InvalidSuperclass, /but was 42/) + end + end + end + + RSpec.describe "omitting table" do + def capture_deprecations + collected = [] + WithModel.deprecator.behavior = ->(message, *) { collected << message } + yield + collected + ensure + WithModel.deprecator.behavior = :raise + end + + # Definition-time warning, so the assertion drives a throwaway example group + # rather than relying on this file's own load order. + def define_group(&body) + RSpec::Core::ExampleGroup.describe("throwaway", &body) + end + + it "warns when no table is specified" do + messages = capture_deprecations do + define_group { with_model(:Warned) } + end + expect(messages.join).to match(/table/) + end + + it "names the 3.0 horizon in the warning" do + messages = capture_deprecations do + define_group { with_model(:Warned) } + end + expect(messages.join).to include("3.0") + end + + # Left to itself ActiveSupport::Deprecation blames the first frame it does not + # recognize as Rails or the standard library, which is with_model's own source + # - the same line for every omission in a suite. + it "blames the with_model call rather than with_model's own source" do + messages = capture_deprecations do + define_group { with_model(:Warned) } + end + + expect(messages.join).to include("#{__FILE__}:#{__LINE__ - 3}") + expect(messages.join).not_to include("lib/with_model.rb") + end + + it "does not warn for table(false)" do + messages = capture_deprecations do + define_group { with_model(:Quiet, superclass: HasTypeColumn) { table(false) } } + end + expect(messages).to be_empty + end + + it "does not warn for an empty table block" do + messages = capture_deprecations do + define_group do + with_model(:Quiet) do + table do + end + end + end + end + expect(messages).to be_empty + end + + it "does not warn for a `table` call with no arguments" do + messages = capture_deprecations do + define_group { with_model(:Quiet) { table } } + end + expect(messages).to be_empty + end + end +end diff --git a/spec/with_model_spec.rb b/spec/with_model_spec.rb index a8697e1..abf3f67 100644 --- a/spec/with_model_spec.rb +++ b/spec/with_model_spec.rb @@ -96,7 +96,9 @@ def fancy_title shadowing_example_ran = false context "with the with_model block" do - with_model :MyConst + with_model :MyConst do + table + end after do shadowing_example_ran = true @@ -116,7 +118,9 @@ def fancy_title end describe "with a plural name" do - with_model :BlogPosts + with_model :BlogPosts do + table + end it "does not singularize the constant name" do expect(BlogPosts).to be @@ -125,7 +129,9 @@ def fancy_title end describe "with a name containing capital letters" do - with_model :BlogPost + with_model :BlogPost do + table + end it "tableizes the table name" do expect(BlogPost.table_name).to match(/_blog_posts_/) @@ -134,7 +140,9 @@ def fancy_title end describe "with a name with underscores" do - with_model :blog_post + with_model :blog_post do + table + end it "constantizes the name" do expect(BlogPost).to be @@ -149,7 +157,9 @@ def fancy_title describe "with a name which is namespaced" do before { stub_const("Stuff", Module.new) } - with_model :"Stuff::BlogPost" + with_model :"Stuff::BlogPost" do + table + end it "creates the model in the namespace" do expect(defined?(BlogPost)).to be_falsey @@ -159,6 +169,7 @@ def fancy_title describe "using the constant in the model block" do with_model :BlogPost do + table model do raise "I am not myself!" unless self == BlogPost end @@ -180,6 +191,7 @@ def foo before { stub_const("AMixin", mixin) } with_model :WithAMixin do + table model do include AMixin end @@ -207,6 +219,7 @@ def self.included(klass) before { stub_const("AMixin", mixin) } with_model :WithAClassEval do + table model do include AMixin @@ -239,8 +252,13 @@ def my_method end end + # Omitting `table` is deprecated, but still supported for the rest of 2.x. + # These three contexts are its only coverage, so they keep omitting it and + # silence the warning rather than adopting the replacement spelling. context "without a block" do - with_model :BlogPost + WithModel.deprecator.silence do + with_model :BlogPost + end it "acts like a normal ActiveRecord model" do record = BlogPost.create! @@ -257,7 +275,9 @@ def my_method end context "with an empty block" do - with_model(:BlogPost) {} + WithModel.deprecator.silence do + with_model(:BlogPost) {} + end it "acts like a normal ActiveRecord model" do record = BlogPost.create! @@ -304,7 +324,9 @@ def my_method end context "without a table or model block" do - with_model :BlogPost + WithModel.deprecator.silence do + with_model :BlogPost + end it "acts like a normal ActiveRecord model" do expect(BlogPost.columns.map(&:name)).to eq ["id"] diff --git a/test/declaration_order_test.rb b/test/declaration_order_test.rb index 1c1bb19..8f00cec 100644 --- a/test/declaration_order_test.rb +++ b/test/declaration_order_test.rb @@ -2,6 +2,10 @@ require "test_helper" +# Models are created in declaration order and destroyed in reverse, so each one +# can depend on the ones above it for as long as it exists. Both halves of that +# are tested here. +# # The model block is evaluated when the model is created, so a model that # refers to another one by constant can only work if the model it refers to # was created first. @@ -22,10 +26,36 @@ class DeclarationOrderTest < Minitest::Test end end + with_model :Shelf do + table do |t| + t.string "name" + end + end + + # `to_table` because `foreign_key: true` would infer a table name from the + # column rather than asking the model, and with_model's table names are + # generated. + with_model :Jar do + table do |t| + t.references "shelf", foreign_key: {to_table: Shelf.table_name} + end + end + def test_a_model_can_refer_to_one_declared_above_it author = Author.create!(name: "Ursula K. Le Guin") book = Book.create!(author: author) assert_equal author, book.reload.author end + + # The assertion for teardown order is that this test finishes cleanly. A + # referenced table cannot be dropped while a row still points at it, so tearing + # these down in declaration order raises ActiveRecord::InvalidForeignKey after + # the body has already passed. + def test_a_table_can_point_at_one_declared_above_it + shelf = Shelf.create!(name: "pantry") + jar = Jar.create!(shelf_id: shelf.id) + + assert_equal shelf.id, jar.reload.shelf_id + end end diff --git a/test/spec_dsl_test.rb b/test/spec_dsl_test.rb new file mode 100644 index 0000000..2236c99 --- /dev/null +++ b/test/spec_dsl_test.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require "test_helper" + +# The README offers Minitest::Spec as well as Minitest::Test. Minitest::Spec +# inherits from Minitest::Test, so extending Minitest::Test reaches describe +# blocks too, and with_model needs no separate wiring for them. +describe "with_model in a spec-style suite" do + with_model :Sofa do + table do |t| + t.string "type" + t.string "fabric" + end + end + + with_model :Loveseat, superclass: :Sofa do + table(false) + end + + it "creates the models a describe block declares" do + assert_empty Sofa.all, "another test's rows outlived it" + + Loveseat.create!(fabric: "tweed") + + assert_equal Sofa.table_name, Loveseat.table_name + assert_instance_of Loveseat, Sofa.first + end + + # Every describe is its own subclass of the one around it, so a nested block + # inherits the models declared above it and can add more of its own. + describe "and a nested describe" do + with_model :Ottoman do + table do |t| + t.string "name" + end + end + + it "sees the models from both levels" do + assert_empty Sofa.all, "another test's rows outlived it" + + Ottoman.create!(name: "pouf") + Loveseat.create!(fabric: "velvet") + + assert_equal 1, Ottoman.count + assert_equal 1, Sofa.count + end + end +end diff --git a/test/sti_test.rb b/test/sti_test.rb new file mode 100644 index 0000000..ae0761f --- /dev/null +++ b/test/sti_test.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +require "test_helper" + +# Stands in for an application's own model, the usual reason to want single table +# inheritance: its table is created once and outlives every test here, so a +# with_model child's rows cannot be disposed of by dropping the table. +module AppModels + class Cupboard < ActiveRecord::Base + end +end + +ActiveRecord::Base.connection.create_table AppModels::Cupboard.table_name, force: true do |t| + t.string "type" + t.string "name" +end + +# A with_model parent and an STI child in the same test case. The child's +# superclass is resolved per-example, so this only works if the parent is +# created before the child and destroyed after it. +class StiTest < Minitest::Test + with_model :Vehicle do + table do |t| + t.string "type" + t.string "name" + end + end + + with_model :Truck, superclass: -> { Vehicle } do + table(false) + end + + with_model :Chest, superclass: "AppModels::Cupboard" do + table(false) + end + + def test_the_child_shares_the_parents_table + assert_equal Vehicle.table_name, Truck.table_name + end + + def test_the_child_stores_its_own_type + truck = Truck.create!(name: "Ford F-150") + + assert_equal "Truck", truck.reload.type + assert_instance_of Truck, Vehicle.first + end + + # Rows in a table the child does not own have to be deleted when it goes away, + # since nothing else will: Cupboard's table is still standing afterwards, and a + # row naming a class that no longer exists makes it unloadable. Both tests write + # one and first insist the table is empty, so whichever minitest happens to run + # second fails if the rows survived teardown - whatever order the seed picks. + def test_the_childs_rows_do_not_outlive_it + assert_empty AppModels::Cupboard.all, "a previous test's rows outlived it" + + Chest.create!(name: "sideboard") + + assert_equal 1, AppModels::Cupboard.count + end + + def test_the_childs_rows_leave_the_superclass_loadable + assert_empty AppModels::Cupboard.all, "a previous test's rows outlived it" + + Chest.create!(name: "wardrobe") + + assert_instance_of Chest, AppModels::Cupboard.first + end +end diff --git a/test/test_helper.rb b/test/test_helper.rb index 9b0cb25..eca1c97 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -7,6 +7,8 @@ WithModel.runner = :minitest +WithModel.deprecator.behavior = :raise + Minitest::Test.class_eval do extend WithModel end