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 4.4 branch.
Discussion
----------
[Messenger] Minor fix of exception message formatting
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
See e.g. 2f27613557
Commits
-------
69187ac9ce Fixed exception message formatting
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 merged into the 4.4 branch.
Discussion
----------
[FrameworkBundle] Fix xsd definition which prevent to add more than one workflow metadata
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets |
| License | MIT
| Doc PR |
Fix xsd definition which prevent to add more than one workflow metadata (only in xml configuration format)
Commits
-------
94e390b96a [FrameworkBundle] Fix error in xsd which prevent to register more than one metadata
This PR was merged into the 4.4 branch.
Discussion
----------
[Console] work around disabled putenv()
If this is set to true, then putenv functions simply won't be run in this script, making this function usable on servers, where putenv is disabled
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | yes <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| License | MIT
| Doc PR | symfony/symfony-docs#... <!-- required for new features -->
Commits
-------
d3f909bff3 [Console] work around disabled putenv()
This PR was merged into the 3.4 branch.
Discussion
----------
[Form] Change 'cache_key' to AbstractRendererEngine::CACHE_KEY_VAR in BaseType
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| License | MIT
The purpose of this change is to find all usages of AbstractRendererEngine::CACHE_KEY_VAR. Currently, if you search for AbstractRendererEngine::CACHE_KEY_VAR you will see only access to it, i.e. (`$view->vars[AbstractRendererEngine::CACHE_KEY_VAR]`), but you can't find it in write level. With this pull request you can see where is was used for write.
Commits
-------
4fcb41c3ba Change 'cache_key' to AbstractRendererEngine::CACHE_KEY_VAR
This PR was merged into the 4.4 branch.
Discussion
----------
[PhpUnitBridge] Fix error with ReflectionClass
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| License | MIT
Introduced in #38094. Wish I know how I missed that.
Commits
-------
ed1bffeed7 [PhpUnitBridge] Fix error with ReflectionClass
This PR was merged into the 5.1 branch.
Discussion
----------
[Notifier] Fix errors parsing in FirebaseTransport
| Q | A
| ------------- | ---
| Branch? | 5.1
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | /
| License | MIT
| Doc PR | /
I noticed that, when parsing the response, this transport use `getHeader()` like a `string[]` instead of `string[][]`.
I did not test it and I'm not sure about this fix: could you have a look @Jeroeny ?
Commits
-------
2818e2cd43 Fix errors parsing in FirebaseTransport
This PR was squashed before being merged into the 4.4 branch.
Discussion
----------
[HttpClient][HttpClientTrait] don't calculate alternatives if option is auth_ntlm
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | no
| License | MIT
| Doc PR | symfony/symfony-docs#... <!-- required for new features -->
If option is `auth_ntlm` an exception is thrown, `$alternatives` then is not used.
Commits
-------
ab1a96c999 [HttpClient][HttpClientTrait] don't calculate alternatives if option is auth_ntlm
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
The purpose of this change is to find all usages of AbstractRendererEngine::CACHE_KEY_VAR. Currently, if you search for AbstractRendererEngine::CACHE_KEY_VAR you will see only access to it, i.e. (`$view->vars[AbstractRendererEngine::CACHE_KEY_VAR]`), but you can't find it in write level. With this pull request you can see where is was used for write.
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 3.4 branch.
Discussion
----------
Upgrade PHPUnit to 8.5 (php 7.2) and 9.3 (php >= 7.3)
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | yes/no
| New feature? | no
| Deprecations? | no
| Tickets | #37564
| License | MIT
| Doc PR | N/A
Our test suite on the 3.4 branch should be compatible with more recent versions of PHPUnit now. In order to make sure that it stays that way, I'm proposing to bump PHPUnit to 8.5 for the php 7.2 job and 9.3 for the php 7.3 and 7.4 jobs.
Commits
-------
ff47516ea0 Upgrade PHPUnit to 8.5 (php 7.2) and 9.3 (php >= 7.3).
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)