fix: Emit signal when storage propeties change (#1236)

### Problem

The value of the D-Bus properties *ProductMountPoints* and
*EncryptionMethods* from the interface
*org.opensuse.Agama.Storage1.Proposal.Calculator* depend on the selected
product. Nevertheless, the *PropertiesChanged* signal is not emitted
when the product changes. Therefore, proxies clients never update such
values.

This implies that the option for FDE (Full Disk Encryption) might not be
shown. See #1004.

### Solution

* For the *ProductMountPoints* property: emit a *PropertiesChanged*
signal when the product changes.
* For the *EncryptionMethods* property: emit a *PropertiesChaged* signal
when software is probed. Note that the availability of certain packages
is checked in order to know whether FDE is a possible encryption method.

service/lib/agama/dbus/clients/base.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-# Copyright (c) [2022-2023] SUSE LLC
+# Copyright (c) [2022-2024] SUSE LLC
 #
 # All Rights Reserved.
 #
@@ -68,24 +68,37 @@ def service
         # @return [Logger]
         attr_reader :logger
 
-        # Registers callback to be called when the properties of the given object changes
+        # Registers a callback to be called when the properties of the given object changes.
+        #
+        # @param dbus_object [::DBus::Object]
+        # @param block [Proc]
+        def on_properties_change(dbus_object, &block)
+          subscribe(dbus_object, "org.freedesktop.DBus.Properties", "PropertiesChanged", &block)
+        end
+
+        # Subscribes to a D-Bus signal.
         #
         # @note Signal subscription is done only once. Otherwise, the latest subscription overrides
         #   the previous one.
         #
         # @param dbus_object [::DBus::Object]
+        # @param interface [String]
+        # @param signal [String]
         # @param block [Proc]
-        def on_properties_change(dbus_object, &block)
-          @on_properties_change_callbacks ||= {}
-          @on_properties_change_callbacks[dbus_object.path] ||= []
-          @on_properties_change_callbacks[dbus_object.path] << block
+        def subscribe(dbus_object, interface, signal, &block)
+          @signal_handlers ||= {}
+          @signal_handlers[dbus_object.path] ||= {}
+          @signal_handlers[dbus_object.path][interface] ||= {}
+          @signal_handlers[dbus_object.path][interface][signal] ||= []
+          @signal_handlers[dbus_object.path][interface][signal] << block
+
+          callbacks = @signal_handlers.dig(dbus_object.path, interface, signal)
 
-          return if @on_properties_change_callbacks[dbus_object.path].size > 1
+          return if callbacks.size > 1
 
-          dbus_properties = dbus_object["org.freedesktop.DBus.Properties"]
-          dbus_properties.on_signal("PropertiesChanged") do |interface, changes, invalid|
-            callbacks = @on_properties_change_callbacks[dbus_object.path]
-            callbacks.each { |c| c.call(interface, changes, invalid) }
+          interface_proxy = dbus_object[interface]
+          interface_proxy.on_signal(signal) do |*args|
+            callbacks.each { |c| c.call(*args) }
           end
         end
 

service/lib/agama/dbus/clients/software.rb

@@ -36,17 +36,10 @@ class Software < Base
         TYPES = [:package, :pattern].freeze
         private_constant :TYPES
 
-        def initialize
-          super
-
-          @dbus_object = service["/org/opensuse/Agama/Software1"]
-          @dbus_object.introspect
-
-          @dbus_product = service["/org/opensuse/Agama/Software1/Product"]
-          @dbus_product.introspect
-
-          @dbus_proposal = service["/org/opensuse/Agama/Software1/Proposal"]
-          @dbus_proposal.introspect
+        # @note This client is singleton because ruby-dbus does not work properly with several
+        #   instances of the same client.
+        def self.instance
+          @instance ||= new
         end
 
         # @return [String]
@@ -176,9 +169,6 @@ def remove_resolvables(unique_id, type, resolvables, optional: false)
 
         # Registers a callback to run when the product changes
         #
-        # @note Signal subscription is done only once. Otherwise, the latest subscription overrides
-        #   the previous one.
-        #
         # @param block [Proc] Callback to run when a product is selected
         def on_product_selected(&block)
           on_properties_change(dbus_product) do |_, changes, _|
@@ -187,6 +177,13 @@ def on_product_selected(&block)
           end
         end
 
+        # Registers a callback to run when the software is probed.
+        #
+        # @param block [Proc]
+        def on_probe_finished(&block)
+          subscribe(dbus_object, "org.opensuse.Agama.Software1", "ProbeFinished", &block)
+        end
+
       private
 
         # @return [::DBus::Object]
@@ -197,6 +194,19 @@ def on_product_selected(&block)
 
         # @return [::DBus::Object]
         attr_reader :dbus_proposal
+
+        def initialize
+          super
+
+          @dbus_object = service["/org/opensuse/Agama/Software1"]
+          @dbus_object.introspect
+
+          @dbus_product = service["/org/opensuse/Agama/Software1/Product"]
+          @dbus_product.introspect
+
+          @dbus_proposal = service["/org/opensuse/Agama/Software1/Proposal"]
+          @dbus_proposal.introspect
+        end
       end
     end
   end

service/lib/agama/dbus/software/manager.rb

@@ -107,6 +107,8 @@ def issues
 
           dbus_method(:UsedDiskSpace, "out SpaceSize:s") { backend.used_disk_space }
 
+          dbus_signal(:ProbeFinished)
+
           dbus_method(:Probe) { probe }
           dbus_method(:Propose) { propose }
           dbus_method(:Install) { install }
@@ -115,6 +117,7 @@ def issues
 
         def probe
           busy_while { backend.probe }
+          self.ProbeFinished
         end
 
         def propose

service/lib/agama/dbus/storage/manager.rb

@@ -60,11 +60,15 @@ class Manager < BaseObject
         def initialize(backend, logger)
           super(PATH, logger: logger)
           @backend = backend
+          @product_mount_points = read_product_mount_points
+          @encryption_methods = read_encryption_methods
+
           register_storage_callbacks
           register_proposal_callbacks
           register_progress_callbacks
           register_service_status_callbacks
           register_iscsi_callbacks
+          register_software_callbacks
 
           add_s390_interfaces if Yast::Arch.s390
         end
@@ -118,18 +122,23 @@ def available_devices
           proposal.available_devices.map { |d| system_devices_tree.path_for(d) }
         end
 
-        # List of meaningful mount points for the current product.
+        # Reads the list of meaningful mount points for the current product.
         #
         # @return [Array<String>]
-        def product_mount_points
-          volume_templates_builder.all.map(&:mount_path).reject(&:empty?)
+        def read_product_mount_points
+          volume_templates_builder
+            .all
+            .map(&:mount_path)
+            .reject(&:empty?)
         end
 
-        # List of possible encryption methods for the current system and product
+        # Reads the list of possible encryption methods for the current system and product.
         #
         # @return [Array<String>]
-        def encryption_methods
-          Agama::Storage::EncryptionSettings.available_methods.map { |m| m.id.to_s }
+        def read_encryption_methods
+          Agama::Storage::EncryptionSettings
+            .available_methods
+            .map { |m| m.id.to_s }
         end
 
         # Path of the D-Bus object containing the calculated proposal
@@ -169,9 +178,11 @@ def calculate_proposal(dbus_settings)
         dbus_interface PROPOSAL_CALCULATOR_INTERFACE do
           dbus_reader :available_devices, "ao"
 
-          dbus_reader :product_mount_points, "as"
+          # PropertiesChanged signal if the product changes, see {#register_software_callbacks}.
+          dbus_reader_attr_accessor :product_mount_points, "as"
 
-          dbus_reader :encryption_methods, "as"
+          # PropertiesChanged signal if software is probed, see {#register_software_callbacks}.
+          dbus_reader_attr_accessor :encryption_methods, "as"
 
           dbus_reader :result, "o"
 
@@ -310,6 +321,18 @@ def register_iscsi_callbacks
           end
         end
 
+        def register_software_callbacks
+          backend.software.on_product_selected do
+            # A PropertiesChanged signal is emitted (see ::DBus::Object.dbus_reader_attr_accessor).
+            self.product_mount_points = read_product_mount_points
+          end
+
+          backend.software.on_probe_finished do
+            # A PropertiesChanged signal is emitted (see ::DBus::Object.dbus_reader_attr_accessor).
+            self.encryption_methods = read_encryption_methods
+          end
+        end
+
         def storage_properties_changed
           properties = interfaces_and_properties[STORAGE_INTERFACE]
           dbus_properties_changed(STORAGE_INTERFACE, properties, [])

service/lib/agama/dbus/y2dir/manager/modules/Package.rb

@@ -26,7 +26,7 @@ module Yast
   class PackageClass < Module
     def main
       puts "Loading mocked module #{__FILE__}"
-      @client = Agama::DBus::Clients::Software.new
+      @client = Agama::DBus::Clients::Software.instance
     end
 
     # Determines whether a package is available.

service/lib/agama/storage/manager.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-# Copyright (c) [2022-2023] SUSE LLC
+# Copyright (c) [2022-2024] SUSE LLC
 #
 # All Rights Reserved.
 #
@@ -157,7 +157,7 @@ def iscsi
       #
       # @return [Agama::DBus::Clients::Software]
       def software
-        @software ||= DBus::Clients::Software.new
+        @software ||= DBus::Clients::Software.instance
       end
 
     private

service/package/rubygem-agama-yast.changes

@@ -1,3 +1,10 @@
+-------------------------------------------------------------------
+Tue May 21 05:32:46 UTC 2024 - José Iván López González <jlopez@suse.com>
+
+- Emit a PropertiesChanged signal for ProductMountPoints and
+  EncryptionMethods properties when the product changes
+  (gh#openSUSE/agama#1236).
+
 -------------------------------------------------------------------
 Fri May 17 09:52:25 UTC 2024 - Imobach Gonzalez Sosa <igonzalezsosa@suse.com>
 

service/test/agama/dbus/clients/software_test.rb

@@ -213,6 +213,31 @@
     end
   end
 
+  describe "#on_probe_finished" do
+    before do
+      allow(dbus_object).to receive(:path).and_return("/org/opensuse/Agama/Test")
+      allow(software_iface).to receive(:on_signal)
+    end
+
+    context "if there are no callbacks for the signal" do
+      it "subscribes to the signal" do
+        expect(software_iface).to receive(:on_signal)
+        subject.on_probe_finished { "test" }
+      end
+    end
+
+    context "if there already are callbacks for the signal" do
+      before do
+        subject.on_probe_finished { "test" }
+      end
+
+      it "does not subscribe to the signal again" do
+        expect(software_iface).to_not receive(:on_signal)
+        subject.on_probe_finished { "test" }
+      end
+    end
+  end
+
   include_examples "issues"
   include_examples "service status"
   include_examples "progress"

service/test/agama/dbus/software/manager_test.rb

@@ -87,10 +87,11 @@
   end
 
   describe "#probe" do
-    it "runs the probing, setting the service as busy meanwhile" do
+    it "runs the probing, setting the service as busy meanwhile, and emits a signal" do
       expect(subject.service_status).to receive(:busy)
       expect(backend).to receive(:probe)
       expect(subject.service_status).to receive(:idle)
+      expect(subject).to receive(:ProbeFinished)
 
       subject.probe
     end

service/test/agama/dbus/storage/manager_test.rb

@@ -68,7 +68,11 @@
       on_sessions_change: nil)
   end
 
-  let(:software) { instance_double(Agama::DBus::Clients::Software, on_product_selected: nil) }
+  let(:software) do
+    instance_double(Agama::DBus::Clients::Software,
+      on_product_selected: nil,
+      on_probe_finished:   nil)
+  end
 
   before do
     allow(Yast::Arch).to receive(:s390).and_return false

service/test/agama/storage/manager_test.rb

@@ -44,10 +44,8 @@
 
   before do
     allow(Agama::DBus::Clients::Questions).to receive(:new).and_return(questions_client)
-    allow(Agama::DBus::Clients::Software).to receive(:new)
-      .and_return(software)
-    allow(Bootloader::FinishClient).to receive(:new)
-      .and_return(bootloader_finish)
+    allow(Agama::DBus::Clients::Software).to receive(:instance).and_return(software)
+    allow(Bootloader::FinishClient).to receive(:new).and_return(bootloader_finish)
     allow(Agama::Security).to receive(:new).and_return(security)
   end