Skip to content

Commit

Permalink
Adding "strict" flag for the group locks. Non-strict lock allows the …
Browse files Browse the repository at this point in the history
…students (read-only) access to other groups, so they can use their older solutions.
  • Loading branch information
krulis-martin committed May 24, 2024
1 parent b839219 commit eba3fa7
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 13 deletions.
26 changes: 21 additions & 5 deletions app/V1Module/presenters/GroupsPresenter.php
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,8 @@ public function checkSetExamPeriod(string $id)
* description="When the exam begins (unix ts in the future, optional if update is performed).")
* @Param(type="post", name="end", validation="timestamp", required=true,
* description="When the exam ends (unix ts in the future, no more than a day after 'begin').")
* @Param(type="post", name="strict", validation="bool", required=false,
* description="Whether locked users are prevented from accessing other groups.")
* @param string $id An identifier of the updated group
* @throws NotFoundException
*/
Expand All @@ -517,11 +519,21 @@ public function actionSetExamPeriod(string $id)
$req = $this->getRequest();
$beginTs = (int)$req->getPost("begin");
$endTs = (int)$req->getPost("end");
$strict = $req->getPost("strict") !== null
? filter_var($req->getPost("strict"), FILTER_VALIDATE_BOOLEAN) : null;
$now = (new DateTime())->getTimestamp();
$nowTolerance = 60; // 60s is a tolerance when comparing with "now"

if ($strict === null) {
if ($group->hasExamPeriodSet()) {
$strict = $group->isExamLockStrict(); // flag is not present -> is not changing
} else {
throw new BadRequestException("The strict flag must be present when new exam is being set.");
}
}

// beginning must be in the future (or must not be modified)
if ((!$group->hasExamPeriodSet() || $beginTs) && $beginTs < $now - 60) {
if ((!$group->hasExamPeriodSet() || $beginTs) && $beginTs < $now - $nowTolerance) {
throw new BadRequestException("The exam must be set in the future.");
}

Expand All @@ -535,7 +547,7 @@ public function actionSetExamPeriod(string $id)
}

