Skip to content

Commit

Permalink
Merge pull request #856 from bdovaz/migrate-to-event-dispatcher
Browse files Browse the repository at this point in the history
feat: migrate from deprecated PublicEmitter to IEventDispatcher
  • Loading branch information
blizzz authored Jul 3, 2024
2 parents ecf7a27 + 0597233 commit 9c4e9bd
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 17 deletions.
24 changes: 13 additions & 11 deletions lib/GroupManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,15 @@

namespace OCA\User_SAML;

use OC\Hooks\PublicEmitter;
use OCA\User_SAML\Exceptions\GroupNotFoundException;
use OCA\User_SAML\Exceptions\NonMigratableGroupException;
use OCA\User_SAML\Jobs\MigrateGroups;
use OCP\BackgroundJob\IJobList;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\Group\Events\BeforeGroupCreatedEvent;
use OCP\Group\Events\BeforeGroupDeletedEvent;
use OCP\Group\Events\GroupCreatedEvent;
use OCP\Group\Events\GroupDeletedEvent;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\IGroup;
Expand All @@ -31,6 +35,8 @@ class GroupManager {
private $ownGroupBackend;
/** @var IConfig */
private $config;
/** @var IEventDispatcher */
private $dispatcher;
/** @var IJobList */
private $jobList;
/** @var SAMLSettings */
Expand All @@ -42,13 +48,15 @@ public function __construct(
IGroupManager $groupManager,
GroupBackend $ownGroupBackend,
IConfig $config,
IEventDispatcher $dispatcher,
IJobList $jobList,
SAMLSettings $settings
) {
$this->db = $db;
$this->groupManager = $groupManager;
$this->ownGroupBackend = $ownGroupBackend;
$this->config = $config;
$this->dispatcher = $dispatcher;
$this->jobList = $jobList;
$this->settings = $settings;
}
Expand Down Expand Up @@ -141,11 +149,9 @@ protected function unassignUserFromGroup(IUser $user, string $gid): void {
if ($this->hasSamlBackend($group)) {
$this->ownGroupBackend->removeFromGroup($user->getUID(), $group->getGID());
if ($this->ownGroupBackend->countUsersInGroup($gid) === 0) {
/** @psalm-suppress UndefinedInterfaceMethod */
$this->groupManager->emit('\OC\Group', 'preDelete', [$group]);
$this->dispatcher->dispatchTyped(new BeforeGroupDeletedEvent($group));
$this->ownGroupBackend->deleteGroup($group->getGID());
/** @psalm-suppress UndefinedInterfaceMethod */
$this->groupManager->emit('\OC\Group', 'postDelete', [$group]);
$this->dispatcher->dispatchTyped(new GroupDeletedEvent($group));
}
} else {
$group->removeUser($user);
Expand All @@ -172,17 +178,13 @@ protected function assignUserToGroup(IUser $user, string $gid): void {
}

protected function createGroupInBackend(string $gid, ?string $originalGid = null): ?IGroup {
if ($this->groupManager instanceof PublicEmitter) {
$this->groupManager->emit('\OC\Group', 'preCreate', [$gid]);
}
$this->dispatcher->dispatchTyped(new BeforeGroupCreatedEvent($gid));
if (!$this->ownGroupBackend->createGroup($gid, $originalGid ?? $gid)) {
return null;
}

$group = $this->groupManager->get($gid);
if ($this->groupManager instanceof PublicEmitter) {
$this->groupManager->emit('\OC\Group', 'postCreate', [$group]);
}
$this->dispatcher->dispatchTyped(new GroupCreatedEvent($group));

return $group;
}
Expand Down
6 changes: 0 additions & 6 deletions tests/stub.phpstub
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,3 @@ namespace OC\Group {
\OCP\Group\Backend\INamedBackend {
}
}

namespace OC\Hooks {
interface PublicEmitter {
public function emit($scope, $method, array $arguments = []);
}
}
28 changes: 28 additions & 0 deletions tests/unit/GroupManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@
use OCA\User_SAML\GroupBackend;
use OCA\User_SAML\GroupManager;
use OCA\User_SAML\SAMLSettings;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\Group\Events\BeforeGroupCreatedEvent;
use OCP\Group\Events\BeforeGroupDeletedEvent;
use OCP\Group\Events\GroupCreatedEvent;
use OCP\Group\Events\GroupDeletedEvent;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\IGroup;
Expand All @@ -29,6 +34,8 @@ class GroupManagerTest extends TestCase {
private $ownGroupBackend;
/** @var IConfig|MockObject */
private $config;
/** @var IEventDispatcher|MockObject */
private $eventDispatcher;
/** @var JobList|MockObject */
private $jobList;
/** @var SAMLSettings|MockObject */
Expand All @@ -42,6 +49,7 @@ protected function setUp(): void {
$this->groupManager = $this->createMock(Manager::class);
$this->ownGroupBackend = $this->createMock(GroupBackend::class);
$this->config = $this->createMock(IConfig::class);
$this->eventDispatcher = $this->createMock(IEventDispatcher::class);
$this->jobList = $this->createMock(JobList::class);
$this->settings = $this->createMock(SAMLSettings::class);
$this->ownGroupManager = $this->createMock(GroupManager::class);
Expand All @@ -55,6 +63,7 @@ public function getGroupManager(array $mockedFunctions = []) {
$this->groupManager,
$this->ownGroupBackend,
$this->config,
$this->eventDispatcher,
$this->jobList,
$this->settings
])
Expand All @@ -66,6 +75,7 @@ public function getGroupManager(array $mockedFunctions = []) {
$this->groupManager,
$this->ownGroupBackend,
$this->config,
$this->eventDispatcher,
$this->jobList,
$this->settings
);
Expand Down Expand Up @@ -167,6 +177,12 @@ public function testUnassignUserFromGroups() {
$this->ownGroupBackend
->expects($this->once())
->method('deleteGroup');
$this->eventDispatcher->expects($this->exactly(2))
->method('dispatchTyped')
->withConsecutive(
[new BeforeGroupDeletedEvent($groupA)],
[new GroupDeletedEvent($groupA)]
);

$this->invokePrivate($this->ownGroupManager, 'handleUserUnassignedFromGroups', [$user, ['groupA']]);
}
Expand Down Expand Up @@ -222,6 +238,12 @@ public function testAssignUserToNonExistingGroups() {
->method('createGroup')
->with('SAML_groupB', 'groupB')
->willReturn(true);
$this->eventDispatcher->expects($this->exactly(2))
->method('dispatchTyped')
->withConsecutive(
[new BeforeGroupCreatedEvent('SAML_groupB')],
[new GroupCreatedEvent($groupB)]
);
// assert user gets added to group
$groupB->expects($this->once())
->method('addUser')
Expand Down Expand Up @@ -270,6 +292,12 @@ public function testAssignUserToGroupsWithCollision() {
->method('createGroup')
->with('SAML_groupC', 'groupC')
->willReturn(true);
$this->eventDispatcher->expects($this->exactly(2))
->method('dispatchTyped')
->withConsecutive(
[new BeforeGroupCreatedEvent('SAML_groupC')],
[new GroupCreatedEvent($groupC)]
);
// assert user gets added to group
$groupC->expects($this->once())
->method('addUser')
Expand Down

0 comments on commit 9c4e9bd

Please sign in to comment.