Merge remote-tracking branch 'origin/master' into 8-6-stable

CHANGELOG

@@ -10,6 +10,7 @@ v 8.6.0 (unreleased)
     setup. A password can be provided during setup (see installation docs), or
     GitLab will ask the user to create a new one upon first visit.
   - Fix issue when pushing to projects ending in .wiki
+  - Add support for wiki with UTF-8 page names (Hiroyuki Sato)
   - Don't load all of GitLab in mail_room
   - Update `omniauth-saml` to 1.5.0 to allow for custom response attributes to be set
   - Memoize @group in Admin::GroupsController (Yatish Mehta)
@@ -39,6 +40,10 @@ v 8.6.0 (unreleased)
   - Add ability to show archived projects on dashboard, explore and group pages
   - Move group activity to separate page
   - Continue parameters are checked to ensure redirection goes to the same instance
+  - User deletion is now done in the background so the request can not time out
+
+v 8.5.7
+  - Bump Git version requirement to 2.7.3
 
 v 8.5.6
   - Obtain a lease before querying LDAP

Gemfile

@@ -58,7 +58,9 @@ gem "gitlab_git", '~> 9.0'
 gem 'gitlab_omniauth-ldap', '~> 1.2.1', require: "omniauth-ldap"
 
 # Git Wiki
-gem 'gollum-lib', '~> 4.1.0'
+# Required manually in config/initializers/gollum.rb to control load order
+gem 'gollum-lib', '~> 4.1.0', require: false
+gem 'gollum-rugged_adapter', '~> 0.4.2', require: false
 
 # Language detection
 gem "github-linguist", "~> 4.7.0", require: "linguist"

Gemfile.lock

@@ -381,6 +381,9 @@ GEM
       rouge (~> 1.9)
       sanitize (~> 2.1.0)
       stringex (~> 2.5.1)
+    gollum-rugged_adapter (0.4.2)
+      mime-types (>= 1.15)
+      rugged (~> 0.24.0, >= 0.21.3)
     gon (6.0.1)
       actionpack (>= 3.0)
       json
@@ -703,7 +706,7 @@ GEM
     rubyntlm (0.5.2)
     rubypants (0.2.0)
     rufus-scheduler (3.1.10)
-    rugged (0.24.0b13)
+    rugged (0.24.0)
     safe_yaml (1.0.4)
     sanitize (2.1.0)
       nokogiri (>= 1.4.4)
@@ -941,6 +944,7 @@ DEPENDENCIES
   gitlab_meta (= 7.0)
   gitlab_omniauth-ldap (~> 1.2.1)
   gollum-lib (~> 4.1.0)
+  gollum-rugged_adapter (~> 0.4.2)
   gon (~> 6.0.1)
   grape (~> 0.13.0)
   grape-entity (~> 0.4.2)

README.md

@@ -68,7 +68,7 @@ GitLab is a Ruby on Rails application that runs on the following software:
 
 - Ubuntu/Debian/CentOS/RHEL
 - Ruby (MRI) 2.1
-- Git 1.7.10+
+- Git 2.7.3+
 - Redis 2.8+
 - MySQL or PostgreSQL
 

app/assets/stylesheets/framework/dropdowns.scss

