Merge branch '10-5-stable-patch-5' into '10-5-stable'

app/models/concerns/storage/legacy_namespace.rb

@@ -7,28 +7,23 @@ module Storage
         raise Gitlab::UpdatePathError.new('Namespace cannot be moved, because at least one project has tags in container registry')
       end
 
-      expires_full_path_cache
-
-      # Move the namespace directory in all storage paths used by member projects
-      repository_storage_paths.each do |repository_storage_path|
-        # Ensure old directory exists before moving it
-        gitlab_shell.add_namespace(repository_storage_path, full_path_was)
-
-        # Ensure new directory exists before moving it (if there's a parent)
-        gitlab_shell.add_namespace(repository_storage_path, parent.full_path) if parent
-
-        unless gitlab_shell.mv_namespace(repository_storage_path, full_path_was, full_path)
+      parent_was = if parent_changed? && parent_id_was.present?
+                     Namespace.find(parent_id_was) # raise NotFound early if needed
+                   end
 
-          Rails.logger.error "Exception moving path #{repository_storage_path} from #{full_path_was} to #{full_path}"
+      expires_full_path_cache
 
-          # if we cannot move namespace directory we should rollback
-          # db changes in order to prevent out of sync between db and fs
-          raise Gitlab::UpdatePathError.new('namespace directory cannot be moved')
-        end
-      end
+      move_repositories
 
+      if parent_changed?
+        former_parent_full_path = parent_was&.full_path
+        parent_full_path = parent&.full_path
+        Gitlab::UploadsTransfer.new.move_namespace(path, former_parent_full_path, parent_full_path)
+        Gitlab::PagesTransfer.new.move_namespace(path, former_parent_full_path, parent_full_path)
+      else
         Gitlab::UploadsTransfer.new.rename_namespace(full_path_was, full_path)
         Gitlab::PagesTransfer.new.rename_namespace(full_path_was, full_path)
+      end
 
       remove_exports!
 
@@ -57,6 +52,26 @@ module Storage
 
     private
 
+    def move_repositories
+      # Move the namespace directory in all storage paths used by member projects
+      repository_storage_paths.each do |repository_storage_path|
+        # Ensure old directory exists before moving it
+        gitlab_shell.add_namespace(repository_storage_path, full_path_was)
+
+        # Ensure new directory exists before moving it (if there's a parent)
+        gitlab_shell.add_namespace(repository_storage_path, parent.full_path) if parent
+
+        unless gitlab_shell.mv_namespace(repository_storage_path, full_path_was, full_path)
+
+          Rails.logger.error "Exception moving path #{repository_storage_path} from #{full_path_was} to #{full_path}"
+
+          # if we cannot move namespace directory we should rollback
+          # db changes in order to prevent out of sync between db and fs
+          raise Gitlab::UpdatePathError.new('namespace directory cannot be moved')
+        end
+      end
+    end
+
     def old_repository_storage_paths
       @old_repository_storage_paths ||= repository_storage_paths
     end

app/models/repository.rb

@@ -35,7 +35,7 @@ class Repository
   CACHED_METHODS = %i(size commit_count rendered_readme contribution_guide
                       changelog license_blob license_key gitignore koding_yml
                       gitlab_ci_yml branch_names tag_names branch_count
-                      tag_count avatar exists? empty? root_ref has_visible_content?
+                      tag_count avatar exists? root_ref has_visible_content?
                       issue_template_names merge_request_template_names).freeze
 
   # Methods that use cache_method but only memoize the value
@@ -359,7 +359,7 @@ class Repository
   def expire_emptiness_caches
     return unless empty?
 
-    expire_method_caches(%i(empty? has_visible_content?))
+    expire_method_caches(%i(has_visible_content?))
   end
 
   def lookup_cache
@@ -509,12 +509,14 @@ class Repository
   end
   cache_method :exists?
 
