From e89a8da3b49670e380a2b01d03eba48090b575e0 Mon Sep 17 00:00:00 2001
From: Patrick Robertson <robertson.patrick@gmail.com>
Date: Mon, 10 Mar 2025 11:30:15 +0000
Subject: [PATCH 1/6] Unit tests for storage types + fix storage too long
 issues for local storage

---
 src/auto_archiver/core/media.py               |  4 +-
 src/auto_archiver/core/storage.py             | 34 ++++++--
 .../modules/local_storage/local_storage.py    | 22 +++++
 .../modules/s3_storage/s3_storage.py          |  4 +
 tests/storages/test_local_storage.py          |  9 ++-
 tests/storages/test_storage_base.py           | 80 ++++++++++++++++++-
 6 files changed, 142 insertions(+), 11 deletions(-)

diff --git a/src/auto_archiver/core/media.py b/src/auto_archiver/core/media.py
index b6820ab..0370c00 100644
--- a/src/auto_archiver/core/media.py
+++ b/src/auto_archiver/core/media.py
@@ -6,7 +6,7 @@ nested media retrieval, and type validation.
 from __future__ import annotations
 import os
 import traceback
-from typing import Any, List
+from typing import Any, List, Iterator
 from dataclasses import dataclass, field
 from dataclasses_json import dataclass_json, config
 import mimetypes
@@ -47,7 +47,7 @@ class Media:
             for any_media in self.all_inner_media(include_self=True):
                 s.store(any_media, url, metadata=metadata)
 