@@ -161,9 +161,8 @@
 
 .dropdown-menu-user-full-name {
   display: block;
-  margin-bottom: 2px;
   font-weight: 600;
-  line-height: 1;
+  line-height: 16px;
   text-overflow: ellipsis;
   overflow: hidden;
   white-space: nowrap;
@@ -171,7 +170,7 @@
 
 .dropdown-menu-user-username {
   display: block;
-  line-height: 1;
+  line-height: 16px;
   text-overflow: ellipsis;
   overflow: hidden;
   white-space: nowrap;

app/controllers/admin/abuse_reports_controller.rb

@@ -6,7 +6,7 @@ class Admin::AbuseReportsController < Admin::ApplicationController
   def destroy
     abuse_report = AbuseReport.find(params[:id])
 
-    abuse_report.remove_user if params[:remove_user]
+    abuse_report.remove_user(deleted_by: current_user) if params[:remove_user]
     abuse_report.destroy
 
     render nothing: true

app/controllers/admin/users_controller.rb

@@ -119,10 +119,10 @@ class Admin::UsersController < Admin::ApplicationController
   end
 
   def destroy
-    DeleteUserService.new(current_user).execute(user)
+    DeleteUserWorker.perform_async(current_user.id, user.id)
 
     respond_to do |format|
-      format.html { redirect_to admin_users_path }
+      format.html { redirect_to admin_users_path, notice: "The user is being deleted." }
       format.json { head :ok }
     end
   end

app/models/abuse_report.rb

@@ -19,9 +19,9 @@ class AbuseReport < ActiveRecord::Base
   validates :message, presence: true
   validates :user_id, uniqueness: { message: 'has already been reported' }
 
-  def remove_user
+  def remove_user(deleted_by:)
     user.block
-    user.destroy
+    DeleteUserWorker.perform_async(deleted_by.id, user.id, delete_solo_owned_groups: true)
   end
 
   def notify

app/models/project_wiki.rb

@@ -2,7 +2,7 @@ class ProjectWiki
   include Gitlab::ShellAdapter
 
   MARKUPS = {
-    'Markdown' => :md,
+    'Markdown' => :markdown,
     'RDoc'     => :rdoc,
     'AsciiDoc' => :asciidoc
   } unless defined?(MARKUPS)
@@ -47,7 +47,7 @@ class ProjectWiki
   def wiki
     @wiki ||= begin
       Gollum::Wiki.new(path_to_repo)
-    rescue Gollum::NoSuchPathError
+    rescue Rugged::OSError
       create_repo!
     end
   end
@@ -90,7 +90,7 @@ class ProjectWiki
   def create_page(title, content, format = :markdown, message = nil)
     commit = commit_details(:created, message, title)
 
-    wiki.write_page(title, format, content, commit)
+    wiki.write_page(title, format.to_sym, content, commit)
 
     update_project_activity
   rescue Gollum::DuplicatePageError => e
@@ -101,7 +101,7 @@ class ProjectWiki
   def update_page(page, content, format = :markdown, message = nil)
     commit = commit_details(:updated, message, page.title)
 
-    wiki.update_page(page, page.name, format, content, commit)
+    wiki.update_page(page, page.name, format.to_sym, content, commit)
 
     update_project_activity
   end

app/models/wiki_page.rb

@@ -62,7 +62,7 @@ class WikiPage
   # The raw content of this page.
   def content
     @attributes[:content] ||= if @page
-                                @page.raw_data
+                                @page.text_data
                               end
   end
 

app/services/delete_user_service.rb

@@ -5,18 +5,22 @@ class DeleteUserService
     @current_user = current_user
   end
 
-  def execute(user)
-    if user.solo_owned_groups.present?
+  def execute(user, options = {})
+    if !options[:delete_solo_owned_groups] && user.solo_owned_groups.present?
       user.errors[:base] << 'You must transfer ownership or delete groups before you can remove user'
-      user
-    else
-      user.personal_projects.each do |project|
-        # Skip repository removal because we remove directory with namespace
-        # that contain all this repositories
-        ::Projects::DestroyService.new(project, current_user, skip_repo: true).pending_delete!
-      end
-
-      user.destroy
+      return user
     end
+
+    user.solo_owned_groups.each do |group|
+      DestroyGroupService.new(group, current_user).execute
+    end
+
+    user.personal_projects.each do |project|
+      # Skip repository removal because we remove directory with namespace
+      # that contain all this repositories
+      ::Projects::DestroyService.new(project, current_user, skip_repo: true).pending_delete!
+    end
+
+    user.destroy
   end
 end

app/services/destroy_group_service.rb

@@ -6,12 +6,12 @@ class DestroyGroupService
   end
 
   def execute
-    @group.projects.each do |project|
+    group.projects.each do |project|
       # Skip repository removal because we remove directory with namespace
       # that contain all this repositories
       ::Projects::DestroyService.new(project, current_user, skip_repo: true).pending_delete!
     end
 
-    @group.destroy
+    group.destroy
   end
 end

app/views/shared/issuable/_filter.html.haml

@@ -16,7 +16,7 @@
           - if params[:assignee_id]
             = hidden_field_tag(:assignee_id, params[:assignee_id])
           = dropdown_tag("Assignee", options: { toggle_class: "js-user-search js-filter-submit js-assignee-search", title: "Filter by assignee", filter: true, dropdown_class: "dropdown-menu-user dropdown-menu-selectable dropdown-menu-assignee",
-            placeholder: "Search assignee", data: { any_user: "Any Author", first_user: (current_user.username if current_user), null_user: true, current_user: true, project_id: (@project.id if @project), selected: params[:assignee_id], field_name: "assignee_id" } })
+            placeholder: "Search assignee", data: { any_user: "Any Assignee", first_user: (current_user.username if current_user), null_user: true, current_user: true, project_id: (@project.id if @project), selected: params[:assignee_id], field_name: "assignee_id" } })
 
         .filter-item.inline.milestone-filter
           - if params[:milestone_title]

