fix(user_ldap): Switch to OCP\IAppConfig and fix Helper constructor calls

Using OCP\AppFramework\Services\IAppConfig is not possible because the
 Helper is queried from places outside of the application DI container
(ajax pages, tests, other applications through ILDAPProviderFactory…)

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
This commit is contained in:
Côme Chilliet 2025-05-22 15:40:16 +02:00
parent bc7309ca1c
commit f48e5aa1f3
No known key found for this signature in database
GPG key ID: A3E2F658B28C760A
12 changed files with 45 additions and 56 deletions

View file

@ -1,8 +1,6 @@
<?php
use OCA\User_LDAP\Helper;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\Server;
use OCP\Util;
@ -17,7 +15,7 @@ use OCP\Util;
\OC_JSON::callCheck();
$prefix = (string)$_POST['ldap_serverconfig_chooser'];
$helper = new Helper(Server::get(IConfig::class), Server::get(IDBConnection::class));
$helper = Server::get(Helper::class);
if ($helper->deleteServerConfiguration($prefix)) {
\OC_JSON::success();
} else {

View file

@ -2,8 +2,6 @@
use OCA\User_LDAP\Configuration;
use OCA\User_LDAP\Helper;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\Server;
/**
@ -16,7 +14,7 @@ use OCP\Server;
\OC_JSON::checkAppEnabled('user_ldap');
\OC_JSON::callCheck();
$helper = new Helper(Server::get(IConfig::class), Server::get(IDBConnection::class));
$helper = Server::get(Helper::class);
$serverConnections = $helper->getServerConfigurationPrefixes();
sort($serverConnections);
$lk = array_pop($serverConnections);

View file

@ -12,7 +12,6 @@ use OCA\User_LDAP\Helper;
use OCA\User_LDAP\LDAP;
use OCA\User_LDAP\User_Proxy;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\Server;
use Symfony\Component\Console\Command\Command;
@ -83,7 +82,7 @@ class Search extends Command {
}
protected function execute(InputInterface $input, OutputInterface $output): int {
$helper = new Helper($this->ocConfig, Server::get(IDBConnection::class));
$helper = Server::get(Helper::class);
$configPrefixes = $helper->getServerConfigurationPrefixes(true);
$ldapWrapper = new LDAP();

View file

@ -11,8 +11,6 @@ use OCA\User_LDAP\Configuration;
use OCA\User_LDAP\ConnectionFactory;
use OCA\User_LDAP\Helper;
use OCA\User_LDAP\LDAP;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\Server;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputArgument;
@ -43,7 +41,7 @@ class SetConfig extends Command {
}
protected function execute(InputInterface $input, OutputInterface $output): int {
$helper = new Helper(Server::get(IConfig::class), Server::get(IDBConnection::class));
$helper = Server::get(Helper::class);
$availableConfigs = $helper->getServerConfigurationPrefixes();
$configID = $input->getArgument('configID');
if (!in_array($configID, $availableConfigs)) {

View file

@ -11,8 +11,6 @@ use OC\ServerNotAvailableException;
use OCA\User_LDAP\Exceptions\ConfigurationIssueException;
use OCP\ICache;
use OCP\ICacheFactory;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\IL10N;
use OCP\Server;
use OCP\Util;
@ -156,7 +154,7 @@ class Connection extends LDAPUtility {
if ($memcache->isAvailable()) {
$this->cache = $memcache->createDistributed();
}
$helper = new Helper(Server::get(IConfig::class), Server::get(IDBConnection::class));
$helper = Server::get(Helper::class);
$this->doNotValidate = !in_array($this->configPrefix,
$helper->getServerConfigurationPrefixes());
$this->logger = Server::get(LoggerInterface::class);

View file

@ -7,9 +7,9 @@
*/
namespace OCA\User_LDAP;
use OCP\AppFramework\Services\IAppConfig;
use OCP\Cache\CappedMemoryCache;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\IAppConfig;
use OCP\IDBConnection;
use OCP\Server;
@ -52,13 +52,13 @@ class Helper {
}
return array_values(array_filter(
$all,
fn (string $prefix): bool => ($this->appConfig->getAppValueString($prefix . 'ldap_configuration_active') === '1')
fn (string $prefix): bool => ($this->appConfig->getValueString('user_ldap', $prefix . 'ldap_configuration_active') === '1')
));
}
protected function getAllServerConfigurationPrefixes(): array {
$unfilled = ['UNFILLED'];
$prefixes = $this->appConfig->getAppValueArray('configuration_prefixes', $unfilled);
$prefixes = $this->appConfig->getValueArray('user_ldap', 'configuration_prefixes', $unfilled);
if ($prefixes !== $unfilled) {
return $prefixes;
}
@ -75,7 +75,7 @@ class Helper {
}
sort($prefixes);
$this->appConfig->setAppValueArray('configuration_prefixes', $prefixes);
$this->appConfig->setValueArray('user_ldap', 'configuration_prefixes', $prefixes);
return $prefixes;
}
@ -93,7 +93,7 @@ class Helper {
$referenceConfigkey = 'ldap_host';
$result = [];
foreach ($prefixes as $prefix) {
$result[$prefix] = $this->appConfig->getAppValueString($prefix . $referenceConfigkey);
$result[$prefix] = $this->appConfig->getValueString('user_ldap', $prefix . $referenceConfigkey);
}
return $result;
@ -115,14 +115,14 @@ class Helper {
}
$prefixes[] = $prefix;
$this->appConfig->setAppValueArray('configuration_prefixes', $prefixes);
$this->appConfig->setValueArray('user_ldap', 'configuration_prefixes', $prefixes);
return $prefix;
}
private function getServersConfig(string $value): array {
$regex = '/' . $value . '$/S';
$keys = $this->appConfig->getAppKeys();
$keys = $this->appConfig->getKeys('user_ldap');
$result = [];
foreach ($keys as $key) {
if (preg_match($regex, $key) === 1) {
@ -164,7 +164,7 @@ class Helper {
$deletedRows = $query->executeStatement();
unset($prefixes[$index]);
$this->appConfig->setAppValueArray('configuration_prefixes', array_values($prefixes));
$this->appConfig->setValueArray('user_ldap', 'configuration_prefixes', array_values($prefixes));
return $deletedRows !== 0;
}
@ -175,7 +175,7 @@ class Helper {
public function haveDisabledConfigurations(): bool {
$all = $this->getServerConfigurationPrefixes();
foreach ($all as $prefix) {
if ($this->appConfig->getAppValueString($prefix . 'ldap_configuration_active') !== '1') {
if ($this->appConfig->getValueString('user_ldap', $prefix . 'ldap_configuration_active') !== '1') {
return true;
}
}

View file

@ -67,7 +67,7 @@ class CleanUp extends TimedJob {
if (isset($arguments['helper'])) {
$this->ldapHelper = $arguments['helper'];
} else {
$this->ldapHelper = new Helper(Server::get(IConfig::class), Server::get(IDBConnection::class));
$this->ldapHelper = Server::get(Helper::class);
}
if (isset($arguments['ocConfig'])) {

View file

@ -8,8 +8,6 @@ namespace OCA\User_LDAP\Settings;
use OCA\User_LDAP\Configuration;
use OCA\User_LDAP\Helper;
use OCP\AppFramework\Http\TemplateResponse;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\IL10N;
use OCP\Server;
use OCP\Settings\IDelegatedSettings;
@ -26,7 +24,7 @@ class Admin implements IDelegatedSettings {
* @return TemplateResponse
*/
public function getForm() {
$helper = new Helper(Server::get(IConfig::class), Server::get(IDBConnection::class));
$helper = Server::get(Helper::class);
$prefixes = $helper->getServerConfigurationPrefixes();
if (count($prefixes) === 0) {
$newPrefix = $helper->getNextServerConfigurationPrefix();

View file

@ -25,7 +25,6 @@ use OCP\HintException;
use OCP\IAppConfig;
use OCP\IAvatarManager;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\Image;
use OCP\IUserManager;
use OCP\Notification\IManager as INotificationManager;
@ -110,7 +109,7 @@ class AccessTest extends TestCase {
$this->createMock(INotificationManager::class),
$this->shareManager])
->getMock();
$helper = new Helper(Server::get(IConfig::class), Server::get(IDBConnection::class));
$helper = Server::get(Helper::class);
return [$lw, $connector, $um, $helper];
}

View file

@ -8,7 +8,7 @@ declare(strict_types=1);
namespace OCA\User_LDAP\Tests;
use OCA\User_LDAP\Helper;
use OCP\AppFramework\Services\IAppConfig;
use OCP\IAppConfig;
use OCP\IDBConnection;
use OCP\Server;
use PHPUnit\Framework\MockObject\MockObject;
@ -32,16 +32,17 @@ class HelperTest extends \Test\TestCase {
}
public function testGetServerConfigurationPrefixes(): void {
$this->appConfig->method('getAppKeys')
$this->appConfig->method('getKeys')
->with('user_ldap')
->willReturn([
'foo',
'ldap_configuration_active',
's1ldap_configuration_active',
]);
$this->appConfig->method('getAppValueArray')
->with('configuration_prefixes')
-> willReturnArgument(1);
$this->appConfig->method('getValueArray')
->with('user_ldap', 'configuration_prefixes')
-> willReturnArgument(2);
$result = $this->helper->getServerConfigurationPrefixes(false);
@ -49,19 +50,20 @@ class HelperTest extends \Test\TestCase {
}
public function testGetServerConfigurationPrefixesActive(): void {
$this->appConfig->method('getAppKeys')
$this->appConfig->method('getKeys')
->with('user_ldap')
->willReturn([
'foo',
'ldap_configuration_active',
's1ldap_configuration_active',
]);
$this->appConfig->method('getAppValueArray')
->with('configuration_prefixes')
-> willReturnArgument(1);
$this->appConfig->method('getValueArray')
->with('user_ldap', 'configuration_prefixes')
-> willReturnArgument(2);
$this->appConfig->method('getAppValueString')
->willReturnCallback(function ($key, $default) {
$this->appConfig->method('getValueString')
->willReturnCallback(function ($app, $key, $default) {
if ($key === 's1ldap_configuration_active') {
return '1';
}
@ -74,7 +76,8 @@ class HelperTest extends \Test\TestCase {
}
public function testGetServerConfigurationHostFromAppKeys(): void {
$this->appConfig->method('getAppKeys')
$this->appConfig->method('getKeys')
->with('user_ldap')
->willReturn([
'foo',
'ldap_host',
@ -85,12 +88,12 @@ class HelperTest extends \Test\TestCase {
's02ldap_configuration_active',
]);
$this->appConfig->method('getAppValueArray')
->with('configuration_prefixes')
-> willReturnArgument(1);
$this->appConfig->method('getValueArray')
->with('user_ldap', 'configuration_prefixes')
-> willReturnArgument(2);
$this->appConfig->method('getAppValueString')
->willReturnCallback(function ($key, $default) {
$this->appConfig->method('getValueString')
->willReturnCallback(function ($app, $key, $default) {
if ($key === 'ldap_host') {
return 'example.com';
}
@ -112,18 +115,18 @@ class HelperTest extends \Test\TestCase {
public function testGetServerConfigurationHost(): void {
$this->appConfig
->expects(self::never())
->method('getAppKeys');
->method('getKeys');
$this->appConfig->method('getAppValueArray')
->with('configuration_prefixes')
$this->appConfig->method('getValueArray')
->with('user_ldap', 'configuration_prefixes')
-> willReturn([
'',
's1',
's02',
]);
$this->appConfig->method('getAppValueString')
->willReturnCallback(function ($key, $default) {
$this->appConfig->method('getValueString')
->willReturnCallback(function ($app, $key, $default) {
if ($key === 'ldap_host') {
return 'example.com';
}

View file

@ -16,7 +16,6 @@ use OCA\User_LDAP\User\Manager;
use OCA\User_LDAP\UserPluginManager;
use OCP\IAvatarManager;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\Image;
use OCP\IUserManager;
use OCP\Server;
@ -125,7 +124,7 @@ abstract class AbstractIntegrationTest {
* initializes the test Helper
*/
protected function initHelper() {
$this->helper = new Helper(Server::get(IConfig::class), Server::get(IDBConnection::class));
$this->helper = Server::get(Helper::class);
}
/**

View file

@ -21,7 +21,6 @@ use OCA\User_LDAP\User_LDAP;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\ICacheFactory;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\IServerContainer;
use OCP\Server;
use Psr\Log\LoggerInterface;
@ -199,7 +198,7 @@ class LDAPProviderTest extends \Test\TestCase {
$server = $this->getServerMock($userBackend, $this->getDefaultGroupBackendMock());
$helper = new Helper(Server::get(IConfig::class), Server::get(IDBConnection::class));
$helper = Server::get(Helper::class);
$ldapProvider = $this->getLDAPProvider($server);
$this->assertEquals(
@ -212,7 +211,7 @@ class LDAPProviderTest extends \Test\TestCase {
$server = $this->getServerMock($userBackend, $this->getDefaultGroupBackendMock());
$helper = new Helper(Server::get(IConfig::class), Server::get(IDBConnection::class));
$helper = Server::get(Helper::class);
$ldapProvider = $this->getLDAPProvider($server);
$this->assertEquals(