From 88ae24b7e8f7d61bfca02e72dd4a7a3a3f14beb6 Mon Sep 17 00:00:00 2001 From: Max Melentiev Date: Tue, 16 Jan 2018 10:56:06 +0300 Subject: [PATCH] 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