Przeglądaj źródła

chore: reduce argument validation complexity

jamesread 6 miesięcy temu
rodzic
commit
8e2db0ad82
2 zmienionych plików z 68 dodań i 59 usunięć
  1. 33 36
      service/internal/api/api.go
  2. 35 23
      service/internal/executor/arguments.go

+ 33 - 36
service/internal/api/api.go

@@ -592,43 +592,8 @@ It uses the same validation logic as the executor, including mangling argument
 values (e.g., datetime formatting, checkbox title-to-value conversion).
 */
 func (api *oliveTinAPI) ValidateArgumentType(ctx ctx.Context, req *connect.Request[apiv1.ValidateArgumentTypeRequest]) (*connect.Response[apiv1.ValidateArgumentTypeResponse], error) {
-	var err error
+	err := api.validateArgumentTypeInternal(req.Msg)
 	desc := ""
-
-	// If binding_id and argument_name are provided, use the full validation path
-	if req.Msg.BindingId != "" && req.Msg.ArgumentName != "" {
-		binding := api.executor.FindBindingByID(req.Msg.BindingId)
-		if binding == nil || binding.Action == nil {
-			return connect.NewResponse(&apiv1.ValidateArgumentTypeResponse{
-				Valid:       false,
-				Description: "action binding not found",
-			}), nil
-		}
-
-		// Find the specific argument
-		var arg *config.ActionArgument
-		for i := range binding.Action.Arguments {
-			if binding.Action.Arguments[i].Name == req.Msg.ArgumentName {
-				arg = &binding.Action.Arguments[i]
-				break
-			}
-		}
-
-		if arg == nil {
-			return connect.NewResponse(&apiv1.ValidateArgumentTypeResponse{
-				Valid:       false,
-				Description: "argument not found",
-			}), nil
-		}
-
-		// Use the same validation path as the executor (includes mangling)
-		err = executor.ValidateArgument(arg, req.Msg.Value, binding.Action)
-	} else {
-		// Fallback to simple type check if binding_id/argument_name not provided
-		// (for backwards compatibility, though this path doesn't handle choices/null checks)
-		err = executor.TypeSafetyCheck("", req.Msg.Value, req.Msg.Type)
-	}
-
 	if err != nil {
 		desc = err.Error()
 	}
@@ -639,6 +604,38 @@ func (api *oliveTinAPI) ValidateArgumentType(ctx ctx.Context, req *connect.Reque
 	}), nil
 }
 
+func (api *oliveTinAPI) validateArgumentTypeInternal(msg *apiv1.ValidateArgumentTypeRequest) error {
+	if msg.BindingId == "" || msg.ArgumentName == "" {
+		return executor.TypeSafetyCheck("", msg.Value, msg.Type)
+	}
+
+	arg, action := api.findArgumentForValidation(msg.BindingId, msg.ArgumentName)
+	if arg == nil {
+		return fmt.Errorf("argument not found")
+	}
+
+	return executor.ValidateArgument(arg, msg.Value, action)
+}
+
+func (api *oliveTinAPI) findArgumentForValidation(bindingId string, argumentName string) (*config.ActionArgument, *config.Action) {
+	binding := api.executor.FindBindingByID(bindingId)
+	if binding == nil || binding.Action == nil {
+		return nil, nil
+	}
+
+	arg := api.findArgumentByName(binding.Action, argumentName)
+	return arg, binding.Action
+}
+
+func (api *oliveTinAPI) findArgumentByName(action *config.Action, name string) *config.ActionArgument {
+	for i := range action.Arguments {
+		if action.Arguments[i].Name == name {
+			return &action.Arguments[i]
+		}
+	}
+	return nil
+}
+
 func (api *oliveTinAPI) WhoAmI(ctx ctx.Context, req *connect.Request[apiv1.WhoAmIRequest]) (*connect.Response[apiv1.WhoAmIResponse], error) {
 	user := auth.UserFromApiCall(ctx, req, api.cfg)
 

+ 35 - 23
service/internal/executor/arguments.go

@@ -388,34 +388,46 @@ func mangleInvalidDatetimeValues(req *ExecutionRequest, arg *config.ActionArgume
 // used during actual execution.
 func MangleArgumentValue(arg *config.ActionArgument, value string, actionTitle string) string {
 	if arg.Type == "datetime" {
-		if value == "" {
-			return value
-		}
+		return mangleDatetimeValue(arg, value, actionTitle)
+	}
+
+	if arg.Type == "checkbox" {
+		return mangleCheckboxValue(arg, value, actionTitle)
+	}
+
+	return value
+}
+
+func mangleDatetimeValue(arg *config.ActionArgument, value string, actionTitle string) string {
+	if value == "" {
+		return value
+	}
 
-		timestamp, err := time.Parse("2006-01-02T15:04", value)
-		if err == nil {
+	timestamp, err := time.Parse("2006-01-02T15:04", value)
+	if err != nil {
+		return value
+	}
+
+	log.WithFields(log.Fields{
+		"arg":         arg.Name,
+		"value":       value,
+		"actionTitle": actionTitle,
+	}).Warnf("Mangled invalid datetime value without seconds to :00 seconds, this issue is commonly caused by Android browsers.")
+
+	return timestamp.Format("2006-01-02T15:04:05")
+}
+
+func mangleCheckboxValue(arg *config.ActionArgument, value string, actionTitle string) string {
+	for _, choice := range arg.Choices {
+		if value == choice.Title {
 			log.WithFields(log.Fields{
 				"arg":         arg.Name,
-				"value":       value,
+				"oldValue":    value,
+				"newValue":    choice.Value,
 				"actionTitle": actionTitle,
-			}).Warnf("Mangled invalid datetime value without seconds to :00 seconds, this issue is commonly caused by Android browsers.")
-
-			return timestamp.Format("2006-01-02T15:04:05")
-		}
-	}
+			}).Infof("Mangled checkbox value")
 
-	if arg.Type == "checkbox" {
-		for _, choice := range arg.Choices {
-			if value == choice.Title {
-				log.WithFields(log.Fields{
-					"arg":         arg.Name,
-					"oldValue":    value,
-					"newValue":    choice.Value,
-					"actionTitle": actionTitle,
-				}).Infof("Mangled checkbox value")
-
-				return choice.Value
-			}
+			return choice.Value
 		}
 	}