From c5a93df0db1ee7cf622ead812cc6d4ad45404aee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 10 Aug 2016 08:52:02 +0100 Subject: [PATCH] Fix add-on handling --- src/modules/AddOnProduct.rb | 111 +++++++++++---------- test/addon_product_test.rb | 165 +++++++++++++++++++++++++++++++ test/data/add_on_products_cd.xml | 11 +++ 3 files changed, 236 insertions(+), 51 deletions(-) create mode 100644 test/data/add_on_products_cd.xml diff --git a/src/modules/AddOnProduct.rb b/src/modules/AddOnProduct.rb index 1a2e0c05f..996dcf8b7 100644 --- a/src/modules/AddOnProduct.rb +++ b/src/modules/AddOnProduct.rb @@ -1572,24 +1572,16 @@ def AddRepo(url, pth, priority) new_repo = { "enabled" => true, "base_urls" => [url], "prod_dir" => pth } # BNC #714027: Possibility to adjust repository priority (usually higher) - Ops.set(new_repo, "priority", priority) if Ops.greater_than(priority, -1) + new_repo["priority"] = priority if priority > -1 - Builtins.y2milestone( - "Adding Repository: %1, product path: %2", - URL.HidePassword(url), - pth - ) + log.info "Adding Repository: #{URL.HidePassword(url)}, product path: #{pth}" new_repo_id = Pkg.RepositoryAdd(new_repo) + log.info "New repository id: #{new_repo_id}" - if new_repo_id == nil || Ops.less_than(new_repo_id, 0) - Builtins.y2error("Unable to add product: %1", URL.HidePassword(url)) + if new_repo_id == nil || new_repo_id < 0 + log.error "Unable to add product: %1", URL.HidePassword(url) # TRANSLATORS: error message, %1 is replaced with product URL - Report.Error( - Builtins.sformat( - _("Unable to add product %1."), - URL.HidePassword(url) - ) - ) + Report.Error(format(_("Unable to add product %s."), URL.HidePassword(url))) return nil end @@ -1714,45 +1706,16 @@ def AddPreselectedAddOnProducts(filelist) Builtins.y2milestone("inst_network_check ret: %1", inc_ret) end # a CD/DVD repository - if Builtins.contains(["cd", "dvd"], scheme) - # if the CD/DVD product is known just try if it's there - # and ask if not - if prodname != "" - found = false - - while !found - repo_id = AddRepo(url, pth, priority) - next false if repo_id == nil - - prod2 = Pkg.SourceProductData(repo_id) - if Ops.get_string(prod2, "label", "") == prodname - found = true - else - Builtins.y2milestone( - "Removing repo %1: Add-on found: %2, expected: %3", - repo_id, - Ops.get_string(prod2, "label", ""), - prodname - ) - Pkg.SourceDelete(repo_id) - - # ask for a different medium - url = AskForCD(url, prodname) - next false if url == nil - end - end + repo_id = + if Builtins.contains(["cd", "dvd"], scheme) + prodname.empty? ? add_repo_from_cd(url, pth, priority) : + add_product_from_cd(url, pth, priority, prodname) else - result = AskForCD(url, prodname) - next false if result == nil - - repo_id = AddRepo(result, pth, priority) - next false if repo_id == nil + # a non CD/DVD repository + AddRepo(url, pth, priority) end - else - # a non CD/DVD repository - repo_id = AddRepo(url, pth, priority) - next false if repo_id == nil - end + next false unless repo_id + if !AcceptedLicenseAndInfoFile(repo_id) Builtins.y2warning("License not accepted, delete the repository") Pkg.SourceDelete(repo_id) @@ -2256,6 +2219,52 @@ def add_rename(old_name, new_name) publish :function => :ImportGpgKeyCallback, :type => "boolean (map , integer)" publish :function => :AcceptNonTrustedGpgKeyCallback, :type => "boolean (map )" publish :function => :SetSignatureCallbacks, :type => "void (string)" + + private + + # Adds a product from a CD/DVD + # + # To add the product, the name should match +prodname+ argument. + # + # @param url [String] Repository URL (cd: or dvd: schemas are expected) + # @param pth [String] Repository path (in the media) + # @param priority [Integer] Repository priority + # @param prodname [String] Expected product's name + # @return [Integer,nil] Repository id; nil if product was not found and user cancelled. + # + # @see add_repo_from_cd + def add_product_from_cd(url, pth, priority, prodname) + current_url = url + loop do + # just try if it's there and ask if not + repo_id = AddRepo(current_url, pth, priority) + if repo_id + found_product = Pkg.SourceProductData(repo_id) + return repo_id if found_product["label"] == prodname + log.info("Removing repo #{repo_id}: Add-on found #{found_product["label"]}, expected: #{prodname}") + Pkg.SourceDelete(repo_id) + end + + # ask for a different medium + current_url = AskForCD(current_url, prodname) + return nil if current_url.nil? + end + end + + # Adds a repository from a CD/DVD + # + # The product contained in the CD/DVD does not matter. + # + # @param url [String] Repository URL (cd: or dvd: schemas are expected) + # @param pth [String] Repository path (in the media) + # @param priority [Integer] Repository priority + # @return [Integer,nil] Repository id; nil if product was not found and user cancelled. + def add_repo_from_cd(url, pth, priority) + result = AskForCD(url, "") + return result if result.nil? + + AddRepo(result, pth, priority) + end end AddOnProduct = AddOnProductClass.new diff --git a/test/addon_product_test.rb b/test/addon_product_test.rb index 38502de3b..487641fbd 100755 --- a/test/addon_product_test.rb +++ b/test/addon_product_test.rb @@ -5,6 +5,8 @@ Yast.import "AddOnProduct" describe Yast::AddOnProduct do + subject { Yast::AddOnProduct } + describe "#renamed?" do it "returns true if product has been renamed" do expect(Yast::AddOnProduct.renamed?("SUSE_SLES", "SLES")).to eq(true) @@ -87,4 +89,167 @@ end end end + + describe "#AddPreselectedAddOnProducts" do + BASE_URL = "cd:/?devices=/dev/disk/by-id/ata-QEMU_DVD-ROM_QM00001".freeze + ADDON_REPO = { + "path" => "/foo", "priority" => 50, "url" => "cd:/?alias=Foo" + }.freeze + + let(:repo) { ADDON_REPO } + let(:filelist) do + [ { "file" => "/add_on_products.xml", "type" => "xml" } ] + end + + before do + subject.SetBaseProductURL(BASE_URL) + allow(subject).to receive(:ParseXMLBasedAddOnProductsFile).and_return([repo]) + end + + context "when filelist is empty" do + let(:filelist) { [] } + + it "just returns true" do + expect(subject).to_not receive(:GetBaseProductURL) + subject.AddPreselectedAddOnProducts(filelist) + end + end + + context "when filelist is nil" do + let(:filelist) { nil } + + it "just returns true" do + expect(subject).to_not receive(:GetBaseProductURL) + subject.AddPreselectedAddOnProducts(filelist) + end + end + + context "when filelist contains XML files" do + it "parses the XML file" do + expect(subject).to receive(:ParseXMLBasedAddOnProductsFile).and_return([]) + subject.AddPreselectedAddOnProducts(filelist) + end + end + + context "when filelist contains plain-text files" do + let(:filelist) do + [ { "file" => "/add_on_products.xml", "type" => "plain" } ] + end + + it "parses the plain file" do + expect(subject).to receive(:ParsePlainAddOnProductsFile).and_return([]) + subject.AddPreselectedAddOnProducts(filelist) + end + end + + context "when the add-on is on a CD/DVD" do + let(:repo_id) { 1 } + let(:cd_url) { "cd:///?device=/dev/sr0" } + + before do + allow(subject).to receive(:AskForCD).and_return(cd_url) + allow(subject).to receive(:AcceptedLicenseAndInfoFile).and_return(true) + allow(Yast::Pkg). to receive(:SourceProductData).with(repo_id) + allow(subject).to receive(:InstallProductsFromRepository) + allow(subject).to receive(:ReIntegrateFromScratch) + allow(subject).to receive(:Integrate) + end + + context "and no product name was given" do + let(:repo) { ADDON_REPO } + + it "adds the repository" do + allow(subject).to receive(:AddRepo).with(cd_url, anything, anything) + subject.AddPreselectedAddOnProducts(filelist) + end + + it "does not add the repository if user cancels the dialog" do + allow(subject).to receive(:AskForCD).and_return(nil) + + expect(subject).to_not receive(:AddRepo) + subject.AddPreselectedAddOnProducts(filelist) + end + + it "does not integrate the repository if it was not added" do + allow(subject).to receive(:AddRepo).with(cd_url, anything, anything) + .and_return(nil) + + expect(subject).to_not receive(:Integrate).with(repo_id) + subject.AddPreselectedAddOnProducts(filelist) + end + end + + context "and a network scheme is used" do + let(:repo) { ADDON_REPO.merge("url" => "http://example.net/repo") } + + it "checks whether the network is working" do + allow(subject).to receive(:AddRepo).and_return(nil) + expect(Yast::WFM).to receive(:CallFunction).with("inst_network_check", []) + subject.AddPreselectedAddOnProducts(filelist) + end + end + + context "and a product name was given" do + let(:repo) { ADDON_REPO.merge("name" => "Foo") } + let(:matching_product) { { "label" => repo["name"] } } + let(:other_product) { { "label" => "other" } } + let(:other_repo_id) { 2 } + + context "and the product is found in the CD/DVD" do + before do + allow(Yast::Pkg).to receive(:SourceProductData).with(repo_id) + .and_return(matching_product) + end + + it "adds the product without asking" do + expect(subject).to_not receive(:AskForCD) + expect(subject).to receive(:AddRepo).with(repo["url"], anything, anything) + .and_return(repo_id) + subject.AddPreselectedAddOnProducts(filelist) + end + end + + context "and the product is not found in the CD/DVD" do + before do + allow(Yast::Pkg).to receive(:SourceProductData).with(repo_id) + .and_return(matching_product) + allow(Yast::Pkg).to receive(:SourceProductData).with(other_repo_id) + .and_return(other_product) + end + + it "does not add the repository if the user cancels the dialog" do + allow(subject).to receive(:AddRepo).with(repo["url"], anything, anything) + .and_return(other_repo_id) + allow(subject).to receive(:AskForCD).and_return(nil) + + expect(Yast::Pkg).to receive(:SourceDelete).with(other_repo_id) + expect(subject).to_not receive(:Integrate).with(other_repo_id) + subject.AddPreselectedAddOnProducts(filelist) + end + + it "adds the product if the user points to a valid CD/DVD" do + allow(subject).to receive(:AddRepo).with(repo["url"], anything, anything) + .and_return(other_repo_id) + allow(subject).to receive(:AskForCD).and_return(cd_url) + allow(subject).to receive(:AddRepo).with(cd_url, anything, anything) + .and_return(repo_id) + + expect(Yast::Pkg).to receive(:SourceDelete).with(other_repo_id) + expect(Yast::Pkg).to_not receive(:SourceDelete).with(repo_id) + expect(subject).to receive(:Integrate).with(repo_id) + subject.AddPreselectedAddOnProducts(filelist) + end + end + end + + it "removes the product is the license is not accepted" do + allow(subject).to receive(:AddRepo).with(cd_url, anything, anything) + .and_return(repo_id) + expect(subject).to receive(:AcceptedLicenseAndInfoFile).and_return(false) + expect(Yast::Pkg).to receive(:SourceDelete).with(repo_id) + subject.AddPreselectedAddOnProducts(filelist) + expect(subject.add_on_products).to be_empty + end + end + end end diff --git a/test/data/add_on_products_cd.xml b/test/data/add_on_products_cd.xml new file mode 100644 index 000000000..07a20f8ea --- /dev/null +++ b/test/data/add_on_products_cd.xml @@ -0,0 +1,11 @@ + + + + + cd:/?alias=FooXXX + /foo + 50 + + +