From 75c54e46ee4dba85ace840e641ff95b5e464f2a0 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Fri, 28 Jan 2022 15:33:27 -0600 Subject: [PATCH] server: implement column filters in Select() 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 --- mapper/mapper.go | 47 ++++++++++++++++++++++++++++-------------- ovsdb/set.go | 4 ++++ server/server.go | 9 +++++--- server/server_test.go | 48 +++++++++++++++++++++++++++++++++++-------- server/transact.go | 6 +++--- 5 files changed, 85 insertions(+), 29 deletions(-) diff --git a/mapper/mapper.go b/mapper/mapper.go index 21cecc13..560316e2 100644 --- a/mapper/mapper.go +++ b/mapper/mapper.go @@ -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 @@ -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()) @@ -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 diff --git a/ovsdb/set.go b/ovsdb/set.go index 60628a23..ab684b01 100644 --- a/ovsdb/set.go +++ b/ovsdb/set.go @@ -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) diff --git a/server/server.go b/server/server.go index 88c0e032..5baa129f 100644 --- a/server/server.go +++ b/server/server.go @@ -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 @@ -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 @@ -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 diff --git a/server/server_test.go b/server/server_test.go index fde1823e..008dbf51 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -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{GoSet: make([]interface{}, 0)} +} + func TestOvsdbServerMonitor(t *testing.T) { defDB, err := model.NewClientDBModel("Open_vSwitch", map[string]model.Model{ "Open_vSwitch": &ovsType{}, @@ -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(), }, }, }, diff --git a/server/transact.go b/server/transact.go index 26aa5411..1cad9546 100644 --- a/server/transact.go +++ b/server/transact.go @@ -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) @@ -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 @@ -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) }