Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Add `#sort` command support. #83

Merged
merged 5 commits into from

3 participants

@larrylv
  • Most codes are referenced from mock_redis' implementation.

  • There is a monkey-patch for redis, because parameter options(<Hash>) can't
    be passed to #sort in the old way.

@guilleiguaran

/cc @caius what do you think about his? :sweat_smile:

spec/spec_helper.rb
@@ -4,6 +4,9 @@
require 'fakeredis'
require "fakeredis/rspec"
+$LOAD_PATH.unshift(File.expand_path(File.join(File.dirname(__FILE__), '..')))
+Dir["spec/support/**/*.rb"].each {|x| require x}
@caius Collaborator
caius added a note

Given ./spec is already in the $LOAD_PATH, we could just replace this with require "support/shared_examples/sortable" to make it more explicit I believe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@caius
Collaborator

I'm not such a fan of patching redis, as it's not under our control and I'd prefer us to be exercising the redis sort method, rather than presuming it will always be sort(key, options = {}).

I've had a bash at reimplementing this without having to patch redis and have gotten the tests passing with it. The kicker is that redis parses the options into an array, of the same format defined in redis-cli and listed on the sort command help page:

[BY pattern] [LIMIT offset count] [GET pattern [GET pattern ...]] [ASC|DESC] [ALPHA] [STORE destination]

The diff of my branch shows how it works, the main change is in FakeRedis::SortMethod#extract_options_from, which carefully nibbles away at the options array, parsing out each viable option into the original hash.

I think I prefer us using redis-rb's actual methods, and remembering that Memory is effectively redis-server, so we might have some extra options parsing to do occasionally for the more difficult methods. I wouldn't be against us trying to extract the options parsing in future, I can imagine there's other commands that need the same sort of parsing too, albeit with key name changes, etc.

What do you think of this approach @larrylv?

@larrylv

@caius I think your solution is great. I have picked your commit, and done a rebase(just squash the removal of the redis monkey patch).

Please take a look!

@caius caius merged commit 8ee3e42 into from
@caius
Collaborator

Many thanks for your contribution! :grinning: :tada:

@larrylv larrylv deleted the branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 21, 2014
  1. @larrylv

    Add `#sort` command support.

    larrylv authored
    * Most codes are referenced from mock_redis' implementation.
  2. @caius @larrylv

    Simplify spec support require

    caius authored larrylv committed
  3. @caius @larrylv

    Explicitly spec sorting by ASC & storing sort with options

    caius authored larrylv committed
  4. @caius @larrylv

    Extract options from redis options array for sort

    caius authored larrylv committed
  5. @caius @larrylv

    Add explicit spec for sort ordering by ALPHA

    caius authored larrylv committed
