From c5206b407f225e14295c6a5e6dbba1634ea2d5c9 Mon Sep 17 00:00:00 2001 From: Holger Just Date: Sun, 7 Jun 2020 16:44:53 +0200 Subject: [PATCH] Allow filter conditionals to be defined as symbols too --- lib/rackstash/filter.rb | 73 +++++++++----- spec/rackstash/filter_spec.rb | 175 ++++++++++++++++++++++------------ 2 files changed, 163 insertions(+), 85 deletions(-) diff --git a/lib/rackstash/filter.rb b/lib/rackstash/filter.rb index 7fc587e..fc827b7 100644 --- a/lib/rackstash/filter.rb +++ b/lib/rackstash/filter.rb @@ -67,17 +67,21 @@ module Rackstash # first use the filter {.registry} to find the matching class. With that # we then create a filter object as before. When giving an object which # responds to `call` already (e.g. a `Proc`) we return it unchanged, - # ignoring any additional passed `args`. - # @param only_if [#call, nil] An optional condition defining whether the - # filter should be applied, usually given as a `Proc` object. Before - # evaluating the newly created filter object, we first call the given - # proc with the event as its argument. The filter is applied only if the - # proc returns a truethy value. - # @param not_if [#call, nil] An optional condition defining whether the - # filter should not be applied, usually given as a `Proc` object. Before - # evaluating the newly created filter object, we first call the given - # proc with the event as its argument. The filter is not applied if the - # proc returns a truethy value. + # ignoring any additional passed `args`, `kwargs`, or condition. + # @param only_if [#call, Symbol, nil] An optional condition defining + # whether the filter should be applied, defined as a `Proc` (or another + # object responding to `call`) or a Symbol. Before evaluating the newly + # created filter object, we first call the given proc or we call the + # method identified by the given Symbol on the filter object, each time + # giving the `event` Hash as its argument. The filter is only applied + # if the proc or filter method returns a truethy value. + # @param not_if [#call, Symbol, nil] An optional condition defining + # whether the filter should not be applied, defined as a `Proc` (or + # another object responding to `call`) or a Symbol. Before evaluating + # the newly created filter object, we first call the given proc or we + # call the method identified by the given Symbol on the filter object, + # each time giving the `event` Hash as its argument. The filter is not + # applied if the proc or filter method returns a truethy value. # @param args [Array] an optional list of arguments which is passed to the # initializer for the new filter object. # @param kwargs [Hash] an optional list of keyword arguments which are @@ -88,30 +92,49 @@ module Rackstash # for the specified class name # @return [Object] a new filter object def build(filter_spec, *args, only_if: nil, not_if: nil, **kwargs, &block) - case filter_spec - when ->(filter) { filter.respond_to?(:call) } - filter_spec - else - args << kwargs unless kwargs.empty? + # TODO: warn if args, kwargs, only_if, not_if were given here + # since they are ignored + return filter_spec if filter_spec.respond_to?(:call) - filter = registry.fetch(filter_spec).new(*args, &block) - conditional_filter(filter, only_if: only_if, not_if: not_if) - end + filter_class = registry.fetch(filter_spec) + filter = filter_class.new(*args, **kwargs, &block) + + conditional_filter(filter, only_if: only_if, not_if: not_if) end private def conditional_filter(filter, only_if: nil, not_if: nil) - return filter if only_if.nil? && not_if.nil? + if only_if.nil? + # Empty conditional, do nothing + elsif only_if.is_a?(Symbol) + apply_condition(filter) { |event| filter.send(only_if, event) } + elsif only_if.respond_to?(:call) + apply_condition(filter) { |event| only_if.call(event) } + else + raise TypeError, 'Invalid only_if filter' + end - conditional = Module.new do + if not_if.nil? + # Empty conditional, do nothing + elsif not_if.is_a?(Symbol) + apply_condition(filter) { |event| !filter.send(not_if, event) } + elsif not_if.respond_to?(:call) + apply_condition(filter) { |event| !not_if.call(event) } + else + raise TypeError, 'Invalid not_if filter' + end + + filter + end + + def apply_condition(filter) + mod = Module.new do define_method(:call) do |event| - return event if only_if && !only_if.call(event) - return event if not_if && not_if.call(event) - super(event) + yield(event) ? super(event) : event end end - filter.extend(conditional) + filter.extend(mod) end end end diff --git a/spec/rackstash/filter_spec.rb b/spec/rackstash/filter_spec.rb index 77f7111..2effab6 100644 --- a/spec/rackstash/filter_spec.rb +++ b/spec/rackstash/filter_spec.rb @@ -11,42 +11,83 @@ require 'securerandom' require 'rackstash/filter' RSpec.describe Rackstash::Filter do - let(:registry) { Rackstash::ClassRegistry.new('filter') } - let(:filter_class) { Class.new do + attr_reader :args, :kwargs + + def initialize(*args, **kwargs) + @args = args + @kwargs = kwargs + + @called = false + @called_with_if = false + @called_with_unless = false + end + + def if(event) + @called_with_if = true + @kwargs[:if] + end + + def unless(event) + @called_with_unless = true + @kwargs[:unless] + end + def call(_event) + @called = true 'filtered' end + + %i[called called_with_if called_with_unless].each do |m| + define_method(:"#{m}?") { instance_variable_get("@#{m}") } + end end } let(:filter_name) { :"filter_class_#{SecureRandom.hex(6)}" } - describe '.build' do - before do - allow(described_class).to receive(:registry).and_return(registry) - described_class.register(filter_class, filter_name) + around(:each) do |example| + original_filters = described_class.registry.to_h + described_class.registry.clear + example.run + original_filters.each do |name, registered_clas| + described_class.registry[name] = registered_clas end + end + before(:each) do + described_class.register(filter_class, filter_name) + end + + describe '.build' do it 'builds a filter from a class' do - args = ['arg1', foo: 'bar'] - expect(filter_class).to receive(:new).with(*args) + args = ['arg1'] + kwargs = { foo: 'bar' } - described_class.build(filter_class, *args) + filter = described_class.build(filter_class, *args, **kwargs) + expect(filter).to be_a filter_class + expect(filter.args).to eq args + expect(filter.kwargs).to eq kwargs end it 'builds a filter from a Symbol' do - args = ['arg1', foo: 'bar'] - expect(filter_class).to receive(:new).with(*args) + args = ['arg1'] + kwargs = { foo: 'bar' } - described_class.build(filter_name.to_sym, *args) + filter = described_class.build(filter_name.to_sym, *args, **kwargs) + expect(filter).to be_a filter_class + expect(filter.args).to eq args + expect(filter.kwargs).to eq kwargs end it 'builds a filter from a String' do - args = ['arg1', foo: 'bar'] - expect(filter_class).to receive(:new).with(*args) + args = ['arg1'] + kwargs = { foo: 'bar' } - described_class.build(filter_name.to_s, *args) + filter = described_class.build(filter_name.to_s, *args, **kwargs) + expect(filter).to be_a filter_class + expect(filter.args).to eq args + expect(filter.kwargs).to eq kwargs end it 'returns an existing filter' do @@ -60,30 +101,60 @@ RSpec.describe Rackstash::Filter do let(:event) { Object.new } it 'applies the only_if conditional for new filters' do - only_if = -> {} + only_if = ->(_event) { false } filter = described_class.build(filter_name, only_if: only_if) + expect(filter.call(event)).to equal event + expect(filter).not_to be_called - expect(only_if).to receive(:call).and_return false - expect { filter.call({}) }.not_to raise_error + only_if = ->(_event) { true } + filter = described_class.build(filter_name, only_if: only_if) + expect(filter.call(event)).to eql 'filtered' + expect(filter).to be_called + end + + it 'applies the only_if filter conditional for new filters' do + filter = described_class.build(filter_name, only_if: :if, if: false) + filter.call(event) + expect(filter).to be_called_with_if + expect(filter).not_to be_called + + filter = described_class.build(filter_name, only_if: :if, if: true) + expect(filter.call(event)).to eql 'filtered' + expect(filter).to be_called + expect(filter).to be_called_with_if end it 'applies the not_if conditional for new filters' do - not_if = -> {} + not_if = ->(_event) { true } filter = described_class.build(filter_name, not_if: not_if) - - expect(not_if).to receive(:call).and_return true expect(filter.call(event)).to equal event + expect(filter).not_to be_called + + not_if = ->(_event) { false } + filter = described_class.build(filter_name, not_if: not_if) + expect(filter.call(event)).to eql 'filtered' + expect(filter).to be_called + end + + it 'applies the not_if filter conditional for new filters' do + filter = described_class.build(filter_name, not_if: :unless, unless: true) + expect(filter.call(event)).to equal event + expect(filter).not_to be_called + expect(filter).to be_called_with_unless + + filter = described_class.build(filter_name, not_if: :unless, unless: false) + expect(filter.call(event)).to eql 'filtered' + expect(filter).to be_called + expect(filter).to be_called_with_unless end it 'applies both conditionals for new filters' do - only_if = -> {} - not_if = -> {} - + only_if = ->(_event) { true } + not_if = ->(_event) { false } filter = described_class.build(filter_name, only_if: only_if, not_if: not_if) - expect(only_if).to receive(:call).and_return true - expect(not_if).to receive(:call).and_return false expect(filter.call(event)).to eql 'filtered' + expect(filter).to be_called end it 'keeps the class hierarchy unchanged' do @@ -94,7 +165,7 @@ RSpec.describe Rackstash::Filter do it 'ignores the conditional for existing filters' do filter = filter_class.new - only_if = -> {} + only_if = ->(_event) {} expect(described_class.build(filter, only_if: only_if)) .to equal filter @@ -105,31 +176,27 @@ RSpec.describe Rackstash::Filter do end it 'passes keyword arguments to the initializer' do - filter_class.class_eval do - def initialize(mandatory:) - @mandatory = mandatory - end + filter = described_class.build(filter_name, 'foo', only_if: -> {}, argument: 'bar') + expect(filter.args).to eq ['foo'] + expect(filter.kwargs).to eq argument: 'bar' + end - attr_reader :mandatory - end + it 'expects callable objects' do + object = false - filter = described_class.build(filter_name, only_if: -> {}, mandatory: 'foo') - expect(filter.mandatory).to eql 'foo' + expect { described_class.build(filter_name, only_if: object) } + .to raise_error(TypeError, 'Invalid only_if filter') + expect { described_class.build(filter_name, not_if: object) } + .to raise_error(TypeError, 'Invalid not_if filter') end end context 'without conditionals' do - it 'passes keyword arguments to the initializer' do - filter_class.class_eval do - def initialize(mandatory:) - @mandatory = mandatory - end + it 'passes arguments to the initializer' do + filter = described_class.build(filter_name, 'foo', argument: 'bar') - attr_reader :mandatory - end - - filter = described_class.build(filter_name, mandatory: 'foo') - expect(filter.mandatory).to eql 'foo' + expect(filter.args).to eq ['foo'] + expect(filter.kwargs).to eq argument: 'bar' end end @@ -158,29 +225,17 @@ RSpec.describe Rackstash::Filter do end describe '.register' do - let(:filter_class) { - Class.new do - def call - end - end - } - it 'registers a filter class' do - expect(described_class.registry).to receive(:[]=).with(:foo, filter_class).ordered - expect(described_class.registry).to receive(:[]=).with(:bar, filter_class).ordered - described_class.register(filter_class, :foo, :bar) + expect(described_class.registry.fetch(:foo)).to equal filter_class + expect(described_class.registry.fetch(:bar)).to equal filter_class end it 'rejects invalid classes' do - expect(described_class.registry).not_to receive(:[]=) - expect { described_class.register(:not_a_class, :foo) }.to raise_error TypeError expect { described_class.register(Class.new, :foo) }.to raise_error TypeError - end - it 'rejects invalid names' do - expect { described_class.register(filter_class, 123) }.to raise_error TypeError + expect(described_class.registry[:foo]).to be_nil end end end