Skip to content

Commit 688cfb4

Browse files
fix(cloud-connection): reseed reports honest result instead of fake success (#2172)
1 parent d6a7464 commit 688cfb4

2 files changed

Lines changed: 206 additions & 5 deletions

File tree

packages/cloud-connection/src/marketplace-install-local-plugin.ts

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -517,7 +517,31 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
517517
}, 400);
518518
}
519519

520-
// Persist flag flip
520+
const inserted = summary.seeded.inserted ?? 0;
521+
const updated = summary.seeded.updated ?? 0;
522+
const errors = summary.seeded.errors ?? 0;
523+
const wrote = inserted + updated > 0;
524+
525+
// HONEST RESULT: the loader runs row-by-row and counts write failures
526+
// (locked DB, missing table, validation reject) into `errors` rather
527+
// than throwing. Previously this handler returned success — and flipped
528+
// `withSampleData` to true — even when every row failed, so the UI said
529+
// "done" while the database stayed empty. Treat a run that landed no
530+
// rows as a failure and report why.
531+
if (!wrote) {
532+
return c.json({
533+
success: false,
534+
error: {
535+
code: 'reseed_no_rows',
536+
message: errors > 0
537+
? `Reseed wrote no rows (${errors} error${errors === 1 ? '' : 's'}).${summary.seeded.errorSample ? ` First error: ${summary.seeded.errorSample}` : ''}`
538+
: 'Reseed wrote no rows. The package declares no seedable records for this runtime.',
539+
details: { inserted, updated, errors },
540+
},
541+
}, 422);
542+
}
543+
544+
// Only mark the install as carrying sample data once rows actually landed.
521545
try {
522546
entry.withSampleData = true;
523547
this.ledger.write(entry);
@@ -527,9 +551,9 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
527551
success: true,
528552
data: {
529553
manifestId,
530-
inserted: summary.seeded.inserted ?? 0,
531-
updated: summary.seeded.updated ?? 0,
532-
errors: summary.seeded.errors ?? 0,
554+
inserted,
555+
updated,
556+
errors,
533557
withSampleData: true,
534558
},
535559
}, 200);
@@ -644,7 +668,7 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
644668
ctx: PluginContext,
645669
manifest: any,
646670
opts: { seedNow: boolean; c?: any },
647-
): Promise<{ translationsLoaded: number; seeded: { mode: 'inline' | 'replayer' | 'skipped'; inserted?: number; updated?: number; errors?: number; reason?: string } }> => {
671+
): Promise<{ translationsLoaded: number; seeded: { mode: 'inline' | 'replayer' | 'skipped'; inserted?: number; updated?: number; errors?: number; reason?: string; errorSample?: string } }> => {
648672
const appId = String(manifest?.id ?? 'unknown');
649673
let translationsLoaded = 0;
650674
let seedSummary: any = { mode: 'skipped', reason: 'no-datasets' };
@@ -758,6 +782,11 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
758782
inserted: result.summary.totalInserted,
759783
updated: result.summary.totalUpdated,
760784
errors: result.errors.length,
785+
// Surface the first write/resolution failure so the
786+
// caller can report WHY nothing landed (e.g. a locked
787+
// DB, a missing table, a failed validation) instead of
788+
// a bare "0 rows".
789+
errorSample: result.errors[0]?.message,
761790
};
762791
ctx.logger?.info?.(`[MarketplaceInstallLocal] inline seed for ${appId}${organizationId ? ` (org=${organizationId})` : ''}: inserted=${seedSummary.inserted} updated=${seedSummary.updated} errors=${seedSummary.errors}`);
763792
}
Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* HONEST RESULT for the reseed endpoint.
5+
*
6+
* The SeedLoaderService runs row-by-row and counts write failures (a locked
7+
* DB, a missing table, a rejected validation) into `result.errors` rather than
8+
* throwing. The reseed handler used to return `success: true` — AND flip the
9+
* install's `withSampleData` flag to true — whenever the loader didn't outright
10+
* skip, so a run that wrote ZERO rows still reported success while the database
11+
* stayed empty (the "提示成功但没有数据" bug). These tests pin the corrected
12+
* behaviour: no rows written => failure + flag stays false; rows written =>
13+
* success + flag flips.
14+
*/
15+
16+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
17+
import { mkdtempSync, rmSync } from 'node:fs';
18+
import { join } from 'node:path';
19+
import { tmpdir } from 'node:os';
20+
21+
// Controls what the (mocked) seed loader reports back. The handler under test
22+
// only cares about result.summary.total{Inserted,Updated} + result.errors.
23+
let seedResult: any = { summary: { totalInserted: 0, totalUpdated: 0 }, errors: [] };
24+
25+
vi.mock('@objectstack/runtime', () => ({
26+
SeedLoaderService: class {
27+
async load() { return seedResult; }
28+
},
29+
}));
30+
vi.mock('@objectstack/spec/data', () => ({
31+
SeedLoaderRequestSchema: { parse: (x: any) => x },
32+
}));
33+
34+
import { MarketplaceInstallLocalPlugin } from './marketplace-install-local-plugin.js';
35+
36+
type Handler = (c: any) => Promise<any>;
37+
38+
function makeRawApp() {
39+
const routes = new Map<string, Handler>();
40+
return {
41+
routes,
42+
get: (p: string, h: Handler) => routes.set(`GET ${p}`, h),
43+
post: (p: string, h: Handler) => routes.set(`POST ${p}`, h),
44+
delete: (p: string, h: Handler) => routes.set(`DELETE ${p}`, h),
45+
};
46+
}
47+
48+
function makeCtx(rawApp: any, services: Record<string, any>) {
49+
const hooks = new Map<string, any>();
50+
return {
51+
ctx: {
52+
hook: (e: string, h: any) => hooks.set(e, h),
53+
getService: (name: string) => {
54+
if (name === 'http-server') return { getRawApp: () => rawApp };
55+
const svc = services[name];
56+
if (svc === undefined) throw new Error(`no ${name}`);
57+
return svc;
58+
},
59+
logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn() },
60+
},
61+
fire: async () => { await hooks.get('kernel:ready')?.(); },
62+
};
63+
}
64+
65+
/** A Hono-ish context. `param` carries the :manifestId route value. */
66+
function makeC(body: any, manifestId?: string) {
67+
const json = vi.fn((payload: any, status?: number) => ({ payload, status: status ?? 200 }));
68+
return {
69+
req: {
70+
url: 'http://localhost:3000/api/v1/marketplace/install-local',
71+
raw: new Request('http://localhost:3000/x'),
72+
json: async () => body,
73+
param: (k: string) => (k === 'manifestId' ? manifestId : undefined),
74+
header: () => undefined,
75+
},
76+
json,
77+
};
78+
}
79+
80+
const SERVICES = () => ({
81+
manifest: { register: vi.fn() },
82+
auth: { api: { getSession: async () => ({ user: { id: 'admin' } }) } },
83+
objectql: { syncSchemas: async () => undefined },
84+
metadata: {},
85+
});
86+
87+
const MANIFEST = {
88+
id: 'app.test.proj',
89+
version: '1.0.0',
90+
objects: [{ name: 'pm_x', fields: { name: { type: 'text' } } }],
91+
data: [{ object: 'pm_x', records: [{ name: 'a' }, { name: 'b' }] }],
92+
};
93+
94+
let dir: string;
95+
beforeEach(() => {
96+
dir = mkdtempSync(join(tmpdir(), 'mil-reseed-'));
97+
seedResult = { summary: { totalInserted: 0, totalUpdated: 0 }, errors: [] };
98+
});
99+
afterEach(() => { rmSync(dir, { recursive: true, force: true }); vi.restoreAllMocks(); });
100+
101+
async function installAndGetRoutes() {
102+
const rawApp = makeRawApp();
103+
const { ctx, fire } = makeCtx(rawApp, SERVICES());
104+
const plugin = new MarketplaceInstallLocalPlugin({ controlPlaneUrl: 'off', storageDir: dir });
105+
await plugin.start(ctx as any);
106+
await fire();
107+
// Install with seed loader reporting nothing — install must still succeed.
108+
const installRes = await rawApp.routes.get('POST /api/v1/marketplace/install-local')!(
109+
makeC({ manifest: MANIFEST }),
110+
);
111+
expect(installRes.payload?.success).toBe(true);
112+
return rawApp;
113+
}
114+
115+
describe('reseed honest result', () => {
116+
it('FAILS (422) when the seed run wrote zero rows but errored', async () => {
117+
const rawApp = await installAndGetRoutes();
118+
seedResult = {
119+
summary: { totalInserted: 0, totalUpdated: 0 },
120+
errors: [{ message: 'database is locked' }, { message: 'database is locked' }],
121+
};
122+
const res = await rawApp.routes.get('POST /api/v1/marketplace/install-local/:manifestId/reseed-sample-data')!(
123+
makeC({}, 'app.test.proj'),
124+
);
125+
expect(res.status).toBe(422);
126+
expect(res.payload?.success).toBe(false);
127+
expect(res.payload?.error?.code).toBe('reseed_no_rows');
128+
// The real failure reason is surfaced, not swallowed.
129+
expect(res.payload?.error?.message).toContain('database is locked');
130+
expect(res.payload?.error?.details).toMatchObject({ inserted: 0, updated: 0, errors: 2 });
131+
});
132+
133+
it('FAILS (422) when the package seeds nothing (0 rows, 0 errors)', async () => {
134+
const rawApp = await installAndGetRoutes();
135+
seedResult = { summary: { totalInserted: 0, totalUpdated: 0 }, errors: [] };
136+
const res = await rawApp.routes.get('POST /api/v1/marketplace/install-local/:manifestId/reseed-sample-data')!(
137+
makeC({}, 'app.test.proj'),
138+
);
139+
expect(res.status).toBe(422);
140+
expect(res.payload?.error?.code).toBe('reseed_no_rows');
141+
});
142+
143+
it('SUCCEEDS and flips withSampleData when rows actually land', async () => {
144+
const rawApp = await installAndGetRoutes();
145+
seedResult = { summary: { totalInserted: 2, totalUpdated: 0 }, errors: [] };
146+
const res = await rawApp.routes.get('POST /api/v1/marketplace/install-local/:manifestId/reseed-sample-data')!(
147+
makeC({}, 'app.test.proj'),
148+
);
149+
expect(res.status).toBe(200);
150+
expect(res.payload?.success).toBe(true);
151+
expect(res.payload?.data).toMatchObject({ inserted: 2, updated: 0, withSampleData: true });
152+
153+
// The ledger now reflects that sample data is present.
154+
const listRes = await rawApp.routes.get('GET /api/v1/marketplace/install-local')!(makeC({}));
155+
const entry = listRes.payload.data.items.find((i: any) => i.manifestId === 'app.test.proj');
156+
expect(entry?.withSampleData).toBe(true);
157+
});
158+
159+
it('partial success (some rows + some errors) still reports the error count', async () => {
160+
const rawApp = await installAndGetRoutes();
161+
seedResult = {
162+
summary: { totalInserted: 1, totalUpdated: 0 },
163+
errors: [{ message: 'one row rejected' }],
164+
};
165+
const res = await rawApp.routes.get('POST /api/v1/marketplace/install-local/:manifestId/reseed-sample-data')!(
166+
makeC({}, 'app.test.proj'),
167+
);
168+
expect(res.status).toBe(200);
169+
expect(res.payload?.success).toBe(true);
170+
expect(res.payload?.data?.errors).toBe(1);
171+
});
172+
});

0 commit comments

Comments
 (0)