Background jobs can take 4k of characters only. We find a good batch size.
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
This commit is contained in:
parent
497ee3e3e6
commit
42ddb12fd9
|
@ -24,13 +24,13 @@
|
||||||
namespace OCA\User_LDAP\Migration;
|
namespace OCA\User_LDAP\Migration;
|
||||||
|
|
||||||
use OCA\User_LDAP\Helper;
|
use OCA\User_LDAP\Helper;
|
||||||
use OCA\User_LDAP\ILDAPWrapper;
|
use OCA\User_LDAP\LDAP;
|
||||||
use OCA\User_LDAP\Mapping\GroupMapping;
|
use OCA\User_LDAP\Mapping\GroupMapping;
|
||||||
use OCA\User_LDAP\User_Proxy;
|
use OCA\User_LDAP\User_Proxy;
|
||||||
use OCP\IConfig;
|
use OCP\IConfig;
|
||||||
|
|
||||||
class UUIDFixGroup extends UUIDFix {
|
class UUIDFixGroup extends UUIDFix {
|
||||||
public function __construct(GroupMapping $mapper, ILDAPWrapper $ldap, IConfig $config, Helper $helper) {
|
public function __construct(GroupMapping $mapper, LDAP $ldap, IConfig $config, Helper $helper) {
|
||||||
$this->mapper = $mapper;
|
$this->mapper = $mapper;
|
||||||
$this->proxy = new User_Proxy($helper->getServerConfigurationPrefixes(true), $ldap, $config);
|
$this->proxy = new User_Proxy($helper->getServerConfigurationPrefixes(true), $ldap, $config);
|
||||||
}
|
}
|
||||||
|
|
|
@ -58,7 +58,7 @@ class UUIDFixInsert implements IRepairStep {
|
||||||
* @since 9.1.0
|
* @since 9.1.0
|
||||||
*/
|
*/
|
||||||
public function getName() {
|
public function getName() {
|
||||||
return 'Insert UUIDFix background job for user and group batches of 500';
|
return 'Insert UUIDFix background job for user and group in batches';
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -75,15 +75,26 @@ class UUIDFixInsert implements IRepairStep {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
$batchSize = 500;
|
|
||||||
foreach ([$this->userMapper, $this->groupMapper] as $mapper) {
|
foreach ([$this->userMapper, $this->groupMapper] as $mapper) {
|
||||||
$offset = 0;
|
$offset = 0;
|
||||||
|
$batchSize = 50;
|
||||||
$jobClass = $mapper instanceof UserMapping ? UUIDFixUser::class : UUIDFixGroup::class;
|
$jobClass = $mapper instanceof UserMapping ? UUIDFixUser::class : UUIDFixGroup::class;
|
||||||
do {
|
do {
|
||||||
|
$retry = false;
|
||||||
$records = $mapper->getList($offset, $batchSize);
|
$records = $mapper->getList($offset, $batchSize);
|
||||||
$this->jobList->add($jobClass, ['records' => $records]);
|
if(count($records) === 0){
|
||||||
$offset += $batchSize;
|
continue;
|
||||||
} while (count($records) === $batchSize);
|
}
|
||||||
|
try {
|
||||||
|
$this->jobList->add($jobClass, ['records' => $records]);
|
||||||
|
$offset += $batchSize;
|
||||||
|
} catch (\InvalidArgumentException $e) {
|
||||||
|
if(strpos($e->getMessage(), 'Background job arguments can\'t exceed 4000') !== false) {
|
||||||
|
$batchSize = intval(floor(count($records) * 0.8));
|
||||||
|
$retry = true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
} while (count($records) === $batchSize || $retry);
|
||||||
}
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -24,13 +24,13 @@
|
||||||
namespace OCA\User_LDAP\Migration;
|
namespace OCA\User_LDAP\Migration;
|
||||||
|
|
||||||
use OCA\User_LDAP\Helper;
|
use OCA\User_LDAP\Helper;
|
||||||
use OCA\User_LDAP\ILDAPWrapper;
|
use OCA\User_LDAP\LDAP;
|
||||||
use OCA\User_LDAP\Mapping\UserMapping;
|
use OCA\User_LDAP\Mapping\UserMapping;
|
||||||
use OCA\User_LDAP\Group_Proxy;
|
use OCA\User_LDAP\Group_Proxy;
|
||||||
use OCP\IConfig;
|
use OCP\IConfig;
|
||||||
|
|
||||||
class UUIDFixUser extends UUIDFix {
|
class UUIDFixUser extends UUIDFix {
|
||||||
public function __construct(UserMapping $mapper, ILDAPWrapper $ldap, IConfig $config, Helper $helper) {
|
public function __construct(UserMapping $mapper, LDAP $ldap, IConfig $config, Helper $helper) {
|
||||||
$this->mapper = $mapper;
|
$this->mapper = $mapper;
|
||||||
$this->proxy = new Group_Proxy($helper->getServerConfigurationPrefixes(true), $ldap, $config);
|
$this->proxy = new Group_Proxy($helper->getServerConfigurationPrefixes(true), $ldap, $config);
|
||||||
}
|
}
|
||||||
|
|
|
@ -23,10 +23,10 @@
|
||||||
|
|
||||||
namespace OCA\User_LDAP\Tests\Migration;
|
namespace OCA\User_LDAP\Tests\Migration;
|
||||||
|
|
||||||
|
use OCA\User_LDAP\LDAP;
|
||||||
use Test\TestCase;
|
use Test\TestCase;
|
||||||
use OCA\User_LDAP\Access;
|
use OCA\User_LDAP\Access;
|
||||||
use OCA\User_LDAP\Helper;
|
use OCA\User_LDAP\Helper;
|
||||||
use OCA\User_LDAP\ILDAPWrapper;
|
|
||||||
use OCA\User_LDAP\Migration\UUIDFixUser;
|
use OCA\User_LDAP\Migration\UUIDFixUser;
|
||||||
use OCA\User_LDAP\Mapping\UserMapping;
|
use OCA\User_LDAP\Mapping\UserMapping;
|
||||||
use OCA\User_LDAP\Mapping\GroupMapping;
|
use OCA\User_LDAP\Mapping\GroupMapping;
|
||||||
|
@ -40,7 +40,7 @@ abstract class AbstractUUIDFixTest extends TestCase {
|
||||||
/** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */
|
/** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */
|
||||||
protected $config;
|
protected $config;
|
||||||
|
|
||||||
/** @var ILDAPWrapper|\PHPUnit_Framework_MockObject_MockObject */
|
/** @var LDAP|\PHPUnit_Framework_MockObject_MockObject */
|
||||||
protected $ldap;
|
protected $ldap;
|
||||||
|
|
||||||
/** @var UserMapping|GroupMapping|\PHPUnit_Framework_MockObject_MockObject */
|
/** @var UserMapping|GroupMapping|\PHPUnit_Framework_MockObject_MockObject */
|
||||||
|
@ -61,7 +61,7 @@ abstract class AbstractUUIDFixTest extends TestCase {
|
||||||
public function setUp() {
|
public function setUp() {
|
||||||
parent::setUp();
|
parent::setUp();
|
||||||
|
|
||||||
$this->ldap = $this->createMock(ILDAPWrapper::class);
|
$this->ldap = $this->createMock(LDAP::class);
|
||||||
$this->config = $this->createMock(IConfig::class);
|
$this->config = $this->createMock(IConfig::class);
|
||||||
$this->access = $this->createMock(Access::class);
|
$this->access = $this->createMock(Access::class);
|
||||||
|
|
||||||
|
|
|
@ -63,7 +63,7 @@ class UUIDFixInsertTest extends TestCase {
|
||||||
}
|
}
|
||||||
|
|
||||||
public function testGetName() {
|
public function testGetName() {
|
||||||
$this->assertSame('Insert UUIDFix background job for user and group batches of 500', $this->job->getName());
|
$this->assertSame('Insert UUIDFix background job for user and group in batches', $this->job->getName());
|
||||||
}
|
}
|
||||||
|
|
||||||
public function recordProvider() {
|
public function recordProvider() {
|
||||||
|
@ -72,11 +72,11 @@ class UUIDFixInsertTest extends TestCase {
|
||||||
'name' => 'Something',
|
'name' => 'Something',
|
||||||
'uuid' => 'AB12-3456-CDEF7-8GH9'
|
'uuid' => 'AB12-3456-CDEF7-8GH9'
|
||||||
];
|
];
|
||||||
array_fill(0, 500, $record);
|
array_fill(0, 50, $record);
|
||||||
|
|
||||||
$userBatches = [
|
$userBatches = [
|
||||||
0 => array_fill(0, 500, $record),
|
0 => array_fill(0, 50, $record),
|
||||||
1 => array_fill(0, 500, $record),
|
1 => array_fill(0, 50, $record),
|
||||||
2 => array_fill(0, 13, $record),
|
2 => array_fill(0, 13, $record),
|
||||||
];
|
];
|
||||||
|
|
||||||
|
@ -89,6 +89,29 @@ class UUIDFixInsertTest extends TestCase {
|
||||||
];
|
];
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function recordProviderTooLongAndNone() {
|
||||||
|
$record = [
|
||||||
|
'dn' => 'cn=somerecord,dc=somewhere',
|
||||||
|
'name' => 'Something',
|
||||||
|
'uuid' => 'AB12-3456-CDEF7-8GH9'
|
||||||
|
];
|
||||||
|
array_fill(0, 50, $record);
|
||||||
|
|
||||||
|
$userBatches = [
|
||||||
|
0 => array_fill(0, 50, $record),
|
||||||
|
1 => array_fill(0, 40, $record),
|
||||||
|
2 => array_fill(0, 32, $record),
|
||||||
|
3 => array_fill(0, 32, $record),
|
||||||
|
4 => array_fill(0, 23, $record),
|
||||||
|
];
|
||||||
|
|
||||||
|
$groupBatches = [0 => []];
|
||||||
|
|
||||||
|
return [
|
||||||
|
['userBatches' => $userBatches, 'groupBatches' => $groupBatches]
|
||||||
|
];
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @dataProvider recordProvider
|
* @dataProvider recordProvider
|
||||||
*/
|
*/
|
||||||
|
@ -100,12 +123,12 @@ class UUIDFixInsertTest extends TestCase {
|
||||||
|
|
||||||
$this->userMapper->expects($this->exactly(3))
|
$this->userMapper->expects($this->exactly(3))
|
||||||
->method('getList')
|
->method('getList')
|
||||||
->withConsecutive([0, 500], [500, 500], [1000, 500])
|
->withConsecutive([0, 50], [50, 50], [100, 50])
|
||||||
->willReturnOnConsecutiveCalls($userBatches[0], $userBatches[1], $userBatches[2]);
|
->willReturnOnConsecutiveCalls($userBatches[0], $userBatches[1], $userBatches[2]);
|
||||||
|
|
||||||
$this->groupMapper->expects($this->exactly(1))
|
$this->groupMapper->expects($this->exactly(1))
|
||||||
->method('getList')
|
->method('getList')
|
||||||
->with(0, 500)
|
->with(0, 50)
|
||||||
->willReturn($groupBatches[0]);
|
->willReturn($groupBatches[0]);
|
||||||
|
|
||||||
$this->jobList->expects($this->exactly(4))
|
$this->jobList->expects($this->exactly(4))
|
||||||
|
@ -116,6 +139,43 @@ class UUIDFixInsertTest extends TestCase {
|
||||||
$this->job->run($out);
|
$this->job->run($out);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @dataProvider recordProviderTooLongAndNone
|
||||||
|
*/
|
||||||
|
public function testRunWithManyAndNone($userBatches, $groupBatches) {
|
||||||
|
$this->config->expects($this->once())
|
||||||
|
->method('getAppValue')
|
||||||
|
->with('user_ldap', 'installed_version', '1.2.1')
|
||||||
|
->willReturn('1.2.0');
|
||||||
|
|
||||||
|
$this->userMapper->expects($this->exactly(5))
|
||||||
|
->method('getList')
|
||||||
|
->withConsecutive([0, 50], [0, 40], [0, 32], [32, 32], [64, 32])
|
||||||
|
->willReturnOnConsecutiveCalls($userBatches[0], $userBatches[1], $userBatches[2], $userBatches[3], $userBatches[4]);
|
||||||
|
|
||||||
|
$this->groupMapper->expects($this->once())
|
||||||
|
->method('getList')
|
||||||
|
->with(0, 50)
|
||||||
|
->willReturn($groupBatches[0]);
|
||||||
|
|
||||||
|
$this->jobList->expects($this->at(0))
|
||||||
|
->method('add')
|
||||||
|
->willThrowException(new \InvalidArgumentException('Background job arguments can\'t exceed 4000 etc'));
|
||||||
|
$this->jobList->expects($this->at(1))
|
||||||
|
->method('add')
|
||||||
|
->willThrowException(new \InvalidArgumentException('Background job arguments can\'t exceed 4000 etc'));
|
||||||
|
$this->jobList->expects($this->at(2))
|
||||||
|
->method('add');
|
||||||
|
$this->jobList->expects($this->at(3))
|
||||||
|
->method('add');
|
||||||
|
$this->jobList->expects($this->at(4))
|
||||||
|
->method('add');
|
||||||
|
|
||||||
|
/** @var IOutput $out */
|
||||||
|
$out = $this->createMock(IOutput::class);
|
||||||
|
$this->job->run($out);
|
||||||
|
}
|
||||||
|
|
||||||
public function testDonNotRun() {
|
public function testDonNotRun() {
|
||||||
$this->config->expects($this->once())
|
$this->config->expects($this->once())
|
||||||
->method('getAppValue')
|
->method('getAppValue')
|
||||||
|
|
Loading…
Reference in New Issue