Fix delete_component to work with hidden tables (#35246)

Adds a test that makes sure it deletes hidden tables. confirmed that
the test previously failed, but now passes.

GitOrigin-RevId: 85707863a1c27b6523c669d14f86d2abf5a4598d

Author: nipunn1313

crates/application/src/exports/tests.rs

@@ -567,7 +567,7 @@ async fn test_export_with_table_delete(rt: TestRuntime) -> anyhow::Result<()> {
     db.commit(tx).await?;
     let mut tx = db.begin(Identity::system()).await?;
     TableModel::new(&mut tx)
-        .delete_table(TableNamespace::test_user(), "table_0".parse()?)
+        .delete_active_table(TableNamespace::test_user(), "table_0".parse()?)
         .await?;
     db.commit(tx).await?;
 

crates/application/src/lib.rs

@@ -2518,7 +2518,7 @@ impl<RT: Runtime> Application<RT> {
             let mut table_model = TableModel::new(&mut tx);
             count += table_model.must_count(table_namespace, &table_name).await?;
             table_model
-                .delete_table(table_namespace, table_name)
+                .delete_active_table(table_namespace, table_name)
                 .await?;
         }
         self.commit(tx, "delete_tables").await?;

crates/application/src/snapshot_import/mod.rs

@@ -895,7 +895,9 @@ async fn finalize_import<RT: Runtime>(
                             .count(namespace, &table_name)
                             .await?
                             .unwrap_or(0);
-                        table_model.delete_table(namespace, table_name).await?;
+                        table_model
+                            .delete_active_table(namespace, table_name)
+                            .await?;
                     }
                     schema_constraints.validate(tx).await?;
                     let mut table_model = TableModel::new(tx);

crates/application/src/system_table_cleanup/mod.rs

@@ -237,7 +237,7 @@ impl<RT: Runtime> SystemTableCleanupWorker<RT> {
                          {component_id:?}"
                     );
                     table_model
-                        .delete_table(*namespace, table_name.clone())
+                        .delete_active_table(*namespace, table_name.clone())
                         .await?;
                 }
             }

crates/application/src/tests/components.rs

