diff --git a/crates/schema/src/auto_migrate.rs b/crates/schema/src/auto_migrate.rs index 6d169a902cc..f2fba813fa8 100644 --- a/crates/schema/src/auto_migrate.rs +++ b/crates/schema/src/auto_migrate.rs @@ -1101,6 +1101,7 @@ mod tests { AlgebraicType, AlgebraicValue, ProductType, ScheduleAt, }; use spacetimedb_primitives::ColId; + use v10::{ExplicitNames, RawModuleDefV10Builder}; use v9::{RawModuleDefV9Builder, TableAccess}; use validate::tests::expect_identifier; @@ -1113,6 +1114,15 @@ mod tests { .expect("new_def should be a valid database definition") } + fn create_module_def_v10(build_module: impl Fn(&mut RawModuleDefV10Builder)) -> ModuleDef { + let mut builder = RawModuleDefV10Builder::new(); + build_module(&mut builder); + builder + .finish() + .try_into() + .expect("new_def should be a valid database definition") + } + fn initial_module_def() -> ModuleDef { let mut builder = RawModuleDefV9Builder::new(); let schedule_at = builder.add_type::(); @@ -1982,6 +1992,53 @@ mod tests { } } + #[test] + fn migrate_view_with_explicit_name() { + fn module_def() -> ModuleDef { + create_module_def_v10(|builder| { + let return_type_ref = builder.add_algebraic_type( + [], + "Person", + AlgebraicType::product([("PersonId", AlgebraicType::U64)]), + true, + ); + builder.add_view( + "PersonAtLevel2", + 0, + true, + true, + ProductType::from([("Level", AlgebraicType::U32)]), + AlgebraicType::array(AlgebraicType::Ref(return_type_ref)), + ); + + let mut explicit = ExplicitNames::default(); + explicit.insert_function("PersonAtLevel2", "Level2Person"); + builder.add_explicit_names(explicit); + }) + } + + let old_def = module_def(); + let new_def = module_def(); + let level_2_person = expect_identifier("Level2Person"); + + let plan = ponder_auto_migrate(&old_def, &new_def).expect("auto migration should succeed"); + let steps = &plan.steps[..]; + + assert!(!plan.disconnects_all_users(), "{plan:#?}"); + assert!( + steps.contains(&AutoMigrateStep::UpdateView(&level_2_person)), + "steps: {steps:?}" + ); + assert!( + !steps.contains(&AutoMigrateStep::AddView(&level_2_person)), + "steps: {steps:?}" + ); + assert!( + !steps.contains(&AutoMigrateStep::RemoveView(&level_2_person)), + "steps: {steps:?}" + ); + } + #[test] fn migrate_view_disconnect_clients() { struct TestCase { diff --git a/crates/schema/src/def/validate/v10.rs b/crates/schema/src/def/validate/v10.rs index 7ebbaae06d4..e840770a8c2 100644 --- a/crates/schema/src/def/validate/v10.rs +++ b/crates/schema/src/def/validate/v10.rs @@ -359,8 +359,13 @@ impl<'a> ModuleValidatorV10<'a> { }) })?; - let mut table_validator = - TableValidator::new(raw_table_name.clone(), product_type_ref, product_type, &mut self.core)?; + let mut table_validator = TableValidator::new( + raw_table_name.clone(), + product_type_ref, + product_type, + &mut self.core, + CoreValidator::resolve_table_ident, + )?; let table_ident = table_validator.table_ident.clone(); @@ -701,10 +706,12 @@ impl<'a> ModuleValidatorV10<'a> { }) })?; + let name = self.core.resolve_function_ident(accessor_name.clone())?; + let params_for_generate = self.core .params_for_generate(¶ms, |position, arg_name| TypeLocation::ViewArg { - view_name: accessor_name.clone(), + view_name: name.as_raw().clone(), position, arg_name, })?; @@ -718,13 +725,11 @@ impl<'a> ModuleValidatorV10<'a> { let return_type_for_generate = self.core.validate_for_type_use( || TypeLocation::ViewReturn { - view_name: accessor_name.clone(), + view_name: name.as_raw().clone(), }, &return_type_for_generate_input, ); - let name = self.core.resolve_function_ident(accessor_name.clone())?; - let mut view_validator = ViewValidator::new( accessor_name.clone(), product_type_ref, @@ -2112,4 +2117,41 @@ mod tests { assert_eq!(schedule.at_column, 1.into()); assert_eq!(schedule.function_kind, FunctionKind::Reducer); } + + #[test] + fn test_child_defs_use_explicit_view_name() { + use spacetimedb_lib::db::raw_def::v10::ExplicitNames; + + let id = |s: &str| Identifier::for_test(s); + + let mut builder = RawModuleDefV10Builder::new(); + let return_type_ref = builder.add_algebraic_type( + [], + "Person", + AlgebraicType::product([("PersonId", AlgebraicType::U64)]), + true, + ); + builder.add_view( + "PersonAtLevel2", + 0, + true, + true, + ProductType::from([("Level", AlgebraicType::U32)]), + AlgebraicType::array(AlgebraicType::Ref(return_type_ref)), + ); + + let mut explicit = ExplicitNames::default(); + explicit.insert_function("PersonAtLevel2", "Level2Person"); + builder.add_explicit_names(explicit); + + let def: ModuleDef = builder.finish().try_into().unwrap(); + let view = def + .view("Level2Person") + .expect("view should use explicit canonical name"); + + assert_eq!(view.name, id("Level2Person")); + assert_eq!(view.accessor_name, id("PersonAtLevel2")); + assert_eq!(view.return_columns[0].view_name, id("Level2Person")); + assert_eq!(view.param_columns[0].view_name, id("Level2Person")); + } } diff --git a/crates/schema/src/def/validate/v9.rs b/crates/schema/src/def/validate/v9.rs index d040435afe5..ec1c2f35919 100644 --- a/crates/schema/src/def/validate/v9.rs +++ b/crates/schema/src/def/validate/v9.rs @@ -201,8 +201,13 @@ impl ModuleValidatorV9<'_> { }) })?; - let mut table_in_progress = - TableValidator::new(raw_table_name.clone(), product_type_ref, product_type, &mut self.core)?; + let mut table_in_progress = TableValidator::new( + raw_table_name.clone(), + product_type_ref, + product_type, + &mut self.core, + CoreValidator::resolve_table_ident, + )?; let table_ident = table_in_progress.table_ident.clone(); @@ -928,6 +933,7 @@ impl CoreValidator<'_> { /// 2. Insert view names into the global namespace. pub(crate) struct ViewValidator<'a, 'b> { inner: TableValidator<'a, 'b>, + view_name: Identifier, params: &'a ProductType, params_for_generate: &'a [(Identifier, AlgebraicTypeUse)], } @@ -941,8 +947,12 @@ impl<'a, 'b> ViewValidator<'a, 'b> { params_for_generate: &'a [(Identifier, AlgebraicTypeUse)], module_validator: &'a mut CoreValidator<'b>, ) -> Result { + let view_name = module_validator.resolve_function_ident(raw_name.clone())?; Ok(Self { - inner: TableValidator::new(raw_name, product_type_ref, product_type, module_validator)?, + inner: TableValidator::new(raw_name, product_type_ref, product_type, module_validator, |_, _| { + Ok(view_name.clone()) + })?, + view_name, params, params_for_generate, }) @@ -967,25 +977,14 @@ impl<'a, 'b> ViewValidator<'a, 'b> { .unwrap_or_else(|| RawIdentifier::new(format!("param_{}", col_id))), ); - // This error will be created multiple times if the view name is invalid, - // but we sort and deduplicate the error stream afterwards, - // so it isn't a huge deal. - // - // This is necessary because we require `ErrorStream` to be nonempty. - // We need to put something in there if the view name is invalid. - let view_name = self - .inner - .module_validator - .resolve_identifier_with_case(self.inner.raw_name.clone()); - - let (name, view_name) = (name, view_name).combine_errors()?; + let name = name?; Ok(ViewParamDef { name, ty: column.algebraic_type.clone(), ty_for_generate: ty_for_generate.clone(), col_id, - view_name, + view_name: self.view_name.clone(), }) } @@ -1004,7 +1003,11 @@ impl<'a, 'b> ViewValidator<'a, 'b> { } } -/// A partially validated table. +/// A partially validated table-shaped definition. +/// +/// This is also used by [`ViewValidator`]. Tables and views do not resolve +/// their source names in the same namespace, so callers provide the +/// appropriate name resolver. pub(crate) struct TableValidator<'a, 'b> { pub(crate) module_validator: &'a mut CoreValidator<'b>, raw_name: RawIdentifier, @@ -1020,8 +1023,9 @@ impl<'a, 'b> TableValidator<'a, 'b> { product_type_ref: AlgebraicTypeRef, product_type: &'a ProductType, module_validator: &'a mut CoreValidator<'b>, + resolve_name: impl FnOnce(&CoreValidator<'b>, RawIdentifier) -> Result, ) -> Result { - let table_ident = module_validator.resolve_table_ident(raw_name.clone())?; + let table_ident = resolve_name(module_validator, raw_name.clone())?; Ok(Self { raw_name, product_type_ref,