Skip to content

Commit

Permalink
Merge branch 'issue_247_delayed_photoload' into v4.1 (Fixes #247)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mstilkerich
mstilkerich committed Nov 22, 2020
2 parents dcb7be1 + 31445b4 commit c4dac9c
Show file tree
Hide file tree
Showing 12 changed files with 545 additions and 137 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
41 changes: 39 additions & 2 deletions carddav.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
{
Expand Down
17 changes: 10 additions & 7 deletions src/Addressbook.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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,
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
114 changes: 10 additions & 104 deletions src/DataConversion.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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' => [
Expand Down Expand Up @@ -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.
*
Expand All @@ -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();
Expand All @@ -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',
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}


Expand Down Expand Up @@ -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 ************
Expand Down Expand Up @@ -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
2 changes: 0 additions & 2 deletions src/Database.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit c4dac9c

Please sign in to comment.