From 123d47cd5b595d4a4344b72e4b655533c66d58ef Mon Sep 17 00:00:00 2001
From: David Leen <dleen@netflix.com>
Date: Wed, 23 Mar 2022 18:09:05 -0700
Subject: [PATCH] Handle importstring pre/post save hooks

Only treat a hook as a replacement if the existing hook is callable. Otherwise
treat it as a new hook.

Closes #753
---
 jupyter_server/services/contents/manager.py |  4 ++--
 tests/services/contents/test_config.py      | 24 +++++++++++++++++++++
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/jupyter_server/services/contents/manager.py b/jupyter_server/services/contents/manager.py
index cd4ce80fbf..2a2cf0c4b7 100644
--- a/jupyter_server/services/contents/manager.py
+++ b/jupyter_server/services/contents/manager.py
@@ -127,7 +127,7 @@ def _validate_pre_save_hook(self, proposal):
             value = import_item(self.pre_save_hook)
         if not callable(value):
             raise TraitError("pre_save_hook must be callable")
-        if self.pre_save_hook is not None:
+        if callable(self.pre_save_hook):
             warnings.warn(
                 f"Overriding existing pre_save_hook ({self.pre_save_hook.__name__}) with a new one ({value.__name__}).",
                 stacklevel=2,
@@ -162,7 +162,7 @@ def _validate_post_save_hook(self, proposal):
             value = import_item(value)
         if not callable(value):
             raise TraitError("post_save_hook must be callable")
-        if self.post_save_hook is not None:
+        if callable(self.post_save_hook):
             warnings.warn(
                 f"Overriding existing post_save_hook ({self.post_save_hook.__name__}) with a new one ({value.__name__}).",
                 stacklevel=2,
diff --git a/tests/services/contents/test_config.py b/tests/services/contents/test_config.py
index 741c10c139..02f4d702ca 100644
--- a/tests/services/contents/test_config.py
+++ b/tests/services/contents/test_config.py
@@ -18,6 +18,30 @@ def test_config_did_something(jp_server_config, jp_serverapp):
     )
 
 
+def example_pre_save_hook():
+    pass
+
+
+def example_post_save_hook():
+    pass
+
+
+@pytest.mark.parametrize(
+    "jp_server_config",
+    [
+        {
+            "ContentsManager": {
+                "pre_save_hook": "tests.services.contents.test_config.example_pre_save_hook",
+                "post_save_hook": "tests.services.contents.test_config.example_post_save_hook",
+            },
+        }
+    ],
+)
+def test_pre_post_save_hook_config(jp_serverapp, jp_server_config):
+    assert jp_serverapp.contents_manager.pre_save_hook.__name__ == "example_pre_save_hook"
+    assert jp_serverapp.contents_manager.post_save_hook.__name__ == "example_post_save_hook"
+
+
 async def test_async_contents_manager(jp_configurable_serverapp):
     config = {"ContentsManager": {"checkpoints_class": AsyncCheckpoints}}
     argv = [