Merge pull request #222 from symfony-cmf/cleanup-admin

lsmith77 · lsmith77 · commit 63e9b66cdcd8 · 2014-04-02T13:48:38.000+02:00
adjust sonata admin
diff --git a/Admin/RouteAdmin.php b/Admin/RouteAdmin.php
@@ -59,24 +59,40 @@ protected function configureListFields(ListMapper $listMapper)
     protected function configureFormFields(FormMapper $formMapper)
     {
         $formMapper
-            ->with('form.group_general')
-            ->add(
-                'parent',
-                'doctrine_phpcr_odm_tree',
-                array('choice_list' => array(), 'select_root_node' => true, 'root_node' => $this->routeRoot)
-            )
-            ->add('name', 'text')
+            ->with('form.group_general', array(
+                'translation_domain' => 'CmfRoutingBundle',
+            ))
+                ->add(
+                    'parent',
+                    'doctrine_phpcr_odm_tree',
+                    array('choice_list' => array(), 'select_root_node' => true, 'root_node' => $this->routeRoot)
+                )
+                ->add('name', 'text')
         ->end();
 
         if (null === $this->getParentFieldDescription()) {
             $formMapper
-                ->with('form.group_general')
+                ->with('form.group_general', array(
+                    'translation_domain' => 'CmfRoutingBundle',
+                ))
                     ->add('content', 'doctrine_phpcr_odm_tree', array('choice_list' => array(), 'required' => false, 'root_node' => $this->contentRoot))
                 ->end()
-                ->with('form.group_advanced')
+                ->with('form.group_advanced', array(
+                    'translation_domain' => 'CmfRoutingBundle',
+                ))
                     ->add('variablePattern', 'text', array('required' => false), array('help' => 'form.help_variable_pattern'))
-                    ->add('defaults', 'sonata_type_immutable_array', array('keys' => $this->configureFieldsForDefaults()))
-                    ->add('options', 'sonata_type_immutable_array', array('keys' => $this->configureFieldsForOptions()), array('help' => 'form.help_options'))
+                    ->add(
+                        'defaults',
+                        'sonata_type_immutable_array',
+                        array('keys' => $this->configureFieldsForDefaults($this->getSubject()->getDefaults()))
+                    )
+                    ->add(
+                        'options',
+                        'sonata_type_immutable_array',
+                        array(
+                            'keys' => $this->configureFieldsForOptions($this->getSubject()->getOptions())),
+                        array('help' => 'form.help_options')
+                    )
                 ->end()
             ->end();
         }
@@ -110,7 +126,14 @@ public function getExportFormats()
         return array();
     }
 
-    protected function configureFieldsForDefaults()
+    /**
+     * Provide default route defaults and extract defaults from $dynamicDefaults.
+     *
+     * @param array $dynamicDefaults
+     *
+     * @return array Value for sonata_type_immutable_array
+     */
+    protected function configureFieldsForDefaults($dynamicDefaults)
     {
         $defaults = array(
             '_controller' => array('_controller', 'text', array('required' => false)),
@@ -121,14 +144,13 @@ protected function configureFieldsForDefaults()
             )),
         );
 
-        $dynamicDefaults = $this->getSubject()->getDefaults();
         foreach ($dynamicDefaults as $name => $value) {
             if (!isset($defaults[$name])) {
                 $defaults[$name] = array($name, 'text', array('required' => false));
             }
         }
 
-        //parse variable pattern and add defaults for it - taken from routecompiler
+        //parse variable pattern and add defaults for tokens - taken from routecompiler
         /** @var $route Route */
         $route =  $this->subject;
         if ($route && $route->getVariablePattern()) {
@@ -141,18 +163,31 @@ protected function configureFieldsForDefaults()
             }
         }
 
+        if ($route && $route->getOption('add_format_pattern')) {
+            $defaults['_format'] = array('_format', 'text', array('required' => true));
+        }
+        if ($route && $route->getOption('add_locale_pattern')) {
+            $defaults['_locale'] = array('_format', 'text', array('required' => false));
+        }
+
         return $defaults;
     }
 
