add some doc comments

yopa-web/src/main.rs

@@ -10,7 +10,7 @@ use std::ops::Deref;
 use std::path::PathBuf;
 
 use actix_session::CookieSession;
-use actix_web::{App, HttpResponse, HttpServer, web};
+use actix_web::{web, App, HttpResponse, HttpServer};
 use actix_web_static_files;
 use actix_web_static_files::ResourceFiles as StaticFiles;
 use clap::Arg;
@@ -108,29 +108,40 @@ const DEF_ADDRESS: &str = "127.0.0.1:8080";
 async fn main() -> std::io::Result<()> {
     let def_addr_help = format!("Set custom server address, default {}", DEF_ADDRESS);
     let app = clap::App::new("yopa")
-        .arg(Arg::with_name("version")
+        .arg(
-            .long("version")
+            Arg::with_name("version")
-            .short("V")
+                .long("version")
-            .help("Show version and exit"))
+                .short("V")
-        .arg(Arg::with_name("verbose")
+                .help("Show version and exit"),
-            .short("v")
+        )
-            .multiple(true)
+        .arg(
-            .help("Increase verbosity of logging"))
+            Arg::with_name("verbose")
-        .arg(Arg::with_name("address")
+                .short("v")
-            .short("a")
+                .multiple(true)
-            .takes_value(true)
+                .help("Increase verbosity of logging"),
-            .help(&def_addr_help))
+        )
-        .arg(Arg::with_name("json")
+        .arg(
-            .long("json")
+            Arg::with_name("address")
-            .short("j")
+                .short("a")
-            .help("Use JSON storage format instead of binary"))
+                .takes_value(true)
-        .arg(Arg::with_name("file")
+                .help(&def_addr_help),
-            .help("Database file to use"));
+        )
+        .arg(
+            Arg::with_name("json")
+                .long("json")
+                .short("j")
+                .help("Use JSON storage format instead of binary"),
+        )
+        .arg(Arg::with_name("file").help("Database file to use"));
 
     let matches = app.get_matches();
 
     if matches.is_present("version") {
-        println!("yopa-web {}, using yopa {}", env!("CARGO_PKG_VERSION"), yopa::VERSION);
+        println!(
+            "yopa-web {}, using yopa {}",
+            env!("CARGO_PKG_VERSION"),
+            yopa::VERSION
+        );
         return Ok(());
     }
 
@@ -147,7 +158,11 @@ async fn main() -> std::io::Result<()> {
 
     let json = matches.is_present("json");
 
-    let file = matches.value_of("file").unwrap_or(if json { "yopa-store.json" } else { "yopa-store.dat" });
+    let file = matches.value_of("file").unwrap_or(if json {
+        "yopa-store.json"
+    } else {
+        "yopa-store.dat"
+    });
 
     let file_path = if file.starts_with('/') {
         std::env::current_dir()?.join(file)
@@ -157,14 +172,12 @@ async fn main() -> std::io::Result<()> {
 
     debug!("Using database file: {}", file_path.display());
 
-    let mut store = if json {
+    let store = if json {
         Storage::new_json(file_path)
     } else {
         Storage::new_bincode(file_path)
-    };
+    }
-
+    .map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e))?;
-    store.load()
-        .map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e))?;
 
     let yopa_store: YopaStoreWrapper = web::Data::new(tokio::sync::RwLock::new(store));
 
@@ -223,7 +236,7 @@ async fn main() -> std::io::Result<()> {
                 HttpResponse::NotFound().body("File or endpoint not found")
             }))
     })
-        .bind(server_address)?
+    .bind(server_address)?
-        .run()
+    .run()
-        .await
+    .await
 }

yopa-web/src/routes/models/object.rs

@@ -1,5 +1,5 @@
 use actix_session::Session;
-use actix_web::{web, Responder, HttpResponse};
+use actix_web::{web, HttpResponse, Responder};
 use serde::{Deserialize, Serialize};
 
 use yopa::model::{ObjectModel, PropertyModel};

yopa-web/src/routes/objects.rs

@@ -85,7 +85,7 @@ fn prepare_object_create_data(rg: &Storage, model_id: ID) -> actix_web::Result<O
                 .get_property_models_for_parents(prop_object_ids)
                 .collect(),
         },
-        objects: rg.get_objects_of_type(related_ids).collect(),
+        objects: rg.get_objects_of_types(related_ids).collect(),
     })
 }
 

yopa-web/src/utils.rs

