Adding numerical suffix to group names to avoid duplicates (withing one parent group).

krulis-martin · krulis-martin · commit 4a43895422d4 · 2026-02-13T12:25:35.000+01:00
diff --git a/app/helpers/Recodex/RecodexApiException.php b/app/helpers/Recodex/RecodexApiException.php
@@ -9,18 +9,39 @@
  */
 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",
             FrontendErrorMappings::E500_000__INTERNAL_SERVER_ERROR,
             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
@@ -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,24 +595,25 @@ 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);
             $this->addAttribute($group['id'], RecodexGroup::ATTR_GROUP_KEY, $event->getSisId());
             return $group['id'];
         }
 
+        Debugger::log(
+            "Failed to create group for '{$event->getSisId()}' after multiple attempts due to duplicate name.",
+            Debugger::WARNING
+        );
         return null;
     }
 
