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

refactor: move server responseHeaders to server headers custom #1402

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
e2ac994
feat: use `headers.custom` to hold response headers
ologbonowiwi Mar 12, 2024
c30c3f4
test: update `responseHeaders` cases to use `headers.custom` instead
ologbonowiwi Mar 12, 2024
012f104
docs: update docs via `./lint.sh --mode=fix`
ologbonowiwi Mar 12, 2024
0a9ebe8
test: add case for overriding existing headers
ologbonowiwi Mar 12, 2024
fbe15a5
feat: use `headers.custom` to hold response headers
ologbonowiwi Mar 12, 2024
ea7ffde
test: update `responseHeaders` cases to use `headers.custom` instead
ologbonowiwi Mar 12, 2024
fa83698
docs: update docs via `./lint.sh --mode=fix`
ologbonowiwi Mar 12, 2024
a784132
test: add case for overriding existing headers
ologbonowiwi Mar 12, 2024
5e0ae9c
Merge branch 'main' into refactor/#1400/move-server-responseHeaders-t…
tusharmath Mar 13, 2024
542ce18
Merge branch 'main' into refactor/#1400/move-server-responseHeaders-t…
tusharmath Mar 13, 2024
c7edd11
refactor(`key_values.rs`): move key_value merge right from `server.rs`
ologbonowiwi Mar 13, 2024
79df0ee
refactor(`headers.rs`): move headers merging from `server.rs`
ologbonowiwi Mar 13, 2024
9307219
feat: allow duplicated headers
ologbonowiwi Mar 13, 2024
44c90ec
test: update snapshot
ologbonowiwi Mar 13, 2024
6e99489
Merge branch 'refactor/#1400/move-server-responseHeaders-to-server-he…
ologbonowiwi Mar 13, 2024
4481d78
refactor: remove unused method
ologbonowiwi Mar 13, 2024
108ddf2
style: run `./lint.sh --mode=fix`
ologbonowiwi Mar 13, 2024
bb1acd5
style: run `./lint.sh --mode=fix`
ologbonowiwi Mar 13, 2024
367aa92
refactor(`key_values.rs`): reduce complexity from O(n^2) to O(n)
ologbonowiwi Mar 13, 2024
7ac04fe
Merge branch 'main' of github.com:tailcallhq/tailcall into refactor/#…
ologbonowiwi Mar 13, 2024
2ece27b
fix: implement map usage correctly
ologbonowiwi Mar 13, 2024
4418df9
refactor: remove unused test submodule
ologbonowiwi Mar 13, 2024
2478fe9
refactor: avoid overusage of memory
ologbonowiwi Mar 13, 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 @@ -286,12 +286,6 @@ directive @server(
"""
queryValidation: Boolean
"""
The `responseHeaders` are key-value pairs included in every server response. Useful
for setting headers like `Access-Control-Allow-Origin` for cross-origin requests
or additional headers for downstream services.
"""
responseHeaders: [KeyValue]
"""
`responseValidation` Tailcall automatically validates responses from upstream services
using inferred schema. @default `false`.
"""
Expand Down Expand Up @@ -577,6 +571,12 @@ input Headers {
value is the least of the values received from upstream services. @default `false`.
"""
cacheControl: Boolean
"""
`headers` are key-value pairs included in every server response. Useful for setting
headers like `Access-Control-Allow-Origin` for cross-origin requests or additional
headers for downstream services.
"""
custom: [KeyValue]
}
"""
The @http operator indicates that a field or node is backed by a REST API.For instance,
Expand Down
14 changes: 7 additions & 7 deletions generated/.tailcallrc.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1215,6 +1215,13 @@
"boolean",
"null"
]
},
"custom": {
"description": "`headers` are key-value pairs included in every server response. Useful for setting headers like `Access-Control-Allow-Origin` for cross-origin requests or additional headers for downstream services.",
"type": "array",
"items": {
"$ref": "#/definitions/KeyValue"
}
}
}
},
Expand Down Expand Up @@ -1588,13 +1595,6 @@
"null"
]
},
"responseHeaders": {
"description": "The `responseHeaders` are key-value pairs included in every server response. Useful for setting headers like `Access-Control-Allow-Origin` for cross-origin requests or additional headers for downstream services.",
"type": "array",
"items": {
"$ref": "#/definitions/KeyValue"
}
},
"responseValidation": {
"description": "`responseValidation` Tailcall automatically validates responses from upstream services using inferred schema. @default `false`.",
"type": [
Expand Down
5 changes: 3 additions & 2 deletions src/blueprint/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ fn validate_hostname(hostname: String) -> Valid<IpAddr, String> {
}
}

fn handle_response_headers(resp_headers: BTreeMap<String, String>) -> Valid<HeaderMap, String> {
fn handle_response_headers(resp_headers: Vec<(String, String)>) -> Valid<HeaderMap, String> {
Valid::from_iter(resp_headers.iter(), |(k, v)| {
let name = Valid::from(
HeaderName::from_bytes(k.as_bytes())
Expand All @@ -173,7 +173,8 @@ fn handle_response_headers(resp_headers: BTreeMap<String, String>) -> Valid<Head
name.zip(value)
})
.map(|headers| headers.into_iter().collect::<HeaderMap>())
.trace("responseHeaders")
.trace("custom")
.trace("headers")
.trace("@server")
.trace("schema")
}
Expand Down
24 changes: 24 additions & 0 deletions src/config/headers.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use serde::{Deserialize, Serialize};

use crate::config::KeyValue;
use crate::is_default;

#[derive(Serialize, Deserialize, Clone, Debug, Default, PartialEq, Eq, schemars::JsonSchema)]
Expand All @@ -10,10 +11,33 @@ pub struct Headers {
/// activated. The `max-age` value is the least of the values received from
/// upstream services. @default `false`.
pub cache_control: Option<bool>,

#[serde(default, skip_serializing_if = "is_default")]
/// `headers` are key-value pairs included in every server
/// response. Useful for setting headers like `Access-Control-Allow-Origin`
/// for cross-origin requests or additional headers for downstream services.
pub custom: Vec<KeyValue>,
}
ologbonowiwi marked this conversation as resolved.
Show resolved Hide resolved

impl Headers {
pub fn enable_cache_control(&self) -> bool {
self.cache_control.unwrap_or(false)
}
}

pub fn merge_headers(current: Option<Headers>, other: Option<Headers>) -> Option<Headers> {
let mut headers = current.clone();

if let Some(other_headers) = other {
if let Some(mut self_headers) = current.clone() {
self_headers.cache_control = other_headers.cache_control.or(self_headers.cache_control);
self_headers.custom.extend(other_headers.custom);

headers = Some(self_headers);
} else {
headers = Some(other_headers);
}
}

headers
}
63 changes: 63 additions & 0 deletions src/config/key_values.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,19 @@ pub struct KeyValue {
pub value: String,
}

pub fn merge_key_value_vecs(current: &[KeyValue], other: &[KeyValue]) -> Vec<KeyValue> {
let mut acc: BTreeMap<&String, &String> =
current.iter().map(|kv| (&kv.key, &kv.value)).collect();

for kv in other {
acc.insert(&kv.key, &kv.value);
}

acc.iter()
.map(|(k, v)| KeyValue { key: k.to_string(), value: v.to_string() })
.collect()
}

impl Serialize for KeyValues {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
Expand Down Expand Up @@ -97,4 +110,54 @@ mod tests {
// Using the deref trait
assert_eq!(kv["a"], "b");
}

#[test]
fn test_merge_with_both_empty() {
let current = vec![];
let other = vec![];
let result = merge_key_value_vecs(&current, &other);
assert!(result.is_empty());
}

#[test]
fn test_merge_with_current_empty() {
let current = vec![];
let other = vec![KeyValue { key: "key1".to_string(), value: "value1".to_string() }];
let result = merge_key_value_vecs(&current, &other);
assert_eq!(result.len(), 1);
assert_eq!(result[0].key, "key1");
assert_eq!(result[0].value, "value1");
}

#[test]
fn test_merge_with_other_empty() {
let current = vec![KeyValue { key: "key1".to_string(), value: "value1".to_string() }];
let other = vec![];
let result = merge_key_value_vecs(&current, &other);
assert_eq!(result.len(), 1);
assert_eq!(result[0].key, "key1");
assert_eq!(result[0].value, "value1");
}

#[test]
fn test_merge_with_unique_keys() {
let current = vec![KeyValue { key: "key1".to_string(), value: "value1".to_string() }];
let other = vec![KeyValue { key: "key2".to_string(), value: "value2".to_string() }];
let result = merge_key_value_vecs(&current, &other);
assert_eq!(result.len(), 2);
assert_eq!(result[0].key, "key1");
assert_eq!(result[0].value, "value1");
assert_eq!(result[1].key, "key2");
assert_eq!(result[1].value, "value2");
}

#[test]
fn test_merge_with_overlapping_keys() {
let current = vec![KeyValue { key: "key1".to_string(), value: "value1".to_string() }];
let other = vec![KeyValue { key: "key1".to_string(), value: "value2".to_string() }];
let result = merge_key_value_vecs(&current, &other);
assert_eq!(result.len(), 1);
assert_eq!(result[0].key, "key1");
assert_eq!(result[0].value, "value2");
}
}
1 change: 1 addition & 0 deletions src/config/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
pub use config::*;
pub use config_module::*;
pub use expr::*;
pub use headers::*;
pub use key_values::*;
pub use link::*;
pub use reader_context::*;
Expand Down
41 changes: 14 additions & 27 deletions src/config/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use std::collections::BTreeMap;

use serde::{Deserialize, Serialize};

use super::{merge_headers, merge_key_value_vecs};
use crate::config::headers::Headers;
use crate::config::KeyValue;
use crate::is_default;
Expand Down Expand Up @@ -64,12 +65,6 @@ pub struct Server {
/// @default `false`.
pub query_validation: Option<bool>,

#[serde(skip_serializing_if = "is_default", default)]
/// The `responseHeaders` are key-value pairs included in every server
/// response. Useful for setting headers like `Access-Control-Allow-Origin`
/// for cross-origin requests or additional headers for downstream services.
pub response_headers: Vec<KeyValue>,

#[serde(default, skip_serializing_if = "is_default")]
/// `responseValidation` Tailcall automatically validates responses from
/// upstream services using inferred schema. @default `false`.
Expand Down Expand Up @@ -165,12 +160,16 @@ impl Server {
.collect()
}

pub fn get_response_headers(&self) -> BTreeMap<String, String> {
self.response_headers
.clone()
.iter()
.map(|kv| (kv.key.clone(), kv.value.clone()))
.collect()
pub fn get_response_headers(&self) -> Vec<(String, String)> {
self.headers
.as_ref()
.map(|h| h.custom.clone())
.map_or(Vec::new(), |headers| {
headers
.iter()
.map(|kv| (kv.key.clone(), kv.value.clone()))
.collect()
})
}

pub fn get_version(self) -> HttpVersion {
Expand All @@ -183,7 +182,7 @@ impl Server {

pub fn merge_right(mut self, other: Self) -> Self {
self.apollo_tracing = other.apollo_tracing.or(self.apollo_tracing);
self.headers = other.headers.or(self.headers);
self.headers = merge_headers(self.headers, other.headers);
self.graphiql = other.graphiql.or(self.graphiql);
tusharmath marked this conversation as resolved.
Show resolved Hide resolved
self.introspection = other.introspection.or(self.introspection);
self.query_validation = other.query_validation.or(self.query_validation);
Expand All @@ -196,7 +195,7 @@ impl Server {
self.workers = other.workers.or(self.workers);
self.port = other.port.or(self.port);
self.hostname = other.hostname.or(self.hostname);
self.vars = other.vars.iter().fold(self.vars, |mut acc, kv| {
self.vars = other.vars.iter().fold(self.vars.to_vec(), |mut acc, kv| {
let position = acc.iter().position(|x| x.key == kv.key);
if let Some(pos) = position {
acc[pos] = kv.clone();
Expand All @@ -205,19 +204,7 @@ impl Server {
};
acc
});
self.response_headers =
other
.response_headers
.iter()
.fold(self.response_headers, |mut acc, kv| {
let position = acc.iter().position(|x| x.key == kv.key);
if let Some(pos) = position {
acc[pos] = kv.clone();
} else {
acc.push(kv.clone());
};
acc
});
self.vars = merge_key_value_vecs(&self.vars, &other.vars);
tusharmath marked this conversation as resolved.
Show resolved Hide resolved
self.version = other.version.or(self.version);
self.pipeline_flush = other.pipeline_flush.or(self.pipeline_flush);
self.script = other.script.or(self.script);
Expand Down
22 changes: 12 additions & 10 deletions tests/execution/custom-headers.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,18 @@
```json @server
{
"server": {
"responseHeaders": [
{
"key": "x-id",
"value": "1"
},
{
"key": "x-name",
"value": "John Doe"
}
]
"headers": {
"custom": [
{
"key": "x-id",
"value": "1"
},
{
"key": "x-name",
"value": "John Doe"
}
]
}
},
"upstream": {},
"schema": {
Expand Down
31 changes: 31 additions & 0 deletions tests/execution/test-response-header-merge.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# test-response-header-value

```graphql @server
schema @server(headers: {custom: [{key: "a", value: "a"}]}) {
tusharmath marked this conversation as resolved.
Show resolved Hide resolved
query: Query
}

type User {
name: String
age: Int
}

type Query {
user: User @const(data: {name: "John"})
}
```

```graphql @server
schema @server(headers: {custom: [{key: "a", value: "b"}]}) {
query: Query
}

type User {
name: String
age: Int
}

type Query {
user: User @const(data: {name: "John"})
}
```
2 changes: 1 addition & 1 deletion tests/execution/test-response-header-value.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
###### sdl error

```graphql @server
schema @server(responseHeaders: [{key: "a", value: "a \n b"}]) {
schema @server(headers: {custom: [{key: "a", value: "a \n b"}]}) {
query: Query
}

Expand Down
2 changes: 1 addition & 1 deletion tests/execution/test-response-headers-multi.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
###### sdl error

```graphql @server
schema @server(responseHeaders: [{key: "a b", value: "a \n b"}, {key: "a c", value: "a \n b"}]) {
schema @server(headers: {custom: [{key: "a b", value: "a \n b"}, {key: "a c", value: "a \n b"}]}) {
query: Query
}

Expand Down
2 changes: 1 addition & 1 deletion tests/execution/test-response-headers-name.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
###### sdl error

```graphql @server
schema @server(responseHeaders: [{key: "🤣", value: "a"}]) {
schema @server(headers: {custom: [{key: "🤣", value: "a"}]}) {
query: Query
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
source: tests/execution_spec.rs
expression: merged
---
schema @server(responseHeaders: [{key: "x-id", value: "1"}, {key: "x-name", value: "John Doe"}]) @upstream {
schema @server(headers: {custom: [{key: "x-id", value: "1"}, {key: "x-name", value: "John Doe"}]}) @upstream {
query: Query
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---
source: tests/execution_spec.rs
expression: merged
---
schema @server(headers: {custom: [{key: "a", value: "a"}, {key: "a", value: "b"}]}) @upstream {
query: Query
}

type Query {
user: User @const(data: {name: "John"})
}

type User {
age: Int
name: String
}
Loading
Loading