Fixes #38040 - Clear retain_package_version_count when mirroring poli…

…cy is not additive

app/controllers/katello/api/v2/repositories_controller.rb

@@ -403,6 +403,13 @@ def update
         end
       end
 
+      additive_policy = ::Katello::RootRepository::MIRRORING_POLICY_ADDITIVE
+      is_repo_param_additive = repo_params[:mirroring_policy] == additive_policy
+      is_root_additive = @repository.root.mirroring_policy == additive_policy
+
+      if @repository.yum? && !(is_repo_param_additive || is_root_additive)
+        repo_params[:retain_package_versions_count] = nil
+      end
       sync_task(::Actions::Katello::Repository::Update, @repository.root, repo_params)
       respond_for_show(:resource => @repository)
     end

app/models/katello/root_repository.rb

@@ -337,6 +337,9 @@ def ensure_valid_retain_package_versions_count
       unless yum?
         errors.add(:retain_package_versions_count, N_("is only allowed for Yum repositories."))
       end
+      if yum? && self.mirroring_policy != MIRRORING_POLICY_ADDITIVE
+        errors.add(:retain_package_versions_count, N_("cannot be set for repositories without 'Additive' mirroring policy."))
+      end
       if self.retain_package_versions_count.to_i < 0
         errors.add(:retain_package_versions_count, N_("must not be a negative value."))
       end

test/actions/katello/repository_test.rb

@@ -141,11 +141,20 @@ def setup
       assert_action_planed action, candlepin_action_class
     end
 
-    it 'plans pulp3 update when retain_package_version is updated' do
+    it 'throws error on pulp3 update when retain_package_version is updated without additive' do
       action = create_action action_class
       action.stubs(:action_subject).with(repository)
+      error = assert_raises(ActiveRecord::RecordInvalid) do
+        plan_action action, repository.root, :retain_package_versions_count => 17
+      end
+      assert_match(/Retain package versions count cannot be set for repositories without 'Additive' mirroring policy/, error.message)
+    end
 
-      plan_action action, repository.root, :retain_package_versions_count => 17
+    it 'plans pulp3 update when retain_package_version is updated with additive' do
+      action = create_action action_class
+      action.stubs(:action_subject).with(repository)
+      repo_params = { retain_package_versions_count: 17, mirroring_policy: ::Katello::RootRepository::MIRRORING_POLICY_ADDITIVE }
+      plan_action action, repository.root, repo_params
       assert_action_planed action, pulp3_action_class
     end
 

test/controllers/api/v2/repositories_controller_test.rb

@@ -545,6 +545,7 @@ def test_show_protected_specific_instance
     end
 
     def test_update_with_gpg_key
+      @repository.root.update!(mirroring_policy: Katello::RootRepository::MIRRORING_POLICY_ADDITIVE)
       key = ContentCredential.find(katello_gpg_keys('fedora_gpg_key').id)
       assert_sync_task(::Actions::Katello::Repository::Update) do |root, attributes|
         assert_equal root, @repository.root
@@ -557,6 +558,7 @@ def test_update_with_gpg_key
     end
 
     def test_update_with_cert
+      @repository.root.update!(mirroring_policy: Katello::RootRepository::MIRRORING_POLICY_ADDITIVE)
       cert = ContentCredential.find(katello_gpg_keys('fedora_cert').id)
       assert_sync_task(::Actions::Katello::Repository::Update) do |root, attributes|
         assert_equal root, @repository.root
@@ -657,8 +659,11 @@ def test_update_with_retain_package_versions_count
       repo = katello_repositories(:fedora_17_unpublished)
       assert_sync_task(::Actions::Katello::Repository::Update) do |_, attributes|
         assert_equal attributes[:retain_package_versions_count], retain_package_versions_count
+        assert_equal attributes[:mirroring_policy], ::Katello::RootRepository::MIRRORING_POLICY_ADDITIVE
       end
-      put :update, params: { :id => repo.id, :retain_package_versions_count => retain_package_versions_count }
+      put :update, params: { :id => repo.id,
+                             :retain_package_versions_count => retain_package_versions_count,
+                             :mirroring_policy => ::Katello::RootRepository::MIRRORING_POLICY_ADDITIVE }
     end
 
     def test_update_with_limit_tags
@@ -675,6 +680,7 @@ def test_update_with_limit_tags
     end
 
     def test_update_non_docker_repo_with_limit_tags
+      @repository.root.update!(mirroring_policy: Katello::RootRepository::MIRRORING_POLICY_ADDITIVE)
       assert_sync_task(::Actions::Katello::Repository::Update) do |root, attributes|
         assert_equal root, @repository.root
         expected = { 'name' => 'new name' }

test/models/root_repository_test.rb

@@ -59,6 +59,7 @@ def test_invalid_retain_package_version_counts
       @root.content_type = 'yum'
       @root.url = 'http://inecas.fedorapeople.org/fakerepos/zoo2/'
       @root.retain_package_versions_count = -2
+      @root.mirroring_policy = 'additive'
       assert_not_valid @root
       assert_equal @root.errors[:retain_package_versions_count], ["must not be a negative value."]
     end
@@ -67,6 +68,7 @@ def test_valid_retain_package_version_counts
       @root.content_type = 'yum'
       @root.url = 'http://inecas.fedorapeople.org/fakerepos/zoo2/'
       @root.retain_package_versions_count = 2
+      @root.mirroring_policy = 'additive'
       assert_valid @root
     end
 
@@ -582,7 +584,12 @@ def test_yum_retain_package_versions_count
       assert @root.valid?
 
       @root.retain_package_versions_count = 2
+      @root.mirroring_policy = 'additive'
       assert @root.valid?
+
+      @root.mirroring_policy = 'mirror_content_only'
+      refute @root.valid?
+      assert_equal @root.errors[:retain_package_versions_count], ["cannot be set for repositories without 'Additive' mirroring policy."]
     end
 
     def test_sha1_checksum_removed