From e0dc8fd6e240fc129b5b10e7a7d06f3122a1c7fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kruli=C5=A1?= Date: Thu, 12 Feb 2026 19:35:45 +0100 Subject: [PATCH 1/2] Better logging of ReCodEx API response body. --- app/helpers/Recodex/RecodexApiHelper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/helpers/Recodex/RecodexApiHelper.php b/app/helpers/Recodex/RecodexApiHelper.php index 8ae944e..04cfa12 100644 --- a/app/helpers/Recodex/RecodexApiHelper.php +++ b/app/helpers/Recodex/RecodexApiHelper.php @@ -142,6 +142,7 @@ private function processJsonBody($response) if ($code !== 200) { Debugger::log("HTTP request to ReCodEx API failed (response $code).", Debugger::DEBUG); + Debugger::log("Response body:\n" . $response->getBody()->getContents(), Debugger::DEBUG); throw new RecodexApiException("HTTP request failed (response $code)."); } @@ -152,7 +153,6 @@ private function processJsonBody($response) } $body = json_decode($response->getBody()->getContents(), true); - Debugger::log($body, Debugger::DEBUG); if (($body['success'] ?? false) !== true) { $code = $body['code']; throw new RecodexApiException($body['error']['message'] ?? "API responded with error code $code."); From 5aa6e71860fd22fb1392675c4bcc7a6903583b2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kruli=C5=A1?= Date: Thu, 12 Feb 2026 23:47:17 +0100 Subject: [PATCH 2/2] Adding numerical suffix to group names to avoid duplicates (withing one parent group). --- app/helpers/Recodex/RecodexApiException.php | 23 ++- app/helpers/Recodex/RecodexApiHelper.php | 157 +++++++++++++++++--- 2 files changed, 155 insertions(+), 25 deletions(-) diff --git a/app/helpers/Recodex/RecodexApiException.php b/app/helpers/Recodex/RecodexApiException.php index 0ecea82..0174d75 100644 --- a/app/helpers/Recodex/RecodexApiException.php +++ b/app/helpers/Recodex/RecodexApiException.php @@ -9,12 +9,15 @@ */ class RecodexApiException extends InternalServerException { + private $response = null; + private $body = null; + /** * Create instance with further details. * @param string $details description * @param Exception $previous Previous exception */ - public function __construct(string $details, $previous = null) + public function __construct(string $details, $previous = null, $response = null, $body = null) { parent::__construct( "ReCodEx API Error - $details", @@ -22,5 +25,23 @@ public function __construct(string $details, $previous = null) null, $previous ); + $this->response = $response; + $this->body = $body; + } + + /** + * Get the response object of the failed API call, if available. + */ + public function getResponse() + { + return $this->response; + } + + /** + * Get the body of the response of the failed API call, if available. + */ + public function getBody() + { + return $this->body; } } diff --git a/app/helpers/Recodex/RecodexApiHelper.php b/app/helpers/Recodex/RecodexApiHelper.php index 04cfa12..bd834d5 100644 --- a/app/helpers/Recodex/RecodexApiHelper.php +++ b/app/helpers/Recodex/RecodexApiHelper.php @@ -20,6 +20,8 @@ class RecodexApiHelper { use Nette\SmartObject; + private const GROUP_DUPLICATE_NAME_ERROR_CODE = '400-504'; + /** @var string Base of the API URL */ private string $apiBase; @@ -44,6 +46,10 @@ class RecodexApiHelper /** @var GuzzleHttp\Client */ private $client; + // caches + private $groupsCache = null; + private $groupsCacheUserId = null; + /** * @param array $config * @param GuzzleHttp\Client|null $client optional injection of HTTP client for testing purposes @@ -143,19 +149,24 @@ private function processJsonBody($response) if ($code !== 200) { Debugger::log("HTTP request to ReCodEx API failed (response $code).", Debugger::DEBUG); Debugger::log("Response body:\n" . $response->getBody()->getContents(), Debugger::DEBUG); - throw new RecodexApiException("HTTP request failed (response $code)."); + throw new RecodexApiException("HTTP request failed (response $code).", null, $response); } $type = $response->getHeaderLine("Content-Type") ?? ''; if (!str_starts_with($type, 'application/json')) { Debugger::log("JSON response expected from ReCodEx API but '$type' returned instead.", Debugger::DEBUG); - throw new RecodexApiException("JSON response was expected but '$type' returned instead."); + throw new RecodexApiException("JSON response was expected but '$type' returned instead.", null, $response); } $body = json_decode($response->getBody()->getContents(), true); if (($body['success'] ?? false) !== true) { $code = $body['code']; - throw new RecodexApiException($body['error']['message'] ?? "API responded with error code $code."); + throw new RecodexApiException( + $body['error']['message'] ?? "API responded with error code $code.", + null, + $response, + $body + ); } return $body['payload'] ?? null; @@ -370,17 +381,24 @@ public function removeExternalId(string $id, string $service): RecodexUser */ public function getGroups(User $user): array { - Debugger::log('ReCodEx::getGroups(' . $user->getId() . ')', Debugger::DEBUG); - $body = $this->get( - "group-attributes", - ['instance' => $user->getInstanceId(), 'service' => $this->extensionId, 'user' => $user->getId()] - ); - $groups = []; - foreach ($body as $groupData) { - $group = new RecodexGroup($groupData, $this->extensionId); - $groups[$group->id] = $group; + if ($this->groupsCacheUserId !== $user->getId()) { + $this->groupsCacheUserId = $user->getId(); + + Debugger::log('ReCodEx::getGroups(' . $user->getId() . ')', Debugger::DEBUG); + $body = $this->get( + "group-attributes", + ['instance' => $user->getInstanceId(), 'service' => $this->extensionId, 'user' => $user->getId()] + ); + $groups = []; + foreach ($body as $groupData) { + $group = new RecodexGroup($groupData, $this->extensionId); + $groups[$group->id] = $group; + } + + $this->groupsCache = $groups; } - return $groups; + + return $this->groupsCache; } /** @@ -463,6 +481,96 @@ public function removeAdminFromGroup(string $groupId, User $admin): void $this->delete("groups/$groupId/members/$adminId"); } + /** + * @param RecodexGroup[] $groups + * @param array $localizedTexts + * @return bool true if any of the groups has the same name in any of the locales as given in $localizedTexts + */ + private function isNameDuplicated(array $groups, array $localizedTexts): bool + { + foreach ($groups as $group) { + foreach ($localizedTexts as $text) { + if (($group->name[$text['locale']] ?? null) === $text['name']) { + return true; + } + } + } + return false; + } + + /** + * Compute suffix for group name in case of duplicate name error. The suffix is incremented until no duplication + * is detected. The suffix is added in format " [num]" to the end of the name. + * @param RecodexGroup[] $groups + * @param array $localizedTexts + * @param string $parentGroupId + * @return int|null suffix to be added to the name in case of duplicate name error, null if no duplication detected + */ + private function getAntiDuplicateSuffix(array $groups, array $localizedTexts, string $parentGroupId): ?int + { + $suffix = null; + $siblings = array_filter($groups, function (RecodexGroup $group) use ($parentGroupId) { + return $group->parentGroupId === $parentGroupId; + }); + + $originalTexts = $localizedTexts; + while ($this->isNameDuplicated($siblings, $localizedTexts)) { + $suffix = ($suffix ?? 1) + 1; // starting suffix is 2, then 3, etc. + $localizedTexts = $originalTexts; + foreach ($localizedTexts as &$text) { + $text['name'] .= " [$suffix]"; + } + } + + return $suffix; + } + + /** + * Create a new group with given parameters. If the group name is duplicated, + * null is returned (can be retried with different name). + * @param string $instanceId ID of the instance where the group is being created + * @param string $parentGroupId ID of the parent group + * @param array $localizedTexts localized texts for the group (locale => ['name' => ..., 'description' => ...]) + * @param int|null $localizedSuffix optional suffix [num] added to the name in case of duplicate name error + * @return array|null + */ + private function createGroupInternal( + string $instanceId, + string $parentGroupId, + array $localizedTexts, + ?int $localizedSuffix = null + ): ?array { + if ($localizedSuffix !== null) { + foreach ($localizedTexts as &$text) { + $text['name'] .= " [$localizedSuffix]"; + } + } + + try { + return $this->post("groups", [], [ + 'instanceId' => $instanceId, + 'parentGroupId' => $parentGroupId, + 'publicStats' => false, + 'detaining' => true, + 'isPublic' => false, + 'isOrganizational' => false, + 'isExam' => false, + 'noAdmin' => true, + 'localizedTexts' => $localizedTexts, + ]); + } catch (RecodexApiException $e) { + $body = $e->getBody(); + if ( + $body && is_array($body) && ($body['code'] ?? 0) === 400 && + ($body['error']['code'] ?? '') === self::GROUP_DUPLICATE_NAME_ERROR_CODE + ) { + // special case, duplicate name error can be retried with different suffix + return null; + } + throw $e; + } + } + /** * Create a new group and make given user an admin. * @param SisScheduleEvent $event event for which the group is being created @@ -487,17 +595,14 @@ public function createGroup(SisScheduleEvent $event, string $parentGroupId, User } } - $group = $this->post("groups", [], [ - 'instanceId' => $admin->getInstanceId(), - 'parentGroupId' => $parentGroupId, - 'publicStats' => false, - 'detaining' => true, - 'isPublic' => false, - 'isOrganizational' => false, - 'isExam' => false, - 'noAdmin' => true, - 'localizedTexts' => $localizedTexts, - ]); + $groups = $this->getGroups($admin); + $suffix = $this->getAntiDuplicateSuffix($groups, $localizedTexts, $parentGroupId); + + $retries = 5; + do { + $group = $this->createGroupInternal($admin->getInstanceId(), $parentGroupId, $localizedTexts, $suffix); + $suffix = ($suffix ?? 1) + 1; // starting suffix is 2, then 3, etc. + } while (!$group && $retries-- > 0); if ($group && !empty($group['id'])) { $this->addAdminToGroup($group['id'], $admin); @@ -505,6 +610,10 @@ public function createGroup(SisScheduleEvent $event, string $parentGroupId, User return $group['id']; } + Debugger::log( + "Failed to create group for '{$event->getSisId()}' after multiple attempts due to duplicate name.", + Debugger::WARNING + ); return null; }