Fixing two related bugs in HasChange and GetChange
This was actually quite nasty as the first bug covered the second one… The first bug is with HasChange. This function uses reflect.DeepEqual to check if two instances are the same/have the same content. This works fine for all types except for Set’s as they contain a function. And reflect.DeepEqual will only say the functions are equal if they are both nil (which they aren’t in a Set). So in effect it means that currently HasChange will always say true for Set’s, even when they are actually being equal. As soon as you fix this problem, you will notice the second one (which the added test is written for). Without saying you want the exact diff, you will end up with a merged value which will (in most cases) be the same. Run all unit tests and a good part of the acc tests to verify this works as expected and all look good.
This commit is contained in:
parent
49b3afe452
commit
c7550595a3
|
@ -79,7 +79,7 @@ func (d *ResourceData) Get(key string) interface{} {
|
||||||
// set and the new value is. This is common, for example, for boolean
|
// set and the new value is. This is common, for example, for boolean
|
||||||
// fields which have a zero value of false.
|
// fields which have a zero value of false.
|
||||||
func (d *ResourceData) GetChange(key string) (interface{}, interface{}) {
|
func (d *ResourceData) GetChange(key string) (interface{}, interface{}) {
|
||||||
o, n := d.getChange(key, getSourceConfig, getSourceDiff)
|
o, n := d.getChange(key, getSourceState, getSourceDiff|getSourceExact)
|
||||||
return o.Value, n.Value
|
return o.Value, n.Value
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -105,6 +105,14 @@ func (d *ResourceData) getRaw(key string, level getSource) getResult {
|
||||||
// HasChange returns whether or not the given key has been changed.
|
// HasChange returns whether or not the given key has been changed.
|
||||||
func (d *ResourceData) HasChange(key string) bool {
|
func (d *ResourceData) HasChange(key string) bool {
|
||||||
o, n := d.GetChange(key)
|
o, n := d.GetChange(key)
|
||||||
|
|
||||||
|
// There is a special case needed for *schema.Set's as they contain
|
||||||
|
// a function and reflect.DeepEqual will only say two functions are
|
||||||
|
// equal when they are both nil (which in this case they are not).
|
||||||
|
if reflect.TypeOf(o).String() == "*schema.Set" {
|
||||||
|
return !reflect.DeepEqual(o.(*Set).m, n.(*Set).m)
|
||||||
|
}
|
||||||
|
|
||||||
return !reflect.DeepEqual(o, n)
|
return !reflect.DeepEqual(o, n)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -1000,6 +1000,37 @@ func TestResourceDataHasChange(t *testing.T) {
|
||||||
|
|
||||||
Change: true,
|
Change: true,
|
||||||
},
|
},
|
||||||
|
|
||||||
|
{
|
||||||
|
Schema: map[string]*Schema{
|
||||||
|
"ports": &Schema{
|
||||||
|
Type: TypeSet,
|
||||||
|
Optional: true,
|
||||||
|
Elem: &Schema{Type: TypeInt},
|
||||||
|
Set: func(a interface{}) int { return a.(int) },
|
||||||
|
},
|
||||||
|
},
|
||||||
|
|
||||||
|
State: &terraform.InstanceState{
|
||||||
|
Attributes: map[string]string{
|
||||||
|
"ports.#": "1",
|
||||||
|
"ports.80": "80",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
|
||||||
|
Diff: &terraform.InstanceDiff{
|
||||||
|
Attributes: map[string]*terraform.ResourceAttrDiff{
|
||||||
|
"ports.#": &terraform.ResourceAttrDiff{
|
||||||
|
Old: "1",
|
||||||
|
New: "0",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
|
||||||
|
Key: "ports",
|
||||||
|
|
||||||
|
Change: true,
|
||||||
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
for i, tc := range cases {
|
for i, tc := range cases {
|
||||||
|
|
Loading…
Reference in New Issue