@@ -46,9 +46,7 @@ pub enum ActixErrorWrapper {
     Storage(#[from] StorageError),
 }
 
-impl ResponseError for ActixErrorWrapper {
+impl ResponseError for ActixErrorWrapper {}
-
-}
 
 pub trait StorageErrorIntoResponseError<T> {
     fn err_to_500(self) -> Result<T, ActixErrorWrapper>;

yopa/src/lib.rs

@@ -1,30 +1,29 @@
 #[macro_use]
-extern crate serde_json;
-#[macro_use]
 extern crate log;
+#[macro_use]
+extern crate serde_json;
 
 use std::borrow::Cow;
 use std::collections::HashMap;
+use std::fs::OpenOptions;
+use std::io::{BufReader, BufWriter};
+use std::path::{Path, PathBuf};
 
 use itertools::Itertools;
 use serde::{Deserialize, Serialize};
 use thiserror::Error;
 
-use crate::model::{PropertyModel, RelationModel};
 use cool::{map_drain_filter, KVVecToKeysOrValues};
+pub use data::TypedValue;
 use id::next_id;
 pub use id::ID;
 use insert::InsertObj;
 use insert::InsertValue;
+pub use model::DataType;
 use model::ObjectModel;
 
-use crate::data::Object;
+use crate::model::{PropertyModel, RelationModel};
 use crate::update::{UpdateObj, UpsertValue};
-pub use data::TypedValue;
-pub use model::DataType;
-use std::path::{PathBuf, Path};
-use std::fs::OpenOptions;
-use std::io::{BufReader, BufWriter};
 
 mod cool;
 pub mod data;
@@ -37,7 +36,7 @@ mod serde_map_as_list;
 #[cfg(test)]
 mod tests;
 
-pub const VERSION : &'static str = env!("CARGO_PKG_VERSION");
+pub const VERSION: &'static str = env!("CARGO_PKG_VERSION");
 
 /// Stupid storage with naive inefficient file persistence
 #[derive(Debug, Default, Serialize, Deserialize, Clone)]
@@ -62,26 +61,26 @@ pub struct Storage {
 
 #[derive(Debug, Clone)]
 pub struct StoreOpts {
-    file : Option<PathBuf>,
+    file: Option<PathBuf>,
-    file_format: FileEncoding
+    file_format: FileEncoding,
 }
 
 impl Default for StoreOpts {
     fn default() -> Self {
         Self {
             file: None,
-            file_format: FileEncoding::JSON
+            file_format: FileEncoding::JSON,
         }
     }
 }
 
-#[derive(Debug,Clone,Copy)]
+#[derive(Debug, Clone, Copy)]
 pub enum FileEncoding {
     JSON,
     BINCODE,
 }
 
-#[derive(Debug,Error)]
+#[derive(Debug, Error)]
 pub enum StorageError {
     #[error("Referenced {0} does not exist")]
     NotExist(Cow<'static, str>),
@@ -103,29 +102,39 @@ impl Storage {
         Self::default()
     }
 
-    pub fn new_json(file : impl AsRef<Path>) -> Self {
+    /// Create a new instance backed by a JSON file. The file is created if needed.
+    /// The backing file can be read or edited by any text editor.
+    pub fn new_json(file: impl AsRef<Path>) -> Result<Self, StorageError> {
         let mut s = Self::new();
         s.opts.file_format = FileEncoding::JSON;
         s.opts.file = Some(file.as_ref().to_path_buf());
-        s
+        s.load()?;
+        Ok(s)
     }
 
-    pub fn new_bincode(file : impl AsRef<Path>) -> Self {
+    /// Create a new instance backed by a BinCode file. The file is created if needed.
+    /// This format has higher data density and faster save/load times,
+    /// but cannot be easily read outside yopa.
+    pub fn new_bincode(file: impl AsRef<Path>) -> Result<Self, StorageError> {
         let mut s = Self::new();
         s.opts.file_format = FileEncoding::BINCODE;
         s.opts.file = Some(file.as_ref().to_path_buf());
-        s
+        s.load()?;
+        Ok(s)
     }
 
-    pub fn set_file(&mut self, file : impl AsRef<Path>, format : FileEncoding) {
+    /// Set backing file and its encoding
+    pub fn set_file(&mut self, file: impl AsRef<Path>, format: FileEncoding) {
         self.opts.file_format = format;
         self.opts.file = Some(file.as_ref().to_path_buf());
     }
 
+    /// Unset backing file
     pub fn unset_file(&mut self) {
         self.opts.file = None;
     }
 
+    /// Manually load from the backing file.
     pub fn load(&mut self) -> Result<(), StorageError> {
         match &self.opts.file {
             None => {
@@ -136,20 +145,16 @@ impl Storage {
 
                 if !path.exists() {
                     warn!("File does not exist, skip load.");
-                    return Ok(())
+                    return Ok(());
                 }
 
                 let f = OpenOptions::new().read(true).open(&path)?;
 
                 let reader = BufReader::new(f);
 
-                let parsed : Self = match self.opts.file_format {
+                let parsed: Self = match self.opts.file_format {
-                    FileEncoding::JSON => {
+                    FileEncoding::JSON => serde_json::from_reader(reader)?,
-                        serde_json::from_reader(reader)?
+                    FileEncoding::BINCODE => bincode::deserialize_from(reader)?,
-                    }
-                    FileEncoding::BINCODE => {
-                        bincode::deserialize_from(reader)?
-                    }
                 };
 
                 let opts = std::mem::replace(&mut self.opts, StoreOpts::default());
@@ -161,6 +166,8 @@ impl Storage {
         Ok(())
     }
 
+    /// Persist to the backing file.
+    /// This must be called after each transaction that should be saved.
     pub fn persist(&mut self) -> Result<(), StorageError> {
         match &self.opts.file {
             None => {
@@ -170,16 +177,18 @@ impl Storage {
             Some(path) => {
                 debug!("Persist to: {}", path.display());
 
-                let f = OpenOptions::new().write(true).create(true).truncate(true).open(&path)?;
+                let f = OpenOptions::new()
+                    .write(true)
+                    .create(true)
+                    .truncate(true)
+                    .open(&path)?;
                 let writer = BufWriter::new(f);
 
                 match self.opts.file_format {
                     FileEncoding::JSON => {
                         serde_json::to_writer(writer, self)?;
                     }
-                    FileEncoding::BINCODE => {
+                    FileEncoding::BINCODE => bincode::serialize_into(writer, self)?,
-                        bincode::serialize_into(writer, self)?
-                    }
                 };
             }
         }
@@ -187,7 +196,190 @@ impl Storage {
         Ok(())
     }
 
-    /// Define a data object
+    /// Get textual description of a model (of any kind), suitable for error messages or logging
+    pub fn describe_model(&self, id: ID) -> String {
+        if let Some(x) = self.obj_models.get(&id) {
+            x.to_string()
+        } else if let Some(x) = self.rel_models.get(&id) {
+            x.to_string()
+        } else if let Some(x) = self.prop_models.get(&id) {
+            x.to_string()
+        } else {
+            id.to_string()
+        }
+    }
+
+    /// Get model name. Accepts ID of object, relation or property models.
+    pub fn get_model_name(&self, id: ID) -> &str {
+        if let Some(x) = self.obj_models.get(&id) {
+            &x.name
+        } else if let Some(x) = self.rel_models.get(&id) {
+            &x.name
+        } else if let Some(x) = self.prop_models.get(&id) {
+            &x.name
+        } else {
+            "???"
+        }
+    }
+
+    //region Model queries
+
+    /// Iterate all object models
+    pub fn get_object_models(&self) -> impl Iterator<Item = &ObjectModel> {
+        self.obj_models.values()
+    }
+
+    /// Get an object model by ID
+    pub fn get_object_model(&self, model_id: ID) -> Option<&ObjectModel> {
+        self.obj_models.get(&model_id)
+    }
+
+    /// Get a relation model by ID
+    pub fn get_relation_model(&self, model_id: ID) -> Option<&RelationModel> {
+        self.rel_models.get(&model_id)
+    }
+
+    /// Get a property model by ID
+    pub fn get_property_model(&self, model_id: ID) -> Option<&PropertyModel> {
+        self.prop_models.get(&model_id)
+    }
+
+    /// Get all property models grouped by the parent object ID.
+    pub fn get_grouped_prop_models(&self) -> HashMap<ID, Vec<&PropertyModel>> {
+        self.prop_models
+            .values()
+            .into_group_map_by(|model| model.object)
+    }
+
+    /// Get property models belonging to a group of parent IDs,
+    /// grouped by the parent ID.
+    pub fn get_grouped_prop_models_for_parents(
+        &self,
+        parent_model_ids: Vec<ID>,
+    ) -> HashMap<ID, Vec<&PropertyModel>> {
+        self.prop_models
+            .values()
+            .filter(|p| parent_model_ids.contains(&p.object))
+            .into_group_map_by(|model| model.object)
+    }
+
+    /// Iterate relation models attached to an object model
+    pub fn get_relation_models_for_object_model(
+        &self,
+        model_id: ID,
+    ) -> impl Iterator<Item = &RelationModel> {
+        self.rel_models
+            .values()
+            .filter(move |model| model.object == model_id)
+    }
+
+    /// Iterate property models attached to an object or relation model
+    pub fn get_property_models_for_parents(
+        &self,
+        parents: Vec<ID>,
+    ) -> impl Iterator<Item = &PropertyModel> {
+        self.prop_models
+            .values()
+            .filter(move |model| parents.contains(&model.object))
+    }
+
+    /// Get all relation models, grouped by their source object model ID
+    pub fn get_grouped_relation_models(&self) -> HashMap<ID, Vec<&RelationModel>> {
+        self.rel_models
+            .values()
+            .into_group_map_by(|model| model.object)
+    }
+
+    /// Get reciprocal relation models, grouped by their destination model ID
+    pub fn get_grouped_reciprocal_relation_models(&self) -> HashMap<ID, Vec<&RelationModel>> {
+        self.rel_models
+            .values()
+            .into_group_map_by(|model| model.related)
+    }
+
+    //endregion Model queries
+
+    //region Data queries
+
+    /// Get all relation for an object
+    pub fn get_relations_for_object(&self, object_id: ID) -> impl Iterator<Item = &data::Relation> {
+        self.relations
+            .values()
+            .filter(move |rel| rel.object == object_id)
+    }
+
+    /// Get all reciprocal relation for an object
+    pub fn get_reciprocal_relations_for_object(
+        &self,
+        object_id: ID,
+    ) -> impl Iterator<Item = &data::Relation> {
+        self.relations
+            .values()
+            .filter(move |rel| rel.related == object_id)
+    }
+
+    /// Get values attached to a parent object or relation
+    pub fn get_values_for_object(&self, object_id: ID) -> impl Iterator<Item = &data::Value> {
+        self.values
+            .values()
+            .filter(move |prop| prop.object == object_id)
+    }
+
+    /// Get values belonging to a list of objects or relations,
+    /// grouped by the parent entity ID.
+    pub fn get_grouped_values_for_objects(
+        &self,
+        parents: Vec<ID>,
+    ) -> HashMap<ID, Vec<&data::Value>> {
+        self.values
+            .values()
+            .filter(move |prop| parents.contains(&prop.object))
+            .into_group_map_by(|model| model.object)
+    }
+
+    /// Get all objects belonging to a model.
+    ///
+    /// Use `get_objects_of_types` to specify more than one model
+    pub fn get_objects_of_type(&self, model_id: ID) -> impl Iterator<Item = &data::Object> {
+        self.objects
+            .values()
+            .filter(move |object| object.model == model_id)
+    }
+
+    /// Get all objects belonging to one of several models
+    pub fn get_objects_of_types(&self, model_ids: Vec<ID>) -> impl Iterator<Item = &data::Object> {
+        self.objects
+            .values()
+            .filter(move |object| model_ids.contains(&object.model))
+    }
+
+    /// Get all objects, grouped by their model ID
+    pub fn get_grouped_objects(&self) -> HashMap<ID, Vec<&data::Object>> {
+        self.objects
+            .values()
+            .into_group_map_by(|object| object.model)
+    }
+
+    /// Get object by ID
+    pub fn get_object(&self, id: ID) -> Option<&data::Object> {
+        self.objects.get(&id)
+    }
+
+    /// Get value by ID
+    pub fn get_value(&self, id: ID) -> Option<&data::Value> {
+        self.values.get(&id)
+    }
+
+    /// Get relation by ID
+    pub fn get_relation(&self, id: ID) -> Option<&data::Relation> {
+        self.relations.get(&id)
+    }
+
+    //endregion Data queries
+
+    //region Model editing
+
+    /// Define an object model
     pub fn define_object(&mut self, mut tpl: model::ObjectModel) -> Result<ID, StorageError> {
         if tpl.name.is_empty() {
             return Err(StorageError::ConstraintViolation(
@@ -213,7 +405,7 @@ impl Storage {
         Ok(id)
     }
 
-    /// Define a relation between two data objects
+    /// Define an object relation model
     pub fn define_relation(&mut self, mut rel: model::RelationModel) -> Result<ID, StorageError> {
         if rel.name.is_empty() || rel.reciprocal_name.is_empty() {
             return Err(StorageError::ConstraintViolation(
@@ -234,9 +426,9 @@ impl Storage {
 
         if let Some((_, colliding)) = self.rel_models.iter().find(|(_, other)| {
             (other.name == rel.name && other.object == rel.object) // Exact match
-            || (other.name == rel.reciprocal_name && other.object == rel.related) // Our reciprocal name collides with related's own relation name
+                || (other.name == rel.reciprocal_name && other.object == rel.related) // Our reciprocal name collides with related's own relation name
-            || (other.reciprocal_name == rel.name && other.related == rel.object) // Our name name collides with a reciprocal name on the other relation
+                || (other.reciprocal_name == rel.name && other.related == rel.object) // Our name name collides with a reciprocal name on the other relation
-            || (other.reciprocal_name == rel.reciprocal_name && other.related == rel.related)
+                || (other.reciprocal_name == rel.reciprocal_name && other.related == rel.related)
             // Reciprocal names collide for the same destination
         }) {
             return Err(StorageError::ConstraintViolation(
@@ -262,7 +454,7 @@ impl Storage {
         Ok(id)
     }
 
-    /// Define a property attached to an object or a relation
+    /// Define a property model, attached to an object or a relation
     pub fn define_property(&mut self, mut prop: model::PropertyModel) -> Result<ID, StorageError> {
         if prop.name.is_empty() {
             return Err(StorageError::ConstraintViolation(
@@ -301,7 +493,7 @@ impl Storage {
             Err(_) => {
                 return Err(StorageError::NotExist(
                     format!("default value {:?} has invalid type", prop.default).into(),
-                ))
+                ));
             }
         };
 
@@ -316,10 +508,10 @@ impl Storage {
         Ok(id)
     }
 
-    /// Delete an object definition and associated data
+    /// Undefine an object model, its properties and relations. Deletes all associated data.
     pub fn undefine_object(&mut self, id: ID) -> Result<ObjectModel, StorageError> {
-        return if let Some(t) = self.obj_models.remove(&id) {
+        return if let Some(model) = self.obj_models.remove(&id) {
-            debug!("Undefine object model \"{}\"", t.name);
+            debug!("Undefine object model \"{}\"", model.name);
             // Remove relation templates
             let removed_relation_ids = map_drain_filter(&mut self.rel_models, |_k, v| {
                 v.object == id || v.related == id
@@ -352,7 +544,7 @@ impl Storage {
 
             // Related object remain untouched, so there can be a problem with orphans. This is up to the application to deal with.
 
-            Ok(t)
+            Ok(model)
         } else {
             Err(StorageError::NotExist(
                 format!("object model {}", id).into(),
@@ -360,10 +552,10 @@ impl Storage {
         };
     }
 
-    /// Delete a relation definition and associated data
+    /// Undefine a relation model and its properties. Deletes all associated data.
     pub fn undefine_relation(&mut self, id: ID) -> Result<model::RelationModel, StorageError> {
-        return if let Some(t) = self.rel_models.remove(&id) {
+        return if let Some(model) = self.rel_models.remove(&id) {
-            debug!("Undefine relation model \"{}\"", t.name);
+            debug!("Undefine relation model \"{}\"", model.name);
 
             // Remove relations
             let removed = map_drain_filter(&mut self.relations, |_k, v| v.model == id);
@@ -384,7 +576,7 @@ impl Storage {
 
             // Related object remain untouched, so there can be a problem with orphans. This is up to the application to deal with.
 
-            Ok(t)
+            Ok(model)
         } else {
             Err(StorageError::NotExist(
                 format!("relation model {}", id).into(),
@@ -392,7 +584,7 @@ impl Storage {
         };
     }
 
-    /// Delete a property definition and associated data
+    /// Undefine a property model and delete associated data
     pub fn undefine_property(&mut self, id: ID) -> Result<model::PropertyModel, StorageError> {
         return if let Some(t) = self.prop_models.remove(&id) {
             debug!("Undefine property model \"{}\"", t.name);
@@ -408,33 +600,124 @@ impl Storage {
         };
     }
 
-    pub fn describe_model(&self, id: ID) -> String {
+    /// Update an object model, matched by its ID
-        if let Some(x) = self.obj_models.get(&id) {
+    pub fn update_object_model(&mut self, model: ObjectModel) -> Result<(), StorageError> {
-            x.to_string()
+        if model.name.is_empty() {
-        } else if let Some(x) = self.rel_models.get(&id) {
+            return Err(StorageError::ConstraintViolation(
-            x.to_string()
+                format!("Model name must not be empty.").into(),
-        } else if let Some(x) = self.prop_models.get(&id) {
+            ));
-            x.to_string()
+        }
+
+        if !self.obj_models.contains_key(&model.id) {
+            return Err(StorageError::NotExist(
+                format!("Object model ID {} does not exist.", model.id).into(),
+            ));
+        }
+
+        if let Some(conflict) = self
+            .obj_models
+            .values()
+            .find(|m| m.id != model.id && m.name == model.name)
+        {
+            return Err(StorageError::ConstraintViolation(
+                format!("Object {} already has the name {}", conflict.id, model.name).into(),
+            ));
+        }
+
+        self.obj_models.insert(model.id, model);
+        Ok(())
+    }
+
+    /// Update a relation model, matched by its ID
+    pub fn update_relation_model(&mut self, mut rel: RelationModel) -> Result<(), StorageError> {
+        if rel.name.is_empty() || rel.reciprocal_name.is_empty() {
+            return Err(StorageError::ConstraintViolation(
+                format!("Relation names must not be empty.").into(),
+            ));
+        }
+
+        // Object and Related can't be changed, so we re-fill them from the existing model
+        if let Some(existing) = self.rel_models.get(&rel.id) {
+            rel.object = existing.object;
+            rel.related = existing.related;
         } else {
-            id.to_string()
+            return Err(StorageError::NotExist(
+                format!("Relation model ID {} does not exist.", rel.id).into(),
+            ));
+        }
+
+        // Difficult checks ...
+
+        // yes this is stupid and inefficient and slow and
+        if let Some((_, colliding)) = self.rel_models.iter().find(|(_, other)| {
+            (other.name == rel.name && other.object == rel.object && rel.id != other.id) // Exact match
+                || (other.name == rel.reciprocal_name && other.object == rel.related && rel.id != other.id) // Our reciprocal name collides with related's own relation name
+                || (other.reciprocal_name == rel.name && other.related == rel.object && rel.id != other.id) // Our name name collides with a reciprocal name on the other relation
+                || (other.reciprocal_name == rel.reciprocal_name && other.related == rel.related && rel.id != other.id) // Reciprocal names collide for the same destination
+        }) {
+            return Err(StorageError::ConstraintViolation(
+                format!("name collision (\"{}\" / \"{}\") with existing relation (\"{}\" / \"{}\")",
+                        rel.name, rel.reciprocal_name,
+                        colliding.name, colliding.reciprocal_name
+                ).into()));
         }
+
+        self.rel_models.insert(rel.id, rel);
+        Ok(())
     }
 
-    pub fn get_model_name(&self, id: ID) -> &str {
+    /// Update a property model, matched by its ID
-        if let Some(x) = self.obj_models.get(&id) {
+    pub fn update_property_model(&mut self, mut prop: PropertyModel) -> Result<(), StorageError> {
-            &x.name
+        if prop.name.is_empty() {
-        } else if let Some(x) = self.rel_models.get(&id) {
+            return Err(StorageError::ConstraintViolation(
-            &x.name
+                format!("Property name must not be empty.").into(),
-        } else if let Some(x) = self.prop_models.get(&id) {
+            ));
-            &x.name
+        }
+
+        // Object can't be changed, so we re-fill them from the existing model
+        if let Some(existing) = self.prop_models.get(&prop.id) {
+            prop.object = existing.object;
         } else {
-            "???"
+            return Err(StorageError::NotExist(
+                format!("Property model ID {} does not exist.", prop.id).into(),
+            ));
         }
+
+        if self
+            .prop_models
+            .iter()
+            .find(|(_, t)| t.object == prop.object && t.name == prop.name && t.id != prop.id)
+            .is_some()
+        {
+            return Err(StorageError::ConstraintViolation(
+                format!(
+                    "property with the name \"{}\" already exists on {}",
+                    prop.name,
+                    self.describe_model(prop.object)
+                )
+                .into(),
+            ));
+        }
+
+        // Ensure the default type is compatible
+        prop.default = match prop.default.clone().cast_to(prop.data_type) {
+            Ok(v) => v,
+            Err(_) => {
+                return Err(StorageError::NotExist(
+                    format!("default value {:?} has invalid type", prop.default).into(),
+                ));
+            }
+        };
+
+        self.prop_models.insert(prop.id, prop);
+        Ok(())
     }
 
-    // DATA
+    //endregion Model editing
 
-    /// Insert object with relations, validating the data model constraints
+    //region Data editing
+
+    /// Insert a data object with properties and relations, validating the data model constraints
     pub fn insert_object(&mut self, insobj: InsertObj) -> Result<ID, StorageError> {
         let obj_model_id = insobj.model;
         debug!("Insert object {:?}", insobj);
@@ -444,7 +727,7 @@ impl Storage {
             None => {
                 return Err(StorageError::NotExist(
                     format!("object model {}", obj_model_id).into(),
-                ))
+                ));
             }
         };
 
@@ -600,117 +883,11 @@ impl Storage {
         Ok(object_id)
     }
 
-    // Reading
+    /// Update an existing object and its values, relations and their values.
-    pub fn get_object_models(&self) -> impl Iterator<Item = &ObjectModel> {
+    ///
-        self.obj_models.values()
+    /// The existing data is synchronized to match the new data, that is, relations and values
-    }
+    /// not specified will be deleted, these with matching IDs will be
-
+    /// overwritten, and ones without an ID will be created and assigned a unique ID.
-    pub fn get_object_model(&self, id: ID) -> Option<&ObjectModel> {
-        self.obj_models.get(&id)
-    }
-
-    pub fn get_relation_model(&self, id: ID) -> Option<&RelationModel> {
-        self.rel_models.get(&id)
-    }
-
-    pub fn get_property_model(&self, id: ID) -> Option<&PropertyModel> {
-        self.prop_models.get(&id)
-    }
-
-    pub fn get_grouped_prop_models(&self) -> HashMap<ID, Vec<&PropertyModel>> {
-        self.prop_models
-            .values()
-            .into_group_map_by(|model| model.object)
-    }
-
-    pub fn get_grouped_prop_models_for_parents(
-        &self,
-        parents: Vec<ID>,
-    ) -> HashMap<ID, Vec<&PropertyModel>> {
-        self.prop_models
-            .values()
-            .filter(|p| parents.contains(&p.object))
-            .into_group_map_by(|model| model.object)
-    }
-
-    pub fn get_relations_for_object(&self, object_id: ID) -> impl Iterator<Item = &data::Relation> {
-        self.relations
-            .values()
-            .filter(move |rel| rel.object == object_id)
-    }
-
-    pub fn get_reciprocal_relations_for_object(
-        &self,
-        object_id: ID,
-    ) -> impl Iterator<Item = &data::Relation> {
-        self.relations
-            .values()
-            .filter(move |rel| rel.related == object_id)
-    }
-
-    pub fn get_values_for_object(&self, object_id: ID) -> impl Iterator<Item = &data::Value> {
-        self.values
-            .values()
-            .filter(move |prop| prop.object == object_id)
-    }
-
-    pub fn get_grouped_values_for_objects(
-        &self,
-        parents: Vec<ID>,
-    ) -> HashMap<ID, Vec<&data::Value>> {
-        self.values
-            .values()
-            .filter(move |prop| parents.contains(&prop.object))
-            .into_group_map_by(|model| model.object)
-    }
-
-    pub fn get_relation_models_for_object_model(
-        &self,
-        model_id: ID,
-    ) -> impl Iterator<Item = &RelationModel> {
-        self.rel_models
-            .values()
-            .filter(move |model| model.object == model_id)
-    }
-
-    pub fn get_property_models_for_parents(
-        &self,
-        parents: Vec<ID>,
-    ) -> impl Iterator<Item = &PropertyModel> {
-        self.prop_models
-            .values()
-            .filter(move |model| parents.contains(&model.object))
-    }
-
-    pub fn get_objects_of_type(&self, model_ids: Vec<ID>) -> impl Iterator<Item = &data::Object> {
-        self.objects
-            .values()
-            .filter(move |object| model_ids.contains(&object.model))
-    }
-
-    pub fn get_grouped_objects(&self) -> HashMap<ID, Vec<&Object>> {
-        self.objects
-            .values()
-            .into_group_map_by(|object| object.model)
-    }
-
-    pub fn get_grouped_relation_models(&self) -> HashMap<ID, Vec<&RelationModel>> {
-        self.rel_models
-            .values()
-            .into_group_map_by(|model| model.object)
-    }
-
-    pub fn get_grouped_reciprocal_relation_models(&self) -> HashMap<ID, Vec<&RelationModel>> {
-        self.rel_models
-            .values()
-            .into_group_map_by(|model| model.related)
-    }
-
-    pub fn get_object(&self, id: ID) -> Option<&Object> {
-        self.objects.get(&id)
-    }
-
-    // Updates
     pub fn update_object(&mut self, updobj: UpdateObj) -> Result<(), StorageError> {
         let old_object = self.objects.get(&updobj.id).ok_or_else(|| {
             StorageError::ConstraintViolation(format!("Object does not exist").into())
@@ -725,7 +902,7 @@ impl Storage {
             None => {
                 return Err(StorageError::NotExist(
                     format!("object model {}", updated_object_model_id).into(),
-                ))
+                ));
             }
         };
 
@@ -937,116 +1114,6 @@ impl Storage {
         Ok(())
     }
 
-    pub fn update_object_model(&mut self, model: ObjectModel) -> Result<(), StorageError> {
-        if model.name.is_empty() {
-            return Err(StorageError::ConstraintViolation(
-                format!("Model name must not be empty.").into(),
-            ));
-        }
-
-        if !self.obj_models.contains_key(&model.id) {
-            return Err(StorageError::NotExist(
-                format!("Object model ID {} does not exist.", model.id).into(),
-            ));
-        }
-
-        if let Some(conflict) = self
-            .obj_models
-            .values()
-            .find(|m| m.id != model.id && m.name == model.name)
-        {
-            return Err(StorageError::ConstraintViolation(
-                format!("Object {} already has the name {}", conflict.id, model.name).into(),
-            ));
-        }
-
-        self.obj_models.insert(model.id, model);
-        Ok(())
-    }
-
-    pub fn update_relation_model(&mut self, mut rel: RelationModel) -> Result<(), StorageError> {
-        if rel.name.is_empty() || rel.reciprocal_name.is_empty() {
-            return Err(StorageError::ConstraintViolation(
-                format!("Relation names must not be empty.").into(),
-            ));
-        }
-
-        // Object and Related can't be changed, so we re-fill them from the existing model
-        if let Some(existing) = self.rel_models.get(&rel.id) {
-            rel.object = existing.object;
-            rel.related = existing.related;
-        } else {
-            return Err(StorageError::NotExist(
-                format!("Relation model ID {} does not exist.", rel.id).into(),
-            ));
-        }
-
-        // Difficult checks ...
-
-        // yes this is stupid and inefficient and slow and
-        if let Some((_, colliding)) = self.rel_models.iter().find(|(_, other)| {
-            (other.name == rel.name && other.object == rel.object && rel.id != other.id) // Exact match
-                || (other.name == rel.reciprocal_name && other.object == rel.related && rel.id != other.id) // Our reciprocal name collides with related's own relation name
-                || (other.reciprocal_name == rel.name && other.related == rel.object && rel.id != other.id) // Our name name collides with a reciprocal name on the other relation
-                || (other.reciprocal_name == rel.reciprocal_name && other.related == rel.related && rel.id != other.id) // Reciprocal names collide for the same destination
-        }) {
-            return Err(StorageError::ConstraintViolation(
-                format!("name collision (\"{}\" / \"{}\") with existing relation (\"{}\" / \"{}\")",
-                        rel.name, rel.reciprocal_name,
-                        colliding.name, colliding.reciprocal_name
-                ).into()));
-        }
-
-        self.rel_models.insert(rel.id, rel);
-        Ok(())
-    }
-
-    pub fn update_property_model(&mut self, mut prop: PropertyModel) -> Result<(), StorageError> {
-        if prop.name.is_empty() {
-            return Err(StorageError::ConstraintViolation(
-                format!("Property name must not be empty.").into(),
-            ));
-        }
-
-        // Object can't be changed, so we re-fill them from the existing model
-        if let Some(existing) = self.prop_models.get(&prop.id) {
-            prop.object = existing.object;
-        } else {
-            return Err(StorageError::NotExist(
-                format!("Property model ID {} does not exist.", prop.id).into(),
-            ));
-        }
-
-        if self
-            .prop_models
-            .iter()
-            .find(|(_, t)| t.object == prop.object && t.name == prop.name && t.id != prop.id)
-            .is_some()
-        {
-            return Err(StorageError::ConstraintViolation(
-                format!(
-                    "property with the name \"{}\" already exists on {}",
-                    prop.name,
-                    self.describe_model(prop.object)
-                )
-                .into(),
-            ));
-        }
-
-        // Ensure the default type is compatible
-        prop.default = match prop.default.clone().cast_to(prop.data_type) {
-            Ok(v) => v,
-            Err(_) => {
-                return Err(StorageError::NotExist(
-                    format!("default value {:?} has invalid type", prop.default).into(),
-                ))
-            }
-        };
-
-        self.prop_models.insert(prop.id, prop);
-        Ok(())
-    }
-
     /// Delete an object and associated data
     pub fn delete_object(&mut self, id: ID) -> Result<data::Object, StorageError> {
         return if let Some(t) = self.objects.remove(&id) {
@@ -1069,4 +1136,6 @@ impl Storage {
             Err(StorageError::NotExist(format!("object {}", id).into()))
         };
     }
+
+    //endregion Data editing
 }