Introduce CBOR format support for REST payloads #169
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
git-subtree-dir: libs/tinycbor git-subtree-split: d2dd95cb8841d88d5a801e3ef9c328fd6200e7bd
With this commit CMake will add the tinycbor files to the sources to be compiled into the driver. CMake will exclude two files: - open_memstream.c that won't be compiled by MSVC (WITHOUT_OPEN_MEMSTREAM compilation flag doesn't exclude its source); - cborparser.c that will be copied (and patched) under the building folder. The patching of the later file above adds one function that allows copy-free extraction of text/byte strings.
This commit adds basic support for CBOR encapsulation, as an alternative to JSON. The format to use is connection-specific and configured in the connection string. The introduction stops short of supporting encoding/decoding of the parameters and result sets values: it will allow building a non-parameterized query and decoding the response object (either with a result-set or with an error), but without unpacking the values in the result set of the latter. Server version querying is also carried over CBOR. All communication is done either over JSON or CBOR, although the driver will carry on if it receives a JSON response to a CBOR request (or the other way around). However, driver-generated "fake" responses to catalog queries are always JSON-formatted (makes maintenance of text responses easier). The commit also enhances the support of the Elasticsearch-formatted errors (both JSON and CBOR) in that the "error" parameter will be parsed if of a map type (generally an ES/SQL error) or passed on as is, if of a string type. The previous behavior was to abandon parsing if "error" wasn't a map and present the entire error answer to the user; this wouldn't work well with a CBOR object now.
- the define is not yet used, though.
- add to the repo the files necessary to generate the legal notices and reports.
bpintea
added
>feature
edsavage
approved these changes
Aug 8, 2019
driver/connect.c
Outdated
| resp.cnt, LCSTR(&resp)); | ||
| goto err; | ||
| if (is_json) { | ||
| if (! parse_es_version_json(dbc, &rsp_body, &es_ver, &state)) { |
There was a problem hiding this comment.
nit: the return codes from parse_es_version_json and parse_es_version_cbor could be assigned to a variable, this would allow for the deduplication of the error handling and assignment of the version number to n
There was a problem hiding this comment.
Thanks. I've compacted the code here.
driver/queries.c
Outdated
| rec->name = col_name->cnt ? *col_name : EMPTY_WSTR; | ||
|
|
||
| assert(! rec->es_type); | ||
| /* lookup the DBC-cashed ES type */ |
driver/queries.c
Outdated
| } | ||
| } | ||
| if (rec->es_type) { | ||
| /* copy fileds pre-calculated at DB connect time */ |
driver/queries.c
Outdated
| n = type_wstr.cnt < wbuff_cnt ? type_wstr.cnt : wbuff_cnt; | ||
| wmemcpy(wbuff, type_wstr.str, n); | ||
| pos = n; | ||
| if (sizeof(": ") - 1 + pos < wbuff_cnt) { |
There was a problem hiding this comment.
nit: assign ": " to a named variable
There was a problem hiding this comment.
thanks, implemented the change.
| }; | ||
| const char fmt_outer_keys[] = "ON"; | ||
| const char fmt_outer_keys[] = "UN"; |
There was a problem hiding this comment.
Q: Was this change intentional?
There was a problem hiding this comment.
Yes, it changes parser's matching on key types, having it accept any ("unknown" type) objects.
driver/tinycbor.c
Outdated
| if ((res = cbor_value_skip_tag(it)) != CborNoError) { | ||
| return res; | ||
| } | ||
| /* is current key is a string, get its name */ |
| if (res != CborNoError) { | ||
| return res; | ||
| } | ||
| // TODO: binary search on ordered keys? |
- reducing code duplication on srv. version checking; - fixing a couple of comment typos; - assigning const strings to named vars.
For sure. They will be added with the next PRs. |
bpintea
added a commit
that referenced
this pull request
Aug 28, 2019
* Squashed 'libs/tinycbor/' content from commit d2dd95c git-subtree-dir: libs/tinycbor git-subtree-split: d2dd95cb8841d88d5a801e3ef9c328fd6200e7bd * add tinycbor library to the project With this commit CMake will add the tinycbor files to the sources to be compiled into the driver. CMake will exclude two files: - open_memstream.c that won't be compiled by MSVC (WITHOUT_OPEN_MEMSTREAM compilation flag doesn't exclude its source); - cborparser.c that will be copied (and patched) under the building folder. The patching of the later file above adds one function that allows copy-free extraction of text/byte strings. * introduce CBOR format support for REST payloads This commit adds basic support for CBOR encapsulation, as an alternative to JSON. The format to use is connection-specific and configured in the connection string. The introduction stops short of supporting encoding/decoding of the parameters and result sets values: it will allow building a non-parameterized query and decoding the response object (either with a result-set or with an error), but without unpacking the values in the result set of the latter. Server version querying is also carried over CBOR. All communication is done either over JSON or CBOR, although the driver will carry on if it receives a JSON response to a CBOR request (or the other way around). However, driver-generated "fake" responses to catalog queries are always JSON-formatted (makes maintenance of text responses easier). The commit also enhances the support of the Elasticsearch-formatted errors (both JSON and CBOR) in that the "error" parameter will be parsed if of a map type (generally an ES/SQL error) or passed on as is, if of a string type. The previous behavior was to abandon parsing if "error" wasn't a map and present the entire error answer to the user; this wouldn't work well with a CBOR object now. * fix a copy&paste define error - the define is not yet used, though. * add the legal files for tinycbor as 3rd party lib. - add to the repo the files necessary to generate the legal notices and reports. * addressing PR review comments - reducing code duplication on srv. version checking; - fixing a couple of comment typos; - assigning const strings to named vars. (cherry picked from commit 992f0e0)
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds basic support for CBOR encapsulation, as an alternative
to JSON.
tinycbor library is employed for parsing/encoding CBOR, at its today's
master version (latest 0.5.2 release plus a few fixes).
The format to use is connection-specific and configured in the
connection string.
The introduction stops short of supporting encoding/decoding of the
parameters and result sets values: it will allow building a
non-parameterized query and decoding the response object (either with a
result-set or with an error), but without unpacking the values in the
result set of the latter. (The rest of the functionality will be part of
subsequent PRs.)
Server version querying is also carried over CBOR.
All communication is done either over JSON or CBOR, although the driver
will carry on if it receives a JSON response to a CBOR request (or the
other way around). However, driver-generated "fake" responses to catalog
queries are always JSON-formatted (makes maintenance of text responses
easier).
The PR also enhances the support of the Elasticsearch-formatted
errors (both JSON and CBOR) in that the "error" parameter will be parsed
if of a map type (generally an ES/SQL error) or passed on as is, if of a
string type. The previous behavior was to abandon parsing if "error"
wasn't a map and present the entire error answer to the user; this
wouldn't work well with a CBOR object now.