+  # We don't need to cache the output of this method because both exists? and
+  # has_visible_content? are already memoized and cached. There's no guarantee
+  # that the values are expired and loaded atomically.
   def empty?
     return true unless exists?
 
     !has_visible_content?
   end
-  cache_method :empty?
 
   # The size of this repository in megabytes.
   def size

changelogs/unreleased/fix-code-search-500-with-non-ascii-filename.yml

+---
+title: Fix code and wiki search results when filename is non-ASCII
+merge_request:
+author:
+type: fixed

changelogs/unreleased/mk-fix-move-upload-files-on-group-transfer.yml

+---
+title: Fix missing uploads after group transfer
+merge_request: 17658
+author:
+type: fixed

changelogs/unreleased/sh-add-missing-acts-as-taggable-indices.yml

+---
+title: Adding missing indexes on taggings table
+merge_request:
+author:
+type: performance

changelogs/unreleased/sh-add-section-name-index.yml

+---
+title: Add index on section_name_id on ci_build_trace_sections table
+merge_request:
+author:
+type: performance

changelogs/unreleased/sh-remove-double-caching-repo-empty.yml

+---
+title: Remove double caching of Repository#empty?
+merge_request:
+author:
+type: fixed

db/migrate/20180306134842_add_missing_indexes_acts_as_taggable_on_engine.rb

+# This migration comes from acts_as_taggable_on_engine (originally 6)
+#
+# It has been modified to handle no-downtime GitLab migrations. Several
+# indexes have been removed since they are not needed for GitLab.
+class AddMissingIndexesActsAsTaggableOnEngine < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :taggings, :tag_id unless index_exists? :taggings, :tag_id
+    add_concurrent_index :taggings, [:taggable_id, :taggable_type] unless index_exists? :taggings, [:taggable_id, :taggable_type]
+  end
+
+  def down
+    remove_concurrent_index :taggings, :tag_id
+    remove_concurrent_index :taggings, [:taggable_id, :taggable_type]
+  end
+end

db/migrate/20180308052825_add_section_name_id_index_on_ci_build_trace_sections.rb

+class AddSectionNameIdIndexOnCiBuildTraceSections < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    # MySQL may already have this as a foreign key
+    unless index_exists?(:ci_build_trace_sections, :section_name_id)
+      add_concurrent_index :ci_build_trace_sections, :section_name_id
+    end
+  end
+
+  def down
+    # We cannot remove index for MySQL because it's needed for foreign key
+    if Gitlab::Database.postgresql?
+      remove_concurrent_index :ci_build_trace_sections, :section_name_id
+    end
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20180216121030) do
+ActiveRecord::Schema.define(version: 20180308052825) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -249,6 +249,7 @@ ActiveRecord::Schema.define(version: 20180216121030) do
 
   add_index "ci_build_trace_sections", ["build_id", "section_name_id"], name: "index_ci_build_trace_sections_on_build_id_and_section_name_id", unique: true, using: :btree
   add_index "ci_build_trace_sections", ["project_id"], name: "index_ci_build_trace_sections_on_project_id", using: :btree
+  add_index "ci_build_trace_sections", ["section_name_id"], name: "index_ci_build_trace_sections_on_section_name_id", using: :btree
 
   create_table "ci_builds", force: :cascade do |t|
     t.string "status"
@@ -1700,7 +1701,9 @@ ActiveRecord::Schema.define(version: 20180216121030) do
   end
 
   add_index "taggings", ["tag_id", "taggable_id", "taggable_type", "context", "tagger_id", "tagger_type"], name: "taggings_idx", unique: true, using: :btree
+  add_index "taggings", ["tag_id"], name: "index_taggings_on_tag_id", using: :btree
   add_index "taggings", ["taggable_id", "taggable_type", "context"], name: "index_taggings_on_taggable_id_and_taggable_type_and_context", using: :btree
+  add_index "taggings", ["taggable_id", "taggable_type"], name: "index_taggings_on_taggable_id_and_taggable_type", using: :btree
 
   create_table "tags", force: :cascade do |t|
     t.string "name"

