From 4590d7729e241cb7f66e018a2a9759cb3baa36e5 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Mon, 21 Sep 2015 09:40:04 -0600 Subject: Simplify the implementation of Active Model's type registry Things like decorations, overrides, and priorities only matter for Active Record, so the Active Model registry can be implemented much more simply. At this point, I wonder if having Active Record's registry inherit from Active Model's is even worth the trouble? The Active Model class was also missing test cases, which have been backfilled. This removes the error when two types are registered with the same name, but given that Active Model is meant to be significantly more generic, I do not think this is an issue for now. If we want, we can raise an error at the point that someone tries to register it. --- activemodel/lib/active_model/type/registry.rb | 107 +++----------------------- activemodel/test/cases/type/registry_test.rb | 39 ++++++++++ 2 files changed, 49 insertions(+), 97 deletions(-) create mode 100644 activemodel/test/cases/type/registry_test.rb (limited to 'activemodel') diff --git a/activemodel/lib/active_model/type/registry.rb b/activemodel/lib/active_model/type/registry.rb index 8070a4bd38..adc88eb624 100644 --- a/activemodel/lib/active_model/type/registry.rb +++ b/activemodel/lib/active_model/type/registry.rb @@ -12,9 +12,7 @@ module ActiveModel end def lookup(symbol, *args) - registration = registrations - .select { |r| r.matches?(symbol, *args) } - .max + registration = find_registration(symbol, *args) if registration registration.call(self, symbol, *args) @@ -23,30 +21,26 @@ module ActiveModel end end - def add_modifier(options, klass, **args) - registrations << decoration_registration_klass.new(options, klass, **args) - end - protected attr_reader :registrations - + private - + def registration_klass Registration end - - def decoration_registration_klass - DecorationRegistration + + def find_registration(symbol, *args) + registrations.find { |r| r.matches?(symbol, *args) } end end class Registration - def initialize(name, block, override: nil) + # Options must be taken because of https://bugs.ruby-lang.org/issues/10856 + def initialize(name, block, **) @name = name @block = block - @override = override end def call(_registry, *args, **kwargs) @@ -58,94 +52,13 @@ module ActiveModel end def matches?(type_name, *args, **kwargs) - type_name == name# && matches_adapter?(**kwargs) - end - - def <=>(other) - # if conflicts_with?(other) - # raise TypeConflictError.new("Type #{name} was registered for all - # adapters, but shadows a native type with - # the same name for #{other.adapter}".squish) - # end - priority <=> other.priority + type_name == name end protected - attr_reader :name, :block, :override - - def priority - result = 0 - # if adapter - # result |= 1 - # end - if override - result |= 2 - end - result - end - - # def priority_except_adapter - # priority & 0b111111100 - # end - - private - - # def matches_adapter?(adapter: nil, **) - # (self.adapter.nil? || adapter == self.adapter) - # end - - # def conflicts_with?(other) - # same_priority_except_adapter?(other) && - # has_adapter_conflict?(other) - # end - - # def same_priority_except_adapter?(other) - # priority_except_adapter == other.priority_except_adapter - # end - - # def has_adapter_conflict?(other) - # (override.nil? && other.adapter) || - # (adapter && other.override.nil?) - # end + attr_reader :name, :block end - - class DecorationRegistration < Registration - def initialize(options, klass) - @options = options - @klass = klass - # @adapter = adapter - end - - def call(registry, *args, **kwargs) - subtype = registry.lookup(*args, **kwargs.except(*options.keys)) - klass.new(subtype) - end - - def matches?(*args, **kwargs) - matches_options?(**kwargs) - end - - def priority - super | 4 - end - - protected - - attr_reader :options, :klass - - private - - def matches_options?(**kwargs) - options.all? do |key, value| - kwargs[key] == value - end - end - end - end - - class TypeConflictError < StandardError end - # :startdoc: end diff --git a/activemodel/test/cases/type/registry_test.rb b/activemodel/test/cases/type/registry_test.rb new file mode 100644 index 0000000000..2a48998a62 --- /dev/null +++ b/activemodel/test/cases/type/registry_test.rb @@ -0,0 +1,39 @@ +require "cases/helper" +require "active_model/type" + +module ActiveModel + class RegistryTest < ActiveModel::TestCase + test "a class can be registered for a symbol" do + registry = Type::Registry.new + registry.register(:foo, ::String) + registry.register(:bar, ::Array) + + assert_equal "", registry.lookup(:foo) + assert_equal [], registry.lookup(:bar) + end + + test "a block can be registered" do + registry = Type::Registry.new + registry.register(:foo) do |*args| + [*args, "block for foo"] + end + registry.register(:bar) do |*args| + [*args, "block for bar"] + end + + assert_equal [:foo, 1, "block for foo"], registry.lookup(:foo, 1) + assert_equal [:foo, 2, "block for foo"], registry.lookup(:foo, 2) + assert_equal [:bar, 1, 2, 3, "block for bar"], registry.lookup(:bar, 1, 2, 3) + end + + test "a reasonable error is given when no type is found" do + registry = Type::Registry.new + + e = assert_raises(ArgumentError) do + registry.lookup(:foo) + end + + assert_equal "Unknown type :foo", e.message + end + end +end -- cgit v1.2.3