From 4f82e59192ccd8db3a0c680729d45e5ce2088797 Mon Sep 17 00:00:00 2001 From: daniwe4 Date: Thu, 18 Nov 2021 13:45:33 +0100 Subject: [PATCH 1/9] Setup: add composer dependency Seld\JsonLint and use it in ConfigReader --- composer.json | 13 +++++- composer.lock | 65 +++++++++++++++++++++++++++- setup/cli.php | 5 +++ src/Setup/CLI/ConfigReader.php | 11 ++++- tests/Setup/CLI/ConfigReaderTest.php | 12 ++--- 5 files changed, 96 insertions(+), 10 deletions(-) diff --git a/composer.json b/composer.json index cef6c60cb381..7cd6f9319e3d 100644 --- a/composer.json +++ b/composer.json @@ -54,7 +54,8 @@ "ramsey/uuid": "^3.8", "enshrined/svg-sanitize": "^0.13.0", "guzzlehttp/guzzle": "^6.3", - "simplesamlphp/simplesamlphp": "^1.18" + "simplesamlphp/simplesamlphp": "^1.18", + "seld/jsonlint": "^1.8" }, "require-dev": { "mikey179/vfsstream": "^1.6", @@ -397,6 +398,16 @@ "approved-by": "see security issue 20339", "approved-date": "2017-04-05" }, + "seld/jsonlint" : { + "source" : "https://github.com/Seldaek/jsonlint", + "used_version" : "v1.8.3", + "wrapped_by" : null, + "added_by" : "Daniel Weise ", + "last_update" : "2021-11-18", + "last_update_by" : "Daniel Weise ", + "approved-by": "Jour Fix", + "approved-date": "2021-11-29" + }, "patches": { "tecnickcom/tcpdf": { "ILIAS TCPDF Patches": "./libs/composer/patches/tcpdf.patch" diff --git a/composer.lock b/composer.lock index 361431142d3f..d0d5796f57b0 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "50df347b74ce2011c4ac32f79dd0df6b", + "content-hash": "f8ec8daf049be704ddf00e48dc0ba1c0", "packages": [ { "name": "cweagans/composer-patches", @@ -2729,6 +2729,69 @@ ], "time": "2020-10-03T10:08:14+00:00" }, + { + "name": "seld/jsonlint", + "version": "1.8.3", + "source": { + "type": "git", + "url": "https://github.com/Seldaek/jsonlint.git", + "reference": "9ad6ce79c342fbd44df10ea95511a1b24dee5b57" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/Seldaek/jsonlint/zipball/9ad6ce79c342fbd44df10ea95511a1b24dee5b57", + "reference": "9ad6ce79c342fbd44df10ea95511a1b24dee5b57", + "shasum": "" + }, + "require": { + "php": "^5.3 || ^7.0 || ^8.0" + }, + "require-dev": { + "phpunit/phpunit": "^4.8.35 || ^5.7 || ^6.0" + }, + "bin": [ + "bin/jsonlint" + ], + "type": "library", + "autoload": { + "psr-4": { + "Seld\\JsonLint\\": "src/Seld/JsonLint/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Jordi Boggiano", + "email": "j.boggiano@seld.be", + "homepage": "http://seld.be" + } + ], + "description": "JSON Linter", + "keywords": [ + "json", + "linter", + "parser", + "validator" + ], + "support": { + "issues": "https://github.com/Seldaek/jsonlint/issues", + "source": "https://github.com/Seldaek/jsonlint/tree/1.8.3" + }, + "funding": [ + { + "url": "https://github.com/Seldaek", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/seld/jsonlint", + "type": "tidelift" + } + ], + "time": "2020-11-11T09:19:24+00:00" + }, { "name": "simplesamlphp/composer-module-installer", "version": "v1.1.8", diff --git a/setup/cli.php b/setup/cli.php index f8786d189bc2..927e7e0242da 100644 --- a/setup/cli.php +++ b/setup/cli.php @@ -144,6 +144,7 @@ function build_container_for_setup(string $executed_in_directory) : \Pimple\Cont $c["config_reader"] = function ($c) use ($executed_in_directory) { return new \ILIAS\Setup\CLI\ConfigReader( + $c["json.parser"], $executed_in_directory ); }; @@ -156,5 +157,9 @@ function build_container_for_setup(string $executed_in_directory) : \Pimple\Cont return new \ilPluginRawReader(); }; + $c["json.parser"] = function ($c) { + return new \Seld\JsonLint\JsonParser(); + }; + return $c; } diff --git a/src/Setup/CLI/ConfigReader.php b/src/Setup/CLI/ConfigReader.php index eaf78805a022..1debaa43fe66 100644 --- a/src/Setup/CLI/ConfigReader.php +++ b/src/Setup/CLI/ConfigReader.php @@ -11,16 +11,19 @@ use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; +use Seld\JsonLint\JsonParser; /** * Read a json-formatted config from a file and overwrite some fields. */ class ConfigReader { + protected JsonParser $json_parser; protected string $base_dir; - public function __construct(string $base_dir = null) + public function __construct(JsonParser $json_parser, string $base_dir = null) { + $this->json_parser = $json_parser; $this->base_dir = $base_dir ?? getcwd(); } @@ -41,7 +44,11 @@ public function readConfigFile(string $name, array $overwrites = []) : array "Config-file '$name' does not exist or is not readable." ); } - $json = json_decode(file_get_contents($name), (bool) JSON_OBJECT_AS_ARRAY); + $json = $this->json_parser->parse( + file_get_contents($name), + JsonParser::PARSE_TO_ASSOC | JsonParser::DETECT_KEY_CONFLICTS + ); + if (!is_array($json)) { throw new \InvalidArgumentException( "Could not find JSON-array in '$name'." diff --git a/tests/Setup/CLI/ConfigReaderTest.php b/tests/Setup/CLI/ConfigReaderTest.php index 9e619039cff2..5c0bbb88258b 100644 --- a/tests/Setup/CLI/ConfigReaderTest.php +++ b/tests/Setup/CLI/ConfigReaderTest.php @@ -6,6 +6,7 @@ use ILIAS\Setup; use PHPUnit\Framework\TestCase; +use Seld\JsonLint\JsonParser; class ConfigReaderTest extends TestCase { @@ -18,8 +19,7 @@ public function testReadConfigFile() : void ] ]; file_put_contents($filename, json_encode($expected)); - - $obj = new Setup\CLI\ConfigReader(); + $obj = new Setup\CLI\ConfigReader(new JsonParser()); $config = $obj->readConfigFile($filename); @@ -36,7 +36,7 @@ public function testBaseDir() : void ]; file_put_contents($filename, json_encode($expected)); - $obj = new Setup\CLI\ConfigReader("/tmp"); + $obj = new Setup\CLI\ConfigReader(new JsonParser(), "/tmp"); $config = $obj->readConfigFile(basename($filename)); @@ -53,7 +53,7 @@ public function testTotalDir() : void ]; file_put_contents($filename, json_encode($expected)); - $obj = new Setup\CLI\ConfigReader("/foo"); + $obj = new Setup\CLI\ConfigReader(new JsonParser(), "/foo"); $config = $obj->readConfigFile($filename); @@ -62,7 +62,7 @@ public function testTotalDir() : void public function testApplyOverwrites() : void { - $cr = new class() extends Setup\CLI\ConfigReader { + $cr = new class(new JsonParser()) extends Setup\CLI\ConfigReader { public function _applyOverwrites($j, $o) { return $this->applyOverwrites($j, $o); @@ -98,7 +98,7 @@ public function _applyOverwrites($j, $o) public function testApplyOverwritesToUnsetField() : void { - $cr = new class() extends Setup\CLI\ConfigReader { + $cr = new class(new JsonParser()) extends Setup\CLI\ConfigReader { public function _applyOverwrites($j, $o) { return $this->applyOverwrites($j, $o); From ad46f6a095259ea6e9ca04b2c4d0a0cc71fdb73a Mon Sep 17 00:00:00 2001 From: Matthias Kunkel Date: Mon, 6 Dec 2021 14:52:47 +0100 Subject: [PATCH 2/9] Added lang var that has been removed accidentally by lang file cleaner - and removed deprecated lang var --- lang/ilias_en.lang | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lang/ilias_en.lang b/lang/ilias_en.lang index a8466854d5b9..3c34a67a4aca 100755 --- a/lang/ilias_en.lang +++ b/lang/ilias_en.lang @@ -1996,7 +1996,6 @@ cntr#:#cntr_view_info_sessions#:#This content presentation groups all sessions f cntr#:#cntr_view_info_simple#:#This content presentation lists all items in one block. cntr#:#cntr_view_sessions#:#Sessions View cntr#:#cntr_view_simple#:#Simple View -common#:#3rd_party_software#:#3rd party software common#:#absolute_path#:#Absolute Path common#:#accept_usr_agreement#:#Accept terms of service? common#:#withdraw_consent_header#:#Revoke acceptance to terms of service @@ -2829,6 +2828,7 @@ common#:#hint#:#Hint common#:#hist_lm_delete_pg#:#Page "%1" [%2] has been deleted. common#:#hist_lm_delete_st#:#Chapter "%1" [%2] has been deleted. common#:#hist_lm_pg_create#:#Page created. +common#:#hist_lm_pg_update#:#Page changed. common#:#hist_lm_st_create#:#Chapter created. common#:#hist_webr_add#:#Added new Weblink with title: "%1" common#:#hist_webr_delete#:#Deleted Weblink with title: "%1" From 1afbaeee4716142344b07cd615e07980547a5d9f Mon Sep 17 00:00:00 2001 From: Matthias Kunkel Date: Mon, 6 Dec 2021 14:59:49 +0100 Subject: [PATCH 3/9] Added lang var that has been removed accidentally by lang file cleaner - and removed deprecated lang var --- lang/ilias_de.lang | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lang/ilias_de.lang b/lang/ilias_de.lang index 0a072ff6caa0..0115ab48b1e5 100755 --- a/lang/ilias_de.lang +++ b/lang/ilias_de.lang @@ -2401,7 +2401,6 @@ cntr#:#cntr_view_sessions#:#Sitzungsansicht cntr#:#cntr_view_simple#:#Einfache Liste cntr#:#container_import_zip_file_invalid#:#Die hochgeladene Datei ist kein gültiger ILIAS Export. Um Verzeichnisstrukturen hochzuladen benutzen Sie bitte ein Datei-Objekt. cntr#:#edit_questions#:#Fragen bearbeiten -common#:#3rd_party_software#:#Drittanbieter-Software common#:#absolute_path#:#Absoluter Pfad common#:#accept_usr_agreement#:#Nutzungsvereinbarung akzeptieren? common#:#withdraw_consent_header#:#Einverständnis zur Nutzungsvereinbarung widerrufen @@ -3290,6 +3289,7 @@ common#:#hint#:#Hinweis common#:#hist_lm_delete_pg#:#Seite "%1" [%2] wurde gelöscht. common#:#hist_lm_delete_st#:#Kapitel "%1" [%2] wurde gelöscht. common#:#hist_lm_pg_create#:#Seite erstellt. +common#:#hist_lm_pg_update#:#Seite geändert. common#:#hist_lm_st_create#:#Kapitel erstellt. common#:#hist_webr_add#:#Neuer Weblink angelegt mit Titel: "%1" common#:#hist_webr_delete#:#Weblink wurde gelöscht. Titel: "%1" From f5926e33ee21aa61629ec94dc573b5dfa5a589d8 Mon Sep 17 00:00:00 2001 From: Lukas Scharmer Date: Thu, 11 Nov 2021 17:44:38 +0100 Subject: [PATCH 4/9] Refactored randomization to use the new refinery randomization --- Modules/Test/classes/class.ilObjTestGUI.php | 3 +- .../classes/class.ilTestPlayerAbstractGUI.php | 18 ++-- Modules/Test/test/ilTestBaseTestCase.php | 9 ++ .../ilTestPlayerDynamicQuestionSetGUITest.php | 1 + Modules/Test/test/ilTestPlayerFactoryTest.php | 1 + .../ilTestPlayerFixedQuestionSetGUITest.php | 1 + .../ilTestPlayerRandomQuestionSetGUITest.php | 1 + .../classes/class.assClozeGap.php | 12 +-- .../classes/class.assClozeTest.php | 9 +- .../classes/class.assClozeTestGUI.php | 15 +++- .../classes/class.assKprimChoiceGUI.php | 2 +- .../classes/class.assMatchingQuestion.php | 20 +++-- .../classes/class.assMatchingQuestionGUI.php | 24 ++---- .../classes/class.assMultipleChoiceGUI.php | 2 +- .../classes/class.assOrderingHorizontal.php | 2 +- .../classes/class.assOrderingQuestion.php | 4 +- .../classes/class.assQuestion.php | 12 +-- .../classes/class.assSingleChoiceGUI.php | 2 +- .../classes/class.ilAssQuestionPreviewGUI.php | 22 ++--- .../classes/class.ilObjQuestionPoolGUI.php | 3 +- .../export/qti12/class.assClozeTestExport.php | 19 ++++- .../feedback/class.ilAssClozeTestFeedback.php | 27 ++++-- .../ilAssLacCompositeValidator.php | 9 +- .../TestQuestionPool/test/assBaseTestCase.php | 6 ++ .../TestQuestionPool/test/assClozeGapTest.php | 83 ++++++++++--------- .../test/assClozeSelectGapTest.php | 20 +++-- src/Refinery/Factory.php | 6 ++ src/Refinery/Random/Group.php | 36 ++++++++ src/Refinery/Random/Seed/GivenSeed.php | 21 +++++ src/Refinery/Random/Seed/RandomSeed.php | 22 +++++ src/Refinery/Random/Seed/Seed.php | 11 +++ .../Transformation/IdentityTransformation.php | 59 +++++++++++++ .../Transformation/ShuffleTransformation.php | 46 ++++++++++ tests/Refinery/Random/GroupTest.php | 40 +++++++++ .../IdentityTransformationTest.php | 39 +++++++++ .../ShuffleTransformationTest.php | 35 ++++++++ 36 files changed, 523 insertions(+), 119 deletions(-) create mode 100644 src/Refinery/Random/Group.php create mode 100644 src/Refinery/Random/Seed/GivenSeed.php create mode 100644 src/Refinery/Random/Seed/RandomSeed.php create mode 100644 src/Refinery/Random/Seed/Seed.php create mode 100644 src/Refinery/Random/Transformation/IdentityTransformation.php create mode 100644 src/Refinery/Random/Transformation/ShuffleTransformation.php create mode 100644 tests/Refinery/Random/GroupTest.php create mode 100644 tests/Refinery/Random/Transformation/IdentityTransformationTest.php create mode 100644 tests/Refinery/Random/Transformation/ShuffleTransformationTest.php diff --git a/Modules/Test/classes/class.ilObjTestGUI.php b/Modules/Test/classes/class.ilObjTestGUI.php index 5d63a28e3f4f..da18c358b3e5 100755 --- a/Modules/Test/classes/class.ilObjTestGUI.php +++ b/Modules/Test/classes/class.ilObjTestGUI.php @@ -156,6 +156,7 @@ public function executeCommand() $tree = $DIC['tree']; $ilias = $DIC['ilias']; $ilUser = $DIC['ilUser']; + $randomGroup = $DIC->refinery()->random(); $cmd = $this->ctrl->getCmd("infoScreen"); @@ -659,7 +660,7 @@ public function executeCommand() $this->ctrl->saveParameter($this, "q_id"); require_once 'Modules/TestQuestionPool/classes/class.ilAssQuestionPreviewGUI.php'; - $gui = new ilAssQuestionPreviewGUI($this->ctrl, $this->tabs_gui, $this->tpl, $this->lng, $ilDB, $ilUser); + $gui = new ilAssQuestionPreviewGUI($this->ctrl, $this->tabs_gui, $this->tpl, $this->lng, $ilDB, $ilUser, $randomGroup); $gui->initQuestion($this->fetchAuthoringQuestionIdParameter(), $this->object->getId()); $gui->initPreviewSettings($this->object->getRefId()); diff --git a/Modules/Test/classes/class.ilTestPlayerAbstractGUI.php b/Modules/Test/classes/class.ilTestPlayerAbstractGUI.php index 3aa628102c43..59f38c1960eb 100755 --- a/Modules/Test/classes/class.ilTestPlayerAbstractGUI.php +++ b/Modules/Test/classes/class.ilTestPlayerAbstractGUI.php @@ -1,6 +1,11 @@ processLocker = null; $this->testSession = null; $this->assSettings = null; + $this->randomGroup = $DIC->refinery()->random(); } protected function checkReadAccess() @@ -2534,17 +2542,13 @@ protected function initTestQuestionConfig(assQuestion $questionOBJ) /** * @param $questionId - * @return ilArrayElementShuffler + * @return Transformation */ protected function buildQuestionAnswerShuffler($questionId) { - require_once 'Services/Randomization/classes/class.ilArrayElementShuffler.php'; - $shuffler = new ilArrayElementShuffler(); - $fixedSeed = $this->buildFixedShufflerSeed($questionId); - $shuffler->setSeed($fixedSeed); - - return $shuffler; + + return $this->randomGroup->shuffleArray(new GivenSeed($fixedSeed)); } /** diff --git a/Modules/Test/test/ilTestBaseTestCase.php b/Modules/Test/test/ilTestBaseTestCase.php index b6a7b976bf0b..5acb4a6e4fff 100644 --- a/Modules/Test/test/ilTestBaseTestCase.php +++ b/Modules/Test/test/ilTestBaseTestCase.php @@ -9,6 +9,8 @@ use ILIAS\Filesystem\Filesystems; use ILIAS\HTTP\Services; use ILIAS\UI\Implementation\Factory; +use ILIAS\Refinery\Factory as RefineryFactory; +use ILIAS\Refinery\Random\Group as RandomGroup; /** * Class ilTestBaseClass @@ -216,4 +218,11 @@ protected function addGlobal_uiRenderer() : void { $this->setGlobalVariable("ui.renderer", $this->createMock(ILIAS\UI\Implementation\DefaultRenderer::class)); } + + protected function addGlobal_refinery() : void + { + $refineryMock = $this->getMockBuilder(RefineryFactory::class)->disableOriginalConstructor()->getMock(); + $refineryMock->expects(self::any())->method('random')->willReturn($this->getMockBuilder(RandomGroup::class)->getMock()); + $this->setGlobalVariable("refinery", $refineryMock); + } } diff --git a/Modules/Test/test/ilTestPlayerDynamicQuestionSetGUITest.php b/Modules/Test/test/ilTestPlayerDynamicQuestionSetGUITest.php index c7df7803149d..3799eac0ecbe 100644 --- a/Modules/Test/test/ilTestPlayerDynamicQuestionSetGUITest.php +++ b/Modules/Test/test/ilTestPlayerDynamicQuestionSetGUITest.php @@ -25,6 +25,7 @@ protected function setUp() : void $this->addGlobal_ilObjDataCache(); $this->addGlobal_rbacsystem(); $this->addGlobal_ilUser(); + $this->addGlobal_refinery(); $_GET["ref_id"] = 2; diff --git a/Modules/Test/test/ilTestPlayerFactoryTest.php b/Modules/Test/test/ilTestPlayerFactoryTest.php index 36bdf681f2da..a1ff8d6ae6a9 100644 --- a/Modules/Test/test/ilTestPlayerFactoryTest.php +++ b/Modules/Test/test/ilTestPlayerFactoryTest.php @@ -40,6 +40,7 @@ public function testGetPlayerGUI() : void $this->addGlobal_ilObjDataCache(); $_GET["ref_id"] = 2; $this->addGlobal_rbacsystem(); + $this->addGlobal_refinery(); $objTest = new ilObjTest(); diff --git a/Modules/Test/test/ilTestPlayerFixedQuestionSetGUITest.php b/Modules/Test/test/ilTestPlayerFixedQuestionSetGUITest.php index 7112fe76b3d5..6a9c5ff64f66 100644 --- a/Modules/Test/test/ilTestPlayerFixedQuestionSetGUITest.php +++ b/Modules/Test/test/ilTestPlayerFixedQuestionSetGUITest.php @@ -27,6 +27,7 @@ protected function setUp() : void $this->addGlobal_ilObjDataCache(); $this->addGlobal_rbacsystem(); $this->addGlobal_ilUser(); + $this->addGlobal_refinery(); $this->testObj = new ilTestPlayerFixedQuestionSetGUI( $this->createMock(ilObjTest::class) diff --git a/Modules/Test/test/ilTestPlayerRandomQuestionSetGUITest.php b/Modules/Test/test/ilTestPlayerRandomQuestionSetGUITest.php index cb5694e430bd..d9d6559067a8 100644 --- a/Modules/Test/test/ilTestPlayerRandomQuestionSetGUITest.php +++ b/Modules/Test/test/ilTestPlayerRandomQuestionSetGUITest.php @@ -25,6 +25,7 @@ protected function setUp() : void $this->addGlobal_ilObjDataCache(); $this->addGlobal_rbacsystem(); $this->addGlobal_ilUser(); + $this->addGlobal_refinery(); $_GET["ref_id"] = "0"; diff --git a/Modules/TestQuestionPool/classes/class.assClozeGap.php b/Modules/TestQuestionPool/classes/class.assClozeGap.php index 5c2f410e09a8..90cf5233a5d3 100755 --- a/Modules/TestQuestionPool/classes/class.assClozeGap.php +++ b/Modules/TestQuestionPool/classes/class.assClozeGap.php @@ -1,6 +1,8 @@ getShuffle()) { - return $shuffler->shuffle($this->items); + return $shuffler->transform($this->items); } return $this->items; @@ -334,11 +336,11 @@ public function getBestSolutionIndexes() } /** - * @param ilRandomArrayElementProvider $shuffler + * @param Transformation $shuffler * @param null | array $combinations * @return string */ - public function getBestSolutionOutput(ilRandomArrayElementProvider $shuffler, $combinations = null) + public function getBestSolutionOutput(Transformation $shuffler, $combinations = null) { global $DIC; $lng = $DIC['lng']; diff --git a/Modules/TestQuestionPool/classes/class.assClozeTest.php b/Modules/TestQuestionPool/classes/class.assClozeTest.php index 98bc43b08025..b2e590bd6eb1 100755 --- a/Modules/TestQuestionPool/classes/class.assClozeTest.php +++ b/Modules/TestQuestionPool/classes/class.assClozeTest.php @@ -1,6 +1,8 @@ start_tag = "[gap]"; $this->end_tag = "[/gap]"; @@ -130,6 +136,7 @@ public function __construct( $this->identical_scoring = 1; $this->gap_combinations_exists = false; $this->gap_combinations = array(); + $this->randomGroup = $DIC->refinery()->random(); } /** @@ -2010,7 +2017,7 @@ public function isAddableAnswerOptionValue(int $qIndex, string $answerOptionValu return false; } - foreach ($gap->getItems(new ilDeterministicArrayElementProvider()) as $item) { + foreach ($gap->getItems($this->randomGroup->dontShuffle()) as $item) { if ($item->getAnswertext() == $answerOptionValue) { return false; } diff --git a/Modules/TestQuestionPool/classes/class.assClozeTestGUI.php b/Modules/TestQuestionPool/classes/class.assClozeTestGUI.php index 803c5170eca7..1642fef8b0c5 100755 --- a/Modules/TestQuestionPool/classes/class.assClozeTestGUI.php +++ b/Modules/TestQuestionPool/classes/class.assClozeTestGUI.php @@ -1,6 +1,9 @@ object = new assClozeTest(); if ($id >= 0) { $this->object->loadFromDb($id); } + + $this->randomGroup = $DIC->refinery()->random(); } public function getCommand($cmd) @@ -1432,7 +1441,7 @@ public function getAggregatedAnswersView($relevant_answers) if ($gap->type == CLOZE_TEXT) { $present_elements = array(); - foreach ($gap->getItems(new ilArrayElementShuffler()) as $item) { + foreach ($gap->getItems($this->randomGroup->shuffleArray(new RandomSeed())) as $item) { /** @var assAnswerCloze $item */ $present_elements[] = $item->getAnswertext(); } @@ -1577,7 +1586,7 @@ protected function getAnswerTextLabel($gapIndex, $answer) case CLOZE_SELECT: - $items = $gap->getItems(new ilDeterministicArrayElementProvider()); + $items = $gap->getItems($this->randomGroup->dontShuffle()); return $items[$answer]->getAnswertext(); } } @@ -1593,7 +1602,7 @@ protected function completeAddAnswerAction($answers, $questionIndex) foreach ($answers as $key => $ans) { $found = false; - foreach ($gap->getItems(new ilDeterministicArrayElementProvider()) as $item) { + foreach ($gap->getItems($this->randomGroup->dontShuffle()) as $item) { if ($ans['answer'] !== $item->getAnswerText()) { continue; } diff --git a/Modules/TestQuestionPool/classes/class.assKprimChoiceGUI.php b/Modules/TestQuestionPool/classes/class.assKprimChoiceGUI.php index b3f14e67cc18..84d108d7daa6 100644 --- a/Modules/TestQuestionPool/classes/class.assKprimChoiceGUI.php +++ b/Modules/TestQuestionPool/classes/class.assKprimChoiceGUI.php @@ -707,7 +707,7 @@ protected function getParticipantsAnswerKeySequence() $choiceKeys = array_keys($this->object->getAnswers()); if ($this->object->isShuffleAnswersEnabled()) { - $choiceKeys = $this->object->getShuffler()->shuffle($choiceKeys); + $choiceKeys = $this->object->getShuffler()->transform($choiceKeys); } return $choiceKeys; diff --git a/Modules/TestQuestionPool/classes/class.assMatchingQuestion.php b/Modules/TestQuestionPool/classes/class.assMatchingQuestion.php index 68a179179cd0..f29aa7b88db3 100755 --- a/Modules/TestQuestionPool/classes/class.assMatchingQuestion.php +++ b/Modules/TestQuestionPool/classes/class.assMatchingQuestion.php @@ -1,6 +1,9 @@ matchingpairs = array(); $this->matching_type = $matching_type; $this->terms = array(); $this->definitions = array(); + $this->randomGroup = $DIC->refinery()->random(); } /** @@ -1380,14 +1388,11 @@ public function toJSON() : string 'onenotcorrect' => $this->formatSAQuestion($this->feedbackOBJ->getGenericFeedbackTestPresentation($this->getId(), false)), 'allcorrect' => $this->formatSAQuestion($this->feedbackOBJ->getGenericFeedbackTestPresentation($this->getId(), true)) ); - - require_once 'Services/Randomization/classes/class.ilArrayElementShuffler.php'; - $this->setShuffler(new ilArrayElementShuffler()); - $seed = $this->getShuffler()->getSeed(); + + $this->setShuffler($this->randomGroup->shuffleArray(new RandomSeed())); $terms = array(); - $this->getShuffler()->setSeed($this->getShuffler()->buildSeedFromString($seed . 'terms')); - foreach ($this->getShuffler()->shuffle($this->getTerms()) as $term) { + foreach ($this->getShuffler()->transform($this->getTerms()) as $term) { $terms[] = array( "text" => $this->formatSAQuestion($term->text), "id" => (int) $this->getId() . $term->identifier @@ -1403,8 +1408,7 @@ public function toJSON() : string // when the second one (the copy) is answered. $definitions = array(); - $this->getShuffler()->setSeed($this->getShuffler()->buildSeedFromString($seed . 'definitions')); - foreach ($this->getShuffler()->shuffle($this->getDefinitions()) as $def) { + foreach ($this->getShuffler()->transform($this->getDefinitions()) as $def) { $definitions[] = array( "text" => $this->formatSAQuestion((string) $def->text), "id" => (int) $this->getId() . $def->identifier diff --git a/Modules/TestQuestionPool/classes/class.assMatchingQuestionGUI.php b/Modules/TestQuestionPool/classes/class.assMatchingQuestionGUI.php index eb46dc5d0c63..e3767ebaea3c 100755 --- a/Modules/TestQuestionPool/classes/class.assMatchingQuestionGUI.php +++ b/Modules/TestQuestionPool/classes/class.assMatchingQuestionGUI.php @@ -632,18 +632,14 @@ public function getPreview($show_question_only = false, $showInlineFeedback = fa $definitions = $this->object->getDefinitions(); switch ($this->object->getShuffle()) { case 1: - $seed = $this->object->getShuffler()->getSeed(); - $this->object->getShuffler()->setSeed($seed . '1'); - $terms = $this->object->getShuffler()->shuffle($terms); - $this->object->getShuffler()->setSeed($seed . '2'); - $definitions = $this->object->getShuffler()->shuffle($definitions); - $this->object->getShuffler()->setSeed($seed); + $terms = $this->object->getShuffler()->transform($terms); + $definitions = $this->object->getShuffler()->transform($definitions); break; case 2: - $terms = $this->object->getShuffler()->shuffle($terms); + $terms = $this->object->getShuffler()->transform($terms); break; case 3: - $definitions = $this->object->getShuffler()->shuffle($definitions); + $definitions = $this->object->getShuffler()->transform($definitions); break; } @@ -828,25 +824,21 @@ public function getTestOutput($active_id, $pass, $is_postponed = false, $user_po $definitions = $this->object->getDefinitions(); switch ($this->object->getShuffle()) { case 1: - $seed = $this->object->getShuffler()->getSeed(); - $this->object->getShuffler()->setSeed($seed . '1'); - $terms = $this->object->getShuffler()->shuffle($terms); + $terms = $this->object->getShuffler()->transform($terms); if (count($solutions)) { $definitions = $this->sortDefinitionsBySolution($solutions, $definitions); } else { - $this->object->getShuffler()->setSeed($seed . '2'); - $definitions = $this->object->getShuffler()->shuffle($definitions); + $definitions = $this->object->getShuffler()->transform($definitions); } - $this->object->getShuffler()->setSeed($seed); break; case 2: - $terms = $this->object->getShuffler()->shuffle($terms); + $terms = $this->object->getShuffler()->transform($terms); break; case 3: if (count($solutions)) { $definitions = $this->sortDefinitionsBySolution($solutions, $definitions); } else { - $definitions = $this->object->getShuffler()->shuffle($definitions); + $definitions = $this->object->getShuffler()->transform($definitions); } break; } diff --git a/Modules/TestQuestionPool/classes/class.assMultipleChoiceGUI.php b/Modules/TestQuestionPool/classes/class.assMultipleChoiceGUI.php index 03ad9ca04c02..94f3e79ea5be 100755 --- a/Modules/TestQuestionPool/classes/class.assMultipleChoiceGUI.php +++ b/Modules/TestQuestionPool/classes/class.assMultipleChoiceGUI.php @@ -737,7 +737,7 @@ public function getChoiceKeys() $choiceKeys = array_keys($this->object->answers); if ($this->object->getShuffle()) { - $choiceKeys = $this->object->getShuffler()->shuffle($choiceKeys); + $choiceKeys = $this->object->getShuffler()->transform($choiceKeys); } return $choiceKeys; diff --git a/Modules/TestQuestionPool/classes/class.assOrderingHorizontal.php b/Modules/TestQuestionPool/classes/class.assOrderingHorizontal.php index 1f8555ba1616..624c4ff7a6f1 100644 --- a/Modules/TestQuestionPool/classes/class.assOrderingHorizontal.php +++ b/Modules/TestQuestionPool/classes/class.assOrderingHorizontal.php @@ -524,7 +524,7 @@ public function getOrderingElements() public function getRandomOrderingElements() { $elements = $this->getOrderingElements(); - $elements = $this->getShuffler()->shuffle($elements); + $elements = $this->getShuffler()->transform($elements); return $elements; } diff --git a/Modules/TestQuestionPool/classes/class.assOrderingQuestion.php b/Modules/TestQuestionPool/classes/class.assOrderingQuestion.php index bf1448de988a..6652b793f23d 100755 --- a/Modules/TestQuestionPool/classes/class.assOrderingQuestion.php +++ b/Modules/TestQuestionPool/classes/class.assOrderingQuestion.php @@ -544,7 +544,7 @@ public function getSolutionOrderingElementList($indexedSolutionValues) */ public function getShuffledOrderingElementList() { - $shuffledRandomIdentifierIndex = $this->getShuffler()->shuffle( + $shuffledRandomIdentifierIndex = $this->getShuffler()->transform( $this->getOrderingElementList()->getRandomIdentifierIndex() ); @@ -1163,7 +1163,7 @@ public function toJSON() : string $answers[$counter] = $orderingElement->getContent(); $counter++; } - $answers = $this->getShuffler()->shuffle($answers); + $answers = $this->getShuffler()->transform($answers); $arr = array(); foreach ($answers as $order => $answer) { array_push($arr, array( diff --git a/Modules/TestQuestionPool/classes/class.assQuestion.php b/Modules/TestQuestionPool/classes/class.assQuestion.php index 73a7a2866cd5..e181739a7dfd 100755 --- a/Modules/TestQuestionPool/classes/class.assQuestion.php +++ b/Modules/TestQuestionPool/classes/class.assQuestion.php @@ -1,6 +1,8 @@ array('png'), 'image/gif' => array('gif') ); - + /** * assQuestion constructor */ @@ -212,7 +214,7 @@ public function __construct( $this->questionActionCmd = 'handleQuestionAction'; - $this->shuffler = new ilDeterministicArrayElementProvider(); + $this->shuffler = $DIC->refinery()->random()->dontShuffle(); $this->lifecycle = ilAssQuestionLifecycle::getDraftInstance(); } @@ -360,12 +362,12 @@ public static function getAllowedImageMaterialFileExtensions() : array return array_unique($extensions); } - public function getShuffler() : ilRandomArrayElementProvider + public function getShuffler() : Transformation { return $this->shuffler; } - public function setShuffler(ilRandomArrayElementProvider $shuffler) : void + public function setShuffler(Transformation $shuffler) : void { $this->shuffler = $shuffler; } diff --git a/Modules/TestQuestionPool/classes/class.assSingleChoiceGUI.php b/Modules/TestQuestionPool/classes/class.assSingleChoiceGUI.php index 6bd9c78d2774..fa33fef78332 100755 --- a/Modules/TestQuestionPool/classes/class.assSingleChoiceGUI.php +++ b/Modules/TestQuestionPool/classes/class.assSingleChoiceGUI.php @@ -610,7 +610,7 @@ public function getChoiceKeys() $choiceKeys = array_keys($this->object->answers); if ($this->object->getShuffle()) { - $choiceKeys = $this->object->getShuffler()->shuffle($choiceKeys); + $choiceKeys = $this->object->getShuffler()->transform($choiceKeys); } return $choiceKeys; diff --git a/Modules/TestQuestionPool/classes/class.ilAssQuestionPreviewGUI.php b/Modules/TestQuestionPool/classes/class.ilAssQuestionPreviewGUI.php index 6af31ee05dc3..471672fb1a47 100644 --- a/Modules/TestQuestionPool/classes/class.ilAssQuestionPreviewGUI.php +++ b/Modules/TestQuestionPool/classes/class.ilAssQuestionPreviewGUI.php @@ -1,6 +1,10 @@ @@ -82,8 +86,10 @@ class ilAssQuestionPreviewGUI * @var ilAssQuestionPreviewHintTracking */ protected $hintTracking; + + private RandomGroup $randomGroup; - public function __construct(ilCtrl $ctrl, ilTabsGUI $tabs, ilGlobalTemplateInterface $tpl, ilLanguage $lng, ilDBInterface $db, ilObjUser $user) + public function __construct(ilCtrl $ctrl, ilTabsGUI $tabs, ilGlobalTemplateInterface $tpl, ilLanguage $lng, ilDBInterface $db, ilObjUser $user, RandomGroup $randomGroup) { $this->ctrl = $ctrl; $this->tabs = $tabs; @@ -91,6 +97,7 @@ public function __construct(ilCtrl $ctrl, ilTabsGUI $tabs, ilGlobalTemplateInter $this->lng = $lng; $this->db = $db; $this->user = $user; + $this->randomGroup = $randomGroup; } public function initQuestion($questionId, $parentObjId) @@ -543,20 +550,15 @@ public function gatewayShowHintListCmd() } /** - * @return ilArrayElementShuffler + * @return Transformation */ private function getQuestionAnswerShuffler() { - require_once 'Services/Randomization/classes/class.ilArrayElementShuffler.php'; - $shuffler = new ilArrayElementShuffler(); - if (!$this->previewSession->randomizerSeedExists()) { - $this->previewSession->setRandomizerSeed($shuffler->buildRandomSeed()); + $this->previewSession->setRandomizerSeed((new RandomSeed())->createSeed()); } - - $shuffler->setSeed($this->previewSession->getRandomizerSeed()); - - return $shuffler; + + return $this->randomGroup->shuffleArray(new GivenSeed($this->previewSession->getRandomizerSeed())); } protected function populateNotesPanel(ilTemplate $tpl, $notesPanelHTML) diff --git a/Modules/TestQuestionPool/classes/class.ilObjQuestionPoolGUI.php b/Modules/TestQuestionPool/classes/class.ilObjQuestionPoolGUI.php index 5acf99b37686..1c622a944a9d 100755 --- a/Modules/TestQuestionPool/classes/class.ilObjQuestionPoolGUI.php +++ b/Modules/TestQuestionPool/classes/class.ilObjQuestionPoolGUI.php @@ -92,6 +92,7 @@ public function executeCommand() $ilDB = $DIC['ilDB']; $ilPluginAdmin = $DIC['ilPluginAdmin']; $ilias = $DIC['ilias']; + $randomGroup = $DIC->refinery()->random(); $writeAccess = $ilAccess->checkAccess("write", "", $_GET["ref_id"]); @@ -162,7 +163,7 @@ public function executeCommand() $this->ctrl->saveParameter($this, "q_id"); require_once 'Modules/TestQuestionPool/classes/class.ilAssQuestionPreviewGUI.php'; - $gui = new ilAssQuestionPreviewGUI($this->ctrl, $this->tabs_gui, $this->tpl, $this->lng, $ilDB, $ilUser); + $gui = new ilAssQuestionPreviewGUI($this->ctrl, $this->tabs_gui, $this->tpl, $this->lng, $ilDB, $ilUser, $randomGroup); $gui->initQuestion((int) $_GET['q_id'], $this->object->getId()); $gui->initPreviewSettings($this->object->getRefId()); diff --git a/Modules/TestQuestionPool/classes/export/qti12/class.assClozeTestExport.php b/Modules/TestQuestionPool/classes/export/qti12/class.assClozeTestExport.php index 3635cb9c76b6..789a06d4f4d1 100644 --- a/Modules/TestQuestionPool/classes/export/qti12/class.assClozeTestExport.php +++ b/Modules/TestQuestionPool/classes/export/qti12/class.assClozeTestExport.php @@ -1,6 +1,8 @@ randomGroup = $DIC->refinery()->random(); + } + /** * Returns a QTI xml representation of the question * @@ -144,7 +157,7 @@ public function toXML($a_include_header = true, $a_include_binary = true, $a_shu $a_xml_writer->xmlStartTag("render_choice", $attrs); // add answers - foreach ($gap->getItems(new ilDeterministicArrayElementProvider()) as $answeritem) { + foreach ($gap->getItems($this->randomGroup->dontShuffle()) as $answeritem) { $attrs = array( "ident" => $answeritem->getOrder() ); @@ -249,7 +262,7 @@ public function toXML($a_include_header = true, $a_include_binary = true, $a_shu $gap = $this->object->getGap($i); switch ($gap->getType()) { case CLOZE_SELECT: - foreach ($gap->getItems(new ilDeterministicArrayElementProvider()) as $answer) { + foreach ($gap->getItems($this->randomGroup->dontShuffle()) as $answer) { $attrs = array( "continue" => "Yes" ); @@ -280,7 +293,7 @@ public function toXML($a_include_header = true, $a_include_binary = true, $a_shu break; case CLOZE_NUMERIC: case CLOZE_TEXT: - foreach ($gap->getItems(new ilDeterministicArrayElementProvider()) as $answer) { + foreach ($gap->getItems($this->randomGroup->dontShuffle()) as $answer) { $attrs = array( "continue" => "Yes" ); diff --git a/Modules/TestQuestionPool/classes/feedback/class.ilAssClozeTestFeedback.php b/Modules/TestQuestionPool/classes/feedback/class.ilAssClozeTestFeedback.php index b1fdf84bda26..eb352cdea876 100644 --- a/Modules/TestQuestionPool/classes/feedback/class.ilAssClozeTestFeedback.php +++ b/Modules/TestQuestionPool/classes/feedback/class.ilAssClozeTestFeedback.php @@ -1,6 +1,8 @@ getItems(new ilDeterministicArrayElementProvider()) as $item) { + foreach ($gap->getItems($this->randomGroup()->dontShuffle()) as $item) { $answers[] = '"' . $item->getAnswertext() . '"'; } @@ -191,7 +193,7 @@ protected function completeFormPropsForFeedbackModeGapAnswers(ilRadioOption $fbM protected function completeFbPropsForTextGap(ilRadioOption $fbModeOpt, assClozeGap $gap, int $gapIndex) : void { - foreach ($gap->getItems(new ilDeterministicArrayElementProvider()) as $answerIndex => $item) { + foreach ($gap->getItems($this->randomGroup()->dontShuffle()) as $answerIndex => $item) { $propertyLabel = $this->questionOBJ->prepareTextareaOutput( $this->buildTextGapGivenAnswerFeedbackLabel($gapIndex, $item), true @@ -235,7 +237,7 @@ protected function completeFbPropsForTextGap(ilRadioOption $fbModeOpt, assClozeG protected function completeFbPropsForSelectGap(ilRadioOption $fbModeOpt, assClozeGap $gap, int $gapIndex) : void { - foreach ($gap->getItems(new ilDeterministicArrayElementProvider()) as $optIndex => $item) { + foreach ($gap->getItems($this->randomGroup()->dontShuffle()) as $optIndex => $item) { $propertyLabel = $this->questionOBJ->prepareTextareaOutput( $this->buildSelectGapOptionFeedbackLabel($gapIndex, $item), true @@ -397,7 +399,7 @@ protected function initFeedbackFieldsPerGapAnswers(ilPropertyFormGUI $form) : vo protected function initFbPropsForTextGap(ilPropertyFormGUI $form, assClozeGap $gap, int $gapIndex) : void { - foreach ($gap->getItems(new ilDeterministicArrayElementProvider()) as $answerIndex => $item) { + foreach ($gap->getItems($this->randomGroup()->dontShuffle()) as $answerIndex => $item) { $value = $this->getSpecificAnswerFeedbackFormValue($gapIndex, $answerIndex); $postVar = $this->buildPostVarForFbFieldPerGapAnswers($gapIndex, $answerIndex); $form->getItemByPostVar($postVar)->setValue($value); @@ -414,7 +416,7 @@ protected function initFbPropsForTextGap(ilPropertyFormGUI $form, assClozeGap $g protected function initFbPropsForSelectGap(ilPropertyFormGUI $form, assClozeGap $gap, int $gapIndex) : void { - foreach ($gap->getItems(new ilDeterministicArrayElementProvider()) as $optIndex => $item) { + foreach ($gap->getItems($this->randomGroup()->dontShuffle()) as $optIndex => $item) { $value = $this->getSpecificAnswerFeedbackFormValue($gapIndex, $optIndex); $postVar = $this->buildPostVarForFbFieldPerGapAnswers($gapIndex, $optIndex); $form->getItemByPostVar($postVar)->setValue($value); @@ -514,7 +516,7 @@ protected function saveFeedbackFieldsPerGapAnswers(ilPropertyFormGUI $form) : vo protected function saveFbPropsForTextGap(ilPropertyFormGUI $form, assClozeGap $gap, int $gapIndex) : void { - foreach ($gap->getItems(new ilDeterministicArrayElementProvider()) as $answerIndex => $item) { + foreach ($gap->getItems($this->randomGroup()->dontShuffle()) as $answerIndex => $item) { $postVar = $this->buildPostVarForFbFieldPerGapAnswers($gapIndex, $answerIndex); $value = $form->getItemByPostVar($postVar)->getValue(); $this->saveSpecificAnswerFeedbackContent( @@ -546,7 +548,7 @@ protected function saveFbPropsForTextGap(ilPropertyFormGUI $form, assClozeGap $g protected function saveFbPropsForSelectGap(ilPropertyFormGUI $form, assClozeGap $gap, int $gapIndex) : void { - foreach ($gap->getItems(new ilDeterministicArrayElementProvider()) as $optIndex => $item) { + foreach ($gap->getItems($this->randomGroup()->dontShuffle()) as $optIndex => $item) { $postVar = $this->buildPostVarForFbFieldPerGapAnswers($gapIndex, $optIndex); $value = $form->getItemByPostVar($postVar)->getValue(); $this->saveSpecificAnswerFeedbackContent( @@ -792,7 +794,7 @@ public function determineAnswerIndexForAnswerValue(assClozeGap $gap, int $answer return self::FB_TEXT_GAP_EMPTY_INDEX; } - $items = $gap->getItems(new ilDeterministicArrayElementProvider()); + $items = $gap->getItems($this->randomGroup()->dontShuffle()); foreach ($items as $answerIndex => $answer) { /* @var assAnswerCloze $answer */ @@ -821,7 +823,7 @@ public function determineAnswerIndexForAnswerValue(assClozeGap $gap, int $answer /* @var assAnswerCloze $item */ - $item = current($gap->getItems(new ilDeterministicArrayElementProvider())); + $item = current($gap->getItems($this->randomGroup()->dontShuffle())); if ($answerValue == $item->getAnswertext()) { return self::FB_NUMERIC_GAP_VALUE_HIT_INDEX; @@ -850,4 +852,11 @@ public function determineAnswerIndexForAnswerValue(assClozeGap $gap, int $answer //} } } + + private function randomGroup() : RandomGroup + { + global $DIC; + + return $DIC->refinery()->random(); + } } diff --git a/Modules/TestQuestionPool/classes/questions/LogicalAnswerCompare/ilAssLacCompositeValidator.php b/Modules/TestQuestionPool/classes/questions/LogicalAnswerCompare/ilAssLacCompositeValidator.php index 6be3b700eb0f..c14c56f9fe97 100644 --- a/Modules/TestQuestionPool/classes/questions/LogicalAnswerCompare/ilAssLacCompositeValidator.php +++ b/Modules/TestQuestionPool/classes/questions/LogicalAnswerCompare/ilAssLacCompositeValidator.php @@ -1,5 +1,7 @@ object_loader = $object_loader; + $this->randomGroup = $DIC->refinery()->random(); } public function validate(ilAssLacAbstractComposite $composite) @@ -240,6 +247,6 @@ private function checkOperatorExistForExpression($operators, $answer_expression, protected function getNonShuffler() { - return new ilDeterministicArrayElementProvider(); + return $this->randomGroup->dontShuffle(); } } diff --git a/Modules/TestQuestionPool/test/assBaseTestCase.php b/Modules/TestQuestionPool/test/assBaseTestCase.php index 7c1a2bfad56a..58d091d6f2eb 100644 --- a/Modules/TestQuestionPool/test/assBaseTestCase.php +++ b/Modules/TestQuestionPool/test/assBaseTestCase.php @@ -2,6 +2,8 @@ /* Copyright (c) 1998-2018 ILIAS open source, Extended GPL, see docs/LICENSE */ use PHPUnit\Framework\TestCase; +use ILIAS\Refinery\Factory as RefineryFactory; +use ILIAS\Refinery\Random\Group as RandomGroup; /** * Class assBaseTestCase @@ -48,6 +50,10 @@ protected function setUp() : void $DIC['rbacsystem'] = $rbacsystem_mock; $GLOBALS['rbacsystem'] = $rbacsystem_mock; + $refineryMock = $this->getMockBuilder(RefineryFactory::class)->disableOriginalConstructor()->getMock(); + $refineryMock->expects(self::any())->method('random')->willReturn($this->getMockBuilder(RandomGroup::class)->getMock()); + $DIC['refinery'] = $refineryMock; + parent::setUp(); } diff --git a/Modules/TestQuestionPool/test/assClozeGapTest.php b/Modules/TestQuestionPool/test/assClozeGapTest.php index 2ed904b33c1d..03a5baf38f2f 100644 --- a/Modules/TestQuestionPool/test/assClozeGapTest.php +++ b/Modules/TestQuestionPool/test/assClozeGapTest.php @@ -1,6 +1,8 @@ createMock('ilUtil', array('stripSlashes'), array(), '', false); $util_mock->expects($this->any())->method('stripSlashes')->will($this->returnArgument(0)); $this->setGlobalVariable('ilUtils', $util_mock); @@ -99,11 +100,14 @@ public function test_arrayShuffle_shouldNotReturnArrayUnshuffled() 'Meyer', 'Jansen', 'Heyser', 'Becker'); $instance->items = $the_unexpected; $instance->setShuffle(true); - - $actual = $instance->getItems(new ilArrayElementShuffler); + $theExpected = ['hua', 'haaa', 'some random values']; + + $transformationMock = $this->getMockBuilder(Transformation::class)->getMock(); + $transformationMock->expects(self::once())->method('transform')->with($the_unexpected)->willReturn($theExpected); + $actual = $instance->getItems($transformationMock); // Assert - $this->assertNotEquals($the_unexpected, $actual); + $this->assertEquals($theExpected, $actual); } public function test_addGetItem_shouldReturnValueUnchanged() @@ -201,7 +205,9 @@ public function test_getItems_shouldReturnItemsAdded() $instance->addItem($item2); $instance->addItem($item3); $instance->addItem($item4); - $actual = $instance->getItems(new ilArrayElementShuffler); + $transformationMock = $this->getMockBuilder(Transformation::class)->getMock(); + $transformationMock->expects(self::never())->method('transform'); + $actual = $instance->getItems($transformationMock); // Assert $this->assertEquals($expected, $actual); @@ -214,38 +220,31 @@ public function test_getItemsWithShuffle_shouldReturnItemsAddedShuffled() $instance = new assClozeGap(0); // 0 - text gap $instance->setShuffle(true); require_once './Modules/TestQuestionPool/classes/class.assAnswerCloze.php'; - $item1 = new assAnswerCloze('Bert', 1.0, 0); - $item2 = new assAnswerCloze('Fred', 1.0, 1); - $item3 = new assAnswerCloze('Karl', 1.0, 2); - $item4 = new assAnswerCloze('Esther', 1.0, 3); - $item5 = new assAnswerCloze('Herbert', 1.0, 4); - $item6 = new assAnswerCloze('Karina', 1.0, 5); - $item7 = new assAnswerCloze('Helmut', 1.0, 6); - $item8 = new assAnswerCloze('Kerstin', 1.0, 7); - $expected = array($item1, $item2, $item3, $item4, $item5, $item6, $item7, $item8); + $expected = [ + new assAnswerCloze('Bert', 1.0, 0), + new assAnswerCloze('Fred', 1.0, 1), + new assAnswerCloze('Karl', 1.0, 2), + new assAnswerCloze('Esther', 1.0, 3), + new assAnswerCloze('Herbert', 1.0, 4), + new assAnswerCloze('Karina', 1.0, 5), + new assAnswerCloze('Helmut', 1.0, 6), + new assAnswerCloze('Kerstin', 1.0, 7), + ]; + + $shuffledArray = ['some shuffled array', 'these values dont matter']; // Act - $instance->addItem($item1); - $instance->addItem($item2); - $instance->addItem($item3); - $instance->addItem($item4); - $instance->addItem($item5); - $instance->addItem($item6); - $instance->addItem($item7); - $instance->addItem($item8); - $actual = $instance->getItems(new ilArrayElementShuffler); + foreach ($expected as $item) { + $instance->addItem($item); + } + + $transformationMock = $this->getMockBuilder(Transformation::class)->getMock(); + $transformationMock->expects(self::once())->method('transform')->with($expected)->willReturn($shuffledArray); + $actual = $instance->getItems($transformationMock); // Assert - $this->assertTrue(is_array($actual)); - $this->assertTrue(in_array($item1, $actual)); - $this->assertTrue(in_array($item2, $actual)); - $this->assertTrue(in_array($item3, $actual)); - $this->assertTrue(in_array($item4, $actual)); - $this->assertTrue(in_array($item5, $actual)); - $this->assertTrue(in_array($item6, $actual)); - $this->assertTrue(in_array($item7, $actual)); - $this->assertTrue(in_array($item8, $actual)); - $this->assertNotEquals($expected, $actual); + + $this->assertEquals($shuffledArray, $actual); } public function test_getItemsRaw_shouldReturnItemsAdded() @@ -527,7 +526,7 @@ public function test_getBestSolutionOutput_shouldReturnBestSolutionOutput_CaseTe $expected = 'Esther'; // Act - $actual = $instance->getBestSolutionOutput(new ilArrayElementShuffler); + $actual = $instance->getBestSolutionOutput($this->getDummyTransformationMock()); // Assert $this->assertEquals($expected, $actual); @@ -563,7 +562,7 @@ public function test_getBestSolutionOutput_shouldReturnBestSolutionOutput_CaseTe $expected2 = 'Esther or Karl'; // Act - $actual = $instance->getBestSolutionOutput(new ilArrayElementShuffler); + $actual = $instance->getBestSolutionOutput($this->getDummyTransformationMock()); // Assert $this->assertTrue(($actual == $expected1) || ($actual == $expected2)); @@ -597,7 +596,7 @@ public function test_getBestSolutionOutput_shouldReturnBestSolutionOutput_CaseNu $expected = 100; // Act - $actual = $instance->getBestSolutionOutput(new ilArrayElementShuffler); + $actual = $instance->getBestSolutionOutput($this->getDummyTransformationMock()); // Assert $this->assertEquals($expected, $actual); @@ -631,9 +630,19 @@ public function test_getBestSolutionOutput_shouldReturnEmptyStringOnUnknownType_ $expected = ''; // Act - $actual = $instance->getBestSolutionOutput(new ilArrayElementShuffler); + $actual = $instance->getBestSolutionOutput($this->getDummyTransformationMock()); // Assert $this->assertEquals($expected, $actual); } + + private function getDummyTransformationMock() : Transformation + { + $transformationMock = $this->getMockBuilder(Transformation::class)->getMock(); + $transformationMock->expects(self::any())->method('transform')->willReturnCallback(static function (array $array) { + return $array; + }); + + return $transformationMock; + } } diff --git a/Modules/TestQuestionPool/test/assClozeSelectGapTest.php b/Modules/TestQuestionPool/test/assClozeSelectGapTest.php index 71e7ba588890..795648511344 100644 --- a/Modules/TestQuestionPool/test/assClozeSelectGapTest.php +++ b/Modules/TestQuestionPool/test/assClozeSelectGapTest.php @@ -3,6 +3,8 @@ require_once __DIR__ . "/assBaseTestCase.php"; +use ILIAS\Refinery\Transformation; + /** * Unit tests * @@ -54,10 +56,12 @@ public function test_arrayShuffle_shouldShuffleArray() // Arrange require_once './Modules/TestQuestionPool/classes/class.assClozeSelectGap.php'; $instance = new assClozeSelectGap(1); // 1 - select gap - $expected = array(1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20); + $expected = ['shfksdfs', 'sfsdf', 'sdfsdfdf']; - $actual = $instance->getItems(new ilArrayElementShuffler()); - $this->assertNotEquals($expected, $actual); + $transformationMock = $this->getMockBuilder(Transformation::class)->getMock(); + $transformationMock->expects(self::once())->method('transform')->willReturn($expected); + $actual = $instance->getItems($transformationMock); + $this->assertEquals($expected, $actual); } public function test_getItemswithShuffle_shouldReturnShuffledItems() @@ -89,9 +93,9 @@ public function test_getItemswithShuffle_shouldReturnShuffledItems() $sequence = [$item1, $item3, $item2, $item4, $item5, $item6, $item7, $item8]; $expectedSequence = array_reverse($sequence); - $randomElmProvider = $this->getMockBuilder(ilRandomArrayElementProvider::class)->getMock(); + $randomElmProvider = $this->getMockBuilder(Transformation::class)->getMock(); $randomElmProvider->expects($this->once()) - ->method('shuffle') + ->method('transform') ->with($sequence) ->willReturn($expectedSequence); @@ -118,7 +122,11 @@ public function test_getItemswithoutShuffle_shouldReturnItemsInOrder() $instance->setType(false); $expected = array($item1, $item2, $item3, $item4); - $actual = $instance->getItems(new ilDeterministicArrayElementProvider()); + $transformationMock = $this->getMockBuilder(Transformation::class)->getMock(); + $transformationMock->expects(self::once())->method('transform')->willReturnCallback(function ($value) { + return $value; + }); + $actual = $instance->getItems($transformationMock); $this->assertEquals($expected, $actual); } diff --git a/src/Refinery/Factory.php b/src/Refinery/Factory.php index 4e6c57181294..f990e3cb07bc 100644 --- a/src/Refinery/Factory.php +++ b/src/Refinery/Factory.php @@ -7,6 +7,7 @@ use ILIAS\Refinery\In; use ILIAS\Refinery\To; use ILIAS\Refinery\ByTrying; +use ILIAS\Refinery\Random\Group as RandomGroup; /** * @author Niels Theen @@ -146,4 +147,9 @@ public function byTrying(array $transformations) : ByTrying { return new ByTrying($transformations, $this->dataFactory, $this->language); } + + public function random() : RandomGroup + { + return new RandomGroup(); + } } diff --git a/src/Refinery/Random/Group.php b/src/Refinery/Random/Group.php new file mode 100644 index 000000000000..4d20f47db377 --- /dev/null +++ b/src/Refinery/Random/Group.php @@ -0,0 +1,36 @@ + + */ +namespace ILIAS\Refinery\Random; + +use ILIAS\Refinery\Transformation; +use ILIAS\Refinery\Random\Transformation\ShuffleTransformation; +use ILIAS\Refinery\Random\Seed\Seed; +use ILIAS\Refinery\Random\Transformation\IdentityTransformation; + +class Group +{ + /** + * Get a transformation which will shuffle a given array. + * Only arrays can be supplied to the transformation. + * + * The transformation will be shuffled with the given $seed. + * + * !! BEWARE OF THE SIDE EFFECT. This Transformation is not SIde Effect free !! + */ + public function shuffleArray(Seed $seed) : Transformation + { + return new ShuffleTransformation($seed); + } + + /** + * Get a transformation which will return the given value as is. + * Everything can be supplied to the transformation. + */ + public function dontShuffle() : Transformation + { + return new IdentityTransformation(); + } +} diff --git a/src/Refinery/Random/Seed/GivenSeed.php b/src/Refinery/Random/Seed/GivenSeed.php new file mode 100644 index 000000000000..b541a6e162de --- /dev/null +++ b/src/Refinery/Random/Seed/GivenSeed.php @@ -0,0 +1,21 @@ + + */ +namespace ILIAS\Refinery\Random\Seed; + +class GivenSeed implements Seed +{ + private int $seed; + + public function __construct(int $seed) + { + $this->seed = $seed; + } + + public function seedRandomGenerator() : void + { + mt_srand($this->seed); + } +} diff --git a/src/Refinery/Random/Seed/RandomSeed.php b/src/Refinery/Random/Seed/RandomSeed.php new file mode 100644 index 000000000000..d45a81dbdeca --- /dev/null +++ b/src/Refinery/Random/Seed/RandomSeed.php @@ -0,0 +1,22 @@ + + */ +namespace ILIAS\Refinery\Random\Seed; + +class RandomSeed extends GivenSeed +{ + public function __construct() + { + parent::__construct($this->createSeed()); + } + + public function createSeed() : int + { + $array = explode(' ', microtime()); + $seed = $array[1] + ($array[0] * 100000); + + return $seed; + } +} diff --git a/src/Refinery/Random/Seed/Seed.php b/src/Refinery/Random/Seed/Seed.php new file mode 100644 index 000000000000..d36c17a23dd6 --- /dev/null +++ b/src/Refinery/Random/Seed/Seed.php @@ -0,0 +1,11 @@ + + */ +namespace ILIAS\Refinery\Random\Seed; + +interface Seed +{ + public function seedRandomGenerator() : void; +} diff --git a/src/Refinery/Random/Transformation/IdentityTransformation.php b/src/Refinery/Random/Transformation/IdentityTransformation.php new file mode 100644 index 000000000000..a4405c3aa921 --- /dev/null +++ b/src/Refinery/Random/Transformation/IdentityTransformation.php @@ -0,0 +1,59 @@ + + */ +namespace ILIAS\Refinery\Random\Transformation; + +use ILIAS\Refinery\Transformation; +use ILIAS\Data\Result; +use ILIAS\Data\Result\Ok; +use ILIAS\Refinery\DeriveInvokeFromTransform; + +class IdentityTransformation implements Transformation +{ + use DeriveInvokeFromTransform; + + /** + * @throws \InvalidArgumentException + */ + public function transform($from) + { + return $this->transformResult($from)->value(); + } + + /** + * @return Result + */ + public function applyTo(Result $result) : Result + { + return $result->then(function ($from) { + return $this->transformResult($from); + }); + } + + /** + * @return Result + */ + protected function validate($from) : Result + { + return new Ok($from); + } + + protected function saveTransform($value) + { + return $value; + } + + /** + * Same as transform but returns a Result instead of an exception. + * + * @return Result + */ + private function transformResult($from) : Result + { + return $this->validate($from)->map(function ($from) { + return $this->saveTransform($from); + }); + } +} diff --git a/src/Refinery/Random/Transformation/ShuffleTransformation.php b/src/Refinery/Random/Transformation/ShuffleTransformation.php new file mode 100644 index 000000000000..69db4d7f30d1 --- /dev/null +++ b/src/Refinery/Random/Transformation/ShuffleTransformation.php @@ -0,0 +1,46 @@ + + */ +namespace ILIAS\Refinery\Random\Transformation; + +use ILIAS\Data\Result; +use ILIAS\Data\Result\Ok; +use ILIAS\Data\Result\Error; +use ILIAS\Refinery\Random\Seed\Seed; + +/** + * !! BEWARE OF THE SIDE EFFECT. This Transformation is not Side Effect free !! + * Shuffling and seeding of the random generator is not side effect free and done when transforming a value. + * This class is an expection to the rule of the normally side effect free transformations. + */ +class ShuffleTransformation extends IdentityTransformation +{ + private Seed $seed; + + public function __construct(Seed $seed) + { + $this->seed = $seed; + } + + /** + * @return Result + */ + protected function validate($from) : Result + { + return is_array($from) ? new Ok($from) : new Error('I need an array'); + } + + /** + * @param array $array + * @return array + */ + protected function saveTransform($array) + { + $this->seed->seedRandomGenerator(); + \shuffle($array); + + return $array; + } +} diff --git a/tests/Refinery/Random/GroupTest.php b/tests/Refinery/Random/GroupTest.php new file mode 100644 index 000000000000..f3df7cd170dc --- /dev/null +++ b/tests/Refinery/Random/GroupTest.php @@ -0,0 +1,40 @@ + + */ +namespace ILIAS\Tests\Refinery\Random; + +use ILIAS\Refinery\Random\Group; +use ILIAS\Refinery\Transformation; +use PHPUnit\Framework\TestCase; +use ILIAS\Refinery\Random\Seed\Seed; +use ILIAS\Refinery\Random\Transformation\ShuffleTransformation; +use ILIAS\Refinery\Random\Transformation\IdentityTransformation; + +class GroupTest extends TestCase +{ + /** + * @var Group + */ + private $group; + + public function setUp() : void + { + $this->group = new Group(); + } + + public function testShuffle() : void + { + $mock = $this->getMockBuilder(Seed::class)->getMock(); + $mock->expects(self::never())->method('seedRandomGenerator'); + $instance = $this->group->shuffleArray($mock); + $this->assertInstanceOf(ShuffleTransformation::class, $instance); + } + + public function testDontShuffle() : void + { + $instance = $this->group->dontShuffle(); + $this->assertInstanceOf(IdentityTransformation::class, $instance); + } +} diff --git a/tests/Refinery/Random/Transformation/IdentityTransformationTest.php b/tests/Refinery/Random/Transformation/IdentityTransformationTest.php new file mode 100644 index 000000000000..56d5757ec101 --- /dev/null +++ b/tests/Refinery/Random/Transformation/IdentityTransformationTest.php @@ -0,0 +1,39 @@ + + */ +namespace ILIAS\Tests\Refinery\Random\Transformation; + +use ILIAS\Data\NotOKException; +use ILIAS\Data\Result\Ok; +use ILIAS\Data\Result\Error; +use ILIAS\Refinery\Random\Transformation\IdentityTransformation; +use PHPUnit\Framework\TestCase; + +class IdentityTransformationTest extends TestCase +{ + public function testTransform() : void + { + $value = 'hejaaa'; + + $actual = (new IdentityTransformation())->transform($value); + + $this->assertEquals($value, $actual); + } + + public function testApplyToOk() : void + { + $value = ['im in an array']; + $result = (new IdentityTransformation())->applyTo(new Ok($value)); + $this->assertInstanceOf(Ok::class, $result); + $this->assertEquals($value, $result->value()); + } + + public function testApplyToError() : void + { + $error = new Error('some error'); + $result = (new IdentityTransformation())->applyTo($error); + $this->assertEquals($error, $result); + } +} diff --git a/tests/Refinery/Random/Transformation/ShuffleTransformationTest.php b/tests/Refinery/Random/Transformation/ShuffleTransformationTest.php new file mode 100644 index 000000000000..44a0e7055561 --- /dev/null +++ b/tests/Refinery/Random/Transformation/ShuffleTransformationTest.php @@ -0,0 +1,35 @@ + + */ +namespace ILIAS\Tests\Refinery\Random\Transformation; + +use ILIAS\Refinery\Random\Transformation\ShuffleTransformation; +use ILIAS\Refinery\Random\Seed\Seed; +use ILIAS\Data\NotOKException; +use ILIAS\Data\Result\Ok; +use ILIAS\Data\Result\Error; +use PHPUnit\Framework\TestCase; + +class ShuffleTransformationTest extends TestCase +{ + public function testTransformResultSuccess() : void + { + $value = ['nrrrrg']; + $seedMock = $this->getMockBuilder(Seed::class)->getMock(); + $seedMock->expects(self::once())->method('seedRandomGenerator'); + + $result = (new ShuffleTransformation($seedMock))->transform($value); + $this->assertEquals($value, $result); + } + + public function testTransformResultFailure() : void + { + $this->expectException(NotOKException::class); + $seedMock = $this->getMockBuilder(Seed::class)->getMock(); + $seedMock->expects(self::never())->method('seedRandomGenerator'); + + $result = (new ShuffleTransformation($seedMock))->transform('im no array'); + } +} From 33db03f3143ded4ce28bddd3eea81e06d73e0e71 Mon Sep 17 00:00:00 2001 From: Lukas Scharmer Date: Fri, 12 Nov 2021 12:05:46 +0100 Subject: [PATCH 5/9] Fix wrong case in comment --- src/Refinery/Random/Group.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Refinery/Random/Group.php b/src/Refinery/Random/Group.php index 4d20f47db377..f112e3a26db0 100644 --- a/src/Refinery/Random/Group.php +++ b/src/Refinery/Random/Group.php @@ -18,7 +18,7 @@ class Group * * The transformation will be shuffled with the given $seed. * - * !! BEWARE OF THE SIDE EFFECT. This Transformation is not SIde Effect free !! + * !! BEWARE OF THE SIDE EFFECT. This Transformation is not Side Effect free !! */ public function shuffleArray(Seed $seed) : Transformation { From 29507f787889f99a149d3de27805f97d017a5902 Mon Sep 17 00:00:00 2001 From: Lukas Scharmer Date: Tue, 23 Nov 2021 17:55:08 +0100 Subject: [PATCH 6/9] Test randomness. Move IdentityTransformation --- .../IdentityTransformation.php | 3 +-- src/Refinery/Random/Group.php | 3 ++- .../Transformation/ShuffleTransformation.php | 5 ++++- .../IdentityTransformationTest.php | 2 +- tests/Refinery/Random/GroupTest.php | 2 +- .../ShuffleTransformationTest.php | 20 +++++++++++++++---- 6 files changed, 25 insertions(+), 10 deletions(-) rename src/Refinery/{Random/Transformation => }/IdentityTransformation.php (93%) rename tests/Refinery/{Random/Transformation => }/IdentityTransformationTest.php (93%) diff --git a/src/Refinery/Random/Transformation/IdentityTransformation.php b/src/Refinery/IdentityTransformation.php similarity index 93% rename from src/Refinery/Random/Transformation/IdentityTransformation.php rename to src/Refinery/IdentityTransformation.php index a4405c3aa921..3a8737107429 100644 --- a/src/Refinery/Random/Transformation/IdentityTransformation.php +++ b/src/Refinery/IdentityTransformation.php @@ -3,9 +3,8 @@ /** * @author Lukas Scharmer */ -namespace ILIAS\Refinery\Random\Transformation; +namespace ILIAS\Refinery; -use ILIAS\Refinery\Transformation; use ILIAS\Data\Result; use ILIAS\Data\Result\Ok; use ILIAS\Refinery\DeriveInvokeFromTransform; diff --git a/src/Refinery/Random/Group.php b/src/Refinery/Random/Group.php index f112e3a26db0..11f74a1caf21 100644 --- a/src/Refinery/Random/Group.php +++ b/src/Refinery/Random/Group.php @@ -8,7 +8,7 @@ use ILIAS\Refinery\Transformation; use ILIAS\Refinery\Random\Transformation\ShuffleTransformation; use ILIAS\Refinery\Random\Seed\Seed; -use ILIAS\Refinery\Random\Transformation\IdentityTransformation; +use ILIAS\Refinery\IdentityTransformation; class Group { @@ -19,6 +19,7 @@ class Group * The transformation will be shuffled with the given $seed. * * !! BEWARE OF THE SIDE EFFECT. This Transformation is not Side Effect free !! + * The internal state of the PRNG will be advanced on every usage. */ public function shuffleArray(Seed $seed) : Transformation { diff --git a/src/Refinery/Random/Transformation/ShuffleTransformation.php b/src/Refinery/Random/Transformation/ShuffleTransformation.php index 69db4d7f30d1..74f1530a345a 100644 --- a/src/Refinery/Random/Transformation/ShuffleTransformation.php +++ b/src/Refinery/Random/Transformation/ShuffleTransformation.php @@ -9,11 +9,14 @@ use ILIAS\Data\Result\Ok; use ILIAS\Data\Result\Error; use ILIAS\Refinery\Random\Seed\Seed; +use ILIAS\Refinery\IdentityTransformation; /** * !! BEWARE OF THE SIDE EFFECT. This Transformation is not Side Effect free !! * Shuffling and seeding of the random generator is not side effect free and done when transforming a value. - * This class is an expection to the rule of the normally side effect free transformations. + * This class is an exception to the rule of the normally side effect free transformations. + * @see https://github.com/ILIAS-eLearning/ILIAS/pull/476/files#diff-a6b45507ea92787f1788b74d31ba62162dd9b09f00e7a9dea804be783c09afecR8 + * and https://github.com/ILIAS-eLearning/ILIAS/pull/1707/files#diff-cbb4c50b8e633da5c3461f5b4bdf0f29c11199213ae2c60788af66b885b6bb5e */ class ShuffleTransformation extends IdentityTransformation { diff --git a/tests/Refinery/Random/Transformation/IdentityTransformationTest.php b/tests/Refinery/IdentityTransformationTest.php similarity index 93% rename from tests/Refinery/Random/Transformation/IdentityTransformationTest.php rename to tests/Refinery/IdentityTransformationTest.php index 56d5757ec101..d84573fdbf75 100644 --- a/tests/Refinery/Random/Transformation/IdentityTransformationTest.php +++ b/tests/Refinery/IdentityTransformationTest.php @@ -8,7 +8,7 @@ use ILIAS\Data\NotOKException; use ILIAS\Data\Result\Ok; use ILIAS\Data\Result\Error; -use ILIAS\Refinery\Random\Transformation\IdentityTransformation; +use ILIAS\Refinery\IdentityTransformation; use PHPUnit\Framework\TestCase; class IdentityTransformationTest extends TestCase diff --git a/tests/Refinery/Random/GroupTest.php b/tests/Refinery/Random/GroupTest.php index f3df7cd170dc..6c06a1b18781 100644 --- a/tests/Refinery/Random/GroupTest.php +++ b/tests/Refinery/Random/GroupTest.php @@ -10,7 +10,7 @@ use PHPUnit\Framework\TestCase; use ILIAS\Refinery\Random\Seed\Seed; use ILIAS\Refinery\Random\Transformation\ShuffleTransformation; -use ILIAS\Refinery\Random\Transformation\IdentityTransformation; +use ILIAS\Refinery\IdentityTransformation; class GroupTest extends TestCase { diff --git a/tests/Refinery/Random/Transformation/ShuffleTransformationTest.php b/tests/Refinery/Random/Transformation/ShuffleTransformationTest.php index 44a0e7055561..28455f8c4ad0 100644 --- a/tests/Refinery/Random/Transformation/ShuffleTransformationTest.php +++ b/tests/Refinery/Random/Transformation/ShuffleTransformationTest.php @@ -3,7 +3,7 @@ /** * @author Lukas Scharmer */ -namespace ILIAS\Tests\Refinery\Random\Transformation; +namespace ILIAS\Tests\Refinery; use ILIAS\Refinery\Random\Transformation\ShuffleTransformation; use ILIAS\Refinery\Random\Seed\Seed; @@ -16,12 +16,16 @@ class ShuffleTransformationTest extends TestCase { public function testTransformResultSuccess() : void { - $value = ['nrrrrg']; + $seed = 0; + $value = ['Donec', 'at', 'pede', 'Phasellus', 'purus', 'Nulla', 'facilisis', 'risus', 'a', 'rhoncus', 'fermentum', 'tellus', 'tellus', 'lacinia', 'purus', 'et', 'dictum', 'nunc', 'justo', 'sit', 'amet', 'elit']; + $expected = $this->shuffleWithSeed($value, $seed); $seedMock = $this->getMockBuilder(Seed::class)->getMock(); - $seedMock->expects(self::once())->method('seedRandomGenerator'); + $seedMock->expects(self::once())->method('seedRandomGenerator')->willReturnCallback(static function () use ($seed) : void { + \mt_srand($seed); + }); $result = (new ShuffleTransformation($seedMock))->transform($value); - $this->assertEquals($value, $result); + $this->assertEquals($expected, $result); } public function testTransformResultFailure() : void @@ -32,4 +36,12 @@ public function testTransformResultFailure() : void $result = (new ShuffleTransformation($seedMock))->transform('im no array'); } + + private function shuffleWithSeed(array $array, int $seed) : array + { + \mt_srand($seed); + \shuffle($array); + + return $array; + } } From e5899beea2efc6f4ef9d7427da5edaaa85a3dae3 Mon Sep 17 00:00:00 2001 From: Lukas Scharmer Date: Wed, 1 Dec 2021 14:59:06 +0100 Subject: [PATCH 7/9] Change DeriveApplyToFromTransform trait to use ->then --- lang/ilias_de.lang | 2 + lang/ilias_en.lang | 2 + src/Refinery/DeriveApplyToFromTransform.php | 16 ++++---- src/Refinery/IdentityTransformation.php | 39 ++----------------- .../Transformation/ShuffleTransformation.php | 24 ++++++------ .../ShuffleTransformationTest.php | 3 +- 6 files changed, 31 insertions(+), 55 deletions(-) diff --git a/lang/ilias_de.lang b/lang/ilias_de.lang index 0115ab48b1e5..3000b81fedaa 100755 --- a/lang/ilias_de.lang +++ b/lang/ilias_de.lang @@ -16649,6 +16649,8 @@ prg#:#will_not_modify_validity_on_non_successful_progress#:#Gültigkeit kann nur prg#:#status_unchanged#:#Status unverändert common#:#file_system_clean_temp_dir_cron#:#Temporäres Verzeichnis aufräumen common#:#file_system_clean_temp_dir_cron_info#:#Dieser Job räumt das ILIAS temp-Verzeichnis auf und löscht von Dateien, welche älter als 10 Tage sind. Damit wird einer Anhäufung ungenutzter Dateien und damit einem erhöhten Speicherplatzverbrauch des temp-Verzeichnisses entgegengewirkt. +common#:#actions_for#:#Aktionen für %s violation#:#not_a_string#:#Wert ist keine Zeichenkette common#:#actions_for#:#Aktionen für %s validation#:#not_greater_than_or_equal#:#Der Wert ist nicht größer oder gleich '%s'. +validation#:#not_an_array#:#Wert ist kein Feld diff --git a/lang/ilias_en.lang b/lang/ilias_en.lang index 3c34a67a4aca..979217f66055 100755 --- a/lang/ilias_en.lang +++ b/lang/ilias_en.lang @@ -16651,6 +16651,8 @@ prg#:#will_not_modify_validity_on_non_successful_progress#:#Can change only when prg#:#status_unchanged#:#Status unchanged common#:#file_system_clean_temp_dir_cron#:#Clean temp directory common#:#file_system_clean_temp_dir_cron_info#:#This job cleans the ILIAS temp-directory of files, which are older than 10 days. This counteracts the accumulation of unused files and therefore prevents an increased use of disk space by the temp directory. +common#:#actions_for#:#Actions for %s violation#:#not_a_string#:#Given value is not a String common#:#actions_for#:#Actions for %s validation#:#not_greater_than_or_equal#:#The value is not greater than or equal '%s'. +validation#:#not_an_array#:#Given value is not an array diff --git a/src/Refinery/DeriveApplyToFromTransform.php b/src/Refinery/DeriveApplyToFromTransform.php index a7f315316d48..aaf7b22da332 100644 --- a/src/Refinery/DeriveApplyToFromTransform.php +++ b/src/Refinery/DeriveApplyToFromTransform.php @@ -5,6 +5,8 @@ namespace ILIAS\Refinery; use ILIAS\Data\Result; +use ILIAS\Data\Result\Ok; +use ILIAS\Data\Result\Error; /** * @author Niels Theen @@ -20,12 +22,12 @@ abstract public function transform($from); public function applyTo(Result $result) : Result { - try { - $value = $this->transform($result->value()); - } catch (\Exception $exception) { - return new Result\Error($exception); - } - - return new Result\Ok($value); + return $result->then(function ($value) use ($result) : Result { + try { + return new Ok($this->transform($result->value())); + } catch (\Exception $exception) { + return new Error($exception); + } + }); } } diff --git a/src/Refinery/IdentityTransformation.php b/src/Refinery/IdentityTransformation.php index 3a8737107429..1917810fe4fd 100644 --- a/src/Refinery/IdentityTransformation.php +++ b/src/Refinery/IdentityTransformation.php @@ -8,51 +8,18 @@ use ILIAS\Data\Result; use ILIAS\Data\Result\Ok; use ILIAS\Refinery\DeriveInvokeFromTransform; +use ILIAS\Refinery\DeriveApplyToFromTransform; class IdentityTransformation implements Transformation { use DeriveInvokeFromTransform; + use DeriveApplyToFromTransform; /** * @throws \InvalidArgumentException */ public function transform($from) { - return $this->transformResult($from)->value(); - } - - /** - * @return Result - */ - public function applyTo(Result $result) : Result - { - return $result->then(function ($from) { - return $this->transformResult($from); - }); - } - - /** - * @return Result - */ - protected function validate($from) : Result - { - return new Ok($from); - } - - protected function saveTransform($value) - { - return $value; - } - - /** - * Same as transform but returns a Result instead of an exception. - * - * @return Result - */ - private function transformResult($from) : Result - { - return $this->validate($from)->map(function ($from) { - return $this->saveTransform($from); - }); + return $from; } } diff --git a/src/Refinery/Random/Transformation/ShuffleTransformation.php b/src/Refinery/Random/Transformation/ShuffleTransformation.php index 74f1530a345a..7cc7e272a46f 100644 --- a/src/Refinery/Random/Transformation/ShuffleTransformation.php +++ b/src/Refinery/Random/Transformation/ShuffleTransformation.php @@ -10,6 +10,10 @@ use ILIAS\Data\Result\Error; use ILIAS\Refinery\Random\Seed\Seed; use ILIAS\Refinery\IdentityTransformation; +use ILIAS\Refinery\Transformation; +use ILIAS\Refinery\ConstraintViolationException; +use ILIAS\Refinery\DeriveInvokeFromTransform; +use ILIAS\Refinery\DeriveApplyToFromTransform; /** * !! BEWARE OF THE SIDE EFFECT. This Transformation is not Side Effect free !! @@ -18,8 +22,11 @@ * @see https://github.com/ILIAS-eLearning/ILIAS/pull/476/files#diff-a6b45507ea92787f1788b74d31ba62162dd9b09f00e7a9dea804be783c09afecR8 * and https://github.com/ILIAS-eLearning/ILIAS/pull/1707/files#diff-cbb4c50b8e633da5c3461f5b4bdf0f29c11199213ae2c60788af66b885b6bb5e */ -class ShuffleTransformation extends IdentityTransformation +class ShuffleTransformation implements Transformation { + use DeriveInvokeFromTransform; + use DeriveApplyToFromTransform; + private Seed $seed; public function __construct(Seed $seed) @@ -28,19 +35,14 @@ public function __construct(Seed $seed) } /** - * @return Result - */ - protected function validate($from) : Result - { - return is_array($from) ? new Ok($from) : new Error('I need an array'); - } - - /** - * @param array $array + * @throws ConstraintViolationException * @return array */ - protected function saveTransform($array) + public function transform($array) { + if (!is_array($array)) { + throw new ConstraintViolationException('not an array', 'not_an_array'); + } $this->seed->seedRandomGenerator(); \shuffle($array); diff --git a/tests/Refinery/Random/Transformation/ShuffleTransformationTest.php b/tests/Refinery/Random/Transformation/ShuffleTransformationTest.php index 28455f8c4ad0..eb24bcd728be 100644 --- a/tests/Refinery/Random/Transformation/ShuffleTransformationTest.php +++ b/tests/Refinery/Random/Transformation/ShuffleTransformationTest.php @@ -11,6 +11,7 @@ use ILIAS\Data\Result\Ok; use ILIAS\Data\Result\Error; use PHPUnit\Framework\TestCase; +use ILIAS\Refinery\ConstraintViolationException; class ShuffleTransformationTest extends TestCase { @@ -30,7 +31,7 @@ public function testTransformResultSuccess() : void public function testTransformResultFailure() : void { - $this->expectException(NotOKException::class); + $this->expectException(ConstraintViolationException::class); $seedMock = $this->getMockBuilder(Seed::class)->getMock(); $seedMock->expects(self::never())->method('seedRandomGenerator'); From cebc1a6ea25d85a8ddba2550643b5e9d97a67194 Mon Sep 17 00:00:00 2001 From: Lukas Scharmer Date: Fri, 3 Dec 2021 11:47:35 +0100 Subject: [PATCH 8/9] Fix DeriveApplyToFromTransform --- src/Refinery/DeriveApplyToFromTransform.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Refinery/DeriveApplyToFromTransform.php b/src/Refinery/DeriveApplyToFromTransform.php index aaf7b22da332..d9a97a74e154 100644 --- a/src/Refinery/DeriveApplyToFromTransform.php +++ b/src/Refinery/DeriveApplyToFromTransform.php @@ -24,7 +24,7 @@ public function applyTo(Result $result) : Result { return $result->then(function ($value) use ($result) : Result { try { - return new Ok($this->transform($result->value())); + return new Ok($this->transform($value)); } catch (\Exception $exception) { return new Error($exception); } From 3327f42e5cdb03b0341242415c35931b9897bafd Mon Sep 17 00:00:00 2001 From: Richard Klees Date: Mon, 6 Dec 2021 17:02:11 +0100 Subject: [PATCH 9/9] Refinery: removed superfluous `use` in DeriveApplyToFromTransform --- src/Refinery/DeriveApplyToFromTransform.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Refinery/DeriveApplyToFromTransform.php b/src/Refinery/DeriveApplyToFromTransform.php index d9a97a74e154..33f32115abb8 100644 --- a/src/Refinery/DeriveApplyToFromTransform.php +++ b/src/Refinery/DeriveApplyToFromTransform.php @@ -22,7 +22,7 @@ abstract public function transform($from); public function applyTo(Result $result) : Result { - return $result->then(function ($value) use ($result) : Result { + return $result->then(function ($value) : Result { try { return new Ok($this->transform($value)); } catch (\Exception $exception) {