This PR was merged into the 4.4 branch.
Discussion
----------
[ErrorCatcher] Pretty print JSON formatted errors
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | yes
| BC breaks? | no
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass? | yes
| Fixed tickets | -
| License | MIT
| Doc PR | not needed
A long string with JSON contents is not very useful to quickly parse the error contents, so I propose to "pretty print" the JSON errors.
Commits
-------
ab926d2 [ErrorCatcher] Pretty print JSON formatted errors
This PR was merged into the 3.4 branch.
Discussion
----------
[FrameworkBundle] reset cache pools between requests
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | -
| License | MIT
| Doc PR | -
Looks like we missed this part: cache pools should all be reset between requests, at least to persist any deferred items. Replaces #32361 (which should be applied when merging 3.4 into 4.2).
Commits
-------
5ff45bac66 [FrameworkBundle] reset cache pools between requests
This PR was merged into the 3.4 branch.
Discussion
----------
[DI] fix processing of regular parameter bags by MergeExtensionConfigurationPass
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | -
| License | MIT
| Doc PR | -
Spotted in and needed by #32294
Commits
-------
b06d0003a3 [DI] fix processing of regular parameter bags by MergeExtensionConfigurationPass
This PR was merged into the 4.4 branch.
Discussion
----------
[Messenger] Use ConnectionRegistry instead of RegistryInterface
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | N/A
| License | MIT
| Doc PR | N/A
This PR changes the constructor type-hint on `DoctrineTransportFactory` from `Symfony\Bridge\Doctrine\RegistryInterface` to the smaller `Doctrine\Common\Persistence\ConnectionRegistry`. Since we only call the `getConnection()` method, this interface is sufficient.
This change allows to use the factory without the Doctrine bridge and makes it easier to use it stand-alone.
Commits
-------
ce6a5ad235 Use ConnectionRegistry instead of RegistryInterface.
This PR was squashed before being merged into the 4.4 branch (closes#32348).
Discussion
----------
[HttpFoundation] Accept must take the lead for Request::getPreferredFormat()
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| BC breaks? | no <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass? | yes <!-- please add some, will be required by reviewers -->
| Fixed tickets | n/a
| License | MIT
| Doc PR | n/a
Follow up PR to #32344: if both `Accept` and `Content-Type` are defined, `Accept` must take the lead because it explicitly tells what format the client expect as a response.
Before:
```
$ curl -H 'Accept: application/json' -H 'Content-Type: text/xml' -i 'https://127.0.0.1:8000/userinfo'
[snip]
content-type: text/xml
```
After:
```
$ curl -H 'Accept: application/json' -H 'Content-Type: text/xml' -i 'https://127.0.0.1:8000/userinfo'
[snip]
content-type: application/json
```
Actually, I'm not sure that inferring the content type of the response using the `Content-Type` provided for the request body is a good idea. The HTTP RFC explicitly states that `Accept` must be used to hint a preferred response format (`Content-Type` on the request indicates the type of associated its the body). I would be in favor of being more conservative: use `Accept` if provided (a best practice anyway), and fallback to the default value (HTML by default) otherwise. WDYT?
Commits
-------
60d997df75 [HttpFoundation] Accept must take the lead for Request::getPreferredFormat()
This PR was submitted for the 4.3 branch but it was merged into the 4.4 branch instead (closes#32207).
Discussion
----------
[FrameworkBundle] Allow to use the BrowserKit assertions with Panther and API Platform's test client
| Q | A
| ------------- | ---
| Branch? | 4.3
| Bug fix? | yes
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| BC breaks? | no <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass? | yes <!-- please add some, will be required by reviewers -->
| Fixed tickets | n/a
| License | MIT
| Doc PR | n/a
I'm adding a testing client for API Platform that implements the interfaces of HttpClient: https://github.com/api-platform/core/pull/2608
Most PHPUnit assertions provided by Symfony are useful and can be reused, but the ones using the crawler are not relevant and pollute auto-complete suggestions (because a web API usually returns JSON, not HTML).
This PR splits the existing trait to allow reusing the HTTP related assertions only.
Commits
-------
cd0341e69b [FrameworkBundle] Allow to use the BrowserKit assertions with Panther and API Platform's test client
* 4.3:
Fixes windows error
[Messenger] Added more test for MessageBus
fixed typo
[Filesystem] added missing deprecations to UPGRADE-4.3.md
Fix authentication for redis transport
only decorate when an event dispatcher was passed
[FrmaeworkBundle] More simplifications in the DI configuration
Fixing validation for messenger transports retry_strategy service key
Removed unused field.
Remove @internal annotations for the serilize methods
[Lock] Stores must implement `putOffExpiration`
Annotated correct return type for getInEdges()/getOutEdges().
deprecate the framework.templating option
* 4.2:
Fixes windows error
Removed unused field.
[Lock] Stores must implement `putOffExpiration`
Annotated correct return type for getInEdges()/getOutEdges().
This PR was submitted for the 4.3 branch but it was merged into the 3.4 branch instead (closes#32187).
Discussion
----------
[PHPUnit] Fixed composer error on Windows
| Q | A
| ------------- | ---
| Branch? | 4.3
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Fixed tickets | #31750
| License | MIT
Fixes bug when composer runs from bat file that described in PATH env.
Commits
-------
1f8927a9a6 Fixes windows error
This PR was merged into the 4.4 branch.
Discussion
----------
[HttpFoundation][HttpKernel] Improving the request/response format autodetection
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | yes
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | -
| License | MIT
| Doc PR | -
Mainly for API-based apps, currently the response header `Content-Type` (if no provided) is guessed based on the request format (`_format` attribute), falling back to `html` by default.
Especially for the new error renderer system, where any kind of error can occur and it becomes an http response, this PR improves this guesser mechanism by taking into account also the `Content-type` of the request.
Example:
```bash
$ curl -X POST -H 'Content-Type: application/json' -i 'https://127.0.0.1:8000/login'
```
**before:**
```bash
HTTP/2 500
cache-control: no-cache, private
content-type: text/html; charset=UTF-8 # <- inaccurate
...
{"title":"Internal Server Error","status":500,"detail":"Invalid credentials!"}
```
Most of the 3rd-party bundles that I know (`api-platform/core`, `FOSRestBundle`) need a dedicated listener to achieve it right.
**after:**
```bash
HTTP/2 500
cache-control: no-cache, private
content-type: application/json
...
{"title":"Internal Server Error","status":500,"detail":"Invalid credentials!"}
```
Of course, this applies to all kind of responses, as long as the `Content-Type` is not explicitly provided. So, as a last chance, the `Accept` heading of the request is also taken into account to detect the preferred format:
```bash
$ curl -H 'Accept: application/json' -i 'https://127.0.0.1:8000/userinfo'
HTTP/2 404
cache-control: no-cache, private
content-type: application/json
...
{"title":"Not Found","status":404,"detail":"No route found for \"GET \/userinfo\""}
```
They could be other places in the code where this new method could also be useful, please advise :)
WDYT?
Commits
-------
1952928471 Improving the request/response format autodetection
This PR was merged into the 4.2 branch.
Discussion
----------
[Lock] Stores must implement `putOffExpiration`
| Q | A
| ------------- | ---
| Branch? | 4.2
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | /
| License | MIT
| Doc PR | /
Following https://github.com/symfony/symfony/pull/32198#pullrequestreview-256165051 every stores MUST implement the method `putOffExpiration` either by ignoring the arguments (by design they lock forever) or using a mechanism to define the expiration.
It was a mistake to add the dockblock `@throws NotSupportedException` tell me if it's a BC break, I'll create a dedicated PR for it.
Commits
-------
c986c86d1c [Lock] Stores must implement `putOffExpiration`
This PR was merged into the 4.3 branch.
Discussion
----------
[Mime] Remove @internal annotations for the serialize methods
| Q | A
| ------------- | ---
| Branch? | 4.3
| Bug fix? | yes (it's not really a bug)
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | ~
| License | MIT
| Doc PR | ~
Currently, when we extend the `Symfony\Component\Mime\Message` and `Symfony\Component\Mime\MessageRaw` classes of the Mime component, we get 2 deprecation messages:
```
The "Symfony\Component\Mime\Message::__serialize()" method is considered internal. It may change without further notice. You should not extend it from "Vendor\FooMessage".
```
and
```
The "Symfony\Component\Mime\Message::__unserialize()" method is considered internal. It may change without further notice. You should not extend it from "Vendor\FooMessage".
```
However, we need to add properties to the new class, and so, we need to extend `__serialize()` and `__unserialize()` methods in the same way as `Symfony\Bridge\Twig\Mime\TemplatedEmail`, to know, retrieve the serialization of the parent class:
```php
public function __serialize(): array
{
return [$this->foo, $this->bar, $this->baz, parent::__serialize()];
}
public function __unserialize(array $data): void
{
[$this->foo, $this->bar, $this->baz, $parentData] = $data;
parent::__unserialize($parentData);
}
```
But given that the third-party components use another namespace, we get the 2 deprecation messages, while the 2 methods must be inevitably used and extended. Of course, the methods `serialize()` and `unserialize()` are always marked by the `@internal` annotation and the `final` keyword.
This PR so deletes the 2 deprecation messages that should not be displayed.
Commits
-------
8544a35e52 Remove @internal annotations for the serilize methods
This PR was merged into the 4.3 branch.
Discussion
----------
[Messenger] Added more test for MessageBus
| Q | A
| ------------- | ---
| Branch? | 4.3
| Bug fix? | no
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets |
| License | MIT
| Doc PR |
[This code](bed50fd542/src/Symfony/Component/Messenger/MessageBus.php (L33-L49)) is quite hard to understand. So It must be covered by tests.
More over, it will help people to understand how it works
Commits
-------
5f4ab23991 [Messenger] Added more test for MessageBus
This PR was merged into the 4.4 branch.
Discussion
----------
[Messager] Simplified MessageBus::__construct()
| Q | A
| ------------- | ---
| Branch? | 4.3
| Bug fix? | no
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets |
| License | MIT
| Doc PR |
The third path deals with generator (or other king of iterator that are
not an IteratorAggregate). It means, very few cases in practice
The previous code saved one object on the first call of `self::dispatch()`
by replacing it at runtime. More over, this object (anon. class) is very
light in memory.
The performance optimization (even if fun) is not useful here. Let's
make the code readable to everyone.
Commits
-------
6b5671f5ae [Messager] Simplified MessageBus::__construct()
The third path deals with generator (or other king of iterator that are
not an IteratorAggregate). It means, very few cases in practice
The previous code saved one object on the first call of `self::dispatch()`
by replacing it at runtime. More over, this object (anon. class) is very
light in memory.
The performance optimization (even if fun) is not useful here. Let's
make the code readable to everyone.
This PR was merged into the 4.4 branch.
Discussion
----------
[PropertyInfo] add static cache to ContextFactory
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no (performance...)
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | #32073, https://github.com/api-platform/api-platform/issues/1159
| License | MIT
| Doc PR | no
The issue is very very well described here #32073, and was also discussed a few weeks ago with @dunglas here https://github.com/api-platform/api-platform/issues/1159.
`ContextFactory::createFromReflector` is heavy, and it's called redundanlty
by `Symfony\Component\PropertyInfo\Extractor\PhpDocExtractor` for each property
and methods. Avoid that by parsing it only once and then use static cache
This is a quite big performance problem.
Commits
-------
063e880861 [PropertyInfo] add static cache to ContextFactory
This PR was merged into the 4.4 branch.
Discussion
----------
[HttpClient] Add support for NTLM authentication
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | yes
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | -
| License | MIT
| Doc PR | -
Someone mentioned NTLM auth on Twitter, so here we are, this adds support for it.
Requires curl.
Commits
-------
548f4fd0ea [HttpClient] Add support for NTLM authentication
This PR was merged into the 4.3 branch.
Discussion
----------
[Messenger] Fix authentication for redis transport
| Q | A
| ------------- | ---
| Branch? | 4.3
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | #32304
| License | MIT
| Doc PR | symfony/symfony-docs#... TODO
This will implement support for password in redis stream transport.
Commits
-------
bedae5dde9 Fix authentication for redis transport
This PR was merged into the 3.4 branch.
Discussion
----------
[DependencyInjection] Annotated correct return type for getInEdges()/getOutEdges()
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | N/A
| License | MIT
| Doc PR | N/A
This changed annotation was quite helpful when analyzing `PhpDumper` for #32266.
Commits
-------
28882f52cb Annotated correct return type for getInEdges()/getOutEdges().
This PR was merged into the 4.4 branch.
Discussion
----------
[Validator] Add a Length::$allowEmptyString option to reject empty strings
| Q | A
| ------------- | ---
| Branch? | 4.4 <!-- see below -->
| Bug fix? | no
| New feature? | yes <!-- please update src/**/CHANGELOG.md files -->
| BC breaks? | no <!-- see https://symfony.com/bc -->
| Deprecations? | yes <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass? | yes <!-- please add some, will be required by reviewers -->
| Fixed tickets | N/A <!-- #-prefixed issue number(s), if any -->
| License | MIT
| Doc PR | Todo (change the warning on top of https://symfony.com/doc/current/reference/constraints/Length.html)
which defaults to `true` in 4.4 but will trigger a deprecation if not set explicitly
in order to make the default `false` in 5.0.
While it could be solved now thanks to #29641 by using both `@Length(min=1)` & `@NotBlank(allowNull=true)` constraints,
as expressed in https://github.com/symfony/symfony/issues/27876#issuecomment-403307783 and following comments, the `@Length(min=1)` behavior doesn't match our expectations when reading it: it feels logical to invalidate empty strings, but it actually doesn't.
Hence the proposal of making the behavior of rejecting empty strings the default in 5.0.
In my opinion, the flag could even be removed later.
Commits
-------
e113e7f812 [Validator] Add a Length::$allowEmptyString option to reject empty strings
This PR was merged into the 4.4 branch.
Discussion
----------
[HttpKernel][DX] Improve the error message when not defining the controller as a service but using construct parameters
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| BC breaks? | no <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass? | yes <!-- please add some, will be required by reviewers -->
| Fixed tickets | none <!-- #-prefixed issue number(s), if any -->
| License | MIT
| Doc PR | not needed <!-- required for new features -->
<!--
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/roadmap):
- 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 4.4.
- Legacy code removals go to the master branch.
-->
I'm working with newcomers to the symfony framework ATM and one of them has got stuck in this errors trying to understand why the constructor argument declared as a a service would not work and didn't check if the controller was declared as a service itself.
I feel like this could be improve to be really specific since "such" is not that specific ;).
Commits
-------
c670d5120e [HttpKernel][DX] Improve the error message when not defining the controller as a service but using contruct parameters
This PR was squashed before being merged into the 4.4 branch (closes#32081).
Discussion
----------
[WIP][Mailer] Overwrite envelope sender and recipients from config
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | yes
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes <!-- please add some, will be required by reviewers -->
| Fixed tickets | #31592#31733 <!-- #-prefixed issue number(s), if any -->
| License | MIT
| Doc PR | symfony/symfony-docs#... <!-- required for new features -->
# Description
This MR adds the following configuration, example:
```yaml
# config/packages/mailer.yaml
framework:
mailer:
envelope:
sender: 'sender@example.org'
recipients: ['redirected@example.org']
```
In turn the `\Symfony\Component\Mailer\EventListener\EnvelopeListener` will be configured to alter the sender and recipient addresses before the message has been sent.
Note: it will only alter the envelope, thus rerouting the message to the correct mailbox. However the message itself will still have the original 'from' and 'to' headers.
# Todos
- [x] Alter configuration and dependency injection
- [x] Create test case
- [ ] Update XML config schema?
- [ ] 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/roadmap):
- 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 4.4.
- Legacy code removals go to the master branch.
-->
Commits
-------
8e0c8006d8 [WIP][Mailer] Overwrite envelope sender and recipients from config
This PR was merged into the 4.3 branch.
Discussion
----------
[FrmaeworkBundle] More simplifications in the DI configuration
| Q | A
| ------------- | ---
| Branch? | 4.3
| Bug fix? | no
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| BC breaks? | no <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass? | yes <!-- please add some, will be required by reviewers -->
| Fixed tickets | -
| License | MIT
| Doc PR | not needed
This is the continuation of #32322 for the 4.3 branch.
Commits
-------
fa6ae8ce50 [FrmaeworkBundle] More simplifications in the DI configuration
This PR was merged into the 4.4 branch.
Discussion
----------
[Filesystem] deprecate calling isAbsolutePath with a null
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| BC breaks? | no <!-- see https://symfony.com/bc -->
| Deprecations? | yes <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass? | yes <!-- please add some, will be required by reviewers -->
| Fixed tickets | N/A <!-- #-prefixed issue number(s), if any -->
| License | MIT
| Doc PR | N/A
This PR is a result of #32247 and deprecates calling `Filesystem::isAbsolutePath()` with a `null` value.
Commits
-------
93190182f6 [Filesystem] depreacte calling isAbsolutePath with a null
