From aab646a9d0f67f42fe8eb3398e6b4b726c501585 Mon Sep 17 00:00:00 2001
From: Morris Jobke <hey@morrisjobke.de>
Date: Tue, 7 Jul 2020 14:54:26 +0200
Subject: [PATCH] Update system addressbook card only when there was a change
 based on a cached etag

Due to our old and new hook system the card dav backend listens to old and new hooks. This triggers this code multiple times and always causes an update. With this change we cache the etag during a request and only trigger the update if the etag has changed. This does not catches all not needed updates, but it does not need another round trip to the database and still covers most cases where multiple attributes are updated during one single request.

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
---
 apps/dav/lib/CardDAV/CardDavBackend.php            | 14 ++++++++++++++
 apps/dav/tests/unit/CardDAV/CardDavBackendTest.php |  8 ++++----
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/apps/dav/lib/CardDAV/CardDavBackend.php b/apps/dav/lib/CardDAV/CardDavBackend.php
index 8fb48d062d..9d602025c7 100644
--- a/apps/dav/lib/CardDAV/CardDavBackend.php
+++ b/apps/dav/lib/CardDAV/CardDavBackend.php
@@ -90,6 +90,8 @@ class CardDavBackend implements BackendInterface, SyncSupport {
 	/** @var EventDispatcherInterface */
 	private $dispatcher;
 
+	private $etagCache = [];
+
 	/**
 	 * CardDavBackend constructor.
 	 *
@@ -653,6 +655,9 @@ class CardDavBackend implements BackendInterface, SyncSupport {
 			])
 			->execute();
 
+		$etagCacheKey = "$addressBookId#$cardUri";
+		$this->etagCache[$etagCacheKey] = $etag;
+
 		$this->addChange($addressBookId, $cardUri, 1);
 		$this->updateProperties($addressBookId, $cardUri, $cardData);
 
@@ -694,6 +699,13 @@ class CardDavBackend implements BackendInterface, SyncSupport {
 		$uid = $this->getUID($cardData);
 		$etag = md5($cardData);
 		$query = $this->db->getQueryBuilder();
+
+		// check for recently stored etag and stop if it is the same
+		$etagCacheKey = "$addressBookId#$cardUri";
+		if (isset($this->etagCache[$etagCacheKey]) && $this->etagCache[$etagCacheKey] === $etag) {
+			return '"' . $etag . '"';
+		}
+
 		$query->update($this->dbCardsTable)
 			->set('carddata', $query->createNamedParameter($cardData, IQueryBuilder::PARAM_LOB))
 			->set('lastmodified', $query->createNamedParameter(time()))
@@ -704,6 +716,8 @@ class CardDavBackend implements BackendInterface, SyncSupport {
 			->andWhere($query->expr()->eq('addressbookid', $query->createNamedParameter($addressBookId)))
 			->execute();
 
+		$this->etagCache[$etagCacheKey] = $etag;
+
 		$this->addChange($addressBookId, $cardUri, 2);
 		$this->updateProperties($addressBookId, $cardUri, $cardData);
 
diff --git a/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php b/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php
index c32aad5a05..0e507bc6b3 100644
--- a/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php
+++ b/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php
@@ -253,7 +253,7 @@ class CardDavBackendTest extends TestCase {
 		$uri = $this->getUniqueID('card');
 		// updateProperties is expected twice, once for createCard and once for updateCard
 		$backend->expects($this->at(0))->method('updateProperties')->with($bookId, $uri, $this->vcardTest0);
-		$backend->expects($this->at(1))->method('updateProperties')->with($bookId, $uri, $this->vcardTest0);
+		$backend->expects($this->at(1))->method('updateProperties')->with($bookId, $uri, $this->vcardTest1);
 
 		// Expect event
 		$this->dispatcher->expects($this->at(0))
@@ -288,13 +288,13 @@ class CardDavBackendTest extends TestCase {
 			->with('\OCA\DAV\CardDAV\CardDavBackend::updateCard', $this->callback(function (GenericEvent $e) use ($bookId, $uri) {
 				return $e->getArgument('addressBookId') === $bookId &&
 					$e->getArgument('cardUri') === $uri &&
-					$e->getArgument('cardData') === $this->vcardTest0;
+					$e->getArgument('cardData') === $this->vcardTest1;
 			}));
 
 		// update the card
-		$backend->updateCard($bookId, $uri, $this->vcardTest0);
+		$backend->updateCard($bookId, $uri, $this->vcardTest1);
 		$card = $backend->getCard($bookId, $uri);
-		$this->assertEquals($this->vcardTest0, $card['carddata']);
+		$this->assertEquals($this->vcardTest1, $card['carddata']);
 
 		// Expect event
 		$this->dispatcher->expects($this->at(0))