From ad5c331c0bebf3a45fb2ac87bdf73f80eed02415 Mon Sep 17 00:00:00 2001 From: buua436 Date: Fri, 23 Oct 2026 16:59:28 +0800 Subject: [PATCH] fix: remove system variable API routes (#21410) --- internal/cli/cli_http.go | 6 --- internal/cli/user_command.go | 96 ---------------------------------- internal/cli/user_parser.go | 63 ---------------------- internal/handler/system.go | 72 ------------------------- internal/router/router.go | 5 -- internal/router/router_test.go | 28 ++++++++++ internal/service/system.go | 68 +----------------------- 7 files changed, 29 insertions(+), 309 deletions(-) diff --git a/internal/cli/cli_http.go b/internal/cli/cli_http.go index 4bbef05a21..5016260b92 100644 --- a/internal/cli/cli_http.go +++ b/internal/cli/cli_http.go @@ -380,10 +380,6 @@ func (c *CLI) ExecuteUserCommand(commandCount int, cmd *Command) (ResponseIf, er return c.APIDeleteAPIKeyCommand(commandCount, cmd) case "api_set_api_key": return c.APISetAPIKeyCommand(commandCount, cmd) - case "api_set_variable": - return c.APISetVariableCommand(commandCount, cmd) - case "api_show_variable": - return c.APIShowVariableCommand(commandCount, cmd) case "api_unset_api_key": return c.APIUnsetAPIKeyCommand(commandCount, cmd) case "api_show_version": @@ -538,8 +534,6 @@ func (c *CLI) ExecuteUserCommand(commandCount int, cmd *Command) (ResponseIf, er return c.CommonListAPIServersCommand(commandCount, cmd) case "api_list_environments": return c.APIListEnvironmentsCommand(commandCount, cmd) - case "api_list_variables": - return c.APIListVariablesCommand(commandCount, cmd) case "api_add_api_server": return c.AddAPIServerCommand(commandCount, cmd) case "api_delete_api_server": diff --git a/internal/cli/user_command.go b/internal/cli/user_command.go index 54855500a2..8380536be1 100644 --- a/internal/cli/user_command.go +++ b/internal/cli/user_command.go @@ -910,87 +910,6 @@ func (c *CLI) APISetAPIKeyCommand(commandCount int, cmd *Command) (ResponseIf, e return &successResult, nil } -// APISetVariableCommand sets variable value -func (c *CLI) APISetVariableCommand(commandCount int, cmd *Command) (ResponseIf, error) { - httpClient, err := c.apiModeClient() - if err != nil { - return nil, err - } - - varName, ok := cmd.Params["var_name"].(string) - if !ok { - return nil, fmt.Errorf("var_name not provided") - } - varValue, ok := cmd.Params["var_value"].(string) - if !ok { - return nil, fmt.Errorf("var_value not provided") - } - - payload := map[string]interface{}{ - "var_name": varName, - "var_value": varValue, - } - resp, err := httpClient.Request(commandCount, "PUT", "/system/variables", httpClient.AuthKind(), nil, payload) - if err != nil { - return nil, fmt.Errorf("failed to set variable: %w", err) - } - - if resp.StatusCode != 200 { - return nil, fmt.Errorf("failed to set variable: HTTP %d, body: %s", resp.StatusCode, string(resp.Body)) - } - - var result MessageResponse - if err = json.Unmarshal(resp.Body, &result); err != nil { - return nil, fmt.Errorf("set variable failed: invalid JSON (%w)", err) - } - - if result.Code != 0 { - return nil, fmt.Errorf("%s", result.Message) - } - - result.Duration = resp.Duration - return &result, nil -} - -// APIShowVariableCommand displays variable value -func (c *CLI) APIShowVariableCommand(commandCount int, cmd *Command) (ResponseIf, error) { - httpClient, err := c.apiModeClient() - if err != nil { - return nil, err - } - - varName, ok := cmd.Params["var_name"].(string) - if !ok { - return nil, fmt.Errorf("var_name not provided") - } - - EncodedVarName := common.EncodeToBase64(varName) - - endPoint := fmt.Sprintf("/system/variables/%s", EncodedVarName) - - resp, err := httpClient.Request(commandCount, "GET", endPoint, httpClient.AuthKind(), nil, nil) - if err != nil { - return nil, fmt.Errorf("failed to get variable: %w", err) - } - - if resp.StatusCode != 200 { - return nil, fmt.Errorf("failed to get variable: HTTP %d, body: %s", resp.StatusCode, string(resp.Body)) - } - - var result CommonResponse - if err = json.Unmarshal(resp.Body, &result); err != nil { - return nil, fmt.Errorf("show variable failed: invalid JSON (%w)", err) - } - - if result.Code != 0 { - return nil, fmt.Errorf("%s", result.Message) - } - - normalizeVariableRows(result.Data) - result.Duration = resp.Duration - return &result, nil -} - // APIShowAPIKeyCommand displays the current API key func (c *CLI) APIShowAPIKeyCommand(commandCount int, cmd *Command) (ResponseIf, error) { if c.Config.CLIMode != APIMode { @@ -3280,21 +3199,6 @@ func (c *CLI) APIListEnvironmentsCommand(commandCount int, cmd *Command) (Respon return HandleCommonResponse(resp, "list environments") } -// APIListVariablesCommand lists all system variables (api mode only). -func (c *CLI) APIListVariablesCommand(commandCount int, cmd *Command) (ResponseIf, error) { - httpClient, err := c.apiModeClient() - if err != nil { - return nil, err - } - - resp, err := httpClient.Request(commandCount, "GET", "/system/variables", httpClient.AuthKind(), nil, nil) - if err != nil { - return nil, fmt.Errorf("failed to list variables: %w", err) - } - - return HandleCommonResponse(resp, "list variables") -} - func (c *CLI) APIStartIngestionCommand(commandCount int, cmd *Command) (ResponseIf, error) { httpClient, err := c.apiModeClient() if err != nil { diff --git a/internal/cli/user_parser.go b/internal/cli/user_parser.go index fe41b7f1c7..7bf74c6d53 100644 --- a/internal/cli/user_parser.go +++ b/internal/cli/user_parser.go @@ -161,8 +161,6 @@ func (p *Parser) parseAPIListCommands() (*Command, error) { return p.parseAPIListAPIServers() case TokenEnvs: return p.parseAPIListEnvironments() - case TokenVars: - return p.parseAPIListVariables() case TokenGroups: return p.parseAPIListModelGroups() default: @@ -607,19 +605,6 @@ func (p *Parser) parseAPIListEnvironments() (*Command, error) { return cmd, nil } -func (p *Parser) parseAPIListVariables() (*Command, error) { - p.nextToken() // consume Variables - - cmd := NewCommand("api_list_variables") - - // Semicolon is optional - if p.curToken.Type == TokenSemicolon { - p.nextToken() - } - - return cmd, nil -} - // endregion LIST commands // region SHOW commands @@ -633,8 +618,6 @@ func (p *Parser) parseAPIShowCommands() (*Command, error) { return p.parseAPIShowKey() case TokenCurrent: return p.parseAPIShowCurrent() - case TokenVar: - return p.parseAPIShowVariable() case TokenProvider: return p.parseAPIShowProviderCommands() case TokenModel: @@ -692,24 +675,6 @@ func (p *Parser) parseAPIShowCurrent() (*Command, error) { return NewCommand("api_show_current"), nil } -func (p *Parser) parseAPIShowVariable() (*Command, error) { - p.nextToken() // consume VAR - varName, err := p.parseQuotedString() - if err != nil { - return nil, err - } - p.nextToken() - - cmd := NewCommand("api_show_variable") - cmd.Params["var_name"] = varName - - // Semicolon is optional - if p.curToken.Type == TokenSemicolon { - p.nextToken() - } - return cmd, nil -} - // SHOW PROVIDER ; // SHOW PROVIDER INSTANCE ; // SHOW PROVIDER INSTANCE BALANCE; @@ -2357,8 +2322,6 @@ func (p *Parser) parseAPISetCommands() (*Command, error) { p.nextToken() // consume SET switch p.curToken.Type { - case TokenVar: - return p.parseAPISetVariable() case TokenDefault: return p.parseAPISetDefault() case TokenKey: @@ -2378,32 +2341,6 @@ func (p *Parser) parseAPISetCommands() (*Command, error) { } } -func (p *Parser) parseAPISetVariable() (*Command, error) { - p.nextToken() // consume VAR - - varName, err := p.parseQuotedString() - if err != nil { - return nil, err - } - p.nextToken() - - varValue, err := p.parseVariableValue() - if err != nil { - return nil, err - } - p.nextToken() - - cmd := NewCommand("api_set_variable") - cmd.Params["var_name"] = varName - cmd.Params["var_value"] = varValue - - // Semicolon is optional - if p.curToken.Type == TokenSemicolon { - p.nextToken() - } - return cmd, nil -} - func (p *Parser) parseAPISetDefault() (*Command, error) { p.nextToken() // consume DEFAULT diff --git a/internal/handler/system.go b/internal/handler/system.go index cbb0991651..aec69a33c9 100644 --- a/internal/handler/system.go +++ b/internal/handler/system.go @@ -168,78 +168,6 @@ func (h *SystemHandler) SetLogLevel(c *gin.Context) { common.SuccessWithData(c, gin.H{"level": req.Level}, "SUCCESS") } -// ListVariables handle list variables -func (h *SystemHandler) ListVariables(c *gin.Context) { - ctx := c.Request.Context() - - variables, err := h.systemService.ListAllVariables(ctx) - if err != nil { - common.ErrorWithCode(c, common.CodeServerError, err.Error()) - return - } - - common.SuccessWithData(c, variables, "SUCCESS") -} - -// SetVariableHTTPRequest set variable request -type SetVariableHTTPRequest struct { - VarName string `json:"var_name" binding:"required"` - VarValue string `json:"var_value" binding:"required"` -} - -// SetVariable handle set variable -// Python logic: update or create a system setting with the given name and value -func (h *SystemHandler) SetVariable(c *gin.Context) { - var req SetVariableHTTPRequest - if err := c.ShouldBindJSON(&req); err != nil { - common.ErrorWithCode(c, common.CodeBadRequest, "Var name is required") - return - } - - if req.VarName == "" { - common.ErrorWithCode(c, common.CodeBadRequest, "Var name is required") - return - } - - if req.VarValue == "" { - common.ErrorWithCode(c, common.CodeBadRequest, "Var value is required") - return - } - - ctx := c.Request.Context() - - if err := h.systemService.SetVariable(ctx, req.VarName, req.VarValue); err != nil { - common.ErrorWithCode(c, common.CodeServerError, err.Error()) - return - } - - common.SuccessNoData(c, "SUCCESS") -} - -func (h *SystemHandler) ShowVariable(c *gin.Context) { - encodedVarName := c.Param("var_name") - - varName, err := common.DecodeFromBase64(encodedVarName) - if err != nil { - common.ErrorWithCode(c, common.CodeBadRequest, err.Error()) - return - } - if varName == "" { - common.ErrorWithCode(c, common.CodeBadRequest, "Var name is required") - return - } - - ctx := c.Request.Context() - - variable, err := h.systemService.ShowVariable(ctx, varName) - if err != nil { - common.ErrorWithCode(c, common.CodeServerError, err.Error()) - return - } - - common.SuccessWithData(c, variable, "SUCCESS") -} - // ListEnvironments handle list environments func (h *SystemHandler) ListEnvironments(c *gin.Context) { environments, err := h.systemService.ListEnvironments() diff --git a/internal/router/router.go b/internal/router/router.go index 4c3c8e9250..6263827ecf 100644 --- a/internal/router/router.go +++ b/internal/router/router.go @@ -662,11 +662,6 @@ func (r *Router) Setup(engine *gin.Engine) { config.PUT("/log", r.systemHandler.SetLogLevel) } - // Variables/Settings - system.GET("/variables", r.systemHandler.ListVariables) - system.PUT("/variables", r.systemHandler.SetVariable) - system.GET("/variables/:var_name", r.systemHandler.ShowVariable) - // Environments system.GET("/environments", r.systemHandler.ListEnvironments) diff --git a/internal/router/router_test.go b/internal/router/router_test.go index aa40fc123c..4682188ced 100644 --- a/internal/router/router_test.go +++ b/internal/router/router_test.go @@ -57,6 +57,34 @@ func TestRouterSetupRemovesActionRoutes(t *testing.T) { } } +func TestSystemVariableRoutesAreNotRegisteredOnAPI(t *testing.T) { + gin.SetMode(gin.TestMode) + originalLogger := common.Logger + common.Logger = zap.NewNop() + t.Cleanup(func() { common.Logger = originalLogger }) + + engine := gin.New() + r := &Router{authHandler: handler.NewAuthHandler()} + r.Setup(engine) + + for _, tt := range []struct { + method string + path string + }{ + {http.MethodGet, "/api/v1/system/variables"}, + {http.MethodPut, "/api/v1/system/variables"}, + {http.MethodGet, "/api/v1/system/variables/register_enabled"}, + } { + resp := httptest.NewRecorder() + req := httptest.NewRequest(tt.method, tt.path, nil) + engine.ServeHTTP(resp, req) + + if resp.Code != http.StatusNotFound { + t.Errorf("%s %s status=%d body=%s; want system variable route to be unavailable on the API service", tt.method, tt.path, resp.Code, resp.Body.String()) + } + } +} + func TestConnectorRoutesDoNotConflictWithOAuthCallbacks(t *testing.T) { gin.SetMode(gin.TestMode) diff --git a/internal/service/system.go b/internal/service/system.go index 5c2c3c04d1..e0587a91b8 100644 --- a/internal/service/system.go +++ b/internal/service/system.go @@ -22,7 +22,6 @@ import ( "fmt" "ragflow/internal/common" "ragflow/internal/engine/kvrocks" - "ragflow/internal/entity" "time" "ragflow/internal/dao" @@ -33,14 +32,11 @@ import ( // SystemService system service type SystemService struct { - systemSettingsDAO *dao.SystemSettingsDAO } // NewSystemService create system service func NewSystemService() *SystemService { - return &SystemService{ - systemSettingsDAO: dao.NewSystemSettingsDAO(), - } + return &SystemService{} } // ConfigResponse system configuration response @@ -398,68 +394,6 @@ func (s *SystemService) Healthz(ctx context.Context) (*HealthzResponse, bool) { return GetComponentsHealthz(ctx) } -// ListAllVariables list all variables -// Returns all system settings from database -func (s *SystemService) ListAllVariables(ctx context.Context) ([]map[string]interface{}, error) { - settings, err := s.systemSettingsDAO.GetAll(ctx, dao.DB) - if err != nil { - return nil, err - } - - return entity.FormatSystemSettings(settings), nil -} - -func (s *SystemService) ShowVariable(ctx context.Context, varName string) ([]map[string]interface{}, error) { - settings, err := s.systemSettingsDAO.GetByName(ctx, dao.DB, varName) - if err != nil { - return nil, err - } - - if len(settings) == 0 { - settings, err = s.systemSettingsDAO.GetByNamePrefix(ctx, dao.DB, varName) - if err != nil { - return nil, err - } - if len(settings) == 0 { - return nil, fmt.Errorf("can't get setting: %s", varName) - } - } - return entity.FormatSystemSettings(settings), nil -} - -// SetVariable set variable -// Creates or updates a system setting -// If the setting exists, updates it; otherwise creates a new one -func (s *SystemService) SetVariable(ctx context.Context, varName, varValue string) error { - settings, err := s.systemSettingsDAO.GetByName(ctx, dao.DB, varName) - if err != nil { - return err - } - - if len(settings) == 1 { - setting := &settings[0] - if err = entity.ValidateSystemSettingValue(*setting, varValue); err != nil { - return err - } - setting.Value = varValue - return s.systemSettingsDAO.UpdateByName(ctx, dao.DB, varName, setting) - } else if len(settings) > 1 { - return fmt.Errorf("can't update more than 1 setting: %s", varName) - } - - dataType := entity.InferSystemSettingDataType(varName) - newSetting := &entity.SystemSettings{ - Name: varName, - Value: varValue, - Source: "admin", - DataType: dataType, - } - if err = entity.ValidateSystemSettingValue(*newSetting, varValue); err != nil { - return err - } - return s.systemSettingsDAO.Create(ctx, dao.DB, newSetting) -} - // Environment methods // ListEnvironments list all environments