From 81d0653f84886709f5f6f69f3953b2e42f4efcea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20Barri=C3=A9?= Date: Thu, 1 Apr 2021 15:04:13 -0400 Subject: [PATCH] Simplify ActiveModel & ActiveRecord Type::Registry ActiveRecord::Type::Registry doesn't need to inherit from ActiveModel::Type::Registry, and it makes both classes more simple. Co-authored-by: Adrianna Chang --- activemodel/lib/active_model/type.rb | 7 +-- activemodel/lib/active_model/type/registry.rb | 47 +++---------------- activemodel/test/cases/type_test.rb | 23 +++++++++ .../type/adapter_specific_registry.rb | 39 ++++++++++++--- 4 files changed, 66 insertions(+), 50 deletions(-) create mode 100644 activemodel/test/cases/type_test.rb diff --git a/activemodel/lib/active_model/type.rb b/activemodel/lib/active_model/type.rb index 1d7a26fff5..735d0e7f57 100644 --- a/activemodel/lib/active_model/type.rb +++ b/activemodel/lib/active_model/type.rb @@ -24,9 +24,10 @@ module ActiveModel class << self attr_accessor :registry # :nodoc: - # Add a new type to the registry, allowing it to be gotten through ActiveModel::Type#lookup - def register(type_name, klass = nil, **options, &block) - registry.register(type_name, klass, **options, &block) + # Add a new type to the registry, allowing it to be referenced as a + # symbol by {attribute}[rdoc-ref:Attributes::ClassMethods#attribute]. + def register(type_name, klass = nil, &block) + registry.register(type_name, klass, &block) end def lookup(*args, **kwargs) # :nodoc: diff --git a/activemodel/lib/active_model/type/registry.rb b/activemodel/lib/active_model/type/registry.rb index a391250ad5..2d874acfe9 100644 --- a/activemodel/lib/active_model/type/registry.rb +++ b/activemodel/lib/active_model/type/registry.rb @@ -1,31 +1,30 @@ # frozen_string_literal: true module ActiveModel - # :stopdoc: module Type - class Registry + class Registry # :nodoc: def initialize - @registrations = [] + @registrations = {} end - def initialize_dup(other) + def initialize_copy(other) @registrations = @registrations.dup super end - def register(type_name, klass = nil, **options, &block) + def register(type_name, klass = nil, &block) unless block_given? block = proc { |_, *args| klass.new(*args) } block.ruby2_keywords if block.respond_to?(:ruby2_keywords) end - registrations << registration_klass.new(type_name, block, **options) + registrations[type_name] = block end def lookup(symbol, *args, **kwargs) - registration = find_registration(symbol, *args, **kwargs) + registration = registrations[symbol] if registration - registration.call(self, symbol, *args, **kwargs) + registration.call(symbol, *args, **kwargs) else raise ArgumentError, "Unknown type #{symbol.inspect}" end @@ -33,38 +32,6 @@ module ActiveModel private attr_reader :registrations - - def registration_klass - Registration - end - - def find_registration(symbol, *args, **kwargs) - registrations.find { |r| r.matches?(symbol, *args, **kwargs) } - end - end - - class Registration - # Options must be taken because of https://bugs.ruby-lang.org/issues/10856 - def initialize(name, block, **) - @name = name - @block = block - end - - def call(_registry, *args, **kwargs) - if kwargs.any? # https://bugs.ruby-lang.org/issues/10856 - block.call(*args, **kwargs) - else - block.call(*args) - end - end - - def matches?(type_name, *args, **kwargs) - type_name == name - end - - private - attr_reader :name, :block end end - # :startdoc: end diff --git a/activemodel/test/cases/type_test.rb b/activemodel/test/cases/type_test.rb new file mode 100644 index 0000000000..70f3f6052e --- /dev/null +++ b/activemodel/test/cases/type_test.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require "cases/helper" + +module ActiveModel + class TypeTest < ActiveModel::TestCase + setup do + @old_registry = ActiveModel::Type.registry + ActiveModel::Type.registry = @old_registry.dup + end + + teardown do + ActiveModel::Type.registry = @old_registry + end + + test "registering a new type" do + type = Struct.new(:args) + ActiveModel::Type.register(:foo, type) + + assert_equal type.new(:arg), ActiveModel::Type.lookup(:foo, :arg) + end + end +end diff --git a/activerecord/lib/active_record/type/adapter_specific_registry.rb b/activerecord/lib/active_record/type/adapter_specific_registry.rb index 77590b821a..9f63fb5316 100644 --- a/activerecord/lib/active_record/type/adapter_specific_registry.rb +++ b/activerecord/lib/active_record/type/adapter_specific_registry.rb @@ -5,15 +5,40 @@ require "active_model/type/registry" module ActiveRecord # :stopdoc: module Type - class AdapterSpecificRegistry < ActiveModel::Type::Registry + class AdapterSpecificRegistry # :nodoc: + def initialize + @registrations = [] + end + + def initialize_copy(other) + @registrations = @registrations.dup + super + end + def add_modifier(options, klass, **args) registrations << DecorationRegistration.new(options, klass, **args) end - private - def registration_klass - Registration + def register(type_name, klass = nil, **options, &block) + unless block_given? + block = proc { |_, *args| klass.new(*args) } + block.ruby2_keywords if block.respond_to?(:ruby2_keywords) end + registrations << Registration.new(type_name, block, **options) + end + + def lookup(symbol, *args, **kwargs) + registration = find_registration(symbol, *args, **kwargs) + + if registration + registration.call(self, symbol, *args, **kwargs) + else + raise ArgumentError, "Unknown type #{symbol.inspect}" + end + end + + private + attr_reader :registrations def find_registration(symbol, *args, **kwargs) registrations @@ -22,7 +47,7 @@ module ActiveRecord end end - class Registration + class Registration # :nodoc: def initialize(name, block, adapter: nil, override: nil) @name = name @block = block @@ -89,7 +114,7 @@ module ActiveRecord end end - class DecorationRegistration < Registration + class DecorationRegistration < Registration # :nodoc: def initialize(options, klass, adapter: nil) @options = options @klass = klass @@ -120,7 +145,7 @@ module ActiveRecord end end - class TypeConflictError < StandardError + class TypeConflictError < StandardError # :nodoc: end # :startdoc: end