Save immediately when triggering manual save (#540)

* Save immediately when triggering manual save

Should fix #537.

I'm not sure between a parameter, and have a different function.
And not quite sure why the require-hash was missing.

* Update projects/jupyter-server-ydoc/jupyter_server_ydoc/test_utils.py

Co-authored-by: Michał Krassowski <5832902+krassowski@users.noreply.github.com>

* reformat with pre-commit

* fix type

* better error message

---------

Co-authored-by: Michał Krassowski <5832902+krassowski@users.noreply.github.com>

Author: Carreau

projects/jupyter-server-ydoc/jupyter_server_ydoc/rooms.py

@@ -278,20 +278,28 @@ def _save_to_disc(self):
             return
 
         self._saving_document = asyncio.create_task(
-            self._maybe_save_document(self._saving_document)
+            self._maybe_save_document(self._saving_document, save_now=True)
         )
         return self._saving_document
 
-    async def _maybe_save_document(self, saving_document: asyncio.Task | None) -> None:
+    async def _maybe_save_document(
+        self, saving_document: asyncio.Task | None, save_now: bool = False
+    ) -> None:
         """
         Saves the content of the document to disk.
 
         ### Note:
             There is a save delay to debounce the save since we could receive a high
             amount of changes in a short period of time. This way we can cancel the
-            previous save.
+            previous save. When save_now is True, the delay is skipped and the save
+            executes immediately.
+
+            Parameters:
+                saving_document: The previous saving task to cancel if needed.
+                save_now: If True, skip the debounce delay, and save immediately.
+                          This is used when manually saving.
         """
-        if self._save_delay is None:
+        if self._save_delay is None and not save_now:
             return
         if saving_document is not None and not saving_document.done():
             # the document is being saved, cancel that
@@ -301,8 +309,10 @@ async def _maybe_save_document(self, saving_document: asyncio.Task | None) -> No
         # because this coroutine is run in a cancellable task and cancellation is handled here
 
         try:
-            # save after X seconds of inactivity
-            await asyncio.sleep(self._save_delay)
+            # When save_now is False, wait X seconds of inactivity before saving (auto-save).
+            # When save_now is True, save immediately without debounce delay (manual save).
+            if not save_now and self._save_delay is not None:
+                await asyncio.sleep(self._save_delay)
 
             self.log.info("Saving the content from room %s", self._room_id)
             saved_model = await self._file.maybe_save_content(

projects/jupyter-server-ydoc/jupyter_server_ydoc/test_utils.py

@@ -33,13 +33,19 @@ def __init__(self, model: dict):
             "mimetype": None,
             "size": 0,
             "writable": False,
+            "hash": "fake_hash",
         }
         self.model.update(model)
 
         self.actions: list[str] = []
 
     def get(
-        self, path: str, content: bool = True, format: str | None = None, type: str | None = None
+        self,
+        path: str,
+        content: bool = True,
+        format: str | None = None,
+        type: str | None = None,
+        require_hash: bool | None = None,
     ) -> dict:
         if not self.model:
             raise HTTPError(404, f"File not found: {path}")

tests/test_documents.py

@@ -69,7 +69,8 @@ async def connect(file_format, file_type, file_path):
         tg.start_soon(connect, file_format, file_type, file_path)
         tg.start_soon(connect, file_format, file_type, file_path)
     t1 = time()
-    assert t1 - t0 < 0.5
+    delta = t1 - t0
+    assert delta < 0.5
 
     await cleanup(jp_serverapp)
 

tests/test_rooms.py

@@ -121,6 +121,109 @@ async def test_should_save_content_when_at_least_one_client_has_autosave_enabled
     assert "save" in cm.actions
 
 
+async def test_manual_save_should_not_have_delay(
+    rtc_create_mock_document_room,
+):
+    content = "test"
+    cm, _, room = rtc_create_mock_document_room("test-id", "test.txt", content, save_delay=0.5)
+
+    await room.initialize()
+
+    # Trigger a manual save
+    room._save_to_disc()
+
+    # Manual save should execute immediately, without waiting for the 0.5s delay
+    # Check that save happens within a very short time (100ms should be enough)
+    await asyncio.sleep(0.1)
+
+    assert cm.actions.count("save") == 1
+
+
+async def test_manual_save_with_pending_autosave_should_cancel_autosave(
+    rtc_create_mock_document_room,
+):
+    content = "test"
+    cm, _, room = rtc_create_mock_document_room("test-id", "test.txt", content, save_delay=1.0)
+
+    await room.initialize()
+
+    room._document.source = "Test 2"
+
+    await asyncio.sleep(0.1)
+
+    assert cm.actions.count("save") == 0
+
+    save_task = room._save_to_disc()
+
+    # Manual save should execute immediately
+    await asyncio.sleep(0.1)
+    assert save_task.done()
+
+    # Check that the manual save was recorded
+    assert cm.actions.count("save") == 1
+
+    await asyncio.sleep(1.0)
+
+    # There should be only one save (the manual one), not two
+    assert cm.actions.count("save") == 1
+
+
+async def test_manual_save_should_execute_immediately_even_with_long_delay(
+    rtc_create_mock_document_room,
+):
+    content = "test"
+    cm, _, room = rtc_create_mock_document_room("test-id", "test.txt", content, save_delay=5.0)
+
+    await room.initialize()
+
+    save_task = room._save_to_disc()
+
+    await asyncio.sleep(0.5)
+
+    assert "save" in cm.actions
+    assert save_task.done()
+
+
+async def test_autosave_should_still_have_delay(
+    rtc_create_mock_document_room,
+):
+    content = "test"
+    save_delay = 0.3
+    cm, _, room = rtc_create_mock_document_room(
+        "test-id", "test.txt", content, save_delay=save_delay
+    )
+
+    await room.initialize()
+
+    room._document.source = "Test 3"
+
+    await asyncio.sleep(0.1)
+    assert "save" not in cm.actions
+
+    # Wait for the delay to complete
+    await asyncio.sleep(save_delay)
+
+    assert "save" in cm.actions
+
+
+async def test_manual_save_should_work_when_save_delay_is_none_and_save_now_is_true(
+    rtc_create_mock_document_room,
+):
+    """Test that manual saves execute even when save_delay is None."""
+    content = "test"
+    # When save_delay is None, autosave is disabled
+    cm, _, room = rtc_create_mock_document_room("test-id", "test.txt", content, save_delay=None)
+
+    await room.initialize()
+
+    # Trigger a manual save with save_now=True
+    # Even though save_delay is None, manual saves should still work
+    await room._maybe_save_document(None, save_now=True)
+
+    # Manual save should have executed
+    assert cm.actions.count("save") == 1
+
+
 # The following test should be restored when package versions are fixed.
 
 # async def test_document_path(rtc_create_mock_document_room):