Revert "tests load modules from storage (#26107)" (#26111)

This reverts commit 14d2100f42243eb58eb2b81c5e6d19f12bc3c0f0.

should fix the test timeout

GitOrigin-RevId: bc6fc9287e85da2b98d244ee4619143ef5860e10

Author: ldanilek

crates/isolate/src/test_helpers.rs

@@ -85,8 +85,8 @@ use maplit::btreemap;
 use model::{
     config::{
         module_loader::{
-            test_module_loader::UncachedModuleLoader,
             ModuleLoader,
+            TransactionModuleLoader,
         },
         types::{
             ConfigMetadata,
@@ -99,10 +99,6 @@ use model::{
         FileStorageId,
     },
     scheduled_jobs::VirtualSchedulerModel,
-    source_packages::{
-        types::SourcePackage,
-        upload_download::upload_package,
-    },
     test_helpers::DbFixturesWithModel,
     udf_config::{
         types::UdfConfig,
@@ -223,8 +219,7 @@ pub static TEST_SOURCE_ISOLATE_ONLY: LazyLock<Vec<ModuleConfig>> = LazyLock::new
 
 pub fn test_environment_data<RT: Runtime>(rt: RT) -> anyhow::Result<EnvironmentData<RT>> {
     let key_broker = KeyBroker::new(DEV_INSTANCE_NAME, InstanceSecret::try_from(DEV_SECRET)?)?;
-    let modules_storage = Arc::new(LocalDirStorage::new(rt.clone())?);
-    let module_loader = Arc::new(UncachedModuleLoader { modules_storage });
+    let module_loader = Arc::new(TransactionModuleLoader);
     let storage = Arc::new(LocalDirStorage::new(rt.clone())?);
     let convex_origin = "http://127.0.0.1:8000".into();
     let file_storage = TransactionalFileStorage::new(rt.clone(), storage.clone(), convex_origin);
@@ -284,10 +279,7 @@ impl<RT: Runtime, P: Persistence + Clone> UdfTest<RT, P> {
             .into_join_future()
             .await?;
         let key_broker = KeyBroker::new(DEV_INSTANCE_NAME, InstanceSecret::try_from(DEV_SECRET)?)?;
-        let modules_storage = Arc::new(LocalDirStorage::new(rt.clone())?);
-        let module_loader = Arc::new(UncachedModuleLoader {
-            modules_storage: modules_storage.clone(),
-        });
+        let module_loader = Arc::new(TransactionModuleLoader);
         let storage = Arc::new(LocalDirStorage::new(rt.clone())?);
         let convex_origin = "http://127.0.0.1:8000".into();
         let file_storage =
@@ -319,39 +311,25 @@ impl<RT: Runtime, P: Persistence + Clone> UdfTest<RT, P> {
         );
 
         let udf_config = UdfConfig::new_for_test(&rt, config.udf_server_version);
-        let modules_by_path: BTreeMap<_, _> = modules
+        let modules_by_path = modules
             .iter()
             .map(|c| (c.path.clone().canonicalize(), c.clone()))
             .collect();
         let analyze_results = match isolate
-            .analyze(udf_config.clone(), modules_by_path.clone(), BTreeMap::new())
+            .analyze(udf_config.clone(), modules_by_path, BTreeMap::new())
             .await?
         {
             Ok(analyze_results) => analyze_results,
             Err(err) => return Ok(Err(err)),
         };
 
-        let (storage_key, sha256, package_size) = upload_package(
-            modules_by_path
-                .iter()
-                .map(|(path, m)| (path.clone(), m))
-                .collect(),
-            modules_storage,
-            None,
-        )
-        .await?;
         let mut tx = database.begin(Identity::system()).await?;
         ConfigModel::new(&mut tx)
             .apply(
                 ConfigMetadata::new(),
                 modules,
                 udf_config,
-                Some(SourcePackage {
-                    storage_key,
-                    sha256,
-                    package_size,
-                    external_deps_package_id: None,
-                }),
+                None,
                 analyze_results,
                 None,
             )

crates/model/src/config/module_loader.rs

@@ -37,63 +37,19 @@ pub trait ModuleLoader<RT: Runtime>: Sync + Send + 'static {
     }
 }
 
-#[cfg(any(test, feature = "testing"))]
-pub mod test_module_loader {
-    use std::sync::Arc;
+// Loads module versions directly from the transaction.
+pub struct TransactionModuleLoader;
 
-    use anyhow::Context;
-    use async_trait::async_trait;
-    use common::{
-        document::ParsedDocument,
-        runtime::Runtime,
-    };
-    use database::Transaction;
-    use storage::Storage;
-
-    use super::ModuleLoader;
-    use crate::{
-        modules::{
-            module_versions::FullModuleSource,
-            types::ModuleMetadata,
-        },
-        source_packages::{
-            upload_download::download_package,
-            SourcePackageModel,
-        },
-    };
-
-    // Loads module versions directly from storage.
-    pub struct UncachedModuleLoader {
-        pub modules_storage: Arc<dyn Storage>,
-    }
-
-    #[async_trait]
-    impl<RT: Runtime> ModuleLoader<RT> for UncachedModuleLoader {
-        async fn get_module_with_metadata(
-            &self,
-            tx: &mut Transaction<RT>,
-            module_metadata: ParsedDocument<ModuleMetadata>,
-        ) -> anyhow::Result<Arc<FullModuleSource>> {
-            let source_package_id = module_metadata
-                .source_package_id
-                .with_context(|| format!("Missing source package {}", module_metadata.id()))?;
-            let source_package = SourcePackageModel::new(tx)
-                .get(source_package_id)
-                .await?
-                .into_value();
-            let package = download_package(
-                self.modules_storage.clone(),
-                source_package.storage_key,
-                source_package.sha256,
-            )
+#[async_trait]
+impl<RT: Runtime> ModuleLoader<RT> for TransactionModuleLoader {
+    async fn get_module_with_metadata(
+        &self,
+        tx: &mut Transaction<RT>,
+        module_metadata: ParsedDocument<ModuleMetadata>,
+    ) -> anyhow::Result<Arc<FullModuleSource>> {
+        let full_source = ModuleModel::new(tx)
+            .get_source(module_metadata.id(), module_metadata.latest_version)
             .await?;
-            let module_config = package
-                .get(&module_metadata.path)
-                .with_context(|| format!("Missing module source {}", module_metadata.id()))?;
-            Ok(Arc::new(FullModuleSource {
-                source: module_config.source.clone(),
-                source_map: module_config.source_map.clone(),
-            }))
-        }
+        Ok(Arc::new(full_source))
     }
 }

crates/model/src/config/tests.rs

@@ -1,7 +1,4 @@
-use std::{
-    collections::BTreeMap,
-    sync::Arc,
-};
+use std::collections::BTreeMap;
 
 use common::{
     db_schema,
@@ -22,32 +19,26 @@ use database::{
 use keybroker::Identity;
 use maplit::btreemap;
 use runtime::testing::TestRuntime;
-use storage::LocalDirStorage;
 use value::heap_size::WithHeapSize;
 
 use crate::{
     auth::AuthInfoModel,
     config::{
-        module_loader::test_module_loader::UncachedModuleLoader,
+        module_loader::TransactionModuleLoader,
         types::{
             ConfigMetadata,
             ModuleConfig,
         },
         ConfigModel,
     },
     modules::module_versions::AnalyzedModule,
-    source_packages::{
-        types::SourcePackage,
-        upload_download::upload_package,
-    },
     test_helpers::DbFixturesWithModel,
     udf_config::types::UdfConfig,
 };
 
 #[convex_macro::test_runtime]
 async fn test_config(rt: TestRuntime) -> anyhow::Result<()> {
     let database = DbFixtures::new(&rt.clone()).await?.with_model().await?.db;
-    let modules_storage = Arc::new(LocalDirStorage::new(rt.clone())?);
 
     // Initialize config
     let mut tx = database.begin(Identity::system()).await?;
@@ -66,26 +57,12 @@ async fn test_config(rt: TestRuntime) -> anyhow::Result<()> {
     };
     let p1 = module1.path.clone().canonicalize();
     let p2 = module2.path.clone().canonicalize();
-    let (storage_key, sha256, package_size) = upload_package(
-        btreemap! {
-            p1.clone() => &module1,
-            p2.clone() => &module2,
-        },
-        modules_storage.clone(),
-        None,
-    )
-    .await?;
     ConfigModel::new(&mut tx)
         .apply(
             config_metadata.clone(),
             vec![module1.clone(), module2.clone()],
             UdfConfig::new_for_test(&rt, "1000.0.0".parse()?),
-            Some(SourcePackage {
-                storage_key,
-                sha256,
-                external_deps_package_id: None,
-                package_size,
-            }),
+            None, // source storage key
             btreemap! {
                 p1 => AnalyzedModule {
                     functions: WithHeapSize::default(),
@@ -108,7 +85,7 @@ async fn test_config(rt: TestRuntime) -> anyhow::Result<()> {
     // Fetch it back and it make sure it's there.
     let mut tx = database.begin(Identity::system()).await?;
     let (config_metadata_read, modules_read, ..) = ConfigModel::new(&mut tx)
-        .get(&UncachedModuleLoader { modules_storage })
+        .get(&TransactionModuleLoader)
         .await
         .expect("getting config should succeed");
     assert_eq!(config_metadata, config_metadata_read);
@@ -120,7 +97,6 @@ async fn test_config(rt: TestRuntime) -> anyhow::Result<()> {
 #[convex_macro::test_runtime]
 async fn test_config_large_modules(rt: TestRuntime) -> anyhow::Result<()> {
     let database = DbFixtures::new(&rt.clone()).await?.with_model().await?.db;
-    let modules_storage = Arc::new(LocalDirStorage::new(rt.clone())?);
 
     // Initialize config
     let mut tx = database.begin(Identity::system()).await?;
@@ -151,26 +127,12 @@ async fn test_config_large_modules(rt: TestRuntime) -> anyhow::Result<()> {
             )
         })
         .collect();
-    let (storage_key, sha256, package_size) = upload_package(
-        modules
-            .iter()
-            .map(|m| (m.path.clone().canonicalize(), m))
-            .collect(),
-        modules_storage.clone(),
-        None,
-    )
-    .await?;
     ConfigModel::new(&mut tx)
         .apply(
             config_metadata.clone(),
             modules.clone(),
             UdfConfig::new_for_test(&rt, "1000.0.0".parse()?),
-            Some(SourcePackage {
-                storage_key,
-                sha256,
-                external_deps_package_id: None,
-                package_size,
-            }),
+            None, // source storage key
             analyzed_result,
             None,
         )
@@ -179,7 +141,7 @@ async fn test_config_large_modules(rt: TestRuntime) -> anyhow::Result<()> {
 
     let mut tx = database.begin(Identity::system()).await?;
     let (config_metadata_read, modules_read, ..) = ConfigModel::new(&mut tx)
-        .get(&UncachedModuleLoader { modules_storage })
+        .get(&TransactionModuleLoader)
         .await
         .expect("getting config should succeed");
     assert_eq!(config_metadata, config_metadata_read);