From 1d5c015ea7493673c5da56880cbe4004592dc649 Mon Sep 17 00:00:00 2001 From: Dave Tucker Date: Wed, 3 Nov 2021 18:06:11 +0000 Subject: [PATCH] Make Columns and Tables Optional This allows for Columns to be declared optional via the ClientDatabaseModel. Fields may be marked as optional within the struct tag. This allows a connection when a mapped table (and columns that reference it) aren't available at runtime to support upgrades. To allow a user to easily see which Tables and Columns are supported two new APIs were added. IsTableSupported and IsColumnSupported Signed-off-by: Dave Tucker --- cache/cache.go | 5 ++ cache/cache_test.go | 30 ++++---- client/api_test_model.go | 2 +- client/client.go | 33 +++++++++ client/client_test.go | 13 ++-- cmd/stress/stress.go | 2 +- example/play_with_ovs/main.go | 2 +- mapper/info.go | 66 ++++++++++++++--- mapper/info_test.go | 119 +++++++++++++++++++++++++++++- mapper/mapper.go | 7 +- model/client.go | 24 ++++-- model/model_test.go | 109 ++++++++++++++++++++++++++- modelgen/dbmodel.go | 2 +- modelgen/dbmodel_test.go | 2 +- ovsdb/serverdb/model.go | 2 +- server/server_integration_test.go | 12 +-- server/server_test.go | 2 +- server/transact_test.go | 8 +- test/ovs/ovs_integration_test.go | 2 +- 19 files changed, 378 insertions(+), 64 deletions(-) diff --git a/cache/cache.go b/cache/cache.go index e78652d2..4aa528b8 100644 --- a/cache/cache.go +++ b/cache/cache.go @@ -5,6 +5,7 @@ import ( "crypto/sha256" "encoding/gob" "encoding/hex" + "errors" "fmt" "log" "os" @@ -877,6 +878,10 @@ func (t *TableCache) ApplyModifications(tableName string, base model.Model, upda current, err := info.FieldByColumn(k) if err != nil { + if errors.Is(err, mapper.ErrOmitted) { + // skip fields that aren't supported by the runtime schema + continue + } return err } diff --git a/cache/cache_test.go b/cache/cache_test.go index 48be903e..dc82ecd2 100644 --- a/cache/cache_test.go +++ b/cache/cache_test.go @@ -81,7 +81,7 @@ func TestRowCache_Rows(t *testing.T) { func TestRowCacheCreate(t *testing.T) { var schema ovsdb.DatabaseSchema - db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}, nil) require.Nil(t, err) err = json.Unmarshal([]byte(` {"name": "Open_vSwitch", @@ -170,7 +170,7 @@ func TestRowCacheCreate(t *testing.T) { func TestRowCacheCreateMultiIndex(t *testing.T) { var schema ovsdb.DatabaseSchema - db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}, nil) require.Nil(t, err) err = json.Unmarshal([]byte(` {"name": "Open_vSwitch", @@ -264,7 +264,7 @@ func TestRowCacheCreateMultiIndex(t *testing.T) { func TestRowCacheUpdate(t *testing.T) { var schema ovsdb.DatabaseSchema - db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}, nil) require.Nil(t, err) err = json.Unmarshal([]byte(` {"name": "Open_vSwitch", @@ -348,7 +348,7 @@ func TestRowCacheUpdate(t *testing.T) { func TestRowCacheUpdateMultiIndex(t *testing.T) { var schema ovsdb.DatabaseSchema - db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}, nil) require.Nil(t, err) err = json.Unmarshal([]byte(` {"name": "Open_vSwitch", @@ -431,7 +431,7 @@ func TestRowCacheUpdateMultiIndex(t *testing.T) { func TestRowCacheDelete(t *testing.T) { var schema ovsdb.DatabaseSchema - db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}, nil) require.Nil(t, err) err = json.Unmarshal([]byte(` {"name": "Open_vSwitch", @@ -629,7 +629,7 @@ func TestEventHandlerFuncs_OnDelete(t *testing.T) { } func TestTableCacheTable(t *testing.T) { - db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}, nil) assert.Nil(t, err) var schema ovsdb.DatabaseSchema err = json.Unmarshal([]byte(` @@ -687,7 +687,7 @@ func TestTableCacheTables(t *testing.T) { map[string]model.Model{ "test1": &testModel{}, "test2": &testModel{}, - "test3": &testModel{}}) + "test3": &testModel{}}, nil) assert.Nil(t, err) var schema ovsdb.DatabaseSchema err = json.Unmarshal([]byte(` @@ -762,7 +762,7 @@ func TestTableCacheTables(t *testing.T) { func TestTableCache_populate(t *testing.T) { t.Log("Create") - db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}, nil) assert.Nil(t, err) var schema ovsdb.DatabaseSchema err = json.Unmarshal([]byte(` @@ -832,7 +832,7 @@ func TestTableCache_populate(t *testing.T) { func TestTableCachePopulate(t *testing.T) { t.Log("Create") - db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}, nil) assert.Nil(t, err) var schema ovsdb.DatabaseSchema err = json.Unmarshal([]byte(` @@ -901,7 +901,7 @@ func TestTableCachePopulate(t *testing.T) { } func TestTableCachePopulate2(t *testing.T) { - db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}, nil) assert.Nil(t, err) var schema ovsdb.DatabaseSchema err = json.Unmarshal([]byte(` @@ -1026,7 +1026,7 @@ func TestIndex(t *testing.T) { Bar string `ovsdb:"bar"` Baz int `ovsdb:"baz"` } - db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &indexTestModel{}}) + db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &indexTestModel{}}, nil) assert.Nil(t, err) var schema ovsdb.DatabaseSchema err = json.Unmarshal([]byte(` @@ -1123,7 +1123,7 @@ func TestIndex(t *testing.T) { func TestTableCacheRowByModelSingleIndex(t *testing.T) { var schema ovsdb.DatabaseSchema - db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}, nil) require.Nil(t, err) err = json.Unmarshal([]byte(` {"name": "Open_vSwitch", @@ -1174,7 +1174,7 @@ func TestTableCacheRowByModelSingleIndex(t *testing.T) { func TestTableCacheRowByModelTwoIndexes(t *testing.T) { var schema ovsdb.DatabaseSchema - db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}, nil) require.Nil(t, err) err = json.Unmarshal([]byte(` {"name": "Open_vSwitch", @@ -1227,7 +1227,7 @@ func TestTableCacheRowByModelTwoIndexes(t *testing.T) { func TestTableCacheRowByModelMultiIndex(t *testing.T) { var schema ovsdb.DatabaseSchema - db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}) + db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testModel{}}, nil) require.Nil(t, err) err = json.Unmarshal([]byte(` {"name": "Open_vSwitch", @@ -1379,7 +1379,7 @@ func TestTableCacheApplyModifications(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testDBModel{}}) + db, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &testDBModel{}}, nil) assert.Nil(t, err) var schema ovsdb.DatabaseSchema err = json.Unmarshal([]byte(` diff --git a/client/api_test_model.go b/client/api_test_model.go index 3cd443ab..6436833f 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.NewClientDBModel("OVN_Northbound", map[string]model.Model{"Logical_Switch": &testLogicalSwitch{}, "Logical_Switch_Port": &testLogicalSwitchPort{}}) + db, err := model.NewClientDBModel("OVN_Northbound", map[string]model.Model{"Logical_Switch": &testLogicalSwitch{}, "Logical_Switch_Port": &testLogicalSwitchPort{}}, nil) assert.Nil(t, err) dbModel, errs := model.NewDatabaseModel(schema, db) assert.Empty(t, errs) diff --git a/client/client.go b/client/client.go index 3639f3d4..a6ca61f6 100644 --- a/client/client.go +++ b/client/client.go @@ -20,6 +20,7 @@ import ( "github.com/go-logr/logr" "github.com/go-logr/stdr" "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" @@ -1121,3 +1122,35 @@ func (o *ovsdbClient) WhereAll(m model.Model, conditions ...model.Condition) Con func (o *ovsdbClient) WhereCache(predicate interface{}) ConditionalAPI { return o.primaryDB().api.WhereCache(predicate) } + +// IsTableSupported checks whether a provided Model is supported by the +// runtime schema +func (o *ovsdbClient) IsTableSupported(m model.Model) bool { + o.primaryDB().modelMutex.RLock() + defer o.primaryDB().modelMutex.RUnlock() + tableName := o.primaryDB().model.FindTable(reflect.TypeOf(m)) + if _, ok := o.primaryDB().model.Schema.Tables[tableName]; ok { + return true + } + return false +} + +// IsColumnSupported checks whether a provided column (derived via field Pointer in a Model) +// is supported by the runtime schema +func (o *ovsdbClient) IsColumnSupported(m model.Model, fieldPtr interface{}) bool { + o.primaryDB().modelMutex.RLock() + defer o.primaryDB().modelMutex.RUnlock() + tableName := o.primaryDB().model.FindTable(reflect.TypeOf(m)) + tSchema, ok := o.primaryDB().model.Schema.Tables[tableName] + if !ok { + return false + } + info, err := mapper.NewInfo(tableName, &tSchema, m) + if err != nil { + return false + } + if _, err := info.ColumnByPtr(fieldPtr); err != nil { + return false + } + return true +} diff --git a/client/client_test.go b/client/client_test.go index fe6ca06c..43d5f616 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -92,6 +92,7 @@ var defDB, _ = model.NewClientDBModel("Open_vSwitch", "Open_vSwitch": &OpenvSwitch{}, "Bridge": &Bridge{}, }, + nil, ) var schema = `{ @@ -562,7 +563,7 @@ func BenchmarkUpdate1(b *testing.B) { clientDBModel, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{ "Bridge": &Bridge{}, "Open_vSwitch": &OpenvSwitch{}, - }) + }, nil) require.NoError(b, err) dbModel, errs := model.NewDatabaseModel(s, clientDBModel) require.Empty(b, errs) @@ -588,7 +589,7 @@ func BenchmarkUpdate2(b *testing.B) { clientDBModel, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{ "Bridge": &Bridge{}, "Open_vSwitch": &OpenvSwitch{}, - }) + }, nil) require.NoError(b, err) dbModel, errs := model.NewDatabaseModel(s, clientDBModel) require.Empty(b, errs) @@ -615,7 +616,7 @@ func BenchmarkUpdate3(b *testing.B) { clientDBModel, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{ "Bridge": &Bridge{}, "Open_vSwitch": &OpenvSwitch{}, - }) + }, nil) require.NoError(b, err) dbModel, errs := model.NewDatabaseModel(s, clientDBModel) require.Empty(b, errs) @@ -643,7 +644,7 @@ func BenchmarkUpdate5(b *testing.B) { clientDBModel, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{ "Bridge": &Bridge{}, "Open_vSwitch": &OpenvSwitch{}, - }) + }, nil) require.NoError(b, err) dbModel, errs := model.NewDatabaseModel(s, clientDBModel) require.Empty(b, errs) @@ -673,7 +674,7 @@ func BenchmarkUpdate8(b *testing.B) { clientDBModel, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{ "Bridge": &Bridge{}, "Open_vSwitch": &OpenvSwitch{}, - }) + }, nil) require.NoError(b, err) dbModel, errs := model.NewDatabaseModel(s, clientDBModel) require.Empty(b, errs) @@ -720,7 +721,7 @@ func TestUpdate(t *testing.T) { clientDBModel, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{ "Bridge": &Bridge{}, "Open_vSwitch": &OpenvSwitch{}, - }) + }, nil) require.NoError(t, err) dbModel, errs := model.NewDatabaseModel(s, clientDBModel) require.Empty(t, errs) diff --git a/cmd/stress/stress.go b/cmd/stress/stress.go index e0443861..5ff22b94 100644 --- a/cmd/stress/stress.go +++ b/cmd/stress/stress.go @@ -249,7 +249,7 @@ func main() { } var err error - clientDBModel, err = model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &ovsType{}, "Bridge": &bridgeType{}}) + clientDBModel, err = model.NewClientDBModel("Open_vSwitch", map[string]model.Model{"Open_vSwitch": &ovsType{}, "Bridge": &bridgeType{}}, nil) if err != nil { log.Fatal(err) } diff --git a/example/play_with_ovs/main.go b/example/play_with_ovs/main.go index 93a82534..e745f6ff 100644 --- a/example/play_with_ovs/main.go +++ b/example/play_with_ovs/main.go @@ -98,7 +98,7 @@ func main() { update = make(chan model.Model) clientDBModel, err := model.NewClientDBModel("Open_vSwitch", - map[string]model.Model{bridgeTable: &vswitchd.Bridge{}, ovsTable: &vswitchd.OpenvSwitch{}}) + map[string]model.Model{bridgeTable: &vswitchd.Bridge{}, ovsTable: &vswitchd.OpenvSwitch{}}, nil) if err != nil { log.Fatal("Unable to create DB model ", err) } diff --git a/mapper/info.go b/mapper/info.go index d1fcb382..a8cee6b9 100644 --- a/mapper/info.go +++ b/mapper/info.go @@ -1,12 +1,23 @@ package mapper import ( + "errors" "fmt" "reflect" + "strings" "github.com/ovn-org/libovsdb/ovsdb" ) +const ( + ovsdbStructTag = "ovsdb" + omitUnsupported = "omitunsupported" +) + +// ErrOmitted is returned when an operation has been performed on a +// column that has been omitted due to not being available in the runtime schema +var ErrOmitted = errors.New("column is not available in runtime schema") + // Info is a struct that wraps an object with its metadata type Info struct { // FieldName indexed by column @@ -16,21 +27,25 @@ type Info struct { // Metadata represents the information needed to know how to map OVSDB columns into an objetss fields type Metadata struct { - Fields map[string]string // Map of ColumnName -> FieldName - TableSchema *ovsdb.TableSchema // TableSchema associated - TableName string // Table name + Fields map[string]string // Map of ColumnName -> FieldName + OmittedFields map[string]string // Map of ColumnName -> Empty Struct + 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) { + if _, ok := i.Metadata.OmittedFields[column]; ok { + return nil, ErrOmitted + } 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 +// hasColumn returns whether a column is present func (i *Info) hasColumn(column string) bool { _, ok := i.Metadata.Fields[column] return ok @@ -38,6 +53,9 @@ func (i *Info) hasColumn(column string) bool { // SetField sets the field in the column to the specified value func (i *Info) SetField(column string, value interface{}) error { + if _, ok := i.Metadata.OmittedFields[column]; ok { + return ErrOmitted + } fieldName, ok := i.Metadata.Fields[column] if !ok { return fmt.Errorf("SetField: column %s not found in orm info", column) @@ -62,7 +80,11 @@ func (i *Info) ColumnByPtr(fieldPtr interface{}) (string, error) { 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") + field := objType.Field(j) + column, omit := parseStructTag(field) + if omit { + return "", ErrOmitted + } if _, ok := i.Metadata.Fields[column]; !ok { return "", fmt.Errorf("field does not have orm column information") } @@ -118,15 +140,21 @@ func NewInfo(tableName string, table *ovsdb.TableSchema, obj interface{}) (*Info objType := objVal.Type() fields := make(map[string]string, objType.NumField()) + omittedFields := make(map[string]string) for i := 0; i < objType.NumField(); i++ { field := objType.Field(i) - colName := field.Tag.Get("ovsdb") + colName, omit := parseStructTag(field) if colName == "" { // Untagged fields are ignored continue } column := table.Column(colName) if column == nil { + // fields that are marked optional in struct tags are safe to skip + if omit { + omittedFields[colName] = field.Name + continue + } return nil, &ErrMapper{ objType: objType.String(), field: field.Name, @@ -153,9 +181,27 @@ func NewInfo(tableName string, table *ovsdb.TableSchema, obj interface{}) (*Info return &Info{ Obj: obj, Metadata: Metadata{ - Fields: fields, - TableSchema: table, - TableName: tableName, + Fields: fields, + OmittedFields: omittedFields, + TableSchema: table, + TableName: tableName, }, }, nil } + +// parseStructTag parses the ovsdb struct tag +// it returns the column name and whether it should be omitted if +// unsupported by the runtime schema +func parseStructTag(field reflect.StructField) (string, bool) { + tagData := field.Tag.Get(ovsdbStructTag) + parts := strings.Split(tagData, ",") + if len(parts) == 0 { + return "", false + } + omit := false + colName := parts[0] + if len(parts) == 2 && parts[1] == omitUnsupported { + omit = true + } + return colName, omit +} diff --git a/mapper/info_test.go b/mapper/info_test.go index cf5ad960..fa25cb56 100644 --- a/mapper/info_test.go +++ b/mapper/info_test.go @@ -3,10 +3,12 @@ package mapper import ( "encoding/json" "fmt" + "reflect" "testing" "github.com/ovn-org/libovsdb/ovsdb" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) var sampleTable = []byte(`{ @@ -33,6 +35,33 @@ var sampleTable = []byte(`{ } }`) +var sampleTableWithOptionalField = []byte(`{ + "columns": { + "aString": { + "type": "string" + }, + "anotherString": { + "type": "string" + }, + "aInteger": { + "type": "integer" + }, + "aSet": { + "type": { + "key": "string", + "max": "unlimited", + "min": 0 + } + }, + "aMap": { + "type": { + "key": "string", + "value": "string" + } + } + } +}`) + func TestNewMapperInfo(t *testing.T) { type test struct { name string @@ -75,10 +104,11 @@ func TestNewMapperInfo(t *testing.T) { func TestMapperInfoSet(t *testing.T) { type obj struct { - Ostring string `ovsdb:"aString"` - Oint int `ovsdb:"aInteger"` - Oset []string `ovsdb:"aSet"` - Omap map[string]string `ovsdb:"aMap"` + Ostring string `ovsdb:"aString"` + OanotherString string `ovsdb:"anotherString,omitunsupported"` + Oint int `ovsdb:"aInteger"` + Oset []string `ovsdb:"aSet"` + Omap map[string]string `ovsdb:"aMap"` } type test struct { @@ -135,6 +165,14 @@ func TestMapperInfoSet(t *testing.T) { column: "aMap", err: true, }, + { + name: "optional - supported by schema", + table: sampleTableWithOptionalField, + obj: &obj{}, + field: "foo", + column: "anotherString", + err: false, + }, } for _, tt := range tests { t.Run(fmt.Sprintf("SetField_%s", tt.name), func(t *testing.T) { @@ -159,6 +197,30 @@ func TestMapperInfoSet(t *testing.T) { } } +func TestMapperInfoSetOmitted(t *testing.T) { + type obj struct { + Ostring string `ovsdb:"aString"` + OanotherString string `ovsdb:"anotherString,omitunsupported"` + Oint int `ovsdb:"aInteger"` + Oset []string `ovsdb:"aSet"` + Omap map[string]string `ovsdb:"aMap"` + } + var table ovsdb.TableSchema + err := json.Unmarshal(sampleTable, &table) + assert.Nil(t, err) + + info, err := NewInfo("Test", &table, &obj{}) + assert.Nil(t, err) + + // SetField returns an error if not supported by the schema + err = info.SetField("anotherString", "foo") + assert.ErrorIs(t, err, ErrOmitted) + + // FieldByColumn will return an error also + _, err = info.FieldByColumn("anotherString") + assert.ErrorIs(t, err, ErrOmitted) +} + func TestMapperInfoColByPtr(t *testing.T) { type obj struct { ostring string `ovsdb:"aString"` @@ -369,3 +431,52 @@ func TestOrmGetIndex(t *testing.T) { }) } } + +func TestParseStructTag(t *testing.T) { + type obj struct { + Valid string `ovsdb:"valid"` + NotMapped string + Optional string `ovsdb:"optional,omitunsupported"` + } + tests := []struct { + name string + fieldName string + want string + want1 bool + }{ + { + "parse valid tag", + "Valid", + "valid", + false, + }, + { + "parse empty tag", + "NotMapped", + "", + false, + }, + { + "parse optional tag", + "Optional", + "optional", + true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + v := reflect.TypeOf(obj{}) + field, ok := v.FieldByName(tt.fieldName) + require.True(t, ok) + got, got1 := parseStructTag(field) + assert.Equal(t, tt.want, got) + assert.Equal(t, tt.want1, got1) + if got != tt.want { + t.Errorf("parseStructTag() got = %v, want %v", got, tt.want) + } + if got1 != tt.want1 { + t.Errorf("parseStructTag() got1 = %v, want %v", got1, tt.want1) + } + }) + } +} diff --git a/mapper/mapper.go b/mapper/mapper.go index 21cecc13..9d1dc88d 100644 --- a/mapper/mapper.go +++ b/mapper/mapper.go @@ -1,6 +1,7 @@ package mapper import ( + "errors" "fmt" "reflect" @@ -74,7 +75,7 @@ func (m Mapper) getData(ovsData ovsdb.Row, result *Info) error { result.Metadata.TableName, name, err.Error()) } - if err := result.SetField(name, nativeElem); err != nil { + if err := result.SetField(name, nativeElem); err != nil && !errors.Is(err, ErrOmitted) { return err } } @@ -94,7 +95,9 @@ func (m Mapper) NewRow(data *Info, fields ...interface{}) (ovsdb.Row, error) { for name, column := range columns { nativeElem, err := data.FieldByColumn(name) if err != nil { - // If provided struct does not have a field to hold this value, skip it + // If provided struct does not have a field to hold this value + // Or it does have a field but it's not supported by the runtime schema, + // skip it continue } diff --git a/model/client.go b/model/client.go index a276736d..ef6743d5 100644 --- a/model/client.go +++ b/model/client.go @@ -10,8 +10,9 @@ import ( // ClientDBModel contains the client information needed to build a DatabaseModel type ClientDBModel struct { - name string - types map[string]reflect.Type + name string + types map[string]reflect.Type + optional map[string]bool } // NewModel returns a new instance of a model from a specific string @@ -41,7 +42,9 @@ func (db ClientDBModel) 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)) + if !db.optional[tableName] { + 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) @@ -57,7 +60,12 @@ func (db ClientDBModel) validate(schema ovsdb.DatabaseSchema) []error { } // NewClientDBModel constructs a ClientDBModel based on a database name and dictionary of models indexed by table name -func NewClientDBModel(name string, models map[string]Model) (ClientDBModel, error) { +// You can also provide a map of optional tables. The absence of these tables will not cause schema validation to fail, +// the contract with the caller is as follows: +// 1. You must ensure you check that a table exists before trying to use it +// 2. Optional columns will be ignored when processing transactions or cache updates if the runtime schema does not support them. +// Therefore, you may also wish to check for support before writing/reading from your Model +func NewClientDBModel(name string, models map[string]Model, optional map[string]bool) (ClientDBModel, error) { types := make(map[string]reflect.Type, len(models)) for table, model := range models { modelType := reflect.TypeOf(model) @@ -77,8 +85,12 @@ func NewClientDBModel(name string, models map[string]Model) (ClientDBModel, erro types[table] = reflect.TypeOf(model) } + if optional == nil { + optional = make(map[string]bool) + } return ClientDBModel{ - types: types, - name: name, + types: types, + name: name, + optional: optional, }, nil } diff --git a/model/model_test.go b/model/model_test.go index e277d7f5..c1f16e38 100644 --- a/model/model_test.go +++ b/model/model_test.go @@ -50,7 +50,7 @@ func TestClientDBModel(t *testing.T) { } for _, tt := range tests { t.Run(fmt.Sprintf("TestNewModel_%s", tt.name), func(t *testing.T) { - db, err := NewClientDBModel(tt.name, tt.obj) + db, err := NewClientDBModel(tt.name, tt.obj, nil) if tt.valid { assert.Nil(t, err) assert.Len(t, db.types, len(tt.obj)) @@ -63,7 +63,7 @@ func TestClientDBModel(t *testing.T) { } func TestNewModel(t *testing.T) { - db, err := NewClientDBModel("testTable", map[string]Model{"Test_A": &modelA{}, "Test_B": &modelB{}}) + db, err := NewClientDBModel("testTable", map[string]Model{"Test_A": &modelA{}, "Test_B": &modelB{}}, nil) assert.Nil(t, err) _, err = db.newModel("Unknown") assert.NotNilf(t, err, "Creating model from unknown table should fail") @@ -95,7 +95,7 @@ func TestValidate(t *testing.T) { aSet []string `ovsdb:"aSet"` aMap map[string]string `ovsdb:"aMap"` }{}, - }) + }, nil) assert.Nil(t, err) tests := []struct { @@ -334,6 +334,109 @@ func TestValidate(t *testing.T) { } } +func TestValidateOptional(t *testing.T) { + model, err := NewClientDBModel("TestDB", map[string]Model{ + "TestTable": &struct { + aUUID string `ovsdb:"_uuid"` + aString string `ovsdb:"aString"` + aInt int `ovsdb:"aInt"` + aFloat float64 `ovsdb:"aFloat"` + aSet []string `ovsdb:"aSet"` + aMap map[string]string `ovsdb:"aMap"` + }{}, + "OptionalTable": &struct { + aUUID string `ovsdb:"_uuid"` + aString string `ovsdb:"aString"` + }{}, + }, map[string]bool{"OptionalTable": true}) + assert.Nil(t, err) + + tests := []struct { + name string + schema []byte + err bool + }{ + { + name: "wrong name", + schema: []byte(`{ + "name": "Wrong" + }`), + err: true, + }, + { + name: "full support", + schema: []byte(`{ + "name": "TestDB", + "tables": { + "TestTable": { + "columns": { + "aString": { "type": "string" }, + "aInt": { "type": "integer" }, + "aFloat": { "type": "real" } , + "aSet": { "type": { + "key": "string", + "max": "unlimited", + "min": 0 + } }, + "aMap": { + "type": { + "key": "string", + "max": "unlimited", + "min": 0, + "value": "string" + } + } + }, + "OptionalTable": { "columns": { "aString": { "type": "string" }}} + } + } + }`), + err: false, + }, + { + name: "missing optional table", + schema: []byte(`{ + "name": "TestDB", + "tables": { + "TestTable": { + "columns": { + "aString": { "type": "string" }, + "aInt": { "type": "integer" }, + "aFloat": { "type": "real" } , + "aSet": { "type": { + "key": "string", + "max": "unlimited", + "min": 0 + } }, + "aMap": { + "type": { + "key": "string", + "max": "unlimited", + "min": 0, + "value": "string" + } + } + } + } + } + }`), + err: false, + }, + } + for _, tt := range tests { + t.Run(fmt.Sprintf("TestValidate %s", tt.name), func(t *testing.T) { + var schema ovsdb.DatabaseSchema + err := json.Unmarshal(tt.schema, &schema) + assert.Nil(t, err) + errors := model.validate(schema) + if tt.err { + assert.Greater(t, len(errors), 0) + } else { + assert.Len(t, errors, 0) + } + }) + } +} func TestClone(t *testing.T) { a := &modelB{UID: "foo", Foo: "bar", Bar: "baz"} diff --git a/modelgen/dbmodel.go b/modelgen/dbmodel.go index e7a1866f..eeb9dca3 100644 --- a/modelgen/dbmodel.go +++ b/modelgen/dbmodel.go @@ -44,7 +44,7 @@ func FullDatabaseModel() (model.ClientDBModel, error) { return model.NewClientDBModel("{{ index . "DatabaseName" }}", map[string]model.Model{ {{ range index . "Tables" }} "{{ .TableName }}" : &{{ .StructName }}{}, {{ end }} - }) + }, nil) } var schema = {{ index . "Schema" | escape }} diff --git a/modelgen/dbmodel_test.go b/modelgen/dbmodel_test.go index bce11b4a..ef6db9b9 100644 --- a/modelgen/dbmodel_test.go +++ b/modelgen/dbmodel_test.go @@ -62,7 +62,7 @@ import ( func FullDatabaseModel() (model.ClientDBModel, error) { return model.NewClientDBModel("AtomicDB", map[string]model.Model{ "atomicTable": &AtomicTable{}, - }) + }, nil) } ` + ` var schema = ` + "`" + `{ diff --git a/ovsdb/serverdb/model.go b/ovsdb/serverdb/model.go index c8e5cec8..8db2913d 100644 --- a/ovsdb/serverdb/model.go +++ b/ovsdb/serverdb/model.go @@ -14,7 +14,7 @@ import ( func FullDatabaseModel() (model.ClientDBModel, error) { return model.NewClientDBModel("_Server", map[string]model.Model{ "Database": &Database{}, - }) + }, nil) } var schema = `{ diff --git a/server/server_integration_test.go b/server/server_integration_test.go index b3cd5e5f..49742c98 100644 --- a/server/server_integration_test.go +++ b/server/server_integration_test.go @@ -58,7 +58,7 @@ func getSchema() (ovsdb.DatabaseSchema, error) { func TestClientServerEcho(t *testing.T) { defDB, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{ "Open_vSwitch": &ovsType{}, - "Bridge": &bridgeType{}}) + "Bridge": &bridgeType{}}, nil) require.Nil(t, err) schema, err := getSchema() @@ -95,7 +95,7 @@ func TestClientServerEcho(t *testing.T) { func TestClientServerInsert(t *testing.T) { defDB, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{ "Open_vSwitch": &ovsType{}, - "Bridge": &bridgeType{}}) + "Bridge": &bridgeType{}}, nil) require.Nil(t, err) schema, err := getSchema() @@ -162,7 +162,7 @@ func TestClientServerInsert(t *testing.T) { func TestClientServerMonitor(t *testing.T) { defDB, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{ "Open_vSwitch": &ovsType{}, - "Bridge": &bridgeType{}}) + "Bridge": &bridgeType{}}, nil) if err != nil { t.Fatal(err) } @@ -288,7 +288,7 @@ func TestClientServerMonitor(t *testing.T) { func TestClientServerInsertAndDelete(t *testing.T) { defDB, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{ "Open_vSwitch": &ovsType{}, - "Bridge": &bridgeType{}}) + "Bridge": &bridgeType{}}, nil) require.Nil(t, err) schema, err := getSchema() @@ -354,7 +354,7 @@ func TestClientServerInsertDuplicate(t *testing.T) { defDB, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{ "Open_vSwitch": &ovsType{}, "Bridge": &bridgeType{}, - }) + }, nil) require.Nil(t, err) schema, err := getSchema() @@ -408,7 +408,7 @@ func TestClientServerInsertDuplicate(t *testing.T) { func TestClientServerInsertAndUpdate(t *testing.T) { defDB, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{ "Open_vSwitch": &ovsType{}, - "Bridge": &bridgeType{}}) + "Bridge": &bridgeType{}}, nil) require.Nil(t, err) schema, err := getSchema() diff --git a/server/server_test.go b/server/server_test.go index fde1823e..49bd68a9 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -68,7 +68,7 @@ func TestExpandNamedUUID(t *testing.T) { func TestOvsdbServerMonitor(t *testing.T) { defDB, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{ "Open_vSwitch": &ovsType{}, - "Bridge": &bridgeType{}}) + "Bridge": &bridgeType{}}, nil) if err != nil { t.Fatal(err) } diff --git a/server/transact_test.go b/server/transact_test.go index aa54a93b..d90bfb14 100644 --- a/server/transact_test.go +++ b/server/transact_test.go @@ -14,7 +14,7 @@ import ( func TestMutateOp(t *testing.T) { defDB, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{ "Open_vSwitch": &ovsType{}, - "Bridge": &bridgeType{}}) + "Bridge": &bridgeType{}}, nil) if err != nil { t.Fatal(err) } @@ -213,7 +213,7 @@ func TestOvsdbServerInsert(t *testing.T) { t.Skip("need a helper for comparing rows as map elements aren't in same order") defDB, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{ "Open_vSwitch": &ovsType{}, - "Bridge": &bridgeType{}}) + "Bridge": &bridgeType{}}, nil) if err != nil { t.Fatal(err) } @@ -271,7 +271,7 @@ func TestOvsdbServerInsert(t *testing.T) { func TestOvsdbServerUpdate(t *testing.T) { defDB, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{ "Open_vSwitch": &ovsType{}, - "Bridge": &bridgeType{}}) + "Bridge": &bridgeType{}}, nil) if err != nil { t.Fatal(err) } @@ -357,7 +357,7 @@ func TestOvsdbServerUpdate(t *testing.T) { func TestMultipleOps(t *testing.T) { defDB, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{ "Open_vSwitch": &ovsType{}, - "Bridge": &bridgeType{}}) + "Bridge": &bridgeType{}}, nil) if err != nil { t.Fatal(err) } diff --git a/test/ovs/ovs_integration_test.go b/test/ovs/ovs_integration_test.go index 4579c11d..da987bbc 100644 --- a/test/ovs/ovs_integration_test.go +++ b/test/ovs/ovs_integration_test.go @@ -172,7 +172,7 @@ var defDB, _ = model.NewClientDBModel("Open_vSwitch", map[string]model.Model{ "Bridge": &bridgeType{}, "IPFIX": &ipfixType{}, "Queue": &queueType{}, -}) +}, nil) func (suite *OVSIntegrationSuite) TestConnectReconnect() { assert.True(suite.T(), suite.client.Connected())