Skip to content

Commit

Permalink
fix: load each context only once (#546)
Browse files Browse the repository at this point in the history
* fix: load each context only once

Ensure each context is only loaded once, and `*` section is loaded only
during the initial loading of the context.

* fix: reduce cognitive complexity
  • Loading branch information
Charles546 committed Jun 15, 2023
1 parent 2df21fc commit 1060096
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 20 deletions.
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ require (
github.com/Masterminds/sprig/v3 v3.2.3
github.com/go-redis/redismock/v8 v8.0.6
github.com/golang-jwt/jwt/v5 v5.0.0
golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1
)

require (
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,8 @@ golang.org/x/exp v0.0.0-20191227195350-da58074b4299/go.mod h1:2RIsYlXP63K8oxa1u0
golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u096TMicItID8zy7Y6sNkU49FU4=
golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM=
golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU=
golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1 h1:k/i9J1pBpvlfR+9QsetwPyERsqu1GIbi967PQMq3Ivc=
golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1/go.mod h1:V1LtkGg67GoY2N1AnLN78QLrzxkLyJw7RJb1gzOOz9w=
golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js=
golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0=
golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=
Expand Down
52 changes: 32 additions & 20 deletions internal/workflow/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (

"github.com/honeydipper/honeydipper/internal/config"
"github.com/honeydipper/honeydipper/pkg/dipper"
"golang.org/x/exp/slices"
)

// ErrWorkflowError is the base error for all workflow related errors.
Expand Down Expand Up @@ -153,7 +154,7 @@ func (w *Session) injectMsg(msg *dipper.Message) {
}

// injectNamedCTX inject a named context into the workflow.
func (w *Session) injectNamedCTX(name string, msg *dipper.Message) {
func (w *Session) injectNamedCTX(name string, msg *dipper.Message, firstTime bool) {
contexts := w.store.Helper.GetConfig().DataSet.Contexts

namedCTXs, ok := contexts[name]
Expand All @@ -167,7 +168,7 @@ func (w *Session) injectNamedCTX(name string, msg *dipper.Message) {

envData := w.buildEnvData(msg)
ctx, ok := namedCTXs.(map[string]interface{})["*"]
if ok {
if firstTime && ok {
ctx = dipper.MustDeepCopyMap(ctx.(map[string]interface{}))
ctx = dipper.Interpolate(ctx, envData)
w.ctx = dipper.MergeMap(w.ctx, ctx)
Expand Down Expand Up @@ -197,18 +198,28 @@ func (w *Session) injectNamedCTX(name string, msg *dipper.Message) {

// initCTX initialize the contextual data used in this workflow.
func (w *Session) initCTX(msg *dipper.Message) {
w.injectNamedCTX(SessionContextDefault, msg)
w.injectNamedCTX(SessionContextDefault, msg, w.parent == "")
if w.parent == "" {
w.injectNamedCTX(SessionContextEvents, msg)
w.injectNamedCTX(SessionContextEvents, msg, true)
}

for _, name := range w.loadedContexts {
if name == SessionContextHooks {
w.isHook = true
}
w.injectNamedCTX(name, msg)
w.injectNamedCTX(name, msg, false)
}

w.injectCTXs(msg)

if w.isHook {
// avoid hook in hook
delete(w.ctx, "hooks")
}
}

// injectCTXs loads the contexts specified through context or contexts fields.
func (w *Session) injectCTXs(msg *dipper.Message) {
envdata := w.buildEnvData(msg)
w.workflow.Context = dipper.InterpolateStr(w.workflow.Context, envdata)
w.workflow.Contexts = dipper.Interpolate(w.workflow.Contexts, envdata)
Expand All @@ -217,8 +228,11 @@ func (w *Session) initCTX(msg *dipper.Message) {
if w.workflow.Context == SessionContextHooks {
w.isHook = true
}
w.injectNamedCTX(w.workflow.Context, msg)
w.loadedContexts = append(w.loadedContexts, w.workflow.Context)

if !slices.Contains(w.loadedContexts, w.workflow.Context) {
w.injectNamedCTX(w.workflow.Context, msg, true)
w.loadedContexts = append(w.loadedContexts, w.workflow.Context)
}
}

if w.workflow.Contexts != nil {
Expand All @@ -230,22 +244,20 @@ func (w *Session) initCTX(msg *dipper.Message) {
if !ok {
panic(fmt.Errorf("%w: expected list of strings in contexts in workflow: %s", ErrWorkflowError, w.workflow.Name))
}
if name != "" {
// at this stage the hooks flag is added only through `context` not `contexts`
// this part of the code is unreachable
// if name == SessionContextHooks {
// w.isHook = true
// }
w.injectNamedCTX(name, msg)
if name == "" {
continue
}
// at this stage the hooks flag is added only through `context` not `contexts`
// this part of the code is unreachable
// if name == SessionContextHooks {
// w.isHook = true
// }
if !slices.Contains(w.loadedContexts, name) {
w.injectNamedCTX(name, msg, true)
w.loadedContexts = append(w.loadedContexts, name)
}
}
}

if w.isHook {
// avoid hook in hook
delete(w.ctx, "hooks")
}
}

// injectMeta injects the meta info into context.
Expand Down Expand Up @@ -399,7 +411,7 @@ func (w *Session) inheritParentData(parent *Session) {

w.event = parent.event
w.ctx = dipper.MustDeepCopyMap(parent.ctx)
w.loadedContexts = append([]string{}, parent.loadedContexts...)
w.loadedContexts = parent.loadedContexts

delete(w.ctx, "hooks") // hooks don't get inherited
}
Expand Down

0 comments on commit 1060096

Please sign in to comment.