Skip to content

Commit

Permalink
serialize query responses according to schema types (#53)
Browse files Browse the repository at this point in the history
This changes response serialization to use simple JSON instead of Extended JSON (except in cases of fields with the type `ExtendedJSON`).

Mutation responses have not been updated with this change yet - that will be in a follow-up PR.

This could be greatly simplified if we had an internal query request type that used copies of object type definitions in all places instead of object type names. I've got some thoughts on changing `v3_to_v2_query_request` to work with an internal type like that instead of with the v2 query request type. But that's a big change that will have to wait until later.
  • Loading branch information
hallettj authored Apr 26, 2024
1 parent 42a32a9 commit c654ec2
Show file tree
Hide file tree
Showing 39 changed files with 1,692 additions and 1,083 deletions.
10 changes: 5 additions & 5 deletions Cargo.lock

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

2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ resolver = "2"
ndc-sdk = { git = "https://github.com/hasura/ndc-sdk-rs.git" }
ndc-models = { git = "http://github.com/hasura/ndc-spec.git", tag = "v0.1.2" }

itertools = "^0.12.1"

# We have a fork of the mongodb driver with a fix for reading metadata from time
# series collections.
# See the upstream PR: https://github.com/mongodb/mongo-rust-driver/pull/1003
Expand Down
2 changes: 1 addition & 1 deletion crates/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ anyhow = "1.0.80"
clap = { version = "4.5.1", features = ["derive", "env"] }
futures-util = "0.3.28"
indexmap = { version = "1", features = ["serde"] } # must match the version that ndc-client uses
itertools = "^0.12.1"
itertools = { workspace = true }
serde = { version = "1.0", features = ["derive"] }
serde_json = { version = "1.0.113", features = ["raw_value"] }
thiserror = "1.0.57"
Expand Down
2 changes: 1 addition & 1 deletion crates/configuration/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ edition = "2021"
[dependencies]
anyhow = "1"
futures = "^0.3"
itertools = "^0.12"
itertools = { workspace = true }
mongodb = "2.8"
mongodb-support = { path = "../mongodb-support" }
ndc-models = { workspace = true }
Expand Down
2 changes: 1 addition & 1 deletion crates/dc-api-test-helpers/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@ edition = "2021"

[dependencies]
dc-api-types = { path = "../dc-api-types" }
itertools = "^0.10"
itertools = { workspace = true }
18 changes: 9 additions & 9 deletions crates/dc-api-test-helpers/src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ use dc_api_types::{Aggregate, Expression, Field, OrderBy, Query};

#[derive(Clone, Debug, Default)]
pub struct QueryBuilder {
aggregates: Option<Option<HashMap<String, Aggregate>>>,
aggregates_limit: Option<Option<i64>>,
fields: Option<Option<HashMap<String, Field>>>,
limit: Option<Option<i64>>,
offset: Option<Option<u64>>,
order_by: Option<Option<OrderBy>>,
aggregates: Option<HashMap<String, Aggregate>>,
aggregates_limit: Option<i64>,
fields: Option<HashMap<String, Field>>,
limit: Option<i64>,
offset: Option<u64>,
order_by: Option<OrderBy>,
predicate: Option<Expression>,
}

Expand All @@ -22,15 +22,15 @@ impl QueryBuilder {
where
I: IntoIterator<Item = (String, Field)>,
{
self.fields = Some(Some(fields.into_iter().collect()));
self.fields = Some(fields.into_iter().collect());
self
}

pub fn aggregates<I>(mut self, aggregates: I) -> Self
where
I: IntoIterator<Item = (String, Aggregate)>,
{
self.aggregates = Some(Some(aggregates.into_iter().collect()));
self.aggregates = Some(aggregates.into_iter().collect());
self
}

Expand All @@ -40,7 +40,7 @@ impl QueryBuilder {
}

pub fn order_by(mut self, order_by: OrderBy) -> Self {
self.order_by = Some(Some(order_by));
self.order_by = Some(order_by);
self
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/dc-api-types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ edition = "2021"
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
itertools = "^0.10"
itertools = { workspace = true }
nonempty = { version = "0.8.1", features = ["serialize"] }
once_cell = "1"
regex = "1"
Expand Down
42 changes: 10 additions & 32 deletions crates/dc-api-types/src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,49 +16,27 @@ pub struct Query {
#[serde(
rename = "aggregates",
default,
with = "::serde_with::rust::double_option",
skip_serializing_if = "Option::is_none"
)]
pub aggregates: Option<Option<::std::collections::HashMap<String, crate::Aggregate>>>,
pub aggregates: Option<::std::collections::HashMap<String, crate::Aggregate>>,
/// Optionally limit the maximum number of rows considered while applying aggregations. This limit does not apply to returned rows.
#[serde(
rename = "aggregates_limit",
default,
with = "::serde_with::rust::double_option",
skip_serializing_if = "Option::is_none"
)]
pub aggregates_limit: Option<Option<i64>>,
pub aggregates_limit: Option<i64>,
/// Fields of the query
#[serde(
rename = "fields",
default,
with = "::serde_with::rust::double_option",
skip_serializing_if = "Option::is_none"
)]
pub fields: Option<Option<::std::collections::HashMap<String, crate::Field>>>,
#[serde(rename = "fields", default, skip_serializing_if = "Option::is_none")]
pub fields: Option<::std::collections::HashMap<String, crate::Field>>,
/// Optionally limit the maximum number of returned rows. This limit does not apply to records considered while apply aggregations.
#[serde(
rename = "limit",
default,
with = "::serde_with::rust::double_option",
skip_serializing_if = "Option::is_none"
)]
pub limit: Option<Option<i64>>,
#[serde(rename = "limit", default, skip_serializing_if = "Option::is_none")]
pub limit: Option<i64>,
/// Optionally offset from the Nth result. This applies to both row and aggregation results.
#[serde(
rename = "offset",
default,
with = "::serde_with::rust::double_option",
skip_serializing_if = "Option::is_none"
)]
pub offset: Option<Option<u64>>,
#[serde(
rename = "order_by",
default,
with = "::serde_with::rust::double_option",
skip_serializing_if = "Option::is_none"
)]
pub order_by: Option<Option<crate::OrderBy>>,
#[serde(rename = "offset", default, skip_serializing_if = "Option::is_none")]
pub offset: Option<u64>,
#[serde(rename = "order_by", default, skip_serializing_if = "Option::is_none")]
pub order_by: Option<crate::OrderBy>,
#[serde(rename = "where", skip_serializing_if = "Option::is_none")]
pub r#where: Option<crate::Expression>,
}
Expand Down
28 changes: 6 additions & 22 deletions crates/dc-api-types/src/query_response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,29 +42,13 @@ pub struct ForEachRow {
pub query: RowSet,
}

