From 7067541f5c2dd91db6d6047b45692b95399ca18d Mon Sep 17 00:00:00 2001 From: Alexander Onnikov Date: Wed, 24 Sep 2025 12:01:13 +0700 Subject: [PATCH] fix conditional patch Signed-off-by: Alexander Onnikov --- server/src/handlers.rs | 191 ++++++++++++++++++++++++++++++++++++++++- tests/src/patch.rs | 42 +++++++++ 2 files changed, 232 insertions(+), 1 deletion(-) diff --git a/server/src/handlers.rs b/server/src/handlers.rs index 91fe001569..3a8c664705 100644 --- a/server/src/handlers.rs +++ b/server/src/handlers.rs @@ -498,12 +498,13 @@ fn validate_patch_conditionals( match any_match(req, etag)? { Some(false) => Err(ApiError::PreconditionFailed), - _ => { + Some(true) => { let parts_data = parts.iter().map(|p| &p.data).collect::>(); let parts_etag = recovery::object_etag(parts_data)?; Ok(Some(ConditionalMatch::IfMatch(parts_etag))) } + None => Ok(None), } } @@ -528,3 +529,191 @@ fn validate_put_conditionals( }, } } + +#[cfg(test)] +mod tests { + use super::*; + use actix_web::test::TestRequest; + + fn object_part(etag: &str) -> ObjectPart { + ObjectPart { + inline: None, + data: PartData { + workspace: Uuid::new_v4(), + key: "test".to_string(), + part: 0, + size: 0, + blob: "test".to_string(), + etag: etag.to_owned(), + date: Utc::now(), + headers: None, + meta: None, + merge_strategy: None, + }, + } + } + + #[test] + fn test_objectpart_etag() { + let parts = vec![object_part("foo"), object_part("bar")]; + + let etag = objectpart_etag(&parts); + assert_eq!(etag, Some(EntityTag::new_strong("bar".to_owned()))); + } + + #[test] + fn test_validate_patch_conditionals_none_none() { + let req = TestRequest::default().to_http_request(); + let parts = vec![]; + + let res = validate_patch_conditionals(&req, &parts); + assert!(res.is_ok()); + assert_eq!(res.unwrap(), None); + } + + #[test] + fn test_validate_patch_conditionals_none_some() { + let req = TestRequest::default().to_http_request(); + let parts = vec![object_part("foo")]; + + let res = validate_patch_conditionals(&req, &parts); + assert!(res.is_ok()); + assert_eq!(res.unwrap(), None); + } + + #[test] + fn test_validate_patch_conditionals_some_some_match() { + let req = TestRequest::default() + .insert_header((header::IF_MATCH, "\"foo\"")) + .to_http_request(); + let parts = vec![object_part("foo")]; + let etag = recovery::object_etag(vec![&parts[0].data]).unwrap(); + + let res = validate_patch_conditionals(&req, &parts); + assert!(res.is_ok()); + assert_eq!(res.unwrap(), Some(ConditionalMatch::IfMatch(etag))); + } + + #[test] + fn test_validate_patch_conditionals_some_some_not_match() { + let req = TestRequest::default() + .insert_header((header::IF_MATCH, "\"bar\"")) + .to_http_request(); + let parts = vec![object_part("foo")]; + + let res = validate_patch_conditionals(&req, &parts); + assert!(res.is_err()); + assert_eq!( + res.unwrap_err().to_string(), + ApiError::PreconditionFailed.to_string() + ); + } + + #[test] + fn test_validate_put_conditionals_none_none() { + let req = TestRequest::default().to_http_request(); + let parts = vec![]; + + let res = validate_put_conditionals(&req, &parts); + assert!(res.is_ok()); + assert_eq!(res.unwrap(), None); + } + + #[test] + fn test_validate_put_conditionals_none_some() { + let req = TestRequest::default().to_http_request(); + let parts = vec![object_part("foo")]; + + let res = validate_put_conditionals(&req, &parts); + assert!(res.is_ok()); + assert_eq!(res.unwrap(), None); + } + + #[test] + fn test_validate_put_conditionals_some_some_match() { + let req = TestRequest::default() + .insert_header((header::IF_MATCH, "\"foo\"")) + .to_http_request(); + let parts = vec![object_part("foo")]; + let etag = recovery::object_etag(vec![&parts[0].data]).unwrap(); + + let res = validate_put_conditionals(&req, &parts); + assert!(res.is_ok()); + assert_eq!(res.unwrap(), Some(ConditionalMatch::IfMatch(etag))); + } + + #[test] + fn test_validate_put_conditionals_some_some_not_match() { + let req = TestRequest::default() + .insert_header((header::IF_MATCH, "\"bar\"")) + .to_http_request(); + let parts = vec![object_part("foo")]; + + let res = validate_put_conditionals(&req, &parts); + assert!(res.is_err()); + assert_eq!( + res.unwrap_err().to_string(), + ApiError::PreconditionFailed.to_string() + ); + } + + #[test] + fn test_validate_put_conditionals_if_match_if_none_match() { + let req = TestRequest::default() + .insert_header((header::IF_MATCH, "\"bar\"")) + .insert_header((header::IF_NONE_MATCH, "*")) + .to_http_request(); + let parts = vec![object_part("foo")]; + + let res = validate_put_conditionals(&req, &parts); + assert!(res.is_err()); + assert_eq!( + res.unwrap_err().to_string(), + ApiError::PreconditionFailed.to_string() + ); + } + + #[test] + fn test_validate_put_conditionals_if_none_match_match() { + let req = TestRequest::default() + .insert_header((header::IF_NONE_MATCH, "\"bar\"")) + .to_http_request(); + let parts = vec![object_part("foo")]; + + let res = validate_put_conditionals(&req, &parts); + assert!(res.is_ok()); + assert_eq!( + res.unwrap(), + Some(ConditionalMatch::IfNoneMatch("*".to_owned())) + ); + } + + #[test] + fn test_validate_put_conditionals_if_none_match_not_match() { + let req = TestRequest::default() + .insert_header((header::IF_NONE_MATCH, "\"foo\"")) + .to_http_request(); + let parts = vec![object_part("foo")]; + + let res = validate_put_conditionals(&req, &parts); + assert_eq!( + res.unwrap_err().to_string(), + ApiError::PreconditionFailed.to_string() + ); + } + + #[test] + fn test_validate_put_conditionals_if_none_match_err() { + let req = TestRequest::default() + .insert_header((header::IF_NONE_MATCH, "*")) + .to_http_request(); + let parts = vec![object_part("foo")]; + + let res = validate_put_conditionals(&req, &parts); + assert!(res.is_err()); + assert_eq!( + res.unwrap_err().to_string(), + ApiError::PreconditionFailed.to_string() + ); + } +} diff --git a/tests/src/patch.rs b/tests/src/patch.rs index 3a161eb6a0..aa2483f3d5 100644 --- a/tests/src/patch.rs +++ b/tests/src/patch.rs @@ -288,3 +288,45 @@ pub async fn put_and_patch_conditional( Ok(()) } + +#[tanu::test] +pub async fn put_and_patch_json_err() -> eyre::Result<()> { + let key = random_key(); + + let http = Client::new(); + + let initial = json!({ + "a": 1 + }); + + // create new blob + let res = http + .key_put(&key) + .body(json::to_string(&initial)?) + .header("huly-merge-strategy", "jsonpatch") + .header("content-type", "application/json") + .send() + .await?; + + check!(res.status().is_success(), "{:#?}", res); + + let patch = json!([ + { "hop": "add", "path": "/a/b/c", "value": 0, "safe": false }, + ]); + + let res = http + .key_patch(&key) + .body(json::to_string(&patch)?) + .header("content-type", "application/json-patch+json") + .send() + .await?; + + check!(res.status().is_success(), "{:#?}", res); + let res = http.key_get(&key).send().await?; + check!(res.status().is_success(), "{:#?}", res); + + let json = res.json::().await?; + assert_eq!(json, json!({ "a": 1 })); + + Ok(()) +}