Repository: ovn-org/libovsdb
Pull request #125 (2021-05-24 12:27:55)

api: use variadic args in Mutate
It makes the API less verbose and more homogeneous with other commands
such as Where()

Fixes: #124 
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>


Patch:
diff --git a/client/api.go b/client/api.go
index fcfac08..4ffc462 100644
--- a/client/api.go
+++ b/client/api.go
@@ -61,7 +61,7 @@ type ConditionalAPI interface {
 	// Mutate returns the operations needed to perform the mutation specified
 	// By the model and the list of Mutation objects
 	// Depending on the Condition, it might return one or many operations
-	Mutate(Model, []Mutation) ([]ovsdb.Operation, error)
+	Mutate(Model, ...Mutation) ([]ovsdb.Operation, error)
 
 	// Update returns the operations needed to update any number of rows according
 	// to the data in the given model.
@@ -325,10 +325,14 @@ func (a api) Create(models ...Model) ([]ovsdb.Operation, error) {
 }
 
 // Mutate returns the operations needed to transform the one Model into another one
-func (a api) Mutate(model Model, mutationObjs []Mutation) ([]ovsdb.Operation, error) {
+func (a api) Mutate(model Model, mutationObjs ...Mutation) ([]ovsdb.Operation, error) {
 	var mutations []interface{}
 	var operations []ovsdb.Operation
 
+	if len(mutationObjs) < 1 {
+		return nil, fmt.Errorf("At least one Mutation must be provided")
+	}
+
 	tableName := a.cache.dbModel.FindTable(reflect.ValueOf(model).Type())
 	table := a.cache.orm.schema.Table(tableName)
 	if table == nil {
diff --git a/cmd/stress/stress.go b/cmd/stress/stress.go
index 0251f70..abc0ea9 100644
--- a/cmd/stress/stress.go
+++ b/cmd/stress/stress.go
@@ -107,12 +107,10 @@ func deleteBridge(ovs *client.OvsdbClient, bridge *ormBridge) {
 		UUID: rootUUID,
 	}
 
-	mutateOp, err := ovs.Where(&ovsRow).Mutate(&ovsRow, []client.Mutation{
-		{
-			Field:   &ovsRow.Bridges,
-			Mutator: "delete",
-			Value:   []string{bridge.UUID},
-		},
+	mutateOp, err := ovs.Where(&ovsRow).Mutate(&ovsRow, client.Mutation{
+		Field:   &ovsRow.Bridges,
+		Mutator: "delete",
+		Value:   []string{bridge.UUID},
 	})
 	if err != nil {
 		log.Fatal(err)
@@ -145,12 +143,10 @@ func createBridge(ovs *client.OvsdbClient, iter int) {
 		log.Fatal(err)
 	}
 	ovsRow := ormOvs{}
-	mutateOp, err := ovs.Where(&ormOvs{UUID: rootUUID}).Mutate(&ovsRow, []client.Mutation{
-		{
-			Field:   &ovsRow.Bridges,
-			Mutator: "insert",
-			Value:   []string{bridge.UUID},
-		},
+	mutateOp, err := ovs.Where(&ormOvs{UUID: rootUUID}).Mutate(&ovsRow, client.Mutation{
+		Field:   &ovsRow.Bridges,
+		Mutator: "insert",
+		Value:   []string{bridge.UUID},
 	})
 	if err != nil {
 		log.Fatal(err)
diff --git a/example/play_with_ovs/play_with_ovs.go b/example/play_with_ovs/play_with_ovs.go
index 63e5e03..8933e80 100644
--- a/example/play_with_ovs/play_with_ovs.go
+++ b/example/play_with_ovs/play_with_ovs.go
@@ -72,12 +72,11 @@ func createBridge(ovs *client.OvsdbClient, bridgeName string) {
 	ovsRow := ormOvs{
 		UUID: rootUUID,
 	}
-	mutateOps, err := ovs.Where(&ovsRow).Mutate(&ovsRow, []client.Mutation{
-		{
-			Field:   &ovsRow.Bridges,
-			Mutator: "insert",
-			Value:   []string{bridge.UUID},
-		}})
+	mutateOps, err := ovs.Where(&ovsRow).Mutate(&ovsRow, client.Mutation{
+		Field:   &ovsRow.Bridges,
+		Mutator: "insert",
+		Value:   []string{bridge.UUID},
+	})
 	if err != nil {
 		log.Fatal(err)
 	}


Test patch:
diff --git a/client/api_test.go b/client/api_test.go
index 087733c..cf4b678 100644
--- a/client/api_test.go
+++ b/client/api_test.go
@@ -745,12 +745,22 @@ func TestAPIMutate(t *testing.T) {
 			},
 			err: false,
 		},
+		{
+			name: "No mutations should error",
+			condition: func(a API) ConditionalAPI {
+				return a.WhereCache(func(lsp *testLogicalSwitchPort) bool {
+					return lsp.Type == "someType"
+				})
+			},
+			mutations: []Mutation{},
+			err:       true,
+		},
 	}
 	for _, tt := range test {
 		t.Run(fmt.Sprintf("ApiMutate: %s", tt.name), func(t *testing.T) {
 			api := newAPI(cache)
 			cond := tt.condition(api)
-			ops, err := cond.Mutate(&testObj, tt.mutations)
+			ops, err := cond.Mutate(&testObj, tt.mutations...)
 			if tt.err {
 				assert.NotNil(t, err)
 			} else {
diff --git a/client/ovs_integration_test.go b/client/ovs_integration_test.go
index 112574d..a086d4d 100644
--- a/client/ovs_integration_test.go
+++ b/client/ovs_integration_test.go
@@ -192,12 +192,10 @@ func TestInsertTransactIntegration(t *testing.T) {
 	// Inserting a Bridge row in Bridge table requires mutating the open_vswitch table.
 	ovsRow := ovsType{}
 	mutateOp, err := ovs.WhereCache(func(*ovsType) bool { return true }).
-		Mutate(&ovsRow, []Mutation{
-			{
-				Field:   &ovsRow.Bridges,
-				Mutator: ovsdb.MutateOperationInsert,
-				Value:   []string{namedUUID},
-			},
+		Mutate(&ovsRow, Mutation{
+			Field:   &ovsRow.Bridges,
+			Mutator: ovsdb.MutateOperationInsert,
+			Value:   []string{namedUUID},
 		})
 	assert.Nil(t, err)
 
@@ -240,12 +238,10 @@ func TestDeleteTransactIntegration(t *testing.T) {
 
 	ovsRow := ovsType{}
 	mutateOp, err := ovs.WhereCache(func(*ovsType) bool { return true }).
-		Mutate(&ovsRow, []Mutation{
-			{
-				Field:   &ovsRow.Bridges,
-				Mutator: ovsdb.MutateOperationDelete,
-				Value:   []string{bridgeUUID},
-			},
+		Mutate(&ovsRow, Mutation{
+			Field:   &ovsRow.Bridges,
+			Mutator: ovsdb.MutateOperationDelete,
+			Value:   []string{bridgeUUID},
 		})
 
 	assert.Nil(t, err)
@@ -544,12 +540,10 @@ func TestInsertDuplicateTransactIntegration(t *testing.T) {
 	// Inserting a Bridge row in Bridge table requires mutating the open_vswitch table.
 	ovsRow := ovsType{}
 	mutateOp, err := ovs.WhereCache(func(*ovsType) bool { return true }).
-		Mutate(&ovsRow, []Mutation{
-			{
-				Field:   &ovsRow.Bridges,
-				Mutator: ovsdb.MutateOperationInsert,
-				Value:   []string{namedUUID},
-			},
+		Mutate(&ovsRow, Mutation{
+			Field:   &ovsRow.Bridges,
+			Mutator: ovsdb.MutateOperationInsert,
+			Value:   []string{namedUUID},
 		})
 	assert.Nil(t, err)