Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: update queries refactor #390

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion crates/context_aware_config/src/api/config/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,11 @@ use superposition_types::{
};
use uuid::Uuid;

use crate::{api::context, helpers::DimensionData};
use crate::{api::context::PutReq, helpers::generate_cac};
use crate::{
api::context::{self, helpers::ensure_description},
helpers::DimensionData,
};
use crate::{
api::dimension::{get_dimension_data, get_dimension_data_map},
helpers::calculate_context_weight,
Expand Down Expand Up @@ -495,8 +498,19 @@ async fn reduce_config_key(
if is_approve {
let _ = context::delete(cid.clone(), user, conn, schema_name);
if let Ok(put_req) = construct_new_payload(request_payload) {
let description = match put_req.description.clone() {
Some(val) => val,
None => ensure_description(
Value::Object(
put_req.context.clone().into_inner().into(),
),
conn,
&schema_name,
)?,
};
let _ = context::put(
put_req,
description,
conn,
false,
&user,
Expand Down
106 changes: 67 additions & 39 deletions crates/context_aware_config/src/api/context/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,21 +67,23 @@ async fn put_handler(
schema_name: SchemaName,
) -> superposition::Result<HttpResponse> {
let tags = parse_config_tags(custom_headers.config_tags)?;
let description = match req.description.clone() {
Some(val) => val,
None => ensure_description(
Value::Object(req.context.clone().into_inner().into()),
&mut db_conn,
&schema_name,
)?,
};
let req_change_reason = req.change_reason.clone();

let (put_response, version_id) = db_conn
.transaction::<_, superposition::AppError, _>(|transaction_conn| {
let mut req_mut = req.into_inner();

// Use the helper function to ensure the description
if req_mut.description.is_none() {
req_mut.description = Some(ensure_description(
Value::Object(req_mut.context.clone().into_inner().into()),
transaction_conn,
&schema_name,
)?);
}

let put_response = operations::put(
Json(req_mut.clone()),
Json(req.into_inner()),
description.clone(),
transaction_conn,
true,
&user,
Expand All @@ -92,14 +94,12 @@ async fn put_handler(
log::info!("context put failed with error: {:?}", err);
err
})?;
let description = req_mut.description.unwrap_or_default();
let change_reason = req_mut.change_reason;

let version_id = add_config_version(
&state,
tags,
description,
change_reason,
req_change_reason,
transaction_conn,
&schema_name,
)?;
Expand Down Expand Up @@ -131,18 +131,20 @@ async fn update_override_handler(
schema_name: SchemaName,
) -> superposition::Result<HttpResponse> {
let tags = parse_config_tags(custom_headers.config_tags)?;
let description = match req.description.clone() {
Some(val) => val,
None => ensure_description(
Value::Object(req.context.clone().into_inner().into()),
&mut db_conn,
&schema_name,
)?,
};
let req_change_reason = req.change_reason.clone();
let (override_resp, version_id) = db_conn
.transaction::<_, superposition::AppError, _>(|transaction_conn| {
let mut req_mut = req.into_inner();
if req_mut.description.is_none() {
req_mut.description = Some(ensure_description(
Value::Object(req_mut.context.clone().into_inner().into()),
transaction_conn,
&schema_name,
)?);
}
let override_resp = operations::put(
Json(req_mut.clone()),
Json(req.into_inner()),
description.clone(),
transaction_conn,
true,
&user,
Expand All @@ -156,8 +158,8 @@ async fn update_override_handler(
let version_id = add_config_version(
&state,
tags,
req_mut.description.unwrap().clone(),
req_mut.change_reason.clone(),
description.clone(),
req_change_reason.clone(),
transaction_conn,
&schema_name,
)?;
Expand Down Expand Up @@ -189,11 +191,22 @@ async fn move_handler(
schema_name: SchemaName,
) -> superposition::Result<HttpResponse> {
let tags = parse_config_tags(custom_headers.config_tags)?;

let description = match req.description.clone() {
Some(val) => val,
None => ensure_description(
Value::Object(req.context.clone().into_inner().into()),
&mut db_conn,
&schema_name,
)?,
};

let (move_response, version_id) = db_conn
.transaction::<_, superposition::AppError, _>(|transaction_conn| {
let move_response = operations::r#move(
path.into_inner(),
req,
description,
transaction_conn,
true,
&user,
Expand Down Expand Up @@ -423,8 +436,26 @@ async fn bulk_operations(
for action in reqs.into_inner().into_iter() {
match action {
ContextAction::Put(put_req) => {
let ctx_condition = put_req.context.to_owned().into_inner();
let ctx_condition_value =
Value::Object(ctx_condition.clone().into());

let description = if put_req.description.is_none() {
ensure_description(
ctx_condition_value.clone(),
transaction_conn,
&schema_name,
)?
} else {
put_req
.description
.clone()
.expect("Description should not be empty")
};

let put_resp = operations::put(
Json(put_req.clone()),
description.clone(),
transaction_conn,
true,
&user,
Expand All @@ -439,21 +470,6 @@ async fn bulk_operations(
err
})?;

let ctx_condition = put_req.context.to_owned().into_inner();
let ctx_condition_value =
Value::Object(ctx_condition.clone().into());

let description = if put_req.description.is_none() {
ensure_description(
ctx_condition_value.clone(),
transaction_conn,
&schema_name,
)?
} else {
put_req
.description
.expect("Description should not be empty")
};
all_descriptions.push(description);
all_change_reasons.push(put_req.change_reason.clone());
response.push(ContextBulkResponse::Put(put_resp));
Expand Down Expand Up @@ -498,9 +514,21 @@ async fn bulk_operations(
};
}
ContextAction::Move((old_ctx_id, move_req)) => {
let description = match move_req.description.clone() {
Some(val) => val,
None => ensure_description(
Value::Object(
move_req.context.clone().into_inner().into(),
),
transaction_conn,
&schema_name,
)?,
};

let move_context_resp = operations::r#move(
old_ctx_id,
Json(move_req),
description,
transaction_conn,
true,
&user,
Expand Down
15 changes: 4 additions & 11 deletions crates/context_aware_config/src/api/context/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ use base64::prelude::*;
use cac_client::utils::json_to_sorted_string;
use chrono::Utc;
use diesel::{ExpressionMethods, QueryDsl, RunQueryDsl, SelectableHelper};
use serde_json::{json, Map, Value};
use serde_json::{Map, Value};
use service_utils::{helpers::extract_dimensions, service::types::SchemaName};
use superposition_macros::{bad_argument, unexpected_error, validation_error};
use superposition_macros::{unexpected_error, validation_error};
use superposition_types::{
database::{
models::cac::Context,
Expand Down Expand Up @@ -213,6 +213,7 @@ pub fn ensure_description(

pub fn create_ctx_from_put_req(
req: Json<PutReq>,
req_description: String,
conn: &mut DBConnection,
user: &User,
schema_name: &SchemaName,
Expand All @@ -221,14 +222,6 @@ pub fn create_ctx_from_put_req(
let condition_val = Value::Object(ctx_condition.clone().into());
let r_override = req.r#override.clone().into_inner();
let ctx_override = Value::Object(r_override.clone().into());
let description = if req.description.is_none() {
let ctx_condition_value = json!(ctx_condition);
ensure_description(ctx_condition_value, conn, schema_name)?
} else {
req.description
.clone()
.ok_or_else(|| bad_argument!("Description should not be empty"))?
};

let workspace_settings = get_workspace(schema_name, conn)?;

Expand Down Expand Up @@ -260,7 +253,7 @@ pub fn create_ctx_from_put_req(
last_modified_at: Utc::now().naive_utc(),
last_modified_by: user.get_email(),
weight,
description,
description: req_description,
change_reason,
})
}
Expand Down
12 changes: 4 additions & 8 deletions crates/context_aware_config/src/api/context/operations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,15 @@ use super::{

pub fn put(
req: Json<PutReq>,
description: String,
conn: &mut PooledConnection<ConnectionManager<PgConnection>>,
already_under_txn: bool,
user: &User,
schema_name: &SchemaName,
replace: bool,
) -> result::Result<PutResp> {
use contexts::dsl::contexts;
let new_ctx = create_ctx_from_put_req(req, conn, user, schema_name)?;
let new_ctx = create_ctx_from_put_req(req, description, conn, user, schema_name)?;

if already_under_txn {
diesel::sql_query("SAVEPOINT put_ctx_savepoint").execute(conn)?;
Expand Down Expand Up @@ -75,6 +76,7 @@ pub fn put(
pub fn r#move(
old_ctx_id: String,
req: Json<MoveReq>,
req_description: String,
conn: &mut PooledConnection<ConnectionManager<PgConnection>>,
already_under_txn: bool,
user: &User,
Expand All @@ -84,12 +86,6 @@ pub fn r#move(
let req = req.into_inner();
let ctx_condition = req.context.to_owned().into_inner();
let ctx_condition_value = Value::Object(ctx_condition.clone().into());
let description = if req.description.is_none() {
ensure_description(ctx_condition_value.clone(), conn, schema_name)?
} else {
req.description
.ok_or_else(|| bad_argument!("Description should not be empty"))?
};
let change_reason = req.change_reason.clone();

let new_ctx_id = hash(&ctx_condition_value);
Expand Down Expand Up @@ -134,7 +130,7 @@ pub fn r#move(
last_modified_at: Utc::now().naive_utc(),
last_modified_by: user.get_email(),
weight,
description,
description: req_description,
change_reason,
};

Expand Down
Loading