-    def all_inner_media(self, include_self=False):
+    def all_inner_media(self, include_self=False) -> Iterator[Media]:
         """Retrieves all media, including nested media within properties or transformations on original media.
         This function returns a generator for all the inner media.
 
diff --git a/src/auto_archiver/core/storage.py b/src/auto_archiver/core/storage.py
index 1535eab..2a14073 100644
--- a/src/auto_archiver/core/storage.py
+++ b/src/auto_archiver/core/storage.py
@@ -27,6 +27,7 @@ class Storage(BaseModule):
         if media.is_stored(in_storage=self): 
             logger.debug(f"{media.key} already stored, skipping")
             return
+
         self.set_key(media, url, metadata)
         self.upload(media, metadata=metadata)
         media.add_url(self.get_cdn_url(media))
@@ -50,34 +51,55 @@ class Storage(BaseModule):
         with open(media.filename, 'rb') as f:
             return self.uploadf(f, media, **kwargs)
 
-    def set_key(self, media: Media, url, metadata: Metadata) -> None:
+    def set_key(self, media: Media, url: str, metadata: Metadata) -> None:
         """takes the media and optionally item info and generates a key"""
         if media.key is not None and len(media.key) > 0: return
         folder = metadata.get_context('folder', '')
         filename, ext = os.path.splitext(media.filename)
 
         # Handle path_generator logic
-        path_generator = self.config.get("path_generator", "url")
+        path_generator = self.path_generator
         if path_generator == "flat":
             path = ""
+            # TODO: this is never used
             filename = slugify(filename)  # Ensure filename is slugified
         elif path_generator == "url":
             path = slugify(url)
         elif path_generator == "random":
-            path = self.config.get("random_path", random_str(24), True)
+            path = random_str(24)
         else:
             raise ValueError(f"Invalid path_generator: {path_generator}")
 
         # Handle filename_generator logic
-        filename_generator = self.config.get("filename_generator", "random")
+        filename_generator = self.filename_generator
         if filename_generator == "random":
             filename = random_str(24)
         elif filename_generator == "static":
             # load the hash_enricher module
-            he = self.module_factory.get_module(HashEnricher, self.config)
+            he = self.module_factory.get_module("hash_enricher", self.config)
             hd = he.calculate_hash(media.filename)
             filename = hd[:24]
         else:
             raise ValueError(f"Invalid filename_generator: {filename_generator}")
 
-        media.key = os.path.join(folder, path, f"{filename}{ext}")
+        key = os.path.join(folder, path, f"{filename}{ext}")
+        if len(key) > self.max_file_length():
+            # truncate the path
+            max_path_length = self.max_file_length() - len(filename) - len(ext) - len(folder) - 1
+            path = path[:max_path_length]
+            logger.warning(f'Filename too long ({len(key)} characters), truncating to {self.max_file_length()} characters')
+            key = os.path.join(folder, path, f"{filename}{ext}")
+
+
+        media.key = key
+
+
+    def max_file_length(self) -> int:
+        """
+        Returns the maximum length of a file name that can be stored in the storage service.
+
+        Files are truncated if they exceed this length.
+        Override this method in subclasses if the storage service has a different maximum file length.
+        """
+        return 255 # safe max file length for most filesystems (macOS, Windows, Linux)
+
diff --git a/src/auto_archiver/modules/local_storage/local_storage.py b/src/auto_archiver/modules/local_storage/local_storage.py
index b995577..c07f682 100644
--- a/src/auto_archiver/modules/local_storage/local_storage.py
+++ b/src/auto_archiver/modules/local_storage/local_storage.py
@@ -10,6 +10,8 @@ from auto_archiver.core import Storage
 
 class LocalStorage(Storage):
 
+    MAX_FILE_LENGTH = 255
+
     def get_cdn_url(self, media: Media) -> str:
         # TODO: is this viable with Storage.configs on path/filename?
         dest = os.path.join(self.save_to, media.key)
@@ -20,11 +22,31 @@ class LocalStorage(Storage):
     def upload(self, media: Media, **kwargs) -> bool:
         # override parent so that we can use shutil.copy2 and keep metadata
         dest = os.path.join(self.save_to, media.key)
+
+        if len(dest) > self.max_file_length():
+            old_dest_length = len(dest)
+            filename, ext = os.path.splitext(media.key)
+            dir, filename = os.path.split(filename)
+            # see whether we should truncate filename or dir
+            if len(dir) > len(filename):
+                dir = dir[:self.MAX_FILE_LENGTH - len(self.save_to) - len(ext) - len(filename) - 1]
+            else:
+                filename = filename[:self.MAX_FILE_LENGTH - len(self.save_to) - len(ext) - len(filename) - 1]
+
+            # override media.key
+            media.key = os.path.join(dir, f"{filename}{ext}")
+            dest = os.path.join(self.save_to, dir, f"{filename}{ext}")
+            logger.warning(f'Filename too long ({old_dest_length} characters), truncating to {len(dest)} characters')
+
         os.makedirs(os.path.dirname(dest), exist_ok=True)
         logger.debug(f'[{self.__class__.__name__}] storing file {media.filename} with key {media.key} to {dest}')
+
         res = shutil.copy2(media.filename, dest)
         logger.info(res)
         return True
 
     # must be implemented even if unused
     def uploadf(self, file: IO[bytes], key: str, **kwargs: dict) -> bool: pass
+
+    def max_file_length(self):
+        return self.MAX_FILE_LENGTH
\ No newline at end of file
diff --git a/src/auto_archiver/modules/s3_storage/s3_storage.py b/src/auto_archiver/modules/s3_storage/s3_storage.py
index 6590ac9..c8569b6 100644
--- a/src/auto_archiver/modules/s3_storage/s3_storage.py
+++ b/src/auto_archiver/modules/s3_storage/s3_storage.py
@@ -67,4 +67,8 @@ class S3Storage(Storage):
         if 'Contents' in resp:
             return resp['Contents'][0]['Key']
         return False
+    
+    def max_file_length(self):
+        # Amazon AWS max file length is 1024, but we will use 1000 to be safe
+        return 1000
 
diff --git a/tests/storages/test_local_storage.py b/tests/storages/test_local_storage.py
index 7617867..7282e03 100644
--- a/tests/storages/test_local_storage.py
+++ b/tests/storages/test_local_storage.py
@@ -11,6 +11,7 @@ from auto_archiver.modules.local_storage import LocalStorage
 @pytest.fixture
 def local_storage(setup_module, tmp_path) -> LocalStorage:
     save_to = tmp_path / "local_archive"
+    save_to.mkdir()
     configs: dict = {
         "path_generator": "flat",
         "filename_generator": "static",
@@ -19,7 +20,6 @@ def local_storage(setup_module, tmp_path) -> LocalStorage:
     }
     return setup_module("local_storage", configs)
 
-
 @pytest.fixture
 def sample_media(tmp_path) -> Media:
     """Fixture creating a Media object with temporary source file"""
@@ -27,6 +27,13 @@ def sample_media(tmp_path) -> Media:
     src_file.write_text("test content")
     return Media(key="subdir/test.txt", filename=str(src_file))
 
+def test_really_long_website_url_save(local_storage, tmp_path):
+    long_filename = os.path.join(local_storage.save_to, "file"*100 + ".txt")
+    src_file = tmp_path / "source.txt"
+    src_file.write_text("test content")
+    media = Media(key=long_filename, filename=str(src_file))
+    assert local_storage.upload(media) is True
+    assert src_file.read_text() == Path(local_storage.get_cdn_url(media)).read_text()
 
 def test_get_cdn_url_relative(local_storage):
     media = Media(key="test.txt", filename="dummy.txt")
diff --git a/tests/storages/test_storage_base.py b/tests/storages/test_storage_base.py
index 7578acd..fd07c32 100644
--- a/tests/storages/test_storage_base.py
+++ b/tests/storages/test_storage_base.py
@@ -2,9 +2,9 @@ from typing import Type
 
 import pytest
 
-from auto_archiver.core.metadata import Metadata
+from auto_archiver.core.metadata import Metadata, Media
 from auto_archiver.core.storage import Storage
-
+from auto_archiver.core.module import ModuleFactory
 
 class TestStorageBase(object):
 
@@ -20,3 +20,79 @@ class TestStorageBase(object):
         self.storage: Type[Storage] = setup_module(
             self.module_name, self.config
         )
+
+
+class TestBaseStorage(Storage):
+
+    name = "test_storage"
+
+    def get_cdn_url(self, media):
+        return "cdn_url"
+    
+    def uploadf(self, file, key, **kwargs):
+        return True
+
+@pytest.fixture
+def storage_base():
+    def _storage_base(config):
+        storage_base = TestBaseStorage()
+        storage_base.config_setup({TestBaseStorage.name : config})
+        storage_base.module_factory = ModuleFactory()
+        return storage_base
+    
+    return _storage_base
+
+@pytest.mark.parametrize(
+    "path_generator, filename_generator, url, expected_key",
+    [
+        ("flat", "static", "https://example.com/file/", "folder/6ae8a75555209fd6c44157c0.txt"),
+        ("flat", "random", "https://example.com/file/", "folder/pretend-random.txt"),
+        ("url", "static", "https://example.com/file/", "folder/https-example-com-file/6ae8a75555209fd6c44157c0.txt"),
+        ("url", "random", "https://example.com/file/", "folder/https-example-com-file/pretend-random.txt"),
+        ("random", "static", "https://example.com/file/", "folder/pretend-random/6ae8a75555209fd6c44157c0.txt"),
+        ("random", "random", "https://example.com/file/", "folder/pretend-random/pretend-random.txt"),
+
+    ],
+)
+def test_storage_setup(storage_base, path_generator, filename_generator, url, expected_key, mocker):
+    mock_random = mocker.patch("auto_archiver.core.storage.random_str")
+    mock_random.return_value = "pretend-random"
+
+    # create dummy.txt file
+    with open("dummy.txt", "w") as f:
+        f.write("test content")
+
+    config: dict = {
+        "path_generator": path_generator,
+        "filename_generator": filename_generator,
+    }
+    storage: Storage = storage_base(config)
+    assert storage.path_generator == path_generator
+    assert storage.filename_generator == filename_generator
+
+    metadata = Metadata()
+    metadata.set_context("folder", "folder")
+    media = Media(filename="dummy.txt")
+    storage.set_key(media, url, metadata)
+    print(media.key)
+    assert media.key == expected_key
+
+
+def test_really_long_name(storage_base):
+    config: dict = {
+        "path_generator": "url",
+        "filename_generator": "static",
+    }
+    storage: Storage = storage_base(config)
+
+    # create dummy.txt file
+    with open("dummy.txt", "w") as f:
+        f.write("test content")
+
+    url = f"https://example.com/{'file'*100}"
+    media = Media(filename="dummy.txt")
+    storage.set_key(media, url, Metadata())
+    assert len(media.key) <= storage.max_file_length()
+    assert media.key == "https-example-com-filefilefilefilefilefilefilefilefilefilefilefile\
+filefilefilefilefilefilefilefilefilefilefilefilefilefilefilefilefilefilefilefilefilefilefile\
+filefilefilefilefilefilefilefilefilefilefilefilefilefilefilefilefile/6ae8a75555209fd6c44157c0.txt"
\ No newline at end of file

From 58bd38e29202b1f9d0603e1cfcbc98b24f03dee8 Mon Sep 17 00:00:00 2001
From: Miguel Sozinho Ramalho <19508417+msramalho@users.noreply.github.com>
Date: Mon, 10 Mar 2025 17:56:45 +0600
Subject: [PATCH 2/6] Adds new extractor for tiktok via unofficial API (#237)

* minor update to defaults in api_db

* readme typo

* adds and tests new tikwm tiktok downloader

* addresses PR comments
---
 README.md                                     |   2 +-
 .../modules/api_db/__manifest__.py            |   4 +-
 .../tiktok_tikwm_extractor/__init__.py        |   1 +
 .../tiktok_tikwm_extractor/__manifest__.py    |  23 +++
 .../tiktok_tikwm_extractor.py                 |  75 +++++++++
 .../extractors/test_tiktok_tikwm_extractor.py | 154 ++++++++++++++++++
 6 files changed, 256 insertions(+), 3 deletions(-)
 create mode 100644 src/auto_archiver/modules/tiktok_tikwm_extractor/__init__.py
 create mode 100644 src/auto_archiver/modules/tiktok_tikwm_extractor/__manifest__.py
 create mode 100644 src/auto_archiver/modules/tiktok_tikwm_extractor/tiktok_tikwm_extractor.py
 create mode 100644 tests/extractors/test_tiktok_tikwm_extractor.py

diff --git a/README.md b/README.md
index b39038e..8baa722 100644
--- a/README.md
+++ b/README.md
@@ -10,7 +10,7 @@
 
 
 
-Auto Archiver is a Python tool to automatically archive content on the web in a secure and verifiable way. It takes URLs from different sources (e.g. a CSV file, Google Sheets, command line etc.) and archives the content of each one. It can archive social media posts, videos, images and webpages. Content can enriched, then saved either locally or remotely (S3 bucket, Google Drive). The status of the archiving process can be appended to a CSV report, or if using Google Sheets – back to the original sheet.
+Auto Archiver is a Python tool to automatically archive content on the web in a secure and verifiable way. It takes URLs from different sources (e.g. a CSV file, Google Sheets, command line etc.) and archives the content of each one. It can archive social media posts, videos, images and webpages. Content can be enriched, then saved either locally or remotely (S3 bucket, Google Drive). The status of the archiving process can be appended to a CSV report, or if using Google Sheets – back to the original sheet.
 
 <div class="hidden_rtd">
   
diff --git a/src/auto_archiver/modules/api_db/__manifest__.py b/src/auto_archiver/modules/api_db/__manifest__.py
index e67b31a..c9ac461 100644
--- a/src/auto_archiver/modules/api_db/__manifest__.py
+++ b/src/auto_archiver/modules/api_db/__manifest__.py
@@ -24,9 +24,9 @@
             "help": "which group of users have access to the archive in case public=false as author",
         },
         "use_api_cache": {
-            "default": True,
+            "default": False,
             "type": "bool",
-            "help": "if False then the API database will be queried prior to any archiving operations and stop if the link has already been archived",
+            "help": "if True then the API database will be queried prior to any archiving operations and stop if the link has already been archived",
         },
         "store_results": {
             "default": True,
diff --git a/src/auto_archiver/modules/tiktok_tikwm_extractor/__init__.py b/src/auto_archiver/modules/tiktok_tikwm_extractor/__init__.py
new file mode 100644
index 0000000..25a20f5
--- /dev/null
+++ b/src/auto_archiver/modules/tiktok_tikwm_extractor/__init__.py
@@ -0,0 +1 @@
+from .tiktok_tikwm_extractor import TiktokTikwmExtractor
\ No newline at end of file
diff --git a/src/auto_archiver/modules/tiktok_tikwm_extractor/__manifest__.py b/src/auto_archiver/modules/tiktok_tikwm_extractor/__manifest__.py
new file mode 100644
index 0000000..56d8e3e
--- /dev/null
+++ b/src/auto_archiver/modules/tiktok_tikwm_extractor/__manifest__.py
@@ -0,0 +1,23 @@
+{
+    "name": "Tiktok Tikwm Extractor",
+    "type": ["extractor"],
+    "requires_setup": False,
+    "dependencies": {
+        "python": ["loguru", "requests"],
+        "bin": []
+    },
+    "description": """
+    Uses an unofficial TikTok video download platform's API to download videos: https://tikwm.com/
+	
+	This extractor complements the generic_extractor which can already get TikTok videos, but this one can extract special videos like those marked as sensitive.
+
+    ### Features
+    - Downloads the video and, if possible, also the video cover.
+	- Stores extra metadata about the post like author information, and more as returned by tikwm.com. 
+
+    ### Notes
+    - If tikwm.com is down, this extractor will not work.
+	- If tikwm.com changes their API, this extractor may break.
+	- If no video is found, this extractor will consider the extraction failed.
+    """
+}
diff --git a/src/auto_archiver/modules/tiktok_tikwm_extractor/tiktok_tikwm_extractor.py b/src/auto_archiver/modules/tiktok_tikwm_extractor/tiktok_tikwm_extractor.py
new file mode 100644
index 0000000..8b07775
--- /dev/null
+++ b/src/auto_archiver/modules/tiktok_tikwm_extractor/tiktok_tikwm_extractor.py
@@ -0,0 +1,75 @@
+import re
+import requests
+from loguru import logger
+from datetime import datetime, timezone
+from yt_dlp.extractor.tiktok import TikTokIE
+
+from auto_archiver.core import Extractor
+from auto_archiver.core import Metadata, Media
+
+
+class TiktokTikwmExtractor(Extractor):
+    """
+    Extractor for TikTok that uses an unofficial API and can capture content that requires a login, like sensitive content.
+    """
+    TIKWM_ENDPOINT = "https://www.tikwm.com/api/?url={url}"
+
+    def download(self, item: Metadata) -> Metadata:
+        url = item.get_url()
+        
+        if not re.match(TikTokIE._VALID_URL, url):
+            return False
+
+        endpoint = TiktokTikwmExtractor.TIKWM_ENDPOINT.format(url=url)
+
+        r = requests.get(endpoint)
+        if r.status_code != 200:
+            logger.error(f"unexpected status code '{r.status_code}' from tikwm.com for {url=}:")
+            return False
+
+        try:
+            json_response = r.json()
+        except ValueError:
+            logger.error(f"failed to parse JSON response from tikwm.com for {url=}")
+            return False
+
+        if not json_response.get('msg') == 'success' or not (api_data := json_response.get('data', {})):
+            logger.error(f"failed to get a valid response from tikwm.com for {url=}: {json_response}")
+            return False
+
+        # tries to get the non-watermarked version first
+        video_url = api_data.pop("play", api_data.pop("wmplay", None))
+        if not video_url:
+            logger.error(f"no valid video URL found in response from tikwm.com for {url=}")
+            return False
+
+        # prepare result, start by downloading video
+        result = Metadata()
+
+        # get the cover if possible
+        cover_url = api_data.pop("origin_cover", api_data.pop("cover", api_data.pop("ai_dynamic_cover", None)))
+        if cover_url and (cover_downloaded := self.download_from_url(cover_url)):
+            result.add_media(Media(cover_downloaded))
+
+        # get the video or fail
+        video_downloaded = self.download_from_url(video_url, f"vid_{api_data.get('id', '')}")
+        if not video_downloaded:
+            logger.error(f"failed to download video from {video_url}")
+            return False
+        video_media = Media(video_downloaded)
+        if duration := api_data.pop("duration", None):
+            video_media.set("duration", duration)
+        result.add_media(video_media)
+
+        # add remaining metadata
+        result.set_title(api_data.pop("title", ""))
+
+        if created_at := api_data.pop("create_time", None):
+            result.set_timestamp(datetime.fromtimestamp(created_at, tz=timezone.utc))
+
+        if (author := api_data.pop("author", None)):
+            result.set("author", author)
+
+        result.set("api_data", api_data)
+
+        return result.success("tikwm")
diff --git a/tests/extractors/test_tiktok_tikwm_extractor.py b/tests/extractors/test_tiktok_tikwm_extractor.py
new file mode 100644
index 0000000..e8ad8df
--- /dev/null
+++ b/tests/extractors/test_tiktok_tikwm_extractor.py
@@ -0,0 +1,154 @@
+from datetime import datetime, timezone
+import time
+import pytest
+
+from auto_archiver.modules.tiktok_tikwm_extractor.tiktok_tikwm_extractor import TiktokTikwmExtractor
+from .test_extractor_base import TestExtractorBase
+
+
+class TestTiktokTikwmExtractor(TestExtractorBase):
+    """
+    Test suite for TestTiktokTikwmExtractor.
+    """
+
+    extractor_module = "tiktok_tikwm_extractor"
+    extractor: TiktokTikwmExtractor
+
+    config = {}
+
+    VALID_EXAMPLE_URL = "https://www.tiktok.com/@example/video/1234"
+
+    @staticmethod
+    def get_mockers(mocker):
+        mock_get = mocker.patch("auto_archiver.modules.tiktok_tikwm_extractor.tiktok_tikwm_extractor.requests.get")
+        mock_logger = mocker.patch("auto_archiver.modules.tiktok_tikwm_extractor.tiktok_tikwm_extractor.logger")
+        return mock_get, mock_logger
+
+    @pytest.mark.parametrize("url,valid_url", [
+        ("https://bellingcat.com", False),
+        ("https://youtube.com", False),
+        ("https://tiktok.co/", False),
+        ("https://tiktok.com/", False),
+        ("https://www.tiktok.com/", False),
+        ("https://api.cool.tiktok.com/", False),
+        (VALID_EXAMPLE_URL, True),
+        ("https://www.tiktok.com/@bbcnews/video/7478038212070411542", True),
+        ("https://www.tiktok.com/@ggs68taiwan.official/video/7441821351142362375", True),
+    ])
+    def test_valid_urls(self, mocker, make_item, url, valid_url):
+        mock_get, mock_logger = self.get_mockers(mocker)
+        if valid_url:
+            mock_get.return_value.status_code = 404
+        assert self.extractor.download(make_item(url)) == False
+        assert mock_get.call_count == int(valid_url)
+        assert mock_logger.error.call_count == int(valid_url)
+
+    def test_invalid_json_responses(self, mocker, make_item):
+        mock_get, mock_logger = self.get_mockers(mocker)
+        mock_get.return_value.status_code = 200
+        mock_get.return_value.json.side_effect = ValueError
+        assert self.extractor.download(make_item(self.VALID_EXAMPLE_URL)) == False
+        mock_get.assert_called_once()
+        mock_get.return_value.json.assert_called_once()
+        mock_logger.error.assert_called_once()
+        assert mock_logger.error.call_args[0][0].startswith("failed to parse JSON response")
+
+        mock_get.return_value.json.side_effect = Exception
+        with pytest.raises(Exception):
+            self.extractor.download(make_item(self.VALID_EXAMPLE_URL))
+        mock_get.assert_called()
+        assert mock_get.call_count == 2
+        assert mock_get.return_value.json.call_count == 2
+
+    @pytest.mark.parametrize("response", [
+        ({"msg": "failure"}),
+        ({"msg": "success"}),
+    ])
+    def test_unsuccessful_responses(self, mocker, make_item, response):
+        mock_get, mock_logger = self.get_mockers(mocker)
+        mock_get.return_value.status_code = 200
+        mock_get.return_value.json.return_value = response
+        assert self.extractor.download(make_item(self.VALID_EXAMPLE_URL)) == False
+        mock_get.assert_called_once()
+        mock_get.return_value.json.assert_called_once()
+        mock_logger.error.assert_called_once()
+        assert mock_logger.error.call_args[0][0].startswith("failed to get a valid response")
+
+    @pytest.mark.parametrize("response,has_vid", [
+        ({"data": {"id": 123}}, False),
+        ({"data": {"wmplay": "url"}}, True),
+        ({"data": {"play": "url"}}, True),
+    ])
+    def test_correct_extraction(self, mocker, make_item, response, has_vid):
+        mock_get, mock_logger = self.get_mockers(mocker)
+        mock_get.return_value.status_code = 200
+        mock_get.return_value.json.return_value = {"msg": "success", **response}
+
+        result = self.extractor.download(make_item(self.VALID_EXAMPLE_URL))
+        if not has_vid:
+            assert result == False
+        else:
+            assert result.is_success()
+            assert len(result.media) == 1
+        mock_get.assert_called()
+        assert mock_get.call_count == 1 + int(has_vid)
+        mock_get.return_value.json.assert_called_once()
+        if not has_vid:
+            mock_logger.error.assert_called_once()
+            assert mock_logger.error.call_args[0][0].startswith("no valid video URL found")
+        else:
+            mock_logger.error.assert_not_called()
+
+    def test_correct_extraction(self, mocker, make_item):
+        mock_get, _ = self.get_mockers(mocker)
+        mock_get.return_value.status_code = 200
+        mock_get.return_value.json.return_value = {"msg": "success", "data": {
+            "wmplay": "url",
+            "origin_cover": "cover.jpg",
+            "title": "Title",
+            "id": 123,
+            "duration": 60,
+            "create_time": 1736301699,
+            "author": "Author",
+            "other": "data"
+        }}
+
+        result = self.extractor.download(make_item(self.VALID_EXAMPLE_URL))
+        assert result.is_success()
+        assert len(result.media) == 2
+        assert result.get_title() == "Title"
+        assert result.get("author") == "Author"
+        assert result.get("api_data") == {"other": "data", "id": 123}
+        assert result.media[1].get("duration") == 60
+        assert result.get("timestamp") == datetime.fromtimestamp(1736301699, tz=timezone.utc)
+
+    @pytest.mark.download
+    def test_download_video(self, make_item):
+        url = "https://www.tiktok.com/@bbcnews/video/7478038212070411542"
+
+        result = self.extractor.download(make_item(url))
+        assert result.is_success()
+        assert len(result.media) == 2
+        assert result.get_title() == "The A23a iceberg is one of the world's oldest and it's so big you can see it from space. #Iceberg  #A23a  #Antarctica  #Ice  #ClimateChange  #DavidAttenborough  #Ocean  #Sea  #SouthGeorgia  #BBCNews "
+        assert result.get("author").get("unique_id") == "bbcnews"
+        assert result.get("api_data").get("id") == '7478038212070411542'
+        assert result.media[1].get("duration") == 59
+        assert result.get("timestamp") == datetime.fromtimestamp(1741122000, tz=timezone.utc)
+
+    @pytest.mark.download
+    def test_download_sensitive_video(self, make_item, mock_sleep):
+        # sleep is needed because of the rate limit
+        mock_sleep.stop()
+        time.sleep(1.1)
+        mock_sleep.start()
+
+        url = "https://www.tiktok.com/@ggs68taiwan.official/video/7441821351142362375"
+
+        result = self.extractor.download(make_item(url))
+        assert result.is_success()
+        assert len(result.media) == 2
+        assert result.get_title() == "Căng nhất lúc này #ggs68 #ggs68taiwan #taiwan #dailoan #tiktoknews"
+        assert result.get("author").get("id") == "7197400619475649562"
+        assert result.get("api_data").get("id") == '7441821351142362375'
+        assert result.media[1].get("duration") == 34
+        assert result.get("timestamp") == datetime.fromtimestamp(1732684060, tz=timezone.utc)

From 770f4c8a3de0a3bd973dd099113bffb7b74a8cfc Mon Sep 17 00:00:00 2001
From: Patrick Robertson <robertson.patrick@gmail.com>
Date: Mon, 10 Mar 2025 16:39:31 +0000
Subject: [PATCH 3/6] Refactoring of storage code:

1. Fix some bugs in local_storage
2. Refactor unit tests to not set Media.key explicitly (unless it's well-known beforehand, which it isn't)
3. Limit length of URL for 'url' type path_generator
4. Throw an error if 'save_to' of local storage is too long
5. A few other tidyups
---
 src/auto_archiver/core/config.py              |  1 -
 src/auto_archiver/core/consts.py              |  2 +
 src/auto_archiver/core/extractor.py           |  3 +-
 src/auto_archiver/core/media.py               |  9 ++--
 src/auto_archiver/core/orchestrator.py        |  4 +-
 src/auto_archiver/core/storage.py             | 42 ++++++++----------
 .../modules/cli_feeder/cli_feeder.py          |  1 -
 .../modules/local_storage/local_storage.py    | 43 ++++++++-----------
 .../modules/s3_storage/s3_storage.py          | 13 ++----
 tests/enrichers/test_pdq_hash_enricher.py     |  4 +-
 tests/storages/test_S3_storage.py             |  8 ++--
 tests/storages/test_atlos_storage.py          |  2 +-
 tests/storages/test_gdrive_storage.py         |  2 +-
 tests/storages/test_local_storage.py          | 29 ++++++-------
 tests/storages/test_storage_base.py           |  7 +--
 15 files changed, 74 insertions(+), 96 deletions(-)

diff --git a/src/auto_archiver/core/config.py b/src/auto_archiver/core/config.py
index 66d2ffb..0282d41 100644
--- a/src/auto_archiver/core/config.py
+++ b/src/auto_archiver/core/config.py
@@ -14,7 +14,6 @@ from loguru import logger
 from copy import deepcopy
 from auto_archiver.core.consts import MODULE_TYPES
 
-from typing import Any, List, Type, Tuple
 
 _yaml: YAML = YAML()
 
diff --git a/src/auto_archiver/core/consts.py b/src/auto_archiver/core/consts.py
index a49884f..597ce4e 100644
--- a/src/auto_archiver/core/consts.py
+++ b/src/auto_archiver/core/consts.py
@@ -1,3 +1,5 @@
+class SetupError(ValueError):
+    pass
 
 MODULE_TYPES = [
     'feeder',
diff --git a/src/auto_archiver/core/extractor.py b/src/auto_archiver/core/extractor.py
index 484a09d..f84be98 100644
--- a/src/auto_archiver/core/extractor.py
+++ b/src/auto_archiver/core/extractor.py
@@ -81,7 +81,8 @@ class Extractor(BaseModule):
             if len(to_filename) > 64:
                 to_filename = to_filename[-64:]
         to_filename = os.path.join(self.tmp_dir, to_filename)
-        if verbose: logger.debug(f"downloading {url[0:50]=} {to_filename=}")
+        if verbose:
+            logger.debug(f"downloading {url[0:50]=} {to_filename=}")
         headers = {
             'User-Agent': 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/81.0.4044.138 Safari/537.36'
         }
diff --git a/src/auto_archiver/core/media.py b/src/auto_archiver/core/media.py
index 0370c00..ecaef19 100644
--- a/src/auto_archiver/core/media.py
+++ b/src/auto_archiver/core/media.py
@@ -21,14 +21,13 @@ class Media:
     Represents a media file with associated properties and storage details.
 
     Attributes:
-    - filename: The file path of the media.
-    - key: An optional identifier for the media.
+    - filename: The file path of the media as saved locally (temporarily, before uploading to the storage).
     - urls: A list of URLs where the media is stored or accessible.
     - properties: Additional metadata or transformations for the media.
     - _mimetype: The media's mimetype (e.g., image/jpeg, video/mp4).
     """
     filename: str
