From 4744169aa013a7360449418d5bfef8845b2ff8b4 Mon Sep 17 00:00:00 2001 From: jamesread Date: Thu, 26 Feb 2026 23:07:07 +0000 Subject: [PATCH] chore: code cleanup, remove todos, etc --- .pre-commit-config.yaml | 33 +++++++++- service/internal/api/api.go | 2 +- service/internal/config/sanitize.go | 3 +- service/internal/executor/executor.go | 65 +++++++++++-------- service/internal/executor/executor_actions.go | 34 ++++++++++ 5 files changed, 105 insertions(+), 32 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 4efad37..dda308e 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -9,6 +9,19 @@ repos: - id: end-of-file-fixer - id: check-yaml - id: check-added-large-files + - id: check-merge-conflict + - id: detect-private-key + - id: mixed-line-ending + args: ['--fix', 'lf'] + - id: check-json + exclude: | + (?x)^( + service/internal/entities/testdata/.*\.json| + integration-tests/tests/.*/entities/.*\.json| + var/entities/.*\.json + )$ + - id: check-case-conflict + - id: detect-aws-credentials # Alternative semantic commit checker - repo: https://github.com/compilerla/conventional-pre-commit @@ -34,9 +47,23 @@ repos: pass_filenames: false always_run: true - - id: it - name: it - entry: make service-codestyle frontend-codestyle + - id: service-unittests + name: service-unittests + entry: make service-unittests + language: system + pass_filenames: false + always_run: true + + - id: service-build + name: service-build + entry: make service + language: system + pass_filenames: false + always_run: true + + - id: it + name: integration-tests + entry: make it language: system pass_filenames: false always_run: true diff --git a/service/internal/api/api.go b/service/internal/api/api.go index c557243..b479e96 100644 --- a/service/internal/api/api.go +++ b/service/internal/api/api.go @@ -1268,7 +1268,7 @@ func (api *oliveTinAPI) RestartAction(ctx ctx.Context, req *connect.Request[apiv return api.StartAction(ctx, &connect.Request[apiv1.StartActionRequest]{ Msg: &apiv1.StartActionRequest{ - // FIXME + BindingId: execReqLogEntry.GetBindingId(), UniqueTrackingId: req.Msg.ExecutionTrackingId, }, }) diff --git a/service/internal/config/sanitize.go b/service/internal/config/sanitize.go index 6b64438..f699b80 100644 --- a/service/internal/config/sanitize.go +++ b/service/internal/config/sanitize.go @@ -259,8 +259,7 @@ func (arg *ActionArgument) sanitize() { arg.sanitizeNoType() - // TODO Validate the default against the type checker, but this creates a - // import loop + // Default value validation runs in executor at config load (validateArgumentDefaults). } func (arg *ActionArgument) sanitizeNoType() { diff --git a/service/internal/executor/executor.go b/service/internal/executor/executor.go index 694cc9b..91312d4 100644 --- a/service/internal/executor/executor.go +++ b/service/internal/executor/executor.go @@ -603,14 +603,14 @@ func getExecutionsCount(rate config.RateSpec, req *ExecutionRequest) int { then := time.Now().Add(-duration) + currentEntityPrefix := "" + if req.Binding != nil && req.Binding.Entity != nil { + currentEntityPrefix = req.Binding.Entity.UniqueKey + } for _, logEntry := range req.executor.GetLogsByBindingId(req.Binding.ID) { - // FIXME - /* - if logEntry.EntityPrefix != req.EntityPrefix { - continue - } - */ - + if logEntry.EntityPrefix != currentEntityPrefix { + continue + } if logEntry.DatetimeStarted.After(then) && !logEntry.Blocked { executions += 1 @@ -761,28 +761,12 @@ func fail(req *ExecutionRequest, err error) bool { func stepRequestAction(req *ExecutionRequest) bool { metricActionsRequested.Inc() - // If there is no binding or action, do not proceed. Leave default - // log entry values (icon/title/id) and stop execution gracefully. - if req.Binding == nil || req.Binding.Action == nil { - log.Warnf("Action request has no binding/action; skipping execution") + if !stepRequestActionHasBinding(req) { return false } - req.logEntry.Binding = req.Binding - req.logEntry.ActionConfigTitle = req.Binding.Action.Title - req.logEntry.ActionTitle = tpl.ParseTemplateOfActionBeforeExec(req.Binding.Action.Title, req.Binding.Entity) - req.logEntry.ActionIcon = req.Binding.Action.Icon - req.logEntry.Tags = req.Tags - - req.executor.logmutex.Lock() - - if _, containsKey := req.executor.LogsByBindingId[req.Binding.ID]; !containsKey { - req.executor.LogsByBindingId[req.Binding.ID] = make([]*InternalLogEntry, 0) - } - - req.executor.LogsByBindingId[req.Binding.ID] = append(req.executor.LogsByBindingId[req.Binding.ID], req.logEntry) - - req.executor.logmutex.Unlock() + stepRequestActionPopulateLogEntry(req) + stepRequestActionRegisterLog(req) log.WithFields(log.Fields{ "actionTitle": req.logEntry.ActionTitle, @@ -794,6 +778,35 @@ func stepRequestAction(req *ExecutionRequest) bool { return true } +func stepRequestActionHasBinding(req *ExecutionRequest) bool { + if req.Binding == nil || req.Binding.Action == nil { + log.Warnf("Action request has no binding/action; skipping execution") + return false + } + return true +} + +func stepRequestActionPopulateLogEntry(req *ExecutionRequest) { + req.logEntry.Binding = req.Binding + req.logEntry.ActionConfigTitle = req.Binding.Action.Title + req.logEntry.ActionTitle = tpl.ParseTemplateOfActionBeforeExec(req.Binding.Action.Title, req.Binding.Entity) + req.logEntry.ActionIcon = req.Binding.Action.Icon + req.logEntry.Tags = req.Tags + if req.Binding.Entity != nil { + req.logEntry.EntityPrefix = req.Binding.Entity.UniqueKey + } +} + +func stepRequestActionRegisterLog(req *ExecutionRequest) { + req.executor.logmutex.Lock() + defer req.executor.logmutex.Unlock() + + if _, containsKey := req.executor.LogsByBindingId[req.Binding.ID]; !containsKey { + req.executor.LogsByBindingId[req.Binding.ID] = make([]*InternalLogEntry, 0) + } + req.executor.LogsByBindingId[req.Binding.ID] = append(req.executor.LogsByBindingId[req.Binding.ID], req.logEntry) +} + func stepLogStart(req *ExecutionRequest) bool { log.WithFields(log.Fields{ "actionTitle": req.logEntry.ActionTitle, diff --git a/service/internal/executor/executor_actions.go b/service/internal/executor/executor_actions.go index fdf504c..c55f114 100644 --- a/service/internal/executor/executor_actions.go +++ b/service/internal/executor/executor_actions.go @@ -41,7 +41,41 @@ type RebuildActionMapRequest struct { DashboardActionTitles []string } +func validateArgumentDefaults(cfg *config.Config) { + if cfg == nil { + return + } + for _, action := range cfg.Actions { + validateActionArgumentDefaults(action) + } +} + +func validateActionArgumentDefaults(action *config.Action) { + if action == nil { + return + } + for i := range action.Arguments { + validateArgumentDefault(action, &action.Arguments[i]) + } +} + +func validateArgumentDefault(action *config.Action, arg *config.ActionArgument) { + if arg.Default == "" { + return + } + if err := ValidateArgument(arg, arg.Default, action); err != nil { + log.WithFields(log.Fields{ + "actionTitle": action.Title, + "argName": arg.Name, + "default": arg.Default, + "error": err, + }).Warn("Argument default value failed validation") + } +} + func (e *Executor) RebuildActionMap() { + validateArgumentDefaults(e.Cfg) + e.MapActionBindingsLock.Lock() clear(e.MapActionBindings)