From 2b3bd239c11a877fbd703afc329a7238874210a4 Mon Sep 17 00:00:00 2001 From: Pratik Mishra Date: Fri, 26 Apr 2024 17:31:18 +0530 Subject: [PATCH] fix: accept version in query param for all get config apis --- .../2024-04-22-122806_config_verions/up.sql | 5 +- .../src/api/config/handlers.rs | 77 +++++++++++-------- .../src/api/context/handlers.rs | 50 ++++++++---- .../src/api/context/types.rs | 21 +++++ .../src/api/default_config/handlers.rs | 24 +++--- .../src/api/default_config/types.rs | 6 ++ crates/context_aware_config/src/db/models.rs | 3 +- crates/context_aware_config/src/db/schema.rs | 3 +- crates/context_aware_config/src/helpers.rs | 5 +- crates/service_utils/src/helpers.rs | 15 +++- crates/service_utils/src/service/types.rs | 7 ++ 11 files changed, 152 insertions(+), 64 deletions(-) diff --git a/crates/context_aware_config/migrations/2024-04-22-122806_config_verions/up.sql b/crates/context_aware_config/migrations/2024-04-22-122806_config_verions/up.sql index 2d6e8dba..e87fdaa2 100644 --- a/crates/context_aware_config/migrations/2024-04-22-122806_config_verions/up.sql +++ b/crates/context_aware_config/migrations/2024-04-22-122806_config_verions/up.sql @@ -2,8 +2,9 @@ -- Name: functions; Type: TABLE; Schema: public; Owner: - -- CREATE TABLE public.config_versions ( - id text PRIMARY KEY, - config json, + id bigint PRIMARY KEY, + config json NOT NULL, + version_type TEXT CHECK (version_type IN ('STABLE', 'NOISY')), created_at timestamp without time zone DEFAULT CURRENT_TIMESTAMP NOT NULL ); -- \ No newline at end of file diff --git a/crates/context_aware_config/src/api/config/handlers.rs b/crates/context_aware_config/src/api/config/handlers.rs index 9643d561..5434245b 100644 --- a/crates/context_aware_config/src/api/config/handlers.rs +++ b/crates/context_aware_config/src/api/config/handlers.rs @@ -19,7 +19,7 @@ use diesel::{ r2d2::{ConnectionManager, PooledConnection}, ExpressionMethods, PgConnection, QueryDsl, RunQueryDsl, }; -use serde_json::{json, Map, Value}; +use serde_json::{from_value, json, Map, Value}; use service_utils::service::types::DbConnection; use service_utils::{bad_argument, db_error, unexpected_error}; @@ -99,6 +99,40 @@ fn is_not_modified(max_created_at: Option, req: &HttpRequest) -> max_created_at.is_some() && parsed_max <= last_modified } +pub fn generate_config_from_version( + version: Option, + conn: &mut PooledConnection>, +) -> superposition::Result { + let config_version = match version { + None => config_versions::config_versions + .select(config_versions::config) + .order_by(config_versions::created_at.desc()) + .first::(conn) + .map_err(|err| { + log::error!("failed to fetch config with error: {}", err); + db_error!(err) + }), + Some(version_val) => { + let version_id = from_value::(version_val).map_err(|e| { + log::error!("failed to decode version_id as integer: {}", e); + bad_argument!("version_id is not of type integer") + })?; + config_versions::config_versions + .select(config_versions::config) + .filter(config_versions::id.eq(version_id)) + .get_result::(conn) + .map_err(|err| { + log::error!("failed to fetch config with error: {}", err); + db_error!(err) + }) + } + }?; + + serde_json::from_value::(config_version).map_err(|err| { + log::error!("failed to decode config: {}", err); + unexpected_error!("failed to decode config") + }) +} pub fn generate_cac( conn: &mut PooledConnection>, ) -> superposition::Result { @@ -161,8 +195,6 @@ async fn get( req: HttpRequest, db_conn: DbConnection, ) -> superposition::Result { - // let DbConnection(mut conn) = db_conn; - // let mut conn: PooledConnection> = db_conn; let DbConnection(mut conn) = db_conn; let max_created_at = get_max_created_at(&mut conn) @@ -188,36 +220,13 @@ async fn get( query_params_map.insert( key, value - .parse::() + .parse::() .map_or_else(|_| json!(value), |int_val| json!(int_val)), ); } - let mut config_version = json!({}); - if let Some(Value::String(version_id)) = query_params_map.get("version") { - config_version = config_versions::config_versions - .select(config_versions::config) - .filter(config_versions::id.eq(version_id)) - .get_result::(&mut conn) - .map_err(|err| { - log::error!("failed to fetch config with error: {}", err); - db_error!(err) - })?; - } else { - config_version = config_versions::config_versions - .select(config_versions::config) - .order_by(config_versions::created_at.desc()) - .first::(&mut conn) - .map_err(|err| { - log::error!("failed to fetch config with error: {}", err); - db_error!(err) - })?; - }; - let mut config = serde_json::from_value::(config_version).map_err(|err| { - log::error!("failed to decode config: {}", err); - unexpected_error!("failed to decode config") - })?; - query_params_map.remove("version"); + let mut config = + generate_config_from_version(query_params_map.remove("version"), &mut conn)?; if let Some(prefix) = query_params_map.get("prefix") { let prefix_list: HashSet<&str> = prefix .as_str() @@ -260,7 +269,7 @@ async fn get_resolved_config( query_params_map.insert( item.0, item.1 - .parse::() + .parse::() .map_or_else(|_| json!(item.1), |int_val| json!(int_val)), ); } @@ -275,7 +284,8 @@ async fn get_resolved_config( return Ok(HttpResponse::NotModified().finish()); } - let res = generate_cac(&mut conn)?; + let res = + generate_config_from_version(query_params_map.remove("version"), &mut conn)?; let cac_client_contexts = res .contexts @@ -344,11 +354,12 @@ async fn get_filtered_config( query_params_map.insert( key, value - .parse::() + .parse::() .map_or_else(|_| json!(value), |int_val| json!(int_val)), ); } - let config = generate_cac(&mut conn)?; + let config = + generate_config_from_version(query_params_map.remove("version"), &mut conn)?; let contexts = config.contexts; let filtered_context = filter_context(&contexts, &query_params_map)?; diff --git a/crates/context_aware_config/src/api/context/handlers.rs b/crates/context_aware_config/src/api/context/handlers.rs index 7ac5811f..7f5dc435 100644 --- a/crates/context_aware_config/src/api/context/handlers.rs +++ b/crates/context_aware_config/src/api/context/handlers.rs @@ -7,8 +7,9 @@ use crate::helpers::{ use crate::{ api::{ context::types::{ - ContextAction, ContextBulkResponse, DimensionCondition, MoveReq, - PaginationParams, PutReq, PutResp, + BulkOperationQParams, ContextAction, ContextBulkResponse, DeleteQParams, + DimensionCondition, MoveQParams, MoveReq, PaginationParams, PutQParams, + PutReq, PutResp, }, dimension::get_all_dimension_schema_map, }, @@ -21,7 +22,10 @@ use crate::{ }, }; use actix_web::web::Data; -use service_utils::service::types::AppState; +use service_utils::{ + helpers::generate_snowflake_id, + service::types::{AppState, ConfigVersionType}, +}; use actix_web::{ delete, get, put, @@ -281,11 +285,15 @@ fn put( async fn put_handler( state: Data, req: Json, + qparams: Query, mut db_conn: DbConnection, user: User, ) -> superposition::Result> { - let mut snowflake_generator = state.snowflake_generator.lock().unwrap(); - let version_id = snowflake_generator.real_time_generate(); + let version_id = generate_snowflake_id(&state)?; + let version_type = qparams + .update_type + .map_or(ConfigVersionType::STABLE, |v| v) + .to_string(); db_conn.transaction::<_, superposition::AppError, _>(|transaction_conn| { let put_response = put(req, transaction_conn, &user) .map(|resp| Json(resp)) @@ -293,7 +301,7 @@ async fn put_handler( log::info!("context put failed with error: {:?}", err); err })?; - add_config_version(version_id, transaction_conn)?; + add_config_version(version_id, version_type, transaction_conn)?; Ok(put_response) }) } @@ -368,11 +376,15 @@ async fn move_handler( state: Data, path: Path, req: Json, + qparams: Query, mut db_conn: DbConnection, user: User, ) -> superposition::Result> { - let mut snowflake_generator = state.snowflake_generator.lock().unwrap(); - let version_id = snowflake_generator.real_time_generate(); + let version_id = generate_snowflake_id(&state)?; + let version_type = qparams + .update_type + .map_or(ConfigVersionType::STABLE, |v| v) + .to_string(); db_conn.transaction::<_, superposition::AppError, _>(|transaction_conn| { let move_reponse = r#move(path.into_inner(), req, transaction_conn, &user) .map(|resp| Json(resp)) @@ -380,7 +392,7 @@ async fn move_handler( log::info!("move api failed with error: {:?}", err); err })?; - add_config_version(version_id, transaction_conn)?; + add_config_version(version_id, version_type, transaction_conn)?; Ok(move_reponse) }) } @@ -438,6 +450,7 @@ async fn list_contexts( async fn delete_context( state: Data, path: Path, + qparams: Query, db_conn: DbConnection, user: User, ) -> superposition::Result { @@ -445,15 +458,18 @@ async fn delete_context( let DbConnection(mut conn) = db_conn; let ctx_id = path.into_inner(); - let mut snowflake_generator = state.snowflake_generator.lock().unwrap(); - let version_id = snowflake_generator.real_time_generate(); + let version_id = generate_snowflake_id(&state)?; + let version_type = qparams + .update_type + .map_or(ConfigVersionType::STABLE, |v| v) + .to_string(); conn.transaction::<_, superposition::AppError, _>(|transaction_conn| { let deleted_row = delete(dsl::contexts.filter(dsl::id.eq(&ctx_id))).execute(transaction_conn); match deleted_row { Ok(0) => Err(not_found!("Context Id `{}` doesn't exists", ctx_id)), Ok(_) => { - add_config_version(version_id, transaction_conn)?; + add_config_version(version_id, version_type, transaction_conn)?; log::info!("{ctx_id} context deleted by {}", user.get_email()); Ok(HttpResponse::NoContent().finish()) } @@ -469,13 +485,17 @@ async fn delete_context( async fn bulk_operations( state: Data, reqs: Json>, + qparams: Query, db_conn: DbConnection, user: User, ) -> superposition::Result>> { use contexts::dsl::contexts; let DbConnection(mut conn) = db_conn; - let mut snowflake_generator = state.snowflake_generator.lock().unwrap(); - let version_id = snowflake_generator.real_time_generate(); + let version_id = generate_snowflake_id(&state)?; + let version_type = qparams + .update_type + .map_or(ConfigVersionType::STABLE, |v| v) + .to_string(); let mut response = Vec::::new(); conn.transaction::<_, superposition::AppError, _>(|transaction_conn| { @@ -530,7 +550,7 @@ async fn bulk_operations( } } } - add_config_version(version_id, transaction_conn)?; + add_config_version(version_id, version_type, transaction_conn)?; Ok(()) // Commit the transaction })?; Ok(Json(response)) diff --git a/crates/context_aware_config/src/api/context/types.rs b/crates/context_aware_config/src/api/context/types.rs index c376fbd1..ef36a370 100644 --- a/crates/context_aware_config/src/api/context/types.rs +++ b/crates/context_aware_config/src/api/context/types.rs @@ -1,5 +1,6 @@ use serde::{Deserialize, Serialize}; use serde_json::{Map, Value}; +use service_utils::service::types::ConfigVersionType; #[derive(Deserialize, Clone)] pub struct PutReq { @@ -49,3 +50,23 @@ pub struct FunctionsInfo { pub name: String, pub code: Option, } + +#[derive(Deserialize, Clone)] +pub struct MoveQParams { + pub update_type: Option, +} + +#[derive(Deserialize, Clone)] +pub struct BulkOperationQParams { + pub update_type: Option, +} + +#[derive(Deserialize, Clone)] +pub struct PutQParams { + pub update_type: Option, +} + +#[derive(Deserialize, Clone)] +pub struct DeleteQParams { + pub update_type: Option, +} diff --git a/crates/context_aware_config/src/api/default_config/handlers.rs b/crates/context_aware_config/src/api/default_config/handlers.rs index b5193e06..dce7a69d 100644 --- a/crates/context_aware_config/src/api/default_config/handlers.rs +++ b/crates/context_aware_config/src/api/default_config/handlers.rs @@ -1,6 +1,12 @@ extern crate base64; -use super::types::CreateReq; -use service_utils::{bad_argument, unexpected_error, validation_error}; +use super::types::{CreateQParams, CreateReq}; +use service_utils::{ + bad_argument, + helpers::generate_snowflake_id, + result as superposition, + service::types::{AppState, ConfigVersionType, DbConnection}, + unexpected_error, validation_error, +}; use superposition_types::{SuperpositionUser, User}; @@ -23,10 +29,6 @@ use diesel::{ }; use jsonschema::{Draft, JSONSchema, ValidationError}; use serde_json::{json, Value}; -use service_utils::{ - result as superposition, - service::types::{AppState, DbConnection}, -}; pub fn endpoints() -> Scope { Scope::new("").service(create).service(get) @@ -37,12 +39,17 @@ async fn create( state: Data, key: web::Path, request: web::Json, + qparams: web::Query, db_conn: DbConnection, user: User, ) -> superposition::Result { let DbConnection(mut conn) = db_conn; let req = request.into_inner(); let key = key.into_inner(); + let version_type = qparams + .update_type + .map_or(ConfigVersionType::STABLE, |v| v) + .to_string(); if req.value.is_none() && req.schema.is_none() && req.function_name.is_none() { log::error!("No data provided in the request body for {key}"); @@ -140,8 +147,7 @@ async fn create( )?; } } - let mut snowflake_generator = state.snowflake_generator.lock().unwrap(); - let version_id = snowflake_generator.real_time_generate(); + let version_id = generate_snowflake_id(&state)?; conn.transaction::<_, superposition::AppError, _>(|transaction_conn| { let upsert = diesel::insert_into(default_configs) .values(&default_config) @@ -161,7 +167,7 @@ async fn create( )) } }?; - add_config_version(version_id, transaction_conn)?; + add_config_version(version_id, version_type, transaction_conn)?; Ok(ok_resp) }) } diff --git a/crates/context_aware_config/src/api/default_config/types.rs b/crates/context_aware_config/src/api/default_config/types.rs index 0e4b1b61..2a93ee22 100644 --- a/crates/context_aware_config/src/api/default_config/types.rs +++ b/crates/context_aware_config/src/api/default_config/types.rs @@ -1,5 +1,6 @@ use serde::{Deserialize, Deserializer}; use serde_json::{Map, Value}; +use service_utils::service::types::ConfigVersionType; #[derive(Debug, Deserialize)] pub struct CreateReq { @@ -17,3 +18,8 @@ where let value: Value = Deserialize::deserialize(deserializer)?; Ok(Some(value)) } + +#[derive(Deserialize, Clone)] +pub struct CreateQParams { + pub update_type: Option, +} diff --git a/crates/context_aware_config/src/db/models.rs b/crates/context_aware_config/src/db/models.rs index 36f7494e..70eacb9a 100644 --- a/crates/context_aware_config/src/db/models.rs +++ b/crates/context_aware_config/src/db/models.rs @@ -81,7 +81,8 @@ pub struct EventLog { #[diesel(check_for_backend(diesel::pg::Pg))] #[diesel(primary_key(id))] pub struct ConfigVersion { - pub id: String, + pub id: i64, pub config: Value, + pub version_type: String, pub created_at: NaiveDateTime, } diff --git a/crates/context_aware_config/src/db/schema.rs b/crates/context_aware_config/src/db/schema.rs index 09b400c3..1e4ed64f 100644 --- a/crates/context_aware_config/src/db/schema.rs +++ b/crates/context_aware_config/src/db/schema.rs @@ -603,8 +603,9 @@ diesel::joinable!(dimensions -> functions (function_name)); diesel::table! { config_versions (id) { - id -> Text, + id -> Int8, config -> Json, + version_type -> Text, created_at -> Timestamp, } } diff --git a/crates/context_aware_config/src/helpers.rs b/crates/context_aware_config/src/helpers.rs index 8f1de9af..c0160811 100644 --- a/crates/context_aware_config/src/helpers.rs +++ b/crates/context_aware_config/src/helpers.rs @@ -256,14 +256,15 @@ pub fn json_to_sorted_string(v: &Value) -> String { pub fn add_config_version( version_id: i64, + version_type: String, db_conn: &mut PooledConnection>, ) -> superposition::Result<()> { use config_versions::dsl::config_versions; let config = generate_cac(db_conn)?; - let config_version = ConfigVersion { - id: version_id.to_string(), + id: version_id, config: json!(config), + version_type: version_type, created_at: Utc::now().naive_utc(), }; diesel::insert_into(config_versions) diff --git a/crates/service_utils/src/helpers.rs b/crates/service_utils/src/helpers.rs index 70e40d8c..7c6afa8e 100644 --- a/crates/service_utils/src/helpers.rs +++ b/crates/service_utils/src/helpers.rs @@ -1,4 +1,5 @@ -use actix_web::{error::ErrorInternalServerError, Error}; +use actix_web::{error::ErrorInternalServerError, web::Data, Error}; +use anyhow::anyhow; use log::info; use serde::de::{self, IntoDeserializer}; use std::{ @@ -8,6 +9,7 @@ use std::{ }; use super::result; +use crate::service::types::AppState; use serde_json::{Map, Value}; //WARN Do NOT use this fxn inside api requests, instead add the required @@ -198,3 +200,14 @@ pub fn get_variable_name_and_value( Ok((variable_name, variable_value)) } + +pub fn generate_snowflake_id(state: &Data) -> result::Result { + let mut snowflake_generator = state.snowflake_generator.lock().map_err(|e| { + log::error!("snowflake_id generation failed {}", e); + result::AppError::UnexpectedError(anyhow!("snowflake_id generation failed {}", e)) + })?; + let id = snowflake_generator.real_time_generate(); + // explicitly dropping snowflake_generator so that lock is released and it can be acquired in bulk-operations handler + drop(snowflake_generator); + Ok(id) +} diff --git a/crates/service_utils/src/service/types.rs b/crates/service_utils/src/service/types.rs index 08cf4fa6..34c12ae1 100644 --- a/crates/service_utils/src/service/types.rs +++ b/crates/service_utils/src/service/types.rs @@ -1,6 +1,7 @@ use crate::db::pgschema_manager::{PgSchemaConnection, PgSchemaManager}; use derive_more::{Deref, DerefMut}; use jsonschema::JSONSchema; +use serde::Deserialize; use serde_json::json; use std::{ @@ -209,3 +210,9 @@ impl FromRequest for DbConnection { ready(result) } } + +#[derive(Copy, Clone, Debug, Deserialize, strum_macros::Display)] +pub enum ConfigVersionType { + STABLE, + NOISY, +}