This PR was merged into the 5.2-dev branch.
Discussion
----------
[Form] Add new way of mapping data using callback functions
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | yes
| Tickets | Fix#37597 (partially)
| License | MIT
| Doc PR | https://github.com/symfony/symfony-docs/pull/14241
Replaces https://github.com/symfony/symfony/pull/37614
## What this solves
Objects and Forms have different mechanisms for structuring data. When you build an object model with a lot of business logic it's valuable to use these mechanisms to better collect the data and the behavior that goes with it. Doing so leads to variant schemas; that is, the object model schema and the form schema don't match up.
You still need to transfer data between the two schemas, and this data transfer becomes a complexity in its own right. If the objects know about the form structure, changes in one tend to ripple to the other.
Currently, the Data Mapper layer separates the objects from the form, transfering data between the two and also isolating them from each other. That's fine, but at present the default data mapper has a limitation: _it's very tied to one property path_ (see [`PropertyPathMapper`](https://github.com/symfony/symfony/blob/5.1/src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php)).
That said, you'll have to write your own data mapper in the following situations:
* When the property path differs for reading and writing
* When several form fields are mapped to a single method
* When you need to read data based on the model's state
* When the mapping of the model depends on the submitted form data
* ...
Also, when we create a new data mapper, we usually forget about checking the status of the given data and forms. Whether the data is empty or not; throw an exception if the given data is not an object/array and whether the form field is submitted/synchronized/disabled or not. Not doing that could lead to unwanted behavior.
## What this proposes
Create a new way to write and read values to/from an object/array using callback functions. This feature would be tied to each form field and would also mean a new way of mapping data, but a very convenient one, in which it won't be necessary to define a new data mapper and take into account all what it would imply when you only need to map one field in a different manner or perhaps in only one direction (writing or reading the value).
This PR adds two new options for each form type: `getter` and `setter`, allowed to be `null` or `callable`:
```php
$builder->add('name', TextType::class, [
'getter' => function (Person $person, FormInterface $form): string {
return $person->firstName().' '.$person->lastName();
},
'setter' => function (Person &$person, ?string $name, FormInterface $form): void {
$person->rename($name);
},
]);
```
This would give us the same possibilities as data mappers, but within the form field scope, where:
* `$person` is the view data, basically the underlying data to the form.
* `$form` is the current child form that is being mapped.
* `$name` is the submitted data that belongs to that field.
These two callbacks will be executed following the same rules as for property paths before read and write any value (e.i. early return if empty data, skip mapping if the form field is not mapped or it's disabled, etc).
## What this also proposes
I based the implementation on solving this problem first:
> https://github.com/symfony/symfony/pull/37614#issuecomment-662957865
> [...] the property_path option defines the rules on how it's accessed. From there, the actual way it is accessed (direct property access, accessors, reflection, whatever) are hidden from view. All that matters is the property path (which is deduced from the name if not explicitly set). [...]
So splitting the default data mapper `PropertyPathMapper` into two artifacts: "[DataMapper](https://github.com/yceruto/symfony/blob/data_accessor/src/Symfony/Component/Form/Extension/Core/DataMapper/DataMapper.php)" and "[DataAccessor](https://github.com/yceruto/symfony/blob/data_accessor/src/Symfony/Component/Form/DataAccessorInterface.php)" would allow us adding multiple data accessors along the way (the code that varies in this case) without having to reinvent the wheel over and over again (the data mapper code).
You can also think about a new `ReflectionAccessor` for instance? or use this `CallbackAccessor` to map your form partially from an external API? yes, you could do it :)
Here is a view of the proposed changes:
![data_accessor](https://user-images.githubusercontent.com/2028198/91452859-16bf8f00-e84d-11ea-8564-d497c2f73730.png)
Where "DataMapper" will take care of common checks, iterates the given child forms, manages the form data and all what is needed for mapping a standard form, whereas "DataAccessor" will take care of how to read and write values to/from the underlying object or array.
## BC
The `PropertyPathMapper` is being deprecated in favor of `DataMapper` class, which uses the `PropertyPathAccessor` by default.
Although `DataMapper` is now the default for each compound form, the behavior must remains the same (tests prove it). So that if `getter` or `setter` option is null (they're by default) the `CallbackAccessor` will falls back to `PropertyPathAccessor` either for reading or writing values.
---
Sorry for the long description, but I think that sometimes it is necessary for too complex issues and big changes. Besides, now you know a little more about what these changes is about.
/cc @xabbuh as creator of [rich-model-forms-bundle](https://github.com/sensiolabs-de/rich-model-forms-bundle) and @alcaeus as you had worked on this issue.
WDYT?
Commits
-------
878effaf47 add new way of mapping data using callback functions
This PR was merged into the 5.2-dev branch.
Discussion
----------
[String] allow translit rules to be given as closure
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | Fix#35061
| License | MIT
| Doc PR | -
Instead of trying to fix#35061 at our level, I propose to add a hook so that ppl can use a transliterator that fits their need (eg voku/portable-ascii or behat/transliterator) while still relying on the `SluggerInterface`.
Commits
-------
0bb48df2b0 [String] allow translit rules to be given as closure
This PR was squashed before being merged into the 5.2-dev branch.
Discussion
----------
[Mailer] Mailjet - Allow using Reply-To with Mailjet API
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | none
| License | MIT
| Doc PR | none
Using this bridge I got an error from Mailjet API given below and this PR fixes this issue.
<details>
<summary>Error from Mailjet</summary>
```json
{
"Messages": [
{
"Status": "error",
"Errors": [
{
"ErrorIdentifier": "xxxx",
"ErrorCode": "send-0011",
"StatusCode": 400,
"ErrorMessage": "Header cannot be customized using the \"Headers\" collection. Please use the dedicated property to set this header.",
"ErrorRelatedTo": [
"Headers[\"Reply-To\"]"
]
}
]
}
]
}
```
</details>
As detailed in [the Mailjet API doc](https://dev.mailjet.com/email/reference/send-emails/index.html#v3_1_post_send), `ReplyTo` is a single email address whereas the `Symfony\Mime\Email` class allows for several emails in `ReplyTo`.
I implemented the [same logic as the Sendgrid bridge](0f5ac5dc8f/src/Symfony/Component/Mailer/Bridge/Sendgrid/Transport/SendgridApiTransport.php (L96)) but added logging (I first thought about throwing an exception but this would reduce transport interchangeability).
Note : `Reply-To` is not listed in the ["Forbidden headers" section of Mailjet's documentation](https://dev.mailjet.com/email/guides/send-api-v31/#add-email-headers) but the API response is clear about the expected input.
Commits
-------
1ff3e293e0 [Mailer] Mailjet - Allow using Reply-To with Mailjet API
This PR was merged into the 4.4 branch.
Discussion
----------
[Messenger][minor ]Improve connection message readability (4.4)
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets |
| License | MIT
| Doc PR |
Same as #38185 but for 4.4.
I created 2 PR since bridges directory has moved in 5.1 (to ease merge)
Commits
-------
63bc620efa [Messenger] Improve error message readability
This PR was merged into the 4.4 branch.
Discussion
----------
[FrameworkBundle] fix filename being cleaned up at end of tests
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets |
| License | MIT
| Doc PR |
Commits
-------
47a5a93e3e fix filename being cleaned up at end of tests
This PR was merged into the 5.2-dev branch.
Discussion
----------
[Notifier] fix factory definition of notifier transports
| Q | A
| ------------- | ---
| Branch? | master <!-- see below -->
| Bug fix? | yes
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets | #37298<!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License | MIT
| Doc PR | - <!-- required for new features -->
With a fresh install of Symfony `symfony new --version=next` and after adding the Notifier component `composer req symfony/notifier` I got the following errors:
```
In PhpDumper.php line 1843:
Cannot dump definition because of invalid class name ('chatter.transport_factory').
```
and
```
In PhpDumper.php line 1843:
Cannot dump definition because of invalid class name ('texter.transport_factory').
```
This PR fixes this bug.
Commits
-------
e8eed5bb34 fix factory definition of notifier transports
* 5.1:
Internal classes are not legacy.
[HttpFoundation] Skip the cookie_max_age fixture on PHP 8.
add choice_translation_domain tests to prevent further regressions
Update validators.tr.xlf
* 4.4:
Internal classes are not legacy.
[HttpFoundation] Skip the cookie_max_age fixture on PHP 8.
add choice_translation_domain tests to prevent further regressions
Update validators.tr.xlf
* 3.4:
[HttpFoundation] Skip the cookie_max_age fixture on PHP 8.
add choice_translation_domain tests to prevent further regressions
Update validators.tr.xlf
This PR was merged into the 3.4 branch.
Discussion
----------
[DoctrineBridge] add choice_translation_domain tests to prevent further regressions
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | Fix #https://github.com/symfony/symfony/pull/37521#issuecomment-678247192
| License | MIT
| Doc PR |
Commits
-------
7775b3707b add choice_translation_domain tests to prevent further regressions
This PR was submitted for the master branch but it was merged into the 4.4 branch instead.
Discussion
----------
[PhpUnitBridge] Internal classes are not legacy
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | #37564
| License | MIT
| Doc PR | N/A
This change works around the issue that we can observe in the failed Travis build of #38103.
We must not call PHPUnit's internal `Test::getGroups()` method with a built-in class, otherwise we will run into a TypeError. This won't be fixed on PHPUnit's side, so we need to prevent that call. Our DeprecationErrorHander might run into this case if a deprecation is triggered while autoloading a class.
And forgive me, I've had a really hard time trying to craft a test case for that. 🙈
Commits
-------
7d55e0c065 Internal classes are not legacy.
This PR was merged into the 3.4 branch.
Discussion
----------
[HttpFoundation] Skip the cookie_max_age fixture on PHP 8
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | #36872
| License | MIT
| Doc PR | N/A
This PR suggest to skip a certain integration test on php 8. The integration test in question triggers a warning by setting a way too high expiration date when calling `setcookie()`. Apparently, this warning has been upgraded to a fatal error on php 8.
Since the integration test is run in a separate process, we might as well adjust the expectation of that test case, but I don't really see the point in asserting fatal error behavior, to be honest.
Commits
-------
d6d9b2927d [HttpFoundation] Skip the cookie_max_age fixture on PHP 8.
This PR was squashed before being merged into the 5.2-dev branch.
Discussion
----------
[RFC][HttpKernel][Security] Allowed adding attributes on controller arguments that will be passed to argument resolvers.
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | Fix#29692
| License | MIT
| Doc PR | Not yet, will do it it this PR is accepted.
This PR allow to configure argument resolvers using PHP8 attributes.
This is basically a fix for #29692 but using a different strategy that the one proposed in the issue:
- it uses PHP attributes instead of doctrine annotation since they can be added directly on method arguments.
- it uses a simpler design by just adding the attribute to the `ArgumentMetadata` and let the individual resolvers decide if they want to react to the presence of the attribute.
- it uses an attributes classe per use-case instead of an unique `Arg` annotation.
As an example, I've added (in the second commit) a `CurrentUser` attribute that allows the `UserValueResolver` to always inject the current user even if the argument is not typed with the `UserInterface` (to allow typing your actual class instead for example).
This would allow to do things like this:
```php
class MyController
{
public function indexAction(#[CurentUser] MyUser $user)
{
}
}
```
Commits
-------
20f316906e [RFC][HttpKernel][Security] Allowed adding attributes on controller arguments that will be passed to argument resolvers.
This PR was squashed before being merged into the 5.2-dev branch.
Discussion
----------
[RFC][lock] Introduce Shared Lock (or Read/Write Lock)
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | /
| License | MIT
| Doc PR | TODO
This PR adds a new method "acquireRead" to the Lock class in order to solve the [single writer multiple readers](https://en.wikipedia.org/wiki/Readers%E2%80%93writer_lock) problem.
usage:
```php
$aliceLock = $factory->createLock('invoice');
$bobLock = $factory->createLock('invoice');
$oscarLock = $factory->createLock('invoice');
$aliceLock->acquireRead(); // true
$bobLock->acquireRead(); // true
$oscarLock->acquire(); // false
```
## next steps
### add more stores
- pdo
- memcached
- zookeeper
- mongodb
### Priority policies
Priority policy (read-preffering or write preffering) is not covered by this PR.
## Promote/Demote
Converting a Read lock to Write Lock (promote) or Write lock to Read lock (demote) is covered by calling `acquireRead` / `acquired` method.
```php
// demote
$lock->acquire();
...
$lock->acquireRead();
// promote
$lock->acquireRead();
...
$lock->acquire();
```
Commits
-------
a7d51937f0 [RFC][lock] Introduce Shared Lock (or Read/Write Lock)
This PR was merged into the 5.2-dev branch.
Discussion
----------
[Messenger] - Add option to confirm message delivery in Amqp connection
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
Hello! My first PR here.
Sometimes we need to be sure that messages are delivered to Amqp. To make it work we need to wait for the confirmation from the Amqp server. So let's wait for it confirmation if confirmation timeout is defined. Default behaviour are not modified - waiting for confirmation will impact performance of sending message.
Commits
-------
5682d76b2a Messenger - Add option to confirm message delivery in Amqp connection
This PR was merged into the 5.2-dev branch.
Discussion
----------
[Cache] add integration with Messenger to allow computing cached values in a worker
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | -
| License | MIT
| Doc PR | -
~~This PR needs and for now embeds #30334. See 2nd commit.~~
Using the new `CacheInterface` enables probabilistic early expiration by default. This means that from time to time, items are elected as early-expired while they are still fresh ([see Wikipedia](https://en.wikipedia.org/wiki/Cache_stampede#Probabilistic_early_expiration) for details).
This PR adds a new `early_expiration_message_bus` option when configuring cache pools. When this option is set, cache pools are configured to send those early-expired items on a Messenger bus, then serve the current value immediately, while the updated value is computed in a worker in the background.
`CacheInterface::get($key, $callback)` accepts any callable, but sending any callable on a bus is not possible (e.g. a `Closure` cannot be serialized). To bypass this constraint, this feature works only with callables in the form `[$service, 'publicMethod']`, where `$service` is any public or [reversible service](https://github.com/symfony/symfony/pull/30334).
This constraint is a serious one: this $service must compute a value when knowing only its key. This means keys should embed enough information for this to happen. I think that's not that hard - and we may find ways to provide additional context in the future.
At least the goal is worth it: in theory, this strategy allows achieving a 100% hit ratio even when invalidation-by-expiration is used.
There are two things one needs to do to enable this behavior:
1. bind a message bus to a cache pool:
```yaml
framework:
cache:
pools:
test.cache:
early_expiration_message_bus: messenger.default_bus
```
2. route EarlyExpirationMessage to a transport:
```yaml
framework:
messenger:
routing:
'Symfony\Component\Cache\Messenger\EarlyExpirationMessage': amqp
```
Commits
-------
6c0911f58c [Cache] add integration with Messenger to allow computing cached values in a worker
This PR was squashed before being merged into the 5.2-dev branch.
Discussion
----------
[SecurityBundle] Comma separated ips for security.access_control
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets |
| License | MIT
| Doc PR | https://github.com/symfony/symfony-docs/pull/14219
There is currently no way to use env vars to configure `security.access_control` ips with multiple values. Ability to use comma separated ips make it able.
Commits
-------
0412e91060 [SecurityBundle] Comma separated ips for security.access_control
This PR was merged into the 5.1 branch.
Discussion
----------
[Security] In the new authenticator system, no auth listener is valid
| Q | A
| ------------- | ---
| Branch? | 5.1
| Bug fix? | yes
| New feature? | yes
| Deprecations? | no
| Tickets | none
| License | MIT
| Doc PR | not needed
In the new authenticator system, `anonymous` is gone and it IS now valid to have no authentication listeners. Before this PR, the following would trigger this error:
```
security:
enable_authenticator_manager: true
firewalls:
main:
lazy: true
```
But this IS valid (and would eventually be the "starting security.yaml" when the new system is the main system).
Cheers!
Commits
-------
6b520db2eb In the new authenticator system, no auth listener is valid
This PR was submitted for the master branch but it was merged into the 3.4 branch instead.
Discussion
----------
Update validators.tr.xlf
Turkish Spelling error corrected.
| Q | A
| ------------- | ---
| Branch? | 4.4 5.1 <!-- 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 | Fix #... <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License | MIT
| Doc PR | symfony/symfony-docs#... <!-- 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/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 master.
-->
Commits
-------
f346eccb60 Update validators.tr.xlf
* 5.1:
fix merge
[Cache] fix previous PR
add mising sr (latn & cyrl) translations
[Cache] fix ProxyAdapter not persisting items with infinite expiration
[HttpClient] fail properly when the server replies with HTTP/0.9
Fix CS
[Cache] Limit cache version character range
[Mailer] Fixed Mailgun API bridge JsonException when API response is not applicaton/json
[String] improve fix
fix tests
[DI] dump OS-indepent paths in the compiled container
[DI] dump OS-indepent paths in the preload file
Run postgres setup trigger in transaction
allow consumers to mock all methods
* 4.4:
add mising sr (latn & cyrl) translations
[Cache] fix ProxyAdapter not persisting items with infinite expiration
[HttpClient] fail properly when the server replies with HTTP/0.9
Fix CS
[Cache] Limit cache version character range
[DI] dump OS-indepent paths in the compiled container
allow consumers to mock all methods