-    key: str = None
+    _key: str = None
     urls: List[str] = field(default_factory=list)
     properties: dict = field(default_factory=dict)
     _mimetype: str = None  # eg: image/jpeg
@@ -67,6 +66,10 @@ class Media:
         # checks if the media is already stored in the given storage
         return len(self.urls) > 0 and len(self.urls) == len(in_storage.config["steps"]["storages"])
 
+    @property
+    def key(self) -> str:
+        return self._key
+
     def set(self, key: str, value: Any) -> Media:
         self.properties[key] = value
         return self
diff --git a/src/auto_archiver/core/orchestrator.py b/src/auto_archiver/core/orchestrator.py
index b78c7e7..6a95046 100644
--- a/src/auto_archiver/core/orchestrator.py
+++ b/src/auto_archiver/core/orchestrator.py
@@ -23,15 +23,13 @@ from .config import read_yaml, store_yaml, to_dot_notation, merge_dicts, is_vali
     DefaultValidatingParser, UniqueAppendAction, AuthenticationJsonParseAction, DEFAULT_CONFIG_FILE
 from .module import ModuleFactory, LazyBaseModule
 from . import validators, Feeder, Extractor, Database, Storage, Formatter, Enricher
-from .consts import MODULE_TYPES
+from .consts import MODULE_TYPES, SetupError
 from auto_archiver.utils.url import check_url_or_raise
 
 if TYPE_CHECKING:
     from .base_module import BaseModule
     from .module import LazyBaseModule
 
