From e50e1d111441f5e516bc622ed20aaa4cb2c0b498 Mon Sep 17 00:00:00 2001 From: Anna Larch Date: Sat, 25 Apr 2026 15:41:53 +0200 Subject: [PATCH 1/3] refactor(room): Split two-arg setActiveSince() into single-arg setters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Part of the incremental migration of `Room` to extend `OCP\AppFramework\Db\Entity` (Phase 1d). `Room::setActiveSince(\DateTime $since, int $callFlag)` combined two concerns — conditionally setting the active-since timestamp and bitwise-ORing the call flag — in a single two-argument method. Entity's magic `__call()` requires standard single-argument setters. - Replace with `setActiveSince(?\DateTime $activeSince)` and `setCallFlag(int $callFlag)` - Move the conditional/bitwise logic into `RoomService::setActiveSince()`, where it already lived conceptually AI-Assisted-By: Claude Sonnet 4.6 Signed-off-by: Anna Larch --- lib/Room.php | 11 ++--- lib/Service/RoomService.php | 5 ++- tests/php/RoomTest.php | 86 +++++++++++++++++++++++++++++++++++++ 3 files changed, 95 insertions(+), 7 deletions(-) create mode 100644 tests/php/RoomTest.php diff --git a/lib/Room.php b/lib/Room.php index 6b80d6d0d61..910f3fe7256 100644 --- a/lib/Room.php +++ b/lib/Room.php @@ -410,11 +410,12 @@ public function setParticipant(?string $userId, Participant $participant): void $this->participant = $participant; } - public function setActiveSince(\DateTime $since, int $callFlag): void { - if (!$this->activeSince) { - $this->activeSince = $since; - } - $this->callFlag |= $callFlag; + public function setActiveSince(?\DateTime $activeSince): void { + $this->activeSince = $activeSince; + } + + public function setCallFlag(int $callFlag): void { + $this->callFlag = $callFlag; } public function getBreakoutRoomMode(): int { diff --git a/lib/Service/RoomService.php b/lib/Service/RoomService.php index a3761d5c7fd..8bc79846ace 100644 --- a/lib/Service/RoomService.php +++ b/lib/Service/RoomService.php @@ -1195,7 +1195,7 @@ public function setActiveSince(Room $room, ?Participant $participant, \DateTime if ($room->getActiveSince() instanceof \DateTime) { // Call is already active, just someone upgrading the call flags - $room->setActiveSince($room->getActiveSince(), $callFlag); + $room->setCallFlag($callFlag); $event = new RoomModifiedEvent($room, ARoomModifiedEvent::PROPERTY_IN_CALL, $callFlag, $oldCallFlag); $this->dispatcher->dispatchTyped($event); @@ -1210,7 +1210,8 @@ public function setActiveSince(Room $room, ?Participant $participant, \DateTime ->andWhere($update->expr()->isNull('active_since')); $result = (bool)$update->executeStatement(); - $room->setActiveSince($since, $callFlag); + $room->setActiveSince($since); + $room->setCallFlag($callFlag); if (!$result) { // Lost the race, someone else updated the database diff --git a/tests/php/RoomTest.php b/tests/php/RoomTest.php new file mode 100644 index 00000000000..2b2f79a91b9 --- /dev/null +++ b/tests/php/RoomTest.php @@ -0,0 +1,86 @@ +createMock(ITimeFactory::class), + 1, + Room::TYPE_GROUP, + Room::READ_WRITE, + Room::LISTABLE_NONE, + 0, + Webinary::LOBBY_NONE, + Webinary::SIP_DISABLED, + null, + 'token', + 'name', + '', + '', + '', + '', + '', + Attendee::PERMISSIONS_DEFAULT, + Participant::FLAG_DISCONNECTED, + null, + null, + 0, + null, + null, + '', + '', + BreakoutRoom::MODE_NOT_CONFIGURED, + BreakoutRoom::STATUS_STOPPED, + Room::RECORDING_NONE, + RecordingService::CONSENT_REQUIRED_NO, + Room::HAS_FEDERATION_NONE, + Room::MENTION_PERMISSIONS_EVERYONE, + '', + 0, + 0, + ); + } + + public function testSetActiveSinceSetsValue(): void { + $room = $this->createRoom(); + $since = new \DateTime(); + $room->setActiveSince($since); + $this->assertSame($since, $room->getActiveSince()); + } + + public function testSetActiveSinceAcceptsNull(): void { + $room = $this->createRoom(); + $room->setActiveSince(new \DateTime()); + $room->setActiveSince(null); + $this->assertNull($room->getActiveSince()); + } + + public function testSetCallFlagSetsValue(): void { + $room = $this->createRoom(); + $room->setCallFlag(Participant::FLAG_WITH_VIDEO); + $this->assertSame(Participant::FLAG_WITH_VIDEO, $room->getCallFlag()); + } + + public function testSetCallFlagReplacesValue(): void { + $room = $this->createRoom(); + $room->setCallFlag(Participant::FLAG_IN_CALL); + $room->setCallFlag(Participant::FLAG_WITH_VIDEO); + $this->assertSame(Participant::FLAG_WITH_VIDEO, $room->getCallFlag()); + } +} From afa63f17e30aced3ebaf2e0f81da365497f06d8f Mon Sep 17 00:00:00 2001 From: Anna Larch Date: Sat, 25 Apr 2026 15:58:05 +0200 Subject: [PATCH 2/3] test(RoomService): Add unit tests for setActiveSince() AI-Assisted-By: Claude Sonnet 4.6 Signed-off-by: Anna Larch --- tests/php/Service/RoomServiceTest.php | 40 +++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/tests/php/Service/RoomServiceTest.php b/tests/php/Service/RoomServiceTest.php index 610879abf03..0ebb5134f6c 100644 --- a/tests/php/Service/RoomServiceTest.php +++ b/tests/php/Service/RoomServiceTest.php @@ -390,4 +390,44 @@ public function testVerifyPassword(): void { $this->assertSame($verificationResult, ['result' => false, 'url' => 'https://test']); $this->assertSame('passy', $room->getPassword()); } + + public function testSetActiveSinceNoOpWhenFlagsUnchanged(): void { + $since = new \DateTime(); + $room = $this->createMock(Room::class); + $room->method('getActiveSince')->willReturn($since); + $room->method('getCallFlag')->willReturn(Participant::FLAG_WITH_VIDEO); + $room->expects($this->never())->method('setActiveSince'); + $room->expects($this->never())->method('setCallFlag'); + + $result = $this->service->setActiveSince($room, null, $since, Participant::FLAG_WITH_VIDEO, false); + + $this->assertFalse($result); + } + + public function testSetActiveSinceUpgradesCallFlagOnly(): void { + $since = new \DateTime(); + $room = $this->createMock(Room::class); + $room->method('getActiveSince')->willReturn($since); + $room->method('getCallFlag')->willReturn(Participant::FLAG_IN_CALL); + $room->method('getId')->willReturn(0); + $room->expects($this->never())->method('setActiveSince'); + $room->expects($this->once())->method('setCallFlag') + ->with(Participant::FLAG_IN_CALL | Participant::FLAG_WITH_VIDEO); + + $result = $this->service->setActiveSince($room, null, $since, Participant::FLAG_WITH_VIDEO, false); + + $this->assertFalse($result); + } + + public function testSetActiveSinceSetsActiveSinceAndCallFlagOnFreshCall(): void { + $since = new \DateTime(); + $room = $this->createMock(Room::class); + $room->method('getActiveSince')->willReturn(null); + $room->method('getCallFlag')->willReturn(Participant::FLAG_DISCONNECTED); + $room->method('getId')->willReturn(0); + $room->expects($this->once())->method('setActiveSince')->with($since); + $room->expects($this->once())->method('setCallFlag')->with(Participant::FLAG_WITH_VIDEO); + + $this->service->setActiveSince($room, null, $since, Participant::FLAG_WITH_VIDEO, false); + } } From f84b0d61ee9848d5563b16113f0e290507601489 Mon Sep 17 00:00:00 2001 From: Anna Larch Date: Sat, 25 Apr 2026 15:59:56 +0200 Subject: [PATCH 3/3] test(RoomService): Add flag-merging test for setActiveSince() on fresh call Verifies that the bitwise OR previously in Room::setActiveSince() is preserved after moving it to the service layer. AI-Assisted-By: Claude Sonnet 4.6 Signed-off-by: Anna Larch --- tests/php/Service/RoomServiceTest.php | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/php/Service/RoomServiceTest.php b/tests/php/Service/RoomServiceTest.php index 0ebb5134f6c..a7aaca3c3fd 100644 --- a/tests/php/Service/RoomServiceTest.php +++ b/tests/php/Service/RoomServiceTest.php @@ -430,4 +430,19 @@ public function testSetActiveSinceSetsActiveSinceAndCallFlagOnFreshCall(): void $this->service->setActiveSince($room, null, $since, Participant::FLAG_WITH_VIDEO, false); } + + public function testSetActiveSinceMergesExistingFlagOnFreshCall(): void { + // Verifies that flag merging (previously $this->callFlag |= $callFlag in Room) + // is preserved after moving to $callFlag |= $oldCallFlag in RoomService. + $since = new \DateTime(); + $room = $this->createMock(Room::class); + $room->method('getActiveSince')->willReturn(null); + $room->method('getCallFlag')->willReturn(Participant::FLAG_IN_CALL); + $room->method('getId')->willReturn(0); + $room->expects($this->once())->method('setActiveSince')->with($since); + $room->expects($this->once())->method('setCallFlag') + ->with(Participant::FLAG_IN_CALL | Participant::FLAG_WITH_VIDEO); + + $this->service->setActiveSince($room, null, $since, Participant::FLAG_WITH_VIDEO, false); + } }