From 9a708b5c71dfa64fd96d9488b5fcf33b6e82cead 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 | 3 +++ 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, 90 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 08048baa..5d0ce9ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Added * Space schema introspection API `crud.schema` (#380). +### Changed +* 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 3b26979a..444eabfa 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 aa947855..9cfb2375 100644 --- a/test/integration/insert_many_test.lua +++ b/test/integration/insert_many_test.lua @@ -2042,3 +2042,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], "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_objects_many', {'customers', {}}) + + t.assert_not_equals(errs, nil) + t.assert_equals(#errs, 1) + t.assert_str_contains(errs[1], "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 adcdd3e3..ce6b97d8 100644 --- a/test/integration/replace_many_test.lua +++ b/test/integration/replace_many_test.lua @@ -2052,3 +2052,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], "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_objects_many', {'customers', {}}) + + t.assert_not_equals(errs, nil) + t.assert_equals(#errs, 1) + t.assert_str_contains(errs[1], "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 76749476..9e085f03 100644 --- a/test/integration/upsert_many_test.lua +++ b/test/integration/upsert_many_test.lua @@ -2054,3 +2054,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], "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_objects_many', {'customers', {}}) + + t.assert_not_equals(errs, nil) + t.assert_equals(#errs, 1) + t.assert_str_contains(errs[1], "At least one object expected") + t.assert_equals(result, nil) +end