Repository: honeycombio/terraform-provider-honeycombio (go)

Issue:
honeycombio_query_specification changes not picked up properly
I've written a bit of terraform to create triggers from a map of specs, but I've noticed when I change the calculation field for my honeycombio_query_specification  and run terraform plan terraform is telling me there are no changes.
```

```
When I run terraform show I can resources I've created. The query spec is:
```
# data.honeycombio_query_specification.trigger_query_spec_test["dummy-app-alert"]:
data "honeycombio_query_specification" "trigger_query_spec_test" {
    filter_combination = "AND"
    id                 = "3957407590"
    json               = jsonencode(
        {
            calculations = [
                {
                    column = "metrics.cpu.usage"
                    op     = "MIN"
                },
            ]
            filters      = [
                {
                    column = "label.app"
                    op     = "="
                    value  = "jazz-backend-api"
                },
            ]
            time_range   = 300
        }
    )
    time_range         = 300

    calculation {
        column = "metrics.cpu.usage"
        op     = "MIN"
    }

    filter {
        column        = "label.app"
        op            = "="
        value         = "jazz-backend-api"
        value_boolean = false
        value_float   = 0
        value_integer = 0
    }
}
```

Note the calculations part of the json where the op is MIN and the column is metrics.cpu.usage.
This was generated with a tfvars.json file containing the alert spec:
```
    "alerts_test": {
        "dummy-app-alert": {
            "description": "An example alert for aac demonstrating the yaml spec",
            "dataset": "kubernetes-metrics",
            "threshold": {
                "operation": ">",
                "value": 0.9
            },
            "recipient": {
                "type": "email",
                "target": "wjmartin@usxpress.com"
            },
            "alert_type": "on_change",
            "query_spec": {
                "calculation": {
                    "operation": "MIN",
                    "field": "metrics.cpu.usage"
                },
                "filter": {
                    "operation": "=",
                    "field": "label.app",
                    "value": "jazz-backend-api"
                },
                "time_range": 300
            },
            "frequency": 300
        }
    }
```
If I make a change to calculation or the time_range in my tfvars.json file, the changes aren't picked up. For example:
```
    "alerts_test": {
        "dummy-app-alert": {
            "description": "An example alert for aac, demonstrating the yaml spec",
            "dataset": "kubernetes-metrics",
            "threshold": {
                "operation": ">",
                "value": 0.8
            },
            "recipient": {
                "type": "email",
                "target": "wjmartin@usxpress.com"
            },
            "alert_type": "on_change",
            "query_spec": {
                "calculation": {
                    "operation": "MAX",
                    "field": "metrics.cpu.utilization"
                },
                "filter": {
                    "operation": "=",
                    "field": "label.app",
                    "value": "jazz-backend-api"
                },
                "time_range": 150
            },
            "frequency": 300
        }
    }
```

I've changed alerts_test.dummy_app_alert.query_spec.calculation both the operation and field parameters. As well as the alerts_test.dummy_app_alert.query_spec.time_range.

Running terraform plan with the changes saved yields:
```
terraform plan -out=tfplan -input=false -var-file="test.tfvars.json"
honeycombio_query.trigger_query_test["dummy-app-alert"]: Refreshing state... [id=oLEiKjd2m3S]
honeycombio_trigger.trigger_test["dummy-app-alert"]: Refreshing state... [id=wwfqnqrxZ3]

No changes. Your infrastructure matches the configuration.
```

