♻️(convert) reuse existing convert yprovider endpoint for content API
reuse convert service instead of renaming it in content
This commit is contained in:
committed by
Manuel Raynaud
parent
8a8a1460e5
commit
ede0a77665
@@ -15,9 +15,9 @@ from rest_framework import serializers
|
||||
|
||||
from core import choices, enums, models, utils, validators
|
||||
from core.services.ai_services import AI_ACTIONS
|
||||
from core.services.yprovider_services import (
|
||||
from core.services.converter_services import (
|
||||
ConversionError,
|
||||
YProviderAPI,
|
||||
YdocConverter,
|
||||
)
|
||||
|
||||
|
||||
@@ -451,7 +451,7 @@ class ServerCreateDocumentSerializer(serializers.Serializer):
|
||||
language = user.language or language
|
||||
|
||||
try:
|
||||
document_content = YProviderAPI().convert(validated_data["content"])
|
||||
document_content = YdocConverter().convert(validated_data["content"])
|
||||
except ConversionError as err:
|
||||
raise serializers.ValidationError(
|
||||
{"content": ["Could not convert content"]}
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
"""API endpoints"""
|
||||
# pylint: disable=too-many-lines
|
||||
|
||||
import base64
|
||||
import json
|
||||
import logging
|
||||
import uuid
|
||||
@@ -37,14 +38,14 @@ from rest_framework.permissions import AllowAny
|
||||
from core import authentication, choices, enums, models
|
||||
from core.services.ai_services import AIService
|
||||
from core.services.collaboration_services import CollaborationService
|
||||
from core.services.yprovider_services import (
|
||||
from core.services.converter_services import (
|
||||
ServiceUnavailableError as YProviderServiceUnavailableError,
|
||||
)
|
||||
from core.services.yprovider_services import (
|
||||
from core.services.converter_services import (
|
||||
ValidationError as YProviderValidationError,
|
||||
)
|
||||
from core.services.yprovider_services import (
|
||||
YProviderAPI,
|
||||
from core.services.converter_services import (
|
||||
YdocConverter,
|
||||
)
|
||||
from core.tasks.mail import send_ask_for_access_mail
|
||||
from core.utils import extract_attachments, filter_descendants
|
||||
@@ -1534,9 +1535,17 @@ class DocumentViewSet(
|
||||
if base64_content is not None:
|
||||
# Convert using the y-provider service
|
||||
try:
|
||||
yprovider = YProviderAPI()
|
||||
result = yprovider.content(base64_content, content_format)
|
||||
content = result["content"]
|
||||
yprovider = YdocConverter()
|
||||
result = yprovider.convert(
|
||||
base64.b64decode(base64_content),
|
||||
"application/vnd.yjs.doc",
|
||||
{
|
||||
"markdown": "text/markdown",
|
||||
"html": "text/html",
|
||||
"json": "application/json",
|
||||
}[content_format],
|
||||
)
|
||||
content = result
|
||||
except YProviderValidationError as e:
|
||||
return drf_response.Response(
|
||||
{"error": str(e)}, status=status.HTTP_400_BAD_REQUEST
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
"""Y-Provider API services."""
|
||||
|
||||
import json
|
||||
from base64 import b64encode
|
||||
|
||||
from django.conf import settings
|
||||
@@ -20,8 +19,8 @@ class ServiceUnavailableError(ConversionError):
|
||||
"""Raised when the conversion service is unavailable."""
|
||||
|
||||
|
||||
class YProviderAPI:
|
||||
"""Service class for Y-Provider API operations."""
|
||||
class YdocConverter:
|
||||
"""Service class for conversion-related operations."""
|
||||
|
||||
@property
|
||||
def auth_header(self):
|
||||
@@ -29,7 +28,7 @@ class YProviderAPI:
|
||||
# Note: Yprovider microservice accepts only raw token, which is not recommended
|
||||
return f"Bearer {settings.Y_PROVIDER_API_KEY}"
|
||||
|
||||
def _request(self, url, data, content_type):
|
||||
def _request(self, url, data, content_type, accept):
|
||||
"""Make a request to the Y-Provider API."""
|
||||
response = requests.post(
|
||||
url,
|
||||
@@ -37,6 +36,7 @@ class YProviderAPI:
|
||||
headers={
|
||||
"Authorization": self.auth_header,
|
||||
"Content-Type": content_type,
|
||||
"Accept": accept,
|
||||
},
|
||||
timeout=settings.CONVERSION_API_TIMEOUT,
|
||||
verify=settings.CONVERSION_API_SECURE,
|
||||
@@ -44,7 +44,9 @@ class YProviderAPI:
|
||||
response.raise_for_status()
|
||||
return response
|
||||
|
||||
def convert(self, text):
|
||||
def convert(
|
||||
self, text, content_type="text/markdown", accept="application/vnd.yjs.doc"
|
||||
):
|
||||
"""Convert a Markdown text into our internal format using an external microservice."""
|
||||
|
||||
if not text:
|
||||
@@ -54,27 +56,17 @@ class YProviderAPI:
|
||||
response = self._request(
|
||||
f"{settings.Y_PROVIDER_API_BASE_URL}{settings.CONVERSION_API_ENDPOINT}/",
|
||||
text,
|
||||
"text/markdown",
|
||||
content_type,
|
||||
accept,
|
||||
)
|
||||
return b64encode(response.content).decode("utf-8")
|
||||
if accept == "application/vnd.yjs.doc":
|
||||
return b64encode(response.content).decode("utf-8")
|
||||
if accept in {"text/markdown", "text/html"}:
|
||||
return response.text
|
||||
if accept == "application/json":
|
||||
return response.json()
|
||||
raise ValidationError("Unsupported format")
|
||||
except requests.RequestException as err:
|
||||
raise ServiceUnavailableError(
|
||||
"Failed to connect to backend service",
|
||||
) from err
|
||||
|
||||
def content(self, base64_content, format_type):
|
||||
"""Convert base64 Yjs content to different formats using the y-provider service."""
|
||||
|
||||
if not base64_content:
|
||||
raise ValidationError("Input content cannot be empty")
|
||||
|
||||
data = json.dumps({"content": base64_content, "format": format_type})
|
||||
try:
|
||||
response = self._request(
|
||||
f"{settings.Y_PROVIDER_API_BASE_URL}content/", data, "application/json"
|
||||
)
|
||||
return response.json()
|
||||
except requests.RequestException as err:
|
||||
raise ServiceUnavailableError(
|
||||
"Failed to connect to backend service",
|
||||
"Failed to connect to conversion service",
|
||||
) from err
|
||||
@@ -2,6 +2,7 @@
|
||||
Tests for Documents API endpoint in impress's core app: content
|
||||
"""
|
||||
|
||||
import base64
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
@@ -21,11 +22,11 @@ pytestmark = pytest.mark.django_db
|
||||
("public", "editor"),
|
||||
],
|
||||
)
|
||||
@patch("core.services.yprovider_services.YProviderAPI.content")
|
||||
@patch("core.services.converter_services.YdocConverter.convert")
|
||||
def test_api_documents_content_public(mock_content, reach, role):
|
||||
"""Anonymous users should be allowed to access content of public documents."""
|
||||
document = factories.DocumentFactory(link_reach=reach, link_role=role)
|
||||
mock_content.return_value = {"content": {"some": "data"}}
|
||||
mock_content.return_value = {"some": "data"}
|
||||
|
||||
response = APIClient().get(f"/api/v1.0/documents/{document.id!s}/content/")
|
||||
|
||||
@@ -34,7 +35,11 @@ def test_api_documents_content_public(mock_content, reach, role):
|
||||
assert data["id"] == str(document.id)
|
||||
assert data["title"] == document.title
|
||||
assert data["content"] == {"some": "data"}
|
||||
mock_content.assert_called_once_with(document.content, "json")
|
||||
mock_content.assert_called_once_with(
|
||||
base64.b64decode(document.content),
|
||||
"application/vnd.yjs.doc",
|
||||
"application/json",
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
@@ -52,12 +57,12 @@ def test_api_documents_content_public(mock_content, reach, role):
|
||||
("authenticated", "editor", None),
|
||||
],
|
||||
)
|
||||
@patch("core.services.yprovider_services.YProviderAPI.content")
|
||||
@patch("core.services.converter_services.YdocConverter.convert")
|
||||
def test_api_documents_content_not_public(mock_content, reach, doc_role, user_role):
|
||||
"""Authenticated users need access to get non-public document content."""
|
||||
user = factories.UserFactory()
|
||||
document = factories.DocumentFactory(link_reach=reach, link_role=doc_role)
|
||||
mock_content.return_value = {"content": {"some": "data"}}
|
||||
mock_content.return_value = {"some": "data"}
|
||||
|
||||
# First anonymous request should fail
|
||||
client = APIClient()
|
||||
@@ -87,18 +92,26 @@ def test_api_documents_content_not_public(mock_content, reach, doc_role, user_ro
|
||||
assert data["id"] == str(document.id)
|
||||
assert data["title"] == document.title
|
||||
assert data["content"] == {"some": "data"}
|
||||
mock_content.assert_called_once_with(document.content, "json")
|
||||
mock_content.assert_called_once_with(
|
||||
base64.b64decode(document.content),
|
||||
"application/vnd.yjs.doc",
|
||||
"application/json",
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"content_format",
|
||||
["markdown", "html", "json"],
|
||||
"content_format, accept",
|
||||
[
|
||||
("markdown", "text/markdown"),
|
||||
("html", "text/html"),
|
||||
("json", "application/json"),
|
||||
],
|
||||
)
|
||||
@patch("core.services.yprovider_services.YProviderAPI.content")
|
||||
def test_api_documents_content_format(mock_content, content_format):
|
||||
@patch("core.services.converter_services.YdocConverter.convert")
|
||||
def test_api_documents_content_format(mock_content, content_format, accept):
|
||||
"""Test that the content endpoint returns a specific format."""
|
||||
document = factories.DocumentFactory(link_reach="public")
|
||||
mock_content.return_value = {"content": "whatever"}
|
||||
mock_content.return_value = {"some": "data"}
|
||||
|
||||
response = APIClient().get(
|
||||
f"/api/v1.0/documents/{document.id!s}/content/?content_format={content_format}"
|
||||
@@ -108,11 +121,13 @@ def test_api_documents_content_format(mock_content, content_format):
|
||||
data = response.json()
|
||||
assert data["id"] == str(document.id)
|
||||
assert data["title"] == document.title
|
||||
assert data["content"] == "whatever"
|
||||
mock_content.assert_called_once_with(document.content, content_format)
|
||||
assert data["content"] == {"some": "data"}
|
||||
mock_content.assert_called_once_with(
|
||||
base64.b64decode(document.content), "application/vnd.yjs.doc", accept
|
||||
)
|
||||
|
||||
|
||||
@patch("core.services.yprovider_services.YProviderAPI._request")
|
||||
@patch("core.services.converter_services.YdocConverter._request")
|
||||
def test_api_documents_content_invalid_format(mock_request):
|
||||
"""Test that the content endpoint rejects invalid formats."""
|
||||
document = factories.DocumentFactory(link_reach="public")
|
||||
@@ -124,7 +139,7 @@ def test_api_documents_content_invalid_format(mock_request):
|
||||
mock_request.assert_not_called()
|
||||
|
||||
|
||||
@patch("core.services.yprovider_services.YProviderAPI._request")
|
||||
@patch("core.services.converter_services.YdocConverter._request")
|
||||
def test_api_documents_content_yservice_error(mock_request):
|
||||
"""Test that service errors are handled properly."""
|
||||
document = factories.DocumentFactory(link_reach="public")
|
||||
@@ -135,7 +150,7 @@ def test_api_documents_content_yservice_error(mock_request):
|
||||
assert response.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR
|
||||
|
||||
|
||||
@patch("core.services.yprovider_services.YProviderAPI._request")
|
||||
@patch("core.services.converter_services.YdocConverter._request")
|
||||
def test_api_documents_content_nonexistent_document(mock_request):
|
||||
"""Test that accessing a nonexistent document returns 404."""
|
||||
client = APIClient()
|
||||
@@ -146,7 +161,7 @@ def test_api_documents_content_nonexistent_document(mock_request):
|
||||
mock_request.assert_not_called()
|
||||
|
||||
|
||||
@patch("core.services.yprovider_services.YProviderAPI._request")
|
||||
@patch("core.services.converter_services.YdocConverter._request")
|
||||
def test_api_documents_content_empty_document(mock_request):
|
||||
"""Test that accessing an empty document returns empty content."""
|
||||
document = factories.DocumentFactory(link_reach="public", content="")
|
||||
|
||||
@@ -16,16 +16,16 @@ from rest_framework.test import APIClient
|
||||
from core import factories
|
||||
from core.api.serializers import ServerCreateDocumentSerializer
|
||||
from core.models import Document, Invitation, User
|
||||
from core.services.yprovider_services import ConversionError, YProviderAPI
|
||||
from core.services.converter_services import ConversionError, YdocConverter
|
||||
|
||||
pytestmark = pytest.mark.django_db
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def mock_convert_md():
|
||||
"""Mock YProviderAPI.convert to return a converted content."""
|
||||
"""Mock YdocConverter.convert to return a converted content."""
|
||||
with patch.object(
|
||||
YProviderAPI,
|
||||
YdocConverter,
|
||||
"convert",
|
||||
return_value="Converted document content",
|
||||
) as mock:
|
||||
|
||||
@@ -1,29 +1,28 @@
|
||||
"""Test y-provider services."""
|
||||
|
||||
import json
|
||||
from base64 import b64decode
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
import requests
|
||||
|
||||
from core.services.yprovider_services import (
|
||||
from core.services.converter_services import (
|
||||
ServiceUnavailableError,
|
||||
ValidationError,
|
||||
YProviderAPI,
|
||||
YdocConverter,
|
||||
)
|
||||
|
||||
|
||||
def test_auth_header(settings):
|
||||
"""Test authentication header generation."""
|
||||
settings.Y_PROVIDER_API_KEY = "test-key"
|
||||
converter = YProviderAPI()
|
||||
converter = YdocConverter()
|
||||
assert converter.auth_header == "Bearer test-key"
|
||||
|
||||
|
||||
def test_convert_empty_text():
|
||||
"""Should raise ValidationError when text is empty."""
|
||||
converter = YProviderAPI()
|
||||
converter = YdocConverter()
|
||||
with pytest.raises(ValidationError, match="Input text cannot be empty"):
|
||||
converter.convert("")
|
||||
|
||||
@@ -31,13 +30,13 @@ def test_convert_empty_text():
|
||||
@patch("requests.post")
|
||||
def test_convert_service_unavailable(mock_post):
|
||||
"""Should raise ServiceUnavailableError when service is unavailable."""
|
||||
converter = YProviderAPI()
|
||||
converter = YdocConverter()
|
||||
|
||||
mock_post.side_effect = requests.RequestException("Connection error")
|
||||
|
||||
with pytest.raises(
|
||||
ServiceUnavailableError,
|
||||
match="Failed to connect to backend service",
|
||||
match="Failed to connect to conversion service",
|
||||
):
|
||||
converter.convert("test text")
|
||||
|
||||
@@ -45,7 +44,7 @@ def test_convert_service_unavailable(mock_post):
|
||||
@patch("requests.post")
|
||||
def test_convert_http_error(mock_post):
|
||||
"""Should raise ServiceUnavailableError when HTTP error occurs."""
|
||||
converter = YProviderAPI()
|
||||
converter = YdocConverter()
|
||||
|
||||
mock_response = MagicMock()
|
||||
mock_response.raise_for_status.side_effect = requests.HTTPError("HTTP Error")
|
||||
@@ -53,7 +52,7 @@ def test_convert_http_error(mock_post):
|
||||
|
||||
with pytest.raises(
|
||||
ServiceUnavailableError,
|
||||
match="Failed to connect to backend service",
|
||||
match="Failed to connect to conversion service",
|
||||
):
|
||||
converter.convert("test text")
|
||||
|
||||
@@ -68,7 +67,7 @@ def test_convert_full_integration(mock_post, settings):
|
||||
settings.CONVERSION_API_TIMEOUT = 5
|
||||
settings.CONVERSION_API_CONTENT_FIELD = "content"
|
||||
|
||||
converter = YProviderAPI()
|
||||
converter = YdocConverter()
|
||||
|
||||
expected_content = b"converted content"
|
||||
mock_response = MagicMock()
|
||||
@@ -85,6 +84,42 @@ def test_convert_full_integration(mock_post, settings):
|
||||
headers={
|
||||
"Authorization": "Bearer test-key",
|
||||
"Content-Type": "text/markdown",
|
||||
"Accept": "application/vnd.yjs.doc",
|
||||
},
|
||||
timeout=5,
|
||||
verify=False,
|
||||
)
|
||||
|
||||
|
||||
@patch("requests.post")
|
||||
def test_convert_full_integration_with_specific_headers(mock_post, settings):
|
||||
"""Test successful conversion with specific content type and accept headers."""
|
||||
settings.Y_PROVIDER_API_BASE_URL = "http://test.com/"
|
||||
settings.Y_PROVIDER_API_KEY = "test-key"
|
||||
settings.CONVERSION_API_ENDPOINT = "conversion-endpoint"
|
||||
settings.CONVERSION_API_TIMEOUT = 5
|
||||
settings.CONVERSION_API_SECURE = False
|
||||
|
||||
converter = YdocConverter()
|
||||
|
||||
expected_response = "# Test Document\n\nThis is test content."
|
||||
mock_response = MagicMock()
|
||||
mock_response.text = expected_response
|
||||
mock_response.raise_for_status.return_value = None
|
||||
mock_post.return_value = mock_response
|
||||
|
||||
result = converter.convert(
|
||||
b"test_content", "application/vnd.yjs.doc", "text/markdown"
|
||||
)
|
||||
|
||||
assert result == expected_response
|
||||
mock_post.assert_called_once_with(
|
||||
"http://test.com/conversion-endpoint/",
|
||||
data=b"test_content",
|
||||
headers={
|
||||
"Authorization": "Bearer test-key",
|
||||
"Content-Type": "application/vnd.yjs.doc",
|
||||
"Accept": "text/markdown",
|
||||
},
|
||||
timeout=5,
|
||||
verify=False,
|
||||
@@ -94,75 +129,20 @@ def test_convert_full_integration(mock_post, settings):
|
||||
@patch("requests.post")
|
||||
def test_convert_timeout(mock_post):
|
||||
"""Should raise ServiceUnavailableError when request times out."""
|
||||
converter = YProviderAPI()
|
||||
converter = YdocConverter()
|
||||
|
||||
mock_post.side_effect = requests.Timeout("Request timed out")
|
||||
|
||||
with pytest.raises(
|
||||
ServiceUnavailableError,
|
||||
match="Failed to connect to backend service",
|
||||
match="Failed to connect to conversion service",
|
||||
):
|
||||
converter.convert("test text")
|
||||
|
||||
|
||||
def test_convert_none_input():
|
||||
"""Should raise ValidationError when input is None."""
|
||||
converter = YProviderAPI()
|
||||
converter = YdocConverter()
|
||||
|
||||
with pytest.raises(ValidationError, match="Input text cannot be empty"):
|
||||
converter.convert(None)
|
||||
|
||||
|
||||
def test_content_empty_content():
|
||||
"""Should raise ValidationError when content is empty."""
|
||||
converter = YProviderAPI()
|
||||
with pytest.raises(ValidationError, match="Input content cannot be empty"):
|
||||
converter.content("", "markdown")
|
||||
|
||||
|
||||
@patch("requests.post")
|
||||
def test_content_service_unavailable(mock_post):
|
||||
"""Should raise ServiceUnavailableError when service is unavailable."""
|
||||
converter = YProviderAPI()
|
||||
|
||||
mock_post.side_effect = requests.RequestException("Connection error")
|
||||
|
||||
with pytest.raises(
|
||||
ServiceUnavailableError,
|
||||
match="Failed to connect to backend service",
|
||||
):
|
||||
converter.content("test_content", "markdown")
|
||||
|
||||
|
||||
@patch("requests.post")
|
||||
def test_content_success(mock_post, settings):
|
||||
"""Test successful content fetch."""
|
||||
settings.Y_PROVIDER_API_BASE_URL = "http://test.com/api/"
|
||||
settings.Y_PROVIDER_API_KEY = "test-key"
|
||||
settings.CONVERSION_API_TIMEOUT = 5
|
||||
settings.CONVERSION_API_SECURE = False
|
||||
|
||||
converter = YProviderAPI()
|
||||
|
||||
expected_response = {
|
||||
"content": "# Test Document\n\nThis is test content.",
|
||||
"format": "markdown",
|
||||
}
|
||||
mock_response = MagicMock()
|
||||
mock_response.json.return_value = expected_response
|
||||
mock_response.raise_for_status.return_value = None
|
||||
mock_post.return_value = mock_response
|
||||
|
||||
result = converter.content("test_content", "markdown")
|
||||
|
||||
assert result == expected_response
|
||||
mock_post.assert_called_once_with(
|
||||
"http://test.com/api/content/",
|
||||
data=json.dumps({"content": "test_content", "format": "markdown"}),
|
||||
headers={
|
||||
"Authorization": "Bearer test-key",
|
||||
"Content-Type": "application/json",
|
||||
},
|
||||
timeout=5,
|
||||
verify=False,
|
||||
)
|
||||
Reference in New Issue
Block a user