[api] fix revoke/decline open requests on package removal

src/api/app/controllers/source_controller.rb

@@ -162,30 +162,9 @@ def delete_project
     # find linking repos
     private_check_and_remove_repositories(params, pro.repositories) or return
 
-    # Find open requests with 'pro' as source or target and decline/revoke them.
-    # Revoke if source or decline if target went away, pick the first action that matches to decide...
-    # Note: As requests are a backend matter, it's pointless to include them into the transaction below
-    pro.open_requests_with_project_as_source_or_target.each do |request|
-      request.bs_request_actions.each do |action|
-        if action.source_project == pro.name
-          request.change_state('revoked', :comment => "The source project '#{pro.name}' was removed")
-          break
-        end
-        if action.target_project == pro.name
-          request.change_state('declined', :comment => "The target project '#{pro.name}' was removed")
-          break
-        end
-      end
-    end
-
-    # Find open requests which have a review involving this project (or it's packages) and remove those reviews
-    # but leave the requests otherwise untouched.
-    pro.open_requests_with_by_project_review.each do |request|
-      request.remove_reviews(:by_project => pro.name)
-    end
-
     Project.transaction do
       logger.info "destroying project object #{pro.name}"
+      pro.revoke_requests
       pro.destroy
 
       params[:user] = User.current.login
@@ -305,27 +284,7 @@ def delete_package
     # exec
     Package.transaction do
 
-      # Find open requests with 'tpkg' as source or target and decline/revoke them.
-      # Revoke if source or decline if target went away, pick the first action that matches to decide...
-      tpkg.open_requests_with_package_as_source_or_target.each do |request|
-        request.bs_request_actions.each do |action|
-          if action.matches_package? :source, tpkg
-            request.change_state('revoked', :comment => "The source package '#{tpkg.project.name} / #{tpkg.name}' was removed")
-            break
-          end
-          if action.matches_package? :target, tpkg
-            request.change_state('declined', :comment => "The target package '#{tpkg.project.name} / #{tpkg.name}' was removed")
-            break
-          end
-        end
-      end
-
-      # Find open requests which have a review involving this package and remove those reviews
-      # but leave the requests otherwise untouched.
-      tpkg.open_requests_with_by_package_review.each do |request|
-        request.remove_reviews(:by_project => tpkg.project.name, :by_package => tpkg.name)
-      end
-
+      tpkg.revoke_requests
       tpkg.destroy
 
       params[:user] = User.current

src/api/app/models/package.rb

@@ -849,6 +849,30 @@ def remove_linked_packages
     BackendPackage.where(links_to_id: self.id).delete_all
   end
 
+  def revoke_requests
+    # Find open requests with this package as source and revoke them.
+    rel = BsRequest.where(state: [:new, :review, :declined]).joins(:bs_request_actions)
+    rel = rel.where('bs_request_actions.source_project = ? and bs_request_actions.source_package = ?', self.project.name, self.name)
+    BsRequest.where(id: rel.pluck('bs_requests.id')).each do |request|
+      request.change_state('revoked', :comment => "The source package '#{self.project.name}' '#{self.name}' was removed")
+    end
+
+    # Find open requests with this package as target and decline them.
+    rel = BsRequest.where(state: [:new, :review, :declined]).joins(:bs_request_actions)
+    rel = rel.where('bs_request_actions.target_project = ? and bs_request_actions.target_package = ?', self.project.name, self.name)
+    BsRequest.where(id: rel.pluck('bs_requests.id')).each do |request|
+      request.change_state('declined', :comment => "The target package '#{self.project.name}' '#{self.name}' was removed")
+    end
+
+    # Find open requests which have a review involving this project (or it's packages) and remove those reviews
+    # but leave the requests otherwise untouched.
+    rel = BsRequest.where(state: [:new, :review])
+    rel = rel.joins(:reviews).where("reviews.state = 'new' and reviews.by_project = ? and reviews.by_package = ? ", self.project.name, self.name)
+    BsRequest.where(id: rel.pluck('bs_requests.id')).each do |request|
+      request.remove_reviews(:by_project => self.project.name, :by_package => self.name)
+    end
+  end
+
   def patchinfo
     begin
       Patchinfo.new(self.source_file('_patchinfo'))

