This PR was squashed before being merged into the 3.4 branch.
Discussion
----------
[DoctrineBridge] indexBy could reference to association columns
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | yes
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets | Fix#37982 <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License | MIT
| Doc PR | -
<!--
Replace this notice by a short README for your feature/bugfix. This will help people
understand your PR and can be used as a start for the documentation.
Additionally (see https://symfony.com/releases):
- Always add tests and ensure they pass.
- Never break backward compatibility (see https://symfony.com/bc).
- Bug fixes must be submitted against the lowest maintained branch where they apply
(lowest branches are regularly merged to upper ones so they get the fixes too.)
- Features and deprecations must be submitted against branch 5.x.
-->
This is my approach to solve #37982. It partials reverts @xabbuh PR #38604
This is my first Symfony contribution, so please, tell me if I need to do something more or something is wrong.
Also, this bug affects 4.x and 5.x versions. I think merging in this branches is done automatically. If not, please tell me.
Thanks you
Commits
-------
f9a0e000e9 failing test for issue 38861
4c36145664 [DoctrineBridge] indexBy could reference to association columns
This PR was merged into the 5.2-dev branch.
Discussion
----------
Adds constants for YamlEncoder options
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| License | MIT
As I understand it encoders and normalizers use (public) constants for context keys so IDEs can help by providing autocomplete. I added these constants for the YamlEncoder where they are missing right now. For reference see other encoders like CsvEncoder or XmlEncoder.
Commits
-------
6ebf7e164e Adds constants for YamlEncoder options
This PR was merged into the 4.4 branch.
Discussion
----------
[DoctrineBridge] also reset id readers
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#38890
| License | MIT
| Doc PR |
Commits
-------
784bd0080d also reset id readers
This PR was merged into the 5.2-dev branch.
Discussion
----------
[Validator] Allow load mappings from attributes without doctrine/annotations
| Q | A
| ------------- | ---
| Branch? | 5.2
| Bug fix? | no
| New feature? | yes
| Deprecations? | yes
| Tickets | #38096
| License | MIT
| Doc PR | TODO
Follows #38309.
Currently, we cannot enable constraint mapping from attributes without having `doctrine/annotations` installed. Lifting that limitation is a bit tricky because `ValidatorBuilder::enableAnnotationMapping()` creates an annotation reader if you don't pass one. This PR aims at deprecating this behavior.
I know it's a bit late for such a change in 5.2 and I should have seen earlier that this part was missing. 😓 Since I don't expect people to go all-in on attributes on day one, it's probably okay to postpone this change to 5.3.
Commits
-------
441c80603e [Validator] Allow load mappings from attributes without doctrine/annotations.
This PR was merged into the 5.2-dev branch.
Discussion
----------
[FrameworkBundle] Allow to use attribute-based configuration of routing/serializer without doctrine/annotations
| Q | A
| ------------- | ---
| Branch? | 5.2
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | N/A
| License | MIT
| Doc PR | N/A
Follows #37474, #38525
Currently, we need `doctrine/annotations` to be installed in order to configure routing and serializer via PHP attributes. Given that for both components we can already replace Doctrine Annotations completely, I'd like to get rid of that limitation.
Commits
-------
e5492e2e55 [FrameworkBundle] Configure PHP Attributes without doctrine/annotations.
This PR was merged into the 5.2-dev branch.
Discussion
----------
[Validator] Override the default option of the choice constraint
| Q | A
| ------------- | ---
| Branch? | 5.2
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | N/A
| License | MIT
| Doc PR | TODO
We have a bundle that run test against the last version of symfony and we detect BC Break when passing a string as the first argument of the class Choice
Our code extends the Choice class and change the defaultOption.
I saw that others constraints class had new construct signature (for php8 attributes), but only some of them kept the array options as their first arguments. Why ?
ping @ogizanagi @derrabus
https://travis-ci.com/github/Elao/PhpEnums/jobs/410045368
```
1) Elao\Enum\Tests\Unit\Bridge\Symfony\Validator\Constraint\EnumTest::testDefaultValueIsEnumClass
Symfony\Component\Validator\Exception\MissingOptionsException: The options "class" must be set for constraint "Elao\Enum\Bridge\Symfony\Validator\Constraint\Enum".
/home/travis/build/Elao/PhpEnums/vendor/symfony/symfony/src/Symfony/Component/Validator/Constraint.php:171
/home/travis/build/Elao/PhpEnums/vendor/symfony/symfony/src/Symfony/Component/Validator/Constraint.php:110
/home/travis/build/Elao/PhpEnums/vendor/symfony/symfony/src/Symfony/Component/Validator/Constraints/Choice.php:75
/home/travis/build/Elao/PhpEnums/src/Bridge/Symfony/Validator/Constraint/Enum.php:39
/home/travis/build/Elao/PhpEnums/tests/Unit/Bridge/Symfony/Validator/Constraint/EnumTest.php:22
```
Commits
-------
d553750054 Allow user to override default options when extending the Choice Constraint
This PR was merged into the 4.4 branch.
Discussion
----------
[ProxyManagerBridge] replace ProxyManager\Version by feature detection
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
I'd like to get rid of this `Version` class, which is the source of so many issues with deps.
This won't remove the class from ocramius/proxy-manager, but that's a separate story that this change could enable in the end :)
Commits
-------
5f77aad6ca [ProxyManagerBridge] replace ProxyManager\Version by feature detection
This PR was merged into the 4.4 branch.
Discussion
----------
[CI] Fix invalid Doctrine parameter syntax
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | Fix#39008
| License | MIT
| Doc PR | no
~~I cannot reproduce locally.. So Im trying to fix this with the help of the CI~~
I have reproduced locally. This fix will help.
```
❯ phpunit Tests/Transport/DoctrineIntegrationTest.php
PHPUnit 8.5.8 by Sebastian Bergmann and contributors.
.E.... 6 / 6 (100%)
Time: 111 ms, Memory: 6.00 MB
There was 1 error:
1) Symfony\Component\Messenger\Bridge\Doctrine\Tests\Transport\DoctrineIntegrationTest::testSendWithDelay
Doctrine\DBAL\SQLParserUtilsException: Value for :body not found in params array. Params array key should be "body"
src/Symfony/Component/Messenger/Bridge/Doctrine/vendor/doctrine/dbal/src/SQLParserUtilsException.php:21
src/Symfony/Component/Messenger/Bridge/Doctrine/vendor/doctrine/dbal/src/SQLParserUtils.php:277
src/Symfony/Component/Messenger/Bridge/Doctrine/vendor/doctrine/dbal/src/SQLParserUtils.php:203
src/Symfony/Component/Messenger/Bridge/Doctrine/vendor/doctrine/dbal/src/Connection.php:1019
src/Symfony/Component/Messenger/Bridge/Doctrine/vendor/doctrine/dbal/src/Query/QueryBuilder.php:210
src/Symfony/Component/Messenger/Bridge/Doctrine/Tests/Transport/DoctrineIntegrationTest.php:66
ERRORS!
Tests: 6, Assertions: 8, Errors: 1.
```
Apply patch:
```
❯ phpunit Tests/Transport/DoctrineIntegrationTest.php
PHPUnit 8.5.8 by Sebastian Bergmann and contributors.
...... 6 / 6 (100%)
Time: 94 ms, Memory: 6.00 MB
OK (6 tests, 9 assertions)
```
Commits
-------
0ab3032c52 [CI] Fixed invalid doctrine parameter syntax
This PR was merged into the 4.4 branch.
Discussion
----------
[Console] Fix ANSI when stdErr is not a tty
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#38981
| License | MIT
| Doc PR | -
Taking the @wouterj 's comment into account (https://github.com/symfony/symfony/issues/38981#issuecomment-721915428)
This PR prevents using the same Formatter for stdOut and stdErr when possible.
When user send a custom formatter (or call `setFormatter`) the previous logic is kept.
Otherwise, symfony is asked to create the Formatter, and thus is able to clone the formatter.
In a future PR targeting 5.3, we could improve the constructor to let people inject 2 distinguished formatters
Commits
-------
f3a398b5af Fix ANSI when stdErr is not a tty
This PR was merged into the 4.4 branch.
Discussion
----------
[DependencyInjection] Fix circular reference with Factory + Lazy Iterrator
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#38970
| License | MIT
| Doc PR | -
The issue, occurs when a `factory` iterates over services (think tagged iterator) that also need the `factory`.
The PhpDumper is not able to detect the loop because the TaggedService iterator is flaged as "lazy" which is ignored in the loop detection. 2d7e0b02c6/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php (L474-L476)
See test case for a reproduce case.
This PR takes into account lazy services when computing loops.
I'm not sure this is the right thing to do /cc @nicolas-grekas .
A better solution could be to do this ONLY when the service is used as a factory?
Commits
-------
51ff060603 Fix circular referene with Factory and LazyIterator
This PR was merged into the 5.2-dev branch.
Discussion
----------
[DoctrineBridge] accept converting Uid-as-strings to db-values
| Q | A
| ------------- | ---
| Branch? | 5.2
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#38929
| License | MIT
| Doc PR | -
In #38605 I made Uid types stricter by taking inspiration from native Doctrine types. But https://github.com/symfony/symfony/issues/38929#issuecomment-720108301 made me realize this doesn't work with ParamConverters. Here is the fix.
Commits
-------
20714d66c9 [DoctrineBridge] accept converting Uid-as-strings to db-values
This PR was merged into the 5.2-dev branch.
Discussion
----------
[Messenger] Do not call getQueueUrl when the url is known in AmazonSqs transport
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | Fix#38849
| License | MIT
| Doc PR | TODO
When user provides a DSN that looks like a queueUrl, we don't need to call the `getQueueUrl` method. This PR inject the known queueUrl and prevent performing a useless call to the API when sending a message
Commits
-------
f1f44d48e0 Do not call getQueueUrl when the url is known
This PR was merged into the 5.2-dev branch.
Discussion
----------
[Messenger] Improve formatting of exception in failed message
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | not really, enhancement of an existing feature
| Deprecations? | no
| Tickets | Fix#32310
| License | MIT
This PR improves the formatting of exception details in failed messenges when displayed using `messenger:failed:show <id> -vv`.
Before:
<img width="807" alt="Screen Shot 2020-11-01 at 1 05 24 PM" src="https://user-images.githubusercontent.com/4370753/97802602-ea593200-1c44-11eb-8bcb-7fcf2d3f1db0.png">
After:
<img width="803" alt="Screen Shot 2020-11-01 at 1 03 09 PM" src="https://user-images.githubusercontent.com/4370753/97802615-f0e7a980-1c44-11eb-8c12-46b2d4510364.png">
I created a `ThrownExceptionDetails` class which will be displayed as a normal exception when dumped with the VarDumper component. Not sure if this is the right way to do it and if the class is in the right namespace, but this is the best solution I could came up with to fix#32310. I'm open for other suggestions.
Commits
-------
2ad1adda69 [Messenger] Improve formatting of thrown exception in show failed message command
This PR was merged into the 5.2-dev branch.
Discussion
----------
[HttpFundation][FrameworkBundle] Deprecate the HEADER_X_FORWARDED_ALL constant
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | no
| Deprecations? | yes
| Tickets | -
| License | MIT
| Doc PR | TODO
The `HEADER_X_FORWARDED_ALL` implicitly trust the `x-forwarded-host` header, leading to possible host header attack (as warned in the [documentation](https://symfony.com/doc/current/reference/configuration/framework.html#trusted-hosts).)
Moreover, this `HEADER_X_FORWARDED_ALL` does not really fowards **all** headers, as ti does not supports `X-Forwarded-Prefix` headers.
This PR deprecate the constant and the new framework bundle configuration. It will be removed in 6.0. People have to use: either:
- `Request::setTrustedProxies(['1.2.3.4'], Request::HEADER_X_FORWARDED_FOR | Request::HEADER_X_FORWARDED_HOST | Request::HEADER_X_FORWARDED_PORT | Request::HEADER_X_FORWARDED_PROTO);`
- `Request::setTrustedProxies(['1.2.3.4'], Request::HEADER_X_FORWARDED_TRAEFIK);`
- `framework.trusted_headers: [x-forwarded-for, x-forwarded-host, x-forwarded-port, x-forwarded-proto]`
Commits
-------
7cf4dd6917 Deprecate HEADER_X_FORWARDED_ALL constant
This PR was merged into the 5.1 branch.
Discussion
----------
[HttpClient] Check status code before decoding content in TraceableResponse
| Q | A
| ------------- | ---
| Branch? | 5.1
| Bug fix? | yes
| New feature? | no
| Deprecations? | np
| Tickets | -
| License | MIT
| Doc PR | -
Using `toArray()` on the response of a traceable client, the status code is currently checked after json decoding, which leads to `JsonException` being thrown instead of `ClientException`.
It should be the opposite, as for non-traceable responses.
Commits
-------
e5595dae73 [HttpClient] Check status code before decoding content in TraceableResponse