Skip to content

Port Repair and Migrator events to IEventDispatcher - #33640

Merged
come-nc merged 12 commits into
masterfrom
fix/fix-symfony-event-typing
Aug 30, 2022
Merged

Port Repair and Migrator events to IEventDispatcher#33640
come-nc merged 12 commits into
masterfrom
fix/fix-symfony-event-typing

Conversation

@come-nc

@come-nccome-nc commented Aug 22, 2022•
edited
Loading

Copy link
Copy Markdown
Contributor

See #32128
This also fixes#33264

@come-nccome-nc added 2. developing Work in progress technical debt 🧱 🤔🚀 labels Aug 22, 2022
@come-nccome-nc added this to the Nextcloud 25 milestone Aug 22, 2022
@come-nccome-nc self-assigned this Aug 22, 2022
@come-nccome-nc linked an issue Aug 22, 2022 that may be closed by this pull request
Update via UI breaks with future version of symfony/event-dispatcher #33264
Closed
2 tasks
@come-nc

come-nc commented Aug 22, 2022

Copy link
Copy Markdown
ContributorAuthor

It seems \OC\DB\Migrator::checkTable is actually never dispatched, is it?

Not since a2f3077

github-advanced-security[bot]
github-advanced-securityAI found potential problems Aug 22, 2022
View reviewed changes

@github-advanced-securitygithub-advanced-securityAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Found 11 potential problems in the proposed changes. Check the Files changed tab for more details.

github-advanced-security[bot]
github-advanced-securityAI found potential problems Aug 22, 2022
View reviewed changes
Comment threadlib/private/Updater.php Fixed
Comment threadlib/private/Updater.php Fixed
Comment threadlib/private/Updater.php Fixed
Comment threadlib/private/Updater.php Fixed
Comment threadlib/private/Updater.php Fixed
github-advanced-security[bot]
github-advanced-securityAI found potential problems Aug 22, 2022
View reviewed changes

@github-advanced-securitygithub-advanced-securityAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Found 38 potential problems in the proposed changes. Check the Files changed tab for more details.

github-advanced-security[bot]
github-advanced-securityAI found potential problems Aug 22, 2022
View reviewed changes

@github-advanced-securitygithub-advanced-securityAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Found 18 potential problems in the proposed changes. Check the Files changed tab for more details.

github-advanced-security[bot]
github-advanced-securityAI found potential problems Aug 22, 2022
View reviewed changes

@github-advanced-securitygithub-advanced-securityAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Found 18 potential problems in the proposed changes. Check the Files changed tab for more details.

github-advanced-security[bot]
github-advanced-securityAI found potential problems Aug 22, 2022
View reviewed changes

@github-advanced-securitygithub-advanced-securityAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Found 18 potential problems in the proposed changes. Check the Files changed tab for more details.

github-advanced-security[bot]
github-advanced-securityAI found potential problems Aug 22, 2022
View reviewed changes

@github-advanced-securitygithub-advanced-securityAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Found 18 potential problems in the proposed changes. Check the Files changed tab for more details.

github-advanced-security[bot]
github-advanced-securityAI found potential problems Aug 22, 2022
View reviewed changes

@github-advanced-securitygithub-advanced-securityAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Found 11 potential problems in the proposed changes. Check the Files changed tab for more details.

github-advanced-security[bot]
github-advanced-securityAI found potential problems Aug 22, 2022
View reviewed changes

@github-advanced-securitygithub-advanced-securityAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Found 11 potential problems in the proposed changes. Check the Files changed tab for more details.

github-advanced-security[bot]
github-advanced-securityAI found potential problems Aug 22, 2022
View reviewed changes

@github-advanced-securitygithub-advanced-securityAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Found 11 potential problems in the proposed changes. Check the Files changed tab for more details.

github-advanced-security[bot]
github-advanced-securityAI found potential problems Aug 22, 2022
View reviewed changes

@github-advanced-securitygithub-advanced-securityAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Found 11 potential problems in the proposed changes. Check the Files changed tab for more details.

github-advanced-security[bot]
github-advanced-securityAI found potential problems Aug 22, 2022
View reviewed changes

@github-advanced-securitygithub-advanced-securityAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Found 11 potential problems in the proposed changes. Check the Files changed tab for more details.

github-advanced-security[bot]
github-advanced-securityAI found potential problems Aug 22, 2022
View reviewed changes

@github-advanced-securitygithub-advanced-securityAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Found 11 potential problems in the proposed changes. Check the Files changed tab for more details.

