From 28ce0d02d48a76ffb53a2bd6415a92dfc517e8ea Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Wed, 31 Aug 2022 17:07:29 -0500 Subject: [PATCH] ovsdb: don't create sized arrays for OVS Sets OVS Sets must be unique, but modelgen created sized arrays to represent OVS Sets, which when marshalled are filled with duplicate default values. For a column schema like: "cvlans": { "type": {"key": {"type": "integer", "minInteger": 0, "maxInteger": 4095}, "min": 0, "max": 4096}}, modelgen would create: CVLANs [4096]int which when marshalled and sent to ovsdb-server becomes: cvlans:{GoSet:[0 0 0 0 0 0 0 ]} and is rejected by ovsdb-server with: {Count:0 Error:ovsdb error Details:set contains duplicate UUID:{GoUUID:} Rows:[]}] and errors [ovsdb error: set contains duplicate]: 1 ovsdb operations failed Instead, generate these fields as slices instead of sized arrays and enforce the size when marshalling the set. Signed-off-by: Dan Williams --- cache/cache_test.go | 35 +++++++++++---- client/api_test.go | 25 +++++------ client/client_test.go | 22 ++++++++-- client/condition_test.go | 6 +-- mapper/mapper.go | 2 +- mapper/mapper_test.go | 54 +++++++++++++++-------- modelgen/table.go | 9 ++-- modelgen/table_test.go | 2 +- ovsdb/bindings.go | 32 +------------- ovsdb/bindings_test.go | 41 +++++++++--------- ovsdb/condition_test.go | 4 +- ovsdb/encoding_test.go | 22 +++++++++- ovsdb/mutation_test.go | 4 +- ovsdb/notation.go | 4 ++ ovsdb/notation_test.go | 8 ++-- ovsdb/schema.go | 3 +- ovsdb/set.go | 69 +++++++++++++++++++++++++---- ovsdb/set_test.go | 44 +++++++++---------- ovsdb/updates2_test.go | 6 +-- server/transact.go | 2 +- server/transact_test.go | 93 ++++++++++++++++++++++++---------------- 21 files changed, 302 insertions(+), 185 deletions(-) diff --git a/cache/cache_test.go b/cache/cache_test.go index d3bad8c3..17844419 100644 --- a/cache/cache_test.go +++ b/cache/cache_test.go @@ -1951,6 +1951,24 @@ func BenchmarkIndexExists(b *testing.B) { } } +func testOvsSet(t assert.TestingT, keyType string, set interface{}) ovsdb.OvsSet { + colSchema := fmt.Sprintf(`{ + "type": { + "key": {"type": "%s"}, + "min": 0, + "max": "unlimited" + } + }`, keyType) + + var column ovsdb.ColumnSchema + err := json.Unmarshal([]byte(colSchema), &column) + assert.NoError(t, err) + + oSet, err := ovsdb.NewOvsSet(&column, set) + assert.NoError(t, err) + return oSet +} + func BenchmarkPopulate2UpdateArray(b *testing.B) { const numRows int = 500 @@ -1977,7 +1995,8 @@ func BenchmarkPopulate2UpdateArray(b *testing.B) { b.ResetTimer() for n := 0; n < b.N; n++ { for i := 0; i < numRows; i++ { - updatedRow := ovsdb.Row(map[string]interface{}{"array": ovsdb.OvsSet{GoSet: updateSet}}) + ovsSet := testOvsSet(b, ovsdb.TypeString, updateSet) + updatedRow := ovsdb.Row(map[string]interface{}{"array": ovsSet}) err := tc.Populate2(ovsdb.TableUpdates2{ "Open_vSwitch": { "foo": &ovsdb.RowUpdate2{ @@ -1995,7 +2014,7 @@ func BenchmarkTableCacheApplyModificationsSet(b *testing.B) { UUID string `ovsdb:"_uuid"` Set []string `ovsdb:"set"` } - aFooSet, _ := ovsdb.NewOvsSet([]string{"foo"}) + aFooSet := testOvsSet(b, ovsdb.TypeString, []string{"foo"}) base := &testDBModel{Set: []string{}} for i := 0; i < 57000; i++ { base.Set = append(base.Set, fmt.Sprintf("foo%d", i)) @@ -2119,10 +2138,10 @@ func TestTableCacheApplyModifications(t *testing.T) { Ptr *string `ovsdb:"ptr"` Ptr2 *string `ovsdb:"ptr2"` } - aEmptySet, _ := ovsdb.NewOvsSet([]string{}) - aFooSet, _ := ovsdb.NewOvsSet([]string{"foo"}) - aFooBarSet, _ := ovsdb.NewOvsSet([]string{"foo", "bar"}) - aFooBarBazQuxSet, _ := ovsdb.NewOvsSet([]string{"foo", "bar", "baz", "qux"}) + aEmptySet := testOvsSet(t, ovsdb.TypeString, []string{}) + aFooSet := testOvsSet(t, ovsdb.TypeString, []string{"foo"}) + aFooBarSet := testOvsSet(t, ovsdb.TypeString, []string{"foo", "bar"}) + aFooBarBazQuxSet := testOvsSet(t, ovsdb.TypeString, []string{"foo", "bar", "baz", "qux"}) aFooMap, _ := ovsdb.NewOvsMap(map[string]string{"foo": "bar"}) aBarMap, _ := ovsdb.NewOvsMap(map[string]string{"bar": "baz"}) aBarBazMap, _ := ovsdb.NewOvsMap(map[string]string{ @@ -2130,9 +2149,9 @@ func TestTableCacheApplyModifications(t *testing.T) { "baz": "quux", }) wallace := "wallace" - aWallaceSet, _ := ovsdb.NewOvsSet([]string{wallace}) + aWallaceSet := testOvsSet(t, ovsdb.TypeString, []string{wallace}) gromit := "gromit" - aWallaceGromitSet, _ := ovsdb.NewOvsSet([]string{wallace, gromit}) + aWallaceGromitSet := testOvsSet(t, ovsdb.TypeString, []string{wallace, gromit}) tests := []struct { name string update ovsdb.Row diff --git a/client/api_test.go b/client/api_test.go index 7c768109..50f077dd 100644 --- a/client/api_test.go +++ b/client/api_test.go @@ -914,7 +914,7 @@ func TestAPIMutate(t *testing.T) { { Op: ovsdb.OperationMutate, Table: "Logical_Switch_Port", - Mutations: []ovsdb.Mutation{{Column: "addresses", Mutator: ovsdb.MutateOperationInsert, Value: testOvsSet(t, []string{"1.1.1.1"})}}, + Mutations: []ovsdb.Mutation{{Column: "addresses", Mutator: ovsdb.MutateOperationInsert, Value: testOvsSet(t, ovsdb.TypeString, ovsdb.Unlimited, []string{"1.1.1.1"})}}, Where: []ovsdb.Condition{{Column: "_uuid", Function: ovsdb.ConditionEqual, Value: ovsdb.UUID{GoUUID: aUUID0}}}, }, }, @@ -940,19 +940,19 @@ func TestAPIMutate(t *testing.T) { { Op: ovsdb.OperationMutate, Table: "Logical_Switch_Port", - Mutations: []ovsdb.Mutation{{Column: "addresses", Mutator: ovsdb.MutateOperationInsert, Value: testOvsSet(t, []string{"2.2.2.2"})}}, + Mutations: []ovsdb.Mutation{{Column: "addresses", Mutator: ovsdb.MutateOperationInsert, Value: testOvsSet(t, ovsdb.TypeString, ovsdb.Unlimited, []string{"2.2.2.2"})}}, Where: []ovsdb.Condition{{Column: "_uuid", Function: ovsdb.ConditionEqual, Value: ovsdb.UUID{GoUUID: aUUID0}}}, }, { Op: ovsdb.OperationMutate, Table: "Logical_Switch_Port", - Mutations: []ovsdb.Mutation{{Column: "addresses", Mutator: ovsdb.MutateOperationInsert, Value: testOvsSet(t, []string{"2.2.2.2"})}}, + Mutations: []ovsdb.Mutation{{Column: "addresses", Mutator: ovsdb.MutateOperationInsert, Value: testOvsSet(t, ovsdb.TypeString, ovsdb.Unlimited, []string{"2.2.2.2"})}}, Where: []ovsdb.Condition{{Column: "_uuid", Function: ovsdb.ConditionEqual, Value: ovsdb.UUID{GoUUID: aUUID1}}}, }, { Op: ovsdb.OperationMutate, Table: "Logical_Switch_Port", - Mutations: []ovsdb.Mutation{{Column: "addresses", Mutator: ovsdb.MutateOperationInsert, Value: testOvsSet(t, []string{"2.2.2.2"})}}, + Mutations: []ovsdb.Mutation{{Column: "addresses", Mutator: ovsdb.MutateOperationInsert, Value: testOvsSet(t, ovsdb.TypeString, ovsdb.Unlimited, []string{"2.2.2.2"})}}, Where: []ovsdb.Condition{{Column: "_uuid", Function: ovsdb.ConditionEqual, Value: ovsdb.UUID{GoUUID: aUUID2}}}, }, }, @@ -976,7 +976,7 @@ func TestAPIMutate(t *testing.T) { { Op: ovsdb.OperationMutate, Table: "Logical_Switch_Port", - Mutations: []ovsdb.Mutation{{Column: "external_ids", Mutator: ovsdb.MutateOperationDelete, Value: testOvsSet(t, []string{"foo"})}}, + Mutations: []ovsdb.Mutation{{Column: "external_ids", Mutator: ovsdb.MutateOperationDelete, Value: testOvsSet(t, ovsdb.TypeString, ovsdb.Unlimited, []string{"foo"})}}, Where: []ovsdb.Condition{{Column: "_uuid", Function: ovsdb.ConditionEqual, Value: ovsdb.UUID{GoUUID: aUUID2}}}, }, }, @@ -1000,7 +1000,7 @@ func TestAPIMutate(t *testing.T) { { Op: ovsdb.OperationMutate, Table: "Logical_Switch_Port", - Mutations: []ovsdb.Mutation{{Column: "external_ids", Mutator: ovsdb.MutateOperationDelete, Value: testOvsSet(t, []string{"foo"})}}, + Mutations: []ovsdb.Mutation{{Column: "external_ids", Mutator: ovsdb.MutateOperationDelete, Value: testOvsSet(t, ovsdb.TypeString, ovsdb.Unlimited, []string{"foo"})}}, Where: []ovsdb.Condition{{Column: "name", Function: ovsdb.ConditionEqual, Value: "foo"}}, }, }, @@ -1136,10 +1136,10 @@ func TestAPIUpdate(t *testing.T) { tcache := apiTestCache(t, testData) testObj := testLogicalSwitchPort{} - testRow := ovsdb.Row(map[string]interface{}{"type": "somethingElse", "tag": testOvsSet(t, []int{6})}) - tagRow := ovsdb.Row(map[string]interface{}{"tag": testOvsSet(t, []int{6})}) + testRow := ovsdb.Row(map[string]interface{}{"type": "somethingElse", "tag": testOvsSet(t, ovsdb.TypeInteger, 1, []int{6})}) + tagRow := ovsdb.Row(map[string]interface{}{"tag": testOvsSet(t, ovsdb.TypeInteger, 1, []int{6})}) var nilInt *int - testNilRow := ovsdb.Row(map[string]interface{}{"type": "somethingElse", "tag": testOvsSet(t, nilInt)}) + testNilRow := ovsdb.Row(map[string]interface{}{"type": "somethingElse", "tag": testOvsSet(t, ovsdb.TypeInteger, 1, nilInt)}) typeRow := ovsdb.Row(map[string]interface{}{"type": "somethingElse"}) fields := []interface{}{&testObj.Tag, &testObj.Type} @@ -1350,7 +1350,7 @@ func TestAPIUpdate(t *testing.T) { Row: tagRow, Where: []ovsdb.Condition{ {Column: "type", Function: ovsdb.ConditionEqual, Value: "sometype"}, - {Column: "enabled", Function: ovsdb.ConditionIncludes, Value: testOvsSet(t, &trueVal)}, + {Column: "enabled", Function: ovsdb.ConditionIncludes, Value: testOvsSet(t, ovsdb.TypeBoolean, 1, &trueVal)}, }, }, }, @@ -1403,7 +1403,7 @@ func TestAPIUpdate(t *testing.T) { Op: ovsdb.OperationUpdate, Table: "Logical_Switch_Port", Row: tagRow, - Where: []ovsdb.Condition{{Column: "tag", Function: ovsdb.ConditionNotEqual, Value: testOvsSet(t, &one)}}, + Where: []ovsdb.Condition{{Column: "tag", Function: ovsdb.ConditionNotEqual, Value: testOvsSet(t, ovsdb.TypeInteger, 1, &one)}}, }, }, err: false, @@ -1843,6 +1843,7 @@ func TestAPIWait(t *testing.T) { tcache := apiTestCache(t, cache.Data{}) timeout0 := 0 + trueSet := testOvsSet(t, ovsdb.TypeBoolean, 1, []interface{}{true}) test := []struct { name string condition func(API) ConditionalAPI @@ -1944,7 +1945,7 @@ func TestAPIWait(t *testing.T) { { Column: "up", Function: ovsdb.ConditionNotEqual, - Value: ovsdb.OvsSet{GoSet: []interface{}{true}}, + Value: trueSet, }, }, Until: string(ovsdb.WaitConditionNotEqual), diff --git a/client/client_test.go b/client/client_test.go index 2850268f..c81d70e4 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -57,7 +57,7 @@ type Bridge struct { DatapathVersion string `ovsdb:"datapath_version"` ExternalIDs map[string]string `ovsdb:"external_ids"` FailMode *BridgeFailMode `ovsdb:"fail_mode"` - FloodVLANs [4096]int `ovsdb:"flood_vlans"` + FloodVLANs []int `ovsdb:"flood_vlans"` FlowTables map[int]string `ovsdb:"flow_tables"` IPFIX *string `ovsdb:"ipfix"` McastSnoopingEnable bool `ovsdb:"mcast_snooping_enable"` @@ -486,8 +486,24 @@ var schema = `{ } }` -func testOvsSet(t *testing.T, set interface{}) ovsdb.OvsSet { - oSet, err := ovsdb.NewOvsSet(set) +func testOvsSet(t *testing.T, keyType string, maxSize int, set interface{}) ovsdb.OvsSet { + maxStr := `"unlimited"` + if maxSize > 0 { + maxStr = fmt.Sprintf("%d", maxSize) + } + colSchema := fmt.Sprintf(`{ + "type": { + "key": {"type": "%s"}, + "min": 0, + "max": %s + } + }`, keyType, maxStr) + + var column ovsdb.ColumnSchema + err := json.Unmarshal([]byte(colSchema), &column) + assert.Nil(t, err) + + oSet, err := ovsdb.NewOvsSet(&column, set) assert.Nil(t, err) return oSet } diff --git a/client/condition_test.go b/client/condition_test.go index 5bd63b33..ce47845c 100644 --- a/client/condition_test.go +++ b/client/condition_test.go @@ -347,7 +347,7 @@ func TestExplicitConditionalWithNoCache(t *testing.T) { { Column: "enabled", Function: ovsdb.ConditionEqual, - Value: testOvsSet(t, &trueVal), + Value: testOvsSet(t, ovsdb.TypeBoolean, 1, &trueVal), }}}, }, { @@ -369,7 +369,7 @@ func TestExplicitConditionalWithNoCache(t *testing.T) { { Column: "enabled", Function: ovsdb.ConditionEqual, - Value: testOvsSet(t, &trueVal), + Value: testOvsSet(t, ovsdb.TypeBoolean, 1, &trueVal), }}, { { @@ -396,7 +396,7 @@ func TestExplicitConditionalWithNoCache(t *testing.T) { { Column: "enabled", Function: ovsdb.ConditionEqual, - Value: testOvsSet(t, &trueVal), + Value: testOvsSet(t, ovsdb.TypeBoolean, 1, &trueVal), }, { Column: "name", diff --git a/mapper/mapper.go b/mapper/mapper.go index 7b762003..30a9a91a 100644 --- a/mapper/mapper.go +++ b/mapper/mapper.go @@ -247,7 +247,7 @@ func (m Mapper) NewMutation(data *Info, column string, mutator ovsdb.Mutator, va // 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) + ovsSet, err := ovsdb.NewOvsSet(columnSchema, value) if err != nil { return nil, err } diff --git a/mapper/mapper_test.go b/mapper/mapper_test.go index 2cefedee..998534d5 100644 --- a/mapper/mapper_test.go +++ b/mapper/mapper_test.go @@ -33,7 +33,7 @@ var ( } aFloat = 42.00 - aFloatSet = [10]float64{ + aFloatSet = []float64{ 3.0, 2.0, 42.0, @@ -157,7 +157,7 @@ var testSchema = []byte(`{ func getOvsTestRow(t *testing.T) ovsdb.Row { ovsRow := ovsdb.NewRow() ovsRow["aString"] = aString - ovsRow["aSet"] = testOvsSet(t, aSet) + ovsRow["aSet"] = testOvsSet(t, ovsdb.TypeString, ovsdb.Unlimited, aSet) // Set's can hold the value if they have len == 1 ovsRow["aSingleSet"] = aString @@ -165,17 +165,17 @@ func getOvsTestRow(t *testing.T) ovsdb.Row { for _, u := range aUUIDSet { us = append(us, ovsdb.UUID{GoUUID: u}) } - ovsRow["aUUIDSet"] = testOvsSet(t, us) + ovsRow["aUUIDSet"] = testOvsSet(t, ovsdb.TypeUUID, ovsdb.Unlimited, us) ovsRow["aUUID"] = ovsdb.UUID{GoUUID: aUUID0} - ovsRow["aIntSet"] = testOvsSet(t, aIntSet) + ovsRow["aIntSet"] = testOvsSet(t, ovsdb.TypeInteger, ovsdb.Unlimited, aIntSet) ovsRow["aFloat"] = aFloat - ovsRow["aFloatSet"] = testOvsSet(t, aFloatSet) + ovsRow["aFloatSet"] = testOvsSet(t, ovsdb.TypeReal, 10, aFloatSet) - ovsRow["aEmptySet"] = testOvsSet(t, []string{}) + ovsRow["aEmptySet"] = testOvsSet(t, ovsdb.TypeString, ovsdb.Unlimited, []string{}) ovsRow["aEnum"] = aEnum @@ -193,7 +193,7 @@ func TestMapperGetData(t *testing.T) { AUUID string `ovsdb:"aUUID"` AIntSet []int `ovsdb:"aIntSet"` AFloat float64 `ovsdb:"aFloat"` - AFloatSet [10]float64 `ovsdb:"aFloatSet"` + AFloatSet []float64 `ovsdb:"aFloatSet"` YetAnotherStringSet []string `ovsdb:"aEmptySet"` AEnum string `ovsdb:"aEnum"` AMap map[string]string `ovsdb:"aMap"` @@ -265,7 +265,7 @@ func TestMapperNewRow(t *testing.T) { }{ SomeSet: aSet, }, - expectedRow: ovsdb.Row(map[string]interface{}{"aSet": testOvsSet(t, aSet)}), + expectedRow: ovsdb.Row(map[string]interface{}{"aSet": testOvsSet(t, ovsdb.TypeString, ovsdb.Unlimited, aSet)}), }, { name: "emptySet with no column specification", objInput: &struct { @@ -289,7 +289,7 @@ func TestMapperNewRow(t *testing.T) { }{ MyUUIDSet: []string{aUUID0, aUUID1}, }, - expectedRow: ovsdb.Row(map[string]interface{}{"aUUIDSet": testOvsSet(t, []ovsdb.UUID{{GoUUID: aUUID0}, {GoUUID: aUUID1}})}), + expectedRow: ovsdb.Row(map[string]interface{}{"aUUIDSet": testOvsSet(t, ovsdb.TypeUUID, ovsdb.Unlimited, []ovsdb.UUID{{GoUUID: aUUID0}, {GoUUID: aUUID1}})}), }, { name: "aIntSet", objInput: &struct { @@ -297,7 +297,7 @@ func TestMapperNewRow(t *testing.T) { }{ MyIntSet: []int{0, 42}, }, - expectedRow: ovsdb.Row(map[string]interface{}{"aIntSet": testOvsSet(t, []int{0, 42})}), + expectedRow: ovsdb.Row(map[string]interface{}{"aIntSet": testOvsSet(t, ovsdb.TypeInteger, ovsdb.Unlimited, []int{0, 42})}), }, { name: "aFloat", objInput: &struct { @@ -309,11 +309,11 @@ func TestMapperNewRow(t *testing.T) { }, { name: "aFloatSet", objInput: &struct { - MyFloatSet [10]float64 `ovsdb:"aFloatSet"` + MyFloatSet []float64 `ovsdb:"aFloatSet"` }{ MyFloatSet: aFloatSet, }, - expectedRow: ovsdb.Row(map[string]interface{}{"aFloatSet": testOvsSet(t, aFloatSet)}), + expectedRow: ovsdb.Row(map[string]interface{}{"aFloatSet": testOvsSet(t, ovsdb.TypeReal, 10, aFloatSet)}), }, { name: "Enum", objInput: &struct { @@ -401,7 +401,7 @@ func TestMapperNewRowFields(t *testing.T) { prepare: func(o *obj) { }, fields: []interface{}{&testObj.MySet}, - expectedRow: ovsdb.Row(map[string]interface{}{"aSet": testOvsSet(t, []string{})}), + expectedRow: ovsdb.Row(map[string]interface{}{"aSet": testOvsSet(t, ovsdb.TypeString, ovsdb.Unlimited, []string{})}), }, { name: "empty maps", prepare: func(o *obj) { @@ -424,7 +424,7 @@ func TestMapperNewRowFields(t *testing.T) { o.MyFloat = aFloat }, fields: []interface{}{&testObj.MyMap, &testObj.MySet}, - expectedRow: ovsdb.Row(map[string]interface{}{"aMap": testOvsMap(t, aMap), "aSet": testOvsSet(t, aSet)}), + expectedRow: ovsdb.Row(map[string]interface{}{"aMap": testOvsMap(t, aMap), "aSet": testOvsSet(t, ovsdb.TypeString, ovsdb.Unlimited, aSet)}), }, } @@ -989,7 +989,7 @@ func TestMapperMutation(t *testing.T) { obj: testType{}, mutator: ovsdb.MutateOperationInsert, value: []string{"foo"}, - expected: ovsdb.NewMutation("set", ovsdb.MutateOperationInsert, testOvsSet(t, []string{"foo"})), + expected: ovsdb.NewMutation("set", ovsdb.MutateOperationInsert, testOvsSet(t, ovsdb.TypeString, ovsdb.Unlimited, []string{"foo"})), err: false, }, { @@ -998,7 +998,7 @@ func TestMapperMutation(t *testing.T) { obj: testType{}, mutator: ovsdb.MutateOperationDelete, value: []string{"foo"}, - expected: ovsdb.NewMutation("set", ovsdb.MutateOperationDelete, testOvsSet(t, []string{"foo"})), + expected: ovsdb.NewMutation("set", ovsdb.MutateOperationDelete, testOvsSet(t, ovsdb.TypeString, ovsdb.Unlimited, []string{"foo"})), err: false, }, { @@ -1007,7 +1007,7 @@ func TestMapperMutation(t *testing.T) { obj: testType{}, mutator: ovsdb.MutateOperationDelete, value: []string{"foo", "bar"}, - expected: ovsdb.NewMutation("map", ovsdb.MutateOperationDelete, testOvsSet(t, []string{"foo", "bar"})), + expected: ovsdb.NewMutation("map", ovsdb.MutateOperationDelete, testOvsSet(t, ovsdb.TypeString, ovsdb.Unlimited, []string{"foo", "bar"})), err: false, }, { @@ -1050,8 +1050,24 @@ func TestMapperMutation(t *testing.T) { } } -func testOvsSet(t *testing.T, set interface{}) ovsdb.OvsSet { - oSet, err := ovsdb.NewOvsSet(set) +func testOvsSet(t *testing.T, keyType string, maxSize int, set interface{}) ovsdb.OvsSet { + maxStr := `"unlimited"` + if maxSize > 0 { + maxStr = fmt.Sprintf("%d", maxSize) + } + colSchema := fmt.Sprintf(`{ + "type": { + "key": {"type": "%s"}, + "min": 0, + "max": %s + } + }`, keyType, maxStr) + + var column ovsdb.ColumnSchema + err := json.Unmarshal([]byte(colSchema), &column) + assert.Nil(t, err) + + oSet, err := ovsdb.NewOvsSet(&column, set) assert.Nil(t, err) return oSet } diff --git a/modelgen/table.go b/modelgen/table.go index d9873417..b16837c6 100644 --- a/modelgen/table.go +++ b/modelgen/table.go @@ -347,12 +347,9 @@ func fieldType(tableName, columnName string, column *ovsdb.ColumnSchema, enumTyp } return AtomicType(column.TypeObj.Key.Type) } - // use array for columns with max > 1 - if column.TypeObj.Max() > 1 { - if enumTypes && FieldEnum(tableName, columnName, column) != nil { - return fmt.Sprintf("[%d]%s", column.TypeObj.Max(), enumName(tableName, columnName)) - } - return fmt.Sprintf("[%d]%s", column.TypeObj.Max(), AtomicType(column.TypeObj.Key.Type)) + // use array for enums with max > 1 + if column.TypeObj.Max() > 1 && enumTypes && FieldEnum(tableName, columnName, column) != nil { + return fmt.Sprintf("[%d]%s", column.TypeObj.Max(), enumName(tableName, columnName)) } // use a slice if enumTypes && FieldEnum(tableName, columnName, column) != nil { diff --git a/modelgen/table_test.go b/modelgen/table_test.go index 0b81928a..0759f274 100644 --- a/modelgen/table_test.go +++ b/modelgen/table_test.go @@ -764,7 +764,7 @@ func buildTestBridge() *vswitchd.Bridge { DatapathVersion: *buildRandStr(), ExternalIDs: map[string]string{*buildRandStr(): *buildRandStr(), *buildRandStr(): *buildRandStr()}, FailMode: &vswitchd.BridgeFailModeSecure, - FloodVLANs: [4096]int{0, 1, 2, 3, 4, 5, 6, 7, 8, 9}, + FloodVLANs: []int{0, 1, 2, 3, 4, 5, 6, 7, 8, 9}, FlowTables: map[int]string{1: *buildRandStr(), 2: *buildRandStr()}, IPFIX: buildRandStr(), McastSnoopingEnable: false, diff --git a/ovsdb/bindings.go b/ovsdb/bindings.go index 4c675858..d6ca263b 100644 --- a/ovsdb/bindings.go +++ b/ovsdb/bindings.go @@ -78,10 +78,6 @@ func NativeType(column *ColumnSchema) reflect.Type { if column.TypeObj.Min() == 1 && column.TypeObj.Max() == 1 { return keyType } - // max is > 1, use an array - if column.TypeObj.Max() > 1 { - return reflect.ArrayOf(column.TypeObj.Max(), keyType) - } return reflect.SliceOf(keyType) default: panic(fmt.Errorf("unknown extended type %s", column.Type)) @@ -255,33 +251,7 @@ func NativeToOvs(column *ColumnSchema, rawElem interface{}) (interface{}, error) case TypeUUID: return UUID{GoUUID: rawElem.(string)}, nil case TypeSet: - var ovsSet OvsSet - if column.TypeObj.Key.Type == TypeUUID { - ovsSlice := []interface{}{} - if _, ok := rawElem.([]string); ok { - for _, v := range rawElem.([]string) { - uuid := UUID{GoUUID: v} - ovsSlice = append(ovsSlice, uuid) - } - } else if _, ok := rawElem.(*string); ok { - v := rawElem.(*string) - if v != nil { - uuid := UUID{GoUUID: *v} - ovsSlice = append(ovsSlice, uuid) - } - } else { - return nil, fmt.Errorf("uuid slice was neither []string or *string") - } - ovsSet = OvsSet{GoSet: ovsSlice} - - } else { - var err error - ovsSet, err = NewOvsSet(rawElem) - if err != nil { - return nil, err - } - } - return ovsSet, nil + return NewOvsSet(column, rawElem) case TypeMap: nativeMapVal := reflect.ValueOf(rawElem) ovsMap := make(map[interface{}]interface{}, nativeMapVal.Len()) diff --git a/ovsdb/bindings_test.go b/ovsdb/bindings_test.go index afac2d06..4a464ef8 100644 --- a/ovsdb/bindings_test.go +++ b/ovsdb/bindings_test.go @@ -20,8 +20,6 @@ var ( aUUID2 = "2f77b348-9768-4866-b761-89d5177ecda2" aUUID3 = "2f77b348-9768-4866-b761-89d5177ecda3" - aSingleUUIDSet, _ = NewOvsSet(UUID{GoUUID: aUUID0}) - aUUIDSet = []string{ aUUID0, aUUID1, @@ -60,26 +58,28 @@ var ( ) func TestOvsToNativeAndNativeToOvs(t *testing.T) { - s, _ := NewOvsSet(aSet) - s1, _ := NewOvsSet([]string{aString}) + s, _ := newOvsSetRaw(TypeString, &Unlimited, aSet) + s1, _ := newOvsSetRaw(TypeString, &Unlimited, []string{aString}) us := make([]UUID, 0) for _, u := range aUUIDSet { us = append(us, UUID{GoUUID: u}) } - uss, _ := NewOvsSet(us) + uss, _ := newOvsSetRaw(TypeUUID, &Unlimited, us) us1 := []UUID{{GoUUID: aUUID0}} - uss1, _ := NewOvsSet(us1) + uss1, _ := newOvsSetRaw(TypeUUID, &Unlimited, us1) - is, _ := NewOvsSet(aIntSet) - fs, _ := NewOvsSet(aFloatSet) + is, _ := newOvsSetRaw(TypeInteger, &Unlimited, aIntSet) + fs, _ := newOvsSetRaw(TypeReal, &Unlimited, aFloatSet) - sis, _ := NewOvsSet([]int{aInt}) - sfs, _ := NewOvsSet([]float64{aFloat}) + sis, _ := newOvsSetRaw(TypeInteger, &Unlimited, []int{aInt}) + sfs, _ := newOvsSetRaw(TypeReal, &Unlimited, []float64{aFloat}) - es, _ := NewOvsSet(aEmptySet) - ens, _ := NewOvsSet(aEnumSet) + one := 1 + eso, _ := newOvsSetRaw(TypeString, &one, aEmptySet) + esu, _ := newOvsSetRaw(TypeString, &Unlimited, aEmptySet) + ens, _ := newOvsSetRaw(TypeString, &Unlimited, aEnumSet) m, _ := NewOvsMap(aMap) @@ -89,7 +89,8 @@ func TestOvsToNativeAndNativeToOvs(t *testing.T) { "key3": {GoUUID: aUUID2}, }) - singleStringSet, _ := NewOvsSet([]string{"foo"}) + singleStringSet, _ := newOvsSetRaw(TypeString, &one, []string{"foo"}) + singleUUIDSet, _ := newOvsSetRaw(TypeUUID, &one, UUID{GoUUID: aUUID0}) tests := []struct { name string @@ -284,9 +285,9 @@ func TestOvsToNativeAndNativeToOvs(t *testing.T) { "max": "unlimited" } }`), - input: es, + input: esu, native: aEmptySet, - ovs: es, + ovs: esu, }, { // Enum @@ -390,9 +391,9 @@ func TestOvsToNativeAndNativeToOvs(t *testing.T) { "max": 1 } }`), - input: aSingleUUIDSet, + input: singleUUIDSet, native: &aUUID0, - ovs: aSingleUUIDSet, + ovs: singleUUIDSet, }, { name: "null UUID set with min 0 max 1", @@ -407,9 +408,9 @@ func TestOvsToNativeAndNativeToOvs(t *testing.T) { "max": 1 } }`), - input: es, + input: eso, native: (*string)(nil), - ovs: es, + ovs: eso, }, { name: "A string with min 0 max 1", @@ -457,7 +458,7 @@ func TestOvsToNativeAndNativeToOvs(t *testing.T) { } func TestOvsToNativeErr(t *testing.T) { - as, _ := NewOvsSet([]string{"foo"}) + as, _ := newOvsSetRaw(TypeString, &Unlimited, []string{"foo"}) s, _ := NewOvsMap(map[string]string{"foo": "bar"}) diff --git a/ovsdb/condition_test.go b/ovsdb/condition_test.go index 4eae9cb8..2565e130 100644 --- a/ovsdb/condition_test.go +++ b/ovsdb/condition_test.go @@ -71,7 +71,7 @@ func TestConditionMarshalUnmarshalJSON(t *testing.T) { }, { "test set", - Condition{"foo", ConditionExcludes, OvsSet{GoSet: []interface{}{"foo", "bar", "baz"}}}, + Condition{"foo", ConditionExcludes, OvsSet{GoSet: []interface{}{"foo", "bar", "baz"}, maxSize: 3}}, `[ "foo", "excludes", ["set",["foo", "bar", "baz"]] ]`, false, }, @@ -83,7 +83,7 @@ func TestConditionMarshalUnmarshalJSON(t *testing.T) { }, { "test uuid set", - Condition{"foo", ConditionExcludes, OvsSet{GoSet: []interface{}{UUID{GoUUID: "foo"}, UUID{GoUUID: "bar"}}}}, + Condition{"foo", ConditionExcludes, OvsSet{GoSet: []interface{}{UUID{GoUUID: "foo"}, UUID{GoUUID: "bar"}}, maxSize: 2}}, `[ "foo", "excludes", ["set",[["named-uuid", "foo"], ["named-uuid", "bar"]]] ]`, false, }, diff --git a/ovsdb/encoding_test.go b/ovsdb/encoding_test.go index 2ea6c60b..fc537574 100644 --- a/ovsdb/encoding_test.go +++ b/ovsdb/encoding_test.go @@ -63,108 +63,128 @@ func TestSet(t *testing.T) { var y *string tests := []struct { name string + setType string input interface{} expected string }{ { "empty set", + TypeString, []string{}, `["set",[]]`, }, { "string", + TypeString, `aa`, `"aa"`, }, { "bool", + TypeBoolean, false, `false`, }, { "float 64", + TypeReal, float64(10), `10`, }, { "float", + TypeReal, 10.2, `10.2`, }, { "string slice", + TypeString, []string{`aa`}, `"aa"`, }, { "string array", + TypeString, [1]string{`aa`}, `"aa"`, }, { "string slice with multiple elements", + TypeString, []string{`aa`, `bb`}, `["set",["aa","bb"]]`, }, { "string array multiple elements", + TypeString, [2]string{`aa`, `bb`}, `["set",["aa","bb"]]`, }, { "float slice", + TypeString, []float64{10.2, 15.4}, `["set",[10.2,15.4]]`, }, { "empty uuid", + TypeUUID, []UUID{}, `["set",[]]`, }, { "uuid", + TypeUUID, UUID{GoUUID: `aa`}, `["named-uuid","aa"]`, }, { "uuid slice single element", + TypeUUID, []UUID{{GoUUID: `aa`}}, `["named-uuid","aa"]`, }, { "uuid slice multiple elements", + TypeUUID, []UUID{{GoUUID: `aa`}, {GoUUID: `bb`}}, `["set",[["named-uuid","aa"],["named-uuid","bb"]]]`, }, { "valid uuid", + TypeUUID, validUUID0, fmt.Sprintf(`["uuid","%v"]`, validUUIDStr0), }, { "valid uuid set single element", + TypeUUID, []UUID{validUUID0}, fmt.Sprintf(`["uuid","%v"]`, validUUIDStr0), }, { "valid uuid set multiple elements", + TypeUUID, []UUID{validUUID0, validUUID1}, fmt.Sprintf(`["set",[["uuid","%v"],["uuid","%v"]]]`, validUUIDStr0, validUUIDStr1), }, { name: "nil pointer of valid *int type", + setType: TypeInteger, input: x, expected: `["set",[]]`, }, { name: "nil pointer of valid *string type", + setType: TypeString, input: y, expected: `["set",[]]`, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - set, err := NewOvsSet(tt.input) + set, err := newOvsSetRaw(tt.setType, &Unlimited, tt.input) assert.Nil(t, err) jsonStr, err := json.Marshal(set) assert.Nil(t, err) diff --git a/ovsdb/mutation_test.go b/ovsdb/mutation_test.go index a70c835b..4b5ecea7 100644 --- a/ovsdb/mutation_test.go +++ b/ovsdb/mutation_test.go @@ -65,7 +65,7 @@ func TestMutationMarshalUnmarshalJSON(t *testing.T) { }, { "test set", - Mutation{"foo", MutateOperationInsert, OvsSet{GoSet: []interface{}{"foo", "bar", "baz"}}}, + Mutation{"foo", MutateOperationInsert, OvsSet{GoSet: []interface{}{"foo", "bar", "baz"}, maxSize: 3}}, `[ "foo", "insert", ["set",["foo", "bar", "baz"]] ]`, false, }, @@ -77,7 +77,7 @@ func TestMutationMarshalUnmarshalJSON(t *testing.T) { }, { "test uuid set", - Mutation{"foo", MutateOperationInsert, OvsSet{GoSet: []interface{}{UUID{GoUUID: "foo"}, UUID{GoUUID: "bar"}}}}, + Mutation{"foo", MutateOperationInsert, OvsSet{GoSet: []interface{}{UUID{GoUUID: "foo"}, UUID{GoUUID: "bar"}}, maxSize: 2}}, `[ "foo", "insert", ["set",[["named-uuid", "foo"], ["named-uuid", "bar"]]] ]`, false, }, diff --git a/ovsdb/notation.go b/ovsdb/notation.go index 5d415012..9fe71c8a 100644 --- a/ovsdb/notation.go +++ b/ovsdb/notation.go @@ -116,6 +116,10 @@ func ovsSliceToGoNotation(val interface{}) (interface{}, error) { case "set": var oSet OvsSet err = json.Unmarshal(bsliced, &oSet) + if err != nil { + return oSet, err + } + oSet.maxSize = len(oSet.GoSet) return oSet, err case "map": var oMap OvsMap diff --git a/ovsdb/notation_test.go b/ovsdb/notation_test.go index ff183f4a..47ec208e 100644 --- a/ovsdb/notation_test.go +++ b/ovsdb/notation_test.go @@ -73,7 +73,7 @@ func TestOpRowsSerialization(t *testing.T) { func TestValidateOvsSet(t *testing.T) { goSlice := []int{1, 2, 3, 4} - oSet, err := NewOvsSet(goSlice) + oSet, err := newOvsSetRaw(TypeInteger, &Unlimited, goSlice) if err != nil { t.Error("Error creating OvsSet ", err) } @@ -86,7 +86,7 @@ func TestValidateOvsSet(t *testing.T) { t.Error("Expected: ", expected, "Got", string(data)) } // Negative condition test - oSet, err = NewOvsSet(struct{ foo string }{}) + oSet, err = newOvsSetRaw(TypeString, &Unlimited, struct{ foo string }{}) if err == nil { t.Error("OvsSet must fail for anything other than Slices and atomic types") t.Error("Got", oSet) @@ -215,13 +215,13 @@ func TestOvsSliceToGoNotation(t *testing.T) { { "set", []interface{}{"set", []interface{}{"foo", "bar", "baz"}}, - OvsSet{GoSet: []interface{}{"foo", "bar", "baz"}}, + OvsSet{GoSet: []interface{}{"foo", "bar", "baz"}, maxSize: 3}, false, }, { "uuid set", []interface{}{"set", []interface{}{[]interface{}{"named-uuid", "foo"}, []interface{}{"named-uuid", "bar"}}}, - OvsSet{GoSet: []interface{}{UUID{GoUUID: "foo"}, UUID{GoUUID: "bar"}}}, + OvsSet{GoSet: []interface{}{UUID{GoUUID: "foo"}, UUID{GoUUID: "bar"}}, maxSize: 2}, false, }, { diff --git a/ovsdb/schema.go b/ovsdb/schema.go index cf80aa50..0659a7c1 100644 --- a/ovsdb/schema.go +++ b/ovsdb/schema.go @@ -363,7 +363,7 @@ func (b BaseType) MarshalJSON() ([]byte, error) { RefType: b.refType, } if len(b.Enum) > 0 { - set, err := NewOvsSet(b.Enum) + set, err := newOvsSetRaw(b.Type, &Unlimited, b.Enum) if err != nil { return nil, err } @@ -437,6 +437,7 @@ func (c *ColumnType) UnmarshalJSON(data []byte) error { default: c.max = nil } + return nil } diff --git a/ovsdb/set.go b/ovsdb/set.go index d5fab1ec..e238ffcf 100644 --- a/ovsdb/set.go +++ b/ovsdb/set.go @@ -12,20 +12,48 @@ import ( // a 2-element JSON array that represents a database set value. The // first element of the array must be the string "set", and the // second element must be an array of zero or more s giving the -// values in the set. All of the s must have the same type. +// values in the set. All of the s must have the same type, and all +// values must be unique within the set. type OvsSet struct { - GoSet []interface{} + GoSet []interface{} + maxSize int // <0 means unlimited } -// NewOvsSet creates a new OVSDB style set from a Go interface (object) -func NewOvsSet(obj interface{}) (OvsSet, error) { +// NewOvsSetRaw creates a new OVSDB style set from a Go interface (object) +// using the column schema to determine the element type and max set length +func NewOvsSet(column *ColumnSchema, obj interface{}) (OvsSet, error) { + return newOvsSetRaw(column.TypeObj.Key.Type, column.TypeObj.max, obj) +} + +func getUUID(val interface{}) (UUID, error) { + uuid, ok := val.(UUID) + if ok { + return uuid, nil + } + str, ok := val.(string) + if ok { + return UUID{GoUUID: str}, nil + } + return UUID{}, fmt.Errorf("expected UUID or string but got %T", val) +} + +// newOvsSetRaw creates a new OVSDB style set from a Go interface (object) +func newOvsSetRaw(keyType string, maxSizePtr *int, obj interface{}) (OvsSet, error) { + maxSize := -1 + if maxSizePtr != nil { + maxSize = *maxSizePtr + } + ovsSet := make([]interface{}, 0) var v reflect.Value if reflect.TypeOf(obj).Kind() == reflect.Ptr { v = reflect.ValueOf(obj).Elem() if v.Kind() == reflect.Invalid { // must be a nil pointer, so just return an empty set - return OvsSet{ovsSet}, nil + return OvsSet{ + GoSet: ovsSet, + maxSize: maxSize, + }, nil } } else { v = reflect.ValueOf(obj) @@ -34,10 +62,27 @@ func NewOvsSet(obj interface{}) (OvsSet, error) { switch v.Kind() { case reflect.Slice, reflect.Array: for i := 0; i < v.Len(); i++ { - ovsSet = append(ovsSet, v.Index(i).Interface()) + if keyType == TypeUUID { + uuid, err := getUUID(v.Index(i).Interface()) + if err != nil { + return OvsSet{}, err + } + ovsSet = append(ovsSet, uuid) + } else { + ovsSet = append(ovsSet, v.Index(i).Interface()) + } } - case reflect.String, - reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, + case reflect.String: + if keyType == TypeUUID { + uuid, err := getUUID(v.Interface()) + if err != nil { + return OvsSet{}, err + } + ovsSet = append(ovsSet, uuid) + } else { + ovsSet = append(ovsSet, v.Interface()) + } + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Float32, reflect.Float64, reflect.Bool: ovsSet = append(ovsSet, v.Interface()) @@ -50,11 +95,17 @@ func NewOvsSet(obj interface{}) (OvsSet, error) { default: return OvsSet{}, fmt.Errorf("ovsset supports only go slice/string/numbers/uuid or pointers to those types") } - return OvsSet{ovsSet}, nil + return OvsSet{ + GoSet: ovsSet, + maxSize: maxSize, + }, nil } // MarshalJSON wil marshal an OVSDB style Set in to a JSON byte array func (o OvsSet) MarshalJSON() ([]byte, error) { + if o.maxSize >= 0 && len(o.GoSet) > o.maxSize { + return nil, fmt.Errorf("OvsSet max size is %d but has %d elements", o.maxSize, len(o.GoSet)) + } switch l := len(o.GoSet); { case l == 1: return json.Marshal(o.GoSet[0]) diff --git a/ovsdb/set_test.go b/ovsdb/set_test.go index 12857a87..2de29f5d 100644 --- a/ovsdb/set_test.go +++ b/ovsdb/set_test.go @@ -19,8 +19,8 @@ var testUUIDs = []string{ "7e191fdb-228d-4bf3-9db4-883c8705ac7e", } -func benchmarkSetMarshalJSON(s interface{}, b *testing.B) { - testSet, err := NewOvsSet(s) +func benchmarkSetMarshalJSON(keyType string, s interface{}, b *testing.B) { + testSet, err := newOvsSetRaw(keyType, &Unlimited, s) if err != nil { b.Fatal(err) } @@ -31,60 +31,60 @@ func benchmarkSetMarshalJSON(s interface{}, b *testing.B) { } } } -func BenchmarkSetMarshalJSONString1(b *testing.B) { benchmarkSetMarshalJSON("foo", b) } +func BenchmarkSetMarshalJSONString1(b *testing.B) { benchmarkSetMarshalJSON(TypeString, "foo", b) } func BenchmarkSetMarshalJSONString2(b *testing.B) { - benchmarkSetMarshalJSON([]string{"foo", "bar"}, b) + benchmarkSetMarshalJSON(TypeString, []string{"foo", "bar"}, b) } func BenchmarkSetMarshalJSONString3(b *testing.B) { - benchmarkSetMarshalJSON([]string{"foo", "bar", "baz"}, b) + benchmarkSetMarshalJSON(TypeString, []string{"foo", "bar", "baz"}, b) } func BenchmarkSetMarshalJSONString5(b *testing.B) { - benchmarkSetMarshalJSON([]string{"foo", "bar", "baz", "quux", "foofoo"}, b) + benchmarkSetMarshalJSON(TypeString, []string{"foo", "bar", "baz", "quux", "foofoo"}, b) } func BenchmarkSetMarshalJSONString8(b *testing.B) { - benchmarkSetMarshalJSON([]string{"foo", "bar", "baz", "quux", "foofoo", "foobar", "foobaz", "fooquux"}, b) + benchmarkSetMarshalJSON(TypeString, []string{"foo", "bar", "baz", "quux", "foofoo", "foobar", "foobaz", "fooquux"}, b) } -func BenchmarkSetMarshalJSONInt1(b *testing.B) { benchmarkSetMarshalJSON(1, b) } +func BenchmarkSetMarshalJSONInt1(b *testing.B) { benchmarkSetMarshalJSON(TypeInteger, 1, b) } func BenchmarkSetMarshalJSONInt2(b *testing.B) { - benchmarkSetMarshalJSON([]int{1, 2}, b) + benchmarkSetMarshalJSON(TypeInteger, []int{1, 2}, b) } func BenchmarkSetMarshalJSONInt3(b *testing.B) { - benchmarkSetMarshalJSON([]int{1, 2, 3}, b) + benchmarkSetMarshalJSON(TypeInteger, []int{1, 2, 3}, b) } func BenchmarkSetMarshalJSONInt5(b *testing.B) { - benchmarkSetMarshalJSON([]int{1, 2, 3, 4, 5}, b) + benchmarkSetMarshalJSON(TypeInteger, []int{1, 2, 3, 4, 5}, b) } func BenchmarkSetMarshalJSONInt8(b *testing.B) { - benchmarkSetMarshalJSON([]int{1, 2, 3, 4, 5, 6, 7, 8}, b) + benchmarkSetMarshalJSON(TypeInteger, []int{1, 2, 3, 4, 5, 6, 7, 8}, b) } -func BenchmarkSetMarshalJSONFloat1(b *testing.B) { benchmarkSetMarshalJSON(1.0, b) } +func BenchmarkSetMarshalJSONFloat1(b *testing.B) { benchmarkSetMarshalJSON(TypeReal, 1.0, b) } func BenchmarkSetMarshalJSONFloat2(b *testing.B) { - benchmarkSetMarshalJSON([]int{1.0, 2.0}, b) + benchmarkSetMarshalJSON(TypeReal, []int{1.0, 2.0}, b) } func BenchmarkSetMarshalJSONFloat3(b *testing.B) { - benchmarkSetMarshalJSON([]int{1.0, 2.0, 3.0}, b) + benchmarkSetMarshalJSON(TypeReal, []int{1.0, 2.0, 3.0}, b) } func BenchmarkSetMarshalJSONFloat5(b *testing.B) { - benchmarkSetMarshalJSON([]int{1.0, 2.0, 3.0, 4.0, 5.0}, b) + benchmarkSetMarshalJSON(TypeReal, []int{1.0, 2.0, 3.0, 4.0, 5.0}, b) } func BenchmarkSetMarshalJSONFloat8(b *testing.B) { - benchmarkSetMarshalJSON([]int{1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0}, b) + benchmarkSetMarshalJSON(TypeReal, []int{1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0}, b) } -func BenchmarkSetMarshalJSONUUID1(b *testing.B) { benchmarkSetMarshalJSON(testUUIDs[0], b) } +func BenchmarkSetMarshalJSONUUID1(b *testing.B) { benchmarkSetMarshalJSON(TypeUUID, testUUIDs[0], b) } func BenchmarkSetMarshalJSONUUID2(b *testing.B) { - benchmarkSetMarshalJSON(testUUIDs[0:2], b) + benchmarkSetMarshalJSON(TypeUUID, testUUIDs[0:2], b) } func BenchmarkSetMarshalJSONUUID3(b *testing.B) { - benchmarkSetMarshalJSON(testUUIDs[0:3], b) + benchmarkSetMarshalJSON(TypeUUID, testUUIDs[0:3], b) } func BenchmarkSetMarshalJSONUUID5(b *testing.B) { - benchmarkSetMarshalJSON(testUUIDs[0:5], b) + benchmarkSetMarshalJSON(TypeUUID, testUUIDs[0:5], b) } func BenchmarkSetMarshalJSONUUID8(b *testing.B) { - benchmarkSetMarshalJSON(testUUIDs, b) + benchmarkSetMarshalJSON(TypeUUID, testUUIDs, b) } func benchmarkSetUnmarshalJSON(data []byte, b *testing.B) { diff --git a/ovsdb/updates2_test.go b/ovsdb/updates2_test.go index 985e73df..3b7cb182 100644 --- a/ovsdb/updates2_test.go +++ b/ovsdb/updates2_test.go @@ -7,9 +7,9 @@ import ( ) func TestAddRowUpdate2Merge(t *testing.T) { - portsA, _ := NewOvsSet([]interface{}{"portA"}) - portsC, _ := NewOvsSet([]interface{}{"portC"}) - portsAC, _ := NewOvsSet([]interface{}{"portA", "portC"}) + portsA, _ := newOvsSetRaw(TypeString, &Unlimited, []interface{}{"portA"}) + portsC, _ := newOvsSetRaw(TypeString, &Unlimited, []interface{}{"portC"}) + portsAC, _ := newOvsSetRaw(TypeString, &Unlimited, []interface{}{"portA", "portC"}) mapPortsA, _ := NewOvsMap(map[interface{}]interface{}{"A": "portA"}) mapPortsC, _ := NewOvsMap(map[interface{}]interface{}{"C": "portC"}) diff --git a/server/transact.go b/server/transact.go index 7bfacf67..fedb70de 100644 --- a/server/transact.go +++ b/server/transact.go @@ -673,7 +673,7 @@ func diff(column *ovsdb.ColumnSchema, a interface{}, b interface{}) interface{} } } if len(c) > 0 { - cSet, _ := ovsdb.NewOvsSet(c) + cSet, _ := ovsdb.NewOvsSet(column, c) return cSet } return nil diff --git a/server/transact_test.go b/server/transact_test.go index a962c05c..cd2369bb 100644 --- a/server/transact_test.go +++ b/server/transact_test.go @@ -2,6 +2,7 @@ package server import ( "encoding/json" + "fmt" "testing" "time" @@ -334,7 +335,8 @@ func TestMutateOp(t *testing.T) { err = o.db.Commit("Open_vSwitch", uuid.New(), gotUpdate) require.NoError(t, err) - bridgeSet, err := ovsdb.NewOvsSet([]ovsdb.UUID{{GoUUID: bridgeUUID}}) + bridgesSchema := info.Metadata.TableSchema.Column("bridges") + bridgeSet, err := ovsdb.NewOvsSet(bridgesSchema, []ovsdb.UUID{{GoUUID: bridgeUUID}}) assert.Nil(t, err) assert.Equal(t, ovsdb.TableUpdates2{ "Open_vSwitch": ovsdb.TableUpdate2{ @@ -355,7 +357,8 @@ func TestMutateOp(t *testing.T) { }, }, gotUpdate) - keyDelete, err := ovsdb.NewOvsSet([]string{"foo"}) + extIDSchema := bridgeInfo.Metadata.TableSchema.Column("external_ids") + keyDelete, err := ovsdb.NewOvsSet(extIDSchema, []string{"foo"}) assert.Nil(t, err) keyValueDelete, err := ovsdb.NewOvsMap(map[string]string{"baz": "quux"}) assert.Nil(t, err) @@ -386,16 +389,41 @@ func TestMutateOp(t *testing.T) { assert.Equal(t, newExternalIds, gotNew["external_ids"]) } +func testOvsSet(t *testing.T, keyType string, maxSize int, set interface{}) ovsdb.OvsSet { + maxStr := `"unlimited"` + if maxSize > 0 { + maxStr = fmt.Sprintf("%d", maxSize) + } + colSchema := fmt.Sprintf(`{ + "type": { + "key": {"type": "%s"}, + "min": 0, + "max": %s + } + }`, keyType, maxStr) + + var column ovsdb.ColumnSchema + err := json.Unmarshal([]byte(colSchema), &column) + assert.Nil(t, err) + + oSet, err := ovsdb.NewOvsSet(&column, set) + assert.Nil(t, err) + return oSet +} + func TestDiff(t *testing.T) { - originSet, _ := ovsdb.NewOvsSet([]interface{}{"foo"}) + originSetOne := testOvsSet(t, ovsdb.TypeString, 1, []interface{}{"foo"}) + originSetTwo := testOvsSet(t, ovsdb.TypeString, 2, []interface{}{"foo"}) - originSetAdd, _ := ovsdb.NewOvsSet([]interface{}{"bar"}) - setAddDiffSingleValue := originSetAdd - setAddDiffMultipleValues, _ := ovsdb.NewOvsSet([]interface{}{"foo", "bar"}) + originSetAddOne := testOvsSet(t, ovsdb.TypeString, ovsdb.Unlimited, []interface{}{"bar"}) + setAddDiffSingleValue := originSetAddOne + originSetAddTwo := testOvsSet(t, ovsdb.TypeString, 2, []interface{}{"bar"}) + setAddDiffMultipleValues := testOvsSet(t, ovsdb.TypeString, 2, []interface{}{"foo", "bar"}) - originSetDel, _ := ovsdb.NewOvsSet([]interface{}{}) - setDelDiffSingleValue := originSetDel - setDelDiffMultipleValues, _ := ovsdb.NewOvsSet([]interface{}{"foo"}) + originSetDelOne := testOvsSet(t, ovsdb.TypeString, ovsdb.Unlimited, []interface{}{}) + setDelDiffSingleValue := originSetDelOne + originSetDelTwo := testOvsSet(t, ovsdb.TypeString, 2, []interface{}{}) + setDelDiffMultipleValues := testOvsSet(t, ovsdb.TypeString, 2, []interface{}{"foo"}) originMap, _ := ovsdb.NewOvsMap(map[interface{}]interface{}{"foo": "bar"}) @@ -444,43 +472,43 @@ func TestDiff(t *testing.T) { { "add to set, max == 1", &singleValueSetSchema, - originSet, - originSetAdd, + originSetOne, + originSetAddOne, setAddDiffSingleValue, }, { "add to set, max > 1", &multipleValueSetSchema, - originSet, - originSetAdd, + originSetTwo, + originSetAddTwo, setAddDiffMultipleValues, }, { "delete from set, max == 1", &singleValueSetSchema, - originSet, - originSetDel, + originSetOne, + originSetDelOne, setDelDiffSingleValue, }, { "delete from set, max > 1", &multipleValueSetSchema, - originSet, - originSetDel, + originSetTwo, + originSetDelTwo, setDelDiffMultipleValues, }, { "noop set, max == 1", &singleValueSetSchema, - originSet, - originSet, - originSet, + originSetOne, + originSetOne, + originSetOne, }, { "noop set, max > 1", &multipleValueSetSchema, - originSet, - originSet, + originSetTwo, + originSetTwo, nil, }, { @@ -622,8 +650,8 @@ func TestOvsdbServerUpdate(t *testing.T) { err = ovsDB.Commit("Open_vSwitch", uuid.New(), updates) assert.NoError(t, err) - halloween, _ := ovsdb.NewOvsSet([]string{"halloween"}) - emptySet, _ := ovsdb.NewOvsSet([]string{}) + halloween := testOvsSet(t, ovsdb.TypeString, 1, []string{"halloween"}) + emptySet := testOvsSet(t, ovsdb.TypeString, 1, []string{}) tests := []struct { name string row ovsdb.Row @@ -729,8 +757,7 @@ func TestMultipleOps(t *testing.T) { var ops []ovsdb.Operation var op ovsdb.Operation - portA, err := ovsdb.NewOvsSet([]ovsdb.UUID{{GoUUID: "portA"}}) - require.NoError(t, err) + portA := testOvsSet(t, ovsdb.TypeUUID, ovsdb.Unlimited, []ovsdb.UUID{{GoUUID: "portA"}}) op = ovsdb.Operation{ Table: "Bridge", Where: []ovsdb.Condition{ @@ -743,8 +770,7 @@ func TestMultipleOps(t *testing.T) { } ops = append(ops, op) - portBC, err := ovsdb.NewOvsSet([]ovsdb.UUID{{GoUUID: "portB"}, {GoUUID: "portC"}}) - require.NoError(t, err) + portBC := testOvsSet(t, ovsdb.TypeUUID, ovsdb.Unlimited, []ovsdb.UUID{{GoUUID: "portB"}, {GoUUID: "portC"}}) op2 := ovsdb.Operation{ Table: "Bridge", Where: []ovsdb.Condition{ @@ -766,14 +792,9 @@ func TestMultipleOps(t *testing.T) { err = o.db.Commit("Open_vSwitch", uuid.New(), updates) require.NoError(t, err) - modifiedPorts, err := ovsdb.NewOvsSet([]ovsdb.UUID{{GoUUID: "portA"}, {GoUUID: "portB"}, {GoUUID: "portC"}}) - assert.Nil(t, err) - - oldPorts, err := ovsdb.NewOvsSet([]ovsdb.UUID{{GoUUID: "port1"}, {GoUUID: "port10"}}) - assert.Nil(t, err) - - newPorts, err := ovsdb.NewOvsSet([]ovsdb.UUID{{GoUUID: "port1"}, {GoUUID: "port10"}, {GoUUID: "portA"}, {GoUUID: "portB"}, {GoUUID: "portC"}}) - assert.Nil(t, err) + modifiedPorts := testOvsSet(t, ovsdb.TypeUUID, ovsdb.Unlimited, []ovsdb.UUID{{GoUUID: "portA"}, {GoUUID: "portB"}, {GoUUID: "portC"}}) + oldPorts := testOvsSet(t, ovsdb.TypeUUID, ovsdb.Unlimited, []ovsdb.UUID{{GoUUID: "port1"}, {GoUUID: "port10"}}) + newPorts := testOvsSet(t, ovsdb.TypeUUID, ovsdb.Unlimited, []ovsdb.UUID{{GoUUID: "port1"}, {GoUUID: "port10"}, {GoUUID: "portA"}, {GoUUID: "portB"}, {GoUUID: "portC"}}) assert.Equal(t, ovsdb.TableUpdates2{ "Bridge": ovsdb.TableUpdate2{