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>

2 files changed, 18 insertions, 4 deletions

diff --git a/apps/dav/lib/CardDAV/CardDavBackend.php b/apps/dav/lib/CardDAV/CardDavBackend.php
index 8fb48d062d0..9d602025c7a 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 c32aad5a056..0e507bc6b3d 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))