Skip to content

expand configuration definition and adjust tests, closes #151 #155

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

Merged
merged 1 commit into from
May 2, 2017
Merged

expand configuration definition and adjust tests, closes #151 #155

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
182 changes: 119 additions & 63 deletions DependencyInjection/Configuration.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@

namespace Http\HttplugBundle\DependencyInjection;

use Http\Client\Common\Plugin\Cache\Generator\CacheKeyGenerator;
use Http\Client\Common\Plugin\Journal;
use Http\Message\CookieJar;
use Http\Message\Formatter;
use Http\Message\StreamFactory;
use Psr\Cache\CacheItemPoolInterface;
use Psr\Log\LoggerInterface;
use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
use Symfony\Component\Config\Definition\Builder\NodeDefinition;
use Symfony\Component\Config\Definition\Builder\TreeBuilder;
Expand Down Expand Up @@ -352,69 +359,33 @@ private function addSharedPluginNodes(ArrayNodeDefinition $pluginNode, $disableA
$children = $pluginNode->children();

$children->append($this->createAuthenticationPluginNode());
$children->append($this->createCachePluginNode());

$children->arrayNode('cache')
->canBeEnabled()
->addDefaultsIfNotSet()
$children
->arrayNode('cookie')
->canBeEnabled()
->children()
->scalarNode('cache_pool')
->info('This must be a service id to a service implementing Psr\Cache\CacheItemPoolInterface')
->scalarNode('cookie_jar')
->info('This must be a service id to a service implementing '.CookieJar::class)
->isRequired()
->cannotBeEmpty()
->end()
->scalarNode('stream_factory')
->info('This must be a service id to a service implementing Http\Message\StreamFactory')
->defaultValue('httplug.stream_factory')
->cannotBeEmpty()
->end()
->arrayNode('config')
->addDefaultsIfNotSet()
->validate()
->ifTrue(function ($config) {
// Cannot set both respect_cache_headers and respect_response_cache_directives
return isset($config['respect_cache_headers'], $config['respect_response_cache_directives']);
})
->thenInvalid('You can\'t provide config option "respect_cache_headers" and "respect_response_cache_directives" simultaniously. Use "respect_response_cache_directives" instead.')
->end()
->children()
->scalarNode('default_ttl')->defaultValue(0)->end()
->enumNode('respect_cache_headers')
->values([null, true, false])
->beforeNormalization()
->always(function ($v) {
@trigger_error('The option "respect_cache_headers" is deprecated since version 1.3 and will be removed in 2.0. Use "respect_response_cache_directives" instead.', E_USER_DEPRECATED);

return $v;
})
->end()
->end()
->variableNode('respect_response_cache_directives')
->validate()
->always(function ($v) {
if (is_null($v) || is_array($v)) {
return $v;
}
throw new InvalidTypeException();
})
->end()
->end()
->end()
->end()
->end()
->end();
// End cache plugin
// End cookie plugin

$children->arrayNode('cookie')
->canBeEnabled()
$children
->arrayNode('history')
->canBeEnabled()
->children()
->scalarNode('cookie_jar')
->info('This must be a service id to a service implementing Http\Message\CookieJar')
->scalarNode('journal')
->info('This must be a service id to a service implementing '.Journal::class)
->isRequired()
->cannotBeEmpty()
->end()
->end()
->end();
// End cookie plugin
// End history plugin

$decoder = $children->arrayNode('decoder');
$disableAll ? $decoder->canBeEnabled() : $decoder->canBeDisabled();
Expand All @@ -425,29 +396,17 @@ private function addSharedPluginNodes(ArrayNodeDefinition $pluginNode, $disableA
->end();
// End decoder plugin

$children->arrayNode('history')
->canBeEnabled()
->children()
->scalarNode('journal')
->info('This must be a service id to a service implementing Http\Client\Plugin\Journal')
->isRequired()
->cannotBeEmpty()
->end()
->end()
->end();
// End history plugin

$logger = $children->arrayNode('logger');
$disableAll ? $logger->canBeEnabled() : $logger->canBeDisabled();
$logger->addDefaultsIfNotSet()
->children()
->scalarNode('logger')
->info('This must be a service id to a service implementing Psr\Log\LoggerInterface')
->info('This must be a service id to a service implementing '.LoggerInterface::class)
->defaultValue('logger')
->cannotBeEmpty()
->end()
->scalarNode('formatter')
->info('This must be a service id to a service implementing Http\Message\Formatter')
->info('This must be a service id to a service implementing '.Formatter::class)
->defaultNull()
->end()
->end()
Expand Down Expand Up @@ -564,4 +523,101 @@ private function validateAuthenticationType(array $expected, array $actual, $aut
implode(', ', $actual)
));
}

