Skip to content

Commit f48e5aa

Browse files
committed
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>
1 parent bc7309c commit f48e5aa

12 files changed

Lines changed: 45 additions & 56 deletions

File tree

apps/user_ldap/ajax/deleteConfiguration.php

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
<?php
22

33
use OCA\User_LDAP\Helper;
4-
use OCP\IConfig;
5-
use OCP\IDBConnection;
64
use OCP\Server;
75
use OCP\Util;
86

@@ -17,7 +15,7 @@
1715
\OC_JSON::callCheck();
1816

1917
$prefix = (string)$_POST['ldap_serverconfig_chooser'];
20-
$helper = new Helper(Server::get(IConfig::class), Server::get(IDBConnection::class));
18+
$helper = Server::get(Helper::class);
2119
if ($helper->deleteServerConfiguration($prefix)) {
2220
\OC_JSON::success();
2321
} else {

apps/user_ldap/ajax/getNewServerConfigPrefix.php

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22

33
use OCA\User_LDAP\Configuration;
44
use OCA\User_LDAP\Helper;
5-
use OCP\IConfig;
6-
use OCP\IDBConnection;
75
use OCP\Server;
86

97
/**
@@ -16,7 +14,7 @@
1614
\OC_JSON::checkAppEnabled('user_ldap');
1715
\OC_JSON::callCheck();
1816

19-
$helper = new Helper(Server::get(IConfig::class), Server::get(IDBConnection::class));
17+
$helper = Server::get(Helper::class);
2018
$serverConnections = $helper->getServerConfigurationPrefixes();
2119
sort($serverConnections);
2220
$lk = array_pop($serverConnections);

apps/user_ldap/lib/Command/Search.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
use OCA\User_LDAP\LDAP;
1313
use OCA\User_LDAP\User_Proxy;
1414
use OCP\IConfig;
15-
use OCP\IDBConnection;
1615
use OCP\Server;
1716

1817
use Symfony\Component\Console\Command\Command;
@@ -83,7 +82,7 @@ protected function validateOffsetAndLimit(int $offset, int $limit): void {
8382
}
8483

8584
protected function execute(InputInterface $input, OutputInterface $output): int {
86-
$helper = new Helper($this->ocConfig, Server::get(IDBConnection::class));
85+
$helper = Server::get(Helper::class);
8786
$configPrefixes = $helper->getServerConfigurationPrefixes(true);
8887
$ldapWrapper = new LDAP();
8988

apps/user_ldap/lib/Command/SetConfig.php

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@
1111
use OCA\User_LDAP\ConnectionFactory;
1212
use OCA\User_LDAP\Helper;
1313
use OCA\User_LDAP\LDAP;
14-
use OCP\IConfig;
15-
use OCP\IDBConnection;
1614
use OCP\Server;
1715
use Symfony\Component\Console\Command\Command;
1816
use Symfony\Component\Console\Input\InputArgument;
@@ -43,7 +41,7 @@ protected function configure(): void {
4341
}
4442

4543
protected function execute(InputInterface $input, OutputInterface $output): int {
46-
$helper = new Helper(Server::get(IConfig::class), Server::get(IDBConnection::class));
44+
$helper = Server::get(Helper::class);
4745
$availableConfigs = $helper->getServerConfigurationPrefixes();
4846
$configID = $input->getArgument('configID');
4947
if (!in_array($configID, $availableConfigs)) {

apps/user_ldap/lib/Connection.php

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@
1111
use OCA\User_LDAP\Exceptions\ConfigurationIssueException;
1212
use OCP\ICache;
1313
use OCP\ICacheFactory;
14-
use OCP\IConfig;
15-
use OCP\IDBConnection;
1614
use OCP\IL10N;
1715
use OCP\Server;
1816
use OCP\Util;
@@ -156,7 +154,7 @@ public function __construct(
156154
if ($memcache->isAvailable()) {
157155
$this->cache = $memcache->createDistributed();
158156
}
159-
$helper = new Helper(Server::get(IConfig::class), Server::get(IDBConnection::class));
157+
$helper = Server::get(Helper::class);
160158
$this->doNotValidate = !in_array($this->configPrefix,
161159
$helper->getServerConfigurationPrefixes());
162160
$this->logger = Server::get(LoggerInterface::class);

apps/user_ldap/lib/Helper.php

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@
77
*/
88
namespace OCA\User_LDAP;
99

10-
use OCP\AppFramework\Services\IAppConfig;
1110
use OCP\Cache\CappedMemoryCache;
1211
use OCP\DB\QueryBuilder\IQueryBuilder;
12+
use OCP\IAppConfig;
1313
use OCP\IDBConnection;
1414
use OCP\Server;
1515

@@ -52,13 +52,13 @@ public function getServerConfigurationPrefixes(bool $activeConfigurations = fals
5252
}
5353
return array_values(array_filter(
5454
$all,
55-
fn (string $prefix): bool => ($this->appConfig->getAppValueString($prefix . 'ldap_configuration_active') === '1')
55+
fn (string $prefix): bool => ($this->appConfig->getValueString('user_ldap', $prefix . 'ldap_configuration_active') === '1')
5656
));
5757
}
5858