-class SetupError(ValueError):
-    pass
 class ArchivingOrchestrator:
 
     # instance variables
diff --git a/src/auto_archiver/core/storage.py b/src/auto_archiver/core/storage.py
index 2a14073..4867146 100644
--- a/src/auto_archiver/core/storage.py
+++ b/src/auto_archiver/core/storage.py
@@ -6,6 +6,7 @@ from __future__ import annotations
 from abc import abstractmethod
 from typing import IO
 import os
+import platform
 
 from loguru import logger
 from slugify import slugify
@@ -43,17 +44,30 @@ class Storage(BaseModule):
     def uploadf(self, file: IO[bytes], key: str, **kwargs: dict) -> bool:
         """
         Uploads (or saves) a file to the storage service/location.
+
+        This method should not be called directly, but instead through the 'store' method,
+        which sets up the media for storage.
         """
         pass
 
     def upload(self, media: Media, **kwargs) -> bool:
+        """
+        Uploads a media object to the storage service.
+
+        This method should not be called directly, but instead be called through the 'store' method,
+        which sets up the media for storage.
+        """
         logger.debug(f'[{self.__class__.__name__}] storing file {media.filename} with key {media.key}')
         with open(media.filename, 'rb') as f:
             return self.uploadf(f, media, **kwargs)
 
     def set_key(self, media: Media, url: str, metadata: Metadata) -> None:
         """takes the media and optionally item info and generates a key"""
