From 843a7196a99d984ed8703ae6366721eebe5d5ccc Mon Sep 17 00:00:00 2001 From: Vidit Date: Wed, 21 Aug 2019 05:34:06 +0530 Subject: [PATCH] action cable setup (#805) * WIP action cable setup * basic action cable setup complete * minor change * minor changes * few changes * initial working functionality complete * Refactoring code * Adding Foreman gem * Scheduling Puma and Passenger servers * WIP action cable setup * basic action cable setup complete * minor change * minor changes * few changes * initial working functionality complete * Refactoring code * Adding Foreman gem * Scheduling Puma and Passenger servers * few minor fix * added a few tests * Refactoring connection module * Using strong params in requests * added documentation * added more docs * added tests * Using puma as dependency and correct image controller * added a few tests * a few changes * remove unnecessary render * few test fixes --- Gemfile | 8 ++- Gemfile.lock | 15 +++-- Procfile | 2 + SYNCHRONOUS_EDITING.md | 30 +++++++++ app/assets/javascripts/application.js | 67 ++++++++++--------- app/assets/javascripts/cable.js | 11 +++ .../channels/concurrent_editing.js | 26 +++++++ app/assets/javascripts/mapknitter/Map.js | 29 ++++++++ app/channels/application_cable/channel.rb | 4 ++ app/channels/application_cable/connection.rb | 17 +++++ app/channels/concurrent_editing_channel.rb | 17 +++++ app/controllers/application_controller.rb | 4 +- app/controllers/images_controller.rb | 6 +- app/models/map.rb | 6 ++ app/views/layouts/application.html.erb | 1 + config/environments/development.rb | 3 + config/environments/production.rb | 2 + config/puma.rb | 5 ++ config/routes.rb | 2 + start.sh | 2 +- .../concurrent_editing_channel_test.rb | 23 +++++++ test/channels/connection_test.rb | 31 +++++++++ test/fixtures/maps.yml | 4 +- test/fixtures/nodes.yml | 13 ++++ test/system/synchronous_editing_test.rb | 16 +++++ 25 files changed, 299 insertions(+), 45 deletions(-) create mode 100644 Procfile create mode 100644 SYNCHRONOUS_EDITING.md create mode 100644 app/assets/javascripts/cable.js create mode 100644 app/assets/javascripts/channels/concurrent_editing.js create mode 100644 app/channels/application_cable/channel.rb create mode 100644 app/channels/application_cable/connection.rb create mode 100644 app/channels/concurrent_editing_channel.rb create mode 100644 config/puma.rb create mode 100644 test/channels/concurrent_editing_channel_test.rb create mode 100644 test/channels/connection_test.rb create mode 100644 test/system/synchronous_editing_test.rb diff --git a/Gemfile b/Gemfile index 61ac297e1..8882cdb05 100644 --- a/Gemfile +++ b/Gemfile @@ -26,6 +26,7 @@ group :dependencies do gem 'bootsnap', '~> 1.4.4' gem 'turbolinks', '~> 5' gem 'mini_magick', '~> 4.8' + gem 'puma', '~> 4.1.0' # if you use amazon s3 for warpable image storage gem 'aws-sdk', '~> 1.5.7' @@ -37,12 +38,15 @@ group :dependencies do # compiling markdown to html gem 'rdiscount', '2.2.0.1' + # Process manager for applications with multiple components + gem 'foreman', '~> 0.85.0' + # asset pipelining gem 'bootstrap-sass' gem 'sassc-rails' gem 'jquery-rails' gem 'sprockets', '3.7.2' - gem "sprockets-rails" + gem 'sprockets-rails' gem 'sass', require: 'sass' gem 'autoprefixer-rails', '~> 9.5.1.1' gem 'uglifier', '~> 4.1.20' @@ -65,11 +69,11 @@ end group :development, :test do gem 'capybara' - gem 'puma' gem 'selenium-webdriver' gem 'byebug', '~> 11.0.1', platforms: [:mri, :mingw, :x64_mingw] gem 'faker', '~> 2.1.2' gem 'pry-rails', '~> 0.3.9' + gem 'action-cable-testing' end group :development do diff --git a/Gemfile.lock b/Gemfile.lock index 0f1ffa1d4..3565f59f6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -4,6 +4,8 @@ GEM RubyInline (3.12.4) ZenTest (~> 4.3) ZenTest (4.11.2) + action-cable-testing (0.6.0) + actioncable (>= 5.0) actioncable (5.2.3) actionpack (= 5.2.3) nio4r (~> 2.0) @@ -89,6 +91,8 @@ GEM faker (2.1.2) i18n (>= 0.8) ffi (1.11.1) + foreman (0.85.0) + thor (~> 0.19.1) friendly_id (5.2.5) activerecord (>= 4.0.0) geokit (1.13.1) @@ -210,7 +214,7 @@ GEM rails-dom-testing (2.0.3) activesupport (>= 4.2.0) nokogiri (>= 1.6) - rails-html-sanitizer (1.1.0) + rails-html-sanitizer (1.2.0) loofah (~> 2.2, >= 2.2.2) rails-perftest (0.0.7) railties (5.2.3) @@ -261,9 +265,8 @@ GEM sass-listen (4.0.0) rb-fsevent (~> 0.9, >= 0.9.4) rb-inotify (~> 0.9, >= 0.9.7) - sassc (2.0.1) + sassc (2.1.0) ffi (~> 1.9) - rake sassc-rails (2.1.2) railties (>= 4.0.0) sassc (>= 2.0) @@ -296,7 +299,7 @@ GEM sqlite3 (1.4.1) terrapin (0.6.0) climate_control (>= 0.0.3, < 1.0) - thor (0.20.3) + thor (0.19.4) thread_safe (0.3.6) tilt (2.0.9) turbolinks (5.2.0) @@ -330,6 +333,7 @@ PLATFORMS DEPENDENCIES RubyInline (~> 3.12.4) + action-cable-testing autoprefixer-rails (~> 9.5.1.1) aws-sdk (~> 1.5.7) bootsnap (~> 1.4.4) @@ -338,6 +342,7 @@ DEPENDENCIES capybara codecov faker (~> 2.1.2) + foreman (~> 0.85.0) friendly_id geokit-rails (= 1.1.4) httparty @@ -357,7 +362,7 @@ DEPENDENCIES passenger popper_js (~> 1.11, >= 1.11.1) pry-rails (~> 0.3.9) - puma + puma (~> 4.1.0) rack_session_access rails (~> 5.2.3) rails-controller-testing diff --git a/Procfile b/Procfile new file mode 100644 index 000000000..841da4133 --- /dev/null +++ b/Procfile @@ -0,0 +1,2 @@ +passenger: passenger start +puma: puma -C config/puma.rb diff --git a/SYNCHRONOUS_EDITING.md b/SYNCHRONOUS_EDITING.md new file mode 100644 index 000000000..66819275f --- /dev/null +++ b/SYNCHRONOUS_EDITING.md @@ -0,0 +1,30 @@ +The new synchronous editing feature +=================================== + +With the introduction of ActionCable to our system, it has been possible +to do perform real-time tasks quite easily. We have used rail's default +action cable to make a _concurrent_editing_channel.rb_ in the _app/channels_ folder, +to handle all the incoming requests and consists of all the business +logic as well. At the frontend we have, _app/javascripts/channels/concurrent_editing.js_ which +handles the logic at the browser or the frontend. + +## Flow of the feature: + +1. When the map is updated, the _speak_ method of _concurrent_editing.js_ is called which requests +the _sync_ method of _concurrent_editing_channel.rb_ to broadcast the updated data to +the connected users. + +2. The broadcasted data is finally caught by the _received_ function of _app/javascripts/channels/concurrent_editing.js_ + +3. Finally the _received_ function calls the _synchronizeData_ function to update + all the fresh data on the map. + + +## Testing: + +1. The _action-cable-testing_ gem is used for the feature's testing. It has some really +cool testing functionality which was required for our use case. + +2. Currently we have separate tests written for connection related features and channel +specific features. The relevant files are test/channels/concurrent_editing_channel_test.rb and +test/channels/connection_test.rb \ No newline at end of file diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index dc9abfe1c..d248e6b45 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -12,41 +12,42 @@ // -//= require leaflet/dist/leaflet-src.js +// = require leaflet/dist/leaflet-src.js -//= require jquery -//= require jquery-ujs -//= require jquery/dist/jquery.js -//= require jquery-ujs/src/rails.js -//= require jquery-ui/jquery-ui.min.js +// = require jquery +// = require jquery-ujs +// = require jquery/dist/jquery.js +// = require jquery-ujs/src/rails.js +// = require jquery-ui/jquery-ui.min.js -//= require blueimp-tmpl/js/tmpl.js -//= require blueimp-file-upload/js/vendor/jquery.ui.widget -//= require blueimp-file-upload/js/jquery.fileupload -//= require blueimp-file-upload/js/jquery.fileupload-process -//= require blueimp-file-upload/js/jquery.fileupload-ui +// = require blueimp-tmpl/js/tmpl.js +// = require blueimp-file-upload/js/vendor/jquery.ui.widget +// = require blueimp-file-upload/js/jquery.fileupload +// = require blueimp-file-upload/js/jquery.fileupload-process +// = require blueimp-file-upload/js/jquery.fileupload-ui -//= require bootstrap/dist/js/bootstrap.js +// = require bootstrap/dist/js/bootstrap.js -//= require leaflet-fullHash.js -//= require leaflet-providers/leaflet-providers.js -//= require leaflet-environmental-layers/dist/LeafletEnvironmentalLayers.js -//= require leaflet-environmental-layers/src/windRoseLayer.js -//= require leaflet-easybutton/src/easy-button.js -//= require sparklines/source/sparkline.js -//= require glfx-js/dist/glfx.js -//= require ion-rangeslider/js/ion.rangeSlider.js -//= require exif-js/exif.js -//= require webgl-distort/dist/webgl-distort.js -//= require mapknitter/core/Class.js -//= require leaflet-spin/example/spin/dist/spin.min.js -//= require leaflet-spin/example/leaflet.spin.min.js -//= require image-sequencer/dist/image-sequencer.js -//= require leaflet-toolbar/dist/leaflet.toolbar.js -//= require leaflet-draw/dist/leaflet.draw-src.js -//= require leaflet-distortableimage/dist/leaflet.distortableimage.js -//= require leaflet-illustrate/dist/Leaflet.Illustrate.js -//= require leaflet-distortableimage/src/edit/tools/EditAction.js -//= require mapknitter/Map.js +// = require leaflet-fullHash.js +// = require leaflet-providers/leaflet-providers.js +// = require leaflet-environmental-layers/dist/LeafletEnvironmentalLayers.js +// = require leaflet-environmental-layers/src/windRoseLayer.js +// = require leaflet-easybutton/src/easy-button.js +// = require sparklines/source/sparkline.js +// = require glfx-js/dist/glfx.js +// = require ion-rangeslider/js/ion.rangeSlider.js +// = require exif-js/exif.js +// = require webgl-distort/dist/webgl-distort.js +// = require mapknitter/core/Class.js +// = require leaflet-spin/example/spin/dist/spin.min.js +// = require leaflet-spin/example/leaflet.spin.min.js +// = require image-sequencer/dist/image-sequencer.js +// = require leaflet-toolbar/dist/leaflet.toolbar.js +// = require leaflet-draw/dist/leaflet.draw-src.js +// = require leaflet-illustrate/dist/Leaflet.Illustrate.js +// = require leaflet-distortableimage/dist/leaflet.distortableimage.js +// = require leaflet-distortableimage/src/edit/tools/EditAction.js +// = require mapknitter/Map.js +// = require cable.js -//= require_tree . +// = require_tree . diff --git a/app/assets/javascripts/cable.js b/app/assets/javascripts/cable.js new file mode 100644 index 000000000..f613e17f5 --- /dev/null +++ b/app/assets/javascripts/cable.js @@ -0,0 +1,11 @@ +// +//= require action_cable +//= require_self +//= require_tree ./channels + +(function() { + this.App || (this.App = {}); + + App.cable = ActionCable.createConsumer(); + +}).call(this); diff --git a/app/assets/javascripts/channels/concurrent_editing.js b/app/assets/javascripts/channels/concurrent_editing.js new file mode 100644 index 000000000..f93e01e45 --- /dev/null +++ b/app/assets/javascripts/channels/concurrent_editing.js @@ -0,0 +1,26 @@ +/* Handles all the frontend interactions with action cable and the server. */ + +App.concurrent_editing = App.cable.subscriptions.create("ConcurrentEditingChannel", { + connected: function() { + // Called when the subscription is ready for use on the server + }, + + disconnected: function() { + // Called when the subscription has been terminated by the server + }, + + received: function(data) { + // Called when there's incoming data on the websocket for this channel + window.mapKnitter.synchronizeData(data.changes); + }, + + speak: function(changes) { + /* Called when an image is updated from Map.js ('saveImage' function). + * This function calls concurrent_editing_channel.rb's 'sync' method + * which is responsible for broadcasting the updated warpables + * to all the user's connected to the concurrent_editing channel. */ + return this.perform("sync", { + changes: changes + }); + } +}); diff --git a/app/assets/javascripts/mapknitter/Map.js b/app/assets/javascripts/mapknitter/Map.js index 0a43a103f..136d1755f 100644 --- a/app/assets/javascripts/mapknitter/Map.js +++ b/app/assets/javascripts/mapknitter/Map.js @@ -386,6 +386,32 @@ MapKnitter.Map = MapKnitter.Class.extend({ if (this.editing._mode !== "lock") { e.stopPropagation(); } }, + /* Called by the concurrent_editing.js channel's 'received' function (app/assets/javascripts/channels/concurrent_editing.js). + * It recieves a list of updated warpables,i.e. list of images with updated corner points. The aim of writing this function + * is to reposition the updated images onto the map on every connected browser (via the ActionCable). */ + + synchronizeData: function(warpables) { + var layers = []; + map.eachLayer(function(l) {layers.push(l)}); + layers = layers.filter(image => (image._url!=undefined || image._url!=null)); + warpables.forEach(function(warpable) { + corners = []; + warpable.nodes.forEach(function(node) { + corners.push(L.latLng(node.lat, node.lon)); + }); + + x = corners[2]; + y = corners [3]; + corners [2] = y; + corners [3] = x; + + console.log(corners); + + layer = layers.filter(l => l._url==warpable.srcmedium)[0]; + layer.setCorners(corners); + }); + }, + saveImageIfChanged: function () { var img = this, edit = img.editing; @@ -424,6 +450,9 @@ MapKnitter.Map = MapKnitter.Class.extend({ beforeSend: function (e) { $('.mk-save').removeClass('fa-check-circle fa-times-circle fa-green fa-red').addClass('fa-spinner fa-spin') }, + success: function(data) { + App.concurrent_editing.speak(data); + }, complete: function (e) { $('.mk-save').removeClass('fa-spinner fa-spin').addClass('fa-check-circle fa-green') }, diff --git a/app/channels/application_cable/channel.rb b/app/channels/application_cable/channel.rb new file mode 100644 index 000000000..d67269728 --- /dev/null +++ b/app/channels/application_cable/channel.rb @@ -0,0 +1,4 @@ +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 000000000..a76243b26 --- /dev/null +++ b/app/channels/application_cable/connection.rb @@ -0,0 +1,17 @@ +module ApplicationCable + class Connection < ActionCable::Connection::Base + identified_by :current_user + + def connect + self.current_user = find_verified_user + end + + private + + def find_verified_user + User.find(cookies.signed["user_id"]) + rescue ActiveRecord::RecordNotFound + reject_unauthorized_connection + end + end +end diff --git a/app/channels/concurrent_editing_channel.rb b/app/channels/concurrent_editing_channel.rb new file mode 100644 index 000000000..79991c3b1 --- /dev/null +++ b/app/channels/concurrent_editing_channel.rb @@ -0,0 +1,17 @@ +class ConcurrentEditingChannel < ApplicationCable::Channel + # This class handles the server side logic of the actioncable communication. + + def subscribed + # Called first to connect user to the channel. + stream_from "concurrent_editing_channel" + end + + def unsubscribed + # Any cleanup needed when channel is unsubscribed + end + + def sync(changes) + # Responsible for broadcasting the updated warpables or simply images to the user's connected on this channel. + ActionCable.server.broadcast 'concurrent_editing_channel', changes + end +end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 6760cf9a4..782efc642 100755 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -20,7 +20,9 @@ def current_user user_id = session[:user_id] if user_id begin - @user = User.find(user_id) + u = User.find(user_id) + cookies.signed["user_id"] = u.id + @user = u rescue StandardError @user = nil end diff --git a/app/controllers/images_controller.rb b/app/controllers/images_controller.rb index 6ef5d486b..cf773b22b 100644 --- a/app/controllers/images_controller.rb +++ b/app/controllers/images_controller.rb @@ -92,7 +92,11 @@ def update @warpable.locked = params[:locked] @warpable.cm_per_pixel = @warpable.get_cm_per_pixel @warpable.save - render html: 'success' + + respond_to do |format| + format.html { render html: 'success' } + format.json { render json: @warpable.map.fetch_map_data } + end else render plain: 'You must be logged in to update the image, unless the map is anonymous.' end diff --git a/app/models/map.rb b/app/models/map.rb index 7d65e3ecb..ab001ff16 100755 --- a/app/models/map.rb +++ b/app/models/map.rb @@ -273,4 +273,10 @@ def add_tag(tagname, user) tagname = tagname.downcase tags.create(name: tagname, user_id: user.id, map_id: id) unless has_tag(tagname) end + + def fetch_map_data + # fetches a list of updated warpables along with their corners in a json format. + data = warpables + data.to_json + end end diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 85bdb4cdf..2f5c573b9 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -13,6 +13,7 @@ <%= stylesheet_link_tag 'application' %> + <%= action_cable_meta_tag %> <%= javascript_include_tag 'application' %> diff --git a/config/environments/development.rb b/config/environments/development.rb index d4ca60dce..2e870ab0e 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -74,4 +74,7 @@ # Use an evented file watcher to asynchronously detect changes in source code, # routes, locales, etc. This feature depends on the listen gem. config.file_watcher = ActiveSupport::EventedFileUpdateChecker + + config.action_cable.url = "ws://localhost:3000/cable" + config.action_cable.allowed_request_origins = [/http:\/\/*/, /https:\/\/*/] end diff --git a/config/environments/production.rb b/config/environments/production.rb index 91ef1928a..7bed767d6 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -103,4 +103,6 @@ # Do not dump schema after migrations. config.active_record.dump_schema_after_migration = false + + config.action_cable.allowed_request_origins = [/http:\/\/*/, /https:\/\/*/] end diff --git a/config/puma.rb b/config/puma.rb new file mode 100644 index 000000000..a6860886a --- /dev/null +++ b/config/puma.rb @@ -0,0 +1,5 @@ +#!/usr/bin/env puma + +environment ENV.fetch("RAILS_ENV") { "production" } + +pidfile '/app/tmp/pids/puma.pid' \ No newline at end of file diff --git a/config/routes.rb b/config/routes.rb index de570b36f..26bafb420 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -2,6 +2,8 @@ root to: 'front_ui#index' + mount ActionCable.server => '/cable' + get 'front-page', to: 'front_ui#index' get 'mappers', to: 'front_ui#nearby_mappers' get 'about', to: 'front_ui#about' diff --git a/start.sh b/start.sh index 517a983d1..674e2b563 100755 --- a/start.sh +++ b/start.sh @@ -24,4 +24,4 @@ if [ -f $pidfile ] ; then rm $pidfile; fi -bundle exec passenger start +bundle exec foreman start diff --git a/test/channels/concurrent_editing_channel_test.rb b/test/channels/concurrent_editing_channel_test.rb new file mode 100644 index 000000000..b458255e3 --- /dev/null +++ b/test/channels/concurrent_editing_channel_test.rb @@ -0,0 +1,23 @@ +require 'test_helper' + +module ApplicationCable + class ConcurrentEditingChannelTest < ActionCable::Channel::TestCase + + def test_synch_editing_broadcast_count + channel_name = "concurrent_editing_channel" + assert_broadcasts channel_name, 0 + ActionCable.server.broadcast channel_name, data: {} + assert_broadcasts channel_name, 1 + end + + + def test_synch_editing_broadcast_message + channel_name = "concurrent_editing_channel" + changes = { :image_change => "test" } + ActionCable.server.broadcast channel_name, data: changes + assert_broadcast_on(channel_name, data: changes) do + ActionCable.server.broadcast channel_name, data: changes + end + end + end +end diff --git a/test/channels/connection_test.rb b/test/channels/connection_test.rb new file mode 100644 index 000000000..309565779 --- /dev/null +++ b/test/channels/connection_test.rb @@ -0,0 +1,31 @@ +require 'test_helper' + +module ApplicationCable + class ConnectionTest < ActionCable::Connection::TestCase + def test_connection_with_user + cookies.signed["user_id"] = users(:chris) + + #this simulates the connection + connect + + # assert connected user + assert_equal "chris", connection.current_user.login + end + + def test_does_not_connect_without_user + + # user not logged in + begin + # trying to connect but fails + connect + rescue Exception=>e + + #compare the error class + assert_equal e.class, ActionCable::Connection::Authorization::UnauthorizedError + end + + #check is connection is nil(which it should be) + assert_equal nil, connection + end + end +end diff --git a/test/fixtures/maps.yml b/test/fixtures/maps.yml index 2e9c1f20d..76dab52c6 100644 --- a/test/fixtures/maps.yml +++ b/test/fixtures/maps.yml @@ -34,7 +34,7 @@ nairobi: lat: -1.2920659 lon: 36.8219462 description: Capital of Kenya - created_at: <% Time.now %> + created_at: <%= Time.now %> license: publicdomain user_id: 2 author: aaron @@ -47,7 +47,7 @@ village: lat: -0.023559 lon: 37.90619300000003 description: A mall in Nairobi - created_at: <% Time.now %> + created_at: <%= Time.now %> license: publicdomain user_id: 2 author: aaron diff --git a/test/fixtures/nodes.yml b/test/fixtures/nodes.yml index 3759b4cee..d16cde45c 100644 --- a/test/fixtures/nodes.yml +++ b/test/fixtures/nodes.yml @@ -56,3 +56,16 @@ four: way_order: 0 body: nil +five: + id: 5 + color: "black" + author: "anonymous" + lat: 42.8377535388083 + lon: -75.3981708900972 + way_id: 0 + order: 0 + name: "" + description: "" + map_id: 1 + way_order: 0 + body: nil diff --git a/test/system/synchronous_editing_test.rb b/test/system/synchronous_editing_test.rb new file mode 100644 index 000000000..2dde3353a --- /dev/null +++ b/test/system/synchronous_editing_test.rb @@ -0,0 +1,16 @@ +require 'application_system_test_case' + +class SynchronousTest < ApplicationSystemTestCase + setup do + Capybara.current_driver = Capybara.javascript_driver + Capybara.asset_host = "http://localhost:3000" + end + + test 'warpables change flow' do + map = maps(:saugus) + original_data = map.fetch_map_data + map.warpables.first.update_column(:nodes, "2,5,1,3") + updated_data = map.fetch_map_data + assert_not_equal updated_data, original_data + end +end \ No newline at end of file