lib/gitlab/project_transfer.rb

 module Gitlab
+  # This class is used to move local, unhashed files owned by projects to their new location
   class ProjectTransfer
-    def move_project(project_path, namespace_path_was, namespace_path)
-      new_namespace_folder = File.join(root_dir, namespace_path)
-      FileUtils.mkdir_p(new_namespace_folder) unless Dir.exist?(new_namespace_folder)
-      from = File.join(root_dir, namespace_path_was, project_path)
-      to = File.join(root_dir, namespace_path, project_path)
+    # nil parent_path (or parent_path_was) represents a root namespace
+    def move_namespace(path, parent_path_was, parent_path)
+      parent_path_was ||= ''
+      parent_path ||= ''
+      new_parent_folder = File.join(root_dir, parent_path)
+      FileUtils.mkdir_p(new_parent_folder)
+      from = File.join(root_dir, parent_path_was, path)
+      to = File.join(root_dir, parent_path, path)
       move(from, to, "")
     end
 
+    alias_method :move_project, :move_namespace
+
     def rename_project(path_was, path, namespace_path)
       base_dir = File.join(root_dir, namespace_path)
       move(path_was, path, base_dir)

lib/gitlab/search_results.rb

@@ -7,8 +7,8 @@ module Gitlab
 
       def initialize(opts = {})
         @id = opts.fetch(:id, nil)
-        @filename = opts.fetch(:filename, nil)
-        @basename = opts.fetch(:basename, nil)
+        @filename = encode_utf8(opts.fetch(:filename, nil))
+        @basename = encode_utf8(opts.fetch(:basename, nil))
         @ref = opts.fetch(:ref, nil)
         @startline = opts.fetch(:startline, nil)
         @data = encode_utf8(opts.fetch(:data, nil))

spec/features/milestone_spec.rb

@@ -94,4 +94,15 @@ feature 'Milestone' do
       end
     end
   end
+
+  feature 'Deleting a milestone' do
+    scenario "The delete milestone button does not show for unauthorized users" do
+      create(:milestone, project: project, title: 8.7)
+      sign_out(user)
+
+      visit group_milestones_path(group)
+
+      expect(page).to have_selector('.js-delete-milestone-button', count: 0)
+    end
+  end
 end

spec/lib/gitlab/project_search_results_spec.rb

@@ -108,14 +108,26 @@ describe Gitlab::ProjectSearchResults do
 
       context 'when the search returns non-ASCII data' do
         context 'with UTF-8' do
-          let(:results) { project.repository.search_files_by_content("файл", 'master') }
+          let(:results) { project.repository.search_files_by_content('файл', 'master') }
 
           it 'returns results as UTF-8' do
             expect(subject.filename).to eq('encoding/russian.rb')
             expect(subject.basename).to eq('encoding/russian')
             expect(subject.ref).to eq('master')
             expect(subject.startline).to eq(1)
-            expect(subject.data).to eq("Хороший файл")
+            expect(subject.data).to eq('Хороший файл')
+          end
+        end
+
+        context 'with UTF-8 in the filename' do
+          let(:results) { project.repository.search_files_by_content('webhook', 'master') }
+
+          it 'returns results as UTF-8' do
+            expect(subject.filename).to eq('encoding/テスト.txt')
+            expect(subject.basename).to eq('encoding/テスト')
+            expect(subject.ref).to eq('master')
+            expect(subject.startline).to eq(3)
+            expect(subject.data).to include('WebHookの確認')
           end
         end
 

spec/lib/gitlab/project_transfer_spec.rb

@@ -21,30 +21,77 @@ describe Gitlab::ProjectTransfer do
 
   describe '#move_project' do
     it "moves project upload to another namespace" do
