Merge pull request #30863 from nextcloud/performance/saving-user-profile-info

Minor optimizations for saving user personal information

3 files changed, 45 insertions, 32 deletions

diff --git a/apps/dav/lib/CardDAV/CardDavBackend.php b/apps/dav/lib/CardDAV/CardDavBackend.php
index 1c1754ff752..f5ed9a548d1 100644
--- a/apps/dav/lib/CardDAV/CardDavBackend.php
+++ b/apps/dav/lib/CardDAV/CardDavBackend.php
@@ -648,23 +648,26 @@ class CardDavBackend implements BackendInterface, SyncSupport {
 	 * @param mixed $addressBookId
 	 * @param string $cardUri
 	 * @param string $cardData
+	 * @param bool $checkAlreadyExists
 	 * @return string
 	 */
-	public function createCard($addressBookId, $cardUri, $cardData) {
+	public function createCard($addressBookId, $cardUri, $cardData, bool $checkAlreadyExists = true) {
 		$etag = md5($cardData);
 		$uid = $this->getUID($cardData);
 
-		$q = $this->db->getQueryBuilder();
-		$q->select('uid')
-			->from($this->dbCardsTable)
-			->where($q->expr()->eq('addressbookid', $q->createNamedParameter($addressBookId)))
-			->andWhere($q->expr()->eq('uid', $q->createNamedParameter($uid)))
-			->setMaxResults(1);
-		$result = $q->execute();
-		$count = (bool)$result->fetchOne();
-		$result->closeCursor();
-		if ($count) {
-			throw new \Sabre\DAV\Exception\BadRequest('VCard object with uid already exists in this addressbook collection.');
+		if ($checkAlreadyExists) {
+			$q = $this->db->getQueryBuilder();
+			$q->select('uid')
+				->from($this->dbCardsTable)
+				->where($q->expr()->eq('addressbookid', $q->createNamedParameter($addressBookId)))
+				->andWhere($q->expr()->eq('uid', $q->createNamedParameter($uid)))
+				->setMaxResults(1);
+			$result = $q->executeQuery();
+			$count = (bool)$result->fetchOne();
+			$result->closeCursor();
+			if ($count) {
+				throw new \Sabre\DAV\Exception\BadRequest('VCard object with uid already exists in this addressbook collection.');
+			}
 		}
 
 		$query = $this->db->getQueryBuilder();
@@ -1268,21 +1271,29 @@ class CardDavBackend implements BackendInterface, SyncSupport {
 				]
 			);
 
-		foreach ($vCard->children() as $property) {
-			if (!in_array($property->name, self::$indexProperties)) {
-				continue;
-			}
-			$preferred = 0;
-			foreach ($property->parameters as $parameter) {
-				if ($parameter->name === 'TYPE' && strtoupper($parameter->getValue()) === 'PREF') {
-					$preferred = 1;
-					break;
+
+		$this->db->beginTransaction();
+
+		try {
+			foreach ($vCard->children() as $property) {
+				if (!in_array($property->name, self::$indexProperties)) {
+					continue;
+				}
+				$preferred = 0;
+				foreach ($property->parameters as $parameter) {
+					if ($parameter->name === 'TYPE' && strtoupper($parameter->getValue()) === 'PREF') {
+						$preferred = 1;
+						break;
+					}
 				}
+				$query->setParameter('name', $property->name);
+				$query->setParameter('value', mb_strcut($property->getValue(), 0, 254));
+				$query->setParameter('preferred', $preferred);
+				$query->execute();
 			}
-			$query->setParameter('name', $property->name);
-			$query->setParameter('value', mb_strcut($property->getValue(), 0, 254));
-			$query->setParameter('preferred', $preferred);
-			$query->execute();
+			$this->db->commit();
+		} catch (\Exception $e) {
+			$this->db->rollBack();
 		}
 	}
 
diff --git a/apps/dav/lib/CardDAV/SyncService.php b/apps/dav/lib/CardDAV/SyncService.php
index b93fd94f741..e1ac3af5cc9 100644
--- a/apps/dav/lib/CardDAV/SyncService.php
+++ b/apps/dav/lib/CardDAV/SyncService.php
@@ -265,12 +265,12 @@ class SyncService {
 		$userId = $user->getUID();
 
 		$cardId = "$name:$userId.vcf";
-		$card = $this->backend->getCard($addressBookId, $cardId);
 		if ($user->isEnabled()) {
+			$card = $this->backend->getCard($addressBookId, $cardId);
 			if ($card === false) {
 				$vCard = $this->converter->createCardFromUser($user);
 				if ($vCard !== null) {
-					$this->backend->createCard($addressBookId, $cardId, $vCard->serialize());
+					$this->backend->createCard($addressBookId, $cardId, $vCard->serialize(), false);
 				}
 			} else {
 				$vCard = $this->converter->createCardFromUser($user);
diff --git a/apps/dav/lib/HookManager.php b/apps/dav/lib/HookManager.php
index f0fdd5cfd4f..b69d9b0cd79 100644
--- a/apps/dav/lib/HookManager.php
+++ b/apps/dav/lib/HookManager.php
@@ -105,10 +105,7 @@ class HookManager {
 			$this->postDeleteUser(['uid' => $uid]);
 		});
 		\OC::$server->getUserManager()->listen('\OC\User', 'postUnassignedUserId', [$this, 'postUnassignedUserId']);
-		Util::connectHook('OC_User',
-			'changeUser',
-			$this,
-			'changeUser');
+		Util::connectHook('OC_User', 'changeUser', $this, 'changeUser');
 	}
 
 	public function postCreateUser($params) {
@@ -164,7 +161,12 @@ class HookManager {
 
 	public function changeUser($params) {
 		$user = $params['user'];
-		$this->syncService->updateUser($user);
+		$feature = $params['feature'];
+		// This case is already covered by the account manager firing up a signal
+		// later on
+		if ($feature !== 'eMailAddress' && $feature !== 'displayName') {
+			$this->syncService->updateUser($user);
+		}
 	}
 
 	public function firstLogin(IUser $user = null) {