From 3a876a42df143a38e7869290d5900b133f58a496 Mon Sep 17 00:00:00 2001 From: Max Melentiev Date: Tue, 16 Jan 2018 10:54:37 +0300 Subject: [PATCH 1/5] Setup codeclimate --- .codeclimate.yml | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .codeclimate.yml diff --git a/.codeclimate.yml b/.codeclimate.yml new file mode 100644 index 0000000..d96d7de --- /dev/null +++ b/.codeclimate.yml @@ -0,0 +1,3 @@ +plugins: + rubocop: + enabled: true From 88ae24b7e8f7d61bfca02e72dd4a7a3a3f14beb6 Mon Sep 17 00:00:00 2001 From: Max Melentiev Date: Tue, 16 Jan 2018 10:56:06 +0300 Subject: [PATCH 2/5] Fix codeclimate issues --- .codeclimate.yml | 5 +++ lib/telegram/bot.rb | 6 +++- lib/telegram/bot/client.rb | 28 ++++++++-------- lib/telegram/bot/config_methods.rb | 4 +-- lib/telegram/bot/routes_helper.rb | 24 +++++++++----- lib/telegram/bot/updates_controller.rb | 21 ++++++------ lib/telegram/bot/updates_poller.rb | 34 +++++++++++-------- spec/integration_helper.rb | 4 +-- spec/spec_helper.rb | 2 +- spec/telegram/bot/updates_poller_spec.rb | 42 +++++++++++++++++------- 10 files changed, 108 insertions(+), 62 deletions(-) diff --git a/.codeclimate.yml b/.codeclimate.yml index d96d7de..25fc0fd 100644 --- a/.codeclimate.yml +++ b/.codeclimate.yml @@ -1,3 +1,8 @@ +checks: + method-complexity: + config: + threshold: 6 # should be just fine + plugins: rubocop: enabled: true diff --git a/lib/telegram/bot.rb b/lib/telegram/bot.rb index ada5111..85e2584 100644 --- a/lib/telegram/bot.rb +++ b/lib/telegram/bot.rb @@ -5,9 +5,13 @@ module Telegram module Bot class Error < StandardError; end - class NotFound < Error; end + + # Raised for valid telegram response with 403 status code. class Forbidden < Error; end + # Raised for valid telegram response with 404 status code. + class NotFound < Error; end + autoload :Async, 'telegram/bot/async' autoload :Botan, 'telegram/bot/botan' autoload :Client, 'telegram/bot/client' diff --git a/lib/telegram/bot/client.rb b/lib/telegram/bot/client.rb index b04032e..953d53d 100644 --- a/lib/telegram/bot/client.rb +++ b/lib/telegram/bot/client.rb @@ -36,6 +36,18 @@ module Telegram def prepare_async_args(action, body = {}) [action.to_s, Async.prepare_hash(prepare_body(body))] end + + def error_for_response(response) + result = JSON.parse(response.body) rescue nil # rubocop:disable RescueModifier + return Error.new(response.reason) unless result + message = result['description'] || '-' + # This errors are raised only for valid responses from Telegram + case response.status + when 403 then Forbidden.new(message) + when 404 then NotFound.new(message) + else Error.new("#{response.reason}: #{message}") + end + end end attr_reader :client, :token, :username, :base_uri @@ -48,19 +60,9 @@ module Telegram end def request(action, body = {}) - res = http_request("#{base_uri}#{action}", self.class.prepare_body(body)) - status = res.status - return JSON.parse(res.body) if status < 300 - result = JSON.parse(res.body) rescue nil # rubocop:disable RescueModifier - err_msg = result && result['description'] || '-' - if result - # This errors are raised only for valid responses from Telegram - case status - when 403 then raise Forbidden, err_msg - when 404 then raise NotFound, err_msg - end - end - raise Error, "#{res.reason}: #{err_msg}" + response = http_request("#{base_uri}#{action}", self.class.prepare_body(body)) + raise self.class.error_for_response(response) if response.status >= 300 + JSON.parse(response.body) end # Endpoint for low-level request. For easy host highjacking & instrumentation. diff --git a/lib/telegram/bot/config_methods.rb b/lib/telegram/bot/config_methods.rb index 459534b..611de4f 100644 --- a/lib/telegram/bot/config_methods.rb +++ b/lib/telegram/bot/config_methods.rb @@ -54,8 +54,8 @@ module Telegram @bots_config ||= if defined?(Rails.application) app = Rails.application - secrets = (app.respond_to?(:credentials) ? app.credentials : app.secrets). - fetch(:telegram, {}).with_indifferent_access + store = app.respond_to?(:credentials) ? app.credentials : app.secrets + secrets = store.fetch(:telegram, {}).with_indifferent_access secrets.fetch(:bots, {}).symbolize_keys.tap do |config| default = secrets[:bot] config[:default] = default if default diff --git a/lib/telegram/bot/routes_helper.rb b/lib/telegram/bot/routes_helper.rb index 13f951a..28aa868 100644 --- a/lib/telegram/bot/routes_helper.rb +++ b/lib/telegram/bot/routes_helper.rb @@ -45,23 +45,31 @@ module Telegram # other_bot => TelegramAuctionController, # admin_chat: TelegramAdminChatController # + # TODO: Deprecate it in favor of telegram_webhook. def telegram_webhooks(controllers, bots = nil, **options) unless controllers.is_a?(Hash) bots = bots ? Array.wrap(bots) : Telegram.bots.values controllers = Hash[bots.map { |x| [x, controllers] }] end controllers.each do |bot, controller| - bot = Client.wrap(bot) controller, bot_options = controller if controller.is_a?(Array) - params = { - to: Middleware.new(bot, controller), - as: RoutesHelper.route_name_for_bot(bot), - format: false, - }.merge!(options).merge!(bot_options || {}) - post("telegram/#{RoutesHelper.escape_token bot.token}", params) - UpdatesPoller.add(bot, controller) if Telegram.bot_poller_mode? + telegram_webhook(controller, bot, options.merge(bot_options || {})) end end + + # Define route which processes requests using given controller and bot. + # + # See telegram_webhooks for examples. + def telegram_webhook(controller, bot, **options) + bot = Client.wrap(bot) + params = { + to: Middleware.new(bot, controller), + as: RoutesHelper.route_name_for_bot(bot), + format: false, + }.merge!(options) + post("telegram/#{RoutesHelper.escape_token bot.token}", params) + UpdatesPoller.add(bot, controller) if Telegram.bot_poller_mode? + end end end end diff --git a/lib/telegram/bot/updates_controller.rb b/lib/telegram/bot/updates_controller.rb index 96f4403..453d6b8 100644 --- a/lib/telegram/bot/updates_controller.rb +++ b/lib/telegram/bot/updates_controller.rb @@ -120,10 +120,17 @@ module Telegram # any commands. def command_from_text(text, username = nil) return unless text - match = text.match CMD_REGEX + match = text.match(CMD_REGEX) return unless match - return if match[3] && username != true && match[3] != username - [match[1], text.split.drop(1)] + mention = match[3] + [match[1], text.split.drop(1)] if username == true || !mention || mention == username + end + + def payload_from_update(update) + update && PAYLOAD_TYPES.find do |type| + item = update[type] + return [item, type] if item + end end end @@ -142,13 +149,7 @@ module Telegram @_update = update @_bot = bot @_chat, @_from = options && options.values_at(:chat, :from) - - payload_data = nil - update && PAYLOAD_TYPES.find do |type| - item = update[type] - payload_data = [item, type] if item - end - @_payload, @_payload_type = payload_data + @_payload, @_payload_type = self.class.payload_from_update(update) end # Accessor to `'chat'` field of payload. Also tries `'chat'` in `'message'` diff --git a/lib/telegram/bot/updates_poller.rb b/lib/telegram/bot/updates_poller.rb index 5980c0f..c5810aa 100644 --- a/lib/telegram/bot/updates_poller.rb +++ b/lib/telegram/bot/updates_poller.rb @@ -45,36 +45,44 @@ module Telegram log { 'Started bot poller.' } while running begin - fetch_updates do |update| - controller.dispatch(bot, update) - end + updates = fetch_updates + process_updates(updates) if updates && updates.any? rescue Interrupt @running = false - rescue StandardError => e - logger.error { ([e.message] + e.backtrace).join("\n") } if logger end end - log { 'Stop polling bot updates.' } + log { 'Stoped polling bot updates.' } end + # Method to stop poller from other thread. def stop return unless running - log { 'Killing polling thread.' } + log { 'Stopping polling bot updates.' } @running = false end - def fetch_updates + def fetch_updates(offset = self.offset) response = bot.async(false) { bot.get_updates(offset: offset, timeout: timeout) } - updates = response.is_a?(Array) ? response : response['result'] - return unless updates && updates.any? + response.is_a?(Array) ? response : response['result'] + rescue Timeout::Error + log { 'Fetch timeout' } + nil + end + + def process_updates(updates) reload! do updates.each do |update| @offset = update['update_id'] + 1 - yield update + process_update(update) end end - rescue Timeout::Error - log { 'Fetch timeout' } + rescue StandardError => e + logger.error { ([e.message] + e.backtrace).join("\n") } if logger + end + + # Override this method to setup custom error collector. + def process_update(update) + controller.dispatch(bot, update) end def reload! diff --git a/spec/integration_helper.rb b/spec/integration_helper.rb index b78bc13..59f51bd 100644 --- a/spec/integration_helper.rb +++ b/spec/integration_helper.rb @@ -24,8 +24,8 @@ class TestApplication < Rails::Application }, } - if Rails.application.respond_to?(:credentials) - Rails.application.credentials.config[:telegram] = telegram_config + if respond_to?(:credentials) + credentials.config[:telegram] = telegram_config else secrets[:secret_key_base] = 'test' secrets[:telegram] = telegram_config diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index c1eef90..54ce8e2 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -16,7 +16,7 @@ $LOAD_PATH.unshift GEM_ROOT.join('lib') require 'telegram/bot' require 'telegram/bot/updates_controller/rspec_helpers' require 'telegram/bot/types' -require 'active_support/core_ext/object/json' +require 'active_support/json' Dir[GEM_ROOT.join('spec/support/**/*.rb')].each { |f| require f } diff --git a/spec/telegram/bot/updates_poller_spec.rb b/spec/telegram/bot/updates_poller_spec.rb index 2346bf9..1a13bbb 100644 --- a/spec/telegram/bot/updates_poller_spec.rb +++ b/spec/telegram/bot/updates_poller_spec.rb @@ -8,12 +8,32 @@ RSpec.describe Telegram::Bot::UpdatesPoller do it { should be } end - describe '#fetch_updates' do - subject { -> { instance.fetch_updates(&block) } } + describe '#process_updates' do + subject { -> { instance.process_updates(updates) } } let(:block) { ->(x) { expect(x).to eq expected_results.shift } } - let(:results) { [{update_id: 12}, {update_id: 34}] } - let(:expected_results) { results.as_json } - let(:request_result) { {ok: true, result: results}.as_json } + let(:updates) { [{update_id: 12}, {update_id: 34}].as_json } + let(:processed_updates) { [] } + before do + allow(controller).to receive(:dispatch) do |bot, update| + expect(bot).to eq self.bot + processed_updates << update + end + end + + it { should change(instance, :offset).to(updates.last['update_id'] + 1) } + it { should change(self, :processed_updates).to(updates) } + + context 'with typed response' do + let(:updates) { super().map { |x| Telegram::Bot::Types::Update.new(x) } } + it { should change(instance, :offset).to(updates.last['update_id'] + 1) } + it { should change(self, :processed_updates).to(updates) } + end + end + + describe '#fetch_updates' do + subject { instance.fetch_updates } + let(:updates) { [{update_id: 12}, {update_id: 34}] } + let(:request_result) { {ok: true, result: updates}.as_json } before do allow(bot).to receive(:get_updates) do expect(bot.async).to be_falsy @@ -21,19 +41,17 @@ RSpec.describe Telegram::Bot::UpdatesPoller do end end - it { should change(instance, :offset).to(results.last[:update_id] + 1) } - it { should change { expected_results }.to([]) } + it { should eq updates.as_json } context 'with typed response' do - let(:request_result) { results.as_json.map { |x| Telegram::Bot::Types::Update.new(x) } } - let(:expected_results) { request_result.dup } - it { should change(instance, :offset).to(results.last[:update_id] + 1) } - it { should change { expected_results }.to([]) } + let(:updates) { super().map { |x| Telegram::Bot::Types::Update.new(x.as_json) } } + let(:request_result) { updates } + it { should eq updates } end context 'when bot is in async mode' do let(:bot) { Telegram::Bot::Client.new('token', async: Class.new) } - it { should change { expected_results }.to([]) } + it { should eq updates.as_json } end end end From 7f1f2bb884fbe84b3d75331048a053c27d43bc28 Mon Sep 17 00:00:00 2001 From: Max Melentiev Date: Tue, 16 Jan 2018 14:13:20 +0300 Subject: [PATCH 3/5] Fix readme --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 116872c..44df1b9 100644 --- a/README.md +++ b/README.md @@ -328,7 +328,7 @@ Callback queries without prefix stay untouched. # This one handles `set_value:%{something}`. def set_value_callback_query(new_value = nil, *) save_this(value) - answer_callback_query('Saved!) + answer_callback_query('Saved!') end # And this one is for `make_cool:%{something}` @@ -526,7 +526,7 @@ Yes, it's threadsafe too. ## Development -After checking out the repo, run `bin/setup` to install dependencies. +After checking out the repo, run `bin/setup` to install dependencies and git hooks. Then, run `appraisal rake spec` to run the tests. You can also run `bin/console` for an interactive prompt that will allow you to experiment. From 28db729320ad56b54ee6a47cd42f2d2f303db445 Mon Sep 17 00:00:00 2001 From: Max Melentiev Date: Thu, 18 Jan 2018 12:43:10 +0300 Subject: [PATCH 4/5] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b283ec..82fd4d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Unreleased - `rescue_from`. +- Support for `credentials` store in Rails 5.2. # 0.12.4 From a67a643c7b21f971f680fb24b083824fae75bbfa Mon Sep 17 00:00:00 2001 From: Max Melentiev Date: Thu, 18 Jan 2018 13:01:57 +0300 Subject: [PATCH 5/5] Build new rubies on travis --- .travis.yml | 14 ++++++++++---- telegram-bot.gemspec | 2 +- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/.travis.yml b/.travis.yml index a9e092b..6d466bc 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,11 +1,17 @@ language: ruby cache: bundler rvm: - - 2.2.3 + - 2.5 + - 2.4 + - 2.3 gemfile: - - gemfiles/rails_42.gemfile - - gemfiles/rails_50.gemfile - - gemfiles/rails_51.gemfile - gemfiles/rails_52.gemfile + - gemfiles/rails_51.gemfile + - gemfiles/rails_50.gemfile + - gemfiles/rails_42.gemfile notifications: email: false + +# for 2.5.0 until 2.5.1 is released: https://github.com/travis-ci/travis-ci/issues/8978 +before_install: + - gem update --system diff --git a/telegram-bot.gemspec b/telegram-bot.gemspec index 1c48534..b665ed9 100644 --- a/telegram-bot.gemspec +++ b/telegram-bot.gemspec @@ -23,6 +23,6 @@ Gem::Specification.new do |spec| spec.add_dependency 'activesupport', '>= 4.0', '< 6.0' spec.add_dependency 'httpclient', '~> 2.7' - spec.add_development_dependency 'bundler', '~> 1.11' + spec.add_development_dependency 'bundler', '~> 1.16' spec.add_development_dependency 'rake', '~> 10.0' end