Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

server: support mutate over maps #225

Merged
merged 1 commit into from
Sep 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
43 changes: 43 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,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)
}