Preserve confirmed vehicle details
This commit is contained in:
@@ -260,6 +260,9 @@ class MobileDeMapper:
|
||||
repair_history=False,
|
||||
slug=self._slugify(" ".join(part for part in [title or f"{brand} {model}", str(year) if year is not None else ""] if part)),
|
||||
last_seen_at=datetime.now(timezone.utc),
|
||||
details_confirmed=False,
|
||||
images_confirmed=False,
|
||||
preserve_existing_details=True,
|
||||
images=self._images_from_listing(raw),
|
||||
)
|
||||
|
||||
@@ -358,6 +361,9 @@ class MobileDeMapper:
|
||||
repair_history=False,
|
||||
slug=self._slugify(" ".join(part for part in [title or f"{brand} {model}", str(year) if year is not None else ""] if part)),
|
||||
last_seen_at=datetime.now(timezone.utc),
|
||||
details_confirmed=True,
|
||||
images_confirmed=True,
|
||||
preserve_existing_details=False,
|
||||
images=self._images_from_listing(detail),
|
||||
)
|
||||
|
||||
|
||||
@@ -27,6 +27,20 @@ _IN_CHUNK_SIZE = 5000
|
||||
CAR_TABLE_NAME = Car.__tablename__
|
||||
MOBILEDE_ORIGIN_PREFIXES = ("mobile.de:", "mobilede:")
|
||||
PARSER_ID_RE = re.compile(r"^car-[A-Za-z0-9]{22}$")
|
||||
DETAIL_VALUE_FALLBACKS: dict[str, set[object]] = {
|
||||
"brand": {None, "", "UNKNOWN"},
|
||||
"model": {None, "", "UNKNOWN"},
|
||||
"year": {None},
|
||||
"price": {None},
|
||||
"mileage": {None, 0},
|
||||
"country": {None, "", "NA"},
|
||||
"color": {None, "", "other"},
|
||||
"drive": {None, "NA"},
|
||||
"gearbox": {None, "NA"},
|
||||
"body_type": {None, "OTHER", "NA"},
|
||||
"engine_volume": {None},
|
||||
"evaluation": {None, ""},
|
||||
}
|
||||
|
||||
|
||||
def _origin_prefix_filter(column, prefixes: tuple[str, ...] = MOBILEDE_ORIGIN_PREFIXES):
|
||||
@@ -173,6 +187,19 @@ class PersistenceService:
|
||||
payload = record.model_dump(mode="python")
|
||||
return {key: value for key, value in payload.items() if key in CAR_DB_FIELDS}
|
||||
|
||||
@staticmethod
|
||||
def _prepare_update_payload(
|
||||
car: Car,
|
||||
payload: dict[str, object],
|
||||
record: CarRecord,
|
||||
) -> dict[str, object]:
|
||||
prepared = dict(payload)
|
||||
if record.preserve_existing_details and not record.details_confirmed:
|
||||
for key, fallback_values in DETAIL_VALUE_FALLBACKS.items():
|
||||
if prepared.get(key) in fallback_values:
|
||||
prepared[key] = getattr(car, key)
|
||||
return prepared
|
||||
|
||||
@staticmethod
|
||||
def _apply_update_payload(car: Car, payload: dict[str, object]) -> None:
|
||||
for key, value in payload.items():
|
||||
@@ -185,6 +212,27 @@ class PersistenceService:
|
||||
def _skip_image_sync(record: CarRecord) -> bool:
|
||||
return bool(getattr(record, "skip_image_sync", False))
|
||||
|
||||
@classmethod
|
||||
def _should_sync_images(
|
||||
cls,
|
||||
record: CarRecord,
|
||||
images: list[dict[str, object]],
|
||||
existing_urls: set[str],
|
||||
) -> bool:
|
||||
if not images or cls._skip_image_sync(record):
|
||||
return False
|
||||
if record.images_confirmed:
|
||||
return True
|
||||
return not existing_urls
|
||||
|
||||
@staticmethod
|
||||
def _incoming_image_urls(images: list[dict[str, object]]) -> set[str]:
|
||||
return {
|
||||
str(image.get("fullres_image") or "")
|
||||
for image in images
|
||||
if image.get("fullres_image")
|
||||
}
|
||||
|
||||
def _is_postgres(self) -> bool:
|
||||
return self.engine.dialect.name == "postgresql"
|
||||
|
||||
@@ -283,6 +331,9 @@ class PersistenceService:
|
||||
images = [image.model_dump(mode="python") for image in record.images]
|
||||
car_by_id = existing_by_id.get(record.origin_id)
|
||||
car_by_url = existing_by_url.get(record.origin_url)
|
||||
existing_car = car_by_id or car_by_url
|
||||
if existing_car is not None:
|
||||
payload = self._prepare_update_payload(existing_car, payload, record)
|
||||
if car_by_id is not None and PARSER_ID_RE.fullmatch(str(car_by_id.parser_id or "")):
|
||||
payload["parser_id"] = car_by_id.parser_id
|
||||
entry: dict[str, object] = {
|
||||
@@ -345,9 +396,12 @@ class PersistenceService:
|
||||
if action == "inserted":
|
||||
insert_images_by_car_id[car_id] = images
|
||||
continue
|
||||
if not images or skip_image_sync:
|
||||
if skip_image_sync:
|
||||
continue
|
||||
if MOBILEDE_SKIP_IMAGES_FOR_UPDATED and existing_images_map.get(car_id):
|
||||
existing_urls = existing_images_map.get(car_id, set())
|
||||
if not self._should_sync_images(entry["record"], images, existing_urls):
|
||||
continue
|
||||
if MOBILEDE_SKIP_IMAGES_FOR_UPDATED and existing_urls and not entry["record"].images_confirmed:
|
||||
continue
|
||||
updated_entries_needing_compare.append(entry)
|
||||
|
||||
@@ -394,11 +448,15 @@ class PersistenceService:
|
||||
car_by_id = session.execute(
|
||||
select(Car).where(Car.origin_id == record.origin_id)
|
||||
).scalar_one_or_none()
|
||||
if car_by_id is not None:
|
||||
payload = self._prepare_update_payload(car_by_id, payload, record)
|
||||
if car_by_id is not None and PARSER_ID_RE.fullmatch(str(car_by_id.parser_id or "")):
|
||||
payload["parser_id"] = car_by_id.parser_id
|
||||
car_by_url = session.execute(
|
||||
select(Car).where(Car.origin_url == record.origin_url)
|
||||
).scalar_one_or_none()
|
||||
if car_by_id is None and car_by_url is not None:
|
||||
payload = self._prepare_update_payload(car_by_url, payload, record)
|
||||
if car_by_url is not None and car_by_url.origin_id != record.origin_id:
|
||||
self._apply_update_payload(car_by_url, payload)
|
||||
session.flush()
|
||||
@@ -414,6 +472,12 @@ class PersistenceService:
|
||||
car_id = int(session.execute(upsert_stmt).scalar_one())
|
||||
action = "updated" if existed or car_by_url is not None else "inserted"
|
||||
|
||||
existing_urls = self._load_existing_image_urls(session, {car_id}).get(car_id, set())
|
||||
images_upserted = 0
|
||||
if self._should_sync_images(record, images, existing_urls):
|
||||
if self._incoming_image_urls(images) == existing_urls:
|
||||
images_upserted = len(existing_urls)
|
||||
else:
|
||||
images_upserted = self._replace_images_for_car(
|
||||
session, car_id, images, record.origin_id,
|
||||
)
|
||||
@@ -429,8 +493,15 @@ class PersistenceService:
|
||||
session.flush()
|
||||
else:
|
||||
action = "updated"
|
||||
payload = self._prepare_update_payload(car, payload, record)
|
||||
self._apply_update_payload(car, payload)
|
||||
session.flush()
|
||||
existing_urls = self._load_existing_image_urls(session, {int(car.id)}).get(int(car.id), set())
|
||||
images_upserted = 0
|
||||
if self._should_sync_images(record, images, existing_urls):
|
||||
if self._incoming_image_urls(images) == existing_urls:
|
||||
images_upserted = len(existing_urls)
|
||||
else:
|
||||
images_upserted = self._replace_images_for_car(session, int(car.id), images, record.origin_id)
|
||||
return {"car_id": int(car.id), "images_upserted": images_upserted, "action": action}
|
||||
self._add_images(session, int(car.id), images)
|
||||
@@ -484,7 +555,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]]] = []
|
||||
updated_cars: list[tuple[Car, list[dict], bool]] = []
|
||||
updated_cars: list[tuple[Car, list[dict], bool, CarRecord]] = []
|
||||
|
||||
for record in records:
|
||||
payload = self._car_payload(record)
|
||||
@@ -497,9 +568,10 @@ class PersistenceService:
|
||||
inserted += 1
|
||||
new_cars.append((car, images))
|
||||
else:
|
||||
payload = self._prepare_update_payload(car, payload, record)
|
||||
self._apply_update_payload(car, payload)
|
||||
updated += 1
|
||||
updated_cars.append((car, images, self._skip_image_sync(record)))
|
||||
updated_cars.append((car, images, self._skip_image_sync(record), record))
|
||||
|
||||
# Один flush.
|
||||
session.flush()
|
||||
@@ -513,11 +585,13 @@ class PersistenceService:
|
||||
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, skip_image_sync in updated_cars:
|
||||
for car, images, skip_image_sync, record in updated_cars:
|
||||
old_image_urls = existing_images_map.get(int(car.id), set())
|
||||
if not images or skip_image_sync:
|
||||
if skip_image_sync:
|
||||
continue
|
||||
if MOBILEDE_SKIP_IMAGES_FOR_UPDATED and old_image_urls:
|
||||
if not self._should_sync_images(record, images, old_image_urls):
|
||||
continue
|
||||
if MOBILEDE_SKIP_IMAGES_FOR_UPDATED and old_image_urls and not record.images_confirmed:
|
||||
continue
|
||||
new_image_urls = {img.get("fullres_image", "") for img in images}
|
||||
if new_image_urls != old_image_urls:
|
||||
|
||||
@@ -41,6 +41,9 @@ class CarRecord(BaseModel):
|
||||
last_seen_at: datetime = Field(default_factory=lambda: datetime.now(timezone.utc))
|
||||
sold_at: datetime | None = None
|
||||
skip_image_sync: bool = False
|
||||
details_confirmed: bool = False
|
||||
images_confirmed: bool = False
|
||||
preserve_existing_details: bool = True
|
||||
images: list[ImageRecord] = Field(default_factory=list)
|
||||
|
||||
|
||||
|
||||
@@ -40,6 +40,9 @@ class TestPersistenceServiceIntegration(unittest.TestCase):
|
||||
origin_url=f"https://www.MOBILEDE.com/VehicleDetail/{origin_id}~US",
|
||||
origin_id=origin_id,
|
||||
slug=f"toyota-camry-{origin_id}",
|
||||
details_confirmed=True,
|
||||
images_confirmed=True,
|
||||
preserve_existing_details=False,
|
||||
images=[
|
||||
ImageRecord(
|
||||
fullres_image="https://vis.MOBILEDE.com/resizer?imageKeys=1&width=845&height=633",
|
||||
@@ -131,6 +134,89 @@ class TestPersistenceServiceIntegration(unittest.TestCase):
|
||||
self.assertEqual(result["images_upserted"], 1)
|
||||
self.assertEqual(len(images), 1)
|
||||
|
||||
def test_search_update_preserves_confirmed_details_and_gallery(self) -> None:
|
||||
detail = self._record("mobile.de:confirmed", price=1000)
|
||||
detail.drive = "4WD"
|
||||
detail.gearbox = "AT"
|
||||
detail.body_type = "SUV"
|
||||
detail.engine_volume = 1998
|
||||
detail.color = "black"
|
||||
detail.mileage = 45000
|
||||
detail.images = [
|
||||
ImageRecord(
|
||||
fullres_image=f"https://example.test/detail-{index}.jpg",
|
||||
preview_image=f"https://example.test/detail-{index}-preview.jpg",
|
||||
order_index=index,
|
||||
)
|
||||
for index in range(3)
|
||||
]
|
||||
self.persistence.upsert_car(detail)
|
||||
|
||||
search = self._record("mobile.de:confirmed", price=1200)
|
||||
search.details_confirmed = False
|
||||
search.images_confirmed = False
|
||||
search.preserve_existing_details = True
|
||||
search.drive = None
|
||||
search.gearbox = None
|
||||
search.body_type = "OTHER"
|
||||
search.engine_volume = None
|
||||
search.color = "other"
|
||||
search.mileage = 0
|
||||
search.images = [
|
||||
ImageRecord(
|
||||
fullres_image="https://example.test/search.jpg",
|
||||
preview_image="https://example.test/search-preview.jpg",
|
||||
order_index=0,
|
||||
)
|
||||
]
|
||||
self.persistence.upsert_car(search)
|
||||
|
||||
with self.persistence.session_scope() as session:
|
||||
car = session.execute(select(Car).where(Car.origin_id == detail.origin_id)).scalar_one()
|
||||
images = session.execute(
|
||||
select(Image).where(Image.car_id == car.id).order_by(Image.order_index)
|
||||
).scalars().all()
|
||||
|
||||
self.assertEqual(car.price, 1200)
|
||||
self.assertEqual(car.drive, "4WD")
|
||||
self.assertEqual(car.gearbox, "AT")
|
||||
self.assertEqual(car.body_type, "SUV")
|
||||
self.assertEqual(car.engine_volume, 1998)
|
||||
self.assertEqual(car.color, "black")
|
||||
self.assertEqual(car.mileage, 45000)
|
||||
self.assertEqual([image.fullres_image for image in images], [
|
||||
"https://example.test/detail-0.jpg",
|
||||
"https://example.test/detail-1.jpg",
|
||||
"https://example.test/detail-2.jpg",
|
||||
])
|
||||
|
||||
def test_confirmed_detail_update_replaces_values_and_gallery(self) -> None:
|
||||
first = self._record("mobile.de:detail-refresh")
|
||||
first.drive = "FWD"
|
||||
first.body_type = "SEDAN"
|
||||
self.persistence.upsert_car(first)
|
||||
|
||||
refreshed = self._record("mobile.de:detail-refresh", price=2000)
|
||||
refreshed.drive = "RWD"
|
||||
refreshed.body_type = "COUPE"
|
||||
refreshed.images = [
|
||||
ImageRecord(
|
||||
fullres_image="https://example.test/refreshed.jpg",
|
||||
preview_image="https://example.test/refreshed-preview.jpg",
|
||||
order_index=0,
|
||||
)
|
||||
]
|
||||
self.persistence.upsert_car(refreshed)
|
||||
|
||||
with self.persistence.session_scope() as session:
|
||||
car = session.execute(select(Car).where(Car.origin_id == first.origin_id)).scalar_one()
|
||||
images = session.execute(select(Image).where(Image.car_id == car.id)).scalars().all()
|
||||
|
||||
self.assertEqual(car.price, 2000)
|
||||
self.assertEqual(car.drive, "RWD")
|
||||
self.assertEqual(car.body_type, "COUPE")
|
||||
self.assertEqual([image.fullres_image for image in images], ["https://example.test/refreshed.jpg"])
|
||||
|
||||
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")
|
||||
|
||||
@@ -106,6 +106,9 @@ class TestMobileDeMapper(unittest.TestCase):
|
||||
self.assertEqual(record.origin_id, "mobile.de:456")
|
||||
self.assertEqual(record.origin, "MOBILEDE")
|
||||
self.assertEqual(record.selling_type, "STOCK")
|
||||
self.assertFalse(record.details_confirmed)
|
||||
self.assertFalse(record.images_confirmed)
|
||||
self.assertTrue(record.preserve_existing_details)
|
||||
|
||||
def test_slug_includes_year_without_duplicate_or_trailing_hyphens(self) -> None:
|
||||
record = self.mapper.listing_to_car_record(
|
||||
@@ -415,6 +418,9 @@ class TestMobileDeMapper(unittest.TestCase):
|
||||
self.assertEqual(record.selling_type, "STOCK")
|
||||
self.assertFalse(record.non_smoking)
|
||||
self.assertFalse(record.repair_history)
|
||||
self.assertTrue(record.details_confirmed)
|
||||
self.assertTrue(record.images_confirmed)
|
||||
self.assertFalse(record.preserve_existing_details)
|
||||
|
||||
def test_drive_prefers_explicit_attr_marker_over_title_noise(self) -> None:
|
||||
record = self.mapper.listing_to_car_record(
|
||||
|
||||
Reference in New Issue
Block a user