github-advanced-security[bot]
github-advanced-securityAI found potential problems Aug 22, 2022
View reviewed changes
Comment threadcore/ajax/update.php Fixed
Comment threadcore/ajax/update.php Fixed
Comment threadcore/ajax/update.php Fixed
Comment threadcore/ajax/update.php Fixed
Comment threadcore/ajax/update.php Fixed
Comment threadcore/ajax/update.php Fixed
Comment threadcore/ajax/update.php Fixed
Comment threadcore/ajax/update.php Fixed
github-advanced-security[bot]
github-advanced-securityAI found potential problems Aug 23, 2022
View reviewed changes
Comment threadcore/ajax/update.php Fixed
github-advanced-security[bot]
github-advanced-securityAI found potential problems Aug 23, 2022
View reviewed changes
Comment threadlib/private/legacy/OC_App.php Fixed
@come-nc
come-ncforce-pushed the fix/fix-symfony-event-typing branch from 931993a to 123d85fCompareAugust 23, 2022 08:34
@come-nccome-nc changed the title Fix symfony event typingPort Repair and Migrator events to IEventDispatcherAug 23, 2022
@come-nccome-nc added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Aug 23, 2022
@come-nc
come-nc requested review from a team and PVince81 and removed request for a teamAugust 23, 2022 10:22
CarlSchwan
CarlSchwan approved these changes Aug 23, 2022
View reviewed changes
Comment threadcore/Command/Upgrade.php Outdated
Comment threadcore/ajax/update.php Outdated
@blizzzblizzz mentioned this pull request Aug 24, 2022
Merged
@come-nc
come-ncforce-pushed the fix/fix-symfony-event-typing branch 2 times, most recently from 5fbe1df to 3808f0cCompareAugust 25, 2022 10:15
nickvergessen
nickvergessen reviewed Aug 25, 2022
View reviewed changes

@nickvergessennickvergessen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Didn't test, but looks good in general

Comment threadcore/Command/Maintenance/Repair.php Outdated
Comment threadcore/ajax/update.php Outdated
Comment threadlib/private/Repair/Events/RepairAdvanceEvent.php
Comment threadlib/private/DB/MigratorExecuteSqlEvent.php
Comment threadlib/private/Repair/Events/RepairAdvanceEvent.php Outdated
Comment threadlib/private/Repair/Events/RepairFinishEvent.php
Comment threadlib/private/Repair/Events/RepairInfoEvent.php
Comment threadlib/private/Repair/Events/RepairStartEvent.php
Comment threadlib/private/Repair/Events/RepairStepEvent.php
Comment threadlib/private/Repair/Events/RepairWarningEvent.php
@come-nc
Switch to string keys for argument of GenericEvent for OC\DB\Migrator
4ba30d4
It seems checkTable is actually never dispatched?
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
Moving to string key for arguments of GenericEvent in Repair
4f260dc
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
Remove listeners of \OC\DB\Migrator::checkTable which is never emitted
5aac997
See commit a2f3077 from PR #24384
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
Migrate Migrator::executeSql to OCP\EventDispatcher\Event
a83a8f0
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
Migrate Repair events to OCP\EventDispatcher\Event
a2a7150
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
Fix RepairTest
5d313da
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
Fix BackgroundRepairTest
e5731b3
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
Add void return types to callables
5cfdf9b
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
Declare strict types in all new Event classes
4ac8173
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
come-ncforce-pushed the fix/fix-symfony-event-typing branch from 3d5cd2b to 4ac8173CompareAugust 25, 2022 14:15
@come-nc
Surround cli output with appropriate xml tags
652ab6d
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
Parameter of RepairAdvanceEvent is actually an increment, not a step id
9a29025
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
Flag repair errors as error level in eventSource->send
0b9a878
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc

come-nc commented Aug 25, 2022

Copy link
Copy Markdown
ContributorAuthor

Tested web updater with a no-problem update, seemed fine.
Need to test app update, error update and cli update I suppose.

@come-nc

come-nc commented Aug 30, 2022

Copy link
Copy Markdown
ContributorAuthor

cli upgrade works fine. Not sure how to test app upgrade or buggy upgrade, I test by switching branches so there is no actual download happenning.

@come-nc
come-nc merged commit f56ecf9 into masterAug 30, 2022
@come-nc
come-nc deleted the fix/fix-symfony-event-typing branch August 30, 2022 12:47
@come-nccome-nc added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Aug 30, 2022
@blizzzblizzz mentioned this pull request Aug 30, 2022
Merged
@ChristophWurstChristophWurst mentioned this pull request Jan 20, 2023
Merged
4 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@skjnldsvskjnldsvskjnldsv approved these changes
@CarlSchwanCarlSchwanCarlSchwan approved these changes
@PVince81PVince81Awaiting requested review from PVince81
@nickvergessennickvergessenAwaiting requested review from nickvergessen

Assignees

@come-nccome-nc

Labels

4. to releaseReady to be released and/or waiting for tests to finishtechnical debt🧱 🤔🚀

Projects

None yet

Milestone

Nextcloud 25

Development

Successfully merging this pull request may close these issues.

Update via UI breaks with future version of symfony/event-dispatcher

5 participants

@come-nc@skjnldsv@nickvergessen@CarlSchwan@github-advanced-security