Skip to content

Commit

Permalink
server: implement column filters in Select()
Browse files Browse the repository at this point in the history 
Since the unit test for Monitor() requests all columns,
we now have to expect empty fields for those columns in
the response.

Previously Select() just ignored the columns that
Monitor() requested and thus elided empty columns from
the result.

Signed-off-by: Dan Williams <[email protected]>
  • Loading branch information
@dcbw
dcbw committed Jan 28, 2022
1 parent a619f0f commit 31489e5
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 29 deletions.
47 changes: 32 additions & 15 deletions mapper/mapper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,20 @@ func (m Mapper) getData(ovsData ovsdb.Row, result *Info) error {
return nil
}

// NewRow transforms an orm struct to a map[string] interface{} that can be used as libovsdb.Row
// NewRowWithColumns 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(data *Info, fields ...interface{}) (ovsdb.Row, error) {
// a list of column names to be added to the row. If alwaysIncludeUUID is true the UUID is always
// included in the results even if not present in columnNames.
func (m Mapper) NewRowWithColumns(data *Info, alwaysIncludeUUID bool, columnNames ...string) (ovsdb.Row, error) {
// validate column names against row info
fields := make(map[string]struct{})
for _, col := range columnNames {
if _, err := data.FieldByColumn(col); err != nil {
return nil, err
}
fields[col] = struct{}{}
}

columns := make(map[string]*ovsdb.ColumnSchema)
for k, v := range data.Metadata.TableSchema.Columns {
columns[k] = v
Expand All @@ -100,24 +110,15 @@ func (m Mapper) NewRow(data *Info, fields ...interface{}) (ovsdb.Row, error) {

// add specific fields
if len(fields) > 0 {
found := false
for _, f := range fields {
col, err := data.ColumnByPtr(f)
if err != nil {
return nil, err
}
if col == name {
found = true
break
}
}
if !found {
wantUUID := alwaysIncludeUUID && (name == "_uuid")
if _, ok := fields[name]; !ok && !wantUUID {
continue
}
}
if len(fields) == 0 && ovsdb.IsDefaultValue(column, nativeElem) {
continue
}

ovsElem, err := ovsdb.NativeToOvs(column, nativeElem)
if err != nil {
return nil, fmt.Errorf("table %s, column %s: failed to generate ovs element. %s", data.Metadata.TableName, name, err.Error())
Expand All @@ -127,6 +128,22 @@ func (m Mapper) NewRow(data *Info, fields ...interface{}) (ovsdb.Row, error) {
return ovsRow, nil
}

// 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(data *Info, fields ...interface{}) (ovsdb.Row, error) {
columnNames := make([]string, 0, len(fields))
for _, f := range fields {
col, err := data.ColumnByPtr(f)
if err != nil {
return nil, err
}
columnNames = append(columnNames, col)
}

return m.NewRowWithColumns(data, false, columnNames...)
}

// NewEqualityCondition returns a list of equality conditions that match a given object
// A list of valid columns that shall be used as a index can be provided.
// If none are provided, we will try to use object's field that matches the '_uuid' ovsdb tag
Expand Down
4 changes: 4 additions & 0 deletions ovsdb/set.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ type OvsSet struct {
func NewOvsSet(obj interface{}) (OvsSet, error) {
var v reflect.Value
if reflect.TypeOf(obj).Kind() == reflect.Ptr {
if reflect.ValueOf(obj).IsZero() {
// Empty set
return OvsSet{make([]interface{}, 0)}, nil
}
v = reflect.ValueOf(obj).Elem()
} else {
v = reflect.ValueOf(obj)
Expand Down
9 changes: 6 additions & 3 deletions server/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,8 @@ func (o *OvsdbServer) Monitor(client *rpc2.Client, args []json.RawMessage, reply

tableUpdates := make(ovsdb.TableUpdates)
for t, request := range request {
rows := transaction.Select(t, nil, request.Columns)
// FIXME: don't include _uuid in results if client didn't request it
rows := transaction.Select(t, nil, true, request.Columns)
for i := range rows.Rows {
tu := make(ovsdb.TableUpdate)
uuid := rows.Rows[i]["_uuid"].(ovsdb.UUID).GoUUID
Expand Down Expand Up @@ -324,7 +325,8 @@ func (o *OvsdbServer) MonitorCond(client *rpc2.Client, args []json.RawMessage, r

tableUpdates := make(ovsdb.TableUpdates2)
for t, request := range request {
rows := transaction.Select(t, nil, request.Columns)
// FIXME: don't include _uuid in results if client didn't request it
rows := transaction.Select(t, nil, true, request.Columns)
for i := range rows.Rows {
tu := make(ovsdb.TableUpdate2)
uuid := rows.Rows[i]["_uuid"].(ovsdb.UUID).GoUUID
Expand Down Expand Up @@ -369,7 +371,8 @@ func (o *OvsdbServer) MonitorCondSince(client *rpc2.Client, args []json.RawMessa

tableUpdates := make(ovsdb.TableUpdates2)
for t, request := range request {
rows := transaction.Select(t, nil, request.Columns)
// FIXME: don't include _uuid in results if client didn't request it
rows := transaction.Select(t, nil, true, request.Columns)
for i := range rows.Rows {
tu := make(ovsdb.TableUpdate2)
uuid := rows.Rows[i]["_uuid"].(ovsdb.UUID).GoUUID
Expand Down
48 changes: 40 additions & 8 deletions server/server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,14 @@ func TestExpandNamedUUID(t *testing.T) {
}
}

func emptyOvsMap() ovsdb.OvsMap {
return ovsdb.OvsMap{GoMap: make(map[interface{}]interface{})}
}

func emptyOvsSet() ovsdb.OvsSet {
return ovsdb.OvsSet{make([]interface{}, 0)}
}

func TestOvsdbServerMonitor(t *testing.T) {
defDB, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{
"Open_vSwitch": &ovsType{},
Expand Down Expand Up @@ -124,26 +132,50 @@ func TestOvsdbServerMonitor(t *testing.T) {
"Bridge": {
fooUUID: &ovsdb.RowUpdate{
New: &ovsdb.Row{
"_uuid": ovsdb.UUID{GoUUID: fooUUID},
"name": "foo",
"_uuid": ovsdb.UUID{GoUUID: fooUUID},
"name": "foo",
"datapath_id": emptyOvsSet(),
"datapath_type": "",
"external_ids": emptyOvsMap(),
"other_config": emptyOvsMap(),
"ports": emptyOvsSet(),
"status": emptyOvsMap(),
},
},
barUUID: &ovsdb.RowUpdate{
New: &ovsdb.Row{
"_uuid": ovsdb.UUID{GoUUID: barUUID},
"name": "bar",
"_uuid": ovsdb.UUID{GoUUID: barUUID},
"name": "bar",
"datapath_id": emptyOvsSet(),
"datapath_type": "",
"external_ids": emptyOvsMap(),
"other_config": emptyOvsMap(),
"ports": emptyOvsSet(),
"status": emptyOvsMap(),
},
},
bazUUID: &ovsdb.RowUpdate{
New: &ovsdb.Row{
"_uuid": ovsdb.UUID{GoUUID: bazUUID},
"name": "baz",
"_uuid": ovsdb.UUID{GoUUID: bazUUID},
"name": "baz",
"datapath_id": emptyOvsSet(),
"datapath_type": "",
"external_ids": emptyOvsMap(),
"other_config": emptyOvsMap(),
"ports": emptyOvsSet(),
"status": emptyOvsMap(),
},
},
quuxUUID: &ovsdb.RowUpdate{
New: &ovsdb.Row{
"_uuid": ovsdb.UUID{GoUUID: quuxUUID},
"name": "quux",
"_uuid": ovsdb.UUID{GoUUID: quuxUUID},
"name": "quux",
"datapath_id": emptyOvsSet(),
"datapath_type": "",
"external_ids": emptyOvsMap(),
"other_config": emptyOvsMap(),
"ports": emptyOvsSet(),
"status": emptyOvsMap(),
},
},
},
Expand Down
6 changes: 3 additions & 3 deletions server/transact.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func (o *OvsdbServer) transact(name string, operations []ovsdb.Operation) ([]ovs
}
}
case ovsdb.OperationSelect:
r := transaction.Select(op.Table, op.Where, op.Columns)
r := transaction.Select(op.Table, op.Where, false, op.Columns)
results = append(results, r)
case ovsdb.OperationUpdate:
r, tu := transaction.Update(name, op.Table, op.Where, op.Row)
Expand Down Expand Up @@ -207,7 +207,7 @@ func (t *Transaction) Insert(table string, rowUUID string, row ovsdb.Row) (ovsdb
}
}

func (t *Transaction) Select(table string, where []ovsdb.Condition, columns []string) ovsdb.OperationResult {
func (t *Transaction) Select(table string, where []ovsdb.Condition, alwaysIncludeUUID bool, columns []string) ovsdb.OperationResult {
var results []ovsdb.Row
dbModel := t.Model

Expand All @@ -222,7 +222,7 @@ func (t *Transaction) Select(table string, where []ovsdb.Condition, columns []st
if err != nil {
panic(err)
}
resultRow, err := m.NewRow(info)
resultRow, err := m.NewRowWithColumns(info, alwaysIncludeUUID, columns...)
if err != nil {
panic(err)
}
Expand Down

0 comments on commit 31489e5

Please sign in to comment.