Skip to content

Commit

Permalink
mapper: Add compatility mode
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
amorenoz committed Oct 13, 2021
1 parent 874ab7a commit b3434b5
Show file tree
Hide file tree
Showing 9 changed files with 434 additions and 50 deletions.
2 changes: 1 addition & 1 deletion cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
1 change: 1 addition & 0 deletions cmd/stress/stress.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
31 changes: 21 additions & 10 deletions mapper/info.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package mapper

import (
"fmt"
"log"
"reflect"

"github.com/ovn-org/libovsdb/ovsdb"
Expand All @@ -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
}
Expand All @@ -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)

Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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
}
Expand Down
80 changes: 70 additions & 10 deletions mapper/info_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)

})
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand Down
6 changes: 3 additions & 3 deletions mapper/mapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
20 changes: 10 additions & 10 deletions mapper/mapper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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...)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
Loading

0 comments on commit b3434b5

Please sign in to comment.