Clarify comments, make code more readable as per review suggestions

SilMock/Google/Service/Directory/Resource/Groups.php

@@ -25,13 +25,8 @@ public function delete(string $groupKey)
             $keysToCheck[] = $groupRecordData['email'];
             if (in_array($groupKey, $keysToCheck)) {
                 $this->deleteRecordById($groupRecord['id']);
-                $mockGroupsAliasesObject = new GroupsAliases($this->dbFile);
-                foreach ($mockGroupsAliasesObject->getRecords() as $aliasRecord) {
-                    $aliasRecordData = json_decode($aliasRecord['data'], true);
-                    if ($aliasRecordData['primaryEmail'] === $groupRecordData['email']) {
-                        $mockGroupsAliasesObject->deleteRecordById($aliasRecord['id']);
-                    }
-                }
+                $groupAliasesObject = new GroupsAliases($this->dbFile);
+                $groupAliasesObject->deletedByGroup($groupRecordData['email']);
             }
         }
     }

SilMock/Google/Service/Directory/Resource/GroupsAliases.php

@@ -15,6 +15,16 @@ public function __construct(?string $dbFile = null)
         parent::__construct($dbFile, 'directory', 'groupsaliases');
     }
 
+    public function deletedByGroup(string $groupKey): void
+    {
+        foreach ($this->getRecords() as $aliasRecord) {
+            $aliasRecordData = json_decode($aliasRecord['data'], true);
+            if ($aliasRecordData['primaryEmail'] === $groupKey) {
+                $this->deleteRecordById($aliasRecord['id']);
+            }
+        }
+    }
+
     public function delete(string $groupKey, string $alias, array $optParams = [])
     {
         $groupAliasRecords = $this->getRecords();

SilMock/tests/Google/Service/Directory/Resource/GroupsTest.php

@@ -17,28 +17,25 @@ class GroupsTest extends TestCase
     public const GROUP_EMAIL_ADDRESS = '[email protected]';
     public const GROUP_ALIAS_ADDRESS = '[email protected]';
 
-    public function testInitialSetup()
+    protected function deleteGroupAndAliasesIfTheyExists(string $groupName)
     {
         $mockGoogleServiceDirectory = new GoogleMock_Directory('anyclient', $this->dataFile);
-        $group = $mockGoogleServiceDirectory->groups->get(self::GROUP_EMAIL_ADDRESS);
-        if ($group !== null) {
-            $mockGoogleServiceDirectory->groups->delete(self::GROUP_EMAIL_ADDRESS);
-        }
-        $group = $mockGoogleServiceDirectory->groups->get(self::GROUP_EMAIL_ADDRESS);
-        self::assertNull($group, "Failed to clean up previous insert tests.");
-        $group = $mockGoogleServiceDirectory->groups->get(self::GROUP_EMAIL_ADDRESS . 'update');
+        $group = $mockGoogleServiceDirectory->groups->get($groupName);
         if ($group !== null) {
-            $mockGoogleServiceDirectory->groups->delete(self::GROUP_EMAIL_ADDRESS . 'update');
+            $mockGoogleServiceDirectory->groups->delete($groupName);
         }
-        $group = $mockGoogleServiceDirectory->groups->get(self::GROUP_EMAIL_ADDRESS . 'update');
-        self::assertNull($group, "Failed to clean up previous update tests.");
+        $group = $mockGoogleServiceDirectory->groups->get($groupName);
+        self::assertNull($group, "Failed to clean up previous tests.");
     }
 
     public function testInsert()
     {
+        $this->deleteGroupAndAliasesIfTheyExists(self::GROUP_EMAIL_ADDRESS);
         $group = new GoogleDirectory_Group();
         $group->setEmail(self::GROUP_EMAIL_ADDRESS);
-        $group->setAliases([self::GROUP_ALIAS_ADDRESS]); // read-only, should not save anything
+        // See https://developers.google.com/admin-sdk/directory/reference/rest/v1/groups#Group
+        // setting the aliases will not add, remove, or change aliases at all.
+        $group->setAliases([self::GROUP_ALIAS_ADDRESS]);
         $group->setName('Sample Group');
         $group->setDescription('A Sample Group used for testing');
 
@@ -59,9 +56,12 @@ public function testInsert()
 
     public function testUpdate()
     {
+        $this->deleteGroupAndAliasesIfTheyExists(self::GROUP_EMAIL_ADDRESS . 'update');
         $group = new GoogleDirectory_Group();
         $group->setEmail(self::GROUP_EMAIL_ADDRESS . 'update');
-        $group->setAliases([self::GROUP_ALIAS_ADDRESS . 'update']); // this shouldn't change aliases.
+        // See https://developers.google.com/admin-sdk/directory/reference/rest/v1/groups#Group
+        // setting the aliases will not add, remove, or change aliases at all.
+        $group->setAliases([self::GROUP_ALIAS_ADDRESS . 'update']);
         $group->setName('Sample Group Update');
         $group->setDescription('A Sample Group used for testing update');
 
@@ -79,19 +79,22 @@ public function testUpdate()
         self::assertTrue($addedGroup instanceof GoogleDirectory_Group);
         self::assertEmpty($addedGroup->getAliases(), "Expecting no group aliases inserted by group.insert");
 
-        // Google group does not update aliases, but for coding simplicity the mock object does.
-        $group->setAliases([self::GROUP_ALIAS_ADDRESS . 'update-change']); // this shouldn't change aliases.
+        // See https://developers.google.com/admin-sdk/directory/reference/rest/v1/groups#Group
+        // setting the aliases will not add, remove, or change aliases at all.
+        $group->setAliases([self::GROUP_ALIAS_ADDRESS . 'update-change']);
         $updatedGroup = $mockGoogleServiceDirectory->groups->update($group->getEmail(), $group);
         self::assertTrue($updatedGroup instanceof GoogleDirectory_Group);
-        self::assertEmpty($updatedGroup->getAliases()); // this should confirm aliases are unset
+        self::assertEmpty($updatedGroup->getAliases(),  "Expecting no group aliases changed by group.update");
     }
 
     protected function deleteTestSetup()
     {
         // Set update a deletable email address
         $group = new GoogleDirectory_Group();
         $group->setEmail(self::GROUP_EMAIL_ADDRESS . 'delete');
-        $group->setAliases([self::GROUP_ALIAS_ADDRESS . 'delete']); // read-only property, should have no effect
+        // See https://developers.google.com/admin-sdk/directory/reference/rest/v1/groups#Group
+        // setting the aliases will not add, remove, or change aliases at all.
+        $group->setAliases([self::GROUP_ALIAS_ADDRESS . 'delete']);
         $group->setName('Sample Deletable Group');
         $group->setDescription('A Sample Deletable Group used for testing');