Skip to content

Commit 75fb513

Browse files
simonLeary42simonLeary42
authored
remove duplicated code from UnityGroup/UnityOrg (#393)
1 parent 126ec72 commit 75fb513

14 files changed

+130
-122
lines changed

‎resources/autoload.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
// load libs
1515
require_once __DIR__ . "/lib/UnityLDAP.php";
1616
require_once __DIR__ . "/lib/UnityUser.php";
17+
require_once __DIR__ . "/lib/PosixGroup.php";
1718
require_once __DIR__ . "/lib/UnityGroup.php";
1819
require_once __DIR__ . "/lib/UnityOrg.php";
1920
require_once __DIR__ . "/lib/UnitySQL.php";

‎resources/lib/PosixGroup.php‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
<?php
2+
3+
namespace UnityWebPortal\lib;
4+
5+
use PHPOpenLDAPer\LDAPEntry;
6+
use \Exception;
7+
8+
/*
9+
does not extend LDAPEntry because UnityGroup extends this and I don't want UnityGroup
10+
to extend LDAPEntry because the functions from LDAPEntry should not be exposed there
11+
*/
12+
class PosixGroup
13+
{
14+
protected LDAPEntry $entry;
15+
16+
public function __construct(LDAPEntry $entry)
17+
{
18+
$this->entry = $entry;
19+
}
20+
21+
public function getDN(): string
22+
{
23+
return $this->entry->getDN();
24+
}
25+
26+
public function equals(PosixGroup $other_group): bool
27+
{
28+
if (!is_a($other_group, self::class)) {
29+
throw new Exception(
30+
"Unable to check equality because the parameter is not a " .
31+
self::class .
32+
" object",
33+
);
34+
}
35+
return $this->getDN() == $other_group->getDN();
36+
}
37+
38+
public function exists(): bool
39+
{
40+
return $this->entry->exists();
41+
}
42+
43+
public function getMemberUIDs(): array
44+
{
45+
$members = $this->entry->getAttribute("memberuid");
46+
sort($members);
47+
return $members;
48+
}
49+
50+
public function addMemberUID(string $uid): void
51+
{
52+
$this->entry->appendAttribute("memberuid", $uid);
53+
$this->entry->write();
54+
}
55+
56+
public function addMemberUIDs(array $uids): void
57+
{
58+
foreach ($uids as $uid) {
59+
$this->entry->appendAttribute("memberuid", $uid);
60+
}
61+
$this->entry->write();
62+
}
63+
64+
public function removeMemberUID(string $uid): void
65+
{
66+
$this->entry->removeAttributeEntryByValue("memberuid", $uid);
67+
$this->entry->write();
68+
}
69+
70+
public function removeMemberUIDs(array $uids): void
71+
{
72+
foreach ($uids as $uid) {
73+
$this->entry->removeAttributeEntryByValue("memberuid", $uid);
74+
}
75+
$this->entry->write();
76+
}
77+
78+
public function memberUIDExists(string $uid): bool
79+
{
80+
return in_array($uid, $this->getMemberUIDs());
81+
}
82+
}

‎resources/lib/UnityGroup.php‎

Lines changed: 8 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,10 @@
88
/**
99
* Class that represents a single PI group in the Unity Cluster.
1010
*/
11-
class UnityGroup
11+
class UnityGroup extends PosixGroup
1212
{
1313
public const string PI_PREFIX = "pi_";
14-
1514
public string $gid;
16-
private LDAPEntry $entry;
1715
private UnityLDAP $LDAP;
1816
private UnitySQL $SQL;
1917
private UnityMailer $MAILER;
@@ -26,42 +24,19 @@ public function __construct(
2624
UnityMailer $MAILER,
2725
UnityWebhook $WEBHOOK,
2826
) {
29-
$gid = trim($gid);
27+
parent::__construct($LDAP->getPIGroupEntry(trim($gid)));
3028
$this->gid = $gid;
31-
$this->entry = $LDAP->getPIGroupEntry($gid);
32-
3329
$this->LDAP = $LDAP;
3430
$this->SQL = $SQL;
3531
$this->MAILER = $MAILER;
3632
$this->WEBHOOK = $WEBHOOK;
3733
}
3834

39-
public function equals(UnityGroup $other_group): bool
40-
{
41-
if (!is_a($other_group, self::class)) {
42-
throw new Exception(
43-
"Unable to check equality because the parameter is not a " .
44-
self::class .
45-
" object",
46-
);
47-
}
48-
49-
return $this->gid == $other_group->gid;
50-
}
51-
5235
public function __toString(): string
5336
{
5437
return $this->gid;
5538
}
5639

57-
/**
58-
* Checks if the current PI is an approved and existent group
59-
*/
60-
public function exists(): bool
61-
{
62-
return $this->entry->exists();
63-
}
64-
6540
public function requestGroup(bool $send_mail_to_admins, bool $send_mail = true): void
6641
{
6742
if ($this->exists()) {
@@ -202,7 +177,7 @@ public function approveUser(UnityUser $new_user, bool $send_mail = true): void
202177
{
203178
$request = $this->SQL->getRequest($new_user->uid, $this->gid);
204179
\ensure($new_user->exists());
205-
$this->addUserToGroup($new_user);
180+
$this->addMemberUID($new_user->uid);
206181
$this->SQL->removeRequest($new_user->uid, $this->gid);
207182
if ($send_mail) {
208183
$this->MAILER->sendMail($new_user->getMail(), "group_user_added", [
@@ -240,14 +215,14 @@ public function denyUser(UnityUser $new_user, bool $send_mail = true): void
240215

241216
public function removeUser(UnityUser $new_user, bool $send_mail = true): void
242217
{
243-
if (!$this->memberExists($new_user)) {
218+
if (!$this->memberUIDExists($new_user->uid)) {
244219
return;
245220
}
246221
if ($new_user->uid == $this->getOwner()->uid) {
247222
throw new Exception("Cannot delete group owner from group. Disband group instead");
248223
}
249224
// remove request, this will fail silently if the request doesn't exist
250-
$this->removeUserFromGroup($new_user);
225+
$this->removeMemberUID($new_user->uid);
251226
if ($send_mail) {
252227
$this->MAILER->sendMail($new_user->getMail(), "group_user_removed", [
253228
"group" => $this->gid,
@@ -264,7 +239,7 @@ public function removeUser(UnityUser $new_user, bool $send_mail = true): void
264239

265240
public function newUserRequest(UnityUser $new_user, bool $send_mail = true): void
266241
{
267-
if ($this->memberExists($new_user)) {
242+
if ($this->memberUIDExists($new_user->uid)) {
268243
UnityHTTPD::errorLog("warning", "user '$new_user' already in group");
269244
return;
270245
}
@@ -310,7 +285,7 @@ public function getRequests(): array
310285

311286
public function getGroupMembers(): array
312287
{
313-
$members = $this->getGroupMemberUIDs();
288+
$members = $this->getMemberUIDs();
314289
$out = [];
315290
foreach ($members as $member) {
316291
$user_obj = new UnityUser(
@@ -325,13 +300,6 @@ public function getGroupMembers(): array
325300
return $out;
326301
}
327302

328-
public function getGroupMemberUIDs(): array
329-
{
330-
$members = $this->entry->getAttribute("memberuid");
331-
sort($members);
332-
return $members;
333-
}
334-
335303
public function requestExists(UnityUser $user): bool
336304
{
337305
$requesters = $this->getRequests();
@@ -358,23 +326,6 @@ private function init(): void
358326
// we need to update the cache here with the memberuid
359327
}
360328

361-
private function addUserToGroup(UnityUser $new_user): void
362-
{
363-
$this->entry->appendAttribute("memberuid", $new_user->uid);
364-
$this->entry->write();
365-
}
366-
367-
private function removeUserFromGroup(UnityUser $old_user): void
368-
{
369-
$this->entry->removeAttributeEntryByValue("memberuid", $old_user->uid);
370-
$this->entry->write();
371-
}
372-
373-
public function memberExists(UnityUser $user): bool
374-
{
375-
return in_array($user->uid, $this->getGroupMemberUIDs());
376-
}
377-
378329
private function addRequest(string $uid): void
379330
{
380331
$this->SQL->addRequest($uid, $this->gid);
@@ -418,7 +369,7 @@ public static function ownerMail2GID(string $email): string
418369
public function getGroupMembersAttributes(array $attributes, array $default_values = []): array
419370
{
420371
return $this->LDAP->getUsersAttributes(
421-
$this->getGroupMemberUIDs(),
372+
$this->getMemberUIDs(),
422373
$attributes,
423374
$default_values,
424375
);

‎resources/lib/UnityOrg.php‎

Lines changed: 8 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,9 @@
33
namespace UnityWebPortal\lib;
44
use PHPOpenLDAPer\LDAPEntry;
55

6-
class UnityOrg
6+
class UnityOrg extends PosixGroup
77
{
88
public string $gid;
9-
private LDAPEntry $entry;
109
private UnityLDAP $LDAP;
1110
private UnitySQL $SQL;
1211
private UnityMailer $MAILER;
@@ -19,16 +18,19 @@ public function __construct(
1918
UnityMailer $MAILER,
2019
UnityWebhook $WEBHOOK,
2120
) {
22-
$gid = trim($gid);
21+
parent::__construct($LDAP->getOrgGroupEntry(trim($gid)));
2322
$this->gid = $gid;
24-
$this->entry = $LDAP->getOrgGroupEntry($this->gid);
25-
2623
$this->LDAP = $LDAP;
2724
$this->SQL = $SQL;
2825
$this->MAILER = $MAILER;
2926
$this->WEBHOOK = $WEBHOOK;
3027
}
3128

29+
public function __toString(): string
30+
{
31+
return $this->gid;
32+
}
33+
3234
public function init(): void
3335
{
3436
\ensure(!$this->entry->exists());
@@ -38,19 +40,9 @@ public function init(): void
3840
$this->entry->write();
3941
}
4042

41-
public function exists(): bool
42-
{
43-
return $this->entry->exists();
44-
}
45-
46-
public function inOrg(UnityUser $user): bool
47-
{
48-
return in_array($user->uid, $this->getOrgMemberUIDs());
49-
}
50-
5143
public function getOrgMembers(): array
5244
{
53-
$members = $this->getOrgMemberUIDs();
45+
$members = $this->getMemberUIDs();
5446
$out = [];
5547
foreach ($members as $member) {
5648
$user_obj = new UnityUser(
@@ -64,23 +56,4 @@ public function getOrgMembers(): array
6456
}
6557
return $out;
6658
}
67-
68-
public function getOrgMemberUIDs(): array
69-
{
70-
$members = $this->entry->getAttribute("memberuid");
71-
sort($members);
72-
return $members;
73-
}
74-
75-
public function addUser(UnityUser $user): void
76-
{
77-
$this->entry->appendAttribute("memberuid", $user->uid);
78-
$this->entry->write();
79-
}
80-
81-
public function removeUser(UnityUser $user): void
82-
{
83-
$this->entry->removeAttributeEntryByValue("memberuid", $user->uid);
84-
$this->entry->write();
85-
}
8659
}

‎resources/lib/UnityUser.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,8 @@ public function init(
9090
$org->init();
9191
}
9292

93-
if (!$org->inOrg($this)) {
94-
$org->addUser($this);
93+
if (!$org->memberUIDExists($this->uid)) {
94+
$org->addMemberUID($this->uid);
9595
}
9696

9797
$this->SQL->addLog($this->uid, $_SERVER["REMOTE_ADDR"], "user_added", $this->uid);
@@ -416,6 +416,6 @@ public function isInGroup(string $uid, UnityGroup $group): bool
416416
$group_checked = $group;
417417
}
418418

419-
return in_array($uid, $group_checked->getGroupMemberUIDs());
419+
return in_array($uid, $group_checked->getMemberUIDs());
420420
}
421421
}

‎test/functional/AccountDeletionRequestTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public function testRequestAccountDeletionUserHasRequest()
4949
switchUser(...$pi_args);
5050
$pi = $USER;
5151
$pi_group = $USER->getPIGroup();
52-
$this->assertEqualsCanonicalizing([$pi->uid], $pi_group->getGroupMemberUIDs());
52+
$this->assertEqualsCanonicalizing([$pi->uid], $pi_group->getMemberUIDs());
5353
$user_args = getBlankUser();
5454
switchUser(...$user_args);
5555
$this->assertEmpty($USER->getPIGroupGIDs());

‎test/functional/PIMemberRequestTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public function testRequestMembership()
3838
$uid = $USER->uid;
3939
$this->assertFalse($USER->isPI());
4040
$this->assertFalse($SQL->requestExists($uid, UnitySQL::REQUEST_BECOME_PI));
41-
$this->assertFalse($pi_group->memberExists($USER));
41+
$this->assertFalse($pi_group->memberUIDExists($USER->uid));
4242
try {
4343
$this->requestMembership($gid);
4444
$this->assertTrue($SQL->requestExists($uid, $gid));

0 commit comments

Comments
 (0)