Have you ever met a method in the codebase that seemed very much unused, but you were not 100% sure and didn’t dare to remove it?
In this tutorial we are gonna build a tool that sends a notification if a marked method gets invoked. This helps us to double-check if the method in question is indeed unused before removing it.
A real life example for Ruby’s Prepend
The final product in action
GitHub
config/invoked_method_reporter.yml
methods_to_report: [
'TargetModule#unused_method_from_module',
'TargetModule.method_of_module',
'User.unused_method_from_class',
'User#unused_method_from_class'
]
target_module.rb
module TargetModule
def self.method_of_module
end
def unused_method_from_module
end
end
Initialiser
InvokedMethodReporter.setup
user.rb
class User
include TargetModule
extend TargetModule
def self.unused_method_from_class
end
def unused_method_from_class
end
end
As a result we will receive a notification in any of the following cases.
TargetModule.method_of_moduleUser.unused_method_from_classUser.new.unused_method_from_classUser.unused_method_from_moduleUser.new.unused_method_from_module
The story
One day at work I found a funny looking method. I did a quick research and I saw no visible reference neither to it nor its name as a text in the codebase.
Most probably this method was unused. But given that the project is an old legacy application where everything is possible and which deals with money, I was somewhat reluctant to just remove it.
If only there were an option to get a slack notification when the method was called to double-check things before removing it.
Well, why not to build one as an exercise?
The theory
Ruby has a handy little tool we can leverage: prepend. This article describes it quite thoroughly. But in short: we can override any existing method without inheritance or touching them at all.
We only need:
- a new module
- override the target method in the new module (and call
super) - prepend the new module into the module or class that contains the target method
The first playful approach
describe UserInvokedMethodReporter do
it 'invokes the reporter then the original impl' do
expect(STDOUT).to receive(:puts).with('User#unused_method_from_class was invoked').ordered
expect(STDOUT).to receive(:puts).with('Original impl with: Param1').ordered
User.new.unused_method_from_class('Param1')
end
end
class User
def unused_method_from_class(param1)
puts "Original impl with: #{param1}"
end
end
module UserInvokedMethodReporter
def unused_method_from_class(*)
puts 'User#unused_method_from_class was invoked'
super
end
end
User.prepend(UserInvokedMethodReporter)
To look like professionals, we start with a test to describe the desired final behaviour. Then we have the User class with a suspected unused method. And finally we perform the override using a new module and the magic prepend method.
This way we can seamlessly inject our rebel reporter functionality before the original implementation. We don’t need to touch the original code. Which can even change we will break nothing, this is what I call loosely coupling.
One inconvenience with this solution is we need to create a module for every class or module we wish to monitor and do the prepend manually.
Second approach: automate
GitHub
Wouldn’t it be cool if we could just call a method and it would bind a reporter to the target method? This would allow us to read a list of the target methods from a config file and automate the whole process.
Metaprogramming is here to the rescue to write code that will write code for us.
Let’s start with the API design
To bind a reporter:
InvokedMethodReporter.bind_to('User#unused_method_from_class')
To send a notification or log what happened:
InvokedMethodReporter.report('User#unused_method_from_class')
Remember we are decent engineers? Meaning thorough tests! (the sooner the better)
describe InvokedMethodReporter do
describe '.bind_to' do
it 'invokes the reporter then the original impl' do
described_class.bind_to('User#unused_method_from_class')
expect(STDOUT).to receive(:puts).with('User#unused_method_from_class was invoked').ordered
expect(STDOUT).to receive(:puts).with('Original impl with: Param1').ordered
User.new.unused_method_from_class('Param1')
end
end
end
Basically in this test we expect that the reporter will be called and then the original implementation, in this exact order.
I have to admit this test doesn’t meet 100% the requirements of the unit testing theory. After the next section you will see it is more like an integration test. I like integration tests and I’m not really a mockist.
The design
The interface InvokedMethodReporter.bind_to looks pretty neat, but observe that we don’t call the bind_to method on an instance of the InvokedMethodReporter class. Which means bind_to is only a helper method. The question arises: should we pack all the functionality there or somewhere else? Well, the answer is as usual: it depends. I’d say if we have only a few lines of code, then don’t bother and put them into the helper method. Maybe it is not visible from here, but we will have so much functionality that it seems an object is about to be born.
Following the single responsible principle, let’s put the binding functionality into a Binder class and let its object do the job. On the other hand InvokedMethodReporter can stay as a Module which contains two helper methods. If needed we can convert it into a class later.
The implementation
# invoked_method_reporter.rb
module InvokedMethodReporter
def self.bind_to(method_definition)
Binder.bind_to(method_definition)
end
def self.report(method_definition)
puts "#{method_definition} was invoked"
end
end
The module is quite self-explanatory. It proxies the binding functionality and defines a basic reporter, which we will pimp up later.
# invoked_method_reporter/binder.rb
module InvokedMethodReporter
class Binder
attr_reader :namespace, :method_name, :method_definition
def self.bind_to(method_definition)
new(method_definition).bind
end
def initialize(method_definition)
@method_definition = method_definition
@namespace, @method_name = method_definition.split('#')
end
def bind
target_const.prepend(module_to_prepend)
end
private
def target_const
Object.const_get(namespace)
end
def module_to_prepend
local_method_name = method_name
local_method_definition = method_definition
Module.new do
define_method(local_method_name) do |*args|
InvokedMethodReporter.report(local_method_definition)
super(*args)
end
end
end
end
end
At the beginning we grab the string User#unused_method_from_class in the constructor. This contains the namespace and the method separated by a # sign. This is a convention to imply the method is defined on the object level (even if everything is an object in Ruby). First we separate the string into well-defined variables.
Then we continue with the binding. For this we need two things:
- get the class constant
- send the prepend message to it with a module that contains the new method definition of the target method
The first one is relatively easy, just use Object.const_get, and once more Ruby’s magic explodes into action. namespace.constantize is also available in the Rails world.
The second part is a bit trickier because we need that special module. Thanks to Ruby’s flexibility we can create one using the Module class. Then we define the target method on it with the define_method method and we are good to go.
But wait, something strange is going on, why are there two local variables copying values from the objet level fields? Unfortunately the object level fields are not accessible inside the block given to Module.new.
So far so good, but one might ask: What about the class level methods and all the other things with the target_module?
Breath easy young padawan, we will get there soon enough.
Next step: attach reporter to class level methods
In this case we have a minor problem with the target_const method.
User.prepend is good enough to override object level methods, but insufficient for the class level. We need to sort of level up. We can use the singleton_class method to reach the higher level.
def target_const
Object.const_get(namespace).singleton_class
end
Another task is to detect if we operate on the object or the class level. The difference comes from the character that separates the namespace from the method name, it can be a . or a #.
With this knowledge secured, let’s review the current design.
We could extend the Binder class with more code and with more if statements, but I really like replacing conditionals with polymorphism. So let’s separate matters into their own place.
A little tip to leverage identifying sameness or difference. As Sandi Metz writes in her 99bottles book:
However, despite the fact that sameness is easier to identify, difference is more useful because it has more meaning.
We can identify two differences:
- the
target_constimplementation - using different character to separate the namespace from the method name
Everything else seems to be the same, so they can stay in the Binder class and we can move the differences into separate child classes.
Sounds like a plan, let’s materialise it in the code.
Tests Tests Tests
describe InvokedMethodReporter do
describe '.bind_to' do
context 'class level' do
it 'invokes the reporter then the original impl' do
described_class.bind_to('User.unused_method_from_class')
expect(STDOUT).to receive(:puts).with('User.unused_method_from_class was invoked').ordered
expect(STDOUT).to receive(:puts).with('Original impl with: Param1').ordered
User.unused_method_from_class('Param1')
end
end
context 'object level' do
it 'invokes the reporter then the original impl' do
described_class.bind_to('User#unused_method_from_class')
expect(STDOUT).to receive(:puts).with('User#unused_method_from_class was invoked').ordered
expect(STDOUT).to receive(:puts).with('Original impl with: Param1').ordered
User.new.unused_method_from_class('Param1')
end
end
end
end
This test only extends the previous one with the class level binding.
The implementation
# invoked_method_reporter/binder.rb
module InvokedMethodReporter
class Binder
attr_reader :namespace, :method_name, :method_definition
def self.bind_to(method_definition)
if method_definition.include?('.')
ClassLevelBinder.new(method_definition).bind
else
ObjectLevelBinder.new(method_definition).bind
end
end
def initialize(method_definition)
@method_definition = method_definition
@namespace, @method_name = method_definition.split(self.class::SEPARATOR)
end
def bind
target_const.prepend(module_to_prepend)
end
private
def module_to_prepend
local_method_name = method_name
local_method_definition = method_definition
Module.new do
define_method(local_method_name) do |*args|
InvokedMethodReporter.report(local_method_definition)
super(*args)
end
end
end
end
end
# invoked_method_reporter/class_level_binder.rb
module InvokedMethodReporter
class ClassLevelBinder < Binder
SEPARATOR = '.'
private
def target_const
Object.const_get(namespace).singleton_class
end
end
end
# invoked_method_reporter/object_level_binder.rb
module InvokedMethodReporter
class ObjectLevelBinder < Binder
SEPARATOR = '#'
private
def target_const
Object.const_get(namespace)
end
end
end
I was hesitating where to put this part:
if method_definition.include?('.')
ClassLevelBinder.new(method_definition).bind
else
ObjectLevelBinder.new(method_definition).bind
end
The other option would be InvokedMethodReporter.bind_to. But I feel this responsibility belongs more to the Binder class. But then the child classes inherit this method. Le me know your opinion in the comment section.
Config and helper methods
# invoked_method_reporter/config.rb
require 'yaml'
module InvokedMethodReporter
class Config
def initialize
@config = YAML.load_file('config/invoked_method_reporter.yml')
end
def methods_to_report
@config['methods_to_report']
end
end
end
# invoked_method_reporter.rb
module InvokedMethodReporter
def self.config
@config ||= Config.new
end
def self.setup
bind_to(config.methods_to_report)
end
def self.bind_to(method_definitions)
Array(method_definitions).each do |method_definition|
Binder.bind_to(method_definition)
end
end
end
A neat trick in the bind_to method: Array(method_definitions) this makes possible to call the method with a single item or with an array, the outcome will be the same.
Advanced testing
GitHub
So far we have used an existing class for testing. This has a few drawbacks in a bigger system:
- the target method can be removed from the class
- the whole class can be removed
- it is not threatening us right now, but this type of low level code can cause unwanted side effects in the tests running after
So our test is tightly coupled to an existing class and can develop hostile activities later on.
Why not to create our own “mock” class? Things to look after:
- choose a class name that most probably won’t appear in real code
- remove the class after the test to avoid side effects
describe InvokedMethodReporter do
class TargetClassForInvokedMethodReporter
def original_impl; end
def self.original_impl; end
def unused_method_from_class
original_impl
end
def self.unused_method_from_class
original_impl
end
end
described_class.bind_to('TargetClassForInvokedMethodReporter#unused_method_from_class')
described_class.bind_to('TargetClassForInvokedMethodReporter.unused_method_from_class')
after(:all) do
Object.send(:remove_const, :TargetClassForInvokedMethodReporter)
end
describe '.bind_to' do
context 'object level' do
it 'invokes the reporter then the original implementation' do
target_class_object = TargetClassForInvokedMethodReporter.new
expect(described_class).to receive(:report).ordered
expect(target_class_object).to receive(:original_impl).ordered
target_class_object.unused_method_from_class
end
end
context 'class level' do
it 'invokes the reporter then the original implementation' do
expect(described_class).to receive(:report).ordered
expect(TargetClassForInvokedMethodReporter).to receive(:original_impl).ordered
TargetClassForInvokedMethodReporter.unused_method_from_class
end
end
end
end
Methods defined in modules
At this point we are good with methods defined in classes. We have three more cases to discuss:
- method defined in module A
- the module A included into a class
- class extended by the module A
- module A defines a “class” level method
Better illustration in code:
module TargetModule
def unused_method_from_module
end
def self.unused_method_in_module
end
end
class TargetClass
include TargetModule
extend TargetModule
end
This gives us three method call options:
TargetClass.new.unused_method_from_module
TargetClass.unused_method_from_module
TargetModule.unused_method_in_module
The good news is our current implementation covers those cases. And naturally there is a but: the reporter for modules has to be called before include TargetModule or extend TargetModule happens.
The reason? Run the following code snippets and examine the console output.
module InvokedMethodReporter
end
module TargetModule
def unused_method_from_module
end
def self.unused_method_in_module
end
end
class TargetClass
include TargetModule
extend TargetModule
end
TargetModule.prepend(InvokedMethodReporter)
p TargetClass.ancestors
module InvokedMethodReporter
end
module TargetModule
def unused_method_from_module
end
def self.unused_method_in_module
end
end
TargetModule.prepend(InvokedMethodReporter)
class TargetClass
include TargetModule
extend TargetModule
end
p TargetClass.ancestors
The console output of the first case
[TargetClass, TargetModule, Object, Kernel, BasicObject]
The console output of the second case
[TargetClass, InvokedMethodReporter, TargetModule, Object, Kernel, BasicObject]
The significant difference is the InvokedMethodReporter is injected into the inheritance chain of TargetClass so it can override methods defined by TargetModule.
Nothing is left except to extend our test suite for modules.
The reporter
After all the hard work, finally we can have the well-deserved fun with reporting. At work we use Rollbar, hooked into a dedicated Slack channel so we can have live notifications.
require 'active_support/backtrace_cleaner'
module InvokedMethodReporter
def self.report(method_definition)
message = "[InvokedMethodReporter] #{method_definition} was invoked"
report_params = {
sender: sender_trace
}
Rollbar.info(message, report_params)
rescue StandardError => e
Rollbar.error('Error in InvokedMethodReporter.report', e)
# we should raise an exception here in case of test env
end
private_class_method
def self.sender_trace
backtrace_cleaner = ActiveSupport::BacktraceCleaner.new
filters = [
'invoked_method_reporter/binder.rb',
'invoked_method_reporter.rb',
'gems'
]
backtrace_cleaner.add_silencer do |line|
filters.any? { |filter| line.include?(filter) }
end
backtrace_cleaner.clean(caller).first
end
end
It is interesting to know who sends the message to our “unused_method”. The backtrace is a good start, but it can contain a lot of non-project-related files, like files from gems etc. The ActiveSupport::BacktraceCleaner is a very useful class to remove unrelated paths from the backtrace.
Rails integration
To make sure we bind the reporter to modules before they get included, we wanna make sure that our initialiser runs first. Not the nicest solution but it will do the work:
# config/initializers/01_invoked_method_reporter.rb
InvokedMethodReporter.setup
The rest of the files can go into the lib folder just like in the final version.
Final thoughts
We have this solution in production for weeks by now. We set up the time limit for one month, meaning if a method doesn’t get invoked for a month that’s good enough to remove it. Soon we can get rid of some dead and unclean part of our codebase. The feeling will be close to indescribable.
Please let me know your opinion.
Comments
Post a Comment