api: fix splice update operation

Splice update operation (`:`) accepts 5 and only 5 arguments.
It was parsed and encoded incorrectly with only 3 argument
(as every other update operations).
Also fixed the same operation for the crud.

Part of #348

Author: DerekBum

CHANGELOG.md

@@ -80,6 +80,7 @@ Versioning](http://semver.org/spec/v2.0.0.html) except to the first release.
   `prefer_replica`, `balance`) setup for crud.GetRequest (#335)
 - Tests with crud 1.4.0 (#336)
 - Tests with case sensitive SQL (#341)
+- Splice update operation accepts 3 arguments instead of 5 (#348)
 
 ## [1.12.0] - 2023-06-07
 

README.md

@@ -253,6 +253,11 @@ interface to get information if the usage of space and index names in requests
 is supported.
 * `Schema` structure no longer implements `SchemaResolver` interface.
 
+#### Client tools changes
+
+* Remove `OpSplice` struct.
+* `Operations.Splice` method now accepts 5 arguments instead of 3.
+
 ## Contributing
 
 See [the contributing guide](CONTRIBUTING.md) for detailed instructions on how

client_tools.go

@@ -59,12 +59,38 @@ type Op struct {
 	Op    string
 	Field int
 	Arg   interface{}
+	// Pos, Len, Replace fields used in the Splice operation.
+	Pos     int
+	Len     int
+	Replace string
 }
 
 func (o Op) EncodeMsgpack(enc *msgpack.Encoder) error {
-	enc.EncodeArrayLen(3)
-	enc.EncodeString(o.Op)
-	enc.EncodeInt(int64(o.Field))
+	isSpliceOperation := o.Op == spliceOperator
+	argsLen := 3
+	if isSpliceOperation {
+		argsLen = 5
+	}
+	if err := enc.EncodeArrayLen(argsLen); err != nil {
+		return err
+	}
+	if err := enc.EncodeString(o.Op); err != nil {
+		return err
+	}
+	if err := enc.EncodeInt(int64(o.Field)); err != nil {
+		return err
+	}
+
+	if isSpliceOperation {
+		if err := enc.EncodeInt(int64(o.Pos)); err != nil {
+			return err
+		}
+		if err := enc.EncodeInt(int64(o.Len)); err != nil {
+			return err
+		}
+		return enc.EncodeString(o.Replace)
+	}
+
 	return enc.Encode(o.Arg)
 }
 
@@ -92,7 +118,12 @@ func NewOperations() *Operations {
 }
 
 func (ops *Operations) append(op string, field int, arg interface{}) *Operations {
-	ops.ops = append(ops.ops, Op{op, field, arg})
+	ops.ops = append(ops.ops, Op{Op: op, Field: field, Arg: arg})
+	return ops
+}
+
+func (ops *Operations) appendSplice(op string, field, pos, len int, replace string) *Operations {
+	ops.ops = append(ops.ops, Op{Op: op, Field: field, Pos: pos, Len: len, Replace: replace})
 	return ops
 }
 
@@ -122,8 +153,8 @@ func (ops *Operations) BitwiseXor(field int, arg interface{}) *Operations {
 }
 
 // Splice adds a splice operation to the collection of update operations.
-func (ops *Operations) Splice(field int, arg interface{}) *Operations {
-	return ops.append(spliceOperator, field, arg)
+func (ops *Operations) Splice(field, pos, len int, replace string) *Operations {
+	return ops.appendSplice(spliceOperator, field, pos, len, replace)
 }
 
 // Insert adds an insert operation to the collection of update operations.
@@ -140,21 +171,3 @@ func (ops *Operations) Delete(field int, arg interface{}) *Operations {
 func (ops *Operations) Assign(field int, arg interface{}) *Operations {
 	return ops.append(assignOperator, field, arg)
 }
-
-type OpSplice struct {
-	Op      string
-	Field   int
-	Pos     int
-	Len     int
-	Replace string
-}
-
-func (o OpSplice) EncodeMsgpack(enc *msgpack.Encoder) error {
-	enc.EncodeArrayLen(5)
-	enc.EncodeString(o.Op)
-	enc.EncodeInt(int64(o.Field))
-	enc.EncodeInt(int64(o.Pos))
-	enc.EncodeInt(int64(o.Len))
-	enc.EncodeString(o.Replace)
-	return nil
-}

crud/operations.go

@@ -1,5 +1,9 @@
 package crud
 
+import (
+	"github.com/vmihailenco/msgpack/v5"
+)
+
 const (
 	// Add - operator for addition.
 	Add Operator = "+"
@@ -23,11 +27,43 @@ const (
 
 // Operation describes CRUD operation as a table
 // {operator, field_identifier, value}.
+// Splice operation described as a table
+// {operator, field_identifier, position, length, replace_string}.
 type Operation struct {
-	// Instruct msgpack to pack this struct as array, so no custom packer
-	// is needed.
-	_msgpack struct{} `msgpack:",asArray"` //nolint: structcheck,unused
 	Operator Operator
 	Field    interface{} // Number or string.
 	Value    interface{}
+	// Pos, Len, Replace fields used in the Splice operation.
+	Pos     int
+	Len     int
+	Replace string
+}
+
+func (o Operation) EncodeMsgpack(enc *msgpack.Encoder) error {
+	isSpliceOperation := o.Operator == Splice
+	argsLen := 3
+	if isSpliceOperation {
+		argsLen = 5
+	}
+	if err := enc.EncodeArrayLen(argsLen); err != nil {
+		return err
+	}
+	if err := enc.EncodeString(string(o.Operator)); err != nil {
+		return err
+	}
+	if err := enc.Encode(o.Field); err != nil {
+		return err
+	}
+
+	if isSpliceOperation {
+		if err := enc.EncodeInt(int64(o.Pos)); err != nil {
+			return err
+		}
+		if err := enc.EncodeInt(int64(o.Len)); err != nil {
+			return err
+		}
+		return enc.EncodeString(o.Replace)
+	}
+
+	return enc.Encode(o.Value)
 }

crud/tarantool_test.go

@@ -40,6 +40,63 @@ var startOpts test_helpers.StartOpts = test_helpers.StartOpts{
 var timeout = float64(1.1)
 
 var operations = []crud.Operation{
+	// Insert new fields,
+	// because double update of the same field results in an error.
+	{
+		Operator: crud.Insert,
+		Field:    4,
+		Value:    0,
+	},
+	{
+		Operator: crud.Insert,
+		Field:    5,
+		Value:    0,
+	},
+	{
+		Operator: crud.Insert,
+		Field:    6,
+		Value:    0,
+	},
+	{
+		Operator: crud.Insert,
+		Field:    7,
+		Value:    0,
+	},
+	{
+		Operator: crud.Insert,
+		Field:    8,
+		Value:    0,
+	},
+	{
+		Operator: crud.Add,
+		Field:    4,
+		Value:    1,
+	},
+	{
+		Operator: crud.Sub,
+		Field:    5,
+		Value:    1,
+	},
+	{
+		Operator: crud.And,
+		Field:    6,
+		Value:    1,
+	},
+	{
+		Operator: crud.Or,
+		Field:    7,
+		Value:    1,
+	},
+	{
+		Operator: crud.Xor,
+		Field:    8,
+		Value:    1,
+	},
+	{
+		Operator: crud.Delete,
+		Field:    4,
+		Value:    5,
+	},
 	{
 		Operator: crud.Assign,
 		Field:    "name",
@@ -542,6 +599,81 @@ func TestCrudProcessData(t *testing.T) {
 	}
 }
 
+func TestCrudUpdateSplice(t *testing.T) {
+	test_helpers.SkipIfCrudSpliceBroken(t)
+
+	conn := connect(t)
+	defer conn.Close()
+
+	req := crud.MakeUpdateRequest(spaceName).
+		Key(key).
+		Operations([]crud.Operation{
+			{
+				Operator: crud.Splice,
+				Field:    "name",
+				Pos:      1,
+				Len:      1,
+				Replace:  "!!",
+			},
+		}).
+		Opts(simpleOperationOpts)
+
+	testCrudRequestPrepareData(t, conn)
+	resp, err := conn.Do(req).Get()
+	testCrudRequestCheck(t, req, resp,
+		err, 2)
+}
+
+func TestCrudUpsertSplice(t *testing.T) {
+	test_helpers.SkipIfCrudSpliceBroken(t)
+
+	conn := connect(t)
+	defer conn.Close()
+
+	req := crud.MakeUpsertRequest(spaceName).
+		Tuple(tuple).
+		Operations([]crud.Operation{
+			{
+				Operator: crud.Splice,
+				Field:    "name",
+				Pos:      1,
+				Len:      1,
+				Replace:  "!!",
+			},
+		}).
+		Opts(simpleOperationOpts)
+
+	testCrudRequestPrepareData(t, conn)
+	resp, err := conn.Do(req).Get()
+	testCrudRequestCheck(t, req, resp,
+		err, 2)
+}
+
+func TestCrudUpsertObjectSplice(t *testing.T) {
+	test_helpers.SkipIfCrudSpliceBroken(t)
+
+	conn := connect(t)
+	defer conn.Close()
+
+	req := crud.MakeUpsertObjectRequest(spaceName).
+		Object(object).
+		Operations([]crud.Operation{
+			{
+				Operator: crud.Splice,
+				Field:    "name",
+				Pos:      1,
+				Len:      1,
+				Replace:  "!!",
+			},
+		}).
+		Opts(simpleOperationOpts)
+
+	testCrudRequestPrepareData(t, conn)
+	resp, err := conn.Do(req).Get()
+	testCrudRequestCheck(t, req, resp,
+		err, 2)
+}
+
 func TestUnflattenRows_IncorrectParams(t *testing.T) {
 	invalidMetadata := []interface{}{
 		map[interface{}]interface{}{

example_test.go

@@ -439,34 +439,29 @@ func ExampleUpdateRequest() {
 
 	for i := 1111; i <= 1112; i++ {
 		conn.Do(tarantool.NewReplaceRequest(spaceNo).
-			Tuple([]interface{}{uint(i), "hello", "world"}),
+			Tuple([]interface{}{uint(i), "text", 1, 1, 1, 1, 1}),
 		).Get()
 	}
 
 	req := tarantool.NewUpdateRequest(617).
 		Key(tarantool.IntKey{1111}).
-		Operations(tarantool.NewOperations().Assign(1, "bye"))
+		Operations(tarantool.NewOperations().
+			Add(2, 1).
+			Subtract(3, 1).
+			BitwiseAnd(4, 1).
+			BitwiseOr(5, 1).
+			BitwiseXor(6, 1).
+			Splice(1, 1, 2, "!!").
+			Insert(7, "new").
+			Assign(7, "updated"))
 	resp, err := conn.Do(req).Get()
 	if err != nil {
 		fmt.Printf("error in do update request is %v", err)
 		return
 	}
 	fmt.Printf("response is %#v\n", resp.Data)
-
-	req = tarantool.NewUpdateRequest("test").
-		Index("primary").
-		Key(tarantool.IntKey{1111}).
-		Operations(tarantool.NewOperations().Assign(1, "hello"))
-	fut := conn.Do(req)
-	resp, err = fut.Get()
-	if err != nil {
-		fmt.Printf("error in do async update request is %v", err)
-		return
-	}
-	fmt.Printf("response is %#v\n", resp.Data)
 	// Output:
-	// response is []interface {}{[]interface {}{0x457, "bye", "world"}}
-	// response is []interface {}{[]interface {}{0x457, "hello", "world"}}
+	// response is []interface {}{[]interface {}{0x457, "t!!t", 2, 0, 1, 1, 0, "updated"}}
 }
 
 func ExampleUpdateRequest_spaceAndIndexNames() {

request_test.go

@@ -120,16 +120,16 @@ func assertBodyEqual(t testing.TB, reference []byte, req Request) {
 
 func getTestOps() ([]Op, *Operations) {
 	ops := []Op{
-		{"+", 1, 2},
-		{"-", 3, 4},
-		{"&", 5, 6},
-		{"|", 7, 8},
-		{"^", 9, 1},
-		{"^", 9, 1}, // The duplication is for test purposes.
-		{":", 2, 3},
-		{"!", 4, 5},
-		{"#", 6, 7},
-		{"=", 8, 9},
+		{Op: "+", Field: 1, Arg: 2},
+		{Op: "-", Field: 3, Arg: 4},
+		{Op: "&", Field: 5, Arg: 6},
+		{Op: "|", Field: 7, Arg: 8},
+		{Op: "^", Field: 9, Arg: 1},
+		{Op: "^", Field: 9, Arg: 1}, // The duplication is for test purposes.
+		{Op: ":", Field: 2, Pos: 3, Len: 1, Replace: "!!"},
+		{Op: "!", Field: 4, Arg: 5},
+		{Op: "#", Field: 6, Arg: 7},
+		{Op: "=", Field: 8, Arg: 9},
 	}
 	operations := NewOperations().
 		Add(1, 2).
@@ -138,7 +138,7 @@ func getTestOps() ([]Op, *Operations) {
 		BitwiseOr(7, 8).
 		BitwiseXor(9, 1).
 		BitwiseXor(9, 1). // The duplication is for test purposes.
-		Splice(2, 3).
+		Splice(2, 3, 1, "!!").
 		Insert(4, 5).
 		Delete(6, 7).
 		Assign(8, 9)