Merge pull request #567 from blockscout/lymanrekolev/stats/fix-missin…

…g-points

Fix fill missing point bug

stats/stats-server/src/read_service.rs

@@ -1,7 +1,4 @@
-use crate::{
-    charts::Charts,
-    serializers::{fill_missing_points, serialize_line_points},
-};
+use crate::{charts::Charts, serializers::serialize_line_points};
 use async_trait::async_trait;
 use chrono::NaiveDate;
 use sea_orm::{DatabaseConnection, DbErr};
@@ -28,7 +25,10 @@ impl ReadService {
 fn map_read_error(err: ReadError) -> Status {
     match &err {
         ReadError::NotFound(_) => tonic::Status::not_found(err.to_string()),
-        _ => tonic::Status::internal(err.to_string()),
+        _ => {
+            tracing::error!(err = ?err, "internal read error");
+            tonic::Status::internal(err.to_string())
+        }
     }
 }
 
@@ -87,9 +87,9 @@ impl StatsService for ReadService {
             .from
             .and_then(|date| NaiveDate::from_str(&date).ok());
         let to = request.to.and_then(|date| NaiveDate::from_str(&date).ok());
-        let mut data = stats::get_chart_data(&self.db, &request.name, from, to)
+        let policy = Some(chart_info.chart.missing_date_policy());
+        let mut data = stats::get_chart_data(&self.db, &request.name, from, to, policy)
             .await
-            .map(|data| fill_missing_points(data, chart_info.chart.missing_date_policy(), from, to))
             .map_err(map_read_error)?;
 
         if chart_info.chart.drop_last_point() {

stats/stats-server/src/serializers.rs

@@ -1,5 +1,4 @@
-use chrono::{Duration, NaiveDate};
-use stats::{DateValue, MissingDatePolicy};
+use stats::DateValue;
 use stats_proto::blockscout::stats::v1::Point;
 
 pub fn serialize_line_points(data: Vec<DateValue>) -> Vec<Point> {
@@ -10,179 +9,3 @@ pub fn serialize_line_points(data: Vec<DateValue>) -> Vec<Point> {
         })
         .collect()
 }
-
-pub fn fill_missing_points(
-    data: Vec<DateValue>,
-    policy: MissingDatePolicy,
-    from: Option<NaiveDate>,
-    to: Option<NaiveDate>,
-) -> Vec<DateValue> {
-    let from = from.or(data.first().map(|v| v.date));
-    let to = to.or(data.last().map(|v| v.date));
-    let (from, to) = match (from, to) {
-        (Some(from), Some(to)) if from <= to => (from, to),
-        _ => return data,
-    };
-
-    match policy {
-        MissingDatePolicy::FillZero => fill_zeros(data, from, to),
-        MissingDatePolicy::FillPrevious => fill_previous(data, from, to),
-    }
-}
-
-fn fill_zeros(data: Vec<DateValue>, from: NaiveDate, to: NaiveDate) -> Vec<DateValue> {
-    let n = (to - from).num_days() as usize;
-    let mut new_data: Vec<DateValue> = Vec::with_capacity(n);
-
-    let mut current_date = from;
-    let mut i = 0;
-    while current_date <= to {
-        let maybe_value_exists = data.get(i).filter(|&v| v.date.eq(&current_date));
-
-        let value = match maybe_value_exists {
-            Some(value) => {
-                i += 1;
-                value.clone()
-            }
-            None => DateValue::zero(current_date),
-        };
-        new_data.push(value);
-        current_date += Duration::days(1);
-    }
-
-    new_data
-}
-
-fn fill_previous(data: Vec<DateValue>, from: NaiveDate, to: NaiveDate) -> Vec<DateValue> {
-    let n = (to - from).num_days() as usize;
-    let mut new_data: Vec<DateValue> = Vec::with_capacity(n);
-    let mut current_date = from;
-    let mut i = 0;
-    while current_date <= to {
-        let maybe_value_exists = data.get(i).filter(|&v| v.date.eq(&current_date));
-        let value = match maybe_value_exists {
-            Some(value) => {
-                i += 1;
-                value.clone()
-            }
-            None => new_data
-                .last()
-                .map(|value| DateValue {
-                    date: current_date,
-                    value: value.value.clone(),
-                })
-                .unwrap_or_else(|| DateValue::zero(current_date)),
-        };
-        new_data.push(value);
-        current_date += Duration::days(1);
-    }
-    new_data
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-    use chrono::NaiveDate;
-    use pretty_assertions::assert_eq;
-    use stats::DateValue;
-
-    fn d(date: &str) -> NaiveDate {
-        date.parse().unwrap()
-    }
-    fn v(date: &str, value: &str) -> DateValue {
-        DateValue {
-            date: d(date),
-            value: value.to_string(),
-        }
-    }
-
-    #[test]
-    fn fill_zeros_works() {
-        for (data, expected, from, to) in [
-            (vec![], vec![], None, None),
-            (vec![], vec![], Some(d("2100-01-01")), None),
-            (
-                vec![],
-                vec![v("2100-01-01", "0")],
-                Some(d("2100-01-01")),
-                Some(d("2100-01-01")),
-            ),
-            (
-                vec![v("2022-01-01", "01")],
-                vec![v("2022-01-01", "01")],
-                Some(d("2022-01-01")),
-                Some(d("2022-01-01")),
-            ),
-            (
-                vec![
-                    v("2022-08-20", "20"),
-                    v("2022-08-22", "22"),
-                    v("2022-08-23", "23"),
-                    v("2022-08-25", "25"),
-                ],
-                vec![
-                    v("2022-08-18", "0"),
-                    v("2022-08-19", "0"),
-                    v("2022-08-20", "20"),
-                    v("2022-08-21", "0"),
-                    v("2022-08-22", "22"),
-                    v("2022-08-23", "23"),
-                    v("2022-08-24", "0"),
-                    v("2022-08-25", "25"),
-                    v("2022-08-26", "0"),
-                    v("2022-08-27", "0"),
-                ],
-                Some(d("2022-08-18")),
-                Some(d("2022-08-27")),
-            ),
-        ] {
-            let actual = fill_missing_points(data, MissingDatePolicy::FillZero, from, to);
-            assert_eq!(expected, actual)
-        }
-    }
-
-    #[test]
-    fn fill_previous_works() {
-        for (data, expected, from, to) in [
-            (vec![], vec![], None, None),
-            (vec![], vec![], Some(d("2100-01-01")), None),
-            (
-                vec![],
-                vec![v("2100-01-01", "0")],
-                Some(d("2100-01-01")),
-                Some(d("2100-01-01")),
-            ),
-            (
-                vec![v("2022-01-01", "01")],
-                vec![v("2022-01-01", "01")],
-                Some(d("2022-01-01")),
-                Some(d("2022-01-01")),
-            ),
-            (
-                vec![
-                    v("2022-08-20", "20"),
-                    v("2022-08-22", "22"),
-                    v("2022-08-23", "23"),
-                    v("2022-08-25", "25"),
-                ],
-                vec![
-                    v("2022-08-18", "0"),
-                    v("2022-08-19", "0"),
-                    v("2022-08-20", "20"),
-                    v("2022-08-21", "20"),
-                    v("2022-08-22", "22"),
-                    v("2022-08-23", "23"),
-                    v("2022-08-24", "23"),
-                    v("2022-08-25", "25"),
-                    v("2022-08-26", "25"),
-                    v("2022-08-27", "25"),
-                ],
-                Some(d("2022-08-18")),
-                Some(d("2022-08-27")),
-            ),
-        ] {
-            let actual = fill_missing_points(data, MissingDatePolicy::FillPrevious, from, to);
-            assert_eq!(expected, actual);
-        }
-    }
-}

stats/stats/src/charts/lines/gas_used_growth.rs

@@ -101,9 +101,8 @@ impl crate::Chart for GasUsedGrowth {
 
 #[cfg(test)]
 mod tests {
-    use crate::tests::simple_test::simple_test_chart;
-
     use super::GasUsedGrowth;
+    use crate::tests::simple_test::{ranged_test_chart, simple_test_chart};
 
     #[tokio::test]
     #[ignore = "needs database to run"]
@@ -125,4 +124,32 @@ mod tests {
         )
         .await;
     }
+
+    #[tokio::test]
+    #[ignore = "needs database to run"]
+    async fn ranged_update_gas_used_growth() {
+        let chart = GasUsedGrowth::default();
+        let value_2022_11_12 = "250680";
+        ranged_test_chart(
+            "ranged_update_gas_used_growth",
+            chart,
+            vec![
+                ("2022-11-20", value_2022_11_12),
+                ("2022-11-21", value_2022_11_12),
+                ("2022-11-22", value_2022_11_12),
+                ("2022-11-23", value_2022_11_12),
+                ("2022-11-24", value_2022_11_12),
+                ("2022-11-25", value_2022_11_12),
+                ("2022-11-26", value_2022_11_12),
+                ("2022-11-27", value_2022_11_12),
+                ("2022-11-28", value_2022_11_12),
+                ("2022-11-29", value_2022_11_12),
+                ("2022-11-30", value_2022_11_12),
+                ("2022-12-01", "288350"),
+            ],
+            "2022-11-20".parse().unwrap(),
+            "2022-12-01".parse().unwrap(),
+        )
+        .await;
+    }
 }

stats/stats/src/charts/lines/new_blocks.rs

@@ -115,7 +115,7 @@ mod tests {
 
         // Note that update is not full, therefore there is no entry with date `2022-11-09`
         updater.update(&db, &blockscout, false).await.unwrap();
-        let data = get_chart_data(&db, updater.name(), None, None)
+        let data = get_chart_data(&db, updater.name(), None, None, None)
             .await
             .unwrap();
         let expected = vec![
@@ -136,7 +136,7 @@ mod tests {
 
         // note that update is full, therefore there is entry with date `2022-11-09`
         updater.update(&db, &blockscout, true).await.unwrap();
-        let data = get_chart_data(&db, updater.name(), None, None)
+        let data = get_chart_data(&db, updater.name(), None, None, None)
             .await
             .unwrap();
         let expected = vec![
@@ -171,7 +171,7 @@ mod tests {
         updater.create(&db).await.unwrap();
 
         updater.update(&db, &blockscout, true).await.unwrap();
-        let data = get_chart_data(&db, updater.name(), None, None)
+        let data = get_chart_data(&db, updater.name(), None, None, None)
             .await
             .unwrap();
         let expected = vec![
@@ -243,7 +243,7 @@ mod tests {
         .unwrap();
 
         updater.update(&db, &blockscout, false).await.unwrap();
-        let data = get_chart_data(&db, updater.name(), None, None)
+        let data = get_chart_data(&db, updater.name(), None, None, None)
             .await
             .unwrap();
         let expected = vec![

stats/stats/src/charts/lines/new_txns.rs

@@ -78,7 +78,7 @@ impl crate::Chart for NewTxns {
 #[cfg(test)]
 mod tests {
     use super::NewTxns;
-    use crate::tests::simple_test::simple_test_chart;
+    use crate::tests::simple_test::{ranged_test_chart, simple_test_chart};
 
     #[tokio::test]
     #[ignore = "needs database to run"]
@@ -100,4 +100,43 @@ mod tests {
         )
         .await;
     }
+
+    #[tokio::test]
+    #[ignore = "needs database to run"]
+    async fn ranged_update_new_txns() {
+        let chart = NewTxns::default();
+        ranged_test_chart(
+            "ranged_update_new_txns",
+            chart,
+            vec![
+                ("2022-11-08", "0"),
+                ("2022-11-09", "5"),
+                ("2022-11-10", "12"),
+                ("2022-11-11", "14"),
+                ("2022-11-12", "5"),
+                ("2022-11-13", "0"),
+                ("2022-11-14", "0"),
+                ("2022-11-15", "0"),
+                ("2022-11-16", "0"),
+                ("2022-11-17", "0"),
+                ("2022-11-18", "0"),
+                ("2022-11-19", "0"),
+                ("2022-11-20", "0"),
+                ("2022-11-21", "0"),
+                ("2022-11-22", "0"),
+                ("2022-11-23", "0"),
+                ("2022-11-24", "0"),
+                ("2022-11-25", "0"),
+                ("2022-11-26", "0"),
+                ("2022-11-27", "0"),
+                ("2022-11-28", "0"),
+                ("2022-11-29", "0"),
+                ("2022-11-30", "0"),
+                ("2022-12-01", "5"),
+            ],
+            "2022-11-08".parse().unwrap(),
+            "2022-12-01".parse().unwrap(),
+        )
+        .await;
+    }
 }

stats/stats/src/charts/updater/dependent.rs

@@ -32,7 +32,7 @@ where
             "updating parent"
         );
         parent.update_with_mutex(db, blockscout, force_full).await?;
-        let data = get_chart_data(db, parent.name(), None, None).await?;
+        let data = get_chart_data(db, parent.name(), None, None, None).await?;
         Ok(data)
     }
 

stats/stats/src/lib.rs

@@ -1,4 +1,5 @@
 mod charts;
+mod missing_date;
 mod read;
 
 pub mod metrics;