Fix issue with areas to elements mapping

src/area/service.rs

@@ -1,7 +1,7 @@
 use super::Area;
 use crate::{
     area_element::{self, model::AreaElement},
-    element::{self, Element},
+    element::Element,
     element_comment::ElementComment,
     event::Event,
     Error, Result,
@@ -12,6 +12,14 @@ use serde_json::{Map, Value};
 use std::collections::{HashMap, HashSet};
 use time::OffsetDateTime;
 
+// it can take a long time to find area_elements
+// let's say it takes 10 minutes
+// on 7th minute a new element was added by osm sync
+// this new area will be mapped to that place by the sync
+// so we can't assume that insert_bulk will always append to an empty dataset
+// but confilct seems unlikely since the place was added after get_elements_within_geometries queried elements snapshot
+// and if an element was deleted, that's not an issue since we never fully delete elements
+// but wat if an element was moved? It could change its area set... TODO
 pub fn insert(tags: Map<String, Value>, conn: &mut Connection) -> Result<Area> {
     let area = Area::insert(tags, conn)?.ok_or("failed to insert area")?;
     let area_elements =
@@ -35,33 +43,51 @@ pub fn patch_tag(
     patch_tags(id_or_alias, tags, conn)
 }
 
-pub fn patch_tags(id_or_alias: &str, tags: Map<String, Value>, conn: &Connection) -> Result<Area> {
-    let area = Area::select_by_id_or_alias(id_or_alias, conn)?.unwrap();
+fn patch_tags(area_id_or_alias: &str, tags: Map<String, Value>, conn: &Connection) -> Result<Area> {
+    if tags.contains_key("url_alias") {
+        return Err(Error::InvalidInput("url_alias can't be changed".into()));
+    }
+    let area =
+        Area::select_by_id_or_alias(area_id_or_alias, conn)?.ok_or("failed to fetch area")?;
     if tags.contains_key("geo_json") {
-        let area_elements = element::service::find_in_area(&area, conn)?;
+        let mut affected_elements: HashSet<Element> = HashSet::new();
+        for area_element in AreaElement::select_by_area_id(area.id, &conn)? {
+            let element = Element::select_by_id(area_element.element_id, &conn)?.ok_or(format!(
+                "failed to fetch element {}",
+                area_element.element_id,
+            ))?;
+            affected_elements.insert(element);
+        }
         let area = Area::patch_tags(area.id, tags, conn)?.unwrap();
-        element::service::generate_areas_mapping_old(&area_elements, conn)?;
-        let area_elements = element::service::find_in_area(&area, conn)?;
-        element::service::generate_areas_mapping_old(&area_elements, conn)?;
+        let elements_in_new_bounds = area_element::service::get_elements_within_geometries(
+            area.geo_json_geometries()?,
+            &conn,
+        )?;
+        for element in elements_in_new_bounds {
+            affected_elements.insert(element);
+        }
+        let affected_elements: Vec<Element> = affected_elements.into_iter().collect();
+        area_element::service::generate_mapping(&affected_elements, conn)?;
         Ok(area)
     } else {
-        Ok(Area::patch_tags(area.id, tags, conn)?.unwrap())
+        Area::patch_tags(area.id, tags, conn)?.ok_or("failed to fetch area".into())
     }
 }
 
 pub fn remove_tag(area_id_or_alias: &str, tag_name: &str, conn: &mut Connection) -> Result<Area> {
+    if tag_name == "url_alias" {
+        return Err(Error::InvalidInput("url_alias can't be removed".into()));
+    }
     if tag_name == "geo_json" {
-        return Err(Error::InvalidInput("geo_json tag can't be removed".into()));
+        return Err(Error::InvalidInput("geo_json can't be removed".into()));
     }
     let area = Area::select_by_id_or_alias(area_id_or_alias, conn)?.unwrap();
     Ok(Area::remove_tag(area.id, tag_name, conn)?.unwrap())
 }
 
 pub fn soft_delete(area_id_or_alias: &str, conn: &Connection) -> Result<Area> {
     let area = Area::select_by_id_or_alias(area_id_or_alias, conn)?.unwrap();
-    let area_elements = element::service::find_in_area(&area, conn)?;
     let area = Area::set_deleted_at(area.id, Some(OffsetDateTime::now_utc()), conn)?.unwrap();
-    element::service::generate_areas_mapping_old(&area_elements, conn)?;
     Ok(area)
 }
 
@@ -178,7 +204,7 @@ mod test {
     use crate::area_element::model::AreaElement;
     use crate::element::Element;
     use crate::osm::overpass::OverpassElement;
-    use crate::test::{mock_conn, phuket_geo_json};
+    use crate::test::{earth_geo_json, mock_conn, phuket_geo_json};
     use crate::Result;
     use serde_json::{json, Map};
 
@@ -216,39 +242,62 @@ mod test {
     fn patch_tags() -> Result<()> {
         let mut conn = mock_conn();
         let area = Area::insert(Area::mock_tags(), &mut conn)?.unwrap();
-        let mut patch_tags = Map::new();
+        let mut patch_set = Map::new();
         let new_tag_name = "foo";
         let new_tag_value = json!("bar");
-        patch_tags.insert(new_tag_name.into(), new_tag_value.clone());
-        let new_alias = json!("test1");
-        patch_tags.insert("url_alias".into(), new_alias.clone());
-        let area = super::patch_tags(&area.id.to_string(), patch_tags, &mut conn)?;
+        patch_set.insert(new_tag_name.into(), new_tag_value.clone());
+        let area = super::patch_tags(&area.id.to_string(), patch_set, &mut conn)?;
         let db_area = Area::select_by_id(area.id, &conn)?.unwrap();
         assert_eq!(area, db_area);
         assert_eq!(new_tag_value, db_area.tags[new_tag_name]);
-        assert_eq!(new_alias, db_area.tags["url_alias"]);
         Ok(())
     }
 
     #[test]
-    fn patch_tags_should_update_areas_tags() -> Result<()> {
+    fn patch_tags_should_update_area_mappings() -> Result<()> {
         let mut conn = mock_conn();
-        let area_element = OverpassElement {
+        let element_in_phuket = OverpassElement {
             lat: Some(7.979623499157051),
             lon: Some(98.33448362485439),
             ..OverpassElement::mock(1)
         };
-        let area_element = Element::insert(&area_element, &conn)?;
-        let area_element =
-            Element::set_tag(area_element.id, "areas", &json!("[{id:1},{id:2}]"), &conn)?;
+        Element::insert(&element_in_phuket, &conn)?;
+        let element_in_london = OverpassElement {
+            lat: Some(50.0),
+            lon: Some(1.0),
+            ..OverpassElement::mock(2)
+        };
+        Element::insert(&element_in_london, &conn)?;
         let mut tags = Area::mock_tags();
-        tags.insert("geo_json".into(), phuket_geo_json());
+        tags.insert("geo_json".into(), earth_geo_json());
         let area = Area::insert(tags.clone(), &mut conn)?.unwrap();
-        let area = super::patch_tags(&area.id.to_string(), tags, &mut conn)?;
+        let area_element_phuket = AreaElement::insert(area.id, element_in_phuket.id, &conn)?;
+        let area_element_london = AreaElement::insert(area.id, element_in_london.id, &conn)?;
+        tags.insert("geo_json".into(), phuket_geo_json());
+        tags.remove("url_alias");
+        let area = super::patch_tags(&area.id.to_string(), tags.clone(), &mut conn)?;
         let db_area = Area::select_by_id(area.id, &conn)?.unwrap();
         assert_eq!(area, db_area);
-        let db_area_element = Element::select_by_id(area_element.id, &conn)?.unwrap();
-        assert_eq!(1, db_area_element.tag("areas").as_array().unwrap().len());
+        assert!(AreaElement::select_by_id(area_element_phuket.id, &conn)?
+            .ok_or("failed to insert area element")?
+            .deleted_at
+            .is_none());
+        assert!(AreaElement::select_by_id(area_element_london.id, &conn)?
+            .ok_or("failed to insert area element")?
+            .deleted_at
+            .is_some());
+        assert_eq!(2, AreaElement::select_by_area_id(area.id, &conn)?.len());
+        tags.insert("geo_json".into(), earth_geo_json());
+        let area = super::patch_tags(&area.id.to_string(), tags, &mut conn)?;
+        assert_eq!(2, AreaElement::select_by_area_id(area.id, &conn)?.len());
+        assert!(AreaElement::select_by_id(area_element_phuket.id, &conn)?
+            .ok_or("failed to insert area element")?
+            .deleted_at
+            .is_none());
+        assert!(AreaElement::select_by_id(area_element_london.id, &conn)?
+            .ok_or("failed to insert area element")?
+            .deleted_at
+            .is_none());
         Ok(())
     }
 

src/area_element/model.rs

@@ -103,6 +103,27 @@ impl AreaElement {
         Ok(res)
     }
 
+    pub fn select_by_area_id_and_element_id(
+        area_id: i64,
+        element_id: i64,
+        conn: &Connection,
+    ) -> Result<Option<AreaElement>> {
+        let query = format!(
+            r#"
+                SELECT {ALL_COLUMNS}
+                FROM {TABLE}
+                WHERE {COL_AREA_ID} = :area_id AND {COL_ELEMENT_ID} = :element_id;
+            "#
+        );
+        Ok(conn
+            .query_row(
+                &query,
+                named_params! { ":area_id": area_id, ":element_id": element_id },
+                mapper(),
+            )
+            .optional()?)
+    }
+
     pub fn select_by_area_id(area_id: i64, conn: &Connection) -> Result<Vec<AreaElement>> {
         let query = format!(
             r#"

src/area_element/service.rs

@@ -10,30 +10,55 @@ pub struct Res {
     pub has_changes: bool,
 }
 
-pub fn generate_areas_mapping(
-    element: &Element,
-    areas: &Vec<Area>,
-    conn: &Connection,
-) -> Result<Res> {
+pub fn generate_mapping(elements: &[Element], conn: &Connection) -> Result<Res> {
     let mut has_changes = false;
-    let element_areas = element::service::find_areas(element, areas)?;
-    let old_mappings = AreaElement::select_by_element_id(element.id, conn)?;
-    let mut old_area_ids: Vec<i64> = old_mappings.into_iter().map(|it| it.area_id).collect();
-    let mut new_area_ids: Vec<i64> = element_areas.into_iter().map(|it| it.id).collect();
-    old_area_ids.sort();
-    new_area_ids.sort();
-    if new_area_ids != old_area_ids {
-        for old_area_id in &old_area_ids {
-            if !new_area_ids.contains(old_area_id) {
-                AreaElement::set_deleted_at(*old_area_id, Some(OffsetDateTime::now_utc()), conn)?;
+    let areas = Area::select_all(&conn)?;
+    for element in elements {
+        let element_areas = element::service::find_areas(element, &areas)?;
+        let old_mappings = AreaElement::select_by_element_id(element.id, conn)?;
+        let old_mappings: Vec<AreaElement> = old_mappings
+            .into_iter()
+            .filter(|it| it.deleted_at.is_none())
+            .collect();
+        let mut old_area_ids: Vec<i64> = old_mappings.into_iter().map(|it| it.area_id).collect();
+        let mut new_area_ids: Vec<i64> = element_areas.into_iter().map(|it| it.id).collect();
+        old_area_ids.sort();
+        new_area_ids.sort();
+        if new_area_ids != old_area_ids {
+            for old_area_id in &old_area_ids {
+                if !new_area_ids.contains(old_area_id) {
+                    let area_element = AreaElement::select_by_area_id_and_element_id(
+                        *old_area_id,
+                        element.id,
+                        conn,
+                    )?
+                    .unwrap();
+                    AreaElement::set_deleted_at(
+                        area_element.id,
+                        Some(OffsetDateTime::now_utc()),
+                        conn,
+                    )?;
+                }
             }
-        }
-        for new_area_id in new_area_ids {
-            if !old_area_ids.contains(&new_area_id) {
-                AreaElement::insert(new_area_id, element.id, conn)?;
+            for new_area_id in new_area_ids {
+                if !old_area_ids.contains(&new_area_id) {
+                    let area_element = AreaElement::select_by_area_id_and_element_id(
+                        new_area_id,
+                        element.id,
+                        conn,
+                    )?;
+                    match area_element {
+                        Some(area_element) => {
+                            AreaElement::set_deleted_at(area_element.id, None, conn)?;
+                        }
+                        None => {
+                            AreaElement::insert(new_area_id, element.id, conn)?;
+                        }
+                    }
+                }
             }
+            has_changes = true;
         }
-        has_changes = true;
     }
     Ok(Res { has_changes })
 }

src/element/model.rs

@@ -3,7 +3,8 @@ use crate::{osm::overpass::OverpassElement, Error};
 use rusqlite::{named_params, Connection, OptionalExtension, Row};
 use serde::Serialize;
 use serde_json::{Map, Value};
-use std::collections::HashMap;
+use std::hash::Hash;
+use std::hash::Hasher;
 #[cfg(not(test))]
 use std::thread::sleep;
 #[cfg(not(test))]
@@ -12,11 +13,11 @@ use std::time::Instant;
 use time::{format_description::well_known::Rfc3339, OffsetDateTime};
 use tracing::{debug, info};
 
-#[derive(Clone, Debug, PartialEq, Serialize)]
+#[derive(Clone, Debug, Serialize)]
 pub struct Element {
     pub id: i64,
     pub overpass_data: OverpassElement,
-    pub tags: HashMap<String, Value>,
+    pub tags: Map<String, Value>,
     #[serde(with = "time::serde::rfc3339")]
     pub created_at: OffsetDateTime,
     #[serde(with = "time::serde::rfc3339")]
@@ -25,6 +26,20 @@ pub struct Element {
     pub deleted_at: Option<OffsetDateTime>,
 }
 
+impl PartialEq for Element {
+    fn eq(&self, other: &Self) -> bool {
+        self.id == other.id
+    }
+}
+
+impl Eq for Element {}
+
+impl Hash for Element {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        self.id.hash(state);
+    }
+}
+
 const TABLE: &str = "element";
 const ALL_COLUMNS: &str = "rowid, overpass_data, tags, created_at, updated_at, deleted_at";
 const COL_ROWID: &str = "rowid";

src/element/service.rs

@@ -1,6 +1,5 @@
 use super::Element;
 use crate::area::Area;
-use crate::area_element;
 use crate::Result;
 use geo::Contains;
 use geo::LineString;
@@ -9,17 +8,10 @@ use geo::Polygon;
 use rusqlite::Connection;
 use serde::Deserialize;
 use serde::Serialize;
-use serde_json::json;
 use serde_json::Value;
 use time::macros::format_description;
 use time::Date;
 use time::OffsetDateTime;
-use tracing::info;
-
-pub fn find_in_area(area: &Area, conn: &Connection) -> Result<Vec<Element>> {
-    let all_elements: Vec<Element> = Element::select_all_except_deleted(conn)?;
-    filter_by_area(&all_elements, area)
-}
 
 pub fn filter_by_area(all_elements: &Vec<Element>, area: &Area) -> Result<Vec<Element>> {
     let geometries = area.geo_json_geometries()?;
@@ -57,40 +49,6 @@ pub fn filter_by_area(all_elements: &Vec<Element>, area: &Area) -> Result<Vec<El
     Ok(area_elements)
 }
 
-pub fn generate_areas_mapping_old(
-    elements: &Vec<Element>,
-    conn: &Connection,
-) -> Result<Vec<Element>> {
-    let mut res: Vec<Element> = vec![];
-    let all_areas: Vec<Area> = Area::select_all(conn)?;
-    for element in elements {
-        let element_areas = find_areas(element, &all_areas)?;
-        let element_areas = areas_to_areas_tag(element_areas);
-        let element = if element.tag("areas") != &element_areas {
-            info!(
-                element = element.id,
-                old = serde_json::to_string(element.tag("areas"))?,
-                new = serde_json::to_string(&element_areas)?,
-                "Change detected, updating areas tag",
-            );
-            Element::set_tag(element.id, "areas", &element_areas, conn)?
-        } else {
-            info!(element = element.id, "No changes, skipping update");
-            element.clone()
-        };
-        area_element::service::generate_areas_mapping(&element, &all_areas, conn)?;
-        res.push(element);
-    }
-    Ok(res)
-}
-
-fn areas_to_areas_tag(areas: Vec<&Area>) -> Value {
-    let element_areas: Vec<Value> = areas.iter().map(|it| {
-        json!({"id": it.id, "url_alias": it.tags.get("url_alias").unwrap_or(&Value::Null).as_str().unwrap_or_default()})
-    }).collect();
-    Value::Array(element_areas)
-}
-
 pub fn find_areas<'a>(element: &Element, areas: &'a Vec<Area>) -> Result<Vec<&'a Area>> {
     let mut element_areas = vec![];