src/api/app/models/project.rb

@@ -98,6 +98,30 @@ def cleanup_before_destroy
     end
   end
 
+  def revoke_requests
+    # Find open requests with 'pro' as source or target and decline/revoke them.
+    # Revoke if source or decline if target went away, pick the first action that matches to decide...
+    # Note: As requests are a backend matter, it's pointless to include them into the transaction below
+    self.open_requests_with_project_as_source_or_target.each do |request|
+      request.bs_request_actions.each do |action|
+        if action.source_project == self.name
+          request.change_state('revoked', :comment => "The source project '#{self.name}' was removed")
+          break
+        end
+        if action.target_project == self.name
+          request.change_state('declined', :comment => "The target project '#{self.name}' was removed")
+          break
+        end
+      end
+    end
+
+    # Find open requests which have a review involving this project (or it's packages) and remove those reviews
+    # but leave the requests otherwise untouched.
+    self.open_requests_with_by_project_review.each do |request|
+      request.remove_reviews(:by_project => self.name)
+    end
+  end
+
   def find_repos(sym)
     self.repositories.each do |repo|
       repo.send(sym).each do |lrep|

src/api/test/functional/request_controller_test.rb

@@ -158,6 +158,51 @@ def test_create_invalid
     Timecop.return
   end
 
+  def test_request_autodecline_on_removal
+    login_Iggy
+    post '/source/home:Iggy/TestPack?target_project=home:Iggy&target_package=TestPack.DELETE', :cmd => :branch
+    assert_response :success
+    post '/source/home:Iggy/TestPack.DELETE?target_project=home:Iggy&target_package=TestPack.DELETE2', :cmd => :branch
+    assert_response :success
+
+    # create requests
+    post '/request?cmd=create', '<request>
+                                   <action type="submit">
+                                     <source project="home:Iggy" package="TestPack.DELETE2"/>
+                                     <target project="home:Iggy" package="DUMMY"/>
+                                   </action>
+                                   <state name="new" />
+                                 </request>'
+    assert_response :success
+    node = ActiveXML::Node.new(@response.body)
+    assert node.has_attribute?(:id)
+    id1 = node.value('id')
+    post '/request?cmd=create', '<request>
+                                   <action type="submit">
+                                     <source project="home:Iggy" package="TestPack.DELETE"/>
+                                     <target project="home:Iggy" package="TestPack.DELETE2"/>
+                                   </action>
+                                   <state name="new" />
+                                 </request>'
+    assert_response :success
+    node = ActiveXML::Node.new(@response.body)
+    assert node.has_attribute?(:id)
+    id2 = node.value('id')
+
+
+    delete '/source/home:Iggy/TestPack.DELETE2'
+    assert_response :success
+    get "/request/#{id1}"
+    assert_response :success
+    assert_xml_tag( tag: 'state', attributes: { name: 'revoked'} )
+    get "/request/#{id2}"
+    assert_response :success
+    assert_xml_tag( tag: 'state', attributes: { name: 'declined'} )
+
+    delete '/source/home:Iggy/TestPack.DELETE'
+    assert_response :success
+  end
+
   def test_submit_request_with_broken_source
     login_Iggy
     post '/source/home:Iggy/TestPack?target_project=home:Iggy&target_package=TestPack.DELETE', :cmd => :branch

src/api/test/unit/code_quality_test.rb

@@ -109,6 +109,7 @@ def setup
       'Owner::search' => 67.56,
       'PackInfo#to_xml' => 53.64,
       'Package#resolve_devel_package' => 52.33,
+      'Package#revoke_requests' => 51.82,
       'PersonController#internal_register' => 108.84,
       'PersonController#put_userinfo' => 56.38,
       'Project#branch_to_repositories_from' => 54.92,