Skip to content

Commit

Permalink
objchange: fix ProposedNew from null objects
Browse files Browse the repository at this point in the history
The codepath for AllAttributesNull was not correct for any nested object
types with collections, and should create single null values for the
correct NestingMode rather than a single object with null attributes.

Since there is no reason to descend into nested object types to create
nullv alues, we can drop the AllAttributesNull function altogether and
create null values as needed during ProposedNew.

The corresponding AllBlockAttributesNull was only called internally in 1
location, and simply delegated to schema.EmptyValue. We can reduce the
package surface area by dropping that function too and calling
EmptyValue directly.
  • Loading branch information
jbardin committed Oct 4, 2021
1 parent 0062e71 commit 18d3542
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 38 deletions.
33 changes: 0 additions & 33 deletions internal/plans/objchange/all_null.go

This file was deleted.

16 changes: 11 additions & 5 deletions internal/plans/objchange/objchange.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@ func ProposedNew(schema *configschema.Block, prior, config cty.Value) cty.Value
// similar to the result of decoding an empty configuration block,
// which simplifies our handling of the top-level attributes/blocks
// below by giving us one non-null level of object to pull values from.
prior = AllBlockAttributesNull(schema)
//
// "All attributes null" happens to be the definition of EmptyValue for
// a Block, so we can just delegate to that
prior = schema.EmptyValue()
}
return proposedNew(schema, prior, config)
}
Expand Down Expand Up @@ -258,12 +261,15 @@ func proposedNewNestedBlock(schema *configschema.NestedBlock, prior, config cty.
}

func proposedNewAttributes(attrs map[string]*configschema.Attribute, prior, config cty.Value) map[string]cty.Value {
if prior.IsNull() {
prior = AllAttributesNull(attrs)
}
newAttrs := make(map[string]cty.Value, len(attrs))
for name, attr := range attrs {
priorV := prior.GetAttr(name)
var priorV cty.Value
if prior.IsNull() {
priorV = cty.NullVal(prior.Type().AttributeType(name))
} else {
priorV = prior.GetAttr(name)
}

configV := config.GetAttr(name)
var newV cty.Value
switch {
Expand Down
104 changes: 104 additions & 0 deletions internal/plans/objchange/objchange_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1536,6 +1536,110 @@ func TestProposedNew(t *testing.T) {
}),
}),
},
"prior null nested objects": {
&configschema.Block{
Attributes: map[string]*configschema.Attribute{
"single": {
NestedType: &configschema.Object{
Nesting: configschema.NestingSingle,
Attributes: map[string]*configschema.Attribute{
"list": {
NestedType: &configschema.Object{
Nesting: configschema.NestingList,
Attributes: map[string]*configschema.Attribute{
"foo": {
Type: cty.String,
},
},
},
Optional: true,
},
},
},
Optional: true,
},
"map": {
NestedType: &configschema.Object{
Nesting: configschema.NestingMap,
Attributes: map[string]*configschema.Attribute{
"map": {
NestedType: &configschema.Object{
Nesting: configschema.NestingList,
Attributes: map[string]*configschema.Attribute{
"foo": {
Type: cty.String,
},
},
},
Optional: true,
},
},
},
Optional: true,
},
},
},
cty.NullVal(cty.Object(map[string]cty.Type{
"single": cty.Object(map[string]cty.Type{
"list": cty.List(cty.Object(map[string]cty.Type{
"foo": cty.String,
})),
}),
"map": cty.Map(cty.Object(map[string]cty.Type{
"list": cty.List(cty.Object(map[string]cty.Type{
"foo": cty.String,
})),
})),
})),
cty.ObjectVal(map[string]cty.Value{
"single": cty.ObjectVal(map[string]cty.Value{
"list": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"foo": cty.StringVal("a"),
}),
cty.ObjectVal(map[string]cty.Value{
"foo": cty.StringVal("b"),
}),
}),
}),
"map": cty.MapVal(map[string]cty.Value{
"one": cty.ObjectVal(map[string]cty.Value{
"list": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"foo": cty.StringVal("a"),
}),
cty.ObjectVal(map[string]cty.Value{
"foo": cty.StringVal("b"),
}),
}),
}),
}),
}),
cty.ObjectVal(map[string]cty.Value{
"single": cty.ObjectVal(map[string]cty.Value{
"list": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"foo": cty.StringVal("a"),
}),
cty.ObjectVal(map[string]cty.Value{
"foo": cty.StringVal("b"),
}),
}),
}),
"map": cty.MapVal(map[string]cty.Value{
"one": cty.ObjectVal(map[string]cty.Value{
"list": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"foo": cty.StringVal("a"),
}),
cty.ObjectVal(map[string]cty.Value{
"foo": cty.StringVal("b"),
}),
}),
}),
}),
}),
},
}

for name, test := range tests {
Expand Down

0 comments on commit 18d3542

Please sign in to comment.