Skip to content
This repository has been archived by the owner on Sep 16, 2021. It is now read-only.

Big config refactoring #36

Closed
wants to merge 4 commits into from
Closed

Big config refactoring #36

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5b07d52
Big config refactoring
wouterj Nov 5, 2013
12d8160
Added fixture configuration
wouterj Nov 5, 2013
41fbd65
Fixed bundle
wouterj Nov 5, 2013
64817fc
Reflected Testing changes
wouterj Nov 11, 2013
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
4 changes: 2 additions & 2 deletions AutoRoute/Factory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ protected function generateRouteStackChain($classFqn)

$routeStackChain = new BuilderUnitChain($this->builder);

foreach ($mapping['content_path'] as $builderName => $builderConfig) {
foreach ($mapping['content_path']['route_stacks'] as $builderName => $builderConfig) {
$builderUnit = $this->generateBuilderUnit($builderConfig);
$routeStackChain->addBuilderUnit($builderName, $builderUnit);
}
Expand Down Expand Up @@ -230,7 +230,7 @@ private function getBuilderService($builderConfig, $type, $aliasKey)
// to be stateless (which is good here)
$service = $this->container->get($serviceId);
unset($builderConfig[$type][$aliasKey]);
$service->init($builderConfig[$type]);
$service->init($builderConfig[$type]['options']);

return $service;
}
Expand Down
8 changes: 6 additions & 2 deletions DependencyInjection/CmfRoutingAutoExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,13 @@ public function load(array $configs, ContainerBuilder $container)
$chainFactoryDef = $container->getDefinition('cmf_routing_auto.factory');

// normalize configuration
foreach ($config['auto_route_mapping'] as $classFqn => $config) {
foreach ($config['auto_route_mappings'] as $classFqn => $config) {
$chainFactoryDef->addMethodCall('registerMapping', array($classFqn, $config));
}
}
}

public function getNamespace()
{
return 'http://cmf.symfony.com/schema/dic/routing_auto';
}
}
152 changes: 82 additions & 70 deletions DependencyInjection/Configuration.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,83 +14,95 @@ class Configuration implements ConfigurationInterface
*/
public function getConfigTreeBuilder()
{
$needsNormalization = function ($v) {
if (!is_array($v)) {
return false;
}

return isset($v['option']);
};
$doNormalization = function ($v) {
$value = array();
foreach ($v['option'] as $option) {
$value[$option['name']] = $option['value'];
}

return $value;
};

$treeBuilder = new TreeBuilder();
$treeBuilder->root('cmf_routing_auto')
->fixXmlConfig('auto_route_mapping')
->children()
->arrayNode('auto_route_mapping')
->useAttributeAsKey('class')
->prototype('array')
->children()
->arrayNode('content_path')
->useAttributeAsKey('name')
->prototype('array')
->children()
->arrayNode('provider')
->beforeNormalization()
->ifTrue($needsNormalization)
->then($doNormalization)
->end()
->prototype('scalar')->end()
->end()
->arrayNode('exists_action')
->beforeNormalization()
->ifTrue($needsNormalization)
->then($doNormalization)
->arrayNode('auto_route_mappings')
->useAttributeAsKey('class')
->prototype('array')
->children()
->arrayNode('content_path')
->fixXmlConfig('route_stack')
->children()
->arrayNode('route_stacks')
->useAttributeAsKey('name')
->prototype('array')
->children()
->arrayNode('provider')
->fixXmlConfig('option')
->children()
->scalarNode('name')->isRequired()->end()
->arrayNode('options')
->useAttributeAsKey('name')
->prototype('scalar')->end()
->end()
->end()
->end() // provider
->arrayNode('exists_action')
->fixXmlConfig('option')
->children()
->scalarNode('strategy')->isRequired()->end()
->arrayNode('options')
->useAttributeAsKey('name')
->prototype('scalar')->end()
->end()
->end()
->end() // exists_action
->arrayNode('not_exists_action')
->fixXmlConfig('option')
->children()
->scalarNode('strategy')->isRequired()->end()
->arrayNode('options')
->useAttributeAsKey('name')
->prototype('scalar')->end()
->end()
->end()
->end() // not_exists_action
->end()
->end()
->end() // route_stacks
->end()
->prototype('scalar')->end()
->end()
->arrayNode('not_exists_action')
->beforeNormalization()
->ifTrue($needsNormalization)
->then($doNormalization)
->end() // content_path
->arrayNode('content_name')
->children()
->arrayNode('provider')
->fixXmlConfig('option')
->children()
->scalarNode('name')->isRequired()->end()
->arrayNode('options')
->useAttributeAsKey('name')
->prototype('scalar')->end()
->end()
->end()
->end() // provider
->arrayNode('exists_action')
->fixXmlConfig('option')
->children()
->scalarNode('strategy')->isRequired()->end()
->arrayNode('options')
->useAttributeAsKey('name')
->prototype('scalar')->end()
->end()
->end()
->end() // exists_action
->arrayNode('not_exists_action')
->fixXmlConfig('option')
->children()
->scalarNode('strategy')->isRequired()->end()
->arrayNode('options')
->useAttributeAsKey('name')
->prototype('scalar')->end()
->end()
->end()
->end() // not_exists_action
->end()
->prototype('scalar')->end()
->end()
->end()
->end()
->end()
->arrayNode('content_name')
->children()
->arrayNode('provider')
->beforeNormalization()
->ifTrue($needsNormalization)
->then($doNormalization)
->end()
->prototype('scalar')->end()
->end()
->arrayNode('exists_action')
->beforeNormalization()
->ifTrue($needsNormalization)
->then($doNormalization)
->end()
->prototype('scalar')->end()
->end()
->arrayNode('not_exists_action')
->beforeNormalization()
->ifTrue($needsNormalization)
->then($doNormalization)
->end()
->prototype('scalar')->end()
->end() // content_name
->end()
->end()
->end()
->end();
->end() // auto_route_mappings
->end()
->end();

return $treeBuilder;
}
Expand Down
88 changes: 49 additions & 39 deletions Tests/Resources/app/config/routingautoroute.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,38 +16,43 @@ cmf_routing:

