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

Add Stronger Types for Enums and Improve Set Encoding #151

Closed
wants to merge 3 commits into from
Closed
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
91 changes: 49 additions & 42 deletions client/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,14 @@ import (
"github.com/ovn-org/libovsdb/model"
"github.com/ovn-org/libovsdb/ovsdb"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

var (
trueVal = true
falseVal = false
one = 1
six = 6
)

func TestAPIListSimple(t *testing.T) {
Expand Down Expand Up @@ -222,25 +230,25 @@ func TestAPIListFields(t *testing.T) {
UUID: aUUID0,
Name: "lsp0",
ExternalIds: map[string]string{"foo": "bar"},
Enabled: []bool{true},
Enabled: &trueVal,
},
&testLogicalSwitchPort{
UUID: aUUID1,
Name: "magiclsp1",
ExternalIds: map[string]string{"foo": "baz"},
Enabled: []bool{false},
Enabled: &falseVal,
},
&testLogicalSwitchPort{
UUID: aUUID2,
Name: "lsp2",
ExternalIds: map[string]string{"unique": "id"},
Enabled: []bool{false},
Enabled: &falseVal,
},
&testLogicalSwitchPort{
UUID: aUUID3,
Name: "magiclsp2",
ExternalIds: map[string]string{"foo": "baz"},
Enabled: []bool{true},
Enabled: &trueVal,
},
}
lspcache := map[string]model.Model{}
Expand Down Expand Up @@ -624,22 +632,22 @@ func TestAPIMutate(t *testing.T) {
Name: "lsp0",
Type: "someType",
ExternalIds: map[string]string{"foo": "bar"},
Enabled: []bool{true},
Tag: []int{1},
Enabled: &trueVal,
Tag: &one,
},
aUUID1: &testLogicalSwitchPort{
UUID: aUUID1,
Name: "lsp1",
Type: "someType",
ExternalIds: map[string]string{"foo": "baz"},
Tag: []int{1},
Tag: &one,
},
aUUID2: &testLogicalSwitchPort{
UUID: aUUID2,
Name: "lsp2",
Type: "someOtherType",
ExternalIds: map[string]string{"foo": "baz"},
Tag: []int{1},
Tag: &one,
},
}
testData := cache.Data{
Expand All @@ -648,7 +656,6 @@ func TestAPIMutate(t *testing.T) {
tcache := apiTestCache(t, testData)

testObj := testLogicalSwitchPort{}

test := []struct {
name string
condition func(API) ConditionalAPI
Expand All @@ -667,16 +674,16 @@ func TestAPIMutate(t *testing.T) {
},
mutations: []model.Mutation{
{
Field: &testObj.Tag,
Field: &testObj.Addresses,
Mutator: ovsdb.MutateOperationInsert,
Value: []int{5},
Value: []string{"1.1.1.1"},
},
},
result: []ovsdb.Operation{
{
Op: ovsdb.OperationMutate,
Table: "Logical_Switch_Port",
Mutations: []ovsdb.Mutation{{Column: "tag", Mutator: ovsdb.MutateOperationInsert, Value: testOvsSet(t, []int{5})}},
Mutations: []ovsdb.Mutation{{Column: "addresses", Mutator: ovsdb.MutateOperationInsert, Value: testOvsSet(t, []string{"1.1.1.1"})}},
Where: []ovsdb.Condition{{Column: "_uuid", Function: ovsdb.ConditionEqual, Value: ovsdb.UUID{GoUUID: aUUID0}}},
},
},
Expand Down Expand Up @@ -777,9 +784,9 @@ func TestAPIMutate(t *testing.T) {
cond := tt.condition(api)
ops, err := cond.Mutate(&testObj, tt.mutations...)
if tt.err {
assert.NotNil(t, err)
require.Error(t, err)
} else {
assert.Nil(t, err)
require.Nil(t, err)
assert.ElementsMatchf(t, tt.result, ops, "ovsdb.Operations should match")
}
})
Expand All @@ -793,23 +800,23 @@ func TestAPIUpdate(t *testing.T) {
Name: "lsp0",
Type: "someType",
ExternalIds: map[string]string{"foo": "bar"},
Enabled: []bool{true},
Tag: []int{1},
Enabled: &trueVal,
Tag: &one,
},
aUUID1: &testLogicalSwitchPort{
UUID: aUUID1,
Name: "lsp1",
Type: "someType",
ExternalIds: map[string]string{"foo": "baz"},
Tag: []int{1},
Enabled: []bool{true},
Tag: &one,
Enabled: &trueVal,
},
aUUID2: &testLogicalSwitchPort{
UUID: aUUID2,
Name: "lsp2",
Type: "someOtherType",
ExternalIds: map[string]string{"foo": "baz"},
Tag: []int{1},
Tag: &one,
},
}
testData := cache.Data{
Expand All @@ -836,7 +843,7 @@ func TestAPIUpdate(t *testing.T) {
},
prepare: func(t *testLogicalSwitchPort) {
t.Type = "somethingElse"
t.Tag = []int{6}
t.Tag = &six
},
result: []ovsdb.Operation{
{
Expand All @@ -857,7 +864,7 @@ func TestAPIUpdate(t *testing.T) {
},
prepare: func(t *testLogicalSwitchPort) {
t.Type = "somethingElse"
t.Tag = []int{6}
t.Tag = &six
},
result: []ovsdb.Operation{
{
Expand All @@ -874,7 +881,7 @@ func TestAPIUpdate(t *testing.T) {
condition: func(a API) ConditionalAPI {
t := testLogicalSwitchPort{
Type: "sometype",
Enabled: []bool{true},
Enabled: &trueVal,
}
return a.Where(&t, model.Condition{
Field: &t.Type,
Expand All @@ -883,7 +890,7 @@ func TestAPIUpdate(t *testing.T) {
})
},
prepare: func(t *testLogicalSwitchPort) {
t.Tag = []int{6}
t.Tag = &six
},
result: []ovsdb.Operation{
{
Expand All @@ -908,11 +915,11 @@ func TestAPIUpdate(t *testing.T) {
model.Condition{
Field: &t.Enabled,
Function: ovsdb.ConditionIncludes,
Value: []bool{true},
Value: &trueVal,
})
},
prepare: func(t *testLogicalSwitchPort) {
t.Tag = []int{6}
t.Tag = &six
},
result: []ovsdb.Operation{
{
Expand All @@ -925,7 +932,7 @@ func TestAPIUpdate(t *testing.T) {
Op: ovsdb.OperationUpdate,
Table: "Logical_Switch_Port",
Row: tagRow,
Where: []ovsdb.Condition{{Column: "enabled", Function: ovsdb.ConditionIncludes, Value: testOvsSet(t, []bool{true})}},
Where: []ovsdb.Condition{{Column: "enabled", Function: ovsdb.ConditionIncludes, Value: testOvsSet(t, &trueVal)}},
},
},
err: false,
Expand All @@ -943,11 +950,11 @@ func TestAPIUpdate(t *testing.T) {
model.Condition{
Field: &t.Enabled,
Function: ovsdb.ConditionIncludes,
Value: []bool{true},
Value: &trueVal,
})
},
prepare: func(t *testLogicalSwitchPort) {
t.Tag = []int{6}
t.Tag = &six
},
result: []ovsdb.Operation{
{
Expand All @@ -956,7 +963,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, []bool{true})},
{Column: "enabled", Function: ovsdb.ConditionIncludes, Value: testOvsSet(t, &trueVal)},
},
},
},
Expand All @@ -967,7 +974,7 @@ func TestAPIUpdate(t *testing.T) {
condition: func(a API) ConditionalAPI {
t := testLogicalSwitchPort{
Type: "sometype",
Enabled: []bool{true},
Enabled: &trueVal,
}
return a.Where(&t, model.Condition{
Field: &t.Type,
Expand All @@ -976,7 +983,7 @@ func TestAPIUpdate(t *testing.T) {
})
},
prepare: func(t *testLogicalSwitchPort) {
t.Tag = []int{6}
t.Tag = &six
},
result: []ovsdb.Operation{
{
Expand All @@ -992,12 +999,12 @@ func TestAPIUpdate(t *testing.T) {
name: "select multiple by predicate change multiple field",
condition: func(a API) ConditionalAPI {
return a.WhereCache(func(t *testLogicalSwitchPort) bool {
return t.Enabled != nil && t.Enabled[0] == true
return t.Enabled != nil && *t.Enabled == true
})
},
prepare: func(t *testLogicalSwitchPort) {
t.Type = "somethingElse"
t.Tag = []int{6}
t.Tag = &six
},
result: []ovsdb.Operation{
{
Expand Down Expand Up @@ -1041,23 +1048,23 @@ func TestAPIDelete(t *testing.T) {
Name: "lsp0",
Type: "someType",
ExternalIds: map[string]string{"foo": "bar"},
Enabled: []bool{true},
Tag: []int{1},
Enabled: &trueVal,
Tag: &one,
},
aUUID1: &testLogicalSwitchPort{
UUID: aUUID1,
Name: "lsp1",
Type: "someType",
ExternalIds: map[string]string{"foo": "baz"},
Tag: []int{1},
Enabled: []bool{true},
Tag: &one,
Enabled: &trueVal,
},
aUUID2: &testLogicalSwitchPort{
UUID: aUUID2,
Name: "lsp2",
Type: "someOtherType",
ExternalIds: map[string]string{"foo": "baz"},
Tag: []int{1},
Tag: &one,
},
}
testData := cache.Data{
Expand Down Expand Up @@ -1107,7 +1114,7 @@ func TestAPIDelete(t *testing.T) {
name: "select by field equality",
condition: func(a API) ConditionalAPI {
t := testLogicalSwitchPort{
Enabled: []bool{true},
Enabled: &trueVal,
}
return a.Where(&t, model.Condition{
Field: &t.Type,
Expand All @@ -1128,7 +1135,7 @@ func TestAPIDelete(t *testing.T) {
name: "select any by field ",
condition: func(a API) ConditionalAPI {
t := testLogicalSwitchPort{
Enabled: []bool{true},
Enabled: &trueVal,
}
return a.Where(&t,
model.Condition{
Expand Down Expand Up @@ -1159,7 +1166,7 @@ func TestAPIDelete(t *testing.T) {
name: "select all by field ",
condition: func(a API) ConditionalAPI {
t := testLogicalSwitchPort{
Enabled: []bool{true},
Enabled: &trueVal,
}
return a.WhereAll(&t,
model.Condition{
Expand Down Expand Up @@ -1188,7 +1195,7 @@ func TestAPIDelete(t *testing.T) {
name: "select multiple by predicate",
condition: func(a API) ConditionalAPI {
return a.WhereCache(func(t *testLogicalSwitchPort) bool {
return t.Enabled != nil && t.Enabled[0] == true
return t.Enabled != nil && *t.Enabled == true
})
},
result: []ovsdb.Operation{
Expand Down
18 changes: 9 additions & 9 deletions client/api_test_model.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,21 +131,21 @@ func (*testLogicalSwitch) Table() string {
//LogicalSwitchPort struct defines an object in Logical_Switch_Port table
type testLogicalSwitchPort struct {
UUID string `ovsdb:"_uuid"`
Up []bool `ovsdb:"up"`
Dhcpv4Options []string `ovsdb:"dhcpv4_options"`
Up *bool `ovsdb:"up"`
Dhcpv4Options *string `ovsdb:"dhcpv4_options"`
Name string `ovsdb:"name"`
DynamicAddresses []string `ovsdb:"dynamic_addresses"`
HaChassisGroup []string `ovsdb:"ha_chassis_group"`
DynamicAddresses *string `ovsdb:"dynamic_addresses"`
HaChassisGroup *string `ovsdb:"ha_chassis_group"`
Options map[string]string `ovsdb:"options"`
Enabled []bool `ovsdb:"enabled"`
Enabled *bool `ovsdb:"enabled"`
Addresses []string `ovsdb:"addresses"`
Dhcpv6Options []string `ovsdb:"dhcpv6_options"`
TagRequest []int `ovsdb:"tag_request"`
Tag []int `ovsdb:"tag"`
Dhcpv6Options *string `ovsdb:"dhcpv6_options"`
TagRequest *int `ovsdb:"tag_request"`
Tag *int `ovsdb:"tag"`
PortSecurity []string `ovsdb:"port_security"`
ExternalIds map[string]string `ovsdb:"external_ids"`
Type string `ovsdb:"type"`
ParentName []string `ovsdb:"parent_name"`
ParentName *string `ovsdb:"parent_name"`
}

// Table returns the table name. It's part of the Model interface
Expand Down
Loading