-    protected function configureFieldsForOptions()
+    /**
+     * Provide default options and extract options from $dynamicOptions.
+     *
+     * @param array $dynamicOptions
+     *
+     * @return array Value for sonata_type_immutable_array
+     */
+    protected function configureFieldsForOptions(array $dynamicOptions)
     {
         $options = array(
-            array('add_locale_pattern', 'checkbox', array('required' => false, 'label' => 'form.label_add_locale_pattern', 'translation_domain' => $this->translationDomain)),
-            array('add_format_pattern', 'checkbox', array('required' => false, 'label' => 'form.label_add_format_pattern', 'translation_domain' => $this->translationDomain)),
-            array('add_trailing_slash', 'checkbox', array('required' => false, 'label' => 'form.label_add_trailing_slash', 'translation_domain' => $this->translationDomain)),
+            'add_locale_pattern' => array('add_locale_pattern', 'checkbox', array('required' => false, 'label' => 'form.label_add_locale_pattern', 'translation_domain' => 'CmfRoutingBundle')),
+            'add_format_pattern' => array('add_format_pattern', 'checkbox', array('required' => false, 'label' => 'form.label_add_format_pattern', 'translation_domain' => 'CmfRoutingBundle')),
+            'add_trailing_slash' => array('add_trailing_slash', 'checkbox', array('required' => false, 'label' => 'form.label_add_trailing_slash', 'translation_domain' => 'CmfRoutingBundle')),
         );
 
-        $dynamicOptions = $this->getSubject()->getOptions();
         foreach ($dynamicOptions as $name => $value) {
             if (!isset($options[$name])) {
                 $options[$name] = array($name, 'text', array('required' => false));
diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php
@@ -131,7 +131,7 @@ public function getConfigTreeBuilder()
                         ->arrayNode('locales')
                             ->prototype('scalar')->end()
                         ->end()
-                        ->scalarNode('limit_candidates')->defaultValue(20)->end()
+                        ->integerNode('limit_candidates')->defaultValue(20)->end()
                         ->booleanNode('match_implicit_locale')->defaultValue(true)->end()
                         ->booleanNode('auto_locale_pattern')->defaultValue(false)->end()
                     ->end()
diff --git a/Doctrine/Phpcr/PrefixCandidates.php b/Doctrine/Phpcr/PrefixCandidates.php
@@ -11,6 +11,7 @@
 
 namespace Symfony\Cmf\Bundle\RoutingBundle\Doctrine\Phpcr;
 
+use PHPCR\Util\PathHelper;
 use Doctrine\Common\Persistence\ManagerRegistry;
 use Doctrine\ODM\PHPCR\DocumentManager;
 use Doctrine\ODM\PHPCR\Query\Builder\QueryBuilder;
@@ -68,7 +69,9 @@ public function isCandidate($name)
     {
         foreach ($this->getPrefixes() as $prefix) {
             // $name is the route document path
-            if ($name === $prefix || 0 === strpos($name, $prefix . '/')) {
+            if (($name === $prefix || 0 === strpos($name, $prefix . '/'))
+                && PathHelper::assertValidAbsolutePath($name, false, false)
+            ) {
                 return true;
             }
         }
@@ -113,6 +116,13 @@ public function getCandidates(Request $request)
             }
         }
 
+        // filter out things like double // or trailing / - this would trigger an exception on the document manager.
+        foreach ($candidates as $key => $candidate) {
+            if (! PathHelper::assertValidAbsolutePath($candidate, false, false)) {
+                unset($candidates[$key]);
+            }
+        }
+
         return $candidates;
     }
 
diff --git a/Doctrine/Phpcr/RedirectRoute.php b/Doctrine/Phpcr/RedirectRoute.php
@@ -260,4 +260,12 @@ public function getRouteChildren()
 
         return $children;
     }
+
+    /**
+     * {@inheritDoc}
+     */
+    protected function isBooleanOption($name)
+    {
+        return $name === 'add_trailing_slash' || parent::isBooleanOption($name);
+    }
 }
diff --git a/Doctrine/Phpcr/Route.php b/Doctrine/Phpcr/Route.php
@@ -310,4 +310,12 @@ public function getChildren()
     {
         return $this->children;
     }
+
+    /**
+     * {@inheritDoc}
+     */
+    protected function isBooleanOption($name)
+    {
+        return $name === 'add_trailing_slash' || parent::isBooleanOption($name);
+    }
 }
diff --git a/Model/Route.php b/Model/Route.php
@@ -180,6 +180,9 @@ public function getOption($name)
         if (null === $option && 'compiler_class' === $name) {
             return 'Symfony\\Component\\Routing\\RouteCompiler';
         }
+        if ($this->isBooleanOption($name)) {
+            return (boolean) $option;
+        }
 
         return $option;
     }
@@ -196,10 +199,25 @@ public function getOptions()
         if (!array_key_exists('compiler_class', $options)) {
             $options['compiler_class'] = 'Symfony\\Component\\Routing\\RouteCompiler';
         }
+        foreach ($options as $key => $value) {
+            if ($this->isBooleanOption($key)) {
+                $options[$key] = (boolean) $value;
+            }
+        }
 
         return $options;
     }
 
