From 1525e49bf3945121aafb4299abc37d3ee0b4dcea Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 23 Mar 2026 14:33:20 +0100 Subject: [PATCH 01/16] Add fieldcopy package and migrate cluster to use it Introduce libs/structs/fieldcopy for reflection-based struct copying with test-time completeness validation. ForceSendFields is auto-filtered. Migrate cluster's RemapState, makeCreateCluster, makeEditCluster. Co-authored-by: Isaac --- bundle/direct/dresources/cluster.go | 162 +++----- bundle/direct/dresources/fieldcopy_test.go | 18 + libs/structs/fieldcopy/fieldcopy.go | 258 ++++++++++++ libs/structs/fieldcopy/fieldcopy_test.go | 442 +++++++++++++++++++++ 4 files changed, 766 insertions(+), 114 deletions(-) create mode 100644 bundle/direct/dresources/fieldcopy_test.go create mode 100644 libs/structs/fieldcopy/fieldcopy.go create mode 100644 libs/structs/fieldcopy/fieldcopy_test.go diff --git a/bundle/direct/dresources/cluster.go b/bundle/direct/dresources/cluster.go index a8f78d12f9..be8d675c6e 100644 --- a/bundle/direct/dresources/cluster.go +++ b/bundle/direct/dresources/cluster.go @@ -8,6 +8,7 @@ import ( "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/bundle/deployplan" + "github.com/databricks/cli/libs/structs/fieldcopy" "github.com/databricks/cli/libs/structs/structpath" "github.com/databricks/cli/libs/utils" "github.com/databricks/databricks-sdk-go" @@ -30,48 +31,40 @@ func (r *ResourceCluster) PrepareState(input *resources.Cluster) *compute.Cluste return &input.ClusterSpec } +// clusterRemapCopy maps ClusterDetails (remote GET response) to ClusterSpec (local state). +var clusterRemapCopy = fieldcopy.Copy[compute.ClusterDetails, compute.ClusterSpec]{ + SkipSrc: []string{ + "ClusterCores", + "ClusterId", + "ClusterLogStatus", + "ClusterMemoryMb", + "ClusterSource", + "CreatorUserName", + "DefaultTags", + "Driver", + "Executors", + "JdbcPort", + "LastRestartedTime", + "LastStateLossTime", + "SparkContextId", + "Spec", + "StartTime", + "State", + "StateMessage", + "TerminatedTime", + "TerminationReason", + }, + SkipDst: []string{ + "ApplyPolicyDefaultValues", // Pulled from input.Spec in post-processing. + }, +} + func (r *ResourceCluster) RemapState(input *compute.ClusterDetails) *compute.ClusterSpec { - spec := &compute.ClusterSpec{ - ApplyPolicyDefaultValues: false, - Autoscale: input.Autoscale, - AutoterminationMinutes: input.AutoterminationMinutes, - AwsAttributes: input.AwsAttributes, - AzureAttributes: input.AzureAttributes, - ClusterLogConf: input.ClusterLogConf, - ClusterName: input.ClusterName, - CustomTags: input.CustomTags, - DataSecurityMode: input.DataSecurityMode, - DockerImage: input.DockerImage, - DriverInstancePoolId: input.DriverInstancePoolId, - DriverNodeTypeId: input.DriverNodeTypeId, - DriverNodeTypeFlexibility: input.DriverNodeTypeFlexibility, - EnableElasticDisk: input.EnableElasticDisk, - EnableLocalDiskEncryption: input.EnableLocalDiskEncryption, - GcpAttributes: input.GcpAttributes, - InitScripts: input.InitScripts, - InstancePoolId: input.InstancePoolId, - IsSingleNode: input.IsSingleNode, - Kind: input.Kind, - NodeTypeId: input.NodeTypeId, - NumWorkers: input.NumWorkers, - PolicyId: input.PolicyId, - RemoteDiskThroughput: input.RemoteDiskThroughput, - RuntimeEngine: input.RuntimeEngine, - SingleUserName: input.SingleUserName, - SparkConf: input.SparkConf, - SparkEnvVars: input.SparkEnvVars, - SparkVersion: input.SparkVersion, - SshPublicKeys: input.SshPublicKeys, - TotalInitialRemoteDiskSize: input.TotalInitialRemoteDiskSize, - UseMlRuntime: input.UseMlRuntime, - WorkloadType: input.WorkloadType, - WorkerNodeTypeFlexibility: input.WorkerNodeTypeFlexibility, - ForceSendFields: utils.FilterFields[compute.ClusterSpec](input.ForceSendFields), - } + spec := clusterRemapCopy.Do(input) if input.Spec != nil { spec.ApplyPolicyDefaultValues = input.Spec.ApplyPolicyDefaultValues } - return spec + return &spec } func (r *ResourceCluster) DoRead(ctx context.Context, id string) (*compute.ClusterDetails, error) { @@ -151,45 +144,15 @@ func (r *ResourceCluster) OverrideChangeDesc(ctx context.Context, p *structpath. return nil } +// clusterCreateCopy maps ClusterSpec (local state) to CreateCluster (API request). +var clusterCreateCopy = fieldcopy.Copy[compute.ClusterSpec, compute.CreateCluster]{ + SkipDst: []string{ + "CloneFrom", // Not supported by DABs. + }, +} + func makeCreateCluster(config *compute.ClusterSpec) compute.CreateCluster { - create := compute.CreateCluster{ - ApplyPolicyDefaultValues: config.ApplyPolicyDefaultValues, - Autoscale: config.Autoscale, - AutoterminationMinutes: config.AutoterminationMinutes, - AwsAttributes: config.AwsAttributes, - AzureAttributes: config.AzureAttributes, - ClusterLogConf: config.ClusterLogConf, - ClusterName: config.ClusterName, - CloneFrom: nil, // Not supported by DABs - CustomTags: config.CustomTags, - DataSecurityMode: config.DataSecurityMode, - DockerImage: config.DockerImage, - DriverInstancePoolId: config.DriverInstancePoolId, - DriverNodeTypeId: config.DriverNodeTypeId, - DriverNodeTypeFlexibility: config.DriverNodeTypeFlexibility, - EnableElasticDisk: config.EnableElasticDisk, - EnableLocalDiskEncryption: config.EnableLocalDiskEncryption, - GcpAttributes: config.GcpAttributes, - InitScripts: config.InitScripts, - InstancePoolId: config.InstancePoolId, - IsSingleNode: config.IsSingleNode, - Kind: config.Kind, - NodeTypeId: config.NodeTypeId, - NumWorkers: config.NumWorkers, - PolicyId: config.PolicyId, - RemoteDiskThroughput: config.RemoteDiskThroughput, - RuntimeEngine: config.RuntimeEngine, - SingleUserName: config.SingleUserName, - SparkConf: config.SparkConf, - SparkEnvVars: config.SparkEnvVars, - SparkVersion: config.SparkVersion, - SshPublicKeys: config.SshPublicKeys, - TotalInitialRemoteDiskSize: config.TotalInitialRemoteDiskSize, - UseMlRuntime: config.UseMlRuntime, - WorkloadType: config.WorkloadType, - WorkerNodeTypeFlexibility: config.WorkerNodeTypeFlexibility, - ForceSendFields: utils.FilterFields[compute.CreateCluster](config.ForceSendFields), - } + create := clusterCreateCopy.Do(config) // If autoscale is not set, we need to send NumWorkers because one of them is required. // If NumWorkers is not nil, we don't need to set it to ForceSendFields as it will be sent anyway. @@ -200,45 +163,16 @@ func makeCreateCluster(config *compute.ClusterSpec) compute.CreateCluster { return create } +// clusterEditCopy maps ClusterSpec (local state) to EditCluster (API request). +var clusterEditCopy = fieldcopy.Copy[compute.ClusterSpec, compute.EditCluster]{ + SkipDst: []string{ + "ClusterId", // Set from function parameter. + }, +} + func makeEditCluster(id string, config *compute.ClusterSpec) compute.EditCluster { - edit := compute.EditCluster{ - ClusterId: id, - ApplyPolicyDefaultValues: config.ApplyPolicyDefaultValues, - Autoscale: config.Autoscale, - AutoterminationMinutes: config.AutoterminationMinutes, - AwsAttributes: config.AwsAttributes, - AzureAttributes: config.AzureAttributes, - ClusterLogConf: config.ClusterLogConf, - ClusterName: config.ClusterName, - CustomTags: config.CustomTags, - DataSecurityMode: config.DataSecurityMode, - DockerImage: config.DockerImage, - DriverInstancePoolId: config.DriverInstancePoolId, - DriverNodeTypeId: config.DriverNodeTypeId, - DriverNodeTypeFlexibility: config.DriverNodeTypeFlexibility, - EnableElasticDisk: config.EnableElasticDisk, - EnableLocalDiskEncryption: config.EnableLocalDiskEncryption, - GcpAttributes: config.GcpAttributes, - InitScripts: config.InitScripts, - InstancePoolId: config.InstancePoolId, - IsSingleNode: config.IsSingleNode, - Kind: config.Kind, - NodeTypeId: config.NodeTypeId, - NumWorkers: config.NumWorkers, - PolicyId: config.PolicyId, - RemoteDiskThroughput: config.RemoteDiskThroughput, - RuntimeEngine: config.RuntimeEngine, - SingleUserName: config.SingleUserName, - SparkConf: config.SparkConf, - SparkEnvVars: config.SparkEnvVars, - SparkVersion: config.SparkVersion, - SshPublicKeys: config.SshPublicKeys, - TotalInitialRemoteDiskSize: config.TotalInitialRemoteDiskSize, - UseMlRuntime: config.UseMlRuntime, - WorkloadType: config.WorkloadType, - WorkerNodeTypeFlexibility: config.WorkerNodeTypeFlexibility, - ForceSendFields: utils.FilterFields[compute.EditCluster](config.ForceSendFields), - } + edit := clusterEditCopy.Do(config) + edit.ClusterId = id // If autoscale is not set, we need to send NumWorkers because one of them is required. // If NumWorkers is not nil, we don't need to set it to ForceSendFields as it will be sent anyway. diff --git a/bundle/direct/dresources/fieldcopy_test.go b/bundle/direct/dresources/fieldcopy_test.go new file mode 100644 index 0000000000..ef01c2c589 --- /dev/null +++ b/bundle/direct/dresources/fieldcopy_test.go @@ -0,0 +1,18 @@ +package dresources + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestFieldCopyValidate(t *testing.T) { + copies := []interface{ Validate() error }{ + &clusterRemapCopy, + &clusterCreateCopy, + &clusterEditCopy, + } + for _, c := range copies { + require.NoError(t, c.Validate()) + } +} diff --git a/libs/structs/fieldcopy/fieldcopy.go b/libs/structs/fieldcopy/fieldcopy.go new file mode 100644 index 0000000000..59349ff3c6 --- /dev/null +++ b/libs/structs/fieldcopy/fieldcopy.go @@ -0,0 +1,258 @@ +// Package fieldcopy provides reflection-based struct-to-struct field copying +// with test-time completeness validation. Field mappings are computed once +// on first use and cached for subsequent calls. +// +// If both Src and Dst have a ForceSendFields []string field, it is handled +// automatically: source values are filtered to only include names of exported +// Dst fields that are being copied (not in SkipDst). +package fieldcopy + +import ( + "fmt" + "reflect" + "strings" + "sync" +) + +const forceSendFieldsName = "ForceSendFields" + +// Copy describes a field-by-field mapping from Src to Dst struct type. +// Fields with matching names and assignable types are copied automatically. +type Copy[Src, Dst any] struct { + // Rename maps destination field name to source field name for fields + // with different names in source and destination. + Rename map[string]string + + // SkipSrc lists source field names intentionally not copied. + SkipSrc []string + + // SkipDst lists destination field names intentionally left at zero value. + SkipDst []string + + once sync.Once + copyFn func(src *Src) Dst +} + +// Do copies fields from src to a new Dst value using precomputed field mappings. +// On first call, field mappings are computed via reflection and cached. +func (c *Copy[Src, Dst]) Do(src *Src) Dst { + c.once.Do(func() { + c.copyFn = c.build() + }) + return c.copyFn(src) +} + +type fieldOp struct { + dstIndex []int + srcIndex []int +} + +func (c *Copy[Src, Dst]) build() func(*Src) Dst { + srcType := reflect.TypeFor[Src]() + dstType := reflect.TypeFor[Dst]() + + skipDst := toSet(c.SkipDst) + + // Detect auto-handling of ForceSendFields: both types must have it as []string + // and it must not be in SkipDst (which would mean manual handling). + var autoFSF bool + var fsfSrcIdx, fsfDstIdx []int + var validFSFNames map[string]bool + + stringSliceType := reflect.TypeFor[[]string]() + if !skipDst[forceSendFieldsName] { + dstFSF, dstOK := dstType.FieldByName(forceSendFieldsName) + srcFSF, srcOK := srcType.FieldByName(forceSendFieldsName) + if dstOK && srcOK && dstFSF.Type == stringSliceType && srcFSF.Type == stringSliceType { + autoFSF = true + fsfSrcIdx = srcFSF.Index + fsfDstIdx = dstFSF.Index + + // Build set of valid field names: exported dst fields that are being copied. + validFSFNames = make(map[string]bool) + for i := range dstType.NumField() { + f := dstType.Field(i) + if f.IsExported() && f.Name != forceSendFieldsName && !skipDst[f.Name] { + validFSFNames[f.Name] = true + } + } + } + } + + var ops []fieldOp + + for i := range dstType.NumField() { + df := dstType.Field(i) + if !df.IsExported() || skipDst[df.Name] { + continue + } + if autoFSF && df.Name == forceSendFieldsName { + continue // handled separately + } + + srcName := df.Name + if c.Rename != nil { + if renamed, ok := c.Rename[df.Name]; ok { + srcName = renamed + } + } + + sf, ok := srcType.FieldByName(srcName) + if !ok || !sf.Type.AssignableTo(df.Type) { + continue + } + + ops = append(ops, fieldOp{dstIndex: df.Index, srcIndex: sf.Index}) + } + + return func(src *Src) Dst { + var dst Dst + sv := reflect.ValueOf(src).Elem() + dv := reflect.ValueOf(&dst).Elem() + for _, op := range ops { + dv.FieldByIndex(op.dstIndex).Set(sv.FieldByIndex(op.srcIndex)) + } + if autoFSF { + srcFSF := sv.FieldByIndex(fsfSrcIdx) + if !srcFSF.IsNil() { + srcFields := srcFSF.Interface().([]string) + var filtered []string + for _, name := range srcFields { + if validFSFNames[name] { + filtered = append(filtered, name) + } + } + if filtered != nil { + dv.FieldByIndex(fsfDstIdx).Set(reflect.ValueOf(filtered)) + } + } + } + return dst + } +} + +// Validate checks that the mapping is complete: +// - Every exported Dst field is either matched in Src, in Rename, or in SkipDst. +// - Every exported Src field is either matched in Dst, a Rename source, or in SkipSrc. +// - No stale entries in SkipSrc, SkipDst, or Rename. +// +// ForceSendFields is considered auto-handled when both types have it as []string +// and it is not in SkipDst. +func (c *Copy[Src, Dst]) Validate() error { + srcType := reflect.TypeFor[Src]() + dstType := reflect.TypeFor[Dst]() + + skipSrc := toSet(c.SkipSrc) + skipDst := toSet(c.SkipDst) + + // Detect auto-handled ForceSendFields. + autoFSF := false + stringSliceType := reflect.TypeFor[[]string]() + if !skipDst[forceSendFieldsName] { + dstFSF, dstOK := dstType.FieldByName(forceSendFieldsName) + srcFSF, srcOK := srcType.FieldByName(forceSendFieldsName) + if dstOK && srcOK && dstFSF.Type == stringSliceType && srcFSF.Type == stringSliceType { + autoFSF = true + } + } + + // Build set of rename targets (src field names used by Rename). + renameTargets := make(map[string]string) // src name → dst name + if c.Rename != nil { + for dstName, srcName := range c.Rename { + renameTargets[srcName] = dstName + } + } + + var errs []string + + // Check every exported dst field is handled. + for i := range dstType.NumField() { + df := dstType.Field(i) + if !df.IsExported() { + continue + } + if skipDst[df.Name] { + continue + } + if autoFSF && df.Name == forceSendFieldsName { + continue + } + + srcName := df.Name + if c.Rename != nil { + if renamed, ok := c.Rename[df.Name]; ok { + srcName = renamed + } + } + + sf, ok := srcType.FieldByName(srcName) + if !ok { + errs = append(errs, fmt.Sprintf("dst field %q: no matching field %q on %v (add to Rename or SkipDst)", df.Name, srcName, srcType)) + continue + } + if !sf.Type.AssignableTo(df.Type) { + errs = append(errs, fmt.Sprintf("dst field %q: type %v not assignable from src type %v (add to SkipDst and handle in post-processing)", df.Name, df.Type, sf.Type)) + } + } + + // Check every exported src field is consumed. + for i := range srcType.NumField() { + sf := srcType.Field(i) + if !sf.IsExported() { + continue + } + if skipSrc[sf.Name] { + continue + } + if autoFSF && sf.Name == forceSendFieldsName { + continue + } + if _, ok := renameTargets[sf.Name]; ok { + continue + } + if _, ok := dstType.FieldByName(sf.Name); ok { + continue + } + errs = append(errs, fmt.Sprintf("src field %q on %v not consumed (add to dst %v, Rename, or SkipSrc)", sf.Name, srcType, dstType)) + } + + // Check for stale SkipSrc entries. + for _, name := range c.SkipSrc { + if _, ok := srcType.FieldByName(name); !ok { + errs = append(errs, fmt.Sprintf("stale SkipSrc entry %q: field does not exist on %v", name, srcType)) + } + } + + // Check for stale SkipDst entries. + for _, name := range c.SkipDst { + if _, ok := dstType.FieldByName(name); !ok { + errs = append(errs, fmt.Sprintf("stale SkipDst entry %q: field does not exist on %v", name, dstType)) + } + } + + // Check for stale Rename entries. + if c.Rename != nil { + for dstName, srcName := range c.Rename { + if _, ok := dstType.FieldByName(dstName); !ok { + errs = append(errs, fmt.Sprintf("stale Rename key %q: field does not exist on %v", dstName, dstType)) + } + if _, ok := srcType.FieldByName(srcName); !ok { + errs = append(errs, fmt.Sprintf("stale Rename value %q: field does not exist on %v", srcName, srcType)) + } + } + } + + if len(errs) > 0 { + return fmt.Errorf("fieldcopy.Validate[%v → %v]:\n %s", srcType, dstType, strings.Join(errs, "\n ")) + } + return nil +} + +func toSet(ss []string) map[string]bool { + m := make(map[string]bool, len(ss)) + for _, s := range ss { + m[s] = true + } + return m +} diff --git a/libs/structs/fieldcopy/fieldcopy_test.go b/libs/structs/fieldcopy/fieldcopy_test.go new file mode 100644 index 0000000000..c70dce52c3 --- /dev/null +++ b/libs/structs/fieldcopy/fieldcopy_test.go @@ -0,0 +1,442 @@ +package fieldcopy_test + +import ( + "testing" + + "github.com/databricks/cli/libs/structs/fieldcopy" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type srcBasic struct { + Name string + Age int + Email string + private string //nolint:unused +} + +type dstBasic struct { + Name string + Age int + Email string +} + +func TestDoBasicCopy(t *testing.T) { + c := fieldcopy.Copy[srcBasic, dstBasic]{} + src := srcBasic{Name: "alice", Age: 30, Email: "a@b.c"} + dst := c.Do(&src) + assert.Equal(t, "alice", dst.Name) + assert.Equal(t, 30, dst.Age) + assert.Equal(t, "a@b.c", dst.Email) +} + +func TestDoCachedSecondCall(t *testing.T) { + c := fieldcopy.Copy[srcBasic, dstBasic]{} + src1 := srcBasic{Name: "alice", Age: 30, Email: "a@b.c"} + dst1 := c.Do(&src1) + assert.Equal(t, "alice", dst1.Name) + + src2 := srcBasic{Name: "bob", Age: 25, Email: "b@c.d"} + dst2 := c.Do(&src2) + assert.Equal(t, "bob", dst2.Name) + assert.Equal(t, 25, dst2.Age) +} + +type srcWithExtra struct { + Name string + Age int + Extra string +} + +type dstSmall struct { + Name string + Age int +} + +func TestDoSkipSrc(t *testing.T) { + c := fieldcopy.Copy[srcWithExtra, dstSmall]{ + SkipSrc: []string{"Extra"}, + } + src := srcWithExtra{Name: "alice", Age: 30, Extra: "ignored"} + dst := c.Do(&src) + assert.Equal(t, "alice", dst.Name) + assert.Equal(t, 30, dst.Age) +} + +type dstWithDefault struct { + Name string + Age int + Default string +} + +func TestDoSkipDst(t *testing.T) { + c := fieldcopy.Copy[srcBasic, dstWithDefault]{ + SkipSrc: []string{"Email"}, + SkipDst: []string{"Default"}, + } + src := srcBasic{Name: "alice", Age: 30, Email: "a@b.c"} + dst := c.Do(&src) + assert.Equal(t, "alice", dst.Name) + assert.Equal(t, 30, dst.Age) + assert.Equal(t, "", dst.Default) +} + +type srcRenamed struct { + FullName string + Age int +} + +type dstRenamed struct { + Name string + Age int +} + +func TestDoRename(t *testing.T) { + c := fieldcopy.Copy[srcRenamed, dstRenamed]{ + Rename: map[string]string{"Name": "FullName"}, + } + src := srcRenamed{FullName: "alice", Age: 30} + dst := c.Do(&src) + assert.Equal(t, "alice", dst.Name) + assert.Equal(t, 30, dst.Age) +} + +func TestValidateBasicPass(t *testing.T) { + c := fieldcopy.Copy[srcBasic, dstBasic]{} + require.NoError(t, c.Validate()) +} + +func TestValidateWithSkips(t *testing.T) { + c := fieldcopy.Copy[srcWithExtra, dstWithDefault]{ + SkipSrc: []string{"Extra"}, + SkipDst: []string{"Default"}, + } + require.NoError(t, c.Validate()) +} + +func TestValidateWithRename(t *testing.T) { + c := fieldcopy.Copy[srcRenamed, dstRenamed]{ + Rename: map[string]string{"Name": "FullName"}, + } + require.NoError(t, c.Validate()) +} + +func TestValidateUnhandledDstField(t *testing.T) { + c := fieldcopy.Copy[dstSmall, dstWithDefault]{} + err := c.Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), `dst field "Default"`) + assert.Contains(t, err.Error(), "SkipDst") +} + +func TestValidateUnconsumedSrcField(t *testing.T) { + c := fieldcopy.Copy[srcWithExtra, dstSmall]{} + err := c.Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), `src field "Extra"`) + assert.Contains(t, err.Error(), "SkipSrc") +} + +func TestValidateStaleSkipSrc(t *testing.T) { + c := fieldcopy.Copy[srcBasic, dstBasic]{ + SkipSrc: []string{"NonExistent"}, + } + err := c.Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), `stale SkipSrc entry "NonExistent"`) +} + +func TestValidateStaleSkipDst(t *testing.T) { + c := fieldcopy.Copy[srcBasic, dstBasic]{ + SkipDst: []string{"NonExistent"}, + } + err := c.Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), `stale SkipDst entry "NonExistent"`) +} + +func TestValidateStaleRenameKey(t *testing.T) { + c := fieldcopy.Copy[srcRenamed, dstRenamed]{ + Rename: map[string]string{ + "Name": "FullName", + "NonExistent": "Age", + }, + } + err := c.Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), `stale Rename key "NonExistent"`) +} + +func TestValidateStaleRenameValue(t *testing.T) { + c := fieldcopy.Copy[srcRenamed, dstRenamed]{ + Rename: map[string]string{ + "Name": "NonExistent", + }, + } + err := c.Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), `stale Rename value "NonExistent"`) +} + +type srcTypeMismatch struct { + Name string + Age string // string instead of int +} + +func TestValidateTypeMismatch(t *testing.T) { + c := fieldcopy.Copy[srcTypeMismatch, dstBasic]{ + SkipDst: []string{"Age", "Email"}, + } + require.NoError(t, c.Validate()) + + // Without SkipDst, the type mismatch is caught. + c2 := fieldcopy.Copy[srcTypeMismatch, dstBasic]{ + SkipDst: []string{"Email"}, + } + err := c2.Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), `dst field "Age"`) + assert.Contains(t, err.Error(), "not assignable") +} + +type srcPointer struct { + Name string + Items *[]string +} + +type dstPointer struct { + Name string + Items *[]string +} + +func TestDoPointerFields(t *testing.T) { + c := fieldcopy.Copy[srcPointer, dstPointer]{} + items := []string{"a", "b"} + src := srcPointer{Name: "alice", Items: &items} + dst := c.Do(&src) + assert.Equal(t, "alice", dst.Name) + require.NotNil(t, dst.Items) + assert.Equal(t, []string{"a", "b"}, *dst.Items) + // Pointer is shared (shallow copy). + assert.Same(t, src.Items, dst.Items) +} + +func TestDoNilPointerFields(t *testing.T) { + c := fieldcopy.Copy[srcPointer, dstPointer]{} + src := srcPointer{Name: "alice", Items: nil} + dst := c.Do(&src) + assert.Equal(t, "alice", dst.Name) + assert.Nil(t, dst.Items) +} + +type srcMap struct { + Tags map[string]string +} + +type dstMap struct { + Tags map[string]string +} + +func TestDoMapFields(t *testing.T) { + c := fieldcopy.Copy[srcMap, dstMap]{} + src := srcMap{Tags: map[string]string{"k": "v"}} + dst := c.Do(&src) + assert.Equal(t, map[string]string{"k": "v"}, dst.Tags) + // Map is shared (shallow copy). + src.Tags["k2"] = "v2" + assert.Equal(t, "v2", dst.Tags["k2"]) +} + +type srcSlice struct { + Items []string +} + +type dstSlice struct { + Items []string +} + +func TestDoSliceFields(t *testing.T) { + c := fieldcopy.Copy[srcSlice, dstSlice]{} + src := srcSlice{Items: []string{"a", "b"}} + dst := c.Do(&src) + assert.Equal(t, []string{"a", "b"}, dst.Items) +} + +func TestDoZeroValue(t *testing.T) { + c := fieldcopy.Copy[srcBasic, dstBasic]{} + src := srcBasic{} + dst := c.Do(&src) + assert.Equal(t, "", dst.Name) + assert.Equal(t, 0, dst.Age) + assert.Equal(t, "", dst.Email) +} + +type srcBool struct { + Enabled bool + Name string +} + +type dstBool struct { + Enabled bool + Name string +} + +func TestDoBoolZeroValue(t *testing.T) { + c := fieldcopy.Copy[srcBool, dstBool]{} + src := srcBool{Enabled: false, Name: "test"} + dst := c.Do(&src) + assert.Equal(t, false, dst.Enabled) + assert.Equal(t, "test", dst.Name) +} + +type srcNested struct { + Name string + Config *nestedConfig +} + +type dstNested struct { + Name string + Config *nestedConfig +} + +type nestedConfig struct { + Value int +} + +func TestDoNestedStructPointer(t *testing.T) { + c := fieldcopy.Copy[srcNested, dstNested]{} + src := srcNested{Name: "alice", Config: &nestedConfig{Value: 42}} + dst := c.Do(&src) + assert.Equal(t, "alice", dst.Name) + require.NotNil(t, dst.Config) + assert.Equal(t, 42, dst.Config.Value) +} + +func TestValidateMultipleErrors(t *testing.T) { + c := fieldcopy.Copy[srcWithExtra, dstWithDefault]{ + // Missing: SkipSrc for Extra, SkipDst for Default + } + err := c.Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), `"Extra"`) + assert.Contains(t, err.Error(), `"Default"`) +} + +// Verify that private (unexported) fields are ignored. +type srcPrivate struct { + Name string + private int //nolint:unused +} + +type dstPrivate struct { + Name string + private int //nolint:unused +} + +func TestValidateIgnoresUnexportedFields(t *testing.T) { + c := fieldcopy.Copy[srcPrivate, dstPrivate]{} + require.NoError(t, c.Validate()) + + dst := c.Do(&srcPrivate{Name: "test"}) + assert.Equal(t, "test", dst.Name) +} + +// ForceSendFields auto-handling tests. + +type srcFSF struct { + Name string + Age int + Extra string + ForceSendFields []string +} + +type dstFSF struct { + Name string + Age int + ForceSendFields []string +} + +func TestDoForceSendFieldsAutoFiltered(t *testing.T) { + c := fieldcopy.Copy[srcFSF, dstFSF]{ + SkipSrc: []string{"Extra"}, + } + src := srcFSF{ + Name: "alice", + Age: 30, + ForceSendFields: []string{"Name", "Age", "Extra"}, + } + dst := c.Do(&src) + assert.Equal(t, "alice", dst.Name) + assert.Equal(t, 30, dst.Age) + // "Extra" should be filtered out since it doesn't exist on dstFSF. + assert.Equal(t, []string{"Name", "Age"}, dst.ForceSendFields) +} + +func TestDoForceSendFieldsNil(t *testing.T) { + c := fieldcopy.Copy[srcFSF, dstFSF]{ + SkipSrc: []string{"Extra"}, + } + src := srcFSF{Name: "alice", ForceSendFields: nil} + dst := c.Do(&src) + assert.Equal(t, "alice", dst.Name) + assert.Nil(t, dst.ForceSendFields) +} + +func TestDoForceSendFieldsEmpty(t *testing.T) { + c := fieldcopy.Copy[srcFSF, dstFSF]{ + SkipSrc: []string{"Extra"}, + } + src := srcFSF{Name: "alice", ForceSendFields: []string{}} + dst := c.Do(&src) + assert.Nil(t, dst.ForceSendFields) +} + +func TestDoForceSendFieldsExcludesSkipDst(t *testing.T) { + c := fieldcopy.Copy[srcFSF, dstFSF]{ + SkipSrc: []string{"Extra"}, + SkipDst: []string{"Age"}, + } + src := srcFSF{ + Name: "alice", + Age: 30, + ForceSendFields: []string{"Name", "Age"}, + } + dst := c.Do(&src) + // "Age" should be filtered out since it's in SkipDst. + assert.Equal(t, []string{"Name"}, dst.ForceSendFields) + // Age should NOT be copied (it's in SkipDst). + assert.Equal(t, 0, dst.Age) +} + +func TestDoForceSendFieldsAllFiltered(t *testing.T) { + c := fieldcopy.Copy[srcFSF, dstFSF]{ + SkipSrc: []string{"Extra"}, + } + src := srcFSF{ForceSendFields: []string{"Extra", "NonExistent"}} + dst := c.Do(&src) + // All entries filtered out → nil. + assert.Nil(t, dst.ForceSendFields) +} + +func TestValidateForceSendFieldsAutoHandled(t *testing.T) { + c := fieldcopy.Copy[srcFSF, dstFSF]{ + SkipSrc: []string{"Extra"}, + } + // ForceSendFields should not need to be in SkipSrc or SkipDst. + require.NoError(t, c.Validate()) +} + +func TestDoForceSendFieldsManualWhenInSkipDst(t *testing.T) { + c := fieldcopy.Copy[srcFSF, dstFSF]{ + SkipSrc: []string{"Extra", "ForceSendFields"}, + SkipDst: []string{"ForceSendFields"}, + } + src := srcFSF{ + Name: "alice", + ForceSendFields: []string{"Name", "Extra"}, + } + dst := c.Do(&src) + // ForceSendFields in SkipDst → not auto-handled, left at nil. + assert.Nil(t, dst.ForceSendFields) + require.NoError(t, c.Validate()) +} From 81ac7ba3a3314c26fee1b6baa905cd2c67f6ef7a Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 23 Mar 2026 15:04:07 +0100 Subject: [PATCH 02/16] Migrate job, pipeline, and model serving endpoint to fieldcopy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - job: makeCreateJob (JobSettings → CreateJob) - pipeline: makePipelineRemote (PipelineSpec → CreatePipeline), pipelineRemoteCopy (GetPipelineResponse → PipelineRemote), DoUpdate (CreatePipeline → EditPipeline) - model_serving_endpoint: autoCaptureConfigOutputToInput, servedEntitiesOutputToInput, RemapState Co-authored-by: Isaac --- bundle/direct/dresources/fieldcopy_test.go | 11 ++ bundle/direct/dresources/job.go | 43 ++---- .../dresources/model_serving_endpoint.go | 89 ++++++------ bundle/direct/dresources/pipeline.go | 128 ++++++------------ 4 files changed, 104 insertions(+), 167 deletions(-) diff --git a/bundle/direct/dresources/fieldcopy_test.go b/bundle/direct/dresources/fieldcopy_test.go index ef01c2c589..2c06ee00ff 100644 --- a/bundle/direct/dresources/fieldcopy_test.go +++ b/bundle/direct/dresources/fieldcopy_test.go @@ -8,9 +8,20 @@ import ( func TestFieldCopyValidate(t *testing.T) { copies := []interface{ Validate() error }{ + // cluster &clusterRemapCopy, &clusterCreateCopy, &clusterEditCopy, + // job + &jobCreateCopy, + // pipeline + &pipelineSpecCopy, + &pipelineRemoteCopy, + &pipelineEditCopy, + // model serving endpoint + &autoCaptureConfigCopy, + &servedEntityCopy, + &servingRemapCopy, } for _, c := range copies { require.NoError(t, c.Validate()) diff --git a/bundle/direct/dresources/job.go b/bundle/direct/dresources/job.go index ba3e932b34..48ad03514e 100644 --- a/bundle/direct/dresources/job.go +++ b/bundle/direct/dresources/job.go @@ -6,7 +6,7 @@ import ( "strconv" "github.com/databricks/cli/bundle/config/resources" - "github.com/databricks/cli/libs/utils" + "github.com/databricks/cli/libs/structs/fieldcopy" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/marshal" "github.com/databricks/databricks-sdk-go/service/jobs" @@ -128,40 +128,15 @@ func (r *ResourceJob) DoDelete(ctx context.Context, id string) error { return r.client.Jobs.DeleteByJobId(ctx, idInt) } -func makeCreateJob(config jobs.JobSettings) (jobs.CreateJob, error) { - // Note, exhaustruct linter validates that all off CreateJob fields are initialized. - // We don't have linter that validates that all of config fields are used. - result := jobs.CreateJob{ - AccessControlList: nil, // Not supported by DABs - BudgetPolicyId: config.BudgetPolicyId, - Continuous: config.Continuous, - Deployment: config.Deployment, - Description: config.Description, - EditMode: config.EditMode, - EmailNotifications: config.EmailNotifications, - Environments: config.Environments, - Format: config.Format, - GitSource: config.GitSource, - Health: config.Health, - JobClusters: config.JobClusters, - MaxConcurrentRuns: config.MaxConcurrentRuns, - Name: config.Name, - NotificationSettings: config.NotificationSettings, - Parameters: config.Parameters, - PerformanceTarget: config.PerformanceTarget, - Queue: config.Queue, - RunAs: config.RunAs, - Schedule: config.Schedule, - Tags: config.Tags, - Tasks: config.Tasks, - TimeoutSeconds: config.TimeoutSeconds, - Trigger: config.Trigger, - UsagePolicyId: config.UsagePolicyId, - WebhookNotifications: config.WebhookNotifications, - ForceSendFields: utils.FilterFields[jobs.CreateJob](config.ForceSendFields, "AccessControlList"), - } +// jobCreateCopy maps JobSettings (local state) to CreateJob (API request). +var jobCreateCopy = fieldcopy.Copy[jobs.JobSettings, jobs.CreateJob]{ + SkipDst: []string{ + "AccessControlList", // Not supported by DABs. + }, +} - return result, nil +func makeCreateJob(config jobs.JobSettings) (jobs.CreateJob, error) { + return jobCreateCopy.Do(&config), nil } func makeResetJob(config jobs.JobSettings, id string) (jobs.ResetJob, error) { diff --git a/bundle/direct/dresources/model_serving_endpoint.go b/bundle/direct/dresources/model_serving_endpoint.go index 8a8aa126b2..99d0db5f25 100644 --- a/bundle/direct/dresources/model_serving_endpoint.go +++ b/bundle/direct/dresources/model_serving_endpoint.go @@ -6,8 +6,8 @@ import ( "time" "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/libs/structs/fieldcopy" "github.com/databricks/cli/libs/structs/structpath" - "github.com/databricks/cli/libs/utils" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/service/serving" ) @@ -34,42 +34,36 @@ func (*ResourceModelServingEndpoint) PrepareState(input *resources.ModelServingE return &input.CreateServingEndpoint } +// autoCaptureConfigCopy maps AutoCaptureConfigOutput to AutoCaptureConfigInput. +var autoCaptureConfigCopy = fieldcopy.Copy[serving.AutoCaptureConfigOutput, serving.AutoCaptureConfigInput]{ + SkipSrc: []string{ + "State", + }, +} + func autoCaptureConfigOutputToInput(output *serving.AutoCaptureConfigOutput) *serving.AutoCaptureConfigInput { if output == nil { return nil } - return &serving.AutoCaptureConfigInput{ - CatalogName: output.CatalogName, - SchemaName: output.SchemaName, - TableNamePrefix: output.TableNamePrefix, - Enabled: output.Enabled, - ForceSendFields: utils.FilterFields[serving.AutoCaptureConfigInput](output.ForceSendFields), - } + result := autoCaptureConfigCopy.Do(output) + return &result +} + +// servedEntityCopy maps ServedEntityOutput to ServedEntityInput. +var servedEntityCopy = fieldcopy.Copy[serving.ServedEntityOutput, serving.ServedEntityInput]{ + SkipSrc: []string{ + "CreationTimestamp", + "Creator", + "FoundationModel", + "State", + }, } func servedEntitiesOutputToInput(output []serving.ServedEntityOutput) []serving.ServedEntityInput { entities := make([]serving.ServedEntityInput, len(output)) for i, entity := range output { - entities[i] = serving.ServedEntityInput{ - EntityName: entity.EntityName, - EntityVersion: entity.EntityVersion, - EnvironmentVars: entity.EnvironmentVars, - ExternalModel: entity.ExternalModel, - InstanceProfileArn: entity.InstanceProfileArn, - MaxProvisionedConcurrency: entity.MaxProvisionedConcurrency, - MaxProvisionedThroughput: entity.MaxProvisionedThroughput, - MinProvisionedConcurrency: entity.MinProvisionedConcurrency, - MinProvisionedThroughput: entity.MinProvisionedThroughput, - Name: entity.Name, - ProvisionedModelUnits: entity.ProvisionedModelUnits, - ScaleToZeroEnabled: entity.ScaleToZeroEnabled, - WorkloadSize: entity.WorkloadSize, - WorkloadType: entity.WorkloadType, - BurstScalingEnabled: entity.BurstScalingEnabled, - ForceSendFields: utils.FilterFields[serving.ServedEntityInput](entity.ForceSendFields), - } + entities[i] = servedEntityCopy.Do(&entity) } - return entities } @@ -86,25 +80,32 @@ func configOutputToInput(output *serving.EndpointCoreConfigOutput) *serving.Endp } } +// servingRemapCopy maps ServingEndpointDetailed (remote GET response) to CreateServingEndpoint (local state). +var servingRemapCopy = fieldcopy.Copy[serving.ServingEndpointDetailed, serving.CreateServingEndpoint]{ + SkipSrc: []string{ + "Config", + "CreationTimestamp", + "Creator", + "DataPlaneInfo", + "EndpointUrl", + "Id", + "LastUpdatedTimestamp", + "PendingConfig", + "PermissionLevel", + "State", + "Task", + }, + SkipDst: []string{ + "Config", // Requires type conversion via configOutputToInput. + "RateLimits", // Deprecated field, not returned by API. + }, +} + func (*ResourceModelServingEndpoint) RemapState(state *RefreshOutput) *serving.CreateServingEndpoint { details := state.EndpointDetails - // Map the remote state (ServingEndpointDetailed) to the local state (CreateServingEndpoint) - // for proper comparison during diff calculation - return &serving.CreateServingEndpoint{ - AiGateway: details.AiGateway, - BudgetPolicyId: details.BudgetPolicyId, - Config: configOutputToInput(details.Config), - Description: details.Description, - EmailNotifications: details.EmailNotifications, - Name: details.Name, - RouteOptimized: details.RouteOptimized, - Tags: details.Tags, - ForceSendFields: utils.FilterFields[serving.CreateServingEndpoint](details.ForceSendFields), - - // Rate limits are a deprecated field that are not returned by the API on GET calls. Thus we map them to nil. - // TODO(shreyas): Add a warning when users try setting top level rate limits. - RateLimits: nil, - } + result := servingRemapCopy.Do(details) + result.Config = configOutputToInput(details.Config) + return &result } type RefreshOutput struct { diff --git a/bundle/direct/dresources/pipeline.go b/bundle/direct/dresources/pipeline.go index 30d2ab5313..f8da5bfd9f 100644 --- a/bundle/direct/dresources/pipeline.go +++ b/bundle/direct/dresources/pipeline.go @@ -4,7 +4,7 @@ import ( "context" "github.com/databricks/cli/bundle/config/resources" - "github.com/databricks/cli/libs/utils" + "github.com/databricks/cli/libs/structs/fieldcopy" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/marshal" "github.com/databricks/databricks-sdk-go/service/pipelines" @@ -65,61 +65,34 @@ func (r *ResourcePipeline) DoRead(ctx context.Context, id string) (*PipelineRemo return makePipelineRemote(resp), nil } +// pipelineSpecCopy maps PipelineSpec (from GET response) to CreatePipeline (local state). +var pipelineSpecCopy = fieldcopy.Copy[pipelines.PipelineSpec, pipelines.CreatePipeline]{ + SkipDst: []string{ + "AllowDuplicateNames", // Request-only field, not in PipelineSpec. + "DryRun", // Request-only field, not in PipelineSpec. + "RunAs", // Pulled from GetPipelineResponse in post-processing. + }, +} + +// pipelineRemoteCopy maps GetPipelineResponse to PipelineRemote extra fields. +var pipelineRemoteCopy = fieldcopy.Copy[pipelines.GetPipelineResponse, PipelineRemote]{ + SkipSrc: []string{ + "Name", + "RunAs", + "Spec", + }, + SkipDst: []string{ + "CreatePipeline", // Populated separately from Spec. + }, +} + func makePipelineRemote(p *pipelines.GetPipelineResponse) *PipelineRemote { - var createPipeline pipelines.CreatePipeline + remote := pipelineRemoteCopy.Do(p) if p.Spec != nil { - spec := p.Spec - createPipeline = pipelines.CreatePipeline{ - // Note: AllowDuplicateNames and DryRun are not in PipelineSpec, - // they are request-only fields, so they stay at their zero values. - AllowDuplicateNames: false, - BudgetPolicyId: spec.BudgetPolicyId, - Catalog: spec.Catalog, - Channel: spec.Channel, - Clusters: spec.Clusters, - Configuration: spec.Configuration, - Continuous: spec.Continuous, - Deployment: spec.Deployment, - Development: spec.Development, - DryRun: false, - Edition: spec.Edition, - Environment: spec.Environment, - EventLog: spec.EventLog, - Filters: spec.Filters, - GatewayDefinition: spec.GatewayDefinition, - Id: spec.Id, - IngestionDefinition: spec.IngestionDefinition, - Libraries: spec.Libraries, - Name: spec.Name, - Notifications: spec.Notifications, - Photon: spec.Photon, - RestartWindow: spec.RestartWindow, - RootPath: spec.RootPath, - RunAs: p.RunAs, - Schema: spec.Schema, - Serverless: spec.Serverless, - Storage: spec.Storage, - Tags: spec.Tags, - Target: spec.Target, - Trigger: spec.Trigger, - UsagePolicyId: spec.UsagePolicyId, - ForceSendFields: utils.FilterFields[pipelines.CreatePipeline](spec.ForceSendFields, "AllowDuplicateNames", "DryRun", "RunAs"), - } - } - return &PipelineRemote{ - CreatePipeline: createPipeline, - Cause: p.Cause, - ClusterId: p.ClusterId, - CreatorUserName: p.CreatorUserName, - EffectiveBudgetPolicyId: p.EffectiveBudgetPolicyId, - EffectivePublishingMode: p.EffectivePublishingMode, - Health: p.Health, - LastModified: p.LastModified, - LatestUpdates: p.LatestUpdates, - PipelineId: p.PipelineId, - RunAsUserName: p.RunAsUserName, - State: p.State, + remote.CreatePipeline = pipelineSpecCopy.Do(p.Spec) + remote.CreatePipeline.RunAs = p.RunAs } + return &remote } func (r *ResourcePipeline) DoCreate(ctx context.Context, config *pipelines.CreatePipeline) (string, *PipelineRemote, error) { @@ -130,43 +103,20 @@ func (r *ResourcePipeline) DoCreate(ctx context.Context, config *pipelines.Creat return response.PipelineId, nil, nil } -func (r *ResourcePipeline) DoUpdate(ctx context.Context, id string, config *pipelines.CreatePipeline, _ Changes) (*PipelineRemote, error) { - request := pipelines.EditPipeline{ - AllowDuplicateNames: config.AllowDuplicateNames, - BudgetPolicyId: config.BudgetPolicyId, - Catalog: config.Catalog, - Channel: config.Channel, - Clusters: config.Clusters, - Configuration: config.Configuration, - Continuous: config.Continuous, - Deployment: config.Deployment, - Development: config.Development, - Edition: config.Edition, - Environment: config.Environment, - EventLog: config.EventLog, - ExpectedLastModified: 0, - Filters: config.Filters, - GatewayDefinition: config.GatewayDefinition, - Id: config.Id, - IngestionDefinition: config.IngestionDefinition, - Libraries: config.Libraries, - Name: config.Name, - Notifications: config.Notifications, - Photon: config.Photon, - RestartWindow: config.RestartWindow, - RootPath: config.RootPath, - RunAs: config.RunAs, - Schema: config.Schema, - Serverless: config.Serverless, - Storage: config.Storage, - Tags: config.Tags, - Target: config.Target, - Trigger: config.Trigger, - UsagePolicyId: config.UsagePolicyId, - PipelineId: id, - ForceSendFields: utils.FilterFields[pipelines.EditPipeline](config.ForceSendFields), - } +// pipelineEditCopy maps CreatePipeline (local state) to EditPipeline (API request). +var pipelineEditCopy = fieldcopy.Copy[pipelines.CreatePipeline, pipelines.EditPipeline]{ + SkipSrc: []string{ + "DryRun", // Request-only field, not in EditPipeline. + }, + SkipDst: []string{ + "ExpectedLastModified", // Left at zero. + "PipelineId", // Set from function parameter. + }, +} +func (r *ResourcePipeline) DoUpdate(ctx context.Context, id string, config *pipelines.CreatePipeline, _ Changes) (*PipelineRemote, error) { + request := pipelineEditCopy.Do(config) + request.PipelineId = id return nil, r.client.Pipelines.Update(ctx, request) } From 36274081a37ec1b0415581de02ccd53fa8996bef Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 23 Mar 2026 15:18:09 +0100 Subject: [PATCH 03/16] Replace SkipSrc/SkipDst with golden file report for field copy validation Remove caller-maintained skip lists from fieldcopy.Copy and replace Validate() with Report() that generates a human-readable summary of unmatched fields. The dresources test uses testdiff golden file approach so unmatched fields are auto-recorded and visible in diffs without requiring manual maintenance. Co-authored-by: Isaac --- bundle/direct/dresources/cluster.go | 39 +--- bundle/direct/dresources/fieldcopy_test.go | 19 +- bundle/direct/dresources/job.go | 16 +- .../dresources/model_serving_endpoint.go | 35 +--- bundle/direct/dresources/pipeline.go | 29 +-- .../dresources/testdata/fieldcopy_report.txt | 76 +++++++ libs/structs/fieldcopy/fieldcopy.go | 194 ++++++++---------- libs/structs/fieldcopy/fieldcopy_test.go | 189 ++++------------- 8 files changed, 237 insertions(+), 360 deletions(-) create mode 100644 bundle/direct/dresources/testdata/fieldcopy_report.txt diff --git a/bundle/direct/dresources/cluster.go b/bundle/direct/dresources/cluster.go index be8d675c6e..8da3dd68b6 100644 --- a/bundle/direct/dresources/cluster.go +++ b/bundle/direct/dresources/cluster.go @@ -32,32 +32,7 @@ func (r *ResourceCluster) PrepareState(input *resources.Cluster) *compute.Cluste } // clusterRemapCopy maps ClusterDetails (remote GET response) to ClusterSpec (local state). -var clusterRemapCopy = fieldcopy.Copy[compute.ClusterDetails, compute.ClusterSpec]{ - SkipSrc: []string{ - "ClusterCores", - "ClusterId", - "ClusterLogStatus", - "ClusterMemoryMb", - "ClusterSource", - "CreatorUserName", - "DefaultTags", - "Driver", - "Executors", - "JdbcPort", - "LastRestartedTime", - "LastStateLossTime", - "SparkContextId", - "Spec", - "StartTime", - "State", - "StateMessage", - "TerminatedTime", - "TerminationReason", - }, - SkipDst: []string{ - "ApplyPolicyDefaultValues", // Pulled from input.Spec in post-processing. - }, -} +var clusterRemapCopy = fieldcopy.Copy[compute.ClusterDetails, compute.ClusterSpec]{} func (r *ResourceCluster) RemapState(input *compute.ClusterDetails) *compute.ClusterSpec { spec := clusterRemapCopy.Do(input) @@ -145,11 +120,7 @@ func (r *ResourceCluster) OverrideChangeDesc(ctx context.Context, p *structpath. } // clusterCreateCopy maps ClusterSpec (local state) to CreateCluster (API request). -var clusterCreateCopy = fieldcopy.Copy[compute.ClusterSpec, compute.CreateCluster]{ - SkipDst: []string{ - "CloneFrom", // Not supported by DABs. - }, -} +var clusterCreateCopy = fieldcopy.Copy[compute.ClusterSpec, compute.CreateCluster]{} func makeCreateCluster(config *compute.ClusterSpec) compute.CreateCluster { create := clusterCreateCopy.Do(config) @@ -164,11 +135,7 @@ func makeCreateCluster(config *compute.ClusterSpec) compute.CreateCluster { } // clusterEditCopy maps ClusterSpec (local state) to EditCluster (API request). -var clusterEditCopy = fieldcopy.Copy[compute.ClusterSpec, compute.EditCluster]{ - SkipDst: []string{ - "ClusterId", // Set from function parameter. - }, -} +var clusterEditCopy = fieldcopy.Copy[compute.ClusterSpec, compute.EditCluster]{} func makeEditCluster(id string, config *compute.ClusterSpec) compute.EditCluster { edit := clusterEditCopy.Do(config) diff --git a/bundle/direct/dresources/fieldcopy_test.go b/bundle/direct/dresources/fieldcopy_test.go index 2c06ee00ff..4be29a3586 100644 --- a/bundle/direct/dresources/fieldcopy_test.go +++ b/bundle/direct/dresources/fieldcopy_test.go @@ -1,13 +1,19 @@ package dresources import ( + "context" + "path/filepath" + "strings" "testing" - "github.com/stretchr/testify/require" + "github.com/databricks/cli/libs/testdiff" ) -func TestFieldCopyValidate(t *testing.T) { - copies := []interface{ Validate() error }{ +func TestFieldCopyReport(t *testing.T) { + ctx := context.Background() + ctx, _ = testdiff.WithReplacementsMap(ctx) + + copies := []interface{ Report() string }{ // cluster &clusterRemapCopy, &clusterCreateCopy, @@ -23,7 +29,12 @@ func TestFieldCopyValidate(t *testing.T) { &servedEntityCopy, &servingRemapCopy, } + + var buf strings.Builder for _, c := range copies { - require.NoError(t, c.Validate()) + buf.WriteString(c.Report()) } + + goldenPath := filepath.Join("testdata", "fieldcopy_report.txt") + testdiff.AssertOutput(t, ctx, buf.String(), "fieldcopy report", goldenPath) } diff --git a/bundle/direct/dresources/job.go b/bundle/direct/dresources/job.go index 48ad03514e..50a59fa4b7 100644 --- a/bundle/direct/dresources/job.go +++ b/bundle/direct/dresources/job.go @@ -101,11 +101,7 @@ func makeJobRemote(job *jobs.Job) *JobRemote { } func (r *ResourceJob) DoCreate(ctx context.Context, config *jobs.JobSettings) (string, *JobRemote, error) { - request, err := makeCreateJob(*config) - if err != nil { - return "", nil, err - } - response, err := r.client.Jobs.Create(ctx, request) + response, err := r.client.Jobs.Create(ctx, jobCreateCopy.Do(config)) if err != nil { return "", nil, err } @@ -129,15 +125,7 @@ func (r *ResourceJob) DoDelete(ctx context.Context, id string) error { } // jobCreateCopy maps JobSettings (local state) to CreateJob (API request). -var jobCreateCopy = fieldcopy.Copy[jobs.JobSettings, jobs.CreateJob]{ - SkipDst: []string{ - "AccessControlList", // Not supported by DABs. - }, -} - -func makeCreateJob(config jobs.JobSettings) (jobs.CreateJob, error) { - return jobCreateCopy.Do(&config), nil -} +var jobCreateCopy = fieldcopy.Copy[jobs.JobSettings, jobs.CreateJob]{} func makeResetJob(config jobs.JobSettings, id string) (jobs.ResetJob, error) { idInt, err := parseJobID(id) diff --git a/bundle/direct/dresources/model_serving_endpoint.go b/bundle/direct/dresources/model_serving_endpoint.go index 99d0db5f25..cbb94cbf73 100644 --- a/bundle/direct/dresources/model_serving_endpoint.go +++ b/bundle/direct/dresources/model_serving_endpoint.go @@ -35,11 +35,7 @@ func (*ResourceModelServingEndpoint) PrepareState(input *resources.ModelServingE } // autoCaptureConfigCopy maps AutoCaptureConfigOutput to AutoCaptureConfigInput. -var autoCaptureConfigCopy = fieldcopy.Copy[serving.AutoCaptureConfigOutput, serving.AutoCaptureConfigInput]{ - SkipSrc: []string{ - "State", - }, -} +var autoCaptureConfigCopy = fieldcopy.Copy[serving.AutoCaptureConfigOutput, serving.AutoCaptureConfigInput]{} func autoCaptureConfigOutputToInput(output *serving.AutoCaptureConfigOutput) *serving.AutoCaptureConfigInput { if output == nil { @@ -50,14 +46,7 @@ func autoCaptureConfigOutputToInput(output *serving.AutoCaptureConfigOutput) *se } // servedEntityCopy maps ServedEntityOutput to ServedEntityInput. -var servedEntityCopy = fieldcopy.Copy[serving.ServedEntityOutput, serving.ServedEntityInput]{ - SkipSrc: []string{ - "CreationTimestamp", - "Creator", - "FoundationModel", - "State", - }, -} +var servedEntityCopy = fieldcopy.Copy[serving.ServedEntityOutput, serving.ServedEntityInput]{} func servedEntitiesOutputToInput(output []serving.ServedEntityOutput) []serving.ServedEntityInput { entities := make([]serving.ServedEntityInput, len(output)) @@ -81,25 +70,7 @@ func configOutputToInput(output *serving.EndpointCoreConfigOutput) *serving.Endp } // servingRemapCopy maps ServingEndpointDetailed (remote GET response) to CreateServingEndpoint (local state). -var servingRemapCopy = fieldcopy.Copy[serving.ServingEndpointDetailed, serving.CreateServingEndpoint]{ - SkipSrc: []string{ - "Config", - "CreationTimestamp", - "Creator", - "DataPlaneInfo", - "EndpointUrl", - "Id", - "LastUpdatedTimestamp", - "PendingConfig", - "PermissionLevel", - "State", - "Task", - }, - SkipDst: []string{ - "Config", // Requires type conversion via configOutputToInput. - "RateLimits", // Deprecated field, not returned by API. - }, -} +var servingRemapCopy = fieldcopy.Copy[serving.ServingEndpointDetailed, serving.CreateServingEndpoint]{} func (*ResourceModelServingEndpoint) RemapState(state *RefreshOutput) *serving.CreateServingEndpoint { details := state.EndpointDetails diff --git a/bundle/direct/dresources/pipeline.go b/bundle/direct/dresources/pipeline.go index f8da5bfd9f..d7f2daaf20 100644 --- a/bundle/direct/dresources/pipeline.go +++ b/bundle/direct/dresources/pipeline.go @@ -66,25 +66,10 @@ func (r *ResourcePipeline) DoRead(ctx context.Context, id string) (*PipelineRemo } // pipelineSpecCopy maps PipelineSpec (from GET response) to CreatePipeline (local state). -var pipelineSpecCopy = fieldcopy.Copy[pipelines.PipelineSpec, pipelines.CreatePipeline]{ - SkipDst: []string{ - "AllowDuplicateNames", // Request-only field, not in PipelineSpec. - "DryRun", // Request-only field, not in PipelineSpec. - "RunAs", // Pulled from GetPipelineResponse in post-processing. - }, -} +var pipelineSpecCopy = fieldcopy.Copy[pipelines.PipelineSpec, pipelines.CreatePipeline]{} // pipelineRemoteCopy maps GetPipelineResponse to PipelineRemote extra fields. -var pipelineRemoteCopy = fieldcopy.Copy[pipelines.GetPipelineResponse, PipelineRemote]{ - SkipSrc: []string{ - "Name", - "RunAs", - "Spec", - }, - SkipDst: []string{ - "CreatePipeline", // Populated separately from Spec. - }, -} +var pipelineRemoteCopy = fieldcopy.Copy[pipelines.GetPipelineResponse, PipelineRemote]{} func makePipelineRemote(p *pipelines.GetPipelineResponse) *PipelineRemote { remote := pipelineRemoteCopy.Do(p) @@ -104,15 +89,7 @@ func (r *ResourcePipeline) DoCreate(ctx context.Context, config *pipelines.Creat } // pipelineEditCopy maps CreatePipeline (local state) to EditPipeline (API request). -var pipelineEditCopy = fieldcopy.Copy[pipelines.CreatePipeline, pipelines.EditPipeline]{ - SkipSrc: []string{ - "DryRun", // Request-only field, not in EditPipeline. - }, - SkipDst: []string{ - "ExpectedLastModified", // Left at zero. - "PipelineId", // Set from function parameter. - }, -} +var pipelineEditCopy = fieldcopy.Copy[pipelines.CreatePipeline, pipelines.EditPipeline]{} func (r *ResourcePipeline) DoUpdate(ctx context.Context, id string, config *pipelines.CreatePipeline, _ Changes) (*PipelineRemote, error) { request := pipelineEditCopy.Do(config) diff --git a/bundle/direct/dresources/testdata/fieldcopy_report.txt b/bundle/direct/dresources/testdata/fieldcopy_report.txt new file mode 100644 index 0000000000..f9f3b19cbb --- /dev/null +++ b/bundle/direct/dresources/testdata/fieldcopy_report.txt @@ -0,0 +1,76 @@ +compute.ClusterDetails → compute.ClusterSpec + src not copied: + - ClusterCores + - ClusterId + - ClusterLogStatus + - ClusterMemoryMb + - ClusterSource + - CreatorUserName + - DefaultTags + - Driver + - Executors + - JdbcPort + - LastRestartedTime + - LastStateLossTime + - SparkContextId + - Spec + - StartTime + - State + - StateMessage + - TerminatedTime + - TerminationReason + dst not set: + - ApplyPolicyDefaultValues +compute.ClusterSpec → compute.CreateCluster + dst not set: + - CloneFrom +compute.ClusterSpec → compute.EditCluster + dst not set: + - ClusterId +jobs.JobSettings → jobs.CreateJob + dst not set: + - AccessControlList +pipelines.PipelineSpec → pipelines.CreatePipeline + dst not set: + - AllowDuplicateNames + - DryRun + - RunAs +pipelines.GetPipelineResponse → dresources.PipelineRemote + src not copied: + - Name + - RunAs + - Spec + - ForceSendFields + dst not set: + - CreatePipeline +pipelines.CreatePipeline → pipelines.EditPipeline + src not copied: + - DryRun + dst not set: + - ExpectedLastModified + - PipelineId +serving.AutoCaptureConfigOutput → serving.AutoCaptureConfigInput + src not copied: + - State +serving.ServedEntityOutput → serving.ServedEntityInput + src not copied: + - CreationTimestamp + - Creator + - FoundationModel + - State +serving.ServingEndpointDetailed → serving.CreateServingEndpoint + src not copied: + - Config + - CreationTimestamp + - Creator + - DataPlaneInfo + - EndpointUrl + - Id + - LastUpdatedTimestamp + - PendingConfig + - PermissionLevel + - State + - Task + dst not set: + - Config + - RateLimits diff --git a/libs/structs/fieldcopy/fieldcopy.go b/libs/structs/fieldcopy/fieldcopy.go index 59349ff3c6..3b4b0967e4 100644 --- a/libs/structs/fieldcopy/fieldcopy.go +++ b/libs/structs/fieldcopy/fieldcopy.go @@ -1,15 +1,16 @@ // Package fieldcopy provides reflection-based struct-to-struct field copying -// with test-time completeness validation. Field mappings are computed once -// on first use and cached for subsequent calls. +// with test-time completeness validation via golden files. // +// Field mappings are computed once on first use and cached for subsequent calls. // If both Src and Dst have a ForceSendFields []string field, it is handled // automatically: source values are filtered to only include names of exported -// Dst fields that are being copied (not in SkipDst). +// Dst fields that are being copied. package fieldcopy import ( "fmt" "reflect" + "sort" "strings" "sync" ) @@ -18,17 +19,13 @@ const forceSendFieldsName = "ForceSendFields" // Copy describes a field-by-field mapping from Src to Dst struct type. // Fields with matching names and assignable types are copied automatically. +// Unmatched fields are left at zero and reported via [Copy.Report] for +// golden file testing. type Copy[Src, Dst any] struct { // Rename maps destination field name to source field name for fields // with different names in source and destination. Rename map[string]string - // SkipSrc lists source field names intentionally not copied. - SkipSrc []string - - // SkipDst lists destination field names intentionally left at zero value. - SkipDst []string - once sync.Once copyFn func(src *Src) Dst } @@ -51,39 +48,26 @@ func (c *Copy[Src, Dst]) build() func(*Src) Dst { srcType := reflect.TypeFor[Src]() dstType := reflect.TypeFor[Dst]() - skipDst := toSet(c.SkipDst) - - // Detect auto-handling of ForceSendFields: both types must have it as []string - // and it must not be in SkipDst (which would mean manual handling). + // Detect auto-handling of ForceSendFields: both types must have it as []string. var autoFSF bool var fsfSrcIdx, fsfDstIdx []int var validFSFNames map[string]bool stringSliceType := reflect.TypeFor[[]string]() - if !skipDst[forceSendFieldsName] { - dstFSF, dstOK := dstType.FieldByName(forceSendFieldsName) - srcFSF, srcOK := srcType.FieldByName(forceSendFieldsName) - if dstOK && srcOK && dstFSF.Type == stringSliceType && srcFSF.Type == stringSliceType { - autoFSF = true - fsfSrcIdx = srcFSF.Index - fsfDstIdx = dstFSF.Index - - // Build set of valid field names: exported dst fields that are being copied. - validFSFNames = make(map[string]bool) - for i := range dstType.NumField() { - f := dstType.Field(i) - if f.IsExported() && f.Name != forceSendFieldsName && !skipDst[f.Name] { - validFSFNames[f.Name] = true - } - } - } + dstFSF, dstOK := dstType.FieldByName(forceSendFieldsName) + srcFSF, srcOK := srcType.FieldByName(forceSendFieldsName) + if dstOK && srcOK && dstFSF.Type == stringSliceType && srcFSF.Type == stringSliceType { + autoFSF = true + fsfSrcIdx = srcFSF.Index + fsfDstIdx = dstFSF.Index + validFSFNames = make(map[string]bool) } var ops []fieldOp for i := range dstType.NumField() { df := dstType.Field(i) - if !df.IsExported() || skipDst[df.Name] { + if !df.IsExported() { continue } if autoFSF && df.Name == forceSendFieldsName { @@ -103,6 +87,9 @@ func (c *Copy[Src, Dst]) build() func(*Src) Dst { } ops = append(ops, fieldOp{dstIndex: df.Index, srcIndex: sf.Index}) + if autoFSF { + validFSFNames[df.Name] = true + } } return func(src *Src) Dst { @@ -131,51 +118,46 @@ func (c *Copy[Src, Dst]) build() func(*Src) Dst { } } -// Validate checks that the mapping is complete: -// - Every exported Dst field is either matched in Src, in Rename, or in SkipDst. -// - Every exported Src field is either matched in Dst, a Rename source, or in SkipSrc. -// - No stale entries in SkipSrc, SkipDst, or Rename. -// -// ForceSendFields is considered auto-handled when both types have it as []string -// and it is not in SkipDst. -func (c *Copy[Src, Dst]) Validate() error { +// Report returns a human-readable summary of unmatched fields for golden file testing. +// Fields that exist on Src but have no match on Dst are listed as "src not copied". +// Fields that exist on Dst but have no match on Src are listed as "dst not set". +func (c *Copy[Src, Dst]) Report() string { srcType := reflect.TypeFor[Src]() dstType := reflect.TypeFor[Dst]() + return c.report(srcType, dstType) +} - skipSrc := toSet(c.SkipSrc) - skipDst := toSet(c.SkipDst) +func (c *Copy[Src, Dst]) report(srcType, dstType reflect.Type) string { + // Build rename lookups. + renameTargets := make(map[string]bool) // src names used by Rename + if c.Rename != nil { + for _, srcName := range c.Rename { + renameTargets[srcName] = true + } + } // Detect auto-handled ForceSendFields. autoFSF := false stringSliceType := reflect.TypeFor[[]string]() - if !skipDst[forceSendFieldsName] { - dstFSF, dstOK := dstType.FieldByName(forceSendFieldsName) - srcFSF, srcOK := srcType.FieldByName(forceSendFieldsName) - if dstOK && srcOK && dstFSF.Type == stringSliceType && srcFSF.Type == stringSliceType { - autoFSF = true - } - } - - // Build set of rename targets (src field names used by Rename). - renameTargets := make(map[string]string) // src name → dst name - if c.Rename != nil { - for dstName, srcName := range c.Rename { - renameTargets[srcName] = dstName + if dstFSF, ok := dstType.FieldByName(forceSendFieldsName); ok { + if srcFSF, ok := srcType.FieldByName(forceSendFieldsName); ok { + if dstFSF.Type == stringSliceType && srcFSF.Type == stringSliceType { + autoFSF = true + } } } - var errs []string - - // Check every exported dst field is handled. + // Collect matched dst field names (including renames). + matchedDst := make(map[string]bool) + matchedSrc := make(map[string]bool) for i := range dstType.NumField() { df := dstType.Field(i) if !df.IsExported() { continue } - if skipDst[df.Name] { - continue - } if autoFSF && df.Name == forceSendFieldsName { + matchedDst[df.Name] = true + matchedSrc[df.Name] = true continue } @@ -187,72 +169,78 @@ func (c *Copy[Src, Dst]) Validate() error { } sf, ok := srcType.FieldByName(srcName) - if !ok { - errs = append(errs, fmt.Sprintf("dst field %q: no matching field %q on %v (add to Rename or SkipDst)", df.Name, srcName, srcType)) - continue - } - if !sf.Type.AssignableTo(df.Type) { - errs = append(errs, fmt.Sprintf("dst field %q: type %v not assignable from src type %v (add to SkipDst and handle in post-processing)", df.Name, df.Type, sf.Type)) + if ok && sf.Type.AssignableTo(df.Type) { + matchedDst[df.Name] = true + matchedSrc[srcName] = true } } - // Check every exported src field is consumed. + // Find unmatched src fields. + var unmatchedSrc []string for i := range srcType.NumField() { sf := srcType.Field(i) - if !sf.IsExported() { - continue - } - if skipSrc[sf.Name] { - continue - } - if autoFSF && sf.Name == forceSendFieldsName { - continue - } - if _, ok := renameTargets[sf.Name]; ok { + if !sf.IsExported() || matchedSrc[sf.Name] || renameTargets[sf.Name] { continue } - if _, ok := dstType.FieldByName(sf.Name); ok { - continue - } - errs = append(errs, fmt.Sprintf("src field %q on %v not consumed (add to dst %v, Rename, or SkipSrc)", sf.Name, srcType, dstType)) - } - - // Check for stale SkipSrc entries. - for _, name := range c.SkipSrc { - if _, ok := srcType.FieldByName(name); !ok { - errs = append(errs, fmt.Sprintf("stale SkipSrc entry %q: field does not exist on %v", name, srcType)) - } + unmatchedSrc = append(unmatchedSrc, sf.Name) } - // Check for stale SkipDst entries. - for _, name := range c.SkipDst { - if _, ok := dstType.FieldByName(name); !ok { - errs = append(errs, fmt.Sprintf("stale SkipDst entry %q: field does not exist on %v", name, dstType)) + // Find unmatched dst fields. + var unmatchedDst []string + for i := range dstType.NumField() { + df := dstType.Field(i) + if !df.IsExported() || matchedDst[df.Name] { + continue } + unmatchedDst = append(unmatchedDst, df.Name) } // Check for stale Rename entries. + var staleRenames []string if c.Rename != nil { for dstName, srcName := range c.Rename { + var issues []string if _, ok := dstType.FieldByName(dstName); !ok { - errs = append(errs, fmt.Sprintf("stale Rename key %q: field does not exist on %v", dstName, dstType)) + issues = append(issues, fmt.Sprintf("dst %q not found", dstName)) } if _, ok := srcType.FieldByName(srcName); !ok { - errs = append(errs, fmt.Sprintf("stale Rename value %q: field does not exist on %v", srcName, srcType)) + issues = append(issues, fmt.Sprintf("src %q not found", srcName)) + } + if len(issues) > 0 { + staleRenames = append(staleRenames, fmt.Sprintf("%s→%s (%s)", dstName, srcName, strings.Join(issues, ", "))) } } + sort.Strings(staleRenames) } - if len(errs) > 0 { - return fmt.Errorf("fieldcopy.Validate[%v → %v]:\n %s", srcType, dstType, strings.Join(errs, "\n ")) + var buf strings.Builder + fmt.Fprintf(&buf, "%v → %v", srcType, dstType) + + if len(unmatchedSrc) == 0 && len(unmatchedDst) == 0 && len(staleRenames) == 0 { + buf.WriteString(": all fields matched\n") + return buf.String() } - return nil -} -func toSet(ss []string) map[string]bool { - m := make(map[string]bool, len(ss)) - for _, s := range ss { - m[s] = true + buf.WriteString("\n") + + if len(unmatchedSrc) > 0 { + buf.WriteString(" src not copied:\n") + for _, name := range unmatchedSrc { + fmt.Fprintf(&buf, " - %s\n", name) + } + } + if len(unmatchedDst) > 0 { + buf.WriteString(" dst not set:\n") + for _, name := range unmatchedDst { + fmt.Fprintf(&buf, " - %s\n", name) + } } - return m + if len(staleRenames) > 0 { + buf.WriteString(" stale renames:\n") + for _, s := range staleRenames { + fmt.Fprintf(&buf, " - %s\n", s) + } + } + + return buf.String() } diff --git a/libs/structs/fieldcopy/fieldcopy_test.go b/libs/structs/fieldcopy/fieldcopy_test.go index c70dce52c3..7afd208536 100644 --- a/libs/structs/fieldcopy/fieldcopy_test.go +++ b/libs/structs/fieldcopy/fieldcopy_test.go @@ -53,10 +53,8 @@ type dstSmall struct { Age int } -func TestDoSkipSrc(t *testing.T) { - c := fieldcopy.Copy[srcWithExtra, dstSmall]{ - SkipSrc: []string{"Extra"}, - } +func TestDoUnmatchedFieldsIgnored(t *testing.T) { + c := fieldcopy.Copy[srcWithExtra, dstSmall]{} src := srcWithExtra{Name: "alice", Age: 30, Extra: "ignored"} dst := c.Do(&src) assert.Equal(t, "alice", dst.Name) @@ -69,11 +67,8 @@ type dstWithDefault struct { Default string } -func TestDoSkipDst(t *testing.T) { - c := fieldcopy.Copy[srcBasic, dstWithDefault]{ - SkipSrc: []string{"Email"}, - SkipDst: []string{"Default"}, - } +func TestDoUnmatchedDstLeftAtZero(t *testing.T) { + c := fieldcopy.Copy[srcBasic, dstWithDefault]{} src := srcBasic{Name: "alice", Age: 30, Email: "a@b.c"} dst := c.Do(&src) assert.Equal(t, "alice", dst.Name) @@ -101,81 +96,44 @@ func TestDoRename(t *testing.T) { assert.Equal(t, 30, dst.Age) } -func TestValidateBasicPass(t *testing.T) { +func TestReportAllMatched(t *testing.T) { c := fieldcopy.Copy[srcBasic, dstBasic]{} - require.NoError(t, c.Validate()) -} - -func TestValidateWithSkips(t *testing.T) { - c := fieldcopy.Copy[srcWithExtra, dstWithDefault]{ - SkipSrc: []string{"Extra"}, - SkipDst: []string{"Default"}, - } - require.NoError(t, c.Validate()) -} - -func TestValidateWithRename(t *testing.T) { - c := fieldcopy.Copy[srcRenamed, dstRenamed]{ - Rename: map[string]string{"Name": "FullName"}, - } - require.NoError(t, c.Validate()) + report := c.Report() + assert.Contains(t, report, "all fields matched") } -func TestValidateUnhandledDstField(t *testing.T) { - c := fieldcopy.Copy[dstSmall, dstWithDefault]{} - err := c.Validate() - require.Error(t, err) - assert.Contains(t, err.Error(), `dst field "Default"`) - assert.Contains(t, err.Error(), "SkipDst") -} - -func TestValidateUnconsumedSrcField(t *testing.T) { +func TestReportUnmatchedSrc(t *testing.T) { c := fieldcopy.Copy[srcWithExtra, dstSmall]{} - err := c.Validate() - require.Error(t, err) - assert.Contains(t, err.Error(), `src field "Extra"`) - assert.Contains(t, err.Error(), "SkipSrc") + report := c.Report() + assert.Contains(t, report, "src not copied:") + assert.Contains(t, report, "- Extra") } -func TestValidateStaleSkipSrc(t *testing.T) { - c := fieldcopy.Copy[srcBasic, dstBasic]{ - SkipSrc: []string{"NonExistent"}, - } - err := c.Validate() - require.Error(t, err) - assert.Contains(t, err.Error(), `stale SkipSrc entry "NonExistent"`) +func TestReportUnmatchedDst(t *testing.T) { + c := fieldcopy.Copy[srcBasic, dstWithDefault]{} + report := c.Report() + assert.Contains(t, report, "dst not set:") + assert.Contains(t, report, "- Default") } -func TestValidateStaleSkipDst(t *testing.T) { - c := fieldcopy.Copy[srcBasic, dstBasic]{ - SkipDst: []string{"NonExistent"}, +func TestReportWithRename(t *testing.T) { + c := fieldcopy.Copy[srcRenamed, dstRenamed]{ + Rename: map[string]string{"Name": "FullName"}, } - err := c.Validate() - require.Error(t, err) - assert.Contains(t, err.Error(), `stale SkipDst entry "NonExistent"`) + report := c.Report() + assert.Contains(t, report, "all fields matched") } -func TestValidateStaleRenameKey(t *testing.T) { +func TestReportStaleRename(t *testing.T) { c := fieldcopy.Copy[srcRenamed, dstRenamed]{ Rename: map[string]string{ "Name": "FullName", "NonExistent": "Age", }, } - err := c.Validate() - require.Error(t, err) - assert.Contains(t, err.Error(), `stale Rename key "NonExistent"`) -} - -func TestValidateStaleRenameValue(t *testing.T) { - c := fieldcopy.Copy[srcRenamed, dstRenamed]{ - Rename: map[string]string{ - "Name": "NonExistent", - }, - } - err := c.Validate() - require.Error(t, err) - assert.Contains(t, err.Error(), `stale Rename value "NonExistent"`) + report := c.Report() + assert.Contains(t, report, "stale renames:") + assert.Contains(t, report, "NonExistent") } type srcTypeMismatch struct { @@ -183,20 +141,21 @@ type srcTypeMismatch struct { Age string // string instead of int } -func TestValidateTypeMismatch(t *testing.T) { - c := fieldcopy.Copy[srcTypeMismatch, dstBasic]{ - SkipDst: []string{"Age", "Email"}, - } - require.NoError(t, c.Validate()) +func TestDoTypeMismatchFieldSkipped(t *testing.T) { + c := fieldcopy.Copy[srcTypeMismatch, dstBasic]{} + src := srcTypeMismatch{Name: "alice", Age: "30"} + dst := c.Do(&src) + assert.Equal(t, "alice", dst.Name) + assert.Equal(t, 0, dst.Age) // not copied due to type mismatch + assert.Equal(t, "", dst.Email) // no match on src +} - // Without SkipDst, the type mismatch is caught. - c2 := fieldcopy.Copy[srcTypeMismatch, dstBasic]{ - SkipDst: []string{"Email"}, - } - err := c2.Validate() - require.Error(t, err) - assert.Contains(t, err.Error(), `dst field "Age"`) - assert.Contains(t, err.Error(), "not assignable") +func TestReportTypeMismatch(t *testing.T) { + c := fieldcopy.Copy[srcTypeMismatch, dstBasic]{} + report := c.Report() + // Age exists on both but types don't match, so it's unmatched on both sides. + assert.Contains(t, report, "src not copied:") + assert.Contains(t, report, "dst not set:") } type srcPointer struct { @@ -312,16 +271,6 @@ func TestDoNestedStructPointer(t *testing.T) { assert.Equal(t, 42, dst.Config.Value) } -func TestValidateMultipleErrors(t *testing.T) { - c := fieldcopy.Copy[srcWithExtra, dstWithDefault]{ - // Missing: SkipSrc for Extra, SkipDst for Default - } - err := c.Validate() - require.Error(t, err) - assert.Contains(t, err.Error(), `"Extra"`) - assert.Contains(t, err.Error(), `"Default"`) -} - // Verify that private (unexported) fields are ignored. type srcPrivate struct { Name string @@ -333,10 +282,8 @@ type dstPrivate struct { private int //nolint:unused } -func TestValidateIgnoresUnexportedFields(t *testing.T) { +func TestDoIgnoresUnexportedFields(t *testing.T) { c := fieldcopy.Copy[srcPrivate, dstPrivate]{} - require.NoError(t, c.Validate()) - dst := c.Do(&srcPrivate{Name: "test"}) assert.Equal(t, "test", dst.Name) } @@ -357,9 +304,7 @@ type dstFSF struct { } func TestDoForceSendFieldsAutoFiltered(t *testing.T) { - c := fieldcopy.Copy[srcFSF, dstFSF]{ - SkipSrc: []string{"Extra"}, - } + c := fieldcopy.Copy[srcFSF, dstFSF]{} src := srcFSF{ Name: "alice", Age: 30, @@ -373,9 +318,7 @@ func TestDoForceSendFieldsAutoFiltered(t *testing.T) { } func TestDoForceSendFieldsNil(t *testing.T) { - c := fieldcopy.Copy[srcFSF, dstFSF]{ - SkipSrc: []string{"Extra"}, - } + c := fieldcopy.Copy[srcFSF, dstFSF]{} src := srcFSF{Name: "alice", ForceSendFields: nil} dst := c.Do(&src) assert.Equal(t, "alice", dst.Name) @@ -383,60 +326,16 @@ func TestDoForceSendFieldsNil(t *testing.T) { } func TestDoForceSendFieldsEmpty(t *testing.T) { - c := fieldcopy.Copy[srcFSF, dstFSF]{ - SkipSrc: []string{"Extra"}, - } + c := fieldcopy.Copy[srcFSF, dstFSF]{} src := srcFSF{Name: "alice", ForceSendFields: []string{}} dst := c.Do(&src) assert.Nil(t, dst.ForceSendFields) } -func TestDoForceSendFieldsExcludesSkipDst(t *testing.T) { - c := fieldcopy.Copy[srcFSF, dstFSF]{ - SkipSrc: []string{"Extra"}, - SkipDst: []string{"Age"}, - } - src := srcFSF{ - Name: "alice", - Age: 30, - ForceSendFields: []string{"Name", "Age"}, - } - dst := c.Do(&src) - // "Age" should be filtered out since it's in SkipDst. - assert.Equal(t, []string{"Name"}, dst.ForceSendFields) - // Age should NOT be copied (it's in SkipDst). - assert.Equal(t, 0, dst.Age) -} - func TestDoForceSendFieldsAllFiltered(t *testing.T) { - c := fieldcopy.Copy[srcFSF, dstFSF]{ - SkipSrc: []string{"Extra"}, - } + c := fieldcopy.Copy[srcFSF, dstFSF]{} src := srcFSF{ForceSendFields: []string{"Extra", "NonExistent"}} dst := c.Do(&src) // All entries filtered out → nil. assert.Nil(t, dst.ForceSendFields) } - -func TestValidateForceSendFieldsAutoHandled(t *testing.T) { - c := fieldcopy.Copy[srcFSF, dstFSF]{ - SkipSrc: []string{"Extra"}, - } - // ForceSendFields should not need to be in SkipSrc or SkipDst. - require.NoError(t, c.Validate()) -} - -func TestDoForceSendFieldsManualWhenInSkipDst(t *testing.T) { - c := fieldcopy.Copy[srcFSF, dstFSF]{ - SkipSrc: []string{"Extra", "ForceSendFields"}, - SkipDst: []string{"ForceSendFields"}, - } - src := srcFSF{ - Name: "alice", - ForceSendFields: []string{"Name", "Extra"}, - } - dst := c.Do(&src) - // ForceSendFields in SkipDst → not auto-handled, left at nil. - assert.Nil(t, dst.ForceSendFields) - require.NoError(t, c.Validate()) -} From 603e4a23e28110c180de6e2125b6805bbde1bd50 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 23 Mar 2026 15:19:23 +0100 Subject: [PATCH 04/16] lint --- libs/structs/fieldcopy/fieldcopy_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/structs/fieldcopy/fieldcopy_test.go b/libs/structs/fieldcopy/fieldcopy_test.go index 7afd208536..cc448c90e5 100644 --- a/libs/structs/fieldcopy/fieldcopy_test.go +++ b/libs/structs/fieldcopy/fieldcopy_test.go @@ -146,7 +146,7 @@ func TestDoTypeMismatchFieldSkipped(t *testing.T) { src := srcTypeMismatch{Name: "alice", Age: "30"} dst := c.Do(&src) assert.Equal(t, "alice", dst.Name) - assert.Equal(t, 0, dst.Age) // not copied due to type mismatch + assert.Equal(t, 0, dst.Age) // not copied due to type mismatch assert.Equal(t, "", dst.Email) // no match on src } From f4d6e95f420e93dcadbb1cdb8a2db5bd0099d9f3 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 23 Mar 2026 15:38:43 +0100 Subject: [PATCH 05/16] Fix autoFSF for embedded structs and rename golden file Skip auto-handling of ForceSendFields when it is promoted from an embedded struct (len(Index) > 1), since fieldcopy only copies direct fields. Rename golden file to out.fieldcopy.txt. Co-authored-by: Isaac --- bundle/direct/dresources/fieldcopy_test.go | 2 +- ...fieldcopy_report.txt => out.fieldcopy.txt} | 0 libs/structs/fieldcopy/fieldcopy.go | 8 +++-- libs/structs/fieldcopy/fieldcopy_test.go | 36 +++++++++++++++++++ 4 files changed, 42 insertions(+), 4 deletions(-) rename bundle/direct/dresources/testdata/{fieldcopy_report.txt => out.fieldcopy.txt} (100%) diff --git a/bundle/direct/dresources/fieldcopy_test.go b/bundle/direct/dresources/fieldcopy_test.go index 4be29a3586..22a6f49300 100644 --- a/bundle/direct/dresources/fieldcopy_test.go +++ b/bundle/direct/dresources/fieldcopy_test.go @@ -35,6 +35,6 @@ func TestFieldCopyReport(t *testing.T) { buf.WriteString(c.Report()) } - goldenPath := filepath.Join("testdata", "fieldcopy_report.txt") + goldenPath := filepath.Join("testdata", "out.fieldcopy.txt") testdiff.AssertOutput(t, ctx, buf.String(), "fieldcopy report", goldenPath) } diff --git a/bundle/direct/dresources/testdata/fieldcopy_report.txt b/bundle/direct/dresources/testdata/out.fieldcopy.txt similarity index 100% rename from bundle/direct/dresources/testdata/fieldcopy_report.txt rename to bundle/direct/dresources/testdata/out.fieldcopy.txt diff --git a/libs/structs/fieldcopy/fieldcopy.go b/libs/structs/fieldcopy/fieldcopy.go index 3b4b0967e4..92142fd647 100644 --- a/libs/structs/fieldcopy/fieldcopy.go +++ b/libs/structs/fieldcopy/fieldcopy.go @@ -56,7 +56,9 @@ func (c *Copy[Src, Dst]) build() func(*Src) Dst { stringSliceType := reflect.TypeFor[[]string]() dstFSF, dstOK := dstType.FieldByName(forceSendFieldsName) srcFSF, srcOK := srcType.FieldByName(forceSendFieldsName) - if dstOK && srcOK && dstFSF.Type == stringSliceType && srcFSF.Type == stringSliceType { + // Only auto-handle when ForceSendFields is a direct (non-promoted) field on both types. + // Promoted fields from embedded structs have len(Index) > 1. + if dstOK && srcOK && len(dstFSF.Index) == 1 && len(srcFSF.Index) == 1 && dstFSF.Type == stringSliceType && srcFSF.Type == stringSliceType { autoFSF = true fsfSrcIdx = srcFSF.Index fsfDstIdx = dstFSF.Index @@ -136,12 +138,12 @@ func (c *Copy[Src, Dst]) report(srcType, dstType reflect.Type) string { } } - // Detect auto-handled ForceSendFields. + // Detect auto-handled ForceSendFields (direct fields only, not promoted from embedded structs). autoFSF := false stringSliceType := reflect.TypeFor[[]string]() if dstFSF, ok := dstType.FieldByName(forceSendFieldsName); ok { if srcFSF, ok := srcType.FieldByName(forceSendFieldsName); ok { - if dstFSF.Type == stringSliceType && srcFSF.Type == stringSliceType { + if len(dstFSF.Index) == 1 && len(srcFSF.Index) == 1 && dstFSF.Type == stringSliceType && srcFSF.Type == stringSliceType { autoFSF = true } } diff --git a/libs/structs/fieldcopy/fieldcopy_test.go b/libs/structs/fieldcopy/fieldcopy_test.go index cc448c90e5..643014ede8 100644 --- a/libs/structs/fieldcopy/fieldcopy_test.go +++ b/libs/structs/fieldcopy/fieldcopy_test.go @@ -339,3 +339,39 @@ func TestDoForceSendFieldsAllFiltered(t *testing.T) { // All entries filtered out → nil. assert.Nil(t, dst.ForceSendFields) } + +// Embedded struct: ForceSendFields promoted from embedded type should NOT +// trigger auto-handling, since we only copy direct fields. + +type InnerFSF struct { + Name string + ForceSendFields []string +} + +type outerDstFSF struct { + InnerFSF + Extra string +} + +func TestDoForceSendFieldsNotAutoHandledWhenPromoted(t *testing.T) { + c := fieldcopy.Copy[srcFSF, outerDstFSF]{} + src := srcFSF{ + Name: "alice", + Extra: "x", + ForceSendFields: []string{"Name", "Extra"}, + } + dst := c.Do(&src) + assert.Equal(t, "x", dst.Extra) + // ForceSendFields is promoted from InnerFSF — autoFSF should not activate. + // The embedded InnerFSF is not copied (type mismatch), so ForceSendFields stays nil. + assert.Nil(t, dst.ForceSendFields) +} + +func TestReportEmbeddedStructShowsAsField(t *testing.T) { + c := fieldcopy.Copy[srcFSF, outerDstFSF]{} + report := c.Report() + // The embedded struct itself appears as unmatched dst field. + assert.Contains(t, report, "- InnerFSF") + // ForceSendFields on src is not auto-handled, so it shows as unmatched. + assert.Contains(t, report, "- ForceSendFields") +} From bc80f4d64b827172ef4c28b9d6ea31df4325304a Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 23 Mar 2026 15:50:59 +0100 Subject: [PATCH 06/16] Move golden file to out.fieldcopy.txt (no testdata subdirectory) Co-authored-by: Isaac --- bundle/direct/dresources/fieldcopy_test.go | 4 +--- bundle/direct/dresources/{testdata => }/out.fieldcopy.txt | 0 2 files changed, 1 insertion(+), 3 deletions(-) rename bundle/direct/dresources/{testdata => }/out.fieldcopy.txt (100%) diff --git a/bundle/direct/dresources/fieldcopy_test.go b/bundle/direct/dresources/fieldcopy_test.go index 22a6f49300..4cc37becaa 100644 --- a/bundle/direct/dresources/fieldcopy_test.go +++ b/bundle/direct/dresources/fieldcopy_test.go @@ -2,7 +2,6 @@ package dresources import ( "context" - "path/filepath" "strings" "testing" @@ -35,6 +34,5 @@ func TestFieldCopyReport(t *testing.T) { buf.WriteString(c.Report()) } - goldenPath := filepath.Join("testdata", "out.fieldcopy.txt") - testdiff.AssertOutput(t, ctx, buf.String(), "fieldcopy report", goldenPath) + testdiff.AssertOutput(t, ctx, buf.String(), "fieldcopy report", "out.fieldcopy.txt") } diff --git a/bundle/direct/dresources/testdata/out.fieldcopy.txt b/bundle/direct/dresources/out.fieldcopy.txt similarity index 100% rename from bundle/direct/dresources/testdata/out.fieldcopy.txt rename to bundle/direct/dresources/out.fieldcopy.txt From 1c8f2d0fa7b40bda74b6900631c91c6f9fcf6ba0 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 23 Mar 2026 15:53:02 +0100 Subject: [PATCH 07/16] Migrate catalog, registered_model, external_location, quality_monitor, sql_warehouse to fieldcopy Replace manual field-by-field struct copying with fieldcopy.Copy for five more resource types. ForceSendFields is now auto-filtered. sql_warehouse requires post-processing for WarehouseType due to different named string types that aren't assignable. Co-authored-by: Isaac --- bundle/direct/dresources/catalog.go | 47 +++------- bundle/direct/dresources/external_location.go | 67 +++------------ bundle/direct/dresources/fieldcopy_test.go | 15 ++++ bundle/direct/dresources/out.fieldcopy.txt | 86 +++++++++++++++++++ bundle/direct/dresources/quality_monitor.go | 47 +++------- bundle/direct/dresources/registered_model.go | 56 +++--------- bundle/direct/dresources/sql_warehouse.go | 49 ++++------- 7 files changed, 164 insertions(+), 203 deletions(-) diff --git a/bundle/direct/dresources/catalog.go b/bundle/direct/dresources/catalog.go index bb1fb82490..30d5e06788 100644 --- a/bundle/direct/dresources/catalog.go +++ b/bundle/direct/dresources/catalog.go @@ -4,7 +4,7 @@ import ( "context" "github.com/databricks/cli/bundle/config/resources" - "github.com/databricks/cli/libs/utils" + "github.com/databricks/cli/libs/structs/fieldcopy" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/service/catalog" ) @@ -21,18 +21,12 @@ func (*ResourceCatalog) PrepareState(input *resources.Catalog) *catalog.CreateCa return &input.CreateCatalog } +// catalogRemapCopy maps CatalogInfo (remote GET response) to CreateCatalog (local state). +var catalogRemapCopy = fieldcopy.Copy[catalog.CatalogInfo, catalog.CreateCatalog]{} + func (*ResourceCatalog) RemapState(info *catalog.CatalogInfo) *catalog.CreateCatalog { - return &catalog.CreateCatalog{ - Comment: info.Comment, - ConnectionName: info.ConnectionName, - Name: info.Name, - Options: info.Options, - Properties: info.Properties, - ProviderName: info.ProviderName, - ShareName: info.ShareName, - StorageRoot: info.StorageRoot, - ForceSendFields: utils.FilterFields[catalog.CreateCatalog](info.ForceSendFields), - } + result := catalogRemapCopy.Do(info) + return &result } func (r *ResourceCatalog) DoRead(ctx context.Context, id string) (*catalog.CatalogInfo, error) { @@ -47,19 +41,13 @@ func (r *ResourceCatalog) DoCreate(ctx context.Context, config *catalog.CreateCa return response.Name, response, nil } +// catalogUpdateCopy maps CreateCatalog (local state) to UpdateCatalog (API request). +var catalogUpdateCopy = fieldcopy.Copy[catalog.CreateCatalog, catalog.UpdateCatalog]{} + // DoUpdate updates the catalog in place and returns remote state. func (r *ResourceCatalog) DoUpdate(ctx context.Context, id string, config *catalog.CreateCatalog, _ Changes) (*catalog.CatalogInfo, error) { - updateRequest := catalog.UpdateCatalog{ - Comment: config.Comment, - EnablePredictiveOptimization: "", // Not supported by DABs - IsolationMode: "", // Not supported by DABs - Name: id, - NewName: "", // Only set if name actually changes (see DoUpdateWithID) - Options: config.Options, - Owner: "", // Not supported by DABs - Properties: config.Properties, - ForceSendFields: utils.FilterFields[catalog.UpdateCatalog](config.ForceSendFields, "EnablePredictiveOptimization", "IsolationMode", "Owner"), - } + updateRequest := catalogUpdateCopy.Do(config) + updateRequest.Name = id response, err := r.client.Catalogs.Update(ctx, updateRequest) if err != nil { @@ -71,17 +59,8 @@ func (r *ResourceCatalog) DoUpdate(ctx context.Context, id string, config *catal // DoUpdateWithID updates the catalog and returns the new ID if the name changes. func (r *ResourceCatalog) DoUpdateWithID(ctx context.Context, id string, config *catalog.CreateCatalog) (string, *catalog.CatalogInfo, error) { - updateRequest := catalog.UpdateCatalog{ - Comment: config.Comment, - EnablePredictiveOptimization: "", // Not supported by DABs - IsolationMode: "", // Not supported by DABs - Name: id, - NewName: "", // Initialized below if needed - Options: config.Options, - Owner: "", // Not supported by DABs - Properties: config.Properties, - ForceSendFields: utils.FilterFields[catalog.UpdateCatalog](config.ForceSendFields, "EnablePredictiveOptimization", "IsolationMode", "Owner"), - } + updateRequest := catalogUpdateCopy.Do(config) + updateRequest.Name = id if config.Name != id { updateRequest.NewName = config.Name diff --git a/bundle/direct/dresources/external_location.go b/bundle/direct/dresources/external_location.go index ca6821611c..755a2ba7d5 100644 --- a/bundle/direct/dresources/external_location.go +++ b/bundle/direct/dresources/external_location.go @@ -4,7 +4,7 @@ import ( "context" "github.com/databricks/cli/bundle/config/resources" - "github.com/databricks/cli/libs/utils" + "github.com/databricks/cli/libs/structs/fieldcopy" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/service/catalog" ) @@ -21,22 +21,12 @@ func (*ResourceExternalLocation) PrepareState(input *resources.ExternalLocation) return &input.CreateExternalLocation } +// externalLocationRemapCopy maps ExternalLocationInfo (remote GET response) to CreateExternalLocation (local state). +var externalLocationRemapCopy = fieldcopy.Copy[catalog.ExternalLocationInfo, catalog.CreateExternalLocation]{} + func (*ResourceExternalLocation) RemapState(info *catalog.ExternalLocationInfo) *catalog.CreateExternalLocation { - return &catalog.CreateExternalLocation{ - Comment: info.Comment, - CredentialName: info.CredentialName, - // Output-only field mirrored into state to avoid churn in remapped config. - EffectiveEnableFileEvents: info.EffectiveEnableFileEvents, - EnableFileEvents: info.EnableFileEvents, - EncryptionDetails: info.EncryptionDetails, - Fallback: info.Fallback, - FileEventQueue: info.FileEventQueue, - Name: info.Name, - ReadOnly: info.ReadOnly, - SkipValidation: false, // This is an input-only parameter, never returned by API - Url: info.Url, - ForceSendFields: utils.FilterFields[catalog.CreateExternalLocation](info.ForceSendFields), - } + result := externalLocationRemapCopy.Do(info) + return &result } func (r *ResourceExternalLocation) DoRead(ctx context.Context, id string) (*catalog.ExternalLocationInfo, error) { @@ -51,52 +41,21 @@ func (r *ResourceExternalLocation) DoCreate(ctx context.Context, config *catalog return response.Name, response, nil } +// externalLocationUpdateCopy maps CreateExternalLocation (local state) to UpdateExternalLocation (API request). +var externalLocationUpdateCopy = fieldcopy.Copy[catalog.CreateExternalLocation, catalog.UpdateExternalLocation]{} + // DoUpdate updates the external location in place and returns remote state. func (r *ResourceExternalLocation) DoUpdate(ctx context.Context, id string, config *catalog.CreateExternalLocation, _ Changes) (*catalog.ExternalLocationInfo, error) { - updateRequest := catalog.UpdateExternalLocation{ - Comment: config.Comment, - CredentialName: config.CredentialName, - // Output-only field; never sent in update payload. - EffectiveEnableFileEvents: false, - EnableFileEvents: config.EnableFileEvents, - EncryptionDetails: config.EncryptionDetails, - Fallback: config.Fallback, - FileEventQueue: config.FileEventQueue, - Force: false, - IsolationMode: "", // Not supported by DABs - Name: id, - NewName: "", // Only set if name actually changes (see DoUpdateWithID) - Owner: "", // Not supported by DABs - ReadOnly: config.ReadOnly, - SkipValidation: config.SkipValidation, - Url: config.Url, - ForceSendFields: utils.FilterFields[catalog.UpdateExternalLocation](config.ForceSendFields, "IsolationMode", "Owner"), - } + updateRequest := externalLocationUpdateCopy.Do(config) + updateRequest.Name = id return r.client.ExternalLocations.Update(ctx, updateRequest) } // DoUpdateWithID updates the external location and returns the new ID if the name changes. func (r *ResourceExternalLocation) DoUpdateWithID(ctx context.Context, id string, config *catalog.CreateExternalLocation) (string, *catalog.ExternalLocationInfo, error) { - updateRequest := catalog.UpdateExternalLocation{ - Comment: config.Comment, - CredentialName: config.CredentialName, - // Output-only field; never sent in update payload. - EffectiveEnableFileEvents: false, - EnableFileEvents: config.EnableFileEvents, - EncryptionDetails: config.EncryptionDetails, - Fallback: config.Fallback, - FileEventQueue: config.FileEventQueue, - Force: false, - IsolationMode: "", // Not supported by DABs - Name: id, - NewName: "", // Initialized below if needed - Owner: "", // Not supported by DABs - ReadOnly: config.ReadOnly, - SkipValidation: config.SkipValidation, - Url: config.Url, - ForceSendFields: utils.FilterFields[catalog.UpdateExternalLocation](config.ForceSendFields, "IsolationMode", "Owner"), - } + updateRequest := externalLocationUpdateCopy.Do(config) + updateRequest.Name = id if config.Name != id { updateRequest.NewName = config.Name diff --git a/bundle/direct/dresources/fieldcopy_test.go b/bundle/direct/dresources/fieldcopy_test.go index 4cc37becaa..832bb318da 100644 --- a/bundle/direct/dresources/fieldcopy_test.go +++ b/bundle/direct/dresources/fieldcopy_test.go @@ -27,6 +27,21 @@ func TestFieldCopyReport(t *testing.T) { &autoCaptureConfigCopy, &servedEntityCopy, &servingRemapCopy, + // catalog + &catalogRemapCopy, + &catalogUpdateCopy, + // registered model + ®isteredModelRemapCopy, + ®isteredModelUpdateCopy, + // external location + &externalLocationRemapCopy, + &externalLocationUpdateCopy, + // quality monitor + &qualityMonitorRemapCopy, + &qualityMonitorUpdateCopy, + // sql warehouse + &sqlWarehouseRemapCopy, + &sqlWarehouseEditCopy, } var buf strings.Builder diff --git a/bundle/direct/dresources/out.fieldcopy.txt b/bundle/direct/dresources/out.fieldcopy.txt index f9f3b19cbb..dbb043dbf3 100644 --- a/bundle/direct/dresources/out.fieldcopy.txt +++ b/bundle/direct/dresources/out.fieldcopy.txt @@ -74,3 +74,89 @@ serving.ServingEndpointDetailed → serving.CreateServingEndpoint dst not set: - Config - RateLimits +catalog.CatalogInfo → catalog.CreateCatalog + src not copied: + - BrowseOnly + - CatalogType + - CreatedAt + - CreatedBy + - EffectivePredictiveOptimizationFlag + - EnablePredictiveOptimization + - FullName + - IsolationMode + - MetastoreId + - Owner + - ProvisioningInfo + - SecurableType + - StorageLocation + - UpdatedAt + - UpdatedBy +catalog.CreateCatalog → catalog.UpdateCatalog + src not copied: + - ConnectionName + - ProviderName + - ShareName + - StorageRoot + dst not set: + - EnablePredictiveOptimization + - IsolationMode + - NewName + - Owner +catalog.RegisteredModelInfo → catalog.CreateRegisteredModelRequest: all fields matched +catalog.CreateRegisteredModelRequest → catalog.UpdateRegisteredModelRequest + dst not set: + - NewName +catalog.ExternalLocationInfo → catalog.CreateExternalLocation + src not copied: + - BrowseOnly + - CreatedAt + - CreatedBy + - CredentialId + - IsolationMode + - MetastoreId + - Owner + - UpdatedAt + - UpdatedBy + dst not set: + - SkipValidation +catalog.CreateExternalLocation → catalog.UpdateExternalLocation + dst not set: + - Force + - IsolationMode + - NewName + - Owner +catalog.MonitorInfo → catalog.CreateMonitor + src not copied: + - DashboardId + - DriftMetricsTableName + - MonitorVersion + - ProfileMetricsTableName + - Status + dst not set: + - SkipBuiltinDashboard + - WarehouseId +catalog.CreateMonitor → catalog.UpdateMonitor + src not copied: + - AssetsDir + - SkipBuiltinDashboard + - WarehouseId + dst not set: + - DashboardId +sql.GetWarehouseResponse → sql.CreateWarehouseRequest + src not copied: + - Health + - Id + - JdbcUrl + - NumActiveSessions + - NumClusters + - OdbcParams + - State + - WarehouseType + dst not set: + - WarehouseType +sql.CreateWarehouseRequest → sql.EditWarehouseRequest + src not copied: + - WarehouseType + dst not set: + - Id + - WarehouseType diff --git a/bundle/direct/dresources/quality_monitor.go b/bundle/direct/dresources/quality_monitor.go index dcc48fb95d..11eb97941a 100644 --- a/bundle/direct/dresources/quality_monitor.go +++ b/bundle/direct/dresources/quality_monitor.go @@ -4,7 +4,7 @@ import ( "context" "github.com/databricks/cli/bundle/config/resources" - "github.com/databricks/cli/libs/utils" + "github.com/databricks/cli/libs/structs/fieldcopy" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/marshal" "github.com/databricks/databricks-sdk-go/service/catalog" @@ -41,27 +41,13 @@ func (*ResourceQualityMonitor) PrepareState(input *resources.QualityMonitor) *Qu } } +// qualityMonitorRemapCopy maps MonitorInfo (remote GET response) to CreateMonitor (local state). +var qualityMonitorRemapCopy = fieldcopy.Copy[catalog.MonitorInfo, catalog.CreateMonitor]{} + func (*ResourceQualityMonitor) RemapState(info *catalog.MonitorInfo) *QualityMonitorState { return &QualityMonitorState{ - CreateMonitor: catalog.CreateMonitor{ - AssetsDir: info.AssetsDir, - BaselineTableName: info.BaselineTableName, - CustomMetrics: info.CustomMetrics, - DataClassificationConfig: info.DataClassificationConfig, - InferenceLog: info.InferenceLog, - LatestMonitorFailureMsg: info.LatestMonitorFailureMsg, - Notifications: info.Notifications, - OutputSchemaName: info.OutputSchemaName, - Schedule: info.Schedule, - SkipBuiltinDashboard: false, - SlicingExprs: info.SlicingExprs, - Snapshot: info.Snapshot, - TableName: info.TableName, - TimeSeries: info.TimeSeries, - WarehouseId: "", - ForceSendFields: utils.FilterFields[catalog.CreateMonitor](info.ForceSendFields), - }, - TableName: info.TableName, + CreateMonitor: qualityMonitorRemapCopy.Do(info), + TableName: info.TableName, } } @@ -83,23 +69,12 @@ func (r *ResourceQualityMonitor) DoCreate(ctx context.Context, config *QualityMo return response.TableName, response, nil } +// qualityMonitorUpdateCopy maps CreateMonitor (local state) to UpdateMonitor (API request). +var qualityMonitorUpdateCopy = fieldcopy.Copy[catalog.CreateMonitor, catalog.UpdateMonitor]{} + func (r *ResourceQualityMonitor) DoUpdate(ctx context.Context, id string, config *QualityMonitorState, _ Changes) (*catalog.MonitorInfo, error) { - updateRequest := catalog.UpdateMonitor{ - TableName: id, - BaselineTableName: config.BaselineTableName, - CustomMetrics: config.CustomMetrics, - DashboardId: "", - DataClassificationConfig: config.DataClassificationConfig, - InferenceLog: config.InferenceLog, - LatestMonitorFailureMsg: "", - Notifications: config.Notifications, - OutputSchemaName: config.OutputSchemaName, - Schedule: config.Schedule, - SlicingExprs: config.SlicingExprs, - Snapshot: config.Snapshot, - TimeSeries: config.TimeSeries, - ForceSendFields: utils.FilterFields[catalog.UpdateMonitor](config.ForceSendFields), - } + updateRequest := qualityMonitorUpdateCopy.Do(&config.CreateMonitor) + updateRequest.TableName = id //nolint:staticcheck // Direct quality_monitor resource still uses legacy monitor endpoints; v1 data-quality migration is separate work. response, err := r.client.QualityMonitors.Update(ctx, updateRequest) diff --git a/bundle/direct/dresources/registered_model.go b/bundle/direct/dresources/registered_model.go index 666716b48b..f6b0bec477 100644 --- a/bundle/direct/dresources/registered_model.go +++ b/bundle/direct/dresources/registered_model.go @@ -4,7 +4,7 @@ import ( "context" "github.com/databricks/cli/bundle/config/resources" - "github.com/databricks/cli/libs/utils" + "github.com/databricks/cli/libs/structs/fieldcopy" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/service/catalog" ) @@ -23,27 +23,12 @@ func (*ResourceRegisteredModel) PrepareState(input *resources.RegisteredModel) * return &input.CreateRegisteredModelRequest } -func (*ResourceRegisteredModel) RemapState(model *catalog.RegisteredModelInfo) *catalog.CreateRegisteredModelRequest { - return &catalog.CreateRegisteredModelRequest{ - CatalogName: model.CatalogName, - Comment: model.Comment, - Name: model.Name, - SchemaName: model.SchemaName, - StorageLocation: model.StorageLocation, - ForceSendFields: utils.FilterFields[catalog.CreateRegisteredModelRequest](model.ForceSendFields), - - Aliases: model.Aliases, - BrowseOnly: model.BrowseOnly, - FullName: model.FullName, - MetastoreId: model.MetastoreId, - Owner: model.Owner, +// registeredModelRemapCopy maps RegisteredModelInfo (remote GET response) to CreateRegisteredModelRequest (local state). +var registeredModelRemapCopy = fieldcopy.Copy[catalog.RegisteredModelInfo, catalog.CreateRegisteredModelRequest]{} - // Clear output only fields. They should not show up on remote diff computation. - CreatedAt: 0, - CreatedBy: "", - UpdatedAt: 0, - UpdatedBy: "", - } +func (*ResourceRegisteredModel) RemapState(model *catalog.RegisteredModelInfo) *catalog.CreateRegisteredModelRequest { + result := registeredModelRemapCopy.Do(model) + return &result } func (r *ResourceRegisteredModel) DoRead(ctx context.Context, id string) (*catalog.RegisteredModelInfo, error) { @@ -64,31 +49,12 @@ func (r *ResourceRegisteredModel) DoCreate(ctx context.Context, config *catalog. return response.FullName, response, nil } -func (r *ResourceRegisteredModel) DoUpdate(ctx context.Context, id string, config *catalog.CreateRegisteredModelRequest, _ Changes) (*catalog.RegisteredModelInfo, error) { - updateRequest := catalog.UpdateRegisteredModelRequest{ - FullName: id, - Comment: config.Comment, - ForceSendFields: utils.FilterFields[catalog.UpdateRegisteredModelRequest](config.ForceSendFields, "Owner", "NewName"), - - // Owner is not part of the configuration tree - Owner: "", - - // Name updates are not supported yet without recreating. Can be added as a follow-up. - // Note: TF also does not support changing name without a recreate so the current behavior matches TF. - NewName: "", +// registeredModelUpdateCopy maps CreateRegisteredModelRequest (local state) to UpdateRegisteredModelRequest (API request). +var registeredModelUpdateCopy = fieldcopy.Copy[catalog.CreateRegisteredModelRequest, catalog.UpdateRegisteredModelRequest]{} - Aliases: config.Aliases, - BrowseOnly: config.BrowseOnly, - CreatedAt: config.CreatedAt, - CreatedBy: config.CreatedBy, - MetastoreId: config.MetastoreId, - UpdatedAt: config.UpdatedAt, - UpdatedBy: config.UpdatedBy, - SchemaName: config.SchemaName, - StorageLocation: config.StorageLocation, - Name: config.Name, - CatalogName: config.CatalogName, - } +func (r *ResourceRegisteredModel) DoUpdate(ctx context.Context, id string, config *catalog.CreateRegisteredModelRequest, _ Changes) (*catalog.RegisteredModelInfo, error) { + updateRequest := registeredModelUpdateCopy.Do(config) + updateRequest.FullName = id response, err := r.client.RegisteredModels.Update(ctx, updateRequest) if err != nil { diff --git a/bundle/direct/dresources/sql_warehouse.go b/bundle/direct/dresources/sql_warehouse.go index 5d9d7793b7..75b575da1c 100644 --- a/bundle/direct/dresources/sql_warehouse.go +++ b/bundle/direct/dresources/sql_warehouse.go @@ -5,7 +5,7 @@ import ( "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/libs/log" - "github.com/databricks/cli/libs/utils" + "github.com/databricks/cli/libs/structs/fieldcopy" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/service/sql" ) @@ -24,23 +24,14 @@ func (*ResourceSqlWarehouse) PrepareState(input *resources.SqlWarehouse) *sql.Cr return &input.CreateWarehouseRequest } +// sqlWarehouseRemapCopy maps GetWarehouseResponse (remote GET response) to CreateWarehouseRequest (local state). +var sqlWarehouseRemapCopy = fieldcopy.Copy[sql.GetWarehouseResponse, sql.CreateWarehouseRequest]{} + func (*ResourceSqlWarehouse) RemapState(warehouse *sql.GetWarehouseResponse) *sql.CreateWarehouseRequest { - return &sql.CreateWarehouseRequest{ - AutoStopMins: warehouse.AutoStopMins, - Channel: warehouse.Channel, - ClusterSize: warehouse.ClusterSize, - CreatorName: warehouse.CreatorName, - EnablePhoton: warehouse.EnablePhoton, - EnableServerlessCompute: warehouse.EnableServerlessCompute, - InstanceProfileArn: warehouse.InstanceProfileArn, - MaxNumClusters: warehouse.MaxNumClusters, - MinNumClusters: warehouse.MinNumClusters, - Name: warehouse.Name, - SpotInstancePolicy: warehouse.SpotInstancePolicy, - Tags: warehouse.Tags, - WarehouseType: sql.CreateWarehouseRequestWarehouseType(warehouse.WarehouseType), - ForceSendFields: utils.FilterFields[sql.CreateWarehouseRequest](warehouse.ForceSendFields), - } + result := sqlWarehouseRemapCopy.Do(warehouse) + // WarehouseType requires explicit conversion between different named string types. + result.WarehouseType = sql.CreateWarehouseRequestWarehouseType(warehouse.WarehouseType) + return &result } // DoRead reads the warehouse by id. @@ -57,25 +48,15 @@ func (r *ResourceSqlWarehouse) DoCreate(ctx context.Context, config *sql.CreateW return waiter.Id, nil, nil } +// sqlWarehouseEditCopy maps CreateWarehouseRequest (local state) to EditWarehouseRequest (API request). +var sqlWarehouseEditCopy = fieldcopy.Copy[sql.CreateWarehouseRequest, sql.EditWarehouseRequest]{} + // DoUpdate updates the warehouse in place. func (r *ResourceSqlWarehouse) DoUpdate(ctx context.Context, id string, config *sql.CreateWarehouseRequest, _ Changes) (*sql.GetWarehouseResponse, error) { - request := sql.EditWarehouseRequest{ - AutoStopMins: config.AutoStopMins, - Channel: config.Channel, - ClusterSize: config.ClusterSize, - CreatorName: config.CreatorName, - EnablePhoton: config.EnablePhoton, - EnableServerlessCompute: config.EnableServerlessCompute, - Id: id, - InstanceProfileArn: config.InstanceProfileArn, - MaxNumClusters: config.MaxNumClusters, - MinNumClusters: config.MinNumClusters, - Name: config.Name, - SpotInstancePolicy: config.SpotInstancePolicy, - Tags: config.Tags, - WarehouseType: sql.EditWarehouseRequestWarehouseType(config.WarehouseType), - ForceSendFields: utils.FilterFields[sql.EditWarehouseRequest](config.ForceSendFields), - } + request := sqlWarehouseEditCopy.Do(config) + request.Id = id + // WarehouseType requires explicit conversion between different named string types. + request.WarehouseType = sql.EditWarehouseRequestWarehouseType(config.WarehouseType) waiter, err := r.client.Warehouses.Edit(ctx, request) if err != nil { From 92c5b922de55e7e1504ebf0b67fd0a9b12d31f40 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 23 Mar 2026 16:35:19 +0100 Subject: [PATCH 08/16] Add init() registration pattern for fieldcopy instances Each dresources file now defines init() to register fieldcopy instances via registerCopy(), which both initializes the copy (Init) and appends to a global list used by the golden file test. Do() panics if called without Init, catching unregistered copies at runtime. Co-authored-by: Isaac --- bundle/direct/dresources/catalog.go | 9 +- bundle/direct/dresources/cluster.go | 12 +- bundle/direct/dresources/external_location.go | 9 +- bundle/direct/dresources/fieldcopy.go | 14 ++ bundle/direct/dresources/fieldcopy_test.go | 34 +---- bundle/direct/dresources/job.go | 6 +- .../dresources/model_serving_endpoint.go | 12 +- bundle/direct/dresources/out.fieldcopy.txt | 132 +++++++++--------- bundle/direct/dresources/pipeline.go | 12 +- bundle/direct/dresources/quality_monitor.go | 9 +- bundle/direct/dresources/registered_model.go | 9 +- bundle/direct/dresources/sql_warehouse.go | 9 +- libs/structs/fieldcopy/fieldcopy.go | 17 ++- libs/structs/fieldcopy/fieldcopy_test.go | 31 ++++ 14 files changed, 190 insertions(+), 125 deletions(-) create mode 100644 bundle/direct/dresources/fieldcopy.go diff --git a/bundle/direct/dresources/catalog.go b/bundle/direct/dresources/catalog.go index 30d5e06788..e801ace25e 100644 --- a/bundle/direct/dresources/catalog.go +++ b/bundle/direct/dresources/catalog.go @@ -22,7 +22,7 @@ func (*ResourceCatalog) PrepareState(input *resources.Catalog) *catalog.CreateCa } // catalogRemapCopy maps CatalogInfo (remote GET response) to CreateCatalog (local state). -var catalogRemapCopy = fieldcopy.Copy[catalog.CatalogInfo, catalog.CreateCatalog]{} +var catalogRemapCopy fieldcopy.Copy[catalog.CatalogInfo, catalog.CreateCatalog] func (*ResourceCatalog) RemapState(info *catalog.CatalogInfo) *catalog.CreateCatalog { result := catalogRemapCopy.Do(info) @@ -42,7 +42,12 @@ func (r *ResourceCatalog) DoCreate(ctx context.Context, config *catalog.CreateCa } // catalogUpdateCopy maps CreateCatalog (local state) to UpdateCatalog (API request). -var catalogUpdateCopy = fieldcopy.Copy[catalog.CreateCatalog, catalog.UpdateCatalog]{} +var catalogUpdateCopy fieldcopy.Copy[catalog.CreateCatalog, catalog.UpdateCatalog] + +func init() { + registerCopy(&catalogRemapCopy) + registerCopy(&catalogUpdateCopy) +} // DoUpdate updates the catalog in place and returns remote state. func (r *ResourceCatalog) DoUpdate(ctx context.Context, id string, config *catalog.CreateCatalog, _ Changes) (*catalog.CatalogInfo, error) { diff --git a/bundle/direct/dresources/cluster.go b/bundle/direct/dresources/cluster.go index 8da3dd68b6..bcca922f41 100644 --- a/bundle/direct/dresources/cluster.go +++ b/bundle/direct/dresources/cluster.go @@ -32,7 +32,7 @@ func (r *ResourceCluster) PrepareState(input *resources.Cluster) *compute.Cluste } // clusterRemapCopy maps ClusterDetails (remote GET response) to ClusterSpec (local state). -var clusterRemapCopy = fieldcopy.Copy[compute.ClusterDetails, compute.ClusterSpec]{} +var clusterRemapCopy fieldcopy.Copy[compute.ClusterDetails, compute.ClusterSpec] func (r *ResourceCluster) RemapState(input *compute.ClusterDetails) *compute.ClusterSpec { spec := clusterRemapCopy.Do(input) @@ -120,7 +120,7 @@ func (r *ResourceCluster) OverrideChangeDesc(ctx context.Context, p *structpath. } // clusterCreateCopy maps ClusterSpec (local state) to CreateCluster (API request). -var clusterCreateCopy = fieldcopy.Copy[compute.ClusterSpec, compute.CreateCluster]{} +var clusterCreateCopy fieldcopy.Copy[compute.ClusterSpec, compute.CreateCluster] func makeCreateCluster(config *compute.ClusterSpec) compute.CreateCluster { create := clusterCreateCopy.Do(config) @@ -135,7 +135,13 @@ func makeCreateCluster(config *compute.ClusterSpec) compute.CreateCluster { } // clusterEditCopy maps ClusterSpec (local state) to EditCluster (API request). -var clusterEditCopy = fieldcopy.Copy[compute.ClusterSpec, compute.EditCluster]{} +var clusterEditCopy fieldcopy.Copy[compute.ClusterSpec, compute.EditCluster] + +func init() { + registerCopy(&clusterRemapCopy) + registerCopy(&clusterCreateCopy) + registerCopy(&clusterEditCopy) +} func makeEditCluster(id string, config *compute.ClusterSpec) compute.EditCluster { edit := clusterEditCopy.Do(config) diff --git a/bundle/direct/dresources/external_location.go b/bundle/direct/dresources/external_location.go index 755a2ba7d5..0961372e65 100644 --- a/bundle/direct/dresources/external_location.go +++ b/bundle/direct/dresources/external_location.go @@ -22,7 +22,7 @@ func (*ResourceExternalLocation) PrepareState(input *resources.ExternalLocation) } // externalLocationRemapCopy maps ExternalLocationInfo (remote GET response) to CreateExternalLocation (local state). -var externalLocationRemapCopy = fieldcopy.Copy[catalog.ExternalLocationInfo, catalog.CreateExternalLocation]{} +var externalLocationRemapCopy fieldcopy.Copy[catalog.ExternalLocationInfo, catalog.CreateExternalLocation] func (*ResourceExternalLocation) RemapState(info *catalog.ExternalLocationInfo) *catalog.CreateExternalLocation { result := externalLocationRemapCopy.Do(info) @@ -42,7 +42,12 @@ func (r *ResourceExternalLocation) DoCreate(ctx context.Context, config *catalog } // externalLocationUpdateCopy maps CreateExternalLocation (local state) to UpdateExternalLocation (API request). -var externalLocationUpdateCopy = fieldcopy.Copy[catalog.CreateExternalLocation, catalog.UpdateExternalLocation]{} +var externalLocationUpdateCopy fieldcopy.Copy[catalog.CreateExternalLocation, catalog.UpdateExternalLocation] + +func init() { + registerCopy(&externalLocationRemapCopy) + registerCopy(&externalLocationUpdateCopy) +} // DoUpdate updates the external location in place and returns remote state. func (r *ResourceExternalLocation) DoUpdate(ctx context.Context, id string, config *catalog.CreateExternalLocation, _ Changes) (*catalog.ExternalLocationInfo, error) { diff --git a/bundle/direct/dresources/fieldcopy.go b/bundle/direct/dresources/fieldcopy.go new file mode 100644 index 0000000000..f0f86c1319 --- /dev/null +++ b/bundle/direct/dresources/fieldcopy.go @@ -0,0 +1,14 @@ +package dresources + +import "github.com/databricks/cli/libs/structs/fieldcopy" + +type fieldCopyReporter interface { + Report() string +} + +var allFieldCopies []fieldCopyReporter + +func registerCopy[Src, Dst any](c *fieldcopy.Copy[Src, Dst]) { + c.Init() + allFieldCopies = append(allFieldCopies, c) +} diff --git a/bundle/direct/dresources/fieldcopy_test.go b/bundle/direct/dresources/fieldcopy_test.go index 832bb318da..41ae0c81b6 100644 --- a/bundle/direct/dresources/fieldcopy_test.go +++ b/bundle/direct/dresources/fieldcopy_test.go @@ -12,40 +12,8 @@ func TestFieldCopyReport(t *testing.T) { ctx := context.Background() ctx, _ = testdiff.WithReplacementsMap(ctx) - copies := []interface{ Report() string }{ - // cluster - &clusterRemapCopy, - &clusterCreateCopy, - &clusterEditCopy, - // job - &jobCreateCopy, - // pipeline - &pipelineSpecCopy, - &pipelineRemoteCopy, - &pipelineEditCopy, - // model serving endpoint - &autoCaptureConfigCopy, - &servedEntityCopy, - &servingRemapCopy, - // catalog - &catalogRemapCopy, - &catalogUpdateCopy, - // registered model - ®isteredModelRemapCopy, - ®isteredModelUpdateCopy, - // external location - &externalLocationRemapCopy, - &externalLocationUpdateCopy, - // quality monitor - &qualityMonitorRemapCopy, - &qualityMonitorUpdateCopy, - // sql warehouse - &sqlWarehouseRemapCopy, - &sqlWarehouseEditCopy, - } - var buf strings.Builder - for _, c := range copies { + for _, c := range allFieldCopies { buf.WriteString(c.Report()) } diff --git a/bundle/direct/dresources/job.go b/bundle/direct/dresources/job.go index 50a59fa4b7..116949bcba 100644 --- a/bundle/direct/dresources/job.go +++ b/bundle/direct/dresources/job.go @@ -125,7 +125,11 @@ func (r *ResourceJob) DoDelete(ctx context.Context, id string) error { } // jobCreateCopy maps JobSettings (local state) to CreateJob (API request). -var jobCreateCopy = fieldcopy.Copy[jobs.JobSettings, jobs.CreateJob]{} +var jobCreateCopy fieldcopy.Copy[jobs.JobSettings, jobs.CreateJob] + +func init() { + registerCopy(&jobCreateCopy) +} func makeResetJob(config jobs.JobSettings, id string) (jobs.ResetJob, error) { idInt, err := parseJobID(id) diff --git a/bundle/direct/dresources/model_serving_endpoint.go b/bundle/direct/dresources/model_serving_endpoint.go index cbb94cbf73..bb94ed4619 100644 --- a/bundle/direct/dresources/model_serving_endpoint.go +++ b/bundle/direct/dresources/model_serving_endpoint.go @@ -35,7 +35,7 @@ func (*ResourceModelServingEndpoint) PrepareState(input *resources.ModelServingE } // autoCaptureConfigCopy maps AutoCaptureConfigOutput to AutoCaptureConfigInput. -var autoCaptureConfigCopy = fieldcopy.Copy[serving.AutoCaptureConfigOutput, serving.AutoCaptureConfigInput]{} +var autoCaptureConfigCopy fieldcopy.Copy[serving.AutoCaptureConfigOutput, serving.AutoCaptureConfigInput] func autoCaptureConfigOutputToInput(output *serving.AutoCaptureConfigOutput) *serving.AutoCaptureConfigInput { if output == nil { @@ -46,7 +46,7 @@ func autoCaptureConfigOutputToInput(output *serving.AutoCaptureConfigOutput) *se } // servedEntityCopy maps ServedEntityOutput to ServedEntityInput. -var servedEntityCopy = fieldcopy.Copy[serving.ServedEntityOutput, serving.ServedEntityInput]{} +var servedEntityCopy fieldcopy.Copy[serving.ServedEntityOutput, serving.ServedEntityInput] func servedEntitiesOutputToInput(output []serving.ServedEntityOutput) []serving.ServedEntityInput { entities := make([]serving.ServedEntityInput, len(output)) @@ -70,7 +70,13 @@ func configOutputToInput(output *serving.EndpointCoreConfigOutput) *serving.Endp } // servingRemapCopy maps ServingEndpointDetailed (remote GET response) to CreateServingEndpoint (local state). -var servingRemapCopy = fieldcopy.Copy[serving.ServingEndpointDetailed, serving.CreateServingEndpoint]{} +var servingRemapCopy fieldcopy.Copy[serving.ServingEndpointDetailed, serving.CreateServingEndpoint] + +func init() { + registerCopy(&autoCaptureConfigCopy) + registerCopy(&servedEntityCopy) + registerCopy(&servingRemapCopy) +} func (*ResourceModelServingEndpoint) RemapState(state *RefreshOutput) *serving.CreateServingEndpoint { details := state.EndpointDetails diff --git a/bundle/direct/dresources/out.fieldcopy.txt b/bundle/direct/dresources/out.fieldcopy.txt index dbb043dbf3..69663784c6 100644 --- a/bundle/direct/dresources/out.fieldcopy.txt +++ b/bundle/direct/dresources/out.fieldcopy.txt @@ -1,3 +1,31 @@ +catalog.CatalogInfo → catalog.CreateCatalog + src not copied: + - BrowseOnly + - CatalogType + - CreatedAt + - CreatedBy + - EffectivePredictiveOptimizationFlag + - EnablePredictiveOptimization + - FullName + - IsolationMode + - MetastoreId + - Owner + - ProvisioningInfo + - SecurableType + - StorageLocation + - UpdatedAt + - UpdatedBy +catalog.CreateCatalog → catalog.UpdateCatalog + src not copied: + - ConnectionName + - ProviderName + - ShareName + - StorageRoot + dst not set: + - EnablePredictiveOptimization + - IsolationMode + - NewName + - Owner compute.ClusterDetails → compute.ClusterSpec src not copied: - ClusterCores @@ -27,28 +55,28 @@ compute.ClusterSpec → compute.CreateCluster compute.ClusterSpec → compute.EditCluster dst not set: - ClusterId -jobs.JobSettings → jobs.CreateJob - dst not set: - - AccessControlList -pipelines.PipelineSpec → pipelines.CreatePipeline - dst not set: - - AllowDuplicateNames - - DryRun - - RunAs -pipelines.GetPipelineResponse → dresources.PipelineRemote +catalog.ExternalLocationInfo → catalog.CreateExternalLocation src not copied: - - Name - - RunAs - - Spec - - ForceSendFields + - BrowseOnly + - CreatedAt + - CreatedBy + - CredentialId + - IsolationMode + - MetastoreId + - Owner + - UpdatedAt + - UpdatedBy dst not set: - - CreatePipeline -pipelines.CreatePipeline → pipelines.EditPipeline - src not copied: - - DryRun + - SkipValidation +catalog.CreateExternalLocation → catalog.UpdateExternalLocation dst not set: - - ExpectedLastModified - - PipelineId + - Force + - IsolationMode + - NewName + - Owner +jobs.JobSettings → jobs.CreateJob + dst not set: + - AccessControlList serving.AutoCaptureConfigOutput → serving.AutoCaptureConfigInput src not copied: - State @@ -74,57 +102,25 @@ serving.ServingEndpointDetailed → serving.CreateServingEndpoint dst not set: - Config - RateLimits -catalog.CatalogInfo → catalog.CreateCatalog - src not copied: - - BrowseOnly - - CatalogType - - CreatedAt - - CreatedBy - - EffectivePredictiveOptimizationFlag - - EnablePredictiveOptimization - - FullName - - IsolationMode - - MetastoreId - - Owner - - ProvisioningInfo - - SecurableType - - StorageLocation - - UpdatedAt - - UpdatedBy -catalog.CreateCatalog → catalog.UpdateCatalog - src not copied: - - ConnectionName - - ProviderName - - ShareName - - StorageRoot - dst not set: - - EnablePredictiveOptimization - - IsolationMode - - NewName - - Owner -catalog.RegisteredModelInfo → catalog.CreateRegisteredModelRequest: all fields matched -catalog.CreateRegisteredModelRequest → catalog.UpdateRegisteredModelRequest +pipelines.PipelineSpec → pipelines.CreatePipeline dst not set: - - NewName -catalog.ExternalLocationInfo → catalog.CreateExternalLocation + - AllowDuplicateNames + - DryRun + - RunAs +pipelines.GetPipelineResponse → dresources.PipelineRemote src not copied: - - BrowseOnly - - CreatedAt - - CreatedBy - - CredentialId - - IsolationMode - - MetastoreId - - Owner - - UpdatedAt - - UpdatedBy + - Name + - RunAs + - Spec + - ForceSendFields dst not set: - - SkipValidation -catalog.CreateExternalLocation → catalog.UpdateExternalLocation + - CreatePipeline +pipelines.CreatePipeline → pipelines.EditPipeline + src not copied: + - DryRun dst not set: - - Force - - IsolationMode - - NewName - - Owner + - ExpectedLastModified + - PipelineId catalog.MonitorInfo → catalog.CreateMonitor src not copied: - DashboardId @@ -142,6 +138,10 @@ catalog.CreateMonitor → catalog.UpdateMonitor - WarehouseId dst not set: - DashboardId +catalog.RegisteredModelInfo → catalog.CreateRegisteredModelRequest: all fields matched +catalog.CreateRegisteredModelRequest → catalog.UpdateRegisteredModelRequest + dst not set: + - NewName sql.GetWarehouseResponse → sql.CreateWarehouseRequest src not copied: - Health diff --git a/bundle/direct/dresources/pipeline.go b/bundle/direct/dresources/pipeline.go index d7f2daaf20..2f612b4a5d 100644 --- a/bundle/direct/dresources/pipeline.go +++ b/bundle/direct/dresources/pipeline.go @@ -66,10 +66,10 @@ func (r *ResourcePipeline) DoRead(ctx context.Context, id string) (*PipelineRemo } // pipelineSpecCopy maps PipelineSpec (from GET response) to CreatePipeline (local state). -var pipelineSpecCopy = fieldcopy.Copy[pipelines.PipelineSpec, pipelines.CreatePipeline]{} +var pipelineSpecCopy fieldcopy.Copy[pipelines.PipelineSpec, pipelines.CreatePipeline] // pipelineRemoteCopy maps GetPipelineResponse to PipelineRemote extra fields. -var pipelineRemoteCopy = fieldcopy.Copy[pipelines.GetPipelineResponse, PipelineRemote]{} +var pipelineRemoteCopy fieldcopy.Copy[pipelines.GetPipelineResponse, PipelineRemote] func makePipelineRemote(p *pipelines.GetPipelineResponse) *PipelineRemote { remote := pipelineRemoteCopy.Do(p) @@ -89,7 +89,13 @@ func (r *ResourcePipeline) DoCreate(ctx context.Context, config *pipelines.Creat } // pipelineEditCopy maps CreatePipeline (local state) to EditPipeline (API request). -var pipelineEditCopy = fieldcopy.Copy[pipelines.CreatePipeline, pipelines.EditPipeline]{} +var pipelineEditCopy fieldcopy.Copy[pipelines.CreatePipeline, pipelines.EditPipeline] + +func init() { + registerCopy(&pipelineSpecCopy) + registerCopy(&pipelineRemoteCopy) + registerCopy(&pipelineEditCopy) +} func (r *ResourcePipeline) DoUpdate(ctx context.Context, id string, config *pipelines.CreatePipeline, _ Changes) (*PipelineRemote, error) { request := pipelineEditCopy.Do(config) diff --git a/bundle/direct/dresources/quality_monitor.go b/bundle/direct/dresources/quality_monitor.go index 11eb97941a..23899648ab 100644 --- a/bundle/direct/dresources/quality_monitor.go +++ b/bundle/direct/dresources/quality_monitor.go @@ -42,7 +42,7 @@ func (*ResourceQualityMonitor) PrepareState(input *resources.QualityMonitor) *Qu } // qualityMonitorRemapCopy maps MonitorInfo (remote GET response) to CreateMonitor (local state). -var qualityMonitorRemapCopy = fieldcopy.Copy[catalog.MonitorInfo, catalog.CreateMonitor]{} +var qualityMonitorRemapCopy fieldcopy.Copy[catalog.MonitorInfo, catalog.CreateMonitor] func (*ResourceQualityMonitor) RemapState(info *catalog.MonitorInfo) *QualityMonitorState { return &QualityMonitorState{ @@ -70,7 +70,12 @@ func (r *ResourceQualityMonitor) DoCreate(ctx context.Context, config *QualityMo } // qualityMonitorUpdateCopy maps CreateMonitor (local state) to UpdateMonitor (API request). -var qualityMonitorUpdateCopy = fieldcopy.Copy[catalog.CreateMonitor, catalog.UpdateMonitor]{} +var qualityMonitorUpdateCopy fieldcopy.Copy[catalog.CreateMonitor, catalog.UpdateMonitor] + +func init() { + registerCopy(&qualityMonitorRemapCopy) + registerCopy(&qualityMonitorUpdateCopy) +} func (r *ResourceQualityMonitor) DoUpdate(ctx context.Context, id string, config *QualityMonitorState, _ Changes) (*catalog.MonitorInfo, error) { updateRequest := qualityMonitorUpdateCopy.Do(&config.CreateMonitor) diff --git a/bundle/direct/dresources/registered_model.go b/bundle/direct/dresources/registered_model.go index f6b0bec477..14b28db500 100644 --- a/bundle/direct/dresources/registered_model.go +++ b/bundle/direct/dresources/registered_model.go @@ -24,7 +24,7 @@ func (*ResourceRegisteredModel) PrepareState(input *resources.RegisteredModel) * } // registeredModelRemapCopy maps RegisteredModelInfo (remote GET response) to CreateRegisteredModelRequest (local state). -var registeredModelRemapCopy = fieldcopy.Copy[catalog.RegisteredModelInfo, catalog.CreateRegisteredModelRequest]{} +var registeredModelRemapCopy fieldcopy.Copy[catalog.RegisteredModelInfo, catalog.CreateRegisteredModelRequest] func (*ResourceRegisteredModel) RemapState(model *catalog.RegisteredModelInfo) *catalog.CreateRegisteredModelRequest { result := registeredModelRemapCopy.Do(model) @@ -50,7 +50,12 @@ func (r *ResourceRegisteredModel) DoCreate(ctx context.Context, config *catalog. } // registeredModelUpdateCopy maps CreateRegisteredModelRequest (local state) to UpdateRegisteredModelRequest (API request). -var registeredModelUpdateCopy = fieldcopy.Copy[catalog.CreateRegisteredModelRequest, catalog.UpdateRegisteredModelRequest]{} +var registeredModelUpdateCopy fieldcopy.Copy[catalog.CreateRegisteredModelRequest, catalog.UpdateRegisteredModelRequest] + +func init() { + registerCopy(®isteredModelRemapCopy) + registerCopy(®isteredModelUpdateCopy) +} func (r *ResourceRegisteredModel) DoUpdate(ctx context.Context, id string, config *catalog.CreateRegisteredModelRequest, _ Changes) (*catalog.RegisteredModelInfo, error) { updateRequest := registeredModelUpdateCopy.Do(config) diff --git a/bundle/direct/dresources/sql_warehouse.go b/bundle/direct/dresources/sql_warehouse.go index 75b575da1c..5f91b32f38 100644 --- a/bundle/direct/dresources/sql_warehouse.go +++ b/bundle/direct/dresources/sql_warehouse.go @@ -25,7 +25,7 @@ func (*ResourceSqlWarehouse) PrepareState(input *resources.SqlWarehouse) *sql.Cr } // sqlWarehouseRemapCopy maps GetWarehouseResponse (remote GET response) to CreateWarehouseRequest (local state). -var sqlWarehouseRemapCopy = fieldcopy.Copy[sql.GetWarehouseResponse, sql.CreateWarehouseRequest]{} +var sqlWarehouseRemapCopy fieldcopy.Copy[sql.GetWarehouseResponse, sql.CreateWarehouseRequest] func (*ResourceSqlWarehouse) RemapState(warehouse *sql.GetWarehouseResponse) *sql.CreateWarehouseRequest { result := sqlWarehouseRemapCopy.Do(warehouse) @@ -49,7 +49,12 @@ func (r *ResourceSqlWarehouse) DoCreate(ctx context.Context, config *sql.CreateW } // sqlWarehouseEditCopy maps CreateWarehouseRequest (local state) to EditWarehouseRequest (API request). -var sqlWarehouseEditCopy = fieldcopy.Copy[sql.CreateWarehouseRequest, sql.EditWarehouseRequest]{} +var sqlWarehouseEditCopy fieldcopy.Copy[sql.CreateWarehouseRequest, sql.EditWarehouseRequest] + +func init() { + registerCopy(&sqlWarehouseRemapCopy) + registerCopy(&sqlWarehouseEditCopy) +} // DoUpdate updates the warehouse in place. func (r *ResourceSqlWarehouse) DoUpdate(ctx context.Context, id string, config *sql.CreateWarehouseRequest, _ Changes) (*sql.GetWarehouseResponse, error) { diff --git a/libs/structs/fieldcopy/fieldcopy.go b/libs/structs/fieldcopy/fieldcopy.go index 92142fd647..627f9ae036 100644 --- a/libs/structs/fieldcopy/fieldcopy.go +++ b/libs/structs/fieldcopy/fieldcopy.go @@ -12,7 +12,6 @@ import ( "reflect" "sort" "strings" - "sync" ) const forceSendFieldsName = "ForceSendFields" @@ -21,21 +20,27 @@ const forceSendFieldsName = "ForceSendFields" // Fields with matching names and assignable types are copied automatically. // Unmatched fields are left at zero and reported via [Copy.Report] for // golden file testing. +// +// Call [Copy.Init] before using [Copy.Do]; Do panics if Init was not called. type Copy[Src, Dst any] struct { // Rename maps destination field name to source field name for fields // with different names in source and destination. Rename map[string]string - once sync.Once copyFn func(src *Src) Dst } +// Init eagerly computes field mappings via reflection. Must be called before Do. +func (c *Copy[Src, Dst]) Init() { + c.copyFn = c.build() +} + // Do copies fields from src to a new Dst value using precomputed field mappings. -// On first call, field mappings are computed via reflection and cached. +// Panics if [Copy.Init] was not called. func (c *Copy[Src, Dst]) Do(src *Src) Dst { - c.once.Do(func() { - c.copyFn = c.build() - }) + if c.copyFn == nil { + panic(fmt.Sprintf("fieldcopy: Do called on uninitialized Copy[%v, %v]; call Init first", reflect.TypeFor[Src](), reflect.TypeFor[Dst]())) + } return c.copyFn(src) } diff --git a/libs/structs/fieldcopy/fieldcopy_test.go b/libs/structs/fieldcopy/fieldcopy_test.go index 643014ede8..ef0246cbc9 100644 --- a/libs/structs/fieldcopy/fieldcopy_test.go +++ b/libs/structs/fieldcopy/fieldcopy_test.go @@ -23,6 +23,7 @@ type dstBasic struct { func TestDoBasicCopy(t *testing.T) { c := fieldcopy.Copy[srcBasic, dstBasic]{} + c.Init() src := srcBasic{Name: "alice", Age: 30, Email: "a@b.c"} dst := c.Do(&src) assert.Equal(t, "alice", dst.Name) @@ -32,6 +33,7 @@ func TestDoBasicCopy(t *testing.T) { func TestDoCachedSecondCall(t *testing.T) { c := fieldcopy.Copy[srcBasic, dstBasic]{} + c.Init() src1 := srcBasic{Name: "alice", Age: 30, Email: "a@b.c"} dst1 := c.Do(&src1) assert.Equal(t, "alice", dst1.Name) @@ -55,6 +57,7 @@ type dstSmall struct { func TestDoUnmatchedFieldsIgnored(t *testing.T) { c := fieldcopy.Copy[srcWithExtra, dstSmall]{} + c.Init() src := srcWithExtra{Name: "alice", Age: 30, Extra: "ignored"} dst := c.Do(&src) assert.Equal(t, "alice", dst.Name) @@ -69,6 +72,7 @@ type dstWithDefault struct { func TestDoUnmatchedDstLeftAtZero(t *testing.T) { c := fieldcopy.Copy[srcBasic, dstWithDefault]{} + c.Init() src := srcBasic{Name: "alice", Age: 30, Email: "a@b.c"} dst := c.Do(&src) assert.Equal(t, "alice", dst.Name) @@ -90,6 +94,7 @@ func TestDoRename(t *testing.T) { c := fieldcopy.Copy[srcRenamed, dstRenamed]{ Rename: map[string]string{"Name": "FullName"}, } + c.Init() src := srcRenamed{FullName: "alice", Age: 30} dst := c.Do(&src) assert.Equal(t, "alice", dst.Name) @@ -98,12 +103,14 @@ func TestDoRename(t *testing.T) { func TestReportAllMatched(t *testing.T) { c := fieldcopy.Copy[srcBasic, dstBasic]{} + c.Init() report := c.Report() assert.Contains(t, report, "all fields matched") } func TestReportUnmatchedSrc(t *testing.T) { c := fieldcopy.Copy[srcWithExtra, dstSmall]{} + c.Init() report := c.Report() assert.Contains(t, report, "src not copied:") assert.Contains(t, report, "- Extra") @@ -111,6 +118,7 @@ func TestReportUnmatchedSrc(t *testing.T) { func TestReportUnmatchedDst(t *testing.T) { c := fieldcopy.Copy[srcBasic, dstWithDefault]{} + c.Init() report := c.Report() assert.Contains(t, report, "dst not set:") assert.Contains(t, report, "- Default") @@ -143,6 +151,7 @@ type srcTypeMismatch struct { func TestDoTypeMismatchFieldSkipped(t *testing.T) { c := fieldcopy.Copy[srcTypeMismatch, dstBasic]{} + c.Init() src := srcTypeMismatch{Name: "alice", Age: "30"} dst := c.Do(&src) assert.Equal(t, "alice", dst.Name) @@ -152,6 +161,7 @@ func TestDoTypeMismatchFieldSkipped(t *testing.T) { func TestReportTypeMismatch(t *testing.T) { c := fieldcopy.Copy[srcTypeMismatch, dstBasic]{} + c.Init() report := c.Report() // Age exists on both but types don't match, so it's unmatched on both sides. assert.Contains(t, report, "src not copied:") @@ -170,6 +180,7 @@ type dstPointer struct { func TestDoPointerFields(t *testing.T) { c := fieldcopy.Copy[srcPointer, dstPointer]{} + c.Init() items := []string{"a", "b"} src := srcPointer{Name: "alice", Items: &items} dst := c.Do(&src) @@ -182,6 +193,7 @@ func TestDoPointerFields(t *testing.T) { func TestDoNilPointerFields(t *testing.T) { c := fieldcopy.Copy[srcPointer, dstPointer]{} + c.Init() src := srcPointer{Name: "alice", Items: nil} dst := c.Do(&src) assert.Equal(t, "alice", dst.Name) @@ -198,6 +210,7 @@ type dstMap struct { func TestDoMapFields(t *testing.T) { c := fieldcopy.Copy[srcMap, dstMap]{} + c.Init() src := srcMap{Tags: map[string]string{"k": "v"}} dst := c.Do(&src) assert.Equal(t, map[string]string{"k": "v"}, dst.Tags) @@ -216,6 +229,7 @@ type dstSlice struct { func TestDoSliceFields(t *testing.T) { c := fieldcopy.Copy[srcSlice, dstSlice]{} + c.Init() src := srcSlice{Items: []string{"a", "b"}} dst := c.Do(&src) assert.Equal(t, []string{"a", "b"}, dst.Items) @@ -223,6 +237,7 @@ func TestDoSliceFields(t *testing.T) { func TestDoZeroValue(t *testing.T) { c := fieldcopy.Copy[srcBasic, dstBasic]{} + c.Init() src := srcBasic{} dst := c.Do(&src) assert.Equal(t, "", dst.Name) @@ -242,6 +257,7 @@ type dstBool struct { func TestDoBoolZeroValue(t *testing.T) { c := fieldcopy.Copy[srcBool, dstBool]{} + c.Init() src := srcBool{Enabled: false, Name: "test"} dst := c.Do(&src) assert.Equal(t, false, dst.Enabled) @@ -264,6 +280,7 @@ type nestedConfig struct { func TestDoNestedStructPointer(t *testing.T) { c := fieldcopy.Copy[srcNested, dstNested]{} + c.Init() src := srcNested{Name: "alice", Config: &nestedConfig{Value: 42}} dst := c.Do(&src) assert.Equal(t, "alice", dst.Name) @@ -284,6 +301,7 @@ type dstPrivate struct { func TestDoIgnoresUnexportedFields(t *testing.T) { c := fieldcopy.Copy[srcPrivate, dstPrivate]{} + c.Init() dst := c.Do(&srcPrivate{Name: "test"}) assert.Equal(t, "test", dst.Name) } @@ -305,6 +323,7 @@ type dstFSF struct { func TestDoForceSendFieldsAutoFiltered(t *testing.T) { c := fieldcopy.Copy[srcFSF, dstFSF]{} + c.Init() src := srcFSF{ Name: "alice", Age: 30, @@ -319,6 +338,7 @@ func TestDoForceSendFieldsAutoFiltered(t *testing.T) { func TestDoForceSendFieldsNil(t *testing.T) { c := fieldcopy.Copy[srcFSF, dstFSF]{} + c.Init() src := srcFSF{Name: "alice", ForceSendFields: nil} dst := c.Do(&src) assert.Equal(t, "alice", dst.Name) @@ -327,6 +347,7 @@ func TestDoForceSendFieldsNil(t *testing.T) { func TestDoForceSendFieldsEmpty(t *testing.T) { c := fieldcopy.Copy[srcFSF, dstFSF]{} + c.Init() src := srcFSF{Name: "alice", ForceSendFields: []string{}} dst := c.Do(&src) assert.Nil(t, dst.ForceSendFields) @@ -334,6 +355,7 @@ func TestDoForceSendFieldsEmpty(t *testing.T) { func TestDoForceSendFieldsAllFiltered(t *testing.T) { c := fieldcopy.Copy[srcFSF, dstFSF]{} + c.Init() src := srcFSF{ForceSendFields: []string{"Extra", "NonExistent"}} dst := c.Do(&src) // All entries filtered out → nil. @@ -355,6 +377,7 @@ type outerDstFSF struct { func TestDoForceSendFieldsNotAutoHandledWhenPromoted(t *testing.T) { c := fieldcopy.Copy[srcFSF, outerDstFSF]{} + c.Init() src := srcFSF{ Name: "alice", Extra: "x", @@ -369,9 +392,17 @@ func TestDoForceSendFieldsNotAutoHandledWhenPromoted(t *testing.T) { func TestReportEmbeddedStructShowsAsField(t *testing.T) { c := fieldcopy.Copy[srcFSF, outerDstFSF]{} + c.Init() report := c.Report() // The embedded struct itself appears as unmatched dst field. assert.Contains(t, report, "- InnerFSF") // ForceSendFields on src is not auto-handled, so it shows as unmatched. assert.Contains(t, report, "- ForceSendFields") } + +func TestDoPanicsWithoutInit(t *testing.T) { + c := fieldcopy.Copy[srcBasic, dstBasic]{} + assert.PanicsWithValue(t, "fieldcopy: Do called on uninitialized Copy[fieldcopy_test.srcBasic, fieldcopy_test.dstBasic]; call Init first", func() { + c.Do(&srcBasic{}) + }) +} From 65d3b11ae0eb08b0506192bf5000ea0d82072ecc Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 23 Mar 2026 16:51:59 +0100 Subject: [PATCH 09/16] Inline small fieldcopy wrappers and move init() to bottom of file Inline makeCreateCluster, makeEditCluster, makeResetJob into their callers. Extract shared forceNumWorkers helper for the duplicated NumWorkers logic. Move all init() functions to the bottom of each resource file and declare copy vars near the first function that uses them. Co-authored-by: Isaac --- bundle/direct/dresources/catalog.go | 10 ++-- bundle/direct/dresources/cluster.go | 47 +++++++------------ bundle/direct/dresources/external_location.go | 10 ++-- bundle/direct/dresources/job.go | 33 +++++-------- .../dresources/model_serving_endpoint.go | 12 ++--- bundle/direct/dresources/pipeline.go | 12 ++--- bundle/direct/dresources/quality_monitor.go | 10 ++-- bundle/direct/dresources/registered_model.go | 10 ++-- bundle/direct/dresources/sql_warehouse.go | 10 ++-- 9 files changed, 67 insertions(+), 87 deletions(-) diff --git a/bundle/direct/dresources/catalog.go b/bundle/direct/dresources/catalog.go index e801ace25e..91e9c75046 100644 --- a/bundle/direct/dresources/catalog.go +++ b/bundle/direct/dresources/catalog.go @@ -44,11 +44,6 @@ func (r *ResourceCatalog) DoCreate(ctx context.Context, config *catalog.CreateCa // catalogUpdateCopy maps CreateCatalog (local state) to UpdateCatalog (API request). var catalogUpdateCopy fieldcopy.Copy[catalog.CreateCatalog, catalog.UpdateCatalog] -func init() { - registerCopy(&catalogRemapCopy) - registerCopy(&catalogUpdateCopy) -} - // DoUpdate updates the catalog in place and returns remote state. func (r *ResourceCatalog) DoUpdate(ctx context.Context, id string, config *catalog.CreateCatalog, _ Changes) (*catalog.CatalogInfo, error) { updateRequest := catalogUpdateCopy.Do(config) @@ -92,3 +87,8 @@ func (r *ResourceCatalog) DoDelete(ctx context.Context, id string) error { ForceSendFields: nil, }) } + +func init() { + registerCopy(&catalogRemapCopy) + registerCopy(&catalogUpdateCopy) +} diff --git a/bundle/direct/dresources/cluster.go b/bundle/direct/dresources/cluster.go index bcca922f41..dd5f63c101 100644 --- a/bundle/direct/dresources/cluster.go +++ b/bundle/direct/dresources/cluster.go @@ -46,20 +46,32 @@ func (r *ResourceCluster) DoRead(ctx context.Context, id string) (*compute.Clust return r.client.Clusters.GetByClusterId(ctx, id) } +// clusterCreateCopy maps ClusterSpec (local state) to CreateCluster (API request). +var clusterCreateCopy fieldcopy.Copy[compute.ClusterSpec, compute.CreateCluster] + func (r *ResourceCluster) DoCreate(ctx context.Context, config *compute.ClusterSpec) (string, *compute.ClusterDetails, error) { - wait, err := r.client.Clusters.Create(ctx, makeCreateCluster(config)) + create := clusterCreateCopy.Do(config) + forceNumWorkers(config, &create.ForceSendFields) + wait, err := r.client.Clusters.Create(ctx, create) if err != nil { return "", nil, err } return wait.ClusterId, nil, nil } +// clusterEditCopy maps ClusterSpec (local state) to EditCluster (API request). +var clusterEditCopy fieldcopy.Copy[compute.ClusterSpec, compute.EditCluster] + func (r *ResourceCluster) DoUpdate(ctx context.Context, id string, config *compute.ClusterSpec, _ Changes) (*compute.ClusterDetails, error) { + edit := clusterEditCopy.Do(config) + edit.ClusterId = id + forceNumWorkers(config, &edit.ForceSendFields) + // Same retry as in TF provider logic // https://github.com/databricks/terraform-provider-databricks/blob/3eecd0f90cf99d7777e79a3d03c41f9b2aafb004/clusters/resource_cluster.go#L624 timeout := 15 * time.Minute _, err := retries.Poll(ctx, timeout, func() (*compute.WaitGetClusterRunning[struct{}], *retries.Err) { - wait, err := r.client.Clusters.Edit(ctx, makeEditCluster(id, config)) + wait, err := r.client.Clusters.Edit(ctx, edit) if err == nil { return wait, nil } @@ -119,39 +131,16 @@ func (r *ResourceCluster) OverrideChangeDesc(ctx context.Context, p *structpath. return nil } -// clusterCreateCopy maps ClusterSpec (local state) to CreateCluster (API request). -var clusterCreateCopy fieldcopy.Copy[compute.ClusterSpec, compute.CreateCluster] - -func makeCreateCluster(config *compute.ClusterSpec) compute.CreateCluster { - create := clusterCreateCopy.Do(config) - - // If autoscale is not set, we need to send NumWorkers because one of them is required. - // If NumWorkers is not nil, we don't need to set it to ForceSendFields as it will be sent anyway. +// forceNumWorkers ensures NumWorkers is sent when Autoscale is not set, +// because the API requires one of them. +func forceNumWorkers(config *compute.ClusterSpec, fsf *[]string) { if config.Autoscale == nil && config.NumWorkers == 0 { - create.ForceSendFields = append(create.ForceSendFields, "NumWorkers") + *fsf = append(*fsf, "NumWorkers") } - - return create } -// clusterEditCopy maps ClusterSpec (local state) to EditCluster (API request). -var clusterEditCopy fieldcopy.Copy[compute.ClusterSpec, compute.EditCluster] - func init() { registerCopy(&clusterRemapCopy) registerCopy(&clusterCreateCopy) registerCopy(&clusterEditCopy) } - -func makeEditCluster(id string, config *compute.ClusterSpec) compute.EditCluster { - edit := clusterEditCopy.Do(config) - edit.ClusterId = id - - // If autoscale is not set, we need to send NumWorkers because one of them is required. - // If NumWorkers is not nil, we don't need to set it to ForceSendFields as it will be sent anyway. - if config.Autoscale == nil && config.NumWorkers == 0 { - edit.ForceSendFields = append(edit.ForceSendFields, "NumWorkers") - } - - return edit -} diff --git a/bundle/direct/dresources/external_location.go b/bundle/direct/dresources/external_location.go index 0961372e65..f4d8843257 100644 --- a/bundle/direct/dresources/external_location.go +++ b/bundle/direct/dresources/external_location.go @@ -44,11 +44,6 @@ func (r *ResourceExternalLocation) DoCreate(ctx context.Context, config *catalog // externalLocationUpdateCopy maps CreateExternalLocation (local state) to UpdateExternalLocation (API request). var externalLocationUpdateCopy fieldcopy.Copy[catalog.CreateExternalLocation, catalog.UpdateExternalLocation] -func init() { - registerCopy(&externalLocationRemapCopy) - registerCopy(&externalLocationUpdateCopy) -} - // DoUpdate updates the external location in place and returns remote state. func (r *ResourceExternalLocation) DoUpdate(ctx context.Context, id string, config *catalog.CreateExternalLocation, _ Changes) (*catalog.ExternalLocationInfo, error) { updateRequest := externalLocationUpdateCopy.Do(config) @@ -87,3 +82,8 @@ func (r *ResourceExternalLocation) DoDelete(ctx context.Context, id string) erro ForceSendFields: nil, }) } + +func init() { + registerCopy(&externalLocationRemapCopy) + registerCopy(&externalLocationUpdateCopy) +} diff --git a/bundle/direct/dresources/job.go b/bundle/direct/dresources/job.go index 116949bcba..5469e2fa0c 100644 --- a/bundle/direct/dresources/job.go +++ b/bundle/direct/dresources/job.go @@ -100,6 +100,9 @@ func makeJobRemote(job *jobs.Job) *JobRemote { } } +// jobCreateCopy maps JobSettings (local state) to CreateJob (API request). +var jobCreateCopy fieldcopy.Copy[jobs.JobSettings, jobs.CreateJob] + func (r *ResourceJob) DoCreate(ctx context.Context, config *jobs.JobSettings) (string, *JobRemote, error) { response, err := r.client.Jobs.Create(ctx, jobCreateCopy.Do(config)) if err != nil { @@ -109,11 +112,14 @@ func (r *ResourceJob) DoCreate(ctx context.Context, config *jobs.JobSettings) (s } func (r *ResourceJob) DoUpdate(ctx context.Context, id string, config *jobs.JobSettings, _ Changes) (*JobRemote, error) { - request, err := makeResetJob(*config, id) + idInt, err := parseJobID(id) if err != nil { return nil, err } - return nil, r.client.Jobs.Reset(ctx, request) + return nil, r.client.Jobs.Reset(ctx, jobs.ResetJob{ + JobId: idInt, + NewSettings: *config, + }) } func (r *ResourceJob) DoDelete(ctx context.Context, id string) error { @@ -124,25 +130,6 @@ func (r *ResourceJob) DoDelete(ctx context.Context, id string) error { return r.client.Jobs.DeleteByJobId(ctx, idInt) } -// jobCreateCopy maps JobSettings (local state) to CreateJob (API request). -var jobCreateCopy fieldcopy.Copy[jobs.JobSettings, jobs.CreateJob] - -func init() { - registerCopy(&jobCreateCopy) -} - -func makeResetJob(config jobs.JobSettings, id string) (jobs.ResetJob, error) { - idInt, err := parseJobID(id) - if err != nil { - return jobs.ResetJob{}, err - } - result := jobs.ResetJob{ - JobId: idInt, - NewSettings: config, - } - return result, err -} - func parseJobID(id string) (int64, error) { result, err := strconv.ParseInt(id, 10, 64) if err != nil { @@ -150,3 +137,7 @@ func parseJobID(id string) (int64, error) { } return result, nil } + +func init() { + registerCopy(&jobCreateCopy) +} diff --git a/bundle/direct/dresources/model_serving_endpoint.go b/bundle/direct/dresources/model_serving_endpoint.go index bb94ed4619..633915d25a 100644 --- a/bundle/direct/dresources/model_serving_endpoint.go +++ b/bundle/direct/dresources/model_serving_endpoint.go @@ -72,12 +72,6 @@ func configOutputToInput(output *serving.EndpointCoreConfigOutput) *serving.Endp // servingRemapCopy maps ServingEndpointDetailed (remote GET response) to CreateServingEndpoint (local state). var servingRemapCopy fieldcopy.Copy[serving.ServingEndpointDetailed, serving.CreateServingEndpoint] -func init() { - registerCopy(&autoCaptureConfigCopy) - registerCopy(&servedEntityCopy) - registerCopy(&servingRemapCopy) -} - func (*ResourceModelServingEndpoint) RemapState(state *RefreshOutput) *serving.CreateServingEndpoint { details := state.EndpointDetails result := servingRemapCopy.Do(details) @@ -303,3 +297,9 @@ func (r *ResourceModelServingEndpoint) DoUpdate(ctx context.Context, id string, func (r *ResourceModelServingEndpoint) DoDelete(ctx context.Context, id string) error { return r.client.ServingEndpoints.DeleteByName(ctx, id) } + +func init() { + registerCopy(&autoCaptureConfigCopy) + registerCopy(&servedEntityCopy) + registerCopy(&servingRemapCopy) +} diff --git a/bundle/direct/dresources/pipeline.go b/bundle/direct/dresources/pipeline.go index 2f612b4a5d..32af98ddec 100644 --- a/bundle/direct/dresources/pipeline.go +++ b/bundle/direct/dresources/pipeline.go @@ -91,12 +91,6 @@ func (r *ResourcePipeline) DoCreate(ctx context.Context, config *pipelines.Creat // pipelineEditCopy maps CreatePipeline (local state) to EditPipeline (API request). var pipelineEditCopy fieldcopy.Copy[pipelines.CreatePipeline, pipelines.EditPipeline] -func init() { - registerCopy(&pipelineSpecCopy) - registerCopy(&pipelineRemoteCopy) - registerCopy(&pipelineEditCopy) -} - func (r *ResourcePipeline) DoUpdate(ctx context.Context, id string, config *pipelines.CreatePipeline, _ Changes) (*PipelineRemote, error) { request := pipelineEditCopy.Do(config) request.PipelineId = id @@ -111,3 +105,9 @@ func (r *ResourcePipeline) DoDelete(ctx context.Context, id string) error { // a) reads back state at least once and fails create if state is "failed" // b) repeatededly reads state until state is "running" (if spec.Contionous is set). // TODO: investigate if we need to mimic this behaviour or can rely on Create status code. + +func init() { + registerCopy(&pipelineSpecCopy) + registerCopy(&pipelineRemoteCopy) + registerCopy(&pipelineEditCopy) +} diff --git a/bundle/direct/dresources/quality_monitor.go b/bundle/direct/dresources/quality_monitor.go index 23899648ab..95aedb8fb4 100644 --- a/bundle/direct/dresources/quality_monitor.go +++ b/bundle/direct/dresources/quality_monitor.go @@ -72,11 +72,6 @@ func (r *ResourceQualityMonitor) DoCreate(ctx context.Context, config *QualityMo // qualityMonitorUpdateCopy maps CreateMonitor (local state) to UpdateMonitor (API request). var qualityMonitorUpdateCopy fieldcopy.Copy[catalog.CreateMonitor, catalog.UpdateMonitor] -func init() { - registerCopy(&qualityMonitorRemapCopy) - registerCopy(&qualityMonitorUpdateCopy) -} - func (r *ResourceQualityMonitor) DoUpdate(ctx context.Context, id string, config *QualityMonitorState, _ Changes) (*catalog.MonitorInfo, error) { updateRequest := qualityMonitorUpdateCopy.Do(&config.CreateMonitor) updateRequest.TableName = id @@ -97,3 +92,8 @@ func (r *ResourceQualityMonitor) DoDelete(ctx context.Context, id string) error }) return err } + +func init() { + registerCopy(&qualityMonitorRemapCopy) + registerCopy(&qualityMonitorUpdateCopy) +} diff --git a/bundle/direct/dresources/registered_model.go b/bundle/direct/dresources/registered_model.go index 14b28db500..47ae0675b3 100644 --- a/bundle/direct/dresources/registered_model.go +++ b/bundle/direct/dresources/registered_model.go @@ -52,11 +52,6 @@ func (r *ResourceRegisteredModel) DoCreate(ctx context.Context, config *catalog. // registeredModelUpdateCopy maps CreateRegisteredModelRequest (local state) to UpdateRegisteredModelRequest (API request). var registeredModelUpdateCopy fieldcopy.Copy[catalog.CreateRegisteredModelRequest, catalog.UpdateRegisteredModelRequest] -func init() { - registerCopy(®isteredModelRemapCopy) - registerCopy(®isteredModelUpdateCopy) -} - func (r *ResourceRegisteredModel) DoUpdate(ctx context.Context, id string, config *catalog.CreateRegisteredModelRequest, _ Changes) (*catalog.RegisteredModelInfo, error) { updateRequest := registeredModelUpdateCopy.Do(config) updateRequest.FullName = id @@ -74,3 +69,8 @@ func (r *ResourceRegisteredModel) DoDelete(ctx context.Context, id string) error FullName: id, }) } + +func init() { + registerCopy(®isteredModelRemapCopy) + registerCopy(®isteredModelUpdateCopy) +} diff --git a/bundle/direct/dresources/sql_warehouse.go b/bundle/direct/dresources/sql_warehouse.go index 5f91b32f38..47f493f472 100644 --- a/bundle/direct/dresources/sql_warehouse.go +++ b/bundle/direct/dresources/sql_warehouse.go @@ -51,11 +51,6 @@ func (r *ResourceSqlWarehouse) DoCreate(ctx context.Context, config *sql.CreateW // sqlWarehouseEditCopy maps CreateWarehouseRequest (local state) to EditWarehouseRequest (API request). var sqlWarehouseEditCopy fieldcopy.Copy[sql.CreateWarehouseRequest, sql.EditWarehouseRequest] -func init() { - registerCopy(&sqlWarehouseRemapCopy) - registerCopy(&sqlWarehouseEditCopy) -} - // DoUpdate updates the warehouse in place. func (r *ResourceSqlWarehouse) DoUpdate(ctx context.Context, id string, config *sql.CreateWarehouseRequest, _ Changes) (*sql.GetWarehouseResponse, error) { request := sqlWarehouseEditCopy.Do(config) @@ -78,3 +73,8 @@ func (r *ResourceSqlWarehouse) DoUpdate(ctx context.Context, id string, config * func (r *ResourceSqlWarehouse) DoDelete(ctx context.Context, oldID string) error { return r.client.Warehouses.DeleteById(ctx, oldID) } + +func init() { + registerCopy(&sqlWarehouseRemapCopy) + registerCopy(&sqlWarehouseEditCopy) +} From 284fabd75a626696cdc0f890f2f410a730270b38 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 23 Mar 2026 17:02:57 +0100 Subject: [PATCH 10/16] Change fieldcopy.Do to return *Dst instead of Dst Returning a pointer eliminates the two-liner pattern in RemapState methods (assign to local, return &local) and is equally convenient for callers that modify the result after copying. Co-authored-by: Isaac --- bundle/direct/dresources/catalog.go | 7 +++---- bundle/direct/dresources/cluster.go | 6 +++--- bundle/direct/dresources/external_location.go | 7 +++---- bundle/direct/dresources/job.go | 2 +- bundle/direct/dresources/model_serving_endpoint.go | 7 +++---- bundle/direct/dresources/pipeline.go | 6 +++--- bundle/direct/dresources/quality_monitor.go | 4 ++-- bundle/direct/dresources/registered_model.go | 5 ++--- bundle/direct/dresources/sql_warehouse.go | 4 ++-- libs/structs/fieldcopy/fieldcopy.go | 10 +++++----- 10 files changed, 27 insertions(+), 31 deletions(-) diff --git a/bundle/direct/dresources/catalog.go b/bundle/direct/dresources/catalog.go index 91e9c75046..5a1b3cdff6 100644 --- a/bundle/direct/dresources/catalog.go +++ b/bundle/direct/dresources/catalog.go @@ -25,8 +25,7 @@ func (*ResourceCatalog) PrepareState(input *resources.Catalog) *catalog.CreateCa var catalogRemapCopy fieldcopy.Copy[catalog.CatalogInfo, catalog.CreateCatalog] func (*ResourceCatalog) RemapState(info *catalog.CatalogInfo) *catalog.CreateCatalog { - result := catalogRemapCopy.Do(info) - return &result + return catalogRemapCopy.Do(info) } func (r *ResourceCatalog) DoRead(ctx context.Context, id string) (*catalog.CatalogInfo, error) { @@ -49,7 +48,7 @@ func (r *ResourceCatalog) DoUpdate(ctx context.Context, id string, config *catal updateRequest := catalogUpdateCopy.Do(config) updateRequest.Name = id - response, err := r.client.Catalogs.Update(ctx, updateRequest) + response, err := r.client.Catalogs.Update(ctx, *updateRequest) if err != nil { return nil, err } @@ -66,7 +65,7 @@ func (r *ResourceCatalog) DoUpdateWithID(ctx context.Context, id string, config updateRequest.NewName = config.Name } - response, err := r.client.Catalogs.Update(ctx, updateRequest) + response, err := r.client.Catalogs.Update(ctx, *updateRequest) if err != nil { return "", nil, err } diff --git a/bundle/direct/dresources/cluster.go b/bundle/direct/dresources/cluster.go index dd5f63c101..529bb67a37 100644 --- a/bundle/direct/dresources/cluster.go +++ b/bundle/direct/dresources/cluster.go @@ -39,7 +39,7 @@ func (r *ResourceCluster) RemapState(input *compute.ClusterDetails) *compute.Clu if input.Spec != nil { spec.ApplyPolicyDefaultValues = input.Spec.ApplyPolicyDefaultValues } - return &spec + return spec } func (r *ResourceCluster) DoRead(ctx context.Context, id string) (*compute.ClusterDetails, error) { @@ -52,7 +52,7 @@ var clusterCreateCopy fieldcopy.Copy[compute.ClusterSpec, compute.CreateCluster] func (r *ResourceCluster) DoCreate(ctx context.Context, config *compute.ClusterSpec) (string, *compute.ClusterDetails, error) { create := clusterCreateCopy.Do(config) forceNumWorkers(config, &create.ForceSendFields) - wait, err := r.client.Clusters.Create(ctx, create) + wait, err := r.client.Clusters.Create(ctx, *create) if err != nil { return "", nil, err } @@ -71,7 +71,7 @@ func (r *ResourceCluster) DoUpdate(ctx context.Context, id string, config *compu // https://github.com/databricks/terraform-provider-databricks/blob/3eecd0f90cf99d7777e79a3d03c41f9b2aafb004/clusters/resource_cluster.go#L624 timeout := 15 * time.Minute _, err := retries.Poll(ctx, timeout, func() (*compute.WaitGetClusterRunning[struct{}], *retries.Err) { - wait, err := r.client.Clusters.Edit(ctx, edit) + wait, err := r.client.Clusters.Edit(ctx, *edit) if err == nil { return wait, nil } diff --git a/bundle/direct/dresources/external_location.go b/bundle/direct/dresources/external_location.go index f4d8843257..d3259ba8b1 100644 --- a/bundle/direct/dresources/external_location.go +++ b/bundle/direct/dresources/external_location.go @@ -25,8 +25,7 @@ func (*ResourceExternalLocation) PrepareState(input *resources.ExternalLocation) var externalLocationRemapCopy fieldcopy.Copy[catalog.ExternalLocationInfo, catalog.CreateExternalLocation] func (*ResourceExternalLocation) RemapState(info *catalog.ExternalLocationInfo) *catalog.CreateExternalLocation { - result := externalLocationRemapCopy.Do(info) - return &result + return externalLocationRemapCopy.Do(info) } func (r *ResourceExternalLocation) DoRead(ctx context.Context, id string) (*catalog.ExternalLocationInfo, error) { @@ -49,7 +48,7 @@ func (r *ResourceExternalLocation) DoUpdate(ctx context.Context, id string, conf updateRequest := externalLocationUpdateCopy.Do(config) updateRequest.Name = id - return r.client.ExternalLocations.Update(ctx, updateRequest) + return r.client.ExternalLocations.Update(ctx, *updateRequest) } // DoUpdateWithID updates the external location and returns the new ID if the name changes. @@ -61,7 +60,7 @@ func (r *ResourceExternalLocation) DoUpdateWithID(ctx context.Context, id string updateRequest.NewName = config.Name } - response, err := r.client.ExternalLocations.Update(ctx, updateRequest) + response, err := r.client.ExternalLocations.Update(ctx, *updateRequest) if err != nil { return "", nil, err } diff --git a/bundle/direct/dresources/job.go b/bundle/direct/dresources/job.go index 5469e2fa0c..b97c14c35e 100644 --- a/bundle/direct/dresources/job.go +++ b/bundle/direct/dresources/job.go @@ -104,7 +104,7 @@ func makeJobRemote(job *jobs.Job) *JobRemote { var jobCreateCopy fieldcopy.Copy[jobs.JobSettings, jobs.CreateJob] func (r *ResourceJob) DoCreate(ctx context.Context, config *jobs.JobSettings) (string, *JobRemote, error) { - response, err := r.client.Jobs.Create(ctx, jobCreateCopy.Do(config)) + response, err := r.client.Jobs.Create(ctx, *jobCreateCopy.Do(config)) if err != nil { return "", nil, err } diff --git a/bundle/direct/dresources/model_serving_endpoint.go b/bundle/direct/dresources/model_serving_endpoint.go index 633915d25a..a1bfa41925 100644 --- a/bundle/direct/dresources/model_serving_endpoint.go +++ b/bundle/direct/dresources/model_serving_endpoint.go @@ -41,8 +41,7 @@ func autoCaptureConfigOutputToInput(output *serving.AutoCaptureConfigOutput) *se if output == nil { return nil } - result := autoCaptureConfigCopy.Do(output) - return &result + return autoCaptureConfigCopy.Do(output) } // servedEntityCopy maps ServedEntityOutput to ServedEntityInput. @@ -51,7 +50,7 @@ var servedEntityCopy fieldcopy.Copy[serving.ServedEntityOutput, serving.ServedEn func servedEntitiesOutputToInput(output []serving.ServedEntityOutput) []serving.ServedEntityInput { entities := make([]serving.ServedEntityInput, len(output)) for i, entity := range output { - entities[i] = servedEntityCopy.Do(&entity) + entities[i] = *servedEntityCopy.Do(&entity) } return entities } @@ -76,7 +75,7 @@ func (*ResourceModelServingEndpoint) RemapState(state *RefreshOutput) *serving.C details := state.EndpointDetails result := servingRemapCopy.Do(details) result.Config = configOutputToInput(details.Config) - return &result + return result } type RefreshOutput struct { diff --git a/bundle/direct/dresources/pipeline.go b/bundle/direct/dresources/pipeline.go index 32af98ddec..355bc0572f 100644 --- a/bundle/direct/dresources/pipeline.go +++ b/bundle/direct/dresources/pipeline.go @@ -74,10 +74,10 @@ var pipelineRemoteCopy fieldcopy.Copy[pipelines.GetPipelineResponse, PipelineRem func makePipelineRemote(p *pipelines.GetPipelineResponse) *PipelineRemote { remote := pipelineRemoteCopy.Do(p) if p.Spec != nil { - remote.CreatePipeline = pipelineSpecCopy.Do(p.Spec) + remote.CreatePipeline = *pipelineSpecCopy.Do(p.Spec) remote.CreatePipeline.RunAs = p.RunAs } - return &remote + return remote } func (r *ResourcePipeline) DoCreate(ctx context.Context, config *pipelines.CreatePipeline) (string, *PipelineRemote, error) { @@ -94,7 +94,7 @@ var pipelineEditCopy fieldcopy.Copy[pipelines.CreatePipeline, pipelines.EditPipe func (r *ResourcePipeline) DoUpdate(ctx context.Context, id string, config *pipelines.CreatePipeline, _ Changes) (*PipelineRemote, error) { request := pipelineEditCopy.Do(config) request.PipelineId = id - return nil, r.client.Pipelines.Update(ctx, request) + return nil, r.client.Pipelines.Update(ctx, *request) } func (r *ResourcePipeline) DoDelete(ctx context.Context, id string) error { diff --git a/bundle/direct/dresources/quality_monitor.go b/bundle/direct/dresources/quality_monitor.go index 95aedb8fb4..14063d43ad 100644 --- a/bundle/direct/dresources/quality_monitor.go +++ b/bundle/direct/dresources/quality_monitor.go @@ -46,7 +46,7 @@ var qualityMonitorRemapCopy fieldcopy.Copy[catalog.MonitorInfo, catalog.CreateMo func (*ResourceQualityMonitor) RemapState(info *catalog.MonitorInfo) *QualityMonitorState { return &QualityMonitorState{ - CreateMonitor: qualityMonitorRemapCopy.Do(info), + CreateMonitor: *qualityMonitorRemapCopy.Do(info), TableName: info.TableName, } } @@ -77,7 +77,7 @@ func (r *ResourceQualityMonitor) DoUpdate(ctx context.Context, id string, config updateRequest.TableName = id //nolint:staticcheck // Direct quality_monitor resource still uses legacy monitor endpoints; v1 data-quality migration is separate work. - response, err := r.client.QualityMonitors.Update(ctx, updateRequest) + response, err := r.client.QualityMonitors.Update(ctx, *updateRequest) if err != nil { return nil, err } diff --git a/bundle/direct/dresources/registered_model.go b/bundle/direct/dresources/registered_model.go index 47ae0675b3..2e090d13e4 100644 --- a/bundle/direct/dresources/registered_model.go +++ b/bundle/direct/dresources/registered_model.go @@ -27,8 +27,7 @@ func (*ResourceRegisteredModel) PrepareState(input *resources.RegisteredModel) * var registeredModelRemapCopy fieldcopy.Copy[catalog.RegisteredModelInfo, catalog.CreateRegisteredModelRequest] func (*ResourceRegisteredModel) RemapState(model *catalog.RegisteredModelInfo) *catalog.CreateRegisteredModelRequest { - result := registeredModelRemapCopy.Do(model) - return &result + return registeredModelRemapCopy.Do(model) } func (r *ResourceRegisteredModel) DoRead(ctx context.Context, id string) (*catalog.RegisteredModelInfo, error) { @@ -56,7 +55,7 @@ func (r *ResourceRegisteredModel) DoUpdate(ctx context.Context, id string, confi updateRequest := registeredModelUpdateCopy.Do(config) updateRequest.FullName = id - response, err := r.client.RegisteredModels.Update(ctx, updateRequest) + response, err := r.client.RegisteredModels.Update(ctx, *updateRequest) if err != nil { return nil, err } diff --git a/bundle/direct/dresources/sql_warehouse.go b/bundle/direct/dresources/sql_warehouse.go index 47f493f472..163f306231 100644 --- a/bundle/direct/dresources/sql_warehouse.go +++ b/bundle/direct/dresources/sql_warehouse.go @@ -31,7 +31,7 @@ func (*ResourceSqlWarehouse) RemapState(warehouse *sql.GetWarehouseResponse) *sq result := sqlWarehouseRemapCopy.Do(warehouse) // WarehouseType requires explicit conversion between different named string types. result.WarehouseType = sql.CreateWarehouseRequestWarehouseType(warehouse.WarehouseType) - return &result + return result } // DoRead reads the warehouse by id. @@ -58,7 +58,7 @@ func (r *ResourceSqlWarehouse) DoUpdate(ctx context.Context, id string, config * // WarehouseType requires explicit conversion between different named string types. request.WarehouseType = sql.EditWarehouseRequestWarehouseType(config.WarehouseType) - waiter, err := r.client.Warehouses.Edit(ctx, request) + waiter, err := r.client.Warehouses.Edit(ctx, *request) if err != nil { return nil, err } diff --git a/libs/structs/fieldcopy/fieldcopy.go b/libs/structs/fieldcopy/fieldcopy.go index 627f9ae036..cb6941e746 100644 --- a/libs/structs/fieldcopy/fieldcopy.go +++ b/libs/structs/fieldcopy/fieldcopy.go @@ -27,7 +27,7 @@ type Copy[Src, Dst any] struct { // with different names in source and destination. Rename map[string]string - copyFn func(src *Src) Dst + copyFn func(src *Src) *Dst } // Init eagerly computes field mappings via reflection. Must be called before Do. @@ -37,7 +37,7 @@ func (c *Copy[Src, Dst]) Init() { // Do copies fields from src to a new Dst value using precomputed field mappings. // Panics if [Copy.Init] was not called. -func (c *Copy[Src, Dst]) Do(src *Src) Dst { +func (c *Copy[Src, Dst]) Do(src *Src) *Dst { if c.copyFn == nil { panic(fmt.Sprintf("fieldcopy: Do called on uninitialized Copy[%v, %v]; call Init first", reflect.TypeFor[Src](), reflect.TypeFor[Dst]())) } @@ -49,7 +49,7 @@ type fieldOp struct { srcIndex []int } -func (c *Copy[Src, Dst]) build() func(*Src) Dst { +func (c *Copy[Src, Dst]) build() func(*Src) *Dst { srcType := reflect.TypeFor[Src]() dstType := reflect.TypeFor[Dst]() @@ -99,7 +99,7 @@ func (c *Copy[Src, Dst]) build() func(*Src) Dst { } } - return func(src *Src) Dst { + return func(src *Src) *Dst { var dst Dst sv := reflect.ValueOf(src).Elem() dv := reflect.ValueOf(&dst).Elem() @@ -121,7 +121,7 @@ func (c *Copy[Src, Dst]) build() func(*Src) Dst { } } } - return dst + return &dst } } From 0ee953b63d867c6dfb2148073c83f6bf8aa43713 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 23 Mar 2026 17:13:32 +0100 Subject: [PATCH 11/16] Add tools/update_golden.py to regenerate golden files for testdiff packages Co-authored-by: Isaac --- tools/update_golden.py | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100755 tools/update_golden.py diff --git a/tools/update_golden.py b/tools/update_golden.py new file mode 100755 index 0000000000..ff47c50c34 --- /dev/null +++ b/tools/update_golden.py @@ -0,0 +1,33 @@ +#!/usr/bin/env python3 +"""Run go test -update for all packages that use libs/testdiff.""" +import os +import subprocess +import sys + +from pathlib import Path + + +def main(): + result = subprocess.run( + ["git", "grep", "-l", "libs/testdiff", "--", "**/*_test.go"], + capture_output=False, + stdout=subprocess.PIPE, + text=True, + check=True, + ) + + dirs = {str(Path(f).parent) for f in result.stdout.splitlines() if f} + dirs.discard("acceptance") + packages = sorted(f"./{d}" for d in dirs) + + if not packages: + print("No packages found.", file=sys.stderr) + sys.exit(1) + + cmd = ["go", "test", *packages, "-update"] + print(" ".join(cmd), file=sys.stderr, flush=True) + os.execvp(cmd[0], cmd) + + +if __name__ == "__main__": + main() From b6e62adb12b8256cfd5d29b08095d7fc3b28788e Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 23 Mar 2026 17:21:38 +0100 Subject: [PATCH 12/16] update Makefile --- Makefile | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 5714483a36..573708d2b4 100644 --- a/Makefile +++ b/Makefile @@ -96,9 +96,14 @@ test-slow-acc: # Updates acceptance test output (local tests) .PHONY: test-update -test-update: +test-update: test-update-golden -go test ./acceptance -run '^TestAccept$$' -update -timeout=${LOCAL_TIMEOUT} +# Updates golden files test output (other than acceptance tests) +.PHONY: test-update-golden +test-update-golden: + ./tools/update_golden.py + # Updates acceptance test output for template tests only .PHONY: test-update-templates test-update-templates: From 7701a225408dd447f1d2b2fa84723165fb482522 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 23 Mar 2026 17:22:22 +0100 Subject: [PATCH 13/16] formater --- tools/update_golden.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/update_golden.py b/tools/update_golden.py index ff47c50c34..f9a243d201 100755 --- a/tools/update_golden.py +++ b/tools/update_golden.py @@ -1,5 +1,6 @@ #!/usr/bin/env python3 """Run go test -update for all packages that use libs/testdiff.""" + import os import subprocess import sys From 3cc068b75a1d4347abfcb54d71a51837abe9a02c Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 23 Mar 2026 17:29:23 +0100 Subject: [PATCH 14/16] Replace init() + registerCopy with newCopy one-liner declarations newCopy[Src, Dst]() creates, initializes, and registers a fieldcopy instance in one call, eliminating all init() functions from resource files. Co-authored-by: Isaac --- bundle/direct/dresources/catalog.go | 9 ++------- bundle/direct/dresources/cluster.go | 12 +++--------- bundle/direct/dresources/external_location.go | 9 ++------- bundle/direct/dresources/fieldcopy.go | 4 +++- bundle/direct/dresources/job.go | 6 +----- bundle/direct/dresources/model_serving_endpoint.go | 12 +++--------- bundle/direct/dresources/pipeline.go | 12 +++--------- bundle/direct/dresources/quality_monitor.go | 9 ++------- bundle/direct/dresources/registered_model.go | 9 ++------- bundle/direct/dresources/sql_warehouse.go | 9 ++------- 10 files changed, 23 insertions(+), 68 deletions(-) diff --git a/bundle/direct/dresources/catalog.go b/bundle/direct/dresources/catalog.go index 5a1b3cdff6..e350394440 100644 --- a/bundle/direct/dresources/catalog.go +++ b/bundle/direct/dresources/catalog.go @@ -4,7 +4,6 @@ import ( "context" "github.com/databricks/cli/bundle/config/resources" - "github.com/databricks/cli/libs/structs/fieldcopy" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/service/catalog" ) @@ -22,7 +21,7 @@ func (*ResourceCatalog) PrepareState(input *resources.Catalog) *catalog.CreateCa } // catalogRemapCopy maps CatalogInfo (remote GET response) to CreateCatalog (local state). -var catalogRemapCopy fieldcopy.Copy[catalog.CatalogInfo, catalog.CreateCatalog] +var catalogRemapCopy = newCopy[catalog.CatalogInfo, catalog.CreateCatalog]() func (*ResourceCatalog) RemapState(info *catalog.CatalogInfo) *catalog.CreateCatalog { return catalogRemapCopy.Do(info) @@ -41,7 +40,7 @@ func (r *ResourceCatalog) DoCreate(ctx context.Context, config *catalog.CreateCa } // catalogUpdateCopy maps CreateCatalog (local state) to UpdateCatalog (API request). -var catalogUpdateCopy fieldcopy.Copy[catalog.CreateCatalog, catalog.UpdateCatalog] +var catalogUpdateCopy = newCopy[catalog.CreateCatalog, catalog.UpdateCatalog]() // DoUpdate updates the catalog in place and returns remote state. func (r *ResourceCatalog) DoUpdate(ctx context.Context, id string, config *catalog.CreateCatalog, _ Changes) (*catalog.CatalogInfo, error) { @@ -87,7 +86,3 @@ func (r *ResourceCatalog) DoDelete(ctx context.Context, id string) error { }) } -func init() { - registerCopy(&catalogRemapCopy) - registerCopy(&catalogUpdateCopy) -} diff --git a/bundle/direct/dresources/cluster.go b/bundle/direct/dresources/cluster.go index 529bb67a37..77608ea167 100644 --- a/bundle/direct/dresources/cluster.go +++ b/bundle/direct/dresources/cluster.go @@ -8,7 +8,6 @@ import ( "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/bundle/deployplan" - "github.com/databricks/cli/libs/structs/fieldcopy" "github.com/databricks/cli/libs/structs/structpath" "github.com/databricks/cli/libs/utils" "github.com/databricks/databricks-sdk-go" @@ -32,7 +31,7 @@ func (r *ResourceCluster) PrepareState(input *resources.Cluster) *compute.Cluste } // clusterRemapCopy maps ClusterDetails (remote GET response) to ClusterSpec (local state). -var clusterRemapCopy fieldcopy.Copy[compute.ClusterDetails, compute.ClusterSpec] +var clusterRemapCopy = newCopy[compute.ClusterDetails, compute.ClusterSpec]() func (r *ResourceCluster) RemapState(input *compute.ClusterDetails) *compute.ClusterSpec { spec := clusterRemapCopy.Do(input) @@ -47,7 +46,7 @@ func (r *ResourceCluster) DoRead(ctx context.Context, id string) (*compute.Clust } // clusterCreateCopy maps ClusterSpec (local state) to CreateCluster (API request). -var clusterCreateCopy fieldcopy.Copy[compute.ClusterSpec, compute.CreateCluster] +var clusterCreateCopy = newCopy[compute.ClusterSpec, compute.CreateCluster]() func (r *ResourceCluster) DoCreate(ctx context.Context, config *compute.ClusterSpec) (string, *compute.ClusterDetails, error) { create := clusterCreateCopy.Do(config) @@ -60,7 +59,7 @@ func (r *ResourceCluster) DoCreate(ctx context.Context, config *compute.ClusterS } // clusterEditCopy maps ClusterSpec (local state) to EditCluster (API request). -var clusterEditCopy fieldcopy.Copy[compute.ClusterSpec, compute.EditCluster] +var clusterEditCopy = newCopy[compute.ClusterSpec, compute.EditCluster]() func (r *ResourceCluster) DoUpdate(ctx context.Context, id string, config *compute.ClusterSpec, _ Changes) (*compute.ClusterDetails, error) { edit := clusterEditCopy.Do(config) @@ -139,8 +138,3 @@ func forceNumWorkers(config *compute.ClusterSpec, fsf *[]string) { } } -func init() { - registerCopy(&clusterRemapCopy) - registerCopy(&clusterCreateCopy) - registerCopy(&clusterEditCopy) -} diff --git a/bundle/direct/dresources/external_location.go b/bundle/direct/dresources/external_location.go index d3259ba8b1..186ebb8555 100644 --- a/bundle/direct/dresources/external_location.go +++ b/bundle/direct/dresources/external_location.go @@ -4,7 +4,6 @@ import ( "context" "github.com/databricks/cli/bundle/config/resources" - "github.com/databricks/cli/libs/structs/fieldcopy" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/service/catalog" ) @@ -22,7 +21,7 @@ func (*ResourceExternalLocation) PrepareState(input *resources.ExternalLocation) } // externalLocationRemapCopy maps ExternalLocationInfo (remote GET response) to CreateExternalLocation (local state). -var externalLocationRemapCopy fieldcopy.Copy[catalog.ExternalLocationInfo, catalog.CreateExternalLocation] +var externalLocationRemapCopy = newCopy[catalog.ExternalLocationInfo, catalog.CreateExternalLocation]() func (*ResourceExternalLocation) RemapState(info *catalog.ExternalLocationInfo) *catalog.CreateExternalLocation { return externalLocationRemapCopy.Do(info) @@ -41,7 +40,7 @@ func (r *ResourceExternalLocation) DoCreate(ctx context.Context, config *catalog } // externalLocationUpdateCopy maps CreateExternalLocation (local state) to UpdateExternalLocation (API request). -var externalLocationUpdateCopy fieldcopy.Copy[catalog.CreateExternalLocation, catalog.UpdateExternalLocation] +var externalLocationUpdateCopy = newCopy[catalog.CreateExternalLocation, catalog.UpdateExternalLocation]() // DoUpdate updates the external location in place and returns remote state. func (r *ResourceExternalLocation) DoUpdate(ctx context.Context, id string, config *catalog.CreateExternalLocation, _ Changes) (*catalog.ExternalLocationInfo, error) { @@ -82,7 +81,3 @@ func (r *ResourceExternalLocation) DoDelete(ctx context.Context, id string) erro }) } -func init() { - registerCopy(&externalLocationRemapCopy) - registerCopy(&externalLocationUpdateCopy) -} diff --git a/bundle/direct/dresources/fieldcopy.go b/bundle/direct/dresources/fieldcopy.go index f0f86c1319..07684845db 100644 --- a/bundle/direct/dresources/fieldcopy.go +++ b/bundle/direct/dresources/fieldcopy.go @@ -8,7 +8,9 @@ type fieldCopyReporter interface { var allFieldCopies []fieldCopyReporter -func registerCopy[Src, Dst any](c *fieldcopy.Copy[Src, Dst]) { +func newCopy[Src, Dst any]() *fieldcopy.Copy[Src, Dst] { + c := &fieldcopy.Copy[Src, Dst]{} c.Init() allFieldCopies = append(allFieldCopies, c) + return c } diff --git a/bundle/direct/dresources/job.go b/bundle/direct/dresources/job.go index b97c14c35e..3b7de3d5f6 100644 --- a/bundle/direct/dresources/job.go +++ b/bundle/direct/dresources/job.go @@ -6,7 +6,6 @@ import ( "strconv" "github.com/databricks/cli/bundle/config/resources" - "github.com/databricks/cli/libs/structs/fieldcopy" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/marshal" "github.com/databricks/databricks-sdk-go/service/jobs" @@ -101,7 +100,7 @@ func makeJobRemote(job *jobs.Job) *JobRemote { } // jobCreateCopy maps JobSettings (local state) to CreateJob (API request). -var jobCreateCopy fieldcopy.Copy[jobs.JobSettings, jobs.CreateJob] +var jobCreateCopy = newCopy[jobs.JobSettings, jobs.CreateJob]() func (r *ResourceJob) DoCreate(ctx context.Context, config *jobs.JobSettings) (string, *JobRemote, error) { response, err := r.client.Jobs.Create(ctx, *jobCreateCopy.Do(config)) @@ -138,6 +137,3 @@ func parseJobID(id string) (int64, error) { return result, nil } -func init() { - registerCopy(&jobCreateCopy) -} diff --git a/bundle/direct/dresources/model_serving_endpoint.go b/bundle/direct/dresources/model_serving_endpoint.go index a1bfa41925..e39694fb6d 100644 --- a/bundle/direct/dresources/model_serving_endpoint.go +++ b/bundle/direct/dresources/model_serving_endpoint.go @@ -6,7 +6,6 @@ import ( "time" "github.com/databricks/cli/bundle/config/resources" - "github.com/databricks/cli/libs/structs/fieldcopy" "github.com/databricks/cli/libs/structs/structpath" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/service/serving" @@ -35,7 +34,7 @@ func (*ResourceModelServingEndpoint) PrepareState(input *resources.ModelServingE } // autoCaptureConfigCopy maps AutoCaptureConfigOutput to AutoCaptureConfigInput. -var autoCaptureConfigCopy fieldcopy.Copy[serving.AutoCaptureConfigOutput, serving.AutoCaptureConfigInput] +var autoCaptureConfigCopy = newCopy[serving.AutoCaptureConfigOutput, serving.AutoCaptureConfigInput]() func autoCaptureConfigOutputToInput(output *serving.AutoCaptureConfigOutput) *serving.AutoCaptureConfigInput { if output == nil { @@ -45,7 +44,7 @@ func autoCaptureConfigOutputToInput(output *serving.AutoCaptureConfigOutput) *se } // servedEntityCopy maps ServedEntityOutput to ServedEntityInput. -var servedEntityCopy fieldcopy.Copy[serving.ServedEntityOutput, serving.ServedEntityInput] +var servedEntityCopy = newCopy[serving.ServedEntityOutput, serving.ServedEntityInput]() func servedEntitiesOutputToInput(output []serving.ServedEntityOutput) []serving.ServedEntityInput { entities := make([]serving.ServedEntityInput, len(output)) @@ -69,7 +68,7 @@ func configOutputToInput(output *serving.EndpointCoreConfigOutput) *serving.Endp } // servingRemapCopy maps ServingEndpointDetailed (remote GET response) to CreateServingEndpoint (local state). -var servingRemapCopy fieldcopy.Copy[serving.ServingEndpointDetailed, serving.CreateServingEndpoint] +var servingRemapCopy = newCopy[serving.ServingEndpointDetailed, serving.CreateServingEndpoint]() func (*ResourceModelServingEndpoint) RemapState(state *RefreshOutput) *serving.CreateServingEndpoint { details := state.EndpointDetails @@ -297,8 +296,3 @@ func (r *ResourceModelServingEndpoint) DoDelete(ctx context.Context, id string) return r.client.ServingEndpoints.DeleteByName(ctx, id) } -func init() { - registerCopy(&autoCaptureConfigCopy) - registerCopy(&servedEntityCopy) - registerCopy(&servingRemapCopy) -} diff --git a/bundle/direct/dresources/pipeline.go b/bundle/direct/dresources/pipeline.go index 355bc0572f..d750176da5 100644 --- a/bundle/direct/dresources/pipeline.go +++ b/bundle/direct/dresources/pipeline.go @@ -4,7 +4,6 @@ import ( "context" "github.com/databricks/cli/bundle/config/resources" - "github.com/databricks/cli/libs/structs/fieldcopy" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/marshal" "github.com/databricks/databricks-sdk-go/service/pipelines" @@ -66,10 +65,10 @@ func (r *ResourcePipeline) DoRead(ctx context.Context, id string) (*PipelineRemo } // pipelineSpecCopy maps PipelineSpec (from GET response) to CreatePipeline (local state). -var pipelineSpecCopy fieldcopy.Copy[pipelines.PipelineSpec, pipelines.CreatePipeline] +var pipelineSpecCopy = newCopy[pipelines.PipelineSpec, pipelines.CreatePipeline]() // pipelineRemoteCopy maps GetPipelineResponse to PipelineRemote extra fields. -var pipelineRemoteCopy fieldcopy.Copy[pipelines.GetPipelineResponse, PipelineRemote] +var pipelineRemoteCopy = newCopy[pipelines.GetPipelineResponse, PipelineRemote]() func makePipelineRemote(p *pipelines.GetPipelineResponse) *PipelineRemote { remote := pipelineRemoteCopy.Do(p) @@ -89,7 +88,7 @@ func (r *ResourcePipeline) DoCreate(ctx context.Context, config *pipelines.Creat } // pipelineEditCopy maps CreatePipeline (local state) to EditPipeline (API request). -var pipelineEditCopy fieldcopy.Copy[pipelines.CreatePipeline, pipelines.EditPipeline] +var pipelineEditCopy = newCopy[pipelines.CreatePipeline, pipelines.EditPipeline]() func (r *ResourcePipeline) DoUpdate(ctx context.Context, id string, config *pipelines.CreatePipeline, _ Changes) (*PipelineRemote, error) { request := pipelineEditCopy.Do(config) @@ -106,8 +105,3 @@ func (r *ResourcePipeline) DoDelete(ctx context.Context, id string) error { // b) repeatededly reads state until state is "running" (if spec.Contionous is set). // TODO: investigate if we need to mimic this behaviour or can rely on Create status code. -func init() { - registerCopy(&pipelineSpecCopy) - registerCopy(&pipelineRemoteCopy) - registerCopy(&pipelineEditCopy) -} diff --git a/bundle/direct/dresources/quality_monitor.go b/bundle/direct/dresources/quality_monitor.go index 14063d43ad..32dbbb8bbe 100644 --- a/bundle/direct/dresources/quality_monitor.go +++ b/bundle/direct/dresources/quality_monitor.go @@ -4,7 +4,6 @@ import ( "context" "github.com/databricks/cli/bundle/config/resources" - "github.com/databricks/cli/libs/structs/fieldcopy" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/marshal" "github.com/databricks/databricks-sdk-go/service/catalog" @@ -42,7 +41,7 @@ func (*ResourceQualityMonitor) PrepareState(input *resources.QualityMonitor) *Qu } // qualityMonitorRemapCopy maps MonitorInfo (remote GET response) to CreateMonitor (local state). -var qualityMonitorRemapCopy fieldcopy.Copy[catalog.MonitorInfo, catalog.CreateMonitor] +var qualityMonitorRemapCopy = newCopy[catalog.MonitorInfo, catalog.CreateMonitor]() func (*ResourceQualityMonitor) RemapState(info *catalog.MonitorInfo) *QualityMonitorState { return &QualityMonitorState{ @@ -70,7 +69,7 @@ func (r *ResourceQualityMonitor) DoCreate(ctx context.Context, config *QualityMo } // qualityMonitorUpdateCopy maps CreateMonitor (local state) to UpdateMonitor (API request). -var qualityMonitorUpdateCopy fieldcopy.Copy[catalog.CreateMonitor, catalog.UpdateMonitor] +var qualityMonitorUpdateCopy = newCopy[catalog.CreateMonitor, catalog.UpdateMonitor]() func (r *ResourceQualityMonitor) DoUpdate(ctx context.Context, id string, config *QualityMonitorState, _ Changes) (*catalog.MonitorInfo, error) { updateRequest := qualityMonitorUpdateCopy.Do(&config.CreateMonitor) @@ -93,7 +92,3 @@ func (r *ResourceQualityMonitor) DoDelete(ctx context.Context, id string) error return err } -func init() { - registerCopy(&qualityMonitorRemapCopy) - registerCopy(&qualityMonitorUpdateCopy) -} diff --git a/bundle/direct/dresources/registered_model.go b/bundle/direct/dresources/registered_model.go index 2e090d13e4..d37acc3387 100644 --- a/bundle/direct/dresources/registered_model.go +++ b/bundle/direct/dresources/registered_model.go @@ -4,7 +4,6 @@ import ( "context" "github.com/databricks/cli/bundle/config/resources" - "github.com/databricks/cli/libs/structs/fieldcopy" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/service/catalog" ) @@ -24,7 +23,7 @@ func (*ResourceRegisteredModel) PrepareState(input *resources.RegisteredModel) * } // registeredModelRemapCopy maps RegisteredModelInfo (remote GET response) to CreateRegisteredModelRequest (local state). -var registeredModelRemapCopy fieldcopy.Copy[catalog.RegisteredModelInfo, catalog.CreateRegisteredModelRequest] +var registeredModelRemapCopy = newCopy[catalog.RegisteredModelInfo, catalog.CreateRegisteredModelRequest]() func (*ResourceRegisteredModel) RemapState(model *catalog.RegisteredModelInfo) *catalog.CreateRegisteredModelRequest { return registeredModelRemapCopy.Do(model) @@ -49,7 +48,7 @@ func (r *ResourceRegisteredModel) DoCreate(ctx context.Context, config *catalog. } // registeredModelUpdateCopy maps CreateRegisteredModelRequest (local state) to UpdateRegisteredModelRequest (API request). -var registeredModelUpdateCopy fieldcopy.Copy[catalog.CreateRegisteredModelRequest, catalog.UpdateRegisteredModelRequest] +var registeredModelUpdateCopy = newCopy[catalog.CreateRegisteredModelRequest, catalog.UpdateRegisteredModelRequest]() func (r *ResourceRegisteredModel) DoUpdate(ctx context.Context, id string, config *catalog.CreateRegisteredModelRequest, _ Changes) (*catalog.RegisteredModelInfo, error) { updateRequest := registeredModelUpdateCopy.Do(config) @@ -69,7 +68,3 @@ func (r *ResourceRegisteredModel) DoDelete(ctx context.Context, id string) error }) } -func init() { - registerCopy(®isteredModelRemapCopy) - registerCopy(®isteredModelUpdateCopy) -} diff --git a/bundle/direct/dresources/sql_warehouse.go b/bundle/direct/dresources/sql_warehouse.go index 163f306231..b3a536ebe0 100644 --- a/bundle/direct/dresources/sql_warehouse.go +++ b/bundle/direct/dresources/sql_warehouse.go @@ -5,7 +5,6 @@ import ( "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/libs/log" - "github.com/databricks/cli/libs/structs/fieldcopy" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/service/sql" ) @@ -25,7 +24,7 @@ func (*ResourceSqlWarehouse) PrepareState(input *resources.SqlWarehouse) *sql.Cr } // sqlWarehouseRemapCopy maps GetWarehouseResponse (remote GET response) to CreateWarehouseRequest (local state). -var sqlWarehouseRemapCopy fieldcopy.Copy[sql.GetWarehouseResponse, sql.CreateWarehouseRequest] +var sqlWarehouseRemapCopy = newCopy[sql.GetWarehouseResponse, sql.CreateWarehouseRequest]() func (*ResourceSqlWarehouse) RemapState(warehouse *sql.GetWarehouseResponse) *sql.CreateWarehouseRequest { result := sqlWarehouseRemapCopy.Do(warehouse) @@ -49,7 +48,7 @@ func (r *ResourceSqlWarehouse) DoCreate(ctx context.Context, config *sql.CreateW } // sqlWarehouseEditCopy maps CreateWarehouseRequest (local state) to EditWarehouseRequest (API request). -var sqlWarehouseEditCopy fieldcopy.Copy[sql.CreateWarehouseRequest, sql.EditWarehouseRequest] +var sqlWarehouseEditCopy = newCopy[sql.CreateWarehouseRequest, sql.EditWarehouseRequest]() // DoUpdate updates the warehouse in place. func (r *ResourceSqlWarehouse) DoUpdate(ctx context.Context, id string, config *sql.CreateWarehouseRequest, _ Changes) (*sql.GetWarehouseResponse, error) { @@ -74,7 +73,3 @@ func (r *ResourceSqlWarehouse) DoDelete(ctx context.Context, oldID string) error return r.client.Warehouses.DeleteById(ctx, oldID) } -func init() { - registerCopy(&sqlWarehouseRemapCopy) - registerCopy(&sqlWarehouseEditCopy) -} From 0e50e9594a89558c1ef0a268f3f1f8c4c4b654f1 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 23 Mar 2026 20:55:00 +0100 Subject: [PATCH 15/16] Replace closure in fieldcopy.Do with struct fields and a direct method Store ops, autoFSF, fsfSrcIdx, fsfDstIdx, validFSFNames, and ready as struct fields populated by Init, instead of capturing them in a closure. Do is now a regular method reading from the struct. Co-authored-by: Isaac --- libs/structs/fieldcopy/fieldcopy.go | 94 ++++++++++++++++------------- 1 file changed, 51 insertions(+), 43 deletions(-) diff --git a/libs/structs/fieldcopy/fieldcopy.go b/libs/structs/fieldcopy/fieldcopy.go index cb6941e746..fa02d59221 100644 --- a/libs/structs/fieldcopy/fieldcopy.go +++ b/libs/structs/fieldcopy/fieldcopy.go @@ -27,21 +27,58 @@ type Copy[Src, Dst any] struct { // with different names in source and destination. Rename map[string]string - copyFn func(src *Src) *Dst + // ops is the list of field copy operations (src index → dst index). + ops []fieldOp + + // autoFSF is true when ForceSendFields is auto-handled. + autoFSF bool + + // fsfSrcIdx is the reflect field index of ForceSendFields on Src. + fsfSrcIdx []int + + // fsfDstIdx is the reflect field index of ForceSendFields on Dst. + fsfDstIdx []int + + // validFSFNames is the set of dst field names eligible for ForceSendFields. + validFSFNames map[string]bool + + // ready is set to true after Init completes. + ready bool } // Init eagerly computes field mappings via reflection. Must be called before Do. func (c *Copy[Src, Dst]) Init() { - c.copyFn = c.build() + c.build() } // Do copies fields from src to a new Dst value using precomputed field mappings. // Panics if [Copy.Init] was not called. func (c *Copy[Src, Dst]) Do(src *Src) *Dst { - if c.copyFn == nil { + if !c.ready { panic(fmt.Sprintf("fieldcopy: Do called on uninitialized Copy[%v, %v]; call Init first", reflect.TypeFor[Src](), reflect.TypeFor[Dst]())) } - return c.copyFn(src) + var dst Dst + sv := reflect.ValueOf(src).Elem() + dv := reflect.ValueOf(&dst).Elem() + for _, op := range c.ops { + dv.FieldByIndex(op.dstIndex).Set(sv.FieldByIndex(op.srcIndex)) + } + if c.autoFSF { + srcFSF := sv.FieldByIndex(c.fsfSrcIdx) + if !srcFSF.IsNil() { + srcFields := srcFSF.Interface().([]string) + var filtered []string + for _, name := range srcFields { + if c.validFSFNames[name] { + filtered = append(filtered, name) + } + } + if filtered != nil { + dv.FieldByIndex(c.fsfDstIdx).Set(reflect.ValueOf(filtered)) + } + } + } + return &dst } type fieldOp struct { @@ -49,35 +86,29 @@ type fieldOp struct { srcIndex []int } -func (c *Copy[Src, Dst]) build() func(*Src) *Dst { +func (c *Copy[Src, Dst]) build() { srcType := reflect.TypeFor[Src]() dstType := reflect.TypeFor[Dst]() // Detect auto-handling of ForceSendFields: both types must have it as []string. - var autoFSF bool - var fsfSrcIdx, fsfDstIdx []int - var validFSFNames map[string]bool - stringSliceType := reflect.TypeFor[[]string]() dstFSF, dstOK := dstType.FieldByName(forceSendFieldsName) srcFSF, srcOK := srcType.FieldByName(forceSendFieldsName) // Only auto-handle when ForceSendFields is a direct (non-promoted) field on both types. // Promoted fields from embedded structs have len(Index) > 1. if dstOK && srcOK && len(dstFSF.Index) == 1 && len(srcFSF.Index) == 1 && dstFSF.Type == stringSliceType && srcFSF.Type == stringSliceType { - autoFSF = true - fsfSrcIdx = srcFSF.Index - fsfDstIdx = dstFSF.Index - validFSFNames = make(map[string]bool) + c.autoFSF = true + c.fsfSrcIdx = srcFSF.Index + c.fsfDstIdx = dstFSF.Index + c.validFSFNames = make(map[string]bool) } - var ops []fieldOp - for i := range dstType.NumField() { df := dstType.Field(i) if !df.IsExported() { continue } - if autoFSF && df.Name == forceSendFieldsName { + if c.autoFSF && df.Name == forceSendFieldsName { continue // handled separately } @@ -93,36 +124,13 @@ func (c *Copy[Src, Dst]) build() func(*Src) *Dst { continue } - ops = append(ops, fieldOp{dstIndex: df.Index, srcIndex: sf.Index}) - if autoFSF { - validFSFNames[df.Name] = true + c.ops = append(c.ops, fieldOp{dstIndex: df.Index, srcIndex: sf.Index}) + if c.autoFSF { + c.validFSFNames[df.Name] = true } } - return func(src *Src) *Dst { - var dst Dst - sv := reflect.ValueOf(src).Elem() - dv := reflect.ValueOf(&dst).Elem() - for _, op := range ops { - dv.FieldByIndex(op.dstIndex).Set(sv.FieldByIndex(op.srcIndex)) - } - if autoFSF { - srcFSF := sv.FieldByIndex(fsfSrcIdx) - if !srcFSF.IsNil() { - srcFields := srcFSF.Interface().([]string) - var filtered []string - for _, name := range srcFields { - if validFSFNames[name] { - filtered = append(filtered, name) - } - } - if filtered != nil { - dv.FieldByIndex(fsfDstIdx).Set(reflect.ValueOf(filtered)) - } - } - } - return &dst - } + c.ready = true } // Report returns a human-readable summary of unmatched fields for golden file testing. From 668efe8e699004c2dd3d6e2e758a29694402cf99 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 23 Mar 2026 20:58:12 +0100 Subject: [PATCH 16/16] Remove trailing blank lines from dresources files Co-authored-by: Isaac --- bundle/direct/dresources/catalog.go | 1 - bundle/direct/dresources/cluster.go | 1 - bundle/direct/dresources/external_location.go | 1 - bundle/direct/dresources/job.go | 1 - bundle/direct/dresources/model_serving_endpoint.go | 1 - bundle/direct/dresources/pipeline.go | 1 - bundle/direct/dresources/quality_monitor.go | 1 - bundle/direct/dresources/registered_model.go | 1 - bundle/direct/dresources/sql_warehouse.go | 1 - 9 files changed, 9 deletions(-) diff --git a/bundle/direct/dresources/catalog.go b/bundle/direct/dresources/catalog.go index e350394440..649a56230d 100644 --- a/bundle/direct/dresources/catalog.go +++ b/bundle/direct/dresources/catalog.go @@ -85,4 +85,3 @@ func (r *ResourceCatalog) DoDelete(ctx context.Context, id string) error { ForceSendFields: nil, }) } - diff --git a/bundle/direct/dresources/cluster.go b/bundle/direct/dresources/cluster.go index 77608ea167..f9b2a80015 100644 --- a/bundle/direct/dresources/cluster.go +++ b/bundle/direct/dresources/cluster.go @@ -137,4 +137,3 @@ func forceNumWorkers(config *compute.ClusterSpec, fsf *[]string) { *fsf = append(*fsf, "NumWorkers") } } - diff --git a/bundle/direct/dresources/external_location.go b/bundle/direct/dresources/external_location.go index 186ebb8555..2e00e397fa 100644 --- a/bundle/direct/dresources/external_location.go +++ b/bundle/direct/dresources/external_location.go @@ -80,4 +80,3 @@ func (r *ResourceExternalLocation) DoDelete(ctx context.Context, id string) erro ForceSendFields: nil, }) } - diff --git a/bundle/direct/dresources/job.go b/bundle/direct/dresources/job.go index 3b7de3d5f6..f4d98e5664 100644 --- a/bundle/direct/dresources/job.go +++ b/bundle/direct/dresources/job.go @@ -136,4 +136,3 @@ func parseJobID(id string) (int64, error) { } return result, nil } - diff --git a/bundle/direct/dresources/model_serving_endpoint.go b/bundle/direct/dresources/model_serving_endpoint.go index e39694fb6d..cc657e392e 100644 --- a/bundle/direct/dresources/model_serving_endpoint.go +++ b/bundle/direct/dresources/model_serving_endpoint.go @@ -295,4 +295,3 @@ func (r *ResourceModelServingEndpoint) DoUpdate(ctx context.Context, id string, func (r *ResourceModelServingEndpoint) DoDelete(ctx context.Context, id string) error { return r.client.ServingEndpoints.DeleteByName(ctx, id) } - diff --git a/bundle/direct/dresources/pipeline.go b/bundle/direct/dresources/pipeline.go index d750176da5..d8e8c6c879 100644 --- a/bundle/direct/dresources/pipeline.go +++ b/bundle/direct/dresources/pipeline.go @@ -104,4 +104,3 @@ func (r *ResourcePipeline) DoDelete(ctx context.Context, id string) error { // a) reads back state at least once and fails create if state is "failed" // b) repeatededly reads state until state is "running" (if spec.Contionous is set). // TODO: investigate if we need to mimic this behaviour or can rely on Create status code. - diff --git a/bundle/direct/dresources/quality_monitor.go b/bundle/direct/dresources/quality_monitor.go index 32dbbb8bbe..4de56ec50d 100644 --- a/bundle/direct/dresources/quality_monitor.go +++ b/bundle/direct/dresources/quality_monitor.go @@ -91,4 +91,3 @@ func (r *ResourceQualityMonitor) DoDelete(ctx context.Context, id string) error }) return err } - diff --git a/bundle/direct/dresources/registered_model.go b/bundle/direct/dresources/registered_model.go index d37acc3387..2a9ef85e7e 100644 --- a/bundle/direct/dresources/registered_model.go +++ b/bundle/direct/dresources/registered_model.go @@ -67,4 +67,3 @@ func (r *ResourceRegisteredModel) DoDelete(ctx context.Context, id string) error FullName: id, }) } - diff --git a/bundle/direct/dresources/sql_warehouse.go b/bundle/direct/dresources/sql_warehouse.go index b3a536ebe0..8929164274 100644 --- a/bundle/direct/dresources/sql_warehouse.go +++ b/bundle/direct/dresources/sql_warehouse.go @@ -72,4 +72,3 @@ func (r *ResourceSqlWarehouse) DoUpdate(ctx context.Context, id string, config * func (r *ResourceSqlWarehouse) DoDelete(ctx context.Context, oldID string) error { return r.client.Warehouses.DeleteById(ctx, oldID) } -