Closed Peace-N closed 5 years ago
Hi @Peace-N
mget needs an array of keys. This change uses only one key of the desired keys.
Also $keys = array_shift($arguments);
returns only a key and it is string, internal mget command you chnaged needs an array.
Usage example of mget is like that:
Say we need to fetch n keys.
$rejsonClient->mget('key_1', 'key_2', ..., 'key_n', '_path_');
The expression in the documentation is ...$keys
means there is a number of arguments, it is called as "variadic parameter" and it fetches arguments as an array.
What was your intent?
SO Ok we finally managed to get it to work, we were passing dynamic arguments from the db and i think the issue was on unpacking the arguments and also add the path as the last argument element. Here is a wrapper function we created and used ..
public function mget(array $args, string $path) {
// Add path to array, this is expected as the last array element.
$args[] = $path;
return $rejsonClient->mget(...$args);
}
Hi again @Peace-N
Command follows raw redis rejson command api. See details at https://oss.redislabs.com/redisjson/commands/#jsonmget
But you are right about one may want to provide keys as an array. We must allow that without breaking backward compatibility and rejson api compatibility.
Would you change this part in your code:
$keys = array_shift($arguments);
to this and send PR again?
if (is_array($keys[0])) {
$keys = $keys[0];
}
And one more addition: Would you add a test case to cover this change. You can find the related test at: https://github.com/mkorkmaz/redislabs-rejson/blob/464886bdcecb9103df63d20320d00de0b2a63a57/tests/Module/ReJSONTest.php#L127
There is an issue on the MultipleGet command class, we have to shift the arguments by one up the same way we pop the path argument ..