diff --git a/internal/application/service/knowledge_clone_image_test.go b/internal/application/service/knowledge_clone_image_test.go index 6c2f9ccc9..a894a530c 100644 --- a/internal/application/service/knowledge_clone_image_test.go +++ b/internal/application/service/knowledge_clone_image_test.go @@ -3,22 +3,24 @@ package service import ( "context" "encoding/json" - "errors" "fmt" "io" "mime/multipart" + "strings" "testing" "github.com/Tencent/WeKnora/internal/types" ) // countingFileService is a minimal FileService stub for cloneChunkImageInfo tests. -// CopyFile records each invocation and returns a deterministic destination path -// derived from (knowledgeID, srcPath) so dedup and rewrite behaviour are verifiable. +// copyOwnedObject copies by GetFile-then-SaveBytes (so extracted images land in +// the servable exports/ namespace), so this stub streams the source path back as +// the file bytes from GetFile and reconstructs a deterministic exports-style +// destination path in SaveBytes, making dedup and rewrite behaviour verifiable. type countingFileService struct { copyCalls int copiedFrom []string - failOnURL string // when non-empty, CopyFile returns an error for this srcPath + failOnURL string // when non-empty, GetFile returns an error for this srcPath deleteCalls int } @@ -28,12 +30,22 @@ func (c *countingFileService) SaveFile(ctx context.Context, file *multipart.File return "", nil } +// SaveBytes records each copy and returns a deterministic exports-namespace path +// derived from the streamed source bytes (which GetFile set to the source path). func (c *countingFileService) SaveBytes(ctx context.Context, data []byte, tenantID uint64, fileName string, temp bool) (string, error) { - return "", nil + src := string(data) + c.copyCalls++ + c.copiedFrom = append(c.copiedFrom, src) + return fmt.Sprintf("local://%d/exports/copy-of-%s", tenantID, src), nil } +// GetFile streams the requested path back as the file content so SaveBytes can +// build a deterministic destination. failOnURL simulates an unreadable source. func (c *countingFileService) GetFile(ctx context.Context, filePath string) (io.ReadCloser, error) { - return nil, errors.New("not implemented") + if c.failOnURL != "" && filePath == c.failOnURL { + return nil, fmt.Errorf("simulated read failure for %s", filePath) + } + return io.NopCloser(strings.NewReader(filePath)), nil } func (c *countingFileService) GetFileURL(ctx context.Context, filePath string) (string, error) { @@ -46,12 +58,7 @@ func (c *countingFileService) DeleteFile(ctx context.Context, filePath string) e } func (c *countingFileService) CopyFile(ctx context.Context, srcPath string, tenantID uint64, knowledgeID string) (string, error) { - if c.failOnURL != "" && srcPath == c.failOnURL { - return "", fmt.Errorf("simulated copy failure for %s", srcPath) - } - c.copyCalls++ - c.copiedFrom = append(c.copiedFrom, srcPath) - return fmt.Sprintf("local://%d/%s/copy-of-%s", tenantID, knowledgeID, srcPath), nil + return "", fmt.Errorf("CopyFile must not be used for embedded image copies") } func mustImageInfoJSON(t *testing.T, imgs []types.ImageInfo) string { @@ -93,7 +100,7 @@ func TestCloneChunkImageInfo_RewritesURLAndMatchedOriginal(t *testing.T) { if err := json.Unmarshal([]byte(out), &got); err != nil { t.Fatalf("unmarshal out: %v", err) } - want := "local://7/k-dst/copy-of-local://1/k0/a.png" + want := "local://7/exports/copy-of-local://1/k0/a.png" if got[0].URL != want { t.Errorf("URL not rewritten: got %q want %q", got[0].URL, want) } @@ -125,7 +132,7 @@ func TestRewriteContentImageURLs_ParentTextChunk(t *testing.T) { // Parent text chunk has NO image_info but embeds the markdown reference. parentContent := "See ![diagram](local://1/k0/a.png) here." got := rewriteContentImageURLs(parentContent, urlCache) - want := "See ![diagram](local://7/k-dst/copy-of-local://1/k0/a.png) here." + want := "See ![diagram](local://7/exports/copy-of-local://1/k0/a.png) here." if got != want { t.Errorf("parent content image URL not rewritten:\n got %q\nwant %q", got, want) } diff --git a/internal/application/service/knowledge_clone_move.go b/internal/application/service/knowledge_clone_move.go index 594dbe0c5..d4d7c640e 100644 --- a/internal/application/service/knowledge_clone_move.go +++ b/internal/application/service/knowledge_clone_move.go @@ -5,6 +5,9 @@ import ( "encoding/json" "errors" "fmt" + "io" + "net/http" + "path/filepath" "slices" "strings" "time" @@ -21,13 +24,18 @@ import ( "golang.org/x/sync/errgroup" ) -// copyOwnedObject performs a real copy of srcPath into a NEW object owned by -// (tenantID, knowledgeID) using the destination FileService, returning the new -// provider:// path. The same-backend check lives inside dstSvc.CopyFile, which -// returns file.ErrCrossBackendCopy when srcPath belongs to a different provider; -// that error is propagated unchanged so callers can fail the clone explicitly. -// srcSvc is accepted for symmetry with the read side but is not used directly: -// server-side copies are issued by the destination service. +// copyOwnedObject copies srcPath into a NEW object owned by the destination +// tenant, returning the new provider:// (resource) path. +// +// Extracted/embedded chunk images MUST land in the tenant's exports/ namespace, +// because GET /knowledge-bases/:id/files only serves objects that pass +// ValidateKBScopedStoragePath (i.e. {tenant}/exports/...). CopyFile writes to +// the knowledge-scoped upload layout ({tenant}/{knowledgeID}/...) used for raw +// source files, which the KB proxy rejects — so a clone that used CopyFile +// produced images that could no longer be rendered. Instead, read the source +// bytes and re-save them via SaveBytes, exactly mirroring how the original +// images were persisted during ingestion (see image_resolver.saveReferencedImage), +// so the copy is a genuine independent object in the servable namespace. func copyOwnedObject( ctx context.Context, srcSvc, dstSvc interfaces.FileService, @@ -35,8 +43,55 @@ func copyOwnedObject( tenantID uint64, knowledgeID string, ) (string, error) { - _ = srcSvc // reserved for future cross-backend streaming fallback - return dstSvc.CopyFile(ctx, srcPath, tenantID, knowledgeID) + _ = knowledgeID // exports objects are tenant-scoped, not knowledge-scoped + rc, err := srcSvc.GetFile(ctx, srcPath) + if err != nil { + return "", fmt.Errorf("read source image %q: %w", srcPath, err) + } + defer rc.Close() + data, err := io.ReadAll(rc) + if err != nil { + return "", fmt.Errorf("buffer source image %q: %w", srcPath, err) + } + + fileName := uuid.New().String() + imageExtForCopy(srcPath, data) + newPath, err := dstSvc.SaveBytes(ctx, data, tenantID, fileName, false) + if err != nil { + return "", fmt.Errorf("save copied image for %q: %w", srcPath, err) + } + return newPath, nil +} + +// imageExtForCopy resolves the file extension to use for a copied image. It +// prefers an image extension already present on the source path, then falls +// back to sniffing the content bytes, and finally defaults to ".png" (matching +// image_resolver's default) so the object is always served with a sane type. +func imageExtForCopy(srcPath string, data []byte) string { + if ext := strings.ToLower(filepath.Ext(srcPath)); isImageExt(ext) { + return ext + } + switch http.DetectContentType(data) { + case "image/png": + return ".png" + case "image/jpeg": + return ".jpg" + case "image/gif": + return ".gif" + case "image/webp": + return ".webp" + case "image/bmp": + return ".bmp" + } + return ".png" +} + +func isImageExt(ext string) bool { + switch ext { + case ".png", ".jpg", ".jpeg", ".gif", ".webp", ".bmp", ".svg": + return true + default: + return false + } } // cloneChunkImageInfo parses a chunk's image_info JSON, copies every referenced