5959
protected function getAllServerConfigurationPrefixes(): array {
6060
$unfilled = ['UNFILLED'];
61-
$prefixes = $this->appConfig->getAppValueArray('configuration_prefixes', $unfilled);
61+
$prefixes = $this->appConfig->getValueArray('user_ldap', 'configuration_prefixes', $unfilled);
6262
if ($prefixes !== $unfilled) {
6363
return $prefixes;
6464
}
@@ -75,7 +75,7 @@ protected function getAllServerConfigurationPrefixes(): array {
7575
}
7676
sort($prefixes);
7777

78-
$this->appConfig->setAppValueArray('configuration_prefixes', $prefixes);
78+
$this->appConfig->setValueArray('user_ldap', 'configuration_prefixes', $prefixes);
7979

8080
return $prefixes;
8181
}
@@ -93,7 +93,7 @@ public function getServerConfigurationHosts(): array {
9393
$referenceConfigkey = 'ldap_host';
9494
$result = [];
9595
foreach ($prefixes as $prefix) {
96-
$result[$prefix] = $this->appConfig->getAppValueString($prefix . $referenceConfigkey);
96+
$result[$prefix] = $this->appConfig->getValueString('user_ldap', $prefix . $referenceConfigkey);
9797
}
9898

9999
return $result;
@@ -115,14 +115,14 @@ public function getNextServerConfigurationPrefix(): string {
115115
}
116116

117117
$prefixes[] = $prefix;
118-
$this->appConfig->setAppValueArray('configuration_prefixes', $prefixes);
118+
$this->appConfig->setValueArray('user_ldap', 'configuration_prefixes', $prefixes);
119119
return $prefix;
120120
}
121121

