Skip to content

Commit 4cb33d1

Browse files
ldanilekConvex, Inc.
authored and
Convex, Inc.
committed
[CX-6721] remove ByComponentDefinition namespaces (#26847)
it's confusing and not *that* benefical to have ByComponentDefinition namespaces. It was only applying to `_modules` and `_module_versions` and `_source_packages`, but we can explode those namespaces to be ByComponent. GitOrigin-RevId: 569673635270f13cdecf37918114806a308e331d
1 parent 60acdf6 commit 4cb33d1

File tree

19 files changed

+165
-213
lines changed

19 files changed

+165
-213
lines changed

crates/application/src/lib.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,9 @@ use common::{
4242
components::{
4343
CanonicalizedComponentFunctionPath,
4444
CanonicalizedComponentModulePath,
45-
ComponentDefinitionId,
4645
ComponentDefinitionPath,
4746
ComponentFunctionPath,
47+
ComponentId,
4848
ComponentPath,
4949
},
5050
document::{
@@ -802,7 +802,7 @@ impl<RT: Runtime> Application<RT> {
802802
) -> anyhow::Result<Option<String>> {
803803
let mut tx = self.begin(identity).await?;
804804
let path = CanonicalizedComponentModulePath {
805-
component: ComponentDefinitionId::Root,
805+
component: ComponentId::Root,
806806
module_path: path.canonicalize(),
807807
};
808808
let Some(metadata) = ModuleModel::new(&mut tx).get_metadata(path.clone()).await? else {
@@ -1639,7 +1639,7 @@ impl<RT: Runtime> Application<RT> {
16391639
tx: &mut Transaction<RT>,
16401640
) -> anyhow::Result<()> {
16411641
let path = CanonicalizedComponentModulePath {
1642-
component: ComponentDefinitionId::Root,
1642+
component: ComponentId::Root,
16431643
module_path: AUTH_CONFIG_FILE_NAME.parse()?,
16441644
};
16451645
let auth_config_metadata = ModuleModel::new(tx).get_metadata(path).await?;
@@ -1967,7 +1967,7 @@ impl<RT: Runtime> Application<RT> {
19671967
ModuleModel::new(&mut tx)
19681968
.put_standalone(
19691969
CanonicalizedComponentModulePath {
1970-
component: ComponentDefinitionId::Root,
1970+
component: ComponentId::Root,
19711971
module_path: module_path.clone(),
19721972
},
19731973
module.source,

crates/application/src/tests/source_package.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use std::collections::BTreeMap;
22

33
use anyhow::Context;
44
use common::{
5-
components::ComponentDefinitionId,
5+
components::ComponentId,
66
runtime::Runtime,
77
types::ModuleEnvironment,
88
};
@@ -73,7 +73,7 @@ pub async fn assemble_package<RT: Runtime>(
7373
modifications: BTreeMap<CanonicalizedModulePath, Option<ModuleConfig>>,
7474
) -> anyhow::Result<Vec<ModuleConfig>> {
7575
let existing_modules = ModuleModel::new(tx)
76-
.get_application_modules(ComponentDefinitionId::Root, module_loader)
76+
.get_application_modules(ComponentId::Root, module_loader)
7777
.await?;
7878
let mut modules = BTreeMap::new();
7979
for (path, module) in existing_modules {

crates/common/src/bootstrap_model/tables.rs

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -145,10 +145,6 @@ enum SerializedTableNamespace {
145145
#[serde(skip_serializing_if = "Option::is_none")]
146146
id: Option<String>,
147147
},
148-
ByComponentDefinition {
149-
#[serde(skip_serializing_if = "Option::is_none")]
150-
id: Option<String>,
151-
},
152148
}
153149

154150
fn table_namespace_from_serialized(
@@ -160,12 +156,6 @@ fn table_namespace_from_serialized(
160156
TableNamespace::ByComponent(id.parse()?)
161157
},
162158
Some(SerializedTableNamespace::ByComponent { id: None }) => TableNamespace::RootComponent,
163-
Some(SerializedTableNamespace::ByComponentDefinition { id: Some(id) }) => {
164-
TableNamespace::ByComponentDefinition(id.parse()?)
165-
},
166-
Some(SerializedTableNamespace::ByComponentDefinition { id: None }) => {
167-
TableNamespace::RootComponentDefinition
168-
},
169159
})
170160
}
171161

@@ -184,16 +174,6 @@ fn table_namespace_to_serialized(
184174
TableNamespace::RootComponent => {
185175
Ok(Some(SerializedTableNamespace::ByComponent { id: None }))
186176
},
187-
TableNamespace::ByComponentDefinition(id) => {
188-
Ok(Some(SerializedTableNamespace::ByComponentDefinition {
189-
id: Some(id.to_string()),
190-
}))
191-
},
192-
TableNamespace::RootComponentDefinition => {
193-
Ok(Some(SerializedTableNamespace::ByComponentDefinition {
194-
id: None,
195-
}))
196-
},
197177
}
198178
}
199179

crates/common/src/components/mod.rs

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -110,19 +110,3 @@ impl ComponentDefinitionId {
110110
matches!(self, ComponentDefinitionId::Root)
111111
}
112112
}
113-
114-
impl From<ComponentDefinitionId> for TableNamespace {
115-
fn from(value: ComponentDefinitionId) -> Self {
116-
if *COMPONENTS_ENABLED {
117-
match value {
118-
ComponentDefinitionId::Root => TableNamespace::RootComponentDefinition,
119-
ComponentDefinitionId::Child(id) => TableNamespace::ByComponentDefinition(id),
120-
}
121-
} else {
122-
match value {
123-
ComponentDefinitionId::Root => TableNamespace::Global,
124-
ComponentDefinitionId::Child(_id) => TableNamespace::Global,
125-
}
126-
}
127-
}
128-
}

crates/common/src/components/module_paths.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
use sync_types::CanonicalizedModulePath;
22

3-
use super::ComponentDefinitionId;
3+
use super::ComponentId;
44

55
#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)]
66
#[cfg_attr(any(test, feature = "testing"), derive(proptest_derive::Arbitrary))]
77
pub struct CanonicalizedComponentModulePath {
8-
pub component: ComponentDefinitionId,
8+
pub component: ComponentId,
99
pub module_path: CanonicalizedModulePath,
1010
}
1111

crates/database/src/bootstrap_model/components/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -309,9 +309,9 @@ impl<'a, RT: Runtime> BootstrapComponentsModel<'a, RT> {
309309
&mut self,
310310
path: CanonicalizedComponentFunctionPath,
311311
) -> anyhow::Result<CanonicalizedComponentModulePath> {
312-
let (definition_id, _) = self.component_path_to_ids(path.component).await?;
312+
let (_, component) = self.component_path_to_ids(path.component).await?;
313313
Ok(CanonicalizedComponentModulePath {
314-
component: definition_id,
314+
component,
315315
module_path: path.udf_path.module().clone(),
316316
})
317317
}

crates/isolate/src/environment/action/phase.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -148,14 +148,14 @@ impl<RT: Runtime> ActionPhase<RT> {
148148
let result = if !*COMPONENTS_ENABLED {
149149
anyhow::ensure!(self.component.is_root());
150150
ModuleModel::new(&mut tx)
151-
.get_all_metadata(ComponentDefinitionId::Root)
151+
.get_all_metadata(ComponentId::Root)
152152
.await?
153153
} else {
154154
let metadata = BootstrapComponentsModel::new(&mut tx)
155155
.resolve_path(self.component.clone())
156156
.await?
157157
.context("Failed to find component")?;
158-
let (component_id, definition_id) = if self.component.is_root() {
158+
let (component_id, _) = if self.component.is_root() {
159159
(ComponentId::Root, ComponentDefinitionId::Root)
160160
} else {
161161
(
@@ -164,7 +164,7 @@ impl<RT: Runtime> ActionPhase<RT> {
164164
)
165165
};
166166
let module_metadata = ModuleModel::new(&mut tx)
167-
.get_all_metadata(definition_id)
167+
.get_all_metadata(component_id)
168168
.await?;
169169

170170
let loaded_resources = ComponentsModel::new(&mut tx)

crates/isolate/src/environment/helpers/module_loader.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use anyhow::{
88
Context,
99
};
1010
use common::{
11-
components::ComponentDefinitionId,
11+
components::ComponentId,
1212
document::ParsedDocument,
1313
knobs::READ_MODULES_FROM_SOURCE_PACKAGE,
1414
runtime::Runtime,
@@ -109,10 +109,9 @@ async fn download_module_source_from_package<RT: Runtime>(
109109
let component = match namespace {
110110
// TODO(lee) global namespace should not have modules, but for existing data this is how
111111
// it's represented.
112-
TableNamespace::Global => ComponentDefinitionId::Root,
113-
TableNamespace::RootComponentDefinition => ComponentDefinitionId::Root,
114-
TableNamespace::ByComponentDefinition(id) => ComponentDefinitionId::Child(id),
115-
_ => anyhow::bail!("_modules table namespace {namespace:?} is not a component definition"),
112+
TableNamespace::Global => ComponentId::Root,
113+
TableNamespace::RootComponent => ComponentId::Root,
114+
TableNamespace::ByComponent(id) => ComponentId::Child(id),
116115
};
117116
for module_metadata in ModuleModel::new(tx).get_all_metadata(component).await? {
118117
match package.remove(&module_metadata.path) {

crates/isolate/src/environment/udf/phase.rs

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ use anyhow::Context;
1313
use common::{
1414
components::{
1515
CanonicalizedComponentModulePath,
16-
ComponentDefinitionId,
1716
ComponentId,
1817
ComponentPath,
1918
},
@@ -92,7 +91,6 @@ enum UdfPreloaded {
9291
observed_time_during_execution: AtomicBool,
9392
env_vars: PreloadedEnvironmentVariables,
9493
component: ComponentId,
95-
component_definition: ComponentDefinitionId,
9694
},
9795
}
9896

@@ -145,7 +143,7 @@ impl<RT: Runtime> UdfPhase<RT> {
145143
.await?;
146144

147145
let component_path = self.component_path.clone();
148-
let (component_definition, component) = with_release_permit(
146+
let (_, component) = with_release_permit(
149147
timeout,
150148
permit_slot,
151149
BootstrapComponentsModel::new(self.tx_mut()?).component_path_to_ids(component_path),
@@ -159,7 +157,6 @@ impl<RT: Runtime> UdfPhase<RT> {
159157
observed_time_during_execution: AtomicBool::new(false),
160158
env_vars,
161159
component,
162-
component_definition,
163160
};
164161
Ok(())
165162
}
@@ -183,15 +180,11 @@ impl<RT: Runtime> UdfPhase<RT> {
183180
format!("Can't dynamically import {module_path:?} in a query or mutation")
184181
));
185182
}
186-
let UdfPreloaded::Ready {
187-
component_definition,
188-
..
189-
} = &self.preloaded
190-
else {
183+
let UdfPreloaded::Ready { component, .. } = &self.preloaded else {
191184
anyhow::bail!("Phase not initialized");
192185
};
193186
let path = CanonicalizedComponentModulePath {
194-
component: *component_definition,
187+
component: *component,
195188
module_path: module_path.clone().canonicalize(),
196189
};
197190
let module = with_release_permit(

crates/isolate/src/isolate2/runner.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ use anyhow::Context as AnyhowContext;
99
use common::{
1010
components::{
1111
CanonicalizedComponentModulePath,
12-
ComponentDefinitionId,
1312
ComponentFunctionPath,
1413
ComponentId,
1514
Reference,
@@ -505,7 +504,7 @@ async fn run_request<RT: Runtime>(
505504
while let Some(module_path) = stack.pop() {
506505
let module_specifier = module_specifier_from_path(&module_path)?;
507506
let path = CanonicalizedComponentModulePath {
508-
component: ComponentDefinitionId::Root,
507+
component: ComponentId::Root,
509508
module_path: module_path.clone(),
510509
};
511510
let Some(module_metadata) = module_loader.get_module(tx, path).await? else {

crates/isolate/src/tests/analyze.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use std::{
44
};
55

66
use common::{
7-
components::ComponentDefinitionId,
7+
components::ComponentId,
88
types::{
99
ModuleEnvironment,
1010
RoutableMethod,
@@ -50,7 +50,7 @@ async fn test_analyze_module(rt: TestRuntime) -> anyhow::Result<()> {
5050
let modules = {
5151
let mut tx = t.database.begin(Identity::system()).await?;
5252
ModuleModel::new(&mut tx)
53-
.get_application_modules(ComponentDefinitionId::Root, t.module_loader.as_ref())
53+
.get_application_modules(ComponentId::Root, t.module_loader.as_ref())
5454
.await?
5555
};
5656

@@ -209,7 +209,7 @@ async fn test_analyze_http_errors(rt: TestRuntime) -> anyhow::Result<()> {
209209
let mut modules = {
210210
let mut tx = t.database.begin(Identity::system()).await?;
211211
ModuleModel::new(&mut tx)
212-
.get_application_modules(ComponentDefinitionId::Root, t.module_loader.as_ref())
212+
.get_application_modules(ComponentId::Root, t.module_loader.as_ref())
213213
.await?
214214
};
215215

@@ -246,7 +246,7 @@ async fn test_analyze_function(rt: TestRuntime) -> anyhow::Result<()> {
246246
let modules = {
247247
let mut tx = t.database.begin(Identity::system()).await?;
248248
ModuleModel::new(&mut tx)
249-
.get_application_modules(ComponentDefinitionId::Root, t.module_loader.as_ref())
249+
.get_application_modules(ComponentId::Root, t.module_loader.as_ref())
250250
.await?
251251
};
252252

@@ -336,7 +336,7 @@ async fn test_analyze_internal_function(rt: TestRuntime) -> anyhow::Result<()> {
336336
let modules = {
337337
let mut tx = t.database.begin(Identity::system()).await?;
338338
ModuleModel::new(&mut tx)
339-
.get_application_modules(ComponentDefinitionId::Root, t.module_loader.as_ref())
339+
.get_application_modules(ComponentId::Root, t.module_loader.as_ref())
340340
.await?
341341
};
342342

@@ -540,7 +540,7 @@ async fn test_analyze_imports_are_none(rt: TestRuntime) -> anyhow::Result<()> {
540540
let mut modules = {
541541
let mut tx = t.database.begin(Identity::system()).await?;
542542
ModuleModel::new(&mut tx)
543-
.get_application_modules(ComponentDefinitionId::Root, t.module_loader.as_ref())
543+
.get_application_modules(ComponentId::Root, t.module_loader.as_ref())
544544
.await?
545545
};
546546

crates/local_backend/src/deploy_config2.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -749,7 +749,7 @@ async fn finish_push_handler(
749749
.await?;
750750

751751
// Diff the component definitions.
752-
let definition_diffs = ComponentDefinitionConfigModel::new(&mut tx)
752+
let (definition_diffs, modules_by_definition) = ComponentDefinitionConfigModel::new(&mut tx)
753753
.apply_component_definitions_diff(
754754
&start_push.analysis,
755755
&start_push.component_definition_packages,
@@ -759,7 +759,11 @@ async fn finish_push_handler(
759759

760760
// Diff component tree.
761761
let component_diffs = ComponentConfigModel::new(&mut tx)
762-
.apply_component_tree_diff(&start_push.app, &start_push.schema_change)
762+
.apply_component_tree_diff(
763+
&start_push.app,
764+
&start_push.schema_change,
765+
modules_by_definition,
766+
)
763767
.await?;
764768

765769
if !req.dry_run {

0 commit comments

Comments
 (0)