diff --git a/.github/workflows/rubocop.yml b/.github/workflows/rubocop.yml new file mode 100644 index 0000000..16132dd --- /dev/null +++ b/.github/workflows/rubocop.yml @@ -0,0 +1,20 @@ +name: Rubocop + +on: [push, pull_request] + +jobs: + lint: + name: "Rubocop" + runs-on: ubuntu-latest + strategy: + fail-fast: false + steps: + - uses: actions/checkout@v3 + - uses: ruby/setup-ruby@v1 + with: + ruby-version: '3.2' + bundler-cache: true + - name: Bundle Update + run: bundle update + - name: Run Rubocop + run: bundle exec rubocop diff --git a/.rubocop.yml b/.rubocop.yml index 8b62c6d..f5275e9 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,18 +1,27 @@ ---- require: - rubocop-rspec AllCops: - TargetRubyVersion: 2.3 + TargetRubyVersion: 2.7 + +Style/Documentation: + Enabled: false + +RSpec/MultipleMemoizedHelpers: + Max: 8 + +RSpec/ExampleLength: + Max: 35 + +RSpec/DescribeClass: + Exclude: + - 'spec/integrations/**' Metrics/BlockLength: Exclude: - "Gemfile" - "spec/**/*" -Style/BracesAroundHashParameters: - EnforcedStyle: context_dependent - Style/StringLiterals: EnforcedStyle: double_quotes @@ -44,3 +53,250 @@ Style/TrailingCommaInHashLiteral: Enabled: true EnforcedStyleForMultiline: consistent_comma +Layout/ExtraSpacing: + Enabled: false + +Layout/SpaceAroundOperators: + Enabled: false + + +Naming/FileName: + Exclude: + - 'lib/graphql-anycable.rb' + +RSpec/FilePath: + Exclude: + - 'spec/graphql/anycable_spec.rb' + +Gemspec/DeprecatedAttributeAssignment: # new in 1.30 + Enabled: true +Gemspec/DevelopmentDependencies: # new in 1.44 + Enabled: true +Gemspec/RequireMFA: # new in 1.23 + Enabled: false +Layout/LineContinuationLeadingSpace: # new in 1.31 + Enabled: true +Layout/LineContinuationSpacing: # new in 1.31 + Enabled: true +Layout/LineEndStringConcatenationIndentation: # new in 1.18 + Enabled: true +Layout/SpaceBeforeBrackets: # new in 1.7 + Enabled: true +Lint/AmbiguousAssignment: # new in 1.7 + Enabled: true +Lint/AmbiguousOperatorPrecedence: # new in 1.21 + Enabled: true +Lint/AmbiguousRange: # new in 1.19 + Enabled: true +Lint/ConstantOverwrittenInRescue: # new in 1.31 + Enabled: true +Lint/DeprecatedConstants: # new in 1.8 + Enabled: true +Lint/DuplicateBranch: # new in 1.3 + Enabled: true +Lint/DuplicateMagicComment: # new in 1.37 + Enabled: true +Lint/DuplicateMatchPattern: # new in 1.50 + Enabled: true +Lint/DuplicateRegexpCharacterClassElement: # new in 1.1 + Enabled: true +Lint/EmptyBlock: # new in 1.1 + Enabled: true +Lint/EmptyClass: # new in 1.3 + Enabled: true +Lint/EmptyInPattern: # new in 1.16 + Enabled: true +Lint/IncompatibleIoSelectWithFiberScheduler: # new in 1.21 + Enabled: true +Lint/LambdaWithoutLiteralBlock: # new in 1.8 + Enabled: true +Lint/NoReturnInBeginEndBlocks: # new in 1.2 + Enabled: true +Lint/NonAtomicFileOperation: # new in 1.31 + Enabled: true +Lint/NumberedParameterAssignment: # new in 1.9 + Enabled: true +Lint/OrAssignmentToConstant: # new in 1.9 + Enabled: true +Lint/RedundantDirGlobSort: # new in 1.8 + Enabled: true +Lint/RefinementImportMethods: # new in 1.27 + Enabled: true +Lint/RequireRangeParentheses: # new in 1.32 + Enabled: true +Lint/RequireRelativeSelfPath: # new in 1.22 + Enabled: true +Lint/SymbolConversion: # new in 1.9 + Enabled: true +Lint/ToEnumArguments: # new in 1.1 + Enabled: true +Lint/TripleQuotes: # new in 1.9 + Enabled: true +Lint/UnexpectedBlockArity: # new in 1.5 + Enabled: true +Lint/UnmodifiedReduceAccumulator: # new in 1.1 + Enabled: true +Lint/UselessRescue: # new in 1.43 + Enabled: true +Lint/UselessRuby2Keywords: # new in 1.23 + Enabled: true +Metrics/CollectionLiteralLength: # new in 1.47 + Enabled: true +Naming/BlockForwarding: # new in 1.24 + Enabled: true +Security/CompoundHash: # new in 1.28 + Enabled: true +Security/IoMethods: # new in 1.22 + Enabled: true +Style/ArgumentsForwarding: # new in 1.1 + Enabled: true +Style/ArrayIntersect: # new in 1.40 + Enabled: true +Style/CollectionCompact: # new in 1.2 + Enabled: true +Style/ComparableClamp: # new in 1.44 + Enabled: true +Style/ConcatArrayLiterals: # new in 1.41 + Enabled: true +Style/DataInheritance: # new in 1.49 + Enabled: true +Style/DirEmpty: # new in 1.48 + Enabled: true +Style/DocumentDynamicEvalDefinition: # new in 1.1 + Enabled: true +Style/EmptyHeredoc: # new in 1.32 + Enabled: true +Style/EndlessMethod: # new in 1.8 + Enabled: true +Style/EnvHome: # new in 1.29 + Enabled: true +Style/FetchEnvVar: # new in 1.28 + Enabled: true +Style/FileEmpty: # new in 1.48 + Enabled: true +Style/FileRead: # new in 1.24 + Enabled: true +Style/FileWrite: # new in 1.24 + Enabled: true +Style/HashConversion: # new in 1.10 + Enabled: true +Style/HashExcept: # new in 1.7 + Enabled: true +Style/IfWithBooleanLiteralBranches: # new in 1.9 + Enabled: true +Style/InPatternThen: # new in 1.16 + Enabled: true +Style/MagicCommentFormat: # new in 1.35 + Enabled: true +Style/MapCompactWithConditionalBlock: # new in 1.30 + Enabled: true +Style/MapToHash: # new in 1.24 + Enabled: true +Style/MapToSet: # new in 1.42 + Enabled: true +Style/MinMaxComparison: # new in 1.42 + Enabled: true +Style/MultilineInPatternThen: # new in 1.16 + Enabled: true +Style/NegatedIfElseCondition: # new in 1.2 + Enabled: true +Style/NestedFileDirname: # new in 1.26 + Enabled: true +Style/NilLambda: # new in 1.3 + Enabled: true +Style/NumberedParameters: # new in 1.22 + Enabled: true +Style/NumberedParametersLimit: # new in 1.22 + Enabled: true +Style/ObjectThen: # new in 1.28 + Enabled: true +Style/OpenStructUse: # new in 1.23 + Enabled: true +Style/OperatorMethodCall: # new in 1.37 + Enabled: true +Style/QuotedSymbols: # new in 1.16 + Enabled: true +Style/RedundantArgument: # new in 1.4 + Enabled: true +Style/RedundantConstantBase: # new in 1.40 + Enabled: true +Style/RedundantDoubleSplatHashBraces: # new in 1.41 + Enabled: true +Style/RedundantEach: # new in 1.38 + Enabled: true +Style/RedundantHeredocDelimiterQuotes: # new in 1.45 + Enabled: true +Style/RedundantInitialize: # new in 1.27 + Enabled: true +Style/RedundantLineContinuation: # new in 1.49 + Enabled: true +Style/RedundantSelfAssignmentBranch: # new in 1.19 + Enabled: true +Style/RedundantStringEscape: # new in 1.37 + Enabled: true +Style/SelectByRegexp: # new in 1.22 + Enabled: true +Style/StringChars: # new in 1.12 + Enabled: true +Style/SwapValues: # new in 1.1 + Enabled: true +Capybara/MatchStyle: # new in 2.17 + Enabled: true +Capybara/NegationMatcher: # new in 2.14 + Enabled: true +Capybara/SpecificActions: # new in 2.14 + Enabled: true +Capybara/SpecificFinders: # new in 2.13 + Enabled: true +Capybara/SpecificMatcher: # new in 2.12 + Enabled: true +RSpec/BeEmpty: # new in 2.20 + Enabled: true +RSpec/BeEq: # new in 2.9.0 + Enabled: true +RSpec/BeNil: # new in 2.9.0 + Enabled: true +RSpec/ChangeByZero: # new in 2.11 + Enabled: true +RSpec/ContainExactly: # new in 2.19 + Enabled: true +RSpec/DuplicatedMetadata: # new in 2.16 + Enabled: true +RSpec/ExcessiveDocstringSpacing: # new in 2.5 + Enabled: true +RSpec/IdenticalEqualityAssertion: # new in 2.4 + Enabled: true +RSpec/IndexedLet: # new in 2.20 + Enabled: true +RSpec/MatchArray: # new in 2.19 + Enabled: true +RSpec/NoExpectationExample: # new in 2.13 + Enabled: true +RSpec/PendingWithoutReason: # new in 2.16 + Enabled: true +RSpec/RedundantAround: # new in 2.19 + Enabled: true +RSpec/SkipBlockInsideExample: # new in 2.19 + Enabled: true +RSpec/SortMetadata: # new in 2.14 + Enabled: true +RSpec/SubjectDeclaration: # new in 2.5 + Enabled: true +RSpec/VerifiedDoubleReference: # new in 2.10.0 + Enabled: true +FactoryBot/ConsistentParenthesesStyle: # new in 2.14 + Enabled: true +FactoryBot/FactoryNameStyle: # new in 2.16 + Enabled: true +FactoryBot/SyntaxMethods: # new in 2.7 + Enabled: true +RSpec/Rails/AvoidSetupHook: # new in 2.4 + Enabled: true +RSpec/Rails/HaveHttpStatus: # new in 2.12 + Enabled: true +RSpec/Rails/InferredSpecType: # new in 2.14 + Enabled: true +RSpec/Rails/MinitestAssertions: # new in 2.17 + Enabled: true +RSpec/Rails/TravelAround: # new in 2.19 + Enabled: true diff --git a/Gemfile b/Gemfile index e74ef60..af6a52b 100644 --- a/Gemfile +++ b/Gemfile @@ -15,6 +15,12 @@ group :development, :test do gem "pry" gem "pry-byebug", platform: :mri - gem "rubocop" - gem "rubocop-rspec" + gem "rubocop", "~> 1.50", ">= 1.50.2", require: false + gem "rubocop-rspec", "~> 2.20", require: false + + gem "bundler", "~> 2.0" + gem "rack" + gem "railties" + gem "rake", ">= 12.3.3" + gem "rspec", "~> 3.0" end diff --git a/graphql-anycable.gemspec b/graphql-anycable.gemspec index a5496e0..d180e8e 100644 --- a/graphql-anycable.gemspec +++ b/graphql-anycable.gemspec @@ -26,15 +26,10 @@ Gem::Specification.new do |spec| spec.executables = spec.files.grep(%r{^exe/}) { |f| File.basename(f) } spec.require_paths = ["lib"] + spec.required_ruby_version = ">= 2.7.0" + spec.add_dependency "anycable", "~> 1.0" spec.add_dependency "anyway_config", ">= 1.3", "< 3" spec.add_dependency "graphql", ">= 1.11", "< 3" spec.add_dependency "redis", ">= 4.2.0" - - spec.add_development_dependency "anycable-rails" - spec.add_development_dependency "bundler", "~> 2.0" - spec.add_development_dependency "rack" - spec.add_development_dependency "railties" - spec.add_development_dependency "rake", ">= 12.3.3" - spec.add_development_dependency "rspec", "~> 3.0" end diff --git a/lib/graphql-anycable.rb b/lib/graphql-anycable.rb index c3896c5..e6084d1 100644 --- a/lib/graphql-anycable.rb +++ b/lib/graphql-anycable.rb @@ -13,7 +13,8 @@ module AnyCable def self.use(schema, **options) if config.use_client_provided_uniq_id? warn "[Deprecated] Using client provided channel uniq IDs could lead to unexpected behaviour, " \ - "please, set GraphQL::AnyCable.config.use_client_provided_uniq_id = false or GRAPHQL_ANYCABLE_USE_CLIENT_PROVIDED_UNIQ_ID=false, " \ + "please, set GraphQL::AnyCable.config.use_client_provided_uniq_id = false " \ + "or GRAPHQL_ANYCABLE_USE_CLIENT_PROVIDED_UNIQ_ID=false, " \ "and update the `#unsubscribed` callback code according to the latest docs." end @@ -27,7 +28,7 @@ def redis adapter = ::AnyCable.broadcast_adapter unless adapter.is_a?(::AnyCable::BroadcastAdapters::Redis) raise "Unsupported AnyCable adapter: #{adapter.class}. " \ - "graphql-anycable works only with redis broadcast adapter." + "graphql-anycable works only with redis broadcast adapter." end ::AnyCable.broadcast_adapter.redis_conn end diff --git a/lib/graphql/anycable/cleaner.rb b/lib/graphql/anycable/cleaner.rb index aab37c2..65151a8 100644 --- a/lib/graphql/anycable/cleaner.rb +++ b/lib/graphql/anycable/cleaner.rb @@ -48,7 +48,7 @@ def clean_fingerprint_subscriptions def clean_topic_fingerprints redis.scan_each(match: "#{adapter::FINGERPRINTS_PREFIX}*") do |key| - redis.zremrangebyscore(key, '-inf', '0') + redis.zremrangebyscore(key, "-inf", "0") redis.zrange(key, 0, -1).each do |fingerprint| next if redis.exists?(adapter::SUBSCRIPTIONS_PREFIX + fingerprint) diff --git a/lib/graphql/anycable/errors.rb b/lib/graphql/anycable/errors.rb index 2e25cc0..a558c1d 100644 --- a/lib/graphql/anycable/errors.rb +++ b/lib/graphql/anycable/errors.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module GraphQL module AnyCable # This error is thrown when ActionCable channel wasn't provided to subscription implementation. diff --git a/lib/graphql/anycable/tasks/clean_expired_subscriptions.rake b/lib/graphql/anycable/tasks/clean_expired_subscriptions.rake index c9043a6..979a713 100644 --- a/lib/graphql/anycable/tasks/clean_expired_subscriptions.rake +++ b/lib/graphql/anycable/tasks/clean_expired_subscriptions.rake @@ -5,7 +5,8 @@ require "graphql-anycable" namespace :graphql do namespace :anycable do desc "Clean up stale graphql channels, subscriptions, and events from redis" - task clean: %i[clean:channels clean:subscriptions clean:events clean:fingerprint_subscriptions clean:topic_fingerprints] + task clean: %i[clean:channels clean:subscriptions clean:events clean:fingerprint_subscriptions + clean:topic_fingerprints] namespace :clean do # Clean up old channels diff --git a/lib/graphql/subscriptions/anycable_subscriptions.rb b/lib/graphql/subscriptions/anycable_subscriptions.rb index 4d9aca3..b6844d5 100644 --- a/lib/graphql/subscriptions/anycable_subscriptions.rb +++ b/lib/graphql/subscriptions/anycable_subscriptions.rb @@ -4,7 +4,7 @@ require "graphql/subscriptions" require "graphql/anycable/errors" -# rubocop: disable Metrics/AbcSize, Metrics/LineLength, Metrics/MethodLength +# rubocop:disable Metrics/AbcSize, Metrics/LineLength, Metrics/MethodLength, Metrics/ClassLength # A subscriptions implementation that sends data as AnyCable broadcastings. # @@ -73,20 +73,20 @@ def execute_all(event, object) fingerprints = redis.zrange(FINGERPRINTS_PREFIX + event.topic, 0, -1) return if fingerprints.empty? - fingerprint_subscription_ids = Hash[fingerprints.zip( + fingerprint_subscription_ids = fingerprints.zip( redis.pipelined do |pipeline| fingerprints.map do |fingerprint| pipeline.smembers(SUBSCRIPTIONS_PREFIX + fingerprint) end - end - )] + end, + ).to_h fingerprint_subscription_ids.each do |fingerprint, subscription_ids| execute_grouped(fingerprint, subscription_ids, event, object) end # Call to +trigger+ returns this. Convenient for playing in console - Hash[fingerprint_subscription_ids.map { |k,v| [k, v.size] }] + fingerprint_subscription_ids.transform_values(&:size) end # The fingerprint has told us that this response should be shared by all subscribers, @@ -118,6 +118,7 @@ def deliver(stream_key, result) anycable.broadcast(stream_key, payload) end + # rubocop:disable Metrics/CyclomaticComplexity # Save query to "storage" (in redis) def write_subscription(query, events) context = query.context.to_h @@ -131,7 +132,6 @@ def write_subscription(query, events) # Store subscription_id in the channel state to cleanup on disconnect write_subscription_id(channel, channel_uniq_id) - events.each do |event| channel.stream_from(SUBSCRIPTIONS_PREFIX + event.fingerprint) end @@ -141,7 +141,7 @@ def write_subscription(query, events) variables: query.provided_variables.to_json, context: @serializer.dump(context.to_h), operation_name: query.operation_name, - events: events.map { |e| [e.topic, e.fingerprint] }.to_h.to_json, + events: events.to_h { |e| [e.topic, e.fingerprint] }.to_json, } redis.multi do |pipeline| @@ -152,18 +152,20 @@ def write_subscription(query, events) pipeline.sadd(SUBSCRIPTIONS_PREFIX + event.fingerprint, [subscription_id]) end next unless config.subscription_expiration_seconds + pipeline.expire(CHANNEL_PREFIX + channel_uniq_id, config.subscription_expiration_seconds) pipeline.expire(SUBSCRIPTION_PREFIX + subscription_id, config.subscription_expiration_seconds) end end + # rubocop:enable Metrics/CyclomaticComplexity # Return the query from "storage" (in redis) def read_subscription(subscription_id) redis.mapped_hmget( "#{SUBSCRIPTION_PREFIX}#{subscription_id}", - :query_string, :variables, :context, :operation_name + :query_string, :variables, :context, :operation_name, ).tap do |subscription| - return if subscription.values.all?(&:nil?) # Redis returns hash with all nils for missing key + break if subscription.values.all?(&:nil?) # Redis returns hash with all nils for missing key subscription[:context] = @serializer.load(subscription[:context]) subscription[:variables] = JSON.parse(subscription[:variables]) @@ -187,7 +189,7 @@ def delete_subscription(subscription_id) # Clean up fingerprints that doesn't have any subscriptions left redis.pipelined do |pipeline| fingerprint_subscriptions.each do |key, score| - pipeline.zremrangebyscore(key, '-inf', '0') if score.value.zero? + pipeline.zremrangebyscore(key, "-inf", "0") if score.value.zero? end end end @@ -242,4 +244,4 @@ def fetch_channel_istate(channel) end end end -# rubocop: enable Metrics/AbcSize, Metrics/LineLength, Metrics/MethodLength +# rubocop:enable Metrics/AbcSize, Metrics/LineLength, Metrics/MethodLength, Metrics/ClassLength diff --git a/spec/graphql/anycable_spec.rb b/spec/graphql/anycable_spec.rb index 8510ebd..ecda276 100644 --- a/spec/graphql/anycable_spec.rb +++ b/spec/graphql/anycable_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true RSpec.describe GraphQL::AnyCable do - subject do + subject(:execute_query) do AnycableSchema.execute( query: query, context: { channel: channel, subscription_id: subscription_id }, @@ -23,9 +23,15 @@ end let(:channel) do - socket = double("Socket", istate: AnyCable::Socket::State.new({})) - connection = double("Connection", anycable_socket: socket) - double("Channel", id: "legacy_id", params: { "channelId" => "legacy_id" }, stream_from: nil, connection: connection) + socket = instance_double(AnyCable::Socket, istate: AnyCable::Socket::State.new({})) + connection = instance_double(FakeConnection, anycable_socket: socket) + instance_double( + FakeConnection::Channel, + id: "legacy_id", + params: { "channelId" => "legacy_id" }, + stream_from: nil, + connection: connection, + ) end let(:anycable) { AnyCable.broadcast_adapter } @@ -35,22 +41,23 @@ end let(:fingerprint) do - ":productUpdated:/SomeSubscription/fBDZmJU1UGTorQWvOyUeaHVwUxJ3T9SEqnetj6SKGXc=/0/RBNvo1WzZ4oRRq0W9-hknpT7T8If536DEMBg9hyq_4o=" + ":productUpdated:/SomeSubscription/" \ + "fBDZmJU1UGTorQWvOyUeaHVwUxJ3T9SEqnetj6SKGXc=/0/RBNvo1WzZ4oRRq0W9-hknpT7T8If536DEMBg9hyq_4o=" end before do allow(anycable).to receive(:broadcast) - allow_any_instance_of(GraphQL::Subscriptions::Event).to receive(:fingerprint).and_return(fingerprint) - allow_any_instance_of(GraphQL::Subscriptions).to receive(:build_id).and_return("ohmycables") + allow_any_instance_of(GraphQL::Subscriptions::Event).to receive(:fingerprint).and_return(fingerprint) # rubocop:disable RSpec/AnyInstance + allow_any_instance_of(GraphQL::Subscriptions).to receive(:build_id).and_return("ohmycables") # rubocop:disable RSpec/AnyInstance end it "subscribes channel to stream updates from GraphQL subscription" do - subject + execute_query expect(channel).to have_received(:stream_from).with("graphql-subscriptions:#{fingerprint}") end it "broadcasts message when event is being triggered" do - subject + execute_query AnycableSchema.subscriptions.trigger(:product_updated, {}, { id: 1, title: "foo" }) expect(anycable).to have_received(:broadcast).with("graphql-subscriptions:#{fingerprint}", expected_result) end @@ -65,15 +72,15 @@ GRAPHQL end - context "triggering update event" do + context "with triggering update event" do it "broadcasts message only for update event" do - subject + execute_query AnycableSchema.subscriptions.trigger(:product_updated, {}, { id: 1, title: "foo" }) expect(anycable).to have_received(:broadcast).with("graphql-subscriptions:#{fingerprint}", expected_result) end end - context "triggering create event" do + context "with triggering create event" do let(:expected_result) do <<~JSON.strip {"result":{"data":{"productCreated":{"id":"1","title":"Gravizapa"}}},"more":true} @@ -81,7 +88,7 @@ end it "broadcasts message only for create event" do - subject + execute_query AnycableSchema.subscriptions.trigger(:product_created, {}, { id: 1, title: "Gravizapa" }) expect(anycable).to have_received(:broadcast).with("graphql-subscriptions:#{fingerprint}", expected_result) @@ -90,11 +97,12 @@ end describe ".delete_channel_subscriptions" do - before do - GraphQL::AnyCable.config.use_client_provided_uniq_id = false + subject(:delete_subscriptions) do + AnycableSchema.subscriptions.delete_channel_subscriptions(channel) end before do + described_class.config.use_client_provided_uniq_id = false AnycableSchema.execute( query: query, context: { channel: channel, subscription_id: subscription_id }, @@ -104,20 +112,16 @@ end after do - GraphQL::AnyCable.config.use_client_provided_uniq_id = false + described_class.config.use_client_provided_uniq_id = false end let(:redis) { AnycableSchema.subscriptions.redis } - subject do - AnycableSchema.subscriptions.delete_channel_subscriptions(channel) - end - it "removes subscription from redis" do expect(redis.exists?("graphql-subscription:some-truly-random-number")).to be true expect(redis.exists?("graphql-channel:some-truly-random-number")).to be true expect(redis.exists?("graphql-fingerprints::productUpdated:")).to be true - subject + delete_subscriptions expect(redis.exists?("graphql-channel:some-truly-random-number")).to be false expect(redis.exists?("graphql-fingerprints::productUpdated:")).to be false expect(redis.exists?("graphql-subscription:some-truly-random-number")).to be false @@ -125,11 +129,12 @@ end describe "legacy .delete_channel_subscriptions" do - before do - GraphQL::AnyCable.config.use_client_provided_uniq_id = true + subject(:delete_subscriptions) do + AnycableSchema.subscriptions.delete_channel_subscriptions(channel.id) end before do + described_class.config.use_client_provided_uniq_id = true AnycableSchema.execute( query: query, context: { channel: channel, subscription_id: subscription_id }, @@ -139,20 +144,16 @@ end after do - GraphQL::AnyCable.config.use_client_provided_uniq_id = false + described_class.config.use_client_provided_uniq_id = false end let(:redis) { AnycableSchema.subscriptions.redis } - subject do - AnycableSchema.subscriptions.delete_channel_subscriptions(channel.id) - end - it "removes subscription from redis" do expect(redis.exists?("graphql-subscription:some-truly-random-number")).to be true expect(redis.exists?("graphql-channel:legacy_id")).to be true expect(redis.exists?("graphql-fingerprints::productUpdated:")).to be true - subject + delete_subscriptions expect(redis.exists?("graphql-channel:legacy_id")).to be false expect(redis.exists?("graphql-fingerprints::productUpdated:")).to be false expect(redis.exists?("graphql-subscription:some-truly-random-number")).to be false @@ -160,7 +161,7 @@ end describe "with missing channel instance in execution context" do - subject do + subject(:execute_query) do AnycableSchema.execute( query: query, context: {}, # Intentionally left blank @@ -176,7 +177,7 @@ end it "raises configuration error" do - expect { subject }.to raise_error( + expect { execute_query }.to raise_error( GraphQL::AnyCable::ChannelConfigurationError, /ActionCable channel wasn't provided in the context for GraphQL query execution!/, ) diff --git a/spec/graphql/broadcast_spec.rb b/spec/graphql/broadcast_spec.rb index fe5d485..ede37c2 100644 --- a/spec/graphql/broadcast_spec.rb +++ b/spec/graphql/broadcast_spec.rb @@ -11,13 +11,13 @@ def subscribe(query) end let(:channel) do - socket = double("Socket", istate: AnyCable::Socket::State.new({})) - connection = double("Connection", anycable_socket: socket) - double("Channel", connection: connection) + socket = instance_double(AnyCable::Socket, istate: AnyCable::Socket::State.new({})) + connection = instance_double(FakeConnection, anycable_socket: socket) + instance_double(FakeConnection::Channel, connection: connection) end let(:object) do - double("Post", id: 1, title: "Broadcasting…", actions: %w[Edit Delete]) + double(Post, id: 1, title: "Broadcasting…", actions: %w[Edit Delete]) # rubocop:disable RSpec/VerifiedDoubles end let(:query) do diff --git a/spec/integration_helper.rb b/spec/integration_helper.rb index e76b85a..e340539 100644 --- a/spec/integration_helper.rb +++ b/spec/integration_helper.rb @@ -3,15 +3,15 @@ require "anycable/rspec" require "rack" -RSpec.shared_context "rpc" do +RSpec.shared_context "with rpc" do include_context "anycable:rpc:command" let(:user) { "john" } let(:schema) { nil } - let(:identifiers) { {current_user: "john", schema: schema.to_s} } + let(:identifiers) { { current_user: "john", schema: schema.to_s } } let(:channel_class) { "GraphqlChannel" } - let(:channel_params) { {channelId: rand(1000).to_s} } - let(:channel_identifier) { {channel: channel_class}.merge(channel_params) } + let(:channel_params) { { channelId: rand(1000).to_s } } + let(:channel_identifier) { { channel: channel_class }.merge(channel_params) } let(:channel_id) { channel_identifier.to_json } let(:handler) { AnyCable::RPC::Handler.new } @@ -28,6 +28,10 @@ def initialize(connection, identifier, params) @params = params end + def id + @params["channelId"] + end + def stream_from(broadcasting) connection.socket.subscribe identifier, broadcasting end @@ -46,36 +50,36 @@ def initialize(socket, identifiers: nil, subscriptions: nil) @subscriptions = subscriptions end + # rubocop:disable Metrics/AbcSize, Metrics/MethodLength def handle_channel_command(identifier, command, data) parsed_id = JSON.parse(identifier) parsed_id.delete("channel") channel = Channel.new(self, identifier, parsed_id) - res = - case command - when "message" - data = JSON.parse(data) - result = - schema.execute( - query: data["query"], - context: identifiers.merge(channel: channel), - variables: Hash(data["variables"]), - operation_name: data["operationName"], - ) - - transmit( - result: result.subscription? ? { data: nil } : result.to_h, - more: result.subscription?, + case command + when "message" + data = JSON.parse(data) + result = + schema.execute( + query: data["query"], + context: identifiers.merge(channel: channel), + variables: Hash(data["variables"]), + operation_name: data["operationName"], ) - when "unsubscribe" - schema.subscriptions.delete_channel_subscriptions(channel) - true - else - raise "Unknown command" - end - res + + transmit( + result: result.subscription? ? { data: nil } : result.to_h, + more: result.subscription?, + ) + when "unsubscribe" + schema.subscriptions.delete_channel_subscriptions(channel) + true + else + raise "Unknown command" + end end + # rubocop:enable Metrics/AbcSize, Metrics/MethodLength def transmit(data) socket.transmit data.to_json @@ -87,7 +91,7 @@ def identifiers_json def close socket.close - end + end end AnyCable.connection_factory = ->(socket, **options) { FakeConnection.new(socket, **options) } @@ -102,5 +106,5 @@ def close metadata[:integration] = true end - config.include_context "rpc", integration: true + config.include_context "with rpc", integration: true end diff --git a/spec/integrations/broadcastable_subscriptions_spec.rb b/spec/integrations/broadcastable_subscriptions_spec.rb index 8b9a0f4..5c49633 100644 --- a/spec/integrations/broadcastable_subscriptions_spec.rb +++ b/spec/integrations/broadcastable_subscriptions_spec.rb @@ -3,6 +3,8 @@ require "integration_helper" RSpec.describe "broadcastable subscriptions" do + subject(:execute_request) { handler.handle(:command, request) } + let(:schema) { BroadcastSchema } let(:query) do @@ -16,31 +18,29 @@ } GQL end - let(:variables) { {id: "a"} } + let(:variables) { { id: "a" } } - let(:subscription_payload) { {query: query, variables: variables} } + let(:subscription_payload) { { query: query, variables: variables } } let(:command) { "message" } - let(:data) { {action: "execute", **subscription_payload} } - - subject { handler.handle(:command, request) } + let(:data) { { action: "execute", **subscription_payload } } before { allow(AnyCable.broadcast_adapter).to receive(:broadcast) } describe "execute" do it "responds with result" do - expect(subject).to be_success - expect(subject.transmissions.size).to eq 1 - expect(subject.transmissions.first).to eq({result: {data: nil}, more: true}.to_json) - expect(subject.streams.size).to eq 1 - expect(subject.istate["sid"]).not_to be_nil + expect(execute_request).to be_success + expect(execute_request.transmissions.size).to eq 1 + expect(execute_request.transmissions.first).to eq({ result: { data: nil }, more: true }.to_json) + expect(execute_request.streams.size).to eq 1 + expect(execute_request.istate["sid"]).not_to be_nil end specify "streams depends only on query params and the same for equal subscriptions" do - expect(subject).to be_success - expect(subject.streams.size).to eq 1 + expect(execute_request).to be_success + expect(execute_request.streams.size).to eq 1 - stream_name = subject.streams.first + stream_name = execute_request.streams.first # update request context and channelId request.connection_identifiers = identifiers.merge(current_user: "alice").to_json @@ -51,7 +51,7 @@ expect(response.streams).to eq([stream_name]) # now update the query param - request.data = data.merge(variables: {id: "b"}).to_json + request.data = data.merge(variables: { id: "b" }).to_json request.identifier = channel_identifier.merge(channelId: rand(1000).to_s).to_json response = handler.handle(:command, request) @@ -88,18 +88,18 @@ # first, subscribe to obtain the connection state subscribe_response = handler.handle(:command, request) expect(subscribe_response).to be_success - + expect(redis.keys("graphql-subscription:*").size).to eq(1) - + istate = subscribe_response.istate - + request.command = "unsubscribe" request.data = "" request.istate[channel_id] = istate.to_h.to_json - + response = handler.handle(:command, request) expect(response).to be_success - + expect(redis.keys("graphql-subscription:*").size).to eq(0) end end @@ -112,18 +112,18 @@ expect(redis.keys("graphql-subscription:*").size).to eq(1) expect(redis.keys("graphql-subscriptions:*").size).to eq(1) - request_2 = request.dup + request2 = request.dup # update request context and channelId - request_2.connection_identifiers = identifiers.merge(current_user: "alice").to_json - request_2.identifier = channel_identifier.merge(channelId: rand(1000).to_s).to_json + request2.connection_identifiers = identifiers.merge(current_user: "alice").to_json + request2.identifier = channel_identifier.merge(channelId: rand(1000).to_s).to_json - response_2 = handler.handle(:command, request_2) + response2 = handler.handle(:command, request2) expect(redis.keys("graphql-subscription:*").size).to eq(2) expect(redis.keys("graphql-subscriptions:*").size).to eq(1) - schema.subscriptions.trigger(:post_updated, {id: "a"}, POSTS.first) + schema.subscriptions.trigger(:post_updated, { id: "a" }, POSTS.first) expect(AnyCable.broadcast_adapter).to have_received(:broadcast).once first_state = response.istate @@ -138,14 +138,14 @@ expect(redis.keys("graphql-subscription:*").size).to eq(1) expect(redis.keys("graphql-subscriptions:*").size).to eq(1) - schema.subscriptions.trigger(:post_updated, {id: "a"}, POSTS.first) + schema.subscriptions.trigger(:post_updated, { id: "a" }, POSTS.first) expect(AnyCable.broadcast_adapter).to have_received(:broadcast).twice - second_state = response_2.istate + second_state = response2.istate - request_2.command = "unsubscribe" - request_2.data = "" - request_2.istate = second_state + request2.command = "unsubscribe" + request2.data = "" + request2.istate = second_state response = handler.handle(:command, request) expect(response).to be_success @@ -153,7 +153,7 @@ expect(redis.keys("graphql-subscription:*").size).to eq(0) expect(redis.keys("graphql-subscriptions:*").size).to eq(0) - schema.subscriptions.trigger(:post_updated, {id: "a"}, POSTS.first) + schema.subscriptions.trigger(:post_updated, { id: "a" }, POSTS.first) expect(AnyCable.broadcast_adapter).to have_received(:broadcast).twice end @@ -172,27 +172,27 @@ # first, subscribe to obtain the connection state subscribe_response = handler.handle(:command, request) expect(subscribe_response).to be_success - + expect(redis.keys("graphql-subscription:*").size).to eq(1) expect(redis.keys("graphql-subscriptions:*").size).to eq(1) - + # update request context request.connection_identifiers = identifiers.merge(current_user: "alice").to_json - + response = handler.handle(:command, request) - + expect(redis.keys("graphql-subscription:*").size).to eq(2) expect(redis.keys("graphql-subscriptions:*").size).to eq(1) - + istate = response.istate - + request.command = "unsubscribe" request.data = "" request.istate = istate - + response = handler.handle(:command, request) expect(response).to be_success - + expect(redis.keys("graphql-subscription:*").size).to eq(1) expect(redis.keys("graphql-subscriptions:*").size).to eq(1) end @@ -203,7 +203,7 @@ let(:command) { "unsubscribe" } specify do - expect(subject).to be_success + expect(execute_request).to be_success end end end diff --git a/spec/integrations/per_client_subscriptions_spec.rb b/spec/integrations/per_client_subscriptions_spec.rb index 9a3a47a..57397a3 100644 --- a/spec/integrations/per_client_subscriptions_spec.rb +++ b/spec/integrations/per_client_subscriptions_spec.rb @@ -1,9 +1,10 @@ - # frozen_string_literal: true require "integration_helper" RSpec.describe "non-broadcastable subscriptions" do + subject(:execute_request) { handler.handle(:command, request) } + let(:schema) { AnycableSchema } let(:query) do @@ -17,31 +18,29 @@ } GQL end - let(:variables) { {id: "a"} } + let(:variables) { { id: "a" } } - let(:subscription_payload) { {query: query, variables: variables} } + let(:subscription_payload) { { query: query, variables: variables } } let(:command) { "message" } - let(:data) { {action: "execute", **subscription_payload} } - - subject { handler.handle(:command, request) } + let(:data) { { action: "execute", **subscription_payload } } before { allow(AnyCable.broadcast_adapter).to receive(:broadcast) } describe "execute" do it "responds with result" do - expect(subject).to be_success - expect(subject.transmissions.size).to eq 1 - expect(subject.transmissions.first).to eq({result: {data: nil}, more: true}.to_json) - expect(subject.streams.size).to eq 1 - expect(subject.istate["sid"]).not_to be_nil + expect(execute_request).to be_success + expect(execute_request.transmissions.size).to eq 1 + expect(execute_request.transmissions.first).to eq({ result: { data: nil }, more: true }.to_json) + expect(execute_request.streams.size).to eq 1 + expect(execute_request.istate["sid"]).not_to be_nil end specify "creates uniq stream for each subscription" do - expect(subject).to be_success - expect(subject.streams.size).to eq 1 + expect(execute_request).to be_success + expect(execute_request.streams.size).to eq 1 - all_streams = Set.new(subject.streams) + all_streams = Set.new(execute_request.streams) # update request channelId request.identifier = channel_identifier.merge(channelId: rand(1000).to_s).to_json @@ -54,7 +53,7 @@ expect(all_streams.size).to eq 2 # now update the query param - request.data = data.merge(variables: {id: "b"}).to_json + request.data = data.merge(variables: { id: "b" }).to_json request.identifier = channel_identifier.merge(channelId: rand(1000).to_s).to_json response = handler.handle(:command, request) @@ -96,18 +95,18 @@ expect(redis.keys("graphql-subscription:*").size).to eq(1) expect(redis.keys("graphql-subscriptions:*").size).to eq(1) - request_2 = request.dup + request2 = request.dup # update request context and channelId - request_2.connection_identifiers = identifiers.merge(current_user: "alice").to_json - request_2.identifier = channel_identifier.merge(channelId: rand(1000).to_s).to_json + request2.connection_identifiers = identifiers.merge(current_user: "alice").to_json + request2.identifier = channel_identifier.merge(channelId: rand(1000).to_s).to_json - response_2 = handler.handle(:command, request_2) + response2 = handler.handle(:command, request2) expect(redis.keys("graphql-subscription:*").size).to eq(2) expect(redis.keys("graphql-subscriptions:*").size).to eq(2) - schema.subscriptions.trigger(:post_updated, {id: "a"}, POSTS.first) + schema.subscriptions.trigger(:post_updated, { id: "a" }, POSTS.first) expect(AnyCable.broadcast_adapter).to have_received(:broadcast).twice first_state = response.istate @@ -122,14 +121,14 @@ expect(redis.keys("graphql-subscription:*").size).to eq(1) expect(redis.keys("graphql-subscriptions:*").size).to eq(1) - schema.subscriptions.trigger(:post_updated, {id: "a"}, POSTS.first) + schema.subscriptions.trigger(:post_updated, { id: "a" }, POSTS.first) expect(AnyCable.broadcast_adapter).to have_received(:broadcast).thrice - second_state = response_2.istate + second_state = response2.istate - request_2.command = "unsubscribe" - request_2.data = "" - request_2.istate = second_state + request2.command = "unsubscribe" + request2.data = "" + request2.istate = second_state response = handler.handle(:command, request) expect(response).to be_success @@ -137,7 +136,7 @@ expect(redis.keys("graphql-subscription:*").size).to eq(0) expect(redis.keys("graphql-subscriptions:*").size).to eq(0) - schema.subscriptions.trigger(:post_updated, {id: "a"}, POSTS.first) + schema.subscriptions.trigger(:post_updated, { id: "a" }, POSTS.first) expect(AnyCable.broadcast_adapter).to have_received(:broadcast).thrice end @@ -156,27 +155,27 @@ # first, subscribe to obtain the connection state subscribe_response = handler.handle(:command, request) expect(subscribe_response).to be_success - + expect(redis.keys("graphql-subscription:*").size).to eq(1) expect(redis.keys("graphql-subscriptions:*").size).to eq(1) - + # update request context request.connection_identifiers = identifiers.merge(current_user: "alice").to_json - + response = handler.handle(:command, request) - + expect(redis.keys("graphql-subscription:*").size).to eq(2) expect(redis.keys("graphql-subscriptions:*").size).to eq(2) - + istate = response.istate - + request.command = "unsubscribe" request.data = "" request.istate = istate - + response = handler.handle(:command, request) expect(response).to be_success - + expect(redis.keys("graphql-subscription:*").size).to eq(1) expect(redis.keys("graphql-subscriptions:*").size).to eq(1) end @@ -187,7 +186,7 @@ let(:command) { "unsubscribe" } specify do - expect(subject).to be_success + expect(execute_request).to be_success end end end diff --git a/spec/integrations/rails_spec.rb b/spec/integrations/rails_spec.rb index da79df7..4fe8a95 100644 --- a/spec/integrations/rails_spec.rb +++ b/spec/integrations/rails_spec.rb @@ -17,7 +17,7 @@ def self.root require "action_cable/server" require "action_cable/server/base" # Only for anycable-rails <1.3.0 -unless defined?(::AnyCable::Rails::Connection) +unless defined?(AnyCable::Rails::Connection) require "anycable/rails/channel_state" require "anycable/rails/actioncable/connection" end @@ -34,8 +34,9 @@ def schema_class class GraphqlChannel < ActionCable::Channel::Base delegate :schema_class, to: :connection + # rubocop:disable Metrics/MethodLength def execute(data) - result = + result = schema_class.execute( query: data["query"], context: context, @@ -46,10 +47,11 @@ def execute(data) transmit( { result: result.subscription? ? { data: nil } : result.to_h, - more: result.subscription? - } + more: result.subscription?, + }, ) end + # rubocop:enable Metrics/MethodLength def unsubscribed schema_class.subscriptions.delete_channel_subscriptions(self) @@ -68,31 +70,10 @@ def context RSpec.describe "Rails integration" do let(:schema) { BroadcastSchema } - let(:channel_class) { "ApplicationCable::GraphqlChannel" } - - if defined?(::AnyCable::Rails::Connection) - before do - allow(AnyCable).to receive(:connection_factory) - .and_return(->(socket, **options) { ::AnyCable::Rails::Connection.new(ApplicationCable::Connection, socket, **options) }) - end - else - before do - allow(AnyCable).to receive(:connection_factory) - .and_return(->(socket, **options) { ApplicationCable::Connection.call(socket, **options) }) - end - end - - let(:variables) { {id: "a"} } - - let(:subscription_payload) { {query: query, variables: variables} } - + let(:variables) { { id: "a" } } + let(:subscription_payload) { { query: query, variables: variables } } let(:command) { "message" } - let(:data) { {action: "execute", **subscription_payload} } - - subject { handler.handle(:command, request) } - - before { allow(AnyCable.broadcast_adapter).to receive(:broadcast) } - + let(:data) { { action: "execute", **subscription_payload } } let(:query) do <<~GQL subscription postSubscription($id: ID!) { @@ -104,8 +85,21 @@ def context } GQL end - let(:redis) { AnycableSchema.subscriptions.redis } + let(:channel_class) { "ApplicationCable::GraphqlChannel" } + + before do + if defined?(AnyCable::Rails::Connection) + allow(AnyCable).to receive(:connection_factory) + .and_return(lambda { |socket, **options| + AnyCable::Rails::Connection.new(ApplicationCable::Connection, socket, **options) + }) + else + allow(AnyCable).to receive(:connection_factory) + .and_return(->(socket, **options) { ApplicationCable::Connection.call(socket, **options) }) + end + allow(AnyCable.broadcast_adapter).to receive(:broadcast) + end it "execute multiple clients + trigger + disconnect one by one" do # first, subscribe to obtain the connection state @@ -115,18 +109,18 @@ def context expect(redis.keys("graphql-subscription:*").size).to eq(1) expect(redis.keys("graphql-subscriptions:*").size).to eq(1) - request_2 = request.dup + request2 = request.dup # update request context and channelId - request_2.connection_identifiers = identifiers.merge(current_user: "alice").to_json - request_2.identifier = channel_identifier.merge(channelId: rand(1000).to_s).to_json + request2.connection_identifiers = identifiers.merge(current_user: "alice").to_json + request2.identifier = channel_identifier.merge(channelId: rand(1000).to_s).to_json - response_2 = handler.handle(:command, request_2) + response2 = handler.handle(:command, request2) expect(redis.keys("graphql-subscription:*").size).to eq(2) expect(redis.keys("graphql-subscriptions:*").size).to eq(1) - schema.subscriptions.trigger(:post_updated, {id: "a"}, POSTS.first) + schema.subscriptions.trigger(:post_updated, { id: "a" }, POSTS.first) expect(AnyCable.broadcast_adapter).to have_received(:broadcast).once first_state = response.istate @@ -141,19 +135,19 @@ def context expect(redis.keys("graphql-subscription:*").size).to eq(1) expect(redis.keys("graphql-subscriptions:*").size).to eq(1) - schema.subscriptions.trigger(:post_updated, {id: "a"}, POSTS.first) + schema.subscriptions.trigger(:post_updated, { id: "a" }, POSTS.first) expect(AnyCable.broadcast_adapter).to have_received(:broadcast).twice - second_state = response_2.istate + second_state = response2.istate # Disconnect the second one via #disconnect call disconnect_request = AnyCable::DisconnectRequest.new( - identifiers: request_2.connection_identifiers, - subscriptions: [request_2.identifier], - env: request_2.env + identifiers: request2.connection_identifiers, + subscriptions: [request2.identifier], + env: request2.env, ) - disconnect_request.istate[request_2.identifier] = second_state.to_h.to_json + disconnect_request.istate[request2.identifier] = second_state.to_h.to_json disconnect_response = handler.handle(:disconnect, disconnect_request) expect(disconnect_response).to be_success @@ -161,7 +155,7 @@ def context expect(redis.keys("graphql-subscription:*").size).to eq(0) expect(redis.keys("graphql-subscriptions:*").size).to eq(0) - schema.subscriptions.trigger(:post_updated, {id: "a"}, POSTS.first) + schema.subscriptions.trigger(:post_updated, { id: "a" }, POSTS.first) expect(AnyCable.broadcast_adapter).to have_received(:broadcast).twice end end diff --git a/spec/redis_helper.rb b/spec/redis_helper.rb index f7187f3..5b9db5e 100644 --- a/spec/redis_helper.rb +++ b/spec/redis_helper.rb @@ -11,7 +11,7 @@ def setup_redis_test_db setup_redis_test_db RSpec.configure do |config| - config.before(:example) do + config.before do GraphQL::AnyCable.redis.flushdb end end diff --git a/spec/support/graphql_schema.rb b/spec/support/graphql_schema.rb index ee946b1..7d6b60c 100644 --- a/spec/support/graphql_schema.rb +++ b/spec/support/graphql_schema.rb @@ -2,7 +2,7 @@ POSTS = [ { id: "a", title: "GraphQL is good?", actions: %w[yes no] }, - { id: "b", title: "Is there life after GraphQL?", actions: %w[no still-no] } + { id: "b", title: "Is there life after GraphQL?", actions: %w[no still-no] }, ].freeze class Product < GraphQL::Schema::Object @@ -22,11 +22,11 @@ class PostUpdated < GraphQL::Schema::Subscription field :post, Post, null: false def subscribe(id:) - {post: POSTS.find { |post| post[:id] == id }} + { post: POSTS.find { |post| post[:id] == id } } end def update(*) - {post: object} + { post: object } end end @@ -53,18 +53,17 @@ class PostCreated < GraphQL::Schema::Subscription payload_type Post end - class PostUpdated < GraphQL::Schema::Subscription argument :id, ID, required: true field :post, Post, null: false def subscribe(id:) - {post: POSTS.find { |post| post[:id] == id }} + { post: POSTS.find { |post| post[:id] == id } } end def update(*) - {post: object} + { post: object } end end