Skip to content

Re-adding the ability to add a resource to the RouteCollectionBuilder #16361

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

Merged
merged 1 commit into from
Nov 5, 2015
Merged
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
20 changes: 20 additions & 0 deletions src/Symfony/Component/Routing/RouteCollectionBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

use Symfony\Component\Config\Exception\FileLoaderLoadException;
use Symfony\Component\Config\Loader\LoaderInterface;
use Symfony\Component\Config\Resource\ResourceInterface;

/**
* Helps add and import routes into a RouteCollection.
Expand All @@ -35,6 +36,7 @@ class RouteCollectionBuilder
private $options = array();
private $schemes;
private $methods;
private $resources = array();

/**
* @param LoaderInterface $loader
Expand Down Expand Up @@ -237,6 +239,20 @@ public function setMethods($methods)
return $this;
}

/**
* Adds a resource for this collection.
*
* @param ResourceInterface $resource
*
* @return $this
*/
private function addResource(ResourceInterface $resource)
Copy link
Member

Choose a reason for hiding this comment

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

private methods should come after public ones.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, shouldn't this method be public? Otherwise I fail to see how $resources should ever be changed.

Copy link
Member Author

Choose a reason for hiding this comment

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

It can be private. As odd as it sees, since this method is being called from inside RouteCollectionBuilder (even though it's not being called on $this, but another object), it is legal to call a private method. This is done in Silex in several places. However, I would be quite happy if this were public. I think (but don't really know) that people may eventually ask for this method if it's not present.

Copy link
Member Author

Choose a reason for hiding this comment

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

tl;dr This code actually works. But maybe we should make it public :)

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I missed that call :)

Copy link
Member

Choose a reason for hiding this comment

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

private is fine

{
$this->resources[] = $resource;

return $this;
}

/**
* Creates the final RouteCollection and returns it.
*
Expand Down Expand Up @@ -291,6 +307,10 @@ public function build()

$routeCollection->addCollection($subCollection);
}

foreach ($this->resources as $resource) {
$routeCollection->addResource($resource);
}
}

return $routeCollection;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\Component\Routing\Tests;

use Symfony\Component\Config\Resource\FileResource;
use Symfony\Component\Routing\Route;
use Symfony\Component\Routing\RouteCollection;
use Symfony\Component\Routing\RouteCollectionBuilder;
Expand All @@ -29,6 +30,7 @@ public function testImport()
$originalRoute = new Route('/foo/path');
$expectedCollection = new RouteCollection();
$expectedCollection->add('one_test_route', $originalRoute);
$expectedCollection->addResource(new FileResource('file_resource.yml'));

$resolvedLoader
->expects($this->once())
Expand All @@ -52,6 +54,8 @@ public function testImport()
$addedCollection = $importedRoutes->build();
$route = $addedCollection->get('one_test_route');
$this->assertSame($originalRoute, $route);
// should return file_resource.yml, which is in the original collection
$this->assertCount(1, $addedCollection->getResources());
}

/**
Expand Down