Skip to content

Commit d193630

Browse files
aspregaasprega
authored
Merge pull request #17 from slope-it/8-better-url-generation
2 parents e280dcd + 909143f commit d193630

18 files changed

+222
-74
lines changed

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
/.idea
22
/vendor
3+
/var
34
phpunit.xml
45
composer.lock
56
.phpunit.result.cache

‎README.md‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,10 @@ class AppKernel extends Kernel
3636
{
3737
public function registerBundles()
3838
{
39-
$bundles = array(
39+
$bundles = [
4040
// ...
4141
new SlopeIt\BreadcrumbBundle\SlopeItBreadcrumbBundle(),
42-
);
42+
];
4343
// ...
4444
}
4545
// ...
@@ -209,7 +209,3 @@ However, in your template you'll just have to iterate over the `items` variable
209209
* Do you think documentation can be improved?
210210

211211
Under any of these circumstances, please fork this repo and create a pull request. We are more than happy to accept contributions!
212-
213-
## Maintainer
214-
215-
[@andreasprega](https://twitter.com/andreasprega)

‎composer.json‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,15 @@
2323
"doctrine/annotations": "^1.0",
2424
"symfony/framework-bundle": "^4.0|^5.0",
2525
"symfony/property-access": "^4.0|^5.0",
26-
"symfony/translation-contracts": "^1.0|^2.0",
26+
"symfony/translation": "^4.0|^5.0",
2727
"symfony/yaml": "^4.0|^5.0",
2828
"twig/twig": "^2.10|^3.0"
2929
},
3030
"require-dev": {
31-
"mockery/mockery": "1.4.3",
32-
"phpunit/phpunit": "8.5.15"
31+
"mockery/mockery": "^1.3",
32+
"phpunit/phpunit": "8.5.15",
33+
"symfony/dependency-injection": "^4.0|^5.0",
34+
"symfony/monolog-bundle": "^3.7"
3335
},
3436
"autoload": {
3537
"psr-4": {

‎src/Service/BreadcrumbItemProcessor.php‎

Lines changed: 51 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
use SlopeIt\BreadcrumbBundle\Model\ProcessedBreadcrumbItem;
77
use Symfony\Component\HttpFoundation\RequestStack;
88
use Symfony\Component\PropertyAccess\PropertyAccessorInterface;
9-
use Symfony\Component\Routing\RouterInterface;
9+
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
1010
use Symfony\Contracts\Translation\TranslatorInterface;
1111

1212
/**
@@ -26,9 +26,9 @@ class BreadcrumbItemProcessor
2626
private $requestStack;
2727

2828
/**
29-
* @var RouterInterface
29+
* @var UrlGeneratorInterface
3030
*/
31-
private $router;
31+
private $urlGenerator;
3232

3333
/**
3434
* @var TranslatorInterface
@@ -38,19 +38,55 @@ class BreadcrumbItemProcessor
3838
public function __construct(
3939
PropertyAccessorInterface $propertyAccessor,
4040
TranslatorInterface $translator,
41-
RouterInterface $router,
41+
UrlGeneratorInterface $urlGenerator,
4242
RequestStack $requestStack
4343
) {
4444
$this->propertyAccessor = $propertyAccessor;
4545
$this->translator = $translator;
46-
$this->router = $router;
46+
$this->urlGenerator = $urlGenerator;
4747
$this->requestStack = $requestStack;
4848
}
4949

5050
/**
51+
* @param BreadcrumbItem[] $items
5152
* @param array<string, mixed> $variables
53+
* @return ProcessedBreadcrumbItem[]
5254
*/
53-
public function process(BreadcrumbItem $item, array $variables): ProcessedBreadcrumbItem
55+
public function process(array $items, array $variables): array
56+
{
57+
$requestContext = $this->urlGenerator->getContext();
58+
59+
// Save original parameters of the request context, so they can be re-applied after url generation is done.
60+
$originalRequestContextParameters = $requestContext->getParameters();
61+
62+
// Use the request context to pre-set parameters that are already present as attributes of the current request.
63+
// This allows for URL generation using parameters already present in the request path without having to
64+
// re-specify them all the time. These parameters won't be added unnecessarily if they are not needed for the
65+
// route being generated.
66+
foreach ($this->requestStack->getCurrentRequest()->attributes as $key => $value) {
67+
if ($key[0] !== '_') {
68+
$requestContext->setParameter($key, $value);
69+
}
70+
}
71+
72+
// Process items with our own "modified" request context
73+
$processedItems = array_map(
74+
function (BreadcrumbItem $item) use ($variables) {
75+
return $this->processItem($item, $variables);
76+
},
77+
$items
78+
);
79+
80+
// Restore parameters originally present in the context so that this method doesn't have any side effects.
81+
$this->urlGenerator->getContext()->setParameters($originalRequestContextParameters);
82+
83+
return $processedItems;
84+
}
85+
86+
/**
87+
* @param array<string, mixed> $variables
88+
*/
89+
private function processItem(BreadcrumbItem $item, array $variables): ProcessedBreadcrumbItem
5490
{
5591
// Process the label
5692
if ($item->getLabel() && $item->getLabel()[0] === '$') {
@@ -62,23 +98,17 @@ public function process(BreadcrumbItem $item, array $variables): ProcessedBreadc
6298
}
6399

64100
// Process the route
65-
// TODO: cache parameters extracted from current request
66-
$params = [];
67-
foreach ($this->requestStack->getCurrentRequest()->attributes as $key => $value) {
68-
if ($key[0] !== '_') {
69-
$params[$key] = $value;
70-
}
71-
}
72-
foreach ($item->getRouteParams() ?: [] as $key => $value) {
73-
if (is_string($value) && $value[0] === '$') {
74-
$params[$key] = $this->parseValue($value, $variables);
75-
} else {
76-
$params[$key] = $value;
101+
if ($item->getRoute() !== null) {
102+
$params = [];
103+
foreach ($item->getRouteParams() ?: [] as $key => $value) {
104+
if (is_string($value) && $value[0] === '$') {
105+
$params[$key] = $this->parseValue($value, $variables);
106+
} else {
107+
$params[$key] = $value;
108+
}
77109
}
78-
}
79110

80-
if ($item->getRoute() !== null) {
81-
$processedUrl = $this->router->generate($item->getRoute(), $params);
111+
$processedUrl = $this->urlGenerator->generate($item->getRoute(), $params);
82112
} else {
83113
$processedUrl = null;
84114
}

‎src/Twig/BreadcrumbExtension.php‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,11 @@ public function isBreadcrumbEmpty(): bool
6767

6868
public function renderBreadcrumb(Environment $twig, array $context): string
6969
{
70-
$breadcrumb = [];
71-
foreach ($this->builder->getItems() as $item) {
72-
$breadcrumb[] = $this->itemProcessor->process($item, $context);
73-
}
74-
75-
return $twig->render($this->template, [ 'items' => $breadcrumb ]);
70+
return $twig->render(
71+
$this->template,
72+
[
73+
'items' => $this->itemProcessor->process($this->builder->getItems(), $context)
74+
]
75+
);
7676
}
7777
}

‎tests/Fixtures/TestKernel.php‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
<?php
2+
declare(strict_types=1);
3+
4+
namespace SlopeIt\Tests\BreadcrumbBundle\Fixtures;
5+
6+
use SlopeIt\BreadcrumbBundle\SlopeItBreadcrumbBundle;
7+
use Symfony\Bundle\FrameworkBundle\FrameworkBundle;
8+
use Symfony\Bundle\MonologBundle\MonologBundle;
9+
use Symfony\Component\Config\Loader\LoaderInterface;
10+
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
11+
use Symfony\Component\DependencyInjection\ContainerBuilder;
12+
use Symfony\Component\HttpKernel\Kernel;
13+
14+
class TestKernel extends Kernel
15+
{
16+
public function registerBundles(): iterable
17+
{
18+
return [
19+
new MonologBundle(),
20+
new FrameworkBundle(),
21+
new SlopeItBreadcrumbBundle(),
22+
];
23+
}
24+
25+
public function build(ContainerBuilder $container)
26+
{
27+
$container->addCompilerPass(new class() implements CompilerPassInterface {
28+
public function process(ContainerBuilder $container): void
29+
{
30+
foreach ($container->getDefinitions() as $definition) {
31+
$definition->setPublic(true);
32+
}
33+
}
34+
});
35+
}
36+
37+
public function registerContainerConfiguration(LoaderInterface $loader)
38+
{
39+
$loader->load(__DIR__.'/config/config.yml');
40+
}
41+
}

‎tests/Fixtures/config/config.yml‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
framework:
2+
secret: 'NOT-SO-SECRET'
3+
test: true
4+
router:
5+
resource: "%kernel.project_dir%/tests/Fixtures/config/routing.yml"
6+
translator:
7+
logging: true
8+
cache: true
9+
annotations:
10+
enabled: true
11+
cache: file

‎tests/Fixtures/config/routing.yml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
parent_route:
2+
path: /parent-path/{component1}/{component2}
3+
4+
child_route:
5+
path: /parent-path/{component1}/child-path/{component3}

‎tests/Integration/Service/BreadcrumbItemProcessorTest.php‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
<?php
2+
declare(strict_types=1);
3+
4+
namespace SlopeIt\Tests\BreadcrumbBundle\Integration\Service;
5+
6+
use SlopeIt\BreadcrumbBundle\Model\BreadcrumbItem;
7+
use SlopeIt\BreadcrumbBundle\Service\BreadcrumbItemProcessor;
8+
use SlopeIt\Tests\BreadcrumbBundle\Fixtures\TestKernel;
9+
use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
10+
use Symfony\Component\HttpFoundation\Request;
11+
use Symfony\Component\HttpFoundation\RequestStack;
12+
use Symfony\Component\HttpKernel\Event\RequestEvent;
13+
use Symfony\Component\HttpKernel\HttpKernelInterface;
14+
15+
class BreadcrumbItemProcessorTest extends KernelTestCase
16+
{
17+
protected static function getKernelClass()
18+
{
19+
return TestKernel::class;
20+
}
21+
22+
public function test_process_item_with_reused_path_parameters()
23+
{
24+
// Preconditions
25+
// NOTE: see `tests/Fixtures/config/routing.yml`
26+
$request = Request::create('http://localhost/parent-path/foo/bar');
27+
28+
// This is done by the HttpKernel when a request is sent
29+
$requestStack = self::getContainer()->get(RequestStack::class);
30+
$requestStack->push($request);
31+
32+
// This framework listener is responsible for setting path parameters as request attributes
33+
$listener = self::getContainer()->get('router_listener');
34+
$listener->onKernelRequest(new RequestEvent(self::$kernel, $request, HttpKernelInterface::MAIN_REQUEST));
35+
36+
// Action
37+
/** @var BreadcrumbItemProcessor $SUT */
38+
$SUT = self::getContainer()->get('slope_it.breadcrumb.item_processor');
39+
$processedBreadcrumbItems = $SUT->process(
40+
[
41+
new BreadcrumbItem('Parent page', 'parent_route'),
42+
new BreadcrumbItem('Child page', 'child_route', ['component3' => 'baz']),
43+
],
44+
[]
45+
);
46+
47+
// Verification: url of parent route was reconstructed as is, without needing to specify "component1" and
48+
// "component2" because already present.
49+
$this->assertSame('/parent-path/foo/bar', $processedBreadcrumbItems[0]->getUrl());
50+
51+
// Verification: url of child route was constructed by implicitly reusing "component1" from current, parent path
52+
// "component3" was explicitly provided in the breadcrum item.
53+
// "component2" was ignored as not present in child path.
54+
$this->assertSame('/parent-path/foo/child-path/baz', $processedBreadcrumbItems[1]->getUrl());
55+
}
56+
}

tests/Annotation/BreadcrumbTest.php renamed to tests/Unit/Annotation/BreadcrumbTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
<?php
22

3-
namespace SlopeIt\Tests\BreadcrumbBundle\Annotation;
3+
namespace SlopeIt\Tests\BreadcrumbBundle\Unit\Annotation;
44

5-
use SlopeIt\BreadcrumbBundle\Annotation\Breadcrumb;
65
use Mockery\Adapter\Phpunit\MockeryPHPUnitIntegration;
76
use PHPUnit\Framework\TestCase;
7+
use SlopeIt\BreadcrumbBundle\Annotation\Breadcrumb;
88

99
/**
1010
* @coversDefaultClass \SlopeIt\BreadcrumbBundle\Annotation\Breadcrumb

0 commit comments

Comments
 (0)