Skip to content

Commit

Permalink
feat: handle params in request body for /query API (#25762)
Browse files Browse the repository at this point in the history 
Closes #25749

This changes the `/query` API handler so that the parameters can be passed in either the request URI or in the request body for either a `GET` or `POST` request.

Parameters can be specified in the URI, the body, or both; if they are specified in both places, those in the body will take precedent.

Error variants in the HTTP server code related to missing request parameters were updated to return `400` status.
  • Loading branch information
@hiltontj
hiltontj authored Jan 8, 2025
1 parent 94b53f9 commit dfc853d
Show file tree
Hide file tree
Showing 5 changed files with 313 additions and 34 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions influxdb3/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,9 @@ hyper.workspace = true
insta.workspace = true
pretty_assertions.workspace = true
reqwest.workspace = true
serde.workspace = true
serde_json.workspace = true
serde_urlencoded.workspace = true
test_helpers.workspace = true
tonic.workspace = true
tower.workspace = true
Expand Down
214 changes: 214 additions & 0 deletions influxdb3/tests/server/query.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use futures::StreamExt;
use hyper::StatusCode;
use influxdb3_client::Precision;
use pretty_assertions::assert_eq;
use serde::Serialize;
use serde_json::{json, Value};
use test_helpers::assert_contains;

Expand Down Expand Up @@ -1362,6 +1363,219 @@ async fn api_v1_query_data_conversion() {
}
}

#[tokio::test]
async fn api_v1_query_uri_and_body() {
let server = TestServer::spawn().await;

server
.write_lp_to_db(
"foo",
"\
cpu,host=a usage=0.9 1\n\
cpu,host=b usage=0.89 1\n\
cpu,host=c usage=0.85 1\n\
mem,host=a usage=0.5 2\n\
mem,host=b usage=0.6 2\n\
mem,host=c usage=0.7 2\
",
Precision::Second,
)
.await
.unwrap();

#[derive(Debug, Serialize)]
struct Params<'a> {
#[serde(rename = "q")]
query: Option<&'a str>,
db: Option<&'a str>,
}

struct TestCase<'a> {
description: &'a str,
uri: Option<Params<'a>>,
body: Option<Params<'a>>,
expected_status: StatusCode,
expected_body: Option<Value>,
}

let test_cases = [
TestCase {
description: "query and db in uri",
uri: Some(Params {
query: Some("SELECT * FROM cpu"),
db: Some("foo"),
}),
body: None,
expected_status: StatusCode::OK,
expected_body: Some(json!({
"results": [
{
"series": [
{
"columns": [
"time",
"host",
"usage"
],
"name": "cpu",
"values": [
[
"1970-01-01T00:00:01Z",
"a",
0.9
],
[
"1970-01-01T00:00:01Z",
"b",
0.89
],
[
"1970-01-01T00:00:01Z",
"c",
0.85
]
]
}
],
"statement_id": 0
}
]
})),
},
TestCase {
description: "query in uri, db in body",
uri: Some(Params {
query: Some("SELECT * FROM cpu"),
db: None,
}),
body: Some(Params {
query: None,
db: Some("foo"),
}),
expected_status: StatusCode::OK,
// don't care about the response:
expected_body: None,
},
TestCase {
description: "query in uri, db in uri overwritten by db in body",
uri: Some(Params {
query: Some("SELECT * FROM cpu"),
db: Some("not_a_valid_db"),
}),
body: Some(Params {
query: None,
db: Some("foo"),
}),
expected_status: StatusCode::OK,
expected_body: None,
},
TestCase {
description: "query in uri, db in uri overwritten by db in body, db not valid",
uri: Some(Params {
query: Some("SELECT * FROM cpu"),
db: Some("foo"),
}),
body: Some(Params {
query: None,
db: Some("not_a_valid_db"),
}),
// db does not exist:
expected_status: StatusCode::NOT_FOUND,
expected_body: None,
},
TestCase {
description: "db in uri, query in body",
uri: Some(Params {
query: None,
db: Some("foo"),
}),
body: Some(Params {
query: Some("SELECT * FROM mem"),
db: None,
}),
expected_status: StatusCode::OK,
expected_body: Some(json!({
"results": [
{
"series": [
{
"columns": [
"time",
"host",
"usage"
],
"name": "mem",
"values": [
[
"1970-01-01T00:00:02Z",
"a",
0.5
],
[
"1970-01-01T00:00:02Z",
"b",
0.6
],
[
"1970-01-01T00:00:02Z",
"c",
0.7
]
]
}
],
"statement_id": 0
}
]
})),
},
TestCase {
description: "no query specified",
uri: Some(Params {
query: None,
db: Some("foo"),
}),
body: Some(Params {
query: None,
db: None,
}),
expected_status: StatusCode::BAD_REQUEST,
expected_body: None,
},
];

for t in test_cases {
let url = format!("{base}/query", base = server.client_addr());
// test both GET and POST:
for mut req in [server.http_client.get(&url), server.http_client.post(&url)] {
println!("test: {desc}", desc = t.description);
if let Some(ref uri) = t.uri {
req = req.query(uri);
}
if let Some(ref body) = t.body {
req = req.body(serde_urlencoded::to_string(body).expect("serialize body"));
}
let resp = req.send().await.expect("send request");
let status = resp.status();
assert_eq!(
t.expected_status, status,
"status code did not match expectation"
);
if let Some(ref expected_body) = t.expected_body {
let actual = resp.json::<Value>().await.expect("parse JSON body");
if expected_body != &actual {
// use a panic so we can format the output for copy/paste more easily:
panic!(
"JSON body did not match expectation,\n\
expected:\n{expected_body:#}\n\
actual:\n{actual:#}"
);
}
}
}
}
}

#[tokio::test]
async fn api_v3_query_sql_meta_cache() {
let server = TestServer::spawn().await;
Expand Down
13 changes: 12 additions & 1 deletion influxdb3_server/src/http.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,10 @@ pub enum Error {
#[error("missing query parameters 'db' and 'q'")]
MissingQueryParams,

/// Missing the `q` parameter in the v1 /query API
#[error("missing query parameter 'q'")]
MissingQueryV1Params,

/// MIssing parameters for write
#[error("missing query parameter 'db'")]
MissingWriteParams,
Expand Down Expand Up @@ -408,6 +412,13 @@ impl Error {
.status(StatusCode::BAD_REQUEST)
.body(Body::from(self.to_string()))
.unwrap(),
Self::MissingQueryParams
| Self::MissingQueryV1Params
| Self::MissingWriteParams
| Self::MissingDeleteDatabaseParams => Response::builder()
.status(StatusCode::BAD_REQUEST)
.body(Body::from(self.to_string()))
.unwrap(),
_ => {
let body = Body::from(self.to_string());
Response::builder()
Expand Down Expand Up @@ -1697,7 +1708,7 @@ pub(crate) async fn route_request<T: TimeProvider>(
(Method::GET | Method::POST, "/api/v3/query_influxql") => {
http_server.query_influxql(req).await
}
(Method::GET, "/query") => http_server.v1_query(req).await,
(Method::GET | Method::POST, "/query") => http_server.v1_query(req).await,
(Method::GET, "/health" | "/api/v1/health") => http_server.health(),
(Method::GET | Method::POST, "/ping") => http_server.ping(),
(Method::GET, "/metrics") => http_server.handle_metrics(),
Expand Down
Loading

0 comments on commit dfc853d

Please sign in to comment.