From 962bc84bbc8c2629473e3b7d248ce68a5061bbce Mon Sep 17 00:00:00 2001
From: Adrian Moreno <amorenoz@redhat.com>
Date: Fri, 1 Oct 2021 11:49:24 +0200
Subject: [PATCH] bindings: convert optional nil pointers to empty sets

An optional field can be a nil pointer which represents, in this case,
an empty set.

Make NativeToOvs convert nil pointers to empty OvsSets so Update()
operations that want to clear an optional field work.

Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
---
 ovsdb/bindings.go                |  8 +++--
 ovsdb/bindings_test.go           | 17 +++++++++
 test/ovs/ovs_integration_test.go | 60 +++++++++++++++++++++++++++++++-
 3 files changed, 81 insertions(+), 4 deletions(-)

diff --git a/ovsdb/bindings.go b/ovsdb/bindings.go
index 734d80b1..f80b751c 100644
--- a/ovsdb/bindings.go
+++ b/ovsdb/bindings.go
@@ -258,7 +258,7 @@ func NativeToOvs(column *ColumnSchema, rawElem interface{}) (interface{}, error)
 	case TypeSet:
 		var ovsSet OvsSet
 		if column.TypeObj.Key.Type == TypeUUID {
-			var ovsSlice []interface{}
+			ovsSlice := []interface{}{}
 			if _, ok := rawElem.([]string); ok {
 				for _, v := range rawElem.([]string) {
 					uuid := UUID{GoUUID: v}
@@ -266,8 +266,10 @@ func NativeToOvs(column *ColumnSchema, rawElem interface{}) (interface{}, error)
 				}
 			} else if _, ok := rawElem.(*string); ok {
 				v := rawElem.(*string)
-				uuid := UUID{GoUUID: *v}
-				ovsSlice = append(ovsSlice, uuid)
+				if v != nil {
+					uuid := UUID{GoUUID: *v}
+					ovsSlice = append(ovsSlice, uuid)
+				}
 			} else {
 				return nil, fmt.Errorf("uuid slice was neither []string or *string")
 			}
diff --git a/ovsdb/bindings_test.go b/ovsdb/bindings_test.go
index bbfac118..afac2d06 100644
--- a/ovsdb/bindings_test.go
+++ b/ovsdb/bindings_test.go
@@ -394,6 +394,23 @@ func TestOvsToNativeAndNativeToOvs(t *testing.T) {
 			native: &aUUID0,
 			ovs:    aSingleUUIDSet,
 		},
+		{
+			name: "null UUID set with min 0 max 1",
+			schema: []byte(`{
+			"type":{
+				"key": {
+					"refTable": "SomeOtherTAble",
+					"refType": "weak",
+					"type": "uuid"
+				},
+				"min": 0,
+				"max": 1
+			}
+			}`),
+			input:  es,
+			native: (*string)(nil),
+			ovs:    es,
+		},
 		{
 			name: "A string with min 0 max 1",
 			schema: []byte(`{
diff --git a/test/ovs/ovs_integration_test.go b/test/ovs/ovs_integration_test.go
index 1deadc45..aa038df4 100644
--- a/test/ovs/ovs_integration_test.go
+++ b/test/ovs/ovs_integration_test.go
@@ -125,6 +125,7 @@ type bridgeType struct {
 	Ports          []string          `ovsdb:"ports"`
 	Status         map[string]string `ovsdb:"status"`
 	BridgeFailMode *BridgeFailMode   `ovsdb:"fail_mode"`
+	IPFIX          *string           `ovsdb:"ipfix"`
 }
 
 // ovsType is the simplified ORM model of the Bridge table
@@ -133,9 +134,16 @@ type ovsType struct {
 	Bridges []string `ovsdb:"bridges"`
 }
 
+// ipfixType is a simplified ORM model for the IPFIX table
+type ipfixType struct {
+	UUID    string   `ovsdb:"_uuid"`
+	Targets []string `ovsdb:"targets"`
+}
+
 var defDB, _ = model.NewDBModel("Open_vSwitch", map[string]model.Model{
 	"Open_vSwitch": &ovsType{},
-	"Bridge":       &bridgeType{}})
+	"Bridge":       &bridgeType{},
+	"IPFIX":        &ipfixType{}})
 
 func (suite *OVSIntegrationSuite) TestConnectReconnect() {
 	assert.True(suite.T(), suite.client.Connected())
@@ -588,3 +596,53 @@ func (suite *OVSIntegrationSuite) createBridge(bridgeName string) (string, error
 	_, err = ovsdb.CheckOperationResults(reply, operations)
 	return reply[0].UUID.GoUUID, err
 }
+
+func (suite *OVSIntegrationSuite) TestCreateIPFIX() {
+	// Create a IPFIX row and update the bridge in the same transaction
+	uuid, err := suite.createBridge("br-ipfix")
+	require.NoError(suite.T(), err)
+	namedUUID := "gopher"
+	ipfix := ipfixType{
+		UUID:    namedUUID,
+		Targets: []string{"127.0.0.1:6650"},
+	}
+	insertOp, err := suite.client.Create(&ipfix)
+	require.NoError(suite.T(), err)
+
+	bridge := bridgeType{
+		UUID:  uuid,
+		IPFIX: &namedUUID,
+	}
+	updateOps, err := suite.client.Where(&bridge).Update(&bridge, &bridge.IPFIX)
+	require.NoError(suite.T(), err)
+	operations := append(insertOp, updateOps...)
+	reply, err := suite.client.Transact(context.TODO(), operations...)
+	require.NoError(suite.T(), err)
+	opErrs, err := ovsdb.CheckOperationResults(reply, operations)
+	if err != nil {
+		for _, oe := range opErrs {
+			suite.T().Error(oe)
+		}
+	}
+
+	// Delete the IPFIX row by removing it's strong reference
+	bridge.IPFIX = nil
+	updateOps, err = suite.client.Where(&bridge).Update(&bridge, &bridge.IPFIX)
+	require.NoError(suite.T(), err)
+	reply, err = suite.client.Transact(context.TODO(), updateOps...)
+	require.NoError(suite.T(), err)
+	opErrs, err = ovsdb.CheckOperationResults(reply, updateOps)
+	if err != nil {
+		for _, oe := range opErrs {
+			suite.T().Error(oe)
+		}
+	}
+	require.NoError(suite.T(), err)
+
+	//Assert the IPFIX table is empty
+	ipfixes := []ipfixType{}
+	err = suite.client.List(&ipfixes)
+	require.NoError(suite.T(), err)
+	require.Empty(suite.T(), ipfixes)
+
+}