From 484277120f975b7e2f30d9b80fc85364d4e86ace Mon Sep 17 00:00:00 2001 From: Titus Date: Wed, 7 Jul 2021 16:35:32 -0600 Subject: [PATCH 1/2] BUG: Correctly handle MemoryStorage#add (#31) * correctly handle MemoryStorage#add when key already exists but is expired * bump version --- lib/atomic_cache/storage/memory.rb | 11 +++++--- lib/atomic_cache/version.rb | 2 +- spec/atomic_cache/storage/memory_spec.rb | 27 +++++++++++++++---- .../storage/shared_memory_spec.rb | 2 +- 4 files changed, 32 insertions(+), 10 deletions(-) diff --git a/lib/atomic_cache/storage/memory.rb b/lib/atomic_cache/storage/memory.rb index e45b2b0..6541e31 100644 --- a/lib/atomic_cache/storage/memory.rb +++ b/lib/atomic_cache/storage/memory.rb @@ -16,7 +16,7 @@ def store_op(key, user_options={}); raise NotImplementedError end def add(raw_key, new_value, ttl, user_options={}) store_op(raw_key, user_options) do |key, options| - return false if store.has_key?(key) + return false if store.has_key?(key) && !ttl_expired?(store[key]) write(key, new_value, ttl, user_options) end end @@ -29,8 +29,7 @@ def read(raw_key, user_options={}) unmarshaled = unmarshal(entry[:value], user_options) return unmarshaled if entry[:ttl].nil? or entry[:ttl] == false - life = Time.now - entry[:written_at] - if (life >= entry[:ttl]) + if ttl_expired?(entry) store.delete(key) nil else @@ -54,6 +53,12 @@ def delete(raw_key) protected + def ttl_expired?(entry) + return false unless entry + life = Time.now - entry[:written_at] + life >= entry[:ttl] + end + def write(key, value, ttl=nil, user_options) store[key] = { value: marshal(value, user_options), diff --git a/lib/atomic_cache/version.rb b/lib/atomic_cache/version.rb index 0641cfb..612b2b8 100644 --- a/lib/atomic_cache/version.rb +++ b/lib/atomic_cache/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module AtomicCache - VERSION = "0.4.0.rc1" + VERSION = "0.4.1.rc1" end diff --git a/spec/atomic_cache/storage/memory_spec.rb b/spec/atomic_cache/storage/memory_spec.rb index 146eb4e..6cca44b 100644 --- a/spec/atomic_cache/storage/memory_spec.rb +++ b/spec/atomic_cache/storage/memory_spec.rb @@ -17,17 +17,34 @@ expect(result).to eq(true) end - it 'does not write the key if it exists' do - entry = { value: Marshal.dump('foo'), ttl: 100, written_at: 100 } + # SharedMemory.new.add("foo", ttl: 100) + + it 'does not write the key if it exists but expiration time is NOT up' do + entry = { value: Marshal.dump('foo'), ttl: 5000, written_at: Time.local(2021, 1, 1, 12, 0, 0) } subject.store[:key] = entry - result = subject.add('key', 'value', 200) - expect(result).to eq(false) + Timecop.freeze(Time.local(2021, 1, 1, 12, 0, 1)) do + result = subject.add('key', 'value', 5000) + expect(result).to eq(false) + end # stored values should not have changed expect(subject.store).to have_key(:key) expect(Marshal.load(subject.store[:key][:value])).to eq('foo') - expect(subject.store[:key][:ttl]).to eq(100) + end + + it 'does write the key if it exists and expiration time IS up' do + entry = { value: Marshal.dump('foo'), ttl: 50, written_at: Time.local(2021, 1, 1, 12, 0, 0) } + subject.store[:key] = entry + + Timecop.freeze(Time.local(2021, 1, 1, 12, 30, 0)) do + result = subject.add('key', 'value', 50) + expect(result).to eq(true) + end + + # stored values should not have changed + expect(subject.store).to have_key(:key) + expect(Marshal.load(subject.store[:key][:value])).to eq('value') end end diff --git a/spec/atomic_cache/storage/shared_memory_spec.rb b/spec/atomic_cache/storage/shared_memory_spec.rb index 7fc1d1b..faa1a22 100644 --- a/spec/atomic_cache/storage/shared_memory_spec.rb +++ b/spec/atomic_cache/storage/shared_memory_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' require_relative 'memory_spec' -describe 'InstanceMemory' do +describe 'SharedMemory' do subject { AtomicCache::Storage::SharedMemory.new } it_behaves_like 'memory storage' end From a1026c996b30fa7b87416adf0dffda66bf2ea917 Mon Sep 17 00:00:00 2001 From: Deploy Bot Date: Wed, 7 Jul 2021 22:38:56 +0000 Subject: [PATCH 2/2] [release] 0.4.1.rc1 --- CHANGELOG.md | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f5759b..dbe08b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,16 @@ # Changelog -## [0.4.0.rc1](https://github.com/Ibotta/atomic_cache/tree/0.4.0.rc1) (2021-07-07) +## [0.4.1.rc1](https://github.com/Ibotta/atomic_cache/tree/0.4.1.rc1) (2021-07-07) -[Full Changelog](https://github.com/Ibotta/atomic_cache/compare/v0.3.0.rc1...0.4.0.rc1) +[Full Changelog](https://github.com/Ibotta/atomic_cache/compare/v0.4.0.rc1...0.4.1.rc1) + +**Merged pull requests:** + +- BUG: Correctly handle MemoryStorage\#add [\#31](https://github.com/Ibotta/atomic_cache/pull/31) ([tstone](https://github.com/tstone)) + +## [v0.4.0.rc1](https://github.com/Ibotta/atomic_cache/tree/v0.4.0.rc1) (2021-07-07) + +[Full Changelog](https://github.com/Ibotta/atomic_cache/compare/v0.3.0.rc1...v0.4.0.rc1) **Merged pull requests:**