diff --git a/webapp/src/Entity/Problem.php b/webapp/src/Entity/Problem.php index 3704fe85bc..edc47ab3be 100644 --- a/webapp/src/Entity/Problem.php +++ b/webapp/src/Entity/Problem.php @@ -52,7 +52,7 @@ class Problem extends BaseApiEntity implements #[ORM\Column(options: ['comment' => 'Descriptive name'])] #[Assert\NotBlank] - private string $name; + private string $name = 'Unknown name'; #[ORM\Column(options: [ 'comment' => 'Maximum run time (in seconds) for this problem', @@ -338,7 +338,11 @@ public function setTypes(array $types): Problem foreach ($types as $type) { $this->types |= $type; } - if (!($this->types & self::TYPE_PASS_FAIL) xor ($this->types & self::TYPE_SCORING)) { + if (!($this->types & self::TYPE_SCORING)) { + // In case the problem is not explicitly a scoring problem, default to pass-fail. + $this->types |= self::TYPE_PASS_FAIL; + } + if (($this->types & self::TYPE_PASS_FAIL) && ($this->types & self::TYPE_SCORING)) { throw new Exception("Invalid problem type: must be exactly one of 'pass-fail' or 'scoring'."); } if ($this->types & self::TYPE_SUBMIT_ANSWER) { diff --git a/webapp/src/Service/ImportProblemService.php b/webapp/src/Service/ImportProblemService.php index bb771291ee..b9b13c240f 100644 --- a/webapp/src/Service/ImportProblemService.php +++ b/webapp/src/Service/ImportProblemService.php @@ -27,6 +27,7 @@ use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; use Symfony\Component\HttpKernel\Exception\ServiceUnavailableHttpException; use Symfony\Component\PropertyAccess\PropertyAccess; +use Symfony\Component\PropertyAccess\PropertyAccessor; use Symfony\Component\Validator\ConstraintViolationInterface; use Symfony\Component\Validator\Validator\ValidatorInterface; use Symfony\Component\Yaml\Yaml; @@ -261,72 +262,12 @@ public function importZippedProblem( return null; } - if ($problemYaml !== false) { - $yamlData = Yaml::parse($problemYaml); - - if (!empty($yamlData)) { - $yamlProblemProperties = []; - if (isset($yamlData['name'])) { - if (is_array($yamlData['name'])) { - foreach ($yamlData['name'] as $name) { - // TODO: select a specific instead of the first language. - $yamlProblemProperties['name'] = $name; - break; - } - } else { - $yamlProblemProperties['name'] = $yamlData['name']; - } - } - - if (isset($yamlData['type'])) { - $types = explode(' ', $yamlData['type']); - // Validation happens later when we set the properties. - $yamlProblemProperties['typesAsString'] = $types; - } else { - $yamlProblemProperties['typesAsString'] = ['pass-fail']; - } - - if (isset($yamlData['validator_flags'])) { - $yamlProblemProperties['special_compare_args'] = $yamlData['validator_flags']; - } - - if (isset($yamlData['validation']) - && ($yamlData['validation'] == 'custom' || - $yamlData['validation'] == 'custom interactive' || - $yamlData['validation'] == 'custom multi-pass')) { - if (!$this->searchAndAddValidator($zip, $messages, $externalId, $yamlData['validation'], $problem)) { - return null; - } - - if ($yamlData['validation'] == 'custom multi-pass') { - $yamlProblemProperties['typesAsString'][] = 'multi-pass'; - } - if ($yamlData['validation'] == 'custom interactive') { - $yamlProblemProperties['typesAsString'][] = 'interactive'; - } - } - - if (isset($yamlData['limits'])) { - if (isset($yamlData['limits']['memory'])) { - $yamlProblemProperties['memlimit'] = 1024 * $yamlData['limits']['memory']; - } - if (isset($yamlData['limits']['output'])) { - $yamlProblemProperties['outputlimit'] = 1024 * $yamlData['limits']['output']; - } - if (isset($yamlData['limits']['validation_passes'])) { - $problem->setMultipassLimit($yamlData['limits']['validation_passes']); - } - } - - foreach ($yamlProblemProperties as $key => $value) { - try { - $propertyAccessor->setValue($problem, $key, $value); - } catch (Exception $e) { - $messages['danger'][] = sprintf('Error: problem.%s: %s', $key, $e->getMessage()); - return null; - } - } - } + $validationMode = 'default'; + if (!$this->parseYaml($problemYaml, $messages, $validationMode, $propertyAccessor, $problem)) { + return null; + } + if (!$this->searchAndAddValidator($zip, $messages, $externalId, $validationMode, $problem)) { + return null; } // Add problem statement, also look in obsolete location. @@ -955,96 +896,186 @@ private function searchAndAddValidator(ZipArchive $zip, ?array &$messages, strin $validatorFiles = []; for ($i = 0; $i < $zip->numFiles; $i++) { $filename = $zip->getNameIndex($i); - if (Utils::startsWith($filename, 'output_validators/') && - !Utils::endsWith($filename, '/')) { - $validatorFiles[] = $filename; + foreach (['output_validators/', 'output_validator'] as $dir) { + if (Utils::startsWith($filename, $dir) && + !Utils::endsWith($filename, '/')) { + $validatorFiles[] = $filename; + } } } if (sizeof($validatorFiles) == 0) { - $messages['danger'][] = 'Custom validator specified but not found.'; + if ($validationMode === 'default') { + return true; + } else { + $messages['danger'][] = 'Custom validator specified but not found.'; + return false; + } + } + + // File(s) have to share common directory. + $validatorDir = mb_substr($validatorFiles[0], 0, mb_strrpos($validatorFiles[0], '/')) . '/'; + $sameDir = true; + foreach ($validatorFiles as $validatorFile) { + if (!Utils::startsWith($validatorFile, $validatorDir)) { + $sameDir = false; + $messages['warning'][] = sprintf('%s does not start with %s.', + $validatorFile, $validatorDir); + break; + } + } + if (!$sameDir) { + $messages['danger'][] = 'Found multiple custom output validators.'; return false; } else { - // File(s) have to share common directory. - $validatorDir = mb_substr($validatorFiles[0], 0, mb_strrpos($validatorFiles[0], '/')) . '/'; - $sameDir = true; + $tmpzipfiledir = exec("mktemp -d --tmpdir=" . + $this->dj->getDomjudgeTmpDir(), + $dontcare, $retval); + if ($retval != 0) { + throw new ServiceUnavailableHttpException( + null, 'Failed to create temporary directory.' + ); + } + chmod($tmpzipfiledir, 0700); foreach ($validatorFiles as $validatorFile) { - if (!Utils::startsWith($validatorFile, $validatorDir)) { - $sameDir = false; - $messages['warning'][] = sprintf('%s does not start with %s.', - $validatorFile, $validatorDir); - break; + $content = $zip->getFromName($validatorFile); + $filebase = basename($validatorFile); + $newfilename = $tmpzipfiledir . "/" . $filebase; + file_put_contents($newfilename, $content); + if ($filebase === 'build' || $filebase === 'run') { + // Mark special files as executable. + chmod($newfilename, 0755); } } - if (!$sameDir) { - $messages['danger'][] = 'Found multiple custom output validators.'; - return false; - } else { - $tmpzipfiledir = exec("mktemp -d --tmpdir=" . - $this->dj->getDomjudgeTmpDir(), - $dontcare, $retval); - if ($retval != 0) { - throw new ServiceUnavailableHttpException( - null, 'Failed to create temporary directory.' - ); - } - chmod($tmpzipfiledir, 0700); - foreach ($validatorFiles as $validatorFile) { - $content = $zip->getFromName($validatorFile); - $filebase = basename($validatorFile); - $newfilename = $tmpzipfiledir . "/" . $filebase; - file_put_contents($newfilename, $content); - if ($filebase === 'build' || $filebase === 'run') { - // Mark special files as executable. - chmod($newfilename, 0755); - } - } - exec("zip -r -j '$tmpzipfiledir/outputvalidator.zip' '$tmpzipfiledir'", - $dontcare, $retval); - if ($retval != 0) { - throw new ServiceUnavailableHttpException( - null, 'Failed to create ZIP file for output validator.' - ); + exec("zip -r -j '$tmpzipfiledir/outputvalidator.zip' '$tmpzipfiledir'", + $dontcare, $retval); + if ($retval != 0) { + throw new ServiceUnavailableHttpException( + null, 'Failed to create ZIP file for output validator.' + ); + } + + $outputValidatorZip = file_get_contents($tmpzipfiledir . '/outputvalidator.zip'); + $outputValidatorName = substr($externalId, 0, 20) . '_cmp'; + if ($this->em->getRepository(Executable::class)->find($outputValidatorName)) { + // Avoid name clash. + $clashCount = 2; + while ($this->em->getRepository(Executable::class)->find( + $outputValidatorName . '_' . $clashCount)) { + $clashCount++; } + $outputValidatorName = $outputValidatorName . "_" . $clashCount; + } + + $combinedRunCompare = $validationMode == 'custom interactive'; + + if (!($tempzipFile = tempnam($this->dj->getDomjudgeTmpDir(), "/executable-"))) { + throw new ServiceUnavailableHttpException(null, 'Failed to create temporary file.'); + } + file_put_contents($tempzipFile, $outputValidatorZip); + $zipArchive = new ZipArchive(); + $zipArchive->open($tempzipFile, ZipArchive::CREATE); + + $executable = new Executable(); + $executable + ->setExecid($outputValidatorName) + ->setImmutableExecutable($this->dj->createImmutableExecutable($zipArchive)) + ->setDescription(sprintf('output validator for %s', $problem->getName())) + ->setType($combinedRunCompare ? 'run' : 'compare'); + $this->em->persist($executable); + + if ($combinedRunCompare) { + $problem->setRunExecutable($executable); + } else { + $problem->setCompareExecutable($executable); + } - $outputValidatorZip = file_get_contents($tmpzipfiledir . '/outputvalidator.zip'); - $outputValidatorName = substr($externalId, 0, 20) . '_cmp'; - if ($this->em->getRepository(Executable::class)->find($outputValidatorName)) { - // Avoid name clash. - $clashCount = 2; - while ($this->em->getRepository(Executable::class)->find( - $outputValidatorName . '_' . $clashCount)) { - $clashCount++; + $messages['info'][] = "Added output validator '$outputValidatorName'."; + } + return true; + } + + // Returns true iff the yaml could be parsed correctly. + public static function parseYaml(bool|string $problemYaml, array &$messages, string &$validationMode, PropertyAccessor $propertyAccessor, Problem $problem): bool + { + if ($problemYaml === false) { + // While there was no problem.yaml, there was also no error in parsing. + return true; + } + + $yamlData = Yaml::parse($problemYaml); + if (empty($yamlData)) { + // Empty yaml is OK. + return true; + } + + $yamlProblemProperties = []; + if (isset($yamlData['name'])) { + if (is_array($yamlData['name'])) { + // Prefer english name, but if not available, use first name. + $englishOrFirstName = null; + foreach ($yamlData['name'] as $lang => $name) { + if ($englishOrFirstName === null || $lang === 'en') { + $englishOrFirstName = $name; } - $outputValidatorName = $outputValidatorName . "_" . $clashCount; } + $yamlProblemProperties['name'] = $englishOrFirstName; + } else { + $yamlProblemProperties['name'] = $yamlData['name']; + } + } - $combinedRunCompare = $validationMode == 'custom interactive'; + $validationMode = 'default'; + if (isset($yamlData['type'])) { + $types = explode(' ', $yamlData['type']); + // Validation happens later when we set the properties. + $yamlProblemProperties['typesAsString'] = $types; + if (in_array('interactive', $types)) { + $validationMode = 'custom interactive'; + } + } else { + $yamlProblemProperties['typesAsString'] = ['pass-fail']; + } - if (!($tempzipFile = tempnam($this->dj->getDomjudgeTmpDir(), "/executable-"))) { - throw new ServiceUnavailableHttpException(null, 'Failed to create temporary file.'); - } - file_put_contents($tempzipFile, $outputValidatorZip); - $zipArchive = new ZipArchive(); - $zipArchive->open($tempzipFile, ZipArchive::CREATE); - - $executable = new Executable(); - $executable - ->setExecid($outputValidatorName) - ->setImmutableExecutable($this->dj->createImmutableExecutable($zipArchive)) - ->setDescription(sprintf('output validator for %s', $problem->getName())) - ->setType($combinedRunCompare ? 'run' : 'compare'); - $this->em->persist($executable); - - if ($combinedRunCompare) { - $problem->setRunExecutable($executable); - } else { - $problem->setCompareExecutable($executable); - } + if (isset($yamlData['validator_flags'])) { + $yamlProblemProperties['special_compare_args'] = $yamlData['validator_flags']; + } - $messages['info'][] = "Added output validator '$outputValidatorName'."; + if (isset($yamlData['validation']) + && ($yamlData['validation'] == 'custom' || + $yamlData['validation'] == 'custom interactive' || + $yamlData['validation'] == 'custom multi-pass')) { + $validationMode = $yamlData['validation']; + + if ($yamlData['validation'] == 'custom multi-pass') { + $yamlProblemProperties['typesAsString'][] = 'multi-pass'; + } + if ($yamlData['validation'] == 'custom interactive') { + $yamlProblemProperties['typesAsString'][] = 'interactive'; } } + + if (isset($yamlData['limits'])) { + if (isset($yamlData['limits']['memory'])) { + $yamlProblemProperties['memlimit'] = 1024 * $yamlData['limits']['memory']; + } + if (isset($yamlData['limits']['output'])) { + $yamlProblemProperties['outputlimit'] = 1024 * $yamlData['limits']['output']; + } + if (isset($yamlData['limits']['validation_passes'])) { + $yamlProblemProperties['multipassLimit'] = $yamlData['limits']['validation_passes']; + } + } + + foreach ($yamlProblemProperties as $key => $value) { + try { + $propertyAccessor->setValue($problem, $key, $value); + } catch (Exception $e) { + $messages['danger'][] = sprintf('Error: problem.%s: %s', $key, $e->getMessage()); + return false; + } + } + return true; } } diff --git a/webapp/tests/Unit/Controller/Jury/ExecutableControllerTest.php b/webapp/tests/Unit/Controller/Jury/ExecutableControllerTest.php index 5b45c00ef2..847a3c119d 100644 --- a/webapp/tests/Unit/Controller/Jury/ExecutableControllerTest.php +++ b/webapp/tests/Unit/Controller/Jury/ExecutableControllerTest.php @@ -11,7 +11,7 @@ class ExecutableControllerTest extends JuryControllerTestCase protected static ?string $editDefault = null; protected static string $baseUrl = '/jury/executables'; - protected static array $exampleEntries = ['adb', 'run', 'output validator for boolfind']; + protected static array $exampleEntries = ['adb', 'run', 'output validator for Boolean']; protected static string $shortTag = 'executable'; protected static array $deleteEntities = ['adb','default run script','rb','default full debug script']; protected static string $deleteEntityIdentifier = 'description'; diff --git a/webapp/tests/Unit/Service/ImportProblemServiceTest.php b/webapp/tests/Unit/Service/ImportProblemServiceTest.php new file mode 100644 index 0000000000..4f6368c30a --- /dev/null +++ b/webapp/tests/Unit/Service/ImportProblemServiceTest.php @@ -0,0 +1,302 @@ +assertTrue($ret); + $this->assertEquals('Unknown name', $problem->getName()); + } + + public function testMinimalYamlTest() + { + $yaml = <<assertTrue($ret); + $this->assertEmpty($messages); + $this->assertEquals('test', $problem->getName()); + $this->assertEquals('pass-fail', $problem->getTypesAsString()); + $this->assertEquals('default', $validationMode); + $this->assertEquals(0, $problem->getTimelimit()); + $this->assertEquals(null, $problem->getMemlimit()); + $this->assertEquals(null, $problem->getOutputlimit()); + $this->assertEquals(null, $problem->getSpecialCompareArgs()); + } + + public function testTypesYamlTest() + { + foreach ([ + 'pass-fail', + 'scoring', + 'multi-pass', + 'interactive', + 'submit-answer', + 'pass-fail multi-pass', + 'pass-fail interactive', + 'pass-fail submit-answer', + 'scoring multi-pass', + 'scoring interactive', + 'scoring submit-answer', + ] as $type) { + $yaml = <<assertTrue($ret, 'Parsing failed for type: ' . $type . ', messages: ' . $messageString); + if (in_array($type, ['interactive', 'multi-pass', 'submit-answer'])) { + // Default to pass-fail if not explicitly set. + $type = 'pass-fail ' . $type; + } + $typesString = str_replace(' ', ', ', $type); + $this->assertEquals($typesString, $problem->getTypesAsString()); + } + } + + public function testUnknownProblemType() + { + $yaml = <<assertFalse($ret); + $messagesString = var_export($messages, true); + $this->assertStringContainsString('Unknown problem type', $messagesString); + } + + public function testInvalidProblemType() { + foreach ([ + 'pass-fail scoring', + 'submit-answer multi-pass', + 'submit-answer interactive', + ] as $type) { + $yaml = <<assertFalse($ret); + $messagesString = var_export($messages, true); + $this->assertStringContainsString('Invalid problem type', $messagesString); + } + } + + public function testValidatorFlags() + { + $yaml = <<assertTrue($ret); + $this->assertEmpty($messages); + $this->assertEquals('float_tolerance 1E-6', $problem->getSpecialCompareArgs()); + } + + public function testCustomValidation() + { + foreach (['custom', 'custom interactive', 'custom multi-pass'] as $mode) { + $yaml = <<assertTrue($ret); + $this->assertEmpty($messages); + $this->assertEquals($mode, $validationMode); + if ($mode === 'custom multi-pass') { + $this->assertEquals('pass-fail, multi-pass', $problem->getTypesAsString()); + $this->assertEquals(2, $problem->getMultipassLimit()); + } elseif ($mode === 'custom interactive') { + $this->assertEquals('pass-fail, interactive', $problem->getTypesAsString()); + } else { + $this->assertEquals('pass-fail', $problem->getTypesAsString()); + } + } + } + + public function testMemoryLimit() + { + $yaml = <<assertTrue($ret); + $this->assertEmpty($messages); + $this->assertEquals(1234*1024, $problem->getMemlimit()); + } + + public function testOutputLimit() + { + $yaml = <<assertTrue($ret); + $this->assertEmpty($messages); + $this->assertEquals(4223*1024, $problem->getOutputlimit()); + } + + public function testMultipassLimit() + { + $yaml = <<assertTrue($ret); + $this->assertEmpty($messages); + $this->assertEquals(7, $problem->getMultipassLimit()); + } + + public function testMaximalProblem() { + $yaml = <<assertTrue($ret); + $this->assertEmpty($messages); + $this->assertEquals('pass-fail, multi-pass', $problem->getTypesAsString()); + $this->assertEquals('custom multi-pass', $validationMode); + $this->assertEquals(23*1024, $problem->getMemlimit()); + $this->assertEquals(42*1024, $problem->getOutputlimit()); + $this->assertEquals(3, $problem->getMultipassLimit()); + $this->assertEquals('special flags', $problem->getSpecialCompareArgs()); + } + + public function testMultipleLanguages() { + $yaml = <<assertTrue($ret); + $this->assertEmpty($messages); + $this->assertEquals('english', $problem->getName()); + } + + public function testKattisExample() + { + $yaml = <<assertTrue($ret); + $this->assertEmpty($messages); + $this->assertEquals('Guess the Number', $problem->getName()); + $this->assertEquals('pass-fail, interactive', $problem->getTypesAsString()); + $this->assertEquals('custom interactive', $validationMode); + $this->assertEquals(0, $problem->getTimelimit()); + $this->assertEquals(null, $problem->getMemlimit()); + $this->assertEquals(null, $problem->getOutputlimit()); + } +}