cmf_routing_auto:

auto_route_mapping:
auto_route_mappings:

##
# e.g. /cms/auto-route/blog/my-blogs-title
Symfony\Cmf\Bundle\RoutingAutoBundle\Tests\Resources\Document\Blog:

# generate or use path components leading up to the final part of the path
content_path:
base:
provider:
name: specified
path: test/auto-route
exists_action:
strategy: use
not_exists_action:
strategy: create
namespace:
provider:
name: specified
path: blog
exists_action:
strategy: use
not_exists_action:
strategy: create
route_stacks:
Copy link
Member

Choose a reason for hiding this comment

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

This is a route stack (singular) - I think

Copy link
Member Author

Choose a reason for hiding this comment

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

no, it's a list of stacks. It's only one stack, yes, but it's still a list. :) Using route_stack would require putting the classname in a node 'class'

Copy link
Member

Choose a reason for hiding this comment

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

Well to be exact its a list of "Build Unit" configurations: https://github.com/symfony-cmf/RoutingAutoBundle/blob/master/AutoRoute/RouteStack/BuilderUnit.php

Copy link
Member Author

Choose a reason for hiding this comment

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

The docs talk about a content_path which has route_stacks, so that's were I took the name from.

Do we want to change it to build_units?

Copy link
Member

Choose a reason for hiding this comment

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

Naming stuff was a real challenge with this bundle. Alot of the logical names were already taken by other CMF things.

BuilderUnits might make more sense, but again I would prefer to wait until after refactoring before changing anything like that.

base:
provider:
name: specified
options:
path: test/auto-route
exists_action:
strategy: use
not_exists_action:
strategy: create
namespace:
provider:
name: specified
options:
path: blog
exists_action:
strategy: use
not_exists_action:
strategy: create

content_name:
provider:
name: content_method
method: getTitle
options:
method: getTitle
exists_action:
strategy: auto_increment
pattern: -%d
options:
pattern: -%d
not_exists_action:
strategy: create

Expand All @@ -57,33 +62,38 @@ cmf_routing_auto:

content_path:

# /cms/auto-route/blog/my-blogs-title
blog_path:
provider:
name: content_object
method: getBlog
exists_action:
strategy: use
not_exists_action:
strategy: throw_exception
route_stacks:
# /cms/auto-route/blog/my-blogs-title
blog_path:
provider:
name: content_object
options:
method: getBlog
exists_action:
strategy: use
not_exists_action:
strategy: throw_exception

date:
provider:
name: content_datetime
method: getDate
date_format: Y/m/d
exists_action:
strategy: use
not_exists_action:
strategy: create
date:
provider:
name: content_datetime
options:
method: getDate
date_format: Y/m/d
exists_action:
strategy: use
not_exists_action:
strategy: create

content_name:
# my-post-title
provider:
name: content_method
method: getTitle
options:
method: getTitle
exists_action:
strategy: auto_increment
pattern: -%d
options:
pattern: -%d
not_exists_action:
strategy: create
34 changes: 22 additions & 12 deletions Tests/Unit/AutoRoute/FactoryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,29 +61,39 @@ public function provideTestGetChain()
array(
array(
'content_path' => array(
'base' => array(
'provider' => array(
'name' => 'fixed',
'message' => 'foobar'
),
'exists_action' => array(
'strategy' => 'create'
),
'not_exists_action' => array(
'strategy' => 'throw_excep',
'route_stacks' => array(
'base' => array(
'provider' => array(
'name' => 'fixed',
'options' => array(
'message' => 'foobar',
),
),
'exists_action' => array(
'strategy' => 'create',
'options' => array(),
),
'not_exists_action' => array(
'strategy' => 'throw_excep',
'options' => array(),
),
),
),
),
'content_name' => array(
'provider' => array(
'name' => 'fixed',
'message' => 'barfoo'
'options' => array(
'message' => 'barfoo',
),
),
'exists_action' => array(
'strategy' => 'create'
'strategy' => 'create',
'options' => array(),
),
'not_exists_action' => array(
'strategy' => 'throw_excep',
'options' => array(),
),
),
),
Expand Down
Loading