From aea7a5c0f263b90d39888950ccf09884c96bbc2d Mon Sep 17 00:00:00 2001 From: Ralf Becker Date: Sun, 16 May 2021 20:42:47 +0200 Subject: [PATCH] * ADS/LDAP: improve caching of searching/listing --- api/src/Accounts.php | 43 +++++++++++++++++++++++++++++++++++---- api/src/Accounts/Ads.php | 6 +++--- api/src/Accounts/Ldap.php | 4 ++-- 3 files changed, 44 insertions(+), 9 deletions(-) diff --git a/api/src/Accounts.php b/api/src/Accounts.php index 4c95ea67ff..3753124cbe 100644 --- a/api/src/Accounts.php +++ b/api/src/Accounts.php @@ -168,6 +168,25 @@ class Accounts $this->backend = new $backend_class($this); } + /** + * Get cache-key for search parameters + * + * @param array $params + * @param ?string& $unlimited on return key for unlimited search + * @return string + */ + public static function cacheKey(array $params, string &$unlimited=null) + { + // normalize our cache-key by not storing anything, plus adding default the default sort (if none requested) + $keys = array_filter($params)+['order' => 'account_lid', 'sort' => 'ASC']; + // sort keys + ksort($keys); + $key = json_encode($keys); + unset($keys['start'], $keys['offset']); + $unlimited = json_encode($keys); + return $key; + } + /** * Searches / lists accounts: users and/or groups * @@ -201,6 +220,7 @@ class Accounts { //error_log(__METHOD__.'('.array2string($param).') '.function_backtrace()); if (!isset($param['active'])) $param['active'] = true; // default is true = only return active accounts + if (!empty($param['offset']) && !isset($param['start'])) $param['start'] = 0; // Check for lang(Group) in search - if there, we search all groups $group_index = array_search(strtolower(lang('Group')), array_map('strtolower', $query = explode(' ',$param['query']))); @@ -224,12 +244,18 @@ class Accounts } self::setup_cache(); $account_search = &self::$cache['account_search']; - $serial = serialize($param); + $serial = self::cacheKey($param, $serial_unlimited); if (isset($account_search[$serial])) { $this->total = $account_search[$serial]['total']; } + // if we already have an unlimited search, we can always return only a part of it + elseif (isset($account_search[$serial_unlimited])) + { + $this->total = $account_search[$serial]['total']; + return array_slice($account_search[$serial]['total']['data'], $param['start'], $param['offset']); + } // no backend understands $param['app'], only sql understands type owngroups or groupmemember[+memberships] // --> do an full search first and then filter and limit that search elseif($param['app'] || $this->config['account_repository'] != 'sql' && @@ -297,8 +323,18 @@ class Accounts // direct search via backend else { - $account_search[$serial]['data'] = $this->backend->search($param); - if ($param['type'] !== 'accounts') + $account_search[$serial] = [ + 'data' => $this->backend->search($param), + 'total' => $this->total = $this->backend->total, + ]; + // check if all rows have been returned --> cache as unlimited query + if ($serial !== $serial_unlimited && count($account_search[$serial]['data']) === (int)$this->backend->total) + { + $account_search[$serial_unlimited] = $account_search[$serial]; + unset($account_search[$serial]); + $serial = $serial_unlimited; + } + if ($param['type'] !== 'accounts' && !is_numeric($param['type'])) { foreach($account_search[$serial]['data'] as &$account) { @@ -309,7 +345,6 @@ class Accounts } } } - $account_search[$serial]['total'] = $this->total = $this->backend->total; } return $account_search[$serial]['data']; } diff --git a/api/src/Accounts/Ads.php b/api/src/Accounts/Ads.php index a3a0623000..fa01bafaa4 100644 --- a/api/src/Accounts/Ads.php +++ b/api/src/Accounts/Ads.php @@ -907,7 +907,7 @@ class Ads $account_search = []; // disabled, we have sorted&limited queries now &$this->cache['account_search']; // check if the query is cached - $serial = serialize($param); + $serial = Api\Accounts::cacheKey($param, $unl_serial); if (isset($account_search[$serial])) { $this->total = $account_search[$serial]['total']; @@ -919,7 +919,7 @@ class Ads if (!($offset = $param['offset'])) $offset = $maxmatchs; unset($param['start']); unset($param['offset']); - $unl_serial = serialize($param); + if (isset($account_search[$unl_serial])) { $this->total = $account_search[$unl_serial]['total']; @@ -1261,7 +1261,7 @@ class Ads } else if (self::$debug) error_log(__METHOD__.'('.array2string($attr_filter).", '$account_type') ldap_search($ds, '$context', '$filter')=$sri allValues=".array2string($allValues)); - //error_log(__METHOD__.'('.array2string($attr_filter).", '$account_type') ldap_search($ds, '$context', '$filter') returning ".array2string($accounts).' '.function_backtrace()); + //error_log(date('Y-m-d H:i:s ').__METHOD__.'('.array2string($attr_filter).", '$account_type', ".json_encode($attrs).", ..., expired=$filter_expired, order_by=$order_by, start=$start, num_rows=$num_rows) ldap_search($ds, '$context', '$filter')\n==> returning ".count($accounts)."/$total ".substr(array2string($accounts), 0, 1024)."\n--> ".function_backtrace()."\n\n", 3, '/var/lib/egroupware/ads.log'); return $accounts; } diff --git a/api/src/Accounts/Ldap.php b/api/src/Accounts/Ldap.php index f18f898449..06536fcee5 100644 --- a/api/src/Accounts/Ldap.php +++ b/api/src/Accounts/Ldap.php @@ -679,7 +679,7 @@ class Ldap $account_search =& Api\Accounts::$cache['account_search']; // check if the query is cached - $serial = serialize($param); + $serial = Api\Accounts::cacheKey($param, $unl_serial); if (isset($account_search[$serial])) { $this->total = $account_search[$serial]['total']; @@ -691,7 +691,7 @@ class Ldap if (!($offset = $param['offset'])) $offset = $maxmatchs; unset($param['start']); unset($param['offset']); - $unl_serial = serialize($param); + if (isset($account_search[$unl_serial])) { $this->total = $account_search[$unl_serial]['total'];