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.

Closes #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 (#348). It now accepts 5 arguments as it should be.
 
 ## [1.12.0] - 2023-06-07
 

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,7 @@
 package crud
 
+import "github.com/vmihailenco/msgpack/v5"
+
 const (
 	// Add - operator for addition.
 	Add Operator = "+"
@@ -23,11 +25,42 @@ 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      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,10 +40,62 @@ 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.Assign,
-		Field:    "name",
-		Value:    "bye",
+		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,
 	},
 }
 
@@ -532,6 +584,81 @@ func TestCrudProcessData(t *testing.T) {
 	}
 }
 
+func TestCrudUpdateSplice(t *testing.T) {
+	test_helpers.SkipIfCrudSpliceUnsupported(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.SkipIfCrudSpliceUnsupported(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.SkipIfCrudSpliceUnsupported(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{}{

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)

test_helpers/utils.go

@@ -200,6 +200,14 @@ func SkipIfWatchOnceUnsupported(t *testing.T) {
 	SkipIfFeatureUnsupported(t, "watch once", 3, 0, 0)
 }
 
+// SkipIfCrudSpliceUnsupported skips test run if Tarantool without support of
+// crud splice update operation is used.
+func SkipIfCrudSpliceUnsupported(t *testing.T) {
+	t.Helper()
+
+	SkipIfFeatureUnsupported(t, "crud update splice", 2, 0, 0)
+}
+
 // CheckEqualBoxErrors checks equivalence of tarantool.BoxError objects.
 //
 // Tarantool errors are not comparable by nature: