Merge pull request #71 from jennybuckley/untyped-separable

Make deduced type use type lookup instead of duplicating logic

Author: k8s-ci-robot

internal/cli/operation.go

@@ -33,7 +33,7 @@ type operationBase struct {
 	typeName string
 }
 
-func (b operationBase) parseFile(path string) (tv typed.TypedValue, err error) {
+func (b operationBase) parseFile(path string) (tv *typed.TypedValue, err error) {
 	bytes, err := ioutil.ReadFile(path)
 	if err != nil {
 		return tv, fmt.Errorf("unable to read file %q: %v", path, err)

internal/fixture/state.go

@@ -29,7 +29,7 @@ import (
 // State of the current test in terms of live object. One can check at
 // any time that Live and Managers match the expectations.
 type State struct {
-	Live     typed.TypedValue
+	Live     *typed.TypedValue
 	Parser   typed.ParseableType
 	Managers fieldpath.ManagedFields
 	Updater  *merge.Updater
@@ -146,7 +146,7 @@ type dummyConverter struct{}
 var _ merge.Converter = dummyConverter{}
 
 // Convert returns the object given in input, not doing any conversion.
-func (dummyConverter) Convert(v typed.TypedValue, version fieldpath.APIVersion) (typed.TypedValue, error) {
+func (dummyConverter) Convert(v *typed.TypedValue, version fieldpath.APIVersion) (*typed.TypedValue, error) {
 	if len(version) == 0 {
 		return nil, fmt.Errorf("cannot convert to invalid version: %q", version)
 	}

merge/deduced_test.go

@@ -168,7 +168,7 @@ func TestDeduced(t *testing.T) {
 				},
 			},
 		},
-		"leaf_apply_twice_dangling": {
+		"leaf_apply_twice_remove": {
 			Ops: []Operation{
 				Apply{
 					Manager:    "default",
@@ -188,9 +188,7 @@ func TestDeduced(t *testing.T) {
 				},
 			},
 			Object: `
-				numeric: 1
 				string: "new string"
-				bool: false
 			`,
 			Managed: fieldpath.ManagedFields{
 				"default": &fieldpath.VersionedSet{
@@ -328,9 +326,7 @@ func TestDeduced(t *testing.T) {
 					Object:     ``,
 				},
 			},
-			Object: `
-				string: "string"
-			`,
+			Object:  ``,
 			Managed: fieldpath.ManagedFields{},
 		},
 		"apply_update_apply_nested": {
@@ -523,7 +519,7 @@ func TestDeduced(t *testing.T) {
 
 	for name, test := range tests {
 		t.Run(name, func(t *testing.T) {
-			if err := test.Test(typed.DeducedParseableType{}); err != nil {
+			if err := test.Test(typed.DeducedParseableType); err != nil {
 				t.Fatal(err)
 			}
 		})

merge/multiple_appliers_test.go

@@ -834,7 +834,7 @@ var _ merge.Converter = repeatingConverter{}
 var missingVersionError error = fmt.Errorf("cannot convert to invalid version")
 
 // Convert implements merge.Converter
-func (r repeatingConverter) Convert(v typed.TypedValue, version fieldpath.APIVersion) (typed.TypedValue, error) {
+func (r repeatingConverter) Convert(v *typed.TypedValue, version fieldpath.APIVersion) (*typed.TypedValue, error) {
 	if len(version) < 2 || string(version)[0] != 'v' {
 		return nil, missingVersionError
 	}

merge/obsolete_versions_test.go

@@ -32,7 +32,7 @@ type specificVersionConverter struct {
 	AcceptedVersions []fieldpath.APIVersion
 }
 
-func (d *specificVersionConverter) Convert(object typed.TypedValue, version fieldpath.APIVersion) (typed.TypedValue, error) {
+func (d *specificVersionConverter) Convert(object *typed.TypedValue, version fieldpath.APIVersion) (*typed.TypedValue, error) {
 	for _, v := range d.AcceptedVersions {
 		if v == version {
 			return object, nil
@@ -53,7 +53,7 @@ func TestObsoleteVersions(t *testing.T) {
 	}
 	state := fixture.State{
 		Updater: &merge.Updater{Converter: converter},
-		Parser:  typed.DeducedParseableType{},
+		Parser:  typed.DeducedParseableType,
 	}
 
 	if err := state.Update(typed.YAMLObject(`{"v1": 0}`), fieldpath.APIVersion("v1"), "v1"); err != nil {

merge/update.go

@@ -23,7 +23,7 @@ import (
 // Converter is an interface to the conversion logic. The converter
 // needs to be able to convert objects from one version to another.
 type Converter interface {
-	Convert(object typed.TypedValue, version fieldpath.APIVersion) (typed.TypedValue, error)
+	Convert(object *typed.TypedValue, version fieldpath.APIVersion) (*typed.TypedValue, error)
 	IsMissingVersionError(error) bool
 }
 
@@ -33,12 +33,12 @@ type Updater struct {
 	Converter Converter
 }
 
-func (s *Updater) update(oldObject, newObject typed.TypedValue, version fieldpath.APIVersion, managers fieldpath.ManagedFields, workflow string, force bool) (fieldpath.ManagedFields, error) {
+func (s *Updater) update(oldObject, newObject *typed.TypedValue, version fieldpath.APIVersion, managers fieldpath.ManagedFields, workflow string, force bool) (fieldpath.ManagedFields, error) {
 	conflicts := fieldpath.ManagedFields{}
 	removed := fieldpath.ManagedFields{}
 	type Versioned struct {
-		oldObject typed.TypedValue
-		newObject typed.TypedValue
+		oldObject *typed.TypedValue
+		newObject *typed.TypedValue
 	}
 	versions := map[fieldpath.APIVersion]Versioned{
 		version: Versioned{
@@ -119,7 +119,7 @@ func (s *Updater) update(oldObject, newObject typed.TypedValue, version fieldpat
 // that you intend to persist (after applying the patch if this is for a
 // PATCH call), and liveObject must be the original object (empty if
 // this is a CREATE call).
-func (s *Updater) Update(liveObject, newObject typed.TypedValue, version fieldpath.APIVersion, managers fieldpath.ManagedFields, manager string) (fieldpath.ManagedFields, error) {
+func (s *Updater) Update(liveObject, newObject *typed.TypedValue, version fieldpath.APIVersion, managers fieldpath.ManagedFields, manager string) (fieldpath.ManagedFields, error) {
 	var err error
 	managers = shallowCopyManagers(managers)
 	managers, err = s.update(liveObject, newObject, version, managers, manager, true)
@@ -146,7 +146,7 @@ func (s *Updater) Update(liveObject, newObject typed.TypedValue, version fieldpa
 // Apply should be called when Apply is run, given the current object as
 // well as the configuration that is applied. This will merge the object
 // and return it.
-func (s *Updater) Apply(liveObject, configObject typed.TypedValue, version fieldpath.APIVersion, managers fieldpath.ManagedFields, manager string, force bool) (typed.TypedValue, fieldpath.ManagedFields, error) {
+func (s *Updater) Apply(liveObject, configObject *typed.TypedValue, version fieldpath.APIVersion, managers fieldpath.ManagedFields, manager string, force bool) (*typed.TypedValue, fieldpath.ManagedFields, error) {
 	managers = shallowCopyManagers(managers)
 	newObject, err := liveObject.Merge(configObject)
 	if err != nil {
@@ -185,7 +185,7 @@ func shallowCopyManagers(managers fieldpath.ManagedFields) fieldpath.ManagedFiel
 // * applyingManager applied it last time
 // * applyingManager didn't apply it this time
 // * no other applier claims to manage it
-func (s *Updater) prune(merged typed.TypedValue, managers fieldpath.ManagedFields, applyingManager string, lastSet *fieldpath.VersionedSet) (typed.TypedValue, error) {
+func (s *Updater) prune(merged *typed.TypedValue, managers fieldpath.ManagedFields, applyingManager string, lastSet *fieldpath.VersionedSet) (*typed.TypedValue, error) {
 	if lastSet == nil || lastSet.Set.Empty() {
 		return merged, nil
 	}
@@ -210,7 +210,7 @@ func (s *Updater) prune(merged typed.TypedValue, managers fieldpath.ManagedField
 
 // addBackOwnedItems adds back any list and map items that were removed by prune,
 // but other appliers (or the current applier's new config) claim to own.
-func (s *Updater) addBackOwnedItems(merged, pruned typed.TypedValue, managedFields fieldpath.ManagedFields, applyingManager string) (typed.TypedValue, error) {
+func (s *Updater) addBackOwnedItems(merged, pruned *typed.TypedValue, managedFields fieldpath.ManagedFields, applyingManager string) (*typed.TypedValue, error) {
 	var err error
 	managedAtVersion := map[fieldpath.APIVersion]*fieldpath.Set{}
 	for _, managerSet := range managedFields {
@@ -253,7 +253,7 @@ func (s *Updater) addBackOwnedItems(merged, pruned typed.TypedValue, managedFiel
 // addBackDanglingItems makes sure that the only items removed by prune are items that were
 // previously owned by the currently applying manager. This will add back unowned items and items
 // which are owned by Updaters that shouldn't be removed.
-func (s *Updater) addBackDanglingItems(merged, pruned typed.TypedValue, lastSet *fieldpath.VersionedSet) (typed.TypedValue, error) {
+func (s *Updater) addBackDanglingItems(merged, pruned *typed.TypedValue, lastSet *fieldpath.VersionedSet) (*typed.TypedValue, error) {
 	convertedPruned, err := s.Converter.Convert(pruned, lastSet.APIVersion)
 	if err != nil {
 		if s.Converter.IsMissingVersionError(err) {

schema/elements.go

@@ -16,6 +16,8 @@ limitations under the License.
 
 package schema
 
+import "sigs.k8s.io/structured-merge-diff/value"
+
 // Schema is a list of named types.
 type Schema struct {
 	Types []TypeDef `yaml:"types,omitempty"`
@@ -77,6 +79,8 @@ const (
 	// Separable means the items of the container type have no particular
 	// relationship (default behavior for maps and structs).
 	Separable = ElementRelationship("separable")
+	// Deduced only applies to untyped (see the documentation there).
+	Deduced = ElementRelationship("deduced")
 )
 
 // Struct represents a type which is composed of a number of different fields.
@@ -180,17 +184,45 @@ type Map struct {
 type Untyped struct {
 	// ElementRelationship states the relationship between the items, if
 	// container-typed data happens to be present here.
+	// * `deduced` implies that the behavior is based on the type of data.
+	//   Structs and maps are both treated as a `separable` Map with `deduced` Untyped elements.
+	//   Lists and Scalars are both treated as an `atomic` Untyped.
 	// * `atomic` implies that all elements depend on each other, and this
 	//   is effectively a scalar / leaf field; it doesn't make sense for
 	//   separate actors to set the elements.
 	// TODO: support "guess" (guesses at associative list keys)
-	// TODO: support "lookup" (calls a lookup function to figure out the
-	//       schema based on the data)
 	// The default behavior for untyped data is `atomic`; it's permitted to
 	// leave this unset to get the default behavior.
 	ElementRelationship ElementRelationship `yaml:"elementRelationship,omitempty"`
 }
 
+// DeduceType determines the behavior based on a value.
+func DeduceType(v *value.Value) TypeRef {
+	if v != nil && v.MapValue != nil {
+		return TypeRef{
+			Inlined: Atom{
+				Map: &Map{
+					ElementType: TypeRef{
+						Inlined: Atom{
+							Untyped: &Untyped{
+								ElementRelationship: Deduced,
+							},
+						},
+					},
+					ElementRelationship: Separable,
+				},
+			},
+		}
+	}
+	return TypeRef{
+		Inlined: Atom{
+			Untyped: &Untyped{
+				ElementRelationship: Atomic,
+			},
+		},
+	}
+}
+
 // FindNamedType is a convenience function that returns the referenced TypeDef,
 // if it exists, or (nil, false) if it doesn't.
 func (s Schema) FindNamedType(name string) (TypeDef, bool) {