From fa844147e9dcaf917542115d664ab955aeae3a9c Mon Sep 17 00:00:00 2001 From: Adrian Moreno Date: Fri, 8 Oct 2021 10:50:55 +0200 Subject: [PATCH 01/10] client_test: fix monitor cookie json MonitorCookies are marshalled as a json object, not an array. Fix it so that benchmark test works Signed-off-by: Adrian Moreno --- client/client_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/client_test.go b/client/client_test.go index 173bab29..8857c5f8 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -491,7 +491,7 @@ func testOvsMap(t *testing.T, set interface{}) ovsdb.OvsMap { func updateBenchmark(ovs *ovsdbClient, updates []byte, b *testing.B) { for n := 0; n < b.N; n++ { - params := []json.RawMessage{[]byte(`["Open_vSwitch","v1"]`), updates} + params := []json.RawMessage{[]byte(`{"databaseName":"Open_vSwitch","id":"v1"}`), updates} var reply []interface{} err := ovs.update(params, &reply) if err != nil { From 8ef77531d46070444f6702c110d78eea13ebf963 Mon Sep 17 00:00:00 2001 From: Adrian Moreno Date: Mon, 11 Oct 2021 12:29:45 +0200 Subject: [PATCH 02/10] example: fix ovsdb-server schema file Signed-off-by: Adrian Moreno --- example/ovsdb-server/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/example/ovsdb-server/main.go b/example/ovsdb-server/main.go index b510beb0..63e6b552 100644 --- a/example/ovsdb-server/main.go +++ b/example/ovsdb-server/main.go @@ -47,7 +47,7 @@ func main() { if err != nil { log.Fatal(err) } - path := filepath.Join(wd, "vswitchd", "vswitchd.ovsschema") + path := filepath.Join(wd, "vswitchd", "ovs.ovsschema") f, err := os.Open(path) if err != nil { log.Fatal(err) From 19c8539d4100515e226f7a3eacfceedbf7d87f99 Mon Sep 17 00:00:00 2001 From: Adrian Moreno Date: Thu, 7 Oct 2021 17:02:22 +0200 Subject: [PATCH 03/10] rename DBModel to DatabaseModelRequest Clearly DBModel does not hold the full database model. Instead, only the combination of model.DBModel, mapper.Mapper and ovsdb.Schema is a useful database model. The fact that server.go had to defined a struct called DatabaseModel with model.DBModel and ovsdb.Schema and dymanically create Mapper objects from them is a proof of this. In order to prepare for a DBModel refactoring, rename it to DatabaseModelRequest as it's what the client requests the DatabaseModel to look like. This patch does not contain functional changes Signed-off-by: Adrian Moreno --- README.md | 14 ++-- cache/cache.go | 24 +++---- cache/cache_test.go | 26 ++++---- client/api.go | 8 +-- client/api_test_model.go | 2 +- client/client.go | 6 +- client/client_test.go | 26 ++++---- client/doc.go | 8 +-- client/monitor.go | 4 +- cmd/stress/stress.go | 8 +-- example/ovsdb-server/main.go | 10 +-- example/play_with_ovs/main.go | 4 +- model/model.go | 94 --------------------------- model/model_test.go | 8 +-- model/request.go | 102 ++++++++++++++++++++++++++++++ modelgen/dbmodel.go | 6 +- modelgen/dbmodel_test.go | 4 +- ovsdb/serverdb/model.go | 4 +- server/database.go | 6 +- server/server.go | 4 +- server/server_integration_test.go | 24 +++---- server/server_test.go | 4 +- server/transact_test.go | 12 ++-- test/ovs/ovs_integration_test.go | 2 +- 24 files changed, 209 insertions(+), 201 deletions(-) create mode 100644 model/request.go diff --git a/README.md b/README.md index 491ad5ec..7d8e7391 100644 --- a/README.md +++ b/README.md @@ -32,16 +32,16 @@ To make the API use go idioms, the following mappings occur: 1. OVSDB Map = Map 1. OVSDB Scalar Type = Equivalent scalar Go type -A Open vSwitch Database is modeled using a DBModel which is a created by assigning table names to pointers to these structs: +A Open vSwitch Database is modeled using a DatabaseModelRequest which is a created by assigning table names to pointers to these structs: - dbModel, _ := model.NewDBModel("OVN_Northbound", map[string]model.Model{ + dbModelReq, _ := model.NewDatabaseModelRequest("OVN_Northbound", map[string]model.Model{ "Logical_Switch": &MyLogicalSwitch{}, }) Finally, a client object can be created: - ovs, _ := client.Connect(context.Background(), dbModel, client.WithEndpoint("tcp:172.18.0.4:6641")) + ovs, _ := client.Connect(context.Background(), dbModelReq, client.WithEndpoint("tcp:172.18.0.4:6641")) client.MonitorAll(nil) // Only needed if you want to use the built-in cache @@ -219,7 +219,7 @@ It can be used as follows: Package name (default "ovsmodel") The result will be the definition of a Model per table defined in the ovsdb schema file. -Additionally, a function called `FullDatabaseModel()` that returns the `DBModel` is created for convenience. +Additionally, a function called `FullDatabaseModel()` that returns the `DatabaseModelRequest` is created for convenience. Example: @@ -237,7 +237,7 @@ Run `go generate` go generate ./... -In your application, load the DBModel, connect to the server and start interacting with the database: +In your application, load the DatabaseModelRequest, connect to the server and start interacting with the database: import ( "fmt" @@ -247,8 +247,8 @@ In your application, load the DBModel, connect to the server and start interacti ) func main() { - dbModel, _ := generated.FullDatabaseModel() - ovs, _ := client.Connect(context.Background(), dbModel, client.WithEndpoint("tcp:localhost:6641")) + dbModelReq, _ := generated.FullDatabaseModel() + ovs, _ := client.Connect(context.Background(), dbModelReq, client.WithEndpoint("tcp:localhost:6641")) ovs.MonitorAll() // Create a *LogicalRouter, as a pointer to a Model is required by the API diff --git a/cache/cache.go b/cache/cache.go index b8101f8b..d38bbb4b 100644 --- a/cache/cache.go +++ b/cache/cache.go @@ -407,7 +407,7 @@ type TableCache struct { cache map[string]*RowCache eventProcessor *eventProcessor mapper *mapper.Mapper - dbModel *model.DBModel + dbModelReq *model.DatabaseModelRequest schema *ovsdb.DatabaseSchema ovsdb.NotificationHandler mutex sync.RWMutex @@ -417,13 +417,13 @@ type TableCache struct { type Data map[string]map[string]model.Model // NewTableCache creates a new TableCache -func NewTableCache(schema *ovsdb.DatabaseSchema, dbModel *model.DBModel, data Data) (*TableCache, error) { - if schema == nil || dbModel == nil { +func NewTableCache(schema *ovsdb.DatabaseSchema, dbModelReq *model.DatabaseModelRequest, data Data) (*TableCache, error) { + if schema == nil || dbModelReq == nil { return nil, fmt.Errorf("tablecache without databasemodel cannot be populated") } eventProcessor := newEventProcessor(bufferSize) cache := make(map[string]*RowCache) - tableTypes := dbModel.Types() + tableTypes := dbModelReq.Types() for name, tableSchema := range schema.Tables { cache[name] = newRowCache(name, tableSchema, tableTypes[name]) } @@ -442,7 +442,7 @@ func NewTableCache(schema *ovsdb.DatabaseSchema, dbModel *model.DBModel, data Da schema: schema, eventProcessor: eventProcessor, mapper: mapper.NewMapper(schema), - dbModel: dbModel, + dbModelReq: dbModelReq, mutex: sync.RWMutex{}, }, nil } @@ -452,9 +452,9 @@ func (t *TableCache) Mapper() *mapper.Mapper { return t.mapper } -// DBModel returns the DBModel -func (t *TableCache) DBModel() *model.DBModel { - return t.dbModel +// DatabaseModelRequest returns the DatabaseModelRequest +func (t *TableCache) DatabaseModelRequest() *model.DatabaseModelRequest { + return t.dbModelReq } // Table returns the a Table from the cache with a given name @@ -517,7 +517,7 @@ func (t *TableCache) Populate(tableUpdates ovsdb.TableUpdates) { t.mutex.Lock() defer t.mutex.Unlock() - for table := range t.dbModel.Types() { + for table := range t.dbModelReq.Types() { updates, ok := tableUpdates[table] if !ok { continue @@ -563,7 +563,7 @@ func (t *TableCache) Populate(tableUpdates ovsdb.TableUpdates) { func (t *TableCache) Populate2(tableUpdates ovsdb.TableUpdates2) { t.mutex.Lock() defer t.mutex.Unlock() - for table := range t.dbModel.Types() { + for table := range t.dbModelReq.Types() { updates, ok := tableUpdates[table] if !ok { continue @@ -628,7 +628,7 @@ func (t *TableCache) Populate2(tableUpdates ovsdb.TableUpdates2) { func (t *TableCache) Purge(schema *ovsdb.DatabaseSchema) { t.mutex.Lock() defer t.mutex.Unlock() - tableTypes := t.dbModel.Types() + tableTypes := t.dbModelReq.Types() for name, tableSchema := range t.schema.Tables { t.cache[name] = newRowCache(name, tableSchema, tableTypes[name]) } @@ -760,7 +760,7 @@ func (t *TableCache) CreateModel(tableName string, row *ovsdb.Row, uuid string) if table == nil { return nil, fmt.Errorf("table %s not found", tableName) } - model, err := t.dbModel.NewModel(tableName) + model, err := t.dbModelReq.NewModel(tableName) if err != nil { return nil, err } diff --git a/cache/cache_test.go b/cache/cache_test.go index 411aa785..866ae133 100644 --- a/cache/cache_test.go +++ b/cache/cache_test.go @@ -84,7 +84,7 @@ func TestRowCache_Rows(t *testing.T) { func TestRowCacheCreate(t *testing.T) { var schema ovsdb.DatabaseSchema - db, err := model.NewDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) require.Nil(t, err) err = json.Unmarshal([]byte(` {"name": "TestDB", @@ -170,7 +170,7 @@ func TestRowCacheCreate(t *testing.T) { func TestRowCacheCreateMultiIndex(t *testing.T) { var schema ovsdb.DatabaseSchema - db, err := model.NewDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) require.Nil(t, err) err = json.Unmarshal([]byte(` {"name": "TestDB", @@ -263,7 +263,7 @@ func TestRowCacheCreateMultiIndex(t *testing.T) { func TestRowCacheUpdate(t *testing.T) { var schema ovsdb.DatabaseSchema - db, err := model.NewDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) require.Nil(t, err) err = json.Unmarshal([]byte(` {"name": "TestDB", @@ -345,7 +345,7 @@ func TestRowCacheUpdate(t *testing.T) { func TestRowCacheUpdateMultiIndex(t *testing.T) { var schema ovsdb.DatabaseSchema - db, err := model.NewDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) require.Nil(t, err) err = json.Unmarshal([]byte(` {"name": "TestDB", @@ -427,7 +427,7 @@ func TestRowCacheUpdateMultiIndex(t *testing.T) { func TestRowCacheDelete(t *testing.T) { var schema ovsdb.DatabaseSchema - db, err := model.NewDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) require.Nil(t, err) err = json.Unmarshal([]byte(` {"name": "TestDB", @@ -687,7 +687,7 @@ func TestTableCacheTables(t *testing.T) { func TestTableCache_populate(t *testing.T) { t.Log("Create") - db, err := model.NewDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) assert.Nil(t, err) var schema ovsdb.DatabaseSchema err = json.Unmarshal([]byte(` @@ -752,7 +752,7 @@ func TestTableCache_populate(t *testing.T) { func TestTableCachePopulate(t *testing.T) { t.Log("Create") - db, err := model.NewDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) assert.Nil(t, err) var schema ovsdb.DatabaseSchema err = json.Unmarshal([]byte(` @@ -816,7 +816,7 @@ func TestTableCachePopulate(t *testing.T) { } func TestTableCachePopulate2(t *testing.T) { - db, err := model.NewDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) assert.Nil(t, err) var schema ovsdb.DatabaseSchema err = json.Unmarshal([]byte(` @@ -934,7 +934,7 @@ func TestIndex(t *testing.T) { Bar string `ovsdb:"bar"` Baz int `ovsdb:"baz"` } - db, err := model.NewDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &indexTestModel{}}) + db, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &indexTestModel{}}) assert.Nil(t, err) var schema ovsdb.DatabaseSchema err = json.Unmarshal([]byte(` @@ -1029,7 +1029,7 @@ func TestIndex(t *testing.T) { func TestTableCacheRowByModelSingleIndex(t *testing.T) { var schema ovsdb.DatabaseSchema - db, err := model.NewDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) require.Nil(t, err) err = json.Unmarshal([]byte(` {"name": "TestDB", @@ -1078,7 +1078,7 @@ func TestTableCacheRowByModelSingleIndex(t *testing.T) { func TestTableCacheRowByModelTwoIndexes(t *testing.T) { var schema ovsdb.DatabaseSchema - db, err := model.NewDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) require.Nil(t, err) err = json.Unmarshal([]byte(` {"name": "TestDB", @@ -1129,7 +1129,7 @@ func TestTableCacheRowByModelTwoIndexes(t *testing.T) { func TestTableCacheRowByModelMultiIndex(t *testing.T) { var schema ovsdb.DatabaseSchema - db, err := model.NewDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) require.Nil(t, err) err = json.Unmarshal([]byte(` {"name": "TestDB", @@ -1278,7 +1278,7 @@ func TestTableCacheApplyModifications(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - db, err := model.NewDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) assert.Nil(t, err) var schema ovsdb.DatabaseSchema err = json.Unmarshal([]byte(` diff --git a/client/api.go b/client/api.go index 04fd5c47..55b63cf5 100644 --- a/client/api.go +++ b/client/api.go @@ -203,7 +203,7 @@ func (a api) conditionFromModel(any bool, model model.Model, cond ...model.Condi // Get is a generic Get function capable of returning (through a provided pointer) // a instance of any row in the cache. -// 'result' must be a pointer to an Model that exists in the DBModel +// 'result' must be a pointer to an Model that exists in the DatabaseModelRequest // // The way the cache is searched depends on the fields already populated in 'result' // Any table index (including _uuid) will be used for comparison @@ -280,7 +280,7 @@ func (a api) Mutate(model model.Model, mutationObjs ...model.Mutation) ([]ovsdb. return nil, fmt.Errorf("at least one Mutation must be provided") } - tableName := a.cache.DBModel().FindTable(reflect.ValueOf(model).Type()) + tableName := a.cache.DatabaseModelRequest().FindTable(reflect.ValueOf(model).Type()) if tableName == "" { return nil, fmt.Errorf("table not found for object") } @@ -414,7 +414,7 @@ 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.DBModel().FindTable(reflect.TypeOf(m)) + table := a.cache.DatabaseModelRequest().FindTable(reflect.TypeOf(m)) if table == "" { return "", &ErrWrongType{reflect.TypeOf(m), "Model not found in Database Model"} } @@ -439,7 +439,7 @@ func (a api) getTableFromFunc(predicate interface{}) (string, error) { fmt.Sprintf("Type %s does not implement Model interface", modelType.String())} } - table := a.cache.DBModel().FindTable(modelType) + table := a.cache.DatabaseModelRequest().FindTable(modelType) if table == "" { return "", &ErrWrongType{predType, fmt.Sprintf("Model %s not found in Database Model", modelType.String())} diff --git a/client/api_test_model.go b/client/api_test_model.go index b90541ac..93932842 100644 --- a/client/api_test_model.go +++ b/client/api_test_model.go @@ -157,7 +157,7 @@ func apiTestCache(t *testing.T, data map[string]map[string]model.Model) *cache.T var schema ovsdb.DatabaseSchema err := json.Unmarshal(apiTestSchema, &schema) assert.Nil(t, err) - db, err := model.NewDBModel("OVN_NorthBound", map[string]model.Model{"Logical_Switch": &testLogicalSwitch{}, "Logical_Switch_Port": &testLogicalSwitchPort{}}) + db, err := model.NewDatabaseModelRequest("OVN_NorthBound", map[string]model.Model{"Logical_Switch": &testLogicalSwitch{}, "Logical_Switch_Port": &testLogicalSwitchPort{}}) assert.Nil(t, err) cache, err := cache.NewTableCache(&schema, db, data) assert.Nil(t, err) diff --git a/client/client.go b/client/client.go index fd911b92..95438553 100644 --- a/client/client.go +++ b/client/client.go @@ -86,7 +86,7 @@ type ovsdbClient struct { // database is everything needed to map between go types and an ovsdb Database type database struct { - model *model.DBModel + model *model.DatabaseModelRequest schema *ovsdb.DatabaseSchema schemaMutex sync.RWMutex cache *cache.TableCache @@ -103,12 +103,12 @@ type database struct { // database model. The client can be configured using one or more Option(s), // like WithTLSConfig. If no WithEndpoint option is supplied, the default of // unix:/var/run/openvswitch/ovsdb.sock is used -func NewOVSDBClient(databaseModel *model.DBModel, opts ...Option) (Client, error) { +func NewOVSDBClient(databaseModel *model.DatabaseModelRequest, opts ...Option) (Client, error) { return newOVSDBClient(databaseModel, opts...) } // newOVSDBClient creates a new ovsdbClient -func newOVSDBClient(databaseModel *model.DBModel, opts ...Option) (*ovsdbClient, error) { +func newOVSDBClient(databaseModel *model.DatabaseModelRequest, opts ...Option) (*ovsdbClient, error) { ovs := &ovsdbClient{ primaryDBName: databaseModel.Name(), databases: map[string]*database{ diff --git a/client/client_test.go b/client/client_test.go index 8857c5f8..b64edf7c 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -87,7 +87,7 @@ type OpenvSwitch struct { SystemVersion *string `ovsdb:"system_version"` } -var defDB, _ = model.NewDBModel("Open_vSwitch", +var defDB, _ = model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{ "Open_vSwitch": &OpenvSwitch{}, "Bridge": &Bridge{}, @@ -559,12 +559,12 @@ func BenchmarkUpdate1(b *testing.B) { var s ovsdb.DatabaseSchema err = json.Unmarshal([]byte(schema), &s) require.NoError(b, err) - dbModel, err := model.NewDBModel("Open_vSwitch", map[string]model.Model{ + dbModelReq, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{ "Bridge": &Bridge{}, "Open_vSwitch": &OpenvSwitch{}, }) require.NoError(b, err) - ovs.primaryDB().cache, err = cache.NewTableCache(&s, dbModel, nil) + ovs.primaryDB().cache, err = cache.NewTableCache(&s, dbModelReq, nil) require.NoError(b, err) update := []byte(`{ "Open_vSwitch": { @@ -583,12 +583,12 @@ func BenchmarkUpdate2(b *testing.B) { var s ovsdb.DatabaseSchema err = json.Unmarshal([]byte(schema), &s) require.NoError(b, err) - dbModel, err := model.NewDBModel("Open_vSwitch", map[string]model.Model{ + dbModelReq, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{ "Bridge": &Bridge{}, "Open_vSwitch": &OpenvSwitch{}, }) require.NoError(b, err) - ovs.primaryDB().cache, err = cache.NewTableCache(&s, dbModel, nil) + ovs.primaryDB().cache, err = cache.NewTableCache(&s, dbModelReq, nil) require.NoError(b, err) update := []byte(`{ "Open_vSwitch": { @@ -608,12 +608,12 @@ func BenchmarkUpdate3(b *testing.B) { var s ovsdb.DatabaseSchema err = json.Unmarshal([]byte(schema), &s) require.NoError(b, err) - dbModel, err := model.NewDBModel("Open_vSwitch", map[string]model.Model{ + dbModelReq, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{ "Bridge": &Bridge{}, "Open_vSwitch": &OpenvSwitch{}, }) require.NoError(b, err) - ovs.primaryDB().cache, err = cache.NewTableCache(&s, dbModel, nil) + ovs.primaryDB().cache, err = cache.NewTableCache(&s, dbModelReq, nil) require.NoError(b, err) update := []byte(`{ "Open_vSwitch": { @@ -634,12 +634,12 @@ func BenchmarkUpdate5(b *testing.B) { var s ovsdb.DatabaseSchema err = json.Unmarshal([]byte(schema), &s) require.NoError(b, err) - dbModel, err := model.NewDBModel("Open_vSwitch", map[string]model.Model{ + dbModelReq, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{ "Bridge": &Bridge{}, "Open_vSwitch": &OpenvSwitch{}, }) require.NoError(b, err) - ovs.primaryDB().cache, err = cache.NewTableCache(&s, dbModel, nil) + ovs.primaryDB().cache, err = cache.NewTableCache(&s, dbModelReq, nil) require.NoError(b, err) update := []byte(`{ "Open_vSwitch": { @@ -662,12 +662,12 @@ func BenchmarkUpdate8(b *testing.B) { var s ovsdb.DatabaseSchema err = json.Unmarshal([]byte(schema), &s) require.NoError(b, err) - dbModel, err := model.NewDBModel("Open_vSwitch", map[string]model.Model{ + dbModelReq, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{ "Bridge": &Bridge{}, "Open_vSwitch": &OpenvSwitch{}, }) require.NoError(b, err) - ovs.primaryDB().cache, err = cache.NewTableCache(&s, dbModel, nil) + ovs.primaryDB().cache, err = cache.NewTableCache(&s, dbModelReq, nil) require.NoError(b, err) update := []byte(`{ "Open_vSwitch": { @@ -707,12 +707,12 @@ func TestUpdate(t *testing.T) { var s ovsdb.DatabaseSchema err = json.Unmarshal([]byte(schema), &s) require.NoError(t, err) - dbModel, err := model.NewDBModel("Open_vSwitch", map[string]model.Model{ + dbModelReq, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{ "Bridge": &Bridge{}, "Open_vSwitch": &OpenvSwitch{}, }) require.NoError(t, err) - ovs.primaryDB().cache, err = cache.NewTableCache(&s, dbModel, nil) + ovs.primaryDB().cache, err = cache.NewTableCache(&s, dbModelReq, nil) require.NoError(t, err) var reply []interface{} update := []byte(`{ diff --git a/client/doc.go b/client/doc.go index b5f39f30..9d10ef6b 100644 --- a/client/doc.go +++ b/client/doc.go @@ -11,21 +11,21 @@ which column in the database. We refer to pointers to this structs as Models. Ex Config map[string]string `ovsdb:"other_config"` } -Based on these Models a Database Model (see DBModel type) is built to represent +Based on these Models a Database Model (see DatabaseModelRequest type) is built to represent the entire OVSDB: - dbModel, _ := client.NewDBModel("OVN_Northbound", + dbModelReq, _ := client.NewDatabaseModelRequest("OVN_Northbound", map[string]client.Model{ "Logical_Switch": &MyLogicalSwitch{}, }) -The DBModel represents the entire Database (or the part of it we're interested in). +The DatabaseModelRequest represents the entire Database (or the part of it we're interested in). Using it, the libovsdb.client package is able to properly encode and decode OVSDB messages and store them in Model instances. A client instance is created by simply specifying the connection information and the database model: - ovs, _ := client.Connect(context.Background(), dbModel) + ovs, _ := client.Connect(context.Background(), dbModelReq) Main API diff --git a/client/monitor.go b/client/monitor.go index d2fd7310..9bb36099 100644 --- a/client/monitor.go +++ b/client/monitor.go @@ -72,7 +72,7 @@ func WithTable(m model.Model, fields ...interface{}) MonitorOption { return func(o *ovsdbClient, monitor *Monitor) error { tableName := o.primaryDB().model.FindTable(reflect.TypeOf(m)) if tableName == "" { - return fmt.Errorf("object of type %s is not part of the DBModel", reflect.TypeOf(m)) + return fmt.Errorf("object of type %s is not part of the DatabaseModelRequest", reflect.TypeOf(m)) } tableMonitor := TableMonitor{ Table: tableName, @@ -87,7 +87,7 @@ func WithConditionalTable(m model.Model, condition model.Condition, fields ...in return func(o *ovsdbClient, monitor *Monitor) error { tableName := o.primaryDB().model.FindTable(reflect.TypeOf(m)) if tableName == "" { - return fmt.Errorf("object of type %s is not part of the DBModel", reflect.TypeOf(m)) + return fmt.Errorf("object of type %s is not part of the DatabaseModelRequest", reflect.TypeOf(m)) } tableMonitor := TableMonitor{ Table: tableName, diff --git a/cmd/stress/stress.go b/cmd/stress/stress.go index 511bb724..cafafcc9 100644 --- a/cmd/stress/stress.go +++ b/cmd/stress/stress.go @@ -43,7 +43,7 @@ var ( parallel = flag.Bool("parallel", false, "run clients in parallel") verbose = flag.Bool("verbose", false, "Be verbose") connection = flag.String("ovsdb", "unix:/var/run/openvswitch/db.sock", "OVSDB connection string") - dbModel *model.DBModel + dbModelReq *model.DatabaseModelRequest ) type result struct { @@ -54,7 +54,7 @@ type result struct { } func cleanup(ctx context.Context) { - ovs, err := client.NewOVSDBClient(dbModel, client.WithEndpoint(*connection)) + ovs, err := client.NewOVSDBClient(dbModelReq, client.WithEndpoint(*connection)) if err != nil { log.Fatal(err) } @@ -96,7 +96,7 @@ func run(ctx context.Context, resultsChan chan result, wg *sync.WaitGroup) { ready := false var rootUUID string - ovs, err := client.NewOVSDBClient(dbModel, client.WithEndpoint(*connection)) + ovs, err := client.NewOVSDBClient(dbModelReq, client.WithEndpoint(*connection)) if err != nil { log.Fatal(err) } @@ -249,7 +249,7 @@ func main() { } var err error - dbModel, err = model.NewDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &ovsType{}, "Bridge": &bridgeType{}}) + dbModelReq, err = model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &ovsType{}, "Bridge": &bridgeType{}}) if err != nil { log.Fatal(err) } diff --git a/example/ovsdb-server/main.go b/example/ovsdb-server/main.go index 63e6b552..68eb99e0 100644 --- a/example/ovsdb-server/main.go +++ b/example/ovsdb-server/main.go @@ -39,7 +39,7 @@ func main() { defer pprof.StopCPUProfile() } - dbModel, err := vswitchd.FullDatabaseModel() + dbModelReq, err := vswitchd.FullDatabaseModel() if err != nil { log.Fatal(err) } @@ -57,12 +57,12 @@ func main() { log.Fatal(err) } - ovsDB := server.NewInMemoryDatabase(map[string]*model.DBModel{ - schema.Name: dbModel, + ovsDB := server.NewInMemoryDatabase(map[string]*model.DatabaseModelRequest{ + schema.Name: dbModelReq, }) s, err := server.NewOvsdbServer(ovsDB, server.DatabaseModel{ - Model: dbModel, + Model: dbModelReq, Schema: schema, }) if err != nil { @@ -80,7 +80,7 @@ func main() { }(s) time.Sleep(1 * time.Second) - c, err := client.NewOVSDBClient(dbModel, client.WithEndpoint(fmt.Sprintf("tcp::%d", *port))) + c, err := client.NewOVSDBClient(dbModelReq, client.WithEndpoint(fmt.Sprintf("tcp::%d", *port))) if err != nil { log.Fatal(err) } diff --git a/example/play_with_ovs/main.go b/example/play_with_ovs/main.go index b4e1bd33..deda060c 100644 --- a/example/play_with_ovs/main.go +++ b/example/play_with_ovs/main.go @@ -97,13 +97,13 @@ func main() { quit = make(chan bool) update = make(chan model.Model) - dbModel, err := model.NewDBModel("Open_vSwitch", + dbModelReq, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{bridgeTable: &vswitchd.Bridge{}, ovsTable: &vswitchd.OpenvSwitch{}}) if err != nil { log.Fatal("Unable to create DB model ", err) } - ovs, err := client.NewOVSDBClient(dbModel, client.WithEndpoint(*connection)) + ovs, err := client.NewOVSDBClient(dbModelReq, client.WithEndpoint(*connection)) if err != nil { log.Fatal(err) } diff --git a/model/model.go b/model/model.go index ef77fc9d..453fd2ea 100644 --- a/model/model.go +++ b/model/model.go @@ -5,7 +5,6 @@ import ( "fmt" "reflect" - "github.com/ovn-org/libovsdb/mapper" "github.com/ovn-org/libovsdb/ovsdb" ) @@ -34,99 +33,6 @@ func Clone(a Model) Model { return b } -// DBModel is a Database model -type DBModel struct { - name string - types map[string]reflect.Type -} - -// NewModel returns a new instance of a model from a specific string -func (db DBModel) NewModel(table string) (Model, error) { - mtype, ok := db.types[table] - if !ok { - return nil, fmt.Errorf("table %s not found in database model", string(table)) - } - model := reflect.New(mtype.Elem()) - return model.Interface().(Model), nil -} - -// Types returns the DBModel Types -// the DBModel types is a map of reflect.Types indexed by string -// The reflect.Type is a pointer to a struct that contains 'ovs' tags -// as described above. Such pointer to struct also implements the Model interface -func (db DBModel) Types() map[string]reflect.Type { - return db.types -} - -// Name returns the database name -func (db DBModel) Name() string { - return db.name -} - -// FindTable returns the string associated with a reflect.Type or "" -func (db DBModel) FindTable(mType reflect.Type) string { - for table, tType := range db.types { - if tType == mType { - return table - } - } - return "" -} - -// Validate validates the DatabaseModel against the input schema -// Returns all the errors detected -func (db DBModel) Validate(schema *ovsdb.DatabaseSchema) []error { - var errors []error - if db.name != schema.Name { - errors = append(errors, fmt.Errorf("database model name (%s) does not match schema (%s)", - db.name, schema.Name)) - } - - for tableName := range db.types { - tableSchema := schema.Table(tableName) - if tableSchema == nil { - errors = append(errors, fmt.Errorf("database model contains a model for table %s that does not exist in schema", tableName)) - continue - } - model, err := db.NewModel(tableName) - if err != nil { - errors = append(errors, err) - continue - } - if _, err := mapper.NewInfo(tableSchema, model); err != nil { - errors = append(errors, err) - } - } - return errors -} - -// NewDBModel constructs a DBModel based on a database name and dictionary of models indexed by table name -func NewDBModel(name string, models map[string]Model) (*DBModel, error) { - types := make(map[string]reflect.Type, len(models)) - for table, model := range models { - modelType := reflect.TypeOf(model) - if modelType.Kind() != reflect.Ptr || modelType.Elem().Kind() != reflect.Struct { - return nil, fmt.Errorf("model is expected to be a pointer to struct") - } - hasUUID := false - for i := 0; i < modelType.Elem().NumField(); i++ { - if field := modelType.Elem().Field(i); field.Tag.Get("ovsdb") == "_uuid" && - field.Type.Kind() == reflect.String { - hasUUID = true - } - } - if !hasUUID { - return nil, fmt.Errorf("model is expected to have a string field called uuid") - } - - types[table] = reflect.TypeOf(model) - } - return &DBModel{ - types: types, - name: name, - }, nil -} - func modelSetUUID(model Model, uuid string) error { modelVal := reflect.ValueOf(model).Elem() for i := 0; i < modelVal.NumField(); i++ { diff --git a/model/model_test.go b/model/model_test.go index 6eaaee41..bbfc1a77 100644 --- a/model/model_test.go +++ b/model/model_test.go @@ -23,7 +23,7 @@ type modelInvalid struct { Foo string } -func TestDBModel(t *testing.T) { +func TestDatabaseModelRequest(t *testing.T) { type Test struct { name string obj map[string]Model @@ -50,7 +50,7 @@ func TestDBModel(t *testing.T) { } for _, tt := range tests { t.Run(fmt.Sprintf("TestNewModel_%s", tt.name), func(t *testing.T) { - db, err := NewDBModel(tt.name, tt.obj) + db, err := NewDatabaseModelRequest(tt.name, tt.obj) if tt.valid { assert.Nil(t, err) assert.Len(t, db.Types(), len(tt.obj)) @@ -63,7 +63,7 @@ func TestDBModel(t *testing.T) { } func TestNewModel(t *testing.T) { - db, err := NewDBModel("testTable", map[string]Model{"Test_A": &modelA{}, "Test_B": &modelB{}}) + db, err := NewDatabaseModelRequest("testTable", map[string]Model{"Test_A": &modelA{}, "Test_B": &modelB{}}) assert.Nil(t, err) _, err = db.NewModel("Unknown") assert.NotNilf(t, err, "Creating model from unknown table should fail") @@ -86,7 +86,7 @@ func TestSetUUID(t *testing.T) { } func TestValidate(t *testing.T) { - model, err := NewDBModel("TestDB", map[string]Model{ + model, err := NewDatabaseModelRequest("TestDB", map[string]Model{ "TestTable": &struct { aUUID string `ovsdb:"_uuid"` aString string `ovsdb:"aString"` diff --git a/model/request.go b/model/request.go new file mode 100644 index 00000000..be303480 --- /dev/null +++ b/model/request.go @@ -0,0 +1,102 @@ +package model + +import ( + "fmt" + "reflect" + + "github.com/ovn-org/libovsdb/mapper" + "github.com/ovn-org/libovsdb/ovsdb" +) + +// DatabaseModelRequest contains the information needed to build a DatabaseModel +type DatabaseModelRequest struct { + name string + types map[string]reflect.Type +} + +// NewModel returns a new instance of a model from a specific string +func (db DatabaseModelRequest) NewModel(table string) (Model, error) { + mtype, ok := db.types[table] + if !ok { + return nil, fmt.Errorf("table %s not found in database model", string(table)) + } + model := reflect.New(mtype.Elem()) + return model.Interface().(Model), nil +} + +// Types returns the DatabaseModelRequest Types +// the DatabaseModelRequest types is a map of reflect.Types indexed by string +// The reflect.Type is a pointer to a struct that contains 'ovs' tags +// as described above. Such pointer to struct also implements the Model interface +func (db DatabaseModelRequest) Types() map[string]reflect.Type { + return db.types +} + +// Name returns the database name +func (db DatabaseModelRequest) Name() string { + return db.name +} + +// FindTable returns the string associated with a reflect.Type or "" +func (db DatabaseModelRequest) FindTable(mType reflect.Type) string { + for table, tType := range db.types { + if tType == mType { + return table + } + } + return "" +} + +// Validate validates the DatabaseModel against the input schema +// Returns all the errors detected +func (db DatabaseModelRequest) Validate(schema *ovsdb.DatabaseSchema) []error { + var errors []error + if db.name != schema.Name { + errors = append(errors, fmt.Errorf("database model name (%s) does not match schema (%s)", + db.name, schema.Name)) + } + + for tableName := range db.types { + tableSchema := schema.Table(tableName) + if tableSchema == nil { + errors = append(errors, fmt.Errorf("database model contains a model for table %s that does not exist in schema", tableName)) + continue + } + model, err := db.NewModel(tableName) + if err != nil { + errors = append(errors, err) + continue + } + if _, err := mapper.NewInfo(tableSchema, model); err != nil { + errors = append(errors, err) + } + } + return errors +} + +// NewDatabaseModelRequest constructs a DatabaseModelRequest based on a database name and dictionary of models indexed by table name +func NewDatabaseModelRequest(name string, models map[string]Model) (*DatabaseModelRequest, error) { + types := make(map[string]reflect.Type, len(models)) + for table, model := range models { + modelType := reflect.TypeOf(model) + if modelType.Kind() != reflect.Ptr || modelType.Elem().Kind() != reflect.Struct { + return nil, fmt.Errorf("model is expected to be a pointer to struct") + } + hasUUID := false + for i := 0; i < modelType.Elem().NumField(); i++ { + if field := modelType.Elem().Field(i); field.Tag.Get("ovsdb") == "_uuid" && + field.Type.Kind() == reflect.String { + hasUUID = true + } + } + if !hasUUID { + return nil, fmt.Errorf("model is expected to have a string field called uuid") + } + + types[table] = reflect.TypeOf(model) + } + return &DatabaseModelRequest{ + types: types, + name: name, + }, nil +} diff --git a/modelgen/dbmodel.go b/modelgen/dbmodel.go index bdff8a61..35faa9e1 100644 --- a/modelgen/dbmodel.go +++ b/modelgen/dbmodel.go @@ -8,7 +8,7 @@ import ( "github.com/ovn-org/libovsdb/ovsdb" ) -// NewDBTemplate return a new DBModel template +// NewDBTemplate return a new DatabaseModelRequest template // It includes the following other templates that can be overridden to customize the generated file // "header" // "preDBDefinitions" @@ -40,8 +40,8 @@ package {{ index . "PackageName" }} {{ template "preDBDefinitions" }} // FullDatabaseModel returns the DatabaseModel object to be used in libovsdb -func FullDatabaseModel() (*model.DBModel, error) { - return model.NewDBModel("{{ index . "DatabaseName" }}", map[string]model.Model{ +func FullDatabaseModel() (*model.DatabaseModelRequest, error) { + return model.NewDatabaseModelRequest("{{ index . "DatabaseName" }}", map[string]model.Model{ {{ range index . "Tables" }} "{{ .TableName }}" : &{{ .StructName }}{}, {{ end }} }) diff --git a/modelgen/dbmodel_test.go b/modelgen/dbmodel_test.go index b9ca9191..8b3d6093 100644 --- a/modelgen/dbmodel_test.go +++ b/modelgen/dbmodel_test.go @@ -59,8 +59,8 @@ import ( ) // FullDatabaseModel returns the DatabaseModel object to be used in libovsdb -func FullDatabaseModel() (*model.DBModel, error) { - return model.NewDBModel("AtomicDB", map[string]model.Model{ +func FullDatabaseModel() (*model.DatabaseModelRequest, error) { + return model.NewDatabaseModelRequest("AtomicDB", map[string]model.Model{ "atomicTable": &AtomicTable{}, }) } diff --git a/ovsdb/serverdb/model.go b/ovsdb/serverdb/model.go index 511112f8..7a8dc9a3 100644 --- a/ovsdb/serverdb/model.go +++ b/ovsdb/serverdb/model.go @@ -11,8 +11,8 @@ import ( ) // FullDatabaseModel returns the DatabaseModel object to be used in libovsdb -func FullDatabaseModel() (*model.DBModel, error) { - return model.NewDBModel("_Server", map[string]model.Model{ +func FullDatabaseModel() (*model.DatabaseModelRequest, error) { + return model.NewDatabaseModelRequest("_Server", map[string]model.Model{ "Database": &Database{}, }) } diff --git a/server/database.go b/server/database.go index ffb7c0ea..b15c1a05 100644 --- a/server/database.go +++ b/server/database.go @@ -22,11 +22,11 @@ type Database interface { type inMemoryDatabase struct { databases map[string]*cache.TableCache - models map[string]*model.DBModel + models map[string]*model.DatabaseModelRequest mutex sync.RWMutex } -func NewInMemoryDatabase(models map[string]*model.DBModel) Database { +func NewInMemoryDatabase(models map[string]*model.DatabaseModelRequest) Database { return &inMemoryDatabase{ databases: make(map[string]*cache.TableCache), models: models, @@ -37,7 +37,7 @@ func NewInMemoryDatabase(models map[string]*model.DBModel) Database { func (db *inMemoryDatabase) CreateDatabase(name string, schema *ovsdb.DatabaseSchema) error { db.mutex.Lock() defer db.mutex.Unlock() - var mo *model.DBModel + var mo *model.DatabaseModelRequest var ok bool if mo, ok = db.models[schema.Name]; !ok { return fmt.Errorf("no db model provided for schema with name %s", name) diff --git a/server/server.go b/server/server.go index 3450a8ff..7acfe435 100644 --- a/server/server.go +++ b/server/server.go @@ -29,7 +29,7 @@ type OvsdbServer struct { } type DatabaseModel struct { - Model *model.DBModel + Model *model.DatabaseModelRequest Schema *ovsdb.DatabaseSchema } @@ -141,7 +141,7 @@ type Transaction struct { Cache *cache.TableCache } -func NewTransaction(schema *ovsdb.DatabaseSchema, model *model.DBModel) Transaction { +func NewTransaction(schema *ovsdb.DatabaseSchema, model *model.DatabaseModelRequest) Transaction { cache, err := cache.NewTableCache(schema, model, nil) if err != nil { panic(err) diff --git a/server/server_integration_test.go b/server/server_integration_test.go index 24a0a934..c3ecd811 100644 --- a/server/server_integration_test.go +++ b/server/server_integration_test.go @@ -56,7 +56,7 @@ func getSchema() (*ovsdb.DatabaseSchema, error) { } func TestClientServerEcho(t *testing.T) { - defDB, err := model.NewDBModel("Open_vSwitch", map[string]model.Model{ + defDB, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{ "Open_vSwitch": &ovsType{}, "Bridge": &bridgeType{}}) require.Nil(t, err) @@ -64,7 +64,7 @@ func TestClientServerEcho(t *testing.T) { schema, err := getSchema() require.Nil(t, err) - ovsDB := NewInMemoryDatabase(map[string]*model.DBModel{"Open_vSwitch": defDB}) + ovsDB := NewInMemoryDatabase(map[string]*model.DatabaseModelRequest{"Open_vSwitch": defDB}) rand.Seed(time.Now().UnixNano()) tmpfile := fmt.Sprintf("/tmp/ovsdb-%d.sock", rand.Intn(10000)) @@ -94,7 +94,7 @@ func TestClientServerEcho(t *testing.T) { } func TestClientServerInsert(t *testing.T) { - defDB, err := model.NewDBModel("Open_vSwitch", map[string]model.Model{ + defDB, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{ "Open_vSwitch": &ovsType{}, "Bridge": &bridgeType{}}) require.Nil(t, err) @@ -102,7 +102,7 @@ func TestClientServerInsert(t *testing.T) { schema, err := getSchema() require.Nil(t, err) - ovsDB := NewInMemoryDatabase(map[string]*model.DBModel{"Open_vSwitch": defDB}) + ovsDB := NewInMemoryDatabase(map[string]*model.DatabaseModelRequest{"Open_vSwitch": defDB}) rand.Seed(time.Now().UnixNano()) tmpfile := fmt.Sprintf("/tmp/ovsdb-%d.sock", rand.Intn(10000)) defer os.Remove(tmpfile) @@ -162,7 +162,7 @@ func TestClientServerInsert(t *testing.T) { } func TestClientServerMonitor(t *testing.T) { - defDB, err := model.NewDBModel("Open_vSwitch", map[string]model.Model{ + defDB, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{ "Open_vSwitch": &ovsType{}, "Bridge": &bridgeType{}}) if err != nil { @@ -174,7 +174,7 @@ func TestClientServerMonitor(t *testing.T) { t.Fatal(err) } - ovsDB := NewInMemoryDatabase(map[string]*model.DBModel{"Open_vSwitch": defDB}) + ovsDB := NewInMemoryDatabase(map[string]*model.DatabaseModelRequest{"Open_vSwitch": defDB}) rand.Seed(time.Now().UnixNano()) tmpfile := fmt.Sprintf("/tmp/ovsdb-%d.sock", rand.Intn(10000)) defer os.Remove(tmpfile) @@ -289,7 +289,7 @@ func TestClientServerMonitor(t *testing.T) { } func TestClientServerInsertAndDelete(t *testing.T) { - defDB, err := model.NewDBModel("Open_vSwitch", map[string]model.Model{ + defDB, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{ "Open_vSwitch": &ovsType{}, "Bridge": &bridgeType{}}) require.Nil(t, err) @@ -297,7 +297,7 @@ func TestClientServerInsertAndDelete(t *testing.T) { schema, err := getSchema() require.Nil(t, err) - ovsDB := NewInMemoryDatabase(map[string]*model.DBModel{"Open_vSwitch": defDB}) + ovsDB := NewInMemoryDatabase(map[string]*model.DatabaseModelRequest{"Open_vSwitch": defDB}) rand.Seed(time.Now().UnixNano()) tmpfile := fmt.Sprintf("/tmp/ovsdb-%d.sock", rand.Intn(10000)) defer os.Remove(tmpfile) @@ -355,7 +355,7 @@ func TestClientServerInsertAndDelete(t *testing.T) { } func TestClientServerInsertDuplicate(t *testing.T) { - defDB, err := model.NewDBModel("Open_vSwitch", map[string]model.Model{ + defDB, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{ "Open_vSwitch": &ovsType{}, "Bridge": &bridgeType{}, }) @@ -364,7 +364,7 @@ func TestClientServerInsertDuplicate(t *testing.T) { schema, err := getSchema() require.Nil(t, err) - ovsDB := NewInMemoryDatabase(map[string]*model.DBModel{"Open_vSwitch": defDB}) + ovsDB := NewInMemoryDatabase(map[string]*model.DatabaseModelRequest{"Open_vSwitch": defDB}) rand.Seed(time.Now().UnixNano()) tmpfile := fmt.Sprintf("/tmp/ovsdb-%d.sock", rand.Intn(10000)) defer os.Remove(tmpfile) @@ -411,7 +411,7 @@ func TestClientServerInsertDuplicate(t *testing.T) { } func TestClientServerInsertAndUpdate(t *testing.T) { - defDB, err := model.NewDBModel("Open_vSwitch", map[string]model.Model{ + defDB, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{ "Open_vSwitch": &ovsType{}, "Bridge": &bridgeType{}}) require.Nil(t, err) @@ -419,7 +419,7 @@ func TestClientServerInsertAndUpdate(t *testing.T) { schema, err := getSchema() require.Nil(t, err) - ovsDB := NewInMemoryDatabase(map[string]*model.DBModel{"Open_vSwitch": defDB}) + ovsDB := NewInMemoryDatabase(map[string]*model.DatabaseModelRequest{"Open_vSwitch": defDB}) rand.Seed(time.Now().UnixNano()) tmpfile := fmt.Sprintf("/tmp/ovsdb-%d.sock", rand.Intn(10000)) defer os.Remove(tmpfile) diff --git a/server/server_test.go b/server/server_test.go index 604300d0..7081c386 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -66,7 +66,7 @@ func TestExpandNamedUUID(t *testing.T) { } func TestOvsdbServerMonitor(t *testing.T) { - defDB, err := model.NewDBModel("Open_vSwitch", map[string]model.Model{ + defDB, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{ "Open_vSwitch": &ovsType{}, "Bridge": &bridgeType{}}) if err != nil { @@ -76,7 +76,7 @@ func TestOvsdbServerMonitor(t *testing.T) { if err != nil { t.Fatal(err) } - ovsDB := NewInMemoryDatabase(map[string]*model.DBModel{"Open_vSwitch": defDB}) + ovsDB := NewInMemoryDatabase(map[string]*model.DatabaseModelRequest{"Open_vSwitch": defDB}) o, err := NewOvsdbServer(ovsDB, DatabaseModel{ Model: defDB, Schema: schema}) require.Nil(t, err) diff --git a/server/transact_test.go b/server/transact_test.go index 926950d7..d52bbe17 100644 --- a/server/transact_test.go +++ b/server/transact_test.go @@ -12,7 +12,7 @@ import ( ) func TestMutateOp(t *testing.T) { - defDB, err := model.NewDBModel("Open_vSwitch", map[string]model.Model{ + defDB, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{ "Open_vSwitch": &ovsType{}, "Bridge": &bridgeType{}}) if err != nil { @@ -22,7 +22,7 @@ func TestMutateOp(t *testing.T) { if err != nil { t.Fatal(err) } - ovsDB := NewInMemoryDatabase(map[string]*model.DBModel{"Open_vSwitch": defDB}) + ovsDB := NewInMemoryDatabase(map[string]*model.DatabaseModelRequest{"Open_vSwitch": defDB}) o, err := NewOvsdbServer(ovsDB, DatabaseModel{ Model: defDB, Schema: schema}) require.Nil(t, err) @@ -204,7 +204,7 @@ func TestDiff(t *testing.T) { func TestOvsdbServerInsert(t *testing.T) { t.Skip("need a helper for comparing rows as map elements aren't in same order") - defDB, err := model.NewDBModel("Open_vSwitch", map[string]model.Model{ + defDB, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{ "Open_vSwitch": &ovsType{}, "Bridge": &bridgeType{}}) if err != nil { @@ -214,7 +214,7 @@ func TestOvsdbServerInsert(t *testing.T) { if err != nil { t.Fatal(err) } - ovsDB := NewInMemoryDatabase(map[string]*model.DBModel{"Open_vSwitch": defDB}) + ovsDB := NewInMemoryDatabase(map[string]*model.DatabaseModelRequest{"Open_vSwitch": defDB}) o, err := NewOvsdbServer(ovsDB, DatabaseModel{ Model: defDB, Schema: schema}) require.Nil(t, err) @@ -257,7 +257,7 @@ func TestOvsdbServerInsert(t *testing.T) { } func TestOvsdbServerUpdate(t *testing.T) { - defDB, err := model.NewDBModel("Open_vSwitch", map[string]model.Model{ + defDB, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{ "Open_vSwitch": &ovsType{}, "Bridge": &bridgeType{}}) if err != nil { @@ -267,7 +267,7 @@ func TestOvsdbServerUpdate(t *testing.T) { if err != nil { t.Fatal(err) } - ovsDB := NewInMemoryDatabase(map[string]*model.DBModel{"Open_vSwitch": defDB}) + ovsDB := NewInMemoryDatabase(map[string]*model.DatabaseModelRequest{"Open_vSwitch": defDB}) o, err := NewOvsdbServer(ovsDB, DatabaseModel{ Model: defDB, Schema: schema}) require.Nil(t, err) diff --git a/test/ovs/ovs_integration_test.go b/test/ovs/ovs_integration_test.go index 1bf20d95..5246295e 100644 --- a/test/ovs/ovs_integration_test.go +++ b/test/ovs/ovs_integration_test.go @@ -167,7 +167,7 @@ type queueType struct { DSCP *int `ovsdb:"dscp"` } -var defDB, _ = model.NewDBModel("Open_vSwitch", map[string]model.Model{ +var defDB, _ = model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{ "Open_vSwitch": &ovsType{}, "Bridge": &bridgeType{}, "IPFIX": &ipfixType{}, From d1894d4634cf7ab553c42a79415edfc3f6a477cd Mon Sep 17 00:00:00 2001 From: Adrian Moreno Date: Thu, 7 Oct 2021 17:38:26 +0200 Subject: [PATCH 04/10] model: Introduce DatabaseModel Replace the one that server.go had to define. For now, it's just a drop-in replacement of the previous type Signed-off-by: Adrian Moreno --- example/ovsdb-server/main.go | 5 +--- model/database.go | 38 +++++++++++++++++++++++++++++++ server/server.go | 22 +++++++++--------- server/server_integration_test.go | 30 +++++------------------- server/server_test.go | 3 +-- server/transact.go | 24 +++++++++---------- server/transact_test.go | 3 +-- 7 files changed, 70 insertions(+), 55 deletions(-) create mode 100644 model/database.go diff --git a/example/ovsdb-server/main.go b/example/ovsdb-server/main.go index 68eb99e0..52fe2724 100644 --- a/example/ovsdb-server/main.go +++ b/example/ovsdb-server/main.go @@ -61,10 +61,7 @@ func main() { schema.Name: dbModelReq, }) - s, err := server.NewOvsdbServer(ovsDB, server.DatabaseModel{ - Model: dbModelReq, - Schema: schema, - }) + s, err := server.NewOvsdbServer(ovsDB, *model.NewDatabaseModel(schema, dbModelReq)) if err != nil { log.Fatal(err) } diff --git a/model/database.go b/model/database.go new file mode 100644 index 00000000..1b13ea5a --- /dev/null +++ b/model/database.go @@ -0,0 +1,38 @@ +package model + +import ( + "github.com/ovn-org/libovsdb/mapper" + "github.com/ovn-org/libovsdb/ovsdb" +) + +// A DatabaseModel represents libovsdb's metadata about the database. +// It's the result of combining the client's DatabaseModelRequest and the server's Schema +type DatabaseModel struct { + request *DatabaseModelRequest + schema *ovsdb.DatabaseSchema + mapper *mapper.Mapper +} + +// NewDatabaseModel returns a new DatabaseModel +func NewDatabaseModel(schema *ovsdb.DatabaseSchema, request *DatabaseModelRequest) *DatabaseModel { + return &DatabaseModel{ + request: request, + schema: schema, + mapper: mapper.NewMapper(schema), + } +} + +// Request returns the DatabaseModel's request +func (db *DatabaseModel) Request() *DatabaseModelRequest { + return db.request +} + +// Schema returns the DatabaseModel's schema +func (db *DatabaseModel) Schema() *ovsdb.DatabaseSchema { + return db.schema +} + +// Mapper returns the DatabaseModel's mapper +func (db *DatabaseModel) Mapper() *mapper.Mapper { + return db.mapper +} diff --git a/server/server.go b/server/server.go index 7acfe435..3a198c6c 100644 --- a/server/server.go +++ b/server/server.go @@ -22,34 +22,34 @@ type OvsdbServer struct { db Database ready bool readyMutex sync.RWMutex - models map[string]DatabaseModel + models map[string]model.DatabaseModel modelsMutex sync.RWMutex monitors map[*rpc2.Client]*connectionMonitors monitorMutex sync.RWMutex } -type DatabaseModel struct { - Model *model.DatabaseModelRequest - Schema *ovsdb.DatabaseSchema -} +//type DatabaseModel struct { +// Model *model.DatabaseModelRequest +// Schema *ovsdb.DatabaseSchema +//} // NewOvsdbServer returns a new OvsdbServer -func NewOvsdbServer(db Database, models ...DatabaseModel) (*OvsdbServer, error) { +func NewOvsdbServer(db Database, models ...model.DatabaseModel) (*OvsdbServer, error) { o := &OvsdbServer{ done: make(chan struct{}, 1), db: db, - models: make(map[string]DatabaseModel), + models: make(map[string]model.DatabaseModel), modelsMutex: sync.RWMutex{}, monitors: make(map[*rpc2.Client]*connectionMonitors), monitorMutex: sync.RWMutex{}, } o.modelsMutex.Lock() for _, model := range models { - o.models[model.Schema.Name] = model + o.models[model.Schema().Name] = model } o.modelsMutex.Unlock() for database, model := range o.models { - if err := o.db.CreateDatabase(database, model.Schema); err != nil { + if err := o.db.CreateDatabase(database, model.Schema()); err != nil { return nil, err } } @@ -113,7 +113,7 @@ func (o *OvsdbServer) ListDatabases(client *rpc2.Client, args []interface{}, rep dbs := []string{} o.modelsMutex.RLock() for _, db := range o.models { - dbs = append(dbs, db.Schema.Name) + dbs = append(dbs, db.Schema().Name) } o.modelsMutex.RUnlock() *reply = dbs @@ -132,7 +132,7 @@ func (o *OvsdbServer) GetSchema(client *rpc2.Client, args []interface{}, reply * return fmt.Errorf("database %s does not exist", db) } o.modelsMutex.RUnlock() - *reply = *model.Schema + *reply = *model.Schema() return nil } diff --git a/server/server_integration_test.go b/server/server_integration_test.go index c3ecd811..9ac74b71 100644 --- a/server/server_integration_test.go +++ b/server/server_integration_test.go @@ -69,10 +69,7 @@ func TestClientServerEcho(t *testing.T) { rand.Seed(time.Now().UnixNano()) tmpfile := fmt.Sprintf("/tmp/ovsdb-%d.sock", rand.Intn(10000)) defer os.Remove(tmpfile) - server, err := NewOvsdbServer(ovsDB, DatabaseModel{ - Model: defDB, - Schema: schema, - }) + server, err := NewOvsdbServer(ovsDB, *model.NewDatabaseModel(schema, defDB)) require.Nil(t, err) go func(t *testing.T, o *OvsdbServer) { @@ -106,10 +103,7 @@ func TestClientServerInsert(t *testing.T) { rand.Seed(time.Now().UnixNano()) tmpfile := fmt.Sprintf("/tmp/ovsdb-%d.sock", rand.Intn(10000)) defer os.Remove(tmpfile) - server, err := NewOvsdbServer(ovsDB, DatabaseModel{ - Model: defDB, - Schema: schema, - }) + server, err := NewOvsdbServer(ovsDB, *model.NewDatabaseModel(schema, defDB)) assert.Nil(t, err) go func(t *testing.T, o *OvsdbServer) { @@ -178,10 +172,7 @@ func TestClientServerMonitor(t *testing.T) { rand.Seed(time.Now().UnixNano()) tmpfile := fmt.Sprintf("/tmp/ovsdb-%d.sock", rand.Intn(10000)) defer os.Remove(tmpfile) - server, err := NewOvsdbServer(ovsDB, DatabaseModel{ - Model: defDB, - Schema: schema, - }) + server, err := NewOvsdbServer(ovsDB, *model.NewDatabaseModel(schema, defDB)) assert.Nil(t, err) go func(t *testing.T, o *OvsdbServer) { @@ -301,10 +292,7 @@ func TestClientServerInsertAndDelete(t *testing.T) { rand.Seed(time.Now().UnixNano()) tmpfile := fmt.Sprintf("/tmp/ovsdb-%d.sock", rand.Intn(10000)) defer os.Remove(tmpfile) - server, err := NewOvsdbServer(ovsDB, DatabaseModel{ - Model: defDB, - Schema: schema, - }) + server, err := NewOvsdbServer(ovsDB, *model.NewDatabaseModel(schema, defDB)) assert.Nil(t, err) go func(t *testing.T, o *OvsdbServer) { @@ -368,10 +356,7 @@ func TestClientServerInsertDuplicate(t *testing.T) { rand.Seed(time.Now().UnixNano()) tmpfile := fmt.Sprintf("/tmp/ovsdb-%d.sock", rand.Intn(10000)) defer os.Remove(tmpfile) - server, err := NewOvsdbServer(ovsDB, DatabaseModel{ - Model: defDB, - Schema: schema, - }) + server, err := NewOvsdbServer(ovsDB, *model.NewDatabaseModel(schema, defDB)) assert.Nil(t, err) go func(t *testing.T, o *OvsdbServer) { @@ -423,10 +408,7 @@ func TestClientServerInsertAndUpdate(t *testing.T) { rand.Seed(time.Now().UnixNano()) tmpfile := fmt.Sprintf("/tmp/ovsdb-%d.sock", rand.Intn(10000)) defer os.Remove(tmpfile) - server, err := NewOvsdbServer(ovsDB, DatabaseModel{ - Model: defDB, - Schema: schema, - }) + server, err := NewOvsdbServer(ovsDB, *model.NewDatabaseModel(schema, defDB)) assert.Nil(t, err) go func(t *testing.T, o *OvsdbServer) { diff --git a/server/server_test.go b/server/server_test.go index 7081c386..9944a3ff 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -77,8 +77,7 @@ func TestOvsdbServerMonitor(t *testing.T) { t.Fatal(err) } ovsDB := NewInMemoryDatabase(map[string]*model.DatabaseModelRequest{"Open_vSwitch": defDB}) - o, err := NewOvsdbServer(ovsDB, DatabaseModel{ - Model: defDB, Schema: schema}) + o, err := NewOvsdbServer(ovsDB, *model.NewDatabaseModel(schema, defDB)) require.Nil(t, err) requests := make(map[string]ovsdb.MonitorRequest) for table, tableSchema := range schema.Tables { diff --git a/server/transact.go b/server/transact.go index 5fc9b598..8ec0a091 100644 --- a/server/transact.go +++ b/server/transact.go @@ -75,14 +75,14 @@ func (o *OvsdbServer) Insert(database string, table string, rowUUID string, row dbModel := o.models[database] o.modelsMutex.Unlock() - m := mapper.NewMapper(dbModel.Schema) - tSchema := dbModel.Schema.Table(table) + m := dbModel.Mapper() + tSchema := dbModel.Schema().Table(table) if rowUUID == "" { rowUUID = uuid.NewString() } - model, err := dbModel.Model.NewModel(table) + model, err := dbModel.Request().NewModel(table) if err != nil { return ovsdb.OperationResult{ Error: err.Error(), @@ -155,7 +155,7 @@ func (o *OvsdbServer) Select(database string, table string, where []ovsdb.Condit dbModel := o.models[database] o.modelsMutex.Unlock() - m := mapper.NewMapper(dbModel.Schema) + m := dbModel.Mapper() var results []ovsdb.Row rows, err := o.db.List(database, table, where...) @@ -184,8 +184,8 @@ func (o *OvsdbServer) Update(database, table string, where []ovsdb.Condition, ro dbModel := o.models[database] o.modelsMutex.Unlock() - m := mapper.NewMapper(dbModel.Schema) - schema := dbModel.Schema.Table(table) + m := dbModel.Mapper() + schema := dbModel.Schema().Table(table) tableUpdate := make(ovsdb.TableUpdate2) rows, err := o.db.List(database, table, where...) if err != nil { @@ -201,7 +201,7 @@ func (o *OvsdbServer) Update(database, table string, where []ovsdb.Condition, ro if err != nil { panic(err) } - new, err := dbModel.Model.NewModel(table) + new, err := dbModel.Request().NewModel(table) if err != nil { panic(err) } @@ -313,8 +313,8 @@ func (o *OvsdbServer) Mutate(database, table string, where []ovsdb.Condition, mu dbModel := o.models[database] o.modelsMutex.Unlock() - m := mapper.NewMapper(dbModel.Schema) - schema := dbModel.Schema.Table(table) + m := dbModel.Mapper() + schema := dbModel.Schema().Table(table) tableUpdate := make(ovsdb.TableUpdate2) @@ -333,7 +333,7 @@ func (o *OvsdbServer) Mutate(database, table string, where []ovsdb.Condition, mu if err != nil { panic(err) } - new, err := dbModel.Model.NewModel(table) + new, err := dbModel.Request().NewModel(table) if err != nil { panic(err) } @@ -452,8 +452,8 @@ func (o *OvsdbServer) Delete(database, table string, where []ovsdb.Condition) (o o.modelsMutex.Lock() dbModel := o.models[database] o.modelsMutex.Unlock() - m := mapper.NewMapper(dbModel.Schema) - schema := dbModel.Schema.Table(table) + m := dbModel.Mapper() + schema := dbModel.Schema().Table(table) tableUpdate := make(ovsdb.TableUpdate2) rows, err := o.db.List(database, table, where...) if err != nil { diff --git a/server/transact_test.go b/server/transact_test.go index d52bbe17..fbfa8032 100644 --- a/server/transact_test.go +++ b/server/transact_test.go @@ -23,8 +23,7 @@ func TestMutateOp(t *testing.T) { t.Fatal(err) } ovsDB := NewInMemoryDatabase(map[string]*model.DatabaseModelRequest{"Open_vSwitch": defDB}) - o, err := NewOvsdbServer(ovsDB, DatabaseModel{ - Model: defDB, Schema: schema}) + o, err := NewOvsdbServer(ovsDB, *model.NewDatabaseModel(schema, defDB)) require.Nil(t, err) ovsUUID := uuid.NewString() From d5556fa2154d7fe9d84bdb9d3346d575cc152a76 Mon Sep 17 00:00:00 2001 From: Adrian Moreno Date: Fri, 8 Oct 2021 09:06:54 +0200 Subject: [PATCH 05/10] database: Add 2-step initialization It is common to first create a DatabaseModel only based on the DatabaseModelRequest, and then add / remove the schema to it when, e.g: when the client (re) connects. Signed-off-by: Adrian Moreno --- model/database.go | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/model/database.go b/model/database.go index 1b13ea5a..9c5f02a1 100644 --- a/model/database.go +++ b/model/database.go @@ -8,6 +8,7 @@ import ( // A DatabaseModel represents libovsdb's metadata about the database. // It's the result of combining the client's DatabaseModelRequest and the server's Schema type DatabaseModel struct { + valid bool request *DatabaseModelRequest schema *ovsdb.DatabaseSchema mapper *mapper.Mapper @@ -36,3 +37,35 @@ func (db *DatabaseModel) Schema() *ovsdb.DatabaseSchema { func (db *DatabaseModel) Mapper() *mapper.Mapper { return db.mapper } + +// NewPartialDatabaseModel returns a DatabaseModel what does not have a schema yet +func NewPartialDatabaseModel(request *DatabaseModelRequest) *DatabaseModel { + return &DatabaseModel{ + valid: false, + request: request, + } +} + +// Valid returns whether the DatabaseModel is fully functional +func (db *DatabaseModel) Valid() bool { + return db.valid +} + +// SetSchema adds the Schema to the DatabaseModel making it valid if it was not before +func (db *DatabaseModel) SetSchema(schema *ovsdb.DatabaseSchema) []error { + errors := db.request.Validate(schema) + if len(errors) > 0 { + return errors + } + db.schema = schema + db.mapper = mapper.NewMapper(schema) + db.valid = true + return errors +} + +// ClearSchema removes the Schema from the DatabaseModel making it not valid +func (db *DatabaseModel) ClearSchema() { + db.schema = nil + db.mapper = nil + db.valid = false +} From 81f034b9b48297fa1bcf0343ee3c0aed42aa26df Mon Sep 17 00:00:00 2001 From: Adrian Moreno Date: Fri, 8 Oct 2021 08:52:59 +0200 Subject: [PATCH 06/10] client: Use DatabaseModel in client an cache For the cache, it's simply replacing three fields with one For the client, use the 2-step DatabaseModel initialization Signed-off-by: Adrian Moreno --- cache/cache.go | 47 +++++++++++++++++++--------------------- cache/cache_test.go | 39 ++++++++++++++++++++++----------- client/api_test_model.go | 3 ++- client/client.go | 42 ++++++++++++++++------------------- client/client_test.go | 20 +++++++++++------ client/monitor.go | 4 ++-- model/database.go | 31 +++++++++++++------------- server/database.go | 3 ++- 8 files changed, 102 insertions(+), 87 deletions(-) diff --git a/cache/cache.go b/cache/cache.go index d38bbb4b..a1c1f200 100644 --- a/cache/cache.go +++ b/cache/cache.go @@ -406,9 +406,7 @@ func (e *EventHandlerFuncs) OnDelete(table string, row model.Model) { type TableCache struct { cache map[string]*RowCache eventProcessor *eventProcessor - mapper *mapper.Mapper - dbModelReq *model.DatabaseModelRequest - schema *ovsdb.DatabaseSchema + dbModel *model.DatabaseModel ovsdb.NotificationHandler mutex sync.RWMutex } @@ -417,18 +415,18 @@ type TableCache struct { type Data map[string]map[string]model.Model // NewTableCache creates a new TableCache -func NewTableCache(schema *ovsdb.DatabaseSchema, dbModelReq *model.DatabaseModelRequest, data Data) (*TableCache, error) { - if schema == nil || dbModelReq == nil { - return nil, fmt.Errorf("tablecache without databasemodel cannot be populated") +func NewTableCache(dbModel *model.DatabaseModel, data Data) (*TableCache, error) { + if !dbModel.Valid() { + return nil, fmt.Errorf("tablecache without valid databasemodel cannot be populated") } eventProcessor := newEventProcessor(bufferSize) cache := make(map[string]*RowCache) - tableTypes := dbModelReq.Types() - for name, tableSchema := range schema.Tables { + tableTypes := dbModel.Request().Types() + for name, tableSchema := range dbModel.Schema().Tables { cache[name] = newRowCache(name, tableSchema, tableTypes[name]) } for table, rowData := range data { - if _, ok := schema.Tables[table]; !ok { + if _, ok := dbModel.Schema().Tables[table]; !ok { return nil, fmt.Errorf("table %s is not in schema", table) } for uuid, row := range rowData { @@ -439,22 +437,20 @@ func NewTableCache(schema *ovsdb.DatabaseSchema, dbModelReq *model.DatabaseModel } return &TableCache{ cache: cache, - schema: schema, eventProcessor: eventProcessor, - mapper: mapper.NewMapper(schema), - dbModelReq: dbModelReq, + dbModel: dbModel, mutex: sync.RWMutex{}, }, nil } // Mapper returns the mapper func (t *TableCache) Mapper() *mapper.Mapper { - return t.mapper + return t.dbModel.Mapper() } // DatabaseModelRequest returns the DatabaseModelRequest func (t *TableCache) DatabaseModelRequest() *model.DatabaseModelRequest { - return t.dbModelReq + return t.dbModel.Request() } // Table returns the a Table from the cache with a given name @@ -517,7 +513,7 @@ func (t *TableCache) Populate(tableUpdates ovsdb.TableUpdates) { t.mutex.Lock() defer t.mutex.Unlock() - for table := range t.dbModelReq.Types() { + for table := range t.dbModel.Request().Types() { updates, ok := tableUpdates[table] if !ok { continue @@ -563,7 +559,7 @@ func (t *TableCache) Populate(tableUpdates ovsdb.TableUpdates) { func (t *TableCache) Populate2(tableUpdates ovsdb.TableUpdates2) { t.mutex.Lock() defer t.mutex.Unlock() - for table := range t.dbModelReq.Types() { + for table := range t.dbModel.Request().Types() { updates, ok := tableUpdates[table] if !ok { continue @@ -624,12 +620,13 @@ func (t *TableCache) Populate2(tableUpdates ovsdb.TableUpdates2) { } // Purge drops all data in the cache and reinitializes it using the -// provided schema -func (t *TableCache) Purge(schema *ovsdb.DatabaseSchema) { +// provided database model +func (t *TableCache) Purge(dbModel *model.DatabaseModel) { t.mutex.Lock() defer t.mutex.Unlock() - tableTypes := t.dbModelReq.Types() - for name, tableSchema := range t.schema.Tables { + t.dbModel = dbModel + tableTypes := t.dbModel.Request().Types() + for name, tableSchema := range t.dbModel.Schema().Tables { t.cache[name] = newRowCache(name, tableSchema, tableTypes[name]) } } @@ -756,16 +753,16 @@ func (e *eventProcessor) Run(stopCh <-chan struct{}) { // CreateModel creates a new Model instance based on the Row information func (t *TableCache) CreateModel(tableName string, row *ovsdb.Row, uuid string) (model.Model, error) { - table := t.mapper.Schema.Table(tableName) + table := t.dbModel.Schema().Table(tableName) if table == nil { return nil, fmt.Errorf("table %s not found", tableName) } - model, err := t.dbModelReq.NewModel(tableName) + model, err := t.dbModel.Request().NewModel(tableName) if err != nil { return nil, err } - err = t.mapper.GetRowData(tableName, row, model) + err = t.dbModel.Mapper().GetRowData(tableName, row, model) if err != nil { return nil, err } @@ -786,11 +783,11 @@ func (t *TableCache) CreateModel(tableName string, row *ovsdb.Row, uuid string) // ApplyModifications applies the contents of a RowUpdate2.Modify to a model // nolint: gocyclo func (t *TableCache) ApplyModifications(tableName string, base model.Model, update ovsdb.Row) error { - table := t.mapper.Schema.Table(tableName) + table := t.dbModel.Schema().Table(tableName) if table == nil { return fmt.Errorf("table %s not found", tableName) } - schema := t.schema.Table(tableName) + schema := t.dbModel.Schema().Table(tableName) if schema == nil { return fmt.Errorf("no schema for table %s", tableName) } diff --git a/cache/cache_test.go b/cache/cache_test.go index 866ae133..cc5f7e21 100644 --- a/cache/cache_test.go +++ b/cache/cache_test.go @@ -107,7 +107,8 @@ func TestRowCacheCreate(t *testing.T) { testData := Data{ "Open_vSwitch": map[string]model.Model{"bar": &testModel{Foo: "bar"}}, } - tc, err := NewTableCache(&schema, db, testData) + dbModel := model.NewDatabaseModel(&schema, db) + tc, err := NewTableCache(dbModel, testData) require.Nil(t, err) tests := []struct { @@ -194,7 +195,8 @@ func TestRowCacheCreateMultiIndex(t *testing.T) { testData := Data{ "Open_vSwitch": map[string]model.Model{"bar": &testModel{Foo: "bar", Bar: "bar"}}, } - tc, err := NewTableCache(&schema, db, testData) + dbModel := model.NewDatabaseModel(&schema, db) + tc, err := NewTableCache(dbModel, testData) require.Nil(t, err) tests := []struct { name string @@ -289,7 +291,8 @@ func TestRowCacheUpdate(t *testing.T) { "foobar": &testModel{Foo: "foobar"}, }, } - tc, err := NewTableCache(&schema, db, testData) + dbModel := model.NewDatabaseModel(&schema, db) + tc, err := NewTableCache(dbModel, testData) require.Nil(t, err) tests := []struct { @@ -372,7 +375,8 @@ func TestRowCacheUpdateMultiIndex(t *testing.T) { "foobar": &testModel{Foo: "foobar", Bar: "foobar"}, }, } - tc, err := NewTableCache(&schema, db, testData) + dbModel := model.NewDatabaseModel(&schema, db) + tc, err := NewTableCache(dbModel, testData) require.Nil(t, err) tests := []struct { @@ -452,7 +456,8 @@ func TestRowCacheDelete(t *testing.T) { "bar": &testModel{Foo: "bar"}, }, } - tc, err := NewTableCache(&schema, db, testData) + dbModel := model.NewDatabaseModel(&schema, db) + tc, err := NewTableCache(dbModel, testData) require.Nil(t, err) tests := []struct { @@ -708,7 +713,8 @@ func TestTableCache_populate(t *testing.T) { } `), &schema) assert.Nil(t, err) - tc, err := NewTableCache(&schema, db, nil) + dbModel := model.NewDatabaseModel(&schema, db) + tc, err := NewTableCache(dbModel, nil) assert.Nil(t, err) testRow := ovsdb.Row(map[string]interface{}{"_uuid": "test", "foo": "bar"}) @@ -773,7 +779,8 @@ func TestTableCachePopulate(t *testing.T) { } `), &schema) assert.Nil(t, err) - tc, err := NewTableCache(&schema, db, nil) + dbModel := model.NewDatabaseModel(&schema, db) + tc, err := NewTableCache(dbModel, nil) assert.Nil(t, err) testRow := ovsdb.Row(map[string]interface{}{"_uuid": "test", "foo": "bar"}) @@ -837,7 +844,8 @@ func TestTableCachePopulate2(t *testing.T) { } `), &schema) assert.Nil(t, err) - tc, err := NewTableCache(&schema, db, nil) + dbModel := model.NewDatabaseModel(&schema, db) + tc, err := NewTableCache(dbModel, nil) assert.Nil(t, err) testRow := ovsdb.Row(map[string]interface{}{"_uuid": "test", "foo": "bar"}) @@ -958,7 +966,8 @@ func TestIndex(t *testing.T) { } `), &schema) assert.Nil(t, err) - tc, err := NewTableCache(&schema, db, nil) + dbModel := model.NewDatabaseModel(&schema, db) + tc, err := NewTableCache(dbModel, nil) assert.Nil(t, err) obj := &indexTestModel{ UUID: "test1", @@ -1056,7 +1065,8 @@ func TestTableCacheRowByModelSingleIndex(t *testing.T) { "bar": &testModel{Foo: "bar", Bar: "bar"}, }, } - tc, err := NewTableCache(&schema, db, testData) + dbModel := model.NewDatabaseModel(&schema, db) + tc, err := NewTableCache(dbModel, testData) require.NoError(t, err) t.Run("get foo by index", func(t *testing.T) { @@ -1105,7 +1115,8 @@ func TestTableCacheRowByModelTwoIndexes(t *testing.T) { "bar": &testModel{Foo: "bar", Bar: "bar"}, }, } - tc, err := NewTableCache(&schema, db, testData) + dbModel := model.NewDatabaseModel(&schema, db) + tc, err := NewTableCache(dbModel, testData) require.NoError(t, err) t.Run("get foo by Foo index", func(t *testing.T) { @@ -1153,7 +1164,8 @@ func TestTableCacheRowByModelMultiIndex(t *testing.T) { testData := Data{ "Open_vSwitch": map[string]model.Model{"foo": myFoo, "bar": &testModel{Foo: "bar", Bar: "bar"}}, } - tc, err := NewTableCache(&schema, db, testData) + dbModel := model.NewDatabaseModel(&schema, db) + tc, err := NewTableCache(dbModel, testData) require.NoError(t, err) t.Run("incomplete index", func(t *testing.T) { @@ -1299,7 +1311,8 @@ func TestTableCacheApplyModifications(t *testing.T) { } `), &schema) require.NoError(t, err) - tc, err := NewTableCache(&schema, db, nil) + dbModel := model.NewDatabaseModel(&schema, db) + tc, err := NewTableCache(dbModel, nil) assert.Nil(t, err) original := model.Clone(tt.base).(*testDBModel) err = tc.ApplyModifications("Open_vSwitch", original, tt.update) diff --git a/client/api_test_model.go b/client/api_test_model.go index 93932842..c56cf821 100644 --- a/client/api_test_model.go +++ b/client/api_test_model.go @@ -159,7 +159,8 @@ func apiTestCache(t *testing.T, data map[string]map[string]model.Model) *cache.T assert.Nil(t, err) db, err := model.NewDatabaseModelRequest("OVN_NorthBound", map[string]model.Model{"Logical_Switch": &testLogicalSwitch{}, "Logical_Switch_Port": &testLogicalSwitchPort{}}) assert.Nil(t, err) - cache, err := cache.NewTableCache(&schema, db, data) + dbModel := model.NewDatabaseModel(&schema, db) + cache, err := cache.NewTableCache(dbModel, data) assert.Nil(t, err) return cache } diff --git a/client/client.go b/client/client.go index 95438553..8fc7ee18 100644 --- a/client/client.go +++ b/client/client.go @@ -17,7 +17,6 @@ import ( "github.com/cenkalti/rpc2" "github.com/cenkalti/rpc2/jsonrpc" "github.com/ovn-org/libovsdb/cache" - "github.com/ovn-org/libovsdb/mapper" "github.com/ovn-org/libovsdb/model" "github.com/ovn-org/libovsdb/ovsdb" "github.com/ovn-org/libovsdb/ovsdb/serverdb" @@ -86,8 +85,7 @@ type ovsdbClient struct { // database is everything needed to map between go types and an ovsdb Database type database struct { - model *model.DatabaseModelRequest - schema *ovsdb.DatabaseSchema + model *model.DatabaseModel schemaMutex sync.RWMutex cache *cache.TableCache cacheMutex sync.RWMutex @@ -103,17 +101,17 @@ type database struct { // database model. The client can be configured using one or more Option(s), // like WithTLSConfig. If no WithEndpoint option is supplied, the default of // unix:/var/run/openvswitch/ovsdb.sock is used -func NewOVSDBClient(databaseModel *model.DatabaseModelRequest, opts ...Option) (Client, error) { - return newOVSDBClient(databaseModel, opts...) +func NewOVSDBClient(databaseModelRequest *model.DatabaseModelRequest, opts ...Option) (Client, error) { + return newOVSDBClient(databaseModelRequest, opts...) } // newOVSDBClient creates a new ovsdbClient -func newOVSDBClient(databaseModel *model.DatabaseModelRequest, opts ...Option) (*ovsdbClient, error) { +func newOVSDBClient(databaseModelRequest *model.DatabaseModelRequest, opts ...Option) (*ovsdbClient, error) { ovs := &ovsdbClient{ - primaryDBName: databaseModel.Name(), + primaryDBName: databaseModelRequest.Name(), databases: map[string]*database{ - databaseModel.Name(): { - model: databaseModel, + databaseModelRequest.Name(): { + model: model.NewPartialDatabaseModel(databaseModelRequest), monitors: make(map[string]*Monitor), }, }, @@ -132,7 +130,7 @@ func newOVSDBClient(databaseModel *model.DatabaseModelRequest, opts ...Option) ( return nil, fmt.Errorf("could not initialize model _Server: %w", err) } ovs.databases[serverDB] = &database{ - model: sm, + model: model.NewPartialDatabaseModel(sm), monitors: make(map[string]*Monitor), } } @@ -287,7 +285,9 @@ func (o *ovsdbClient) tryEndpoint(ctx context.Context, u *url.URL) error { return err } - errors := db.model.Validate(schema) + db.schemaMutex.Lock() + errors := db.model.SetSchema(schema) + db.schemaMutex.Unlock() if len(errors) > 0 { var combined []string for _, err := range errors { @@ -300,13 +300,9 @@ func (o *ovsdbClient) tryEndpoint(ctx context.Context, u *url.URL) error { return err } - db.schemaMutex.Lock() - db.schema = schema - db.schemaMutex.Unlock() - db.cacheMutex.Lock() if db.cache == nil { - db.cache, err = cache.NewTableCache(schema, db.model, nil) + db.cache, err = cache.NewTableCache(db.model, nil) if err != nil { db.cacheMutex.Unlock() o.rpcClient.Close() @@ -315,7 +311,7 @@ func (o *ovsdbClient) tryEndpoint(ctx context.Context, u *url.URL) error { } db.api = newAPI(db.cache) } else { - db.cache.Purge(db.schema) + db.cache.Purge(db.model) } db.cacheMutex.Unlock() } @@ -425,7 +421,7 @@ func (o *ovsdbClient) Schema() *ovsdb.DatabaseSchema { db := o.primaryDB() db.schemaMutex.RLock() defer db.schemaMutex.RUnlock() - return db.schema + return db.model.Schema() } // Cache returns the TableCache that is populated from @@ -619,7 +615,7 @@ func (o *ovsdbClient) transact(ctx context.Context, dbName string, operation ... var reply []ovsdb.OperationResult db := o.databases[dbName] db.schemaMutex.RLock() - schema := o.databases[dbName].schema + schema := o.databases[dbName].model.Schema() db.schemaMutex.RUnlock() if schema == nil { return nil, fmt.Errorf("cannot transact to database %s: schema unknown", dbName) @@ -645,7 +641,7 @@ func (o *ovsdbClient) transact(ctx context.Context, dbName string, operation ... // MonitorAll is a convenience method to monitor every table/column func (o *ovsdbClient) MonitorAll(ctx context.Context) (MonitorCookie, error) { m := newMonitor() - for name := range o.primaryDB().model.Types() { + for name := range o.primaryDB().model.Request().Types() { m.Tables = append(m.Tables, TableMonitor{Table: name}) } return o.Monitor(ctx, m) @@ -705,9 +701,9 @@ func (o *ovsdbClient) monitor(ctx context.Context, cookie MonitorCookie, reconne dbName := cookie.DatabaseName db := o.databases[dbName] db.schemaMutex.RLock() - mapper := mapper.NewMapper(db.schema) + mapper := db.model.Mapper() db.schemaMutex.RUnlock() - typeMap := o.databases[dbName].model.Types() + typeMap := o.databases[dbName].model.Request().Types() requests := make(map[string]ovsdb.MonitorRequest) for _, o := range monitor.Tables { m, ok := typeMap[o.Table] @@ -908,7 +904,7 @@ func (o *ovsdbClient) handleDisconnectNotification() { db.schemaMutex.Lock() defer db.schemaMutex.Unlock() - db.schema = nil + db.model.ClearSchema() db.monitorsMutex.Lock() defer db.monitorsMutex.Unlock() diff --git a/client/client_test.go b/client/client_test.go index b64edf7c..45942358 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -564,7 +564,8 @@ func BenchmarkUpdate1(b *testing.B) { "Open_vSwitch": &OpenvSwitch{}, }) require.NoError(b, err) - ovs.primaryDB().cache, err = cache.NewTableCache(&s, dbModelReq, nil) + dbModel := model.NewDatabaseModel(&s, dbModelReq) + ovs.primaryDB().cache, err = cache.NewTableCache(dbModel, nil) require.NoError(b, err) update := []byte(`{ "Open_vSwitch": { @@ -588,7 +589,8 @@ func BenchmarkUpdate2(b *testing.B) { "Open_vSwitch": &OpenvSwitch{}, }) require.NoError(b, err) - ovs.primaryDB().cache, err = cache.NewTableCache(&s, dbModelReq, nil) + dbModel := model.NewDatabaseModel(&s, dbModelReq) + ovs.primaryDB().cache, err = cache.NewTableCache(dbModel, nil) require.NoError(b, err) update := []byte(`{ "Open_vSwitch": { @@ -613,7 +615,8 @@ func BenchmarkUpdate3(b *testing.B) { "Open_vSwitch": &OpenvSwitch{}, }) require.NoError(b, err) - ovs.primaryDB().cache, err = cache.NewTableCache(&s, dbModelReq, nil) + dbModel := model.NewDatabaseModel(&s, dbModelReq) + ovs.primaryDB().cache, err = cache.NewTableCache(dbModel, nil) require.NoError(b, err) update := []byte(`{ "Open_vSwitch": { @@ -639,7 +642,8 @@ func BenchmarkUpdate5(b *testing.B) { "Open_vSwitch": &OpenvSwitch{}, }) require.NoError(b, err) - ovs.primaryDB().cache, err = cache.NewTableCache(&s, dbModelReq, nil) + dbModel := model.NewDatabaseModel(&s, dbModelReq) + ovs.primaryDB().cache, err = cache.NewTableCache(dbModel, nil) require.NoError(b, err) update := []byte(`{ "Open_vSwitch": { @@ -667,7 +671,8 @@ func BenchmarkUpdate8(b *testing.B) { "Open_vSwitch": &OpenvSwitch{}, }) require.NoError(b, err) - ovs.primaryDB().cache, err = cache.NewTableCache(&s, dbModelReq, nil) + dbModel := model.NewDatabaseModel(&s, dbModelReq) + ovs.primaryDB().cache, err = cache.NewTableCache(dbModel, nil) require.NoError(b, err) update := []byte(`{ "Open_vSwitch": { @@ -712,7 +717,8 @@ func TestUpdate(t *testing.T) { "Open_vSwitch": &OpenvSwitch{}, }) require.NoError(t, err) - ovs.primaryDB().cache, err = cache.NewTableCache(&s, dbModelReq, nil) + dbModel := model.NewDatabaseModel(&s, dbModelReq) + ovs.primaryDB().cache, err = cache.NewTableCache(dbModel, nil) require.NoError(t, err) var reply []interface{} update := []byte(`{ @@ -736,7 +742,7 @@ func TestOperationWhenNotConnected(t *testing.T) { var s ovsdb.DatabaseSchema err = json.Unmarshal([]byte(schema), &s) require.NoError(t, err) - ovs.primaryDB().schema = &s + _ = ovs.primaryDB().model.SetSchema(&s) tests := []struct { name string diff --git a/client/monitor.go b/client/monitor.go index 9bb36099..6982d256 100644 --- a/client/monitor.go +++ b/client/monitor.go @@ -70,7 +70,7 @@ type TableMonitor struct { func WithTable(m model.Model, fields ...interface{}) MonitorOption { return func(o *ovsdbClient, monitor *Monitor) error { - tableName := o.primaryDB().model.FindTable(reflect.TypeOf(m)) + tableName := o.primaryDB().model.Request().FindTable(reflect.TypeOf(m)) if tableName == "" { return fmt.Errorf("object of type %s is not part of the DatabaseModelRequest", reflect.TypeOf(m)) } @@ -85,7 +85,7 @@ func WithTable(m model.Model, fields ...interface{}) MonitorOption { func WithConditionalTable(m model.Model, condition model.Condition, fields ...interface{}) MonitorOption { return func(o *ovsdbClient, monitor *Monitor) error { - tableName := o.primaryDB().model.FindTable(reflect.TypeOf(m)) + tableName := o.primaryDB().model.Request().FindTable(reflect.TypeOf(m)) if tableName == "" { return fmt.Errorf("object of type %s is not part of the DatabaseModelRequest", reflect.TypeOf(m)) } diff --git a/model/database.go b/model/database.go index 9c5f02a1..38dd8e77 100644 --- a/model/database.go +++ b/model/database.go @@ -17,27 +17,13 @@ type DatabaseModel struct { // NewDatabaseModel returns a new DatabaseModel func NewDatabaseModel(schema *ovsdb.DatabaseSchema, request *DatabaseModelRequest) *DatabaseModel { return &DatabaseModel{ + valid: true, request: request, schema: schema, mapper: mapper.NewMapper(schema), } } -// Request returns the DatabaseModel's request -func (db *DatabaseModel) Request() *DatabaseModelRequest { - return db.request -} - -// Schema returns the DatabaseModel's schema -func (db *DatabaseModel) Schema() *ovsdb.DatabaseSchema { - return db.schema -} - -// Mapper returns the DatabaseModel's mapper -func (db *DatabaseModel) Mapper() *mapper.Mapper { - return db.mapper -} - // NewPartialDatabaseModel returns a DatabaseModel what does not have a schema yet func NewPartialDatabaseModel(request *DatabaseModelRequest) *DatabaseModel { return &DatabaseModel{ @@ -69,3 +55,18 @@ func (db *DatabaseModel) ClearSchema() { db.mapper = nil db.valid = false } + +// Request returns the DatabaseModel's request +func (db *DatabaseModel) Request() *DatabaseModelRequest { + return db.request +} + +// Schema returns the DatabaseModel's schema +func (db *DatabaseModel) Schema() *ovsdb.DatabaseSchema { + return db.schema +} + +// Mapper returns the DatabaseModel's mapper +func (db *DatabaseModel) Mapper() *mapper.Mapper { + return db.mapper +} diff --git a/server/database.go b/server/database.go index b15c1a05..3a73ea45 100644 --- a/server/database.go +++ b/server/database.go @@ -42,7 +42,8 @@ func (db *inMemoryDatabase) CreateDatabase(name string, schema *ovsdb.DatabaseSc if mo, ok = db.models[schema.Name]; !ok { return fmt.Errorf("no db model provided for schema with name %s", name) } - database, err := cache.NewTableCache(schema, mo, nil) + dbModel := model.NewDatabaseModel(schema, mo) + database, err := cache.NewTableCache(dbModel, nil) if err != nil { return nil } From cf54b19b6fe743f6b5c1c963b0b8962e40211bdc Mon Sep 17 00:00:00 2001 From: Adrian Moreno Date: Thu, 7 Oct 2021 18:12:26 +0200 Subject: [PATCH 07/10] model: move methods from Request to DatabaseModel Now that client, cache and server uses the DatabaseModel as central point of model creation and introspection, we can hide the DatabaseModelRequest and move its pulbic functions to the DatabaseModel Signed-off-by: Adrian Moreno --- cache/cache.go | 14 +++++++------- client/api.go | 6 +++--- client/client.go | 4 ++-- client/monitor.go | 4 ++-- model/database.go | 33 ++++++++++++++++++++++++++++++++- model/model_test.go | 8 ++++---- model/request.go | 24 +++--------------------- server/transact.go | 6 +++--- 8 files changed, 56 insertions(+), 43 deletions(-) diff --git a/cache/cache.go b/cache/cache.go index a1c1f200..63d21520 100644 --- a/cache/cache.go +++ b/cache/cache.go @@ -421,7 +421,7 @@ func NewTableCache(dbModel *model.DatabaseModel, data Data) (*TableCache, error) } eventProcessor := newEventProcessor(bufferSize) cache := make(map[string]*RowCache) - tableTypes := dbModel.Request().Types() + tableTypes := dbModel.Types() for name, tableSchema := range dbModel.Schema().Tables { cache[name] = newRowCache(name, tableSchema, tableTypes[name]) } @@ -449,8 +449,8 @@ func (t *TableCache) Mapper() *mapper.Mapper { } // DatabaseModelRequest returns the DatabaseModelRequest -func (t *TableCache) DatabaseModelRequest() *model.DatabaseModelRequest { - return t.dbModel.Request() +func (t *TableCache) DatabaseModel() *model.DatabaseModel { + return t.dbModel } // Table returns the a Table from the cache with a given name @@ -513,7 +513,7 @@ func (t *TableCache) Populate(tableUpdates ovsdb.TableUpdates) { t.mutex.Lock() defer t.mutex.Unlock() - for table := range t.dbModel.Request().Types() { + for table := range t.dbModel.Types() { updates, ok := tableUpdates[table] if !ok { continue @@ -559,7 +559,7 @@ func (t *TableCache) Populate(tableUpdates ovsdb.TableUpdates) { func (t *TableCache) Populate2(tableUpdates ovsdb.TableUpdates2) { t.mutex.Lock() defer t.mutex.Unlock() - for table := range t.dbModel.Request().Types() { + for table := range t.dbModel.Types() { updates, ok := tableUpdates[table] if !ok { continue @@ -625,7 +625,7 @@ func (t *TableCache) Purge(dbModel *model.DatabaseModel) { t.mutex.Lock() defer t.mutex.Unlock() t.dbModel = dbModel - tableTypes := t.dbModel.Request().Types() + tableTypes := t.dbModel.Types() for name, tableSchema := range t.dbModel.Schema().Tables { t.cache[name] = newRowCache(name, tableSchema, tableTypes[name]) } @@ -757,7 +757,7 @@ func (t *TableCache) CreateModel(tableName string, row *ovsdb.Row, uuid string) if table == nil { return nil, fmt.Errorf("table %s not found", tableName) } - model, err := t.dbModel.Request().NewModel(tableName) + model, err := t.dbModel.NewModel(tableName) if err != nil { return nil, err } diff --git a/client/api.go b/client/api.go index 55b63cf5..8a0323a8 100644 --- a/client/api.go +++ b/client/api.go @@ -280,7 +280,7 @@ func (a api) Mutate(model model.Model, mutationObjs ...model.Mutation) ([]ovsdb. return nil, fmt.Errorf("at least one Mutation must be provided") } - tableName := a.cache.DatabaseModelRequest().FindTable(reflect.ValueOf(model).Type()) + tableName := a.cache.DatabaseModel().FindTable(reflect.ValueOf(model).Type()) if tableName == "" { return nil, fmt.Errorf("table not found for object") } @@ -414,7 +414,7 @@ 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.DatabaseModelRequest().FindTable(reflect.TypeOf(m)) + table := a.cache.DatabaseModel().FindTable(reflect.TypeOf(m)) if table == "" { return "", &ErrWrongType{reflect.TypeOf(m), "Model not found in Database Model"} } @@ -439,7 +439,7 @@ func (a api) getTableFromFunc(predicate interface{}) (string, error) { fmt.Sprintf("Type %s does not implement Model interface", modelType.String())} } - table := a.cache.DatabaseModelRequest().FindTable(modelType) + table := a.cache.DatabaseModel().FindTable(modelType) if table == "" { return "", &ErrWrongType{predType, fmt.Sprintf("Model %s not found in Database Model", modelType.String())} diff --git a/client/client.go b/client/client.go index 8fc7ee18..96876374 100644 --- a/client/client.go +++ b/client/client.go @@ -641,7 +641,7 @@ func (o *ovsdbClient) transact(ctx context.Context, dbName string, operation ... // MonitorAll is a convenience method to monitor every table/column func (o *ovsdbClient) MonitorAll(ctx context.Context) (MonitorCookie, error) { m := newMonitor() - for name := range o.primaryDB().model.Request().Types() { + for name := range o.primaryDB().model.Types() { m.Tables = append(m.Tables, TableMonitor{Table: name}) } return o.Monitor(ctx, m) @@ -703,7 +703,7 @@ func (o *ovsdbClient) monitor(ctx context.Context, cookie MonitorCookie, reconne db.schemaMutex.RLock() mapper := db.model.Mapper() db.schemaMutex.RUnlock() - typeMap := o.databases[dbName].model.Request().Types() + typeMap := o.databases[dbName].model.Types() requests := make(map[string]ovsdb.MonitorRequest) for _, o := range monitor.Tables { m, ok := typeMap[o.Table] diff --git a/client/monitor.go b/client/monitor.go index 6982d256..9bb36099 100644 --- a/client/monitor.go +++ b/client/monitor.go @@ -70,7 +70,7 @@ type TableMonitor struct { func WithTable(m model.Model, fields ...interface{}) MonitorOption { return func(o *ovsdbClient, monitor *Monitor) error { - tableName := o.primaryDB().model.Request().FindTable(reflect.TypeOf(m)) + tableName := o.primaryDB().model.FindTable(reflect.TypeOf(m)) if tableName == "" { return fmt.Errorf("object of type %s is not part of the DatabaseModelRequest", reflect.TypeOf(m)) } @@ -85,7 +85,7 @@ func WithTable(m model.Model, fields ...interface{}) MonitorOption { func WithConditionalTable(m model.Model, condition model.Condition, fields ...interface{}) MonitorOption { return func(o *ovsdbClient, monitor *Monitor) error { - tableName := o.primaryDB().model.Request().FindTable(reflect.TypeOf(m)) + tableName := o.primaryDB().model.FindTable(reflect.TypeOf(m)) if tableName == "" { return fmt.Errorf("object of type %s is not part of the DatabaseModelRequest", reflect.TypeOf(m)) } diff --git a/model/database.go b/model/database.go index 38dd8e77..e5f07379 100644 --- a/model/database.go +++ b/model/database.go @@ -1,6 +1,9 @@ package model import ( + "fmt" + "reflect" + "github.com/ovn-org/libovsdb/mapper" "github.com/ovn-org/libovsdb/ovsdb" ) @@ -39,7 +42,7 @@ func (db *DatabaseModel) Valid() bool { // SetSchema adds the Schema to the DatabaseModel making it valid if it was not before func (db *DatabaseModel) SetSchema(schema *ovsdb.DatabaseSchema) []error { - errors := db.request.Validate(schema) + errors := db.request.validate(schema) if len(errors) > 0 { return errors } @@ -70,3 +73,31 @@ func (db *DatabaseModel) Schema() *ovsdb.DatabaseSchema { func (db *DatabaseModel) Mapper() *mapper.Mapper { return db.mapper } + +// NewModel returns a new instance of a model from a specific string +func (db DatabaseModel) NewModel(table string) (Model, error) { + mtype, ok := db.request.types[table] + if !ok { + return nil, fmt.Errorf("table %s not found in database model", string(table)) + } + model := reflect.New(mtype.Elem()) + return model.Interface().(Model), nil +} + +// Types returns the DatabaseModel Types +// the DatabaseModel types is a map of reflect.Types indexed by string +// The reflect.Type is a pointer to a struct that contains 'ovs' tags +// as described above. Such pointer to struct also implements the Model interface +func (db DatabaseModel) Types() map[string]reflect.Type { + return db.request.types +} + +// FindTable returns the string associated with a reflect.Type or "" +func (db DatabaseModel) FindTable(mType reflect.Type) string { + for table, tType := range db.request.types { + if tType == mType { + return table + } + } + return "" +} diff --git a/model/model_test.go b/model/model_test.go index bbfc1a77..84d1b765 100644 --- a/model/model_test.go +++ b/model/model_test.go @@ -53,7 +53,7 @@ func TestDatabaseModelRequest(t *testing.T) { db, err := NewDatabaseModelRequest(tt.name, tt.obj) if tt.valid { assert.Nil(t, err) - assert.Len(t, db.Types(), len(tt.obj)) + assert.Len(t, db.types, len(tt.obj)) assert.Equal(t, tt.name, db.Name()) } else { assert.NotNil(t, err) @@ -65,9 +65,9 @@ func TestDatabaseModelRequest(t *testing.T) { func TestNewModel(t *testing.T) { db, err := NewDatabaseModelRequest("testTable", map[string]Model{"Test_A": &modelA{}, "Test_B": &modelB{}}) assert.Nil(t, err) - _, err = db.NewModel("Unknown") + _, err = db.newModel("Unknown") assert.NotNilf(t, err, "Creating model from unknown table should fail") - model, err := db.NewModel("Test_A") + model, err := db.newModel("Test_A") assert.Nilf(t, err, "Creating model from valid table should succeed") assert.IsTypef(t, model, &modelA{}, "model creation should return the appropriate type") } @@ -324,7 +324,7 @@ func TestValidate(t *testing.T) { var schema ovsdb.DatabaseSchema err := json.Unmarshal(tt.schema, &schema) assert.Nil(t, err) - errors := model.Validate(&schema) + errors := model.validate(&schema) if tt.err { assert.Greater(t, len(errors), 0) } else { diff --git a/model/request.go b/model/request.go index be303480..e1e2ce44 100644 --- a/model/request.go +++ b/model/request.go @@ -15,7 +15,7 @@ type DatabaseModelRequest struct { } // NewModel returns a new instance of a model from a specific string -func (db DatabaseModelRequest) NewModel(table string) (Model, error) { +func (db DatabaseModelRequest) newModel(table string) (Model, error) { mtype, ok := db.types[table] if !ok { return nil, fmt.Errorf("table %s not found in database model", string(table)) @@ -24,32 +24,14 @@ func (db DatabaseModelRequest) NewModel(table string) (Model, error) { return model.Interface().(Model), nil } -// Types returns the DatabaseModelRequest Types -// the DatabaseModelRequest types is a map of reflect.Types indexed by string -// The reflect.Type is a pointer to a struct that contains 'ovs' tags -// as described above. Such pointer to struct also implements the Model interface -func (db DatabaseModelRequest) Types() map[string]reflect.Type { - return db.types -} - // Name returns the database name func (db DatabaseModelRequest) Name() string { return db.name } -// FindTable returns the string associated with a reflect.Type or "" -func (db DatabaseModelRequest) FindTable(mType reflect.Type) string { - for table, tType := range db.types { - if tType == mType { - return table - } - } - return "" -} - // Validate validates the DatabaseModel against the input schema // Returns all the errors detected -func (db DatabaseModelRequest) Validate(schema *ovsdb.DatabaseSchema) []error { +func (db DatabaseModelRequest) validate(schema *ovsdb.DatabaseSchema) []error { var errors []error if db.name != schema.Name { errors = append(errors, fmt.Errorf("database model name (%s) does not match schema (%s)", @@ -62,7 +44,7 @@ func (db DatabaseModelRequest) Validate(schema *ovsdb.DatabaseSchema) []error { errors = append(errors, fmt.Errorf("database model contains a model for table %s that does not exist in schema", tableName)) continue } - model, err := db.NewModel(tableName) + model, err := db.newModel(tableName) if err != nil { errors = append(errors, err) continue diff --git a/server/transact.go b/server/transact.go index 8ec0a091..d64d1f50 100644 --- a/server/transact.go +++ b/server/transact.go @@ -82,7 +82,7 @@ func (o *OvsdbServer) Insert(database string, table string, rowUUID string, row rowUUID = uuid.NewString() } - model, err := dbModel.Request().NewModel(table) + model, err := dbModel.NewModel(table) if err != nil { return ovsdb.OperationResult{ Error: err.Error(), @@ -201,7 +201,7 @@ func (o *OvsdbServer) Update(database, table string, where []ovsdb.Condition, ro if err != nil { panic(err) } - new, err := dbModel.Request().NewModel(table) + new, err := dbModel.NewModel(table) if err != nil { panic(err) } @@ -333,7 +333,7 @@ func (o *OvsdbServer) Mutate(database, table string, where []ovsdb.Condition, mu if err != nil { panic(err) } - new, err := dbModel.Request().NewModel(table) + new, err := dbModel.NewModel(table) if err != nil { panic(err) } From 3ff43c595e82fb1b2fa7227e14a30ce006f070c1 Mon Sep 17 00:00:00 2001 From: Adrian Moreno Date: Fri, 8 Oct 2021 13:42:59 +0200 Subject: [PATCH 08/10] mapper: make Mapper use mapper.Info as input All around the codebase we're creating mapper.Info structures and then calling Mapper functions that create Info structures again. Given that mapper.Info already defines all the metadata that Mapper needs to do the native-to-ovs transations, it makes sense to use Info structures as input to all functions. That simplifies the code inside the mapper module. Also, I'd expect some performance improvement since we were creating multiple Info structs unnecessarily in the host path. It's true that, for now, it makes it sligthly more cumbersone to call mapper functions, since the Info struct has to be created first and it now requires an additional argument (the table name). However, this can be improved later on by having the database model build the Info structs for us. Signed-off-by: Adrian Moreno --- cache/cache.go | 29 ++++--- cache/cache_test.go | 12 +-- client/api.go | 18 ++--- client/client.go | 14 +++- client/condition.go | 40 +++++++--- mapper/info.go | 46 ++++++----- mapper/info_test.go | 9 ++- mapper/mapper.go | 170 ++++++++++++---------------------------- mapper/mapper_test.go | 44 ++++++++--- model/request.go | 2 +- server/server.go | 4 +- server/transact.go | 54 +++++++------ server/transact_test.go | 22 ++++-- 13 files changed, 228 insertions(+), 236 deletions(-) diff --git a/cache/cache.go b/cache/cache.go index 63d21520..be3c4801 100644 --- a/cache/cache.go +++ b/cache/cache.go @@ -90,7 +90,7 @@ func (r *RowCache) RowByModel(m model.Model) model.Model { if reflect.TypeOf(m) != r.dataType { return nil } - info, _ := mapper.NewInfo(&r.schema, m) + info, _ := mapper.NewInfo(r.name, &r.schema, m) uuid, err := info.FieldByColumn("_uuid") if err != nil { return nil @@ -120,7 +120,7 @@ func (r *RowCache) Create(uuid string, m model.Model, checkIndexes bool) error { if reflect.TypeOf(m) != r.dataType { return fmt.Errorf("expected data of type %s, but got %s", r.dataType.String(), reflect.TypeOf(m).String()) } - info, err := mapper.NewInfo(&r.schema, m) + info, err := mapper.NewInfo(r.name, &r.schema, m) if err != nil { return err } @@ -156,11 +156,11 @@ func (r *RowCache) Update(uuid string, m model.Model, checkIndexes bool) error { return fmt.Errorf("row %s does not exist", uuid) } oldRow := model.Clone(r.cache[uuid]) - oldInfo, err := mapper.NewInfo(&r.schema, oldRow) + oldInfo, err := mapper.NewInfo(r.name, &r.schema, oldRow) if err != nil { return err } - newInfo, err := mapper.NewInfo(&r.schema, m) + newInfo, err := mapper.NewInfo(r.name, &r.schema, m) if err != nil { return err } @@ -218,7 +218,7 @@ func (r *RowCache) Update(uuid string, m model.Model, checkIndexes bool) error { } func (r *RowCache) IndexExists(row model.Model) error { - info, err := mapper.NewInfo(&r.schema, row) + info, err := mapper.NewInfo(r.name, &r.schema, row) if err != nil { return err } @@ -252,7 +252,7 @@ func (r *RowCache) Delete(uuid string) error { return fmt.Errorf("row %s does not exist", uuid) } oldRow := r.cache[uuid] - oldInfo, err := mapper.NewInfo(&r.schema, oldRow) + oldInfo, err := mapper.NewInfo(r.name, &r.schema, oldRow) if err != nil { return err } @@ -325,7 +325,7 @@ func (r *RowCache) RowsByCondition(conditions []ovsdb.Condition) ([]model.Model, } else { for _, uuid := range r.Rows() { row := r.Row(uuid) - info, err := mapper.NewInfo(&r.schema, row) + info, err := mapper.NewInfo(r.name, &r.schema, row) if err != nil { return nil, err } @@ -761,18 +761,17 @@ func (t *TableCache) CreateModel(tableName string, row *ovsdb.Row, uuid string) if err != nil { return nil, err } - - err = t.dbModel.Mapper().GetRowData(tableName, row, model) + info, err := mapper.NewInfo(tableName, table, model) + if err != nil { + return nil, err + } + err = t.dbModel.Mapper().GetRowData(row, info) if err != nil { return nil, err } if uuid != "" { - mapperInfo, err := mapper.NewInfo(table, model) - if err != nil { - return nil, err - } - if err := mapperInfo.SetField("_uuid", uuid); err != nil { + if err := info.SetField("_uuid", uuid); err != nil { return nil, err } } @@ -791,7 +790,7 @@ func (t *TableCache) ApplyModifications(tableName string, base model.Model, upda if schema == nil { return fmt.Errorf("no schema for table %s", tableName) } - info, err := mapper.NewInfo(schema, base) + info, err := mapper.NewInfo(tableName, schema, base) if err != nil { return err } diff --git a/cache/cache_test.go b/cache/cache_test.go index cc5f7e21..b2d1db4b 100644 --- a/cache/cache_test.go +++ b/cache/cache_test.go @@ -253,7 +253,7 @@ func TestRowCacheCreateMultiIndex(t *testing.T) { } } else { assert.Nil(t, err) - mapperInfo, err := mapper.NewInfo(tSchema, tt.model) + mapperInfo, err := mapper.NewInfo("Open_vSwitch", tSchema, tt.model) require.Nil(t, err) h, err := valueFromIndex(mapperInfo, newIndex("foo", "bar")) require.Nil(t, err) @@ -419,7 +419,7 @@ func TestRowCacheUpdateMultiIndex(t *testing.T) { assert.Error(t, err) } else { assert.Nil(t, err) - mapperInfo, err := mapper.NewInfo(tSchema, tt.model) + mapperInfo, err := mapper.NewInfo("Open_vSwitch", tSchema, tt.model) require.Nil(t, err) h, err := valueFromIndex(mapperInfo, newIndex("foo", "bar")) require.Nil(t, err) @@ -982,7 +982,7 @@ func TestIndex(t *testing.T) { t.Run("Index by single column", func(t *testing.T) { idx, err := table.Index("foo") assert.Nil(t, err) - info, err := mapper.NewInfo(schema.Table("Open_vSwitch"), obj) + info, err := mapper.NewInfo("Open_vSwitch", schema.Table("Open_vSwitch"), obj) assert.Nil(t, err) v, err := valueFromIndex(info, newIndex("foo")) assert.Nil(t, err) @@ -994,7 +994,7 @@ func TestIndex(t *testing.T) { obj2 := obj obj2.Foo = "wrong" assert.Nil(t, err) - info, err := mapper.NewInfo(schema.Table("Open_vSwitch"), obj2) + info, err := mapper.NewInfo("Open_vSwitch", schema.Table("Open_vSwitch"), obj2) assert.Nil(t, err) v, err := valueFromIndex(info, newIndex("foo")) assert.Nil(t, err) @@ -1012,7 +1012,7 @@ func TestIndex(t *testing.T) { t.Run("Index by multi-column", func(t *testing.T) { idx, err := table.Index("bar", "baz") assert.Nil(t, err) - info, err := mapper.NewInfo(schema.Table("Open_vSwitch"), obj) + info, err := mapper.NewInfo("Open_vSwitch", schema.Table("Open_vSwitch"), obj) assert.Nil(t, err) v, err := valueFromIndex(info, newIndex("bar", "baz")) assert.Nil(t, err) @@ -1023,7 +1023,7 @@ func TestIndex(t *testing.T) { assert.Nil(t, err) obj2 := obj obj2.Baz++ - info, err := mapper.NewInfo(schema.Table("Open_vSwitch"), obj) + info, err := mapper.NewInfo("Open_vSwitch", schema.Table("Open_vSwitch"), obj) assert.Nil(t, err) v, err := valueFromIndex(info, newIndex("bar", "baz")) assert.Nil(t, err) diff --git a/client/api.go b/client/api.go index 8a0323a8..b92933c3 100644 --- a/client/api.go +++ b/client/api.go @@ -246,7 +246,7 @@ func (a api) Create(models ...model.Model) ([]ovsdb.Operation, error) { table := a.cache.Mapper().Schema.Table(tableName) // Read _uuid field, and use it as named-uuid - info, err := mapper.NewInfo(table, model) + info, err := mapper.NewInfo(tableName, table, model) if err != nil { return nil, err } @@ -256,7 +256,7 @@ func (a api) Create(models ...model.Model) ([]ovsdb.Operation, error) { return nil, err } - row, err := a.cache.Mapper().NewRow(tableName, model) + row, err := a.cache.Mapper().NewRow(info) if err != nil { return nil, err } @@ -294,7 +294,7 @@ func (a api) Mutate(model model.Model, mutationObjs ...model.Mutation) ([]ovsdb. return nil, err } - info, err := mapper.NewInfo(table, model) + info, err := mapper.NewInfo(tableName, table, model) if err != nil { return nil, err } @@ -305,7 +305,7 @@ func (a api) Mutate(model model.Model, mutationObjs ...model.Mutation) ([]ovsdb. return nil, err } - mutation, err := a.cache.Mapper().NewMutation(tableName, model, col, mobj.Mutator, mobj.Value) + mutation, err := a.cache.Mapper().NewMutation(info, col, mobj.Mutator, mobj.Value) if err != nil { return nil, err } @@ -335,12 +335,12 @@ func (a api) Update(model model.Model, fields ...interface{}) ([]ovsdb.Operation return nil, err } tableSchema := a.cache.Mapper().Schema.Table(table) + info, err := mapper.NewInfo(table, tableSchema, model) + if err != nil { + return nil, err + } if len(fields) > 0 { - info, err := mapper.NewInfo(tableSchema, model) - if err != nil { - return nil, err - } for _, f := range fields { colName, err := info.ColumnByPtr(f) if err != nil { @@ -357,7 +357,7 @@ func (a api) Update(model model.Model, fields ...interface{}) ([]ovsdb.Operation return nil, err } - row, err := a.cache.Mapper().NewRow(table, model, fields...) + row, err := a.cache.Mapper().NewRow(info, fields...) if err != nil { return nil, err } diff --git a/client/client.go b/client/client.go index 96876374..d8d45e9f 100644 --- a/client/client.go +++ b/client/client.go @@ -17,6 +17,7 @@ import ( "github.com/cenkalti/rpc2" "github.com/cenkalti/rpc2/jsonrpc" "github.com/ovn-org/libovsdb/cache" + "github.com/ovn-org/libovsdb/mapper" "github.com/ovn-org/libovsdb/model" "github.com/ovn-org/libovsdb/ovsdb" "github.com/ovn-org/libovsdb/ovsdb/serverdb" @@ -134,7 +135,7 @@ func newOVSDBClient(databaseModelRequest *model.DatabaseModelRequest, opts ...Op monitors: make(map[string]*Monitor), } } - ovs.metrics.init(databaseModel.Name()) + ovs.metrics.init(databaseModelRequest.Name()) return ovs, nil } @@ -146,7 +147,7 @@ func newOVSDBClient(databaseModelRequest *model.DatabaseModelRequest, opts ...Op func (o *ovsdbClient) Connect(ctx context.Context) error { // add the "model" value to the structured logger // to make it easier to tell between different DBs (e.g. ovn nbdb vs. sbdb) - l := o.options.logger.WithValues("model", o.primaryDB().model.Name()) + l := o.options.logger.WithValues("model", o.primaryDB().model.Request().Name()) o.options.logger = &l o.registerMetrics() @@ -701,7 +702,8 @@ func (o *ovsdbClient) monitor(ctx context.Context, cookie MonitorCookie, reconne dbName := cookie.DatabaseName db := o.databases[dbName] db.schemaMutex.RLock() - mapper := db.model.Mapper() + mmapper := db.model.Mapper() + schema := db.model.Schema() db.schemaMutex.RUnlock() typeMap := o.databases[dbName].model.Types() requests := make(map[string]ovsdb.MonitorRequest) @@ -710,7 +712,11 @@ func (o *ovsdbClient) monitor(ctx context.Context, cookie MonitorCookie, reconne if !ok { return fmt.Errorf("type for table %s does not exist in model", o.Table) } - request, err := mapper.NewMonitorRequest(o.Table, m, o.Fields) + info, err := mapper.NewInfo(o.Table, schema.Table(o.Table), m) + if err != nil { + return err + } + request, err := mmapper.NewMonitorRequest(info, o.Fields) if err != nil { return err } diff --git a/client/condition.go b/client/condition.go index cba7a7f7..ec396dce 100644 --- a/client/condition.go +++ b/client/condition.go @@ -28,12 +28,16 @@ type Conditional interface { type equalityConditional struct { mapper *mapper.Mapper tableName string - model model.Model + info *mapper.Info singleOp bool } func (c *equalityConditional) Matches(m model.Model) (bool, error) { - return c.mapper.EqualFields(c.tableName, c.model, m) + info, err := mapper.NewInfo(c.tableName, c.mapper.Schema.Table(c.tableName), m) + if err != nil { + return false, err + } + return c.mapper.EqualFields(c.info, info) } func (c *equalityConditional) Table() string { @@ -44,7 +48,7 @@ func (c *equalityConditional) Table() string { func (c *equalityConditional) Generate() ([][]ovsdb.Condition, error) { var result [][]ovsdb.Condition - conds, err := c.mapper.NewEqualityCondition(c.tableName, c.model) + conds, err := c.mapper.NewEqualityCondition(c.info) if err != nil { return nil, err } @@ -59,11 +63,15 @@ func (c *equalityConditional) Generate() ([][]ovsdb.Condition, error) { } // NewEqualityCondition creates a new equalityConditional -func newEqualityConditional(mapper *mapper.Mapper, table string, all bool, model model.Model, fields ...interface{}) (Conditional, error) { +func newEqualityConditional(m *mapper.Mapper, table string, all bool, model model.Model, fields ...interface{}) (Conditional, error) { + info, err := mapper.NewInfo(table, m.Schema.Table(table), model) + if err != nil { + return nil, err + } return &equalityConditional{ - mapper: mapper, + mapper: m, tableName: table, - model: model, + info: info, singleOp: all, }, nil } @@ -72,7 +80,7 @@ func newEqualityConditional(mapper *mapper.Mapper, table string, all bool, model type explicitConditional struct { mapper *mapper.Mapper tableName string - model model.Model + info *mapper.Info conditions []model.Condition singleOp bool } @@ -91,7 +99,7 @@ func (c *explicitConditional) Generate() ([][]ovsdb.Condition, error) { var conds []ovsdb.Condition for _, cond := range c.conditions { - ovsdbCond, err := c.mapper.NewCondition(c.tableName, c.model, cond.Field, cond.Function, cond.Value) + ovsdbCond, err := c.mapper.NewCondition(c.info, cond.Field, cond.Function, cond.Value) if err != nil { return nil, err } @@ -109,11 +117,15 @@ func (c *explicitConditional) Generate() ([][]ovsdb.Condition, error) { } // newIndexCondition creates a new equalityConditional -func newExplicitConditional(mapper *mapper.Mapper, table string, all bool, model model.Model, cond ...model.Condition) (Conditional, error) { +func newExplicitConditional(m *mapper.Mapper, table string, all bool, model model.Model, cond ...model.Condition) (Conditional, error) { + info, err := mapper.NewInfo(table, m.Schema.Table(table), model) + if err != nil { + return nil, err + } return &explicitConditional{ - mapper: mapper, + mapper: m, tableName: table, - model: model, + info: info, conditions: cond, singleOp: all, }, nil @@ -153,7 +165,11 @@ func (c *predicateConditional) Generate() ([][]ovsdb.Condition, error) { return nil, err } if match { - elemCond, err := c.cache.Mapper().NewEqualityCondition(c.tableName, elem) + info, err := mapper.NewInfo(c.tableName, c.cache.Mapper().Schema.Table(c.tableName), elem) + if err != nil { + return nil, err + } + elemCond, err := c.cache.Mapper().NewEqualityCondition(info) if err != nil { return nil, err } diff --git a/mapper/info.go b/mapper/info.go index 1ad981b6..e364ddd2 100644 --- a/mapper/info.go +++ b/mapper/info.go @@ -7,37 +7,42 @@ import ( "github.com/ovn-org/libovsdb/ovsdb" ) -// Info is a struct that handles the type map of an object -// The object must have exported tagged fields with the 'ovs' +// Info is a struct that wraps an object with its metadata type Info struct { // FieldName indexed by column - fields map[string]string - obj interface{} - table *ovsdb.TableSchema + Obj interface{} + Metadata *Metadata +} + +// Metadata represents the information needed to know how to map OVSDB columns into an objet's fields +type Metadata struct { + Fields map[string]string // Map of ColumnName -> FieldName + TableSchema *ovsdb.TableSchema // TableSchema associated + TableName string // Table name } // FieldByColumn returns the field value that corresponds to a column func (i *Info) FieldByColumn(column string) (interface{}, error) { - fieldName, ok := i.fields[column] + fieldName, ok := i.Metadata.Fields[column] if !ok { return nil, fmt.Errorf("FieldByColumn: column %s not found in orm info", column) } - return reflect.ValueOf(i.obj).Elem().FieldByName(fieldName).Interface(), nil + return reflect.ValueOf(i.Obj).Elem().FieldByName(fieldName).Interface(), nil } // FieldByColumn returns the field value that corresponds to a column func (i *Info) hasColumn(column string) bool { - _, ok := i.fields[column] + _, ok := i.Metadata.Fields[column] return ok } // SetField sets the field in the column to the specified value func (i *Info) SetField(column string, value interface{}) error { - fieldName, ok := i.fields[column] + fieldName, ok := i.Metadata.Fields[column] if !ok { return fmt.Errorf("SetField: column %s not found in orm info", column) } - fieldValue := reflect.ValueOf(i.obj).Elem().FieldByName(fieldName) + fieldValue := reflect.ValueOf(i.Obj).Elem().FieldByName(fieldName) if !fieldValue.Type().AssignableTo(reflect.TypeOf(value)) { return fmt.Errorf("column %s: native value %v (%s) is not assignable to field %s (%s)", @@ -53,12 +58,12 @@ func (i *Info) ColumnByPtr(fieldPtr interface{}) (string, error) { if fieldPtrVal.Kind() != reflect.Ptr { return "", ovsdb.NewErrWrongType("ColumnByPointer", "pointer to a field in the struct", fieldPtr) } - offset := fieldPtrVal.Pointer() - reflect.ValueOf(i.obj).Pointer() - objType := reflect.TypeOf(i.obj).Elem() + offset := fieldPtrVal.Pointer() - reflect.ValueOf(i.Obj).Pointer() + objType := reflect.TypeOf(i.Obj).Elem() for j := 0; j < objType.NumField(); j++ { if objType.Field(j).Offset == offset { column := objType.Field(j).Tag.Get("ovsdb") - if _, ok := i.fields[column]; !ok { + if _, ok := i.Metadata.Fields[column]; !ok { return "", fmt.Errorf("field does not have orm column information") } return column, nil @@ -74,7 +79,7 @@ func (i *Info) getValidIndexes() ([][]string, error) { var possibleIndexes [][]string possibleIndexes = append(possibleIndexes, []string{"_uuid"}) - possibleIndexes = append(possibleIndexes, i.table.Indexes...) + possibleIndexes = append(possibleIndexes, i.Metadata.TableSchema.Indexes...) // Iterate through indexes and validate them OUTER: @@ -83,7 +88,7 @@ OUTER: if !i.hasColumn(col) { continue OUTER } - columnSchema := i.table.Column(col) + columnSchema := i.Metadata.TableSchema.Column(col) if columnSchema == nil { continue OUTER } @@ -101,7 +106,7 @@ OUTER: } // NewInfo creates a MapperInfo structure around an object based on a given table schema -func NewInfo(table *ovsdb.TableSchema, obj interface{}) (*Info, error) { +func NewInfo(tableName string, table *ovsdb.TableSchema, obj interface{}) (*Info, error) { objPtrVal := reflect.ValueOf(obj) if objPtrVal.Type().Kind() != reflect.Ptr { return nil, ovsdb.NewErrWrongType("NewMapperInfo", "pointer to a struct", obj) @@ -146,8 +151,11 @@ func NewInfo(table *ovsdb.TableSchema, obj interface{}) (*Info, error) { } return &Info{ - fields: fields, - obj: obj, - table: table, + Obj: obj, + Metadata: &Metadata{ + Fields: fields, + TableSchema: table, + TableName: tableName, + }, }, nil } diff --git a/mapper/info_test.go b/mapper/info_test.go index b50633f3..cf5ad960 100644 --- a/mapper/info_test.go +++ b/mapper/info_test.go @@ -58,7 +58,7 @@ func TestNewMapperInfo(t *testing.T) { err := json.Unmarshal(tt.table, &table) assert.Nil(t, err) - info, err := NewInfo(&table, tt.obj) + info, err := NewInfo("Test", &table, tt.obj) if tt.err { assert.NotNil(t, err) } else { @@ -67,6 +67,7 @@ func TestNewMapperInfo(t *testing.T) { for _, col := range tt.expectedCols { assert.Truef(t, info.hasColumn(col), "Expected column should be present in Mapper Info") } + assert.Equal(t, "Test", info.Metadata.TableName) }) } @@ -141,7 +142,7 @@ func TestMapperInfoSet(t *testing.T) { err := json.Unmarshal(tt.table, &table) assert.Nil(t, err) - info, err := NewInfo(&table, tt.obj) + info, err := NewInfo("Test", &table, tt.obj) assert.Nil(t, err) err = info.SetField(tt.column, tt.field) @@ -222,7 +223,7 @@ func TestMapperInfoColByPtr(t *testing.T) { err := json.Unmarshal(tt.table, &table) assert.Nil(t, err) - info, err := NewInfo(&table, tt.obj) + info, err := NewInfo("Test", &table, tt.obj) assert.Nil(t, err) col, err := info.ColumnByPtr(tt.field) @@ -354,7 +355,7 @@ func TestOrmGetIndex(t *testing.T) { } for _, tt := range tests { t.Run(fmt.Sprintf("GetValidIndexes_%s", tt.name), func(t *testing.T) { - info, err := NewInfo(&table, tt.obj) + info, err := NewInfo("Test", &table, tt.obj) assert.Nil(t, err) indexes, err := info.getValidIndexes() diff --git a/mapper/mapper.go b/mapper/mapper.go index 2a66680c..ae01e4b2 100644 --- a/mapper/mapper.go +++ b/mapper/mapper.go @@ -36,20 +36,20 @@ func (e *ErrMapper) Error() string { e.objType, e.field, e.fieldType, e.fieldTag, e.reason) } -// ErrNoTable describes a error in the provided table information -type ErrNoTable struct { - table string -} +//// ErrNoTable describes a error in the provided table information +//type ErrNoTable struct { +// table string +//} +// +//func (e *ErrNoTable) Error() string { +// return fmt.Sprintf("Table not found: %s", e.table) +//} -func (e *ErrNoTable) Error() string { - return fmt.Sprintf("Table not found: %s", e.table) -} - -func newErrNoTable(table string) error { - return &ErrNoTable{ - table: table, - } -} +//func newErrNoTable(table string) error { +// return &ErrNoTable{ +// table: table, +// } +//} // NewMapper returns a new mapper func NewMapper(schema *ovsdb.DatabaseSchema) *Mapper { @@ -60,29 +60,19 @@ func NewMapper(schema *ovsdb.DatabaseSchema) *Mapper { // GetRowData transforms a Row to a struct based on its tags // The result object must be given as pointer to an object with the right tags -func (m Mapper) GetRowData(tableName string, row *ovsdb.Row, result interface{}) error { +func (m Mapper) GetRowData(row *ovsdb.Row, result *Info) error { if row == nil { return nil } - return m.getData(tableName, *row, result) + return m.getData(*row, result) } // getData transforms a map[string]interface{} containing OvS types (e.g: a ResultRow // has this format) to orm struct // The result object must be given as pointer to an object with the right tags -func (m Mapper) getData(tableName string, ovsData ovsdb.Row, result interface{}) error { - table := m.Schema.Table(tableName) - if table == nil { - return newErrNoTable(tableName) - } - - mapperInfo, err := NewInfo(table, result) - if err != nil { - return err - } - - for name, column := range table.Columns { - if !mapperInfo.hasColumn(name) { +func (m Mapper) getData(ovsData ovsdb.Row, result *Info) error { + for name, column := range result.Metadata.TableSchema.Columns { + if !result.hasColumn(name) { // If provided struct does not have a field to hold this value, skip it continue } @@ -96,10 +86,10 @@ func (m Mapper) getData(tableName string, ovsData ovsdb.Row, result interface{}) nativeElem, err := ovsdb.OvsToNative(column, ovsElem) if err != nil { return fmt.Errorf("table %s, column %s: failed to extract native element: %s", - tableName, name, err.Error()) + result.Metadata.TableName, name, err.Error()) } - if err := mapperInfo.SetField(name, nativeElem); err != nil { + if err := result.SetField(name, nativeElem); err != nil { return err } } @@ -109,24 +99,15 @@ func (m Mapper) getData(tableName string, ovsData ovsdb.Row, result interface{}) // NewRow transforms an orm struct to a map[string] interface{} that can be used as libovsdb.Row // By default, default or null values are skipped. This behavior can be modified by specifying // a list of fields (pointers to fields in the struct) to be added to the row -func (m Mapper) NewRow(tableName string, data interface{}, fields ...interface{}) (ovsdb.Row, error) { - table := m.Schema.Table(tableName) - if table == nil { - return nil, newErrNoTable(tableName) - } - mapperInfo, err := NewInfo(table, data) - if err != nil { - return nil, err - } - +func (m Mapper) NewRow(data *Info, fields ...interface{}) (ovsdb.Row, error) { columns := make(map[string]*ovsdb.ColumnSchema) - for k, v := range table.Columns { + for k, v := range data.Metadata.TableSchema.Columns { columns[k] = v } columns["_uuid"] = &ovsdb.UUIDColumn ovsRow := make(map[string]interface{}, len(columns)) for name, column := range columns { - nativeElem, err := mapperInfo.FieldByColumn(name) + nativeElem, err := data.FieldByColumn(name) if err != nil { // If provided struct does not have a field to hold this value, skip it continue @@ -136,7 +117,7 @@ func (m Mapper) NewRow(tableName string, data interface{}, fields ...interface{} if len(fields) > 0 { found := false for _, f := range fields { - col, err := mapperInfo.ColumnByPtr(f) + col, err := data.ColumnByPtr(f) if err != nil { return nil, err } @@ -154,7 +135,7 @@ func (m Mapper) NewRow(tableName string, data interface{}, fields ...interface{} } ovsElem, err := ovsdb.NativeToOvs(column, nativeElem) if err != nil { - return nil, fmt.Errorf("table %s, column %s: failed to generate ovs element. %s", tableName, name, err.Error()) + return nil, fmt.Errorf("table %s, column %s: failed to generate ovs element. %s", data.Metadata.TableName, name, err.Error()) } ovsRow[name] = ovsElem } @@ -169,25 +150,15 @@ func (m Mapper) NewRow(tableName string, data interface{}, fields ...interface{} // object has valid data. The order in which they are traversed matches the order defined // in the schema. // By `valid data` we mean non-default data. -func (m Mapper) NewEqualityCondition(tableName string, data interface{}, fields ...interface{}) ([]ovsdb.Condition, error) { +func (m Mapper) NewEqualityCondition(data *Info, fields ...interface{}) ([]ovsdb.Condition, error) { var conditions []ovsdb.Condition var condIndex [][]string - table := m.Schema.Table(tableName) - if table == nil { - return nil, newErrNoTable(tableName) - } - - mapperInfo, err := NewInfo(table, data) - if err != nil { - return nil, err - } - // If index is provided, use it. If not, obtain the valid indexes from the mapper info if len(fields) > 0 { providedIndex := []string{} for i := range fields { - if col, err := mapperInfo.ColumnByPtr(fields[i]); err == nil { + if col, err := data.ColumnByPtr(fields[i]); err == nil { providedIndex = append(providedIndex, col) } else { return nil, err @@ -196,7 +167,7 @@ func (m Mapper) NewEqualityCondition(tableName string, data interface{}, fields condIndex = append(condIndex, providedIndex) } else { var err error - condIndex, err = mapperInfo.getValidIndexes() + condIndex, err = data.getValidIndexes() if err != nil { return nil, err } @@ -208,12 +179,12 @@ func (m Mapper) NewEqualityCondition(tableName string, data interface{}, fields // Pick the first valid index for _, col := range condIndex[0] { - field, err := mapperInfo.FieldByColumn(col) + field, err := data.FieldByColumn(col) if err != nil { return nil, err } - column := table.Column(col) + column := data.Metadata.TableSchema.Column(col) if column == nil { return nil, fmt.Errorf("column %s not found", col) } @@ -229,47 +200,27 @@ func (m Mapper) NewEqualityCondition(tableName string, data interface{}, fields // EqualFields compares two mapped objects. // The indexes to use for comparison are, the _uuid, the table indexes and the columns that correspond // to the mapped fields pointed to by 'fields'. They must be pointers to fields on the first mapped element (i.e: one) -func (m Mapper) EqualFields(tableName string, one, other interface{}, fields ...interface{}) (bool, error) { +func (m Mapper) EqualFields(one, other *Info, fields ...interface{}) (bool, error) { indexes := []string{} - - table := m.Schema.Table(tableName) - if table == nil { - return false, newErrNoTable(tableName) - } - - info, err := NewInfo(table, one) - if err != nil { - return false, err - } for _, f := range fields { - col, err := info.ColumnByPtr(f) + col, err := one.ColumnByPtr(f) if err != nil { return false, err } indexes = append(indexes, col) } - return m.equalIndexes(table, one, other, indexes...) + return m.equalIndexes(one, other, indexes...) } // NewCondition returns a ovsdb.Condition based on the model -func (m Mapper) NewCondition(tableName string, data interface{}, field interface{}, function ovsdb.ConditionFunction, value interface{}) (*ovsdb.Condition, error) { - table := m.Schema.Table(tableName) - if table == nil { - return nil, newErrNoTable(tableName) - } - - info, err := NewInfo(table, data) - if err != nil { - return nil, err - } - - column, err := info.ColumnByPtr(field) +func (m Mapper) NewCondition(data *Info, field interface{}, function ovsdb.ConditionFunction, value interface{}) (*ovsdb.Condition, error) { + column, err := data.ColumnByPtr(field) if err != nil { return nil, err } // Check that the condition is valid - columnSchema := table.Column(column) + columnSchema := data.Metadata.TableSchema.Column(column) if columnSchema == nil { return nil, fmt.Errorf("column %s not found", column) } @@ -290,23 +241,13 @@ func (m Mapper) NewCondition(tableName string, data interface{}, field interface // NewMutation creates a RFC7047 mutation object based on an ORM object and the mutation fields (in native format) // It takes care of field validation against the column type -func (m Mapper) NewMutation(tableName string, data interface{}, column string, mutator ovsdb.Mutator, value interface{}) (*ovsdb.Mutation, error) { - table := m.Schema.Table(tableName) - if table == nil { - return nil, newErrNoTable(tableName) - } - - mapperInfo, err := NewInfo(table, data) - if err != nil { - return nil, err - } - +func (m Mapper) NewMutation(data *Info, column string, mutator ovsdb.Mutator, value interface{}) (*ovsdb.Mutation, error) { // Check the column exists in the object - if !mapperInfo.hasColumn(column) { + if !data.hasColumn(column) { return nil, fmt.Errorf("mutation contains column %s that does not exist in object %v", column, data) } // Check that the mutation is valid - columnSchema := table.Column(column) + columnSchema := data.Metadata.TableSchema.Column(column) if columnSchema == nil { return nil, fmt.Errorf("column %s not found", column) } @@ -315,6 +256,7 @@ func (m Mapper) NewMutation(tableName string, data interface{}, column string, m } var ovsValue interface{} + var err error // Usually a mutation value is of the same type of the value being mutated // except for delete mutation of maps where it can also be a list of same type of // keys (rfc7047 5.1). Handle this special case here. @@ -341,24 +283,15 @@ func (m Mapper) NewMutation(tableName string, data interface{}, column string, m // For any of the indexes defined in the Table Schema, the values all of its columns are simultaneously equal // (as per RFC7047) // The values of all of the optional indexes passed as variadic parameter to this function are equal. -func (m Mapper) equalIndexes(table *ovsdb.TableSchema, one, other interface{}, indexes ...string) (bool, error) { +func (m Mapper) equalIndexes(one, other *Info, indexes ...string) (bool, error) { match := false - oneMapperInfo, err := NewInfo(table, one) - if err != nil { - return false, err - } - otherMapperInfo, err := NewInfo(table, other) - if err != nil { - return false, err - } - - oneIndexes, err := oneMapperInfo.getValidIndexes() + oneIndexes, err := one.getValidIndexes() if err != nil { return false, err } - otherIndexes, err := otherMapperInfo.getValidIndexes() + otherIndexes, err := other.getValidIndexes() if err != nil { return false, err } @@ -371,14 +304,14 @@ func (m Mapper) equalIndexes(table *ovsdb.TableSchema, one, other interface{}, i if reflect.DeepEqual(ridx, lidx) { // All columns in an index must be simultaneously equal for _, col := range lidx { - if !oneMapperInfo.hasColumn(col) || !otherMapperInfo.hasColumn(col) { + if !one.hasColumn(col) || !other.hasColumn(col) { break } - lfield, err := oneMapperInfo.FieldByColumn(col) + lfield, err := one.FieldByColumn(col) if err != nil { return false, err } - rfield, err := otherMapperInfo.FieldByColumn(col) + rfield, err := other.FieldByColumn(col) if err != nil { return false, err } @@ -401,23 +334,18 @@ func (m Mapper) equalIndexes(table *ovsdb.TableSchema, one, other interface{}, i // NewMonitorRequest returns a monitor request for the provided tableName // If fields is provided, the request will be constrained to the provided columns // If no fields are provided, all columns will be used -func (m *Mapper) NewMonitorRequest(tableName string, data interface{}, fields []interface{}) (*ovsdb.MonitorRequest, error) { +func (m *Mapper) NewMonitorRequest(data *Info, fields []interface{}) (*ovsdb.MonitorRequest, error) { var columns []string - schema := m.Schema.Tables[tableName] - info, err := NewInfo(&schema, data) - if err != nil { - return nil, err - } if len(fields) > 0 { for _, f := range fields { - column, err := info.ColumnByPtr(f) + column, err := data.ColumnByPtr(f) if err != nil { return nil, err } columns = append(columns, column) } } else { - for c := range info.table.Columns { + for c := range data.Metadata.TableSchema.Columns { columns = append(columns, c) } } diff --git a/mapper/mapper_test.go b/mapper/mapper_test.go index 5cb50e8b..7cb3aa80 100644 --- a/mapper/mapper_test.go +++ b/mapper/mapper_test.go @@ -226,7 +226,11 @@ func TestMapperGetData(t *testing.T) { test := ormTestType{ NonTagged: "something", } - err := mapper.GetRowData("TestTable", &ovsRow, &test) + testInfo, err := NewInfo("TestTable", schema.Table("TestTable"), &test) + assert.NoError(t, err) + + err = mapper.GetRowData(&ovsRow, testInfo) + assert.NoError(t, err) /*End code under test*/ if err != nil { @@ -341,7 +345,9 @@ func TestMapperNewRow(t *testing.T) { for _, test := range tests { t.Run(fmt.Sprintf("NewRow: %s", test.name), func(t *testing.T) { mapper := NewMapper(&schema) - row, err := mapper.NewRow("TestTable", test.objInput) + info, err := NewInfo("TestTable", schema.Table("TestTable"), test.objInput) + assert.NoError(t, err) + row, err := mapper.NewRow(info) if test.shoulderr { assert.NotNil(t, err) } else { @@ -432,7 +438,9 @@ func TestMapperNewRowFields(t *testing.T) { testObj.MyFloat = 0 test.prepare(&testObj) - row, err := mapper.NewRow("TestTable", &testObj, test.fields...) + info, err := NewInfo("TestTable", schema.Table("TestTable"), &testObj) + assert.NoError(t, err) + row, err := mapper.NewRow(info, test.fields...) if test.err { assert.NotNil(t, err) } else { @@ -584,7 +592,10 @@ func TestMapperCondition(t *testing.T) { for _, tt := range tests { t.Run(fmt.Sprintf("newEqualityCondition_%s", tt.name), func(t *testing.T) { tt.prepare(&testObj) - conds, err := mapper.NewEqualityCondition("TestTable", &testObj, tt.index...) + info, err := NewInfo("TestTable", schema.Table("TestTable"), &testObj) + assert.NoError(t, err) + + conds, err := mapper.NewEqualityCondition(info, tt.index...) if tt.err { if err == nil { t.Errorf("expected an error but got none") @@ -835,7 +846,11 @@ func TestMapperEqualIndexes(t *testing.T) { } for _, test := range tests { t.Run(fmt.Sprintf("Equal %s", test.name), func(t *testing.T) { - eq, err := mapper.equalIndexes(mapper.Schema.Table("TestTable"), &test.obj1, &test.obj2, test.indexes...) + info1, err := NewInfo("TestTable", schema.Table("TestTable"), &test.obj1) + assert.NoError(t, err) + info2, err := NewInfo("TestTable", schema.Table("TestTable"), &test.obj2) + assert.NoError(t, err) + eq, err := mapper.equalIndexes(info1, info2, test.indexes...) assert.Nil(t, err) assert.Equalf(t, test.expected, eq, "equal value should match expected") }) @@ -858,11 +873,15 @@ func TestMapperEqualIndexes(t *testing.T) { Int1: 42, Int2: 25, } - eq, err := mapper.EqualFields("TestTable", &obj1, &obj2, &obj1.Int1, &obj1.Int2) + info1, err := NewInfo("TestTable", schema.Table("TestTable"), &obj1) + assert.NoError(t, err) + info2, err := NewInfo("TestTable", schema.Table("TestTable"), &obj2) + assert.NoError(t, err) + eq, err := mapper.EqualFields(info1, info2, &obj1.Int1, &obj1.Int2) assert.Nil(t, err) assert.True(t, eq) // Using pointers to second value is not supported - _, err = mapper.EqualFields("TestTable", &obj1, &obj2, &obj2.Int1, &obj2.Int2) + _, err = mapper.EqualFields(info1, info2, &obj2.Int1, &obj2.Int2) assert.NotNil(t, err) } @@ -1012,7 +1031,10 @@ func TestMapperMutation(t *testing.T) { } for _, test := range tests { t.Run(fmt.Sprintf("newMutation%s", test.name), func(t *testing.T) { - mutation, err := mapper.NewMutation("TestTable", &test.obj, test.column, test.mutator, test.value) + info, err := NewInfo("TestTable", schema.Table("TestTable"), &test.obj) + assert.NoError(t, err) + + mutation, err := mapper.NewMutation(info, test.column, test.mutator, test.value) if test.err { if err == nil { t.Errorf("expected an error but got none") @@ -1097,10 +1119,12 @@ func TestNewMonitorRequest(t *testing.T) { require.NoError(t, err) mapper := NewMapper(&schema) testTable := &testType{} - mr, err := mapper.NewMonitorRequest("TestTable", testTable, nil) + info, err := NewInfo("TestTable", schema.Table("TestTable"), testTable) + assert.NoError(t, err) + mr, err := mapper.NewMonitorRequest(info, nil) require.NoError(t, err) assert.ElementsMatch(t, mr.Columns, []string{"name", "config", "composed_1", "composed_2", "int1", "int2"}) - mr2, err := mapper.NewMonitorRequest("TestTable", testTable, []interface{}{&testTable.Int1, &testTable.MyName}) + mr2, err := mapper.NewMonitorRequest(info, []interface{}{&testTable.Int1, &testTable.MyName}) require.NoError(t, err) assert.ElementsMatch(t, mr2.Columns, []string{"int1", "name"}) } diff --git a/model/request.go b/model/request.go index e1e2ce44..e8b82501 100644 --- a/model/request.go +++ b/model/request.go @@ -49,7 +49,7 @@ func (db DatabaseModelRequest) validate(schema *ovsdb.DatabaseSchema) []error { errors = append(errors, err) continue } - if _, err := mapper.NewInfo(tableSchema, model); err != nil { + if _, err := mapper.NewInfo(tableName, tableSchema, model); err != nil { errors = append(errors, err) } } diff --git a/server/server.go b/server/server.go index 3a198c6c..6cf292df 100644 --- a/server/server.go +++ b/server/server.go @@ -141,8 +141,8 @@ type Transaction struct { Cache *cache.TableCache } -func NewTransaction(schema *ovsdb.DatabaseSchema, model *model.DatabaseModelRequest) Transaction { - cache, err := cache.NewTableCache(schema, model, nil) +func NewTransaction(schema *ovsdb.DatabaseSchema, model *model.DatabaseModel) Transaction { + cache, err := cache.NewTableCache(model, nil) if err != nil { panic(err) } diff --git a/server/transact.go b/server/transact.go index d64d1f50..b0ba5341 100644 --- a/server/transact.go +++ b/server/transact.go @@ -89,7 +89,13 @@ func (o *OvsdbServer) Insert(database string, table string, rowUUID string, row }, nil } - err = m.GetRowData(table, &row, model) + mapperInfo, err := mapper.NewInfo(table, tSchema, model) + if err != nil { + return ovsdb.OperationResult{ + Error: err.Error(), + }, nil + } + err = m.GetRowData(&row, mapperInfo) if err != nil { return ovsdb.OperationResult{ Error: err.Error(), @@ -97,12 +103,6 @@ func (o *OvsdbServer) Insert(database string, table string, rowUUID string, row } if rowUUID != "" { - mapperInfo, err := mapper.NewInfo(tSchema, model) - if err != nil { - return ovsdb.OperationResult{ - Error: err.Error(), - }, nil - } if err := mapperInfo.SetField("_uuid", rowUUID); err != nil { return ovsdb.OperationResult{ Error: err.Error(), @@ -110,7 +110,7 @@ func (o *OvsdbServer) Insert(database string, table string, rowUUID string, row } } - resultRow, err := m.NewRow(table, model) + resultRow, err := m.NewRow(mapperInfo) if err != nil { return ovsdb.OperationResult{ Error: err.Error(), @@ -163,7 +163,11 @@ func (o *OvsdbServer) Select(database string, table string, where []ovsdb.Condit panic(err) } for _, row := range rows { - resultRow, err := m.NewRow(table, row) + info, err := mapper.NewInfo(table, dbModel.Schema().Table(table), row) + if err != nil { + panic(err) + } + resultRow, err := m.NewRow(info) if err != nil { panic(err) } @@ -194,10 +198,10 @@ func (o *OvsdbServer) Update(database, table string, where []ovsdb.Condition, ro }, nil } for _, old := range rows { - info, _ := mapper.NewInfo(schema, old) - uuid, _ := info.FieldByColumn("_uuid") + oldInfo, _ := mapper.NewInfo(table, schema, old) + uuid, _ := oldInfo.FieldByColumn("_uuid") - oldRow, err := m.NewRow(table, old) + oldRow, err := m.NewRow(oldInfo) if err != nil { panic(err) } @@ -205,15 +209,15 @@ func (o *OvsdbServer) Update(database, table string, where []ovsdb.Condition, ro if err != nil { panic(err) } - err = m.GetRowData(table, &oldRow, new) + newInfo, err := mapper.NewInfo(table, schema, new) if err != nil { panic(err) } - info, err = mapper.NewInfo(schema, new) + err = m.GetRowData(&oldRow, newInfo) if err != nil { panic(err) } - err = info.SetField("_uuid", uuid) + err = newInfo.SetField("_uuid", uuid) if err != nil { panic(err) } @@ -235,7 +239,7 @@ func (o *OvsdbServer) Update(database, table string, where []ovsdb.Condition, ro Details: fmt.Sprintf("column %s is of table %s not mutable", column, table), }, nil } - old, err := info.FieldByColumn(column) + old, err := newInfo.FieldByColumn(column) if err != nil { panic(err) } @@ -254,7 +258,7 @@ func (o *OvsdbServer) Update(database, table string, where []ovsdb.Condition, ro continue } - err = info.SetField(column, native) + err = newInfo.SetField(column, native) if err != nil { panic(err) } @@ -270,7 +274,7 @@ func (o *OvsdbServer) Update(database, table string, where []ovsdb.Condition, ro } } - newRow, err := m.NewRow(table, new) + newRow, err := m.NewRow(newInfo) if err != nil { panic(err) } @@ -324,12 +328,12 @@ func (o *OvsdbServer) Mutate(database, table string, where []ovsdb.Condition, mu } for _, old := range rows { - oldInfo, err := mapper.NewInfo(schema, old) + oldInfo, err := mapper.NewInfo(table, schema, old) if err != nil { panic(err) } uuid, _ := oldInfo.FieldByColumn("_uuid") - oldRow, err := m.NewRow(table, old) + oldRow, err := m.NewRow(oldInfo) if err != nil { panic(err) } @@ -337,11 +341,11 @@ func (o *OvsdbServer) Mutate(database, table string, where []ovsdb.Condition, mu if err != nil { panic(err) } - err = m.GetRowData(table, &oldRow, new) + newInfo, err := mapper.NewInfo(table, schema, new) if err != nil { panic(err) } - newInfo, err := mapper.NewInfo(schema, new) + err = m.GetRowData(&oldRow, newInfo) if err != nil { panic(err) } @@ -424,7 +428,7 @@ func (o *OvsdbServer) Mutate(database, table string, where []ovsdb.Condition, mu }, nil } - newRow, err := m.NewRow(table, new) + newRow, err := m.NewRow(newInfo) if err != nil { panic(err) } @@ -460,9 +464,9 @@ func (o *OvsdbServer) Delete(database, table string, where []ovsdb.Condition) (o panic(err) } for _, row := range rows { - info, _ := mapper.NewInfo(schema, row) + info, _ := mapper.NewInfo(table, schema, row) uuid, _ := info.FieldByColumn("_uuid") - oldRow, err := m.NewRow(table, row) + oldRow, err := m.NewRow(info) if err != nil { panic(err) } diff --git a/server/transact_test.go b/server/transact_test.go index fbfa8032..c2849654 100644 --- a/server/transact_test.go +++ b/server/transact_test.go @@ -32,7 +32,9 @@ func TestMutateOp(t *testing.T) { m := mapper.NewMapper(schema) ovs := ovsType{} - ovsRow, err := m.NewRow("Open_vSwitch", &ovs) + info, err := mapper.NewInfo("Open_vSwitch", schema.Table("Open_vSwitch"), &ovs) + require.NoError(t, err) + ovsRow, err := m.NewRow(info) require.Nil(t, err) bridge := bridgeType{ @@ -43,7 +45,9 @@ func TestMutateOp(t *testing.T) { "waldo": "fred", }, } - bridgeRow, err := m.NewRow("Bridge", &bridge) + bridgeInfo, err := mapper.NewInfo("Bridge", schema.Table("Bridge"), &bridge) + require.NoError(t, err) + bridgeRow, err := m.NewRow(bridgeInfo) require.Nil(t, err) res, updates := o.Insert("Open_vSwitch", "Open_vSwitch", ovsUUID, ovsRow) @@ -214,8 +218,7 @@ func TestOvsdbServerInsert(t *testing.T) { t.Fatal(err) } ovsDB := NewInMemoryDatabase(map[string]*model.DatabaseModelRequest{"Open_vSwitch": defDB}) - o, err := NewOvsdbServer(ovsDB, DatabaseModel{ - Model: defDB, Schema: schema}) + o, err := NewOvsdbServer(ovsDB, *model.NewDatabaseModel(schema, defDB)) require.Nil(t, err) m := mapper.NewMapper(schema) @@ -231,7 +234,9 @@ func TestOvsdbServerInsert(t *testing.T) { }, } bridgeUUID := uuid.NewString() - bridgeRow, err := m.NewRow("Bridge", &bridge) + bridgeInfo, err := mapper.NewInfo("Bridge", schema.Table("Bridge"), &bridge) + require.NoError(t, err) + bridgeRow, err := m.NewRow(bridgeInfo) require.Nil(t, err) res, updates := o.Insert("Open_vSwitch", "Bridge", bridgeUUID, bridgeRow) @@ -267,8 +272,7 @@ func TestOvsdbServerUpdate(t *testing.T) { t.Fatal(err) } ovsDB := NewInMemoryDatabase(map[string]*model.DatabaseModelRequest{"Open_vSwitch": defDB}) - o, err := NewOvsdbServer(ovsDB, DatabaseModel{ - Model: defDB, Schema: schema}) + o, err := NewOvsdbServer(ovsDB, *model.NewDatabaseModel(schema, defDB)) require.Nil(t, err) m := mapper.NewMapper(schema) @@ -281,7 +285,9 @@ func TestOvsdbServerUpdate(t *testing.T) { }, } bridgeUUID := uuid.NewString() - bridgeRow, err := m.NewRow("Bridge", &bridge) + bridgeInfo, err := mapper.NewInfo("Bridge", schema.Table("Bridge"), &bridge) + require.NoError(t, err) + bridgeRow, err := m.NewRow(bridgeInfo) require.Nil(t, err) res, updates := o.Insert("Open_vSwitch", "Bridge", bridgeUUID, bridgeRow) From f4daab40ae63e5d98365f272bd97e9afaeae8d47 Mon Sep 17 00:00:00 2001 From: Adrian Moreno Date: Mon, 11 Oct 2021 11:05:06 +0200 Subject: [PATCH 09/10] model: Use DatabaseModel to create mapper.Info The core mapper API uses mapper.Info sctructs which can be created just by inspecting a TableSchema. However, having the DatabaseModel now centralizing accesses to the mapper API and containing both the Model types and the Schema, we can pre-create the mapper.Info.Metadata sctructs and cache them so we create Info sctructs more efficiently Signed-off-by: Adrian Moreno --- cache/cache.go | 44 ++++---- cache/cache_test.go | 168 ++++++++++++++++++++++-------- client/api.go | 13 +-- client/api_test_model.go | 5 +- client/client.go | 10 +- client/client_test.go | 18 ++-- client/condition.go | 26 ++--- client/condition_test.go | 4 +- example/ovsdb-server/main.go | 7 +- model/database.go | 72 ++++++++++--- server/database.go | 5 +- server/server_integration_test.go | 24 +++-- server/server_test.go | 4 +- server/transact.go | 18 ++-- server/transact_test.go | 20 ++-- 15 files changed, 298 insertions(+), 140 deletions(-) diff --git a/cache/cache.go b/cache/cache.go index be3c4801..c557cd2b 100644 --- a/cache/cache.go +++ b/cache/cache.go @@ -66,7 +66,7 @@ func newIndex(columns ...string) index { // RowCache is a collections of Models hashed by UUID type RowCache struct { name string - schema ovsdb.TableSchema + dbModel *model.DatabaseModel dataType reflect.Type cache map[string]model.Model indexes columnToValue @@ -90,7 +90,7 @@ func (r *RowCache) RowByModel(m model.Model) model.Model { if reflect.TypeOf(m) != r.dataType { return nil } - info, _ := mapper.NewInfo(r.name, &r.schema, m) + info, _ := r.dbModel.NewModelInfo(m) uuid, err := info.FieldByColumn("_uuid") if err != nil { return nil @@ -120,11 +120,11 @@ func (r *RowCache) Create(uuid string, m model.Model, checkIndexes bool) error { if reflect.TypeOf(m) != r.dataType { return fmt.Errorf("expected data of type %s, but got %s", r.dataType.String(), reflect.TypeOf(m).String()) } - info, err := mapper.NewInfo(r.name, &r.schema, m) + info, err := r.dbModel.NewModelInfo(m) if err != nil { return err } - newIndexes := newColumnToValue(r.schema.Indexes) + newIndexes := newColumnToValue(r.dbModel.Schema().Table(r.name).Indexes) for index := range r.indexes { val, err := valueFromIndex(info, index) if err != nil { @@ -156,16 +156,17 @@ func (r *RowCache) Update(uuid string, m model.Model, checkIndexes bool) error { return fmt.Errorf("row %s does not exist", uuid) } oldRow := model.Clone(r.cache[uuid]) - oldInfo, err := mapper.NewInfo(r.name, &r.schema, oldRow) + oldInfo, err := r.dbModel.NewModelInfo(oldRow) if err != nil { return err } - newInfo, err := mapper.NewInfo(r.name, &r.schema, m) + newInfo, err := r.dbModel.NewModelInfo(m) if err != nil { return err } - newIndexes := newColumnToValue(r.schema.Indexes) - oldIndexes := newColumnToValue(r.schema.Indexes) + indexes := r.dbModel.Schema().Table(r.name).Indexes + newIndexes := newColumnToValue(indexes) + oldIndexes := newColumnToValue(indexes) var errs []error for index := range r.indexes { var err error @@ -218,7 +219,7 @@ func (r *RowCache) Update(uuid string, m model.Model, checkIndexes bool) error { } func (r *RowCache) IndexExists(row model.Model) error { - info, err := mapper.NewInfo(r.name, &r.schema, row) + info, err := r.dbModel.NewModelInfo(row) if err != nil { return err } @@ -252,7 +253,7 @@ func (r *RowCache) Delete(uuid string) error { return fmt.Errorf("row %s does not exist", uuid) } oldRow := r.cache[uuid] - oldInfo, err := mapper.NewInfo(r.name, &r.schema, oldRow) + oldInfo, err := r.dbModel.NewModelInfo(oldRow) if err != nil { return err } @@ -280,6 +281,7 @@ func (r *RowCache) Rows() []string { func (r *RowCache) RowsByCondition(conditions []ovsdb.Condition) ([]model.Model, error) { var results []model.Model + schema := r.dbModel.Schema().Table(r.name) if len(conditions) == 0 { uuids := r.Rows() for _, uuid := range uuids { @@ -308,7 +310,7 @@ func (r *RowCache) RowsByCondition(conditions []ovsdb.Condition) ([]model.Model, } } else if index, err := r.Index(condition.Column); err != nil { for k, v := range index { - tSchema := r.schema.Columns[condition.Column] + tSchema := schema.Columns[condition.Column] nativeValue, err := ovsdb.OvsToNative(tSchema, condition.Value) if err != nil { return nil, err @@ -325,7 +327,7 @@ func (r *RowCache) RowsByCondition(conditions []ovsdb.Condition) ([]model.Model, } else { for _, uuid := range r.Rows() { row := r.Row(uuid) - info, err := mapper.NewInfo(r.name, &r.schema, row) + info, err := r.dbModel.NewModelInfo(row) if err != nil { return nil, err } @@ -422,8 +424,8 @@ func NewTableCache(dbModel *model.DatabaseModel, data Data) (*TableCache, error) eventProcessor := newEventProcessor(bufferSize) cache := make(map[string]*RowCache) tableTypes := dbModel.Types() - for name, tableSchema := range dbModel.Schema().Tables { - cache[name] = newRowCache(name, tableSchema, tableTypes[name]) + for name := range dbModel.Schema().Tables { + cache[name] = newRowCache(name, dbModel, tableTypes[name]) } for table, rowData := range data { if _, ok := dbModel.Schema().Tables[table]; !ok { @@ -626,8 +628,8 @@ func (t *TableCache) Purge(dbModel *model.DatabaseModel) { defer t.mutex.Unlock() t.dbModel = dbModel tableTypes := t.dbModel.Types() - for name, tableSchema := range t.dbModel.Schema().Tables { - t.cache[name] = newRowCache(name, tableSchema, tableTypes[name]) + for name := range t.dbModel.Schema().Tables { + t.cache[name] = newRowCache(name, t.dbModel, tableTypes[name]) } } @@ -643,11 +645,11 @@ func (t *TableCache) Run(stopCh <-chan struct{}) { // newRowCache creates a new row cache with the provided data // if the data is nil, and empty RowCache will be created -func newRowCache(name string, schema ovsdb.TableSchema, dataType reflect.Type) *RowCache { +func newRowCache(name string, dbModel *model.DatabaseModel, dataType reflect.Type) *RowCache { r := &RowCache{ name: name, - schema: schema, - indexes: newColumnToValue(schema.Indexes), + dbModel: dbModel, + indexes: newColumnToValue(dbModel.Schema().Table(name).Indexes), dataType: dataType, cache: make(map[string]model.Model), mutex: sync.RWMutex{}, @@ -761,7 +763,7 @@ func (t *TableCache) CreateModel(tableName string, row *ovsdb.Row, uuid string) if err != nil { return nil, err } - info, err := mapper.NewInfo(tableName, table, model) + info, err := t.dbModel.NewModelInfo(model) if err != nil { return nil, err } @@ -790,7 +792,7 @@ func (t *TableCache) ApplyModifications(tableName string, base model.Model, upda if schema == nil { return fmt.Errorf("no schema for table %s", tableName) } - info, err := mapper.NewInfo(tableName, schema, base) + info, err := t.dbModel.NewModelInfo(base) if err != nil { return err } diff --git a/cache/cache_test.go b/cache/cache_test.go index b2d1db4b..55a342e7 100644 --- a/cache/cache_test.go +++ b/cache/cache_test.go @@ -5,7 +5,6 @@ import ( "reflect" "testing" - "github.com/ovn-org/libovsdb/mapper" "github.com/ovn-org/libovsdb/model" "github.com/ovn-org/libovsdb/ovsdb" "github.com/stretchr/testify/assert" @@ -87,7 +86,7 @@ func TestRowCacheCreate(t *testing.T) { db, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) require.Nil(t, err) err = json.Unmarshal([]byte(` - {"name": "TestDB", + {"name": "Open_vSwitch", "tables": { "Open_vSwitch": { "indexes": [["foo"]], @@ -107,7 +106,8 @@ func TestRowCacheCreate(t *testing.T) { testData := Data{ "Open_vSwitch": map[string]model.Model{"bar": &testModel{Foo: "bar"}}, } - dbModel := model.NewDatabaseModel(&schema, db) + dbModel, errs := model.NewDatabaseModel(&schema, db) + require.Empty(t, errs) tc, err := NewTableCache(dbModel, testData) require.Nil(t, err) @@ -174,7 +174,7 @@ func TestRowCacheCreateMultiIndex(t *testing.T) { db, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) require.Nil(t, err) err = json.Unmarshal([]byte(` - {"name": "TestDB", + {"name": "Open_vSwitch", "tables": { "Open_vSwitch": { "indexes": [["foo", "bar"]], @@ -191,11 +191,11 @@ func TestRowCacheCreateMultiIndex(t *testing.T) { } `), &schema) require.Nil(t, err) - tSchema := schema.Table("Open_vSwitch") testData := Data{ "Open_vSwitch": map[string]model.Model{"bar": &testModel{Foo: "bar", Bar: "bar"}}, } - dbModel := model.NewDatabaseModel(&schema, db) + dbModel, errs := model.NewDatabaseModel(&schema, db) + require.Empty(t, errs) tc, err := NewTableCache(dbModel, testData) require.Nil(t, err) tests := []struct { @@ -253,7 +253,7 @@ func TestRowCacheCreateMultiIndex(t *testing.T) { } } else { assert.Nil(t, err) - mapperInfo, err := mapper.NewInfo("Open_vSwitch", tSchema, tt.model) + mapperInfo, err := dbModel.NewModelInfo(tt.model) require.Nil(t, err) h, err := valueFromIndex(mapperInfo, newIndex("foo", "bar")) require.Nil(t, err) @@ -268,7 +268,7 @@ func TestRowCacheUpdate(t *testing.T) { db, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) require.Nil(t, err) err = json.Unmarshal([]byte(` - {"name": "TestDB", + {"name": "Open_vSwitch", "tables": { "Open_vSwitch": { "indexes": [["foo"]], @@ -291,7 +291,8 @@ func TestRowCacheUpdate(t *testing.T) { "foobar": &testModel{Foo: "foobar"}, }, } - dbModel := model.NewDatabaseModel(&schema, db) + dbModel, errs := model.NewDatabaseModel(&schema, db) + require.Empty(t, errs) tc, err := NewTableCache(dbModel, testData) require.Nil(t, err) @@ -351,7 +352,7 @@ func TestRowCacheUpdateMultiIndex(t *testing.T) { db, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) require.Nil(t, err) err = json.Unmarshal([]byte(` - {"name": "TestDB", + {"name": "Open_vSwitch", "tables": { "Open_vSwitch": { "indexes": [["foo", "bar"]], @@ -367,7 +368,6 @@ func TestRowCacheUpdateMultiIndex(t *testing.T) { } } `), &schema) - tSchema := schema.Table("Open_vSwitch") require.Nil(t, err) testData := Data{ "Open_vSwitch": map[string]model.Model{ @@ -375,7 +375,8 @@ func TestRowCacheUpdateMultiIndex(t *testing.T) { "foobar": &testModel{Foo: "foobar", Bar: "foobar"}, }, } - dbModel := model.NewDatabaseModel(&schema, db) + dbModel, errs := model.NewDatabaseModel(&schema, db) + assert.Empty(t, errs) tc, err := NewTableCache(dbModel, testData) require.Nil(t, err) @@ -419,7 +420,7 @@ func TestRowCacheUpdateMultiIndex(t *testing.T) { assert.Error(t, err) } else { assert.Nil(t, err) - mapperInfo, err := mapper.NewInfo("Open_vSwitch", tSchema, tt.model) + mapperInfo, err := dbModel.NewModelInfo(tt.model) require.Nil(t, err) h, err := valueFromIndex(mapperInfo, newIndex("foo", "bar")) require.Nil(t, err) @@ -434,7 +435,7 @@ func TestRowCacheDelete(t *testing.T) { db, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) require.Nil(t, err) err = json.Unmarshal([]byte(` - {"name": "TestDB", + {"name": "Open_vSwitch", "tables": { "Open_vSwitch": { "indexes": [["foo"]], @@ -456,7 +457,8 @@ func TestRowCacheDelete(t *testing.T) { "bar": &testModel{Foo: "bar"}, }, } - dbModel := model.NewDatabaseModel(&schema, db) + dbModel, errs := model.NewDatabaseModel(&schema, db) + require.Empty(t, errs) tc, err := NewTableCache(dbModel, testData) require.Nil(t, err) @@ -628,6 +630,29 @@ func TestEventHandlerFuncs_OnDelete(t *testing.T) { } func TestTableCacheTable(t *testing.T) { + db, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + assert.Nil(t, err) + var schema ovsdb.DatabaseSchema + err = json.Unmarshal([]byte(` + {"name": "Open_vSwitch", + "tables": { + "Open_vSwitch": { + "indexes": [["foo"]], + "columns": { + "foo": { + "type": "string" + }, + "bar": { + "type": "string" + } + } + } + } + } + `), &schema) + assert.Nil(t, err) + dbModel, errs := model.NewDatabaseModel(&schema, db) + require.Empty(t, errs) tests := []struct { name string cache map[string]*RowCache @@ -636,15 +661,15 @@ func TestTableCacheTable(t *testing.T) { }{ { "returns nil for an empty table", - map[string]*RowCache{"bar": newRowCache("bar", ovsdb.TableSchema{}, nil)}, + map[string]*RowCache{"Open_vSwitch": newRowCache("Open_vSwitch", dbModel, nil)}, "foo", nil, }, { - "returns nil for an empty table", - map[string]*RowCache{"bar": newRowCache("bar", ovsdb.TableSchema{}, nil)}, - "bar", - newRowCache("bar", ovsdb.TableSchema{}, nil), + "returns valid row cache for valid table", + map[string]*RowCache{"Open_vSwitch": newRowCache("Open_vSwitch", dbModel, nil)}, + "Open_vSwitch", + newRowCache("Open_vSwitch", dbModel, nil), }, } for _, tt := range tests { @@ -659,6 +684,52 @@ func TestTableCacheTable(t *testing.T) { } func TestTableCacheTables(t *testing.T) { + db, err := model.NewDatabaseModelRequest("TestDB", + map[string]model.Model{ + "test1": &testModel{}, + "test2": &testModel{}, + "test3": &testModel{}}) + assert.Nil(t, err) + var schema ovsdb.DatabaseSchema + err = json.Unmarshal([]byte(` + {"name": "TestDB", + "tables": { + "test1": { + "columns": { + "foo": { + "type": "string" + }, + "bar": { + "type": "string" + } + } + }, + "test2": { + "columns": { + "foo": { + "type": "string" + }, + "bar": { + "type": "string" + } + } + }, + "test3": { + "columns": { + "foo": { + "type": "string" + }, + "bar": { + "type": "string" + } + } + } + } + } + `), &schema) + assert.Nil(t, err) + dbModel, errs := model.NewDatabaseModel(&schema, db) + require.Empty(t, errs) tests := []struct { name string cache map[string]*RowCache @@ -667,9 +738,9 @@ func TestTableCacheTables(t *testing.T) { { "returns a table that exists", map[string]*RowCache{ - "test1": newRowCache("test1", ovsdb.TableSchema{}, nil), - "test2": newRowCache("test2", ovsdb.TableSchema{}, nil), - "test3": newRowCache("test3", ovsdb.TableSchema{}, nil), + "test1": newRowCache("test1", dbModel, nil), + "test2": newRowCache("test2", dbModel, nil), + "test3": newRowCache("test3", dbModel, nil), }, []string{"test1", "test2", "test3"}, }, @@ -696,7 +767,7 @@ func TestTableCache_populate(t *testing.T) { assert.Nil(t, err) var schema ovsdb.DatabaseSchema err = json.Unmarshal([]byte(` - {"name": "TestDB", + {"name": "Open_vSwitch", "tables": { "Open_vSwitch": { "indexes": [["foo"]], @@ -713,7 +784,8 @@ func TestTableCache_populate(t *testing.T) { } `), &schema) assert.Nil(t, err) - dbModel := model.NewDatabaseModel(&schema, db) + dbModel, errs := model.NewDatabaseModel(&schema, db) + require.Empty(t, errs) tc, err := NewTableCache(dbModel, nil) assert.Nil(t, err) @@ -762,7 +834,7 @@ func TestTableCachePopulate(t *testing.T) { assert.Nil(t, err) var schema ovsdb.DatabaseSchema err = json.Unmarshal([]byte(` - {"name": "TestDB", + {"name": "Open_vSwitch", "tables": { "Open_vSwitch": { "indexes": [["foo"]], @@ -779,7 +851,8 @@ func TestTableCachePopulate(t *testing.T) { } `), &schema) assert.Nil(t, err) - dbModel := model.NewDatabaseModel(&schema, db) + dbModel, errs := model.NewDatabaseModel(&schema, db) + require.Empty(t, errs) tc, err := NewTableCache(dbModel, nil) assert.Nil(t, err) @@ -827,7 +900,7 @@ func TestTableCachePopulate2(t *testing.T) { assert.Nil(t, err) var schema ovsdb.DatabaseSchema err = json.Unmarshal([]byte(` - {"name": "TestDB", + {"name": "Open_vSwitch", "tables": { "Open_vSwitch": { "indexes": [["foo"]], @@ -844,7 +917,8 @@ func TestTableCachePopulate2(t *testing.T) { } `), &schema) assert.Nil(t, err) - dbModel := model.NewDatabaseModel(&schema, db) + dbModel, errs := model.NewDatabaseModel(&schema, db) + require.Empty(t, errs) tc, err := NewTableCache(dbModel, nil) assert.Nil(t, err) @@ -946,7 +1020,7 @@ func TestIndex(t *testing.T) { assert.Nil(t, err) var schema ovsdb.DatabaseSchema err = json.Unmarshal([]byte(` - {"name": "TestDB", + {"name": "Open_vSwitch", "tables": { "Open_vSwitch": { "indexes": [["foo"], ["bar","baz"]], @@ -966,7 +1040,8 @@ func TestIndex(t *testing.T) { } `), &schema) assert.Nil(t, err) - dbModel := model.NewDatabaseModel(&schema, db) + dbModel, errs := model.NewDatabaseModel(&schema, db) + assert.Empty(t, errs) tc, err := NewTableCache(dbModel, nil) assert.Nil(t, err) obj := &indexTestModel{ @@ -982,7 +1057,7 @@ func TestIndex(t *testing.T) { t.Run("Index by single column", func(t *testing.T) { idx, err := table.Index("foo") assert.Nil(t, err) - info, err := mapper.NewInfo("Open_vSwitch", schema.Table("Open_vSwitch"), obj) + info, err := dbModel.NewModelInfo(obj) assert.Nil(t, err) v, err := valueFromIndex(info, newIndex("foo")) assert.Nil(t, err) @@ -994,7 +1069,7 @@ func TestIndex(t *testing.T) { obj2 := obj obj2.Foo = "wrong" assert.Nil(t, err) - info, err := mapper.NewInfo("Open_vSwitch", schema.Table("Open_vSwitch"), obj2) + info, err := dbModel.NewModelInfo(obj2) assert.Nil(t, err) v, err := valueFromIndex(info, newIndex("foo")) assert.Nil(t, err) @@ -1012,7 +1087,7 @@ func TestIndex(t *testing.T) { t.Run("Index by multi-column", func(t *testing.T) { idx, err := table.Index("bar", "baz") assert.Nil(t, err) - info, err := mapper.NewInfo("Open_vSwitch", schema.Table("Open_vSwitch"), obj) + info, err := dbModel.NewModelInfo(obj) assert.Nil(t, err) v, err := valueFromIndex(info, newIndex("bar", "baz")) assert.Nil(t, err) @@ -1023,7 +1098,7 @@ func TestIndex(t *testing.T) { assert.Nil(t, err) obj2 := obj obj2.Baz++ - info, err := mapper.NewInfo("Open_vSwitch", schema.Table("Open_vSwitch"), obj) + info, err := dbModel.NewModelInfo(obj) assert.Nil(t, err) v, err := valueFromIndex(info, newIndex("bar", "baz")) assert.Nil(t, err) @@ -1041,7 +1116,7 @@ func TestTableCacheRowByModelSingleIndex(t *testing.T) { db, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) require.Nil(t, err) err = json.Unmarshal([]byte(` - {"name": "TestDB", + {"name": "Open_vSwitch", "tables": { "Open_vSwitch": { "indexes": [["foo"]], @@ -1065,7 +1140,8 @@ func TestTableCacheRowByModelSingleIndex(t *testing.T) { "bar": &testModel{Foo: "bar", Bar: "bar"}, }, } - dbModel := model.NewDatabaseModel(&schema, db) + dbModel, errs := model.NewDatabaseModel(&schema, db) + require.Empty(t, errs) tc, err := NewTableCache(dbModel, testData) require.NoError(t, err) @@ -1091,7 +1167,7 @@ func TestTableCacheRowByModelTwoIndexes(t *testing.T) { db, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) require.Nil(t, err) err = json.Unmarshal([]byte(` - {"name": "TestDB", + {"name": "Open_vSwitch", "tables": { "Open_vSwitch": { "indexes": [["foo"], ["bar"]], @@ -1115,7 +1191,8 @@ func TestTableCacheRowByModelTwoIndexes(t *testing.T) { "bar": &testModel{Foo: "bar", Bar: "bar"}, }, } - dbModel := model.NewDatabaseModel(&schema, db) + dbModel, errs := model.NewDatabaseModel(&schema, db) + require.Empty(t, errs) tc, err := NewTableCache(dbModel, testData) require.NoError(t, err) @@ -1143,7 +1220,7 @@ func TestTableCacheRowByModelMultiIndex(t *testing.T) { db, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) require.Nil(t, err) err = json.Unmarshal([]byte(` - {"name": "TestDB", + {"name": "Open_vSwitch", "tables": { "Open_vSwitch": { "indexes": [["foo", "bar"]], @@ -1164,7 +1241,8 @@ func TestTableCacheRowByModelMultiIndex(t *testing.T) { testData := Data{ "Open_vSwitch": map[string]model.Model{"foo": myFoo, "bar": &testModel{Foo: "bar", Bar: "bar"}}, } - dbModel := model.NewDatabaseModel(&schema, db) + dbModel, errs := model.NewDatabaseModel(&schema, db) + require.Empty(t, errs) tc, err := NewTableCache(dbModel, testData) require.NoError(t, err) @@ -1187,6 +1265,7 @@ func TestTableCacheRowByModelMultiIndex(t *testing.T) { func TestTableCacheApplyModifications(t *testing.T) { type testDBModel struct { + UUID string `ovsdb:"_uuid"` Value string `ovsdb:"value"` Set []string `ovsdb:"set"` Map map[string]string `ovsdb:"map"` @@ -1290,12 +1369,12 @@ func TestTableCacheApplyModifications(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - db, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db, err := model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testDBModel{}}) assert.Nil(t, err) var schema ovsdb.DatabaseSchema err = json.Unmarshal([]byte(` { - "name": "TestDB", + "name": "Open_vSwitch", "tables": { "Open_vSwitch": { "indexes": [["foo"]], @@ -1311,7 +1390,8 @@ func TestTableCacheApplyModifications(t *testing.T) { } `), &schema) require.NoError(t, err) - dbModel := model.NewDatabaseModel(&schema, db) + dbModel, errs := model.NewDatabaseModel(&schema, db) + require.Empty(t, errs) tc, err := NewTableCache(dbModel, nil) assert.Nil(t, err) original := model.Clone(tt.base).(*testDBModel) diff --git a/client/api.go b/client/api.go index b92933c3..cd5d53eb 100644 --- a/client/api.go +++ b/client/api.go @@ -8,7 +8,6 @@ import ( "reflect" "github.com/ovn-org/libovsdb/cache" - "github.com/ovn-org/libovsdb/mapper" "github.com/ovn-org/libovsdb/model" "github.com/ovn-org/libovsdb/ovsdb" ) @@ -187,13 +186,13 @@ func (a api) conditionFromModel(any bool, model model.Model, cond ...model.Condi } if len(cond) == 0 { - conditional, err = newEqualityConditional(a.cache.Mapper(), tableName, any, model) + conditional, err = newEqualityConditional(a.cache.DatabaseModel(), tableName, any, model) if err != nil { conditional = newErrorConditional(err) } } else { - conditional, err = newExplicitConditional(a.cache.Mapper(), tableName, any, model, cond...) + conditional, err = newExplicitConditional(a.cache.DatabaseModel(), tableName, any, model, cond...) if err != nil { conditional = newErrorConditional(err) } @@ -243,10 +242,8 @@ func (a api) Create(models ...model.Model) ([]ovsdb.Operation, error) { return nil, err } - table := a.cache.Mapper().Schema.Table(tableName) - // Read _uuid field, and use it as named-uuid - info, err := mapper.NewInfo(tableName, table, model) + info, err := a.cache.DatabaseModel().NewModelInfo(model) if err != nil { return nil, err } @@ -294,7 +291,7 @@ func (a api) Mutate(model model.Model, mutationObjs ...model.Mutation) ([]ovsdb. return nil, err } - info, err := mapper.NewInfo(tableName, table, model) + info, err := a.cache.DatabaseModel().NewModelInfo(model) if err != nil { return nil, err } @@ -335,7 +332,7 @@ func (a api) Update(model model.Model, fields ...interface{}) ([]ovsdb.Operation return nil, err } tableSchema := a.cache.Mapper().Schema.Table(table) - info, err := mapper.NewInfo(table, tableSchema, model) + info, err := a.cache.DatabaseModel().NewModelInfo(model) if err != nil { return nil, err } diff --git a/client/api_test_model.go b/client/api_test_model.go index c56cf821..468c2f8e 100644 --- a/client/api_test_model.go +++ b/client/api_test_model.go @@ -157,9 +157,10 @@ func apiTestCache(t *testing.T, data map[string]map[string]model.Model) *cache.T var schema ovsdb.DatabaseSchema err := json.Unmarshal(apiTestSchema, &schema) assert.Nil(t, err) - db, err := model.NewDatabaseModelRequest("OVN_NorthBound", map[string]model.Model{"Logical_Switch": &testLogicalSwitch{}, "Logical_Switch_Port": &testLogicalSwitchPort{}}) + db, err := model.NewDatabaseModelRequest("OVN_Northbound", map[string]model.Model{"Logical_Switch": &testLogicalSwitch{}, "Logical_Switch_Port": &testLogicalSwitchPort{}}) assert.Nil(t, err) - dbModel := model.NewDatabaseModel(&schema, db) + dbModel, errs := model.NewDatabaseModel(&schema, db) + assert.Empty(t, errs) cache, err := cache.NewTableCache(dbModel, data) assert.Nil(t, err) return cache diff --git a/client/client.go b/client/client.go index d8d45e9f..7fd824e2 100644 --- a/client/client.go +++ b/client/client.go @@ -17,7 +17,6 @@ import ( "github.com/cenkalti/rpc2" "github.com/cenkalti/rpc2/jsonrpc" "github.com/ovn-org/libovsdb/cache" - "github.com/ovn-org/libovsdb/mapper" "github.com/ovn-org/libovsdb/model" "github.com/ovn-org/libovsdb/ovsdb" "github.com/ovn-org/libovsdb/ovsdb/serverdb" @@ -703,16 +702,19 @@ func (o *ovsdbClient) monitor(ctx context.Context, cookie MonitorCookie, reconne db := o.databases[dbName] db.schemaMutex.RLock() mmapper := db.model.Mapper() - schema := db.model.Schema() db.schemaMutex.RUnlock() typeMap := o.databases[dbName].model.Types() requests := make(map[string]ovsdb.MonitorRequest) for _, o := range monitor.Tables { - m, ok := typeMap[o.Table] + _, ok := typeMap[o.Table] if !ok { return fmt.Errorf("type for table %s does not exist in model", o.Table) } - info, err := mapper.NewInfo(o.Table, schema.Table(o.Table), m) + model, err := db.model.NewModel(o.Table) + if err != nil { + return err + } + info, err := db.model.NewModelInfo(model) if err != nil { return err } diff --git a/client/client_test.go b/client/client_test.go index 45942358..32157057 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -564,7 +564,8 @@ func BenchmarkUpdate1(b *testing.B) { "Open_vSwitch": &OpenvSwitch{}, }) require.NoError(b, err) - dbModel := model.NewDatabaseModel(&s, dbModelReq) + dbModel, errs := model.NewDatabaseModel(&s, dbModelReq) + require.Empty(b, errs) ovs.primaryDB().cache, err = cache.NewTableCache(dbModel, nil) require.NoError(b, err) update := []byte(`{ @@ -589,7 +590,8 @@ func BenchmarkUpdate2(b *testing.B) { "Open_vSwitch": &OpenvSwitch{}, }) require.NoError(b, err) - dbModel := model.NewDatabaseModel(&s, dbModelReq) + dbModel, errs := model.NewDatabaseModel(&s, dbModelReq) + require.Empty(b, errs) ovs.primaryDB().cache, err = cache.NewTableCache(dbModel, nil) require.NoError(b, err) update := []byte(`{ @@ -615,7 +617,8 @@ func BenchmarkUpdate3(b *testing.B) { "Open_vSwitch": &OpenvSwitch{}, }) require.NoError(b, err) - dbModel := model.NewDatabaseModel(&s, dbModelReq) + dbModel, errs := model.NewDatabaseModel(&s, dbModelReq) + require.Empty(b, errs) ovs.primaryDB().cache, err = cache.NewTableCache(dbModel, nil) require.NoError(b, err) update := []byte(`{ @@ -642,7 +645,8 @@ func BenchmarkUpdate5(b *testing.B) { "Open_vSwitch": &OpenvSwitch{}, }) require.NoError(b, err) - dbModel := model.NewDatabaseModel(&s, dbModelReq) + dbModel, errs := model.NewDatabaseModel(&s, dbModelReq) + require.Empty(b, errs) ovs.primaryDB().cache, err = cache.NewTableCache(dbModel, nil) require.NoError(b, err) update := []byte(`{ @@ -671,7 +675,8 @@ func BenchmarkUpdate8(b *testing.B) { "Open_vSwitch": &OpenvSwitch{}, }) require.NoError(b, err) - dbModel := model.NewDatabaseModel(&s, dbModelReq) + dbModel, errs := model.NewDatabaseModel(&s, dbModelReq) + require.Empty(b, errs) ovs.primaryDB().cache, err = cache.NewTableCache(dbModel, nil) require.NoError(b, err) update := []byte(`{ @@ -717,7 +722,8 @@ func TestUpdate(t *testing.T) { "Open_vSwitch": &OpenvSwitch{}, }) require.NoError(t, err) - dbModel := model.NewDatabaseModel(&s, dbModelReq) + dbModel, errs := model.NewDatabaseModel(&s, dbModelReq) + require.Empty(t, errs) ovs.primaryDB().cache, err = cache.NewTableCache(dbModel, nil) require.NoError(t, err) var reply []interface{} diff --git a/client/condition.go b/client/condition.go index ec396dce..a7a48426 100644 --- a/client/condition.go +++ b/client/condition.go @@ -26,18 +26,18 @@ type Conditional interface { // The conditions are based on the equality of the first available index. // The priority of indexes is: uuid, {schema index} type equalityConditional struct { - mapper *mapper.Mapper + dbModel *model.DatabaseModel tableName string info *mapper.Info singleOp bool } func (c *equalityConditional) Matches(m model.Model) (bool, error) { - info, err := mapper.NewInfo(c.tableName, c.mapper.Schema.Table(c.tableName), m) + info, err := c.dbModel.NewModelInfo(m) if err != nil { return false, err } - return c.mapper.EqualFields(c.info, info) + return c.dbModel.Mapper().EqualFields(c.info, info) } func (c *equalityConditional) Table() string { @@ -48,7 +48,7 @@ func (c *equalityConditional) Table() string { func (c *equalityConditional) Generate() ([][]ovsdb.Condition, error) { var result [][]ovsdb.Condition - conds, err := c.mapper.NewEqualityCondition(c.info) + conds, err := c.dbModel.Mapper().NewEqualityCondition(c.info) if err != nil { return nil, err } @@ -63,13 +63,13 @@ func (c *equalityConditional) Generate() ([][]ovsdb.Condition, error) { } // NewEqualityCondition creates a new equalityConditional -func newEqualityConditional(m *mapper.Mapper, table string, all bool, model model.Model, fields ...interface{}) (Conditional, error) { - info, err := mapper.NewInfo(table, m.Schema.Table(table), model) +func newEqualityConditional(dbModel *model.DatabaseModel, table string, all bool, model model.Model, fields ...interface{}) (Conditional, error) { + info, err := dbModel.NewModelInfo(model) if err != nil { return nil, err } return &equalityConditional{ - mapper: m, + dbModel: dbModel, tableName: table, info: info, singleOp: all, @@ -78,7 +78,7 @@ func newEqualityConditional(m *mapper.Mapper, table string, all bool, model mode // explicitConditional generates conditions based on the provided Condition list type explicitConditional struct { - mapper *mapper.Mapper + dbModel *model.DatabaseModel tableName string info *mapper.Info conditions []model.Condition @@ -99,7 +99,7 @@ func (c *explicitConditional) Generate() ([][]ovsdb.Condition, error) { var conds []ovsdb.Condition for _, cond := range c.conditions { - ovsdbCond, err := c.mapper.NewCondition(c.info, cond.Field, cond.Function, cond.Value) + ovsdbCond, err := c.dbModel.Mapper().NewCondition(c.info, cond.Field, cond.Function, cond.Value) if err != nil { return nil, err } @@ -117,13 +117,13 @@ func (c *explicitConditional) Generate() ([][]ovsdb.Condition, error) { } // newIndexCondition creates a new equalityConditional -func newExplicitConditional(m *mapper.Mapper, table string, all bool, model model.Model, cond ...model.Condition) (Conditional, error) { - info, err := mapper.NewInfo(table, m.Schema.Table(table), model) +func newExplicitConditional(dbModel *model.DatabaseModel, table string, all bool, model model.Model, cond ...model.Condition) (Conditional, error) { + info, err := dbModel.NewModelInfo(model) if err != nil { return nil, err } return &explicitConditional{ - mapper: m, + dbModel: dbModel, tableName: table, info: info, conditions: cond, @@ -165,7 +165,7 @@ func (c *predicateConditional) Generate() ([][]ovsdb.Condition, error) { return nil, err } if match { - info, err := mapper.NewInfo(c.tableName, c.cache.Mapper().Schema.Table(c.tableName), elem) + info, err := c.cache.DatabaseModel().NewModelInfo(elem) if err != nil { return nil, err } diff --git a/client/condition_test.go b/client/condition_test.go index 7ce98730..1c980a98 100644 --- a/client/condition_test.go +++ b/client/condition_test.go @@ -128,7 +128,7 @@ func TestEqualityConditional(t *testing.T) { } for _, tt := range test { t.Run(fmt.Sprintf("Equality Conditional: %s", tt.name), func(t *testing.T) { - cond, err := newEqualityConditional(tcache.Mapper(), "Logical_Switch_Port", tt.all, tt.model) + cond, err := newEqualityConditional(tcache.DatabaseModel(), "Logical_Switch_Port", tt.all, tt.model) assert.Nil(t, err) for model, shouldMatch := range tt.matches { matches, err := cond.Matches(model) @@ -431,7 +431,7 @@ func TestExplicitConditional(t *testing.T) { } for _, tt := range test { t.Run(fmt.Sprintf("Explicit Conditional: %s", tt.name), func(t *testing.T) { - cond, err := newExplicitConditional(tcache.Mapper(), "Logical_Switch_Port", tt.all, testObj, tt.args...) + cond, err := newExplicitConditional(tcache.DatabaseModel(), "Logical_Switch_Port", tt.all, testObj, tt.args...) assert.Nil(t, err) _, err = cond.Matches(testObj) assert.NotNilf(t, err, "Explicit conditions should fail to match on cache") diff --git a/example/ovsdb-server/main.go b/example/ovsdb-server/main.go index 52fe2724..ad728986 100644 --- a/example/ovsdb-server/main.go +++ b/example/ovsdb-server/main.go @@ -61,7 +61,12 @@ func main() { schema.Name: dbModelReq, }) - s, err := server.NewOvsdbServer(ovsDB, *model.NewDatabaseModel(schema, dbModelReq)) + dbModel, errs := model.NewDatabaseModel(schema, dbModelReq) + if len(errs) > 0 { + log.Fatal(errs) + } + + s, err := server.NewOvsdbServer(ovsDB, *dbModel) if err != nil { log.Fatal(err) } diff --git a/model/database.go b/model/database.go index e5f07379..a19e8b41 100644 --- a/model/database.go +++ b/model/database.go @@ -11,20 +11,21 @@ import ( // A DatabaseModel represents libovsdb's metadata about the database. // It's the result of combining the client's DatabaseModelRequest and the server's Schema type DatabaseModel struct { - valid bool - request *DatabaseModelRequest - schema *ovsdb.DatabaseSchema - mapper *mapper.Mapper + valid bool + request *DatabaseModelRequest + schema *ovsdb.DatabaseSchema + mapper *mapper.Mapper + metadata map[reflect.Type]*mapper.Metadata } // NewDatabaseModel returns a new DatabaseModel -func NewDatabaseModel(schema *ovsdb.DatabaseSchema, request *DatabaseModelRequest) *DatabaseModel { - return &DatabaseModel{ - valid: true, - request: request, - schema: schema, - mapper: mapper.NewMapper(schema), +func NewDatabaseModel(schema *ovsdb.DatabaseSchema, request *DatabaseModelRequest) (*DatabaseModel, []error) { + dbModel := NewPartialDatabaseModel(request) + errs := dbModel.SetSchema(schema) + if len(errs) > 0 { + return nil, errs } + return dbModel, nil } // NewPartialDatabaseModel returns a DatabaseModel what does not have a schema yet @@ -48,8 +49,12 @@ func (db *DatabaseModel) SetSchema(schema *ovsdb.DatabaseSchema) []error { } db.schema = schema db.mapper = mapper.NewMapper(schema) + errs := db.generateModelInfo() + if len(errs) > 0 { + return errs + } db.valid = true - return errors + return []error{} } // ClearSchema removes the Schema from the DatabaseModel making it not valid @@ -75,7 +80,7 @@ func (db *DatabaseModel) Mapper() *mapper.Mapper { } // NewModel returns a new instance of a model from a specific string -func (db DatabaseModel) NewModel(table string) (Model, error) { +func (db *DatabaseModel) NewModel(table string) (Model, error) { mtype, ok := db.request.types[table] if !ok { return nil, fmt.Errorf("table %s not found in database model", string(table)) @@ -88,12 +93,12 @@ func (db DatabaseModel) NewModel(table string) (Model, error) { // the DatabaseModel types is a map of reflect.Types indexed by string // The reflect.Type is a pointer to a struct that contains 'ovs' tags // as described above. Such pointer to struct also implements the Model interface -func (db DatabaseModel) Types() map[string]reflect.Type { +func (db *DatabaseModel) Types() map[string]reflect.Type { return db.request.types } // FindTable returns the string associated with a reflect.Type or "" -func (db DatabaseModel) FindTable(mType reflect.Type) string { +func (db *DatabaseModel) FindTable(mType reflect.Type) string { for table, tType := range db.request.types { if tType == mType { return table @@ -101,3 +106,42 @@ func (db DatabaseModel) FindTable(mType reflect.Type) string { } return "" } + +// generateModelMetadata creates metadata objects from all models included in the +// database and caches them for future re-use +func (db *DatabaseModel) generateModelInfo() []error { + errors := []error{} + metadata := make(map[reflect.Type]*mapper.Metadata, len(db.request.types)) + for tableName, tType := range db.request.types { + tableSchema := db.schema.Table(tableName) + if tableSchema == nil { + errors = append(errors, fmt.Errorf("Database Model contains model for table %s which is not present in schema", tableName)) + continue + } + obj, err := db.NewModel(tableName) + if err != nil { + errors = append(errors, err) + continue + } + info, err := mapper.NewInfo(tableName, tableSchema, obj) + if err != nil { + errors = append(errors, err) + continue + } + metadata[tType] = info.Metadata + } + db.metadata = metadata + return errors +} + +// NewModelInfo returns a mapper.Info object based on a provided model +func (db *DatabaseModel) NewModelInfo(obj interface{}) (*mapper.Info, error) { + meta, ok := db.metadata[reflect.TypeOf(obj)] + if !ok { + return nil, ovsdb.NewErrWrongType("NewModelInfo", "type that is part of the DatabaseModel", obj) + } + return &mapper.Info{ + Obj: obj, + Metadata: meta, + }, nil +} diff --git a/server/database.go b/server/database.go index 3a73ea45..46acd7a5 100644 --- a/server/database.go +++ b/server/database.go @@ -42,7 +42,10 @@ func (db *inMemoryDatabase) CreateDatabase(name string, schema *ovsdb.DatabaseSc if mo, ok = db.models[schema.Name]; !ok { return fmt.Errorf("no db model provided for schema with name %s", name) } - dbModel := model.NewDatabaseModel(schema, mo) + dbModel, errs := model.NewDatabaseModel(schema, mo) + if len(errs) > 0 { + return fmt.Errorf("Failed to create DatabaseModel: %#+v", errs) + } database, err := cache.NewTableCache(dbModel, nil) if err != nil { return nil diff --git a/server/server_integration_test.go b/server/server_integration_test.go index 9ac74b71..6da49181 100644 --- a/server/server_integration_test.go +++ b/server/server_integration_test.go @@ -69,7 +69,9 @@ func TestClientServerEcho(t *testing.T) { rand.Seed(time.Now().UnixNano()) tmpfile := fmt.Sprintf("/tmp/ovsdb-%d.sock", rand.Intn(10000)) defer os.Remove(tmpfile) - server, err := NewOvsdbServer(ovsDB, *model.NewDatabaseModel(schema, defDB)) + dbModel, errs := model.NewDatabaseModel(schema, defDB) + require.Empty(t, errs) + server, err := NewOvsdbServer(ovsDB, *dbModel) require.Nil(t, err) go func(t *testing.T, o *OvsdbServer) { @@ -103,7 +105,9 @@ func TestClientServerInsert(t *testing.T) { rand.Seed(time.Now().UnixNano()) tmpfile := fmt.Sprintf("/tmp/ovsdb-%d.sock", rand.Intn(10000)) defer os.Remove(tmpfile) - server, err := NewOvsdbServer(ovsDB, *model.NewDatabaseModel(schema, defDB)) + dbModel, errs := model.NewDatabaseModel(schema, defDB) + require.Empty(t, errs) + server, err := NewOvsdbServer(ovsDB, *dbModel) assert.Nil(t, err) go func(t *testing.T, o *OvsdbServer) { @@ -172,7 +176,9 @@ func TestClientServerMonitor(t *testing.T) { rand.Seed(time.Now().UnixNano()) tmpfile := fmt.Sprintf("/tmp/ovsdb-%d.sock", rand.Intn(10000)) defer os.Remove(tmpfile) - server, err := NewOvsdbServer(ovsDB, *model.NewDatabaseModel(schema, defDB)) + dbModel, errs := model.NewDatabaseModel(schema, defDB) + require.Empty(t, errs) + server, err := NewOvsdbServer(ovsDB, *dbModel) assert.Nil(t, err) go func(t *testing.T, o *OvsdbServer) { @@ -292,7 +298,9 @@ func TestClientServerInsertAndDelete(t *testing.T) { rand.Seed(time.Now().UnixNano()) tmpfile := fmt.Sprintf("/tmp/ovsdb-%d.sock", rand.Intn(10000)) defer os.Remove(tmpfile) - server, err := NewOvsdbServer(ovsDB, *model.NewDatabaseModel(schema, defDB)) + dbModel, errs := model.NewDatabaseModel(schema, defDB) + require.Empty(t, errs) + server, err := NewOvsdbServer(ovsDB, *dbModel) assert.Nil(t, err) go func(t *testing.T, o *OvsdbServer) { @@ -356,7 +364,9 @@ func TestClientServerInsertDuplicate(t *testing.T) { rand.Seed(time.Now().UnixNano()) tmpfile := fmt.Sprintf("/tmp/ovsdb-%d.sock", rand.Intn(10000)) defer os.Remove(tmpfile) - server, err := NewOvsdbServer(ovsDB, *model.NewDatabaseModel(schema, defDB)) + dbModel, errs := model.NewDatabaseModel(schema, defDB) + require.Empty(t, errs) + server, err := NewOvsdbServer(ovsDB, *dbModel) assert.Nil(t, err) go func(t *testing.T, o *OvsdbServer) { @@ -408,7 +418,9 @@ func TestClientServerInsertAndUpdate(t *testing.T) { rand.Seed(time.Now().UnixNano()) tmpfile := fmt.Sprintf("/tmp/ovsdb-%d.sock", rand.Intn(10000)) defer os.Remove(tmpfile) - server, err := NewOvsdbServer(ovsDB, *model.NewDatabaseModel(schema, defDB)) + dbModel, errs := model.NewDatabaseModel(schema, defDB) + require.Empty(t, errs) + server, err := NewOvsdbServer(ovsDB, *dbModel) assert.Nil(t, err) go func(t *testing.T, o *OvsdbServer) { diff --git a/server/server_test.go b/server/server_test.go index 9944a3ff..5e93a2a1 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -77,7 +77,9 @@ func TestOvsdbServerMonitor(t *testing.T) { t.Fatal(err) } ovsDB := NewInMemoryDatabase(map[string]*model.DatabaseModelRequest{"Open_vSwitch": defDB}) - o, err := NewOvsdbServer(ovsDB, *model.NewDatabaseModel(schema, defDB)) + dbModel, errs := model.NewDatabaseModel(schema, defDB) + require.Empty(t, errs) + o, err := NewOvsdbServer(ovsDB, *dbModel) require.Nil(t, err) requests := make(map[string]ovsdb.MonitorRequest) for table, tableSchema := range schema.Tables { diff --git a/server/transact.go b/server/transact.go index b0ba5341..088d8903 100644 --- a/server/transact.go +++ b/server/transact.go @@ -6,7 +6,6 @@ import ( "github.com/google/uuid" "github.com/ovn-org/libovsdb/cache" - "github.com/ovn-org/libovsdb/mapper" "github.com/ovn-org/libovsdb/ovsdb" ) @@ -76,7 +75,6 @@ func (o *OvsdbServer) Insert(database string, table string, rowUUID string, row o.modelsMutex.Unlock() m := dbModel.Mapper() - tSchema := dbModel.Schema().Table(table) if rowUUID == "" { rowUUID = uuid.NewString() @@ -89,7 +87,7 @@ func (o *OvsdbServer) Insert(database string, table string, rowUUID string, row }, nil } - mapperInfo, err := mapper.NewInfo(table, tSchema, model) + mapperInfo, err := dbModel.NewModelInfo(model) if err != nil { return ovsdb.OperationResult{ Error: err.Error(), @@ -163,7 +161,7 @@ func (o *OvsdbServer) Select(database string, table string, where []ovsdb.Condit panic(err) } for _, row := range rows { - info, err := mapper.NewInfo(table, dbModel.Schema().Table(table), row) + info, err := dbModel.NewModelInfo(row) if err != nil { panic(err) } @@ -198,7 +196,7 @@ func (o *OvsdbServer) Update(database, table string, where []ovsdb.Condition, ro }, nil } for _, old := range rows { - oldInfo, _ := mapper.NewInfo(table, schema, old) + oldInfo, _ := dbModel.NewModelInfo(old) uuid, _ := oldInfo.FieldByColumn("_uuid") oldRow, err := m.NewRow(oldInfo) @@ -209,7 +207,7 @@ func (o *OvsdbServer) Update(database, table string, where []ovsdb.Condition, ro if err != nil { panic(err) } - newInfo, err := mapper.NewInfo(table, schema, new) + newInfo, err := dbModel.NewModelInfo(new) if err != nil { panic(err) } @@ -328,7 +326,7 @@ func (o *OvsdbServer) Mutate(database, table string, where []ovsdb.Condition, mu } for _, old := range rows { - oldInfo, err := mapper.NewInfo(table, schema, old) + oldInfo, err := dbModel.NewModelInfo(old) if err != nil { panic(err) } @@ -341,7 +339,7 @@ func (o *OvsdbServer) Mutate(database, table string, where []ovsdb.Condition, mu if err != nil { panic(err) } - newInfo, err := mapper.NewInfo(table, schema, new) + newInfo, err := dbModel.NewModelInfo(new) if err != nil { panic(err) } @@ -457,14 +455,14 @@ func (o *OvsdbServer) Delete(database, table string, where []ovsdb.Condition) (o dbModel := o.models[database] o.modelsMutex.Unlock() m := dbModel.Mapper() - schema := dbModel.Schema().Table(table) + tableUpdate := make(ovsdb.TableUpdate2) rows, err := o.db.List(database, table, where...) if err != nil { panic(err) } for _, row := range rows { - info, _ := mapper.NewInfo(table, schema, row) + info, _ := dbModel.NewModelInfo(row) uuid, _ := info.FieldByColumn("_uuid") oldRow, err := m.NewRow(info) if err != nil { diff --git a/server/transact_test.go b/server/transact_test.go index c2849654..d7c6b870 100644 --- a/server/transact_test.go +++ b/server/transact_test.go @@ -23,7 +23,9 @@ func TestMutateOp(t *testing.T) { t.Fatal(err) } ovsDB := NewInMemoryDatabase(map[string]*model.DatabaseModelRequest{"Open_vSwitch": defDB}) - o, err := NewOvsdbServer(ovsDB, *model.NewDatabaseModel(schema, defDB)) + dbModel, errs := model.NewDatabaseModel(schema, defDB) + require.Empty(t, errs) + o, err := NewOvsdbServer(ovsDB, *dbModel) require.Nil(t, err) ovsUUID := uuid.NewString() @@ -32,7 +34,7 @@ func TestMutateOp(t *testing.T) { m := mapper.NewMapper(schema) ovs := ovsType{} - info, err := mapper.NewInfo("Open_vSwitch", schema.Table("Open_vSwitch"), &ovs) + info, err := dbModel.NewModelInfo(&ovs) require.NoError(t, err) ovsRow, err := m.NewRow(info) require.Nil(t, err) @@ -45,7 +47,7 @@ func TestMutateOp(t *testing.T) { "waldo": "fred", }, } - bridgeInfo, err := mapper.NewInfo("Bridge", schema.Table("Bridge"), &bridge) + bridgeInfo, err := dbModel.NewModelInfo(&bridge) require.NoError(t, err) bridgeRow, err := m.NewRow(bridgeInfo) require.Nil(t, err) @@ -218,7 +220,9 @@ func TestOvsdbServerInsert(t *testing.T) { t.Fatal(err) } ovsDB := NewInMemoryDatabase(map[string]*model.DatabaseModelRequest{"Open_vSwitch": defDB}) - o, err := NewOvsdbServer(ovsDB, *model.NewDatabaseModel(schema, defDB)) + dbModel, errs := model.NewDatabaseModel(schema, defDB) + require.Empty(t, errs) + o, err := NewOvsdbServer(ovsDB, *dbModel) require.Nil(t, err) m := mapper.NewMapper(schema) @@ -234,7 +238,7 @@ func TestOvsdbServerInsert(t *testing.T) { }, } bridgeUUID := uuid.NewString() - bridgeInfo, err := mapper.NewInfo("Bridge", schema.Table("Bridge"), &bridge) + bridgeInfo, err := dbModel.NewModelInfo(&bridge) require.NoError(t, err) bridgeRow, err := m.NewRow(bridgeInfo) require.Nil(t, err) @@ -272,7 +276,9 @@ func TestOvsdbServerUpdate(t *testing.T) { t.Fatal(err) } ovsDB := NewInMemoryDatabase(map[string]*model.DatabaseModelRequest{"Open_vSwitch": defDB}) - o, err := NewOvsdbServer(ovsDB, *model.NewDatabaseModel(schema, defDB)) + dbModel, errs := model.NewDatabaseModel(schema, defDB) + require.Empty(t, errs) + o, err := NewOvsdbServer(ovsDB, *dbModel) require.Nil(t, err) m := mapper.NewMapper(schema) @@ -285,7 +291,7 @@ func TestOvsdbServerUpdate(t *testing.T) { }, } bridgeUUID := uuid.NewString() - bridgeInfo, err := mapper.NewInfo("Bridge", schema.Table("Bridge"), &bridge) + bridgeInfo, err := dbModel.NewModelInfo(&bridge) require.NoError(t, err) bridgeRow, err := m.NewRow(bridgeInfo) require.Nil(t, err) From 94fd06116518fe29cdb65665672f7831761d880f Mon Sep 17 00:00:00 2001 From: Adrian Moreno Date: Mon, 11 Oct 2021 18:24:36 +0200 Subject: [PATCH 10/10] mapper: Add compatility mode Allow the user to set a Compatibility flag on the DatabaseModelRequest. When that flag is set, the verification phase will not fail if a column is missing on the schema or has a different type. Instead it will just skip the column. Same goes for missing tables, they will just be skipped. Signed-off-by: Adrian Moreno --- cache/cache.go | 2 +- cmd/stress/stress.go | 1 + mapper/info.go | 31 +++-- mapper/info_test.go | 80 ++++++++++-- mapper/mapper.go | 6 +- mapper/mapper_test.go | 20 +-- model/database.go | 22 +++- model/database_test.go | 288 +++++++++++++++++++++++++++++++++++++++++ model/request.go | 34 +++-- 9 files changed, 434 insertions(+), 50 deletions(-) create mode 100644 model/database_test.go diff --git a/cache/cache.go b/cache/cache.go index c557cd2b..21b7208e 100644 --- a/cache/cache.go +++ b/cache/cache.go @@ -797,7 +797,7 @@ func (t *TableCache) ApplyModifications(tableName string, base model.Model, upda return err } for k, v := range update { - if k == "_uuid" { + if k == "_uuid" || !t.dbModel.HasColumn(tableName, k) { continue } diff --git a/cmd/stress/stress.go b/cmd/stress/stress.go index cafafcc9..476f570f 100644 --- a/cmd/stress/stress.go +++ b/cmd/stress/stress.go @@ -250,6 +250,7 @@ func main() { var err error dbModelReq, err = model.NewDatabaseModelRequest("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &ovsType{}, "Bridge": &bridgeType{}}) + dbModelReq.SetCompatibility(true) if err != nil { log.Fatal(err) } diff --git a/mapper/info.go b/mapper/info.go index e364ddd2..c7fbd499 100644 --- a/mapper/info.go +++ b/mapper/info.go @@ -2,6 +2,7 @@ package mapper import ( "fmt" + "log" "reflect" "github.com/ovn-org/libovsdb/ovsdb" @@ -25,13 +26,13 @@ type Metadata struct { func (i *Info) FieldByColumn(column string) (interface{}, error) { fieldName, ok := i.Metadata.Fields[column] if !ok { - return nil, fmt.Errorf("FieldByColumn: column %s not found in orm info", column) + return nil, fmt.Errorf("FieldByColumn: column %s not found in mapper info", column) } return reflect.ValueOf(i.Obj).Elem().FieldByName(fieldName).Interface(), nil } -// FieldByColumn returns the field value that corresponds to a column -func (i *Info) hasColumn(column string) bool { +// HasColumn returns whether the column exists in the Metadata fields +func (i *Info) HasColumn(column string) bool { _, ok := i.Metadata.Fields[column] return ok } @@ -40,7 +41,7 @@ func (i *Info) hasColumn(column string) bool { func (i *Info) SetField(column string, value interface{}) error { fieldName, ok := i.Metadata.Fields[column] if !ok { - return fmt.Errorf("SetField: column %s not found in orm info", column) + return fmt.Errorf("SetField: column %s not found in mapper info", column) } fieldValue := reflect.ValueOf(i.Obj).Elem().FieldByName(fieldName) @@ -64,12 +65,12 @@ func (i *Info) ColumnByPtr(fieldPtr interface{}) (string, error) { if objType.Field(j).Offset == offset { column := objType.Field(j).Tag.Get("ovsdb") if _, ok := i.Metadata.Fields[column]; !ok { - return "", fmt.Errorf("field does not have orm column information") + return "", fmt.Errorf("field does not have mapper column information") } return column, nil } } - return "", fmt.Errorf("field pointer does not correspond to orm struct") + return "", fmt.Errorf("field pointer does not correspond to mapper struct") } // getValidIndexes inspects the object and returns the a list of indexes (set of columns) for witch @@ -85,7 +86,7 @@ func (i *Info) getValidIndexes() ([][]string, error) { OUTER: for _, idx := range possibleIndexes { for _, col := range idx { - if !i.hasColumn(col) { + if !i.HasColumn(col) { continue OUTER } columnSchema := i.Metadata.TableSchema.Column(col) @@ -106,7 +107,7 @@ OUTER: } // NewInfo creates a MapperInfo structure around an object based on a given table schema -func NewInfo(tableName string, table *ovsdb.TableSchema, obj interface{}) (*Info, error) { +func NewInfo(tableName string, table *ovsdb.TableSchema, obj interface{}, compat bool) (*Info, error) { objPtrVal := reflect.ValueOf(obj) if objPtrVal.Type().Kind() != reflect.Ptr { return nil, ovsdb.NewErrWrongType("NewMapperInfo", "pointer to a struct", obj) @@ -127,25 +128,35 @@ func NewInfo(tableName string, table *ovsdb.TableSchema, obj interface{}) (*Info } column := table.Column(colName) if column == nil { - return nil, &ErrMapper{ + err := &ErrMapper{ objType: objType.String(), field: field.Name, fieldType: field.Type.String(), fieldTag: colName, reason: "Column does not exist in schema", } + if compat { + log.Printf("Warning: %s. Skipping", err.Error()) + continue + } + return nil, err } // Perform schema-based type checking expType := ovsdb.NativeType(column) if expType != field.Type { - return nil, &ErrMapper{ + err := &ErrMapper{ objType: objType.String(), field: field.Name, fieldType: field.Type.String(), fieldTag: colName, reason: fmt.Sprintf("Wrong type, column expects %s", expType), } + if compat { + log.Printf("Warning: %s. Skipping", err.Error()) + continue + } + return nil, err } fields[colName] = field.Name } diff --git a/mapper/info_test.go b/mapper/info_test.go index cf5ad960..7771c473 100644 --- a/mapper/info_test.go +++ b/mapper/info_test.go @@ -39,17 +39,77 @@ func TestNewMapperInfo(t *testing.T) { table []byte obj interface{} expectedCols []string + compat bool err bool } tests := []test{ { - name: "no_orm", + name: "Info from object without tags should return no columns", table: sampleTable, obj: &struct { foo string bar int }{}, - err: false, + expectedCols: []string{}, + compat: false, + err: false, + }, + { + name: "Valid model should contain columns", + table: sampleTable, + obj: &struct { + Foo string `ovsdb:"aString"` + Bar int `ovsdb:"aInteger"` + }{}, + expectedCols: []string{"aString", "aInteger"}, + compat: true, + err: false, + }, + { + name: "Extra column no compat should error", + table: sampleTable, + obj: &struct { + Foo string `ovsdb:"aString"` + Bar int `ovsdb:"aInteger"` + Baz int `ovsdb:"aNonExistingCol"` + }{}, + expectedCols: []string{"aString", "aInteger"}, + compat: false, + err: true, + }, + { + name: "Extra column compat should not error", + table: sampleTable, + obj: &struct { + Foo string `ovsdb:"aString"` + Bar int `ovsdb:"aInteger"` + Baz int `ovsdb:"aNonExistingCol"` + }{}, + expectedCols: []string{"aString", "aInteger"}, + compat: true, + err: false, + }, + { + name: "Different column typ no compat should error", + table: sampleTable, + obj: &struct { + Foo string `ovsdb:"aString"` + Bar string `ovsdb:"aInt"` + }{}, + expectedCols: []string{"aString", "aInt"}, + compat: false, + err: true, + }, + { + name: "Different column type compat should not error", + table: sampleTable, + obj: &struct { + Foo string `ovsdb:"aString"` + Bar string `ovsdb:"aInt"` + }{}, + expectedCols: []string{"aString"}, + compat: true, + err: false, }, } for _, tt := range tests { @@ -58,16 +118,16 @@ func TestNewMapperInfo(t *testing.T) { err := json.Unmarshal(tt.table, &table) assert.Nil(t, err) - info, err := NewInfo("Test", &table, tt.obj) + info, err := NewInfo("Test", &table, tt.obj, tt.compat) if tt.err { assert.NotNil(t, err) } else { assert.Nil(t, err) + for _, col := range tt.expectedCols { + assert.Truef(t, info.HasColumn(col), "Expected column %s should be present in Mapper Info", col) + } + assert.Equal(t, "Test", info.Metadata.TableName) } - for _, col := range tt.expectedCols { - assert.Truef(t, info.hasColumn(col), "Expected column should be present in Mapper Info") - } - assert.Equal(t, "Test", info.Metadata.TableName) }) } @@ -142,7 +202,7 @@ func TestMapperInfoSet(t *testing.T) { err := json.Unmarshal(tt.table, &table) assert.Nil(t, err) - info, err := NewInfo("Test", &table, tt.obj) + info, err := NewInfo("Test", &table, tt.obj, false) assert.Nil(t, err) err = info.SetField(tt.column, tt.field) @@ -223,7 +283,7 @@ func TestMapperInfoColByPtr(t *testing.T) { err := json.Unmarshal(tt.table, &table) assert.Nil(t, err) - info, err := NewInfo("Test", &table, tt.obj) + info, err := NewInfo("Test", &table, tt.obj, false) assert.Nil(t, err) col, err := info.ColumnByPtr(tt.field) @@ -355,7 +415,7 @@ func TestOrmGetIndex(t *testing.T) { } for _, tt := range tests { t.Run(fmt.Sprintf("GetValidIndexes_%s", tt.name), func(t *testing.T) { - info, err := NewInfo("Test", &table, tt.obj) + info, err := NewInfo("Test", &table, tt.obj, false) assert.Nil(t, err) indexes, err := info.getValidIndexes() diff --git a/mapper/mapper.go b/mapper/mapper.go index ae01e4b2..97fc017f 100644 --- a/mapper/mapper.go +++ b/mapper/mapper.go @@ -72,7 +72,7 @@ func (m Mapper) GetRowData(row *ovsdb.Row, result *Info) error { // The result object must be given as pointer to an object with the right tags func (m Mapper) getData(ovsData ovsdb.Row, result *Info) error { for name, column := range result.Metadata.TableSchema.Columns { - if !result.hasColumn(name) { + if !result.HasColumn(name) { // If provided struct does not have a field to hold this value, skip it continue } @@ -243,7 +243,7 @@ func (m Mapper) NewCondition(data *Info, field interface{}, function ovsdb.Condi // It takes care of field validation against the column type func (m Mapper) NewMutation(data *Info, column string, mutator ovsdb.Mutator, value interface{}) (*ovsdb.Mutation, error) { // Check the column exists in the object - if !data.hasColumn(column) { + if !data.HasColumn(column) { return nil, fmt.Errorf("mutation contains column %s that does not exist in object %v", column, data) } // Check that the mutation is valid @@ -304,7 +304,7 @@ func (m Mapper) equalIndexes(one, other *Info, indexes ...string) (bool, error) if reflect.DeepEqual(ridx, lidx) { // All columns in an index must be simultaneously equal for _, col := range lidx { - if !one.hasColumn(col) || !other.hasColumn(col) { + if !one.HasColumn(col) || !other.HasColumn(col) { break } lfield, err := one.FieldByColumn(col) diff --git a/mapper/mapper_test.go b/mapper/mapper_test.go index 7cb3aa80..9e304060 100644 --- a/mapper/mapper_test.go +++ b/mapper/mapper_test.go @@ -226,7 +226,7 @@ func TestMapperGetData(t *testing.T) { test := ormTestType{ NonTagged: "something", } - testInfo, err := NewInfo("TestTable", schema.Table("TestTable"), &test) + testInfo, err := NewInfo("TestTable", schema.Table("TestTable"), &test, false) assert.NoError(t, err) err = mapper.GetRowData(&ovsRow, testInfo) @@ -345,7 +345,7 @@ func TestMapperNewRow(t *testing.T) { for _, test := range tests { t.Run(fmt.Sprintf("NewRow: %s", test.name), func(t *testing.T) { mapper := NewMapper(&schema) - info, err := NewInfo("TestTable", schema.Table("TestTable"), test.objInput) + info, err := NewInfo("TestTable", schema.Table("TestTable"), test.objInput, false) assert.NoError(t, err) row, err := mapper.NewRow(info) if test.shoulderr { @@ -438,7 +438,7 @@ func TestMapperNewRowFields(t *testing.T) { testObj.MyFloat = 0 test.prepare(&testObj) - info, err := NewInfo("TestTable", schema.Table("TestTable"), &testObj) + info, err := NewInfo("TestTable", schema.Table("TestTable"), &testObj, false) assert.NoError(t, err) row, err := mapper.NewRow(info, test.fields...) if test.err { @@ -592,7 +592,7 @@ func TestMapperCondition(t *testing.T) { for _, tt := range tests { t.Run(fmt.Sprintf("newEqualityCondition_%s", tt.name), func(t *testing.T) { tt.prepare(&testObj) - info, err := NewInfo("TestTable", schema.Table("TestTable"), &testObj) + info, err := NewInfo("TestTable", schema.Table("TestTable"), &testObj, false) assert.NoError(t, err) conds, err := mapper.NewEqualityCondition(info, tt.index...) @@ -846,9 +846,9 @@ func TestMapperEqualIndexes(t *testing.T) { } for _, test := range tests { t.Run(fmt.Sprintf("Equal %s", test.name), func(t *testing.T) { - info1, err := NewInfo("TestTable", schema.Table("TestTable"), &test.obj1) + info1, err := NewInfo("TestTable", schema.Table("TestTable"), &test.obj1, false) assert.NoError(t, err) - info2, err := NewInfo("TestTable", schema.Table("TestTable"), &test.obj2) + info2, err := NewInfo("TestTable", schema.Table("TestTable"), &test.obj2, false) assert.NoError(t, err) eq, err := mapper.equalIndexes(info1, info2, test.indexes...) assert.Nil(t, err) @@ -873,9 +873,9 @@ func TestMapperEqualIndexes(t *testing.T) { Int1: 42, Int2: 25, } - info1, err := NewInfo("TestTable", schema.Table("TestTable"), &obj1) + info1, err := NewInfo("TestTable", schema.Table("TestTable"), &obj1, false) assert.NoError(t, err) - info2, err := NewInfo("TestTable", schema.Table("TestTable"), &obj2) + info2, err := NewInfo("TestTable", schema.Table("TestTable"), &obj2, false) assert.NoError(t, err) eq, err := mapper.EqualFields(info1, info2, &obj1.Int1, &obj1.Int2) assert.Nil(t, err) @@ -1031,7 +1031,7 @@ func TestMapperMutation(t *testing.T) { } for _, test := range tests { t.Run(fmt.Sprintf("newMutation%s", test.name), func(t *testing.T) { - info, err := NewInfo("TestTable", schema.Table("TestTable"), &test.obj) + info, err := NewInfo("TestTable", schema.Table("TestTable"), &test.obj, false) assert.NoError(t, err) mutation, err := mapper.NewMutation(info, test.column, test.mutator, test.value) @@ -1119,7 +1119,7 @@ func TestNewMonitorRequest(t *testing.T) { require.NoError(t, err) mapper := NewMapper(&schema) testTable := &testType{} - info, err := NewInfo("TestTable", schema.Table("TestTable"), testTable) + info, err := NewInfo("TestTable", schema.Table("TestTable"), testTable, false) assert.NoError(t, err) mr, err := mapper.NewMonitorRequest(info, nil) require.NoError(t, err) diff --git a/model/database.go b/model/database.go index a19e8b41..7e15ffd3 100644 --- a/model/database.go +++ b/model/database.go @@ -15,7 +15,7 @@ type DatabaseModel struct { request *DatabaseModelRequest schema *ovsdb.DatabaseSchema mapper *mapper.Mapper - metadata map[reflect.Type]*mapper.Metadata + metadata map[string]*mapper.Metadata } // NewDatabaseModel returns a new DatabaseModel @@ -111,11 +111,10 @@ func (db *DatabaseModel) FindTable(mType reflect.Type) string { // database and caches them for future re-use func (db *DatabaseModel) generateModelInfo() []error { errors := []error{} - metadata := make(map[reflect.Type]*mapper.Metadata, len(db.request.types)) - for tableName, tType := range db.request.types { + metadata := make(map[string]*mapper.Metadata, len(db.request.types)) + for tableName := range db.request.types { tableSchema := db.schema.Table(tableName) if tableSchema == nil { - errors = append(errors, fmt.Errorf("Database Model contains model for table %s which is not present in schema", tableName)) continue } obj, err := db.NewModel(tableName) @@ -123,12 +122,12 @@ func (db *DatabaseModel) generateModelInfo() []error { errors = append(errors, err) continue } - info, err := mapper.NewInfo(tableName, tableSchema, obj) + info, err := mapper.NewInfo(tableName, tableSchema, obj, db.request.compat) if err != nil { errors = append(errors, err) continue } - metadata[tType] = info.Metadata + metadata[tableName] = info.Metadata } db.metadata = metadata return errors @@ -136,7 +135,7 @@ func (db *DatabaseModel) generateModelInfo() []error { // NewModelInfo returns a mapper.Info object based on a provided model func (db *DatabaseModel) NewModelInfo(obj interface{}) (*mapper.Info, error) { - meta, ok := db.metadata[reflect.TypeOf(obj)] + meta, ok := db.metadata[db.FindTable(reflect.TypeOf(obj))] if !ok { return nil, ovsdb.NewErrWrongType("NewModelInfo", "type that is part of the DatabaseModel", obj) } @@ -145,3 +144,12 @@ func (db *DatabaseModel) NewModelInfo(obj interface{}) (*mapper.Info, error) { Metadata: meta, }, nil } + +func (db *DatabaseModel) HasColumn(tableName, column string) bool { + meta, ok := db.metadata[tableName] + if !ok { + return false + } + _, ok = meta.Fields[column] + return ok +} diff --git a/model/database_test.go b/model/database_test.go new file mode 100644 index 00000000..8f377146 --- /dev/null +++ b/model/database_test.go @@ -0,0 +1,288 @@ +package model + +import ( + "encoding/json" + "fmt" + "testing" + + "github.com/ovn-org/libovsdb/ovsdb" + "github.com/stretchr/testify/require" +) + +var tableA = ` + "TableA": { + "columns": { + "aString": { + "type": "string" + }, + "aInteger": { + "type": "integer" + }, + "aSet": { + "type": { + "key": "string", + "max": "unlimited", + "min": 0 + } + } + } +}` +var tableB = ` + "TableB": { + "columns": { + "aString": { + "type": "string" + }, + "aInteger": { + "type": "integer" + }, + "aSet": { + "type": { + "key": "string", + "max": "unlimited", + "min": 0 + } + } + } +}` + +var schema = ` { + "cksum": "223619766 22548", + "name": "TestSchema", + "tables": {` + tableA + "," + tableB + ` + } + } +` + +func TestNewDatabaseModel(t *testing.T) { + + tests := []struct { + name string + schema string + requestTypes map[string]Model + compat bool + expectedCols map[string][]string + expectedNotCols map[string][]string + err bool + }{ + { + name: "Fully matching model should succeed", + schema: schema, + requestTypes: map[string]Model{ + "TableA": &struct { + UUID string `ovsdb:"_uuid"` + Foo string `ovsdb:"aString"` + Bar int `ovsdb:"aInteger"` + Baz []string `ovsdb:"aSet"` + }{}, + "TableB": &struct { + UUID string `ovsdb:"_uuid"` + Foo string `ovsdb:"aString"` + Bar int `ovsdb:"aInteger"` + Baz []string `ovsdb:"aSet"` + }{}, + }, + expectedCols: map[string][]string{ + "TableA": {"aString", "aInteger", "aSet"}, + "TableB": {"aString", "aInteger", "aSet"}, + }, + compat: false, + err: false, + }, + { + name: "Model with less tables should succeed", + schema: schema, + requestTypes: map[string]Model{ + "TableA": &struct { + UUID string `ovsdb:"_uuid"` + Foo string `ovsdb:"aString"` + Bar int `ovsdb:"aInteger"` + Baz []string `ovsdb:"aSet"` + }{}, + }, + expectedCols: map[string][]string{ + "TableA": {"aString", "aInteger", "aSet"}, + }, + compat: false, + err: false, + }, + { + name: "Model with less tables should succeed", + schema: schema, + requestTypes: map[string]Model{ + "TableA": &struct { + UUID string `ovsdb:"_uuid"` + Foo string `ovsdb:"aString"` + Bar int `ovsdb:"aInteger"` + Baz []string `ovsdb:"aSet"` + }{}, + }, + expectedCols: map[string][]string{ + "TableA": {"aString", "aInteger", "aSet"}, + }, + compat: false, + err: false, + }, + { + name: "Model more tables should fail", + schema: schema, + requestTypes: map[string]Model{ + "TableA": &struct { + UUID string `ovsdb:"_uuid"` + Foo string `ovsdb:"aString"` + Bar int `ovsdb:"aInteger"` + Baz []string `ovsdb:"aSet"` + }{}, + "TableB": &struct { + UUID string `ovsdb:"_uuid"` + Foo string `ovsdb:"aString"` + Bar int `ovsdb:"aInteger"` + Baz []string `ovsdb:"aSet"` + }{}, + "TableC": &struct { + UUID string `ovsdb:"_uuid"` + Foo string `ovsdb:"aString"` + Bar int `ovsdb:"aInteger"` + Baz []string `ovsdb:"aSet"` + }{}, + }, + compat: false, + err: true, + }, + { + name: "Model with more tables (compat) should succeed", + schema: schema, + requestTypes: map[string]Model{ + "TableA": &struct { + UUID string `ovsdb:"_uuid"` + Foo string `ovsdb:"aString"` + Bar int `ovsdb:"aInteger"` + Baz []string `ovsdb:"aSet"` + }{}, + "TableB": &struct { + UUID string `ovsdb:"_uuid"` + Foo string `ovsdb:"aString"` + Bar int `ovsdb:"aInteger"` + Baz []string `ovsdb:"aSet"` + }{}, + "TableC": &struct { + UUID string `ovsdb:"_uuid"` + Foo string `ovsdb:"aString"` + Bar int `ovsdb:"aInteger"` + Baz []string `ovsdb:"aSet"` + }{}, + }, + expectedCols: map[string][]string{ + "TableA": {"aString", "aInteger", "aSet"}, + "TableB": {"aString", "aInteger", "aSet"}, + }, + expectedNotCols: map[string][]string{ + "TableC": {"aString", "aInteger", "aSet"}, + }, + compat: true, + err: false, + }, + { + name: "Model with more columns should fail", + schema: schema, + requestTypes: map[string]Model{ + "TableA": &struct { + UUID string `ovsdb:"_uuid"` + Foo string `ovsdb:"aString"` + Bar int `ovsdb:"aInteger"` + Baz []string `ovsdb:"aSet"` + FooBar []string `ovsdb:"aSecondSet"` + }{}, + "TableB": &struct { + UUID string `ovsdb:"_uuid"` + Foo string `ovsdb:"aString"` + Bar int `ovsdb:"aInteger"` + Baz []string `ovsdb:"aSet"` + }{}, + }, + compat: false, + err: true, + }, + { + name: "Model with more columns (compat) should succeed", + schema: schema, + requestTypes: map[string]Model{ + "TableA": &struct { + UUID string `ovsdb:"_uuid"` + Foo string `ovsdb:"aString"` + Bar int `ovsdb:"aInteger"` + Baz []string `ovsdb:"aSet"` + FooBar []string `ovsdb:"aSecondSet"` + }{}, + "TableB": &struct { + UUID string `ovsdb:"_uuid"` + Foo string `ovsdb:"aString"` + Bar int `ovsdb:"aInteger"` + Baz []string `ovsdb:"aSet"` + }{}, + }, + expectedCols: map[string][]string{ + "TableA": {"aString", "aInteger", "aSet"}, + "TableB": {"aString", "aInteger", "aSet"}, + }, + expectedNotCols: map[string][]string{ + "TableA": {"aSecondSet"}, + }, + compat: false, + err: true, + }, + } + for _, tt := range tests { + t.Run(fmt.Sprintf("NewDatabaseModel%s", tt.name), func(t *testing.T) { + var schema ovsdb.DatabaseSchema + err := json.Unmarshal([]byte(tt.schema), &schema) + require.NoError(t, err) + req, err := NewDatabaseModelRequest("TestSchema", tt.requestTypes) + if tt.compat { + req.SetCompatibility(true) + } + require.NoError(t, err) + + // Test single step creation + db, errs := NewDatabaseModel(&schema, req) + + if tt.err { + require.NotEmpty(t, errs) + } else { + require.Empty(t, errs) + for table, cols := range tt.expectedCols { + for _, col := range cols { + require.Truef(t, db.HasColumn(table, col), "table %s column %s should be present in model", table, col) + } + } + for table, cols := range tt.expectedNotCols { + for _, col := range cols { + require.Falsef(t, db.HasColumn(table, col), "table %s column %s should not be present in model", table, col) + } + } + } + + // Test 2-step step creation + db = NewPartialDatabaseModel(req) + errs = db.SetSchema(&schema) + + if tt.err { + require.NotEmpty(t, errs) + } else { + require.Empty(t, errs) + for table, cols := range tt.expectedCols { + for _, col := range cols { + require.Truef(t, db.HasColumn(table, col), "table %s column %s should be present in model", table, col) + } + } + for table, cols := range tt.expectedNotCols { + for _, col := range cols { + require.Falsef(t, db.HasColumn(table, col), "table %s column %s should not be present in model", table, col) + } + } + } + + }) + } + +} diff --git a/model/request.go b/model/request.go index e8b82501..c62fb74d 100644 --- a/model/request.go +++ b/model/request.go @@ -2,6 +2,7 @@ package model import ( "fmt" + "log" "reflect" "github.com/ovn-org/libovsdb/mapper" @@ -10,12 +11,13 @@ import ( // DatabaseModelRequest contains the information needed to build a DatabaseModel type DatabaseModelRequest struct { - name string - types map[string]reflect.Type + name string + types map[string]reflect.Type + compat bool } // NewModel returns a new instance of a model from a specific string -func (db DatabaseModelRequest) newModel(table string) (Model, error) { +func (db *DatabaseModelRequest) newModel(table string) (Model, error) { mtype, ok := db.types[table] if !ok { return nil, fmt.Errorf("table %s not found in database model", string(table)) @@ -24,14 +26,21 @@ func (db DatabaseModelRequest) newModel(table string) (Model, error) { return model.Interface().(Model), nil } +// SetCompatible requests the DatabaseModel to try to be compatible with the server schema. +// On this mode, the model will try to fill in fields if they exist on the database but will not fail +// if additional columns or tables are declared in the model. +func (db *DatabaseModelRequest) SetCompatibility(compat bool) { + db.compat = compat +} + // Name returns the database name -func (db DatabaseModelRequest) Name() string { +func (db *DatabaseModelRequest) Name() string { return db.name } // Validate validates the DatabaseModel against the input schema // Returns all the errors detected -func (db DatabaseModelRequest) validate(schema *ovsdb.DatabaseSchema) []error { +func (db *DatabaseModelRequest) validate(schema *ovsdb.DatabaseSchema) []error { var errors []error if db.name != schema.Name { errors = append(errors, fmt.Errorf("database model name (%s) does not match schema (%s)", @@ -41,7 +50,12 @@ func (db DatabaseModelRequest) validate(schema *ovsdb.DatabaseSchema) []error { for tableName := range db.types { tableSchema := schema.Table(tableName) if tableSchema == nil { - errors = append(errors, fmt.Errorf("database model contains a model for table %s that does not exist in schema", tableName)) + err := fmt.Errorf("database model contains a model for table %s that does not exist in schema", tableName) + if db.compat { + log.Printf("Warning: %s. Skipping", err.Error()) + } else { + errors = append(errors, err) + } continue } model, err := db.newModel(tableName) @@ -49,7 +63,8 @@ func (db DatabaseModelRequest) validate(schema *ovsdb.DatabaseSchema) []error { errors = append(errors, err) continue } - if _, err := mapper.NewInfo(tableName, tableSchema, model); err != nil { + _, err = mapper.NewInfo(tableName, tableSchema, model, db.compat) + if err != nil { errors = append(errors, err) } } @@ -78,7 +93,8 @@ func NewDatabaseModelRequest(name string, models map[string]Model) (*DatabaseMod types[table] = reflect.TypeOf(model) } return &DatabaseModelRequest{ - types: types, - name: name, + types: types, + name: name, + compat: false, }, nil }