[BACKEND] Do not write download archive file if no files changed (#161)

* [BACKEND] Do not write download archive file if no files changed

* ensure empty log for playlists

* test dl args

* test main completely

src/ytdl_sub/cli/main.py

@@ -89,14 +89,14 @@ def _download_subscription_from_cli(
     return subscription, subscription.download(dry_run=args.dry_run)
 
 
-def main():
+def main() -> List[Tuple[Subscription, FileHandlerTransactionLog]]:
     """
     Entrypoint for ytdl-sub, without the error handling
     """
     # If no args are provided, print help and exit
     if len(sys.argv) < 2:
         parser.print_help()
-        return
+        return []
 
     args, extra_args = parser.parse_known_args()
 
@@ -112,11 +112,15 @@ def main():
         )
 
     for subscription, transaction_log in transaction_logs:
-        logger.info(
-            "Downloads for %s:\n%s\n",
-            subscription.name,
-            transaction_log.to_output_message(subscription.output_directory),
-        )
+        if transaction_log.is_empty:
+            logger.info("No files changed for %s", subscription.name)
+        else:
+            logger.info(
+                "Downloads for %s:\n%s\n",
+                subscription.name,
+                transaction_log.to_output_message(subscription.output_directory),
+            )
 
     # Ran successfully, so we can delete the debug file
     Logger.cleanup(delete_debug_file=True)
+    return transaction_logs

src/ytdl_sub/utils/file_handler.py

@@ -93,6 +93,15 @@ class FileHandlerTransactionLog:
         self.files_created: Dict[str, FileMetadata] = {}
         self.files_removed: Set[str] = set()
 
+    @property
+    def is_empty(self) -> bool:
+        """
+        Returns
+        -------
+        True if no transaction logs are recorded. False otherwise
+        """
+        return len(self.files_created) == 0 and len(self.files_removed) == 0
+
     def log_created_file(
         self, file_name: str, file_metadata: Optional[FileMetadata] = None
     ) -> "FileHandlerTransactionLog":

src/ytdl_sub/ytdl_additions/enhanced_download_archive.py

@@ -537,14 +537,15 @@ class EnhancedDownloadArchive:
 
     def save_download_mappings(self) -> "EnhancedDownloadArchive":
         """
-        Saves the updated download mappings to the output directory
+        Saves the updated download mappings to the output directory if any files were changed.
 
         Returns
         -------
         self
         """
-        self._download_mapping.to_file(output_json_file=self._mapping_working_file_path)
-        self.save_file_to_output_directory(file_name=self._mapping_file_name)
+        if not self.get_file_handler_transaction_log().is_empty:
+            self._download_mapping.to_file(output_json_file=self._mapping_working_file_path)
+            self.save_file_to_output_directory(file_name=self._mapping_file_name)
         return self
 
     def save_file_to_output_directory(

tests/conftest.py

@@ -1,9 +1,15 @@
 import contextlib
+import json
 import logging
+import tempfile
+from typing import Any
 from typing import Callable
-from unittest.mock import MagicMock
+from typing import Dict
+from typing import List
 from unittest.mock import patch
 
+import pytest
+
 from ytdl_sub.utils.logger import Logger
 
 
@@ -26,3 +32,48 @@ def assert_debug_log(logger: logging.Logger, expected_message: str):
             return
 
     assert False, f"{expected_message} was not found in a logger.debug call"
+
+
+def preset_dict_to_dl_args(preset_dict: Dict) -> str:
+    """
+    Parameters
+    ----------
+    preset_dict
+        Preset dict to convert
+
+    Returns
+    -------
+    Preset dict converted to CLI parameters
+    """
+
+    def _recursive_preset_args(cli_key: str, current_value: Dict | Any) -> List[str]:
+        if isinstance(current_value, dict):
+            preset_args: List[str] = []
+            for v_key, v_value in current_value.items():
+                preset_args.extend(
+                    _recursive_preset_args(
+                        cli_key=f"{cli_key}.{v_key}" if cli_key else v_key, current_value=v_value
+                    )
+                )
+            return preset_args
+        elif isinstance(current_value, list):
+            return [
+                f"--{cli_key}[{idx + 1}] {current_value[idx]}" for idx in range(len(current_value))
+            ]
+        else:
+            return [f"--{cli_key} {current_value}"]
+
+    return " ".join(_recursive_preset_args(cli_key="", current_value=preset_dict))
+
+
+@pytest.fixture
+def preset_dict_to_subscription_yaml_generator() -> Callable:
+    @contextlib.contextmanager
+    def _preset_dict_to_subscription_yaml_generator(subscription_name: str, preset_dict: Dict):
+        subscription_dict = {subscription_name: preset_dict}
+        with tempfile.NamedTemporaryFile(suffix=".yaml") as tmp_file:
+            tmp_file.write(json.dumps(subscription_dict).encode("utf-8"))
+            tmp_file.flush()
+            yield tmp_file.name
+
+    return _preset_dict_to_subscription_yaml_generator

tests/e2e/conftest.py

@@ -1,8 +1,15 @@
+import sys
 import tempfile
+from typing import List
+from typing import Tuple
+from unittest.mock import patch
 
 import pytest
 
+from ytdl_sub.cli.main import main
 from ytdl_sub.config.config_file import ConfigFile
+from ytdl_sub.subscriptions.subscription import Subscription
+from ytdl_sub.utils.file_handler import FileHandlerTransactionLog
 
 
 @pytest.fixture()
@@ -12,8 +19,13 @@ def output_directory():
 
 
 @pytest.fixture()
-def music_video_config():
-    return ConfigFile.from_file_path(config_path="examples/kodi_music_videos_config.yaml")
+def music_video_config_path():
+    return "examples/kodi_music_videos_config.yaml"
+
+
+@pytest.fixture()
+def music_video_config(music_video_config_path):
+    return ConfigFile.from_file_path(config_path=music_video_config_path)
 
 
 @pytest.fixture()
@@ -41,3 +53,9 @@ def timestamps_file_path():
         tmp.writelines(timestamps)
         tmp.seek(0)
         yield tmp.name
+
+
+def mock_run_from_cli(args: str) -> List[Tuple[Subscription, FileHandlerTransactionLog]]:
+    args_list = ["ytdl-sub"] + args.split()
+    with patch.object(sys, "argv", args_list):
+        return main()

tests/e2e/youtube/test_playlist.py

@@ -2,6 +2,7 @@ from pathlib import Path
 
 import pytest
 from conftest import assert_debug_log
+from e2e.conftest import mock_run_from_cli
 from e2e.expected_download import ExpectedDownloadFile
 from e2e.expected_download import ExpectedDownloads
 from e2e.expected_transaction_log import assert_transaction_log_matches
@@ -87,5 +88,47 @@ class TestPlaylistAsKodiMusicVideo:
                 logger=ytdl_sub.downloaders.downloader.download_logger,
                 expected_message="ExistingVideoReached, stopping additional downloads",
             ):
-                playlist_subscription.download()
+                transaction_log = playlist_subscription.download()
                 expected_playlist_download.assert_files_exist(relative_directory=output_directory)
+                assert transaction_log.is_empty
+
+    @pytest.mark.parametrize("dry_run", [True, False])
+    def test_playlist_download_from_cli_sub(
+        self,
+        preset_dict_to_subscription_yaml_generator,
+        music_video_config_path,
+        playlist_preset_dict,
+        expected_playlist_download,
+        output_directory,
+        dry_run,
+    ):
+        with preset_dict_to_subscription_yaml_generator(
+            subscription_name="music_video_playlist_test", preset_dict=playlist_preset_dict
+        ) as subscription_path:
+            args = "--dry-run " if dry_run else ""
+            args += f"--config {music_video_config_path} "
+            args += f"sub {subscription_path}"
+            subscription_transaction_log = mock_run_from_cli(args=args)
+
+            assert len(subscription_transaction_log) == 1
+            transaction_log = subscription_transaction_log[0][1]
+
+            assert_transaction_log_matches(
+                output_directory=output_directory,
+                transaction_log=transaction_log,
+                transaction_log_summary_file_name="youtube/test_playlist.txt",
+            )
+
+            if not dry_run:
+                expected_playlist_download.assert_files_exist(relative_directory=output_directory)
+
+                # Ensure another invocation will hit ExistingVideoReached
+                with assert_debug_log(
+                    logger=ytdl_sub.downloaders.downloader.download_logger,
+                    expected_message="ExistingVideoReached, stopping additional downloads",
+                ):
+                    transaction_log = mock_run_from_cli(args=args)[0][1]
+                    expected_playlist_download.assert_files_exist(
+                        relative_directory=output_directory
+                    )
+                    assert transaction_log.is_empty

tests/e2e/youtube/test_video.py

@@ -1,6 +1,8 @@
 from pathlib import Path
 
 import pytest
+from conftest import preset_dict_to_dl_args
+from e2e.conftest import mock_run_from_cli
 from e2e.expected_download import ExpectedDownloadFile
 from e2e.expected_download import ExpectedDownloads
 from e2e.expected_transaction_log import assert_transaction_log_matches
@@ -8,21 +10,6 @@ from e2e.expected_transaction_log import assert_transaction_log_matches
 from ytdl_sub.subscriptions.subscription import Subscription
 
 
-@pytest.fixture
-def playlist_preset_dict(output_directory):
-    return {
-        "preset": "yt_music_video_playlist",
-        "youtube": {"playlist_url": "https://youtube.com/playlist?list=PL5BC0FC26BECA5A35"},
-        # override the output directory with our fixture-generated dir
-        "output_options": {"output_directory": output_directory},
-        # download the worst format so it is fast
-        "ytdl_options": {
-            "format": "worst[ext=mp4]",
-        },
-        "overrides": {"artist": "JMC"},
-    }
-
-
 @pytest.fixture
 def single_video_preset_dict(output_directory):
     return {
@@ -38,6 +25,11 @@ def single_video_preset_dict(output_directory):
     }
 
 
+@pytest.fixture
+def single_video_preset_dict_dl_args(single_video_preset_dict):
+    return preset_dict_to_dl_args(single_video_preset_dict)
+
+
 @pytest.fixture
 def expected_single_video_download():
     # turn off black formatter here for readability
@@ -91,6 +83,31 @@ class TestYoutubeVideo:
         if not dry_run:
             expected_single_video_download.assert_files_exist(relative_directory=output_directory)
 
+    @pytest.mark.parametrize("dry_run", [True, False])
+    def test_single_video_download_from_cli_dl(
+        self,
+        music_video_config_path,
+        single_video_preset_dict_dl_args,
+        expected_single_video_download,
+        output_directory,
+        dry_run,
+    ):
+        args = "--dry-run " if dry_run else ""
+        args += f"--config {music_video_config_path} "
+        args += f"dl {single_video_preset_dict_dl_args}"
+        subscription_transaction_log = mock_run_from_cli(args=args)
+
+        assert len(subscription_transaction_log) == 1
+        transaction_log = subscription_transaction_log[0][1]
+
+        assert_transaction_log_matches(
+            output_directory=output_directory,
+            transaction_log=transaction_log,
+            transaction_log_summary_file_name="youtube/test_video.txt",
+        )
+        if not dry_run:
+            expected_single_video_download.assert_files_exist(relative_directory=output_directory)
+
     @pytest.mark.parametrize("dry_run", [True, False])
     def test_single_video_with_timestamp_chapters_download(
         self,

tests/unit/utils/test_yaml.py

@@ -8,7 +8,7 @@ from ytdl_sub.utils.yaml import load_yaml
 
 
 @pytest.fixture
-def bad_yaml():
+def bad_yaml() -> str:
     return """
     this: 
       is:
@@ -20,7 +20,7 @@ def bad_yaml():
 
 
 @pytest.fixture
-def bad_yaml_file_path(bad_yaml):
+def bad_yaml_file_path(bad_yaml) -> str:
     with tempfile.NamedTemporaryFile(suffix=".yaml") as tmp_file:
         tmp_file.write(bad_yaml.encode("utf-8"))
         tmp_file.flush()