Skip to content

Commit d2db6ca

Browse files
turt2liveMadLittleMods
authored andcommitted
Switch list quarantined media API to use max to_id instead of current (element-hq#19677)
Following up on element-hq#19558 (comment) Changelog for this PR is intended to overlap with the above PR. `get_current_quarantined_media_stream_id` wasn't being used anywhere else, so we can replace it like we do in this PR. ### Pull Request Checklist <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: - Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from `EventStore` to `EventWorkerStore`.". - Use markdown where necessary, mostly for `code blocks`. - End with either a period (.) or an exclamation mark (!). - Start with a capital letter. - Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry. * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) --------- Co-authored-by: Eric Eastwood <erice@element.io> Co-authored-by: Eric Eastwood <madlittlemods@gmail.com>
1 parent 96257fb commit d2db6ca

3 files changed

Lines changed: 23 additions & 4 deletions

File tree

changelog.d/19677.feature

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Add a ["Listing quarantined media changes" Admin API](https://element-hq.github.io/synapse/latest/admin_api/media_admin_api.html#listing-quarantined-media-changes) for retrieving a paginated record of when media became (un)quarantined.

synapse/rest/admin/media.py

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -249,13 +249,22 @@ async def on_GET(self, request: SynapseRequest) -> tuple[int, JsonDict]:
249249

250250
from_id = parse_integer(request, "from", default=0)
251251
limit = 100 # arbitrary; not enough to cause problems (hopefully)
252-
to_id = await self.store.get_current_quarantined_media_stream_id()
253252

254-
if to_id < from_id:
255-
# The caller is trying to get future data, which isn't possible.
253+
# Validate the `from` token
254+
max_id = await self.store.get_max_allocated_quarantined_media_stream_id()
255+
if from_id > max_id:
256+
# The caller is trying to get future data, which we don't allow because
257+
# we know it's an invalid state that should never happen. We could
258+
# wait until we reach the token but we might as well not waste our
259+
# resources on that which is why `wait_for_quarantined_media_stream_id(...)`
260+
# has assertions around this.
256261
raise SynapseError(
257262
HTTPStatus.BAD_REQUEST,
258-
"The `from` position is ahead of the currently persisted position.",
263+
"The `from` token is considered invalid because it includes stream positions "
264+
"greater than the furthest persisted position across all of the workers."
265+
"This indicates either a Synapse programming error (as we should never hand out "
266+
"invalid future tokens) or a fabricated `from` token. If you've modified the token, "
267+
"you can try paginating from the beginning again.",
259268
errcode=Codes.INVALID_PARAM,
260269
)
261270

@@ -271,6 +280,7 @@ async def on_GET(self, request: SynapseRequest) -> tuple[int, JsonDict]:
271280
errcode=Codes.UNKNOWN,
272281
)
273282

283+
to_id = await self.store.get_current_quarantined_media_stream_id()
274284
changes = await self.store.get_quarantined_media_changes(
275285
from_id=from_id,
276286
to_id=to_id,

synapse/storage/databases/main/room.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1310,6 +1310,14 @@ async def get_current_quarantined_media_stream_id(self) -> int:
13101310
"""
13111311
return self._quarantined_media_changes_id_gen.get_current_token()
13121312

1313+
async def get_max_allocated_quarantined_media_stream_id(self) -> int:
1314+
"""Gets the maximum allocated position of the quarantined media changes stream.
1315+
1316+
Returns:
1317+
int - the maximum stream ID
1318+
"""
1319+
return await self._quarantined_media_changes_id_gen.get_max_allocated_token()
1320+
13131321
async def wait_for_quarantined_media_stream_id(self, target_id: int) -> bool:
13141322
"""Waits until the quarantined media changes stream reaches the given stream ID.
13151323

0 commit comments

Comments
 (0)