Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
minor #33661 [DoctrineBridge] Use VarCloner data instead of legacy ar…
…ray for query params (ostrolucky)

This PR was merged into the 4.4 branch.

Discussion
----------

[DoctrineBridge] Use VarCloner data instead of legacy array for query params

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        |

When I implemented Symfony 4.0 support in DoctrineBundle, I have run into issue that DoctrineBridge does not return VarCloner instance here, so I had to introduce [conversion inside DoctrineBundle](https://github.com/doctrine/DoctrineBundle/blob/af8ac792c9b970ff2bc25b49ab9b31afd9e03dbf/DataCollector/DoctrineDataCollector.php#L135-L141).

We need this because `WebProfilerBundle\Twig\WebProfilerExtension::dumpData()` requires this instance since Symfony 4.0. Not returning this instance here was oversight during work on Symfony 4.0. I did not contribute this sooner, because we can't remove code in DoctrineBundle until we drop Symfony 3.4 support anyways. But not doing this in Symfony 4.4 would mean having to keep transformation code not just during 3.4 LTS lifetime, but 4.4 LTS lifetime too.

Commits
-------

81c6df5 Use VarCloner data instead of legacy array for query params
  • Loading branch information
fabpot committed Sep 26, 2019
2 parents b56a4b4 + 81c6df5 commit 7f43dc4
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 2 deletions.
Expand Up @@ -166,6 +166,8 @@ private function sanitizeQuery(string $connectionName, array $query): array
}
}

$query['params'] = $this->cloneVar($query['params']);

return $query;
}

Expand Down
Expand Up @@ -17,6 +17,7 @@
use Symfony\Bridge\Doctrine\DataCollector\DoctrineDataCollector;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\VarDumper\Cloner\Data;

class DoctrineDataCollectorTest extends TestCase
{
Expand Down Expand Up @@ -96,9 +97,11 @@ public function testCollectQueryWithNoParams()
$c->collect(new Request(), new Response());

$collectedQueries = $c->getQueries();
$this->assertEquals([], $collectedQueries['default'][0]['params']);
$this->assertInstanceOf(Data::class, $collectedQueries['default'][0]['params']);
$this->assertEquals([], $collectedQueries['default'][0]['params']->getValue());
$this->assertTrue($collectedQueries['default'][0]['explainable']);
$this->assertEquals([], $collectedQueries['default'][1]['params']);
$this->assertInstanceOf(Data::class, $collectedQueries['default'][1]['params']);
$this->assertEquals([], $collectedQueries['default'][1]['params']->getValue());
$this->assertTrue($collectedQueries['default'][1]['explainable']);
}

Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Bridge/Doctrine/composer.json
Expand Up @@ -36,6 +36,7 @@
"symfony/security-core": "^4.4|^5.0",
"symfony/expression-language": "^3.4|^4.0|^5.0",
"symfony/validator": "^3.4.31|^4.3.4|^5.0",
"symfony/var-dumper": "^3.4|^4.0|^5.0",
"symfony/translation": "^3.4|^4.0|^5.0",
"doctrine/annotations": "~1.7",
"doctrine/cache": "~1.6",
Expand Down

0 comments on commit 7f43dc4

Please sign in to comment.