Skip to content

Commit 1024fbf

Browse files
authored
direct: do not set incorrect ForceSendFields (#3576)
## Changes Only copy ForceSendFields if we're copying the value from the field. If we're omitting the value, we should omit ForceSendFields, otherwise we're might be copying zero. ## Tests New unit tests.
1 parent dfc36b9 commit 1024fbf

File tree

6 files changed

+88
-6
lines changed

6 files changed

+88
-6
lines changed

bundle/terranova/tnresources/job.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ func makeCreateJob(config jobs.JobSettings) (jobs.CreateJob, error) {
9090
Trigger: config.Trigger,
9191
UsagePolicyId: config.UsagePolicyId,
9292
WebhookNotifications: config.WebhookNotifications,
93-
ForceSendFields: filterFields[jobs.CreateJob](config.ForceSendFields),
93+
ForceSendFields: filterFields[jobs.CreateJob](config.ForceSendFields, "AccessControlList"),
9494
}
9595

9696
return result, nil

bundle/terranova/tnresources/schema.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func (r *ResourceSchema) DoUpdate(ctx context.Context, id string, config *catalo
4242
NewName: "", // We recreate schemas on name change intentionally.
4343
Owner: "", // Not supported by DABs
4444
Properties: config.Properties,
45-
ForceSendFields: filterFields[catalog.UpdateSchema](config.ForceSendFields),
45+
ForceSendFields: filterFields[catalog.UpdateSchema](config.ForceSendFields, "EnablePredictiveOptimization", "NewName", "Owner"),
4646
}
4747

4848
response, err := r.client.Schemas.Update(ctx, updateRequest)
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
package tnresources
2+
3+
import (
4+
"context"
5+
"encoding/json"
6+
"testing"
7+
8+
"github.com/databricks/databricks-sdk-go/service/catalog"
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
11+
)
12+
13+
func TestResourceSchema_DoUpdate_WithUnsupportedForceSendFields(t *testing.T) {
14+
_, client := setupTestServerClient(t)
15+
16+
adapter := (*ResourceSchema)(nil).New(client)
17+
ctx := context.Background()
18+
19+
config := &catalog.CreateSchema{
20+
CatalogName: "main",
21+
Name: "test_schema",
22+
Comment: "original comment",
23+
Properties: map[string]string{"key": "value"},
24+
StorageRoot: "",
25+
ForceSendFields: nil,
26+
}
27+
28+
id, _, err := adapter.DoCreate(ctx, config)
29+
require.NoError(t, err)
30+
31+
config.Comment = "updated comment"
32+
config.Properties = map[string]string{"key": "updated_value"}
33+
config.ForceSendFields = []string{
34+
"Comment",
35+
"Properties",
36+
"EnablePredictiveOptimization", // Unsupported - should be filtered out
37+
"NewName", // Unsupported - should be filtered out
38+
"Owner", // Unsupported - should be filtered out
39+
}
40+
41+
_, err = adapter.DoUpdate(ctx, id, config)
42+
require.NoError(t, err)
43+
44+
result, err := adapter.DoRefresh(ctx, id)
45+
require.NoError(t, err)
46+
47+
resultJSON, err := json.Marshal(result)
48+
require.NoError(t, err)
49+
expected := `{
50+
"catalog_name": "main",
51+
"comment": "updated comment",
52+
"properties": {"key": "updated_value"},
53+
"full_name": "main.test_schema",
54+
"name": "test_schema"
55+
}`
56+
assert.JSONEq(t, expected, string(resultJSON))
57+
}

bundle/terranova/tnresources/util.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,23 @@ import (
66
"github.com/databricks/databricks-sdk-go/retries"
77
)
88

9-
// filterFields creates a new slice with fields present only in the provided type.
9+
// filterFields creates a new slice with fields present only in the provided type,
10+
// excluding any fields specified in the excludeFields list.
1011
// We must use that when copying structs because JSON marshaller in SDK crashes if it sees unknown field.
11-
func filterFields[T any](fields []string) []string {
12+
func filterFields[T any](fields []string, excludeFields ...string) []string {
1213
var result []string
1314
typeOfT := reflect.TypeOf((*T)(nil)).Elem()
1415

16+
excludeMap := make(map[string]bool)
17+
for _, exclude := range excludeFields {
18+
excludeMap[exclude] = true
19+
}
20+
1521
for _, field := range fields {
22+
// Skip if field is in exclude list
23+
if excludeMap[field] {
24+
continue
25+
}
1626
if _, ok := typeOfT.FieldByName(field); ok {
1727
result = append(result, field)
1828
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
package tnresources
2+
3+
import (
4+
"testing"
5+
6+
"github.com/databricks/databricks-sdk-go/service/catalog"
7+
"github.com/stretchr/testify/assert"
8+
)
9+
10+
func TestFilterFields(t *testing.T) {
11+
fields := []string{"Comment", "NewName", "Owner", "Name", "NotExistingField"}
12+
result := filterFields[catalog.UpdateVolumeRequestContent](fields, "NewName", "Owner")
13+
expected := []string{"Comment", "Name"}
14+
assert.Equal(t, expected, result)
15+
}

bundle/terranova/tnresources/volume.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ func (r *ResourceVolume) DoUpdate(ctx context.Context, id string, config *catalo
4444
NewName: "", // Not supported by Update(). Needs DoUpdateWithID()
4545
Owner: "", // Not supported by DABs
4646

47-
ForceSendFields: nil,
47+
ForceSendFields: filterFields[catalog.UpdateVolumeRequestContent](config.ForceSendFields, "NewName", "Owner"),
4848
}
4949

5050
nameFromID, err := getNameFromID(id)
@@ -76,7 +76,7 @@ func (r *ResourceVolume) DoUpdateWithID(ctx context.Context, id string, config *
7676
NewName: "", // Initialized below if needed
7777
Owner: "", // Not supported by DABs
7878

79-
ForceSendFields: nil,
79+
ForceSendFields: filterFields[catalog.UpdateVolumeRequestContent](config.ForceSendFields, "Owner"),
8080
}
8181

8282
items := strings.Split(id, ".")

0 commit comments

Comments
 (0)