A copy of HTTP client API for TS (#1371)

[hmmr]

This is the same as #1371 plus one additional commit to fix one dialyzer issue. That fix would be merged from lehoff-b#3 into Torben's branch.

[jonmeredith]

Moving final reviews here.

Hit another problem with key listing.

bin/riak-shell
Erlang R16B02_basho10 (erts-5.10.3) [source] [64-bit] [smp:4:4] [async-threads:10] [kernel-poll:false] [frame-pointer]

version "riak_shell 0.9/sql 1.3", use 'quit;' or 'q;' to exit or 'help;' for helpConnected...
riak-shell(1)>CREATE TABLE t2 (ts TIMESTAMP NOT NULL, PRIMARY KEY((quantum(ts, 1, d)),ts));  
riak-shell(2)>INSERT INTO t2 VALUES (1),(2),(3);                    
riak-shell(3)>SELECT * FROM t2 WHERE ts > 0 AND ts< 10 
->;           
+--+          
|ts|
+--+
|1 |
|2 |
|3 |
+--+

Followed by

curl -v -v -v  http://localhost:8098/ts/v1/tables/t2/list_keys
*   Trying 127.0.0.1...
* Connected to localhost (127.0.0.1) port 8098 (#0)
> GET /ts/v1/tables/t2/list_keys HTTP/1.1
> Host: localhost:8098
> User-Agent: curl/7.43.0
> Accept: */*
> 
< HTTP/1.1 200 OK
< Vary: Accept-Encoding
< Transfer-Encoding: chunked
< Server: MochiWeb/1.1 WebMachine/1.10.8 (that head fake, tho)
< Date: Wed, 06 Apr 2016 17:56:04 GMT
< Content-Type: text/plain
< 
* Illegal or missing hexadecimal sequence in chunked-encoding
* Closing connection 0

[jonmeredith]

Also, URLs have trailing /, they work when retrieving (this is a different table).

curl -v -v -v  http://localhost:8098/ts/v1/tables/t/list_keys
*   Trying 127.0.0.1...
* Connected to localhost (127.0.0.1) port 8098 (#0)
> GET /ts/v1/tables/t/list_keys HTTP/1.1
> Host: localhost:8098
> User-Agent: curl/7.43.0
> Accept: */*
> 
< HTTP/1.1 200 OK
< Vary: Accept-Encoding
< Transfer-Encoding: chunked
< Server: MochiWeb/1.1 WebMachine/1.10.8 (that head fake, tho)
< Date: Wed, 06 Apr 2016 18:02:10 GMT
< Content-Type: text/plain
< 
http://127.0.0.1:8098/ts/v1/tables/t/keys/f/f/s/s/ts/789/
http://127.0.0.1:8098/ts/v1/tables/t/keys/f/f/s/s/ts/456/
http://127.0.0.1:8098/ts/v1/tables/t/keys/f/f/s/s/ts/123/

[jonmeredith]

Here's the rest of the functional review

Required

• Streaming response failure for listkey.

>CREATE TABLE t (ts TIMESTAMP NOT NULL, PRIMARY KEY((ts),ts));
riak-shell(6)>DESCRIBE t;
+------+---------+-------+-----------+---------+
|Column|  Type   |Is Null|Primary Key|Local Key|
+------+---------+-------+-----------+---------+
|  ts  |timestamp| false |     1     |    1    |
+------+---------+-------+-----------+---------+

riak-shell(7)>INSERT INTO t VALUES (1);
riak-shell(10)>SELECT * FROM t WHERE ts = 1;
+--+
|ts|
+--+
|1 |
+--+

Retrieval causes problems.

 curl -v -v -s  http://localhost:8098/ts/v1/tables/t2/list_keys

*   Trying 127.0.0.1...
* Connected to localhost (127.0.0.1) port 8098 (#0)
> GET /ts/v1/tables/t2/list_keys HTTP/1.1
> Host: localhost:8098
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 200 OK
< Vary: Accept-Encoding
< Transfer-Encoding: chunked
< Server: MochiWeb/1.1 WebMachine/1.10.8 (that head fake, tho)
< Date: Wed, 06 Apr 2016 21:59:08 GMT
< Content-Type: text/plain
<
* Illegal or missing hexadecimal sequence in chunked-encoding
* Closing connection 0

• List keys does not correctly escape URLs

If this data is inserted

riak-shell(1)>INSERT INTO t(f,s,ts) VALUES ('f/g space :&?','s',2);
riak-shell(2)>SELECT * FROM t WHERE f = 'f/g space :&?' AND s = 's' AND ts > 1 AND ts < 3;
+-------------+-+--+
|      f      |s|ts|
+-------------+-+--+
|f/g space :&?|s|2 |
+-------------+-+--+

It produces this entry in the list keys, not URL encoded.

http://127.0.0.1:8098/ts/v1/tables/t/keys/f/f/g space :&?/s/s/ts/2/

Issuing the get fails as you would expected with bad request.

 curl -v  'http://127.0.0.1:8098/ts/v1/tables/t/keys/f/f/g space :&?/s/s/ts/2/'
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 8098 (#0)
> GET /ts/v1/tables/t/keys/f/f/g space :&?/s/s/ts/2/ HTTP/1.1
> Host: 127.0.0.1:8098
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 400 Bad Request
< Server: MochiWeb/1.0 (Any of you quaids got a smint?)
< Date: Wed, 06 Apr 2016 21:44:23 GMT
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact

Thanks to lovely webmachine, the URL encoded version works fine

curl 'http://127.0.0.1:8098/ts/v1/tables/t/keys/f/f%2Fg%20space%20%3A%26%3F/s/s/ts/2/'
{"f":"f/g space :&?","s":"s","ts":2}

Future
* Fix to security capabilities.
* Undefined fields are ignored by HTTP API, but forbidden by SQL shell

riak-shell(14)>INSERT INTO t(f,s,ts,extra) VALUES ('f','s',999,'x');
Error (1018): undefined fields: extra

vs

curl -X POST http://127.0.0.1:8098/ts/v1/tables/t/keys --data "{"f":"f","s":"s","ts":123,"extra":1}"; echo
{"success":true}

* Registers interesting dispatch table - api_version is not used in timeseries at all, and no difference in the URL so just increases dispatch timing.

{["ts",api_version,"tables",table,"list_keys"], riak_kv_wm_timeseries_listkeys,
[{api_version,3},{prefix,"riak"},{riak,local}]},
{["ts",api_version,"tables",table,"keys",''], riak_kv_wm_timeseries,
[{api_version,3},{prefix,"riak"},{riak,local}]},
{["ts",api_version,"query"], riak_kv_wm_timeseries_query,
[{api_version,3},{prefix,"riak"},{riak,local}]},
{["ts",api_version,"tables",table,"list_keys"], riak_kv_wm_timeseries_listkeys,
[{bucket_type,<<"default">>},
{api_version,2},
{prefix,"riak"},
{riak,local}]},
{["ts",api_version,"tables",table,"keys",''], riak_kv_wm_timeseries,
[{bucket_type,<<"default">>},
{api_version,2},
{prefix,"riak"},
{riak,local}]},
{["ts",api_version,"query"], riak_kv_wm_timeseries_query,
[{bucket_type,<<"default">>},
{api_version,2},
{prefix,"riak"},
{riak,local}]},

* Poor error message on incorrect timeout data type

curl -X POST -H 'Content-Type: text/plain' 'http://127.0.0.1:8098/ts/v1/tables/t/keys?timeout=a' --data "{"f":"f","s":"s","ts":123,"extra":1}"
{"error":"parameter error: [116,105,109,101,111,117,116,32,110,111,116,32,97,110,32,105,\n 110,116,101,103,101,114,32,118,97,108,117,101,58,32,"a"]"}

which I believe translates to

io:format("~s\n" ,[[116,105,109,101,111,117,116,32,110,111,116,32,97,110,32,105,110,116,101,103,101,114,32,118,97,108,117,101,58,32,""a""]]).
timeout not an integer value: "a"

Perhaps a problem in Webmachine, I didn't chase it down. Perhaps wrapping those `io_lib:format` with a `lists:flatten`.

* Unexpected parameters cause a similar message

curl -X POST -H 'Content-Type: text/plain' 'http://127.0.0.1:8098/ts/v1/tables/t/keys?badparam=a' --data "{"f":"f","s":"s","ts":123,"extra":1}"
{"error":"parameter error: [105,110,99,111,114,114,101,99,116,32,112,97,114,97,109,116,\n 101,114,115,58,32,\n [91,[[123,["\"badparam\"",44,"\"a\""],125]],93]]"}