-        if media.key is not None and len(media.key) > 0: return
+        
+        if media.key is not None and len(media.key) > 0:
+            # media key is already set
+            return
+
         folder = metadata.get_context('folder', '')
         filename, ext = os.path.splitext(media.filename)
 
@@ -61,10 +75,8 @@ class Storage(BaseModule):
         path_generator = self.path_generator
         if path_generator == "flat":
             path = ""
-            # TODO: this is never used
-            filename = slugify(filename)  # Ensure filename is slugified
         elif path_generator == "url":
-            path = slugify(url)
+            path = slugify(url)[:70]
         elif path_generator == "random":
             path = random_str(24)
         else:
@@ -81,25 +93,7 @@ class Storage(BaseModule):
             filename = hd[:24]
         else:
             raise ValueError(f"Invalid filename_generator: {filename_generator}")
-
+        
         key = os.path.join(folder, path, f"{filename}{ext}")
-        if len(key) > self.max_file_length():
-            # truncate the path
-            max_path_length = self.max_file_length() - len(filename) - len(ext) - len(folder) - 1
-            path = path[:max_path_length]
-            logger.warning(f'Filename too long ({len(key)} characters), truncating to {self.max_file_length()} characters')
-            key = os.path.join(folder, path, f"{filename}{ext}")
-
-
-        media.key = key
-
-
-    def max_file_length(self) -> int:
-        """
-        Returns the maximum length of a file name that can be stored in the storage service.
-
-        Files are truncated if they exceed this length.
-        Override this method in subclasses if the storage service has a different maximum file length.
-        """
-        return 255 # safe max file length for most filesystems (macOS, Windows, Linux)
 
