Skip to content

Instantly share code, notes, and snippets.

@brushbox

brushbox/breaking_suppress.rb

Last active May 14, 2016 01:40
Show Gist options
  • Save brushbox/c77636a60452c847de32c772a9e9d81e to your computer and use it in GitHub Desktop.
Save brushbox/c77636a60452c847de32c772a9e9d81e to your computer and use it in GitHub Desktop.
Download ZIP
Is suppress broken?
Raw
breaking_suppress.rb
# This code shows how the implementation of ActiveRecord::Suppressor#suppress is broken if nested.
Notification.suppress do
#... suppressed code calling down into other things.
complex_logic
# NOT supressed! Surprise!
end
# this method should be considered as passing control down through enough levels of logic that
# it is reasonable to believe that the person calling the inner suppress is unaware that
# Notification is already suppressed.
def complex_logic
#... suppressed (but we aren’t aware of this)
Notifications.suppress do
#... suppressed
end
#... NOT suppressed, and we are OK with that.
end
# If the above isn't clear it is simply trying to show this:
Notification.suppress do
# In a suppress block: suppressed here
# ...
Notification.suppress do
# In a suppress block: suppressed here
# ...
end
# In a suppress block: but NOT suppressed here!
# ...
end
Raw
suppress_fix.rb
# this shows a fix to ActiveRecord::Suppressor#suppress that, while fixing the nesting issue,
# will lead to other confusion in the face of nesting.
module ClassMethods
def suppress(&block)
old_suppressed = SuppressorRegistry.suppressed[name]
SuppressorRegistry.suppressed[name] = true
yield
ensure
SuppressorRegistry.suppressed[name] = old_suppressed
end
end
# the above fixes the previous issue - but the person who implemented the inner suppress block
# may be surprised to find that Notification remains suppressed outside their suppress block.
# E.g:
Notification.suppress do
#... suppressed
complex_logic
#... suppressed: FIXED
end
def complex_logic
#... suppressed (but we aren’t aware of this)
Notifications.suppress do
#... suppressed
end
#... suppressed. SURPRISE!
end
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment