Skip to content
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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

DAV custom props: catch Exception and rollback transaction in case #33090

Merged
merged 1 commit into from
Jul 4, 2022
Merged

DAV custom props: catch Exception and rollback transaction in case #33090

merged 1 commit into from
Jul 4, 2022

Conversation

blizzz
Copy link
Member

@blizzz blizzz commented Jul 1, 2022

  • before exceptions were not caught, a started transaction might not have
    been finished
  • also resolve depractions and use IQueryBuilder

@blizzz
DAV custom props: catch Exception and rollback transaction in case
794177e 
- before exceptions were not caught, a started transaction might not have
  been finished
- also resolve depractions and use IQueryBuilder

Signed-off-by: Arthur Schiwon <[email protected]>
@blizzz blizzz added bug 3. to review Waiting for reviews feature: dav feature: carddav Related to CardDAV internals labels Jul 1, 2022
@blizzz blizzz added this to the Nextcloud 25 milestone Jul 1, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jul 1, 2022

Possible performance regression detected

Show Output 
1 queries added

= /remote.php/dav/files/test
≠ /remote.php/dav/files/test/test.txt with 1 queries added
  + UPDATE "oc_preferences" SET "configvalue" = :dcValue1 WHERE ("userid" = :dcValue2) AND ("appid" = :dcValue3) AND ("configkey" = :dcValue4)
≠ /remote.php/dav/files/test/many_files with 1 queries removed
  - UPDATE "oc_preferences" SET "configvalue" = :dcValue1 WHERE ("userid" = :dcValue2) AND ("appid" = :dcValue3) AND ("configkey" = :dcValue4)
= /remote.php/dav/files/test/new_file.txt
≠ /remote.php/dav/files/test/new_file.txt with 1 queries added
  + UPDATE "oc_preferences" SET "configvalue" = :dcValue1 WHERE ("userid" = :dcValue2) AND ("appid" = :dcValue3) AND ("configkey" = :dcValue4)

@blizzz
Copy link
Member Author

blizzz commented Jul 1, 2022

Possible performance regression detected

No queries were actually added.

@szaimen
Copy link
Contributor

szaimen commented Jul 1, 2022

Possible performance regression detected

I fear this is a flaky test :/

CarlSchwan
CarlSchwan approved these changes Jul 4, 2022
View reviewed changes
Copy link
Member

@CarlSchwan CarlSchwan left a comment

Choose a reason for hiding this comment

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

So much nicer ❤️

@CarlSchwan
Copy link
Member

Possible performance regression detected

No queries were actually added.

the reason is that query using raw SQL are not watched by the profiler, that's a reason why it's good to port then to IQueryBuilder

@CarlSchwan CarlSchwan merged commit ec465bf into master Jul 4, 2022
@CarlSchwan CarlSchwan deleted the fix/noid/proppatch-properties-transaction-rollback branch July 4, 2022 13:27
@CarlSchwan
Copy link
Member

backports?

@blizzz
Copy link
Member Author

blizzz commented Jul 5, 2022

/backport to stable24

@blizzz
Copy link
Member Author

blizzz commented Jul 5, 2022

/backport to stable23

@blizzz
Copy link
Member Author

blizzz commented Jul 5, 2022

/backport to stable22

@backportbot-nextcloud
Copy link

The backport to stable24 failed. Please do this backport manually.

@backportbot-nextcloud
Copy link

The backport to stable23 failed. Please do this backport manually.

@backportbot-nextcloud
Copy link

The backport to stable22 failed. Please do this backport manually.

@blizzz blizzz mentioned this pull request Jul 6, 2022
Merged
@blizzz blizzz mentioned this pull request Jul 29, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChristophWurst ChristophWurst ChristophWurst approved these changes

@tcitworld tcitworld tcitworld approved these changes

@CarlSchwan CarlSchwan CarlSchwan approved these changes

@come-nc come-nc Awaiting requested review from come-nc

@icewind1991 icewind1991 Awaiting requested review from icewind1991

Assignees
No one assigned
Labels
3. to review Waiting for reviews bug feature: carddav Related to CardDAV internals feature: dav
Projects
None yet
Milestone
Nextcloud 25
Development

Successfully merging this pull request may close these issues.
6 participants
@blizzz @szaimen @CarlSchwan @ChristophWurst @tcitworld @skjnldsv