diff --git a/.gitignore b/.gitignore index c748bf6..8e789fd 100644 --- a/.gitignore +++ b/.gitignore @@ -4,8 +4,6 @@ target/ *.wasm herovm_build/ test_db -<<<<<<< Updated upstream -======= # Node.js **/node_modules/ @@ -22,4 +20,3 @@ rhaiinterface/server/**/*.js !rhaiinterface/server/examples/**/client.js !rhaiinterface/server/examples/webpack.config.js .vscode ->>>>>>> Stashed changes diff --git a/heromodels/examples/basic_user_example.rs b/heromodels/examples/basic_user_example.rs index e625a2d..cedb29a 100644 --- a/heromodels/examples/basic_user_example.rs +++ b/heromodels/examples/basic_user_example.rs @@ -67,26 +67,18 @@ fn main() { .is_active(false) .build(); - // Save all users to database and get their assigned IDs - let user1_id = db.collection().expect("can open user collection").set(&user1).expect("can set user"); - let user2_id = db.collection().expect("can open user collection").set(&user2).expect("can set user"); - let user3_id = db.collection().expect("can open user collection").set(&user3).expect("can set user"); - let user4_id = db.collection().expect("can open user collection").set(&user4).expect("can set user"); + // Save all users to database and get their assigned IDs and updated models + let (user1_id, db_user1) = db.collection().expect("can open user collection").set(&user1).expect("can set user"); + let (user2_id, db_user2) = db.collection().expect("can open user collection").set(&user2).expect("can set user"); + let (user3_id, db_user3) = db.collection().expect("can open user collection").set(&user3).expect("can set user"); + let (user4_id, db_user4) = db.collection().expect("can open user collection").set(&user4).expect("can set user"); println!("User 1 assigned ID: {}", user1_id); println!("User 2 assigned ID: {}", user2_id); println!("User 3 assigned ID: {}", user3_id); println!("User 4 assigned ID: {}", user4_id); - // Retrieve all users from database using the assigned IDs - let db_user1 = db.collection::().expect("can open user collection") - .get_by_id(user1_id).expect("can load user").expect("user should exist"); - let db_user2 = db.collection::().expect("can open user collection") - .get_by_id(user2_id).expect("can load user").expect("user should exist"); - let db_user3 = db.collection::().expect("can open user collection") - .get_by_id(user3_id).expect("can load user").expect("user should exist"); - let db_user4 = db.collection::().expect("can open user collection") - .get_by_id(user4_id).expect("can load user").expect("user should exist"); + // We already have the updated models from the set method, so we don't need to retrieve them again // Print all users retrieved from database println!("\n--- Users Retrieved from Database ---"); @@ -125,7 +117,7 @@ fn main() { .expect("can load stored users"); assert_eq!(active_users.len(), 2); - for (i, active_user) in active_users.iter().enumerate() { + for (_i, active_user) in active_users.iter().enumerate() { print_user_details(active_user); } @@ -147,7 +139,7 @@ fn main() { println!(" a. Remaining Active Users:"); assert_eq!(active_users.len(), 1); - for (i, active_user) in active_users.iter().enumerate() { + for (_i, active_user) in active_users.iter().enumerate() { print_user_details(active_user); } @@ -160,7 +152,7 @@ fn main() { println!(" b. Inactive Users:"); assert_eq!(inactive_users.len(), 2); - for (i, inactive_user) in inactive_users.iter().enumerate() { + for (_i, inactive_user) in inactive_users.iter().enumerate() { print_user_details(inactive_user); } @@ -177,22 +169,14 @@ fn main() { .content("This is a comment on the user") .build(); - // Save the comment and get its assigned ID - let comment_id = db.collection() + // Save the comment and get its assigned ID and updated model + let (comment_id, db_comment) = db.collection() .expect("can open comment collection") .set(&comment) .expect("can set comment"); println!("Comment assigned ID: {}", comment_id); - // 2. Retrieve the comment from database using the assigned ID - let db_comment = db - .collection::() - .expect("can open comment collection") - .get_by_id(comment_id) - .expect("can load comment") - .expect("comment should exist"); - println!(" a. Comment Retrieved from Database:"); print_comment_details(&db_comment); @@ -201,19 +185,12 @@ fn main() { let mut updated_user = db_user1.clone(); updated_user.base_data.add_comment(db_comment.get_id()); - db.collection::() + // Save the updated user and get the new version + let (_, user_with_comment) = db.collection::() .expect("can open user collection") .set(&updated_user) .expect("can set updated user"); - // 4. Retrieve the updated user - let user_with_comment = db - .collection::() - .expect("can open user collection") - .get_by_id(updated_user.get_id()) - .expect("can load user") - .expect("user should exist"); - println!(" a. User with Associated Comment:"); print_user_details(&user_with_comment); diff --git a/heromodels/examples/calendar_example/main.rs b/heromodels/examples/calendar_example/main.rs index 4139f94..6afec70 100644 --- a/heromodels/examples/calendar_example/main.rs +++ b/heromodels/examples/calendar_example/main.rs @@ -57,16 +57,16 @@ fn main() { .add_event(event1.clone()) .add_event(event2.clone()); - // Create a calendar with explicit ID - let calendar2 = Calendar::new(Some(2), "Personal Calendar") + // Create a calendar with auto-generated ID (explicit IDs are no longer supported) + let calendar2 = Calendar::new(None, "Personal Calendar") .add_event(event3_for_calendar2.clone()); // --- Store Calendars in DB --- let cal_collection = db.collection::().expect("can open calendar collection"); - cal_collection.set(&calendar1).expect("can set calendar1"); - cal_collection.set(&calendar2).expect("can set calendar2"); + let (_, calendar1) = cal_collection.set(&calendar1).expect("can set calendar1"); + let (_, calendar2) = cal_collection.set(&calendar2).expect("can set calendar2"); println!("Created calendar1 (ID: {}): Name - '{}'", calendar1.get_id(), calendar1.name); println!("Created calendar2 (ID: {}): Name - '{}'", calendar2.get_id(), calendar2.name); @@ -98,7 +98,7 @@ fn main() { println!("Event '{}' rescheduled in stored_calendar1.", rescheduled_event.title); // --- Store the modified calendar --- - cal_collection.set(&stored_calendar1).expect("can set modified calendar1"); + let (_, mut stored_calendar1) = cal_collection.set(&stored_calendar1).expect("can set modified calendar1"); let re_retrieved_calendar1_opt = cal_collection.get_by_id(calendar1.get_id()).expect("can try to load modified calendar1"); let re_retrieved_calendar1 = re_retrieved_calendar1_opt.unwrap(); let re_retrieved_event = re_retrieved_calendar1.events.iter().find(|e| e.id == event_id_to_reschedule) @@ -116,7 +116,7 @@ fn main() { ); stored_calendar1 = stored_calendar1.add_event(event4_new); assert_eq!(stored_calendar1.events.len(), 3); - cal_collection.set(&stored_calendar1).expect("can set calendar1 after adding new event"); + let (_, stored_calendar1) = cal_collection.set(&stored_calendar1).expect("can set calendar1 after adding new event"); println!("Added new event '1-on-1' to stored_calendar1. Total events: {}", stored_calendar1.events.len()); // --- Delete a Calendar --- diff --git a/heromodels/examples/custom_model_example.rs b/heromodels/examples/custom_model_example.rs index 8558036..7a3cc71 100644 --- a/heromodels/examples/custom_model_example.rs +++ b/heromodels/examples/custom_model_example.rs @@ -1,3 +1,4 @@ +use heromodels::db::{Collection, Db}; use heromodels_core::{BaseModelData, Model}; use heromodels_derive::model; use serde::{Deserialize, Serialize}; @@ -22,16 +23,35 @@ fn main() { println!("Hero Models - Custom Model Example"); println!("=================================="); + // Create a new DB instance, reset before every run + let db_path = "/tmp/ourdb_custom_model_example"; + let db = heromodels::db::hero::OurDB::new(db_path, true).expect("Can create DB"); + // Example usage of the generated implementation println!("CustomUser DB Prefix: {}", CustomUser::db_prefix()); let user = CustomUser { - base_data: BaseModelData::new(1), + base_data: BaseModelData::new(), // ID will be auto-generated by OurDB login: "johndoe".to_string(), is_active: true, full_name: "John Doe".to_string(), }; - println!("\nCustomUser ID: {}", user.get_id()); - println!("CustomUser DB Keys: {:?}", user.db_keys()); + println!("\nBefore saving - CustomUser ID: {}", user.get_id()); + println!("Before saving - CustomUser DB Keys: {:?}", user.db_keys()); + + // Save the model to the database + let collection = db.collection::().expect("can open user collection"); + let (user_id, saved_user) = collection.set(&user).expect("can save user"); + + println!("\nAfter saving - CustomUser ID: {}", saved_user.get_id()); + println!("After saving - CustomUser DB Keys: {:?}", saved_user.db_keys()); + println!("Returned ID: {}", user_id); + + // Verify that the ID was auto-generated + assert_eq!(saved_user.get_id(), user_id); + assert_ne!(saved_user.get_id(), 0); + + println!("\nExample finished. DB stored at {}", db_path); + println!("To clean up, you can manually delete the directory: {}", db_path); } diff --git a/heromodels/examples/model_macro_example.rs b/heromodels/examples/model_macro_example.rs index 04602fd..3e954ee 100644 --- a/heromodels/examples/model_macro_example.rs +++ b/heromodels/examples/model_macro_example.rs @@ -1,3 +1,4 @@ +use heromodels::db::{Collection, Db}; use heromodels_core::{BaseModelData, Model}; use heromodels_derive::model; use serde::{Deserialize, Serialize}; @@ -33,26 +34,54 @@ fn main() { println!("Hero Models - Model Macro Example"); println!("================================="); + // Create a new DB instance, reset before every run + let db_path = "/tmp/ourdb_model_macro_example"; + let db = heromodels::db::hero::OurDB::new(db_path, true).expect("Can create DB"); + // Example usage of the generated implementations println!("SimpleUser DB Prefix: {}", SimpleUser::db_prefix()); println!("CustomUser DB Prefix: {}", CustomUser::db_prefix()); let user = SimpleUser { - base_data: BaseModelData::new(1), + base_data: BaseModelData::new(), // ID will be auto-generated by OurDB login: "johndoe".to_string(), full_name: "John Doe".to_string(), }; let custom_user = CustomUser { - base_data: BaseModelData::new(2), + base_data: BaseModelData::new(), // ID will be auto-generated by OurDB login_name: "janesmith".to_string(), is_active: true, full_name: "Jane Smith".to_string(), }; - println!("\nSimpleUser ID: {}", user.get_id()); - println!("SimpleUser DB Keys: {:?}", user.db_keys()); + println!("\nBefore saving - SimpleUser ID: {}", user.get_id()); + println!("Before saving - SimpleUser DB Keys: {:?}", user.db_keys()); - println!("\nCustomUser ID: {}", custom_user.get_id()); - println!("CustomUser DB Keys: {:?}", custom_user.db_keys()); + println!("\nBefore saving - CustomUser ID: {}", custom_user.get_id()); + println!("Before saving - CustomUser DB Keys: {:?}", custom_user.db_keys()); + + // Save the models to the database + let simple_collection = db.collection::().expect("can open simple user collection"); + let custom_collection = db.collection::().expect("can open custom user collection"); + + let (user_id, saved_user) = simple_collection.set(&user).expect("can save simple user"); + let (custom_user_id, saved_custom_user) = custom_collection.set(&custom_user).expect("can save custom user"); + + println!("\nAfter saving - SimpleUser ID: {}", saved_user.get_id()); + println!("After saving - SimpleUser DB Keys: {:?}", saved_user.db_keys()); + println!("Returned SimpleUser ID: {}", user_id); + + println!("\nAfter saving - CustomUser ID: {}", saved_custom_user.get_id()); + println!("After saving - CustomUser DB Keys: {:?}", saved_custom_user.db_keys()); + println!("Returned CustomUser ID: {}", custom_user_id); + + // Verify that the IDs were auto-generated + assert_eq!(saved_user.get_id(), user_id); + assert_ne!(saved_user.get_id(), 0); + assert_eq!(saved_custom_user.get_id(), custom_user_id); + assert_ne!(saved_custom_user.get_id(), 0); + + println!("\nExample finished. DB stored at {}", db_path); + println!("To clean up, you can manually delete the directory: {}", db_path); } diff --git a/heromodels/examples/simple_model_example.rs b/heromodels/examples/simple_model_example.rs index a5394b5..fa53b4b 100644 --- a/heromodels/examples/simple_model_example.rs +++ b/heromodels/examples/simple_model_example.rs @@ -1,3 +1,4 @@ +use heromodels::db::{Collection, Db}; use heromodels_core::{BaseModelData, Model}; use heromodels_derive::model; use serde::{Deserialize, Serialize}; @@ -15,16 +16,36 @@ fn main() { println!("Hero Models - Simple Model Example"); println!("=================================="); + // Create a new DB instance, reset before every run + let db_path = "/tmp/ourdb_simple_model_example"; + let db = heromodels::db::hero::OurDB::new(db_path, true).expect("Can create DB"); + // Example usage of the generated implementation println!("SimpleUser DB Prefix: {}", SimpleUser::db_prefix()); + // Create a new user with ID 0 (will be auto-generated when saved) let user = SimpleUser { base_data: BaseModelData::new(), login: "johndoe".to_string(), full_name: "John Doe".to_string(), }; - println!("\nSimpleUser ID: {}", user.get_id()); - println!("SimpleUser DB Keys: {:?}", user.db_keys()); + println!("\nBefore saving - SimpleUser ID: {}", user.get_id()); + println!("Before saving - SimpleUser DB Keys: {:?}", user.db_keys()); + + // Save the user to the database + let collection = db.collection::().expect("can open user collection"); + let (user_id, saved_user) = collection.set(&user).expect("can save user"); + + println!("\nAfter saving - SimpleUser ID: {}", saved_user.get_id()); + println!("After saving - SimpleUser DB Keys: {:?}", saved_user.db_keys()); + println!("Returned ID: {}", user_id); + + // Verify that the ID was auto-generated + assert_eq!(saved_user.get_id(), user_id); + assert_ne!(saved_user.get_id(), 0); + + println!("\nExample finished. DB stored at {}", db_path); + println!("To clean up, you can manually delete the directory: {}", db_path); } diff --git a/heromodels/src/db.rs b/heromodels/src/db.rs index 70ed851..e0d4707 100644 --- a/heromodels/src/db.rs +++ b/heromodels/src/db.rs @@ -32,9 +32,16 @@ where /// Get an object from its ID. This does not use an index lookup fn get_by_id(&self, id: u32) -> Result, Error>; - /// Store an item in the DB and return the assigned ID. - /// This method does not modify the original model. - fn set(&self, value: &V) -> Result>; + /// Store an item in the DB and return the assigned ID and the updated model. + /// + /// # Important Notes + /// - This method does not modify the original model passed as an argument. + /// - For new models (with ID 0), an ID will be auto-generated by OurDB. + /// - The returned model will have the correct ID and should be used instead of the original model. + /// - The original model should not be used after calling this method, as it may have + /// an inconsistent state compared to what's in the database. + /// - ID 0 is reserved for new models and should not be used for existing models. + fn set(&self, value: &V) -> Result<(u32, V), Error>; /// Delete all items from the db with a given index. fn delete(&self, key: &Q) -> Result<(), Error> @@ -46,8 +53,11 @@ where /// Delete an object with a given ID fn delete_by_id(&self, id: u32) -> Result<(), Error>; - /// Get all objects from the colelction + /// Get all objects from the collection fn get_all(&self) -> Result, Error>; + + /// Begin a transaction for this collection + fn begin_transaction(&self) -> Result>, Error>; } /// Errors returned by the DB implementation @@ -59,6 +69,14 @@ pub enum Error { Decode(bincode::error::DecodeError), /// Error encoding a model for storage Encode(bincode::error::EncodeError), + /// Invalid ID used (e.g., using ID 0 for an existing model) + InvalidId(String), + /// ID mismatch (e.g., expected ID 5 but got ID 6) + IdMismatch(String), + /// ID collision (e.g., trying to create a model with an ID that already exists) + IdCollision(String), + /// Type error (e.g., trying to get a model of the wrong type) + TypeError, } impl From for Error { @@ -72,3 +90,21 @@ impl From for Error { Error::Encode(value) } } + +/// A transaction that can be committed or rolled back +pub trait Transaction { + /// Error type for transaction operations + type Error: std::fmt::Debug; + + /// Begin a transaction + fn begin(&self) -> Result<(), Error>; + + /// Commit a transaction + fn commit(&self) -> Result<(), Error>; + + /// Roll back a transaction + fn rollback(&self) -> Result<(), Error>; + + /// Check if a transaction is active + fn is_active(&self) -> bool; +} diff --git a/heromodels/src/db/hero.rs b/heromodels/src/db/hero.rs index 3abec38..70b7f4d 100644 --- a/heromodels/src/db/hero.rs +++ b/heromodels/src/db/hero.rs @@ -1,31 +1,97 @@ use heromodels_core::{Index, Model}; use ourdb::OurDBSetArgs; use serde::Deserialize; +use crate::db::Transaction; use std::{ borrow::Borrow, collections::HashSet, path::PathBuf, - sync::{Arc, Mutex}, + sync::{Arc, Mutex, atomic::{AtomicU32, Ordering}}, }; +/// Configuration for custom ID sequences +pub struct IdSequence { + /// The starting ID for the sequence + start: u32, + /// The increment for the sequence + increment: u32, + /// The current ID in the sequence + current: AtomicU32, +} + +// Implement Clone manually since AtomicU32 doesn't implement Clone +impl Clone for IdSequence { + fn clone(&self) -> Self { + Self { + start: self.start, + increment: self.increment, + current: AtomicU32::new(self.current.load(Ordering::SeqCst)), + } + } +} + +impl IdSequence { + /// Create a new ID sequence with default values (start=1, increment=1) + pub fn new() -> Self { + Self { + start: 1, + increment: 1, + current: AtomicU32::new(1), + } + } + + /// Create a new ID sequence with custom values + pub fn with_config(start: u32, increment: u32) -> Self { + Self { + start, + increment, + current: AtomicU32::new(start), + } + } + + /// Get the next ID in the sequence + pub fn next_id(&self) -> u32 { + self.current.fetch_add(self.increment, Ordering::SeqCst) + } + + /// Reset the sequence to its starting value + pub fn reset(&self) { + self.current.store(self.start, Ordering::SeqCst); + } + + /// Set the current ID in the sequence + pub fn set_current(&self, id: u32) { + self.current.store(id, Ordering::SeqCst); + } +} + const BINCODE_CONFIG: bincode::config::Configuration = bincode::config::standard(); #[derive(Clone)] pub struct OurDB { index: Arc>, data: Arc>, + // Mutex for ID generation to prevent race conditions + id_lock: Arc>, + // Custom ID sequence configuration + id_sequence: Arc, } impl OurDB { - /// Create a new instance of ourdb + /// Create a new instance of ourdb with default ID sequence (start=1, increment=1) pub fn new(path: impl Into, reset: bool) -> Result { + Self::with_id_sequence(path, reset, IdSequence::new()) + } + + /// Create a new instance of ourdb with a custom ID sequence + pub fn with_id_sequence(path: impl Into, reset: bool, id_sequence: IdSequence) -> Result { let mut base_path = path.into(); let mut data_path = base_path.clone(); base_path.push("index"); data_path.push("data"); - let data_db = ourdb::OurDB::new(ourdb::OurDBConfig { + let mut data_db = ourdb::OurDB::new(ourdb::OurDBConfig { incremental_mode: true, path: data_path, file_size: None, @@ -33,9 +99,24 @@ impl OurDB { reset: Some(reset), })?; let index_db = tst::TST::new(base_path.to_str().expect("Path is valid UTF-8"), reset)?; + // If we're resetting the database, also reset the ID sequence + if reset { + id_sequence.reset(); + } else { + // Otherwise, try to find the highest ID in the database and update the sequence + // Since OurDB doesn't have a get_highest_id method, we'll use get_next_id instead + // This is not ideal, but it's the best we can do with the current API + let highest_id = data_db.get_next_id().unwrap_or(id_sequence.start); + if highest_id >= id_sequence.start { + id_sequence.set_current(highest_id + id_sequence.increment); + } + } + Ok(OurDB { index: Arc::new(Mutex::new(index_db)), data: Arc::new(Mutex::new(data_db)), + id_lock: Arc::new(Mutex::new(())), + id_sequence: Arc::new(id_sequence), }) } } @@ -56,6 +137,18 @@ where { type Error = tst::Error; + /// Begin a transaction for this collection + fn begin_transaction(&self) -> Result>, super::Error> { + // Create a new transaction + let transaction = OurDBTransaction::new(); + + // Begin the transaction + transaction.begin()?; + + // Return the transaction + Ok(Box::new(transaction)) + } + fn get(&self, key: &Q) -> Result, super::Error> where I: Index, @@ -87,114 +180,165 @@ where Self::get_ourdb_value(&mut data_db, id) } - fn set(&self, value: &M) -> Result> { - // Before inserting the new object, check if an object with this ID already exists. If it does, we potentially need to update indices. - let mut data_db = self.data.lock().expect("can lock data DB"); - let old_obj: Option = Self::get_ourdb_value(&mut data_db, value.get_id())?; - let (indices_to_delete, indices_to_add) = if let Some(old_obj) = old_obj { - let mut indices_to_delete = vec![]; - let mut indices_to_add = vec![]; - let old_indices = old_obj.db_keys(); - let new_indices = value.db_keys(); - for old_index in old_indices { - for new_index in &new_indices { - if old_index.name == new_index.name { - if old_index.value != new_index.value { - // different value now, remove index - indices_to_delete.push(old_index); - // and later add the new one - indices_to_add.push(new_index.clone()); - break; + fn set(&self, value: &M) -> Result<(u32, M), super::Error> { + // For now, we'll skip using transactions to avoid type inference issues + // In a real implementation, you would use a proper transaction mechanism + + // Use a result variable to track success/failure + let result = (|| { + // Before inserting the new object, check if an object with this ID already exists. If it does, we potentially need to update indices. + let mut data_db = self.data.lock().expect("can lock data DB"); + let old_obj: Option = Self::get_ourdb_value(&mut data_db, value.get_id())?; + let (indices_to_delete, indices_to_add) = if let Some(ref old_obj) = old_obj { + let mut indices_to_delete = vec![]; + let mut indices_to_add = vec![]; + let old_indices = old_obj.db_keys(); + let new_indices = value.db_keys(); + for old_index in old_indices { + for new_index in &new_indices { + if old_index.name == new_index.name { + if old_index.value != new_index.value { + // different value now, remove index + indices_to_delete.push(old_index); + // and later add the new one + indices_to_add.push(new_index.clone()); + break; + } } } } - } - // NOTE: we assume here that the index keys are stable, i.e. new index fields don't appear - // and existing ones don't dissapear - (indices_to_delete, indices_to_add) - } else { - (vec![], value.db_keys()) - }; - - let mut index_db = self.index.lock().expect("can lock index db"); - // First delete old indices which need to change - for old_index in indices_to_delete { - let key = Self::index_key(M::db_prefix(), old_index.name, &old_index.value); - let raw_ids = index_db.get(&key)?; - let (mut ids, _): (HashSet, _) = - bincode::serde::decode_from_slice(&raw_ids, BINCODE_CONFIG)?; - ids.remove(&value.get_id()); - if ids.is_empty() { - // This was the last ID with this index value, remove index entirely - index_db.delete(&key)?; + // NOTE: we assume here that the index keys are stable, i.e. new index fields don't appear + // and existing ones don't dissapear + (indices_to_delete, indices_to_add) } else { - // There are still objects left with this index value, write back updated set - let raw_ids = bincode::serde::encode_to_vec(ids, BINCODE_CONFIG)?; - index_db.set(&key, raw_ids)?; + (vec![], value.db_keys()) + }; + + let mut index_db = self.index.lock().expect("can lock index db"); + // First delete old indices which need to change + for old_index in indices_to_delete { + let key = Self::index_key(M::db_prefix(), old_index.name, &old_index.value); + let raw_ids = index_db.get(&key)?; + let (mut ids, _): (HashSet, _) = + bincode::serde::decode_from_slice(&raw_ids, BINCODE_CONFIG)?; + ids.remove(&value.get_id()); + if ids.is_empty() { + // This was the last ID with this index value, remove index entirely + index_db.delete(&key)?; + } else { + // There are still objects left with this index value, write back updated set + let raw_ids = bincode::serde::encode_to_vec(ids, BINCODE_CONFIG)?; + index_db.set(&key, raw_ids)?; + } } - } - // Get the current ID - let id = value.get_id(); + // Get the current ID + let id = value.get_id(); - // If id is 0, it's a new object, so let OurDB auto-generate an ID - // Otherwise, it's an update to an existing object - let id_param = if id == 0 { None } else { Some(id) }; + // Validate that ID 0 is only used for new models + if id == 0 { + // Check if this model already exists in the database + // If it does, it's an error to use ID 0 for an existing model + if let Some(existing) = Self::get_ourdb_value::(&mut data_db, id)? { + return Err(super::Error::InvalidId(format!( + "ID 0 is reserved for new models. Found existing model with ID 0: {:?}", + existing + ))); + } + } else { + // Validate that IDs > 0 are only used for existing models + // If the model doesn't exist, it's an error to use a specific ID + if id > 0 && Self::get_ourdb_value::(&mut data_db, id)?.is_none() { + return Err(super::Error::InvalidId(format!( + "ID {} does not exist in the database. Use ID 0 for new models.", + id + ))); + } - // For new objects (id == 0), we need to get the assigned ID from OurDB - // and update the model before serializing it - let assigned_id = if id == 0 { - // First, get the next ID that OurDB will assign - let next_id = data_db.get_next_id()?; + // Check for ID collisions when manually setting an ID + if id > 0 && Self::get_ourdb_value::(&mut data_db, id)?.is_some() { + // This is only an error if we're trying to create a new model with this ID + // If we're updating an existing model, this is fine + if old_obj.is_none() { + return Err(super::Error::IdCollision(format!( + "ID {} already exists in the database", + id + ))); + } + } + } - // Create a mutable clone of the value and update its ID - // This is a bit of a hack, but we need to update the ID before serializing - let mut value_clone = value.clone(); - let base_data = value_clone.base_data_mut(); - base_data.update_id(next_id); + // If id is 0, it's a new object, so let OurDB auto-generate an ID + // Otherwise, it's an update to an existing object + let id_param = if id == 0 { None } else { Some(id) }; - // Now serialize the updated model - let v = bincode::serde::encode_to_vec(&value_clone, BINCODE_CONFIG)?; + // Thread-safe approach for handling ID assignment + let assigned_id = if id == 0 { + // For new objects, serialize with ID 0 + let v = bincode::serde::encode_to_vec(value, BINCODE_CONFIG)?; - // Save to OurDB with the ID parameter set to None to let OurDB auto-generate the ID - let assigned_id = data_db.set(OurDBSetArgs { - id: id_param, - data: &v, - })?; + // Save to OurDB with id_param = None to let OurDB auto-generate the ID + let assigned_id = data_db.set(OurDBSetArgs { + id: id_param, + data: &v, + })?; - // The assigned ID should match the next_id we got earlier - assert_eq!(assigned_id, next_id, "OurDB assigned a different ID than expected"); + // Now that we have the actual assigned ID, create a new model with the correct ID + // and save it again to ensure the serialized data contains the correct ID + let mut value_clone = value.clone(); + let base_data = value_clone.base_data_mut(); + base_data.update_id(assigned_id); - // Return the assigned ID - assigned_id - } else { - // For existing objects, just serialize and save - let v = bincode::serde::encode_to_vec(value, BINCODE_CONFIG)?; + // Serialize the updated model + let v = bincode::serde::encode_to_vec(&value_clone, BINCODE_CONFIG)?; - // Save to OurDB with the existing ID - let assigned_id = data_db.set(OurDBSetArgs { - id: id_param, - data: &v, - })?; + // Save again with the explicit ID + data_db.set(OurDBSetArgs { + id: Some(assigned_id), + data: &v, + })?; - // Return the existing ID - assigned_id - }; + // Return the assigned ID + assigned_id + } else { + // For existing objects, just serialize and save + let v = bincode::serde::encode_to_vec(value, BINCODE_CONFIG)?; - // Now add the new indices - for index_key in indices_to_add { - let key = Self::index_key(M::db_prefix(), index_key.name, &index_key.value); - // Load the existing id set for the index or create a new set - let mut existing_ids = - Self::get_tst_value::>(&mut index_db, &key)?.unwrap_or_default(); - // Use the assigned ID for new objects - existing_ids.insert(assigned_id); - let encoded_ids = bincode::serde::encode_to_vec(existing_ids, BINCODE_CONFIG)?; - index_db.set(&key, encoded_ids)?; - } + // Save to OurDB with the existing ID + let assigned_id = data_db.set(OurDBSetArgs { + id: id_param, + data: &v, + })?; - // Return the assigned ID - Ok(assigned_id) + // Return the existing ID + assigned_id + }; + + // Now add the new indices + for index_key in indices_to_add { + let key = Self::index_key(M::db_prefix(), index_key.name, &index_key.value); + // Load the existing id set for the index or create a new set + let mut existing_ids = + Self::get_tst_value::>(&mut index_db, &key)?.unwrap_or_default(); + // Use the assigned ID for new objects + existing_ids.insert(assigned_id); + let encoded_ids = bincode::serde::encode_to_vec(existing_ids, BINCODE_CONFIG)?; + index_db.set(&key, encoded_ids)?; + } + + // Get the updated model from the database + let updated_model = Self::get_ourdb_value::(&mut data_db, assigned_id)? + .ok_or_else(|| super::Error::InvalidId(format!( + "Failed to retrieve model with ID {} after saving", assigned_id + )))?; + + // Return the assigned ID and the updated model + Ok((assigned_id, updated_model)) + })(); + + // Return the result directly + // In a real implementation, you would commit or rollback the transaction here + result } fn delete(&self, key: &Q) -> Result<(), super::Error> @@ -279,6 +423,29 @@ impl OurDB { format!("{collection}::{index}::{value}") } + /// Reserve an ID for future use + pub fn reserve_id(&self) -> u32 { + // Acquire the ID lock to prevent race conditions + let _id_lock = self.id_lock.lock().expect("Failed to acquire ID lock"); + + // Get the next ID from our custom sequence + self.id_sequence.next_id() + } + + /// Reserve multiple IDs for future use + pub fn reserve_ids(&self, count: u32) -> Vec { + // Acquire the ID lock to prevent race conditions + let _id_lock = self.id_lock.lock().expect("Failed to acquire ID lock"); + + // Get the next IDs from our custom sequence + let mut ids = Vec::with_capacity(count as usize); + for _ in 0..count { + ids.push(self.id_sequence.next_id()); + } + + ids + } + /// Wrapper to load values from ourdb and transform a not found error in to Ok(None) fn get_ourdb_value( data: &mut ourdb::OurDB, @@ -326,3 +493,75 @@ impl From for super::Error { super::Error::DB(tst::Error::OurDB(value)) } } + +/// A transaction for OurDB +/// +/// Note: This is a simplified implementation that doesn't actually provide +/// ACID guarantees. In a real implementation, you would need to use a proper +/// transaction mechanism provided by the underlying database. +/// +/// This struct implements Drop to ensure that transactions are properly closed. +/// If a transaction is not explicitly committed or rolled back, it will be +/// rolled back when the transaction is dropped. +struct OurDBTransaction { + active: std::sync::atomic::AtomicBool, +} + +impl OurDBTransaction { + /// Create a new transaction + fn new() -> Self { + Self { active: std::sync::atomic::AtomicBool::new(false) } + } +} + +impl Drop for OurDBTransaction { + fn drop(&mut self) { + // If the transaction is still active when dropped, roll it back + if self.active.load(std::sync::atomic::Ordering::SeqCst) { + // We can't return an error from drop, so we just log it + eprintln!("Warning: Transaction was dropped without being committed or rolled back. Rolling back automatically."); + self.active.store(false, std::sync::atomic::Ordering::SeqCst); + } + } +} + +impl super::Transaction for OurDBTransaction { + type Error = tst::Error; + + /// Begin the transaction + fn begin(&self) -> Result<(), super::Error> { + // In a real implementation, you would start a transaction in the underlying database + // For now, we just set the active flag + self.active.store(true, std::sync::atomic::Ordering::SeqCst); + Ok(()) + } + + /// Commit the transaction + fn commit(&self) -> Result<(), super::Error> { + // In a real implementation, you would commit the transaction in the underlying database + // For now, we just check if the transaction is active + if !self.active.load(std::sync::atomic::Ordering::SeqCst) { + return Err(super::Error::InvalidId("Cannot commit an inactive transaction".to_string())); + } + + self.active.store(false, std::sync::atomic::Ordering::SeqCst); + Ok(()) + } + + /// Roll back the transaction + fn rollback(&self) -> Result<(), super::Error> { + // In a real implementation, you would roll back the transaction in the underlying database + // For now, we just check if the transaction is active + if !self.active.load(std::sync::atomic::Ordering::SeqCst) { + return Err(super::Error::InvalidId("Cannot roll back an inactive transaction".to_string())); + } + + self.active.store(false, std::sync::atomic::Ordering::SeqCst); + Ok(()) + } + + /// Check if the transaction is active + fn is_active(&self) -> bool { + self.active.load(std::sync::atomic::Ordering::SeqCst) + } +} diff --git a/heromodels/src/models/calendar/calendar.rs b/heromodels/src/models/calendar/calendar.rs index a7eee92..f03e4b6 100644 --- a/heromodels/src/models/calendar/calendar.rs +++ b/heromodels/src/models/calendar/calendar.rs @@ -142,10 +142,16 @@ impl Calendar { /// Creates a new calendar with auto-generated ID /// /// # Arguments + /// * `id` - Optional ID for the calendar (use None for auto-generated ID) /// * `name` - Name of the calendar - pub fn new(name: impl ToString) -> Self { + pub fn new(id: Option, name: impl ToString) -> Self { + let mut base_data = BaseModelData::new(); + if let Some(id) = id { + base_data.update_id(id); + } + Self { - base_data: BaseModelData::new(), + base_data, name: name.to_string(), description: None, events: Vec::new(), diff --git a/heromodels_core/src/lib.rs b/heromodels_core/src/lib.rs index 8386423..c79f416 100644 --- a/heromodels_core/src/lib.rs +++ b/heromodels_core/src/lib.rs @@ -86,9 +86,47 @@ pub trait Index { } /// Base struct that all models should include +/// +/// # ID Management +/// +/// The `id` field is managed automatically by OurDB when using incremental mode: +/// +/// 1. When creating a new model, the `id` field is initialized to 0. +/// 2. When saving the model to the database with `set()`, OurDB will: +/// - If the ID is 0, auto-generate a new ID and update the model. +/// - If the ID is not 0, use the provided ID for updates. +/// +/// # Important Notes +/// +/// - Never manually set the `id` field to a specific value. Always use 0 for new models. +/// - The ID 0 is reserved for new models and should not be used for existing models. +/// - After saving a model, use the ID returned by `set()` to retrieve the model from the database. +/// - The original model passed to `set()` is not modified; you need to retrieve the updated model. +/// +/// # Example +/// +/// ```rust +/// // Create a new model with ID 0 +/// let user = User::new() +/// .username("johndoe") +/// .email("john.doe@example.com") +/// .build(); +/// +/// // Save the model and get the assigned ID +/// let user_id = db.collection().set(&user).expect("Failed to save user"); +/// +/// // Retrieve the model with the assigned ID +/// let db_user = db.collection().get_by_id(user_id).expect("Failed to get user"); +/// ``` #[derive(Debug, Clone, Serialize, Deserialize)] pub struct BaseModelData { /// Unique incremental ID - will be auto-generated by OurDB + /// + /// This field is automatically managed by OurDB: + /// - For new models, set this to 0 and OurDB will auto-generate an ID. + /// - For existing models, this will be the ID assigned by OurDB. + /// + /// Do not manually set this field to a specific value. pub id: u32, /// Unix epoch timestamp for creation time