mirror of
https://github.com/suitenumerique/docs.git
synced 2026-08-17 21:25:43 +02:00
✨(collaboration) erase content in yhub from clean_document command
The clean_document command makes a reset of a document deleting its content and all the attachments linked to this subdocument and its children. The hard delete api in yhub make the room, so the document id, not usable at all and this is not not what we want. We added a new custom api in yhub to manage this case, the document is hard deleted and then the Tombstone to make the room reusable again.
This commit is contained in:
@@ -21,6 +21,18 @@ and this project adheres to
|
||||
|
||||
### Added
|
||||
|
||||
- ✨(collaboration) erase the content of a document on the collaboration server
|
||||
when `clean_document` resets it. The command cleared the database and the
|
||||
object storage, but the content lives on the collaboration server now: it kept
|
||||
serving the document the reset was supposed to erase, and the manual
|
||||
remediation was to run SQL against yhub's own database. It goes through the
|
||||
new backend-internal `POST /collaboration/reset-ydoc/v1/{org}/{docid}`, which
|
||||
hard-deletes the room and then drops the deletion record so it stays usable —
|
||||
the document keeps its id and goes on being edited, which is why neither of
|
||||
yhub's deletions fits on its own. The documents it could not erase are named
|
||||
on stderr: their content is still served, and the reset is not done until they
|
||||
are dealt with. Note that erasing the room does not erase the copy each editor
|
||||
holds — reset a document when nobody is editing it
|
||||
- ✨(collaboration) delete a document on the collaboration server when it is
|
||||
deleted in Docs, and restore it when it comes back out of the trashbin. The
|
||||
content lives there now, so until it was told, the clients already editing a
|
||||
|
||||
@@ -1,13 +1,5 @@
|
||||
"""Clean a document by resetting it (keeping its title) and deleting all descendants."""
|
||||
|
||||
# TODO(yhub): this sandbox reset no longer erases the document content. It purges
|
||||
# the S3 versions, but yhub durably retains the Yjs document in its own Postgres
|
||||
# and re-serves it on the next websocket connect (CRDT merge with the empty
|
||||
# seed resurrects the purged content). yhub has no delete API; until it grows
|
||||
# one, the interim remediation is to run, against yhub's stores:
|
||||
# DELETE FROM yhub_ydoc_v1 WHERE org='docs' AND docid='<document_id>';
|
||||
# and drop the `yhub:room:docs:<document_id>:*` redis keys.
|
||||
|
||||
import logging
|
||||
|
||||
from django.conf import settings
|
||||
@@ -28,6 +20,7 @@ from core.models import (
|
||||
LinkTrace,
|
||||
Thread,
|
||||
)
|
||||
from core.services.yhub_services import YHubError, YHubService
|
||||
|
||||
logger = logging.getLogger("impress.commands.clean_document")
|
||||
|
||||
@@ -150,8 +143,56 @@ class Command(BaseCommand):
|
||||
logger.warning("Failed to delete S3 attachment %s", key)
|
||||
|
||||
self.stdout.write(f"Deleted {len(all_attachment_keys)} attachment(s) from S3.")
|
||||
|
||||
# After the object storage, never before: what is erased here can be
|
||||
# seeded back from a legacy object that is still in the bucket, and the
|
||||
# first read of the document is all it takes.
|
||||
self._erase_collaboration_content(all_documents)
|
||||
|
||||
self.stdout.write("Done.")
|
||||
|
||||
def _erase_collaboration_content(self, documents):
|
||||
"""
|
||||
Erase the content of the documents on the collaboration server.
|
||||
|
||||
That is where the content lives: without this the reset only clears the
|
||||
database and the object storage, and the next editor to connect is
|
||||
served the document that was supposed to be gone.
|
||||
|
||||
The room is emptied and left usable rather than deleted — the root
|
||||
document keeps its id and goes on being edited. Its descendants are
|
||||
deleted for good by then and would not mind either way.
|
||||
|
||||
The editors are disconnected, but each of them holds a copy of the
|
||||
document: one that reconnects with it syncs the content back into the
|
||||
emptied room. Run this when nobody is editing, and have anyone who was
|
||||
reload the page.
|
||||
"""
|
||||
service = YHubService()
|
||||
failed = []
|
||||
|
||||
for doc in documents:
|
||||
try:
|
||||
service.reset_ydoc(doc)
|
||||
except YHubError:
|
||||
logger.warning(
|
||||
"Failed to erase the collaboration content of document %s", doc.id
|
||||
)
|
||||
failed.append(doc.id)
|
||||
|
||||
erased = len(documents) - len(failed)
|
||||
self.stdout.write(f"Erased collaboration content for {erased} document(s).")
|
||||
|
||||
if failed:
|
||||
# loud, and by id: the content of these documents is still being
|
||||
# served, so the reset is not done until they are dealt with
|
||||
self.stderr.write(
|
||||
"Collaboration content NOT erased for "
|
||||
f"{len(failed)} document(s): {', '.join(str(id) for id in failed)}. "
|
||||
"Their content is still served by the collaboration server, "
|
||||
"run the command again."
|
||||
)
|
||||
|
||||
def _clean_root_relations(self, document):
|
||||
"""
|
||||
Delete the relations attached to the root document: accesses (except
|
||||
|
||||
@@ -320,6 +320,25 @@ class YHubService:
|
||||
"""
|
||||
return self.request("post", self.build_url("restore-ydoc", document))
|
||||
|
||||
def reset_ydoc(self, document):
|
||||
"""
|
||||
Erase the content of a document on the collaboration server.
|
||||
|
||||
The document itself stays: its room is emptied and left usable, as if
|
||||
it had never been written. This is what resetting a document means once
|
||||
the content lives there — deleting the room would answer 404 for a
|
||||
document that goes on existing.
|
||||
|
||||
Irreversible, and it is meant to be: the editors are disconnected and
|
||||
the content is gone from the collaboration server for good, history
|
||||
included. Only the backend can ask for it.
|
||||
"""
|
||||
return self.request(
|
||||
"post",
|
||||
self.build_url("reset-ydoc", document),
|
||||
headers=self.build_user_header(self.user_id),
|
||||
)
|
||||
|
||||
def migrate(self, document, force=False):
|
||||
"""
|
||||
Replay the legacy version history of a document into the collaboration server.
|
||||
|
||||
@@ -11,10 +11,23 @@ from botocore.exceptions import ClientError
|
||||
|
||||
from core import choices, factories, models
|
||||
from core.choices import LinkReachChoices, LinkRoleChoices
|
||||
from core.services.yhub_services import ServiceUnavailableError
|
||||
|
||||
pytestmark = pytest.mark.django_db
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True, name="mock_yhub")
|
||||
def mock_yhub_fixture():
|
||||
"""
|
||||
Stand in for the collaboration server, which holds the content the command
|
||||
erases. Autouse: every run of the command reaches it.
|
||||
"""
|
||||
with mock.patch(
|
||||
"core.management.commands.clean_document.YHubService"
|
||||
) as mock_service:
|
||||
yield mock_service.return_value
|
||||
|
||||
|
||||
def purged_keys(mock_storage):
|
||||
"""
|
||||
Return the set of object keys whose versions were purged from S3, i.e. the
|
||||
@@ -378,3 +391,50 @@ def test_clean_document_with_options(settings):
|
||||
child.file_key,
|
||||
grandchild.file_key,
|
||||
}
|
||||
|
||||
|
||||
def test_clean_document_erases_the_collaboration_content(settings, mock_yhub):
|
||||
"""
|
||||
The content lives on the collaboration server, so resetting a document
|
||||
means erasing it there too — for the root and for the descendants the
|
||||
command deletes.
|
||||
"""
|
||||
settings.DEBUG = True
|
||||
|
||||
root = factories.DocumentFactory(title="Root")
|
||||
child = factories.DocumentFactory(parent=root)
|
||||
grandchild = factories.DocumentFactory(parent=child)
|
||||
|
||||
with mock.patch("core.management.commands.clean_document.default_storage"):
|
||||
call_command("clean_document", str(root.id))
|
||||
|
||||
assert mock_yhub.reset_ydoc.call_args_list == [
|
||||
mock.call(root),
|
||||
mock.call(child),
|
||||
mock.call(grandchild),
|
||||
]
|
||||
|
||||
|
||||
def test_clean_document_reports_the_documents_it_could_not_erase(
|
||||
settings, mock_yhub, capsys
|
||||
):
|
||||
"""
|
||||
A document the collaboration server would not erase is named, and does not
|
||||
deprive the ones after it of their erasure: its content is still served, so
|
||||
the reset is not done.
|
||||
"""
|
||||
settings.DEBUG = True
|
||||
|
||||
root = factories.DocumentFactory(title="Root")
|
||||
child = factories.DocumentFactory(parent=root)
|
||||
mock_yhub.reset_ydoc.side_effect = [ServiceUnavailableError("yhub is down"), None]
|
||||
|
||||
with mock.patch("core.management.commands.clean_document.default_storage"):
|
||||
call_command("clean_document", str(root.id))
|
||||
|
||||
assert mock_yhub.reset_ydoc.call_args_list == [mock.call(root), mock.call(child)]
|
||||
|
||||
captured = capsys.readouterr()
|
||||
assert "Erased collaboration content for 1 document(s)." in captured.out
|
||||
assert str(root.id) in captured.err
|
||||
assert str(child.id) not in captured.err
|
||||
|
||||
@@ -273,6 +273,24 @@ def test_restore_ydoc(mock_request):
|
||||
)
|
||||
|
||||
|
||||
@patch("requests.request")
|
||||
def test_reset_ydoc(mock_request):
|
||||
"""Should ask yhub to erase the content of the document."""
|
||||
mock_request.return_value.ok = True
|
||||
user = UserFactory.build()
|
||||
|
||||
response = YHubService(user=user).reset_ydoc(DOCUMENT)
|
||||
|
||||
assert response is mock_request.return_value
|
||||
args, kwargs = mock_request.call_args
|
||||
assert args == (
|
||||
"post",
|
||||
f"http://yhub:3002/collaboration/reset-ydoc/v1/docs/{DOCUMENT.id!s}",
|
||||
)
|
||||
# who erased the content, for the record yhub keeps of the deletion it does
|
||||
assert kwargs["headers"]["X-User-Id"] == str(user.pk)
|
||||
|
||||
|
||||
@patch("requests.request")
|
||||
def test_restore_ydoc_erased_content(mock_request):
|
||||
"""A document whose content was erased should report the conflict it is."""
|
||||
|
||||
@@ -46,6 +46,9 @@ It is not a fork of yhub — it is a thin wrapper:
|
||||
deletion of a document — admin JWT only, like `reset-connections`. Deleting
|
||||
one needs nothing custom, the built-in `DELETE .../ydoc/` does it (see
|
||||
"Deletion" below); restoring has no built-in route,
|
||||
- exposes `POST /collaboration/reset-ydoc/v1/{org}/{docid}`, which erases the
|
||||
content of a document and leaves its room usable — admin JWT only, and
|
||||
irreversible (see "Deletion" below),
|
||||
- notifies the Django backend on
|
||||
`POST /api/v1.0/documents/{id}/content-updated/` whenever the worker
|
||||
persists new content for a document, so that lists ordered by `updated_at`
|
||||
@@ -65,9 +68,9 @@ the websocket and the built-in document APIs (`ydoc`, `rollback`, `prune`,
|
||||
`changeset`, `activity`) are all guarded by the same cookie-based document
|
||||
authorization and are meant to be reachable by browsers, as is
|
||||
`/collaboration/jwks/v1`, which carries public keys and nothing else. The one
|
||||
exception is `/collaboration/reset-connections/`, `/collaboration/migrate/` and
|
||||
`/collaboration/restore-ydoc/`, which are backend-internal and should not be
|
||||
routed through the public ingress.
|
||||
exception is `/collaboration/reset-connections/`, `/collaboration/migrate/`,
|
||||
`/collaboration/restore-ydoc/` and `/collaboration/reset-ydoc/`, which are
|
||||
backend-internal and should not be routed through the public ingress.
|
||||
|
||||
## Container image
|
||||
|
||||
@@ -141,11 +144,46 @@ a subtree without asking what became of each document in it.
|
||||
|
||||
Erasing the content for good is a third operation (`YHub.deleteDoc(room, {
|
||||
hard: true })`), reachable from inside this process only — yhub deliberately
|
||||
keeps it off the REST API. Nothing here calls it: Docs never erases a document
|
||||
either, a soft-deleted one simply stops being restorable after
|
||||
`TRASHBIN_CUTOFF_DAYS`. Note that a hard deletion is final for that room — the
|
||||
keeps it off the REST API. It is not what deleting a document in Docs does: a
|
||||
soft-deleted one simply stops being restorable after `TRASHBIN_CUTOFF_DAYS`,
|
||||
and its content is kept. Note that a hard deletion is final for that room — the
|
||||
docid can never be written again, and `restore-ydoc` answers 409 for it.
|
||||
|
||||
### Resetting (`POST /collaboration/reset-ydoc/v1/{org}/{docid}`)
|
||||
|
||||
One caller does erase content: the backend's `clean_document` command, which
|
||||
resets the onboarding sandbox. It empties a document rather than deleting it —
|
||||
the Django document keeps its id and goes on being edited — so neither deletion
|
||||
fits: a soft one answers 404 for a document that still exists, and a hard one is
|
||||
final for the room.
|
||||
|
||||
This endpoint hard-deletes and then drops the deletion record, which is what
|
||||
leaves the room writable again. That order matters: the record is also the
|
||||
barrier that refuses every write while the erasure runs, so a compaction that
|
||||
was already merging cannot put the content back. Compaction is disabled for the
|
||||
room around the whole sequence, and the content is read back afterwards — if it
|
||||
reappeared, the erasure runs once more, and the endpoint answers 500 rather than
|
||||
report an erasure it did not achieve.
|
||||
|
||||
Irreversible, admin JWT only, and backend-internal.
|
||||
|
||||
**Erasing a room does not erase the copies of it.** The editors are disconnected
|
||||
(close code 4404), but a Yjs client holds the whole document in memory: one that
|
||||
reconnects with its copy syncs it back into the empty room, and the content is
|
||||
returned. The room accepting writes again is what makes this a reset rather than
|
||||
a deletion, so the room itself cannot refuse them.
|
||||
|
||||
Connected clients could be dealt with, and deliberately are not: broadcasting an
|
||||
update that deletes everything, before the kick, empties them for good — a Yjs
|
||||
client with garbage collection on (what an editor runs, Docs refuses `gc=false`
|
||||
connections to users) drops the deleted content rather than keeping it as
|
||||
history, so it has nothing left to push back. What that does not cover is a
|
||||
client that was offline or backgrounded at that moment, which comes back with
|
||||
its copy intact either way.
|
||||
|
||||
So: reset a document when nobody is editing it, and have anyone who was reload
|
||||
the page.
|
||||
|
||||
## Soft migration (`SOFT_MIGRATION=true`)
|
||||
|
||||
Documents were historically stored by the Django backend in the S3 media
|
||||
|
||||
@@ -61,6 +61,9 @@ const UUID4 =
|
||||
// an empty Yjs update (what `Y.encodeStateAsUpdate(new Y.Doc())` encodes to) —
|
||||
// hardcoded so we don't import @y/y for two bytes
|
||||
const EMPTY_YDOC = new Uint8Array([0, 0]);
|
||||
// yhub's "no effective content" convention: an empty update encodes to 2 bytes,
|
||||
// and anything up to 3 is read as an empty document
|
||||
const EMPTY_UPDATE_MAX_BYTES = 3;
|
||||
// uws buffers the whole body before the handler sees it, so this cap does not
|
||||
// bound upload memory — it bounds what a single create hands to a compute
|
||||
// worker and writes to the valkey stream as one message. Creates carry one
|
||||
@@ -69,6 +72,7 @@ const EMPTY_YDOC = new Uint8Array([0, 0]);
|
||||
const MAX_CREATE_BYTES = 10 * 1024 * 1024;
|
||||
|
||||
const touchLog = logger.child({ module: 'updated-at-notifier' });
|
||||
const resetLog = logger.child({ module: 'reset-ydoc' });
|
||||
|
||||
const BACKEND_NOTIFY_TIMEOUT_MS = 5000;
|
||||
// Audience of the tokens the backend accepts from us. It must match the one
|
||||
@@ -354,6 +358,36 @@ const jsonResponse = (status, body) =>
|
||||
headers: { 'content-type': 'application/json' },
|
||||
});
|
||||
|
||||
// Does the room hold anything? Covers persisted rows and the messages still on
|
||||
// the stream, which is what makes it an answer about the content rather than
|
||||
// about the storage.
|
||||
const hasContent = async (yhub, room) => {
|
||||
const { gcDoc } = await yhub.getDoc(
|
||||
room,
|
||||
{ gc: true, nongc: false },
|
||||
{ gcOnMerge: false },
|
||||
);
|
||||
return gcDoc != null && gcDoc.byteLength > EMPTY_UPDATE_MAX_BYTES;
|
||||
};
|
||||
|
||||
// Erase every trace of a room's content and leave it writable again.
|
||||
//
|
||||
// The erasure is yhub's hard deletion: it clears the stream, disconnects the
|
||||
// editors and drops every row and asset, irreversibly. Its tombstone is also
|
||||
// the barrier that stops a compaction still in flight from writing the content
|
||||
// back — every `store` is refused while it is there, and the purge runs behind
|
||||
// it — so the room is only made writable again, by dropping the tombstone,
|
||||
// once there is nothing left to write back.
|
||||
//
|
||||
// Dropping the tombstone is what makes this a reset rather than a deletion:
|
||||
// yhub has no such operation, a hard deletion is final for the room and even
|
||||
// `restoreDoc` refuses it. Here the document id belongs to a Django document
|
||||
// that goes on living, so the room has to be usable again.
|
||||
const eraseContent = async (yhub, room, by) => {
|
||||
await yhub.deleteDoc(room, { hard: true, by });
|
||||
await yhub.persistence.deleteTombstone(room);
|
||||
};
|
||||
|
||||
const api = [
|
||||
// GET /collaboration/jwks/v1 — the public keys verifying the tokens we sign
|
||||
// to call the backend, in the JSON Web Key Set format (RFC 7517). Global
|
||||
@@ -493,9 +527,9 @@ const api = [
|
||||
// ("HTTP/1.1 413 ") — legal, and callers switch on the code
|
||||
return jsonResponse(413, { error: 'Update too large' });
|
||||
}
|
||||
// <= 3 bytes is yhub's "no effective content" convention (an empty
|
||||
// update encodes to 2 bytes) — reject before it reaches a worker
|
||||
if (update.byteLength <= 3) {
|
||||
// yhub's "no effective content" convention — reject before it reaches
|
||||
// a worker
|
||||
if (update.byteLength <= EMPTY_UPDATE_MAX_BYTES) {
|
||||
return jsonResponse(400, { error: 'Empty update' });
|
||||
}
|
||||
// covers persisted state AND uncompacted stream messages. Not atomic
|
||||
@@ -509,7 +543,7 @@ const api = [
|
||||
{ gc: true, nongc: false },
|
||||
{ gcOnMerge: false },
|
||||
);
|
||||
if (gcDoc != null && gcDoc.byteLength > 3) {
|
||||
if (gcDoc != null && gcDoc.byteLength > EMPTY_UPDATE_MAX_BYTES) {
|
||||
return jsonResponse(409, { error: 'Document already exists' });
|
||||
}
|
||||
// Only the backend admin token may attribute the content to another
|
||||
@@ -603,6 +637,61 @@ const api = [
|
||||
},
|
||||
},
|
||||
}),
|
||||
// POST /collaboration/reset-ydoc/v1/{org}/{docid} — erase the content of a
|
||||
// document and leave the room usable, as if it had never been written.
|
||||
//
|
||||
// What the backend's `clean_document` command needs to reset the onboarding
|
||||
// sandbox: the Django document keeps its id and goes on being edited, so
|
||||
// deleting the room is not an option — a hard deletion is final and even a
|
||||
// soft one would answer 404 for a document that still exists. Backend-internal
|
||||
// and admin-only, like the deletions it is built on: this destroys content
|
||||
// with no way back.
|
||||
createApiEndpoint('reset-ydoc', {
|
||||
accessPurpose: 'reset',
|
||||
post: {
|
||||
handler: async (req) => {
|
||||
if (req.org !== ORG) {
|
||||
return jsonResponse(400, { error: 'Unknown org' });
|
||||
}
|
||||
if (!UUID4.test(req.docid)) {
|
||||
return jsonResponse(400, { error: 'Room name is invalid' });
|
||||
}
|
||||
if (req.branch !== 'main') {
|
||||
return jsonResponse(400, { error: 'Unknown branch' });
|
||||
}
|
||||
const by = req.headers['x-user-id'] || req.authInfo.userid;
|
||||
// Nothing compacts this room while the erasure runs: this drops the
|
||||
// task already waiting for it and refuses to enqueue another, which
|
||||
// leaves one writer to race with — a task a worker had claimed before
|
||||
// this call. The tombstone barrier covers it right up to the moment
|
||||
// the room is made writable again, so it can only land after that,
|
||||
// and the second pass below is what picks it up.
|
||||
await req.yhub.stream.disableCompaction(req.room);
|
||||
try {
|
||||
await eraseContent(req.yhub, req.room, by);
|
||||
if (await hasContent(req.yhub, req.room)) {
|
||||
resetLog.warn(
|
||||
{ docid: req.docid },
|
||||
'content came back while it was being erased, erasing again',
|
||||
);
|
||||
await eraseContent(req.yhub, req.room, by);
|
||||
if (await hasContent(req.yhub, req.room)) {
|
||||
// saying it is erased when it is not is the one answer this
|
||||
// endpoint must never give
|
||||
return jsonResponse(500, {
|
||||
error: 'Document content came back after being erased',
|
||||
});
|
||||
}
|
||||
}
|
||||
} finally {
|
||||
// even on failure: leaving compaction off would freeze the room for
|
||||
// every later edit, a worse state than the one we came to fix
|
||||
await req.yhub.stream.enableCompaction(req.room);
|
||||
}
|
||||
return jsonResponse(200, { message: 'Document content erased' });
|
||||
},
|
||||
},
|
||||
}),
|
||||
];
|
||||
|
||||
// Django orders the document lists by `updated_at` and no edit goes through it
|
||||
|
||||
Reference in New Issue
Block a user