[api] fix deletion of repositories when linking repositories have mul…

…tiple path elements (bnc#792430)

src/api/app/controllers/source_controller.rb

@@ -1237,15 +1237,15 @@ def private_remove_repositories( repositories, full_remove = false )
       linking_repos = repo.linking_repositories
       prj = repo.project
 
-
+      # full remove, otherwise the model will take care of the cleanup
       if full_remove == true
         # recursive for INDIRECT linked repositories
         unless linking_repos.length < 1
           private_remove_repositories( linking_repos, true )
         end
 
         # try to remove the repository 
-        # but never remove the deleted one
+        # but never remove the special repository named "deleted"
         unless repo == del_repo
           # permission check
           unless @http_user.can_modify_project?(prj)
@@ -1254,17 +1254,6 @@ def private_remove_repositories( repositories, full_remove = false )
             return
           end
         end
-      else
-        # just modify foreign linking repositories. Always allowed
-        linking_repos.each do |lrepo|
-          lrepo.path_elements.each do |pe|
-            if pe.link == repo
-              pe.link = del_repo
-              pe.save
-            end
-          end
-          lrepo.project.store({:lowprio => true}) # low prio storage
-        end
       end
 
       # remove this repository, but be careful, because we may have done it already.

src/api/app/models/repository.rb

@@ -27,15 +27,19 @@ def validate_duplicates
   def cleanup_before_destroy
     # change all linking repository pathes
     del_repo = nil
-
     self.linking_repositories.each do |lrep|
-      lrep.path_elements.includes(:link).each do |pe|
-#        next unless Repository.find(pe.repository_id).db_project_id == self.db_project_id
+      lrep.path_elements.includes(:link, :repository).each do |pe|
+        next unless pe.link == self # this is not pointing to our repo
         del_repo ||= Project.find_by_name("deleted").repositories[0]
-        pe.link = del_repo
-        pe.save
-        lrep.project.store
+        if lrep.path_elements.where(repository_id: del_repo).size > 0
+          # repo has already a path element pointing to del_repo
+          pe.destroy 
+        else
+          pe.link = del_repo
+          pe.save
+        end
       end
+      lrep.project.store({:lowprio => true})
     end
 
   end

src/api/test/functional/source_controller_test.rb

@@ -878,6 +878,12 @@ def test_repository_dependencies
     assert_response :success
     put "/source/home:tom:projectC/_meta", "<project name='home:tom:projectC'> <title/> <description/> <repository name='repoC'> <path project='home:tom:projectB' repository='repoB' /> </repository> </project>"
     assert_response :success
+    put "/source/home:tom:projectD/_meta", "<project name='home:tom:projectD'> <title/> <description/> <repository name='repoD'> " \
+                                           " <path project='home:tom:projectA' repository='repoA' />" \
+                                           " <path project='home:tom:projectB' repository='repoB' />" \
+                                           " <path project='home:tom:projectC' repository='repoC' />" \
+                                           "</repository> </project>"
+    assert_response :success
     # delete a repo
     put "/source/home:tom:projectA/_meta", "<project name='home:tom:projectA'> <title/> <description/> </project>"
     assert_response 400
@@ -893,14 +899,24 @@ def test_repository_dependencies
     assert_response :success
     assert_xml_tag :tag => 'path', :attributes => { :project => "home:tom:projectB", :repository => "repoB" } # unmodified
 
+    # delete another repo
+    put "/source/home:tom:projectB/_meta?force=1", "<project name='home:tom:projectB'> <title/> <description/> </project>"
+    assert_response :success
+    get "/source/home:tom:projectD/_meta"
+    assert_response :success
+    assert_xml_tag :tag => 'path', :attributes => { :project => "deleted", :repository => "deleted" }
+    assert_xml_tag :tag => 'path', :attributes => { :project => "home:tom:projectC", :repository => "repoC" } # unmodified
+
     # cleanup
     delete "/source/home:tom:projectA"
     assert_response :success
     delete "/source/home:tom:projectB"
-    assert_response 403 # projectC still linking
+    assert_response :success
     delete "/source/home:tom:projectC"
+    assert_response 403 # projectD is still using it
+    delete "/source/home:tom:projectD"
     assert_response :success
-    delete "/source/home:tom:projectB"
+    delete "/source/home:tom:projectC"
     assert_response :success
   end