Skip to content

Commit

Permalink
More type annotations
Browse files Browse the repository at this point in the history
  • Loading branch information
@mstilkerich
mstilkerich committed Jan 8, 2021
1 parent 441e366 commit 7aba0c6
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 36 deletions.
17 changes: 13 additions & 4 deletions src/Addressbook.php
View file
Expand Up @@ -36,6 +36,7 @@

/**
* @psalm-import-type FullAbookRow from AbstractDatabase
* @psalm-import-type SaveData from DataConversion
*/
class Addressbook extends rcube_addressbook
{
Expand Down Expand Up @@ -138,6 +139,7 @@ public function get_name(): string
public function set_search_set($filter): void
{
if (is_array($filter) && (empty($filter) || $filter[0] instanceof DbAndCondition)) {
/** @var list<DbAndCondition> $filter */
$this->filter = $filter;
} else {
throw new \Exception(__METHOD__ . " requires a DbAndCondition[] type filter");
Expand Down Expand Up @@ -358,12 +360,13 @@ public function get_result(): ?rcube_result_set
* @param mixed $id Record identifier(s)
* @param bool $assoc True to return record as associative array, otherwise a result set is returned
*
* @return rcube_result_set|array Result object with all record fields
* @return rcube_result_set|SaveData Result object with all record fields
*/
// phpcs:ignore PSR1.Methods.CamelCapsMethodName -- method name defined by rcube_addressbook class
public function get_record($id, $assoc = false)
{
try {
$id = (string) $id;
$this->logger->debug("get_record($id, $assoc)");
$db = $this->db;

Expand All @@ -381,7 +384,13 @@ public function get_record($id, $assoc = false)
} catch (\Exception $e) {
$this->logger->error("Could not get contact $id: " . $e->getMessage());
$this->set_error(rcube_addressbook::ERROR_SEARCH, $e->getMessage());
return $assoc ? [] : new rcube_result_set();
if ($assoc) {
/** @var SaveData $ret Psalm does not consider the empty array a subtype */
$ret = [];
return $ret;
} else {
return new rcube_result_set();
}
}
}

Expand Down Expand Up @@ -1193,18 +1202,18 @@ private function listRecordsReadDB(?array $cols, int $subset, rcube_result_set $
* correct property of the VCard contained the value. Thus, if such search fields are queried, the DB result needs
* to be post-filtered with a check for these particular fields against the VCard properties.
*
* This function returns two values in an associative array with entries:
* This function returns an associative array with entries:
* "filter": An array of DbAndCondition for use with the AbstractDatabase::get() function.
* "postSearchMode": true if all conditions must match ("AND"), false if a single match is sufficient ("OR")
* "postSearchFilter": An array of two-element arrays, each with: [ column name, lower-cased search value ]
*
* @param string|string[] $fields Field names to search in
* @param string|string[] $value Search value, or array of values, one for each field in $fields
* @param int $mode Matching mode, see Addressbook::search() for extended description.
* @return array{filter: list<DbAndCondition>, postSearchMode: bool, postSearchFilter: list<array{string,string}>}
*/
private function buildDatabaseSearchFilter($fields, $value, int $mode): array
{
/** @var DbAndCondition[] */
$conditions = [];
$postSearchMode = false;
$postSearchFilter = [];
Expand Down
86 changes: 62 additions & 24 deletions src/DataConversion.php
View file
Expand Up @@ -11,10 +11,49 @@
use rcube_utils;

/**
* @psalm-type SaveDataSimpleField = string
* @psalm-type SaveDataMultiField = list<string>
* @psalm-type SaveDataAddressField = array<string,string>
* @psalm-type SaveData = array<string, SaveDataSimpleField|SaveDataMultiField|SaveDataAddressField>
* @psalm-type SaveData = array{
* name?: string,
* cuid?: string,
* kind?: string,
* ID?: string,
* birthday?: string,
* nickname?: string,
* notes?: string,
* photo?: string | DelayedPhotoLoader,
* jobtitle?: string,
* showas?: string,
* anniversary?: string,
* assistant?: string,
* gender?: string,
* manager?: string,
* spouse?: string,
* maidenname?: string,
* organization?: string,
* department?: string
* } & array<string, SaveDataMultiField|SaveDataAddressField>
*
* @psalm-type SaveDataFromDC = array{
* name: string,
* kind: string,
* cuid?: string,
* ID?: string,
* birthday?: string,
* nickname?: string,
* notes?: string,
* photo?: string | DelayedPhotoLoader,
* jobtitle?: string,
* showas?: string,
* anniversary?: string,
* assistant?: string,
* gender?: string,
* manager?: string,
* spouse?: string,
* maidenname?: string,
* organization?: string,
* department?: string
* } & array<string, SaveDataMultiField|SaveDataAddressField>
*/
class DataConversion
{
Expand Down Expand Up @@ -221,7 +260,7 @@ public function isMultivalueProperty(string $attrname): bool
*
* @param VCard $vcard Sabre VCard object
*
* @return array Roundcube representation of the VCard
* @return SaveDataFromDC Roundcube representation of the VCard
*/
public function toRoundcube(VCard $vcard, AddressbookCollection $davAbook): array
{
Expand Down Expand Up @@ -352,7 +391,7 @@ private function toRoundcubeIMPP(VObject\Property $prop): string
/**
* Creates a new or updates an existing vcard from save data.
*
* @param array<string,string|string[]> $save_data The roundcube representation of the contact / group
* @param SaveData $save_data The roundcube representation of the contact / group
* @param ?VCard $vcard The original VCard from that the address data was originally passed to roundcube. If a new
* VCard should be created, this parameter must be null.
* @return VCard Returns the created / updated VCard. If a VCard was passed in the $vcard parameter, it is updated
Expand Down Expand Up @@ -381,13 +420,13 @@ public function fromRoundcube(array $save_data, ?VCard $vcard = null): VCard
if ($isGroup) {
$vcard->N = [$save_data['name'],"","","",""];
} else {
$vcard->N = [
$save_data['surname'] ?? "",
$save_data['firstname'] ?? "",
$save_data['middlename'] ?? "",
$save_data['prefix'] ?? "",
$save_data['suffix'] ?? ""
];
$nAttr = array_fill(0, 5, "");
foreach (['surname', 'firstname', 'middlename', 'prefix', 'suffix'] as $idx => $nameKey) {
if (isset($save_data[$nameKey])) {
$nAttr[$idx] = $save_data[$nameKey];
}
}
$vcard->N = $nAttr;
}

$this->setOrgProperty($save_data, $vcard);
Expand Down Expand Up @@ -420,7 +459,7 @@ private function dateTimeString(): string
*
* If neither organization nor department are given (or empty), the ORG property is deleted from the VCard.
*
* @param array<string,string|string[]> $save_data The roundcube representation of the contact
* @param SaveData $save_data The roundcube representation of the contact
* @param VCard $vcard The VCard to set the ORG property for.
*/
private function setOrgProperty(array $save_data, VCard $vcard): void
Expand All @@ -430,7 +469,7 @@ private function setOrgProperty(array $save_data, VCard $vcard): void
$orgParts[] = $save_data['organization'];
}

if (!empty($save_data['department']) && is_string($save_data['department'])) {
if (!empty($save_data['department'])) {
// the first element of ORG corresponds to organization, if that field is not filled but organization is
// we need to store an empty value explicitly (otherwise, department would become organization when reading
// back the VCard).
Expand All @@ -456,7 +495,7 @@ private function setOrgProperty(array $save_data, VCard $vcard): void
* - Special case photo: It is only set if it was edited. If it is deleted, it is set to an empty string. If it
* was not changed, no photo key is present in save_data.
*
* @param array $save_data The roundcube representation of the contact
* @param SaveData $save_data The roundcube representation of the contact
* @param VCard $vcard The VCard to set the ORG property for.
*/
private function setSingleValueProperties(array $save_data, VCard $vcard): void
Expand Down Expand Up @@ -492,7 +531,7 @@ private function setSingleValueProperties(array $save_data, VCard $vcard): void
* a colon (e.g. "email:home"). The value of each setting is an array. These arrays may include empty members if
* the field was part of the edit mask but not filled.
*
* @param array<string,string|string[]> $save_data The roundcube representation of the contact
* @param SaveData $save_data The roundcube representation of the contact
* @param VCard $vcard The VCard to set the ORG property for.
*/
private function setMultiValueProperties(array $save_data, VCard $vcard): void
Expand Down Expand Up @@ -520,7 +559,9 @@ private function setMultiValueProperties(array $save_data, VCard $vcard): void
}

foreach ($subtypes as $subtype) {
foreach ((array) $save_data["$rckey:$subtype"] as $value) {
/** @var SaveDataMultiField $values */
$values = (array) $save_data["$rckey:$subtype"];
foreach ($values as $value) {
$prop = null;

$mkey = str_replace("-", "_", strtoupper($vkey));
Expand Down Expand Up @@ -928,15 +969,14 @@ private function addextrasubtypes(): void
* If an existing ShowAs=COMPANY setting is given, but the organization field is empty, the setting will be reset to
* INDIVIDUAL.
*
* @param array<string,string|string[]> $save_data The address data as roundcube's internal format, as entered by
* @param SaveData $save_data The address data as roundcube's internal format, as entered by
* the user. For update of an existing contact, the showas key must
* be populated with the previous value.
* @return string INDIVIDUAL or COMPANY
*/
private function determineShowAs(array $save_data): string
{
$showAs = $save_data['showas'] ?? "";
assert(is_string($showAs), "showAs must be string");

if (empty($showAs)) { // new contact
if (empty($save_data['surname']) && empty($save_data['firstname']) && !empty($save_data['organization'])) {
Expand Down Expand Up @@ -967,24 +1007,21 @@ private function determineShowAs(array $save_data): string
* From a VCard, the FN is mandatory. However, we may be served non-compliant VCards, or VCards with an empty FN
* value. In those cases, we will set the display name, otherwise we will take the value provided in the VCard.
*
* @param array<string,string|string[]> $save_data The address data as roundcube's internal format.
* @param SaveData $save_data The address data as roundcube's internal format.
* @return string The composed displayname
*/
private static function composeDisplayname(array $save_data): string
{
$showAs = $save_data['showas'] ?? "";
assert(is_string($showAs), "showAs must be string");

if (strcasecmp($showAs, 'COMPANY') == 0 && !empty($save_data['organization'])) {
assert(is_string($save_data['organization']), "organization is a string attribute");
return $save_data['organization'];
}

// try from name
$dname = [];
foreach (["firstname", "surname"] as $attr) {
if (!empty($save_data[$attr])) {
assert(is_string($save_data[$attr]), "$attr is a string attribute");
$dname[] = $save_data[$attr];
}
}
Expand All @@ -997,8 +1034,9 @@ private static function composeDisplayname(array $save_data): string
$epKeys = preg_grep(";^(email|phone):;", array_keys($save_data));
sort($epKeys, SORT_STRING);
foreach ($epKeys as $epKey) {
assert(is_array($save_data[$epKey]), "$attr is a multi-value attribute");
foreach ($save_data[$epKey] as $epVal) {
/** @var SaveDataMultiField */
$epVals = $save_data[$epKey];
foreach ($epVals as $epVal) {
if (!empty($epVal)) {
return $epVal;
}
Expand Down
10 changes: 4 additions & 6 deletions src/Db/AbstractDatabase.php
View file
Expand Up @@ -23,7 +23,7 @@
* presetname: ?string
* }
*
* @psalm-import-type SaveData from \MStilkerich\CardDavAddressbook4Roundcube\DataConversion
* @psalm-import-type SaveDataFromDC from \MStilkerich\CardDavAddressbook4Roundcube\DataConversion
*/
abstract class AbstractDatabase
{
Expand Down Expand Up @@ -152,7 +152,7 @@ abstract public function delete($conditions, string $table = 'contacts'): int;
* @param string $etag of the VCard in the given version on the CardDAV server
* @param string $uri path to the VCard on the CardDAV server
* @param string $vcfstr string representation of the VCard
* @param SaveData $save_data associative array containing the roundcube save data for the contact
* @param SaveDataFromDC $save_data associative array containing the roundcube save data for the contact
* @param ?string $dbid optionally, database id of the contact if the store operation is an update
*
* @return string The database id of the created or updated card.
Expand Down Expand Up @@ -197,7 +197,7 @@ public function storeContact(
* to indicate this.
*
* @param string $abookid Database ID of the addressbook the group shall be inserted to
* @param SaveData $save_data associative array containing at least name and cuid (card UID)
* @param SaveDataFromDC $save_data associative array containing at least name and cuid (card UID)
* @param ?string $dbid optionally, database id of the group if the store operation is an update
* @param ?string $etag of the VCard in the given version on the CardDAV server
* @param ?string $uri path to the VCard on the CardDAV server
Expand Down Expand Up @@ -227,7 +227,7 @@ public function storeGroup(
* @param ?string $etag The ETag value of the CardDAV-server address object that this object is created from.
* @param ?string $uri The URI of the CardDAV-server address object that this object is created from.
* @param ?string $vcfstr The VCard string of the CardDAV-server address object that this object is created from.
* @param SaveData $save_data The Roundcube representation of the address object.
* @param SaveDataFromDC $save_data The Roundcube representation of the address object.
* @param ?string $dbid If an existing object is updated, this specifies its database id.
* @param list<string> $xcol Database column names of attributes to insert.
* @param DbInsRow $xval The values to insert into the column specified by $xcol at the corresponding index.
Expand All @@ -245,7 +245,6 @@ protected function storeAddressObject(
array $xval = []
): string {
$xcol[] = 'name';
/** @var string $name */
$name = $save_data['name'];
$xval[] = $name;

Expand All @@ -271,7 +270,6 @@ protected function storeAddressObject(
}
if (isset($save_data['cuid'])) {
$xcol[] = 'cuid';
/** @var string $cuid */
$cuid = $save_data["cuid"];
$xval[] = $cuid;
}
Expand Down
8 changes: 6 additions & 2 deletions src/SyncHandlerRoundcube.php
View file
Expand Up @@ -275,7 +275,7 @@ private function getCategoryTypeGroupsForUser(VCard $card): array
foreach ($categories as $category) {
if (!isset($group_ids_by_name[$category])) {
$gsave_data = [
'name' => $category,
'name' => (string) $category,
'kind' => 'group'
];
$dbid = $db->storeGroup($abookId, $gsave_data);
Expand Down Expand Up @@ -307,7 +307,7 @@ private function updateContactCard(string $uri, string $etag, VCard $card): void
$db = $this->db;

$save_data = $this->dataConverter->toRoundcube($card, $this->davAbook);
$this->logger->info("Changed Individual $uri " . $save_data['name']);
$this->logger->info("Changed Individual $uri");

$dbid = $this->localCards[$uri]["id"] ?? null;
if (isset($dbid)) {
Expand All @@ -320,6 +320,10 @@ private function updateContactCard(string $uri, string $etag, VCard $card): void
$db->endTransaction();

// remember in the local cache - might be needed in finalizeSync to map UID to DB ID without DB query
if (!isset($save_data["cuid"])) {
throw new \Exception("VCard $uri has not UID property");
}

if (!isset($this->localCardsByUID[$save_data["cuid"]])) {
$this->localCardsByUID[$save_data["cuid"]] = $dbid;
}
Expand Down
3 changes: 3 additions & 0 deletions tests/unit/DataConversionTest.php
View file
Expand Up @@ -293,6 +293,7 @@ public function testNewPhotoIsStoredToCacheIfNeeded(string $basename, bool $getE

$dc = new DataConversion("42", $db, $cache, $logger);
$saveData = $dc->toRoundcube($vcard, $abook);
$this->assertTrue(isset($saveData['photo']));
$this->comparePhoto($saveDataExpected["photo"], (string) $saveData["photo"]);

$this->assertPhotoDownloadWarning($logger, $basename);
Expand Down Expand Up @@ -339,6 +340,7 @@ public function testPhotoIsUsedFromCacheIfAvailable(string $basename, bool $getE
$dc = new DataConversion("42", $db, $cache, $logger);
$saveData = $dc->toRoundcube($vcard, $abook);

$this->assertTrue(isset($saveData['photo']));
if ($getExp) {
$this->comparePhoto($cachedPhotoData, (string) $saveData["photo"]);
} else {
Expand Down Expand Up @@ -408,6 +410,7 @@ public function testOutdatedPhotoIsReplacedInCache(string $basename, bool $getEx

$dc = new DataConversion("42", $db, $cache, $logger);
$saveData = $dc->toRoundcube($vcard, $abook);
$this->assertTrue(isset($saveData['photo']));
$this->comparePhoto($saveDataExpected["photo"], (string) $saveData["photo"]);

$this->assertPhotoDownloadWarning($logger, $basename);
Expand Down

0 comments on commit 7aba0c6

Please sign in to comment.