Repository navigation
feat(common-contacts): stop copying members into every instance - #4970
Conversation
what check if according to PR name we just suppose to stop copy contacts |
|
also name of the PR doesn't sound like 22 files and 1k lines of code) |
96cd221 to
1c3fd5f
Compare
|
@MontaGhanmy just understood that it won't work without groups. If the groups are synced but contacts no, it doesn't make any sense, because what will be in groups then? |
|
|
||
| // The feed writes the members. | ||
| if inst.HasCommonContacts() { | ||
| return nil |
There was a problem hiding this comment.
This return also skips copyManagedGroups, since CopyOrgDirectoryFromOrgInstance copies the groups before the contacts. A member onboarded under common_contacts has no io.cozy.contacts.groups db.
The next b2b.group.member.added fails in ensureGroupExists with not_found, gets nacked until the DLQ, and because forEachOrgInstance joins errors it fails for every later member of that group too.
Could SyncGroupMembersAdded upsert the group when it is missing? That also heals instances onboarded while this was deployed. Copying the groups here alone would not.
| } | ||
| // The feed deletes the member. | ||
| if inst.HasCommonContacts() { | ||
| continue |
There was a problem hiding this comment.
This skips every common_contacts instance, but the feed DELETE (domain audience) only reaches the org instance.
Copies written on member instances by b2b.group.* events (trustedForSharing, no carddavPath) are never removed: alice deleted, bob keeps her.
Can we skip only contacts that have a carddavPath here?
| return fmt.Errorf("user.created: update settings: %w", err) | ||
| } | ||
|
|
||
| // The feed writes the members. |
There was a problem hiding this comment.
Creation is decided by the new instance's context, deletion by each target instance's context. If an org spans a common_contacts context and one without it, a legacy member gets copied into cc instances and is never removed. If an org cannot span contexts, a comment saying so is enough.
Otherwise decide per target instance in syncCreatedOrgContact as well.
There was a problem hiding this comment.
Covered by the user.deleted change: it now skips only the feed's contacts, whatever the instance's context, so a legacy copy on a common_contacts instance gets removed too.
| } | ||
| } | ||
|
|
||
| func TestCommonContactsStopsMemberCopies(t *testing.T) { |
There was a problem hiding this comment.
Can you assert that settings.email and matrix_id are still set under common_contacts? The PR promises it but the test only checks nothing was copied.
|
|
||
| - `user.password.updated` → updates an instance passphrase when a `user.password.updated` routing key is received. | ||
| - `user.created` → validates and processes user creation events. | ||
| - `user.created` → validates and processes user creation events. Copies no organization contact when the context sets `common_contacts`. |
There was a problem hiding this comment.
It now also skips the groups. The group paragraph further down still says groups are in every instance.
There was a problem hiding this comment.
Small change, and doing the check per target instance on user.deleted is the right call.
One thing blocks for me though: under common_contacts, user.created now also skips the group copy. So the next b2b.group.member.added for that group ends up in the DLQ, for the whole org.
I reproduced it on a local stack.
56dfd2d to
fad56b9
Compare
The feed DELETE only reaches the org instance, so skipping every common_contacts instance left the copies written by the b2b.group.* events on the member instances. Skip only the contacts the feed wrote, the ones with a carddavPath.
A member onboarded under common_contacts gets no copy of the groups, so b2b.group.member.added failed on their instance and the whole event went to the dead letter queue. Create the group when it is missing.
fad56b9 to
6c944e5
Compare
Summary
common_contactsenables the contacts service;centralized_org_directorycontrols directory replication.user.createdkeeps copying contacts and historical groups even when the contacts service is enabled.common_contacts: false; a missing organization instance returns an error.centralized_org_directory: falsefor the first backend rollout. Existing copies remain in place; the sharing UI and group-change propagation must support centralization before enabling it.Part of linagora/twake-workplace-private#1778.