Skip to content

Commit c3ba5b5

Browse files
DifferentialOrangeoleg-jukovec
authored andcommitted
crud: more careful response process
In go-tarantool, crud API result decoding assumes that there are always two response values, except for truncate. Such approach is based on actual experience: success result is always `return res, nil`, not `return res`. But this is not a feature guaranteed by crud module API, just a current implementation quirk [1] (since in Lua function result process there is no any difference). See also similar patch in tarantool/python [2]. 1. https://github.com/tarantool/crud/blob/53457477974fed42351cbd87f566d11e9f7e39bb/crud/common/schema.lua#L88 2. tarantool/tarantool-python@a4b734a
1 parent a8f320a commit c3ba5b5

File tree

2 files changed

+67
-55
lines changed

2 files changed

+67
-55
lines changed

crud/result.go

Lines changed: 54 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,8 @@ func (r *Result) DecodeMsgpack(d *msgpack.Decoder) error {
7575
return err
7676
}
7777

78-
if arrLen < 2 {
79-
return fmt.Errorf("array len doesn't match: %d", arrLen)
78+
if arrLen == 0 {
79+
return fmt.Errorf("unexpected empty response array")
8080
}
8181

8282
l, err := d.DecodeMapLen()
@@ -130,27 +130,32 @@ func (r *Result) DecodeMsgpack(d *msgpack.Decoder) error {
130130
}
131131
}
132132

