fix(catalog): sabitlenmiş browse satırlarını görüntülendikçe yenile
Some checks failed
QA Gate (P0/P1) / Test affected app (pull_request) Has been cancelled
Some checks failed
QA Gate (P0/P1) / Test affected app (pull_request) Has been cancelled
`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) <noreply@anthropic.com>
This commit is contained in:
173
apps/api/src/catalog/catalog-browse-heal.spec.ts
Normal file
173
apps/api/src/catalog/catalog-browse-heal.spec.ts
Normal file
@@ -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<void>) => 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<Row[] | null>;
|
||||
};
|
||||
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();
|
||||
});
|
||||
});
|
||||
@@ -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<typeof eq>[],
|
||||
): 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.
|
||||
*/
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user