From eb18c9a12232275b5c680fa5325fdc7d26d24a59 Mon Sep 17 00:00:00 2001 From: Semih Yesilyurt Date: Sun, 20 Sep 2026 08:35:09 +0300 Subject: [PATCH] =?UTF-8?q?fix(catalog):=20sabitlenmi=C5=9F=20browse=20sat?= =?UTF-8?q?=C4=B1rlar=C4=B1n=C4=B1=20g=C3=B6r=C3=BCnt=C3=BClendik=C3=A7e?= =?UTF-8?q?=20yenile?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `catalog_vehicles` kalıcı bir cache: `getModels` markanın tek bir satırı varsa erken dönüyor, dolayısıyla `fetchVehicleList` o marka için bir daha hiç çağrılmıyor. PSA ve Volvo hâlâ P4 iken listelenmiş 188 satır (prod 2026-09-20: 127 LEGACY_PSA + 61 LEGACY_VOLVO) bu yüzden kalıcı olarak çakılı kalmıştı: - Peugeot/Citroën browse donmuş 2024-02-13 anlık görüntüsünü sunuyor, - Volvo/Polestar browse HTTP 503 dönen bir uca gidiyor. Kod düzeltmesi bu satırlara hiçbir zaman ulaşmıyordu. `isStaleBrowseArchitecture()` + `CatalogService.healStaleBrowseRows()`: satırın kayıtlı mimarisi servis tablosundakiyle uyuşmuyorsa marka bir kez yeniden listeleniyor, yeni satırlar yazılıp eskiler aynı transaction'da siliniyor. VIN tarafındaki onarma ile aynı temkinli kurallar: marka başına günde bir deneme (Redis kilidi), başarısız veya boş listede eski satırlar korunuyor, silme ancak yenisi elde edilince ve servis bazında yapılıyor. 8 yeni test; api paketi 610 test geçiyor. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/catalog/catalog-browse-heal.spec.ts | 173 ++++++++++++++++++ apps/api/src/catalog/catalog.service.ts | 117 +++++++++++- apps/api/src/integrations/pl24/pl24-tree.ts | 25 +++ 3 files changed, 313 insertions(+), 2 deletions(-) create mode 100644 apps/api/src/catalog/catalog-browse-heal.spec.ts diff --git a/apps/api/src/catalog/catalog-browse-heal.spec.ts b/apps/api/src/catalog/catalog-browse-heal.spec.ts new file mode 100644 index 0000000..2b5eeb7 --- /dev/null +++ b/apps/api/src/catalog/catalog-browse-heal.spec.ts @@ -0,0 +1,173 @@ +import { describe, expect, it, vi } from "vitest"; +import { isStaleBrowseArchitecture } from "../integrations/pl24/pl24-tree"; +import { CatalogService } from "./catalog.service"; + +/** + * Regression lock for the pinned-browse-rows bug (plv2.md, finding consumers-09). + * + * `catalog_vehicles` is a permanent cache: `getModels` returns early as soon as + * a brand has any row, so `fetchVehicleList` never runs again for that brand. + * The 188 rows listed while PSA and Volvo were still P4 (127 LEGACY_PSA + 61 + * LEGACY_VOLVO on prod, 2026-09-20) were therefore stuck serving the frozen + * 2024-02-13 PSA snapshot and a Volvo endpoint that answers HTTP 503. + */ + +type Row = { + id: string; + serviceName: string; + architecture: string | null; + brandId: string | null; +}; + +const psaRow = (id: string): Row => ({ + id, + serviceName: "peugeot_parts", + architecture: "LEGACY_PSA", + brandId: "b1", +}); +const freshRow = (id: string): Row => ({ + id, + serviceName: "peugeot_parts", + architecture: "P5_MODERN", + brandId: "b1", +}); + +function makeService(opts: { + lockFree?: boolean; + fetched?: Array<{ model: string; vehicleId: string }>; + fetchThrows?: boolean; + refetched?: Row[]; +}) { + const deleted: string[][] = []; + const inserted: unknown[][] = []; + + const tx = { + insert: () => ({ + values: (v: unknown[]) => { + inserted.push(v); + return { onConflictDoNothing: async () => undefined }; + }, + }), + delete: () => ({ + where: async (cond: unknown) => { + // drizzle's inArray() is opaque here; record the call itself. + deleted.push([String(cond)]); + return undefined; + }, + }), + }; + + const db = { + select: () => ({ from: () => ({ where: async () => opts.refetched ?? [] }) }), + transaction: async (cb: (t: typeof tx) => Promise) => cb(tx), + }; + const redis = { setNx: vi.fn(async () => opts.lockFree !== false) }; + const pl24Service = { + fetchVehicleList: vi.fn(async () => { + if (opts.fetchThrows) throw new Error("upstream 503"); + return opts.fetched ?? []; + }), + }; + + const svc = new CatalogService( + db as never, + redis as never, + pl24Service as never, + {} as never, + ) as never as { + healStaleBrowseRows: (brand: string, rows: Row[], where: unknown[]) => Promise; + }; + return { svc, redis, pl24Service, deleted, inserted }; +} + +describe("isStaleBrowseArchitecture", () => { + it("flags a row whose stored architecture is not the current one", () => { + expect( + isStaleBrowseArchitecture({ + storedArchitecture: "LEGACY_PSA", + currentArchitecture: "P5_MODERN", + }), + ).toBe(true); + expect( + isStaleBrowseArchitecture({ + storedArchitecture: "LEGACY_VOLVO", + currentArchitecture: "P5_MODERN", + }), + ).toBe(true); + }); + + it("leaves a matching row alone", () => { + expect( + isStaleBrowseArchitecture({ + storedArchitecture: "P5_MODERN", + currentArchitecture: "P5_MODERN", + }), + ).toBe(false); + // Brands that are still P4 upstream must not be touched. + expect( + isStaleBrowseArchitecture({ + storedArchitecture: "LEGACY_FORD", + currentArchitecture: "LEGACY_FORD", + }), + ).toBe(false); + }); + + it("does nothing without both sides (unknown service / unlabelled row)", () => { + expect( + isStaleBrowseArchitecture({ storedArchitecture: null, currentArchitecture: "P5_MODERN" }), + ).toBe(false); + expect( + isStaleBrowseArchitecture({ storedArchitecture: "LEGACY_PSA", currentArchitecture: null }), + ).toBe(false); + }); +}); + +describe("healStaleBrowseRows", () => { + it("does not touch PL24 when every row is current", async () => { + const { svc, pl24Service, redis } = makeService({}); + const out = await svc.healStaleBrowseRows("Peugeot", [freshRow("a"), freshRow("b")], []); + expect(out).toBeNull(); + expect(pl24Service.fetchVehicleList).not.toHaveBeenCalled(); + expect(redis.setNx).not.toHaveBeenCalled(); + }); + + it("re-lists a stale brand and replaces its rows", async () => { + const refetched = [freshRow("new1"), freshRow("new2")]; + const { svc, pl24Service, deleted, inserted } = makeService({ + fetched: [ + { model: "208", vehicleId: "p5-208" }, + { model: "308", vehicleId: "p5-308" }, + ], + refetched, + }); + const out = await svc.healStaleBrowseRows("Peugeot", [psaRow("old1"), psaRow("old2")], []); + expect(pl24Service.fetchVehicleList).toHaveBeenCalledWith("peugeot_parts"); + expect(inserted).toHaveLength(1); + expect(inserted[0]).toHaveLength(2); + expect(deleted).toHaveLength(1); + expect(out).toEqual(refetched); + }); + + it("keeps the stale rows when the re-list throws", async () => { + const { svc, deleted, inserted } = makeService({ fetchThrows: true }); + const out = await svc.healStaleBrowseRows("Volvo", [psaRow("old1")], []); + expect(out).toBeNull(); + expect(inserted).toHaveLength(0); + expect(deleted).toHaveLength(0); + }); + + it("keeps the stale rows when upstream returns an empty model list", async () => { + const { svc, deleted, inserted } = makeService({ fetched: [] }); + const out = await svc.healStaleBrowseRows("Volvo", [psaRow("old1")], []); + expect(out).toBeNull(); + expect(inserted).toHaveLength(0); + expect(deleted).toHaveLength(0); + }); + + it("attempts at most once a day per brand", async () => { + const { svc, pl24Service } = makeService({ lockFree: false }); + const out = await svc.healStaleBrowseRows("Peugeot", [psaRow("old1")], []); + expect(out).toBeNull(); + expect(pl24Service.fetchVehicleList).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/api/src/catalog/catalog.service.ts b/apps/api/src/catalog/catalog.service.ts index c567c36..8897153 100644 --- a/apps/api/src/catalog/catalog.service.ts +++ b/apps/api/src/catalog/catalog.service.ts @@ -19,7 +19,7 @@ import { userBrands, userSubscriptions, } from "../database/schema/core"; -import { isPl24LeafNode } from "../integrations/pl24/pl24-tree"; +import { isPl24LeafNode, isStaleBrowseArchitecture } from "../integrations/pl24/pl24-tree"; import { PL24Service } from "../integrations/pl24/pl24.service"; import { type PL24DecodedCategory, @@ -170,7 +170,8 @@ export class CatalogService { .where(and(...whereConditions)); if (dbVehicles.length > 0) { - return dbVehicles; + const healed = await this.healStaleBrowseRows(brandName, dbVehicles, whereConditions); + return healed ?? dbVehicles; } // Fetch from PL24 for each service @@ -225,6 +226,118 @@ export class CatalogService { return allVehicles; } + /** + * Re-list a brand whose stored browse rows were created under a retired PL24 + * architecture, and replace them with the current ones. + * + * WHY (plv2.md, finding consumers-09): `getModels` treats `catalog_vehicles` + * as a permanent cache — one row for the brand and `fetchVehicleList` is never + * called again. The rows listed while PSA and Volvo were still P4 are pinned + * forever, so Peugeot/Citroën browse keeps serving the frozen 2024-02-13 + * snapshot and Volvo/Polestar browse keeps calling an endpoint that answers + * HTTP 503. A code fix alone never reaches these rows. + * + * Deliberately conservative, mirroring `healStalePl24Architecture` on the + * VIN side: + * - only rows whose stored architecture disagrees with the service table; + * - one attempt per brand per day (Redis lock) so a permanently failing + * re-list cannot hammer the one surviving account on every page view; + * - a failed or empty re-list leaves the old rows in place — stale data + * beats an empty catalog; + * - the stale rows are deleted only once the replacements are committed, + * and per service, so one broken sub-catalog cannot wipe a working one. + * + * Deleting a browse row cascades to its categories and parts. That is + * intended: those rows describe the retired tree and would otherwise survive + * as unreachable orphans under the new listing. + * + * Returns the refreshed rows, or null when nothing was migrated (caller keeps + * what it already had). + */ + private async healStaleBrowseRows( + brandName: string, + dbVehicles: (typeof catalogVehicles.$inferSelect)[], + whereConditions: ReturnType[], + ): Promise<(typeof catalogVehicles.$inferSelect)[] | null> { + const staleServices = [ + ...new Set( + dbVehicles + .filter((v) => + isStaleBrowseArchitecture({ + storedArchitecture: v.architecture, + currentArchitecture: PL24_SERVICE_CATALOGS[v.serviceName]?.architecture, + }), + ) + .map((v) => v.serviceName), + ), + ]; + if (staleServices.length === 0) return null; + + if (!(await this.redis.setNx(`pl24:browse-heal:${brandName}`, "1", 86_400))) return null; + + this.logger.log( + `[pl24-browse-heal] ${brandName}: ${staleServices.join(", ")} listed under a retired architecture — re-listing`, + ); + + let migrated = 0; + for (const svc of staleServices) { + const staleIds = dbVehicles.filter((v) => v.serviceName === svc).map((v) => v.id); + try { + const fetched = await this.pl24Service.fetchVehicleList(svc); + if (fetched.length === 0) { + this.logger.warn( + `[pl24-browse-heal] ${svc}: upstream returned no models — keeping ${staleIds.length} stale row(s)`, + ); + continue; + } + const config = PL24_SERVICE_CATALOGS[svc]; + const brandId = dbVehicles.find((v) => v.serviceName === svc)?.brandId ?? null; + await this.db.transaction(async (tx) => { + await tx + .insert(catalogVehicles) + .values( + fetched.map((v) => ({ + source: "pl24" as const, + serviceName: svc, + brandName, + brandId, + model: v.model, + year: v.year || null, + engine: v.engine || null, + bodyType: v.bodyType || null, + transmission: v.transmission || null, + market: v.market || null, + serviceVehicleId: v.vehicleId, + catalogPath: v.catalogPath || null, + architecture: config?.architecture || "P5_MODERN", + metadata: v.metadata || null, + categoriesFetched: false, + updatedAt: new Date(), + })), + ) + .onConflictDoNothing(); + if (staleIds.length > 0) { + await tx.delete(catalogVehicles).where(inArray(catalogVehicles.id, staleIds)); + } + }); + migrated += staleIds.length; + this.logger.log( + `[pl24-browse-heal] ${svc}: ${staleIds.length} stale row(s) → ${fetched.length} fresh model(s)`, + ); + } catch (err) { + this.logger.warn( + `[pl24-browse-heal] ${svc} re-list failed, keeping stale rows: ${(err as Error).message}`, + ); + } + } + + if (migrated === 0) return null; + return await this.db + .select() + .from(catalogVehicles) + .where(and(...whereConditions)); + } + /** * Get a single catalog vehicle by ID. */ diff --git a/apps/api/src/integrations/pl24/pl24-tree.ts b/apps/api/src/integrations/pl24/pl24-tree.ts index 7a5e9ac..7b35b91 100644 --- a/apps/api/src/integrations/pl24/pl24-tree.ts +++ b/apps/api/src/integrations/pl24/pl24-tree.ts @@ -86,3 +86,28 @@ export function isStalePl24Architecture(opts: { if (!storedIsLegacyPsaOrVolvo) return false; return current.startsWith("/p5"); } + +/** + * True when a stored `catalog_vehicles` browse row was listed under an + * architecture the service table no longer uses. + * + * Browse rows are a PERMANENT cache: `CatalogService.getModels` returns early + * whenever the brand already has rows, so `fetchVehicleList` is never called + * again for that brand. The 188 rows listed while PSA and Volvo were still P4 + * (127 LEGACY_PSA + 61 LEGACY_VOLVO on prod, 2026-09-20) are therefore pinned + * forever: Peugeot/Citroën browse serves the frozen 2024-02-13 snapshot and + * Volvo/Polestar browse serves an endpoint that answers HTTP 503. Detecting the + * mismatch at list time lets the brand re-list itself once, the same way + * `isStalePl24Architecture` heals a VIN-decoded vehicle. + */ +export function isStaleBrowseArchitecture(opts: { + storedArchitecture?: string | null; + currentArchitecture?: string | null; +}): boolean { + const stored = opts.storedArchitecture?.trim(); + const current = opts.currentArchitecture?.trim(); + // An unknown service (no config) or an unlabelled row is left alone: without a + // current architecture to compare against there is nothing to migrate TO. + if (!stored || !current) return false; + return stored !== current; +}