Skip to content

Commit

Permalink
Merge pull request #32 from ParentSquare/fix-redis-pipeline-deprecations
Browse files Browse the repository at this point in the history
  • Loading branch information
aried3r committed Mar 2, 2022
2 parents 3519ad0 + a9f8894 commit d9faf8f
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 22 deletions.
4 changes: 2 additions & 2 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ GEM
ast (~> 2.4.1)
rainbow (3.0.0)
rake (13.0.1)
redis (4.2.1)
redis (4.6.0)
rexml (3.2.5)
rspec (3.9.0)
rspec-core (~> 3.9.0)
Expand Down Expand Up @@ -93,4 +93,4 @@ DEPENDENCIES
simplecov

BUNDLED WITH
2.1.4
2.2.21
13 changes: 9 additions & 4 deletions lib/modis/finder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ def all

records = Modis.with_connection do |redis|
ids = redis.smembers(key_for(:all))
redis.pipelined do
ids.map { |id| record_for(redis, id) }
redis.pipelined do |pipeline|
ids.map { |id| record_for(pipeline, id) }
end
end

Expand All @@ -42,8 +42,13 @@ def find_all(ids)
raise RecordNotFound, "Couldn't find #{name} without an ID" if ids.empty?

records = Modis.with_connection do |redis|
blk = proc { |id| record_for(redis, id) }
ids.count == 1 ? ids.map(&blk) : redis.pipelined { ids.map(&blk) }
if ids.count == 1
ids.map { |id| record_for(redis, id) }
else
redis.pipelined do |pipeline|
ids.map { |id| record_for(pipeline, id) }
end
end
end

models = records_to_models(records)
Expand Down
22 changes: 11 additions & 11 deletions lib/modis/persistence.rb
Original file line number Diff line number Diff line change
Expand Up @@ -135,13 +135,13 @@ def save!(args = {})
def destroy
self.class.transaction do |redis|
run_callbacks :destroy do
redis.pipelined do
remove_from_indexes(redis)
redis.pipelined do |pipeline|
remove_from_indexes(pipeline)
if self.class.all_index_enabled?
redis.srem(self.class.key_for(:all), id)
redis.srem(self.class.sti_base_key_for(:all), id) if self.class.sti_child?
pipeline.srem(self.class.key_for(:all), id)
pipeline.srem(self.class.sti_base_key_for(:all), id) if self.class.sti_child?
end
redis.del(key)
pipeline.del(key)
end
end
end
Expand Down Expand Up @@ -210,19 +210,19 @@ def persist
self.class.transaction do |redis|
run_callbacks :save do
run_callbacks callback do
redis.pipelined do
redis.pipelined do |pipeline|
attrs = coerced_attributes
key = self.class.sti_child? ? self.class.sti_base_key_for(id) : self.class.key_for(id)
future = attrs.any? ? redis.hmset(key, attrs) : :unchanged
future = attrs.any? ? pipeline.hmset(key, attrs) : :unchanged

if new_record?
if self.class.all_index_enabled?
redis.sadd(self.class.key_for(:all), id)
redis.sadd(self.class.sti_base_key_for(:all), id) if self.class.sti_child?
pipeline.sadd(self.class.key_for(:all), id)
pipeline.sadd(self.class.sti_base_key_for(:all), id) if self.class.sti_child?
end
add_to_indexes(redis)
add_to_indexes(pipeline)
else
update_indexes(redis)
update_indexes(pipeline)
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/modis/transaction.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def self.included(base)

module ClassMethods
def transaction
Modis.with_connection { |redis| redis.multi { yield(redis) } }
Modis.with_connection { |redis| redis.multi { |transaction| yield(transaction) } }
end
end
end
Expand Down
18 changes: 14 additions & 4 deletions spec/persistence_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,13 @@ class MockModelNoAllIndex < MockModel

it 'does not track the ID if the underlying Redis command failed' do
redis = double(hmset: double(value: nil), sadd: nil)
expect(model.class).to receive(:transaction).and_yield(redis)
expect(redis).to receive(:pipelined).and_yield
if Gem::Version.new(Redis::VERSION) > Gem::Version.new('4.6.0')
expect(model.class).to receive(:transaction).and_yield(Redis::PipelinedConnection.new(Redis::Pipeline::Multi.new(redis)))
expect(redis).to receive(:pipelined).and_yield(Redis::PipelinedConnection.new(Redis::Pipeline.new(redis)))
else
expect(model.class).to receive(:transaction).and_yield(redis)
expect(redis).to receive(:pipelined).and_yield(redis)
end
model.save
expect { model.class.find(model.id) }.to raise_error(Modis::RecordNotFound)
end
Expand All @@ -135,8 +140,13 @@ class MockModelNoAllIndex < MockModel
model.age = 11
redis = double
expect(redis).to receive(:hmset).with("modis:persistence_spec:mock_model:1", ["age", "\v"]).and_return(double(value: 'OK'))
expect(model.class).to receive(:transaction).and_yield(redis)
expect(redis).to receive(:pipelined).and_yield
if Gem::Version.new(Redis::VERSION) > Gem::Version.new('4.6.0')
expect(model.class).to receive(:transaction).and_yield(Redis::PipelinedConnection.new(Redis::Pipeline::Multi.new(redis)))
expect(redis).to receive(:pipelined).and_yield(Redis::PipelinedConnection.new(Redis::Pipeline.new(redis)))
else
expect(model.class).to receive(:transaction).and_yield(redis)
expect(redis).to receive(:pipelined).and_yield(redis)
end
model.save!
expect(model.age).to eq(11)
end
Expand Down
2 changes: 2 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@

require 'modis'

Redis.raise_deprecations = true if Gem.loaded_specs['redis'].version >= Gem::Version.new('4.6.0')

Modis.configure do |config|
config.namespace = 'modis'
end
Expand Down

0 comments on commit d9faf8f

Please sign in to comment.