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(contactsinteraction): Read, update or insert in DB transaction #37933

Merged

Conversation

ChristophWurst
Copy link
Member

@ChristophWurst ChristophWurst commented Apr 26, 2023

Summary

The transaction guarantees there are no two concurrent inserts for the same interaction.

I have not seen a conflict in production. I just noticed that this code is prone to fail in rare cases.

This is best reviewed as https://github.com/nextcloud/server/pull/37933/files?w=1

TODO

  • Span DB transaction for the read-write operation

Checklist

The transaction guarantees there are no two concurrent inserts for the
same interaction.

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
if ($event->getFederatedCloudId() !== null) {
$contact->setFederatedCloudId($event->getFederatedCloudId());
}
$contact->setLastContact($this->timeFactory->getTime());

Check notice

Code scanning / Psalm

DeprecatedMethod

The method OCP\AppFramework\Utility\ITimeFactory::getTime has been marked as deprecated
$event->getFederatedCloudId()
);
if (!empty($existing)) {
$now = $this->timeFactory->getTime();

Check notice

Code scanning / Psalm

DeprecatedMethod

The method OCP\AppFramework\Utility\ITimeFactory::getTime has been marked as deprecated
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
Copy link
Member

@st3iny st3iny left a comment

Choose a reason for hiding this comment

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

🧹

@st3iny st3iny 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 2, 2023
@st3iny st3iny added this to the Nextcloud 27 milestone May 2, 2023
This was referenced May 3, 2023
@ChristophWurst ChristophWurst merged commit 7e3d5d6 into master May 12, 2023
@ChristophWurst ChristophWurst deleted the fix/contactsinteraction/transactional-read-update-insert branch May 12, 2023 12:02
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 bug technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants