Skip to content

Commit dbd00f3

Browse files
Catch errors when adding problems YAML.
Fixes #2482.
1 parent 2897cb0 commit dbd00f3

File tree

3 files changed

+48
-7
lines changed

3 files changed

+48
-7
lines changed

webapp/src/Controller/API/ProblemController.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,12 @@ public function addProblemsAction(Request $request): array
102102
// Note: we read the JSON as YAML, since any JSON is also YAML and this allows us
103103
// to import files with YAML inside them that match the JSON format
104104
$data = Yaml::parseFile($file->getRealPath(), Yaml::PARSE_DATETIME);
105-
if ($this->importExportService->importProblemsData($contest, $data, $ids)) {
105+
if ($this->importExportService->importProblemsData($contest, $data, $ids, $messages)) {
106106
return $ids;
107107
}
108+
if (!empty($messages)) {
109+
throw new BadRequestHttpException($this->dj->jsonEncode($messages));
110+
}
108111
throw new BadRequestHttpException("Error while adding problems");
109112
}
110113

webapp/src/Controller/Jury/ImportExportController.php

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -238,11 +238,15 @@ public function indexAction(Request $request): Response
238238
$this->addFlash('danger', "Parse error in YAML/JSON file (" . $file->getClientOriginalName() . "): " . $e->getMessage());
239239
return $this->redirectToRoute('jury_import_export');
240240
}
241-
if ($this->importExportService->importProblemsData($problemsImportForm->get('contest')->getData(), $data)) {
241+
if ($this->importExportService->importProblemsData($problemsImportForm->get('contest')->getData(), $data, $ids, $messages)) {
242242
$this->addFlash('success',
243243
sprintf('The file %s is successfully imported.', $file->getClientOriginalName()));
244244
} else {
245-
$this->addFlash('danger', 'Failed importing problems');
245+
if (!empty($messages)) {
246+
$this->postMessages($messages);
247+
} else {
248+
$this->addFlash('danger', 'Failed importing problems');
249+
}
246250
}
247251
return $this->redirectToRoute('jury_import_export');
248252
}

webapp/src/Service/ImportExportService.php

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -309,8 +309,9 @@ public function importContestData(mixed $data, ?string &$errorMessage = null, st
309309
* problems?: array{name?: string, short-name?: string, id?: string, label?: string,
310310
* letter?: string, label?: string, letter?: string}} $problems
311311
* @param string[]|null $ids
312+
* @param array<string, string[]> $messages
312313
*/
313-
public function importProblemsData(Contest $contest, array $problems, array &$ids = null): bool
314+
public function importProblemsData(Contest $contest, array $problems, array &$ids = null, ?array &$messages = []): bool
314315
{
315316
// For problemset.yaml the root key is called `problems`, so handle that case
316317
// TODO: Move this check away to make the $problems array shape easier
@@ -329,8 +330,20 @@ public function importProblemsData(Contest $contest, array $problems, array &$id
329330
->setTimelimit($problemData['time_limit'] ?? 10)
330331
->setExternalid($problemData['id'] ?? $problemData['short-name'] ?? $problemLabel ?? null);
331332

332-
$this->em->persist($problem);
333-
$this->em->flush();
333+
$hasErrors = false;
334+
$errors = $this->validator->validate($problem);
335+
if ($errors->count()) {
336+
$hasErrors = true;
337+
/** @var ConstraintViolationInterface $error */
338+
foreach ($errors as $error) {
339+
$messages['danger'][] = sprintf(
340+
'Error: problems.%s.%s: %s',
341+
$problem->getExternalid(),
342+
$error->getPropertyPath(),
343+
$error->getMessage()
344+
);
345+
}
346+
}
334347

335348
$contestProblem = new ContestProblem();
336349
$contestProblem
@@ -339,14 +352,35 @@ public function importProblemsData(Contest $contest, array $problems, array &$id
339352
// We need to set both the entities and the IDs because of the composite primary key.
340353
->setProblem($problem)
341354
->setContest($contest);
355+
356+
$errors = $this->validator->validate($contestProblem);
357+
if ($errors->count()) {
358+
$hasErrors = true;
359+
/** @var ConstraintViolationInterface $error */
360+
foreach ($errors as $error) {
361+
$messages['danger'][] = sprintf(
362+
'Error: problems.%s.contestproblem.%s: %s',
363+
$problem->getExternalid(),
364+
$error->getPropertyPath(),
365+
$error->getMessage()
366+
);
367+
}
368+
}
369+
370+
if ($hasErrors) {
371+
return false;
372+
373+
}
374+
375+
$this->em->persist($problem);
342376
$this->em->persist($contestProblem);
377+
$this->em->flush();
343378

344379
$ids[] = $problem->getApiId($this->eventLogService);
345380
}
346381

347382
$this->em->flush();
348383

349-
// For now this method will never fail so always return true.
350384
return true;
351385
}
352386

0 commit comments

Comments
 (0)