remove duplicated code from UnityGroup/UnityOrg

resources/autoload.php

@@ -14,6 +14,7 @@
 // load libs
 require_once __DIR__ . "/lib/UnityLDAP.php";
 require_once __DIR__ . "/lib/UnityUser.php";
+require_once __DIR__ . "/lib/PosixGroup.php";
 require_once __DIR__ . "/lib/UnityGroup.php";
 require_once __DIR__ . "/lib/UnityOrg.php";
 require_once __DIR__ . "/lib/UnitySQL.php";

resources/lib/PosixGroup.php

@@ -0,0 +1,82 @@
+<?php
+
+namespace UnityWebPortal\lib;
+
+use PHPOpenLDAPer\LDAPEntry;
+use \Exception;
+
+/*
+does not extend LDAPEntry because UnityGroup extends this and I don't want UnityGroup
+to extend LDAPEntry because the functions from LDAPEntry should not be exposed there
+*/
+class PosixGroup
+{
+    protected LDAPEntry $entry;
+
+    public function __construct(LDAPEntry $entry)
+    {
+        $this->entry = $entry;
+    }
+
+    public function getDN(): string
+    {
+        return $this->entry->getDN();
+    }
+
+    public function equals(PosixGroup $other_group): bool
+    {
+        if (!is_a($other_group, self::class)) {
+            throw new Exception(
+                "Unable to check equality because the parameter is not a " .
+                    self::class .
+                    " object",
+            );
+        }
+        return $this->getDN() == $other_group->getDN();
+    }
+
+    public function exists(): bool
+    {
+        return $this->entry->exists();
+    }
+
+    public function getMemberUIDs(): array
+    {
+        $members = $this->entry->getAttribute("memberuid");
+        sort($members);
+        return $members;
+    }
+
+    public function addMemberUID(string $uid): void
+    {
+        $this->entry->appendAttribute("memberuid", $uid);
+        $this->entry->write();
+    }
+
+    public function addMemberUIDs(array $uids): void
+    {
+        foreach ($uids as $uid) {
+            $this->entry->appendAttribute("memberuid", $uid);
+        }
+        $this->entry->write();
+    }
+
+    public function removeMemberUID(string $uid): void
+    {
+        $this->entry->removeAttributeEntryByValue("memberuid", $uid);
+        $this->entry->write();
+    }
+
+    public function removeMemberUIDs(array $uids): void
+    {
+        foreach ($uids as $uid) {
+            $this->entry->removeAttributeEntryByValue("memberuid", $uid);
+        }
+        $this->entry->write();
+    }
+
+    public function memberUIDExists(string $uid): bool
+    {
+        return in_array($uid, $this->getMemberUIDs());
+    }
+}

resources/lib/UnityGroup.php

@@ -8,12 +8,10 @@
 /**
  * Class that represents a single PI group in the Unity Cluster.
  */
