feat: minor updates per the latest Tiles API (#97)

- click_url's aespFlag parameter is deprecated
- note os-family isn't currently for targeting
- include more metadata in sentry errors

Closes #96

src/adm/filter.rs

@@ -1,5 +1,10 @@
-use std::{collections::HashMap, fmt::Debug};
+use std::{
+    collections::{HashMap, HashSet},
+    fmt::Debug,
+    iter::FromIterator,
+};
 
+use lazy_static::lazy_static;
 use url::Url;
 
 use super::{AdmAdvertiserFilterSettings, AdmTile, DEFAULT};
@@ -9,6 +14,16 @@ use crate::{
     web::middleware::sentry as l_sentry,
 };
 
+lazy_static! {
+    static ref REQ_CLICK_PARAMS: Vec<&'static str> = vec!["ci", "ctag", "key", "version"];
+    static ref ALL_CLICK_PARAMS: HashSet<&'static str> = {
+        let opt_click_params = vec!["click-status"];
+        let mut all = HashSet::from_iter(REQ_CLICK_PARAMS.clone());
+        all.extend(opt_click_params);
+        all
+    };
+}
+
 #[allow(rustdoc::private_intra_doc_links)]
 /// Filter criteria for ADM Tiles
 ///
@@ -56,6 +71,7 @@ impl AdmFilter {
         let url = &tile.advertiser_url;
         let species = "Advertiser";
         tags.add_tag("type", species);
+        tags.add_extra("tile", &tile.name);
         tags.add_extra("url", &url);
         let parsed: Url = match url.parse() {
             Ok(v) => v,
@@ -81,48 +97,42 @@ impl AdmFilter {
         let url = &tile.click_url;
         let species = "Click";
         tags.add_tag("type", species);
+        tags.add_extra("tile", &tile.name);
         tags.add_extra("url", &url);
 
-        // Check the required fields are present for the `click_url`
-        // pg 15 of 5.7.21 spec. (sort for efficiency)
-        // The list of sorted required query param keys for click_urls
-        let req_keys = vec!["aespFlag", "ci", "ctag", "key", "version"];
-        // the list of optionally appearing query param keys
-        let opt_keys = vec!["click-status"];
-
-        let mut all_keys = req_keys.clone();
-        all_keys.extend(opt_keys.clone());
-
+        // Check the required fields are present for the `click_url` pg 15 of
+        // 5.7.21 spec
         let parsed: Url = match url.parse() {
             Ok(v) => v,
             Err(e) => {
                 tags.add_extra("parse_error", &e.to_string());
                 return Err(HandlerErrorKind::InvalidHost(species, url.to_string()).into());
             }
         };
-        let mut query_keys = parsed
+        let query_keys = parsed
             .query_pairs()
             .map(|p| p.0.to_string())
-            .collect::<Vec<String>>();
-        query_keys.sort();
+            .collect::<HashSet<String>>();
 
         // run the gauntlet of checks.
         if !check_url(parsed, "Click", &filter.click_hosts)? {
             dbg!("bad url", url.to_string());
             tags.add_extra("reason", "bad host");
             return Err(HandlerErrorKind::InvalidHost(species, url.to_string()).into());
         }
-        for key in req_keys {
-            if !query_keys.contains(&key.to_owned()) {
-                dbg!("missing param", key, url.to_string());
+        for key in &*REQ_CLICK_PARAMS {
+            if !query_keys.contains(*key) {
+                dbg!("missing param", &key, url.to_string());
                 tags.add_extra("reason", "missing required query param");
+                tags.add_extra("param", &key);
                 return Err(HandlerErrorKind::InvalidHost(species, url.to_string()).into());
             }
         }
         for key in query_keys {
-            if !all_keys.contains(&key.as_str()) {
-                dbg!("invalid param", key, url.to_string());
+            if !ALL_CLICK_PARAMS.contains(key.as_str()) {
+                dbg!("invalid param", &key, url.to_string());
                 tags.add_extra("reason", "invalid query param");
+                tags.add_extra("param", &key);
                 return Err(HandlerErrorKind::InvalidHost(species, url.to_string()).into());
             }
         }
@@ -141,6 +151,7 @@ impl AdmFilter {
         let url = &tile.impression_url;
         let species = "Impression";
         tags.add_tag("type", species);
+        tags.add_extra("tile", &tile.name);
         tags.add_extra("url", &url);
         let parsed: Url = match url.parse() {
             Ok(v) => v,
@@ -157,6 +168,7 @@ impl AdmFilter {
         if query_keys != vec!["id"] {
             dbg!("missing param", "id", url.to_string());
             tags.add_extra("reason", "invalid query param");
+            tags.add_extra("param", "id");
             return Err(HandlerErrorKind::InvalidHost(species, url.to_string()).into());
         }
         check_url(parsed, species, &filter.impression_hosts)?;

src/adm/tiles.rs

@@ -49,7 +49,6 @@ pub async fn get_tiles(
         &[
             ("partner", settings.partner_id.as_str()),
             ("sub1", settings.sub1.as_str()),
-            ("ip", &location.fake_ip), // TODO: remove once ADM API finalized
             ("country-code", &location.country()),
             ("region-code", &location.region()),
             // ("dma-code", location.dma),

src/server/cache.rs

@@ -22,7 +22,8 @@ pub struct AudienceKey {
     pub region_code: String,
     /// The form-factor (e.g. desktop, phone) of the device
     pub form_factor: FormFactor,
-    // XXX: this may not be a targetting parameter? (if so it shouldn't be here)
+    // XXX: *not* currently a targetting parameter (shouldn't be here),
+    // temporarily needed for tile_cache_updater
     /// The family of Operating System (e.g. windows, macos) of the device
     pub os_family: OsFamily,
 }

src/server/location.rs

@@ -17,7 +17,6 @@ const GOOG_LOC_HEADER: &str = "x-client-geo-location";
 /// The returned, stripped location.
 #[derive(Serialize, Debug, Default, Clone)]
 pub struct LocationResult {
-    pub fake_ip: String, // TODO: remove once ADM API is finalized
     #[serde(skip_serializing_if = "Option::is_none")]
     pub city: Option<String>,
     #[serde(skip_serializing_if = "Option::is_none")]

src/web/mock_adm_response1.json

@@ -3,23 +3,23 @@
         {
             "id": 601,
             "name": "Acme",
-            "click_url": "https://example.com/ctp?version=16.0.0&key=22.1&ci=6.2&ctag=1612376952400200000&aespFlag=altinst",
+            "click_url": "https://example.com/ctp?version=16.0.0&key=22.1&ci=6.2&ctag=1612376952400200000",
             "image_url": "https://cdn.example.com/601.jpg",
             "advertiser_url": "https://www.acme.biz/?foo=1&device=Computers&cmpgn=123601",
             "impression_url": "https://example.net/static?id=0000"
         },
         {
             "id": 703,
             "name": "Dunder Mifflin",
-            "click_url": "https://example.com/ctp?version=16.0.0&key=7.2&ci=8.9&ctag=E1DE38C8972D0281F5556659A&aespFlag=altinst",
+            "click_url": "https://example.com/ctp?version=16.0.0&key=7.2&ci=8.9&ctag=E1DE38C8972D0281F5556659A",
             "image_url": "https://cdn.example.com/703.jpg",
             "advertiser_url": "https://www.dunderm.biz/?tag=bar&ref=baz",
             "impression_url": "https://example.com/static?id=DEADB33F"
         },
         {
             "id": 805,
             "name": "Los Pollos Hermanos",
-            "click_url": "https://example.com/ctp?version=16.0.0&key=3.3&ci=4.4&ctag=1612376952400200000&aespFlag=altinst",
+            "click_url": "https://example.com/ctp?version=16.0.0&key=3.3&ci=4.4&ctag=1612376952400200000",
             "image_url": "https://cdn.example.com/805.jpg",
             "advertiser_url": "https://www.lph-nm.biz/?utm_source=google&utm_campaign=quux*{keyword}_m*{match-type}_d*BARBAZ_22",
             "impression_url": "https://example.net/static?id=ABAD1DEA"

src/web/test.rs

@@ -163,26 +163,26 @@ async fn basic() {
 
 #[actix_rt::test]
 async fn basic_bad_reply() {
-    let missing_aespflag = r#"{
+    let missing_ci = r#"{
         "tiles": [
             {
                 "id": 601,
                 "name": "Acme",
-                "click_url": "https://example.com/ctp?version=16.0.0&key=22.1&ci=6.2&ctag=1612376952400200000",
+                "click_url": "https://example.com/ctp?version=16.0.0&key=22.1&ctag=1612376952400200000",
                 "image_url": "https://cdn.example.com/601.jpg",
                 "advertiser_url": "https://www.acme.biz/?foo=1&device=Computers&cmpgn=123601",
                 "impression_url": "https://example.net/static?id=0000"
             },
             {
                 "id": 703,
                 "name": "Dunder Mifflin",
-                "click_url": "https://example.com/ctp?version=16.0.0&key=7.2&ci=8.9&ctag=E1DE38C8972D0281F5556659A&aespFlag=altinst",
+                "click_url": "https://example.com/ctp?version=16.0.0&key=7.2&ci=8.9&ctag=E1DE38C8972D0281F5556659A",
                 "image_url": "https://cdn.example.com/703.jpg",
                 "advertiser_url": "https://www.dunderm.biz/?tag=bar&ref=baz",
                 "impression_url": "https://example.net/static?id=DEADB33F"
             }
         ]}"#;
-    let (_, addr) = init_mock_adm(missing_aespflag.to_owned());
+    let (_, addr) = init_mock_adm(missing_ci.to_owned());
     let settings = Settings {
         adm_endpoint_url: format!("http://{}:{}/?partner=foo&sub1=bar", addr.ip(), addr.port()),
         adm_settings: json!(adm_settings()).to_string(),