From 9877a5730227ff4d5b3f8c6987fccda7aef548c3 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Fri, 21 Feb 2025 09:51:15 -0500 Subject: [PATCH] Validate only one Subscription selection --- lib/graphql/static_validation/all_rules.rb | 2 +- .../rules/not_single_subscription_error.rb | 25 ++++++++ .../rules/subscription_root_exists.rb | 17 ------ ...xists_and_single_subscription_selection.rb | 26 ++++++++ ...n_root_exists_and_single_selection_spec.rb | 61 +++++++++++++++++++ .../rules/subscription_root_exists_spec.rb | 33 ---------- spec/graphql/subscriptions_spec.rb | 33 ---------- 7 files changed, 113 insertions(+), 84 deletions(-) create mode 100644 lib/graphql/static_validation/rules/not_single_subscription_error.rb delete mode 100644 lib/graphql/static_validation/rules/subscription_root_exists.rb create mode 100644 lib/graphql/static_validation/rules/subscription_root_exists_and_single_subscription_selection.rb create mode 100644 spec/graphql/static_validation/rules/subscription_root_exists_and_single_selection_spec.rb delete mode 100644 spec/graphql/static_validation/rules/subscription_root_exists_spec.rb diff --git a/lib/graphql/static_validation/all_rules.rb b/lib/graphql/static_validation/all_rules.rb index 6c6c8b8df8..7796a4d78f 100644 --- a/lib/graphql/static_validation/all_rules.rb +++ b/lib/graphql/static_validation/all_rules.rb @@ -34,7 +34,7 @@ module StaticValidation GraphQL::StaticValidation::VariableUsagesAreAllowed, GraphQL::StaticValidation::MutationRootExists, GraphQL::StaticValidation::QueryRootExists, - GraphQL::StaticValidation::SubscriptionRootExists, + GraphQL::StaticValidation::SubscriptionRootExistsAndSingleSubscriptionSelection, GraphQL::StaticValidation::InputObjectNamesAreUnique, GraphQL::StaticValidation::OneOfInputObjectsAreValid, ] diff --git a/lib/graphql/static_validation/rules/not_single_subscription_error.rb b/lib/graphql/static_validation/rules/not_single_subscription_error.rb new file mode 100644 index 0000000000..726c0c4f41 --- /dev/null +++ b/lib/graphql/static_validation/rules/not_single_subscription_error.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true +module GraphQL + module StaticValidation + class NotSingleSubscriptionError < StaticValidation::Error + def initialize(message, path: nil, nodes: []) + super(message, path: path, nodes: nodes) + end + + # A hash representation of this Message + def to_h + extensions = { + "code" => code, + } + + super.merge({ + "extensions" => extensions + }) + end + + def code + "notSingleSubscription" + end + end + end +end diff --git a/lib/graphql/static_validation/rules/subscription_root_exists.rb b/lib/graphql/static_validation/rules/subscription_root_exists.rb deleted file mode 100644 index b3c41748f1..0000000000 --- a/lib/graphql/static_validation/rules/subscription_root_exists.rb +++ /dev/null @@ -1,17 +0,0 @@ -# frozen_string_literal: true -module GraphQL - module StaticValidation - module SubscriptionRootExists - def on_operation_definition(node, _parent) - if node.operation_type == "subscription" && context.types.subscription_root.nil? - add_error(GraphQL::StaticValidation::SubscriptionRootExistsError.new( - 'Schema is not configured for subscriptions', - nodes: node - )) - else - super - end - end - end - end -end diff --git a/lib/graphql/static_validation/rules/subscription_root_exists_and_single_subscription_selection.rb b/lib/graphql/static_validation/rules/subscription_root_exists_and_single_subscription_selection.rb new file mode 100644 index 0000000000..fdb7589432 --- /dev/null +++ b/lib/graphql/static_validation/rules/subscription_root_exists_and_single_subscription_selection.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true +module GraphQL + module StaticValidation + module SubscriptionRootExistsAndSingleSubscriptionSelection + def on_operation_definition(node, parent) + if node.operation_type == "subscription" + if context.types.subscription_root.nil? + add_error(GraphQL::StaticValidation::SubscriptionRootExistsError.new( + 'Schema is not configured for subscriptions', + nodes: node + )) + elsif node.selections.size != 1 + add_error(GraphQL::StaticValidation::NotSingleSubscriptionError.new( + 'A subscription operation may only have one selection', + nodes: node, + )) + else + super + end + else + super + end + end + end + end +end diff --git a/spec/graphql/static_validation/rules/subscription_root_exists_and_single_selection_spec.rb b/spec/graphql/static_validation/rules/subscription_root_exists_and_single_selection_spec.rb new file mode 100644 index 0000000000..bf10fd682e --- /dev/null +++ b/spec/graphql/static_validation/rules/subscription_root_exists_and_single_selection_spec.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true +require "spec_helper" + +describe GraphQL::StaticValidation::SubscriptionRootExistsAndSingleSubscriptionSelection do + include StaticValidationHelpers + + let(:query_string) {%| + subscription { + test + } + |} + + let(:schema) { + Class.new(GraphQL::Schema) do + query_root = Class.new(GraphQL::Schema::Object) do + graphql_name "Query" + end + + query query_root + end + } + + it "errors when a subscription is performed on a schema without a subscription root" do + assert_equal(1, errors.length) + missing_subscription_root_error = { + "message"=>"Schema is not configured for subscriptions", + "locations"=>[{"line"=>2, "column"=>5}], + "path"=>["subscription"], + "extensions"=>{"code"=>"missingSubscriptionConfiguration"} + } + assert_includes(errors, missing_subscription_root_error) + end + + describe "when multiple subscription selections" do + let(:query_string) { + "subscription { subscription1 subscription2 }" + } + + let(:schema) { + Class.new(GraphQL::Schema) do + subscription(Class.new(GraphQL::Schema::Object) do + graphql_name "Subscription" + field :subscription1, String + field :subscription2, String + end) + end + } + + it "returns an error" do + expected_errs = [ + { + "message" => "A subscription operation may only have one selection", + "locations" => [{"line" => 1, "column" => 1}], + "path" => ["subscription"], + "extensions" => {"code" => "notSingleSubscription"} + } + ] + assert_equal(expected_errs, errors) + end + end +end diff --git a/spec/graphql/static_validation/rules/subscription_root_exists_spec.rb b/spec/graphql/static_validation/rules/subscription_root_exists_spec.rb deleted file mode 100644 index a4e3ddc8cf..0000000000 --- a/spec/graphql/static_validation/rules/subscription_root_exists_spec.rb +++ /dev/null @@ -1,33 +0,0 @@ -# frozen_string_literal: true -require "spec_helper" - -describe GraphQL::StaticValidation::SubscriptionRootExists do - include StaticValidationHelpers - - let(:query_string) {%| - subscription { - test - } - |} - - let(:schema) { - Class.new(GraphQL::Schema) do - query_root = Class.new(GraphQL::Schema::Object) do - graphql_name "Query" - end - - query query_root - end - } - - it "errors when a subscription is performed on a schema without a subscription root" do - assert_equal(1, errors.length) - missing_subscription_root_error = { - "message"=>"Schema is not configured for subscriptions", - "locations"=>[{"line"=>2, "column"=>5}], - "path"=>["subscription"], - "extensions"=>{"code"=>"missingSubscriptionConfiguration"} - } - assert_includes(errors, missing_subscription_root_error) - end -end diff --git a/spec/graphql/subscriptions_spec.rb b/spec/graphql/subscriptions_spec.rb index c00adb6602..580b64c731 100644 --- a/spec/graphql/subscriptions_spec.rb +++ b/spec/graphql/subscriptions_spec.rb @@ -326,7 +326,6 @@ def to_param query_str = <<-GRAPHQL subscription ($id: ID!){ firstPayload: payload(id: $id) { str, int } - otherPayload: payload(id: "900") { int } } GRAPHQL @@ -416,38 +415,6 @@ def to_param end end - it "sends updated data for multifield subscriptions" do - query_str = <<-GRAPHQL - subscription ($id: ID!){ - payload(id: $id) { str, int } - event { int } - } - GRAPHQL - - # Initial subscriptions - res = schema.execute(query_str, context: { socket: "1" }, variables: { "id" => "100" }, root_value: root_object) - empty_response = {} - - # Initial response is nil, no broadcasts yet - assert_equal(empty_response, res["data"]) - assert_equal [], deliveries["1"] - - # Application stuff happens. - # The application signals graphql via `subscriptions.trigger`: - schema.subscriptions.trigger(:payload, {"id" => "100"}, root_object.payload) - - # Let's see what GraphQL sent over the wire: - assert_equal({"str" => "Update", "int" => 1}, deliveries["1"][0]["data"]["payload"]) - assert_nil(deliveries["1"][0]["data"]["event"]) - - # Trigger another field subscription - schema.subscriptions.trigger(:event, {}, OpenStruct.new(int: 1)) - - # Now we should get result for another field - assert_nil(deliveries["1"][1]["data"]["payload"]) - assert_equal({"int" => 1}, deliveries["1"][1]["data"]["event"]) - end - describe "passing a document into #execute" do it "sends the updated data" do query_str = <<-GRAPHQL