Skip to content

Commit

Permalink
Make preset re-discovery optional by configuration
Browse files Browse the repository at this point in the history
  • Loading branch information
mstilkerich committed Nov 20, 2021
1 parent b2c005c commit 5278e5c
Show file tree
Hide file tree
Showing 4 changed files with 128 additions and 77 deletions.
14 changes: 10 additions & 4 deletions CHANGELOG.md
Expand Up @@ -2,10 +2,16 @@

## Version 4.3.0 (to 4.2.2)

- New: For preset addressbooks, are re-discovery is performed upon every login. This means that newly added addressbooks
on the server are discovered and added, whereas addressbooks that have been removed from the server are also removed
from roundcube. For manually-added addressbooks, this will require changes to the rcmcarddav data model, which is
planned for version 5.
- New: For preset addressbooks, are re-discovery is performed upon every login by default. This means that newly added
addressbooks on the server are discovered and added, whereas addressbooks that have been removed from the server are
also removed from roundcube.
- __NOTE TO ADMINS__: If you are using addressbook presets, please read the documentation on the new preset setting
`rediscover_mode` to decide if re-discovery is desired or not. The new default is functionally safe, but performance
can be improved if the new behavior is not needed.
- For manually-added addressbooks, this will require changes to the rcmcarddav data model, which is planned for
version 5.
- Version 5 will also have a more elaborate version of re-discovery that will allow to configure it such that it does
not happen on every login.
- MySQL: Convert potentially used row format COMPACT (was default up to MySQL 5.7.8, Maria DB 10.2.1) to DYANMIC in
migration 12, which would otherwise fail (Fixes #362). It requires some other settings that have to be configured in
the MySQL server configuration additionally, all of which are also defaults since MySQL 5.7.7 / Maria DB 10.2.2.
Expand Down
137 changes: 88 additions & 49 deletions carddav.php
Expand Up @@ -45,7 +45,8 @@
* fixed: list<ConfigurablePresetAttribute>,
* require_always: list<string>,
* hide: bool,
* carddav_name_only: bool
* carddav_name_only: bool,
* rediscover_mode: 'none' | 'fulldiscovery',
* }
* @psalm-type AbookSettings = array{
* name?: string,
Expand Down Expand Up @@ -110,6 +111,7 @@ class carddav extends rcube_plugin
'hide' => false,
'fixed' => [],
'require_always' => [],
'rediscover_mode' => 'fulldiscovery',
];

