♻️(backend) take adavantage of yhub 0.5.0 json encoding returns

The version 0.5.0 can manage response format by using accept and
content-type headers. In python we can't use for now the lib0 decoder so
we have to use the json format. When the lib0 decoder will be available
in pycrdt we will use it. So we can now use directly the /ydoc api to
fetch a document content instead the custom api made for this.

CHANGELOG.md

@@ -98,7 +98,6 @@ and this project adheres to
   on its own
 - 🔧(dev) generate the JWT signing key of the collaboration server when
   bootstrapping the dev stack, alongside the backend one
-- ✨(collaboration) add a get-ydoc endpoint on yhub
 - ✨(backend) serve `documents/{id}/formatted-content/` from yhub
 - ✨(backend) duplicate a document through the collaboration server
 - ✨(backend) call YHubService to seed initial document content
@@ -139,6 +138,11 @@ and this project adheres to
   identity of the user the sandbox is created for. A collaboration server that
   cannot be reached skips the sandbox, as a missing template already did, and
   never fails the signup
+- ♻️(backend) read the content of a document from the built-in `ydoc` endpoint
+  of the collaboration server: `YHubService` asks for JSON, which yhub speaks
+  since 0.5.0, so the custom `get-ydoc` endpoint it used to need is gone.
+  `create-ydoc` stays, no built-in offers what it does — a strict create, and
+  content credited to the user rather than to the backend
 - 🔥(backend) remove the unused `CollaborationService`
 - 💥(backend) remove the `documents/{id}/content/` endpoint
 - 💥(backend) remove the `documents/{id}/can-edit/` endpoint

src/backend/core/services/yhub_services.py

@@ -13,6 +13,12 @@ update), `rollback`, `prune`, `changeset` and `activity`, all at `v1`. yhub also
 accepts a `branch` query parameter, but our auth plugin only ever grants access
 to the `main` branch, so this service never sends it.
 
+Since yhub 0.5.0 those endpoints answer JSON to a request asking for it, with
+the binary fields base64 encoded, so this service sends `Accept:
+application/json` and reads them without a lib0 decoder. Their errors come back
+the same way, as a JSON `{"error": ...}` this service reports along with the
+status.
+
 A few routes are about the server itself rather than about a document, and
 carry no room: `/{prefix}/jwks/{version}` publishes the public keys validating
 the tokens yhub signs to call us back.
@@ -21,6 +27,7 @@ This service only owns the transport for now, the endpoints are added as we
 need them.
 """
 
+import base64
 import logging
 
 from django.conf import settings
@@ -72,6 +79,11 @@ class YHubService:
     # Version of the endpoints we call, the one all the built-ins are at.
     api_version = "v1"
 
+    # A Yjs update carrying no content encodes to 2 bytes, and yhub reads
+    # anything up to 3 as an empty document. `ydoc` answers the encoding of an
+    # empty document for a room it holds nothing for, never an empty body.
+    empty_update_max_bytes = 3
+
     def __init__(self, user=None):
         """Bind the service to the user a call is made on behalf of, if any."""
         self.user = user
@@ -173,7 +185,7 @@ class YHubService:
 
     def request(self, method, url, data=None, headers=None):
         """
-        Send an authenticated request to the yhub API.
+        Send an authenticated request to the yhub API, asking it for JSON.
 
         Return the raw response, it is up to the caller to decode its body: the
         endpoints do not all answer with the same payload.
@@ -185,7 +197,8 @@ class YHubService:
                 data=data,
                 headers={
                     "Authorization": self.auth_header,
-                    "Content-Type": "application/octet-stream",
+                    # what makes yhub answer JSON rather than its lib0 encoding
+                    "Accept": "application/json",
                     **(headers or {}),
                 },
                 timeout=self.timeout,
@@ -203,44 +216,73 @@ class YHubService:
                 response.status_code,
                 response.text[:200] if response.text else "empty",
             )
+            detail = self.json_body(response).get("error")
             raise APIError(
-                f"The yhub API answered {response.status_code} on {url}",
+                f"The yhub API answered {response.status_code} on {url}"
+                + (f": {detail}" if detail else ""),
                 status_code=response.status_code,
             )
 
         return response
 
+    @staticmethod
+    def json_body(response):
+        """
+        Return the JSON body of a response, an empty dict when it has none.
+
+        yhub reports its errors as `{"error": ...}` and its endpoints answer
+        JSON, but a failure can also come from something else on the way (a
+        proxy, a gateway): what it says is a bonus, never something to fail on.
+        """
+        try:
+            body = response.json()
+        except ValueError:
+            return {}
+
+        return body if isinstance(body, dict) else {}
+
     def get_ydoc(self, document):
         """
         Return the current Yjs state of a document, None when it has none.
 
         The raw update is what `create_ydoc` takes, so the state of a document
-        can be copied into another one. The built-in `ydoc` endpoint is not
-        used, it answers the lib0 encoding of an envelope rather than the
-        update itself.
+        can be copied into another one. The built-in `ydoc` endpoint answers
+        `{"doc": ...}`, the update base64 encoded, and the encoding of an empty
+        document for a room it holds no content for.
         """
-        response = self.request("get", self.build_url("get-ydoc", document))
+        response = self.request("get", self.build_url("ydoc", document))
 
-        return response.content or None
+        try:
+            update = base64.b64decode(self.json_body(response)["doc"])
+        except (KeyError, TypeError, ValueError) as err:
+            raise APIError(
+                f"The yhub API answered no readable document on {response.url}"
+            ) from err
+
+        return update if len(update) > self.empty_update_max_bytes else None
 
     def create_ydoc(self, document, update):
         """
         Seed the initial Yjs state of a document.
 
         The body is the raw binary update, what pycrdt's `get_update()`
-        returns, and not the lib0 encoding the built-in `ydoc` endpoint speaks.
-        The content is attributed to the user the service is bound to, yhub
-        only takes our word for it because the token grants admin.
+        returns. This is not the built-in `ydoc` endpoint, which would take the
+        same update base64 encoded but knows nothing of the two things this one
+        is for: it is a strict create, and it attributes the content to the
+        user the service is bound to rather than to the backend calling it.
 
-        It is a strict create: yhub answers 409 when the document already has
-        content, 413 over 10MB and 400 on an update it cannot apply, all
-        reported as an `APIError` carrying the status.
+        yhub answers 409 when the document already has content, 413 over 10MB
+        and 400 on an update it cannot apply, all reported as an `APIError`
+        carrying the status.
         """
         return self.request(
             "post",
             self.build_url("create-ydoc", document),
             data=update,
-            headers=self.build_user_header(self.user_id),
+            headers={
+                "Content-Type": "application/octet-stream",
+                **self.build_user_header(self.user_id),
+            },
         )
 
     def reset_connections(self, document, user_id=None):

src/backend/core/tests/test_services_yhub_services.py

@@ -1,5 +1,6 @@
 """Test yhub services."""
 
+from base64 import b64encode
 from unittest.mock import patch
 from uuid import uuid4
 
@@ -127,7 +128,8 @@ def test_request(mock_request):
     assert kwargs["data"] == b"body"
     assert kwargs["timeout"] == 30
     assert kwargs["headers"]["Authorization"].startswith("Bearer ")
-    assert kwargs["headers"]["Content-Type"] == "application/octet-stream"
+    # asked of every endpoint: yhub answers its lib0 encoding otherwise
+    assert kwargs["headers"]["Accept"] == "application/json"
 
 
 @patch("requests.request")
@@ -236,23 +238,67 @@ def test_reset_connections_of_a_single_user(mock_request):
 def test_get_ydoc(mock_request):
     """Should return the raw update the collaboration server holds."""
     mock_request.return_value.ok = True
-    mock_request.return_value.content = b"\x01\x02raw yjs update"
+    mock_request.return_value.json.return_value = {
+        "doc": b64encode(b"\x01\x02raw yjs update").decode()
+    }
 
     update = YHubService().get_ydoc(DOCUMENT)
 
     assert update == b"\x01\x02raw yjs update"
-    args, _kwargs = mock_request.call_args
+    args, kwargs = mock_request.call_args
+    # the built-in endpoint, which answers the update base64 encoded in json
     assert args == (
         "get",
-        f"http://yhub:3002/collaboration/get-ydoc/v1/docs/{DOCUMENT.id!s}",
+        f"http://yhub:3002/collaboration/ydoc/v1/docs/{DOCUMENT.id!s}",
     )
+    assert kwargs["headers"]["Accept"] == "application/json"
 
 
 @patch("requests.request")
 def test_get_ydoc_without_content(mock_request):
     """A document the collaboration server holds no content for should return None."""
     mock_request.return_value.ok = True
-    # yhub answers 204 No Content, hence an empty body
-    mock_request.return_value.content = b""
+    # a room with no content answers the encoding of an empty document
+    mock_request.return_value.json.return_value = {
+        "doc": b64encode(b"\x00\x00").decode()
+    }
 
     assert YHubService().get_ydoc(DOCUMENT) is None
+
+
+@patch("requests.request")
+def test_get_ydoc_unreadable_answer(mock_request):
+    """An answer we cannot read a document out of should raise, never look empty."""
+    mock_request.return_value.ok = True
+    mock_request.return_value.json.return_value = {"unexpected": "payload"}
+
+    with pytest.raises(APIError):
+        YHubService().get_ydoc(DOCUMENT)
+
+
+@patch("requests.request")
+def test_request_error_reports_what_yhub_said(mock_request):
+    """The message yhub puts in its json error should travel with the status."""
+    mock_request.return_value.ok = False
+    mock_request.return_value.status_code = 409
+    mock_request.return_value.text = '{"error": "Document already exists"}'
+    mock_request.return_value.json.return_value = {"error": "Document already exists"}
+
+    with pytest.raises(APIError, match="Document already exists") as excinfo:
+        YHubService().create_ydoc(DOCUMENT, b"\x01\x02update")
+
+    assert excinfo.value.status_code == 409
+
+
+@patch("requests.request")
+def test_request_error_without_json_body(mock_request):
+    """An error from something else on the way should be reported all the same."""
+    mock_request.return_value.ok = False
+    mock_request.return_value.status_code = 502
+    mock_request.return_value.text = "<html>Bad Gateway</html>"
+    mock_request.return_value.json.side_effect = ValueError("not json")
+
+    with pytest.raises(APIError, match="answered 502") as excinfo:
+        YHubService().get_ydoc(DOCUMENT)
+
+    assert excinfo.value.status_code == 502