Merge branch '13333-consider-updating-http-parser-rb-to-0-6-0' into 'master'

Replace the tinder gem by bare HTTP requests

## What does this MR do?

It removes the `tinder` gem (used to talk to the Campfire API) and replaces its use by bare HTTP requests.

## Are there points in the code the reviewer needs to double check?

N/A.

## Why was this MR needed?

To simplify the potential dependency hell discussed in #13333.  
The initial issue was about updating `http_parser.rb` which was required by `twitter-stream` which was required by `tinder`.  
Those 3 gems are not needed anymore.  

## What are the relevant issue numbers?

#13333

## Screenshots (if relevant)

N/A.

## Does this MR meet the acceptance criteria?

- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- Tests
  - [x] Added for this feature/bug
  - [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
Closes #13333

See merge request !5758
This commit is contained in:
Rémy Coutable 2016-08-11 11:23:53 +00:00
commit 624f35beed
7 changed files with 147 additions and 26 deletions

View file

@ -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)

View file

@ -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'

View file

@ -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)

View file

@ -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)

View file

@ -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
}
]
}

View file

@ -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
}
]
}

View file

@ -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