feat[Go]: implement POST /api/v1/files/link-to-datasets#15674
Open
hunnyboy1217 wants to merge 2 commits into
Open
feat[Go]: implement POST /api/v1/files/link-to-datasets#15674hunnyboy1217 wants to merge 2 commits into
hunnyboy1217 wants to merge 2 commits into
Conversation
Ports Python file2document_api.py convert() endpoint to Go, closing issue infiniflow#15673. POST /api/v1/files/link-to-datasets (FileHandler.LinkToDatasets) New files: internal/service/file2document.go – File2DocumentService with: LinkToDatasets – validates files/KBs, expands folders, checks team permissions, schedules _convertFiles goroutine convertFiles – deletes existing docs+mappings per file, then creates fresh documents in each target KB and new mappings getAllInnermostFileIDs – recursive folder expansion (mirrors Python FileService.get_all_innermost_file_ids) checkFileTeamPermission / checkKBTeamPermission – mirrors Python check_file_team_permission / check_kb_team_permission getParser – maps (file type, extension, kb.parser_id) → parser ID Changed files: internal/dao/file2document.go – added Create method internal/handler/file.go – FileHandler gains file2DocumentService; LinkToDatasets HTTP handler added internal/router/router.go – route registered in /api/v1/files group Behaviour matches Python exactly: - file_ids and kb_ids are required; missing either → error - All files validated before any work begins - All KBs validated before any work begins - Folder IDs expanded to leaf-file IDs recursively - File and KB team permissions checked for every expanded file - Goroutine (fire-and-forget) for the blocking DB work matches Python's loop.run_in_executor pattern - Returns true immediately without waiting for conversion
dosubot
Bot
added
the
size:L
Contributor
📝 WalkthroughWalkthroughAdds File2DocumentService to link files/folders to knowledge bases, expand folders, enforce tenant/team permissions, and asynchronously convert and relink documents; exposes POST /api/v1/files/link-to-datasets, adds File2Document DAO create, and documents insertion/removal operations. ChangesFile-to-Dataset Linking Workflow
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/service/file2document.go`:
- Around line 145-223: The convertFiles flow deletes existing document mappings
and documents then recreates documents per-kb without transactional guarantees,
causing inconsistent state if any step fails; modify convertFiles to use a
per-file transactional approach (or an atomic two-phase approach): for each
fileID wrap operations in a DB transaction (or create new documents and
file2document mappings first using documentDAO.Create and
file2DocumentDAO.Create, verify all creates succeeded, then call
documentDAO.Delete and file2DocumentDAO.DeleteByFileID or replace old mappings),
and ensure error handling rolls back the transaction or aborts cleanup on
failure; locate and update the logic around file2DocumentDAO.DeleteByFileID,
documentDAO.Delete, documentDAO.Create, and file2DocumentDAO.Create to implement
the transactional/atomic reorder and proper rollback.
- Around line 94-106: The loop that expands folder IDs (using filesSet,
FileTypeFolder and s.getAllInnermostFileIDs) can produce duplicate leaf IDs when
requests mix folders and direct file IDs; change the collection logic to
deduplicate by using a set (map[string]struct{}) named e.g. seen to track added
IDs and only append to allFileIDs the first time an ID is encountered (for both
inner results from getAllInnermostFileIDs and direct ids), ensuring each leaf
file is scheduled for conversion exactly once.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b79c150d-0fd8-492c-921a-620b2dd65e92
📒 Files selected for processing (4)
internal/dao/file2document.gointernal/handler/file.gointernal/router/router.gointernal/service/file2document.go
Comment on lines
+145
to
+223
| mappings, err := s.file2DocumentDAO.GetByFileID(fileID) | ||
| if err != nil { | ||
| common.Warn("convertFiles: GetByFileID failed", zap.String("fileID", fileID), zap.Error(err)) | ||
| } | ||
| for _, m := range mappings { | ||
| if m.DocumentID == nil { | ||
| continue | ||
| } | ||
| doc, err := s.documentDAO.GetByID(*m.DocumentID) | ||
| if err != nil || doc == nil { | ||
| continue | ||
| } | ||
| // Get tenant from KB. | ||
| kb, err := s.kbDAO.GetByID(doc.KbID) | ||
| if err != nil || kb == nil { | ||
| continue | ||
| } | ||
| // Hard-delete document (ignoring chunk store cleanup for simplicity). | ||
| if _, err := s.documentDAO.Delete(*m.DocumentID); err != nil { | ||
| common.Warn("convertFiles: Delete document failed", | ||
| zap.String("docID", *m.DocumentID), zap.Error(err)) | ||
| } | ||
| } | ||
| if err := s.file2DocumentDAO.DeleteByFileID(fileID); err != nil { | ||
| common.Warn("convertFiles: DeleteByFileID failed", zap.String("fileID", fileID), zap.Error(err)) | ||
| } | ||
|
|
||
| // Get source file. | ||
| file, err := s.fileDAO.GetByID(fileID) | ||
| if err != nil || file == nil { | ||
| continue | ||
| } | ||
|
|
||
| // Create document + mapping in each target KB. | ||
| for _, kbID := range kbIDs { | ||
| kb, ok := func() (*entity.Knowledgebase, bool) { | ||
| kb, err := s.kbDAO.GetByID(kbID) | ||
| return kb, err == nil && kb != nil | ||
| }() | ||
| if !ok { | ||
| continue | ||
| } | ||
|
|
||
| parserID := getParser(file.Type, file.Name, kb.ParserID) | ||
| suffix := strings.TrimPrefix(filepath.Ext(file.Name), ".") | ||
| doc := &entity.Document{ | ||
| ID: common.GenerateUUID(), | ||
| KbID: kb.ID, | ||
| ParserID: parserID, | ||
| ParserConfig: entity.JSONMap(kb.ParserConfig), | ||
| CreatedBy: userID, | ||
| Type: file.Type, | ||
| Name: &file.Name, | ||
| Suffix: suffix, | ||
| Size: file.Size, | ||
| } | ||
| if file.Location != nil { | ||
| doc.Location = file.Location | ||
| } | ||
| if kb.PipelineID != nil { | ||
| doc.PipelineID = kb.PipelineID | ||
| } | ||
|
|
||
| if err := s.documentDAO.Create(doc); err != nil { | ||
| common.Warn("convertFiles: Create document failed", | ||
| zap.String("kbID", kbID), zap.String("fileID", fileID), zap.Error(err)) | ||
| continue | ||
| } | ||
|
|
||
| mapping := &entity.File2Document{ | ||
| ID: common.GenerateUUID(), | ||
| FileID: &fileID, | ||
| DocumentID: &doc.ID, | ||
| } | ||
| if err := s.file2DocumentDAO.Create(mapping); err != nil { | ||
| common.Warn("convertFiles: Create file2document mapping failed", | ||
| zap.String("fileID", fileID), zap.String("docID", doc.ID), zap.Error(err)) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Current delete-then-recreate flow can leave inconsistent file/document state.
convertFiles performs destructive deletes with warn-and-continue behavior and no per-file transaction. If any intermediate step fails, you can end up with removed mappings and missing recreated links (or orphaned docs).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/service/file2document.go` around lines 145 - 223, The convertFiles
flow deletes existing document mappings and documents then recreates documents
per-kb without transactional guarantees, causing inconsistent state if any step
fails; modify convertFiles to use a per-file transactional approach (or an
atomic two-phase approach): for each fileID wrap operations in a DB transaction
(or create new documents and file2document mappings first using
documentDAO.Create and file2DocumentDAO.Create, verify all creates succeeded,
then call documentDAO.Delete and file2DocumentDAO.DeleteByFileID or replace old
mappings), and ensure error handling rolls back the transaction or aborts
cleanup on failure; locate and update the logic around
file2DocumentDAO.DeleteByFileID, documentDAO.Delete, documentDAO.Create, and
file2DocumentDAO.Create to implement the transactional/atomic reorder and proper
rollback.
Hz-186
suggested changes
Jun 5, 2026
Contributor
Hz-186
left a comment
Hz-186
left a comment
There was a problem hiding this comment.
The Go background conversion directly deletes and creates document rows through DAO calls. Python uses DocumentService.remove_document() and DocumentService.insert(), which update dataset/document counters and clean up related tasks, chunks, metadata, thumbnails, and graph/ index state.
In my local test, Go created the new document and file2document mapping, but the target KB doc_num stayed at 0, confirming the counter update path is skipped.
There are also response-shape mismatches for errors. Missing file_ids returns Python code=101, data=null with the validate_request message, while Go returns code=102, data=false and exposes the Gin binding error. 401 and file-not-found responses also omit/change data.
Please route document removal/insertion through equivalent service logic and align the error response shapes with Python.
…w#15674) Applies the final-review feedback from Hz-186 and CodeRabbit. Hz-186 — counters not updated / error shapes: - convertFiles now routes document removal/insertion through DocumentService instead of raw DAO calls, so KB doc_num/chunk_num/token_num counters are updated (target KB doc_num previously stayed at 0): * new DocumentService.RemoveDocumentKeepFile — mirrors Python remove_document: deletes tasks, doc-store chunks/metadata, and the document row with counter decrement, WITHOUT touching the file record or its mappings (caller clears mappings via delete_by_file_id) * new DocumentService.InsertDocument — mirrors Python insert: creates the document row and increments KB doc_num in a single transaction - error response shapes aligned with Python: * missing file_ids/kb_ids now returns code=101 (ARGUMENT_ERROR), data=null with the validate_request-style message "required argument are missing: <fields>; " (was code=102, data=false) * all error paths use jsonError → data=null (was data=false); file/dataset not-found and no-authorization map to code=102 like Python's get_data_error_result in convert() CodeRabbit — robustness: - no longer propagate raw DAO/runtime errors through the API: the service returns sentinel errors (ErrLinkFileNotFound / ErrLinkDatasetNotFound / ErrLinkNoAuthorization / ErrLinkInternal); internal failures surface a generic "Internal server error." message instead of leaking DB details - deduplicate expanded file IDs after folder expansion so a leaf file reached via both a folder and a direct ID (or overlapping folders) is converted once - InsertDocument wraps the document row + counter bump in one transaction so a failed insert cannot leave a stale counter No route or request/response field changes beyond the corrected error codes.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
internal/service/file2document.go (1)
221-235:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRollback the new document when mapping creation fails.
InsertDocumentcommits beforefile2DocumentDAO.Create(mapping). If the mapping insert fails, this leaves a live document in the KB withdoc_numalready incremented but nofile2documentrow, so future relinks cannot find and clean it up.💡 Proposed fix
if err := s.file2DocumentDAO.Create(mapping); err != nil { common.Warn("convertFiles: Create file2document mapping failed", zap.String("fileID", fileID), zap.String("docID", doc.ID), zap.Error(err)) + if delErr := s.documentSvc.DeleteDocument(doc.ID); delErr != nil { + common.Warn("convertFiles: rollback orphan document failed", + zap.String("docID", doc.ID), zap.Error(delErr)) + } + continue }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/service/file2document.go` around lines 221 - 235, When file2document mapping creation fails after a successful s.documentSvc.InsertDocument(doc), delete or rollback the newly created document to avoid orphaned docs: after s.file2DocumentDAO.Create(mapping) returns an error, call the document removal API (e.g., s.documentSvc.DeleteDocument or the appropriate delete/remove method) with doc.ID, log both the mapping create error and any delete error, and ensure the code continues (or retries) only after attempting the cleanup; update the existing warn log block to perform this delete using the same doc.ID and keep the mapping creation and ID generation (entity.File2Document, common.GenerateUUID) as-is.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/service/document.go`:
- Around line 288-296: In DocumentService.InsertDocument's transaction block,
guard the KB counter update by capturing the update result instead of only
checking Error: call tx.Model(&entity.Knowledgebase{}).Where("id = ?",
doc.KbID).Update("doc_num", gorm.Expr("doc_num + 1")) into a result variable,
return an error and rollback if result.Error != nil, and also return an error
when result.RowsAffected == 0 (indicating no KB row was updated) so the
transaction fails when the KB is missing (follow the same pattern used in
deleteDocRecordWithCounters); reference the tx variable, doc.KbID, and the
Update("doc_num", gorm.Expr("doc_num + 1")) call when making the change.
In `@internal/service/file2document.go`:
- Around line 143-145: Deduplicate the kb IDs before launching the background
goroutine: build a unique slice from the validated kbMap keys (instead of using
raw req.KbIDs) and pass that deduplicated slice into the goroutine that runs the
conversion; update the usage of kbIDs in the anonymous func launched in
file2document.go so it iterates over the unique KB IDs (derived from kbMap) to
avoid creating duplicate documents/mappings.
---
Duplicate comments:
In `@internal/service/file2document.go`:
- Around line 221-235: When file2document mapping creation fails after a
successful s.documentSvc.InsertDocument(doc), delete or rollback the newly
created document to avoid orphaned docs: after
s.file2DocumentDAO.Create(mapping) returns an error, call the document removal
API (e.g., s.documentSvc.DeleteDocument or the appropriate delete/remove method)
with doc.ID, log both the mapping create error and any delete error, and ensure
the code continues (or retries) only after attempting the cleanup; update the
existing warn log block to perform this delete using the same doc.ID and keep
the mapping creation and ID generation (entity.File2Document,
common.GenerateUUID) as-is.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 986c869b-e234-4eb4-95e0-9b61e287a1ad
📒 Files selected for processing (3)
internal/handler/file.gointernal/service/document.gointernal/service/file2document.go
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/handler/file.go
Comment on lines
+288
to
+296
| return dao.DB.Transaction(func(tx *gorm.DB) error { | ||
| if err := tx.Create(doc).Error; err != nil { | ||
| return fmt.Errorf("failed to create document: %w", err) | ||
| } | ||
| if err := tx.Model(&entity.Knowledgebase{}). | ||
| Where("id = ?", doc.KbID). | ||
| Update("doc_num", gorm.Expr("doc_num + 1")).Error; err != nil { | ||
| return fmt.Errorf("failed to increment doc_num for KB %s: %w", doc.KbID, err) | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Document / Knowledgebase model definitions =="
rg -nC2 'type Document struct|type Knowledgebase struct|KbID|foreignKey|constraint' --type=go
echo
echo "== Migrations / schema references for documents.kb_id =="
rg -nC2 'documents|knowledgebase|kb_id|FOREIGN KEY|foreign key|constraint' --type=go --type=sqlRepository: infiniflow/ragflow
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== 1) Inspect Document entity KbID gorm tags =="
sed -n '1,120p' internal/entity/document.go | cat -n
echo
echo "== 2) Inspect knowledgebase entity (for delete behavior / tags) =="
sed -n '1,140p' internal/entity/kb.go | cat -n
echo
echo "== 3) Inspect the function doing tx.Create + KB doc_num increment =="
sed -n '240,340p' internal/service/document.go | cat -n
echo
echo "== 4) Inspect the test that mentions inserting a doc with a missing KB row =="
rg -n "orphan-doc|no-such-kb|kb_id that has no KB" internal/service/document_test.go
# Print around the referenced region without dumping whole file
start=680
end=780
sed -n "${start},${end}p" internal/service/document_test.go | cat -n
echo
echo "== 5) Search repo for actual FK constraints referencing kb_id -> knowledgebase/id =="
rg -n --hidden --glob '*.{sql,psql,postgres,mysql,sqlite,duckdb,migration,ddl}' "FOREIGN KEY" .
rg -n --hidden --glob '*.sql' "kb_id[^\\n]*REFERENCES|REFERENCES[^\\n]*kb_id|kb_id[^\\n]*references|references[^\\n]*kb_id" .
rg -n --hidden --glob '*.go' "foreignKey|constraint:|Constraints|OnDelete|OnUpdate|references|REFERENCES" internal/entity internal/daoRepository: infiniflow/ragflow
Length of output: 21725
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== 1) Find setupServiceTestDB definition (DB type + FK enforcement) =="
rg -n "func setupServiceTestDB|setupServiceTestDB\\(" internal/service/document_test.go internal/service -S
rg -n "PRAGMA foreign_keys|foreign_keys=on|ForeignKey|constraint" internal/service -S
echo
echo "== 2) Inspect the test DB setup file(s) =="
# Prefer the file that contains setupServiceTestDB
file="$(rg -n "func setupServiceTestDB" -S internal/service | head -n1 | awk -F: '{print $1}')"
if [ -n "${file:-}" ]; then
echo "Using: $file"
sed -n '1,220p' "$file" | cat -n
fi
echo
echo "== 3) Search for migrations/DDL mentioning document.kb_id foreign keys (kb_id -> knowledgebase.id) =="
rg -n --hidden --glob '*.{sql,psql,postgres,mysql,sqlite,migration,ddl}' "document.*kb_id|documents.*kb_id|kb_id.*REFERENCES|REFERENCES.*knowledgebase|knowledgebase.*id" .
rg -n --hidden --glob '*.go' "AutoMigrate|Migrate|schema|foreignKey|constraint|OnDelete|OnUpdate" internal -SRepository: infiniflow/ragflow
Length of output: 13752
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== A) Check if any SQL/DDL/migrations exist in repo =="
fd -t f -i -d 6 -e sql -e ddl -e psql -e postgres -e sqlite -e mysql -e migration -e go -a --max-results 2000 . > /tmp/ddl_files.txt || true
wc -l /tmp/ddl_files.txt || true
head -n 50 /tmp/ddl_files.txt || true
echo
echo "== B) Search repo for FK definitions involving kb_id -> knowledgebase/id =="
rg -n --hidden --glob '*.{sql,ddl,migration,psql,postgres,mysql,sqlite}' "kb_id.*REFERENCES|REFERENCES.*kb_id|document.*kb_id|documents.*kb_id|knowledgebase\\.id" . || true
echo
echo "== C) Inspect deleteDocRecordWithCounters for RowsAffected handling =="
rg -n "deleteDocRecordWithCounters\\(" internal/service/document.go
sed -n '90,170p' internal/service/document.go | cat -nRepository: infiniflow/ragflow
Length of output: 6259
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== 1) Show deleteDocRecordWithCounters function body =="
ln=$(rg -n "func \\(s \\*DocumentService\\) deleteDocRecordWithCounters" -n internal/service/document.go | head -n1 | cut -d: -f1)
echo "function starts at line: $ln"
start=$((ln-20))
end=$((ln+80))
sed -n "${start},${end}p" internal/service/document.go | cat -n
echo
echo "== 2) Inspect docker init.sql for any FK/constraint on document/kb =="
# Only show relevant lines to keep output small
rg -n "document|documents|knowledgebase|knowledgebase\\.id|kb_id|FOREIGN KEY|REFERENCES" docker/init.sql docker/oceanbase/init.d/vec_memory.sql 2>/dev/null || true
# broaden to all docker init scripts
rg -n "FOREIGN KEY|REFERENCES|documents|knowledgebase|kb_id" docker -S || trueRepository: infiniflow/ragflow
Length of output: 4922
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== 1) Search docker/init.sql (and all docker init scripts) for FK/REFERENCES =="
rg -n "FOREIGN KEY|REFERENCES" docker -S || true
echo
echo "== 2) Search for document table DDL + kb_id inside init scripts =="
rg -n "CREATE TABLE.*document|CREATE TABLE.*documents|\\bdocument\\b" docker -S || true
rg -n "kb_id" docker -S || true
echo
echo "== 3) Search repo-wide for DDL that mentions knowledgebase/doc_num/documents.kb_id =="
rg -n --hidden --glob '*.{sql,ddl,psql,postgres,mysql,sqlite}' "knowledgebase|doc_num|CREATE TABLE.*document|\\bdocuments\\b|\\bkb_id\\b" . || trueRepository: infiniflow/ragflow
Length of output: 420
Guard the KB doc_num increment with RowsAffected in DocumentService.InsertDocument.
entity.Document.KbID has no gorm FK/constraint tags, and the test suite inserts an “orphan” document (KbID: "no-such-kb") successfully, so the DB-level FK enforcement isn’t reliably present. As a result, the current Update(...).Error can be nil even when the KB row doesn’t exist (0 rows updated), leaving the counters inconsistent. Capture the update result and roll back when RowsAffected == 0 (similar to the deleteDocRecordWithCounters pattern).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/service/document.go` around lines 288 - 296, In
DocumentService.InsertDocument's transaction block, guard the KB counter update
by capturing the update result instead of only checking Error: call
tx.Model(&entity.Knowledgebase{}).Where("id = ?", doc.KbID).Update("doc_num",
gorm.Expr("doc_num + 1")) into a result variable, return an error and rollback
if result.Error != nil, and also return an error when result.RowsAffected == 0
(indicating no KB row was updated) so the transaction fails when the KB is
missing (follow the same pattern used in deleteDocRecordWithCounters); reference
the tx variable, doc.KbID, and the Update("doc_num", gorm.Expr("doc_num + 1"))
call when making the change.
Comment on lines
+143
to
+145
| // ── 6. Run conversion in background (fire-and-forget) ──────────────────── | ||
| kbIDs := req.KbIDs | ||
| go func() { |
Contributor
There was a problem hiding this comment.
Deduplicate kb_ids before scheduling conversion.
kbMap collapses duplicates for validation, but the goroutine still receives raw req.KbIDs. A payload like ["kb-a", "kb-a"] will create two documents and two mappings for every file in the same dataset.
💡 Proposed fix
- kbIDs := req.KbIDs
+ kbIDs := dedupeStrings(req.KbIDs)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/service/file2document.go` around lines 143 - 145, Deduplicate the kb
IDs before launching the background goroutine: build a unique slice from the
validated kbMap keys (instead of using raw req.KbIDs) and pass that deduplicated
slice into the goroutine that runs the conversion; update the usage of kbIDs in
the anonymous func launched in file2document.go so it iterates over the unique
KB IDs (derived from kbMap) to avoid creating duplicate documents/mappings.
Contributor
Author
|
Hi, @Hz-186 , |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #15674 +/- ##
=======================================
Coverage 93.16% 93.16%
=======================================
Files 10 10
Lines 717 717
Branches 118 118
=======================================
Hits 668 668
Misses 29 29
Partials 20 20 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
Author
|
Hi, @Haruko386, |
Contributor
Author
|
Hi, @yingfeng , Could you please review this PR? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Closes #15673 — ports the Python
file2document_api.pyconvert()endpoint to Go./api/v1/files/link-to-datasetsFileHandler.LinkToDatasetsType of change
Implementation notes
Files changed:
Functional parity table:
file_idsandkb_idsboth required; missing either →CodeDataErrormirroring Python@validate_requestfileDAO.GetByIDs(fileIDs)builds a set; any missing ID →"File not found!"kbDAO.GetByID(kbID)per KB; missing →"Can't find this dataset!"getAllInnermostFileIDsrecursively callsfileDAO.ListByParentID— mirrorsFileService.get_all_innermost_file_idscheckFileTeamPermission:file.TenantID == userIDOR user in tenant's team — mirrorscheck_file_team_permissioncheckKBTeamPermission:kb.TenantID == userIDOR user in tenant's team — mirrorscheck_kb_team_permissiongo convertFiles(...)goroutine after all validation passes — mirrorsloop.run_in_executor(None, _convert_files, …)convertFiles: for each file → delete existing mappings + hard-delete old documents → create newDocumentin each target KB → createFile2Documentmapping — mirrors Python_convert_filesgetParserkb.ParserID— mirrorsFileService.get_parsertruereturned to caller as soon as goroutine is scheduled