From dd82ec1000f75eb72182121302aea22a83e5c5b7 Mon Sep 17 00:00:00 2001 From: Afsal Thaj Date: Wed, 3 Apr 2024 19:06:32 +1100 Subject: [PATCH] Generic Response mapping using Expr (#349) * Update examples * Update more info * Update examples * Update wasm-rpc * Initial version of expr transformation * Remove resolved variables * Constructor * Remove ResolvedVariables * Rename resolved variables from path * Fix spec path variables * Form type annotated value from request * Fix test compilation errors * Cache template met * Add typed value function in worker service * Remove compile time error * Start fixing errors * Fix more errors * Fix all main errors * Fix more errors * Temporarily depend on wasm-rpc * Add tests for type infernce * Add comments * Upgrade to latest wasm rpc * Remove json dependency from type annotated value module * Fix more errors * Fix more test cases with 30 remaining * Handle optional field * Fix more errors * Fix evaluator tests with three remaining errors * Fix all tests in evaluator * Fix type inference and add more tests * Fix all tests in http_request * Fix all parser tests * Fix all tests * Fix clippy * Fix clippy * Remove comments * Add more test for type inference * Make the concept of ApiDefinition generic * Move validator to generic * make golem worker binding a generic idea * Start moving everything to an expression module * Start introducing more modules * Start compiling * Initial draft of code organisation * Fix errors * Fix errors * Fix errors * Add constraints * Use Has patterns to support generic * Response binding * First compiled version of draft * Optimise imports * Make modules private and use pub for crate * Tighten privacy in every module * Avoid leaking Response mapping outside base module * Move internal logic to internal module * make sure to use mod internal pattern * Refactor oas_worker_bridge * Further maximise private modules * Try to compile golem worker service * Fix errors * Fix all compile time errors * Compile everything * Make sure everything compile * Fix clippy * Remove unused trait * Fix more errors * Fix more errors * Rename * Rearrange further * Reorganise code * Reformat and rename service * Fix clippy * Reformat and clippy for worker-service * Fix format * Fix formatting * Make ApiDefinition concept generic (#342) * Make the concept of ApiDefinition generic * Move validator to generic * make golem worker binding a generic idea * Start moving everything to an expression module * Start introducing more modules * Start compiling * Initial draft of code organisation * Fix errors * Fix errors * Fix errors * Add constraints * Use Has patterns to support generic * Response binding * First compiled version of draft * Optimise imports * Make modules private and use pub for crate * Tighten privacy in every module * Avoid leaking Response mapping outside base module * Move internal logic to internal module * make sure to use mod internal pattern * Refactor oas_worker_bridge * Further maximise private modules * Try to compile golem worker service * Fix errors * Fix all compile time errors * Compile everything * Make sure everything compile * Fix clippy * Remove unused trait * Fix more errors * Fix more errors * Rename * Rearrange further * Reorganise code * Reformat and rename service * Fix clippy * Reformat and clippy for worker-service * Fix formatting * Fix worker service test * Delete commented code * Update yaml file * Fix construction * Fix construction * Fix none * Add test for nest construction * Add test for Result * Add test * Reformat code * Use proper pattern match * Use err * Generic response mapping * Fix warnings * Remove todo comment * Fix tests * Fix tests * Reformat code * Upgrade golem-examples * Update open api yaml --- .../src/api/register_api_definition_api.rs | 59 +---- .../http/http_oas_api_definition.rs | 39 +-- .../http/http_response_mapping.rs | 228 ++++++++++++++++++ .../src/api_definition/http/mod.rs | 4 + .../src/evaluator/mod.rs | 6 + .../worker_binding/golem_worker_binding.rs | 10 +- .../worker_response.rs | 14 +- openapi/golem-service.yaml | 15 +- 8 files changed, 266 insertions(+), 109 deletions(-) create mode 100644 golem-worker-service-base/src/api_definition/http/http_response_mapping.rs diff --git a/golem-worker-service-base/src/api/register_api_definition_api.rs b/golem-worker-service-base/src/api/register_api_definition_api.rs index 518c0de56..dcdf06889 100644 --- a/golem-worker-service-base/src/api/register_api_definition_api.rs +++ b/golem-worker-service-base/src/api/register_api_definition_api.rs @@ -1,4 +1,3 @@ -use std::collections::HashMap; use std::result::Result; use poem_openapi::*; @@ -37,16 +36,7 @@ pub struct GolemWorkerBinding { pub worker_id: serde_json::value::Value, pub function_name: String, pub function_params: Vec, - pub response: Option, -} - -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, Object)] -pub struct ResponseMapping { - pub body: serde_json::value::Value, - // ${function.return} - pub status: serde_json::value::Value, - // "200" or if ${response.body.id == 1} "200" else "400" - pub headers: HashMap, + pub response: Option, } impl TryFrom for HttpApiDefinition { @@ -119,52 +109,13 @@ impl TryInto for Route { } } -impl TryFrom for ResponseMapping { - type Error = String; - - fn try_from(value: crate::worker_binding::ResponseMapping) -> Result { - let body = serde_json::to_value(value.body).map_err(|e| e.to_string())?; - let status = serde_json::to_value(value.status).map_err(|e| e.to_string())?; - let mut headers = HashMap::new(); - for (key, value) in value.headers { - let v = serde_json::to_value(value).map_err(|e| e.to_string())?; - headers.insert(key.to_string(), v); - } - Ok(Self { - body, - status, - headers, - }) - } -} - -impl TryInto for ResponseMapping { - type Error = String; - - fn try_into(self) -> Result { - let body: Expr = serde_json::from_value(self.body).map_err(|e| e.to_string())?; - let status: Expr = serde_json::from_value(self.status).map_err(|e| e.to_string())?; - let mut headers = HashMap::new(); - for (key, value) in self.headers { - let v: Expr = serde_json::from_value(value).map_err(|e| e.to_string())?; - headers.insert(key.to_string(), v); - } - - Ok(crate::worker_binding::ResponseMapping { - body, - status, - headers, - }) - } -} - impl TryFrom for GolemWorkerBinding { type Error = String; fn try_from(value: crate::worker_binding::GolemWorkerBinding) -> Result { - let response: Option = match value.response { + let response: Option = match value.response { Some(v) => { - let r = ResponseMapping::try_from(v)?; + let r = Expr::to_json_value(&v.0).map_err(|e| e.to_string())?; Some(r) } None => None, @@ -192,8 +143,8 @@ impl TryInto for GolemWorkerBinding { fn try_into(self) -> Result { let response: Option = match self.response { Some(v) => { - let r: crate::worker_binding::ResponseMapping = v.try_into()?; - Some(r) + let r = Expr::from_json_value(&v).map_err(|e| e.to_string())?; + Some(crate::worker_binding::ResponseMapping(r)) } None => None, }; diff --git a/golem-worker-service-base/src/api_definition/http/http_oas_api_definition.rs b/golem-worker-service-base/src/api_definition/http/http_oas_api_definition.rs index e411084ea..1d557bcc9 100644 --- a/golem-worker-service-base/src/api_definition/http/http_oas_api_definition.rs +++ b/golem-worker-service-base/src/api_definition/http/http_oas_api_definition.rs @@ -26,13 +26,13 @@ pub fn get_api_definition_from_oas(open_api: &str) -> Result Result, String> { let response = worker_bridge_info.get("response"); match response { - Some(response) => Ok(Some(ResponseMapping { - status: Expr::from_json_value(response.get("status").ok_or("No status found")?) - .map_err(|err| err.to_string())?, - headers: { - let mut header_map = HashMap::new(); - - let header_iter = response - .get("headers") - .ok_or("No headers found")? - .as_object() - .ok_or("headers is not an object")? - .iter(); - - for (header_name, value) in header_iter { - let value_str = value.as_str().ok_or("Header value is not a string")?; - header_map.insert( - header_name.clone(), - Expr::from_primitive_string(value_str) - .map_err(|err| err.to_string())?, - ); - } + Some(response) => { + let response_mapping_expr = + Expr::from_json_value(response).map_err(|err| err.to_string())?; + + let _ = + HttpResponseMapping::try_from(&ResponseMapping(response_mapping_expr.clone())) + .map_err(|err| err.to_string())?; - header_map - }, - body: Expr::from_json_value(response.get("body").ok_or("No body found")?) - .map_err(|err| err.to_string())?, - })), + Ok(Some(ResponseMapping(response_mapping_expr))) + } None => Ok(None), } } diff --git a/golem-worker-service-base/src/api_definition/http/http_response_mapping.rs b/golem-worker-service-base/src/api_definition/http/http_response_mapping.rs new file mode 100644 index 000000000..93b8b4fd7 --- /dev/null +++ b/golem-worker-service-base/src/api_definition/http/http_response_mapping.rs @@ -0,0 +1,228 @@ +use std::collections::HashMap; +use std::fmt::Display; + +use crate::expression::Expr; +use crate::worker_binding::ResponseMapping; + +#[derive(PartialEq, Debug, Clone)] +pub struct HttpResponseMapping { + pub body: Expr, // ${function.return} + pub status: Expr, // "200" or if ${response.body.id == 1} "200" else "400" + pub headers: HashMap, +} + +impl Display for HttpResponseMapping { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let response_mapping: ResponseMapping = self.clone().into(); + let expr_json = Expr::to_json_value(&response_mapping.0).unwrap(); + + write!(f, "{}", expr_json) + } +} + +impl From for ResponseMapping { + fn from(http_response_mapping: HttpResponseMapping) -> Self { + ResponseMapping(Expr::Record( + vec![ + ("body".to_string(), Box::new(http_response_mapping.body)), + ("status".to_string(), Box::new(http_response_mapping.status)), + ( + "headers".to_string(), + Box::new(Expr::Record( + http_response_mapping + .headers + .into_iter() + .map(|(key, value)| (key, Box::new(value))) + .collect::)>>(), + )), + ), + ] + .into_iter() + .collect(), + )) + } +} + +impl TryFrom<&ResponseMapping> for HttpResponseMapping { + type Error = String; + + fn try_from(response_mapping: &ResponseMapping) -> Result { + let mut headers = HashMap::new(); + let generic_expr = &response_mapping.0; + + match generic_expr { + Expr::Record(obj) => { + let mut body = None; + let mut status = None; + + for (key, value) in obj { + match key.as_str() { + "body" => body = Some(value), + "status" => status = Some(value), + "headers" => { + if let Expr::Record(headers_obj) = value.as_ref().clone() { + for (header_key, header_value) in headers_obj { + headers + .insert(header_key.clone(), header_value.as_ref().clone()); + } + } else { + return Err("headers must be an object".to_string()); + } + } + _ => return Err(format!("Unknown key: {}", key)), + } + } + + match (body, status) { + (Some(body), Some(status)) => Ok(HttpResponseMapping { + body: body.as_ref().clone(), + status: status.as_ref().clone(), + headers, + }), + (None, _) => Err("body is required in http response mapping".to_string()), + (_, None) => Err("status is required in http response mapping".to_string()), + } + } + _ => Err("response mapping must be a record".to_string()), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::expression::Expr; + use crate::worker_binding::ResponseMapping; + + #[test] + fn test_try_from_response_mapping() { + let response_mapping = ResponseMapping(Expr::Record( + vec![ + ( + "body".to_string(), + Box::new(Expr::Variable("function.return".to_string())), + ), + ( + "status".to_string(), + Box::new(Expr::Literal("200".to_string())), + ), + ( + "headers".to_string(), + Box::new(Expr::Record(vec![( + "Content-Type".to_string(), + Box::new(Expr::Literal("application/json".to_string())), + )])), + ), + ] + .into_iter() + .collect(), + )); + + let http_response_mapping = HttpResponseMapping::try_from(&response_mapping).unwrap(); + + assert_eq!( + http_response_mapping.body, + Expr::Variable("function.return".to_string()) + ); + assert_eq!( + http_response_mapping.status, + Expr::Literal("200".to_string()) + ); + assert_eq!(http_response_mapping.headers.len(), 1); + assert_eq!( + http_response_mapping.headers.get("Content-Type").unwrap(), + &Expr::Literal("application/json".to_string()) + ); + } + + #[test] + fn test_try_from_response_mapping_missing_body() { + let response_mapping = ResponseMapping(Expr::Record( + vec![ + ( + "status".to_string(), + Box::new(Expr::Literal("200".to_string())), + ), + ( + "headers".to_string(), + Box::new(Expr::Record(vec![( + "Content-Type".to_string(), + Box::new(Expr::Literal("application/json".to_string())), + )])), + ), + ] + .into_iter() + .collect(), + )); + + let result = HttpResponseMapping::try_from(&response_mapping); + + assert_eq!( + result, + Err("body is required in http response mapping".to_string()) + ); + } + + #[test] + fn test_try_from_response_mapping_missing_status() { + let response_mapping = ResponseMapping(Expr::Record( + vec![ + ( + "body".to_string(), + Box::new(Expr::Variable("function.return".to_string())), + ), + ( + "headers".to_string(), + Box::new(Expr::Record(vec![( + "Content-Type".to_string(), + Box::new(Expr::Literal("application/json".to_string())), + )])), + ), + ] + .into_iter() + .collect(), + )); + + let result = HttpResponseMapping::try_from(&response_mapping); + + assert_eq!( + result, + Err("status is required in http response mapping".to_string()) + ); + } + + #[test] + fn test_try_from_response_mapping_headers_not_object() { + let response_mapping = ResponseMapping(Expr::Record( + vec![ + ( + "body".to_string(), + Box::new(Expr::Variable("worker.response".to_string())), + ), + ( + "status".to_string(), + Box::new(Expr::Literal("200".to_string())), + ), + ( + "headers".to_string(), + Box::new(Expr::Literal("application/json".to_string())), + ), + ] + .into_iter() + .collect(), + )); + + let result = HttpResponseMapping::try_from(&response_mapping); + + assert_eq!(result, Err("headers must be an object".to_string())); + } + + #[test] + fn test_try_from_response_mapping_not_record() { + let response_mapping = ResponseMapping(Expr::Literal("200".to_string())); + + let result = HttpResponseMapping::try_from(&response_mapping); + + assert_eq!(result, Err("response mapping must be a record".to_string())); + } +} diff --git a/golem-worker-service-base/src/api_definition/http/mod.rs b/golem-worker-service-base/src/api_definition/http/mod.rs index d6b2fa413..06dcf4444 100644 --- a/golem-worker-service-base/src/api_definition/http/mod.rs +++ b/golem-worker-service-base/src/api_definition/http/mod.rs @@ -1,4 +1,8 @@ pub use http_api_definition::*; pub use http_oas_api_definition::get_api_definition_from_oas; +pub(crate) use http_response_mapping::HttpResponseMapping; + mod http_api_definition; mod http_oas_api_definition; + +mod http_response_mapping; diff --git a/golem-worker-service-base/src/evaluator/mod.rs b/golem-worker-service-base/src/evaluator/mod.rs index 8875898b1..5a565c018 100644 --- a/golem-worker-service-base/src/evaluator/mod.rs +++ b/golem-worker-service-base/src/evaluator/mod.rs @@ -30,6 +30,12 @@ pub enum EvaluationError { Message(String), } +impl From for EvaluationError { + fn from(string: String) -> Self { + EvaluationError::Message(string) + } +} + impl Display for EvaluationError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { diff --git a/golem-worker-service-base/src/worker_binding/golem_worker_binding.rs b/golem-worker-service-base/src/worker_binding/golem_worker_binding.rs index 4df55fa0c..5826ac6be 100644 --- a/golem-worker-service-base/src/worker_binding/golem_worker_binding.rs +++ b/golem-worker-service-base/src/worker_binding/golem_worker_binding.rs @@ -1,5 +1,3 @@ -use std::collections::HashMap; - use bincode::{Decode, Encode}; use serde::{Deserialize, Serialize}; @@ -17,11 +15,5 @@ pub struct GolemWorkerBinding { pub response: Option, } -// TODO; https://github.com/golemcloud/golem/issues/318 -// This will make GolemWorkerBidning generic for all protocols #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, Encode, Decode)] -pub struct ResponseMapping { - pub body: Expr, // ${function.return} - pub status: Expr, // "200" or if ${response.body.id == 1} "200" else "400" - pub headers: HashMap, -} +pub struct ResponseMapping(pub Expr); diff --git a/golem-worker-service-base/src/worker_bridge_execution/worker_response.rs b/golem-worker-service-base/src/worker_bridge_execution/worker_response.rs index 22f6c3013..2f4b948c4 100644 --- a/golem-worker-service-base/src/worker_bridge_execution/worker_response.rs +++ b/golem-worker-service-base/src/worker_bridge_execution/worker_response.rs @@ -118,6 +118,7 @@ impl WorkerRequestExecutor for NoOpWorkerRequestExecutor { } mod internal { + use crate::api_definition::http::HttpResponseMapping; use crate::evaluator::EvaluationError; use crate::evaluator::Evaluator; use crate::expression::Expr; @@ -146,12 +147,17 @@ mod internal { let type_annotated_value = input_request.merge(&worker_response.result_with_worker_response_key()); - let status_code = get_status_code(&response_mapping.status, &type_annotated_value)?; + let http_response_mapping = HttpResponseMapping::try_from(response_mapping)?; - let headers = - ResolvedResponseHeaders::from(&response_mapping.headers, &type_annotated_value)?; + let status_code = + get_status_code(&http_response_mapping.status, &type_annotated_value)?; - let response_body = response_mapping.body.evaluate(&type_annotated_value)?; + let headers = ResolvedResponseHeaders::from( + &http_response_mapping.headers, + &type_annotated_value, + )?; + + let response_body = http_response_mapping.body.evaluate(&type_annotated_value)?; Ok(IntermediateHttpResponse { body: response_body, diff --git a/openapi/golem-service.yaml b/openapi/golem-service.yaml index e4ade02dc..18b17b0b9 100644 --- a/openapi/golem-service.yaml +++ b/openapi/golem-service.yaml @@ -1928,8 +1928,7 @@ components: functionParams: type: array items: {} - response: - $ref: '#/components/schemas/ResponseMapping' + response: {} required: - template - workerId @@ -2014,18 +2013,6 @@ components: required: - workerId - oplogIdx - ResponseMapping: - type: object - properties: - body: {} - status: {} - headers: - type: object - additionalProperties: {} - required: - - body - - status - - headers ResumeResponse: type: object Route: