* [Tarantool-patches] [PATCH v3 0/2] add an empty body to the unprepare response
@ 2020-03-11 9:13 Chris Sosnin
2020-03-11 9:13 ` [Tarantool-patches] [PATCH v3 1/2] test: introduce iproto_request helper function Chris Sosnin
2020-03-11 9:14 ` [Tarantool-patches] [PATCH v3 2/2] iproto: add an empty body to the unprepare response Chris Sosnin
0 siblings, 2 replies; 7+ messages in thread
From: Chris Sosnin @ 2020-03-11 9:13 UTC (permalink / raw)
To: tarantool-patches
issue: https://github.com/tarantool/tarantool/issues/4769
branch: https://github.com/tarantool/tarantool/tree/ksosnin/gh-4769-unprepare-response-body
Changes in v2:
- Rewrite tests from Python to Lua.
Changes in v3:
- As suggested, I created a helper function for
sending iproto requests (which can also be used
in future iproto.test.lua) and renamed the test files
using gh-4769- pattern.
Chris Sosnin (2):
test: introduce iproto_request helper function
iproto: add an empty body to the unprepare response
src/box/iproto.cc | 19 +++---
test/box/box.lua | 22 ++++++-
.../gh-4769-unprepare-response-body.result | 62 +++++++++++++++++++
.../gh-4769-unprepare-response-body.test.lua | 30 +++++++++
4 files changed, 124 insertions(+), 9 deletions(-)
create mode 100644 test/box/gh-4769-unprepare-response-body.result
create mode 100644 test/box/gh-4769-unprepare-response-body.test.lua
--
2.21.1 (Apple Git-122.3)
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Tarantool-patches] [PATCH v3 1/2] test: introduce iproto_request helper function
2020-03-11 9:13 [Tarantool-patches] [PATCH v3 0/2] add an empty body to the unprepare response Chris Sosnin
@ 2020-03-11 9:13 ` Chris Sosnin
2020-03-11 9:14 ` [Tarantool-patches] [PATCH v3 2/2] iproto: add an empty body to the unprepare response Chris Sosnin
1 sibling, 0 replies; 7+ messages in thread
From: Chris Sosnin @ 2020-03-11 9:13 UTC (permalink / raw)
To: tarantool-patches
It is needed for performing iproto tests in Lua.
Needed for #4769
---
test/box/box.lua | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/test/box/box.lua b/test/box/box.lua
index 2a8e0e4fa..6fad07015 100644
--- a/test/box/box.lua
+++ b/test/box/box.lua
@@ -1,6 +1,8 @@
#!/usr/bin/env tarantool
os = require('os')
+local msgpack = require('msgpack')
+
box.cfg{
listen = os.getenv("LISTEN"),
memtx_memory = 107374182,
@@ -38,4 +40,22 @@ function sorted(data)
return data
end
-_G.protected_globals = {'cfg_filter', 'sorted'}
+function iproto_request(socket, query_header, query_body)
+ local header = msgpack.encode(query_header)
+ local body = msgpack.encode(query_body)
+ local size = msgpack.encode(header:len() + body:len())
+ assert(socket:write(size .. header .. body) ~= nil,
+ 'Failed to send request')
+ size = socket:read(5)
+ assert(size ~= nil, 'Failed to read response')
+ size = msgpack.decode(size)
+ local response = socket:read(size)
+ local header, header_len = msgpack.decode(response)
+ body = msgpack.decode(response:sub(header_len))
+ return {
+ ['header'] = header,
+ ['body'] = body,
+ }
+end
+
+_G.protected_globals = {'cfg_filter', 'sorted', 'iproto_request'}
--
2.21.1 (Apple Git-122.3)
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Tarantool-patches] [PATCH v3 2/2] iproto: add an empty body to the unprepare response
2020-03-11 9:13 [Tarantool-patches] [PATCH v3 0/2] add an empty body to the unprepare response Chris Sosnin
2020-03-11 9:13 ` [Tarantool-patches] [PATCH v3 1/2] test: introduce iproto_request helper function Chris Sosnin
@ 2020-03-11 9:14 ` Chris Sosnin
2020-03-15 15:31 ` Vladislav Shpilevoy
1 sibling, 1 reply; 7+ messages in thread
From: Chris Sosnin @ 2020-03-11 9:14 UTC (permalink / raw)
To: tarantool-patches
Absence of the body in the unprepare response forces users to perform
additional checks to avoid errors. Adding an empty body fixes this problem.
Closes #4769
---
src/box/iproto.cc | 19 +++---
.../gh-4769-unprepare-response-body.result | 62 +++++++++++++++++++
.../gh-4769-unprepare-response-body.test.lua | 30 +++++++++
3 files changed, 103 insertions(+), 8 deletions(-)
create mode 100644 test/box/gh-4769-unprepare-response-body.result
create mode 100644 test/box/gh-4769-unprepare-response-body.test.lua
diff --git a/src/box/iproto.cc b/src/box/iproto.cc
index f313af6ae..5a3fb5d0b 100644
--- a/src/box/iproto.cc
+++ b/src/box/iproto.cc
@@ -1780,21 +1780,24 @@ tx_process_sql(struct cmsg *m)
* become out of date during yield.
*/
out = msg->connection->tx.p_obuf;
+ if (is_unprepare) {
+ if (iproto_reply_ok(out, msg->header.sync, schema_version) != 0)
+ goto error;
+ iproto_wpos_create(&msg->wpos, out);
+ return;
+ }
struct obuf_svp header_svp;
/* Prepare memory for the iproto header. */
if (iproto_prepare_header(out, &header_svp, IPROTO_HEADER_LEN) != 0) {
port_destroy(&port);
goto error;
}
- /* Nothing to dump in case of UNPREPARE request. */
- if (!is_unprepare) {
- if (port_dump_msgpack(&port, out) != 0) {
- port_destroy(&port);
- obuf_rollback_to_svp(out, &header_svp);
- goto error;
- }
+ if (port_dump_msgpack(&port, out) != 0) {
port_destroy(&port);
+ obuf_rollback_to_svp(out, &header_svp);
+ goto error;
}
+ port_destroy(&port);
iproto_reply_sql(out, &header_svp, msg->header.sync, schema_version);
iproto_wpos_create(&msg->wpos, out);
return;
@@ -1871,7 +1874,7 @@ net_send_msg(struct cmsg *m)
}
/**
- * Complete sending an iproto error:
+ * Complete sending an iproto error:
* recycle the error object and flush output.
*/
static void
diff --git a/test/box/gh-4769-unprepare-response-body.result b/test/box/gh-4769-unprepare-response-body.result
new file mode 100644
index 000000000..a5230fad1
--- /dev/null
+++ b/test/box/gh-4769-unprepare-response-body.result
@@ -0,0 +1,62 @@
+-- test-run result file version 2
+test_run = require('test_run').new()
+ | ---
+ | ...
+test_run:cmd("setopt delimiter ';'")
+ | ---
+ | - true
+ | ...
+
+net_box = require('net.box')
+msgpack = require('msgpack')
+urilib = require('uri')
+
+IPROTO_REQUEST_TYPE = 0x00
+IPROTO_PREPARE = 13
+IPROTO_SQL_TEXT = 0x40
+IPROTO_STMT_ID = 0x43
+
+test_run:cmd("setopt delimiter ''");
+ | ---
+ | ...
+
+box.schema.user.grant('guest', 'read, write, execute', 'universe')
+ | ---
+ | ...
+uri = urilib.parse(box.cfg.listen)
+ | ---
+ | ...
+socket = net_box.establish_connection(uri.host, uri.service)
+ | ---
+ | ...
+
+header = { [IPROTO_REQUEST_TYPE] = IPROTO_PREPARE }
+ | ---
+ | ...
+body = { [IPROTO_SQL_TEXT] = 'SELECT 1' }
+ | ---
+ | ...
+response = iproto_request(socket, header, body)
+ | ---
+ | ...
+
+body = { [IPROTO_STMT_ID] = response['body'][IPROTO_STMT_ID] }
+ | ---
+ | ...
+-- Decoding of the response will fail if there's no body.
+response = iproto_request(socket, header, body)
+ | ---
+ | ...
+response.body
+ | ---
+ | - {}
+ | ...
+
+box.schema.user.revoke('guest', 'read, write, execute', 'universe')
+ | ---
+ | ...
+socket:close()
+ | ---
+ | - true
+ | ...
+
diff --git a/test/box/gh-4769-unprepare-response-body.test.lua b/test/box/gh-4769-unprepare-response-body.test.lua
new file mode 100644
index 000000000..70e41b3e5
--- /dev/null
+++ b/test/box/gh-4769-unprepare-response-body.test.lua
@@ -0,0 +1,30 @@
+test_run = require('test_run').new()
+test_run:cmd("setopt delimiter ';'")
+
+net_box = require('net.box')
+msgpack = require('msgpack')
+urilib = require('uri')
+
+IPROTO_REQUEST_TYPE = 0x00
+IPROTO_PREPARE = 13
+IPROTO_SQL_TEXT = 0x40
+IPROTO_STMT_ID = 0x43
+
+test_run:cmd("setopt delimiter ''");
+
+box.schema.user.grant('guest', 'read, write, execute', 'universe')
+uri = urilib.parse(box.cfg.listen)
+socket = net_box.establish_connection(uri.host, uri.service)
+
+header = { [IPROTO_REQUEST_TYPE] = IPROTO_PREPARE }
+body = { [IPROTO_SQL_TEXT] = 'SELECT 1' }
+response = iproto_request(socket, header, body)
+
+body = { [IPROTO_STMT_ID] = response['body'][IPROTO_STMT_ID] }
+-- Decoding of the response will fail if there's no body.
+response = iproto_request(socket, header, body)
+response.body
+
+box.schema.user.revoke('guest', 'read, write, execute', 'universe')
+socket:close()
+
--
2.21.1 (Apple Git-122.3)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Tarantool-patches] [PATCH v3 2/2] iproto: add an empty body to the unprepare response
2020-03-11 9:14 ` [Tarantool-patches] [PATCH v3 2/2] iproto: add an empty body to the unprepare response Chris Sosnin
@ 2020-03-15 15:31 ` Vladislav Shpilevoy
2020-03-16 9:02 ` Chris Sosnin
0 siblings, 1 reply; 7+ messages in thread
From: Vladislav Shpilevoy @ 2020-03-15 15:31 UTC (permalink / raw)
To: Chris Sosnin, tarantool-patches
Hi! Thanks for the patch!
See 2 comments below.
> diff --git a/src/box/iproto.cc b/src/box/iproto.cc
> index f313af6ae..5a3fb5d0b 100644
> --- a/src/box/iproto.cc
> +++ b/src/box/iproto.cc
> @@ -1871,7 +1874,7 @@ net_send_msg(struct cmsg *m)
> }
>
> /**
> - * Complete sending an iproto error:
> + * Complete sending an iproto error:
1. Seems like a stray not related change.
> * recycle the error object and flush output.
> */
> static void
> diff --git a/test/box/gh-4769-unprepare-response-body.result b/test/box/gh-4769-unprepare-response-body.result
> new file mode 100644
> index 000000000..a5230fad1
> --- /dev/null
> +++ b/test/box/gh-4769-unprepare-response-body.result
> @@ -0,0 +1,62 @@
> +-- test-run result file version 2
> +test_run = require('test_run').new()
> + | ---
> + | ...
> +test_run:cmd("setopt delimiter ';'")
2. Why? You never use the new delimiter until you
reset it back to normal.
You don't even need test_run = require('test_run').new(),
because without the needless delimiter change the
test_run object is also unused.
> + | ---
> + | - true
> + | ...
> +
> +net_box = require('net.box')
> +msgpack = require('msgpack')
> +urilib = require('uri')
> +
> +IPROTO_REQUEST_TYPE = 0x00
> +IPROTO_PREPARE = 13
> +IPROTO_SQL_TEXT = 0x40
> +IPROTO_STMT_ID = 0x43
> +
> +test_run:cmd("setopt delimiter ''");
> + | ---
> + | ...
> +
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Tarantool-patches] [PATCH v3 2/2] iproto: add an empty body to the unprepare response
2020-03-15 15:31 ` Vladislav Shpilevoy
@ 2020-03-16 9:02 ` Chris Sosnin
2020-03-16 22:01 ` Vladislav Shpilevoy
0 siblings, 1 reply; 7+ messages in thread
From: Chris Sosnin @ 2020-03-16 9:02 UTC (permalink / raw)
To: Vladislav Shpilevoy; +Cc: tarantool-patches
Hi! Thank you for the review!
> On 15 Mar 2020, at 18:31, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
>
> Hi! Thanks for the patch!
>
> See 2 comments below.
>
>> diff --git a/src/box/iproto.cc b/src/box/iproto.cc
>> index f313af6ae..5a3fb5d0b 100644
>> --- a/src/box/iproto.cc
>> +++ b/src/box/iproto.cc
>> @@ -1871,7 +1874,7 @@ net_send_msg(struct cmsg *m)
>> }
>>
>> /**
>> - * Complete sending an iproto error:
>> + * Complete sending an iproto error:
>
> 1. Seems like a stray not related change.
I didn’t notice that my editor trimmed this trailing whitespace. I’m sorry.
Reverted on branch.
>
>> * recycle the error object and flush output.
>> */
>> static void
>> diff --git a/test/box/gh-4769-unprepare-response-body.result b/test/box/gh-4769-unprepare-response-body.result
>> new file mode 100644
>> index 000000000..a5230fad1
>> --- /dev/null
>> +++ b/test/box/gh-4769-unprepare-response-body.result
>> @@ -0,0 +1,62 @@
>> +-- test-run result file version 2
>> +test_run = require('test_run').new()
>> + | ---
>> + | ...
>> +test_run:cmd("setopt delimiter ';'")
>
> 2. Why? You never use the new delimiter until you
> reset it back to normal.
>
> You don't even need test_run = require('test_run').new(),
> because without the needless delimiter change the
> test_run object is also unused.
I did this to reduce output in the .result file. Fixed:
+-- test-run result file version 2
+net_box = require('net.box')
+ | ---
+ | ...
+msgpack = require('msgpack')
+ | ---
+ | ...
+urilib = require('uri')
+ | ---
+ | ...
+
+IPROTO_REQUEST_TYPE = 0x00
+ | ---
+ | ...
+IPROTO_PREPARE = 13
+ | ---
+ | ...
+IPROTO_SQL_TEXT = 0x40
+ | ---
+ | ...
+IPROTO_STMT_ID = 0x43
+ | ---
+ | ...
+
+box.schema.user.grant('guest', 'read, write, execute', 'universe')
>
>> + | ---
>> + | - true
>> + | ...
>> +
>> +net_box = require('net.box')
>> +msgpack = require('msgpack')
>> +urilib = require('uri')
>> +
>> +IPROTO_REQUEST_TYPE = 0x00
>> +IPROTO_PREPARE = 13
>> +IPROTO_SQL_TEXT = 0x40
>> +IPROTO_STMT_ID = 0x43
>> +
>> +test_run:cmd("setopt delimiter ''");
>> + | ---
>> + | ...
>> +
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Tarantool-patches] [PATCH v3 2/2] iproto: add an empty body to the unprepare response
2020-03-16 9:02 ` Chris Sosnin
@ 2020-03-16 22:01 ` Vladislav Shpilevoy
2020-03-17 14:15 ` Nikita Pettik
0 siblings, 1 reply; 7+ messages in thread
From: Vladislav Shpilevoy @ 2020-03-16 22:01 UTC (permalink / raw)
To: Chris Sosnin; +Cc: tarantool-patches
LGTM.
On 16/03/2020 10:02, Chris Sosnin wrote:
> Hi! Thank you for the review!
>
>> On 15 Mar 2020, at 18:31, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
>>
>> Hi! Thanks for the patch!
>>
>> See 2 comments below.
>>
>>> diff --git a/src/box/iproto.cc b/src/box/iproto.cc
>>> index f313af6ae..5a3fb5d0b 100644
>>> --- a/src/box/iproto.cc
>>> +++ b/src/box/iproto.cc
>>> @@ -1871,7 +1874,7 @@ net_send_msg(struct cmsg *m)
>>> }
>>>
>>> /**
>>> - * Complete sending an iproto error:
>>> + * Complete sending an iproto error:
>>
>> 1. Seems like a stray not related change.
>
> I didn’t notice that my editor trimmed this trailing whitespace. I’m sorry.
> Reverted on branch.
>
>>
>>> * recycle the error object and flush output.
>>> */
>>> static void
>>> diff --git a/test/box/gh-4769-unprepare-response-body.result b/test/box/gh-4769-unprepare-response-body.result
>>> new file mode 100644
>>> index 000000000..a5230fad1
>>> --- /dev/null
>>> +++ b/test/box/gh-4769-unprepare-response-body.result
>>> @@ -0,0 +1,62 @@
>>> +-- test-run result file version 2
>>> +test_run = require('test_run').new()
>>> + | ---
>>> + | ...
>>> +test_run:cmd("setopt delimiter ';'")
>>
>> 2. Why? You never use the new delimiter until you
>> reset it back to normal.
>>
>> You don't even need test_run = require('test_run').new(),
>> because without the needless delimiter change the
>> test_run object is also unused.
>
> I did this to reduce output in the .result file. Fixed:
>
> +-- test-run result file version 2
> +net_box = require('net.box')
> + | ---
> + | ...
> +msgpack = require('msgpack')
> + | ---
> + | ...
> +urilib = require('uri')
> + | ---
> + | ...
> +
> +IPROTO_REQUEST_TYPE = 0x00
> + | ---
> + | ...
> +IPROTO_PREPARE = 13
> + | ---
> + | ...
> +IPROTO_SQL_TEXT = 0x40
> + | ---
> + | ...
> +IPROTO_STMT_ID = 0x43
> + | ---
> + | ...
> +
> +box.schema.user.grant('guest', 'read, write, execute', 'universe')
>
>>
>>> + | ---
>>> + | - true
>>> + | ...
>>> +
>>> +net_box = require('net.box')
>>> +msgpack = require('msgpack')
>>> +urilib = require('uri')
>>> +
>>> +IPROTO_REQUEST_TYPE = 0x00
>>> +IPROTO_PREPARE = 13
>>> +IPROTO_SQL_TEXT = 0x40
>>> +IPROTO_STMT_ID = 0x43
>>> +
>>> +test_run:cmd("setopt delimiter ''");
>>> + | ---
>>> + | ...
>>> +
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Tarantool-patches] [PATCH v3 2/2] iproto: add an empty body to the unprepare response
2020-03-16 22:01 ` Vladislav Shpilevoy
@ 2020-03-17 14:15 ` Nikita Pettik
0 siblings, 0 replies; 7+ messages in thread
From: Nikita Pettik @ 2020-03-17 14:15 UTC (permalink / raw)
To: Vladislav Shpilevoy; +Cc: tarantool-patches
On 16 Mar 23:01, Vladislav Shpilevoy wrote:
> LGTM.
>
Pushed to master and 2.3. Changelogs are updated correspondingly.
Branch is dropped.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-03-17 14:15 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-11 9:13 [Tarantool-patches] [PATCH v3 0/2] add an empty body to the unprepare response Chris Sosnin
2020-03-11 9:13 ` [Tarantool-patches] [PATCH v3 1/2] test: introduce iproto_request helper function Chris Sosnin
2020-03-11 9:14 ` [Tarantool-patches] [PATCH v3 2/2] iproto: add an empty body to the unprepare response Chris Sosnin
2020-03-15 15:31 ` Vladislav Shpilevoy
2020-03-16 9:02 ` Chris Sosnin
2020-03-16 22:01 ` Vladislav Shpilevoy
2020-03-17 14:15 ` Nikita Pettik
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox