From e2ac994a78a07560eb9a1c169656a4a3f6bcb957 Mon Sep 17 00:00:00 2001 From: Wesley Matos Date: Tue, 12 Mar 2024 10:16:36 -0300 Subject: [PATCH 01/19] feat: use `headers.custom` to hold response headers --- src/blueprint/server.rs | 3 +- src/config/headers.rs | 7 ++++ src/config/mod.rs | 1 + src/config/server.rs | 71 +++++++++++++++++++++-------------------- 4 files changed, 47 insertions(+), 35 deletions(-) diff --git a/src/blueprint/server.rs b/src/blueprint/server.rs index 638e4a819b..7a0cbc6589 100644 --- a/src/blueprint/server.rs +++ b/src/blueprint/server.rs @@ -173,7 +173,8 @@ fn handle_response_headers(resp_headers: BTreeMap) -> Valid()) - .trace("responseHeaders") + .trace("custom") + .trace("headers") .trace("@server") .trace("schema") } diff --git a/src/config/headers.rs b/src/config/headers.rs index 47959edcb4..d98b74bdee 100644 --- a/src/config/headers.rs +++ b/src/config/headers.rs @@ -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)] @@ -10,6 +11,12 @@ pub struct Headers { /// activated. The `max-age` value is the least of the values received from /// upstream services. @default `false`. pub cache_control: Option, + + #[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, } impl Headers { diff --git a/src/config/mod.rs b/src/config/mod.rs index cf461d1f7a..3cb7612c09 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -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::*; diff --git a/src/config/server.rs b/src/config/server.rs index 5a3823f779..c1ebfadede 100644 --- a/src/config/server.rs +++ b/src/config/server.rs @@ -64,12 +64,6 @@ pub struct Server { /// @default `false`. pub query_validation: Option, - #[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, - #[serde(default, skip_serializing_if = "is_default")] /// `responseValidation` Tailcall automatically validates responses from /// upstream services using inferred schema. @default `false`. @@ -166,11 +160,15 @@ impl Server { } pub fn get_response_headers(&self) -> BTreeMap { - self.response_headers - .clone() - .iter() - .map(|kv| (kv.key.clone(), kv.value.clone())) - .collect() + self.headers + .as_ref() + .map(|h| h.custom.clone()) + .map_or(BTreeMap::new(), |headers| { + headers + .iter() + .map(|kv| (kv.key.clone(), kv.value.clone())) + .collect() + }) } pub fn get_version(self) -> HttpVersion { @@ -181,9 +179,35 @@ impl Server { self.pipeline_flush.unwrap_or(true) } + fn merge_key_value_iterators( + &self, + iter: std::slice::Iter<'_, KeyValue>, + other: std::slice::Iter<'_, KeyValue>, + ) -> Vec { + other.fold(iter.cloned().collect(), |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 + }) + } + 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); + if let Some(headers) = other.headers { + if let Some(mut self_headers) = self.headers.clone() { + self_headers.cache_control = headers.cache_control.or(self_headers.cache_control); + self_headers.custom = self + .merge_key_value_iterators(self_headers.custom.iter(), headers.custom.iter()); + + self.headers = Some(self_headers); + } else { + self.headers = Some(headers); + } + } self.graphiql = other.graphiql.or(self.graphiql); self.introspection = other.introspection.or(self.introspection); self.query_validation = other.query_validation.or(self.query_validation); @@ -196,28 +220,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| { - 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.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 = self.merge_key_value_iterators(self.vars.iter(), other.vars.iter()); self.version = other.version.or(self.version); self.pipeline_flush = other.pipeline_flush.or(self.pipeline_flush); self.script = other.script.or(self.script); From c30c3f4c2291709ccc4807542cb4f555ee9cfc1b Mon Sep 17 00:00:00 2001 From: Wesley Matos Date: Tue, 12 Mar 2024 10:17:10 -0300 Subject: [PATCH 02/19] test: update `responseHeaders` cases to use `headers.custom` instead --- tests/execution/custom-headers.md | 22 ++++++++++--------- tests/execution/test-response-header-value.md | 2 +- .../execution/test-response-headers-multi.md | 2 +- tests/execution/test-response-headers-name.md | 2 +- ...cution_spec__custom-headers.md_merged.snap | 2 +- ..._test-response-header-value.md_errors.snap | 3 ++- ...test-response-headers-multi.md_errors.snap | 12 ++++++---- ..._test-response-headers-name.md_errors.snap | 3 ++- 8 files changed, 28 insertions(+), 20 deletions(-) diff --git a/tests/execution/custom-headers.md b/tests/execution/custom-headers.md index b94d59476d..e128b0d7fb 100644 --- a/tests/execution/custom-headers.md +++ b/tests/execution/custom-headers.md @@ -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": { diff --git a/tests/execution/test-response-header-value.md b/tests/execution/test-response-header-value.md index 66b193301b..3d612a4187 100644 --- a/tests/execution/test-response-header-value.md +++ b/tests/execution/test-response-header-value.md @@ -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 } diff --git a/tests/execution/test-response-headers-multi.md b/tests/execution/test-response-headers-multi.md index 4054e4a17d..028bed3390 100644 --- a/tests/execution/test-response-headers-multi.md +++ b/tests/execution/test-response-headers-multi.md @@ -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 } diff --git a/tests/execution/test-response-headers-name.md b/tests/execution/test-response-headers-name.md index 3753ef759b..32ce9625fd 100644 --- a/tests/execution/test-response-headers-name.md +++ b/tests/execution/test-response-headers-name.md @@ -3,7 +3,7 @@ ###### sdl error ```graphql @server -schema @server(responseHeaders: [{key: "🤣", value: "a"}]) { +schema @server(headers: {custom: [{key: "🤣", value: "a"}]}) { query: Query } diff --git a/tests/snapshots/execution_spec__custom-headers.md_merged.snap b/tests/snapshots/execution_spec__custom-headers.md_merged.snap index a8118dc36a..a77f8116ac 100644 --- a/tests/snapshots/execution_spec__custom-headers.md_merged.snap +++ b/tests/snapshots/execution_spec__custom-headers.md_merged.snap @@ -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 } diff --git a/tests/snapshots/execution_spec__test-response-header-value.md_errors.snap b/tests/snapshots/execution_spec__test-response-header-value.md_errors.snap index 81470e2462..e525f70886 100644 --- a/tests/snapshots/execution_spec__test-response-header-value.md_errors.snap +++ b/tests/snapshots/execution_spec__test-response-header-value.md_errors.snap @@ -8,7 +8,8 @@ expression: errors "trace": [ "schema", "@server", - "responseHeaders" + "headers", + "custom" ], "description": null } diff --git a/tests/snapshots/execution_spec__test-response-headers-multi.md_errors.snap b/tests/snapshots/execution_spec__test-response-headers-multi.md_errors.snap index 9c77808677..d12fe47ea0 100644 --- a/tests/snapshots/execution_spec__test-response-headers-multi.md_errors.snap +++ b/tests/snapshots/execution_spec__test-response-headers-multi.md_errors.snap @@ -8,7 +8,8 @@ expression: errors "trace": [ "schema", "@server", - "responseHeaders" + "headers", + "custom" ], "description": null }, @@ -17,7 +18,8 @@ expression: errors "trace": [ "schema", "@server", - "responseHeaders" + "headers", + "custom" ], "description": null }, @@ -26,7 +28,8 @@ expression: errors "trace": [ "schema", "@server", - "responseHeaders" + "headers", + "custom" ], "description": null }, @@ -35,7 +38,8 @@ expression: errors "trace": [ "schema", "@server", - "responseHeaders" + "headers", + "custom" ], "description": null } diff --git a/tests/snapshots/execution_spec__test-response-headers-name.md_errors.snap b/tests/snapshots/execution_spec__test-response-headers-name.md_errors.snap index 283f255c52..29374aeb1d 100644 --- a/tests/snapshots/execution_spec__test-response-headers-name.md_errors.snap +++ b/tests/snapshots/execution_spec__test-response-headers-name.md_errors.snap @@ -8,7 +8,8 @@ expression: errors "trace": [ "schema", "@server", - "responseHeaders" + "headers", + "custom" ], "description": null } From 012f10463635a42ef43fc6d1f39afda18c5c1f39 Mon Sep 17 00:00:00 2001 From: Wesley Matos Date: Tue, 12 Mar 2024 10:20:48 -0300 Subject: [PATCH 03/19] docs: update docs via `./lint.sh --mode=fix` --- generated/.tailcallrc.graphql | 12 ++++++------ generated/.tailcallrc.schema.json | 14 +++++++------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/generated/.tailcallrc.graphql b/generated/.tailcallrc.graphql index aaa9fd8b59..49f2fce523 100644 --- a/generated/.tailcallrc.graphql +++ b/generated/.tailcallrc.graphql @@ -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`. """ @@ -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, diff --git a/generated/.tailcallrc.schema.json b/generated/.tailcallrc.schema.json index 757538a1ab..06b3c41086 100644 --- a/generated/.tailcallrc.schema.json +++ b/generated/.tailcallrc.schema.json @@ -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" + } } } }, @@ -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": [ From 0a9ebe8c385863c17ad167ccc37b26fb72b707a3 Mon Sep 17 00:00:00 2001 From: Wesley Matos Date: Tue, 12 Mar 2024 10:33:56 -0300 Subject: [PATCH 04/19] test: add case for overriding existing headers --- tests/execution/test-response-header-merge.md | 31 +++++++++++++++++++ ..._test-response-header-merge.md_merged.snap | 16 ++++++++++ 2 files changed, 47 insertions(+) create mode 100644 tests/execution/test-response-header-merge.md create mode 100644 tests/snapshots/execution_spec__test-response-header-merge.md_merged.snap diff --git a/tests/execution/test-response-header-merge.md b/tests/execution/test-response-header-merge.md new file mode 100644 index 0000000000..8750494391 --- /dev/null +++ b/tests/execution/test-response-header-merge.md @@ -0,0 +1,31 @@ +# test-response-header-value + +```graphql @server +schema @server(headers: {custom: [{key: "a", value: "a"}]}) { + 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"}) +} +``` diff --git a/tests/snapshots/execution_spec__test-response-header-merge.md_merged.snap b/tests/snapshots/execution_spec__test-response-header-merge.md_merged.snap new file mode 100644 index 0000000000..eecdb5cd60 --- /dev/null +++ b/tests/snapshots/execution_spec__test-response-header-merge.md_merged.snap @@ -0,0 +1,16 @@ +--- +source: tests/execution_spec.rs +expression: merged +--- +schema @server(headers: {custom: [{key: "a", value: "b"}]}) @upstream { + query: Query +} + +type Query { + user: User @const(data: {name: "John"}) +} + +type User { + age: Int + name: String +} From fbe15a5f7fe5f29cdbb26d0df086d9c1a8b3ec44 Mon Sep 17 00:00:00 2001 From: Wesley Matos Date: Tue, 12 Mar 2024 10:16:36 -0300 Subject: [PATCH 05/19] feat: use `headers.custom` to hold response headers --- src/blueprint/server.rs | 3 +- src/config/headers.rs | 7 ++++ src/config/mod.rs | 1 + src/config/server.rs | 71 +++++++++++++++++++++-------------------- 4 files changed, 47 insertions(+), 35 deletions(-) diff --git a/src/blueprint/server.rs b/src/blueprint/server.rs index 638e4a819b..7a0cbc6589 100644 --- a/src/blueprint/server.rs +++ b/src/blueprint/server.rs @@ -173,7 +173,8 @@ fn handle_response_headers(resp_headers: BTreeMap) -> Valid()) - .trace("responseHeaders") + .trace("custom") + .trace("headers") .trace("@server") .trace("schema") } diff --git a/src/config/headers.rs b/src/config/headers.rs index 47959edcb4..d98b74bdee 100644 --- a/src/config/headers.rs +++ b/src/config/headers.rs @@ -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)] @@ -10,6 +11,12 @@ pub struct Headers { /// activated. The `max-age` value is the least of the values received from /// upstream services. @default `false`. pub cache_control: Option, + + #[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, } impl Headers { diff --git a/src/config/mod.rs b/src/config/mod.rs index cf461d1f7a..3cb7612c09 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -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::*; diff --git a/src/config/server.rs b/src/config/server.rs index 5a3823f779..c1ebfadede 100644 --- a/src/config/server.rs +++ b/src/config/server.rs @@ -64,12 +64,6 @@ pub struct Server { /// @default `false`. pub query_validation: Option, - #[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, - #[serde(default, skip_serializing_if = "is_default")] /// `responseValidation` Tailcall automatically validates responses from /// upstream services using inferred schema. @default `false`. @@ -166,11 +160,15 @@ impl Server { } pub fn get_response_headers(&self) -> BTreeMap { - self.response_headers - .clone() - .iter() - .map(|kv| (kv.key.clone(), kv.value.clone())) - .collect() + self.headers + .as_ref() + .map(|h| h.custom.clone()) + .map_or(BTreeMap::new(), |headers| { + headers + .iter() + .map(|kv| (kv.key.clone(), kv.value.clone())) + .collect() + }) } pub fn get_version(self) -> HttpVersion { @@ -181,9 +179,35 @@ impl Server { self.pipeline_flush.unwrap_or(true) } + fn merge_key_value_iterators( + &self, + iter: std::slice::Iter<'_, KeyValue>, + other: std::slice::Iter<'_, KeyValue>, + ) -> Vec { + other.fold(iter.cloned().collect(), |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 + }) + } + 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); + if let Some(headers) = other.headers { + if let Some(mut self_headers) = self.headers.clone() { + self_headers.cache_control = headers.cache_control.or(self_headers.cache_control); + self_headers.custom = self + .merge_key_value_iterators(self_headers.custom.iter(), headers.custom.iter()); + + self.headers = Some(self_headers); + } else { + self.headers = Some(headers); + } + } self.graphiql = other.graphiql.or(self.graphiql); self.introspection = other.introspection.or(self.introspection); self.query_validation = other.query_validation.or(self.query_validation); @@ -196,28 +220,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| { - 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.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 = self.merge_key_value_iterators(self.vars.iter(), other.vars.iter()); self.version = other.version.or(self.version); self.pipeline_flush = other.pipeline_flush.or(self.pipeline_flush); self.script = other.script.or(self.script); From ea7ffded71570e6397e0223d21831f67e1191686 Mon Sep 17 00:00:00 2001 From: Wesley Matos Date: Tue, 12 Mar 2024 10:17:10 -0300 Subject: [PATCH 06/19] test: update `responseHeaders` cases to use `headers.custom` instead --- tests/execution/custom-headers.md | 22 ++++++++++--------- tests/execution/test-response-header-value.md | 2 +- .../execution/test-response-headers-multi.md | 2 +- tests/execution/test-response-headers-name.md | 2 +- ...cution_spec__custom-headers.md_merged.snap | 2 +- ..._test-response-header-value.md_errors.snap | 3 ++- ...test-response-headers-multi.md_errors.snap | 12 ++++++---- ..._test-response-headers-name.md_errors.snap | 3 ++- 8 files changed, 28 insertions(+), 20 deletions(-) diff --git a/tests/execution/custom-headers.md b/tests/execution/custom-headers.md index b94d59476d..e128b0d7fb 100644 --- a/tests/execution/custom-headers.md +++ b/tests/execution/custom-headers.md @@ -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": { diff --git a/tests/execution/test-response-header-value.md b/tests/execution/test-response-header-value.md index 66b193301b..3d612a4187 100644 --- a/tests/execution/test-response-header-value.md +++ b/tests/execution/test-response-header-value.md @@ -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 } diff --git a/tests/execution/test-response-headers-multi.md b/tests/execution/test-response-headers-multi.md index 4054e4a17d..028bed3390 100644 --- a/tests/execution/test-response-headers-multi.md +++ b/tests/execution/test-response-headers-multi.md @@ -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 } diff --git a/tests/execution/test-response-headers-name.md b/tests/execution/test-response-headers-name.md index 3753ef759b..32ce9625fd 100644 --- a/tests/execution/test-response-headers-name.md +++ b/tests/execution/test-response-headers-name.md @@ -3,7 +3,7 @@ ###### sdl error ```graphql @server -schema @server(responseHeaders: [{key: "🤣", value: "a"}]) { +schema @server(headers: {custom: [{key: "🤣", value: "a"}]}) { query: Query } diff --git a/tests/snapshots/execution_spec__custom-headers.md_merged.snap b/tests/snapshots/execution_spec__custom-headers.md_merged.snap index a8118dc36a..a77f8116ac 100644 --- a/tests/snapshots/execution_spec__custom-headers.md_merged.snap +++ b/tests/snapshots/execution_spec__custom-headers.md_merged.snap @@ -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 } diff --git a/tests/snapshots/execution_spec__test-response-header-value.md_errors.snap b/tests/snapshots/execution_spec__test-response-header-value.md_errors.snap index 81470e2462..e525f70886 100644 --- a/tests/snapshots/execution_spec__test-response-header-value.md_errors.snap +++ b/tests/snapshots/execution_spec__test-response-header-value.md_errors.snap @@ -8,7 +8,8 @@ expression: errors "trace": [ "schema", "@server", - "responseHeaders" + "headers", + "custom" ], "description": null } diff --git a/tests/snapshots/execution_spec__test-response-headers-multi.md_errors.snap b/tests/snapshots/execution_spec__test-response-headers-multi.md_errors.snap index 9c77808677..d12fe47ea0 100644 --- a/tests/snapshots/execution_spec__test-response-headers-multi.md_errors.snap +++ b/tests/snapshots/execution_spec__test-response-headers-multi.md_errors.snap @@ -8,7 +8,8 @@ expression: errors "trace": [ "schema", "@server", - "responseHeaders" + "headers", + "custom" ], "description": null }, @@ -17,7 +18,8 @@ expression: errors "trace": [ "schema", "@server", - "responseHeaders" + "headers", + "custom" ], "description": null }, @@ -26,7 +28,8 @@ expression: errors "trace": [ "schema", "@server", - "responseHeaders" + "headers", + "custom" ], "description": null }, @@ -35,7 +38,8 @@ expression: errors "trace": [ "schema", "@server", - "responseHeaders" + "headers", + "custom" ], "description": null } diff --git a/tests/snapshots/execution_spec__test-response-headers-name.md_errors.snap b/tests/snapshots/execution_spec__test-response-headers-name.md_errors.snap index 283f255c52..29374aeb1d 100644 --- a/tests/snapshots/execution_spec__test-response-headers-name.md_errors.snap +++ b/tests/snapshots/execution_spec__test-response-headers-name.md_errors.snap @@ -8,7 +8,8 @@ expression: errors "trace": [ "schema", "@server", - "responseHeaders" + "headers", + "custom" ], "description": null } From fa8369824976ae379881bbeb4b8230c46f1708f7 Mon Sep 17 00:00:00 2001 From: Wesley Matos Date: Tue, 12 Mar 2024 10:20:48 -0300 Subject: [PATCH 07/19] docs: update docs via `./lint.sh --mode=fix` --- generated/.tailcallrc.graphql | 12 ++++++------ generated/.tailcallrc.schema.json | 14 +++++++------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/generated/.tailcallrc.graphql b/generated/.tailcallrc.graphql index aaa9fd8b59..49f2fce523 100644 --- a/generated/.tailcallrc.graphql +++ b/generated/.tailcallrc.graphql @@ -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`. """ @@ -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, diff --git a/generated/.tailcallrc.schema.json b/generated/.tailcallrc.schema.json index 757538a1ab..06b3c41086 100644 --- a/generated/.tailcallrc.schema.json +++ b/generated/.tailcallrc.schema.json @@ -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" + } } } }, @@ -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": [ From a7841327850c0d898bf10bf463e3175c6cb46006 Mon Sep 17 00:00:00 2001 From: Wesley Matos Date: Tue, 12 Mar 2024 10:33:56 -0300 Subject: [PATCH 08/19] test: add case for overriding existing headers --- tests/execution/test-response-header-merge.md | 31 +++++++++++++++++++ ..._test-response-header-merge.md_merged.snap | 16 ++++++++++ 2 files changed, 47 insertions(+) create mode 100644 tests/execution/test-response-header-merge.md create mode 100644 tests/snapshots/execution_spec__test-response-header-merge.md_merged.snap diff --git a/tests/execution/test-response-header-merge.md b/tests/execution/test-response-header-merge.md new file mode 100644 index 0000000000..8750494391 --- /dev/null +++ b/tests/execution/test-response-header-merge.md @@ -0,0 +1,31 @@ +# test-response-header-value + +```graphql @server +schema @server(headers: {custom: [{key: "a", value: "a"}]}) { + 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"}) +} +``` diff --git a/tests/snapshots/execution_spec__test-response-header-merge.md_merged.snap b/tests/snapshots/execution_spec__test-response-header-merge.md_merged.snap new file mode 100644 index 0000000000..eecdb5cd60 --- /dev/null +++ b/tests/snapshots/execution_spec__test-response-header-merge.md_merged.snap @@ -0,0 +1,16 @@ +--- +source: tests/execution_spec.rs +expression: merged +--- +schema @server(headers: {custom: [{key: "a", value: "b"}]}) @upstream { + query: Query +} + +type Query { + user: User @const(data: {name: "John"}) +} + +type User { + age: Int + name: String +} From c7edd11f42f6acbe3d1de5b5f68c81238533c74b Mon Sep 17 00:00:00 2001 From: Wesley Matos Date: Wed, 13 Mar 2024 10:25:49 -0300 Subject: [PATCH 09/19] refactor(`key_values.rs`): move key_value merge right from `server.rs` --- src/config/key_values.rs | 14 ++++++++++++++ src/config/server.rs | 20 +++----------------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/config/key_values.rs b/src/config/key_values.rs index 8e48f48de1..b15f048f41 100644 --- a/src/config/key_values.rs +++ b/src/config/key_values.rs @@ -26,6 +26,20 @@ pub struct KeyValue { pub value: String, } +pub fn merge_key_value_vecs(current: &[KeyValue], other: &[KeyValue]) -> Vec { + other + .iter() + .fold(current.iter().cloned().collect(), |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 + }) +} + impl Serialize for KeyValues { fn serialize(&self, serializer: S) -> Result where diff --git a/src/config/server.rs b/src/config/server.rs index c1ebfadede..0726aa7a60 100644 --- a/src/config/server.rs +++ b/src/config/server.rs @@ -6,6 +6,8 @@ use crate::config::headers::Headers; use crate::config::KeyValue; use crate::is_default; +use super::merge_key_value_vecs; + #[derive(Serialize, Deserialize, Clone, Debug, Default, PartialEq, Eq, schemars::JsonSchema)] #[serde(rename_all = "camelCase")] /// The `@server` directive, when applied at the schema level, offers a @@ -179,22 +181,6 @@ impl Server { self.pipeline_flush.unwrap_or(true) } - fn merge_key_value_iterators( - &self, - iter: std::slice::Iter<'_, KeyValue>, - other: std::slice::Iter<'_, KeyValue>, - ) -> Vec { - other.fold(iter.cloned().collect(), |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 - }) - } - pub fn merge_right(mut self, other: Self) -> Self { self.apollo_tracing = other.apollo_tracing.or(self.apollo_tracing); if let Some(headers) = other.headers { @@ -220,7 +206,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 = self.merge_key_value_iterators(self.vars.iter(), other.vars.iter()); + self.vars = merge_key_value_vecs(&self.vars, &other.vars); self.version = other.version.or(self.version); self.pipeline_flush = other.pipeline_flush.or(self.pipeline_flush); self.script = other.script.or(self.script); From 79df0ee45ad6477b54fe68b27f3f2f1e73703087 Mon Sep 17 00:00:00 2001 From: Wesley Matos Date: Wed, 13 Mar 2024 10:26:42 -0300 Subject: [PATCH 10/19] refactor(`headers.rs`): move headers merging from `server.rs` --- src/config/headers.rs | 17 +++++++++++++++++ src/config/server.rs | 26 ++++++++++++++------------ 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/src/config/headers.rs b/src/config/headers.rs index d98b74bdee..b7cdad25a2 100644 --- a/src/config/headers.rs +++ b/src/config/headers.rs @@ -24,3 +24,20 @@ impl Headers { self.cache_control.unwrap_or(false) } } + +pub fn merge_headers(current: Option, other: Option) -> Option { + 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 +} diff --git a/src/config/server.rs b/src/config/server.rs index 0726aa7a60..ea9d7216cb 100644 --- a/src/config/server.rs +++ b/src/config/server.rs @@ -6,7 +6,7 @@ use crate::config::headers::Headers; use crate::config::KeyValue; use crate::is_default; -use super::merge_key_value_vecs; +use super::{merge_headers, merge_key_value_vecs}; #[derive(Serialize, Deserialize, Clone, Debug, Default, PartialEq, Eq, schemars::JsonSchema)] #[serde(rename_all = "camelCase")] @@ -183,17 +183,7 @@ impl Server { pub fn merge_right(mut self, other: Self) -> Self { self.apollo_tracing = other.apollo_tracing.or(self.apollo_tracing); - if let Some(headers) = other.headers { - if let Some(mut self_headers) = self.headers.clone() { - self_headers.cache_control = headers.cache_control.or(self_headers.cache_control); - self_headers.custom = self - .merge_key_value_iterators(self_headers.custom.iter(), headers.custom.iter()); - - self.headers = Some(self_headers); - } else { - self.headers = Some(headers); - } - } + self.headers = merge_headers(self.headers, other.headers); self.graphiql = other.graphiql.or(self.graphiql); self.introspection = other.introspection.or(self.introspection); self.query_validation = other.query_validation.or(self.query_validation); @@ -206,6 +196,18 @@ 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.iter().cloned().collect(), |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); self.version = other.version.or(self.version); self.pipeline_flush = other.pipeline_flush.or(self.pipeline_flush); From 9307219bec2e72ab1917a48243317e21ee68eae4 Mon Sep 17 00:00:00 2001 From: Wesley Matos Date: Wed, 13 Mar 2024 10:27:08 -0300 Subject: [PATCH 11/19] feat: allow duplicated headers --- src/blueprint/server.rs | 2 +- src/config/server.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/blueprint/server.rs b/src/blueprint/server.rs index 7a0cbc6589..51823752cb 100644 --- a/src/blueprint/server.rs +++ b/src/blueprint/server.rs @@ -160,7 +160,7 @@ fn validate_hostname(hostname: String) -> Valid { } } -fn handle_response_headers(resp_headers: BTreeMap) -> Valid { +fn handle_response_headers(resp_headers: Vec<(String, String)>) -> Valid { Valid::from_iter(resp_headers.iter(), |(k, v)| { let name = Valid::from( HeaderName::from_bytes(k.as_bytes()) diff --git a/src/config/server.rs b/src/config/server.rs index ea9d7216cb..9567152a4a 100644 --- a/src/config/server.rs +++ b/src/config/server.rs @@ -161,11 +161,11 @@ impl Server { .collect() } - pub fn get_response_headers(&self) -> BTreeMap { + pub fn get_response_headers(&self) -> Vec<(String, String)> { self.headers .as_ref() .map(|h| h.custom.clone()) - .map_or(BTreeMap::new(), |headers| { + .map_or(Vec::new(), |headers| { headers .iter() .map(|kv| (kv.key.clone(), kv.value.clone())) From 44c90ec4c5d4362b811a58cdcb9a8c68dd10dafe Mon Sep 17 00:00:00 2001 From: Wesley Matos Date: Wed, 13 Mar 2024 10:27:18 -0300 Subject: [PATCH 12/19] test: update snapshot --- .../execution_spec__test-response-header-merge.md_merged.snap | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/snapshots/execution_spec__test-response-header-merge.md_merged.snap b/tests/snapshots/execution_spec__test-response-header-merge.md_merged.snap index eecdb5cd60..0763dc8528 100644 --- a/tests/snapshots/execution_spec__test-response-header-merge.md_merged.snap +++ b/tests/snapshots/execution_spec__test-response-header-merge.md_merged.snap @@ -2,7 +2,7 @@ source: tests/execution_spec.rs expression: merged --- -schema @server(headers: {custom: [{key: "a", value: "b"}]}) @upstream { +schema @server(headers: {custom: [{key: "a", value: "a"}, {key: "a", value: "b"}]}) @upstream { query: Query } From 4481d78bfdf75cadea089f27e1daad42f3bc2d70 Mon Sep 17 00:00:00 2001 From: Wesley Matos Date: Wed, 13 Mar 2024 10:28:19 -0300 Subject: [PATCH 13/19] refactor: remove unused method --- src/config/server.rs | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/src/config/server.rs b/src/config/server.rs index 644db1cb82..9567152a4a 100644 --- a/src/config/server.rs +++ b/src/config/server.rs @@ -181,22 +181,6 @@ impl Server { self.pipeline_flush.unwrap_or(true) } - fn merge_key_value_iterators( - &self, - iter: std::slice::Iter<'_, KeyValue>, - other: std::slice::Iter<'_, KeyValue>, - ) -> Vec { - other.fold(iter.cloned().collect(), |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 - }) - } - pub fn merge_right(mut self, other: Self) -> Self { self.apollo_tracing = other.apollo_tracing.or(self.apollo_tracing); self.headers = merge_headers(self.headers, other.headers); From 108ddf2a3f6036eba7bb31cc9e0fe0c2178db5c8 Mon Sep 17 00:00:00 2001 From: Wesley Matos Date: Wed, 13 Mar 2024 10:30:19 -0300 Subject: [PATCH 14/19] style: run `./lint.sh --mode=fix` --- src/config/key_values.rs | 2 +- src/config/server.rs | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/config/key_values.rs b/src/config/key_values.rs index b15f048f41..b858b78601 100644 --- a/src/config/key_values.rs +++ b/src/config/key_values.rs @@ -29,7 +29,7 @@ pub struct KeyValue { pub fn merge_key_value_vecs(current: &[KeyValue], other: &[KeyValue]) -> Vec { other .iter() - .fold(current.iter().cloned().collect(), |mut acc, kv| { + .fold(current.to_vec(), |mut acc, kv| { let position = acc.iter().position(|x| x.key == kv.key); if let Some(pos) = position { acc[pos] = kv.clone(); diff --git a/src/config/server.rs b/src/config/server.rs index 9567152a4a..74bf828dbf 100644 --- a/src/config/server.rs +++ b/src/config/server.rs @@ -2,12 +2,11 @@ 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; -use super::{merge_headers, merge_key_value_vecs}; - #[derive(Serialize, Deserialize, Clone, Debug, Default, PartialEq, Eq, schemars::JsonSchema)] #[serde(rename_all = "camelCase")] /// The `@server` directive, when applied at the schema level, offers a @@ -199,7 +198,7 @@ impl Server { self.vars = other .vars .iter() - .fold(self.vars.iter().cloned().collect(), |mut acc, kv| { + .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(); From bb1acd54aff5e97c99d40ab73b4b204f8ffff417 Mon Sep 17 00:00:00 2001 From: Wesley Matos Date: Wed, 13 Mar 2024 10:33:01 -0300 Subject: [PATCH 15/19] style: run `./lint.sh --mode=fix` --- src/config/key_values.rs | 20 +++++++++----------- src/config/server.rs | 21 +++++++++------------ 2 files changed, 18 insertions(+), 23 deletions(-) diff --git a/src/config/key_values.rs b/src/config/key_values.rs index b858b78601..d7e973f0fa 100644 --- a/src/config/key_values.rs +++ b/src/config/key_values.rs @@ -27,17 +27,15 @@ pub struct KeyValue { } pub fn merge_key_value_vecs(current: &[KeyValue], other: &[KeyValue]) -> Vec { - other - .iter() - .fold(current.to_vec(), |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 - }) + other.iter().fold(current.to_vec(), |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 + }) } impl Serialize for KeyValues { diff --git a/src/config/server.rs b/src/config/server.rs index 74bf828dbf..a37157f6c1 100644 --- a/src/config/server.rs +++ b/src/config/server.rs @@ -195,18 +195,15 @@ 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.to_vec(), |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 = 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(); + } else { + acc.push(kv.clone()); + }; + acc + }); self.vars = merge_key_value_vecs(&self.vars, &other.vars); self.version = other.version.or(self.version); self.pipeline_flush = other.pipeline_flush.or(self.pipeline_flush); From 367aa92837a11bbd0e06d68a55a59a3394be6125 Mon Sep 17 00:00:00 2001 From: Wesley Matos Date: Wed, 13 Mar 2024 10:44:17 -0300 Subject: [PATCH 16/19] refactor(`key_values.rs`): reduce complexity from O(n^2) to O(n) --- src/config/key_values.rs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/config/key_values.rs b/src/config/key_values.rs index d7e973f0fa..2564d58d4f 100644 --- a/src/config/key_values.rs +++ b/src/config/key_values.rs @@ -27,15 +27,16 @@ pub struct KeyValue { } pub fn merge_key_value_vecs(current: &[KeyValue], other: &[KeyValue]) -> Vec { - other.iter().fold(current.to_vec(), |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 - }) + let mut map: BTreeMap = current + .iter() + .map(|kv| (kv.key.clone(), kv.value.clone())) + .collect(); + for kv in other { + map.entry(kv.key.clone()).or_insert(kv.value.clone()); + } + map.into_iter() + .map(|(key, value)| KeyValue { key, value }) + .collect() } impl Serialize for KeyValues { From 2ece27b39196a2334c3c554adcaf32f2f6054d01 Mon Sep 17 00:00:00 2001 From: Wesley Matos Date: Wed, 13 Mar 2024 12:13:47 -0300 Subject: [PATCH 17/19] fix: implement map usage correctly --- src/config/key_values.rs | 67 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 61 insertions(+), 6 deletions(-) diff --git a/src/config/key_values.rs b/src/config/key_values.rs index 2564d58d4f..2e17eb614a 100644 --- a/src/config/key_values.rs +++ b/src/config/key_values.rs @@ -27,16 +27,16 @@ pub struct KeyValue { } pub fn merge_key_value_vecs(current: &[KeyValue], other: &[KeyValue]) -> Vec { - let mut map: BTreeMap = current + let mut acc: BTreeMap = current .iter() - .map(|kv| (kv.key.clone(), kv.value.clone())) + .map(|kv| (kv.key.clone(), kv.clone())) .collect(); + for kv in other { - map.entry(kv.key.clone()).or_insert(kv.value.clone()); + acc.insert(kv.key.clone(), kv.clone()); } - map.into_iter() - .map(|(key, value)| KeyValue { key, value }) - .collect() + + acc.into_iter().map(|(_, v)| v).collect() } impl Serialize for KeyValues { @@ -110,4 +110,59 @@ mod tests { // Using the deref trait assert_eq!(kv["a"], "b"); } + + #[cfg(test)] + mod merge_key_value_vecs_tests { + use super::*; + + #[test] + fn test_merge_with_both_empty() { + let current = vec![]; + let other = vec![]; + let result = merge_key_value_vecs(¤t, &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(¤t, &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(¤t, &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(¤t, &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(¤t, &other); + assert_eq!(result.len(), 1); + assert_eq!(result[0].key, "key1"); + assert_eq!(result[0].value, "value2"); // `other`'s value should overwrite `current`'s value + } + } } From 4418df91122d189e756f2fcc23a0af7fcbdcce1b Mon Sep 17 00:00:00 2001 From: Wesley Matos Date: Wed, 13 Mar 2024 12:16:44 -0300 Subject: [PATCH 18/19] refactor: remove unused test submodule --- src/config/key_values.rs | 101 +++++++++++++++++++-------------------- 1 file changed, 48 insertions(+), 53 deletions(-) diff --git a/src/config/key_values.rs b/src/config/key_values.rs index 2e17eb614a..5773dfadff 100644 --- a/src/config/key_values.rs +++ b/src/config/key_values.rs @@ -111,58 +111,53 @@ mod tests { assert_eq!(kv["a"], "b"); } - #[cfg(test)] - mod merge_key_value_vecs_tests { - use super::*; - - #[test] - fn test_merge_with_both_empty() { - let current = vec![]; - let other = vec![]; - let result = merge_key_value_vecs(¤t, &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(¤t, &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(¤t, &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(¤t, &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(¤t, &other); - assert_eq!(result.len(), 1); - assert_eq!(result[0].key, "key1"); - assert_eq!(result[0].value, "value2"); // `other`'s value should overwrite `current`'s value - } + #[test] + fn test_merge_with_both_empty() { + let current = vec![]; + let other = vec![]; + let result = merge_key_value_vecs(¤t, &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(¤t, &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(¤t, &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(¤t, &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(¤t, &other); + assert_eq!(result.len(), 1); + assert_eq!(result[0].key, "key1"); + assert_eq!(result[0].value, "value2"); // `other`'s value should overwrite `current`'s value } } From 2478fe968bdd441f875b13d5b22f0c008a65cab5 Mon Sep 17 00:00:00 2001 From: Wesley Matos Date: Wed, 13 Mar 2024 12:22:03 -0300 Subject: [PATCH 19/19] refactor: avoid overusage of memory --- src/config/key_values.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/config/key_values.rs b/src/config/key_values.rs index 5773dfadff..c36c53fa8a 100644 --- a/src/config/key_values.rs +++ b/src/config/key_values.rs @@ -27,16 +27,16 @@ pub struct KeyValue { } pub fn merge_key_value_vecs(current: &[KeyValue], other: &[KeyValue]) -> Vec { - let mut acc: BTreeMap = current - .iter() - .map(|kv| (kv.key.clone(), kv.clone())) - .collect(); + let mut acc: BTreeMap<&String, &String> = + current.iter().map(|kv| (&kv.key, &kv.value)).collect(); for kv in other { - acc.insert(kv.key.clone(), kv.clone()); + acc.insert(&kv.key, &kv.value); } - acc.into_iter().map(|(_, v)| v).collect() + acc.iter() + .map(|(k, v)| KeyValue { key: k.to_string(), value: v.to_string() }) + .collect() } impl Serialize for KeyValues { @@ -158,6 +158,6 @@ mod tests { let result = merge_key_value_vecs(¤t, &other); assert_eq!(result.len(), 1); assert_eq!(result[0].key, "key1"); - assert_eq!(result[0].value, "value2"); // `other`'s value should overwrite `current`'s value + assert_eq!(result[0].value, "value2"); } }