Fix broken URI joining for `teamcity_url` with suffixes

If one had configured a `teamcity_url` like http://foo.bar/teamcity in
the previous implementation the plugin directed it's request i.e. to
http://foo.bar/httpAuth/... instead of http://foo.bar/teamcity/httpAuth/...

`URI.join` only works correctly, if the prefix URL has
  - at least one or more  trailing '/'
  - the appended parts are _not_ prefixed with '/'

The current implementation should work with all sorts of TeamCity base
URLs.

Signed-off-by: Rémy Coutable <remy@rymai.me>

3 files changed, 25 insertions, 23 deletions

diff --git a/CHANGELOG b/CHANGELOG
index bba6991f184..3d46901ba69 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -39,6 +39,7 @@ v 8.9.0 (unreleased)
   - Fix issues filter when ordering by milestone
   - Added artifacts:when to .gitlab-ci.yml - this requires GitLab Runner 1.3
   - Bamboo Service: Fix missing credentials & URL handling when base URL contains a path (Benjamin Schmid)
+  - TeamCity Service: Fix URL handling when base URL contains a path
   - Todos will display target state if issuable target is 'Closed' or 'Merged'
   - Fix bug when sorting issues by milestone due date and filtering by two or more labels
   - Add support for using Yubikeys (U2F) for two-factor authentication
diff --git a/app/models/project_services/teamcity_service.rb b/app/models/project_services/teamcity_service.rb
index b0dcb52eba1..a4a967c9bc9 100644
--- a/app/models/project_services/teamcity_service.rb
+++ b/app/models/project_services/teamcity_service.rb
@@ -1,6 +1,4 @@
 class TeamcityService < CiService
-  include HTTParty
-
   prop_accessor :teamcity_url, :build_type, :username, :password
 
   validates :teamcity_url, presence: true, url: true, if: :activated?
@@ -64,15 +62,7 @@ class TeamcityService < CiService
   end
 
   def build_info(sha)
-    url = URI.join(
-      teamcity_url,
-      "/httpAuth/app/rest/builds/branch:unspecified:any,number:#{sha}"
-    ).to_s
-    auth = {
-      username: username,
-      password: password
-    }
-    @response = HTTParty.get(url, verify: false, basic_auth: auth)
+    @response = get_path("httpAuth/app/rest/builds/branch:unspecified:any,number:#{sha}")
   end
 
   def build_page(sha, ref)
@@ -81,14 +71,11 @@ class TeamcityService < CiService
     if @response.code != 200
       # If actual build link can't be determined,
       # send user to build summary page.
-      URI.join(teamcity_url, "/viewLog.html?buildTypeId=#{build_type}").to_s
+      build_url("viewLog.html?buildTypeId=#{build_type}")
     else
       # If actual build link is available, go to build result page.
       built_id = @response['build']['id']
-      URI.join(
-        teamcity_url,
-        "/viewLog.html?buildId=#{built_id}&buildTypeId=#{build_type}"
-      ).to_s
+      build_url("viewLog.html?buildId=#{built_id}&buildTypeId=#{build_type}")
     end
   end
 
@@ -123,8 +110,8 @@ class TeamcityService < CiService
 
     branch = Gitlab::Git.ref_name(data[:ref])
 
-    self.class.post(
-      URI.join(teamcity_url, '/httpAuth/app/rest/buildQueue').to_s,
+    HTTParty.post(
+      build_url('httpAuth/app/rest/buildQueue'),
       body: "<build branchName=\"#{branch}\">"\
             "<buildType id=\"#{build_type}\"/>"\
             '</build>',
@@ -132,4 +119,18 @@ class TeamcityService < CiService
       basic_auth: auth
     )
   end
+
+  private
+
+  def build_url(path)
+    URI.join("#{teamcity_url}/", path).to_s
+  end
+
+  def get_path(path)
+    HTTParty.get(build_url(path), verify: false,
+                                  basic_auth: {
+                                    username: username,
+                                    password: password
+                                  })
+  end
 end
diff --git a/spec/models/project_services/teamcity_service_spec.rb b/spec/models/project_services/teamcity_service_spec.rb
index 24a708ca849..474715d24c3 100644
--- a/spec/models/project_services/teamcity_service_spec.rb
+++ b/spec/models/project_services/teamcity_service_spec.rb
@@ -126,19 +126,19 @@ describe TeamcityService, models: true do
     it 'returns a specific URL when status is 500' do
       stub_request(status: 500)
 
-      expect(service.build_page('123', 'unused')).to eq('http://gitlab.com/viewLog.html?buildTypeId=foo')
+      expect(service.build_page('123', 'unused')).to eq('http://gitlab.com/teamcity/viewLog.html?buildTypeId=foo')
     end
 
     it 'returns a build URL when teamcity_url has no trailing slash' do
       stub_request(body: %Q({"build":{"id":"666"}}))
 
-      expect(service(teamcity_url: 'http://gitlab.com').build_page('123', 'unused')).to eq('http://gitlab.com/viewLog.html?buildId=666&buildTypeId=foo')
+      expect(service(teamcity_url: 'http://gitlab.com/teamcity').build_page('123', 'unused')).to eq('http://gitlab.com/teamcity/viewLog.html?buildId=666&buildTypeId=foo')
     end
 
     it 'returns a build URL when teamcity_url has a trailing slash' do
       stub_request(body: %Q({"build":{"id":"666"}}))
 
-      expect(service(teamcity_url: 'http://gitlab.com/').build_page('123', 'unused')).to eq('http://gitlab.com/viewLog.html?buildId=666&buildTypeId=foo')
+      expect(service(teamcity_url: 'http://gitlab.com/teamcity/').build_page('123', 'unused')).to eq('http://gitlab.com/teamcity/viewLog.html?buildId=666&buildTypeId=foo')
     end
   end
 
@@ -180,7 +180,7 @@ describe TeamcityService, models: true do
     end
   end
 
-  def service(teamcity_url: 'http://gitlab.com')
+  def service(teamcity_url: 'http://gitlab.com/teamcity')
     described_class.create(
       project: create(:empty_project),
       properties: {
@@ -193,7 +193,7 @@ describe TeamcityService, models: true do
   end
 
   def stub_request(status: 200, body: nil, build_status: 'success')
-    teamcity_full_url = 'http://mic:password@gitlab.com/httpAuth/app/rest/builds/branch:unspecified:any,number:123'
+    teamcity_full_url = 'http://mic:password@gitlab.com/teamcity/httpAuth/app/rest/builds/branch:unspecified:any,number:123'
     body ||= %Q({"build":{"status":"#{build_status}","id":"666"}})
 
     WebMock.stub_request(:get, teamcity_full_url).to_return(