Skip to content

Commit

Permalink
Introduce new exercise-compilation soft exception which does not trig…
Browse files Browse the repository at this point in the history
…ger email sending
  • Loading branch information
Neloop committed Mar 25, 2019
1 parent bc8d760 commit 18aefa0
Show file tree
Hide file tree
Showing 10 changed files with 71 additions and 20 deletions.
16 changes: 12 additions & 4 deletions app/V1Module/presenters/ReferenceExerciseSolutionsPresenter.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
namespace App\V1Module\Presenters;

use App\Exceptions\BadRequestException;
use App\Exceptions\ExerciseCompilationException;
use App\Exceptions\ExerciseCompilationSoftException;
use App\Exceptions\ExerciseConfigException;
use App\Exceptions\InternalServerErrorException;
use App\Exceptions\InvalidArgumentException;
Expand Down Expand Up @@ -475,16 +477,22 @@ private function finishSubmission(
try {
$generatorResult = $this->jobConfigGenerator
->generateJobConfig($this->getCurrentUser(), $exercise, $runtimeEnvironment, $compilationParams);
} catch (Exception $e) {
} catch (ExerciseConfigException | ExerciseCompilationException $e) {
$submission = new ReferenceSolutionSubmission($referenceSolution, null,
"", $this->getCurrentUser(), $isDebug);
$this->referenceSubmissions->persist($submission, false);

$failure = SubmissionFailure::forReferenceSubmission(SubmissionFailure::TYPE_CONFIG_ERROR, $e->getMessage(), $submission);
$failureType = $e instanceof ExerciseCompilationSoftException ? SubmissionFailure::TYPE_SOFT_CONFIG_ERROR : SubmissionFailure::TYPE_CONFIG_ERROR;
$sendEmail = $e instanceof ExerciseCompilationSoftException ? false : true;

$failure = SubmissionFailure::forReferenceSubmission($failureType, $e->getMessage(), $submission);
$this->submissionFailures->persist($failure);

$reportMessage = "Reference submission '{$submission->getId()}' errored - {$e->getMessage()}";
$this->failureHelper->report(FailureHelper::TYPE_API_ERROR, $reportMessage);
if ($sendEmail) {
$reportMessage = "Reference submission '{$submission->getId()}' errored - {$e->getMessage()}";
$this->failureHelper->report(FailureHelper::TYPE_API_ERROR, $reportMessage);
}

throw $e; // rethrow
}

Expand Down
22 changes: 16 additions & 6 deletions app/V1Module/presenters/SubmitPresenter.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

namespace App\V1Module\Presenters;

use App\Exceptions\ExerciseCompilationException;
use App\Exceptions\ExerciseCompilationSoftException;
use App\Exceptions\ExerciseConfigException;
use App\Exceptions\ForbiddenRequestException;
use App\Exceptions\JobConfigStorageException;
Expand Down Expand Up @@ -254,15 +256,20 @@ public function actionSubmit(string $id) {
* @param Exception $exception
* @param string $failureType
* @param string $reportType
* @param bool $sendEmail
* @throws Exception
*/
private function submissionFailed(AssignmentSolutionSubmission $submission, Exception $exception,
string $failureType = SubmissionFailure::TYPE_BROKER_REJECT,
string $reportType = FailureHelper::TYPE_BACKEND_ERROR) {
string $reportType = FailureHelper::TYPE_BACKEND_ERROR,
bool $sendEmail = true) {
$failure = SubmissionFailure::forSubmission($failureType, $exception->getMessage(), $submission);
$this->submissionFailures->persist($failure);
$reportMessage = "Submission '{$submission->getId()}' errored - {$exception->getMessage()}";
$this->failureHelper->report($reportType, $reportMessage);

if ($sendEmail) {
$reportMessage = "Submission '{$submission->getId()}' errored - {$exception->getMessage()}";
$this->failureHelper->report($reportType, $reportMessage);
}
throw $exception; // rethrow
}

Expand Down Expand Up @@ -297,11 +304,14 @@ private function finishSubmission(AssignmentSolution $solution, bool $isDebug =
$solution->getAssignment(),
$solution->getSolution()->getRuntimeEnvironment(),
$compilationParams);
} catch (Exception $e) {
} catch (ExerciseConfigException | ExerciseCompilationException $e) {
$submission = new AssignmentSolutionSubmission($solution, "", $this->getCurrentUser(), $isDebug);
$this->assignmentSubmissions->persist($submission, false);
$this->submissionFailed($submission, $e, SubmissionFailure::TYPE_CONFIG_ERROR,
FailureHelper::TYPE_API_ERROR);

$failureType = $e instanceof ExerciseCompilationSoftException ? SubmissionFailure::TYPE_SOFT_CONFIG_ERROR : SubmissionFailure::TYPE_CONFIG_ERROR;
$sendEmail = $e instanceof ExerciseCompilationSoftException ? false : true;

$this->submissionFailed($submission, $e, $failureType, FailureHelper::TYPE_API_ERROR, $sendEmail);
// this return is here just to fool static analysis,
// submissionFailed method throws an exception and therefore following return is never reached
return [];
Expand Down
5 changes: 3 additions & 2 deletions app/exceptions/ExerciseCompilationException.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ class ExerciseCompilationException extends ApiException {
/**
* Create instance with further description.
* @param string $msg description
* @param int $code
*/
public function __construct(string $msg = 'Please contact system administrator') {
parent::__construct("Exercise compilation error - $msg", IResponse::S500_INTERNAL_SERVER_ERROR);
public function __construct(string $msg = 'Please contact system administrator', $code = IResponse::S500_INTERNAL_SERVER_ERROR) {
parent::__construct("Exercise compilation error - $msg", $code);
}
}
20 changes: 20 additions & 0 deletions app/exceptions/ExerciseCompilationSoftException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?php

namespace App\Exceptions;

use Nette\Http\IResponse;

/**
* Exception used in exercise compilation to job configuration.
* Used if user inputs are incorrect or invalid (e.g. uploaded files).
*/
class ExerciseCompilationSoftException extends ExerciseCompilationException {

/**
* Constructor.
* @param string $msg description
*/
public function __construct(string $msg = 'Please contact system administrator') {
parent::__construct($msg, IResponse::S400_BAD_REQUEST);
}
}
7 changes: 4 additions & 3 deletions app/helpers/ExerciseConfig/Compilation/VariablesResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace App\Helpers\ExerciseConfig\Compilation;

use App\Exceptions\ExerciseCompilationException;
use App\Exceptions\ExerciseCompilationSoftException;
use App\Exceptions\ExerciseConfigException;
use App\Helpers\ExerciseConfig\Compilation\Tree\MergeTree;
use App\Helpers\ExerciseConfig\Compilation\Tree\PortNode;
Expand Down Expand Up @@ -38,7 +39,7 @@ private function resolveSubmitVariableReference(?Variable $variable, Compilation
// but user which submitted solution did not provide the value for reference
$reference = $variable->getReference();
if (!$params->getSolutionParams()->getVariable($reference)) {
throw new ExerciseCompilationException("Variable '{$reference}' was not provided on submit");
throw new ExerciseCompilationSoftException("Variable '{$reference}' was not provided on submit");
}

// set user provided variable to actual variable
Expand All @@ -48,7 +49,7 @@ private function resolveSubmitVariableReference(?Variable $variable, Compilation
if ($variable->isFile()) {
foreach ($variable->getValueAsArray() as $value) {
if (!in_array($value, $params->getFiles())) {
throw new ExerciseCompilationException("File '{$value}' in variable '{$reference}' could not be found among submitted files");
throw new ExerciseCompilationSoftException("File '{$value}' in variable '{$reference}' could not be found among submitted files");
}
}
}
Expand Down Expand Up @@ -79,7 +80,7 @@ private function resolveFileInputsRegexp(?Variable $variable,

if (empty($matches)) {
// there were no matches, but variable value cannot be empty!
throw new ExerciseCompilationException("None of the submitted files matched regular expression '{$value}' in variable '{$variable->getName()}'");
throw new ExerciseCompilationSoftException("None of the submitted files matched regular expression '{$value}' in variable '{$variable->getName()}'");
}

// construct resulting variable from given variable info
Expand Down
3 changes: 2 additions & 1 deletion app/helpers/ExerciseConfig/Compiler.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace App\Helpers\ExerciseConfig;

use App\Exceptions\ExerciseCompilationException;
use App\Exceptions\ExerciseCompilationSoftException;
use App\Exceptions\ExerciseConfigException;
use App\Helpers\Evaluation\IExercise;
use App\Helpers\ExerciseConfig\Compilation\BaseCompiler;
Expand Down Expand Up @@ -62,7 +63,7 @@ public function compile(IExercise $exercise,
// check submitted files if they are unique
$uniqueFiles = array_unique($params->getFiles());
if (count($params->getFiles()) !== count($uniqueFiles)) {
throw new ExerciseCompilationException("Submitted files contains two or more files with the same name.");
throw new ExerciseCompilationSoftException("Submitted files contains two or more files with the same name.");
}

if ($exercise->getExerciseConfig() === null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace App\Helpers\ExerciseConfig\Pipeline\Box;

use App\Exceptions\ExerciseCompilationException;
use App\Exceptions\ExerciseCompilationSoftException;
use App\Exceptions\ExerciseConfigException;
use App\Helpers\ExerciseConfig\Compilation\CompilationParams;
use App\Helpers\ExerciseConfig\Pipeline\Box\Params\BoxCategories;
Expand Down Expand Up @@ -114,7 +115,6 @@ protected function compileInternal(?Variable $inputVariable,
* @param string $local
* @param CompilationParams $params
* @return Task
* @throws ExerciseConfigException
* @throws ExerciseCompilationException
*/
protected function compileTask(bool $isRemote, string $input, string $local, CompilationParams $params): Task {
Expand All @@ -125,7 +125,7 @@ protected function compileTask(bool $isRemote, string $input, string $local, Com
// check if soon-to-be fetched file does not collide with files given by user
$basename = basename($local);
if (in_array($basename, $params->getFiles())) {
throw new ExerciseCompilationException("File '{$basename}' is already defined by author of the exercise");
throw new ExerciseCompilationSoftException("File '{$basename}' is already defined by author of the exercise");
}

// remote file has to have fetch task
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace App\Helpers\ExerciseConfig\Pipeline\Box;

use App\Exceptions\ExerciseCompilationException;
use App\Exceptions\ExerciseCompilationSoftException;
use App\Exceptions\ExerciseConfigException;
use App\Helpers\ExerciseConfig\Compilation\CompilationParams;
use App\Helpers\ExerciseConfig\Pipeline\Box\Params\BoxCategories;
Expand Down Expand Up @@ -64,7 +65,7 @@ protected function compileInternal(Variable $remote, Variable $local, Compilatio

// check if soon-to-be fetched file does not collide with files given by user
if (in_array($basename, $params->getFiles())) {
throw new ExerciseCompilationException("File '{$basename}' is already defined by author of the exercise");
throw new ExerciseCompilationSoftException("File '{$basename}' is already defined by author of the exercise");
}

// create task
Expand Down
2 changes: 1 addition & 1 deletion app/model/entity/ReferenceSolutionSubmission.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public function jsonSerialize() {
}

$failures = $this->getFailures()->filter(function (SubmissionFailure $failure) {
return $failure->getType() === SubmissionFailure::TYPE_CONFIG_ERROR;
return $failure->isConfigErrorFailure();
})->map(function (SubmissionFailure $failure) {
return $failure->toSimpleArray();
})->toArray();
Expand Down
9 changes: 9 additions & 0 deletions app/model/entity/SubmissionFailure.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ class SubmissionFailure implements JsonSerializable {
*/
const TYPE_CONFIG_ERROR = "config_error";

/**
* The exercise configuration is invalid and it cannot be compiled, due to user error
*/
const TYPE_SOFT_CONFIG_ERROR = "soft_config_error";

/**
* @ORM\Id
* @ORM\Column(type="guid")
Expand Down Expand Up @@ -120,6 +125,10 @@ public function getSubmission(): Submission {
return $this->assignmentSolutionSubmission ?? $this->referenceSolutionSubmission;
}

public function isConfigErrorFailure(): bool {
return $this->type === self::TYPE_CONFIG_ERROR || $this->type === self::TYPE_SOFT_CONFIG_ERROR;
}

public function toSimpleArray() {
return [
"description" => $this->description,
Expand Down

0 comments on commit 18aefa0

Please sign in to comment.