Dev: Fix PHP-DI CActiveRecord factory (#3731)

* Dev: Update DI config

* Dev: Remove debug code

application/libraries/DI.php

@@ -2,6 +2,7 @@
 
 namespace LimeSurvey;
 
+use DI\ContainerBuilder;
 use CActiveRecord;
 use LSYii_Application;
 use LimeSurvey\PluginManager\PluginManager;
@@ -41,34 +42,22 @@ public static function getContainer()
      */
     public static function makeContainer()
     {
-        $container = new \DI\Container;
-
-        // Type hinting on a Yii model / active-record class should return its
-        // - static instance e.g Survey::model(). It is not possible to type hint
-        // - on this, so instead we configure the container to return the correct
-        // - object based on the call time class name, whenever we type hint on
-        // - CActiveRecord or anything that extends CActiveRecord
-        $container->set(CActiveRecord::class, function (CActiveRecord $entry) {
-            $class = $entry->getName();
-            return $class::model();
-        });
-
-        $container->set(LSYii_Application::class, function () {
-            return App();
-        });
-
-        $container->set(PluginManager::class, function () {
-            return App()->getPluginManager();
-        });
-
-        $container->set(CHttpSession::class, function () {
-            return App()->session;
-        });
-
-        $container->set(CDbConnection::class, function () {
-            return App()->db;
-        });
-
-        return $container;
+        $builder = new ContainerBuilder();
+        $builder->addDefinitions([
+            LSYii_Application::class => function () {
+                return App();
+            },
+            PluginManager::class => function () {
+                return App()->getPluginManager();
+            },
+            CHttpSession::class => function () {
+                return App()->session;
+            },
+            CDbConnection::class => function () {
+                return App()->db;
+            }
+        ]);
+
+        return $builder->build();
     }
 }

application/models/services/QuestionAggregateService/AnswersService.php

@@ -25,6 +25,17 @@ class AnswersService
 {
     use ValidateTrait;
 
+    private Answer $modelAnswer;
+    private AnswerL10n $modelAnswerL10n;
+
+    public function __construct(
+        Answer $modelAnswer,
+        AnswerL10n $modelAnswerL10n
+    ) {
+        $this->modelAnswer = $modelAnswer;
+        $this->modelAnswerL10n = $modelAnswerL10n;
+    }
+
     /**
      * Based on QuestionAdministrationController::actionSaveQuestionData()
      *
@@ -105,8 +116,7 @@ private function storeAnswerOption(
             // We use the container to create a model instance
             // allowing us to mock the model instance via
             // container configuration in unit tests
-            $answer = DI::getContainer()
-                ->make(Answer::class)->findByPk($answerId);
+            $answer = $this->modelAnswer->findByPk($answerId);
             if (!$answer) {
                 $answer = DI::getContainer()
                     ->make(Answer::class);
@@ -157,13 +167,12 @@ private function storeAnswerOption(
      */
     private function storeAnswerL10n(Answer $answer, $language, $text)
     {
-        $l10n = DI::getContainer()
-            ->make(AnswerL10n::class)->findByAttributes(
-                [
-                    'aid' => $answer->aid,
-                    'language' => $language
-                ]
-            );
+        $l10n = $this->modelAnswerL10n->findByAttributes(
+            [
+                'aid' => $answer->aid,
+                'language' => $language
+            ]
+        );
         if (!$l10n) {
             $l10n = DI::getContainer()->make(AnswerL10n::class);
         }
@@ -172,7 +181,6 @@ private function storeAnswerL10n(Answer $answer, $language, $text)
             'language' => $language,
             'answer' => $text
         ]);
-
         if (!$l10n->save()) {
             throw new PersistErrorException(
                 gT('Could not save answer option')

tests/unit/services/QuestionAggregateService/AnswersServiceTest.php

@@ -33,9 +33,6 @@ public function testSaveThrowsExceptionBadRequestOnMissingCode()
             BadRequestException::class
         );
 
-        $modelQuestionAttribute = Mockery::mock(QuestionAttribute::class)
-            ->makePartial();
-
         $question = Mockery::mock(Question::class)
             ->makePartial();
         $question->shouldReceive('settAttributes')
@@ -50,9 +47,8 @@ public function testSaveThrowsExceptionBadRequestOnMissingCode()
         );
         $question->shouldReceive('deleteAllAnswers')->once();
 
-
-        $answersService = new AnswersService(
-            $modelQuestionAttribute
+        $answersService = DI::getContainer()->get(
+            AnswersService::class
         );
 
         $answersService->save($question, [
@@ -71,9 +67,6 @@ public function testSaveThrowsExceptionPersistErrorOnQuestionSaveAnswerFailure()
             PersistErrorException::class
         );
 
-        $modelQuestionAttribute = Mockery::mock(QuestionAttribute::class)
-            ->makePartial();
-
         $question = Mockery::mock(Question::class)
             ->makePartial();
         $question->shouldReceive('settAttributes')
@@ -94,15 +87,8 @@ public function testSaveThrowsExceptionPersistErrorOnQuestionSaveAnswerFailure()
         $answer->shouldReceive('save')
             ->andReturn(false);
 
-        DI::getContainer()->set(
-            Answer::class,
-            function () use ($answer) {
-                return $answer;
-            }
-        );
-
-        $answersService = new AnswersService(
-            $modelQuestionAttribute
+        $answersService = DI::getContainer()->get(
+            AnswersService::class
         );
 
         $answersService->save($question, [
@@ -121,9 +107,6 @@ public function testSaveThrowsExceptionPersistErrorOnQuestionSaveAnswerL10nFailu
             PersistErrorException::class
         );
 
-        $modelQuestionAttribute = Mockery::mock(QuestionAttribute::class)
-            ->makePartial();
-
         $question = Mockery::mock(Question::class)
             ->makePartial();
         $question->shouldReceive('settAttributes')
@@ -143,27 +126,15 @@ public function testSaveThrowsExceptionPersistErrorOnQuestionSaveAnswerL10nFailu
         $answer->shouldReceive('settAttributes');
         $answer->shouldReceive('save')
             ->andReturn(true);
-        DI::getContainer()->set(
-            Answer::class,
-            function () use ($answer) {
-                return $answer;
-            }
-        );
 
         $l10n = Mockery::mock(AnswerL10n::class)
             ->makePartial();
         $l10n->shouldReceive('settAttributes');
         $l10n->shouldReceive('save')
             ->andReturn(false);
-        DI::getContainer()->set(
-            AnswerL10n::class,
-            function () use ($l10n) {
-                return $l10n;
-            }
-        );
 
-        $answersService = new AnswersService(
-            $modelQuestionAttribute
+        $answersService = DI::getContainer()->get(
+            AnswersService::class
         );
 
         $answersService->save($question, [