reduce adressbook change events and handling

[blizzz]

This was observed when updating the Avatar from LDAP.

When setting a new avatar against the IAvatar::set() implementation, first the old avatar is removed. That cleans up the files and does some config settings. However, this cause also the addressbook to be updated with an empty avatar.

Then, when the new avatar is set, this happened again.

Now on LDAP this happened twice due to a wrong passOn parameter in the LDAP's proxy, which would lead to do it twice.

It can be easily reproduced:

• have LDAP configured
• log in with an LDAP user that gets it avatar from LDAP
• open the personal settings page
• run SQL query select count(*) from oc_addressbookchanges
• refresh the personal settings page

Observed behaviour: the changes count is +4

Desired behaviour: the changes count is +1 (with this patch)

Now you could argue there should be no change when the avatar is still the same and there you are right. There will be a follow up, because that solution might cover the deeper buried behaviour.

[georgehrke]

I don't have an LDAP to test, but code wise this looks fine 👍

I'm ok with this as a hot fix, but it should stay a hot fix and not become the permanent solution.

[kesselb]

Code looks good 👍 Don't have a ldap setup either 🙈

server/lib/private/Server.php

Lines 1236 to 1237 in 7927aeb

	$avatar = $manager->getAvatar($user->getUID());
	$avatar->remove();

Here is another place for the follow up. If we are going to delete the user anyway we don't have to trigger a change. This might save some calls.

[blizzz]

I'm ok with this as a hot fix, but it should stay a hot fix and not become the permanent solution.

can you elaborate?

[georgehrke]

Why can't we replace it in one step. Is there really a need for remove and recreate?

[blizzz]

Why can't we replace it in one step. Is there really a need for remove and recreate?

My understanding is that there are several different sized version of the avatar. Deleting them first ensures that no old one is left.

[rullzer]

Master is no 18 development. If this needs to go into 17 please follow the normal backport procedures after merging.

[blizzz]

/backport to stable17

[blizzz]

/backport to stable16

[blizzz]

still looking for another reviewer :)

[nickvergessen]

🐘

[backportbot-nextcloud]

backport to stable17 in #17351

[backportbot-nextcloud]

backport to stable16 in #17352