From b83d8d09aa6a44d9754d74e0e2f1adf54d8ddbf7 Mon Sep 17 00:00:00 2001 From: "David L. Qiu" Date: Thu, 5 Jun 2025 15:21:14 -0700 Subject: [PATCH 1/7] implement basic OOB deletion handling --- jupyter_server_documents/rooms/yroom.py | 36 ++++- .../rooms/yroom_file_api.py | 132 +++++++++++++++--- .../websockets/clients.py | 14 +- src/docprovider/yprovider.ts | 25 +++- 4 files changed, 172 insertions(+), 35 deletions(-) diff --git a/jupyter_server_documents/rooms/yroom.py b/jupyter_server_documents/rooms/yroom.py index e346f4e..08ef888 100644 --- a/jupyter_server_documents/rooms/yroom.py +++ b/jupyter_server_documents/rooms/yroom.py @@ -106,7 +106,8 @@ def __init__( loop=self._loop, fileid_manager=self._fileid_manager, contents_manager=self._contents_manager, - on_outofband_change=self.reload_ydoc + on_outofband_change=self.reload_ydoc, + on_outofband_move=self.handle_outofband_move ) # Load the YDoc content after initializing @@ -589,7 +590,8 @@ def reload_ydoc(self) -> None: loop=self._loop, fileid_manager=self._fileid_manager, contents_manager=self._contents_manager, - on_outofband_change=self.reload_ydoc + on_outofband_change=self.reload_ydoc, + on_outofband_move=self.handle_outofband_move ) self.file_api.load_ydoc_content() @@ -603,6 +605,36 @@ def reload_ydoc(self) -> None: self._jupyter_ydoc.observe(self._on_jupyter_ydoc_update) + def handle_outofband_move(self) -> None: + """ + Handles an out-of-band move/deletion by stopping the YRoom immediately. + This is similar to `self.stop()` with some key differences: + + - This closes all WS connections with close code 4001, a special close + code reserved to indicate out-of-band move/deletion. + - This does not apply any pending YDoc updates from other clients. + - This does not save the file before exiting (as the new path is unknown). + """ + # Disconnect all clients with close code 4001 + self.clients.stop(close_code=4001) + + # Remove all observers + self._ydoc.unobserve(self._ydoc_subscription) + self._awareness.unobserve(self._awareness_subscription) + + # Purge the message queue immediately, dropping all queued messages + while not self._message_queue.empty(): + self._message_queue.get_nowait() + self._message_queue.task_done() + + # Enqueue `None` to stop the `_process_message_queue()` background task + self._message_queue.put_nowait(None) + + # Finally, stop FileAPI immediately (without saving) and return. + if self.file_api: + self.file_api.stop() + + async def stop(self) -> None: """ Stops the YRoom gracefully. diff --git a/jupyter_server_documents/rooms/yroom_file_api.py b/jupyter_server_documents/rooms/yroom_file_api.py index 0ebf057..ff24aa0 100644 --- a/jupyter_server_documents/rooms/yroom_file_api.py +++ b/jupyter_server_documents/rooms/yroom_file_api.py @@ -11,6 +11,7 @@ from jupyter_ydoc.ybasedoc import YBaseDoc from jupyter_server.utils import ensure_async import logging +from tornado.web import HTTPError if TYPE_CHECKING: from typing import Any, Callable, Literal @@ -45,7 +46,32 @@ class YRoomFileAPI: _save_scheduled: bool _ydoc_content_loading: bool _ydoc_content_loaded: asyncio.Event + _last_modified: datetime | None + """ + The last file modified timestamp known to this instance. If this value + changes unexpectedly, that indicates an out-of-band change to the file. + """ + + _last_path: str | None + """ + The last file path known to this instance. If this value changes + unexpectedly, that indicates an out-of-band move/deletion on the file. + """ + + _on_outofband_change: Callable[[], Any] + """ + The callback to run when an out-of-band change is detected by this instance. + This attribute is only set in the constructor. See `YRoom` for details on + how out-of-band changes are handled. + """ + + _on_outofband_move: Callable[[], Any] + """ + The callback to run when an out-of-band move/deletion is detected by this + instance. This attribute is only set in the constructor. See `YRoom` for + details on how out-of-band changes are handled. + """ _save_loop_task: asyncio.Task @@ -58,7 +84,8 @@ def __init__( fileid_manager: BaseFileIdManager, contents_manager: AsyncContentsManager | ContentsManager, loop: asyncio.AbstractEventLoop, - on_outofband_change: Callable[[], Any] + on_outofband_change: Callable[[], Any], + on_outofband_move: Callable[[], Any] ): # Bind instance attributes self.room_id = room_id @@ -69,7 +96,9 @@ def __init__( self._fileid_manager = fileid_manager self._contents_manager = contents_manager self._on_outofband_change = on_outofband_change + self._on_outofband_move = on_outofband_move self._save_scheduled = False + self._last_path = None self._last_modified = None # Initialize loading & loaded states @@ -80,7 +109,7 @@ def __init__( self._save_loop_task = self._loop.create_task(self._watch_file()) - def get_path(self) -> str: + def get_path(self) -> str | None: """ Returns the relative path to the file by querying the FileIdManager. The path is relative to the `ServerApp.root_dir` configurable trait. @@ -88,13 +117,7 @@ def get_path(self) -> str: Raises a `RuntimeError` if the file ID does not refer to a valid file path. """ - rel_path = self._fileid_manager.get_path(self.file_id) - if not rel_path: - raise RuntimeError( - f"Unable to locate file with ID: '{self.file_id}'." - ) - - return rel_path + return self._fileid_manager.get_path(self.file_id) @property @@ -135,8 +158,13 @@ def load_ydoc_content(self) -> None: async def _load_ydoc_content(self) -> None: - # Load the content of the file from the given file ID. + # Get the path specified on the file ID path = self.get_path() + if not path: + raise RuntimeError(f"Could not find path for room '{self.room_id}'.") + self._last_path = path + + # Load the content of the file from the path file_data = await ensure_async(self._contents_manager.get( path, type=self.file_type, @@ -168,7 +196,8 @@ def schedule_save(self) -> None: async def _watch_file(self) -> None: """ - Defines a background task that continuously saves the YDoc every 500ms. + Defines a background task that continuously saves the YDoc every 500ms, + checking for out-of-band changes before doing so. Note that consumers must call `self.schedule_save()` for the next tick of this task to save. @@ -206,24 +235,81 @@ async def _watch_file(self) -> None: async def _check_oob_changes(self): """ - Checks for out-of-band changes. Called in the `self._watch_file()` - background task. - - Calls the `on_outofband_change()` function passed to the constructor if - an out-of-band change is detected. This is guaranteed to always run - before each save through the `ContentsManager`. + TODO: rename this _validate_file_integrity()? + + Checks for out-of-band operations in the `self._watch_file()` background + task. This is guaranteed to always run before each save in + `self._watch_file()` This detects the following events and acts in + response: + + - In-band move: calls `self._on_inband_move()` (TODO) + - In-band deletion: calls `self._on_inband_deletion()` (TODO) + - Out-of-band move/deletion: calls `self._on_outofband_move()` + - Out-of-band change: calls `self._on_outofband_change()` """ - # Build arguments to `CM.get()` + # Ensure that the last known path is defined. This should always be set + # by `load_ydoc_content()`. + if not self._last_path: + raise RuntimeError(f"No last known path for '{self.room_id}'. This should never happen.") + + # Get path. If the path does not match the last known path, the file was + # moved/deleted in-band via the `ContentsManager`, as it was detected by + # `jupyter_server_fileid.manager:ArbitraryFileIdManager`. + # If this happens, run the designated callback and return early. path = self.get_path() + if path != self._last_path: + if path: + self.log.warning( + f"File was moved to '{path}'. " + f"Room ID: '{self.room_id}', " + f"Last known path: '{self._last_path}'." + ) + # TODO + # self._on_inband_move() + pass + else: + self.log.warning( + "File was deleted. " + f"Room ID: '{self.room_id}', " + f"Last known path: '{self._last_path}'." + ) + # TODO + # self._on_inband_delete() + return + + # Otherwise, set the last known path + self._last_path = path + + # Build arguments to `CM.get()` file_format = self.file_format file_type = self.file_type if self.file_type in SAVEABLE_FILE_TYPES else "file" - # Check for out-of-band file changes - file_data = await ensure_async(self._contents_manager.get( - path=path, format=file_format, type=file_type, content=False - )) + # Get the file metadata from the `ContentsManager`. + # If this raises `HTTPError(404)`, that indicates the file was + # moved/deleted out-of-band. + try: + file_data = await ensure_async(self._contents_manager.get( + path=path, format=file_format, type=file_type, content=False + )) + except HTTPError as e: + # If not 404, re-raise the exception as it is unknown + if (e.status_code != 404): + raise e + + # Otherwise, this indicates the file was moved/deleted out-of-band. + # Run the designated callback and return early. + self.log.warning( + "File was deleted out-of-band. " + f"Room ID: '{self.room_id}', " + f"Last known path: '{self._last_path}'." + ) + self._on_outofband_move() + return + - # If an out-of-band file change is detected, run the designated callback + # Finally, if the file was not moved/deleted, check for out-of-band + # changes to the file content using the metadata. + # If an out-of-band file change is detected, run the designated callback. if self._last_modified != file_data['last_modified']: self.log.warning( "Out-of-band file change detected. " diff --git a/jupyter_server_documents/websockets/clients.py b/jupyter_server_documents/websockets/clients.py index 55cd0fe..c0f662a 100644 --- a/jupyter_server_documents/websockets/clients.py +++ b/jupyter_server_documents/websockets/clients.py @@ -197,14 +197,16 @@ def close_all(self, close_code: int): for client in clients: client.websocket.close(code=close_code) - def stop(self): + def stop(self, close_code: int = 1001): """ - Closes all Websocket connections with close code 1001 (server - shutting down), removes all clients from this group, and ignores any - future calls to `add()`. + Closes all Websocket connections with the given close code, removes all + clients from this group, and ignores any future calls to `add()`. + + If a close code is not specified, it defaults to 1001 (indicates server + shutting down). """ - # Close all Websockets with code 1001 - self.close_all(close_code=1001) + # Close all Websockets with given close code + self.close_all(close_code) # Set `_stopped` to `True` to ignore future calls to `add()` self._stopped = True diff --git a/src/docprovider/yprovider.ts b/src/docprovider/yprovider.ts index 0496bb7..159972d 100644 --- a/src/docprovider/yprovider.ts +++ b/src/docprovider/yprovider.ts @@ -145,12 +145,18 @@ export class WebSocketProvider implements IDocumentProvider { // Handle close events based on code const close_code = event.code; - // 4000 := server close code on out-of-band change + // 4000 := indicates out-of-band change if (close_code === 4000) { this._handleOobChange(); return; } + // 4001 := indicates out-of-band move/deletion + if (close_code === 4001) { + this._handleOobMove(); + return; + } + // If the close code is unhandled, log an error to the browser console and // show a popup asking user to refresh the page. console.error('WebSocket connection was closed. Close event: ', event); @@ -166,9 +172,8 @@ export class WebSocketProvider implements IDocumentProvider { }; /** - * Handles an out-of-band change that requires reseting the YDoc before - * re-connecting. The server extension indicates this by closing the YRoom - * Websocket connection with close code 4000. + * Handles an out-of-band change indicated by close code 4000. This requires + * resetting the YDoc, re-connecting, then emitting a notification to the user. */ private _handleOobChange() { // Reset YDoc @@ -186,6 +191,18 @@ export class WebSocketProvider implements IDocumentProvider { ); } + /** + * Handles an out-of-band move/deletion indicated by close code 4001. This + * requires closing the affected JupyterLab tab containing the YDoc, then + * emitting a notification to the user. + */ + private _handleOobMove() { + this._sharedModel.dispose(); + Notification.info('This file has been moved/deleted on disk.', { + autoClose: false + }); + } + private _onSync = (isSynced: boolean) => { if (isSynced) { if (this._yWebsocketProvider) { From 3839fc815dd6ad4811bcd72542e187f223b6a18c Mon Sep 17 00:00:00 2001 From: "David L. Qiu" Date: Thu, 5 Jun 2025 15:21:14 -0700 Subject: [PATCH 2/7] fix #96, close tab on out-of-band move/delete --- src/docprovider/filebrowser.ts | 1 + src/docprovider/ydrive.ts | 5 +++ src/docprovider/yprovider.ts | 73 +++++++++++++++++++++++++++++----- 3 files changed, 70 insertions(+), 9 deletions(-) diff --git a/src/docprovider/filebrowser.ts b/src/docprovider/filebrowser.ts index 39d0584..cf2cb8a 100644 --- a/src/docprovider/filebrowser.ts +++ b/src/docprovider/filebrowser.ts @@ -66,6 +66,7 @@ export const rtcContentProvider: JupyterFrontEndPlugin { @@ -212,6 +264,7 @@ export class WebSocketProvider implements IDocumentProvider { } }; + private _app: JupyterFrontEnd; private _contentType: string; private _format: string; private _isDisposed: boolean; @@ -219,9 +272,6 @@ export class WebSocketProvider implements IDocumentProvider { private _ready = new PromiseDelegate(); private _serverUrl: string; private _sharedModel: YDocument; - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - private _sharedModelFactory: ISharedModelFactory; private _yWebsocketProvider: YWebsocketProvider | null; private _trans: TranslationBundle; } @@ -234,6 +284,11 @@ export namespace WebSocketProvider { * The instantiation options for a WebSocketProvider. */ export interface IOptions { + /** + * The top-level application. Used to close document tabs when the file was + * deleted. + */ + app: JupyterFrontEnd; /** * The server URL */ From d9088843c2c9841a5fbb2b95ae1d6613d7569020 Mon Sep 17 00:00:00 2001 From: "David L. Qiu" Date: Thu, 5 Jun 2025 15:21:14 -0700 Subject: [PATCH 3/7] add callback to delete rooms when stopped --- jupyter_server_documents/rooms/yroom.py | 50 ++++++++++++++----- .../rooms/yroom_manager.py | 12 ++++- 2 files changed, 49 insertions(+), 13 deletions(-) diff --git a/jupyter_server_documents/rooms/yroom.py b/jupyter_server_documents/rooms/yroom.py index 08ef888..14d17fc 100644 --- a/jupyter_server_documents/rooms/yroom.py +++ b/jupyter_server_documents/rooms/yroom.py @@ -66,6 +66,11 @@ class YRoom: _ydoc_subscription: pycrdt.Subscription """Subscription to YDoc changes.""" + _on_stop: callable[[], Any] | None + """ + Callback to run after stopping, provided in the constructor. + """ + _fileid_manager: BaseFileIdManager _contents_manager: AsyncContentsManager | ContentsManager @@ -78,6 +83,7 @@ def __init__( loop: asyncio.AbstractEventLoop, fileid_manager: BaseFileIdManager, contents_manager: AsyncContentsManager | ContentsManager, + on_stop: callable[[], Any] | None = None ): # Bind instance attributes self.room_id = room_id @@ -85,6 +91,7 @@ def __init__( self._loop = loop self._fileid_manager = fileid_manager self._contents_manager = contents_manager + self._on_stop = on_stop # Initialize YjsClientGroup, YDoc, YAwareness, JupyterYDoc self._client_group = YjsClientGroup(room_id=room_id, log=self.log, loop=self._loop) @@ -607,16 +614,27 @@ def reload_ydoc(self) -> None: def handle_outofband_move(self) -> None: """ - Handles an out-of-band move/deletion by stopping the YRoom immediately. - This is similar to `self.stop()` with some key differences: + Handles an out-of-band move/deletion by stopping the YRoom immediately + with close code 4001. + """ + self.stop_immediately(close_code=4001) + + + def stop_immediately(self, close_code: int) -> None: + """ + Stops the YRoom immediately, closing all Websockets with the given + `close_code`. This is similar to `self.stop()` with some key + differences: - - This closes all WS connections with close code 4001, a special close - code reserved to indicate out-of-band move/deletion. - This does not apply any pending YDoc updates from other clients. - - This does not save the file before exiting (as the new path is unknown). + - This does not save the file before exiting. + + This should be reserved for scenarios where it does not make sense to + apply pending updates or save the file, e.g. when the file has been + deleted from disk. """ - # Disconnect all clients with close code 4001 - self.clients.stop(close_code=4001) + # Disconnect all clients with given `close_code` + self.clients.stop(close_code=close_code) # Remove all observers self._ydoc.unobserve(self._ydoc_subscription) @@ -630,14 +648,19 @@ def handle_outofband_move(self) -> None: # Enqueue `None` to stop the `_process_message_queue()` background task self._message_queue.put_nowait(None) - # Finally, stop FileAPI immediately (without saving) and return. + # Stop FileAPI immediately (without saving) if self.file_api: self.file_api.stop() + # Finally, run the provided callback (if any) and return + if self._on_stop: + self._on_stop() + async def stop(self) -> None: """ - Stops the YRoom gracefully. + Stops the YRoom gracefully by disconnecting all clients with close code + 1001, applying all pending updates, and saving the YDoc before exiting. """ # First, disconnect all clients by stopping the client group. self.clients.stop() @@ -648,16 +671,19 @@ async def stop(self) -> None: if self._jupyter_ydoc: self._jupyter_ydoc.unobserve() - # Finish processing all messages, then stop the queue to end the + # Finish processing all messages, then enqueue `None` to stop the # `_process_message_queue()` background task. await self._message_queue.join() self._message_queue.put_nowait(None) - # Finally, stop FileAPI and return. This saves the final content of the - # JupyterYDoc in the process. + # Stop FileAPI, saving the content before doing so if self.file_api: await self.file_api.stop_then_save() + # Finally, run the provided callback (if any) and return + if self._on_stop: + self._on_stop() + def should_ignore_state_update(event: pycrdt.MapEvent) -> bool: """ Returns whether an update to the `state` dictionary should be ignored by diff --git a/jupyter_server_documents/rooms/yroom_manager.py b/jupyter_server_documents/rooms/yroom_manager.py index 4d0b48d..27fb876 100644 --- a/jupyter_server_documents/rooms/yroom_manager.py +++ b/jupyter_server_documents/rooms/yroom_manager.py @@ -54,6 +54,7 @@ def get_room(self, room_id: str) -> YRoom | None: loop=self.loop, fileid_manager=self.fileid_manager, contents_manager=self.contents_manager, + on_stop=lambda: self._handle_yroom_stop(room_id), ) self._rooms_by_id[room_id] = yroom return yroom @@ -63,7 +64,16 @@ def get_room(self, room_id: str) -> YRoom | None: exc_info=True ) return None - + + + def _handle_yroom_stop(self, room_id: str) -> None: + """ + Callback that is run when the YRoom is stopped. This ensures the room is + removed from the dictionary for garbage collection, even if the room was + stopped directly without `YRoomManager.delete_room()`. + """ + self._rooms_by_id.pop(room_id, None) + async def delete_room(self, room_id: str) -> None: """ From 54a95a13d7331217fc52cacf23ce0187c1efb09b Mon Sep 17 00:00:00 2001 From: "David L. Qiu" Date: Thu, 5 Jun 2025 15:21:14 -0700 Subject: [PATCH 4/7] fix #101, handle in-band moves & deletions --- jupyter_server_documents/rooms/yroom.py | 14 +++++- .../rooms/yroom_file_api.py | 41 ++++++++--------- src/docprovider/yprovider.ts | 46 ++++++++++++++----- 3 files changed, 66 insertions(+), 35 deletions(-) diff --git a/jupyter_server_documents/rooms/yroom.py b/jupyter_server_documents/rooms/yroom.py index 14d17fc..a085c8b 100644 --- a/jupyter_server_documents/rooms/yroom.py +++ b/jupyter_server_documents/rooms/yroom.py @@ -114,7 +114,8 @@ def __init__( fileid_manager=self._fileid_manager, contents_manager=self._contents_manager, on_outofband_change=self.reload_ydoc, - on_outofband_move=self.handle_outofband_move + on_outofband_move=self.handle_outofband_move, + on_inband_deletion=self.handle_inband_deletion ) # Load the YDoc content after initializing @@ -598,7 +599,8 @@ def reload_ydoc(self) -> None: fileid_manager=self._fileid_manager, contents_manager=self._contents_manager, on_outofband_change=self.reload_ydoc, - on_outofband_move=self.handle_outofband_move + on_outofband_move=self.handle_outofband_move, + on_inband_deletion=self.handle_inband_deletion ) self.file_api.load_ydoc_content() @@ -619,6 +621,14 @@ def handle_outofband_move(self) -> None: """ self.stop_immediately(close_code=4001) + + def handle_inband_deletion(self) -> None: + """ + Handles an in-band file deletion by stopping the YRoom immediately with + close code 4002. + """ + self.stop_immediately(close_code=4002) + def stop_immediately(self, close_code: int) -> None: """ diff --git a/jupyter_server_documents/rooms/yroom_file_api.py b/jupyter_server_documents/rooms/yroom_file_api.py index ff24aa0..494e163 100644 --- a/jupyter_server_documents/rooms/yroom_file_api.py +++ b/jupyter_server_documents/rooms/yroom_file_api.py @@ -61,16 +61,17 @@ class YRoomFileAPI: _on_outofband_change: Callable[[], Any] """ - The callback to run when an out-of-band change is detected by this instance. - This attribute is only set in the constructor. See `YRoom` for details on - how out-of-band changes are handled. + The callback to run when an out-of-band file change is detected. """ _on_outofband_move: Callable[[], Any] """ - The callback to run when an out-of-band move/deletion is detected by this - instance. This attribute is only set in the constructor. See `YRoom` for - details on how out-of-band changes are handled. + The callback to run when an out-of-band file move/deletion is detected. + """ + + _on_inband_deletion: Callable[[], Any] + """ + The callback to run when an in-band move file deletion is detected. """ _save_loop_task: asyncio.Task @@ -85,7 +86,8 @@ def __init__( contents_manager: AsyncContentsManager | ContentsManager, loop: asyncio.AbstractEventLoop, on_outofband_change: Callable[[], Any], - on_outofband_move: Callable[[], Any] + on_outofband_move: Callable[[], Any], + on_inband_deletion: Callable[[], Any] ): # Bind instance attributes self.room_id = room_id @@ -97,6 +99,7 @@ def __init__( self._contents_manager = contents_manager self._on_outofband_change = on_outofband_change self._on_outofband_move = on_outofband_move + self._on_inband_deletion = on_inband_deletion self._save_scheduled = False self._last_path = None self._last_modified = None @@ -209,7 +212,7 @@ async def _watch_file(self) -> None: while True: try: await asyncio.sleep(0.5) - await self._check_oob_changes() + await self._check_file() if self._save_scheduled: # `asyncio.shield()` prevents the save task from being # cancelled halfway and corrupting the file. We need to @@ -233,17 +236,15 @@ async def _watch_file(self) -> None: f"for YRoom '{self.room_id}'." ) - async def _check_oob_changes(self): + async def _check_file(self): """ - TODO: rename this _validate_file_integrity()? - - Checks for out-of-band operations in the `self._watch_file()` background - task. This is guaranteed to always run before each save in - `self._watch_file()` This detects the following events and acts in - response: + Checks for in-band/out-of-band file operations in the + `self._watch_file()` background task. This is guaranteed to always run + before each save in `self._watch_file()` This detects the following + events and acts in response: - - In-band move: calls `self._on_inband_move()` (TODO) - - In-band deletion: calls `self._on_inband_deletion()` (TODO) + - In-band move: logs warning (no handling needed) + - In-band deletion: calls `self._on_inband_deletion()` - Out-of-band move/deletion: calls `self._on_outofband_move()` - Out-of-band change: calls `self._on_outofband_change()` """ @@ -264,17 +265,13 @@ async def _check_oob_changes(self): f"Room ID: '{self.room_id}', " f"Last known path: '{self._last_path}'." ) - # TODO - # self._on_inband_move() - pass else: self.log.warning( "File was deleted. " f"Room ID: '{self.room_id}', " f"Last known path: '{self._last_path}'." ) - # TODO - # self._on_inband_delete() + self._on_inband_deletion() return # Otherwise, set the last known path diff --git a/src/docprovider/yprovider.ts b/src/docprovider/yprovider.ts index 82f2e61..5f74ead 100644 --- a/src/docprovider/yprovider.ts +++ b/src/docprovider/yprovider.ts @@ -197,6 +197,12 @@ export class WebSocketProvider implements IDocumentProvider { return; } + // 4002 := indicates in-band deletion + if (close_code === 4002) { + this._handleIbDeletion(); + return; + } + // If the close code is unhandled, log an error to the browser console and // show a popup asking user to refresh the page. console.error('WebSocket connection was closed. Close event: ', event); @@ -234,24 +240,42 @@ export class WebSocketProvider implements IDocumentProvider { /** * Handles an out-of-band move/deletion indicated by close code 4001. * - * - If the parent document widget is open, then this closes the tab, emits a - * warning notification to the user, and disposes the shared model to halt - * reconnection. + * This always stops the provider from reconnecting. If the parent document + * widget is open, this method also closes the tab and emits a warning + * notification to the user. * - * - Otherwise, this method just disposes the shared model. The user does not - * need to be notified if the document isn't open. + * No notification is emitted if the document isn't open, since the user does + * not need to be notified. */ private _handleOobMove() { + this._stopCloseAndNotify( + `The file at '${this._path}' no longer exists, and was either moved or deleted. The document tab has been closed.` + ); + } + + /** + * Handles an in-band deletion indicated by close code 4002. This behaves + * similarly to `_handleOobMove()`, but with a different notification message. + */ + private _handleIbDeletion() { + this._stopCloseAndNotify( + `The file at '${this._path}' was deleted. The document tab has been closed.` + ); + } + + /** + * Stops the provider from reconnecting. If the parent document widget is + * open, this method also closes the tab and emits a warning notification to + * the user with the given message. + */ + private _stopCloseAndNotify(message: string) { this._sharedModel.dispose(); const documentWidget = this.parentDocumentWidget; if (documentWidget) { documentWidget.close(); - Notification.warning( - `The file at '${this._path}' no longer exists, and was either moved or deleted. The document tab has been closed.`, - { - autoClose: 5000 - } - ); + Notification.warning(message, { + autoClose: 10000 + }); } } From 7456b2c7d1de1e72f12176867ea5f51eb4e96130 Mon Sep 17 00:00:00 2001 From: "David L. Qiu" Date: Thu, 5 Jun 2025 15:21:14 -0700 Subject: [PATCH 5/7] fix #98, only notify on OOB change if doc is open & visible --- src/docprovider/yprovider.ts | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/docprovider/yprovider.ts b/src/docprovider/yprovider.ts index 5f74ead..b2b394a 100644 --- a/src/docprovider/yprovider.ts +++ b/src/docprovider/yprovider.ts @@ -219,7 +219,8 @@ export class WebSocketProvider implements IDocumentProvider { /** * Handles an out-of-band change indicated by close code 4000. This requires - * resetting the YDoc, re-connecting, then emitting a notification to the user. + * resetting the YDoc and re-connecting. A notification is emitted to the user + * if the document widget containing the shared model is open & visible. */ private _handleOobChange() { // Reset YDoc @@ -227,14 +228,20 @@ export class WebSocketProvider implements IDocumentProvider { const sharedModel = this._sharedModel as YFile | YNotebook; sharedModel.reset(); - // Re-connect and display a notification to the user + // Re-connect this.reconnect(); - Notification.info( - 'The contents of this file were changed on disk. The document state has been reset.', - { - autoClose: false - } - ); + + // Emit notification if document is open & visible to the user (i.e. the tab + // exists & the content of that tab is being shown) + const docWidget = this.parentDocumentWidget; + if (docWidget && docWidget.isVisible) { + Notification.info( + `The file '${this._path}' was modified on disk. The document tab has been reset.`, + { + autoClose: 10000 + } + ); + } } /** @@ -249,7 +256,7 @@ export class WebSocketProvider implements IDocumentProvider { */ private _handleOobMove() { this._stopCloseAndNotify( - `The file at '${this._path}' no longer exists, and was either moved or deleted. The document tab has been closed.` + `The file '${this._path}' no longer exists, and was either moved or deleted. The document tab has been closed.` ); } @@ -259,7 +266,7 @@ export class WebSocketProvider implements IDocumentProvider { */ private _handleIbDeletion() { this._stopCloseAndNotify( - `The file at '${this._path}' was deleted. The document tab has been closed.` + `The file '${this._path}' was deleted. The document tab has been closed.` ); } From 73ff17f2e0510a9dfcc83a31482dbc35d6fd6e40 Mon Sep 17 00:00:00 2001 From: "David L. Qiu" Date: Fri, 6 Jun 2025 11:19:07 -0700 Subject: [PATCH 6/7] fix OOB change after IB move, thanks @3coins --- .../rooms/yroom_file_api.py | 2 +- src/docprovider/yprovider.ts | 46 ++++++++++++++++--- 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/jupyter_server_documents/rooms/yroom_file_api.py b/jupyter_server_documents/rooms/yroom_file_api.py index 494e163..c257191 100644 --- a/jupyter_server_documents/rooms/yroom_file_api.py +++ b/jupyter_server_documents/rooms/yroom_file_api.py @@ -155,7 +155,6 @@ def load_ydoc_content(self) -> None: if self._ydoc_content_loaded.is_set() or self._ydoc_content_loading: return - self.log.info(f"Loading content for room ID '{self.room_id}'.") self._ydoc_content_loading = True self._loop.create_task(self._load_ydoc_content()) @@ -168,6 +167,7 @@ async def _load_ydoc_content(self) -> None: self._last_path = path # Load the content of the file from the path + self.log.info(f"Loading content for room ID '{self.room_id}', found at path: '{path}'.") file_data = await ensure_async(self._contents_manager.get( path, type=self.file_type, diff --git a/src/docprovider/yprovider.ts b/src/docprovider/yprovider.ts index b2b394a..f9c1a79 100644 --- a/src/docprovider/yprovider.ts +++ b/src/docprovider/yprovider.ts @@ -140,16 +140,49 @@ export class WebSocketProvider implements IDocumentProvider { this._connect(); } + /** + * Gets the file ID for this path. This should only be called once when the + * provider connects for the first time, because any future in-band moves may + * cause `this._path` to not refer to the correct file. + */ + private async _getFileId(): Promise { + let fileId: string | null = null; + try { + const resp = await requestAPI(`api/fileid/index?path=${this._path}`, { + method: 'POST' + }); + if (resp && 'id' in resp && typeof resp['id'] === 'string') { + fileId = resp['id']; + } + } catch (e) { + console.error(`Could not get file ID for path '${this._path}'.`); + return null; + } + return fileId; + } + private async _connect(): Promise { - // Fetch file ID from the file ID service. - const resp = await requestAPI(`api/fileid/index?path=${this._path}`, { - method: 'POST' - }); - const fileId: string = resp['id']; + // Fetch file ID from the file ID service, if not cached + if (!this._fileId) { + this._fileId = await this._getFileId(); + } + + // If file ID could not be retrieved, show an error dialog asking for a bug + // report, as this error is irrecoverable. + if (!this._fileId) { + showErrorMessage( + this._trans.__('File ID error'), + `The file '${this._path}' cannot be opened because its file ID could not be retrieved. Please report this issue on GitHub.`, + [Dialog.okButton()] + ); + return; + } + // Otherwise, initialize the `YWebsocketProvider` to connect + console.log({ fileId: this._fileId }); this._yWebsocketProvider = new YWebsocketProvider( this._serverUrl, - `${this._format}:${this._contentType}:${fileId}`, + `${this._format}:${this._contentType}:${this._fileId}`, this._sharedModel.ydoc, { disableBc: true, @@ -305,6 +338,7 @@ export class WebSocketProvider implements IDocumentProvider { private _sharedModel: YDocument; private _yWebsocketProvider: YWebsocketProvider | null; private _trans: TranslationBundle; + private _fileId: string | null = null; } /** From c7a441105fea36437f18dd779dc32b69d0ddb567 Mon Sep 17 00:00:00 2001 From: "David L. Qiu" Date: Fri, 6 Jun 2025 11:23:44 -0700 Subject: [PATCH 7/7] remove console log --- src/docprovider/yprovider.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/docprovider/yprovider.ts b/src/docprovider/yprovider.ts index f9c1a79..194376b 100644 --- a/src/docprovider/yprovider.ts +++ b/src/docprovider/yprovider.ts @@ -179,7 +179,6 @@ export class WebSocketProvider implements IDocumentProvider { } // Otherwise, initialize the `YWebsocketProvider` to connect - console.log({ fileId: this._fileId }); this._yWebsocketProvider = new YWebsocketProvider( this._serverUrl, `${this._format}:${this._contentType}:${this._fileId}`,