// the end should also be in the future (this is necessary only for updates)
if ($endTs < $now - 60) {
if ($endTs < $now - $nowTolerance) {
throw new BadRequestException("The exam end must be set in the future.");
}

Expand All @@ -544,10 +556,14 @@ public function actionSetExamPeriod(string $id)

if ($group->hasExamPeriodSet()) {
if ($group->getExamBegin()->getTimestamp() <= $now) { // ... already begun
if ($strict !== $group->isExamLockStrict()) {
throw new BadRequestException("The strict flag cannot be changed once the exam begins.");
}

// the exam already begun, we need to fix any group-locked users
foreach ($group->getStudents() as $student) {
if ($student->getGroupLock()?->getId() === $id) {
$student->setGroupLock($group, $end);
$student->setGroupLock($group, $end, $strict);
if ($student->isIpLocked()) {
$student->setIpLock($student->getIpLockRaw(), $end);
}
Expand Down Expand Up @@ -579,7 +595,7 @@ public function actionSetExamPeriod(string $id)
}
}

$group->setExamPeriod($begin, $end);
$group->setExamPeriod($begin, $end, $strict);
$this->groups->persist($group);

$this->sendSuccessResponse($this->groupViewFactory->getGroup($group));
Expand Down Expand Up @@ -1140,7 +1156,7 @@ public function actionLockStudent(string $id, string $userId)

$expiration = $group->getExamEnd();
$user->setIpLock($this->getHttpRequest()->getRemoteAddress(), $expiration);
$user->setGroupLock($group, $expiration);
$user->setGroupLock($group, $expiration, $group->isExamLockStrict());
$this->users->persist($user, false);

// make sure the locking is also logged
Expand Down
17 changes: 16 additions & 1 deletion app/model/entity/Group.php
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,13 @@ public function isDirectlyArchived(): bool
*/
protected $examEnd = null;

/**
* @ORM\Column(type="boolean")
* Whether the group-lock for the exam should be strict
* (under strict lock, the user cannot read data from other groups).
*/
protected $examLockStrict = false;

/**
* @var Collection
* @ORM\OneToMany(targetEntity="GroupExam", mappedBy="group",
Expand All @@ -240,8 +247,9 @@ public function isDirectlyArchived(): bool
* Switch the group into an exam group by setting the begin and end dates of the exam.
* @param DateTime $begin when the exam starts
* @param DateTime $end when the exam ends
* @param bool $strict if true, locked users cannot acceess other groups (for reading)
*/
public function setExamPeriod(DateTime $begin, DateTime $end): void
public function setExamPeriod(DateTime $begin, DateTime $end, bool $strict = false): void
{
// asserts
if ($begin >= $end) {
Expand All @@ -254,6 +262,7 @@ public function setExamPeriod(DateTime $begin, DateTime $end): void

$this->examBegin = $begin;
$this->examEnd = $end;
$this->examLockStrict = $strict;
$this->isOrganizational = false;
}

Expand All @@ -264,6 +273,7 @@ public function removeExamPeriod(): void
{
$this->examBegin = null;
$this->examEnd = null;
$this->examLockStrict = false;
}

/**
Expand All @@ -276,6 +286,11 @@ public function hasExamPeriodSet(DateTime $at = null): bool
return $this->examBegin !== null && $this->examEnd !== null && $this->examEnd > $at;
}

public function isExamLockStrict(): bool
{
return $this->examLockStrict;
}

/**
* @ORM\ManyToOne(targetEntity="Group", inversedBy="childGroups")
*/
Expand Down
18 changes: 16 additions & 2 deletions app/model/entity/GroupExam.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
* @ORM\Entity
* @ORM\Table(indexes={@ORM\Index(name="group_begin_idx", columns={"group_id", "begin"})})
* Holds history record of an exam that took place in a group.
* The `examBegin`, `examEnd` fields are copied from group to `begin`, `end` fields here.
* The `examBegin`, `examEnd` fields are copied from group to `begin`, `end` fields here,
* `examLockStrict` is copied to `lockStrict` field.
* This entity is created when the first user locks in (i.e., only exams with users are recorded in history).
*/
class GroupExam implements JsonSerializable
Expand Down Expand Up @@ -39,17 +40,24 @@ class GroupExam implements JsonSerializable
*/
protected $end = null;

/**
* @ORM\Column(type="boolean")
* Saved value from examLockStrict flag.
*/
protected $lockStrict = false;

/**
* Constructor
* @param Group $group
* @param DateTime $begin
* @param DateTime $end
*/
public function __construct(Group $group, DateTime $begin, DateTime $end)
public function __construct(Group $group, DateTime $begin, DateTime $end, bool $strict)
{
$this->group = $group;
$this->begin = $begin;
$this->end = $end;
$this->lockStrict = $strict;
}

public function jsonSerialize(): mixed
Expand All @@ -60,6 +68,7 @@ public function jsonSerialize(): mixed
"groupId" => $group ? $group->getId() : null,
"begin" => $this->getBegin()->getTimestamp(),
"end" => $this->getEnd()->getTimestamp(),
"strict" => $this->lockStrict,
];
}

Expand Down Expand Up @@ -87,4 +96,9 @@ public function getEnd(): DateTime
{
return $this->end;
}

public function isLockStrict(): bool
{
return $this->lockStrict;
}
}
18 changes: 17 additions & 1 deletion app/model/entity/User.php
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,11 @@ public function verifyIpLock(string $currentIp): bool
*/
protected $groupLock = null;

/**
* @ORM\Column(type="boolean")
*/
protected $groupLockStrict = false;

/**
* @ORM\Column(type="datetime", nullable=true)
* @var DateTime|null
Expand All @@ -519,6 +524,14 @@ public function isGroupLocked(): bool
($this->groupLockExpiration === null || $this->groupLockExpiration > (new DateTime()));
}

/**
* @return bool True if the lock is strict and the user should not access other groups at all.
*/
public function isGroupLockStrict(): bool
{
return $this->groupLockStrict;
}

/**
* @return Group|null The group in which the user is currently locked, null if no lock is active.
*/
Expand All @@ -536,8 +549,9 @@ public function getGroupLockExpiration(): ?DateTimeInterface
* Lock the user within a group.
* @param Group $group
* @param DateTime|null $expiration of the lock, if null, the lock will never expire
* @param bool $strict if true, the user may not even accress other groups for reading
*/
public function setGroupLock(Group $group, DateTime $expiration = null): void
public function setGroupLock(Group $group, DateTime $expiration = null, bool $strict = false): void
{
// basic asserts to be on the safe side
if (!$group->hasExamPeriodSet()) {
Expand All @@ -549,12 +563,14 @@ public function setGroupLock(Group $group, DateTime $expiration = null): void

$this->groupLock = $group;
$this->groupLockExpiration = $expiration;
$this->groupLockStrict = $strict;
}

public function removeGroupLock(): void
{
$this->groupLock = null;
$this->groupLockExpiration = null;
$this->groupLockStrict = false;
}


Expand Down
14 changes: 11 additions & 3 deletions app/model/repository/GroupExams.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,26 @@ public function __construct(EntityManagerInterface $em)
* @param Group $group
* @param DateTime|null $begin if null, exam begin from the group is taken
* @param DateTime|null $end if null, exam end from the group is taken
* @param bool|null $strict if null, examLockStrict value is taken
* @return GroupExam
*/
public function findOrCreate(Group $group, DateTime $begin = null, DateTime $end = null): GroupExam
{
public function findOrCreate(
Group $group,
DateTime $begin = null,
DateTime $end = null,
bool $strict = null
): GroupExam {
$begin = $begin ?? $group->getExamBegin();
$end = $end ?? $group->getExamEnd();
$strict = $strict === null ? $group->isExamLockStrict() : $strict;

$exam = $this->findBy(["group" => $group, "begin" => $begin]);
if (count($exam) > 1) {
throw new Exception("Data corruption, there is more than one group exam starting at the same time.");
}

if (!$exam) {
$exam = new GroupExam($group, $begin, $end ?? $group->getExamEnd());
$exam = new GroupExam($group, $begin, $end, $strict);
$this->persist($exam);
} else {
$exam = reset($exam);
Expand Down
2 changes: 1 addition & 1 deletion tests/Presenters/GroupsPresenter.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -1337,7 +1337,7 @@ class TestGroupsPresenter extends Tester\TestCase
'V1:Groups',
'POST',
['action' => 'setExamPeriod', 'id' => $group->getId()],
['begin' => $begin, 'end' => $end]
['begin' => $begin, 'end' => $end, 'strict' => true]
);

Assert::equal($group->getId(), $payload['id']);
Expand Down

0 comments on commit eba3fa7

Please sign in to comment.