From fe097a653ecf7f14955ad5773c30d6cc0d8dca9f Mon Sep 17 00:00:00 2001 From: Yaroslav <39729785+viralpraxis@users.noreply.github.com> Date: Wed, 13 Dec 2023 08:26:48 +0300 Subject: [PATCH] Add support for `expire`-related command options `nx`, `xx`, `lt` and `gt` (#290) Resolves https://github.com/sds/mock_redis/issues/286 I also addes some specs to run against redis 7. Some of the existing specs fail, so I marked new specs with `redis: 7.0` tag. Running `bundle exec rspec --tag redis:7.0` should run only redis-7-related specs, and running with `redis:6.2` or without tag at all should run all the specs except of redis-7-related. Not sure if this is an appropriate/optimal solution --- .github/workflows/tests.yml | 3 +- lib/mock_redis/database.rb | 43 ++++++++--- spec/commands/expire_spec.rb | 77 +++++++++++++++++++ spec/commands/expireat_spec.rb | 15 ++++ spec/commands/pexpire_spec.rb | 15 ++++ spec/commands/pexpireat_spec.rb | 25 +++++- spec/spec_helper.rb | 5 ++ ...aises_on_invalid_expire_command_options.rb | 20 +++++ 8 files changed, 188 insertions(+), 15 deletions(-) create mode 100644 spec/support/shared_examples/raises_on_invalid_expire_command_options.rb diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 60dc279..22a6307 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -19,6 +19,7 @@ jobs: - '3.2' redis-version: - '6.2' + - '7.0' services: redis: @@ -41,7 +42,7 @@ jobs: bundler-cache: true - name: Run tests - run: bundle exec rspec + run: bundle exec rspec --tag redis:${{ matrix.redis-version }} - name: Code coverage reporting uses: coverallsapp/github-action@master diff --git a/lib/mock_redis/database.rb b/lib/mock_redis/database.rb index 5adcb8d..25da212 100644 --- a/lib/mock_redis/database.rb +++ b/lib/mock_redis/database.rb @@ -43,6 +43,9 @@ def initialize_copy(_source) # Redis commands go below this line and above 'private' + # FIXME: Current implementation of `call` does not work propetly with kwarg-options. + # i.e. `call("EXPIRE", "foo", 40, "NX")` (which redis-rb will simply transmit to redis-server) + # will be passed to `#expire` without keywords transformation. def call(command, &_block) public_send(command[0].downcase, *command[1..]) end @@ -89,32 +92,42 @@ def echo(msg) msg.to_s end - def expire(key, seconds) + def expire(key, seconds, nx: nil, xx: nil, lt: nil, gt: nil) # rubocop:disable Metrics/ParameterLists assert_valid_integer(seconds) - pexpire(key, seconds.to_i * 1000) + pexpire(key, seconds.to_i * 1000, nx: nx, xx: xx, lt: lt, gt: gt) end - def pexpire(key, ms) + def pexpire(key, ms, nx: nil, xx: nil, lt: nil, gt: nil) # rubocop:disable Metrics/ParameterLists assert_valid_integer(ms) now, miliseconds = @base.now now_ms = (now * 1000) + miliseconds - pexpireat(key, now_ms + ms.to_i) + pexpireat(key, now_ms + ms.to_i, nx: nx, xx: xx, lt: lt, gt: gt) end - def expireat(key, timestamp) + def expireat(key, timestamp, nx: nil, xx: nil, lt: nil, gt: nil) # rubocop:disable Metrics/ParameterLists assert_valid_integer(timestamp) - pexpireat(key, timestamp.to_i * 1000) + pexpireat(key, timestamp.to_i * 1000, nx: nx, xx: xx, lt: lt, gt: gt) end - def pexpireat(key, timestamp_ms) + def pexpireat(key, timestamp_ms, nx: nil, xx: nil, lt: nil, gt: nil) # rubocop:disable Metrics/ParameterLists assert_valid_integer(timestamp_ms) - if exists?(key) - timestamp = Rational(timestamp_ms.to_i, 1000) - set_expiration(key, @base.time_at(timestamp)) + if nx && gt || gt && lt || lt && nx || nx && xx + raise Redis::CommandError, <<~TXT.chomp + ERR NX and XX, GT or LT options at the same time are not compatible + TXT + end + + return false unless exists?(key) + + expiry = expiration(key) + new_expiry = @base.time_at(Rational(timestamp_ms.to_i, 1000)) + + if should_update_expiration?(expiry, new_expiry, nx: nx, xx: xx, lt: lt, gt: gt) + set_expiration(key, new_expiry) true else false @@ -324,7 +337,7 @@ def extract_timeout(arglist) end def expiration(key) - expire_times.find { |(_, k)| k == key.to_s }.first + expire_times.find { |(_, k)| k == key.to_s }&.first end def has_expiration?(key) @@ -339,6 +352,14 @@ def looks_like_float?(str) !!Float(str) rescue false end + def should_update_expiration?(expiry, new_expiry, nx:, xx:, lt:, gt:) # rubocop:disable Metrics/ParameterLists + return false if nx && expiry || xx && !expiry + return false if lt && expiry && new_expiry > expiry + return false if gt && (!expiry || new_expiry < expiry) + + true + end + def redis_pattern_to_ruby_regex(pattern) Regexp.new( "^#{pattern}$". diff --git a/spec/commands/expire_spec.rb b/spec/commands/expire_spec.rb index a7fc3b5..312f8e5 100644 --- a/spec/commands/expire_spec.rb +++ b/spec/commands/expire_spec.rb @@ -29,6 +29,19 @@ expect(@redises.expire(@key.to_sym, 9)).to eq(true) end + it 'works with options', redis: '7.0' do + expect(@redises.expire(@key, 20)).to eq(true) + expect(@redises.expire(@key, 10, lt: true)).to eq(true) + expect(@redises.expire(@key, 15, lt: true)).to eq(false) + expect(@redises.expire(@key, 20, gt: true)).to eq(true) + expect(@redises.expire(@key, 15, gt: true)).to eq(false) + expect(@redises.expire(@key, 10, xx: true)).to eq(true) + expect(@redises.expire(@key, 10, nx: true)).to eq(false) + expect(@redises.persist(@key)).to eq(true) + expect(@redises.expire(@key, 10, xx: true)).to eq(false) + expect(@redises.expire(@key, 10, nx: true)).to eq(true) + end + context '[mock only]' do # These are mock-only since we can't actually manipulate time in # the real Redis. @@ -42,6 +55,8 @@ allow(Time).to receive(:now).and_return(@now) end + it_should_behave_like 'raises on invalid expire command options', :expire + it 'removes keys after enough time has passed' do @mock.expire(@key, 5) allow(Time).to receive(:now).and_return(@now + 5) @@ -107,5 +122,67 @@ expect(@mock.get(other_key)).to be_nil end end + + context 'with `nx` option' do + it do + @mock.set(@key, 'string') + + expect(@mock.expire(@key, 10)).to eq(true) + expect(@mock.expire(@key, 10, nx: true)).to eq(false) + + @mock.persist(@key) + + expect(@mock.expire(@key, 10, nx: true)).to eq(true) + end + end + + context 'with `xx` option' do + it do + @mock.set(@key, 'string') + + expect(@mock.expire(@key, 10)).to eq(true) + expect(@mock.expire(@key, 10, xx: true)).to eq(true) + + @mock.persist(@key) + + expect(@mock.expire(@key, 10, xx: true)).to eq(false) + end + end + + context 'with `gt` option' do + it do + @mock.set(@key, 'string') + + expect(@mock.expire(@key, 10)).to eq(true) + expect(@mock.expire(@key, 5, gt: true)).to eq(false) + expect(@mock.ttl(@key)).to eq(10) + expect(@mock.expire(@key, 20, gt: true)).to eq(true) + expect(@mock.ttl(@key)).to eq(20) + end + + it 'behaves correcly with non-volatile key' do + @mock.set(@key, 'string') + + expect(@mock.expire(@key, 20, gt: true)).to eq(false) + end + end + + context 'with `lt` option' do + it do + @mock.set(@key, 'string') + + expect(@mock.expire(@key, 10)).to eq(true) + expect(@mock.expire(@key, 20, lt: true)).to eq(false) + expect(@mock.ttl(@key)).to eq(10) + expect(@mock.expire(@key, 5, lt: true)).to eq(true) + expect(@mock.ttl(@key)).to eq(5) + end + + it 'behaves correcly with non-volatile key' do + @mock.set(@key, 'string') + + expect(@mock.expire(@key, 20, lt: true)).to eq(true) + end + end end end diff --git a/spec/commands/expireat_spec.rb b/spec/commands/expireat_spec.rb index 69a902b..3e3e8fe 100644 --- a/spec/commands/expireat_spec.rb +++ b/spec/commands/expireat_spec.rb @@ -25,6 +25,19 @@ end.to raise_error(Redis::CommandError) end + it 'works with options', redis: '7.0' do + expect(@redises.expire(@key, Time.now.to_i + 20)).to eq(true) + expect(@redises.expire(@key, Time.now.to_i + 10, lt: true)).to eq(true) + expect(@redises.expire(@key, Time.now.to_i + 15, lt: true)).to eq(false) + expect(@redises.expire(@key, Time.now.to_i + 20, gt: true)).to eq(true) + expect(@redises.expire(@key, Time.now.to_i + 15, gt: true)).to eq(false) + expect(@redises.expire(@key, Time.now.to_i + 10, xx: true)).to eq(true) + expect(@redises.expire(@key, Time.now.to_i + 10, nx: true)).to eq(false) + expect(@redises.persist(@key)).to eq(true) + expect(@redises.expire(@key, Time.now.to_i + 10, xx: true)).to eq(false) + expect(@redises.expire(@key, Time.now.to_i + 10, nx: true)).to eq(true) + end + context '[mock only]' do # These are mock-only since we can't actually manipulate time in # the real Redis. @@ -38,6 +51,8 @@ allow(Time).to receive(:now).and_return(@now) end + it_should_behave_like 'raises on invalid expire command options', :expireat + it 'removes keys after enough time has passed' do @mock.expireat(@key, @now.to_i + 5) allow(Time).to receive(:now).and_return(@now + 5) diff --git a/spec/commands/pexpire_spec.rb b/spec/commands/pexpire_spec.rb index f4cf09e..44ed96c 100644 --- a/spec/commands/pexpire_spec.rb +++ b/spec/commands/pexpire_spec.rb @@ -25,6 +25,19 @@ end.to raise_error(Redis::CommandError) end + it 'works with options', redis: '7.0' do + expect(@redises.expire(@key, 20)).to eq(true) + expect(@redises.expire(@key, 10, lt: true)).to eq(true) + expect(@redises.expire(@key, 15, lt: true)).to eq(false) + expect(@redises.expire(@key, 20, gt: true)).to eq(true) + expect(@redises.expire(@key, 15, gt: true)).to eq(false) + expect(@redises.expire(@key, 10, xx: true)).to eq(true) + expect(@redises.expire(@key, 10, nx: true)).to eq(false) + expect(@redises.persist(@key)).to eq(true) + expect(@redises.expire(@key, 10, xx: true)).to eq(false) + expect(@redises.expire(@key, 10, nx: true)).to eq(true) + end + it 'stringifies key' do expect(@redises.pexpire(@key.to_sym, 9)).to eq(true) end @@ -42,6 +55,8 @@ allow(Time).to receive(:now).and_return(@now) end + it_should_behave_like 'raises on invalid expire command options', :pexpire + it 'removes keys after enough time has passed' do @mock.pexpire(@key, 5) allow(Time).to receive(:now).and_return(@now + Rational(6, 1000)) diff --git a/spec/commands/pexpireat_spec.rb b/spec/commands/pexpireat_spec.rb index 4c5e14d..a96cf1a 100644 --- a/spec/commands/pexpireat_spec.rb +++ b/spec/commands/pexpireat_spec.rb @@ -6,17 +6,21 @@ @redises.set(@key, 'spork') end + def now_ms + (Time.now.to_f * 1000).to_i + end + it 'returns true for a key that exists' do - expect(@redises.pexpireat(@key, (Time.now.to_f * 1000).to_i + 1)).to eq(true) + expect(@redises.pexpireat(@key, now_ms + 1)).to eq(true) end it 'returns false for a key that does not exist' do expect(@redises.pexpireat('mock-redis-test:nonesuch', - (Time.now.to_f * 1000).to_i + 1)).to eq(false) + now_ms + 1)).to eq(false) end it 'removes a key immediately when timestamp is now' do - @redises.pexpireat(@key, (Time.now.to_f * 1000).to_i) + @redises.pexpireat(@key, now_ms) expect(@redises.get(@key)).to be_nil end @@ -26,6 +30,19 @@ end.to raise_error(Redis::CommandError) end + it 'works with options', redis: '7.0' do + expect(@redises.expire(@key, now_ms + 20)).to eq(true) + expect(@redises.expire(@key, now_ms + 10, lt: true)).to eq(true) + expect(@redises.expire(@key, now_ms + 15, lt: true)).to eq(false) + expect(@redises.expire(@key, now_ms + 20, gt: true)).to eq(true) + expect(@redises.expire(@key, now_ms + 15, gt: true)).to eq(false) + expect(@redises.expire(@key, now_ms + 10, xx: true)).to eq(true) + expect(@redises.expire(@key, now_ms + 10, nx: true)).to eq(false) + expect(@redises.persist(@key)).to eq(true) + expect(@redises.expire(@key, now_ms + 10, xx: true)).to eq(false) + expect(@redises.expire(@key, now_ms + 10, nx: true)).to eq(true) + end + context '[mock only]' do # These are mock-only since we can't actually manipulate time in # the real Redis. @@ -39,6 +56,8 @@ allow(Time).to receive(:now).and_return(@now) end + it_should_behave_like 'raises on invalid expire command options', :pexpireat + it 'removes keys after enough time has passed' do @mock.pexpireat(@key, (@now.to_f * 1000).to_i + 5) allow(Time).to receive(:now).and_return(@now + 0.006) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index d35fe1f..0aa31a7 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -75,4 +75,9 @@ def args_for_method(method) end @redises._gsub_clear end + + # By default, all the specs are considered to be compatible with redis 6, + # specs for redis 7 should be marked with `redis: 7.0` tag. + config.run_all_when_everything_filtered = true + config.filter_run_excluding redis: 7.0 end diff --git a/spec/support/shared_examples/raises_on_invalid_expire_command_options.rb b/spec/support/shared_examples/raises_on_invalid_expire_command_options.rb new file mode 100644 index 0000000..236b4de --- /dev/null +++ b/spec/support/shared_examples/raises_on_invalid_expire_command_options.rb @@ -0,0 +1,20 @@ +RSpec.shared_examples_for 'raises on invalid expire command options' do |command| + [%i[nx xx], %i[nx lt], %i[nx gt], %i[lt gt]].each do |options| + context "with `#{options[0]}` and `#{options[1]}` options" do + it 'raises `Redis::CommandError`' do + expect { @mock.public_send(command, @key, 1, **options.zip([true, true]).to_h) } + .to raise_error( + Redis::CommandError, + 'ERR NX and XX, GT or LT options at the same time are not compatible' + ) + end + end + + context 'with unexpected key' do + it 'raises `ArgumentError`' do + expect { @mock.public_send(command, @key, 1, foo: true) } + .to raise_error(ArgumentError) + end + end + end +end