Skip to content

MSC3911: AP2 - Database preparations and storage helpers - #91

Merged
jason-famedly merged 1 commit into
msc3911from
jason/msc3911-sql-schema
Aug 22, 2025
Merged

MSC3911: AP2 - Database preparations and storage helpers#91
jason-famedly merged 1 commit into
msc3911from
jason/msc3911-sql-schema

Conversation

@jason-famedly

@jason-famedly jason-famedly commented Aug 19, 2025

Copy link
Copy Markdown
Member

Create a new table media_attachments to contain

server_name media_id restrictions_json
test.com Skckel3slSlkdhg '{"restrictions": {"event_id": "$event_id_hash"}'
test.com jshAy7DjvjsLE83 '{"restrictions": {"profile_user_id": "@jason-famedly:server.com"}'

Add a new column to remote_media_cache for restricted boolean values

Add a new column to local_media_repository for restricted for boolean values

Test series added at tests.storage.test_media

  • Update LocalMedia to have a restricted boolean object
  • Update RemoteMedia to have a restricted boolean object
  • Add new database schema files to add new columns/tables:
    • Update local_media_repository to have new columns from description above
    • Update remote_media_cache to have new columns from description above
    • Create new table media_attachments based on table above
  • Create new MediaRestrictions object to contain an event_id and profile_user_id. Do we want this to be validated at this level or have it be validated before creation of the object?
  • Create new storage helpers in MediaRepositoryStore:
    • Set media restrictions
    • Get media restrictions

For: famedly/product-management#3350

@jason-famedly
jason-famedly force-pushed the jason/msc3911-sql-schema branch from 6a3b632 to 0ccc40c Compare August 19, 2025 14:21
@jason-famedly
jason-famedly changed the base branch from master to msc3911 August 19, 2025 16:13
@jason-famedly
jason-famedly force-pushed the jason/msc3911-sql-schema branch 5 times, most recently from 5275f90 to 184bc6a Compare August 20, 2025 12:47
@jason-famedly
jason-famedly marked this pull request as ready for review August 20, 2025 12:57
@jason-famedly
jason-famedly requested a review from a team as a code owner August 20, 2025 12:57
@jason-famedly

Copy link
Copy Markdown
Member Author

I guess this is ready. I'm sure I over thought it, but if there are any suggestions for additional tests I'd like to hear those thoughts

@jason-famedly
jason-famedly force-pushed the jason/msc3911-sql-schema branch from 6e83960 to c485d30 Compare August 20, 2025 14:57
@jason-famedly jason-famedly changed the title MSC3911: database sql schema MSC3911: AP2 - Database preparations and storage helpers Aug 20, 2025
Comment thread synapse/media/media_repository.py Outdated
@jason-famedly

Copy link
Copy Markdown
Member Author

Since we are now just storing the raw json for the media restrictions, should I keep the media restriction object? I'd like to have that nice type safe way of parsing, but if we are to incorporate unknown restrictions I don't really have a way to model that....unless the object exists but neither of the known restrictions is set?

Comment thread synapse/storage/databases/main/media_repository.py

@itsoyou itsoyou left a comment

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.

LGTM 👍🤸‍♀️

@jason-famedly
jason-famedly force-pushed the jason/msc3911-sql-schema branch 2 times, most recently from 178cd8a to 42306da Compare August 22, 2025 13:39
@jason-famedly
jason-famedly force-pushed the jason/msc3911-sql-schema branch from 42306da to 0120afb Compare August 22, 2025 13:46
@jason-famedly

Copy link
Copy Markdown
Member Author

The test failures appear to be either known flakes or unrelated to anything that would have happened here. Merging

@jason-famedly
jason-famedly merged commit 1742606 into msc3911 Aug 22, 2025
18 of 23 checks passed
@jason-famedly
jason-famedly deleted the jason/msc3911-sql-schema branch August 22, 2025 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants