This PR was merged into the 5.3-dev branch.
Discussion
----------
[Notifier] Microsoft Teams: JSON structure error fix for simple message
Code caused following error - 'Unable to post the Microsoft Teams message: ... Summary or Text is required. '. The structure of the message should contain the 'text' attribute instead of 'title'.
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix
| License | MIT
Commits
-------
636dfc6b18 [Notifier] Microsoft Teams: JSON structure error fix for simple message
Code caused following error - 'Unable to post the Microsoft Teams message: ... Summary or Text is required. '. The structure of the message should contain the 'text' attribute instead of 'title'.
This PR was merged into the 4.4 branch.
Discussion
----------
RequestMatcher issue when `_controller` is a closure
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| License | MIT
## Description
If the `matches` method of `RequestMatcher` is used on an attribute which is a closure it crashes.
## How did we get it
On a project that is using FOS HTTP Cache, we have this configuration
```yaml
fos_http_cache:
cache_control:
rules:
-
match:
attributes: { _controller: ^App\\Controller\\.*::.* }
headers:
overwrite: true
cache_control: { public: true, private: false, must_revalidate: true, s_maxage: 3600 }
```
Everything works fine unless you are reaching a controller that is a closure.
You get a
```TypeError: preg_match(): Argument #2 ($subject) must be of type string, Closure given``` which is to me logical.
## Proposed solution
Just testing the type of attribute value and return false before crashing `preg_match`
This PR adds a quick unit test to enforce this.
Commits
-------
66491238e3 Fix issue with RequestMatcher when attribute is a closure
This PR was merged into the 4.4 branch.
Discussion
----------
[PropertyInfo] Use the right context for methods defined in traits
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#34191
| License | MIT
| Doc PR |
Pull request #40175 only partially fixed#34191 - it solved the problem for properties used in traits but it did not address the same issue with methods.
I have therefore applied the same style of fix and confirmed it works properly with tests.
Commits
-------
c7e9493c5b [PropertyInfo] Use the right context for methods defined in traits
This PR was merged into the 5.3-dev branch.
Discussion
----------
[Intl] Switch from json to php resources
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | Fix #23545
| License | MIT
| Doc PR | -
take over #34214
Commits
-------
24bfc3bedd [Intl] Switch from json to php resources
This PR was merged into the 5.3-dev branch.
Discussion
----------
[Routing] Fix supporting string "methods" and "schemes" on the Route annotation
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | https://github.com/symfony/symfony/issues/40796
| License | MIT
| Doc PR | -
Commits
-------
b5f61c31e5 [Routing] Fix supporting string "methods" and "schemes" on the Route annotation
This PR was squashed before being merged into the 5.3-dev branch.
Discussion
----------
[Config] Don't regenerate the ConfigBuilders if they already exists
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | yes... or optimisation
| Deprecations? | no
| Tickets |
| License | MIT
| Doc PR |
When the user is trying to use the `Symfony\Config\AcmeConfig` ConfigBuilder, the `PhpFileLoader` will check if this class is loaded (ie `class_exists()`. Since it is normally generated and put in the `kernel.build_dir`, the class is not found by a PSR-4 class loader. So the `class_exists()` check will most likely fail. It will only return true if a third party bundle define `Symfony\Config\AcmeConfig` and make sure it is loaded with their composer.json. This is a great feature for third party bundles to be able to provide a config class.
However, if the class `Symfony\Config\AcmeConfig` does not exist, we try to generate it. Always... Which means that every time you rebuild the container, we generate these files.
This PR will do a check if `kernel.build_dir/Symfony/Config/AcmeConfig.php` exists first, and generate the file only if it is missing.
Commits
-------
88896e8d57 [Config] Don't regenerate the ConfigBuilders if they already exists
This PR was squashed before being merged into the 5.3-dev branch.
Discussion
----------
[Config][FrameworkBundle] Add CacheWarmer for ConfigBuilder
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | yes
| Deprecations? |
| Tickets |
| License | MIT
| Doc PR |
Make sure ConfigBuilder exists before you write your first line of config.
This is similar to #40803
Commits
-------
c1d6c0e57a [Config][FrameworkBundle] Add CacheWarmer for ConfigBuilder
This PR was merged into the 5.3-dev branch.
Discussion
----------
[DependencyInjection][Routing] Access environment in PHP config
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | yes
| Deprecations? |
| Tickets |
| License | MIT
| Doc PR |
This will allow me to write config like:
```php
use Symfony\Config\FrameworkConfig;
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
return static function (FrameworkConfig $framework, ContainerConfigurator $container) {
if ('test' === $container->env()) {
$framework->test(true);
$framework->session()->storageFactoryId('session.storage.mock_file');
}
};
```
This PR will also revert parts of #40214. It is much simpler to maintain and write PHP config without `->when()`. Instead we add `ContainerConfigurator::env(): ?string` and `RoutingConfigurator::env(): ?string`.
```php
use App\Controller\BlogController;
use Symfony\Component\Routing\Loader\Configurator\RoutingConfigurator;
return function (RoutingConfigurator $routes) {
if ($routes->env() === 'dev') {
$routes->add('debug-stats', '/blog-debyg')
->controller([BlogController::class, 'debug'])
;
}
$routes->add('blog_list', '/blog')
->controller([BlogController::class, 'list'])
;
};
```
Commits
-------
11dfaa4742 [DependencyInjection][Routing] Access environment in PHP config
This PR was merged into the 5.3-dev branch.
Discussion
----------
Improve deprecation message for session
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
The `session` service is not public since we deprecate it, which lead to headache when people migrate to 5.3:
When people expect the service to be public (like in tests), the service does not exist anymore (removed or inlined), it's now an alias to `.session.do-not-use` and deprecation telling how to migrate has not been triggered because the service has been removed / inlined.
This PR makes the `session` service/alias public, and also improves the deprecation message a little bit.
/cc @javiereguiluz , @wouterj
Commits
-------
b568768cec Improvide deprecation message for session
This PR was merged into the 4.4 branch.
Discussion
----------
[FrameworkBundle] Fix missing class in messenger service locator definitions
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
Should make travis green once merged up to 5.x.
Commits
-------
615bcaef08 [FrameworkBundle] Fix messenger.receiver_locator service definition
This PR was squashed before being merged into the 4.4 branch.
Discussion
----------
Fix CS in IsbnValidator and IssnValidator
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets |
| License | MIT
| Doc PR |
Commits
-------
dc90c195e3 Fix CS in IsbnValidator and IssnValidator
This PR was merged into the 5.3-dev branch.
Discussion
----------
Remove the experimental flag from the authenticator system 🚀
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | todo
Here we go.
Commits
-------
a2f5693c5e Remove experimental flag from the authenticator system 🚀
This PR was squashed before being merged into the 5.3-dev branch.
Discussion
----------
[Form] Add support for sorting fields
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | Fix#5827
| License | MIT
| Doc PR | TODO
This PR adds a new `priority` option used later to sort all fields in order of importance instead of simply numerical order, i.e. fields with higher priority will be rendered first, fields with lower priority will be rendered last. The default priority would be "0" for all fields. Fields with equal priority will keep the original order (stable sorting).
History of previous proposals and discussions: #3452, #4254, #5827, #6111, #11241
This kind of feature has been abandoned in the past because complex proposals, and somehow rejected because ideally we should do the sorting at the view level ([customizing form](https://symfony.com/doc/current/form/form_customization.html) themes or layout templates) and it's true for most cases (the simple ones I think) but the fact is that it's often quite complex to accomplish that way, mainly for [dynamic forms](https://symfony.com/doc/current/form/dynamic_form_modification.html).
Let's focus the following analysis on explaining *why* and *when* the `priority` option could save us a lot of time, getting rid of cumbersome workarounds with templates to change the rendering order *only*.
---
A common example could be the building of a multi-steps form with a convenient type hierarchy and including fields dynamically based on the data. Let's take this sample:
![image](https://user-images.githubusercontent.com/2028198/113465635-a5a81180-9403-11eb-839f-3a32d5f84f47.png)
```php
class WorkflowType extends AbstractType
{
public function buildForm(FormBuilderInterface $builder, array $options)
{
// get current enabled transitions from the workflow definition...
foreach ($enabledTransitions as $transitionName) {
$builder->add($transitionName, SubmitType::class, ['attr' => ['value' => $transitionName]]);
}
}
}
class PersonRegistrationType extends AbstractType
{
public function buildForm(FormBuilderInterface $builder, array $options)
{
$builder
->add('firstName')
->add('lastName');
$builder->addEventListener(FormEvents::PRE_SET_DATA, function (FormEvent $event) {
$person = $event->getData();
if ($person->isLegalType()) {
$event->getForm()->add('company');
}
});
}
public function getParent()
{
return WorkflowType::class;
}
}
```
These classes model the required form. However, according to the form building process and taken into account that the underlaying person data was set with "legal" type (from the previous step), this will be the rendering result:
```html
<button type="submit" name="form[register]" value="register">Register</button> {# wrong place #}
<input name="form[first_name]">
<input name="form[last_name]">
<input name="form[company]"> {# wrong place #}
```
Now, taking the view customization path to fix the order, you likely will face some problems regarding to:
- How do I render the dynamic submit buttons at the bottom?
- How can the specific form fields be rendered in the correct order if they vary from step to step? (being it a generic template)
- if you solve the previous questions, you will also need to check if the company field is defined or not before rendering it in the right order.
There could be more and different problems depending on the context and you can find workarounds using the block prefixes system to customize each step block theme, but to me it's enough to think about other alternatives.
On the other side, using the `priority` option, the solution is quite easy. Setting the right priority value will guarantee the right rendering order:
```php
// ...
$builder->add($transitionName, SubmitType::class, ['priority' => -1, // ...]);
// ...
$event->getForm()->add('company', null, ['priority' => 1]);
```
That's ! There is no need to customize templates at all when you only need to change the fields order.
This way, these fields are being pre-defined in the order they will be rendered at view level. This sorting process will take place on `$form->createView()` method, just after `buildView()` and before `finishView()` calls.
Could we reconsider the usefulness of this feature? please.
Cheers!
Commits
-------
62650bbdc7 [Form] Add support for sorting fields
This PR was squashed before being merged into the 5.3-dev branch.
Discussion
----------
[FrameworkBundle] Add argument KernelInterface to BuildDebugContainerTrait::getContainerBuilder()
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets |
| License | MIT
| Doc PR |
This comes from a suggestion by `@yceruto` https://github.com/symfony/symfony/pull/40804#discussion_r612746851
This change will make the trait usable outside the `Command` namespace. I did not find a better namespace to move the trait to though.. This patch will help #40804
The trait is internal so the change is okey.
Commits
-------
0e9652a4bc [FrameworkBundle] Add argument KernelInterface to BuildDebugContainerTrait::getContainerBuilder()
This PR was merged into the 5.3-dev branch.
Discussion
----------
[DependencyInjection] Autowire arguments using attributes
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | N/A
| License | MIT
| Doc PR | TODO
This PR allows us to bind arguments via attributes. This mechanism is enabled for tagged iterators and service locators for now.
To receive an iterator with all services tagged with `my_tag`:
```php
use Symfony\Component\DependencyInjection\Attribute\TaggedIterator;
class MyService
{
public function __construct(
#[TaggedIterator('my_tag')]
private iterator $taggedServices,
) {
}
}
```
To receive a locator with all services tagged with `my_tag`:
```php
use Psr\Container\ContainerInterface;
use Symfony\Component\DependencyInjection\Attribute\TaggedLocator;
class MyService
{
public function __construct(
#[TaggedLocator('my_tag')]
private ContainerInterface $taggedServices,
) {
}
}
```
Commits
-------
91fbc90238 Autowire arguments using attributes
b86aa3d068 [DependencyInjection] Bind constructor arguments via attributes
This PR was squashed before being merged into the 5.3-dev branch.
Discussion
----------
[Messenger] Support Redis Cluster
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | Fix#38264
| License | MIT
| Doc PR | symfony/symfony-docs#14956
This PR brings support for Redis Cluster in the Messenger component:
- The first commit _Support RedisCluster instance_ allows to pass a `RedisCluster` object when instanciating the `Connection` class, which brings support for Redis Cluster without any friction.
- The second commit _Support multiple hosts DSN for Redis Cluster_ is more opiniated and brings a DSN format to configure a Redis Cluster from `config/packages/messenger.yaml`.
Instanciating `Connection` with a `RedisCluster` object:
```php
$redis = new \RedisCluster(null, ['host-01:6379', 'host-02:6379', 'host-03:6379', 'host-04:6379']);
$connection = new Connection([], [], [], $redis);
```
Configuring a Redis Cluster from YAML:
```yaml
// config/packages/messenger.yaml
framework:
messenger:
metadata:
default: 'redis://host-01:6379,redis://host-02:6379,redis://host-03:6379'
lazy: 'redis://host-01:6379?lazy=1,redis://host-02:6379,redis://host-03:6379'
# Configuration will be `lazy = true` and `auto_setup = true`
multipleConfig: 'redis://host-01:6379?lazy=1&auto_setup=false,redis://host-02:6379,redis://host-03:6379?auto_setup=true'
```
This format allows to define multiple hosts for a Redis Cluster and still contains valid URLs. Custom configuration is still supported, it can be specified on only one of the URLs in the DSN (see `lazy` above). If the user provides multiple configurations on different URLs, they are simply merged with the following code and if an option is defined multiple times then the latest takes precedence (see `multipleConfig` above).
I understand the way the DSN is handled could not suit you. Please, if you close this PR only for the DSN part, just tell me and I will make a new PR with only the first commit.
Commits
-------
04530fb2d7 [Messenger] Support Redis Cluster
This PR was squashed before being merged into the 5.3-dev branch.
Discussion
----------
[Config][DependencyInjection] Add configuration builder for writing PHP config
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets |
| License | MIT
| Doc PR | https://github.com/symfony/symfony-docs/pull/15181
I've spent most part of today to generate this PR. It is far from complete but it is ready for review.
This PR will build classes and store them in the build_dir. The classes will help you write PHP config. It will basically generate an array for you.
### Before
```php
// config/packages/security.php
<?php
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
return static function (ContainerConfigurator $container) {
$array = [
'firewalls' => [
'main' => [
'pattern' => '^/*',
'lazy' => true,
'anonymous' => [],
],
],
'access_control' => [
[
'path' => '^/user',
'roles' => [
0 => 'ROLE_USER',
],
],
[
'path' => '^/admin',
'roles' => 'ROLE_ADMIN',
],
],
'role_hierarchy' => [
'ROLE_ADMIN' => ['ROLE_USER'],
'ROLE_SUPER_ADMIN' => ['ROLE_ADMIN', 'ROLE_ALLOWED_TO_SWITCH',
],
],
];
$container->extension('security', $array);
}
```
### After
```php
// config/packages/security.php
<?php
use Symfony\Config\SecurityConfig;
return static function (SecurityConfig $security) {
$security
->roleHierarchy('ROLE_ADMIN', ['ROLE_USER'])
->roleHierarchy('ROLE_SUPER_ADMIN', ['ROLE_ADMIN', 'ROLE_ALLOWED_TO_SWITCH'])
->accessControl()
->path('^/user')
->role('ROLE_USER');
$security->accessControl(['path' => '^/admin', 'roles' => 'ROLE_ADMIN']);
$security->firewall('main')
->pattern('^/*')
->lazy(true)
->anonymous();
};
```
### About autogeneration
This PR is generating the extension's `ConfigBuilder`s when they are first used. Since the PR is already very large, I prefer to follow up with additional PRs to include a cache warmer and command to rebuild the `ConfigBuilder`s.
The generated `ConfigBuilder` uses a "ucfirst() camelCased" extension alias. If the alias is `acme_foo` the root `ConfigBuilder` will be `Symfony\Config\AcmeFooConfig`.
The recommended way of using this class is:
```php
// config/packages/acme_foo.php
use Symfony\Config\AcmeFooConfig;
return static function (AcmeFooConfig $foo) {
// ...
// No need to return
}
```
One may also init the class directly, But this will not help you with generation or autoloading
```php
// config/packages/acme_foo.php
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
return static function (ContainerConfigurator $container) {
$foo = new \Symfony\Config\AcmeFooConfig();
// ...
$container->extension('acme_foo', $foo->toArray());
}
```
**I do think we should only talk about the first way**
If a third party bundle like this idea and want to provide their own `ConfigBuilder`, they have two options:
1) Create the class `Symfony\Config\TheBundleConfig` themselves and make sure they configure composer to autoload that file and that the class implements `ConfigBuilderInterface`. We will never regenerate a file that already exists.
2) Create any class implementing `ConfigBuilderInterface` and ask their users to use that class in their config in the same way they would use `Symfony\Config\TheBundleConfig`.
The first way is obviously the smoothest way of doing things.
### BC
There is a great discussion about backwards compatibility for the generated files. We can assure that the class generator don't introduce a BC break with our tests. However, if the bundle changes their configuration definition it may break BC. Things like renaming, changing type or changing a single value to array is obvious BC breaks, however, these can be fixed in the config definition with normalisation.
The generator does not support normalisation. It is way way more complicated to reverse engineer that. I think a future update could fix this in one of two ways:
1) Add extra definition rules to help the class generator
2) Allow the bundle to patch part of the generated code
I hate BC breaks as much as the next person, but all the BC breaks in the generated classes will be caught when building the container (not at runtime), so I am fine with not having a 100% complete solution for this issue in the initial PR.
### Other limitations
If a bundle is using a custom extension alias, then we cannot guess it.. so a user have to use a cache warmer because we cannot generate the `ConfigBuilder` on the fly.
### TODO
- [x] Add tests
- [x] Update changelog
- [x] Write documentation
-------------
The generated code can be found in this example app: https://github.com/Nyholm/sf-issue-40600/tree/main/var/cache/dev/Symfony/Config
Commits
-------
460b46f730 [Config][DependencyInjection] Add configuration builder for writing PHP config
* 5.2:
[WebProfiler] Use ControllerReference instead of URL in twig render()
[Serializer][Validator] Update some phpDoc relative to "getters"
Update README.md
[SecurityBundle] Empty line starting with dash under "access_control" causes all rules to be skipped
[Cache] Apply NullAdapter as Null Object
* 4.4:
[Serializer][Validator] Update some phpDoc relative to "getters"
Update README.md
[SecurityBundle] Empty line starting with dash under "access_control" causes all rules to be skipped
[Cache] Apply NullAdapter as Null Object
This PR was merged into the 5.2 branch.
Discussion
----------
[WebProfilerBundle] Use ControllerReference instead of URL in twig render()
| Q | A
| ------------- | ---
| Branch? | 5.2
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#40709
| License | MIT
| Doc PR |
Use `ControllerReference` instead of `UrlGenerator`'s URL. Helps to deal with different baseUrl
Feel free to help me with some advice. Thank you in advance
Commits
-------
f2ee8bc7ae [WebProfiler] Use ControllerReference instead of URL in twig render()
This PR was squashed before being merged into the 5.3-dev branch.
Discussion
----------
[Security] Stop using a shared changelog for our security packages
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets |
| License | MIT
| Doc PR |
I understand there are historical reasons for why our four security packages share a changelog. However, I dont believe it makes much sense moving forward.
I suggest that ~~6.0~~ will start using separate changelogs.
#### Update
Lets start in 5.4 for the reasons explained by Christophe
Commits
-------
0b1103ae48 [Security] Stop using a shared changelog for our security packages
This PR was merged into the 5.3-dev branch.
Discussion
----------
[Security] [LoginLink] remove experimental
| Q | A
| ------------- | ---
| Branch? | 5.3
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | n/a
| License | MIT
| Doc PR | tbd
Symfony's new security Login Link functionality is no longer experimental as of 5.3
Commits
-------
f2842f26e7 [Security][LoginLink] remove experimental
This PR was merged into the 5.3-dev branch.
Discussion
----------
Fixed the deprecation message about Master/Main requests
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
I saw this while running tests in an app just updated to 5.x version:
```
Remaining direct deprecation notices (61)
61x: Since symfony/http-kernel 5.3: "%s()" is deprecated, use "isMainRequest()" instead.
```
Commits
-------
65f7408c61 Fixed the deprecation message about Master/Main requests