From 541fe1d9017462f353118e67526ea112a04106f5 Mon Sep 17 00:00:00 2001 From: Nicky Gerritsen Date: Fri, 24 Nov 2023 15:00:53 +0100 Subject: [PATCH] Split the problem text content from the problem in the database. This is to prepare for the next Doctrine release, which doesn't allow partial queries anymore. This is also to make it consistent with other blobs in the database like submission files and problem attachments. This is preparation for #2069. --- webapp/migrations/Version20231124133426.php | 42 ++++++++++ .../src/Controller/API/ProblemController.php | 5 +- .../Jury/ClarificationController.php | 2 +- .../src/Controller/Jury/ContestController.php | 2 +- .../src/Controller/Jury/ProblemController.php | 7 +- webapp/src/Entity/Problem.php | 82 +++++++++++-------- webapp/src/Entity/ProblemTextContent.php | 51 ++++++++++++ webapp/src/Service/DOMJudgeService.php | 7 +- webapp/src/Service/ImportProblemService.php | 7 +- webapp/src/Service/ScoreboardService.php | 2 +- webapp/src/Service/StatisticsService.php | 2 +- .../Controller/Team/ProblemControllerTest.php | 24 +++--- 12 files changed, 172 insertions(+), 61 deletions(-) create mode 100644 webapp/migrations/Version20231124133426.php create mode 100644 webapp/src/Entity/ProblemTextContent.php diff --git a/webapp/migrations/Version20231124133426.php b/webapp/migrations/Version20231124133426.php new file mode 100644 index 0000000000..6048da4d36 --- /dev/null +++ b/webapp/migrations/Version20231124133426.php @@ -0,0 +1,42 @@ +addSql('CREATE TABLE problem_text_content (probid INT UNSIGNED NOT NULL COMMENT \'Problem ID\', content LONGBLOB NOT NULL COMMENT \'Text content(DC2Type:blobtext)\', PRIMARY KEY(probid)) DEFAULT CHARACTER SET utf8mb4 COLLATE `utf8mb4_unicode_ci` ENGINE = InnoDB COMMENT = \'Stores contents of problem texts\' '); + $this->addSql('INSERT INTO problem_text_content (probid, content) SELECT probid, problemtext FROM problem'); + $this->addSql('ALTER TABLE problem_text_content ADD CONSTRAINT FK_21B6AD6BEF049279 FOREIGN KEY (probid) REFERENCES problem (probid) ON DELETE CASCADE'); + $this->addSql('ALTER TABLE problem DROP problemtext'); + } + + public function down(Schema $schema): void + { + // this down() migration is auto-generated, please modify it to your needs + $this->addSql('ALTER TABLE problem ADD problemtext LONGBLOB DEFAULT NULL COMMENT \'Problem text in HTML/PDF/ASCII\''); + $this->addSql('UPDATE problem INNER JOIN problem_text_content USING (probid) SET problem.problemtext = problem_text_content.content'); + $this->addSql('ALTER TABLE problem_text_content DROP FOREIGN KEY FK_21B6AD6BEF049279'); + $this->addSql('DROP TABLE problem_text_content'); + } + + public function isTransactional(): bool + { + return false; + } +} diff --git a/webapp/src/Controller/API/ProblemController.php b/webapp/src/Controller/API/ProblemController.php index c2519bfc3d..0192af43e9 100644 --- a/webapp/src/Controller/API/ProblemController.php +++ b/webapp/src/Controller/API/ProblemController.php @@ -414,7 +414,8 @@ public function singleAction(Request $request, string $id): Response public function statementAction(Request $request, string $id): Response { $queryBuilder = $this->getQueryBuilder($request) - ->addSelect('partial p.{probid,problemtext}') + ->leftJoin('p.problemTextContent', 'content') + ->addSelect('content') ->setParameter('id', $id) ->andWhere(sprintf('%s = :id', $this->getIdField())); @@ -448,7 +449,7 @@ protected function getQueryBuilder(Request $request): QueryBuilder ->from(ContestProblem::class, 'cp') ->join('cp.problem', 'p') ->leftJoin('p.testcases', 'tc') - ->select('cp, partial p.{probid,externalid,name,timelimit,memlimit,problemtext_type}, COUNT(tc.testcaseid) AS testdatacount') + ->select('cp, p, COUNT(tc.testcaseid) AS testdatacount') ->andWhere('cp.contest = :cid') ->andWhere('cp.allowSubmit = 1') ->setParameter('cid', $contestId) diff --git a/webapp/src/Controller/Jury/ClarificationController.php b/webapp/src/Controller/Jury/ClarificationController.php index ebbbfca552..f0a6746927 100644 --- a/webapp/src/Controller/Jury/ClarificationController.php +++ b/webapp/src/Controller/Jury/ClarificationController.php @@ -242,7 +242,7 @@ protected function getClarificationFormData(?Team $team = null): array /** @var ContestProblem[] $contestproblems */ $contestproblems = $this->em->createQueryBuilder() ->from(ContestProblem::class, 'cp') - ->select('cp, partial p.{probid,externalid,name}') + ->select('cp, p') ->innerJoin('cp.problem', 'p') ->where('cp.contest IN (:contests)') ->setParameter('contests', $contests) diff --git a/webapp/src/Controller/Jury/ContestController.php b/webapp/src/Controller/Jury/ContestController.php index e749fde6dd..a7bb7b5283 100644 --- a/webapp/src/Controller/Jury/ContestController.php +++ b/webapp/src/Controller/Jury/ContestController.php @@ -403,7 +403,7 @@ public function viewAction(Request $request, int $contestId): Response $problems = $this->em->createQueryBuilder() ->from(ContestProblem::class, 'cp') ->join('cp.problem', 'p') - ->select('cp', 'partial p.{probid,externalid,name,timelimit,memlimit,problemtext_type}') + ->select('cp', 'p') ->andWhere('cp.contest = :contest') ->setParameter('contest', $contest) ->orderBy('cp.shortname') diff --git a/webapp/src/Controller/Jury/ProblemController.php b/webapp/src/Controller/Jury/ProblemController.php index 65f81d7510..83c1227681 100644 --- a/webapp/src/Controller/Jury/ProblemController.php +++ b/webapp/src/Controller/Jury/ProblemController.php @@ -62,7 +62,7 @@ public function __construct( public function indexAction(): Response { $problems = $this->em->createQueryBuilder() - ->select('partial p.{probid,externalid,name,timelimit,memlimit,outputlimit,problemtext_type}', 'COUNT(tc.testcaseid) AS testdatacount') + ->select('p', 'COUNT(tc.testcaseid) AS testdatacount') ->from(Problem::class, 'p') ->leftJoin('p.testcases', 'tc') ->orderBy('p.probid', 'ASC') @@ -239,7 +239,8 @@ public function exportAction(int $problemId): StreamedResponse $problem = $this->em->createQueryBuilder() ->from(Problem::class, 'p') ->leftJoin('p.contest_problems', 'cp', Join::WITH, 'cp.contest = :contest') - ->select('p', 'cp') + ->leftJoin('p.problemTextContent', 'content') + ->select('p', 'cp', 'content') ->andWhere('p.probid = :problemId') ->setParameter('problemId', $problemId) ->setParameter('contest', $this->dj->getCurrentContest()) @@ -295,7 +296,7 @@ public function exportAction(int $problemId): StreamedResponse if (!empty($problem->getProblemtext())) { $zip->addFromString('problem.' . $problem->getProblemtextType(), - stream_get_contents($problem->getProblemtext())); + $problem->getProblemtext()); } foreach ([true, false] as $isSample) { diff --git a/webapp/src/Entity/Problem.php b/webapp/src/Entity/Problem.php index 379b164ad4..81ccb59566 100644 --- a/webapp/src/Entity/Problem.php +++ b/webapp/src/Entity/Problem.php @@ -90,17 +90,6 @@ class Problem extends BaseApiEntity #[Serializer\Exclude] private bool $combined_run_compare = false; - /** - * @var resource|string|null - */ - #[ORM\Column( - type: 'blob', - nullable: true, - options: ['comment' => 'Problem text in HTML/PDF/ASCII'] - )] - #[Serializer\Exclude] - private mixed $problemtext = null; - #[Assert\File] #[Serializer\Exclude] private ?UploadedFile $problemtextFile = null; @@ -155,6 +144,22 @@ class Problem extends BaseApiEntity #[Serializer\Exclude] private Collection $testcases; + /** + * @var Collection + * + * We use a OneToMany instead of a OneToOne here, because otherwise this + * relation will always be loaded. See the commit message of commit + * 9e421f96691ec67ed62767fe465a6d8751edd884 for a more elaborate explanation + */ + #[ORM\OneToMany( + mappedBy: 'problem', + targetEntity: ProblemTextContent::class, + cascade: ['persist'], + orphanRemoval: true + )] + #[Serializer\Exclude] + private Collection $problemTextContent; + /** * @var Collection */ @@ -259,21 +264,12 @@ public function getCombinedRunCompare(): bool return $this->combined_run_compare; } - /** - * @param resource|string|null $problemtext - */ - public function setProblemtext($problemtext): Problem - { - $this->problemtext = $problemtext; - return $this; - } - public function setProblemtextFile(?UploadedFile $problemtextFile): Problem { $this->problemtextFile = $problemtextFile; // Clear the problem text to make sure the entity is modified. - $this->problemtext = ''; + $this->setProblemTextContent(null); return $this; } @@ -281,17 +277,14 @@ public function setProblemtextFile(?UploadedFile $problemtextFile): Problem public function setClearProblemtext(bool $clearProblemtext): Problem { $this->clearProblemtext = $clearProblemtext; - $this->problemtext = null; + $this->setProblemTextContent(null); return $this; } - /** - * @return resource|string - */ - public function getProblemtext() + public function getProblemtext(): ?string { - return $this->problemtext; + return $this->getProblemTextContent()?->getContent(); } public function getProblemtextFile(): ?UploadedFile @@ -339,11 +332,12 @@ public function getRunExecutable(): ?Executable public function __construct() { - $this->testcases = new ArrayCollection(); - $this->submissions = new ArrayCollection(); - $this->clarifications = new ArrayCollection(); - $this->contest_problems = new ArrayCollection(); - $this->attachments = new ArrayCollection(); + $this->testcases = new ArrayCollection(); + $this->submissions = new ArrayCollection(); + $this->clarifications = new ArrayCollection(); + $this->contest_problems = new ArrayCollection(); + $this->attachments = new ArrayCollection(); + $this->problemTextContent = new ArrayCollection(); } public function addTestcase(Testcase $testcase): Problem @@ -433,13 +427,29 @@ public function getAttachments(): Collection return $this->attachments; } + public function setProblemTextContent(?ProblemTextContent $content): self + { + $this->problemTextContent->clear(); + if ($content) { + $this->problemTextContent->add($content); + $content->setProblem($this); + } + + return $this; + } + + public function getProblemTextContent(): ?ProblemTextContent + { + return $this->problemTextContent->first() ?: null; + } + #[ORM\PrePersist] #[ORM\PreUpdate] public function processProblemText(): void { if ($this->isClearProblemtext()) { $this - ->setProblemtext(null) + ->setProblemTextContent(null) ->setProblemtextType(null); } elseif ($this->getProblemtextFile()) { $content = file_get_contents($this->getProblemtextFile()->getRealPath()); @@ -476,8 +486,10 @@ public function processProblemText(): void throw new Exception('Problem statement has unknown file type.'); } + $problemTextContent = (new ProblemTextContent()) + ->setContent($content); $this - ->setProblemtext($content) + ->setProblemTextContent($problemTextContent) ->setProblemtextType($problemTextType); } } @@ -492,7 +504,7 @@ public function getProblemTextStreamedResponse(): StreamedResponse }; $filename = sprintf('prob-%s.%s', $this->getName(), $this->getProblemtextType()); - $problemText = stream_get_contents($this->getProblemtext()); + $problemText = $this->getProblemtext(); $response = new StreamedResponse(); $response->setCallback(function () use ($problemText) { diff --git a/webapp/src/Entity/ProblemTextContent.php b/webapp/src/Entity/ProblemTextContent.php new file mode 100644 index 0000000000..3a4d537782 --- /dev/null +++ b/webapp/src/Entity/ProblemTextContent.php @@ -0,0 +1,51 @@ + 'utf8mb4_unicode_ci', + 'charset' => 'utf8mb4', + 'comment' => 'Stores contents of problem texts', +])] +class ProblemTextContent +{ + /** + * We use a ManyToOne instead of a OneToOne here, because otherwise the + * reverse of this relation will always be loaded. See the commit message of commit + * 9e421f96691ec67ed62767fe465a6d8751edd884 for a more elaborate explanation. + */ + #[ORM\Id] + #[ORM\ManyToOne(inversedBy: 'problemTextContent')] + #[ORM\JoinColumn(name: 'probid', referencedColumnName: 'probid', onDelete: 'CASCADE')] + private Problem $problem; + + #[ORM\Column(type: 'blobtext', options: ['comment' => 'Text content'])] + private string $content; + + public function getProblem(): Problem + { + return $this->problem; + } + + public function setProblem(Problem $problem): self + { + $this->problem = $problem; + + return $this; + } + + public function getContent(): string + { + return $this->content; + } + + public function setContent(string $content): self + { + $this->content = $content; + + return $this; + } +} diff --git a/webapp/src/Service/DOMJudgeService.php b/webapp/src/Service/DOMJudgeService.php index a75731c175..02be143d6c 100644 --- a/webapp/src/Service/DOMJudgeService.php +++ b/webapp/src/Service/DOMJudgeService.php @@ -839,7 +839,8 @@ public function getSamplesZipForContest(Contest $contest): StreamedResponse ->innerJoin('c.problems', 'cp') ->innerJoin('cp.problem', 'p') ->leftJoin('p.attachments', 'a') - ->select('c', 'cp', 'p', 'a') + ->leftJoin('p.problemTextContent', 'content') + ->select('c', 'cp', 'p', 'a', 'content') ->andWhere('c.cid = :cid') ->setParameter('cid', $contest->getCid()) ->getQuery() @@ -861,7 +862,7 @@ public function getSamplesZipForContest(Contest $contest): StreamedResponse if ($problem->getProblem()->getProblemtextType()) { $filename = sprintf('%s/statement.%s', $problem->getShortname(), $problem->getProblem()->getProblemtextType()); - $zip->addFromString($filename, stream_get_contents($problem->getProblem()->getProblemtext())); + $zip->addFromString($filename, $problem->getProblem()->getProblemtext()); } /** @var ProblemAttachment $attachment */ @@ -972,7 +973,7 @@ public function getTwigDataForProblemsAction( ->join('cp.problem', 'p') ->leftJoin('p.testcases', 'tc') ->leftJoin('p.attachments', 'a') - ->select('partial p.{probid,name,externalid,problemtext_type,timelimit,memlimit,combined_run_compare}', 'cp', 'a') + ->select('p', 'cp', 'a') ->andWhere('cp.contest = :contest') ->andWhere('cp.allowSubmit = 1') ->setParameter('contest', $contest) diff --git a/webapp/src/Service/ImportProblemService.php b/webapp/src/Service/ImportProblemService.php index 3d8aabd534..10f77ea328 100644 --- a/webapp/src/Service/ImportProblemService.php +++ b/webapp/src/Service/ImportProblemService.php @@ -9,6 +9,7 @@ use App\Entity\Problem; use App\Entity\ProblemAttachment; use App\Entity\ProblemAttachmentContent; +use App\Entity\ProblemTextContent; use App\Entity\Submission; use App\Entity\Team; use App\Entity\Testcase; @@ -209,7 +210,7 @@ public function importZippedProblem( ->setCombinedRunCompare(false) ->setMemlimit(null) ->setOutputlimit(null) - ->setProblemtext(null) + ->setProblemTextContent(null) ->setProblemtextType(null); $contestProblem @@ -313,8 +314,10 @@ public function importZippedProblem( $filename = sprintf('%sproblem.%s', $dir, $type); $text = $zip->getFromName($filename); if ($text !== false) { + $content = (new ProblemTextContent()) + ->setContent($text); $problem - ->setProblemtext($text) + ->setProblemTextContent($content) ->setProblemtextType($type); $messages['info'][] = "Added/updated problem statement from: $filename"; break 2; diff --git a/webapp/src/Service/ScoreboardService.php b/webapp/src/Service/ScoreboardService.php index fe4d6178db..93bbd3cd36 100644 --- a/webapp/src/Service/ScoreboardService.php +++ b/webapp/src/Service/ScoreboardService.php @@ -1003,7 +1003,7 @@ protected function getProblems(Contest $contest): array { $queryBuilder = $this->em->createQueryBuilder() ->from(ContestProblem::class, 'cp') - ->select('cp, partial p.{probid,externalid,name,problemtext_type}') + ->select('cp, p') ->innerJoin('cp.problem', 'p') ->andWhere('cp.allowSubmit = 1') ->andWhere('cp.contest = :contest') diff --git a/webapp/src/Service/StatisticsService.php b/webapp/src/Service/StatisticsService.php index 44f468c57e..e6489a7263 100644 --- a/webapp/src/Service/StatisticsService.php +++ b/webapp/src/Service/StatisticsService.php @@ -232,7 +232,7 @@ public function getTeamStats(Contest $contest, Team $team): array // - The submission was made by a team in a visible category /** @var Judging[] $judgings */ $judgings = $this->em->createQueryBuilder() - ->select('j, jr', 's', 'team', 'partial p.{timelimit,name,probid}') + ->select('j, jr', 's', 'team', 'p') ->from(Judging::class, 'j') ->join('j.submission', 's') ->join('s.problem', 'p') diff --git a/webapp/tests/Unit/Controller/Team/ProblemControllerTest.php b/webapp/tests/Unit/Controller/Team/ProblemControllerTest.php index d32c4f799d..27e18f328e 100644 --- a/webapp/tests/Unit/Controller/Team/ProblemControllerTest.php +++ b/webapp/tests/Unit/Controller/Team/ProblemControllerTest.php @@ -19,7 +19,7 @@ class ProblemControllerTest extends BaseTestCase */ public function testIndex(bool $withLimits): void { - $problems = [ + $problems = [ 'hello', 'fltcmp', 'boolfind', @@ -35,16 +35,16 @@ public function testIndex(bool $withLimits): void 'C', ]; /** @var EntityManagerInterface $em */ - $em = self::getContainer()->get(EntityManagerInterface::class); + $em = self::getContainer()->get(EntityManagerInterface::class); $problemTextsData = $em->createQueryBuilder() ->from(Problem::class, 'p') - ->select('p.externalid, p.problemtext') + ->leftJoin('p.problemTextContent', 'c') + ->select('p.externalid, c.content') ->getQuery() ->getResult(); - $problemTexts = []; + $problemTexts = []; foreach ($problemTextsData as $data) { - $problemTexts[array_search($data['externalid'], $problems)] = - stream_get_contents($data['problemtext']); + $problemTexts[array_search($data['externalid'], $problems)] = $data['content']; } $this->withChangedConfiguration('show_limits_on_team_page', $withLimits, @@ -109,7 +109,7 @@ public function testSamples(): void /** @var Testcase[] $samples */ $samples = [ 1 => $problem->getTestcases()->get(0), - 2 => $problem->getTestcases()->get(2) + 2 => $problem->getTestcases()->get(2), ]; $samples[1]->setSample(true); $samples[2]->setSample(true); @@ -124,16 +124,16 @@ public function testSamples(): void // The first and last card should not have any samples. self::assertSame(0, - $cardBodies->eq(0)->filter('.list-group .list-group-item')->count()); + $cardBodies->eq(0)->filter('.list-group .list-group-item')->count()); self::assertSame(0, - $cardBodies->eq(2)->filter('.list-group .list-group-item')->count()); + $cardBodies->eq(2)->filter('.list-group .list-group-item')->count()); // Check the link to download all samples. $link = $cardBodies->eq(1)->filter('a')->eq(1); self::assertSame('samples', $link->text(null, true)); self::assertSame(sprintf('/team/%d/samples.zip', - $problem->getProbid()), - $link->attr('href')); + $problem->getProbid()), + $link->attr('href')); // Download the sample and make sure the contents are correct. $this->client->click($link->link()); @@ -179,7 +179,7 @@ public function testInteractiveSamples(): void self::assertNotSame('samples', $link->text(null, true)); // Download the sample and make sure the contents are correct. - $this->client->request('GET', '/team/'.$problem->getProbid().'/samples.zip'); + $this->client->request('GET', '/team/' . $problem->getProbid() . '/samples.zip'); $response = $this->client->getResponse(); self::assertEquals(404, $response->getStatusCode()); }