Skip to content

Commit

Permalink
feat(user-prefs): iterator instead of array on search
Browse files Browse the repository at this point in the history
Signed-off-by: Maxence Lange <maxence@artificial-owl.com>
  • Loading branch information
ArtificialOwl committed Oct 17, 2024
1 parent e04730a commit dd9c55b
Show file tree
Hide file tree
Showing 10 changed files with 73 additions and 89 deletions.
11 changes: 6 additions & 5 deletions lib/private/AllConfig.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@
*/
namespace OC;

use OC\Config\UserPreferences;
use OCP\Cache\CappedMemoryCache;
use OCP\Config\Exceptions\TypeConflictException;
use OCP\Config\IUserPreferences;
use OCP\Config\ValueType;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\PreConditionNotMetException;
use OCP\UserPreferences\Exceptions\TypeConflictException;
use OCP\UserPreferences\IUserPreferences;
use OCP\UserPreferences\ValueType;

/**
* Class to combine all the configuration options ownCloud offers
Expand Down Expand Up @@ -380,7 +381,7 @@ public function getUserValueForUsers($appName, $key, $userIds) {
* @deprecated 31.0.0 - use {@see IUserPreferences::searchUsersByValueString} directly
*/
public function getUsersForUserValue($appName, $key, $value) {
return \OCP\Server::get(IUserPreferences::class)->searchUsersByValueDeprecated($appName, $key, $value);
return iterator_to_array(\OCP\Server::get(IUserPreferences::class)->searchUsersByValueString($appName, $key, $value));

Check failure on line 384 in lib/private/AllConfig.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

LessSpecificReturnStatement

lib/private/AllConfig.php:384:10: LessSpecificReturnStatement: The type 'array<array-key, string>' is more general than the declared return type 'list<string>' for OC\AllConfig::getUsersForUserValue (see https://psalm.dev/129)
}

/**
Expand All @@ -397,7 +398,7 @@ public function getUsersForUserValueCaseInsensitive($appName, $key, $value) {
return $this->getUsersForUserValue($appName, $key, strtolower($value));
}

return \OCP\Server::get(IUserPreferences::class)->searchUsersByValueDeprecated($appName, $key, $value, true);
return iterator_to_array(\OCP\Server::get(IUserPreferences::class)->searchUsersByValueString($appName, $key, $value, true));

Check failure on line 401 in lib/private/AllConfig.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

LessSpecificReturnStatement

lib/private/AllConfig.php:401:10: LessSpecificReturnStatement: The type 'array<array-key, string>' is more general than the declared return type 'list<string>' for OC\AllConfig::getUsersForUserValueCaseInsensitive (see https://psalm.dev/129)
}

public function getSystemConfig() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,22 @@
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OC;
namespace OC\Config;

use Generator;
use InvalidArgumentException;
use JsonException;
use OCP\Config\Exceptions\IncorrectTypeException;
use OCP\Config\Exceptions\TypeConflictException;
use OCP\Config\Exceptions\UnknownKeyException;
use OCP\Config\IUserPreferences;
use OCP\Config\ValueType;
use OCP\DB\Exception as DBException;
use OCP\DB\IResult;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\Security\ICrypto;
use OCP\UserPreferences\Exceptions\IncorrectTypeException;
use OCP\UserPreferences\Exceptions\TypeConflictException;
use OCP\UserPreferences\Exceptions\UnknownKeyException;
use OCP\UserPreferences\IUserPreferences;
use OCP\UserPreferences\ValueType;
use Psr\Log\LoggerInterface;

/**
Expand Down Expand Up @@ -393,40 +394,24 @@ public function getValuesByUsers(
* @param string $value preference value
* @param bool $caseInsensitive non-case-sensitive search, only works if $value is a string
*
* @return list<string>
* @return Generator<string>
* @since 31.0.0
*/
public function searchUsersByValueString(string $app, string $key, string $value, bool $caseInsensitive = false): array {
public function searchUsersByValueString(string $app, string $key, string $value, bool $caseInsensitive = false): Generator {
return $this->searchUsersByTypedValue($app, $key, $value, $caseInsensitive);
}

/**
* @inheritDoc
*
* @param string $app id of the app
* @param string $key preference key
* @param string $value preference value
* @param bool $caseInsensitive non-case-sensitive search, only works if $value is a string
* @internal
* @deprecated since 31.0.0 - {@see }
* @return list<string>
* @since 31.0.0
*/
public function searchUsersByValueDeprecated(string $app, string $key, string $value, bool $caseInsensitive = false): array {
return $this->searchUsersByTypedValue($app, $key, $value, $caseInsensitive, true);
}

/**
* @inheritDoc
*
* @param string $app id of the app
* @param string $key preference key
* @param int $value preference value
*
* @return list<string>
* @return Generator<string>
* @since 31.0.0
*/
public function searchUsersByValueInt(string $app, string $key, int $value): array {
public function searchUsersByValueInt(string $app, string $key, int $value): Generator {
return $this->searchUsersByValueString($app, $key, (string)$value);
}

Expand All @@ -437,10 +422,10 @@ public function searchUsersByValueInt(string $app, string $key, int $value): arr
* @param string $key preference key
* @param array $values list of preference values
*
* @return list<string>
* @return Generator<string>
* @since 31.0.0
*/
public function searchUsersByValues(string $app, string $key, array $values): array {
public function searchUsersByValues(string $app, string $key, array $values): Generator {
return $this->searchUsersByTypedValue($app, $key, $values);
}

Expand All @@ -451,10 +436,10 @@ public function searchUsersByValues(string $app, string $key, array $values): ar
* @param string $key preference key
* @param bool $value preference value
*
* @return list<string>
* @return Generator<string>
* @since 31.0.0
*/
public function searchUsersByValueBool(string $app, string $key, bool $value): array {
public function searchUsersByValueBool(string $app, string $key, bool $value): Generator {
$values = ['0', 'off', 'false', 'no'];
if ($value) {
$values = ['1', 'on', 'true', 'yes'];
Expand All @@ -470,11 +455,10 @@ public function searchUsersByValueBool(string $app, string $key, bool $value): a
* @param string $key
* @param string|array $value
* @param bool $caseInsensitive
* @param bool $withinNotIndexedField DEPRECATED: should only be used to stay compatible with not-indexed/pre-31 preferences value
*
* @return list<string>
* @return Generator<string>
*/
private function searchUsersByTypedValue(string $app, string $key, string|array $value, bool $caseInsensitive = false): array {
private function searchUsersByTypedValue(string $app, string $key, string|array $value, bool $caseInsensitive = false): Generator {
$this->assertParams('', $app, $key, allowEmptyUser: true);

$qb = $this->connection->getQueryBuilder();
Expand Down Expand Up @@ -515,15 +499,10 @@ private function searchUsersByTypedValue(string $app, string $key, string|array
}

$qb->andWhere($where);

$userIds = [];
$result = $qb->executeQuery();
$rows = $result->fetchAll();
foreach ($rows as $row) {
$userIds[] = $row['userid'];
while ($row = $result->fetch()) {
yield $row['userid'];
}

return $userIds;
}

/**
Expand Down
4 changes: 2 additions & 2 deletions lib/private/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@
use OCP\Collaboration\Reference\IReferenceManager;
use OCP\Command\IBus;
use OCP\Comments\ICommentsManager;
use OCP\Config\IUserPreferences;
use OCP\Contacts\ContactsMenu\IActionFactory;
use OCP\Contacts\ContactsMenu\IContactsStore;
use OCP\Defaults;
Expand Down Expand Up @@ -237,7 +238,6 @@
use OCP\User\Events\UserLoggedInWithCookieEvent;
use OCP\User\Events\UserLoggedOutEvent;
use OCP\User\IAvailabilityCoordinator;
use OCP\UserPreferences\IUserPreferences;
use Psr\Container\ContainerExceptionInterface;
use Psr\Container\ContainerInterface;
use Psr\Log\LoggerInterface;
Expand Down Expand Up @@ -567,7 +567,7 @@ public function __construct($webRoot, \OC\Config $config) {
});

$this->registerAlias(IAppConfig::class, \OC\AppConfig::class);
$this->registerAlias(IUserPreferences::class, \OC\UserPreferences::class);
$this->registerAlias(IUserPreferences::class, \OC\Config\UserPreferences::class);

$this->registerService(IFactory::class, function (Server $c) {
return new \OC\L10N\Factory(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCP\UserPreferences\Exceptions;
namespace OCP\Config\Exceptions;

use Exception;

/**
* @since 31.0.0
*/
class UserPreferencesException extends Exception {
class IncorrectTypeException extends Exception {
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCP\UserPreferences\Exceptions;
namespace OCP\Config\Exceptions;

use Exception;

/**
* @since 31.0.0
*/
class TypeConflictException extends UserPreferencesException {
class TypeConflictException extends Exception {
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCP\UserPreferences\Exceptions;
namespace OCP\Config\Exceptions;

use Exception;

/**
* @since 31.0.0
*/
class UnknownKeyException extends UserPreferencesException {
class UnknownKeyException extends Exception {
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,30 @@
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCP\UserPreferences;
namespace OCP\Config;

use OCP\UserPreferences\Exceptions\IncorrectTypeException;
use OCP\UserPreferences\Exceptions\UnknownKeyException;
use Generator;
use OCP\Config\Exceptions\IncorrectTypeException;
use OCP\Config\Exceptions\UnknownKeyException;

/**
* This class provides an easy way for apps to store user preferences in the
* database.
* Supports **lazy loading**
*
* ### What is lazy loading ?
* In order to avoid loading useless user preferences into memory for each request,
* only non-lazy values are now loaded.
*
* Once a value that is lazy is requested, all lazy values will be loaded.
*
* Similarly, some methods from this class are marked with a warning about ignoring
* lazy loading. Use them wisely and only on parts of the code that are called
* during specific requests or actions to avoid loading the lazy values all the time.
*
* @since 31.0.0
*/

interface IUserPreferences {
public const FLAG_SENSITIVE = 1; // value is sensitive

Check failure on line 34 in lib/public/Config/IUserPreferences.php

View workflow job for this annotation

GitHub Actions / static-code-analysis-ocp

InvalidDocblock

lib/public/Config/IUserPreferences.php:34:2: InvalidDocblock: PHPDoc is required for constants in OCP. (see https://psalm.dev/008)
public const FLAG_INDEXED = 2; // value should be indexed

Check failure on line 35 in lib/public/Config/IUserPreferences.php

View workflow job for this annotation

GitHub Actions / static-code-analysis-ocp

InvalidDocblock

lib/public/Config/IUserPreferences.php:35:2: InvalidDocblock: PHPDoc is required for constants in OCP. (see https://psalm.dev/008)
Expand Down Expand Up @@ -191,10 +207,10 @@ public function getValuesByUsers(string $app, string $key, ?ValueType $typedAs =
* @param string $value preference value
* @param bool $caseInsensitive non-case-sensitive search, only works if $value is a string
*
* @return list<string>
* @return Generator<string>
* @since 31.0.0
*/
public function searchUsersByValueString(string $app, string $key, string $value, bool $caseInsensitive = false): array;
public function searchUsersByValueString(string $app, string $key, string $value, bool $caseInsensitive = false): Generator;

/**
* List all users storing a specific preference key/value pair.
Expand All @@ -206,10 +222,10 @@ public function searchUsersByValueString(string $app, string $key, string $value
* @param string $key preference key
* @param int $value preference value
*
* @return list<string>
* @return Generator<string>
* @since 31.0.0
*/
public function searchUsersByValueInt(string $app, string $key, int $value): array;
public function searchUsersByValueInt(string $app, string $key, int $value): Generator;

/**
* List all users storing a specific preference key/value pair.
Expand All @@ -221,10 +237,10 @@ public function searchUsersByValueInt(string $app, string $key, int $value): arr
* @param string $key preference key
* @param array $values list of possible preference values
*
* @return list<string>
* @return Generator<string>
* @since 31.0.0
*/
public function searchUsersByValues(string $app, string $key, array $values): array;
public function searchUsersByValues(string $app, string $key, array $values): Generator;

/**
* List all users storing a specific preference key/value pair.
Expand All @@ -236,10 +252,10 @@ public function searchUsersByValues(string $app, string $key, array $values): ar
* @param string $key preference key
* @param bool $value preference value
*
* @return list<string>
* @return Generator<string>
* @since 31.0.0
*/
public function searchUsersByValueBool(string $app, string $key, bool $value): array;
public function searchUsersByValueBool(string $app, string $key, bool $value): Generator;

/**
* Get user preference assigned to a preference key.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,14 @@
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCP\UserPreferences;
namespace OCP\Config;

use OCP\UserPreferences\Exceptions\IncorrectTypeException;
use OCP\Config\Exceptions\IncorrectTypeException;
use UnhandledMatchError;

/**
* Listing of available value type for user preferences
* Listing of available value type for typed config value
*
* @see IUserPreferences
* @since 31.0.0
*/
enum ValueType: int {
Expand Down
15 changes: 0 additions & 15 deletions lib/public/UserPreferences/Exceptions/IncorrectTypeException.php

This file was deleted.

12 changes: 6 additions & 6 deletions tests/lib/UserPreferencesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@
*/
namespace lib;

use OC\UserPreferences;
use OC\Config\UserPreferences;
use OCP\Config\Exceptions\TypeConflictException;
use OCP\Config\Exceptions\UnknownKeyException;
use OCP\Config\IUserPreferences;
use OCP\Config\ValueType;
use OCP\IDBConnection;
use OCP\Security\ICrypto;
use OCP\UserPreferences\Exceptions\TypeConflictException;
use OCP\UserPreferences\Exceptions\UnknownKeyException;
use OCP\UserPreferences\IUserPreferences;
use OCP\UserPreferences\ValueType;
use Psr\Log\LoggerInterface;
use Test\TestCase;

Expand Down Expand Up @@ -275,7 +275,7 @@ protected function tearDown(): void {
* @return IUserPreferences
*/
private function generateUserPreferences(array $preLoading = []): IUserPreferences {
$preferences = new \OC\UserPreferences(
$preferences = new \OC\Config\UserPreferences(
$this->connection,
$this->logger,
$this->crypto,
Expand Down

0 comments on commit dd9c55b

Please sign in to comment.