Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: enable POST requests batching with dl. #3140

Merged
merged 47 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
b177fcc
- impl body batching
laststylebender14 Nov 23, 2024
9daf410
- basic integration test
laststylebender14 Nov 23, 2024
dee4a2c
- define bodyKey
laststylebender14 Nov 23, 2024
8727152
- add validation back.
laststylebender14 Nov 23, 2024
3768ec3
- introduce body key.
laststylebender14 Nov 23, 2024
46f40a1
- integration tests defined in github issue.
laststylebender14 Nov 23, 2024
b9e1b45
- lint changes
laststylebender14 Nov 23, 2024
a765d1b
- skip test case
laststylebender14 Nov 23, 2024
dd6be22
- snap update: now group by supported on POST request too.
laststylebender14 Nov 23, 2024
7c2b13e
- lint changes
laststylebender14 Nov 23, 2024
ff7e464
- reduce clonning
laststylebender14 Nov 23, 2024
5cb3aff
- introduce new feature flag to avoid sorting of bodies in production.
laststylebender14 Nov 23, 2024
b7f8b17
- lint changes
laststylebender14 Nov 23, 2024
34de904
- do ser manually
laststylebender14 Nov 23, 2024
1d17e4c
- remove white space.
laststylebender14 Nov 23, 2024
691aa8e
- add validation check
laststylebender14 Nov 23, 2024
9414b25
- add todo's
laststylebender14 Nov 23, 2024
e3c39fa
- drop body key and put constrait on the body to have only one dynami…
laststylebender14 Nov 25, 2024
eb03a6f
- add proper validations
laststylebender14 Nov 25, 2024
8f0da83
- rename variables and add docs
laststylebender14 Nov 25, 2024
b125d88
- fix validation
laststylebender14 Nov 25, 2024
34971a0
- lint changes
laststylebender14 Nov 25, 2024
737cbbd
- lint fixes
laststylebender14 Nov 25, 2024
8d136ed
Merge branch 'main' into feat/impl-body-batching-with-dl
laststylebender14 Nov 25, 2024
863b148
- clean up
laststylebender14 Nov 25, 2024
096352f
Merge branch 'feat/impl-body-batching-with-dl' of https://github.com/…
laststylebender14 Nov 25, 2024
07e40f7
- add validation errors for batching
laststylebender14 Nov 25, 2024
db58def
- update error message
laststylebender14 Nov 25, 2024
902a580
- add feature flag
laststylebender14 Nov 25, 2024
512a7a2
Merge branch 'main' into feat/impl-body-batching-with-dl
laststylebender14 Nov 29, 2024
a14e998
- fix conflict changes
laststylebender14 Nov 29, 2024
3cf3566
feat: improve perf of body batching (#3177)
laststylebender14 Dec 6, 2024
a845e83
perf: optimize the body batching flow (#3196)
laststylebender14 Dec 9, 2024
c178372
Merge branch 'main' into feat/impl-body-batching-with-dl
laststylebender14 Dec 9, 2024
e0c8e11
- conflict changes
laststylebender14 Dec 9, 2024
a29d2c5
- revert: the trait impls
laststylebender14 Dec 9, 2024
c978675
- module renamed to dynamic req
laststylebender14 Dec 9, 2024
6cf3156
- lint changes & code optimise
laststylebender14 Dec 9, 2024
1310419
Merge branch 'main' into feat/impl-body-batching-with-dl
laststylebender14 Dec 9, 2024
e233d58
- simplify check
laststylebender14 Dec 9, 2024
95d45d9
- refactor: rename fields and minor clean up
laststylebender14 Dec 9, 2024
b580c3e
- lint changes
laststylebender14 Dec 9, 2024
bf45b92
Merge branch 'main' into feat/impl-body-batching-with-dl
laststylebender14 Dec 9, 2024
dbbf2a0
Merge branch 'main' into feat/impl-body-batching-with-dl
tusharmath Dec 9, 2024
dfa6e27
drop unused fields
tusharmath Dec 9, 2024
c4a572a
fix: add validation for batchKey requiring either body or query param…
laststylebender14 Dec 9, 2024
b8ce74a
refactor: update CI configuration and remove integration_test feature…
tusharmath Dec 9, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,6 @@ default = ["cli", "js"]
# Feature flag to force JIT engine inside integration tests
force_jit = []


[workspace]
members = [
".",
Expand Down
12 changes: 6 additions & 6 deletions generated/.tailcallrc.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -173,10 +173,10 @@ directive @http(
batchKey: [String!]
"""
The body of the API call. It's used for methods like POST or PUT that send data to
the server. You can pass it as a static object or use a Mustache template to substitute
variables from the GraphQL variables.
the server. You can pass it as a static object or use a Mustache template with object
to substitute variables from the GraphQL variables.
"""
body: String
body: JSON
"""
Enables deduplication of IO operations to enhance performance.This flag prevents
duplicate IO requests from being executed concurrently, reducing resource load. Caution:
Expand Down Expand Up @@ -918,10 +918,10 @@ input Http {
batchKey: [String!]
"""
The body of the API call. It's used for methods like POST or PUT that send data to
the server. You can pass it as a static object or use a Mustache template to substitute
variables from the GraphQL variables.
the server. You can pass it as a static object or use a Mustache template with object
to substitute variables from the GraphQL variables.
"""
body: String
body: JSON
"""
Enables deduplication of IO operations to enhance performance.This flag prevents
duplicate IO requests from being executed concurrently, reducing resource load. Caution:
Expand Down
10 changes: 8 additions & 2 deletions src/core/blueprint/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,18 @@ pub enum BlueprintError {
#[error("Protobuf files were not specified in the config")]
ProtobufFilesNotSpecifiedInConfig,

#[error("GroupBy is only supported for GET requests")]
GroupByOnlyForGet,
#[error("GroupBy is only supported for GET and POST requests")]
GroupByOnlyForGetAndPost,

#[error("Request body batching requires exactly one dynamic value in the body.")]
BatchRequiresDynamicParameter,

#[error("Batching capability was used without enabling it in upstream")]
IncorrectBatchingUsage,

#[error("batchKey requires either body or query parameters")]
BatchKeyRequiresEitherBodyOrQuery,

#[error("script is required")]
ScriptIsRequired,

Expand Down
107 changes: 92 additions & 15 deletions src/core/blueprint/operators/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,22 +22,24 @@
Err(e) => Valid::from_validation_err(BlueprintError::from_validation_string(e)),
};

Valid::<(), BlueprintError>::fail(BlueprintError::GroupByOnlyForGet)
.when(|| !http.batch_key.is_empty() && http.method != Method::GET)
.and(
Valid::<(), BlueprintError>::fail(BlueprintError::IncorrectBatchingUsage).when(|| {
(config_module.upstream.get_delay() < 1
|| config_module.upstream.get_max_size() < 1)
&& !http.batch_key.is_empty()
}),
)
Valid::<(), BlueprintError>::fail(BlueprintError::IncorrectBatchingUsage)
.when(|| {
(config_module.upstream.get_delay() < 1 || config_module.upstream.get_max_size() < 1)
&& !http.batch_key.is_empty()
})
.and(
Valid::from_iter(http.query.iter(), |query| {
validate_argument(config_module, Mustache::parse(query.value.as_str()), field)
})
.unit()
.trace("query"),
)
.and(
Valid::<(), BlueprintError>::fail(BlueprintError::BatchKeyRequiresEitherBodyOrQuery)
.when(|| {
!http.batch_key.is_empty() && (http.body.is_none() && http.query.is_empty())
}),
)
.and(Valid::succeed(http.url.as_str()))
.zip(mustache_headers)
.and_then(|(base_url, headers)| {
Expand Down Expand Up @@ -67,6 +69,22 @@
Err(e) => Valid::fail(BlueprintError::Error(e)),
}
})
.and_then(|request_template| {
if !http.batch_key.is_empty() && (http.body.is_some() || http.method != Method::GET) {
if let Some(body) = http.body.as_ref() {
let dynamic_paths = count_dynamic_paths(body);
if dynamic_paths != 1 {
Valid::fail(BlueprintError::BatchRequiresDynamicParameter).trace("body")
} else {
Valid::succeed(request_template)
}
} else {
Valid::fail(BlueprintError::BatchRequiresDynamicParameter).trace("body")

Check warning on line 82 in src/core/blueprint/operators/http.rs

View check run for this annotation

Codecov / codecov/patch

src/core/blueprint/operators/http.rs#L82

Added line #L82 was not covered by tests
}
} else {
Valid::succeed(request_template)
}
})
.map(|req_template| {
// marge http and upstream on_request
let on_request = http
Expand All @@ -76,13 +94,18 @@
let on_response_body = http.on_response_body.clone();
let hook = WorkerHooks::try_new(on_request, on_response_body).ok();

let io = if !http.batch_key.is_empty() && http.method == Method::GET {
let io = if !http.batch_key.is_empty() {
// Find a query parameter that contains a reference to the {{.value}} key
let key = http.query.iter().find_map(|q| {
Mustache::parse(&q.value)
.expression_contains("value")
.then(|| q.key.clone())
});
let key = if http.method == Method::GET {
http.query.iter().find_map(|q| {
Mustache::parse(&q.value)
.expression_contains("value")
.then(|| q.key.clone())
})
} else {
None
};

IR::IO(IO::Http {
req_template,
group_by: Some(GroupBy::new(http.batch_key.clone(), key)),
Expand All @@ -105,3 +128,57 @@
})
.and_then(apply_select)
}

/// Count the number of dynamic expressions in the JSON value.
fn count_dynamic_paths(json: &serde_json::Value) -> usize {
let mut count = 0;
match json {
serde_json::Value::Array(arr) => {
for v in arr {
count += count_dynamic_paths(v)
}
}
serde_json::Value::Object(obj) => {
for (_, v) in obj {
count += count_dynamic_paths(v)
}
}
serde_json::Value::String(s) => {
if !Mustache::parse(s).is_const() {
count += 1;
}
}
_ => {}

Check warning on line 151 in src/core/blueprint/operators/http.rs

View check run for this annotation

Codecov / codecov/patch

src/core/blueprint/operators/http.rs#L151

Added line #L151 was not covered by tests
}
count
}

#[cfg(test)]
mod test {
use serde_json::json;

use super::*;

#[test]
fn test_extract_expression_keys_from_nested_objects() {
let json = r#"{"body":"d","userId":"{{.value.uid}}","nested":{"other":"{{test}}"}}"#;
let json = serde_json::from_str(json).unwrap();
let keys = count_dynamic_paths(&json);
assert_eq!(keys, 2);
}

#[test]
fn test_extract_expression_keys_from_mixed_json() {
let json = r#"{"body":"d","userId":"{{.value.uid}}","nested":{"other":"{{test}}"},"meta":[{"key": "id", "value": "{{.value.userId}}"}]}"#;
let json = serde_json::from_str(json).unwrap();
let keys = count_dynamic_paths(&json);
assert_eq!(keys, 3);
}

#[test]
fn test_with_non_json_value() {
let json = json!(r#"{{.value}}"#);
let keys = count_dynamic_paths(&json);
assert_eq!(keys, 1);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ Index {
),
headers: {},
body: Some(
"{{.args.input}}",
String("{{.args.input}}"),
),
description: None,
encoding: ApplicationJson,
Expand Down Expand Up @@ -127,7 +127,7 @@ Index {
),
headers: {},
body: Some(
"{{.args.input}}",
String("{{.args.input}}"),
),
description: None,
encoding: ApplicationJson,
Expand Down Expand Up @@ -205,7 +205,7 @@ Index {
),
headers: {},
body: Some(
"{{.args.input}}",
String("{{.args.input}}"),
),
description: None,
encoding: ApplicationJson,
Expand Down Expand Up @@ -286,7 +286,7 @@ Index {
),
headers: {},
body: Some(
"{{.args.input}}",
String("{{.args.input}}"),
),
description: None,
encoding: ApplicationJson,
Expand Down
5 changes: 3 additions & 2 deletions src/core/config/directives/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,9 @@ pub struct Http {
#[serde(default, skip_serializing_if = "is_default")]
/// The body of the API call. It's used for methods like POST or PUT that
/// send data to the server. You can pass it as a static object or use a
/// Mustache template to substitute variables from the GraphQL variables.
pub body: Option<String>,
/// Mustache template with object to substitute variables from the GraphQL
/// variables.
pub body: Option<Value>,

#[serde(default, skip_serializing_if = "is_default")]
/// The `encoding` parameter specifies the encoding of the request body. It
Expand Down
10 changes: 6 additions & 4 deletions src/core/config/transformer/subgraph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ impl KeysExtractor {
Valid::from_iter(
[
Self::parse_str(http.url.as_str()).trace("url"),
Self::parse_str_option(http.body.as_deref()).trace("body"),
Self::parse_json_option(http.body.as_ref()).trace("body"),
Self::parse_key_value_iter(http.headers.iter()).trace("headers"),
Self::parse_key_value_iter(http.query.iter().map(|q| KeyValue {
key: q.key.to_string(),
Expand Down Expand Up @@ -355,9 +355,9 @@ impl KeysExtractor {
.map_to(keys)
}

fn parse_str_option(s: Option<&str>) -> Valid<Keys, String> {
fn parse_json_option(s: Option<&serde_json::Value>) -> Valid<Keys, String> {
if let Some(s) = s {
Self::parse_str(s)
Self::parse_str(&s.to_string())
} else {
Valid::succeed(Keys::new())
}
Expand Down Expand Up @@ -483,7 +483,9 @@ mod tests {
fn test_extract_http() {
let http = Http {
url: "http://tailcall.run/users/{{.value.id}}".to_string(),
body: Some(r#"{ "obj": "{{.value.obj}}"} "#.to_string()),
body: Some(serde_json::Value::String(
r#"{ "obj": "{{.value.obj}}"} "#.to_string(),
)),
headers: vec![KeyValue {
key: "{{.value.header.key}}".to_string(),
value: "{{.value.header.value}}".to_string(),
Expand Down
2 changes: 1 addition & 1 deletion src/core/endpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ pub struct Endpoint {
pub input: JsonSchema,
pub output: JsonSchema,
pub headers: HeaderMap,
pub body: Option<String>,
pub body: Option<serde_json::Value>,
pub description: Option<String>,
pub encoding: Encoding,
}
Expand Down
5 changes: 4 additions & 1 deletion src/core/generator/json/operation_generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,10 @@ impl OperationTypeGenerator {
let arg_name_gen = NameGenerator::new(prefix.as_str());
let arg_name = arg_name_gen.next();

http_resolver.body = Some(format!("{{{{.args.{}}}}}", arg_name));
http_resolver.body = Some(serde_json::Value::String(format!(
"{{{{.args.{}}}}}",
arg_name
)));
http_resolver.method = request_sample.method.to_owned();

field.args.insert(
Expand Down
9 changes: 5 additions & 4 deletions src/core/generator/proto/connect_rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ impl From<Grpc> for Http {

Self {
url: new_url,
body: body.map(|b| b.to_string()),
body,
method: crate::core::http::Method::POST,
headers,
batch_key,
Expand Down Expand Up @@ -91,7 +91,7 @@ mod tests {

assert_eq!(http.url, "http://localhost:8080/package.service/method");
assert_eq!(http.method, crate::core::http::Method::POST);
assert_eq!(http.body, Some(r#"{"key":"value"}"#.to_string()));
assert_eq!(http.body, Some(json!({"key": "value"})));
}

#[test]
Expand All @@ -109,7 +109,7 @@ mod tests {

let http = Http::from(grpc);

assert_eq!(http.body, Some("{}".to_string()));
assert_eq!(http.body, Some(json!({})));
}

#[test]
Expand All @@ -136,6 +136,7 @@ mod tests {
.value,
"bar".to_string()
);
assert_eq!(http.body, Some(json!({})));
}

#[test]
Expand All @@ -155,7 +156,7 @@ mod tests {

assert_eq!(http.url, "http://localhost:8080/package.service/method");
assert_eq!(http.method, crate::core::http::Method::POST);
assert_eq!(http.body, Some(r#"{"key":"value"}"#.to_string()));
assert_eq!(http.body, Some(json!({"key": "value"})));
assert_eq!(
http.headers
.iter()
Expand Down
Loading
Loading