From 2ae8ee7f76da4c0606b421479c7a2b1562d2594b Mon Sep 17 00:00:00 2001 From: Mark Tompsett Date: Tue, 18 Jun 2024 12:16:41 -0400 Subject: [PATCH 1/6] Add query filtering on roles --- .../Service/Directory/Resource/Members.php | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/SilMock/Google/Service/Directory/Resource/Members.php b/SilMock/Google/Service/Directory/Resource/Members.php index 6d22472..1c1cab0 100644 --- a/SilMock/Google/Service/Directory/Resource/Members.php +++ b/SilMock/Google/Service/Directory/Resource/Members.php @@ -69,19 +69,19 @@ public function listMembers($groupKey, $optParams = []) $this->validateGroupExists($groupKey); $pageSize = $optParams['pageSize'] ?? 10; $pageToken = $optParams['pageToken'] ?? 0; + $query = $optParams['query'] ?? null; + $expectedRole = $query ? substr($query, strpos($query, '=') + 1) : null; $members = new GoogleDirectory_Members(); $directoryMemberRecords = $this->getRecords(); $memberCounter = 0; foreach ($directoryMemberRecords as $memberRecord) { $memberData = json_decode($memberRecord['data'], true); if ($memberData['groupKey'] === $groupKey) { - $memberCounter = $memberCounter + 1; if ($memberCounter >= ($pageToken * $pageSize)) { - $currentMembers = $members->getMembers(); - $currentMember = new GoogleDirectory_Member(); - ObjectUtils::initialize($currentMember, $memberData['member']); - $currentMembers[] = $currentMember; - $members->setMembers($currentMembers); + if (is_null($expectedRole) || $expectedRole === $memberData['member']['role']) { + $memberCounter = $memberCounter + 1; + $this->addToMembers($memberData, $members); + } } } $currentMembers = $members->getMembers(); @@ -98,6 +98,15 @@ public function listMembers($groupKey, $optParams = []) return $members; } + protected function addToMembers(array $memberData, GoogleDirectory_Members $members): void + { + $currentMembers = $members->getMembers(); + $currentMember = new GoogleDirectory_Member(); + ObjectUtils::initialize($currentMember, $memberData['member']); + $currentMembers[] = $currentMember; + $members->setMembers($currentMembers); + } + protected function validateGroupExists(string $groupKey): void { $mockGroupsObject = new Groups($this->dbFile); From 9b90a2597d49ddfdc178f3c81b273f10a4082526 Mon Sep 17 00:00:00 2001 From: Mark Tompsett Date: Tue, 18 Jun 2024 12:17:15 -0400 Subject: [PATCH 2/6] Add tests to confirm that MEMBER and not OWNER members exist --- .../Directory/Resource/MembersTest.php | 59 ++++++++++++++++++- 1 file changed, 56 insertions(+), 3 deletions(-) diff --git a/SilMock/tests/Google/Service/Directory/Resource/MembersTest.php b/SilMock/tests/Google/Service/Directory/Resource/MembersTest.php index a82c0ca..2c9e824 100644 --- a/SilMock/tests/Google/Service/Directory/Resource/MembersTest.php +++ b/SilMock/tests/Google/Service/Directory/Resource/MembersTest.php @@ -21,6 +21,7 @@ public function testInsert() $member = new GoogleDirectory_Member(); $member->setEmail($emailAddress); + $member->setRole('MEMBER'); $mockGoogleServiceDirectory = new GoogleMock_Directory('anyclient', $this->dataFile); try { @@ -72,7 +73,7 @@ public function testHasMember() self::assertTrue($hasMember); } - public function testListMembers() + public function testListMembersAll() { $groupEmailAddress = 'sample_group@groups.example.com'; $mockGoogleServiceDirectory = new GoogleMock_Directory('anyclient', $this->dataFile); @@ -88,8 +89,60 @@ public function testListMembers() ); } self::assertNotEmpty( - $members, - 'Was expecting the members.list method to have at least one member.' + $members->getMembers(), + 'Was expecting the members.list method to have at least one member entry.' + ); + } + + public function testListMembersOnlyMember() + { + $groupEmailAddress = 'sample_group@groups.example.com'; + $mockGoogleServiceDirectory = new GoogleMock_Directory('anyclient', $this->dataFile); + $members = []; + try { + $members = $mockGoogleServiceDirectory->members->listMembers( + $groupEmailAddress, + [ + 'query' => 'roles=MEMBER' + ] + ); + } catch (Exception $exception) { + self::fail( + sprintf( + 'Was expecting the members.list method to function, but got: %s', + $exception->getMessage() + ) + ); + } + self::assertNotEmpty( + $members->getMembers(), + 'Was expecting the members.list method to have at least one member type entry.' + ); + } + + public function testListMembersOnlyOwner() + { + $groupEmailAddress = 'sample_group@groups.example.com'; + $mockGoogleServiceDirectory = new GoogleMock_Directory('anyclient', $this->dataFile); + $members = []; + try { + $members = $mockGoogleServiceDirectory->members->listMembers( + $groupEmailAddress, + [ + 'query' => 'roles=OWNER' + ] + ); + } catch (Exception $exception) { + self::fail( + sprintf( + 'Was expecting the members.list method to function, but got: %s', + $exception->getMessage() + ) + ); + } + self::assertEmpty( + $members->getMembers(), + 'Was expecting the members.list method to have no owner types.' ); } } From b2a96e0c59ca62ad1a2303a38afa2f6b05b4a41d Mon Sep 17 00:00:00 2001 From: Mark Tompsett Date: Tue, 18 Jun 2024 19:13:58 -0400 Subject: [PATCH 3/6] Implement even more clear roles extraction --- .../Service/Directory/Resource/Members.php | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/SilMock/Google/Service/Directory/Resource/Members.php b/SilMock/Google/Service/Directory/Resource/Members.php index 1c1cab0..577e374 100644 --- a/SilMock/Google/Service/Directory/Resource/Members.php +++ b/SilMock/Google/Service/Directory/Resource/Members.php @@ -70,7 +70,7 @@ public function listMembers($groupKey, $optParams = []) $pageSize = $optParams['pageSize'] ?? 10; $pageToken = $optParams['pageToken'] ?? 0; $query = $optParams['query'] ?? null; - $expectedRole = $query ? substr($query, strpos($query, '=') + 1) : null; + $expectedRoles = $this->extractRoles($query); $members = new GoogleDirectory_Members(); $directoryMemberRecords = $this->getRecords(); $memberCounter = 0; @@ -78,7 +78,7 @@ public function listMembers($groupKey, $optParams = []) $memberData = json_decode($memberRecord['data'], true); if ($memberData['groupKey'] === $groupKey) { if ($memberCounter >= ($pageToken * $pageSize)) { - if (is_null($expectedRole) || $expectedRole === $memberData['member']['role']) { + if (empty($expectedRoles) || in_array($memberData['member']['role'], $expectedRoles)) { $memberCounter = $memberCounter + 1; $this->addToMembers($memberData, $members); } @@ -98,6 +98,23 @@ public function listMembers($groupKey, $optParams = []) return $members; } + protected function extractRoles(?string $query): array + { + if (! empty($query) && str_contains($query, 'roles')) { + $roleSegmentStart = substr($query, strpos($query, 'roles')); + $roleSegmentEnd = strrpos($roleSegmentStart, ' '); + if ($roleSegmentEnd === false) { + $roleSegmentEnd = strlen($roleSegmentStart); + } + $roleSegment = trim(substr($roleSegmentStart, 0, $roleSegmentEnd)); + $roleValue = substr($roleSegment, 6); // roles= is 0-5 + $expectedRoles = explode(',', $roleValue); + } else { + $expectedRoles = []; + } + return $expectedRoles; + } + protected function addToMembers(array $memberData, GoogleDirectory_Members $members): void { $currentMembers = $members->getMembers(); From d637e6468137ea952b2188d4388bb93dd1d956c2 Mon Sep 17 00:00:00 2001 From: Mark Tompsett Date: Tue, 18 Jun 2024 19:21:51 -0400 Subject: [PATCH 4/6] Address a sonar issue --- SilMock/Google/Service/Directory/Resource/Members.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/SilMock/Google/Service/Directory/Resource/Members.php b/SilMock/Google/Service/Directory/Resource/Members.php index 577e374..f0b8ddb 100644 --- a/SilMock/Google/Service/Directory/Resource/Members.php +++ b/SilMock/Google/Service/Directory/Resource/Members.php @@ -76,13 +76,13 @@ public function listMembers($groupKey, $optParams = []) $memberCounter = 0; foreach ($directoryMemberRecords as $memberRecord) { $memberData = json_decode($memberRecord['data'], true); - if ($memberData['groupKey'] === $groupKey) { - if ($memberCounter >= ($pageToken * $pageSize)) { - if (empty($expectedRoles) || in_array($memberData['member']['role'], $expectedRoles)) { + if ( + $memberData['groupKey'] === $groupKey // Matches the expected group + && $memberCounter >= ($pageToken * $pageSize) // Matches the subsection of all the members + && (empty($expectedRoles) || in_array($memberData['member']['role'], $expectedRoles)) // Matches role + ) { $memberCounter = $memberCounter + 1; $this->addToMembers($memberData, $members); - } - } } $currentMembers = $members->getMembers(); $currentResultSize = count($currentMembers); From a5ae1182cb412fb5cd747ce02e0bb06777e00644 Mon Sep 17 00:00:00 2001 From: Mark Tompsett Date: Tue, 18 Jun 2024 19:26:29 -0400 Subject: [PATCH 5/6] Address another sonar issue --- .../Directory/Resource/MembersTest.php | 31 ++++++++----------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/SilMock/tests/Google/Service/Directory/Resource/MembersTest.php b/SilMock/tests/Google/Service/Directory/Resource/MembersTest.php index 2c9e824..e43cc4f 100644 --- a/SilMock/tests/Google/Service/Directory/Resource/MembersTest.php +++ b/SilMock/tests/Google/Service/Directory/Resource/MembersTest.php @@ -81,12 +81,7 @@ public function testListMembersAll() try { $members = $mockGoogleServiceDirectory->members->listMembers($groupEmailAddress); } catch (Exception $exception) { - self::fail( - sprintf( - 'Was expecting the members.list method to function, but got: %s', - $exception->getMessage() - ) - ); + $this->failure($exception); } self::assertNotEmpty( $members->getMembers(), @@ -107,12 +102,7 @@ public function testListMembersOnlyMember() ] ); } catch (Exception $exception) { - self::fail( - sprintf( - 'Was expecting the members.list method to function, but got: %s', - $exception->getMessage() - ) - ); + $this->failure($exception); } self::assertNotEmpty( $members->getMembers(), @@ -133,16 +123,21 @@ public function testListMembersOnlyOwner() ] ); } catch (Exception $exception) { - self::fail( - sprintf( - 'Was expecting the members.list method to function, but got: %s', - $exception->getMessage() - ) - ); + $this->failure($exception); } self::assertEmpty( $members->getMembers(), 'Was expecting the members.list method to have no owner types.' ); } + + protected function failure(Exception $exception): void + { + self::fail( + sprintf( + 'Was expecting the members.insert method to function, but got: %s', + $exception->getMessage() + ) + ); + } } From 7da953bdbb4a330eac07c22acd00f210833d8955 Mon Sep 17 00:00:00 2001 From: Mark Tompsett Date: Tue, 18 Jun 2024 19:46:45 -0400 Subject: [PATCH 6/6] Address some more sonar annoyances --- SilMock/Google/Service/Directory/Tokens.php | 2 +- .../Service/Directory/UsersAliasesResource.php | 8 +++++--- .../Google/Service/Directory/UsersResource.php | 17 ++++++++++------- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/SilMock/Google/Service/Directory/Tokens.php b/SilMock/Google/Service/Directory/Tokens.php index 7a6ebb1..b3cd1ea 100644 --- a/SilMock/Google/Service/Directory/Tokens.php +++ b/SilMock/Google/Service/Directory/Tokens.php @@ -50,7 +50,7 @@ protected function assertIsValidUserKey(string $userId) */ protected function isValidEmailAddress(string $email): bool { - return (filter_var($email, FILTER_VALIDATE_EMAIL) !== false); + return filter_var($email, FILTER_VALIDATE_EMAIL) !== false; } protected function listTokensFor(string $userId): array diff --git a/SilMock/Google/Service/Directory/UsersAliasesResource.php b/SilMock/Google/Service/Directory/UsersAliasesResource.php index d7faf8e..6168749 100644 --- a/SilMock/Google/Service/Directory/UsersAliasesResource.php +++ b/SilMock/Google/Service/Directory/UsersAliasesResource.php @@ -11,6 +11,8 @@ class UsersAliasesResource extends DbClass { + public const ACCOUNT_DOESNT_EXIST = "Account doesn't exist: "; + public function __construct(?string $dbFile = null) { parent::__construct($dbFile, 'directory', 'users_alias'); @@ -37,7 +39,7 @@ public function delete($userKey, $alias) $matchingUsers = $dir->users->get($userKey); if ($matchingUsers === null) { - throw new Exception("Account doesn't exist: " . $userKey, 201407101645); + throw new Exception(self::ACCOUNT_DOESNT_EXIST . $userKey, 201407101645); } // Get all the aliases for that user @@ -87,7 +89,7 @@ public function insert($userKey, $postBody) $matchingUsers = $dir->users->get($userKey); if ($matchingUsers === null) { - throw new Exception("Account doesn't exist: " . $userKey, 201407110830); + throw new Exception(self::ACCOUNT_DOESNT_EXIST . $userKey, 201407110830); } if ($postBody->$key === null) { @@ -145,7 +147,7 @@ public function listUsersAliases($userKey): ?Google_Service_Directory_Aliases $matchingUsers = $dir->users->get($userKey); if ($matchingUsers === null) { - throw new Exception("Account doesn't exist: " . $userKey, 201407101420); + throw new Exception(self::ACCOUNT_DOESNT_EXIST . $userKey, 201407101420); } $foundAliases = $this->fetchAliasesByUser($key, $userKey); diff --git a/SilMock/Google/Service/Directory/UsersResource.php b/SilMock/Google/Service/Directory/UsersResource.php index a6bc211..7fb787e 100644 --- a/SilMock/Google/Service/Directory/UsersResource.php +++ b/SilMock/Google/Service/Directory/UsersResource.php @@ -112,18 +112,20 @@ protected function getDbUserByAlias($userKey) } $allUsers = $this->getAllDbUsers(); - - foreach ($allUsers as $aUser) { - if (! isset($aUser['data'])) { - continue; + $usersWithData = array_filter( + $allUsers, + function ($user) { + return isset($user['data']); } - + ); + + foreach ($usersWithData as $aUser) { $userData = json_decode($aUser['data'], true); if ($userData === null) { continue; } - $primaryEmail = isset($userData['primaryEmail']) ? $userData['primaryEmail'] : null; + $primaryEmail = $userData['primaryEmail'] ?? null; $aliasesResource = $this->getAliasesForUser($primaryEmail); if ($aliasesResource) { @@ -408,7 +410,8 @@ private function doesUserMatch($entry, $query = '') } } elseif (is_array($checkValue)) { throw new \Exception( - "Did not expect something other than name as an array. Got VALUE: " . var_dump($checkValue) + "Did not expect something other than name as an array. Got VALUE: " + . var_export($checkValue, true) ); } } elseif (isset($entry['name'][$field])) {