app/workers/delete_user_worker.rb

@@ -0,0 +1,10 @@
+class DeleteUserWorker
+  include Sidekiq::Worker
+
+  def perform(current_user_id, delete_user_id, options = {})
+    delete_user  = User.find(delete_user_id)
+    current_user = User.find(current_user_id)
+
+    DeleteUserService.new(current_user).execute(delete_user, options.symbolize_keys)
+  end
+end

config/initializers/gollum.rb

@@ -0,0 +1,13 @@
+module Gollum
+  GIT_ADAPTER = "rugged"
+end
+require "gollum-lib"
+
+module Gollum
+  class Committer
+    # Patch for UTF-8 path
+    def method_missing(name, *args)
+      index.send(name, *args)
+    end
+  end
+end

doc/install/installation.md

@@ -76,7 +76,7 @@ Make sure you have the right version of Git installed
     # Install Git
     sudo apt-get install -y git-core
 
-    # Make sure Git is version 1.7.10 or higher, for example 1.7.12 or 2.0.0
+    # Make sure Git is version 2.7.3 or higher
     git --version
 
 Is the system packaged Git too old? Remove it and compile from source.

features/support/capybara.rb

@@ -9,7 +9,7 @@ Capybara.register_driver :poltergeist do |app|
   Capybara::Poltergeist::Driver.new(app, js_errors: true, timeout: timeout, window_size: [1366, 768])
 end
 
-Capybara.default_wait_time = timeout
+Capybara.default_max_wait_time = timeout
 Capybara.ignore_hidden_elements = false
 
 unless ENV['CI'] || ENV['CI_SERVER']

lib/gitlab/diff/parser.rb

@@ -4,6 +4,8 @@ module Gitlab
       include Enumerable
 
       def parse(lines)
+        return [] if lines.blank?
+
         @lines = lines
         line_obj_index = 0
         line_old = 1

lib/tasks/gitlab/check.rake

@@ -913,7 +913,7 @@ namespace :gitlab do
   end
 
   def check_git_version
