diff --git a/src/Controller/SantaController.php b/src/Controller/SantaController.php index d84ef4a..1b86cf9 100644 --- a/src/Controller/SantaController.php +++ b/src/Controller/SantaController.php @@ -187,8 +187,7 @@ public function exclusions(FormFactoryInterface $formFactory, Request $request, $config = $this->getConfigOrThrow404($request); $selectedUsers = $config->getSelectedUsers(); - /** @var true|false $areExclusionsAllowed */ - $areExclusionsAllowed = false; // \count($selectedUsers) <= 100; + $areExclusionsAllowed = \count($selectedUsers) <= 100; $form = null; // We remove exclusions from users that aren't selected anymore and create empty ones for those who are @@ -359,22 +358,29 @@ public function validate(FormFactoryInterface $formFactory, Spoiler $spoiler, Re if ($form->isValid()) { $shuffleButton = $form->get('shuffle'); if ($shuffleButton instanceof SubmitButton && $shuffleButton->isClicked()) { - $config->setShuffledUsers($this->rudolph->associateUsers($config)); - $this->saveConfig($request, $config); - - $secretSanta = new SecretSanta( - 'shuffle', - [], - $config - ); - $this->statisticCollector->incrementShuffleCount($secretSanta); - - return $this->redirectToRoute('validate', [ - 'application' => $application->getCode(), - 'reshuffled' => 1, - ]); + try { + $config->setShuffledUsers($this->rudolph->associateUsers($config)); + $this->saveConfig($request, $config); + + $secretSanta = new SecretSanta( + 'shuffle', + [], + $config + ); + $this->statisticCollector->incrementShuffleCount($secretSanta); + + return $this->redirectToRoute('validate', [ + 'application' => $application->getCode(), + 'reshuffled' => 1, + ]); + } catch (RudolphException $e) { + // Previous shuffle is kept, we just display the error + $errors[] = $e->getMessage(); + } } + } + if (!$errors && $form->isValid()) { $secretSanta = $this->prepareSecretSanta($config); $session = $request->getSession(); $session->set( @@ -395,13 +401,9 @@ public function validate(FormFactoryInterface $formFactory, Spoiler $spoiler, Re return $this->redirectToRoute('send_messages', ['hash' => $secretSanta->getHash()]); } - $errors = array_map(function (FormError $error) { + $errors = array_unique([...$errors, ...array_map(function (FormError $error) { return $error->getMessage(); - }, iterator_to_array($form->getErrors(true, false))); - - if ($errors) { - $errors = array_unique($errors); - } + }, iterator_to_array($form->getErrors(true, false)))]); } $content = $this->twig->render('santa/application/validate_' . $application->getCode() . '.html.twig', [ diff --git a/src/Santa/Rudolph.php b/src/Santa/Rudolph.php index c131057..d86deb7 100644 --- a/src/Santa/Rudolph.php +++ b/src/Santa/Rudolph.php @@ -26,6 +26,11 @@ class Rudolph { private const int MAX_ATTEMPTS = 10; + /** + * Upper bound of recursive steps per attempt, to avoid exponential blowups on pathological exclusions. + */ + private const int MAX_STEPS = 100_000; + /** * @return array */ @@ -39,34 +44,24 @@ public function associateUsers(Config $config): array mt_srand(); - // Simple path: no exclusions, just shuffle and assign in a circle - if (!$exclusions) { - $associations = []; - $userCount = \count($users); - - shuffle($users); - - for ($i = 1; $i < $userCount; ++$i) { - $associations[$users[$i - 1]] = $users[$i]; - $associations[$users[$userCount - 1]] = $users[0]; - } - - $associations[$users[$userCount - 1]] = $users[0]; - - return $associations; - } - $users = array_values($users); for ($attempt = 0; $attempt < self::MAX_ATTEMPTS; ++$attempt) { $shuffled = $users; shuffle($shuffled); + // Without exclusions, any shuffled order is a valid single loop + if (!$exclusions) { + return $this->chainToAssociations($shuffled); + } + $used = array_fill_keys($shuffled, false); - $result = $this->assignSantaRecursive($shuffled, $exclusions, $used, []); + $used[$shuffled[0]] = true; + $steps = 0; + $chain = $this->buildChainRecursive($shuffled, $exclusions, $used, [$shuffled[0]], $steps); - if (null !== $result) { - return $result; + if (null !== $chain) { + return $this->chainToAssociations($chain); } } @@ -74,52 +69,71 @@ public function associateUsers(Config $config): array } /** - * Recursive function to assign Secret Santa pairs using backtracking algorithm. + * Turn an ordered chain into associations: each user offers to the next one, the last one offers to the first one. + * + * @param list $chain + * + * @return array + */ + private function chainToAssociations(array $chain): array + { + $associations = []; + $count = \count($chain); + + for ($i = 0; $i < $count; ++$i) { + $associations[$chain[$i]] = $chain[($i + 1) % $count]; + } + + return $associations; + } + + /** + * Recursive backtracking building a single loop (Hamiltonian cycle) respecting exclusions. * * @param list $users * @param array> $exclusions * @param array $used - * @param array $current + * @param list $chain * - * @return array|null + * @return list|null */ - private function assignSantaRecursive(array $users, array $exclusions, array &$used, array $current): ?array + private function buildChainRecursive(array $users, array $exclusions, array &$used, array $chain, int &$steps): ?array { - $index = \count($current); - - // All users have been assigned - if ($index === \count($users)) { - return $current; + if (++$steps > self::MAX_STEPS) { + return null; } - $giver = $users[$index]; + $giver = $chain[\count($chain) - 1]; + + // Everyone is in the chain: the last one must be allowed to offer to the first one to close the loop + if (\count($chain) === \count($users)) { + return \in_array($chain[0], $exclusions[$giver] ?? [], true) ? null : $chain; + } // Find possible receivers for the current giver - $possibleReceivers = array_filter($users, function ($receiver) use ($giver, $exclusions, $used) { + $possibleReceivers = array_values(array_filter($users, function ($receiver) use ($giver, $exclusions, $used) { return !$used[$receiver] - && $receiver !== $giver && !\in_array($receiver, $exclusions[$giver] ?? [], true); - }); + })); // Randomize possible receivers to ensure different results on each run - $possibleReceivers = array_values($possibleReceivers); shuffle($possibleReceivers); foreach ($possibleReceivers as $receiver) { $used[$receiver] = true; - $current[$giver] = $receiver; + $chain[] = $receiver; - $result = $this->assignSantaRecursive($users, $exclusions, $used, $current); + $result = $this->buildChainRecursive($users, $exclusions, $used, $chain, $steps); if (null !== $result) { return $result; } // Backtrack - unset($current[$giver]); + array_pop($chain); $used[$receiver] = false; } - // No valid assignment found for this user + // No valid receiver found for this user return null; } diff --git a/templates/content/faq.html.twig b/templates/content/faq.html.twig index a61401f..4d5dd94 100644 --- a/templates/content/faq.html.twig +++ b/templates/content/faq.html.twig @@ -197,9 +197,6 @@

- 🚧 👷 This feature is a work in progress - any funding or help is welcome! -

-

Yes, you can! On the second step of the application, you will be able to fill the exclusion form. There, you can specify pairs of users that should not be matched together. This is useful for couples, family members, or just people who don't want to gift to each other. diff --git a/templates/santa/exclusions.html.twig b/templates/santa/exclusions.html.twig index 31b44b7..235daab 100644 --- a/templates/santa/exclusions.html.twig +++ b/templates/santa/exclusions.html.twig @@ -269,14 +269,8 @@

Define some exclusions

- {% if false %} -

- Exclusions are not enabled because there are too many participants.

- Go to the next step to continue. -

- {% endif %}

- Exclusions will be available soon. Stay tuned!

+ Exclusions are not enabled because there are too many participants.

Go to the next step to continue.

diff --git a/tests/Santa/RudolphTest.php b/tests/Santa/RudolphTest.php index 13675cb..7952a60 100644 --- a/tests/Santa/RudolphTest.php +++ b/tests/Santa/RudolphTest.php @@ -123,6 +123,69 @@ public function testItCreateAssociations(Config $config): void self::assertContains($user, $associations); self::assertNotSame($user, $associations[$user]); } + + foreach ($config->getExclusions() as $giver => $excluded) { + self::assertNotContains($associations[$giver] ?? null, $excluded); + } + + // Associations must form a single loop + $visited = []; + $current = array_key_first($associations); + do { + $visited[] = $current; + $current = $associations[$current]; + } while ($current !== array_key_first($associations)); + self::assertCount(\count($associations), $visited); + } + + public function testItAlwaysCreatesASingleLoopWithRandomExclusions(): void + { + for ($run = 0; $run < 200; ++$run) { + $count = random_int(2, 20); + $users = []; + for ($i = 0; $i < $count; ++$i) { + $users['user' . $i] = new User('user' . $i, 'User ' . $i); + } + + $config = new Config('app', 'org', null); + $config->setAvailableUsers($users); + $config->setUsersLoaded(true); + $config->setSelectedUsers(array_keys($users)); + + // Pick a hidden valid loop and never exclude its edges, so a solution is guaranteed to exist + $loop = array_keys($users); + shuffle($loop); + $allowed = []; + foreach ($loop as $i => $giver) { + $allowed[$giver] = $loop[($i + 1) % $count]; + } + + $exclusions = []; + foreach (array_keys($users) as $giver) { + $exclusions[$giver] = []; + foreach (array_keys($users) as $receiver) { + if ($receiver !== $giver && $receiver !== $allowed[$giver] && random_int(0, 100) < 30) { + $exclusions[$giver][] = $receiver; + } + } + } + $config->setExclusions($exclusions); + + $associations = $this->SUT->associateUsers($config); + + self::assertCount($count, $associations); + foreach ($associations as $giver => $receiver) { + self::assertNotContains($receiver, $exclusions[$giver]); + } + + $visited = []; + $current = array_key_first($associations); + do { + $visited[] = $current; + $current = $associations[$current]; + } while ($current !== array_key_first($associations)); + self::assertCount($count, $visited, 'Associations must form a single loop'); + } } /** @@ -186,5 +249,22 @@ public static function userListDataProvider(): iterable 'user2' => ['user3'], ]); yield 'with exclusions' => [$testConfig]; + + // Only 2 valid loops exist here: 1>3>2>4>1 and 1>4>2>3>1 - a matching-based algorithm would often + // produce two loops (1>2>1 and 3>4>3) + $testConfig = clone $config; + $testConfig->setSelectedUsers([ + 'user1', + 'user2', + 'user3', + 'user4', + ]); + $testConfig->setExclusions([ + 'user1' => ['user2'], + 'user2' => ['user1'], + 'user3' => ['user4'], + 'user4' => ['user3'], + ]); + yield 'exclusions forcing a single loop' => [$testConfig]; } }