Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core: Add GetOkExists schema function #15723

Merged
merged 4 commits into from
Aug 3, 2017
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions helper/schema/resource_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,22 @@ func (d *ResourceData) GetOk(key string) (interface{}, bool) {
return r.Value, exists
}

// GetOkExists returns the data for a given key and whether or not the key
// has been set to a non-zero value. This is only useful for determining
// if boolean attributes have been set, if they are Optional but do not
// have a Default value.
//
// This is nearly the same function as GetOk, yet it does not check
// for the zero value of the attribute's type. This allows for attributes
// without a default, to fully check for a literal assignment, regardless
// of the zero-value for that type.
// This should only be used if absolutely required/needed.
func (d *ResourceData) GetOkExists(key string) (interface{}, bool) {
r := d.getRaw(key, getSourceSet)
exists := r.Exists && !r.Computed
return r.Value, exists
}

func (d *ResourceData) getRaw(key string, level getSource) getResult {
var parts []string
if key != "" {
Expand Down
243 changes: 243 additions & 0 deletions helper/schema/resource_data_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1082,6 +1082,249 @@ func TestResourceDataGetOk(t *testing.T) {
}
}

func TestResourceDataGetOkExists(t *testing.T) {
cases := []struct {
Name string
Schema map[string]*Schema
State *terraform.InstanceState
Diff *terraform.InstanceDiff
Key string
Value interface{}
Ok bool
}{
/*
* Primitives
*/
{
Name: "string-literal-empty",
Schema: map[string]*Schema{
"availability_zone": {
Type: TypeString,
Optional: true,
Computed: true,
ForceNew: true,
},
},

State: nil,

Diff: &terraform.InstanceDiff{
Attributes: map[string]*terraform.ResourceAttrDiff{
"availability_zone": {
Old: "",
New: "",
},
},
},

Key: "availability_zone",
Value: "",
Ok: true,
},

{
Name: "string-computed-empty",
Schema: map[string]*Schema{
"availability_zone": {
Type: TypeString,
Optional: true,
Computed: true,
ForceNew: true,
},
},

State: nil,

Diff: &terraform.InstanceDiff{
Attributes: map[string]*terraform.ResourceAttrDiff{
"availability_zone": {
Old: "",
New: "",
NewComputed: true,
},
},
},

Key: "availability_zone",
Value: "",
Ok: false,
},

{
Name: "string-optional-computed-nil-diff",
Schema: map[string]*Schema{
"availability_zone": {
Type: TypeString,
Optional: true,
Computed: true,
ForceNew: true,
},
},

State: nil,

Diff: nil,

Key: "availability_zone",
Value: "",
Ok: false,
},

/*
* Lists
*/

{
Name: "list-optional",
Schema: map[string]*Schema{
"ports": {
Type: TypeList,
Optional: true,
Elem: &Schema{Type: TypeInt},
},
},

State: nil,

Diff: nil,

Key: "ports",
Value: []interface{}{},
Ok: false,
},

/*
* Map
*/

{
Name: "map-optional",
Schema: map[string]*Schema{
"ports": {
Type: TypeMap,
Optional: true,
},
},

State: nil,

Diff: nil,

Key: "ports",
Value: map[string]interface{}{},
Ok: false,
},

/*
* Set
*/

{
Name: "set-optional",
Schema: map[string]*Schema{
"ports": {
Type: TypeSet,
Optional: true,
Elem: &Schema{Type: TypeInt},
Set: func(a interface{}) int { return a.(int) },
},
},

State: nil,

Diff: nil,

Key: "ports",
Value: []interface{}{},
Ok: false,
},

{
Name: "set-optional-key",
Schema: map[string]*Schema{
"ports": {
Type: TypeSet,
Optional: true,
Elem: &Schema{Type: TypeInt},
Set: func(a interface{}) int { return a.(int) },
},
},

State: nil,

Diff: nil,

Key: "ports.0",
Value: 0,
Ok: false,
},

{
Name: "bool-literal-empty",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is also one of the troublesome combinations, and we need the the test to look like string-literal-empty above, except with TypeBool. That will at least give us one other data point where GetOkExists differs from GetOk.

Schema: map[string]*Schema{
"availability_zone": {
Type: TypeBool,
Optional: true,
Computed: true,
ForceNew: true,
},
},

State: nil,
Diff: nil,

Key: "availability_zone",
Value: false,
Ok: false,
},

{
Name: "bool-literal-set",
Schema: map[string]*Schema{
"availability_zone": {
Type: TypeBool,
Optional: true,
Computed: true,
ForceNew: true,
},
},

State: nil,

Diff: &terraform.InstanceDiff{
Attributes: map[string]*terraform.ResourceAttrDiff{
"availability_zone": {
New: "true",
},
},
},

Key: "availability_zone",
Value: true,
Ok: true,
},
}

for _, tc := range cases {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any new table-driven tests should be using subtests. As a rule of thumb, any old test that I touch gets converted too. This makes debugging changes much easier, since you can see all the case failures at once, rather than one at a time.

d, err := schemaMap(tc.Schema).Data(tc.State, tc.Diff)
if err != nil {
t.Fatalf("%s err: %s", tc.Name, err)
}

v, ok := d.GetOkExists(tc.Key)
if s, ok := v.(*Set); ok {
v = s.List()
}

if !reflect.DeepEqual(v, tc.Value) {
t.Fatalf("Bad %s: \n%#v", tc.Name, v)
}
if ok != tc.Ok {
t.Fatalf("%s: expected ok: %t, got: %t", tc.Name, tc.Ok, ok)
}
}
}

func TestResourceDataTimeout(t *testing.T) {
cases := []struct {
Name string
Expand Down