However, were I to change any part of the alerts_test.dummy_app_alert.query_spec.filter , then I find my changes are picked up. For example, changing the filter.operation.
```
    "alerts_test": {
        "dummy-app-alert": {
            "description": "An example alert for aac, demonstrating the yaml spec",
            "dataset": "kubernetes-metrics",
            "threshold": {
                "operation": ">",
                "value": 0.8
            },
            "recipient": {
                "type": "email",
                "target": "wjmartin@usxpress.com"
            },
            "alert_type": "on_change",
            "query_spec": {
                "calculation": {
                    "operation": "MAX",
                    "field": "metrics.cpu.utilization"
                },
                "filter": {
                    "operation": ">",
                    "field": "label.app",
                    "value": "jazz-backend-api"
                },
                "time_range": 150
            },
            "frequency": 300
        }
    }
```
On plan:
```
terraform plan -out=tfplan -input=false -var-file="test.tfvars.json"
honeycombio_query.trigger_query_test["dummy-app-alert"]: Refreshing state... [id=oLEiKjd2m3S]
honeycombio_trigger.trigger_test["dummy-app-alert"]: Refreshing state... [id=wwfqnqrxZ3]
Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  ~ update in-place
-/+ destroy and then create replacement

Terraform will perform the following actions:

  # honeycombio_query.trigger_query_test["dummy-app-alert"] must be replaced
-/+ resource "honeycombio_query" "trigger_query_test" {
      ~ id         = "oLEiKjd2m3S" -> (known after apply)
      ~ query_json = jsonencode(
          ~ {
              ~ calculations = [
                  ~ {
                      ~ column = "metrics.cpu.usage" -> "metrics.cpu.utilization"
                      ~ op     = "MIN" -> "MAX"
                    },
                ]
              ~ filters      = [
                  ~ {
                      ~ op     = "=" -> ">"
                        # (2 unchanged elements hidden)
                    },
                ]
              ~ time_range   = 300 -> 150
            } # forces replacement
        )
        # (1 unchanged attribute hidden)
    }
```

**Versions**
Terraform v1.1.7
Honeycombio terraform provider version 0.13.0


**Steps to reproduce**
1. Create a simple trigger using terraform. For example, you may use `main.tf` as 
```
data "honeycombio_query_specification" "query_specification" {
    calculation {
        op = "MAX"
        column = [any valid column from the dataset of your choosing]
    }

    filter { 
        column = "[any valid column from the dataset of your choosing]"
        op = "="
        value = "[any valid value]"
    }
    time_range = 300
}

resource "honeycombio_query" "trigger_query" {
    dataset = [Any dataset of your choice in your honeycomb environment]
    query_json = data.honeycombio_query_specification.query_specification.json
}

resource "honeycombio_trigger" "trigger" {
    name = "honeycomb-bug"
    description = "Created to reproduce observed honeycomb bug"
    query_id = honeycombio_query.trigger_query.id
    dataset = [Any dataset of your choice in your honeycomb environment, but must match dataset in honeycomb_query]
    frequency = 300
    alert_type = "on_change"
    threshold {
        op = ">"
        value = 0.9
    }
}
```
with a settings file such as:
```
terraform { 
    required_providers { 
        honeycombio = {
            source = "honeycombio/honeycombio"
            version = "~> 0.13.0"
        }
    }
    required_version = "~> 1.1.7"
}

provider "honeycombio" {
}
```

2. Run `terraform apply` to create the infrastructure. When prompted, give a honeycomb API key with sufficient permissions to create the trigger. The trigger should be created. 

3. Make a change to the `honeycombio_query_specification` in the calculation. For example, changing the operation from MAX to MIN. 
```
data "honeycombio_query_specification" "query_specification" {
    calculation {
        op = "MAX"
        column = [any valid column from the dataset of your choosing]
    }

    filter { 
        column = "[any valid column from the dataset of your choosing]"
        op = "="
        value = "[any valid value]"
    }
    time_range = 300
}

resource "honeycombio_query" "trigger_query" {
    dataset = [Any dataset of your choice in your honeycomb environment]
    query_json = data.honeycombio_query_specification.query_specification.json
}

resource "honeycombio_trigger" "trigger" {
    name = "honeycomb-bug"
    description = "Created to reproduce observed honeycomb bug"
    query_id = honeycombio_query.trigger_query.id
    dataset = [Any dataset of your choice in your honeycomb environment, but must match dataset in honeycomb_query]
    frequency = 300
    alert_type = "on_change"
    threshold {
        op = ">"
        value = 0.9
    }
}
```
Once again, run `terraform apply`. The output suggested no changes are made. 


