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>

diff --git a/apps/dav/lib/CardDAV/CardDavBackend.php b/apps/dav/lib/CardDAV/CardDavBackend.php
index 8fb48d062d09a102bed5cc7eb483b5ca88d8d6c3..9d602025c7a49771e75ba3a966d891e5eb198103 100644 (file)
--- 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 c32aad5a056d114875741f33d587e6f4030ea690..0e507bc6b3db3fca3f1eef411d7294dcd92b90c2 100644 (file)
--- 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))