Skip to content

Commit

Permalink
Refactor inheritance in attributes and backend plugins
Browse files Browse the repository at this point in the history
- removes register methods which we don't need
- freezes attributes/backends after inheritance, to ensure they are not
  modified later
  • Loading branch information
shioyama committed Nov 11, 2020
1 parent 1612ce0 commit 6dbb884
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 20 deletions.
30 changes: 17 additions & 13 deletions lib/mobility/plugins/attributes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,15 @@ def inspect
end

included_hook do |klass|
klass.extend ClassMethods
@names.each { |name| klass.register_mobility_attribute(name) }
names = @names

klass.class_eval do
extend ClassMethods
names.each { |name| mobility_attributes << name.to_s }
mobility_attributes.uniq!
rescue FrozenError
raise FrozenAttributesError, "Attempting to translate these attributes on #{klass}, which has already been subclassed: #{names.join(', ')}."
end
end

module ClassMethods
Expand All @@ -44,23 +51,20 @@ def mobility_attribute?(name)
mobility_attributes.include?(name.to_s)
end

# Register a new attribute name. Public, but treat as internal.
# @param [String, Symbol] Attribute name
def register_mobility_attribute(name)
(self.mobility_attributes << name.to_s).uniq!
end

def inherited(klass)
super
mobility_attributes.each { |name| klass.register_mobility_attribute(name) }
end

# Return translated attribute names on this model.
# @return [Array<String>] Attribute names
def mobility_attributes
@mobility_attributes ||= []
end

def inherited(klass)
super
attrs = mobility_attributes.freeze # ensure attributes are not modified after being inherited
klass.class_eval { @mobility_attributes = attrs.dup }
end
end

class FrozenAttributesError < Error; end
end

register_plugin(:attributes, Attributes)
Expand Down
14 changes: 7 additions & 7 deletions lib/mobility/plugins/backend.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,11 @@ def included(klass)

backend_class.setup_model(klass, names)

@names.each do |name|
klass.register_mobility_backend_class(name, @backend_class)
names = @names
backend_class = @backend_class

klass.class_eval do
names.each { |name| mobility_backend_classes[name.to_sym] = backend_class }
end

backend_class
Expand Down Expand Up @@ -137,12 +140,9 @@ def mobility_backend_class(name)
raise KeyError, "No backend for: #{name}"
end

def register_mobility_backend_class(name, backend_class)
mobility_backend_classes[name.to_sym] = backend_class
end

def inherited(klass)
klass.mobility_backend_classes.merge!(@mobility_backend_classes)
parent_classes = mobility_backend_classes.freeze # ensure backend classes are not modified after being inherited
klass.class_eval { @mobility_backend_classes = parent_classes.dup }
super
end

Expand Down
47 changes: 47 additions & 0 deletions spec/mobility/plugins/attributes_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,18 @@
end
end

describe ".mobility_attributes" do
it "returns attributes included in one of multiple pluggable modules" do
translations1 = translations_class.new("title", "content")
translations2 = translations_class.new("author")
klass = Class.new
klass.include translations1
klass.include translations2

expect(klass.mobility_attributes).to eq(%w[title content author])
end
end

describe ".mobility_attribute?" do
it "returns true if name is an attribute" do
translations = translations_class.new("title", "content")
Expand All @@ -46,4 +58,39 @@
expect(klass.mobility_attribute?("foo")).to eq(false)
end
end

describe "inheritance" do
it "inherits mobility attributes from parent" do
mod1 = translations_class.new("title", "content")
klass1 = Class.new
klass1.include mod1

klass2 = Class.new(klass1)

expect(klass1.mobility_attributes).to match_array(%w[title content])
expect(klass2.mobility_attributes).to match_array(%w[title content])

mod2 = translations_class.new("author")
klass2.include mod2

expect(klass1.mobility_attributes).to match_array(%w[title content])
expect(klass2.mobility_attributes).to match_array(%w[title content author])
end

it "freezes inherited attributes to ensure they are not changed after subclassing" do
mod1 = translations_class.new("title")
stub_const("Foo", Class.new)
klass1 = Foo
klass1.include mod1

Class.new(klass1)
expect(klass1.mobility_attributes).to be_frozen

mod2 = translations_class.new("content")
expect {
klass1.include mod2
}.to raise_error(Mobility::Plugins::Attributes::FrozenAttributesError,
"Attempting to translate these attributes on Foo, which has already been subclassed: content.")
end
end
end
12 changes: 12 additions & 0 deletions spec/mobility/plugins/backend_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,18 @@
content_backend_class = model_subclass.mobility_backend_class("content")
expect(content_backend_class).to be <(backend_class_2)
end

it "freezes backends after subclassing" do
backend_class_1 = Class.new
backend_class_1.include Mobility::Backend

mod1 = translations_class.new("title", backend: backend_class_1)
model_class.include mod1

Class.new(model_class)

expect(model_class.send(:mobility_backend_classes)).to be_frozen
end
end

describe "#mobility_backends" do
Expand Down

0 comments on commit 6dbb884

Please sign in to comment.