diff options
Diffstat (limited to 'spec/lib/gitlab/instrumentation/redis_cluster_validator_spec.rb')
| -rw-r--r-- | spec/lib/gitlab/instrumentation/redis_cluster_validator_spec.rb | 135 |
1 files changed, 65 insertions, 70 deletions
diff --git a/spec/lib/gitlab/instrumentation/redis_cluster_validator_spec.rb b/spec/lib/gitlab/instrumentation/redis_cluster_validator_spec.rb index 58c75bff9dd..892b8e69124 100644 --- a/spec/lib/gitlab/instrumentation/redis_cluster_validator_spec.rb +++ b/spec/lib/gitlab/instrumentation/redis_cluster_validator_spec.rb @@ -7,107 +7,102 @@ require 'rspec-parameterized' RSpec.describe Gitlab::Instrumentation::RedisClusterValidator do include RailsHelpers - describe '.validate!' do + describe '.validate' do using RSpec::Parameterized::TableSyntax - context 'Rails environments' do - where(:env, :should_raise) do - 'production' | false - 'staging' | false - 'development' | true - 'test' | true - end - - with_them do - it do - stub_rails_env(env) - - args = [[:mget, 'foo', 'bar']] - - if should_raise - expect { described_class.validate!(args) } - .to raise_error(described_class::CrossSlotError) - else - expect { described_class.validate!(args) }.not_to raise_error - end - end - end - end - - where(:command, :arguments, :should_raise) do - :rename | %w(foo bar) | true - :RENAME | %w(foo bar) | true - 'rename' | %w(foo bar) | true - 'RENAME' | %w(foo bar) | true - :rename | %w(iaa ahy) | false # 'iaa' and 'ahy' hash to the same slot - :rename | %w({foo}:1 {foo}:2) | false - :rename | %w(foo foo bar) | false # This is not a valid command but should not raise here - :mget | %w(foo bar) | true - :mget | %w(foo foo bar) | true - :mget | %w(foo foo) | false - :blpop | %w(foo bar 1) | true - :blpop | %w(foo foo 1) | false - :mset | %w(foo a bar a) | true - :mset | %w(foo a foo a) | false - :del | %w(foo bar) | true - :del | [%w(foo bar)] | true # Arguments can be a nested array - :del | %w(foo foo) | false - :hset | %w(foo bar) | false # Not a multi-key command - :mget | [] | false # This is invalid, but not because it's a cross-slot command + where(:command, :arguments, :keys, :is_valid) do + :rename | %w(foo bar) | 2 | false + :RENAME | %w(foo bar) | 2 | false + 'rename' | %w(foo bar) | 2 | false + 'RENAME' | %w(foo bar) | 2 | false + :rename | %w(iaa ahy) | 2 | true # 'iaa' and 'ahy' hash to the same slot + :rename | %w({foo}:1 {foo}:2) | 2 | true + :rename | %w(foo foo bar) | 2 | true # This is not a valid command but should not raise here + :mget | %w(foo bar) | 2 | false + :mget | %w(foo foo bar) | 3 | false + :mget | %w(foo foo) | 2 | true + :blpop | %w(foo bar 1) | 2 | false + :blpop | %w(foo foo 1) | 2 | true + :mset | %w(foo a bar a) | 2 | false + :mset | %w(foo a foo a) | 2 | true + :del | %w(foo bar) | 2 | false + :del | [%w(foo bar)] | 2 | false # Arguments can be a nested array + :del | %w(foo foo) | 2 | true + :hset | %w(foo bar) | 1 | nil # Single key write + :get | %w(foo) | 1 | nil # Single key read + :mget | [] | 0 | true # This is invalid, but not because it's a cross-slot command end with_them do it do args = [[command] + arguments] - - if should_raise - expect { described_class.validate!(args) } - .to raise_error(described_class::CrossSlotError) + if is_valid.nil? + expect(described_class.validate(args)).to eq(nil) else - expect { described_class.validate!(args) }.not_to raise_error + expect(described_class.validate(args)[:valid]).to eq(is_valid) + expect(described_class.validate(args)[:allowed]).to eq(false) + expect(described_class.validate(args)[:command_name]).to eq(command.to_s.upcase) + expect(described_class.validate(args)[:key_count]).to eq(keys) end end end - where(:arguments, :should_raise) do - [[:get, "foo"], [:get, "bar"]] | true - [[:get, "foo"], [:mget, "foo", "bar"]] | true # mix of single-key and multi-key cmds - [[:get, "{foo}:name"], [:get, "{foo}:profile"]] | false - [[:del, "foo"], [:del, "bar"]] | true - [] | false # pipeline or transaction opened and closed without ops + where(:arguments, :should_raise, :output) do + [ + [ + [[:get, "foo"], [:get, "bar"]], + true, + { valid: false, key_count: 2, command_name: 'PIPELINE/MULTI', allowed: false } + ], + [ + [[:get, "foo"], [:mget, "foo", "bar"]], + true, + { valid: false, key_count: 3, command_name: 'PIPELINE/MULTI', allowed: false } + ], + [ + [[:get, "{foo}:name"], [:get, "{foo}:profile"]], + false, + { valid: true, key_count: 2, command_name: 'PIPELINE/MULTI', allowed: false } + ], + [ + [[:del, "foo"], [:del, "bar"]], + true, + { valid: false, key_count: 2, command_name: 'PIPELINE/MULTI', allowed: false } + ], + [ + [], + false, + nil # pipeline or transaction opened and closed without ops + ] + ] end with_them do it do - if should_raise - expect { described_class.validate!(arguments) } - .to raise_error(described_class::CrossSlotError) - else - expect { described_class.validate!(arguments) }.not_to raise_error - end + expect(described_class.validate(arguments)).to eq(output) end end end describe '.allow_cross_slot_commands' do - it 'does not raise for invalid arguments' do - expect do + it 'skips validation for allowed commands' do + expect( described_class.allow_cross_slot_commands do - described_class.validate!([[:mget, 'foo', 'bar']]) + described_class.validate([[:mget, 'foo', 'bar']]) end - end.not_to raise_error + ).to eq({ valid: true, key_count: 2, command_name: 'MGET', allowed: true }) end it 'allows nested invocation' do - expect do + expect( described_class.allow_cross_slot_commands do described_class.allow_cross_slot_commands do - described_class.validate!([[:mget, 'foo', 'bar']]) + described_class.validate([[:mget, 'foo', 'bar']]) end - described_class.validate!([[:mget, 'foo', 'bar']]) + described_class.validate([[:mget, 'foo', 'bar']]) end - end.not_to raise_error + ).to eq({ valid: true, key_count: 2, command_name: 'MGET', allowed: true }) end end end |