From 81e79e88ac28e8d83676e155692d27d0526adb98 Mon Sep 17 00:00:00 2001
From: Heinrich Lee Yu <heinrich@gitlab.com>
Date: Wed, 29 Apr 2020 20:52:43 +0800
Subject: [PATCH] Broadcast issue updates to an ActionCable channel
This allows us to update the issue sidebar in real-time
---
app/channels/application_cable/channel.rb | 6 +++
app/channels/application_cable/connection.rb | 22 +++++++++
app/channels/issues_channel.rb | 13 +++++
app/models/active_session.rb | 4 +-
app/services/issues/update_service.rb | 4 ++
config/cable.yml.example | 14 ++++++
lib/quality/test_level.rb | 1 +
scripts/prepare_build.sh | 3 ++
.../application_cable/connection_spec.rb | 47 +++++++++++++++++++
spec/channels/issues_channel_spec.rb | 36 ++++++++++++++
spec/lib/quality/test_level_spec.rb | 4 +-
spec/services/issues/update_service_spec.rb | 28 +++++++++++
12 files changed, 178 insertions(+), 4 deletions(-)
create mode 100644 app/channels/application_cable/channel.rb
create mode 100644 app/channels/application_cable/connection.rb
create mode 100644 app/channels/issues_channel.rb
create mode 100644 config/cable.yml.example
create mode 100644 spec/channels/application_cable/connection_spec.rb
create mode 100644 spec/channels/issues_channel_spec.rb
diff --git a/app/channels/application_cable/channel.rb b/app/channels/application_cable/channel.rb
new file mode 100644
index 0000000000000..9aec2305390f8
--- /dev/null
+++ b/app/channels/application_cable/channel.rb
@@ -0,0 +1,6 @@
+# frozen_string_literal: true
+
+module ApplicationCable
+ class Channel < ActionCable::Channel::Base
+ end
+end
diff --git a/app/channels/application_cable/connection.rb b/app/channels/application_cable/connection.rb
new file mode 100644
index 0000000000000..87c833f3593c2
--- /dev/null
+++ b/app/channels/application_cable/connection.rb
@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+module ApplicationCable
+ class Connection < ActionCable::Connection::Base
+ identified_by :current_user
+
+ def connect
+ self.current_user = find_user_from_session_store
+ end
+
+ private
+
+ def find_user_from_session_store
+ session = ActiveSession.sessions_from_ids([session_id]).first
+ Warden::SessionSerializer.new('rack.session' => session).fetch(:user)
+ end
+
+ def session_id
+ Rack::Session::SessionId.new(cookies[Gitlab::Application.config.session_options[:key]])
+ end
+ end
+end
diff --git a/app/channels/issues_channel.rb b/app/channels/issues_channel.rb
new file mode 100644
index 0000000000000..5f3909b77167f
--- /dev/null
+++ b/app/channels/issues_channel.rb
@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+class IssuesChannel < ApplicationCable::Channel
+ def subscribed
+ project = Project.find_by_full_path(params[:project_path])
+ return reject unless project
+
+ issue = project.issues.find_by_iid(params[:iid])
+ return reject unless issue && Ability.allowed?(current_user, :read_issue, issue)
+
+ stream_for issue
+ end
+end
diff --git a/app/models/active_session.rb b/app/models/active_session.rb
index 050155398ab39..065bd5507be80 100644
--- a/app/models/active_session.rb
+++ b/app/models/active_session.rb
@@ -124,7 +124,7 @@ def self.session_ids_for_user(user_id)
end
end
- # Lists the ActiveSession objects for the given session IDs.
+ # Lists the session Hash objects for the given session IDs.
#
# session_ids - An array of Rack::Session::SessionId objects
#
@@ -143,7 +143,7 @@ def self.sessions_from_ids(session_ids)
end
end
- # Deserializes an ActiveSession object from Redis.
+ # Deserializes a session Hash object from Redis.
#
# raw_session - Raw bytes from Redis
#
diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb
index 78ebbd7bff259..ee1a22634af0b 100644
--- a/app/services/issues/update_service.rb
+++ b/app/services/issues/update_service.rb
@@ -21,6 +21,10 @@ def before_update(issue, skip_spam_check: false)
spam_check(issue, current_user) unless skip_spam_check
end
+ def after_update(issue)
+ IssuesChannel.broadcast_to(issue, event: 'updated') if Feature.enabled?(:broadcast_issue_updates, issue.project)
+ end
+
def handle_changes(issue, options)
old_associations = options.fetch(:old_associations, {})
old_labels = old_associations.fetch(:labels, [])
diff --git a/config/cable.yml.example b/config/cable.yml.example
new file mode 100644
index 0000000000000..ee3a8da9be8c9
--- /dev/null
+++ b/config/cable.yml.example
@@ -0,0 +1,14 @@
+# This file is used for configuring ActionCable in our CI environment
+# When using GDK or Omnibus, cable.yml is generated from a different template
+development:
+ adapter: redis
+ url: redis://localhost:6379
+ channel_prefix: gitlab_development
+test:
+ adapter: redis
+ url: redis://localhost:6379
+ channel_prefix: gitlab_test
+production:
+ adapter: redis
+ url: unix:/var/run/redis/redis.sock
+ channel_prefix: gitlab_production
diff --git a/lib/quality/test_level.rb b/lib/quality/test_level.rb
index bbd8b4dcc3f62..97b86fa8c2e51 100644
--- a/lib/quality/test_level.rb
+++ b/lib/quality/test_level.rb
@@ -14,6 +14,7 @@ class TestLevel
],
unit: %w[
bin
+ channels
config
db
dependencies
diff --git a/scripts/prepare_build.sh b/scripts/prepare_build.sh
index e80d752f09fd0..7bf3f887e976b 100644
--- a/scripts/prepare_build.sh
+++ b/scripts/prepare_build.sh
@@ -33,6 +33,9 @@ if [ -f config/database_geo.yml ]; then
sed -i 's/username: git/username: postgres/g' config/database_geo.yml
fi
+cp config/cable.yml.example config/cable.yml
+sed -i 's|url:.*$|url: redis://redis:6379|g' config/cable.yml
+
cp config/resque.yml.example config/resque.yml
sed -i 's|url:.*$|url: redis://redis:6379|g' config/resque.yml
diff --git a/spec/channels/application_cable/connection_spec.rb b/spec/channels/application_cable/connection_spec.rb
new file mode 100644
index 0000000000000..f3d671335282f
--- /dev/null
+++ b/spec/channels/application_cable/connection_spec.rb
@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ApplicationCable::Connection, :clean_gitlab_redis_shared_state do
+ let(:session_id) { Rack::Session::SessionId.new('6919a6f1bb119dd7396fadc38fd18d0d') }
+
+ before do
+ Gitlab::Redis::SharedState.with do |redis|
+ redis.set("session:gitlab:#{session_id.private_id}", Marshal.dump(session_hash))
+ end
+
+ cookies[Gitlab::Application.config.session_options[:key]] = session_id.public_id
+ end
+
+ context 'when user is logged in' do
+ let(:user) { create(:user) }
+ let(:session_hash) { { 'warden.user.user.key' => [[user.id], user.encrypted_password[0, 29]] } }
+
+ it 'sets current_user' do
+ connect
+
+ expect(connection.current_user).to eq(user)
+ end
+
+ context 'with a stale password' do
+ let(:partial_password_hash) { build(:user, password: 'some_old_password').encrypted_password[0, 29] }
+ let(:session_hash) { { 'warden.user.user.key' => [[user.id], partial_password_hash] } }
+
+ it 'sets current_user to nil' do
+ connect
+
+ expect(connection.current_user).to be_nil
+ end
+ end
+ end
+
+ context 'when user is not logged in' do
+ let(:session_hash) { {} }
+
+ it 'sets current_user to nil' do
+ connect
+
+ expect(connection.current_user).to be_nil
+ end
+ end
+end
diff --git a/spec/channels/issues_channel_spec.rb b/spec/channels/issues_channel_spec.rb
new file mode 100644
index 0000000000000..1c88cc734564a
--- /dev/null
+++ b/spec/channels/issues_channel_spec.rb
@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe IssuesChannel do
+ let_it_be(:issue) { create(:issue) }
+
+ it 'rejects when project path is invalid' do
+ subscribe(project_path: 'invalid_project_path', iid: issue.iid)
+
+ expect(subscription).to be_rejected
+ end
+
+ it 'rejects when iid is invalid' do
+ subscribe(project_path: issue.project.full_path, iid: non_existing_record_iid)
+
+ expect(subscription).to be_rejected
+ end
+
+ it 'rejects when the user does not have access' do
+ stub_connection current_user: nil
+
+ subscribe(project_path: issue.project.full_path, iid: issue.iid)
+
+ expect(subscription).to be_rejected
+ end
+
+ it 'subscribes to a stream when the user has access' do
+ stub_connection current_user: issue.author
+
+ subscribe(project_path: issue.project.full_path, iid: issue.iid)
+
+ expect(subscription).to be_confirmed
+ expect(subscription).to have_stream_for(issue)
+ end
+end
diff --git a/spec/lib/quality/test_level_spec.rb b/spec/lib/quality/test_level_spec.rb
index 6042ab2478739..b784a92fa8533 100644
--- a/spec/lib/quality/test_level_spec.rb
+++ b/spec/lib/quality/test_level_spec.rb
@@ -21,7 +21,7 @@
context 'when level is unit' do
it 'returns a pattern' do
expect(subject.pattern(:unit))
- .to eq("spec/{bin,config,db,dependencies,factories,finders,frontend,graphql,haml_lint,helpers,initializers,javascripts,lib,models,policies,presenters,rack_servers,replicators,routing,rubocop,serializers,services,sidekiq,support_specs,tasks,uploaders,validators,views,workers,elastic_integration}{,/**/}*_spec.rb")
+ .to eq("spec/{bin,channels,config,db,dependencies,factories,finders,frontend,graphql,haml_lint,helpers,initializers,javascripts,lib,models,policies,presenters,rack_servers,replicators,routing,rubocop,serializers,services,sidekiq,support_specs,tasks,uploaders,validators,views,workers,elastic_integration}{,/**/}*_spec.rb")
end
end
@@ -89,7 +89,7 @@
context 'when level is unit' do
it 'returns a regexp' do
expect(subject.regexp(:unit))
- .to eq(%r{spec/(bin|config|db|dependencies|factories|finders|frontend|graphql|haml_lint|helpers|initializers|javascripts|lib|models|policies|presenters|rack_servers|replicators|routing|rubocop|serializers|services|sidekiq|support_specs|tasks|uploaders|validators|views|workers|elastic_integration)})
+ .to eq(%r{spec/(bin|channels|config|db|dependencies|factories|finders|frontend|graphql|haml_lint|helpers|initializers|javascripts|lib|models|policies|presenters|rack_servers|replicators|routing|rubocop|serializers|services|sidekiq|support_specs|tasks|uploaders|validators|views|workers|elastic_integration)})
end
end
diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb
index c32bef5a1a578..556a0d605d59e 100644
--- a/spec/services/issues/update_service_spec.rb
+++ b/spec/services/issues/update_service_spec.rb
@@ -842,5 +842,33 @@ def update_issue(opts)
let(:open_issuable) { issue }
let(:closed_issuable) { create(:closed_issue, project: project) }
end
+
+ context 'real-time updates' do
+ let(:update_params) { { assignee_ids: [user2.id] } }
+
+ context 'when broadcast_issue_updates is enabled' do
+ before do
+ stub_feature_flags(broadcast_issue_updates: true)
+ end
+
+ it 'broadcasts to the issues channel' do
+ expect(IssuesChannel).to receive(:broadcast_to).with(issue, event: 'updated')
+
+ update_issue(update_params)
+ end
+ end
+
+ context 'when broadcast_issue_updates is disabled' do
+ before do
+ stub_feature_flags(broadcast_issue_updates: false)
+ end
+
+ it 'does not broadcast to the issues channel' do
+ expect(IssuesChannel).not_to receive(:broadcast_to)
+
+ update_issue(update_params)
+ end
+ end
+ end
end
end
--
GitLab