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

feat: Remove unused contacts in housekeeping #4791

Closed
wants to merge 1 commit into from

Conversation

Septias
Copy link
Collaborator

@Septias Septias commented Oct 5, 2023

Remove contacts that don't have associated chats in housekeeping.

Remove contacts that don't have associated chats in housekeeping.
@Septias Septias requested a review from link2xt October 5, 2023 10:32
context
.sql
.execute(
"DELETE FROM contacts WHERE origin=? AND id NOT IN (SELECT contact_id FROM chats_contacts);",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I commented here #4785 (comment) that it is probably better to remove the contact before deleting files so avatar gets deleted in a single housekeeping run. It is marked as resolved, but seems to be lost during rebase.

@r10s
Copy link
Member

r10s commented Oct 5, 2023

Remove contacts that don't have associated chats in housekeeping.

the description is misleading - in that generality, it sounds like a non-goal and i just wanted to comment :)

by reading the code, however, i realised that it is only about contacts that were already manually deleted by the user before :)

as this still sounds fragile, i'd enhance the test to add two contact without a chat before housekeeping:

  • add one contact using dc_create_contact() (Claire or so)
  • add one contact using dc_add_address_book() (Dave or so)
  • check that both contacts exist before housekeeping (creation succeeded)
  • check that both contacts still exist after housekeeping (skipped by pruning tombstones)

for blocked contacts, things seem to be fine, they do not use the "origin" (so, blocked contacts do not get the origin "Hidden")

Copy link
Collaborator

@link2xt link2xt left a comment

Choose a reason for hiding this comment

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

There is one more problem. What if the contact is removed from all chats, but still has messages where they are author? In this case messages will not be associated to any existing contact anymore.

@Septias
Copy link
Collaborator Author

Septias commented Oct 5, 2023

I guess, the problem is that we have two different requirements for the contact deletion. For the UI, contact deletion should only hide contacts for creating new chats and for the bot (where this issue arose from) deleting contacts should delete everything associated with him.
For the bot code, I now just deleted the 1:1 chat with the user and call delete contact afterward, so all messages & chats should be deleted this way. In my opinion, we could then close this issue because fixing @link2xt problem seems pretty tedious.

@link2xt
Copy link
Collaborator

link2xt commented Oct 5, 2023

I opened an issue #4792 as this is a problem not only for housekeeping, but for contact deletion as well.

@link2xt
Copy link
Collaborator

link2xt commented Oct 5, 2023

For the bot code, I now just deleted the 1:1 chat with the user and call delete contact afterward, so all messages & chats should be deleted this way.

There may be messages from this user in other chats, e.g. if someone tries to add the bot to a group.

@Septias Septias deleted the sk/houskeeping_on_empty_contacts2 branch October 10, 2023 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants