[DoctrineBridge] Fix bug with date immutable and datetime immutable #39469
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…l-intl-icu (nicolas-grekas) This PR was merged into the 5.x branch. Discussion ---------- [Intl] deprecate polyfills in favor of symfony/polyfill-intl-icu | Q | A | ------------- | --- | Branch? | 5.x | Bug fix? | no | New feature? | no | Deprecations? | yes | Tickets | Fix #37758 | License | MIT | Doc PR | - Follows symfony/polyfill#310 /cc @stof Commits ------- 6ad0169 [Intl] deprecate polyfills in favor of symfony/polyfill-intl-icu
* 5.2: Bump branch-version Bump Symfony version to 5.2.0 Update VERSION for 5.2.0-RC1 Update CHANGELOG for 5.2.0-RC1
…Nyholm) This PR was squashed before being merged into the 5.3-dev branch. Discussion ---------- [Messenger][SQS] Make sure one can enable debug logs | Q | A | ------------- | --- | Branch? | 5.x | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | | License | MIT | Doc PR | Needed If you add `&debug=true` on your DSN, then we will use `LoggerInterface::debug()` to print HTTP requests and responses. This has a negative impact on performance, but it will be helpful when debugging. Commits ------- 66edc59 [Messenger][SQS] Make sure one can enable debug logs
…it client (alexander-schranz)
This PR was squashed before being merged into the 5.3-dev branch.
Discussion
----------
[BrowserKit] Add jsonRequest function to the browser-kit client
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | Fix -
| License | MIT
| Doc PR | symfony/symfony-docs#...
If you use the FOSRestBundle for your Api's you have maybe many tests using just:
```php
$client->request('POST', '/api/contacts', ['param' => 1]);
```
To test your JSON api as in the real browser request FOSRestBundle converts the json body into the `$request->request` object. I think something similar is done by ApiPlatform. If you have tests like above they will now fail as the integer is converted to string see also #38591.
This PR add a new `jsonRequest` which will look like the following and will so fix the above problem:
```php
$client->jsonRequest('POST', '/api/contacts', ['param' => 1]);
```
Commits
-------
c2fa2cb [BrowserKit] Add jsonRequest function to the browser-kit client
* 5.2: [Filesystem] fix cleaning up tmp files when dumpFile() fails [MimeType] Add missing alias for @mime_type
* 5.2: Add tests on CacheDataCollector [ProxyManagerBridge] fix tests [ProxyManagerBridge] relax fixture in tests Fix circular detection with multiple paths
* 5.2: prevent hash collisions caused by reused object hashes autoconfigure behavior describing tags on decorators [Validator][RecursiveContextualValidator] Prevent validated hash collisions
…sport injectable (jacekwilczynski) This PR was merged into the 5.3-dev branch. Discussion ---------- [Messenger] Make all the dependencies of AmazonSqsTransport injectable | Q | A | ------------- | --- | Branch? | 5.x for features | Bug fix? | no | New feature? | yes - updated changelog | Deprecations? | no | Tickets | Fix #38640 | License | MIT | Doc PR | symfony/symfony-docs#... <!-- required for new features --> This is a pure refactoring PR that enables more flexibility with service injection without actually changing any behaviour or breaking backwards compatibility. It satisfies only 1 of 2 acceptance criteria of #38640 but since they're independent, I'm not marking the PR as WIP. ## Receiver & sender injection into AmazonSqsTransport It is now possible to inject your own receiver and sender into `Symfony\Component\Messenger\Bridge\AmazonSqs\Transport\AmazonSqsTransport`. ### Recommended way - AmazonSqsTransport::create For clean dependency injection, I recommed using the `create` static method, which obliges you to pass all dependencies: ```php $transport = AmazonSqsTransport::create($connection, $receiver, $sender); ``` For example, this code from `Symfony\Component\Messenger\Bridge\AmazonSqs\Transport\AmazonSqsTransportFactory`: ```php return new AmazonSqsTransport(Connection::fromDsn($dsn, $options), $serializer); ``` could be replaced with this: ```php $connection = Connection::fromDsn($dsn, $options); return AmazonSqsTransport::create( $connection, new AmazonSqsReceiver($connection, $serializer), new AmazonSqsSender($connection, $serializer) ); ``` I didn't replace that code in the factory because I didn't find it essential but I will certainly do it in my custom factory in my project, passing my own receiver implementation. ### Using the main constructor You can still use the main constructor but it's most suited for backwards compatibility, i.e. when you don't want to inject a receiver or a sender. With the full list of arguments it gets a bit messy due to their optionality. #### Minimal call ```php new AmazonSqsTransport($connection); ``` As before this PR, a receiver and a sender will be created using the default serializer, i.e. `Symfony\Component\Messenger\Transport\Serialization\PhpSerializer`. #### With a custom serializer ```php new AmazonSqsTransport($connection, $serializer); ``` As before this PR, a receiver and a sender will be created using the passed serializer. #### With a custom receiver and sender ```php new AmazonSqsTransport($connection, null, $receiver, $sender); ``` The injected services will be used. The second parameter (serializer) is unnecessary because it was only ever used while creating a receiver and a sender inside the transport. Because of this, I recommend using the new static `create` method. Commits ------- 281af26 [Messenger] Make all the dependencies of AmazonSqsTransport injectable
* 5.2: remove unreachable code [Browserkit] Add changelog entry for request parameters string cast Update ExceptionEvent.php fix firebase transport factory DI tag type [Validator] Resolve IsinValidator's dependency on the validator. [HttpFoundation] Fix for virtualhosts based on URL path
* 5.2: [Ldap] Fix undefined variable $con. Use GithubAction to run ldap tests Adds LDAP Adapter test in integration group Fix critical extension when reseting paged control Reinitialize globBrace after unserialization
* 5.2: do not depend on the actual time to fix a transient test Run Redis Sentinel tests in GithubAction Minor : Removed typo (extra "the" term) Check if method inheritEnvironmentVariables exists [PhpUnitBridge] Fix test fixture file name
…for errors (ogizanagi) This PR was squashed before being merged into the 5.3-dev branch. Discussion ---------- [Console][Yaml] Linter: add Github annotations format for errors | Q | A | ------------- | --- | Branch? | 5.x <!-- see below --> | Bug fix? | no | New feature? | yes <!-- 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 #", no need to create an issue if none exist, explain below instead --> | License | MIT | Doc PR | TODO Github actions [can write errors and warning](https://docs.github.com/en/free-pro-team@latest/actions/reference/workflow-commands-for-github-actions#setting-an-error-message) directly in their output, which result into annotations into the Github checks. It can even provide a filename, line & col number, allowing to display the annnotations inside the PR diff directly, at the right place. More advanced usage of annotations can be made using the [API](https://docs.github.com/en/free-pro-team@latest/rest/reference/checks#list-check-run-annotations), but regarding the linters provided in Symfony components, it seems the shortcut using output is a great way to enhance the integration with Github Actions. This PR starts by proposing these changes in the yaml linter: - add the `github` format, which is the same as the `txt` one, except for errors and warning, for which we'll adapt the output to the Github annotations format. - remove the `txt` format as default, and autodetect if the script is running in a Github action context, then use `github` format. If it's not, we fallback to `txt` as before. Once we agree on the details, we could perform the same for other linters (xliff, twig, ...) Here is a PR using it: ogizanagi/symfony-lint-gha-demo#2 and some screenshots: | PR checks run | PR checks annotations | PR diff | | -- | -- | -- | |  |  |  | ~~(tests to add)~~ --- This was inspired by [PHPStan](https://github.com/phpstan/phpstan-src/blob/d77bd87da9f2fad0440fc1614158cdfc1b7cc88a/src/Command/ErrorFormatter/GithubErrorFormatter.php) which is already auto-adapting the output according to the CI, using https://github.com/OndraM/ci-detector Commits ------- f0bbdc8 [Console][Yaml] Linter: add Github annotations format for errors
…age (tyx) This PR was merged into the 5.3-dev branch. Discussion ---------- [Messenger] Allow InMemoryTransport to serialize message | Q | A | ------------- | --- | Branch? | 5.x | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | Fix #38893 | License | MIT | Doc PR | wip This introduce a query parameter in dsn to enable serialization as @Nyholm has suggested in #38893 `in-memory://?serialize=true` Commits ------- 46a8007 [Messenger] Allow InMemoryTransport to serialize message
* 5.2: Bump Symfony version to 5.2.0 Update VERSION for 5.2.0-RC2 Update CHANGELOG for 5.2.0-RC2 Display debug info [HttpFoundation] Typo on deprecation package name [DoctrineBridge] drop binary variants of UID types [HttpClient] don't fallback to HTTP/1.1 when HTTP/2 streams break Default to user provider, if available, in password upgrader fix lexing nested sequences/mappings
* 5.2: Fix test. [PhpUnitBridge] Fix qualification of deprecations triggered by the debug class loader Improve return phpdoc for Normalizer Use a partial buffer in SymfonyStyle Fix console closing tag Fix typo in comment [VarDumper] fix casting resources turned into objects on PHP 8 Removing AnonymousPassport
…heGarious) This PR was merged into the 5.3-dev branch. Discussion ---------- [Console] Added Invalid constant into Command Class | Q | A | ------------- | --- | Branch? | 5.x | Bug fix? | no | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | License | MIT <!-- 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 5.x. --> Regarding into this [link](https://www.gnu.org/software/bash/manual/html_node/Exit-Status.html#:~:text=A%20non%2Dzero%20exit%20status,N%20as%20the%20exit%20status), we have 3 exits status standard : - 0 is for success - 1 is for error - 2 is for _indicate incorrect usage, generally invalid options or missing arguments_ I think if we use a constant into Command class, we need to implement invalid exit status too. Commits ------- 449147b Added Invalid constant into Command Class
…(karlshea) This PR was merged into the 5.3-dev branch. Discussion ---------- [Ldap] Ldap Entry case-sensitive attribute key option | Q | A | ------------- | --- | Branch? | 5.x | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | N/A | License | MIT | Doc PR | N/A See PR #36432 Commits ------- d3b9440 [Ldap] Ldap Entry case-sensitive attribute key option
… fabpot) This PR was merged into the 5.3-dev branch. Discussion ---------- [DomCrawler] Cache discovered namespaces | Q | A | ------------- | --- | Branch? | 5.x | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | Address #39067 | License | MIT Discovering namespaces is by far the most expensive task when filtering for nodes and the xpath contains prefixes. When `Crawler::filterRelativeXPath` is called multiple times with (identical) prefixes the slowdown is huge. This fix brings the runtime of the linked ticket down from 27 seconds to 9 seconds in my test. Compared to a pure PHP version which takes < 0.5 seconds the design of the crawler API is the limiting factor. There are still many repeated namespace queries caused by new Crawler instances. Ideas to solve this are discussed in the ticket. Commits ------- a8e85ec Make some CS changes 4c74dea Cache discovered namespaces in DomCrawler
Both classes have an optional argument `$readTimeout` that can be set during initialization for `\RedisSentinel` and during `connect`/`pconnect` respectively.
… `\Redis` (ferrastas) This PR was merged into the 5.3-dev branch. Discussion ---------- [Cache] Make use of `read_timeout` in `\RedisSentinel` and `\Redis` | Q | A | ------------- | --- | Branch? | 5.x | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #39429 | License | MIT This is a bugfix for #39363 a feature introduced in 5.x As described in issue #39429, `\RedisSentinel` accepts an optional read timeout value during construction. `read_timeout` is already part of the connection options, this PR just make use of it. Commits ------- 14e36a2 [Cache] Make use of `read_timeout` in `\RedisSentinel` and `\Redis`
…wing the exception (renan) This PR was merged into the 5.3-dev branch. Discussion ---------- [Cache] Bugfix provide the correct host and port when throwing the exception | Q | A | ------------- | --- | Branch? | 5.x | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | none | License | MIT | Doc PR | none The message before was including the result of the `getMasterAddrByName` call of which ended with: ``` Failed to retrieve master information from master name "mymaster" and address ":0". ``` Commits ------- 276bbb5 [Cache] Bugfix provide the correct host and port when throwing the exception
* 5.2: Fix licence Fix CS in link binary [Cache] remove no-op Fix CS in Changelogs [Notifier][Sinch] Add tests [Notifier] [Nexmo] Add tests [Notifier][OvhCloud] Add tests [Notifier] [Free Mobile] Rename method to match other bridges [Cache] fix setting "read_timeout" when using Redis Fix CS in changelogs [Notifier] Streamline README files
… a message failed
…n handling of a message failed (loevgaard) This PR was squashed before being merged into the 5.3-dev branch. Discussion ---------- [Messenger] Added more descriptive exception message when handling of a message failed | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no (more of a patch thing) | New feature? | no | Deprecations? | no | License | MIT I use Symfony Messenger extensively and I run into the `HandlerFailedException` from time to time. What bothers me is that the exception doesn't carry the name of the message that failed right there in the exception message. Here is an example from Sentry:  As you can see I get the error message, but I have to look through all my messages (in different bundles etc) to find the sinner. This PR adds the message name directly to the exception message. Commits ------- d985ca9 [Messenger] Added more descriptive exception message when handling of a message failed
* 5.2: Fix CS in Changelogs in 5.2
This PR was merged into the 5.3-dev branch. Discussion ---------- Fix CS in changelogs | Q | A | ------------- | --- | Branch? | 5.x | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - the last one :) Commits ------- fb22eec Fix CS in changelogs
* 5.2: [PropertyInfo][Serializer] Fixed extracting ignored properties [travis] fix checking if the current branch has same major as the next release
* 5.2: Cleanup composer.json files [Notifier] Add PHP 8 support for bridges
|
Hey! I see that this is your first PR. That is great! Welcome! Symfony has a contribution guide which I suggest you to read. In short:
Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change. When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor! I am going to sit back now and wait for the reviews. Cheers! Carsonbot |
jderusse
reviewed
Dec 11, 2020
derrabus
added a commit
that referenced
this pull request
Dec 16, 2020
…LE and DATETIME_IMMUTABLE (guillaume-sainthillier) This PR was squashed before being merged into the 5.2 branch. Discussion ---------- [DoctrineBridge] Guess correct form types for DATE_IMMUTABLE and DATETIME_IMMUTABLE | Q | A | ------------- | --- | Branch? | 5.2 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #39468 | License | MIT | Doc PR | - Rebased PR #39469 Fixes #39468 related to Doctrine Form Type Guesser with DateImmutable type Commits ------- 238d318 [DoctrineBridge] Guess correct form types for DATE_IMMUTABLE and DATETIME_IMMUTABLE
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.