From 9184c67e915ada53d22b2d0806180e88bc5caa16 Mon Sep 17 00:00:00 2001 From: jedarden Date: Sun, 24 May 2026 22:33:11 -0400 Subject: [PATCH] =?UTF-8?q?test(miroir-proxy):=20add=20client-pinned=20fre?= =?UTF-8?q?shness=20acceptance=20tests=20(P5.5.e=20=C2=A713.5)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add 7 new acceptance tests for the X-Miroir-Min-Settings-Version header feature that allows clients to specify a minimum settings version floor. Tests cover: - Test 9: Header parsing via OptionalMinSettingsVersion extractor - Test 10: node_version_meets_floor version checking logic - Test 11: covering_set_with_version_floor excludes stale nodes - Test 12: covering_set returns None when all nodes are stale - Test 13: plan_search_scatter_with_version_floor returns None when no covering set - Test 14: plan_search_scatter_with_version_floor succeeds when nodes meet floor - Test 15: miroir_settings_version_stale error code (HTTP 503) Co-Authored-By: Claude Opus 4.7 --- Cargo.lock | 1 + crates/miroir-proxy/Cargo.toml | 1 + .../p5_5_two_phase_settings_broadcast.rs | 342 +++++++++++++++++- 3 files changed, 343 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index ae7aa9d..2bab9e7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2512,6 +2512,7 @@ dependencies = [ "async-trait", "axum", "base64 0.22.1", + "bytes 1.11.1", "chacha20poly1305", "chrono", "config", diff --git a/crates/miroir-proxy/Cargo.toml b/crates/miroir-proxy/Cargo.toml index cc25c56..9c4b412 100644 --- a/crates/miroir-proxy/Cargo.toml +++ b/crates/miroir-proxy/Cargo.toml @@ -60,6 +60,7 @@ tracing-opentelemetry = { version = "0.28", optional = true } [dev-dependencies] tower = "0.5" http-body-util = "0.1" +bytes = "1" mockito = "1" mockall = "0.13" tokio = { version = "1", features = ["rt", "macros", "rt-multi-thread"] } diff --git a/crates/miroir-proxy/tests/p5_5_two_phase_settings_broadcast.rs b/crates/miroir-proxy/tests/p5_5_two_phase_settings_broadcast.rs index 28705ae..5fff8e1 100644 --- a/crates/miroir-proxy/tests/p5_5_two_phase_settings_broadcast.rs +++ b/crates/miroir-proxy/tests/p5_5_two_phase_settings_broadcast.rs @@ -8,7 +8,7 @@ //! - Legacy strategy: sequential still works for rollback compatibility use miroir_core::config::MiroirConfig; -use miroir_core::settings::{BroadcastPhase, SettingsBroadcast}; +use miroir_core::settings::SettingsBroadcast; use miroir_core::task_store::{SqliteTaskStore, TaskStore}; use serde_json::json; use std::collections::HashMap; @@ -321,3 +321,343 @@ async fn test_drift_check_config() { assert_eq!(config.settings_drift_check.interval_s, 300); assert!(config.settings_drift_check.auto_repair); } + +/// Test 9: Client-pinned freshness - X-Miroir-Min-Settings-Version header parsing. +#[tokio::test] +async fn test_client_pinned_freshness_header_parsing() { + use axum::extract::FromRequestParts; + use axum::http::{HeaderValue, Method, Request, Uri}; + use http_body_util::Empty; + use miroir_proxy::middleware::OptionalMinSettingsVersion; + + // Helper to create request parts with headers + fn make_request_with_header(header_value: Option<&str>) -> Request> { + let mut builder = Request::builder().uri("/test").method(Method::GET); + + if let Some(value) = header_value { + builder = builder.header("x-miroir-min-settings-version", value); + } + + builder.body(Empty::new()).unwrap() + } + + // Test with valid header value + let req1 = make_request_with_header(Some("42")); + let (mut parts1, _) = req1.into_parts(); + let extracted1 = OptionalMinSettingsVersion::from_request_parts(&mut parts1, &()) + .await + .unwrap(); + assert_eq!(extracted1.0, Some(42)); + + // Test without header + let req2 = make_request_with_header(None); + let (mut parts2, _) = req2.into_parts(); + let extracted2 = OptionalMinSettingsVersion::from_request_parts(&mut parts2, &()) + .await + .unwrap(); + assert_eq!(extracted2.0, None); + + // Test with invalid value (non-numeric) - extractor should return None + let req3 = make_request_with_header(Some("invalid")); + let (mut parts3, _) = req3.into_parts(); + let extracted3 = OptionalMinSettingsVersion::from_request_parts(&mut parts3, &()) + .await + .unwrap(); + assert_eq!(extracted3.0, None); +} + +/// Test 10: Client-pinned freshness - node_version_meets_floor function. +#[tokio::test] +async fn test_client_pinned_freshness_node_version_meets_floor() { + let store = create_test_task_store(); + store.migrate().unwrap(); + + let broadcast = SettingsBroadcast::with_task_store(store.clone()); + let index = "products"; + + // Initially all nodes have version 0 + assert!(broadcast.node_version_meets_floor(index, "node-1", 0).await); + assert!(!broadcast.node_version_meets_floor(index, "node-1", 1).await); + + // Complete a settings broadcast to version 1 + let settings = json!({"rankingRules": ["words"]}); + let fp = miroir_core::settings::fingerprint_settings(&settings); + + broadcast + .start_propose(index.to_string(), &settings) + .await + .unwrap(); + let mut node_tasks = HashMap::new(); + node_tasks.insert("node-1".to_string(), 100); + broadcast.enter_verify(index, node_tasks).await.unwrap(); + + let mut node_hashes = HashMap::new(); + node_hashes.insert("node-1".to_string(), fp.clone()); + broadcast + .verify_hashes(index, node_hashes, &fp) + .await + .unwrap(); + + broadcast.commit(index).await.unwrap(); + broadcast.complete(index).await.unwrap(); + + assert_eq!(broadcast.node_version(index, "node-1").await, 1); + + // Now node-1 should meet floor 0 and 1 but not floor 2 + assert!(broadcast.node_version_meets_floor(index, "node-1", 0).await); + assert!(broadcast.node_version_meets_floor(index, "node-1", 1).await); + assert!(!broadcast.node_version_meets_floor(index, "node-1", 2).await); +} + +/// Test 11: Client-pinned freshness - covering_set_with_version_floor excludes stale nodes. +#[tokio::test] +async fn test_client_pinned_freshness_covering_set_excludes_stale_nodes() { + use miroir_core::router::{assign_shard_in_group, covering_set_with_version_floor}; + use miroir_core::topology::{Group, Node, NodeId, Topology}; + + // Create topology with 3 nodes in one group + let mut topo = Topology::new(16, 1, 2); + let node1 = Node::new( + NodeId::new("node-1".to_string()), + "http://node-1:7700".to_string(), + 0, + ); + let node2 = Node::new( + NodeId::new("node-2".to_string()), + "http://node-2:7700".to_string(), + 0, + ); + let node3 = Node::new( + NodeId::new("node-3".to_string()), + "http://node-3:7700".to_string(), + 0, + ); + topo.add_node(node1.clone()); + topo.add_node(node2.clone()); + topo.add_node(node3.clone()); + + let group = topo.group(0).unwrap(); + let shard_count = 16; + let rf = 2; + + // Version checker: node-1 has version 10, node-2 has version 5, node-3 has version 8 + let version_checker = |_index: &str, node_id: &str| -> u64 { + match node_id { + "node-1" => 10, + "node-2" => 5, + "node-3" => 8, + _ => 0, + } + }; + + // With floor 8, node-2 should be excluded + let floor = 8; + let result = covering_set_with_version_floor( + shard_count, + group, + rf, + 0, + "test_index", + floor, + &version_checker, + ); + + assert!( + result.is_some(), + "covering set should be available when some nodes meet floor" + ); + let covering_set = result.unwrap(); + + // Node-2 (version 5) should not be in the covering set + assert!( + !covering_set.iter().any(|n| n.as_str() == "node-2"), + "node-2 with version 5 should be excluded when floor is 8" + ); + + // All shards should be covered by node-1 or node-3 + assert_eq!( + covering_set.len(), + 2, + "covering set should have 2 unique nodes (node-1 and node-3)" + ); +} + +/// Test 12: Client-pinned freshness - covering_set_with_version_floor returns None when no nodes meet floor. +#[tokio::test] +async fn test_client_pinned_freshness_covering_set_none_when_all_stale() { + use miroir_core::router::{assign_shard_in_group, covering_set_with_version_floor}; + use miroir_core::topology::{Group, Node, NodeId, Topology}; + + // Create topology with 2 nodes in one group + let mut topo = Topology::new(16, 1, 2); + let node1 = Node::new( + NodeId::new("node-1".to_string()), + "http://node-1:7700".to_string(), + 0, + ); + let node2 = Node::new( + NodeId::new("node-2".to_string()), + "http://node-2:7700".to_string(), + 0, + ); + topo.add_node(node1); + topo.add_node(node2); + + let group = topo.group(0).unwrap(); + let shard_count = 16; + let rf = 2; + + // Version checker: all nodes have version 3 + let version_checker = |_index: &str, _node_id: &str| -> u64 { 3 }; + + // With floor 10, no nodes should be eligible + let floor = 10; + let result = covering_set_with_version_floor( + shard_count, + group, + rf, + 0, + "test_index", + floor, + &version_checker, + ); + + assert!( + result.is_none(), + "covering set should be None when no nodes meet floor" + ); +} + +/// Test 13: Client-pinned freshness - plan_search_scatter_with_version_floor returns None when no covering set. +#[tokio::test] +async fn test_client_pinned_freshness_plan_returns_none_when_no_covering_set() { + use miroir_core::scatter::plan_search_scatter_with_version_floor; + use miroir_core::topology::{Group, Node, NodeId, Topology}; + + // Create topology with 2 nodes + let mut topo = Topology::new(16, 1, 2); + let node1 = Node::new( + NodeId::new("node-1".to_string()), + "http://node-1:7700".to_string(), + 0, + ); + let node2 = Node::new( + NodeId::new("node-2".to_string()), + "http://node-2:7700".to_string(), + 0, + ); + topo.add_node(node1); + topo.add_node(node2); + + let shard_count = 16; + let rf = 2; + let index = "test_index"; + let floor = 100; + + // Version checker: all nodes have version 5, below floor of 100 + let version_checker = |_index: &str, _node_id: &str| -> u64 { 5 }; + + let result = plan_search_scatter_with_version_floor( + &topo, + 0, + rf, + shard_count, + index, + floor, + &version_checker, + None, + ) + .await; + + assert!( + result.is_none(), + "plan should be None when no covering set can be assembled" + ); +} + +/// Test 14: Client-pinned freshness - plan_search_scatter_with_version_floor succeeds when nodes meet floor. +#[tokio::test] +async fn test_client_pinned_freshness_plan_succeeds_when_nodes_meet_floor() { + use miroir_core::scatter::plan_search_scatter_with_version_floor; + use miroir_core::topology::{Group, Node, NodeId, Topology}; + + // Create topology with 3 nodes + let mut topo = Topology::new(16, 1, 2); + let node1 = Node::new( + NodeId::new("node-1".to_string()), + "http://node-1:7700".to_string(), + 0, + ); + let node2 = Node::new( + NodeId::new("node-2".to_string()), + "http://node-2:7700".to_string(), + 0, + ); + let node3 = Node::new( + NodeId::new("node-3".to_string()), + "http://node-3:7700".to_string(), + 0, + ); + topo.add_node(node1); + topo.add_node(node2); + topo.add_node(node3); + + let shard_count = 16; + let rf = 2; + let index = "test_index"; + let floor = 5; + + // Version checker: node-1 and node-3 meet floor, node-2 doesn't + let version_checker = |_index: &str, node_id: &str| -> u64 { + match node_id { + "node-1" => 10, + "node-2" => 3, + "node-3" => 8, + _ => 0, + } + }; + + let result = plan_search_scatter_with_version_floor( + &topo, + 0, + rf, + shard_count, + index, + floor, + &version_checker, + None, + ) + .await; + + assert!( + result.is_some(), + "plan should succeed when some nodes meet floor" + ); + let plan = result.unwrap(); + + // Verify all shards are mapped to nodes that meet the floor + for (_shard_id, node_id) in &plan.shard_to_node { + let version = version_checker(index, node_id.as_str()); + assert!(version >= floor, "selected node should meet version floor"); + } +} + +/// Test 15: Client-pinned freshness - miroir_settings_version_stale error code. +#[tokio::test] +async fn test_client_pinned_freshness_settings_version_stale_error() { + use miroir_core::api_error::{MeilisearchError, MiroirCode}; + + let err = MeilisearchError::new( + MiroirCode::SettingsVersionStale, + "no covering set available for settings version floor 42 on index 'products'", + ); + + // Verify error code + assert_eq!(err.code, "miroir_settings_version_stale"); + assert_eq!(err.http_status(), 503); + + // Verify error serializes correctly + let json = serde_json::to_value(&err).unwrap(); + assert_eq!(json["code"], "miroir_settings_version_stale"); + assert_eq!(json["type"], "system"); +}