diff --git a/docs/internal/feature-flags/api-writes.md b/docs/internal/feature-flags/api-writes.md index c8ff3ab895d9..9240154948d1 100644 --- a/docs/internal/feature-flags/api-writes.md +++ b/docs/internal/feature-flags/api-writes.md @@ -87,7 +87,7 @@ A stored version 2 row that is not remote-configured and carries no encrypted pa - Writer-only rules (`check_writer_rules` in `facade/config_writes.py`, currently the `$false`, `$true` and `"null"` reservation and the regex check below) apply to a created or replacing document and to the stored document being enabled. The shared validator, and with it the flags caches and the stored side of an update, does not apply them: tightening a writer rule never hides a stored row from evaluation or blocks replacing it. A `"null"` value the stored document already serves, as the default, a rule value or a variant value, passes the reservation, as in v1. - Experiment-rule variant keys follow the v1 variant-key rule for every return type: a new `"null"` key is rejected, writer-only, unless the stored rule with the same ID already uses that key. The key belongs to that rule's experiment, so another rule cannot take it over. - `regex` and `not_regex` patterns must compile with Python's `re`, as on the v1 write path. This is a writer-only rule: readers store an uncompilable pattern and report an evaluation error when evaluation reaches it. The flags service compiles with `fancy_regex`, whose dialect differs at the edges, so, as in v1, a pattern the stored document already holds is not checked again, and enabling checks no pattern. -- Rule types are `targeted_release`, `percentage_rollout` and `experiment`. An experiment rule names no Experiment: an Experiment links to its rule through the rule ID (`Experiment.feature_flag_rule_id`), and writing the rule creates or links no Experiment row. The rule splits the subjects its `rollout_percentage` enrolls across two to twenty `variants` (unique keys of letters, digits, `-` and `_`; weights with at most two decimal places that total exactly 100; values of the flag's return type, equal values allowed), returns the flag default while `paused` is true, and may carry a `holdout` that belongs to the rule: `{seed, exclusion_percentage}`, where the client chooses `exclusion_percentage` and the server owns the seed. At `/flags?v=3` a variant assignment reports `experiment_split` with the rule ID and the variant key; no response carries an experiment or holdout ID. An `experiment_id` on the rule and an `id` in its holdout are unknown fields, whatever their value, `null` included. +- Rule types are `targeted_release`, `percentage_rollout` and `experiment`. An experiment rule names no Experiment: an Experiment links to its rule through the rule ID (`Experiment.feature_flag_rule_id`), and writing the rule creates or links no Experiment row. The rule splits the subjects its `rollout_percentage` enrolls across two to twenty `variants` (unique keys of letters, digits, `-` and `_`; weights with at most two decimal places that total exactly 100; values of the flag's return type, equal values allowed), returns the flag default while `paused` is true, and may carry a `holdout` that belongs to the rule: `{seed, exclusion_percentage}`, where the client chooses `exclusion_percentage` and the server owns the seed. At `/flags?v=4` a variant assignment reports `experiment_split` with the rule ID and the variant key; no response carries an experiment or holdout ID. An `experiment_id` on the rule and an `id` in its holdout are unknown fields, whatever their value, `null` included. - Rule IDs, the seeds of percentage rollouts and experiment rules, and holdout seeds are server-owned and identify rules rather than list positions. An echoed ID keeps its seeds through reordering and unrelated edits, and a rule keeps its seed while it stays a percentage rollout or an experiment rule; a rule that omits its ID is new and receives fresh identity. An unknown ID, an ID used twice, a client-chosen seed, and a changed seed are all rejected. - `FeatureFlag.version` is required on every update and must equal the locked row's value. Unlike v1 there is no `original_flag` merge: a stale token is a conflict even when no individual field clashes. The token applies to disabling and deleting too. - Identity resolution, validation and the enable check run against the locked row inside the existing update transaction. A stored document with an unsupported shape or semantics is rejected rather than replaced or enabled. Configuration and metadata byte limits apply to a replacement, so an oversized stored document can be edited back under a lowered limit; enabling requires the stored document to fit the current limits. diff --git a/rust/feature-flags/src/api/endpoint.rs b/rust/feature-flags/src/api/endpoint.rs index 94d21c9b57af..715692451926 100644 --- a/rust/feature-flags/src/api/endpoint.rs +++ b/rust/feature-flags/src/api/endpoint.rs @@ -5,7 +5,7 @@ use crate::{ errors::{ClientFacingError, FlagError}, flags_rate_limiter::RateLimitResult, types::{ - ConfigResponse, FlagsQueryParams, FlagsResponse, FlagsResponseV3, LegacyFlagsResponse, + ConfigResponse, FlagsQueryParams, FlagsResponse, FlagsResponseV4, LegacyFlagsResponse, ServiceResponse, }, }, @@ -167,13 +167,13 @@ fn get_minimal_flags_response( headers: &HeaderMap, version: Option<&str>, is_from_legacy_decide: bool, - v3_enabled: bool, + v4_enabled: bool, ) -> Json { let request_id = extract_request_id(headers); // `None` is preserved so `get_versioned_response` can apply its // decide/non-decide defaults. - let version_num = version.map(|v| v.parse::().unwrap_or(1)); + let version_num = version.map(parse_version); let mut config = ConfigResponse::new(); config.set( @@ -192,12 +192,16 @@ fn get_minimal_flags_response( response.config = config; let (service_response, _) = - get_versioned_response(is_from_legacy_decide, version_num, v3_enabled, response).expect( + get_versioned_response(is_from_legacy_decide, version_num, v4_enabled, response).expect( "get_versioned_response is total for any (bool, Option, bool, FlagsResponse)", ); Json(service_response) } +fn parse_version(version: &str) -> i32 { + version.parse::().unwrap_or(1) +} + /// Determines the response format based on whether the request came from decide and the version parameter. /// /// When the request is from decide (X-Original-Endpoint: decide): @@ -207,15 +211,15 @@ fn get_minimal_flags_response( /// - v>=4 -> FlagsV2 response format /// /// When the request is not from decide: -/// - v>=3 -> FlagsV3 response format when `v3_enabled`, otherwise FlagsV2 -/// - v=2 -> FlagsV2 response format +/// - v=4 -> FlagsV4 response format when `v4_enabled`, otherwise FlagsV2 +/// - any other v>=2 -> FlagsV2 response format /// - v=1 or missing -> FlagsV1 response format /// /// Returns a tuple of (response, format_name) for logging purposes fn get_versioned_response( is_from_legacy_decide: bool, version: Option, - v3_enabled: bool, + v4_enabled: bool, response: FlagsResponse, ) -> Result<(ServiceResponse, &'static str), FlagError> { if is_from_legacy_decide { @@ -247,10 +251,10 @@ fn get_versioned_response( )) } } - } else if serves_flags_v3(is_from_legacy_decide, version, v3_enabled) { + } else if serves_flags_v4(is_from_legacy_decide, version, v4_enabled) { Ok(( - ServiceResponse::V3(FlagsResponseV3::from_response(response)), - "FlagsV3", + ServiceResponse::V4(FlagsResponseV4::from_response(response)), + "FlagsV4", )) } else if version.is_some_and(|v| v >= 2) { Ok((ServiceResponse::V2(response), "FlagsV2")) @@ -262,10 +266,13 @@ fn get_versioned_response( } } -/// True when [`get_versioned_response`] returns the v3 record. The body log +/// True when [`get_versioned_response`] returns the v4 record. The body log /// uses it too, so the logged record matches the record the client receives. -fn serves_flags_v3(is_from_legacy_decide: bool, version: Option, v3_enabled: bool) -> bool { - v3_enabled && !is_from_legacy_decide && version.is_some_and(|v| v >= 3) +/// +/// Only an exact `v=4` selects it: clients already send `v=3` and arbitrary +/// `v` values (cache busters, typos), and those keep the v2 record. +fn serves_flags_v4(is_from_legacy_decide: bool, version: Option, v4_enabled: bool) -> bool { + v4_enabled && !is_from_legacy_decide && version == Some(4) } /// Feature flag evaluation endpoint. @@ -313,7 +320,7 @@ where &headers, query_params.version.as_deref(), false, - state.config.flags_v3_response_enabled, + state.config.flags_v4_response_enabled, ) .into_response()); } @@ -453,7 +460,7 @@ where &headers, query_params.version.as_deref(), is_from_legacy_decide, - state.config.flags_v3_response_enabled, + state.config.flags_v4_response_enabled, ) .into_response()); } @@ -472,11 +479,8 @@ where } // Parse version from query params (needed for response formatting) - let query_version = modified_query_params - .version - .as_deref() - .map(|v| v.parse::().unwrap_or(1)); - let v3_enabled = state.config.flags_v3_response_enabled; + let query_version = modified_query_params.version.as_deref().map(parse_version); + let v4_enabled = state.config.flags_v4_response_enabled; // Install a body-logging side channel only when at least one team is // opted in. The OnceLock is filled inside the request decode flow with @@ -581,11 +585,11 @@ where log.team_id, decoded_body, &response, - serves_flags_v3(is_from_legacy_decide, query_version, v3_enabled), + serves_flags_v4(is_from_legacy_decide, query_version, v4_enabled), ); // Determine the response format based on whether request is from decide and version - match get_versioned_response(is_from_legacy_decide, query_version, v3_enabled, response) + match get_versioned_response(is_from_legacy_decide, query_version, v4_enabled, response) { Ok((versioned_response, response_format)) => { log.http_status = 200; @@ -721,6 +725,36 @@ mod tests { http::Uri, }; + #[rstest] + #[case::exact_v4("4", false, true, "FlagsV4")] + #[case::v4_setting_off("4", false, false, "FlagsV2")] + #[case::v3("3", false, true, "FlagsV2")] + #[case::v3_setting_off("3", false, false, "FlagsV2")] + #[case::v5("5", false, true, "FlagsV2")] + #[case::cache_buster("17380067", false, true, "FlagsV2")] + #[case::v2("2", false, true, "FlagsV2")] + #[case::fractional_v4("4.0", false, true, "FlagsV1")] + #[case::huge("12345678901234567890123456789012345678901234", false, true, "FlagsV1")] + #[case::junk("abc", false, true, "FlagsV1")] + #[case::decide_v4("4", true, true, "FlagsV2")] + #[case::decide_v3("3", true, true, "FlagsV1")] + fn test_only_an_exact_v4_selects_the_v4_record( + #[case] v: &str, + #[case] is_from_legacy_decide: bool, + #[case] v4_enabled: bool, + #[case] expected: &str, + ) { + let version = Some(parse_version(v)); + let response = FlagsResponse::new(false, HashMap::new(), None, Uuid::new_v4()); + let (_, format) = + get_versioned_response(is_from_legacy_decide, version, v4_enabled, response).unwrap(); + assert_eq!(format, expected); + assert_eq!( + serves_flags_v4(is_from_legacy_decide, version, v4_enabled), + expected == "FlagsV4" + ); + } + #[tokio::test] async fn test_query_param_extraction() { // Test case 1: Full query string diff --git a/rust/feature-flags/src/api/types.rs b/rust/feature-flags/src/api/types.rs index 9d0f9de0b184..b605479df781 100644 --- a/rust/feature-flags/src/api/types.rs +++ b/rust/feature-flags/src/api/types.rs @@ -134,7 +134,7 @@ pub struct FlagsQueryParams { pub enum ServiceResponse { Default(LegacyFlagsResponse), V2(FlagsResponse), - V3(FlagsResponseV3), + V4(FlagsResponseV4), DecideV1(DecideV1Response), DecideV2(DecideV2Response), } @@ -224,7 +224,7 @@ pub struct FlagsResponse { /// Set to `true` when the team is gated into slim `$feature_flag_called` events /// (TeamFeatureFlagsConfig.minimal_flag_called_events). Omitted otherwise, so SDKs /// that see no field at all fall back to full events, same as legacy teams. - /// Only reaches the wire on this v2 shape and on `FlagsResponseV3`: `LegacyFlagsResponse`, + /// Only reaches the wire on this v2 shape and on `FlagsResponseV4`: `LegacyFlagsResponse`, /// `DecideV1Response`, and `DecideV2Response` intentionally never carry it over. SDKs old /// enough to hit those response shapes predate this field and have no code path that reads /// it, so there's nothing gained by sending it to them. @@ -451,7 +451,7 @@ pub struct FlagDetails { pub config_outcome: ConfigOutcome, } -/// The config format and, for a v2 flag that evaluated, the typed outcome the v3 record needs. +/// The config format and, for a v2 flag that evaluated, the typed outcome the v4 record needs. #[derive(Debug, Clone, PartialEq, Eq, Default)] pub enum ConfigOutcome { #[default] @@ -471,9 +471,9 @@ impl ConfigOutcome { #[derive(Debug, PartialEq, Deserialize, Serialize)] #[serde(rename_all = "camelCase")] -pub struct FlagsResponseV3 { +pub struct FlagsResponseV4 { pub errors_while_computing_flags: bool, - pub flags: HashMap, + pub flags: HashMap, #[serde(skip_serializing_if = "Option::is_none")] pub quota_limited: Option>, pub request_id: Uuid, @@ -484,14 +484,14 @@ pub struct FlagsResponseV3 { pub config: ConfigResponse, } -impl FlagsResponseV3 { +impl FlagsResponseV4 { pub fn from_response(response: FlagsResponse) -> Self { Self { errors_while_computing_flags: response.errors_while_computing_flags, flags: response .flags .into_iter() - .map(|(key, flag)| (key, FlagDetailsV3::from(flag))) + .map(|(key, flag)| (key, FlagDetailsV4::from(flag))) .collect(), quota_limited: response.quota_limited, request_id: response.request_id, @@ -502,22 +502,22 @@ impl FlagsResponseV3 { } } -/// `/flags?v=3` record: one typed `value` instead of `enabled` and `variant`. +/// `/flags?v=4` record: one typed `value` instead of `enabled` and `variant`. #[derive(Debug, PartialEq, Deserialize, Serialize)] -pub struct FlagDetailsV3 { +pub struct FlagDetailsV4 { pub key: String, /// `null` is a null flag default or a failed record. pub value: Value, #[serde(default, skip_serializing_if = "std::ops::Not::not")] pub failed: bool, pub reason: FlagEvaluationReason, - pub metadata: FlagDetailsMetadataV3, + pub metadata: FlagDetailsMetadataV4, #[serde(skip_serializing_if = "Option::is_none")] pub conditions: Option>, } #[derive(Debug, PartialEq, Eq, Deserialize, Serialize)] -pub struct FlagDetailsMetadataV3 { +pub struct FlagDetailsMetadataV4 { pub id: i32, pub version: i32, pub config_version: u8, @@ -532,7 +532,7 @@ pub struct FlagDetailsMetadataV3 { pub variant_key: Option, } -impl From for FlagDetailsV3 { +impl From for FlagDetailsV4 { fn from(flag: FlagDetails) -> Self { let legacy_value = match flag.to_value() { FlagValue::Boolean(enabled) => Value::Bool(enabled), @@ -586,7 +586,7 @@ impl From for FlagDetailsV3 { value, failed: flag.failed, reason, - metadata: FlagDetailsMetadataV3 { + metadata: FlagDetailsMetadataV4 { id: flag.metadata.id, version: flag.metadata.version, config_version, @@ -2528,7 +2528,7 @@ mod tests { } #[test] - fn test_v3_record_of_a_failed_v1_flag_has_a_null_value() { + fn test_v4_record_of_a_failed_v1_flag_has_a_null_value() { let flag = FlagDetails { key: "v1-failed".to_string(), enabled: false, @@ -2550,7 +2550,7 @@ mod tests { config_outcome: ConfigOutcome::V1, }; - let record = FlagDetailsV3::from(flag); + let record = FlagDetailsV4::from(flag); assert_eq!(record.value, Value::Null); assert!(record.failed); diff --git a/rust/feature-flags/src/config.rs b/rust/feature-flags/src/config.rs index 33aeffb11c24..e813e4906064 100644 --- a/rust/feature-flags/src/config.rs +++ b/rust/feature-flags/src/config.rs @@ -719,9 +719,9 @@ pub struct Config { #[envconfig(from = "FLAGS_SESSION_REPLAY_QUOTA_CHECK", default = "false")] pub flags_session_replay_quota_check: bool, - /// Serve the v3 record on `/flags?v=3` and above. Off, those requests get the v2 record. - #[envconfig(from = "FLAGS_V3_RESPONSE_ENABLED", default = "false")] - pub flags_v3_response_enabled: bool, + /// Serve the v4 record on `/flags?v=4`. Off, those requests get the v2 record. + #[envconfig(from = "FLAGS_V4_RESPONSE_ENABLED", default = "false")] + pub flags_v4_response_enabled: bool, // Flag definitions rate limiting // Default rate limit for all teams (requests per minute) @@ -1304,7 +1304,7 @@ impl Config { element_chain_as_string_excluded_teams: TeamIdCollection::None, debug: FlexBool(false), flags_session_replay_quota_check: false, - flags_v3_response_enabled: false, + flags_v4_response_enabled: false, flag_definitions_default_rate_per_minute: 600, flag_definitions_rate_limits: FlagDefinitionsRateLimits::default(), flag_definitions_conditional_rate_per_minute: 6000, @@ -1544,7 +1544,7 @@ mod tests { assert_eq!(config.new_analytics_capture_endpoint, "/i/v0/e/"); assert_eq!(config.debug, FlexBool(false)); assert!(!config.flags_session_replay_quota_check); - assert!(!config.flags_v3_response_enabled); + assert!(!config.flags_v4_response_enabled); assert_eq!(config.skip_writes, FlexBool(false)); // Bot filter ships in LogOnly mode by default — pin the safe // posture so a future env-var rename / refactor can't silently diff --git a/rust/feature-flags/src/flags/evaluate_v2.rs b/rust/feature-flags/src/flags/evaluate_v2.rs index faf539706869..d253384757a4 100644 --- a/rust/feature-flags/src/flags/evaluate_v2.rs +++ b/rust/feature-flags/src/flags/evaluate_v2.rs @@ -145,7 +145,7 @@ impl<'a> Evaluation<'a> { } } -/// The parts of an [`Evaluation`] the v3 record reads, owned so they outlive the configuration. +/// The parts of an [`Evaluation`] the v4 record reads, owned so they outlive the configuration. #[derive(Clone, Debug, PartialEq, Eq)] pub struct EvaluationDetail { pub code: &'static str, diff --git a/rust/feature-flags/src/flags/flag_matching.rs b/rust/feature-flags/src/flags/flag_matching.rs index a260a8803f46..aaf9a6a14567 100644 --- a/rust/feature-flags/src/flags/flag_matching.rs +++ b/rust/feature-flags/src/flags/flag_matching.rs @@ -111,7 +111,7 @@ pub struct FeatureFlagMatch { pub reason: FeatureFlagMatchReason, pub condition_index: Option, pub payload: Option, - /// Set only by `get_match_v2`; the v3 record is built from it. + /// Set only by `get_match_v2`; the v4 record is built from it. pub evaluation_v2: Option, } @@ -2103,7 +2103,7 @@ impl FeatureFlagMatcher { use_explicit_exact_matching: self.use_explicit_exact_matching, now: self.now, })?; - // `/flags?v=2` reports these v1 reasons; `/flags?v=3` reports the evaluator's own code. + // `/flags?v=2` reports these v1 reasons; `/flags?v=4` reports the evaluator's own code. let reason = match evaluation { Evaluation::TargetingMatch { .. } | Evaluation::ExperimentSplit { .. } => { FeatureFlagMatchReason::ConditionMatch diff --git a/rust/feature-flags/src/handler/body_logger.rs b/rust/feature-flags/src/handler/body_logger.rs index 18ef59f059d4..09d0e456fd15 100644 --- a/rust/feature-flags/src/handler/body_logger.rs +++ b/rust/feature-flags/src/handler/body_logger.rs @@ -12,7 +12,7 @@ //! `*-targeting-*`); exact keys (no `*`) match by string equality. use crate::api::instance_setting::{constance_key, fetch_instance_setting_raw_value}; -use crate::api::types::{FlagDetails, FlagDetailsV3, FlagsResponse}; +use crate::api::types::{FlagDetails, FlagDetailsV4, FlagsResponse}; use crate::config::BodyLogTeams; use crate::metrics::consts::FLAG_BODY_LOG_REFRESH_TOTAL; use arc_swap::ArcSwap; @@ -218,8 +218,8 @@ impl BodyLogger { /// decompress, and base64-wrapped bodies are logged as the JSON they /// actually parsed as — not as the raw base64 string. /// - /// Set `serves_v3` when the client receives the `/flags?v=3` record. Each - /// logged flag is then that v3 record, because the internal record cannot + /// Set `serves_v4` when the client receives the `/flags?v=4` record. Each + /// logged flag is then that v4 record, because the internal record cannot /// show its typed `value` or its rule fields. pub fn log_response( &self, @@ -227,7 +227,7 @@ impl BodyLogger { team_id: Option, decoded_body: Option, response: &FlagsResponse, - serves_v3: bool, + serves_v4: bool, ) { let (Some(team_id), Some(decoded)) = (team_id, decoded_body) else { return; @@ -240,7 +240,7 @@ impl BodyLogger { truncate_body(&decoded, self.request_max_bytes); let request_body = String::from_utf8_lossy(truncated); let (response_flags_body, total, logged) = - serialize_filtered_response(response, &patterns, serves_v3); + serialize_filtered_response(response, &patterns, serves_v4); // Override the default target (module path) with a stable, semantic // identifier. Keeps the `feature_flags::` prefix so `RUST_LOG` filters @@ -327,12 +327,12 @@ impl<'a, F> LoggedResponse<'a, F> { } /// Serialize the response with `flags` filtered to keys matching `patterns`. -/// With `serves_v3`, each flag is serialized as its v3 record. +/// With `serves_v4`, each flag is serialized as its v4 record. /// Returns the JSON string plus `(total_flags, logged_flags)` counts. fn serialize_filtered_response( response: &FlagsResponse, patterns: &TeamPatterns, - serves_v3: bool, + serves_v4: bool, ) -> (String, usize, usize) { let total = response.flags.len(); @@ -340,9 +340,9 @@ fn serialize_filtered_response( .flags .iter() .filter(|(key, _)| patterns.matches(key)); - let (serialized, logged) = if serves_v3 { - let flags: HashMap<&String, FlagDetailsV3> = filtered - .map(|(key, flag)| (key, FlagDetailsV3::from(flag.clone()))) + let (serialized, logged) = if serves_v4 { + let flags: HashMap<&String, FlagDetailsV4> = filtered + .map(|(key, flag)| (key, FlagDetailsV4::from(flag.clone()))) .collect(); let logged = flags.len(); ( @@ -817,7 +817,7 @@ mod tests { } #[test] - fn serialize_filtered_response_logs_v3_record_when_served() { + fn serialize_filtered_response_logs_v4_record_when_served() { let mut resp = make_response(&["null-default", "other"]); let flag = resp.flags.get_mut("null-default").unwrap(); flag.enabled = false; @@ -829,15 +829,15 @@ mod tests { let v2_flag = &serde_json::from_str::(&v2_body).unwrap()["flags"]["null-default"]; assert_eq!(v2_flag["enabled"], Value::Bool(false)); - let (v3_body, total, logged) = serialize_filtered_response(&resp, &patterns, true); + let (v4_body, total, logged) = serialize_filtered_response(&resp, &patterns, true); assert_eq!((total, logged), (2, 1)); - let v3_flags = &serde_json::from_str::(&v3_body).unwrap()["flags"]; - assert!(v3_flags.get("other").is_none()); - let v3_flag = &v3_flags["null-default"]; - assert_eq!(v3_flag.get("value"), Some(&Value::Null)); - assert_eq!(v3_flag["reason"]["code"], "no_rule_match"); - assert_eq!(v3_flag["metadata"]["config_version"], 2); - assert!(v3_flag.get("enabled").is_none()); + let v4_flags = &serde_json::from_str::(&v4_body).unwrap()["flags"]; + assert!(v4_flags.get("other").is_none()); + let v4_flag = &v4_flags["null-default"]; + assert_eq!(v4_flag.get("value"), Some(&Value::Null)); + assert_eq!(v4_flag["reason"]["code"], "no_rule_match"); + assert_eq!(v4_flag["metadata"]["config_version"], 2); + assert!(v4_flag.get("enabled").is_none()); } #[test] diff --git a/rust/feature-flags/src/handler/canonical_log.rs b/rust/feature-flags/src/handler/canonical_log.rs index 3499a91c8dd2..596057edbf99 100644 --- a/rust/feature-flags/src/handler/canonical_log.rs +++ b/rust/feature-flags/src/handler/canonical_log.rs @@ -187,7 +187,7 @@ pub struct FlagsCanonicalLogLine { pub lib: Option, pub lib_version: Option, pub api_version: Option, - /// The response shape served, such as `FlagsV2` or `FlagsV3`. + /// The response shape served, such as `FlagsV2` or `FlagsV4`. pub response_format: Option<&'static str>, // Populated during authentication @@ -733,7 +733,7 @@ mod tests { log.lib = Some("posthog-python".to_string()); log.lib_version = Some("1.0.0".to_string()); log.api_version = Some("3".to_string()); - log.response_format = Some("FlagsV3"); + log.response_format = Some("FlagsV4"); log.team_id = Some(123); log.distinct_id = Some("user_abc".to_string()); log.device_id = Some("device_123".to_string()); diff --git a/rust/feature-flags/tests/test_flags.rs b/rust/feature-flags/tests/test_flags.rs index 4d8ddb19f41d..87e2534de5ba 100644 --- a/rust/feature-flags/tests/test_flags.rs +++ b/rust/feature-flags/tests/test_flags.rs @@ -1742,9 +1742,9 @@ async fn it_sets_quota_limited_in_legacy_and_v2() -> Result<()> { } #[tokio::test] -async fn test_minimal_flag_called_events_reaches_v2_and_v3_but_not_legacy_response() -> Result<()> { +async fn test_minimal_flag_called_events_reaches_v2_and_v4_but_not_legacy_response() -> Result<()> { let mut config = DEFAULT_TEST_CONFIG.clone(); - config.flags_v3_response_enabled = true; + config.flags_v4_response_enabled = true; let distinct_id = "user1".to_string(); let client = setup_redis_client(Some(config.redis_url.clone())).await; @@ -1771,13 +1771,13 @@ async fn test_minimal_flag_called_events_reaches_v2_and_v3_but_not_legacy_respon let v2: FlagsResponse = res.json().await?; assert_eq!(v2.minimal_flag_called_events, Some(true)); - // V3 response: carried over from the v2 response. + // V4 response: carried over from the v2 response. let res = server - .send_flags_request(payload.to_string(), Some("3"), None) + .send_flags_request(payload.to_string(), Some("4"), None) .await; assert_eq!(StatusCode::OK, res.status()); - let v3 = res.json::().await?; - assert_eq!(v3["minimalFlagCalledEvents"], true); + let v4 = res.json::().await?; + assert_eq!(v4["minimalFlagCalledEvents"], true); // Legacy response (v=1): LegacyFlagsResponse doesn't carry the field at all, so a // gated team's v1 clients never see the signal and keep sending full events. diff --git a/rust/feature-flags/tests/test_flags_v3.rs b/rust/feature-flags/tests/test_flags_v4.rs similarity index 83% rename from rust/feature-flags/tests/test_flags_v3.rs rename to rust/feature-flags/tests/test_flags_v4.rs index 30345116cf7e..fa989da2ee67 100644 --- a/rust/feature-flags/tests/test_flags_v3.rs +++ b/rust/feature-flags/tests/test_flags_v4.rs @@ -54,14 +54,14 @@ fn corpus_flag(case: &Value, id: i32) -> Value { } async fn serve( - v3_enabled: bool, + v4_enabled: bool, distinct_id: &str, properties: Value, mut flags: Vec, ) -> Result<(ServerHandle, String)> { let mut config = DEFAULT_TEST_CONFIG.clone(); config.internal_request_token = Some(INTERNAL_TOKEN.to_string()); - config.flags_v3_response_enabled = v3_enabled; + config.flags_v4_response_enabled = v4_enabled; let redis = setup_redis_client(Some(config.redis_url.clone())).await; let team = insert_new_team_in_redis(redis.clone()).await?; let db = TestContext::new(None).await; @@ -76,10 +76,10 @@ async fn serve( } /// A team with one person, three v1 flags and six v2 flags covering every boolean outcome. -async fn server(v3_enabled: bool) -> Result<(ServerHandle, String)> { +async fn server(v4_enabled: bool) -> Result<(ServerHandle, String)> { let flags = boolean_flags(); serve( - v3_enabled, + v4_enabled, "ann", json!({"email": "ann@example.com"}), flags, @@ -186,14 +186,14 @@ fn keys(value: &Value) -> Vec<&str> { } #[tokio::test] -async fn v3_returns_the_typed_record_for_v1_and_v2_flags() -> Result<()> { +async fn v4_returns_the_typed_record_for_v1_and_v2_flags() -> Result<()> { let (server, token) = server(true).await?; let body = json!({"token": token, "distinct_id": "ann"}); - let v3 = post(&server, "v=3&config=false", &[], body.clone()).await?; - wire::validate_v3(&v3).unwrap_or_else(|error| panic!("{error:?}")); - assert_eq!(v3["errorsWhileComputingFlags"], true); - let flags = &v3["flags"]; + let v4 = post(&server, "v=4&config=false", &[], body.clone()).await?; + wire::validate_v4(&v4).unwrap_or_else(|error| panic!("{error:?}")); + assert_eq!(v4["errorsWhileComputingFlags"], true); + let flags = &v4["flags"]; assert_eq!(flags.as_object().unwrap().len(), 9); let expected = [ @@ -327,27 +327,24 @@ async fn v3_returns_the_typed_record_for_v1_and_v2_flags() -> Result<()> { assert_eq!(flags["v1-on"]["metadata"]["version"], 5); assert_eq!(flags["v2-true"]["metadata"]["version"], 3); - let v99 = post(&server, "v=99&config=false", &[], body.clone()).await?; - assert_eq!(v99["flags"], v3["flags"]); - let subset = post( &server, - "v=3&config=false", + "v=4&config=false", &[], json!({"token": token, "distinct_id": "ann", "flag_keys": ["v2-true", "missing"]}), ) .await?; assert_eq!(keys(&subset["flags"]), ["v2-true"]); - wire::validate_v3(&subset).unwrap(); + wire::validate_v4(&subset).unwrap(); let detailed = post( &server, - "v=3&config=false&detailed_analysis=true", + "v=4&config=false&detailed_analysis=true", INTERNAL, body, ) .await?; - wire::validate_v3(&detailed).unwrap(); + wire::validate_v4(&detailed).unwrap(); assert_eq!( detailed["flags"]["v1-on"]["conditions"] .as_array() @@ -396,18 +393,18 @@ async fn v2_legacy_and_decide_shapes_are_unchanged() -> Result<()> { assert_eq!(decide_v4["flags"]["v2-true"]["enabled"], true); assert_eq!(decide_v4["flags"]["v2-true"].get("value"), None); - let minimal: Value = reqwest::get(format!("http://{}/flags?v=3", server.addr)) + let minimal: Value = reqwest::get(format!("http://{}/flags?v=4", server.addr)) .await? .json() .await?; assert_eq!(minimal["flags"], json!({})); assert_eq!(minimal["supportedCompression"], json!(["gzip", "gzip-js"])); - wire::validate_v3(&minimal).unwrap(); + wire::validate_v4(&minimal).unwrap(); Ok(()) } #[tokio::test] -async fn v3_and_above_serve_the_v2_record_while_the_setting_is_off() -> Result<()> { +async fn v4_serves_the_v2_record_while_the_setting_is_off() -> Result<()> { let string_case = corpus::value_cases() .into_iter() .find(|case| case["id"] == "v2_value.string.target") @@ -427,7 +424,7 @@ async fn v3_and_above_serve_the_v2_record_while_the_setting_is_off() -> Result<( assert_eq!(v2["flags"]["v2_value.string.target"]["variant"], "compact"); assert_eq!(v2["flags"]["v2-true"]["enabled"], true); assert_eq!(v2["flags"]["v1-variant"]["variant"], "compact"); - for version in ["v=3", "v=99"] { + for version in ["v=3", "v=4", "v=99"] { let response = post( &server, &format!("{version}&{config}"), @@ -450,12 +447,79 @@ async fn v3_and_above_serve_the_v2_record_while_the_setting_is_off() -> Result<( .send() }; let minimal_v2: Value = get("v=2").await?.json().await?; - let minimal_v3: Value = get("v=3").await?.json().await?; - assert_eq!(without_time(minimal_v3), without_time(minimal_v2)); + let minimal_v4: Value = get("v=4").await?.json().await?; + assert_eq!(without_time(minimal_v4), without_time(minimal_v2)); + Ok(()) +} + +#[tokio::test] +async fn only_an_exact_v4_selects_the_v4_record() -> Result<()> { + let (server, token) = server(true).await?; + let body = json!({"token": token, "distinct_id": "ann"}); + let request_id = [("X-REQUEST-ID", "00000000-0000-4000-8000-000000000004")]; + let without_time = |mut response: Value| { + response.as_object_mut().unwrap().remove("evaluatedAt"); + response + }; + + let v4 = post(&server, "v=4&config=false", &request_id, body.clone()).await?; + assert_eq!(v4["flags"]["v2-true"]["value"], true); + assert_eq!(v4["flags"]["v2-true"].get("enabled"), None); + + let v2 = without_time(post(&server, "v=2&config=false", &request_id, body.clone()).await?); + for version in ["v=3", "v=5", "v=99", "v=9992", "v=17380067"] { + let response = post( + &server, + &format!("{version}&config=false"), + &request_id, + body.clone(), + ) + .await?; + assert_eq!(without_time(response), v2, "{version}"); + } + + let v1 = without_time(post(&server, "v=1&config=false", &request_id, body.clone()).await?); + for version in [ + "v=4.0", + "v=4.5", + "v=12345678901234567890123456789012345678901234", + "v=abc", + ] { + let response = post( + &server, + &format!("{version}&config=false"), + &request_id, + body.clone(), + ) + .await?; + assert_eq!(without_time(response), v1, "{version}"); + } + + let decide_v4 = post(&server, "v=4&config=false", DECIDE, body).await?; + assert_eq!(decide_v4["flags"]["v2-true"]["enabled"], true); + assert_eq!(decide_v4["flags"]["v2-true"].get("value"), None); + + let get = |version: &str| { + reqwest::Client::new() + .get(format!("http://{}/flags?{version}", server.addr)) + .header(request_id[0].0, request_id[0].1) + .send() + }; + let minimal_v2: Value = get("v=2").await?.json().await?; + for version in ["v=3", "v=5"] { + let minimal: Value = get(version).await?.json().await?; + assert_eq!( + without_time(minimal), + without_time(minimal_v2.clone()), + "{version}" + ); + } + let minimal_v4: Value = get("v=4").await?.json().await?; + wire::validate_v4(&minimal_v4).unwrap(); Ok(()) } -/// Serves the success cases and checks their v3 and v2 records. Each identifier gets its own +/// Serves the success cases and checks their v4 and v2 records. Each identifier gets its own /// server, because the experiment corpus picks identifiers per outcome. Returns the cases checked. async fn check_on_the_wire(cases: Vec) -> Result { let cases: Vec<_> = cases @@ -492,13 +556,13 @@ async fn check_on_the_wire(cases: Vec) -> Result { ) .await?; let body = json!({"token": token, "distinct_id": distinct_id}); - let v3 = post(&server, "v=3&config=false", &[], body.clone()).await?; - wire::validate_v3(&v3).unwrap_or_else(|error| panic!("{error:?}")); + let v4 = post(&server, "v=4&config=false", &[], body.clone()).await?; + wire::validate_v4(&v4).unwrap_or_else(|error| panic!("{error:?}")); let v2 = post(&server, "v=2&config=false", &[], body).await?; for case in group { let key = case["id"].as_str().unwrap(); let (expected, legacy) = (&case["expected"], &case["legacy"]); - let record = &v3["flags"][key]; + let record = &v4["flags"][key]; assert_eq!(record["value"], expected["value"], "{key}"); assert_eq!(record["reason"]["code"], expected["reason"], "{key}"); assert_eq!( @@ -540,7 +604,7 @@ async fn check_on_the_wire(cases: Vec) -> Result { } #[tokio::test] -async fn v3_carries_every_typed_corpus_value_through_the_request_path() -> Result<()> { +async fn v4_carries_every_typed_corpus_value_through_the_request_path() -> Result<()> { assert_eq!(check_on_the_wire(corpus::value_cases()).await?, 38); Ok(()) } diff --git a/rust/feature-flags/tests/test_rules_v2_evaluation.rs b/rust/feature-flags/tests/test_rules_v2_evaluation.rs index 9b7539493a62..efff52fa29cd 100644 --- a/rust/feature-flags/tests/test_rules_v2_evaluation.rs +++ b/rust/feature-flags/tests/test_rules_v2_evaluation.rs @@ -17,7 +17,7 @@ mod corpus; #[path = "test_rules_v2_evaluation/wire.rs"] mod wire; -/// `wire::validate_v3` accepts every wire fixture case marked `valid` and fails every other +/// `wire::validate_v4` accepts every wire fixture case marked `valid` and fails every other /// case at the layer its `expected_failure.layer` declares. #[test] fn vendored_response_fixtures_agree_with_the_validator() { @@ -25,7 +25,7 @@ fn vendored_response_fixtures_agree_with_the_validator() { let (mut valid, mut invalid) = (0, 0); for case in fixtures["cases"].as_array().unwrap() { let id = case["id"].as_str().unwrap(); - let verdict = wire::validate_v3(&wire::fixture_case(&fixtures, case)); + let verdict = wire::validate_v4(&wire::fixture_case(&fixtures, case)); if case["expected"] == "valid" { assert_eq!(verdict, Ok(()), "{id}"); valid += 1; @@ -460,7 +460,7 @@ fn evaluation_is_repeatable_and_diagnostics_do_not_retain_inputs() { #[tokio::test] async fn corpus_cases_project_through_the_matcher_and_the_legacy_formats() { use feature_flags::api::types::{ - DecideV1Response, DecideV2Response, FlagValue, FlagsResponse, FlagsResponseV3, + DecideV1Response, DecideV2Response, FlagValue, FlagsResponse, FlagsResponseV4, LegacyFlagsResponse, }; use feature_flags::cohorts::cohort_cache_manager::CohortCacheManager; @@ -635,9 +635,9 @@ async fn corpus_cases_project_through_the_matcher_and_the_legacy_formats() { enabled.then_some(map_value).as_ref(), "{id}" ); - let v3 = serde_json::to_value(FlagsResponseV3::from_response(response)).unwrap(); - wire::validate_v3(&v3).unwrap_or_else(|error| panic!("{id}: {error:?}")); - let record = &v3["flags"][&key]; + let v4 = serde_json::to_value(FlagsResponseV4::from_response(response)).unwrap(); + wire::validate_v4(&v4).unwrap_or_else(|error| panic!("{id}: {error:?}")); + let record = &v4["flags"][&key]; assert_eq!(record["metadata"]["config_version"], 2, "{id}"); assert_eq!(record.get("enabled"), None, "{id}"); if failed { diff --git a/rust/feature-flags/tests/test_rules_v2_evaluation/wire.rs b/rust/feature-flags/tests/test_rules_v2_evaluation/wire.rs index d3a8efab8207..41b1d487d3c1 100644 --- a/rust/feature-flags/tests/test_rules_v2_evaluation/wire.rs +++ b/rust/feature-flags/tests/test_rules_v2_evaluation/wire.rs @@ -1,4 +1,4 @@ -//! Validates a `/flags?v=3` response the way the harness does. +//! Validates a `/flags?v=4` response the way the harness does. use std::sync::LazyLock; @@ -7,6 +7,7 @@ use serde_json::Value; use crate::corpus; +// The harness contract still names the v4 record's schemas `flags_response_v3*`. const PRODUCER_ID: &str = "https://posthog.com/contracts/feature_flag_rules_v2/flags_response_v3.schema.json"; @@ -81,7 +82,7 @@ fn first_seed(value: &Value, path: &str) -> Option { } /// `Err((layer, detail))` names the first failing layer. -pub fn validate_v3(response: &Value) -> Result<(), (&'static str, String)> { +pub fn validate_v4(response: &Value) -> Result<(), (&'static str, String)> { if let Some(error) = first_error(&PRODUCER, response) { return Err(("schema", error)); }