mirror of
https://github.com/langgenius/dify.git
synced 2026-09-08 11:04:27 +08:00
fix(knowledge-fs): restore metadata field writes
This commit is contained in:
parent
7e55a51587
commit
a60865a8b3
@ -1,6 +1,6 @@
|
||||
{
|
||||
"schemaVersion": 5,
|
||||
"subtreeTree": "af4a3d87a3ac4dcfd91a0ea6247de6609add0aae",
|
||||
"subtreeTree": "222470dcafc7a985cfb78896c31180f21cf88d75",
|
||||
"openapiSha256": "3a712231fa850c4f5151bc283205da9062086a0b693f0d1ab01c2c526323f018",
|
||||
"capabilityV2AuthManifestSha256": "fc0a47e23cce12544882f0298522b4933002e892b84ce1815df7e81d36a7a0c7",
|
||||
"capabilityV2AuthTestVectorSha256": "ae0de37b1ff05c40f905cf17a7b410d8971acacf64db07d5ee3d6fecfa559ce3",
|
||||
|
||||
@ -0,0 +1,42 @@
|
||||
# Metadata write deletion admission
|
||||
|
||||
Date: 2026-08-17
|
||||
|
||||
## What changed
|
||||
|
||||
- Metadata field creation, rename, and deletion now use the shared knowledge-space deletion
|
||||
admission lock instead of querying a non-existent `knowledge_spaces.state` column.
|
||||
- The shared admission verifies the current `lifecycle_state`, the space's `deletion_job_id`, and
|
||||
the durable `deletion_jobs.active_slot` before any metadata mutation runs in the same transaction.
|
||||
- Added PostgreSQL and TiDB regressions for the canonical admission SQL and for rejecting a write
|
||||
while an active durable deletion job exists.
|
||||
|
||||
## Why
|
||||
|
||||
Creating a metadata field failed in the test environment with PostgreSQL error `42703` because the
|
||||
repository queried `knowledge_spaces.state`. The current schema stores the lifecycle in
|
||||
`lifecycle_state`; it has no `state` column. The database error escaped the domain layer and was
|
||||
reported to the Console as `KNOWLEDGE_FS_INTERNAL_ERROR` with HTTP 503.
|
||||
|
||||
Using the shared deletion admission fixes the schema mismatch and preserves the intended race-safe
|
||||
write fence. A direct column substitution would still miss deletion jobs that have acquired their
|
||||
durable active slot.
|
||||
|
||||
## Verification
|
||||
|
||||
- The new PostgreSQL and TiDB tests were first run against the previous implementation and failed
|
||||
on the `state` SQL and missing deletion-job admission.
|
||||
- `pnpm --dir knowledge-fs --filter @knowledge/api exec vitest run
|
||||
src/knowledge-space-metadata-repository.test.ts` passed all 16 tests.
|
||||
- The complete `@knowledge/api` suite passed: 410 files passed, 1 skipped; 4,494 tests passed, 3
|
||||
skipped.
|
||||
- `pnpm --dir knowledge-fs --filter @knowledge/api typecheck` passed.
|
||||
- `pnpm --dir knowledge-fs build` passed all 12 workspace builds.
|
||||
- Focused Biome checks for both changed TypeScript files and `git diff --check` passed.
|
||||
|
||||
## Risks and follow-up
|
||||
|
||||
- A knowledge space with an active deletion job continues to return the existing metadata
|
||||
not-found domain result; this change does not alter the public error contract.
|
||||
- Reads and document metadata reconciliation are unchanged. Only metadata field catalog mutations
|
||||
acquire the canonical deletion fence.
|
||||
@ -8,6 +8,7 @@ import type {
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import {
|
||||
KnowledgeSpaceMetadataNotFoundError,
|
||||
KnowledgeSpaceMetadataValidationError,
|
||||
createDatabaseKnowledgeSpaceMetadataRepository,
|
||||
} from "./knowledge-space-metadata-repository";
|
||||
@ -21,6 +22,105 @@ const now = "2026-08-10T12:00:00.000Z";
|
||||
describe.each(["postgres", "tidb"] as const)(
|
||||
"database knowledge-space metadata repository (%s)",
|
||||
(dialect) => {
|
||||
it("admits metadata writes through the canonical knowledge-space deletion gate", async () => {
|
||||
const calls: DatabaseExecuteInput[] = [];
|
||||
let inserted = false;
|
||||
const database = testDatabase(dialect, async (input) => {
|
||||
calls.push(input);
|
||||
if (input.tableName === "knowledge_spaces") {
|
||||
return {
|
||||
rows: [
|
||||
{
|
||||
deletion_job_id: null,
|
||||
id: knowledgeSpaceId,
|
||||
lifecycle_state: "active",
|
||||
},
|
||||
],
|
||||
rowsAffected: 1,
|
||||
};
|
||||
}
|
||||
if (input.tableName === "deletion_jobs") {
|
||||
return { rows: [], rowsAffected: 0 };
|
||||
}
|
||||
if (input.operation === "insert") {
|
||||
inserted = true;
|
||||
return { rows: [], rowsAffected: 1 };
|
||||
}
|
||||
if (input.sql.includes("COUNT(*)")) {
|
||||
return { rows: [{ field_count: 0 }], rowsAffected: 1 };
|
||||
}
|
||||
if (input.sql.includes("name") && input.sql.includes("LIMIT 1")) {
|
||||
return { rows: [], rowsAffected: 0 };
|
||||
}
|
||||
return {
|
||||
rows: inserted ? [fieldRow({ binding_count: 0 })] : [],
|
||||
rowsAffected: inserted ? 1 : 0,
|
||||
};
|
||||
});
|
||||
const repository = createDatabaseKnowledgeSpaceMetadataRepository({
|
||||
database,
|
||||
generateFieldId: () => fieldId,
|
||||
maxListLimit: 100,
|
||||
});
|
||||
|
||||
await expect(
|
||||
repository.create({
|
||||
knowledgeSpaceId,
|
||||
name: "priority",
|
||||
now,
|
||||
subjectId: "account:1",
|
||||
tenantId,
|
||||
type: "string",
|
||||
}),
|
||||
).resolves.toMatchObject({ id: fieldId });
|
||||
|
||||
const spaceAdmission = calls.find((call) => call.tableName === "knowledge_spaces");
|
||||
expect(spaceAdmission?.sql).toContain("lifecycle_state");
|
||||
expect(spaceAdmission?.sql).toContain("deletion_job_id");
|
||||
expect(spaceAdmission?.sql).not.toMatch(/["`]state["`]/u);
|
||||
expect(calls.some((call) => call.tableName === "deletion_jobs")).toBe(true);
|
||||
});
|
||||
|
||||
it("rejects metadata writes while a durable deletion job is active", async () => {
|
||||
const calls: DatabaseExecuteInput[] = [];
|
||||
const database = testDatabase(dialect, async (input) => {
|
||||
calls.push(input);
|
||||
if (input.tableName === "knowledge_spaces") {
|
||||
return {
|
||||
rows: [
|
||||
{
|
||||
deletion_job_id: null,
|
||||
id: knowledgeSpaceId,
|
||||
lifecycle_state: "active",
|
||||
},
|
||||
],
|
||||
rowsAffected: 1,
|
||||
};
|
||||
}
|
||||
if (input.tableName === "deletion_jobs") {
|
||||
return { rows: [{ id: "active-deletion-job" }], rowsAffected: 1 };
|
||||
}
|
||||
return { rows: [], rowsAffected: 0 };
|
||||
});
|
||||
const repository = createDatabaseKnowledgeSpaceMetadataRepository({
|
||||
database,
|
||||
generateFieldId: () => fieldId,
|
||||
maxListLimit: 100,
|
||||
});
|
||||
|
||||
await expect(
|
||||
repository.create({
|
||||
knowledgeSpaceId,
|
||||
name: "priority",
|
||||
now,
|
||||
subjectId: "account:1",
|
||||
tenantId,
|
||||
type: "string",
|
||||
}),
|
||||
).rejects.toBeInstanceOf(KnowledgeSpaceMetadataNotFoundError);
|
||||
expect(calls.some((call) => call.operation === "insert")).toBe(false);
|
||||
});
|
||||
|
||||
it("lists a bounded field catalog with binding counts and tenant-space keyset scope", async () => {
|
||||
let select: DatabaseExecuteInput | undefined;
|
||||
const database = testDatabase(dialect, async (input) => {
|
||||
@ -60,7 +160,10 @@ describe.each(["postgres", "tidb"] as const)(
|
||||
const database = testDatabase(dialect, async (input) => {
|
||||
calls.push(input);
|
||||
if (input.tableName === "knowledge_spaces") {
|
||||
return { rows: [{ id: knowledgeSpaceId }], rowsAffected: 1 };
|
||||
return { rows: [activeSpaceRow()], rowsAffected: 1 };
|
||||
}
|
||||
if (input.tableName === "deletion_jobs") {
|
||||
return { rows: [], rowsAffected: 0 };
|
||||
}
|
||||
if (input.operation === "insert") {
|
||||
inserted = true;
|
||||
@ -200,7 +303,10 @@ describe.each(["postgres", "tidb"] as const)(
|
||||
const database = testDatabase(dialect, async (input) => {
|
||||
calls.push(input);
|
||||
if (input.tableName === "knowledge_spaces") {
|
||||
return { rows: [{ id: knowledgeSpaceId }], rowsAffected: 1 };
|
||||
return { rows: [activeSpaceRow()], rowsAffected: 1 };
|
||||
}
|
||||
if (input.tableName === "deletion_jobs") {
|
||||
return { rows: [], rowsAffected: 0 };
|
||||
}
|
||||
if (input.tableName === "knowledge_space_metadata_fields" && input.operation === "select") {
|
||||
if (input.sql.includes("LIMIT 1")) return { rows: [], rowsAffected: 0 };
|
||||
@ -289,6 +395,14 @@ function fieldRow(overrides: DatabaseRow = {}): DatabaseRow {
|
||||
};
|
||||
}
|
||||
|
||||
function activeSpaceRow(): DatabaseRow {
|
||||
return {
|
||||
deletion_job_id: null,
|
||||
id: knowledgeSpaceId,
|
||||
lifecycle_state: "active",
|
||||
};
|
||||
}
|
||||
|
||||
function testDatabase(
|
||||
dialect: DatabaseAdapter["dialect"],
|
||||
execute: (input: DatabaseExecuteInput) => Promise<DatabaseExecuteResult>,
|
||||
|
||||
@ -9,6 +9,7 @@ import type {
|
||||
|
||||
import { nonnegativeSafeIntegerColumn, stringColumn } from "./database-row-utils";
|
||||
import { databasePlaceholder, quoteDatabaseIdentifier } from "./database-sql-utils";
|
||||
import { lockKnowledgeSpaceForDeletionAdmission } from "./knowledge-space-deletion-admission";
|
||||
|
||||
export type KnowledgeSpaceMetadataFieldType = "number" | "string" | "time";
|
||||
|
||||
@ -443,14 +444,9 @@ async function requireWritableSpace(
|
||||
executor: DatabaseExecutor,
|
||||
input: MetadataScope,
|
||||
): Promise<void> {
|
||||
const result = await executor.execute({
|
||||
maxRows: 1,
|
||||
operation: "select",
|
||||
params: [input.tenantId, input.knowledgeSpaceId],
|
||||
sql: `SELECT ${q(database, "id")} FROM ${q(database, "knowledge_spaces")} WHERE ${q(database, "tenant_id")} = ${p(database, 1)} AND ${q(database, "id")} = ${p(database, 2)} AND ${q(database, "state")} <> 'deleting' FOR UPDATE;`,
|
||||
tableName: "knowledge_spaces",
|
||||
});
|
||||
if (!result.rows[0]) throw new KnowledgeSpaceMetadataNotFoundError("Knowledge space not found");
|
||||
if (!(await lockKnowledgeSpaceForDeletionAdmission(database, executor, input))) {
|
||||
throw new KnowledgeSpaceMetadataNotFoundError("Knowledge space not found");
|
||||
}
|
||||
}
|
||||
|
||||
function mapField(row: DatabaseRow): KnowledgeSpaceMetadataField {
|
||||
|
||||
Loading…
Reference in New Issue
Block a user