Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Messenger Doctrine] Fixed regression by #50524 causing data loss#50716

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
brusch wants to merge4,808 commits intosymfony:5.4frombrusch:patch-2
Closed

[Messenger Doctrine] Fixed regression by #50524 causing data loss#50716

brusch wants to merge4,808 commits intosymfony:5.4frombrusch:patch-2

Conversation

@brusch
Copy link
Contributor

Regression caused by#50524

When using$platform->getAlterSchemaSQL($schemaDiff) instead of$schemaDiff->toSaveSql($platform) causes to call\Doctrine\DBAL\Schema\SchemaDiff::_toSql($platform, false) instead of\Doctrine\DBAL\Schema\SchemaDiff::_toSql($platform, true). When $saveMode=false the diff from the schema is getting remove, so actually all other tables ... in the DB are getting deleted

QA
Branch?6.3
Bug fix?yes
New feature?no
Deprecations?no >
LicenseMIT

nicolas-grekasand others added30 commitsMay 12, 2023 18:35
…sible (weaverryan)This PR was merged into the 6.3 branch.Discussion----------[AssetMapper] Improving XSD to use attributes whenever possible| Q             | A| ------------- | ---| Branch?       | 6.3| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Tickets       | None| License       | MIT| Doc PR        | TODOSome notes from `@stof` - I was using `<elements>` for scalar config that works much better as attributes.Cheers!Commits-------6326a5e [AssetMapper] Improving XSD to use attributes whenever possible
This PR was squashed before being merged into the 6.3 branch.Discussion----------[AssetMapper] Add cached asset factory| Q             | A| ------------- | ---| Branch?       | 6.3| Bug fix?      | no| New feature?  | yes| Deprecations? | no| Tickets       | None| License       | MIT| Doc PR        | Still TODOHi!This is built on top of#50264. I CAN separate them if needed - sorry for the noise.tl;dr When using asset mapper, to load the page, we need to build the importmap & resolve a few paths, like `{{ asset('styles/app.css') }}`. Because asset mapper loads the contents and performs some regex on CSS and JS files, this can slow down the page in dev (in prod, `asset-map:compile` removes all overhead).This PR fixes that (I can see a big improvement in my test app). We "cache" the `MappedAsset` objects via the config cache system so that if an underlying file changes (e.g. `assets/styles/app.css`), we only need to rebuild that ONE `MappedAsset` when the page is loading.Cheers!Commits-------ce77ed8 [AssetMapper] Add cached asset factory
* 5.4:  [HttpKernel] Account for Response::getDate() possibly returning a DateTimeImmutable  [Messenger] Respect `isRetryable` decision of the retry strategy when deciding if failed message should be re-delivered
* 6.2:  [HttpKernel] Account for Response::getDate() possibly returning a DateTimeImmutable  [Messenger] Respect `isRetryable` decision of the retry strategy when deciding if failed message should be re-delivered
This PR was merged into the 6.3 branch.Discussion----------[Notifier] Bring consistency to bridges| Q             | A| ------------- | ---| Branch?       | 6.3| Bug fix?      | no| New feature?  | no| Deprecations? | no| Tickets       | -| License       | MIT| Doc PR        | -Commits-------657e9be [Notifier] Bring consistency to bridges
* 5.4:  [FrameworkBundle] Generate caches consistently on successive run of `cache:clear` command
* 6.2:  [FrameworkBundle] Generate caches consistently on successive run of `cache:clear` command
…epelko)This PR was submitted for the 6.2 branch but it was merged into the 6.3 branch instead.Discussion----------[Console] Remove redundant method getSaturation()| Q             | A| ------------- | ---| Branch?       | 6.2| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Tickets       | none| License       | MIT| Doc PR        | none1. in `getSaturation()`, `$diff` is compared against `int` zero. This will only yield `true` when RGB values contain min/max color value (`0` or `255`). It will be always `false` for floats.2. in `degradeHexColorToAnsi4()` value returned from `getSaturation()` is `round`ed, which returns float, thus making comparison to `int` zero useless.3. in the same method, bitwise operations (final `return` statement) will convert number to `int` anyway, so the explicit cast was removedIn conclusion, `getSaturation` is redundant. Tests are expanded to show that even in min/max cases everything works fine.Commits-------38021b5 [Console] Remove redundant method getSaturation()
This PR was merged into the 6.3 branch.Discussion----------Ensure DoctrineIntegrationTest always run in UTC| Q             | A| ------------- | ---| Branch?       | 6.3| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Tickets       |Fix#50327| License       | MIT| Doc PR        | N/ACommits-------c036acf Ensure DoctrineIntegrationTest always run in UTC
This PR was merged into the 6.3 branch.Discussion----------[VarDumper] Fix HTML of invisible characters| Q             | A| ------------- | ---| Branch?       | 6.3| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Tickets       | -| License       | MIT| Doc PR        | -Commits-------6b0ebc8 [VarDumper] Fix HTML of invisible characters
…value if possible when a parameter is not found (MatTheCat)This PR was merged into the 6.3 branch.Discussion----------[HttpKernel] Make `QueryParameterValueResolver` provide a value if possible when a parameter is not found| Q             | A| ------------- | ---| Branch?       | 6.3| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Tickets       |Fix#50339| License | MIT| Doc PR        | N/AAs `QueryParameterValueResolver` is “targeted” it must provide a value itself if possible.When a parameter is not found in the query string, we want to return the argument’s default value if it has one, or `null` if it is nullable. Else the behavior do not change: we throw a `NotFoundHttpException`.Commits-------00df479 Provide a value if possible when parameter is not found
…medJsonResponse (alexander-schranz)This PR was merged into the 6.3 branch.Discussion----------[HttpFoundation] Fix problem with empty generator in StreamedJsonResponse| Q             | A| ------------- | ---| Branch?       | 6.3 (Feature `StreamedJsonResponse`:#47709)| Bug fix?      | yes| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tickets       | Fix - was reported to me on Slack by `@norkunas`| License       | MIT| Doc PR        | symfony/symfony-docs#... <!-- required for new features -->Currently when the Generator is empty the return is invalid JSON which should not happen. So adding a testcase and a fix to the problem with the empty generator.Commits-------39bb6b6 Fix problem with empty generator in StreamedJsonResponse
…view (javiereguiluz)This PR was merged into the 6.3 branch.Discussion----------[WebProfilerBundle] Tweak the HTML code of the Twig entry view| Q             | A| ------------- | ---| Branch?       | 6.3| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Tickets       | -| License       | MIT| Doc PR        | -Stretched links are used to make entire sections, like a card, clickable. Seehttps://getbootstrap.com/docs/5.3/helpers/stretched-link/Maybe `@PhilETaylor` can answer us about this (he made the original PR to add this in#49887) but I think it wasn't intended to make entire Twig panel clickable to open the template. So, in this PR I propose to remove this behavior.Here you can see how in the Security panel, the link is not stretched but in the Twig panel it is:![profiler-panel](https://github.com/symfony/symfony/assets/73419/155170f3-115d-4043-98cd-8844c1a3a4bf)Commits-------c9ed471 [WebProfilerBundle] Tweak the HTML code of the Twig entry view
… in `HtmlDumper` (ohader)This PR was squashed before being merged into the 6.3 branch.Discussion----------[VarDumper] Reduce stylesheet assignments via JavaScript in `HtmlDumper`| Q             | A| ------------- | ---| Branch?       | 6.3| Bug fix?      | no| New feature?  | no| Deprecations? | no| Tickets       | *none*| License       | MITThe dynamic `<style>` element in the JavaScript variable `refStyle` can be replaced by static CSS and element states.The new rule `.sf-dump-hover:hover` substitutes JavaScript event handling for `mouseover` events.This is a preparation to have the possibility to assign `nonce` HTML attributes to inline `<script>` and `<style>` nodes, e.g. shown as proof-of-concept athttps://review.typo3.org/c/Packages/TYPO3.CMS/+/78512/2/typo3/sysext/adminpanel/Classes/Utility/HtmlDumper.phpCommits-------53046a3 [VarDumper] Reduce stylesheet assignments via JavaScript in `HtmlDumper`
nicolas-grekasand others added11 commitsJune 9, 2023 01:46
This PR was merged into the 6.2 branch.Discussion----------[Clock] Fix MockClock::modify() on PHP 8.3| Q             | A| ------------- | ---| Branch?       | 6.2| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Tickets       | -| License       | MIT| Doc PR        | -Commits-------7361374 [Clock] Fix MockClock::modify() on PHP 8.3
* 6.2:  [Clock] Fix MockClock::modify() on PHP 8.3
This PR was merged into the 6.3 branch.Discussion----------[HttpClient] Remove final keyword on `AsyncResponse`| Q             | A| ------------- | ---| Branch?       | 6.3| Bug fix?      | kind of| New feature?  | no| Deprecations? | no| Tickets       || License       | MIT| Doc PR        |In symfony < 6.3, it was possible to override HTTP response headers. See this [blog post](https://jolicode.com/blog/aggressive-caching-with-symfony-http-client) for a use caseWith Symfony 6.3, this is not possible anymore. I created a reproducer here:https://github.com/lyrixx/test/tree/http-cache-ppAnd `@nicolas`-grekas came with another solution:lyrixx/test#11So here we go!Commits-------fbd6d18 [HttpClient] Remove final keyword on AsyncResponse
…vars (Okhoshi)This PR was merged into the 6.3 branch.Discussion----------[DependencyInjection] Fix support for `false` boolean env vars| Q             | A| ------------- | ---| Branch?       | 6.3| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Tickets       | -| License       | MIT| Doc PR        | -After upgrading `symfony/dependency-injection` package to version 6.3, some of our env vars couldn't be found anymore.For some scripts, we are providing an adhoc default value to env vars by setting `$_SERVER` directly. However, since version 6.3 of the DependencyInjection component, setting an env var to `false` (the boolean value, like in the snippet below) doesn't work anymore, and Symfony reports that the variable cannot be found.```php$_SERVER['FOO'] = false;```It seems to be a side effect of the changes made in#48705.Commits-------5101d18 [DependencyInjection] Fix support for `false` boolean env vars
…ltzu)This PR was merged into the 6.3 branch.Discussion----------[Messenger] Don't mark `RedispatchMessage` as internal| Q             | A| ------------- | ---| Branch?       | 6.3| Bug fix?      | yes| New feature?  | no| Deprecations? | no| License       | MITAs discussed in#49734 (comment), `RedispatchMessage` is meant to be used in your project, hence it shouldn't be marked as internal (which makes IDEs suggest not to use it).Commits-------1209b81 Don't mark RedispatchMessage as internal
This PR was merged into the 6.3 branch.Discussion----------[Security] Fix log message in `OidcTokenHandler`| Q             | A| ------------- | ---| Branch?       | 6.3| Bug fix?      | no| New feature?  | no| Deprecations? | no| Tickets       | -| License       | MIT| Doc PR        | -Commits-------5f29bd8 [Security] Fix log message in OidcTokenHandler
When using `$platform->getAlterSchemaSQL($schemaDiff)` instead of `$schemaDiff->toSaveSql($platform)` causes to call `\Doctrine\DBAL\Schema\SchemaDiff::_toSql($platform, false)` instead of `\Doctrine\DBAL\Schema\SchemaDiff::_toSql($platform, true)`.  When $saveMode=false the diff from the schema is getting remove, so actually all other tables ... in the DB are getting deleted
$schemaDiff =$this->compareSchemas($comparator,method_exists($schemaManager,'introspectSchema') ?$schemaManager->introspectSchema() :$schemaManager->createSchema(),$this->getSchema());
$platform =$this->driverConnection->getDatabasePlatform();
$queries =method_exists($platform,'getAlterSchemaSQL') ?$platform->getAlterSchemaSQL($schemaDiff) :$schemaDiff->toSaveSql($platform);
$queries =$schemaDiff->toSaveSql($platform);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

But toSaveSql is deprecated, isn't it? so we need to find the future-proof version of this.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Yeah, that's true, but I haven't found an adequate replacement yet 🥲 Therefore I'd keep the current implementation until there's a solution offered by Doctrine.
Probablydoctrine/dbal needs to add a parameter togetAlterSchemaSQL() or offers an additional method for that 🤔

@blankse
Copy link

@brusch#50524 was merged in branch5.4. So I think we should fix it also in branch5.4, right?

jdreesen and brusch reacted with thumbs up emoji

@stof
Copy link
Member

@brusch if you want to change the target branch to 5.4, you need to rebase your branch so that it is based on 5.4 too. If the branch is still based on 6.3, that would merge 6.3 into 5.4 which is a no-go.

@brusch
Copy link
ContributorAuthor

@stof yep, on it 😉

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas left review comments

@lyrixxlyrixxAwaiting requested review from lyrixxlyrixx is a code owner

@ycerutoycerutoAwaiting requested review from ycerutoyceruto is a code owner

@wouterjwouterjAwaiting requested review from wouterj

@chalasrchalasrAwaiting requested review from chalasrchalasr is a code owner

@dunglasdunglasAwaiting requested review from dunglasdunglas is a code owner

@OskarStarkOskarStarkAwaiting requested review from OskarStarkOskarStark is a code owner

@xabbuhxabbuhAwaiting requested review from xabbuhxabbuh is a code owner

Assignees

No one assigned

Projects

None yet

Milestone

6.3

Development

Successfully merging this pull request may close these issues.

37 participants

@brusch@blankse@stof@nicolas-grekas@carsonbot@chalasr@weaverryan@fabpot@ivanpepelko@javiereguiluz@alex-dev@MatTheCat@fancyweb@alexander-schranz@ohader@HypeMC@ZhukV@VincentLanglet@alamirault@garak@wouterj@PhilETaylor@OskarStark@aegypius@Myks92@vtsykun@vincentchalamon@dunglas@lyrixx@wiseguy1394@alexandre-daubois@flofloflo@okhoshi@xabbuh@upchuk@derrabus@valtzu

[8]ページ先頭

©2009-2025 Movatter.jp