From b3e308b2d497eab2840ece99554115258a3db04b Mon Sep 17 00:00:00 2001 From: David Sheldrick Date: Tue, 22 Oct 2024 14:07:05 +0100 Subject: [PATCH] [botcom] Fix file deletion and creation (#4751) - make file deletion work properly - There was a problem with one of the sql queries preventing the records from being removed - Need to also kick people out of the room when the file is deleted and remove the blob from R2 - fix the file creation interaction (was not showing up in sidebar) ### Change type - [x] `other` --- apps/dotcom/client/src/tla/app/TldrawApp.ts | 12 +++++++---- .../dialogs/TlaDeleteFileDialog.tsx | 10 +++++++-- .../sync-worker/src/TLAppDurableObject.ts | 13 +++++++++++- .../sync-worker/src/TLDrawDurableObject.ts | 21 +++++++++++++++++++ packages/sync-core/src/lib/TLSyncRoom.ts | 21 ++++--------------- 5 files changed, 53 insertions(+), 24 deletions(-) diff --git a/apps/dotcom/client/src/tla/app/TldrawApp.ts b/apps/dotcom/client/src/tla/app/TldrawApp.ts index c1398911a6ec..eadc1f4d9095 100644 --- a/apps/dotcom/client/src/tla/app/TldrawApp.ts +++ b/apps/dotcom/client/src/tla/app/TldrawApp.ts @@ -20,6 +20,7 @@ import { computed, createTLUser, getUserPreferences, + uniq, } from 'tldraw' import { globalEditor } from '../../utils/globalEditor' import { getCurrentEditor } from '../utils/getCurrentEditor' @@ -122,9 +123,13 @@ export class TldrawApp { const fileEditRecords = this.getUserFileEdits() // Incluude any files that the user has edited - const fileRecords = fileEditRecords - .map((r) => this.get(r.fileId)) - .filter(Boolean) as TldrawAppFile[] + const fileRecords = uniq([ + ...this.getUserOwnFiles(), + ...(fileEditRecords + .map((r) => this.get(r.fileId)) + .concat() + .filter(Boolean) as TldrawAppFile[]), + ]) // A map of file IDs to the most recent date we have for them // the default date is the file's creation date; but we'll use the @@ -267,7 +272,6 @@ export class TldrawApp { } async deleteFile(_fileId: TldrawAppFileId) { - // TODO we still need to remove the file completely - you can still visit this file if you have the link. this.store.remove([_fileId]) } diff --git a/apps/dotcom/client/src/tla/components/dialogs/TlaDeleteFileDialog.tsx b/apps/dotcom/client/src/tla/components/dialogs/TlaDeleteFileDialog.tsx index b147c3ddf89d..972942cd2da7 100644 --- a/apps/dotcom/client/src/tla/components/dialogs/TlaDeleteFileDialog.tsx +++ b/apps/dotcom/client/src/tla/components/dialogs/TlaDeleteFileDialog.tsx @@ -1,5 +1,6 @@ -import { TldrawAppFileId } from '@tldraw/dotcom-shared' +import { TldrawAppFileId, TldrawAppFileRecordType } from '@tldraw/dotcom-shared' import { useCallback } from 'react' +import { useLocation, useNavigate } from 'react-router-dom' import { TldrawUiButton, TldrawUiButtonLabel, @@ -21,11 +22,16 @@ export function TlaDeleteFileDialog({ }) { const app = useApp() const raw = useRaw() + const location = useLocation() + const navigate = useNavigate() const handleDelete = useCallback(() => { app.deleteFile(fileId) + if (location.pathname.endsWith(TldrawAppFileRecordType.parseId(fileId))) { + navigate('/q') + } onClose() - }, [app, fileId, onClose]) + }, [app, fileId, location.pathname, navigate, onClose]) return ( <> diff --git a/apps/dotcom/sync-worker/src/TLAppDurableObject.ts b/apps/dotcom/sync-worker/src/TLAppDurableObject.ts index 669f8626080a..1f62a2d741b6 100644 --- a/apps/dotcom/sync-worker/src/TLAppDurableObject.ts +++ b/apps/dotcom/sync-worker/src/TLAppDurableObject.ts @@ -106,7 +106,7 @@ export class TLAppDurableObject extends DurableObject { this.updateRecord = env.DB.prepare( `insert into records (id, topicId, record, lastModifiedEpoch) values (?1, ?2, ?3, ?4) on conflict (id, topicId) do update set record = ?3, lastModifiedEpoch = ?4` ) - this.deleteRecord = env.DB.prepare(`delete from records where topicId = ?`) + this.deleteRecord = env.DB.prepare(`delete from records where id = ?`) } readonly router = Router() @@ -239,6 +239,17 @@ export class TLAppDurableObject extends DurableObject { } } + for (const deletedId of deletedIds) { + if (TldrawAppFileRecordType.isId(deletedId)) { + const slug = TldrawAppFileRecordType.parseId(deletedId) + const docDurableObject = this.env.TLDR_DOC.get( + this.env.TLDR_DOC.idFromName(`/${ROOM_PREFIX}/${slug}`) + ) + // no need to await, fire and forget + docDurableObject.appFileRecordDidDelete(slug) + } + } + this._lastPersistedClock = clock // use a shorter timeout for this 'inner' loop than the 'outer' alarm-scheduled loop // just in case there's any possibility of setting up a neverending queue diff --git a/apps/dotcom/sync-worker/src/TLDrawDurableObject.ts b/apps/dotcom/sync-worker/src/TLDrawDurableObject.ts index e9004778ac2e..39b0b51e6762 100644 --- a/apps/dotcom/sync-worker/src/TLDrawDurableObject.ts +++ b/apps/dotcom/sync-worker/src/TLDrawDurableObject.ts @@ -568,4 +568,25 @@ export class TLDrawDurableObject extends DurableObject { } } } + + async appFileRecordDidDelete(slug: string) { + if (!this._documentInfo) { + this.setDocumentInfo({ + version: CURRENT_DOCUMENT_INFO_VERSION, + slug, + isApp: true, + isOrWasCreateMode: false, + }) + } + const room = await this.getRoom() + for (const session of room.getSessions()) { + room.closeSession(session.sessionId, TLSyncErrorCloseEventReason.NOT_FOUND) + } + room.close() + // setting _room to null will prevent any further persists from going through + this._room = null + // delete from R2 + const key = getR2KeyForRoom({ slug, isApp: true }) + await this.r2.rooms.delete(key) + } } diff --git a/packages/sync-core/src/lib/TLSyncRoom.ts b/packages/sync-core/src/lib/TLSyncRoom.ts index 92ddaa0341a9..851eecd59ce0 100644 --- a/packages/sync-core/src/lib/TLSyncRoom.ts +++ b/packages/sync-core/src/lib/TLSyncRoom.ts @@ -43,7 +43,6 @@ import { } from './diff' import { interval } from './interval' import { - TLConnectRequest, TLIncompatibilityReason, TLSocketClientSentEvent, TLSocketServerSentDataEvent, @@ -378,18 +377,6 @@ export class TLSyncRoom { return this.state.get().documents[id] } - private getTombstoneIds(message?: TLConnectRequest) { - let deletedDocs = Object.entries(this.state.get().tombstones) - if (message) { - deletedDocs = deletedDocs.filter( - ([_id, deletedAtClock]) => deletedAtClock > message.lastServerClock - ) - } - - const deletedDocsIds = deletedDocs.map(([id]) => id) - return deletedDocsIds - } - private addDocument(id: string, state: R, clock: number): Result { let { documents, tombstones } = this.state.get() if (hasOwnProperty(tombstones, id)) { @@ -680,7 +667,6 @@ export class TLSyncRoom { this.log?.warn?.('Received message from unknown session') return } - switch (message.type) { case 'connect': { return this.handleConnectRequest(session, message) @@ -813,10 +799,9 @@ export class TLSyncRoom { // or if the server exits/crashes with unpersisted changes message.lastServerClock > this.clock ) { - const deletedDocsIds = this.getTombstoneIds(message) const diff: NetworkDiff = {} for (const [id, doc] of Object.entries(this.state.get().documents)) { - if (id !== session.presenceId && !deletedDocsIds.includes(id)) { + if (id !== session.presenceId) { diff[id] = [RecordOpType.Put, doc.state] } } @@ -854,6 +839,9 @@ export class TLSyncRoom { doc.state.id !== session.presenceId ) : [] + const deletedDocsIds = Object.entries(this.state.get().tombstones) + .filter(([_id, deletedAtClock]) => deletedAtClock > message.lastServerClock) + .map(([id]) => id) for (const doc of updatedDocs) { diff[doc.state.id] = [RecordOpType.Put, doc.state] @@ -862,7 +850,6 @@ export class TLSyncRoom { diff[doc.state.id] = [RecordOpType.Put, doc.state] } - const deletedDocsIds = this.getTombstoneIds(message) for (const docId of deletedDocsIds) { diff[docId] = [RecordOpType.Remove] }