From d13fe63f69c936994903550bf41992cd65d171ac Mon Sep 17 00:00:00 2001
From: Anastasia Neklepaeva <63649739+AnaNek@users.noreply.github.com>
Date: Tue, 12 Oct 2021 11:02:34 +0300
Subject: [PATCH] Fix opts table damaging by CRUD methods (#200)

If we have `opts` variable and we pass it to some crud method
(`upsert_object` for example) and then use `opts` variable again
for crud method (`get` for example) we receive an error because
`*_object` method damages `opts` variable adding
`add_space_schema_hash` parameter to `opts` variable.
---
 CHANGELOG.md                                |   4 +
 crud/common/utils.lua                       |  10 ++
 crud/insert.lua                             |   4 +-
 crud/replace.lua                            |   4 +-
 crud/upsert.lua                             |   4 +-
 test/integration/borders_test.lua           |  48 +++++++
 test/integration/len_test.lua               |  16 +++
 test/integration/pairs_test.lua             |  51 +++++++
 test/integration/select_test.lua            |  43 ++++++
 test/integration/simple_operations_test.lua | 139 ++++++++++++++++++++
 test/integration/truncate_test.lua          |  16 +++
 11 files changed, 330 insertions(+), 9 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 68c09ace..2ae5c559 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -16,6 +16,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
   versions (< 1.10.8) anymore.
 * Names of errors generated by CRUD operations have been unified.
 
+### Fixed
+
+* Damaging of opts table by CRUD methods.
+
 ### Added
 
 * Added integration with service coveralls.io.
diff --git a/crud/common/utils.lua b/crud/common/utils.lua
index 8a9ef7c9..a42fbc29 100644
--- a/crud/common/utils.lua
+++ b/crud/common/utils.lua
@@ -1,6 +1,7 @@
 local errors = require('errors')
 local ffi = require('ffi')
 local vshard = require('vshard')
+local fun = require('fun')
 
 local schema = require('crud.common.schema')
 local dev_checks = require('crud.common.dev_checks')
@@ -524,4 +525,13 @@ function utils.flatten_obj_reload(space_name, obj)
     return schema.wrap_func_reload(flatten_obj, space_name, obj)
 end
 
+-- Merge two options map.
+--
+-- `opts_a` and/or `opts_b` can be `nil`.
+--
+-- If `opts_a.foo` and `opts_b.foo` exists, prefer `opts_b.foo`.
+function utils.merge_options(opts_a, opts_b)
+    return fun.chain(opts_a or {}, opts_b or {}):tomap()
+end
+
 return utils
diff --git a/crud/insert.lua b/crud/insert.lua
index 9d4332cd..fbb0e462 100644
--- a/crud/insert.lua
+++ b/crud/insert.lua
@@ -144,10 +144,8 @@ end
 function insert.object(space_name, obj, opts)
     checks('string', 'table', '?table')
 
-    opts = opts or {}
-
     -- insert can fail if router uses outdated schema to flatten object
-    opts.add_space_schema_hash = true
+    opts = utils.merge_options(opts, {add_space_schema_hash = true})
 
     local tuple, err = utils.flatten_obj_reload(space_name, obj)
     if err ~= nil then
diff --git a/crud/replace.lua b/crud/replace.lua
index 26d57217..2069398e 100644
--- a/crud/replace.lua
+++ b/crud/replace.lua
@@ -148,10 +148,8 @@ end
 function replace.object(space_name, obj, opts)
     checks('string', 'table', '?table')
 
-    opts = opts or {}
-
     -- replace can fail if router uses outdated schema to flatten object
-    opts.add_space_schema_hash = true
+    opts = utils.merge_options(opts, {add_space_schema_hash = true})
 
     local tuple, err = utils.flatten_obj_reload(space_name, obj)
     if err ~= nil then
diff --git a/crud/upsert.lua b/crud/upsert.lua
index d91d8ba5..2ce3add9 100644
--- a/crud/upsert.lua
+++ b/crud/upsert.lua
@@ -157,10 +157,8 @@ end
 function upsert.object(space_name, obj, user_operations, opts)
     checks('string', 'table', 'table', '?table')
 
-    opts = opts or {}
-
     -- upsert can fail if router uses outdated schema to flatten object
-    opts.add_space_schema_hash = true
+    opts = utils.merge_options(opts, {add_space_schema_hash = true})
 
     local tuple, err = utils.flatten_obj_reload(space_name, obj)
     if err ~= nil then
diff --git a/test/integration/borders_test.lua b/test/integration/borders_test.lua
index 07aeff2f..3519f4af 100644
--- a/test/integration/borders_test.lua
+++ b/test/integration/borders_test.lua
@@ -311,3 +311,51 @@ pgroup.test_equal_secondary_keys = function(g)
     local objects = crud.unflatten_rows(result.rows, result.metadata)
     t.assert_equals(objects, helpers.get_objects_by_idxs(customers, {2}))
 end
+
+pgroup.test_opts_not_damaged = function(g)
+    helpers.insert_objects(g, 'customers', {
+        {
+            id = 1, name = "Elizabeth", last_name = "Jackson",
+            age = 21, city = "New York",
+        }, {
+            id = 2, name = "Mary", last_name = "Brown",
+            age = 33, city = "Los Angeles",
+        }, {
+            id = 3, name = "David", last_name = "Smith",
+            age = 12, city = "Los Angeles",
+        }, {
+            id = 4, name = "William", last_name = "White",
+            age = 8, city = "Chicago",
+        },
+    })
+
+    -- min
+    local min_opts = {timeout = 1, fields = {'name', 'age'}}
+    local new_min_opts, err = g.cluster.main_server:eval([[
+        local crud = require('crud')
+
+        local min_opts = ...
+
+        local _, err = crud.min('customers', nil, min_opts)
+
+        return min_opts, err
+    ]], {min_opts})
+
+    t.assert_equals(err, nil)
+    t.assert_equals(new_min_opts, min_opts)
+
+    -- max
+    local max_opts = {timeout = 1, fields = {'name', 'age'}}
+    local new_max_opts, err = g.cluster.main_server:eval([[
+        local crud = require('crud')
+
+        local max_opts = ...
+
+        local _, err = crud.max('customers', nil, max_opts)
+
+        return max_opts, err
+    ]], {min_opts})
+
+    t.assert_equals(err, nil)
+    t.assert_equals(new_max_opts, max_opts)
+end
diff --git a/test/integration/len_test.lua b/test/integration/len_test.lua
index 730eaccc..bd45bc14 100644
--- a/test/integration/len_test.lua
+++ b/test/integration/len_test.lua
@@ -62,3 +62,19 @@ pgroup.test_len_empty_space = function(g)
     t.assert_equals(err, nil)
     t.assert_equals(result, 0)
 end
+
+pgroup.test_opts_not_damaged = function(g)
+    local len_opts = {timeout = 1}
+    local new_len_opts, err = g.cluster.main_server:eval([[
+        local crud = require('crud')
+
+        local len_opts = ...
+
+        local _, err = crud.len('customers', len_opts)
+
+        return len_opts, err
+    ]], {len_opts})
+
+    t.assert_equals(err, nil)
+    t.assert_equals(new_len_opts, len_opts)
+end
diff --git a/test/integration/pairs_test.lua b/test/integration/pairs_test.lua
index 6f0436d4..fdb5db5c 100644
--- a/test/integration/pairs_test.lua
+++ b/test/integration/pairs_test.lua
@@ -754,3 +754,54 @@ pgroup.test_pairs_timeout = function(g)
     ]])
     t.assert_equals(objects, raw_rows)
 end
