Skip to content

Commit

Permalink
Fix bug with decoding multiget result
Browse files Browse the repository at this point in the history
Problem:
    GET operation with multiple keys was throwing Yajl::ParseError

Solution:
    take into account that memcached backend returns key/value pairs in result
    to multiget query

Change-Id: I7a0ebeae40f87b67e9f9eb0c36b3acb7b72d590a
Reviewed-on: http://review.couchbase.org/9353
Reviewed-by: Matt Ingenthron <matt@couchbase.com>
Tested-by: Sergey Avseyev <sergey.avseyev@gmail.com>
  • Loading branch information
avsej committed Sep 5, 2011
1 parent 89c4666 commit 16f6a9f
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 22 deletions.
45 changes: 31 additions & 14 deletions lib/couchbase/memcached.rb
Expand Up @@ -104,21 +104,38 @@ def flush
# behaviour if you pass Array of keys, which is much faster than normal
# mode.
#
# @note If you pass a String key, and the key does not exist on the
# server, <b>Memcached::NotFound</b> will be raised. If you pass an array
# of keys, memcached's <tt>multiget</tt> mode will be used, and a hash of
# key/value pairs will be returned. The hash will contain only the keys
# that were found.
#
# @param [Array, String] keys list of String keys or single key
#
# @param [Hash] options the options for operation
# @option options [Symbol] :format (self.default_format) format of the value
#
# @raise [Memcached::NotFound] if the key does not exist on the server.
def get(keys, options = {})
# @overload get(key, options = {})
# Get single key.
#
# @param [String] key
# @param [Hash] options the options for operation
# @option options [Symbol] :format (self.default_format) format of the value
# @return [Object] the value is associated with the key
# @raise [Memcached::NotFound] if the key does not exist on the server.
#
# @overload get(*keys, options = {})
# Get multiple keys aka multiget (mget).
#
# @param [Array] keys the list of keys
# @param [Hash] options the options for operation
# @option options [Symbol] :format (self.default_format) format of the value
# @return [Hash] result map, where keys are keys which were requested
# and values are the values from the storage. Result will contain only
# existing keys and in this case no exception will be raised.
def get(*args)
options = args.last.is_a?(Hash) ? args.pop : {}
raise ArgumentError, "You must provide at least one key" if args.empty?
keys = args.length == 1 ? args.pop : args
format = options[:format] || @default_format
decode(@memcached.get(keys, format == :marshal), format)
rv = @memcached.get(keys, format == :marshal)
if keys.is_a?(Array)
rv.keys.each do |key|
rv[key] = decode(rv[key], format)
end
rv
else
decode(rv, format)
end
end

# Shortcut to <tt>#get</tt> operation. Gets a key's value from the server
Expand Down
47 changes: 39 additions & 8 deletions test/test_memcached.rb
Expand Up @@ -3,26 +3,57 @@
class TestMemcached < MiniTest::Unit::TestCase

def test_race_absence_during_initialization
client = connection
session = connect
assert_raises(Memcached::NotFound) do
client.get(test_id)
session.get(test_id)
end
end

def test_experimental_features_is_always_on
assert_respond_to connection, :touch
assert_respond_to connection(:experimental_features => false), :touch
assert_respond_to connection('experimental_features' => false), :touch
assert_respond_to connect, :touch
assert_respond_to connect(:experimental_features => false), :touch
assert_respond_to connect('experimental_features' => false), :touch
end

def test_single_get
session = connect
session.set(test_id, "foo")
assert "foo", session.get(test_id)
end

def test_single_get_missing
assert_raises(Memcached::NotFound) do
connect.get("missing-key")
end
end

def test_multi_get
session = connect
session.set(test_id(1), "foo")
session.set(test_id(2), "bar")
expected = {test_id(1) => "foo", test_id(2) => "bar"}
assert expected, session.get(test_id(1), test_id(2))
ids = [1, 2].map{|n| test_id(n)}
assert expected, session.get(ids)
end

def test_multi_get_missing
session = connect
session.set(test_id(1), "foo")
expected = {test_id(1) => "foo"}
assert expected, session.get(test_id(1), test_id(2))
ids = [1, 2].map{|n| test_id(n)}
assert expected, session.get(ids)
end

protected

def connection(options = {})
def connect(options = {})
uri = options.delete(:pool_uri) || "http://localhost:8091/pools/default"
Couchbase.new(uri, options)
end

def test_id
name = caller.first[/.*[` ](.*)'/, 1]
def test_id(suffix = nil)
"#{caller.first[/.*[` ](.*)'/, 1]}#{suffix}"
end
end

0 comments on commit 16f6a9f

Please sign in to comment.