Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
May 20, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9c69e78
fix: Don't fetch the current item for a null test session
hectoras May 16, 2024
024d5dd
chore: Use DI to fetch services from ConcurringSessionService
hectoras May 17, 2024
d06c1ab
chore: Fix tests
hectoras May 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions model/Container/TestQtiServiceProvider.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +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) 2021-2023 (original work) Open Assessment Technologies SA;
*
* @author Ricardo Quintanilha <[email protected]>
* Copyright (c) 2021-2024 (original work) Open Assessment Technologies SA;
*/

declare(strict_types=1);
Expand Down Expand Up @@ -47,7 +45,9 @@
use oat\taoQtiTest\model\Service\StoreTraceVariablesService;
use oat\taoQtiTest\model\Service\TimeoutService;
use oat\taoQtiTest\models\runner\QtiRunnerService;
use oat\taoQtiTest\models\runner\time\TimerAdjustmentServiceInterface;
use oat\taoQtiTest\models\TestModelService;
use oat\taoQtiTest\models\TestSessionService;
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;

use function Symfony\Component\DependencyInjection\Loader\Configurator\service;
Expand Down Expand Up @@ -172,6 +172,8 @@ public function __invoke(ContainerConfigurator $configurator): void
service(DeliveryExecutionService::SERVICE_ID),
service(FeatureFlagChecker::class),
service(StateServiceInterface::SERVICE_ID),
service(TestSessionService::SERVICE_ID),
service(TimerAdjustmentServiceInterface::SERVICE_ID),
]
);
}
Expand Down
45 changes: 19 additions & 26 deletions model/Service/ConcurringSessionService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
namespace oat\taoQtiTest\model\Service;

use common_Exception;
use oat\oatbox\service\ServiceManager;
use oat\tao\model\featureFlag\FeatureFlagCheckerInterface;
use oat\taoDelivery\model\execution\DeliveryExecution;
use oat\taoDelivery\model\execution\DeliveryExecutionInterface;
Expand All @@ -33,7 +32,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;
Expand All @@ -44,6 +43,7 @@
class ConcurringSessionService
{
private const PAUSE_REASON_CONCURRENT_TEST = 'PAUSE_REASON_CONCURRENT_TEST';

/**
* @var string Controls whether a delivery execution state should be kept as is or reset each time it starts.
* `false` – the state will be reset on each restart.
Expand All @@ -58,6 +58,8 @@ class ConcurringSessionService
private DeliveryExecutionService $deliveryExecutionService;
private FeatureFlagCheckerInterface $featureFlagChecker;
private StateServiceInterface $stateService;
private TestSessionService $testSessionService;
private TimerAdjustmentServiceInterface $timerAdjustmentService;
private ?PHPSession $currentSession;

public function __construct(
Expand All @@ -67,15 +69,19 @@ public function __construct(
DeliveryExecutionService $deliveryExecutionService,
FeatureFlagCheckerInterface $featureFlagChecker,
StateServiceInterface $stateService,
TestSessionService $testSessionService,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Can you please make sure there is no other DI Service provider instantiating this class apart of TestQtiServiceProvider? Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just checked and it is only used in the TestQtiServiceProvider from this extension, and all instantiations at runtime are done though the PSR container.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

TimerAdjustmentServiceInterface $timerAdjustmentService,
PHPSession $currentSession = 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->testSessionService = $testSessionService;
$this->timerAdjustmentService = $timerAdjustmentService;
$this->currentSession = $currentSession ?? PHPSession::singleton();
}

public function pauseActiveDeliveryExecutionsForUser($activeExecution): void
Expand Down Expand Up @@ -166,9 +172,9 @@ public function adjustTimers(DeliveryExecution $execution): void
)
);

$testSession = $this->getTestSessionService()->getTestSession($execution);
$testSession = $this->testSessionService->getTestSession($execution);

if ($testSession->getCurrentAssessmentItemRef()) {
if ($testSession && $testSession->getCurrentAssessmentItemRef()) {
$duration = $testSession->getTimerDuration(
$testSession->getCurrentAssessmentItemRef()->getIdentifier(),
$testSession->getTimerTarget()
Expand All @@ -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}";
Expand All @@ -210,10 +216,14 @@ 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->timerAdjustmentService->increase(
$testSession,
$delta,
TimerAdjustmentServiceInterface::TYPE_TIME_ADJUSTMENT
);

$testSession->suspend();
$this->getTestSessionService()->persist($testSession);
$this->testSessionService->persist($testSession);
}
}
}
Expand Down Expand Up @@ -297,21 +307,4 @@ private function getContextByDeliveryExecution(DeliveryExecutionInterface $execu
$execution->getUserIdentifier()
);
}

private function getTimerAdjustmentService(): TimerAdjustmentService
{
/** @noinspection PhpIncompatibleReturnTypeInspection */
return $this->getServiceManager()->get(TimerAdjustmentService::SERVICE_ID);
}

private function getTestSessionService(): TestSessionService
{
/** @noinspection PhpIncompatibleReturnTypeInspection */
return $this->getServiceManager()->get(TestSessionService::SERVICE_ID);
}

private function getServiceManager()
{
return ServiceManager::getServiceManager();
}
}
124 changes: 122 additions & 2 deletions test/unit/model/Service/ConcurringSessionServiceTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
Expand All @@ -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
{
Expand All @@ -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),
Expand All @@ -66,6 +72,8 @@ protected function setUp(): void
$this->deliveryExecutionService,
$this->featureFlagChecker,
$this->stateService,
$this->testSessionService,
$this->timerAdjustmentService,
$this->currentSession
);
}
Expand Down Expand Up @@ -271,7 +279,7 @@ public function testPausesExecutionsForOtherDeliveries(): void
$this->subject->pauseConcurrentSessions($execution);
}

public function testPauseActiveDeliveryExecutionsForUser()
public function testPauseActiveDeliveryExecutionsForUser(): void
{
$this->featureFlagChecker
->method('isEnabled')
Expand Down Expand Up @@ -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);
}
}
Loading