From 02d26c191b921385258afbecd22617d8d9f2d5ca Mon Sep 17 00:00:00 2001 From: Gil Mizrahi Date: Wed, 14 Feb 2024 12:00:11 +0200 Subject: [PATCH] update ndc-spec from 0.1.0-rc.13 to 0.1.0-rc.14 (#93) --- Cargo.lock | 4 +- Dockerfile | 2 +- rust-connector-sdk/Cargo.toml | 4 +- rust-connector-sdk/src/connector.rs | 22 +++- rust-connector-sdk/src/connector/example.rs | 18 ++- rust-connector-sdk/src/default_main.rs | 16 ++- .../src/default_main/v2_compat.rs | 102 +++++++++-------- rust-connector-sdk/src/routes.rs | 107 +++++++++++------- 8 files changed, 166 insertions(+), 109 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2b0214e4..0ccd8284 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1049,7 +1049,7 @@ dependencies = [ [[package]] name = "ndc-client" version = "0.1.0" -source = "git+http://github.com/hasura/ndc-spec.git?tag=v0.1.0-rc.13#1f9b2a996ad74ac4bc97a783c4d014a3fd46b08e" +source = "git+http://github.com/hasura/ndc-spec.git?tag=v0.1.0-rc.14#cd24992ea77010e1ef2dff18f7d5656fb0546f3b" dependencies = [ "async-trait", "indexmap 2.1.0", @@ -1104,7 +1104,7 @@ dependencies = [ [[package]] name = "ndc-test" version = "0.1.0" -source = "git+http://github.com/hasura/ndc-spec.git?tag=v0.1.0-rc.13#1f9b2a996ad74ac4bc97a783c4d014a3fd46b08e" +source = "git+http://github.com/hasura/ndc-spec.git?tag=v0.1.0-rc.14#cd24992ea77010e1ef2dff18f7d5656fb0546f3b" dependencies = [ "async-trait", "clap", diff --git a/Dockerfile b/Dockerfile index bcf1b80e..0fb9a76a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM rust:1.70.0-slim-buster AS build +FROM rust:1.76.0-slim-buster AS build WORKDIR app diff --git a/rust-connector-sdk/Cargo.toml b/rust-connector-sdk/Cargo.toml index 2b16b735..7763a8b5 100644 --- a/rust-connector-sdk/Cargo.toml +++ b/rust-connector-sdk/Cargo.toml @@ -13,8 +13,8 @@ path = "bin/main.rs" [dependencies] gdc_rust_types = { git = "https://github.com/hasura/gdc_rust_types.git", rev = "3273434" } -ndc-client = { git = "http://github.com/hasura/ndc-spec.git", tag = "v0.1.0-rc.13" } -ndc-test = { git = "http://github.com/hasura/ndc-spec.git", tag = "v0.1.0-rc.13" } +ndc-client = { git = "http://github.com/hasura/ndc-spec.git", tag = "v0.1.0-rc.14" } +ndc-test = { git = "http://github.com/hasura/ndc-spec.git", tag = "v0.1.0-rc.14" } async-trait = "^0.1.74" axum = "^0.6.20" diff --git a/rust-connector-sdk/src/connector.rs b/rust-connector-sdk/src/connector.rs index 83b584cb..593bef22 100644 --- a/rust-connector-sdk/src/connector.rs +++ b/rust-connector-sdk/src/connector.rs @@ -105,7 +105,7 @@ pub enum QueryError { /// Errors which occur when explaining a query. /// -/// See [`Connector::explain`]. +/// See [`Connector::query_explain`, `Connector::mutation_explain`]. #[derive(Debug, Error)] pub enum ExplainError { /// The request was invalid or did not match the @@ -123,7 +123,7 @@ pub enum ExplainError { /// or just an unimplemented feature. #[error("unsupported operation: {0}")] UnsupportedOperation(String), - #[error("error explaining query: {0}")] + #[error("explain error: {0}")] Other(#[from] Box), } @@ -163,8 +163,8 @@ pub enum MutationError { /// /// /// It provides methods which implement the standard endpoints -/// defined by the specification: capabilities, schema, query, mutation -/// and explain. +/// defined by the specification: capabilities, schema, query, mutation, +/// query/explain, and mutation/explain. /// /// In addition, it introduces names for types to manage /// state and configuration (if any), and provides any necessary context @@ -269,14 +269,24 @@ pub trait Connector { /// Explain a query by creating an execution plan /// - /// This function implements the [explain endpoint](https://hasura.github.io/ndc-spec/specification/explain.html) + /// This function implements the [query/explain endpoint](https://hasura.github.io/ndc-spec/specification/explain.html) /// from the NDC specification. - async fn explain( + async fn query_explain( configuration: &Self::Configuration, state: &Self::State, request: models::QueryRequest, ) -> Result, ExplainError>; + /// Explain a mutation by creating an execution plan + /// + /// This function implements the [mutation/explain endpoint](https://hasura.github.io/ndc-spec/specification/explain.html) + /// from the NDC specification. + async fn mutation_explain( + configuration: &Self::Configuration, + state: &Self::State, + request: models::MutationRequest, + ) -> Result, ExplainError>; + /// Execute a mutation /// /// This function implements the [mutation endpoint](https://hasura.github.io/ndc-spec/specification/mutations/index.html) diff --git a/rust-connector-sdk/src/connector/example.rs b/rust-connector-sdk/src/connector/example.rs index f00834dd..55df2604 100644 --- a/rust-connector-sdk/src/connector/example.rs +++ b/rust-connector-sdk/src/connector/example.rs @@ -52,13 +52,17 @@ impl Connector for Example { async fn get_capabilities() -> JsonResponse { models::CapabilitiesResponse { - versions: "^0.1.0".into(), + version: "0.1.0".into(), capabilities: models::Capabilities { - explain: None, relationships: None, query: models::QueryCapabilities { variables: None, aggregates: None, + explain: None, + }, + mutation: models::MutationCapabilities { + transactional: None, + explain: None, }, }, } @@ -84,7 +88,7 @@ impl Connector for Example { .into()) } - async fn explain( + async fn query_explain( _configuration: &Self::Configuration, _state: &Self::State, _request: models::QueryRequest, @@ -92,6 +96,14 @@ impl Connector for Example { todo!() } + async fn mutation_explain( + _configuration: &Self::Configuration, + _state: &Self::State, + _request: models::MutationRequest, + ) -> Result, ExplainError> { + todo!() + } + async fn mutation( _configuration: &Self::Configuration, _state: &Self::State, diff --git a/rust-connector-sdk/src/default_main.rs b/rust-connector-sdk/src/default_main.rs index 2d5ee6ea..5f068e33 100644 --- a/rust-connector-sdk/src/default_main.rs +++ b/rust-connector-sdk/src/default_main.rs @@ -294,8 +294,9 @@ where .route("/metrics", get(get_metrics::)) .route("/schema", get(get_schema::)) .route("/query", post(post_query::)) - .route("/explain", post(post_explain::)) + .route("/query/explain", post(post_query_explain::)) .route("/mutation", post(post_mutation::)) + .route("/mutation/explain", post(post_mutation_explain::)) .layer( TraceLayer::new_for_http() .make_span_with(make_span) @@ -373,7 +374,7 @@ where .route("/query", post(v2_compat::post_query::)) // .route("/mutation", post(v2_compat::post_mutation::)) // .route("/raw", post(v2_compat::post_raw::)) - .route("/explain", post(v2_compat::post_explain::)) + .route("/query/explain", post(v2_compat::post_explain::)) .layer( TraceLayer::new_for_http() .make_span_with(make_span) @@ -464,11 +465,18 @@ async fn get_schema( routes::get_schema::(&state.configuration).await } -async fn post_explain( +async fn post_query_explain( State(state): State>, WithRejection(Json(request), _): WithRejection, JsonRejection>, ) -> Result, (StatusCode, Json)> { - routes::post_explain::(&state.configuration, &state.state, request).await + routes::post_query_explain::(&state.configuration, &state.state, request).await +} + +async fn post_mutation_explain( + State(state): State>, + WithRejection(Json(request), _): WithRejection, JsonRejection>, +) -> Result, (StatusCode, Json)> { + routes::post_mutation_explain::(&state.configuration, &state.state, request).await } async fn post_mutation( diff --git a/rust-connector-sdk/src/default_main/v2_compat.rs b/rust-connector-sdk/src/default_main/v2_compat.rs index eb2bf198..88a8f1d0 100644 --- a/rust-connector-sdk/src/default_main/v2_compat.rs +++ b/rust-connector-sdk/src/default_main/v2_compat.rs @@ -67,17 +67,31 @@ pub async fn get_capabilities( models::Type::Named { name } => Some((function_name, name)), models::Type::Nullable { .. } => None, models::Type::Array { .. } => None, + models::Type::Predicate { .. } => None, }, ), )), comparison_operators: Some(IndexMap::from_iter( scalar_type.comparison_operators.into_iter().filter_map( - |(operator_name, comparison_operator)| match comparison_operator - .argument_type - { - models::Type::Named { name } => Some((operator_name, name)), - models::Type::Nullable { .. } => None, - models::Type::Array { .. } => None, + |(operator_name, comparison_operator)| match comparison_operator { + models::ComparisonOperatorDefinition::Equal => { + Some(("equal".to_string(), "equal".to_string())) + } + models::ComparisonOperatorDefinition::In => { + Some(("in".to_string(), "in".to_string())) + } + models::ComparisonOperatorDefinition::Custom { + argument_type: models::Type::Named { name }, + } => Some((operator_name, name)), + models::ComparisonOperatorDefinition::Custom { + argument_type: models::Type::Nullable { .. }, + } => None, + models::ComparisonOperatorDefinition::Custom { + argument_type: models::Type::Array { .. }, + } => None, + models::ComparisonOperatorDefinition::Custom { + argument_type: models::Type::Predicate { .. }, + } => None, }, ), )), @@ -116,7 +130,7 @@ pub async fn get_capabilities( }, config_schemas: get_openapi_config_schema_response(), display_name: None, - release_name: Some(v3_capabilities.versions.to_owned()), + release_name: Some(v3_capabilities.version.to_owned()), }; Ok(Json(response)) @@ -355,6 +369,7 @@ fn get_field_type(column_type: &models::Type, schema: &models::SchemaResponse) - nullable: matches!(**element_type, models::Type::Nullable { .. }), }) } + models::Type::Predicate { .. } => todo!(), } } @@ -415,7 +430,7 @@ pub async fn post_explain( }), ) })?; - let response = C::explain(&state.configuration, &state.state, request.clone()) + let response = C::query_explain(&state.configuration, &state.state, request.clone()) .await .and_then(JsonResponse::into_value) .map_err(|err| match err { @@ -479,7 +494,7 @@ fn map_query_request(request: QueryRequest) -> Result models::Field::Column { column }, + } => models::Field::Column { + column, + fields: None, + }, Field::Relationship { query, relationship, @@ -808,12 +826,13 @@ fn map_order_by_path( relationship: format!("{}.{}", source_table, segment), arguments, predicate: if let Some(predicate) = &relation.r#where { - Box::new(map_expression(predicate, &target_table, relationships)?) + Some(Box::new(map_expression( + predicate, + &target_table, + relationships, + )?)) } else { - // hack: predicate is not optional, so default to empty "And" expression, which evaluates to true. - Box::new(models::Expression::And { - expressions: vec![], - }) + None }, }); @@ -897,28 +916,12 @@ fn map_expression( } => models::Expression::BinaryComparisonOperator { column: map_comparison_column(column)?, operator: match operator { - BinaryComparisonOperator::LessThan => models::BinaryComparisonOperator::Other { - name: "less_than".to_string(), - }, - BinaryComparisonOperator::LessThanOrEqual => { - models::BinaryComparisonOperator::Other { - name: "less_than_or_equal".to_string(), - } - } - BinaryComparisonOperator::Equal => models::BinaryComparisonOperator::Equal, - BinaryComparisonOperator::GreaterThan => models::BinaryComparisonOperator::Other { - name: "greater_than".to_string(), - }, - BinaryComparisonOperator::GreaterThanOrEqual => { - models::BinaryComparisonOperator::Other { - name: "greater_than_or_equal".to_string(), - } - } - BinaryComparisonOperator::Other(operator) => { - models::BinaryComparisonOperator::Other { - name: operator.to_owned(), - } - } + BinaryComparisonOperator::LessThan => "less_than".to_string(), + BinaryComparisonOperator::LessThanOrEqual => "less_than_or_equal".to_string(), + BinaryComparisonOperator::Equal => "equal".to_string(), + BinaryComparisonOperator::GreaterThan => "greater_than".to_string(), + BinaryComparisonOperator::GreaterThanOrEqual => "greater_than_or_equal".to_string(), + BinaryComparisonOperator::Other(operator) => operator.to_owned(), }, value: match value { ComparisonValue::Scalar { @@ -937,10 +940,10 @@ fn map_expression( operator, value_type: _, values, - } => models::Expression::BinaryArrayComparisonOperator { + } => models::Expression::BinaryComparisonOperator { column: map_comparison_column(column)?, operator: match operator { - BinaryArrayComparisonOperator::In => models::BinaryArrayComparisonOperator::In, + BinaryArrayComparisonOperator::In => "in".to_string(), BinaryArrayComparisonOperator::Other(operator) => { return Err(ErrorResponse { details: None, @@ -949,12 +952,9 @@ fn map_expression( }) } }, - values: values - .iter() - .map(|value| models::ComparisonValue::Scalar { - value: value.clone(), - }) - .collect(), + value: models::ComparisonValue::Scalar { + value: serde_json::to_value(values).unwrap(), + }, }, Expression::Exists { in_table, r#where } => match in_table { ExistsInTable::Unrelated { table } => models::Expression::Exists { @@ -962,7 +962,11 @@ fn map_expression( collection: get_name(table)?, arguments: BTreeMap::new(), }, - predicate: Box::new(map_expression(r#where, &get_name(table)?, relationships)?), + predicate: Some(Box::new(map_expression( + r#where, + &get_name(table)?, + relationships, + )?)), }, ExistsInTable::Related { relationship } => { let (target_table, arguments) = @@ -973,7 +977,11 @@ fn map_expression( relationship: format!("{}.{}", collection, relationship), arguments, }, - predicate: Box::new(map_expression(r#where, &target_table, relationships)?), + predicate: Some(Box::new(map_expression( + r#where, + &target_table, + relationships, + )?)), } } }, diff --git a/rust-connector-sdk/src/routes.rs b/rust-connector-sdk/src/routes.rs index f7291e4a..80149657 100644 --- a/rust-connector-sdk/src/routes.rs +++ b/rust-connector-sdk/src/routes.rs @@ -76,55 +76,74 @@ pub async fn get_schema( }) } -pub async fn post_explain( +/// Invoke the connector's mutation_explain method and potentially map errors back to error responses. +pub async fn post_mutation_explain( + configuration: &C::Configuration, + state: &C::State, + request: models::MutationRequest, +) -> Result, (StatusCode, Json)> { + C::mutation_explain(configuration, state, request) + .await + .map_err(convert_explain_error) +} + +/// Invoke the connector's query_explain method and potentially map errors back to error responses. +pub async fn post_query_explain( configuration: &C::Configuration, state: &C::State, request: models::QueryRequest, ) -> Result, (StatusCode, Json)> { - C::explain(configuration, state, request) + C::query_explain(configuration, state, request) .await - .map_err(|e| match e { - crate::connector::ExplainError::Other(err) => ( - StatusCode::INTERNAL_SERVER_ERROR, - Json(models::ErrorResponse { - message: "Internal error".into(), - details: serde_json::Value::Object(serde_json::Map::from_iter([( - "cause".into(), - serde_json::Value::String(err.to_string()), - )])), - }), - ), - crate::connector::ExplainError::InvalidRequest(detail) => ( - StatusCode::BAD_REQUEST, - Json(models::ErrorResponse { - message: "Invalid request".into(), - details: serde_json::Value::Object(serde_json::Map::from_iter([( - "detail".into(), - serde_json::Value::String(detail), - )])), - }), - ), - crate::connector::ExplainError::UnprocessableContent(detail) => ( - StatusCode::UNPROCESSABLE_ENTITY, - Json(models::ErrorResponse { - message: "Unprocessable content".into(), - details: serde_json::Value::Object(serde_json::Map::from_iter([( - "detail".into(), - serde_json::Value::String(detail), - )])), - }), - ), - crate::connector::ExplainError::UnsupportedOperation(detail) => ( - StatusCode::NOT_IMPLEMENTED, - Json(models::ErrorResponse { - message: "Unsupported operation".into(), - details: serde_json::Value::Object(serde_json::Map::from_iter([( - "detail".into(), - serde_json::Value::String(detail), - )])), - }), - ), - }) + .map_err(convert_explain_error) +} + +/// Convert an sdk explain error to an error response and status code. +fn convert_explain_error( + error: crate::connector::ExplainError, +) -> (StatusCode, Json) { + match error { + crate::connector::ExplainError::Other(err) => ( + StatusCode::INTERNAL_SERVER_ERROR, + Json(models::ErrorResponse { + message: "Internal error".into(), + details: serde_json::Value::Object(serde_json::Map::from_iter([( + "cause".into(), + serde_json::Value::String(err.to_string()), + )])), + }), + ), + crate::connector::ExplainError::InvalidRequest(detail) => ( + StatusCode::BAD_REQUEST, + Json(models::ErrorResponse { + message: "Invalid request".into(), + details: serde_json::Value::Object(serde_json::Map::from_iter([( + "detail".into(), + serde_json::Value::String(detail), + )])), + }), + ), + crate::connector::ExplainError::UnprocessableContent(detail) => ( + StatusCode::UNPROCESSABLE_ENTITY, + Json(models::ErrorResponse { + message: "Unprocessable content".into(), + details: serde_json::Value::Object(serde_json::Map::from_iter([( + "detail".into(), + serde_json::Value::String(detail), + )])), + }), + ), + crate::connector::ExplainError::UnsupportedOperation(detail) => ( + StatusCode::NOT_IMPLEMENTED, + Json(models::ErrorResponse { + message: "Unsupported operation".into(), + details: serde_json::Value::Object(serde_json::Map::from_iter([( + "detail".into(), + serde_json::Value::String(detail), + )])), + }), + ), + } } pub async fn post_mutation(