From 4d2da5fd2585fead823c4450e54613dadf882c0d Mon Sep 17 00:00:00 2001 From: James Lopez Date: Fri, 22 Jan 2016 17:55:12 +0100 Subject: WIP - spec failure on .atom project URL --- spec/controllers/projects_controller_spec.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 665526fde93..ad143c14c43 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -86,6 +86,17 @@ describe ProjectsController do end end end + + context "when the url contains .atom" do + let(:public_project_with_dot) { create(:project, :public, name: 'my.atom', path: 'my.atom') } + + it 'loads a project' do + get :show, namespace_id: public_project_with_dot.namespace.path, id: public_project_with_dot.path + + expect(assigns(:project)).to eq(public_project_with_dot) + expect(response.status).to eq(200) + end + end end describe "#destroy" do -- cgit v1.2.1 From eb51a4ac1b7702873ecb9de7ddafdb989370437c Mon Sep 17 00:00:00 2001 From: James Lopez Date: Fri, 29 Jan 2016 15:35:21 +0100 Subject: refactor previous test and add validation to project model --- lib/gitlab/regex.rb | 4 ++-- spec/controllers/projects_controller_spec.rb | 9 +++------ 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb index 53ab2686b43..3331bd123b2 100644 --- a/lib/gitlab/regex.rb +++ b/lib/gitlab/regex.rb @@ -34,12 +34,12 @@ module Gitlab def project_path_regex - @project_path_regex ||= /\A[a-zA-Z0-9_.][a-zA-Z0-9_\-\.]*(? Date: Fri, 29 Jan 2016 18:52:49 +0100 Subject: WIP - add migration --- ...5155_remove_dot_atom_path_ending_of_projects.rb | 54 ++++++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 db/migrate/20160129135155_remove_dot_atom_path_ending_of_projects.rb diff --git a/db/migrate/20160129135155_remove_dot_atom_path_ending_of_projects.rb b/db/migrate/20160129135155_remove_dot_atom_path_ending_of_projects.rb new file mode 100644 index 00000000000..c7b986aca91 --- /dev/null +++ b/db/migrate/20160129135155_remove_dot_atom_path_ending_of_projects.rb @@ -0,0 +1,54 @@ +class RemoveDotAtomPathEndingOfProjects < ActiveRecord::Migration + + class ProjectPath + def initilize(old_path) + @old_path = old_path + end + + def clean_path + @_clean_path ||= PathCleaner.clean(@old_path) + end + end + + module PathCleaner + def initialize(path) + @path = path + end + + def self.clean(*args) + new(*args).clean + end + + def clean + path = cleaned_path + count = 0 + while path_exists?(path) + path = "#{cleaned_path}#{count}" + count += 1 + end + path + end + + def cleaned_path + @_cleaned_path ||= path.gsub(/\.atom\z/, '-atom') + end + + def path_exists?(path) + Project.find_by_path(path) + end + end + + def up + projects_with_dot_atom.each do |project| + remove_dot(project) + end + end + + private + + def remove_dot(project) + #TODO + end + + +end -- cgit v1.2.1 From 927ab48101d44976e174b13323d085aa5f846155 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 1 Feb 2016 15:47:11 +0100 Subject: WIP - refactored migration --- CHANGELOG | 1 + ...5155_remove_dot_atom_path_ending_of_projects.rb | 49 ++++++++++++++++++---- 2 files changed, 43 insertions(+), 7 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index fe0504ec996..2e6e65e7ae2 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -17,6 +17,7 @@ v 8.5.0 (unreleased) - Update the ExternalIssue regex pattern (Blake Hitchcock) - Deprecate API "merge_request/:merge_request_id/comments". Use "merge_requests/:merge_request_id/notes" instead - Deprecate API "merge_request/:merge_request_id/...". Use "merge_requests/:merge_request_id/..." instead + - Prevent parse error when name of project ends with .atom and prevent path issues v 8.4.2 - Bump required gitlab-workhorse version to bring in a fix for missing diff --git a/db/migrate/20160129135155_remove_dot_atom_path_ending_of_projects.rb b/db/migrate/20160129135155_remove_dot_atom_path_ending_of_projects.rb index c7b986aca91..622b0f5db08 100644 --- a/db/migrate/20160129135155_remove_dot_atom_path_ending_of_projects.rb +++ b/db/migrate/20160129135155_remove_dot_atom_path_ending_of_projects.rb @@ -1,8 +1,12 @@ class RemoveDotAtomPathEndingOfProjects < ActiveRecord::Migration + include Gitlab::ShellAdapter class ProjectPath - def initilize(old_path) + attr_reader :old_path, :id + + def initialize(old_path, id) @old_path = old_path + @id = id end def clean_path @@ -10,7 +14,7 @@ class RemoveDotAtomPathEndingOfProjects < ActiveRecord::Migration end end - module PathCleaner + class PathCleaner def initialize(path) @path = path end @@ -30,7 +34,7 @@ class RemoveDotAtomPathEndingOfProjects < ActiveRecord::Migration end def cleaned_path - @_cleaned_path ||= path.gsub(/\.atom\z/, '-atom') + @_cleaned_path ||= @path.gsub(/\.atom\z/, '-atom') end def path_exists?(path) @@ -38,17 +42,48 @@ class RemoveDotAtomPathEndingOfProjects < ActiveRecord::Migration end end + def projects_with_dot_atom + select_all("SELECT id, path FROM projects WHERE lower(path) LIKE '%.atom'") + end + def up projects_with_dot_atom.each do |project| - remove_dot(project) + binding.pry + project_path = ProjectPath.new(project['path'], project['id']) + clean_path(project_path) if move_path(project_path) end end private - def remove_dot(project) - #TODO + def clean_path(project_path) + execute "UPDATE projects SET path = '#{project_path.clean_path}' WHERE id = #{project.id}" end - + def move_path(project_path) + # Based on RemovePeriodsAtEndsOfUsernames + # Don't attempt to move if original path only contains periods. + return if project_path.clean_path =~ /\A\.+\z/ + if gitlab_shell.mv_namespace(project_path.old_path, project_path.clean_path) + # If repositories moved successfully we need to remove old satellites + # and send update instructions to users. + # However we cannot allow rollback since we moved namespace dir + # So we basically we mute exceptions in next actions + begin + gitlab_shell.rm_satellites(project_path.old_path) + # We cannot send update instructions since models and mailers + # can't safely be used from migrations as they may be written for + # later versions of the database. + # send_update_instructions + rescue + # Returning false does not rollback after_* transaction but gives + # us information about failing some of tasks + false + end + else + # if we cannot move namespace directory we should avoid + # db changes in order to prevent out of sync between db and fs + false + end + end end -- cgit v1.2.1 From 94b27f9d795c05134a777f754adbea2ab3accd33 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 1 Feb 2016 15:49:52 +0100 Subject: added TODO to not working method --- db/migrate/20160129135155_remove_dot_atom_path_ending_of_projects.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/db/migrate/20160129135155_remove_dot_atom_path_ending_of_projects.rb b/db/migrate/20160129135155_remove_dot_atom_path_ending_of_projects.rb index 622b0f5db08..0bc23f5627f 100644 --- a/db/migrate/20160129135155_remove_dot_atom_path_ending_of_projects.rb +++ b/db/migrate/20160129135155_remove_dot_atom_path_ending_of_projects.rb @@ -60,6 +60,7 @@ class RemoveDotAtomPathEndingOfProjects < ActiveRecord::Migration execute "UPDATE projects SET path = '#{project_path.clean_path}' WHERE id = #{project.id}" end + #TODO: Fix this def move_path(project_path) # Based on RemovePeriodsAtEndsOfUsernames # Don't attempt to move if original path only contains periods. -- cgit v1.2.1 From ecb174bfeadffbccc5c5cf35b6e94b65f5fbce79 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 2 Feb 2016 17:28:14 +0100 Subject: fixed move project method in migration --- ...5155_remove_dot_atom_path_ending_of_projects.rb | 46 ++++++---------------- db/schema.rb | 2 +- lib/gitlab/backend/shell.rb | 2 +- 3 files changed, 15 insertions(+), 35 deletions(-) diff --git a/db/migrate/20160129135155_remove_dot_atom_path_ending_of_projects.rb b/db/migrate/20160129135155_remove_dot_atom_path_ending_of_projects.rb index 0bc23f5627f..9ca695d713f 100644 --- a/db/migrate/20160129135155_remove_dot_atom_path_ending_of_projects.rb +++ b/db/migrate/20160129135155_remove_dot_atom_path_ending_of_projects.rb @@ -2,11 +2,12 @@ class RemoveDotAtomPathEndingOfProjects < ActiveRecord::Migration include Gitlab::ShellAdapter class ProjectPath - attr_reader :old_path, :id + attr_reader :old_path, :id, :namespace_path - def initialize(old_path, id) + def initialize(old_path, id, namespace_path) @old_path = old_path @id = id + @namespace_path = namespace_path end def clean_path @@ -43,48 +44,27 @@ class RemoveDotAtomPathEndingOfProjects < ActiveRecord::Migration end def projects_with_dot_atom - select_all("SELECT id, path FROM projects WHERE lower(path) LIKE '%.atom'") + select_all("SELECT p.id, p.path, n.path as namespace_path FROM projects p inner join namespaces n on n.id = p.namespace_id WHERE lower(p.path) LIKE '%.atom'") end def up projects_with_dot_atom.each do |project| - binding.pry - project_path = ProjectPath.new(project['path'], project['id']) - clean_path(project_path) if move_path(project_path) + project_path = ProjectPath.new(project['path'], project['id'], project['namespace_path']) + clean_path(project_path) if rename_project_repo(project_path) end end private def clean_path(project_path) - execute "UPDATE projects SET path = '#{project_path.clean_path}' WHERE id = #{project.id}" + execute "UPDATE projects SET path = '#{project_path.clean_path}' WHERE id = #{project_path.id}" end - #TODO: Fix this - def move_path(project_path) - # Based on RemovePeriodsAtEndsOfUsernames - # Don't attempt to move if original path only contains periods. - return if project_path.clean_path =~ /\A\.+\z/ - if gitlab_shell.mv_namespace(project_path.old_path, project_path.clean_path) - # If repositories moved successfully we need to remove old satellites - # and send update instructions to users. - # However we cannot allow rollback since we moved namespace dir - # So we basically we mute exceptions in next actions - begin - gitlab_shell.rm_satellites(project_path.old_path) - # We cannot send update instructions since models and mailers - # can't safely be used from migrations as they may be written for - # later versions of the database. - # send_update_instructions - rescue - # Returning false does not rollback after_* transaction but gives - # us information about failing some of tasks - false - end - else - # if we cannot move namespace directory we should avoid - # db changes in order to prevent out of sync between db and fs - false - end + def rename_project_repo(project_path) + old_path_with_namespace = File.join(project_path.namespace_path, project_path.old_path) + new_path_with_namespace = File.join(project_path.namespace_path, project_path.clean_path) + + gitlab_shell.mv_repository("#{old_path_with_namespace}.wiki", "#{new_path_with_namespace}.wiki") + gitlab_shell.mv_repository(old_path_with_namespace, new_path_with_namespace) end end diff --git a/db/schema.rb b/db/schema.rb index 2ad2c23fba5..ad9e9e8a5de 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160128233227) do +ActiveRecord::Schema.define(version: 20160129135155) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" diff --git a/lib/gitlab/backend/shell.rb b/lib/gitlab/backend/shell.rb index 4c15d58d680..f751458ac66 100644 --- a/lib/gitlab/backend/shell.rb +++ b/lib/gitlab/backend/shell.rb @@ -47,7 +47,7 @@ module Gitlab # new_path - new project path with namespace # # Ex. - # mv_repository("gitlab/gitlab-ci", "randx/gitlab-ci-new.git") + # mv_repository("gitlab/gitlab-ci", "randx/gitlab-ci-new") # def mv_repository(path, new_path) Gitlab::Utils.system_silent([gitlab_shell_projects_path, 'mv-project', -- cgit v1.2.1 From 7b868c61ab371fc9319e6dd1baa2c089bc275618 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Thu, 4 Feb 2016 13:20:55 +0100 Subject: refactored migration and spec based on feedback --- ...29135155_remove_dot_atom_path_ending_of_projects.rb | 18 ++++++++++++------ spec/controllers/projects_controller_spec.rb | 4 ++-- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/db/migrate/20160129135155_remove_dot_atom_path_ending_of_projects.rb b/db/migrate/20160129135155_remove_dot_atom_path_ending_of_projects.rb index 9ca695d713f..bb79ac026c2 100644 --- a/db/migrate/20160129135155_remove_dot_atom_path_ending_of_projects.rb +++ b/db/migrate/20160129135155_remove_dot_atom_path_ending_of_projects.rb @@ -4,19 +4,21 @@ class RemoveDotAtomPathEndingOfProjects < ActiveRecord::Migration class ProjectPath attr_reader :old_path, :id, :namespace_path - def initialize(old_path, id, namespace_path) + def initialize(old_path, id, namespace_path, namespace_id) @old_path = old_path @id = id @namespace_path = namespace_path + @namespace_id = namespace_id end def clean_path - @_clean_path ||= PathCleaner.clean(@old_path) + @_clean_path ||= PathCleaner.clean(@old_path, @namespace_id) end end class PathCleaner - def initialize(path) + def initialize(path, namespace_id) + @namespace_id = namespace_id @path = path end @@ -34,22 +36,24 @@ class RemoveDotAtomPathEndingOfProjects < ActiveRecord::Migration path end + private + def cleaned_path @_cleaned_path ||= @path.gsub(/\.atom\z/, '-atom') end def path_exists?(path) - Project.find_by_path(path) + Project.find_by_path_and_namespace_id(path, @namespace_id) end end def projects_with_dot_atom - select_all("SELECT p.id, p.path, n.path as namespace_path FROM projects p inner join namespaces n on n.id = p.namespace_id WHERE lower(p.path) LIKE '%.atom'") + select_all("SELECT p.id, p.path, n.path as namespace_path, n.id as namespace_id FROM projects p inner join namespaces n on n.id = p.namespace_id WHERE lower(p.path) LIKE '%.atom'") end def up projects_with_dot_atom.each do |project| - project_path = ProjectPath.new(project['path'], project['id'], project['namespace_path']) + project_path = ProjectPath.new(project['path'], project['id'], project['namespace_path'], project['namespace_id']) clean_path(project_path) if rename_project_repo(project_path) end end @@ -66,5 +70,7 @@ class RemoveDotAtomPathEndingOfProjects < ActiveRecord::Migration gitlab_shell.mv_repository("#{old_path_with_namespace}.wiki", "#{new_path_with_namespace}.wiki") gitlab_shell.mv_repository(old_path_with_namespace, new_path_with_namespace) + rescue + false end end diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 245cf96d644..6eee4dfe229 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -88,10 +88,10 @@ describe ProjectsController do end context "when the url contains .atom" do - let(:public_project_with_dot_atom) { create(:project, :public, name: 'my.atom', path: 'my.atom') } + let(:public_project_with_dot_atom) { build(:project, :public, name: 'my.atom', path: 'my.atom') } it 'expect an error creating the project' do - expect { public_project_with_dot_atom }.to raise_error(ActiveRecord::RecordInvalid) + expect(public_project_with_dot_atom).not_to be_valid end end end -- cgit v1.2.1 From 5dc77d7577bf19586f6cd756678d0c2660e7f868 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Thu, 4 Feb 2016 15:35:03 +0100 Subject: refactored migration based on feedback --- .../20160129135155_remove_dot_atom_path_ending_of_projects.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/db/migrate/20160129135155_remove_dot_atom_path_ending_of_projects.rb b/db/migrate/20160129135155_remove_dot_atom_path_ending_of_projects.rb index bb79ac026c2..091de54978b 100644 --- a/db/migrate/20160129135155_remove_dot_atom_path_ending_of_projects.rb +++ b/db/migrate/20160129135155_remove_dot_atom_path_ending_of_projects.rb @@ -61,7 +61,7 @@ class RemoveDotAtomPathEndingOfProjects < ActiveRecord::Migration private def clean_path(project_path) - execute "UPDATE projects SET path = '#{project_path.clean_path}' WHERE id = #{project_path.id}" + execute "UPDATE projects SET path = #{sanitize(project_path.clean_path)} WHERE id = #{project_path.id}" end def rename_project_repo(project_path) @@ -73,4 +73,8 @@ class RemoveDotAtomPathEndingOfProjects < ActiveRecord::Migration rescue false end + + def sanitize(value) + ActiveRecord::Base.connection.quote(value) + end end -- cgit v1.2.1