From ef4f3caa5fa719522b5cb5ca083dcb8007530fae Mon Sep 17 00:00:00 2001 From: Max Melentiev Date: Fri, 6 Oct 2017 11:05:25 +0300 Subject: [PATCH 1/3] Make #session raise error when store is not configured Add more explanation on sessions in readme --- README.md | 39 ++++++++++++++---- .../bot/updates_controller/session.rb | 17 +++++--- .../bot/updates_controller/session_spec.rb | 41 +++++++++++++++++++ 3 files changed, 85 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 1d5e076..cd95aff 100644 --- a/README.md +++ b/README.md @@ -199,12 +199,33 @@ end #### Session -There is support for sessions using `ActiveSupport::Cache` stores. +This API is very close to ActiveController's session API, but works different +under the hood. Cookies can not be used to store session id or +whole session (like CookieStore does). So it uses key-value store and `session_key` +method to build identifier from update. + +Store can be one of numerous `ActiveSupport::Cache` stores. +While `:file_store` is suitable for development and single-server deployments +without heavy load, it doesn't scale well. Key-value databases with persistance +like Redis are more appropriate for production use. ```ruby -# configure store in env files: +# In rails app store can be configured in env files: config.telegram_updates_controller.session_store = :redis_store, {expires_in: 1.month} +# In other app it can be done for all controllers with: +Telegram::Bot::UpdatesController.session_store = :redis_store, {expires_in: 1.month} +# or for specific one: +OneOfUpdatesController.session_store = :redis_store, {expires_in: 1.month} +``` + +Default session id is made from bot's username and `(from || chat)['id']`. +It means that session will be the same for updates from user in every chat, +and different for every user in the same group chat. +To change this behavior you can override `session_key` method, or even +define multiple sessions in single controller. For details see `Session` module. + +```ruby class Telegram::WebhookController < Telegram::Bot::UpdatesController include Telegram::Bot::UpdatesController::Session # or just shortcut: @@ -222,12 +243,16 @@ class Telegram::WebhookController < Telegram::Bot::UpdatesController end private - # By default it uses bot's username and user's id as a session key. - # Chat's id is used only when `from` field is empty. - # Override `session_key` method to change this behavior. + + # In this case session will persist for user only in specific chat. + # Same user in other chat will have different session. def session_key - # In this case session will persist for user only in specific chat: - "#{bot.username}:#{chat['id']}:#{from['id']}" + "#{bot.username}:#{chat['id']}:#{from['id']}" if chat && from + end + + # This session will be the same for all updates in chat. + def chat_session + @_chat_session ||= self.class.build_session(chat && "#{bot.username}:#{chat['id']}") end end ``` diff --git a/lib/telegram/bot/updates_controller/session.rb b/lib/telegram/bot/updates_controller/session.rb index 3324fb8..c40313e 100644 --- a/lib/telegram/bot/updates_controller/session.rb +++ b/lib/telegram/bot/updates_controller/session.rb @@ -8,19 +8,26 @@ module Telegram module Session extend ActiveSupport::Concern + module ClassMethods + # Builds session with given key and optional store (default to session_store). + # This way it's easier to define multiple custom sessions, + # ex. one for group chat and one for user. + def build_session(key, store = session_store) + raise 'session_store is not configured' unless store + key ? SessionHash.new(store, key) : NullSessionHash.new + end + end + def process_action(*) super ensure - session.commit + session.commit if @_session end protected def session - @_session ||= begin - key = session_key - key ? SessionHash.new(self.class.session_store, key) : NullSessionHash.new - end + @_session ||= self.class.build_session(session_key) end def session_key diff --git a/spec/telegram/bot/updates_controller/session_spec.rb b/spec/telegram/bot/updates_controller/session_spec.rb index 83c61d2..103fd56 100644 --- a/spec/telegram/bot/updates_controller/session_spec.rb +++ b/spec/telegram/bot/updates_controller/session_spec.rb @@ -54,4 +54,45 @@ RSpec.describe Telegram::Bot::UpdatesController::Session do end end end + + describe '.build_session' do + subject { controller_class.build_session(key, *args) } + let(:key) {} + let(:args) { [] } + it { expect { subject }.to raise_error(/session_store is not configured/) } + + shared_examples 'NullSessionHash when key is not present' do |store_proc| + it { should be_instance_of(described_class::NullSessionHash) } + + context 'and key is present' do + let(:key) { :test_key } + it 'is valid SessionHash' do + expect(subject).to be_instance_of(described_class::SessionHash) + expect(subject.id).to eq key + expect(subject.instance_variable_get(:@store)).to be(instance_exec(&store_proc)) + end + end + end + + context 'when store configured' do + before { controller_class.session_store = nil } + include_examples 'NullSessionHash when key is not present', + -> { controller_class.session_store } + end + + context 'when store is given' do + let(:args) { [double(:store)] } + include_examples 'NullSessionHash when key is not present', -> { args[0] } + end + end + + describe '.session_store=' do + subject { ->(val) { controller_class.session_store = val } } + it 'casts to AS::Cache' do + expect { subject[:null_store] }.to change(controller_class, :session_store). + to(instance_of(ActiveSupport::Cache::NullStore)) + expect { subject[nil] }.to change(controller_class, :session_store). + to(instance_of(ActiveSupport::Cache::MemoryStore)) + end + end end From e529a79339dc21d2c0817d0f2907d9b9cdb017e9 Mon Sep 17 00:00:00 2001 From: Max Melentiev Date: Fri, 6 Oct 2017 11:07:11 +0300 Subject: [PATCH 2/3] Don't use Rails.cache as fallback for session_store It's NullStore in development by default so session does not work, and indeed should not be dependent. --- lib/telegram/bot/railtie.rb | 2 +- spec/integration/requests/default_bot_spec.rb | 5 +++++ spec/integration/requests/other_bot_spec.rb | 5 +++++ spec/integration_helper.rb | 14 ++++++++++++++ 4 files changed, 25 insertions(+), 1 deletion(-) diff --git a/lib/telegram/bot/railtie.rb b/lib/telegram/bot/railtie.rb index e8f6eaa..cc6fee4 100644 --- a/lib/telegram/bot/railtie.rb +++ b/lib/telegram/bot/railtie.rb @@ -18,7 +18,7 @@ module Telegram ActiveSupport.on_load('telegram.bot.updates_controller') do self.logger = options.logger || Rails.logger - self.session_store = options.session_store || Rails.cache + self.session_store = options.session_store if options.session_store end end diff --git a/spec/integration/requests/default_bot_spec.rb b/spec/integration/requests/default_bot_spec.rb index 5d6f40f..3791341 100644 --- a/spec/integration/requests/default_bot_spec.rb +++ b/spec/integration/requests/default_bot_spec.rb @@ -5,4 +5,9 @@ RSpec.describe DefaultBotController, :telegram_bot, type: :request do subject { -> { dispatch_command :start } } it { should respond_with_message 'from default' } end + + describe '#load_session' do + subject { -> { dispatch_command :load_session } } + it { should_not raise_error } + end end diff --git a/spec/integration/requests/other_bot_spec.rb b/spec/integration/requests/other_bot_spec.rb index 7139dc4..ddcdb90 100644 --- a/spec/integration/requests/other_bot_spec.rb +++ b/spec/integration/requests/other_bot_spec.rb @@ -6,4 +6,9 @@ RSpec.describe OtherBotController, :telegram_bot, type: :request do subject { -> { dispatch_command :start } } it { should respond_with_message 'from other' } end + + describe '#load_session' do + subject { -> { dispatch_command :load_session } } + it { should raise_error(/session_store is not configured/) } + end end diff --git a/spec/integration_helper.rb b/spec/integration_helper.rb index 9b23fe7..b625597 100644 --- a/spec/integration_helper.rb +++ b/spec/integration_helper.rb @@ -4,6 +4,7 @@ require 'action_dispatch' require 'action_dispatch/testing/integration' require 'rails' +require 'telegram/bot/railtie' require 'rspec/rails/adapters' require 'rspec/rails/fixture_support' require 'rspec/rails/example/rails_example_group' @@ -13,6 +14,7 @@ ENV['RAILS_ENV'] = 'test' class TestApplication < Rails::Application config.eager_load = false config.log_level = :debug + config.action_dispatch.show_exceptions = false secrets[:secret_key_base] = 'test' secrets[:telegram] = { bot: 'default_token', @@ -34,6 +36,18 @@ Rails.application.initialize! Object.const_set("#{bot_name}_bot_controller".camelize, controller) end +[DefaultBotController, OtherBotController].each do |klass| + klass.class_eval do + use_session! + + define_method :load_session do |*| + session[:test] + end + end +end + +DefaultBotController.session_store = :memory_store + RSpec.configure do |config| config.include RSpec::Rails::RequestExampleGroup, type: :request From 7e41bd4f9bc032f9e89e6dcd7a121556cae6fa2d Mon Sep 17 00:00:00 2001 From: Max Melentiev Date: Fri, 6 Oct 2017 11:10:05 +0300 Subject: [PATCH 3/3] Allow use different sessions for MessageContext --- .../bot/updates_controller/message_context.rb | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/lib/telegram/bot/updates_controller/message_context.rb b/lib/telegram/bot/updates_controller/message_context.rb index bc3fbbf..3186300 100644 --- a/lib/telegram/bot/updates_controller/message_context.rb +++ b/lib/telegram/bot/updates_controller/message_context.rb @@ -7,14 +7,11 @@ module Telegram include Session - included do - # As we use before_action context is cleared anyway, - # no matter we used it or not. - singleton_class.send :attr_reader, :context_handlers, :context_to_action - @context_handlers = {} - end - module ClassMethods + def context_handlers + @_context_handlers ||= {} + end + # Registers handler for context. # # context_handler :rename do |*| @@ -39,6 +36,8 @@ module Telegram context_handlers[context] = action || context end + attr_reader :context_to_action + # Use it to use context value as action name for all contexts # which miss handlers. # For security reasons it supports only action methods and will @@ -59,10 +58,16 @@ module Telegram # according to previous request. attr_reader :context + # Controller may have multiple sessions, let it be possible + # to select session for message context. + def message_context_session + session + end + # Fetches context and finds handler for it. If message has new command, # it has higher priority than contextual action. def action_for_message - val = session.delete(:context) + val = message_context_session.delete(:context) @context = val && val.to_sym super || context && begin handler = handler_for_context @@ -72,7 +77,7 @@ module Telegram # Save context for the next request. def save_context(context) - session[:context] = context + message_context_session[:context] = context end def handler_for_context