-    required_version = Gitlab::VersionInfo.new(1, 7, 10)
+    required_version = Gitlab::VersionInfo.new(2, 7, 3)
     current_version = Gitlab::VersionInfo.parse(run(%W(#{Gitlab.config.git.bin_path} --version)))
 
     puts "Your git bin path is \"#{Gitlab.config.git.bin_path}\""

spec/lib/gitlab/diff/parser_spec.rb

@@ -90,4 +90,9 @@ eos
       end
     end
   end
+
+  context 'when lines is empty' do
+    it { expect(parser.parse([])).to eq([]) }
+    it { expect(parser.parse(nil)).to eq([]) }
+  end
 end

spec/models/abuse_report_spec.rb

@@ -13,7 +13,8 @@
 require 'rails_helper'
 
 RSpec.describe AbuseReport, type: :model do
-  subject { create(:abuse_report) }
+  subject     { create(:abuse_report) }
+  let(:user)  { create(:user) }
 
   it { expect(subject).to be_valid }
 
@@ -31,17 +32,14 @@ RSpec.describe AbuseReport, type: :model do
 
   describe '#remove_user' do
     it 'blocks the user' do
-      report = build(:abuse_report)
-
-      allow(report.user).to receive(:destroy)
-
-      expect { report.remove_user }.to change { report.user.blocked? }.to(true)
+      expect { subject.remove_user(deleted_by: user) }.to change { subject.user.blocked? }.to(true)
     end
 
-    it 'removes the user' do
-      report = build(:abuse_report)
+    it 'lets a worker delete the user' do
+      expect(DeleteUserWorker).to receive(:perform_async).with(user.id, subject.user.id,
+                                                              delete_solo_owned_groups: true)
 
-      expect { report.remove_user }.to change { User.count }.by(-1)
+      subject.remove_user(deleted_by: user)
     end
   end
 

spec/services/delete_user_service_spec.rb

@@ -0,0 +1,58 @@
+require 'spec_helper'
+
+describe DeleteUserService, services: true do
+  describe "Deletes a user and all their personal projects" do
+    let!(:user)         { create(:user) }
+    let!(:current_user) { create(:user) }
+    let!(:namespace)    { create(:namespace, owner: user) }
+    let!(:project)      { create(:project, namespace: namespace) }
+
+    context 'no options are given' do
+      it 'deletes the user' do
+        DeleteUserService.new(current_user).execute(user)
+
+        expect { User.find(user.id)       }.to  raise_error(ActiveRecord::RecordNotFound)
+      end
+
+      it 'will delete the project in the near future' do
+        expect_any_instance_of(Projects::DestroyService).to receive(:pending_delete!).once
+
+        DeleteUserService.new(current_user).execute(user)
+      end
+    end
+
+    context "solo owned groups present" do
+      let(:solo_owned)  { create(:group) }
+      let(:member)      { create(:group_member) }
+      let(:user)        { member.user }
+
+      before do
+        solo_owned.group_members = [member]
+        DeleteUserService.new(current_user).execute(user)
+      end
+
+      it 'does not delete the user' do
+        expect(User.find(user.id)).to eq user
+      end
+    end
+
+    context "deletions with solo owned groups" do
+      let(:solo_owned)      { create(:group) }
+      let(:member)          { create(:group_member) }
+      let(:user)            { member.user }
+
+      before do
+        solo_owned.group_members = [member]
+        DeleteUserService.new(current_user).execute(user, delete_solo_owned_groups: true)
+      end
+
+      it 'deletes solo owned groups' do
+        expect { Project.find(solo_owned.id) }.to raise_error(ActiveRecord::RecordNotFound)
+      end
+
+      it 'deletes the user' do
+        expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound)
+      end
+    end
+  end
+end

spec/support/capybara.rb

@@ -10,7 +10,7 @@ Capybara.register_driver :poltergeist do |app|
   Capybara::Poltergeist::Driver.new(app, js_errors: true, timeout: timeout, window_size: [1366, 768])
 end
 
-Capybara.default_wait_time = timeout
+Capybara.default_max_wait_time = timeout
 Capybara.ignore_hidden_elements = true
 
 unless ENV['CI'] || ENV['CI_SERVER']

spec/support/wait_for_ajax.rb

@@ -1,6 +1,6 @@
 module WaitForAjax
   def wait_for_ajax
-    Timeout.timeout(Capybara.default_wait_time) do
+    Timeout.timeout(Capybara.default_max_wait_time) do
       loop until finished_all_ajax_requests?
     end
   end

spec/workers/delete_user_worker_spec.rb

@@ -0,0 +1,20 @@
+require 'spec_helper'
+
+describe DeleteUserWorker do
+  let!(:user)         { create(:user) }
+  let!(:current_user) { create(:user) }
+
+  it "calls the DeleteUserWorker with the params it was given" do
+    expect_any_instance_of(DeleteUserService).to receive(:execute).
+                                                      with(user, {})
+
+    DeleteUserWorker.new.perform(current_user.id, user.id)
+  end
+
+  it "uses symbolized keys" do
+    expect_any_instance_of(DeleteUserService).to receive(:execute).
+                                                      with(user, test: "test")
+
+    DeleteUserWorker.new.perform(current_user.id, user.id, "test" => "test")
+  end
+end