122122
private function getServersConfig(string $value): array {
123123
$regex = '/' . $value . '$/S';
124124

125-
$keys = $this->appConfig->getAppKeys();
125+
$keys = $this->appConfig->getKeys('user_ldap');
126126
$result = [];
127127
foreach ($keys as $key) {
128128
if (preg_match($regex, $key) === 1) {
@@ -164,7 +164,7 @@ public function deleteServerConfiguration($prefix) {
164164
$deletedRows = $query->executeStatement();
165165

166166
unset($prefixes[$index]);
167-
$this->appConfig->setAppValueArray('configuration_prefixes', array_values($prefixes));
167+
$this->appConfig->setValueArray('user_ldap', 'configuration_prefixes', array_values($prefixes));
168168

169169
return $deletedRows !== 0;
170170
}
@@ -175,7 +175,7 @@ public function deleteServerConfiguration($prefix) {
175175
public function haveDisabledConfigurations(): bool {
176176
$all = $this->getServerConfigurationPrefixes();
177177
foreach ($all as $prefix) {
178-
if ($this->appConfig->getAppValueString($prefix . 'ldap_configuration_active') !== '1') {
178+
if ($this->appConfig->getValueString('user_ldap', $prefix . 'ldap_configuration_active') !== '1') {
179179
return true;
180180
}
181181
}

apps/user_ldap/lib/Jobs/CleanUp.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public function setArguments($arguments): void {
6767
if (isset($arguments['helper'])) {
6868
$this->ldapHelper = $arguments['helper'];
6969
} else {
70-
$this->ldapHelper = new Helper(Server::get(IConfig::class), Server::get(IDBConnection::class));
70+
$this->ldapHelper = Server::get(Helper::class);
7171
}
7272

7373
if (isset($arguments['ocConfig'])) {

apps/user_ldap/lib/Settings/Admin.php

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@
88
use OCA\User_LDAP\Configuration;
99
use OCA\User_LDAP\Helper;
1010
use OCP\AppFramework\Http\TemplateResponse;
11-
use OCP\IConfig;
12-
use OCP\IDBConnection;
1311
use OCP\IL10N;
1412
use OCP\Server;
1513
use OCP\Settings\IDelegatedSettings;
@@ -26,7 +24,7 @@ public function __construct(
2624
* @return TemplateResponse
2725
*/
2826
public function getForm() {
29-
$helper = new Helper(Server::get(IConfig::class), Server::get(IDBConnection::class));
27+
$helper = Server::get(Helper::class);
3028
$prefixes = $helper->getServerConfigurationPrefixes();
3129
if (count($prefixes) === 0) {
3230
$newPrefix = $helper->getNextServerConfigurationPrefix();

apps/user_ldap/tests/AccessTest.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
use OCP\IAppConfig;
2626
use OCP\IAvatarManager;
2727
use OCP\IConfig;
28-
use OCP\IDBConnection;
2928
use OCP\Image;
3029
use OCP\IUserManager;
3130
use OCP\Notification\IManager as INotificationManager;
@@ -110,7 +109,7 @@ private function getConnectorAndLdapMock() {
110109
$this->createMock(INotificationManager::class),
111110
$this->shareManager])
112111
->getMock();
113-
$helper = new Helper(Server::get(IConfig::class), Server::get(IDBConnection::class));
112+
$helper = Server::get(Helper::class);
114113

115114
return [$lw, $connector, $um, $helper];
116115
}

apps/user_ldap/tests/HelperTest.php

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
namespace OCA\User_LDAP\Tests;
99

1010
use OCA\User_LDAP\Helper;
11-
use OCP\AppFramework\Services\IAppConfig;
11+
use OCP\IAppConfig;
1212
use OCP\IDBConnection;
1313
use OCP\Server;
1414
use PHPUnit\Framework\MockObject\MockObject;
@@ -32,36 +32,38 @@ protected function setUp(): void {
3232
}
3333

3434
public function testGetServerConfigurationPrefixes(): void {
35-
$this->appConfig->method('getAppKeys')
35+
$this->appConfig->method('getKeys')
36+
->with('user_ldap')
3637
->willReturn([
3738
'foo',
3839
'ldap_configuration_active',
3940
's1ldap_configuration_active',
4041
]);
4142

42-
$this->appConfig->method('getAppValueArray')
43-
->with('configuration_prefixes')
44-
-> willReturnArgument(1);
43+
$this->appConfig->method('getValueArray')
44+
->with('user_ldap', 'configuration_prefixes')
45+
-> willReturnArgument(2);
4546

4647
$result = $this->helper->getServerConfigurationPrefixes(false);
4748

4849
$this->assertEquals(['', 's1'], $result);
4950
}
5051

5152
public function testGetServerConfigurationPrefixesActive(): void {
52-
$this->appConfig->method('getAppKeys')
53+
$this->appConfig->method('getKeys')
54+
->with('user_ldap')
5355
->willReturn([
5456
'foo',
5557
'ldap_configuration_active',
5658
's1ldap_configuration_active',
5759
]);
5860

59-
$this->appConfig->method('getAppValueArray')
60-
->with('configuration_prefixes')
61-
-> willReturnArgument(1);
61+
$this->appConfig->method('getValueArray')
62+
->with('user_ldap', 'configuration_prefixes')
63+
-> willReturnArgument(2);
6264

63-
$this->appConfig->method('getAppValueString')
64-
->willReturnCallback(function ($key, $default) {
65+
$this->appConfig->method('getValueString')
66+
->willReturnCallback(function ($app, $key, $default) {
6567
if ($key === 's1ldap_configuration_active') {
6668
return '1';
6769
}
@@ -74,7 +76,8 @@ public function testGetServerConfigurationPrefixesActive(): void {
7476
}
7577

7678
public function testGetServerConfigurationHostFromAppKeys(): void {
77-
$this->appConfig->method('getAppKeys')
79+
$this->appConfig->method('getKeys')
80+
->with('user_ldap')
7881
->willReturn([
7982
'foo',
8083
'ldap_host',
@@ -85,12 +88,12 @@ public function testGetServerConfigurationHostFromAppKeys(): void {
8588
's02ldap_configuration_active',
8689
]);
8790

88-
$this->appConfig->method('getAppValueArray')
89-
->with('configuration_prefixes')
90-
-> willReturnArgument(1);
91+
$this->appConfig->method('getValueArray')
92+
->with('user_ldap', 'configuration_prefixes')
93+
-> willReturnArgument(2);
9194

92-
$this->appConfig->method('getAppValueString')
93-
->willReturnCallback(function ($key, $default) {
95+
$this->appConfig->method('getValueString')
96+
->willReturnCallback(function ($app, $key, $default) {
9497
if ($key === 'ldap_host') {
9598
return 'example.com';
9699
}
@@ -112,18 +115,18 @@ public function testGetServerConfigurationHostFromAppKeys(): void {
112115
public function testGetServerConfigurationHost(): void {
113116
$this->appConfig
114117
->expects(self::never())
115-
->method('getAppKeys');
118+
->method('getKeys');
116119

117-
$this->appConfig->method('getAppValueArray')
118-
->with('configuration_prefixes')
120+
$this->appConfig->method('getValueArray')
121+
->with('user_ldap', 'configuration_prefixes')
119122
-> willReturn([
120123
'',
121124
's1',
122125
's02',
123126
]);
124127

125-
$this->appConfig->method('getAppValueString')
126-
->willReturnCallback(function ($key, $default) {
128+
$this->appConfig->method('getValueString')
129+
->willReturnCallback(function ($app, $key, $default) {
127130
if ($key === 'ldap_host') {
128131
return 'example.com';
129132
}

0 commit comments

Comments
 (0)