diff --git a/mapper/mapper.go b/mapper/mapper.go index 787c7c16..2a66680c 100644 --- a/mapper/mapper.go +++ b/mapper/mapper.go @@ -315,7 +315,10 @@ func (m Mapper) NewMutation(tableName string, data interface{}, column string, m } var ovsValue interface{} - if mutator == "delete" && columnSchema.Type == ovsdb.TypeMap { + // Usually a mutation value is of the same type of the value being mutated + // except for delete mutation of maps where it can also be a list of same type of + // keys (rfc7047 5.1). Handle this special case here. + if mutator == "delete" && columnSchema.Type == ovsdb.TypeMap && reflect.TypeOf(value).Kind() != reflect.Map { // It's OK to cast the value to a list of elements because validation has passed ovsSet, err := ovsdb.NewOvsSet(value) if err != nil { diff --git a/mapper/mapper_test.go b/mapper/mapper_test.go index 98a67073..71485c88 100644 --- a/mapper/mapper_test.go +++ b/mapper/mapper_test.go @@ -983,7 +983,7 @@ func TestMapperMutation(t *testing.T) { err: false, }, { - name: "Delete elements from map ", + name: "Delete keys from map ", column: "map", obj: testType{}, mutator: ovsdb.MutateOperationDelete, @@ -991,6 +991,15 @@ func TestMapperMutation(t *testing.T) { expected: ovsdb.NewMutation("map", ovsdb.MutateOperationDelete, testOvsSet(t, []string{"foo", "bar"})), err: false, }, + { + name: "Delete key value pairs from map ", + column: "map", + obj: testType{}, + mutator: ovsdb.MutateOperationDelete, + value: map[string]string{"foo": "bar"}, + expected: ovsdb.NewMutation("map", ovsdb.MutateOperationDelete, testOvsMap(t, map[string]string{"foo": "bar"})), + err: false, + }, { name: "Insert elements in map ", column: "map", diff --git a/ovsdb/bindings.go b/ovsdb/bindings.go index dcf92177..734d80b1 100644 --- a/ovsdb/bindings.go +++ b/ovsdb/bindings.go @@ -115,6 +115,32 @@ func OvsToNativeAtomic(basicType string, ovsElem interface{}) (interface{}, erro } } +func OvsToNativeSlice(baseType string, ovsElem interface{}) (interface{}, error) { + naType := NativeTypeFromAtomic(baseType) + var nativeSet reflect.Value + switch ovsSet := ovsElem.(type) { + case OvsSet: + nativeSet = reflect.MakeSlice(reflect.SliceOf(naType), 0, len(ovsSet.GoSet)) + for _, v := range ovsSet.GoSet { + nv, err := OvsToNativeAtomic(baseType, v) + if err != nil { + return nil, err + } + nativeSet = reflect.Append(nativeSet, reflect.ValueOf(nv)) + } + + default: + nativeSet = reflect.MakeSlice(reflect.SliceOf(naType), 0, 1) + nv, err := OvsToNativeAtomic(baseType, ovsElem) + if err != nil { + return nil, err + } + + nativeSet = reflect.Append(nativeSet, reflect.ValueOf(nv)) + } + return nativeSet.Interface(), nil +} + // OvsToNative transforms an ovs type to native one based on the column type information func OvsToNative(column *ColumnSchema, ovsElem interface{}) (interface{}, error) { switch column.Type { @@ -172,28 +198,7 @@ func OvsToNative(column *ColumnSchema, ovsElem interface{}) (interface{}, error) } return array.Interface(), nil case reflect.Slice: - var nativeSet reflect.Value - switch ovsSet := ovsElem.(type) { - case OvsSet: - nativeSet = reflect.MakeSlice(naType, 0, len(ovsSet.GoSet)) - for _, v := range ovsSet.GoSet { - nv, err := OvsToNativeAtomic(column.TypeObj.Key.Type, v) - if err != nil { - return nil, err - } - nativeSet = reflect.Append(nativeSet, reflect.ValueOf(nv)) - } - - default: - nativeSet = reflect.MakeSlice(naType, 0, 1) - nv, err := OvsToNativeAtomic(column.TypeObj.Key.Type, ovsElem) - if err != nil { - return nil, err - } - - nativeSet = reflect.Append(nativeSet, reflect.ValueOf(nv)) - } - return nativeSet.Interface(), nil + return OvsToNativeSlice(column.TypeObj.Key.Type, ovsElem) default: return nil, fmt.Errorf("native type was not slice, array or pointer. got %d", naType.Kind()) } diff --git a/server/mutate.go b/server/mutate.go index 10104b17..5f66d6bf 100644 --- a/server/mutate.go +++ b/server/mutate.go @@ -67,6 +67,15 @@ func mutateInsert(current, value interface{}) interface{} { } return v.Interface() } + if vc.Kind() == reflect.Map && vv.Kind() == reflect.Map { + iter := vv.MapRange() + for iter.Next() { + k := iter.Key() + if !vc.MapIndex(k).IsValid() { + vc.SetMapIndex(k, iter.Value()) + } + } + } return current } @@ -88,6 +97,22 @@ func mutateDelete(current, value interface{}) interface{} { } return v.Interface() } + if vc.Kind() == reflect.Map && vv.Type() == reflect.SliceOf(vc.Type().Key()) { + for i := 0; i < vv.Len(); i++ { + vc.SetMapIndex(vv.Index(i), reflect.Value{}) + } + } + if vc.Kind() == reflect.Map && vv.Kind() == reflect.Map { + iter := vv.MapRange() + for iter.Next() { + vvk := iter.Key() + vvv := iter.Value() + vcv := vc.MapIndex(vvk) + if reflect.DeepEqual(vcv.Interface(), vvv.Interface()) { + vc.SetMapIndex(vvk, reflect.Value{}) + } + } + } return current } diff --git a/server/mutate_test.go b/server/mutate_test.go index b75a7d0f..da2ded38 100644 --- a/server/mutate_test.go +++ b/server/mutate_test.go @@ -259,6 +259,21 @@ func TestMutateInsert(t *testing.T) { []string{"baz", "quux", "foo"}, []string{"foo", "bar", "baz", "quux"}, }, + { + "insert key value pairs", + map[string]string{ + "foo": "bar", + }, + ovsdb.MutateOperationInsert, + map[string]string{ + "foo": "ignored", + "baz": "quux", + }, + map[string]string{ + "foo": "bar", + "baz": "quux", + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -292,6 +307,33 @@ func TestMutateDelete(t *testing.T) { []string{"bar", "baz"}, []string{"foo"}, }, + { + "delete key value pairs", + map[string]string{ + "foo": "bar", + "baz": "quux", + }, + ovsdb.MutateOperationDelete, + map[string]string{ + "foo": "ignored", + "baz": "quux", + }, + map[string]string{ + "foo": "bar", + }, + }, + { + "delete keys", + map[string]string{ + "foo": "bar", + "baz": "quux", + }, + ovsdb.MutateOperationDelete, + []string{"foo"}, + map[string]string{ + "baz": "quux", + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/server/transact.go b/server/transact.go index c20d3939..6585b9ff 100644 --- a/server/transact.go +++ b/server/transact.go @@ -2,6 +2,7 @@ package server import ( "fmt" + "reflect" "github.com/google/uuid" "github.com/ovn-org/libovsdb/cache" @@ -322,9 +323,20 @@ func (o *OvsdbServer) Mutate(database, table string, where []ovsdb.Condition, mu } for _, mutation := range mutations { column := schema.Column(mutation.Column) - nativeValue, err := ovsdb.OvsToNative(column, mutation.Value) - if err != nil { - panic(err) + var nativeValue interface{} + // Usually a mutation value is of the same type of the value being mutated + // except for delete mutation of maps where it can also be a list of same type of + // keys (rfc7047 5.1). Handle this special case here. + if mutation.Mutator == "delete" && column.Type == ovsdb.TypeMap && reflect.TypeOf(mutation.Value) != reflect.TypeOf(ovsdb.OvsMap{}) { + nativeValue, err = ovsdb.OvsToNativeSlice(column.TypeObj.Key.Type, mutation.Value) + if err != nil { + panic(err) + } + } else { + nativeValue, err = ovsdb.OvsToNative(column, mutation.Value) + if err != nil { + panic(err) + } } if err := ovsdb.ValidateMutation(column, mutation.Mutator, nativeValue); err != nil { panic(err) diff --git a/server/transact_test.go b/server/transact_test.go index abe3a323..497e9134 100644 --- a/server/transact_test.go +++ b/server/transact_test.go @@ -38,6 +38,11 @@ func TestMutateOp(t *testing.T) { bridge := bridgeType{ Name: "foo", + ExternalIds: map[string]string{ + "foo": "bar", + "baz": "qux", + "waldo": "fred", + }, } bridgeRow, err := m.NewRow("Bridge", &bridge) require.Nil(t, err) @@ -81,4 +86,42 @@ func TestMutateOp(t *testing.T) { }, }, }, gotUpdate) + + keyDelete, err := ovsdb.NewOvsSet([]string{"foo"}) + assert.Nil(t, err) + keyValueDelete, err := ovsdb.NewOvsMap(map[string]string{"baz": "qux"}) + assert.Nil(t, err) + gotResult, gotUpdate = o.Mutate( + "Open_vSwitch", + "Bridge", + []ovsdb.Condition{ + ovsdb.NewCondition("_uuid", ovsdb.ConditionEqual, ovsdb.UUID{GoUUID: bridgeUUID}), + }, + []ovsdb.Mutation{ + *ovsdb.NewMutation("external_ids", ovsdb.MutateOperationDelete, keyDelete), + *ovsdb.NewMutation("external_ids", ovsdb.MutateOperationDelete, keyValueDelete), + }, + ) + assert.Equal(t, ovsdb.OperationResult{Count: 1}, gotResult) + + oldExternalIds, err := ovsdb.NewOvsMap(bridge.ExternalIds) + assert.Nil(t, err) + newExternalIds, err := ovsdb.NewOvsMap(map[string]string{"waldo": "fred"}) + assert.Nil(t, err) + assert.Equal(t, ovsdb.TableUpdates{ + "Bridge": ovsdb.TableUpdate{ + bridgeUUID: &ovsdb.RowUpdate{ + Old: &ovsdb.Row{ + "_uuid": ovsdb.UUID{GoUUID: bridgeUUID}, + "name": "foo", + "external_ids": oldExternalIds, + }, + New: &ovsdb.Row{ + "_uuid": ovsdb.UUID{GoUUID: bridgeUUID}, + "name": "foo", + "external_ids": newExternalIds, + }, + }, + }, + }, gotUpdate) }