structaccess: two ForceSendFields fixes in Set

libs/structs/structaccess/set.go

@@ -140,13 +140,24 @@ func setStructField(parentVal reflect.Value, fieldName string, valueVal reflect.
 		return fmt.Errorf("field %q cannot be set", sf.Name)
 	}
 
-	// Handle ForceSendFields: remove if setting nil, add if setting empty value
-	err := updateForceSendFields(parentVal, sf.Name, embeddedIndex, valueVal, sf)
+	// Assign first: a value that cannot be converted must leave the struct exactly as it was.
+	converted, err := convertValue(valueVal, fv.Type())
 	if err != nil {
 		return err
 	}
+	if err := assignValue(fv, converted); err != nil {
+		return err
+	}
 
-	return assignValue(fv, valueVal)
+	// ForceSendFields is decided from the converted value, not the caller's: setting an
+	// omitempty int64 from the string "0" stores 0, which is empty and must be forced, while
+	// the string "0" is not empty and would have left the field to be omitted.
+	if !valueVal.IsValid() {
+		// Setting nil: the field is being made absent, which convertValue renders as the zero
+		// value. Pass the invalid value through so it is removed from ForceSendFields.
+		return updateForceSendFields(parentVal, sf.Name, embeddedIndex, valueVal, sf)
+	}
+	return updateForceSendFields(parentVal, sf.Name, embeddedIndex, converted, sf)
 }
 
 // setMapValue sets a value in a map

libs/structs/structaccess/set_test.go

@@ -1,6 +1,7 @@
 package structaccess_test
 
 import (
+	"encoding/json"
 	"testing"
 
 	"github.com/databricks/cli/libs/structs/structaccess"
@@ -791,3 +792,27 @@ func TestSet_MixedForceSendFields(t *testing.T) {
 		assert.Equal(t, []string{"SecondFieldOmit"}, obj.Second.ForceSendFields) // no duplicates
 	})
 }
+
+// A value that cannot be converted must leave the struct untouched, ForceSendFields included.
+func TestSet_FailedConversionLeavesForceSendFields(t *testing.T) {
+	job := &jobs.JobSettings{Name: "n"} //exhaustruct:ignore
+
+	require.Error(t, structaccess.SetByString(job, "max_concurrent_runs", ""))
+	assert.Empty(t, job.ForceSendFields)
+	assert.Equal(t, "n", job.Name)
+}
+
+// ForceSendFields is decided from the value actually stored, not the one the caller passed:
+// setting an omitempty numeric field from the string "0" stores zero, which has to be forced
+// or the field marshals as absent.
+func TestSet_StringZeroIntoOmitemptyNumberIsForced(t *testing.T) {
+	job := &jobs.JobSettings{Name: "n"} //exhaustruct:ignore
+
+	require.NoError(t, structaccess.SetByString(job, "max_concurrent_runs", "0"))
+	assert.Equal(t, 0, job.MaxConcurrentRuns)
+	assert.Contains(t, job.ForceSendFields, "MaxConcurrentRuns")
+
+	blob, err := json.Marshal(job)
+	require.NoError(t, err)
+	assert.Contains(t, string(blob), `"max_concurrent_runs":0`)
+}