Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

RedisResourceManager breaks if being configured via StorageCacheAbstractServiceFactory/array and server key isn't first in options array #109

Closed
rarog opened this issue Jul 21, 2016 · 3 comments
Assignees
Labels

Comments

@rarog
Copy link

rarog commented Jul 21, 2016

I take the example from the docs to demonstrate the problem:

'caches' => array( 'Cache\Transient' => array( 'adapter' => 'redis', 'ttl' => 3600, 'options' => array( 'server' => array( 'host'=>'localhost', 'port' => 6379, 'timeout' => 2.5, ), 'database' => 5, 'password' => '', ), 'plugins' => array( 'exception_handler' => array( 'throw_exceptions' => false, ), 'serializer', ), ), ),

The relevant part is the options array.

'options' => array( 'server' => array( 'host'=>'localhost', 'port' => 6379, 'timeout' => 2.5, ), 'database' => 0, 'password' => '', ),
would work, but for example
'options' => array( 'database' => 0, 'server' => array( 'host'=>'localhost', 'port' => 6379, 'timeout' => 2.5, ), 'password' => '', ),
wouldn't.

All the setters in RedisResourceManager look if the id is already initialised, if not, every setter calls setResource with only his own parameter.
setResource merges this with the default array, which has empty array for the server key. At the end of the function setResource always calls normalizeServer, which checks if key host is set and throws an exception, if it's not.
As it is now, the code fails for every initialisation, where server key isn't the first inside the options array, because an exception is thrown.

I'm not making own fix via pull request, because this should be a design decision.

This problem also exists in the LTS 2.4 branch in exactly the same way.

@rarog rarog changed the title RedisResourceManager breaks if being configured via StorageCacheAbstractServiceFactory/array and service isn't first member of array RedisResourceManager breaks if being configured via StorageCacheAbstractServiceFactory/array and server key isn't first options array Jul 21, 2016
@rarog rarog changed the title RedisResourceManager breaks if being configured via StorageCacheAbstractServiceFactory/array and server key isn't first options array RedisResourceManager breaks if being configured via StorageCacheAbstractServiceFactory/array and server key isn't first in options array Jul 21, 2016
@marc-mabe marc-mabe self-assigned this Aug 16, 2016
@marc-mabe marc-mabe added the bug label Aug 16, 2016
@marc-mabe
Copy link
Member

It would be simple to just remove the exception at this point and throw it on trying to connect but I need to think about this a little more if this would cause side effects.

@bcremer
Copy link

bcremer commented Oct 17, 2017

I stubled upon this issue while setting lib_options for the redis adapter. Turns out the server configuration must come first.

Script to reproduce

<?php
require __DIR__ . '/vendor/autoload.php';

$config = [
    'adapter' => [
        'name' => 'redis',
        'options' => [
            'server' => [
                'host' => 'localhost',
                'port' => 6379,
            ],
        ]
    ]
];

// works
$redisCache = \Zend\Cache\StorageFactory::factory($config);
var_dump(get_class($redisCache));

$config = [
    'adapter' => [
        'name' => 'redis',
        'options' => [
            'server' => [
                'host' => 'localhost',
                'port' => 6379,
            ],
            'lib_options' => [
                \Redis::OPT_SERIALIZER => \Redis::SERIALIZER_IGBINARY,
            ],
        ]
    ]
];

// works
$redisCache = \Zend\Cache\StorageFactory::factory($config);
var_dump(get_class($redisCache));

$config = [
    'adapter' => [
        'name' => 'redis',
        'options' => [
            'lib_options' => [
                \Redis::OPT_SERIALIZER => \Redis::SERIALIZER_IGBINARY,
            ],
            'server' => [
                'host' => 'localhost',
                'port' => 6379,
            ],
        ]
    ]
];

// fails
$redisCache = \Zend\Cache\StorageFactory::factory($config);
var_dump(get_class($redisCache));

thomasvargiu added a commit to thomasvargiu/zend-cache that referenced this issue Apr 18, 2018
thomasvargiu added a commit to thomasvargiu/zend-cache that referenced this issue Apr 18, 2018
weierophinney added a commit that referenced this issue Apr 18, 2018
@samsonasik
Copy link
Contributor

should can be closed in favor of #151

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants