summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDouwe Maan <douwe@gitlab.com>2017-05-25 17:27:25 +0000
committerTimothy Andrew <mail@timothyandrew.net>2017-05-31 04:01:48 +0000
commit2c6fc0fff6204b20ea3cdd7b8c579692ac2b0ca5 (patch)
tree8fdecfc1ffd660306aafbd4e424f2385771c2aa3
parent88d0ccd551f0334304de665ece94ba9810c60de7 (diff)
downloadgitlab-ce-2c6fc0fff6204b20ea3cdd7b8c579692ac2b0ca5.tar.gz
Merge branch 'bvl-security-9-2-28917-contain-uploads-in-system-dir' into 'security-9-2'
(security-9-2) Upload files into `public/upload/system` instead of `public/upload` See merge request !2104 Conflicts: app/validators/dynamic_path_validator.rb Fixed conflicts based on 3c7c859c359bf5d3955dd300d6861ff33af21ca7
-rw-r--r--app/uploaders/file_uploader.rb7
-rw-r--r--app/uploaders/gitlab_uploader.rb20
-rw-r--r--changelogs/unreleased/28917-contain-uploads-in-system-dir.yml4
-rw-r--r--config/routes/uploads.rb4
-rw-r--r--db/migrate/20170316163800_rename_system_namespaces.rb231
-rw-r--r--db/migrate/20170316163845_move_uploads_to_system_dir.rb59
-rw-r--r--db/post_migrate/20170317162059_update_upload_paths_to_system.rb55
-rw-r--r--db/post_migrate/20170406111121_clean_upload_symlinks.rb52
-rw-r--r--features/steps/groups.rb2
-rw-r--r--features/steps/profile/profile.rb2
-rw-r--r--features/steps/project/project.rb2
-rw-r--r--spec/factories/uploads.rb8
-rw-r--r--spec/features/admin/admin_appearance_spec.rb4
-rw-r--r--spec/features/uploads/user_uploads_avatar_to_group_spec.rb2
-rw-r--r--spec/features/uploads/user_uploads_avatar_to_profile_spec.rb2
-rw-r--r--spec/helpers/application_helper_spec.rb9
-rw-r--r--spec/helpers/emails_helper_spec.rb2
-rw-r--r--spec/helpers/groups_helper_spec.rb2
-rw-r--r--spec/helpers/page_layout_helper_spec.rb2
-rw-r--r--spec/javascripts/vue_shared/components/commit_spec.js4
-rw-r--r--spec/migrations/clean_upload_symlinks_spec.rb46
-rw-r--r--spec/migrations/move_uploads_to_system_dir_spec.rb68
-rw-r--r--spec/migrations/rename_system_namespaces_spec.rb252
-rw-r--r--spec/migrations/update_upload_paths_to_system_spec.rb53
-rw-r--r--spec/models/group_spec.rb2
-rw-r--r--spec/models/namespace_spec.rb10
-rw-r--r--spec/models/project_spec.rb2
-rw-r--r--spec/models/user_spec.rb2
-rw-r--r--spec/requests/openid_connect_spec.rb2
-rw-r--r--spec/services/projects/participants_service_spec.rb4
-rw-r--r--spec/uploaders/attachment_uploader_spec.rb11
-rw-r--r--spec/uploaders/avatar_uploader_spec.rb11
-rw-r--r--spec/uploaders/file_uploader_spec.rb10
33 files changed, 916 insertions, 30 deletions
diff --git a/app/uploaders/file_uploader.rb b/app/uploaders/file_uploader.rb
index 7e94218c23d..652277e3b78 100644
--- a/app/uploaders/file_uploader.rb
+++ b/app/uploaders/file_uploader.rb
@@ -13,6 +13,13 @@ class FileUploader < GitlabUploader
)
end
+ # Not using `GitlabUploader.base_dir` because all project namespaces are in
+ # the `public/uploads` dir.
+ #
+ def self.base_dir
+ root_dir
+ end
+
# Returns the part of `store_dir` that can change based on the model's current
# path
#
diff --git a/app/uploaders/gitlab_uploader.rb b/app/uploaders/gitlab_uploader.rb
index e0a6c9b4067..449850bf0d5 100644
--- a/app/uploaders/gitlab_uploader.rb
+++ b/app/uploaders/gitlab_uploader.rb
@@ -3,16 +3,28 @@ class GitlabUploader < CarrierWave::Uploader::Base
File.join(CarrierWave.root, upload_record.path)
end
- def self.base_dir
+ def self.root_dir
'uploads'
end
- delegate :base_dir, to: :class
+ # When object storage is used, keep the `root_dir` as `base_dir`.
+ # The files aren't really in folders there, they just have a name.
+ # The files that contain user input in their name, also contain a hash, so
+ # the names are still unique
+ #
+ # This method is overridden in the `FileUploader`
+ def self.base_dir
+ return root_dir unless file_storage?
+
+ File.join(root_dir, 'system')
+ end
- def file_storage?
- self.class.storage == CarrierWave::Storage::File
+ def self.file_storage?
+ self.storage == CarrierWave::Storage::File
end
+ delegate :base_dir, :file_storage?, to: :class
+
# Reduce disk IO
def move_to_cache
true
diff --git a/changelogs/unreleased/28917-contain-uploads-in-system-dir.yml b/changelogs/unreleased/28917-contain-uploads-in-system-dir.yml
new file mode 100644
index 00000000000..cddab46d815
--- /dev/null
+++ b/changelogs/unreleased/28917-contain-uploads-in-system-dir.yml
@@ -0,0 +1,4 @@
+---
+title: Move uploads from 'public/uploads' to 'public/uploads/system'
+merge_request:
+author:
diff --git a/config/routes/uploads.rb b/config/routes/uploads.rb
index b315186b178..ed3fd21d04f 100644
--- a/config/routes/uploads.rb
+++ b/config/routes/uploads.rb
@@ -1,6 +1,6 @@
scope path: :uploads do
# Note attachments and User/Group/Project avatars
- get ":model/:mounted_as/:id/:filename",
+ get "system/:model/:mounted_as/:id/:filename",
to: "uploads#show",
constraints: { model: /note|user|group|project/, mounted_as: /avatar|attachment/, filename: /[^\/]+/ }
@@ -10,7 +10,7 @@ scope path: :uploads do
constraints: { model: /personal_snippet/, id: /\d+/, filename: /[^\/]+/ }
# Appearance
- get ":model/:mounted_as/:id/:filename",
+ get "system/:model/:mounted_as/:id/:filename",
to: "uploads#show",
constraints: { model: /appearance/, mounted_as: /logo|header_logo/, filename: /.+/ }
diff --git a/db/migrate/20170316163800_rename_system_namespaces.rb b/db/migrate/20170316163800_rename_system_namespaces.rb
new file mode 100644
index 00000000000..b5408fbf112
--- /dev/null
+++ b/db/migrate/20170316163800_rename_system_namespaces.rb
@@ -0,0 +1,231 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+class RenameSystemNamespaces < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+ include Gitlab::ShellAdapter
+ disable_ddl_transaction!
+
+ class User < ActiveRecord::Base
+ self.table_name = 'users'
+ end
+
+ class Namespace < ActiveRecord::Base
+ self.table_name = 'namespaces'
+ belongs_to :parent, class_name: 'RenameSystemNamespaces::Namespace'
+ has_one :route, as: :source
+ has_many :children, class_name: 'RenameSystemNamespaces::Namespace', foreign_key: :parent_id
+ belongs_to :owner, class_name: 'RenameSystemNamespaces::User'
+
+ # Overridden to have the correct `source_type` for the `route` relation
+ def self.name
+ 'Namespace'
+ end
+
+ def full_path
+ if route && route.path.present?
+ @full_path ||= route.path
+ else
+ update_route if persisted?
+
+ build_full_path
+ end
+ end
+
+ def build_full_path
+ if parent && path
+ parent.full_path + '/' + path
+ else
+ path
+ end
+ end
+
+ def update_route
+ prepare_route
+ route.save
+ end
+
+ def prepare_route
+ route || build_route(source: self)
+ route.path = build_full_path
+ route.name = build_full_name
+ @full_path = nil
+ @full_name = nil
+ end
+
+ def build_full_name
+ if parent && name
+ parent.human_name + ' / ' + name
+ else
+ name
+ end
+ end
+
+ def human_name
+ owner&.name
+ end
+ end
+
+ class Route < ActiveRecord::Base
+ self.table_name = 'routes'
+ belongs_to :source, polymorphic: true
+ end
+
+ class Project < ActiveRecord::Base
+ self.table_name = 'projects'
+
+ def repository_storage_path
+ Gitlab.config.repositories.storages[repository_storage]['path']
+ end
+ end
+
+ DOWNTIME = false
+
+ def up
+ return unless system_namespace
+
+ old_path = system_namespace.path
+ old_full_path = system_namespace.full_path
+ # Only remove the last occurrence of the path name to get the parent namespace path
+ namespace_path = remove_last_occurrence(old_full_path, old_path)
+ new_path = rename_path(namespace_path, old_path)
+ new_full_path = join_namespace_path(namespace_path, new_path)
+
+ Namespace.where(id: system_namespace).update_all(path: new_path) # skips callbacks & validations
+
+ replace_statement = replace_sql(Route.arel_table[:path], old_full_path, new_full_path)
+ route_matches = [old_full_path, "#{old_full_path}/%"]
+
+ update_column_in_batches(:routes, :path, replace_statement) do |table, query|
+ query.where(Route.arel_table[:path].matches_any(route_matches))
+ end
+
+ clear_cache_for_namespace(system_namespace)
+
+ # tasks here are based on `Namespace#move_dir`
+ move_repositories(system_namespace, old_full_path, new_full_path)
+ move_namespace_folders(uploads_dir, old_full_path, new_full_path) if file_storage?
+ move_namespace_folders(pages_dir, old_full_path, new_full_path)
+ end
+
+ def down
+ # nothing to do
+ end
+
+ def remove_last_occurrence(string, pattern)
+ string.reverse.sub(pattern.reverse, "").reverse
+ end
+
+ def move_namespace_folders(directory, old_relative_path, new_relative_path)
+ old_path = File.join(directory, old_relative_path)
+ return unless File.directory?(old_path)
+
+ new_path = File.join(directory, new_relative_path)
+ FileUtils.mv(old_path, new_path)
+ end
+
+ def move_repositories(namespace, old_full_path, new_full_path)
+ repo_paths_for_namespace(namespace).each do |repository_storage_path|
+ # Ensure old directory exists before moving it
+ gitlab_shell.add_namespace(repository_storage_path, old_full_path)
+
+ unless gitlab_shell.mv_namespace(repository_storage_path, old_full_path, new_full_path)
+ say "Exception moving path #{repository_storage_path} from #{old_full_path} to #{new_full_path}"
+ end
+ end
+ end
+
+ def rename_path(namespace_path, path_was)
+ counter = 0
+ path = "#{path_was}#{counter}"
+
+ while route_exists?(join_namespace_path(namespace_path, path))
+ counter += 1
+ path = "#{path_was}#{counter}"
+ end
+
+ path
+ end
+
+ def route_exists?(full_path)
+ Route.where(Route.arel_table[:path].matches(full_path)).any?
+ end
+
+ def join_namespace_path(namespace_path, path)
+ if namespace_path.present?
+ File.join(namespace_path, path)
+ else
+ path
+ end
+ end
+
+ def system_namespace
+ @system_namespace ||= Namespace.where(parent_id: nil).
+ where(arel_table[:path].matches(system_namespace_path)).
+ first
+ end
+
+ def system_namespace_path
+ "system"
+ end
+
+ def clear_cache_for_namespace(namespace)
+ project_ids = projects_for_namespace(namespace).pluck(:id)
+
+ update_column_in_batches(:projects, :description_html, nil) do |table, query|
+ query.where(table[:id].in(project_ids))
+ end
+
+ update_column_in_batches(:issues, :description_html, nil) do |table, query|
+ query.where(table[:project_id].in(project_ids))
+ end
+
+ update_column_in_batches(:merge_requests, :description_html, nil) do |table, query|
+ query.where(table[:target_project_id].in(project_ids))
+ end
+
+ update_column_in_batches(:notes, :note_html, nil) do |table, query|
+ query.where(table[:project_id].in(project_ids))
+ end
+
+ update_column_in_batches(:milestones, :description_html, nil) do |table, query|
+ query.where(table[:project_id].in(project_ids))
+ end
+ end
+
+ def projects_for_namespace(namespace)
+ namespace_ids = child_ids_for_parent(namespace, ids: [namespace.id])
+ namespace_or_children = Project.arel_table[:namespace_id].in(namespace_ids)
+ Project.unscoped.where(namespace_or_children)
+ end
+
+ # This won't scale to huge trees, but it should do for a handful of namespaces
+ # called `system`.
+ def child_ids_for_parent(namespace, ids: [])
+ namespace.children.each do |child|
+ ids << child.id
+ child_ids_for_parent(child, ids: ids) if child.children.any?
+ end
+ ids
+ end
+
+ def repo_paths_for_namespace(namespace)
+ projects_for_namespace(namespace).distinct.
+ select(:repository_storage).map(&:repository_storage_path)
+ end
+
+ def uploads_dir
+ File.join(Rails.root, "public", "uploads")
+ end
+
+ def pages_dir
+ Settings.pages.path
+ end
+
+ def file_storage?
+ CarrierWave::Uploader::Base.storage == CarrierWave::Storage::File
+ end
+
+ def arel_table
+ Namespace.arel_table
+ end
+end
diff --git a/db/migrate/20170316163845_move_uploads_to_system_dir.rb b/db/migrate/20170316163845_move_uploads_to_system_dir.rb
new file mode 100644
index 00000000000..7115444c35a
--- /dev/null
+++ b/db/migrate/20170316163845_move_uploads_to_system_dir.rb
@@ -0,0 +1,59 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class MoveUploadsToSystemDir < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+ disable_ddl_transaction!
+
+ DOWNTIME = false
+ DIRECTORIES_TO_MOVE = %w(user project note group appeareance)
+
+ def up
+ return unless file_storage?
+
+ FileUtils.mkdir_p(new_upload_dir)
+
+ DIRECTORIES_TO_MOVE.each do |dir|
+ source = File.join(old_upload_dir, dir)
+ destination = File.join(new_upload_dir, dir)
+ next unless File.directory?(source)
+ next if File.directory?(destination)
+
+ say "Moving #{source} -> #{destination}"
+ FileUtils.mv(source, destination)
+ FileUtils.ln_s(destination, source)
+ end
+ end
+
+ def down
+ return unless file_storage?
+ return unless File.directory?(new_upload_dir)
+
+ DIRECTORIES_TO_MOVE.each do |dir|
+ source = File.join(new_upload_dir, dir)
+ destination = File.join(old_upload_dir, dir)
+ next unless File.directory?(source)
+ next if File.directory?(destination) && !File.symlink?(destination)
+
+ say "Moving #{source} -> #{destination}"
+ FileUtils.rm(destination) if File.symlink?(destination)
+ FileUtils.mv(source, destination)
+ end
+ end
+
+ def file_storage?
+ CarrierWave::Uploader::Base.storage == CarrierWave::Storage::File
+ end
+
+ def base_directory
+ Rails.root
+ end
+
+ def old_upload_dir
+ File.join(base_directory, "public", "uploads")
+ end
+
+ def new_upload_dir
+ File.join(base_directory, "public", "uploads", "system")
+ end
+end
diff --git a/db/post_migrate/20170317162059_update_upload_paths_to_system.rb b/db/post_migrate/20170317162059_update_upload_paths_to_system.rb
new file mode 100644
index 00000000000..9a77b0bbdfb
--- /dev/null
+++ b/db/post_migrate/20170317162059_update_upload_paths_to_system.rb
@@ -0,0 +1,55 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class UpdateUploadPathsToSystem < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+ AFFECTED_MODELS = %w(User Project Note Namespace Appearance)
+
+ def up
+ update_column_in_batches(:uploads, :path, replace_sql(arel_table[:path], base_directory, new_upload_dir)) do |_table, query|
+ query.where(uploads_to_switch_to_new_path)
+ end
+ end
+
+ def down
+ update_column_in_batches(:uploads, :path, replace_sql(arel_table[:path], new_upload_dir, base_directory)) do |_table, query|
+ query.where(uploads_to_switch_to_old_path)
+ end
+ end
+
+ # "SELECT \"uploads\".* FROM \"uploads\" WHERE \"uploads\".\"model_type\" IN ('User', 'Project', 'Note', 'Namespace', 'Appearance') AND (\"uploads\".\"path\" ILIKE 'uploads/%' AND NOT (\"uploads\".\"path\" ILIKE 'uploads/system/%'))"
+ def uploads_to_switch_to_new_path
+ affected_uploads.and(starting_with_base_directory).and(starting_with_new_upload_directory.not)
+ end
+
+ # "SELECT \"uploads\".* FROM \"uploads\" WHERE \"uploads\".\"model_type\" IN ('User', 'Project', 'Note', 'Namespace', 'Appearance') AND (\"uploads\".\"path\" ILIKE 'uploads/%' AND \"uploads\".\"path\" ILIKE 'uploads/system/%')"
+ def uploads_to_switch_to_old_path
+ affected_uploads.and(starting_with_new_upload_directory)
+ end
+
+ def starting_with_base_directory
+ arel_table[:path].matches("#{base_directory}/%")
+ end
+
+ def starting_with_new_upload_directory
+ arel_table[:path].matches("#{new_upload_dir}/%")
+ end
+
+ def affected_uploads
+ arel_table[:model_type].in(AFFECTED_MODELS)
+ end
+
+ def base_directory
+ "uploads"
+ end
+
+ def new_upload_dir
+ File.join(base_directory, "system")
+ end
+
+ def arel_table
+ Arel::Table.new(:uploads)
+ end
+end
diff --git a/db/post_migrate/20170406111121_clean_upload_symlinks.rb b/db/post_migrate/20170406111121_clean_upload_symlinks.rb
new file mode 100644
index 00000000000..3ac9a6c10bc
--- /dev/null
+++ b/db/post_migrate/20170406111121_clean_upload_symlinks.rb
@@ -0,0 +1,52 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class CleanUploadSymlinks < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+ disable_ddl_transaction!
+
+ DOWNTIME = false
+ DIRECTORIES_TO_MOVE = %w(user project note group appeareance)
+
+ def up
+ return unless file_storage?
+
+ DIRECTORIES_TO_MOVE.each do |dir|
+ symlink_location = File.join(old_upload_dir, dir)
+ next unless File.symlink?(symlink_location)
+ say "removing symlink: #{symlink_location}"
+ FileUtils.rm(symlink_location)
+ end
+ end
+
+ def down
+ return unless file_storage?
+
+ DIRECTORIES_TO_MOVE.each do |dir|
+ symlink = File.join(old_upload_dir, dir)
+ destination = File.join(new_upload_dir, dir)
+
+ next if File.directory?(symlink)
+ next unless File.directory?(destination)
+
+ say "Creating symlink #{symlink} -> #{destination}"
+ FileUtils.ln_s(destination, symlink)
+ end
+ end
+
+ def file_storage?
+ CarrierWave::Uploader::Base.storage == CarrierWave::Storage::File
+ end
+
+ def base_directory
+ Rails.root
+ end
+
+ def old_upload_dir
+ File.join(base_directory, "public", "uploads")
+ end
+
+ def new_upload_dir
+ File.join(base_directory, "public", "uploads", "system")
+ end
+end
diff --git a/features/steps/groups.rb b/features/steps/groups.rb
index 83d8abbab1f..25bb374b868 100644
--- a/features/steps/groups.rb
+++ b/features/steps/groups.rb
@@ -81,7 +81,7 @@ class Spinach::Features::Groups < Spinach::FeatureSteps
step 'I should see new group "Owned" avatar' do
expect(owned_group.avatar).to be_instance_of AvatarUploader
- expect(owned_group.avatar.url).to eq "/uploads/group/avatar/#{Group.find_by(name: "Owned").id}/banana_sample.gif"
+ expect(owned_group.avatar.url).to eq "/uploads/system/group/avatar/#{Group.find_by(name: "Owned").id}/banana_sample.gif"
end
step 'I should see the "Remove avatar" button' do
diff --git a/features/steps/profile/profile.rb b/features/steps/profile/profile.rb
index 24cfbaad7fe..254c26bb6af 100644
--- a/features/steps/profile/profile.rb
+++ b/features/steps/profile/profile.rb
@@ -36,7 +36,7 @@ class Spinach::Features::Profile < Spinach::FeatureSteps
step 'I should see new avatar' do
expect(@user.avatar).to be_instance_of AvatarUploader
- expect(@user.avatar.url).to eq "/uploads/user/avatar/#{@user.id}/banana_sample.gif"
+ expect(@user.avatar.url).to eq "/uploads/system/user/avatar/#{@user.id}/banana_sample.gif"
end
step 'I should see the "Remove avatar" button' do
diff --git a/features/steps/project/project.rb b/features/steps/project/project.rb
index 280d70925f7..03d6704e1ab 100644
--- a/features/steps/project/project.rb
+++ b/features/steps/project/project.rb
@@ -37,7 +37,7 @@ class Spinach::Features::Project < Spinach::FeatureSteps
step 'I should see new project avatar' do
expect(@project.avatar).to be_instance_of AvatarUploader
url = @project.avatar.url
- expect(url).to eq "/uploads/project/avatar/#{@project.id}/banana_sample.gif"
+ expect(url).to eq "/uploads/system/project/avatar/#{@project.id}/banana_sample.gif"
end
step 'I should see the "Remove avatar" button' do
diff --git a/spec/factories/uploads.rb b/spec/factories/uploads.rb
new file mode 100644
index 00000000000..1383420fb44
--- /dev/null
+++ b/spec/factories/uploads.rb
@@ -0,0 +1,8 @@
+FactoryGirl.define do
+ factory :upload do
+ model { build(:project) }
+ path { "uploads/system/project/avatar/avatar.jpg" }
+ size 100.kilobytes
+ uploader "AvatarUploader"
+ end
+end
diff --git a/spec/features/admin/admin_appearance_spec.rb b/spec/features/admin/admin_appearance_spec.rb
index 96d715ef383..595366ce352 100644
--- a/spec/features/admin/admin_appearance_spec.rb
+++ b/spec/features/admin/admin_appearance_spec.rb
@@ -63,11 +63,11 @@ feature 'Admin Appearance', feature: true do
end
def logo_selector
- '//img[@src^="/uploads/appearance/logo"]'
+ '//img[@src^="/uploads/system/appearance/logo"]'
end
def header_logo_selector
- '//img[@src^="/uploads/appearance/header_logo"]'
+ '//img[@src^="/uploads/system/appearance/header_logo"]'
end
def logo_fixture
diff --git a/spec/features/uploads/user_uploads_avatar_to_group_spec.rb b/spec/features/uploads/user_uploads_avatar_to_group_spec.rb
index f88a515f7fc..d9d6f2e2382 100644
--- a/spec/features/uploads/user_uploads_avatar_to_group_spec.rb
+++ b/spec/features/uploads/user_uploads_avatar_to_group_spec.rb
@@ -18,7 +18,7 @@ feature 'User uploads avatar to group', feature: true do
visit group_path(group)
- expect(page).to have_selector(%Q(img[src$="/uploads/group/avatar/#{group.id}/dk.png"]))
+ expect(page).to have_selector(%Q(img[src$="/uploads/system/group/avatar/#{group.id}/dk.png"]))
# Cheating here to verify something that isn't user-facing, but is important
expect(group.reload.avatar.file).to exist
diff --git a/spec/features/uploads/user_uploads_avatar_to_profile_spec.rb b/spec/features/uploads/user_uploads_avatar_to_profile_spec.rb
index 0dfd29045e5..eb8dbd76aab 100644
--- a/spec/features/uploads/user_uploads_avatar_to_profile_spec.rb
+++ b/spec/features/uploads/user_uploads_avatar_to_profile_spec.rb
@@ -16,7 +16,7 @@ feature 'User uploads avatar to profile', feature: true do
visit user_path(user)
- expect(page).to have_selector(%Q(img[src$="/uploads/user/avatar/#{user.id}/dk.png"]))
+ expect(page).to have_selector(%Q(img[src$="/uploads/system/user/avatar/#{user.id}/dk.png"]))
# Cheating here to verify something that isn't user-facing, but is important
expect(user.reload.avatar.file).to exist
diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb
index 01bdf01ad22..1c4ea46f9cd 100644
--- a/spec/helpers/application_helper_spec.rb
+++ b/spec/helpers/application_helper_spec.rb
@@ -1,3 +1,4 @@
+# coding: utf-8
require 'spec_helper'
describe ApplicationHelper do
@@ -57,7 +58,7 @@ describe ApplicationHelper do
it 'returns an url for the avatar' do
project = create(:empty_project, avatar: File.open(uploaded_image_temp_path))
- avatar_url = "http://#{Gitlab.config.gitlab.host}/uploads/project/avatar/#{project.id}/banana_sample.gif"
+ avatar_url = "http://#{Gitlab.config.gitlab.host}/uploads/system/project/avatar/#{project.id}/banana_sample.gif"
expect(helper.project_icon(project.full_path).to_s).
to eq "<img src=\"#{avatar_url}\" alt=\"Banana sample\" />"
end
@@ -78,7 +79,7 @@ describe ApplicationHelper do
user = create(:user, avatar: File.open(uploaded_image_temp_path))
expect(helper.avatar_icon(user.email).to_s).
- to match("/uploads/user/avatar/#{user.id}/banana_sample.gif")
+ to match("/uploads/system/user/avatar/#{user.id}/banana_sample.gif")
end
it 'returns an url for the avatar with relative url' do
@@ -89,7 +90,7 @@ describe ApplicationHelper do
user = create(:user, avatar: File.open(uploaded_image_temp_path))
expect(helper.avatar_icon(user.email).to_s).
- to match("/gitlab/uploads/user/avatar/#{user.id}/banana_sample.gif")
+ to match("/gitlab/uploads/system/user/avatar/#{user.id}/banana_sample.gif")
end
it 'calls gravatar_icon when no User exists with the given email' do
@@ -103,7 +104,7 @@ describe ApplicationHelper do
user = create(:user, avatar: File.open(uploaded_image_temp_path))
expect(helper.avatar_icon(user).to_s).
- to match("/uploads/user/avatar/#{user.id}/banana_sample.gif")
+ to match("/uploads/system/user/avatar/#{user.id}/banana_sample.gif")
end
end
end
diff --git a/spec/helpers/emails_helper_spec.rb b/spec/helpers/emails_helper_spec.rb
index cd112dbb2fb..c68e4f56b05 100644
--- a/spec/helpers/emails_helper_spec.rb
+++ b/spec/helpers/emails_helper_spec.rb
@@ -52,7 +52,7 @@ describe EmailsHelper do
)
expect(header_logo).to eq(
- %{<img style="height: 50px" src="/uploads/appearance/header_logo/#{appearance.id}/dk.png" alt="Dk" />}
+ %{<img style="height: 50px" src="/uploads/system/appearance/header_logo/#{appearance.id}/dk.png" alt="Dk" />}
)
end
end
diff --git a/spec/helpers/groups_helper_spec.rb b/spec/helpers/groups_helper_spec.rb
index c8b0d86425f..0337afa4452 100644
--- a/spec/helpers/groups_helper_spec.rb
+++ b/spec/helpers/groups_helper_spec.rb
@@ -9,7 +9,7 @@ describe GroupsHelper do
group.avatar = fixture_file_upload(avatar_file_path)
group.save!
expect(group_icon(group.path).to_s).
- to match("/uploads/group/avatar/#{group.id}/banana_sample.gif")
+ to match("/uploads/system/group/avatar/#{group.id}/banana_sample.gif")
end
it 'gives default avatar_icon when no avatar is present' do
diff --git a/spec/helpers/page_layout_helper_spec.rb b/spec/helpers/page_layout_helper_spec.rb
index 2cc0b40b2d0..dff2784f21f 100644
--- a/spec/helpers/page_layout_helper_spec.rb
+++ b/spec/helpers/page_layout_helper_spec.rb
@@ -60,7 +60,7 @@ describe PageLayoutHelper do
%w(project user group).each do |type|
context "with @#{type} assigned" do
it "uses #{type.titlecase} avatar if available" do
- object = double(avatar_url: 'http://example.com/uploads/avatar.png')
+ object = double(avatar_url: 'http://example.com/uploads/system/avatar.png')
assign(type, object)
expect(helper.page_image).to eq object.avatar_url
diff --git a/spec/javascripts/vue_shared/components/commit_spec.js b/spec/javascripts/vue_shared/components/commit_spec.js
index df547299d75..01dabf5320e 100644
--- a/spec/javascripts/vue_shared/components/commit_spec.js
+++ b/spec/javascripts/vue_shared/components/commit_spec.js
@@ -22,7 +22,7 @@ describe('Commit component', () => {
shortSha: 'b7836edd',
title: 'Commit message',
author: {
- avatar_url: 'https://gitlab.com/uploads/user/avatar/300478/avatar.png',
+ avatar_url: 'https://gitlab.com/uploads/system/user/avatar/300478/avatar.png',
web_url: 'https://gitlab.com/jschatz1',
username: 'jschatz1',
},
@@ -44,7 +44,7 @@ describe('Commit component', () => {
shortSha: 'b7836edd',
title: 'Commit message',
author: {
- avatar_url: 'https://gitlab.com/uploads/user/avatar/300478/avatar.png',
+ avatar_url: 'https://gitlab.com/uploads/system/user/avatar/300478/avatar.png',
web_url: 'https://gitlab.com/jschatz1',
username: 'jschatz1',
},
diff --git a/spec/migrations/clean_upload_symlinks_spec.rb b/spec/migrations/clean_upload_symlinks_spec.rb
new file mode 100644
index 00000000000..cecb3ddac53
--- /dev/null
+++ b/spec/migrations/clean_upload_symlinks_spec.rb
@@ -0,0 +1,46 @@
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20170406111121_clean_upload_symlinks.rb')
+
+describe CleanUploadSymlinks do
+ let(:migration) { described_class.new }
+ let(:test_dir) { File.join(Rails.root, "tmp", "tests", "move_uploads_test") }
+ let(:uploads_dir) { File.join(test_dir, "public", "uploads") }
+ let(:new_uploads_dir) { File.join(uploads_dir, "system") }
+ let(:original_path) { File.join(new_uploads_dir, 'user') }
+ let(:symlink_path) { File.join(uploads_dir, 'user') }
+
+ before do
+ FileUtils.remove_dir(test_dir) if File.directory?(test_dir)
+ FileUtils.mkdir_p(uploads_dir)
+ allow(migration).to receive(:base_directory).and_return(test_dir)
+ allow(migration).to receive(:say)
+ end
+
+ describe "#up" do
+ before do
+ FileUtils.mkdir_p(original_path)
+ FileUtils.ln_s(original_path, symlink_path)
+ end
+
+ it 'removes the symlink' do
+ migration.up
+
+ expect(File.symlink?(symlink_path)).to be(false)
+ end
+ end
+
+ describe '#down' do
+ before do
+ FileUtils.mkdir_p(File.join(original_path))
+ FileUtils.touch(File.join(original_path, 'dummy.file'))
+ end
+
+ it 'creates a symlink' do
+ expected_path = File.join(symlink_path, "dummy.file")
+ migration.down
+
+ expect(File.exist?(expected_path)).to be(true)
+ expect(File.symlink?(symlink_path)).to be(true)
+ end
+ end
+end
diff --git a/spec/migrations/move_uploads_to_system_dir_spec.rb b/spec/migrations/move_uploads_to_system_dir_spec.rb
new file mode 100644
index 00000000000..37d66452447
--- /dev/null
+++ b/spec/migrations/move_uploads_to_system_dir_spec.rb
@@ -0,0 +1,68 @@
+require "spec_helper"
+require Rails.root.join("db", "migrate", "20170316163845_move_uploads_to_system_dir.rb")
+
+describe MoveUploadsToSystemDir do
+ let(:migration) { described_class.new }
+ let(:test_dir) { File.join(Rails.root, "tmp", "move_uploads_test") }
+ let(:uploads_dir) { File.join(test_dir, "public", "uploads") }
+ let(:new_uploads_dir) { File.join(uploads_dir, "system") }
+
+ before do
+ FileUtils.remove_dir(test_dir) if File.directory?(test_dir)
+ FileUtils.mkdir_p(uploads_dir)
+ allow(migration).to receive(:base_directory).and_return(test_dir)
+ allow(migration).to receive(:say)
+ end
+
+ describe "#up" do
+ before do
+ FileUtils.mkdir_p(File.join(uploads_dir, 'user'))
+ FileUtils.touch(File.join(uploads_dir, 'user', 'dummy.file'))
+ end
+
+ it 'moves the directory to the new path' do
+ expected_path = File.join(new_uploads_dir, 'user', 'dummy.file')
+
+ migration.up
+
+ expect(File.exist?(expected_path)).to be(true)
+ end
+
+ it 'creates a symlink in the old location' do
+ symlink_path = File.join(uploads_dir, 'user')
+ expected_path = File.join(symlink_path, 'dummy.file')
+
+ migration.up
+
+ expect(File.exist?(expected_path)).to be(true)
+ expect(File.symlink?(symlink_path)).to be(true)
+ end
+ end
+
+ describe "#down" do
+ before do
+ FileUtils.mkdir_p(File.join(new_uploads_dir, 'user'))
+ FileUtils.touch(File.join(new_uploads_dir, 'user', 'dummy.file'))
+ end
+
+ it 'moves the directory to the old path' do
+ expected_path = File.join(uploads_dir, 'user', 'dummy.file')
+
+ migration.down
+
+ expect(File.exist?(expected_path)).to be(true)
+ end
+
+ it 'removes the symlink if it existed' do
+ FileUtils.ln_s(File.join(new_uploads_dir, 'user'), File.join(uploads_dir, 'user'))
+
+ directory = File.join(uploads_dir, 'user')
+ expected_path = File.join(directory, 'dummy.file')
+
+ migration.down
+
+ expect(File.exist?(expected_path)).to be(true)
+ expect(File.symlink?(directory)).to be(false)
+ end
+ end
+end
diff --git a/spec/migrations/rename_system_namespaces_spec.rb b/spec/migrations/rename_system_namespaces_spec.rb
new file mode 100644
index 00000000000..ad1b83d8e2e
--- /dev/null
+++ b/spec/migrations/rename_system_namespaces_spec.rb
@@ -0,0 +1,252 @@
+require "spec_helper"
+require Rails.root.join("db", "migrate", "20170316163800_rename_system_namespaces.rb")
+
+describe RenameSystemNamespaces, truncate: true do
+ let(:migration) { described_class.new }
+ let(:test_dir) { File.join(Rails.root, "tmp", "tests", "rename_namespaces_test") }
+ let(:uploads_dir) { File.join(test_dir, "public", "uploads") }
+ let(:system_namespace) do
+ namespace = build(:namespace, path: "system")
+ namespace.save(validate: false)
+ namespace
+ end
+
+ def save_invalid_routable(routable)
+ routable.__send__(:prepare_route)
+ routable.save(validate: false)
+ end
+
+ before do
+ FileUtils.remove_dir(test_dir) if File.directory?(test_dir)
+ FileUtils.mkdir_p(uploads_dir)
+ FileUtils.remove_dir(TestEnv.repos_path) if File.directory?(TestEnv.repos_path)
+ allow(migration).to receive(:say)
+ allow(migration).to receive(:uploads_dir).and_return(uploads_dir)
+ end
+
+ describe "#system_namespace" do
+ it "only root namespaces called with path `system`" do
+ system_namespace
+ system_namespace_with_parent = build(:namespace, path: 'system', parent: create(:namespace))
+ system_namespace_with_parent.save(validate: false)
+
+ expect(migration.system_namespace.id).to eq(system_namespace.id)
+ end
+ end
+
+ describe "#up" do
+ before do
+ system_namespace
+ end
+
+ it "doesn't break if there are no namespaces called system" do
+ Namespace.delete_all
+
+ migration.up
+ end
+
+ it "renames namespaces called system" do
+ migration.up
+
+ expect(system_namespace.reload.path).to eq("system0")
+ end
+
+ it "renames the route to the namespace" do
+ migration.up
+
+ expect(system_namespace.reload.full_path).to eq("system0")
+ end
+
+ it "renames the route for projects of the namespace" do
+ project = build(:project, path: "project-path", namespace: system_namespace)
+ save_invalid_routable(project)
+
+ migration.up
+
+ expect(project.route.reload.path).to eq("system0/project-path")
+ end
+
+ it "doesn't touch routes of namespaces that look like system" do
+ namespace = create(:group, path: 'systemlookalike')
+ project = create(:project, namespace: namespace, path: 'the-project')
+
+ migration.up
+
+ expect(project.route.reload.path).to eq('systemlookalike/the-project')
+ expect(namespace.route.reload.path).to eq('systemlookalike')
+ end
+
+ it "moves the the repository for a project in the namespace" do
+ project = build(:project, namespace: system_namespace, path: "system-project")
+ save_invalid_routable(project)
+ TestEnv.copy_repo(project)
+ expected_repo = File.join(TestEnv.repos_path, "system0", "system-project.git")
+
+ migration.up
+
+ expect(File.directory?(expected_repo)).to be(true)
+ end
+
+ it "moves the uploads for the namespace" do
+ allow(migration).to receive(:move_namespace_folders).with(Settings.pages.path, "system", "system0")
+ expect(migration).to receive(:move_namespace_folders).with(uploads_dir, "system", "system0")
+
+ migration.up
+ end
+
+ it "moves the pages for the namespace" do
+ allow(migration).to receive(:move_namespace_folders).with(uploads_dir, "system", "system0")
+ expect(migration).to receive(:move_namespace_folders).with(Settings.pages.path, "system", "system0")
+
+ migration.up
+ end
+
+ describe "clears the markdown cache for projects in the system namespace" do
+ let!(:project) do
+ project = build(:project, namespace: system_namespace)
+ save_invalid_routable(project)
+ project
+ end
+
+ it 'removes description_html from projects' do
+ migration.up
+
+ expect(project.reload.description_html).to be_nil
+ end
+
+ it 'removes issue descriptions' do
+ issue = create(:issue, project: project, description_html: 'Issue description')
+
+ migration.up
+
+ expect(issue.reload.description_html).to be_nil
+ end
+
+ it 'removes merge request descriptions' do
+ merge_request = create(:merge_request,
+ source_project: project,
+ target_project: project,
+ description_html: 'MergeRequest description')
+
+ migration.up
+
+ expect(merge_request.reload.description_html).to be_nil
+ end
+
+ it 'removes note html' do
+ note = create(:note,
+ project: project,
+ noteable: create(:issue, project: project),
+ note_html: 'note description')
+
+ migration.up
+
+ expect(note.reload.note_html).to be_nil
+ end
+
+ it 'removes milestone description' do
+ milestone = create(:milestone,
+ project: project,
+ description_html: 'milestone description')
+
+ migration.up
+
+ expect(milestone.reload.description_html).to be_nil
+ end
+ end
+
+ context "system namespace -> subgroup -> system0 project" do
+ it "updates the route of the project correctly" do
+ subgroup = build(:group, path: "subgroup", parent: system_namespace)
+ save_invalid_routable(subgroup)
+ project = build(:project, path: "system0", namespace: subgroup)
+ save_invalid_routable(project)
+
+ migration.up
+
+ expect(project.route.reload.path).to eq("system0/subgroup/system0")
+ end
+ end
+ end
+
+ describe "#move_repositories" do
+ let(:namespace) { create(:group, name: "hello-group") }
+ it "moves a project for a namespace" do
+ create(:project, namespace: namespace, path: "hello-project")
+ expected_path = File.join(TestEnv.repos_path, "bye-group", "hello-project.git")
+
+ migration.move_repositories(namespace, "hello-group", "bye-group")
+
+ expect(File.directory?(expected_path)).to be(true)
+ end
+
+ it "moves a namespace in a subdirectory correctly" do
+ child_namespace = create(:group, name: "sub-group", parent: namespace)
+ create(:project, namespace: child_namespace, path: "hello-project")
+
+ expected_path = File.join(TestEnv.repos_path, "hello-group", "renamed-sub-group", "hello-project.git")
+
+ migration.move_repositories(child_namespace, "hello-group/sub-group", "hello-group/renamed-sub-group")
+
+ expect(File.directory?(expected_path)).to be(true)
+ end
+
+ it "moves a parent namespace with subdirectories" do
+ child_namespace = create(:group, name: "sub-group", parent: namespace)
+ create(:project, namespace: child_namespace, path: "hello-project")
+ expected_path = File.join(TestEnv.repos_path, "renamed-group", "sub-group", "hello-project.git")
+
+ migration.move_repositories(child_namespace, "hello-group", "renamed-group")
+
+ expect(File.directory?(expected_path)).to be(true)
+ end
+ end
+
+ describe "#move_namespace_folders" do
+ it "moves a namespace with files" do
+ source = File.join(uploads_dir, "parent-group", "sub-group")
+ FileUtils.mkdir_p(source)
+ destination = File.join(uploads_dir, "parent-group", "moved-group")
+ FileUtils.touch(File.join(source, "test.txt"))
+ expected_file = File.join(destination, "test.txt")
+
+ migration.move_namespace_folders(uploads_dir, File.join("parent-group", "sub-group"), File.join("parent-group", "moved-group"))
+
+ expect(File.exist?(expected_file)).to be(true)
+ end
+
+ it "moves a parent namespace uploads" do
+ source = File.join(uploads_dir, "parent-group", "sub-group")
+ FileUtils.mkdir_p(source)
+ destination = File.join(uploads_dir, "moved-parent", "sub-group")
+ FileUtils.touch(File.join(source, "test.txt"))
+ expected_file = File.join(destination, "test.txt")
+
+ migration.move_namespace_folders(uploads_dir, "parent-group", "moved-parent")
+
+ expect(File.exist?(expected_file)).to be(true)
+ end
+ end
+
+ describe "#child_ids_for_parent" do
+ it "collects child ids for all levels" do
+ parent = create(:namespace)
+ first_child = create(:namespace, parent: parent)
+ second_child = create(:namespace, parent: parent)
+ third_child = create(:namespace, parent: second_child)
+ all_ids = [parent.id, first_child.id, second_child.id, third_child.id]
+
+ collected_ids = migration.child_ids_for_parent(parent, ids: [parent.id])
+
+ expect(collected_ids).to contain_exactly(*all_ids)
+ end
+ end
+
+ describe "#remove_last_ocurrence" do
+ it "removes only the last occurance of a string" do
+ input = "this/is/system/namespace/with/system"
+
+ expect(migration.remove_last_occurrence(input, "system")).to eq("this/is/system/namespace/with/")
+ end
+ end
+end
diff --git a/spec/migrations/update_upload_paths_to_system_spec.rb b/spec/migrations/update_upload_paths_to_system_spec.rb
new file mode 100644
index 00000000000..7df44515424
--- /dev/null
+++ b/spec/migrations/update_upload_paths_to_system_spec.rb
@@ -0,0 +1,53 @@
+require "spec_helper"
+require Rails.root.join("db", "post_migrate", "20170317162059_update_upload_paths_to_system.rb")
+
+describe UpdateUploadPathsToSystem do
+ let(:migration) { described_class.new }
+
+ before do
+ allow(migration).to receive(:say)
+ end
+
+ describe "#uploads_to_switch_to_new_path" do
+ it "contains only uploads with the old path for the correct models" do
+ _upload_for_other_type = create(:upload, model: create(:ci_pipeline), path: "uploads/ci_pipeline/avatar.jpg")
+ _upload_with_system_path = create(:upload, model: create(:empty_project), path: "uploads/system/project/avatar.jpg")
+ _upload_with_other_path = create(:upload, model: create(:empty_project), path: "thelongsecretforafileupload/avatar.jpg")
+ old_upload = create(:upload, model: create(:empty_project), path: "uploads/project/avatar.jpg")
+ group_upload = create(:upload, model: create(:group), path: "uploads/group/avatar.jpg")
+
+ expect(Upload.where(migration.uploads_to_switch_to_new_path)).to contain_exactly(old_upload, group_upload)
+ end
+ end
+
+ describe "#uploads_to_switch_to_old_path" do
+ it "contains only uploads with the new path for the correct models" do
+ _upload_for_other_type = create(:upload, model: create(:ci_pipeline), path: "uploads/ci_pipeline/avatar.jpg")
+ upload_with_system_path = create(:upload, model: create(:empty_project), path: "uploads/system/project/avatar.jpg")
+ _upload_with_other_path = create(:upload, model: create(:empty_project), path: "thelongsecretforafileupload/avatar.jpg")
+ _old_upload = create(:upload, model: create(:empty_project), path: "uploads/project/avatar.jpg")
+
+ expect(Upload.where(migration.uploads_to_switch_to_old_path)).to contain_exactly(upload_with_system_path)
+ end
+ end
+
+ describe "#up", truncate: true do
+ it "updates old upload records to the new path" do
+ old_upload = create(:upload, model: create(:empty_project), path: "uploads/project/avatar.jpg")
+
+ migration.up
+
+ expect(old_upload.reload.path).to eq("uploads/system/project/avatar.jpg")
+ end
+ end
+
+ describe "#down", truncate: true do
+ it "updates the new system patsh to the old paths" do
+ new_upload = create(:upload, model: create(:empty_project), path: "uploads/system/project/avatar.jpg")
+
+ migration.down
+
+ expect(new_upload.reload.path).to eq("uploads/project/avatar.jpg")
+ end
+ end
+end
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index 3d60e52f23f..365df733c30 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -185,7 +185,7 @@ describe Group, models: true do
group.add_master(user)
end
- let(:avatar_path) { "/uploads/group/avatar/#{group.id}/dk.png" }
+ let(:avatar_path) { "/uploads/system/group/avatar/#{group.id}/dk.png" }
it { should eq "http://#{Gitlab.config.gitlab.host}#{avatar_path}" }
end
diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb
index 312302afdbb..5fc43eab283 100644
--- a/spec/models/namespace_spec.rb
+++ b/spec/models/namespace_spec.rb
@@ -43,6 +43,12 @@ describe Namespace, models: true do
end
end
+ context "is case insensitive" do
+ let(:group) { build(:group, path: "System") }
+
+ it { expect(group).not_to be_valid }
+ end
+
context 'top-level group' do
let(:group) { build(:group, path: 'tree') }
@@ -178,8 +184,8 @@ describe Namespace, models: true do
let(:parent) { create(:group, name: 'parent', path: 'parent') }
let(:child) { create(:group, name: 'child', path: 'child', parent: parent) }
let!(:project) { create(:project_empty_repo, path: 'the-project', namespace: child) }
- let(:uploads_dir) { File.join(CarrierWave.root, 'uploads') }
- let(:pages_dir) { TestEnv.pages_path }
+ let(:uploads_dir) { File.join(CarrierWave.root, GitlabUploader.base_dir) }
+ let(:pages_dir) { File.join(TestEnv.pages_path) }
before do
FileUtils.mkdir_p(File.join(uploads_dir, 'parent', 'child', 'the-project'))
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 429b3dd83af..692f28ea5e7 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -812,7 +812,7 @@ describe Project, models: true do
context 'when avatar file is uploaded' do
let(:project) { create(:empty_project, :with_avatar) }
- let(:avatar_path) { "/uploads/project/avatar/#{project.id}/dk.png" }
+ let(:avatar_path) { "/uploads/system/project/avatar/#{project.id}/dk.png" }
it { should eq "http://#{Gitlab.config.gitlab.host}#{avatar_path}" }
end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 9be4996192b..76a66888658 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -948,7 +948,7 @@ describe User, models: true do
subject { user.avatar_url }
context 'when avatar file is uploaded' do
- let(:avatar_path) { "/uploads/user/avatar/#{user.id}/dk.png" }
+ let(:avatar_path) { "/uploads/system/user/avatar/#{user.id}/dk.png" }
it { should eq "http://#{Gitlab.config.gitlab.host}#{avatar_path}" }
end
diff --git a/spec/requests/openid_connect_spec.rb b/spec/requests/openid_connect_spec.rb
index a4f85c22943..75d8fc92a43 100644
--- a/spec/requests/openid_connect_spec.rb
+++ b/spec/requests/openid_connect_spec.rb
@@ -79,7 +79,7 @@ describe 'OpenID Connect requests' do
'email_verified' => true,
'website' => 'https://example.com',
'profile' => 'http://localhost/alice',
- 'picture' => "http://localhost/uploads/user/avatar/#{user.id}/dk.png",
+ 'picture' => "http://localhost/uploads/system/user/avatar/#{user.id}/dk.png",
})
end
end
diff --git a/spec/services/projects/participants_service_spec.rb b/spec/services/projects/participants_service_spec.rb
index 063b3bd76eb..d524c9aff17 100644
--- a/spec/services/projects/participants_service_spec.rb
+++ b/spec/services/projects/participants_service_spec.rb
@@ -14,7 +14,7 @@ describe Projects::ParticipantsService, services: true do
groups = participants.groups
expect(groups.size).to eq 1
- expect(groups.first[:avatar_url]).to eq "#{base_url}/uploads/group/avatar/#{group.id}/dk.png"
+ expect(groups.first[:avatar_url]).to eq "#{base_url}/uploads/system/group/avatar/#{group.id}/dk.png"
end
it 'should return an url for the avatar with relative url' do
@@ -25,7 +25,7 @@ describe Projects::ParticipantsService, services: true do
groups = participants.groups
expect(groups.size).to eq 1
- expect(groups.first[:avatar_url]).to eq "#{base_url}/gitlab/uploads/group/avatar/#{group.id}/dk.png"
+ expect(groups.first[:avatar_url]).to eq "#{base_url}/gitlab/uploads/system/group/avatar/#{group.id}/dk.png"
end
end
end
diff --git a/spec/uploaders/attachment_uploader_spec.rb b/spec/uploaders/attachment_uploader_spec.rb
index ea714fb08f0..d82dbe871d5 100644
--- a/spec/uploaders/attachment_uploader_spec.rb
+++ b/spec/uploaders/attachment_uploader_spec.rb
@@ -3,6 +3,17 @@ require 'spec_helper'
describe AttachmentUploader do
let(:uploader) { described_class.new(build_stubbed(:user)) }
+ describe "#store_dir" do
+ it "stores in the system dir" do
+ expect(uploader.store_dir).to start_with("uploads/system/user")
+ end
+
+ it "uses the old path when using object storage" do
+ expect(described_class).to receive(:file_storage?).and_return(false)
+ expect(uploader.store_dir).to start_with("uploads/user")
+ end
+ end
+
describe '#move_to_cache' do
it 'is true' do
expect(uploader.move_to_cache).to eq(true)
diff --git a/spec/uploaders/avatar_uploader_spec.rb b/spec/uploaders/avatar_uploader_spec.rb
index c4d558805ab..201fe6949aa 100644
--- a/spec/uploaders/avatar_uploader_spec.rb
+++ b/spec/uploaders/avatar_uploader_spec.rb
@@ -3,6 +3,17 @@ require 'spec_helper'
describe AvatarUploader do
let(:uploader) { described_class.new(build_stubbed(:user)) }
+ describe "#store_dir" do
+ it "stores in the system dir" do
+ expect(uploader.store_dir).to start_with("uploads/system/user")
+ end
+
+ it "uses the old path when using object storage" do
+ expect(described_class).to receive(:file_storage?).and_return(false)
+ expect(uploader.store_dir).to start_with("uploads/user")
+ end
+ end
+
describe '#move_to_cache' do
it 'is false' do
expect(uploader.move_to_cache).to eq(false)
diff --git a/spec/uploaders/file_uploader_spec.rb b/spec/uploaders/file_uploader_spec.rb
index d9113ef4095..47e9365e13d 100644
--- a/spec/uploaders/file_uploader_spec.rb
+++ b/spec/uploaders/file_uploader_spec.rb
@@ -15,6 +15,16 @@ describe FileUploader do
end
end
+ describe "#store_dir" do
+ it "stores in the namespace path" do
+ project = build_stubbed(:empty_project)
+ uploader = described_class.new(project)
+
+ expect(uploader.store_dir).to include(project.path_with_namespace)
+ expect(uploader.store_dir).not_to include("system")
+ end
+ end
+
describe 'initialize' do
it 'generates a secret if none is provided' do
expect(SecureRandom).to receive(:hex).and_return('secret')