+
+pgroup.test_opts_not_damaged = function(g)
+    local customers = helpers.insert_objects(g, 'customers', {
+        {
+            id = 1, name = "Elizabeth", last_name = "Jackson",
+            age = 12, city = "Los Angeles",
+        }, {
+            id = 2, name = "Mary", last_name = "Brown",
+            age = 46, city = "London",
+        }, {
+            id = 3, name = "David", last_name = "Smith",
+            age = 33, city = "Los Angeles",
+        }, {
+            id = 4, name = "William", last_name = "White",
+            age = 46, city = "Chicago",
+        },
+    })
+
+    table.sort(customers, function(obj1, obj2) return obj1.id < obj2.id end)
+
+    local expected_customers = {
+        {id = 3, name = "David", age = 33},
+        {id = 4, name = "William", age = 46},
+    }
+
+    -- after tuple should be in `fields` format + primary key
+    local fields = {'name', 'age'}
+    local after = {"Mary", 46, 2}
+
+    local pairs_opts = {
+        timeout = 1, bucket_id = 1161,
+        batch_size = 105, first = 2, after = after,
+        fields = fields, mode = 'read', prefer_replica = false,
+        balance = false, force_map_call = false, use_tomap = true,
+    }
+    local new_pairs_opts, objects = g.cluster.main_server:eval([[
+         local crud = require('crud')
+
+         local pairs_opts = ...
+
+         local objects = {}
+         for _, object in crud.pairs('customers', nil, pairs_opts) do
+             table.insert(objects, object)
+         end
+
+         return pairs_opts, objects
+     ]], {pairs_opts})
+
+    t.assert_equals(objects, expected_customers)
+    t.assert_equals(new_pairs_opts, pairs_opts)
+end
diff --git a/test/integration/select_test.lua b/test/integration/select_test.lua
index df322e83..74192138 100644
--- a/test/integration/select_test.lua
+++ b/test/integration/select_test.lua
@@ -1538,3 +1538,46 @@ pgroup.test_select_timeout = function(g)
         {4, 1161, "William", "White", 81, "Chicago"},
     })
 end
