Skip to content

Commit 4b46617

Browse files
authored
Use IndexMap for mounts and validate duplicates [Namespaces Part 2] (#5062)
# Description of Changes - Replace ModuleDef `mounts` type with IndexMap rather than Vec to explicitly require name uniqueness - Validate mount name uniqueness # API and ABI breaking changes No # Expected complexity level and risk 2 - Fairly simple change to new (as of yet unused) module def type # Testing - [ ] Added a test to check that the duplicate name check is working correctly
1 parent d3b4a96 commit 4b46617

3 files changed

Lines changed: 45 additions & 7 deletions

File tree

crates/schema/src/def.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ pub struct ModuleDef {
153153
raw_module_def_version: RawModuleDefVersion,
154154

155155
/// Mounted submodules, keyed by the namespace they are mounted under.
156-
mounts: Vec<(String, ModuleDef)>,
156+
mounts: IndexMap<String, ModuleDef>,
157157
}
158158

159159
#[derive(Debug, Clone, Copy, Eq, PartialEq)]
@@ -171,7 +171,7 @@ impl ModuleDef {
171171
}
172172

173173
/// The mounted submodules of the module definition.
174-
pub fn mounts(&self) -> &[(String, ModuleDef)] {
174+
pub fn mounts(&self) -> &IndexMap<String, ModuleDef> {
175175
&self.mounts
176176
}
177177

crates/schema/src/def/validate/v10.rs

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -271,8 +271,8 @@ pub fn validate(def: RawModuleDefV10) -> Result<ModuleDef> {
271271

272272
let (tables, types, reducers, procedures, views, mounts) = (tables_types_reducers_procedures_views, mounts)
273273
.combine_errors()
274-
.map(|((tables, types, reducers, procedures, views), mounts)| {
275-
(tables, types, reducers, procedures, views, mounts)
274+
.and_then(|((tables, types, reducers, procedures, views), mounts)| {
275+
validate_mount_names_are_unique(mounts).map(|mounts| (tables, types, reducers, procedures, views, mounts))
276276
})
277277
.map_err(|errors: ValidationErrors| errors.sort_deduplicate())?;
278278

@@ -302,6 +302,21 @@ fn validate_mount(mount: RawModuleMountV10) -> Result<(String, ModuleDef)> {
302302
Ok((mount.namespace, validate(mount.module)?))
303303
}
304304

305+
fn validate_mount_names_are_unique(mounts: Vec<(String, ModuleDef)>) -> Result<IndexMap<String, ModuleDef>> {
306+
let mut errors = vec![];
307+
let mut map = IndexMap::with_capacity(mounts.len());
308+
309+
for (namespace, def) in mounts {
310+
if map.contains_key(&namespace) {
311+
errors.push(ValidationError::DuplicateName { name: namespace.into() });
312+
} else {
313+
map.insert(namespace, def);
314+
}
315+
}
316+
317+
ValidationErrors::add_extra_errors(Ok(map), errors)
318+
}
319+
305320
/// Change the visibility of scheduled functions and lifecycle reducers to Internal.
306321
///
307322
fn change_scheduled_functions_and_lifetimes_visibility(
@@ -907,6 +922,7 @@ mod tests {
907922
use spacetimedb_lib::db::raw_def::*;
908923
use spacetimedb_lib::ScheduleAt;
909924
use spacetimedb_primitives::{ColId, ColList, ColSet};
925+
use spacetimedb_sats::raw_identifier::RawIdentifier;
910926
use spacetimedb_sats::{AlgebraicType, AlgebraicTypeRef, AlgebraicValue, ProductType, SumValue};
911927
use v9::{Lifecycle, TableAccess, TableType};
912928

@@ -1299,8 +1315,8 @@ mod tests {
12991315
let mounts = def.mounts();
13001316

13011317
assert_eq!(mounts.len(), 1);
1302-
assert_eq!(mounts[0].0, "authlib");
1303-
assert!(mounts[0].1.table(&expect_identifier("sessions")).is_some());
1318+
let mounted = mounts.get("authlib").expect("authlib mount should exist");
1319+
assert!(mounted.table(&expect_identifier("sessions")).is_some());
13041320
}
13051321

13061322
#[test]
@@ -1319,6 +1335,28 @@ mod tests {
13191335
});
13201336
}
13211337

1338+
#[test]
1339+
fn duplicate_mount_namespace() {
1340+
let raw = RawModuleDefV10 {
1341+
sections: vec![RawModuleDefV10Section::Mounts(vec![
1342+
RawModuleMountV10 {
1343+
namespace: "authlib".to_string(),
1344+
module: RawModuleDefV10::default(),
1345+
},
1346+
RawModuleMountV10 {
1347+
namespace: "authlib".to_string(),
1348+
module: RawModuleDefV10::default(),
1349+
},
1350+
])],
1351+
};
1352+
1353+
let result: Result<ModuleDef> = raw.try_into();
1354+
1355+
expect_error_matching!(result, ValidationError::DuplicateName { name } => {
1356+
name == &RawIdentifier::from("authlib")
1357+
});
1358+
}
1359+
13221360
#[test]
13231361
fn invalid_unique_constraint_column_ref() {
13241362
let mut builder = RawModuleDefV10Builder::new();

crates/schema/src/def/validate/v9.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ pub fn validate(def: RawModuleDefV9) -> Result<ModuleDef> {
166166
lifecycle_reducers,
167167
procedures,
168168
raw_module_def_version: RawModuleDefVersion::V9OrEarlier,
169-
mounts: Vec::new(),
169+
mounts: IndexMap::new(),
170170
})
171171
}
172172

0 commit comments

Comments
 (0)