From 51af45001f5969b1a71300c992c289a8b8c65e0f Mon Sep 17 00:00:00 2001 From: zelu Date: Thu, 12 Oct 2017 10:21:43 +0200 Subject: [PATCH 1/5] Return 404 when user visits grid page which does not exist --- .../Request/DataGridSetDataHandlerSpec.php | 16 ++++++++++ .../Component/DataSource/DataSource.php | 20 +++++++++++++ .../Exception/PageNotFoundException.php | 9 ++++++ .../Request/DataGridSetDataHandler.php | 9 +++++- .../DataSource/Tests/DataSourceTest.php | 30 +++++++++++++++++++ .../Driver/Doctrine/DoctrineDriverTest.php | 3 +- .../Doctrine/ORM/DoctrineDriverTest.php | 3 +- .../Extension/Symfony/FormExtensionTest.php | 4 +-- 8 files changed, 89 insertions(+), 5 deletions(-) create mode 100644 src/AdminPanel/Component/DataSource/Exception/PageNotFoundException.php diff --git a/spec/AdminPanel/Symfony/AdminBundle/Admin/CRUD/Context/Request/DataGridSetDataHandlerSpec.php b/spec/AdminPanel/Symfony/AdminBundle/Admin/CRUD/Context/Request/DataGridSetDataHandlerSpec.php index 6acdf00..7dfa360 100644 --- a/spec/AdminPanel/Symfony/AdminBundle/Admin/CRUD/Context/Request/DataGridSetDataHandlerSpec.php +++ b/spec/AdminPanel/Symfony/AdminBundle/Admin/CRUD/Context/Request/DataGridSetDataHandlerSpec.php @@ -5,6 +5,7 @@ namespace spec\AdminPanel\Symfony\AdminBundle\Admin\CRUD\Context\Request; use AdminPanel\Component\DataSource\DataSource; +use AdminPanel\Component\DataSource\Exception\PageNotFoundException; use AdminPanel\Symfony\AdminBundle\Event\AdminEvent; use AdminPanel\Symfony\AdminBundle\Event\ListEvent; use AdminPanel\Symfony\AdminBundle\Event\ListEvents; @@ -14,6 +15,7 @@ use Symfony\Component\EventDispatcher\EventDispatcher; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; class DataGridSetDataHandlerSpec extends ObjectBehavior { @@ -96,4 +98,18 @@ public function it_return_response_from_datagrid_post_bind_data( $this->handleRequest($event, $request) ->shouldReturnAnInstanceOf('Symfony\Component\HttpFoundation\Response'); } + + public function it_throws_not_found_exception_if_current_page_is_out_of_bound( + DataSource $dataSource, + DataGrid $dataGrid, + ListEvent $event, + Request $request + ) { + $dataSource->getResult()->willThrow(new PageNotFoundException()); + + $event->getDataGrid()->willReturn($dataGrid); + $event->getDataSource()->willReturn($dataSource); + + $this->shouldThrow(NotFoundHttpException::class)->during('handleRequest', [$event, $request]); + } } diff --git a/src/AdminPanel/Component/DataSource/DataSource.php b/src/AdminPanel/Component/DataSource/DataSource.php index b54ffb8..5cead83 100644 --- a/src/AdminPanel/Component/DataSource/DataSource.php +++ b/src/AdminPanel/Component/DataSource/DataSource.php @@ -10,6 +10,7 @@ use AdminPanel\Component\DataSource\Event\DataSourceEvent\ResultEventArgs; use AdminPanel\Component\DataSource\Event\DataSourceEvent\ViewEventArgs; use AdminPanel\Component\DataSource\Exception\DataSourceException; +use AdminPanel\Component\DataSource\Exception\PageNotFoundException; use AdminPanel\Component\DataSource\Field\FieldTypeInterface; use Symfony\Component\EventDispatcher\EventDispatcher; use AdminPanel\Component\DataSource\Event\DataSourceEvents; @@ -251,6 +252,8 @@ public function getResult() $result = $this->driver->getResult($this->fields, $this->getFirstResult(), $this->getMaxResults()); + $this->assertPage($result); + foreach ($this->getFields() as $field) { $field->setDirty(false); } @@ -473,4 +476,21 @@ private function checkFieldsClarity() $this->dirty = false; } } + + private function assertPage($result) + { + if (!$result instanceof \IteratorAggregate) { + // Invalid results + return; + } + + if (count($result) === 0) { + // List with no results at all + return; + } + + if (count($result->getIterator()) === 0) { + throw new PageNotFoundException(); + } + } } diff --git a/src/AdminPanel/Component/DataSource/Exception/PageNotFoundException.php b/src/AdminPanel/Component/DataSource/Exception/PageNotFoundException.php new file mode 100644 index 0000000..9108945 --- /dev/null +++ b/src/AdminPanel/Component/DataSource/Exception/PageNotFoundException.php @@ -0,0 +1,9 @@ +getResponse(); } - $event->getDataGrid()->setData($event->getDataSource()->getResult()); + try { + $event->getDataGrid()->setData($event->getDataSource()->getResult()); + } catch (PageNotFoundException $e) { + throw new NotFoundHttpException($e->getMessage(), $e, $e->getCode()); + } + $this->eventDispatcher->dispatch(ListEvents::LIST_DATAGRID_DATA_POST_BIND, $event); if ($event->hasResponse()) { diff --git a/tests/AdminPanel/Component/DataSource/Tests/DataSourceTest.php b/tests/AdminPanel/Component/DataSource/Tests/DataSourceTest.php index 5b963bc..7dbbeb3 100644 --- a/tests/AdminPanel/Component/DataSource/Tests/DataSourceTest.php +++ b/tests/AdminPanel/Component/DataSource/Tests/DataSourceTest.php @@ -318,6 +318,36 @@ public function testPagination() $this->assertEquals($datasource->getFirstResult(), $first); } + /** + * Checks if exception is thrown when page is out of bound + */ + public function testPageNotFoundException() + { + $driver = $this->createMock(DriverInterface::class); + $dataSource = new DataSource($driver); + + $driver + ->expects($this->once()) + ->method('getResult') + ->will($this->returnValue( + new class implements \IteratorAggregate, \Countable + { + public function count() + { + return 12; + } + + public function getIterator() + { + return new \ArrayIterator([]); + } + } + )); + + $this->expectException('AdminPanel\Component\DataSource\Exception\PageNotFoundException'); + $dataSource->getResult(); + } + /** * Checks preGetParameters and postGetParameters calls. */ diff --git a/tests/AdminPanel/Component/DataSource/Tests/Driver/Doctrine/DoctrineDriverTest.php b/tests/AdminPanel/Component/DataSource/Tests/Driver/Doctrine/DoctrineDriverTest.php index df3fb57..f4cc20c 100644 --- a/tests/AdminPanel/Component/DataSource/Tests/Driver/Doctrine/DoctrineDriverTest.php +++ b/tests/AdminPanel/Component/DataSource/Tests/Driver/Doctrine/DoctrineDriverTest.php @@ -589,6 +589,7 @@ public function testHavingClauseInEntityField() ->select('n') ->from('AdminPanel\Component\DataSource\Tests\Fixtures\News', 'n') ->join('n.category', 'c') + ->groupBy('n.category') ; $driverOptions = [ @@ -615,7 +616,7 @@ public function testHavingClauseInEntityField() $this->assertEquals( $this->testDoctrineExtension->getQueryBuilder()->getQuery()->getDQL(), - 'SELECT n FROM AdminPanel\Component\DataSource\Tests\Fixtures\News n INNER JOIN n.category c HAVING n.category IN (:category)' + 'SELECT n FROM AdminPanel\Component\DataSource\Tests\Fixtures\News n INNER JOIN n.category c GROUP BY n.category HAVING n.category IN (:category)' ); } diff --git a/tests/AdminPanel/Component/DataSource/Tests/Driver/Doctrine/ORM/DoctrineDriverTest.php b/tests/AdminPanel/Component/DataSource/Tests/Driver/Doctrine/ORM/DoctrineDriverTest.php index 7327629..b96c543 100644 --- a/tests/AdminPanel/Component/DataSource/Tests/Driver/Doctrine/ORM/DoctrineDriverTest.php +++ b/tests/AdminPanel/Component/DataSource/Tests/Driver/Doctrine/ORM/DoctrineDriverTest.php @@ -616,6 +616,7 @@ public function testHavingClauseInEntityField() ->select('n') ->from('AdminPanel\Component\DataSource\Tests\Fixtures\News', 'n') ->join('n.category', 'c') + ->groupBy('n.category') ; $driverOptions = [ @@ -642,7 +643,7 @@ public function testHavingClauseInEntityField() $this->assertEquals( $this->testDoctrineExtension->getQueryBuilder()->getQuery()->getDQL(), - 'SELECT n FROM AdminPanel\Component\DataSource\Tests\Fixtures\News n INNER JOIN n.category c HAVING n.category IN (:category)' + 'SELECT n FROM AdminPanel\Component\DataSource\Tests\Fixtures\News n INNER JOIN n.category c GROUP BY n.category HAVING n.category IN (:category)' ); } diff --git a/tests/AdminPanel/Component/DataSource/Tests/Extension/Symfony/FormExtensionTest.php b/tests/AdminPanel/Component/DataSource/Tests/Extension/Symfony/FormExtensionTest.php index 5775457..a1330c4 100644 --- a/tests/AdminPanel/Component/DataSource/Tests/Extension/Symfony/FormExtensionTest.php +++ b/tests/AdminPanel/Component/DataSource/Tests/Extension/Symfony/FormExtensionTest.php @@ -450,11 +450,11 @@ public function testFormFields($type, $comparison, $expected) if ($comparison == 'isNull') { $this->assertEquals( - 'empty', + 'null', $form['fields']['name']->vars['choices'][0]->label ); $this->assertEquals( - 'not empty', + 'notnull', $form['fields']['name']->vars['choices'][1]->label ); } From 63f63ddf036da760ff1fae817ae1199d883532d6 Mon Sep 17 00:00:00 2001 From: zelu Date: Thu, 12 Oct 2017 10:33:35 +0200 Subject: [PATCH 2/5] Revert changes in FormExtensionTest --- .../DataSource/Tests/Extension/Symfony/FormExtensionTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/AdminPanel/Component/DataSource/Tests/Extension/Symfony/FormExtensionTest.php b/tests/AdminPanel/Component/DataSource/Tests/Extension/Symfony/FormExtensionTest.php index a1330c4..5775457 100644 --- a/tests/AdminPanel/Component/DataSource/Tests/Extension/Symfony/FormExtensionTest.php +++ b/tests/AdminPanel/Component/DataSource/Tests/Extension/Symfony/FormExtensionTest.php @@ -450,11 +450,11 @@ public function testFormFields($type, $comparison, $expected) if ($comparison == 'isNull') { $this->assertEquals( - 'null', + 'empty', $form['fields']['name']->vars['choices'][0]->label ); $this->assertEquals( - 'notnull', + 'not empty', $form['fields']['name']->vars['choices'][1]->label ); } From ab429e5290e0b8b66ce26284ea8485a19bcfbe20 Mon Sep 17 00:00:00 2001 From: zelu Date: Thu, 12 Oct 2017 10:37:19 +0200 Subject: [PATCH 3/5] Add symfony/security component --- composer.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index fb84601..3492f80 100644 --- a/composer.json +++ b/composer.json @@ -20,7 +20,8 @@ "symfony/translation": "^3.0", "sensio/framework-extra-bundle": "^3.0", "symfony/twig-bundle": "^3.0", - "doctrine/orm": "^2.5" + "doctrine/orm": "^2.5", + "symfony/security": "^3.3" }, "require-dev": { "symfony/class-loader": "^3.0", From a6b70ba3b05b4be416f3e1020ea3988b16858b89 Mon Sep 17 00:00:00 2001 From: zelu Date: Thu, 12 Oct 2017 10:45:55 +0200 Subject: [PATCH 4/5] Fix ResourceRepositoryPass spec --- .../Compiler/ResourceRepositoryPassSpec.php | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/spec/AdminPanel/Symfony/AdminBundle/DependencyInjection/Compiler/ResourceRepositoryPassSpec.php b/spec/AdminPanel/Symfony/AdminBundle/DependencyInjection/Compiler/ResourceRepositoryPassSpec.php index beff13c..c69146b 100644 --- a/spec/AdminPanel/Symfony/AdminBundle/DependencyInjection/Compiler/ResourceRepositoryPassSpec.php +++ b/spec/AdminPanel/Symfony/AdminBundle/DependencyInjection/Compiler/ResourceRepositoryPassSpec.php @@ -24,14 +24,7 @@ public function it_loads_resources_config_only_if_resource_repository_extension_ ) { $container->hasExtension(Argument::type('string'))->willReturn(false); $container->hasExtension('admin_panel_resource_repository')->willReturn(true); - - $container->addResource(Argument::allOf( - Argument::type('Symfony\Component\Config\Resource\FileResource'), - Argument::that(function ($value) { - return $value instanceof FileResource && - preg_match('/context\/resource\.xml$/', $value->getResource()); - }) - ))->shouldBeCalled(); + $container->fileExists(Argument::type('string'))->willReturn(true); $container->getParameterBag()->willReturn($bag); $container->setDefinition( From 0d59a08908cf2eac9965a9348e9c7d2485b8cebf Mon Sep 17 00:00:00 2001 From: zelu Date: Thu, 12 Oct 2017 12:08:51 +0200 Subject: [PATCH 5/5] Update symfony/framework-bundle version --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 3492f80..655393b 100644 --- a/composer.json +++ b/composer.json @@ -7,7 +7,7 @@ "require": { "php": "^7.0", "twig/twig": "^1.7", - "symfony/framework-bundle" : "^3.0", + "symfony/framework-bundle" : "^3.3", "symfony/form" : "^3.0", "symfony/intl" : "^3.0", "symfony/options-resolver": "^3.0",