Skip to content

Commit fd8506d

Browse files
elderingvmcj
authored andcommitted
Make problem.externalid a unique key.
This was only meant to be unique per contest, but this caused subtle issues with the POST /api/problems endpoint. In that case, there might be multiple problems with the given externalid and if this is used as external problem ID, then that would fail. Note that removing the UniqueEntity message just means that a default message about duplicate keys will be shown e.g. in forms when you try to add one. This is consistent with the settings we have for other entities. In more detail: If there is already a problem with externalid `foo`, but not associated to any contest, and you try to add a contest that also contains a problem with externalid `foo`, then first that problem gets created (which works), but after that when data is loaded at https://github.com/DOMjudge/domjudge/blob/8231f1291effadb328161462dd86a9f5223fc025/webapp/src/Service/ImportProblemService.php#L856-L859 if `externalIdFieldForEntity` is `externalid`, `getOneOrNullResult` throws an exception. I tried fixing this case by adding a check on the same contest, but this code is also called from POST /api/problems without the context of a contest. In that case it becomes unclear how to handle this, especially if there are already multiple problems with that externalid: should it create a new one or edit (which?) one of the existing ones? To avoid this complication I decided it is easier to just have a unique external ID across problems globally. (cherry picked from commit 6ace309)
1 parent b61294f commit fd8506d

File tree

2 files changed

+44
-0
lines changed

2 files changed

+44
-0
lines changed
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace DoctrineMigrations;
6+
7+
use Doctrine\DBAL\Schema\Schema;
8+
use Doctrine\Migrations\AbstractMigration;
9+
10+
/**
11+
* Auto-generated Migration: Please modify to your needs!
12+
*/
13+
final class Version20230508153514 extends AbstractMigration
14+
{
15+
public function getDescription(): string
16+
{
17+
return 'Make problem.externalid a unique index.';
18+
}
19+
20+
public function up(Schema $schema): void
21+
{
22+
// this up() migration is auto-generated, please modify it to your needs
23+
$this->addSql('DROP INDEX externalid ON problem');
24+
$this->addSql('CREATE UNIQUE INDEX externalid ON problem (externalid(190))');
25+
}
26+
27+
public function down(Schema $schema): void
28+
{
29+
// this down() migration is auto-generated, please modify it to your needs
30+
$this->addSql('DROP INDEX externalid ON problem');
31+
$this->addSql('CREATE INDEX externalid ON problem (externalid(190))');
32+
}
33+
34+
public function isTransactional(): bool
35+
{
36+
return false;
37+
}
38+
}

webapp/src/Entity/Problem.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,12 @@
2020
* @ORM\Entity()
2121
* @ORM\Table(
2222
* name="problem",
23+
* uniqueConstraints={
24+
* @ORM\UniqueConstraint(
25+
* name="externalid",
26+
* columns={"externalid"}
27+
* )
28+
* },
2329
* options={"collation"="utf8mb4_unicode_ci", "charset"="utf8mb4","comment"="Problems the teams can submit solutions for"},
2430
* indexes={
2531
* @ORM\Index(name="externalid", columns={"externalid"}, options={"lengths": {190}}),

0 commit comments

Comments
 (0)