@@ -1,5 +1,10 @@
+use std::collections::BTreeSet;
+
 use common::{
-    bootstrap_model::components::ComponentState,
+    bootstrap_model::{
+        components::ComponentState,
+        tables::TableState,
+    },
     components::{
         CanonicalizedComponentFunctionPath,
         ComponentId,
@@ -489,3 +494,51 @@ async fn test_infinite_loop_in_component(rt: TestRuntime) -> anyhow::Result<()>
     assert_contains(&err.error, "Cross component call depth limit exceeded");
     Ok(())
 }
+
+#[convex_macro::test_runtime]
+async fn test_delete_component_with_hidden_tables(rt: TestRuntime) -> anyhow::Result<()> {
+    let application = Application::new_for_tests(&rt).await?;
+    application.load_component_tests_modules("mounted").await?;
+
+    // insert table for import
+    let mut tx = application.begin(Identity::system()).await?;
+    let (_, component_id) =
+        BootstrapComponentsModel::new(&mut tx).must_component_path_to_ids(&component_path())?;
+    let mut table_model = TableModel::new(&mut tx);
+    let hidden_table_name = "hiddentable".parse()?;
+    let tablet_id_and_table_number = table_model
+        .insert_table_for_import(
+            TableNamespace::from(component_id),
+            &hidden_table_name,
+            None,
+            &BTreeSet::new(),
+        )
+        .await?;
+    application.commit_test(tx).await?;
+
+    // Ensure the hidden table exists
+    let mut tx = application.begin(Identity::system()).await?;
+    let mut table_model = TableModel::new(&mut tx);
+    let table_metadata = table_model
+        .get_table_metadata(tablet_id_and_table_number.tablet_id)
+        .await?
+        .into_value();
+    assert_eq!(table_metadata.namespace, TableNamespace::from(component_id));
+    assert_eq!(table_metadata.state, TableState::Hidden);
+
+    // Delete the component
+    let component_id = unmount_component(&application).await?;
+    application
+        .delete_component(&Identity::system(), component_id)
+        .await?;
+
+    // Ensure the hidden table is also deleted
+    let mut tx = application.begin(Identity::system()).await?;
+    let mut table_model = TableModel::new(&mut tx);
+    let table_metadata = table_model
+        .get_table_metadata(tablet_id_and_table_number.tablet_id)
+        .await?
+        .into_value();
+    assert_eq!(table_metadata.state, TableState::Deleting);
+    Ok(())
+}

crates/database/src/bootstrap_model/table.rs

@@ -194,7 +194,7 @@ impl<'a, RT: Runtime> TableModel<'a, RT> {
         self.tx.table_mapping().iter_active_user_tables().count()
     }
 
-    pub async fn delete_table(
+    pub async fn delete_active_table(
         &mut self,
         namespace: TableNamespace,
         table_name: TableName,
@@ -223,6 +223,11 @@ impl<'a, RT: Runtime> TableModel<'a, RT> {
             .await
     }
 
+    pub async fn delete_table(&mut self, tablet_id: TabletId) -> anyhow::Result<()> {
+        self.delete_table_by_id_bypassing_schema_enforcement(tablet_id)
+            .await
+    }
+
     async fn delete_table_by_id_bypassing_schema_enforcement(
         &mut self,
         tablet_id: TabletId,
@@ -384,7 +389,7 @@ impl<'a, RT: Runtime> TableModel<'a, RT> {
             anyhow::bail!(ErrorMetadata::bad_request(
                 "TableConflict",
                 format!(
-                    "New table {table} has IDs that conflict with existing internal table. \
+                    "New table `{table}` has IDs that conflict with existing internal table. \
                      Consider importing this table without `_id` fields or import into a new \
                      deployment."
                 )
@@ -610,7 +615,7 @@ mod tests {
         let mut tx = new_tx(rt).await?;
         let mut model = TableModel::new(&mut tx);
         model
-            .delete_table(TableNamespace::test_user(), TableName::from_str("missing")?)
+            .delete_active_table(TableNamespace::test_user(), TableName::from_str("missing")?)
             .await
     }
 
@@ -635,7 +640,7 @@ mod tests {
             .insert_table_metadata(TableNamespace::test_user(), &table_name)
             .await?;
         model
-            .delete_table(TableNamespace::test_user(), table_name.clone())
+            .delete_active_table(TableNamespace::test_user(), table_name.clone())
             .await?;
         assert!(!model.table_exists(TableNamespace::test_user(), &table_name));
         Ok(())
@@ -655,7 +660,7 @@ mod tests {
             .insert_table_metadata(TableNamespace::test_user(), &table_name)
             .await?;
         model
-            .delete_table(TableNamespace::test_user(), table_name.clone())
+            .delete_active_table(TableNamespace::test_user(), table_name.clone())
             .await?;
         assert!(!model.table_exists(TableNamespace::test_user(), &table_name));
         Ok(())
@@ -675,7 +680,7 @@ mod tests {
             .insert_table_metadata(TableNamespace::test_user(), &table_name)
             .await?;
         model
-            .delete_table(TableNamespace::test_user(), table_name.clone())
+            .delete_active_table(TableNamespace::test_user(), table_name.clone())
             .await?;
         assert!(!model.table_exists(TableNamespace::test_user(), &table_name));
         Ok(())
@@ -693,7 +698,7 @@ mod tests {
             .insert_table_metadata(TableNamespace::test_user(), &table_name)
             .await?;
         let result = model
-            .delete_table(TableNamespace::test_user(), table_name.clone())
+            .delete_active_table(TableNamespace::test_user(), table_name.clone())
             .await;
         let error = result.unwrap_err();
         assert!(
@@ -723,7 +728,7 @@ mod tests {
             .insert_table_metadata(TableNamespace::test_user(), &table_name)
             .await?;
         let result = model
-            .delete_table(TableNamespace::test_user(), table_name.clone())
+            .delete_active_table(TableNamespace::test_user(), table_name.clone())
             .await;
         let error = result.unwrap_err();
         assert!(
@@ -754,7 +759,7 @@ mod tests {
             .insert_table_metadata(TableNamespace::test_user(), &table_name)
             .await?;
         model
-            .delete_table(TableNamespace::test_user(), table_name.clone())
+            .delete_active_table(TableNamespace::test_user(), table_name.clone())
             .await?;
 
         let mut schema_model = SchemaModel::new_root_for_test(&mut tx);

crates/database/src/tests/mod.rs

@@ -1569,7 +1569,7 @@ async fn test_interrupted_import_then_delete_table(rt: TestRuntime) -> anyhow::R
         .is_none());
     // Delete the active table.
     TableModel::new(&mut tx)
-        .delete_table(TableNamespace::test_user(), table_name.clone())
+        .delete_active_table(TableNamespace::test_user(), table_name.clone())
         .await?;
     database.commit(tx).await?;
 

crates/database/src/tests/streaming_export_tests.rs

@@ -202,7 +202,7 @@ async fn document_deltas_should_ignore_rows_from_deleted_tables(
     let mut tx = db.begin(Identity::system()).await?;
     let mut model = TableModel::new(&mut tx);
     model
-        .delete_table(TableNamespace::test_user(), "table".parse()?)
+        .delete_active_table(TableNamespace::test_user(), "table".parse()?)
         .await?;
     db.commit(tx).await?;
 
@@ -241,7 +241,7 @@ async fn document_deltas_should_not_ignore_rows_from_tables_that_were_not_delete
     let mut tx = db.begin(Identity::system()).await?;
     let mut model = TableModel::new(&mut tx);
     model
-        .delete_table(TableNamespace::test_user(), "table2".parse()?)
+        .delete_active_table(TableNamespace::test_user(), "table2".parse()?)
         .await?;
     let table_mapping = tx.table_mapping().clone();
     let ts_latest = db.commit(tx).await?;

crates/model/src/components/config.rs

@@ -724,15 +724,13 @@ impl<'a, RT: Runtime> ComponentConfigModel<'a, RT> {
             // tables defined in the schema cannot be deleted, so we delete the _schemas
             // table first to remove that restriction
             TableModel::new(self.tx)
-                .delete_table(namespace, SCHEMAS_TABLE.clone())
+                .delete_active_table(namespace, SCHEMAS_TABLE.clone())
                 .await?;
 
-            // then delete all tables, including system tables
+            // then delete all tables, including system tables and hidden tables
             let namespaced_table_mapping = self.tx.table_mapping().namespace(namespace);
-            for (_, _, table_name) in namespaced_table_mapping.iter() {
-                TableModel::new(self.tx)
-                    .delete_table(namespace, table_name.clone())
-                    .await?;
+            for (tablet_id, ..) in namespaced_table_mapping.iter() {
+                TableModel::new(self.tx).delete_table(tablet_id).await?;
             }
         }
 

crates/model/src/migrations.rs

@@ -282,7 +282,7 @@ impl<RT: Runtime> MigrationWorker<RT> {
                     .expect("Invalid built-in virtual_tables table");
                 let mut tx = self.db.begin_system().await?;
                 TableModel::new(&mut tx)
-                    .delete_table(TableNamespace::Global, virtual_tables_table)
+                    .delete_active_table(TableNamespace::Global, virtual_tables_table)
                     .await?;
                 self.db
                     .commit_with_write_source(tx, "migration_111")