+    /**
+     * Helper method to check if an option is a boolean option to allow better forms.
+     *
+     * @param string $name
+     */
+    protected function isBooleanOption($name)
+    {
+        return in_array($name, array('add_format_pattern', 'add_locale_pattern'));
+    }
+
     /**
      * We need to overwrite this to avoid issues with the legacy code in
      * SymfonyRoute.
diff --git a/Tests/Functional/Doctrine/Phpcr/RouteProviderTest.php b/Tests/Functional/Doctrine/Phpcr/RouteProviderTest.php
@@ -99,11 +99,17 @@ public function testGetRouteCollectionForRequestFormat()
         $this->assertEquals(null, $root->getDefault('_format'));
     }
 
+    /**
+     * The root route will always be found.
+     */
     public function testGetRouteCollectionForRequestNophpcrUrl()
     {
         $collection = $this->repository->getRouteCollectionForRequest(Request::create(':///'));
         $this->assertInstanceOf('Symfony\Component\Routing\RouteCollection', $collection);
-        $this->assertCount(0, $collection);
+        $this->assertCount(1, $collection);
+        $routes = $collection->all();
+        list ($key, $route) = each($routes);
+        $this->assertEquals(self::ROUTE_ROOT, $key);
     }
 
     public function testGetRoutesByNames()
@@ -113,6 +119,7 @@ public function testGetRoutesByNames()
         $routeNames = array(
             self::ROUTE_ROOT . '/testroute/noroute/child',
             self::ROUTE_ROOT . '/testroute/noroute',
+            self::ROUTE_ROOT . '/testroute/', // trailing slash is invalid for phpcr
             self::ROUTE_ROOT . '/testroute'
         );
 
diff --git a/Tests/Unit/DependencyInjection/ConfigurationTest.php b/Tests/Unit/DependencyInjection/ConfigurationTest.php
@@ -71,7 +71,7 @@ public function testSupportsAllConfigFormats()
                 'uri_filter_regexp' => '',
                 'route_filters_by_id' => array(),
                 'locales' => array('en', 'fr'),
-                'limit_candidates' => true,
+                'limit_candidates' => 20,
                 'auto_locale_pattern' => true,
                 'match_implicit_locale' => true,
             ),

Original file line number	Diff line number	Diff line change
`@@ -11,6 +11,7 @@`
`11`	`11`
`12`	`12`	`namespace Symfony\Cmf\Bundle\RoutingBundle\Doctrine\Phpcr;`
`13`	`13`
	`14`	`+use PHPCR\Util\PathHelper;`
`14`	`15`	`use Doctrine\Common\Persistence\ManagerRegistry;`
`15`	`16`	`use Doctrine\ODM\PHPCR\DocumentManager;`
`16`	`17`	`use Doctrine\ODM\PHPCR\Query\Builder\QueryBuilder;`
`@@ -68,7 +69,9 @@ public function isCandidate($name)`
`68`	`69`	`{`
`69`	`70`	`foreach ($this->getPrefixes() as $prefix) {`
`70`	`71`	`// $name is the route document path`
`71`		`- if ($name === $prefix \|\| 0 === strpos($name, $prefix . '/')) {`
	`72`	`+ if (($name === $prefix \|\| 0 === strpos($name, $prefix . '/'))`
	`73`	`+ && PathHelper::assertValidAbsolutePath($name, false, false)`
	`74`	`+ ) {`
`72`	`75`	`return true;`
`73`	`76`	`}`
`74`	`77`	`}`
`@@ -113,6 +116,13 @@ public function getCandidates(Request $request)`
`113`	`116`	`}`
`114`	`117`	`}`
`115`	`118`
	`119`	`+ // filter out things like double // or trailing / - this would trigger an exception on the document manager.`
	`120`	`+ foreach ($candidates as $key => $candidate) {`
	`121`	`+ if (! PathHelper::assertValidAbsolutePath($candidate, false, false)) {`
	`122`	`+ unset($candidates[$key]);`
	`123`	`+ }`
	`124`	`+ }`
	`125`	`+`
`116`	`126`	`return $candidates;`
`117`	`127`	`}`
`118`	`128`
Original file line number	Diff line number	Diff line change
`@@ -260,4 +260,12 @@ public function getRouteChildren()`
`260`	`260`
`261`	`261`	`return $children;`
`262`	`262`	`}`
	`263`	`+`
	`264`	`+ /**`
	`265`	`+ * {@inheritDoc}`
	`266`	`+ */`
	`267`	`+ protected function isBooleanOption($name)`
	`268`	`+ {`
	`269`	`+ return $name === 'add_trailing_slash' \|\| parent::isBooleanOption($name);`
	`270`	`+ }`
`263`	`271`	`}`
Original file line number	Diff line number	Diff line change
`@@ -310,4 +310,12 @@ public function getChildren()`
`310`	`310`	`{`
`311`	`311`	`return $this->children;`
`312`	`312`	`}`
	`313`	`+`
	`314`	`+ /**`
	`315`	`+ * {@inheritDoc}`
	`316`	`+ */`
	`317`	`+ protected function isBooleanOption($name)`
	`318`	`+ {`
	`319`	`+ return $name === 'add_trailing_slash' \|\| parent::isBooleanOption($name);`
	`320`	`+ }`
`313`	`321`	`}`