Make sure props are stored inside space.meta.env

Also fixed a bug in the create space txn payload

server/src/engine/core/space.rs

@@ -47,18 +47,37 @@ pub struct Space {
 #[derive(Debug, Default)]
 /// Space metadata
 pub struct SpaceMeta {
-    pub(super) env: RwLock<DictGeneric>,
+    pub(super) props: RwLock<DictGeneric>,
 }
 
 impl SpaceMeta {
     pub const KEY_ENV: &str = "env";
+    pub fn new_with_meta(props: DictGeneric) -> Self {
+        Self {
+            props: RwLock::new(props),
+        }
+    }
     pub fn with_env(env: DictGeneric) -> Self {
         Self {
-            env: RWLIdx::new(env),
+            props: RwLock::new(into_dict!("env" => DictEntryGeneric::Map(env))),
         }
     }
-    pub fn env(&self) -> &RwLock<DictGeneric> {
-        &self.env
+    pub fn dict(&self) -> &RwLock<DictGeneric> {
+        &self.props
+    }
+    pub fn get_env<'a>(rwl: &'a parking_lot::RwLockReadGuard<'a, DictGeneric>) -> &'a DictGeneric {
+        match rwl.get(Self::KEY_ENV).unwrap() {
+            DictEntryGeneric::Data(_) => unreachable!(),
+            DictEntryGeneric::Map(m) => m,
+        }
+    }
+    pub fn get_env_mut<'a>(
+        rwl: &'a mut parking_lot::RwLockWriteGuard<'a, DictGeneric>,
+    ) -> &'a mut DictGeneric {
+        match rwl.get_mut(Self::KEY_ENV).unwrap() {
+            DictEntryGeneric::Data(_) => unreachable!(),
+            DictEntryGeneric::Map(m) => m,
+        }
     }
 }
 
