Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Commit

Permalink
Add a MXCUri class to make working with mxc uri's easier. (#13162)
Browse files Browse the repository at this point in the history
  • Loading branch information
anoadragon453 authored Sep 15, 2022
1 parent 957e3d7 commit 918c74b
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 74 deletions.
1 change: 1 addition & 0 deletions changelog.d/13162.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Bump the minimum dependency of `matrix_common` to 1.3.0 to make use of the `MXCUri` class. Use `MXCUri` to simplify media retention test code.
10 changes: 5 additions & 5 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ typing-extensions = ">=3.10.0.1"
cryptography = ">=3.4.7"
# ijson 3.1.4 fixes a bug with "." in property names
ijson = ">=3.1.4"
matrix-common = "^1.2.1"
matrix-common = "^1.3.0"
# We need packaging.requirements.Requirement, added in 16.1.
packaging = ">=16.1"
# At the time of writing, we only use functions from the version `importlib.metadata`
Expand Down
6 changes: 4 additions & 2 deletions synapse/rest/media/v1/media_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
from io import BytesIO
from typing import IO, TYPE_CHECKING, Dict, List, Optional, Set, Tuple

from matrix_common.types.mxc_uri import MXCUri

import twisted.internet.error
import twisted.web.http
from twisted.internet.defer import Deferred
Expand Down Expand Up @@ -186,7 +188,7 @@ async def create_content(
content: IO,
content_length: int,
auth_user: UserID,
) -> str:
) -> MXCUri:
"""Store uploaded content for a local user and return the mxc URL
Args:
Expand Down Expand Up @@ -219,7 +221,7 @@ async def create_content(

await self._generate_thumbnails(None, media_id, media_id, media_type)

return "mxc://%s/%s" % (self.server_name, media_id)
return MXCUri(self.server_name, media_id)

async def get_local_media(
self, request: SynapseRequest, media_id: str, name: Optional[str]
Expand Down
6 changes: 4 additions & 2 deletions synapse/rest/media/v1/upload_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ async def _async_render_POST(self, request: SynapseRequest) -> None:
# the default 404, as that would just be confusing.
raise SynapseError(400, "Bad content")

logger.info("Uploaded content with URI %r", content_uri)
logger.info("Uploaded content with URI '%s'", content_uri)

respond_with_json(request, 200, {"content_uri": content_uri}, send_cors=True)
respond_with_json(
request, 200, {"content_uri": str(content_uri)}, send_cors=True
)
102 changes: 38 additions & 64 deletions tests/rest/media/test_media_retention.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@
# limitations under the License.

import io
from typing import Iterable, Optional, Tuple
from typing import Iterable, Optional

from matrix_common.types.mxc_uri import MXCUri

from twisted.test.proto_helpers import MemoryReactor

Expand Down Expand Up @@ -63,9 +65,9 @@ def _create_media_and_set_attributes(
last_accessed_ms: Optional[int],
is_quarantined: Optional[bool] = False,
is_protected: Optional[bool] = False,
) -> str:
) -> MXCUri:
# "Upload" some media to the local media store
mxc_uri = self.get_success(
mxc_uri: MXCUri = self.get_success(
media_repository.create_content(
media_type="text/plain",
upload_name=None,
Expand All @@ -75,13 +77,11 @@ def _create_media_and_set_attributes(
)
)

media_id = mxc_uri.split("/")[-1]

# Set the last recently accessed time for this media
if last_accessed_ms is not None:
self.get_success(
self.store.update_cached_last_access_time(
local_media=(media_id,),
local_media=(mxc_uri.media_id,),
remote_media=(),
time_ms=last_accessed_ms,
)
Expand All @@ -92,7 +92,7 @@ def _create_media_and_set_attributes(
self.get_success(
self.store.quarantine_media_by_id(
server_name=self.hs.config.server.server_name,
media_id=media_id,
media_id=mxc_uri.media_id,
quarantined_by="@theadmin:test",
)
)
Expand All @@ -101,18 +101,18 @@ def _create_media_and_set_attributes(
# Mark this media as protected from quarantine
self.get_success(
self.store.mark_local_media_as_safe(
media_id=media_id,
media_id=mxc_uri.media_id,
safe=True,
)
)

return media_id
return mxc_uri

def _cache_remote_media_and_set_attributes(
media_id: str,
last_accessed_ms: Optional[int],
is_quarantined: Optional[bool] = False,
) -> str:
) -> MXCUri:
# Pretend to cache some remote media
self.get_success(
self.store.store_cached_remote_media(
Expand Down Expand Up @@ -146,7 +146,7 @@ def _cache_remote_media_and_set_attributes(
)
)

return media_id
return MXCUri(self.remote_server_name, media_id)

# Start with the local media store
self.local_recently_accessed_media = _create_media_and_set_attributes(
Expand Down Expand Up @@ -214,28 +214,16 @@ def test_local_media_retention(self) -> None:
# Remote media should be unaffected.
self._assert_if_mxc_uris_purged(
purged=[
(
self.hs.config.server.server_name,
self.local_not_recently_accessed_media,
),
(self.hs.config.server.server_name, self.local_never_accessed_media),
self.local_not_recently_accessed_media,
self.local_never_accessed_media,
],
not_purged=[
(self.hs.config.server.server_name, self.local_recently_accessed_media),
(
self.hs.config.server.server_name,
self.local_not_recently_accessed_quarantined_media,
),
(
self.hs.config.server.server_name,
self.local_not_recently_accessed_protected_media,
),
(self.remote_server_name, self.remote_recently_accessed_media),
(self.remote_server_name, self.remote_not_recently_accessed_media),
(
self.remote_server_name,
self.remote_not_recently_accessed_quarantined_media,
),
self.local_recently_accessed_media,
self.local_not_recently_accessed_quarantined_media,
self.local_not_recently_accessed_protected_media,
self.remote_recently_accessed_media,
self.remote_not_recently_accessed_media,
self.remote_not_recently_accessed_quarantined_media,
],
)

Expand All @@ -261,49 +249,35 @@ def test_remote_media_cache_retention(self) -> None:
# Remote media accessed <30 days ago should still exist.
self._assert_if_mxc_uris_purged(
purged=[
(self.remote_server_name, self.remote_not_recently_accessed_media),
self.remote_not_recently_accessed_media,
],
not_purged=[
(self.remote_server_name, self.remote_recently_accessed_media),
(self.hs.config.server.server_name, self.local_recently_accessed_media),
(
self.hs.config.server.server_name,
self.local_not_recently_accessed_media,
),
(
self.hs.config.server.server_name,
self.local_not_recently_accessed_quarantined_media,
),
(
self.hs.config.server.server_name,
self.local_not_recently_accessed_protected_media,
),
(
self.remote_server_name,
self.remote_not_recently_accessed_quarantined_media,
),
(self.hs.config.server.server_name, self.local_never_accessed_media),
self.remote_recently_accessed_media,
self.local_recently_accessed_media,
self.local_not_recently_accessed_media,
self.local_not_recently_accessed_quarantined_media,
self.local_not_recently_accessed_protected_media,
self.remote_not_recently_accessed_quarantined_media,
self.local_never_accessed_media,
],
)

def _assert_if_mxc_uris_purged(
self, purged: Iterable[Tuple[str, str]], not_purged: Iterable[Tuple[str, str]]
self, purged: Iterable[MXCUri], not_purged: Iterable[MXCUri]
) -> None:
def _assert_mxc_uri_purge_state(
server_name: str, media_id: str, expect_purged: bool
) -> None:
def _assert_mxc_uri_purge_state(mxc_uri: MXCUri, expect_purged: bool) -> None:
"""Given an MXC URI, assert whether it has been purged or not."""
if server_name == self.hs.config.server.server_name:
if mxc_uri.server_name == self.hs.config.server.server_name:
found_media_dict = self.get_success(
self.store.get_local_media(media_id)
self.store.get_local_media(mxc_uri.media_id)
)
else:
found_media_dict = self.get_success(
self.store.get_cached_remote_media(server_name, media_id)
self.store.get_cached_remote_media(
mxc_uri.server_name, mxc_uri.media_id
)
)

mxc_uri = f"mxc://{server_name}/{media_id}"

if expect_purged:
self.assertIsNone(
found_media_dict, msg=f"{mxc_uri} unexpectedly not purged"
Expand All @@ -315,7 +289,7 @@ def _assert_mxc_uri_purge_state(
)

# Assert that the given MXC URIs have either been correctly purged or not.
for server_name, media_id in purged:
_assert_mxc_uri_purge_state(server_name, media_id, expect_purged=True)
for server_name, media_id in not_purged:
_assert_mxc_uri_purge_state(server_name, media_id, expect_purged=False)
for mxc_uri in purged:
_assert_mxc_uri_purge_state(mxc_uri, expect_purged=True)
for mxc_uri in not_purged:
_assert_mxc_uri_purge_state(mxc_uri, expect_purged=False)

0 comments on commit 918c74b

Please sign in to comment.