133-
code, err := d.PeekCode()
134-
if err != nil {
135-
return err
136-
}
137-
138-
var retErr error
139-
if msgpackIsArray(code) {
140-
crudErr := newErrorMany(r.rowType)
141-
if err := d.Decode(&crudErr); err != nil {
142-
return err
143-
}
144-
retErr = *crudErr
145-
} else if code != msgpcode.Nil {
146-
crudErr := newError(r.rowType)
147-
if err := d.Decode(&crudErr); err != nil {
133+
if arrLen > 1 {
134+
code, err := d.PeekCode()
135+
if err != nil {
148136
return err
149137
}
150-
retErr = *crudErr
151-
} else {
152-
if err := d.DecodeNil(); err != nil {
153-
return err
138+
139+
if msgpackIsArray(code) {
140+
crudErr := newErrorMany(r.rowType)
141+
if err := d.Decode(&crudErr); err != nil {
142+
return err
143+
}
144+
if crudErr != nil {
145+
return *crudErr
146+
}
147+
} else if code != msgpcode.Nil {
148+
crudErr := newError(r.rowType)
149+
if err := d.Decode(&crudErr); err != nil {
150+
return err
151+
}
152+
if crudErr != nil {
153+
return *crudErr
154+
}
155+
} else {
156+
if err := d.DecodeNil(); err != nil {
157+
return err
158+
}
154159
}
155160
}
156161

@@ -160,7 +165,7 @@ func (r *Result) DecodeMsgpack(d *msgpack.Decoder) error {
160165
}
161166
}
162167

163-
return retErr
168+
return nil
164169
}
165170

166171
// NumberResult describes CRUD result as an object containing number.
@@ -175,18 +180,24 @@ func (r *NumberResult) DecodeMsgpack(d *msgpack.Decoder) error {
175180
return err
176181
}
177182

178-
if arrLen < 2 {
179-
return fmt.Errorf("array len doesn't match: %d", arrLen)
183+
if arrLen == 0 {
184+
return fmt.Errorf("unexpected empty response array")
180185
}
181186

182187
if r.Value, err = d.DecodeUint64(); err != nil {
183188
return err
184189
}
185190

186-
var crudErr *Error = nil
191+
if arrLen > 1 {
192+
var crudErr *Error = nil
187193

188-
if err := d.Decode(&crudErr); err != nil {
189-
return err
194+
if err := d.Decode(&crudErr); err != nil {
195+
return err
196+
}
197+
198+
if crudErr != nil {
199+
return crudErr
200+
}
190201
}
191202

192203
for i := 2; i < arrLen; i++ {
@@ -195,10 +206,6 @@ func (r *NumberResult) DecodeMsgpack(d *msgpack.Decoder) error {
195206
}
196207
}
197208

198-
if crudErr != nil {
199-
return crudErr
200-
}
201-
202209
return nil
203210
}
204211

@@ -213,26 +220,31 @@ func (r *BoolResult) DecodeMsgpack(d *msgpack.Decoder) error {
213220
if err != nil {
214221
return err
215222
}
216-
if arrLen < 2 {
217-
if r.Value, err = d.DecodeBool(); err != nil {
218-
return err
219-
}
220223

221-
return nil
224+
if arrLen == 0 {
225+
return fmt.Errorf("unexpected empty response array")
222226
}
223227

224-
if _, err = d.DecodeInterface(); err != nil {
228+
if r.Value, err = d.DecodeBool(); err != nil {
225229
return err
226230
}
227231

228-
var crudErr *Error = nil
232+
if arrLen > 1 {
233+
var crudErr *Error = nil
229234

230-
if err := d.Decode(&crudErr); err != nil {
231-
return err
235+
if err := d.Decode(&crudErr); err != nil {
236+
return err
237+
}
238+
239+
if crudErr != nil {
240+
return crudErr
241+
}
232242
}
233243

234-
if crudErr != nil {
235-
return crudErr
244+
for i := 2; i < arrLen; i++ {
245+
if err := d.Skip(); err != nil {
246+
return err
247+
}
236248
}
237249

238250
return nil

crud/tarantool_test.go

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -254,71 +254,71 @@ var testGenerateDataCases = []struct {
254254
}{
255255
{
256256
"Insert",
257-
2,
257+
1,
258258
1,
259259
crud.MakeInsertRequest(spaceName).
260260
Tuple(tuple).
261261
Opts(simpleOperationOpts),
262262
},
263263
{
264264
"InsertObject",
265-
2,
265+
1,
266266
1,
267267
crud.MakeInsertObjectRequest(spaceName).
268268
Object(object).
269269
Opts(simpleOperationObjectOpts),
270270
},
271271
{
272272
"InsertMany",
273-
2,
273+
1,
274274
10,
275275
crud.MakeInsertManyRequest(spaceName).
276276
Tuples(tuples).
277277
Opts(opManyOpts),
278278
},
279279
{
280280
"InsertObjectMany",
281-
2,
281+
1,
282282
10,
283283
crud.MakeInsertObjectManyRequest(spaceName).
284284
Objects(objects).
285285
Opts(opObjManyOpts),
286286
},
287287
{
288288
"Replace",
289-
2,
289+
1,
290290
1,
291291
crud.MakeReplaceRequest(spaceName).
292292
Tuple(tuple).
293293
Opts(simpleOperationOpts),
294294
},
295295
{
296296
"ReplaceObject",
297-
2,
297+
1,
298298
1,
299299
crud.MakeReplaceObjectRequest(spaceName).
300300
Object(object).
301301
Opts(simpleOperationObjectOpts),
302302
},
303303
{
304304
"ReplaceMany",
305-
2,
305+
1,
306306
10,
307307
crud.MakeReplaceManyRequest(spaceName).
308308
Tuples(tuples).
309309
Opts(opManyOpts),
310310
},
311311
{
312312
"ReplaceObjectMany",
313-
2,
313+
1,
314314
10,
315315
crud.MakeReplaceObjectManyRequest(spaceName).
316316
Objects(objects).
317317
Opts(opObjManyOpts),
318318
},
319319
{
320320
"Upsert",
321-
2,
321+
1,
322322
1,
323323
crud.MakeUpsertRequest(spaceName).
324324
Tuple(tuple).
@@ -327,7 +327,7 @@ var testGenerateDataCases = []struct {
327327
},
328328
{
329329
"UpsertObject",
330-
2,
330+
1,
331331
1,
332332
crud.MakeUpsertObjectRequest(spaceName).
333333
Object(object).
@@ -336,15 +336,15 @@ var testGenerateDataCases = []struct {
336336
},
337337
{
338338
"UpsertMany",
339-
2,
339+
1,
340340
10,
341341
crud.MakeUpsertManyRequest(spaceName).
342342
TuplesOperationsData(tuplesOperationsData).
343343
Opts(opManyOpts),
344344
},
345345
{
346346
"UpsertObjectMany",
347-
2,
347+
1,
348348
10,
349349
crud.MakeUpsertObjectManyRequest(spaceName).
350350
ObjectsOperationsData(objectsOperationData).
@@ -475,7 +475,7 @@ func testCrudRequestCheck(t *testing.T, req tarantool.Request,
475475

476476
// resp.Data[0] - CRUD res.
477477
// resp.Data[1] - CRUD err.
478-
if expectedLen >= 2 {
478+
if expectedLen >= 2 && resp.Data[1] != nil {
479479
if crudErr, err := getCrudError(req, resp.Data[1]); err != nil {
480480
t.Fatalf("Failed to get CRUD error: %#v", err)
481481
} else if crudErr != nil {

0 commit comments

Comments
 (0)