From 43883dca484fb74844514cb045783d41b4299223 Mon Sep 17 00:00:00 2001 From: Robin Slot Date: Thu, 12 Sep 2024 11:34:44 +0200 Subject: [PATCH 1/2] New group sync endpoints with separate add/remove lists --- core/classes/Group_Sync/GroupSyncManager.php | 127 ++++++++++++++++++ docker-compose.yaml | 3 + .../endpoints/SyncMinecraftGroupsEndpoint.php | 38 ++++++ .../endpoints/UpdateGroupsEndpoint.php | 3 + .../endpoints/SetDiscordRolesEndpoint.php | 2 +- .../endpoints/SyncDiscordRolesEndpoint.php | 40 ++++++ 6 files changed, 212 insertions(+), 1 deletion(-) create mode 100644 modules/Core/includes/endpoints/SyncMinecraftGroupsEndpoint.php create mode 100644 modules/Discord Integration/includes/endpoints/SyncDiscordRolesEndpoint.php diff --git a/core/classes/Group_Sync/GroupSyncManager.php b/core/classes/Group_Sync/GroupSyncManager.php index b06d62b0ca..4c150e7a27 100644 --- a/core/classes/Group_Sync/GroupSyncManager.php +++ b/core/classes/Group_Sync/GroupSyncManager.php @@ -153,6 +153,10 @@ private function compileValidatorMessages(Language $language): array * @param string $sending_injector_class Class name of injector broadcasting this change * @param array $group_ids Array of Group IDs native to the sending injector which were added/removed to the user * + * @deprecated broadcastGroupChange should be used instead. This function has issues, such as that on the first sync it will replace + * groups on one side with groups from the other side, depending on which side happens to sync first. On the first sync, + * the desired behaviour is for the new roles on both sides to become the union of roles on both sides. + * * @return array Array of logs of changed groups */ public function broadcastChange(User $user, string $sending_injector_class, array $group_ids): array @@ -283,6 +287,129 @@ public function broadcastChange(User $user, string $sending_injector_class, arra return $logs; } + /** + * Execute respective `addGroup()` or `removeGroup()` function on each of the injectors (e.g. Nameless itself, Minecraft, Discord) + * synced to the changed group. + * + * @param User $user NamelessMC user to apply changes to + * @param string $sending_injector_class Class name of injector broadcasting this change + * @param array $group_ids_add Array of injector-native Group IDs were added to the user + * @param array $group_ids_remove Array of injector-native Group IDs were removed from the user + * + * @return array Array of logs of changed groups + */ + public function broadcastGroupChange(User $user, string $sending_injector_class, array $group_ids_add, array $group_ids_remove): array + { + $sending_injector = $this->getInjectorByClass($sending_injector_class); + + if ($sending_injector === null) { + throw new InvalidArgumentException("Can't find injector by class: " . $sending_injector_class); + } + + $logs = []; + + $modified = []; + + $namelessmc_injector = $this->getInjectorByClass(NamelessMCGroupSyncInjector::class); + $namelessmc_column = $namelessmc_injector->getColumnName(); + + // Get all group sync rules where this injector is not null + $rules = DB::getInstance()->query("SELECT * FROM nl2_group_sync WHERE {$sending_injector->getColumnName()} IS NOT NULL")->results(); + + $batched_changes = []; + foreach ($rules as $rule) { + foreach ($this->getEnabledInjectors() as $injector) { + if ($injector == $sending_injector) { + continue; + } + + $injector_class = get_class($injector); + + $batchable = $injector instanceof BatchableGroupSyncInjector; + if ($batchable && !array_key_exists($injector_class, $batched_changes)) { + $batched_changes[$injector_class] = [ + 'add' => [], + 'remove' => [], + ]; + } + + $injector_column = $injector->getColumnName(); + $injector_group_id = $rule->{$injector_column}; + $sending_group_id = $rule->{$sending_injector->getColumnName()}; + + // Skip this injector if it doesn't have a group id setup for this rule + if ($injector_group_id === null) { + continue; + } + + if (!isset($modified[$injector_column])) { + $modified[$injector_column] = []; + } + + // Skip this specific injector for this rule if we have already modified the user + // with the same injector group id + if (in_array($injector_group_id, $modified[$injector_column])) { + continue; + } + + if (in_array($sending_group_id, $group_ids_add)) { + // Add group to user + $modified[$injector_column][] = $injector_group_id; + if ($batchable) { + $batched_changes[$injector_class]['add'][] = $injector_group_id; + } elseif ($injector->addGroup($user, $injector_group_id)) { + $logs['added'][] = "{$injector_column} -> {$injector_group_id}"; + } + } elseif (in_array($sending_group_id, $group_ids_remove)) { + // Remove group from user + $modified[$injector_column][] = $injector_group_id; + if ($batchable) { + $batched_changes[$injector_class]['remove'][] = $injector_group_id; + } elseif ($injector->removeGroup($user, $injector_group_id)) { + $logs['removed'][] = "{$injector_column} -> {$injector_group_id}"; + } + } + } + } + + foreach ($batched_changes as $injector_class => $data) { + $add = $data['add']; + $remove = $data['remove']; + + /** @var GroupSyncInjector&BatchableGroupSyncInjector $injector */ + $injector = $this->getInjectorByClass($injector_class); + $injector_column = $injector->getColumnName(); + + if ($injector instanceof BatchableGroupSyncInjector) { + /** @var GroupSyncInjector&BatchableGroupSyncInjector $injector */ + $batchable_injector = $injector; + if (count($add)) { + $result = $injector->batchAddGroups($user, $add); + if (is_array($result)) { + foreach ($result as $res) { + if ($res['status'] === 'added') { + $logs['added'][] = "{$injector_column} -> {$res['group_id']}"; + } + } + } + } + + if (count($remove)) { + $result = $injector->batchRemoveGroups($user, $remove); + if (is_array($result)) { + foreach ($result as $res) { + if ($res['status'] === 'removed') { + $logs['removed'][] = "{$injector_column} -> {$res['group_id']}"; + } + } + } + } + } + } + + return $logs; + } + /** * Get an enabled `GroupSyncInjector` from its class name, if it exists. * diff --git a/docker-compose.yaml b/docker-compose.yaml index 7a446d2ece..7950808b37 100755 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -14,6 +14,9 @@ # Reinstall: # docker compose exec php php -f dev/scripts/cli_install.php '--' '--iSwearIKnowWhatImDoing' '--reinstall' # +# Run phpstan: +# docker compose exec php vendor/bin/phpstan --configuration=dev/phpstan.neon +# # Uninstall # docker compose down # rm -rf core/config.php cache/* diff --git a/modules/Core/includes/endpoints/SyncMinecraftGroupsEndpoint.php b/modules/Core/includes/endpoints/SyncMinecraftGroupsEndpoint.php new file mode 100644 index 0000000000..cadba5fc56 --- /dev/null +++ b/modules/Core/includes/endpoints/SyncMinecraftGroupsEndpoint.php @@ -0,0 +1,38 @@ +_route = 'minecraft/sync-groups'; + $this->_module = 'Core'; + $this->_description = 'Update a users groups based on added or removed groups from the Minecraft server'; + $this->_method = 'POST'; + } + + public function execute(Nameless2API $api): void { + $api->validateParams($_POST, ['server_id', 'user']); + + $server_id = $_POST['server_id']; + $integration = Integrations::getInstance()->getIntegration('Minecraft'); + + if (!$integration || $server_id != Settings::get('group_sync_mc_server')) { + $api->returnArray(['message' => $api->getLanguage()->get('api', 'groups_updates_ignored')]); + } + + $user = $api->getUser('id', $_POST['user']); + + $log = GroupSyncManager::getInstance()->broadcastGroupChange( + $user, + MinecraftGroupSyncInjector::class, + $_POST['add'] ?? [], + $_POST['remove'] ?? [], + ); + + Log::getInstance()->log(Log::Action('mc_group_sync/role_set'), json_encode($log), $user->data()->id); + + $api->returnArray([ + 'message' => $api->getLanguage()->get('api', 'groups_updates_successfully'), + 'log' => $log, + ]); + } +} diff --git a/modules/Core/includes/endpoints/UpdateGroupsEndpoint.php b/modules/Core/includes/endpoints/UpdateGroupsEndpoint.php index d74a3101fb..8b7dbc7ffb 100644 --- a/modules/Core/includes/endpoints/UpdateGroupsEndpoint.php +++ b/modules/Core/includes/endpoints/UpdateGroupsEndpoint.php @@ -1,5 +1,8 @@ _route = 'discord/sync-roles'; + $this->_module = 'Discord Integration'; + $this->_description = 'Set a NamelessMC user\'s according to the supplied Discord Role ID list'; + $this->_method = 'POST'; + } + + public function execute(Nameless2API $api): void { + $api->validateParams($_POST, ['user']); + + if (!Discord::isBotSetup()) { + $api->throwError(DiscordApiErrors::ERROR_DISCORD_INTEGRATION_DISABLED); + } + + $user = $api->getUser('id', $_POST['user']); + + $log_array = GroupSyncManager::getInstance()->broadcastGroupChange( + $user, + DiscordGroupSyncInjector::class, + $_POST['add'] ?? [], + $_POST['remove'] ?? [] + ); + + if (count($log_array)) { + Log::getInstance()->log(Log::Action('discord/role_set'), json_encode($log_array), $user->data()->id); + } + + $api->returnArray(array_merge(['message' => Discord::getLanguageTerm('group_updated')], $log_array)); + } +} From 339ba7581f1a8170809559cd429e1076e97a81d1 Mon Sep 17 00:00:00 2001 From: Robin Slot Date: Fri, 13 Sep 2024 13:47:45 +0200 Subject: [PATCH 2/2] Code review changes --- core/classes/Group_Sync/GroupSyncManager.php | 36 +++++++++---------- .../endpoints/SyncMinecraftGroupsEndpoint.php | 8 ++--- .../endpoints/SyncDiscordRolesEndpoint.php | 14 ++------ 3 files changed, 22 insertions(+), 36 deletions(-) diff --git a/core/classes/Group_Sync/GroupSyncManager.php b/core/classes/Group_Sync/GroupSyncManager.php index 4c150e7a27..7a87a72833 100644 --- a/core/classes/Group_Sync/GroupSyncManager.php +++ b/core/classes/Group_Sync/GroupSyncManager.php @@ -373,34 +373,30 @@ public function broadcastGroupChange(User $user, string $sending_injector_class, } foreach ($batched_changes as $injector_class => $data) { - $add = $data['add']; - $remove = $data['remove']; - /** @var GroupSyncInjector&BatchableGroupSyncInjector $injector */ $injector = $this->getInjectorByClass($injector_class); $injector_column = $injector->getColumnName(); - if ($injector instanceof BatchableGroupSyncInjector) { - /** @var GroupSyncInjector&BatchableGroupSyncInjector $injector */ - $batchable_injector = $injector; - if (count($add)) { - $result = $injector->batchAddGroups($user, $add); - if (is_array($result)) { - foreach ($result as $res) { - if ($res['status'] === 'added') { - $logs['added'][] = "{$injector_column} -> {$res['group_id']}"; - } + $add = $data['add']; + $remove = $data['remove']; + + if (count($add)) { + $result = $injector->batchAddGroups($user, $add); + if (is_array($result)) { + foreach ($result as $res) { + if ($res['status'] === 'added') { + $logs['added'][] = "{$injector_column} -> {$res['group_id']}"; } } } + } - if (count($remove)) { - $result = $injector->batchRemoveGroups($user, $remove); - if (is_array($result)) { - foreach ($result as $res) { - if ($res['status'] === 'removed') { - $logs['removed'][] = "{$injector_column} -> {$res['group_id']}"; - } + if (count($remove)) { + $result = $injector->batchRemoveGroups($user, $remove); + if (is_array($result)) { + foreach ($result as $res) { + if ($res['status'] === 'removed') { + $logs['removed'][] = "{$injector_column} -> {$res['group_id']}"; } } } diff --git a/modules/Core/includes/endpoints/SyncMinecraftGroupsEndpoint.php b/modules/Core/includes/endpoints/SyncMinecraftGroupsEndpoint.php index cadba5fc56..ffe05642d5 100644 --- a/modules/Core/includes/endpoints/SyncMinecraftGroupsEndpoint.php +++ b/modules/Core/includes/endpoints/SyncMinecraftGroupsEndpoint.php @@ -3,14 +3,14 @@ class SyncMinecraftGroupsEndpoint extends KeyAuthEndpoint { public function __construct() { - $this->_route = 'minecraft/sync-groups'; + $this->_route = 'minecraft/{user}/sync-groups'; $this->_module = 'Core'; $this->_description = 'Update a users groups based on added or removed groups from the Minecraft server'; $this->_method = 'POST'; } - public function execute(Nameless2API $api): void { - $api->validateParams($_POST, ['server_id', 'user']); + public function execute(Nameless2API $api, User $user): void { + $api->validateParams($_POST, ['server_id']); $server_id = $_POST['server_id']; $integration = Integrations::getInstance()->getIntegration('Minecraft'); @@ -19,8 +19,6 @@ public function execute(Nameless2API $api): void { $api->returnArray(['message' => $api->getLanguage()->get('api', 'groups_updates_ignored')]); } - $user = $api->getUser('id', $_POST['user']); - $log = GroupSyncManager::getInstance()->broadcastGroupChange( $user, MinecraftGroupSyncInjector::class, diff --git a/modules/Discord Integration/includes/endpoints/SyncDiscordRolesEndpoint.php b/modules/Discord Integration/includes/endpoints/SyncDiscordRolesEndpoint.php index c010c2deed..cfe66f74e2 100644 --- a/modules/Discord Integration/includes/endpoints/SyncDiscordRolesEndpoint.php +++ b/modules/Discord Integration/includes/endpoints/SyncDiscordRolesEndpoint.php @@ -1,29 +1,21 @@ _route = 'discord/sync-roles'; + $this->_route = 'discord/{user}/sync-roles'; $this->_module = 'Discord Integration'; $this->_description = 'Set a NamelessMC user\'s according to the supplied Discord Role ID list'; $this->_method = 'POST'; } - public function execute(Nameless2API $api): void { - $api->validateParams($_POST, ['user']); + public function execute(Nameless2API $api, User $user): void { + $api->validateParams($_POST, []); if (!Discord::isBotSetup()) { $api->throwError(DiscordApiErrors::ERROR_DISCORD_INTEGRATION_DISABLED); } - $user = $api->getUser('id', $_POST['user']); - $log_array = GroupSyncManager::getInstance()->broadcastGroupChange( $user, DiscordGroupSyncInjector::class,