mirror of https://github.com/databricks/cli.git
Assert customer marshalling is implemented for resources (#1425)
## Changes This PR ensures every resource implements a custom marshaller / unmarshaller. This is required because we directly embed Go SDK structs. which implement custom marshalling overrides. Since the struct is embedded, the [customer marshalling overrides](https://pkg.go.dev/encoding/json#example-package-CustomMarshalJSON) are promoted to the top level. If the embedded struct itself is nil, then JSON marshal / unmarshal will panic because it tries to call `MarshalJSON` / `UnmarshalJSON` on a nil object. Fixing this issue at the Go SDK level does not seem possible. Discussed with @hectorcast-db.
This commit is contained in:
parent
95bbe2ece1
commit
63617253bd
|
@ -1,6 +1,8 @@
|
||||||
package config
|
package config
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"encoding/json"
|
||||||
|
"reflect"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
"github.com/databricks/cli/bundle/config/paths"
|
"github.com/databricks/cli/bundle/config/paths"
|
||||||
|
@ -125,3 +127,57 @@ func TestVerifySafeMergeForRegisteredModels(t *testing.T) {
|
||||||
err := r.VerifySafeMerge(&other)
|
err := r.VerifySafeMerge(&other)
|
||||||
assert.ErrorContains(t, err, "multiple resources named bar (registered_model at bar.yml, registered_model at bar2.yml)")
|
assert.ErrorContains(t, err, "multiple resources named bar (registered_model at bar.yml, registered_model at bar2.yml)")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// This test ensures that all resources have a custom marshaller and unmarshaller.
|
||||||
|
// This is required because DABs resources map to Databricks APIs, and they do so
|
||||||
|
// by embedding the corresponding Go SDK structs.
|
||||||
|
//
|
||||||
|
// Go SDK structs often implement custom marshalling and unmarshalling methods (based on the API specifics).
|
||||||
|
// If the Go SDK struct implements custom marshalling and unmarshalling and we do not
|
||||||
|
// for the resources at the top level, marshalling and unmarshalling operations will panic.
|
||||||
|
// Thus we will be overly cautious and ensure that all resources need a custom marshaller and unmarshaller.
|
||||||
|
//
|
||||||
|
// Why do we not assert this using an interface to assert MarshalJSON and UnmarshalJSON
|
||||||
|
// are implemented at the top level?
|
||||||
|
// If a method is implemented for an embedded struct, the top level struct will
|
||||||
|
// also have that method and satisfy the interface. This is why we cannot assert
|
||||||
|
// that the methods are implemented at the top level using an interface.
|
||||||
|
//
|
||||||
|
// Why don't we use reflection to assert that the methods are implemented at the
|
||||||
|
// top level?
|
||||||
|
// Same problem as above, the golang reflection package does not seem to provide
|
||||||
|
// a way to directly assert that MarshalJSON and UnmarshalJSON are implemented
|
||||||
|
// at the top level.
|
||||||
|
func TestCustomMarshallerIsImplemented(t *testing.T) {
|
||||||
|
r := Resources{}
|
||||||
|
rt := reflect.TypeOf(r)
|
||||||
|
|
||||||
|
for i := 0; i < rt.NumField(); i++ {
|
||||||
|
field := rt.Field(i)
|
||||||
|
|
||||||
|
// Fields in Resources are expected be of the form map[string]*resourceStruct
|
||||||
|
assert.Equal(t, field.Type.Kind(), reflect.Map, "Resource %s is not a map", field.Name)
|
||||||
|
kt := field.Type.Key()
|
||||||
|
assert.Equal(t, kt.Kind(), reflect.String, "Resource %s is not a map with string keys", field.Name)
|
||||||
|
vt := field.Type.Elem()
|
||||||
|
assert.Equal(t, vt.Kind(), reflect.Ptr, "Resource %s is not a map with pointer values", field.Name)
|
||||||
|
|
||||||
|
// Marshalling a resourceStruct will panic if resourceStruct does not have a custom marshaller
|
||||||
|
// This is because resourceStruct embeds a Go SDK struct that implements
|
||||||
|
// a custom marshaller.
|
||||||
|
// Eg: resource.Job implements MarshalJSON
|
||||||
|
v := reflect.Zero(vt.Elem()).Interface()
|
||||||
|
assert.NotPanics(t, func() {
|
||||||
|
json.Marshal(v)
|
||||||
|
}, "Resource %s does not have a custom marshaller", field.Name)
|
||||||
|
|
||||||
|
// Unmarshalling a *resourceStruct will panic if the resource does not have a custom unmarshaller
|
||||||
|
// This is because resourceStruct embeds a Go SDK struct that implements
|
||||||
|
// a custom unmarshaller.
|
||||||
|
// Eg: *resource.Job implements UnmarshalJSON
|
||||||
|
v = reflect.New(vt.Elem()).Interface()
|
||||||
|
assert.NotPanics(t, func() {
|
||||||
|
json.Unmarshal([]byte("{}"), v)
|
||||||
|
}, "Resource %s does not have a custom unmarshaller", field.Name)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
Loading…
Reference in New Issue