-      FileUtils.mkdir_p(File.join(@root_dir, @namespace_path_was, @project_path))
+      path_to_be_moved = File.join(@root_dir, @namespace_path_was, @project_path)
+      expected_path = File.join(@root_dir, @namespace_path, @project_path)
+      FileUtils.mkdir_p(path_to_be_moved)
+
       @project_transfer.move_project(@project_path, @namespace_path_was, @namespace_path)
 
-      expected_path = File.join(@root_dir, @namespace_path, @project_path)
       expect(Dir.exist?(expected_path)).to be_truthy
     end
   end
 
+  describe '#move_namespace' do
+    context 'when moving namespace from root into another namespace' do
+      it "moves namespace projects' upload" do
+        child_namespace = 'test_child_namespace'
+        path_to_be_moved = File.join(@root_dir, child_namespace, @project_path)
+        expected_path = File.join(@root_dir, @namespace_path, child_namespace, @project_path)
+        FileUtils.mkdir_p(path_to_be_moved)
+
+        @project_transfer.move_namespace(child_namespace, nil, @namespace_path)
+
+        expect(Dir.exist?(expected_path)).to be_truthy
+      end
+    end
+
+    context 'when moving namespace from one parent to another' do
+      it "moves namespace projects' upload" do
+        child_namespace = 'test_child_namespace'
+        path_to_be_moved = File.join(@root_dir, @namespace_path_was, child_namespace, @project_path)
+        expected_path = File.join(@root_dir, @namespace_path, child_namespace, @project_path)
+        FileUtils.mkdir_p(path_to_be_moved)
+
+        @project_transfer.move_namespace(child_namespace, @namespace_path_was, @namespace_path)
+
+        expect(Dir.exist?(expected_path)).to be_truthy
+      end
+    end
+
+    context 'when moving namespace from having a parent to root' do
+      it "moves namespace projects' upload" do
+        child_namespace = 'test_child_namespace'
+        path_to_be_moved = File.join(@root_dir, @namespace_path_was, child_namespace, @project_path)
+        expected_path = File.join(@root_dir, child_namespace, @project_path)
+        FileUtils.mkdir_p(path_to_be_moved)
+
+        @project_transfer.move_namespace(child_namespace, @namespace_path_was, nil)
+
+        expect(Dir.exist?(expected_path)).to be_truthy
+      end
+    end
+  end
+
   describe '#rename_project' do
     it "renames project" do
-      FileUtils.mkdir_p(File.join(@root_dir, @namespace_path, @project_path_was))
+      path_to_be_moved = File.join(@root_dir, @namespace_path, @project_path_was)
+      expected_path = File.join(@root_dir, @namespace_path, @project_path)
+      FileUtils.mkdir_p(path_to_be_moved)
+
       @project_transfer.rename_project(@project_path_was, @project_path, @namespace_path)
 
-      expected_path = File.join(@root_dir, @namespace_path, @project_path)
       expect(Dir.exist?(expected_path)).to be_truthy
     end
   end
 
   describe '#rename_namespace' do
     it "renames namespace" do
-      FileUtils.mkdir_p(File.join(@root_dir, @namespace_path_was, @project_path))
+      path_to_be_moved = File.join(@root_dir, @namespace_path_was, @project_path)
+      expected_path = File.join(@root_dir, @namespace_path, @project_path)
+      FileUtils.mkdir_p(path_to_be_moved)
+
       @project_transfer.rename_namespace(@namespace_path_was, @namespace_path)
 
-      expected_path = File.join(@root_dir, @namespace_path, @project_path)
       expect(Dir.exist?(expected_path)).to be_truthy
     end
   end

spec/models/namespace_spec.rb

@@ -168,19 +168,7 @@ describe Namespace do
   end
 
   describe '#move_dir', :request_store do