+
+pgroup.test_opts_not_damaged = function(g)
+    local customers = helpers.insert_objects(g, 'customers', {
+        {
+            id = 1, name = "Elizabeth", last_name = "Jackson",
+            age = 12, city = "Los Angeles",
+        }, {
+            id = 2, name = "Mary", last_name = "Brown",
+            age = 46, city = "London",
+        }, {
+            id = 3, name = "David", last_name = "Smith",
+            age = 33, city = "Los Angeles",
+        }, {
+            id = 4, name = "William", last_name = "White",
+            age = 46, city = "Chicago",
+        },
+    })
+
+    table.sort(customers, function(obj1, obj2) return obj1.id < obj2.id end)
+
+    -- after tuple should be in `fields` format + primary key
+    local fields = {'name', 'age'}
+    local after = {"Mary", 46, 2}
+
+    local select_opts = {
+        timeout = 1, bucket_id = 1161,
+        batch_size = 105, first = 2, after = after,
+        fields = fields, mode = 'read', prefer_replica = false,
+        balance = false, force_map_call = false,
+    }
+    local new_select_opts, err = g.cluster.main_server:eval([[
+         local crud = require('crud')
+
+         local select_opts = ...
+
+         local _, err = crud.select('customers', nil, select_opts)
+
+         return select_opts, err
+     ]], {select_opts})
+
+    t.assert_equals(err, nil)
+    t.assert_equals(new_select_opts, select_opts)
+end
diff --git a/test/integration/simple_operations_test.lua b/test/integration/simple_operations_test.lua
index 90f908b1..8c33daa2 100644
--- a/test/integration/simple_operations_test.lua
+++ b/test/integration/simple_operations_test.lua
@@ -1007,3 +1007,142 @@ pgroup.test_partial_result_bad_input = function(g)
     t.assert_equals(result, nil)
     t.assert_str_contains(err.err, 'Space format doesn\'t contain field named "lastname"')
 end
