Skip to content

Commit

Permalink
server: support mutate over maps
Browse files Browse the repository at this point in the history
Signed-off-by: Jaime Caamaño Ruiz <jcaamano@redhat.com>
  • Loading branch information
jcaamano committed Sep 14, 2021
1 parent ca80b79 commit 0853837
Show file tree
Hide file tree
Showing 7 changed files with 165 additions and 27 deletions.
5 changes: 4 additions & 1 deletion mapper/mapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
11 changes: 10 additions & 1 deletion mapper/mapper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -983,14 +983,23 @@ func TestMapperMutation(t *testing.T) {
err: false,
},
{
name: "Delete elements from map ",
name: "Delete keys from map ",
column: "map",
obj: testType{},
mutator: ovsdb.MutateOperationDelete,
value: []string{"foo", "bar"},
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",
Expand Down
49 changes: 27 additions & 22 deletions ovsdb/bindings.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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())
}
Expand Down
25 changes: 25 additions & 0 deletions server/mutate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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
}

Expand Down
42 changes: 42 additions & 0 deletions server/mutate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
18 changes: 15 additions & 3 deletions server/transact.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package server

import (
"fmt"
"reflect"

"github.com/google/uuid"
"github.com/ovn-org/libovsdb/cache"
Expand Down Expand Up @@ -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)
Expand Down
42 changes: 42 additions & 0 deletions server/transact_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -81,4 +86,41 @@ 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.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)
}

0 comments on commit 0853837

Please sign in to comment.