diff --git a/.rubocop.yml b/.rubocop.yml index 2c357dff0..ce367bd87 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -41,6 +41,7 @@ RSpec/MultipleExpectations: Metrics/ClassLength: Exclude: - lib/rmt/downloader.rb + Max: 250 Naming/FileName: Exclude: diff --git a/app/models/product.rb b/app/models/product.rb index 8f65f7f60..91bf4d483 100644 --- a/app/models/product.rb +++ b/app/models/product.rb @@ -153,7 +153,7 @@ def self.recommended_extensions(root_product_ids) joins(:product_extensions_associations).where(products_extensions: { recommended: true, root_product_id: root_product_ids }) end - def create_service! + def find_or_create_service! service = Service.find_by(product_id: id) return service if service diff --git a/app/models/repository.rb b/app/models/repository.rb index c65c602cd..64d84a3ba 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -18,8 +18,6 @@ class Repository < ApplicationRecord validates :local_path, presence: true validates :friendly_id, presence: true - before_destroy :ensure_destroy_possible - class << self def remove_suse_repos_without_tokens! @@ -70,10 +68,4 @@ def custom? scc_id.nil? end - private - - def ensure_destroy_possible - throw(:abort) unless custom? - end - end diff --git a/app/services/repository_service.rb b/app/services/repository_service.rb index d967feb7b..7f5ebe7ca 100644 --- a/app/services/repository_service.rb +++ b/app/services/repository_service.rb @@ -3,7 +3,7 @@ class RepositoryService class RepositoryNotFound < RuntimeError end - def create_repository!(product, url, attributes, custom: false) + def update_or_create_repository!(product, url, attributes, custom: false) repository = if custom Repository.find_or_initialize_by(external_url: url) else diff --git a/engines/scc_proxy/lib/scc_proxy/engine.rb b/engines/scc_proxy/lib/scc_proxy/engine.rb index 2d57185f9..93a48775a 100644 --- a/engines/scc_proxy/lib/scc_proxy/engine.rb +++ b/engines/scc_proxy/lib/scc_proxy/engine.rb @@ -283,7 +283,6 @@ def scc_upgrade(auth, product, system_login, mode, logger) end end - # rubocop:disable Metrics/ClassLength class Engine < ::Rails::Engine isolate_namespace SccProxy config.generators.api_only = true @@ -523,6 +522,5 @@ def get_system(systems) end end end - # rubocop:enable Metrics/ClassLength end # rubocop:enable Metrics/ModuleLength diff --git a/lib/rmt/cli/repos_custom.rb b/lib/rmt/cli/repos_custom.rb index cccf333ea..482626fe1 100644 --- a/lib/rmt/cli/repos_custom.rb +++ b/lib/rmt/cli/repos_custom.rb @@ -35,7 +35,7 @@ def add(url, name) raise RMT::CLI::Error.new(_("Couldn't add custom repository.")) end - repository_service.create_repository!(nil, url, { + repository_service.update_or_create_repository!(nil, url, { name: name.strip, mirroring_enabled: true, autorefresh: true, diff --git a/lib/rmt/scc.rb b/lib/rmt/scc.rb index 59fccdf2a..8e50fc145 100644 --- a/lib/rmt/scc.rb +++ b/lib/rmt/scc.rb @@ -23,13 +23,32 @@ def sync data.each { |item| create_product(item) } data.each { |item| migration_paths(item) } - update_repositories(scc_api_client.list_repositories) + # Update repositories with details (eg. access token) from API + repositories_data = scc_api_client.list_repositories + update_repositories(repositories_data) Repository.remove_suse_repos_without_tokens! + remove_obsolete_repositories(repositories_data) update_subscriptions(scc_api_client.list_subscriptions) end + def remove_obsolete_repositories(repos_data) + @logger.info _('Removing obsolete repositories') + return if repos_data.empty? + + scc_repo_ids = repos_data.pluck(:id) + + + # Find repositories in RMT that no longer exist in SCC + # Only consider repositories that have a non-null scc_id + repos_to_remove = Repository.only_scc.where.not(scc_id: scc_repo_ids) + if repos_to_remove.any? + repos_to_remove.destroy_all + @logger.debug("Successfully removed #{repos_to_remove.count} obsolete repositories") + end + end + def export(path) credentials_set? || (raise CredentialsError, 'SCC credentials not set.') @@ -185,10 +204,21 @@ def create_product(item, root_product_id = nil, base_product = nil, recommended end def create_service(item, product) - product.create_service! + service = product.find_or_create_service! + item[:repositories].each do |repo_item| - repository_service.create_repository!(product, repo_item[:url], repo_item) + repository_service.update_or_create_repository!(product, repo_item[:url], repo_item) end + + # detect repositories removed from the product in SCC + removed_repos = service.repositories.only_scc.where.not(scc_id: item[:repositories].pluck(:id)) + disassociate_repositories(service, removed_repos) if removed_repos.present? + + end + + def disassociate_repositories(service, repos) + service.repositories.delete(repos) + @logger.debug("Removed repositories #{repos.pluck(:scc_id)} from '#{service.product.friendly_name}'") end def migration_paths(item) diff --git a/package/obs/rmt-server.changes b/package/obs/rmt-server.changes index 0fe7923d6..5678987df 100644 --- a/package/obs/rmt-server.changes +++ b/package/obs/rmt-server.changes @@ -7,6 +7,7 @@ Mon Dec 23 14:07:00 UTC 2024 - Luís Caparroz * rmt-server-pubcloud: * Update Micro check due to Micro 6.0 and 6.1 identifier to keep bsc#1230419 in place * Update Zypper path allowing check to handle paid extensions (i.e. LTSS) (bsc#1230157) + * Remove obsolete repositories and associations from rmt during SCC sync (bsc#1232808) ------------------------------------------------------------------- Mon Dec 23 08:03:56 UTC 2024 - Parag Jain diff --git a/spec/factories/products.rb b/spec/factories/products.rb index 4612dfb00..8e2120740 100644 --- a/spec/factories/products.rb +++ b/spec/factories/products.rb @@ -117,7 +117,7 @@ trait :with_service do after :create do |product, _evaluator| - product.create_service! + product.find_or_create_service! end end diff --git a/spec/factories/services.rb b/spec/factories/services.rb index fe74e26ae..f7902653c 100644 --- a/spec/factories/services.rb +++ b/spec/factories/services.rb @@ -66,3 +66,10 @@ end end end + +FactoryBot.define do + factory :repositories_services_association do + association :repository + association :service + end +end diff --git a/spec/lib/rmt/scc_spec.rb b/spec/lib/rmt/scc_spec.rb index f1e3b9d61..6bdec095c 100644 --- a/spec/lib/rmt/scc_spec.rb +++ b/spec/lib/rmt/scc_spec.rb @@ -247,7 +247,7 @@ describe '#remove_suse_repos_without_tokens' do let(:api_double) { double } - let!(:suse_repo_with_token) { FactoryBot.create(:repository, :with_products, auth_token: 'auth_token') } + let!(:suse_repo_with_token) { FactoryBot.create(:repository, :with_products, auth_token: 'auth_token', scc_id: 200000) } let!(:suse_repo_without_token) do FactoryBot.create( :repository, @@ -262,7 +262,8 @@ :with_products, auth_token: nil, external_url: 'https://installer-updates.suse.com/repos/not/updates', - installer_updates: true + installer_updates: true, + scc_id: 200001 ) end let!(:custom_repo) do @@ -314,6 +315,119 @@ def scc end end + describe '#remove_obsolete_repositories' do + let(:api_double) { instance_double 'SUSE::Connect::Api' } + let(:logger) { instance_double('RMT::Logger').as_null_object } + let!(:suse_repo_one) { create(:repository, :with_products, scc_id: 1) } + let!(:suse_repo_two) { create(:repository, :with_products, scc_id: 2) } + let!(:custom_repo) { create(:repository, :with_products, scc_id: nil) } + + before do + allow(Settings).to receive(:scc).and_return OpenStruct.new(username: 'foo', password: 'bar') + allow(RMT::Logger).to receive(:new).and_return(logger) + end + + context 'when repos_data is empty' do + it 'returns early and does not remove any repositories' do + expect(Repository).not_to receive(:only_scc) + described_class.new.remove_obsolete_repositories([]) + expect(Repository.count).to eq(3) + end + end + + context 'when repos_data contains all existing SCC repositories' do + let(:repos_data) do + [ + { id: suse_repo_one.scc_id }, + { id: suse_repo_two.scc_id } + ] + end + + it 'does not remove any repositories' do + expect { described_class.new.remove_obsolete_repositories(repos_data) }.not_to change(Repository, :count) + end + end + + context 'when repos_data is missing some existing SCC repositories' do + let(:repos_data) do + [ + { id: suse_repo_one.scc_id } + ] + end + + it 'removes only the obsolete SCC repositories' do + expect { described_class.new.remove_obsolete_repositories(repos_data) }.to change(Repository, :count).by(-1) + + expect(Repository.find_by(id: suse_repo_one.id)).to be_present + expect(Repository.find_by(id: suse_repo_two.id)).to be_nil + expect(Repository.find_by(id: custom_repo.id)).to be_present + end + end + + context 'when repos_data does not contain any existing SCC repository IDs' do + let(:repos_data) do + [ + { id: 999 } + ] + end + + it 'removes all SCC repositories' do + expect { described_class.new.remove_obsolete_repositories(repos_data) }.to change(Repository, :count).by(-2) + + expect(Repository.find_by(id: suse_repo_one.id)).to be_nil + expect(Repository.find_by(id: suse_repo_two.id)).to be_nil + expect(Repository.find_by(id: custom_repo.id)).to be_present + end + end + end + + + describe '#disassociate_repositories' do + let(:logger) { instance_double('RMT::Logger').as_null_object } + let(:product) { create(:product) } + let(:service) { create(:service, product: product) } + + before do + allow(RMT::Logger).to receive(:new).and_return(logger) + end + + context 'when existing_repo_ids is empty' do + it 'does not remove any associations' do + expect { described_class.new.send(:disassociate_repositories, service, []) }.not_to change(RepositoriesServicesAssociation, :count) + end + end + + context 'when existing_repo_ids contains repository IDs' do + let!(:repo_one) { create(:repository, :with_products, scc_id: 101) } + let!(:repo_two) { create(:repository, :with_products, scc_id: 102) } + let!(:repo_three) { create(:repository, :with_products, scc_id: 103) } + let(:existing_repo_ids) { [repo_one.scc_id, repo_two.scc_id] } + + before do + # Associate repositories with the product through services + [repo_one, repo_two, repo_three].each do |repo| + create(:repositories_services_association, + repository: repo, + service: service) + end + end + + it 'removes repository associations for the specified repo IDs' do + expect do + described_class.new.send(:disassociate_repositories, service, [repo_one, repo_two]) + end.to change(RepositoriesServicesAssociation, :count).by(-2) + end + + it 'only removes associations for specified repositories' do + described_class.new.send(:disassociate_repositories, service, [repo_one, repo_two]) + + expect(product.repositories).not_to include(repo_one) + expect(product.repositories).not_to include(repo_two) + expect(product.repositories).to include(repo_three) + end + end + end + describe '#export' do let(:path) { '/tmp/usb' } diff --git a/spec/models/product_spec.rb b/spec/models/product_spec.rb index 5dc3771b3..56dea3964 100644 --- a/spec/models/product_spec.rb +++ b/spec/models/product_spec.rb @@ -218,13 +218,13 @@ end end - describe '#create_service!' do + describe '#find_or_create_service!' do context 'when service already exists' do let!(:product) { create :product } let!(:service) { create :service, product_id: product.id } it 'returns the existing service' do - expect(product.create_service!).to eq(service) + expect(product.find_or_create_service!).to eq(service) end end @@ -234,7 +234,7 @@ let!(:other_service) { create :service, id: product.id, product_id: other_product.id } it 'creates a service with a random ID' do - expect(product.create_service!.id).not_to eq(other_service.id) + expect(product.find_or_create_service!.id).not_to eq(other_service.id) end end @@ -242,7 +242,7 @@ let!(:product) { create :product } it 'creates a service with a matching ID' do - expect(product.create_service!.id).to eq(product.id) + expect(product.find_or_create_service!.id).to eq(product.id) end end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 3c49cd6fa..7233de218 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -227,14 +227,6 @@ end describe '#destroy' do - context 'when it is an official repository' do - subject { repository.destroy } - - let!(:repository) { create :repository } - - it { is_expected.to be_falsey } - end - context 'when it is a custom repository' do subject { repository.destroy } diff --git a/spec/services/repository_service_spec.rb b/spec/services/repository_service_spec.rb index 0035395b5..d028aa837 100644 --- a/spec/services/repository_service_spec.rb +++ b/spec/services/repository_service_spec.rb @@ -6,7 +6,7 @@ let(:product) { create :product, :with_service } describe '#create_repository' do - subject(:repository) { service.create_repository!(product, url, attributes, custom: custom).reload } + subject(:repository) { service.update_or_create_repository!(product, url, attributes, custom: custom).reload } let(:attributes) do { @@ -38,10 +38,10 @@ context 'URLs of SCC repositories changes' do subject(:repository) do - service.create_repository!(product, old_url, attributes, custom: custom) + service.update_or_create_repository!(product, old_url, attributes, custom: custom) expect(Repository.find_by(external_url: old_url)).not_to eq(nil) - service.create_repository!(product, url, attributes, custom: custom).reload + service.update_or_create_repository!(product, url, attributes, custom: custom).reload end let(:old_url) { 'https://foo.bar.com/bar/foo' } @@ -53,10 +53,10 @@ context 'self heals SCC repos' do subject(:repository) do - service.create_repository!(product, url, attributes, custom: custom).update(scc_id: old_scc_id) + service.update_or_create_repository!(product, url, attributes, custom: custom).update(scc_id: old_scc_id) expect(Repository.find_by(scc_id: old_scc_id)).not_to eq(nil) - service.create_repository!(product, url, attributes, custom: custom).reload + service.update_or_create_repository!(product, url, attributes, custom: custom).reload end let(:old_scc_id) { 666 } @@ -68,8 +68,8 @@ context 'custom repo with same url' do subject(:repository) do - service.create_repository!(product, url, attributes, custom: custom).update(scc_id: nil) - service.create_repository!(product, url, attributes, custom: custom).reload + service.update_or_create_repository!(product, url, attributes, custom: custom).update(scc_id: nil) + service.update_or_create_repository!(product, url, attributes, custom: custom).reload end it_behaves_like 'scc repositories' @@ -90,9 +90,9 @@ context 'already existing repositories with changing URL', :skip_sqlite do subject(:repository) do - service.create_repository!(product, url, attributes, custom: custom).reload + service.update_or_create_repository!(product, url, attributes, custom: custom).reload url = 'https://foo.bar.com/bar/foo' - service.create_repository!(product, url, attributes, custom: custom).reload + service.update_or_create_repository!(product, url, attributes, custom: custom).reload end it('raises error when the id is the same') { expect { repository }.to raise_error(/Duplicate entry/) }