feat: support etags in clients (#299)

flipt-client-browser/src/index.ts

@@ -6,14 +6,19 @@ import {
   BooleanResult,
   EngineOpts,
   EvaluationRequest,
+  IFetcher,
   VariantResult
 } from './models.js';
 
 export class FliptEvaluationClient {
   private engine: Engine;
-  private fetcher: () => Promise<Response>;
+  private fetcher: IFetcher;
+  private etag?: string;
 
-  private constructor(engine: Engine, fetcher: () => Promise<Response>) {
+  private constructor(
+    engine: Engine,
+    fetcher: ({ etag }: { etag?: string }) => Promise<Response>
+  ) {
     this.engine = engine;
     this.fetcher = fetcher;
   }
@@ -64,11 +69,22 @@ export class FliptEvaluationClient {
     let fetcher = engine_opts.fetcher;
 
     if (!fetcher) {
-      fetcher = async () => {
+      fetcher = async ({ etag }: { etag?: string }) => {
+        if (etag) {
+          headers.append('If-None-Match', etag);
+        }
+
         const resp = await fetch(url, {
           method: 'GET',
           headers
         });
+
+        // check for 304 status code
+        if (resp.status === 304) {
+          return resp;
+        }
+
+        // ok only checks for range 200-299
         if (!resp.ok) {
           throw new Error('Failed to fetch data');
         }
@@ -77,7 +93,12 @@ export class FliptEvaluationClient {
       };
     }
 
-    const resp = await fetcher();
+    // should be no etag on first fetch
+    const resp = await fetcher({});
+    if (!resp) {
+      throw new Error('Failed to fetch data');
+    }
+
     const data = await resp.json();
 
     const engine = new Engine(namespace, data);
@@ -89,9 +110,18 @@ export class FliptEvaluationClient {
    * @returns void
    */
   public async refresh() {
-    const resp = await this.fetcher();
-    const data = await resp.json();
+    const opts = { etag: this.etag };
+    const resp = await this.fetcher(opts);
+
+    if (resp.status === 304) {
+      let etag = resp.headers.get('etag');
+      if (etag) {
+        this.etag = etag;
+      }
+      return;
+    }
 
+    const data = await resp.json();
     this.engine.snapshot(data);
   }
 

flipt-client-browser/src/models.ts

@@ -1,5 +1,5 @@
-export interface fetcher {
-  (): Promise<Response>;
+export interface IFetcher {
+  ({ etag }: { etag?: string }): Promise<Response>;
 }
 
 export interface AuthenticationStrategy {}
@@ -16,7 +16,7 @@ export interface EngineOpts {
   url?: string;
   authentication?: AuthenticationStrategy;
   reference?: string;
-  fetcher?: fetcher;
+  fetcher?: IFetcher;
 }
 
 export interface EvaluationRequest {

flipt-engine-ffi/Cargo.toml

@@ -21,6 +21,7 @@ path = "../flipt-evaluation"
 
 [dev-dependencies]
 mockall = "0.12.1"
+mockito = "1.4.0"
 
 [lib]
 name = "fliptengine"

flipt-engine-ffi/src/evaluator/mod.rs

@@ -36,7 +36,11 @@ where
     pub fn replace_snapshot(&mut self) {
         match self.parser.parse(&self.namespace) {
             Ok(doc) => {
-                match Snapshot::build(&self.namespace, doc) {
+                // if doc is none then return, nothing to do
+                if doc.is_none() {
+                    return;
+                }
+                match Snapshot::build(&self.namespace, doc.unwrap()) {
                     Ok(s) => {
                         self.replace_store(s, None);
                     }
@@ -126,6 +130,7 @@ where
         batch_evaluation(&self.store, &self.namespace, requests)
     }
 }
+
 #[cfg(test)]
 mod tests {
     use std::collections::HashMap;
@@ -139,7 +144,7 @@ mod tests {
     mock! {
         pub Parser{}
         impl parser::Parser for Parser {
-            fn parse(&self, namespace: &str) -> Result<source::Document, Error>;
+            fn parse(&mut self, namespace: &str) -> Result<Option<source::Document>, Error>;
         }
     }
 
@@ -153,6 +158,7 @@ mod tests {
             }
         }
     }
+
     #[test]
     fn test_parser_with_error() {
         let expected_error = "server error: can't connect";
@@ -182,7 +188,9 @@ mod tests {
     #[test]
     fn test_parser_with_empty_snapshot() {
         let mut parser = MockParser::new();
-        parser.expect_parse().returning(|_| Ok(Document::default()));
+        parser
+            .expect_parse()
+            .returning(|_| Ok(Some(Document::default())));
         let evaluator =
             Evaluator::new_snapshot_evaluator("namespace", parser).expect("expect valid evaluator");
 
@@ -218,10 +226,23 @@ mod tests {
         );
     }
 
+    #[test]
+    fn test_parser_with_no_snapshot() {
+        let mut parser = MockParser::new();
+        parser.expect_parse().returning(|_| Ok(None));
+        let evaluator =
+            Evaluator::new_snapshot_evaluator("namespace", parser).expect("expect valid evaluator");
+
+        let response = evaluator.list_flags();
+        assert!(response.is_ok());
+    }
+
     #[test]
     fn test_list_flags_from_another_namespace() {
         let mut parser = MockParser::new();
-        parser.expect_parse().returning(|_| Ok(Document::default()));
+        parser
+            .expect_parse()
+            .returning(|_| Ok(Some(Document::default())));
         let mut evaluator =
             Evaluator::new_snapshot_evaluator("namespace", parser).expect("expect valid evaluator");
         evaluator.replace_store(Snapshot::empty("other"), None);

flipt-engine-ffi/src/parser/http.rs

@@ -61,6 +61,7 @@ pub struct HTTPParser {
     http_url: String,
     authentication: HeaderMap,
     reference: Option<String>,
+    etag: Option<String>,
 }
 
 pub struct HTTPParserBuilder {
@@ -97,6 +98,7 @@ impl HTTPParserBuilder {
             http_url: self.http_url,
             authentication: self.authentication,
             reference: self.reference,
+            etag: None,
         }
     }
 }
@@ -121,12 +123,8 @@ impl HTTPParser {
 }
 
 impl Parser for HTTPParser {
-    fn parse(&self, namespace: &str) -> Result<source::Document, Error> {
+    fn parse(&mut self, namespace: &str) -> Result<Option<source::Document>, Error> {
         let mut headers = HeaderMap::new();
-        headers.insert(
-            reqwest::header::CONTENT_TYPE,
-            reqwest::header::HeaderValue::from_static("application/json"),
-        );
         headers.insert(
             reqwest::header::ACCEPT,
             reqwest::header::HeaderValue::from_static("application/json"),
@@ -137,6 +135,14 @@ impl Parser for HTTPParser {
             reqwest::header::HeaderValue::from_static("1.38.0"),
         );
 
+        // add etag / if-none-match header if we have one
+        if let Some(etag) = &self.etag {
+            headers.insert(
+                reqwest::header::IF_NONE_MATCH,
+                reqwest::header::HeaderValue::from_str(etag).unwrap(),
+            );
+        }
+
         for (key, value) in self.authentication.iter() {
             headers.insert(key, value.clone());
         }
@@ -154,6 +160,16 @@ impl Parser for HTTPParser {
             Err(e) => return Err(Error::Server(format!("failed to make request: {}", e))),
         };
 
+        // check if we have a 304 response
+        if response.status() == reqwest::StatusCode::NOT_MODIFIED {
+            return Ok(None);
+        }
+
+        // check if we have a new etag
+        if let Some(etag) = response.headers().get(reqwest::header::ETAG) {
+            self.etag = Some(etag.to_str().unwrap().to_string());
+        }
+
         let response_text = match response.text() {
             Ok(t) => t,
             Err(e) => return Err(Error::Server(format!("failed to get response body: {}", e))),
@@ -164,13 +180,126 @@ impl Parser for HTTPParser {
             Err(e) => return Err(Error::InvalidJSON(e.to_string())),
         };
 
-        Ok(document)
+        Ok(Some(document))
     }
 }
 
 #[cfg(test)]
 mod tests {
     use crate::parser::http::Authentication;
+    use crate::parser::http::HTTPParserBuilder;
+    use fliptevaluation::parser::Parser;
+
+    #[test]
+    fn test_http_parse() {
+        let mut server = mockito::Server::new();
+        let mock = server
+            .mock("GET", "/internal/v1/evaluation/snapshot/namespace/default")
+            .with_status(200)
+            .with_header("content-type", "application/json")
+            .with_header("etag", "etag")
+            .with_body(r#"{"namespace": {"key": "default"}, "flags":[]}"#)
+            .create();
+
+        let url = server.url();
+        let mut parser = HTTPParserBuilder::new(&url)
+            .authentication(Authentication::None)
+            .build();
+
+        let result = parser.parse("default");
+
+        assert!(result.is_ok());
+        mock.assert();
+
+        assert_eq!(parser.etag, Some("etag".to_string()));
+    }
+
+    #[test]
+    fn test_http_parse_not_modified() {
+        let mut server = mockito::Server::new();
+        let mock = server
+            .mock("GET", "/internal/v1/evaluation/snapshot/namespace/default")
+            .match_header("if-none-match", "etag")
+            .with_status(304)
+            .create();
+
+        let url = server.url();
+        let mut parser = HTTPParserBuilder::new(&url)
+            .authentication(Authentication::None)
+            .build();
+
+        parser.etag = Some("etag".to_string());
+
+        let result = parser.parse("default");
+
+        assert!(result.is_ok());
+        assert!(result.unwrap().is_none());
+
+        mock.assert();
+    }
+
+    #[test]
+    fn test_http_parse_error() {
+        let mut server = mockito::Server::new();
+        let mock = server
+            .mock("GET", "/internal/v1/evaluation/snapshot/namespace/default")
+            .with_status(500)
+            .create();
+
+        let url = server.url();
+        let mut parser = HTTPParserBuilder::new(&url)
+            .authentication(Authentication::None)
+            .build();
+
+        let result = parser.parse("default");
+
+        assert!(!result.is_ok());
+        mock.assert();
+    }
+
+    #[test]
+    fn test_http_parse_token_auth() {
+        let mut server = mockito::Server::new();
+        let mock = server
+            .mock("GET", "/internal/v1/evaluation/snapshot/namespace/default")
+            .match_header("authorization", "Bearer foo")
+            .with_status(200)
+            .with_header("content-type", "application/json")
+            .with_body(r#"{"namespace": {"key": "default"}, "flags":[]}"#)
+            .create();
+
+        let url = server.url();
+        let mut parser = HTTPParserBuilder::new(&url)
+            .authentication(Authentication::ClientToken("foo".to_string()))
+            .build();
+
+        let result = parser.parse("default");
+
+        assert!(result.is_ok());
+        mock.assert();
+    }
+
+    #[test]
+    fn test_http_parse_jwt_auth() {
+        let mut server = mockito::Server::new();
+        let mock = server
+            .mock("GET", "/internal/v1/evaluation/snapshot/namespace/default")
+            .match_header("authorization", "JWT foo")
+            .with_status(200)
+            .with_header("content-type", "application/json")
+            .with_body(r#"{"namespace": {"key": "default"}, "flags":[]}"#)
+            .create();
+
+        let url = server.url();
+        let mut parser = HTTPParserBuilder::new(&url)
+            .authentication(Authentication::JwtToken("foo".to_string()))
+            .build();
+
+        let result = parser.parse("default");
+
+        assert!(result.is_ok());
+        mock.assert();
+    }
 
     #[test]
     fn test_http_parser_url() {

flipt-evaluation/src/parser/mod.rs

@@ -1,5 +1,5 @@
 pub trait Parser {
-    fn parse(&self, namespace: &str) -> Result<source::Document, Error>;
+    fn parse(&mut self, namespace: &str) -> Result<Option<source::Document>, Error>;
 }
 
 #[cfg(test)]
@@ -24,7 +24,7 @@ impl TestParser {
 
 #[cfg(test)]
 impl Parser for TestParser {
-    fn parse(&self, _: &str) -> Result<source::Document, Error> {
+    fn parse(&mut self, _: &str) -> Result<Option<source::Document>, Error> {
         let f = match &self.path {
             Some(path) => path.to_owned(),
             None => {
@@ -41,6 +41,6 @@ impl Parser for TestParser {
             Err(e) => return Err(Error::InvalidJSON(e.to_string())),
         };
 
-        Ok(document)
+        Ok(Some(document))
     }
 }