Skip to content

Commit

Permalink
FIX: add_to_serializer not correctly accounting for inheritance chains
Browse files Browse the repository at this point in the history
This is a very long standing bug we had, if a plugin attempted to amend a
serializer core was not "correcting" the situation for all descendant classes
this often only showed up in production cause production eager loads serializers
prior to plugins amending them.

This is a critical fix for various plugins
  • Loading branch information
SamSaffron committed Aug 27, 2019
1 parent 6477531 commit a3d42e2
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 9 deletions.
19 changes: 11 additions & 8 deletions lib/plugin/instance.rb
Original file line number Diff line number Diff line change
Expand Up @@ -90,18 +90,21 @@ def enabled?

def add_to_serializer(serializer, attr, define_include_method = true, &block)
reloadable_patch do |plugin|
klass = "#{serializer.to_s.classify}Serializer".constantize rescue "#{serializer.to_s}Serializer".constantize
base = "#{serializer.to_s.classify}Serializer".constantize rescue "#{serializer.to_s}Serializer".constantize

unless attr.to_s.start_with?("include_")
klass.attributes(attr)
# we have to work through descendants cause serializers may already be baked and cached
([base] + base.descendants).each do |klass|
unless attr.to_s.start_with?("include_")
klass.attributes(attr)

if define_include_method
# Don't include serialized methods if the plugin is disabled
klass.public_send(:define_method, "include_#{attr}?") { plugin.enabled? }
if define_include_method
# Don't include serialized methods if the plugin is disabled
klass.public_send(:define_method, "include_#{attr}?") { plugin.enabled? }
end
end
end

klass.public_send(:define_method, attr, &block)
klass.public_send(:define_method, attr, &block)
end
end
end

Expand Down
34 changes: 33 additions & 1 deletion spec/components/plugin/instance_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,25 @@
context "with a plugin that extends things" do

class Trout; end
class TroutSerializer < ApplicationSerializer; end
class TroutSerializer < ApplicationSerializer
attribute :name

def name
"a trout"
end
end
class TroutJuniorSerializer < TroutSerializer

attribute :i_am_child

def name
"a trout jr"
end

def i_am_child
true
end
end

class TroutPlugin < Plugin::Instance
attr_accessor :enabled
Expand All @@ -47,6 +65,12 @@ def enabled?
@plugin = TroutPlugin.new
@trout = Trout.new

poison = TroutSerializer.new(@trout)
poison.attributes

poison = TroutJuniorSerializer.new(@trout)
poison.attributes

# New method
@plugin.add_to_class(:trout, :status?) { "evil" }

Expand All @@ -57,7 +81,9 @@ def enabled?

# Serializer
@plugin.add_to_serializer(:trout, :scales) { 1024 }

@serializer = TroutSerializer.new(@trout)
@child_serializer = TroutJuniorSerializer.new(@trout)
end

after do
Expand All @@ -74,14 +100,20 @@ def enabled?
expect(@serializer.scales).to eq(1024)
expect(@serializer.include_scales?).to eq(true)

expect(@child_serializer.attributes[:scales]).to eq(1024)

# When a plugin is disabled
@plugin.enabled = false
expect(@trout.status?).to eq(nil)
DiscourseEvent.trigger(:hello)
expect(@hello_count).to eq(1)
expect(@serializer.scales).to eq(1024)
expect(@serializer.include_scales?).to eq(false)
expect(@serializer.name).to eq("a trout")

expect(@child_serializer.scales).to eq(1024)
expect(@child_serializer.include_scales?).to eq(false)
expect(@child_serializer.name).to eq("a trout jr")
end
end
end
Expand Down

0 comments on commit a3d42e2

Please sign in to comment.