+        media._key = key
\ No newline at end of file
diff --git a/src/auto_archiver/modules/cli_feeder/cli_feeder.py b/src/auto_archiver/modules/cli_feeder/cli_feeder.py
index 20ca6ae..fac584d 100644
--- a/src/auto_archiver/modules/cli_feeder/cli_feeder.py
+++ b/src/auto_archiver/modules/cli_feeder/cli_feeder.py
@@ -15,7 +15,6 @@ class CLIFeeder(Feeder):
         for url in urls:
             logger.debug(f"Processing {url}")
             m = Metadata().set_url(url)
-            m.set_context("folder", "cli")
             yield m
 
         logger.success(f"Processed {len(urls)} URL(s)")
\ No newline at end of file
diff --git a/src/auto_archiver/modules/local_storage/local_storage.py b/src/auto_archiver/modules/local_storage/local_storage.py
index c07f682..6f1e217 100644
--- a/src/auto_archiver/modules/local_storage/local_storage.py
+++ b/src/auto_archiver/modules/local_storage/local_storage.py
@@ -6,37 +6,34 @@ from loguru import logger
 
 from auto_archiver.core import Media
 from auto_archiver.core import Storage
-
+from auto_archiver.core.consts import SetupError
 
 class LocalStorage(Storage):
 
-    MAX_FILE_LENGTH = 255
+
+    def setup(self) -> None:
+        if len(self.save_to) > 200:
+            raise SetupError(f"Your save_to path is  long, this will cause issues saving files on your computer. Please use a shorter path")
 
     def get_cdn_url(self, media: Media) -> str:
-        # TODO: is this viable with Storage.configs on path/filename?
-        dest = os.path.join(self.save_to, media.key)
+        dest = media.key
+
         if self.save_absolute:
             dest = os.path.abspath(dest)
         return dest
 
+    def set_key(self, media, url, metadata):
+        # clarify we want to save the file to the save_to folder
+
+        old_folder = metadata.get('folder', '')
+        metadata.set_context('folder', os.path.join(self.save_to, metadata.get('folder', '')))
+        super().set_key(media, url, metadata)
+        # don't impact other storages that might want a different 'folder' set
+        metadata.set_context('folder', old_folder)
+
     def upload(self, media: Media, **kwargs) -> bool:
         # override parent so that we can use shutil.copy2 and keep metadata
-        dest = os.path.join(self.save_to, media.key)
-
-        if len(dest) > self.max_file_length():
-            old_dest_length = len(dest)
-            filename, ext = os.path.splitext(media.key)
-            dir, filename = os.path.split(filename)
-            # see whether we should truncate filename or dir
-            if len(dir) > len(filename):
-                dir = dir[:self.MAX_FILE_LENGTH - len(self.save_to) - len(ext) - len(filename) - 1]
-            else:
-                filename = filename[:self.MAX_FILE_LENGTH - len(self.save_to) - len(ext) - len(filename) - 1]
-
-            # override media.key
-            media.key = os.path.join(dir, f"{filename}{ext}")
-            dest = os.path.join(self.save_to, dir, f"{filename}{ext}")
-            logger.warning(f'Filename too long ({old_dest_length} characters), truncating to {len(dest)} characters')
+        dest = media.key
 
         os.makedirs(os.path.dirname(dest), exist_ok=True)
         logger.debug(f'[{self.__class__.__name__}] storing file {media.filename} with key {media.key} to {dest}')
@@ -46,7 +43,5 @@ class LocalStorage(Storage):
         return True
 
     # must be implemented even if unused
-    def uploadf(self, file: IO[bytes], key: str, **kwargs: dict) -> bool: pass
-
-    def max_file_length(self):
-        return self.MAX_FILE_LENGTH
\ No newline at end of file
+    def uploadf(self, file: IO[bytes], key: str, **kwargs: dict) -> bool:
+        pass
\ No newline at end of file
diff --git a/src/auto_archiver/modules/s3_storage/s3_storage.py b/src/auto_archiver/modules/s3_storage/s3_storage.py
index c8569b6..183a944 100644
--- a/src/auto_archiver/modules/s3_storage/s3_storage.py
+++ b/src/auto_archiver/modules/s3_storage/s3_storage.py
@@ -42,7 +42,7 @@ class S3Storage(Storage):
                 logger.warning(f"Unable to get mimetype for {media.key=}, error: {e}")
         self.s3.upload_fileobj(file, Bucket=self.bucket, Key=media.key, ExtraArgs=extra_args)
         return True
-    
+
     def is_upload_needed(self, media: Media) -> bool:
         if self.random_no_duplicate:
             # checks if a folder with the hash already exists, if so it skips the upload
@@ -50,13 +50,13 @@ class S3Storage(Storage):
             path = os.path.join(NO_DUPLICATES_FOLDER, hd[:24])
 
             if existing_key:=self.file_in_folder(path):
-                media.key = existing_key
+                media._key = existing_key
                 media.set("previously archived", True)
                 logger.debug(f"skipping upload of {media.filename} because it already exists in {media.key}")
                 return False
             
             _, ext = os.path.splitext(media.key)
-            media.key = os.path.join(path, f"{random_str(24)}{ext}")
+            media._key = os.path.join(path, f"{random_str(24)}{ext}")
         return True
 
     def file_in_folder(self, path:str) -> str:
@@ -66,9 +66,4 @@ class S3Storage(Storage):
         resp = self.s3.list_objects(Bucket=self.bucket, Prefix=path, Delimiter='/', MaxKeys=1)
         if 'Contents' in resp:
             return resp['Contents'][0]['Key']
