Skip to content

Commit

Permalink
Unmount and remount connects to the same component data (#28828)
Browse files Browse the repository at this point in the history
This PR implements some of the desired component unmount + remount behavior.
* Adds a new `ComponentState` to the metadata, unmounting instead of deleting the component when you push a configuration without an existing component.
* Remounts components at the same path to the same data.
* Adds an application-level test that unmounts + remounts by pushing a `mounted` project, then an `empty` project, then the `mounted` project again.

This does not make the data read-only while unmounted, will follow up about that.

GitOrigin-RevId: 9fe3753af05308066a0cb385d6357d3bb1a828a1
  • Loading branch information
emmaling27 authored and Convex, Inc. committed Aug 9, 2024
1 parent 7d99d72 commit e785a05
Show file tree
Hide file tree
Showing 27 changed files with 1,209 additions and 17 deletions.
86 changes: 85 additions & 1 deletion crates/application/src/tests/components.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use common::{
bootstrap_model::components::ComponentState,
components::{
CanonicalizedComponentFunctionPath,
ComponentId,
Expand All @@ -12,6 +13,7 @@ use common::{
RequestId,
};
use database::{
BootstrapComponentsModel,
TableModel,
UserFacingModel,
};
Expand Down Expand Up @@ -44,12 +46,21 @@ async fn run_function(
application: &Application<TestRuntime>,
udf_path: CanonicalizedUdfPath,
args: Vec<JsonValue>,
) -> anyhow::Result<Result<FunctionReturn, FunctionError>> {
run_component_function(application, udf_path, args, ComponentPath::root()).await
}

async fn run_component_function(
application: &Application<TestRuntime>,
udf_path: CanonicalizedUdfPath,
args: Vec<JsonValue>,
component: ComponentPath,
) -> anyhow::Result<Result<FunctionReturn, FunctionError>> {
application
.any_udf(
RequestId::new(),
CanonicalizedComponentFunctionPath {
component: ComponentPath::root(),
component,
udf_path,
},
args,
Expand Down Expand Up @@ -246,3 +257,76 @@ async fn test_delete_tables_in_component(rt: TestRuntime) -> anyhow::Result<()>
assert!(!table_model.table_exists(table_namespace, &table_name));
Ok(())
}

#[convex_macro::test_runtime]
async fn test_unmount_and_remount_component(rt: TestRuntime) -> anyhow::Result<()> {
let application = Application::new_for_tests(&rt).await?;
application.load_component_tests_modules("mounted").await?;
let component_path = ComponentPath::deserialize(Some("component"))?;
run_component_function(
&application,
"messages:insertMessage".parse()?,
vec![assert_obj!("channel" => "sports", "text" => "the celtics won!").into()],
component_path.clone(),
)
.await??;

// Unmount component
application.load_component_tests_modules("empty").await?;

let mut tx = application.begin(Identity::system()).await?;
let mut components_model = BootstrapComponentsModel::new(&mut tx);
let (_, component_id) = components_model
.component_path_to_ids(component_path.clone())
.await?;
let component = components_model
.load_component(component_id)
.await?
.unwrap();
assert!(matches!(component.state, ComponentState::Unmounted));

// Data should still exist in unmounted component tables
let mut table_model = TableModel::new(&mut tx);
let count = table_model
.count(component_id.into(), &"messages".parse()?)
.await?;
assert_eq!(count, 1);

// Calling component function after the component is unmounted should fail
let result = run_component_function(
&application,
"messages:listMessages".parse()?,
vec![assert_obj!().into()],
ComponentPath::deserialize(Some("component"))?,
)
.await?;
assert!(result.is_err());

// Remount the component
application.load_component_tests_modules("mounted").await?;

let mut tx = application.begin(Identity::system()).await?;
let mut components_model = BootstrapComponentsModel::new(&mut tx);
let component = components_model
.load_component(component_id)
.await?
.unwrap();
assert!(matches!(component.state, ComponentState::Active));

// Data should still exist in remounted component tables
let mut table_model = TableModel::new(&mut tx);
let count = table_model
.count(component_id.into(), &"messages".parse()?)
.await?;
assert_eq!(count, 1);

// Calling functions from the remounted component should work
run_component_function(
&application,
"messages:listMessages".parse()?,
vec![assert_obj!().into()],
ComponentPath::deserialize(Some("component"))?,
)
.await??;
Ok(())
}
23 changes: 23 additions & 0 deletions crates/common/src/bootstrap_model/components/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,17 @@ use crate::components::{
pub struct ComponentMetadata {
pub definition_id: DeveloperDocumentId,
pub component_type: ComponentType,
pub state: ComponentState,
}

#[derive(Debug, Clone, Eq, PartialEq)]
#[cfg_attr(any(test, feature = "testing"), derive(proptest_derive::Arbitrary))]
pub enum ComponentState {
/// The component is mounted and can be used.
Active,
/// The component is unmounted. Component functions are not available, and
/// tables in the component are read-only.
Unmounted,
}

impl ComponentMetadata {
Expand Down Expand Up @@ -59,6 +70,7 @@ struct SerializedComponentMetadata {
pub parent: Option<String>,
pub name: Option<String>,
pub args: Option<Vec<(String, SerializedResource)>>,
pub state: Option<String>,
}

impl TryFrom<ComponentMetadata> for SerializedComponentMetadata {
Expand All @@ -77,11 +89,16 @@ impl TryFrom<ComponentMetadata> for SerializedComponentMetadata {
),
),
};
let state = match m.state {
ComponentState::Active => "active",
ComponentState::Unmounted => "unmounted",
};
Ok(Self {
definition_id: m.definition_id.to_string(),
parent,
name,
args,
state: Some(state.to_string()),
})
}
}
Expand All @@ -102,9 +119,15 @@ impl TryFrom<SerializedComponentMetadata> for ComponentMetadata {
},
_ => anyhow::bail!("Invalid component type"),
};
let state = match m.state.as_deref() {
None | Some("active") => ComponentState::Active,
Some("unmounted") => ComponentState::Unmounted,
Some(invalid_state) => anyhow::bail!("Invalid component state: {invalid_state}"),
};
Ok(Self {
definition_id: m.definition_id.parse()?,
component_type,
state,
})
}
}
Expand Down
3 changes: 3 additions & 0 deletions crates/database/src/bootstrap_model/components/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,7 @@ mod tests {
ComponentInstantiation,
},
ComponentMetadata,
ComponentState,
ComponentType,
},
components::{
Expand Down Expand Up @@ -481,6 +482,7 @@ mod tests {
ComponentMetadata {
definition_id: root_definition_id.into(),
component_type: ComponentType::App,
state: ComponentState::Active,
}
.try_into()?,
)
Expand All @@ -495,6 +497,7 @@ mod tests {
name: "subcomponent_child".parse()?,
args: Default::default(),
},
state: ComponentState::Active,
}
.try_into()?,
)
Expand Down
2 changes: 1 addition & 1 deletion crates/isolate/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const COMPONENT_TESTS_DIR: &str = "../../npm-packages/component-tests";
const COMPONENT_TESTS_CHILD_DIR_EXCEPTIONS: [&str; 3] = [".rush", "node_modules", "projects"];
/// Directory where test projects that use components live.
const COMPONENT_TESTS_PROJECTS_DIR: &str = "../../npm-packages/component-tests/projects";
const COMPONENT_TESTS_PROJECTS: [&str; 2] = ["basic", "with-schema"];
const COMPONENT_TESTS_PROJECTS: [&str; 4] = ["basic", "with-schema", "mounted", "empty"];
/// Components in `component-tests` directory that are used in projects.
const COMPONENTS: [&str; 3] = ["component", "envVars", "errors"];

