feat[Go]: implement delete/ rebuild/ listlog api for connector#15300
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughAdds SyncLogs task_type, DAO methods to cancel/list/rebuild connector logs and documents, service helpers and workflows for delete/rebuild (including engine chunk deletion), handler endpoints for list/delete/rebuild with tests, and route registrations. ChangesConnector Delete and Rebuild Lifecycle Operations
Sequence Diagram(s)sequenceDiagram
participant Client
participant Handler
participant Service
participant ConnectorDAO
participant Engine
Client->>Handler: POST /connectors/:id/rebuild { kb_id }
Handler->>Service: RebuildConnector(connectorID, userID, kbID)
Service->>ConnectorDAO: ListDocumentsByKBAndSourceType(kbID, sourceType)
ConnectorDAO-->>Service: documents[]
Service->>Engine: DeleteChunks(ragflow_<tenantID>, documents)
Engine-->>Service: (ignored errors)
Service->>ConnectorDAO: RebuildConnector(connector, kbID, documents)
ConnectorDAO-->>Service: success
Service-->>Handler: (true, CodeSuccess, nil)
Handler-->>Client: 200 { code, data: true }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
internal/handler/connector_test.go (1)
115-171: ⚡ Quick winAdd handler coverage for
RebuildConnectortoo.The rebuild endpoint introduces its own JSON binding and validation branches, but this file only exercises delete. A small table test for success, missing
kb_id, blankkb_id, and service failure would catch most regressions here.🤖 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/handler/connector_test.go` around lines 115 - 171, Add table-driven tests in the same test file to cover ConnectorHandler.RebuildConnector: create tests for success, missing kb_id (no kb_id field in JSON), blank kb_id (empty string), and service failure (fakeConnectorService returning an error/code). For each case instantiate ConnectorHandler with the appropriate fakeConnectorService, register a route that sets the user context and calls h.RebuildConnector, send a request with the JSON body per case (including content-type), assert HTTP 200 and then unmarshal the response JSON and verify the "code", "data" and "message" fields match expected values for each scenario; reuse the existing test patterns (resp recorder, json.Unmarshal and comparisons) used in TestConnectorHandlerDeleteConnector to keep style consistent.
🤖 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/handler/connector_test.go`:
- Around line 24-27: The fakeConnectorService currently implements Rebuild()
which causes a mismatch because ConnectorHandler.connectorService expects
RebuildConnector; replace or add the correct stub by implementing a
RebuildConnector method on fakeConnectorService with the same signature expected
by the real interface (match parameters/return types used by ConnectorHandler),
and remove or deprecate the Rebuild method; ensure tt.service (used in tests) is
the fakeConnectorService so ConnectorHandler{connectorService: tt.service}
compiles.
In `@internal/handler/connector.go`:
- Around line 180-213: Add or correct the Swagger `@Router` annotations so they
match the real registered routes: add an `@Router` entry for DeleteConnector using
the route template (e.g. "/connector/{connector_id} [delete]") and update
RebuildConnector's `@Router` to use the same template style (e.g.
"/connector/{connector_id}/rebuild [post]"); edit the annotations above the
functions DeleteConnector and RebuildConnector to replace the incorrect or
missing `@Router` lines so generated docs reflect the actual endpoints.
In `@internal/router/router.go`:
- Around line 414-419: The legacy connector route group
(authorized.Group("/v1/connector")) is missing the DELETE route for connector
deletion; add a DELETE route mapping "/:connector_id" to the existing handler
method (r.connectorHandler.DeleteConnector) so the legacy namespace exposes the
same delete API as the newer namespace alongside ListConnectors, GetConnector
and RebuildConnector.
In `@internal/service/connector.go`:
- Around line 221-249: The RebuildConnector flow currently only authorizes
access to the connector but never ensures the supplied kbID is actually linked
to that connector; add a validation after loading connector (and after
canAccessConnector) that the kbID belongs to the connector (e.g., call a DAO
method such as connectorDAO.HasConnectorKBLink(connector.ID, kbID) or
connectorDAO.GetConnectorKBLink/connector2kb lookup) and also verify the KB's
tenant/owner matches connector.TenantID if applicable; if the kbID is not linked
or tenant mismatches, return false with common.CodeAuthenticationError (or
CodeDataError) and a clear error so connectorDAO.RebuildConnector and
connectorDAO.ListDocumentsByKBAndSourceType cannot operate on unrelated KBs.
- Around line 247-264: deleteConnectorDocumentChunks currently removes search
chunks out-of-band (called before the DB transaction), uses
context.Background(), ignores errors and can leave index/db inconsistent if the
DB operation fails; change the flow so chunk deletion is coordinated with the DB
transaction: instead of calling deleteConnectorDocumentChunks before
s.connectorDAO.RebuildConnector, either move the chunk deletion into the same
transactional operation (e.g., add chunk removal to
connectorDAO.RebuildConnector) or have deleteConnectorDocumentChunks return an
error and be invoked after a successful DB commit; also stop using
context.Background() — accept/passthrough the caller context (or create a
cancellable context tied to the request), check engine.Get() != nil and
propagate and handle DeleteChunks errors so failures cause the overall operation
to rollback/return an error rather than silently dropping chunks.
- Around line 44-47: The exported ConnectorService.Rebuild method currently
panics; either remove it if unused or implement it to perform the service-level
rebuild without panicking: locate ConnectorService.Rebuild and replace the panic
with the actual rebuild sequence (e.g., call the underlying components such as
s.repo.Rebuild(), s.store.Rebuild() or s.cache.Clear()/Reload() as appropriate),
handle and log errors via s.logger (or return them if you change the signature),
and ensure the method is safe for concurrent use and does not call panic. Use
the existing service fields (e.g., s.repo, s.store, s.cache, s.logger) to
perform the work and return gracefully or surface errors rather than panicking.
---
Nitpick comments:
In `@internal/handler/connector_test.go`:
- Around line 115-171: Add table-driven tests in the same test file to cover
ConnectorHandler.RebuildConnector: create tests for success, missing kb_id (no
kb_id field in JSON), blank kb_id (empty string), and service failure
(fakeConnectorService returning an error/code). For each case instantiate
ConnectorHandler with the appropriate fakeConnectorService, register a route
that sets the user context and calls h.RebuildConnector, send a request with the
JSON body per case (including content-type), assert HTTP 200 and then unmarshal
the response JSON and verify the "code", "data" and "message" fields match
expected values for each scenario; reuse the existing test patterns (resp
recorder, json.Unmarshal and comparisons) used in
TestConnectorHandlerDeleteConnector to keep style consistent.
🪄 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: bfb738c0-6ef1-4973-b8e4-5e017189b887
📒 Files selected for processing (6)
internal/dao/connector.gointernal/entity/connector.gointernal/handler/connector.gointernal/handler/connector_test.gointernal/router/router.gointernal/service/connector.go
Comment on lines
+180
to
+213
| // DeleteConnector delete connector | ||
| // @Description Detele Connector | ||
| // @Tags connector | ||
| // @Accept json | ||
| // @Produce json | ||
| func (h *ConnectorHandler) DeleteConnector(c *gin.Context) { | ||
| user, errorCode, errorMessage := GetUser(c) | ||
| if errorCode != common.CodeSuccess { | ||
| jsonError(c, errorCode, errorMessage) | ||
| return | ||
| } | ||
|
|
||
| ok, code, err := h.connectorService.DeleteConnector(c.Param("connector_id"), user.ID) | ||
| if err != nil { | ||
| jsonError(c, code, err.Error()) | ||
| return | ||
| } | ||
|
|
||
| c.JSON(http.StatusOK, gin.H{ | ||
| "code": common.CodeSuccess, | ||
| "data": ok, | ||
| "message": "success", | ||
| }) | ||
| } | ||
|
|
||
| // RebuildConnector rebuild connector | ||
| // @Summary Rebuild Connector | ||
| // @Description Trigger a rebuild for an accessible connector and knowledge base | ||
| // @Tags connector | ||
| // @Accept json | ||
| // @Produce json | ||
| // @Success 200 {object} map[string]interface{} | ||
| // @Router /connector/:connector_id/rebuild [post] | ||
| func (h *ConnectorHandler) RebuildConnector(c *gin.Context) { |
Contributor
There was a problem hiding this comment.
Fix the Swagger annotations for the new endpoints.
DeleteConnector is missing an @Router entry, and RebuildConnector documents /connector/:connector_id/rebuild instead of the registered route template. The generated API docs won't match the real endpoints.
🤖 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/handler/connector.go` around lines 180 - 213, Add or correct the
Swagger `@Router` annotations so they match the real registered routes: add an
`@Router` entry for DeleteConnector using the route template (e.g.
"/connector/{connector_id} [delete]") and update RebuildConnector's `@Router` to
use the same template style (e.g. "/connector/{connector_id}/rebuild [post]");
edit the annotations above the functions DeleteConnector and RebuildConnector to
replace the incorrect or missing `@Router` lines so generated docs reflect the
actual endpoints.
Comment on lines
414
to
419
| connector := authorized.Group("/v1/connector") | ||
| { | ||
| connector.GET("/list", r.connectorHandler.ListConnectors) | ||
| connector.GET("/:connector_id", r.connectorHandler.GetConnector) | ||
| connector.POST("/:connector_id/rebuild", r.connectorHandler.RebuildConnector) | ||
| } |
Contributor
There was a problem hiding this comment.
Expose delete on the legacy /v1/connector group too.
GetConnector and RebuildConnector are available on both connector namespaces, but DELETE /v1/connector/:connector_id is missing here. Clients still using the legacy route won't be able to use the new delete API.
Suggested route addition
connector.GET("/list", r.connectorHandler.ListConnectors)
connector.GET("/:connector_id", r.connectorHandler.GetConnector)
+ connector.DELETE("/:connector_id", r.connectorHandler.DeleteConnector)
connector.POST("/:connector_id/rebuild", r.connectorHandler.RebuildConnector)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| connector := authorized.Group("/v1/connector") | |
| { | |
| connector.GET("/list", r.connectorHandler.ListConnectors) | |
| connector.GET("/:connector_id", r.connectorHandler.GetConnector) | |
| connector.POST("/:connector_id/rebuild", r.connectorHandler.RebuildConnector) | |
| } | |
| connector := authorized.Group("/v1/connector") | |
| { | |
| connector.GET("/list", r.connectorHandler.ListConnectors) | |
| connector.GET("/:connector_id", r.connectorHandler.GetConnector) | |
| connector.DELETE("/:connector_id", r.connectorHandler.DeleteConnector) | |
| connector.POST("/:connector_id/rebuild", r.connectorHandler.RebuildConnector) | |
| } |
🤖 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/router/router.go` around lines 414 - 419, The legacy connector route
group (authorized.Group("/v1/connector")) is missing the DELETE route for
connector deletion; add a DELETE route mapping "/:connector_id" to the existing
handler method (r.connectorHandler.DeleteConnector) so the legacy namespace
exposes the same delete API as the newer namespace alongside ListConnectors,
GetConnector and RebuildConnector.
Comment on lines
+44
to
+47
| func (s *ConnectorService) Rebuild() { | ||
| //TODO implement me | ||
| panic("implement me") | ||
| } |
Contributor
There was a problem hiding this comment.
Remove or implement Rebuild() before merge.
This exported method will panic the server on any call path.
Possible fix
-func (s *ConnectorService) Rebuild() {
- //TODO implement me
- panic("implement me")
-}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func (s *ConnectorService) Rebuild() { | |
| //TODO implement me | |
| panic("implement me") | |
| } |
🤖 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/connector.go` around lines 44 - 47, The exported
ConnectorService.Rebuild method currently panics; either remove it if unused or
implement it to perform the service-level rebuild without panicking: locate
ConnectorService.Rebuild and replace the panic with the actual rebuild sequence
(e.g., call the underlying components such as s.repo.Rebuild(),
s.store.Rebuild() or s.cache.Clear()/Reload() as appropriate), handle and log
errors via s.logger (or return them if you change the signature), and ensure the
method is safe for concurrent use and does not call panic. Use the existing
service fields (e.g., s.repo, s.store, s.cache, s.logger) to perform the work
and return gracefully or surface errors rather than panicking.
Comment on lines
+221
to
+249
| func (s *ConnectorService) RebuildConnector(connectorID, userID, kbID string) (bool, common.ErrorCode, error) { | ||
| if connectorID == "" { | ||
| return false, common.CodeDataError, fmt.Errorf("connector_id is required") | ||
| } | ||
| if kbID == "" { | ||
| return false, common.CodeArgumentError, fmt.Errorf("required argument is missing: kb_id") | ||
| } | ||
|
|
||
| connector, err := s.connectorDAO.GetByID(connectorID) | ||
| if err != nil { | ||
| if errors.Is(err, gorm.ErrRecordNotFound) { | ||
| return false, common.CodeDataError, fmt.Errorf("Can't find this Connector!") | ||
| } | ||
| return false, common.CodeServerError, err | ||
| } | ||
|
|
||
| if !s.canAccessConnector(connector, userID) { | ||
| return false, common.CodeAuthenticationError, fmt.Errorf("No authorization.") | ||
| } | ||
|
|
||
| sourceType := fmt.Sprintf("%s/%s", connector.Source, connector.ID) | ||
| documents, err := s.connectorDAO.ListDocumentsByKBAndSourceType(kbID, sourceType) | ||
| if err != nil { | ||
| return false, common.CodeServerError, err | ||
| } | ||
|
|
||
| s.deleteConnectorDocumentChunks(connector.TenantID, kbID, documents) | ||
|
|
||
| if err := s.connectorDAO.RebuildConnector(connector, kbID, documents); err != nil { |
Contributor
There was a problem hiding this comment.
Validate that kb_id is actually linked to this connector.
This flow only authorizes the connector. A caller can pass any kb_id, and the DAO then updates knowledgebase by that raw ID and creates rebuild logs for the pair without checking connector2kb or KB ownership. That opens the door to mutating unrelated datasets if an ID is known.
🤖 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/connector.go` around lines 221 - 249, The RebuildConnector
flow currently only authorizes access to the connector but never ensures the
supplied kbID is actually linked to that connector; add a validation after
loading connector (and after canAccessConnector) that the kbID belongs to the
connector (e.g., call a DAO method such as
connectorDAO.HasConnectorKBLink(connector.ID, kbID) or
connectorDAO.GetConnectorKBLink/connector2kb lookup) and also verify the KB's
tenant/owner matches connector.TenantID if applicable; if the kbID is not linked
or tenant mismatches, return false with common.CodeAuthenticationError (or
CodeDataError) and a clear error so connectorDAO.RebuildConnector and
connectorDAO.ListDocumentsByKBAndSourceType cannot operate on unrelated KBs.
Comment on lines
+247
to
+264
| s.deleteConnectorDocumentChunks(connector.TenantID, kbID, documents) | ||
|
|
||
| if err := s.connectorDAO.RebuildConnector(connector, kbID, documents); err != nil { | ||
| return false, common.CodeServerError, err | ||
| } | ||
| return true, common.CodeSuccess, nil | ||
| } | ||
|
|
||
| func (s *ConnectorService) deleteConnectorDocumentChunks(tenantID, kbID string, documents []*entity.Document) { | ||
| docEngine := engine.Get() | ||
| if docEngine == nil { | ||
| return | ||
| } | ||
|
|
||
| indexName := fmt.Sprintf("ragflow_%s", tenantID) | ||
| for _, document := range documents { | ||
| _, _ = docEngine.DeleteChunks(context.Background(), map[string]interface{}{"doc_id": document.ID}, indexName, kbID) | ||
| } |
Contributor
There was a problem hiding this comment.
Don't delete search chunks out-of-band and ignore the result.
deleteConnectorDocumentChunks runs before the DB transaction, uses context.Background(), and drops the engine error. If chunk deletion fails, rebuild can still return success; if the later transaction fails, the chunks are already gone. That leaves the search index and database out of sync.
🤖 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/connector.go` around lines 247 - 264,
deleteConnectorDocumentChunks currently removes search chunks out-of-band
(called before the DB transaction), uses context.Background(), ignores errors
and can leave index/db inconsistent if the DB operation fails; change the flow
so chunk deletion is coordinated with the DB transaction: instead of calling
deleteConnectorDocumentChunks before s.connectorDAO.RebuildConnector, either
move the chunk deletion into the same transactional operation (e.g., add chunk
removal to connectorDAO.RebuildConnector) or have deleteConnectorDocumentChunks
return an error and be invoked after a successful DB commit; also stop using
context.Background() — accept/passthrough the caller context (or create a
cancellable context tied to the request), check engine.Get() != nil and
propagate and handle DeleteChunks errors so failures cause the overall operation
to rollback/return an error rather than silently dropping chunks.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #15300 +/- ##
=======================================
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 Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/handler/connector.go (1)
283-293:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNormalize
kb_idbefore calling service.You validate trimmed
kb_id, but pass the untrimmed value. Leading/trailing whitespace can still leak into DAO queries.Suggested patch
- if strings.TrimSpace(req.KbID) == "" { + kbID := strings.TrimSpace(req.KbID) + if kbID == "" { c.JSON(http.StatusOK, gin.H{ "code": common.CodeDataError, "data": nil, "message": "kb_id cannot be empty", }) return } - ok, code, err := h.connectorService.RebuildConnector(c.Param("connector_id"), user.ID, req.KbID) + ok, code, err := h.connectorService.RebuildConnector(c.Param("connector_id"), user.ID, kbID)🤖 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/handler/connector.go` around lines 283 - 293, Trim and normalize req.KbID before using it: after the existing validation that checks strings.TrimSpace(req.KbID), assign the trimmed value back (e.g., kbID := strings.TrimSpace(req.KbID)) and pass that kbID into h.connectorService.RebuildConnector(c.Param("connector_id"), user.ID, kbID) so the service/DAO never receives untrimmed whitespace from req.KbID.
♻️ Duplicate comments (1)
internal/handler/connector.go (1)
230-235:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix Swagger route annotations for delete/rebuild endpoints.
DeleteConnectorstill has no@Router, andRebuildConnectorstill documents a different path template than the registered route. Generated docs won’t match runtime endpoints.Suggested patch
// DeleteConnector delete connector +// `@Summary` Delete Connector // `@Description` Detele Connector // `@Tags` connector // `@Accept` json // `@Produce` json +// `@Success` 200 {object} map[string]interface{} +// `@Router` /api/v1/connectors/{connector_id} [delete] func (h *ConnectorHandler) DeleteConnector(c *gin.Context) { @@ // `@Accept` json // `@Produce` json // `@Success` 200 {object} map[string]interface{} -// `@Router` /connector/:connector_id/rebuild [post] +// `@Router` /api/v1/connectors/{connector_id}/rebuild [post] func (h *ConnectorHandler) RebuildConnector(c *gin.Context) {Also applies to: 255-263
🤖 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/handler/connector.go` around lines 230 - 235, Add or correct the Swagger `@Router` annotations so generated docs match runtime endpoints: add an appropriate `@Router` entry to the DeleteConnector function's comment block (so DeleteConnector has a `@Router` line) and update the RebuildConnector function's `@Router` annotation to use the exact path template used by the registered route (fix the path/placeholders to match the actual route). Locate the comment blocks above the DeleteConnector and RebuildConnector functions in connector.go and modify their Swagger annotations (`@Router`, HTTP verb and path) to exactly match the runtime route definitions.
🧹 Nitpick comments (1)
internal/handler/connector_test.go (1)
185-271: ⚡ Quick winAdd handler tests for
RebuildConnectorpaths.This PR adds a new endpoint, but there’s no corresponding handler test yet. Please add table-driven cases for:
- success (
kb_idpresent),- missing/empty
kb_id,- service error propagation (e.g., auth/data/server code mapping).
🤖 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/handler/connector_test.go` around lines 185 - 271, Add table-driven tests in internal/handler/connector_test.go mirroring the style of TestConnectorHandlerListLogs that exercise ConnectorHandler.RebuildConnector via the test router; create cases for (1) success when a valid kb_id is provided (assert response code, message, and any returned data), (2) missing/empty kb_id (assert appropriate validation error response), and (3) service error propagation by using fakeConnectorService with different code/err values (e.g., common.CodeAuthenticationError and a data/server error) to verify the handler maps service errors to the correct HTTP response and body. Use the same test scaffolding: instantiate h := &ConnectorHandler{connectorService: tt.service}, register a route that sets the user in context and calls h.RebuildConnector, send requests with appropriate query/body parameters, unmarshal the JSON response, and assert body["code"], body["message"], and any body["data"] fields as per each case.
🤖 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.
Outside diff comments:
In `@internal/handler/connector.go`:
- Around line 283-293: Trim and normalize req.KbID before using it: after the
existing validation that checks strings.TrimSpace(req.KbID), assign the trimmed
value back (e.g., kbID := strings.TrimSpace(req.KbID)) and pass that kbID into
h.connectorService.RebuildConnector(c.Param("connector_id"), user.ID, kbID) so
the service/DAO never receives untrimmed whitespace from req.KbID.
---
Duplicate comments:
In `@internal/handler/connector.go`:
- Around line 230-235: Add or correct the Swagger `@Router` annotations so
generated docs match runtime endpoints: add an appropriate `@Router` entry to the
DeleteConnector function's comment block (so DeleteConnector has a `@Router` line)
and update the RebuildConnector function's `@Router` annotation to use the exact
path template used by the registered route (fix the path/placeholders to match
the actual route). Locate the comment blocks above the DeleteConnector and
RebuildConnector functions in connector.go and modify their Swagger annotations
(`@Router`, HTTP verb and path) to exactly match the runtime route definitions.
---
Nitpick comments:
In `@internal/handler/connector_test.go`:
- Around line 185-271: Add table-driven tests in
internal/handler/connector_test.go mirroring the style of
TestConnectorHandlerListLogs that exercise ConnectorHandler.RebuildConnector via
the test router; create cases for (1) success when a valid kb_id is provided
(assert response code, message, and any returned data), (2) missing/empty kb_id
(assert appropriate validation error response), and (3) service error
propagation by using fakeConnectorService with different code/err values (e.g.,
common.CodeAuthenticationError and a data/server error) to verify the handler
maps service errors to the correct HTTP response and body. Use the same test
scaffolding: instantiate h := &ConnectorHandler{connectorService: tt.service},
register a route that sets the user in context and calls h.RebuildConnector,
send requests with appropriate query/body parameters, unmarshal the JSON
response, and assert body["code"], body["message"], and any body["data"] fields
as per each case.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 05f23164-1380-442b-b5dd-7296b4e24fbe
📒 Files selected for processing (6)
internal/dao/connector.gointernal/entity/connector.gointernal/handler/connector.gointernal/handler/connector_test.gointernal/router/router.gointernal/service/connector.go
🚧 Files skipped from review as they are similar to previous changes (2)
- internal/router/router.go
- internal/service/connector.go
JPette1783
added a commit
to JPette1783/ragflow
that referenced
this pull request
May 28, 2026
…ed by infiniflow#15300 Per reviewer request, remove Create, Get, Update, Delete, ListLogs, and Rebuild connector endpoints (already implemented in infiniflow#15300). Keep only POST /:connector_id/test which is not covered there. - handler: remove all methods except ListConnectors and TestConnector; introduce connectorServiceIface for testability; update test to cover TestConnector cases - service: remove CreateConnector, UpdateConnector, DeleteConnector, ListLogs, Rebuild, and all scheduling/task helpers; keep accessible and TestConnector - dao: remove SyncLogsDAO and Connector2KbDAO (no longer used) - entity: remove ConnectorTaskType and ConnectorInputType constants - router: remove all connector routes except GET / and POST /:id/test
JinHai-CN
pushed a commit
that referenced
this pull request
May 28, 2026
## Summary - Add Go REST support for `GET /api/v1/system/healthz`. - Return Python-compatible `ok`/`nok` dependency fields for DB, Redis, document engine, and storage. - Return HTTP 200 only when all checks pass; otherwise return HTTP 500 with `_meta` failure details. - Add focused service coverage for the unhealthy dependency response when Go dependencies are not initialized. ## Scope This is a small, isolated slice of #15240. It avoids current open connector PRs (#15274, #15300, #15265, #15264), tenant/member PRs (#15295, #15301, #15276), MCP PRs (#15281, #15253, #15254, #15260, #15261, #15262), and the memory-message PR (#15256). Refs #15240
JPette1783
added a commit
to JPette1783/ragflow
that referenced
this pull request
May 28, 2026
Upstream PR infiniflow#15300 merged the Get/List/Delete/Rebuild/ListLog connector endpoints and our branch keeps only the Test endpoint, so the merge had to interleave handlers, tests, services, and DAO additions. Resolved conflicts by keeping every method from both sides: - internal/router/router.go: keep upstream's GET/POST/DELETE routes plus our POST /:connector_id/test route on the connectors group - internal/handler/connector.go: union the connectorServiceIface methods (ListConnectors, CreateConnector, GetConnector, ListLog, DeleteConnector, RebuildConnector, TestConnector); union the handler functions; consolidate imports (errors, net/http, strconv, strings, common, entity, service) - internal/handler/connector_test.go: union fakeConnectorService fields and methods so it satisfies the wider interface; add the missing CreateConnector method; keep both TestConnectorHandlerTestConnector and the upstream Delete/ListLogs tests - internal/service/connector.go: keep upstream's CreateConnectorRequest, RebuildConnectorRequest, canAccessConnector, cancelConnectorTasks, CreateConnector, GetConnector, DeleteConnector, RebuildConnector, deleteConnectorDocumentChunks, ListLog AND our accessible + TestConnector - internal/dao/connector.go: keep upstream's CancelRunningOrScheduledLogs, ListDocumentsByKBAndSourceType, RebuildConnector, createRebuildSyncLog, ListLogsByConnectorID
This was referenced May 28, 2026
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?
implement delete, rebuild api for connector
Type of change