-    let(:namespace) { create(:namespace) }
-    let!(:project) { create(:project_empty_repo, namespace: namespace) }
-
-    it "raises error when directory exists" do
-      expect { namespace.move_dir }.to raise_error("namespace directory cannot be moved")
-    end
-
-    it "moves dir if path changed" do
-      namespace.update_attributes(path: namespace.full_path + '_new')
-
-      expect(gitlab_shell.exists?(project.repository_storage_path, "#{namespace.path}/#{project.path}.git")).to be_truthy
-    end
-
+    shared_examples "namespace restrictions" do
       context "when any project has container images" do
         let(:container_repository) { create(:container_repository) }
 
@@ -188,7 +176,7 @@ describe Namespace do
           stub_container_registry_config(enabled: true)
           stub_container_registry_tags(repository: :any, tags: ['tag'])
 
-        create(:project, namespace: namespace, container_repositories: [container_repository])
+          create(:project, :hashed, namespace: namespace, container_repositories: [container_repository])
 
           allow(namespace).to receive(:path_was).and_return(namespace.path)
           allow(namespace).to receive(:path).and_return('new_path')
@@ -198,45 +186,102 @@ describe Namespace do
           expect { namespace.move_dir }.to raise_error(/Namespace cannot be moved/)
         end
       end
+    end
+
+    context 'legacy storage' do
+      let(:namespace) { create(:namespace) }
+      let!(:project) { create(:project_empty_repo, namespace: namespace) }
+
+      it_behaves_like 'namespace restrictions'
+
+      it "raises error when directory exists" do
+        expect { namespace.move_dir }.to raise_error("namespace directory cannot be moved")
+      end
+
+      it "moves dir if path changed" do
+        namespace.update_attributes(path: namespace.full_path + '_new')
+
+        expect(gitlab_shell.exists?(project.repository_storage_path, "#{namespace.path}/#{project.path}.git")).to be_truthy
+      end
 
-    context 'with subgroups' do
+      context 'with subgroups', :nested_groups do
         let(:parent) { create(:group, name: 'parent', path: 'parent') }
+        let(:new_parent) { create(:group, name: 'new_parent', path: 'new_parent') }
         let(:child) { create(:group, name: 'child', path: 'child', parent: parent) }
         let!(:project) { create(:project_empty_repo, path: 'the-project', namespace: child, skip_disk_validation: true) }
         let(:uploads_dir) { FileUploader.root }
         let(:pages_dir) { File.join(TestEnv.pages_path) }
 
+        def expect_project_directories_at(namespace_path)
+          expected_repository_path = File.join(TestEnv.repos_path, namespace_path, 'the-project.git')
+          expected_upload_path = File.join(uploads_dir, namespace_path, 'the-project')
+          expected_pages_path = File.join(pages_dir, namespace_path, 'the-project')
+
+          expect(File.directory?(expected_repository_path)).to be_truthy
+          expect(File.directory?(expected_upload_path)).to be_truthy
+          expect(File.directory?(expected_pages_path)).to be_truthy
+        end
+
         before do
-        FileUtils.mkdir_p(File.join(uploads_dir, 'parent', 'child', 'the-project'))
-        FileUtils.mkdir_p(File.join(pages_dir, 'parent', 'child', 'the-project'))
+          FileUtils.mkdir_p(File.join(TestEnv.repos_path, "#{project.full_path}.git"))
+          FileUtils.mkdir_p(File.join(uploads_dir, project.full_path))
+          FileUtils.mkdir_p(File.join(pages_dir, project.full_path))
         end
 
         context 'renaming child' do
           it 'correctly moves the repository, uploads and pages' do
-          expected_repository_path = File.join(TestEnv.repos_path, 'parent', 'renamed', 'the-project.git')
-          expected_upload_path = File.join(uploads_dir, 'parent', 'renamed', 'the-project')
-          expected_pages_path = File.join(pages_dir, 'parent', 'renamed', 'the-project')
-
-          child.update_attributes!(path: 'renamed')
+            child.update!(path: 'renamed')
 
