From 51ef54b74c82e1cd4f1a0ac6157c9cd468eab653 Mon Sep 17 00:00:00 2001 From: Yoofi Quansah Date: Thu, 7 Dec 2023 09:24:20 -0600 Subject: [PATCH] feat: do not fail on non-existent namespace in Rust core (#26) * feat: do not fail on non-existent namespace in Rust core * chore: fix ruby tests descriptions * chore: fix node test description --- flipt-client-go/evaluation_test.go | 28 +++++++++++++++++++ flipt-client-node/src/evaluation.test.ts | 34 +++++++++++++++++++++++ flipt-client-python/tests/__init__.py | 20 +++++++++++++ flipt-client-ruby/spec/evaluation_spec.rb | 30 ++++++++++++++++++++ flipt-engine/examples/evaluation.rs | 2 +- flipt-engine/src/evaluator/mod.rs | 12 ++++---- flipt-engine/src/lib.rs | 2 +- flipt-engine/src/models/source.rs | 12 ++++++++ flipt-engine/src/store/mod.rs | 13 ++++----- 9 files changed, 137 insertions(+), 16 deletions(-) diff --git a/flipt-client-go/evaluation_test.go b/flipt-client-go/evaluation_test.go index 25e28bbe..b6aa458e 100644 --- a/flipt-client-go/evaluation_test.go +++ b/flipt-client-go/evaluation_test.go @@ -57,3 +57,31 @@ func TestBoolean(t *testing.T) { assert.True(t, boolean.Result.Enabled) assert.Equal(t, "MATCH_EVALUATION_REASON", boolean.Result.Reason) } + +func TestVariantFailure(t *testing.T) { + evaluationClient, err := evaluation.NewClient(evaluation.WithURL(fliptUrl), evaluation.WithAuthToken(authToken)) + require.NoError(t, err) + + variant, err := evaluationClient.EvaluateVariant(context.TODO(), "nonexistent", "someentity", map[string]string{ + "fizz": "buzz", + }) + require.NoError(t, err) + + assert.Nil(t, variant.Result) + assert.Equal(t, "failure", variant.Status) + assert.Equal(t, "failed to get flag information default/nonexistent", variant.ErrorMessage) +} + +func TestBooleanFailure(t *testing.T) { + evaluationClient, err := evaluation.NewClient(evaluation.WithURL(fliptUrl), evaluation.WithAuthToken(authToken)) + require.NoError(t, err) + + boolean, err := evaluationClient.EvaluateBoolean(context.TODO(), "nonexistent", "someentity", map[string]string{ + "fizz": "buzz", + }) + require.NoError(t, err) + + assert.Nil(t, boolean.Result) + assert.Equal(t, "failure", boolean.Status) + assert.Equal(t, "failed to get flag information default/nonexistent", boolean.ErrorMessage) +} diff --git a/flipt-client-node/src/evaluation.test.ts b/flipt-client-node/src/evaluation.test.ts index 93ea6ae2..4a90732b 100644 --- a/flipt-client-node/src/evaluation.test.ts +++ b/flipt-client-node/src/evaluation.test.ts @@ -47,3 +47,37 @@ test('boolean', () => { expect(boolean.result.enabled).toEqual(true); expect(boolean.result.reason).toEqual('MATCH_EVALUATION_REASON'); }); + +test('variant failure', () => { + const fec = new FliptEvaluationClient('default', { + url: fliptUrl, + auth_token: authToken + }); + + const variant = fec.evaluateVariant('nonexistent', 'someentity', { + fizz: 'buzz' + }); + + expect(variant.result).toBeNull(); + expect(variant.status).toEqual('failure'); + expect(variant.error_message).toEqual( + 'failed to get flag information default/nonexistent' + ); +}); + +test('boolean failure', () => { + const fec = new FliptEvaluationClient('default', { + url: fliptUrl, + auth_token: authToken + }); + + const boolean = fec.evaluateVariant('nonexistent', 'someentity', { + fizz: 'buzz' + }); + + expect(boolean.result).toBeNull(); + expect(boolean.status).toEqual('failure'); + expect(boolean.error_message).toEqual( + 'failed to get flag information default/nonexistent' + ); +}); diff --git a/flipt-client-python/tests/__init__.py b/flipt-client-python/tests/__init__.py index 72b1e01a..04deb7c1 100644 --- a/flipt-client-python/tests/__init__.py +++ b/flipt-client-python/tests/__init__.py @@ -40,6 +40,26 @@ def test_boolean(self): self.assertEqual("flag_boolean", boolean.result.flag_key) self.assertEqual("MATCH_EVALUATION_REASON", boolean.result.reason) + def test_failure_variant(self): + variant = self.flipt_client.evaluate_variant( + "nonexistent", "someentity", {"fizz": "buzz"} + ) + self.assertIsNone(variant.result) + self.assertEqual( + variant.error_message, "failed to get flag information default/nonexistent" + ) + self.assertEqual("failure", variant.status) + + def test_failure_boolean(self): + boolean = self.flipt_client.evaluate_boolean( + "nonexistent", "someentity", {"fizz": "buzz"} + ) + self.assertIsNone(boolean.result) + self.assertEqual( + boolean.error_message, "failed to get flag information default/nonexistent" + ) + self.assertEqual("failure", boolean.status) + if __name__ == "__main__": unittest.main() diff --git a/flipt-client-ruby/spec/evaluation_spec.rb b/flipt-client-ruby/spec/evaluation_spec.rb index 69e7f1ce..c8b1b65a 100644 --- a/flipt-client-ruby/spec/evaluation_spec.rb +++ b/flipt-client-ruby/spec/evaluation_spec.rb @@ -47,4 +47,34 @@ expect(resp['result']['reason']).to eq('MATCH_EVALUATION_REASON') end end + + describe '#evaluate_variant failure' do + it 'gracefully handles failures for variant flag evaluation' do + url = ENV.fetch('FLIPT_URL', 'http://localhost:8080') + auth_token = ENV.fetch('FLIPT_AUTH_TOKEN', 'secret') + client = Flipt::EvaluationClient.new('default', { url: url, auth_token: auth_token }) + + resp = client.evaluate_variant({ flag_key: 'nonexistent', entity_id: 'someentity', context: { "fizz": 'buzz' } }) + + expect(resp).to_not be_nil + expect(resp['result']).to be_nil + expect(resp['status']).to eq('failure') + expect(resp['error_message']).to eq('failed to get flag information default/nonexistent') + end + end + + describe '#evaluate_boolean failure' do + it 'gracefully handles failures for boolean flag evaluation' do + url = ENV.fetch('FLIPT_URL', 'http://localhost:8080') + auth_token = ENV.fetch('FLIPT_AUTH_TOKEN', 'secret') + client = Flipt::EvaluationClient.new('default', { url: url, auth_token: auth_token }) + + resp = client.evaluate_boolean({ flag_key: 'nonexistent', entity_id: 'someentity', context: { "fizz": 'buzz' } }) + + expect(resp).to_not be_nil + expect(resp['result']).to be_nil + expect(resp['status']).to eq('failure') + expect(resp['error_message']).to eq('failed to get flag information default/nonexistent') + end + end end diff --git a/flipt-engine/examples/evaluation.rs b/flipt-engine/examples/evaluation.rs index c594c0a4..e2b62606 100644 --- a/flipt-engine/examples/evaluation.rs +++ b/flipt-engine/examples/evaluation.rs @@ -9,7 +9,7 @@ fn main() { parser::HTTPParser::new("http://localhost:8080", Some("secret")), ); - let eng = fliptengine::Engine::new(evaluator.unwrap(), Default::default()); + let eng = fliptengine::Engine::new(evaluator, Default::default()); let mut context: HashMap = HashMap::new(); context.insert("fizz".into(), "buzz".into()); diff --git a/flipt-engine/src/evaluator/mod.rs b/flipt-engine/src/evaluator/mod.rs index 3d576c9a..74299df0 100644 --- a/flipt-engine/src/evaluator/mod.rs +++ b/flipt-engine/src/evaluator/mod.rs @@ -109,14 +109,14 @@ impl

Evaluator where P: Parser + Send, { - pub fn new_snapshot_evaluator(namespaces: Vec, parser: P) -> Result { - let snap = Snapshot::build(&namespaces, &parser)?; + pub fn new_snapshot_evaluator(namespaces: Vec, parser: P) -> Self { + let snap = Snapshot::build(&namespaces, &parser); Evaluator::new(namespaces, parser, snap) } pub fn replace_snapshot(&mut self) { let snap = Snapshot::build(&self.namespaces, &self.parser); - self.replace_store(snap.unwrap()); + self.replace_store(snap); } } @@ -125,13 +125,13 @@ where P: Parser + Send, S: Store + Send, { - pub fn new(namespaces: Vec, parser: P, store_impl: S) -> Result { - Ok(Self { + pub fn new(namespaces: Vec, parser: P, store_impl: S) -> Self { + Self { namespaces, parser, store: store_impl, mtx: Arc::new(RwLock::new(0)), - }) + } } pub fn replace_store(&mut self, store_impl: S) { diff --git a/flipt-engine/src/lib.rs b/flipt-engine/src/lib.rs index d3a00576..c58e6ef2 100644 --- a/flipt-engine/src/lib.rs +++ b/flipt-engine/src/lib.rs @@ -186,7 +186,7 @@ pub unsafe extern "C" fn initialize_engine( let parser = parser::HTTPParser::new(&http_url, auth_token.clone().as_deref()); let evaluator = evaluator::Evaluator::new_snapshot_evaluator(namespaces_vec, parser); - Box::into_raw(Box::new(Engine::new(evaluator.unwrap(), engine_opts))) as *mut c_void + Box::into_raw(Box::new(Engine::new(evaluator, engine_opts))) as *mut c_void } /// # Safety diff --git a/flipt-engine/src/models/source.rs b/flipt-engine/src/models/source.rs index 9bb75ec8..1075bb9d 100644 --- a/flipt-engine/src/models/source.rs +++ b/flipt-engine/src/models/source.rs @@ -8,6 +8,18 @@ pub struct Document { pub flags: Vec, } +impl Default for Document { + fn default() -> Self { + Self { + namespace: Namespace { + key: "".into(), + name: None, + }, + flags: Vec::new(), + } + } +} + #[derive(Deserialize)] #[serde(rename_all = "camelCase")] pub struct Namespace { diff --git a/flipt-engine/src/store/mod.rs b/flipt-engine/src/store/mod.rs index 5ba20605..24fff403 100644 --- a/flipt-engine/src/store/mod.rs +++ b/flipt-engine/src/store/mod.rs @@ -1,10 +1,10 @@ use crate::models::common; use crate::models::flipt; +use crate::models::source::Document; use crate::parser::Parser; #[cfg(test)] use mockall::automock; -use snafu::Whatever; use std::collections::HashMap; #[cfg_attr(test, automock)] @@ -40,14 +40,14 @@ struct Namespace { } impl Snapshot { - pub fn build

(namespaces: &Vec, parser: &P) -> Result + pub fn build

(namespaces: &Vec, parser: &P) -> Snapshot where P: Parser + Send, { let mut ns: HashMap = HashMap::new(); for n in namespaces { - let doc = parser.parse(n)?; + let doc = parser.parse(n).unwrap_or(Document::default()); let mut flags: HashMap = HashMap::new(); let mut eval_rules: HashMap> = HashMap::new(); @@ -207,7 +207,7 @@ impl Snapshot { ); } - Ok(Self { namespaces: ns }) + Self { namespaces: ns } } } @@ -268,10 +268,7 @@ mod tests { fn test_snapshot() { let tp = TestParser::new(); - let snapshot = match Snapshot::build(&vec!["default".into()], &tp) { - Ok(s) => s, - Err(e) => panic!("{}", e), - }; + let snapshot = Snapshot::build(&vec!["default".into()], &tp); let flag_variant = snapshot .get_flag("default", "flag1")