This PR was squashed before being merged into the 5.1-dev branch.
Discussion
----------
Automatically provide Messenger Doctrine schema to "diff"
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | Alternative to #36629
| License | MIT
| Doc PR | TODO - WILL be needed
This follows this conversation: https://github.com/symfony/symfony/pull/36629#issuecomment-621745821 - it automatically adds SQL to Doctrine's migration/diff system when features are added the require a database table:
The new feature works for:
### A) Messenger Doctrine transport
**FULL support**
Works perfectly: configure a doctrine transport and run `make:migration`
**Note**: There is no current way to disable this. So if you have `auto_setup` ON and you
run `make:migration` before trying Messenger, it will generate the table SQL. Adding a
flag to disable it might be very complicated, because we need to know (in DoctrineBundle, at compile time) whether or not this feature is enabled/disabled so that we can decide *not* to add `messenger_messages` to the `schema_filter`.
### B) `PdoAdapter` from Cache
**FULL support**
Works perfectly: configure a doctrine transport and run `make:migration`
### C) `PdoStore` from Lock
**PARTIAL support**
I added `PdoStore::configureSchema()` but did NOT add a listener. While `PdoStore` *does* accept a DBAL `Connection`, I don't think it's possible via the `framework.lock` config to create a `PdoStore` that is passed a `Connection`. In other words: if we added a listener that called `PdoStore::configureSchema` if the user configured a `pdo` lock, that service will *never* have a `Connection` object... so it's kind of worthless.
**NEED**: A proper way to inject a DBAL `Connection` into `PdoStore` via `framework.lock` config.
### D) `PdoSessionHandler`
**NO support**
This class doesn't accept a DBAL `Connection` object. And so, we can't reliably create a listener to add the schema because (if there are multiple connections) we wouldn't know which Connection to use.
We could compare (`===`) the `PDO` instance inside `PdoSessionHandler` to the wrapped `PDO` connection in Doctrine. That would only work if the user has configured their `PdoSessionHandler` to re-use the Doctrine PDO connection.
The `PdoSessionHandler` *already* has a `createTable()` method on it to help with manual migration. But... it's not easy to call from a migration because you would need to fetch the `PdoSessionHandler` service from the container. Adding something
**NEED**: Either:
A) A way for `PdoSessionHandler` to use a DBAL Connection
or
B) We try to hack this feature by comparing the `PDO` instances in the event subscriber
or
C) We add an easier way to access the `createTable()` method from inside a migration.
TODOs
* [X] Determine service injection XML needed for getting all PdoAdapter pools
* [ ] Finish DoctrineBundle PR: https://github.com/doctrine/DoctrineBundle/pull/1163
Commits
-------
2dd9c3c3c8 Automatically provide Messenger Doctrine schema to "diff"
This PR was squashed before being merged into the 5.1-dev branch.
Discussion
----------
[ExpressionLanguage] Added expression language syntax validator
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | #35700
| License | MIT
| Doc PR | N/A <!-- required for new features -->
Proposal implementation #35700
The current solution is a compromise between support complexity and cleanliness.
I tried different solutions to the issue. A beautiful solution was obtained only with full duplication of the parser code. That is unacceptable because parser complexity is quite high.
The main problem in this solution is that nodes instances are created which are then not used. I do not think that linter can be a bottleneck and will greatly affect performance. If this is corrected, the parser code becomes a bunch of if's.
JFI: I did not added parsing without variable names, because this breaks caching and potential location for vulnerabilities.
Commits
-------
a5cd965494 [ExpressionLanguage] Added expression language syntax validator
This PR was merged into the 5.1-dev branch.
Discussion
----------
[Security/Core] Add CustomUserMessageAccountStatusException
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| License | MIT
| Doc PR | Not really needed
When implementing the `UserCheckerInterface`, we can throw `AccountStatusException`. Similar to `CustomUserMessageAuthenticationException`, this exception allow to throw an `AccountStatusException` with a custom message.
Commits
-------
9233efbe06 Add CustomUserMessageAccountStatusException
This PR was merged into the 3.4 branch.
Discussion
----------
[PhpUnitBridge] Mark parent class also covered in CoverageListener
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets |
| License | MIT
| Doc PR |
Commits
-------
dcb5653728 [PhpUnitBridge] Mark parent class also covered in CoverageListener
This PR was squashed before being merged into the 5.1-dev branch.
Discussion
----------
[Messenger] Add support for RecoverableException
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | N/A
| License | MIT
| Doc PR | N/A
The messenger supports the `UnrecoverableException` preventing the messenger retry mechanism
when the Handler will never be able to process the Message.
This PR adds the opposite behavior to always retry the message.
UseCase:
- High concurency Consumers use non-blocking lock
- 503/429 errors from 3rd party API
Commits
-------
e7c31675f7 [Messenger] Add support for RecoverableException
This PR was merged into the 5.0 branch.
Discussion
----------
[5.0] Use PHP 7.2 minimum in tests run with github actions
| Q | A
| ------------- | ---
| Branch? | 5.0
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | Re #36647
| License | MIT
| Doc PR | -
Commits
-------
8b386f2e81 Use PHP 7.2 minimum in tests run with github actions
* 5.0: (26 commits)
[Filesystem] Handle paths on different drives
[WebProfiler] Do not add src-elem CSP directives if they do not exist
[Yaml] fix parse error when unindented collections contain a comment
Execute docker dependent tests with github actions
Update exception.html.php
[3.4][Inflector] Improve testSingularize() argument name
[Inflector] Fix testPluralize() arguments names
[PhpUnitBridge] fix PHP 5.3 compat again
Skip validation when email is an empty object
fix sr_Latn translation
[Validator] fix lazy property usage.
Fix annotation
[Debug][ErrorHandler] cleanup phpunit.xml.dist files
[Translation] Fix for translation:update command updating ICU messages
[PhpUnitBridge] fix compat with PHP 5.3
bumped Symfony version to 5.0.9
updated VERSION for 5.0.8
updated CHANGELOG for 5.0.8
bumped Symfony version to 4.4.9
updated VERSION for 4.4.8
...
* 4.4: (23 commits)
[Filesystem] Handle paths on different drives
[WebProfiler] Do not add src-elem CSP directives if they do not exist
[Yaml] fix parse error when unindented collections contain a comment
Execute docker dependent tests with github actions
Update exception.html.php
[3.4][Inflector] Improve testSingularize() argument name
[Inflector] Fix testPluralize() arguments names
[PhpUnitBridge] fix PHP 5.3 compat again
Skip validation when email is an empty object
fix sr_Latn translation
[Validator] fix lazy property usage.
Fix annotation
[Debug][ErrorHandler] cleanup phpunit.xml.dist files
[Translation] Fix for translation:update command updating ICU messages
[PhpUnitBridge] fix compat with PHP 5.3
bumped Symfony version to 4.4.9
updated VERSION for 4.4.8
updated CHANGELOG for 4.4.8
provide a useful message when extension types don't match
[Cache] Fixed not supported Redis eviction policies
...
* 3.4:
[Filesystem] Handle paths on different drives
[WebProfiler] Do not add src-elem CSP directives if they do not exist
[Yaml] fix parse error when unindented collections contain a comment
[3.4][Inflector] Improve testSingularize() argument name
[PhpUnitBridge] fix PHP 5.3 compat again
Skip validation when email is an empty object
fix sr_Latn translation
[Validator] fix lazy property usage.
Fix annotation
[PhpUnitBridge] fix compat with PHP 5.3
[DX] Show the ParseException message in YAML file loaders
This PR was merged into the 4.4 branch.
Discussion
----------
[Console] Default hidden question to 1 attempt for non-tty session
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#36565
| License | MIT
| Doc PR |
### Problem 1
`validateAttempts()` method repeats validation forever by default, until exception extending `RuntimeException` isn't thrown. This currently happens disregarding if user is in tty session where they can actually type input, or non-tty session. This presents a problem when user code throws custom exceptions for hidden questions -> loop doesn't stop. As far as I can tell this issue is in all Symfony versions, but it was uncovered only after we stopped marking interactive flag to false automatically ourselves. Actually, all 3 problems were already existing problems, just hidden until now.
### Problem 2
Infinite loop problem is related to hidden questions, but this one isn't. If validation fails, another attempt to read & validate happens. This means user will get two prompts: 2x same question with 2 different error messages. One error message coming from validator, second error message about inability to read input (because this loop repeats until this kind of error happens, so last output will always be this error). As an example, output in practice would look like following
```
What do you want to do:
>
[ERROR] Action must not be empty.
What do you want to do:
>
Aborted.
```
So even if loop stops, output is more than expected.
### Problem 3
This is purely cosmetic issue, but currently user gets `stty: stdin isn't a terminal` printed additionally when question helper tries to ask a hidden question without having tty. I have fixed this in same fashion as was already done for [getShell() method](ee7fc5544e/src/Symfony/Component/Console/Helper/QuestionHelper.php (L500)).
### More details
Well root of the first problem is that `\Symfony\Component\Console\Helper\QuestionHelper::getHiddenResponse` is inconsistent. In some cases it does throw `MissingInputException` (which extends `RuntimeException`), in others doesn't. This is because in others, `shell_exec` is used, which won't return `false` even in non-tty sessions. Initially I attempted to fix this and make them consistent by checking for empty result + `isTty` call, but during my testing I found that at least last, `bash -c` method returns `\n` as output both when passing empty input and when passing newline as input. This means we cannot differentiate with this technique when input is really empty, or at least I can't currently tell how, maybe someone does. I had also idea to use proc_open and check if `STDERR` cotains message about stdin not being a terminal, but I realized these functions might not be available. In future we should modernize this method to use less hacky techniques. Other solutions, eg. Inquirer.js or [hoa/console](https://github.com/hoaproject/Console/blob/master/Source/Readline/Readline.php) have much more elegant solutions. Anyway, since I encountered this issue and additionally this doesn't solve Problem 2, I stopped trying to fix this on this level.
### Alternative solution
Alternative solution to problem 1 and 3 would be to fallback to default in case of hidden questions when tty is missing. But this still doesn't solve problem 2 and I can't think about solution right now which would fix problem 2 separately. We also didn't really reach consensus if reading passwords via stdin is desired. I tried this in `Inquirer.js` and this library *does read password from stdin*
Commits
-------
ee7fc5544e [Console] Default hidden question to 1 attempt for non-tty session
This PR was squashed before being merged into the 3.4 branch.
Discussion
----------
[Filesystem] Handle paths on different drives
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| License | MIT
`makePathRelative` strips and ignores the drive letters given Windows paths on different drives, resulting in a relative path which does not resolve to the desired target.
This PR makes `makePathRelative` notice paths on different drives, and return the full (absolute) target path in case instead.
Commits
-------
00e727ae4e [Filesystem] Handle paths on different drives
This PR was merged into the 5.1-dev branch.
Discussion
----------
[DependencyInjection] Add a mechanism to deprecate public services to private
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
This PR adds a mechanism to easily deprecate public services to private, ie a public service will be private in a next version.
It works with a tag and a compiler pass. The pass creates a deprecated public alias of the public service.
It is ran "after removing" because we want to trigger only on direct accesses to the public service (`$container->get()`): if the public service was removed, we don't need to do anything because there is no impact.
~All references to the public service are kept because the deprecated public alias is created after `ResolveReferencesToAliasesPass` so only direct access actually uses the deprecated alias.~
All references to the public service are altered to a direct reference to the alias target service so that they don't trigger the alias deprecation.
Thanks to @nicolas-grekas for sharing some ideas and helping me !
Commits
-------
3e80e461a9 [DependencyInjection] Add a mechanism to deprecate public services to private
This PR was merged into the 3.4 branch.
Discussion
----------
[WebProfiler] Do not add src-elem CSP directives if they do not exist
| Q | A
| ------------- | ---
| Branch? | 3.4, 4.4, 5.0
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#36643
| License | MIT
| Doc PR | n/a
In the latest 3.4.*, 4.4.* and 5.0.* branches the `script-src-elem` and `style-src-elem` directives are added to the Content-Security-Policy header if they don't exist by copying the `default-src`. This causes browsers to ignore the `script-src` and `style-src` directives which likely contain scripts and styles the developer wanted to allow.
As mentioned in the fixed ticket, we shouldn't be adding these directives if they don't exist because the browser will automatically fallback to `script-src` and `style-src` which we have already added `unsafe-inlen` and the `nonce-*` to.
This will need to be merged into 3.4, 4.4 and 5.0, but I was unsure which branch I am meant to base it off to start with. I've put it on 4.4 but can move it to another if required.
Commits
-------
d9c47087c9 [WebProfiler] Do not add src-elem CSP directives if they do not exist
This PR was merged into the 3.4 branch.
Discussion
----------
[DX] Show the ParseException message in all YAML file loaders
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
This PR synchronizes the exception message in the Routing, Validator and Translation YAML file loaders with the DependencyInjection YAML file loader behavior. Adding the ParseException message is a big DX gain because it highlights the problem directly instead of having to scroll down 7 previous exceptions.
I'm targetting 3.4 because DX can be considered as a bug fix AFAIK.
Commits
-------
fc6cf3d3c6 [DX] Show the ParseException message in YAML file loaders
This PR was squashed before being merged into the 3.4 branch.
Discussion
----------
[Yaml] fix parse error when unindented collections contain a comment
| Q | A
| ------------- | ---
| Branch? | 5.0
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#36558
| License | MIT
### Problem
The method `Parser::getNextEmbedBlock` did not determine the yaml-block correctly when there was a comment before the first unindented collection item. This was caused by the fact that the check for unindented collection items was done for the _first line of the block only_. So in case this first line is a comment, this check will result in _false_, while in fact the parser is in an unindented collection.
### Solution
In the solution I implemented the parser will check for comment lines as well. As long as the loop encounters a comment line, it will check (in the next iteration) whether the line is an unindented collection item. So this check will be done until all comments before the first uncommented item are parsed.
Commits
-------
58bb2c52ac [Yaml] fix parse error when unindented collections contain a comment
This PR was merged into the 4.4 branch.
Discussion
----------
Execute docker dependent tests with github actions
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | Fixes#36427
| License | MIT
| Doc PR | -
* redis, memcached, rabbitmq and vulcain dependent tests moved to the github action
* run on PHP 7.1 and 7.4 only
* use the `integration` group for all tests that depend on docker services
* do not exclude the `integration` group on Travis, but make sure tests that depend on docker services are skipped properly
[<img width="1222" alt="image" src="https://user-images.githubusercontent.com/190447/80806323-48339100-8bb2-11ea-95cd-5ce773c74ce6.png">](https://github.com/jakzal/symfony/runs/636461875?check_suite_focus=true)
Commits
-------
d710c1b654 Execute docker dependent tests with github actions
This PR was merged into the 3.4 branch.
Discussion
----------
[Validator] Skip validation when email is an empty object
| Q | A
| ------------- | ---
| Branch? | 3.4
| 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 -->
When the value passed to the email validator is an empty object the validator is still called and will mark the value as invalid. The object should be skipped in this case, as it is also done in the `UrlValidator`
bfdbb244fe/src/Symfony/Component/Validator/Constraints/UrlValidator.php (L59-L62)
<!--
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
-------
de5d68ef2a Skip validation when email is an empty object
This PR was submitted for the master branch but it was merged into the 4.4 branch instead.
Discussion
----------
Update exception.html.php
| Q | A
| ------------- | ---
| Branch? | master for features / 3.4, 4.4 or 5.0 for bug fixes <!-- 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 | NIL <!-- prefix each issue number with "Fix #", if any -->
| 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.
-->
Removed the second use of `count` and replaced with the result of the first call to `count()`.
In other words: the use of `$last = count($exceptionAsArray) - 1;` is rewritten as `$last = $exceptionAsArrayCount - 1;` to use the result of the last count.
Commits
-------
1c9162d2ad Update exception.html.php
This PR was merged into the 4.4 branch.
Discussion
----------
[Inflector] Fix testPluralize() arguments names
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
This is minor but the testPluralize() method arguments names are inversed ($plural contains the singular, $singular contains the plural, etc.). I lost some time on this because I thought the test was wrong 😅 I guess it's better to have a test that is easier to understand.
Commits
-------
fb42f98315 [Inflector] Fix testPluralize() arguments names
Javier Eguiluz
Fabien Potencier
Ryan Weaver
Andrey Sevastianov
Laurent VOULLEMIER
Grégoire Pineau
azjezz
Christian Flothmann
Jérémy Derussé
Nicolas Grekas
Jakub Zalas
Christian Rishøj
Thomas Calvet
Nathan Dench
Wouter Diesveld
Olatunbosun Egberinde