+
+pgroup.test_opts_not_damaged = function(g)
+    -- insert
+    local insert_opts = {timeout = 1, bucket_id = 655, fields = {'name', 'age'}}
+    local new_insert_opts, err = g.cluster.main_server:eval([[
+        local crud = require('crud')
+
+        local insert_opts = ...
+
+        local _, err = crud.insert('customers', {22, box.NULL, 'Elizabeth', 24}, insert_opts)
+
+        return insert_opts, err
+    ]], {insert_opts})
+
+    t.assert_equals(err, nil)
+    t.assert_equals(new_insert_opts, insert_opts)
+
+    -- insert_object
+    local insert_opts = {timeout = 1, bucket_id = 477, fields = {'name', 'age'}}
+    local new_insert_opts, err = g.cluster.main_server:eval([[
+        local crud = require('crud')
+
+        local insert_opts = ...
+
+        local _, err = crud.insert_object('customers', {id = 1, name = 'Fedor', age = 59}, insert_opts)
+
+        return insert_opts, err
+    ]], {insert_opts})
+
+    t.assert_equals(err, nil)
+    t.assert_equals(new_insert_opts, insert_opts)
+
+    -- upsert
+    local upsert_opts = {timeout = 1, bucket_id = 907, fields = {'name', 'age'}}
+    local new_upsert_opts, err = g.cluster.main_server:eval([[
+        local crud = require('crud')
+
+        local upsert_opts = ...
+
+        local _, err = crud.upsert('customers', {33, box.NULL, 'Peter', 35}, {{'+', 'age', 1}}, upsert_opts)
+
+        return upsert_opts, err
+    ]], {upsert_opts})
+
+    t.assert_equals(err, nil)
+    t.assert_equals(new_upsert_opts, upsert_opts)
+
+    -- upsert_object
+    local upsert_opts = {timeout = 1, bucket_id = 401, fields = {'name', 'age'}}
+    local new_upsert_opts, err = g.cluster.main_server:eval([[
+        local crud = require('crud')
+
+        local upsert_opts = ...
+
+        local _, err = crud.upsert_object('customers',
+            {id = 2, name = 'Alex', age = 30}, {{'+', 'age', 1}},
+            upsert_opts)
+
+        return upsert_opts, err
+    ]], {upsert_opts})
+
+    t.assert_equals(err, nil)
+    t.assert_equals(new_upsert_opts, upsert_opts)
+
+    -- get
+    local get_opts = {timeout = 1, bucket_id = 401, fields = {'name', 'age'}}
+    local new_get_opts, err = g.cluster.main_server:eval([[
+        local crud = require('crud')
+
+        local get_opts = ...
+
+        local _, err = crud.get('customers', 2, get_opts)
+
+        return get_opts, err
+    ]], {get_opts})
+
+    t.assert_equals(err, nil)
+    t.assert_equals(new_get_opts, get_opts)
+
+    -- update
+    local update_opts = {timeout = 1, bucket_id = 401, fields = {'name', 'age'}}
+    local new_update_opts, err = g.cluster.main_server:eval([[
+        local crud = require('crud')
+
+        local update_opts = ...
+
+        local _, err = crud.update('customers', 2, {{'+', 'age', 10}}, update_opts)
+
+        return update_opts, err
+    ]], {update_opts})
+
+    t.assert_equals(err, nil)
+    t.assert_equals(new_update_opts, update_opts)
+
+    -- replace
+    local replace_opts = {timeout = 1, bucket_id = 655, fields = {'name', 'age'}}
+    local new_replace_opts, err = g.cluster.main_server:eval([[
+        local crud = require('crud')
+
+        local replace_opts = ...
+
+        local _, err = crud.replace('customers', {22, box.NULL, 'Elizabeth', 25}, replace_opts)
+
+        return replace_opts, err
+    ]], {replace_opts})
+
+    t.assert_equals(err, nil)
+    t.assert_equals(new_replace_opts, replace_opts)
+
+    -- replace_object
+    local replace_opts = {timeout = 1, bucket_id = 477, fields = {'name', 'age'}}
+    local new_replace_opts, err = g.cluster.main_server:eval([[
+        local crud = require('crud')
+
+        local replace_opts = ...
+
+        local _, err = crud.replace_object('customers', {id = 1, name = 'Fedor', age = 60}, replace_opts)
+
+        return replace_opts, err
+    ]], {replace_opts})
+
+    t.assert_equals(err, nil)
+    t.assert_equals(new_replace_opts, replace_opts)
+
+    -- delete
+    local delete_opts = {timeout = 1, bucket_id = 401, fields = {'name', 'age'}}
+    local new_delete_opts, err = g.cluster.main_server:eval([[
+        local crud = require('crud')
+
+        local delete_opts = ...
+
+        local _, err = crud.delete('customers', 2, delete_opts)
+
+        return delete_opts, err
+    ]], {delete_opts})
+
+    t.assert_equals(err, nil)
+    t.assert_equals(new_delete_opts, delete_opts)
+end
diff --git a/test/integration/truncate_test.lua b/test/integration/truncate_test.lua
index 79c17794..934048b3 100644
--- a/test/integration/truncate_test.lua
+++ b/test/integration/truncate_test.lua
@@ -73,3 +73,19 @@ pgroup.test_truncate = function(g)
     t.assert_equals(err, nil)
     t.assert_equals(#result.rows, 0)
 end
+
+pgroup.test_opts_not_damaged = function(g)
+    local truncate_opts = {timeout = 1}
+    local new_truncate_opts, err = g.cluster.main_server:eval([[
+        local crud = require('crud')
+
+        local truncate_opts = ...
+
+        local _, err = crud.truncate('customers', truncate_opts)
+
+        return truncate_opts, err
+    ]], {truncate_opts})
+
+    t.assert_equals(err, nil)
+    t.assert_equals(new_truncate_opts, truncate_opts)
+end