Skip to content

Commit

Permalink
Add stronger typing for Enums
Browse files Browse the repository at this point in the history 
This commit:
1. Adjusts the mapper to handle stronger enum types
2. Adjusts modelgen to produce these types

Fixes: ovn-org#154

Signed-off-by: Dave Tucker <dave@dtucker.co.uk>
  • Loading branch information
@dave-tucker
dave-tucker committed Jul 15, 2021
1 parent ab07436 commit 5dc9643
Show file tree
Hide file tree
Showing 5 changed files with 126 additions and 32 deletions.
31 changes: 26 additions & 5 deletions mapper/info.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,28 @@ func (i *Info) SetField(column string, value interface{}) error {
return fmt.Errorf("column %s not found in orm info", column)
}
fieldValue := reflect.ValueOf(i.obj).Elem().FieldByName(fieldName)

v := reflect.ValueOf(value)
if !fieldValue.Type().AssignableTo(reflect.TypeOf(value)) {
return fmt.Errorf("column %s: native value %v (%s) is not assignable to field %s (%s)",
column, value, reflect.TypeOf(value), fieldName, fieldValue.Type())
if v.Type().ConvertibleTo(fieldValue.Type()) {
// handle enum
v = v.Convert(fieldValue.Type())
} else if fieldValue.Kind() == reflect.Slice {
// handle set of enums
if !v.Type().Elem().ConvertibleTo(fieldValue.Type().Elem()) {
return fmt.Errorf("column %s: element %v (%s) is not convertible to field %s element (%s)",
column, value, reflect.TypeOf(value), fieldName, fieldValue.Type())
}
nv := reflect.Zero(fieldValue.Type())
for i := 0; i < v.Len(); i++ {
nv = reflect.Append(nv, v.Index(i).Convert(fieldValue.Type().Elem()))
}
v = nv
} else {
return fmt.Errorf("column %s: native value %v (%s) is not assignable or convertible to field %s (%s)",
column, value, reflect.TypeOf(value), fieldName, fieldValue.Type())
}
}
fieldValue.Set(reflect.ValueOf(value))
fieldValue.Set(v)
return nil
}

Expand Down Expand Up @@ -133,7 +149,12 @@ func NewInfo(table *ovsdb.TableSchema, obj interface{}) (*Info, error) {

// Perform schema-based type checking
expType := ovsdb.NativeType(column)
if expType != field.Type {
// check for slice of enums
if expType.Kind() == reflect.Slice && expType.Elem().Kind() == reflect.String {
// it's a slice of enums
} else if expType.Kind() == reflect.String && field.Type.Kind() == reflect.String {
// it' an enum
} else if expType != field.Type {
return nil, &ErrMapper{
objType: objType.String(),
field: field.Name,
Expand Down
90 changes: 73 additions & 17 deletions mapper/info_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,25 +10,57 @@ import (
)

var sampleTable = []byte(`{
"columns": {
"columns": {
"aString": {
"type": "string"
"type": "string"
},
"aInteger": {
"type": "integer"
"type": "integer"
},
"aSet": {
"type": {
"key": "string",
"max": "unlimited",
"min": 0
}
"type": {
"key": "string",
"max": "unlimited",
"min": 0
}
},
"aMap": {
"type": {
"key": "string",
"value": "string"
}
"type": {
"key": "string",
"value": "string"
}
},
"aEnum": {
"type": {
"key": {
"enum": [
"set",
[
"enum1",
"enum2",
"enum3"
]
],
"type": "string"
}
}
},
"aEnumSet": {
"type": {
"key": {
"enum": [
"set",
[
"enum1",
"enum2",
"enum3"
]
],
"type": "string"
},
"max": "unlimited",
"min": 0
}
}
}
}`)
Expand Down Expand Up @@ -73,11 +105,19 @@ func TestNewMapperInfo(t *testing.T) {
}

func TestMapperInfoSet(t *testing.T) {
type enum string
const (
enum1 enum = "one"
enum2 enum = "two"
enum3 enum = "three"
)
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"`
Oint int `ovsdb:"aInteger"`
Oset []string `ovsdb:"aSet"`
Omap map[string]string `ovsdb:"aMap"`
Oenum enum `ovsdb:"aEnum"`
OenumSet []enum `ovsdb:"aEnumSet"`
}

type test struct {
Expand Down Expand Up @@ -127,13 +167,29 @@ func TestMapperInfoSet(t *testing.T) {
err: false,
},
{
name: "un-assignable",
name: "unassignable",
table: sampleTable,
obj: &obj{},
field: []string{"foo"},
column: "aMap",
err: true,
},
{
name: "enum",
table: sampleTable,
obj: &obj{},
field: enum1,
column: "aEnum",
err: false,
},
{
name: "enumSet",
table: sampleTable,
obj: &obj{},
field: []enum{enum2, enum3},
column: "aEnumSet",
err: false,
},
}
for _, tt := range tests {
t.Run(fmt.Sprintf("SetField_%s", tt.name), func(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion modelgen/table.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 6 additions & 6 deletions modelgen/table_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ func TestNewTableTemplate(t *testing.T) {
package test
type (
AtomicTableEventType = string
AtomicTableProtocol = string
AtomicTableEventType string
AtomicTableProtocol string
)
const (
Expand Down Expand Up @@ -114,8 +114,8 @@ type AtomicTable struct {
package test
type (
AtomicTableEventType = string
AtomicTableProtocol = string
AtomicTableEventType string
AtomicTableProtocol string
)
const (
Expand Down Expand Up @@ -163,8 +163,8 @@ func {{ index . "TestName" }} () string {
package test
type (
AtomicTableEventType = string
AtomicTableProtocol = string
AtomicTableEventType string
AtomicTableProtocol string
)
const (
Expand Down
23 changes: 20 additions & 3 deletions ovsdb/bindings.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,11 +182,28 @@ func NativeToOvsAtomic(basicType string, nativeElem interface{}) (interface{}, e
// NativeToOvs transforms an native type to a ovs type based on the column type information
func NativeToOvs(column *ColumnSchema, rawElem interface{}) (interface{}, error) {
naType := NativeType(column)

if t := reflect.TypeOf(rawElem); t != naType {
rawElemType := reflect.TypeOf(rawElem)
if column.Type == TypeEnum && rawElemType != strType && rawElemType.Kind() == reflect.String {
// cast type alias back to string
rawElem = rawElem.(string)
rawElemType = reflect.TypeOf(rawElem)
}
if column.Type == TypeSet && len(column.TypeObj.Key.Enum) > 0 && rawElemType.Elem() != strType && rawElemType.Elem().Kind() == reflect.String {
// convert a set of enums where the type is an alias, to a set of strings
tmp := []string{}
v := reflect.ValueOf(rawElem)
for i := 0; i < v.Len(); i++ {
// convert value to string first, because the String method won't panic
// but will instead return a string of <T value> which isn't what we want
vi := v.Index(i).Convert(strType)
tmp = append(tmp, vi.String())
}
rawElem = tmp
rawElemType = reflect.TypeOf(rawElem)
}
if t := rawElemType; t != naType {
return nil, NewErrWrongType("NativeToOvs", naType.String(), rawElem)
}

switch column.Type {
case TypeInteger, TypeReal, TypeString, TypeBoolean, TypeEnum:
return rawElem, nil
Expand Down

0 comments on commit 5dc9643

Please sign in to comment.