-        return False
-    
-    def max_file_length(self):
-        # Amazon AWS max file length is 1024, but we will use 1000 to be safe
-        return 1000
-
+        return False
\ No newline at end of file
diff --git a/tests/enrichers/test_pdq_hash_enricher.py b/tests/enrichers/test_pdq_hash_enricher.py
index a8470fb..61fa778 100644
--- a/tests/enrichers/test_pdq_hash_enricher.py
+++ b/tests/enrichers/test_pdq_hash_enricher.py
@@ -14,8 +14,8 @@ def enricher(setup_module):
 def metadata_with_images():
     m = Metadata()
     m.set_url("https://example.com")
-    m.add_media(Media(filename="image1.jpg", key="image1"))
-    m.add_media(Media(filename="image2.jpg", key="image2"))
+    m.add_media(Media(filename="image1.jpg", _key="image1"))
+    m.add_media(Media(filename="image2.jpg", _key="image2"))
     return m
 
 
diff --git a/tests/storages/test_S3_storage.py b/tests/storages/test_S3_storage.py
index fe60329..abf9763 100644
--- a/tests/storages/test_S3_storage.py
+++ b/tests/storages/test_S3_storage.py
@@ -37,10 +37,10 @@ class TestS3Storage:
     def test_get_cdn_url_generation(self):
         """Test CDN URL formatting """
         media = Media("test.txt")
-        media.key = "path/to/file.txt"
+        media._key = "path/to/file.txt"
         url = self.storage.get_cdn_url(media)
         assert url == "https://cdn.example.com/path/to/file.txt"
-        media.key = "another/path.jpg"
+        media._key = "another/path.jpg"
         assert self.storage.get_cdn_url(media) == "https://cdn.example.com/another/path.jpg"
 
     def test_uploadf_sets_acl_public(self, mocker):
@@ -72,7 +72,7 @@ class TestS3Storage:
         self.storage.random_no_duplicate = True
         mock_file_in_folder = mocker.patch.object(S3Storage, 'file_in_folder', return_value="existing_folder/existing_file.txt")
         media = Media("test.txt")
-        media.key = "original_path.txt"
+        media._key = "original_path.txt"
         mock_calculate_hash = mocker.patch('auto_archiver.modules.s3_storage.s3_storage.calculate_file_hash', return_value="beepboop123beepboop123beepboop123")
         assert self.storage.is_upload_needed(media) is False
         assert media.key == "existing_folder/existing_file.txt"
@@ -84,7 +84,7 @@ class TestS3Storage:
 
     def test_uploads_with_correct_parameters(self, mocker):
         media = Media("test.txt")
-        media.key = "original_key.txt"
+        media._key = "original_key.txt"
         mocker.patch.object(S3Storage, 'is_upload_needed', return_value=True)
         media.mimetype = 'image/png'
         mock_file = mocker.MagicMock()
diff --git a/tests/storages/test_atlos_storage.py b/tests/storages/test_atlos_storage.py
index bcd8f18..d33ff2f 100644
--- a/tests/storages/test_atlos_storage.py
+++ b/tests/storages/test_atlos_storage.py
@@ -44,7 +44,7 @@ def media(tmp_path) -> Media:
     file_path.write_bytes(content)
     media = Media(filename=str(file_path))
     media.properties = {"something": "Title"}
-    media.key = "key"
+    media._key = "key"
     return media
 
 
diff --git a/tests/storages/test_gdrive_storage.py b/tests/storages/test_gdrive_storage.py
index f5ff87c..d87f5e8 100644
--- a/tests/storages/test_gdrive_storage.py
+++ b/tests/storages/test_gdrive_storage.py
@@ -50,7 +50,7 @@ def test_get_id_from_parent_and_name(gdrive_storage, mocker):
 
 def test_path_parts():
     media = Media(filename="test.jpg")
-    media.key = "folder1/folder2/test.jpg"
+    media._key = "folder1/folder2/test.jpg"
 
 
 
diff --git a/tests/storages/test_local_storage.py b/tests/storages/test_local_storage.py
index 7282e03..1fd6774 100644
--- a/tests/storages/test_local_storage.py
+++ b/tests/storages/test_local_storage.py
@@ -4,9 +4,9 @@ from pathlib import Path
 
 import pytest
 
-from auto_archiver.core import Media
+from auto_archiver.core import Media, Metadata
 from auto_archiver.modules.local_storage import LocalStorage
-
+from auto_archiver.core.consts import SetupError
 
 @pytest.fixture
 def local_storage(setup_module, tmp_path) -> LocalStorage:
@@ -25,37 +25,32 @@ def sample_media(tmp_path) -> Media:
     """Fixture creating a Media object with temporary source file"""
     src_file = tmp_path / "source.txt"
     src_file.write_text("test content")
-    return Media(key="subdir/test.txt", filename=str(src_file))
+    return Media(filename=str(src_file))
 
-def test_really_long_website_url_save(local_storage, tmp_path):
-    long_filename = os.path.join(local_storage.save_to, "file"*100 + ".txt")
-    src_file = tmp_path / "source.txt"
-    src_file.write_text("test content")
-    media = Media(key=long_filename, filename=str(src_file))
-    assert local_storage.upload(media) is True
-    assert src_file.read_text() == Path(local_storage.get_cdn_url(media)).read_text()
+def test_too_long_save_path(setup_module):
+    with pytest.raises(SetupError):
+        setup_module("local_storage", {"save_to": "long"*100})
 
 def test_get_cdn_url_relative(local_storage):
-    media = Media(key="test.txt", filename="dummy.txt")
+    media = Media(filename="dummy.txt")
+    local_storage.set_key(media, "https://example.com", Metadata())
     expected = os.path.join(local_storage.save_to, media.key)
     assert local_storage.get_cdn_url(media) == expected
 
-
-
 def test_get_cdn_url_absolute(local_storage):
-    media = Media(key="test.txt", filename="dummy.txt")
+    media = Media(filename="dummy.txt")
     local_storage.save_absolute = True
+    local_storage.set_key(media, "https://example.com", Metadata())
     expected = os.path.abspath(os.path.join(local_storage.save_to, media.key))
     assert local_storage.get_cdn_url(media) == expected
 
 def test_upload_file_contents_and_metadata(local_storage, sample_media):
+    local_storage.store(sample_media, "https://example.com", Metadata())
     dest = os.path.join(local_storage.save_to, sample_media.key)
-    assert local_storage.upload(sample_media) is True
     assert Path(sample_media.filename).read_text() == Path(dest).read_text()
 
-
 def test_upload_nonexistent_source(local_storage):
-    media = Media(key="missing.txt", filename="nonexistent.txt")
+    media = Media(_key="missing.txt", filename="nonexistent.txt")
     with pytest.raises(FileNotFoundError):
         local_storage.upload(media)
 
diff --git a/tests/storages/test_storage_base.py b/tests/storages/test_storage_base.py
index fd07c32..30a91b9 100644
--- a/tests/storages/test_storage_base.py
+++ b/tests/storages/test_storage_base.py
@@ -54,7 +54,7 @@ def storage_base():
 
     ],
 )
-def test_storage_setup(storage_base, path_generator, filename_generator, url, expected_key, mocker):
+def test_storage_name_generation(storage_base, path_generator, filename_generator, url, expected_key, mocker):
     mock_random = mocker.patch("auto_archiver.core.storage.random_str")
     mock_random.return_value = "pretend-random"
 
