* Fix kubecfg test, which was writing an internal object straight to yaml
* Address review comments
pull/6/head
Daniel Smith 2014-09-11 12:31:46 -07:00
parent 52d2c221b8
commit b8c955ea17
7 changed files with 82 additions and 64 deletions

View File

@ -40,17 +40,17 @@ type Converter struct {
// If non-nil, will be called to print helpful debugging info. Quite verbose.
Debug DebugLogger
// Name is called to retrieve the name of a type; this name is used for the
// NameFunc is called to retrieve the name of a type; this name is used for the
// purpose of deciding whether two types match or not (i.e., will we attempt to
// do a conversion). The default returns the go type name.
Name func(t reflect.Type) string
NameFunc func(t reflect.Type) string
}
// NewConverter creates a new Converter object.
func NewConverter() *Converter {
return &Converter{
funcs: map[typePair]reflect.Value{},
Name: func(t reflect.Type) string { return t.Name() },
funcs: map[typePair]reflect.Value{},
NameFunc: func(t reflect.Type) string { return t.Name() },
}
}
@ -71,62 +71,70 @@ type Scope interface {
Flags() FieldMatchingFlags
// Meta returns any information originally passed to Convert.
Meta() map[string]interface{}
Meta() *Meta
}
// Meta is supplied by Scheme, when it calls Convert.
type Meta struct {
SrcVersion string
DestVersion string
// TODO: If needed, add a user data field here.
}
// scope contains information about an ongoing conversion.
type scope struct {
converter *Converter
meta map[string]interface{}
meta *Meta
flags FieldMatchingFlags
srcTagStack []reflect.StructTag
destTagStack []reflect.StructTag
}
// push adds a level to the src/dest tag stacks.
func (sa *scope) push() {
sa.srcTagStack = append(sa.srcTagStack, "")
sa.destTagStack = append(sa.destTagStack, "")
func (s *scope) push() {
s.srcTagStack = append(s.srcTagStack, "")
s.destTagStack = append(s.destTagStack, "")
}
// pop removes a level to the src/dest tag stacks.
func (sa *scope) pop() {
n := len(sa.srcTagStack)
sa.srcTagStack = sa.srcTagStack[:n-1]
sa.destTagStack = sa.destTagStack[:n-1]
func (s *scope) pop() {
n := len(s.srcTagStack)
s.srcTagStack = s.srcTagStack[:n-1]
s.destTagStack = s.destTagStack[:n-1]
}
func (sa *scope) setSrcTag(tag reflect.StructTag) {
sa.srcTagStack[len(sa.srcTagStack)-1] = tag
func (s *scope) setSrcTag(tag reflect.StructTag) {
s.srcTagStack[len(s.srcTagStack)-1] = tag
}
func (sa *scope) setDestTag(tag reflect.StructTag) {
sa.destTagStack[len(sa.destTagStack)-1] = tag
func (s *scope) setDestTag(tag reflect.StructTag) {
s.destTagStack[len(s.destTagStack)-1] = tag
}
// Convert continues a conversion.
func (sa *scope) Convert(src, dest interface{}, flags FieldMatchingFlags) error {
return sa.converter.Convert(src, dest, flags, sa.meta)
func (s *scope) Convert(src, dest interface{}, flags FieldMatchingFlags) error {
return s.converter.Convert(src, dest, flags, s.meta)
}
// SrcTag returns the tag of the struct containing the current source item, if any.
func (sa *scope) SrcTag() reflect.StructTag {
return sa.srcTagStack[len(sa.srcTagStack)-1]
func (s *scope) SrcTag() reflect.StructTag {
return s.srcTagStack[len(s.srcTagStack)-1]
}
// DestTag returns the tag of the struct containing the current dest item, if any.
func (sa *scope) DestTag() reflect.StructTag {
return sa.destTagStack[len(sa.destTagStack)-1]
func (s *scope) DestTag() reflect.StructTag {
return s.destTagStack[len(s.destTagStack)-1]
}
// Flags returns the flags with which the current conversion was started.
func (sa *scope) Flags() FieldMatchingFlags {
return sa.flags
func (s *scope) Flags() FieldMatchingFlags {
return s.flags
}
// Meta returns the meta object that was originally passed to Convert.
func (sa *scope) Meta() map[string]interface{} {
return sa.meta
func (s *scope) Meta() *Meta {
return s.meta
}
// Register registers a conversion func with the Converter. conversionFunc must take
@ -202,9 +210,9 @@ func (f FieldMatchingFlags) IsSet(flag FieldMatchingFlags) bool {
// Read the comments on the various FieldMatchingFlags constants to understand
// what the 'flags' parameter does.
// 'meta' is given to allow you to pass information to conversion functions,
// it is not used by Convert other than storing it in the scope.
// it is not used by Convert() other than storing it in the scope.
// Not safe for objects with cyclic references!
func (c *Converter) Convert(src, dest interface{}, flags FieldMatchingFlags, meta map[string]interface{}) error {
func (c *Converter) Convert(src, dest interface{}, flags FieldMatchingFlags, meta *Meta) error {
dv, sv := reflect.ValueOf(dest), reflect.ValueOf(src)
if dv.Kind() != reflect.Ptr {
return fmt.Errorf("Need pointer, but got %#v", dest)
@ -244,7 +252,7 @@ func (c *Converter) convert(sv, dv reflect.Value, scope *scope) error {
return ret.(error)
}
if !scope.flags.IsSet(AllowDifferentFieldTypeNames) && c.Name(dt) != c.Name(st) {
if !scope.flags.IsSet(AllowDifferentFieldTypeNames) && c.NameFunc(dt) != c.NameFunc(st) {
return fmt.Errorf("Can't convert %v to %v because type names don't match.", st, dt)
}

View File

@ -103,7 +103,7 @@ func TestConverter_fuzz(t *testing.T) {
f := fuzz.New().NilChance(.5).NumElements(0, 100)
c := NewConverter()
c.Name = func(t reflect.Type) string {
c.NameFunc = func(t reflect.Type) string {
// Hide the fact that we don't have separate packages for these things.
return map[reflect.Type]string{
reflect.TypeOf(TestType1{}): "TestType1",
@ -168,7 +168,7 @@ func TestConverter_meta(t *testing.T) {
checks := 0
err := c.Register(
func(in *Foo, out *Bar, s Scope) error {
if s.Meta()["test"] != "passes" {
if s.Meta() == nil || s.Meta().SrcVersion != "test" || s.Meta().DestVersion != "passes" {
t.Errorf("Meta did not get passed!")
}
checks++
@ -181,7 +181,7 @@ func TestConverter_meta(t *testing.T) {
}
err = c.Register(
func(in *string, out *string, s Scope) error {
if s.Meta()["test"] != "passes" {
if s.Meta() == nil || s.Meta().SrcVersion != "test" || s.Meta().DestVersion != "passes" {
t.Errorf("Meta did not get passed a second time!")
}
checks++
@ -191,7 +191,7 @@ func TestConverter_meta(t *testing.T) {
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}
err = c.Convert(&Foo{}, &Bar{}, 0, map[string]interface{}{"test": "passes"})
err = c.Convert(&Foo{}, &Bar{}, 0, &Meta{SrcVersion: "test", DestVersion: "passes"})
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}

View File

@ -86,15 +86,19 @@ func NewScheme() *Scheme {
ExternalVersion: "v1",
MetaInsertionFactory: metaInsertion{},
}
s.converter.Name = func(t reflect.Type) string {
if kind, ok := s.typeToKind[t]; ok {
return kind
}
return t.Name()
}
s.converter.NameFunc = s.nameFunc
return s
}
// nameFunc returns the name of the type that we wish to use for encoding. Defaults to
// the go name of the type if the type is not registered.
func (s *Scheme) nameFunc(t reflect.Type) string {
if kind, ok := s.typeToKind[t]; ok {
return kind
}
return t.Name()
}
// AddKnownTypes registers all types passed in 'types' as being members of version 'version.
// Encode() will refuse objects unless their type has been registered with AddKnownTypes.
// All objects passed to types should be pointers to structs. The name that go reports for
@ -161,22 +165,25 @@ func (s *Scheme) NewObject(versionName, typeName string) (interface{}, error) {
//
// Note that, if you need to copy sub-objects that didn't change, you can use the
// conversion.Scope object that will be passed to your conversion function.
// Additionally, all conversions started by Scheme will set the "srcVersion" and
// "destVersion" keys on the meta object. Example:
// Additionally, all conversions started by Scheme will set the SrcVersion and
// DestVersion fields on the Meta object. Example:
//
// s.AddConversionFuncs(
// func(in *InternalObject, out *ExternalObject, scope conversion.Scope) error {
// // You can depend on this being set to the source version, e.g., "".
// s.Meta()["srcVersion"].(string)
// // You can depend on Meta() being non-nil, and this being set to
// // the source version, e.g., ""
// s.Meta().SrcVersion
// // You can depend on this being set to the destination version,
// // e.g., "v1beta1".
// s.Meta()["destVersion"].(string)
// s.Meta().DestVersion
// // Call scope.Convert to copy sub-fields.
// s.Convert(&in.SubFieldThatMoved, &out.NewLocation.NewName, 0)
// return nil
// },
// )
//
// (For more detail about conversion functions, see Converter.Register's comment.)
//
// Also note that the default behavior, if you don't add a conversion function, is to
// sanely copy fields that have the same names and same type names. It's OK if the
// destination type has extra fields, but it must not remove any. So you only need to
@ -196,7 +203,7 @@ func (s *Scheme) AddConversionFuncs(conversionFuncs ...interface{}) error {
// possible. You can call this with types that haven't been registered (for example,
// a to test conversion of types that are nested within registered types), but in
// that case, the conversion.Scope object passed to your conversion functions won't
// have "srcVersion" or "destVersion" keys set correctly in Meta().
// have SrcVersion or DestVersion fields set correctly in Meta().
func (s *Scheme) Convert(in, out interface{}) error {
inVersion := "unknown"
outVersion := "unknown"
@ -209,11 +216,11 @@ func (s *Scheme) Convert(in, out interface{}) error {
return s.converter.Convert(in, out, 0, s.generateConvertMeta(inVersion, outVersion))
}
// generateConvertMeta assembles a map for the meta value we pass to Convert.
func (s *Scheme) generateConvertMeta(srcVersion, destVersion string) map[string]interface{} {
return map[string]interface{}{
"srcVersion": srcVersion,
"destVersion": destVersion,
// generateConvertMeta constructs the meta value we pass to Convert.
func (s *Scheme) generateConvertMeta(srcVersion, destVersion string) *Meta {
return &Meta{
SrcVersion: srcVersion,
DestVersion: destVersion,
}
}

View File

@ -293,10 +293,10 @@ func TestMetaValues(t *testing.T) {
// Register functions to verify that scope.Meta() gets set correctly.
err := s.AddConversionFuncs(
func(in *InternalSimple, out *ExternalSimple, scope Scope) error {
if e, a := "", scope.Meta()["srcVersion"].(string); e != a {
if e, a := "", scope.Meta().SrcVersion; e != a {
t.Errorf("Expected '%v', got '%v'", e, a)
}
if e, a := "externalVersion", scope.Meta()["destVersion"].(string); e != a {
if e, a := "externalVersion", scope.Meta().DestVersion; e != a {
t.Errorf("Expected '%v', got '%v'", e, a)
}
scope.Convert(&in.TestString, &out.TestString, 0)
@ -304,10 +304,10 @@ func TestMetaValues(t *testing.T) {
return nil
},
func(in *ExternalSimple, out *InternalSimple, scope Scope) error {
if e, a := "externalVersion", scope.Meta()["srcVersion"].(string); e != a {
if e, a := "externalVersion", scope.Meta().SrcVersion; e != a {
t.Errorf("Expected '%v', got '%v'", e, a)
}
if e, a := "", scope.Meta()["destVersion"].(string); e != a {
if e, a := "", scope.Meta().DestVersion; e != a {
t.Errorf("Expected '%v', got '%v'", e, a)
}
scope.Convert(&in.TestString, &out.TestString, 0)
@ -381,10 +381,10 @@ func TestMetaValuesUnregisteredConvert(t *testing.T) {
// Register functions to verify that scope.Meta() gets set correctly.
err := s.AddConversionFuncs(
func(in *InternalSimple, out *ExternalSimple, scope Scope) error {
if e, a := "unknown", scope.Meta()["srcVersion"].(string); e != a {
if e, a := "unknown", scope.Meta().SrcVersion; e != a {
t.Errorf("Expected '%v', got '%v'", e, a)
}
if e, a := "unknown", scope.Meta()["destVersion"].(string); e != a {
if e, a := "unknown", scope.Meta().DestVersion; e != a {
t.Errorf("Expected '%v', got '%v'", e, a)
}
scope.Convert(&in.TestString, &out.TestString, 0)

View File

@ -17,6 +17,7 @@ limitations under the License.
package kubecfg
import (
"encoding/json"
"testing"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
@ -34,7 +35,9 @@ func TestParseBadStorage(t *testing.T) {
func DoParseTest(t *testing.T, storage string, obj runtime.Object, p *Parser) {
jsonData, _ := runtime.DefaultCodec.Encode(obj)
yamlData, _ := yaml.Marshal(obj)
var tmp map[string]interface{}
json.Unmarshal(jsonData, &tmp)
yamlData, _ := yaml.Marshal(tmp)
t.Logf("Intermediate yaml:\n%v\n", string(yamlData))
t.Logf("Intermediate json:\n%v\n", string(jsonData))
jsonGot, jsonErr := p.ToWireFormat(jsonData, storage, runtime.DefaultCodec)

View File

@ -54,8 +54,8 @@ func GetScheme(schemeName string) *Scheme {
// from a conversion.Scope.
func fromScope(s conversion.Scope) (inVersion, outVersion string, scheme *Scheme) {
scheme = DefaultScheme
inVersion = s.Meta()["srcVersion"].(string)
outVersion = s.Meta()["destVersion"].(string)
inVersion = s.Meta().SrcVersion
outVersion = s.Meta().DestVersion
// If a scheme tag was provided, use it. Look at the struct tag corresponding
// to version "".
if name := s.SrcTag().Get("scheme"); inVersion == "" && name != "" {

View File

@ -52,10 +52,10 @@ func TestScheme(t *testing.T) {
// Register functions to verify that scope.Meta() gets set correctly.
err := runtime.DefaultScheme.AddConversionFuncs(
func(in *InternalSimple, out *ExternalSimple, scope conversion.Scope) error {
if e, a := "", scope.Meta()["srcVersion"].(string); e != a {
if e, a := "", scope.Meta().SrcVersion; e != a {
t.Errorf("Expected '%v', got '%v'", e, a)
}
if e, a := "externalVersion", scope.Meta()["destVersion"].(string); e != a {
if e, a := "externalVersion", scope.Meta().DestVersion; e != a {
t.Errorf("Expected '%v', got '%v'", e, a)
}
scope.Convert(&in.JSONBase, &out.JSONBase, 0)
@ -64,10 +64,10 @@ func TestScheme(t *testing.T) {
return nil
},
func(in *ExternalSimple, out *InternalSimple, scope conversion.Scope) error {
if e, a := "externalVersion", scope.Meta()["srcVersion"].(string); e != a {
if e, a := "externalVersion", scope.Meta().SrcVersion; e != a {
t.Errorf("Expected '%v', got '%v'", e, a)
}
if e, a := "", scope.Meta()["destVersion"].(string); e != a {
if e, a := "", scope.Meta().DestVersion; e != a {
t.Errorf("Expected '%v', got '%v'", e, a)
}
scope.Convert(&in.JSONBase, &out.JSONBase, 0)