Skip to content

Commit

Permalink
Stop building show/list/form when accessing fieldDescription (#6148)
Browse files Browse the repository at this point in the history
* Update interface

* Use state to avoid infinite loop

* Add test
  • Loading branch information
VincentLanglet authored Jun 19, 2020
1 parent c2ec317 commit 5386609
Show file tree
Hide file tree
Showing 5 changed files with 131 additions and 22 deletions.
4 changes: 0 additions & 4 deletions src/Action/RetrieveAutocompleteItemsAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -184,8 +184,6 @@ private function retrieveFilterFieldDescription(
AdminInterface $admin,
string $field
): FieldDescriptionInterface {
$admin->getFilterFieldDescriptions();

$fieldDescription = $admin->getFilterFieldDescription($field);

if (!$fieldDescription) {
Expand All @@ -208,8 +206,6 @@ private function retrieveFormFieldDescription(
AdminInterface $admin,
string $field
): FieldDescriptionInterface {
$admin->getFormFieldDescriptions();

$fieldDescription = $admin->getFormFieldDescription($field);

if (!$fieldDescription) {
Expand Down
27 changes: 21 additions & 6 deletions src/Admin/AbstractAdmin.php
Original file line number Diff line number Diff line change
Expand Up @@ -399,10 +399,14 @@ abstract class AbstractAdmin implements AdminInterface, DomainObjectInterface, A
* @var array<string, bool>
*/
protected $loaded = [
'view_fields' => false,
'view_groups' => false,
'view_fields' => false, // NEXT_MAJOR: Remove this unused value.
'view_groups' => false, // NEXT_MAJOR: Remove this unused value.
'routes' => false,
'tab_menu' => false,
'show' => false,
'list' => false,
'form' => false,
'datagrid' => false,
];

/**
Expand Down Expand Up @@ -845,12 +849,17 @@ public function getFilterParameters()
return $parameters;
}

/**
* NEXT_MAJOR: Change the visibility to protected (similar to buildShow, buildForm, ...).
*/
public function buildDatagrid()
{
if ($this->datagrid) {
if ($this->loaded['datagrid']) {
return;
}

$this->loaded['datagrid'] = true;

$filterParameters = $this->getFilterParameters();

// transform _sort_by from a string to a FieldDescriptionInterface for the datagrid.
Expand Down Expand Up @@ -3255,10 +3264,12 @@ protected function configureTabMenu(ItemInterface $menu, $action, ?AdminInterfac
*/
protected function buildShow()
{
if ($this->show) {
if ($this->loaded['show']) {
return;
}

$this->loaded['show'] = true;

$this->show = $this->getShowBuilder()->getBaseList();
$mapper = new ShowMapper($this->getShowBuilder(), $this->show, $this);

Expand All @@ -3274,10 +3285,12 @@ protected function buildShow()
*/
protected function buildList()
{
if ($this->list) {
if ($this->loaded['list']) {
return;
}

$this->loaded['list'] = true;

$this->list = $this->getListBuilder()->getBaseList();
$mapper = new ListMapper($this->getListBuilder(), $this->list, $this);

Expand Down Expand Up @@ -3333,10 +3346,12 @@ protected function buildList()
*/
protected function buildForm()
{
if ($this->form) {
if ($this->loaded['form']) {
return;
}

$this->loaded['form'] = true;

// append parent object if any
// todo : clean the way the Admin class can retrieve set the object
if ($this->isChild() && $this->getParentAssociationMapping()) {
Expand Down
38 changes: 26 additions & 12 deletions src/Admin/AdminInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,19 @@
/**
* @author Thomas Rabaix <[email protected]>
*
* @method array configureActionButtons(string $action, ?object $object = null)
* @method string getSearchResultLink(object $object)
* @method void showMosaicButton(bool $isShown)
* @method bool isDefaultFilter(string $name)
* @method bool isCurrentRoute(string $name, ?string $adminCode)
* @method bool canAccessObject(string $action, object $object)
* @method mixed getPersistentParameter(string $name)
* @method array getExportFields
* @method array getSubClasses
* @method AdminInterface getRoot
* @method string getRootCode
* @method array getActionButtons(string $action, ?object $object)
* @method array configureActionButtons(string $action, ?object $object = null)
* @method string getSearchResultLink(object $object)
* @method void showMosaicButton(bool $isShown)
* @method bool isDefaultFilter(string $name)
* @method bool isCurrentRoute(string $name, ?string $adminCode)
* @method bool canAccessObject(string $action, object $object)
* @method mixed getPersistentParameter(string $name)
* @method array getExportFields()
* @method array getSubClasses()
* @method AdminInterface getRoot()
* @method string getRootCode()
* @method array getActionButtons(string $action, ?object $object)
* @method FieldDescriptionCollection|null getList()
*/
interface AdminInterface extends AccessRegistryInterface, FieldDescriptionRegistryInterface, LifecycleHookProviderInterface, MenuBuilderInterface, ParentAdminInterface, UrlGeneratorInterface
{
Expand Down Expand Up @@ -270,6 +271,9 @@ public function getValidator();
*/
public function getShow();

// NEXT_MAJOR: uncomment this method in 4.0
// public function getList(): ?FieldDescriptionCollection;

public function setFormTheme(array $formTheme);

/**
Expand Down Expand Up @@ -360,6 +364,8 @@ public function setSubject($subject);
public function getSubject();

/**
* NEXT_MAJOR: Remove this method, since it's already in FieldDescriptionRegistryInterface.
*
* Returns a list FieldDescription.
*
* @param string $name
Expand All @@ -369,6 +375,8 @@ public function getSubject();
public function getListFieldDescription($name);

/**
* NEXT_MAJOR: Remove this method, since it's already in FieldDescriptionRegistryInterface.
*
* Returns true if the list FieldDescription exists.
*
* @param string $name
Expand All @@ -378,6 +386,8 @@ public function getListFieldDescription($name);
public function hasListFieldDescription($name);

/**
* NEXT_MAJOR: Remove this method, since it's already in FieldDescriptionRegistryInterface.
*
* Returns the collection of list FieldDescriptions.
*
* @return array
Expand Down Expand Up @@ -542,13 +552,17 @@ public function setShowGroups(array $showGroups);
public function reorderShowGroup($group, array $keys);

/**
* NEXT_MAJOR: Remove this method, since it's already in FieldDescriptionRegistryInterface.
*
* add a FieldDescription.
*
* @param string $name
*/
public function addFormFieldDescription($name, FieldDescriptionInterface $fieldDescription);

/**
* NEXT_MAJOR: Remove this method, since it's already in FieldDescriptionRegistryInterface.
*
* Remove a FieldDescription.
*
* @param string $name
Expand Down
46 changes: 46 additions & 0 deletions tests/Admin/AdminTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@
use Sonata\AdminBundle\Security\Handler\AclSecurityHandlerInterface;
use Sonata\AdminBundle\Security\Handler\SecurityHandlerInterface;
use Sonata\AdminBundle\Templating\MutableTemplateRegistryInterface;
use Sonata\AdminBundle\Tests\App\Builder\DatagridBuilder;
use Sonata\AdminBundle\Tests\App\Builder\FormContractor;
use Sonata\AdminBundle\Tests\App\Builder\ListBuilder;
use Sonata\AdminBundle\Tests\App\Builder\ShowBuilder;
use Sonata\AdminBundle\Tests\Fixtures\Admin\AvoidInfiniteLoopAdmin;
use Sonata\AdminBundle\Tests\Fixtures\Admin\CommentAdmin;
use Sonata\AdminBundle\Tests\Fixtures\Admin\CommentVoteAdmin;
use Sonata\AdminBundle\Tests\Fixtures\Admin\CommentWithCustomRouteAdmin;
Expand All @@ -67,6 +72,9 @@
use Symfony\Component\Form\FormBuilderInterface;
use Symfony\Component\Form\FormEvent;
use Symfony\Component\Form\FormEvents;
use Symfony\Component\Form\FormFactory;
use Symfony\Component\Form\FormRegistry;
use Symfony\Component\Form\ResolvedFormTypeFactory;
use Symfony\Component\HttpFoundation\ParameterBag;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\PropertyAccess\PropertyAccess;
Expand Down Expand Up @@ -1597,6 +1605,13 @@ public function testGetFormWithCollectionParentValue(): void
$this->assertInstanceOf(Collection::class, $tag->getPosts());
$this->assertCount(2, $tag->getPosts());
$this->assertContains($post, $tag->getPosts());
}

public function testGetFormWithArrayParentValue(): void
{
$post = new Post();
$tagAdmin = $this->createTagAdmin($post);
$tag = $tagAdmin->getSubject();

// Case of an array
$tag->setPosts([]);
Expand Down Expand Up @@ -2495,6 +2510,37 @@ public function testAdminWithoutControllerName(): void
$this->assertNull($admin->getBaseControllerName());
}

public function testAdminAvoidInifiniteLoop(): void
{
$this->expectNotToPerformAssertions();

$formFactory = new FormFactory(new FormRegistry([], new ResolvedFormTypeFactory()));

$admin = new AvoidInfiniteLoopAdmin('code', \stdClass::class, null);
$admin->setSubject(new \stdClass());

$admin->setFormContractor(new FormContractor($formFactory));

$admin->setShowBuilder(new ShowBuilder());

$admin->setListBuilder(new ListBuilder());

$pager = $this->createStub(PagerInterface::class);
$admin->setDatagridBuilder(new DatagridBuilder($formFactory, $pager));

$validator = $this->createMock(ValidatorInterface::class);
$validator->method('getMetadataFor')->willReturn($this->createStub(MemberMetadata::class));
$admin->setValidator($validator);

$routeGenerator = $this->createStub(RouteGeneratorInterface::class);
$admin->setRouteGenerator($routeGenerator);

$admin->getForm();
$admin->getShow();
$admin->getList();
$admin->getDatagrid();
}

/**
* NEXT_MAJOR: Remove this test and its data provider.
*
Expand Down
38 changes: 38 additions & 0 deletions tests/Fixtures/Admin/AvoidInfiniteLoopAdmin.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<?php

declare(strict_types=1);

namespace Sonata\AdminBundle\Tests\Fixtures\Admin;

use Sonata\AdminBundle\Admin\AbstractAdmin;
use Sonata\AdminBundle\Datagrid\DatagridMapper;
use Sonata\AdminBundle\Datagrid\ListMapper;
use Sonata\AdminBundle\Form\FormMapper;
use Sonata\AdminBundle\Show\ShowMapper;

final class AvoidInfiniteLoopAdmin extends AbstractAdmin
{
protected function configureDatagridFilters(DatagridMapper $datagridMapper): void
{
$this->getFilterFieldDescriptions();
$this->hasFilterFieldDescription('help');
}

protected function configureListFields(ListMapper $listMapper): void
{
$this->getListFieldDescriptions();
$this->hasFilterFieldDescription('help');
}

protected function configureFormFields(FormMapper $formMapper): void
{
$this->getFormFieldDescriptions();
$this->hasFilterFieldDescription('help');
}

protected function configureShowFields(ShowMapper $showMapper): void
{
$this->getShowFieldDescriptions();
$this->hasFilterFieldDescription('help');
}
}

0 comments on commit 5386609

Please sign in to comment.