Merge pull request #210 from dave-tucker/fix/203

client: Filter immutable fields from Update

client/api.go

@@ -320,14 +320,32 @@ func (a api) Mutate(model model.Model, mutationObjs ...model.Mutation) ([]ovsdb.
 	return operations, nil
 }
 
-// Update is a generic function capable of updating any field in any row in the database
+// Update is a generic function capable of updating any mutable field in any row in the database
 // Additional fields can be passed (variadic opts) to indicate fields to be updated
+// All immutable fields will be ignored
 func (a api) Update(model model.Model, fields ...interface{}) ([]ovsdb.Operation, error) {
 	var operations []ovsdb.Operation
 	table, err := a.getTableFromModel(model)
 	if err != nil {
 		return nil, err
 	}
+	tableSchema := a.cache.Mapper().Schema.Table(table)
+
+	if len(fields) > 0 {
+		info, err := mapper.NewInfo(tableSchema, model)
+		if err != nil {
+			return nil, err
+		}
+		for _, f := range fields {
+			colName, err := info.ColumnByPtr(f)
+			if err != nil {
+				return nil, err
+			}
+			if !tableSchema.Columns[colName].Mutable() {
+				return nil, fmt.Errorf("unable to update field %s of table %s as it is not mutable", colName, table)
+			}
+		}
+	}
 
 	conditions, err := a.cond.Generate()
 	if err != nil {
@@ -339,6 +357,17 @@ func (a api) Update(model model.Model, fields ...interface{}) ([]ovsdb.Operation
 		return nil, err
 	}
 
+	for colName, column := range tableSchema.Columns {
+		if !column.Mutable() {
+			delete(row, colName)
+		}
+	}
+	delete(row, "_uuid")
+
+	if len(row) == 0 {
+		return nil, fmt.Errorf("attempted to update using an empty row. please check that all fields you wish to update are mutable")
+	}
+
 	for _, condition := range conditions {
 		operations = append(operations,
 			ovsdb.Operation{

server/server_integration_test.go

@@ -456,15 +456,10 @@ func TestClientServerInsertAndUpdate(t *testing.T) {
 	// try to modify immutable field
 	bridgeRow.UUID = uuid
 	bridgeRow.Name = "br-update2"
-	ops, err = ovs.Where(bridgeRow).Update(bridgeRow)
-	require.NoError(t, err)
-	reply, err = ovs.Transact(context.Background(), ops...)
-	require.NoError(t, err)
-	_, err = ovsdb.CheckOperationResults(reply, ops)
+	_, err = ovs.Where(bridgeRow).Update(bridgeRow, &bridgeRow.Name)
 	require.Error(t, err)
 	bridgeRow.Name = "br-update"
 
-	/* FIXME: https://github.com/ovn-org/libovsdb/issues/203
 	// update many fields
 	bridgeRow.UUID = uuid
 	bridgeRow.Name = "br-update"
@@ -485,7 +480,6 @@ func TestClientServerInsertAndUpdate(t *testing.T) {
 		}
 		return reflect.DeepEqual(br, bridgeRow)
 	}, 2*time.Second, 50*time.Millisecond)
-	*/
 
 	newExternalIds := map[string]string{"foo": "bar"}
 	bridgeRow.ExternalIds = newExternalIds

test/ovs/ovs_integration_test.go

@@ -509,37 +509,29 @@ func (suite *OVSIntegrationSuite) TestUpdate() {
 	err = suite.client.Get(bridgeRow)
 	require.NoError(suite.T(), err)
 
-	// FIXME: https://github.com/ovn-org/libovsdb/issues/203
-	/*
-		// update many fields
-		bridgeRow.UUID = uuid
-		bridgeRow.ExternalIds["baz"] = "foobar"
-		bridgeRow.OtherConfig = map[string]string{"foo": "bar"}
-		ops, err = suite.client.Where(bridgeRow).Update(bridgeRow)
-		require.NoError(suite.T(), err)
-		reply, err = suite.client.Transact(ops...)
-		require.NoError(suite.T(), err)
-		opErrs, err := ovsdb.CheckOperationResults(reply, ops)
-		require.NoErrorf(suite.T(), err, "%+v", opErrs)
-
-		require.Eventually(suite.T(), func() bool {
-			br := &bridgeType{UUID: uuid}
-			err = suite.client.Get(br)
-			if err != nil {
-				return false
-			}
-			return reflect.DeepEqual(bridgeRow, br)
-		}, 2*time.Second, 500*time.Millisecond)
-
-	*/
-
-	// try to modify immutable field
-	bridgeRow.Name = "br-update2"
+	// update many fields
+	bridgeRow.UUID = uuid
+	bridgeRow.ExternalIds["baz"] = "foobar"
+	bridgeRow.OtherConfig = map[string]string{"foo": "bar"}
 	ops, err := suite.client.Where(bridgeRow).Update(bridgeRow)
 	require.NoError(suite.T(), err)
 	reply, err := suite.client.Transact(context.TODO(), ops...)
 	require.NoError(suite.T(), err)
-	_, err = ovsdb.CheckOperationResults(reply, ops)
+	opErrs, err := ovsdb.CheckOperationResults(reply, ops)
+	require.NoErrorf(suite.T(), err, "%+v", opErrs)
+
+	require.Eventually(suite.T(), func() bool {
+		br := &bridgeType{UUID: uuid}
+		err = suite.client.Get(br)
+		if err != nil {
+			return false
+		}
+		return reflect.DeepEqual(bridgeRow, br)
+	}, 2*time.Second, 500*time.Millisecond)
+
+	// try to modify immutable field
+	bridgeRow.Name = "br-update2"
+	_, err = suite.client.Where(bridgeRow).Update(bridgeRow, &bridgeRow.Name)
 	require.Error(suite.T(), err)
 	// set name back again
 	bridgeRow.Name = "br-update"