From fece926e2bb890e2505f334a6f22d4088ff55d76 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Thu, 18 Sep 2014 06:08:10 -0400 Subject: [PATCH] Remove unnecessary EmbeddedObjects Clarify the tests in embedded_test.go to indicate that unmarshalling of an internal object (runtime.EmbeddedObject) will not work. Instead, consumers should decode raw external objects. --- pkg/api/conversion.go | 30 -------------- pkg/api/v1beta1/conversion.go | 31 --------------- pkg/api/v1beta2/conversion.go | 31 --------------- pkg/runtime/embedded.go | 41 -------------------- pkg/runtime/embedded_test.go | 73 +++++++++++++++-------------------- pkg/runtime/scheme.go | 2 +- 6 files changed, 32 insertions(+), 176 deletions(-) diff --git a/pkg/api/conversion.go b/pkg/api/conversion.go index 36ba27675bc..5c8cd1c8097 100644 --- a/pkg/api/conversion.go +++ b/pkg/api/conversion.go @@ -23,33 +23,3 @@ import ( // Codec is the identity codec for this package - it can only convert itself // to itself. var Codec = runtime.CodecFor(Scheme, "") - -// EmbeddedObject implements a Codec specific version of an -// embedded object. -type EmbeddedObject struct { - runtime.Object -} - -// UnmarshalJSON implements the json.Unmarshaler interface. -func (a *EmbeddedObject) UnmarshalJSON(b []byte) error { - obj, err := runtime.CodecUnmarshalJSON(Codec, b) - a.Object = obj - return err -} - -// MarshalJSON implements the json.Marshaler interface. -func (a EmbeddedObject) MarshalJSON() ([]byte, error) { - return runtime.CodecMarshalJSON(Codec, a.Object) -} - -// SetYAML implements the yaml.Setter interface. -func (a *EmbeddedObject) SetYAML(tag string, value interface{}) bool { - obj, ok := runtime.CodecSetYAML(Codec, tag, value) - a.Object = obj - return ok -} - -// GetYAML implements the yaml.Getter interface. -func (a EmbeddedObject) GetYAML() (tag string, value interface{}) { - return runtime.CodecGetYAML(Codec, a.Object) -} diff --git a/pkg/api/v1beta1/conversion.go b/pkg/api/v1beta1/conversion.go index 14bb3956b60..4105ac8d353 100644 --- a/pkg/api/v1beta1/conversion.go +++ b/pkg/api/v1beta1/conversion.go @@ -19,7 +19,6 @@ package v1beta1 import ( newer "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/conversion" - "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" ) func init() { @@ -80,33 +79,3 @@ func init() { ) } - -// EmbeddedObject implements a Codec specific version of an -// embedded object. -type EmbeddedObject struct { - runtime.Object -} - -// UnmarshalJSON implements the json.Unmarshaler interface. -func (a *EmbeddedObject) UnmarshalJSON(b []byte) error { - obj, err := runtime.CodecUnmarshalJSON(Codec, b) - a.Object = obj - return err -} - -// MarshalJSON implements the json.Marshaler interface. -func (a EmbeddedObject) MarshalJSON() ([]byte, error) { - return runtime.CodecMarshalJSON(Codec, a.Object) -} - -// SetYAML implements the yaml.Setter interface. -func (a *EmbeddedObject) SetYAML(tag string, value interface{}) bool { - obj, ok := runtime.CodecSetYAML(Codec, tag, value) - a.Object = obj - return ok -} - -// GetYAML implements the yaml.Getter interface. -func (a EmbeddedObject) GetYAML() (tag string, value interface{}) { - return runtime.CodecGetYAML(Codec, a.Object) -} diff --git a/pkg/api/v1beta2/conversion.go b/pkg/api/v1beta2/conversion.go index fa7c913a0e1..9d253f4bd7c 100644 --- a/pkg/api/v1beta2/conversion.go +++ b/pkg/api/v1beta2/conversion.go @@ -19,7 +19,6 @@ package v1beta2 import ( newer "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/conversion" - "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" ) func init() { @@ -79,33 +78,3 @@ func init() { }, ) } - -// EmbeddedObject implements a Codec specific version of an -// embedded object. -type EmbeddedObject struct { - runtime.Object -} - -// UnmarshalJSON implements the json.Unmarshaler interface. -func (a *EmbeddedObject) UnmarshalJSON(b []byte) error { - obj, err := runtime.CodecUnmarshalJSON(Codec, b) - a.Object = obj - return err -} - -// MarshalJSON implements the json.Marshaler interface. -func (a EmbeddedObject) MarshalJSON() ([]byte, error) { - return runtime.CodecMarshalJSON(Codec, a.Object) -} - -// SetYAML implements the yaml.Setter interface. -func (a *EmbeddedObject) SetYAML(tag string, value interface{}) bool { - obj, ok := runtime.CodecSetYAML(Codec, tag, value) - a.Object = obj - return ok -} - -// GetYAML implements the yaml.Getter interface. -func (a EmbeddedObject) GetYAML() (tag string, value interface{}) { - return runtime.CodecGetYAML(Codec, a.Object) -} diff --git a/pkg/runtime/embedded.go b/pkg/runtime/embedded.go index 9bfb2f67555..11ac2894651 100644 --- a/pkg/runtime/embedded.go +++ b/pkg/runtime/embedded.go @@ -20,47 +20,6 @@ import ( "gopkg.in/v1/yaml" ) -// EmbeddedObject must have an appropriate encoder and decoder functions, such that on the -// wire, it's stored as a []byte, but in memory, the contained object is accessable as an -// Object via the Get() function. Only valid API objects may be stored via EmbeddedObject. -// The purpose of this is to allow an API object of type known only at runtime to be -// embedded within other API objects. -// -// Define a Codec variable in your package and import the runtime package and -// then use the commented section below - -/* -// EmbeddedObject implements a Codec specific version of an -// embedded object. -type EmbeddedObject struct { - runtime.Object -} - -// UnmarshalJSON implements the json.Unmarshaler interface. -func (a *EmbeddedObject) UnmarshalJSON(b []byte) error { - obj, err := runtime.CodecUnmarshalJSON(Codec, b) - a.Object = obj - return err -} - -// MarshalJSON implements the json.Marshaler interface. -func (a EmbeddedObject) MarshalJSON() ([]byte, error) { - return runtime.CodecMarshalJSON(Codec, a.Object) -} - -// SetYAML implements the yaml.Setter interface. -func (a *EmbeddedObject) SetYAML(tag string, value interface{}) bool { - obj, ok := runtime.CodecSetYAML(Codec, tag, value) - a.Object = obj - return ok -} - -// GetYAML implements the yaml.Getter interface. -func (a EmbeddedObject) GetYAML() (tag string, value interface{}) { - return runtime.CodecGetYAML(Codec, a.Object) -} -*/ - // Encode()/Decode() are the canonical way of converting an API object to/from // wire format. This file provides utility functions which permit doing so // recursively, such that API objects of types known only at run time can be diff --git a/pkg/runtime/embedded_test.go b/pkg/runtime/embedded_test.go index bc461eeab7a..4f34a90f5f4 100644 --- a/pkg/runtime/embedded_test.go +++ b/pkg/runtime/embedded_test.go @@ -27,52 +27,29 @@ import ( var scheme = runtime.NewScheme() var Codec = runtime.CodecFor(scheme, "v1test") -// EmbeddedObject implements a Codec specific version of an -// embedded object. -type EmbeddedObject struct { - runtime.Object -} - -// UnmarshalJSON implements the json.Unmarshaler interface. -func (a *EmbeddedObject) UnmarshalJSON(b []byte) error { - obj, err := runtime.CodecUnmarshalJSON(Codec, b) - a.Object = obj - return err -} - -// MarshalJSON implements the json.Marshaler interface. -func (a EmbeddedObject) MarshalJSON() ([]byte, error) { - return runtime.CodecMarshalJSON(Codec, a.Object) -} - -// SetYAML implements the yaml.Setter interface. -func (a *EmbeddedObject) SetYAML(tag string, value interface{}) bool { - obj, ok := runtime.CodecSetYAML(Codec, tag, value) - a.Object = obj - return ok -} - -// GetYAML implements the yaml.Getter interface. -func (a EmbeddedObject) GetYAML() (tag string, value interface{}) { - return runtime.CodecGetYAML(Codec, a.Object) -} - type EmbeddedTest struct { runtime.JSONBase `yaml:",inline" json:",inline"` - Object EmbeddedObject `yaml:"object,omitempty" json:"object,omitempty"` - EmptyObject EmbeddedObject `yaml:"emptyObject,omitempty" json:"emptyObject,omitempty"` + Object runtime.EmbeddedObject `yaml:"object,omitempty" json:"object,omitempty"` + EmptyObject runtime.EmbeddedObject `yaml:"emptyObject,omitempty" json:"emptyObject,omitempty"` } -func (*EmbeddedTest) IsAnAPIObject() {} +type EmbeddedTestExternal struct { + runtime.JSONBase `yaml:",inline" json:",inline"` + Object runtime.RawExtension `yaml:"object,omitempty" json:"object,omitempty"` + EmptyObject runtime.RawExtension `yaml:"emptyObject,omitempty" json:"emptyObject,omitempty"` +} + +func (*EmbeddedTest) IsAnAPIObject() {} +func (*EmbeddedTestExternal) IsAnAPIObject() {} func TestEmbeddedObject(t *testing.T) { s := scheme s.AddKnownTypes("", &EmbeddedTest{}) - s.AddKnownTypes("v1test", &EmbeddedTest{}) + s.AddKnownTypeWithName("v1test", "EmbeddedTest", &EmbeddedTestExternal{}) outer := &EmbeddedTest{ JSONBase: runtime.JSONBase{ID: "outer"}, - Object: EmbeddedObject{ + Object: runtime.EmbeddedObject{ &EmbeddedTest{ JSONBase: runtime.JSONBase{ID: "inner"}, }, @@ -95,18 +72,30 @@ func TestEmbeddedObject(t *testing.T) { t.Errorf("Expected: %#v but got %#v", e, a) } + // test JSON decoding of the external object, which should preserve + // raw bytes + var externalViaJSON EmbeddedTestExternal + err = json.Unmarshal(wire, &externalViaJSON) + if err != nil { + t.Fatalf("Unexpected decode error %v", err) + } + if externalViaJSON.Kind == "" || externalViaJSON.APIVersion == "" || externalViaJSON.ID != "outer" { + t.Errorf("Expected objects to have type info set, got %#v", externalViaJSON) + } + if !reflect.DeepEqual(externalViaJSON.EmptyObject.RawJSON, []byte("null")) || len(externalViaJSON.Object.RawJSON) == 0 { + t.Errorf("Expected deserialization of nested objects into bytes, got %#v", externalViaJSON) + } + // test JSON decoding, too, since Decode uses yaml unmarshalling. + // Generic Unmarshalling of JSON cannot load the nested objects because there is + // no default schema set. Consumers wishing to get direct JSON decoding must use + // the external representation var decodedViaJSON EmbeddedTest err = json.Unmarshal(wire, &decodedViaJSON) if err != nil { t.Fatalf("Unexpected decode error %v", err) } - - // Things that Decode would have done for us: - decodedViaJSON.Kind = "" - decodedViaJSON.APIVersion = "" - - if e, a := outer, &decodedViaJSON; !reflect.DeepEqual(e, a) { - t.Errorf("Expected: %#v but got %#v", e, a) + if a := decodedViaJSON; a.Object.Object != nil || a.EmptyObject.Object != nil { + t.Errorf("Expected embedded objects to be nil: %#v", a) } } diff --git a/pkg/runtime/scheme.go b/pkg/runtime/scheme.go index 90df7c664e2..84cbc67cebf 100644 --- a/pkg/runtime/scheme.go +++ b/pkg/runtime/scheme.go @@ -121,7 +121,7 @@ func (self *Scheme) embeddedObjectToRawExtension(in *EmbeddedObject, out *RawExt } // rawExtensionToEmbeddedObject does the conversion you would expect from the name, using the information -// given in conversion.Scope. It's placed in the DefaultScheme as a ConversionFunc to enable plugins; +// given in conversion.Scope. It's placed in all schemes as a ConversionFunc to enable plugins; // see the comment for RawExtension. func (self *Scheme) rawExtensionToEmbeddedObject(in *RawExtension, out *EmbeddedObject, s conversion.Scope) error { if len(in.RawJSON) == 4 && string(in.RawJSON) == "null" {