fix(api): add sync safety guards against degenerate SDS responses
Add SYNC_GUARDS thresholds so a partial/degenerate upstream response never triggers a destructive operation: - skip stale category deletion when the fetched tree is suspiciously small vs the existing SDS category count - skip delist detection unless both leaf-category and seen-product counts are healthy Verified: 77 tests pass; live SDS returns 226 categories (guard off), incident-case ratios (2/226, 2/2) are correctly blocked.
This commit is contained in:
@@ -0,0 +1,36 @@
|
||||
import { Test } from '@nestjs/testing';
|
||||
import { of } from 'rxjs';
|
||||
import { HttpService } from '@nestjs/axios';
|
||||
import { ConfigService } from '@nestjs/config';
|
||||
import { SdsClientService } from './sds-client.service';
|
||||
|
||||
describe('SdsClientService', () => {
|
||||
let service: SdsClientService;
|
||||
let http: { post: jest.Mock; get: jest.Mock };
|
||||
|
||||
beforeEach(async () => {
|
||||
http = { post: jest.fn(), get: jest.fn() };
|
||||
const moduleRef = await Test.createTestingModule({
|
||||
providers: [
|
||||
SdsClientService,
|
||||
{ provide: HttpService, useValue: http },
|
||||
{ provide: ConfigService, useValue: { get: jest.fn(() => undefined) } },
|
||||
],
|
||||
}).compile();
|
||||
service = moduleRef.get(SdsClientService);
|
||||
});
|
||||
|
||||
describe('fetchCategoryTree', () => {
|
||||
it('throws when the upstream returns a degenerate small tree', async () => {
|
||||
http.post.mockReturnValue(of({ data: [{ id: 1, name: 'Only' }] }));
|
||||
await expect(service.fetchCategoryTree()).rejects.toThrow(/degenerate/i);
|
||||
});
|
||||
|
||||
it('returns the tree when it is healthy', async () => {
|
||||
const tree = Array.from({ length: 20 }, (_, i) => ({ id: i + 1, name: `C${i}` }));
|
||||
http.post.mockReturnValue(of({ data: tree }));
|
||||
const result = await service.fetchCategoryTree();
|
||||
expect(result).toHaveLength(20);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -9,6 +9,13 @@ const POD_HEADERS = {
|
||||
Referer: 'https://inkpod.vip/',
|
||||
} as const;
|
||||
|
||||
/**
|
||||
* Minimum number of category nodes a healthy `category/tree/3` response contains.
|
||||
* Below this the response is treated as degenerate and rejected so the caller
|
||||
* never runs a destructive sync against a partial tree.
|
||||
*/
|
||||
export const MIN_SDS_CATEGORY_NODES = 10;
|
||||
|
||||
export interface SdsCategoryTreeNode {
|
||||
id: number | string | null;
|
||||
name?: string;
|
||||
@@ -109,6 +116,12 @@ export class SdsClientService {
|
||||
if (!Array.isArray(data)) {
|
||||
throw new Error(`SDS category tree returned ${typeof data}, expected array`);
|
||||
}
|
||||
if (data.length < MIN_SDS_CATEGORY_NODES) {
|
||||
throw new Error(
|
||||
`SDS category tree is degenerate (${data.length} nodes < ${MIN_SDS_CATEGORY_NODES}) — ` +
|
||||
`aborting to avoid destructive sync`,
|
||||
);
|
||||
}
|
||||
return data as SdsCategoryTreeNode[];
|
||||
}
|
||||
|
||||
|
||||
@@ -1,6 +1,10 @@
|
||||
import { Test } from '@nestjs/testing';
|
||||
import { ConfigModule } from '@nestjs/config';
|
||||
import { SyncService } from './sync.service';
|
||||
import {
|
||||
SyncService,
|
||||
shouldRunDelistDetection,
|
||||
shouldSkipStaleDeletion,
|
||||
} from './sync.service';
|
||||
import { SdsClientService } from './sds-client.service';
|
||||
import { PrismaService } from '../prisma/prisma.service';
|
||||
|
||||
@@ -168,6 +172,98 @@ describe('SyncService', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('sync guard thresholds', () => {
|
||||
describe('shouldSkipStaleDeletion', () => {
|
||||
it('skips stale deletion when the fetched count is below the hard floor', () => {
|
||||
expect(shouldSkipStaleDeletion(2, 226)).toBe(true);
|
||||
expect(shouldSkipStaleDeletion(9, 226)).toBe(true);
|
||||
});
|
||||
|
||||
it('skips stale deletion when fetched is far smaller than existing (ratio guard)', () => {
|
||||
expect(shouldSkipStaleDeletion(100, 250)).toBe(true);
|
||||
});
|
||||
|
||||
it('does NOT skip when fetched count is healthy', () => {
|
||||
expect(shouldSkipStaleDeletion(226, 226)).toBe(false);
|
||||
expect(shouldSkipStaleDeletion(200, 226)).toBe(false);
|
||||
});
|
||||
|
||||
it('does NOT skip when there are no existing SDS categories', () => {
|
||||
expect(shouldSkipStaleDeletion(0, 0)).toBe(false);
|
||||
expect(shouldSkipStaleDeletion(2, 0)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('shouldRunDelistDetection', () => {
|
||||
it('skips delist detection when leaf categories are too few', () => {
|
||||
expect(shouldRunDelistDetection(2, 500)).toBe(false);
|
||||
expect(shouldRunDelistDetection(9, 500)).toBe(false);
|
||||
});
|
||||
|
||||
it('skips delist detection when the seen product count is too small', () => {
|
||||
expect(shouldRunDelistDetection(148, 2)).toBe(false);
|
||||
expect(shouldRunDelistDetection(148, 49)).toBe(false);
|
||||
});
|
||||
|
||||
it('runs delist detection only when both metrics are healthy', () => {
|
||||
expect(shouldRunDelistDetection(148, 500)).toBe(true);
|
||||
expect(shouldRunDelistDetection(10, 50)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
it('category sync keeps existing SDS categories when upstream returns a degenerate tree', async () => {
|
||||
const stamp = Date.now();
|
||||
const keep = await prisma.category.create({
|
||||
data: { sdsCategoryId: `keep-${stamp}`, categoryName: `Keep ${stamp}` },
|
||||
});
|
||||
createdSdsCategoryIds.push(`keep-${stamp}`);
|
||||
|
||||
sds.fetchCategoryTree.mockResolvedValueOnce([
|
||||
{ id: `g-${stamp}-1`, name: 'Tiny 1' },
|
||||
{ id: `g-${stamp}-2`, name: 'Tiny 2' },
|
||||
]);
|
||||
createdSdsCategoryIds.push(`g-${stamp}-1`, `g-${stamp}-2`);
|
||||
|
||||
const result = await service.syncCategories();
|
||||
expect(result.deletedStale).toBe(0);
|
||||
|
||||
const still = await prisma.category.findUnique({ where: { id: keep.id } });
|
||||
expect(still).not.toBeNull();
|
||||
});
|
||||
|
||||
it('product sync does NOT delist origin goods when it sees too few products', async () => {
|
||||
const stamp = Date.now();
|
||||
const leaf = await prisma.category.create({
|
||||
data: { sdsCategoryId: `leafguard-${stamp}`, categoryName: `LeafGuard ${stamp}` },
|
||||
});
|
||||
createdSdsCategoryIds.push(`leafguard-${stamp}`);
|
||||
|
||||
const active = await prisma.originGood.create({
|
||||
data: { sdsGoodId: `active-${stamp}`, delisted: false, goodName: 'Active' },
|
||||
});
|
||||
createdSdsGoodIds.push(`active-${stamp}`);
|
||||
|
||||
sds.fetchProductsPage.mockImplementation(async (categoryId) => {
|
||||
if (categoryId === `leafguard-${stamp}`) {
|
||||
return {
|
||||
content: [
|
||||
{ id: `guardp-${stamp}-1`, name: 'P1' },
|
||||
{ id: `guardp-${stamp}-2`, name: 'P2' },
|
||||
],
|
||||
};
|
||||
}
|
||||
return { content: [] };
|
||||
});
|
||||
|
||||
const result = await service.syncProducts();
|
||||
expect(result.delisted).toBe(0);
|
||||
|
||||
const still = await prisma.originGood.findUnique({ where: { id: active.id } });
|
||||
expect(still?.delisted).toBe(false);
|
||||
createdSdsGoodIds.push(`guardp-${stamp}-1`, `guardp-${stamp}-2`);
|
||||
});
|
||||
});
|
||||
|
||||
describe('getStatus', () => {
|
||||
it('returns recent logs ordered by startedAt desc', async () => {
|
||||
const logs = await service.getStatus(5);
|
||||
|
||||
@@ -8,6 +8,7 @@ export interface CategorySyncResult {
|
||||
inserted: number;
|
||||
updated: number;
|
||||
total: number;
|
||||
deletedStale: number;
|
||||
}
|
||||
|
||||
export interface ProductSyncResult {
|
||||
@@ -15,6 +16,44 @@ export interface ProductSyncResult {
|
||||
updated: number;
|
||||
total: number;
|
||||
leafCategories: number;
|
||||
delisted: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* Safety guards so a degenerate/partial SDS response never triggers a
|
||||
* destructive operation (stale category deletion / mass delist marking).
|
||||
* Upstream normally returns ~226 categories and ~150 leaf categories with
|
||||
* hundreds of products — the floors below only trigger on abnormal responses.
|
||||
*/
|
||||
export const SYNC_GUARDS = {
|
||||
MIN_CATEGORY_COUNT: 10,
|
||||
MIN_CATEGORY_RATIO: 0.5,
|
||||
MIN_LEAF_CATEGORIES: 10,
|
||||
MIN_SEEN_GOODS: 50,
|
||||
} as const;
|
||||
|
||||
/**
|
||||
* True when the fetched category count is suspiciously small compared to the
|
||||
* categories already synced from SDS, i.e. the upstream response is likely
|
||||
* partial/degenerate. In that case stale deletion must be skipped.
|
||||
*/
|
||||
export function shouldSkipStaleDeletion(fetched: number, existingSds: number): boolean {
|
||||
if (existingSds <= 0) return false;
|
||||
return (
|
||||
fetched < SYNC_GUARDS.MIN_CATEGORY_COUNT ||
|
||||
fetched < SYNC_GUARDS.MIN_CATEGORY_RATIO * existingSds
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* True only when both the leaf-category count and the number of seen products
|
||||
* are healthy enough to trust the "not seen upstream => delisted" conclusion.
|
||||
*/
|
||||
export function shouldRunDelistDetection(leafCategories: number, seenGoods: number): boolean {
|
||||
return (
|
||||
leafCategories >= SYNC_GUARDS.MIN_LEAF_CATEGORIES &&
|
||||
seenGoods >= SYNC_GUARDS.MIN_SEEN_GOODS
|
||||
);
|
||||
}
|
||||
|
||||
@Injectable()
|
||||
@@ -82,8 +121,23 @@ export class SyncService {
|
||||
this.logger.log(`Fetched ${flat.length} SDS categories`);
|
||||
const seenSdsIds = new Set(flat.map((n) => n.sdsId));
|
||||
|
||||
// Guard: if the upstream tree is suspiciously small vs what we already
|
||||
// have from SDS, skip stale deletion entirely — a partial response must
|
||||
// never wipe the category library.
|
||||
const existingSdsCount = await this.prisma.category.count({
|
||||
where: { sdsCategoryId: { not: null } },
|
||||
});
|
||||
const skipStaleDeletion = shouldSkipStaleDeletion(flat.length, existingSdsCount);
|
||||
if (skipStaleDeletion) {
|
||||
this.logger.warn(
|
||||
`Skipping stale category deletion: fetched=${flat.length} existingSds=${existingSdsCount} ` +
|
||||
`(below guard thresholds)`,
|
||||
);
|
||||
}
|
||||
|
||||
// Single transaction: upsert + wire parents + delete stale.
|
||||
// SDS tree is the source of truth — anything not in the response gets deleted.
|
||||
// SDS tree is the source of truth — anything not in the response gets deleted
|
||||
// (unless the response looks degenerate, see guard above).
|
||||
const { inserted, updated, deletedStale } = await this.prisma.$transaction(async (tx) => {
|
||||
let ins = 0;
|
||||
let upd = 0;
|
||||
@@ -133,50 +187,55 @@ export class SyncService {
|
||||
|
||||
// 3. Delete stale categories (in DB but not in SDS response)
|
||||
// Detach parent links first, then delete leaf-first to respect FK constraints.
|
||||
const staleCats = await tx.category.findMany({
|
||||
where: { sdsCategoryId: { notIn: [...seenSdsIds] } },
|
||||
select: { id: true },
|
||||
});
|
||||
const staleIds = staleCats.map((c) => c.id);
|
||||
|
||||
// Protect categories that have configured goods — onDelete: Restrict
|
||||
const goodsInStale = await tx.good.groupBy({
|
||||
by: ['categoryId'],
|
||||
where: { categoryId: { in: staleIds } },
|
||||
});
|
||||
const protectedIds = new Set(goodsInStale.map((g) => g.categoryId));
|
||||
const deletableIds = staleIds.filter((id) => !protectedIds.has(id));
|
||||
|
||||
// Detach all deletable categories from their parents
|
||||
if (deletableIds.length > 0) {
|
||||
await tx.category.updateMany({
|
||||
where: { id: { in: deletableIds } },
|
||||
data: { parentCategoryId: null },
|
||||
// Skipped entirely when the response looks degenerate (see guard above).
|
||||
let deletedStale = 0;
|
||||
if (!skipStaleDeletion) {
|
||||
const staleCats = await tx.category.findMany({
|
||||
where: { sdsCategoryId: { notIn: [...seenSdsIds] } },
|
||||
select: { id: true },
|
||||
});
|
||||
// Also detach any non-deletable children pointing to deletable parents
|
||||
await tx.category.updateMany({
|
||||
where: { parentCategoryId: { in: deletableIds } },
|
||||
data: { parentCategoryId: null },
|
||||
const staleIds = staleCats.map((c) => c.id);
|
||||
|
||||
// Protect categories that have configured goods — onDelete: Restrict
|
||||
const goodsInStale = await tx.good.groupBy({
|
||||
by: ['categoryId'],
|
||||
where: { categoryId: { in: staleIds } },
|
||||
});
|
||||
// Delete leaf-first (repeatedly remove nodes with no children)
|
||||
let remaining = [...deletableIds];
|
||||
while (remaining.length > 0) {
|
||||
const withChildren = await tx.category.findMany({
|
||||
where: { parentCategoryId: { in: remaining } },
|
||||
select: { parentCategoryId: true },
|
||||
distinct: ['parentCategoryId'],
|
||||
const protectedIds = new Set(goodsInStale.map((g) => g.categoryId));
|
||||
const deletableIds = staleIds.filter((id) => !protectedIds.has(id));
|
||||
deletedStale = deletableIds.length;
|
||||
|
||||
// Detach all deletable categories from their parents
|
||||
if (deletableIds.length > 0) {
|
||||
await tx.category.updateMany({
|
||||
where: { id: { in: deletableIds } },
|
||||
data: { parentCategoryId: null },
|
||||
});
|
||||
const hasChildSet = new Set(
|
||||
withChildren.filter((c) => c.parentCategoryId).map((c) => c.parentCategoryId!.toString()),
|
||||
);
|
||||
const leaves = remaining.filter((id) => !hasChildSet.has(id.toString()));
|
||||
if (leaves.length === 0) break; // safety: circular dependency
|
||||
await tx.category.deleteMany({ where: { id: { in: leaves } } });
|
||||
remaining = remaining.filter((id) => !leaves.some((l) => l === id));
|
||||
// Also detach any non-deletable children pointing to deletable parents
|
||||
await tx.category.updateMany({
|
||||
where: { parentCategoryId: { in: deletableIds } },
|
||||
data: { parentCategoryId: null },
|
||||
});
|
||||
// Delete leaf-first (repeatedly remove nodes with no children)
|
||||
let remaining = [...deletableIds];
|
||||
while (remaining.length > 0) {
|
||||
const withChildren = await tx.category.findMany({
|
||||
where: { parentCategoryId: { in: remaining } },
|
||||
select: { parentCategoryId: true },
|
||||
distinct: ['parentCategoryId'],
|
||||
});
|
||||
const hasChildSet = new Set(
|
||||
withChildren.filter((c) => c.parentCategoryId).map((c) => c.parentCategoryId!.toString()),
|
||||
);
|
||||
const leaves = remaining.filter((id) => !hasChildSet.has(id.toString()));
|
||||
if (leaves.length === 0) break; // safety: circular dependency
|
||||
await tx.category.deleteMany({ where: { id: { in: leaves } } });
|
||||
remaining = remaining.filter((id) => !leaves.some((l) => l === id));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return { inserted: ins, updated: upd, deletedStale: deletableIds.length };
|
||||
return { inserted: ins, updated: upd, deletedStale };
|
||||
});
|
||||
|
||||
await this.prisma.syncLog.update({
|
||||
@@ -187,7 +246,7 @@ export class SyncService {
|
||||
message: `inserted=${inserted} updated=${updated} total=${flat.length} staleDeleted=${deletedStale}`,
|
||||
},
|
||||
});
|
||||
return { inserted, updated, total: flat.length };
|
||||
return { inserted, updated, total: flat.length, deletedStale };
|
||||
} catch (err) {
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
await this.prisma.syncLog.update({
|
||||
@@ -256,9 +315,13 @@ export class SyncService {
|
||||
|
||||
// Detect delisted products: mark origin goods not seen in upstream as delisted,
|
||||
// and re-activate any previously delisted goods that reappeared.
|
||||
// Guard: only trust this conclusion when the sync covered a healthy number of
|
||||
// leaf categories and saw a healthy number of products — otherwise a partial
|
||||
// sync must never mass-delist the product library.
|
||||
let delistedCount = 0;
|
||||
let reactivatedCount = 0;
|
||||
if (seenSdsGoodIds.size > 0) {
|
||||
const runDelist = shouldRunDelistDetection(leafRows.length, seenSdsGoodIds.size);
|
||||
if (runDelist) {
|
||||
const delistedResult = await this.prisma.originGood.updateMany({
|
||||
where: { sdsGoodId: { notIn: [...seenSdsGoodIds] }, delisted: false },
|
||||
data: { delisted: true },
|
||||
@@ -270,7 +333,10 @@ export class SyncService {
|
||||
delistedCount = delistedResult.count;
|
||||
reactivatedCount = reactivatedResult.count;
|
||||
} else {
|
||||
this.logger.warn('No products seen from SDS — skipping delist detection');
|
||||
this.logger.warn(
|
||||
`Skipping delist detection: leafCategories=${leafRows.length} seenGoods=${seenSdsGoodIds.size} ` +
|
||||
`(below guard thresholds)`,
|
||||
);
|
||||
}
|
||||
|
||||
await this.prisma.syncLog.update({
|
||||
@@ -281,7 +347,7 @@ export class SyncService {
|
||||
message: `inserted=${inserted} updated=${updated} total=${total} delisted=${delistedCount} reactivated=${reactivatedCount} leafCategories=${leafRows.length}`,
|
||||
},
|
||||
});
|
||||
return { inserted, updated, total, leafCategories: leafRows.length };
|
||||
return { inserted, updated, total, leafCategories: leafRows.length, delisted: delistedCount };
|
||||
} catch (err) {
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
await this.prisma.syncLog.update({
|
||||
|
||||
Reference in New Issue
Block a user