Skip to content

Commit

Permalink
Don't deepcopy AST when generating vars (#6058)
Browse files Browse the repository at this point in the history
(cherry picked from commit db6fbe2)

# Conflicts:
#	internal/pkg/composable/controller.go
  • Loading branch information
swiatekm authored and mergify[bot] committed Nov 21, 2024
1 parent b388809 commit 7f7a76f
Show file tree
Hide file tree
Showing 4 changed files with 175 additions and 0 deletions.
69 changes: 69 additions & 0 deletions internal/pkg/agent/transpiler/ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ type Node interface {
//Close clones the current node.
Clone() Node

// ShallowClone makes a shallow clone of the node.
ShallowClone() Node

// Hash compute a sha256 hash of the current node and recursively call any children.
Hash() []byte

Expand Down Expand Up @@ -137,6 +140,19 @@ func (d *Dict) Clone() Node {
return &Dict{value: nodes}
}

// ShallowClone makes a shallow clone of the node.
func (d *Dict) ShallowClone() Node {
nodes := make([]Node, 0, len(d.value))
for _, i := range d.value {
if i == nil {
continue
}
nodes = append(nodes, i)

}
return &Dict{value: nodes}
}

// Hash compute a sha256 hash of the current node and recursively call any children.
func (d *Dict) Hash() []byte {
h := sha256.New()
Expand Down Expand Up @@ -246,6 +262,11 @@ func (k *Key) Clone() Node {
return &Key{name: k.name, value: nil}
}

// ShallowClone makes a shallow clone of the node.
func (k *Key) ShallowClone() Node {
return &Key{name: k.name, value: k.value}
}

// Hash compute a sha256 hash of the current node and recursively call any children.
func (k *Key) Hash() []byte {
h := sha256.New()
Expand Down Expand Up @@ -364,6 +385,18 @@ func (l *List) Clone() Node {
return &List{value: nodes}
}

// ShallowClone makes a shallow clone of the node.
func (l *List) ShallowClone() Node {
nodes := make([]Node, 0, len(l.value))
for _, i := range l.value {
if i == nil {
continue
}
nodes = append(nodes, i)
}
return &List{value: nodes}
}

// Apply applies the vars to all nodes in the list.
func (l *List) Apply(vars *Vars) (Node, error) {
nodes := make([]Node, 0, len(l.value))
Expand Down Expand Up @@ -429,6 +462,11 @@ func (s *StrVal) Clone() Node {
return &k
}

// ShallowClone makes a shallow clone of the node.
func (s *StrVal) ShallowClone() Node {
return s.Clone()
}

// Hash we return the byte slice of the string.
func (s *StrVal) Hash() []byte {
return []byte(s.value)
Expand Down Expand Up @@ -480,6 +518,11 @@ func (s *IntVal) Clone() Node {
return &k
}

// ShallowClone makes a shallow clone of the node.
func (s *IntVal) ShallowClone() Node {
return s.Clone()
}

// Apply does nothing.
func (s *IntVal) Apply(_ *Vars) (Node, error) {
return s, nil
Expand Down Expand Up @@ -531,6 +574,11 @@ func (s *UIntVal) Clone() Node {
return &k
}

// ShallowClone makes a shallow clone of the node.
func (s *UIntVal) ShallowClone() Node {
return s.Clone()
}

// Hash we convert the value into a string and return the byte slice.
func (s *UIntVal) Hash() []byte {
return []byte(s.String())
Expand Down Expand Up @@ -583,6 +631,11 @@ func (s *FloatVal) Clone() Node {
return &k
}

// ShallowClone makes a shallow clone of the node.
func (s *FloatVal) ShallowClone() Node {
return s.Clone()
}

// Hash return a string representation of the value, we try to return the minimal precision we can.
func (s *FloatVal) Hash() []byte {
return []byte(strconv.FormatFloat(s.value, 'f', -1, 64))
Expand Down Expand Up @@ -637,6 +690,11 @@ func (s *BoolVal) Clone() Node {
return &k
}

// ShallowClone makes a shallow clone of the node.
func (s *BoolVal) ShallowClone() Node {
return s.Clone()
}

// Hash returns a single byte to represent the boolean value.
func (s *BoolVal) Hash() []byte {
if s.value {
Expand Down Expand Up @@ -750,6 +808,11 @@ func (a *AST) Clone() *AST {
return &AST{root: a.root.Clone()}
}

// ShallowClone makes a shallow clone of the node.
func (a *AST) ShallowClone() *AST {
return &AST{root: a.root.ShallowClone()}
}

// Hash calculates a hash from all the included nodes in the tree.
func (a *AST) Hash() []byte {
return a.root.Hash()
Expand Down Expand Up @@ -929,6 +992,12 @@ func Lookup(a *AST, selector Selector) (Node, bool) {
return current, true
}

// Insert inserts an AST into an existing AST, will return and error if the target position cannot
// accept a new node.
func (a *AST) Insert(b *AST, to Selector) error {
return Insert(a, b.root, to)
}

// Insert inserts a node into an existing AST, will return and error if the target position cannot
// accept a new node.
func Insert(a *AST, node Node, to Selector) error {
Expand Down
68 changes: 68 additions & 0 deletions internal/pkg/agent/transpiler/ast_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -838,6 +838,74 @@ func TestHash(t *testing.T) {
}
}

func TestShallowClone(t *testing.T) {
tests := map[string]struct {
input *AST
}{
"dict": {
input: &AST{
root: &Dict{
value: []Node{
&Key{name: "integer", value: &IntVal{value: 1}},
&Key{name: "float", value: &FloatVal{value: 1.1234}},
&Key{name: "bool1", value: &BoolVal{value: true}},
},
},
},
},
"list": {
input: &AST{
root: &List{
value: []Node{
&IntVal{value: 1},
&FloatVal{value: 1.1234},
&BoolVal{value: true},
},
},
},
},
"key": {
input: &AST{
root: &Key{name: "integer", value: &IntVal{value: 1}},
},
},
"str": {
input: &AST{
root: &StrVal{value: "value"},
},
},
"bool": {
input: &AST{
root: &BoolVal{value: true},
},
},
"integer": {
input: &AST{
root: &IntVal{value: 1},
},
},
"float": {
input: &AST{
root: &FloatVal{value: 1.1234},
},
},
}

for name, test := range tests {
t.Run(name, func(t *testing.T) {
cloned := test.input.ShallowClone()
assert.Equal(t, test.input, cloned)
err := test.input.Insert(&AST{root: &BoolVal{value: true}}, "key")
if err == nil {
assert.NotEqual(t, test.input, cloned)
} else if list, ok := test.input.root.(*List); ok {
list.value = append(list.value, &IntVal{value: 7})
assert.NotEqual(t, test.input, cloned)
}
})
}
}

func mustMakeVars(mapping map[string]interface{}) *Vars {
v, err := NewVars("", mapping, nil)
if err != nil {
Expand Down
10 changes: 10 additions & 0 deletions internal/pkg/agent/transpiler/vars.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ func NewVars(id string, mapping map[string]interface{}, fetchContextProviders ma
return NewVarsWithProcessors(id, mapping, "", nil, fetchContextProviders)
}

// NewVarsFromAst returns a new instance of vars. It takes the mapping as an *AST.
func NewVarsFromAst(id string, tree *AST, fetchContextProviders mapstr.M) *Vars {
return &Vars{id, tree, "", nil, fetchContextProviders}
}

// NewVarsWithProcessors returns a new instance of vars with attachment of processors.
func NewVarsWithProcessors(id string, mapping map[string]interface{}, processorKey string, processors Processors, fetchContextProviders mapstr.M) (*Vars, error) {
tree, err := NewAST(mapping)
Expand All @@ -42,6 +47,11 @@ func NewVarsWithProcessors(id string, mapping map[string]interface{}, processorK
return &Vars{id, tree, processorKey, processors, fetchContextProviders}, nil
}

// NewVarsWithProcessorsFromAst returns a new instance of vars with attachment of processors. It takes the mapping as an *AST.
func NewVarsWithProcessorsFromAst(id string, tree *AST, processorKey string, processors Processors, fetchContextProviders mapstr.M) *Vars {
return &Vars{id, tree, processorKey, processors, fetchContextProviders}
}

// Replace returns a new value based on variable replacement.
func (v *Vars) Replace(value string) (Node, error) {
var processors Processors
Expand Down
28 changes: 28 additions & 0 deletions internal/pkg/composable/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,34 @@ func (c *controller) Close() {
}
}

<<<<<<< HEAD
=======
func (c *controller) generateVars(fetchContextProviders mapstr.M) []*transpiler.Vars {
// build the vars list of mappings
vars := make([]*transpiler.Vars, 1)
mapping := map[string]interface{}{}
for name, state := range c.contextProviders {
mapping[name] = state.Current()
}
// this is ensured not to error, by how the mappings states are verified
mappingAst, _ := transpiler.NewAST(mapping)
vars[0] = transpiler.NewVarsFromAst("", mappingAst, fetchContextProviders)

// add to the vars list for each dynamic providers mappings
for name, state := range c.dynamicProviders {
for _, mappings := range state.Mappings() {
local := mappingAst.ShallowClone()
dynamicAst, _ := transpiler.NewAST(mappings.mapping)
_ = local.Insert(dynamicAst, name)
id := fmt.Sprintf("%s-%s", name, mappings.id)
v := transpiler.NewVarsWithProcessorsFromAst(id, local, name, mappings.processors, fetchContextProviders)
vars = append(vars, v)
}
}
return vars
}

>>>>>>> db6fbe2429 (Don't deepcopy AST when generating vars (#6058))
type contextProviderState struct {
context.Context

Expand Down

0 comments on commit 7f7a76f

Please sign in to comment.