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

@@ -12,6 +12,7 @@ message Action {
 	bool can_exec = 4;
 	repeated ActionArgument arguments = 5;
 	string popup_on_start = 6;
+	int32 order = 7;
 }
 
 message ActionArgument {

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())

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) {

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))
+}

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 }}'",

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

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"
+	"sort"
-	"strconv"
 )
 
-type ActionWithEntity struct {
+func buildDashboardResponse(ex *executor.Executor, cfg *config.Config, user *acl.AuthenticatedUser) *pb.GetDashboardComponentsResponse {
-	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 {
 	res := &pb.GetDashboardComponentsResponse{}
 
-	for _, action := range cfgActions {
+	ex.MapActionIdToBindingLock.RLock()
-		if !acl.IsAllowedView(cfg, user, action) {
+
+	for actionId, actionBinding := range ex.MapActionIdToBinding {
+		if !acl.IsAllowedView(cfg, user, actionBinding.Action) {
 			continue
 		}
 
-		if action.Entity != "" {
+		res.Actions = append(res.Actions, buildAction(actionId, actionBinding, user))
-			res.Actions = append(res.Actions, buildActionEntities(action.Entity, action)...)
+	}
+
+	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 {
-			btn := actionCfgToPb(action, user)
+			return res.Actions[i].Order < res.Actions[j].Order
-			res.Actions = append(res.Actions, btn)
-		}
 		}
+	})
 
 	return res
 }
 
-func buildActionEntities(entityTitle string, tpl *config.Action) []*pb.Action {
+func buildAction(actionId string, actionBinding *executor.ActionBinding, user *acl.AuthenticatedUser) *pb.Action {
-	ret := make([]*pb.Action, 0)
+	action := actionBinding.Action
-
-	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",
-	}
 
 	btn := pb.Action{
-		Id:           virtualActionId,
+		Id:           actionId,
-		Title:        action.Title,
+		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
-}

internal/grpcapi/grpcApi_test.go

@@ -21,8 +21,8 @@ const bufSize = 1024 * 1024
 
 var lis *bufconn.Listener
 
-func init() {
+func initServer(cfg *config.Config) *executor.Executor {
-	ex := executor.DefaultExecutor()
+	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{})

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