Expand Down
32 changes: 17 additions & 15 deletions crates/model/src/components/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use common::{
components::{
definition::ComponentDefinitionMetadata,
ComponentMetadata,
ComponentState,
ComponentType,
},
schema::SchemaState,
Expand Down Expand Up @@ -402,8 +403,8 @@ impl<'a, RT: Runtime> ComponentConfigModel<'a, RT> {
let mut stack = vec![(ComponentPath::root(), None, existing_root, Some(app))];
let mut diffs = BTreeMap::new();
while let Some((path, parent_and_name, existing_node, new_node)) = stack.pop() {
let new_metadata = match new_node {
Some(new_node) => {
let new_metadata = new_node
.map(|new_node| {
let definition_id = *definition_id_by_path
.get(&new_node.definition_path)
.context("Missing definition ID for component")?;
Expand All @@ -418,13 +419,13 @@ impl<'a, RT: Runtime> ComponentConfigModel<'a, RT> {
args: new_node.args.clone(),
},
};
Some(ComponentMetadata {
Ok(ComponentMetadata {
definition_id,
component_type,
state: ComponentState::Active,
})
},
None => None,
};
})
.transpose()?;

// Diff the node itself.
let (internal_id, diff) = match (existing_node, new_metadata) {
Expand Down Expand Up @@ -452,8 +453,8 @@ impl<'a, RT: Runtime> ComponentConfigModel<'a, RT> {
)
.await?
},
// Delete an existing node.
(Some(existing_node), None) => self.delete_component(existing_node).await?,
// Unmount an existing node.
(Some(existing_node), None) => self.unmount_component(existing_node).await?,
(None, None) => anyhow::bail!("Unexpected None/None in stack"),
};
diffs.insert(path.clone(), diff);
Expand Down Expand Up @@ -613,7 +614,7 @@ impl<'a, RT: Runtime> ComponentConfigModel<'a, RT> {
))
}

async fn delete_component(
async fn unmount_component(
&mut self,
existing: &ParsedDocument<ComponentMetadata>,
) -> anyhow::Result<(DeveloperDocumentId, ComponentDiff)> {
Expand All @@ -622,9 +623,10 @@ impl<'a, RT: Runtime> ComponentConfigModel<'a, RT> {
} else {
ComponentId::Child(existing.id().into())
};
// TODO: Delete the component's system tables.
let mut unmounted_metadata = existing.clone().into_value();
unmounted_metadata.state = ComponentState::Unmounted;
SystemMetadataModel::new_global(self.tx)
.delete(existing.id())
.replace(existing.id(), unmounted_metadata.try_into()?)
.await?;
let module_diff = ModuleModel::new(self.tx)
.apply(component_id, vec![], None, BTreeMap::new())
Expand All @@ -638,7 +640,7 @@ impl<'a, RT: Runtime> ComponentConfigModel<'a, RT> {
Ok((
existing.id().into(),
ComponentDiff {
diff_type: ComponentDiffType::Delete,
diff_type: ComponentDiffType::Unmount,
module_diff,
udf_config_diff: None,
cron_diff,
Expand Down Expand Up @@ -703,7 +705,7 @@ struct TreeDiffChild<'a> {
pub enum ComponentDiffType {
Create,
Modify,
Delete,
Unmount,
}

pub struct ComponentDiff {
Expand All @@ -718,7 +720,7 @@ pub struct ComponentDiff {
pub enum SerializedComponentDiffType {
Create,
Modify,
Delete,
Unmount,
}

#[derive(Serialize)]
Expand All @@ -737,7 +739,7 @@ impl TryFrom<ComponentDiffType> for SerializedComponentDiffType {
Ok(match value {
ComponentDiffType::Create => Self::Create,
ComponentDiffType::Modify => Self::Modify,
ComponentDiffType::Delete => Self::Delete,
ComponentDiffType::Unmount => Self::Unmount,
})
}
}
Expand Down
26 changes: 26 additions & 0 deletions npm-packages/common/config/rush/pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions npm-packages/component-tests/projects/empty/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@

.env.local
Loading

0 comments on commit e785a05

Please sign in to comment.