Please let me know if any additional info is needed. Your patience is appreciated as I'm not very familiar with submitting Github issues. Thank you!


Pull request:
bugfix: query equivalence for time_range, filter ops, and calculations
## Which problem is this PR solving?

- Closes #275

## Short description of the changes

The QuerySpec 'equivalence' functionality introduced in #236 to suppress diffs failed to detect changes in `time_range`, `filter_combination`, and `calculation`.


Patch:
diff --git a/client/query_spec.go b/client/query_spec.go
index 118fc37..8260967 100644
--- a/client/query_spec.go
+++ b/client/query_spec.go
@@ -50,6 +50,8 @@ type QuerySpec struct {
 	Granularity *int `json:"granularity,omitempty"`
 }
 
+const DefaultQueryTimeRange = 2 * 60 * 60
+
 // Determines if two QuerySpecs are equivalent
 func (qs *QuerySpec) EquivalentTo(other QuerySpec) bool {
 	// The order of Calculations is important for visualization rendering, so we're looking for equality
@@ -67,9 +69,14 @@ func (qs *QuerySpec) EquivalentTo(other QuerySpec) bool {
 	if !calcMatch {
 		// 'COUNT' is the default Calculation and equivalent to an empty Calculations -- check that before we give up
 		defaultCalc := []CalculationSpec{{Op: CalculationOpCount}}
-		if (other.Calculations != nil && reflect.DeepEqual(qs.Calculations, defaultCalc)) ||
-			(qs.Calculations != nil && reflect.DeepEqual(other.Calculations, defaultCalc)) ||
-			(len(qs.Calculations) > 1 || len(other.Calculations) > 1) {
+		qsC, oC := defaultCalc, defaultCalc
+		if len(qs.Calculations) != 0 {
+			qsC = qs.Calculations
+		}
+		if len(other.Calculations) != 0 {
+			oC = other.Calculations
+		}
+		if !reflect.DeepEqual(qsC, oC) {
 			return false
 		}
 	}
@@ -78,12 +85,8 @@ func (qs *QuerySpec) EquivalentTo(other QuerySpec) bool {
 	if !Equivalent(qs.Filters, other.Filters) {
 		return false
 	}
-	if qs.FilterCombination != other.FilterCombination {
-		// 'AND' is the default and equivalent to an empty Filter Combination
-		if (qs.FilterCombination == FilterCombinationAnd && other.FilterCombination != "") ||
-			(other.FilterCombination == FilterCombinationAnd && qs.FilterCombination != "") {
-			return false
-		}
+	if ValueOrDefault(qs.FilterCombination, DefaultFilterCombination) != ValueOrDefault(other.FilterCombination, DefaultFilterCombination) {
+		return false
 	}
 	if !reflect.DeepEqual(qs.Breakdowns, other.Breakdowns) {
 		return false
@@ -98,12 +101,8 @@ func (qs *QuerySpec) EquivalentTo(other QuerySpec) bool {
 	if !reflect.DeepEqual(qs.Limit, other.Limit) {
 		return false
 	}
-	if !reflect.DeepEqual(qs.TimeRange, other.TimeRange) {
-		// 7200 is the default, so an unset value is equivalent to 7200
-		if (reflect.DeepEqual(qs.TimeRange, 7200) && other.TimeRange != nil) ||
-			(reflect.DeepEqual(other.TimeRange, 7200) && qs.TimeRange != nil) {
-			return false
-		}
+	if PtrValueOrDefault(qs.TimeRange, DefaultQueryTimeRange) != PtrValueOrDefault(other.TimeRange, DefaultQueryTimeRange) {
+		return false
 	}
 	if !reflect.DeepEqual(qs.StartTime, other.StartTime) || !reflect.DeepEqual(qs.EndTime, other.EndTime) {
 		return false
@@ -241,8 +240,9 @@ type FilterCombination string
 
 // Declaration of filter combinations.
 const (
-	FilterCombinationOr  FilterCombination = "OR"
-	FilterCombinationAnd FilterCombination = "AND"
+	FilterCombinationOr      FilterCombination = "OR"
+	FilterCombinationAnd     FilterCombination = "AND"
+	DefaultFilterCombination                   = FilterCombinationAnd
 )
 
 // FilterCombinations returns an exhaustive list of filter combinations.
diff --git a/client/type_helpers.go b/client/type_helpers.go
index dcfc15d..e7e90e4 100644
--- a/client/type_helpers.go
+++ b/client/type_helpers.go
@@ -46,3 +46,21 @@ func Equivalent[T any](a, b []T) bool {
 
 	return true
 }
+
+func IsZero[T comparable](v T) bool {
+	return v == *new(T)
+}
+
+func PtrValueOrDefault[T any](v *T, d T) T {
+	if v != nil {
+		return *v
+	}
+	return d
+}
+
+func ValueOrDefault[T comparable](v, d T) T {
+	if !IsZero(v) {
+		return v
+	}
+	return d
+}
diff --git a/honeycombio/data_source_query_specification.go b/honeycombio/data_source_query_specification.go
index 8687bf2..4308f4c 100644
--- a/honeycombio/data_source_query_specification.go
+++ b/honeycombio/data_source_query_specification.go
@@ -113,8 +113,8 @@ func dataSourceHoneycombioQuerySpec() *schema.Resource {
 			"filter_combination": {
 				Type:         schema.TypeString,
 				Optional:     true,
-				Default:      "AND",
-				ValidateFunc: validation.StringInSlice([]string{"AND", "OR"}, false),
+				Default:      honeycombio.DefaultFilterCombination,
+				ValidateFunc: validation.StringInSlice([]string{string(honeycombio.FilterCombinationAnd), string(honeycombio.FilterCombinationOr)}, false),
 			},
 			"breakdowns": {
 				Type:     schema.TypeList,
@@ -158,7 +158,7 @@ func dataSourceHoneycombioQuerySpec() *schema.Resource {
 				// Terraform isn't able to set the computed value causing a
 				// constant diff. By using a default value instead, we don't
 				// need the feedback from the API.
-				Default: 7200,
+				Default: honeycombio.DefaultQueryTimeRange,
 			},
 			"start_time": {
 				Type:     schema.TypeInt,
@@ -213,10 +213,10 @@ func dataSourceHoneycombioQuerySpecRead(ctx context.Context, d *schema.ResourceD
 		}
 	}
 
-	// The API doesn't return filter_combination if it is 'AND' (the default)
+	// The API doesn't return filter_combination if it's the default
 	var filterCombination honeycombio.FilterCombination
 	filterString := d.Get("filter_combination").(string)
-	if filterString != "" && filterString != "AND" {
+	if filterString != "" && filterString != string(honeycombio.DefaultFilterCombination) {
 		// doing it this way to support possible different filter types in future
 		// and having one less place to update them
 		filterCombination = honeycombio.FilterCombination(filterString)


Test patch:
diff --git a/client/board_test.go b/client/board_test.go
index ba80298..5f35bd0 100644
--- a/client/board_test.go
+++ b/client/board_test.go
@@ -82,7 +82,7 @@ func TestBoards(t *testing.T) {
 					Op: CalculationOpCount,
 				},
 			},
-			TimeRange: ToPtr(7200), // 2 hours
+			TimeRange: ToPtr(DefaultQueryTimeRange),
 		})
 		assert.NoError(t, err)
 		b.ColumnLayout = BoardColumnStyleMulti
diff --git a/client/query_spec_test.go b/client/query_spec_test.go
index 5031c0f..b932edf 100644
--- a/client/query_spec_test.go
+++ b/client/query_spec_test.go
@@ -88,7 +88,7 @@ func TestQuerySpec_EquivalentTo(t *testing.T) {
 					},
 				},
 				FilterCombination: "AND",
-				TimeRange:         ToPtr(7200),
+				TimeRange:         ToPtr(DefaultQueryTimeRange),
 			},
 			QuerySpec{},
 			true,
@@ -123,7 +123,7 @@ func TestQuerySpec_EquivalentTo(t *testing.T) {
 					},
 				},
 				Breakdowns: []string{"colB", "colA"},
-				TimeRange:  ToPtr(7200),
+				TimeRange:  ToPtr(DefaultQueryTimeRange),
 			},
 			QuerySpec{
 				Calculations: []CalculationSpec{
@@ -183,12 +183,93 @@ func TestQuerySpec_EquivalentTo(t *testing.T) {
 			},
 			false,
 		},
+		{
+			"Different time ranges",
+			QuerySpec{
+				Calculations: []CalculationSpec{
+					{
+						Op: "COUNT",
+					},
+				},
+				TimeRange: ToPtr(1800),
+			},
+			QuerySpec{
+				Calculations: []CalculationSpec{
+					{
+						Op: "COUNT",
+					},
+				},
+			},
+			false,
+		},
+		{
+			"Different FilterCombinations",
+			QuerySpec{
+				FilterCombination: "OR",
+			},
+			QuerySpec{},
+			false,
+		},
+		{
+			"Calculation different from DefaultCalc",
+			QuerySpec{
+				Calculations: []CalculationSpec{
+					{
+						Op:     "MIN",
+						Column: ToPtr("metrics.cpu.utilization"),
+					},
+				},
+			},
+			QuerySpec{},
+			false,
+		},
+		{
+			"Different Calculations",
+			QuerySpec{
+				Calculations: []CalculationSpec{
+					{
+						Op:     "MIN",
+						Column: ToPtr("metrics.cpu.utilization"),
+					},
+				},
+			},
+			QuerySpec{
+				Calculations: []CalculationSpec{
+					{
+						Op:     "MAX",
+						Column: ToPtr("metrics.cpu.utilization"),
+					},
+				},
+			},
+			false,
+		},
+		{
+			"Different Number of Calculations",
+			QuerySpec{
+				Calculations: []CalculationSpec{
+					{
+						Op:     "MIN",
+						Column: ToPtr("metrics.cpu.utilization"),
+					},
+				},
+			},
+			QuerySpec{
+				Calculations: []CalculationSpec{
+					{
+						Op: "COUNT_DISTINCT",
+					},
+					{
+						Op:     "MAX",
+						Column: ToPtr("metrics.cpu.utilization"),
+					},
+				},
+			},
+			false,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := tt.a.EquivalentTo(tt.b); got != tt.want {
-				t.Errorf("QuerySpec.EquivalentTo() = %v, want %v", got, tt.want)
-			}
+			assert.Equal(t, tt.want, tt.a.EquivalentTo(tt.b))
 		})
 	}
 }
diff --git a/honeycombio/resource_query_test.go b/honeycombio/resource_query_test.go
index b684bb1..d7877f9 100644
--- a/honeycombio/resource_query_test.go
+++ b/honeycombio/resource_query_test.go
@@ -94,7 +94,7 @@ func testAccCheckQueryExists(t *testing.T, dataset string, name string, duration
 					Value:  float64(duration),
 				},
 			},
-			TimeRange: honeycombio.ToPtr(7200),
+			TimeRange: honeycombio.ToPtr(honeycombio.DefaultQueryTimeRange),
 		}
 
 		ok = assert.Equal(t, expectedQuery, createdQuery)