Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spike for constraints #27

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open
2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ rand = "0.8.4"
serde_json = "1.0.68"
serde_plain = "1.0.0"
surf = "2.3.1"
semver = "1.0.5"
enum_dispatch = "0.3.8"

[dependencies.chrono]
features = ["serde"]
Expand Down
146 changes: 137 additions & 9 deletions src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@ use std::collections::HashMap;
use std::default::Default;

use chrono::Utc;
use enum_dispatch::enum_dispatch;
use serde::{Deserialize, Serialize};

use crate::{Context, Evaluate};

#[derive(Serialize, Deserialize, Debug)]
// #[serde(deny_unknown_fields)]
pub struct Features {
Expand Down Expand Up @@ -40,24 +43,117 @@ pub struct Strategy {
pub parameters: Option<HashMap<String, String>>,
}

#[derive(Clone, Serialize, Deserialize, Debug)]
pub enum StrConstraint {
StrEndsWith(Vec<String>, bool),
StrStartsWith(Vec<String>, bool),
StrContains(Vec<String>, bool),
}

#[derive(Clone, Serialize, Deserialize, Debug)]
pub enum NumericConstraint {
NumEq(String),
NumGt(String),
NumGte(String),
NumLt(String),
NumLte(String),
}

#[derive(Clone, Serialize, Deserialize, Debug)]
pub enum DateConstraint {
DateAfter(String),
DateBefore(String),
}

#[derive(Clone, Serialize, Deserialize, Debug)]
pub enum SemverConstraint {
SemverEq(String),
SemverGt(String),
SemverLt(String),
}

#[derive(Clone, Serialize, Deserialize, Debug)]
pub enum ContainsConstraint {
In(Vec<String>),
NotIn(Vec<String>),
}

#[enum_dispatch]
#[derive(Clone, Serialize, Deserialize, Debug)]
pub enum ConstraintExpression {
NumericConstraint,
StrConstraint,
DateConstraint,
SemverConstraint,
ContainsConstraint,
}

#[enum_dispatch(ConstraintExpression)]
pub trait EvaluatorConstructor {
fn yield_evaluator(self, getter: crate::strategy::Expr) -> Evaluate;
}

pub trait Expression: Fn(&Context) -> Option<&String> {
fn clone_boxed(&self) -> Box<dyn Expression + Send + Sync + 'static>;
}

#[derive(Clone, Serialize, Deserialize, Debug)]
// #[serde(deny_unknown_fields)]
pub struct Constraint {
#[serde(rename = "contextName")]
pub context_name: String,
pub inverted: Option<bool>,
#[serde(flatten)]
pub expression: ConstraintExpression,
}

#[derive(Clone, Serialize, Deserialize, Debug)]
#[serde(tag = "operator", content = "values")]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a shame this needs to change: the enums are much less ergonomic now :( - single attribute structs :(.

I wonder if struct flattening + an internally tagged representation could work: a family of related enums:

  • one with values
  • one with value

// #[serde(deny_unknown_fields)]
pub enum ConstraintExpression {
#[serde(rename = "IN")]
In(Vec<String>),
#[serde(rename = "NOT_IN")]
NotIn(Vec<String>),
}
// #[derive(Clone, Serialize, Deserialize, Debug)]
// #[serde(tag = "operator")]
// // #[serde(deny_unknown_fields)]
// pub enum ConstraintExpression {
// #[serde(rename = "IN")]
// In { values: Vec<String> },
// #[serde(rename = "NOT_IN")]
// NotIn { values: Vec<String> },
// #[serde(rename = "STR_ENDS_WITH")]
// StrEndsWith {
// values: Vec<String>,
// #[serde(rename = "caseInsensitive")]
// case_insensitive: Option<bool>,
// },
// #[serde(rename = "STR_STARTS_WITH")]
// StrStartsWith {
// values: Vec<String>,
// #[serde(rename = "caseInsensitive")]
// case_insensitive: Option<bool>,
// },
// #[serde(rename = "STR_CONTAINS")]
// StrContains {
// values: Vec<String>,
// #[serde(rename = "caseInsensitive")]
// case_insensitive: Option<bool>,
// },
// #[serde(rename = "NUM_EQ")]
// NumEq { value: String },
// #[serde(rename = "NUM_GT")]
// NumGt { value: String },
// #[serde(rename = "NUM_GTE")]
// NumGte { value: String },
// #[serde(rename = "NUM_LT")]
// NumLt { value: String },
// #[serde(rename = "NUM_LTE")]
// NumLte { value: String },
// #[serde(rename = "DATE_AFTER")]
// DateAfter { value: String },
// #[serde(rename = "DATE_BEFORE")]
// DateBefore { value: String },
// #[serde(rename = "SEMVER_EQ")]
// SemverEq { value: String },
// #[serde(rename = "SEMVER_GT")]
// SemverGt { value: String },
// #[serde(rename = "SEMVER_LT")]
// SemverLt { value: String },
// }

#[derive(Clone, Serialize, Deserialize, Debug)]
// #[serde(deny_unknown_fields)]
Expand Down Expand Up @@ -134,6 +230,38 @@ pub struct MetricsBucket {
#[cfg(test)]
mod tests {
use super::Registration;
use super::*;

#[test]
fn parses_advanced_constraint_structure() -> Result<(), serde_json::Error> {
let data = r#"
{
"contextName": "customField",
"operator": "STR_STARTS_WITH",
"values": ["some-string"]
}"#;
let _: super::Constraint = serde_json::from_str(data)?;

let data = r#"
{
"contextName": "customField",
"operator": "NUM_GTE",
"value": "7"
}"#;
let _: super::Constraint = serde_json::from_str(data)?;

let data = r#"
{
"contextName": "customField",
"operator": "STR_STARTS_WITH",
"values": ["some-string"],
"caseInsensitive": true,
"inverted": true
}"#;
let _: super::Constraint = serde_json::from_str(data)?;

Ok(())
}

#[test]
fn parse_reference_doc() -> Result<(), serde_json::Error> {
Expand Down
7 changes: 7 additions & 0 deletions src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
//! <https://docs.getunleash.io/user_guide/unleash_context>
use std::{collections::HashMap, net::IpAddr};

use chrono::Utc;
use serde::{de, Deserialize};

// Custom IP Address newtype that can be deserialised from strings e.g. 127.0.0.1 for use with tests.
Expand Down Expand Up @@ -40,4 +41,10 @@ pub struct Context {
pub app_name: String,
#[serde(default)]
pub environment: String,
#[serde(default = "current_time", rename = "currentTime")]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should test #[serde(rename_all = "camelCase")] at some point, might be sufficient.

pub current_time: String,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be a string.

IIRC correctly chrono doesn't have a sensible default, so I suggest:
a Chrono tuple newtype with serde derived impls, a std::default::Default implementation for that, then inclusion here, which as the chrono::serde default is rfc3339 should just work.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, I'm not sure whether we should be capturing the current time at all. For the proxy use case, clients should not be permitted to time-travel; for the non-proxy use case access to the current time is probably cheap enough, since time access is such a well known thing.

We could compile rules that relate to get compiled into a unix time expression for even faster evaluation, if scaling the proxy is required - though I suspect only unleash themselves would hit that scale.

}

fn current_time() -> String {
Utc::now().to_rfc3339()
}
Loading