Merge branch 'refactoring_user_url' into 'master'

Refactoring network related code

https://dev.gitlab.org/gitlab/gitlab-ci/issues/139

See merge request !74

10 files changed, 45 insertions, 51 deletions

diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index f10d42b..dfef9bf 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -75,7 +75,7 @@ class ProjectsController < ApplicationController
 
   def destroy
     project.destroy
-    Network.new.disable_ci(current_user.url, project.gitlab_id, current_user.private_token)
+    Network.new.disable_ci(project.gitlab_id, current_user.private_token)
 
     EventService.new.remove_project(current_user, project)
 
diff --git a/app/controllers/user_sessions_controller.rb b/app/controllers/user_sessions_controller.rb
index 891bbb8..5700615 100644
--- a/app/controllers/user_sessions_controller.rb
+++ b/app/controllers/user_sessions_controller.rb
@@ -18,7 +18,7 @@ class UserSessionsController < ApplicationController
     token = client.auth_code.get_token(params[:code], redirect_uri: callback_user_sessions_url).token
     
     @user_session = UserSession.new
-    user = @user_session.authenticate(access_token: token, url: GitlabCi.config.gitlab_server.url)
+    user = @user_session.authenticate(access_token: token)
 
     if user && sign_in(user)
       redirect_to root_path
diff --git a/app/models/network.rb b/app/models/network.rb
index 631eee6..72eaeed 100644
--- a/app/models/network.rb
+++ b/app/models/network.rb
@@ -5,38 +5,35 @@ class Network
 
   API_PREFIX = '/api/v3/'
 
-  def authenticate(url, api_opts)
+  def authenticate(api_opts)
     opts = {
-      body: api_opts.to_json,
-      headers: { "Content-Type" => "application/json" },
+      body: api_opts.to_json
     }
 
     endpoint = File.join(url, API_PREFIX, 'user')
-    response = self.class.get(endpoint, opts)
+    response = self.class.get(endpoint, default_opts.merge(opts))
 
     build_response(response)
   end
 
-  def authenticate_by_token(url, api_opts)
+  def authenticate_by_token(api_opts)
     opts = {
-      query: api_opts,
-      headers: { "Content-Type" => "application/json" },
+      query: api_opts
     }
 
     endpoint = File.join(url, API_PREFIX, 'user.json')
-    response = self.class.get(endpoint, opts)
+    response = self.class.get(endpoint, default_opts.merge(opts))
 
     build_response(response)
   end
 
 
-  def projects(url, api_opts, scope = :owned)
+  def projects(api_opts, scope = :owned)
     # Dont load archived projects
     api_opts.merge!(archived: false)
 
     opts = {
-      query: api_opts,
-      headers: { "Content-Type" => "application/json" },
+      query: api_opts
     }
 
     query = if scope == :owned
@@ -46,48 +43,45 @@ class Network
             end
 
     endpoint = File.join(url, API_PREFIX, query)
-    response = self.class.get(endpoint, opts)
+    response = self.class.get(endpoint, default_opts.merge(opts))
 
     build_response(response)
   end
 
-  def project(url, api_opts, project_id)
+  def project(api_opts, project_id)
     opts = {
-      query: api_opts,
-      headers: { "Content-Type" => "application/json" },
+      query: api_opts
     }
 
     query = "projects/#{project_id}.json"
 
     endpoint = File.join(url, API_PREFIX, query)
-    response = self.class.get(endpoint, opts)
+    response = self.class.get(endpoint, default_opts.merge(opts))
 
     build_response(response)
   end
 
-  def project_hooks(url, api_opts, project_id)
+  def project_hooks(api_opts, project_id)
     opts = {
-      query: api_opts,
-      headers: { "Content-Type" => "application/json" },
+      query: api_opts
     }
 
     query = "projects/#{project_id}/hooks.json"
 
     endpoint = File.join(url, API_PREFIX, query)
-    response = self.class.get(endpoint, opts)
+    response = self.class.get(endpoint, default_opts.merge(opts))
 
     build_response(response)
   end
 
-  def enable_ci(url, project_id, ci_opts, token)
+  def enable_ci(project_id, api_opts, token)
     opts = {
-      body: ci_opts.to_json,
-      headers: { "Content-Type" => "application/json" },
+      body: api_opts.to_json
     }
 
     query = "projects/#{project_id}/services/gitlab-ci.json?private_token=#{token}"
     endpoint = File.join(url, API_PREFIX, query)
-    response = self.class.put(endpoint, opts)
+    response = self.class.put(endpoint, default_opts.merge(opts))
 
     case response.code
     when 200
@@ -99,21 +93,27 @@ class Network
     end
   end
 
-  def disable_ci(url, project_id, token)
-    opts = {
-      headers: { "Content-Type" => "application/json" },
-    }
-
+  def disable_ci(project_id, token)
     query = "projects/#{project_id}/services/gitlab-ci.json?private_token=#{token}"
 
     endpoint = File.join(url, API_PREFIX, query)
-    response = self.class.delete(endpoint, opts)
+    response = self.class.delete(endpoint, default_opts)
 
     build_response(response)
   end
 
   private
 
