mirror of
https://github.com/Tencent/WeKnora.git
synced 2026-09-19 02:18:25 +08:00
ochan.kwon
7478e3cddb
Extracts the 25 repeated NewCompositeRetrieveEngine call sites across seven services into two factory functions with tenant-ownership verification: CreateRetrieveEngineForKB (synchronous) and CreateRetrieveEngineFromPayload (async task handlers). Promotes GetByStoreID from concrete-only to interfaces.RetrieveEngineRegistry. Extends KBDeletePayload / IndexDeletePayload with an omitempty VectorStoreID snapshot. Design highlights - Factory verifies tenant ownership of the resolved store (defense-in-depth) so a gap in PR3 validation or a tampered Asynq payload cannot cross tenants. Cross-tenant attempts return ErrVectorStoreForbidden with a structured log entry. - Sentinel errors (ErrTenantInfoMissing, ErrVectorStoreNotFound, ErrVectorStoreForbidden) let async handlers classify non-retryable failures as asynq.SkipRetry. - nil and empty-string pointers for vectorStoreID normalize to "unbound" so callers never send an empty UUID to GetByStoreID. - resolveBoundEngine constructs engineInfos directly (with slices.Clone of Support()) instead of going through NewCompositeRetrieveEngine. The latter only reads from the byEngineType map and cannot reach DB stores in byStoreID -- multiple stores can share the same engine type. Unbound fallbacks still reuse NewCompositeRetrieveEngine so the tenant effective-engines path is byte-for-byte unchanged. - DeleteKnowledgeBase now reads the KB before soft-delete so the enqueued KBDeletePayload carries a VectorStoreID snapshot; once the row is soft-deleted GORM's default scope hides it. Cost: +1 SELECT per KB-delete request (a rare admin operation). - enqueueIndexDeleteTask gains a vectorStoreID *string parameter that the single caller (tag.go) populates from the owning KB. The worker (ProcessIndexDelete) validates ownership via the factory. - cleanupKnowledgeResources loads the KB so bound-store cleanup routes correctly. If the load fails it warns and falls back to tenant effective engines; orphan vectors become observable in logs instead of disappearing silently. - DeleteKnowledges (batch) stays on the tenant effective-engines path -- batches can span stores and multi-store fan-out is PR4 scope. Noted inline. Merge safety All knowledge bases have VectorStoreID = NULL at merge time (introduced nullable in PR1, Tencent/WeKnora#994). Every factory call falls back to NewCompositeRetrieveEngine(registry, tenantInfo.GetEffectiveEngines()), which is the existing code path for these KBs. Async payloads from before this change decode to VectorStoreID = nil (omitempty) and take the same fallback. Testing - go build ./... - go vet ./... - go test -race -count=1 ./internal/application/service/retriever/... covers unbound (nil/empty), bound, cross-tenant, unregistered store, ownership infra error, legacy payload shapes (missing field/null/ empty string/nil), tampered payload, and parallel invocation. - grep -r NewCompositeRetrieveEngine internal/application/service returns zero results outside factory.go/composite.go. Refs: Part of Tencent/WeKnora#993. Depends on Tencent/WeKnora#994.