/**
* Create configuration for cache plugin.
*
* @return NodeDefinition Definition for the cache node in the plugins list.
*/
private function createCachePluginNode()
{
$builder = new TreeBuilder();

$config = $builder->root('config');
$config
->fixXmlConfig('method')
Copy link
Contributor Author

@alcohol alcohol Apr 28, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@stof Should this be considered a BC btw?

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, as there is no XSD file, the existing XML config file will continue to work, for people lucky enough to have a working config before (it was broken when having a single method). And it will allow everyone to have a working config when using XML, for all cases.

Btw, it cannot be considered a BC. The sentence should be considered a BC break. The important work in this sentence is break 😄

->fixXmlConfig('respect_response_cache_directive')
->addDefaultsIfNotSet()
->validate()
->ifTrue(function ($config) {
// Cannot set both respect_cache_headers and respect_response_cache_directives
return isset($config['respect_cache_headers'], $config['respect_response_cache_directives']);
})
->thenInvalid('You can\'t provide config option "respect_cache_headers" and "respect_response_cache_directives" simultaniously. Use "respect_response_cache_directives" instead.')
->end()
->children()
->scalarNode('cache_key_generator')
->info('This must be a service id to a service implementing '.CacheKeyGenerator::class)
->end()
->integerNode('cache_lifetime')
->info('The minimum time we should store a cache item')
->end()
->integerNode('default_ttl')
->info('The default max age of a Response')
->end()
->enumNode('hash_algo')
->info('Hashing algorithm to use')
->values(hash_algos())
->cannotBeEmpty()
->end()
->arrayNode('methods')
->info('Which request methods to cache')
->defaultValue(['GET', 'HEAD'])
->prototype('scalar')
->validate()
->ifTrue(function ($v) {
/* RFC7230 sections 3.1.1 and 3.2.6 except limited to uppercase characters. */
return preg_match('/[^A-Z0-9!#$%&\'*+\-.^_`|~]+/', $v);
})
->thenInvalid('Invalid method: %s')
->end()
->end()
->end()
->enumNode('respect_cache_headers')
->info('Whether we should care about cache headers or not [DEPRECATED]')
->values([null, true, false])
->beforeNormalization()
->always(function ($v) {
@trigger_error('The option "respect_cache_headers" is deprecated since version 1.3 and will be removed in 2.0. Use "respect_response_cache_directives" instead.', E_USER_DEPRECATED);

return $v;
})
->end()
->end()
->variableNode('respect_response_cache_directives')
->info('A list of cache directives to respect when caching responses')
->validate()
->always(function ($v) {
if (is_null($v) || is_array($v)) {
return $v;
}

throw new InvalidTypeException();
})
->end()
->end()
->end()
;

$cache = $builder->root('cache');
$cache
->canBeEnabled()
->addDefaultsIfNotSet()
->children()
->scalarNode('cache_pool')
->info('This must be a service id to a service implementing '.CacheItemPoolInterface::class)
->isRequired()
->cannotBeEmpty()
->end()
->scalarNode('stream_factory')
->info('This must be a service id to a service implementing '.StreamFactory::class)
->defaultValue('httplug.stream_factory')
->cannotBeEmpty()
->end()
->end()
->append($config)
;

return $cache;
}
}
6 changes: 5 additions & 1 deletion Tests/Resources/Fixtures/config/full.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,12 @@
'cache_pool' => 'my_cache_pool',
'stream_factory' => 'my_other_stream_factory',
'config' => [
'cache_lifetime' => 2592000,
'default_ttl' => 42,
'respect_response_cache_directives' => ['X-Foo', 'X-Bar'],
'hash_algo' => 'sha1',
'methods' => ['GET'],
'cache_key_generator' => null,
'respect_response_cache_directives' => ['X-Foo'],
],
],
'cookie' => [
Expand Down
6 changes: 3 additions & 3 deletions Tests/Resources/Fixtures/config/full.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@
<my_service type="service" service="my_auth_service"/>
</authentication>
<cache cache-pool="my_cache_pool" stream-factory="my_other_stream_factory">
<config default-ttl="42">
<respect-response-cache-directives>X-Foo</respect-response-cache-directives>
<respect-response-cache-directives>X-Bar</respect-response-cache-directives>
<config default-ttl="42" cache-lifetime="2592000" hash-algo="sha1" cache-key-generator="null">
<respect-response-cache-directive>X-Foo</respect-response-cache-directive>
<method>GET</method>
</config>
</cache>
<cookie cookie-jar="my_cookie_jar"/>
Expand Down
6 changes: 5 additions & 1 deletion Tests/Resources/Fixtures/config/full.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,14 @@ httplug:
cache_pool: my_cache_pool
stream_factory: my_other_stream_factory
config:
cache_lifetime: 2592000
default_ttl: 42
hash_algo: sha1
methods:
- GET
cache_key_generator: null
respect_response_cache_directives:
- X-Foo
- X-Bar
cookie:
cookie_jar: my_cookie_jar
decoder:
Expand Down
10 changes: 7 additions & 3 deletions Tests/Unit/DependencyInjection/ConfigurationTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class ConfigurationTest extends AbstractExtensionConfigurationTestCase
'enabled' => false,
'stream_factory' => 'httplug.stream_factory',
'config' => [
'default_ttl' => 0,
'methods' => ['GET', 'HEAD'],
],
],
'cookie' => [
Expand Down Expand Up @@ -194,8 +194,12 @@ public function testSupportsAllConfigFormats()
'cache_pool' => 'my_cache_pool',
'stream_factory' => 'my_other_stream_factory',
'config' => [
'cache_lifetime' => 2592000,
'default_ttl' => 42,
'respect_response_cache_directives' => ['X-Foo', 'X-Bar'],
'hash_algo' => 'sha1',
'methods' => ['GET'],
'cache_key_generator' => null,
'respect_response_cache_directives' => ['X-Foo'],
],
],
'cookie' => [
Expand Down Expand Up @@ -317,7 +321,7 @@ public function testCacheConfigDeprecationCompatibility()
'enabled' => true,
'cache_pool' => 'my_cache_pool',
'config' => [
'default_ttl' => 0,
'methods' => ['GET', 'HEAD'],
'respect_cache_headers' => true,
],
]);
Expand Down