+  def url
+    GitlabCi.config.gitlab_server.url
+  end
+
+  def default_opts
+    {
+      headers: { "Content-Type" => "application/json" },
+    }
+  end
+
   def build_response(response)
     case response.code
     when 200
diff --git a/app/models/project.rb b/app/models/project.rb
index a3cb161..31ce6ab 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -94,7 +94,7 @@ ls -la
       opts = { private_token: user.private_token }
       opts.merge! options
 
-      projects = Network.new.projects(user.url, opts.compact, scope)
+      projects = Network.new.projects(opts.compact, scope)
 
       if projects
         projects.map { |pr| OpenStruct.new(pr) }
diff --git a/app/models/user.rb b/app/models/user.rb
index 753717e..4a1fd9b 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -61,7 +61,7 @@ class User
     }
 
     Rails.cache.fetch(cache_key('manage', project_gitlab_id, sync_at)) do
-      !!Network.new.project_hooks(self.url, opts, project_gitlab_id)
+      !!Network.new.project_hooks(opts, project_gitlab_id)
     end
   end
 
@@ -82,7 +82,7 @@ class User
     }
 
     Rails.cache.fetch(cache_key("project_info", project_gitlab_id, sync_at)) do
-      Network.new.project(self.url, opts, project_gitlab_id)
+      Network.new.project(opts, project_gitlab_id)
     end
   end
 end
diff --git a/app/models/user_session.rb b/app/models/user_session.rb
index 30b5b5d..d1c0711 100644
--- a/app/models/user_session.rb
+++ b/app/models/user_session.rb
@@ -3,17 +3,15 @@ class UserSession
   include StaticModel
   extend ActiveModel::Naming
 
-  attr_accessor :url
-
   def authenticate(auth_opts)
-    authenticate_via(auth_opts) do |url, network, options|
-      network.authenticate(url, options)
+    authenticate_via(auth_opts) do |network, options|
+      network.authenticate(options)
     end
   end
 
   def authenticate_by_token(auth_opts)
-    result = authenticate_via(auth_opts) do |url, network, options|
-      network.authenticate_by_token(url, options)
+    result = authenticate_via(auth_opts) do |network, options|
+      network.authenticate_by_token(options)
     end
 
     result
@@ -22,14 +20,10 @@ class UserSession
   private
 
   def authenticate_via(options, &block)
-    url = options.delete(:url)
-
-    return nil unless GitlabCi.config.gitlab_server.url.include?(url)
-
-    user = block.call(url, Network.new, options)
+    user = block.call(Network.new, options)
 
     if user
-      return User.new(user.merge({ "url" => url }))
+      return User.new(user)
     else
       nil
     end
diff --git a/app/services/create_project_service.rb b/app/services/create_project_service.rb
index ddbdc8b..63cbd86 100644
--- a/app/services/create_project_service.rb
+++ b/app/services/create_project_service.rb
@@ -13,7 +13,7 @@ class CreateProjectService
         project_url: project_route.gsub(":project_id", @project.id.to_s),
       }
 
-      unless Network.new.enable_ci(current_user.url, @project.gitlab_id, opts, current_user.private_token)
+      unless Network.new.enable_ci(@project.gitlab_id, opts, current_user.private_token)
         raise ActiveRecord::Rollback
       end
     end
diff --git a/app/views/projects/_gitlab.html.haml b/app/views/projects/_gitlab.html.haml
index d21e200..9847842 100644
--- a/app/views/projects/_gitlab.html.haml
+++ b/app/views/projects/_gitlab.html.haml
@@ -1,6 +1,6 @@
 .clearfix.light
   .pull-left.fetch-status
-    Fetched from GitLab (#{link_to current_user.url, current_user.url, no_turbolink})
+    Fetched from GitLab (#{link_to GitlabCi.config.gitlab_server.url, GitlabCi.config.gitlab_server.url, no_turbolink})
     - if params[:search].present?
       by keyword: "#{params[:search]}",
     #{time_ago_in_words(current_user.sync_at)} ago.
diff --git a/app/views/projects/index.html.haml b/app/views/projects/index.html.haml
index 824ba80..4ec8f68 100644
--- a/app/views/projects/index.html.haml
+++ b/app/views/projects/index.html.haml
@@ -8,7 +8,7 @@
   .projects
     %p.fetch-status.light
       %i.icon-refresh.icon-spin
-      Please wait while we fetch from GitLab (#{current_user.url})
+      Please wait while we fetch from GitLab (#{GitlabCi.config.gitlab_server.url})
   :javascript
     $.get("#{gitlab_projects_path}")
 - else
diff --git a/app/views/user_sessions/show.html.haml b/app/views/user_sessions/show.html.haml
index 4432ed0..12aea2a 100644
--- a/app/views/user_sessions/show.html.haml
+++ b/app/views/user_sessions/show.html.haml
@@ -12,4 +12,4 @@
 
   %p
     %span.light GitLab profile:
-    %strong= link_to @user.username, @user.url + '/u/' + @user.username, target: "_blank"
+    %strong= link_to @user.username, GitlabCi.config.gitlab_server.url + '/u/' + @user.username, target: "_blank"