Skip to content

Commit

Permalink
Avoid (incorrectly) deserializing already serialized v2 query response (
Browse files Browse the repository at this point in the history
#27)

* Avoid (incorrectly) deserializing already serialized v2 query response

* Fix compilation error

* More build fixes

* Fix formatting

* Add changelog
  • Loading branch information
dmoverton authored Apr 4, 2024
1 parent e755b1e commit a9886fe
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions arion-compose/project-ndc-test.nix
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions crates/mongodb-connector/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
31 changes: 18 additions & 13 deletions crates/mongodb-connector/src/mongo_connector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,16 @@ 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,
query::handle_query_request,
};
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::{
Expand All @@ -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};

Expand Down Expand Up @@ -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))
}
}
}
}

0 comments on commit a9886fe

Please sign in to comment.