@@ -92,7 +92,4 @@ def test_really_long_name(storage_base):
     url = f"https://example.com/{'file'*100}"
     media = Media(filename="dummy.txt")
     storage.set_key(media, url, Metadata())
-    assert len(media.key) <= storage.max_file_length()
-    assert media.key == "https-example-com-filefilefilefilefilefilefilefilefilefilefilefile\
-filefilefilefilefilefilefilefilefilefilefilefilefilefilefilefilefilefilefilefilefilefilefile\
-filefilefilefilefilefilefilefilefilefilefilefilefilefilefilefilefile/6ae8a75555209fd6c44157c0.txt"
\ No newline at end of file
+    assert media.key == f"https-example-com-{'file'*13}/6ae8a75555209fd6c44157c0.txt"
\ No newline at end of file

From a9c34772896e1971e74005cfb9bb95b7ef39dd92 Mon Sep 17 00:00:00 2001
From: Patrick Robertson <robertson.patrick@gmail.com>
Date: Mon, 10 Mar 2025 16:43:14 +0000
Subject: [PATCH 4/6] Improve docs on the path_generator and filename_generator
 config options

---
 src/auto_archiver/core/storage.py | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/src/auto_archiver/core/storage.py b/src/auto_archiver/core/storage.py
index 4867146..63ccf8d 100644
--- a/src/auto_archiver/core/storage.py
+++ b/src/auto_archiver/core/storage.py
@@ -1,5 +1,22 @@
 """
 Base module for Storage modules – modular components that store media objects in various locations.
+
+If you are looking to implement a new storage module, you should subclass the `Storage` class and
+implement the `get_cdn_url` and `uploadf` methods.
+
+Your module **must** also have two config variables 'path_generator' and 'filename_generator' which
+determine how the key is generated for the media object. The 'path_generator' and 'filename_generator'
+variables can be set to one of the following values:
+- 'flat': A flat structure with no subfolders
+- 'url': A structure based on the URL of the media object
+- 'random': A random structure
+
+The 'filename_generator' variable can be set to one of the following values:
+- 'random': A random string
+- 'static': A replicable strategy such as a hash
+
+If you don't want to use this naming convention, you can override the `set_key` method in your subclass.
+
 """
 
 from __future__ import annotations

From 2b91dc95146a860e55f9b9003d0969527431f473 Mon Sep 17 00:00:00 2001
From: Patrick Robertson <robertson.patrick@gmail.com>
Date: Mon, 10 Mar 2025 16:51:16 +0000
Subject: [PATCH 5/6] Fix up unit tests

---
 tests/storages/test_local_storage.py |  3 +++
 tests/storages/test_storage_base.py  | 24 ++++++++++++------------
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/tests/storages/test_local_storage.py b/tests/storages/test_local_storage.py
index 1fd6774..c3581df 100644
--- a/tests/storages/test_local_storage.py
+++ b/tests/storages/test_local_storage.py
@@ -32,12 +32,15 @@ def test_too_long_save_path(setup_module):
         setup_module("local_storage", {"save_to": "long"*100})
 
 def test_get_cdn_url_relative(local_storage):
+    local_storage.filename_generator = "random"
     media = Media(filename="dummy.txt")
     local_storage.set_key(media, "https://example.com", Metadata())
     expected = os.path.join(local_storage.save_to, media.key)
     assert local_storage.get_cdn_url(media) == expected
 
 def test_get_cdn_url_absolute(local_storage):
+    local_storage.filename_generator = "random"
+
     media = Media(filename="dummy.txt")
     local_storage.save_absolute = True
     local_storage.set_key(media, "https://example.com", Metadata())
diff --git a/tests/storages/test_storage_base.py b/tests/storages/test_storage_base.py
index 30a91b9..53dfbd7 100644
--- a/tests/storages/test_storage_base.py
+++ b/tests/storages/test_storage_base.py
@@ -32,6 +32,13 @@ class TestBaseStorage(Storage):
     def uploadf(self, file, key, **kwargs):
         return True
 
+@pytest.fixture
+def dummy_file(tmp_path):
+    # create dummy.txt file
+    dummy_file = tmp_path / "dummy.txt"
+    dummy_file.write_text("test content")
+    return str(dummy_file)
+
 @pytest.fixture
 def storage_base():
     def _storage_base(config):
@@ -54,14 +61,11 @@ def storage_base():
 
     ],
 )
-def test_storage_name_generation(storage_base, path_generator, filename_generator, url, expected_key, mocker):
+def test_storage_name_generation(storage_base, path_generator, filename_generator, url, 
+                                 expected_key, mocker, tmp_path, dummy_file):
     mock_random = mocker.patch("auto_archiver.core.storage.random_str")
     mock_random.return_value = "pretend-random"
 
-    # create dummy.txt file
-    with open("dummy.txt", "w") as f:
-        f.write("test content")
-
     config: dict = {
         "path_generator": path_generator,
         "filename_generator": filename_generator,
@@ -72,24 +76,20 @@ def test_storage_name_generation(storage_base, path_generator, filename_generato
 
     metadata = Metadata()
     metadata.set_context("folder", "folder")
-    media = Media(filename="dummy.txt")
+    media = Media(filename=dummy_file)
     storage.set_key(media, url, metadata)
     print(media.key)
     assert media.key == expected_key
 
 
-def test_really_long_name(storage_base):
+def test_really_long_name(storage_base, dummy_file):
     config: dict = {
         "path_generator": "url",
         "filename_generator": "static",
     }
     storage: Storage = storage_base(config)
 
-    # create dummy.txt file
-    with open("dummy.txt", "w") as f:
-        f.write("test content")
-
     url = f"https://example.com/{'file'*100}"
-    media = Media(filename="dummy.txt")
+    media = Media(filename=dummy_file)
     storage.set_key(media, url, Metadata())
     assert media.key == f"https-example-com-{'file'*13}/6ae8a75555209fd6c44157c0.txt"
\ No newline at end of file

From 3fcec57492b9383535b1bc5960ba9e3d40d1e030 Mon Sep 17 00:00:00 2001
From: Miguel Sozinho Ramalho <19508417+msramalho@users.noreply.github.com>
Date: Mon, 10 Mar 2025 17:17:59 +0000
Subject: [PATCH 6/6] minor string fix

---
 src/auto_archiver/modules/local_storage/local_storage.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/auto_archiver/modules/local_storage/local_storage.py b/src/auto_archiver/modules/local_storage/local_storage.py
index 6f1e217..2b1a101 100644
--- a/src/auto_archiver/modules/local_storage/local_storage.py
+++ b/src/auto_archiver/modules/local_storage/local_storage.py
@@ -13,7 +13,7 @@ class LocalStorage(Storage):
 
     def setup(self) -> None:
         if len(self.save_to) > 200:
-            raise SetupError(f"Your save_to path is  long, this will cause issues saving files on your computer. Please use a shorter path")
+            raise SetupError(f"Your save_to path is too long, this will cause issues saving files on your computer. Please use a shorter path.")
 
     def get_cdn_url(self, media: Media) -> str:
         dest = media.key