From 16a3c8d7c51cbb0fbbb04f8fdda8ce86cd95863c Mon Sep 17 00:00:00 2001
From: Coenen Benjamin <benjamin.coenen@hotmail.com>
Date: Thu, 7 Sep 2023 11:00:55 +0200
Subject: [PATCH] fix(subscription): force the deduplication to be enabled by
 default as it's documented (#3773)

`subscription.enable_deduplication` was documented to be `true` by
default but it wasn't really the case.

---------

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
Co-authored-by: Jeremy Lempereur <jeremy.lempereur@iomentum.com>
---
 ...njjj_subscription_default_configuration.md |  8 +++++
 ...nfiguration__tests__schema_generation.snap |  6 ++--
 apollo-router/src/plugins/subscription.rs     | 29 +++++++++++++++----
 3 files changed, 33 insertions(+), 10 deletions(-)
 create mode 100644 .changesets/fix_bnjjj_subscription_default_configuration.md

diff --git a/.changesets/fix_bnjjj_subscription_default_configuration.md b/.changesets/fix_bnjjj_subscription_default_configuration.md
new file mode 100644
index 0000000000..2c4a5c207a
--- /dev/null
+++ b/.changesets/fix_bnjjj_subscription_default_configuration.md
@@ -0,0 +1,8 @@
+### fix(subscription): force the deduplication to be enabled by default as it's documented ([PR #3773](https://github.com/apollographql/router/pull/3773))
+
+A bug was introduced in router v1.25.0 which caused [subscription deduplication](https://www.apollographql.com/docs/router/executing-operations/subscription-support#subscription-deduplication) to be disabled by default.
+As documented, the router will enable deduplication by default, providing you with subscriptions that scale.
+
+Should you decide to disable it, you can still explicitly set `enable_deduplication` to `false`.
+
+By [@bnjjj](https://github.com/bnjjj) in https://github.com/apollographql/router/pull/3773
diff --git a/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap
index ff9123ab31..14ab198d54 100644
--- a/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap
+++ b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap
@@ -1641,17 +1641,15 @@ expression: "&schema"
     "subscription": {
       "description": "Subscriptions configuration",
       "type": "object",
-      "required": [
-        "enabled"
-      ],
       "properties": {
         "enable_deduplication": {
           "description": "Enable the deduplication of subscription (for example if we detect the exact same request to subgraph we won't open a new websocket to the subgraph in passthrough mode) (default: true)",
-          "default": false,
+          "default": true,
           "type": "boolean"
         },
         "enabled": {
           "description": "Enable subscription",
+          "default": true,
           "type": "boolean"
         },
         "max_opened_subscriptions": {
diff --git a/apollo-router/src/plugins/subscription.rs b/apollo-router/src/plugins/subscription.rs
index 850f584b60..afd7e5a6d1 100644
--- a/apollo-router/src/plugins/subscription.rs
+++ b/apollo-router/src/plugins/subscription.rs
@@ -56,31 +56,32 @@ pub(crate) struct Subscription {
 
 /// Subscriptions configuration
 #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema)]
-#[serde(deny_unknown_fields)]
+#[serde(deny_unknown_fields, default)]
 pub(crate) struct SubscriptionConfig {
     /// Enable subscription
     pub(crate) enabled: bool,
     /// Select a subscription mode (callback or passthrough)
-    #[serde(default)]
     pub(crate) mode: SubscriptionModeConfig,
     /// Enable the deduplication of subscription (for example if we detect the exact same request to subgraph we won't open a new websocket to the subgraph in passthrough mode)
     /// (default: true)
-    #[serde(default)]
+    #[serde(default = "enable_deduplication_default")]
     pub(crate) enable_deduplication: bool,
     /// This is a limit to only have maximum X opened subscriptions at the same time. By default if it's not set there is no limit.
-    #[serde(default)]
     pub(crate) max_opened_subscriptions: Option<usize>,
     /// It represent the capacity of the in memory queue to know how many events we can keep in a buffer
-    #[serde(default)]
     pub(crate) queue_capacity: Option<usize>,
 }
 
+fn enable_deduplication_default() -> bool {
+    true
+}
+
 impl Default for SubscriptionConfig {
     fn default() -> Self {
         Self {
             enabled: true,
             mode: Default::default(),
-            enable_deduplication: true,
+            enable_deduplication: enable_deduplication_default(),
             max_opened_subscriptions: None,
             queue_capacity: None,
         }
@@ -1268,6 +1269,22 @@ mod tests {
 
         let subgraph_cfg = config_without_mode.mode.get_subgraph_config("test");
         assert_eq!(subgraph_cfg, None);
+
+        let sub_config: SubscriptionConfig = serde_json::from_value(serde_json::json!({
+            "mode": {
+                "preview_callback": {
+                    "public_url": "http://localhost:4000",
+                    "path": "/subscription/callback",
+                    "subgraphs": ["test"]
+                }
+            }
+        }))
+        .unwrap();
+
+        assert!(sub_config.enabled);
+        assert!(sub_config.enable_deduplication);
+        assert!(sub_config.max_opened_subscriptions.is_none());
+        assert!(sub_config.queue_capacity.is_none());
     }
 }