From 1c963a567c99150a7bd1f9d70bcffe5b83efde6f Mon Sep 17 00:00:00 2001
From: Fantix King <fantix.king@gmail.com>
Date: Thu, 23 May 2024 10:01:15 -0400
Subject: [PATCH] server: fix again monitor_fs() to track target recreation
 (#7379)

This is now using a platform-agnostic approach. The feature was initially
added in #7286 and broken by #7299.
---
 edb/server/server.py     | 41 +++++++++++++++++++++-------------------
 tests/test_server_ops.py | 16 ++++++++++++++++
 2 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/edb/server/server.py b/edb/server/server.py
index e3dcaf57cc5..b53b0f66b94 100644
--- a/edb/server/server.py
+++ b/edb/server/server.py
@@ -370,55 +370,58 @@ def cancel_pgext_connection(self, pid, secret):
 
     def monitor_fs(
         self,
-        path: str | pathlib.Path,
+        file_path: str | pathlib.Path,
         cb: Callable[[], None],
     ) -> Callable[[], None]:
         if not self._use_monitor_fs:
             return lambda: None
 
+        if isinstance(file_path, str):
+            path = pathlib.Path(file_path)
+            path_str = file_path
+        else:
+            path = file_path
+            path_str = str(file_path)
         handle = None
-        parent_dir = pathlib.Path(path).parent
+        parent_dir = path.parent
 
         def watch_dir(file_modified, _event):
             nonlocal handle
-            if parent_dir / os.fsdecode(file_modified) == pathlib.Path(path):
+            if parent_dir / os.fsdecode(file_modified) == path:
                 try:
                     new_handle = self.__loop._monitor_fs(  # type: ignore
-                        str(path), callback)
+                        path_str, callback)
                 except FileNotFoundError:
                     pass
                 else:
                     finalizer()
                     handle = new_handle
                     self._file_watch_handles.append(handle)
+                    cb()
 
-        def callback(file_modified, event):
+        def callback(_file_modified, _event):
             nonlocal handle
-            if event == 2:  # CHANGE
-                cb()
-            elif (
-                event == 1 and  # RENAME - macOS issues this event for CHANGE
-                parent_dir / os.fsdecode(file_modified) == pathlib.Path(path)
-            ):
+            # First, cancel the existing watcher and call cb() regardless of
+            # what event it is. This is because macOS issues RENAME while Linux
+            # issues CHANGE, and we don't have enough knowledge about renaming.
+            # The idea here is to re-watch the file path after every event, so
+            # that even if the file is recreated, we still watch the right one.
+            finalizer()
+            try:
                 cb()
-            elif event == 1 or event == 3:  # RENAME, RENAME_CHANGE
-                # File is likely renamed or deleted, stop watching
-                finalizer()
+            finally:
                 try:
                     # Then, see if we can directly re-watch the target path
                     handle = self.__loop._monitor_fs(  # type: ignore
-                        str(path), callback)
+                        path_str, callback)
                 except FileNotFoundError:
                     # If not, watch the parent directory to wait for recreation
                     handle = self.__loop._monitor_fs(  # type: ignore
                         str(parent_dir), watch_dir)
                 self._file_watch_handles.append(handle)
-            else:
-                # Unknown events are ignored
-                pass
 
         # ... we depend on an event loop internal _monitor_fs
-        handle = self.__loop._monitor_fs(str(path), callback)  # type: ignore
+        handle = self.__loop._monitor_fs(path_str, callback)  # type: ignore
 
         def finalizer():
             try:
diff --git a/tests/test_server_ops.py b/tests/test_server_ops.py
index 1934ee43a28..8657bb1dab7 100644
--- a/tests/test_server_ops.py
+++ b/tests/test_server_ops.py
@@ -901,6 +901,22 @@ async def test_server_ops_readiness(self):
 
                         conn = await sd.connect()
                         await conn.aclose()
+
+                # Re-create the file, the server should pick it up
+                rf = open(rf_name, "w")
+                print("not_ready", file=rf, flush=True)
+                await sd.connect()
+                async for tr in self.try_until_succeeds(
+                    ignore=(errors.AccessError, AssertionError),
+                ):
+                    async with tr:
+                        with self.http_con(server=sd) as http_con:
+                            _, _, status = self.http_con_request(
+                                http_con,
+                                path='/server/status/ready',
+                            )
+                            self.assertEqual(
+                                status, http.HTTPStatus.SERVICE_UNAVAILABLE)
         finally:
             if os.path.exists(rf_name):
                 rf.close()