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.

Sign up for GitHub

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

Add Serialization option #411

Merged
merged 5 commits into from
Apr 17, 2018
Merged

Add Serialization option #411

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fae13ae
Add Serialization option
yellow1912 Apr 12, 2018
705769d
Minor code style update. Remove HHVM support
yellow1912 Apr 12, 2018
644bdf5
Switch to using Redis Class Constants instead of hard-coded values
yellow1912 Apr 14, 2018
a27f2db
Fix IGBINARY Dependency
yellow1912 Apr 17, 2018
3b9d70d
Remove ?? to support php5
yellow1912 Apr 17, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions DependencyInjection/Configuration/Configuration.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ private function addClientsSection(ArrayNodeDefinition $rootNode)
->scalarNode('read_write_timeout')->defaultNull()->end()
->booleanNode('iterable_multibulk')->defaultFalse()->end()
->booleanNode('throw_errors')->defaultTrue()->end()
->scalarNode('serialization')->defaultValue('default')->end()
->scalarNode('profile')->defaultValue('default')
->end()
->scalarNode('cluster')->defaultNull()->end()
Expand Down
39 changes: 38 additions & 1 deletion DependencyInjection/SncRedisExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,12 @@ protected function loadPhpredisClient(array $client, ContainerBuilder $container
if (null !== $dsn->getDatabase()) {
$phpredisDef->addMethodCall('select', array($dsn->getDatabase()));
}
if ($client['options']['serialization']) {
$phpredisDef->addMethodCall(
'setOption',
array(\Redis::OPT_SERIALIZER, $this->loadSerializationType($client['options']['serialization']))
);
}
$container->setDefinition($phpredisId, $phpredisDef);

$container->setAlias(sprintf('snc_redis.%s', $client['alias']), $phpredisId);
Expand Down Expand Up @@ -427,7 +433,38 @@ protected function loadSwiftMailer(array $config, ContainerBuilder $container)
}

/**
* Loads the profiler storage configuration.
* Load the correct serializer for Redis
*
* @param string $type
*
* @return string
* @throws InvalidConfigurationException
*/
public function loadSerializationType($type)
{
$types = array(
'none' => \Redis::SERIALIZER_NONE,
'php' => \Redis::SERIALIZER_PHP,
'igbinary' => \Redis::SERIALIZER_IGBINARY
);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the dependency on these constants, why can it even be invoked if phpredis is not loaded?

Also single quotes ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean to say that we should use this instead?

$types = array(
'none' => 0, \Redis::SERIALIZER_NONE,
'php' => 1, \Redis::SERIALIZER_PHP,
'igbinary' => 2 \Redis::SERIALIZER_IGBINARY
);

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says we're using the integers instead of the constants as they would fail if phpredis is not loaded, but in that case the serialization wouldn't work anyway, so how come we have to account for it?


// allow user to pass in default serialization in which case we should automatically decide for them
if ('default' == $type) {
return defined('Redis::SERIALIZER_IGBINARY') ? $types['igbinary'] : $types['php'];
} elseif (array_key_exists($type, $types)) {
return $types[$type];
}

throw new InvalidConfigurationException(
sprintf(
'%s in not a valid serializer. Valid serializers: %s',
$type,
implode(", ", array_keys($types))
)
);
}

/* Loads the profiler storage configuration.
*
* @param array $config A configuration array
* @param ContainerBuilder $container A ContainerBuilder instance
Expand Down
10 changes: 10 additions & 0 deletions Resources/config/schema/redis-1.0.xsd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,22 @@
<xsd:attribute name="read-write-timeout" type="xsd:int" />
<xsd:attribute name="iterable-multibulk" type="xsd:boolean" />
<xsd:attribute name="throw-errors" type="xsd:boolean" />
<xsd:attribute name="serialization" type="phpredis-serialization-type" />
<xsd:attribute name="profile" type="client-profile" />
<xsd:attribute name="cluster" type="xsd:string" />
<xsd:attribute name="prefix" type="xsd:string" />
<xsd:attribute name="replication" type="xsd:boolean" />
</xsd:complexType>

<xsd:simpleType name="phpredis-serialization-type">
<xsd:restriction base="xsd:string">
<xsd:enumeration value="default" />
<xsd:enumeration value="none" />
<xsd:enumeration value="php" />
<xsd:enumeration value="igbinary" />
</xsd:restriction>
</xsd:simpleType>

<xsd:simpleType name="client-profile">
<xsd:restriction base="xsd:string">
<xsd:enumeration value="1.2" />
Expand Down
46 changes: 46 additions & 0 deletions Tests/DependencyInjection/SncRedisExtensionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,37 @@ public function testClientReplicationOption()
$this->assertEquals(array('snc_redis.default' => array(array('alias' => 'default'))), $container->findTaggedServiceIds('snc_redis.client'));
}

/**
* Test valid config of the serialization option
*/
public function testClientSerializationOption()
{
$extension = new SncRedisExtension();
$config = $this->parseYaml($this->getSerializationYamlConfig());
$extension->load(array($config), $container = $this->getContainer());
$options = $container->getDefinition('snc_redis.client.default_options')->getArgument(0);
$parameters = $container->getDefinition('snc_redis.default')->getArgument(0);
$masterParameters = $container->getDefinition((string) $parameters[0])->getArgument(0);
$this->assertSame($options['serialization'], $masterParameters['serialization']);
}

/**
* Test validity of serialization type
*/
public function testLoadSerializationType()
{
$extension = new SncRedisExtension();
$config = $this->parseYaml($this->getSerializationYamlConfig());
$extension->load(array($config), $container = $this->getContainer());
$options = $container->getDefinition('snc_redis.client.default_options')->getArgument(0);
$serializationType = $extension->loadSerializationType($options['serialization']);
$this->assertTrue(is_integer($serializationType));

$defaultType = defined('Redis::SERIALIZER_IGBINARY') ? 2 : 1;

$this->assertEquals($defaultType, $serializationType);
}

/**
* Test valid config of the sentinel replication option
*/
Expand Down Expand Up @@ -361,6 +392,21 @@ private function parseYaml($yaml)
return $parser->parse($yaml);
}

private function getSerializationYamlConfig()
{
return <<<'EOF'
clients:
default:
type: predis
alias: default
dsn:
- redis://localhost?alias=master
- redis://otherhost
options:
serialization: "default"
EOF;
}

private function getMinimalYamlConfig()
{
return <<<'EOF'
Expand Down