Storage D-Bus reorganization

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

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-# Copyright (c) [2022] SUSE LLC
+# Copyright (c) [2022-2023] SUSE LLC
 #
 # All Rights Reserved.
 #
@@ -33,6 +33,13 @@ class Base
         #   @return [String]
         abstract_method :service_name
 
+        # Constructor
+        #
+        # @param logger [Logger, nil]
+        def initialize(logger: nil)
+          @logger = logger || Logger.new($stdout)
+        end
+
         # D-Bus service
         #
         # @return [::DBus::Service]
@@ -42,6 +49,9 @@ def service
 
       private
 
+        # @return [Logger]
+        attr_reader :logger
+
         # Registers callback to be called when the properties of the given object changes
         #
         # @note Signal subscription is done only once. Otherwise, the latest subscription overrides

service/lib/dinstaller/dbus/clients/question.rb

@@ -25,8 +25,6 @@ module DInstaller
   module DBus
     module Clients
       # D-Bus client for asking a question.
-      # Its interface is a subset of {DInstaller::Question}
-      # so it can be used in the block of {DInstaller::CanAskQuestion#ask}.
       class Question < Base
         LUKS_ACTIVATION_IFACE = "org.opensuse.DInstaller.Question.LuksActivation1"
         private_constant :LUKS_ACTIVATION_IFACE
@@ -51,8 +49,6 @@ def service_name
           @service_name ||= "org.opensuse.DInstaller.Questions"
         end
 
-        # TODO: what other methods are useful?
-
         # @return [String] Question text
         def text
           @dbus_iface["Text"].to_s

service/lib/dinstaller/dbus/clients/questions_manager.rb → service/lib/dinstaller/dbus/clients/questions.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-# Copyright (c) [2022] SUSE LLC
+# Copyright (c) [2022-2023] SUSE LLC
 #
 # All Rights Reserved.
 #
@@ -27,10 +27,11 @@ module DInstaller
   module DBus
     module Clients
       # D-Bus client for asking a question.
-      # It has the same interface as {DInstaller::QuestionsManager}
-      # so it can be used for {DInstaller::CanAskQuestion}.
-      class QuestionsManager < Base
-        def initialize
+      class Questions < Base
+        # Constructor
+        #
+        # @param logger [Logger, nil]
+        def initialize(logger: nil)
           super
 
           @dbus_object = service["/org/opensuse/DInstaller/Questions1"]
@@ -47,31 +48,27 @@ def service_name
         # @param question [DInstaller::Question]
         # @return [DBus::Clients::Question]
         def add(question)
-          q_path = add_dbus_question(question)
-          DBus::Clients::Question.new(q_path)
-        end
-
-        def add_dbus_question(question)
-          if question.is_a?(DInstaller::LuksActivationQuestion)
-            add_luks_activation_question(question)
-          else
-            add_generic_question(question)
-          end
+          dbus_path = add_question(question)
+          DBus::Clients::Question.new(dbus_path)
         end
 
         # Deletes the given question
         #
+        # @raise [::DBus::Error] if trying to delete a question twice
+        #
         # @param question [DBus::Clients::Question]
         # @return [void]
-        # @raise [::DBus::Error] if trying to delete a question twice
         def delete(question)
           @dbus_object.Delete(question.dbus_object.path)
         end
 
-        # Waits until specified questions are answered.
+        # Waits until specified questions are answered
+        #
         # @param questions [Array<DBus::Clients::Question>]
         # @return [void]
         def wait(questions)
+          logger.info "Waiting for questions to be answered"
+
           # TODO: detect if no UI showed up to display the questions and time out?
           # for example:
           # (0..Float::INFINITY).each { |i| break if i > 100 && !question.displayed; ... }
@@ -86,11 +83,50 @@ def wait(questions)
           end
         end
 
+        # Asks the given question and waits until the question is answered
+        #
+        # @example
+        #   ask(question1)                           #=> Symbol
+        #   ask(question2) { |q| q.answer == :yes }  #=> Boolean
+        #
+        # @param question [DInstaller::Question]
+        # @yield [DInstaller::DBus::Clients::Question] Gives the answered question to the block.
+        # @return [Symbol, Object] The question answer, or the result of the block in case a block
+        #   is given.
+        def ask(question)
+          question_client = add(question)
+          wait([question_client])
+
+          answer = question_client.answer
+          logger.info("#{question.text} #{answer}")
+
+          result = block_given? ? yield(question_client) : answer
+          delete(question_client)
+
+          result
+        end
+
       private
 
         # @return [::DBus::Object]
         attr_reader :dbus_object
 
