Skip to content

Commit 0f4dbea

Browse files
authored
Revert recursive mounts from module def (#5098)
# Description of Changes Revert recursive module mounts in schema def. This appears to be causing unintended issues for language bindings. The intention was for this to not cause any issues, but it appears to be problematic when adding new enum veriants to the def sections (e.g. for procedures). # API and ABI breaking changes No # Expected complexity level and risk Reverting to a previous version # Testing <!-- Describe any testing you've done, and any testing you'd like your reviewers to do, so that you're confident that all the changes work as expected! --> - [ ] <!-- maybe a test you want to do --> - [ ] <!-- maybe a test you want a reviewer to do, so they can check it off when they're satisfied. -->
1 parent de5466f commit 0f4dbea

6 files changed

Lines changed: 6 additions & 180 deletions

File tree

crates/bindings-csharp/Runtime/Internal/Autogen/RawModuleDefV10Section.g.cs

Lines changed: 1 addition & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/bindings-csharp/Runtime/Internal/Autogen/RawModuleMountV10.g.cs

Lines changed: 0 additions & 36 deletions
This file was deleted.

crates/lib/src/db/raw_def/v10.rs

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -89,17 +89,6 @@ pub enum RawModuleDefV10Section {
8989

9090
/// Names provided explicitly by the user that do not follow from the case conversion policy.
9191
ExplicitNames(ExplicitNames),
92-
93-
/// Mounted submodules, keyed by the namespace they are mounted under.
94-
Mounts(Vec<RawModuleMountV10>),
95-
}
96-
97-
#[derive(Debug, Clone, SpacetimeType)]
98-
#[sats(crate = crate)]
99-
#[cfg_attr(feature = "test", derive(PartialEq, Eq, PartialOrd, Ord))]
100-
pub struct RawModuleMountV10 {
101-
pub namespace: String,
102-
pub module: RawModuleDefV10,
10392
}
10493

10594
#[derive(Debug, Clone, Copy, Default, SpacetimeType)]
@@ -521,14 +510,6 @@ pub struct RawViewDefV10 {
521510
}
522511

523512
impl RawModuleDefV10 {
524-
/// Get the mounted submodules for this module definition.
525-
pub fn mounts(&self) -> Option<&Vec<RawModuleMountV10>> {
526-
self.sections.iter().find_map(|s| match s {
527-
RawModuleDefV10Section::Mounts(mounts) => Some(mounts),
528-
_ => None,
529-
})
530-
}
531-
532513
/// Get the types section, if present.
533514
pub fn types(&self) -> Option<&Vec<RawTypeDefV10>> {
534515
self.sections.iter().find_map(|s| match s {

crates/schema/src/def.rs

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ use spacetimedb_data_structures::map::{Equivalent, HashMap};
3333
use spacetimedb_lib::db::raw_def;
3434
use spacetimedb_lib::db::raw_def::v10::{
3535
ExplicitNames, RawConstraintDefV10, RawIndexDefV10, RawLifeCycleReducerDefV10, RawModuleDefV10,
36-
RawModuleDefV10Section, RawModuleMountV10, RawProcedureDefV10, RawReducerDefV10, RawRowLevelSecurityDefV10,
37-
RawScheduleDefV10, RawScopedTypeNameV10, RawSequenceDefV10, RawTableDefV10, RawTypeDefV10, RawViewDefV10,
36+
RawModuleDefV10Section, RawProcedureDefV10, RawReducerDefV10, RawRowLevelSecurityDefV10, RawScheduleDefV10,
37+
RawScopedTypeNameV10, RawSequenceDefV10, RawTableDefV10, RawTypeDefV10, RawViewDefV10,
3838
};
3939
use spacetimedb_lib::db::raw_def::v9::{
4040
Lifecycle, RawColumnDefaultValueV9, RawConstraintDataV9, RawConstraintDefV9, RawIndexAlgorithm, RawIndexDefV9,
@@ -151,9 +151,6 @@ pub struct ModuleDef {
151151
/// was authored under.
152152
#[allow(unused)]
153153
raw_module_def_version: RawModuleDefVersion,
154-
155-
/// Mounted submodules, keyed by the namespace they are mounted under.
156-
mounts: IndexMap<String, ModuleDef>,
157154
}
158155

159156
#[derive(Debug, Clone, Copy, Eq, PartialEq)]
@@ -170,11 +167,6 @@ impl ModuleDef {
170167
self.raw_module_def_version
171168
}
172169

173-
/// The mounted submodules of the module definition.
174-
pub fn mounts(&self) -> &IndexMap<String, ModuleDef> {
175-
&self.mounts
176-
}
177-
178170
/// The tables of the module definition.
179171
pub fn tables(&self) -> impl Iterator<Item = &TableDef> {
180172
self.tables.values()
@@ -445,7 +437,6 @@ impl From<ModuleDef> for RawModuleDefV9 {
445437
row_level_security_raw,
446438
procedures,
447439
raw_module_def_version: _,
448-
mounts: _,
449440
} = val;
450441

451442
// Extract column defaults from tables before consuming tables
@@ -502,7 +493,6 @@ impl From<ModuleDef> for RawModuleDefV10 {
502493
row_level_security_raw,
503494
procedures,
504495
raw_module_def_version: _,
505-
mounts,
506496
} = val;
507497

508498
let mut sections = Vec::new();
@@ -615,17 +605,6 @@ impl From<ModuleDef> for RawModuleDefV10 {
615605
// Always emit ExplicitNames so canonical names survive the round-trip.
616606
sections.push(RawModuleDefV10Section::ExplicitNames(explicit_names));
617607

618-
let mounts: Vec<_> = mounts
619-
.into_iter()
620-
.map(|(namespace, module)| RawModuleMountV10 {
621-
namespace,
622-
module: module.into(),
623-
})
624-
.collect();
625-
if !mounts.is_empty() {
626-
sections.push(RawModuleDefV10Section::Mounts(mounts));
627-
}
628-
629608
RawModuleDefV10 { sections }
630609
}
631610
}

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

Lines changed: 3 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -81,12 +81,6 @@ pub fn validate(def: RawModuleDefV10) -> Result<ModuleDef> {
8181
.cloned()
8282
.map(ExplicitNamesLookup::new)
8383
.unwrap_or_default();
84-
let mounts = def
85-
.mounts()
86-
.into_iter()
87-
.flat_map(|mounts| mounts.iter().cloned())
88-
.map(validate_mount)
89-
.collect_all_errors::<Vec<_>>();
9084

9185
// Original `typespace` needs to be preserved to be assign `accesor_name`s to columns.
9286
let typespace_with_accessor_names = typespace.clone();
@@ -269,12 +263,8 @@ pub fn validate(def: RawModuleDefV10) -> Result<ModuleDef> {
269263
.map(|rls| (rls.sql.clone(), rls.to_owned()))
270264
.collect();
271265

272-
let (tables, types, reducers, procedures, views, mounts) = (tables_types_reducers_procedures_views, mounts)
273-
.combine_errors()
274-
.and_then(|((tables, types, reducers, procedures, views), mounts)| {
275-
validate_mount_names_are_unique(mounts).map(|mounts| (tables, types, reducers, procedures, views, mounts))
276-
})
277-
.map_err(|errors: ValidationErrors| errors.sort_deduplicate())?;
266+
let (tables, types, reducers, procedures, views) =
267+
(tables_types_reducers_procedures_views).map_err(|errors| errors.sort_deduplicate())?;
278268

279269
let typespace_for_generate = typespace_for_generate.finish();
280270

@@ -291,32 +281,9 @@ pub fn validate(def: RawModuleDefV10) -> Result<ModuleDef> {
291281
lifecycle_reducers,
292282
procedures,
293283
raw_module_def_version: RawModuleDefVersion::V10,
294-
mounts,
295284
})
296285
}
297286

298-
fn validate_mount(mount: RawModuleMountV10) -> Result<(String, ModuleDef)> {
299-
Identifier::new(mount.namespace.clone().into())
300-
.map_err(|error| ValidationErrors::from(ValidationError::IdentifierError { error }))?;
301-
302-
Ok((mount.namespace, validate(mount.module)?))
303-
}
304-
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-
320287
/// Change the visibility of scheduled functions and lifecycle reducers to Internal.
321288
///
322289
fn change_scheduled_functions_and_lifetimes_visibility(
@@ -915,14 +882,11 @@ mod tests {
915882

916883
use itertools::Itertools;
917884
use spacetimedb_data_structures::expect_error_matching;
918-
use spacetimedb_lib::db::raw_def::v10::{
919-
CaseConversionPolicy, RawModuleDefV10, RawModuleDefV10Builder, RawModuleDefV10Section, RawModuleMountV10,
920-
};
885+
use spacetimedb_lib::db::raw_def::v10::{CaseConversionPolicy, RawModuleDefV10Builder};
921886
use spacetimedb_lib::db::raw_def::v9::{btree, direct, hash};
922887
use spacetimedb_lib::db::raw_def::*;
923888
use spacetimedb_lib::ScheduleAt;
924889
use spacetimedb_primitives::{ColId, ColList, ColSet};
925-
use spacetimedb_sats::raw_identifier::RawIdentifier;
926890
use spacetimedb_sats::{AlgebraicType, AlgebraicTypeRef, AlgebraicValue, ProductType, SumValue};
927891
use v9::{Lifecycle, TableAccess, TableType};
928892

@@ -1297,66 +1261,6 @@ mod tests {
12971261
});
12981262
}
12991263

1300-
#[test]
1301-
fn validates_mounted_submodules_recursively() {
1302-
let mut mounted_builder = RawModuleDefV10Builder::new();
1303-
mounted_builder
1304-
.build_table_with_new_type("Sessions", ProductType::from([("id", AlgebraicType::U64)]), true)
1305-
.finish();
1306-
1307-
let raw = RawModuleDefV10 {
1308-
sections: vec![RawModuleDefV10Section::Mounts(vec![RawModuleMountV10 {
1309-
namespace: "authlib".to_string(),
1310-
module: mounted_builder.finish(),
1311-
}])],
1312-
};
1313-
1314-
let def: ModuleDef = raw.try_into().expect("mounted module should validate");
1315-
let mounts = def.mounts();
1316-
1317-
assert_eq!(mounts.len(), 1);
1318-
let mounted = mounts.get("authlib").expect("authlib mount should exist");
1319-
assert!(mounted.table(&expect_identifier("sessions")).is_some());
1320-
}
1321-
1322-
#[test]
1323-
fn invalid_mount_namespace() {
1324-
let raw = RawModuleDefV10 {
1325-
sections: vec![RawModuleDefV10Section::Mounts(vec![RawModuleMountV10 {
1326-
namespace: "".to_string(),
1327-
module: RawModuleDefV10::default(),
1328-
}])],
1329-
};
1330-
1331-
let result: Result<ModuleDef> = raw.try_into();
1332-
1333-
expect_error_matching!(result, ValidationError::IdentifierError { error } => {
1334-
error == &IdentifierError::Empty {}
1335-
});
1336-
}
1337-
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-
13601264
#[test]
13611265
fn invalid_unique_constraint_column_ref() {
13621266
let mut builder = RawModuleDefV10Builder::new();

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,6 @@ pub fn validate(def: RawModuleDefV9) -> Result<ModuleDef> {
166166
lifecycle_reducers,
167167
procedures,
168168
raw_module_def_version: RawModuleDefVersion::V9OrEarlier,
169-
mounts: IndexMap::new(),
170169
})
171170
}
172171

0 commit comments

Comments
 (0)