From 519f33fdec6b94629ebac87057b45662dd87dfb3 Mon Sep 17 00:00:00 2001 From: Tomasz Tomczyk Date: Fri, 10 May 2024 10:48:14 +0100 Subject: [PATCH 1/3] Ensure unsubscribing only removes a single subscription --- lib/absinthe/subscription.ex | 4 -- test/absinthe/execution/subscription_test.exs | 53 +++++++++++++++++++ 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/lib/absinthe/subscription.ex b/lib/absinthe/subscription.ex index 6eed461311..ecbe12dc72 100644 --- a/lib/absinthe/subscription.ex +++ b/lib/absinthe/subscription.ex @@ -175,10 +175,6 @@ defmodule Absinthe.Subscription do def unsubscribe(pubsub, doc_id) do registry = pubsub |> registry_name - for field_key <- pdict_fields(doc_id) do - Registry.unregister(registry, field_key) - end - Registry.unregister(registry, doc_id) pdict_delete_fields(doc_id) diff --git a/test/absinthe/execution/subscription_test.exs b/test/absinthe/execution/subscription_test.exs index 875d03cb1c..e58d3d6c5d 100644 --- a/test/absinthe/execution/subscription_test.exs +++ b/test/absinthe/execution/subscription_test.exs @@ -122,6 +122,7 @@ defmodule Absinthe.Execution.SubscriptionTest do defmodule Schema do use Absinthe.Schema + import_types Absinthe.Type.Custom query do field :foo, :string @@ -190,6 +191,19 @@ defmodule Absinthe.Execution.SubscriptionTest do end end + field :schedule, :string do + arg :location_id, non_null(:id) + arg :date, :date + + config fn + args, _ -> + { + :ok, + topic: args.location_id + } + end + end + field :multiple_topics, :string do config fn _, _ -> {:ok, topic: ["topic_1", "topic_2", "topic_3"]} @@ -400,6 +414,45 @@ defmodule Absinthe.Execution.SubscriptionTest do ) end + @query """ + subscription ($locationId: ID!, $date: Date) { + schedule(locationId: $locationId, date: $date) + } + """ + test "subscribing twice and unsubscribing once keeps one subscription active" do + location_id = "12" + date1 = "2020-01-01" + date2 = "2020-01-02" + + assert {:ok, %{"subscribed" => topic1}} = + run_subscription( + @query, + Schema, + variables: %{"locationId" => location_id, "date" => date1}, + context: %{pubsub: PubSub} + ) + + assert {:ok, %{"subscribed" => topic2}} = + run_subscription( + @query, + Schema, + variables: %{"locationId" => location_id, "date" => date2}, + context: %{pubsub: PubSub} + ) + + Absinthe.Subscription.unsubscribe(PubSub, topic1) + + Absinthe.Subscription.publish(PubSub, "foo", schedule: location_id) + + assert_receive({:broadcast, msg}) + + assert %{ + event: "subscription:data", + result: %{data: %{"schedule" => "foo"}}, + topic: topic2 + } == msg + end + @query """ subscription ($userId: ID!) { user(id: $userId) { id name } From e64a6fa93cb50fa6344474e35a6b1e59aa981c20 Mon Sep 17 00:00:00 2001 From: Tomasz Tomczyk Date: Fri, 10 May 2024 14:02:57 +0100 Subject: [PATCH 2/3] Does not receive messages after unsubscribing --- test/absinthe/execution/subscription_test.exs | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/test/absinthe/execution/subscription_test.exs b/test/absinthe/execution/subscription_test.exs index e58d3d6c5d..8fd67acc9a 100644 --- a/test/absinthe/execution/subscription_test.exs +++ b/test/absinthe/execution/subscription_test.exs @@ -453,6 +453,47 @@ defmodule Absinthe.Execution.SubscriptionTest do } == msg end + @query """ + subscription ($locationId: ID!, $date: Date) { + schedule(locationId: $locationId, date: $date) + } + """ + test "subscribing twice and unsubscribing once means new messages are only sent to one subscription" do + location_id = "12" + date1 = "2020-01-01" + date2 = "2020-01-02" + + assert {:ok, %{"subscribed" => topic1}} = + run_subscription( + @query, + Schema, + variables: %{"locationId" => location_id, "date" => date1}, + context: %{pubsub: PubSub} + ) + + assert {:ok, %{"subscribed" => topic2}} = + run_subscription( + @query, + Schema, + variables: %{"locationId" => location_id, "date" => date2}, + context: %{pubsub: PubSub} + ) + + Absinthe.Subscription.unsubscribe(PubSub, topic1) + + Absinthe.Subscription.publish(PubSub, "foo", schedule: location_id) + + assert_receive({:broadcast, msg}) + + assert %{ + event: "subscription:data", + result: %{data: %{"schedule" => "foo"}}, + topic: topic2 + } == msg + + refute_receive({:broadcast, _}) + end + @query """ subscription ($userId: ID!) { user(id: $userId) { id name } From 55039aeca3b216b8cba2b4b53f8ad4d5661e1a91 Mon Sep 17 00:00:00 2001 From: Tomasz Tomczyk Date: Fri, 10 May 2024 14:05:26 +0100 Subject: [PATCH 3/3] Registry doesn't grow indefinitely --- test/absinthe/execution/subscription_test.exs | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/test/absinthe/execution/subscription_test.exs b/test/absinthe/execution/subscription_test.exs index 8fd67acc9a..89f374ad56 100644 --- a/test/absinthe/execution/subscription_test.exs +++ b/test/absinthe/execution/subscription_test.exs @@ -77,6 +77,10 @@ defmodule Absinthe.Execution.SubscriptionTest do # this pubsub is local and doesn't support clusters :ok end + + def list_registry_keys() do + Registry.keys(__MODULE__, self()) + end end defmodule PubSubWithDocsetRunner do @@ -494,6 +498,48 @@ defmodule Absinthe.Execution.SubscriptionTest do refute_receive({:broadcast, _}) end + @query """ + subscription ($clientId: ID!) { + thing(clientId: $clientId) + } + """ + test "repeatedly subscribing and unsubscribing on the same topic doesn't grow registry indefinitely" do + client_id = "abc" + + for _i <- 1..3 do + assert {:ok, %{"subscribed" => topic}} = + run_subscription(@query, Schema, + variables: %{"clientId" => client_id}, + context: %{pubsub: PubSub} + ) + + Absinthe.Subscription.unsubscribe(PubSub, topic) + end + + assert [] == PubSub.list_registry_keys() + end + + @query """ + subscription ($clientId: ID!) { + thing(clientId: $clientId) + } + """ + test "repeatedly subscribing and unsubscribing on multiple topics doesn't grow registry indefinitely" do + client_id = "abc" + + for i <- 1..3 do + assert {:ok, %{"subscribed" => topic}} = + run_subscription(@query, Schema, + variables: %{"clientId" => "#{client_id}#{i}"}, + context: %{pubsub: PubSub} + ) + + Absinthe.Subscription.unsubscribe(PubSub, topic) + end + + assert [] == PubSub.list_registry_keys() + end + @query """ subscription ($userId: ID!) { user(id: $userId) { id name }