This PR was merged into the 5.1-dev branch.
Discussion
----------
[Security] Use new IS_* attributes in the expression language functions
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | n/a
| License | MIT
| Doc PR | n/a
#31189 has been merged which introduces some new attributes (`IS_ANONYMOUS` & friends). We can now modify the code behind the `is_*()` expression language functions to use these new attributes. This avoids any possibility of having them out of sync.
In case you - just like me - are interested why `isGranted("IS_AUTHENTICATED_FULLY")` wasn't used before: These functions were implemented without `auth_checker` being available. The auth checker variable was introduced in 4.2 by #27305, so now we can use this.
Commits
-------
3f0c599289 Use new IS_* attributes in the expression language functions
This PR was squashed before being merged into the 5.0 branch.
Discussion
----------
[Notifier] Dispatch message event in null transport
| Q | A
| ------------- | ---
| Branch? | 5.0
| Bug fix? | yes
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets | - <!-- prefix each issue number with "Fix #", if any -->
| License | MIT
| Doc PR | - <!-- required for new features -->
I think we should be able to log notifications via the `NotificationLoggerListener` even if they were sent to a null transport. The mailer component does it the same way.
Commits
-------
a0d99ce398 [Notifier] Dispatch message event in null transport
This PR was merged into the 5.1-dev branch.
Discussion
----------
[Validator] add alpha3 option to Language constraint
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | Fix https://github.com/symfony/symfony/pull/35116#issuecomment-570038901
| License | MIT
| Doc PR |
Commits
-------
ce73b98e2c add alpha3 option to Language constraint
This PR was merged into the 5.1-dev branch.
Discussion
----------
[Security] Add IS_IMPERSONATOR, IS_ANONYMOUS and IS_REMEMBERED
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| BC breaks? | no
| Deprecations? | yes
| Tests pass? | yes
| Fixed tickets | https://github.com/symfony/symfony/issues/29848
| License | MIT
| Doc PR | symfony/symfony-docs#11487
This continues work of @HeahDude and finally finishes one of the code PRs I've been working on during the ⭐️ EUFOSSA Hackathon.
Changes
---
The PRs modifies some of the attributes used by the `AuthenticatedVoter`:
* New `IS_IMPERSONATOR`, `IS_ANONYMOUS` and `IS_REMEMBERED` attributes are introduced to indicate the user either impersonated, anonymous or rembered.
* <s>`IS_AUTHENTICATED_ANONYMOUSLY` actually meant "is authenticated, either anonymous or fully". As this is confusing, it is replaced by `IS_AUTHENTICATED`.</s>
* <s>All `is_*()` functions in expressions are deprecated in favor of `is_granted('IS_*')`. It's not worth duplicating the `AuthenticatedVoter` logic in two places now we have shorter `IS_*` attributes</s>
**Before**
```php
if ($authorizationChecker->isGranted('ROLE_PREVIOUS_ADMIN')) {
// ...
}
```
<s>
```yaml
security:
# ...
access_control:
- { path: ^/protected, roles: 'IS_AUTHENTICATED_ANONYMOUSLY' }
```
</s>
**After**
```php
if ($authorizationChecker->isGranted('IS_IMPERSONATOR')) {
// ...
}
```
<s>
```yaml
security:
# ...
access_control:
- { path: ^/protected, roles: 'IS_AUTHENTICATED' }
```
</s>
<s>Discussion
---
The only thing I'm wondering is how we combine this with the `is_authenticated()` expression function:
98929dc292/src/Symfony/Component/Security/Core/Authorization/ExpressionLanguageProvider.php (L33-L37)
As you can see, the `IS_AUTHENTICATED` attribute and `is_authenticated()` expression function do not have the same meaning. Should we somehow deprecate the current behavior of `is_authenticated()` or should we find another name for `IS_AUTHENTICATED` (that would be a shame imo).</s>
Commits
-------
6c522a7d98 Added IS_ANONYMOUS, IS_REMEMBERED, IS_IMPERSONATOR
* 5.0: (28 commits)
[DoctrineBridge] Use new Types::* constants and support new json type
Fix bad merge in README of Nexmo Notifier bridge
[Debug][ErrorHandler] improved deprecation notices for methods new args and return type
[BrowserKit] Nested file array prevents uploading file
[ExpressionLanguage] Fixed collisions of character operators with object properties
remove usage of already deleted Symfony\Component\EventDispatcher\Event
[Notifier] Add correct tags for NullTransportFactory
[Validator] Remove specific check for Valid targets
[PhpUnitBridge] Use trait instead of extending deprecated class
Fix versioned namespace clears
fix remember me
Use strict assertion in asset tests
[DoctrineBridge][DoctrineExtractor] Fix indexBy with custom and some core types
Do not rely on the current locale when dumping a Graphviz object
fix typo
[Ldap] force default network timeout
[Config] don't throw on missing excluded paths
Docs: Typo, grammar
[Validator] Add the missing translations for the Polish ("pl") locale
[PhpUnitBridge] Add compatibility to PHPUnit 9 #35662
...
* 4.4: (25 commits)
[DoctrineBridge] Use new Types::* constants and support new json type
[Debug][ErrorHandler] improved deprecation notices for methods new args and return type
[BrowserKit] Nested file array prevents uploading file
[ExpressionLanguage] Fixed collisions of character operators with object properties
[Validator] Remove specific check for Valid targets
[PhpUnitBridge] Use trait instead of extending deprecated class
Fix versioned namespace clears
fix remember me
Use strict assertion in asset tests
[DoctrineBridge][DoctrineExtractor] Fix indexBy with custom and some core types
Do not rely on the current locale when dumping a Graphviz object
fix typo
[Ldap] force default network timeout
[Config] don't throw on missing excluded paths
Docs: Typo, grammar
[Validator] Add the missing translations for the Polish ("pl") locale
[PhpUnitBridge] Add compatibility to PHPUnit 9 #35662
[Routing] Add locale requirement for localized routes
[Console] Inline exact-match handling with 4.4
Set previous exception when rethrown from controller resolver
...
This PR was merged into the 4.4 branch.
Discussion
----------
[Cache] Fix versioned namespace atomic clears
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| License | MIT
When using namespace versioning to achieve atomic cache clears, only delete cache keys matching the old/current version.
This resolves tag inconsistency issues whereby the process running the clear would delete keys set against the new version by more recently spawned concurrent processes. Most seriously this could result in newly set data keys remaining, but with empty associated tag sets meaning the invalidation via tags was no longer possible.
Clearing specific prefixes is not supported when using versioned namespaces as it is desirable to clear all old keys as they will no longer be used and would otherwise eventually fill cache memory.
Commits
-------
971b177d27 Fix versioned namespace clears
This PR was merged into the 3.4 branch.
Discussion
----------
[3.4][DoctrineBridge] Use new Types::* constants and support new json type
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
All `Type::*` constants were moved and deprecated. This PR makes sure we use the good ones when they exist so we are prepared for their removal. It allows to be deprecation free. If deprecated constants could be detected, we would have failing tests 😄
Also, `json_array` was deprecated and renamed to `json`, so I added support for this new type.
Some new components also use these constants on upper branches, so I will submit PRs there.
Commits
-------
3e35fa59ea [DoctrineBridge] Use new Types::* constants and support new json type
* 3.4:
[ExpressionLanguage] Fixed collisions of character operators with object properties
[Validator] Remove specific check for Valid targets
[PhpUnitBridge] Use trait instead of extending deprecated class
fix remember me
Use strict assertion in asset tests
[DoctrineBridge][DoctrineExtractor] Fix indexBy with custom and some core types
Do not rely on the current locale when dumping a Graphviz object
fix typo
[Ldap] force default network timeout
[Config] don't throw on missing excluded paths
Docs: Typo, grammar
[Validator] Add the missing translations for the Polish ("pl") locale
[Console] Inline exact-match handling with 4.4
Set previous exception when rethrown from controller resolver
[VarDumper] fixed DateCaster not displaying additional fields
[HttpKernel] fix registering DebugHandlersListener regardless of the PHP_SAPI
This PR was merged into the 4.4 branch.
Discussion
----------
[Debug][ErrorHandler] improved deprecation notices for methods new args and return type
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | ~
| License | MIT
| Doc PR | ~
~I don't know if this should be considered a bug fix or a feature.~
Commits
-------
83d4aa7683 [Debug][ErrorHandler] improved deprecation notices for methods new args and return type
This PR was merged into the 5.0 branch.
Discussion
----------
Fix bad merge in README of Nexmo Notifier bridge
| Q | A
| ------------- | ---
| Branch? | 5.0
| License | MIT
[Bad merge](85f793bec6 (diff-ba05e8af79b613430b91ba4d4ccbea0d)) made b52b7b9fd6 end up in Nexmo README :)
Commits
-------
a8d0c5b1d7 Fix bad merge in README of Nexmo Notifier bridge
This PR was merged into the 3.4 branch.
Discussion
----------
[Validator] Remove specific check for Valid targets
| Q | A
| ------------- | ---
| Branch? | 3.4 <!-- see below -->
| Bug fix? | no
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets | N/A <!-- prefix each issue number with "Fix #", if any -->
| License | MIT
| Doc PR | N/A
As covered by `ClassMetadataTest::testAddConstraintDoesNotAcceptValid`, this check is useless, as `Valid` already accepts only properties as targets.
This check is a [leftover of a time](9b07b0c672) `Valid` was extending `Traverse` which was allowing classes & properties.
The `Valid` targets are properly checked by the lines above, the same way as other constraints.
Commits
-------
0086562c77 [Validator] Remove specific check for Valid targets
This PR was merged into the 4.4 branch.
Discussion
----------
[BrowserKit] Nested file array prevents uploading file
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets |
| License | MIT
| Doc PR | n/a
The HttpBrowser doesn't play nicely with Symfony\Component\DomCrawler::getPhpFiles().
The former assumes a flat array structure, while the latter explicitly un-flattens it, causing files to silently get ignored by the DomCrawler's submitForm.
Commits
-------
e15f05e03f [BrowserKit] Nested file array prevents uploading file
This PR was merged into the 5.0 branch.
Discussion
----------
[Notifier] Add correct tags for NullTransportFactory
| Q | A
| ------------- | ---
| Branch? | 5.0
| Bug fix? | yes
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets | - <!-- prefix each issue number with "Fix #", if any -->
| License | MIT
| Doc PR | - <!-- required for new features -->
I tried to disable the delivery of notifications in `dev` environment with the following configuration:
```
framework:
notifier:
chatter_transports:
slack: 'null://null'
texter_transports:
twilio: 'null://null'
channel_policy:
urgent: ['chat/slack', 'sms/twilio']
high: ['email']
medium: ['email']
low: ['email']
```
While sending the notification like this:
```
$notification = (new Notification())
->subject('Test subject')
->importance(Notification::IMPORTANCE_URGENT)
->content('Test content')
;
$this->notifier->send($notification);
```
I got an `UnsupportedSchemeException`: The "null" scheme is not supported.
After some digging I figured out that this Exception occurred because the `NullTransportFactory` was not tagged with the `chatter.transport_factory` and `texter.transport_factory` tags. Which is the reason the `NullTransportFactory` was not injected in the `Transport` class and so the `NullTransport` couldn't be used.
This PR should fix this Bug.
Commits
-------
1ff5e3c83f [Notifier] Add correct tags for NullTransportFactory
This PR was merged into the 5.1-dev branch.
Discussion
----------
[HttpFoundation] Fixed Mime dependency missing error
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | kinda
| New feature? | no
| Deprecations? | no
| Tickets | ~
| License | MIT
| Doc PR | ~
Follows #35642, by adding a missing exception and a note in the UPGRADE file (CHANGELOG in HttpFoundation was already up to date).
Reported in symfony/symfony-docs#1307
Commits
-------
fef0de3eb6 [HttpFoundation] Fixed Mimes dependency missing error
This PR was squashed before being merged into the 3.4 branch.
Discussion
----------
[ExpressionLanguage] Fixed collisions of character operators with object properties
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | n/a
| License | MIT
| Doc PR | n/a
Expression `foo.not in [bar]` compiles to invalid php code:
```
$foo->not in[$bar]
```
Added check for absence of a dot before of the character operators.
PS. I apologize for not starting the issue before create PR. I considered this bug is minor, but obvious.
Commits
-------
4b83ae7547 [ExpressionLanguage] Fixed collisions of character operators with object properties
This PR was merged into the 3.4 branch.
Discussion
----------
[DoctrineBridge][DoctrineExtractor] Fix indexBy with custom and some core types
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | https://github.com/symfony/symfony/issues/35542 and https://github.com/symfony/symfony/issues/35604
| License | MIT
| Doc PR | -
For https://github.com/symfony/symfony/issues/35604:
To guess the collection key type, the `getPhpType()` method is called. But it does not handle most objects and arrays core types. This is why an indexBy datetime does not work.
For https://github.com/symfony/symfony/issues/35542:
When the php type cannot be guessed, null is returned. In this case, we cannot pass a valid builtin type to PropertyInfo Type, so we should return null.
Commits
-------
018ec1ae5c [DoctrineBridge][DoctrineExtractor] Fix indexBy with custom and some core types
This PR was squashed before being merged into the 3.4 branch.
Discussion
----------
[PhpUnitBridge] Use trait instead of extending deprecated class
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | Fix#32086
| License | MIT
| Doc PR |
Use `TestListenerDefaultImplementation` instead of deprecated `BaseTestListener` for `CoverageListenerForV6`
As this is my very first pull request for this project, I'd be very glad for hints and suggestions in case I missed something.
Commits
-------
034e1de6e6 [PhpUnitBridge] Use trait instead of extending deprecated class
This PR was merged into the 5.1-dev branch.
Discussion
----------
[Form] Added support for caching choice lists based on options
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | ~
| License | MIT
| Doc PR | symfony/symfony-docs#13182
Currently, the `CachingFactoryDecorator` is responsible for unnecessary memory usage, anytime a choice option is set with a callback option defined as an anonymous function or a loader, then a new hash is generated for the choice list, while we may expect the list to be reused once "finally" configured in a form type or choice type extension.
A simple case is when using one of the core intl choice types in a collection:
```php
// ...
class SomeFormType extends AbstractType
{
public function buildForm(FormBuilderInterface $builder, array $options)
{
$builder
->add('some_choices', ChoiceType::class, [
// before: cached choice list (unnecessary overhead)
// after: no cache (better perf)
'choices' => $someObjects,
'choice_value' => function (?object $choice) { /* return some string */ },
])
// see below the nested effects
->add('nested_fields', CollectionType::class, [
'entry_type' => NestedFormType::class,
])
// ...
}
// ...
class NestedFormType extends AbstractType
{
public function buildForm(FormBuilderInterface $builder, array $options)
{
$builder
// ...
->add('some_other_choices', ChoiceType::class, [
// before: cached choice list for every entry because we define a new closure instance for each field
// after: no cache, a bit better for the same result, but much better if it were not nested in a collection
'choices' => $someOtherObjects,
'choice_value' => function (?object $otherChoice) { /* return some string */ },
])
->add('some_loaded_choices', ChoiceType::class, [
// before: cached but for every entry since every field will have its
// own instance of loader, generating a new hash
// after: no cache, same pro/cons as above
'choice_loader' => new CallbackChoiceLoader(function() { /* return some choices */}),
// or
'choice_loader' => new SomeLoader(),
])
->add('person', EntityType::class, [
// before: cached but for every entry, because we define extra `choice_*` option
// after: no cache, same pro/cons as above
'class' => SomeEntity::class,
'choice_label' => function (?SomeEntity $choice) { /* return some label */},
])
// before: cached for every entry, because the type define some "choice_*" option
// after: cached only once, better perf since the same loader is used for every entry
->add('country', CountryType::class)
// before: cached for every entry, because the type define some "choice_*" option
// after: no cache, same pro/cons as above
->add('locale', LocaleType::class, [
'preferred_choices' => [ /* some preferred locales */ ],
'group_by' => function (?string $locale, $label) { /* return some group */ },
])
// ...
```
In such cases, we would expect every entries to use the same cached intl choice list, but not, as many list and views as entries will be kept in cache. This is even worse if some callback options like `choice_label`, `choice_value`, `choice_attr`, `choice_name`, `preferred_choices` or `group_by` are used.
This PR helps making cache explicit when needed and ~deprecate~ drop unexpected implicit caching of choice list for most simple cases responsible of unnecessary overhead.
The result is better performance just by upgrading to 5.1 \o/.
But to solve the cases above when cache is needed per options, one should now use the new `ChoiceList` static methods to wrap option values, which is already done internally in this PR.
```php
use Symfony\Component\Form\ChoiceList\ChoiceList;
// ...
class NestedFormType extends AbstractType
{
public function buildForm(FormBuilderInterface $builder, array $options)
{
$builder
// explicitly shared cached choice lists between entries
->add('some_other_choices', ChoiceType::class, [
'choices' => $someOtherObjects,
'choice_value' => ChoiceList::value($this, function (?object $otherChoice) {
/* return some string */
}),
]),
->add('some_loaded_choices', ChoiceType::class, [
'choice_loader' => ChoiceList::lazy($this, function() {
/* return some choices */
})),
// or
'choice_loader' => ChoiceList::loader($this, new SomeLoader()),
]),
->add('person', EntityType::class, [
'class' => SomeEntity::class,
'choice_label' => ChoiceList::label($this, function (?SomeEntity $choice) {
/* return some label */
},
])
// nothing to do :)
->add('country', CountryType::class)
->add('locale', LocaleType::class, [
'preferred_choices' => ChoiceList::preferred($this, [ /* some preferred locales */ ]),
'group_by' => ChoiceList::groupBy($this, function (?string $locale, $label) {
/* return some group */
}),
])
// ...
```
I've done some nice profiling with Blackfire and the simple example above in a fresh website skeleton and only two empty entries as initial data, then submitting an empty form. That gives the following results:
* Rendering the form - Before vs After
<img width="714" alt="Screenshot 2020-02-16 at 9 24 58 PM" src="https://user-images.githubusercontent.com/10107633/74612132-de533180-5102-11ea-9cc4-296a16949d90.png">
* Rendering the form - Before vs After with `ChoiceList` helpers
<img width="670" alt="Screenshot 2020-02-16 at 9 26 51 PM" src="https://user-images.githubusercontent.com/10107633/74612155-122e5700-5103-11ea-9c16-5d80a7541f4b.png">
* Submitting the form - Before vs After
<img width="670" alt="Screenshot 2020-02-16 at 9 28 01 PM" src="https://user-images.githubusercontent.com/10107633/74612172-3be77e00-5103-11ea-9a18-4294e05402d2.png">
* Submitting the form - Before vs After with `ChoiceList` helpers
<img width="670" alt="Screenshot 2020-02-16 at 9 29 10 PM" src="https://user-images.githubusercontent.com/10107633/74612193-689b9580-5103-11ea-86b9-5b4906200021.png">
_________
TODO:
- [x] Docs
- [x] More profiling
- [x] Add some tests
#EUFOSSA
Commits
-------
b25973cc2e [Form] Added support for caching choice lists based on options