diff --git a/CHANGELOG.md b/CHANGELOG.md index e6f1339e..eba08816 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ - Support several levels of departments separated by semicolon that end up as structured value in the VCard - Fix #318: Some attributes (e.g. gender) could not be deleted when updating a contact - Fix #53: Only create displayname when not present in VCard / not provided by roundcube +- New: Download externally referenced photos on demand, drastically speeding up sync with when photos are stored + separately from the VCard (e.g. iCloud). For details see #247. ## Version 4.0.3 (to 4.0.2) diff --git a/carddav.php b/carddav.php index 940f2c87..01ae1eb4 100644 --- a/carddav.php +++ b/carddav.php @@ -68,6 +68,9 @@ class carddav extends rcube_plugin /** @var Database $db */ private $db; + /** @var ?rcube_cache $cache */ + private $cache; + // the dummy task is used by the calendar plugin, which requires // the addressbook to be initialized public $task = 'addressbook|login|mail|settings|dummy'; @@ -117,7 +120,13 @@ public function __construct($api, array $options = []) $this->logger = $options["logger"] ?? new RoundcubeLogger("carddav", \Psr\Log\LogLevel::ERROR); $this->httpLogger = $options["logger_http"] ?? new RoundcubeLogger("carddav_http", \Psr\Log\LogLevel::ERROR); - $this->db = $options["db"] ?? new Database($this->logger, \rcube::get_instance()->db); + $rcube = \rcube::get_instance(); + $this->db = $options["db"] ?? new Database($this->logger, $rcube->db); + + if (isset($options["cache"])) { + $this->cache = $options["cache"]; + // the roundcube cache object cannot be retrieved at this point + } } public function init(): void @@ -352,7 +361,15 @@ public function getAddressbook(array $p): array $this->decryptPassword($config["password"]) ); - $abook = new Addressbook($abookId, $this->db, $logger, $config, $readonly, $requiredProps); + $abook = new Addressbook( + $abookId, + $this->db, + $this->getRoundcubeCache(), + $logger, + $config, + $readonly, + $requiredProps + ); $p['instance'] = $abook; // refresh the address book if the update interval expired this requires a completely initialized @@ -1114,6 +1131,26 @@ private function getAdminSettings(): array return $prefs; } + /** + * Returns a handle to the roundcube cache for the user. + * + * Note: this must be called at a time where the user is already logged on, specifically it must not be called + * during the constructor of this plugin. + */ + private function getRoundcubeCache(): rcube_cache + { + if (!isset($this->cache)) { + // TODO make TTL and cache type configurable + $this->cache = rcube::get_instance()->get_cache("carddav", "db", "1w"); + } + + if (!isset($this->cache)) { + throw new \Exception("Attempt to request cache where not available yet"); + } + + return $this->cache; + } + // password helpers private function getDesKey(): string { diff --git a/src/Addressbook.php b/src/Addressbook.php index 3ca65fce..56d830af 100644 --- a/src/Addressbook.php +++ b/src/Addressbook.php @@ -41,6 +41,9 @@ class Addressbook extends rcube_addressbook /** @var Database $db Database access object */ private $db; + /** @var \rcube_cache $cache */ + private $cache; + /** @var LoggerInterface $logger Log object */ private $logger; @@ -85,6 +88,7 @@ class Addressbook extends rcube_addressbook public function __construct( string $dbid, Database $db, + \rcube_cache $cache, LoggerInterface $logger, array $config, bool $readonly, @@ -93,13 +97,14 @@ public function __construct( $this->logger = $logger; $this->config = $config; $this->db = $db; + $this->cache = $cache; $this->groups = true; $this->readonly = $readonly; $this->requiredProps = $requiredProps; $this->id = $dbid; - $this->dataConverter = new DataConversion($dbid, $db, $logger); + $this->dataConverter = new DataConversion($dbid, $db, $cache, $logger); $this->coltypes = $this->dataConverter->getColtypes(); $this->ready = true; @@ -396,7 +401,7 @@ public function get_record($id, $assoc = false) $davAbook = $this->getCardDavObj(); $contact = $db->get($id, 'vcard', 'contacts', true, 'id', ["abook_id" => $this->id]); $vcard = $this->parseVCard($contact['vcard']); - [ 'save_data' => $save_data ] = $this->dataConverter->toRoundcube($vcard, $davAbook); + $save_data = $this->dataConverter->toRoundcube($vcard, $davAbook); $save_data['ID'] = $id; $this->result = new rcube_result_set(1); @@ -696,7 +701,7 @@ public function get_group($group_id): array * * @param string $name The group name * - * @return mixed False on error, array with record props in success + * @return array|false False on error, array with record props in success */ // phpcs:ignore PSR1.Methods.CamelCapsMethodName -- method name defined by rcube_addressbook class public function create_group($name) @@ -784,7 +789,7 @@ function (array &$groups, string $_contact_id) use ($groupname): bool { * @param string $newname New name to set for this group * @param string &$newid New group identifier (if changed, otherwise don't set) * - * @return boolean|string New name on success, false if no data was changed + * @return string|false New name on success, false if no data was changed */ // phpcs:ignore PSR1.Methods.CamelCapsMethodName -- method name defined by rcube_addressbook class public function rename_group($group_id, $newname, &$newid) @@ -1169,10 +1174,8 @@ private function listRecordsReadDB(?array $cols, int $subset, rcube_result_set $ // needed by the calendar plugin if (is_array($cols) && in_array('vcard', $cols)) { - $save_data['save_data']['vcard'] = $contact['vcard']; + $save_data['vcard'] = $contact['vcard']; } - - $save_data = $save_data['save_data']; } else { $save_data = []; $cols = $cols ?? []; // note: $cols is always an array at this point, this is for the static analyzer diff --git a/src/DataConversion.php b/src/DataConversion.php index a9ea1626..3211f688 100644 --- a/src/DataConversion.php +++ b/src/DataConversion.php @@ -11,12 +11,6 @@ class DataConversion { - /** - * @var int MAX_PHOTO_SIZE Maximum size of a photo dimension in pixels. - * Used when a photo is cropped for the X-ABCROP-RECTANGLE extension. - */ - private const MAX_PHOTO_SIZE = 256; - /** @var array VCF2RC maps VCard property names to roundcube keys */ private const VCF2RC = [ 'simple' => [ @@ -97,6 +91,9 @@ class DataConversion /** @var Database The database object to use for DB access */ private $db; + /** @var \rcube_cache $cache */ + private $cache; + /** * Constructs a data conversion instance. * @@ -109,12 +106,14 @@ class DataConversion * * @param string $abookId The database ID of the addressbook the data conversion object is bound to. * @param Database $db The database object. + * @param \rcube_cache $cache The roundcube cache object. * @param LoggerInterface $logger The logger object. */ - public function __construct(string $abookId, Database $db, LoggerInterface $logger) + public function __construct(string $abookId, Database $db, \rcube_cache $cache, LoggerInterface $logger) { $this->abookId = $abookId; $this->db = $db; + $this->cache = $cache; $this->logger = $logger; $this->addextrasubtypes(); @@ -138,14 +137,10 @@ public function getColtypes(): array * * @param VCard $vcard Sabre VCard object * - * @return array associative array with keys: - * - save_data: Roundcube representation of the VCard - * - vcf: VCard object created from the given VCard - * - needs_update: boolean that indicates whether the card was modified + * @return array Roundcube representation of the VCard */ public function toRoundcube(VCard $vcard, AddressbookCollection $davAbook): array { - $needs_update = false; $save_data = [ // DEFAULTS 'kind' => 'individual', @@ -158,26 +153,9 @@ public function toRoundcube(VCard $vcard, AddressbookCollection $davAbook): arra } } - // inline photo if external reference - // note: isset($vcard->PHOTO) is true if $save_data['photo'] exists, the check - // is for the static analyzer + // Set a proxy for photo computation / retrieval on demand if (key_exists('photo', $save_data) && isset($vcard->PHOTO)) { - $kind = $vcard->PHOTO['VALUE']; - if (($kind instanceof VObject\Parameter) && strcasecmp('uri', (string) $kind) == 0) { - if ($this->downloadPhoto($save_data, $davAbook)) { - $props = []; - foreach ($vcard->PHOTO->parameters() as $property => $value) { - if (strcasecmp($property, 'VALUE') != 0) { - $props[$property] = $value; - } - } - $props['ENCODING'] = 'b'; - unset($vcard->PHOTO); - $vcard->add('PHOTO', $save_data['photo'], $props); - $needs_update = true; - } - } - $save_data["photo"] = self::xabcropphoto($vcard->PHOTO) ?? $save_data["photo"]; + $save_data["photo"] = new DelayedPhotoLoader($vcard, $davAbook, $this->cache, $this->logger); } $property = $vcard->N; @@ -225,11 +203,7 @@ public function toRoundcube(VCard $vcard, AddressbookCollection $davAbook): arra $save_data["name"] = self::composeDisplayname($save_data); } - return [ - 'save_data' => $save_data, - 'vcf' => $vcard, - 'needs_update' => $needs_update, - ]; + return $save_data; } @@ -674,21 +648,6 @@ private function addextrasubtypes(): void } } - private function downloadPhoto(array &$save_data, AddressbookCollection $davAbook): bool - { - $uri = $save_data['photo']; - try { - $this->logger->info("downloadPhoto: Attempt to download photo from $uri"); - $response = $davAbook->downloadResource($uri); - $save_data['photo'] = $response['body']; - } catch (\Exception $e) { - $this->logger->warning("downloadPhoto: Attempt to download photo from $uri failed: $e"); - return false; - } - - return true; - } - /****************************************************************************************************************** ************ + + + ************ ************ X-ABShowAs Extension ************ @@ -780,59 +739,6 @@ private static function composeDisplayname(array $save_data): string // still no name? set to unknown and hope the user will fix it return 'Unset Displayname'; } - - /****************************************************************************************************************** - ************ + + + ************ - ************ X-ABCROP-RECTANGLE Extension ************ - ************ + + + ************ - *****************************************************************************************************************/ - - /** - * Crops the given PHOTO property if it contains an X-ABCROP-RECTANGLE parameter. - * - * The parameter looks like this: - * X-ABCROP-RECTANGLE=ABClipRect_1&60&179&181&181&qZ54yqewvBZj2mycxrnqsA== - * - * - The 1st number is the horizontal offset (X) from the left - * - The 2nd number is the vertical offset (Y) from the bottom - * - The 3rd number is the crop width - * - The 4th number is the crop height - * - * The meaning of the base64 encoded last part of the parameter is unknown and ignored. - * - * The resulting cropped photo is returned as binary string. In case the given photo lacks the X-ABCROP-RECTANGLE - * parameter or the GD library is not available, null is returned instead. - */ - private static function xabcropphoto(VObject\Property $photo): ?string - { - if (!function_exists('gd_info')) { - return null; - } - - $abcrop = $photo['X-ABCROP-RECTANGLE']; - if (!($abcrop instanceof VObject\Parameter)) { - return null; - } - - $parts = explode('&', (string) $abcrop); - $x = intval($parts[1]); - $y = intval($parts[2]); - $w = intval($parts[3]); - $h = intval($parts[4]); - $dw = min($w, self::MAX_PHOTO_SIZE); - $dh = min($h, self::MAX_PHOTO_SIZE); - - $src = imagecreatefromstring((string) $photo); - $dst = imagecreatetruecolor($dw, $dh); - imagecopyresampled($dst, $src, 0, 0, $x, imagesy($src) - $y - $h, $dw, $dh, $w, $h); - - ob_start(); - imagepng($dst); - $data = ob_get_contents(); - ob_end_clean(); - - return $data; - } } // vim: ts=4:sw=4:expandtab:fenc=utf8:ff=unix:tw=120 diff --git a/src/Database.php b/src/Database.php index 732d7a9f..960ab5cc 100644 --- a/src/Database.php +++ b/src/Database.php @@ -35,8 +35,6 @@ class Database /** * Initializes a Database instance. * - * Must be called before using any methods in this class. - * * @param rcube_db $dbh The roundcube database handle */ public function __construct(LoggerInterface $logger, rcube_db $dbh) diff --git a/src/DelayedPhotoLoader.php b/src/DelayedPhotoLoader.php new file mode 100644 index 00000000..44616485 --- /dev/null +++ b/src/DelayedPhotoLoader.php @@ -0,0 +1,265 @@ +vcard = $vcard; + $this->davAbook = $davAbook; + $this->cache = $cache; + $this->logger = $logger; + + $logger->debug("Wrapping photo"); + } + + /** + * Retrieves the picture data. + * + * This is done as the implicit string conversion to allow triggering the retrieval in roundcube's code when the + * photo data is actually requested. + */ + public function __toString(): string + { + $this->logger->debug("Unwrapping photo"); + return $this->computePhotoFromProperty(); + } + + /** + * Computes the photo data from the PHOTO property. + * + * Processing and cache retrieval/update are performed as necessary. + * + * @return string The processed photo data. An empty string in case of error or no photo available. + */ + private function computePhotoFromProperty(): string + { + + $vcard = $this->vcard; + $photoProp = $vcard->PHOTO; + if (!isset($photoProp)) { + return ""; + } + + $this->logger->debug("Creating photo data from PHOTO property"); + + // First we determine whether the photo needs processing (download/crop) + $cropProp = $photoProp['X-ABCROP-RECTANGLE']; + + // check if photo needs to be downloaded + $kind = $photoProp['VALUE']; + if (($kind instanceof VObject\Parameter) && strcasecmp('uri', (string) $kind) == 0) { + $photoUri = (string) $photoProp; + } + + // true if the photo must be processed (downloaded/cropped) and the result should be cached + // Photo that are stored inline in the VCard and provided as is will not be put in the cache + $cachePhoto = ($cropProp instanceof VObject\Parameter) || isset($photoUri); + + // check roundcube cache + if ($cachePhoto) { + $photoData = $this->fetchFromRoundcubeCache($photoProp); + if (isset($photoData)) { + return $photoData; + } + } + + // retrieve PHOTO data + if (isset($photoUri)) { + $photoData = $this->downloadPhoto($photoUri); + } else { + $photoData = (string) $photoProp; + } + + // crop photo if needed + if (isset($photoData) && ($cropProp instanceof VObject\Parameter)) { + $photoData = $this->xabcropphoto($photoData, $cropProp) ?? $photoData; + } + + // store to cache if requested + if (isset($photoData) && $cachePhoto) { + $this->storeToRoundcubeCache($photoData, $photoProp); + } + + return $photoData ?? ""; + } + + private function downloadPhoto(string $uri): ?string + { + try { + $this->logger->debug("downloadPhoto: Attempt to download photo from $uri"); + $response = $this->davAbook->downloadResource($uri); + return $response['body']; + } catch (\Exception $e) { + $this->logger->warning("downloadPhoto: Attempt to download photo from $uri failed: $e"); + } + + return null; + } + + /** + * Fetches the photo for this property from the roundcube cache (if available). + * + * The function checks that the cache object still matches the photo property, otherwise the cache object is pruned + * and this function returns null to trigger a recomputation. + * + * @return ?string Returns the photo data from the roundcube cache, null if not present or outdated. + */ + private function fetchFromRoundcubeCache(VObject\Property $photoProp): ?string + { + $key = $this->determineCacheKey(); + $cacheObject = $this->cache->get($key); + + if (!isset($cacheObject)) { + $this->logger->debug("Roundcube cache miss $key"); + return null; + } + + if (md5($photoProp->serialize()) !== $cacheObject["photoPropMd5"]) { + $this->logger->debug("Roundcube cached photo outdated - removing $key"); + $this->cache->remove($key); + return null; + } + + $this->logger->debug("Roundcube cache hit $key"); + return $cacheObject["photo"]; + } + + /** + * Stores the photo data to the roundcube cache. + * + * The cache object includes a checksum that allows to check whether the stored object matches a possibly changed + * PHOTO property on future retrieval. + */ + private function storeToRoundcubeCache(string $photoData, VObject\Property $photoProp): void + { + $photoPropMd5 = md5($photoProp->serialize()); + $cacheObject = [ + 'photoPropMd5' => $photoPropMd5, + 'photo' => $photoData + ]; + + $key = $this->determineCacheKey(); + $this->logger->debug("Storing to roundcube cache $key"); + $this->cache->set($key, $cacheObject); + } + + /** + * Compute the key for this photo property in the roundcube cache. + * + * The key is composed of the following components separated by _: + * - a prefix "photo" (namespace to separate from other uses by the plugin) + * - a component including the user id of the roundcube user (only keys of logged in user can be retrieved); + * probably not needed as the cache itself is user-specific, but just in case. + * - a component containing the MD5 of the card UID (to find a photo cached for a VCard) + */ + private function determineCacheKey(): string + { + $uid = (string) $this->vcard->UID; + + $key = "photo_"; + $key .= $_SESSION['user_id'] . "_" ; + $key .= md5($uid); + + return $key; + } + + /****************************************************************************************************************** + ************ + + + ************ + ************ X-ABCROP-RECTANGLE Extension ************ + ************ + + + ************ + *****************************************************************************************************************/ + + /** + * Crops the given photo if the PHOTO property contains an X-ABCROP-RECTANGLE parameter. + * + * The parameter looks like this: + * X-ABCROP-RECTANGLE=ABClipRect_1&60&179&181&181&qZ54yqewvBZj2mycxrnqsA== + * + * - The 1st number is the horizontal offset (X) from the left + * - The 2nd number is the vertical offset (Y) from the bottom + * - The 3rd number is the crop width + * - The 4th number is the crop height + * + * The meaning of the base64 encoded last part of the parameter is unknown and ignored. + * + * @return ?string The resulting cropped photo as binary string. Null in case the given photo was not modified, + * e.g. for lack of the X-ABCROP-RECTANGLE parameter or GD is not available. + */ + private function xabcropphoto(string $photoData, VObject\Parameter $cropProp): ?string + { + if (!function_exists('gd_info')) { + // @codeCoverageIgnoreStart + return null; + // @codeCoverageIgnoreEnd + } + + $parts = explode('&', (string) $cropProp); + $x = intval($parts[1]); + $y = intval($parts[2]); + $w = intval($parts[3]); + $h = intval($parts[4]); + $dw = min($w, self::MAX_PHOTO_SIZE); + $dh = min($h, self::MAX_PHOTO_SIZE); + + $src = imagecreatefromstring($photoData); + $dst = imagecreatetruecolor($dw, $dh); + imagecopyresampled($dst, $src, 0, 0, $x, imagesy($src) - $y - $h, $dw, $dh, $w, $h); + + ob_start(); + imagepng($dst); + $photoData = ob_get_contents(); + ob_end_clean(); + + return $photoData; + } +} + +// vim: ts=4:sw=4:expandtab:fenc=utf8:ff=unix:tw=120 diff --git a/src/SyncHandlerRoundcube.php b/src/SyncHandlerRoundcube.php index b34d087f..c7e343b7 100644 --- a/src/SyncHandlerRoundcube.php +++ b/src/SyncHandlerRoundcube.php @@ -300,8 +300,7 @@ private function updateContactCard(string $uri, string $etag, VCard $card): void $abookId = $this->rcAbook->getId(); $db = $this->db; - // card may be changed during conversion, in particular inlining of the PHOTO - [ 'save_data' => $save_data, 'vcf' => $card ] = $this->dataConverter->toRoundcube($card, $this->davAbook); + $save_data = $this->dataConverter->toRoundcube($card, $this->davAbook); $this->logger->info("Changed Individual $uri " . $save_data['name']); $dbid = $this->localCards[$uri]["id"] ?? null; @@ -369,8 +368,7 @@ private function updateGroupCard(string $uri, string $etag, VCard $card): void $dbh = $db->getDbHandle(); $abookId = $this->rcAbook->getId(); - // card may be changed during conversion, in particular inlining of the PHOTO - [ 'save_data' => $save_data, 'vcf' => $card ] = $this->dataConverter->toRoundcube($card, $this->davAbook); + $save_data = $this->dataConverter->toRoundcube($card, $this->davAbook); $dbid = $this->localGrpCards[$uri]["id"] ?? null; if (isset($dbid)) { diff --git a/tests/dbinterop/TestData.php b/tests/dbinterop/TestData.php index 6f69485f..d402e793 100644 --- a/tests/dbinterop/TestData.php +++ b/tests/dbinterop/TestData.php @@ -133,8 +133,7 @@ public static function initDatabase(Database $db): void TestCase::assertArrayHasKey($tbl, self::$data, "No init data for table $tbl"); foreach (self::$data[$tbl] as &$row) { - $dbid = self::insertRow($db, $tbl, $cols, $row); - $row["id"] = $dbid; + $row["id"] = self::insertRow($db, $tbl, $cols, $row); } } } diff --git a/tests/unit/DataConversionTest.php b/tests/unit/DataConversionTest.php index 755f3202..a5de378e 100644 --- a/tests/unit/DataConversionTest.php +++ b/tests/unit/DataConversionTest.php @@ -10,13 +10,14 @@ use MStilkerich\Tests\CardDavAddressbook4Roundcube\TestInfrastructure; use PHPUnit\Framework\TestCase; use MStilkerich\CardDavClient\{Account,AddressbookCollection}; -use MStilkerich\CardDavAddressbook4Roundcube\{Database,DataConversion}; +use MStilkerich\CardDavAddressbook4Roundcube\{Database,DataConversion,DelayedPhotoLoader}; final class DataConversionTest extends TestCase { public static function setUpBeforeClass(): void { TestInfrastructure::init(); + $_SESSION['user_id'] = 105; } public function setUp(): void @@ -53,13 +54,12 @@ public function vcardImportSamplesProvider(): array */ public function testCorrectConversionOfVcardToRoundcube(string $vcfFile, string $jsonFile): void { - [ $logger, $db, $abook ] = $this->initStubs(); + [ $logger, $db, $cache, $abook ] = $this->initStubs(); - $dc = new DataConversion("42", $db, $logger); + $dc = new DataConversion("42", $db, $cache, $logger); $vcard = $this->readVCard($vcfFile); $saveDataExpected = $this->readJsonArray($jsonFile); - $result = $dc->toRoundcube($vcard, $abook); - $saveData = $result["save_data"]; + $saveData = $dc->toRoundcube($vcard, $abook); $this->assertEquals($saveDataExpected, $saveData, "Converted VCard does not result in expected roundcube data"); } @@ -68,7 +68,7 @@ public function testCorrectConversionOfVcardToRoundcube(string $vcfFile, string */ public function testNewCustomLabelIsInsertedToDatabase(): void { - [ $logger, $db, $abook ] = $this->initStubs(); + [ $logger, $db, $cache, $abook ] = $this->initStubs(); $db->expects($this->once()) ->method("get") @@ -89,7 +89,7 @@ public function testNewCustomLabelIsInsertedToDatabase(): void ) ->will($this->returnValue("49")); - $dc = new DataConversion("42", $db, $logger); + $dc = new DataConversion("42", $db, $cache, $logger); $vcard = $this->readVCard("tests/unit/data/vcardImport/XAbLabel.vcf"); $dc->toRoundcube($vcard, $abook); } @@ -99,7 +99,7 @@ public function testNewCustomLabelIsInsertedToDatabase(): void */ public function testKnownCustomLabelPresentedToRoundcube(): void { - [ $logger, $db ] = $this->initStubs(); + [ $logger, $db, $cache ] = $this->initStubs(); $db->expects($this->once()) ->method("get") @@ -112,7 +112,7 @@ public function testKnownCustomLabelPresentedToRoundcube(): void ) ->will($this->returnValue([ ["typename" => "email", "subtype" => "SpecialLabel"] ])); - $dc = new DataConversion("42", $db, $logger); + $dc = new DataConversion("42", $db, $cache, $logger); $coltypes = $dc->getColtypes(); $this->assertContains("SpecialLabel", $coltypes["email"]["subtypes"], "SpecialLabel not contained in coltypes"); } @@ -122,7 +122,7 @@ public function testKnownCustomLabelPresentedToRoundcube(): void */ public function testKnownCustomLabelIsNotInsertedToDatabase(): void { - [ $logger, $db, $abook ] = $this->initStubs(); + [ $logger, $db, $cache, $abook ] = $this->initStubs(); $db->expects($this->once()) ->method("get") @@ -137,7 +137,7 @@ public function testKnownCustomLabelIsNotInsertedToDatabase(): void $db->expects($this->never()) ->method("insert"); - $dc = new DataConversion("42", $db, $logger); + $dc = new DataConversion("42", $db, $cache, $logger); $vcard = $this->readVCard("tests/unit/data/vcardImport/XAbLabel.vcf"); $dc->toRoundcube($vcard, $abook); } @@ -154,7 +154,7 @@ public function vcardCreateSamplesProvider(): array */ public function testCorrectCreationOfVcardFromRoundcube(string $vcfFile, string $jsonFile): void { - [ $logger, $db ] = $this->initStubs(); + [ $logger, $db, $cache ] = $this->initStubs(); $db->expects($this->once()) ->method("get") @@ -166,7 +166,7 @@ public function testCorrectCreationOfVcardFromRoundcube(string $vcfFile, string $this->equalTo('abook_id') ) ->will($this->returnValue([ ["typename" => "email", "subtype" => "SpecialLabel"] ])); - $dc = new DataConversion("42", $db, $logger); + $dc = new DataConversion("42", $db, $cache, $logger); $vcardExpected = $this->readVCard($vcfFile); $saveData = $this->readJsonArray($jsonFile); $result = $dc->fromRoundcube($saveData); @@ -186,7 +186,7 @@ public function vcardUpdateSamplesProvider(): array */ public function testCorrectUpdateOfVcardFromRoundcube(string $vcfFile, string $jsonFile): void { - [ $logger, $db ] = $this->initStubs(); + [ $logger, $db, $cache ] = $this->initStubs(); $db->expects($this->once()) ->method("get") @@ -202,7 +202,7 @@ public function testCorrectUpdateOfVcardFromRoundcube(string $vcfFile, string $j ["typename" => "email", "subtype" => "SpecialLabel2"] ])); - $dc = new DataConversion("42", $db, $logger); + $dc = new DataConversion("42", $db, $cache, $logger); $vcardOriginal = $this->readVCard($vcfFile); $vcardExpected = $this->readVCard("$vcfFile.new"); $saveData = $this->readJsonArray($jsonFile); @@ -211,6 +211,184 @@ public function testCorrectUpdateOfVcardFromRoundcube(string $vcfFile, string $j $this->compareVCards($vcardExpected, $result); } + public function cachePhotosSamplesProvider(): array + { + return [ + "InlinePhoto.vcf" => ["tests/unit/data/vcardImport/InlinePhoto", false, false], + "UriPhotoCrop.vcf" => ["tests/unit/data/vcardImport/UriPhotoCrop", true, true], + "InvalidUriPhoto.vcf" => ["tests/unit/data/vcardImport/InvalidUriPhoto", true, false], + "UriPhoto.vcf" => ["tests/unit/data/vcardImport/UriPhoto", true, true], + ]; + } + + /** + * Tests whether a PHOTO is stored/not stored to the roundcube cache as expected. + * + * @dataProvider cachePhotosSamplesProvider + */ + public function testNewPhotoIsStoredToCacheIfNeeded(string $basename, bool $getExp, bool $storeExp): void + { + [ $logger, $db, $cache, $abook ] = $this->initStubs(); + + $vcard = $this->readVCard("$basename.vcf"); + $this->assertInstanceOf(VObject\Property::class, $vcard->PHOTO); + + $key = "photo_105_" . md5((string) $vcard->UID); + $saveDataExpected = $this->readJsonArray("$basename.json"); + + + // photo should be stored to cache if not already stored in vcard in the final form + if ($getExp) { + // simulate cache miss + $cache->expects($this->once()) + ->method("get") + ->with($this->equalTo($key)) + ->will($this->returnValue(null)); + } else { + $cache->expects($this->never())->method("get"); + } + + if ($storeExp) { + $cache->expects($this->once()) + ->method("set") + ->with( + $this->equalTo($key), + $this->equalTo([ + 'photoPropMd5' => md5($vcard->PHOTO->serialize()), + 'photo' => $saveDataExpected["photo"] + ]) + ) + ->will($this->returnValue(true)); + } else { + $cache->expects($this->never())->method("set"); + } + + $dc = new DataConversion("42", $db, $cache, $logger); + $saveData = $dc->toRoundcube($vcard, $abook); + $this->assertEquals($saveDataExpected["photo"], $saveData["photo"]); + } + + /** + * Tests that a photo is retrieved from the roundcube cache if available, skipping processing. + * + * @dataProvider cachePhotosSamplesProvider + */ + public function testPhotoIsUsedFromCacheIfAvailable(string $basename, bool $getExp, bool $storeExp): void + { + [ $logger, $db, $cache, $abook ] = $this->initStubs(); + + // we use this file as some placeholder for cached data that is not used in any of the vcards photos + $cachedPhotoData = file_get_contents("tests/unit/data/srv/pixel.jpg"); + $this->assertNotFalse($cachedPhotoData); + + $vcard = $this->readVCard("$basename.vcf"); + $this->assertInstanceOf(VObject\Property::class, $vcard->PHOTO); + + $key = "photo_105_" . md5((string) $vcard->UID); + $saveDataExpected = $this->readJsonArray("$basename.json"); + + if ($getExp) { + // simulate cache hit + $cache->expects($this->once()) + ->method("get") + ->with($this->equalTo($key)) + ->will( + $this->returnValue([ + 'photoPropMd5' => md5($vcard->PHOTO->serialize()), + 'photo' => $cachedPhotoData + ]) + ); + } else { + $cache->expects($this->never())->method("get"); + } + + // no cache update expected + $cache->expects($this->never())->method("set"); + + $dc = new DataConversion("42", $db, $cache, $logger); + $saveData = $dc->toRoundcube($vcard, $abook); + + if ($getExp) { + $this->assertEquals($cachedPhotoData, $saveData["photo"]); + } else { + $this->assertEquals($saveDataExpected["photo"], $saveData["photo"]); + } + } + + /** + * Tests that an outdated photo in the cache is replaced by a newly processed one. + * + * @dataProvider cachePhotosSamplesProvider + */ + public function testOutdatedPhotoIsReplacedInCache(string $basename, bool $getExp, bool $storeExp): void + { + [ $logger, $db, $cache, $abook ] = $this->initStubs(); + + // we use this file as some placeholder for cached data that is not used in any of the vcards photos + $cachedPhotoData = file_get_contents("tests/unit/data/srv/pixel.jpg"); + $this->assertNotFalse($cachedPhotoData); + + $vcard = $this->readVCard("$basename.vcf"); + $this->assertInstanceOf(VObject\Property::class, $vcard->PHOTO); + + $key = "photo_105_" . md5((string) $vcard->UID); + $saveDataExpected = $this->readJsonArray("$basename.json"); + + if ($getExp) { + // simulate cache hit with non-matching md5sum + $cache->expects($this->once()) + ->method("get") + ->with($this->equalTo($key)) + ->will( + $this->returnValue([ + 'photoPropMd5' => md5("foo"), // will not match the current md5 + 'photo' => $cachedPhotoData + ]) + ); + + // expect that the old record is purged + $cache->expects($this->once()) + ->method("remove") + ->with($this->equalTo($key)); + } else { + $cache->expects($this->never())->method("get"); + } + + // a new record should be inserted if photo requires caching + if ($storeExp) { + $cache->expects($this->once()) + ->method("set") + ->with( + $this->equalTo($key), + $this->equalTo([ + 'photoPropMd5' => md5($vcard->PHOTO->serialize()), + 'photo' => $saveDataExpected["photo"] + ]) + ) + ->will($this->returnValue(true)); + } else { + $cache->expects($this->never())->method("set"); + } + + $dc = new DataConversion("42", $db, $cache, $logger); + $saveData = $dc->toRoundcube($vcard, $abook); + $this->assertEquals($saveDataExpected["photo"], $saveData["photo"]); + } + + /** + * Tests that a delayed photo loader handles vcards lacking a PHOTO property. + */ + public function testPhotoloaderHandlesVcardWithoutPhotoProperty(): void + { + [ $logger, $_db, $cache, $abook ] = $this->initStubs(); + + $vcard = $this->readVCard("tests/unit/data/vcardImport/AllAttr.vcf"); + $this->assertNull($vcard->PHOTO); + + $proxy = new DelayedPhotoLoader($vcard, $abook, $cache, $logger); + $this->assertEquals("", $proxy); + } + private function initStubs(): array { $logger = TestInfrastructure::$logger; @@ -225,8 +403,9 @@ private function initStubs(): array throw new \Exception("URI $uri not known to stub"); })); $db = $this->createMock(Database::class); + $cache = $this->createMock(\rcube_cache::class); - return [ $logger, $db, $abook ]; + return [ $logger, $db, $cache, $abook ]; } private function readVCard(string $vcfFile): VCard diff --git a/tests/unit/data/srv/pixel.jpg b/tests/unit/data/srv/pixel.jpg new file mode 100644 index 00000000..cc2be28c Binary files /dev/null and b/tests/unit/data/srv/pixel.jpg differ diff --git a/tests/unit/data/vcardImport/InvalidUriPhoto.json b/tests/unit/data/vcardImport/InvalidUriPhoto.json new file mode 100644 index 00000000..18e6d730 --- /dev/null +++ b/tests/unit/data/vcardImport/InvalidUriPhoto.json @@ -0,0 +1,11 @@ +{ + "name": "Max Mustermann", + "firstname": "Max", + "surname": "Mustermann", + "cuid": "B92B8FDB-E918-481F-AD6F-9713D5324AC5", + "kind": "individual", + "email:internet": [ + "special@maxmu.de" + ], + "photo": "" +} diff --git a/tests/unit/data/vcardImport/InvalidUriPhoto.vcf b/tests/unit/data/vcardImport/InvalidUriPhoto.vcf new file mode 100644 index 00000000..ca1497ec --- /dev/null +++ b/tests/unit/data/vcardImport/InvalidUriPhoto.vcf @@ -0,0 +1,10 @@ +BEGIN:VCARD +VERSION:3.0 +UID:B92B8FDB-E918-481F-AD6F-9713D5324AC5 +N:Mustermann;Max;;; +FN:Max Mustermann +EMAIL;TYPE=internet:special@maxmu.de +PRODID:-//Apple Inc.//iCloud Web Address Book 2018B71//EN +REV:2020-10-03T10:30:20Z +PHOTO;VALUE=uri:http://localhost/doesNotExist.jpg +END:VCARD