From 08d07436ddebca0f5fe1c35d9c925354a52109eb Mon Sep 17 00:00:00 2001 From: Nils Haagen Date: Tue, 19 Nov 2024 16:14:33 +0100 Subject: [PATCH 01/13] Test: Make Access To Evaluation More Efficient See: https://mantis.ilias.de/view.php?id=33607 --- .../Evaluation/class.ilTestEvaluationData.php | 220 ++++++++++ .../class.ilTestEvaluationFactory.php | 348 ++++++++++++++++ .../class.ilTestEvaluationPassData.php | 14 +- .../class.ilTestEvaluationUserData.php | 12 +- .../ILIAS/Test/classes/class.ilObjTest.php | 180 +------- .../Test/classes/class.ilTestEvaluation.php | 75 ---- .../classes/class.ilTestEvaluationData.php | 383 ------------------ .../Test/classes/class.ilTestExportGUI.php | 3 +- .../Test/src/Results/Data/AttemptOverview.php | 8 +- .../ILIAS/Test/src/Results/Data/Factory.php | 4 - .../Test/tests/ilTestEvaluationDataTest.php | 107 ++++- .../ILIAS/Test/tests/ilTestEvaluationTest.php | 43 -- 12 files changed, 671 insertions(+), 726 deletions(-) create mode 100644 components/ILIAS/Test/classes/Evaluation/class.ilTestEvaluationData.php create mode 100644 components/ILIAS/Test/classes/Evaluation/class.ilTestEvaluationFactory.php rename components/ILIAS/Test/classes/{ => Evaluation}/class.ilTestEvaluationPassData.php (96%) mode change 100755 => 100644 rename components/ILIAS/Test/classes/{ => Evaluation}/class.ilTestEvaluationUserData.php (98%) mode change 100755 => 100644 delete mode 100755 components/ILIAS/Test/classes/class.ilTestEvaluation.php delete mode 100755 components/ILIAS/Test/classes/class.ilTestEvaluationData.php delete mode 100755 components/ILIAS/Test/tests/ilTestEvaluationTest.php diff --git a/components/ILIAS/Test/classes/Evaluation/class.ilTestEvaluationData.php b/components/ILIAS/Test/classes/Evaluation/class.ilTestEvaluationData.php new file mode 100644 index 000000000000..ce4cd124019a --- /dev/null +++ b/components/ILIAS/Test/classes/Evaluation/class.ilTestEvaluationData.php @@ -0,0 +1,220 @@ + $participants + */ + public function __construct( + protected array $participants + ) { + } + + public function setDatasets(int $datasets): void + { + $this->datasets = $datasets; + } + + public function getDatasets(): int + { + return $this->datasets; + } + + public function addQuestionTitle(int $question_id, string $question_title): void + { + $this->question_titles[$question_id] = $question_title; + } + + /** + * @return array + */ + public function getQuestionTitles(): array + { + return $this->question_titles; + } + + public function getQuestionTitle(?int $question_id): string + { + if (array_key_exists($question_id, $this->question_titles)) { + return $this->question_titles[$question_id]; + } + + return ''; + } + + public function getTotalFinishedParticipants(): int + { + $finishedParticipants = 0; + + foreach ($this->participants as $active_id => $participant) { + if (!$participant->isSubmitted()) { + continue; + } + + $finishedParticipants++; + } + + return $finishedParticipants; + } + + /** + * @return array + */ + public function getParticipants(): array + { + if (is_array($this->arr_filter) && count($this->arr_filter) > 0) { + $filtered_participants = []; + $courseids = []; + $groupids = []; + + if (array_key_exists(self::FILTER_BY_GROUP, $this->arr_filter)) { + $ids = ilObject::_getIdsForTitle($this->arr_filter[self::FILTER_BY_GROUP], 'grp', true); + $groupids = array_merge($groupids, $ids); + } + if (array_key_exists(self::FILTER_BY_COURSE, $this->arr_filter)) { + $ids = ilObject::_getIdsForTitle($this->arr_filter[self::FILTER_BY_COURSE], 'crs', true); + $courseids = array_merge($courseids, $ids); + } + foreach ($this->participants as $active_id => $participant) { + $remove = false; + if (array_key_exists(self::FILTER_BY_NAME, $this->arr_filter)) { + if (!(strpos(strtolower($participant->getName()), strtolower((string) $this->arr_filter[self::FILTER_BY_NAME])) !== false)) { + $remove = true; + } + } + if (!$remove) { + if (array_key_exists(self::FILTER_BY_GROUP, $this->arr_filter)) { + $groups = ilParticipants::_getMembershipByType($participant->getUserID(), ['grp']); + $foundfilter = false; + if (count(array_intersect($groupids, $groups))) { + $foundfilter = true; + } + if (!$foundfilter) { + $remove = true; + } + } + } + if (!$remove) { + if (array_key_exists(self::FILTER_BY_COURSE, $this->arr_filter)) { + $courses = ilParticipants::_getMembershipByType($participant->getUserID(), ['crs']); + $foundfilter = false; + if (count(array_intersect($courseids, $courses))) { + $foundfilter = true; + } + if (!$foundfilter) { + $remove = true; + } + } + } + if (!$remove) { + if (array_key_exists(self::FILTER_BY_ACTIVE_ID, $this->arr_filter)) { + if ($active_id != $this->arr_filter[self::FILTER_BY_ACTIVE_ID]) { + $remove = true; + } + } + } + if (!$remove) { + $filtered_participants[$active_id] = $participant; + } + } + return $filtered_participants; + } else { + return $this->participants; + } + } + + public function resetFilter(): void + { + $this->arr_filter = []; + } + + public function setFilter(string $by, string $text): void + { + if (in_array( + $by, + [self::FILTER_BY_ACTIVE_ID, self::FILTER_BY_NAME, self::FILTER_BY_COURSE, self::FILTER_BY_GROUP], + true + )) { + $this->arr_filter = [$by => $text]; + } + } + + public function setFilterArray(array $arr_filter): void + { + $this->arr_filter = $arr_filter; + } + + public function addParticipant(int $active_id, ilTestEvaluationUserData $participant): void + { + $this->participants[$active_id] = $participant; + } + + public function getParticipant(int $active_id): ?ilTestEvaluationUserData + { + return $this->participants[$active_id] ?? null; + } + + public function participantExists($active_id): bool + { + return array_key_exists($active_id, $this->participants); + } + + public function removeParticipant($active_id) + { + unset($this->participants[$active_id]); + } + + public function getStatistics(): Statistics + { + if ($this->statistics === null) { + $this->statistics = new Statistics($this); + } + return $this->statistics; + } + + public function getParticipantIds(): array + { + return array_keys($this->participants); + } +} diff --git a/components/ILIAS/Test/classes/Evaluation/class.ilTestEvaluationFactory.php b/components/ILIAS/Test/classes/Evaluation/class.ilTestEvaluationFactory.php new file mode 100644 index 000000000000..0912264ab70a --- /dev/null +++ b/components/ILIAS/Test/classes/Evaluation/class.ilTestEvaluationFactory.php @@ -0,0 +1,348 @@ + + */ + private function getAccessFilteredActiveIds(): array + { + if (($participants_list = $this->test_obj->getAccessFilteredParticipantList()) !== null) { + return $participants_list->getAllActiveIds(); + } + $participants = $this->test_obj->getTestParticipants(); + return array_keys($participants); + } + + /** + * @param list $active_ids + */ + private function retrieveEvaluationData(array $active_ids): \Generator + { + $query = ' + SELECT tst_test_result.question_fi, + tst_test_result.points result_points, + tst_test_result.answered, + tst_test_result.manual, + + qpl_questions.original_id, + qpl_questions.title questiontitle, + qpl_questions.points qpl_maxpoints, + + tst_active.submitted, + tst_active.last_finished_pass, + tst_pass_result.*, + + usr_data.usr_id, + usr_data.firstname, + usr_data.lastname, + usr_data.title, + usr_data.login + + FROM tst_test_result, qpl_questions, tst_active + + LEFT JOIN tst_pass_result ON tst_active.active_id = tst_pass_result.active_fi + LEFT JOIN usr_data ON tst_active.user_fi = usr_data.usr_id + + WHERE tst_active.active_id = tst_test_result.active_fi + AND qpl_questions.question_id = tst_test_result.question_fi + AND tst_active.test_fi = %s + AND %s + + ORDER BY tst_active.active_id ASC, tst_test_result.pass ASC, tst_test_result.tstamp DESC + '; + + $ret = []; + $result = $this->db->query( + sprintf( + $query, + $this->db->quote($this->test_obj->getTestId(), 'integer'), + $this->db->in('tst_active.active_id', $active_ids, false, 'integer'), + ) + ); + while ($row = $this->db->fetchAssoc($result)) { + yield $row; + } + } + + public function getEvaluationData(): ilTestEvaluationData + { + $eval_data_rows = $this->retrieveEvaluationData($this->getAccessFilteredActiveIds()); + $scoring_settings = $this->test_obj->getPassScoring(); + $participants = []; + $current_user = null; + + foreach ($eval_data_rows as $row) { + + if ($current_user != $row['active_fi']) { + $current_user = $row['active_fi']; + + $user_eval_data = new ilTestEvaluationUserData($scoring_settings); + + $user_eval_data->setName( + $this->test_obj->buildName($row['usr_id'], $row['firstname'], $row['lastname'], $row['title']) + ); + + if ($row['login'] !== null) { + $user_eval_data->setLogin($row['login']); + } + if ($row['usr_id'] !== null) { + $user_eval_data->setUserID($row['usr_id']); + } + $user_eval_data->setSubmitted((bool) $row['submitted']); + $user_eval_data->setLastFinishedPass($row['last_finished_pass']); + + + $visiting_time = $this->test_obj->getVisitingTimeOfParticipant($row['active_fi']); + $user_eval_data->setFirstVisit($visiting_time["first_access"]); + $user_eval_data->setLastVisit($visiting_time["last_access"]); + + $pass = new \ilTestEvaluationPassData(); + $pass->setPass($row['pass']); + $pass->setReachedPoints($row['points']); + + if ($row['questioncount'] == 0) { + list($count, $points) = array_values( + $this->test_obj->getQuestionCountAndPointsForPassOfParticipant($row['active_fi'], $row['pass']) + ); + $pass->setMaxPoints($points); + $pass->setQuestionCount($count); + } else { + $pass->setMaxPoints($row['maxpoints']); + $pass->setQuestionCount($row['questioncount']); + } + + $pass->setNrOfAnsweredQuestions($row['answeredquestions']); + $pass->setWorkingTime($row['workingtime']); + $pass->setExamId((string) $row['exam_id']); + $pass->setRequestedHintsCount($row['hint_count']); + $pass->setDeductedHintPoints($row['hint_points']); + } + + $pass->addAnsweredQuestion( + $row["question_fi"], + $row["qpl_maxpoints"], + $row["result_points"], + (bool) $row['answered'], + null, + $row['manual'] + ); + + $user_eval_data->addPass($row['pass'], $pass); + + $participants[$row['active_fi']] = $user_eval_data; + } + + $evaluation_data = $this->addQuestionsToParticipantPasses(new ilTestEvaluationData($participants)); + return $this->addMarksToParticipants($evaluation_data); + } + + + private function addQuestionsToParticipantPasses(ilTestEvaluationData $evaluation_data): ilTestEvaluationData + { + foreach ($evaluation_data->getParticipantIds() as $active_id) { + $user_eval_data = $evaluation_data->getParticipant($active_id); + $add_user_questions = $this->test_obj->isRandomTest() ? + $this->retrieveQuestionsForParticipantPassesForRandomTests( + $active_id, + $user_eval_data, + $this->test_obj->getQuestionCountWithoutReloading() + ) : + $this->retrieveQuestionsForParticipantPassesForSequencedTests($active_id); + + foreach ($add_user_questions as $q) { + $user_eval_data->addQuestion( + $q['original_id'], + $q['question_id'], + $q['max_points'], + $q['sequence'], + $q['pass'] + ); + $evaluation_data->addQuestionTitle($q['question_id'], $q['title']); + } + } + return $evaluation_data; + } + + private function retrieveQuestionsForParticipantPassesForRandomTests( + int $active_id, + ilTestEvaluationUserData $user_eval_data, + int $question_count + ): array { + $ret = []; + for ($testpass = 0; $testpass <= $user_eval_data->getLastPass(); $testpass++) { + $this->db->setLimit($question_count, 0); + $query = ' + SELECT tst_test_rnd_qst.sequence, tst_test_rnd_qst.question_fi, qpl_questions.original_id, + tst_test_rnd_qst.pass, qpl_questions.points, qpl_questions.title + FROM tst_test_rnd_qst, qpl_questions + WHERE tst_test_rnd_qst.question_fi = qpl_questions.question_id + AND tst_test_rnd_qst.pass = %s + AND tst_test_rnd_qst.active_fi = %s ORDER BY tst_test_rnd_qst.sequence + '; + + $result = $this->db->queryF( + $query, + ['integer','integer'], + [$testpass, $active_id] + ); + + if ($result->numRows()) { + while ($row = $this->db->fetchAssoc($result)) { + $tpass = array_key_exists('pass', $row) ? $row['pass'] : 0; + + if ( + !isset($row['question_fi'], $row['points'], $row['sequence']) || + !is_numeric($row['question_fi']) || !is_numeric($row['points']) || !is_numeric($row['sequence']) + ) { + continue; + } + + $ret[] = [ + 'original_id' => (int) $row['original_id'], + 'question_id' => (int) $row['question_fi'], + 'max_points' => (float) $row['points'], + 'sequence' => (int) $row['sequence'], + 'pass' => $tpass, + 'title' => $row['title'] + ]; + } + } + } + return $ret; + } + + private function retrieveQuestionsForParticipantPassesForSequencedTests( + int $active_id + ): array { + $ret = []; + + $query = ' + SELECT tst_test_question.sequence, tst_test_question.question_fi, + qpl_questions.points, qpl_questions.title, qpl_questions.original_id + FROM tst_test_question, tst_active, qpl_questions + WHERE tst_test_question.question_fi = qpl_questions.question_id + AND tst_active.active_id = %s + AND tst_active.test_fi = tst_test_question.test_fi + ORDER BY tst_test_question.sequence + '; + + $result = $this->db->queryF( + $query, + ['integer'], + [$active_id] + ); + + if ($result->numRows()) { + $questionsbysequence = []; + while ($row = $this->db->fetchAssoc($result)) { + $questionsbysequence[$row['sequence']] = $row; + } + + $seqresult = $this->db->queryF( + 'SELECT * FROM tst_sequence WHERE active_fi = %s', + ['integer'], + [$active_id] + ); + + while ($seqrow = $this->db->fetchAssoc($seqresult)) { + $questionsequence = unserialize($seqrow["sequence"]); + foreach ($questionsequence as $sidx => $seq) { + if (!isset($questionsbysequence[$seq])) { + continue; + } + $ret[] = [ + 'original_id' => $questionsbysequence[$seq]['original_id'] ?? 0, + 'question_id' => $questionsbysequence[$seq]['question_fi'], + 'max_points' => $questionsbysequence[$seq]['points'], + 'sequence' => $sidx + 1, + 'pass' => $seqrow['pass'], + 'title' => $questionsbysequence[$seq]["title"] + ]; + } + } + } + return $ret; + } + + private function addMarksToParticipants(ilTestEvaluationData $evaluation_data): ilTestEvaluationData + { + $mark_schema = $this->test_obj->getMarkSchema(); + + foreach ($evaluation_data->getParticipantIds() as $active_id) { + $user_eval_data = $evaluation_data->getParticipant($active_id); + + $percentage = $user_eval_data->getReachedPointsInPercent(); + $mark = $mark_schema->getMatchingMark($percentage); + + if ($mark !== null) { + $user_eval_data->setMark($mark); + for ($i = 0;$i < $user_eval_data->getPassCount();$i++) { + $pass_data = $user_eval_data->getPass($i); + $mark = $mark_schema->getMatchingMark( + $pass_data->getReachedPointsInPercent() + ); + if ($mark !== null) { + $pass_data->setMark($mark); + } + } + } + + } + + return $evaluation_data; + } + + public function getAllActivesPasses(): array + { + $query = ' + SELECT active_fi, pass + FROM tst_active actives + INNER JOIN tst_pass_result passes + ON active_fi = active_id + WHERE test_fi = %s + '; + + $res = $this->db->queryF($query, ['integer'], [$this->test_obj->getTestId()]); + + $passes = []; + while ($row = $this->db->fetchAssoc($res)) { + if (!isset($passes[$row['active_fi']])) { + $passes[$row['active_fi']] = []; + } + + $passes[$row['active_fi']][] = $row['pass']; + } + + return $passes; + } +} diff --git a/components/ILIAS/Test/classes/class.ilTestEvaluationPassData.php b/components/ILIAS/Test/classes/Evaluation/class.ilTestEvaluationPassData.php old mode 100755 new mode 100644 similarity index 96% rename from components/ILIAS/Test/classes/class.ilTestEvaluationPassData.php rename to components/ILIAS/Test/classes/Evaluation/class.ilTestEvaluationPassData.php index eb49fcd9ff13..2d0e6dd4843a --- a/components/ILIAS/Test/classes/class.ilTestEvaluationPassData.php +++ b/components/ILIAS/Test/classes/Evaluation/class.ilTestEvaluationPassData.php @@ -22,18 +22,8 @@ use ILIAS\Test\Scoring\Marks\Mark; /** -* Class ilTestEvaluationPassData -* -* @author Helmut Schottmüller -* @author Björn Heyser -* @version $Id$ -* -* @throws ilTestEvaluationException -* -* @defgroup ModulesTest Modules/Test -* @extends ilObject -*/ - + * @deprecated 11; Result/EvaluationData will be refined. + */ class ilTestEvaluationPassData { /** diff --git a/components/ILIAS/Test/classes/class.ilTestEvaluationUserData.php b/components/ILIAS/Test/classes/Evaluation/class.ilTestEvaluationUserData.php old mode 100755 new mode 100644 similarity index 98% rename from components/ILIAS/Test/classes/class.ilTestEvaluationUserData.php rename to components/ILIAS/Test/classes/Evaluation/class.ilTestEvaluationUserData.php index b4f0656b973f..f09f4078639d --- a/components/ILIAS/Test/classes/class.ilTestEvaluationUserData.php +++ b/components/ILIAS/Test/classes/Evaluation/class.ilTestEvaluationUserData.php @@ -21,16 +21,8 @@ use ILIAS\Test\Scoring\Marks\Mark; /** -* Class ilTestEvaluationUserData -* -* @author Helmut Schottmüller -* @author Björn Heyser -* @version $Id$ -* -* @defgroup ModulesTest Modules/Test -* @extends ilObject -*/ - + * @deprecated 11; Result/EvaluationData will be refined. + */ class ilTestEvaluationUserData { private array $question_titles; diff --git a/components/ILIAS/Test/classes/class.ilObjTest.php b/components/ILIAS/Test/classes/class.ilObjTest.php index eaf33b1bdc9e..cfc42eddc99a 100755 --- a/components/ILIAS/Test/classes/class.ilObjTest.php +++ b/components/ILIAS/Test/classes/class.ilObjTest.php @@ -2470,179 +2470,8 @@ public function buildStatisticsAccessFilteredParticipantList(): ilTestParticipan public function getUnfilteredEvaluationData(): ilTestEvaluationData { - $data = new ilTestEvaluationData($this->db, $this); - - $query = " - SELECT tst_test_result.*, - qpl_questions.original_id, - qpl_questions.title questiontitle, - qpl_questions.points maxpoints - - FROM tst_test_result, qpl_questions, tst_active - - WHERE tst_active.active_id = tst_test_result.active_fi - AND qpl_questions.question_id = tst_test_result.question_fi - AND tst_active.test_fi = %s - - ORDER BY tst_active.active_id ASC, tst_test_result.pass ASC, tst_test_result.tstamp DESC - "; - - $result = $this->db->queryF( - $query, - ['integer'], - [$this->getTestId()] - ); - - while ($row = $this->db->fetchAssoc($result)) { - if (!$data->participantExists($row["active_fi"])) { - continue; - } - - $participant_object = $data->getParticipant($row["active_fi"]); - $pass_object = $participant_object->getPass($row["pass"]); - - if (!($pass_object instanceof ilTestEvaluationPassData)) { - continue; - } - - $pass_object->addAnsweredQuestion( - $row['question_fi'], - $row['maxpoints'], - $row['points'], - (bool) $row['answered'], - null, - $row['manual'] - ); - } - - foreach (array_keys($data->getParticipants()) as $active_id) { - if ($this->isRandomTest()) { - for ($testpass = 0; $testpass <= $data->getParticipant($active_id)->getLastPass(); $testpass++) { - $this->db->setLimit($this->getQuestionCount(), 0); - - $query = " - SELECT tst_test_rnd_qst.sequence, tst_test_rnd_qst.question_fi, qpl_questions.original_id, - tst_test_rnd_qst.pass, qpl_questions.points, qpl_questions.title - FROM tst_test_rnd_qst, qpl_questions - WHERE tst_test_rnd_qst.question_fi = qpl_questions.question_id - AND tst_test_rnd_qst.pass = %s - AND tst_test_rnd_qst.active_fi = %s ORDER BY tst_test_rnd_qst.sequence - "; - - $result = $this->db->queryF( - $query, - ['integer','integer'], - [$testpass, $active_id] - ); - - if ($result->numRows()) { - while ($row = $this->db->fetchAssoc($result)) { - $tpass = array_key_exists("pass", $row) ? $row["pass"] : 0; - - if ( - !isset($row["question_fi"], $row["points"], $row["sequence"]) || - !is_numeric($row["question_fi"]) || !is_numeric($row["points"]) || !is_numeric($row["sequence"]) - ) { - continue; - } - - $data->getParticipant($active_id)->addQuestion( - (int) $row["original_id"], - (int) $row["question_fi"], - (float) $row["points"], - (int) $row["sequence"], - $tpass - ); - - $data->addQuestionTitle($row["question_fi"], $row["title"]); - } - } - } - } else { - $query = " - SELECT tst_test_question.sequence, tst_test_question.question_fi, - qpl_questions.points, qpl_questions.title, qpl_questions.original_id - FROM tst_test_question, tst_active, qpl_questions - WHERE tst_test_question.question_fi = qpl_questions.question_id - AND tst_active.active_id = %s - AND tst_active.test_fi = tst_test_question.test_fi - ORDER BY tst_test_question.sequence - "; - - $result = $this->db->queryF( - $query, - ['integer'], - [$active_id] - ); - - if ($result->numRows()) { - $questionsbysequence = []; - - while ($row = $this->db->fetchAssoc($result)) { - $questionsbysequence[$row['sequence']] = $row; - } - - $seqresult = $this->db->queryF( - "SELECT * FROM tst_sequence WHERE active_fi = %s", - ['integer'], - [$active_id] - ); - - while ($seqrow = $this->db->fetchAssoc($seqresult)) { - $questionsequence = unserialize($seqrow['sequence']); - - foreach ($questionsequence as $sidx => $seq) { - if (!isset($questionsbysequence[$seq])) { - continue; - } - $data->getParticipant($active_id)->addQuestion( - $questionsbysequence[$seq]['original_id'] ?? 0, - $questionsbysequence[$seq]['question_fi'], - $questionsbysequence[$seq]['points'], - $sidx + 1, - $seqrow['pass'] - ); - - $data->addQuestionTitle( - $questionsbysequence[$seq]['question_fi'], - $questionsbysequence[$seq]['title'] - ); - } - } - } - } - } - - foreach (array_keys($data->getParticipants()) as $active_id) { - $user_data = $data->getParticipant($active_id); - $mark = $this->getMarkSchema()->getMatchingMark( - $user_data->getReachedPointsInPercent() - ); - - if ($mark !== null) { - $user_data->setMark($mark); - } - - for ($i = 0;$i < $user_data->getPassCount();$i++) { - $attempt_data = $user_data->getPass($i); - if ($attempt_data === null) { - continue; - } - $mark = $this->getMarkSchema()->getMatchingMark( - $attempt_data->getReachedPointsInPercent() - ); - if ($mark !== null) { - $attempt_data->setMark($mark); - } - } - - $visiting_time = $this->participant_repository->getFirstAndLastVisitForActiveId($active_id); - - $user_data->setFirstVisit($visiting_time['first_access']); - $user_data->setLastVisit($visiting_time['last_access']); - } - - return $data; + return (new ilTestEvaluationFactory($this->db, $this)) + ->getEvaluationData(); } public function getQuestionCountAndPointsForPassOfParticipant(int $active_id, int $pass): array @@ -7704,4 +7533,9 @@ public static function _lookupRandomTest(int $obj_id): bool return $question_set_type === self::QUESTION_SET_TYPE_RANDOM; } + + public function getVisitingTimeOfParticipant(int $active_id): array + { + return $this->participant_repository->getFirstAndLastVisitForActiveId($active_id); + } } diff --git a/components/ILIAS/Test/classes/class.ilTestEvaluation.php b/components/ILIAS/Test/classes/class.ilTestEvaluation.php deleted file mode 100755 index a54c98ccde83..000000000000 --- a/components/ILIAS/Test/classes/class.ilTestEvaluation.php +++ /dev/null @@ -1,75 +0,0 @@ - - * @version $Id$ - * - * @package components\ILIAS/Test - */ -class ilTestEvaluation -{ - protected $db; - - /** - * @var integer - */ - protected $testId; - - /** - * ilTestEvaluation constructor. - * @param ilDBInterface $db - * @param $testId - */ - public function __construct(ilDBInterface $db, $testId) - { - $this->db = $db; - $this->testId = $testId; - } - - /** - * @param $testId - * @return array - */ - public function getAllActivesPasses(): array - { - $query = " - SELECT active_fi, pass - FROM tst_active actives - INNER JOIN tst_pass_result passes - ON active_fi = active_id - WHERE test_fi = %s - "; - - $res = $this->db->queryF($query, ['integer'], [$this->testId]); - - $passes = []; - - while ($row = $this->db->fetchAssoc($res)) { - if (!isset($passes[$row['active_fi']])) { - $passes[$row['active_fi']] = []; - } - - $passes[$row['active_fi']][] = $row['pass']; - } - - return $passes; - } -} diff --git a/components/ILIAS/Test/classes/class.ilTestEvaluationData.php b/components/ILIAS/Test/classes/class.ilTestEvaluationData.php deleted file mode 100755 index c17e876b86ad..000000000000 --- a/components/ILIAS/Test/classes/class.ilTestEvaluationData.php +++ /dev/null @@ -1,383 +0,0 @@ - -* @author Björn Heyser -* @version $Id$ -* -* @defgroup ModulesTest Modules/Test -* @extends ilObject -*/ - -class ilTestEvaluationData -{ - public const FILTER_BY_NONE = ''; - public const FILTER_BY_NAME = 'name'; - public const FILTER_BY_GROUP = 'group'; - public const FILTER_BY_COURSE = 'course'; - public const FILTER_BY_ACTIVE_ID = 'active_id'; - - public array $question_titles = []; - - /** - * @var array - */ - protected array $participants = []; - protected ?Statistics $statistics = null; - protected ?array $arr_filter = null; - protected int $datasets; - protected ?ilTestParticipantList $access_filtered_participant_list = null; - - public function __sleep(): array - { - return ['question_titles', 'participants', 'statistics', 'arr_filter', 'datasets', 'test']; - } - - public function __construct( - protected ilDBInterface $db, - protected ?ilObjTest $test = null - ) { - if ($test !== null) { - if ($this->getTest()->getAccessFilteredParticipantList()) { - $this->setAccessFilteredParticipantList( - $this->getTest()->getAccessFilteredParticipantList() - ); - } - - $this->generateOverview(); - } - } - - public function getAccessFilteredParticipantList(): ?ilTestParticipantList - { - return $this->access_filtered_participant_list; - } - - public function setAccessFilteredParticipantList(ilTestParticipantList $access_filtered_participant_list): void - { - $this->access_filtered_participant_list = $access_filtered_participant_list; - } - - protected function checkParticipantAccess(int $active_id): bool - { - if ($this->getAccessFilteredParticipantList() === null) { - return true; - } - - return $this->getAccessFilteredParticipantList()->isActiveIdInList($active_id); - } - - protected function loadRows(): array - { - $query = ' - SELECT usr_data.usr_id, - usr_data.firstname, - usr_data.lastname, - usr_data.title, - usr_data.login, - tst_pass_result.*, - tst_active.submitted, - tst_active.last_finished_pass, - (SELECT MIN(started) AS attempt_started - FROM tst_times - WHERE active_fi = tst_active.active_id - AND pass = tst_pass_result.pass - ) as start_time, - (SELECT MAX(finished) AS attempt_started - FROM tst_times - WHERE active_fi = tst_active.active_id - AND pass = tst_pass_result.pass - ) as last_access_time, - COALESCE(tst_active.last_started_pass, -1) <> COALESCE(tst_active.last_finished_pass, -1) unfinished_attempt - FROM tst_pass_result, tst_active - LEFT JOIN usr_data - ON tst_active.user_fi = usr_data.usr_id - WHERE tst_active.active_id = tst_pass_result.active_fi - AND tst_active.test_fi = %s - ORDER BY usr_data.lastname, - usr_data.firstname, - tst_pass_result.active_fi, - tst_pass_result.pass, - tst_pass_result.tstamp - '; - - $result = $this->db->queryF( - $query, - ['integer'], - [$this->getTest()->getTestId()] - ); - - $rows = []; - - while ($row = $this->db->fetchAssoc($result)) { - if (!$this->checkParticipantAccess($row['active_fi'])) { - continue; - } - - $rows[] = $row; - } - - return $rows; - } - - public function generateOverview(): void - { - $this->participants = []; - - $pass = null; - $thissets = 0; - - foreach ($this->loadRows() as $row) { - $thissets++; - - $remove = false; - - if (!$this->participantExists($row['active_fi'])) { - $this->addParticipant($row['active_fi'], new ilTestEvaluationUserData($this->getTest()->getPassScoring())); - - $this->getParticipant($row['active_fi'])->setName( - $this->getTest()->buildName($row['usr_id'], $row['firstname'], $row['lastname'], $row['title']) - ); - - if ($row['login'] !== null) { - $this->getParticipant($row['active_fi'])->setLogin($row['login']); - } - if ($row['usr_id'] !== null) { - $this->getParticipant($row['active_fi'])->setUserID($row['usr_id']); - } - $this->getParticipant($row['active_fi'])->setSubmitted((bool) $row['submitted']); - $this->getParticipant($row['active_fi'])->setLastFinishedPass($row['last_finished_pass']); - } - - if ($this->getParticipant($row['active_fi'])->getPass($row['pass']) === null) { - $pass = new ilTestEvaluationPassData(); - $pass->setPass($row['pass']); - $this->getParticipant($row['active_fi'])->addPass($row['pass'], $pass); - } - - $this->getParticipant($row['active_fi'])->getPass($row['pass'])->setReachedPoints($row['points']); - - if ($row['questioncount'] == 0) { - $data = $this->test->getQuestionCountAndPointsForPassOfParticipant($row['active_fi'], $row['pass']); - $this->getParticipant($row['active_fi'])->getPass($row['pass'])->setMaxPoints($data['points']); - $this->getParticipant($row['active_fi'])->getPass($row['pass'])->setQuestionCount($data['count']); - } else { - $this->getParticipant($row['active_fi'])->getPass($row['pass'])->setMaxPoints($row['maxpoints']); - $this->getParticipant($row['active_fi'])->getPass($row['pass'])->setQuestionCount($row['questioncount']); - } - - $this->getParticipant($row['active_fi'])->getPass($row['pass'])->setStartTime($row['start_time']); - $this->getParticipant($row['active_fi'])->getPass($row['pass'])->setLastAccessTime($row['last_access_time']); - $this->getParticipant($row['active_fi'])->getPass($row['pass'])->setNrOfAnsweredQuestions($row['answeredquestions']); - $this->getParticipant($row['active_fi'])->getPass($row['pass'])->setWorkingTime($row['workingtime']); - $this->getParticipant($row['active_fi'])->getPass($row['pass'])->setExamId((string) $row['exam_id']); - $this->getParticipant($row['active_fi'])->getPass($row['pass'])->setUnfinishedAttempt((bool) $row['unfinished_attempt']); - $this->getParticipant($row['active_fi'])->getPass($row['pass'])->setStatusOfAttempt( - $row['finalized_by'] ? StatusOfAttempt::tryFrom($row['finalized_by']) : null - ); - - - $this->getParticipant($row['active_fi'])->getPass($row['pass'])->setRequestedHintsCount($row['hint_count']); - $this->getParticipant($row['active_fi'])->getPass($row['pass'])->setDeductedHintPoints($row['hint_points']); - } - } - - public function getTest(): ilObjTest - { - return $this->test; - } - - public function setTest(ilObjTest $test): void - { - $this->test = &$test; - } - - public function setDatasets(int $datasets): void - { - $this->datasets = $datasets; - } - - public function getDatasets(): int - { - return $this->datasets; - } - - public function addQuestionTitle(int $question_id, string $question_title): void - { - $this->question_titles[$question_id] = $question_title; - } - - /** - * @return array - */ - public function getQuestionTitles(): array - { - return $this->question_titles; - } - - public function getQuestionTitle(?int $question_id): string - { - if (array_key_exists($question_id, $this->question_titles)) { - return $this->question_titles[$question_id]; - } - - return ''; - } - - public function getTotalFinishedParticipants(): int - { - $finishedParticipants = 0; - - foreach ($this->participants as $participant) { - if (!$participant->isSubmitted()) { - continue; - } - - $finishedParticipants++; - } - - return $finishedParticipants; - } - - /** - * @return array - */ - public function getParticipants(): array - { - if (is_array($this->arr_filter) && count($this->arr_filter) > 0) { - $filtered_participants = []; - $courseids = []; - $groupids = []; - - if (array_key_exists(self::FILTER_BY_GROUP, $this->arr_filter)) { - $ids = ilObject::_getIdsForTitle($this->arr_filter[self::FILTER_BY_GROUP], 'grp', true); - $groupids = array_merge($groupids, $ids); - } - if (array_key_exists(self::FILTER_BY_COURSE, $this->arr_filter)) { - $ids = ilObject::_getIdsForTitle($this->arr_filter[self::FILTER_BY_COURSE], 'crs', true); - $courseids = array_merge($courseids, $ids); - } - foreach ($this->participants as $active_id => $participant) { - $remove = false; - if (array_key_exists(self::FILTER_BY_NAME, $this->arr_filter)) { - if (!(strpos(strtolower($participant->getName()), strtolower((string) $this->arr_filter[self::FILTER_BY_NAME])) !== false)) { - $remove = true; - } - } - if (!$remove) { - if (array_key_exists(self::FILTER_BY_GROUP, $this->arr_filter)) { - $groups = ilParticipants::_getMembershipByType($participant->getUserID(), ['grp']); - $foundfilter = false; - if (count(array_intersect($groupids, $groups))) { - $foundfilter = true; - } - if (!$foundfilter) { - $remove = true; - } - } - } - if (!$remove) { - if (array_key_exists(self::FILTER_BY_COURSE, $this->arr_filter)) { - $courses = ilParticipants::_getMembershipByType($participant->getUserID(), ['crs']); - $foundfilter = false; - if (count(array_intersect($courseids, $courses))) { - $foundfilter = true; - } - if (!$foundfilter) { - $remove = true; - } - } - } - if (!$remove) { - if (array_key_exists(self::FILTER_BY_ACTIVE_ID, $this->arr_filter)) { - if ($active_id != $this->arr_filter[self::FILTER_BY_ACTIVE_ID]) { - $remove = true; - } - } - } - if (!$remove) { - $filtered_participants[$active_id] = $participant; - } - } - return $filtered_participants; - } else { - return $this->participants; - } - } - - public function resetFilter(): void - { - $this->arr_filter = []; - } - - public function setFilter(string $by, string $text): void - { - if (in_array( - $by, - [self::FILTER_BY_ACTIVE_ID, self::FILTER_BY_NAME, self::FILTER_BY_COURSE, self::FILTER_BY_GROUP], - true - )) { - $this->arr_filter = [$by => $text]; - } - } - - public function setFilterArray(array $arr_filter): void - { - $this->arr_filter = $arr_filter; - } - - public function addParticipant(int $active_id, ilTestEvaluationUserData $participant): void - { - $this->participants[$active_id] = $participant; - } - - public function getParticipant(int $active_id): ?ilTestEvaluationUserData - { - return $this->participants[$active_id] ?? null; - } - - public function participantExists($active_id): bool - { - return array_key_exists($active_id, $this->participants); - } - - public function removeParticipant($active_id) - { - unset($this->participants[$active_id]); - } - - public function getStatistics(): Statistics - { - if ($this->statistics === null) { - $this->statistics = new Statistics($this); - } - return $this->statistics; - } - - public function getParticipantIds(): array - { - return array_keys($this->participants); - } -} diff --git a/components/ILIAS/Test/classes/class.ilTestExportGUI.php b/components/ILIAS/Test/classes/class.ilTestExportGUI.php index e9b0f5a941d9..6c6613a04893 100755 --- a/components/ILIAS/Test/classes/class.ilTestExportGUI.php +++ b/components/ILIAS/Test/classes/class.ilTestExportGUI.php @@ -78,8 +78,7 @@ public function createTestArchiveExport() { if ($this->access->checkAccess('write', '', $this->obj->getRefId())) { // prepare generation before contents are processed (for mathjax) - - $evaluation = new ilTestEvaluation($this->db, $this->obj->getTestId()); + $evaluation = new ilTestEvaluationFactory($this->db, $this->obj); $allActivesPasses = $evaluation->getAllActivesPasses(); $participantData = new ilTestParticipantData($this->db, $this->lng); $participantData->setActiveIdsFilter(array_keys($allActivesPasses)); diff --git a/components/ILIAS/Test/src/Results/Data/AttemptOverview.php b/components/ILIAS/Test/src/Results/Data/AttemptOverview.php index 28dfb1b309c1..c6116b11acb0 100644 --- a/components/ILIAS/Test/src/Results/Data/AttemptOverview.php +++ b/components/ILIAS/Test/src/Results/Data/AttemptOverview.php @@ -138,11 +138,11 @@ public function getAsDescriptiveListing( $items + [ $lng->txt('tst_stat_result_timeontask') => $this->buildHumanReadableTime($this->time_on_task), $lng->txt('tst_stat_result_firstvisit') => $this->attempt_started_date - ->setTimezone($environment['timezone']) - ->format($environment['datetimeformat']), + ?->setTimezone($environment['timezone']) + ->format($environment['datetimeformat']) ?? '', $lng->txt('tst_stat_result_lastvisit') => $this->last_access - ->setTimezone($environment['timezone']) - ->format($environment['datetimeformat']), + ?->setTimezone($environment['timezone']) + ->format($environment['datetimeformat']) ?? '', $lng->txt('tst_nr_of_passes') => (string) $this->nr_of_attempts, $lng->txt('scored_pass') => (string) $this->scored_attempt, $lng->txt('tst_stat_result_rank_participant') => (string) $this->rank diff --git a/components/ILIAS/Test/src/Results/Data/Factory.php b/components/ILIAS/Test/src/Results/Data/Factory.php index 0df640a77c94..19f5eb01a5d9 100644 --- a/components/ILIAS/Test/src/Results/Data/Factory.php +++ b/components/ILIAS/Test/src/Results/Data/Factory.php @@ -322,10 +322,6 @@ private function buildAttemptResults( private function retrieveResultData(\ilObjTest $test_obj): \ilTestEvaluationData { if (!isset($this->test_data[$test_obj->getId()])) { - $test_obj->setAccessFilteredParticipantList( - $test_obj->buildStatisticsAccessFilteredParticipantList() - ); - $this->test_data[$test_obj->getId()] = $test_obj->getCompleteEvaluationData(); } diff --git a/components/ILIAS/Test/tests/ilTestEvaluationDataTest.php b/components/ILIAS/Test/tests/ilTestEvaluationDataTest.php index e8721fc50946..781361fe9c28 100755 --- a/components/ILIAS/Test/tests/ilTestEvaluationDataTest.php +++ b/components/ILIAS/Test/tests/ilTestEvaluationDataTest.php @@ -19,8 +19,7 @@ declare(strict_types=1); /** - * Class ilTestEvaluationDataTest - * @author Marvin Beym + * @deprecated 11; Result/EvaluationData will be refined. */ class ilTestEvaluationDataTest extends ilTestBaseTestCase { @@ -29,8 +28,11 @@ class ilTestEvaluationDataTest extends ilTestBaseTestCase protected function setUp(): void { parent::setUp(); - - $this->testObj = new ilTestEvaluationData($this->createMock(ilDBInterface::class)); + $user_data = [ + new ilTestEvaluationUserData(0), + new ilTestEvaluationUserData(1), + ]; + $this->testObj = new ilTestEvaluationData($user_data); } public function test_instantiateObject_shouldReturnInstance(): void @@ -45,22 +47,6 @@ public function test__sleep(): void $this->assertEquals($expected, $this->testObj->__sleep()); } - public function testAccessFilteredParticipantList(): void - { - $value_mock = $this->createMock(ilTestParticipantList::class); - $this->testObj->setAccessFilteredParticipantList($value_mock); - - $this->assertEquals($value_mock, $this->testObj->getAccessFilteredParticipantList()); - } - - public function testTest(): void - { - $value_mock = $this->getTestObjMock(); - $this->testObj->setTest($value_mock); - - $this->assertEquals($value_mock, $this->testObj->getTest()); - } - public function testDatasets(): void { $this->testObj->setDatasets(20); @@ -83,5 +69,86 @@ public function testQuestionTitle(): void } $this->assertEquals($expected, $this->testObj->getQuestionTitles()); + + $this->assertEquals($expected[2150], $this->testObj->getQuestionTitle(2150)); + } + + public function testEvaluationFactory(): void + { + $records = []; + $records[] = [ + "question_fi" => 9, + "result_points" => 1.2, + "answered" => true, + "manual" => 1, + "original_id" => null, + "questiontitle" => "some title", + "qpl_maxpoints" => 2.4, + "submitted" => true, + "last_finished_pass" => 1, + "active_fi" => 7 , + "pass" => 1, + "points" => 10.3, + "maxpoints" => 32, + "questioncount" => 32, + "answeredquestions" => 1, + "workingtime" => 28, + "tstamp" => 1731941437, + "hint_count" => 0, + "hint_points" => 0, + "obligations_answered" => true, + "exam_id" => "I0_T355_A7_P1", + "usr_id" => 6, + "firstname" => "root", + "lastname" => "user", + "title" => "", + "login" => "root" + ]; + $records[] = null; + + $test_obj = $this->createMock(ilObjTest::class); + $test_obj + ->expects($this->once()) + ->method('getPassScoring'); + $test_obj + ->expects($this->once()) + ->method('getAccessFilteredParticipantList') + ->willReturn(null); + $test_obj + ->expects($this->once()) + ->method('getTestParticipants') + ->willReturn([7]); + $test_obj + ->expects($this->once()) + ->method('getVisitingTimeOfParticipant') + ->willReturn( + [ + 'first_access' => new \DateTimeImmutable(), + 'last_access' => new \DateTimeImmutable() + ] + ); + + $db = $this->createMock(ilDBInterface::class); + $db + ->expects($this->exactly(2)) + ->method('fetchAssoc') + ->willReturnCallback( + function ($res) use (&$records) { + return array_shift($records); + } + ); + + $factory = new ilTestEvaluationFactory($db, $test_obj); + $data = $factory->getEvaluationData(); + $this->assertInstanceOf(ilTestEvaluationData::class, $data); + + $this->assertEquals( + [7], + $data->getParticipantIds() + ); + $this->assertInstanceOf( + ilTestEvaluationUserData::class, + $data->getParticipant(7) + ); } } diff --git a/components/ILIAS/Test/tests/ilTestEvaluationTest.php b/components/ILIAS/Test/tests/ilTestEvaluationTest.php deleted file mode 100755 index 1040896666a8..000000000000 --- a/components/ILIAS/Test/tests/ilTestEvaluationTest.php +++ /dev/null @@ -1,43 +0,0 @@ - - */ -class ilTestEvaluationTest extends ilTestBaseTestCase -{ - private ilTestEvaluation $testObj; - - protected function setUp(): void - { - parent::setUp(); - - $this->testObj = new ilTestEvaluation( - $this->createMock(ilDBInterface::class), - 0 - ); - } - - public function test_instantiateObject_shouldReturnInstance(): void - { - $this->assertInstanceOf(ilTestEvaluation::class, $this->testObj); - } -} From 86b2c576aaebcf5e6b8dc3a981858186bcebd002 Mon Sep 17 00:00:00 2001 From: Stephan Kergomard Date: Mon, 16 Dec 2024 14:17:49 +0100 Subject: [PATCH 02/13] Test: Fix Missing Evaluation Data --- .../class.ilTestEvaluationFactory.php | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/components/ILIAS/Test/classes/Evaluation/class.ilTestEvaluationFactory.php b/components/ILIAS/Test/classes/Evaluation/class.ilTestEvaluationFactory.php index 0912264ab70a..75c47d0c0113 100644 --- a/components/ILIAS/Test/classes/Evaluation/class.ilTestEvaluationFactory.php +++ b/components/ILIAS/Test/classes/Evaluation/class.ilTestEvaluationFactory.php @@ -68,14 +68,14 @@ private function retrieveEvaluationData(array $active_ids): \Generator usr_data.title, usr_data.login - FROM tst_test_result, qpl_questions, tst_active + FROM tst_active + LEFT JOIN tst_test_result ON tst_active.active_id = tst_test_result.active_fi LEFT JOIN tst_pass_result ON tst_active.active_id = tst_pass_result.active_fi + LEFT JOIN qpl_questions ON qpl_questions.question_id = tst_test_result.question_fi LEFT JOIN usr_data ON tst_active.user_fi = usr_data.usr_id - WHERE tst_active.active_id = tst_test_result.active_fi - AND qpl_questions.question_id = tst_test_result.question_fi - AND tst_active.test_fi = %s + WHERE tst_active.test_fi = %s AND %s ORDER BY tst_active.active_id ASC, tst_test_result.pass ASC, tst_test_result.tstamp DESC @@ -148,14 +148,16 @@ public function getEvaluationData(): ilTestEvaluationData $pass->setDeductedHintPoints($row['hint_points']); } - $pass->addAnsweredQuestion( - $row["question_fi"], - $row["qpl_maxpoints"], - $row["result_points"], - (bool) $row['answered'], - null, - $row['manual'] - ); + if ($row['question_fi'] !== null) { + $pass->addAnsweredQuestion( + $row["question_fi"], + $row["qpl_maxpoints"], + $row["result_points"], + (bool) $row['answered'], + null, + $row['manual'] + ); + } $user_eval_data->addPass($row['pass'], $pass); From f6a49a5dd33eb6324f6c5c647082276b1697967e Mon Sep 17 00:00:00 2001 From: Lukas Eichenauer Date: Thu, 12 Dec 2024 13:53:11 +0100 Subject: [PATCH 03/13] Test: Cast Points to Fload in TextQuestion See: https://mantis.ilias.de/view.php?id=43168 and https://mantis.ilias.de/view.php?id=43170 --- .../TestQuestionPool/classes/class.assTextQuestion.php | 5 ++++- .../TestQuestionPool/classes/class.assTextQuestionGUI.php | 6 +++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/components/ILIAS/TestQuestionPool/classes/class.assTextQuestion.php b/components/ILIAS/TestQuestionPool/classes/class.assTextQuestion.php index f273db61ce3a..ff9ad77c4a11 100755 --- a/components/ILIAS/TestQuestionPool/classes/class.assTextQuestion.php +++ b/components/ILIAS/TestQuestionPool/classes/class.assTextQuestion.php @@ -649,7 +649,10 @@ public function setAnswers($answers): void for ($i = 0; $i < $count; $i++) { if ($withPoints) { - $this->addAnswer($answers['answer'][$i], $answers['points'][$i]); + $this->addAnswer( + $answers['answer'][$i], + $this->refinery->kindlyTo()->float()->transform($answers['points'][$i]) + ); } else { $this->addAnswer($answers[$i], 0); } diff --git a/components/ILIAS/TestQuestionPool/classes/class.assTextQuestionGUI.php b/components/ILIAS/TestQuestionPool/classes/class.assTextQuestionGUI.php index 0848d2a5a97a..ca61d0170471 100755 --- a/components/ILIAS/TestQuestionPool/classes/class.assTextQuestionGUI.php +++ b/components/ILIAS/TestQuestionPool/classes/class.assTextQuestionGUI.php @@ -553,7 +553,7 @@ public function writeAnswerSpecificPostData(ilPropertyFormGUI $form): void switch ($this->object->getKeywordRelation()) { case assTextQuestion::SCORING_MODE_KEYWORD_RELATION_NONE: $this->object->setAnswers([]); - $points = str_replace(',', '.', $this->request_data_collector->string('non_keyword_points')); + $points = $this->request_data_collector->float('non_keyword_points'); break; case assTextQuestion::SCORING_MODE_KEYWORD_RELATION_ANY: $this->object->setAnswers($this->request_data_collector->raw('any_keyword')); @@ -561,11 +561,11 @@ public function writeAnswerSpecificPostData(ilPropertyFormGUI $form): void break; case assTextQuestion::SCORING_MODE_KEYWORD_RELATION_ALL: $this->object->setAnswers($this->request_data_collector->raw('all_keyword')); - $points = str_replace(',', '.', $this->request_data_collector->string('all_keyword_points')); + $points = $this->request_data_collector->float('all_keyword_points'); break; case assTextQuestion::SCORING_MODE_KEYWORD_RELATION_ONE: $this->object->setAnswers($this->request_data_collector->raw('one_keyword')); - $points = (float) str_replace(',', '.', $this->request_data_collector->string('one_keyword_points')); + $points = $this->request_data_collector->float('one_keyword_points'); break; } $this->object->setPoints((float) $points); From ff532d30a3a99df81019f04444926e4b2b191c22 Mon Sep 17 00:00:00 2001 From: Stephan Kergomard Date: Mon, 16 Dec 2024 16:13:00 +0100 Subject: [PATCH 04/13] Test: Ensure All Attempts Are in Evaluation --- .../class.ilTestEvaluationFactory.php | 35 ++++++++++--------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/components/ILIAS/Test/classes/Evaluation/class.ilTestEvaluationFactory.php b/components/ILIAS/Test/classes/Evaluation/class.ilTestEvaluationFactory.php index 75c47d0c0113..0c2c4423aad6 100644 --- a/components/ILIAS/Test/classes/Evaluation/class.ilTestEvaluationFactory.php +++ b/components/ILIAS/Test/classes/Evaluation/class.ilTestEvaluationFactory.php @@ -100,11 +100,12 @@ public function getEvaluationData(): ilTestEvaluationData $scoring_settings = $this->test_obj->getPassScoring(); $participants = []; $current_user = null; + $current_attempt = null; foreach ($eval_data_rows as $row) { - - if ($current_user != $row['active_fi']) { + if ($current_user !== $row['active_fi']) { $current_user = $row['active_fi']; + $current_attempt = null; $user_eval_data = new ilTestEvaluationUserData($scoring_settings); @@ -125,31 +126,33 @@ public function getEvaluationData(): ilTestEvaluationData $visiting_time = $this->test_obj->getVisitingTimeOfParticipant($row['active_fi']); $user_eval_data->setFirstVisit($visiting_time["first_access"]); $user_eval_data->setLastVisit($visiting_time["last_access"]); + } - $pass = new \ilTestEvaluationPassData(); - $pass->setPass($row['pass']); - $pass->setReachedPoints($row['points']); + if ($current_attempt !== $row['pass']) { + $attempt = new \ilTestEvaluationPassData(); + $attempt->setPass($row['pass']); + $attempt->setReachedPoints($row['points']); if ($row['questioncount'] == 0) { list($count, $points) = array_values( $this->test_obj->getQuestionCountAndPointsForPassOfParticipant($row['active_fi'], $row['pass']) ); - $pass->setMaxPoints($points); - $pass->setQuestionCount($count); + $attempt->setMaxPoints($points); + $attempt->setQuestionCount($count); } else { - $pass->setMaxPoints($row['maxpoints']); - $pass->setQuestionCount($row['questioncount']); + $attempt->setMaxPoints($row['maxpoints']); + $attempt->setQuestionCount($row['questioncount']); } - $pass->setNrOfAnsweredQuestions($row['answeredquestions']); - $pass->setWorkingTime($row['workingtime']); - $pass->setExamId((string) $row['exam_id']); - $pass->setRequestedHintsCount($row['hint_count']); - $pass->setDeductedHintPoints($row['hint_points']); + $attempt->setNrOfAnsweredQuestions($row['answeredquestions']); + $attempt->setWorkingTime($row['workingtime']); + $attempt->setExamId((string) $row['exam_id']); + $attempt->setRequestedHintsCount($row['hint_count']); + $attempt->setDeductedHintPoints($row['hint_points']); } if ($row['question_fi'] !== null) { - $pass->addAnsweredQuestion( + $attempt->addAnsweredQuestion( $row["question_fi"], $row["qpl_maxpoints"], $row["result_points"], @@ -159,7 +162,7 @@ public function getEvaluationData(): ilTestEvaluationData ); } - $user_eval_data->addPass($row['pass'], $pass); + $user_eval_data->addPass($row['pass'], $attempt); $participants[$row['active_fi']] = $user_eval_data; } From 68dcac96e8c2d87092958f929e61609590fdd734 Mon Sep 17 00:00:00 2001 From: Stephan Kergomard Date: Mon, 16 Dec 2024 16:14:10 +0100 Subject: [PATCH 05/13] Test: Ensure Results Don't Contain Negative Values See: https://mantis.ilias.de/view.php?id=43199 --- .../ILIAS/Test/classes/class.ilObjTest.php | 17 +++++++++++------ .../Test/src/Results/Data/AttemptOverview.php | 2 +- .../ILIAS/Test/src/Setup/Test9DBUpdateSteps.php | 5 +++++ 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/components/ILIAS/Test/classes/class.ilObjTest.php b/components/ILIAS/Test/classes/class.ilObjTest.php index cfc42eddc99a..024ec5b3773b 100755 --- a/components/ILIAS/Test/classes/class.ilObjTest.php +++ b/components/ILIAS/Test/classes/class.ilObjTest.php @@ -7362,6 +7362,10 @@ public function updateTestResultCache(int $active_id, ilAssQuestionProcessLocker [$active_id] ); + if ($reached < 0.0) { + $reached = 0.0; + } + $mark_short_name = $mark->getShortName(); if ($mark_short_name === '') { $mark_short_name = ' '; @@ -7425,7 +7429,8 @@ public function updateTestPassResults( if ($result->numRows() > 0) { $row = $this->db->fetchAssoc($result); - if ($row['reachedpoints'] === null) { + if ($row['reachedpoints'] === null + || $row['reachedpoints'] < 0.0) { $row['reachedpoints'] = 0.0; } if ($row['hint_count'] === null) { @@ -7445,7 +7450,7 @@ public function updateTestPassResults( 'pass' => ['integer', $pass] ], [ - 'points' => ['float', $row['reachedpoints'] ?: 0], + 'points' => ['float', $row['reachedpoints']], 'maxpoints' => ['float', $data['points']], 'questioncount' => ['integer', $data['count']], 'answeredquestions' => ['integer', $row['answeredquestions']], @@ -7470,10 +7475,10 @@ public function updateTestPassResults( return [ 'active_fi' => $active_id, 'pass' => $pass, - 'points' => $row["reachedpoints"] ?? 0.0, - 'maxpoints' => $data["points"], - 'questioncount' => $data["count"], - 'answeredquestions' => $row["answeredquestions"], + 'points' => $row['reachedpoints'], + 'maxpoints' => $data['points'], + 'questioncount' => $data['count'], + 'answeredquestions' => $row['answeredquestions'], 'workingtime' => $time, 'tstamp' => time(), 'hint_count' => $row['hint_count'], diff --git a/components/ILIAS/Test/src/Results/Data/AttemptOverview.php b/components/ILIAS/Test/src/Results/Data/AttemptOverview.php index c6116b11acb0..dc618fd95ea7 100644 --- a/components/ILIAS/Test/src/Results/Data/AttemptOverview.php +++ b/components/ILIAS/Test/src/Results/Data/AttemptOverview.php @@ -144,7 +144,7 @@ public function getAsDescriptiveListing( ?->setTimezone($environment['timezone']) ->format($environment['datetimeformat']) ?? '', $lng->txt('tst_nr_of_passes') => (string) $this->nr_of_attempts, - $lng->txt('scored_pass') => (string) $this->scored_attempt, + $lng->txt('scored_pass') => (string) ($this->scored_attempt + 1), $lng->txt('tst_stat_result_rank_participant') => (string) $this->rank ] ); diff --git a/components/ILIAS/Test/src/Setup/Test9DBUpdateSteps.php b/components/ILIAS/Test/src/Setup/Test9DBUpdateSteps.php index 6f7f28509055..3a1769247bdd 100755 --- a/components/ILIAS/Test/src/Setup/Test9DBUpdateSteps.php +++ b/components/ILIAS/Test/src/Setup/Test9DBUpdateSteps.php @@ -358,4 +358,9 @@ public function step_19(): void } } + public function step_20(): void + { + $this->db->manipulate('UPDATE tst_pass_result SET points = 0 WHERE points < 0'); + $this->db->manipulate('UPDATE tst_result_cache SET reached_points = 0 WHERE reached_points < 0'); + } } From 08d70571c4cb8fd48bccee62c0b3c4cc98ec304a Mon Sep 17 00:00:00 2001 From: Tim Schmitz Date: Mon, 16 Dec 2024 17:14:15 +0100 Subject: [PATCH 06/13] Feeds: reintroduce autoloading in endpoints (43253) --- components/ILIAS/Feeds/resources/feed.php | 5 ++++- components/ILIAS/Feeds/resources/privfeed.php | 20 +++++++++++++++++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/components/ILIAS/Feeds/resources/feed.php b/components/ILIAS/Feeds/resources/feed.php index 4a21d2772e80..08ad1f398454 100644 --- a/components/ILIAS/Feeds/resources/feed.php +++ b/components/ILIAS/Feeds/resources/feed.php @@ -20,6 +20,9 @@ * News feed script. * @author Alexander Killing */ + +require_once '../vendor/composer/vendor/autoload.php'; + ilContext::init(ilContext::CONTEXT_RSS); ilInitialisation::initILIAS(); @@ -48,4 +51,4 @@ $writer->showFeed(); } elseif ($requested_blog_id > 0) { ilObjBlog::deliverRSS($requested_blog_id); -} \ No newline at end of file +} diff --git a/components/ILIAS/Feeds/resources/privfeed.php b/components/ILIAS/Feeds/resources/privfeed.php index a7e7ad4785c0..8abbbd09c973 100644 --- a/components/ILIAS/Feeds/resources/privfeed.php +++ b/components/ILIAS/Feeds/resources/privfeed.php @@ -1,6 +1,20 @@ addItem($feed_item); $blankFeedWriter->showFeed(); } -} \ No newline at end of file +} From 47b75db986b5a45d58c2ee4f649616ff71015adb Mon Sep 17 00:00:00 2001 From: Tim Schmitz Date: Mon, 16 Dec 2024 18:42:07 +0100 Subject: [PATCH 07/13] Form: restore previous format for getInput of ilDateTimeInputGUI (43255) --- .../ILIAS/Form/classes/class.ilDateTimeInputGUI.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/components/ILIAS/Form/classes/class.ilDateTimeInputGUI.php b/components/ILIAS/Form/classes/class.ilDateTimeInputGUI.php index 84b0f0090986..138012dc9b97 100755 --- a/components/ILIAS/Form/classes/class.ilDateTimeInputGUI.php +++ b/components/ILIAS/Form/classes/class.ilDateTimeInputGUI.php @@ -172,10 +172,11 @@ public function checkInput(): bool public function getInput(): ?string { if ($this->valid && $this->getDate() !== null) { - return $this->getDate()->get( - IL_CAL_FKT_DATE, - $this->getDatetimeFormatForInput() - ); + // getInput() should return a generic format + $post_format = $this->getShowTime() + ? IL_CAL_DATETIME + : IL_CAL_DATE; + return $this->getDate()->get($post_format); } return null; } From 8a5c5ec01737ca8698eb035a975ad943fc94cae7 Mon Sep 17 00:00:00 2001 From: mjansen Date: Wed, 18 Sep 2024 08:14:42 +0200 Subject: [PATCH 08/13] Dependencies: Remove `rector` files from `scripts` # Conflicts: # scripts/Rector/ilUtils/README.md # scripts/Rector/ilUtils/ilutil_rector.php --- scripts/Rector/ChangeLicenseHeader.php | 139 --------- .../Rector/DIC/DICDependencyManipulator.php | 282 ------------------ scripts/Rector/DIC/DICMember.php | 76 ----- scripts/Rector/DIC/DICMemberMap.php | 47 --- scripts/Rector/DIC/DICMemberResolver.php | 188 ------------ scripts/Rector/README.md | 28 -- .../RemoveRequiresAndIncludesRector.php | 63 ---- scripts/Rector/basic_rector.php | 15 - scripts/Rector/code_quality.php | 18 -- scripts/Rector/ilUtils/Example.php | 36 --- scripts/Rector/ilUtils/README.md | 29 -- .../ilUtils/ReplaceUtilSendMessageRector.php | 141 --------- scripts/Rector/ilUtils/ilutil_rector.php | 44 --- scripts/Rector/ilias_9.php | 22 -- 14 files changed, 1128 deletions(-) delete mode 100755 scripts/Rector/ChangeLicenseHeader.php delete mode 100755 scripts/Rector/DIC/DICDependencyManipulator.php delete mode 100755 scripts/Rector/DIC/DICMember.php delete mode 100755 scripts/Rector/DIC/DICMemberMap.php delete mode 100755 scripts/Rector/DIC/DICMemberResolver.php delete mode 100755 scripts/Rector/README.md delete mode 100755 scripts/Rector/RemoveRequiresAndIncludesRector.php delete mode 100755 scripts/Rector/basic_rector.php delete mode 100755 scripts/Rector/code_quality.php delete mode 100755 scripts/Rector/ilUtils/Example.php delete mode 100755 scripts/Rector/ilUtils/README.md delete mode 100755 scripts/Rector/ilUtils/ReplaceUtilSendMessageRector.php delete mode 100755 scripts/Rector/ilUtils/ilutil_rector.php delete mode 100755 scripts/Rector/ilias_9.php diff --git a/scripts/Rector/ChangeLicenseHeader.php b/scripts/Rector/ChangeLicenseHeader.php deleted file mode 100755 index 52be019ec1aa..000000000000 --- a/scripts/Rector/ChangeLicenseHeader.php +++ /dev/null @@ -1,139 +0,0 @@ -standard_comment = new Comment($this->license_header_default); - } - - /** - * @return class-string[] - */ - public function getNodeTypes(): array - { - return [ - Node\Stmt\Class_::class, - Node\Stmt\Interface_::class, - Node\Stmt\Trait_::class - ]; - } - - /** - * @param Node\Stmt\Global_ $node - */ - public function refactor(Node $node) - { - if (preg_match(self::IGNORE_SUBPATHS, $this->file->getFilePath()) > 0) { - return $node; - } - $node->setAttribute('comments', $this->filterComments($node)); - $current = $node; - $previous = $node->getAttribute(AttributeKeys::PREVIOUS_NODE); - while (is_object($previous) && in_array($previous::class, $this->previous_search)) { - if ($previous instanceof \PhpParser\Node\Name) { - $previous = $previous->getAttribute(AttributeKeys::PARENT_NODE); - } - if ($previous instanceof Node\Expr\Empty_) { - $this->removeNode($previous); - } - $current = $previous; - $current->setAttribute( - AttributeKeys::COMMENTS, - $this->filterComments($current) - ); - $previous = $current->getAttribute(AttributeKeys::PREVIOUS_NODE); - } - - $current->setAttribute(AttributeKeys::COMMENTS, $this->filterComments($current, [$this->standard_comment])); - - return $node; - } - - /** - * @return Comment[] - */ - private function filterComments(Node $node, array $default = []): array - { - foreach ($node->getComments() as $comment) { - if (preg_match(self::EXISTING_LICENSE_PATTERN, $comment->getText()) > 0) { - continue; - } - $default[] = $comment; - } - return $default; - } - - public function getRuleDefinition(): RuleDefinition - { - return new RuleDefinition( - 'Adds or replaces a license-header in each class-file', - [ - new CodeSample( - // code before - '', - // code after - '' - ), - ] - ); - } -} diff --git a/scripts/Rector/DIC/DICDependencyManipulator.php b/scripts/Rector/DIC/DICDependencyManipulator.php deleted file mode 100755 index f1ecf6be6a04..000000000000 --- a/scripts/Rector/DIC/DICDependencyManipulator.php +++ /dev/null @@ -1,282 +0,0 @@ -getDICVariable()]); - } - - public function addStmtToMethodIfNotThereYetAtFirstPosition( - ClassMethod $classMethod, - Stmt\Class_ $class, - Stmt $stmt - ): void { - $class_method_string = $class->name->name . '::' . $classMethod->name->name; - $stmt_string = $this->nodeComparator->printWithoutComments($stmt); - if (isset($this->duplicate_checker[$class_method_string][$stmt_string]) - && $this->duplicate_checker[$class_method_string][$stmt_string] === true) { - return; - } - $stmts = $this->stmtsManipulator->filterOutExistingStmts( - $classMethod, - [$stmt] - ); - // all stmts are already there → skip - if ($stmts === []) { - return; - } - $first = null; - foreach ($classMethod->getStmts() as $inner_statement) { - if ($inner_statement->getAttributes() === []) { - continue; - } - $first = $inner_statement; - break; - } - if ($first !== null) { - $this->nodesToAddCollector->addNodeBeforeNode($stmt, $first); - } else { - $classMethod->stmts[] = $stmt; - } - $this->duplicate_checker[$class_method_string][$stmt_string] = true; - } - - private function createConstructor( - \PhpParser\Node\Stmt\Class_ $class - ): ClassMethod { - if (isset($this->added_constructors[$class->name->name])) { - return $this->added_constructors[$class->name->name]; - } - $classMethod = $this->nodeFactory->createPublicMethod( - \Rector\Core\ValueObject\MethodName::CONSTRUCT - ); - // implement parent constructor call - if ($this->hasClassParentClassMethod( - $class, - \Rector\Core\ValueObject\MethodName::CONSTRUCT - )) { - $classMethod->stmts[] = $this->createParentClassMethodCall( - \Rector\Core\ValueObject\MethodName::CONSTRUCT - ); - } - $first_class_method = array_filter($class->stmts, function (\PhpParser\Node $node): bool { - return $node instanceof ClassMethod; - }); - $first_class_method = array_shift($first_class_method); - if ($first_class_method !== null) { - $this->nodesToAddCollector->addNodeBeforeNode($classMethod, $first_class_method); - } else { - array_unshift($class->stmts, $classMethod); - } - $this->outputStyle->newline(); - $this->outputStyle->warning( - 'created constructor for ' . $class->name->name . '. Please check the parent-call for missing parameters!' - ); - $this->outputStyle->newline(); - - $this->added_constructors[$class->name->name] = $classMethod; - - return $classMethod; - } - - public function addStmtToConstructorIfNotThereYetAtFirstPosition( - \PhpParser\Node\Stmt\Class_ $class, - Stmt $stmt - ): void { - $classMethod = $class->getMethod( - \Rector\Core\ValueObject\MethodName::CONSTRUCT - ); - if (!$classMethod instanceof \PhpParser\Node\Stmt\ClassMethod) { - $classMethod = $this->createConstructor($class); - } - $this->addStmtToMethodIfNotThereYetAtFirstPosition( - $classMethod, - $class, - $stmt - ); - } - - public function ensureGlobalDICinConstructor(Stmt\Class_ $class): void - { - $stmt = $this->getGlobalDIC(); - $this->addStmtToConstructorIfNotThereYetAtFirstPosition( - $class, - $stmt - ); - $this->duplicate_checker[$class->name->name][$this->nodeComparator->printWithoutComments($stmt)] = true; - } - - public function ensureGlobalDICinMethod(ClassMethod $classMethod, Stmt\Class_ $class): Variable - { - $this->addStmtToMethodIfNotThereYetAtFirstPosition( - $classMethod, - $class, - $this->getGlobalDIC() - ); - return $this->getDICVariable(); - } - - public function addStmtToMethodIfNotThereAfterGlobalDIC( - ClassMethod $classMethod, - Stmt\Class_ $class, - Stmt $stmt - ): void { - $class_method_string = $class->name->name . '::' . $classMethod->name->name; - $statement_string = $this->nodeComparator->printWithoutComments($stmt); - if (isset($this->duplicate_checker[$class_method_string][$statement_string]) - && $this->duplicate_checker[$class_method_string][$statement_string] === true) { - return; - } - $stmts = $this->stmtsManipulator->filterOutExistingStmts( - $classMethod, - [$stmt] - ); - // all stmts are already there → skip - if ($stmts === []) { - return; - } - - $node = $this->betterNodeFinder->findFirst($classMethod->stmts, function (\PhpParser\Node $node): bool { - if (!$node instanceof Stmt\Global_) { - return false; - } - foreach ($node->vars as $var) { - if (!(property_exists($var, 'name') && $var->name !== null)) { - continue; - } - if ($var->name !== self::DIC) { - continue; - } - return true; - } - return false; - }); - $dic_statement_string = $this->nodeComparator->printWithoutComments($this->getGlobalDIC()); - if (!$node instanceof \PhpParser\Node - && !isset($this->duplicate_checker[$class_method_string][$dic_statement_string]) // we already added global $DIC in this run - && !$this->duplicate_checker[$class_method_string][$dic_statement_string] - ) { - throw new ShouldNotHappenException( - 'no dic found: ' . $class_method_string . ' (' . $statement_string . ') ' - ); - } - - // get first existing statement - $first_existing = array_filter($classMethod->stmts, function (\PhpParser\Node $node): bool { - if ($node->getAttributes() === []) { - return false; - } - return !$node instanceof Stmt\Global_; - }); - $first_existing = array_shift($first_existing); - if ($first_existing !== null) { - $this->nodesToAddCollector->addNodeBeforeNode($stmt, $first_existing); - } else { - // we use a fallback to add the element in first place. - // the nodesToAddCollector does not work here, becaue there are only - // "new" nodes without position - $classMethod->stmts[] = $stmt; - } - $this->duplicate_checker[$class_method_string][$statement_string] = true; - } - - public function addStmtToConstructorIfNotThereAfterGlobalDIC( - \PhpParser\Node\Stmt\Class_ $class, - Stmt $stmt - ): void { - $classMethod = $class->getMethod( - \Rector\Core\ValueObject\MethodName::CONSTRUCT - ); - if (!$classMethod instanceof \PhpParser\Node\Stmt\ClassMethod) { - $classMethod = $this->createConstructor($class); - } - $this->addStmtToMethodIfNotThereAfterGlobalDIC( - $classMethod, - $class, - $stmt - ); - } - - private function hasClassParentClassMethod( - \PhpParser\Node\Stmt\Class_ $class, - string $methodName - ): bool { - $scope = $class->getAttribute( - \Rector\NodeTypeResolver\Node\AttributeKey::SCOPE - ); - if (!$scope instanceof \PHPStan\Analyser\Scope) { - return \false; - } - $classReflection = $scope->getClassReflection(); - if (!$classReflection instanceof \PHPStan\Reflection\ClassReflection) { - return \false; - } - foreach ($classReflection->getParents() as $parentClassReflection) { - if ($parentClassReflection->hasMethod($methodName)) { - return \true; - } - } - return \false; - } - - private function createParentClassMethodCall( - string $methodName - ): \PhpParser\Node\Stmt\Expression { - $staticCall = new \PhpParser\Node\Expr\StaticCall( - new \PhpParser\Node\Name( - \Rector\Core\Enum\ObjectReference::PARENT()->getValue() - ), - $methodName - ); - - // append arguments - - return new \PhpParser\Node\Stmt\Expression($staticCall); - } -} diff --git a/scripts/Rector/DIC/DICMember.php b/scripts/Rector/DIC/DICMember.php deleted file mode 100755 index 08b656daf85e..000000000000 --- a/scripts/Rector/DIC/DICMember.php +++ /dev/null @@ -1,76 +0,0 @@ -alternative_classes = $alternative_classes; - } - - public function getName(): string - { - return $this->name; - } - - public function getMainClass(): string - { - return $this->main_class; - } - - /** - * @return mixed[] - */ - public function getAlternativeClasses(): array - { - return $this->alternative_classes; - } - - /** - * @return mixed[] - */ - public function getDicServiceMethod(): array - { - return $this->dic_service_method; - } - - public function getPropertyName(): string - { - return $this->property_name; - } -} diff --git a/scripts/Rector/DIC/DICMemberMap.php b/scripts/Rector/DIC/DICMemberMap.php deleted file mode 100755 index 2ce948135401..000000000000 --- a/scripts/Rector/DIC/DICMemberMap.php +++ /dev/null @@ -1,47 +0,0 @@ -setAlternativeClasses([\ilTemplate::class, \ilGlobalTemplate::class, \ilGlobalPageTemplate::class]); - $this->map[self::TPL] = $dicMember; - } - - public function getByName(string $name): DICMember - { - if (!isset($this->map[$name])) { - throw new \InvalidArgumentException("The dependency '$name' is currently not configured"); - } - return $this->map[$name]; - } -} diff --git a/scripts/Rector/DIC/DICMemberResolver.php b/scripts/Rector/DIC/DICMemberResolver.php deleted file mode 100755 index 3dba16a591fb..000000000000 --- a/scripts/Rector/DIC/DICMemberResolver.php +++ /dev/null @@ -1,188 +0,0 @@ -DICMemberMap = new DICMemberMap(); - } - - /** - * @return Expr|MethodCall - */ - private function getStaticDICCall( - DICMember $DICMember, - Class_ $class, - ClassMethod $classMethod - ): \PhpParser\Node\Expr\Variable { - // $DIC; - $dic_variable = $this->dicDependencyManipulator->ensureGlobalDICinMethod($classMethod, $class); - // new variable like $main_tpl; - $variable = new Variable($DICMember->getPropertyName()); - // MethodCall to get DIC Dependency - $expression = new Expression( - new Assign( - $variable, - $this->appendDICMethods( - $DICMember, - $dic_variable - ) - ) - ); - $this->dicDependencyManipulator->addStmtToMethodIfNotThereAfterGlobalDIC( - $classMethod, - $class, - $expression - ); - - return $variable; - } - - public function ensureDICDependency( - string $name, - Class_ $class, - ClassMethod $classMethod - ): Expr { - $DICMember = $this->getDICMemberByName($name); - - // return simple $GLOBALS access in static methods or - // return simple $GLOBALS access in static methods if we are in - // constructor itself, since currently we have problems to assign the - // member then... - $classMethodName = $classMethod->name->name ?? null; - if ($classMethod->isStatic()) { - return $this->getStaticDICCall($DICMember, $class, $classMethod); - } - if ($classMethodName === \Rector\Core\ValueObject\MethodName::CONSTRUCT) { - return $this->getStaticDICCall($DICMember, $class, $classMethod); - } - - // Test primary class - $mainClass = $DICMember->getMainClass(); - $dicPropertyFetch = $this->typeProvidingExprFromClassResolver->resolveTypeProvidingExprFromClass( - $class, - $classMethod, - $this->getObjectType($mainClass) - ); - if ($dicPropertyFetch instanceof PropertyFetch) { - return $dicPropertyFetch; - } - - // try alternatives - $alternatives = $DICMember->getAlternativeClasses(); - foreach ($alternatives as $alternative) { - $dicPropertyFetch = $this->typeProvidingExprFromClassResolver->resolveTypeProvidingExprFromClass( - $class, - $classMethod, - $this->getObjectType($alternative) - ); - if ($dicPropertyFetch instanceof PropertyFetch) { - return $dicPropertyFetch; - } - } - - // Add property - $this->propertyToAddCollector->addPropertyWithoutConstructorToClass( - $DICMember->getPropertyName(), - $this->getObjectType($mainClass), - $class - ); - - $dicPropertyFetch = new PropertyFetch( - new Variable(self::THIS), - $DICMember->getPropertyName() - ); - // Method call to get DIC dependency - $methodCall = $this->appendDICMethods( - $DICMember, - new Variable(self::DIC) - ); - // global $DIC - $this->dicDependencyManipulator->ensureGlobalDICinConstructor( - $class - ); - // $this->xy = $DIC->xy() - $expression = new Expression( - new Assign( - $dicPropertyFetch, - $methodCall - ) - ); - $this->dicDependencyManipulator->addStmtToConstructorIfNotThereAfterGlobalDIC( - $class, - $expression - ); - - return $dicPropertyFetch; - } - - private function appendDICMethods(DICMember $dicMember, Expr $expr): \PhpParser\Node\Expr - { - foreach ($dicMember->getDicServiceMethod() as $call) { - $expr = new MethodCall( - $expr, - $call - ); - } - return $expr; - } - - private function getDICMemberByName(string $name): DICMember - { - return $this->DICMemberMap->getByName($name); - } - - private function getObjectType(string $name): ObjectType - { - return new ObjectType($name); - } -} diff --git a/scripts/Rector/README.md b/scripts/Rector/README.md deleted file mode 100755 index 121cfe7aa64b..000000000000 --- a/scripts/Rector/README.md +++ /dev/null @@ -1,28 +0,0 @@ -Rector - Instant Upgrades and Automated Refactoring -=================================================== - -We can use Rector to do some automated refactoring. This is a good way to upgrade your codebase to a new version of PHP. - -## Usage - -There is basic configuration which includes two costim rules, removing all requires/includes and addid the -License-header if needed. - -```bash -./vendor/composer/vendor/bin/rector process --config scripts/Rector/basic_rector.php YOUR_DIRECTORY -``` - -You can try to update your Code to support PHP 8.0 to 8.2 with the follwoing rule-set: - -```bash -./vendor/composer/vendor/bin/rector process --config scripts/Rector/ilias_9.php YOUR_DIRECTORY -``` - -Please check the changes and revert the ones you don't want to have. - -The following rule-set contains some general improvements for your code 8such as early returns, removing unused -variables, etc.): - -```bash -./vendor/composer/vendor/bin/rector process --config scripts/Rector/code_quality.php YOUR_DIRECTORY -``` diff --git a/scripts/Rector/RemoveRequiresAndIncludesRector.php b/scripts/Rector/RemoveRequiresAndIncludesRector.php deleted file mode 100755 index 13c885b1f32b..000000000000 --- a/scripts/Rector/RemoveRequiresAndIncludesRector.php +++ /dev/null @@ -1,63 +0,0 @@ -> - */ - public function getNodeTypes(): array - { - return [\PhpParser\Node\Expr\Include_::class]; - } - - /** - * @param Node\Expr\Include_ $node - */ - public function refactor(Node $node): \PhpParser\Node\Expr\Include_ - { - if (!$this->isObjectType($node, new ObjectType(Node\Expr\Assign::class))) { - $this->nodeRemover->removeNode($node); - } - - return $node; - } - - public function getRuleDefinition(): RuleDefinition - { - return new RuleDefinition( - 'Remove requires and includes', - [ - new CodeSample( - // code before - 'require_once "./..."', - // code after - '' - ), - ] - ); - } -} diff --git a/scripts/Rector/basic_rector.php b/scripts/Rector/basic_rector.php deleted file mode 100755 index ab221e76e622..000000000000 --- a/scripts/Rector/basic_rector.php +++ /dev/null @@ -1,15 +0,0 @@ -disableParallel(); - // We start with a single and sinle (own) rule. remove requires and include. - $rectorConfig->rule(RemoveRequiresAndIncludesRector::class); - // The second rule will add (or replace) e license-header for every class-file - $rectorConfig->rule(ChangeLicenseHeader::class); -}; diff --git a/scripts/Rector/code_quality.php b/scripts/Rector/code_quality.php deleted file mode 100755 index a95949ae553c..000000000000 --- a/scripts/Rector/code_quality.php +++ /dev/null @@ -1,18 +0,0 @@ -disableParallel(); - $rectorConfig->phpVersion(PhpVersion::PHP_80); - $rectorConfig->sets([ - SetList::DEAD_CODE, - SetList::TYPE_DECLARATION, - SetList::CODE_QUALITY, - SetList::EARLY_RETURN, - ]); -}; diff --git a/scripts/Rector/ilUtils/Example.php b/scripts/Rector/ilUtils/Example.php deleted file mode 100755 index 5eb9886b58bb..000000000000 --- a/scripts/Rector/ilUtils/Example.php +++ /dev/null @@ -1,36 +0,0 @@ -> - */ - public function getNodeTypes(): array - { - return [\PhpParser\Node\Expr\StaticCall::class]; - } - - private function isApplicable(Node $node): bool - { - /** @var $node \PhpParser\Node\Expr\StaticCall::class */ - if (!$node->class instanceof \PhpParser\Node\Name) { - return false; - } - $staticCallClassName = $node->class->toString(); - if ($staticCallClassName !== \ilUtil::class) { - // not calling ilUtil - return false; - } - if (!$node->name instanceof \PhpParser\Node\Identifier) { - // node has no name - return false; - } - // not interested in method since not in list - return in_array($node->name->name, $this->old_method_names); - } - - /** - * @param Node $node the Static Call to ilUtil:sendXY - */ - public function refactor(Node $node): ?\PhpParser\Node\Expr\MethodCall - { - if (!$this->isApplicable($node)) { - return null; // leave the node as it is - } - $class_where_call_happens = $this->betterNodeFinder->findParentType( - $node, - \PhpParser\Node\Stmt\Class_::class - ); - if (!$class_where_call_happens instanceof \PhpParser\Node\Stmt\Class_) { - // not on class, abort - return null; // leave the node as it is - } - $method_where_call_happend = $this->betterNodeFinder->findParentType( - $node, - \PhpParser\Node\Stmt\ClassMethod::class - ); - if (!$method_where_call_happend instanceof \PhpParser\Node\Stmt\ClassMethod) { - // not in a method, abort - return null; // leave the node as it is - } - - if ($method_where_call_happend->isStatic()) { -// return null; - } - - // prepend a new argument with the type of the message, aka sendInfo goes to setOnScreenMessage('info', ... - $message_type = strtolower(str_replace('send', '', $node->name->name)); - $arg = $this->nodeFactory->createArg($message_type); - $arguments = $node->args; - array_unshift($arguments, $arg); - - // ensure a dic property for ilGlobalTemplate is in the class. or we get another Expr to fetch ilGlobalTemplate - try { - $dicPropertyFetch = $this->dicMemberResolver->ensureDICDependency( - DICMemberMap::TPL, - $class_where_call_happens, - $method_where_call_happend - ); - } catch (ShouldNotHappenException $e) { - throw new ShouldNotHappenException( - "Could not process " . $this->file->getFilePath() . ': ' . $e->getMessage() - ); - } - - // return new method call - $methodCall = new \PhpParser\Node\Expr\MethodCall( - $dicPropertyFetch, - $this->new_method_name, - $arguments - ); - return $methodCall; - } - - public function getRuleDefinition(): \Symplify\RuleDocGenerator\ValueObject\RuleDefinition - { - return new RuleDefinition('lorem', [ - new CodeSample( - "\ilUtil::sendQuestion('my_text', true);", - "\$this->main_tpl->setOnScreenMessage('question', 'my_text', true)" - ) - ]); - } -} diff --git a/scripts/Rector/ilUtils/ilutil_rector.php b/scripts/Rector/ilUtils/ilutil_rector.php deleted file mode 100755 index 06f68382fcf3..000000000000 --- a/scripts/Rector/ilUtils/ilutil_rector.php +++ /dev/null @@ -1,44 +0,0 @@ -disableParallel(); - $rectorConfig->parameters()->set(Option::SKIP, [ - // there a several classes which make Rector break (multiple classes - // in one file, wrong declarations in inheritance, ...) - "components/ILIAS/LTIConsumer", - "components/ILIAS/LTIProvider", - "components/ILIAS/SOAPAuth/include" - ]); - $rectorConfig->parameters()->set(Option::DEBUG, false); - - $rectorConfig->phpVersion(PhpVersion::PHP_80); - - $rectorConfig->services()->set(DICMemberResolver::class)->autowire(); - $rectorConfig->services()->set(DICDependencyManipulator::class)->autowire(); - $rectorConfig->services()->set(ReplaceUtilSendMessageRector::class); -}; diff --git a/scripts/Rector/ilias_9.php b/scripts/Rector/ilias_9.php deleted file mode 100755 index 6302310e521e..000000000000 --- a/scripts/Rector/ilias_9.php +++ /dev/null @@ -1,22 +0,0 @@ -disableParallel(); - $rectorConfig->phpVersion(PhpVersion::PHP_80); - $rectorConfig->sets([ - SetList::PHP_80, - SetList::PHP_81, - SetList::PHP_82, - LevelSetList::UP_TO_PHP_82, - DowngradeLevelSetList::DOWN_TO_PHP_80, - ]); -}; From 70fe9bdc6b4c588880d0f8411fc141d6d65e15b1 Mon Sep 17 00:00:00 2001 From: Michael Jansen Date: Tue, 17 Dec 2024 06:37:27 +0100 Subject: [PATCH 09/13] Dependencies: Remove `rector` files from `scripts` (#8080) From 659e721363994d4ce74516935779eb2b1fc62da5 Mon Sep 17 00:00:00 2001 From: mjansen Date: Fri, 4 Oct 2024 09:36:07 +0200 Subject: [PATCH 10/13] BackgroundTasks: Use "Fire & Forget" approach when starting workers asynchronously --- .../src/Implementation/TaskManager/AsyncTaskManager.php | 2 +- .../classes/class.ilSoapBackgroundTasksAdministration.php | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/components/ILIAS/BackgroundTasks/src/Implementation/TaskManager/AsyncTaskManager.php b/components/ILIAS/BackgroundTasks/src/Implementation/TaskManager/AsyncTaskManager.php index 8ac4128f63d8..7ad8bbcb4781 100755 --- a/components/ILIAS/BackgroundTasks/src/Implementation/TaskManager/AsyncTaskManager.php +++ b/components/ILIAS/BackgroundTasks/src/Implementation/TaskManager/AsyncTaskManager.php @@ -44,7 +44,7 @@ public function run(Bucket $bucket): void // Call SOAP-Server $soap_client = new \ilSoapClient(); - $soap_client->setResponseTimeout(1); + $soap_client->setResponseTimeout(0); $soap_client->enableWSDL(true); $soap_client->init(); $session_id = session_id(); diff --git a/components/ILIAS/BackgroundTasks_/classes/class.ilSoapBackgroundTasksAdministration.php b/components/ILIAS/BackgroundTasks_/classes/class.ilSoapBackgroundTasksAdministration.php index 96b7e9edc076..a96ae9e732f7 100755 --- a/components/ILIAS/BackgroundTasks_/classes/class.ilSoapBackgroundTasksAdministration.php +++ b/components/ILIAS/BackgroundTasks_/classes/class.ilSoapBackgroundTasksAdministration.php @@ -34,6 +34,8 @@ public function __construct(bool $use_nusoap = true) */ public function runAsync(string $sid) { + ignore_user_abort(true); + $this->initAuth($sid); $this->initIlias(); From 64f54231219f61638f6914837d218108ce37eaad Mon Sep 17 00:00:00 2001 From: Tim Schmitz Date: Mon, 21 Oct 2024 10:41:31 +0200 Subject: [PATCH 11/13] File: fix creation of metadata (42065) --- .../ILIAS/File/classes/trait.ilObjFileMetadata.php | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/components/ILIAS/File/classes/trait.ilObjFileMetadata.php b/components/ILIAS/File/classes/trait.ilObjFileMetadata.php index 0ba52f528f9a..9f977ee544b5 100755 --- a/components/ILIAS/File/classes/trait.ilObjFileMetadata.php +++ b/components/ILIAS/File/classes/trait.ilObjFileMetadata.php @@ -96,11 +96,14 @@ protected function doCreateMetaData(): void global $DIC; // add file size and format to LOM - $DIC->learningObjectMetadata()->manipulate($this->getId(), 0, $this->getType()) - ->prepareCreateOrUpdate($this->getPathToSize(), (string) $this->getFileSize()) -// ->prepareCreateOrUpdate($this->getPathToFirstFormat(), $this->getFileType()) // TIDI thwors exception - ->prepareCreateOrUpdate($this->getPathToVersion(), (string) $this->getVersion()) - ->execute(); + $manipulator = $DIC->learningObjectMetadata() + ->manipulate($this->getId(), 0, $this->getType()) + ->prepareCreateOrUpdate($this->getPathToSize(), (string) $this->getFileSize()) + ->prepareCreateOrUpdate($this->getPathToVersion(), (string) $this->getVersion()); + if ($this->getFileType() !== '') { + $manipulator = $manipulator->prepareCreateOrUpdate($this->getPathToFirstFormat(), $this->getFileType()); + } + $manipulator->execute(); } protected function beforeMDUpdateListener(string $a_element): bool From 518d371bf84c2486390051fe2ad809450eb90b60 Mon Sep 17 00:00:00 2001 From: Lukas Eichenauer Date: Mon, 2 Dec 2024 15:20:00 +0100 Subject: [PATCH 12/13] fix: resource not found handling by merging 8f52610 and fixing default paths --- .../WebAccessChecker/class.ilWACException.php | 3 ++ .../classes/class.ilWACPath.php | 11 ++++---- .../classes/class.ilWebAccessChecker.php | 28 ++++++++----------- .../class.ilWebAccessCheckerDelivery.php | 15 ++++++++-- 4 files changed, 32 insertions(+), 25 deletions(-) diff --git a/components/ILIAS/WebAccessChecker/class.ilWACException.php b/components/ILIAS/WebAccessChecker/class.ilWACException.php index d46c292b5e51..50030023bebe 100755 --- a/components/ILIAS/WebAccessChecker/class.ilWACException.php +++ b/components/ILIAS/WebAccessChecker/class.ilWACException.php @@ -1,4 +1,5 @@ 'The SALT cannot be written to your /data directory. Please check the write permissions on the webserver.', self::MAX_LIFETIME => 'You can only only use lifetimes shorter than ilWACSignedPath::MAX_LIFETIME', self::ACCESS_DENIED_NO_LOGIN => 'No Login found', + self::NOT_FOUND => 'Resource not Found', ]; diff --git a/components/ILIAS/WebAccessChecker/classes/class.ilWACPath.php b/components/ILIAS/WebAccessChecker/classes/class.ilWACPath.php index e5bf5692706d..4cda18939eee 100755 --- a/components/ILIAS/WebAccessChecker/classes/class.ilWACPath.php +++ b/components/ILIAS/WebAccessChecker/classes/class.ilWACPath.php @@ -1,4 +1,5 @@ $v) { + foreach (array_keys($result) as $k => $v) { if (is_numeric($k)) { unset($result[$k]); } @@ -233,7 +234,7 @@ public static function setVideoSuffixes(array $video_suffixes): void protected function normalizePath(string $path): string { $path = ltrim($path, '.'); - $path = urldecode($path); + $path = rawurldecode($path); // cut everything before "data/" (for installations using a subdirectory) $path = strstr($path, '/' . self::DIR_DATA . '/'); @@ -244,8 +245,8 @@ protected function normalizePath(string $path): string $real_data_dir = realpath("./" . self::DIR_DATA); $realpath = realpath("." . $original_path); - if (strpos($realpath, $real_data_dir) !== 0) { - throw new ilWACException(ilWACException::ACCESS_DENIED); + if (!str_starts_with($realpath, $real_data_dir)) { + throw new ilWACException(ilWACException::NOT_FOUND, "Path is not in data directory"); } $normalized_path = ltrim( @@ -257,7 +258,7 @@ protected function normalizePath(string $path): string '/' ); - return "/" . self::DIR_DATA . '/' . $normalized_path . (!empty($query) ? '?' . $query : ''); + return "/" . self::DIR_DATA . '/' . $normalized_path . (empty($query) ? '' : '?' . $query); } public function getPrefix(): string diff --git a/components/ILIAS/WebAccessChecker/classes/class.ilWebAccessChecker.php b/components/ILIAS/WebAccessChecker/classes/class.ilWebAccessChecker.php index 383343e9e38d..0617d4ec65f9 100755 --- a/components/ILIAS/WebAccessChecker/classes/class.ilWebAccessChecker.php +++ b/components/ILIAS/WebAccessChecker/classes/class.ilWebAccessChecker.php @@ -49,17 +49,12 @@ class ilWebAccessChecker * @var int[] */ protected array $applied_checking_methods = []; - private Services $http; - private CookieFactory $cookieFactory; /** * ilWebAccessChecker constructor. */ - public function __construct(Services $httpState, CookieFactory $cookieFactory) + public function __construct(private Services $http, private CookieFactory $cookieFactory) { - $this->setPathObject(new ilWACPath($httpState->request()->getRequestTarget())); - $this->http = $httpState; - $this->cookieFactory = $cookieFactory; } /** @@ -67,12 +62,11 @@ public function __construct(Services $httpState, CookieFactory $cookieFactory) */ public function check(): bool { - if ($this->getPathObject() === null) { - throw new ilWACException(ilWACException::CODE_NO_PATH); - } + $path_object = new ilWACPath($this->http->request()->getRequestTarget()); + $this->setPathObject($path_object); // Check if Path has been signed with a token - $ilWACSignedPath = new ilWACSignedPath($this->getPathObject(), $this->http, $this->cookieFactory); + $ilWACSignedPath = new ilWACSignedPath($path_object, $this->http, $this->cookieFactory); if ($ilWACSignedPath->isSignedPath()) { $this->addAppliedCheckingMethod(self::CM_FILE_TOKEN); if ($ilWACSignedPath->isSignedPathValid()) { @@ -101,11 +95,11 @@ public function check(): bool $this->initILIAS(); // Check if Path is within accepted paths - if ($this->getPathObject()->getModuleType() !== 'rs') { - $clean_path = $this->getPathObject()->getCleanURLdecodedPath(); + if ($path_object->getModuleType() !== 'rs') { + $clean_path = $path_object->getCleanURLdecodedPath(); $path = realpath(__DIR__ . '/../../../../public/' . $clean_path); $data_dir = realpath(CLIENT_WEB_DIR); - if (strpos($path, $data_dir) !== 0) { + if (!str_starts_with($path, $data_dir)) { return false; } if (dirname($path) === $data_dir && is_file($path)) { @@ -113,11 +107,11 @@ public function check(): bool } } - if (ilWACSecurePath::hasCheckingInstanceRegistered($this->getPathObject())) { + if (ilWACSecurePath::hasCheckingInstanceRegistered($path_object)) { // Maybe the path has been registered, lets check - $checkingInstance = ilWACSecurePath::getCheckingInstance($this->getPathObject()); + $checkingInstance = ilWACSecurePath::getCheckingInstance($path_object); $this->addAppliedCheckingMethod(self::CM_CHECKINGINSTANCE); - $canBeDelivered = $checkingInstance->canBeDelivered($this->getPathObject()); + $canBeDelivered = $checkingInstance->canBeDelivered($path_object); if ($canBeDelivered) { $this->sendHeader('checked using fallback'); if ($ilWACSignedPath->isFolderSigned() && $this->isRevalidateFolderTokens()) { @@ -131,7 +125,7 @@ public function check(): bool // none of the checking mechanisms could have been applied. no access $this->setChecked(true); $this->addAppliedCheckingMethod(self::CM_SECFOLDER); - return !$this->getPathObject()->isInSecFolder(); + return !$path_object->isInSecFolder(); } protected function sendHeader(string $message): void diff --git a/components/ILIAS/WebAccessChecker/classes/class.ilWebAccessCheckerDelivery.php b/components/ILIAS/WebAccessChecker/classes/class.ilWebAccessCheckerDelivery.php index f01028150469..8759c9b786ae 100755 --- a/components/ILIAS/WebAccessChecker/classes/class.ilWebAccessCheckerDelivery.php +++ b/components/ILIAS/WebAccessChecker/classes/class.ilWebAccessCheckerDelivery.php @@ -32,6 +32,7 @@ class ilWebAccessCheckerDelivery { private ilWebAccessChecker $wac; private Services $http; + private string $img_dir; public static function run(Services $httpState, CookieFactory $cookieFactory): void @@ -48,6 +49,7 @@ public function __construct(Services $httpState, CookieFactory $cookieFactory) { $this->wac = new ilWebAccessChecker($httpState, $cookieFactory); $this->http = $httpState; + $this->img_dir = realpath(__DIR__ . '/../templates/images'); } @@ -100,7 +102,7 @@ protected function deny(): void protected function deliverDummyImage(): void { - $ilFileDelivery = new Delivery('./components/ILIAS/WebAccessChecker/templates/images/access_denied.png', $this->http); + $ilFileDelivery = new Delivery($this->img_dir . '/access_denied.png', $this->http); $ilFileDelivery->setDisposition($this->wac->getDisposition()); $ilFileDelivery->deliver(); } @@ -108,11 +110,18 @@ protected function deliverDummyImage(): void protected function deliverDummyVideo(): void { - $ilFileDelivery = new Delivery('./components/ILIAS/WebAccessChecker/templates/images/access_denied.mp4', $this->http); + $ilFileDelivery = new Delivery($this->img_dir . '/access_denied.mp4', $this->http); $ilFileDelivery->setDisposition($this->wac->getDisposition()); $ilFileDelivery->stream(); } + protected function handleNotFoundError(ilWACException $e): void + { + $response = $this->http + ->response() + ->withStatus(404); + $this->http->saveResponse($response); + } protected function handleAccessErrors(ilWACException $e): void { @@ -139,7 +148,7 @@ protected function handleAccessErrors(ilWACException $e): void protected function handleErrors(ilWACException $e): void { $response = $this->http->response() - ->withStatus(500); + ->withStatus(500); /** * @var \Psr\Http\Message\StreamInterface $stream From 607085c927cfb43ddec01d17143efdf95d1b1d7a Mon Sep 17 00:00:00 2001 From: fneumann Date: Mon, 30 Sep 2024 11:56:40 +0200 Subject: [PATCH 13/13] Fix #42183: Async background tasks fail silently Improve logging of async background task Check enabled soap to prevent orphaned background task entries Don't check the return value of the SOAP call This will be false in case of time-consuming background tasks. We have to keep the "fire and forget". --- .../TaskManager/AsyncTaskManager.php | 28 +++++++++++++------ 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/components/ILIAS/BackgroundTasks/src/Implementation/TaskManager/AsyncTaskManager.php b/components/ILIAS/BackgroundTasks/src/Implementation/TaskManager/AsyncTaskManager.php index 7ad8bbcb4781..25a1e466e02a 100755 --- a/components/ILIAS/BackgroundTasks/src/Implementation/TaskManager/AsyncTaskManager.php +++ b/components/ILIAS/BackgroundTasks/src/Implementation/TaskManager/AsyncTaskManager.php @@ -23,6 +23,7 @@ use ILIAS\BackgroundTasks\Implementation\Tasks\UserInteraction\UserInteractionRequiredException; use ILIAS\BackgroundTasks\Implementation\Tasks\UserInteraction\UserInteractionSkippedException; use ILIAS\BackgroundTasks\Task\UserInteraction; +use ILIAS\Export\ImportStatus\Exception\ilException; class AsyncTaskManager extends BasicTaskManager { @@ -36,11 +37,19 @@ public function run(Bucket $bucket): void { global $DIC; + // check this before saving the bucket state to prevent an orphaned entry with 0% + if (!$DIC->settings()->get('soap_user_administration')) { + $DIC->logger()->bgtk()->warning("SOAP not enabled, fallback to sync version"); + $sync_manager = new SyncTaskManager($this->persistence); + $sync_manager->run($bucket); + return; + } + $bucket->setState(State::SCHEDULED); $bucket->setCurrentTask($bucket->getTask()); $DIC->backgroundTasks()->persistence()->saveBucketAndItsTasks($bucket); - $DIC->logger()->root()->info("[BT] Trying to call webserver"); + $DIC->logger()->bgtk()->info("Trying to call webserver"); // Call SOAP-Server $soap_client = new \ilSoapClient(); @@ -63,12 +72,13 @@ public function run(Bucket $bucket): void $session_id . '::' . $client_id, )); } catch (\Throwable $t) { - $DIC->logger()->root()->info("[BT] Calling Webserver failed, fallback to sync version"); + $DIC->logger()->bgtk()->warning($t->getMessage()); + $DIC->logger()->bgtk()->warning("Calling webserver failed, fallback to sync version"); $sync_manager = new SyncTaskManager($this->persistence); $sync_manager->run($bucket); - } finally { - $DIC->logger()->root()->info("[BT] Calling webserver successful"); + return; } + $DIC->logger()->bgtk()->info("Calling webserver successful"); } /** @@ -81,13 +91,13 @@ public function runAsync() $n_of_tasks = $ilIliasIniFile->readVariable("background_tasks", "number_of_concurrent_tasks"); $n_of_tasks = $n_of_tasks ? $n_of_tasks : 5; - $DIC->logger()->root()->info("[BackgroundTask] Starting background job."); + $DIC->logger()->bgtk()->info("Starting background job."); $persistence = $DIC->backgroundTasks()->persistence(); // TODO search over all clients. $MAX_PARALLEL_JOBS = $n_of_tasks; if (count($persistence->getBucketIdsByState(State::RUNNING)) >= $MAX_PARALLEL_JOBS) { - $DIC->logger()->root()->info("[BT] Too many running jobs, worker going down."); + $DIC->logger()->bgtk()->info("Too many running jobs, worker going down."); return; } @@ -114,14 +124,14 @@ public function runAsync() $this->persistence->saveBucketAndItsTasks($bucket); } catch (\Exception $e) { $persistence->deleteBucket($bucket); - $DIC->logger()->root()->info("[BT] Exception while async computing: " + $DIC->logger()->bgtk()->info("Exception while async computing: " . $e->getMessage()); - $DIC->logger()->root()->info("[BT] Stack Trace: " + $DIC->logger()->bgtk()->info("Stack Trace: " . $e->getTraceAsString()); } } - $DIC->logger()->root()->info("[BT] One worker going down because there's nothing left to do."); + $DIC->logger()->bgtk()->info("One worker going down because there's nothing left to do."); return true; }