-          expect(File.directory?(expected_repository_path)).to be(true)
-          expect(File.directory?(expected_upload_path)).to be(true)
-          expect(File.directory?(expected_pages_path)).to be(true)
+            expect_project_directories_at('parent/renamed')
           end
         end
 
         context 'renaming parent' do
           it 'correctly moves the repository, uploads and pages' do
-          expected_repository_path = File.join(TestEnv.repos_path, 'renamed', 'child', 'the-project.git')
-          expected_upload_path = File.join(uploads_dir, 'renamed', 'child', 'the-project')
-          expected_pages_path = File.join(pages_dir, 'renamed', 'child', 'the-project')
+            parent.update!(path: 'renamed')
+
+            expect_project_directories_at('renamed/child')
+          end
+        end
+
+        context 'moving from one parent to another' do
+          it 'correctly moves the repository, uploads and pages' do
+            child.update!(parent: new_parent)
 
-          parent.update_attributes!(path: 'renamed')
+            expect_project_directories_at('new_parent/child')
+          end
+        end
 
-          expect(File.directory?(expected_repository_path)).to be(true)
-          expect(File.directory?(expected_upload_path)).to be(true)
-          expect(File.directory?(expected_pages_path)).to be(true)
+        context 'moving from having a parent to root' do
+          it 'correctly moves the repository, uploads and pages' do
+            child.update!(parent: nil)
+
+            expect_project_directories_at('child')
+          end
         end
+
+        context 'moving from root to having a parent' do
+          it 'correctly moves the repository, uploads and pages' do
+            parent.update!(parent: new_parent)
+
+            expect_project_directories_at('new_parent/parent/child')
+          end
+        end
+      end
+    end
+
+    context 'hashed storage' do
+      let(:namespace) { create(:namespace) }
+      let!(:project) { create(:project_empty_repo, :hashed, namespace: namespace) }
+
+      it_behaves_like 'namespace restrictions'
+
+      it "repository directory remains unchanged if path changed" do
+        before_disk_path = project.disk_path
+        namespace.update_attributes(path: namespace.full_path + '_new')
+
+        expect(before_disk_path).to eq(project.disk_path)
+        expect(gitlab_shell.exists?(project.repository_storage_path, "#{project.disk_path}.git")).to be_truthy
       end
     end
 
@@ -244,7 +289,7 @@ describe Namespace do
       parent = create(:group, name: 'mygroup', path: 'mygroup')
       subgroup = create(:group, name: 'mysubgroup', path: 'mysubgroup', parent: parent)
       project_in_parent_group = create(:project, :repository, namespace: parent, name: 'foo1')
-      hashed_project_in_subgroup = create(:project, :repository, :hashed, namespace: subgroup, name: 'foo2')
+      hashed_project_in_subgroup = create(:project, :hashed, :repository, namespace: subgroup, name: 'foo2')
       legacy_project_in_subgroup = create(:project, :repository, namespace: subgroup, name: 'foo3')
 
       parent.update(path: 'mygroup_new')
@@ -481,7 +526,6 @@ describe Namespace do
       end
     end
 
-    # Note: Group transfers are not yet implemented
     context 'when a group is transferred into a root group' do
       context 'when the root group "Share with group lock" is enabled' do
         let(:root_group) { create(:group, share_with_group_lock: true) }

spec/models/repository_spec.rb

@@ -1419,7 +1419,6 @@ describe Repository do
     it 'expires the caches for an empty repository' do
       allow(repository).to receive(:empty?).and_return(true)
 
-      expect(cache).to receive(:expire).with(:empty?)
       expect(cache).to receive(:expire).with(:has_visible_content?)
 
       repository.expire_emptiness_caches
@@ -1428,7 +1427,6 @@ describe Repository do
     it 'does not expire the cache for a non-empty repository' do
       allow(repository).to receive(:empty?).and_return(false)
 
-      expect(cache).not_to receive(:expire).with(:empty?)
       expect(cache).not_to receive(:expire).with(:has_visible_content?)
 
       repository.expire_emptiness_caches