Merge pull request #183 from abf/rosa-build:167-allow-regenerate-metadata-of-repository-for-personal-platforms

#167: Allow regenerate metadata of repository for personal platforms

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')

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

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")

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

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),

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)
+      def repository_regenerate_metadata(repository, build_for_platform)
-        return false if Resque.redis.lrange(REGENERATE_METADATA, 0, -1).include? repository_id.to_s
+        key = "#{repository.id}-#{build_for_platform.id}"
-        redis.lpush REGENERATE_METADATA, repository_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,35 +338,34 @@ 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
+      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}"
+        regen_repos_and_pl.select{ |kind| kind =~ /^#{rep.id}\-/ }.each do |lock_str|
-        next if locked_rep_and_pl.include?("#{rep.id}-#{rep.platform_id}")
+          next if locked_rep_and_pl.include?(lock_str)
-        @redis.lrem REGENERATE_METADATA, 0, rep.id
+          @redis.lrem REGENERATE_METADATA, 0, lock_str
 
-        platform_path = "#{rep.platform.path}/repository"
+          build_for_platform  = Platform.find lock_str.gsub(/^[\d]*\-/, '')
-        distrib_type  = rep.platform.distrib_type
           cmd_params          = {
             'RELEASED'            => rep.platform.released,
             'REPOSITORY_NAME'     => rep.name,
-          'TYPE'                => distrib_type,
+            'TYPE'                => build_for_platform.distrib_type,
             'REGENERATE_METADATA' => true,
             'SAVE_TO_PLATFORM'    => rep.platform.name,
-          'BUILD_FOR_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,
+            :distrib_type => build_for_platform.distrib_type,
             :cmd_params   => cmd_params,
-          :platform     => {:platform_path => platform_path},
+            :platform     => {:platform_path => "#{rep.platform.path}/repository"},
             :repository   => {:id => rep.id},
             :type         => :publish,
             :time_living  => 9600, # 160 min
@@ -381,6 +382,7 @@ module AbfWorker
 
           @redis.lpush(LOCKED_REP_AND_PLATFORMS, lock_str)
         end
+      end
       return true
     end
   end

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}.
+    lambda { delete :destroy, :id => @personal_repository.id, :platform_id => @personal_repository.platform.id}
-      should change{ Repository.count }.by(0)
+      .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}.
+    lambda { delete :destroy, :id => @personal_repository.id, :platform_id => @personal_repository.platform.id}
-      should change{ Repository.count }.by(0)
+      .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}.
+    lambda { delete :destroy, :id => @personal_repository.id, :platform_id => @personal_repository.platform.id}
-      should change{ Repository.count }.by(-1)
+      .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)

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