From 4bac315568aa8de209f1c68b1f3567c505f49563 Mon Sep 17 00:00:00 2001 From: James Read Date: Mon, 27 May 2024 23:52:06 +0100 Subject: [PATCH] feature: The executor now controls the public action map - meaning actions work without the API, and pages load faster (and much more!) (#324) * feature: The executor now controls the public action map * bugfix: Build the action map if there are no entities! * bugfix: Sort by title if the actions have the same config order --- OliveTin.proto | 1 + cmd/OliveTin/main.go | 6 +- internal/executor/executor.go | 16 +++- internal/executor/executor_actions.go | 89 +++++++++++++++++++++ internal/executor/executor_test.go | 4 +- internal/grpcapi/grpcApi.go | 18 +++-- internal/grpcapi/grpcApiActions.go | 108 ++++++-------------------- internal/grpcapi/grpcApi_test.go | 11 ++- internal/websocket/websocket.go | 14 ++-- 9 files changed, 163 insertions(+), 104 deletions(-) create mode 100644 internal/executor/executor_actions.go diff --git a/OliveTin.proto b/OliveTin.proto index be4e4e9..121dc71 100644 --- a/OliveTin.proto +++ b/OliveTin.proto @@ -12,6 +12,7 @@ message Action { bool can_exec = 4; repeated ActionArgument arguments = 5; string popup_on_start = 6; + int32 order = 7; } message ActionArgument { diff --git a/cmd/OliveTin/main.go b/cmd/OliveTin/main.go index e590f95..838f0e2 100644 --- a/cmd/OliveTin/main.go +++ b/cmd/OliveTin/main.go @@ -135,9 +135,10 @@ func main() { log.Debugf("Config: %+v", cfg) - executor := executor.DefaultExecutor() + executor := executor.DefaultExecutor(cfg) + executor.RebuildActionMap() executor.AddListener(websocket.ExecutionListener) - config.AddListener(websocket.OnConfigChanged) + config.AddListener(executor.RebuildActionMap) go onstartup.Execute(cfg, executor) go oncron.Schedule(cfg, executor) @@ -145,6 +146,7 @@ func main() { go oncalendarfile.Schedule(cfg, executor) entityfiles.AddListener(websocket.OnEntityChanged) + entityfiles.AddListener(executor.RebuildActionMap) go entityfiles.SetupEntityFileWatchers(cfg) go updatecheck.StartUpdateChecker(version, commit, cfg, cfg.GetDir()) diff --git a/internal/executor/executor.go b/internal/executor/executor.go index fc1e73d..2f572c9 100644 --- a/internal/executor/executor.go +++ b/internal/executor/executor.go @@ -31,12 +31,23 @@ var ( }) ) +type ActionBinding struct { + Action *config.Action + EntityPrefix string + ConfigOrder int +} + // Executor represents a helper class for executing commands. It's main method // is ExecRequest type Executor struct { Logs map[string]*InternalLogEntry LogsByActionId map[string][]*InternalLogEntry + MapActionIdToBinding map[string]*ActionBinding + MapActionIdToBindingLock sync.RWMutex + + Cfg *config.Config + listeners []listener chainOfCommand []executorStepFunc @@ -92,10 +103,12 @@ type executorStepFunc func(*ExecutionRequest) bool // DefaultExecutor returns an Executor, with a sensible "chain of command" for // executing actions. -func DefaultExecutor() *Executor { +func DefaultExecutor(cfg *config.Config) *Executor { e := Executor{} + e.Cfg = cfg e.Logs = make(map[string]*InternalLogEntry) e.LogsByActionId = make(map[string][]*InternalLogEntry) + e.MapActionIdToBinding = make(map[string]*ActionBinding) e.chainOfCommand = []executorStepFunc{ stepRequestAction, @@ -117,6 +130,7 @@ func DefaultExecutor() *Executor { type listener interface { OnExecutionStarted(actionTitle string) OnExecutionFinished(logEntry *InternalLogEntry) + OnActionMapRebuilt() } func (e *Executor) AddListener(m listener) { diff --git a/internal/executor/executor_actions.go b/internal/executor/executor_actions.go new file mode 100644 index 0000000..830e4dc --- /dev/null +++ b/internal/executor/executor_actions.go @@ -0,0 +1,89 @@ +package executor + +import ( + "crypto/sha256" + "fmt" + config "github.com/OliveTin/OliveTin/internal/config" + sv "github.com/OliveTin/OliveTin/internal/stringvariables" + log "github.com/sirupsen/logrus" + "strconv" +) + +func (e *Executor) FindActionBindingByID(id string) *config.Action { + e.MapActionIdToBindingLock.RLock() + pair, found := e.MapActionIdToBinding[id] + e.MapActionIdToBindingLock.RUnlock() + + if found { + log.Infof("findActionBinding %v, %v", id, pair.Action.ID) + return pair.Action + } + + return nil +} + +func (e *Executor) RebuildActionMap() { + e.MapActionIdToBindingLock.Lock() + + clear(e.MapActionIdToBinding) + + for configOrder, action := range e.Cfg.Actions { + if action.Entity != "" { + registerActionsFromEntities(e, configOrder, action.Entity, action) + } else { + registerAction(e, configOrder, action) + } + } + + e.MapActionIdToBindingLock.Unlock() + + for _, l := range e.listeners { + l.OnActionMapRebuilt() + } +} + +func registerAction(e *Executor, configOrder int, action *config.Action) { + actionId := hashActionToID(action, "") + + e.MapActionIdToBinding[actionId] = &ActionBinding{ + Action: action, + EntityPrefix: "noent", + ConfigOrder: configOrder, + } +} + +func registerActionsFromEntities(e *Executor, configOrder int, entityTitle string, tpl *config.Action) { + entityCount, _ := strconv.Atoi(sv.Get("entities." + entityTitle + ".count")) + + for i := 0; i < entityCount; i++ { + registerActionFromEntity(e, configOrder, tpl, entityTitle, i) + } +} + +func registerActionFromEntity(e *Executor, configOrder int, tpl *config.Action, entityTitle string, entityIndex int) { + prefix := sv.GetEntityPrefix(entityTitle, entityIndex) + + virtualActionId := hashActionToID(tpl, prefix) + + e.MapActionIdToBinding[virtualActionId] = &ActionBinding{ + Action: tpl, + EntityPrefix: prefix, + ConfigOrder: configOrder, + } +} + +func hashActionToID(action *config.Action, entityPrefix string) string { + if action.ID != "" && entityPrefix == "" { + return action.ID + } + + h := sha256.New() + + if entityPrefix == "" { + h.Write([]byte(action.Title)) + } else { + h.Write([]byte(action.ID + "." + entityPrefix)) + } + + return fmt.Sprintf("%x", h.Sum(nil)) +} diff --git a/internal/executor/executor_test.go b/internal/executor/executor_test.go index a486488..8fe955d 100644 --- a/internal/executor/executor_test.go +++ b/internal/executor/executor_test.go @@ -9,10 +9,10 @@ import ( ) func testingExecutor() (*Executor, *config.Config) { - e := DefaultExecutor() - cfg := config.DefaultConfig() + e := DefaultExecutor(cfg) + a1 := &config.Action{ Title: "Do some tickles", Shell: "echo 'Tickling {{ person }}'", diff --git a/internal/grpcapi/grpcApi.go b/internal/grpcapi/grpcApi.go index 14c30d6..d11d889 100644 --- a/internal/grpcapi/grpcApi.go +++ b/internal/grpcapi/grpcApi.go @@ -59,7 +59,9 @@ func (api *oliveTinAPI) StartAction(ctx ctx.Context, req *pb.StartActionRequest) args[arg.Name] = arg.Value } - pair := publicActionIdToActionMap[req.ActionId] + api.executor.MapActionIdToBindingLock.RLock() + pair := api.executor.MapActionIdToBinding[req.ActionId] + api.executor.MapActionIdToBindingLock.RUnlock() execReq := executor.ExecutionRequest{ Action: pair.Action, @@ -81,7 +83,7 @@ func (api *oliveTinAPI) StartActionAndWait(ctx ctx.Context, req *pb.StartActionA args := make(map[string]string) execReq := executor.ExecutionRequest{ - Action: findActionByPublicID(req.ActionId), + Action: api.executor.FindActionBindingByID(req.ActionId), TrackingID: uuid.NewString(), Arguments: args, AuthenticatedUser: acl.UserFromContext(ctx, cfg), @@ -106,7 +108,7 @@ func (api *oliveTinAPI) StartActionByGet(ctx ctx.Context, req *pb.StartActionByG args := make(map[string]string) execReq := executor.ExecutionRequest{ - Action: findActionByPublicID(req.ActionId), + Action: api.executor.FindActionBindingByID(req.ActionId), TrackingID: uuid.NewString(), Arguments: args, AuthenticatedUser: acl.UserFromContext(ctx, cfg), @@ -124,7 +126,7 @@ func (api *oliveTinAPI) StartActionByGetAndWait(ctx ctx.Context, req *pb.StartAc args := make(map[string]string) execReq := executor.ExecutionRequest{ - Action: findActionByPublicID(req.ActionId), + Action: api.executor.FindActionBindingByID(req.ActionId), TrackingID: uuid.NewString(), Arguments: args, AuthenticatedUser: acl.UserFromContext(ctx, cfg), @@ -233,7 +235,7 @@ func (api *oliveTinAPI) WatchExecution(req *pb.WatchExecutionRequest, srv pb.Oli func (api *oliveTinAPI) GetDashboardComponents(ctx ctx.Context, req *pb.GetDashboardComponentsRequest) (*pb.GetDashboardComponentsResponse, error) { user := acl.UserFromContext(ctx, cfg) - res := actionsCfgToPb(cfg.Actions, user) + res := buildDashboardResponse(api.executor, cfg, user) if len(res.Actions) == 0 { log.Warn("Zero actions found - check that you have some actions defined, with a view permission") @@ -347,13 +349,17 @@ func (api *oliveTinAPI) DumpPublicIdActionMap(ctx ctx.Context, req *pb.DumpPubli return res, nil } - for k, v := range publicActionIdToActionMap { + api.executor.MapActionIdToBindingLock.RLock() + + for k, v := range api.executor.MapActionIdToBinding { res.Contents[k] = &pb.ActionEntityPair{ ActionTitle: v.Action.Title, EntityPrefix: v.EntityPrefix, } } + api.executor.MapActionIdToBindingLock.RUnlock() + res.Alert = "Dumping variables has been enabled in the configuration. Please set InsecureAllowDumpActionMap = false again after you don't need it anymore" return res, nil diff --git a/internal/grpcapi/grpcApiActions.go b/internal/grpcapi/grpcApiActions.go index 9d7eecc..2ccbef2 100644 --- a/internal/grpcapi/grpcApiActions.go +++ b/internal/grpcapi/grpcApiActions.go @@ -1,90 +1,50 @@ package grpcapi import ( - "crypto/sha256" - "fmt" pb "github.com/OliveTin/OliveTin/gen/grpc" acl "github.com/OliveTin/OliveTin/internal/acl" config "github.com/OliveTin/OliveTin/internal/config" + executor "github.com/OliveTin/OliveTin/internal/executor" sv "github.com/OliveTin/OliveTin/internal/stringvariables" - log "github.com/sirupsen/logrus" - "strconv" + "sort" ) -type ActionWithEntity struct { - Action *config.Action - EntityPrefix string -} - -var publicActionIdToActionMap map[string]ActionWithEntity - -func init() { - publicActionIdToActionMap = make(map[string]ActionWithEntity) -} - -func actionsCfgToPb(cfgActions []*config.Action, user *acl.AuthenticatedUser) *pb.GetDashboardComponentsResponse { +func buildDashboardResponse(ex *executor.Executor, cfg *config.Config, user *acl.AuthenticatedUser) *pb.GetDashboardComponentsResponse { res := &pb.GetDashboardComponentsResponse{} - for _, action := range cfgActions { - if !acl.IsAllowedView(cfg, user, action) { + ex.MapActionIdToBindingLock.RLock() + + for actionId, actionBinding := range ex.MapActionIdToBinding { + if !acl.IsAllowedView(cfg, user, actionBinding.Action) { continue } - if action.Entity != "" { - res.Actions = append(res.Actions, buildActionEntities(action.Entity, action)...) - } else { - btn := actionCfgToPb(action, user) - res.Actions = append(res.Actions, btn) - } + res.Actions = append(res.Actions, buildAction(actionId, actionBinding, user)) } + ex.MapActionIdToBindingLock.RUnlock() + + sort.Slice(res.Actions, func(i, j int) bool { + if res.Actions[i].Order == res.Actions[j].Order { + return res.Actions[i].Title < res.Actions[j].Title + } else { + return res.Actions[i].Order < res.Actions[j].Order + } + }) + return res } -func buildActionEntities(entityTitle string, tpl *config.Action) []*pb.Action { - ret := make([]*pb.Action, 0) - - entityCount, _ := strconv.Atoi(sv.Get("entities." + entityTitle + ".count")) - - for i := 0; i < entityCount; i++ { - ret = append(ret, buildEntityAction(tpl, entityTitle, i)) - } - - return ret -} - -func buildEntityAction(tpl *config.Action, entityTitle string, entityIndex int) *pb.Action { - prefix := sv.GetEntityPrefix(entityTitle, entityIndex) - - virtualActionId := createPublicID(tpl, prefix) - - publicActionIdToActionMap[virtualActionId] = ActionWithEntity{ - Action: tpl, - EntityPrefix: prefix, - } - - return &pb.Action{ - Id: virtualActionId, - Title: sv.ReplaceEntityVars(prefix, tpl.Title), - Icon: tpl.Icon, - PopupOnStart: tpl.PopupOnStart, - } -} - -func actionCfgToPb(action *config.Action, user *acl.AuthenticatedUser) *pb.Action { - virtualActionId := createPublicID(action, "") - - publicActionIdToActionMap[virtualActionId] = ActionWithEntity{ - Action: action, - EntityPrefix: "noent", - } +func buildAction(actionId string, actionBinding *executor.ActionBinding, user *acl.AuthenticatedUser) *pb.Action { + action := actionBinding.Action btn := pb.Action{ - Id: virtualActionId, - Title: action.Title, + Id: actionId, + Title: sv.ReplaceEntityVars(actionBinding.EntityPrefix, action.Title), Icon: action.Icon, CanExec: acl.IsAllowedExec(cfg, user, action), PopupOnStart: action.PopupOnStart, + Order: int32(actionBinding.ConfigOrder), } for _, cfgArg := range action.Arguments { @@ -143,25 +103,3 @@ func buildChoicesSimple(choices []config.ActionArgumentChoice) []*pb.ActionArgum return ret } - -func createPublicID(action *config.Action, entityPrefix string) string { - if action.Entity == "" { - return action.ID - } else { - h := sha256.New() - h.Write([]byte(action.ID + "." + entityPrefix)) - - return fmt.Sprintf("%x", h.Sum(nil)) - } -} - -func findActionByPublicID(id string) *config.Action { - pair, found := publicActionIdToActionMap[id] - - if found { - log.Infof("findPublic %v, %v", id, pair.Action.ID) - return pair.Action - } - - return nil -} diff --git a/internal/grpcapi/grpcApi_test.go b/internal/grpcapi/grpcApi_test.go index e85801a..b3504b8 100644 --- a/internal/grpcapi/grpcApi_test.go +++ b/internal/grpcapi/grpcApi_test.go @@ -21,8 +21,8 @@ const bufSize = 1024 * 1024 var lis *bufconn.Listener -func init() { - ex := executor.DefaultExecutor() +func initServer(cfg *config.Config) *executor.Executor { + ex := executor.DefaultExecutor(cfg) lis = bufconn.Listen(bufSize) s := grpc.NewServer() @@ -33,6 +33,8 @@ func init() { log.Fatalf("Server exited with error: %v", err) } }() + + return ex } func bufDialer(context.Context, string) (net.Conn, error) { @@ -57,12 +59,17 @@ func getNewTestServerAndClient(t *testing.T, injectedConfig *config.Config) (*gr func TestGetActionsAndStart(t *testing.T) { cfg = config.DefaultConfig() + + ex := initServer(cfg) + btn1 := &config.Action{} btn1.Title = "blat" btn1.ID = "blat" btn1.Shell = "echo 'test'" cfg.Actions = append(cfg.Actions, btn1) + ex.RebuildActionMap() + conn, client := getNewTestServerAndClient(t, cfg) respGb, err := client.GetDashboardComponents(context.Background(), &pb.GetDashboardComponentsRequest{}) diff --git a/internal/websocket/websocket.go b/internal/websocket/websocket.go index 461bff3..243d64a 100644 --- a/internal/websocket/websocket.go +++ b/internal/websocket/websocket.go @@ -30,12 +30,6 @@ var marshalOptions = protojson.MarshalOptions{ EmitUnpopulated: true, } -func OnConfigChanged() { - evt := &pb.EventConfigChanged{} - - broadcast(evt) -} - var ExecutionListener WebsocketExecutionListener type WebsocketExecutionListener struct{} @@ -53,6 +47,10 @@ func OnEntityChanged() { broadcast(&pb.EventEntityChanged{}) } +func (WebsocketExecutionListener) OnActionMapRebuilt() { + broadcast(&pb.EventConfigChanged{}) +} + /* The default checkOrigin function checks that the origin (browser) matches the request origin. However in OliveTin we expect many users to deliberately proxy @@ -156,8 +154,12 @@ func HandleWebsocket(w http.ResponseWriter, r *http.Request) bool { conn: c, } + sendmutex.Lock() + clients = append(clients, wsclient) + sendmutex.Unlock() + go wsclient.messageLoop() return true