This page is out of date. Refresh to see the latest.
View
116 lib/fakeredis/sort_method.rb
@@ -0,0 +1,116 @@
+# Codes are mostly referenced from MockRedis' implementation.
+module FakeRedis
+ module SortMethod
+ def sort(key, *redis_options_array)
+ return [] unless key
+
+ unless %w(list set zset).include? type(key)
+ warn "Operation against a key holding the wrong kind of value: Expected list, set or zset at #{key}."
+ raise Redis::CommandError.new("WRONGTYPE Operation against a key holding the wrong kind of value")
+ end
+
+ # redis_options is an array of format [BY pattern] [LIMIT offset count] [GET pattern [GET pattern ...]] [ASC|DESC] [ALPHA] [STORE destination]
+ # Lets nibble it back into a hash
+ options = extract_options_from(redis_options_array)
+
+ # And now to actually do the work of this method
+
+ projected = project(data[key], options[:by], options[:get])
+ sorted = sort_by(projected, options[:order])
+ sliced = slice(sorted, options[:limit])
+ # We have to flatten it down as redis-rb adds back the array to the return value
+ result = sliced.flatten(1)
+
+ options[:store] ? rpush(options[:store], sliced) : sliced.flatten(1)
+ end
+
+ private
+
+ ASCENDING_SORT = Proc.new { |a, b| a.first <=> b.first }
+ DESCENDING_SORT = Proc.new { |a, b| b.first <=> a.first }
+
+ def extract_options_from(options_array)
+ # Defaults
+ options = {
+ :limit => [],
+ :order => "ASC",
+ :get => []
+ }
+
+ if options_array.first == "BY"
+ options_array.shift
+ options[:by] = options_array.shift
+ end
+
+ if options_array.first == "LIMIT"
+ options_array.shift
+ options[:limit] = [options_array.shift, options_array.shift]
+ end
+
+ while options_array.first == "GET"
+ options_array.shift
+ options[:get] << options_array.shift
+ end
+
+ if %w(ASC DESC ALPHA).include?(options_array.first)
+ options[:order] = options_array.shift
+ options[:order] = "ASC" if options[:order] == "ALPHA"
+ end
+
+ if options_array.first == "STORE"
+ options_array.shift
+ options[:store] = options_array.shift
+ end
+
+ options
+ end
+
+ def project(enumerable, by, get_patterns)
+ enumerable.map do |*elements|
+ element = elements.flatten.first
+ weight = by ? lookup_from_pattern(by, element) : element
+ value = element
+
+ if get_patterns.length > 0
+ value = get_patterns.map do |pattern|
+ pattern == "#" ? element : lookup_from_pattern(pattern, element)
+ end
+ value = value.first if value.length == 1
+ end
+
+ [weight, value]
+ end
+ end
+
+ def sort_by(projected, direction)
+ sorter =
+ case direction.upcase
+ when "DESC"
+ DESCENDING_SORT
+ when "ASC", "ALPHA"
+ ASCENDING_SORT
+ else
+ raise "Invalid direction '#{direction}'"
+ end
+
+ projected.sort(&sorter).map(&:last)
+ end
+
+ def slice(sorted, limit)
+ skip = limit.first || 0
+ take = limit.last || sorted.length
+
+ sorted[skip...(skip + take)] || sorted
+ end
+
+ def lookup_from_pattern(pattern, element)
+ key = pattern.sub('*', element)
+
+ if (hash_parts = key.split('->')).length > 1
+ hget hash_parts.first, hash_parts.last
+ else
+ get key
+ end
+ end
+ end
+end
View
7 lib/redis/connection/memory.rb
@@ -2,6 +2,7 @@
require 'redis/connection/registry'
require 'redis/connection/command_helper'
require "fakeredis/expiring_hash"
+require "fakeredis/sort_method"
require "fakeredis/sorted_set_argument_handler"
require "fakeredis/sorted_set_store"
require "fakeredis/zset"
@@ -11,6 +12,7 @@ module Connection
class Memory
include Redis::Connection::CommandHelper
include FakeRedis
+ include SortMethod
attr_accessor :buffer, :options
@@ -104,7 +106,6 @@ def read
# * brpop
# * brpoplpush
# * discard
- # * sort
# * subscribe
# * psubscribe
# * publish
@@ -687,10 +688,6 @@ def msetnx(*pairs)
true
end
- def sort(key)
- # TODO: Implement
- end
-
def incr(key)
data.merge!({ key => (data[key].to_i + 1).to_s || "1"})
data[key].to_i
View
68 spec/sort_method_spec.rb
@@ -0,0 +1,68 @@
+require 'spec_helper'
+
+module FakeRedis
+ describe "#sort" do
+ before(:each) do
+ @client = Redis.new
+
+ @client.set('fake-redis-test:values_1', 'a')
+ @client.set('fake-redis-test:values_2', 'b')
+
+ @client.set('fake-redis-test:weight_1', '2')
+ @client.set('fake-redis-test:weight_2', '1')
+
+ @client.hset('fake-redis-test:hash_1', 'key', 'x')
+ @client.hset('fake-redis-test:hash_2', 'key', 'y')
+ end
+
+ context "WRONGTYPE Operation" do
+ it "should not allow #sort on Strings" do
+ @client.set("key1", "Hello")
+ expect {
+ @client.sort("key1")
+ }.to raise_error(Redis::CommandError)
+ end
+
+ it "should not allow #sort on Hashes" do
+ @client.hset("key1", "k1", "val1")
+ @client.hset("key1", "k2", "val2")
+ expect {
+ @client.sort("key1")
+ }.to raise_error(Redis::CommandError)
+ end
+ end
+
+ context "list" do
+ before do
+ @key = "fake-redis-test:list_sort"
+
+ @client.rpush(@key, '1')
+ @client.rpush(@key, '2')
+ end
+
+ it_should_behave_like "a sortable"
+ end
+
+ context "set" do
+ before do
+ @key = "fake-redis-test:set_sort"
+
+ @client.sadd(@key, '1')
+ @client.sadd(@key, '2')
+ end
+
+ it_should_behave_like "a sortable"
+ end
+
+ context "zset" do
+ before do
+ @key = "fake-redis-test:zset_sort"
+
+ @client.zadd(@key, 100, '1')
+ @client.zadd(@key, 99, '2')
+ end
+
+ it_should_behave_like "a sortable"
+ end
+ end
+end
View
2  spec/spec_helper.rb
@@ -4,6 +4,8 @@
require 'fakeredis'
require "fakeredis/rspec"
+require "support/shared_examples/sortable"
+
def fakeredis?
true
end
View
69 spec/support/shared_examples/sortable.rb
@@ -0,0 +1,69 @@
+shared_examples_for "a sortable" do
+ it 'returns empty array on nil' do
+ @client.sort(nil).should == []
+ end
+
+ context 'ordering' do
+ it 'orders ascending by default' do
+ @client.sort(@key).should == ['1', '2']
+ end
+
+ it 'orders by ascending when specified' do
+ @client.sort(@key, :order => "ASC").should == ['1', '2']
+ end
+
+ it 'orders by descending when specified' do
+ @client.sort(@key, :order => "DESC").should == ['2', '1']
+ end
+
+ it "orders by ascending when alpha is specified" do
+ @client.sort(@key, :order => "ALPHA").should == ["1", "2"]
+ end
+ end
+
+ context 'projections' do
+ it 'projects element when :get is "#"' do
+ @client.sort(@key, :get => '#').should == ['1', '2']
+ end
+
+ it 'projects through a key pattern' do
+ @client.sort(@key, :get => 'fake-redis-test:values_*').should == ['a', 'b']
+ end
+
+ it 'projects through a key pattern and reflects element' do
+ @client.sort(@key, :get => ['#', 'fake-redis-test:values_*']).should == [['1', 'a'], ['2', 'b']]
+ end
+
+ it 'projects through a hash key pattern' do
+ @client.sort(@key, :get => 'fake-redis-test:hash_*->key').should == ['x', 'y']
+ end
+ end
+
+ context 'weights' do
+ it 'weights by projecting through a key pattern' do
+ @client.sort(@key, :by => "fake-redis-test:weight_*").should == ['2', '1']
+ end
+
+ it 'weights by projecting through a key pattern and a specific order' do
+ @client.sort(@key, :order => "DESC", :by => "fake-redis-test:weight_*").should == ['1', '2']
+ end
+ end
+
+ context 'limit' do
+ it 'only returns requested window in the enumerable' do
+ @client.sort(@key, :limit => [0, 1]).should == ['1']
+ end
+ end
+
+ context 'store' do
+ it 'stores into another key' do
+ @client.sort(@key, :store => "fake-redis-test:some_bucket").should == 2
+ @client.lrange("fake-redis-test:some_bucket", 0, -1).should == ['1', '2']
+ end
+
+ it "stores into another key with other options specified" do
+ @client.sort(@key, :store => "fake-redis-test:some_bucket", :by => "fake-redis-test:weight_*").should == 2
+ @client.lrange("fake-redis-test:some_bucket", 0, -1).should == ['2', '1']
+ end
+ end
+end
Something went wrong with that request. Please try again.