@@ -184,7 +203,7 @@ impl Space {
         // commit txn
         if TI::NONNULL {
             // prepare and commit txn
-            let s_read = space.metadata().env().read();
+            let s_read = space.metadata().dict().read();
             txn_driver.try_commit(gnstxn::CreateSpaceTxn::new(&s_read, &space_name, &space))?;
         }
         // update global state
@@ -207,15 +226,20 @@ impl Space {
         }: AlterSpace,
     ) -> DatabaseResult<()> {
         gns.with_space(&space_name, |space| {
-            let mut space_env = space.meta.env.write();
+            let mut space_props = space.meta.props.write();
+            let DictEntryGeneric::Map(space_env_mut) =
+                space_props.get_mut(SpaceMeta::KEY_ENV).unwrap()
+            else {
+                unreachable!()
+            };
             match updated_props.remove(SpaceMeta::KEY_ENV) {
                 Some(DictEntryGeneric::Map(env)) if updated_props.is_empty() => {
-                    if !dict::rmerge_metadata(&mut space_env, env) {
+                    if !dict::rmerge_metadata(space_env_mut, env) {
                         return Err(DatabaseError::DdlSpaceBadProperty);
                     }
                 }
                 Some(DictEntryGeneric::Data(l)) if updated_props.is_empty() & l.is_null() => {
-                    space_env.clear()
+                    space_env_mut.clear()
                 }
                 None => {}
                 _ => return Err(DatabaseError::DdlSpaceBadProperty),
@@ -244,8 +268,8 @@ impl Space {
 #[cfg(test)]
 impl PartialEq for SpaceMeta {
     fn eq(&self, other: &Self) -> bool {
-        let x = self.env.read();
-        let y = other.env.read();
+        let x = self.props.read();
+        let y = other.props.read();
         *x == *y
     }
 }

server/src/engine/core/tests/ddl_space/alter.rs

@@ -61,8 +61,9 @@ fn alter_update_prop_env_var() {
         &gns,
         "create space myspace with { env: { MY_NEW_PROP: 100 } }",
         |space| {
+            let rl = space.meta.dict().read();
             assert_eq!(
-                space.meta.env.read().get("MY_NEW_PROP").unwrap(),
+                SpaceMeta::get_env(&rl).get("MY_NEW_PROP").unwrap(),
                 &(Datacell::new_uint(100).into())
             )
         },
@@ -92,8 +93,9 @@ fn alter_remove_prop_env_var() {
         &gns,
         "create space myspace with { env: { MY_NEW_PROP: 100 } }",
         |space| {
+            let rl = space.meta.dict().read();
             assert_eq!(
-                space.meta.env.read().get("MY_NEW_PROP").unwrap(),
+                SpaceMeta::get_env(&rl).get("MY_NEW_PROP").unwrap(),
                 &(Datacell::new_uint(100).into())
             )
         },
@@ -133,8 +135,9 @@ fn alter_remove_all_env() {
         &gns,
         "create space myspace with { env: { MY_NEW_PROP: 100 } }",
         |space| {
+            let rl = space.meta.dict().read();
             assert_eq!(
-                space.meta.env.read().get("MY_NEW_PROP").unwrap(),
+                SpaceMeta::get_env(&rl).get("MY_NEW_PROP").unwrap(),
                 &(Datacell::new_uint(100).into())
             )
         },

server/src/engine/data/dict.rs

@@ -52,14 +52,6 @@ pub enum DictEntryGeneric {
     patchsets
 */
 
-#[derive(Debug, PartialEq, Default)]
-struct DictGenericPatch(HashMap<Box<str>, Option<DictGenericPatchEntry>>);
-#[derive(Debug, PartialEq)]
-enum DictGenericPatchEntry {
-    Data(Datacell),
-    Map(DictGenericPatch),
-}
-
 /// Accepts a dict with possible null values, and removes those null values
 pub fn rflatten_metadata(mut new: DictGeneric) -> DictGeneric {
     _rflatten_metadata(&mut new);
@@ -79,25 +71,38 @@ fn _rflatten_metadata(new: &mut DictGeneric) {
 /// Recursively merge a [`DictGeneric`] into a [`DictGeneric`] with the use of an intermediary
 /// patchset to avoid inconsistent states
 pub fn rmerge_metadata(current: &mut DictGeneric, new: DictGeneric) -> bool {
-    let mut patch = DictGenericPatch::default();
-    let current_ref = current as &_;
-    let r = rmerge_metadata_prepare_patch(current_ref, new, &mut patch);
-    if r {
-        merge_data_with_patch(current, patch);
+    match rprepare_metadata_patch(current as &_, new) {
+        Some(patch) => {
+            rmerge_data_with_patch(current, patch);
+            true
+        }
+        None => false,
     }
-    r
 }
 
-fn merge_data_with_patch(current: &mut DictGeneric, patch: DictGenericPatch) {
-    for (key, patch) in patch.0 {
+pub fn rprepare_metadata_patch(current: &DictGeneric, new: DictGeneric) -> Option<DictGeneric> {
+    let mut patch = Default::default();
+    if rmerge_metadata_prepare_patch(current, new, &mut patch) {
+        Some(patch)
+    } else {
+        None
+    }
+}
+
+fn rmerge_data_with_patch(current: &mut DictGeneric, patch: DictGeneric) {
+    for (key, patch) in patch {
         match patch {
-            Some(DictGenericPatchEntry::Data(d)) => {
+            DictEntryGeneric::Data(d) if d.is_init() => {
                 current.st_upsert(key, DictEntryGeneric::Data(d));
             }
-            Some(DictGenericPatchEntry::Map(m)) => match current.get_mut(&key) {
+            DictEntryGeneric::Data(_) => {
+                // null
+                let _ = current.remove(&key);
+            }
+            DictEntryGeneric::Map(m) => match current.get_mut(&key) {
                 Some(current_recursive) => match current_recursive {
                     DictEntryGeneric::Map(current_m) => {
-                        merge_data_with_patch(current_m, m);
+                        rmerge_data_with_patch(current_m, m);
                     }
                     _ => {
                         // can never reach here since the patch is always correct
@@ -106,12 +111,9 @@ fn merge_data_with_patch(current: &mut DictGeneric, patch: DictGenericPatch) {
                 },
                 None => {
                     let mut new = DictGeneric::new();
-                    merge_data_with_patch(&mut new, m);
+                    rmerge_data_with_patch(&mut new, m);
                 }
             },
-            None => {
-                let _ = current.remove(&key);
-            }
         }
     }
 }
@@ -119,7 +121,7 @@ fn merge_data_with_patch(current: &mut DictGeneric, patch: DictGenericPatch) {
 fn rmerge_metadata_prepare_patch(
     current: &DictGeneric,
     new: DictGeneric,
-    patch: &mut DictGenericPatch,
+    patch: &mut DictGeneric,
 ) -> bool {
     let mut new = new.into_iter();
     let mut okay = true;
@@ -131,9 +133,7 @@ fn rmerge_metadata_prepare_patch(
                 if new_data.is_init() =>
             {
                 if this_data.kind() == new_data.kind() {
-                    patch
-                        .0
-                        .insert(key, Some(DictGenericPatchEntry::Data(new_data)));
+                    patch.insert(key, DictEntryGeneric::Data(new_data));
                 } else {
                     okay = false;
                 }
@@ -145,32 +145,28 @@ fn rmerge_metadata_prepare_patch(
                 Some(DictEntryGeneric::Map(this_recursive_data)),
                 DictEntryGeneric::Map(new_recursive_data),
             ) => {
-                let mut this_patch = DictGenericPatch::default();
+                let mut this_patch = DictGeneric::new();
                 let _okay = rmerge_metadata_prepare_patch(
                     this_recursive_data,
                     new_recursive_data,
                     &mut this_patch,
                 );
-                patch
-                    .0
-                    .insert(key, Some(DictGenericPatchEntry::Map(this_patch)));
+                patch.insert(key, DictEntryGeneric::Map(this_patch));
                 okay &= _okay;
             }
             // null -> non-null: flatten insert
             (None, DictEntryGeneric::Data(l)) if l.is_init() => {
-                let _ = patch.0.insert(key, Some(DictGenericPatchEntry::Data(l)));
+                let _ = patch.insert(key, DictEntryGeneric::Data(l));
             }
             (None, DictEntryGeneric::Map(m)) => {
-                let mut this_patch = DictGenericPatch::default();
+                let mut this_patch = DictGeneric::new();
                 okay &= rmerge_metadata_prepare_patch(&into_dict!(), m, &mut this_patch);
-                let _ = patch
-                    .0
-                    .insert(key, Some(DictGenericPatchEntry::Map(this_patch)));
+                let _ = patch.insert(key, DictEntryGeneric::Map(this_patch));
             }
             // non-null -> null: remove
             (Some(_), DictEntryGeneric::Data(l)) => {
                 debug_assert!(l.is_null());
-                patch.0.insert(key, None);
+                patch.insert(key, DictEntryGeneric::Data(Datacell::null()));
             }
             (None, DictEntryGeneric::Data(l)) => {
                 debug_assert!(l.is_null());

server/src/engine/storage/v1/inf/obj.rs

@@ -272,6 +272,7 @@ impl<'a> PersistObject for ModelLayoutRef<'a> {
 }
 
 pub struct SpaceLayout;
+#[derive(Debug)]
 pub struct SpaceLayoutMD {
     uuid: Uuid,
     prop_c: usize,
@@ -323,7 +324,7 @@ impl<'a> PersistObject for SpaceLayoutRef<'a> {
                 super::map::MapIndexSizeMD(md.prop_c),
             )?;
         Ok(Space::new_restore_empty(
-            SpaceMeta::with_env(space_meta),
+            SpaceMeta::new_with_meta(space_meta),
             md.uuid,
         ))
     }

server/src/engine/storage/v1/inf/tests.rs

@@ -112,7 +112,7 @@ fn model() {
 fn space() {
     let uuid = Uuid::new();
     let space = Space::new_with_uuid(Default::default(), SpaceMeta::default(), uuid);
-    let space_meta_read = space.metadata().env().read();
+    let space_meta_read = space.metadata().dict().read();
     let enc = super::enc::enc_full::<obj::SpaceLayoutRef>(obj::SpaceLayoutRef::from((
         &space,
         &*space_meta_read,

server/src/engine/txn/gns/space.rs

@@ -101,7 +101,7 @@ impl<'a> PersistObject for CreateSpaceTxn<'a> {
     }
     fn obj_enc(buf: &mut Vec<u8>, data: Self::InputType) {
         buf.extend(data.space_name.as_bytes());
-        <obj::SpaceLayoutRef as PersistObject>::meta_enc(buf, (data.space, data.space_meta).into());
+        <obj::SpaceLayoutRef as PersistObject>::obj_enc(buf, (data.space, data.space_meta).into());
     }
     unsafe fn obj_dec(s: &mut BufferedScanner, md: Self::Metadata) -> SDSSResult<Self::OutputType> {
         let space_name =
@@ -138,14 +138,14 @@ impl<'a> GNSEvent for CreateSpaceTxn<'a> {
 /// Transaction payload for an `alter space ...` query
 pub struct AlterSpaceTxn<'a> {
     space_id: super::SpaceIDRef<'a>,
-    space_meta: &'a DictGeneric,
+    updated_props: &'a DictGeneric,
 }
 
 impl<'a> AlterSpaceTxn<'a> {
-    pub const fn new(space_id: super::SpaceIDRef<'a>, space_meta: &'a DictGeneric) -> Self {
+    pub const fn new(space_id: super::SpaceIDRef<'a>, updated_props: &'a DictGeneric) -> Self {
         Self {
             space_id,
-            space_meta,
+            updated_props,
         }
     }
 }
@@ -171,7 +171,7 @@ impl<'a> PersistObject for AlterSpaceTxn<'a> {
     }
     fn meta_enc(buf: &mut Vec<u8>, data: Self::InputType) {
         <super::SpaceID as PersistObject>::meta_enc(buf, data.space_id);
-        buf.extend(data.space_meta.len().u64_bytes_le());
+        buf.extend(data.updated_props.len().u64_bytes_le());
     }
     unsafe fn meta_dec(scanner: &mut BufferedScanner) -> SDSSResult<Self::Metadata> {
         Ok(AlterSpaceTxnMD {
@@ -181,7 +181,10 @@ impl<'a> PersistObject for AlterSpaceTxn<'a> {
     }
     fn obj_enc(buf: &mut Vec<u8>, data: Self::InputType) {
         <super::SpaceID as PersistObject>::obj_enc(buf, data.space_id);
-        <map::PersistMapImpl<map::GenericDictSpec> as PersistObject>::obj_enc(buf, data.space_meta);
+        <map::PersistMapImpl<map::GenericDictSpec> as PersistObject>::obj_enc(
+            buf,
+            data.updated_props,
+        );
     }
     unsafe fn obj_dec(s: &mut BufferedScanner, md: Self::Metadata) -> SDSSResult<Self::OutputType> {
         let space_id = <super::SpaceID as PersistObject>::obj_dec(s, md.space_id_meta)?;
@@ -211,10 +214,10 @@ impl<'a> GNSEvent for AlterSpaceTxn<'a> {
         let gns = gns.spaces().read();
         match gns.st_get(&space_id.name) {
             Some(space) => {
-                let mut wmeta = space.metadata().env().write();
-                space_meta
-                    .into_iter()
-                    .for_each(|(k, v)| wmeta.st_upsert(k, v));
+                let mut wmeta = space.metadata().dict().write();
+                if !crate::engine::data::dict::rmerge_metadata(&mut wmeta, space_meta) {
+                    return Err(TransactionError::OnRestoreDataConflictMismatch);
+                }
             }
             None => return Err(TransactionError::OnRestoreDataMissing),
         }

server/src/engine/txn/gns/tests/io.rs

@@ -47,7 +47,7 @@ mod space_tests {
     #[test]
     fn create() {
         let orig_space = Space::empty();
-        let space_r = orig_space.metadata().env().read();
+        let space_r = orig_space.metadata().dict().read();
         let txn = CreateSpaceTxn::new(&space_r, "myspace", &orig_space);
         let encoded = enc::enc_full_self(txn);
         let decoded = dec::dec_full::<CreateSpaceTxn>(&encoded).unwrap();
@@ -62,7 +62,7 @@ mod space_tests {
     #[test]
     fn alter() {
         let space = Space::empty();
-        let space_r = space.metadata().env().read();
+        let space_r = space.metadata().dict().read();
         let txn = AlterSpaceTxn::new(SpaceIDRef::new("myspace", &space), &space_r);
         let encoded = enc::enc_full_self(txn);
         let decoded = dec::dec_full::<AlterSpaceTxn>(&encoded).unwrap();