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.

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

fix(dav): Run system address book create-if-not-exists in transaction #37965

Merged
merged 1 commit into from
May 16, 2023

Conversation

ChristophWurst
Copy link
Member

@ChristophWurst ChristophWurst commented Apr 28, 2023

Summary

This might break in clustered DB setup

@CarlSchwan Jun 24, 2022

TODO

  • Wrap business transactions in database transactions

Checklist

}
$this->atomic(function() use ($addressBookId, $cardUri, $vCard) {
$existingCard = $this->backend->getCard($addressBookId, $cardUri);
if ($existingCard === false) {

Check notice

Code scanning / Psalm

DocblockTypeContradiction

array<array-key, mixed> does not contain false
$this->backend->deleteCard($addressBookId, $cardId);
$this->atomic(function() use ($addressBookId, $cardId, $user) {
$card = $this->backend->getCard($addressBookId, $cardId);
if ($card === false) {

Check notice

Code scanning / Psalm

DocblockTypeContradiction

array<array-key, mixed> does not contain false
Copy link
Member

@tcitworld tcitworld left a comment

Choose a reason for hiding this comment

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

Don't really like injecting IDBConnection here, but the alternative is catching UniqueConstraintViolationException for each insertion and do the update afterwards, which doesn't suit me much more.

@tcitworld

This comment was marked as outdated.

@ChristophWurst

This comment was marked as outdated.

This was referenced May 3, 2023
@ChristophWurst ChristophWurst force-pushed the fix/transactional-system-addressbook-sync branch from c37c76b to e866a95 Compare May 10, 2023 08:51
@ChristophWurst ChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels May 10, 2023
@ChristophWurst ChristophWurst enabled auto-merge May 10, 2023 08:54
@ChristophWurst ChristophWurst disabled auto-merge May 12, 2023 12:01
@ChristophWurst ChristophWurst marked this pull request as draft May 12, 2023 12:01
@ChristophWurst ChristophWurst added 2. developing Work in progress and removed 4. to release Ready to be released and/or waiting for tests to finish labels May 12, 2023
@ChristophWurst

This comment was marked as resolved.

@ChristophWurst ChristophWurst force-pushed the fix/transactional-system-addressbook-sync branch from e866a95 to 2cc672d Compare May 12, 2023 12:15
@ChristophWurst ChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 2. developing Work in progress labels May 12, 2023
@ChristophWurst ChristophWurst marked this pull request as ready for review May 12, 2023 12:15
@szaimen
Copy link
Contributor

szaimen commented May 16, 2023

CI failure unrelated

@szaimen szaimen merged commit 25dd264 into master May 16, 2023
@szaimen szaimen deleted the fix/transactional-system-addressbook-sync branch May 16, 2023 09:33
@blizzz blizzz mentioned this pull request May 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish technical debt
Projects
Development

Successfully merging this pull request may close these issues.

6 participants