diff --git a/app/controllers/platforms/repositories_controller.rb b/app/controllers/platforms/repositories_controller.rb index 9b92e1360..c56c80aaf 100644 --- a/app/controllers/platforms/repositories_controller.rb +++ b/app/controllers/platforms/repositories_controller.rb @@ -141,7 +141,8 @@ class Platforms::RepositoriesController < Platforms::BaseController end def regenerate_metadata - if AbfWorker::BuildListsPublishTaskManager.repository_regenerate_metadata @repository.id + build_for_platform = Platform.main.find params[:build_for_platform_id] if @repository.platform.personal? + if AbfWorker::BuildListsPublishTaskManager.repository_regenerate_metadata @repository, (build_for_platform || @repository.platform) flash[:notice] = t('flash.repository.regenerate_in_queue') else flash[:error] = t('flash.repository.regenerate_already_in_queue') diff --git a/app/models/ability.rb b/app/models/ability.rb index 09a9ca41d..85eebe6be 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -171,8 +171,6 @@ class Ability cannot(:cancel, MassBuild) {|mass_build| mass_build.stop_build} - cannot(:regenerate_metadata, Repository) {|repository| !repository.platform.main?} - if @user.system? can :key_pair, Repository else diff --git a/app/models/pull_request.rb b/app/models/pull_request.rb index 0966ec9e4..f47e05880 100644 --- a/app/models/pull_request.rb +++ b/app/models/pull_request.rb @@ -164,7 +164,7 @@ class PullRequest < ActiveRecord::Base def merge clone message = "Merge pull request ##{serial_id} from #{from_project_owner_uname}/#{from_project_name}:#{from_ref}\r\n #{title}" - %x(cd #{path} && git checkout #{to_ref} && git merge --no-ff #{from_branch} -m #{message.shellescape}) + %x(cd #{path} && git checkout #{to_ref.shellescape} && git merge --no-ff #{from_branch.shellescape} -m #{message.shellescape}) end def clone @@ -196,6 +196,7 @@ class PullRequest < ActiveRecord::Base unless tags.include? from_ref system 'git', 'branch', '-D', from_branch system 'git', 'fetch', head, "+#{from_ref}:#{from_branch}" + system 'git', 'checkout', to_ref end end end diff --git a/app/views/platforms/repositories/_form.html.haml b/app/views/platforms/repositories/_form.html.haml index f054218b7..262bd3cf1 100644 --- a/app/views/platforms/repositories/_form.html.haml +++ b/app/views/platforms/repositories/_form.html.haml @@ -10,13 +10,6 @@ .both .hr -- if ['edit', 'update'].include?(controller.action_name) && can?(:regenerate_metadata, @repository) - .leftside= t('layout.repositories.regenerate_metadata') - .rightside - = link_to t('layout.repositories.regenerate_metadata').split.first, regenerate_metadata_platform_repository_path(@platform, @repository), - :method => :put, :confirm => t('layout.confirm'), :class => :button - .hr{:style => 'padding-bottom:20px;'} -.both .button_block = submit_tag t("layout.save") diff --git a/app/views/platforms/repositories/_regenerate_metadata.html.haml b/app/views/platforms/repositories/_regenerate_metadata.html.haml new file mode 100644 index 000000000..89bee4eb5 --- /dev/null +++ b/app/views/platforms/repositories/_regenerate_metadata.html.haml @@ -0,0 +1,8 @@ +.hr += form_for @repository, :url => regenerate_metadata_platform_repository_path(@platform, @repository), :html => { :class => :form, :method => :put } do |f| + .leftside= t('layout.repositories.regenerate_metadata') + .rightside + = select_tag :build_for_platform_id, options_from_collection_for_select(Platform.main, :id, :name) if @platform.personal? + = f.submit t('layout.repositories.regenerate_metadata'), :confirm => t('layout.confirm') + .hr{:style => 'padding-bottom:20px;'} +.both \ No newline at end of file diff --git a/app/views/platforms/repositories/edit.html.haml b/app/views/platforms/repositories/edit.html.haml index e9fff3fe4..088c0fae4 100644 --- a/app/views/platforms/repositories/edit.html.haml +++ b/app/views/platforms/repositories/edit.html.haml @@ -8,6 +8,8 @@ = render "form", :f => f %br += render 'regenerate_metadata' if can?(:regenerate_metadata, @repository) + - if @platform.main? = render "shared/members_table", :remove_members_path => remove_members_platform_repository_path(@platform, @repository), diff --git a/config/environments/development.rb b/config/environments/development.rb index 98a9d21f5..facbb837e 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -1,4 +1,21 @@ # -*- encoding : utf-8 -*- + +# https://github.com/rails/rails/issues/2639#issuecomment-6591735 +class DisableAssetsLogger + def initialize(app) + @app = app + Rails.application.assets.logger = Logger.new('/dev/null') + end + + def call(env) + previous_level = Rails.logger.level + Rails.logger.level = Logger::ERROR if env['PATH_INFO'].index("/assets/") == 0 + @app.call(env) + ensure + Rails.logger.level = previous_level + end +end + Rosa::Application.configure do # Settings specified here will take precedence over those in config/application.rb @@ -40,4 +57,6 @@ Rosa::Application.configure do # Log the query plan for queries taking more than this (works with SQLite, MySQL, and PostgreSQL) config.active_record.auto_explain_threshold_in_seconds = 0.5 + + config.middleware.insert_before Rails::Rack::Logger, DisableAssetsLogger end diff --git a/lib/abf_worker/build_lists_publish_task_manager.rb b/lib/abf_worker/build_lists_publish_task_manager.rb index 75334e703..79449636f 100644 --- a/lib/abf_worker/build_lists_publish_task_manager.rb +++ b/lib/abf_worker/build_lists_publish_task_manager.rb @@ -3,6 +3,7 @@ module AbfWorker class BuildListsPublishTaskManager REDIS_MAIN_KEY = 'abf-worker::build-lists-publish-task-manager::' + # LOCKED_REP_AND_PLATFORMS: ['save_to_repository_id-build_for_platform_id', ...] %w(RESIGN_REPOSITORIES PROJECTS_FOR_CLEANUP LOCKED_PROJECTS_FOR_CLEANUP @@ -56,9 +57,10 @@ module AbfWorker redis.lpush RESIGN_REPOSITORIES, key_pair.repository_id end - def repository_regenerate_metadata(repository_id) - return false if Resque.redis.lrange(REGENERATE_METADATA, 0, -1).include? repository_id.to_s - redis.lpush REGENERATE_METADATA, repository_id + def repository_regenerate_metadata(repository, build_for_platform) + key = "#{repository.id}-#{build_for_platform.id}" + return false if Resque.redis.lrange(REGENERATE_METADATA, 0, -1).include? key + redis.lpush REGENERATE_METADATA, key end def unlock_repository(repository_id) @@ -336,50 +338,50 @@ module AbfWorker return true end - # Only for main platforms! def create_tasks_for_repository_regenerate_metadata - worker_queue = 'publish_worker_default' - worker_class = 'AbfWorker::PublishWorkerDefault' - regen_repos = @redis.lrange REGENERATE_METADATA, 0, -1 - locked_rep_and_pl = @redis.lrange(LOCKED_REP_AND_PLATFORMS, 0, -1) + worker_queue = 'publish_worker_default' + worker_class = 'AbfWorker::PublishWorkerDefault' + regen_repos_and_pl = @redis.lrange REGENERATE_METADATA, 0, -1 + locked_rep_and_pl = @redis.lrange(LOCKED_REP_AND_PLATFORMS, 0, -1) + regen_repos = regen_repos_and_pl.map{ |r| r.gsub(/\-[\d]*$/, '') } Repository.where(:id => regen_repos).each do |rep| - lock_str = "#{rep.id}-#{rep.platform_id}" - next if locked_rep_and_pl.include?("#{rep.id}-#{rep.platform_id}") - @redis.lrem REGENERATE_METADATA, 0, rep.id + regen_repos_and_pl.select{ |kind| kind =~ /^#{rep.id}\-/ }.each do |lock_str| + next if locked_rep_and_pl.include?(lock_str) + @redis.lrem REGENERATE_METADATA, 0, lock_str - platform_path = "#{rep.platform.path}/repository" - distrib_type = rep.platform.distrib_type - cmd_params = { - 'RELEASED' => rep.platform.released, - 'REPOSITORY_NAME' => rep.name, - 'TYPE' => distrib_type, - 'REGENERATE_METADATA' => true, - 'SAVE_TO_PLATFORM' => rep.platform.name, - 'BUILD_FOR_PLATFORM' => rep.platform.name - }.map{ |k, v| "#{k}=#{v}" }.join(' ') + build_for_platform = Platform.find lock_str.gsub(/^[\d]*\-/, '') + cmd_params = { + 'RELEASED' => rep.platform.released, + 'REPOSITORY_NAME' => rep.name, + 'TYPE' => build_for_platform.distrib_type, + 'REGENERATE_METADATA' => true, + 'SAVE_TO_PLATFORM' => rep.platform.name, + 'BUILD_FOR_PLATFORM' => build_for_platform.name + }.map{ |k, v| "#{k}=#{v}" }.join(' ') - options = { - :id => Time.now.to_i, - :arch => 'x86_64', - :distrib_type => distrib_type, - :cmd_params => cmd_params, - :platform => {:platform_path => platform_path}, - :repository => {:id => rep.id}, - :type => :publish, - :time_living => 9600, # 160 min - :skip_feedback => true, - :extra => {:lock_str => lock_str, :regenerate => true} - } + options = { + :id => Time.now.to_i, + :arch => 'x86_64', + :distrib_type => build_for_platform.distrib_type, + :cmd_params => cmd_params, + :platform => {:platform_path => "#{rep.platform.path}/repository"}, + :repository => {:id => rep.id}, + :type => :publish, + :time_living => 9600, # 160 min + :skip_feedback => true, + :extra => {:lock_str => lock_str, :regenerate => true} + } - Resque.push( - worker_queue, - 'class' => worker_class, - 'args' => [options.merge({ - })] - ) + Resque.push( + worker_queue, + 'class' => worker_class, + 'args' => [options.merge({ + })] + ) - @redis.lpush(LOCKED_REP_AND_PLATFORMS, lock_str) + @redis.lpush(LOCKED_REP_AND_PLATFORMS, lock_str) + end end return true end diff --git a/spec/controllers/platforms/repositories_controller_spec.rb b/spec/controllers/platforms/repositories_controller_spec.rb index 80cca3236..4f7d5fa09 100644 --- a/spec/controllers/platforms/repositories_controller_spec.rb +++ b/spec/controllers/platforms/repositories_controller_spec.rb @@ -29,6 +29,12 @@ shared_examples_for 'user without change projects in repository rights' do @repository.projects.should_not include(@project) end + it 'should not be able to perform regenerate_metadata action' do + put :regenerate_metadata, :id => @repository.id, :platform_id => @platform.id + response.should redirect_to(redirect_path) + regenerate_metadata_queue.should be_empty + end + it 'should not be able to remove project from repository' do delete :remove_project, :id => @repository.id, :platform_id => @platform.id, :project_id => @project.id response.should redirect_to(redirect_path) @@ -42,6 +48,18 @@ shared_examples_for 'registered user or guest' do response.should redirect_to(redirect_path) end + it 'should not be able to perform regenerate_metadata action' do + put :regenerate_metadata, :id => @repository.id, :platform_id => @platform.id + response.should redirect_to(redirect_path) + regenerate_metadata_queue.should be_empty + end + + it 'should not be able to perform regenerate_metadata action of personal repository' do + put :regenerate_metadata, :id => @personal_repository.id, :platform_id => @personal_repository.platform.id + response.should redirect_to(redirect_path) + regenerate_metadata_queue.should be_empty + end + it 'should not be able to perform create action' do post :create, @create_params lambda { post :create, @create_params }.should change{ Repository.count }.by(0) @@ -88,8 +106,8 @@ shared_examples_for 'registered user or guest' do end it 'should not be able to destroy personal repository' do - lambda { delete :destroy, :id => @personal_repository.id, :platform_id => @personal_repository.platform.id}. - should change{ Repository.count }.by(0) + lambda { delete :destroy, :id => @personal_repository.id, :platform_id => @personal_repository.platform.id} + .should change{ Repository.count }.by(0) response.should redirect_to(redirect_path) end end @@ -121,6 +139,24 @@ shared_examples_for 'platform admin user' do response.should render_template(:new) end + it 'should be able to perform regenerate_metadata action' do + put :regenerate_metadata, :id => @repository.id, :platform_id => @platform.id + response.should redirect_to(platform_repository_path(@platform, @repository)) + regenerate_metadata_queue.should == ["#{@repository.id}-#{@platform.id}"] + end + + it 'should be able to perform regenerate_metadata action of personal repository' do + put :regenerate_metadata, :id => @personal_repository.id, :platform_id => @personal_repository.platform.id, :build_for_platform_id => @platform.id + response.should redirect_to(platform_repository_path(@personal_repository.platform, @personal_repository)) + regenerate_metadata_queue.should == ["#{@personal_repository.id}-#{@platform.id}"] + end + + it 'should not be able to perform regenerate_metadata action of personal repository when build_for_platform does not exist' do + put :regenerate_metadata, :id => @personal_repository.id, :platform_id => @personal_repository.platform.id + response.should render_template(:file => "#{Rails.root}/public/404.html") + regenerate_metadata_queue.should be_empty + end + it 'should be able to create repository' do lambda { post :create, @create_params }.should change{ Repository.count }.by(1) response.should redirect_to(platform_repository_path(@platform, Repository.last)) @@ -162,14 +198,14 @@ shared_examples_for 'platform admin user' do it 'should not be able to destroy personal repository with name "main"' do # hook for "ActiveRecord::ActiveRecordError: name is marked as readonly" Repository.where(:id => @personal_repository.id).update_all("name = 'main'") - lambda { delete :destroy, :id => @personal_repository.id, :platform_id => @personal_repository.platform.id}. - should change{ Repository.count }.by(0) + lambda { delete :destroy, :id => @personal_repository.id, :platform_id => @personal_repository.platform.id} + .should change{ Repository.count }.by(0) response.should redirect_to(forbidden_path) end it 'should be able to destroy personal repository with name not "main"' do - lambda { delete :destroy, :id => @personal_repository.id, :platform_id => @personal_repository.platform.id}. - should change{ Repository.count }.by(-1) + lambda { delete :destroy, :id => @personal_repository.id, :platform_id => @personal_repository.platform.id} + .should change{ Repository.count }.by(-1) response.should redirect_to(platform_repositories_path(@personal_repository.platform)) end @@ -177,8 +213,10 @@ shared_examples_for 'platform admin user' do end describe Platforms::RepositoriesController do + let(:regenerate_metadata_queue) { @redis_instance.lrange(AbfWorker::BuildListsPublishTaskManager::REGENERATE_METADATA, 0, -1) } before(:each) do stub_symlink_methods + stub_redis @platform = FactoryGirl.create(:platform) @repository = FactoryGirl.create(:repository, :platform => @platform) diff --git a/spec/lib/abf-worker/build_lists_publish_task_manager_spec.rb b/spec/lib/abf-worker/build_lists_publish_task_manager_spec.rb index dc95cea49..bd8e50965 100644 --- a/spec/lib/abf-worker/build_lists_publish_task_manager_spec.rb +++ b/spec/lib/abf-worker/build_lists_publish_task_manager_spec.rb @@ -15,7 +15,7 @@ describe AbfWorker::BuildListsPublishTaskManager do subject { AbfWorker::BuildListsPublishTaskManager } let(:build_list) { FactoryGirl.create(:build_list_core, :new_core => true) } - describe 'when no items for publishing' do + context 'when no items for publishing' do before do stub_redis subject.new.run @@ -41,7 +41,7 @@ describe AbfWorker::BuildListsPublishTaskManager do end - describe 'when one build_list for publishing' do + context 'when one build_list for publishing' do before do stub_redis build_list.update_column(:status, BuildList::BUILD_PUBLISH) @@ -75,7 +75,7 @@ describe AbfWorker::BuildListsPublishTaskManager do end - describe 'grouping build lists for publishing into same repository' do + context 'grouping build lists for publishing into same repository' do let(:build_list2) { FactoryGirl.create(:build_list_core, :new_core => true, :save_to_platform => build_list.save_to_platform, @@ -117,7 +117,7 @@ describe AbfWorker::BuildListsPublishTaskManager do end - describe 'creates not more than 4 tasks for publishing' do + context 'creates not more than 4 tasks for publishing' do before do stub_redis build_list.update_column(:status, BuildList::BUILD_PUBLISH) @@ -142,7 +142,7 @@ describe AbfWorker::BuildListsPublishTaskManager do end - describe 'creates task for removing project from repository' do + context 'creates task for removing project from repository' do before do stub_redis build_list.update_column(:status, BuildList::BUILD_PUBLISHED) @@ -179,7 +179,7 @@ describe AbfWorker::BuildListsPublishTaskManager do end - describe 'grouping build lists for publishing and tasks for removing project from repository' do + context 'grouping build lists for publishing and tasks for removing project from repository' do let(:build_list2) { FactoryGirl.create(:build_list_core, :new_core => true, :save_to_platform => build_list.save_to_platform, @@ -233,7 +233,7 @@ describe AbfWorker::BuildListsPublishTaskManager do end end - describe 'resign packages in repository' do + context 'resign packages in repository' do before do stub_redis build_list.update_column(:status, BuildList::BUILD_PUBLISH) @@ -264,6 +264,66 @@ describe AbfWorker::BuildListsPublishTaskManager do end + context 'regenerate metadata' do + before do + stub_redis + end + + context 'for repository of main platform' do + let(:repository) { FactoryGirl.create(:repository) } + before do + subject.repository_regenerate_metadata repository, repository.platform + subject.new.run + end + + it "ensure that 'locked rep and platforms' has only one item" do + queue = @redis_instance.lrange(subject::LOCKED_REP_AND_PLATFORMS, 0, -1) + queue.should have(1).item + queue.should include("#{repository.id}-#{repository.platform.id}") + end + + it "ensure that 'regenerate metadata' queue without items" do + queue = @redis_instance.lrange(subject::REGENERATE_METADATA, 0, -1) + queue.should be_empty + end + + it 'ensure that new task has been created' do + @redis_instance.lrange('queue:publish_worker_default', 0, -1).should have(1).item + end + end + + context 'for repository of personal platform' do + let(:main_platform) { FactoryGirl.create(:platform) } + let(:repository) { FactoryGirl.create(:personal_repository) } + before do + subject.repository_regenerate_metadata repository, main_platform + subject.new.run + end + + it "ensure that 'locked rep and platforms' has only one item" do + @redis_instance.lrange(subject::LOCKED_REP_AND_PLATFORMS, 0, -1) + .should == ["#{repository.id}-#{main_platform.id}"] + end + + it "ensure that 'regenerate metadata' queue without items" do + @redis_instance.lrange(subject::REGENERATE_METADATA, 0, -1).should be_empty + end + + it 'ensure that new task has been created' do + @redis_instance.lrange('queue:publish_worker_default', 0, -1).should have(1).item + end + end + + it 'ensure that two tasks for regenerate one repository will not be created' do + repository = FactoryGirl.create(:repository) + 2.times do + subject.repository_regenerate_metadata repository, repository.platform + subject.new.run + end + @redis_instance.lrange('queue:publish_worker_default', 0, -1).should have(1).item + end + + end after(:all) do APP_CONFIG['abf_worker']['publish_workers_count'] = @publish_workers_count