Skip to content

refactor(room): Move validateTimer() out of getLobbyState()/getLobbyTimer() getters#17810

Open
miaulalala wants to merge 1 commit intomainfrom
refactor/noid/move-room-validateLobbyTimer
Open

refactor(room): Move validateTimer() out of getLobbyState()/getLobbyTimer() getters#17810
miaulalala wants to merge 1 commit intomainfrom
refactor/noid/move-room-validateLobbyTimer

Conversation

@miaulalala
Copy link
Copy Markdown
Contributor

@miaulalala miaulalala commented Apr 25, 2026

getLobbyState() and getLobbyTimer() previously triggered a database write as a side effect
when called β€” if the lobby timer had expired, they would silently reset the lobby state in the
DB and fire events. This is incompatible with Entity's assumption that getters are pure.

  • Make getLobbyState() and getLobbyTimer() pure getters (remove bool $validateTime
    parameter)
  • Extract the timer check into RoomService::validateLobbyTimer(Room $room) using proper
    constructor DI
  • Call it explicitly at the relevant entry points: RoomFormatter::formatRoomV4(),
    InjectionMiddleware::checkLobbyState(), and
    RoomPropertiesHelper::getPropertiesForSignaling()
  • As a bonus, ITimeFactory is no longer needed in Room's constructor

The two callers that previously passed getLobbyState(false) to suppress the side effect
(RoomService::setLobby() and updateLocalRoomInfo()) simply use the pure getter now.

AI-Assisted-By: Claude Sonnet 4.6 noreply@anthropic.com

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

πŸ› οΈ API Checklist

🏁 Checklist

  • ⛑️ Tests (unit and/or integration) are included or not possible
  • πŸ“˜ API documentation in docs/ has been updated or is not required
  • πŸ”– Capability is added or not needed

@miaulalala miaulalala self-assigned this Apr 25, 2026
@miaulalala miaulalala added feature: api πŸ› οΈ OCS API for conversations, chats and participants feature: meetings πŸ“… Covering the webinary usecase incl. Lobby labels Apr 25, 2026
Comment thread lib/Room.php Outdated
@miaulalala miaulalala force-pushed the refactor/noid/move-room-validateLobbyTimer branch from cbdac8f to d094467 Compare April 25, 2026 12:57
…tLobbyTimer()

Part of the incremental migration of `Room` to extend
`OCP\AppFramework\Db\Entity` (Phase 1b).

`getLobbyState()` and `getLobbyTimer()` previously triggered a database
write as a side effect when called β€” if the lobby timer had expired, they
would silently reset the lobby state in the DB and fire events. This is
incompatible with Entity's assumption that getters are pure.

- Make `getLobbyState()` and `getLobbyTimer()` pure getters (remove `bool $validateTime` parameter)
- Extract the timer check into `RoomService::validateLobbyTimer(Room $room)` using proper constructor DI
- Call it explicitly at the relevant entry points: `RoomFormatter::formatRoomV4()`, `InjectionMiddleware::checkLobbyState()`, and `RoomPropertiesHelper::getPropertiesForSignaling()`
- As a bonus, `ITimeFactory` is no longer needed in `Room`'s constructor

The two callers that previously passed `getLobbyState(false)` to suppress
the side effect (`RoomService::setLobby()` and `updateLocalRoomInfo()`)
simply use the pure getter now.

AI-Assisted-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Anna Larch <anna@nextcloud.com>
@miaulalala miaulalala force-pushed the refactor/noid/move-room-validateLobbyTimer branch from d094467 to 20e762a Compare April 25, 2026 13:01
Comment thread tests/php/RoomTest.php
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We don't need this file πŸ™ˆ

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I wanted it to verify behaviour during the refactor. It will be removed at the end.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: api πŸ› οΈ OCS API for conversations, chats and participants feature: meetings πŸ“… Covering the webinary usecase incl. Lobby

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants