diff --git a/crates/application/src/deploy_config.rs b/crates/application/src/deploy_config.rs index a3eeab43..81f2bd68 100644 --- a/crates/application/src/deploy_config.rs +++ b/crates/application/src/deploy_config.rs @@ -61,7 +61,7 @@ use model::{ ComponentDiff, SchemaChange, }, - file_based_routing::add_file_based_routing, + file_based_routing::file_based_exports, type_checking::{ CheckedComponent, InitializerEvaluator, @@ -166,7 +166,7 @@ impl Application { ) .await?; - let evaluated_components = self + let mut evaluated_components = self .evaluate_components( config, &component_definition_packages, @@ -203,6 +203,19 @@ impl Application { } schema_change }; + + // TODO(ENG-7533): Clean up exports from the start push response when we've + // updated clients to use `functions` directly. + for (path, definition) in evaluated_components.iter_mut() { + // We don't need to include exports for the root since we don't use codegen + // for the app's `api` object. + if path.is_root() { + continue; + } + anyhow::ensure!(definition.definition.exports.is_empty()); + definition.definition.exports = file_based_exports(&definition.functions)?; + } + let resp = StartPushResponse { environment_variables, external_deps_id, @@ -351,11 +364,6 @@ impl Application { }, ); } - - // Add in file-based routing. - for definition in evaluated_components.values_mut() { - add_file_based_routing(definition)?; - } Ok(evaluated_components) } @@ -491,7 +499,7 @@ impl Application { pub async fn finish_push( &self, identity: Identity, - start_push: StartPushResponse, + mut start_push: StartPushResponse, ) -> anyhow::Result { // Download all source packages. We can remove this once we don't store source // in the database. @@ -506,6 +514,13 @@ impl Application { downloaded_source_packages.insert(definition_path.clone(), package); } + // TODO(ENG-7533): Strip out exports from the `StartPushResponse` since we don't + // want to actually store it in the database. Remove this path once + // we've stopped sending exports down to the client. + for definition in start_push.analysis.values_mut() { + definition.definition.exports = BTreeMap::new(); + } + let mut tx = self.begin(identity.clone()).await?; // Validate that environment variables haven't changed since `start_push`. diff --git a/crates/database/src/bootstrap_model/components/mod.rs b/crates/database/src/bootstrap_model/components/mod.rs index 3c1bd6ad..9010cdf6 100644 --- a/crates/database/src/bootstrap_model/components/mod.rs +++ b/crates/database/src/bootstrap_model/components/mod.rs @@ -53,6 +53,7 @@ use crate::{ SystemIndex, SystemTable, }, + metrics, ResolvedQuery, Transaction, COMPONENT_DEFINITIONS_TABLE, @@ -274,11 +275,15 @@ impl<'a, RT: Runtime> BootstrapComponentsModel<'a, RT> { ComponentDefinitionId::Child(id) => id, }; let component_definition_doc_id = self.resolve_component_definition_id(internal_id)?; - self.tx - .get(component_definition_doc_id) - .await? - .map(TryInto::try_into) - .transpose() + let Some(doc) = self.tx.get(component_definition_doc_id).await? else { + return Ok(None); + }; + let mut doc: ParsedDocument = doc.try_into()?; + if !doc.exports.is_empty() { + metrics::log_nonempty_component_exports(); + doc.exports = BTreeMap::new(); + } + Ok(Some(doc)) } pub async fn load_definition_metadata( @@ -321,7 +326,11 @@ impl<'a, RT: Runtime> BootstrapComponentsModel<'a, RT> { )?; let mut definitions = BTreeMap::new(); while let Some(doc) = query.next(self.tx, None).await? { - let definition: ParsedDocument = doc.try_into()?; + let mut definition: ParsedDocument = doc.try_into()?; + if !definition.exports.is_empty() { + metrics::log_nonempty_component_exports(); + definition.exports = BTreeMap::new(); + } anyhow::ensure!(definitions .insert(definition.path.clone(), definition) .is_none()); diff --git a/crates/database/src/metrics.rs b/crates/database/src/metrics.rs index b2365104..67ebaa12 100644 --- a/crates/database/src/metrics.rs +++ b/crates/database/src/metrics.rs @@ -1010,3 +1010,11 @@ pub mod vector { CancelableTimer::new(&DATABASE_VECTOR_SEARCH_WITH_RETRIES_QUERY_SECONDS) } } + +register_convex_counter!( + DATABASE_NONEMPTY_COMPONENT_EXPORTS_TOTAL, + "Nonempty component definition loaded from database" +); +pub fn log_nonempty_component_exports() { + log_counter(&DATABASE_NONEMPTY_COMPONENT_EXPORTS_TOTAL, 1); +} diff --git a/crates/model/src/components/file_based_routing.rs b/crates/model/src/components/file_based_routing.rs index 27ced686..3fa0c6e2 100644 --- a/crates/model/src/components/file_based_routing.rs +++ b/crates/model/src/components/file_based_routing.rs @@ -8,13 +8,22 @@ use common::{ components::Reference, }; use errors::ErrorMetadata; -use sync_types::CanonicalizedUdfPath; +use sync_types::{ + path::PathComponent, + CanonicalizedModulePath, + CanonicalizedUdfPath, +}; -use super::types::EvaluatedComponentDefinition; -use crate::modules::module_versions::Visibility; +use crate::modules::module_versions::{ + AnalyzedModule, + Visibility, +}; -pub fn add_file_based_routing(evaluated: &mut EvaluatedComponentDefinition) -> anyhow::Result<()> { - for (module_path, module) in &evaluated.functions { +pub fn file_based_exports( + functions: &BTreeMap, +) -> anyhow::Result> { + let mut exports = BTreeMap::new(); + for (module_path, module) in functions { let mut identifiers = vec![]; let stripped = module_path.clone().strip(); @@ -27,7 +36,7 @@ pub fn add_file_based_routing(evaluated: &mut EvaluatedComponentDefinition) -> a path.push(function.name.clone().into()); let (last, prefix) = path.split_last().unwrap(); - let mut current = &mut evaluated.definition.exports; + let mut current = &mut exports; for identifier in prefix { let current_node = current .entry(identifier.clone()) @@ -60,5 +69,5 @@ pub fn add_file_based_routing(evaluated: &mut EvaluatedComponentDefinition) -> a } } } - Ok(()) + Ok(exports) } diff --git a/crates/model/src/components/mod.rs b/crates/model/src/components/mod.rs index af9765fc..985cd975 100644 --- a/crates/model/src/components/mod.rs +++ b/crates/model/src/components/mod.rs @@ -34,6 +34,7 @@ use database::{ Transaction, }; use errors::ErrorMetadata; +use file_based_routing::file_based_exports; use sync_types::{ path::PathComponent, CanonicalizedUdfPath, @@ -153,17 +154,32 @@ impl<'a, RT: Runtime> ComponentsModel<'a, RT> { Ok(result) } + pub async fn load_component_exports( + &mut self, + component_id: ComponentId, + ) -> anyhow::Result> { + let mut modules = BTreeMap::new(); + for module in ModuleModel::new(self.tx) + .get_all_metadata(component_id) + .await? + { + let module = module.into_value(); + let Some(analyze_result) = module.analyze_result else { + continue; + }; + modules.insert(module.path, analyze_result); + } + file_based_exports(&modules) + } + #[async_recursion] pub async fn resolve_export( &mut self, component_id: ComponentId, attributes: &[PathComponent], ) -> anyhow::Result> { - let mut m = BootstrapComponentsModel::new(self.tx); - let definition_id = m.component_definition(component_id).await?; - let definition = m.load_definition_metadata(definition_id).await?; - - let mut current = &definition.exports; + let exports = self.load_component_exports(component_id).await?; + let mut current = &exports; let mut attribute_iter = attributes.iter(); while let Some(attribute) = attribute_iter.next() { let Some(export) = current.get(attribute) else { @@ -344,11 +360,8 @@ impl<'a, RT: Runtime> ComponentsModel<'a, RT> { &mut self, component_id: ComponentId, ) -> anyhow::Result, Resource>> { - let mut m = BootstrapComponentsModel::new(self.tx); - let definition_id = m.component_definition(component_id).await?; - let definition = m.load_definition_metadata(definition_id).await?; - - let mut stack = vec![(vec![], &definition.exports)]; + let exports = self.load_component_exports(component_id).await?; + let mut stack = vec![(vec![], &exports)]; let mut result = BTreeMap::new(); while let Some((path, internal_node)) = stack.pop() { for (name, export) in internal_node { diff --git a/crates/model/src/components/type_checking.rs b/crates/model/src/components/type_checking.rs index ce1ebe3f..6aa644c4 100644 --- a/crates/model/src/components/type_checking.rs +++ b/crates/model/src/components/type_checking.rs @@ -31,7 +31,10 @@ use value::{ TableNamespace, }; -use super::types::EvaluatedComponentDefinition; +use super::{ + file_based_routing::file_based_exports, + types::EvaluatedComponentDefinition, +}; use crate::{ modules::HTTP_MODULE_PATH, virtual_system_mapping, @@ -151,7 +154,7 @@ impl<'a> TypecheckContext<'a> { } // Finally, resolve our exports and build the component. - let component = builder.check_exports(&evaluated.definition.exports)?; + let component = builder.build_exports()?; Ok(component) } @@ -323,11 +326,9 @@ impl<'a> CheckedComponentBuilder<'a> { Ok(()) } - fn check_exports( - self, - exports: &BTreeMap, - ) -> anyhow::Result { - let exports = self.resolve_exports(exports)?; + fn build_exports(self) -> anyhow::Result { + let exports = file_based_exports(&self.evaluated.functions)?; + let exports = self.resolve_exports(&exports)?; Ok(CheckedComponent { definition_path: self.definition_path.clone(), component_path: self.component_path.clone(),