/** @var PasswordStoreScheme encryption scheme */
Expand Down Expand Up @@ -251,67 +253,86 @@ public function initPresets(): void
try {
$logger->debug(__METHOD__);

// Get all existing addressbooks of this user that have been created from presets
$existing_abooks = $this->getAddressbooks(false, true);

// Group the addressbooks by their preset
$existing_presets = [];
foreach ($existing_abooks as $abookrow) {
/** @var string $presetname Not null because filtered by getAddressbooks() */
$localAbookRowsByPreset = [];
foreach ($this->getAddressbooks(false, true) as $abookrow) {
/** @psalm-var string $presetname Not null because filtered by getAddressbooks() */
$presetname = $abookrow['presetname'];
if (!key_exists($presetname, $existing_presets)) {
$existing_presets[$presetname] = [];
}
$existing_presets[$presetname][$abookrow['url']] = $abookrow;
$localAbookRowsByPreset[$presetname][$abookrow['url']] = $abookrow;
}

// Walk over the current presets configured by the admin and add, update or delete addressbooks
// Walk over the current presets configured by the admin and add or update addressbooks
foreach ($this->presets as $presetname => $preset) {
$preset['presetname'] = $presetname;
$abname = $preset['name'];

try {
// first discover all the current addressbooks on the server for this preset
$username = self::replacePlaceholdersUsername($preset['username']);
$url = self::replacePlaceholdersUrl($preset['url']);
$password = self::replacePlaceholdersPassword($preset['password']);
try {
$account = Config::makeAccount($url, $username, $password, null);
} catch (\Exception $e) {
$logger->info("Skip adding preset for $rcUserId: required bearer token not available");
continue;
// determine the rediscovery mode
if (isset($localAbookRowsByPreset[$presetname])) {
$rediscoverMode = $preset['rediscover_mode'];

// record all known addressbooks for this preset for update of fixed settings in case we do not
// perform discovery. If we do discover, this will be overwritten.
$updateAbookRows = $localAbookRowsByPreset[$presetname];
} else {
// if we don't have any addressbooks for the preset, force performing a full discovery
$rediscoverMode = 'fulldiscovery';
$updateAbookRows = [];
}
$abooks = $this->determineAddressbooksToAdd($account);

// insert all newly discovered addressbooks, update those already present in the DB
foreach ($abooks as $abook) {
$url = $abook->getUri();
$existing_preset = $existing_presets[$presetname][$url] ?? null;
if (isset($existing_preset)) {
$logger->debug("Updating preset ($presetname) addressbook $url for $rcUserId");
$this->updatePresetAddressbook($preset, $existing_preset);
if ($rediscoverMode == 'fulldiscovery') {
// discover the addressbooks at the server
$username = self::replacePlaceholdersUsername($preset['username']);
$url = self::replacePlaceholdersUrl($preset['url']);
$password = self::replacePlaceholdersPassword($preset['password']);
try {
$account = Config::makeAccount($url, $username, $password, null);
} catch (\Exception $e) {
$logger->info("Skip adding preset for $rcUserId: required bearer token not available");
continue;
}
$abooks = $this->determineAddressbooksToAdd($account);

// delete from existing_presets list so we know it was processed
unset($existing_presets[$presetname][$url]);
} else {
$logger->info("Inserting preset ($presetname) addressbook $url for $rcUserId");
if ($preset['carddav_name_only']) {
$preset['name'] = $abook->getName();
} else {
$preset['name'] = "$abname (" . $abook->getName() . ')';
// insert all newly discovered addressbooks, record those already present in the DB for update
$updateAbookRows = [];
foreach ($abooks as $abook) {
$url = $abook->getUri();

// determine name for the addressbook
$abookName = $abook->getName();
if (!$preset['carddav_name_only']) {
$abookName = $preset['name'] . " ($abookName)";
}

$preset['url'] = $url;
$this->insertAddressbook($preset);
$abookrow = $localAbookRowsByPreset[$presetname][$url] ?? null;
if (isset($abookrow)) {
$abookrow['srvname'] = $abookName;
$updateAbookRows[] = $abookrow;
} else {
$logger->info("Inserting new addressbook for preset $presetname at $url for $rcUserId");
$abookrow = $preset;
$abookrow['url'] = $url;
$abookrow['name'] = $abookName;
$this->insertAddressbook($abookrow);
}
}
}

// Update the fixed prefs in all addressbooks that we already knew locally
foreach ($updateAbookRows as $abookrow) {
$url = $abookrow['url'];
$logger->debug("Updating preset ($presetname) addressbook $url for $rcUserId");
$this->updatePresetAddressbook($preset, $abookrow);

// delete from localAbookRowsByPreset list so we know it was processed
unset($localAbookRowsByPreset[$presetname][$url]);
}
} catch (\Exception $e) {
$logger->error("Error adding addressbook from preset $presetname: {$e->getMessage()}");
}
}

// delete existing preset addressbooks that were removed by admin or do not exist on server anymore
foreach ($existing_presets as $presetname => $ep) {
foreach ($localAbookRowsByPreset as $presetname => $ep) {
foreach ($ep as $abookrow) {
$url = $abookrow['url'];
$logger->info("Deleting preset ($presetname) addressbook $url for $rcUserId");
Expand Down Expand Up @@ -863,15 +884,27 @@ private function updatePresetAddressbook(array $preset, array $abookrow): void
if (isset($abookrow[$k]) && isset($preset[$k])) {
// only update the name if it is used
if ($k === 'name') {
if (!$preset['carddav_name_only']) {
$fullname = $abookrow['name'];
$cnpos = strpos($fullname, ' (');
if ($cnpos === false && $preset['name'] != $fullname) {
$pa['name'] = $preset['name'];
} elseif ($cnpos !== false && $preset['name'] != substr($fullname, 0, $cnpos)) {
$pa['name'] = $preset['name'] . substr($fullname, $cnpos);
$newname = $abookrow['name'];

// if we performed a rediscovery, we have the desired name including the current server-side name in
// the srvname field of $abookrow - use it
if (isset($abookrow['srvname'])) {
$newname = (string) $abookrow['srvname'];

// otherwise we can only update the admin-configured name of the addressbook
// AdminName (ServersideName)
} elseif (!$preset['carddav_name_only']) {
$cnpos = strpos($newname, ' (');
if ($cnpos === false && $preset['name'] != $newname) {
$newname = $preset['name'];
} elseif ($cnpos !== false && $preset['name'] != substr($newname, 0, $cnpos)) {
$newname = $preset['name'] . substr($newname, $cnpos);
}
}

if ($abookrow['name'] != $newname) {
$pa['name'] = $newname;
}
} elseif ($k === 'url') {
// the URL cannot be automatically updated, as it was discovered and normally will
// not exactly match the discovery URI. Resetting it to the discovery URI would
Expand Down Expand Up @@ -1312,7 +1345,13 @@ private function addPreset(string $presetname, array $preset): void
}
} else {
if (isset($preset[$attr]) && is_string($preset[$attr])) {
$result[$attr] = $preset[$attr];
$value = $preset[$attr];

if (($attr == 'rediscover_mode') && (!in_array($value, [ 'none', 'fulldiscovery' ]))) {
$logger->error("Preset $presetname: invalid value for setting $attr");
} else {
$result[$attr] = $value;
}
}
}
}
Expand Down
1 change: 1 addition & 0 deletions config.inc.php.dist
Expand Up @@ -46,6 +46,7 @@ $prefs['<Presetname>'] = [
'active' => <true or false>,
'readonly' => <true or false>,
'refresh_time' => '<Refresh Time in Hours, Format HH[:MM[:SS]]>',
'rediscover_mode' => '<One of: none|fulldiscovery',

// attributes that are fixed (i.e., not editable by the user) and auto-updated for this preset
'fixed' => [ < 0 or more of the other attribute keys > ],
Expand Down

0 comments on commit 5278e5c

Please sign in to comment.