-class UnityGroup
+class UnityGroup extends PosixGroup
 {
     public const string PI_PREFIX = "pi_";
-
     public string $gid;
-    private LDAPEntry $entry;
     private UnityLDAP $LDAP;
     private UnitySQL $SQL;
     private UnityMailer $MAILER;
@@ -26,42 +24,19 @@ public function __construct(
         UnityMailer $MAILER,
         UnityWebhook $WEBHOOK,
     ) {
-        $gid = trim($gid);
+        parent::__construct($LDAP->getPIGroupEntry(trim($gid)));
         $this->gid = $gid;
-        $this->entry = $LDAP->getPIGroupEntry($gid);
-
         $this->LDAP = $LDAP;
         $this->SQL = $SQL;
         $this->MAILER = $MAILER;
         $this->WEBHOOK = $WEBHOOK;
     }
 
-    public function equals(UnityGroup $other_group): bool
-    {
-        if (!is_a($other_group, self::class)) {
-            throw new Exception(
-                "Unable to check equality because the parameter is not a " .
-                    self::class .
-                    " object",
-            );
-        }
-
-        return $this->gid == $other_group->gid;
-    }
-
     public function __toString(): string
     {
         return $this->gid;
     }
 
-    /**
-     * Checks if the current PI is an approved and existent group
-     */
-    public function exists(): bool
-    {
-        return $this->entry->exists();
-    }
-
     public function requestGroup(bool $send_mail_to_admins, bool $send_mail = true): void
     {
         if ($this->exists()) {
@@ -202,7 +177,7 @@ public function approveUser(UnityUser $new_user, bool $send_mail = true): void
     {
         $request = $this->SQL->getRequest($new_user->uid, $this->gid);
         \ensure($new_user->exists());
-        $this->addUserToGroup($new_user);
+        $this->addMemberUID($new_user->uid);
         $this->SQL->removeRequest($new_user->uid, $this->gid);
         if ($send_mail) {
             $this->MAILER->sendMail($new_user->getMail(), "group_user_added", [
@@ -240,14 +215,14 @@ public function denyUser(UnityUser $new_user, bool $send_mail = true): void
 
     public function removeUser(UnityUser $new_user, bool $send_mail = true): void
     {
-        if (!$this->memberExists($new_user)) {
+        if (!$this->memberUIDExists($new_user->uid)) {
             return;
         }
         if ($new_user->uid == $this->getOwner()->uid) {
             throw new Exception("Cannot delete group owner from group. Disband group instead");
         }
         // remove request, this will fail silently if the request doesn't exist
-        $this->removeUserFromGroup($new_user);
+        $this->removeMemberUID($new_user->uid);
         if ($send_mail) {
             $this->MAILER->sendMail($new_user->getMail(), "group_user_removed", [
                 "group" => $this->gid,
@@ -264,7 +239,7 @@ public function removeUser(UnityUser $new_user, bool $send_mail = true): void
 
     public function newUserRequest(UnityUser $new_user, bool $send_mail = true): void
     {
-        if ($this->memberExists($new_user)) {
+        if ($this->memberUIDExists($new_user->uid)) {
             UnityHTTPD::errorLog("warning", "user '$new_user' already in group");
             return;
         }
@@ -310,7 +285,7 @@ public function getRequests(): array
 
     public function getGroupMembers(): array
     {
-        $members = $this->getGroupMemberUIDs();
+        $members = $this->getMemberUIDs();
         $out = [];
         foreach ($members as $member) {
             $user_obj = new UnityUser(
@@ -325,13 +300,6 @@ public function getGroupMembers(): array
         return $out;
     }
 
-    public function getGroupMemberUIDs(): array
-    {
-        $members = $this->entry->getAttribute("memberuid");
-        sort($members);
-        return $members;
-    }
-
     public function requestExists(UnityUser $user): bool
     {
         $requesters = $this->getRequests();
@@ -358,23 +326,6 @@ private function init(): void
         // we need to update the cache here with the memberuid
     }
 
-    private function addUserToGroup(UnityUser $new_user): void
-    {
-        $this->entry->appendAttribute("memberuid", $new_user->uid);
-        $this->entry->write();
-    }
-
-    private function removeUserFromGroup(UnityUser $old_user): void
-    {
-        $this->entry->removeAttributeEntryByValue("memberuid", $old_user->uid);
-        $this->entry->write();
-    }
-
-    public function memberExists(UnityUser $user): bool
-    {
-        return in_array($user->uid, $this->getGroupMemberUIDs());
-    }
-
     private function addRequest(string $uid): void
     {
         $this->SQL->addRequest($uid, $this->gid);
@@ -418,7 +369,7 @@ public static function ownerMail2GID(string $email): string
     public function getGroupMembersAttributes(array $attributes, array $default_values = []): array
     {
         return $this->LDAP->getUsersAttributes(
-            $this->getGroupMemberUIDs(),
+            $this->getMemberUIDs(),
             $attributes,
             $default_values,
         );

resources/lib/UnityOrg.php

@@ -3,10 +3,9 @@
 namespace UnityWebPortal\lib;
 use PHPOpenLDAPer\LDAPEntry;
 
-class UnityOrg
+class UnityOrg extends PosixGroup
 {
     public string $gid;
-    private LDAPEntry $entry;
     private UnityLDAP $LDAP;
     private UnitySQL $SQL;
     private UnityMailer $MAILER;
@@ -19,16 +18,19 @@ public function __construct(
         UnityMailer $MAILER,
         UnityWebhook $WEBHOOK,
     ) {
-        $gid = trim($gid);
+        parent::__construct($LDAP->getOrgGroupEntry(trim($gid)));
         $this->gid = $gid;
-        $this->entry = $LDAP->getOrgGroupEntry($this->gid);
-
         $this->LDAP = $LDAP;
         $this->SQL = $SQL;
         $this->MAILER = $MAILER;
         $this->WEBHOOK = $WEBHOOK;
     }
 
+    public function __toString(): string
+    {
+        return $this->gid;
+    }
+
     public function init(): void
     {
         \ensure(!$this->entry->exists());
@@ -38,19 +40,9 @@ public function init(): void
         $this->entry->write();
     }
 
-    public function exists(): bool
-    {
-        return $this->entry->exists();
-    }
-
-    public function inOrg(UnityUser $user): bool
-    {
-        return in_array($user->uid, $this->getOrgMemberUIDs());
-    }
-
     public function getOrgMembers(): array
     {
-        $members = $this->getOrgMemberUIDs();
+        $members = $this->getMemberUIDs();
         $out = [];
         foreach ($members as $member) {
             $user_obj = new UnityUser(
@@ -64,23 +56,4 @@ public function getOrgMembers(): array
         }
         return $out;
     }
-
-    public function getOrgMemberUIDs(): array
-    {
-        $members = $this->entry->getAttribute("memberuid");
-        sort($members);
-        return $members;
-    }
-
-    public function addUser(UnityUser $user): void
-    {
-        $this->entry->appendAttribute("memberuid", $user->uid);
-        $this->entry->write();
-    }
-
-    public function removeUser(UnityUser $user): void
-    {
-        $this->entry->removeAttributeEntryByValue("memberuid", $user->uid);
-        $this->entry->write();
-    }
 }

resources/lib/UnityUser.php

@@ -90,8 +90,8 @@ public function init(
             $org->init();
         }
 
-        if (!$org->inOrg($this)) {
-            $org->addUser($this);
+        if (!$org->memberUIDExists($this->uid)) {
+            $org->addMemberUID($this->uid);
         }
 
         $this->SQL->addLog($this->uid, $_SERVER["REMOTE_ADDR"], "user_added", $this->uid);
@@ -416,6 +416,6 @@ public function isInGroup(string $uid, UnityGroup $group): bool
             $group_checked = $group;
         }
 
-        return in_array($uid, $group_checked->getGroupMemberUIDs());
+        return in_array($uid, $group_checked->getMemberUIDs());
     }
 }

test/functional/AccountDeletionRequestTest.php

@@ -49,7 +49,7 @@ public function testRequestAccountDeletionUserHasRequest()
         switchUser(...$pi_args);
         $pi = $USER;
         $pi_group = $USER->getPIGroup();
-        $this->assertEqualsCanonicalizing([$pi->uid], $pi_group->getGroupMemberUIDs());
+        $this->assertEqualsCanonicalizing([$pi->uid], $pi_group->getMemberUIDs());
         $user_args = getBlankUser();
         switchUser(...$user_args);
         $this->assertEmpty($USER->getPIGroupGIDs());

test/functional/PIMemberRequestTest.php

@@ -38,7 +38,7 @@ public function testRequestMembership()
         $uid = $USER->uid;
         $this->assertFalse($USER->isPI());
         $this->assertFalse($SQL->requestExists($uid, UnitySQL::REQUEST_BECOME_PI));
-        $this->assertFalse($pi_group->memberExists($USER));
+        $this->assertFalse($pi_group->memberUIDExists($USER->uid));
         try {
             $this->requestMembership($gid);
             $this->assertTrue($SQL->requestExists($uid, $gid));