From a19f52835190c70937b46925aee7b443b908321d Mon Sep 17 00:00:00 2001 From: wojtekt Date: Thu, 3 Sep 2020 19:40:04 +0200 Subject: [PATCH] Remove FieldMatchingFlags --- pkg/apis/flowcontrol/internalbootstrap/BUILD | 1 - .../internalbootstrap/default-internal.go | 11 +-- .../apimachinery/pkg/conversion/converter.go | 90 ++----------------- .../pkg/conversion/converter_test.go | 20 ++--- .../k8s.io/apimachinery/pkg/runtime/scheme.go | 25 ++---- .../conversion-gen/generators/conversion.go | 10 +-- 6 files changed, 31 insertions(+), 126 deletions(-) diff --git a/pkg/apis/flowcontrol/internalbootstrap/BUILD b/pkg/apis/flowcontrol/internalbootstrap/BUILD index 565f63683d7..b7a4a3aad9d 100644 --- a/pkg/apis/flowcontrol/internalbootstrap/BUILD +++ b/pkg/apis/flowcontrol/internalbootstrap/BUILD @@ -9,7 +9,6 @@ go_library( "//pkg/apis/flowcontrol:go_default_library", "//pkg/apis/flowcontrol/install:go_default_library", "//staging/src/k8s.io/api/flowcontrol/v1alpha1:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/conversion:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", "//staging/src/k8s.io/apiserver/pkg/apis/flowcontrol/bootstrap:go_default_library", ], diff --git a/pkg/apis/flowcontrol/internalbootstrap/default-internal.go b/pkg/apis/flowcontrol/internalbootstrap/default-internal.go index dd7d6753b7a..7685f9beef0 100644 --- a/pkg/apis/flowcontrol/internalbootstrap/default-internal.go +++ b/pkg/apis/flowcontrol/internalbootstrap/default-internal.go @@ -18,7 +18,6 @@ package internalbootstrap import ( fcv1a1 "k8s.io/api/flowcontrol/v1alpha1" - "k8s.io/apimachinery/pkg/conversion" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/apis/flowcontrol/bootstrap" "k8s.io/kubernetes/pkg/apis/flowcontrol" @@ -49,11 +48,10 @@ func NewAPFScheme() *runtime.Scheme { func internalizeFSes(exts []*fcv1a1.FlowSchema) map[string]*flowcontrol.FlowSchema { ans := make(map[string]*flowcontrol.FlowSchema, len(exts)) - converter := NewAPFScheme().Converter() + scheme := NewAPFScheme() for _, ext := range exts { var untyped flowcontrol.FlowSchema - err := converter.Convert(ext, &untyped, 0, &conversion.Meta{}) - if err != nil { + if err := scheme.Convert(ext, &untyped, nil); err != nil { panic(err) } ans[ext.Name] = &untyped @@ -63,11 +61,10 @@ func internalizeFSes(exts []*fcv1a1.FlowSchema) map[string]*flowcontrol.FlowSche func internalizePLs(exts []*fcv1a1.PriorityLevelConfiguration) map[string]*flowcontrol.PriorityLevelConfiguration { ans := make(map[string]*flowcontrol.PriorityLevelConfiguration, len(exts)) - converter := NewAPFScheme().Converter() + scheme := NewAPFScheme() for _, ext := range exts { var untyped flowcontrol.PriorityLevelConfiguration - err := converter.Convert(ext, &untyped, 0, &conversion.Meta{}) - if err != nil { + if err := scheme.Convert(ext, &untyped, nil); err != nil { panic(err) } ans[ext.Name] = &untyped diff --git a/staging/src/k8s.io/apimachinery/pkg/conversion/converter.go b/staging/src/k8s.io/apimachinery/pkg/conversion/converter.go index 94ccad96a6c..79134847644 100644 --- a/staging/src/k8s.io/apimachinery/pkg/conversion/converter.go +++ b/staging/src/k8s.io/apimachinery/pkg/conversion/converter.go @@ -47,12 +47,6 @@ type Converter struct { ignoredConversions map[typePair]struct{} ignoredUntypedConversions map[typePair]struct{} - // Map from an input type to a function which can apply a key name mapping - inputFieldMappingFuncs map[reflect.Type]FieldMappingFunc - - // Map from an input type to a set of default conversion flags. - inputDefaultFlags map[reflect.Type]FieldMatchingFlags - // 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. @@ -67,9 +61,6 @@ func NewConverter(nameFn NameFunc) *Converter { ignoredConversions: make(map[typePair]struct{}), ignoredUntypedConversions: make(map[typePair]struct{}), nameFunc: nameFn, - - inputFieldMappingFuncs: make(map[reflect.Type]FieldMappingFunc), - inputDefaultFlags: make(map[reflect.Type]FieldMatchingFlags), } c.RegisterUntypedConversionFunc( (*[]byte)(nil), (*[]byte)(nil), @@ -88,11 +79,9 @@ func (c *Converter) WithConversions(fns ConversionFuncs) *Converter { return &copied } -// DefaultMeta returns the conversion FieldMappingFunc and meta for a given type. -func (c *Converter) DefaultMeta(t reflect.Type) (FieldMatchingFlags, *Meta) { - return c.inputDefaultFlags[t], &Meta{ - KeyNameMapping: c.inputFieldMappingFuncs[t], - } +// DefaultMeta returns meta for a given type. +func (c *Converter) DefaultMeta(t reflect.Type) *Meta { + return &Meta{} } // Convert_Slice_byte_To_Slice_byte prevents recursing into every byte @@ -112,19 +101,12 @@ func Convert_Slice_byte_To_Slice_byte(in *[]byte, out *[]byte, s Scope) error { type Scope interface { // Call Convert to convert sub-objects. Note that if you call it with your own exact // parameters, you'll run out of stack space before anything useful happens. - Convert(src, dest interface{}, flags FieldMatchingFlags) error - - // Flags returns the flags with which the conversion was started. - Flags() FieldMatchingFlags + Convert(src, dest interface{}) error // Meta returns any information originally passed to Convert. Meta() *Meta } -// FieldMappingFunc can convert an input field value into different values, depending on -// the value of the source or destination struct tags. -type FieldMappingFunc func(key string, sourceTag, destTag reflect.StructTag) (source string, dest string) - func NewConversionFuncs() ConversionFuncs { return ConversionFuncs{ untyped: make(map[typePair]ConversionFunc), @@ -165,9 +147,6 @@ func (c ConversionFuncs) Merge(other ConversionFuncs) ConversionFuncs { // Meta is supplied by Scheme, when it calls Convert. type Meta struct { - // KeyNameMapping is an optional function which may map the listed key (field name) - // into a source and destination value. - KeyNameMapping FieldMappingFunc // Context is an optional field that callers may use to pass info to conversion functions. Context interface{} } @@ -176,17 +155,11 @@ type Meta struct { type scope struct { converter *Converter meta *Meta - flags FieldMatchingFlags } // Convert continues a conversion. -func (s *scope) Convert(src, dest interface{}, flags FieldMatchingFlags) error { - return s.converter.Convert(src, dest, flags, s.meta) -} - -// Flags returns the flags with which the current conversion was started. -func (s *scope) Flags() FieldMatchingFlags { - return s.flags +func (s *scope) Convert(src, dest interface{}) error { + return s.converter.Convert(src, dest, s.meta) } // Meta returns the meta object that was originally passed to Convert. @@ -224,65 +197,16 @@ func (c *Converter) RegisterIgnoredConversion(from, to interface{}) error { return nil } -// RegisterInputDefaults registers a field name mapping function, used when converting -// from maps to structs. Inputs to the conversion methods are checked for this type and a mapping -// applied automatically if the input matches in. A set of default flags for the input conversion -// may also be provided, which will be used when no explicit flags are requested. -func (c *Converter) RegisterInputDefaults(in interface{}, fn FieldMappingFunc, defaultFlags FieldMatchingFlags) error { - fv := reflect.ValueOf(in) - ft := fv.Type() - if ft.Kind() != reflect.Ptr { - return fmt.Errorf("expected pointer 'in' argument, got: %v", ft) - } - c.inputFieldMappingFuncs[ft] = fn - c.inputDefaultFlags[ft] = defaultFlags - return nil -} - -// FieldMatchingFlags contains a list of ways in which struct fields could be -// copied. These constants may be | combined. -type FieldMatchingFlags int - -const ( - // Loop through destination fields, search for matching source - // field to copy it from. Source fields with no corresponding - // destination field will be ignored. If SourceToDest is - // specified, this flag is ignored. If neither is specified, - // or no flags are passed, this flag is the default. - DestFromSource FieldMatchingFlags = 0 - // Loop through source fields, search for matching dest field - // to copy it into. Destination fields with no corresponding - // source field will be ignored. - SourceToDest FieldMatchingFlags = 1 << iota - // Don't treat it as an error if the corresponding source or - // dest field can't be found. - IgnoreMissingFields - // Don't require type names to match. - AllowDifferentFieldTypeNames -) - -// IsSet returns true if the given flag or combination of flags is set. -func (f FieldMatchingFlags) IsSet(flag FieldMatchingFlags) bool { - if flag == DestFromSource { - // The bit logic doesn't work on the default value. - return f&SourceToDest != SourceToDest - } - return f&flag == flag -} - // Convert will translate src to dest if it knows how. Both must be pointers. // If no conversion func is registered and the default copying mechanism // doesn't work on this type pair, an error will be returned. -// 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. // Not safe for objects with cyclic references! -func (c *Converter) Convert(src, dest interface{}, flags FieldMatchingFlags, meta *Meta) error { +func (c *Converter) Convert(src, dest interface{}, meta *Meta) error { pair := typePair{reflect.TypeOf(src), reflect.TypeOf(dest)} scope := &scope{ converter: c, - flags: flags, meta: meta, } diff --git a/staging/src/k8s.io/apimachinery/pkg/conversion/converter_test.go b/staging/src/k8s.io/apimachinery/pkg/conversion/converter_test.go index 5073c510e57..8bcdb9f9b4e 100644 --- a/staging/src/k8s.io/apimachinery/pkg/conversion/converter_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/conversion/converter_test.go @@ -27,7 +27,7 @@ func TestConverter_byteSlice(t *testing.T) { c := NewConverter(DefaultNameFunc) src := []byte{1, 2, 3} dest := []byte{} - err := c.Convert(&src, &dest, 0, nil) + err := c.Convert(&src, &dest, nil) if err != nil { t.Fatalf("expected no error") } @@ -58,7 +58,7 @@ func TestConverter_MismatchedTypes(t *testing.T) { src := []string{"5"} var dest int - if err := c.Convert(&src, &dest, 0, nil); err != nil { + if err := c.Convert(&src, &dest, nil); err != nil { t.Fatalf("unexpected error: %v", err) } if e, a := 5, dest; e != a { @@ -107,7 +107,7 @@ func TestConverter_CallsRegisteredFunctions(t *testing.T) { x := A{"hello, intrepid test reader!", 3} y := B{} - if err := c.Convert(&x, &y, 0, nil); err != nil { + if err := c.Convert(&x, &y, nil); err != nil { t.Fatalf("unexpected error %v", err) } if e, a := x.Foo, y.Bar; e != a { @@ -120,7 +120,7 @@ func TestConverter_CallsRegisteredFunctions(t *testing.T) { z := B{"all your test are belong to us", 42} w := A{} - if err := c.Convert(&z, &w, 0, nil); err != nil { + if err := c.Convert(&z, &w, nil); err != nil { t.Fatalf("unexpected error %v", err) } if e, a := z.Bar, w.Foo; e != a { @@ -141,7 +141,7 @@ func TestConverter_CallsRegisteredFunctions(t *testing.T) { ); err != nil { t.Fatalf("unexpected error %v", err) } - if err := c.Convert(&A{}, &C{}, 0, nil); err == nil { + if err := c.Convert(&A{}, &C{}, nil); err == nil { t.Errorf("unexpected non-error") } } @@ -169,7 +169,7 @@ func TestConverter_IgnoredConversion(t *testing.T) { } a := A{} b := B{} - if err := c.Convert(&a, &b, 0, nil); err != nil { + if err := c.Convert(&a, &b, nil); err != nil { t.Errorf("%v", err) } if count != 0 { @@ -206,7 +206,7 @@ func TestConverter_GeneratedConversionOverridden(t *testing.T) { a := A{} b := B{} - if err := c.Convert(&a, &b, 0, nil); err != nil { + if err := c.Convert(&a, &b, nil); err != nil { t.Errorf("%v", err) } } @@ -249,10 +249,10 @@ func TestConverter_WithConversionOverridden(t *testing.T) { a := A{} b := B{} - if err := c.Convert(&a, &b, 0, nil); err == nil || err.Error() != "conversion function should be overridden" { + if err := c.Convert(&a, &b, nil); err == nil || err.Error() != "conversion function should be overridden" { t.Errorf("unexpected error: %v", err) } - if err := newc.Convert(&a, &b, 0, nil); err != nil { + if err := newc.Convert(&a, &b, nil); err != nil { t.Errorf("%v", err) } } @@ -278,7 +278,7 @@ func TestConverter_meta(t *testing.T) { ); err != nil { t.Fatalf("Unexpected error: %v", err) } - if err := c.Convert(&Foo{}, &Bar{}, 0, &Meta{}); err != nil { + if err := c.Convert(&Foo{}, &Bar{}, &Meta{}); err != nil { t.Fatalf("Unexpected error: %v", err) } if checks != 1 { diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/scheme.go b/staging/src/k8s.io/apimachinery/pkg/runtime/scheme.go index 7a1e70663dd..697dd4ed77c 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/scheme.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/scheme.go @@ -18,7 +18,6 @@ package runtime import ( "fmt" - "net/url" "reflect" "strings" @@ -105,9 +104,6 @@ func NewScheme() *Scheme { // Enable couple default conversions by default. utilruntime.Must(RegisterEmbeddedConversions(s)) utilruntime.Must(RegisterStringConversions(s)) - - utilruntime.Must(s.RegisterInputDefaults(&map[string][]string{}, JSONKeyMapper, conversion.AllowDifferentFieldTypeNames|conversion.IgnoreMissingFields)) - utilruntime.Must(s.RegisterInputDefaults(&url.Values{}, JSONKeyMapper, conversion.AllowDifferentFieldTypeNames|conversion.IgnoreMissingFields)) return s } @@ -337,14 +333,6 @@ func (s *Scheme) AddFieldLabelConversionFunc(gvk schema.GroupVersionKind, conver return nil } -// RegisterInputDefaults sets the provided field mapping function and field matching -// as the defaults for the provided input type. The fn may be nil, in which case no -// mapping will happen by default. Use this method to register a mechanism for handling -// a specific input type in conversion, such as a map[string]string to structs. -func (s *Scheme) RegisterInputDefaults(in interface{}, fn conversion.FieldMappingFunc, defaultFlags conversion.FieldMatchingFlags) error { - return s.converter.RegisterInputDefaults(in, fn, defaultFlags) -} - // AddTypeDefaultingFunc registers a function that is passed a pointer to an // object and can default fields on the object. These functions will be invoked // when Default() is called. The function will never be called unless the @@ -428,12 +416,9 @@ func (s *Scheme) Convert(in, out interface{}, context interface{}) error { in = typed } - flags, meta := s.generateConvertMeta(in) + meta := s.generateConvertMeta(in) meta.Context = context - if flags == 0 { - flags = conversion.AllowDifferentFieldTypeNames - } - return s.converter.Convert(in, out, flags, meta) + return s.converter.Convert(in, out, meta) } // ConvertFieldLabel alters the given field label and value for an kind field selector from @@ -530,9 +515,9 @@ func (s *Scheme) convertToVersion(copy bool, in Object, target GroupVersioner) ( in = in.DeepCopyObject() } - flags, meta := s.generateConvertMeta(in) + meta := s.generateConvertMeta(in) meta.Context = target - if err := s.converter.Convert(in, out, flags, meta); err != nil { + if err := s.converter.Convert(in, out, meta); err != nil { return nil, err } @@ -560,7 +545,7 @@ func (s *Scheme) unstructuredToTyped(in Unstructured) (Object, error) { } // generateConvertMeta constructs the meta value we pass to Convert. -func (s *Scheme) generateConvertMeta(in interface{}) (conversion.FieldMatchingFlags, *conversion.Meta) { +func (s *Scheme) generateConvertMeta(in interface{}) *conversion.Meta { return s.converter.DefaultMeta(reflect.TypeOf(in)) } diff --git a/staging/src/k8s.io/code-generator/cmd/conversion-gen/generators/conversion.go b/staging/src/k8s.io/code-generator/cmd/conversion-gen/generators/conversion.go index 1e7fb794e95..d0007aa1ed1 100644 --- a/staging/src/k8s.io/code-generator/cmd/conversion-gen/generators/conversion.go +++ b/staging/src/k8s.io/code-generator/cmd/conversion-gen/generators/conversion.go @@ -828,11 +828,11 @@ func (g *genConversion) doMap(inType, outType *types.Type, sw *generator.Snippet if conversionExists { sw.Do("return err\n", nil) sw.Do("}\n", nil) - } - if inType.Key == outType.Key { - sw.Do("(*out)[key] = *newVal\n", nil) - } else { - sw.Do("(*out)[$.|raw$(key)] = *newVal\n", outType.Key) + if inType.Key == outType.Key { + sw.Do("(*out)[key] = *newVal\n", nil) + } else { + sw.Do("(*out)[$.|raw$(key)] = *newVal\n", outType.Key) + } } } } else {