refactor: kill the old adM API's country mapping

and cleanup the extractors (killing the old placement/country params)

Closes #195

src/adm/filter.rs

@@ -7,7 +7,10 @@ use std::{
 use lazy_static::lazy_static;
 use url::Url;
 
-use super::{tiles::AdmTile, AdmAdvertiserFilterSettings, Tile, DEFAULT};
+use super::{
+    tiles::{AdmTile, Tile},
+    AdmAdvertiserFilterSettings, DEFAULT,
+};
 use crate::{
     error::{HandlerError, HandlerErrorKind, HandlerResult},
     metrics::Metrics,

src/adm/mod.rs

@@ -12,4 +12,4 @@ mod tiles;
 
 pub use filter::AdmFilter;
 pub(crate) use settings::{AdmAdvertiserFilterSettings, AdmSettings, DEFAULT};
-pub use tiles::{get_tiles, Tile};
+pub use tiles::{get_tiles, TileResponse};

src/adm/tiles.rs

@@ -11,7 +11,7 @@ use crate::{
     server::{location::LocationResult, ServerState},
     settings::Settings,
     tags::Tags,
-    web::{FormFactor, OsFamily},
+    web::DeviceInfo,
 };
 
 /// The payload provided by ADM
@@ -107,32 +107,25 @@ impl Tile {
 
 /// Main handler for the User Agent HTTP request
 ///
-#[allow(clippy::too_many_arguments)]
 pub async fn get_tiles(
-    reqwest_client: &reqwest::Client,
-    adm_endpoint_url: &str,
-    location: &LocationResult,
-    os_family: OsFamily,
-    form_factor: FormFactor,
     state: &ServerState,
+    location: &LocationResult,
+    device_info: DeviceInfo,
     tags: &mut Tags,
     metrics: &Metrics,
     headers: Option<&HeaderMap>,
 ) -> Result<TileResponse, HandlerError> {
-    // XXX: Assumes adm_endpoint_url includes
-    // ?partner=<mozilla_partner_name>&sub1=<mozilla_tag_id> (probably should
-    // validate this on startup)
     let settings = &state.settings;
     let adm_url = Url::parse_with_params(
-        adm_endpoint_url,
+        &state.adm_endpoint_url,
         &[
             ("partner", settings.adm_partner_id.clone().unwrap().as_str()),
             ("sub1", settings.adm_sub1.clone().unwrap().as_str()),
             ("country-code", &location.country()),
             ("region-code", &location.region()),
             // ("dma-code", location.dma),
-            ("form-factor", &form_factor.to_string()),
-            ("os-family", &os_family.to_string()),
+            ("form-factor", &device_info.form_factor.to_string()),
+            ("os-family", &device_info.os_family.to_string()),
             ("sub2", "newtab"),
             ("v", "1.0"),
             // XXX: some value for results seems required, it defaults to 0
@@ -157,8 +150,8 @@ pub async fn get_tiles(
         trace!("Getting fake response: {:?}", &test_response);
         AdmTileResponse::fake_response(&state.settings, test_response)?
     } else {
-        // TODO: Add timeout
-        reqwest_client
+        state
+            .reqwest_client
             .get(adm_url)
             .timeout(Duration::from_secs(settings.adm_timeout))
             .send()

src/server/mod.rs

@@ -1,5 +1,5 @@
 //! Main application server
-use std::{collections::HashMap, sync::Arc, time::Duration};
+use std::time::Duration;
 
 use actix_cors::Cors;
 use actix_web::{
@@ -33,7 +33,6 @@ pub struct ServerState {
     /// Metric reporting
     pub metrics: Box<StatsdClient>,
     pub adm_endpoint_url: String,
-    pub adm_country_ip_map: Arc<HashMap<String, String>>,
     pub reqwest_client: reqwest::Client,
     pub tiles_cache: cache::TilesCache,
     pub mmdb: location::Location,
@@ -51,7 +50,6 @@ impl std::fmt::Debug for ServerState {
         fmt.debug_struct("ServerState")
             .field("metrics", &self.metrics)
             .field("adm_endpoint_url", &self.adm_endpoint_url)
-            .field("adm_country_ip_map", &self.adm_country_ip_map)
             .field("reqwest_client", &self.reqwest_client)
             .field("tiles_cache", &self.tiles_cache)
             .field("mmdb", &mmdb_status.to_owned())
@@ -97,7 +95,6 @@ impl Server {
         let state = ServerState {
             metrics: Box::new(metrics.clone()),
             adm_endpoint_url: settings.adm_endpoint_url.clone(),
-            adm_country_ip_map: Arc::new(settings.build_adm_country_ip_map()),
             reqwest_client: reqwest::Client::builder()
                 .user_agent(REQWEST_USER_AGENT)
                 .build()?,

src/settings.rs

@@ -1,7 +1,5 @@
 //! Application settings objects and initialization
 
-use std::collections::HashMap;
-
 use actix_web::{dev::ServiceRequest, web::Data, HttpRequest};
 use config::{Config, ConfigError, Environment, File};
 use serde::Deserialize;
@@ -13,20 +11,6 @@ static PREFIX: &str = "contile";
 
 static DEFAULT_PORT: u16 = 8000;
 
-// issue11: AdM doesn't return any tiles for any of the example non-finalized
-// fake IPs, so replaced "US": "174.245.240.112" with "130.245.32.23" (from
-// their provided stage URL params) that does result in tiles
-static DEFAULT_ADM_COUNTRY_IP_MAP: &str = r#"
-{
-    "US": "130.245.32.23",
-    "UK": "86.164.248.137",
-    "DE": "87.182.235.159",
-    "FR": "31.39.255.255",
-    "IT": "5.62.79.255",
-    "JP": "27.98.191.255"
-}
-"#;
-
 // TODO: Call this `EnvSettings` that serializes into
 // real `Settings`?
 //
@@ -79,8 +63,6 @@ pub struct Settings {
     pub adm_sub1: Option<String>,
     /// adm Endpoint URL
     pub adm_endpoint_url: String,
-    /// adm country to default IP map (Hash in JSON format)
-    pub adm_country_ip_map: String,
     /// max tiles to accept from ADM (default: 2)
     pub adm_max_tiles: u8,
     /// number of tiles to query from ADM (default: 10)
@@ -126,7 +108,6 @@ impl Default for Settings {
             adm_endpoint_url: "".to_owned(),
             adm_partner_id: None,
             adm_sub1: None,
-            adm_country_ip_map: DEFAULT_ADM_COUNTRY_IP_MAP.to_owned(),
             adm_max_tiles: 2,
             adm_query_tile_count: 10,
             adm_timeout: 5,
@@ -207,20 +188,6 @@ impl Settings {
     pub fn banner(&self) -> String {
         format!("http://{}:{}", self.host, self.port)
     }
-
-    /// convert the `adm_country_ip_map` setting from a string to a hashmap
-    pub(crate) fn build_adm_country_ip_map(&self) -> HashMap<String, String> {
-        let mut map: HashMap<String, String> =
-            serde_json::from_str(&self.adm_country_ip_map).expect("Invalid ADM_COUNTRY_IP_MAP");
-        map = map
-            .into_iter()
-            .map(|(key, val)| (key.to_uppercase(), val))
-            .collect();
-        if !map.contains_key("US") {
-            panic!("Invalid ADM_COUNTRY_IP_MAP");
-        }
-        map
-    }
 }
 
 impl<'a> From<&'a HttpRequest> for &'a Settings {

src/web/extractors.rs

@@ -2,35 +2,29 @@
 //!
 //! Handles ensuring the header's, body, and query parameters are correct, extraction to
 //! relevant types, and failing correctly with the appropriate errors if issues arise.
+use std::net::{IpAddr, SocketAddr};
+
 use actix_web::{
-    dev::Payload, http::header, http::header::HeaderValue, web, Error, FromRequest, HttpRequest,
+    dev::Payload,
+    http::header,
+    http::{HeaderName, HeaderValue},
+    web, Error, FromRequest, HttpRequest,
 };
 use futures::future::{self, FutureExt, LocalBoxFuture};
 use lazy_static::lazy_static;
-use serde::Deserialize;
 
-use crate::{error::HandlerErrorKind, metrics::Metrics};
+use crate::{
+    metrics::Metrics,
+    server::{location::LocationResult, ServerState},
+    web::user_agent::{get_device_info, DeviceInfo},
+};
 
 lazy_static! {
     static ref EMPTY_HEADER: HeaderValue = HeaderValue::from_static("");
+    static ref X_FORWARDED_FOR: HeaderName = HeaderName::from_static("x-forwarded-for");
 }
 
-const VALID_PLACEMENTS: &[&str] = &["urlbar", "newtab", "search"];
-
-#[derive(Debug, Deserialize)]
-pub struct TilesParams {
-    country: Option<String>,
-    placement: Option<String>,
-}
-
-#[derive(Debug)]
-pub struct TilesRequest {
-    pub country: Option<String>,
-    pub placement: Option<String>,
-    pub ua: String,
-}
-
-impl FromRequest for TilesRequest {
+impl FromRequest for DeviceInfo {
     type Config = ();
     type Error = Error;
     type Future = LocalBoxFuture<'static, Result<Self, Self::Error>>;
@@ -44,25 +38,57 @@ impl FromRequest for TilesRequest {
                 .unwrap_or(&EMPTY_HEADER)
                 .to_str()
                 .unwrap_or_default();
+            Ok(get_device_info(&ua)?)
+        }
+        .boxed_local()
+    }
+}
 
-            let params = web::Query::<TilesParams>::from_request(&req, &mut Payload::None).await?;
-            let placement = match &params.placement {
-                Some(v) => {
-                    let placement = v.to_lowercase();
-                    if !validate_placement(&v) {
-                        Err(HandlerErrorKind::Validation(
-                            "Invalid placement parameter".to_owned(),
-                        ))?;
-                    };
-                    Some(placement)
+impl FromRequest for LocationResult {
+    type Config = ();
+    type Error = Error;
+    type Future = LocalBoxFuture<'static, Result<Self, Self::Error>>;
+
+    fn from_request(req: &HttpRequest, _: &mut Payload) -> Self::Future {
+        let req = req.clone();
+        async move {
+            let state = req.app_data::<web::Data<ServerState>>().expect("No State!");
+            let settings = &state.settings;
+            let metrics = Metrics::from(&req);
+
+            let mut addr = None;
+            if let Some(header) = req.headers().get(&*X_FORWARDED_FOR) {
+                if let Ok(value) = header.to_str() {
+                    // Expect a typical X-Forwarded-For where the first address is the
+                    // client's, the front ends should ensure this
+                    addr = value
+                        .split(',')
+                        .next()
+                        .map(|addr| addr.trim())
+                        .and_then(|addr| {
+                            // Fallback to parsing as SocketAddr for when a port
+                            // number's included
+                            addr.parse::<IpAddr>()
+                                .or_else(|_| addr.parse::<SocketAddr>().map(|socket| socket.ip()))
+                                .ok()
+                        });
+                }
+            }
+
+            if let Some(addr) = addr {
+                if state.mmdb.is_available() {
+                    let result = state
+                        .mmdb
+                        .mmdb_locate(addr, &["en".to_owned()], &metrics)
+                        .await?;
+                    if let Some(location) = result {
+                        return Ok(location);
+                    }
                 }
-                None => None,
-            };
-            Ok(Self {
-                country: params.country.clone().map(|v| v.to_uppercase()),
-                placement,
-                ua: ua.to_owned(),
-            })
+            } else {
+                metrics.incr("location.unknown.ip");
+            }
+            Ok(LocationResult::from_header(req.head(), settings, &metrics))
         }
         .boxed_local()
     }
@@ -77,7 +103,3 @@ impl FromRequest for Metrics {
         future::ok(Metrics::from(req)).boxed_local()
     }
 }
-
-fn validate_placement(placement: &str) -> bool {
-    VALID_PLACEMENTS.contains(&placement)
-}