diff --git a/app/V1Module/presenters/ExercisesConfigPresenter.php b/app/V1Module/presenters/ExercisesConfigPresenter.php index 2fec70812..50643002e 100644 --- a/app/V1Module/presenters/ExercisesConfigPresenter.php +++ b/app/V1Module/presenters/ExercisesConfigPresenter.php @@ -653,12 +653,10 @@ public function actionSetScoreConfig(string $id) { // validate score configuration $calculator = $this->calculators->getCalculator($exercise->getScoreCalculator()); - if (!$calculator->isScoreConfigValid($config)) { - throw new ExerciseConfigException("Exercise score configuration is not valid"); - } + $normalizedConfig = $calculator->validateAndNormalizeScore($config); // throws if validation fails $exercise->updatedNow(); - $exercise->setScoreConfig($config); + $exercise->setScoreConfig($normalizedConfig); $this->exercises->flush(); // check exercise configuration diff --git a/app/helpers/Evaluation/ScoreCalculators/IScoreCalculator.php b/app/helpers/Evaluation/ScoreCalculators/IScoreCalculator.php index deecfb3f3..c5e7d52b7 100644 --- a/app/helpers/Evaluation/ScoreCalculators/IScoreCalculator.php +++ b/app/helpers/Evaluation/ScoreCalculators/IScoreCalculator.php @@ -2,6 +2,7 @@ namespace App\Helpers\Evaluation; use App\Exceptions\SubmissionEvaluationFailedException; +use App\Exceptions\ExerciseConfigException; /** * Interface for score computations. Purpose is to merge scores of all @@ -24,6 +25,15 @@ public function computeScore(string $scoreConfig, array $testResults): float; */ public function isScoreConfigValid(string $scoreConfig): bool; + /** + * Performs validation and normalization on config string. + * This should be used instead of validation when the score config is processed as API input. + * @param string $scoreConfig YAML configuration for the score calculator + * @return string Normalized and polished YAML with score configuration + * @throws ExerciseConfigException + */ + public function validateAndNormalizeScore(string $scoreConfig): string; + /** * Make default configuration for array of test names. Each test will * have the same priority as others. diff --git a/app/helpers/Evaluation/ScoreCalculators/SimpleScoreCalculator.php b/app/helpers/Evaluation/ScoreCalculators/SimpleScoreCalculator.php index f1fd42593..089bc378e 100644 --- a/app/helpers/Evaluation/ScoreCalculators/SimpleScoreCalculator.php +++ b/app/helpers/Evaluation/ScoreCalculators/SimpleScoreCalculator.php @@ -3,6 +3,7 @@ namespace App\Helpers\Evaluation; use App\Exceptions\SubmissionEvaluationFailedException; +use App\Exceptions\ExerciseConfigException; use App\Helpers\Evaluation\IScoreCalculator; use Symfony\Component\Yaml\Yaml; use Symfony\Component\Yaml\Exception\ParseException; @@ -17,6 +18,32 @@ * weights can be arbitrary, what matters are ratios. */ class SimpleScoreCalculator implements IScoreCalculator { + /** + * Internal function that safely retrieves score config weights. + * @param string $scoreConfig + * @return array|null Null if the config is invalid, name => weight array otherwise. + */ + private function getTestWeights(string $scoreConfig): ?array { + try { + $config = Yaml::parse($scoreConfig); + $normalizedWeights = []; + + if (isset($config['testWeights']) && is_array($config['testWeights'])) { + foreach ($config['testWeights'] as $name => $value) { + if (!is_integer($value)) { + return null; + } + $normalizedWeights[trim($name)] = $value; + } + } else { + return null; + } + } catch (ParseException $e) { + return null; + } + + return $normalizedWeights; + } /** * Function that computes the resulting score from simple YML config and test results score @@ -26,13 +53,11 @@ class SimpleScoreCalculator implements IScoreCalculator { * @throws SubmissionEvaluationFailedException */ public function computeScore(string $scoreConfig, array $testResults): float { - if (!$this->isScoreConfigValid($scoreConfig)) { + $weights = $this->getTestWeights($scoreConfig); + if ($weights === null) { throw new SubmissionEvaluationFailedException("Assignment score configuration is invalid"); } - $config = Yaml::parse($scoreConfig); - $weights = $config['testWeights']; - // assign zero ratio to all tests which does not have specified value foreach ($testResults as $name => $score) { if (!array_key_exists($name, $weights)) { @@ -57,23 +82,22 @@ public function computeScore(string $scoreConfig, array $testResults): float { * @return bool If the configuration is valid or not */ public function isScoreConfigValid(string $scoreConfig): bool { - try { - $config = Yaml::parse($scoreConfig); + return $this->getTestWeights($scoreConfig) !== null; + } - if (isset($config['testWeights']) && is_array($config['testWeights'])) { - foreach ($config['testWeights'] as $value) { - if (!is_integer($value)) { - return false; - } - } - } else { - return false; - } - } catch (ParseException $e) { - return false; + /** + * Performs validation and normalization on config string. + * This should be used instead of validation when the score config is processed as API input. + * @param string $scoreConfig YAML configuration for the score calculator + * @return string Normalized and polished YAML with score configuration + * @throws ExerciseConfigException + */ + public function validateAndNormalizeScore(string $scoreConfig): string { + $weights = $this->getTestWeights($scoreConfig); + if ($weights === null) { + throw new ExerciseConfigException("Exercise score configuration is not valid"); } - - return true; + return Yaml::dump([ 'testWeights' => $weights ]); } /** diff --git a/tests/Presenters/ExercisesConfigPresenter.phpt b/tests/Presenters/ExercisesConfigPresenter.phpt index 29e80c2de..ab1c7ca66 100644 --- a/tests/Presenters/ExercisesConfigPresenter.phpt +++ b/tests/Presenters/ExercisesConfigPresenter.phpt @@ -9,6 +9,7 @@ use App\Model\Entity\ExerciseTest; use App\Model\Entity\HardwareGroup; use App\V1Module\Presenters\ExercisesConfigPresenter; use Tester\Assert; +use Symfony\Component\Yaml\Yaml; /** * @testCase @@ -515,8 +516,9 @@ class TestExercisesConfigPresenter extends Tester\TestCase $result = $response->getPayload(); Assert::equal(200, $result['code']); - $payload = $result['payload']; - Assert::equal("testWeights:\n \"Test 1\": 100\n \"Test 2\": 100\n \"Test 3\": 100", $payload); + $payload = Yaml::parse($result['payload']); + $expected = Yaml::parse($config); + Assert::equal($expected, $payload); } public function testGetTests() {