switch chat media messages to client-side MinIO uploads
Frontend now uploads media directly to MinIO and sends only the resulting object_key; the backend never touches file bytes. Mattermost post bodies carry "<type>:<object_key>" instead of a permanent public URL, and download links are signed fresh on every read (send + list) so they can't outlive their expiry. Also wires up MINIO_SECURE, which settings.py previously never read from env. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
49d6bd6cd2
commit
d85be31e77
8 changed files with 136 additions and 63 deletions
|
|
@ -27,10 +27,15 @@ MATTERMOST_SERVICE_USERNAME=chat-service
|
|||
MATTERMOST_SERVICE_EMAIL=chat-service@local.invalid
|
||||
|
||||
# MinIO object storage
|
||||
# The frontend uploads media directly to MinIO and sends us only the
|
||||
# resulting object key — these credentials are used solely to sign
|
||||
# short-lived download URLs when messages are read back.
|
||||
MINIO_ENDPOINT=localhost:9000
|
||||
MINIO_ACCESS_KEY=minioadmin
|
||||
MINIO_SECRET_KEY=minioadmin
|
||||
MINIO_BUCKET_CHAT=chat
|
||||
MINIO_SECURE=false
|
||||
MINIO_PRESIGN_EXPIRY_SECONDS=3600
|
||||
|
||||
# Chat long-poll tuning
|
||||
CHAT_LONG_POLL_TIMEOUT_SECONDS=25
|
||||
|
|
|
|||
|
|
@ -1,11 +1,28 @@
|
|||
from rest_framework import serializers
|
||||
|
||||
MEDIA_MESSAGE_TYPES = {"image", "video", "voice"}
|
||||
|
||||
|
||||
class SendMessageSerializer(serializers.Serializer):
|
||||
sender_id = serializers.UUIDField()
|
||||
message_type = serializers.ChoiceField(choices=["text", "image", "file"], default="text")
|
||||
message_type = serializers.ChoiceField(
|
||||
choices=["text", "image", "video", "voice"], default="text"
|
||||
)
|
||||
text = serializers.CharField(required=False, allow_null=True, allow_blank=True)
|
||||
file = serializers.FileField(required=False, allow_null=True)
|
||||
object_key = serializers.CharField(required=False, allow_null=True, allow_blank=True)
|
||||
|
||||
def validate(self, attrs):
|
||||
message_type = attrs.get("message_type", "text")
|
||||
if message_type in MEDIA_MESSAGE_TYPES:
|
||||
if not attrs.get("object_key"):
|
||||
raise serializers.ValidationError(
|
||||
{"object_key": "object_key is required for image, video, and voice messages."}
|
||||
)
|
||||
elif not attrs.get("text"):
|
||||
raise serializers.ValidationError(
|
||||
{"text": "text is required for text messages."}
|
||||
)
|
||||
return attrs
|
||||
|
||||
|
||||
class MessageSerializer(serializers.Serializer):
|
||||
|
|
|
|||
|
|
@ -10,6 +10,8 @@ from apps.chat.models import Conversation, MattermostAccountMapping
|
|||
from apps.chat.services.account import AccountService
|
||||
from apps.chat.services.storage import StorageService
|
||||
|
||||
MEDIA_MESSAGE_TYPES = {"image", "video", "voice"}
|
||||
|
||||
|
||||
class MessageService:
|
||||
def __init__(
|
||||
|
|
@ -40,20 +42,20 @@ class MessageService:
|
|||
sender_id: UUID,
|
||||
message_type: str,
|
||||
text: str | None = None,
|
||||
file=None,
|
||||
object_key: str | None = None,
|
||||
) -> dict:
|
||||
if not self._account.validate_user(sender_id):
|
||||
raise ValueError(f"Sender {sender_id} is not valid")
|
||||
|
||||
file_url: str | None = None
|
||||
if file is not None:
|
||||
filename = getattr(file, "name", f"{sender_id}")
|
||||
file_url = self._storage.upload_file(file, filename)
|
||||
|
||||
if message_type == "text":
|
||||
mm_message = text or ""
|
||||
# Media was already uploaded straight to MinIO by the client; the
|
||||
# Mattermost post body only ever carries "<type>:<object_key>", never
|
||||
# a URL, so a stored link can't outlive its presigned expiry.
|
||||
download_url: str | None = None
|
||||
if message_type in MEDIA_MESSAGE_TYPES:
|
||||
mm_message = f"{message_type}:{object_key}"
|
||||
download_url = self._storage.get_download_url(object_key)
|
||||
else:
|
||||
mm_message = file_url or text or ""
|
||||
mm_message = text or ""
|
||||
|
||||
conversation = Conversation.objects.get(id=conversation_id)
|
||||
post_id = self._mm.post_message(conversation.mattermost_channel_id, mm_message)
|
||||
|
|
@ -63,7 +65,7 @@ class MessageService:
|
|||
post_id=post_id,
|
||||
sender_id=sender_id,
|
||||
message_type=message_type,
|
||||
payload={"text": text, "file_url": file_url},
|
||||
payload={"text": text, "object_key": object_key, "url": download_url},
|
||||
)
|
||||
for publisher in self._publishers:
|
||||
publisher.publish(event)
|
||||
|
|
@ -73,7 +75,7 @@ class MessageService:
|
|||
"sender_id": sender_id,
|
||||
"message_type": message_type,
|
||||
"text": text,
|
||||
"url": file_url,
|
||||
"url": download_url,
|
||||
"created_at": None,
|
||||
}
|
||||
|
||||
|
|
@ -102,23 +104,33 @@ class MessageService:
|
|||
|
||||
return [self._normalize_post(p, mappings) for p in raw_posts]
|
||||
|
||||
@staticmethod
|
||||
def _normalize_post(post: dict, mappings: dict) -> dict:
|
||||
def _normalize_post(self, post: dict, mappings: dict) -> dict:
|
||||
mm_uid = post.get("user_id")
|
||||
sender_id = mappings.get(mm_uid)
|
||||
msg = post.get("message", "")
|
||||
is_url = msg.startswith("http")
|
||||
created_ms = post.get("create_at")
|
||||
created_at = (
|
||||
datetime.fromtimestamp(created_ms / 1000, tz=timezone.utc)
|
||||
if created_ms
|
||||
else None
|
||||
)
|
||||
|
||||
message_type, sep, object_key = msg.partition(":")
|
||||
if sep and message_type in MEDIA_MESSAGE_TYPES and object_key:
|
||||
return {
|
||||
"post_id": post.get("id"),
|
||||
"sender_id": sender_id,
|
||||
"message_type": "file" if is_url else "text",
|
||||
"text": None if is_url else msg,
|
||||
"url": msg if is_url else None,
|
||||
"message_type": message_type,
|
||||
"text": None,
|
||||
"url": self._storage.get_download_url(object_key),
|
||||
"created_at": created_at,
|
||||
}
|
||||
|
||||
return {
|
||||
"post_id": post.get("id"),
|
||||
"sender_id": sender_id,
|
||||
"message_type": "text",
|
||||
"text": msg,
|
||||
"url": None,
|
||||
"created_at": created_at,
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,12 +1,17 @@
|
|||
import io
|
||||
import mimetypes
|
||||
from uuid import uuid4
|
||||
from datetime import timedelta
|
||||
|
||||
from django.conf import settings
|
||||
from minio import Minio
|
||||
|
||||
|
||||
class StorageService:
|
||||
"""
|
||||
Thin adapter around MinIO for the object-key-in-message-body flow: the
|
||||
frontend uploads media directly to MinIO and only ever sends us the
|
||||
resulting object key, so this service's job is limited to turning that
|
||||
key back into a short-lived download URL on read.
|
||||
"""
|
||||
|
||||
def __init__(self):
|
||||
endpoint = getattr(settings, "MINIO_ENDPOINT", None)
|
||||
if not endpoint:
|
||||
|
|
@ -20,23 +25,21 @@ class StorageService:
|
|||
secure=secure,
|
||||
)
|
||||
self._bucket = getattr(settings, "MINIO_BUCKET_CHAT", "chat")
|
||||
scheme = "https" if secure else "http"
|
||||
self._base_url = f"{scheme}://{endpoint}"
|
||||
|
||||
def upload_file(self, file, filename: str) -> str:
|
||||
data = file.read() if hasattr(file, "read") else file
|
||||
size = len(data)
|
||||
|
||||
content_type, _ = mimetypes.guess_type(filename)
|
||||
if not content_type:
|
||||
content_type = "application/octet-stream"
|
||||
|
||||
object_name = f"{uuid4().hex}/{filename}"
|
||||
self._client.put_object(
|
||||
self._bucket,
|
||||
object_name,
|
||||
io.BytesIO(data),
|
||||
size,
|
||||
content_type=content_type,
|
||||
self._expiry = timedelta(
|
||||
seconds=getattr(settings, "MINIO_PRESIGN_EXPIRY_SECONDS", 3600)
|
||||
)
|
||||
return f"{self._base_url}/{self._bucket}/{object_name}"
|
||||
|
||||
def get_download_url(self, object_key: str) -> str | None:
|
||||
"""
|
||||
Sign a time-limited download URL for an object the client already
|
||||
uploaded. Generated fresh on every call rather than cached/stored,
|
||||
since a stored link would eventually expire while still displayed.
|
||||
Returns None on failure so one bad key can't fail an entire message
|
||||
list — callers should treat that as "link unavailable", not an error.
|
||||
"""
|
||||
try:
|
||||
return self._client.presigned_get_object(
|
||||
self._bucket, object_key, expires=self._expiry
|
||||
)
|
||||
except Exception:
|
||||
return None
|
||||
|
|
|
|||
|
|
@ -7,14 +7,13 @@ from apps.chat.models import Conversation, MattermostAccountMapping
|
|||
from apps.chat.services.message import MessageService
|
||||
|
||||
|
||||
def _make_service(*, mm_post_id="post-1", storage_url=None, publishers=None):
|
||||
def _make_service(*, mm_post_id="post-1", download_url=None, publishers=None):
|
||||
"""Return a MessageService with all external deps mocked."""
|
||||
mm_client = Mock()
|
||||
mm_client.post_message.return_value = mm_post_id
|
||||
|
||||
storage = Mock()
|
||||
if storage_url:
|
||||
storage.upload_file.return_value = storage_url
|
||||
storage.get_download_url.return_value = download_url
|
||||
|
||||
return (
|
||||
MessageService(
|
||||
|
|
@ -33,11 +32,12 @@ def test_send_text_message():
|
|||
sender_id = uuid.uuid4()
|
||||
|
||||
ws_publisher = Mock()
|
||||
svc, mm_client, _ = _make_service(mm_post_id="post-abc", publishers=[ws_publisher])
|
||||
svc, mm_client, storage = _make_service(mm_post_id="post-abc", publishers=[ws_publisher])
|
||||
|
||||
result = svc.send(conv.id, sender_id, "text", text="Hello world")
|
||||
|
||||
mm_client.post_message.assert_called_once_with("ch-send-text", "Hello world")
|
||||
storage.get_download_url.assert_not_called()
|
||||
ws_publisher.publish.assert_called_once()
|
||||
|
||||
event = ws_publisher.publish.call_args[0][0]
|
||||
|
|
@ -47,6 +47,7 @@ def test_send_text_message():
|
|||
|
||||
assert result["post_id"] == "post-abc"
|
||||
assert result["text"] == "Hello world"
|
||||
assert result["url"] is None
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
|
|
@ -54,17 +55,16 @@ def test_send_image_message():
|
|||
conv = Conversation.objects.create(mattermost_channel_id="ch-send-image")
|
||||
sender_id = uuid.uuid4()
|
||||
|
||||
file_url = "http://minio.local/chat/abc123/photo.jpg"
|
||||
svc, mm_client, storage = _make_service(storage_url=file_url, publishers=[])
|
||||
object_key = "images/abc123/photo.jpg"
|
||||
download_url = "http://minio.local/chat/images/abc123/photo.jpg?X-Amz-Signature=..."
|
||||
svc, mm_client, storage = _make_service(download_url=download_url, publishers=[])
|
||||
|
||||
fake_file = Mock()
|
||||
fake_file.name = "photo.jpg"
|
||||
result = svc.send(conv.id, sender_id, "image", object_key=object_key)
|
||||
|
||||
result = svc.send(conv.id, sender_id, "image", file=fake_file)
|
||||
|
||||
storage.upload_file.assert_called_once_with(fake_file, "photo.jpg")
|
||||
mm_client.post_message.assert_called_once_with("ch-send-image", file_url)
|
||||
assert result["url"] == file_url
|
||||
storage.get_download_url.assert_called_once_with(object_key)
|
||||
mm_client.post_message.assert_called_once_with("ch-send-image", f"image:{object_key}")
|
||||
assert result["url"] == download_url
|
||||
assert result["message_type"] == "image"
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
|
|
@ -77,6 +77,7 @@ def test_list_messages_normalized():
|
|||
user_id=sender_uuid, mattermost_user_id=mm_user_id
|
||||
)
|
||||
|
||||
object_key = "voice/def456/clip.ogg"
|
||||
raw_posts = [
|
||||
{
|
||||
"id": "p1",
|
||||
|
|
@ -87,14 +88,15 @@ def test_list_messages_normalized():
|
|||
{
|
||||
"id": "p2",
|
||||
"user_id": mm_user_id,
|
||||
"message": "http://minio.local/chat/file.pdf",
|
||||
"message": f"voice:{object_key}",
|
||||
"create_at": 1700000001000,
|
||||
},
|
||||
]
|
||||
|
||||
download_url = "http://minio.local/chat/voice/def456/clip.ogg?X-Amz-Signature=..."
|
||||
mm_client = Mock()
|
||||
mm_client.get_posts.return_value = raw_posts
|
||||
svc, _, _ = _make_service(publishers=[])
|
||||
svc, _, storage = _make_service(download_url=download_url, publishers=[])
|
||||
svc._mm = mm_client # inject after construction to keep _make_service simple
|
||||
|
||||
messages = svc.list_messages(conv.id, page=0, per_page=20)
|
||||
|
|
@ -108,9 +110,39 @@ def test_list_messages_normalized():
|
|||
assert text_msg["text"] == "First message"
|
||||
assert text_msg["url"] is None
|
||||
|
||||
file_msg = messages[1]
|
||||
assert file_msg["post_id"] == "p2"
|
||||
assert file_msg["sender_id"] == sender_uuid
|
||||
assert file_msg["message_type"] == "file"
|
||||
assert file_msg["url"] == "http://minio.local/chat/file.pdf"
|
||||
assert file_msg["text"] is None
|
||||
voice_msg = messages[1]
|
||||
assert voice_msg["post_id"] == "p2"
|
||||
assert voice_msg["sender_id"] == sender_uuid
|
||||
assert voice_msg["message_type"] == "voice"
|
||||
assert voice_msg["url"] == download_url
|
||||
assert voice_msg["text"] is None
|
||||
|
||||
storage.get_download_url.assert_called_once_with(object_key)
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_list_messages_treats_colon_in_plain_text_as_text():
|
||||
"""A plain-text message that happens to contain a colon but doesn't use
|
||||
a recognized media-type prefix must not be mistaken for a media message.
|
||||
"""
|
||||
conv = Conversation.objects.create(mattermost_channel_id="ch-list-colon")
|
||||
|
||||
raw_posts = [
|
||||
{
|
||||
"id": "p1",
|
||||
"user_id": None,
|
||||
"message": "Meeting at 10:30",
|
||||
"create_at": 1700000000000,
|
||||
},
|
||||
]
|
||||
|
||||
mm_client = Mock()
|
||||
mm_client.get_posts.return_value = raw_posts
|
||||
svc, _, storage = _make_service(publishers=[])
|
||||
svc._mm = mm_client
|
||||
|
||||
messages = svc.list_messages(conv.id, page=0, per_page=20)
|
||||
|
||||
assert messages[0]["message_type"] == "text"
|
||||
assert messages[0]["text"] == "Meeting at 10:30"
|
||||
storage.get_download_url.assert_not_called()
|
||||
|
|
|
|||
|
|
@ -27,7 +27,7 @@ class MessageView(APIView):
|
|||
sender_id=data["sender_id"],
|
||||
message_type=data["message_type"],
|
||||
text=data.get("text"),
|
||||
file=data.get("file"),
|
||||
object_key=data.get("object_key"),
|
||||
)
|
||||
|
||||
return Response(MessageSerializer(result).data, status=status.HTTP_201_CREATED)
|
||||
|
|
|
|||
|
|
@ -24,3 +24,5 @@ MINIO_ENDPOINT=localhost:9000
|
|||
MINIO_ACCESS_KEY=minioadmin
|
||||
MINIO_SECRET_KEY=minioadmin
|
||||
MINIO_BUCKET_CHAT=chat
|
||||
MINIO_SECURE=false
|
||||
MINIO_PRESIGN_EXPIRY_SECONDS=3600
|
||||
|
|
|
|||
|
|
@ -288,5 +288,7 @@ MINIO_ENDPOINT = config('MINIO_ENDPOINT', default=None)
|
|||
MINIO_ACCESS_KEY = config('MINIO_ACCESS_KEY', default=None)
|
||||
MINIO_SECRET_KEY = config('MINIO_SECRET_KEY', default=None)
|
||||
MINIO_BUCKET_CHAT = config('MINIO_BUCKET_CHAT', default='chat')
|
||||
MINIO_SECURE = config('MINIO_SECURE', default=False, cast=bool)
|
||||
MINIO_PRESIGN_EXPIRY_SECONDS = config('MINIO_PRESIGN_EXPIRY_SECONDS', default=3600, cast=int)
|
||||
GDAL_LIBRARY_PATH = config('GDAL_LIBRARY_PATH', default=None)
|
||||
GEOS_LIBRARY_PATH = config('GEOS_LIBRARY_PATH', default=None)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue