fix: Don't fetch the current item for a null test session

oat-sa · May 16, 2024 · ab125c2 · ab125c2
1 parent 1508e93
commit ab125c2
Show file tree

Hide file tree

Showing 2 changed files with 156 additions and 19 deletions.
diff --git a/model/Service/ConcurringSessionService.php b/model/Service/ConcurringSessionService.php
@@ -33,7 +33,7 @@
 use oat\taoQtiTest\models\container\QtiTestDeliveryContainer;
 use oat\taoQtiTest\models\runner\QtiRunnerService;
 use oat\taoQtiTest\models\runner\QtiRunnerServiceContext;
-use oat\taoQtiTest\models\runner\time\TimerAdjustmentService;
+use oat\taoQtiTest\models\runner\time\TimerAdjustmentServiceInterface;
 use oat\taoQtiTest\models\TestSessionService;
 use PHPSession;
 use Psr\Log\LoggerInterface;
@@ -59,6 +59,8 @@ class ConcurringSessionService
     private FeatureFlagCheckerInterface $featureFlagChecker;
     private StateServiceInterface $stateService;
     private ?PHPSession $currentSession;
+    private ?TestSessionService $testSessionService;
+    private ?TimerAdjustmentServiceInterface $timerAdjustmentService;
 
     public function __construct(
         LoggerInterface $logger,
@@ -67,15 +69,19 @@ public function __construct(
         DeliveryExecutionService $deliveryExecutionService,
         FeatureFlagCheckerInterface $featureFlagChecker,
         StateServiceInterface $stateService,
-        PHPSession $currentSession = null
+        PHPSession $currentSession = null,
+        TestSessionService $testSessionService = null,
+        TimerAdjustmentServiceInterface $timerAdjustmentService = null
     ) {
         $this->logger = $logger;
         $this->qtiRunnerService = $qtiRunnerService;
         $this->runtimeService = $runtimeService;
         $this->deliveryExecutionService = $deliveryExecutionService;
         $this->featureFlagChecker = $featureFlagChecker;
-        $this->currentSession = $currentSession ?? PHPSession::singleton();
         $this->stateService = $stateService;
+        $this->currentSession = $currentSession ?? PHPSession::singleton();
+        $this->testSessionService = $testSessionService;
+        $this->timerAdjustmentService = $timerAdjustmentService;
     }
 
     public function pauseActiveDeliveryExecutionsForUser($activeExecution): void
@@ -168,7 +174,7 @@ public function adjustTimers(DeliveryExecution $execution): void
 
         $testSession = $this->getTestSessionService()->getTestSession($execution);
 
-        if ($testSession->getCurrentAssessmentItemRef()) {
+        if ($testSession && $testSession->getCurrentAssessmentItemRef()) {
             $duration = $testSession->getTimerDuration(
                 $testSession->getCurrentAssessmentItemRef()->getIdentifier(),
                 $testSession->getTimerTarget()
@@ -182,10 +188,10 @@ public function adjustTimers(DeliveryExecution $execution): void
                 )
             );
 
-            $ids = [
+            $ids = array_unique([
                 $execution->getIdentifier(),
                 $execution->getOriginalIdentifier()
-            ];
+            ]);
 
             foreach ($ids as $executionId) {
                 $key = "itemDuration-{$executionId}";
@@ -210,7 +216,11 @@ public function adjustTimers(DeliveryExecution $execution): void
                 if ($delta > 0) {
                     $this->logger->debug(sprintf('Adjusting timers by %d s', $delta));
 
-                    $this->getTimerAdjustmentService()->increase($testSession, $delta);
+                    $this->getTimerAdjustmentService()->increase(
+                        $testSession,
+                        $delta,
+                        TimerAdjustmentServiceInterface::TYPE_TIME_ADJUSTMENT
+                    );
 
                     $testSession->suspend();
                     $this->getTestSessionService()->persist($testSession);
@@ -298,20 +308,27 @@ private function getContextByDeliveryExecution(DeliveryExecutionInterface $execu
         );
     }
 
-    private function getTimerAdjustmentService(): TimerAdjustmentService
+    private function getTimerAdjustmentService(): TimerAdjustmentServiceInterface
     {
-        /** @noinspection PhpIncompatibleReturnTypeInspection */
-        return $this->getServiceManager()->get(TimerAdjustmentService::SERVICE_ID);
+        if (!$this->timerAdjustmentService instanceof TimerAdjustmentServiceInterface) {
+            /** @noinspection PhpFieldAssignmentTypeMismatchInspection */
+            $this->timerAdjustmentService = ServiceManager::getServiceManager()->get(
+                TimerAdjustmentServiceInterface::SERVICE_ID
+            );
+        }
+
+        return $this->timerAdjustmentService;
     }
 
     private function getTestSessionService(): TestSessionService
     {
-        /** @noinspection PhpIncompatibleReturnTypeInspection */
-        return $this->getServiceManager()->get(TestSessionService::SERVICE_ID);
-    }
+        if (!$this->testSessionService instanceof TestSessionService) {
+            /** @noinspection PhpFieldAssignmentTypeMismatchInspection */
+            $this->testSessionService = ServiceManager::getServiceManager()->get(
+                TestSessionService::SERVICE_ID
+            );
+        }
 
-    private function getServiceManager()
-    {
-        return ServiceManager::getServiceManager();
+        return $this->testSessionService;
     }
 }
diff --git a/test/unit/model/Service/ConcurringSessionServiceTest.php b/test/unit/model/Service/ConcurringSessionServiceTest.php
@@ -15,7 +15,7 @@
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  *
- * Copyright (c) 2023 (original work) Open Assessment Technologies SA;
+ * Copyright (c) 2023-2024 (original work) Open Assessment Technologies SA.
  */
 
 declare(strict_types=1);
@@ -33,6 +33,8 @@
 use oat\taoQtiTest\models\runner\QtiRunnerService;
 use oat\taoQtiTest\models\runner\QtiRunnerServiceContext;
 use oat\taoQtiTest\models\runner\session\TestSession;
+use oat\taoQtiTest\models\runner\time\TimerAdjustmentServiceInterface;
+use oat\taoQtiTest\models\TestSessionService;
 use oat\taoTests\models\runner\time\TimePoint;
 use PHPSession;
 use PHPUnit\Framework\TestCase;
@@ -49,6 +51,8 @@ class ConcurringSessionServiceTest extends TestCase
     private PHPSession $currentSession;
     private ConcurringSessionService $subject;
     private StateServiceInterface $stateService;
+    private TestSessionService $testSessionService;
+    private TimerAdjustmentServiceInterface $timerAdjustmentService;
 
     protected function setUp(): void
     {
@@ -58,6 +62,8 @@ protected function setUp(): void
         $this->featureFlagChecker = $this->createMock(FeatureFlagCheckerInterface::class);
         $this->currentSession = $this->createMock(PHPSession::class);
         $this->stateService = $this->createMock(StateServiceInterface::class);
+        $this->testSessionService = $this->createMock(TestSessionService::class);
+        $this->timerAdjustmentService = $this->createMock(TimerAdjustmentServiceInterface::class);
 
         $this->subject = new ConcurringSessionService(
             $this->createMock(LoggerInterface::class),
@@ -66,7 +72,9 @@ protected function setUp(): void
             $this->deliveryExecutionService,
             $this->featureFlagChecker,
             $this->stateService,
-            $this->currentSession
+            $this->currentSession,
+            $this->testSessionService,
+            $this->timerAdjustmentService
         );
     }
 
@@ -271,7 +279,7 @@ public function testPausesExecutionsForOtherDeliveries(): void
         $this->subject->pauseConcurrentSessions($execution);
     }
 
-    public function testPauseActiveDeliveryExecutionsForUser()
+    public function testPauseActiveDeliveryExecutionsForUser(): void
     {
         $this->featureFlagChecker
             ->method('isEnabled')
@@ -422,4 +430,116 @@ public function testPauseActiveDeliveryExecutionsForUser()
 
         $this->subject->pauseActiveDeliveryExecutionsForUser($execution);
     }
+
+    public function testAdjustTimersAddsAPositiveAdjustment(): void
+    {
+        $execution = $this->createMock(DeliveryExecution::class);
+        $execution
+            ->expects($this->atLeastOnce())
+            ->method('getIdentifier')
+            ->willReturn('https://example.com/execution/1');
+        $execution
+            ->expects($this->atLeastOnce())
+            ->method('getOriginalIdentifier')
+            ->willReturn('https://example.com/execution/1');
+
+        $itemRef = $this->createMock(AssessmentItemRef::class);
+        $itemRef
+            ->expects($this->once())
+            ->method('getIdentifier')
+            ->willReturn('itemRef');
+
+        $duration = $this->createMock(QtiDuration::class);
+        $duration
+            ->expects($this->atLeastOnce())
+            ->method('getSeconds')
+            ->with(true)
+            ->willReturn(4);
+
+        $testSession = $this->createMock(TestSession::class);
+        $testSession
+            ->expects($this->exactly(2))
+            ->method('getCurrentAssessmentItemRef')
+            ->willReturn($itemRef);
+        $testSession
+            ->expects($this->once())
+            ->method('getTimerTarget')
+            ->willReturn(TimePoint::TARGET_SERVER);
+        $testSession
+            ->expects($this->once())
+            ->method('getTimerDuration')
+            ->with('itemRef', TimePoint::TARGET_SERVER)
+            ->willReturn($duration);
+
+        $this->testSessionService
+            ->expects($this->once())
+            ->method('getTestSession')
+            ->with($execution)
+            ->willReturn($testSession);
+
+        $this->currentSession
+            ->expects($this->once())
+            ->method('hasAttribute')
+            ->with('itemDuration-https://example.com/execution/1')
+            ->willReturn(true);
+
+        $this->currentSession
+            ->expects($this->once())
+            ->method('getAttribute')
+            ->with('itemDuration-https://example.com/execution/1')
+            ->willReturn(1.5);
+
+        $this->currentSession
+            ->expects($this->once())
+            ->method('removeAttribute')
+            ->with('itemDuration-https://example.com/execution/1');
+
+        $this->timerAdjustmentService
+            ->expects($this->once())
+            ->method('increase')
+            ->with(
+                $testSession,
+                ceil(2.5),
+                TimerAdjustmentServiceInterface::TYPE_TIME_ADJUSTMENT
+            );
+
+        $testSession
+            ->expects($this->once())
+            ->method('suspend');
+
+        $this->testSessionService
+            ->expects($this->once())
+            ->method('persist')
+            ->with($testSession);
+
+        $this->subject->adjustTimers($execution);
+    }
+
+    public function testAdjustTimersSkipsAdjustmentIfNoTestSessionExists(): void
+    {
+        $execution = $this->createMock(DeliveryExecution::class);
+        $execution
+            ->method('getIdentifier')
+            ->willReturn('https://example.com/execution/1');
+
+        $this->testSessionService
+            ->expects($this->once())
+            ->method('getTestSession')
+            ->with($execution)
+            ->willReturn(null);
+
+        $this->currentSession
+            ->expects($this->never())
+            ->method('removeAttribute');
+
+        $this->timerAdjustmentService
+            ->expects($this->never())
+            ->method('increase');
+
+        $this->testSessionService
+            ->expects($this->never())
+            ->method('persist');
+
+        $this->subject->adjustTimers($execution);
+    }
 }