diff --git a/mobilede_scraper/mobile_de/mapper.py b/mobilede_scraper/mobile_de/mapper.py index e77a41f..b3c3b13 100644 --- a/mobilede_scraper/mobile_de/mapper.py +++ b/mobilede_scraper/mobile_de/mapper.py @@ -114,8 +114,11 @@ _COLOR_MAP = { } _IMAGE_FIELD_HINTS = ["image", "images", "media", "gallery", "photo", "pic", "picture", "url", "src", "uri", "ref"] -_IMAGE_URL_MARKERS = ["img.classistatic.de", "/images/", "/image/", "jpg", "jpeg", "png", "gif", "bmp", "tiff", "webp"] -_MOBILEDE_IMAGE_RULE = "mo-640.jpg" +_MOBILEDE_IMAGE_HOST = "img.classistatic.de" +_MOBILEDE_IMAGE_PATH_PREFIX = "/api/v1/mo-prod/images/" +_MOBILEDE_FULLRES_IMAGE_RULE = "mo-1600" +_MOBILEDE_PREVIEW_IMAGE_RULE = "mo-360" +_MOBILEDE_IMAGE_ID_RE = re.compile(r"^[0-9a-f]{2}/[0-9a-f-]{8,}$", re.IGNORECASE) _COLOR_VALUE_KEYS = {"ecol", "color", "exteriorcolor", "manufacturercolorname", "vehiclecolor", "paint"} _COUNTRY_VALUE_KEYS = {"country", "countrycode"} _BODY_VALUE_KEYS = {"category", "bodytype", "vehiclecategory", "body"} @@ -633,10 +636,20 @@ class MobileDeMapper: @staticmethod def _images_from_listing(raw: dict[str, Any]) -> list[ImageRecord]: - urls = MobileDeMapper._extract_image_urls(raw) + source_urls = dict.fromkeys(MobileDeMapper._extract_image_urls(raw)) return [ - ImageRecord(fullres_image=url, preview_image=url, order_index=index) - for index, url in enumerate(dict.fromkeys(url for url in urls if url)) + ImageRecord( + fullres_image=MobileDeMapper._with_mobilede_image_rule( + source_url, + _MOBILEDE_FULLRES_IMAGE_RULE, + ), + preview_image=MobileDeMapper._with_mobilede_image_rule( + source_url, + _MOBILEDE_PREVIEW_IMAGE_RULE, + ), + order_index=index, + ) + for index, source_url in enumerate(source_urls) ] @staticmethod @@ -648,7 +661,7 @@ class MobileDeMapper: if isinstance(value, str): if parent_hint or MobileDeMapper._looks_like_image_url(value): normalized = MobileDeMapper._normalize_image_url(value) - if MobileDeMapper._looks_like_image_url(normalized): + if normalized: urls.append(normalized) return urls if isinstance(value, list): @@ -669,12 +682,7 @@ class MobileDeMapper: @staticmethod def _looks_like_image_url(value: str) -> bool: - text = str(value or "").strip().lower() - if not text: - return False - if not (text.startswith("http://") or text.startswith("https://") or text.startswith("//") or text.startswith("/")): - return False - return any(marker in text for marker in _IMAGE_URL_MARKERS) + return bool(MobileDeMapper._normalize_image_url(value)) @staticmethod def _normalize_image_url(value: str) -> str: @@ -683,19 +691,35 @@ class MobileDeMapper: return "" if url.startswith("//"): url = f"https:{url}" + elif url.startswith(_MOBILEDE_IMAGE_HOST): + url = f"https://{url}" elif not (url.startswith("http://") or url.startswith("https://")): - url = f"https://{url.lstrip('/')}" - return MobileDeMapper._normalize_mobilede_image_rule(url) + return "" + + parsed = urlsplit(url) + if parsed.netloc.lower() != _MOBILEDE_IMAGE_HOST: + return "" + if not parsed.path.startswith(_MOBILEDE_IMAGE_PATH_PREFIX): + return "" + + image_id = parsed.path[len(_MOBILEDE_IMAGE_PATH_PREFIX):].strip("/") + if not _MOBILEDE_IMAGE_ID_RE.fullmatch(image_id): + return "" + + query_pairs = [ + (key, query_value) + for key, query_value in parse_qsl(parsed.query, keep_blank_values=True) + if key.lower() != "rule" + ] + return urlunsplit(("https", _MOBILEDE_IMAGE_HOST, parsed.path, urlencode(query_pairs), "")) @staticmethod - def _normalize_mobilede_image_rule(url: str) -> str: + def _with_mobilede_image_rule(url: str, rule: str) -> str: parsed = urlsplit(url) - if "img.classistatic.de" not in parsed.netloc.lower(): - return url - if "/api/v1/mo-prod/images/" not in parsed.path: - return url - query_pairs = parse_qsl(parsed.query, keep_blank_values=True) - if any(key.lower() == "rule" for key, _value in query_pairs): - return url - query_pairs.append(("rule", _MOBILEDE_IMAGE_RULE)) + query_pairs = [ + (key, query_value) + for key, query_value in parse_qsl(parsed.query, keep_blank_values=True) + if key.lower() != "rule" + ] + query_pairs.append(("rule", rule)) return urlunsplit((parsed.scheme, parsed.netloc, parsed.path, urlencode(query_pairs), parsed.fragment)) diff --git a/mobilede_scraper/storage/db.py b/mobilede_scraper/storage/db.py index 169942d..792ccfc 100644 --- a/mobilede_scraper/storage/db.py +++ b/mobilede_scraper/storage/db.py @@ -252,6 +252,8 @@ class PersistenceService: images: list[dict[str, object]], origin_id: str, ) -> int: + if not images: + return 0 nested = session.begin_nested() try: session.execute(delete(Image).where(Image.car_id == car_id)) @@ -325,6 +327,14 @@ class PersistenceService: entry["car_id"] = car_id insert_images_by_car_id: dict[int, list[dict[str, object]]] = {} + updated_entries = [ + entry for entry in entries + if str(entry.get("action") or "updated") == "updated" + ] + existing_images_map = self._load_existing_image_urls( + session, + {int(entry["car_id"]) for entry in updated_entries}, + ) updated_entries_needing_compare: list[dict[str, object]] = [] for entry in entries: @@ -332,11 +342,13 @@ class PersistenceService: action = str(entry.get("action") or "updated") images = entry["images"] skip_image_sync = bool(entry.get("skip_image_sync")) - if action == "updated" and (MOBILEDE_SKIP_IMAGES_FOR_UPDATED or skip_image_sync): - continue if action == "inserted": insert_images_by_car_id[car_id] = images continue + if not images or skip_image_sync: + continue + if MOBILEDE_SKIP_IMAGES_FOR_UPDATED and existing_images_map.get(car_id): + continue updated_entries_needing_compare.append(entry) for car_id, images in insert_images_by_car_id.items(): @@ -344,8 +356,6 @@ class PersistenceService: images_total += len(images) if updated_entries_needing_compare: - car_ids = {int(entry["car_id"]) for entry in updated_entries_needing_compare} - existing_images_map = self._load_existing_image_urls(session, car_ids) images_by_car_id: dict[int, list[dict[str, object]]] = {} replace_ids: list[int] = [] @@ -474,7 +484,7 @@ class PersistenceService: existing_by_id, existing_by_url = self._load_existing_cars(session, origin_ids, origin_urls) new_cars: list[tuple[Car, list[dict]]] = [] - update_image_candidates: list[tuple[Car, list[dict]]] = [] + updated_cars: list[tuple[Car, list[dict], bool]] = [] for record in records: payload = self._car_payload(record) @@ -489,9 +499,7 @@ class PersistenceService: else: self._apply_update_payload(car, payload) updated += 1 - if MOBILEDE_SKIP_IMAGES_FOR_UPDATED or self._skip_image_sync(record): - continue - update_image_candidates.append((car, images)) + updated_cars.append((car, images, self._skip_image_sync(record))) # Один flush. session.flush() @@ -502,12 +510,16 @@ class PersistenceService: images_total += len(images) update_cars_needing_images: list[tuple[Car, list[dict]]] = [] - if update_image_candidates: - update_ids = [int(car.id) for car, _ in update_image_candidates] + if updated_cars: + update_ids = [int(item[0].id) for item in updated_cars] existing_images_map = self._load_existing_image_urls(session, set(update_ids)) - for car, images in update_image_candidates: - new_image_urls = {img.get("fullres_image", "") for img in images} + for car, images, skip_image_sync in updated_cars: old_image_urls = existing_images_map.get(int(car.id), set()) + if not images or skip_image_sync: + continue + if MOBILEDE_SKIP_IMAGES_FOR_UPDATED and old_image_urls: + continue + new_image_urls = {img.get("fullres_image", "") for img in images} if new_image_urls != old_image_urls: update_cars_needing_images.append((car, images)) else: diff --git a/tests/test_db.py b/tests/test_db.py index b344043..666636d 100644 --- a/tests/test_db.py +++ b/tests/test_db.py @@ -101,6 +101,36 @@ class TestPersistenceServiceIntegration(unittest.TestCase): self.assertEqual(len(images), 1) self.assertIn("imageKeys=2", images[0].fullres_image) + def test_empty_update_preserves_existing_images(self) -> None: + first = self._record("mobile.de:keep-images") + self.persistence.upsert_car(first) + + update = self._record("mobile.de:keep-images", price=1500) + update.images = [] + result = self.persistence.upsert_car(update) + + with self.persistence.session_scope() as session: + images = session.execute(select(Image)).scalars().all() + + self.assertEqual(result["action"], "updated") + self.assertEqual(result["images_upserted"], 0) + self.assertEqual(len(images), 1) + + def test_batch_update_adds_images_when_existing_gallery_is_empty(self) -> None: + first = self._record("mobile.de:add-images") + first.images = [] + self.persistence.upsert_car(first) + + update = self._record("mobile.de:add-images", price=1500) + result = self.persistence.upsert_cars_batch([update]) + + with self.persistence.session_scope() as session: + images = session.execute(select(Image)).scalars().all() + + self.assertEqual(result["updated"], 1) + self.assertEqual(result["images_upserted"], 1) + self.assertEqual(len(images), 1) + def test_start_sync_run_marks_stale_running_runs_as_failed(self) -> None: first_run_id = self.persistence.start_sync_run("lane-a") second_run_id = self.persistence.start_sync_run("lane-b") diff --git a/tests/test_mappers.py b/tests/test_mappers.py index 27d5db7..2c10d79 100644 --- a/tests/test_mappers.py +++ b/tests/test_mappers.py @@ -11,10 +11,12 @@ class TestMobileDeMapper(unittest.TestCase): self.mapper = MobileDeMapper() def test_deduplicates_images(self) -> None: + first = "https://img.classistatic.de/api/v1/mo-prod/images/a1/a1111111-1111-4111-8111-111111111111" + second = "https://img.classistatic.de/api/v1/mo-prod/images/b2/b2222222-2222-4222-8222-222222222222" urls = [ - "https://img.classistatic.de/api/v1/mo-prod/images/1", - "https://img.classistatic.de/api/v1/mo-prod/images/1", - "//img.classistatic.de/api/v1/mo-prod/images/2", + f"{first}?rule=mo-80w", + f"{first}?rule=mo-1600", + second.replace("https:", ""), ] record = self.mapper.listing_to_car_record( @@ -27,8 +29,10 @@ class TestMobileDeMapper(unittest.TestCase): ) self.assertEqual(len(record.images), 2) - self.assertEqual(record.images[0].fullres_image, f"{urls[0]}?rule=mo-640.jpg") - self.assertEqual(record.images[1].fullres_image, "https://img.classistatic.de/api/v1/mo-prod/images/2?rule=mo-640.jpg") + self.assertEqual(record.images[0].fullres_image, f"{first}?rule=mo-1600") + self.assertEqual(record.images[0].preview_image, f"{first}?rule=mo-360") + self.assertEqual(record.images[1].fullres_image, f"{second}?rule=mo-1600") + self.assertEqual(record.images[1].preview_image, f"{second}?rule=mo-360") def test_extracts_nested_gallery_images_without_detail_fetch(self) -> None: record = self.mapper.listing_to_car_record( @@ -37,11 +41,11 @@ class TestMobileDeMapper(unittest.TestCase): url="https://suchen.mobile.de/fahrzeuge/details.html?id=124", title="Honda Civic", raw={ - "image": "https://img.classistatic.de/api/v1/mo-prod/images/main", - "images": [{"ref": "img.classistatic.de/api/v1/mo-prod/images/ref-1"}], + "image": "https://img.classistatic.de/api/v1/mo-prod/images/a1/a1111111-1111-4111-8111-111111111111", + "images": [{"ref": "img.classistatic.de/api/v1/mo-prod/images/b2/b2222222-2222-4222-8222-222222222222"}], "mediaGallery": [ - {"uri": "//img.classistatic.de/api/v1/mo-prod/images/gallery-1"}, - {"picture": {"src": "https://img.classistatic.de/api/v1/mo-prod/images/gallery-2"}}, + {"uri": "//img.classistatic.de/api/v1/mo-prod/images/c3/c3333333-3333-4333-8333-333333333333"}, + {"picture": {"src": "https://img.classistatic.de/api/v1/mo-prod/images/d4/d4444444-4444-4444-8444-444444444444"}}, ], "trackingUrl": "https://example.test/not-an-image", }, @@ -51,28 +55,45 @@ class TestMobileDeMapper(unittest.TestCase): self.assertEqual( [image.fullres_image for image in record.images], [ - "https://img.classistatic.de/api/v1/mo-prod/images/main?rule=mo-640.jpg", - "https://img.classistatic.de/api/v1/mo-prod/images/ref-1?rule=mo-640.jpg", - "https://img.classistatic.de/api/v1/mo-prod/images/gallery-1?rule=mo-640.jpg", - "https://img.classistatic.de/api/v1/mo-prod/images/gallery-2?rule=mo-640.jpg", + "https://img.classistatic.de/api/v1/mo-prod/images/a1/a1111111-1111-4111-8111-111111111111?rule=mo-1600", + "https://img.classistatic.de/api/v1/mo-prod/images/b2/b2222222-2222-4222-8222-222222222222?rule=mo-1600", + "https://img.classistatic.de/api/v1/mo-prod/images/c3/c3333333-3333-4333-8333-333333333333?rule=mo-1600", + "https://img.classistatic.de/api/v1/mo-prod/images/d4/d4444444-4444-4444-8444-444444444444?rule=mo-1600", ], ) - def test_keeps_existing_mobilede_image_rule(self) -> None: + def test_replaces_existing_mobilede_image_rule(self) -> None: + source = "https://img.classistatic.de/api/v1/mo-prod/images/a1/a1111111-1111-4111-8111-111111111111" record = self.mapper.listing_to_car_record( MobileDeListing( id="125", url="https://suchen.mobile.de/fahrzeuge/details.html?id=125", title="BMW 320", - raw={"images": [{"uri": "img.classistatic.de/api/v1/mo-prod/images/abc?rule=mo-1024.jpg"}]}, + raw={"images": [{"uri": f"{source}?rule=mo-80w"}]}, ) ) - self.assertEqual( - [image.fullres_image for image in record.images], - ["https://img.classistatic.de/api/v1/mo-prod/images/abc?rule=mo-1024.jpg"], + self.assertEqual(record.images[0].fullres_image, f"{source}?rule=mo-1600") + self.assertEqual(record.images[0].preview_image, f"{source}?rule=mo-360") + + def test_rejects_non_vehicle_mobilede_images(self) -> None: + record = self.mapper.listing_to_car_record( + MobileDeListing( + id="126", + url="https://suchen.mobile.de/fahrzeuge/details.html?id=126", + title="BMW 320", + raw={ + "images": [ + "https://img.classistatic.de/api/v1/mo-prod/images/co2class-G?rule=mo-640", + "https://img.classistatic.de/images/logo.svg", + "https://example.test/image.jpg", + ] + }, + ) ) + self.assertEqual(record.images, []) + def test_origin_id_uses_canonical_prefix(self) -> None: record = self.mapper.listing_to_car_record( MobileDeListing(