Browse Source

_write_event: document fix in changelog; re-block target path without filename; replace shutil.copy + os.remove with shutil.move

Fabian Peter Hammerle 2 years ago
parent
commit
634ce5672e
3 changed files with 24 additions and 8 deletions
  1. 4 0
      CHANGELOG.md
  2. 3 3
      ical2vdir/__init__.py
  3. 17 5
      tests/vdir_test.py

+ 4 - 0
CHANGELOG.md

@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Removed
 - compatibility with `python3.5` & `python3.6`
 
+### Fixed
+- fix export when output and temporary directory are on different filesystems
+  (https://github.com/fphammerle/ical2vdir/pull/85)
+
 ## [0.1.2] - 2020-06-18
 ### Fixed
 - python3.5:

+ 3 - 3
ical2vdir/__init__.py

@@ -93,6 +93,8 @@ def _event_vdir_filename(event: icalendar.cal.Event) -> str:
 
 
 def _write_event(event: icalendar.cal.Event, path: pathlib.Path) -> None:
+    if path.is_dir():
+        raise IsADirectoryError(path)  # similar to os.rename
     # > Creating and modifying items or metadata files should happen atomically.
     # https://vdirsyncer.readthedocs.io/en/stable/vdir.html#writing-to-vdirs
     temp_fd, temp_path = tempfile.mkstemp(
@@ -104,9 +106,7 @@ def _write_event(event: icalendar.cal.Event, path: pathlib.Path) -> None:
         # https://tools.ietf.org/html/rfc5545#section-3.1
         os.write(temp_fd, event.to_ical())
         os.close(temp_fd)
-        # python3.5 expects Union[bytes, str]
-        shutil.copy(temp_path, str(path))
-        os.remove(temp_path)
+        shutil.move(temp_path, path)
     finally:
         if os.path.exists(temp_path):
             os.unlink(temp_path)

+ 17 - 5
tests/vdir_test.py

@@ -18,6 +18,7 @@
 import copy
 import os
 import pathlib
+import tempfile
 import unittest.mock
 
 import icalendar.cal
@@ -55,12 +56,23 @@ END:VEVENT
 
 def test__write_event_cleanup(tmp_path: pathlib.Path) -> None:
     event = icalendar.cal.Event.from_ical(_SINGLE_EVENT_ICAL)
-    with unittest.mock.patch("os.unlink") as unlink_mock:
-        with pytest.raises(IsADirectoryError):
-            ical2vdir._write_event(event, tmp_path)
-    unlink_mock.assert_called_once()
+    with pytest.raises(IsADirectoryError):
+        ical2vdir._write_event(event, tmp_path)
+    assert tmp_path.is_dir()  # did not overwrite
+
+
+def test__write_event_move_failed(tmp_path: pathlib.Path) -> None:
+    event = icalendar.cal.Event.from_ical(_SINGLE_EVENT_ICAL)
+    output_path = tmp_path.joinpath("test.ics")
+    with unittest.mock.patch("os.unlink") as unlink_mock, unittest.mock.patch(
+        "shutil.move", side_effect=Exception("test")
+    ), pytest.raises(Exception, match=r"^test$"):
+        ical2vdir._write_event(event, output_path)
+    assert not output_path.exists()
+    unlink_mock.assert_called_once()  # cleanup temporary file
     unlink_args, _ = unlink_mock.call_args
-    os.unlink(unlink_args[0])
+    (temp_path,) = unlink_args
+    assert os.path.dirname(temp_path) == os.path.dirname(tempfile.mkdtemp())
 
 
 @pytest.mark.parametrize(