Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[9.x] Make sure the prefix override behaviours are the same between phpredis and predis drivers #42279

Merged
merged 1 commit into from
May 9, 2022

Conversation

s-patompong
Copy link
Contributor

Yesterday I was doing research for the company (mainly to do with Redis prefix options) and I found that there are two problems with the implementation of the \Illuminate\Redis\Connectors\PhpRedisConnector::connect method.

The first issue is that we use Arr::pull to pull the options from the config array, like so:

$connector = function () use ($config, $options, $formattedOptions) {
    return $this->createClient(array_merge(
        $config, $options, Arr::pull($config, 'options', [])
    ));
};

The issue with this code is that the Arr::pull won't take effect and the final array from the array_merge will contain the options key, as I can show you here in a simple dd snippet:

$config = [
    'host' => 'redis.com',
    'options' => [
        'prefix' => 'config_redis_',
    ],
];

$options = [
    'prefix' => 'redis_'
];

dd(array_merge($config, $options, Arr::pull($config, 'options', [])));

The output is

[
  "host" => "redis.com"
  "options" => array:1 [
    "prefix" => "config_redis_"
  ]
  "prefix" => "config_redis_"
]

This PR will make sure that we pull out the options key from the config so that we don't merge it into the final array. This is merely just a DX touch.

Secondly, I found that in the \Illuminate\Redis\Connectors\PredisConnector::connect method, we have the code to override the prefix inside the options key of each redis config with the config's prefix key itself.

This means for predis driver, the prefix for this config will be config_:

[
    'scheme' => 'tcp',
    'host' => $host,
    'port' => $port,
    'database' => 5,
    'timeout' => 0.5,
    'options' => [
        'prefix' => 'config_options_',
    ],
    'prefix' => 'config_',
]

However, if you use phpredis driver, the prefix that will be used in the connector is config_options_.

This PR makes sure that the prefix override behaviour of the phpredis is working in the same way with the predis connector. This means such config would resolves in the prefix = config_.

@s-patompong s-patompong changed the title Make sure the prefix override behaviours are the same between phpredis and predis Make sure the prefix override behaviours are the same between phpredis and predis driver May 6, 2022
@s-patompong s-patompong changed the title Make sure the prefix override behaviours are the same between phpredis and predis driver Make sure the prefix override behaviours are the same between phpredis and predis drivers May 6, 2022
@s-patompong s-patompong changed the title Make sure the prefix override behaviours are the same between phpredis and predis drivers [9.x] Make sure the prefix override behaviours are the same between phpredis and predis drivers May 7, 2022
@taylorotwell taylorotwell merged commit d8054de into laravel:9.x May 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants