From a6d73090d7095c9ad733314aa764c84bf128b23e Mon Sep 17 00:00:00 2001 From: Neil Kakkar Date: Wed, 8 May 2024 20:06:35 +0100 Subject: [PATCH 01/21] feat(flags): Do token validation and extract distinct id --- Cargo.lock | 39 ++++++++++++++++++++--- feature-flags/Cargo.toml | 1 + feature-flags/src/api.rs | 4 +++ feature-flags/src/config.rs | 2 +- feature-flags/src/lib.rs | 1 + feature-flags/src/redis.rs | 54 +++++++++++++++----------------- feature-flags/src/v0_endpoint.rs | 18 +++++------ feature-flags/src/v0_request.rs | 13 +++++--- 8 files changed, 83 insertions(+), 49 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0f475fa..8642ade 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -707,6 +707,7 @@ dependencies = [ "redis", "reqwest 0.12.3", "serde", + "serde-pickle", "serde_json", "thiserror", "tokio", @@ -1395,6 +1396,12 @@ version = "2.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8f518f335dce6725a761382244631d86cf0ccb2863413590b31338feb467f9c3" +[[package]] +name = "iter-read" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c397ca3ea05ad509c4ec451fea28b4771236a376ca1c69fd5143aae0cf8f93c4" + [[package]] name = "itertools" version = "0.12.1" @@ -1680,6 +1687,16 @@ dependencies = [ "winapi", ] +[[package]] +name = "num-bigint" +version = "0.4.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c165a9ab64cf766f73521c0dd2cfdff64f488b8f0b3e621face3462d3db536d7" +dependencies = [ + "num-integer", + "num-traits", +] + [[package]] name = "num-bigint-dig" version = "0.8.4" @@ -1705,11 +1722,10 @@ checksum = "51d515d32fb182ee37cda2ccdcb92950d6a3c2893aa280e540671c2cd0f3b1d9" [[package]] name = "num-integer" -version = "0.1.45" +version = "0.1.46" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "225d3389fb3509a24c93f5c29eb6bde2586b98d9f016636dff58d7c6f7569cd9" +checksum = "7969661fd2958a5cb096e56c8e1ad0444ac2bbcd0061bd28660485a44879858f" dependencies = [ - "autocfg", "num-traits", ] @@ -1726,9 +1742,9 @@ dependencies = [ [[package]] name = "num-traits" -version = "0.2.17" +version = "0.2.19" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "39e3200413f237f41ab11ad6d161bc7239c84dcb631773ccd7de3dfe4b5c267c" +checksum = "071dfc062690e90b734c0b2273ce72ad0ffa95f0c74596bc250dcfd960262841" dependencies = [ "autocfg", "libm", @@ -2533,6 +2549,19 @@ dependencies = [ "serde_derive", ] +[[package]] +name = "serde-pickle" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c762ad136a26407c6a80825813600ceeab5e613660d93d79a41f0ec877171e71" +dependencies = [ + "byteorder", + "iter-read", + "num-bigint", + "num-traits", + "serde", +] + [[package]] name = "serde_derive" version = "1.0.196" diff --git a/feature-flags/Cargo.toml b/feature-flags/Cargo.toml index ddfe070..1e0c111 100644 --- a/feature-flags/Cargo.toml +++ b/feature-flags/Cargo.toml @@ -24,6 +24,7 @@ redis = { version = "0.23.3", features = [ serde = { workspace = true } serde_json = { workspace = true } thiserror = { workspace = true } +serde-pickle = { version = "1.1.1"} [lints] workspace = true diff --git a/feature-flags/src/api.rs b/feature-flags/src/api.rs index c94eed6..ebad1f5 100644 --- a/feature-flags/src/api.rs +++ b/feature-flags/src/api.rs @@ -25,6 +25,9 @@ pub enum FlagError { #[error("failed to parse request: {0}")] RequestParsingError(#[from] serde_json::Error), + #[error("failed to parse redis data: {0}")] + DataParsingError(#[from] serde_pickle::Error), + #[error("Empty distinct_id in request")] EmptyDistinctId, #[error("No distinct_id in request")] @@ -44,6 +47,7 @@ impl IntoResponse for FlagError { match self { FlagError::RequestDecodingError(_) | FlagError::RequestParsingError(_) + | FlagError::DataParsingError(_) | FlagError::EmptyDistinctId | FlagError::MissingDistinctId => (StatusCode::BAD_REQUEST, self.to_string()), diff --git a/feature-flags/src/config.rs b/feature-flags/src/config.rs index 3fa6f50..cc7ad37 100644 --- a/feature-flags/src/config.rs +++ b/feature-flags/src/config.rs @@ -4,7 +4,7 @@ use envconfig::Envconfig; #[derive(Envconfig, Clone)] pub struct Config { - #[envconfig(default = "127.0.0.1:0")] + #[envconfig(default = "127.0.0.1:3001")] pub address: SocketAddr, #[envconfig(default = "postgres://posthog:posthog@localhost:15432/test_database")] diff --git a/feature-flags/src/lib.rs b/feature-flags/src/lib.rs index 9175b5c..71a5e69 100644 --- a/feature-flags/src/lib.rs +++ b/feature-flags/src/lib.rs @@ -5,3 +5,4 @@ pub mod router; pub mod server; pub mod v0_endpoint; pub mod v0_request; +pub mod team; diff --git a/feature-flags/src/redis.rs b/feature-flags/src/redis.rs index 8c03820..70b7146 100644 --- a/feature-flags/src/redis.rs +++ b/feature-flags/src/redis.rs @@ -2,7 +2,7 @@ use std::time::Duration; use anyhow::Result; use async_trait::async_trait; -use redis::AsyncCommands; +use redis::{AsyncCommands, RedisError}; use tokio::time::timeout; // average for all commands is <10ms, check grafana @@ -10,12 +10,15 @@ const REDIS_TIMEOUT_MILLISECS: u64 = 10; /// A simple redis wrapper /// Copied from capture/src/redis.rs. -/// TODO: Modify this to support hincrby, get, and set commands. +/// TODO: Modify this to support hincrby #[async_trait] pub trait Client { // A very simplified wrapper, but works for our usage async fn zrangebyscore(&self, k: String, min: String, max: String) -> Result>; + + async fn get(&self, k: String) -> Result; + async fn set(&self, k: String, v: String) -> Result<()>; } pub struct RedisClient { @@ -40,38 +43,31 @@ impl Client for RedisClient { Ok(fut?) } -} -// TODO: Find if there's a better way around this. -#[derive(Clone)] -pub struct MockRedisClient { - zrangebyscore_ret: Vec, -} + async fn get(&self, k: String) -> Result { + let mut conn = self.client.get_async_connection().await?; -impl MockRedisClient { - pub fn new() -> MockRedisClient { - MockRedisClient { - zrangebyscore_ret: Vec::new(), - } - } + let results = conn.get(k.clone()); + // TODO: Is this safe? Should we be doing something else for error handling here? + let fut: Result, RedisError> = timeout(Duration::from_secs(REDIS_TIMEOUT_MILLISECS), results).await?; - pub fn zrangebyscore_ret(&mut self, ret: Vec) -> Self { - self.zrangebyscore_ret = ret; + // TRICKY: We serialise data to json, then django pickles it. + // Here we deserialize the bytes using serde_pickle, to get the json string. + let string_response: String = serde_pickle::from_slice(&fut?, Default::default())?; - self.clone() + Ok(string_response) } -} -impl Default for MockRedisClient { - fn default() -> Self { - Self::new() - } -} + async fn set(&self, k: String, v: String) -> Result<()> { + // TRICKY: We serialise data to json, then django pickles it. + // Here we serialize the json string to bytes using serde_pickle. + let bytes = serde_pickle::to_vec(&v, Default::default())?; -#[async_trait] -impl Client for MockRedisClient { - // A very simplified wrapper, but works for our usage - async fn zrangebyscore(&self, _k: String, _min: String, _max: String) -> Result> { - Ok(self.zrangebyscore_ret.clone()) + let mut conn = self.client.get_async_connection().await?; + + let results = conn.set(k, bytes); + let fut = timeout(Duration::from_secs(REDIS_TIMEOUT_MILLISECS), results).await?; + + Ok(fut?) } -} +} \ No newline at end of file diff --git a/feature-flags/src/v0_endpoint.rs b/feature-flags/src/v0_endpoint.rs index 8f77611..4a46d45 100644 --- a/feature-flags/src/v0_endpoint.rs +++ b/feature-flags/src/v0_endpoint.rs @@ -33,7 +33,7 @@ use crate::{ )] #[debug_handler] pub async fn flags( - _state: State, + state: State, InsecureClientIp(ip): InsecureClientIp, meta: Query, headers: HeaderMap, @@ -59,19 +59,19 @@ pub async fn flags( .get("content-type") .map_or("", |v| v.to_str().unwrap_or("")) { - "application/x-www-form-urlencoded" => { - return Err(FlagError::RequestDecodingError(String::from( - "invalid form data", - ))); + "application/json" => { + tracing::Span::current().record("content_type", "application/json"); + FlagRequest::from_bytes(body) } ct => { - tracing::Span::current().record("content_type", ct); - - FlagRequest::from_bytes(body) + return Err(FlagError::RequestDecodingError(format!( + "unsupported content type: {}", + ct + ))); } }?; - let token = request.extract_and_verify_token()?; + let token = request.extract_and_verify_token(state.redis.clone()).await?; tracing::Span::current().record("token", &token); diff --git a/feature-flags/src/v0_request.rs b/feature-flags/src/v0_request.rs index f2269df..2954b2e 100644 --- a/feature-flags/src/v0_request.rs +++ b/feature-flags/src/v0_request.rs @@ -1,11 +1,11 @@ -use std::collections::HashMap; +use std::{collections::HashMap, sync::Arc}; use bytes::Bytes; use serde::{Deserialize, Serialize}; use serde_json::Value; use tracing::instrument; -use crate::api::FlagError; +use crate::{api::FlagError, redis::Client, team::Team}; #[derive(Deserialize, Default)] pub struct FlagsQueryParams { @@ -54,15 +54,18 @@ impl FlagRequest { Ok(serde_json::from_str::(&payload)?) } - pub fn extract_and_verify_token(&self) -> Result { + pub async fn extract_and_verify_token(&self, redis_client: Arc) -> Result { let token = match self { FlagRequest { token: Some(token), .. } => token.to_string(), _ => return Err(FlagError::NoTokenError), }; - // TODO: Get tokens from redis, confirm this one is valid - // validate_token(&token)?; + + let team = Team::from_redis(redis_client, token.clone()).await?; + + // TODO: Remove this, is useless, doing just for now because + tracing::Span::current().record("team_id", &team.id); Ok(token) } } From d0e9bc04bef92e2570ab6c27d69a2ddeee282672 Mon Sep 17 00:00:00 2001 From: Neil Kakkar Date: Wed, 8 May 2024 20:07:57 +0100 Subject: [PATCH 02/21] add mod --- feature-flags/src/team.rs | 133 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 133 insertions(+) create mode 100644 feature-flags/src/team.rs diff --git a/feature-flags/src/team.rs b/feature-flags/src/team.rs new file mode 100644 index 0000000..cfa54c3 --- /dev/null +++ b/feature-flags/src/team.rs @@ -0,0 +1,133 @@ +use std::sync::Arc; + +use crate::{api::FlagError, redis::Client}; + +use serde::{Deserialize, Serialize}; +use tracing::instrument; + + +// TRICKY: I'm still not sure where the :1: is coming from. +// The Django prefix is `posthog` only. +// It's from here: https://docs.djangoproject.com/en/4.2/topics/cache/#cache-versioning +// F&!£%% on the bright side we don't use this functionality yet. +// Will rely on integration tests to catch this. +const TEAM_TOKEN_CACHE_PREFIX: &str = "posthog:1:team_token:"; + +// TODO: Check what happens if json has extra stuff, does serde ignore it? Yes +// Make sure we don't serialize and store team data in redis. Let main decide endpoint control this... +// and track misses. Revisit if this becomes an issue. +// because otherwise very annoying to keep this in sync with main django which has a lot of extra fields we need here. +// will lead to inconsistent behaviour. +// This is turning out to be very annoying, because we have django key prefixes to be mindful of as well. +// Wonder if it would be better to make these caches independent? This generates that new problem of CRUD happening in Django, +// which needs to update this cache immediately, so they can't really ever be independent. +// True for both team cache and flags cache. Hmm. Just I guess need to add tests around the key prefixes... +#[derive(Debug, Deserialize, Serialize)] +pub struct Team { + pub id: i64, + pub name: String, + pub api_token: String, +} + +impl Team { + /// Validates a token, and returns a team if it exists. + /// + + #[instrument(skip_all)] + pub async fn from_redis( + client: Arc, + token: String, + ) -> Result { + + // TODO: Instead of failing here, i.e. if not in redis, fallback to pg + let serialized_team = client + .get( + format!("{TEAM_TOKEN_CACHE_PREFIX}{}", token) + ) + .await + .map_err(|e| { + tracing::error!("failed to fetch data: {}", e); + // TODO: Can be other errors if serde_pickle destructuring fails? + FlagError::TokenValidationError + })?; + + let team: Team = serde_json::from_str(&serialized_team).map_err(|e| { + tracing::error!("failed to parse data to team: {}", e); + // TODO: Internal error, shouldn't send back to client + FlagError::RequestParsingError(e) + })?; + + Ok(team) + } +} + +#[cfg(test)] +mod tests { + use std::sync::Arc; + use anyhow::Error; + + use crate::redis::RedisClient; + use rand::{distributions::Alphanumeric, Rng}; + + use super::*; + + fn random_string(prefix: &str, length: usize) -> String { + let suffix: String = rand::thread_rng() + .sample_iter(Alphanumeric) + .take(length) + .map(char::from) + .collect(); + format!("{}{}", prefix, suffix) + } + + async fn insert_new_team_in_redis(client: Arc) -> Result { + let id = rand::thread_rng().gen_range(0..10_000_000); + let token = random_string("phc_", 12); + let team = Team { + id: id, + name: "team".to_string(), + api_token: token, + }; + + let serialized_team = serde_json::to_string(&team)?; + client + .set( + format!("{TEAM_TOKEN_CACHE_PREFIX}{}", team.api_token.clone()), + serialized_team, + ) + .await?; + + Ok(team) + } + + #[tokio::test] + async fn test_fetch_team_from_redis() { + let client = RedisClient::new("redis://localhost:6379/".to_string()) + .expect("Failed to create redis client"); + let client = Arc::new(client); + + let team = insert_new_team_in_redis(client.clone()).await.unwrap(); + + let target_token = team.api_token; + + let team_from_redis = Team::from_redis(client.clone(), target_token.clone()).await.unwrap(); + assert_eq!( + team_from_redis.api_token, target_token + ); + assert_eq!( + team_from_redis.id, team.id + ); + } + + #[tokio::test] + async fn test_fetch_invalid_team_from_redis() { + let client = RedisClient::new("redis://localhost:6379/".to_string()) + .expect("Failed to create redis client"); + let client = Arc::new(client); + + match Team::from_redis(client.clone(), "banana".to_string()).await { + Err(FlagError::TokenValidationError) => (), + _ => panic!("Expected TokenValidationError"), + }; + } +} From 327074cde9d40f0790439e1f1f84e0ab9a5a4626 Mon Sep 17 00:00:00 2001 From: Neil Kakkar Date: Thu, 9 May 2024 11:02:35 +0100 Subject: [PATCH 03/21] add more tests, common scaffolding --- feature-flags/src/lib.rs | 8 +++ feature-flags/src/team.rs | 49 +++---------------- feature-flags/src/test_utils.rs | 43 ++++++++++++++++ .../tests/{common.rs => common/mod.rs} | 24 ++++----- feature-flags/tests/test_flags.rs | 42 ++++++++++++++-- 5 files changed, 111 insertions(+), 55 deletions(-) create mode 100644 feature-flags/src/test_utils.rs rename feature-flags/tests/{common.rs => common/mod.rs} (77%) diff --git a/feature-flags/src/lib.rs b/feature-flags/src/lib.rs index 71a5e69..c9d07cc 100644 --- a/feature-flags/src/lib.rs +++ b/feature-flags/src/lib.rs @@ -6,3 +6,11 @@ pub mod server; pub mod v0_endpoint; pub mod v0_request; pub mod team; + +// Test modules don't need to be compiled with main binary +// #[cfg(test)] +// TODO: To use in integration tests, we need to compile with binary +// or make it a separate feature using cfg(feature = "integration-tests") +// and then use this feature only in tests. +// For now, ok to just include in binary +pub mod test_utils; diff --git a/feature-flags/src/team.rs b/feature-flags/src/team.rs index cfa54c3..d55aa93 100644 --- a/feature-flags/src/team.rs +++ b/feature-flags/src/team.rs @@ -11,7 +11,7 @@ use tracing::instrument; // It's from here: https://docs.djangoproject.com/en/4.2/topics/cache/#cache-versioning // F&!£%% on the bright side we don't use this functionality yet. // Will rely on integration tests to catch this. -const TEAM_TOKEN_CACHE_PREFIX: &str = "posthog:1:team_token:"; +pub const TEAM_TOKEN_CACHE_PREFIX: &str = "posthog:1:team_token:"; // TODO: Check what happens if json has extra stuff, does serde ignore it? Yes // Make sure we don't serialize and store team data in redis. Let main decide endpoint control this... @@ -63,48 +63,13 @@ impl Team { #[cfg(test)] mod tests { - use std::sync::Arc; - use anyhow::Error; - - use crate::redis::RedisClient; - use rand::{distributions::Alphanumeric, Rng}; - + use crate::test_utils::{insert_new_team_in_redis, setup_redis_client}; use super::*; - fn random_string(prefix: &str, length: usize) -> String { - let suffix: String = rand::thread_rng() - .sample_iter(Alphanumeric) - .take(length) - .map(char::from) - .collect(); - format!("{}{}", prefix, suffix) - } - - async fn insert_new_team_in_redis(client: Arc) -> Result { - let id = rand::thread_rng().gen_range(0..10_000_000); - let token = random_string("phc_", 12); - let team = Team { - id: id, - name: "team".to_string(), - api_token: token, - }; - - let serialized_team = serde_json::to_string(&team)?; - client - .set( - format!("{TEAM_TOKEN_CACHE_PREFIX}{}", team.api_token.clone()), - serialized_team, - ) - .await?; - - Ok(team) - } #[tokio::test] async fn test_fetch_team_from_redis() { - let client = RedisClient::new("redis://localhost:6379/".to_string()) - .expect("Failed to create redis client"); - let client = Arc::new(client); + let client = setup_redis_client(None); let team = insert_new_team_in_redis(client.clone()).await.unwrap(); @@ -121,10 +86,12 @@ mod tests { #[tokio::test] async fn test_fetch_invalid_team_from_redis() { - let client = RedisClient::new("redis://localhost:6379/".to_string()) - .expect("Failed to create redis client"); - let client = Arc::new(client); + let client = setup_redis_client(None); + // TODO: It's not ideal that this can fail on random errors like connection refused. + // Is there a way to be more specific throughout this code? + // Or maybe I shouldn't be mapping conn refused to token validation error, and instead handling it as a + // top level 500 error instead of 400 right now. match Team::from_redis(client.clone(), "banana".to_string()).await { Err(FlagError::TokenValidationError) => (), _ => panic!("Expected TokenValidationError"), diff --git a/feature-flags/src/test_utils.rs b/feature-flags/src/test_utils.rs new file mode 100644 index 0000000..1a91c8b --- /dev/null +++ b/feature-flags/src/test_utils.rs @@ -0,0 +1,43 @@ +use std::sync::Arc; +use anyhow::Error; + +use crate::{redis::{Client, RedisClient}, team::{self, Team}}; +use rand::{distributions::Alphanumeric, Rng}; + +pub fn random_string(prefix: &str, length: usize) -> String { + let suffix: String = rand::thread_rng() + .sample_iter(Alphanumeric) + .take(length) + .map(char::from) + .collect(); + format!("{}{}", prefix, suffix) +} + +pub async fn insert_new_team_in_redis(client: Arc) -> Result { + let id = rand::thread_rng().gen_range(0..10_000_000); + let token = random_string("phc_", 12); + let team = Team { + id: id, + name: "team".to_string(), + api_token: token, + }; + + let serialized_team = serde_json::to_string(&team)?; + client + .set( + format!("{}{}", team::TEAM_TOKEN_CACHE_PREFIX, team.api_token.clone()), + serialized_team, + ) + .await?; + + Ok(team) +} + +pub fn setup_redis_client(url: Option) -> Arc { + let redis_url = match url { + Some(value) => value, + None => "redis://localhost:6379/".to_string(), + }; + let client = RedisClient::new(redis_url).expect("Failed to create redis client"); + Arc::new(client) +} \ No newline at end of file diff --git a/feature-flags/tests/common.rs b/feature-flags/tests/common/mod.rs similarity index 77% rename from feature-flags/tests/common.rs rename to feature-flags/tests/common/mod.rs index f66a11f..5a63285 100644 --- a/feature-flags/tests/common.rs +++ b/feature-flags/tests/common/mod.rs @@ -4,8 +4,7 @@ use std::string::ToString; use std::sync::Arc; use once_cell::sync::Lazy; -use rand::distributions::Alphanumeric; -use rand::Rng; +use reqwest::header::CONTENT_TYPE; use tokio::net::TcpListener; use tokio::sync::Notify; @@ -44,6 +43,18 @@ impl ServerHandle { client .post(format!("http://{:?}/flags", self.addr)) .body(body) + .header(CONTENT_TYPE, "application/json") + .send() + .await + .expect("failed to send request") + } + + pub async fn send_invalid_header_for_flags_request>(&self, body: T) -> reqwest::Response { + let client = reqwest::Client::new(); + client + .post(format!("http://{:?}/flags", self.addr)) + .body(body) + .header(CONTENT_TYPE, "xyz") .send() .await .expect("failed to send request") @@ -55,12 +66,3 @@ impl Drop for ServerHandle { self.shutdown.notify_one() } } - -pub fn random_string(prefix: &str, length: usize) -> String { - let suffix: String = rand::thread_rng() - .sample_iter(Alphanumeric) - .take(length) - .map(char::from) - .collect(); - format!("{}_{}", prefix, suffix) -} diff --git a/feature-flags/tests/test_flags.rs b/feature-flags/tests/test_flags.rs index 82f41f0..5302ea9 100644 --- a/feature-flags/tests/test_flags.rs +++ b/feature-flags/tests/test_flags.rs @@ -5,14 +5,20 @@ use reqwest::StatusCode; use serde_json::{json, Value}; use crate::common::*; -mod common; + +use feature_flags::test_utils::{insert_new_team_in_redis, setup_redis_client}; + +pub mod common; #[tokio::test] async fn it_sends_flag_request() -> Result<()> { - let token = random_string("token", 16); + let config = DEFAULT_CONFIG.clone(); + let distinct_id = "user_distinct_id".to_string(); - let config = DEFAULT_CONFIG.clone(); + let client = setup_redis_client(Some(config.redis_url.clone())); + let team = insert_new_team_in_redis(client.clone()).await.unwrap(); + let token = team.api_token; let server = ServerHandle::for_config(config).await; @@ -41,3 +47,33 @@ async fn it_sends_flag_request() -> Result<()> { Ok(()) } + + +#[tokio::test] +async fn it_rejects_invalid_headers_flag_request() -> Result<()> { + let config = DEFAULT_CONFIG.clone(); + + let distinct_id = "user_distinct_id".to_string(); + + let client = setup_redis_client(Some(config.redis_url.clone())); + let team = insert_new_team_in_redis(client.clone()).await.unwrap(); + let token = team.api_token; + + let server = ServerHandle::for_config(config).await; + + let payload = json!({ + "token": token, + "distinct_id": distinct_id, + "groups": {"group1": "group1"} + }); + let res = server.send_invalid_header_for_flags_request(payload.to_string()).await; + assert_eq!(StatusCode::BAD_REQUEST, res.status()); + + // We don't want to deserialize the data into a flagResponse struct here, + // because we want to assert the shape of the raw json data. + let response_text = res.text().await?; + + assert_eq!(response_text, "failed to decode request: unsupported content type: xyz"); + + Ok(()) +} \ No newline at end of file From 838dd2c471ac4ddeecd51021e7b1e8ea5ab4a45d Mon Sep 17 00:00:00 2001 From: Neil Kakkar Date: Thu, 9 May 2024 11:19:05 +0100 Subject: [PATCH 04/21] lint --- feature-flags/src/lib.rs | 2 +- feature-flags/src/redis.rs | 5 +++-- feature-flags/src/team.rs | 26 +++++++++----------------- feature-flags/src/test_utils.rs | 15 +++++++++++---- feature-flags/src/v0_endpoint.rs | 4 +++- feature-flags/src/v0_request.rs | 9 ++++++--- feature-flags/tests/common/mod.rs | 5 ++++- feature-flags/tests/test_flags.rs | 12 ++++++++---- 8 files changed, 45 insertions(+), 33 deletions(-) diff --git a/feature-flags/src/lib.rs b/feature-flags/src/lib.rs index c9d07cc..195a55c 100644 --- a/feature-flags/src/lib.rs +++ b/feature-flags/src/lib.rs @@ -3,9 +3,9 @@ pub mod config; pub mod redis; pub mod router; pub mod server; +pub mod team; pub mod v0_endpoint; pub mod v0_request; -pub mod team; // Test modules don't need to be compiled with main binary // #[cfg(test)] diff --git a/feature-flags/src/redis.rs b/feature-flags/src/redis.rs index 70b7146..3f6dd7f 100644 --- a/feature-flags/src/redis.rs +++ b/feature-flags/src/redis.rs @@ -49,7 +49,8 @@ impl Client for RedisClient { let results = conn.get(k.clone()); // TODO: Is this safe? Should we be doing something else for error handling here? - let fut: Result, RedisError> = timeout(Duration::from_secs(REDIS_TIMEOUT_MILLISECS), results).await?; + let fut: Result, RedisError> = + timeout(Duration::from_secs(REDIS_TIMEOUT_MILLISECS), results).await?; // TRICKY: We serialise data to json, then django pickles it. // Here we deserialize the bytes using serde_pickle, to get the json string. @@ -70,4 +71,4 @@ impl Client for RedisClient { Ok(fut?) } -} \ No newline at end of file +} diff --git a/feature-flags/src/team.rs b/feature-flags/src/team.rs index d55aa93..54f7318 100644 --- a/feature-flags/src/team.rs +++ b/feature-flags/src/team.rs @@ -5,7 +5,6 @@ use crate::{api::FlagError, redis::Client}; use serde::{Deserialize, Serialize}; use tracing::instrument; - // TRICKY: I'm still not sure where the :1: is coming from. // The Django prefix is `posthog` only. // It's from here: https://docs.djangoproject.com/en/4.2/topics/cache/#cache-versioning @@ -31,26 +30,22 @@ pub struct Team { impl Team { /// Validates a token, and returns a team if it exists. - /// - + #[instrument(skip_all)] pub async fn from_redis( client: Arc, token: String, ) -> Result { - // TODO: Instead of failing here, i.e. if not in redis, fallback to pg let serialized_team = client - .get( - format!("{TEAM_TOKEN_CACHE_PREFIX}{}", token) - ) + .get(format!("{TEAM_TOKEN_CACHE_PREFIX}{}", token)) .await .map_err(|e| { tracing::error!("failed to fetch data: {}", e); // TODO: Can be other errors if serde_pickle destructuring fails? FlagError::TokenValidationError })?; - + let team: Team = serde_json::from_str(&serialized_team).map_err(|e| { tracing::error!("failed to parse data to team: {}", e); // TODO: Internal error, shouldn't send back to client @@ -63,9 +58,8 @@ impl Team { #[cfg(test)] mod tests { - use crate::test_utils::{insert_new_team_in_redis, setup_redis_client}; use super::*; - + use crate::test_utils::{insert_new_team_in_redis, setup_redis_client}; #[tokio::test] async fn test_fetch_team_from_redis() { @@ -75,13 +69,11 @@ mod tests { let target_token = team.api_token; - let team_from_redis = Team::from_redis(client.clone(), target_token.clone()).await.unwrap(); - assert_eq!( - team_from_redis.api_token, target_token - ); - assert_eq!( - team_from_redis.id, team.id - ); + let team_from_redis = Team::from_redis(client.clone(), target_token.clone()) + .await + .unwrap(); + assert_eq!(team_from_redis.api_token, target_token); + assert_eq!(team_from_redis.id, team.id); } #[tokio::test] diff --git a/feature-flags/src/test_utils.rs b/feature-flags/src/test_utils.rs index 1a91c8b..1604079 100644 --- a/feature-flags/src/test_utils.rs +++ b/feature-flags/src/test_utils.rs @@ -1,7 +1,10 @@ -use std::sync::Arc; use anyhow::Error; +use std::sync::Arc; -use crate::{redis::{Client, RedisClient}, team::{self, Team}}; +use crate::{ + redis::{Client, RedisClient}, + team::{self, Team}, +}; use rand::{distributions::Alphanumeric, Rng}; pub fn random_string(prefix: &str, length: usize) -> String { @@ -25,7 +28,11 @@ pub async fn insert_new_team_in_redis(client: Arc) -> Result) -> Arc { }; let client = RedisClient::new(redis_url).expect("Failed to create redis client"); Arc::new(client) -} \ No newline at end of file +} diff --git a/feature-flags/src/v0_endpoint.rs b/feature-flags/src/v0_endpoint.rs index 4a46d45..bbd7ff3 100644 --- a/feature-flags/src/v0_endpoint.rs +++ b/feature-flags/src/v0_endpoint.rs @@ -71,7 +71,9 @@ pub async fn flags( } }?; - let token = request.extract_and_verify_token(state.redis.clone()).await?; + let token = request + .extract_and_verify_token(state.redis.clone()) + .await?; tracing::Span::current().record("token", &token); diff --git a/feature-flags/src/v0_request.rs b/feature-flags/src/v0_request.rs index 2954b2e..f75ef56 100644 --- a/feature-flags/src/v0_request.rs +++ b/feature-flags/src/v0_request.rs @@ -54,17 +54,20 @@ impl FlagRequest { Ok(serde_json::from_str::(&payload)?) } - pub async fn extract_and_verify_token(&self, redis_client: Arc) -> Result { + pub async fn extract_and_verify_token( + &self, + redis_client: Arc, + ) -> Result { let token = match self { FlagRequest { token: Some(token), .. } => token.to_string(), _ => return Err(FlagError::NoTokenError), }; - + let team = Team::from_redis(redis_client, token.clone()).await?; - // TODO: Remove this, is useless, doing just for now because + // TODO: Remove this, is useless, doing just for now because tracing::Span::current().record("team_id", &team.id); Ok(token) } diff --git a/feature-flags/tests/common/mod.rs b/feature-flags/tests/common/mod.rs index 5a63285..c8644fe 100644 --- a/feature-flags/tests/common/mod.rs +++ b/feature-flags/tests/common/mod.rs @@ -49,7 +49,10 @@ impl ServerHandle { .expect("failed to send request") } - pub async fn send_invalid_header_for_flags_request>(&self, body: T) -> reqwest::Response { + pub async fn send_invalid_header_for_flags_request>( + &self, + body: T, + ) -> reqwest::Response { let client = reqwest::Client::new(); client .post(format!("http://{:?}/flags", self.addr)) diff --git a/feature-flags/tests/test_flags.rs b/feature-flags/tests/test_flags.rs index 5302ea9..2ceba24 100644 --- a/feature-flags/tests/test_flags.rs +++ b/feature-flags/tests/test_flags.rs @@ -48,7 +48,6 @@ async fn it_sends_flag_request() -> Result<()> { Ok(()) } - #[tokio::test] async fn it_rejects_invalid_headers_flag_request() -> Result<()> { let config = DEFAULT_CONFIG.clone(); @@ -66,14 +65,19 @@ async fn it_rejects_invalid_headers_flag_request() -> Result<()> { "distinct_id": distinct_id, "groups": {"group1": "group1"} }); - let res = server.send_invalid_header_for_flags_request(payload.to_string()).await; + let res = server + .send_invalid_header_for_flags_request(payload.to_string()) + .await; assert_eq!(StatusCode::BAD_REQUEST, res.status()); // We don't want to deserialize the data into a flagResponse struct here, // because we want to assert the shape of the raw json data. let response_text = res.text().await?; - assert_eq!(response_text, "failed to decode request: unsupported content type: xyz"); + assert_eq!( + response_text, + "failed to decode request: unsupported content type: xyz" + ); Ok(()) -} \ No newline at end of file +} From ad04232e0aeaf505a26c7776ff4ee84e1136d7f4 Mon Sep 17 00:00:00 2001 From: Neil Kakkar Date: Thu, 9 May 2024 14:15:37 +0100 Subject: [PATCH 05/21] clean up --- feature-flags/src/api.rs | 13 ++-- feature-flags/src/redis.rs | 35 +++++++++-- feature-flags/src/team.rs | 105 ++++++++++++++++++++++--------- feature-flags/src/test_utils.rs | 2 +- feature-flags/src/v0_endpoint.rs | 3 + feature-flags/src/v0_request.rs | 80 ++++++++++++++++++++--- 6 files changed, 192 insertions(+), 46 deletions(-) diff --git a/feature-flags/src/api.rs b/feature-flags/src/api.rs index ebad1f5..ccf4735 100644 --- a/feature-flags/src/api.rs +++ b/feature-flags/src/api.rs @@ -25,9 +25,6 @@ pub enum FlagError { #[error("failed to parse request: {0}")] RequestParsingError(#[from] serde_json::Error), - #[error("failed to parse redis data: {0}")] - DataParsingError(#[from] serde_pickle::Error), - #[error("Empty distinct_id in request")] EmptyDistinctId, #[error("No distinct_id in request")] @@ -40,6 +37,11 @@ pub enum FlagError { #[error("rate limited")] RateLimited, + + #[error("failed to parse redis cache data")] + DataParsingError, + #[error("redis unavailable")] + RedisUnavailable, } impl IntoResponse for FlagError { @@ -47,7 +49,6 @@ impl IntoResponse for FlagError { match self { FlagError::RequestDecodingError(_) | FlagError::RequestParsingError(_) - | FlagError::DataParsingError(_) | FlagError::EmptyDistinctId | FlagError::MissingDistinctId => (StatusCode::BAD_REQUEST, self.to_string()), @@ -56,6 +57,10 @@ impl IntoResponse for FlagError { } FlagError::RateLimited => (StatusCode::TOO_MANY_REQUESTS, self.to_string()), + + FlagError::DataParsingError | FlagError::RedisUnavailable => { + (StatusCode::SERVICE_UNAVAILABLE, self.to_string()) + } } .into_response() } diff --git a/feature-flags/src/redis.rs b/feature-flags/src/redis.rs index 3f6dd7f..3aeec47 100644 --- a/feature-flags/src/redis.rs +++ b/feature-flags/src/redis.rs @@ -3,11 +3,26 @@ use std::time::Duration; use anyhow::Result; use async_trait::async_trait; use redis::{AsyncCommands, RedisError}; +use thiserror::Error; use tokio::time::timeout; // average for all commands is <10ms, check grafana const REDIS_TIMEOUT_MILLISECS: u64 = 10; +#[derive(Error, Debug)] +pub enum CustomRedisError { + #[error("Not found in redis")] + NotFound, + + #[error("Pickle error: {0}")] + PickleError(#[from] serde_pickle::Error), + + #[error("Redis error: {0}")] + Other(#[from] RedisError), + + #[error("Timeout error")] + Timeout(#[from] tokio::time::error::Elapsed), +} /// A simple redis wrapper /// Copied from capture/src/redis.rs. /// TODO: Modify this to support hincrby @@ -17,7 +32,7 @@ pub trait Client { // A very simplified wrapper, but works for our usage async fn zrangebyscore(&self, k: String, min: String, max: String) -> Result>; - async fn get(&self, k: String) -> Result; + async fn get(&self, k: String) -> Result; async fn set(&self, k: String, v: String) -> Result<()>; } @@ -44,14 +59,26 @@ impl Client for RedisClient { Ok(fut?) } - async fn get(&self, k: String) -> Result { + // TODO: Ask Xavier if there's a better way to handle this. + // The problem: I want to match on the error type from this function, and do appropriate things like 400 or 500 response. + // Buuut, if I use anyhow::Error, I can't reverse-coerce into a NotFound or serde_pickle::Error. + // Thus, I need to create a custom error enum of all possible errors + my own custom not found, so I can match on it. + // Is this the canonical way? + async fn get(&self, k: String) -> Result { let mut conn = self.client.get_async_connection().await?; - let results = conn.get(k.clone()); - // TODO: Is this safe? Should we be doing something else for error handling here? + let results = conn.get(k); let fut: Result, RedisError> = timeout(Duration::from_secs(REDIS_TIMEOUT_MILLISECS), results).await?; + // return NotFound error when empty or not found + if match &fut { + Ok(v) => v.is_empty(), + Err(_) => false, + } { + return Err(CustomRedisError::NotFound); + } + // TRICKY: We serialise data to json, then django pickles it. // Here we deserialize the bytes using serde_pickle, to get the json string. let string_response: String = serde_pickle::from_slice(&fut?, Default::default())?; diff --git a/feature-flags/src/team.rs b/feature-flags/src/team.rs index 54f7318..ac62ea9 100644 --- a/feature-flags/src/team.rs +++ b/feature-flags/src/team.rs @@ -1,26 +1,16 @@ -use std::sync::Arc; - -use crate::{api::FlagError, redis::Client}; - use serde::{Deserialize, Serialize}; +use std::sync::Arc; use tracing::instrument; -// TRICKY: I'm still not sure where the :1: is coming from. -// The Django prefix is `posthog` only. -// It's from here: https://docs.djangoproject.com/en/4.2/topics/cache/#cache-versioning -// F&!£%% on the bright side we don't use this functionality yet. -// Will rely on integration tests to catch this. +use crate::{ + api::FlagError, + redis::{Client, CustomRedisError}, +}; + +// TRICKY: This cache data is coming from django-redis. If it ever goes out of sync, we'll bork. +// TODO: Add integration tests across repos to ensure this doesn't happen. pub const TEAM_TOKEN_CACHE_PREFIX: &str = "posthog:1:team_token:"; -// TODO: Check what happens if json has extra stuff, does serde ignore it? Yes -// Make sure we don't serialize and store team data in redis. Let main decide endpoint control this... -// and track misses. Revisit if this becomes an issue. -// because otherwise very annoying to keep this in sync with main django which has a lot of extra fields we need here. -// will lead to inconsistent behaviour. -// This is turning out to be very annoying, because we have django key prefixes to be mindful of as well. -// Wonder if it would be better to make these caches independent? This generates that new problem of CRUD happening in Django, -// which needs to update this cache immediately, so they can't really ever be independent. -// True for both team cache and flags cache. Hmm. Just I guess need to add tests around the key prefixes... #[derive(Debug, Deserialize, Serialize)] pub struct Team { pub id: i64, @@ -40,16 +30,21 @@ impl Team { let serialized_team = client .get(format!("{TEAM_TOKEN_CACHE_PREFIX}{}", token)) .await - .map_err(|e| { - tracing::error!("failed to fetch data: {}", e); - // TODO: Can be other errors if serde_pickle destructuring fails? - FlagError::TokenValidationError + .map_err(|e| match e { + CustomRedisError::NotFound => FlagError::TokenValidationError, + CustomRedisError::PickleError(_) => { + tracing::error!("failed to fetch data: {}", e); + FlagError::DataParsingError + } + _ => { + tracing::error!("Unknown redis error: {}", e); + FlagError::RedisUnavailable + } })?; let team: Team = serde_json::from_str(&serialized_team).map_err(|e| { tracing::error!("failed to parse data to team: {}", e); - // TODO: Internal error, shouldn't send back to client - FlagError::RequestParsingError(e) + FlagError::DataParsingError })?; Ok(team) @@ -58,8 +53,14 @@ impl Team { #[cfg(test)] mod tests { + use rand::Rng; + use redis::AsyncCommands; + use super::*; - use crate::test_utils::{insert_new_team_in_redis, setup_redis_client}; + use crate::{ + team, + test_utils::{insert_new_team_in_redis, random_string, setup_redis_client}, + }; #[tokio::test] async fn test_fetch_team_from_redis() { @@ -80,13 +81,59 @@ mod tests { async fn test_fetch_invalid_team_from_redis() { let client = setup_redis_client(None); - // TODO: It's not ideal that this can fail on random errors like connection refused. - // Is there a way to be more specific throughout this code? - // Or maybe I shouldn't be mapping conn refused to token validation error, and instead handling it as a - // top level 500 error instead of 400 right now. match Team::from_redis(client.clone(), "banana".to_string()).await { Err(FlagError::TokenValidationError) => (), _ => panic!("Expected TokenValidationError"), }; } + + #[tokio::test] + async fn test_cant_connect_to_redis_error_is_not_token_validation_error() { + let client = setup_redis_client(Some("redis://localhost:1111/".to_string())); + + match Team::from_redis(client.clone(), "banana".to_string()).await { + Err(FlagError::RedisUnavailable) => (), + _ => panic!("Expected RedisUnavailable"), + }; + } + + #[tokio::test] + async fn test_corrupted_data_in_redis_is_handled() { + // TODO: Extend this test with fallback to pg + let id = rand::thread_rng().gen_range(0..10_000_000); + let token = random_string("phc_", 12); + let team = Team { + id, + name: "team".to_string(), + api_token: token, + }; + let serialized_team = serde_json::to_string(&team).expect("Failed to serialise team"); + + // manually insert non-pickled data in redis + let client = + redis::Client::open("redis://localhost:6379/").expect("Failed to create redis client"); + let mut conn = client + .get_async_connection() + .await + .expect("Failed to get redis connection"); + conn.set::( + format!( + "{}{}", + team::TEAM_TOKEN_CACHE_PREFIX, + team.api_token.clone() + ), + serialized_team, + ) + .await + .expect("Failed to write data to redis"); + + // now get client connection for data + let client = setup_redis_client(None); + + match Team::from_redis(client.clone(), team.api_token.clone()).await { + Err(FlagError::DataParsingError) => (), + Err(other) => panic!("Expected DataParsingError, got {:?}", other), + Ok(_) => panic!("Expected DataParsingError"), + }; + } } diff --git a/feature-flags/src/test_utils.rs b/feature-flags/src/test_utils.rs index 1604079..75db86d 100644 --- a/feature-flags/src/test_utils.rs +++ b/feature-flags/src/test_utils.rs @@ -20,7 +20,7 @@ pub async fn insert_new_team_in_redis(client: Arc) -> Result Result { tracing::debug!(len = bytes.len(), "decoding new request"); @@ -65,10 +62,77 @@ impl FlagRequest { _ => return Err(FlagError::NoTokenError), }; - let team = Team::from_redis(redis_client, token.clone()).await?; + // validate token + Team::from_redis(redis_client, token.clone()).await?; + + // TODO: fallback when token not found in redis - // TODO: Remove this, is useless, doing just for now because - tracing::Span::current().record("team_id", &team.id); Ok(token) } + + pub fn extract_distinct_id(&self) -> Result { + let distinct_id = match &self.distinct_id { + None => return Err(FlagError::MissingDistinctId), + Some(id) => id, + }; + + match distinct_id.len() { + 0 => Err(FlagError::EmptyDistinctId), + 1..=200 => Ok(distinct_id.to_owned()), + _ => Ok(distinct_id.chars().take(200).collect()), + } + } +} + +#[cfg(test)] +mod tests { + use crate::api::FlagError; + use crate::v0_request::FlagRequest; + use bytes::Bytes; + use serde_json::json; + + #[test] + fn empty_distinct_id_not_accepted() { + let json = json!({ + "distinct_id": "", + "token": "my_token1", + }); + let bytes = Bytes::from(json.to_string()); + + let flag_payload = FlagRequest::from_bytes(bytes).expect("failed to parse request"); + + match flag_payload.extract_distinct_id() { + Err(FlagError::EmptyDistinctId) => (), + _ => panic!("expected empty distinct id error"), + }; + } + + #[test] + fn too_large_distinct_id_is_truncated() { + let json = json!({ + "distinct_id": std::iter::repeat("a").take(210).collect::(), + "token": "my_token1", + }); + let bytes = Bytes::from(json.to_string()); + + let flag_payload = FlagRequest::from_bytes(bytes).expect("failed to parse request"); + + assert_eq!(flag_payload.extract_distinct_id().unwrap().len(), 200); + } + + #[test] + fn distinct_id_is_returned_correctly() { + let json = json!({ + "$distinct_id": "alakazam", + "token": "my_token1", + }); + let bytes = Bytes::from(json.to_string()); + + let flag_payload = FlagRequest::from_bytes(bytes).expect("failed to parse request"); + + match flag_payload.extract_distinct_id() { + Ok(id) => assert_eq!(id, "alakazam"), + _ => panic!("expected distinct id"), + }; + } } From 6e7335bc5add0a97252ee092cf6841cf65c6cdeb Mon Sep 17 00:00:00 2001 From: Neil Kakkar Date: Fri, 10 May 2024 19:14:21 +0100 Subject: [PATCH 06/21] feat(flags): Add definition and fetch from redis --- feature-flags/src/lib.rs | 1 + feature-flags/src/test_utils.rs | 45 +++++++++++++++++++++++++++++++-- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/feature-flags/src/lib.rs b/feature-flags/src/lib.rs index 195a55c..5afe995 100644 --- a/feature-flags/src/lib.rs +++ b/feature-flags/src/lib.rs @@ -4,6 +4,7 @@ pub mod redis; pub mod router; pub mod server; pub mod team; +pub mod flag_definitions; pub mod v0_endpoint; pub mod v0_request; diff --git a/feature-flags/src/test_utils.rs b/feature-flags/src/test_utils.rs index 75db86d..57ac225 100644 --- a/feature-flags/src/test_utils.rs +++ b/feature-flags/src/test_utils.rs @@ -1,9 +1,9 @@ use anyhow::Error; +use serde_json::json; use std::sync::Arc; use crate::{ - redis::{Client, RedisClient}, - team::{self, Team}, + flag_definitions, redis::{Client, RedisClient}, team::{self, Team} }; use rand::{distributions::Alphanumeric, Rng}; @@ -40,6 +40,47 @@ pub async fn insert_new_team_in_redis(client: Arc) -> Result, team_id: i64, json_value: Option) -> Result<(), Error> { + + let payload = match json_value { + Some(value) => value, + None => json!([{ + "id": 1, + "key": "flag1", + "name": "flag1 description", + "active": true, + "deleted": false, + "team_id": team_id, + "filters": { + "groups": [ + { + "properties": [ + { + "key": "email", + "value": "a@b.com", + "type": "person", + }, + ] + }, + ], + }, + }]).to_string(), + }; + + client + .set( + format!( + "{}{}", + flag_definitions::TEAM_FLAGS_CACHE_PREFIX, + team_id + ), + payload, + ) + .await?; + + Ok(()) +} + pub fn setup_redis_client(url: Option) -> Arc { let redis_url = match url { Some(value) => value, From 56e85a011f9a15dfbb9bd6cb8b390fa2264a5413 Mon Sep 17 00:00:00 2001 From: Neil Kakkar Date: Fri, 10 May 2024 19:15:14 +0100 Subject: [PATCH 07/21] add file --- feature-flags/src/flag_definitions.rs | 201 ++++++++++++++++++++++++++ 1 file changed, 201 insertions(+) create mode 100644 feature-flags/src/flag_definitions.rs diff --git a/feature-flags/src/flag_definitions.rs b/feature-flags/src/flag_definitions.rs new file mode 100644 index 0000000..a1345be --- /dev/null +++ b/feature-flags/src/flag_definitions.rs @@ -0,0 +1,201 @@ +use serde::{Deserialize, Serialize}; +use std::sync::Arc; +use tracing::instrument; + +use crate::{ + api::FlagError, + redis::{Client, CustomRedisError}, +}; + +// TRICKY: This cache data is coming from django-redis. If it ever goes out of sync, we'll bork. +// TODO: Add integration tests across repos to ensure this doesn't happen. +pub const TEAM_FLAGS_CACHE_PREFIX: &str = "posthog:1:team_feature_flags_"; + +// TODO: Hmm, revisit when dealing with groups, but seems like +// ideal to just treat it as a u8 and do our own validation on top +#[derive(Debug, Deserialize, Serialize)] +pub enum GroupTypeIndex { + +} + +#[derive(Debug, Deserialize, Serialize)] +pub enum OperatorType { + #[serde(rename = "exact")] + Exact, + #[serde(rename = "is_not")] + IsNot, + #[serde(rename = "icontains")] + Icontains, + #[serde(rename = "not_icontains")] + NotIcontains, + #[serde(rename = "regex")] + Regex, + #[serde(rename = "not_regex")] + NotRegex, + #[serde(rename = "gt")] + Gt, + #[serde(rename = "lt")] + Lt, + #[serde(rename = "gte")] + Gte, + #[serde(rename = "lte")] + Lte, + #[serde(rename = "is_set")] + IsSet, + #[serde(rename = "is_not_set")] + IsNotSet, + #[serde(rename = "is_date_exact")] + IsDateExact, + #[serde(rename = "is_date_after")] + IsDateAfter, + #[serde(rename = "is_date_before")] + IsDateBefore, +} + +#[derive(Debug, Deserialize, Serialize)] +pub struct PropertyFilter { + pub key: String, + pub value: serde_json::Value, + pub operator: Option, + #[serde(rename = "type")] + pub prop_type: String, + pub group_type_index: Option, +} + +#[derive(Debug, Deserialize, Serialize)] +pub struct FlagGroupType { + pub properties: Option>, + pub rollout_percentage: Option, + pub variant: Option, +} + +#[derive(Debug, Deserialize, Serialize)] +pub struct MultivariateFlagVariant { + pub key: String, + pub name: Option, + pub rollout_percentage: f32, +} + +#[derive(Debug, Deserialize, Serialize)] +pub struct MultivariateFlagOptions { + pub variants: Vec, +} + +// TODO: test name with https://www.fileformat.info/info/charset/UTF-16/list.htm values, like '𝖕𝖗𝖔𝖕𝖊𝖗𝖙𝖞': `𝓿𝓪𝓵𝓾𝓮` + + +#[derive(Debug, Deserialize, Serialize)] +pub struct FlagFilters { + pub groups: Vec, + pub multivariate: Option, + pub aggregation_group_type_index: Option, + pub payloads: Option, + pub super_groups: Option>, +} + +#[derive(Debug, Deserialize, Serialize)] +pub struct FeatureFlag { + pub id: i64, + pub team_id: i64, + pub name: Option, + pub key: String, + pub filters: FlagFilters, + #[serde(default)] + pub deleted: bool, + #[serde(default)] + pub active: bool, + #[serde(default)] + pub ensure_experience_continuity: bool, +} + +#[derive(Debug, Deserialize, Serialize)] + +pub struct FeatureFlagList { + pub flags: Vec, +} + +impl FeatureFlagList { + /// Returns feature flags given a team_id + + #[instrument(skip_all)] + pub async fn from_redis( + client: Arc, + team_id: i64, + ) -> Result { + // TODO: Instead of failing here, i.e. if not in redis, fallback to pg + let serialized_flags = client + .get(format!("{TEAM_FLAGS_CACHE_PREFIX}{}", team_id)) + .await + .map_err(|e| match e { + CustomRedisError::NotFound => FlagError::TokenValidationError, + CustomRedisError::PickleError(_) => { + tracing::error!("failed to fetch data: {}", e); + println!("failed to fetch data: {}", e); + FlagError::DataParsingError + } + _ => { + tracing::error!("Unknown redis error: {}", e); + FlagError::RedisUnavailable + } + })?; + + let flags_list: Vec = serde_json::from_str(&serialized_flags).map_err(|e| { + tracing::error!("failed to parse data to flags list: {}", e); + println!("failed to parse data: {}", e); + + FlagError::DataParsingError + })?; + + Ok(FeatureFlagList { + flags: flags_list, + }) + } +} + +#[cfg(test)] +mod tests { + use rand::Rng; + + use super::*; + use crate::test_utils::{insert_new_team_in_redis, insert_flags_for_team_in_redis, setup_redis_client}; + + #[tokio::test] + async fn test_fetch_flags_from_redis() { + let client = setup_redis_client(None); + + let team = insert_new_team_in_redis(client.clone()).await.unwrap(); + + insert_flags_for_team_in_redis(client.clone(), team.id, None).await.expect("Failed to insert flags"); + + let flags_from_redis = FeatureFlagList::from_redis(client.clone(), team.id) + .await + .unwrap(); + assert_eq!(flags_from_redis.flags.len(), 1); + let flag = flags_from_redis.flags.get(0).unwrap(); + assert_eq!(flag.key, "flag1"); + assert_eq!(flag.team_id, team.id); + assert_eq!(flag.filters.groups.len(), 1); + assert_eq!(flag.filters.groups[0].properties.as_ref().unwrap().len(), 1); + } + + #[tokio::test] + async fn test_fetch_invalid_team_from_redis() { + let client = setup_redis_client(None); + + match FeatureFlagList::from_redis(client.clone(), 1234).await { + Err(FlagError::TokenValidationError) => (), + _ => panic!("Expected TokenValidationError"), + }; + } + + #[tokio::test] + async fn test_cant_connect_to_redis_error_is_not_token_validation_error() { + let client = setup_redis_client(Some("redis://localhost:1111/".to_string())); + + match FeatureFlagList::from_redis(client.clone(), 1234).await { + Err(FlagError::RedisUnavailable) => (), + _ => panic!("Expected RedisUnavailable"), + }; + } + +} From d040b944c8cfc7c50cb76ad98511b08adb044367 Mon Sep 17 00:00:00 2001 From: Neil Kakkar Date: Wed, 29 May 2024 14:22:39 +0100 Subject: [PATCH 08/21] wip --- feature-flags/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/feature-flags/src/lib.rs b/feature-flags/src/lib.rs index 5afe995..0aaee3c 100644 --- a/feature-flags/src/lib.rs +++ b/feature-flags/src/lib.rs @@ -5,6 +5,7 @@ pub mod router; pub mod server; pub mod team; pub mod flag_definitions; +pub mod flag_matching; pub mod v0_endpoint; pub mod v0_request; From 278fab7b14d8ae3ae18e28a9aebbebb56e485e09 Mon Sep 17 00:00:00 2001 From: Neil Kakkar Date: Wed, 29 May 2024 17:49:19 +0100 Subject: [PATCH 09/21] build basic flag matching --- Cargo.lock | 2 + feature-flags/Cargo.toml | 2 + feature-flags/src/flag_definitions.rs | 32 +- feature-flags/src/flag_matching.rs | 140 ++ feature-flags/src/test_utils.rs | 34 +- .../tests/test_flag_matching_consistency.rs | 2139 +++++++++++++++++ 6 files changed, 2339 insertions(+), 10 deletions(-) create mode 100644 feature-flags/src/flag_matching.rs create mode 100644 feature-flags/tests/test_flag_matching_consistency.rs diff --git a/Cargo.lock b/Cargo.lock index 8642ade..162877b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -702,6 +702,7 @@ dependencies = [ "axum-client-ip", "bytes", "envconfig", + "hex", "once_cell", "rand", "redis", @@ -709,6 +710,7 @@ dependencies = [ "serde", "serde-pickle", "serde_json", + "sha1", "thiserror", "tokio", "tracing", diff --git a/feature-flags/Cargo.toml b/feature-flags/Cargo.toml index 1e0c111..85dd0b9 100644 --- a/feature-flags/Cargo.toml +++ b/feature-flags/Cargo.toml @@ -25,6 +25,8 @@ serde = { workspace = true } serde_json = { workspace = true } thiserror = { workspace = true } serde-pickle = { version = "1.1.1"} +hex = "0.4.3" +sha1 = "0.10.6" [lints] workspace = true diff --git a/feature-flags/src/flag_definitions.rs b/feature-flags/src/flag_definitions.rs index a1345be..9366573 100644 --- a/feature-flags/src/flag_definitions.rs +++ b/feature-flags/src/flag_definitions.rs @@ -18,7 +18,7 @@ pub enum GroupTypeIndex { } -#[derive(Debug, Deserialize, Serialize)] +#[derive(Debug, Clone, Deserialize, Serialize)] pub enum OperatorType { #[serde(rename = "exact")] Exact, @@ -52,7 +52,7 @@ pub enum OperatorType { IsDateBefore, } -#[derive(Debug, Deserialize, Serialize)] +#[derive(Debug, Clone, Deserialize, Serialize)] pub struct PropertyFilter { pub key: String, pub value: serde_json::Value, @@ -62,21 +62,21 @@ pub struct PropertyFilter { pub group_type_index: Option, } -#[derive(Debug, Deserialize, Serialize)] +#[derive(Debug, Clone, Deserialize, Serialize)] pub struct FlagGroupType { pub properties: Option>, - pub rollout_percentage: Option, + pub rollout_percentage: Option, pub variant: Option, } -#[derive(Debug, Deserialize, Serialize)] +#[derive(Debug, Clone, Deserialize, Serialize)] pub struct MultivariateFlagVariant { pub key: String, pub name: Option, - pub rollout_percentage: f32, + pub rollout_percentage: f64, } -#[derive(Debug, Deserialize, Serialize)] +#[derive(Debug, Clone, Deserialize, Serialize)] pub struct MultivariateFlagOptions { pub variants: Vec, } @@ -84,7 +84,7 @@ pub struct MultivariateFlagOptions { // TODO: test name with https://www.fileformat.info/info/charset/UTF-16/list.htm values, like '𝖕𝖗𝖔𝖕𝖊𝖗𝖙𝖞': `𝓿𝓪𝓵𝓾𝓮` -#[derive(Debug, Deserialize, Serialize)] +#[derive(Debug, Clone, Deserialize, Serialize)] pub struct FlagFilters { pub groups: Vec, pub multivariate: Option, @@ -93,7 +93,7 @@ pub struct FlagFilters { pub super_groups: Option>, } -#[derive(Debug, Deserialize, Serialize)] +#[derive(Debug, Clone, Deserialize, Serialize)] pub struct FeatureFlag { pub id: i64, pub team_id: i64, @@ -108,6 +108,20 @@ pub struct FeatureFlag { pub ensure_experience_continuity: bool, } +impl FeatureFlag { + pub fn get_group_type_index(&self) -> Option { + self.filters.aggregation_group_type_index + } + + pub fn get_conditions(&self) -> &Vec { + &self.filters.groups + } + + pub fn get_variants(&self) -> Vec { + self.filters.multivariate.clone().map_or(vec![], |m| m.variants) + } +} + #[derive(Debug, Deserialize, Serialize)] pub struct FeatureFlagList { diff --git a/feature-flags/src/flag_matching.rs b/feature-flags/src/flag_matching.rs new file mode 100644 index 0000000..3456783 --- /dev/null +++ b/feature-flags/src/flag_matching.rs @@ -0,0 +1,140 @@ +use sha1::{Digest, Sha1}; + +use crate::flag_definitions::{FeatureFlag, FlagGroupType}; + +#[derive(Debug, PartialEq, Eq)] +pub struct FeatureFlagMatch { + pub matches: bool, + pub variant: Option, + //reason + //condition_index + //payload +} + +#[derive(Debug)] +pub struct FeatureFlagMatcher { + // pub flags: Vec, + pub distinct_id: String, +} + +const LONG_SCALE: u64 = 0xfffffffffffffff; + +impl FeatureFlagMatcher { + + pub fn new(distinct_id: String) -> Self { + FeatureFlagMatcher { + // flags, + distinct_id, + } + } + + pub fn get_match(&self, feature_flag: &FeatureFlag) -> FeatureFlagMatch { + + if self.hashed_identifier(feature_flag).is_none() { + return FeatureFlagMatch { + matches: false, + variant: None, + }; + } + + // TODO: super groups + // TODO: Variant overrides condition sort + + for (index, condition) in feature_flag.get_conditions().iter().enumerate() { + let (is_match, evaluation_reason) = self.is_condition_match(feature_flag, condition, index); + + if is_match { + + let variant = match condition.variant.clone() { + Some(variant_override) => { + if feature_flag.get_variants().iter().any(|v| v.key == variant_override) { + Some(variant_override) + } else { + self.get_matching_variant(feature_flag) + } + } + None => { + self.get_matching_variant(feature_flag) + } + }; + + // let payload = self.get_matching_payload(is_match, variant, feature_flag); + return FeatureFlagMatch { + matches: true, + variant, + }; + } + } + FeatureFlagMatch { + matches: false, + variant: None, + } + } + + pub fn is_condition_match(&self, feature_flag: &FeatureFlag, condition: &FlagGroupType, _index: usize) -> (bool, String) { + let rollout_percentage = condition.rollout_percentage.unwrap_or(100.0); + let mut condition_match = true; + if condition.properties.is_some() { + // TODO: Handle matching conditions + if !condition.properties.as_ref().unwrap().is_empty() { + condition_match = false; + } + } + + if !condition_match { + return (false, "NO_CONDITION_MATCH".to_string()); + } else if rollout_percentage == 100.0 { + // TODO: Check floating point schenanigans if any + return (true, "CONDITION_MATCH".to_string()); + } + + if self.get_hash(feature_flag, "") > (rollout_percentage / 100.0) { + return (false, "OUT_OF_ROLLOUT_BOUND".to_string()); + } + + (true, "CONDITION_MATCH".to_string()) + + } + + pub fn hashed_identifier(&self, feature_flag: &FeatureFlag) -> Option { + if feature_flag.get_group_type_index().is_none() { + // TODO: Use hash key overrides for experience continuity + Some(self.distinct_id.clone()) + } else { + // TODO: Handle getting group key + Some("".to_string()) + } + } + + /// This function takes a identifier and a feature flag key and returns a float between 0 and 1. + /// Given the same identifier and key, it'll always return the same float. These floats are + /// uniformly distributed between 0 and 1, so if we want to show this feature to 20% of traffic + /// we can do _hash(key, identifier) < 0.2 + pub fn get_hash(&self, feature_flag: &FeatureFlag, salt: &str) -> f64 { + // check if hashed_identifier is None + let hashed_identifier = self.hashed_identifier(feature_flag).expect("hashed_identifier is None when computing hash"); + let hash_key = format!("{}.{}{}", feature_flag.key, hashed_identifier, salt); + let mut hasher = Sha1::new(); + hasher.update(hash_key.as_bytes()); + let result = hasher.finalize(); + // :TRICKY: Convert the first 15 characters of the digest to a hexadecimal string + // not sure if this is correct, padding each byte as 2 characters + let hex_str: String = result.iter().map(|byte| format!("{:02x}", byte)).collect::()[..15].to_string(); + let hash_val = u64::from_str_radix(&hex_str, 16).unwrap(); + + hash_val as f64 / LONG_SCALE as f64 + } + + pub fn get_matching_variant(&self, feature_flag: &FeatureFlag) -> Option { + let hash = self.get_hash(feature_flag, "variant"); + let mut total_percentage = 0.0; + + for variant in feature_flag.get_variants() { + total_percentage += variant.rollout_percentage / 100.0; + if hash < total_percentage { + return Some(variant.key.clone()); + } + } + None + } +} diff --git a/feature-flags/src/test_utils.rs b/feature-flags/src/test_utils.rs index 57ac225..a9cc08b 100644 --- a/feature-flags/src/test_utils.rs +++ b/feature-flags/src/test_utils.rs @@ -3,7 +3,7 @@ use serde_json::json; use std::sync::Arc; use crate::{ - flag_definitions, redis::{Client, RedisClient}, team::{self, Team} + flag_definitions::{self, FeatureFlag}, redis::{Client, RedisClient}, team::{self, Team} }; use rand::{distributions::Alphanumeric, Rng}; @@ -89,3 +89,35 @@ pub fn setup_redis_client(url: Option) -> Arc { let client = RedisClient::new(redis_url).expect("Failed to create redis client"); Arc::new(client) } + +pub fn create_flag_from_json(json_value: Option) -> Vec { + + let payload = match json_value { + Some(value) => value, + None => json!([{ + "id": 1, + "key": "flag1", + "name": "flag1 description", + "active": true, + "deleted": false, + "team_id": 1, + "filters": { + "groups": [ + { + "properties": [ + { + "key": "email", + "value": "a@b.com", + "type": "person", + }, + ], + "rollout_percentage": 50, + }, + ], + }, + }]).to_string(), + }; + + let flags: Vec = serde_json::from_str(&payload).expect("Failed to parse data to flags list"); + flags +} \ No newline at end of file diff --git a/feature-flags/tests/test_flag_matching_consistency.rs b/feature-flags/tests/test_flag_matching_consistency.rs new file mode 100644 index 0000000..5fdffa7 --- /dev/null +++ b/feature-flags/tests/test_flag_matching_consistency.rs @@ -0,0 +1,2139 @@ +/// These tests are common between all libraries doing local evaluation of feature flags. +/// This ensures there are no mismatches between implementations. + + +use feature_flags::flag_matching::{FeatureFlagMatch, FeatureFlagMatcher}; +// use feature_flags::flag_definitions::{FeatureFlag, FlagGroupType}; + +use feature_flags::test_utils::create_flag_from_json; +use serde_json::json; + +#[test] +fn it_is_consistent_with_rollout_calculation_for_simple_flags() { + + let flags = create_flag_from_json(Some(json!([{ + "id": 1, + "key": "simple-flag", + "name": "Simple flag", + "active": true, + "deleted": false, + "team_id": 1, + "filters": { + "groups": [ + { + "properties": [], + "rollout_percentage": 45, + }, + ], + }, + }]).to_string())); + + + let results = vec![ + false, + true, + true, + false, + true, + false, + false, + true, + false, + true, + false, + true, + true, + false, + true, + false, + false, + false, + true, + true, + false, + true, + false, + false, + true, + false, + true, + true, + false, + false, + false, + true, + true, + true, + true, + false, + false, + false, + false, + false, + false, + true, + true, + false, + true, + true, + false, + false, + false, + true, + true, + false, + false, + false, + false, + true, + false, + true, + false, + true, + false, + true, + true, + false, + true, + false, + true, + false, + true, + true, + false, + false, + true, + false, + false, + true, + false, + true, + false, + false, + true, + false, + false, + false, + true, + true, + false, + true, + true, + false, + true, + true, + true, + true, + true, + false, + true, + true, + false, + false, + true, + true, + true, + true, + false, + false, + true, + false, + true, + true, + true, + false, + false, + false, + false, + false, + true, + false, + false, + true, + true, + true, + false, + false, + true, + false, + true, + false, + false, + true, + false, + false, + false, + false, + false, + false, + false, + false, + true, + true, + false, + false, + true, + false, + false, + true, + true, + false, + false, + true, + false, + true, + false, + true, + true, + true, + false, + false, + false, + true, + false, + false, + false, + false, + true, + true, + false, + true, + true, + false, + true, + false, + true, + true, + false, + true, + false, + true, + true, + true, + false, + true, + false, + false, + true, + true, + false, + true, + false, + true, + true, + false, + false, + true, + true, + true, + true, + false, + true, + true, + false, + false, + true, + false, + true, + false, + false, + true, + true, + false, + true, + false, + true, + false, + false, + false, + false, + false, + false, + false, + true, + false, + true, + true, + false, + false, + true, + false, + true, + false, + false, + false, + true, + false, + true, + false, + false, + false, + true, + false, + false, + true, + false, + true, + true, + false, + false, + false, + false, + true, + false, + false, + false, + false, + false, + false, + false, + false, + false, + false, + false, + false, + false, + true, + true, + false, + true, + false, + true, + true, + false, + true, + false, + true, + false, + false, + false, + true, + true, + true, + true, + false, + false, + false, + false, + false, + true, + true, + true, + false, + false, + true, + true, + false, + false, + false, + false, + false, + true, + false, + true, + true, + true, + true, + false, + true, + true, + true, + false, + false, + true, + false, + true, + false, + false, + true, + true, + true, + false, + true, + false, + false, + false, + true, + true, + false, + true, + false, + true, + false, + true, + true, + true, + true, + true, + false, + false, + true, + false, + true, + false, + true, + true, + true, + false, + true, + false, + true, + true, + false, + true, + true, + true, + true, + true, + false, + false, + false, + false, + false, + true, + false, + true, + false, + false, + true, + true, + false, + false, + false, + true, + false, + true, + true, + true, + true, + false, + false, + false, + false, + true, + true, + false, + false, + true, + true, + false, + true, + true, + true, + true, + false, + true, + true, + true, + false, + false, + true, + true, + false, + false, + true, + false, + false, + true, + false, + false, + false, + false, + false, + false, + false, + false, + false, + false, + true, + true, + false, + false, + true, + false, + false, + true, + false, + true, + false, + false, + true, + false, + false, + false, + false, + false, + false, + true, + false, + false, + false, + false, + false, + false, + false, + false, + false, + true, + true, + true, + false, + false, + false, + true, + false, + true, + false, + false, + false, + true, + false, + false, + false, + false, + false, + false, + false, + true, + false, + false, + false, + false, + false, + false, + false, + false, + true, + false, + true, + false, + true, + true, + true, + false, + false, + false, + true, + true, + true, + false, + true, + false, + true, + true, + false, + false, + false, + true, + false, + false, + false, + false, + true, + false, + true, + false, + true, + true, + false, + true, + false, + false, + false, + true, + false, + false, + true, + true, + false, + true, + false, + false, + false, + false, + false, + false, + true, + true, + false, + false, + true, + false, + false, + true, + true, + true, + false, + false, + false, + true, + false, + false, + false, + false, + true, + false, + true, + false, + false, + false, + true, + false, + true, + true, + false, + true, + false, + true, + false, + true, + false, + false, + true, + false, + false, + true, + false, + true, + false, + true, + false, + true, + false, + false, + true, + true, + true, + true, + false, + true, + false, + false, + false, + false, + false, + true, + false, + false, + true, + false, + false, + true, + true, + false, + false, + false, + false, + true, + true, + true, + false, + false, + true, + false, + false, + true, + true, + true, + true, + false, + false, + false, + true, + false, + false, + false, + true, + false, + false, + true, + true, + true, + true, + false, + false, + true, + true, + false, + true, + false, + true, + false, + false, + true, + true, + false, + true, + true, + true, + true, + false, + false, + true, + false, + false, + true, + true, + false, + true, + false, + true, + false, + false, + true, + false, + false, + false, + false, + true, + true, + true, + false, + true, + false, + false, + true, + false, + false, + true, + false, + false, + false, + false, + true, + false, + true, + false, + true, + true, + false, + false, + true, + false, + true, + true, + true, + false, + false, + false, + false, + true, + true, + false, + true, + false, + false, + false, + true, + false, + false, + false, + false, + true, + true, + true, + false, + false, + false, + true, + true, + true, + true, + false, + true, + true, + false, + true, + true, + true, + false, + true, + false, + false, + true, + false, + true, + true, + true, + true, + false, + true, + false, + true, + false, + true, + false, + false, + true, + true, + false, + false, + true, + false, + true, + false, + false, + false, + false, + true, + false, + true, + false, + false, + false, + true, + true, + true, + false, + false, + false, + true, + false, + true, + true, + false, + false, + false, + false, + false, + true, + false, + true, + false, + false, + true, + true, + false, + true, + true, + true, + true, + false, + false, + true, + false, + false, + true, + false, + true, + false, + true, + true, + false, + false, + false, + true, + false, + true, + true, + false, + false, + false, + true, + false, + true, + false, + true, + true, + false, + true, + false, + false, + true, + false, + false, + false, + true, + true, + true, + false, + false, + false, + false, + false, + true, + false, + false, + true, + true, + true, + true, + true, + false, + false, + false, + false, + false, + false, + false, + false, + true, + true, + true, + false, + false, + true, + true, + false, + true, + true, + false, + true, + false, + true, + false, + false, + false, + true, + false, + false, + true, + false, + false, + true, + true, + true, + true, + false, + false, + true, + false, + true, + true, + false, + false, + true, + false, + false, + true, + true, + false, + true, + false, + false, + true, + true, + true, + false, + false, + false, + false, + false, + true, + false, + true, + false, + false, + false, + false, + false, + true, + true, + false, + true, + true, + true, + false, + false, + false, + false, + true, + true, + true, + true, + false, + true, + true, + false, + true, + false, + true, + false, + true, + false, + false, + false, + false, + true, + true, + true, + true, + false, + false, + true, + false, + true, + true, + false, + false, + false, + false, + false, + false, + true, + false, + true, + false, + true, + true, + false, + false, + true, + true, + true, + true, + false, + false, + true, + false, + true, + true, + false, + false, + true, + true, + true, + false, + true, + false, + false, + true, + true, + false, + false, + false, + true, + false, + false, + true, + false, + false, + false, + true, + true, + true, + true, + false, + true, + false, + true, + false, + true, + false, + true, + false, + false, + true, + false, + false, + true, + false, + true, + true, + ]; + + + + for i in 0..1000 { + let distinct_id = format!("distinct_id_{}", i); + + let feature_flag_match = FeatureFlagMatcher::new(distinct_id).get_match(&flags[0]); + + if results[i] { + assert_eq!( + feature_flag_match, + FeatureFlagMatch { + matches: true, + variant: None, + } + ); + } else { + assert_eq!( + feature_flag_match, + FeatureFlagMatch { + matches: false, + variant: None, + } + ); + } + } + +} + +#[test] +fn it_is_consistent_with_rollout_calculation_for_multivariate_flags() { + + let flags = create_flag_from_json(Some(json!([{ + "id": 1, + "key": "multivariate-flag", + "name": "Multivariate flag", + "active": true, + "deleted": false, + "team_id": 1, + "filters": { + "groups": [ + { + "properties": [], + "rollout_percentage": 55, + }, + ], + "multivariate": { + "variants": [ + { + "key": "first-variant", + "name": "First Variant", + "rollout_percentage": 50, + }, + { + "key": "second-variant", + "name": "Second Variant", + "rollout_percentage": 20, + }, + { + "key": "third-variant", + "name": "Third Variant", + "rollout_percentage": 20, + }, + { + "key": "fourth-variant", + "name": "Fourth Variant", + "rollout_percentage": 5, + }, + { + "key": "fifth-variant", + "name": "Fifth Variant", + "rollout_percentage": 5, + }, + ], + }, + }, + }]).to_string())); + + + let results = vec![ + Some("second-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("fourth-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("third-variant".to_string()), + None, + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("third-variant".to_string()), + None, + Some("third-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("third-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + Some("second-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("second-variant".to_string()), + Some("second-variant".to_string()), + Some("third-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("fourth-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("second-variant".to_string()), + None, + Some("third-variant".to_string()), + None, + None, + None, + None, + None, + None, + Some("first-variant".to_string()), + Some("fifth-variant".to_string()), + None, + Some("second-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + None, + Some("third-variant".to_string()), + Some("third-variant".to_string()), + None, + None, + None, + None, + Some("third-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("third-variant".to_string()), + Some("third-variant".to_string()), + None, + Some("third-variant".to_string()), + Some("second-variant".to_string()), + Some("third-variant".to_string()), + None, + None, + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + None, + None, + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + None, + None, + None, + None, + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + None, + Some("second-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("second-variant".to_string()), + Some("second-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + None, + None, + Some("third-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + None, + None, + None, + None, + Some("first-variant".to_string()), + None, + None, + None, + None, + None, + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + None, + None, + Some("fifth-variant".to_string()), + Some("second-variant".to_string()), + None, + Some("second-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("fifth-variant".to_string()), + Some("third-variant".to_string()), + None, + None, + Some("fourth-variant".to_string()), + None, + None, + None, + None, + Some("third-variant".to_string()), + None, + None, + Some("third-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("second-variant".to_string()), + Some("second-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + None, + None, + None, + Some("second-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("third-variant".to_string()), + Some("third-variant".to_string()), + None, + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + Some("fifth-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + None, + Some("second-variant".to_string()), + Some("third-variant".to_string()), + Some("third-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + None, + Some("third-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + Some("second-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("third-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + Some("third-variant".to_string()), + None, + Some("third-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("fifth-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + None, + None, + None, + Some("third-variant".to_string()), + None, + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + None, + Some("third-variant".to_string()), + None, + None, + Some("third-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + None, + None, + Some("fourth-variant".to_string()), + Some("fourth-variant".to_string()), + Some("third-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("fifth-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("fifth-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + Some("second-variant".to_string()), + Some("fifth-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + None, + None, + Some("third-variant".to_string()), + None, + Some("second-variant".to_string()), + Some("fifth-variant".to_string()), + None, + Some("third-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("fourth-variant".to_string()), + None, + None, + Some("second-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + Some("fourth-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + None, + None, + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("third-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("third-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + None, + None, + Some("second-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("fifth-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + Some("second-variant".to_string()), + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("fourth-variant".to_string()), + Some("first-variant".to_string()), + Some("third-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("third-variant".to_string()), + None, + Some("fourth-variant".to_string()), + Some("fifth-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("second-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + None, + None, + Some("third-variant".to_string()), + Some("third-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("second-variant".to_string()), + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + None, + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("third-variant".to_string()), + None, + None, + None, + None, + Some("third-variant".to_string()), + Some("fourth-variant".to_string()), + Some("fourth-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + Some("second-variant".to_string()), + Some("first-variant".to_string()), + Some("third-variant".to_string()), + None, + Some("third-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("third-variant".to_string()), + None, + None, + None, + Some("fourth-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + Some("fourth-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("third-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("second-variant".to_string()), + Some("fourth-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + None, + None, + None, + Some("second-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + Some("third-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + Some("fifth-variant".to_string()), + Some("fourth-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + None, + Some("fourth-variant".to_string()), + None, + None, + None, + Some("fourth-variant".to_string()), + None, + None, + Some("third-variant".to_string()), + None, + None, + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("third-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + None, + None, + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("second-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + None, + Some("second-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("second-variant".to_string()), + None, + None, + Some("fifth-variant".to_string()), + Some("third-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + None, + None, + None, + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("third-variant".to_string()), + Some("third-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + None, + Some("third-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + None, + Some("fourth-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + Some("third-variant".to_string()), + None, + None, + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("second-variant".to_string()), + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("second-variant".to_string()), + Some("third-variant".to_string()), + Some("second-variant".to_string()), + Some("third-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + None, + Some("second-variant".to_string()), + None, + None, + None, + None, + Some("first-variant".to_string()), + None, + Some("third-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + None, + Some("second-variant".to_string()), + Some("third-variant".to_string()), + Some("second-variant".to_string()), + Some("fourth-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + Some("second-variant".to_string()), + None, + None, + None, + None, + None, + Some("first-variant".to_string()), + None, + None, + None, + None, + None, + Some("first-variant".to_string()), + None, + Some("second-variant".to_string()), + None, + None, + None, + None, + Some("second-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + Some("third-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + None, + Some("third-variant".to_string()), + None, + None, + Some("second-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + None, + None, + Some("second-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + None, + None, + None, + None, + Some("first-variant".to_string()), + Some("second-variant".to_string()), + None, + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("fifth-variant".to_string()), + None, + None, + None, + Some("first-variant".to_string()), + None, + Some("third-variant".to_string()), + None, + None, + Some("second-variant".to_string()), + None, + None, + None, + None, + None, + Some("fourth-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + None, + Some("second-variant".to_string()), + None, + Some("second-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("second-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + Some("fifth-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("fifth-variant".to_string()), + None, + None, + Some("third-variant".to_string()), + None, + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("third-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + None, + None, + None, + None, + Some("first-variant".to_string()), + None, + None, + None, + None, + Some("second-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("fifth-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("fourth-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("fourth-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("second-variant".to_string()), + Some("third-variant".to_string()), + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("third-variant".to_string()), + Some("third-variant".to_string()), + Some("third-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("second-variant".to_string()), + None, + None, + Some("second-variant".to_string()), + None, + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("fifth-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("fifth-variant".to_string()), + None, + None, + None, + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("fourth-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + Some("second-variant".to_string()), + Some("third-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + None, + None, + None, + None, + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("third-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + Some("third-variant".to_string()), + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("second-variant".to_string()), + None, + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + Some("second-variant".to_string()), + None, + Some("third-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("fifth-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + None, + None, + None, + Some("first-variant".to_string()), + Some("fourth-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("fifth-variant".to_string()), + None, + None, + None, + Some("second-variant".to_string()), + None, + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("second-variant".to_string()), + None, + None, + Some("third-variant".to_string()), + Some("second-variant".to_string()), + Some("third-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + Some("third-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("third-variant".to_string()), + Some("second-variant".to_string()), + Some("third-variant".to_string()), + None, + None, + Some("third-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + ]; + + for i in 0..1000 { + let distinct_id = format!("distinct_id_{}", i); + + let feature_flag_match = FeatureFlagMatcher::new(distinct_id).get_match(&flags[0]); + + if results[i].is_some() { + assert_eq!( + feature_flag_match, + FeatureFlagMatch { + matches: true, + variant: results[i].clone(), + } + ); + } else { + assert_eq!( + feature_flag_match, + FeatureFlagMatch { + matches: false, + variant: None, + } + ); + } + } +} From 99eaae4cc193dcb773b10d9d73af40872021f52e Mon Sep 17 00:00:00 2001 From: Neil Kakkar Date: Wed, 29 May 2024 18:08:38 +0100 Subject: [PATCH 10/21] format --- feature-flags/src/flag_definitions.rs | 34 +- feature-flags/src/flag_matching.rs | 34 +- feature-flags/src/lib.rs | 4 +- feature-flags/src/test_utils.rs | 29 +- .../tests/test_flag_matching_consistency.rs | 3207 ++++++----------- 5 files changed, 1197 insertions(+), 2111 deletions(-) diff --git a/feature-flags/src/flag_definitions.rs b/feature-flags/src/flag_definitions.rs index 9366573..e76bcfd 100644 --- a/feature-flags/src/flag_definitions.rs +++ b/feature-flags/src/flag_definitions.rs @@ -14,9 +14,7 @@ pub const TEAM_FLAGS_CACHE_PREFIX: &str = "posthog:1:team_feature_flags_"; // TODO: Hmm, revisit when dealing with groups, but seems like // ideal to just treat it as a u8 and do our own validation on top #[derive(Debug, Deserialize, Serialize)] -pub enum GroupTypeIndex { - -} +pub enum GroupTypeIndex {} #[derive(Debug, Clone, Deserialize, Serialize)] pub enum OperatorType { @@ -83,7 +81,6 @@ pub struct MultivariateFlagOptions { // TODO: test name with https://www.fileformat.info/info/charset/UTF-16/list.htm values, like '𝖕𝖗𝖔𝖕𝖊𝖗𝖙𝖞': `𝓿𝓪𝓵𝓾𝓮` - #[derive(Debug, Clone, Deserialize, Serialize)] pub struct FlagFilters { pub groups: Vec, @@ -118,7 +115,10 @@ impl FeatureFlag { } pub fn get_variants(&self) -> Vec { - self.filters.multivariate.clone().map_or(vec![], |m| m.variants) + self.filters + .multivariate + .clone() + .map_or(vec![], |m| m.variants) } } @@ -153,16 +153,15 @@ impl FeatureFlagList { } })?; - let flags_list: Vec = serde_json::from_str(&serialized_flags).map_err(|e| { - tracing::error!("failed to parse data to flags list: {}", e); - println!("failed to parse data: {}", e); + let flags_list: Vec = + serde_json::from_str(&serialized_flags).map_err(|e| { + tracing::error!("failed to parse data to flags list: {}", e); + println!("failed to parse data: {}", e); - FlagError::DataParsingError - })?; + FlagError::DataParsingError + })?; - Ok(FeatureFlagList { - flags: flags_list, - }) + Ok(FeatureFlagList { flags: flags_list }) } } @@ -171,7 +170,9 @@ mod tests { use rand::Rng; use super::*; - use crate::test_utils::{insert_new_team_in_redis, insert_flags_for_team_in_redis, setup_redis_client}; + use crate::test_utils::{ + insert_flags_for_team_in_redis, insert_new_team_in_redis, setup_redis_client, + }; #[tokio::test] async fn test_fetch_flags_from_redis() { @@ -179,7 +180,9 @@ mod tests { let team = insert_new_team_in_redis(client.clone()).await.unwrap(); - insert_flags_for_team_in_redis(client.clone(), team.id, None).await.expect("Failed to insert flags"); + insert_flags_for_team_in_redis(client.clone(), team.id, None) + .await + .expect("Failed to insert flags"); let flags_from_redis = FeatureFlagList::from_redis(client.clone(), team.id) .await @@ -211,5 +214,4 @@ mod tests { _ => panic!("Expected RedisUnavailable"), }; } - } diff --git a/feature-flags/src/flag_matching.rs b/feature-flags/src/flag_matching.rs index 3456783..47e2edb 100644 --- a/feature-flags/src/flag_matching.rs +++ b/feature-flags/src/flag_matching.rs @@ -20,7 +20,6 @@ pub struct FeatureFlagMatcher { const LONG_SCALE: u64 = 0xfffffffffffffff; impl FeatureFlagMatcher { - pub fn new(distinct_id: String) -> Self { FeatureFlagMatcher { // flags, @@ -29,7 +28,6 @@ impl FeatureFlagMatcher { } pub fn get_match(&self, feature_flag: &FeatureFlag) -> FeatureFlagMatch { - if self.hashed_identifier(feature_flag).is_none() { return FeatureFlagMatch { matches: false, @@ -41,21 +39,23 @@ impl FeatureFlagMatcher { // TODO: Variant overrides condition sort for (index, condition) in feature_flag.get_conditions().iter().enumerate() { - let (is_match, evaluation_reason) = self.is_condition_match(feature_flag, condition, index); + let (is_match, evaluation_reason) = + self.is_condition_match(feature_flag, condition, index); if is_match { - let variant = match condition.variant.clone() { Some(variant_override) => { - if feature_flag.get_variants().iter().any(|v| v.key == variant_override) { + if feature_flag + .get_variants() + .iter() + .any(|v| v.key == variant_override) + { Some(variant_override) } else { self.get_matching_variant(feature_flag) } } - None => { - self.get_matching_variant(feature_flag) - } + None => self.get_matching_variant(feature_flag), }; // let payload = self.get_matching_payload(is_match, variant, feature_flag); @@ -71,7 +71,12 @@ impl FeatureFlagMatcher { } } - pub fn is_condition_match(&self, feature_flag: &FeatureFlag, condition: &FlagGroupType, _index: usize) -> (bool, String) { + pub fn is_condition_match( + &self, + feature_flag: &FeatureFlag, + condition: &FlagGroupType, + _index: usize, + ) -> (bool, String) { let rollout_percentage = condition.rollout_percentage.unwrap_or(100.0); let mut condition_match = true; if condition.properties.is_some() { @@ -93,7 +98,6 @@ impl FeatureFlagMatcher { } (true, "CONDITION_MATCH".to_string()) - } pub fn hashed_identifier(&self, feature_flag: &FeatureFlag) -> Option { @@ -112,14 +116,20 @@ impl FeatureFlagMatcher { /// we can do _hash(key, identifier) < 0.2 pub fn get_hash(&self, feature_flag: &FeatureFlag, salt: &str) -> f64 { // check if hashed_identifier is None - let hashed_identifier = self.hashed_identifier(feature_flag).expect("hashed_identifier is None when computing hash"); + let hashed_identifier = self + .hashed_identifier(feature_flag) + .expect("hashed_identifier is None when computing hash"); let hash_key = format!("{}.{}{}", feature_flag.key, hashed_identifier, salt); let mut hasher = Sha1::new(); hasher.update(hash_key.as_bytes()); let result = hasher.finalize(); // :TRICKY: Convert the first 15 characters of the digest to a hexadecimal string // not sure if this is correct, padding each byte as 2 characters - let hex_str: String = result.iter().map(|byte| format!("{:02x}", byte)).collect::()[..15].to_string(); + let hex_str: String = result + .iter() + .map(|byte| format!("{:02x}", byte)) + .collect::()[..15] + .to_string(); let hash_val = u64::from_str_radix(&hex_str, 16).unwrap(); hash_val as f64 / LONG_SCALE as f64 diff --git a/feature-flags/src/lib.rs b/feature-flags/src/lib.rs index 0aaee3c..edc2a29 100644 --- a/feature-flags/src/lib.rs +++ b/feature-flags/src/lib.rs @@ -1,11 +1,11 @@ pub mod api; pub mod config; +pub mod flag_definitions; +pub mod flag_matching; pub mod redis; pub mod router; pub mod server; pub mod team; -pub mod flag_definitions; -pub mod flag_matching; pub mod v0_endpoint; pub mod v0_request; diff --git a/feature-flags/src/test_utils.rs b/feature-flags/src/test_utils.rs index a9cc08b..92bc8a4 100644 --- a/feature-flags/src/test_utils.rs +++ b/feature-flags/src/test_utils.rs @@ -3,7 +3,9 @@ use serde_json::json; use std::sync::Arc; use crate::{ - flag_definitions::{self, FeatureFlag}, redis::{Client, RedisClient}, team::{self, Team} + flag_definitions::{self, FeatureFlag}, + redis::{Client, RedisClient}, + team::{self, Team}, }; use rand::{distributions::Alphanumeric, Rng}; @@ -40,8 +42,11 @@ pub async fn insert_new_team_in_redis(client: Arc) -> Result, team_id: i64, json_value: Option) -> Result<(), Error> { - +pub async fn insert_flags_for_team_in_redis( + client: Arc, + team_id: i64, + json_value: Option, +) -> Result<(), Error> { let payload = match json_value { Some(value) => value, None => json!([{ @@ -64,16 +69,13 @@ pub async fn insert_flags_for_team_in_redis(client: Arc, team_id: i }, ], }, - }]).to_string(), + }]) + .to_string(), }; client .set( - format!( - "{}{}", - flag_definitions::TEAM_FLAGS_CACHE_PREFIX, - team_id - ), + format!("{}{}", flag_definitions::TEAM_FLAGS_CACHE_PREFIX, team_id), payload, ) .await?; @@ -91,7 +93,6 @@ pub fn setup_redis_client(url: Option) -> Arc { } pub fn create_flag_from_json(json_value: Option) -> Vec { - let payload = match json_value { Some(value) => value, None => json!([{ @@ -115,9 +116,11 @@ pub fn create_flag_from_json(json_value: Option) -> Vec { }, ], }, - }]).to_string(), + }]) + .to_string(), }; - let flags: Vec = serde_json::from_str(&payload).expect("Failed to parse data to flags list"); + let flags: Vec = + serde_json::from_str(&payload).expect("Failed to parse data to flags list"); flags -} \ No newline at end of file +} diff --git a/feature-flags/tests/test_flag_matching_consistency.rs b/feature-flags/tests/test_flag_matching_consistency.rs index 5fdffa7..57cce35 100644 --- a/feature-flags/tests/test_flag_matching_consistency.rs +++ b/feature-flags/tests/test_flag_matching_consistency.rs @@ -1,7 +1,5 @@ /// These tests are common between all libraries doing local evaluation of feature flags. /// This ensures there are no mismatches between implementations. - - use feature_flags::flag_matching::{FeatureFlagMatch, FeatureFlagMatcher}; // use feature_flags::flag_definitions::{FeatureFlag, FlagGroupType}; @@ -10,1029 +8,102 @@ use serde_json::json; #[test] fn it_is_consistent_with_rollout_calculation_for_simple_flags() { - - let flags = create_flag_from_json(Some(json!([{ - "id": 1, - "key": "simple-flag", - "name": "Simple flag", - "active": true, - "deleted": false, - "team_id": 1, - "filters": { - "groups": [ - { - "properties": [], - "rollout_percentage": 45, - }, - ], - }, - }]).to_string())); - + let flags = create_flag_from_json(Some( + json!([{ + "id": 1, + "key": "simple-flag", + "name": "Simple flag", + "active": true, + "deleted": false, + "team_id": 1, + "filters": { + "groups": [ + { + "properties": [], + "rollout_percentage": 45, + }, + ], + }, + }]) + .to_string(), + )); let results = vec![ - false, - true, - true, - false, - true, - false, - false, - true, - false, - true, - false, - true, - true, - false, - true, - false, - false, - false, - true, - true, - false, - true, - false, - false, - true, - false, - true, - true, - false, - false, - false, - true, - true, - true, - true, - false, - false, - false, - false, - false, - false, - true, - true, - false, - true, - true, - false, - false, - false, - true, - true, - false, - false, - false, - false, - true, - false, - true, - false, - true, - false, - true, - true, - false, - true, - false, - true, - false, - true, - true, - false, - false, - true, - false, - false, - true, - false, - true, - false, - false, - true, - false, - false, - false, - true, - true, - false, - true, - true, - false, - true, - true, - true, - true, - true, - false, - true, - true, - false, - false, - true, - true, - true, - true, - false, - false, - true, - false, - true, - true, - true, - false, - false, - false, - false, - false, - true, - false, - false, - true, - true, - true, - false, - false, - true, - false, - true, - false, - false, - true, - false, - false, - false, - false, - false, - false, - false, - false, - true, - true, - false, - false, - true, - false, - false, - true, - true, - false, - false, - true, - false, - true, - false, - true, - true, - true, - false, - false, - false, - true, - false, - false, - false, - false, - true, - true, - false, - true, - true, - false, - true, - false, - true, - true, - false, - true, - false, - true, - true, - true, - false, - true, - false, - false, - true, - true, - false, - true, - false, - true, - true, - false, - false, - true, - true, - true, - true, - false, - true, - true, - false, - false, - true, - false, - true, - false, - false, - true, - true, - false, - true, - false, - true, - false, - false, - false, - false, - false, - false, - false, - true, - false, - true, - true, - false, - false, - true, - false, - true, - false, - false, - false, - true, - false, - true, - false, - false, - false, - true, - false, - false, - true, - false, - true, - true, - false, - false, - false, - false, - true, - false, - false, - false, - false, - false, - false, - false, - false, - false, - false, - false, - false, - false, - true, - true, - false, - true, - false, - true, - true, - false, - true, - false, - true, - false, - false, - false, - true, - true, - true, - true, - false, - false, - false, - false, - false, - true, - true, - true, - false, - false, - true, - true, - false, - false, - false, - false, - false, - true, - false, - true, - true, - true, - true, - false, - true, - true, - true, - false, - false, - true, - false, - true, - false, - false, - true, - true, - true, - false, - true, - false, - false, - false, - true, - true, - false, - true, - false, - true, - false, - true, - true, - true, - true, - true, - false, - false, - true, - false, - true, - false, - true, - true, - true, - false, - true, - false, - true, - true, - false, - true, - true, - true, - true, - true, - false, - false, - false, - false, - false, - true, - false, - true, - false, - false, - true, - true, - false, - false, - false, - true, - false, - true, - true, - true, - true, - false, - false, - false, - false, - true, - true, - false, - false, - true, - true, - false, - true, - true, - true, - true, - false, - true, - true, - true, - false, - false, - true, - true, - false, - false, - true, - false, - false, - true, - false, - false, - false, - false, - false, - false, - false, - false, - false, - false, - true, - true, - false, - false, - true, - false, - false, - true, - false, - true, - false, - false, - true, - false, - false, - false, - false, - false, - false, - true, - false, - false, - false, - false, - false, - false, - false, - false, - false, - true, - true, - true, - false, - false, - false, - true, - false, - true, - false, - false, - false, - true, - false, - false, - false, - false, - false, - false, - false, - true, - false, - false, - false, - false, - false, - false, - false, - false, - true, - false, - true, - false, - true, - true, - true, - false, - false, - false, - true, - true, - true, - false, - true, - false, - true, - true, - false, - false, - false, - true, - false, - false, - false, - false, - true, - false, - true, - false, - true, - true, - false, - true, - false, - false, - false, - true, - false, - false, - true, - true, - false, - true, - false, - false, - false, - false, - false, - false, - true, - true, - false, - false, - true, - false, - false, - true, - true, - true, - false, - false, - false, - true, - false, - false, - false, - false, - true, - false, - true, - false, - false, - false, - true, - false, - true, - true, - false, - true, - false, - true, - false, - true, - false, - false, - true, - false, - false, - true, - false, - true, - false, - true, - false, - true, - false, - false, - true, - true, - true, - true, - false, - true, - false, - false, - false, - false, - false, - true, - false, - false, - true, - false, - false, - true, - true, - false, - false, - false, - false, - true, - true, - true, - false, - false, - true, - false, - false, - true, - true, - true, - true, - false, - false, - false, - true, - false, - false, - false, - true, - false, - false, - true, - true, - true, - true, - false, - false, - true, - true, - false, - true, - false, - true, - false, - false, - true, - true, - false, - true, - true, - true, - true, - false, - false, - true, - false, - false, - true, - true, - false, - true, - false, - true, - false, - false, - true, - false, - false, - false, - false, - true, - true, - true, - false, - true, - false, - false, - true, - false, - false, - true, - false, - false, - false, - false, - true, - false, - true, - false, - true, - true, - false, - false, - true, - false, - true, - true, - true, - false, - false, - false, - false, - true, - true, - false, - true, - false, - false, - false, - true, - false, - false, - false, - false, - true, - true, - true, - false, - false, - false, - true, - true, - true, - true, - false, - true, - true, - false, - true, - true, - true, - false, - true, - false, - false, - true, - false, - true, - true, - true, - true, - false, - true, - false, - true, - false, - true, - false, - false, - true, - true, - false, - false, - true, - false, - true, - false, - false, - false, - false, - true, - false, - true, - false, - false, - false, - true, - true, - true, - false, - false, - false, - true, - false, - true, - true, - false, - false, - false, - false, - false, - true, - false, - true, - false, - false, - true, - true, - false, - true, - true, - true, - true, - false, - false, - true, - false, - false, - true, - false, - true, - false, - true, - true, - false, - false, - false, - true, - false, - true, - true, - false, - false, - false, - true, - false, - true, - false, - true, - true, - false, - true, - false, - false, - true, - false, - false, - false, - true, - true, - true, - false, - false, - false, - false, - false, - true, - false, - false, - true, - true, - true, - true, - true, - false, - false, - false, - false, - false, - false, - false, - false, - true, - true, - true, - false, - false, - true, - true, - false, - true, - true, - false, - true, - false, - true, - false, - false, - false, - true, - false, - false, - true, - false, - false, - true, - true, - true, - true, - false, - false, - true, - false, - true, - true, - false, - false, - true, - false, - false, - true, - true, - false, - true, - false, - false, - true, - true, - true, - false, - false, - false, - false, - false, - true, - false, - true, - false, - false, - false, - false, - false, - true, - true, - false, - true, - true, - true, - false, - false, - false, - false, - true, - true, - true, - true, - false, - true, - true, - false, - true, - false, - true, - false, - true, - false, - false, - false, - false, - true, - true, - true, - true, - false, - false, - true, - false, - true, - true, - false, - false, - false, - false, - false, - false, - true, - false, - true, - false, - true, - true, - false, - false, - true, - true, - true, - true, - false, - false, - true, - false, - true, - true, - false, - false, - true, - true, - true, - false, - true, - false, - false, - true, - true, - false, - false, - false, - true, - false, - false, - true, - false, - false, - false, - true, - true, - true, - true, - false, - true, - false, - true, - false, - true, - false, - true, - false, - false, - true, - false, - false, - true, - false, - true, - true, - ]; - - + false, true, true, false, true, false, false, true, false, true, false, true, true, false, + true, false, false, false, true, true, false, true, false, false, true, false, true, true, + false, false, false, true, true, true, true, false, false, false, false, false, false, + true, true, false, true, true, false, false, false, true, true, false, false, false, false, + true, false, true, false, true, false, true, true, false, true, false, true, false, true, + true, false, false, true, false, false, true, false, true, false, false, true, false, + false, false, true, true, false, true, true, false, true, true, true, true, true, false, + true, true, false, false, true, true, true, true, false, false, true, false, true, true, + true, false, false, false, false, false, true, false, false, true, true, true, false, + false, true, false, true, false, false, true, false, false, false, false, false, false, + false, false, true, true, false, false, true, false, false, true, true, false, false, true, + false, true, false, true, true, true, false, false, false, true, false, false, false, + false, true, true, false, true, true, false, true, false, true, true, false, true, false, + true, true, true, false, true, false, false, true, true, false, true, false, true, true, + false, false, true, true, true, true, false, true, true, false, false, true, false, true, + false, false, true, true, false, true, false, true, false, false, false, false, false, + false, false, true, false, true, true, false, false, true, false, true, false, false, + false, true, false, true, false, false, false, true, false, false, true, false, true, true, + false, false, false, false, true, false, false, false, false, false, false, false, false, + false, false, false, false, false, true, true, false, true, false, true, true, false, true, + false, true, false, false, false, true, true, true, true, false, false, false, false, + false, true, true, true, false, false, true, true, false, false, false, false, false, true, + false, true, true, true, true, false, true, true, true, false, false, true, false, true, + false, false, true, true, true, false, true, false, false, false, true, true, false, true, + false, true, false, true, true, true, true, true, false, false, true, false, true, false, + true, true, true, false, true, false, true, true, false, true, true, true, true, true, + false, false, false, false, false, true, false, true, false, false, true, true, false, + false, false, true, false, true, true, true, true, false, false, false, false, true, true, + false, false, true, true, false, true, true, true, true, false, true, true, true, false, + false, true, true, false, false, true, false, false, true, false, false, false, false, + false, false, false, false, false, false, true, true, false, false, true, false, false, + true, false, true, false, false, true, false, false, false, false, false, false, true, + false, false, false, false, false, false, false, false, false, true, true, true, false, + false, false, true, false, true, false, false, false, true, false, false, false, false, + false, false, false, true, false, false, false, false, false, false, false, false, true, + false, true, false, true, true, true, false, false, false, true, true, true, false, true, + false, true, true, false, false, false, true, false, false, false, false, true, false, + true, false, true, true, false, true, false, false, false, true, false, false, true, true, + false, true, false, false, false, false, false, false, true, true, false, false, true, + false, false, true, true, true, false, false, false, true, false, false, false, false, + true, false, true, false, false, false, true, false, true, true, false, true, false, true, + false, true, false, false, true, false, false, true, false, true, false, true, false, true, + false, false, true, true, true, true, false, true, false, false, false, false, false, true, + false, false, true, false, false, true, true, false, false, false, false, true, true, true, + false, false, true, false, false, true, true, true, true, false, false, false, true, false, + false, false, true, false, false, true, true, true, true, false, false, true, true, false, + true, false, true, false, false, true, true, false, true, true, true, true, false, false, + true, false, false, true, true, false, true, false, true, false, false, true, false, false, + false, false, true, true, true, false, true, false, false, true, false, false, true, false, + false, false, false, true, false, true, false, true, true, false, false, true, false, true, + true, true, false, false, false, false, true, true, false, true, false, false, false, true, + false, false, false, false, true, true, true, false, false, false, true, true, true, true, + false, true, true, false, true, true, true, false, true, false, false, true, false, true, + true, true, true, false, true, false, true, false, true, false, false, true, true, false, + false, true, false, true, false, false, false, false, true, false, true, false, false, + false, true, true, true, false, false, false, true, false, true, true, false, false, false, + false, false, true, false, true, false, false, true, true, false, true, true, true, true, + false, false, true, false, false, true, false, true, false, true, true, false, false, + false, true, false, true, true, false, false, false, true, false, true, false, true, true, + false, true, false, false, true, false, false, false, true, true, true, false, false, + false, false, false, true, false, false, true, true, true, true, true, false, false, false, + false, false, false, false, false, true, true, true, false, false, true, true, false, true, + true, false, true, false, true, false, false, false, true, false, false, true, false, + false, true, true, true, true, false, false, true, false, true, true, false, false, true, + false, false, true, true, false, true, false, false, true, true, true, false, false, false, + false, false, true, false, true, false, false, false, false, false, true, true, false, + true, true, true, false, false, false, false, true, true, true, true, false, true, true, + false, true, false, true, false, true, false, false, false, false, true, true, true, true, + false, false, true, false, true, true, false, false, false, false, false, false, true, + false, true, false, true, true, false, false, true, true, true, true, false, false, true, + false, true, true, false, false, true, true, true, false, true, false, false, true, true, + false, false, false, true, false, false, true, false, false, false, true, true, true, true, + false, true, false, true, false, true, false, true, false, false, true, false, false, true, + false, true, true, + ]; for i in 0..1000 { let distinct_id = format!("distinct_id_{}", i); @@ -1057,1061 +128,1061 @@ fn it_is_consistent_with_rollout_calculation_for_simple_flags() { ); } } - } #[test] fn it_is_consistent_with_rollout_calculation_for_multivariate_flags() { - - let flags = create_flag_from_json(Some(json!([{ - "id": 1, - "key": "multivariate-flag", - "name": "Multivariate flag", - "active": true, - "deleted": false, - "team_id": 1, - "filters": { - "groups": [ - { - "properties": [], - "rollout_percentage": 55, - }, - ], - "multivariate": { - "variants": [ - { - "key": "first-variant", - "name": "First Variant", - "rollout_percentage": 50, - }, - { - "key": "second-variant", - "name": "Second Variant", - "rollout_percentage": 20, - }, + let flags = create_flag_from_json(Some( + json!([{ + "id": 1, + "key": "multivariate-flag", + "name": "Multivariate flag", + "active": true, + "deleted": false, + "team_id": 1, + "filters": { + "groups": [ { - "key": "third-variant", - "name": "Third Variant", - "rollout_percentage": 20, - }, - { - "key": "fourth-variant", - "name": "Fourth Variant", - "rollout_percentage": 5, - }, - { - "key": "fifth-variant", - "name": "Fifth Variant", - "rollout_percentage": 5, + "properties": [], + "rollout_percentage": 55, }, ], + "multivariate": { + "variants": [ + { + "key": "first-variant", + "name": "First Variant", + "rollout_percentage": 50, + }, + { + "key": "second-variant", + "name": "Second Variant", + "rollout_percentage": 20, + }, + { + "key": "third-variant", + "name": "Third Variant", + "rollout_percentage": 20, + }, + { + "key": "fourth-variant", + "name": "Fourth Variant", + "rollout_percentage": 5, + }, + { + "key": "fifth-variant", + "name": "Fifth Variant", + "rollout_percentage": 5, + }, + ], + }, }, - }, - }]).to_string())); - + }]) + .to_string(), + )); let results = vec![ - Some("second-variant".to_string()), - Some("second-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - Some("second-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - None, - Some("first-variant".to_string()), - Some("third-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("second-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - Some("fourth-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("third-variant".to_string()), - None, - None, - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("third-variant".to_string()), - None, - Some("third-variant".to_string()), - Some("second-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("third-variant".to_string()), - None, - None, - Some("first-variant".to_string()), - Some("second-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("second-variant".to_string()), - None, - Some("first-variant".to_string()), - None, - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("second-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("second-variant".to_string()), - Some("second-variant".to_string()), - Some("third-variant".to_string()), - Some("second-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("second-variant".to_string()), - Some("fourth-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("second-variant".to_string()), - None, - Some("third-variant".to_string()), - None, - None, - None, - None, - None, - None, - Some("first-variant".to_string()), - Some("fifth-variant".to_string()), - None, - Some("second-variant".to_string()), - Some("first-variant".to_string()), - Some("second-variant".to_string()), - None, - Some("third-variant".to_string()), - Some("third-variant".to_string()), - None, - None, - None, - None, - Some("third-variant".to_string()), - None, - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("third-variant".to_string()), - Some("third-variant".to_string()), - None, - Some("third-variant".to_string()), - Some("second-variant".to_string()), - Some("third-variant".to_string()), - None, - None, - Some("second-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - Some("first-variant".to_string()), - None, - None, - None, - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - None, - None, - None, - None, - None, - None, - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("second-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("second-variant".to_string()), - None, - Some("second-variant".to_string()), - Some("first-variant".to_string()), - Some("second-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("second-variant".to_string()), - Some("second-variant".to_string()), - None, - Some("first-variant".to_string()), - None, - None, - None, - Some("third-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - Some("first-variant".to_string()), - None, - None, - None, - None, - Some("first-variant".to_string()), - None, - None, - None, - None, - None, - None, - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("third-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - Some("first-variant".to_string()), - None, - None, - Some("fifth-variant".to_string()), - Some("second-variant".to_string()), - None, - Some("second-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("third-variant".to_string()), - Some("first-variant".to_string()), - Some("fifth-variant".to_string()), - Some("third-variant".to_string()), - None, - None, - Some("fourth-variant".to_string()), - None, - None, - None, - None, - Some("third-variant".to_string()), - None, - None, - Some("third-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("second-variant".to_string()), - Some("second-variant".to_string()), - Some("second-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("third-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - None, - None, - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("second-variant".to_string()), - None, - None, - None, - Some("second-variant".to_string()), - None, - None, - Some("first-variant".to_string()), - None, - Some("first-variant".to_string()), - None, - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("third-variant".to_string()), - Some("first-variant".to_string()), - Some("third-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("second-variant".to_string()), - Some("third-variant".to_string()), - Some("third-variant".to_string()), - None, - Some("second-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("second-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("first-variant".to_string()), - None, - None, - Some("first-variant".to_string()), - Some("fifth-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - None, - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("second-variant".to_string()), - None, - Some("second-variant".to_string()), - Some("third-variant".to_string()), - Some("third-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("third-variant".to_string()), - None, - None, - Some("first-variant".to_string()), - None, - Some("third-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("third-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("second-variant".to_string()), - Some("second-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - None, - Some("second-variant".to_string()), - None, - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("third-variant".to_string()), - None, - Some("first-variant".to_string()), - None, - Some("third-variant".to_string()), - None, - Some("third-variant".to_string()), - Some("second-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - Some("first-variant".to_string()), - Some("third-variant".to_string()), - Some("first-variant".to_string()), - Some("second-variant".to_string()), - Some("fifth-variant".to_string()), - None, - None, - Some("first-variant".to_string()), - None, - None, - None, - Some("third-variant".to_string()), - None, - Some("second-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - None, - None, - Some("third-variant".to_string()), - None, - None, - Some("third-variant".to_string()), - None, - None, - Some("first-variant".to_string()), - Some("third-variant".to_string()), - None, - None, - Some("first-variant".to_string()), - None, - None, - Some("fourth-variant".to_string()), - Some("fourth-variant".to_string()), - Some("third-variant".to_string()), - Some("second-variant".to_string()), - Some("first-variant".to_string()), - Some("third-variant".to_string()), - Some("fifth-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("fifth-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - None, - Some("second-variant".to_string()), - Some("fifth-variant".to_string()), - Some("second-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("second-variant".to_string()), - None, - None, - Some("third-variant".to_string()), - None, - Some("second-variant".to_string()), - Some("fifth-variant".to_string()), - None, - Some("third-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - Some("fourth-variant".to_string()), - None, - None, - Some("second-variant".to_string()), - None, - None, - Some("first-variant".to_string()), - Some("fourth-variant".to_string()), - Some("first-variant".to_string()), - Some("second-variant".to_string()), - None, - None, - None, - Some("first-variant".to_string()), - Some("third-variant".to_string()), - Some("third-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("first-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("third-variant".to_string()), - Some("third-variant".to_string()), - None, - None, - Some("first-variant".to_string()), - None, - None, - Some("second-variant".to_string()), - Some("second-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("fifth-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - None, - Some("second-variant".to_string()), - Some("third-variant".to_string()), - Some("first-variant".to_string()), - Some("fourth-variant".to_string()), - Some("first-variant".to_string()), - Some("third-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("third-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("third-variant".to_string()), - None, - Some("fourth-variant".to_string()), - Some("fifth-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("second-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("second-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("second-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("first-variant".to_string()), - None, - Some("first-variant".to_string()), - None, - Some("first-variant".to_string()), - None, - None, - None, - Some("third-variant".to_string()), - Some("third-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - Some("second-variant".to_string()), - Some("third-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - None, - Some("second-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("third-variant".to_string()), - None, - Some("first-variant".to_string()), - None, - None, - None, - Some("first-variant".to_string()), - Some("third-variant".to_string()), - Some("third-variant".to_string()), - None, - None, - None, - None, - Some("third-variant".to_string()), - Some("fourth-variant".to_string()), - Some("fourth-variant".to_string()), - Some("first-variant".to_string()), - Some("second-variant".to_string()), - None, - Some("first-variant".to_string()), - None, - Some("second-variant".to_string()), - Some("first-variant".to_string()), - Some("third-variant".to_string()), - None, - Some("third-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("third-variant".to_string()), - None, - None, - None, - Some("fourth-variant".to_string()), - Some("second-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - Some("first-variant".to_string()), - Some("fourth-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("third-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - Some("third-variant".to_string()), - None, - Some("first-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("third-variant".to_string()), - Some("second-variant".to_string()), - Some("fourth-variant".to_string()), - None, - Some("first-variant".to_string()), - None, - None, - None, - None, - Some("second-variant".to_string()), - Some("first-variant".to_string()), - Some("second-variant".to_string()), - None, - Some("first-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("second-variant".to_string()), - Some("third-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - None, - Some("third-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("third-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("second-variant".to_string()), - Some("first-variant".to_string()), - Some("fifth-variant".to_string()), - Some("fourth-variant".to_string()), - Some("first-variant".to_string()), - Some("second-variant".to_string()), - None, - Some("fourth-variant".to_string()), - None, - None, - None, - Some("fourth-variant".to_string()), - None, - None, - Some("third-variant".to_string()), - None, - None, - None, - Some("first-variant".to_string()), - Some("third-variant".to_string()), - Some("third-variant".to_string()), - Some("second-variant".to_string()), - Some("first-variant".to_string()), - Some("second-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("first-variant".to_string()), - None, - None, - None, - None, - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("second-variant".to_string()), - None, - None, - Some("first-variant".to_string()), - None, - Some("second-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("third-variant".to_string()), - Some("second-variant".to_string()), - None, - None, - Some("fifth-variant".to_string()), - Some("third-variant".to_string()), - None, - None, - Some("first-variant".to_string()), - None, - None, - None, - Some("first-variant".to_string()), - Some("second-variant".to_string()), - Some("third-variant".to_string()), - Some("third-variant".to_string()), - None, - None, - Some("first-variant".to_string()), - None, - Some("third-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - None, - None, - Some("fourth-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - None, - Some("third-variant".to_string()), - None, - None, - Some("second-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - Some("second-variant".to_string()), - Some("third-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - Some("second-variant".to_string()), - Some("third-variant".to_string()), - Some("second-variant".to_string()), - Some("third-variant".to_string()), - None, - None, - Some("first-variant".to_string()), - None, - None, - Some("first-variant".to_string()), - None, - Some("second-variant".to_string()), - None, - None, - None, - None, - Some("first-variant".to_string()), - None, - Some("third-variant".to_string()), - None, - Some("first-variant".to_string()), - None, - None, - Some("second-variant".to_string()), - Some("third-variant".to_string()), - Some("second-variant".to_string()), - Some("fourth-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("first-variant".to_string()), - None, - Some("second-variant".to_string()), - None, - None, - None, - None, - None, - Some("first-variant".to_string()), - None, - None, - None, - None, - None, - Some("first-variant".to_string()), - None, - Some("second-variant".to_string()), - None, - None, - None, - None, - Some("second-variant".to_string()), - None, - Some("first-variant".to_string()), - None, - Some("third-variant".to_string()), - None, - None, - Some("first-variant".to_string()), - Some("third-variant".to_string()), - None, - Some("third-variant".to_string()), - None, - None, - Some("second-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("second-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - None, - None, - None, - Some("second-variant".to_string()), - None, - None, - Some("first-variant".to_string()), - Some("third-variant".to_string()), - None, - Some("first-variant".to_string()), - None, - None, - None, - None, - None, - Some("first-variant".to_string()), - Some("second-variant".to_string()), - None, - None, - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("fifth-variant".to_string()), - None, - None, - None, - Some("first-variant".to_string()), - None, - Some("third-variant".to_string()), - None, - None, - Some("second-variant".to_string()), - None, - None, - None, - None, - None, - Some("fourth-variant".to_string()), - Some("second-variant".to_string()), - Some("first-variant".to_string()), - Some("second-variant".to_string()), - None, - Some("second-variant".to_string()), - None, - Some("second-variant".to_string()), - None, - Some("first-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("second-variant".to_string()), - None, - Some("first-variant".to_string()), - None, - Some("fifth-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - None, - Some("first-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("third-variant".to_string()), - None, - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - Some("fifth-variant".to_string()), - None, - None, - Some("third-variant".to_string()), - None, - Some("third-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("third-variant".to_string()), - Some("third-variant".to_string()), - None, - Some("first-variant".to_string()), - None, - None, - None, - None, - None, - Some("first-variant".to_string()), - None, - None, - None, - None, - Some("second-variant".to_string()), - Some("first-variant".to_string()), - Some("second-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("fifth-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - Some("fourth-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - Some("fourth-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("second-variant".to_string()), - Some("third-variant".to_string()), - Some("third-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("third-variant".to_string()), - Some("third-variant".to_string()), - Some("third-variant".to_string()), - None, - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("second-variant".to_string()), - None, - None, - Some("second-variant".to_string()), - None, - Some("third-variant".to_string()), - Some("first-variant".to_string()), - Some("second-variant".to_string()), - Some("fifth-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("fifth-variant".to_string()), - None, - None, - None, - Some("third-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("second-variant".to_string()), - Some("fourth-variant".to_string()), - Some("first-variant".to_string()), - Some("second-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - None, - Some("second-variant".to_string()), - Some("third-variant".to_string()), - None, - None, - Some("first-variant".to_string()), - None, - None, - None, - None, - None, - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("third-variant".to_string()), - None, - Some("first-variant".to_string()), - None, - Some("third-variant".to_string()), - Some("third-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("second-variant".to_string()), - None, - Some("second-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - None, - Some("second-variant".to_string()), - None, - Some("third-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("fifth-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - Some("first-variant".to_string()), - None, - None, - None, - Some("first-variant".to_string()), - Some("fourth-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("fifth-variant".to_string()), - None, - None, - None, - Some("second-variant".to_string()), - None, - None, - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("second-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("third-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("second-variant".to_string()), - None, - None, - Some("third-variant".to_string()), - Some("second-variant".to_string()), - Some("third-variant".to_string()), - None, - Some("first-variant".to_string()), - Some("third-variant".to_string()), - Some("second-variant".to_string()), - Some("first-variant".to_string()), - Some("third-variant".to_string()), - None, - None, - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - None, - None, - Some("first-variant".to_string()), - Some("third-variant".to_string()), - Some("second-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("third-variant".to_string()), - Some("second-variant".to_string()), - Some("third-variant".to_string()), - None, - None, - Some("third-variant".to_string()), - Some("first-variant".to_string()), - None, - Some("first-variant".to_string()), - ]; + Some("second-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("fourth-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("third-variant".to_string()), + None, + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("third-variant".to_string()), + None, + Some("third-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("third-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + Some("second-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("second-variant".to_string()), + Some("second-variant".to_string()), + Some("third-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("fourth-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("second-variant".to_string()), + None, + Some("third-variant".to_string()), + None, + None, + None, + None, + None, + None, + Some("first-variant".to_string()), + Some("fifth-variant".to_string()), + None, + Some("second-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + None, + Some("third-variant".to_string()), + Some("third-variant".to_string()), + None, + None, + None, + None, + Some("third-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("third-variant".to_string()), + Some("third-variant".to_string()), + None, + Some("third-variant".to_string()), + Some("second-variant".to_string()), + Some("third-variant".to_string()), + None, + None, + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + None, + None, + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + None, + None, + None, + None, + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + None, + Some("second-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("second-variant".to_string()), + Some("second-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + None, + None, + Some("third-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + None, + None, + None, + None, + Some("first-variant".to_string()), + None, + None, + None, + None, + None, + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + None, + None, + Some("fifth-variant".to_string()), + Some("second-variant".to_string()), + None, + Some("second-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("fifth-variant".to_string()), + Some("third-variant".to_string()), + None, + None, + Some("fourth-variant".to_string()), + None, + None, + None, + None, + Some("third-variant".to_string()), + None, + None, + Some("third-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("second-variant".to_string()), + Some("second-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + None, + None, + None, + Some("second-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("third-variant".to_string()), + Some("third-variant".to_string()), + None, + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + Some("fifth-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + None, + Some("second-variant".to_string()), + Some("third-variant".to_string()), + Some("third-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + None, + Some("third-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + Some("second-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("third-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + Some("third-variant".to_string()), + None, + Some("third-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("fifth-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + None, + None, + None, + Some("third-variant".to_string()), + None, + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + None, + Some("third-variant".to_string()), + None, + None, + Some("third-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + None, + None, + Some("fourth-variant".to_string()), + Some("fourth-variant".to_string()), + Some("third-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("fifth-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("fifth-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + Some("second-variant".to_string()), + Some("fifth-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + None, + None, + Some("third-variant".to_string()), + None, + Some("second-variant".to_string()), + Some("fifth-variant".to_string()), + None, + Some("third-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("fourth-variant".to_string()), + None, + None, + Some("second-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + Some("fourth-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + None, + None, + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("third-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("third-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + None, + None, + Some("second-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("fifth-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + Some("second-variant".to_string()), + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("fourth-variant".to_string()), + Some("first-variant".to_string()), + Some("third-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("third-variant".to_string()), + None, + Some("fourth-variant".to_string()), + Some("fifth-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("second-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + None, + None, + Some("third-variant".to_string()), + Some("third-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("second-variant".to_string()), + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + None, + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("third-variant".to_string()), + None, + None, + None, + None, + Some("third-variant".to_string()), + Some("fourth-variant".to_string()), + Some("fourth-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + Some("second-variant".to_string()), + Some("first-variant".to_string()), + Some("third-variant".to_string()), + None, + Some("third-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("third-variant".to_string()), + None, + None, + None, + Some("fourth-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + Some("fourth-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("third-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("second-variant".to_string()), + Some("fourth-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + None, + None, + None, + Some("second-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + Some("third-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + Some("fifth-variant".to_string()), + Some("fourth-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + None, + Some("fourth-variant".to_string()), + None, + None, + None, + Some("fourth-variant".to_string()), + None, + None, + Some("third-variant".to_string()), + None, + None, + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("third-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + None, + None, + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("second-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + None, + Some("second-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("second-variant".to_string()), + None, + None, + Some("fifth-variant".to_string()), + Some("third-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + None, + None, + None, + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("third-variant".to_string()), + Some("third-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + None, + Some("third-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + None, + Some("fourth-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + Some("third-variant".to_string()), + None, + None, + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("second-variant".to_string()), + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("second-variant".to_string()), + Some("third-variant".to_string()), + Some("second-variant".to_string()), + Some("third-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + None, + Some("second-variant".to_string()), + None, + None, + None, + None, + Some("first-variant".to_string()), + None, + Some("third-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + None, + Some("second-variant".to_string()), + Some("third-variant".to_string()), + Some("second-variant".to_string()), + Some("fourth-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + Some("second-variant".to_string()), + None, + None, + None, + None, + None, + Some("first-variant".to_string()), + None, + None, + None, + None, + None, + Some("first-variant".to_string()), + None, + Some("second-variant".to_string()), + None, + None, + None, + None, + Some("second-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + Some("third-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + None, + Some("third-variant".to_string()), + None, + None, + Some("second-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + None, + None, + Some("second-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + None, + None, + None, + None, + Some("first-variant".to_string()), + Some("second-variant".to_string()), + None, + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("fifth-variant".to_string()), + None, + None, + None, + Some("first-variant".to_string()), + None, + Some("third-variant".to_string()), + None, + None, + Some("second-variant".to_string()), + None, + None, + None, + None, + None, + Some("fourth-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + None, + Some("second-variant".to_string()), + None, + Some("second-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("second-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + Some("fifth-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("fifth-variant".to_string()), + None, + None, + Some("third-variant".to_string()), + None, + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("third-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + None, + None, + None, + None, + Some("first-variant".to_string()), + None, + None, + None, + None, + Some("second-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("fifth-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("fourth-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("fourth-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("second-variant".to_string()), + Some("third-variant".to_string()), + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("third-variant".to_string()), + Some("third-variant".to_string()), + Some("third-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("second-variant".to_string()), + None, + None, + Some("second-variant".to_string()), + None, + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("fifth-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("fifth-variant".to_string()), + None, + None, + None, + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("fourth-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + Some("second-variant".to_string()), + Some("third-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + None, + None, + None, + None, + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("third-variant".to_string()), + None, + Some("first-variant".to_string()), + None, + Some("third-variant".to_string()), + Some("third-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("second-variant".to_string()), + None, + Some("second-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + Some("second-variant".to_string()), + None, + Some("third-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("fifth-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + None, + None, + None, + Some("first-variant".to_string()), + Some("fourth-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("fifth-variant".to_string()), + None, + None, + None, + Some("second-variant".to_string()), + None, + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("second-variant".to_string()), + None, + None, + Some("third-variant".to_string()), + Some("second-variant".to_string()), + Some("third-variant".to_string()), + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + Some("third-variant".to_string()), + None, + None, + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + None, + None, + Some("first-variant".to_string()), + Some("third-variant".to_string()), + Some("second-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("third-variant".to_string()), + Some("second-variant".to_string()), + Some("third-variant".to_string()), + None, + None, + Some("third-variant".to_string()), + Some("first-variant".to_string()), + None, + Some("first-variant".to_string()), + ]; for i in 0..1000 { let distinct_id = format!("distinct_id_{}", i); From 238c3c3b3cc7dcef238f49455f13463a94bdf6bf Mon Sep 17 00:00:00 2001 From: Neil Kakkar Date: Thu, 30 May 2024 14:21:11 +0100 Subject: [PATCH 11/21] some comments for future fixes --- feature-flags/src/flag_definitions.rs | 7 +++---- feature-flags/src/flag_matching.rs | 15 +++++++++++++-- feature-flags/src/team.rs | 1 + 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/feature-flags/src/flag_definitions.rs b/feature-flags/src/flag_definitions.rs index e76bcfd..ca5b26f 100644 --- a/feature-flags/src/flag_definitions.rs +++ b/feature-flags/src/flag_definitions.rs @@ -129,8 +129,7 @@ pub struct FeatureFlagList { } impl FeatureFlagList { - /// Returns feature flags given a team_id - + /// Returns feature flags from redis given a team_id #[instrument(skip_all)] pub async fn from_redis( client: Arc, @@ -143,6 +142,8 @@ impl FeatureFlagList { .map_err(|e| match e { CustomRedisError::NotFound => FlagError::TokenValidationError, CustomRedisError::PickleError(_) => { + // TODO: Implement From trait for FlagError so we don't need to map + // CustomRedisError ourselves tracing::error!("failed to fetch data: {}", e); println!("failed to fetch data: {}", e); FlagError::DataParsingError @@ -167,8 +168,6 @@ impl FeatureFlagList { #[cfg(test)] mod tests { - use rand::Rng; - use super::*; use crate::test_utils::{ insert_flags_for_team_in_redis, insert_new_team_in_redis, setup_redis_client, diff --git a/feature-flags/src/flag_matching.rs b/feature-flags/src/flag_matching.rs index 47e2edb..d0693fa 100644 --- a/feature-flags/src/flag_matching.rs +++ b/feature-flags/src/flag_matching.rs @@ -11,6 +11,16 @@ pub struct FeatureFlagMatch { //payload } +// TODO: Rework FeatureFlagMatcher - python has a pretty awkward interface, where we pass in all flags, and then again +// the flag to match. I don't think there's any reason anymore to store the flags in the matcher, since we can just +// pass the flag to match directly to the get_match method. This will also make the matcher more stateless. +// Potentially, we could also make the matcher a long-lived object, with caching for group keys and such. +// It just takes in the flag and distinct_id and returns the match... +// Or, make this fully stateless +// and have a separate cache struct for caching group keys, cohort definitions, etc. - and check size, if we can keep it in memory +// for all teams. If not, we can have a LRU cache, or a cache that stores only the most recent N keys. +// But, this can be a future refactor, for now just focusing on getting the basic matcher working, write lots and lots of tests +// and then we can easily refactor stuff around. #[derive(Debug)] pub struct FeatureFlagMatcher { // pub flags: Vec, @@ -35,14 +45,15 @@ impl FeatureFlagMatcher { }; } - // TODO: super groups + // TODO: super groups for early access // TODO: Variant overrides condition sort for (index, condition) in feature_flag.get_conditions().iter().enumerate() { - let (is_match, evaluation_reason) = + let (is_match, _evaluation_reason) = self.is_condition_match(feature_flag, condition, index); if is_match { + // TODO: This is a bit awkward, we should handle overrides only when variants exist. let variant = match condition.variant.clone() { Some(variant_override) => { if feature_flag diff --git a/feature-flags/src/team.rs b/feature-flags/src/team.rs index ac62ea9..e872aa4 100644 --- a/feature-flags/src/team.rs +++ b/feature-flags/src/team.rs @@ -42,6 +42,7 @@ impl Team { } })?; + // TODO: Consider an LRU cache for teams as well, with small TTL to skip redis/pg lookups let team: Team = serde_json::from_str(&serialized_team).map_err(|e| { tracing::error!("failed to parse data to team: {}", e); FlagError::DataParsingError From 313784bda6a9778f9abe03e1cdd2bbbc3704f409 Mon Sep 17 00:00:00 2001 From: Neil Kakkar Date: Thu, 30 May 2024 17:30:18 +0100 Subject: [PATCH 12/21] feat(flags): Add basic property matching --- Cargo.lock | 5 +- feature-flags/Cargo.toml | 1 + feature-flags/README.md | 18 ++ feature-flags/src/flag_definitions.rs | 2 +- feature-flags/src/lib.rs | 1 + feature-flags/src/property_matching.rs | 275 +++++++++++++++++++++++++ 6 files changed, 299 insertions(+), 3 deletions(-) create mode 100644 feature-flags/README.md create mode 100644 feature-flags/src/property_matching.rs diff --git a/Cargo.lock b/Cargo.lock index 162877b..919b61a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -706,6 +706,7 @@ dependencies = [ "once_cell", "rand", "redis", + "regex", "reqwest 0.12.3", "serde", "serde-pickle", @@ -2251,9 +2252,9 @@ dependencies = [ [[package]] name = "regex" -version = "1.10.3" +version = "1.10.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b62dbe01f0b06f9d8dc7d49e05a0785f153b00b2c227856282f671e0318c9b15" +checksum = "c117dbdfde9c8308975b6a18d71f3f385c89461f7b3fb054288ecf2a2058ba4c" dependencies = [ "aho-corasick", "memchr", diff --git a/feature-flags/Cargo.toml b/feature-flags/Cargo.toml index 85dd0b9..de0d8cf 100644 --- a/feature-flags/Cargo.toml +++ b/feature-flags/Cargo.toml @@ -27,6 +27,7 @@ thiserror = { workspace = true } serde-pickle = { version = "1.1.1"} hex = "0.4.3" sha1 = "0.10.6" +regex = "1.10.4" [lints] workspace = true diff --git a/feature-flags/README.md b/feature-flags/README.md new file mode 100644 index 0000000..458c519 --- /dev/null +++ b/feature-flags/README.md @@ -0,0 +1,18 @@ + +# Testing + +``` +cargo test --package feature-flags +``` + +# Running + +``` +RUST_LOG=debug cargo run --bin feature-flags +``` + +# Format code + +``` +cargo fmt --package feature-flags +``` \ No newline at end of file diff --git a/feature-flags/src/flag_definitions.rs b/feature-flags/src/flag_definitions.rs index ca5b26f..89c32d6 100644 --- a/feature-flags/src/flag_definitions.rs +++ b/feature-flags/src/flag_definitions.rs @@ -16,7 +16,7 @@ pub const TEAM_FLAGS_CACHE_PREFIX: &str = "posthog:1:team_feature_flags_"; #[derive(Debug, Deserialize, Serialize)] pub enum GroupTypeIndex {} -#[derive(Debug, Clone, Deserialize, Serialize)] +#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] pub enum OperatorType { #[serde(rename = "exact")] Exact, diff --git a/feature-flags/src/lib.rs b/feature-flags/src/lib.rs index edc2a29..7f03747 100644 --- a/feature-flags/src/lib.rs +++ b/feature-flags/src/lib.rs @@ -2,6 +2,7 @@ pub mod api; pub mod config; pub mod flag_definitions; pub mod flag_matching; +pub mod property_matching; pub mod redis; pub mod router; pub mod server; diff --git a/feature-flags/src/property_matching.rs b/feature-flags/src/property_matching.rs new file mode 100644 index 0000000..cb0972a --- /dev/null +++ b/feature-flags/src/property_matching.rs @@ -0,0 +1,275 @@ + +use std::collections::HashMap; + +use serde_json::Value; +use regex::Regex; +use crate::flag_definitions::{OperatorType, PropertyFilter}; + +#[derive(Debug, PartialEq, Eq)] +pub enum FlagMatchingError { + ValidationError(String), + MissingProperty(String), + InconclusiveOperatorMatch, + InvalidRegexPattern, +} + +pub fn match_property(property: &PropertyFilter, matching_property_values: &HashMap, partial_props: bool) -> Result { + // only looks for matches where key exists in override_property_values + // doesn't support operator is_not_set with partial_props + + if partial_props { + if !matching_property_values.contains_key(&property.key) { + return Err(FlagMatchingError::MissingProperty(format!("can't match properties without a value. Missing property: {}", property.key))); + } + } + + let key = &property.key; + let operator = property.operator.clone().unwrap_or(OperatorType::Exact); + let value = &property.value; + let match_value = matching_property_values.get(key); + + match operator { + OperatorType::Exact | OperatorType::IsNot => { + let compute_exact_match = |value: &Value, override_value: &Value| -> bool { + if is_truthy_or_falsy_property_value(value) { + // Do boolean handling, such that passing in "true" or "True" or "false" or "False" as matching value is equivalent + let truthy = is_truthy_property_value(value); + return override_value.to_string().to_lowercase() == truthy.to_string().to_lowercase(); + } + + if value.is_array() { + // TODO: Check if `to_string()` coerces all types to string correctly. + return value.as_array().unwrap().iter().map(|v| v.to_string().to_lowercase()).collect::>().contains(&override_value.to_string().to_lowercase()); + } + return value.to_string().to_lowercase() == override_value.to_string().to_lowercase(); + }; + + if let Some(match_value) = match_value { + if operator == OperatorType::Exact { + Ok(compute_exact_match(value, match_value)) + } else { + Ok(!compute_exact_match(value, match_value)) + } + } else { + return Ok(false); + } + }, + OperatorType::IsSet => { + Ok(matching_property_values.contains_key(key)) + }, + OperatorType::IsNotSet => { + if partial_props { + if matching_property_values.contains_key(key) { + Ok(false) + } else { + Err(FlagMatchingError::InconclusiveOperatorMatch) + } + } else { + Ok(!matching_property_values.contains_key(key)) + } + }, + OperatorType::Icontains | OperatorType::NotIcontains => { + if let Some(match_value) = match_value { + let is_contained = match_value.to_string().to_lowercase().contains(&value.to_string().to_lowercase()); + if operator == OperatorType::Icontains { + Ok(is_contained) + } else { + Ok(!is_contained) + } + } else { + // When value doesn't exist, it's not a match + Ok(false) + } + }, + OperatorType::Regex | OperatorType::NotRegex => { + + if match_value.is_none() { + return Ok(false); + } + + let pattern = match Regex::new(&value.to_string()) { + Ok(pattern) => pattern, + Err(_) => return Err(FlagMatchingError::InvalidRegexPattern), + }; + let haystack = match_value.unwrap_or(&Value::Null).to_string(); + let match_ = pattern.find(&haystack); + + if operator == OperatorType::Regex { + Ok(match_.is_some()) + } else { + Ok(match_.is_none()) + } + }, + OperatorType::Gt | OperatorType::Gte | OperatorType::Lt | OperatorType::Lte => { + + if match_value.is_none() { + return Ok(false); + } + // TODO: Move towards only numeric matching of these operators??? + + let compare = |lhs: f64, rhs: f64, operator: OperatorType| -> bool { + match operator { + OperatorType::Gt => lhs > rhs, + OperatorType::Gte => lhs >= rhs, + OperatorType::Lt => lhs < rhs, + OperatorType::Lte => lhs <= rhs, + _ => false, + } + }; + + let parsed_value = match match_value.unwrap_or(&Value::Null).as_f64() { + Some(parsed_value) => parsed_value, + None => return Err(FlagMatchingError::ValidationError("value is not a number".to_string())), + }; + + if let Some(override_value) = value.as_f64() { + Ok(compare(override_value, parsed_value, operator)) + } else { + return Err(FlagMatchingError::ValidationError("override value is not a number".to_string())); + } + }, + OperatorType::IsDateExact | OperatorType::IsDateAfter | OperatorType::IsDateBefore => { + // TODO: Handle date operators + return Ok(false); + // let parsed_date = determine_parsed_date_for_property_matching(match_value); + + // if parsed_date.is_none() { + // return Ok(false); + // } + + // if let Some(override_value) = value.as_str() { + // let override_date = match parser::parse(override_value) { + // Ok(override_date) => override_date, + // Err(_) => return Ok(false), + // }; + + // match operator { + // OperatorType::IsDateBefore => Ok(override_date < parsed_date.unwrap()), + // OperatorType::IsDateAfter => Ok(override_date > parsed_date.unwrap()), + // _ => Ok(false), + // } + // } else { + // Ok(false) + // } + }, + } + +} + +fn is_truthy_or_falsy_property_value(value: &Value) -> bool { + if value.is_boolean() { + return true; + } + + if value.is_string() { + let parsed_value = value.as_str().unwrap().to_lowercase(); + return parsed_value == "true" || parsed_value == "false"; + } + + if value.is_array() { + return value.as_array().unwrap().iter().all(|v| is_truthy_or_falsy_property_value(v)); + } + + false +} + +fn is_truthy_property_value(value: &Value) -> bool { + if value.is_boolean() { + return value.as_bool().unwrap(); + } + + if value.is_string() { + let parsed_value = value.as_str().unwrap().to_lowercase(); + return parsed_value == "true"; + } + + if value.is_array() { + return value.as_array().unwrap().iter().all(|v| is_truthy_property_value(v)); + } + + false +} + +// def test_match_properties_exact(self): +// property_a = Property(key="key", value="value") + +// self.assertTrue(match_property(property_a, {"key": "value"})) + +// self.assertFalse(match_property(property_a, {"key": "value2"})) +// self.assertFalse(match_property(property_a, {"key": ""})) +// self.assertFalse(match_property(property_a, {"key": None})) + +// with self.assertRaises(ValidationError): +// match_property(property_a, {"key2": "value"}) +// match_property(property_a, {}) + +// &property_b = Property(key="key", value="value", operator="exact") +// self.assertTrue(match_property(&property_b, {"key": "value"})) + +// self.assertFalse(match_property(&property_b, {"key": "value2"})) + +// &property_c = Property(key="key", value=["value1", "value2", "value3"], operator="exact") +// self.assertTrue(match_property(&property_c, {"key": "value1"})) +// self.assertTrue(match_property(&property_c, {"key": "value2"})) +// self.assertTrue(match_property(&property_c, {"key": "value3"})) + +// self.assertFalse(match_property(&property_c, {"key": "value4"})) + +// with self.assertRaises(ValidationError): +// match_property(&property_c, {"key2": "value"}) + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + #[test] + fn test_match_properties_exact_with_partial_props() { + let property_a = PropertyFilter { + key: "key".to_string(), + value: json!("value"), + operator: None, + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert_eq!(match_property(&property_a, &[("key".to_string(), json!("value"))].iter().cloned().collect(), true).unwrap(), true); + + assert_eq!(match_property(&property_a, &[("key".to_string(), json!("value2"))].iter().cloned().collect(), true).unwrap(), false); + assert_eq!(match_property(&property_a, &[("key".to_string(), json!(""))].iter().cloned().collect(), true).unwrap(), false); + assert_eq!(match_property(&property_a, &[("key".to_string(), json!(null))].iter().cloned().collect(), true).unwrap(), false); + + assert_eq!(match_property(&property_a, &[("key2".to_string(), json!("value"))].iter().cloned().collect(), true).is_err(), true); + assert_eq!(match_property(&property_a, &[("key2".to_string(), json!("value"))].iter().cloned().collect(), true).err().unwrap(), FlagMatchingError::MissingProperty("can't match properties without a value. Missing property: key".to_string())); + assert_eq!(match_property(&property_a, &[].iter().cloned().collect(), true).is_err(), true); + + let property_b = PropertyFilter { + key: "key".to_string(), + value: json!("value"), + operator: Some(OperatorType::Exact), + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert_eq!(match_property(&property_b, &[("key".to_string(), json!("value"))].iter().cloned().collect(), true).unwrap(), true); + + assert_eq!(match_property(&property_b, &[("key".to_string(), json!("value2"))].iter().cloned().collect(), true).unwrap(), false); + + let property_c = PropertyFilter { + key: "key".to_string(), + value: json!(["value1", "value2", "value3"]), + operator: Some(OperatorType::Exact), + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert_eq!(match_property(&property_c, &[("key".to_string(), json!("value1"))].iter().cloned().collect(), true).unwrap(), true); + assert_eq!(match_property(&property_c, &[("key".to_string(), json!("value2"))].iter().cloned().collect(), true).unwrap(), true); + assert_eq!(match_property(&property_c, &[("key".to_string(), json!("value3"))].iter().cloned().collect(), true).unwrap(), true); + + assert_eq!(match_property(&property_c, &[("key".to_string(), json!("value4"))].iter().cloned().collect(), true).unwrap(), false); + + assert_eq!(match_property(&property_c, &[("key2".to_string(), json!("value"))].iter().cloned().collect(), true).is_err(), true); + } +} + From cdcac3010aa53e68127a5ea5bba314aa4511bfaa Mon Sep 17 00:00:00 2001 From: Neil Kakkar Date: Thu, 30 May 2024 17:39:16 +0100 Subject: [PATCH 13/21] clean up --- feature-flags/src/property_matching.rs | 253 ++++++++++++++++++------- 1 file changed, 180 insertions(+), 73 deletions(-) diff --git a/feature-flags/src/property_matching.rs b/feature-flags/src/property_matching.rs index cb0972a..61db1c2 100644 --- a/feature-flags/src/property_matching.rs +++ b/feature-flags/src/property_matching.rs @@ -1,9 +1,8 @@ - use std::collections::HashMap; -use serde_json::Value; -use regex::Regex; use crate::flag_definitions::{OperatorType, PropertyFilter}; +use regex::Regex; +use serde_json::Value; #[derive(Debug, PartialEq, Eq)] pub enum FlagMatchingError { @@ -13,13 +12,20 @@ pub enum FlagMatchingError { InvalidRegexPattern, } -pub fn match_property(property: &PropertyFilter, matching_property_values: &HashMap, partial_props: bool) -> Result { +pub fn match_property( + property: &PropertyFilter, + matching_property_values: &HashMap, + partial_props: bool, +) -> Result { // only looks for matches where key exists in override_property_values // doesn't support operator is_not_set with partial_props if partial_props { if !matching_property_values.contains_key(&property.key) { - return Err(FlagMatchingError::MissingProperty(format!("can't match properties without a value. Missing property: {}", property.key))); + return Err(FlagMatchingError::MissingProperty(format!( + "can't match properties without a value. Missing property: {}", + property.key + ))); } } @@ -34,14 +40,22 @@ pub fn match_property(property: &PropertyFilter, matching_property_values: &Hash if is_truthy_or_falsy_property_value(value) { // Do boolean handling, such that passing in "true" or "True" or "false" or "False" as matching value is equivalent let truthy = is_truthy_property_value(value); - return override_value.to_string().to_lowercase() == truthy.to_string().to_lowercase(); + return override_value.to_string().to_lowercase() + == truthy.to_string().to_lowercase(); } if value.is_array() { // TODO: Check if `to_string()` coerces all types to string correctly. - return value.as_array().unwrap().iter().map(|v| v.to_string().to_lowercase()).collect::>().contains(&override_value.to_string().to_lowercase()); + return value + .as_array() + .unwrap() + .iter() + .map(|v| v.to_string().to_lowercase()) + .collect::>() + .contains(&override_value.to_string().to_lowercase()); } - return value.to_string().to_lowercase() == override_value.to_string().to_lowercase(); + return value.to_string().to_lowercase() + == override_value.to_string().to_lowercase(); }; if let Some(match_value) = match_value { @@ -53,10 +67,8 @@ pub fn match_property(property: &PropertyFilter, matching_property_values: &Hash } else { return Ok(false); } - }, - OperatorType::IsSet => { - Ok(matching_property_values.contains_key(key)) - }, + } + OperatorType::IsSet => Ok(matching_property_values.contains_key(key)), OperatorType::IsNotSet => { if partial_props { if matching_property_values.contains_key(key) { @@ -67,10 +79,13 @@ pub fn match_property(property: &PropertyFilter, matching_property_values: &Hash } else { Ok(!matching_property_values.contains_key(key)) } - }, + } OperatorType::Icontains | OperatorType::NotIcontains => { if let Some(match_value) = match_value { - let is_contained = match_value.to_string().to_lowercase().contains(&value.to_string().to_lowercase()); + let is_contained = match_value + .to_string() + .to_lowercase() + .contains(&value.to_string().to_lowercase()); if operator == OperatorType::Icontains { Ok(is_contained) } else { @@ -80,9 +95,8 @@ pub fn match_property(property: &PropertyFilter, matching_property_values: &Hash // When value doesn't exist, it's not a match Ok(false) } - }, + } OperatorType::Regex | OperatorType::NotRegex => { - if match_value.is_none() { return Ok(false); } @@ -99,9 +113,8 @@ pub fn match_property(property: &PropertyFilter, matching_property_values: &Hash } else { Ok(match_.is_none()) } - }, + } OperatorType::Gt | OperatorType::Gte | OperatorType::Lt | OperatorType::Lte => { - if match_value.is_none() { return Ok(false); } @@ -119,15 +132,21 @@ pub fn match_property(property: &PropertyFilter, matching_property_values: &Hash let parsed_value = match match_value.unwrap_or(&Value::Null).as_f64() { Some(parsed_value) => parsed_value, - None => return Err(FlagMatchingError::ValidationError("value is not a number".to_string())), + None => { + return Err(FlagMatchingError::ValidationError( + "value is not a number".to_string(), + )) + } }; if let Some(override_value) = value.as_f64() { Ok(compare(override_value, parsed_value, operator)) } else { - return Err(FlagMatchingError::ValidationError("override value is not a number".to_string())); + return Err(FlagMatchingError::ValidationError( + "override value is not a number".to_string(), + )); } - }, + } OperatorType::IsDateExact | OperatorType::IsDateAfter | OperatorType::IsDateBefore => { // TODO: Handle date operators return Ok(false); @@ -151,9 +170,8 @@ pub fn match_property(property: &PropertyFilter, matching_property_values: &Hash // } else { // Ok(false) // } - }, + } } - } fn is_truthy_or_falsy_property_value(value: &Value) -> bool { @@ -167,7 +185,11 @@ fn is_truthy_or_falsy_property_value(value: &Value) -> bool { } if value.is_array() { - return value.as_array().unwrap().iter().all(|v| is_truthy_or_falsy_property_value(v)); + return value + .as_array() + .unwrap() + .iter() + .all(|v| is_truthy_or_falsy_property_value(v)); } false @@ -184,40 +206,16 @@ fn is_truthy_property_value(value: &Value) -> bool { } if value.is_array() { - return value.as_array().unwrap().iter().all(|v| is_truthy_property_value(v)); + return value + .as_array() + .unwrap() + .iter() + .all(|v| is_truthy_property_value(v)); } false } -// def test_match_properties_exact(self): -// property_a = Property(key="key", value="value") - -// self.assertTrue(match_property(property_a, {"key": "value"})) - -// self.assertFalse(match_property(property_a, {"key": "value2"})) -// self.assertFalse(match_property(property_a, {"key": ""})) -// self.assertFalse(match_property(property_a, {"key": None})) - -// with self.assertRaises(ValidationError): -// match_property(property_a, {"key2": "value"}) -// match_property(property_a, {}) - -// &property_b = Property(key="key", value="value", operator="exact") -// self.assertTrue(match_property(&property_b, {"key": "value"})) - -// self.assertFalse(match_property(&property_b, {"key": "value2"})) - -// &property_c = Property(key="key", value=["value1", "value2", "value3"], operator="exact") -// self.assertTrue(match_property(&property_c, {"key": "value1"})) -// self.assertTrue(match_property(&property_c, {"key": "value2"})) -// self.assertTrue(match_property(&property_c, {"key": "value3"})) - -// self.assertFalse(match_property(&property_c, {"key": "value4"})) - -// with self.assertRaises(ValidationError): -// match_property(&property_c, {"key2": "value"}) - #[cfg(test)] mod tests { use super::*; @@ -233,16 +231,70 @@ mod tests { group_type_index: None, }; - assert_eq!(match_property(&property_a, &[("key".to_string(), json!("value"))].iter().cloned().collect(), true).unwrap(), true); + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key".to_string(), json!("value"))]), + true + ) + .expect("expected match to exist"), + true + ); + + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key".to_string(), json!("value2"))]), + true + ) + .expect("expected match to exist"), + false + ); + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key".to_string(), json!(""))]), + true + ) + .expect("expected match to exist"), + false + ); + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key".to_string(), json!(null))]), + true + ) + .expect("expected match to exist"), + false + ); + + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key2".to_string(), json!("value"))]), + true + ) + .is_err(), + true + ); + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key2".to_string(), json!("value"))]), + true + ) + .err() + .expect("expected match to exist"), + FlagMatchingError::MissingProperty( + "can't match properties without a value. Missing property: key".to_string() + ) + ); + assert_eq!( + match_property(&property_a, &HashMap::from([]), true).is_err(), + true + ); - assert_eq!(match_property(&property_a, &[("key".to_string(), json!("value2"))].iter().cloned().collect(), true).unwrap(), false); - assert_eq!(match_property(&property_a, &[("key".to_string(), json!(""))].iter().cloned().collect(), true).unwrap(), false); - assert_eq!(match_property(&property_a, &[("key".to_string(), json!(null))].iter().cloned().collect(), true).unwrap(), false); - - assert_eq!(match_property(&property_a, &[("key2".to_string(), json!("value"))].iter().cloned().collect(), true).is_err(), true); - assert_eq!(match_property(&property_a, &[("key2".to_string(), json!("value"))].iter().cloned().collect(), true).err().unwrap(), FlagMatchingError::MissingProperty("can't match properties without a value. Missing property: key".to_string())); - assert_eq!(match_property(&property_a, &[].iter().cloned().collect(), true).is_err(), true); - let property_b = PropertyFilter { key: "key".to_string(), value: json!("value"), @@ -251,9 +303,25 @@ mod tests { group_type_index: None, }; - assert_eq!(match_property(&property_b, &[("key".to_string(), json!("value"))].iter().cloned().collect(), true).unwrap(), true); - - assert_eq!(match_property(&property_b, &[("key".to_string(), json!("value2"))].iter().cloned().collect(), true).unwrap(), false); + assert_eq!( + match_property( + &property_b, + &HashMap::from([("key".to_string(), json!("value"))]), + true + ) + .expect("expected match to exist"), + true + ); + + assert_eq!( + match_property( + &property_b, + &HashMap::from([("key".to_string(), json!("value2"))]), + true + ) + .expect("expected match to exist"), + false + ); let property_c = PropertyFilter { key: "key".to_string(), @@ -263,13 +331,52 @@ mod tests { group_type_index: None, }; - assert_eq!(match_property(&property_c, &[("key".to_string(), json!("value1"))].iter().cloned().collect(), true).unwrap(), true); - assert_eq!(match_property(&property_c, &[("key".to_string(), json!("value2"))].iter().cloned().collect(), true).unwrap(), true); - assert_eq!(match_property(&property_c, &[("key".to_string(), json!("value3"))].iter().cloned().collect(), true).unwrap(), true); - - assert_eq!(match_property(&property_c, &[("key".to_string(), json!("value4"))].iter().cloned().collect(), true).unwrap(), false); - - assert_eq!(match_property(&property_c, &[("key2".to_string(), json!("value"))].iter().cloned().collect(), true).is_err(), true); + assert_eq!( + match_property( + &property_c, + &HashMap::from([("key".to_string(), json!("value1"))]), + true + ) + .expect("expected match to exist"), + true + ); + assert_eq!( + match_property( + &property_c, + &HashMap::from([("key".to_string(), json!("value2"))]), + true + ) + .expect("expected match to exist"), + true + ); + assert_eq!( + match_property( + &property_c, + &HashMap::from([("key".to_string(), json!("value3"))]), + true + ) + .expect("expected match to exist"), + true + ); + + assert_eq!( + match_property( + &property_c, + &HashMap::from([("key".to_string(), json!("value4"))]), + true + ) + .expect("expected match to exist"), + false + ); + + assert_eq!( + match_property( + &property_c, + &HashMap::from([("key2".to_string(), json!("value"))]), + true + ) + .is_err(), + true + ); } } - From 77a0c7e2fe40b9e8cc8251aa72ca856ebf3073cf Mon Sep 17 00:00:00 2001 From: Neil Kakkar Date: Thu, 30 May 2024 17:42:14 +0100 Subject: [PATCH 14/21] clean up --- feature-flags/src/property_matching.rs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/feature-flags/src/property_matching.rs b/feature-flags/src/property_matching.rs index 61db1c2..c8026d3 100644 --- a/feature-flags/src/property_matching.rs +++ b/feature-flags/src/property_matching.rs @@ -48,7 +48,7 @@ pub fn match_property( // TODO: Check if `to_string()` coerces all types to string correctly. return value .as_array() - .unwrap() + .expect("expected array value") .iter() .map(|v| v.to_string().to_lowercase()) .collect::>() @@ -180,14 +180,17 @@ fn is_truthy_or_falsy_property_value(value: &Value) -> bool { } if value.is_string() { - let parsed_value = value.as_str().unwrap().to_lowercase(); + let parsed_value = value + .as_str() + .expect("expected string value") + .to_lowercase(); return parsed_value == "true" || parsed_value == "false"; } if value.is_array() { return value .as_array() - .unwrap() + .expect("expected array value") .iter() .all(|v| is_truthy_or_falsy_property_value(v)); } @@ -197,18 +200,21 @@ fn is_truthy_or_falsy_property_value(value: &Value) -> bool { fn is_truthy_property_value(value: &Value) -> bool { if value.is_boolean() { - return value.as_bool().unwrap(); + return value.as_bool().expect("expected boolean value"); } if value.is_string() { - let parsed_value = value.as_str().unwrap().to_lowercase(); + let parsed_value = value + .as_str() + .expect("expected string value") + .to_lowercase(); return parsed_value == "true"; } if value.is_array() { return value .as_array() - .unwrap() + .expect("expected array value") .iter() .all(|v| is_truthy_property_value(v)); } From 0a44930a4560dd9de5ece1b75e33c54296f73fc5 Mon Sep 17 00:00:00 2001 From: Neil Kakkar Date: Tue, 4 Jun 2024 13:47:50 +0100 Subject: [PATCH 15/21] add tests for all match operators --- Cargo.lock | 4 +- feature-flags/README.md | 18 + feature-flags/src/flag_definitions.rs | 3 + feature-flags/src/property_matching.rs | 1258 +++++++++++++++++++++++- 4 files changed, 1266 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 919b61a..ebe3297 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1035,9 +1035,9 @@ dependencies = [ [[package]] name = "hermit-abi" -version = "0.3.5" +version = "0.3.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d0c62115964e08cb8039170eb33c1d0e2388a256930279edca206fff675f82c3" +checksum = "d231dfb89cfffdbc30e7fc41579ed6066ad03abda9e567ccafae602b97ec5024" [[package]] name = "hex" diff --git a/feature-flags/README.md b/feature-flags/README.md index 458c519..1c95009 100644 --- a/feature-flags/README.md +++ b/feature-flags/README.md @@ -5,6 +5,24 @@ cargo test --package feature-flags ``` +### To watch changes + +``` +brew install cargo-watch +``` + +and then run: + +``` +cargo watch -x test --package feature-flags +``` + +To run a specific test: + +``` +cargo watch -x "test --package feature-flags --lib -- property_matching::tests::test_match_properties_math_operators --exact --show-output" +``` + # Running ``` diff --git a/feature-flags/src/flag_definitions.rs b/feature-flags/src/flag_definitions.rs index 89c32d6..04f39fe 100644 --- a/feature-flags/src/flag_definitions.rs +++ b/feature-flags/src/flag_definitions.rs @@ -53,6 +53,9 @@ pub enum OperatorType { #[derive(Debug, Clone, Deserialize, Serialize)] pub struct PropertyFilter { pub key: String, + // TODO: Probably need a default for value? + // incase operators like is_set, is_not_set are used + // not guaranteed to have a value, if say created via api pub value: serde_json::Value, pub operator: Option, #[serde(rename = "type")] diff --git a/feature-flags/src/property_matching.rs b/feature-flags/src/property_matching.rs index c8026d3..e08246e 100644 --- a/feature-flags/src/property_matching.rs +++ b/feature-flags/src/property_matching.rs @@ -12,6 +12,21 @@ pub enum FlagMatchingError { InvalidRegexPattern, } +pub fn to_string_representation(value: &Value) -> String { + + if value.is_string() { + return value.as_str().expect("string slice should always exist for string value").to_string(); + } + value.to_string() +} + +pub fn to_f64_representation(value: &Value) -> Option { + if value.is_number() { + return value.as_f64(); + } + to_string_representation(value).parse::().ok() +} + pub fn match_property( property: &PropertyFilter, matching_property_values: &HashMap, @@ -45,17 +60,16 @@ pub fn match_property( } if value.is_array() { - // TODO: Check if `to_string()` coerces all types to string correctly. return value .as_array() .expect("expected array value") .iter() - .map(|v| v.to_string().to_lowercase()) + .map(|v| to_string_representation(v).to_lowercase()) .collect::>() - .contains(&override_value.to_string().to_lowercase()); + .contains(&to_string_representation(override_value).to_lowercase()); } - return value.to_string().to_lowercase() - == override_value.to_string().to_lowercase(); + return to_string_representation(value).to_lowercase() + == to_string_representation(override_value).to_lowercase(); }; if let Some(match_value) = match_value { @@ -82,10 +96,13 @@ pub fn match_property( } OperatorType::Icontains | OperatorType::NotIcontains => { if let Some(match_value) = match_value { - let is_contained = match_value - .to_string() + // TODO: Check eq_ignore_ascii_case and to_ascii_lowercase + // see https://doc.rust-lang.org/std/string/struct.String.html#method.to_lowercase + // do we want to lowercase non-ascii stuff? + let is_contained = to_string_representation(match_value) .to_lowercase() - .contains(&value.to_string().to_lowercase()); + .contains(&to_string_representation(value).to_lowercase()); + if operator == OperatorType::Icontains { Ok(is_contained) } else { @@ -101,11 +118,14 @@ pub fn match_property( return Ok(false); } - let pattern = match Regex::new(&value.to_string()) { + let pattern = match Regex::new(&to_string_representation(value)) { Ok(pattern) => pattern, - Err(_) => return Err(FlagMatchingError::InvalidRegexPattern), + Err(_) => return Ok(false), + //TODO: Should we return Err here and handle elsewhere? + //Err(FlagMatchingError::InvalidRegexPattern) + // python just returns false here }; - let haystack = match_value.unwrap_or(&Value::Null).to_string(); + let haystack = to_string_representation(match_value.unwrap_or(&Value::Null)); let match_ = pattern.find(&haystack); if operator == OperatorType::Regex { @@ -130,7 +150,7 @@ pub fn match_property( } }; - let parsed_value = match match_value.unwrap_or(&Value::Null).as_f64() { + let parsed_value = match to_f64_representation(match_value.unwrap_or(&Value::Null)) { Some(parsed_value) => parsed_value, None => { return Err(FlagMatchingError::ValidationError( @@ -139,8 +159,8 @@ pub fn match_property( } }; - if let Some(override_value) = value.as_f64() { - Ok(compare(override_value, parsed_value, operator)) + if let Some(override_value) = to_f64_representation(value) { + Ok(compare(parsed_value, override_value, operator)) } else { return Err(FlagMatchingError::ValidationError( "override value is not a number".to_string(), @@ -223,7 +243,12 @@ fn is_truthy_property_value(value: &Value) -> bool { } #[cfg(test)] -mod tests { +mod test_match_properties { + /// Copy of https://github.com/PostHog/posthog/blob/master/posthog/queries/test/test_base.py#L35 + /// with some modifications to match Rust's behavior + /// and to test the match_property function + + use super::*; use serde_json::json; @@ -385,4 +410,1207 @@ mod tests { true ); } + + #[test] + fn test_match_properties_is_not() { + let property_a = PropertyFilter { + key: "key".to_string(), + value: json!("value"), + operator: Some(OperatorType::IsNot), + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key".to_string(), json!("value2"))]), + true + ) + .expect("expected match to exist"), + true + ); + + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key".to_string(), json!(""))]), + true + ) + .expect("expected match to exist"), + true + ); + + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key".to_string(), json!(null))]), + true + ) + .expect("expected match to exist"), + true + ); + + // partial mode returns error when key doesn't exist + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key2".to_string(), json!("value1"))]), + true + ) + .is_err(), + true + ); + + let property_c = PropertyFilter { + key: "key".to_string(), + value: json!(["value1", "value2", "value3"]), + operator: Some(OperatorType::IsNot), + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert_eq!( + match_property( + &property_c, + &HashMap::from([("key".to_string(), json!("value4"))]), + true + ) + .expect("expected match to exist"), + true + ); + + assert_eq!( + match_property( + &property_c, + &HashMap::from([("key".to_string(), json!("value5"))]), + true + ) + .expect("expected match to exist"), + true + ); + + assert_eq!( + match_property( + &property_c, + &HashMap::from([("key".to_string(), json!("value6"))]), + true + ) + .expect("expected match to exist"), + true + ); + + assert_eq!( + match_property( + &property_c, + &HashMap::from([("key".to_string(), json!(""))]), + true + ) + .expect("expected match to exist"), + true + ); + + assert_eq!( + match_property( + &property_c, + &HashMap::from([("key".to_string(), json!(null))]), + true + ) + .expect("expected match to exist"), + true + ); + + assert_eq!( + match_property( + &property_c, + &HashMap::from([("key".to_string(), json!("value2"))]), + true + ) + .expect("expected match to exist"), + false + ); + + assert_eq!( + match_property( + &property_c, + &HashMap::from([("key".to_string(), json!("value3"))]), + true + ) + .expect("expected match to exist"), + false + ); + + assert_eq!( + match_property( + &property_c, + &HashMap::from([("key".to_string(), json!("value1"))]), + true + ) + .expect("expected match to exist"), + false + ); + + assert_eq!( + match_property( + &property_c, + &HashMap::from([("key2".to_string(), json!("value1"))]), + true + ) + .is_err(), + true + ); + } + + #[test] + fn test_match_properties_is_set() { + let property_a = PropertyFilter { + key: "key".to_string(), + value: json!("value"), + operator: Some(OperatorType::IsSet), + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key".to_string(), json!("value"))]), + true + ) + .expect("expected match to exist"), + true + ); + + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key".to_string(), json!("value2"))]), + true + ) + .expect("expected match to exist"), + true + ); + + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key".to_string(), json!(""))]), + true + ) + .expect("expected match to exist"), + true + ); + + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key".to_string(), json!(null))]), + true + ) + .expect("expected match to exist"), + true + ); + + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key2".to_string(), json!("value1"))]), + true + ) + .is_err(), + true + ); + + assert_eq!( + match_property(&property_a, &HashMap::from([]), true).is_err(), + true + ); + } + + #[test] + fn test_match_properties_icontains() { + let property_a = PropertyFilter { + key: "key".to_string(), + value: json!("valUe"), + operator: Some(OperatorType::Icontains), + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key".to_string(), json!("value"))]), + true + ) + .expect("expected match to exist"), + true + ); + + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key".to_string(), json!("value2"))]), + true + ) + .expect("expected match to exist"), + true + ); + + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key".to_string(), json!("value3"))]), + true + ) + .expect("expected match to exist"), + true + ); + + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key".to_string(), json!("vaLue4"))]), + true + ) + .expect("expected match to exist"), + true + ); + + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key".to_string(), json!("343tfvalue5"))]), + true + ) + .expect("expected match to exist"), + true + ); + + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key".to_string(), json!("Alakazam"))]), + true + ) + .expect("expected match to exist"), + false + ); + + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key".to_string(), json!(123))]), + true + ) + .expect("expected match to exist"), + false + ); + + let property_b = PropertyFilter { + key: "key".to_string(), + value: json!("3"), + operator: Some(OperatorType::Icontains), + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert_eq!( + match_property( + &property_b, + &HashMap::from([("key".to_string(), json!("3"))]), + true + ) + .expect("expected match to exist"), + true + ); + + assert_eq!( + match_property( + &property_b, + &HashMap::from([("key".to_string(), json!(323))]), + true + ) + .expect("expected match to exist"), + true + ); + + assert_eq!( + match_property( + &property_b, + &HashMap::from([("key".to_string(), json!("val3"))]), + true + ) + .expect("expected match to exist"), + true + ); + + assert_eq!( + match_property( + &property_b, + &HashMap::from([("key".to_string(), json!("three"))]), + true + ) + .expect("expected match to exist"), + false + ); + } + + #[test] + fn test_match_properties_regex() { + let property_a = PropertyFilter { + key: "key".to_string(), + value: json!(r"\.com$"), + operator: Some(OperatorType::Regex), + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key".to_string(), json!("value.com"))]), + true + ) + .expect("expected match to exist"), + true + ); + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key".to_string(), json!("value2.com"))]), + true + ) + .expect("expected match to exist"), + true + ); + + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key".to_string(), json!(".com343tfvalue5"))]), + true + ) + .expect("expected match to exist"), + false + ); + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key".to_string(), json!("Alakazam"))]), + true + ) + .expect("expected match to exist"), + false + ); + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key".to_string(), json!(123))]), + true + ) + .expect("expected match to exist"), + false + ); + + let property_b = PropertyFilter { + key: "key".to_string(), + value: json!("3"), + operator: Some(OperatorType::Regex), + prop_type: "person".to_string(), + group_type_index: None, + }; + assert_eq!( + match_property( + &property_b, + &HashMap::from([("key".to_string(), json!("3"))]), + true + ) + .expect("expected match to exist"), + true + ); + assert_eq!( + match_property( + &property_b, + &HashMap::from([("key".to_string(), json!(323))]), + true + ) + .expect("expected match to exist"), + true + ); + assert_eq!( + match_property( + &property_b, + &HashMap::from([("key".to_string(), json!("val3"))]), + true + ) + .expect("expected match to exist"), + true + ); + + assert_eq!( + match_property( + &property_b, + &HashMap::from([("key".to_string(), json!("three"))]), + true + ) + .expect("expected match to exist"), + false + ); + + // invalid regex + let property_c = PropertyFilter { + key: "key".to_string(), + value: json!(r"?*"), + operator: Some(OperatorType::Regex), + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert_eq!( + match_property( + &property_c, + &HashMap::from([("key".to_string(), json!("value"))]), + true + ) + .expect("expected match to exist"), + false + ); + assert_eq!( + match_property( + &property_c, + &HashMap::from([("key".to_string(), json!("value2"))]), + true + ) + .expect("expected match to exist"), + false + ); + + // non string value + let property_d = PropertyFilter { + key: "key".to_string(), + value: json!(4), + operator: Some(OperatorType::Regex), + prop_type: "person".to_string(), + group_type_index: None, + }; + assert_eq!( + match_property( + &property_d, + &HashMap::from([("key".to_string(), json!("4"))]), + true + ) + .expect("expected match to exist"), + true + ); + assert_eq!( + match_property( + &property_d, + &HashMap::from([("key".to_string(), json!(4))]), + true + ) + .expect("expected match to exist"), + true + ); + + assert_eq!( + match_property( + &property_d, + &HashMap::from([("key".to_string(), json!("value"))]), + true + ) + .expect("expected match to exist"), + false + ); + + } + + #[test] + fn test_match_properties_math_operators() { + let property_a = PropertyFilter { + key: "key".to_string(), + value: json!(1), + operator: Some(OperatorType::Gt), + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key".to_string(), json!(2))]), + true + ) + .expect("expected match to exist"), + true + ); + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key".to_string(), json!(3))]), + true + ) + .expect("expected match to exist"), + true + ); + + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key".to_string(), json!(0))]), + true + ) + .expect("expected match to exist"), + false + ); + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key".to_string(), json!(-1))]), + true + ) + .expect("expected match to exist"), + false + ); + + // # we handle type mismatches so this should be true + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key".to_string(), json!("23"))]), + true + ) + .expect("expected match to exist"), + true + ); + + let property_b = PropertyFilter { + key: "key".to_string(), + value: json!(1), + operator: Some(OperatorType::Lt), + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert_eq!( + match_property( + &property_b, + &HashMap::from([("key".to_string(), json!(0))]), + true + ) + .expect("expected match to exist"), + true + ); + assert_eq!( + match_property( + &property_b, + &HashMap::from([("key".to_string(), json!(-1))]), + true + ) + .expect("expected match to exist"), + true + ); + assert_eq!( + match_property( + &property_b, + &HashMap::from([("key".to_string(), json!(-3))]), + true + ) + .expect("expected match to exist"), + true + ); + + assert_eq!( + match_property( + &property_b, + &HashMap::from([("key".to_string(), json!(1))]), + true + ) + .expect("expected match to exist"), + false + ); + assert_eq!( + match_property( + &property_b, + &HashMap::from([("key".to_string(), json!("1"))]), + true + ) + .expect("expected match to exist"), + false + ); + assert_eq!( + match_property( + &property_b, + &HashMap::from([("key".to_string(), json!("3"))]), + true + ) + .expect("expected match to exist"), + false + ); + + let property_c = PropertyFilter { + key: "key".to_string(), + value: json!(1), + operator: Some(OperatorType::Gte), + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert_eq!( + match_property( + &property_c, + &HashMap::from([("key".to_string(), json!(1))]), + true + ) + .expect("expected match to exist"), + true + ); + assert_eq!( + match_property( + &property_c, + &HashMap::from([("key".to_string(), json!(2))]), + true + ) + .expect("expected match to exist"), + true + ); + + assert_eq!( + match_property( + &property_c, + &HashMap::from([("key".to_string(), json!(0))]), + true + ) + .expect("expected match to exist"), + false + ); + assert_eq!( + match_property( + &property_c, + &HashMap::from([("key".to_string(), json!(-1))]), + true + ) + .expect("expected match to exist"), + false + ); + // # now we handle type mismatches so this should be true + assert_eq!( + match_property( + &property_c, + &HashMap::from([("key".to_string(), json!("3"))]), + true + ) + .expect("expected match to exist"), + true + ); + + let property_d = PropertyFilter { + key: "key".to_string(), + value: json!("43"), + operator: Some(OperatorType::Lt), + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert_eq!( + match_property( + &property_d, + &HashMap::from([("key".to_string(), json!("41"))]), + true + ) + .expect("expected match to exist"), + true + ); + assert_eq!( + match_property( + &property_d, + &HashMap::from([("key".to_string(), json!("42"))]), + true + ) + .expect("expected match to exist"), + true + ); + assert_eq!( + match_property( + &property_d, + &HashMap::from([("key".to_string(), json!(42))]), + true + ) + .expect("expected match to exist"), + true + ); + + assert_eq!( + match_property( + &property_d, + &HashMap::from([("key".to_string(), json!("43"))]), + true + ) + .expect("expected match to exist"), + false + ); + assert_eq!( + match_property( + &property_d, + &HashMap::from([("key".to_string(), json!("44"))]), + true + ) + .expect("expected match to exist"), + false + ); + assert_eq!( + match_property( + &property_d, + &HashMap::from([("key".to_string(), json!(44))]), + true + ) + .expect("expected match to exist"), + false + ); + + let property_e = PropertyFilter { + key: "key".to_string(), + value: json!("30"), + operator: Some(OperatorType::Lt), + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert_eq!( + match_property( + &property_e, + &HashMap::from([("key".to_string(), json!("29"))]), + true + ) + .expect("expected match to exist"), + true + ); + + // # depending on the type of override, we adjust type comparison + // This is wonky, do we want to continue this behavior? :/ + // TODO: Come back to this + // assert_eq!( + // match_property( + // &property_e, + // &HashMap::from([("key".to_string(), json!("100"))]), + // true + // ) + // .expect("expected match to exist"), + // true + // ); + assert_eq!( + match_property( + &property_e, + &HashMap::from([("key".to_string(), json!(100))]), + true + ) + .expect("expected match to exist"), + false + ); + + // let property_f = PropertyFilter { + // key: "key".to_string(), + // value: json!("123aloha"), + // operator: Some(OperatorType::Gt), + // prop_type: "person".to_string(), + // group_type_index: None, + // }; + + // TODO: This test fails because 123aloha is not a number + // and currently we don't support string comparison.. + // assert_eq!( + // match_property( + // &property_f, + // &HashMap::from([("key".to_string(), json!("123"))]), + // true + // ) + // .expect("expected match to exist"), + // false + // ); + // assert_eq!( + // match_property( + // &property_f, + // &HashMap::from([("key".to_string(), json!(122))]), + // true + // ) + // .expect("expected match to exist"), + // false + // ); + + // # this turns into a string comparison + // TODO: Fix + // assert_eq!( + // match_property( + // &property_f, + // &HashMap::from([("key".to_string(), json!(129))]), + // true + // ) + // .expect("expected match to exist"), + // true + // ); + } + + #[test] + fn test_none_property_value_with_all_operators() { + let property_a = PropertyFilter { + key: "key".to_string(), + value: json!("null"), + operator: Some(OperatorType::IsNot), + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert_eq!( + match_property(&property_a, &HashMap::from([("key".to_string(), json!(null))]), true) + .expect("expected match to exist"), + false + ); + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key".to_string(), json!("non"))]), + true + ) + .expect("expected match to exist"), + true + ); + + let property_b = PropertyFilter { + key: "key".to_string(), + value: json!(null), + operator: Some(OperatorType::IsSet), + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert_eq!( + match_property(&property_b, &HashMap::from([("key".to_string(), json!(null))]), true) + .expect("expected match to exist"), + true + ); + + let property_c = PropertyFilter { + key: "key".to_string(), + value: json!("nu"), + operator: Some(OperatorType::Icontains), + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert_eq!( + match_property(&property_c, &HashMap::from([("key".to_string(), json!(null))]), true) + .expect("expected match to exist"), + true + ); + assert_eq!( + match_property( + &property_c, + &HashMap::from([("key".to_string(), json!("smh"))]), + true + ) + .expect("expected match to exist"), + false + ); + + let property_d = PropertyFilter { + key: "key".to_string(), + value: json!("Nu"), + operator: Some(OperatorType::Regex), + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert_eq!( + match_property(&property_d, &HashMap::from([("key".to_string(), json!(null))]), true) + .expect("expected match to exist"), + false + ); + + let property_d_upper_case = PropertyFilter { + key: "key".to_string(), + value: json!("Nu"), + operator: Some(OperatorType::Regex), + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert_eq!( + match_property(&property_d_upper_case, &HashMap::from([("key".to_string(), json!(null))]), true) + .expect("expected match to exist"), + false + ); + + // TODO: Fails because not a number + // let property_e = PropertyFilter { + // key: "key".to_string(), + // value: json!(1), + // operator: Some(OperatorType::Gt), + // prop_type: "person".to_string(), + // group_type_index: None, + // }; + + // assert_eq!( + // match_property(&property_e, &HashMap::from([("key".to_string(), json!(null))]), true) + // .expect("expected match to exist"), + // true + // ); + } + + #[test] + fn test_match_properties_all_operators_with_full_props() { + let property_a = PropertyFilter { + key: "key".to_string(), + value: json!("value"), + operator: None, + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert_eq!( + match_property( + &property_a, + &HashMap::from([("key2".to_string(), json!("value"))]), + false + ).expect("Expected no errors with full props mode for non-existent keys"), + false + ); + assert_eq!( + match_property(&property_a, &HashMap::from([]), false), + Ok(false) + ); + + let property_exact = PropertyFilter { + key: "key".to_string(), + value: json!(["value1", "value2", "value3"]), + operator: Some(OperatorType::Exact), + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert_eq!( + match_property( + &property_exact, + &HashMap::from([("key2".to_string(), json!("value"))]), + false + ).expect("Expected no errors with full props mode"), + false + ); + + let property_is_set = PropertyFilter { + key: "key".to_string(), + value: json!("value"), + operator: Some(OperatorType::IsSet), + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert_eq!( + match_property( + &property_is_set, + &HashMap::from([("key2".to_string(), json!("value"))]), + false + ).expect("Expected no errors with full props mode"), + false + ); + + let property_is_not_set = PropertyFilter { + key: "key".to_string(), + value: json!(null), + operator: Some(OperatorType::IsNotSet), + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert_eq!( + match_property( + &property_is_not_set, + &HashMap::from([("key2".to_string(), json!("value"))]), + false + ).expect("Expected no errors with full props mode"), + true + ); + assert_eq!( + match_property( + &property_is_not_set, + &HashMap::from([("key".to_string(), json!("value"))]), + false + ).expect("Expected no errors with full props mode"), + false + ); + + // is not set with partial props returns false when key exists + assert_eq!( + match_property( + &property_is_not_set, + &HashMap::from([("key".to_string(), json!("value"))]), + true + ).expect("Expected no errors with full props mode"), + false + ); + // is not set returns error when key doesn't exist + assert_eq!( + match_property( + &property_is_not_set, + &HashMap::from([("key2".to_string(), json!("value"))]), + true + ).is_err(), + true + ); + + let property_icontains = PropertyFilter { + key: "key".to_string(), + value: json!("valUe"), + operator: Some(OperatorType::Icontains), + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert_eq!( + match_property( + &property_icontains, + &HashMap::from([("key2".to_string(), json!("value"))]), + false + ).expect("Expected no errors with full props mode"), + false + ); + + let property_not_icontains = PropertyFilter { + key: "key".to_string(), + value: json!("valUe"), + operator: Some(OperatorType::NotIcontains), + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert_eq!( + match_property( + &property_not_icontains, + &HashMap::from([("key2".to_string(), json!("value"))]), + false + ).expect("Expected no errors with full props mode"), + false + ); + + let property_regex = PropertyFilter { + key: "key".to_string(), + value: json!(r"\.com$"), + operator: Some(OperatorType::Regex), + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert_eq!( + match_property( + &property_regex, + &HashMap::from([("key2".to_string(), json!("value.com"))]), + false + ).expect("Expected no errors with full props mode"), + false + ); + + let property_not_regex = PropertyFilter { + key: "key".to_string(), + value: json!(r"\.com$"), + operator: Some(OperatorType::NotRegex), + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert_eq!( + match_property( + &property_not_regex, + &HashMap::from([("key2".to_string(), json!("value.com"))]), + false + ).expect("Expected no errors with full props mode"), + false + ); + + let property_gt = PropertyFilter { + key: "key".to_string(), + value: json!(1), + operator: Some(OperatorType::Gt), + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert_eq!( + match_property( + &property_gt, + &HashMap::from([("key2".to_string(), json!(2))]), + false + ).expect("Expected no errors with full props mode"), + false + ); + + let property_gte = PropertyFilter { + key: "key".to_string(), + value: json!(1), + operator: Some(OperatorType::Gte), + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert_eq!( + match_property( + &property_gte, + &HashMap::from([("key2".to_string(), json!(2))]), + false + ).expect("Expected no errors with full props mode"), + false + ); + + let property_lt = PropertyFilter { + key: "key".to_string(), + value: json!(1), + operator: Some(OperatorType::Lt), + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert_eq!( + match_property( + &property_lt, + &HashMap::from([("key2".to_string(), json!(0))]), + false + ).expect("Expected no errors with full props mode"), + false + ); + + let property_lte = PropertyFilter { + key: "key".to_string(), + value: json!(1), + operator: Some(OperatorType::Lte), + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert_eq!( + match_property( + &property_lte, + &HashMap::from([("key2".to_string(), json!(0))]), + false + ).expect("Expected no errors with full props mode"), + false + ); + + // TODO: Handle date operators + let property_is_date_before = PropertyFilter { + key: "key".to_string(), + value: json!("2021-01-01"), + operator: Some(OperatorType::IsDateBefore), + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert_eq!( + match_property( + &property_is_date_before, + &HashMap::from([("key2".to_string(), json!("2021-01-02"))]), + false + ).expect("Expected no errors with full props mode"), + false + ); + + + } } From a06ea51898445bb8a0560b15b49e9ae25692f017 Mon Sep 17 00:00:00 2001 From: Neil Kakkar Date: Tue, 4 Jun 2024 13:48:19 +0100 Subject: [PATCH 16/21] fmt --- feature-flags/src/property_matching.rs | 101 ++++++++++++++++--------- 1 file changed, 67 insertions(+), 34 deletions(-) diff --git a/feature-flags/src/property_matching.rs b/feature-flags/src/property_matching.rs index e08246e..56255c6 100644 --- a/feature-flags/src/property_matching.rs +++ b/feature-flags/src/property_matching.rs @@ -13,9 +13,11 @@ pub enum FlagMatchingError { } pub fn to_string_representation(value: &Value) -> String { - if value.is_string() { - return value.as_str().expect("string slice should always exist for string value").to_string(); + return value + .as_str() + .expect("string slice should always exist for string value") + .to_string(); } value.to_string() } @@ -247,8 +249,6 @@ mod test_match_properties { /// Copy of https://github.com/PostHog/posthog/blob/master/posthog/queries/test/test_base.py#L35 /// with some modifications to match Rust's behavior /// and to test the match_property function - - use super::*; use serde_json::json; @@ -410,7 +410,7 @@ mod test_match_properties { true ); } - + #[test] fn test_match_properties_is_not() { let property_a = PropertyFilter { @@ -922,7 +922,6 @@ mod test_match_properties { .expect("expected match to exist"), false ); - } #[test] @@ -1261,8 +1260,12 @@ mod test_match_properties { }; assert_eq!( - match_property(&property_a, &HashMap::from([("key".to_string(), json!(null))]), true) - .expect("expected match to exist"), + match_property( + &property_a, + &HashMap::from([("key".to_string(), json!(null))]), + true + ) + .expect("expected match to exist"), false ); assert_eq!( @@ -1284,8 +1287,12 @@ mod test_match_properties { }; assert_eq!( - match_property(&property_b, &HashMap::from([("key".to_string(), json!(null))]), true) - .expect("expected match to exist"), + match_property( + &property_b, + &HashMap::from([("key".to_string(), json!(null))]), + true + ) + .expect("expected match to exist"), true ); @@ -1298,8 +1305,12 @@ mod test_match_properties { }; assert_eq!( - match_property(&property_c, &HashMap::from([("key".to_string(), json!(null))]), true) - .expect("expected match to exist"), + match_property( + &property_c, + &HashMap::from([("key".to_string(), json!(null))]), + true + ) + .expect("expected match to exist"), true ); assert_eq!( @@ -1321,8 +1332,12 @@ mod test_match_properties { }; assert_eq!( - match_property(&property_d, &HashMap::from([("key".to_string(), json!(null))]), true) - .expect("expected match to exist"), + match_property( + &property_d, + &HashMap::from([("key".to_string(), json!(null))]), + true + ) + .expect("expected match to exist"), false ); @@ -1335,8 +1350,12 @@ mod test_match_properties { }; assert_eq!( - match_property(&property_d_upper_case, &HashMap::from([("key".to_string(), json!(null))]), true) - .expect("expected match to exist"), + match_property( + &property_d_upper_case, + &HashMap::from([("key".to_string(), json!(null))]), + true + ) + .expect("expected match to exist"), false ); @@ -1371,7 +1390,8 @@ mod test_match_properties { &property_a, &HashMap::from([("key2".to_string(), json!("value"))]), false - ).expect("Expected no errors with full props mode for non-existent keys"), + ) + .expect("Expected no errors with full props mode for non-existent keys"), false ); assert_eq!( @@ -1392,7 +1412,8 @@ mod test_match_properties { &property_exact, &HashMap::from([("key2".to_string(), json!("value"))]), false - ).expect("Expected no errors with full props mode"), + ) + .expect("Expected no errors with full props mode"), false ); @@ -1409,7 +1430,8 @@ mod test_match_properties { &property_is_set, &HashMap::from([("key2".to_string(), json!("value"))]), false - ).expect("Expected no errors with full props mode"), + ) + .expect("Expected no errors with full props mode"), false ); @@ -1426,7 +1448,8 @@ mod test_match_properties { &property_is_not_set, &HashMap::from([("key2".to_string(), json!("value"))]), false - ).expect("Expected no errors with full props mode"), + ) + .expect("Expected no errors with full props mode"), true ); assert_eq!( @@ -1434,7 +1457,8 @@ mod test_match_properties { &property_is_not_set, &HashMap::from([("key".to_string(), json!("value"))]), false - ).expect("Expected no errors with full props mode"), + ) + .expect("Expected no errors with full props mode"), false ); @@ -1444,7 +1468,8 @@ mod test_match_properties { &property_is_not_set, &HashMap::from([("key".to_string(), json!("value"))]), true - ).expect("Expected no errors with full props mode"), + ) + .expect("Expected no errors with full props mode"), false ); // is not set returns error when key doesn't exist @@ -1453,7 +1478,8 @@ mod test_match_properties { &property_is_not_set, &HashMap::from([("key2".to_string(), json!("value"))]), true - ).is_err(), + ) + .is_err(), true ); @@ -1470,7 +1496,8 @@ mod test_match_properties { &property_icontains, &HashMap::from([("key2".to_string(), json!("value"))]), false - ).expect("Expected no errors with full props mode"), + ) + .expect("Expected no errors with full props mode"), false ); @@ -1487,7 +1514,8 @@ mod test_match_properties { &property_not_icontains, &HashMap::from([("key2".to_string(), json!("value"))]), false - ).expect("Expected no errors with full props mode"), + ) + .expect("Expected no errors with full props mode"), false ); @@ -1504,7 +1532,8 @@ mod test_match_properties { &property_regex, &HashMap::from([("key2".to_string(), json!("value.com"))]), false - ).expect("Expected no errors with full props mode"), + ) + .expect("Expected no errors with full props mode"), false ); @@ -1521,7 +1550,8 @@ mod test_match_properties { &property_not_regex, &HashMap::from([("key2".to_string(), json!("value.com"))]), false - ).expect("Expected no errors with full props mode"), + ) + .expect("Expected no errors with full props mode"), false ); @@ -1538,7 +1568,8 @@ mod test_match_properties { &property_gt, &HashMap::from([("key2".to_string(), json!(2))]), false - ).expect("Expected no errors with full props mode"), + ) + .expect("Expected no errors with full props mode"), false ); @@ -1555,7 +1586,8 @@ mod test_match_properties { &property_gte, &HashMap::from([("key2".to_string(), json!(2))]), false - ).expect("Expected no errors with full props mode"), + ) + .expect("Expected no errors with full props mode"), false ); @@ -1572,7 +1604,8 @@ mod test_match_properties { &property_lt, &HashMap::from([("key2".to_string(), json!(0))]), false - ).expect("Expected no errors with full props mode"), + ) + .expect("Expected no errors with full props mode"), false ); @@ -1589,7 +1622,8 @@ mod test_match_properties { &property_lte, &HashMap::from([("key2".to_string(), json!(0))]), false - ).expect("Expected no errors with full props mode"), + ) + .expect("Expected no errors with full props mode"), false ); @@ -1607,10 +1641,9 @@ mod test_match_properties { &property_is_date_before, &HashMap::from([("key2".to_string(), json!("2021-01-02"))]), false - ).expect("Expected no errors with full props mode"), + ) + .expect("Expected no errors with full props mode"), false ); - - } } From 23503d79a6ae19326b65179db3ce4ba5479b7c2a Mon Sep 17 00:00:00 2001 From: Neil Kakkar Date: Tue, 4 Jun 2024 13:53:29 +0100 Subject: [PATCH 17/21] fmt --- feature-flags/src/property_matching.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/feature-flags/src/property_matching.rs b/feature-flags/src/property_matching.rs index 56255c6..e680afc 100644 --- a/feature-flags/src/property_matching.rs +++ b/feature-flags/src/property_matching.rs @@ -244,11 +244,11 @@ fn is_truthy_property_value(value: &Value) -> bool { false } +/// Copy of https://github.com/PostHog/posthog/blob/master/posthog/queries/test/test_base.py#L35 +/// with some modifications to match Rust's behavior +/// and to test the match_property function #[cfg(test)] mod test_match_properties { - /// Copy of https://github.com/PostHog/posthog/blob/master/posthog/queries/test/test_base.py#L35 - /// with some modifications to match Rust's behavior - /// and to test the match_property function use super::*; use serde_json::json; From b7871993c4468651459703058e4d7d81a389d4e5 Mon Sep 17 00:00:00 2001 From: Neil Kakkar Date: Mon, 10 Jun 2024 10:13:37 +0100 Subject: [PATCH 18/21] add sqlx --- Cargo.lock | 1 + feature-flags/Cargo.toml | 1 + 2 files changed, 2 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index ebe3297..2be1f7d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -712,6 +712,7 @@ dependencies = [ "serde-pickle", "serde_json", "sha1", + "sqlx", "thiserror", "tokio", "tracing", diff --git a/feature-flags/Cargo.toml b/feature-flags/Cargo.toml index de0d8cf..c353476 100644 --- a/feature-flags/Cargo.toml +++ b/feature-flags/Cargo.toml @@ -23,6 +23,7 @@ redis = { version = "0.23.3", features = [ ] } serde = { workspace = true } serde_json = { workspace = true } +sqlx = { workspace = true } thiserror = { workspace = true } serde-pickle = { version = "1.1.1"} hex = "0.4.3" From f25c63e233f022f3c30ba990ac6abb3ee4f6c7d7 Mon Sep 17 00:00:00 2001 From: Neil Kakkar Date: Mon, 10 Jun 2024 10:24:34 +0100 Subject: [PATCH 19/21] fix bad merge --- feature-flags/src/flag_definitions.rs | 2 +- feature-flags/src/property_matching.rs | 12 +++++------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/feature-flags/src/flag_definitions.rs b/feature-flags/src/flag_definitions.rs index 5efa0d1..fbbd044 100644 --- a/feature-flags/src/flag_definitions.rs +++ b/feature-flags/src/flag_definitions.rs @@ -16,7 +16,7 @@ pub const TEAM_FLAGS_CACHE_PREFIX: &str = "posthog:1:team_feature_flags_"; #[derive(Debug, Deserialize)] pub enum GroupTypeIndex {} -#[derive(Debug, Clone, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, Deserialize)] #[serde(rename_all = "snake_case")] pub enum OperatorType { Exact, diff --git a/feature-flags/src/property_matching.rs b/feature-flags/src/property_matching.rs index e680afc..0bffdbd 100644 --- a/feature-flags/src/property_matching.rs +++ b/feature-flags/src/property_matching.rs @@ -37,13 +37,11 @@ pub fn match_property( // only looks for matches where key exists in override_property_values // doesn't support operator is_not_set with partial_props - if partial_props { - if !matching_property_values.contains_key(&property.key) { - return Err(FlagMatchingError::MissingProperty(format!( - "can't match properties without a value. Missing property: {}", - property.key - ))); - } + if partial_props && !matching_property_values.contains_key(&property.key) { + return Err(FlagMatchingError::MissingProperty(format!( + "can't match properties without a value. Missing property: {}", + property.key + ))); } let key = &property.key; From acec81ac76d959913b6c1f4dff130bbeb8b2295e Mon Sep 17 00:00:00 2001 From: Neil Kakkar Date: Mon, 10 Jun 2024 10:26:54 +0100 Subject: [PATCH 20/21] fix --- Cargo.lock | 1 - feature-flags/Cargo.toml | 1 - 2 files changed, 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2be1f7d..ebe3297 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -712,7 +712,6 @@ dependencies = [ "serde-pickle", "serde_json", "sha1", - "sqlx", "thiserror", "tokio", "tracing", diff --git a/feature-flags/Cargo.toml b/feature-flags/Cargo.toml index c353476..de0d8cf 100644 --- a/feature-flags/Cargo.toml +++ b/feature-flags/Cargo.toml @@ -23,7 +23,6 @@ redis = { version = "0.23.3", features = [ ] } serde = { workspace = true } serde_json = { workspace = true } -sqlx = { workspace = true } thiserror = { workspace = true } serde-pickle = { version = "1.1.1"} hex = "0.4.3" From 2dec74a61632187dd94c0fc3f4768d37a0179e95 Mon Sep 17 00:00:00 2001 From: Neil Kakkar Date: Mon, 10 Jun 2024 10:30:55 +0100 Subject: [PATCH 21/21] clippy --- Cargo.lock | 1 - feature-flags/Cargo.toml | 1 - feature-flags/src/property_matching.rs | 16 ++++++++-------- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ebe3297..804ab47 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -702,7 +702,6 @@ dependencies = [ "axum-client-ip", "bytes", "envconfig", - "hex", "once_cell", "rand", "redis", diff --git a/feature-flags/Cargo.toml b/feature-flags/Cargo.toml index de0d8cf..08ff21e 100644 --- a/feature-flags/Cargo.toml +++ b/feature-flags/Cargo.toml @@ -25,7 +25,6 @@ serde = { workspace = true } serde_json = { workspace = true } thiserror = { workspace = true } serde-pickle = { version = "1.1.1"} -hex = "0.4.3" sha1 = "0.10.6" regex = "1.10.4" diff --git a/feature-flags/src/property_matching.rs b/feature-flags/src/property_matching.rs index 0bffdbd..9f7d9ea 100644 --- a/feature-flags/src/property_matching.rs +++ b/feature-flags/src/property_matching.rs @@ -68,8 +68,8 @@ pub fn match_property( .collect::>() .contains(&to_string_representation(override_value).to_lowercase()); } - return to_string_representation(value).to_lowercase() - == to_string_representation(override_value).to_lowercase(); + to_string_representation(value).to_lowercase() + == to_string_representation(override_value).to_lowercase() }; if let Some(match_value) = match_value { @@ -79,7 +79,7 @@ pub fn match_property( Ok(!compute_exact_match(value, match_value)) } } else { - return Ok(false); + Ok(false) } } OperatorType::IsSet => Ok(matching_property_values.contains_key(key)), @@ -162,14 +162,14 @@ pub fn match_property( if let Some(override_value) = to_f64_representation(value) { Ok(compare(parsed_value, override_value, operator)) } else { - return Err(FlagMatchingError::ValidationError( + Err(FlagMatchingError::ValidationError( "override value is not a number".to_string(), - )); + )) } } OperatorType::IsDateExact | OperatorType::IsDateAfter | OperatorType::IsDateBefore => { // TODO: Handle date operators - return Ok(false); + Ok(false) // let parsed_date = determine_parsed_date_for_property_matching(match_value); // if parsed_date.is_none() { @@ -212,7 +212,7 @@ fn is_truthy_or_falsy_property_value(value: &Value) -> bool { .as_array() .expect("expected array value") .iter() - .all(|v| is_truthy_or_falsy_property_value(v)); + .all(is_truthy_or_falsy_property_value); } false @@ -236,7 +236,7 @@ fn is_truthy_property_value(value: &Value) -> bool { .as_array() .expect("expected array value") .iter() - .all(|v| is_truthy_property_value(v)); + .all(is_truthy_property_value); } false