fix: do not write lazyjson twice when in context manager (#5268)

* fix: do not write lazyjson twice when in context manager

* tests: simpler use of tempdir

* test: more test simplification

* test: add test for double write

* test: ensure data is written

Author: beckermr

conda_forge_tick/lazy_json_backends.py

@@ -1023,8 +1023,15 @@ def _load(self) -> None:
                 data_str = file_backend.hget(self.hashmap, self.node)
             else:
                 backend = LAZY_JSON_BACKENDS[CF_TICK_GRAPH_DATA_PRIMARY_BACKEND]()
-                backend.hsetnx(self.hashmap, self.node, dumps({}))
-                data_str = backend.hget(self.hashmap, self.node)
+                if backend.hexists(self.hashmap, self.node):
+                    data_str = backend.hget(self.hashmap, self.node)
+                else:
+                    data_str = dumps({})
+                    if not self._in_context:
+                        # need to push file to backend since will not get
+                        # done at end of context manager
+                        backend.hset(self.hashmap, self.node, data_str)
+
                 if isinstance(data_str, bytes):  # type: ignore[unreachable]
                     data_str = data_str.decode("utf-8")  # type: ignore[unreachable]
 

tests/test_lazy_json_backends.py

@@ -945,3 +945,29 @@ def _sleep():
                         raise e
                     else:
                         pass
+
+
+def test_lazy_json_backends_contexts_double_write():
+    with tempfile.TemporaryDirectory() as tmpdir, pushd(tmpdir):
+        with lazy_json_override_backends(["file"], use_file_cache=False):
+            lzj = LazyJson("test.json")
+            # old behavior always makes a file on init
+            # we remove it here so that we can test
+            assert os.path.exists(lzj.sharded_path)
+            os.remove(lzj.sharded_path)
+
+            with lzj:
+                # file should not yet exist
+                assert not os.path.exists(lzj.sharded_path)
+
+                # even if we get the data, the file should not yet exist
+                assert lzj.data == {}
+                assert not os.path.exists(lzj.sharded_path)
+
+                # setting data doesn't make the file
+                lzj["hi"] = "world"
+                assert not os.path.exists(lzj.sharded_path)
+
+            # now the file exists at the end of the context block
+            assert os.path.exists(lzj.sharded_path)
+            assert lzj.data == {"hi": "world"}

tests/test_utils.py

@@ -1,11 +1,14 @@
 import contextlib
+import tempfile
 import textwrap
 from io import StringIO
 from unittest import mock
 from unittest.mock import MagicMock, mock_open
 
 import pytest
 
+from conda_forge_tick.lazy_json_backends import LazyJson
+from conda_forge_tick.os_utils import pushd
 from conda_forge_tick.utils import (
     DEFAULT_GRAPH_FILENAME,
     _munge_dict_repr,
@@ -93,24 +96,26 @@ def test_get_keys_default_none():
 
 
 def test_load_graph():
-    with mock.patch("builtins.open", mock_open(read_data=DEMO_GRAPH)) as mock_file:
+    with tempfile.TemporaryDirectory() as tmpdir, pushd(tmpdir):
+        with open(LazyJson(DEFAULT_GRAPH_FILENAME).sharded_path, "w") as fp:
+            fp.write(DEMO_GRAPH)
+
         gx = load_graph()
 
         assert gx is not None
 
         assert gx.nodes.keys() == {"package1", "package2"}
 
-    mock_file.assert_has_calls([mock.call(DEFAULT_GRAPH_FILENAME)])
-
 
 def test_load_graph_empty_graph():
-    with mock.patch("builtins.open", mock_open(read_data=EMPTY_JSON)) as mock_file:
+    with tempfile.TemporaryDirectory() as tmpdir, pushd(tmpdir):
+        with open(LazyJson(DEFAULT_GRAPH_FILENAME).sharded_path, "w") as fp:
+            fp.write(EMPTY_JSON)
+
         gx = load_graph()
 
         assert gx is None
 
-    mock_file.assert_has_calls([mock.call(DEFAULT_GRAPH_FILENAME)])
-
 
 @mock.patch("os.path.exists")
 def test_load_graph_file_does_not_exist(exists_mock: MagicMock):
@@ -122,24 +127,24 @@ def test_load_graph_file_does_not_exist(exists_mock: MagicMock):
     mock_file.assert_has_calls([mock.call(DEFAULT_GRAPH_FILENAME, "w")])
 
 
-@mock.patch("os.path.isfile")
-def test_load_existing_graph(isfile_mock: MagicMock):
-    isfile_mock.return_value = True
-    with mock.patch("builtins.open", mock_open(read_data=DEMO_GRAPH)) as mock_file:
+def test_load_existing_graph():
+    with tempfile.TemporaryDirectory() as tmpdir, pushd(tmpdir):
+        with open(LazyJson(DEFAULT_GRAPH_FILENAME).sharded_path, "w") as fp:
+            fp.write(DEMO_GRAPH)
+
         gx = load_existing_graph()
 
         assert gx.nodes.keys() == {"package1", "package2"}
 
-    mock_file.assert_has_calls([mock.call(DEFAULT_GRAPH_FILENAME)])
-
 
 def test_load_existing_graph_empty_graph():
-    with mock.patch("builtins.open", mock_open(read_data=EMPTY_JSON)) as mock_file:
+    with tempfile.TemporaryDirectory() as tmpdir, pushd(tmpdir):
+        with open(LazyJson(DEFAULT_GRAPH_FILENAME).sharded_path, "w") as fp:
+            fp.write(EMPTY_JSON)
+
         with pytest.raises(ValueError, match="empty JSON"):
             load_existing_graph()
 
-    mock_file.assert_has_calls([mock.call(DEFAULT_GRAPH_FILENAME)])
-
 
 @mock.patch("os.path.exists")
 def test_load_existing_graph_file_does_not_exist(exists_mock: MagicMock):