From a9886fea920c9793eec76c99072bc975079add6e Mon Sep 17 00:00:00 2001 From: David Overton Date: Fri, 5 Apr 2024 08:49:03 +1100 Subject: [PATCH] Avoid (incorrectly) deserializing already serialized v2 query response (#27) * Avoid (incorrectly) deserializing already serialized v2 query response * Fix compilation error * More build fixes * Fix formatting * Add changelog --- CHANGELOG.md | 1 + Cargo.lock | 1 + arion-compose/project-ndc-test.nix | 1 + crates/mongodb-connector/Cargo.toml | 1 + .../mongodb-connector/src/mongo_connector.rs | 31 +++++++++++-------- 5 files changed, 22 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c9e05c7..8b9ec2d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ This changelog documents the changes between release versions. ## [Unreleased] +- Fix bug in v2 to v3 conversion of query responses containing nested objects ([PR #27](https://github.com/hasura/ndc-mongodb/pull/27)) ## [0.0.3] - 2024-03-28 - Use separate schema files for each collection ([PR #14](https://github.com/hasura/ndc-mongodb/pull/14)) diff --git a/Cargo.lock b/Cargo.lock index 857e971c..7a54a570 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1537,6 +1537,7 @@ version = "0.1.0" dependencies = [ "anyhow", "async-trait", + "bytes", "configuration", "dc-api", "dc-api-test-helpers", diff --git a/arion-compose/project-ndc-test.nix b/arion-compose/project-ndc-test.nix index 79839f0a..541a0cf0 100644 --- a/arion-compose/project-ndc-test.nix +++ b/arion-compose/project-ndc-test.nix @@ -11,6 +11,7 @@ in inherit pkgs; command = "test"; database-uri = "mongodb://mongodb:${mongodb-port}/chinook"; + service.depends_on.mongodb.condition = "service_healthy"; }; mongodb = import ./service-mongodb.nix { diff --git a/crates/mongodb-connector/Cargo.toml b/crates/mongodb-connector/Cargo.toml index e89e8392..fa8333f7 100644 --- a/crates/mongodb-connector/Cargo.toml +++ b/crates/mongodb-connector/Cargo.toml @@ -6,6 +6,7 @@ edition = "2021" [dependencies] anyhow = "1" async-trait = "^0.1" +bytes = "^1" configuration = { path = "../configuration" } dc-api = { path = "../dc-api" } dc-api-types = { path = "../dc-api-types" } diff --git a/crates/mongodb-connector/src/mongo_connector.rs b/crates/mongodb-connector/src/mongo_connector.rs index 79460304..bb19504a 100644 --- a/crates/mongodb-connector/src/mongo_connector.rs +++ b/crates/mongodb-connector/src/mongo_connector.rs @@ -2,6 +2,7 @@ use std::path::Path; use anyhow::anyhow; use async_trait::async_trait; +use bytes::Bytes; use configuration::Configuration; use mongodb_agent_common::{ explain::explain_query, health::check_health, interface_types::MongoConfig, @@ -9,7 +10,8 @@ use mongodb_agent_common::{ }; use ndc_sdk::{ connector::{ - Connector, ConnectorSetup, ExplainError, FetchMetricsError, HealthError, InitializationError, MutationError, ParseError, QueryError, SchemaError + Connector, ConnectorSetup, ExplainError, FetchMetricsError, HealthError, + InitializationError, MutationError, ParseError, QueryError, SchemaError, }, json_response::JsonResponse, models::{ @@ -23,7 +25,8 @@ use crate::{ api_type_conversions::{ v2_to_v3_explain_response, v2_to_v3_query_response, v3_to_v2_query_request, QueryContext, }, - error_mapping::{mongo_agent_error_to_explain_error, mongo_agent_error_to_query_error}, schema, + error_mapping::{mongo_agent_error_to_explain_error, mongo_agent_error_to_query_error}, + schema, }; use crate::{capabilities::mongo_capabilities_response, mutation::handle_mutation_request}; @@ -159,16 +162,18 @@ impl Connector for MongoConnector { .await .map_err(mongo_agent_error_to_query_error)?; - // TODO: This requires parsing and reserializing the response from MongoDB. We can avoid - // this by passing a response format enum to the query pipeline builder that will format - // responses differently for v3 vs v2. MVC-7 - let response = response_json - .into_value() - .map_err(|e| QueryError::Other(Box::new(e)))?; - - // TODO: If we are able to push v3 response formatting to the MongoDB aggregation pipeline - // then we can switch to using `map_unserialized` here to avoid deserializing and - // reserializing the response. MVC-7 - Ok(v2_to_v3_query_response(response).into()) + match response_json { + dc_api::JsonResponse::Value(v2_response) => { + Ok(JsonResponse::Value(v2_to_v3_query_response(v2_response))) + } + dc_api::JsonResponse::Serialized(bytes) => { + let v2_value: serde_json::Value = serde_json::de::from_slice(&bytes) + .map_err(|e| QueryError::Other(Box::new(e)))?; + let v3_bytes: Bytes = serde_json::to_vec(&vec![v2_value]) + .map_err(|e| QueryError::Other(Box::new(e)))? + .into(); + Ok(JsonResponse::Serialized(v3_bytes)) + } + } } }