Fix: Checking if the proposal has been changed (right sorting). (#416)

* Fix: Checking if the proposal has been changed (right sorting).

package/yast2-packager.changes

@@ -1,3 +1,10 @@
+-------------------------------------------------------------------
+Tue Mar 26 17:27:18 CET 2019 - schubi@suse.de
+
+- Fix: Checking if the proposal has been changed (right sorting).
+  (bsc#1125718)
+- 4.1.35
+
 -------------------------------------------------------------------
 Mon Mar 25 15:34:46 UTC 2019 - Stefan Hundhammer <shundhammer@suse.com>
 

package/yast2-packager.spec

@@ -17,7 +17,7 @@
 
 
 Name:           yast2-packager
-Version:        4.1.34
+Version:        4.1.35
 Release:        0
 
 BuildRoot:      %{_tmppath}/%{name}-%{version}-build

src/modules/Packages.rb

@@ -16,6 +16,7 @@ class PackagesClass < Module
     include ERB::Util
 
     attr_reader :missing_remote_packages, :missing_remote_kind
+    attr_accessor :cached_proposal
 
     # All known types of resolvables
     RESOLVABLE_TYPES = [:product, :patch, :package, :pattern, :language].freeze
@@ -88,15 +89,11 @@ def main
       @init_error = nil
 
       # cache for the proposed summary
-      @cached_proposal = nil
+      @cached_proposal_summary = nil
 
       # the selection used for the cached proposal
       # the default values 'nil' say that the proposal hasn't been called yet
-      @cached_proposal_packages = nil
-      @cached_proposal_patterns = nil
-      @cached_proposal_products = nil
-      @cached_proposal_patches = nil
-      @cached_proposal_languages = nil
+      @cached_proposal = nil
 
       @install_sources = false # Installing source packages ?
       @timestamp = 0 # last time of getting the target map
@@ -161,12 +158,7 @@ def main
     def ResetProposalCache
       Builtins.y2milestone("Reseting the software proposal cache")
 
-      @cached_proposal_packages = nil
-      @cached_proposal_patterns = nil
-      @cached_proposal_products = nil
-      @cached_proposal_patches = nil
-      @cached_proposal_languages = nil
-
+      @cached_proposal = nil
       nil
     end
 
@@ -2030,59 +2022,23 @@ def Proposal(force_reset, reinit, _simple)
 
       # if the cache is valid and reset or reinitialization is not required
       # then the cached proposal can be used
-      if !@cached_proposal.nil? && force_reset == false && reinit == false
-        # selected packages
-        selected_packages = Pkg.GetPackages(:selected, false)
-
-        # selected patterns
-        selected_patterns = Builtins.filter(
-          Pkg.ResolvableProperties("", :pattern, "")
-        ) do |p|
-          Ops.get_symbol(p, "status", :unknown) == :selected
-        end
-
-        # selected products
-        selected_products = Builtins.filter(
-          Pkg.ResolvableProperties("", :product, "")
-        ) do |p|
-          Ops.get_symbol(p, "status", :unknown) == :selected
-        end
+      if !@cached_proposal_summary.nil? && force_reset == false && reinit == false
 
-        # selected patches
-        selected_patches = Builtins.filter(
-          Pkg.ResolvableProperties("", :patch, "")
-        ) do |p|
-          Ops.get_symbol(p, "status", :unknown) == :selected
-        end
-
-        # selected languages
-        selected_languages = Convert.convert(
-          Builtins.union([Pkg.GetPackageLocale], Pkg.GetAdditionalLocales),
-          from: "list",
-          to:   "list <string>"
-        )
-
-        # if the package selection has not been changed the cache is up to date
-        if selected_packages == @cached_proposal_packages &&
-            selected_patterns == @cached_proposal_patterns &&
-            selected_products == @cached_proposal_products &&
-            selected_patches == @cached_proposal_patches &&
-            selected_languages == @cached_proposal_languages
+        # if the package, pattern,... selection has not been changed the cache is up to date
+        if !proposal_changed?
           Builtins.y2milestone("using cached software proposal")
-          return deep_copy(@cached_proposal)
+          return deep_copy(@cached_proposal_summary)
         # do not show the error message during the first proposal
         # (and the only way to change to software selection manually -> software_proposal/AskUser)
         #
         # 'nil' is the default value
         # See also ResetProposalCache()
-        elsif !@cached_proposal_packages.nil? &&
-            !@cached_proposal_patterns.nil? &&
-            !@cached_proposal_products.nil? &&
-            !@cached_proposal_patches.nil? &&
-            !@cached_proposal_languages.nil?
-          Builtins.y2error(
-            "invalid cache: the software selection has been chaged"
+        elsif !cached_proposal.nil?
+          log.error(
+            "invalid cache: the software selection has been changed"
           )
+          log_proposal_diff
+
           # bnc #436925
           Report.Message(
             _(
@@ -2162,33 +2118,12 @@ def Proposal(force_reset, reinit, _simple)
 
       # Question: is `desktop appropriate for SLE?
       ret = Summary([:product, :pattern, :size, :desktop], false)
-      # TODO: simple proposal
 
-      # cache the proposal
-      @cached_proposal = deep_copy(ret)
+      # cache the proposal summary
+      @cached_proposal_summary = deep_copy(ret)
 
       # remember the status
-      @cached_proposal_packages = Pkg.GetPackages(:selected, false)
-      @cached_proposal_patterns = Builtins.filter(
-        Pkg.ResolvableProperties("", :pattern, "")
-      ) do |p|
-        Ops.get_symbol(p, "status", :unknown) == :selected
-      end
-      @cached_proposal_products = Builtins.filter(
-        Pkg.ResolvableProperties("", :product, "")
-      ) do |p|
-        Ops.get_symbol(p, "status", :unknown) == :selected
-      end
-      @cached_proposal_patches = Builtins.filter(
-        Pkg.ResolvableProperties("", :patch, "")
-      ) do |p|
-        Ops.get_symbol(p, "status", :unknown) == :selected
-      end
-      @cached_proposal_languages = Convert.convert(
-        Builtins.union([Pkg.GetPackageLocale], Pkg.GetAdditionalLocales),
-        from: "list",
-        to:   "list <string>"
-      )
+      @cached_proposal = current_proposal
 
       UI.CloseDialog
 
@@ -2420,6 +2355,42 @@ def remote_x11_packages
 
   private
 
+    def fetch_selected(category)
+      items = Pkg.ResolvableProperties("", category, "").select { |i| i["status"] == :selected }
+      items.map { |i| i["name"] }.sort
+    end
+
+    # Current package, pattern, product, patch and language selection.
+    #
+    # @return [Hash] selected packages, patterns, products,...
+    def current_proposal
+      { "packages"  => Pkg.GetPackages(:selected, false).sort,
+        "patterns"  => fetch_selected(:pattern),
+        "products"  => fetch_selected(:product),
+        "patches"   => fetch_selected(:patch),
+        "languages" => [Pkg.GetPackageLocale].concat(Pkg.GetAdditionalLocales).compact.sort }
+    end
+
+    # Checking if there have been changes around package, pattern, product,
+    # patch and language selection.
+    #
+    # @return [Boolean] changed ?
+    def proposal_changed?
+      current_proposal != cached_proposal
+    end
+
+    #
+    # Log changed proposal
+    #
+    def log_proposal_diff
+      current_proposal.each do |kind, current|
+        if current != cached_proposal[kind]
+          log.info("New #{kind}: #{current - cached_proposal[kind]}")
+          log.info("Removed #{kind}: #{cached_proposal[kind] - current}")
+        end
+      end
+    end
+
     # Reads product feature defined by parameters, logs what it gets
     # and returns list of items split by whitespaces
     #

test/packages_test.rb

@@ -1728,4 +1728,45 @@ def expect_source_urls(mapping)
       expect(subject.sourceAccessPackages.sort).to eq(["cifs-mount", "nfs-client"])
     end
   end
+
+  describe "#proposal_changed?" do
+    before do
+      allow(Yast::Pkg).to receive(:ResolvableProperties).with("", :patch, "")
+        .and_return([])
+
+      allow(Yast::Pkg).to receive(:ResolvableProperties).with("", :pattern, "")
+        .and_return(
+          [pattern("name" => "minimal_base", "status" => :selected),
+           pattern("name" => "base", "status" => :selected)]
+        )
+      allow(Yast::Pkg).to receive(:ResolvableProperties).with("", :product, "")
+        .and_return([product("name" => "SLES", "status" => :selected)])
+      allow(Yast::Pkg).to receive(:GetAdditionalLocales).and_return([])
+      allow(Yast::Pkg).to receive(:GetPackageLocale).and_return("en_US")
+
+      subject.cached_proposal = { "packages" => ["SUSEConnect 0.3.16 5.13 x86_64",
+                                                 "aaa_base 84.87+git20180409.04c9dae 3.3.2 x86_64",
+                                                 "bash 4.4 9.7.1 x86_64"],
+        "patterns" => ["base", "minimal_base"],
+        "products" => ["SLES"], "patches" => [], "languages" => ["en_US"] }
+    end
+
+    context "current selection has not been changed" do
+      it "returns false" do
+        allow(Yast::Pkg).to receive(:GetPackages)
+          .and_return(["aaa_base 84.87+git20180409.04c9dae 3.3.2 x86_64",
+                       "bash 4.4 9.7.1 x86_64", "SUSEConnect 0.3.16 5.13 x86_64"])
+        expect(Yast::Packages.send(:proposal_changed?)).to eq false
+      end
+    end
+
+    context "package selection has been changed" do
+      it "returns true" do
+        allow(Yast::Pkg).to receive(:GetPackages)
+          .and_return(["aaa_base 84.87+git20180409.04c9dae 3.3.2 x86_64",
+                       "bash 4.4 9.7.1 x86_64"])
+        expect(Yast::Packages.send(:proposal_changed?)).to eq true
+      end
+    end
+  end
 end