# Optional
* Only riak_kv_wm_timeseries_query had init/service_available updated to use the new method discussed.

[hmmr]

Fixed all recently cropped up issues, and updated the test for it (basho/riak_test#1037, with complementary checks using curl in https://gist.github.com/hmmr/94abdb4d648a44009e93).

Because of #1378, there is a massive manual merge ahead.

[jonmeredith]

Encoding of list key URLs fixed.

The first issue still seems to be a problem

riak-shell(13)>CREATE TABLE t7(ts TIMESTAMP NOT NULL, PRIMARY KEY ((quantum(ts,1,d)),ts));                                                
riak-shell(14)>INSERT INTO t7 VALUES (1),(2),(3);
riak-shell(15)>SELECT * FROM t7 WHERE ts > 0 AND ts < 10;
+--+          
|ts|
+--+
|1 |
|2 |
|3 |
+--+

But

curl  --trace - -s  http://localhost:8098/ts/v1/tables/t7/list_keys 
== Info:   Trying 127.0.0.1...
== Info: Connected to localhost (127.0.0.1) port 8098 (#0)
=> Send header, 103 bytes (0x67)
0000: 47 45 54 20 2f 74 73 2f 76 31 2f 74 61 62 6c 65 GET /ts/v1/table
0010: 73 2f 74 37 2f 6c 69 73 74 5f 6b 65 79 73 20 48 s/t7/list_keys H
0020: 54 54 50 2f 31 2e 31 0d 0a 48 6f 73 74 3a 20 6c TTP/1.1..Host: l
0030: 6f 63 61 6c 68 6f 73 74 3a 38 30 39 38 0d 0a 55 ocalhost:8098..U
0040: 73 65 72 2d 41 67 65 6e 74 3a 20 63 75 72 6c 2f ser-Agent: curl/
0050: 37 2e 34 33 2e 30 0d 0a 41 63 63 65 70 74 3a 20 7.43.0..Accept: 
0060: 2a 2f 2a 0d 0a 0d 0a                            */*....
<= Recv header, 17 bytes (0x11)
0000: 48 54 54 50 2f 31 2e 31 20 32 30 30 20 4f 4b 0d HTTP/1.1 200 OK.
0010: 0a                                              .
<= Recv header, 23 bytes (0x17)
0000: 56 61 72 79 3a 20 41 63 63 65 70 74 2d 45 6e 63 Vary: Accept-Enc
0010: 6f 64 69 6e 67 0d 0a                            oding..
<= Recv header, 28 bytes (0x1c)
0000: 54 72 61 6e 73 66 65 72 2d 45 6e 63 6f 64 69 6e Transfer-Encodin
0010: 67 3a 20 63 68 75 6e 6b 65 64 0d 0a             g: chunked..
<= Recv header, 62 bytes (0x3e)
0000: 53 65 72 76 65 72 3a 20 4d 6f 63 68 69 57 65 62 Server: MochiWeb
0010: 2f 31 2e 31 20 57 65 62 4d 61 63 68 69 6e 65 2f /1.1 WebMachine/
0020: 31 2e 31 30 2e 38 20 28 74 68 61 74 20 68 65 61 1.10.8 (that hea
0030: 64 20 66 61 6b 65 2c 20 74 68 6f 29 0d 0a       d fake, tho)..
<= Recv header, 37 bytes (0x25)
0000: 44 61 74 65 3a 20 46 72 69 2c 20 30 38 20 41 70 Date: Fri, 08 Ap
0010: 72 20 32 30 31 36 20 32 30 3a 34 36 3a 32 38 20 r 2016 20:46:28 
0020: 47 4d 54 0d 0a                                  GMT..
<= Recv header, 26 bytes (0x1a)
0000: 43 6f 6e 74 65 6e 74 2d 54 79 70 65 3a 20 74 65 Content-Type: te
0010: 78 74 2f 70 6c 61 69 6e 0d 0a                   xt/plain..
<= Recv header, 2 bytes (0x2)
0000: 0d 0a                                           ..
<= Recv data, 214 bytes (0xd6)
0000: 48 54 54 50 2f 31 2e 31 20 35 30 30 20 49 6e 74 HTTP/1.1 500 Int
0010: 65 72 6e 61 6c 20 53 65 72 76 65 72 20 45 72 72 ernal Server Err
0020: 6f 72 0d 0a 56 61 72 79 3a 20 41 63 63 65 70 74 or..Vary: Accept
0030: 2d 45 6e 63 6f 64 69 6e 67 0d 0a 54 72 61 6e 73 -Encoding..Trans
0040: 66 65 72 2d 45 6e 63 6f 64 69 6e 67 3a 20 63 68 fer-Encoding: ch
0050: 75 6e 6b 65 64 0d 0a 53 65 72 76 65 72 3a 20 4d unked..Server: M
0060: 6f 63 68 69 57 65 62 2f 31 2e 31 20 57 65 62 4d ochiWeb/1.1 WebM
0070: 61 63 68 69 6e 65 2f 31 2e 31 30 2e 38 20 28 74 achine/1.10.8 (t
0080: 68 61 74 20 68 65 61 64 20 66 61 6b 65 2c 20 74 hat head fake, t
0090: 68 6f 29 0d 0a 44 61 74 65 3a 20 46 72 69 2c 20 ho)..Date: Fri, 
00a0: 30 38 20 41 70 72 20 32 30 31 36 20 32 30 3a 34 08 Apr 2016 20:4
00b0: 36 3a 32 38 20 47 4d 54 0d 0a 43 6f 6e 74 65 6e 6:28 GMT..Conten
00c0: 74 2d 54 79 70 65 3a 20 74 65 78 74 2f 70 6c 61 t-Type: text/pla
00d0: 69 6e 0d 0a 0d 0a                               in....
== Info: Illegal or missing hexadecimal sequence in chunked-encoding
== Info: Closing connection 0

Note the info message back from curl.

Illegal or missing hexadecimal sequence in chunked-encoding

I've only seen this when the number or primary keys are short. No errors in the logs that I could find.

[jonmeredith]

Parameter and timeout issues both fixed.

[hmmr]

There was a regression in riak_ttb_codec:encode/1, fixed in basho/riak_pb#185.

[macintux]

@hmmr can you please separate the KV bits that correspond to basho/riak-erlang-client#227 (RIAK-2316) and basho/riak_pb#190 (RIAK-2492) into a dedicated KV PR? It's quite confusing to review the coverage relocation intertwingled with the HTTP work.

That would also mean basho/riak_test#1037 would need to be split into HTTP tests vs coverage test changes.

Thanks. That would be very, very helpful for Brett and I reviewing this massive set of changes.

[hmmr]

This big PR is now split into a series of #1396, #1397 (comes together with basho/riak_pb#190 (RIAK-2492) and an updated basho/riak-erlang-client#277) and #1398.