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

feat: Adjust server-side timers when resuming an execution #2438

Merged
merged 18 commits into from
Jan 25, 2024
Merged

feat: Adjust server-side timers when resuming an execution #2438

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
851360e
feat: Adjust server timers when resuming an execution
hectoras Jan 15, 2024
e9bc636
chore: Cleanup
hectoras Jan 15, 2024
83dd3f8
chore: Cleanup
hectoras Jan 15, 2024
a5b8f75
chore: Update year in license header
hectoras Jan 16, 2024
8b5b6f2
chore: Get the former order for duration cache updates
hectoras Jan 16, 2024
229d5a3
fix: Explicitly provide the component to apply the adjustment in
hectoras Jan 16, 2024
fb85c06
chore: Revert to the simpler approach applying the adjustment tot he …
hectoras Jan 16, 2024
c5d9648
chore: Change log msg
hectoras Jan 16, 2024
8da35ae
fix: Store the pause timestamp associated to all execution IDs
hectoras Jan 16, 2024
4393a42
chore: Cleanup
hectoras Jan 16, 2024
0519afb
chore: Cleanup
hectoras Jan 16, 2024
b4e1bd2
chore: Cleanup
hectoras Jan 16, 2024
4c1ca83
chore: Coding style improvements
hectoras Jan 17, 2024
46d1793
chore: Use a lower log level for debug messages in ConcurringSessionS…
hectoras Jan 17, 2024
46991ef
fix: Don't try to update the duration cache for non-available sources
hectoras Jan 18, 2024
75fb5c8
fix: Pass the execution user ID when fetching the service context
hectoras Jan 22, 2024
f841314
fix: Store the item duration instead of the current timestamp on test…
hectoras Jan 25, 2024
c6b4f7b
test: Update unit tests
hectoras Jan 25, 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
135 changes: 127 additions & 8 deletions model/Service/ConcurringSessionService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,15 @@
* 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);

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 @@ -31,8 +32,12 @@
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\TestSessionService;
use PHPSession;
use Psr\Log\LoggerInterface;
use qtism\common\datatypes\QtiDuration;
use qtism\data\AssessmentItemRef;
use Throwable;

class ConcurringSessionService
Expand Down Expand Up @@ -81,20 +86,23 @@ public function pauseConcurrentSessions(DeliveryExecution $activeExecution): voi
);

foreach ($userExecutions as $execution) {
if ($execution->getOriginalIdentifier() !== $activeExecution->getOriginalIdentifier()) {
$executionId = $execution->getOriginalIdentifier();

if ($executionId !== $activeExecution->getOriginalIdentifier()) {
try {
$this->setConcurringSession($execution->getOriginalIdentifier());
$this->setConcurringSession($executionId);

$context = $this->getContextByDeliveryExecution($execution);

$this->storeItemDuration($context, $executionId);
$this->qtiRunnerService->endTimer($context);
$this->qtiRunnerService->pause($context);
} catch (Throwable $e) {
$this->logger->warning(
sprintf(
'%s: Unable to pause delivery execution %s: %s',
self::class,
$execution->getOriginalIdentifier(),
$executionId,
$e->getMessage()
)
);
Expand Down Expand Up @@ -124,9 +132,101 @@ public function setConcurringSession(string $executionId): void
);
}

public function adjustTimers(DeliveryExecution $execution): void
{
$this->logger->debug(
sprintf(
'Adjusting timers on execution %s restart',
$execution->getIdentifier()
)
);

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

if ($testSession->getCurrentAssessmentItemRef()) {
$duration = $testSession->getTimerDuration(
$testSession->getCurrentAssessmentItemRef()->getIdentifier(),
$testSession->getTimerTarget()
);

$this->logger->debug(
sprintf(
'Timer duration on execution %s timer adjustment = %f',
$execution->getIdentifier(),
$duration->getSeconds(true)
)
);

$ids = [
$execution->getIdentifier(),
$execution->getOriginalIdentifier()
];

foreach ($ids as $executionId) {
$key = "itemDuration-{$executionId}";

if (!$this->currentSession->hasAttribute($key)) {
continue;
}

$oldDuration = $this->currentSession->getAttribute($key);
$this->currentSession->removeAttribute($key);

$this->logger->debug(
sprintf(
'Timer duration on execution %s pause was %f',
$execution->getIdentifier(),
$oldDuration
)
);

$delta = (int) ceil($duration->getSeconds(true) - $oldDuration);

if ($delta > 0) {
$this->logger->debug(sprintf('Adjusting timers by %d s', $delta));

$this->getTimerAdjustmentService()->increase($testSession, $delta);

$testSession->suspend();
$this->getTestSessionService()->persist($testSession);
}
}
}
}

private function storeItemDuration(
QtiRunnerServiceContext $context,
string $executionId
): void {
$testSession = $context->getTestSession();
$itemRef = $testSession->getCurrentAssessmentItemRef();

if ($itemRef instanceof AssessmentItemRef) {
/** @var QtiDuration $duration */
$duration = $context->getTestSession()->getTimerDuration(
$itemRef->getIdentifier(),
$testSession->getTimerTarget()
);

$this->logger->debug(
sprintf(
'duration when execution %s was paused = %f',
$executionId,
$duration->getSeconds(true)
)
);

$this->currentSession->setAttribute(
"itemDuration-{$executionId}",
$duration->getSeconds(true)
);
}
}

private function getContextByDeliveryExecution(DeliveryExecutionInterface $execution): QtiRunnerServiceContext
{
$container = $this->runtimeService->getDeliveryContainer($execution->getDelivery()->getUri());
$delivery = $execution->getDelivery();
$container = $this->runtimeService->getDeliveryContainer($delivery->getUri());

if (!$container instanceof QtiTestDeliveryContainer) {
throw new common_Exception(
Expand All @@ -137,17 +237,36 @@ private function getContextByDeliveryExecution(DeliveryExecutionInterface $execu
);
}

$testDefinition = $container->getSourceTest($execution);
$sessionId = $execution->getIdentifier();
$testDefinitionUri = $container->getSourceTest($execution);
$testCompilation = sprintf(
'%s|%s',
$container->getPrivateDirId($execution),
$container->getPublicDirId($execution)
);

return $this->qtiRunnerService->getServiceContext(
$testDefinition,
$testDefinitionUri,
$testCompilation,
$execution->getOriginalIdentifier()
$sessionId,
$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();
}
}
12 changes: 8 additions & 4 deletions models/classes/runner/session/TestSession.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) 2016-2017 (original work) Open Assessment Technologies SA
* Copyright (c) 2016-2024 (original work) Open Assessment Technologies SA
*
*/

Expand Down Expand Up @@ -336,14 +336,18 @@ protected function updateCurrentDurationCache()
{
$target = $this->getTimerTarget();
$routeItem = $this->getCurrentRouteItem();

$sources = [
$routeItem->getAssessmentTest(),
$this->getAssessmentTest(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we sure, that we can use $this (TestSession) instead of RoutItem ?

Copy link
Contributor Author

@hectoras hectoras Jan 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This solves a bug: If this ends up being called on a test (lets call it test B) other than the one we are current running (lets call it test A), $routeItem is null. That is, getCurrentRouteItem() returns null when called from a request associated with the test runner session for test A to get the current item for test B.

    protected function getCurrentRouteItem()
    {
        if ($this->isRunning() === true) {
            return $this->getRoute()->current();
        }

        return false;
    }

The test we are pausing (B) is / was running, but not in the current request (which is "bound" to A), and we need this to work also in that case (so the elapsed time for the item of test B is stored when suspending the test).

Both $routeItem->getAssessmentItemRef() and $this->getAssessmentTest() return an instance of AssessmentTest that refers to the same test.

$this->getCurrentTestPart(),
$this->getCurrentAssessmentSection(),
$routeItem->getAssessmentItemRef(),
];

foreach ($sources as $source) {
if ($routeItem instanceof RouteItem) {
$sources[] = $routeItem->getAssessmentItemRef();
}

foreach (array_filter($sources) as $source) {
$this->updateDurationCache($source->getIdentifier(), $target);
}
}
Expand Down
57 changes: 50 additions & 7 deletions test/unit/model/Service/ConcurringSessionServiceTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,13 @@
use oat\taoQtiTest\models\container\QtiTestDeliveryContainer;
use oat\taoQtiTest\models\runner\QtiRunnerService;
use oat\taoQtiTest\models\runner\QtiRunnerServiceContext;
use oat\taoQtiTest\models\runner\session\TestSession;
use oat\taoTests\models\runner\time\TimePoint;
use PHPSession;
use PHPUnit\Framework\TestCase;
use Psr\Log\LoggerInterface;
use qtism\common\datatypes\QtiDuration;
use qtism\data\AssessmentItemRef;

class ConcurringSessionServiceTest extends TestCase
{
Expand Down Expand Up @@ -129,7 +133,6 @@ public function testPausesExecutionsForOtherDeliveries(): void
->with('FEATURE_FLAG_PAUSE_CONCURRENT_SESSIONS')
->willReturn(true);

$deliveryResource = $this->createMock(core_kernel_classes_Resource::class);
$otherDeliveryResource = $this->createMock(core_kernel_classes_Resource::class);
$executionDomainObject = $this->createMock(DeliveryExecution::class);
$otherExecutionDomainObject = $this->createMock(DeliveryExecution::class);
Expand All @@ -147,6 +150,10 @@ public function testPausesExecutionsForOtherDeliveries(): void
->expects($this->atLeastOnce())
->method('getOriginalIdentifier')
->willReturn('https://example.com/execution/2');
$otherExecutionDomainObject
->expects($this->atLeastOnce())
->method('getIdentifier')
->willReturn('https://example.com/execution/2');

$this->deliveryExecutionService
->expects($this->atLeastOnce())
Expand Down Expand Up @@ -191,7 +198,39 @@ public function testPausesExecutionsForOtherDeliveries(): void
->with('https://example.com/delivery/2')
->willReturn($qtiTestDeliveryContainer);

$itemRef = $this->createMock(AssessmentItemRef::class);
$itemRef
->expects($this->once())
->method('getIdentifier')
->willReturn('itemRef');

$duration = $this->createMock(QtiDuration::class);
$duration
->expects($this->exactly(2))
->method('getSeconds')
->with(true)
->willReturn(123);

$testSession = $this->createMock(TestSession::class);
$testSession
->expects($this->once())
->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);

$context = $this->createMock(QtiRunnerServiceContext::class);
$context
->expects($this->exactly(2))
->method('getTestSession')
->willReturn($testSession);

$this->qtiRunnerService
->expects($this->once())
Expand All @@ -202,14 +241,18 @@ public function testPausesExecutionsForOtherDeliveries(): void
'https://example.com/execution/2'
)->willReturn($context);

// Expectations
//
$this->currentSession
->expects($this->once())
->expects($this->exactly(2))
->method('setAttribute')
->with(
'pauseReason-https://example.com/execution/2',
'PAUSE_REASON_CONCURRENT_TEST'
->withConsecutive(
[
'pauseReason-https://example.com/execution/2',
'PAUSE_REASON_CONCURRENT_TEST'
],
[
'itemDuration-https://example.com/execution/2',
123
]
)->willReturn(null);

$this->qtiRunnerService
Expand Down
Loading