From 9f0e33ec8a36323d067a39f3b9db7d1237e8051d Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Thu, 5 Oct 2023 15:09:45 +0300 Subject: [PATCH] api: return error if no tuples/objects Before this patch, the behavior of `*_many` requests if empty array of tuples/objects were rather confusing. For example, due to format processing all `*_object_many` operations resulted in `nil, {}` -- error request with zero errors. `insert_many` and `replace_many` calls result in `nil, nil` -- no result, no error. `upsert_many` results in `{metadata = metadata}, nil` with no `rows` in response. Thus, for all six `*_many` calls trying to execute the request with empty array on input result in malformed response. After this patch, trying to run `*_many` request with empty array of tuples/objects will result in `nil, {err}`, similar to existing `*_many` API. Single tuple crud API already does not allow to run with no tuples/objects. Closes #377 --- CHANGELOG.md | 1 + README.md | 6 +++--- crud/insert_many.lua | 8 ++++++++ crud/replace_many.lua | 8 ++++++++ crud/upsert_many.lua | 8 ++++++++ test/integration/insert_many_test.lua | 20 ++++++++++++++++++++ test/integration/replace_many_test.lua | 20 ++++++++++++++++++++ test/integration/upsert_many_test.lua | 20 ++++++++++++++++++++ 8 files changed, 88 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b1db30a..412b8f8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. * `deps.sh` installs the `vshard` instead of the `cartridge` by default (#364). You could to specify an environment variable `CARTIRDGE_VERSION` to install the `cartridge` and run tests cases with it. +* Return explicit error for `*_many` call with no tuples/objects (#377). ### Fixed * `crud.readview` resource cleanup on garbage collect (#379). diff --git a/README.md b/README.md index 69dc24b6..a30cb870 100644 --- a/README.md +++ b/README.md @@ -288,7 +288,7 @@ local result, err = crud.insert_object_many(space_name, objects, opts) where: * `space_name` (`string`) - name of the space to insert an object -* `tuples` / `objects` (`table`) - array of tuples/objects to insert +* `tuples` / `objects` (`table`) - array of tuples/objects to insert (at least one) * `opts`: * `timeout` (`?number`) - `vshard.call` timeout and vshard master discovery timeout (in seconds), default value is 2 @@ -639,7 +639,7 @@ local result, err = crud.replace_object_many(space_name, objects, opts) where: * `space_name` (`string`) - name of the space to insert/replace an object -* `tuples` / `objects` (`table`) - array of tuples/objects to replace +* `tuples` / `objects` (`table`) - array of tuples/objects to replace (at least one) * `opts`: * `timeout` (`?number`) - `vshard.call` timeout and vshard master discovery timeout (in seconds), default value is 2 @@ -855,7 +855,7 @@ where: * `tuples_operation_data` / `objects_operation_data` (`table`) - array of tuples/objects to insert and update [operations](https://www.tarantool.io/en/doc/latest/reference/reference_lua/box_space/#box-space-update) - in format {{tuple_1, operation_1}, ..., {tuple_n, operation_n}}, + in format {{tuple_1, operation_1}, ..., {tuple_n, operation_n}} (at least one), if there is tuple with duplicate key then existing tuple will be updated with update operations * `opts`: diff --git a/crud/insert_many.lua b/crud/insert_many.lua index d7415297..255b834b 100644 --- a/crud/insert_many.lua +++ b/crud/insert_many.lua @@ -244,6 +244,10 @@ function insert_many.tuples(space_name, tuples, opts) fetch_latest_metadata = '?boolean', }) + if next(tuples) == nil then + return nil, {InsertManyError:new("At least one tuple expected")} + end + opts = opts or {} local vshard_router, err = utils.get_vshard_router_instance(opts.vshard_router) @@ -284,6 +288,10 @@ function insert_many.objects(space_name, objs, opts) fetch_latest_metadata = '?boolean', }) + if next(objs) == nil then + return nil, {InsertManyError:new("At least one object expected")} + end + opts = opts or {} local vshard_router, err = utils.get_vshard_router_instance(opts.vshard_router) diff --git a/crud/replace_many.lua b/crud/replace_many.lua index 66385502..c808e607 100644 --- a/crud/replace_many.lua +++ b/crud/replace_many.lua @@ -247,6 +247,10 @@ function replace_many.tuples(space_name, tuples, opts) fetch_latest_metadata = '?boolean', }) + if next(tuples) == nil then + return nil, {ReplaceManyError:new("At least one tuple expected")} + end + opts = opts or {} local vshard_router, err = utils.get_vshard_router_instance(opts.vshard_router) @@ -287,6 +291,10 @@ function replace_many.objects(space_name, objs, opts) fetch_latest_metadata = '?boolean', }) + if next(objs) == nil then + return nil, {ReplaceManyError:new("At least one object expected")} + end + opts = opts or {} local vshard_router, err = utils.get_vshard_router_instance(opts.vshard_router) diff --git a/crud/upsert_many.lua b/crud/upsert_many.lua index 236c4135..cc0e7aad 100644 --- a/crud/upsert_many.lua +++ b/crud/upsert_many.lua @@ -263,6 +263,10 @@ function upsert_many.tuples(space_name, tuples_operation_data, opts) fetch_latest_metadata = '?boolean', }) + if next(tuples_operation_data) == nil then + return nil, {UpsertManyError:new("At least one tuple expected")} + end + opts = opts or {} local vshard_router, err = utils.get_vshard_router_instance(opts.vshard_router) @@ -303,6 +307,10 @@ function upsert_many.objects(space_name, objs_operation_data, opts) fetch_latest_metadata = '?boolean', }) + if next(objs_operation_data) == nil then + return nil, {UpsertManyError:new("At least one object expected")} + end + opts = opts or {} local vshard_router, err = utils.get_vshard_router_instance(opts.vshard_router) diff --git a/test/integration/insert_many_test.lua b/test/integration/insert_many_test.lua index 1e62ebdb..c8a73cfb 100644 --- a/test/integration/insert_many_test.lua +++ b/test/integration/insert_many_test.lua @@ -2032,3 +2032,23 @@ pgroup.test_noreturn_opt = function(g) t.assert_equals(#errs, 3) t.assert_equals(result, nil) end + +pgroup.test_zero_tuples = function(g) + local result, errs = g.cluster.main_server.net_box:call( + 'crud.insert_many', {'customers', {}}) + + t.assert_not_equals(errs, nil) + t.assert_equals(#errs, 1) + t.assert_str_contains(errs[1].err, "At least one tuple expected") + t.assert_equals(result, nil) +end + +pgroup.test_zero_objects = function(g) + local result, errs = g.cluster.main_server.net_box:call( + 'crud.insert_object_many', {'customers', {}}) + + t.assert_not_equals(errs, nil) + t.assert_equals(#errs, 1) + t.assert_str_contains(errs[1].err, "At least one object expected") + t.assert_equals(result, nil) +end diff --git a/test/integration/replace_many_test.lua b/test/integration/replace_many_test.lua index 3968841e..6b8c75aa 100644 --- a/test/integration/replace_many_test.lua +++ b/test/integration/replace_many_test.lua @@ -2043,3 +2043,23 @@ pgroup.test_noreturn_opt = function(g) t.assert_equals(#errs, 3) t.assert_equals(result, nil) end + +pgroup.test_zero_tuples = function(g) + local result, errs = g.cluster.main_server.net_box:call( + 'crud.replace_many', {'customers', {}}) + + t.assert_not_equals(errs, nil) + t.assert_equals(#errs, 1) + t.assert_str_contains(errs[1].err, "At least one tuple expected") + t.assert_equals(result, nil) +end + +pgroup.test_zero_objects = function(g) + local result, errs = g.cluster.main_server.net_box:call( + 'crud.replace_object_many', {'customers', {}}) + + t.assert_not_equals(errs, nil) + t.assert_equals(#errs, 1) + t.assert_str_contains(errs[1].err, "At least one object expected") + t.assert_equals(result, nil) +end diff --git a/test/integration/upsert_many_test.lua b/test/integration/upsert_many_test.lua index 7a0afd1e..4e938954 100644 --- a/test/integration/upsert_many_test.lua +++ b/test/integration/upsert_many_test.lua @@ -2044,3 +2044,23 @@ pgroup.test_noreturn_opt = function(g) t.assert_equals(#errs, 3) t.assert_equals(result, nil) end + +pgroup.test_zero_tuples = function(g) + local result, errs = g.cluster.main_server.net_box:call( + 'crud.upsert_many', {'customers', {}}) + + t.assert_not_equals(errs, nil) + t.assert_equals(#errs, 1) + t.assert_str_contains(errs[1].err, "At least one tuple expected") + t.assert_equals(result, nil) +end + +pgroup.test_zero_objects = function(g) + local result, errs = g.cluster.main_server.net_box:call( + 'crud.upsert_object_many', {'customers', {}}) + + t.assert_not_equals(errs, nil) + t.assert_equals(#errs, 1) + t.assert_str_contains(errs[1].err, "At least one object expected") + t.assert_equals(result, nil) +end