Bugfix/noid/fix too many event triggers

[nickvergessen]

Add a break point at

server/lib/private/Server.php

Line 1261 in 3f9cdee

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

And create a user via console

Before: called 6 times
After: called 1 time

The event was already in place before:
https://github.com/nextcloud/server/blame/ec4b1584fe46df81e3eda1ff374f575d153a966f/lib/private/Server.php#L432
So when I added it in:
https://github.com/nextcloud/server/blame/01b4db62fbc4230cff953a2385d305b149744b86/lib/private/User/User.php#L468

It got actually called twices everytime.

And then the avatar manager listened to it's own event making some extra loops 🙈

[blizzz]

don't you fire it (potentially) twice in the triggerChange() anyhow, with both dispatcher and emitter?

[nickvergessen]

don't you fire it (potentially) twice in the triggerChange() anyhow, with both dispatcher and emitter?

that was exactly the case and what I removed now....

[rullzer]

why this?

[nickvergessen]

Because the very same event is dispatched already in the OC\User\User::triggerChange:

server/lib/private/User/User.php

Lines 466 to 470 in 01b4db6

	$this->dispatcher->dispatch(IUser::class . '::changeUser', newGenericEvent($this, [
	'feature' => $feature,
	'value' => $value,
	'oldValue' => $oldValue,
	]));

Right before the legacy hook is emitted, which will call this place and trigger the event again.

[nickvergessen]

There is actually another problem.

The DAV SyncService listens to both, the User change event:

server/apps/dav/lib/HookManager.php

Line 96 in 3f9cdee

Util::connectHook('OC_User',

As well as the Account manager changeUser:

server/apps/dav/lib/AppInfo/Application.php

Lines 151 to 155 in 3f9cdee

	$dispatcher->addListener('OC\AccountManager::userUpdated', function(GenericEvent$event) {
	$user = $event->getSubject();
	$syncService = $this->getContainer()->query(SyncService::class);
	$syncService->updateUser($user);
	});

Therefor the vCard of the user is actually updated 3 times when changing the displayname:

1. User hook for the avatar
2. Account manager for the displayname
3. User hook for the display name

The main issue is, that the Account manager only triggers an update when email/display name changes, and not for any of the other values:

server/lib/private/Accounts/Hooks.php

Lines 68 to 81 in 3f9cdee

	switch ($feature) {
	case'eMailAddress':
	if ($accountData[AccountManager::PROPERTY_EMAIL]['value'] !== $newValue) {
	$accountData[AccountManager::PROPERTY_EMAIL]['value'] = $newValue;
	$accountManager->updateUser($user, $accountData);
	}
	break;
	case'displayName':
	if ($accountData[AccountManager::PROPERTY_DISPLAYNAME]['value'] !== $newValue) {
	$accountData[AccountManager::PROPERTY_DISPLAYNAME]['value'] = $newValue;
	$accountManager->updateUser($user, $accountData);
	}
	break;
	}

I think we should unify this too, getting the update count down to 2 at least.
That all together would cut down the user:add time from 3.5s to 0.8s for me.

[nickvergessen]

Extracted to #15022 so we can merge this quickly for 16

[MorrisJobke]

tested and works 👍

[rullzer]

Makes sense