Skip to content

Commit

Permalink
api: fix splice update operation
Browse files Browse the repository at this point in the history
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
  • Loading branch information
DerekBum committed Nov 15, 2023
1 parent 39ec344 commit 15cb5f6
Show file tree
Hide file tree
Showing 6 changed files with 224 additions and 41 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
61 changes: 37 additions & 24 deletions client_tools.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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.
Expand All @@ -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
}
40 changes: 37 additions & 3 deletions crud/operations.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package crud

import "github.com/vmihailenco/msgpack/v5"

const (
// Add - operator for addition.
Add Operator = "+"
Expand All @@ -23,11 +25,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)
}
133 changes: 130 additions & 3 deletions crud/tarantool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
}

Expand Down Expand Up @@ -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{}{
Expand Down
22 changes: 11 additions & 11 deletions request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand All @@ -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)
Expand Down
8 changes: 8 additions & 0 deletions test_helpers/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down

0 comments on commit 15cb5f6

Please sign in to comment.