From fb748daf538e43efcf8884f017391bcbfccf2ea2 Mon Sep 17 00:00:00 2001 From: Thomas Balthazar Date: Wed, 10 Aug 2016 12:25:01 +0200 Subject: [PATCH] Replace the tinder gem by bare HTTP requests --- CHANGELOG | 1 + Gemfile | 3 - Gemfile.lock | 16 ----- .../project_services/campfire_service.rb | 51 +++++++++++++--- .../project_services/campfire/rooms.json | 22 +++++++ .../project_services/campfire/rooms2.json | 22 +++++++ .../project_services/campfire_service_spec.rb | 58 +++++++++++++++++++ 7 files changed, 147 insertions(+), 26 deletions(-) create mode 100644 spec/fixtures/project_services/campfire/rooms.json create mode 100644 spec/fixtures/project_services/campfire/rooms2.json diff --git a/CHANGELOG b/CHANGELOG index 7bfeff2a4ec..77e9889dd26 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.11.0 (unreleased) + - Remove the http_parser.rb dependency by removing the tinder gem. !5758 (tbalthazar) - Fix don't pass a local variable called `i` to a partial. !20510 (herminiotorres) - Fix rename `add_users_into_project` and `projects_ids`. !20512 (herminiotorres) - Fix the title of the toggle dropdown button. !5515 (herminiotorres) diff --git a/Gemfile b/Gemfile index 104929665e8..5c781cda748 100644 --- a/Gemfile +++ b/Gemfile @@ -163,9 +163,6 @@ gem 'redis-rails', '~> 4.0.0' gem 'redis', '~> 3.2' gem 'connection_pool', '~> 2.0' -# Campfire integration -gem 'tinder', '~> 1.10.0' - # HipChat integration gem 'hipchat', '~> 1.5.0' diff --git a/Gemfile.lock b/Gemfile.lock index 87f08d6f372..ff4c03ac61e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -335,7 +335,6 @@ GEM activesupport (>= 2) nokogiri (~> 1.4) htmlentities (4.3.4) - http_parser.rb (0.5.3) httparty (0.13.7) json (~> 1.8) multi_xml (>= 0.5.2) @@ -672,7 +671,6 @@ GEM redis-namespace (>= 1.5.2) rufus-scheduler (>= 2.0.24) sidekiq (>= 4.0.0) - simple_oauth (0.1.9) simplecov (0.12.0) docile (~> 1.1.0) json (>= 1.8, < 3) @@ -742,21 +740,8 @@ GEM tilt (2.0.5) timecop (0.8.1) timfel-krb5-auth (0.8.3) - tinder (1.10.1) - eventmachine (~> 1.0) - faraday (~> 0.9.0) - faraday_middleware (~> 0.9) - hashie (>= 1.0) - json (~> 1.8.0) - mime-types - multi_json (~> 1.7) - twitter-stream (~> 0.1) turbolinks (2.5.3) coffee-rails - twitter-stream (0.1.16) - eventmachine (>= 0.12.8) - http_parser.rb (~> 0.5.1) - simple_oauth (~> 0.1.4) tzinfo (1.2.2) thread_safe (~> 0.1) u2f (0.2.1) @@ -981,7 +966,6 @@ DEPENDENCIES teaspoon-jasmine (~> 2.2.0) test_after_commit (~> 0.4.2) thin (~> 1.7.0) - tinder (~> 1.10.0) turbolinks (~> 2.5.0) u2f (~> 0.2.1) uglifier (~> 2.7.2) diff --git a/app/models/project_services/campfire_service.rb b/app/models/project_services/campfire_service.rb index 511b2eac792..5af93860d09 100644 --- a/app/models/project_services/campfire_service.rb +++ b/app/models/project_services/campfire_service.rb @@ -1,4 +1,6 @@ class CampfireService < Service + include HTTParty + prop_accessor :token, :subdomain, :room validates :token, presence: true, if: :activated? @@ -29,18 +31,53 @@ class CampfireService < Service def execute(data) return unless supported_events.include?(data[:object_kind]) - room = gate.find_room_by_name(self.room) - return true unless room - + self.class.base_uri base_uri message = build_message(data) - - room.speak(message) + speak(self.room, message, auth) end private - def gate - @gate ||= Tinder::Campfire.new(subdomain, token: token) + def base_uri + @base_uri ||= "https://#{subdomain}.campfirenow.com" + end + + def auth + # use a dummy password, as explained in the Campfire API doc: + # https://github.com/basecamp/campfire-api#authentication + @auth ||= { + basic_auth: { + username: token, + password: 'X' + } + } + end + + # Post a message into a room, returns the message Hash in case of success. + # Returns nil otherwise. + # https://github.com/basecamp/campfire-api/blob/master/sections/messages.md#create-message + def speak(room_name, message, auth) + room = rooms(auth).find { |r| r["name"] == room_name } + return nil unless room + + path = "/room/#{room["id"]}/speak.json" + body = { + body: { + message: { + type: 'TextMessage', + body: message + } + } + } + res = self.class.post(path, auth.merge(body)) + res.code == 201 ? res : nil + end + + # Returns a list of rooms, or []. + # https://github.com/basecamp/campfire-api/blob/master/sections/rooms.md#get-rooms + def rooms(auth) + res = self.class.get("/rooms.json", auth) + res.code == 200 ? res["rooms"] : [] end def build_message(push) diff --git a/spec/fixtures/project_services/campfire/rooms.json b/spec/fixtures/project_services/campfire/rooms.json new file mode 100644 index 00000000000..71e9645c955 --- /dev/null +++ b/spec/fixtures/project_services/campfire/rooms.json @@ -0,0 +1,22 @@ +{ + "rooms": [ + { + "name": "test-room", + "locked": false, + "created_at": "2009/01/07 20:43:11 +0000", + "updated_at": "2009/03/18 14:31:39 +0000", + "topic": "The room topic\n", + "id": 123, + "membership_limit": 4 + }, + { + "name": "another room", + "locked": true, + "created_at": "2009/03/18 14:30:42 +0000", + "updated_at": "2013/01/27 14:14:27 +0000", + "topic": "Comment, ideas, GitHub notifications for eCommittee App", + "id": 456, + "membership_limit": 4 + } + ] +} diff --git a/spec/fixtures/project_services/campfire/rooms2.json b/spec/fixtures/project_services/campfire/rooms2.json new file mode 100644 index 00000000000..3d5f635d8b3 --- /dev/null +++ b/spec/fixtures/project_services/campfire/rooms2.json @@ -0,0 +1,22 @@ +{ + "rooms": [ + { + "name": "test-room-not-found", + "locked": false, + "created_at": "2009/01/07 20:43:11 +0000", + "updated_at": "2009/03/18 14:31:39 +0000", + "topic": "The room topic\n", + "id": 123, + "membership_limit": 4 + }, + { + "name": "another room", + "locked": true, + "created_at": "2009/03/18 14:30:42 +0000", + "updated_at": "2013/01/27 14:14:27 +0000", + "topic": "Comment, ideas, GitHub notifications for eCommittee App", + "id": 456, + "membership_limit": 4 + } + ] +} diff --git a/spec/models/project_services/campfire_service_spec.rb b/spec/models/project_services/campfire_service_spec.rb index 3e6da42803b..1adf93258f3 100644 --- a/spec/models/project_services/campfire_service_spec.rb +++ b/spec/models/project_services/campfire_service_spec.rb @@ -39,4 +39,62 @@ describe CampfireService, models: true do it { is_expected.not_to validate_presence_of(:token) } end end + + describe "#execute" do + let(:user) { create(:user) } + let(:project) { create(:project) } + + before do + @campfire_service = CampfireService.new + allow(@campfire_service).to receive_messages( + project_id: project.id, + project: project, + service_hook: true, + token: 'verySecret', + subdomain: 'project-name', + room: 'test-room' + ) + @sample_data = Gitlab::PushDataBuilder.build_sample(project, user) + @rooms_url = 'https://verySecret:X@project-name.campfirenow.com/rooms.json' + @headers = { 'Content-Type' => 'application/json; charset=utf-8' } + end + + it "calls Campfire API to get a list of rooms and speak in a room" do + # make sure a valid list of rooms is returned + body = File.read(Rails.root + 'spec/fixtures/project_services/campfire/rooms.json') + WebMock.stub_request(:get, @rooms_url).to_return( + body: body, + status: 200, + headers: @headers + ) + # stub the speak request with the room id found in the previous request's response + speak_url = 'https://verySecret:X@project-name.campfirenow.com/room/123/speak.json' + WebMock.stub_request(:post, speak_url) + + @campfire_service.execute(@sample_data) + + expect(WebMock).to have_requested(:get, @rooms_url).once + expect(WebMock).to have_requested(:post, speak_url).with( + body: /#{project.path}.*#{@sample_data[:before]}.*#{@sample_data[:after]}/ + ).once + end + + it "calls Campfire API to get a list of rooms but shouldn't speak in a room" do + # return a list of rooms that do not contain a room named 'test-room' + body = File.read(Rails.root + 'spec/fixtures/project_services/campfire/rooms2.json') + WebMock.stub_request(:get, @rooms_url).to_return( + body: body, + status: 200, + headers: @headers + ) + # we want to make sure no request is sent to the /speak endpoint, here is a basic + # regexp that matches this endpoint + speak_url = 'https://verySecret:X@project-name.campfirenow.com/room/.*/speak.json' + + @campfire_service.execute(@sample_data) + + expect(WebMock).to have_requested(:get, @rooms_url).once + expect(WebMock).not_to have_requested(:post, /#{speak_url}/) + end + end end