fix: use variance merge for ConfigModule (#2957)

src/core/config/config_module.rs

@@ -17,7 +17,7 @@ mod merge;
 
 /// A wrapper on top of Config that contains all the resolved extensions and
 /// computed values.
-#[derive(Clone, Debug, Default, MergeRight)]
+#[derive(Clone, Debug, Default)]
 pub struct ConfigModule {
     extensions: Extensions,
     cache: Cache,
@@ -48,12 +48,6 @@ impl From<Config> for Cache {
     }
 }
 
-impl MergeRight for Cache {
-    fn merge_right(self, other: Self) -> Self {
-        Cache::from(self.config.merge_right(other.config))
-    }
-}
-
 impl ConfigModule {
     pub fn new(config: Config, extensions: Extensions) -> Self {
         ConfigModule { cache: Cache::from(config), extensions }

src/core/config/config_module/merge.rs

@@ -270,7 +270,8 @@ impl Invariant for Cache {
             types.extend(merged_types);
             enums.extend(merged_enums);
 
-            let config = Config { types, enums, unions: self.config.unions.merge_right(other.config.unions), ..self.config };
+            let config = Config {
+                types, enums, unions: self.config.unions.merge_right(other.config.unions), server: self.config.server.merge_right(other.config.server), upstream: self.config.upstream.merge_right(other.config.upstream), schema: self.config.schema.merge_right(other.config.schema), links: self.config.links.merge_right(other.config.links), telemetry: self.config.telemetry.merge_right(other.config.telemetry)  };
 
             Cache {
                 config,
@@ -284,9 +285,10 @@ impl Invariant for Cache {
 
 impl Invariant for ConfigModule {
     fn unify(self, other: Self) -> Valid<Self, String> {
-        self.cache
-            .unify(other.cache)
-            .map(|cache| Self { cache, extensions: self.extensions })
+        self.cache.unify(other.cache).map(|cache| Self {
+            cache,
+            extensions: self.extensions.merge_right(other.extensions),
+        })
     }
 }
 

src/core/config/config_module/snapshots/tailcall__core__config__config_module__merge__tests__federation_router.snap

@@ -2,7 +2,7 @@
 source: src/core/config/config_module/merge.rs
 expression: merged.to_sdl()
 ---
-schema @server @upstream {
+schema @server(port: 8000) @upstream(baseURL: "http://jsonplaceholder.typicode.com", batch: {delay: 100, headers: []}, httpCache: 42) {
   query: Query
 }
 

src/core/config/reader.rs

@@ -8,11 +8,12 @@ use url::Url;
 
 use super::{ConfigModule, Content, Link, LinkType, PrivateKey};
 use crate::core::config::{Config, ConfigReaderContext, Source};
-use crate::core::merge_right::MergeRight;
 use crate::core::proto_reader::ProtoReader;
 use crate::core::resource_reader::{Cached, Resource, ResourceReader};
 use crate::core::rest::EndpointSet;
 use crate::core::runtime::TargetRuntime;
+use crate::core::valid::{Valid, Validator};
+use crate::core::variance::Invariant;
 
 /// Reads the configuration from a file or from an HTTP URL and resolves all
 /// linked extensions to create a ConfigModule.
@@ -36,7 +37,7 @@ impl ConfigReader {
     #[async_recursion::async_recursion]
     async fn ext_links(
         &self,
-        mut config_module: ConfigModule,
+        config_module: ConfigModule,
         parent_dir: Option<&'async_recursion Path>,
     ) -> anyhow::Result<ConfigModule> {
         let links: Vec<Link> = config_module
@@ -57,6 +58,8 @@ impl ConfigReader {
         }
 
         let mut extensions = config_module.extensions().clone();
+        let mut config_module = Valid::succeed(config_module);
+
         // let mut base_config = config_module.config().clone();
 
         for link in links.iter() {
@@ -68,13 +71,16 @@ impl ConfigReader {
                     let content = source.content;
 
                     let config = Config::from_source(Source::detect(&source.path)?, &content)?;
-                    config_module = config_module.merge_right(config.clone().into());
+                    config_module = config_module.and_then(|config_module| {
+                        config_module.unify(ConfigModule::from(config.clone()))
+                    });
 
                     if !config.links.is_empty() {
                         let cfg_module = self
                             .ext_links(ConfigModule::from(config), Path::new(&link.src).parent())
                             .await?;
-                        config_module = config_module.merge_right(cfg_module.clone());
+                        config_module =
+                            config_module.and_then(|config_module| config_module.unify(cfg_module));
                     }
                 }
                 LinkType::Protobuf => {
@@ -134,9 +140,9 @@ impl ConfigReader {
             }
         }
 
-        // Recreating the ConfigModule in order to recompute the values of
-        // `input_types`, `output_types` and `interface_types`
-        Ok(config_module.set_extensions(extensions))
+        Ok(config_module
+            .map(|config_module| config_module.set_extensions(extensions))
+            .to_result()?)
     }
 
     /// Reads the certificate from a given file
@@ -182,7 +188,7 @@ impl ConfigReader {
         files: &[T],
     ) -> anyhow::Result<ConfigModule> {
         let files = self.resource_reader.read_files(files).await?;
-        let mut config_module = ConfigModule::default();
+        let mut config_module = Valid::succeed(ConfigModule::default());
 
         for file in files.iter() {
             let source = Source::detect(&file.path)?;
@@ -197,10 +203,11 @@ impl ConfigReader {
                 .await?;
 
             // Merge it with the original config set
-            config_module = config_module.merge_right(new_config_module);
+            config_module =
+                config_module.and_then(|config_module| config_module.unify(new_config_module));
         }
 
-        Ok(config_module)
+        Ok(config_module.to_result()?)
     }
 
     /// Resolves all the links in a Config to create a ConfigModule

src/core/mod.rs

@@ -40,7 +40,7 @@ pub mod tracing;
 mod transform;
 pub mod try_fold;
 pub mod valid;
-mod variance;
+pub mod variance;
 pub mod worker;
 mod wrapping_type;
 

tailcall-wasm/src/builder.rs

@@ -7,6 +7,8 @@ use tailcall::core::config::ConfigModule;
 use tailcall::core::merge_right::MergeRight;
 use tailcall::core::rest::EndpointSet;
 use tailcall::core::runtime::TargetRuntime;
+use tailcall::core::valid::Validator;
+use tailcall::core::variance::Invariant;
 use wasm_bindgen::prelude::wasm_bindgen;
 use wasm_bindgen::JsValue;
 
@@ -40,7 +42,10 @@ impl TailcallBuilder {
 
     async fn with_config_inner(mut self, url: String) -> anyhow::Result<TailcallBuilder> {
         if url::Url::parse(&url).is_ok() {
-            self.module = self.module.merge_right(self.reader.read(url).await?);
+            self.module = self
+                .module
+                .unify(self.reader.read(url).await?)
+                .to_result()?;
         } else {
             return Err(anyhow::anyhow!("Config can only be loaded over URL"));
         }

tests/core/snapshots/test-merge-input.md_merged.snap

@@ -0,0 +1,15 @@
+---
+source: tests/core/spec.rs
+expression: formatter
+---
+schema @server @upstream(baseURL: "http://jsonplacheholder.typicode.com") {
+  query: Query
+}
+
+input Test {
+  b: String
+}
+
+type Query {
+  foo(x: Test): Boolean @http(path: "/foo")
+}

tests/core/snapshots/test-merge-invalid.md_error.snap

@@ -0,0 +1,14 @@
+---
+source: tests/core/spec.rs
+expression: errors
+---
+[
+  {
+    "message": "Type mismatch: expected `String`, got `Int`",
+    "trace": [
+      "Foo",
+      "a"
+    ],
+    "description": null
+  }
+]