From 754b46b053cc0379a265a3c536d8349bb772983f Mon Sep 17 00:00:00 2001 From: Yan Zhu Date: Mon, 18 Sep 2023 18:38:45 +0800 Subject: [PATCH] refactor WhereCache func to import performance Signed-off-by: Yan Zhu --- cache/cache_test.go | 236 ++++++++++++++++++------------- client/api.go | 61 +++----- client/api_test.go | 98 ++++++------- client/client.go | 4 +- client/client_test.go | 8 ++ client/condition.go | 8 +- client/condition_test.go | 8 +- cmd/stress/stress.go | 8 ++ model/model.go | 21 +-- model/model_test.go | 34 +++-- test/ovs/ovs_integration_test.go | 22 ++- test/test_data.go | 12 ++ 12 files changed, 294 insertions(+), 226 deletions(-) diff --git a/cache/cache_test.go b/cache/cache_test.go index abca11d5..1b56e14d 100644 --- a/cache/cache_test.go +++ b/cache/cache_test.go @@ -24,6 +24,10 @@ type testModel struct { Datapath *string `ovsdb:"datapath"` } +func (t *testModel) Table() string { + return "Open_vSwitch" +} + const testSchemaFmt string = `{ "name": "Open_vSwitch", "tables": { @@ -369,14 +373,19 @@ func TestRowCacheCreateMultiIndex(t *testing.T) { } } +type testModel1 struct { + UUID string `ovsdb:"_uuid"` + Foo string `ovsdb:"foo"` + Bar map[string]string `ovsdb:"bar"` +} + +func (t *testModel1) Table() string { + return "Open_vSwitch" +} + func TestRowCacheCreateMultiClientIndex(t *testing.T) { - type testModel struct { - UUID string `ovsdb:"_uuid"` - Foo string `ovsdb:"foo"` - Bar map[string]string `ovsdb:"bar"` - } var schema ovsdb.DatabaseSchema - db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel1{}}) require.Nil(t, err) db.SetIndexes(map[string][]model.ClientIndex{ @@ -419,7 +428,7 @@ func TestRowCacheCreateMultiClientIndex(t *testing.T) { require.Nil(t, err) testData := Data{ - "Open_vSwitch": map[string]model.Model{"bar": &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}}, + "Open_vSwitch": map[string]model.Model{"bar": &testModel1{Foo: "bar", Bar: map[string]string{"bar": "bar"}}}, } dbModel, errs := model.NewDatabaseModel(schema, db) require.Empty(t, errs) @@ -432,22 +441,22 @@ func TestRowCacheCreateMultiClientIndex(t *testing.T) { tests := []struct { name string uuid string - model *testModel + model *testModel1 wantErr bool expected []expected }{ { name: "inserts a new row", uuid: "foo", - model: &testModel{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, + model: &testModel1{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, wantErr: false, expected: []expected{ { - index: &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + index: &testModel1{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, uuids: newUUIDSet("bar"), }, { - index: &testModel{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, + index: &testModel1{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, uuids: newUUIDSet("foo"), }, }, @@ -455,17 +464,17 @@ func TestRowCacheCreateMultiClientIndex(t *testing.T) { { name: "error duplicate uuid", uuid: "bar", - model: &testModel{Foo: "foo", Bar: map[string]string{"bar": "bar"}}, + model: &testModel1{Foo: "foo", Bar: map[string]string{"bar": "bar"}}, wantErr: true, }, { name: "inserts duplicate index", uuid: "baz", - model: &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + model: &testModel1{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, wantErr: false, expected: []expected{ { - index: &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + index: &testModel1{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, uuids: newUUIDSet("bar", "baz"), }, }, @@ -473,15 +482,15 @@ func TestRowCacheCreateMultiClientIndex(t *testing.T) { { name: "new row with one duplicate value", uuid: "baz", - model: &testModel{Foo: "foo", Bar: map[string]string{"bar": "bar"}}, + model: &testModel1{Foo: "foo", Bar: map[string]string{"bar": "bar"}}, wantErr: false, expected: []expected{ { - index: &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + index: &testModel1{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, uuids: newUUIDSet("bar"), }, { - index: &testModel{Foo: "foo", Bar: map[string]string{"bar": "bar"}}, + index: &testModel1{Foo: "foo", Bar: map[string]string{"bar": "bar"}}, uuids: newUUIDSet("baz"), }, }, @@ -489,15 +498,15 @@ func TestRowCacheCreateMultiClientIndex(t *testing.T) { { name: "new row with other duplicate value", uuid: "baz", - model: &testModel{Foo: "bar", Bar: map[string]string{"bar": "foo"}}, + model: &testModel1{Foo: "bar", Bar: map[string]string{"bar": "foo"}}, wantErr: false, expected: []expected{ { - index: &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + index: &testModel1{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, uuids: newUUIDSet("bar"), }, { - index: &testModel{Foo: "bar", Bar: map[string]string{"bar": "foo"}}, + index: &testModel1{Foo: "bar", Bar: map[string]string{"bar": "foo"}}, uuids: newUUIDSet("baz"), }, }, @@ -505,15 +514,15 @@ func TestRowCacheCreateMultiClientIndex(t *testing.T) { { name: "new row with nil map index", uuid: "baz", - model: &testModel{Foo: "bar"}, + model: &testModel1{Foo: "bar"}, wantErr: false, expected: []expected{ { - index: &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + index: &testModel1{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, uuids: newUUIDSet("bar"), }, { - index: &testModel{Foo: "bar"}, + index: &testModel1{Foo: "bar"}, uuids: newUUIDSet("baz"), }, }, @@ -791,15 +800,20 @@ func TestRowCacheUpdateMultiIndex(t *testing.T) { } } +type testModel2 struct { + UUID string `ovsdb:"_uuid"` + Foo string `ovsdb:"foo"` + Bar map[string]string `ovsdb:"bar"` + Baz string `ovsdb:"baz"` +} + +func (t *testModel2) Table() string { + return "Open_vSwitch" +} + func TestRowCacheUpdateMultiClientIndex(t *testing.T) { - type testModel struct { - UUID string `ovsdb:"_uuid"` - Foo string `ovsdb:"foo"` - Bar map[string]string `ovsdb:"bar"` - Baz string `ovsdb:"baz"` - } var schema ovsdb.DatabaseSchema - db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel2{}}) require.Nil(t, err) db.SetIndexes(map[string][]model.ClientIndex{ @@ -846,9 +860,9 @@ func TestRowCacheUpdateMultiClientIndex(t *testing.T) { testData := Data{ "Open_vSwitch": map[string]model.Model{ - "foo": &testModel{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, - "bar": &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, - "foobar": &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + "foo": &testModel2{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, + "bar": &testModel2{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + "foobar": &testModel2{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, }, } dbModel, errs := model.NewDatabaseModel(schema, db) @@ -862,27 +876,27 @@ func TestRowCacheUpdateMultiClientIndex(t *testing.T) { tests := []struct { name string uuid string - model *testModel + model *testModel2 wantErr bool expected []expected }{ { name: "error if row does not exist", uuid: "baz", - model: &testModel{Foo: "baz", Bar: map[string]string{"bar": "baz"}}, + model: &testModel2{Foo: "baz", Bar: map[string]string{"bar": "baz"}}, wantErr: true, }, { name: "update non-index", uuid: "foo", - model: &testModel{Foo: "foo", Bar: map[string]string{"bar": "foo"}, Baz: "bar"}, + model: &testModel2{Foo: "foo", Bar: map[string]string{"bar": "foo"}, Baz: "bar"}, expected: []expected{ { - index: &testModel{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, + index: &testModel2{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, uuids: newUUIDSet("foo"), }, { - index: &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + index: &testModel2{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, uuids: newUUIDSet("bar", "foobar"), }, }, @@ -890,15 +904,15 @@ func TestRowCacheUpdateMultiClientIndex(t *testing.T) { { name: "update one index column", uuid: "foo", - model: &testModel{Foo: "foo", Bar: map[string]string{"bar": "baz"}}, + model: &testModel2{Foo: "foo", Bar: map[string]string{"bar": "baz"}}, wantErr: false, expected: []expected{ { - index: &testModel{Foo: "foo", Bar: map[string]string{"bar": "baz"}}, + index: &testModel2{Foo: "foo", Bar: map[string]string{"bar": "baz"}}, uuids: newUUIDSet("foo"), }, { - index: &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + index: &testModel2{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, uuids: newUUIDSet("bar", "foobar"), }, }, @@ -906,15 +920,15 @@ func TestRowCacheUpdateMultiClientIndex(t *testing.T) { { name: "update other index column", uuid: "foo", - model: &testModel{Foo: "baz", Bar: map[string]string{"bar": "foo"}}, + model: &testModel2{Foo: "baz", Bar: map[string]string{"bar": "foo"}}, wantErr: false, expected: []expected{ { - index: &testModel{Foo: "baz", Bar: map[string]string{"bar": "foo"}}, + index: &testModel2{Foo: "baz", Bar: map[string]string{"bar": "foo"}}, uuids: newUUIDSet("foo"), }, { - index: &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + index: &testModel2{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, uuids: newUUIDSet("bar", "foobar"), }, }, @@ -922,15 +936,15 @@ func TestRowCacheUpdateMultiClientIndex(t *testing.T) { { name: "update both index columns", uuid: "foo", - model: &testModel{Foo: "baz", Bar: map[string]string{"bar": "baz"}}, + model: &testModel2{Foo: "baz", Bar: map[string]string{"bar": "baz"}}, wantErr: false, expected: []expected{ { - index: &testModel{Foo: "baz", Bar: map[string]string{"bar": "baz"}}, + index: &testModel2{Foo: "baz", Bar: map[string]string{"bar": "baz"}}, uuids: newUUIDSet("foo"), }, { - index: &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + index: &testModel2{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, uuids: newUUIDSet("bar", "foobar"), }, }, @@ -938,11 +952,11 @@ func TestRowCacheUpdateMultiClientIndex(t *testing.T) { { name: "update unique index to existing index", uuid: "foo", - model: &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + model: &testModel2{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, wantErr: false, expected: []expected{ { - index: &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + index: &testModel2{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, uuids: newUUIDSet("foo", "bar", "foobar"), }, }, @@ -950,15 +964,15 @@ func TestRowCacheUpdateMultiClientIndex(t *testing.T) { { name: "update multi index to different index", uuid: "foobar", - model: &testModel{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, + model: &testModel2{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, wantErr: false, expected: []expected{ { - index: &testModel{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, + index: &testModel2{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, uuids: newUUIDSet("foo", "foobar"), }, { - index: &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + index: &testModel2{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, uuids: newUUIDSet("bar"), }, }, @@ -1038,14 +1052,19 @@ func TestRowCacheDelete(t *testing.T) { } } +type testModel3 struct { + UUID string `ovsdb:"_uuid"` + Foo string `ovsdb:"foo"` + Bar map[string]string `ovsdb:"bar"` +} + +func (t *testModel3) Table() string { + return "Open_vSwitch" +} + func TestRowCacheDeleteClientIndex(t *testing.T) { - type testModel struct { - UUID string `ovsdb:"_uuid"` - Foo string `ovsdb:"foo"` - Bar map[string]string `ovsdb:"bar"` - } var schema ovsdb.DatabaseSchema - db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel3{}}) require.Nil(t, err) db.SetIndexes(map[string][]model.ClientIndex{ @@ -1089,9 +1108,9 @@ func TestRowCacheDeleteClientIndex(t *testing.T) { testData := Data{ "Open_vSwitch": map[string]model.Model{ - "foo": &testModel{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, - "bar": &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, - "foobar": &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + "foo": &testModel3{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, + "bar": &testModel3{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + "foobar": &testModel3{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, }, } dbModel, errs := model.NewDatabaseModel(schema, db) @@ -1105,24 +1124,24 @@ func TestRowCacheDeleteClientIndex(t *testing.T) { tests := []struct { name string uuid string - model *testModel + model *testModel3 wantErr bool expected []expected }{ { name: "error if row does not exist", uuid: "baz", - model: &testModel{Foo: "baz", Bar: map[string]string{"bar": "baz"}}, + model: &testModel3{Foo: "baz", Bar: map[string]string{"bar": "baz"}}, wantErr: true, }, { name: "delete a row with unique index", uuid: "foo", - model: &testModel{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, + model: &testModel3{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, wantErr: false, expected: []expected{ { - index: &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + index: &testModel3{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, uuids: newUUIDSet("bar", "foobar"), }, }, @@ -1130,15 +1149,15 @@ func TestRowCacheDeleteClientIndex(t *testing.T) { { name: "delete a row with duplicated index", uuid: "foobar", - model: &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + model: &testModel3{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, wantErr: false, expected: []expected{ { - index: &testModel{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, + index: &testModel3{Foo: "foo", Bar: map[string]string{"bar": "foo"}}, uuids: newUUIDSet("foo"), }, { - index: &testModel{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, + index: &testModel3{Foo: "bar", Bar: map[string]string{"bar": "bar"}}, uuids: newUUIDSet("bar"), }, }, @@ -1898,6 +1917,15 @@ func setupRowByModelSingleIndex(t require.TestingT) (*testModel, *TableCache) { return myFoo, tc } +type badModel struct { + UUID string `ovsdb:"_uuid"` + Baz string `ovsdb:"baz"` +} + +func (b *badModel) Table() string { + return "bad" +} + func TestTableCacheRowByModelSingleIndex(t *testing.T) { myFoo, tc := setupRowByModelSingleIndex(t) @@ -1921,10 +1949,6 @@ func TestTableCacheRowByModelSingleIndex(t *testing.T) { }) t.Run("wrong model type", func(t *testing.T) { - type badModel struct { - UUID string `ovsdb:"_uuid"` - Baz string `ovsdb:"baz"` - } _, _, err := tc.Table("Open_vSwitch").RowByModel(&badModel{Baz: "baz"}) assert.Error(t, err) }) @@ -2125,15 +2149,20 @@ func TestTableCacheRowByModelMultiIndex(t *testing.T) { }) } +type testModel4 struct { + UUID string `ovsdb:"_uuid"` + Foo string `ovsdb:"foo"` + Bar string `ovsdb:"bar"` + Baz map[string]string `ovsdb:"baz"` +} + +func (t *testModel4) Table() string { + return "Open_vSwitch" +} + func TestTableCacheRowsByModels(t *testing.T) { - type testModel struct { - UUID string `ovsdb:"_uuid"` - Foo string `ovsdb:"foo"` - Bar string `ovsdb:"bar"` - Baz map[string]string `ovsdb:"baz"` - } var schema ovsdb.DatabaseSchema - db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel4{}}) require.NoError(t, err) db.SetIndexes(map[string][]model.ClientIndex{ "Open_vSwitch": { @@ -2185,12 +2214,12 @@ func TestTableCacheRowsByModels(t *testing.T) { testData := Data{ "Open_vSwitch": map[string]model.Model{ - "foo": &testModel{Foo: "foo", Bar: "foo", Baz: map[string]string{"baz": "foo", "other": "other"}}, - "bar": &testModel{Foo: "bar", Bar: "bar", Baz: map[string]string{"baz": "bar", "other": "other"}}, - "foobar": &testModel{Foo: "foobar", Bar: "bar", Baz: map[string]string{"baz": "foobar", "other": "other"}}, - "baz": &testModel{Foo: "baz", Bar: "baz", Baz: map[string]string{"baz": "baz", "other": "other"}}, - "quux": &testModel{Foo: "quux", Bar: "quux", Baz: map[string]string{"baz": "quux", "other": "other"}}, - "quuz": &testModel{Foo: "quuz", Bar: "quux", Baz: map[string]string{"baz": "quux", "other": "other"}}, + "foo": &testModel4{Foo: "foo", Bar: "foo", Baz: map[string]string{"baz": "foo", "other": "other"}}, + "bar": &testModel4{Foo: "bar", Bar: "bar", Baz: map[string]string{"baz": "bar", "other": "other"}}, + "foobar": &testModel4{Foo: "foobar", Bar: "bar", Baz: map[string]string{"baz": "foobar", "other": "other"}}, + "baz": &testModel4{Foo: "baz", Bar: "baz", Baz: map[string]string{"baz": "baz", "other": "other"}}, + "quux": &testModel4{Foo: "quux", Bar: "quux", Baz: map[string]string{"baz": "quux", "other": "other"}}, + "quuz": &testModel4{Foo: "quuz", Bar: "quux", Baz: map[string]string{"baz": "quux", "other": "other"}}, }, } dbModel, errs := model.NewDatabaseModel(schema, db) @@ -2204,14 +2233,14 @@ func TestTableCacheRowsByModels(t *testing.T) { { name: "by non index, no result", models: []model.Model{ - &testModel{Foo: "no", Bar: "no", Baz: map[string]string{"baz": "no"}}, + &testModel4{Foo: "no", Bar: "no", Baz: map[string]string{"baz": "no"}}, }, rows: nil, }, { name: "by single column client index, single result", models: []model.Model{ - &testModel{Bar: "foo"}, + &testModel4{Bar: "foo"}, }, rows: map[string]model.Model{ "foo": testData["Open_vSwitch"]["foo"], @@ -2220,8 +2249,8 @@ func TestTableCacheRowsByModels(t *testing.T) { { name: "by single column client index, multiple models, multiple results", models: []model.Model{ - &testModel{Bar: "foo"}, - &testModel{Bar: "baz"}, + &testModel4{Bar: "foo"}, + &testModel4{Bar: "baz"}, }, rows: map[string]model.Model{ "foo": testData["Open_vSwitch"]["foo"], @@ -2231,7 +2260,7 @@ func TestTableCacheRowsByModels(t *testing.T) { { name: "by single column client index, multiple results", models: []model.Model{ - &testModel{Bar: "bar"}, + &testModel4{Bar: "bar"}, }, rows: map[string]model.Model{ "bar": testData["Open_vSwitch"]["bar"], @@ -2241,7 +2270,7 @@ func TestTableCacheRowsByModels(t *testing.T) { { name: "by multi column client index, single result", models: []model.Model{ - &testModel{Bar: "baz", Baz: map[string]string{"baz": "baz"}}, + &testModel4{Bar: "baz", Baz: map[string]string{"baz": "baz"}}, }, rows: map[string]model.Model{ "baz": testData["Open_vSwitch"]["baz"], @@ -2250,7 +2279,7 @@ func TestTableCacheRowsByModels(t *testing.T) { { name: "by client index, multiple results", models: []model.Model{ - &testModel{Bar: "quux", Baz: map[string]string{"baz": "quux"}}, + &testModel4{Bar: "quux", Baz: map[string]string{"baz": "quux"}}, }, rows: map[string]model.Model{ "quux": testData["Open_vSwitch"]["quux"], @@ -2260,8 +2289,8 @@ func TestTableCacheRowsByModels(t *testing.T) { { name: "by client index, multiple models, multiple results", models: []model.Model{ - &testModel{Bar: "quux", Baz: map[string]string{"baz": "quux"}}, - &testModel{Bar: "bar", Baz: map[string]string{"baz": "foobar"}}, + &testModel4{Bar: "quux", Baz: map[string]string{"baz": "quux"}}, + &testModel4{Bar: "bar", Baz: map[string]string{"baz": "foobar"}}, }, rows: map[string]model.Model{ "quux": testData["Open_vSwitch"]["quux"], @@ -2273,7 +2302,7 @@ func TestTableCacheRowsByModels(t *testing.T) { { name: "by schema index prioritized over client index", models: []model.Model{ - &testModel{Foo: "foo", Bar: "bar", Baz: map[string]string{"baz": "bar"}}, + &testModel4{Foo: "foo", Bar: "bar", Baz: map[string]string{"baz": "bar"}}, }, rows: map[string]model.Model{ "foo": testData["Open_vSwitch"]["foo"], @@ -2302,6 +2331,10 @@ type rowsByConditionTestModel struct { Empty string `ovsdb:"empty"` } +func (r *rowsByConditionTestModel) Table() string { + return "Open_vSwitch" +} + func setupRowsByConditionCache(t require.TestingT) *TableCache { var schema ovsdb.DatabaseSchema db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &rowsByConditionTestModel{}}) @@ -2709,11 +2742,16 @@ func BenchmarkRowsByCondition(b *testing.B) { } } +type testDBModel struct { + UUID string `ovsdb:"_uuid"` + Set []string `ovsdb:"set"` +} + +func (t *testDBModel) Table() string { + return "Open_vSwitch" +} + func BenchmarkPopulate2SingleModify(b *testing.B) { - type testDBModel struct { - UUID string `ovsdb:"_uuid"` - Set []string `ovsdb:"set"` - } aFooSet, _ := ovsdb.NewOvsSet([]string{"foo"}) base := &testDBModel{Set: []string{}} for i := 0; i < 57000; i++ { diff --git a/client/api.go b/client/api.go index 8d5a30ef..15de0aea 100644 --- a/client/api.go +++ b/client/api.go @@ -21,10 +21,10 @@ type API interface { // If it has a capacity != 0, only 'capacity' elements will be filled in List(ctx context.Context, result interface{}) error - // Create a Conditional API from a Function that is used to filter cached data + // WhereCache creates a Conditional API from a Function that is used to filter cached data // The function must accept a Model implementation and return a boolean. E.g: - // ConditionFromFunc(func(l *LogicalSwitch) bool { return l.Enabled }) - WhereCache(predicate interface{}) ConditionalAPI + // WhereCache(&LogicalSwitchPort{}, func(m model.Model) bool { l := m.(*LogicalSwitchPort); return l.Enabled }) + WhereCache(model.Model, func(model.Model) bool) ConditionalAPI // Create a ConditionalAPI from a Model's index data, where operations // apply to elements that match the values provided in one or more @@ -120,13 +120,20 @@ func (a api) List(ctx context.Context, result interface{}) error { // structs var appendValue func(reflect.Value) var m model.Model + var ok bool if resultVal.Type().Elem().Kind() == reflect.Ptr { - m = reflect.New(resultVal.Type().Elem().Elem()).Interface() + m, ok = reflect.New(resultVal.Type().Elem().Elem()).Interface().(model.Model) + if !ok { + return &ErrWrongType{resultPtr.Type(), "Expected pointer to slice of valid Models"} + } appendValue = func(v reflect.Value) { resultVal.Set(reflect.Append(resultVal, v)) } } else { - m = reflect.New(resultVal.Type().Elem()).Interface() + m, ok = reflect.New(resultVal.Type().Elem()).Interface().(model.Model) + if !ok { + return &ErrWrongType{resultPtr.Type(), "Expected pointer to slice of valid Models"} + } appendValue = func(v reflect.Value) { resultVal.Set(reflect.Append(resultVal, reflect.Indirect(v))) } @@ -194,14 +201,17 @@ func (a api) WhereAll(m model.Model, cond ...model.Condition) ConditionalAPI { } // WhereCache returns a conditionalAPI based a Predicate -func (a api) WhereCache(predicate interface{}) ConditionalAPI { - return newConditionalAPI(a.cache, a.conditionFromFunc(predicate), a.logger) +func (a api) WhereCache(m model.Model, predicate func(model.Model) bool) ConditionalAPI { + return newConditionalAPI(a.cache, a.conditionFromFunc(m, predicate), a.logger) } // Conditional interface implementation // FromFunc returns a Condition from a function -func (a api) conditionFromFunc(predicate interface{}) Conditional { - table, err := a.getTableFromFunc(predicate) +func (a api) conditionFromFunc(m model.Model, predicate func(model.Model) bool) Conditional { + if predicate == nil { + return newErrorConditional(fmt.Errorf("expect predicate as a function, got nil")) + } + table, err := a.getTableFromModel(m) if err != nil { return newErrorConditional(err) } @@ -538,43 +548,14 @@ func (a api) Wait(untilConFun ovsdb.WaitCondition, timeout *int, model model.Mod // getTableFromModel returns the table name from a Model object after performing // type verifications on the model -func (a api) getTableFromModel(m interface{}) (string, error) { - if _, ok := m.(model.Model); !ok { - return "", &ErrWrongType{reflect.TypeOf(m), "Type does not implement Model interface"} - } - table := a.cache.DatabaseModel().FindTable(reflect.TypeOf(m)) +func (a api) getTableFromModel(m model.Model) (string, error) { + table := m.Table() if table == "" { return "", &ErrWrongType{reflect.TypeOf(m), "Model not found in Database Model"} } return table, nil } -// getTableFromModel returns the table name from a the predicate after performing -// type verifications -func (a api) getTableFromFunc(predicate interface{}) (string, error) { - predType := reflect.TypeOf(predicate) - if predType == nil || predType.Kind() != reflect.Func { - return "", &ErrWrongType{predType, "Expected function"} - } - if predType.NumIn() != 1 || predType.NumOut() != 1 || predType.Out(0).Kind() != reflect.Bool { - return "", &ErrWrongType{predType, "Expected func(Model) bool"} - } - - modelInterface := reflect.TypeOf((*model.Model)(nil)).Elem() - modelType := predType.In(0) - if !modelType.Implements(modelInterface) { - return "", &ErrWrongType{predType, - fmt.Sprintf("Type %s does not implement Model interface", modelType.String())} - } - - table := a.cache.DatabaseModel().FindTable(modelType) - if table == "" { - return "", &ErrWrongType{predType, - fmt.Sprintf("Model %s not found in Database Model", modelType.String())} - } - return table, nil -} - // newAPI returns a new API to interact with the database func newAPI(cache *cache.TableCache, logger *logr.Logger) API { return api{ diff --git a/client/api_test.go b/client/api_test.go index 7c768109..fea0d14a 100644 --- a/client/api_test.go +++ b/client/api_test.go @@ -217,13 +217,15 @@ func TestAPIListPredicate(t *testing.T) { test := []struct { name string - predicate interface{} + m model.Model + predicate func(model.Model) bool content []model.Model err bool }{ { name: "none", - predicate: func(t *testLogicalSwitch) bool { + m: &testLogicalSwitch{}, + predicate: func(t model.Model) bool { return false }, content: []model.Model{}, @@ -231,7 +233,8 @@ func TestAPIListPredicate(t *testing.T) { }, { name: "all", - predicate: func(t *testLogicalSwitch) bool { + m: &testLogicalSwitch{}, + predicate: func(t model.Model) bool { return true }, content: lscacheList, @@ -239,30 +242,26 @@ func TestAPIListPredicate(t *testing.T) { }, { name: "nil function must fail", + m: &testLogicalSwitch{}, err: true, }, { name: "arbitrary condition", - predicate: func(t *testLogicalSwitch) bool { + m: &testLogicalSwitch{}, + predicate: func(m model.Model) bool { + t := m.(*testLogicalSwitch) return strings.HasPrefix(t.Name, "magic") }, content: []model.Model{lscacheList[1], lscacheList[3]}, err: false, }, - { - name: "error wrong type", - predicate: func(t testLogicalSwitch) string { - return "foo" - }, - err: true, - }, } for _, tt := range test { t.Run(fmt.Sprintf("ApiListPredicate: %s", tt.name), func(t *testing.T) { var result []*testLogicalSwitch api := newAPI(tcache, &discardLogger) - cond := api.WhereCache(tt.predicate) + cond := api.WhereCache(tt.m, tt.predicate) err := cond.List(context.Background(), &result) if tt.err { assert.NotNil(t, err) @@ -536,26 +535,14 @@ func TestAPIListMulti(t *testing.T) { func TestConditionFromFunc(t *testing.T) { test := []struct { name string - arg interface{} + m model.Model + arg func(model.Model) bool err bool }{ - { - name: "wrong function must fail", - arg: func(s string) bool { - return false - }, - err: true, - }, - { - name: "wrong function must fail2 ", - arg: func(t *testLogicalSwitch) string { - return "foo" - }, - err: true, - }, { name: "correct func should succeed", - arg: func(t *testLogicalSwitch) bool { + m: &testLogicalSwitch{}, + arg: func(m model.Model) bool { return true }, err: false, @@ -566,7 +553,7 @@ func TestConditionFromFunc(t *testing.T) { t.Run(fmt.Sprintf("conditionFromFunc: %s", tt.name), func(t *testing.T) { cache := apiTestCache(t, nil) apiIface := newAPI(cache, &discardLogger) - condition := apiIface.(api).conditionFromFunc(tt.arg) + condition := apiIface.(api).conditionFromFunc(tt.m, tt.arg) if tt.err { assert.IsType(t, &errorConditional{}, condition) } else { @@ -584,23 +571,6 @@ func TestConditionFromModel(t *testing.T) { conds []model.Condition err bool }{ - { - name: "wrong model must fail", - models: []model.Model{ - &struct{ a string }{}, - }, - err: true, - }, - { - name: "wrong condition must fail", - models: []model.Model{ - &struct { - a string `ovsdb:"_uuid"` - }{}, - }, - conds: []model.Condition{{Field: "foo"}}, - err: true, - }, { name: "correct model must succeed", models: []model.Model{ @@ -1009,7 +979,8 @@ func TestAPIMutate(t *testing.T) { { name: "select single by predicate name insert element in map", condition: func(a API) ConditionalAPI { - return a.WhereCache(func(lsp *testLogicalSwitchPort) bool { + return a.WhereCache(&testLogicalSwitchPort{}, func(m model.Model) bool { + lsp := m.(*testLogicalSwitchPort) return lsp.Name == "lsp2" }) }, @@ -1033,7 +1004,8 @@ func TestAPIMutate(t *testing.T) { { name: "select many by predicate name insert element in map", condition: func(a API) ConditionalAPI { - return a.WhereCache(func(lsp *testLogicalSwitchPort) bool { + return a.WhereCache(&testLogicalSwitchPort{}, func(m model.Model) bool { + lsp := m.(*testLogicalSwitchPort) return lsp.Type == "someType" }) }, @@ -1063,7 +1035,8 @@ func TestAPIMutate(t *testing.T) { { name: "No mutations should error", condition: func(a API) ConditionalAPI { - return a.WhereCache(func(lsp *testLogicalSwitchPort) bool { + return a.WhereCache(&testLogicalSwitchPort{}, func(m model.Model) bool { + lsp := m.(*testLogicalSwitchPort) return lsp.Type == "someType" }) }, @@ -1411,7 +1384,8 @@ 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 a.WhereCache(&testLogicalSwitchPort{}, func(m model.Model) bool { + t := m.(*testLogicalSwitchPort) return t.Enabled != nil && *t.Enabled == true }) }, @@ -1645,7 +1619,8 @@ func TestAPIDelete(t *testing.T) { { name: "select multiple by predicate", condition: func(a API) ConditionalAPI { - return a.WhereCache(func(t *testLogicalSwitchPort) bool { + return a.WhereCache(&testLogicalSwitchPort{}, func(m model.Model) bool { + t := m.(*testLogicalSwitchPort) return t.Enabled != nil && *t.Enabled == true }) }, @@ -1724,29 +1699,36 @@ func BenchmarkAPIList(b *testing.B) { test := []struct { name string - predicate interface{} + m model.Model + predicate func(model.Model) bool }{ { name: "predicate returns none", - predicate: func(t *testLogicalSwitchPort) bool { + m: &testLogicalSwitchPort{}, + predicate: func(t model.Model) bool { return false }, }, { name: "predicate returns all", - predicate: func(t *testLogicalSwitchPort) bool { + m: &testLogicalSwitchPort{}, + predicate: func(t model.Model) bool { return true }, }, { name: "predicate on an arbitrary condition", - predicate: func(t *testLogicalSwitchPort) bool { + m: &testLogicalSwitchPort{}, + predicate: func(m model.Model) bool { + t := m.(*testLogicalSwitchPort) return strings.HasPrefix(t.Name, "ls1") }, }, { name: "predicate matches name", - predicate: func(t *testLogicalSwitchPort) bool { + m: &testLogicalSwitchPort{}, + predicate: func(m model.Model) bool { + t := m.(*testLogicalSwitchPort) return t.Name == lscacheList[index].Name }, }, @@ -1763,7 +1745,7 @@ func BenchmarkAPIList(b *testing.B) { api := newAPI(tcache, &discardLogger) var cond ConditionalAPI if tt.predicate != nil { - cond = api.WhereCache(tt.predicate) + cond = api.WhereCache(tt.m, tt.predicate) } else { cond = api.Where(lscacheList[index]) } @@ -1979,7 +1961,7 @@ func TestAPIWait(t *testing.T) { { name: "no operation", condition: func(a API) ConditionalAPI { - return a.WhereCache(func(t *testLogicalSwitchPort) bool { return false }) + return a.WhereCache(&testLogicalSwitchPort{}, func(t model.Model) bool { return false }) }, until: "==", prepare: func() (model.Model, []interface{}) { diff --git a/client/client.go b/client/client.go index 10ea757e..7a526c27 100644 --- a/client/client.go +++ b/client/client.go @@ -1475,6 +1475,6 @@ func (o *ovsdbClient) WhereAll(m model.Model, conditions ...model.Condition) Con } // WhereCache implements the API interface's WhereCache function -func (o *ovsdbClient) WhereCache(predicate interface{}) ConditionalAPI { - return o.primaryDB().api.WhereCache(predicate) +func (o *ovsdbClient) WhereCache(m model.Model, predicate func(model.Model) bool) ConditionalAPI { + return o.primaryDB().api.WhereCache(m, predicate) } diff --git a/client/client_test.go b/client/client_test.go index 1f66651a..23ac2da9 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -81,6 +81,10 @@ type Bridge struct { STPEnable bool `ovsdb:"stp_enable"` } +func (b *Bridge) Table() string { + return "Bridge" +} + // OpenvSwitch defines an object in Open_vSwitch table type OpenvSwitch struct { UUID string `ovsdb:"_uuid"` @@ -103,6 +107,10 @@ type OpenvSwitch struct { SystemVersion *string `ovsdb:"system_version"` } +func (o *OpenvSwitch) Table() string { + return "Open_vSwitch" +} + var defDB, _ = model.NewClientDBModel("Open_vSwitch", map[string]model.Model{ "Open_vSwitch": &OpenvSwitch{}, diff --git a/client/condition.go b/client/condition.go index 1dfabda0..435111c0 100644 --- a/client/condition.go +++ b/client/condition.go @@ -2,7 +2,6 @@ package client import ( "fmt" - "reflect" "github.com/ovn-org/libovsdb/cache" "github.com/ovn-org/libovsdb/mapper" @@ -176,7 +175,7 @@ func newExplicitConditional(table string, cache *cache.TableCache, matchAll bool // to match on models. type predicateConditional struct { tableName string - predicate interface{} + predicate func(model.Model) bool cache *cache.TableCache } @@ -192,8 +191,7 @@ func (c *predicateConditional) Matches() (map[string]model.Model, error) { // run the predicate on a shallow copy of the models for speed and only // clone the matches for u, m := range tableCache.RowsShallow() { - ret := reflect.ValueOf(c.predicate).Call([]reflect.Value{reflect.ValueOf(m)}) - if ret[0].Bool() { + if c.predicate(m) { found[u] = model.Clone(m) } } @@ -215,7 +213,7 @@ func (c *predicateConditional) Generate() ([][]ovsdb.Condition, error) { } // newPredicateConditional creates a new predicateConditional -func newPredicateConditional(table string, cache *cache.TableCache, predicate interface{}) (Conditional, error) { +func newPredicateConditional(table string, cache *cache.TableCache, predicate func(model.Model) bool) (Conditional, error) { return &predicateConditional{ tableName: table, predicate: predicate, diff --git a/client/condition_test.go b/client/condition_test.go index 5bd63b33..00fa8eeb 100644 --- a/client/condition_test.go +++ b/client/condition_test.go @@ -204,14 +204,15 @@ func TestPredicateConditional(t *testing.T) { test := []struct { name string - predicate interface{} + predicate func(model.Model) bool condition [][]ovsdb.Condition matches map[string]model.Model err bool }{ { name: "simple value comparison", - predicate: func(lsp *testLogicalSwitchPort) bool { + predicate: func(m model.Model) bool { + lsp := m.(*testLogicalSwitchPort) return lsp.UUID == aUUID0 }, condition: [][]ovsdb.Condition{ @@ -225,7 +226,8 @@ func TestPredicateConditional(t *testing.T) { }, { name: "by random field", - predicate: func(lsp *testLogicalSwitchPort) bool { + predicate: func(m model.Model) bool { + lsp := m.(*testLogicalSwitchPort) return lsp.Enabled != nil && *lsp.Enabled == false }, condition: [][]ovsdb.Condition{ diff --git a/cmd/stress/stress.go b/cmd/stress/stress.go index 2ebbdcb7..e333be0e 100644 --- a/cmd/stress/stress.go +++ b/cmd/stress/stress.go @@ -29,12 +29,20 @@ type bridgeType struct { Status map[string]string `ovsdb:"status"` } +func (b *bridgeType) Table() string { + return "Bridge" +} + // ORMovs is the simplified ORM model of the Bridge table type ovsType struct { UUID string `ovsdb:"_uuid"` Bridges []string `ovsdb:"bridges"` } +func (o *ovsType) Table() string { + return "Open_vSwitch" +} + var ( cpuprofile = flag.String("cpuprofile", "", "write cpu profile to this file") memprofile = flag.String("memoryprofile", "", "write memory profile to this file") diff --git a/model/model.go b/model/model.go index c8575f5b..57eb9db5 100644 --- a/model/model.go +++ b/model/model.go @@ -16,20 +16,25 @@ import ( // The struct may also have non-tagged fields (which will be ignored by the API calls) // The Model interface must be implemented by the pointer to such type // Example: -//type MyLogicalRouter struct { -// UUID string `ovsdb:"_uuid"` -// Name string `ovsdb:"name"` -// ExternalIDs map[string]string `ovsdb:"external_ids"` -// LoadBalancers []string `ovsdb:"load_balancer"` -//} -type Model interface{} +// +// type MyLogicalRouter struct { +// UUID string `ovsdb:"_uuid"` +// Name string `ovsdb:"name"` +// ExternalIDs map[string]string `ovsdb:"external_ids"` +// LoadBalancers []string `ovsdb:"load_balancer"` +// } +type Model interface { + Table() string +} type CloneableModel interface { + Model CloneModel() Model CloneModelInto(Model) } type ComparableModel interface { + Model EqualsModel(Model) bool } @@ -43,7 +48,7 @@ func Clone(a Model) Model { b := reflect.New(val.Type()).Interface() aBytes, _ := json.Marshal(a) _ = json.Unmarshal(aBytes, b) - return b + return b.(Model) } // CloneInto deep copies a model into another one diff --git a/model/model_test.go b/model/model_test.go index e75a4d45..2e40daf8 100644 --- a/model/model_test.go +++ b/model/model_test.go @@ -14,16 +14,28 @@ type modelA struct { UUID string `ovsdb:"_uuid"` } +func (m *modelA) Table() string { + return "A" +} + type modelB struct { UID string `ovsdb:"_uuid"` Foo string `ovsdb:"bar"` Bar string `ovsdb:"baz"` } +func (m *modelB) Table() string { + return "B" +} + type modelInvalid struct { Foo string } +func (m *modelInvalid) Table() string { + return "Invalid" +} + func TestClientDBModel(t *testing.T) { type Test struct { name string @@ -86,16 +98,22 @@ func TestSetUUID(t *testing.T) { } +type testTable struct { + AUUID string `ovsdb:"_uuid"` + AString string `ovsdb:"aString"` + AInt int `ovsdb:"aInt"` + AFloat float64 `ovsdb:"aFloat"` + ASet []string `ovsdb:"aSet"` + AMap map[string]string `ovsdb:"aMap"` +} + +func (t *testTable) Table() string { + return "TestTable" +} + func TestValidate(t *testing.T) { model, err := NewClientDBModel("TestDB", map[string]Model{ - "TestTable": &struct { - aUUID string `ovsdb:"_uuid"` - aString string `ovsdb:"aString"` - aInt int `ovsdb:"aInt"` - aFloat float64 `ovsdb:"aFloat"` - aSet []string `ovsdb:"aSet"` - aMap map[string]string `ovsdb:"aMap"` - }{}, + "TestTable": &testTable{}, }) assert.Nil(t, err) diff --git a/test/ovs/ovs_integration_test.go b/test/ovs/ovs_integration_test.go index daafc926..a3621a3d 100644 --- a/test/ovs/ovs_integration_test.go +++ b/test/ovs/ovs_integration_test.go @@ -166,6 +166,10 @@ type bridgeType struct { DatapathID *string `ovsdb:"datapath_id"` } +func (b *bridgeType) Table() string { + return "Bridge" +} + // ovsType is the ORM model of the OVS table type ovsType struct { UUID string `ovsdb:"_uuid"` @@ -188,18 +192,30 @@ type ovsType struct { SystemVersion *string `ovsdb:"system_version"` } +func (o *ovsType) Table() string { + return "Open_vSwitch" +} + // ipfixType is a simplified ORM model for the IPFIX table type ipfixType struct { UUID string `ovsdb:"_uuid"` Targets []string `ovsdb:"targets"` } +func (i *ipfixType) Table() string { + return "IPFIX" +} + // queueType is the simplified ORM model of the Queue table type queueType struct { UUID string `ovsdb:"_uuid"` DSCP *int `ovsdb:"dscp"` } +func (q *queueType) Table() string { + return "Queue" +} + var defDB, _ = model.NewClientDBModel("Open_vSwitch", map[string]model.Model{ "Open_vSwitch": &ovsType{}, "Bridge": &bridgeType{}, @@ -585,7 +601,7 @@ func (suite *OVSIntegrationSuite) TestInsertAndDeleteTransactIntegration() { require.NoError(suite.T(), err) ovsRow := ovsType{} - delMutateOp, err := suite.clientWithoutInactvityCheck.WhereCache(func(*ovsType) bool { return true }). + delMutateOp, err := suite.clientWithoutInactvityCheck.WhereCache(&ovsType{}, func(m model.Model) bool { return true }). Mutate(&ovsRow, model.Mutation{ Field: &ovsRow.Bridges, Mutator: ovsdb.MutateOperationDelete, @@ -828,7 +844,7 @@ func (suite *OVSIntegrationSuite) createBridge(bridgeName string) (string, error // Inserting a Bridge row in Bridge table requires mutating the open_vswitch table. ovsRow := ovsType{} - mutateOp, err := suite.clientWithoutInactvityCheck.WhereCache(func(*ovsType) bool { return true }). + mutateOp, err := suite.clientWithoutInactvityCheck.WhereCache(&ovsType{}, func(m model.Model) bool { return true }). Mutate(&ovsRow, model.Mutation{ Field: &ovsRow.Bridges, Mutator: ovsdb.MutateOperationInsert, @@ -1157,7 +1173,7 @@ func (suite *OVSIntegrationSuite) TestMultipleOpsSameRow() { ops = append(ops, op...) ovs := ovsType{} - op, err = suite.clientWithoutInactvityCheck.WhereCache(func(*ovsType) bool { return true }).Mutate(&ovs, model.Mutation{ + op, err = suite.clientWithoutInactvityCheck.WhereCache(&ovsType{}, func(m model.Model) bool { return true }).Mutate(&ovs, model.Mutation{ Field: &ovs.Bridges, Mutator: ovsdb.MutateOperationInsert, Value: []string{bridgeUUID}, diff --git a/test/test_data.go b/test/test_data.go index 6be6b8b2..55a8b105 100644 --- a/test/test_data.go +++ b/test/test_data.go @@ -144,12 +144,20 @@ type BridgeType struct { Status map[string]string `ovsdb:"status"` } +func (b *BridgeType) Table() string { + return "Bridge" +} + // OvsType is the simplified ORM model of the Bridge table type OvsType struct { UUID string `ovsdb:"_uuid"` Bridges []string `ovsdb:"bridges"` } +func (o *OvsType) Table() string { + return "Open_vSwitch" +} + type FlowSampleCollectorSetType struct { UUID string `ovsdb:"_uuid"` Bridge string `ovsdb:"bridge"` @@ -158,6 +166,10 @@ type FlowSampleCollectorSetType struct { IPFIX *string // `ovsdb:"ipfix"` } +func (f *FlowSampleCollectorSetType) Table() string { + return "FlowSampleCollectorSet" +} + func GetModel() (model.DatabaseModel, error) { client, err := model.NewClientDBModel( "Open_vSwitch",