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

perf: optimize the body batching flow #3196

Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
0d39765
- use Value instead String inplace of body in http
laststylebender14 Nov 29, 2024
2cd2404
- utilise body from data loader request.
laststylebender14 Nov 29, 2024
91cb955
- refactor: use request wrapper to hold request and it's deserialized…
laststylebender14 Nov 29, 2024
503ff49
- handle nested keys for body.
laststylebender14 Nov 29, 2024
6b56dbb
- lint changes
laststylebender14 Nov 29, 2024
1808ba1
- clean up
laststylebender14 Nov 29, 2024
92668b8
- make tests pass
laststylebender14 Nov 29, 2024
c705093
- make body optional
laststylebender14 Nov 29, 2024
87f8ee8
- fix tests
laststylebender14 Nov 29, 2024
0d2ce7e
- reduce duplication
laststylebender14 Nov 29, 2024
58d56b2
- make checks compile time safe.
laststylebender14 Nov 29, 2024
e8ca763
- fix check for methods
laststylebender14 Nov 29, 2024
0e4d685
Merge branch 'feat/impl-body-batching-with-dl' into perf/improve-perf…
laststylebender14 Nov 29, 2024
0de2aab
- fix merging issues
laststylebender14 Nov 29, 2024
a14cf98
- lint changes
laststylebender14 Nov 29, 2024
dbdfaae
- fix test cases
laststylebender14 Nov 29, 2024
81e2ad5
- lint changes
laststylebender14 Nov 29, 2024
80ad2cf
- code clean up
laststylebender14 Dec 3, 2024
845a6c0
- impl len method on object
laststylebender14 Dec 3, 2024
e3d0473
- fix typo
laststylebender14 Dec 3, 2024
9b6df44
- impl is_empty
laststylebender14 Dec 3, 2024
01cea8f
- handle case when body is just key
laststylebender14 Dec 3, 2024
6666771
- fix tests
laststylebender14 Dec 3, 2024
89628c4
- use the constraint for performance advantage.
laststylebender14 Dec 4, 2024
41fa71b
- use Option instead of vec.
laststylebender14 Dec 4, 2024
116d592
- lint changes
laststylebender14 Dec 4, 2024
8c07ae9
rename files
tusharmath Dec 6, 2024
816225a
Merge branch 'feat/impl-body-batching-with-dl' into perf/optimize-the…
laststylebender14 Dec 9, 2024
163de1b
- merge conflict fixes
laststylebender14 Dec 9, 2024
1c61a8f
- fix body for tests
laststylebender14 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
12 changes: 6 additions & 6 deletions generated/.tailcallrc.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -167,10 +167,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 @@ -900,10 +900,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
6 changes: 1 addition & 5 deletions generated/.tailcallrc.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -654,11 +654,7 @@
}
},
"body": {
"description": "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.",
"type": [
"string",
"null"
]
"description": "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 with object to substitute variables from the GraphQL variables."
},
"dedupe": {
"description": "Enables deduplication of IO operations to enhance performance.\n\nThis flag prevents duplicate IO requests from being executed concurrently, reducing resource load. Caution: May lead to issues with APIs that expect unique results for identical inputs, such as nonce-based APIs.",
Expand Down
9 changes: 5 additions & 4 deletions src/core/blueprint/dynamic_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use async_graphql_value::{ConstValue, Name};
use indexmap::IndexMap;
use serde_json::Value;

use crate::core::json::JsonLike;
use crate::core::mustache::Mustache;

#[derive(Debug, Clone, PartialEq)]
Expand Down Expand Up @@ -90,7 +91,7 @@ impl<A> DynamicValue<A> {
}
}

impl TryFrom<&Value> for DynamicValue<ConstValue> {
impl<A: for<'a> JsonLike<'a>> TryFrom<&Value> for DynamicValue<A> {
type Error = anyhow::Error;

fn try_from(value: &Value) -> Result<Self, Self::Error> {
Expand All @@ -104,19 +105,19 @@ impl TryFrom<&Value> for DynamicValue<ConstValue> {
Ok(DynamicValue::Object(out))
}
Value::Array(arr) => {
let out: Result<Vec<DynamicValue<ConstValue>>, Self::Error> =
let out: Result<Vec<DynamicValue<A>>, Self::Error> =
arr.iter().map(DynamicValue::try_from).collect();
Ok(DynamicValue::Array(out?))
}
Value::String(s) => {
let m = Mustache::parse(s.as_str());
if m.is_const() {
Ok(DynamicValue::Value(ConstValue::from_json(value.clone())?))
Ok(DynamicValue::Value(A::clone_from(value)))
} else {
Ok(DynamicValue::Mustache(m))
}
}
_ => Ok(DynamicValue::Value(ConstValue::from_json(value.clone())?)),
_ => Ok(DynamicValue::Value(A::clone_from(value))),
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/core/blueprint/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ pub enum BlueprintError {
#[error("GroupBy is only supported for GET and POST requests")]
GroupByOnlyForGetAndPost,

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

#[error("Batching capability was used without enabling it in upstream")]
IncorrectBatchingUsage,
Expand Down
139 changes: 80 additions & 59 deletions src/core/blueprint/operators/http.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use regex::Regex;
use std::borrow::Cow;

use tailcall_valid::{Valid, Validator};
use template_validation::validate_argument;

Expand All @@ -22,39 +23,10 @@ pub fn compile_http(
Err(e) => Valid::from_validation_err(BlueprintError::from_validation_string(e)),
};

Valid::<(), BlueprintError>::fail(BlueprintError::GroupByOnlyForGetAndPost)
.when(|| !http.batch_key.is_empty() && !matches!(http.method, Method::GET | Method::POST))
.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()
}),
)
.and_then(|_| {
let result = if http.method == Method::POST {
if !http.batch_key.is_empty() {
let keys = http
.body
.as_ref()
.map(|b| extract_expression_keys(b).len())
.unwrap_or_default();

if keys == 1 {
Valid::succeed(())
} else {
Valid::fail(
BlueprintError::RequestBatchingRequiresAtLeastOneDynamicParameter,
)
}
} else {
Valid::succeed(())
}
} else {
Valid::succeed(())
};

result.trace("body")
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| {
Expand Down Expand Up @@ -92,6 +64,23 @@ pub fn compile_http(
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) {
let keys = http.body.as_ref().map(|b| extract_expression_paths(b));
if let Some(keys) = keys {
// only one dynamic value allowed in body for batching to work.
if keys.len() != 1 {
Valid::fail(BlueprintError::BatchRequiresDynamicParameter).trace("body")
} else {
Valid::succeed(request_template)
}
} else {
Valid::fail(BlueprintError::BatchRequiresDynamicParameter).trace("body")
}
} else {
Valid::succeed(request_template)
}
})
.map(|req_template| {
// marge http and upstream on_request
let http_filter = http
Expand All @@ -111,17 +100,7 @@ pub fn compile_http(
.then(|| q.key.clone())
})
} else {
// find the key from the body where value is mustache template.
http.body
.as_ref()
.map(|b| extract_expression_keys(b))
.and_then(|keys| {
if keys.len() == 1 {
Some(keys[0].clone())
} else {
None
}
})
None
};

IR::IO(IO::Http {
Expand Down Expand Up @@ -149,33 +128,75 @@ pub fn compile_http(

/// extracts the keys from the json representation, if the value is of mustache
/// template type.
fn extract_expression_keys(json: &str) -> Vec<String> {
let mut keys = Vec::new();
let re = Regex::new(r#""([^"]+)"\s*:\s*"\{\{.*?\}\}""#).unwrap();
for cap in re.captures_iter(json) {
if let Some(key) = cap.get(1) {
keys.push(key.as_str().to_string());
fn extract_expression_paths(json: &serde_json::Value) -> Vec<Vec<Cow<'_, str>>> {
fn extract_paths<'a>(
json: &'a serde_json::Value,
path: &mut Vec<Cow<'a, str>>,
) -> Vec<Vec<Cow<'a, str>>> {
let mut keys = vec![];
match json {
serde_json::Value::Array(arr) => {
arr.iter().enumerate().for_each(|(idx, v)| {
let idx = idx.to_string();
path.push(Cow::Owned(idx));
keys.extend(extract_paths(v, path));
});
}
serde_json::Value::Object(obj) => {
obj.iter().for_each(|(k, v)| {
path.push(Cow::Borrowed(k));
keys.extend(extract_paths(v, path));
path.pop();
});
}
serde_json::Value::String(s) => {
if !Mustache::parse(s).is_const() {
keys.push(path.to_vec());
}
}
_ => {}
}
keys
}
println!("[Finder]: input: {:#?} and output: {:#?}", json, keys);
keys

extract_paths(json, &mut Vec::new())
}

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

use super::*;

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

#[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 = extract_expression_paths(&json);
assert_eq!(keys.len(), 3);
assert_eq!(
keys,
vec![
vec!["userId"],
vec!["nested", "other"],
vec!["meta", "0", "value"]
]
);
}

#[test]
fn test_with_non_json_value() {
let json = r#"{{.value}}"#;
let keys = extract_expression_keys(json);
assert_eq!(keys, Vec::<String>::new());
let json = json!(r#"{{.value}}"#);
let keys = extract_expression_paths(&json);
assert!(keys.iter().all(|f| f.is_empty()));
}
}
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
Loading
Loading