/// A row set must contain either rows, or aggregates, or possibly both
#[skip_serializing_none]
#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
#[serde(untagged)]
pub enum RowSet {
Aggregate {
/// The results of the aggregates returned by the query
aggregates: HashMap<String, serde_json::Value>,
/// The rows returned by the query, corresponding to the query's fields
rows: Option<Vec<HashMap<String, ResponseFieldValue>>>,
},
Rows {
/// Rows returned by a query that did not request aggregates.
rows: Vec<HashMap<String, ResponseFieldValue>>,
},
}

impl Default for RowSet {
fn default() -> Self {
RowSet::Rows {
rows: Default::default(),
}
}
#[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize)]
pub struct RowSet {
/// The results of the aggregates returned by the query
pub aggregates: Option<HashMap<String, serde_json::Value>>,
/// The rows returned by the query, corresponding to the query's fields
pub rows: Option<Vec<HashMap<String, ResponseFieldValue>>>,
}

#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
Expand Down
1 change: 1 addition & 0 deletions crates/integration-tests/src/tests/local_relationship.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ async fn joins_local_relationships() -> anyhow::Result<()> {
user {
email
comments(limit: 2, order_by: {id: Asc}) {
id
email
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,43 +5,53 @@ expression: "query(r#\"\n query Movies {\n movie
data:
movies:
- imdb:
rating: 6.2
rating:
$numberDouble: "6.2"
votes: 1189
title: Blacksmith Scene
- imdb:
rating: 7.4
rating:
$numberDouble: "7.4"
votes: 9847
title: The Great Train Robbery
- imdb:
rating: 7.1
rating:
$numberDouble: "7.1"
votes: 448
title: The Land Beyond the Sunset
- imdb:
rating: 6.6
rating:
$numberDouble: "6.6"
votes: 1375
title: A Corner in Wheat
- imdb:
rating: 7.3
rating:
$numberDouble: "7.3"
votes: 1034
title: "Winsor McCay, the Famous Cartoonist of the N.Y. Herald and His Moving Comics"
- imdb:
rating: 6
rating:
$numberInt: "6"
votes: 371
title: Traffic in Souls
- imdb:
rating: 7.3
rating:
$numberDouble: "7.3"
votes: 1837
title: Gertie the Dinosaur
- imdb:
rating: 5.8
rating:
$numberDouble: "5.8"
votes: 223
title: In the Land of the Head Hunters
- imdb:
rating: 7.6
rating:
$numberDouble: "7.6"
votes: 744
title: The Perils of Pauline
- imdb:
rating: 6.8
rating:
$numberDouble: "6.8"
votes: 15715
title: The Birth of a Nation
errors: ~
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
---
source: crates/integration-tests/src/tests/local_relationship.rs
expression: "query(r#\"\n query {\n movies(limit: 2, order_by: {title: Asc}, where: {title: {_iregex: \"Rear\"}}) {\n id\n title\n comments(limit: 2, order_by: {id: Asc}) {\n email\n text\n movie {\n id\n title\n }\n user {\n email\n comments(limit: 2, order_by: {id: Asc}) {\n email\n text\n user {\n email\n comments(limit: 2, order_by: {id: Asc}) {\n email\n }\n }\n }\n }\n }\n }\n }\n \"#).variables(json!({\n \"limit\": 11, \"movies_limit\": 2\n })).run().await?"
expression: "query(r#\"\n query {\n movies(limit: 2, order_by: {title: Asc}, where: {title: {_iregex: \"Rear\"}}) {\n id\n title\n comments(limit: 2, order_by: {id: Asc}) {\n email\n text\n movie {\n id\n title\n }\n user {\n email\n comments(limit: 2, order_by: {id: Asc}) {\n email\n text\n user {\n email\n comments(limit: 2, order_by: {id: Asc}) {\n id\n email\n }\n }\n }\n }\n }\n }\n }\n \"#).variables(json!({\n \"limit\": 11, \"movies_limit\": 2\n })).run().await?"
---
data:
movies:
- comments:
- email: iain_glen@gameofthron.es
movie:
id:
$oid: 573a1398f29313caabceb0b1
id: 573a1398f29313caabceb0b1
title: A Night in the Life of Jimmy Reardon
text: Debitis tempore cum natus quaerat dolores quibusdam perferendis. Pariatur aspernatur officia libero quod pariatur nobis neque. Maiores non ipsam iste repellendus distinctio praesentium iure.
user:
Expand All @@ -18,24 +17,26 @@ data:
user:
comments:
- email: iain_glen@gameofthron.es
id: 5a9427648b0beebeb69579f3
- email: iain_glen@gameofthron.es
id: 5a9427648b0beebeb6957b0f
email: iain_glen@gameofthron.es
- email: iain_glen@gameofthron.es
text: Impedit consectetur ex cupiditate enim. Placeat assumenda reiciendis iste neque similique nesciunt aperiam.
user:
comments:
- email: iain_glen@gameofthron.es
id: 5a9427648b0beebeb69579f3
- email: iain_glen@gameofthron.es
id: 5a9427648b0beebeb6957b0f
email: iain_glen@gameofthron.es
email: iain_glen@gameofthron.es
id:
$oid: 573a1398f29313caabceb0b1
id: 573a1398f29313caabceb0b1
title: A Night in the Life of Jimmy Reardon
- comments:
- email: owen_teale@gameofthron.es
movie:
id:
$oid: 573a1394f29313caabcdfa00
id: 573a1394f29313caabcdfa00
title: Rear Window
text: Nobis corporis rem hic ipsa cum impedit. Esse nihil cum est minima ducimus temporibus minima. Sed reprehenderit tempore similique nam. Ipsam nesciunt veniam aut amet ut.
user:
Expand All @@ -45,17 +46,20 @@ data:
user:
comments:
- email: owen_teale@gameofthron.es
id: 5a9427648b0beebeb6957b44
- email: owen_teale@gameofthron.es
id: 5a9427648b0beebeb6957cf6
email: owen_teale@gameofthron.es
- email: owen_teale@gameofthron.es
text: Repudiandae repellat quia officiis. Quidem voluptatum vel id itaque et. Corrupti corporis magni voluptas quae itaque fugiat quae.
user:
comments:
- email: owen_teale@gameofthron.es
id: 5a9427648b0beebeb6957b44
- email: owen_teale@gameofthron.es
id: 5a9427648b0beebeb6957cf6
email: owen_teale@gameofthron.es
email: owen_teale@gameofthron.es
id:
$oid: 573a1394f29313caabcdfa00
id: 573a1394f29313caabcdfa00
title: Rear Window
errors: ~
2 changes: 1 addition & 1 deletion crates/mongodb-agent-common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ futures-util = "0.3.28"
http = "^0.2"
indexmap = { version = "1", features = ["serde"] } # must match the version that ndc-client uses
indent = "^0.1"
itertools = "^0.10"
itertools = { workspace = true }
mongodb = "2.8"
once_cell = "1"
regex = "1"
Expand Down
Loading

0 comments on commit c654ec2

Please sign in to comment.