Extract admin operation validation

apps/admin-api/src/service.rs

@@ -7,8 +7,8 @@ use crank_core::{
     AuditSink, AuthProfile, CapabilityProfile, CommunityCapabilityProfile, EditionCapabilities,
     ExecutionMode, IdentityError, IdentityProvider, InvocationLog, InvocationLogId, NoopAuditSink,
     OperationSecurityLevel, OwnerOnlyPolicyEngine, PolicyEngine, ProductEdition, Protocol,
-    ResponseCachePolicy, Target, ToolQualityMappingRule, ToolQualityMappingSet,
+    ToolQualityMappingRule, ToolQualityMappingSet, ToolQualitySchemaKind, ToolQualitySchemaNode,
-    ToolQualitySchemaKind, ToolQualitySchemaNode, UsagePeriod, WorkspaceId,
+    UsagePeriod, WorkspaceId,
 };
 use crank_mapping::{MappingRule, MappingSet};
 use crank_registry::{
@@ -28,6 +28,7 @@ mod auth;
 mod demo;
 mod import_export;
 mod observability;
+mod operation_validation;
 mod operations;
 mod samples;
 mod secrets;
@@ -35,6 +36,9 @@ mod upstreams;
 mod workspaces;
 
 use crate::{auth::AuthSettings, error::ApiError, storage::LocalArtifactStorage};
+use operation_validation::{
+    validate_idempotency_policy, validate_protocol_target, validate_response_cache_policy,
+};
 
 #[derive(Clone)]
 pub struct AdminService {
@@ -431,16 +435,6 @@ fn build_request_preview(
     }))
 }
 
-fn validate_protocol_target(protocol: Protocol, target: &Target) -> Result<(), ApiError> {
-    let is_match = matches!((protocol, target), (Protocol::Rest, Target::Rest(_)));
-
-    if is_match {
-        return Ok(());
-    }
-
-    Err(ApiError::validation("protocol and target kind must match"))
-}
-
 fn validate_profile_display_name(value: &str) -> Result<String, ApiError> {
     let display_name = value.trim();
     if display_name.is_empty() {
@@ -686,95 +680,6 @@ fn agent_mcp_endpoint(workspace_slug: &str, agent_slug: &str) -> String {
     format!("/mcp/v1/{workspace_slug}/{agent_slug}")
 }
 
-fn validate_response_cache_policy(
-    target: &Target,
-    execution_config: &crank_core::ExecutionConfig,
-) -> Result<(), ApiError> {
-    let Some(ResponseCachePolicy { ttl_ms }) = execution_config.response_cache.as_ref() else {
-        return Ok(());
-    };
-
-    if *ttl_ms == 0 {
-        return Err(ApiError::validation_with_context(
-            "response cache ttl must be greater than zero".to_owned(),
-            json!({
-                "field": "execution_config.response_cache.ttl_ms",
-            }),
-        ));
-    }
-
-    match target {
-        Target::Rest(rest_target) if rest_target.method == crank_core::HttpMethod::Get => {}
-        _ => {
-            return Err(ApiError::validation_with_context(
-                "response cache is supported only for REST GET operations".to_owned(),
-                json!({
-                    "field": "execution_config.response_cache",
-                }),
-            ));
-        }
-    }
-
-    if execution_config.auth_profile_ref.is_some() {
-        return Err(ApiError::validation_with_context(
-            "response cache is not supported for operations with auth_profile_ref".to_owned(),
-            json!({
-                "field": "execution_config.auth_profile_ref",
-            }),
-        ));
-    }
-
-    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),
@@ -827,140 +732,7 @@ fn tool_quality_mapping_rule(rule: &MappingRule) -> ToolQualityMappingRule {
 #[cfg(test)]
 #[allow(clippy::items_after_test_module)]
 mod tests {
-    use std::collections::BTreeMap;
+    use super::{validate_profile_display_name, validate_profile_email};
-
-    use crank_core::{
-        ExecutionConfig, HttpMethod, IdempotencyMode, IdempotencyPolicy, ResponseCachePolicy,
-        RestTarget, Target,
-    };
-
-    use super::{
-        validate_idempotency_policy, validate_profile_display_name, validate_profile_email,
-        validate_response_cache_policy,
-    };
-
-    fn cacheable_execution_config() -> ExecutionConfig {
-        ExecutionConfig {
-            timeout_ms: 1_000,
-            retry_policy: None,
-            response_cache: Some(ResponseCachePolicy { ttl_ms: 5_000 }),
-            idempotency: None,
-            safety: 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()),
-            }),
-            safety: None,
-            auth_profile_ref: None,
-            headers: BTreeMap::new(),
-        }
-    }
-
-    #[test]
-    fn accepts_response_cache_for_rest_get() {
-        let target = Target::Rest(RestTarget {
-            base_url: "http://example.invalid".to_owned(),
-            method: HttpMethod::Get,
-            path_template: "/catalog".to_owned(),
-            static_headers: BTreeMap::new(),
-        });
-
-        let result = validate_response_cache_policy(&target, &cacheable_execution_config());
-
-        assert!(result.is_ok());
-    }
-
-    #[test]
-    fn rejects_response_cache_for_non_get_rest_operation() {
-        let target = Target::Rest(RestTarget {
-            base_url: "http://example.invalid".to_owned(),
-            method: HttpMethod::Post,
-            path_template: "/catalog".to_owned(),
-            static_headers: BTreeMap::new(),
-        });
-
-        let error =
-            validate_response_cache_policy(&target, &cacheable_execution_config()).unwrap_err();
-
-        assert!(matches!(error, crate::error::ApiError::Validation { .. }));
-        assert_eq!(
-            error.to_string(),
-            "response cache is supported only for REST GET operations"
-        );
-    }
-
-    #[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() {

apps/admin-api/src/service/operation_validation.rs

@@ -0,0 +1,241 @@
+use crank_core::{Protocol, ResponseCachePolicy, Target};
+use serde_json::json;
+
+use crate::error::ApiError;
+
+pub(super) fn validate_protocol_target(
+    protocol: Protocol,
+    target: &Target,
+) -> Result<(), ApiError> {
+    let is_match = matches!((protocol, target), (Protocol::Rest, Target::Rest(_)));
+
+    if is_match {
+        return Ok(());
+    }
+
+    Err(ApiError::validation("protocol and target kind must match"))
+}
+
+pub(super) fn validate_response_cache_policy(
+    target: &Target,
+    execution_config: &crank_core::ExecutionConfig,
+) -> Result<(), ApiError> {
+    let Some(ResponseCachePolicy { ttl_ms }) = execution_config.response_cache.as_ref() else {
+        return Ok(());
+    };
+
+    if *ttl_ms == 0 {
+        return Err(ApiError::validation_with_context(
+            "response cache ttl must be greater than zero".to_owned(),
+            json!({
+                "field": "execution_config.response_cache.ttl_ms",
+            }),
+        ));
+    }
+
+    match target {
+        Target::Rest(rest_target) if rest_target.method == crank_core::HttpMethod::Get => {}
+        _ => {
+            return Err(ApiError::validation_with_context(
+                "response cache is supported only for REST GET operations".to_owned(),
+                json!({
+                    "field": "execution_config.response_cache",
+                }),
+            ));
+        }
+    }
+
+    if execution_config.auth_profile_ref.is_some() {
+        return Err(ApiError::validation_with_context(
+            "response cache is not supported for operations with auth_profile_ref".to_owned(),
+            json!({
+                "field": "execution_config.auth_profile_ref",
+            }),
+        ));
+    }
+
+    Ok(())
+}
+
+pub(super) 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(())
+}
+
+#[cfg(test)]
+mod tests {
+    use std::collections::BTreeMap;
+
+    use crank_core::{
+        ExecutionConfig, HttpMethod, IdempotencyMode, IdempotencyPolicy, ResponseCachePolicy,
+        RestTarget, Target,
+    };
+
+    use super::{validate_idempotency_policy, validate_response_cache_policy};
+
+    fn cacheable_execution_config() -> ExecutionConfig {
+        ExecutionConfig {
+            timeout_ms: 1_000,
+            retry_policy: None,
+            response_cache: Some(ResponseCachePolicy { ttl_ms: 5_000 }),
+            idempotency: None,
+            safety: 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()),
+            }),
+            safety: None,
+            auth_profile_ref: None,
+            headers: BTreeMap::new(),
+        }
+    }
+
+    #[test]
+    fn accepts_response_cache_for_rest_get() {
+        let target = Target::Rest(RestTarget {
+            base_url: "http://example.invalid".to_owned(),
+            method: HttpMethod::Get,
+            path_template: "/catalog".to_owned(),
+            static_headers: BTreeMap::new(),
+        });
+
+        let result = validate_response_cache_policy(&target, &cacheable_execution_config());
+
+        assert!(result.is_ok());
+    }
+
+    #[test]
+    fn rejects_response_cache_for_non_get_rest_operation() {
+        let target = Target::Rest(RestTarget {
+            base_url: "http://example.invalid".to_owned(),
+            method: HttpMethod::Post,
+            path_template: "/catalog".to_owned(),
+            static_headers: BTreeMap::new(),
+        });
+
+        let error =
+            validate_response_cache_policy(&target, &cacheable_execution_config()).unwrap_err();
+
+        assert!(matches!(error, crate::error::ApiError::Validation { .. }));
+        assert_eq!(
+            error.to_string(),
+            "response cache is supported only for REST GET operations"
+        );
+    }
+
+    #[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"
+        );
+    }
+}