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

Stop building show/list/form when accessing fieldDescription #6148

Merged
merged 3 commits into from
Jun 19, 2020
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
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);
VincentLanglet marked this conversation as resolved.
Show resolved Hide resolved
$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');
}
}