Merge pull request #9 from kozalosev/bugfix/callbacks

Fix callbacks and caching

docker-compose.yaml

@@ -8,9 +8,12 @@ services:
     environment:
       - TELOXIDE_TOKEN
       - GOOGLE_MAPS_API_KEY
+      - YANDEX_MAPS_GEOCODER_API_KEY
+      - YANDEX_MAPS_PLACES_API_KEY
       - RUST_LOG
       - CACHE_TIME
       - GAPI_MODE
+      - YAPI_MODE
       - MSG_LOC_LIMIT
       - WEBHOOK_URL
     expose:

src/handlers/mod.rs

@@ -45,7 +45,7 @@ pub async fn inline_handler(bot: Bot, q: InlineQuery) -> HandlerResult {
         return Ok(());
     }
 
-    log::info!("Got inline query: {}", q.query);
+    log::info!("Got an inline query: {}", q.query);
     INLINE_COUNTER.inc();
 
     let lang_code = &ensure_lang_code(q.from.id, q.from.language_code.clone());
@@ -96,6 +96,10 @@ pub async fn message_handler(bot: Bot, msg: Message) -> HandlerResult {
 }
 
 pub async fn callback_handler(bot: Bot, q: CallbackQuery) -> HandlerResult {
+    log::info!("Got a callback query for {}: {}",
+        q.from.id,
+        q.data.clone().unwrap_or("<null>".to_string()));
+
     let mut answer = bot.answer_callback_query(q.clone().id);
     if let (Some(chat_id), Some(data)) = (q.chat_id(), q.data) {
         let parts: Vec<&str> = data.split(",").collect();
@@ -105,7 +109,6 @@ pub async fn callback_handler(bot: Bot, q: CallbackQuery) -> HandlerResult {
         let latitude: f64 = parts.get(0).unwrap().parse()?;
         let longitude: f64 = parts.get(1).unwrap().parse()?;
         bot.send_location(chat_id, latitude, longitude).await?;
-
     } else {
         let lang_code = q.from.language_code.unwrap_or(String::default());
         answer.text = Some(t!("error.old-message", locale = lang_code.as_str()));
@@ -127,7 +130,7 @@ async fn cmd_loc_handler(bot: Bot, msg: Message) -> HandlerResult {
 async fn resolve_locations_for_message(msg: &Message) -> Result<Vec<Location>, Box<dyn std::error::Error + Send + Sync>> {
     let text = msg.text().ok_or("no text")?.to_string();
     let from = msg.from().ok_or("no from")?;
-    log::info!("Got message query: {}", text);
+    log::info!("Got a message query: {}", text);
 
     let lang_code = &ensure_lang_code(from.id, from.language_code.clone());
     resolve_locations(text, lang_code).await

src/handlers/senders.rs

@@ -60,6 +60,8 @@ async fn send_locations_keyboard(bot: &Bot, chat_id: ChatId, locations: Vec<Loca
     let mut msg = bot.send_message(chat_id, t!("title.address-list.has-data", locale = lang_code));
     let keyboard = InlineKeyboardMarkup::new(buttons);
     msg.reply_markup = Some(InlineKeyboard(keyboard));
+
+    log::debug!("Send locations keyboard for {}: {:?}", chat_id, *msg);
     msg.await
 }
 

src/loc/cache.rs

@@ -0,0 +1,45 @@
+use http_cache::{CacheMode, HitOrMiss, HttpCache, MokaManager, XCACHELOOKUP};
+use http_cache_reqwest::{Cache, CacheOptions};
+use reqwest::header::HeaderValue;
+use reqwest_middleware::{ClientBuilder, ClientWithMiddleware};
+
+pub fn caching_client() -> ClientWithMiddleware {
+    let client = reqwest::Client::builder()
+        .build().expect("couldn't create an HTTP client");
+    let client = ClientBuilder::new(client)
+        .with(Cache(HttpCache {
+            mode: CacheMode::Default,
+            manager: MokaManager::default(),
+            options: Some(CacheOptions::default()),
+        }))
+        .build();
+    client
+}
+
+pub trait WithCachedResponseCounters {
+    fn cached_resp_counter(&self) -> &prometheus::Counter;
+    fn fetched_resp_counter(&self) -> &prometheus::Counter;
+
+    fn inc_resp_counter(&self, resp: &reqwest::Response) {
+        let resp_counter = if from_cache(resp) {
+            self.cached_resp_counter()
+        } else {
+            self.fetched_resp_counter()
+        };
+        resp_counter.inc();
+    }
+}
+
+fn from_cache(resp: &reqwest::Response) -> bool {
+    log::debug!("Response headers: {:?}", resp.headers());
+
+    let hit = HitOrMiss::HIT.to_string();
+    let predicate = |x: &&HeaderValue| {
+        let value = x.to_str().unwrap_or("");
+        value == hit
+    };
+    resp.headers()
+        .get(XCACHELOOKUP)
+        .filter(predicate)
+        .is_some()
+}

src/loc/google.rs

@@ -1,8 +1,10 @@
 use std::str::FromStr;
 use async_trait::async_trait;
 use once_cell::sync::Lazy;
+use reqwest_middleware::ClientWithMiddleware;
 use strum_macros::EnumString;
-use crate::loc::{Location, LocFinder, LocResult};
+use super::cache::WithCachedResponseCounters;
+use super::{cache, Location, LocFinder, LocResult};
 use crate::metrics;
 
 const FINDER_ENV_API_KEY: &str = "GOOGLE_MAPS_API_KEY";
@@ -27,11 +29,14 @@ pub fn preload_env_vars() {
 }
 
 pub struct GoogleLocFinder {
+    client: ClientWithMiddleware,
     api_key: String,
 
     geocode_req_counter: prometheus::Counter,
     place_req_counter: prometheus::Counter,
     text_req_counter: prometheus::Counter,
+    cached_resp_counter: prometheus::Counter,
+    fetched_resp_counter: prometheus::Counter,
 }
 
 impl GoogleLocFinder {
@@ -41,12 +46,19 @@ impl GoogleLocFinder {
         let place_opts   = base_opts.clone().const_label("API", "place");
         let text_opts    = base_opts.clone().const_label("API", "place-text");
 
+        let resp_opts = prometheus::Opts::new("google_maps_api_responses_total", "count of responses from the Google Maps API split by the source");
+        let from_cache_opts = resp_opts.clone().const_label("source", "cache");
+        let from_remote_opts = resp_opts.const_label("source", "remote");
+
         GoogleLocFinder {
+            client: cache::caching_client(),
             api_key: api_key.to_string(),
 
             geocode_req_counter: metrics::REGISTRY.register_counter("Google Maps API (geocode) requests", geocode_opts),
             place_req_counter:   metrics::REGISTRY.register_counter("Google Maps API (place) requests", place_opts),
             text_req_counter:    metrics::REGISTRY.register_counter("Google Maps API (place, text) requests", text_opts),
+            cached_resp_counter:  metrics::REGISTRY.register_counter("Google Maps API requests", from_cache_opts),
+            fetched_resp_counter: metrics::REGISTRY.register_counter("Google Maps API requests", from_remote_opts),
         }
     }
 
@@ -76,11 +88,13 @@ impl GoogleLocFinder {
 
         let url = format!("https://maps.googleapis.com/maps/api/geocode/json?key={}&address={}&language={}&region={}",
                           self.api_key, address, lang_code, lang_code);
-        let resp = reqwest::get(url).await?.json::<serde_json::Value>().await?;
+        let resp = self.client.get(url).send().await?;
+        self.inc_resp_counter(&resp);
 
-        log::info!("response from Google Maps Geocoding API: {}", resp);
+        let json = resp.json::<serde_json::Value>().await?;
+        log::info!("response from Google Maps Geocoding API: {json}");
 
-        let results = resp["results"].as_array().unwrap().iter()
+        let results = json["results"].as_array().unwrap().iter()
             .filter_map(map_resp)
             .collect();
         Ok(results)
@@ -91,11 +105,13 @@ impl GoogleLocFinder {
 
         let url = format!("https://maps.googleapis.com/maps/api/place/findplacefromtext/json?key={}&input={}&inputtype=textquery&language={}&fields=formatted_address,geometry,name",
                           self.api_key, address, lang_code);
-        let resp = reqwest::get(url).await?.json::<serde_json::Value>().await?;
+        let resp = self.client.get(url).send().await?;
+        self.inc_resp_counter(&resp);
 
-        log::info!("response from Google Maps Find Place API: {}", resp);
+        let json = resp.json::<serde_json::Value>().await?;
+        log::info!("response from Google Maps Find Place API: {json}");
 
-        let results: Vec<Location> = resp["candidates"].as_array().unwrap().iter()
+        let results: Vec<Location> = json["candidates"].as_array().unwrap().iter()
             .filter_map(map_resp)
             .collect();
 
@@ -107,11 +123,13 @@ impl GoogleLocFinder {
 
         let url = format!("https://maps.googleapis.com/maps/api/place/textsearch/json?key={}&query={}&language={}&region={}",
                           self.api_key, address, lang_code, lang_code);
-        let resp = reqwest::get(url).await?.json::<serde_json::Value>().await?;
+        let resp = self.client.get(url).send().await?;
+        self.inc_resp_counter(&resp);
 
-        log::info!("response from Google Maps Text Search API: {}", resp);
+        let json = resp.json::<serde_json::Value>().await?;
+        log::info!("response from Google Maps Text Search API: {json}");
 
-        let results: Vec<Location> = resp["results"].as_array().unwrap().iter()
+        let results: Vec<Location> = json["results"].as_array().unwrap().iter()
             .filter_map(map_resp)
             .collect();
 
@@ -131,6 +149,16 @@ impl LocFinder for GoogleLocFinder {
     }
 }
 
+impl WithCachedResponseCounters for GoogleLocFinder {
+    fn cached_resp_counter(&self) -> &prometheus::Counter {
+        &self.cached_resp_counter
+    }
+
+    fn fetched_resp_counter(&self) -> &prometheus::Counter {
+        &self.fetched_resp_counter
+    }
+}
+
 fn map_resp(v: &serde_json::Value) -> Option<Location> {
     let address = Some(v["formatted_address"].as_str()?.to_string());
 

src/loc/mod.rs

@@ -4,6 +4,7 @@ use async_trait::async_trait;
 pub mod google;
 pub mod yandex;
 pub mod osm;
+pub mod cache;
 
 #[cfg(test)]
 mod test;

src/loc/osm.rs

@@ -1,10 +1,9 @@
 use async_trait::async_trait;
 use reqwest::header::{ACCEPT_LANGUAGE, USER_AGENT};
-use reqwest_middleware::{ClientBuilder, ClientWithMiddleware};
-use http_cache::{HitOrMiss, XCACHELOOKUP};
-use http_cache_reqwest::{Cache, CacheMode, MokaManager, HttpCache, CacheOptions};
+use reqwest_middleware::ClientWithMiddleware;
 use prometheus::Opts;
-use super::{LocFinder, LocResult, Location};
+use super::cache::WithCachedResponseCounters;
+use super::{cache, LocFinder, LocResult, Location};
 use crate::metrics;
 
 pub struct OpenStreetMapLocFinder {
@@ -17,24 +16,14 @@ pub struct OpenStreetMapLocFinder {
 
 impl OpenStreetMapLocFinder {
     pub fn new() -> OpenStreetMapLocFinder {
-        let client = reqwest::Client::builder()
-            .build().expect("couldn't create an HTTP client");
-        let client = ClientBuilder::new(client)
-            .with(Cache(HttpCache {
-                mode: CacheMode::Default,
-                manager: MokaManager::default(),
-                options: Some(CacheOptions::default()),
-            }))
-            .build();
-
         let api_req_opts = Opts::new("open_street_map_api_requests_total", "count of requests to the OpenStreetMap API");
 
         let resp_opts = Opts::new("open_street_map_api_responses_total", "count of responses from the OpenStreetMap API split by the source");
         let from_cache_opts = resp_opts.clone().const_label("source", "cache");
         let from_remote_opts = resp_opts.const_label("source", "remote");
 
         OpenStreetMapLocFinder {
-            client,
+            client: cache::caching_client(),
 
             api_req_counter: metrics::REGISTRY.register_counter("OpenStreetMap API requests", api_req_opts),
             cached_resp_counter: metrics::REGISTRY.register_counter("OpenStreetMap API requests", from_cache_opts),
@@ -52,15 +41,8 @@ impl LocFinder for OpenStreetMapLocFinder {
         let resp = self.client.get(url)
             .header(USER_AGENT, "kozalosev/LocPlaceBot")
             .header(ACCEPT_LANGUAGE, lang_code)
-            .send()
-            .await?;
-
-        let resp_counter = resp.headers()
-            .get(XCACHELOOKUP)
-            .filter(|x| x.to_str().unwrap_or("") == HitOrMiss::HIT.to_string())
-            .map(|_| &self.cached_resp_counter)
-            .unwrap_or(&self.fetched_resp_counter);
-        resp_counter.inc();
+            .send().await?;
+        self.inc_resp_counter(&resp);
 
         let json = resp.json::<serde_json::Value>().await?;
         log::info!("response from Open Street Map Nominatim API: {json}");
@@ -72,6 +54,16 @@ impl LocFinder for OpenStreetMapLocFinder {
     }
 }
 
+impl WithCachedResponseCounters for OpenStreetMapLocFinder {
+    fn cached_resp_counter(&self) -> &prometheus::Counter {
+        &self.cached_resp_counter
+    }
+
+    fn fetched_resp_counter(&self) -> &prometheus::Counter {
+        &self.fetched_resp_counter
+    }
+}
+
 fn map_resp(v: &serde_json::Value) -> Option<Location> {
     let address = Some(v["display_name"].as_str()?.to_string());