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.

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

Added child restriction to Route & RedirectRoute documents #389

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
Changelog
=========

* **2017-02-09**: [BC BREAK] Added child restrictions to the `Route` and `RedirectRoute` documents.
See the UPGRADE guide for detailed information.
* **2017-02-03**: [BC BREAK] Removed unused `cmf_routing.dynamic.persistence.phpcr.content_basepath`

2.0.0-RC1
Expand Down
10 changes: 10 additions & 0 deletions UPGRADE-2.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,16 @@
$route->setParentDocument($routeRoot);
```

## Doctrine PHPCR ODM

* It is no longer possible to add a child to the `RedirectRoute` document.
This behaviour can be changed by overriding the `child-class` setting of the
PHPCR ODM mapping.

* Only instances of `RouteObjectInterface` are allowed as children of the
`Route` document. This behaviour can be changed by overriding the
`child-class` setting of the PHPCR ODM mapping.

## Configuration

* Removed the `route_basepath` setting, use `route_basepaths` instead.
Expand Down
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
},
"require-dev": {
"symfony-cmf/testing": "^2.0",
"doctrine/phpcr-odm": "^1.4|^2.0",
"doctrine/phpcr-odm": "^1.4.2",
"symfony/phpunit-bridge": "^3.2",
"matthiasnoback/symfony-dependency-injection-test": "~0.6",
"matthiasnoback/symfony-config-test": "^1.3.1",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
https://github.com/doctrine/phpcr-odm/raw/master/doctrine-phpcr-odm-mapping.xsd"
>

<document name="Symfony\Cmf\Bundle\RoutingBundle\Doctrine\Phpcr\RedirectRoute">
<document name="Symfony\Cmf\Bundle\RoutingBundle\Doctrine\Phpcr\RedirectRoute" is-leaf="true">
Copy link
Member

Choose a reason for hiding this comment

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

i disagree with this. we could have an intermediate node redirect to the default child node. as in /topics redirecting to /topics/symfony (ok, not the best example but i think there are valid use cases like this)

<id name="id">
<generator strategy="PARENT"/>
</id>
Expand Down
1 change: 1 addition & 0 deletions src/Resources/config/doctrine-phpcr/Route.phpcr.xml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
<nodename name="name"/>
<parent-document name="parent"/>
<children name="children"/>
<child-class name="Symfony\Cmf\Component\Routing\RouteObjectInterface"/>
Copy link
Member Author

Choose a reason for hiding this comment

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

We have a couple of options here:

  • Restrict to this interface
  • Restrict to Symfony\Component\Routing\Route
  • Restrict to the Route and RedirectRoute models of this bundle

I choose the first option, as it seems to be the most flexible without completely allowing anything.

</document>

</doctrine-mapping>
24 changes: 23 additions & 1 deletion tests/Functional/Doctrine/Phpcr/RedirectRouteTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\Cmf\Bundle\RoutingBundle\Tests\Functional\Doctrine\Phpcr;

use Doctrine\ODM\PHPCR\Document\Generic;
use Symfony\Cmf\Bundle\RoutingBundle\Doctrine\Phpcr\RedirectRoute;
use Symfony\Cmf\Bundle\RoutingBundle\Doctrine\Phpcr\Route;
use Symfony\Cmf\Bundle\RoutingBundle\Tests\Functional\BaseTestCase;
Expand All @@ -21,7 +22,7 @@ class RedirectRouteTest extends BaseTestCase
{
const ROUTE_ROOT = '/test/redirectroute';

public function setUp()
protected function setUp()
{
parent::setUp();
$this->db('PHPCR')->createTestNode();
Expand Down Expand Up @@ -59,6 +60,27 @@ public function testRedirectDoctrine()
$this->assertEquals(['test' => 'toast'], $defaults);
}

/**
* @expectedException \Doctrine\ODM\PHPCR\Exception\OutOfBoundsException
* @expectedExceptionMessage It cannot have children
*/
public function testPersistChild()
{
$root = $this->getDm()->find(null, self::ROUTE_ROOT);

$redirect = new RedirectRoute();
$redirect->setPosition($root, 'redirect');
$redirect->setDefault('test', 'toast');
$this->getDm()->persist($redirect);

$child = new Generic();
$child->setParentDocument($redirect);
$child->setNodename('foo');
$this->getDm()->persist($child);

$this->getDm()->flush();
}

/**
* @expectedException \LogicException
*/
Expand Down
2 changes: 1 addition & 1 deletion tests/Functional/Doctrine/Phpcr/RouteProviderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class RouteProviderTest extends BaseTestCase
/** @var RouteProvider */
private $repository;

public function setUp()
protected function setUp()
{
parent::setUp();
$this->db('PHPCR')->createTestNode();
Expand Down
19 changes: 18 additions & 1 deletion tests/Functional/Doctrine/Phpcr/RouteTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,16 @@

namespace Symfony\Cmf\Bundle\RoutingBundle\Tests\Functional\Doctrine\Phpcr;

use Doctrine\ODM\PHPCR\Document\Generic;
use Doctrine\ODM\PHPCR\Exception\OutOfBoundsException;
use Symfony\Cmf\Bundle\RoutingBundle\Doctrine\Phpcr\Route;
use Symfony\Cmf\Bundle\RoutingBundle\Tests\Functional\BaseTestCase;

class RouteTest extends BaseTestCase
{
const ROUTE_ROOT = '/test/routing';

public function setUp()
protected function setUp()
{
parent::setUp();
$this->db('PHPCR')->createTestNode();
Expand Down Expand Up @@ -89,6 +91,21 @@ public function testPersistEmptyOptions()
return $route;
}

/**
* @expectedException \Doctrine\ODM\PHPCR\Exception\OutOfBoundsException
*/
public function testPersistInvalidChild()
{
$root = $this->getDm()->find(null, self::ROUTE_ROOT);

$document = new Generic();
$document->setParentDocument($root);
$document->setNodeName('foo');

$this->getDm()->persist($document);
$this->getDm()->flush();
}

public function testConditionOption()
{
$route = new Route();
Expand Down