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>
This commit is contained in:
Morris Jobke 2020-07-07 14:54:26 +02:00
parent 09b9f94c38
commit aab646a9d0
No known key found for this signature in database
GPG Key ID: FE03C3A163FEDE68
2 changed files with 18 additions and 4 deletions

View File

@ -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);

View File

@ -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))