Add idempotency policy validation
This commit is contained in:
@@ -2336,6 +2336,7 @@ mod tests {
|
||||
timeout_ms: 1_000,
|
||||
retry_policy: None,
|
||||
response_cache: None,
|
||||
idempotency: None,
|
||||
auth_profile_ref: None,
|
||||
headers: BTreeMap::new(),
|
||||
},
|
||||
|
||||
@@ -2860,6 +2860,7 @@ impl AdminService {
|
||||
self.validate_operation_capabilities(payload.protocol, payload.security_level)?;
|
||||
validate_protocol_target(payload.protocol, &payload.target)?;
|
||||
validate_response_cache_policy(&payload.target, &payload.execution_config)?;
|
||||
validate_idempotency_policy(&payload.target, &payload.execution_config)?;
|
||||
payload.input_mapping.validate_paths()?;
|
||||
payload.output_mapping.validate_paths()?;
|
||||
Ok(())
|
||||
@@ -2869,6 +2870,7 @@ impl AdminService {
|
||||
self.validate_operation_capabilities(operation.protocol, operation.security_level)?;
|
||||
validate_protocol_target(operation.protocol, &operation.target)?;
|
||||
validate_response_cache_policy(&operation.target, &operation.execution_config)?;
|
||||
validate_idempotency_policy(&operation.target, &operation.execution_config)?;
|
||||
operation.input_mapping.validate_paths()?;
|
||||
operation.output_mapping.validate_paths()?;
|
||||
Ok(())
|
||||
@@ -3443,6 +3445,7 @@ fn demo_rest_operation_payload() -> OperationPayload {
|
||||
timeout_ms: 10_000,
|
||||
retry_policy: None,
|
||||
response_cache: None,
|
||||
idempotency: None,
|
||||
auth_profile_ref: None,
|
||||
headers: BTreeMap::new(),
|
||||
},
|
||||
@@ -3509,6 +3512,7 @@ fn demo_archived_operation_payload() -> OperationPayload {
|
||||
timeout_ms: 6_000,
|
||||
retry_policy: None,
|
||||
response_cache: None,
|
||||
idempotency: None,
|
||||
auth_profile_ref: None,
|
||||
headers: BTreeMap::new(),
|
||||
},
|
||||
@@ -3804,6 +3808,54 @@ fn validate_response_cache_policy(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn validate_idempotency_policy(
|
||||
target: &Target,
|
||||
execution_config: &crank_core::ExecutionConfig,
|
||||
) -> Result<(), ApiError> {
|
||||
let Some(policy) = execution_config.idempotency.as_ref() else {
|
||||
return Ok(());
|
||||
};
|
||||
|
||||
if policy.mode == crank_core::IdempotencyMode::Disabled {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
if policy.ttl_ms == 0 {
|
||||
return Err(ApiError::validation_with_context(
|
||||
"idempotency ttl must be greater than zero".to_owned(),
|
||||
json!({
|
||||
"field": "execution_config.idempotency.ttl_ms",
|
||||
}),
|
||||
));
|
||||
}
|
||||
|
||||
match target {
|
||||
Target::Rest(rest_target) if rest_target.method != crank_core::HttpMethod::Get => {}
|
||||
_ => {
|
||||
return Err(ApiError::validation_with_context(
|
||||
"idempotency is supported only for mutating REST operations".to_owned(),
|
||||
json!({
|
||||
"field": "execution_config.idempotency",
|
||||
}),
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
if policy.mode == crank_core::IdempotencyMode::Required
|
||||
&& policy.input_field.as_deref().is_none_or(str::is_empty)
|
||||
&& policy.header_name.as_deref().is_none_or(str::is_empty)
|
||||
{
|
||||
return Err(ApiError::validation_with_context(
|
||||
"required idempotency needs input_field or header_name".to_owned(),
|
||||
json!({
|
||||
"field": "execution_config.idempotency",
|
||||
}),
|
||||
));
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn tool_quality_schema_node(schema: &Schema) -> ToolQualitySchemaNode {
|
||||
ToolQualitySchemaNode {
|
||||
kind: tool_quality_schema_kind(&schema.kind),
|
||||
@@ -3858,10 +3910,14 @@ fn tool_quality_mapping_rule(rule: &MappingRule) -> ToolQualityMappingRule {
|
||||
mod tests {
|
||||
use std::collections::BTreeMap;
|
||||
|
||||
use crank_core::{ExecutionConfig, HttpMethod, ResponseCachePolicy, RestTarget, Target};
|
||||
use crank_core::{
|
||||
ExecutionConfig, HttpMethod, IdempotencyMode, IdempotencyPolicy, ResponseCachePolicy,
|
||||
RestTarget, Target,
|
||||
};
|
||||
|
||||
use super::{
|
||||
validate_profile_display_name, validate_profile_email, validate_response_cache_policy,
|
||||
validate_idempotency_policy, validate_profile_display_name, validate_profile_email,
|
||||
validate_response_cache_policy,
|
||||
};
|
||||
|
||||
fn cacheable_execution_config() -> ExecutionConfig {
|
||||
@@ -3869,6 +3925,23 @@ mod tests {
|
||||
timeout_ms: 1_000,
|
||||
retry_policy: None,
|
||||
response_cache: Some(ResponseCachePolicy { ttl_ms: 5_000 }),
|
||||
idempotency: None,
|
||||
auth_profile_ref: None,
|
||||
headers: BTreeMap::new(),
|
||||
}
|
||||
}
|
||||
|
||||
fn idempotent_execution_config(mode: IdempotencyMode) -> ExecutionConfig {
|
||||
ExecutionConfig {
|
||||
timeout_ms: 1_000,
|
||||
retry_policy: None,
|
||||
response_cache: None,
|
||||
idempotency: Some(IdempotencyPolicy {
|
||||
mode,
|
||||
ttl_ms: 5_000,
|
||||
input_field: Some("request_id".to_owned()),
|
||||
header_name: Some("Idempotency-Key".to_owned()),
|
||||
}),
|
||||
auth_profile_ref: None,
|
||||
headers: BTreeMap::new(),
|
||||
}
|
||||
@@ -3907,6 +3980,67 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn accepts_idempotency_for_mutating_rest_operation() {
|
||||
let target = Target::Rest(RestTarget {
|
||||
base_url: "http://example.invalid".to_owned(),
|
||||
method: HttpMethod::Post,
|
||||
path_template: "/orders".to_owned(),
|
||||
static_headers: BTreeMap::new(),
|
||||
});
|
||||
|
||||
let result = validate_idempotency_policy(
|
||||
&target,
|
||||
&idempotent_execution_config(IdempotencyMode::Required),
|
||||
);
|
||||
|
||||
assert!(result.is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_idempotency_for_rest_get() {
|
||||
let target = Target::Rest(RestTarget {
|
||||
base_url: "http://example.invalid".to_owned(),
|
||||
method: HttpMethod::Get,
|
||||
path_template: "/orders".to_owned(),
|
||||
static_headers: BTreeMap::new(),
|
||||
});
|
||||
|
||||
let error = validate_idempotency_policy(
|
||||
&target,
|
||||
&idempotent_execution_config(IdempotencyMode::Optional),
|
||||
)
|
||||
.unwrap_err();
|
||||
|
||||
assert!(matches!(error, crate::error::ApiError::Validation { .. }));
|
||||
assert_eq!(
|
||||
error.to_string(),
|
||||
"idempotency is supported only for mutating REST operations"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_required_idempotency_without_key_source() {
|
||||
let target = Target::Rest(RestTarget {
|
||||
base_url: "http://example.invalid".to_owned(),
|
||||
method: HttpMethod::Post,
|
||||
path_template: "/orders".to_owned(),
|
||||
static_headers: BTreeMap::new(),
|
||||
});
|
||||
let mut config = idempotent_execution_config(IdempotencyMode::Required);
|
||||
let policy = config.idempotency.as_mut().unwrap();
|
||||
policy.input_field = None;
|
||||
policy.header_name = None;
|
||||
|
||||
let error = validate_idempotency_policy(&target, &config).unwrap_err();
|
||||
|
||||
assert!(matches!(error, crate::error::ApiError::Validation { .. }));
|
||||
assert_eq!(
|
||||
error.to_string(),
|
||||
"required idempotency needs input_field or header_name"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn validates_profile_identity_fields() {
|
||||
assert_eq!(
|
||||
|
||||
@@ -1852,6 +1852,7 @@ mod tests {
|
||||
timeout_ms: 1_000,
|
||||
retry_policy: None,
|
||||
response_cache: None,
|
||||
idempotency: None,
|
||||
auth_profile_ref: None,
|
||||
headers: BTreeMap::new(),
|
||||
},
|
||||
|
||||
@@ -82,6 +82,7 @@ fn operation() -> RegistryOperation {
|
||||
timeout_ms: 10_000,
|
||||
retry_policy: None,
|
||||
response_cache: None,
|
||||
idempotency: None,
|
||||
auth_profile_ref: None,
|
||||
headers: BTreeMap::new(),
|
||||
},
|
||||
|
||||
@@ -56,9 +56,9 @@ pub use observability::{
|
||||
InvocationLevel, InvocationLog, InvocationSource, InvocationStatus, UsagePeriod, UsageRollup,
|
||||
};
|
||||
pub use operation::{
|
||||
ConfigExport, ExecutionConfig, GeneratedDraft, GeneratedDraftStatus, Operation,
|
||||
OperationStatus, ResponseCachePolicy, RestTarget, RetryPolicy, Samples, Target,
|
||||
ToolDescription, ToolExample, WizardState,
|
||||
ConfigExport, ExecutionConfig, GeneratedDraft, GeneratedDraftStatus, IdempotencyMode,
|
||||
IdempotencyPolicy, Operation, OperationStatus, ResponseCachePolicy, RestTarget, RetryPolicy,
|
||||
Samples, Target, ToolDescription, ToolExample, WizardState,
|
||||
};
|
||||
pub use protocol::{AuthKind, ExportMode, HttpMethod, Protocol};
|
||||
pub use secret::{Secret, SecretKind, SecretStatus, SecretVersion};
|
||||
|
||||
@@ -52,7 +52,25 @@ pub struct ResponseCachePolicy {
|
||||
pub ttl_ms: u64,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)]
|
||||
#[serde(rename_all = "snake_case")]
|
||||
pub enum IdempotencyMode {
|
||||
Disabled,
|
||||
Optional,
|
||||
Required,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub struct IdempotencyPolicy {
|
||||
pub mode: IdempotencyMode,
|
||||
pub ttl_ms: u64,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub input_field: Option<String>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub header_name: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize)]
|
||||
pub struct ExecutionConfig {
|
||||
pub timeout_ms: u64,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
@@ -60,6 +78,8 @@ pub struct ExecutionConfig {
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub response_cache: Option<ResponseCachePolicy>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub idempotency: Option<IdempotencyPolicy>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub auth_profile_ref: Option<AuthProfileId>,
|
||||
#[serde(default, skip_serializing_if = "BTreeMap::is_empty")]
|
||||
pub headers: BTreeMap<String, String>,
|
||||
@@ -352,6 +372,7 @@ updated_at: 2026-03-25T08:10:00Z
|
||||
timeout_ms: 10_000,
|
||||
retry_policy: None,
|
||||
response_cache: None,
|
||||
idempotency: None,
|
||||
auth_profile_ref: Some(AuthProfileId::new("auth_01")),
|
||||
headers: BTreeMap::new(),
|
||||
},
|
||||
|
||||
@@ -2053,7 +2053,7 @@ mod tests {
|
||||
retry_policy: Some(RetryPolicy { max_attempts: 3 }),
|
||||
auth_profile_ref: Some("auth_crank".into()),
|
||||
headers: BTreeMap::from([("X-Request-Id".to_owned(), "static".to_owned())]),
|
||||
response_cache: None,
|
||||
..ExecutionConfig::default()
|
||||
},
|
||||
tool_description: ToolDescription {
|
||||
title: "Create lead".to_owned(),
|
||||
|
||||
@@ -69,6 +69,7 @@ fn no_input_get_operation() -> Operation<Schema, MappingSet> {
|
||||
timeout_ms: 1_000,
|
||||
retry_policy: None,
|
||||
response_cache: None,
|
||||
idempotency: None,
|
||||
auth_profile_ref: None,
|
||||
headers: BTreeMap::new(),
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user