+        # Adds a question using the proper D-Bus method according to the question type
+        #
+        # @param question [DInstaller::Question]
+        # @return [::DBus::ObjectPath]
+        def add_question(question)
+          if question.is_a?(DInstaller::LuksActivationQuestion)
+            add_luks_activation_question(question)
+          else
+            add_generic_question(question)
+          end
+        end
+
+        # Adds a generic question
+        #
+        # @param question [DInstaller::Question]
+        # @return [::DBus::ObjectPath]
         def add_generic_question(question)
           @dbus_object.New(
             question.text,
@@ -99,6 +135,10 @@ def add_generic_question(question)
           )
         end
 
+        # Adds a question for activating LUKS
+        #
+        # @param question [DInstaller::LuksActivationQuestion]
+        # @return [::DBus::ObjectPath]
         def add_luks_activation_question(question)
           @dbus_object.NewLuksActivation(
             question.device, question.label, question.size, question.attempt

service/lib/dinstaller/dbus/clients/storage.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-# Copyright (c) [2022] SUSE LLC
+# Copyright (c) [2022-2023] SUSE LLC
 #
 # All Rights Reserved.
 #
@@ -33,20 +33,15 @@ class Storage < Base
         include WithProgress
         include WithValidation
 
-        PROPOSAL_IFACE = "org.opensuse.DInstaller.Storage.Proposal1"
-        private_constant :PROPOSAL_IFACE
-
-        def initialize
-          super
+        STORAGE_IFACE = "org.opensuse.DInstaller.Storage1.Proposal"
+        private_constant :STORAGE_IFACE
 
-          @dbus_object = service["/org/opensuse/DInstaller/Storage1"]
-          @dbus_object.introspect
+        PROPOSAL_CALCULATOR_IFACE = "org.opensuse.DInstaller.Storage1.Proposal.Calculator"
+        private_constant :PROPOSAL_CALCULATOR_IFACE
 
-          @dbus_proposal = service.object("/org/opensuse/DInstaller/Storage/Proposal1")
-          @dbus_proposal.introspect
-        end
+        PROPOSAL_IFACE = "org.opensuse.DInstaller.Storage1.Proposal"
+        private_constant :PROPOSAL_IFACE
 
-        # @return [String]
         def service_name
           @service_name ||= "org.opensuse.DInstaller.Storage"
         end
@@ -75,21 +70,25 @@ def finish
         #
         # @return [Array<String>] name of the devices
         def available_devices
-          dbus_proposal[PROPOSAL_IFACE]["AvailableDevices"]
+          dbus_object[PROPOSAL_CALCULATOR_IFACE]["AvailableDevices"]
             .map(&:first)
         end
 
         # Devices selected for the installation
         #
         # @return [Array<String>] name of the devices
         def candidate_devices
+          return [] unless dbus_proposal
+
           dbus_proposal[PROPOSAL_IFACE]["CandidateDevices"]
         end
 
         # Actions to perform in the storage devices
         #
         # @return [Array<String>]
         def actions
+          return [] unless dbus_proposal
+
           dbus_proposal[PROPOSAL_IFACE]["Actions"].map do |a|
             a["Text"]
           end
@@ -99,16 +98,24 @@ def actions
         #
         # @param candidate_devices [Array<String>] name of the new candidate devices
         def calculate(candidate_devices)
-          dbus_proposal.Calculate({ "CandidateDevices" => candidate_devices })
+          calculator_iface = dbus_object[PROPOSAL_CALCULATOR_IFACE]
+          calculator_iface.Calculate({ "CandidateDevices" => candidate_devices })
         end
 
       private
 
         # @return [::DBus::Object]
-        attr_reader :dbus_object
+        def dbus_object
+          @dbus_object ||= service["/org/opensuse/DInstaller/Storage1"].tap(&:introspect)
+        end
 
-        # @return [::DBus::Object]
-        attr_reader :dbus_proposal
+        # @return [::DBus::Object, nil]
+        def dbus_proposal
+          path = dbus_object["org.opensuse.DInstaller.Storage1.Proposal.Calculator"]["Result"]
+          return nil if path == "/"
+
+          service.object(path).tap(&:introspect)
+        end
       end
     end
   end