fix: save foreign numeric-id downloads as inactive (#906)

This commit is contained in:
Jens
2026-03-25 14:26:09 +01:00
committed by GitHub
parent 5d1e9f1f80
commit ae9a720bbb
7 changed files with 234 additions and 20 deletions

View File

@@ -21,7 +21,7 @@ from .model.ad_model import MAX_DESCRIPTION_LENGTH, Ad, AdPartial, Contact, calc
from .model.config_model import DEFAULT_DOWNLOAD_DIR, Config
from .update_checker import UpdateChecker
from .utils import diagnostics, dicts, error_handlers, loggers, misc, xdg_paths
from .utils.exceptions import CaptchaEncountered, PublishSubmissionUncertainError
from .utils.exceptions import CaptchaEncountered, PublishedAdsFetchIncompleteError, PublishSubmissionUncertainError
from .utils.files import abspath
from .utils.i18n import Locale, get_current_locale, pluralize, set_current_locale
from .utils.misc import ainput, ensure, is_frozen
@@ -365,6 +365,19 @@ class KleinanzeigenBot(WebScrapingMixin): # noqa: PLR0904
return workspace.download_dir
return Path(abspath(trimmed_dir, relative_to = str(Path(self.config_file_path).parent))).resolve()
def _resolve_download_ad_activity(self, ad_id:int, published_ads_by_id:dict[int, dict[str, Any]]) -> tuple[bool, bool]:
"""Resolve downloaded ad activity and ownership for numeric selectors.
Returns:
tuple[bool, bool]:
active value and ownership (True own / False foreign).
"""
published_ad = published_ads_by_id.get(ad_id)
if published_ad is None:
return False, False
return published_ad.get("state") == "active", True
def _resolve_workspace(self) -> None:
"""
Resolve workspace paths after CLI args are parsed.
@@ -1493,9 +1506,12 @@ class KleinanzeigenBot(WebScrapingMixin): # noqa: PLR0904
return False
async def _fetch_published_ads(self) -> list[dict[str, Any]]:
async def _fetch_published_ads(self, *, strict:bool = False) -> list[dict[str, Any]]:
"""Fetch all published ads, handling API pagination.
Args:
strict: If True, raise PublishedAdsFetchIncompleteError when pagination data is incomplete.
Returns:
List of all published ads across all pages.
"""
@@ -1504,20 +1520,27 @@ class KleinanzeigenBot(WebScrapingMixin): # noqa: PLR0904
MAX_PAGE_LIMIT:Final[int] = 100
SNIPPET_LIMIT:Final[int] = 500
def _handle_incomplete_fetch(template:str, *args:Any, cause:Exception | None = None) -> None:
if strict:
raise PublishedAdsFetchIncompleteError(_(template) % args) from cause
while True:
# Safety check: don't paginate beyond reasonable limit
if page > MAX_PAGE_LIMIT:
LOG.warning("Stopping pagination after %s pages to avoid infinite loop", MAX_PAGE_LIMIT)
_handle_incomplete_fetch("Stopping pagination after %s pages to avoid infinite loop", MAX_PAGE_LIMIT)
break
try:
response = await self.web_request(f"{self.root_url}/m-meine-anzeigen-verwalten.json?sort=DEFAULT&pageNum={page}")
except TimeoutError as ex:
LOG.warning("Pagination request failed on page %s: %s", page, ex)
_handle_incomplete_fetch("Pagination request failed on page %s: %s", page, ex, cause = ex)
break
if not isinstance(response, dict):
LOG.warning("Unexpected pagination response type on page %s: %s", page, type(response).__name__)
_handle_incomplete_fetch("Unexpected pagination response type on page %s: %s", page, type(response).__name__)
break
content = response.get("content", "")
@@ -1527,6 +1550,7 @@ class KleinanzeigenBot(WebScrapingMixin): # noqa: PLR0904
content = content.decode("utf-8", errors = "replace")
if not isinstance(content, str):
LOG.warning("Unexpected response content type on page %s: %s", page, type(content).__name__)
_handle_incomplete_fetch("Unexpected response content type on page %s: %s", page, type(content).__name__)
break
try:
@@ -1534,14 +1558,17 @@ class KleinanzeigenBot(WebScrapingMixin): # noqa: PLR0904
except (json.JSONDecodeError, TypeError) as ex:
if not content:
LOG.warning("Empty JSON response content on page %s", page)
_handle_incomplete_fetch("Empty JSON response content on page %s", page, cause = ex)
break
snippet = content[:SNIPPET_LIMIT] + ("..." if len(content) > SNIPPET_LIMIT else "")
LOG.warning("Failed to parse JSON response on page %s: %s (content: %s)", page, ex, snippet)
_handle_incomplete_fetch("Failed to parse JSON response on page %s: %s (content: %s)", page, ex, snippet, cause = ex)
break
if not isinstance(json_data, dict):
snippet = content[:SNIPPET_LIMIT] + ("..." if len(content) > SNIPPET_LIMIT else "")
LOG.warning("Unexpected JSON payload on page %s (content: %s)", page, snippet)
_handle_incomplete_fetch("Unexpected JSON payload on page %s (content: %s)", page, snippet)
break
page_ads = json_data.get("ads", [])
@@ -1550,6 +1577,7 @@ class KleinanzeigenBot(WebScrapingMixin): # noqa: PLR0904
if len(preview) > SNIPPET_LIMIT:
preview = preview[:SNIPPET_LIMIT] + "..."
LOG.warning("Unexpected 'ads' type on page %s: %s value: %s", page, type(page_ads).__name__, preview)
_handle_incomplete_fetch("Unexpected 'ads' type on page %s: %s value: %s", page, type(page_ads).__name__, preview)
break
filtered_page_ads:list[dict[str, Any]] = []
@@ -1568,6 +1596,7 @@ class KleinanzeigenBot(WebScrapingMixin): # noqa: PLR0904
if len(preview) > SNIPPET_LIMIT:
preview = preview[:SNIPPET_LIMIT] + "..."
LOG.warning("Filtered %s malformed ad entries on page %s (sample: %s)", rejected_count, page, preview)
_handle_incomplete_fetch("Filtered %s malformed ad entries on page %s (sample: %s)", rejected_count, page, preview)
ads.extend(filtered_page_ads)
@@ -1582,6 +1611,7 @@ class KleinanzeigenBot(WebScrapingMixin): # noqa: PLR0904
if current_page_num is None:
LOG.warning("Invalid 'pageNum' in paging info: %s, stopping pagination", paging.get("pageNum"))
_handle_incomplete_fetch("Invalid 'pageNum' in paging info: %s, stopping pagination", paging.get("pageNum"))
break
if total_pages is None:
@@ -1604,6 +1634,7 @@ class KleinanzeigenBot(WebScrapingMixin): # noqa: PLR0904
next_page = misc.coerce_page_number(paging.get("next"))
if next_page is None:
LOG.warning("Invalid 'next' page value in paging info: %s, stopping pagination", paging.get("next"))
_handle_incomplete_fetch("Invalid 'next' page value in paging info: %s, stopping pagination", paging.get("next"))
break
page = next_page
@@ -2473,10 +2504,19 @@ class KleinanzeigenBot(WebScrapingMixin): # noqa: PLR0904
Determines which download mode was chosen with the arguments, and calls the specified download routine.
This downloads either all, only unsaved(new), or specific ads given by ID.
"""
# Normalize comma-separated keyword selectors; set deduplication collapses "new,new" → {"new"}
selector_tokens = {s.strip() for s in self.ads_selector.split(",")}
if "all" in selector_tokens:
effective_selector = "all"
elif len(selector_tokens) == 1:
effective_selector = next(iter(selector_tokens)) # e.g. "new,new" → "new"
else:
effective_selector = self.ads_selector # numeric IDs: "123,456" — unchanged
# Fetch published ads once from manage-ads JSON to avoid repetitive API calls during extraction
# Build lookup dict inline and pass directly to extractor (no cache abstraction needed)
LOG.info("Fetching published ads...")
published_ads = await self._fetch_published_ads()
published_ads = await self._fetch_published_ads(strict = bool(_NUMERIC_IDS_RE.match(effective_selector)))
published_ads_by_id:dict[int, dict[str, Any]] = {}
for published_ad in published_ads:
try:
@@ -2492,15 +2532,6 @@ class KleinanzeigenBot(WebScrapingMixin): # noqa: PLR0904
LOG.info("Ads download directory: %s", download_dir)
ad_extractor = extract.AdExtractor(self.browser, self.config, download_dir, published_ads_by_id = published_ads_by_id)
# Normalize comma-separated keyword selectors; set deduplication collapses "new,new" → {"new"}
selector_tokens = {s.strip() for s in self.ads_selector.split(",")}
if "all" in selector_tokens:
effective_selector = "all"
elif len(selector_tokens) == 1:
effective_selector = next(iter(selector_tokens)) # e.g. "new,new" → "new"
else:
effective_selector = self.ads_selector # numeric IDs: "123,456" — unchanged
if effective_selector in {"all", "new"}: # explore ads overview for these two modes
LOG.info("Scanning your ad overview...")
own_ad_urls = await ad_extractor.extract_own_ads_urls()
@@ -2553,7 +2584,11 @@ class KleinanzeigenBot(WebScrapingMixin): # noqa: PLR0904
for ad_id in ids: # call download routine for every id
exists = await ad_extractor.navigate_to_ad_page(ad_id)
if exists:
await ad_extractor.download_ad(ad_id)
resolved_active, ownership = self._resolve_download_ad_activity(ad_id, published_ads_by_id)
if not ownership:
LOG.warning("Ad id %d is not in your published profile ads. Saving downloaded ad as inactive.", ad_id)
await ad_extractor.download_ad(ad_id, active = resolved_active)
LOG.info("Downloaded ad with id %d", ad_id)
else:
LOG.error("The page with the id %d does not exist!", ad_id)

View File

@@ -98,19 +98,20 @@ class AdExtractor(WebScrapingMixin):
def _render_download_folder_name(self, ad_id:int, title:str) -> str:
return self._render_download_name_with_budget(self.config.download.folder_name_template, ad_id, title, self.config.download.folder_name_max_length)
async def download_ad(self, ad_id:int) -> None:
async def download_ad(self, ad_id:int, *, active:bool | None = None) -> None:
"""
Downloads an ad to a specific location, specified by config and ad ID.
NOTE: Requires that the driver session currently is on the ad page.
:param ad_id: the ad ID
:param active: optional override for ad activity state
"""
download_dir = self.download_dir
LOG.info("Using download directory: %s", download_dir)
# Extract ad info into a staging directory and determine final target directory
ad_cfg, staging_dir, final_dir, ad_file_stem = await self._extract_ad_page_info_with_directory_handling(download_dir, ad_id)
ad_cfg, staging_dir, final_dir, ad_file_stem = await self._extract_ad_page_info_with_directory_handling(download_dir, ad_id, active_override = active)
# Save the ad configuration file to staging first (offload to executor to avoid blocking the event loop)
ad_file_path = str(staging_dir / f"{ad_file_stem}.yaml")
@@ -328,7 +329,7 @@ class AdExtractor(WebScrapingMixin):
"""
return await self.web_text(By.ID, "viewad-title")
async def _extract_ad_page_info(self, directory:str, ad_id:int, ad_file_stem:str) -> AdPartial:
async def _extract_ad_page_info(self, directory:str, ad_id:int, ad_file_stem:str, *, active_override:bool | None = None) -> AdPartial:
"""
Extracts ad information and downloads images to the specified directory.
NOTE: Requires that the driver session currently is on the ad page.
@@ -336,9 +337,10 @@ class AdExtractor(WebScrapingMixin):
:param directory: the directory to download images to
:param ad_id: the ad ID
:param ad_file_stem: the rendered filename stem shared by the ad config and images
:param active_override: optional override for ad activity state
:return: an AdPartial object containing the ad information
"""
info:dict[str, Any] = {"active": True}
info:dict[str, Any] = {"active": active_override if active_override is not None else True}
# Extract title first (needed for directory creation)
title = await self._extract_title_from_ad_page()
@@ -410,12 +412,19 @@ class AdExtractor(WebScrapingMixin):
return ad_cfg
async def _extract_ad_page_info_with_directory_handling(self, relative_directory:Path, ad_id:int) -> tuple[AdPartial, Path, Path, str]:
async def _extract_ad_page_info_with_directory_handling(
self,
relative_directory:Path,
ad_id:int,
*,
active_override:bool | None = None,
) -> tuple[AdPartial, Path, Path, str]:
"""
Extracts ad information and handles directory creation/renaming.
:param relative_directory: Base directory for downloads
:param ad_id: The ad ID
:param active_override: optional override for ad activity state
:return: AdPartial with staging/final directory information and rendered ad file stem
"""
title = await self._extract_title_from_ad_page()
@@ -456,7 +465,7 @@ class AdExtractor(WebScrapingMixin):
LOG.info("New directory for ad created at %s.", staging_dir)
try:
ad_cfg = await self._extract_ad_page_info(str(staging_dir), ad_id, ad_file_stem)
ad_cfg = await self._extract_ad_page_info(str(staging_dir), ad_id, ad_file_stem, active_override = active_override)
except Exception:
if await files.exists(staging_dir):
try:

View File

@@ -272,6 +272,7 @@ kleinanzeigen_bot/__init__.py:
"%s were downloaded from your profile.": "%s wurden aus Ihrem Profil heruntergeladen."
"new ad": "neue Anzeige"
"Starting download of ad(s) with the id(s):": "Starte Download der Anzeige(n) mit den ID(s):"
"Ad id %d is not in your published profile ads. Saving downloaded ad as inactive.": "Anzeigen-ID %d ist nicht in Ihren veröffentlichten Profilanzeigen. Speichere die heruntergeladene Anzeige als inaktiv."
"Downloaded ad with id %d": "Anzeige mit der ID %d heruntergeladen"
"The page with the id %d does not exist!": "Die Seite mit der ID %d existiert nicht!"

View File

@@ -21,3 +21,7 @@ class PublishSubmissionUncertainError(KleinanzeigenBotError):
def __init__(self, reason:str) -> None:
super().__init__(reason)
class PublishedAdsFetchIncompleteError(KleinanzeigenBotError):
"""Raised when published ads cannot be fetched completely for ownership-critical operations."""

View File

@@ -1283,6 +1283,30 @@ class TestAdExtractorDownload:
assert final_dir.exists()
assert not staging_dir.exists()
@pytest.mark.asyncio
async def test_download_ad_passes_active_override(self, extractor:extract_module.AdExtractor, tmp_path:Path) -> None:
"""Test that download_ad forwards the active override to extraction."""
download_base = tmp_path / "downloaded-ads"
final_dir = download_base / "ad_12345_Test Advertisement Title"
staging_dir = download_base / ".tmp-ad_12345"
extractor.download_dir = download_base
staging_dir.mkdir(parents = True)
with (
patch("kleinanzeigen_bot.extract.dicts.save_dict", autospec = True),
patch.object(extractor, "_extract_ad_page_info_with_directory_handling", new_callable = AsyncMock) as mock_extract_with_dir,
):
mock_extract_with_dir.return_value = (
_create_test_ad_partial(active = False),
staging_dir,
final_dir,
"ad_12345",
)
await extractor.download_ad(12345, active = False)
mock_extract_with_dir.assert_awaited_once_with(download_base, 12345, active_override = False)
@pytest.mark.asyncio
async def test_download_ad_writes_schema_compliant_yaml(self, extractor:extract_module.AdExtractor, tmp_path:Path) -> None:
"""Test that downloaded ad YAML validates against ad.schema.json."""

View File

@@ -18,7 +18,7 @@ from kleinanzeigen_bot._version import __version__
from kleinanzeigen_bot.model.ad_model import Ad
from kleinanzeigen_bot.model.config_model import AdDefaults, Config, DiagnosticsConfig, PublishingConfig
from kleinanzeigen_bot.utils import dicts, loggers, xdg_paths
from kleinanzeigen_bot.utils.exceptions import PublishSubmissionUncertainError
from kleinanzeigen_bot.utils.exceptions import PublishedAdsFetchIncompleteError, PublishSubmissionUncertainError
from kleinanzeigen_bot.utils.web_scraping_mixin import By, Element
@@ -362,6 +362,125 @@ class TestKleinanzeigenBotInitialization:
mock_extractor.assert_called_once()
assert mock_extractor.call_args.args[2] == (tmp_path / "ads").resolve()
@pytest.mark.parametrize(("published_ads_by_id", "ad_id", "expected_active", "expected_ownership"), [
({123: {"id": 123, "state": "active"}}, 123, True, True),
({123: {"id": 123, "state": "inactive"}}, 123, False, True),
({123: {"id": 123, "state": "paused"}}, 123, False, True),
({}, 123, False, False),
])
def test_resolve_download_ad_activity(
self,
test_bot:KleinanzeigenBot,
published_ads_by_id:dict[int, dict[str, Any]],
ad_id:int,
expected_active:bool,
expected_ownership:bool,
) -> None:
resolved_active, ownership = test_bot._resolve_download_ad_activity(ad_id, published_ads_by_id)
assert resolved_active is expected_active
assert ownership is expected_ownership
@pytest.mark.asyncio
@pytest.mark.parametrize(
"scenario",
[
{
"published_ads": [{"id": 123, "state": "active"}],
"expected_active": True,
"expect_warning": False,
},
{
"published_ads": [{"id": 999, "state": "active"}],
"expected_active": False,
"expect_warning": True,
},
{
"published_ads": [{"id": 123, "state": "inactive"}],
"expected_active": False,
"expect_warning": False,
},
{
"published_ads": [{"id": 123, "state": "paused"}],
"expected_active": False,
"expect_warning": False,
},
],
)
async def test_download_ads_numeric_selector_resolves_and_passes_active_state(
self,
test_bot:KleinanzeigenBot,
tmp_path:Path,
caplog:pytest.LogCaptureFixture,
scenario:dict[str, Any],
) -> None:
published_ads = scenario["published_ads"]
expected_active = scenario["expected_active"]
expect_warning = scenario["expect_warning"]
test_bot.workspace = xdg_paths.Workspace.for_config(tmp_path / "config.yaml", "kleinanzeigen-bot")
test_bot.ads_selector = "123"
test_bot.browser = MagicMock()
extractor_mock = MagicMock()
extractor_mock.navigate_to_ad_page = AsyncMock(return_value = True)
extractor_mock.download_ad = AsyncMock()
caplog.set_level(logging.WARNING)
with (
patch.object(test_bot, "_fetch_published_ads", new_callable = AsyncMock, return_value = published_ads) as mock_fetch_published_ads,
patch("kleinanzeigen_bot.extract.AdExtractor", return_value = extractor_mock),
):
await test_bot.download_ads()
mock_fetch_published_ads.assert_awaited_once_with(strict = True)
extractor_mock.download_ad.assert_awaited_once_with(123, active = expected_active)
warning_messages = [record.getMessage() for record in caplog.records if record.levelno >= logging.WARNING]
if expect_warning:
assert any("123" in msg for msg in warning_messages)
else:
assert len(warning_messages) == 0
@pytest.mark.asyncio
async def test_download_ads_numeric_selector_fails_when_published_ads_fetch_incomplete(self, test_bot:KleinanzeigenBot, tmp_path:Path) -> None:
test_bot.workspace = xdg_paths.Workspace.for_config(tmp_path / "config.yaml", "kleinanzeigen-bot")
test_bot.ads_selector = "123"
test_bot.browser = MagicMock()
with (
patch.object(
test_bot,
"_fetch_published_ads",
new_callable = AsyncMock,
side_effect = PublishedAdsFetchIncompleteError("incomplete fetch"),
),
patch("kleinanzeigen_bot.extract.AdExtractor") as mock_extractor,
pytest.raises(PublishedAdsFetchIncompleteError, match = "incomplete fetch"),
):
await test_bot.download_ads()
mock_extractor.assert_not_called()
@pytest.mark.asyncio
async def test_download_ads_all_selector_uses_tolerant_published_ads_fetch(self, test_bot:KleinanzeigenBot, tmp_path:Path) -> None:
test_bot.workspace = xdg_paths.Workspace.for_config(tmp_path / "config.yaml", "kleinanzeigen-bot")
test_bot.ads_selector = "all"
test_bot.browser = MagicMock()
extractor_mock = MagicMock()
extractor_mock.extract_own_ads_urls = AsyncMock(return_value = [])
with (
patch.object(test_bot, "_fetch_published_ads", new_callable = AsyncMock, return_value = []) as mock_fetch_published_ads,
patch("kleinanzeigen_bot.extract.AdExtractor", return_value = extractor_mock),
):
await test_bot.download_ads()
mock_fetch_published_ads.assert_awaited_once_with(strict = False)
class TestKleinanzeigenBotLogging:
"""Tests for logging functionality."""

View File

@@ -10,6 +10,7 @@ import pytest
from kleinanzeigen_bot import KleinanzeigenBot
from kleinanzeigen_bot.utils import misc
from kleinanzeigen_bot.utils.exceptions import PublishedAdsFetchIncompleteError
@pytest.mark.unit
@@ -263,6 +264,18 @@ class TestJSONPagination:
if "Filtered 3 malformed ad entries on page 1" not in caplog.text:
pytest.fail(f"expected malformed-entry warning in logs, got: {caplog.text}")
@pytest.mark.asyncio
async def test_fetch_published_ads_strict_raises_on_malformed_entries(self, bot:KleinanzeigenBot) -> None:
"""Strict fetch should raise when malformed entries are detected."""
response_data = {"ads": [42, {"id": 1, "state": "active"}, "broken"], "paging": {"pageNum": 1, "last": 1}}
mock_request = AsyncMock(return_value = {"content": json.dumps(response_data)})
with (
patch.object(bot, "web_request", mock_request),
pytest.raises(PublishedAdsFetchIncompleteError, match = "Filtered 2 malformed ad entries on page 1"),
):
await bot._fetch_published_ads(strict = True)
@pytest.mark.asyncio
async def test_fetch_published_ads_timeout(self, bot:KleinanzeigenBot) -> None:
"""Test handling of timeout during pagination."""
@@ -274,6 +287,15 @@ class TestJSONPagination:
if result != []:
pytest.fail(f"Expected empty list on timeout, got {result}")
@pytest.mark.asyncio
async def test_fetch_published_ads_strict_raises_on_timeout(self, bot:KleinanzeigenBot) -> None:
"""Strict fetch should raise when pagination cannot be completed."""
with (
patch.object(bot, "web_request", new_callable = AsyncMock, side_effect = TimeoutError("timeout")),
pytest.raises(PublishedAdsFetchIncompleteError, match = "Pagination request failed on page 1"),
):
await bot._fetch_published_ads(strict = True)
@pytest.mark.asyncio
async def test_fetch_published_ads_handles_non_string_content_type(self, bot:KleinanzeigenBot, caplog:pytest.LogCaptureFixture) -> None:
"""Unexpected non-string content types should stop pagination with warning."""