From 200e8582f8fc0c7949f052673b25e1aff1490f7e Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Tue, 30 May 2017 18:30:42 +0000 Subject: Revert "Merge remote-tracking branch 'dev/security-9-1' into 9-1-stable" This reverts commit cc1b069ff9bfee3374b005e588b03de10afba689, reversing changes made to 729c75f700b75ea7b67e61ab01694f9d12623af1. --- app/controllers/autocomplete_controller.rb | 2 +- app/uploaders/file_uploader.rb | 7 - app/uploaders/gitlab_uploader.rb | 20 +- app/validators/namespace_validator.rb | 5 +- app/validators/project_path_validator.rb | 2 +- .../28917-contain-uploads-in-system-dir.yml | 4 - changelogs/unreleased/bvl-markup-pipeline.yml | 4 - .../bvl-validate-urls-in-markdown-using-uri.yml | 4 - changelogs/unreleased/dz-api-x-frame.yml | 4 - changelogs/unreleased/dz-restrict-autocomplete.yml | 4 - changelogs/unreleased/hamlit-xss-fix.yml | 4 - config/routes/uploads.rb | 4 +- .../20170316163800_rename_system_namespaces.rb | 231 -------------------- .../20170316163845_move_uploads_to_system_dir.rb | 59 ----- ...20170317162059_update_upload_paths_to_system.rb | 55 ----- .../20170406111121_clean_upload_symlinks.rb | 52 ----- features/steps/groups.rb | 2 +- features/steps/profile/profile.rb | 2 +- features/steps/project/project.rb | 2 +- lib/api/api.rb | 1 - lib/gitlab/database/migration_helpers.rb | 25 +-- spec/controllers/autocomplete_controller_spec.rb | 30 +-- spec/factories/uploads.rb | 8 - spec/features/admin/admin_appearance_spec.rb | 4 +- .../uploads/user_uploads_avatar_to_group_spec.rb | 2 +- .../uploads/user_uploads_avatar_to_profile_spec.rb | 2 +- spec/helpers/application_helper_spec.rb | 9 +- spec/helpers/emails_helper_spec.rb | 2 +- spec/helpers/groups_helper_spec.rb | 2 +- spec/helpers/page_layout_helper_spec.rb | 2 +- .../vue_shared/components/commit_spec.js | 2 +- spec/lib/gitlab/database/migration_helpers_spec.rb | 33 --- spec/migrations/clean_upload_symlinks_spec.rb | 46 ---- spec/migrations/move_uploads_to_system_dir_spec.rb | 68 ------ spec/migrations/rename_system_namespaces_spec.rb | 238 --------------------- .../update_upload_paths_to_system_spec.rb | 53 ----- spec/models/namespace_spec.rb | 24 ++- spec/models/project_spec.rb | 2 +- spec/requests/openid_connect_spec.rb | 2 +- .../services/projects/participants_service_spec.rb | 4 +- spec/uploaders/attachment_uploader_spec.rb | 11 - spec/uploaders/avatar_uploader_spec.rb | 11 - spec/uploaders/file_uploader_spec.rb | 10 - 43 files changed, 56 insertions(+), 1002 deletions(-) delete mode 100644 changelogs/unreleased/28917-contain-uploads-in-system-dir.yml delete mode 100644 changelogs/unreleased/bvl-markup-pipeline.yml delete mode 100644 changelogs/unreleased/bvl-validate-urls-in-markdown-using-uri.yml delete mode 100644 changelogs/unreleased/dz-api-x-frame.yml delete mode 100644 changelogs/unreleased/dz-restrict-autocomplete.yml delete mode 100644 changelogs/unreleased/hamlit-xss-fix.yml delete mode 100644 db/migrate/20170316163800_rename_system_namespaces.rb delete mode 100644 db/migrate/20170316163845_move_uploads_to_system_dir.rb delete mode 100644 db/post_migrate/20170317162059_update_upload_paths_to_system.rb delete mode 100644 db/post_migrate/20170406111121_clean_upload_symlinks.rb delete mode 100644 spec/factories/uploads.rb delete mode 100644 spec/migrations/clean_upload_symlinks_spec.rb delete mode 100644 spec/migrations/move_uploads_to_system_dir_spec.rb delete mode 100644 spec/migrations/rename_system_namespaces_spec.rb delete mode 100644 spec/migrations/update_upload_paths_to_system_spec.rb diff --git a/app/controllers/autocomplete_controller.rb b/app/controllers/autocomplete_controller.rb index f94f88305a4..b79ca034c5b 100644 --- a/app/controllers/autocomplete_controller.rb +++ b/app/controllers/autocomplete_controller.rb @@ -21,7 +21,7 @@ class AutocompleteController < ApplicationController @users = [current_user, *@users].uniq end - if params[:author_id].present? && current_user + if params[:author_id].present? author = User.find_by_id(params[:author_id]) @users = [author, *@users].uniq if author end diff --git a/app/uploaders/file_uploader.rb b/app/uploaders/file_uploader.rb index 953feb9b9e8..d2783ce5b2f 100644 --- a/app/uploaders/file_uploader.rb +++ b/app/uploaders/file_uploader.rb @@ -13,13 +13,6 @@ 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 78e9580661b..d662ba6820c 100644 --- a/app/uploaders/gitlab_uploader.rb +++ b/app/uploaders/gitlab_uploader.rb @@ -3,28 +3,16 @@ class GitlabUploader < CarrierWave::Uploader::Base File.join(CarrierWave.root, upload_record.path) end - def self.root_dir + def self.base_dir 'uploads' end - # 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 + delegate :base_dir, to: :class - def self.file_storage? - self.storage == CarrierWave::Storage::File + def file_storage? + self.class.storage == CarrierWave::Storage::File end - delegate :base_dir, :file_storage?, to: :class - # Reduce disk IO def move_to_cache true diff --git a/app/validators/namespace_validator.rb b/app/validators/namespace_validator.rb index 0a1656d867a..77ca033e97f 100644 --- a/app/validators/namespace_validator.rb +++ b/app/validators/namespace_validator.rb @@ -33,7 +33,6 @@ class NamespaceValidator < ActiveModel::EachValidator u unsubscribes users - system ].freeze WILDCARD_ROUTES = %w[tree commits wikis new edit create update logs_tree @@ -48,9 +47,9 @@ class NamespaceValidator < ActiveModel::EachValidator def self.reserved?(value, strict: false) if strict - STRICT_RESERVED.include?(value.to_s.downcase) + STRICT_RESERVED.include?(value) else - RESERVED.include?(value.to_s.downcase) + RESERVED.include?(value) end end diff --git a/app/validators/project_path_validator.rb b/app/validators/project_path_validator.rb index eeee7f8aada..ee2ae65be7b 100644 --- a/app/validators/project_path_validator.rb +++ b/app/validators/project_path_validator.rb @@ -15,7 +15,7 @@ class ProjectPathValidator < ActiveModel::EachValidator # 'tree' as project name and 'deploy_keys' as route. # RESERVED = (NamespaceValidator::STRICT_RESERVED - - %w[dashboard help ci admin search notes services assets profile public system]).freeze + %w[dashboard help ci admin search notes services assets profile public]).freeze def self.valid?(value) !reserved?(value) diff --git a/changelogs/unreleased/28917-contain-uploads-in-system-dir.yml b/changelogs/unreleased/28917-contain-uploads-in-system-dir.yml deleted file mode 100644 index cddab46d815..00000000000 --- a/changelogs/unreleased/28917-contain-uploads-in-system-dir.yml +++ /dev/null @@ -1,4 +0,0 @@ ---- -title: Move uploads from 'public/uploads' to 'public/uploads/system' -merge_request: -author: diff --git a/changelogs/unreleased/bvl-markup-pipeline.yml b/changelogs/unreleased/bvl-markup-pipeline.yml deleted file mode 100644 index d73bad03340..00000000000 --- a/changelogs/unreleased/bvl-markup-pipeline.yml +++ /dev/null @@ -1,4 +0,0 @@ ---- -title: Make Asciidoc & other markup go through pipeline to prevent XSS -merge_request: -author: diff --git a/changelogs/unreleased/bvl-validate-urls-in-markdown-using-uri.yml b/changelogs/unreleased/bvl-validate-urls-in-markdown-using-uri.yml deleted file mode 100644 index 03c4e531d73..00000000000 --- a/changelogs/unreleased/bvl-validate-urls-in-markdown-using-uri.yml +++ /dev/null @@ -1,4 +0,0 @@ ---- -title: Validate URLs in markdown using URI to detect the host correctly -merge_request: -author: diff --git a/changelogs/unreleased/dz-api-x-frame.yml b/changelogs/unreleased/dz-api-x-frame.yml deleted file mode 100644 index 0483a9e076a..00000000000 --- a/changelogs/unreleased/dz-api-x-frame.yml +++ /dev/null @@ -1,4 +0,0 @@ ---- -title: Restrict API X-Frame-Options to same origin -merge_request: -author: diff --git a/changelogs/unreleased/dz-restrict-autocomplete.yml b/changelogs/unreleased/dz-restrict-autocomplete.yml deleted file mode 100644 index 65c944653f8..00000000000 --- a/changelogs/unreleased/dz-restrict-autocomplete.yml +++ /dev/null @@ -1,4 +0,0 @@ ---- -title: Allow users autocomplete by author_id only for authenticated users -merge_request: -author: diff --git a/changelogs/unreleased/hamlit-xss-fix.yml b/changelogs/unreleased/hamlit-xss-fix.yml deleted file mode 100644 index ba4713846e9..00000000000 --- a/changelogs/unreleased/hamlit-xss-fix.yml +++ /dev/null @@ -1,4 +0,0 @@ ---- -title: Fix for XSS in project import view caused by Hamlit filter usage. -merge_request: -author: diff --git a/config/routes/uploads.rb b/config/routes/uploads.rb index 27fdfc08adc..2b22148a134 100644 --- a/config/routes/uploads.rb +++ b/config/routes/uploads.rb @@ -1,11 +1,11 @@ scope path: :uploads do # Note attachments and User/Group/Project avatars - get "system/:model/:mounted_as/:id/:filename", + get ":model/:mounted_as/:id/:filename", to: "uploads#show", constraints: { model: /note|user|group|project/, mounted_as: /avatar|attachment/, filename: /[^\/]+/ } # Appearance - get "system/:model/:mounted_as/:id/:filename", + get ":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 deleted file mode 100644 index b5408fbf112..00000000000 --- a/db/migrate/20170316163800_rename_system_namespaces.rb +++ /dev/null @@ -1,231 +0,0 @@ -# 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 deleted file mode 100644 index 7115444c35a..00000000000 --- a/db/migrate/20170316163845_move_uploads_to_system_dir.rb +++ /dev/null @@ -1,59 +0,0 @@ -# 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 deleted file mode 100644 index 9a77b0bbdfb..00000000000 --- a/db/post_migrate/20170317162059_update_upload_paths_to_system.rb +++ /dev/null @@ -1,55 +0,0 @@ -# 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 deleted file mode 100644 index 3ac9a6c10bc..00000000000 --- a/db/post_migrate/20170406111121_clean_upload_symlinks.rb +++ /dev/null @@ -1,52 +0,0 @@ -# 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 d6135288c7c..4dc87dc4d9c 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/system/group/avatar/#{Group.find_by(name: "Owned").id}/banana_sample.gif" + expect(owned_group.avatar.url).to eq "/uploads/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 254c26bb6af..24cfbaad7fe 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/system/user/avatar/#{@user.id}/banana_sample.gif" + expect(@user.avatar.url).to eq "/uploads/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 71a74da1f91..975c879149e 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/system/project/avatar/#{@project.id}/banana_sample.gif" + expect(url).to eq "/uploads/project/avatar/#{@project.id}/banana_sample.gif" end step 'I should see the "Remove avatar" button' do diff --git a/lib/api/api.rb b/lib/api/api.rb index 6b78443cbcb..1bf20f76ad6 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -44,7 +44,6 @@ module API end before { allow_access_with_scope :api } - before { header['X-Frame-Options'] = 'SAMEORIGIN' } rescue_from Gitlab::Access::AccessDeniedError do rack_response({ 'message' => '403 Forbidden' }.to_json, 403) diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index 40ca88b45a3..a6873ac63a0 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -105,7 +105,7 @@ module Gitlab # here is based on Rails' foreign_key_name() method, which unfortunately # is private so we can't rely on it directly. def concurrent_foreign_key_name(table, column) - "fk_#{Digest::SHA256.hexdigest('#{table}_#{column}_fk').first(10)}" + "fk_#{Digest::SHA256.hexdigest("#{table}_#{column}_fk").first(10)}" end # Long-running migrations may take more than the timeout allowed by @@ -490,29 +490,6 @@ module Gitlab columns(table).find { |column| column.name == name } end - - # This will replace the first occurance of a string in a column with - # the replacement - # On postgresql we can use `regexp_replace` for that. - # On mysql we find the location of the pattern, and overwrite it - # with the replacement - def replace_sql(column, pattern, replacement) - quoted_pattern = Arel::Nodes::Quoted.new(pattern.to_s) - quoted_replacement = Arel::Nodes::Quoted.new(replacement.to_s) - - if Database.mysql? - locate = Arel::Nodes::NamedFunction. - new('locate', [quoted_pattern, column]) - insert_in_place = Arel::Nodes::NamedFunction. - new('insert', [column, locate, pattern.size, quoted_replacement]) - - Arel::Nodes::SqlLiteral.new(insert_in_place.to_sql) - else - replace = Arel::Nodes::NamedFunction. - new("regexp_replace", [column, quoted_pattern, quoted_replacement]) - Arel::Nodes::SqlLiteral.new(replace.to_sql) - end - end end end end diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb index 14b105c69e5..7d2f6dd9d0a 100644 --- a/spec/controllers/autocomplete_controller_spec.rb +++ b/spec/controllers/autocomplete_controller_spec.rb @@ -156,32 +156,22 @@ describe AutocompleteController do end context 'author of issuable included' do - let(:body) { JSON.parse(response.body) } - - context 'authenticated' do - before do - sign_in(user) - end - - it 'includes the author' do - get(:users, author_id: non_member.id) + before do + sign_in(user) + end - expect(body.first["username"]).to eq non_member.username - end + let(:body) { JSON.parse(response.body) } - it 'rejects non existent user ids' do - get(:users, author_id: 99999) + it 'includes the author' do + get(:users, author_id: non_member.id) - expect(body.collect { |u| u['id'] }).not_to include(99999) - end + expect(body.first["username"]).to eq non_member.username end - context 'without authenticating' do - it 'returns empty result' do - get(:users, author_id: non_member.id) + it 'rejects non existent user ids' do + get(:users, author_id: 99999) - expect(body).to be_empty - end + expect(body.collect { |u| u['id'] }).not_to include(99999) end end diff --git a/spec/factories/uploads.rb b/spec/factories/uploads.rb deleted file mode 100644 index 1383420fb44..00000000000 --- a/spec/factories/uploads.rb +++ /dev/null @@ -1,8 +0,0 @@ -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 595366ce352..96d715ef383 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/system/appearance/logo"]' + '//img[@src^="/uploads/appearance/logo"]' end def header_logo_selector - '//img[@src^="/uploads/system/appearance/header_logo"]' + '//img[@src^="/uploads/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 d9d6f2e2382..f88a515f7fc 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/system/group/avatar/#{group.id}/dk.png"])) + expect(page).to have_selector(%Q(img[src$="/uploads/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 eb8dbd76aab..0dfd29045e5 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/system/user/avatar/#{user.id}/dk.png"])) + expect(page).to have_selector(%Q(img[src$="/uploads/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 6c747b4da20..5c07ea8a872 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -1,4 +1,3 @@ -# coding: utf-8 require 'spec_helper' describe ApplicationHelper do @@ -58,7 +57,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/system/project/avatar/#{project.id}/banana_sample.gif" + avatar_url = "http://#{Gitlab.config.gitlab.host}/uploads/project/avatar/#{project.id}/banana_sample.gif" expect(helper.project_icon(project.full_path).to_s). to eq "\"Banana" end @@ -79,7 +78,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/system/user/avatar/#{user.id}/banana_sample.gif") + to match("/uploads/user/avatar/#{user.id}/banana_sample.gif") end it 'returns an url for the avatar with relative url' do @@ -90,7 +89,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/system/user/avatar/#{user.id}/banana_sample.gif") + to match("/gitlab/uploads/user/avatar/#{user.id}/banana_sample.gif") end it 'calls gravatar_icon when no User exists with the given email' do @@ -104,7 +103,7 @@ describe ApplicationHelper do user = create(:user, avatar: File.open(uploaded_image_temp_path)) expect(helper.avatar_icon(user).to_s). - to match("/uploads/system/user/avatar/#{user.id}/banana_sample.gif") + to match("/uploads/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 c68e4f56b05..cd112dbb2fb 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( - %{Dk} + %{Dk} ) end end diff --git a/spec/helpers/groups_helper_spec.rb b/spec/helpers/groups_helper_spec.rb index 0337afa4452..c8b0d86425f 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/system/group/avatar/#{group.id}/banana_sample.gif") + to match("/uploads/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 dff2784f21f..2cc0b40b2d0 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/system/avatar.png') + object = double(avatar_url: 'http://example.com/uploads/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 9410aff90bf..df547299d75 100644 --- a/spec/javascripts/vue_shared/components/commit_spec.js +++ b/spec/javascripts/vue_shared/components/commit_spec.js @@ -44,7 +44,7 @@ describe('Commit component', () => { shortSha: 'b7836edd', title: 'Commit message', author: { - avatar_url: 'https://gitlab.com/uploads/system/user/avatar/300478/avatar.png', + avatar_url: 'https://gitlab.com/uploads/user/avatar/300478/avatar.png', web_url: 'https://gitlab.com/jschatz1', username: 'jschatz1', }, diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index 95b01b3abcf..8d109cba71a 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -682,37 +682,4 @@ describe Gitlab::Database::MigrationHelpers, lib: true do expect(model.column_for(:users, :kittens)).to be_nil end end - - describe '#replace_sql' do - context 'using postgres' do - before do - allow(Gitlab::Database).to receive(:mysql?).and_return(false) - end - - it 'builds the sql with correct functions' do - expect(model.replace_sql(Arel::Table.new(:users)[:first_name], "Alice", "Eve").to_s). - to include('regexp_replace') - end - end - - context 'using mysql' do - before do - allow(Gitlab::Database).to receive(:mysql?).and_return(true) - end - - it 'builds the sql with the correct functions' do - expect(model.replace_sql(Arel::Table.new(:users)[:first_name], "Alice", "Eve").to_s). - to include('locate', 'insert') - end - end - - describe 'results' do - let!(:user) { create(:user, name: 'Kathy Alice Aliceson') } - - it 'replaces the correct part of the string' do - model.update_column_in_batches(:users, :name, model.replace_sql(Arel::Table.new(:users)[:name], 'Alice', 'Eve')) - expect(user.reload.name).to eq('Kathy Eve Aliceson') - end - end - end end diff --git a/spec/migrations/clean_upload_symlinks_spec.rb b/spec/migrations/clean_upload_symlinks_spec.rb deleted file mode 100644 index cecb3ddac53..00000000000 --- a/spec/migrations/clean_upload_symlinks_spec.rb +++ /dev/null @@ -1,46 +0,0 @@ -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 deleted file mode 100644 index 37d66452447..00000000000 --- a/spec/migrations/move_uploads_to_system_dir_spec.rb +++ /dev/null @@ -1,68 +0,0 @@ -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 deleted file mode 100644 index 18587f2de4e..00000000000 --- a/spec/migrations/rename_system_namespaces_spec.rb +++ /dev/null @@ -1,238 +0,0 @@ -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 - - 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 = create(:project, path: "project-path", namespace: system_namespace) - - 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 - create(:project, namespace: system_namespace, path: "system-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) { create(:project, namespace: system_namespace) } - - 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 = create(:group, path: "subgroup", parent: system_namespace) - project = create(:project, path: "system0", namespace: subgroup) - - 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 deleted file mode 100644 index 7df44515424..00000000000 --- a/spec/migrations/update_upload_paths_to_system_spec.rb +++ /dev/null @@ -1,53 +0,0 @@ -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/namespace_spec.rb b/spec/models/namespace_spec.rb index 49fa68a15a0..78009fdb942 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -36,12 +36,6 @@ describe Namespace, models: true do it { expect(group).not_to be_valid } 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') } @@ -176,8 +170,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', 'system') } - let(:pages_dir) { File.join(TestEnv.pages_path) } + let(:uploads_dir) { File.join(CarrierWave.root, 'uploads') } + let(:pages_dir) { TestEnv.pages_path } before do FileUtils.mkdir_p(File.join(uploads_dir, 'parent', 'child', 'the-project')) @@ -211,6 +205,20 @@ describe Namespace, models: true do expect(File.directory?(expected_pages_path)).to be(true) 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_attributes!(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) + end + end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index fbd9c33196c..daab307389a 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -787,7 +787,7 @@ describe Project, models: true do end let(:avatar_path) do - "/uploads/system/project/avatar/#{project.id}/uploads/avatar.png" + "/uploads/project/avatar/#{project.id}/uploads/avatar.png" end it { should eq "http://#{Gitlab.config.gitlab.host}#{avatar_path}" } diff --git a/spec/requests/openid_connect_spec.rb b/spec/requests/openid_connect_spec.rb index 237310b133d..5206634bca5 100644 --- a/spec/requests/openid_connect_spec.rb +++ b/spec/requests/openid_connect_spec.rb @@ -81,7 +81,7 @@ describe 'OpenID Connect requests' do 'email_verified' => true, 'website' => 'https://example.com', 'profile' => 'http://localhost/alice', - 'picture' => "http://localhost/uploads/system/user/avatar/#{user.id}/dk.png", + 'picture' => "http://localhost/uploads/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 d524c9aff17..063b3bd76eb 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/system/group/avatar/#{group.id}/dk.png" + expect(groups.first[:avatar_url]).to eq "#{base_url}/uploads/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/system/group/avatar/#{group.id}/dk.png" + expect(groups.first[:avatar_url]).to eq "#{base_url}/gitlab/uploads/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 d82dbe871d5..ea714fb08f0 100644 --- a/spec/uploaders/attachment_uploader_spec.rb +++ b/spec/uploaders/attachment_uploader_spec.rb @@ -3,17 +3,6 @@ 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 201fe6949aa..c4d558805ab 100644 --- a/spec/uploaders/avatar_uploader_spec.rb +++ b/spec/uploaders/avatar_uploader_spec.rb @@ -3,17 +3,6 @@ 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 47e9365e13d..d9113ef4095 100644 --- a/spec/uploaders/file_uploader_spec.rb +++ b/spec/uploaders/file_uploader_spec.rb @@ -15,16 +15,6 @@ 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') -- cgit v1.2.1