diff --git a/service/lib/dinstaller/can_ask_question.rb b/service/lib/dinstaller/can_ask_question.rb deleted file mode 100644 index 38b94952f2..0000000000 --- a/service/lib/dinstaller/can_ask_question.rb +++ /dev/null @@ -1,50 +0,0 @@ -# frozen_string_literal: true - -# Copyright (c) [2022] SUSE LLC -# -# All Rights Reserved. -# -# This program is free software; you can redistribute it and/or modify it -# under the terms of version 2 of the GNU General Public License as published -# by the Free Software Foundation. -# -# This program is distributed in the hope that it will be useful, but WITHOUT -# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or -# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for -# more details. -# -# You should have received a copy of the GNU General Public License along -# with this program; if not, contact SUSE LLC. -# -# To contact SUSE LLC about this file by physical or electronic mail, you may -# find current contact information at www.suse.com. - -module DInstaller - # Mixin providing a method to ask a question and wait - module CanAskQuestion - # @!method questions_manager - # @note Classes including this mixin must define a #questions_manager method - # @return [QuestionsManager,DBus::Clients::QuestionsManager] - - # Asks the given question and waits until the question is answered - # - # @example - # ask(question1) #=> Symbol - # ask(question2) { |q| q.answer == :yes } #=> Boolean - # - # @param question [Question] - # @yield [Question,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) - # asked_question has the same interface as question - # but it may be a D-Bus proxy, if our questions_manager is also one - asked_question = questions_manager.add(question) - questions_manager.wait([asked_question]) - result = block_given? ? yield(asked_question) : asked_question.answer - questions_manager.delete(asked_question) - - result - end - end -end diff --git a/service/lib/dinstaller/dbus/clients/base.rb b/service/lib/dinstaller/dbus/clients/base.rb index f17b7b2295..91a7d02631 100644 --- a/service/lib/dinstaller/dbus/clients/base.rb +++ b/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 diff --git a/service/lib/dinstaller/dbus/clients/question.rb b/service/lib/dinstaller/dbus/clients/question.rb index 28ade0ba3b..2e6dbfee86 100644 --- a/service/lib/dinstaller/dbus/clients/question.rb +++ b/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 diff --git a/service/lib/dinstaller/dbus/clients/questions_manager.rb b/service/lib/dinstaller/dbus/clients/questions.rb similarity index 63% rename from service/lib/dinstaller/dbus/clients/questions_manager.rb rename to service/lib/dinstaller/dbus/clients/questions.rb index 8f31a410d6..cbc156fae1 100644 --- a/service/lib/dinstaller/dbus/clients/questions_manager.rb +++ b/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] # @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 diff --git a/service/lib/dinstaller/dbus/clients/storage.rb b/service/lib/dinstaller/dbus/clients/storage.rb index b826fd2962..9f1b4b9e1b 100644 --- a/service/lib/dinstaller/dbus/clients/storage.rb +++ b/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,7 +70,7 @@ def finish # # @return [Array] name of the devices def available_devices - dbus_proposal[PROPOSAL_IFACE]["AvailableDevices"] + dbus_object[PROPOSAL_CALCULATOR_IFACE]["AvailableDevices"] .map(&:first) end @@ -83,6 +78,8 @@ def available_devices # # @return [Array] name of the devices def candidate_devices + return [] unless dbus_proposal + dbus_proposal[PROPOSAL_IFACE]["CandidateDevices"] end @@ -90,6 +87,8 @@ def candidate_devices # # @return [Array] 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] 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 diff --git a/service/lib/dinstaller/dbus/questions.rb b/service/lib/dinstaller/dbus/questions.rb index 8fe61a9edb..1049ab411a 100644 --- a/service/lib/dinstaller/dbus/questions.rb +++ b/service/lib/dinstaller/dbus/questions.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2022] SUSE LLC +# Copyright (c) [2022-2023] SUSE LLC # # All Rights Reserved. # @@ -21,18 +21,13 @@ require "dbus" require "pathname" +require "dinstaller/question" +require "dinstaller/luks_activation_question" require "dinstaller/dbus/question" module DInstaller module DBus # This class represents a D-Bus object implementing ObjectManager interface for questions - # - # {DBus::Questions} uses a {QuestionsManager} as backend and exports a {DBus::Question} object - # when a {DInstaller::Question} is added to the questions manager. A {DBus::Question} is - # unexported when a {DInstaller::Question} is deleted from the questions manager. - # - # Callbacks of {QuestionsManager} are used to ensure that the proper D-Bus actions are performed - # when adding, deleting or waiting for answers. class Questions < ::DBus::Object include ::DBus::ObjectManager @@ -44,16 +39,9 @@ class Questions < ::DBus::Object # Constructor # - # The callbacks of the backend are configured to perform the proper D-Bus actions, see - # {#register_callbacks}. - # - # @param backend [DInstaller::QuestionsManager] # @param logger [Logger] - def initialize(backend, logger) - @backend = backend - @logger = logger - - register_callbacks + def initialize(logger: nil) + @logger = logger || Logger.new($stdout) super(PATH) end @@ -63,45 +51,54 @@ def initialize(backend, logger) dbus_method :New, "in text:s, in options:as, in default_option:as, out q:o" do |text, options, default_option| - backend_q = DInstaller::Question.new( + question = DInstaller::Question.new( text, options: options.map(&:to_sym), default_option: default_option.map(&:to_sym).first ) - backend.add(backend_q) - path_for(backend_q) + + export(question) end dbus_method :NewLuksActivation, "in device:s, in label:s, in size:s, in attempt:y, out q:o" do |device, label, size, attempt| - backend_q = DInstaller::LuksActivationQuestion.new( + question = DInstaller::LuksActivationQuestion.new( device, label: label, size: size, attempt: attempt ) - backend.add(backend_q) - path_for(backend_q) + export(question) end dbus_method :Delete, "in question:o" do |question_path| - dbus_q = @service.get_node(question_path)&.object - raise ArgumentError, "Object path #{question_path} not found" unless dbus_q - raise ArgumentError, "Object #{question_path} is not a Question" unless - dbus_q.is_a? DInstaller::DBus::Question + dbus_question = @service.get_node(question_path)&.object + + raise ArgumentError, "Object path #{question_path} not found" unless dbus_question + + if !dbus_question.is_a?(DInstaller::DBus::Question) + raise ArgumentError, "Object #{question_path} is not a Question" + end - backend_q = dbus_q.backend - backend.delete(backend_q) + @service.unexport(dbus_question) end end private - # @return [DInstaller::QuestionsManager] - attr_reader :backend - # @return [Logger] attr_reader :logger + # Exports a new question object + # + # @param question [DInstaller::Question] + # @return [::DBus::ObjectPath] + def export(question) + dbus_question = DBus::Question.new(path_for(question), question, logger) + @service.export(dbus_question) + + dbus_question.path + end + # Builds the question path (e.g., /org/opensuse/DInstaller/Questions1/1) # # @param question [DInstaller::Question] @@ -111,27 +108,6 @@ def path_for(question) ::DBus::ObjectPath.new(path.to_s) end - - # Callbacks with actions to do when adding, deleting or waiting for questions - def register_callbacks - # When adding a question, a new question is exported on D-Bus. - backend.on_add do |question| - dbus_object = DBus::Question.new(path_for(question), question, logger) - @service.export(dbus_object) - end - - # When removing a question, the question is unexported from D-Bus. - backend.on_delete do |question| - dbus_object = @service.descendants_for(PATH).find do |q| - q.is_a?(DBus::Question) && q.id == question.id - end - - @service.unexport(dbus_object) if dbus_object - end - - # Bus dispatches messages while waiting for questions to be answered - backend.on_wait { @service.bus.dispatch_message_queue } - end end end end diff --git a/service/lib/dinstaller/dbus/questions_service.rb b/service/lib/dinstaller/dbus/questions_service.rb index dd7083f62f..7590b37129 100644 --- a/service/lib/dinstaller/dbus/questions_service.rb +++ b/service/lib/dinstaller/dbus/questions_service.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2022] SUSE LLC +# Copyright (c) [2022-2023] SUSE LLC # # All Rights Reserved. # @@ -22,14 +22,13 @@ require "dbus" require "dinstaller/dbus/bus" require "dinstaller/dbus/questions" -require "dinstaller/questions_manager" module DInstaller module DBus - # D-Bus service (org.opensuse.DInstaller.Software) + # D-Bus service (org.opensuse.DInstaller.Questions) # # It connects to the system D-Bus and answers requests on objects below - # `/org/opensuse/DInstaller/Software`. + # `/org/opensuse/DInstaller/Questions1`. class QuestionsService SERVICE_NAME = "org.opensuse.DInstaller.Questions" private_constant :SERVICE_NAME @@ -39,12 +38,13 @@ class QuestionsService # @return [::DBus::Connection] attr_reader :bus + # Constructor + # # @param _config [Config] Configuration object # @param logger [Logger] def initialize(_config, logger = nil) @logger = logger || Logger.new($stdout) @bus = Bus.current - @backend ||= DInstaller::QuestionsManager.new(logger) end # Exports the software object through the D-Bus service @@ -62,7 +62,7 @@ def dispatch private # @return [Logger] - attr_reader :logger, :backend + attr_reader :logger # @return [::DBus::Service] def service @@ -72,7 +72,7 @@ def service # @return [Array<::DBus::Object>] def dbus_objects @dbus_objects ||= [ - DInstaller::DBus::Questions.new(backend, logger) + DInstaller::DBus::Questions.new(logger: logger) ] end end diff --git a/service/lib/dinstaller/dbus/storage/manager.rb b/service/lib/dinstaller/dbus/storage/manager.rb index 86be7f741b..0f751d7eb8 100644 --- a/service/lib/dinstaller/dbus/storage/manager.rb +++ b/service/lib/dinstaller/dbus/storage/manager.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2022] SUSE LLC +# Copyright (c) [2022-2023] SUSE LLC # # All Rights Reserved. # @@ -25,16 +25,19 @@ require "dinstaller/dbus/interfaces/progress" require "dinstaller/dbus/interfaces/service_status" require "dinstaller/dbus/interfaces/validation" +require "dinstaller/dbus/storage/proposal_settings_converter" +require "dinstaller/dbus/storage/volume_converter" module DInstaller module DBus module Storage - # D-Bus object to manage software installation + # D-Bus object to manage storage installation class Manager < BaseObject include WithServiceStatus - include Interfaces::Progress - include Interfaces::ServiceStatus - include Interfaces::Validation + include ::DBus::ObjectManager + include DBus::Interfaces::Progress + include DBus::Interfaces::ServiceStatus + include DBus::Interfaces::Validation PATH = "/org/opensuse/DInstaller/Storage1" private_constant :PATH @@ -72,13 +75,92 @@ def finish busy_while { backend.finish } end + PROPOSAL_CALCULATOR_INTERFACE = "org.opensuse.DInstaller.Storage1.Proposal.Calculator" + private_constant :PROPOSAL_CALCULATOR_INTERFACE + + dbus_interface PROPOSAL_CALCULATOR_INTERFACE do + dbus_reader :available_devices, "a(ssa{sv})" + + dbus_reader :volume_templates, "aa{sv}" + + dbus_reader :result, "o" + + # result: 0 success; 1 error + dbus_method :Calculate, "in settings:a{sv}, out result:u" do |settings| + busy_while { calculate_proposal(settings) } + end + end + + # List of disks available for installation + # + # Each device is represented by an array containing the name of the device and the label to + # represent that device in the UI when further information is needed. + # + # @return [Array] + def available_devices + proposal.available_devices.map do |dev| + [dev.name, proposal.device_label(dev), {}] + end + end + + # Volumes used as template for creating a new proposal + # + # @return [Hash] + def volume_templates + converter = VolumeConverter.new + proposal.volume_templates.map { |v| converter.to_dbus(v) } + end + + # Path of the D-Bus object containing the calculated proposal + # + # @return [::DBus::ObjectPath] Proposal object path or root path if no exported proposal yet + def result + dbus_proposal&.path || ::DBus::ObjectPath.new("/") + end + + # Calculates a new proposal + # + # @param dbus_settings [Hash] + # @return [Integer] 0 success; 1 error + def calculate_proposal(dbus_settings) + logger.info("Calculating storage proposal from D-Bus settings: #{dbus_settings}") + + converter = ProposalSettingsConverter.new + success = proposal.calculate(converter.to_dinstaller(dbus_settings)) + + success ? 0 : 1 + end + private - # @return [DInstaller::Software::Manager] + # @return [DInstaller::Storage::Manager] attr_reader :backend + # @return [DBus::Storage::Proposal, nil] + attr_reader :dbus_proposal + + # @return [DInstaller::Storage::Proposal] + def proposal + backend.proposal + end + def register_proposal_callbacks - backend.proposal.on_calculate { update_validation } + proposal.on_calculate do + export_proposal + properties_changed + update_validation + end + end + + def properties_changed + properties = interfaces_and_properties[PROPOSAL_CALCULATOR_INTERFACE] + dbus_properties_changed(PROPOSAL_CALCULATOR_INTERFACE, properties, []) + end + + def export_proposal + @service.unexport(dbus_proposal) if dbus_proposal + @dbus_proposal = DBus::Storage::Proposal.new(proposal, logger) + @service.export(@dbus_proposal) end end end diff --git a/service/lib/dinstaller/dbus/storage/proposal.rb b/service/lib/dinstaller/dbus/storage/proposal.rb index 053e022d64..df2a6c0dfc 100644 --- a/service/lib/dinstaller/dbus/storage/proposal.rb +++ b/service/lib/dinstaller/dbus/storage/proposal.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2022] SUSE LLC +# Copyright (c) [2022-2023] SUSE LLC # # All Rights Reserved. # @@ -21,20 +21,14 @@ require "dbus" require "dinstaller/dbus/base_object" -require "dinstaller/dbus/with_service_status" -require "dinstaller/dbus/interfaces/service_status" -require "dinstaller/storage/proposal_settings" -require "dinstaller/storage/volume" +require "dinstaller/dbus/storage/volume_converter" module DInstaller module DBus module Storage - # D-Bus object to manage a storage proposal + # D-Bus object to manage the storage proposal class Proposal < BaseObject - include WithServiceStatus - include Interfaces::ServiceStatus - - PATH = "/org/opensuse/DInstaller/Storage/Proposal1" + PATH = "/org/opensuse/DInstaller/Storage1/Proposal" private_constant :PATH # Constructor @@ -44,50 +38,21 @@ class Proposal < BaseObject def initialize(backend, logger) super(PATH, logger: logger) @backend = backend - - register_callbacks - register_service_status_callbacks end - STORAGE_PROPOSAL_INTERFACE = "org.opensuse.DInstaller.Storage.Proposal1" + STORAGE_PROPOSAL_INTERFACE = "org.opensuse.DInstaller.Storage1.Proposal" private_constant :STORAGE_PROPOSAL_INTERFACE dbus_interface STORAGE_PROPOSAL_INTERFACE do - dbus_reader :available_devices, "a(ssa{sv})" - dbus_reader :candidate_devices, "as" dbus_reader :lvm, "b", dbus_name: "LVM" dbus_reader :encryption_password, "s" - # @see {#to_dbus_volume} - dbus_reader :volume_templates, "aa{sv}" - - # @see {#to_dbus_volume} dbus_reader :volumes, "aa{sv}" dbus_reader :actions, "aa{sv}" - - # result: 0 success; 1 error - dbus_method :Calculate, "in settings:a{sv}, out result:u" do |settings| - success = busy_while { calculate(settings) } - - success ? 0 : 1 - end - end - - # List of disks available for installation - # - # Each device is represented by an array containing the name of the device (as expected by - # {#calculate} for the setting CandidateDevices), the second one is the label to represent - # that device in the UI when further information is needed. - # - # @return [Array] - def available_devices - backend.available_devices.map do |dev| - [dev.name, backend.device_label(dev), {}] - end end # Devices used by the storage proposal @@ -115,20 +80,14 @@ def encryption_password backend.calculated_settings&.encryption_password || "" end - # Volumes used as template for creating a new volume - # - # @return [Hash] - def volume_templates - backend.volume_templates.map { |v| to_dbus_volume(v) } - end - # Volumes used to calculate the storage proposal # # @return [Hash] def volumes return [] unless backend.calculated_settings - backend.calculated_settings.volumes.map { |v| to_dbus_volume(v) } + converter = VolumeConverter.new + backend.calculated_settings.volumes.map { |v| converter.to_dbus(v) } end # List of sorted actions in D-Bus format @@ -140,15 +99,6 @@ def actions backend.actions.map { |a| to_dbus_action(a) } end - # Calculates a new proposal - # - # @param dbus_settings [DInstaller::Storage::ProposalSettings] - def calculate(dbus_settings) - logger.info("Calculating storage proposal from D-Bus settings: #{dbus_settings}") - - backend.calculate(to_proposal_settings(dbus_settings)) - end - private # @return [DInstaller::Storage::Proposal] @@ -157,109 +107,6 @@ def calculate(dbus_settings) # @return [Logger] attr_reader :logger - # Registers callback to be called when the proposal is calculated - def register_callbacks - backend.on_calculate do - properties = interfaces_and_properties[STORAGE_PROPOSAL_INTERFACE] - dbus_properties_changed(STORAGE_PROPOSAL_INTERFACE, properties, []) - end - end - - # Relationship between D-Bus settings and ProposalSettings - # - # For each D-Bus setting there is a list with the setter to use and the conversion from a - # D-Bus value to the value expected by the ProposalSettings setter. - SETTINGS_CONVERSIONS = { - "CandidateDevices" => ["candidate_devices=", proc { |v| v }], - "LVM" => ["lvm=", proc { |v| v }], - "EncryptionPassword" => ["encryption_password=", proc { |v| v.empty? ? nil : v }], - "Volumes" => ["volumes=", proc { |v, o| o.send(:to_proposal_volumes, v) }] - }.freeze - private_constant :SETTINGS_CONVERSIONS - - # Converts settings from D-Bus format to ProposalSettings - # - # @param dbus_settings [Hash] - # @return [DInstaller::Storage::ProposalSettings] - def to_proposal_settings(dbus_settings) - DInstaller::Storage::ProposalSettings.new.tap do |proposal_settings| - dbus_settings.each do |dbus_property, dbus_value| - setter, value_converter = SETTINGS_CONVERSIONS[dbus_property] - proposal_settings.public_send(setter, value_converter.call(dbus_value, self)) - end - end - end - - # Relationship between D-Bus volumes and Volumes - # - # For each D-Bus volume setting there is a list with the setter to use and the conversion - # from a D-Bus value to the value expected by the Volume setter. - VOLUME_CONVERSIONS = { - "MountPoint" => ["mount_point=", proc { |v| v }], - "DeviceType" => ["device_type=", proc { |v| v.to_sym }], - "Encrypted" => ["encrypted=", proc { |v| v }], - "FsType" => ["fs_type=", proc { |v, o| o.send(:to_fs_type, v) }], - "MinSize" => ["min_size=", proc { |v| Y2Storage::DiskSize.new(v) }], - "MaxSize" => ["max_size=", proc { |v| Y2Storage::DiskSize.new(v) }], - "FixedSizeLimits" => ["fixed_size_limits=", proc { |v| v }], - "Snapshots" => ["snapshots=", proc { |v| v }] - }.freeze - private_constant :VOLUME_CONVERSIONS - - # Converts volumes from D-Bus format to a list of Volumes - # - # @param dbus_volumes [Array] - # @return [Array] - def to_proposal_volumes(dbus_volumes) - dbus_volumes.map { |v| to_proposal_volume(v) } - end - - # Converts a volume from D-Bus format to Volume - # - # @param dbus_volume [Hash] - # @return [DInstaller::Storage::Volume] - def to_proposal_volume(dbus_volume) - DInstaller::Storage::Volume.new.tap do |volume| - dbus_volume.each do |dbus_property, dbus_value| - setter, value_converter = VOLUME_CONVERSIONS[dbus_property] - volume.public_send(setter, value_converter.call(dbus_value, self)) - end - end - end - - # Converts a filesystem type from D-Bus format to a real filesystem type object - # - # @param dbus_fs_type [String] - # @return [Y2Storage::Filesystems::Type] - def to_fs_type(dbus_fs_type) - Y2Storage::Filesystems::Type.all.find { |t| t.to_human_string == dbus_fs_type } - end - - # Converts a Volume to D-Bus format - # - # @param volume {DInstaller::Storage::Volume} - # @return [Hash] - def to_dbus_volume(volume) - dbus_volume = { - "MountPoint" => volume.mount_point, - "Optional" => volume.optional, - "DeviceType" => volume.device_type.to_s, - "Encrypted" => volume.encrypted, - "FsTypes" => volume.fs_types.map(&:to_human_string), - "FsType" => volume.fs_type&.to_human_string, - "MinSize" => volume.min_size&.to_i, - "MaxSize" => volume.max_size&.to_i, - "FixedSizeLimits" => volume.fixed_size_limits, - "AdaptiveSizes" => volume.adaptive_sizes?, - "Snapshots" => volume.snapshots, - "SnapshotsConfigurable" => volume.snapshots_configurable, - "SnapshotsAffectSizes" => volume.snapshots_affect_sizes?, - "SizeRelevantVolumes" => volume.size_relevant_volumes - } - - dbus_volume.compact.reject { |_, v| v.respond_to?(:empty?) && v.empty? } - end - # Converts an action to D-Bus format # # @param action [Y2Storage::CompoundAction] diff --git a/service/lib/dinstaller/dbus/storage/proposal_settings_converter.rb b/service/lib/dinstaller/dbus/storage/proposal_settings_converter.rb new file mode 100644 index 0000000000..a309e379a8 --- /dev/null +++ b/service/lib/dinstaller/dbus/storage/proposal_settings_converter.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +# Copyright (c) [2023] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +require "dinstaller/storage/proposal_settings" +require "dinstaller/dbus/storage/volume_converter" + +module DInstaller + module DBus + module Storage + # Utility class offering methods to convert volumes between D-Installer and D-Bus formats + # + # @note In the future this class might be not needed if proposal volumes and templates are + # exported as objects in D-Bus. + class ProposalSettingsConverter + # Converts the given D-Bus settings to its equivalent D-Installer proposal settings + # + # @param dbus_settings [Hash] + # @return [DInstaller::Storage::ProposalSettings] + def to_dinstaller(dbus_settings) + ToDInstaller.new(dbus_settings).convert + end + + # Internal class to generate a D-Installer proposal settings + class ToDInstaller + # Constructor + # + # @param dbus_settings [Hash] + def initialize(dbus_settings) + @dbus_settings = dbus_settings + end + + # Converts settings from D-Bus to D-Installer format + # + # @return [DInstaller::Storage::ProposalSettings] + def convert + DInstaller::Storage::ProposalSettings.new.tap do |proposal_settings| + dbus_settings.each do |dbus_property, dbus_value| + setter, value_converter = SETTINGS_CONVERSIONS[dbus_property] + proposal_settings.public_send(setter, value_converter.call(dbus_value, self)) + end + end + end + + private + + # @return [Hash] + attr_reader :dbus_settings + + # Relationship between D-Bus settings and D-Installer proposal settings + # + # For each D-Bus setting there is a list with the setter to use and the conversion from a + # D-Bus value to the value expected by the ProposalSettings setter. + SETTINGS_CONVERSIONS = { + "CandidateDevices" => ["candidate_devices=", proc { |v| v }], + "LVM" => ["lvm=", proc { |v| v }], + "EncryptionPassword" => ["encryption_password=", proc { |v| v.empty? ? nil : v }], + "Volumes" => ["volumes=", proc { |v, o| o.send(:to_dinstaller_volumes, v) }] + }.freeze + private_constant :SETTINGS_CONVERSIONS + + # Converts volumes from D-Bus to the D-Installer format + # + # @param dbus_volumes [Array] + # @return [Array] + def to_dinstaller_volumes(dbus_volumes) + converter = VolumeConverter.new + dbus_volumes.map { |v| converter.to_dinstaller(v) } + end + end + end + end + end +end diff --git a/service/lib/dinstaller/dbus/storage/volume_converter.rb b/service/lib/dinstaller/dbus/storage/volume_converter.rb new file mode 100644 index 0000000000..0093dad50c --- /dev/null +++ b/service/lib/dinstaller/dbus/storage/volume_converter.rb @@ -0,0 +1,138 @@ +# frozen_string_literal: true + +# Copyright (c) [2023] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +require "dinstaller/storage/volume" +require "y2storage/disk_size" +require "y2storage/filesystems/type" + +module DInstaller + module DBus + module Storage + # Utility class offering methods to convert volumes between D-Installer and D-Bus formats + # + # @note In the future this class might be not needed if proposal volumes and templates are + # exported as objects in D-Bus. + class VolumeConverter + # Converts the given D-Bus volume to its equivalent DInstaller::Volume object + # + # @param dbus_volume [Hash] + # @return [Storage::Volume] + def to_dinstaller(dbus_volume) + ToDInstaller.new(dbus_volume).convert + end + + # Converts the given volume to its equivalent D-Bus volume + + # @param volume [Storage::Volume] + # @return [Hash] + def to_dbus(volume) + ToDBus.new(volume).convert + end + + # Internal class to generate a DInstaller volume + class ToDInstaller + # Constructor + # + # @param dbus_volume [Hash] + def initialize(dbus_volume) + @dbus_volume = dbus_volume + end + + # @return [Storage::Volume] + def convert + DInstaller::Storage::Volume.new.tap do |volume| + dbus_volume.each do |dbus_property, dbus_value| + setter, value_converter = VOLUME_CONVERSIONS[dbus_property] + volume.public_send(setter, value_converter.call(dbus_value, self)) + end + end + end + + private + + # @return [Hash] + attr_reader :dbus_volume + + # Relationship between D-Bus volumes and Volumes + # + # For each D-Bus volume setting there is a list with the setter to use and the conversion + # from a D-Bus value to the value expected by the Volume setter. + VOLUME_CONVERSIONS = { + "MountPoint" => ["mount_point=", proc { |v| v }], + "DeviceType" => ["device_type=", proc { |v| v.to_sym }], + "Encrypted" => ["encrypted=", proc { |v| v }], + "FsType" => ["fs_type=", proc { |v, o| o.send(:to_fs_type, v) }], + "MinSize" => ["min_size=", proc { |v| Y2Storage::DiskSize.new(v) }], + "MaxSize" => ["max_size=", proc { |v| Y2Storage::DiskSize.new(v) }], + "FixedSizeLimits" => ["fixed_size_limits=", proc { |v| v }], + "Snapshots" => ["snapshots=", proc { |v| v }] + }.freeze + private_constant :VOLUME_CONVERSIONS + + # Converts a filesystem type from D-Bus format to a real filesystem type object + # + # @param dbus_fs_type [String] + # @return [Y2Storage::Filesystems::Type] + def to_fs_type(dbus_fs_type) + Y2Storage::Filesystems::Type.all.find { |t| t.to_human_string == dbus_fs_type } + end + end + + # Internal class to generate a D-Bus volume + class ToDBus + # Constructor + # + # @param volume [Storage::Volume] + def initialize(volume) + @volume = volume + end + + # @return [Hash] + def convert # rubocop:disable Metrics/AbcSize + dbus_volume = { + "MountPoint" => volume.mount_point, + "Optional" => volume.optional, + "DeviceType" => volume.device_type.to_s, + "Encrypted" => volume.encrypted, + "FsTypes" => volume.fs_types.map(&:to_human_string), + "FsType" => volume.fs_type&.to_human_string, + "MinSize" => volume.min_size&.to_i, + "MaxSize" => volume.max_size&.to_i, + "FixedSizeLimits" => volume.fixed_size_limits, + "AdaptiveSizes" => volume.adaptive_sizes?, + "Snapshots" => volume.snapshots, + "SnapshotsConfigurable" => volume.snapshots_configurable, + "SnapshotsAffectSizes" => volume.snapshots_affect_sizes?, + "SizeRelevantVolumes" => volume.size_relevant_volumes + } + + dbus_volume.compact.reject { |_, v| v.respond_to?(:empty?) && v.empty? } + end + + private + + # @return [Storage::Volume] + attr_reader :volume + end + end + end + end +end diff --git a/service/lib/dinstaller/dbus/storage_service.rb b/service/lib/dinstaller/dbus/storage_service.rb index 2ecbec64ae..003f0783e4 100644 --- a/service/lib/dinstaller/dbus/storage_service.rb +++ b/service/lib/dinstaller/dbus/storage_service.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2022] SUSE LLC +# Copyright (c) [2022-2023] SUSE LLC # # All Rights Reserved. # @@ -73,8 +73,7 @@ def service # @return [Array<::DBus::Object>] def dbus_objects @dbus_objects ||= [ - DInstaller::DBus::Storage::Manager.new(@backend, logger), - DInstaller::DBus::Storage::Proposal.new(@backend.proposal, logger) + DInstaller::DBus::Storage::Manager.new(@backend, logger) ] end end diff --git a/service/lib/dinstaller/dbus/y2dir/software/modules/PackageCallbacks.rb b/service/lib/dinstaller/dbus/y2dir/software/modules/PackageCallbacks.rb index f4a57a5692..0c6e32674d 100644 --- a/service/lib/dinstaller/dbus/y2dir/software/modules/PackageCallbacks.rb +++ b/service/lib/dinstaller/dbus/y2dir/software/modules/PackageCallbacks.rb @@ -1,4 +1,4 @@ -# Copyright (c) [2022] SUSE LLC +# Copyright (c) [2022-2023] SUSE LLC # # All Rights Reserved. # @@ -20,7 +20,7 @@ require "yast" require "logger" require "dinstaller/software/callbacks" -require "dinstaller/dbus/clients/questions_manager" +require "dinstaller/dbus/clients/questions" # :nodoc: module Yast @@ -31,19 +31,29 @@ def main end # @see https://github.com/yast/yast-yast2/blob/19180445ab935a25edd4ae0243aa7a3bcd09c9de/library/packages/src/modules/PackageCallbacks.rb#L183 - def InitPackageCallbacks(logger = ::Logger.new($stdout)) + def InitPackageCallbacks(logger = nil) + @logger = logger || ::Logger.new($stdout) + DInstaller::Software::Callbacks::Signature.new( - questions_manager, logger + questions_client, logger ).setup DInstaller::Software::Callbacks::Media.new( - questions_manager, logger + questions_client, logger ).setup end - def questions_manager - @questions_manager ||= DInstaller::DBus::Clients::QuestionsManager.new + # Returns the client to ask questions + # + # @return [DInstaller::DBus::Clients::Questions] + def questions_client + @questions_client ||= DInstaller::DBus::Clients::Questions.new(logger: logger) end + + private + + # @return [Logger] + attr_reader :logger end PackageCallbacks = PackageCallbacksClass.new diff --git a/service/lib/dinstaller/question.rb b/service/lib/dinstaller/question.rb index 0b5abdc17a..439889089c 100644 --- a/service/lib/dinstaller/question.rb +++ b/service/lib/dinstaller/question.rb @@ -24,8 +24,6 @@ module DInstaller # # Questions are used when some information needs to be asked. For example, a question could be # created for asking whether to continue or not when an error is detected. - # - # Questions are managed by a questions manager, see {QuestionsManager}. class Question # Each question is identified by an unique id # diff --git a/service/lib/dinstaller/questions_manager.rb b/service/lib/dinstaller/questions_manager.rb deleted file mode 100644 index ee64c92106..0000000000 --- a/service/lib/dinstaller/questions_manager.rb +++ /dev/null @@ -1,142 +0,0 @@ -# frozen_string_literal: true - -# Copyright (c) [2022] SUSE LLC -# -# All Rights Reserved. -# -# This program is free software; you can redistribute it and/or modify it -# under the terms of version 2 of the GNU General Public License as published -# by the Free Software Foundation. -# -# This program is distributed in the hope that it will be useful, but WITHOUT -# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or -# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for -# more details. -# -# You should have received a copy of the GNU General Public License along -# with this program; if not, contact SUSE LLC. -# -# To contact SUSE LLC about this file by physical or electronic mail, you may -# find current contact information at www.suse.com. - -module DInstaller - # Manager for questions - # - # Allows to configure callbacks with the actions to perform when adding, deleting or waiting for - # questions. - class QuestionsManager - # @return [Array] - attr_reader :questions - - # Constructor - # - # @param logger [Logger] - def initialize(logger) - @logger = logger - @questions = [] - @on_add_callbacks = [] - @on_delete_callbacks = [] - @on_wait_callbacks = [] - end - - # Adds a question - # - # Callbacks are called after adding the question, see {#on_add}. - # - # @yieldparam question [Question] added question - # - # @param question [Question] - # @return [Question,nil] the actually added question (to be passed to {#delete} later) - def add(question) - return nil if include?(question) - - questions << question - on_add_callbacks.each { |c| c.call(question) } - - question - end - - # Deletes the given question - # - # Callbacks are called after deleting the question, see {#on_delete}. - # - # @yieldparam question [Question] deleted question - # - # @param question [Question] - # @return [Question,nil] whether the question was deleted - def delete(question) - return nil unless include?(question) - - questions.delete(question) - on_delete_callbacks.each { |c| c.call(question) } - - question - end - - # Waits until all specified questions are answered. - # There may be other questions, asked from other services, which - # are waited for by remote question managers, so we ignore those. - # - # Callbacks are periodically called while waiting, see {#on_wait}. - # @param questions [Array] - def wait(questions) - logger.info "Waiting for questions to be answered" - - loop do - on_wait_callbacks.each(&:call) - break if questions.all?(&:answered?) - - sleep(0.1) - end - end - - # Registers a callback to be called when a new question is added - # - # @param block [Proc] - def on_add(&block) - on_add_callbacks << block - end - - # Registers a callback to be called when a question is deleted - # - # @param block [Proc] - def on_delete(&block) - on_delete_callbacks << block - end - - # Registers a callback to be called while waiting for questions be answered - # - # @param block [Proc] - def on_wait(&block) - on_wait_callbacks << block - end - - private - - # @return [Logger] - attr_reader :logger - - # Callbacks to be called when the a new question is added - # - # @return [Array] - attr_reader :on_add_callbacks - - # Callbacks to be called when the a question is deleted - # - # @return [Array] - attr_reader :on_delete_callbacks - - # Callbacks to be called when waiting for answers - # - # @return [Array] - attr_reader :on_wait_callbacks - - # Whether a question with the same id as the given question is already in the list of questions - # - # @param question [Question] - # @return [Boolean] - def include?(question) - questions.any? { |q| q.id == question.id } - end - end -end diff --git a/service/lib/dinstaller/software/callbacks/media.rb b/service/lib/dinstaller/software/callbacks/media.rb index 046daaecc2..662f87ca47 100644 --- a/service/lib/dinstaller/software/callbacks/media.rb +++ b/service/lib/dinstaller/software/callbacks/media.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2021] SUSE LLC +# Copyright (c) [2021-2023] SUSE LLC # # All Rights Reserved. # @@ -20,7 +20,6 @@ # find current contact information at www.suse.com. require "yast" -require "dinstaller/can_ask_question" require "dinstaller/question" Yast.import "Pkg" @@ -30,12 +29,12 @@ module Software module Callbacks # Callbacks related to media handling class Media - include CanAskQuestion - - # @param questions_manager [DBus::Clients::QuestionsManager] + # Constructor + # + # @param questions_client [DInstaller::DBus::Clients::Questions] # @param logger [Logger] - def initialize(questions_manager, logger) - @questions_manager = questions_manager + def initialize(questions_client, logger) + @questions_client = questions_client @logger = logger end @@ -60,18 +59,16 @@ def media_change(_error_code, error, _url, _product, _current, _current_label, _ question = DInstaller::Question.new( error, options: [:Retry, :Skip], default_option: :Retry ) - ask(question) do |q| - logger.info "#{q.text}: #{q.answer}" - - (q.answer == :Retry) ? "" : "S" + questions_client.ask(question) do |question_client| + (question_client.answer == :Retry) ? "" : "S" end end # rubocop:enable Metrics/ParameterLists private - # @return [DBus::Clients::QuestionsManager] - attr_reader :questions_manager + # @return [DInstaller::DBus::Clients::Questions] + attr_reader :questions_client # @return [Logger] attr_reader :logger diff --git a/service/lib/dinstaller/software/callbacks/signature.rb b/service/lib/dinstaller/software/callbacks/signature.rb index 6f2de3d554..36317f1b35 100644 --- a/service/lib/dinstaller/software/callbacks/signature.rb +++ b/service/lib/dinstaller/software/callbacks/signature.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2022] SUSE LLC +# Copyright (c) [2022-2023] SUSE LLC # # All Rights Reserved. # @@ -20,7 +20,6 @@ # find current contact information at www.suse.com. require "yast" -require "dinstaller/can_ask_question" require "dinstaller/question" Yast.import "Pkg" @@ -30,12 +29,12 @@ module Software module Callbacks # Callbacks related to signatures handling class Signature - include CanAskQuestion - - # @param questions_manager [DBus::Clients::QuestionsManager] + # Constructor + # + # @param questions_client [DInstaller::DBus::Clients::Questions] # @param logger [Logger] - def initialize(questions_manager, logger) - @questions_manager = questions_manager + def initialize(questions_client, logger) + @questions_client = questions_client @logger = logger end @@ -72,9 +71,8 @@ def accept_unsigned_file(filename, repo_id) question = DInstaller::Question.new( message, options: [:Yes, :No], default_option: :No ) - ask(question) do |q| - logger.info "#{q.text} #{q.answer}" - q.answer == :Yes + questions_client.ask(question) do |question_client| + question_client.answer == :Yes end end @@ -94,16 +92,15 @@ def import_gpg_key(key, _repo_id) message, options: [:Trust, :Skip], default_option: :Skip ) - ask(question) do |q| - logger.info "#{q.text} #{q.answer}" - q.answer == :Trust + questions_client.ask(question) do |question_client| + question_client.answer == :Trust end end private - # @return [DBus::Clients::QuestionsManager] - attr_reader :questions_manager + # @return [DInstaller::DBus::Clients::Questions] + attr_reader :questions_client # @return [Logger] attr_reader :logger diff --git a/service/lib/dinstaller/storage/callbacks/activate.rb b/service/lib/dinstaller/storage/callbacks/activate.rb index 2406513899..213c620594 100644 --- a/service/lib/dinstaller/storage/callbacks/activate.rb +++ b/service/lib/dinstaller/storage/callbacks/activate.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2022] SUSE LLC +# Copyright (c) [2022-2023] SUSE LLC # # All Rights Reserved. # @@ -33,12 +33,12 @@ class Activate < ::Storage::ActivateCallbacksLuks # Constructor # - # @param questions_manager [QuestionsManager] + # @param questions_client [DInstaller::DBus::Clients::Questions] # @param logger [Logger] - def initialize(questions_manager, logger) + def initialize(questions_client, logger) super() - @questions_manager = questions_manager + @questions_client = questions_client @logger = logger @issues = Y2Issues::List.new end @@ -56,7 +56,7 @@ def message(_message); end # device. # @return [Boolean] def multipath(looks_like_real_multipath) - callback = ActivateMultipath.new(questions_manager, logger) + callback = ActivateMultipath.new(questions_client, logger) callback.call(looks_like_real_multipath) end @@ -70,7 +70,7 @@ def multipath(looks_like_real_multipath) # # @return [Storage::PairBoolString] Whether to activate the device and the password def luks(info, attempt) - callback = ActivateLuks.new(questions_manager, logger) + callback = ActivateLuks.new(questions_client, logger) activate, password = callback.call(info, attempt) @@ -79,8 +79,8 @@ def luks(info, attempt) private - # @return [QuestionsManager] - attr_reader :questions_manager + # @return [DInstaller::DBus::Clients::Questions] + attr_reader :questions_client # @return [Logger] attr_reader :logger diff --git a/service/lib/dinstaller/storage/callbacks/activate_luks.rb b/service/lib/dinstaller/storage/callbacks/activate_luks.rb index 3cf7664ee6..8ab8cd27c3 100644 --- a/service/lib/dinstaller/storage/callbacks/activate_luks.rb +++ b/service/lib/dinstaller/storage/callbacks/activate_luks.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2022] SUSE LLC +# Copyright (c) [2022-2023] SUSE LLC # # All Rights Reserved. # @@ -20,7 +20,6 @@ # find current contact information at www.suse.com. require "dinstaller/luks_activation_question" -require "dinstaller/can_ask_question" require "y2storage/disk_size" module DInstaller @@ -28,14 +27,12 @@ module Storage module Callbacks # Callbacks for LUKS activation class ActivateLuks - include CanAskQuestion - # Constructor # - # @param questions_manager [QuestionsManager] + # @param questions_client [DInstaller::DBus::Clients::Questions] # @param logger [Logger] - def initialize(questions_manager, logger) - @questions_manager = questions_manager + def initialize(questions_client, logger) + @questions_client = questions_client @logger = logger end @@ -52,11 +49,9 @@ def initialize(questions_manager, logger) def call(info, attempt) question = question(info, attempt) - ask(question) do |q| - logger.info("#{q.text} #{q.answer}") - - activate = q.answer == :decrypt - password = q.password + questions_client.ask(question) do |question_client| + activate = question_client.answer == :decrypt + password = question_client.password [activate, password] end @@ -64,8 +59,8 @@ def call(info, attempt) private - # @return [QuestionsManager] - attr_reader :questions_manager + # @return [DInstaller::DBus::Clients::Questions] + attr_reader :questions_client # @return [Logger] attr_reader :logger diff --git a/service/lib/dinstaller/storage/callbacks/activate_multipath.rb b/service/lib/dinstaller/storage/callbacks/activate_multipath.rb index 982dfcf704..aa82ada694 100644 --- a/service/lib/dinstaller/storage/callbacks/activate_multipath.rb +++ b/service/lib/dinstaller/storage/callbacks/activate_multipath.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2022] SUSE LLC +# Copyright (c) [2022-2023] SUSE LLC # # All Rights Reserved. # @@ -20,21 +20,18 @@ # find current contact information at www.suse.com. require "dinstaller/question" -require "dinstaller/can_ask_question" module DInstaller module Storage module Callbacks # Callbacks for multipath activation class ActivateMultipath - include CanAskQuestion - # Constructor # - # @param questions_manager [QuestionsManager] + # @param questions_client [DInstaller::DBus::Clients::Questions] # @param logger [Logger] - def initialize(questions_manager, logger) - @questions_manager = questions_manager + def initialize(questions_client, logger) + @questions_client = questions_client @logger = logger end @@ -47,17 +44,15 @@ def initialize(questions_manager, logger) def call(looks_like_real_multipath) return false unless looks_like_real_multipath - ask(question) do |q| - logger.info("#{q.text} #{q.answer}") - - q.answer == :yes + questions_client.ask(question) do |question_client| + question_client.answer == :yes end end private - # @return [QuestionsManager] - attr_reader :questions_manager + # @return [DInstaller::DBus::Clients::Questions] + attr_reader :questions_client # @return [Logger] attr_reader :logger diff --git a/service/lib/dinstaller/storage/manager.rb b/service/lib/dinstaller/storage/manager.rb index 8d06759f80..c9d6fc4e26 100644 --- a/service/lib/dinstaller/storage/manager.rb +++ b/service/lib/dinstaller/storage/manager.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2022] SUSE LLC +# Copyright (c) [2022-2023] SUSE LLC # # All Rights Reserved. # @@ -29,9 +29,8 @@ require "dinstaller/storage/proposal_settings" require "dinstaller/storage/callbacks" require "dinstaller/with_progress" -require "dinstaller/can_ask_question" require "dinstaller/security" -require "dinstaller/dbus/clients/questions_manager" +require "dinstaller/dbus/clients/questions" require "dinstaller/dbus/clients/software" require "dinstaller/helpers" @@ -42,7 +41,6 @@ module Storage # Manager to handle storage configuration class Manager include WithProgress - include CanAskQuestion include Helpers def initialize(config, logger) @@ -133,7 +131,7 @@ def validate # Activates the devices, calling activation callbacks if needed def activate_devices - callbacks = Callbacks::Activate.new(questions_manager, logger) + callbacks = Callbacks::Activate.new(questions_client, logger) Y2Storage::StorageManager.instance.activate(callbacks) end @@ -173,9 +171,9 @@ def security # Returns the client to ask questions # - # @return [DInstaller::DBus::Clients::QuestionsManager] - def questions_manager - @questions_manager ||= DInstaller::DBus::Clients::QuestionsManager.new + # @return [DInstaller::DBus::Clients::Questions] + def questions_client + @questions_client ||= DInstaller::DBus::Clients::Questions.new(logger: logger) end # Returns the client to ask the software service diff --git a/service/test/dinstaller/cockpit_manager_test.rb b/service/test/dinstaller/cockpit_manager_test.rb index 6dd78b2a7f..f16c1d95f3 100644 --- a/service/test/dinstaller/cockpit_manager_test.rb +++ b/service/test/dinstaller/cockpit_manager_test.rb @@ -28,7 +28,7 @@ describe DInstaller::CockpitManager do subject(:cockpit) { described_class.new(logger) } - let(:logger) { Logger.new($stdout) } + let(:logger) { Logger.new($stdout, level: :warn) } before do # Avoid problems with FileFromUrl side effects. diff --git a/service/test/dinstaller/config_reader_test.rb b/service/test/dinstaller/config_reader_test.rb index 981f4a6b54..c59fbfad58 100644 --- a/service/test/dinstaller/config_reader_test.rb +++ b/service/test/dinstaller/config_reader_test.rb @@ -24,7 +24,8 @@ describe DInstaller::ConfigReader do let(:workdir) { File.join(FIXTURES_PATH, "root_dir") } - subject { described_class.new(workdir: workdir) } + let(:logger) { Logger.new($stdout, level: :warn) } + subject { described_class.new(logger: logger, workdir: workdir) } before do allow(Yast::Directory).to receive(:tmpdir).and_return(File.join(workdir, "tmpdir")) allow(subject).to receive(:copy_file) diff --git a/service/test/dinstaller/dbus/clients/questions_manager_test.rb b/service/test/dinstaller/dbus/clients/questions_test.rb similarity index 93% rename from service/test/dinstaller/dbus/clients/questions_manager_test.rb rename to service/test/dinstaller/dbus/clients/questions_test.rb index ec3e4417b0..680969912f 100644 --- a/service/test/dinstaller/dbus/clients/questions_manager_test.rb +++ b/service/test/dinstaller/dbus/clients/questions_test.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2022] SUSE LLC +# Copyright (c) [2022-2023] SUSE LLC # # All Rights Reserved. # @@ -20,11 +20,11 @@ # find current contact information at www.suse.com. require_relative "../../../test_helper" -require "dinstaller/dbus/clients/questions_manager" +require "dinstaller/dbus/clients/questions" require "dinstaller/question" require "dbus" -describe DInstaller::DBus::Clients::QuestionsManager do +describe DInstaller::DBus::Clients::Questions do before do allow(DInstaller::DBus::Bus).to receive(:current).and_return(bus) allow(bus).to receive(:service).with("org.opensuse.DInstaller.Questions").and_return(service) @@ -33,6 +33,10 @@ allow(dbus_object).to receive(:default_iface=) end + subject { described_class.new(logger: logger) } + + let(:logger) { Logger.new($stdout, level: :warn) } + let(:bus) { instance_double(DInstaller::DBus::Bus) } let(:service) { instance_double(::DBus::Service) } let(:dbus_object) { instance_double(::DBus::ProxyObject) } diff --git a/service/test/dinstaller/dbus/clients/storage_test.rb b/service/test/dinstaller/dbus/clients/storage_test.rb index 8527c62f02..a7257963cb 100644 --- a/service/test/dinstaller/dbus/clients/storage_test.rb +++ b/service/test/dinstaller/dbus/clients/storage_test.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2022] SUSE LLC +# Copyright (c) [2022-2023] SUSE LLC # # All Rights Reserved. # @@ -34,12 +34,16 @@ allow(dbus_object).to receive(:introspect) allow(dbus_object).to receive(:[]).with("org.opensuse.DInstaller.Storage1") .and_return(storage_iface) + allow(dbus_object).to receive(:[]).with("org.opensuse.DInstaller.Storage1.Proposal.Calculator") + .and_return(proposal_calculator_iface) - allow(service).to receive(:object).with("/org/opensuse/DInstaller/Storage/Proposal1") + allow(service).to receive(:object).with("/org/opensuse/DInstaller/Storage1/Proposal") .and_return(dbus_proposal) allow(dbus_proposal).to receive(:introspect) - allow(dbus_proposal).to receive(:[]).with("org.opensuse.DInstaller.Storage.Proposal1") + allow(dbus_proposal).to receive(:[]).with("org.opensuse.DInstaller.Storage1.Proposal") .and_return(proposal_iface) + + allow(proposal_calculator_iface).to receive(:[]).with("Result").and_return(proposal_path) end let(:bus) { instance_double(DInstaller::DBus::Bus) } @@ -47,10 +51,13 @@ let(:dbus_object) { instance_double(::DBus::ProxyObject) } let(:storage_iface) { instance_double(::DBus::ProxyObjectInterface) } + let(:proposal_calculator_iface) { instance_double(::DBus::ProxyObjectInterface) } let(:dbus_proposal) { instance_double(::DBus::ProxyObject) } let(:proposal_iface) { instance_double(::DBus::ProxyObjectInterface) } + let(:proposal_path) { "/" } + subject { described_class.new } describe "#probe" do @@ -96,7 +103,7 @@ describe "#available_devices" do before do - allow(proposal_iface).to receive(:[]).with("AvailableDevices").and_return( + allow(proposal_calculator_iface).to receive(:[]).with("AvailableDevices").and_return( [ ["/dev/sda", "/dev/sda (50 GiB)"], ["/dev/sdb", "/dev/sda (20 GiB)"] @@ -110,57 +117,82 @@ end describe "#candidate_devices" do - before do - allow(proposal_iface).to receive(:[]).with("CandidateDevices").and_return(["/dev/sda"]) + context "if a proposal object is not exported yet" do + let(:proposal_path) { "/" } + + it "returns an empty list" do + expect(subject.candidate_devices).to eq([]) + end end - it "returns the name of the candidate devices for the installation" do - expect(subject.candidate_devices).to contain_exactly("/dev/sda") + context "if a proposal object is already exported" do + let(:proposal_path) { "/org/opensuse/DInstaller/Storage1/Proposal" } + + before do + allow(proposal_iface).to receive(:[]).with("CandidateDevices").and_return(["/dev/sda"]) + end + + it "returns the name of the candidate devices for the installation" do + expect(subject.candidate_devices).to contain_exactly("/dev/sda") + end end end describe "#calculate" do # Using partial double because methods are dynamically added to the proxy object - let(:dbus_proposal) { double(::DBus::ProxyObject) } + let(:proposal_calculator_iface) { double(::DBus::ProxyObjectInterface) } it "calculates the proposal with the given devices" do - expect(dbus_proposal).to receive(:Calculate).with({ "CandidateDevices" => ["/dev/sdb"] }) + expect(proposal_calculator_iface) + .to receive(:Calculate).with({ "CandidateDevices" => ["/dev/sdb"] }) subject.calculate(["/dev/sdb"]) end end describe "#actions" do - before do - allow(proposal_iface).to receive(:[]).with("Actions").and_return( - [ - { - "Text" => "Create GPT on /dev/vdc", - "Subvolume" => false - }, - { - "Text" => "Create partition /dev/vdc1 (8.00 MiB) as BIOS Boot Partition", - "Subvolume" => false - }, - { - "Text" => "Create partition /dev/vdc2 (27.99 GiB) for / with btrfs", - "Subvolume" => false - }, - { - "Text" => "Create partition /dev/vdc3 (2.00 GiB) for swap", - "Subvolume" => false - } - ] - ) - end + context "if a proposal object is not exported yet" do + let(:proposal_path) { "/" } - it "returns the actions to perform" do - expect(subject.actions).to include(/Create GPT/) - expect(subject.actions).to include(/Create partition \/dev\/vdc1/) - expect(subject.actions).to include(/Create partition \/dev\/vdc2/) - expect(subject.actions).to include(/Create partition \/dev\/vdc3/) + it "returns an empty list" do + expect(subject.actions).to eq([]) + end end - include_examples "validation" + context "if a proposal object is already exported" do + let(:proposal_path) { "/org/opensuse/DInstaller/Storage1/Proposal" } + + before do + allow(proposal_iface).to receive(:[]).with("Actions").and_return( + [ + { + "Text" => "Create GPT on /dev/vdc", + "Subvolume" => false + }, + { + "Text" => "Create partition /dev/vdc1 (8.00 MiB) as BIOS Boot Partition", + "Subvolume" => false + }, + { + "Text" => "Create partition /dev/vdc2 (27.99 GiB) for / with btrfs", + "Subvolume" => false + }, + { + "Text" => "Create partition /dev/vdc3 (2.00 GiB) for swap", + "Subvolume" => false + } + ] + ) + end + + it "returns the actions to perform" do + expect(subject.actions).to include(/Create GPT/) + expect(subject.actions).to include(/Create partition \/dev\/vdc1/) + expect(subject.actions).to include(/Create partition \/dev\/vdc2/) + expect(subject.actions).to include(/Create partition \/dev\/vdc3/) + end + end end + + include_examples "validation" end diff --git a/service/test/dinstaller/dbus/questions_test.rb b/service/test/dinstaller/dbus/questions_test.rb index 196723b801..065b94d6e4 100644 --- a/service/test/dinstaller/dbus/questions_test.rb +++ b/service/test/dinstaller/dbus/questions_test.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2022] SUSE LLC +# Copyright (c) [2022-2023] SUSE LLC # # All Rights Reserved. # @@ -21,19 +21,16 @@ require_relative "../../test_helper" require "dinstaller/dbus/questions" -require "dinstaller/questions_manager" require "dinstaller/question" require "dinstaller/luks_activation_question" require "dbus" describe DInstaller::DBus::Questions do - subject { described_class.new(backend, logger) } - before do subject.instance_variable_set(:@service, service) end - let(:backend) { DInstaller::QuestionsManager.new(logger) } + subject { described_class.new(logger: logger) } let(:logger) { Logger.new($stdout, level: :warn) } @@ -41,59 +38,6 @@ let(:system_bus) { instance_double(DBus::SystemBus) } - it "configures callbacks for exporting a D-Bus question when a new question is added" do - question1 = DInstaller::Question.new("test1") - question2 = DInstaller::Question.new("test2") - - expect(service).to receive(:export) do |dbus_object| - id = dbus_object.path.split("/").last.to_i - expect(id).to eq(question1.id) - end - - expect(service).to receive(:export) do |dbus_object| - id = dbus_object.path.split("/").last.to_i - expect(id).to eq(question2.id) - end - - backend.add(question1) - backend.add(question2) - end - - it "configures callbacks for unexporting a D-Bus question when a question is deleted" do - dbus_objects = [] - question1 = DInstaller::Question.new("test1") - question2 = DInstaller::Question.new("test2") - - expect(service).to receive(:export).twice do |dbus_object| - dbus_objects << dbus_object - end - - backend.add(question1) - backend.add(question2) - - expect(service) - .to receive(:descendants_for) - .with("/org/opensuse/DInstaller/Questions1") - .and_return(dbus_objects) - - expect(service).to receive(:unexport) do |dbus_object| - id = dbus_object.path.split("/").last.to_i - expect(id).to eq(question1.id) - end - - backend.delete(question1) - end - - it "configures callbacks to dispatch D-Bus messages while waiting for answers" do - allow(backend).to receive(:loop).and_yield - allow(backend).to receive(:sleep) - - expect(system_bus).to receive(:dispatch_message_queue) - - question1 = DInstaller::Question.new("test1") - backend.wait([question1]) - end - describe "Questions interface" do let(:interface) { "org.opensuse.DInstaller.Questions1" } let(:full_method_name) { described_class.make_method_name(interface, method_name) } @@ -101,35 +45,71 @@ describe "#New" do let(:method_name) { "New" } - it "adds a question and returns its path" do - expect(backend).to receive(:add) - expect(subject.public_send(full_method_name, "How you doin?", ["fine", "great"], [])) - .to start_with "/org/opensuse/DInstaller/Questions1/" + it "exports a question and returns its path" do + expect(service).to receive(:export) do |question| + expect(question).is_a?(DInstaller::DBus::Question) + expect(question.backend).is_a?(DInstaller::Question) + expect(question.text).to match(/How you doin/) + end + + result = subject.public_send(full_method_name, "How you doin?", ["fine", "great"], []) + + expect(result).to start_with("/org/opensuse/DInstaller/Questions1/") end end describe "#NewLuksActivation" do let(:method_name) { "NewLuksActivation" } - it "adds a question and returns its path" do - expect(backend).to receive(:add) - expect(subject.public_send(full_method_name, "/dev/tape1", "New games", "90 minutes", 1)) - .to start_with "/org/opensuse/DInstaller/Questions1/" + it "exports a question and returns its path" do + expect(service).to receive(:export) do |question| + expect(question).is_a?(DInstaller::DBus::Question) + expect(question).is_a?(DInstaller::LuksActivationQuestion) + end + + result = subject.public_send(full_method_name, "/dev/tape1", "New games", "90 minutes", 1) + + expect(result).to start_with("/org/opensuse/DInstaller/Questions1/") end end describe "#Delete" do let(:method_name) { "Delete" } - it "deletes the question" do - q = DInstaller::Question.new("Huh?", options: []) - path = "/org/opensuse/DInstaller/Questions1/666" - dbus_q = DInstaller::DBus::Question.new(path, q, logger) - node = instance_double(DBus::Node, object: dbus_q) + before do + allow(service).to receive(:get_node).with(path).and_return(node) + end + + context "when the given object path does not exist" do + let(:path) { "/org/opensuse/DInstaller/Questions1/666" } + let(:node) { nil } + + it "raises an error" do + expect { subject.public_send(full_method_name, path) }.to raise_error(/not found/) + end + end + + context "when the given object path is not a question" do + let(:path) { "/org/opensuse/DInstaller/Foo/1" } + let(:node) { instance_double(DBus::Node, object: dbus_object) } + let(:dbus_object) { "test" } + + it "raises an error" do + expect { subject.public_send(full_method_name, path) }.to raise_error(/not a Question/) + end + end + + context "when the given object path is a question" do + let(:path) { "/org/opensuse/DInstaller/Questions1/1" } + let(:node) { instance_double(DBus::Node, object: dbus_object) } + let(:dbus_object) { DInstaller::DBus::Question.new(path, question, logger) } + let(:question) { DInstaller::Question.new("Do you want to test?", options: [:yes, :no]) } + + it "unexports the question" do + expect(service).to receive(:unexport).with(dbus_object) - expect(service).to receive(:get_node).with(path).and_return(node) - expect(backend).to receive(:delete).with(q) - expect { subject.public_send(full_method_name, path) }.to_not raise_error + subject.public_send(full_method_name, path) + end end end end diff --git a/service/test/dinstaller/dbus/storage/manager_test.rb b/service/test/dinstaller/dbus/storage/manager_test.rb new file mode 100644 index 0000000000..a5bca34603 --- /dev/null +++ b/service/test/dinstaller/dbus/storage/manager_test.rb @@ -0,0 +1,280 @@ +# frozen_string_literal: true + +# Copyright (c) [2023] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +require_relative "../../../test_helper" +require "dinstaller/dbus/storage/manager" +require "dinstaller/dbus/storage/proposal" +require "dinstaller/storage/manager" +require "dinstaller/storage/proposal" +require "dinstaller/storage/proposal_settings" +require "dinstaller/storage/volume" +require "y2storage" +require "dbus" + +describe DInstaller::DBus::Storage::Manager do + subject { described_class.new(backend, logger) } + + let(:logger) { Logger.new($stdout, level: :warn) } + + let(:backend) do + instance_double(DInstaller::Storage::Manager, + proposal: proposal, + on_progress_change: nil, + on_progress_finish: nil) + end + + let(:proposal) do + instance_double(DInstaller::Storage::Proposal, on_calculate: nil, calculated_settings: settings) + end + + let(:settings) { nil } + + describe "#available_devices" do + before do + allow(proposal).to receive(:available_devices).and_return(devices) + end + + context "if there is no available devices" do + let(:devices) { [] } + + it "returns an empty list" do + expect(subject.available_devices).to eq([]) + end + end + + context "if there are available devices" do + before do + allow(proposal).to receive(:device_label).with(device1).and_return("Device 1") + allow(proposal).to receive(:device_label).with(device2).and_return("Device 2") + end + + let(:devices) { [device1, device2] } + + let(:device1) { instance_double(Y2Storage::Disk, name: "/dev/vda") } + let(:device2) { instance_double(Y2Storage::Disk, name: "/dev/vdb") } + + it "retuns the device name and label for each device" do + result = subject.available_devices + + expect(result).to contain_exactly( + ["/dev/vda", "Device 1", {}], + ["/dev/vdb", "Device 2", {}] + ) + end + end + end + + describe "#volume_templates" do + before do + allow(proposal).to receive(:volume_templates).and_return(templates) + end + + context "if there are no volume templates" do + let(:templates) { [] } + + it "returns an empty list" do + expect(subject.volume_templates).to eq([]) + end + end + + context "if there are volume templates" do + let(:templates) { [volume1_template, volume2_template] } + + let(:volume1_template) do + DInstaller::Storage::Volume.new(Y2Storage::VolumeSpecification.new({})).tap do |volume| + volume.mount_point = "/test" + volume.device_type = :partition + volume.encrypted = true + volume.fs_type = Y2Storage::Filesystems::Type::EXT3 + volume.min_size = Y2Storage::DiskSize.new(1024) + volume.max_size = Y2Storage::DiskSize.new(2048) + volume.fixed_size_limits = true + volume.snapshots = true + end + end + + let(:volume2_template) { DInstaller::Storage::Volume.new } + + before do + allow(volume1_template).to receive(:size_relevant_volumes).and_return(["/home"]) + allow(volume1_template).to receive(:fs_types) + .and_return([Y2Storage::Filesystems::Type::EXT3]) + end + + it "returns a list with a hash for each volume template" do + expect(subject.volume_templates.size).to eq(2) + expect(subject.volume_templates).to all(be_a(Hash)) + + template1, template2 = subject.volume_templates + + expect(template1).to eq({ + "MountPoint" => "/test", + "Optional" => false, + "DeviceType" => "partition", + "Encrypted" => true, + "FsTypes" => ["Ext3"], + "FsType" => "Ext3", + "MinSize" => 1024, + "MaxSize" => 2048, + "FixedSizeLimits" => true, + "AdaptiveSizes" => true, + "Snapshots" => true, + "SnapshotsConfigurable" => false, + "SnapshotsAffectSizes" => false, + "SizeRelevantVolumes" => ["/home"] + }) + + expect(template2).to eq({ + "Optional" => true, + "AdaptiveSizes" => false, + "SnapshotsConfigurable" => false, + "SnapshotsAffectSizes" => false + }) + end + end + end + + describe "#result" do + before do + allow(subject).to receive(:dbus_proposal).and_return(dbus_proposal) + end + + context "when there is no exported proposal object yet" do + let(:dbus_proposal) { nil } + + it "returns root path" do + expect(subject.result.to_s).to eq("/") + end + end + + context "when there is an exported proposal object" do + let(:dbus_proposal) do + instance_double(DInstaller::DBus::Storage::Proposal, path: ::DBus::ObjectPath.new("/test")) + end + + it "returns the proposal object path" do + expect(subject.result.to_s).to eq("/test") + end + end + end + + describe "#calculate_proposal" do + let(:dbus_settings) do + { + "CandidateDevices" => ["/dev/vda"], + "LVM" => true, + "EncryptionPassword" => "n0ts3cr3t", + "Volumes" => dbus_volumes + } + end + + let(:dbus_volumes) do + [ + { "MountPoint" => "/" }, + { "MountPoint" => "swap" } + ] + end + + it "calculates a proposal with settings having values from D-Bus" do + expect(proposal).to receive(:calculate) do |settings| + expect(settings).to be_a(DInstaller::Storage::ProposalSettings) + expect(settings.candidate_devices).to contain_exactly("/dev/vda") + expect(settings.lvm).to eq(true) + expect(settings.encryption_password).to eq("n0ts3cr3t") + expect(settings.volumes).to contain_exactly( + an_object_having_attributes(mount_point: "/"), + an_object_having_attributes(mount_point: "swap") + ) + end + + subject.calculate_proposal(dbus_settings) + end + + context "when the D-Bus settings does not include some values" do + let(:dbus_settings) { {} } + + it "calculates a proposal with settings having default values for the missing settings" do + expect(proposal).to receive(:calculate) do |settings| + expect(settings).to be_a(DInstaller::Storage::ProposalSettings) + expect(settings.candidate_devices).to eq([]) + expect(settings.lvm).to be_nil + expect(settings.encryption_password).to be_nil + expect(settings.volumes).to eq([]) + end + + subject.calculate_proposal(dbus_settings) + end + end + + context "when the D-Bus settings includes a volume" do + let(:dbus_volumes) { [dbus_volume1] } + + let(:dbus_volume1) do + { + "DeviceType" => "lvm_lv", + "Encrypted" => true, + "MountPoint" => "/", + "FixedSizeLimits" => true, + "MinSize" => 1024, + "MaxSize" => 2048, + "FsType" => "Ext3", + "Snapshots" => true + } + end + + it "calculates a proposal with settings having a volume with values from D-Bus" do + expect(proposal).to receive(:calculate) do |settings| + volume = settings.volumes.first + + expect(volume.device_type).to eq(:lvm_lv) + expect(volume.encrypted).to eq(true) + expect(volume.mount_point).to eq("/") + expect(volume.fixed_size_limits).to eq(true) + expect(volume.min_size.to_i).to eq(1024) + expect(volume.max_size.to_i).to eq(2048) + expect(volume.snapshots).to eq(true) + end + + subject.calculate_proposal(dbus_settings) + end + + context "and the D-Bus volume does not include some values" do + let(:dbus_volume1) { { "MountPoint" => "/" } } + + it "calculates a proposal with settings having a volume with missing values" do + expect(proposal).to receive(:calculate) do |settings| + volume = settings.volumes.first + + expect(volume.device_type).to be_nil + expect(volume.encrypted).to be_nil + expect(volume.mount_point).to eq("/") + expect(volume.fixed_size_limits).to be_nil + expect(volume.min_size).to be_nil + expect(volume.max_size).to be_nil + expect(volume.snapshots).to be_nil + end + + subject.calculate_proposal(dbus_settings) + end + end + end + end +end diff --git a/service/test/dinstaller/dbus/storage/proposal_test.rb b/service/test/dinstaller/dbus/storage/proposal_test.rb index 290df86cb2..8d4a449139 100644 --- a/service/test/dinstaller/dbus/storage/proposal_test.rb +++ b/service/test/dinstaller/dbus/storage/proposal_test.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2022] SUSE LLC +# Copyright (c) [2022-2023] SUSE LLC # # All Rights Reserved. # @@ -21,7 +21,6 @@ require_relative "../../../test_helper" require "dinstaller/dbus/storage/proposal" -require "dinstaller/dbus/interfaces/service_status" require "dinstaller/storage/proposal" require "dinstaller/storage/proposal_settings" require "dinstaller/storage/volume" @@ -33,65 +32,11 @@ let(:logger) { Logger.new($stdout, level: :warn) } let(:backend) do - instance_double(DInstaller::Storage::Proposal, on_calculate: nil, calculated_settings: settings) + instance_double(DInstaller::Storage::Proposal, calculated_settings: settings) end let(:settings) { nil } - let(:service_status_interface) do - DInstaller::DBus::Interfaces::ServiceStatus::SERVICE_STATUS_INTERFACE - end - - before do - allow_any_instance_of(described_class).to receive(:register_service_status_callbacks) - end - - it "defines ServiceStatus D-Bus interface" do - expect(subject.intfs.keys).to include(service_status_interface) - end - - describe ".new" do - it "configures callbacks from ServiceStatus interface" do - expect_any_instance_of(described_class).to receive(:register_service_status_callbacks) - subject - end - end - - describe "#available_devices" do - before do - allow(backend).to receive(:available_devices).and_return(devices) - end - - context "if there is no available devices" do - let(:devices) { [] } - - it "returns an empty list" do - expect(subject.available_devices).to eq([]) - end - end - - context "if there are available devices" do - before do - allow(backend).to receive(:device_label).with(device1).and_return("Device 1") - allow(backend).to receive(:device_label).with(device2).and_return("Device 2") - end - - let(:devices) { [device1, device2] } - - let(:device1) { instance_double(Y2Storage::Disk, name: "/dev/vda") } - let(:device2) { instance_double(Y2Storage::Disk, name: "/dev/vdb") } - - it "retuns the device name and label for each device" do - result = subject.available_devices - - expect(result).to contain_exactly( - ["/dev/vda", "Device 1", {}], - ["/dev/vdb", "Device 2", {}] - ) - end - end - end - describe "#candidate_devices" do context "if a proposal has not been calculated yet" do let(:settings) { nil } @@ -153,76 +98,6 @@ end end - describe "#volume_templates" do - before do - allow(backend).to receive(:volume_templates).and_return(templates) - end - - context "if there are no volume templates" do - let(:templates) { [] } - - it "returns an empty list" do - expect(subject.volume_templates).to eq([]) - end - end - - context "if there are volume templates" do - let(:templates) { [volume1_template, volume2_template] } - - let(:volume1_template) do - DInstaller::Storage::Volume.new(Y2Storage::VolumeSpecification.new({})).tap do |volume| - volume.mount_point = "/test" - volume.device_type = :partition - volume.encrypted = true - volume.fs_type = Y2Storage::Filesystems::Type::EXT3 - volume.min_size = Y2Storage::DiskSize.new(1024) - volume.max_size = Y2Storage::DiskSize.new(2048) - volume.fixed_size_limits = true - volume.snapshots = true - end - end - - let(:volume2_template) { DInstaller::Storage::Volume.new } - - before do - allow(volume1_template).to receive(:size_relevant_volumes).and_return(["/home"]) - allow(volume1_template).to receive(:fs_types) - .and_return([Y2Storage::Filesystems::Type::EXT3]) - end - - it "returns a list with a hash for each volume template" do - expect(subject.volume_templates.size).to eq(2) - expect(subject.volume_templates).to all(be_a(Hash)) - - template1, template2 = subject.volume_templates - - expect(template1).to eq({ - "MountPoint" => "/test", - "Optional" => false, - "DeviceType" => "partition", - "Encrypted" => true, - "FsTypes" => ["Ext3"], - "FsType" => "Ext3", - "MinSize" => 1024, - "MaxSize" => 2048, - "FixedSizeLimits" => true, - "AdaptiveSizes" => true, - "Snapshots" => true, - "SnapshotsConfigurable" => false, - "SnapshotsAffectSizes" => false, - "SizeRelevantVolumes" => ["/home"] - }) - - expect(template2).to eq({ - "Optional" => true, - "AdaptiveSizes" => false, - "SnapshotsConfigurable" => false, - "SnapshotsAffectSizes" => false - }) - end - end - end - describe "#volumes" do let(:settings) do DInstaller::Storage::ProposalSettings.new.tap { |s| s.volumes = calculated_volumes } @@ -309,106 +184,4 @@ end end end - - describe "#calculate" do - let(:dbus_settings) do - { - "CandidateDevices" => ["/dev/vda"], - "LVM" => true, - "EncryptionPassword" => "n0ts3cr3t", - "Volumes" => dbus_volumes - } - end - - let(:dbus_volumes) do - [ - { "MountPoint" => "/" }, - { "MountPoint" => "swap" } - ] - end - - it "calculates a proposal with settings having values from D-Bus" do - expect(backend).to receive(:calculate) do |settings| - expect(settings).to be_a(DInstaller::Storage::ProposalSettings) - expect(settings.candidate_devices).to contain_exactly("/dev/vda") - expect(settings.lvm).to eq(true) - expect(settings.encryption_password).to eq("n0ts3cr3t") - expect(settings.volumes).to contain_exactly( - an_object_having_attributes(mount_point: "/"), - an_object_having_attributes(mount_point: "swap") - ) - end - - subject.calculate(dbus_settings) - end - - context "when the D-Bus settings does not include some values" do - let(:dbus_settings) { {} } - - it "calculates a proposal with settings having default values for the missing settings" do - expect(backend).to receive(:calculate) do |settings| - expect(settings).to be_a(DInstaller::Storage::ProposalSettings) - expect(settings.candidate_devices).to eq([]) - expect(settings.lvm).to be_nil - expect(settings.encryption_password).to be_nil - expect(settings.volumes).to eq([]) - end - - subject.calculate(dbus_settings) - end - end - - context "when the D-Bus settings includes a volume" do - let(:dbus_volumes) { [dbus_volume1] } - - let(:dbus_volume1) do - { - "DeviceType" => "lvm_lv", - "Encrypted" => true, - "MountPoint" => "/", - "FixedSizeLimits" => true, - "MinSize" => 1024, - "MaxSize" => 2048, - "FsType" => "Ext3", - "Snapshots" => true - } - end - - it "calculates a proposal with settings having a volume with values from D-Bus" do - expect(backend).to receive(:calculate) do |settings| - volume = settings.volumes.first - - expect(volume.device_type).to eq(:lvm_lv) - expect(volume.encrypted).to eq(true) - expect(volume.mount_point).to eq("/") - expect(volume.fixed_size_limits).to eq(true) - expect(volume.min_size.to_i).to eq(1024) - expect(volume.max_size.to_i).to eq(2048) - expect(volume.snapshots).to eq(true) - end - - subject.calculate(dbus_settings) - end - - context "and the D-Bus volume does not include some values" do - let(:dbus_volume1) { { "MountPoint" => "/" } } - - it "calculates a proposal with settings having a volume with missing values" do - expect(backend).to receive(:calculate) do |settings| - volume = settings.volumes.first - - expect(volume.device_type).to be_nil - expect(volume.encrypted).to be_nil - expect(volume.mount_point).to eq("/") - expect(volume.fixed_size_limits).to be_nil - expect(volume.min_size).to be_nil - expect(volume.max_size).to be_nil - expect(volume.snapshots).to be_nil - end - - subject.calculate(dbus_settings) - end - end - end - end end diff --git a/service/test/dinstaller/manager_test.rb b/service/test/dinstaller/manager_test.rb index cc7a683b86..d4fe9bee1b 100644 --- a/service/test/dinstaller/manager_test.rb +++ b/service/test/dinstaller/manager_test.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2022] SUSE LLC +# Copyright (c) [2022-2023] SUSE LLC # # All Rights Reserved. # @@ -24,7 +24,6 @@ require "dinstaller/config" require "dinstaller/question" require "dinstaller/dbus/service_status" -require "dinstaller/questions_manager" describe DInstaller::Manager do subject { described_class.new(config, logger) } @@ -56,7 +55,6 @@ on_service_status_change: nil, valid?: true ) end - let(:questions_manager) { instance_double(DInstaller::QuestionsManager) } let(:product) { nil } @@ -66,7 +64,6 @@ allow(DInstaller::DBus::Clients::Software).to receive(:new).and_return(software) allow(DInstaller::DBus::Clients::Storage).to receive(:new).and_return(storage) allow(DInstaller::DBus::Clients::Users).to receive(:new).and_return(users) - allow(DInstaller::QuestionsManager).to receive(:new).and_return(questions_manager) end describe "#startup_phase" do diff --git a/service/test/dinstaller/questions_manager_test.rb b/service/test/dinstaller/questions_manager_test.rb deleted file mode 100644 index d29e6791e5..0000000000 --- a/service/test/dinstaller/questions_manager_test.rb +++ /dev/null @@ -1,165 +0,0 @@ -# frozen_string_literal: true - -# Copyright (c) [2022] SUSE LLC -# -# All Rights Reserved. -# -# This program is free software; you can redistribute it and/or modify it -# under the terms of version 2 of the GNU General Public License as published -# by the Free Software Foundation. -# -# This program is distributed in the hope that it will be useful, but WITHOUT -# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or -# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for -# more details. -# -# You should have received a copy of the GNU General Public License along -# with this program; if not, contact SUSE LLC. -# -# To contact SUSE LLC about this file by physical or electronic mail, you may -# find current contact information at www.suse.com. - -require_relative "../test_helper" -require "dinstaller/questions_manager" -require "dinstaller/question" - -describe DInstaller::QuestionsManager do - subject { described_class.new(logger) } - - let(:logger) { Logger.new($stdout, level: :warn) } - - let(:callback) { proc {} } - - let(:question1) { DInstaller::Question.new("test1", options: [:yes, :no]) } - let(:question2) { DInstaller::LuksActivationQuestion.new("sda1") } - - describe "#add" do - before do - subject.on_add(&callback) - end - - context "if it does not contain the given question yet" do - it "adds the question" do - subject.add(question1) - - expect(subject.questions).to include(question1) - end - - it "calls the #on_add callbacks" do - expect(callback).to receive(:call) - - subject.add(question1) - end - - it "returns truthy value" do - expect(subject.add(question1)).to be_truthy - end - end - - context "if it already contains the given question" do - before do - subject.add(question1) - end - - it "does not add the question" do - subject.add(question1) - - expect(subject.questions.size).to eq(1) - end - - it "does not call the #on_add callbacks" do - expect(callback).to_not receive(:call) - - subject.add(question1) - end - - it "returns falsy value" do - expect(subject.add(question1)).to be_falsy - end - end - end - - describe "#delete" do - before do - subject.on_delete(&callback) - end - - context "if it contains the given question" do - before do - subject.add(question1) - end - - it "deletes the question" do - subject.delete(question1) - - expect(subject.questions).to_not include(question1) - end - - it "calls the #on_delete callbacks" do - expect(callback).to receive(:call) - - subject.delete(question1) - end - - it "returns truthy value" do - expect(subject.delete(question1)).to be_truthy - end - end - - context "if it does not contain the given question" do - before do - subject.add(question2) - end - - it "does not delete any question" do - subject.delete(question1) - - expect(subject.questions).to contain_exactly(question2) - end - - it "does not call the #on_delete callbacks" do - expect(callback).to_not receive(:call) - - subject.delete(question1) - end - - it "returns falsy value" do - expect(subject.delete(question1)).to be_falsy - end - end - end - - describe "#wait" do - # This callback ensures that both questions are answered after calling it for third time - let(:callback) do - times = 0 - - proc do - times += 1 - question1.answer = :yes if times == 2 - question2.answer = :skip if times == 3 - end - end - - before do - subject.on_wait(&callback) - - subject.add(question1) - subject.add(question2) - - allow(subject).to receive(:sleep) - end - - it "waits until all questions are answered" do - expect(subject).to receive(:sleep).exactly(2).times - - subject.wait([question1, question2]) - end - - it "calls the #on_wait callbacks while waiting" do - expect(callback).to receive(:call).and_call_original.exactly(3).times - - subject.wait([question1, question2]) - end - end -end diff --git a/service/test/dinstaller/software/callbacks/media_test.rb b/service/test/dinstaller/software/callbacks/media_test.rb index ff491a54be..8967799e13 100644 --- a/service/test/dinstaller/software/callbacks/media_test.rb +++ b/service/test/dinstaller/software/callbacks/media_test.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2022] SUSE LLC +# Copyright (c) [2022-2023] SUSE LLC # # All Rights Reserved. # @@ -21,29 +21,27 @@ require_relative "../../../test_helper" require "dinstaller/software/callbacks/media" -require "dinstaller/dbus/clients/questions_manager" -require "dinstaller/question" +require "dinstaller/dbus/clients/questions" +require "dinstaller/dbus/clients/question" describe DInstaller::Software::Callbacks::Media do - subject { described_class.new(questions_manager, logger) } + subject { described_class.new(questions_client, logger) } - let(:questions_manager) do - instance_double(DInstaller::DBus::Clients::QuestionsManager) - end + let(:questions_client) { instance_double(DInstaller::DBus::Clients::Questions) } - let(:logger) { Logger.new($stdout) } + let(:logger) { Logger.new($stdout, level: :warn) } describe "#media_changed" do - let(:asked_question) do - instance_double(DInstaller::Question, text: "Better safe than sorry", answer: answer) - end - let(:answer) { :Retry } - before do - allow(subject).to receive(:ask).and_yield(asked_question) + allow(questions_client).to receive(:ask).and_yield(question_client) + allow(question_client).to receive(:answer).and_return(answer) end + let(:question_client) { instance_double(DInstaller::DBus::Clients::Question) } + context "when the user answers :Retry" do + let(:answer) { :Retry } + it "returns ''" do ret = subject.media_change( "NOT_FOUND", "Package not found", "", "", 0, "", 0, "", true, [], 0 diff --git a/service/test/dinstaller/software/callbacks/signature_test.rb b/service/test/dinstaller/software/callbacks/signature_test.rb index 45010f0e7b..468f3292b4 100644 --- a/service/test/dinstaller/software/callbacks/signature_test.rb +++ b/service/test/dinstaller/software/callbacks/signature_test.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2022] SUSE LLC +# Copyright (c) [2022-2023] SUSE LLC # # All Rights Reserved. # @@ -21,29 +21,29 @@ require_relative "../../../test_helper" require "dinstaller/software/callbacks/signature" -require "dinstaller/dbus/clients/questions_manager" -require "dinstaller/question" +require "dinstaller/dbus/clients/questions" +require "dinstaller/dbus/clients/question" describe DInstaller::Software::Callbacks::Signature do - subject { described_class.new(questions_manager, logger) } - - let(:questions_manager) do - instance_double(DInstaller::DBus::Clients::QuestionsManager) + before do + allow(questions_client).to receive(:ask).and_yield(question_client) + allow(question_client).to receive(:answer).and_return(answer) end - let(:logger) { Logger.new($stdout) } + let(:questions_client) { instance_double(DInstaller::DBus::Clients::Questions) } - describe "#accept_unsigned_file" do - let(:asked_question) do - instance_double(DInstaller::Question, text: "Better safe than sorry", answer: answer) - end - let(:answer) { :Yes } + let(:question_client) { instance_double(DInstaller::DBus::Clients::Question) } - before do - allow(subject).to receive(:ask).and_yield(asked_question) - end + let(:answer) { nil } + let(:logger) { Logger.new($stdout, level: :warn) } + + subject { described_class.new(questions_client, logger) } + + describe "#accept_unsigned_file" do context "when the user answers :Yes" do + let(:answer) { :Yes } + it "returns true" do expect(subject.accept_unsigned_file("repomd.xml", -1)).to eq(true) end @@ -64,33 +64,30 @@ end it "includes the name and the URL in the question" do - expect(subject).to receive(:ask) do |question| + expect(questions_client).to receive(:ask) do |question| expect(question.text).to include("OSS (http://localhost/repo)") end + expect(subject.accept_unsigned_file("repomd.xml", 1)) end end context "when the repo information is not available" do before do - allow(Yast::Pkg).to receive(:SourceGeneralData).with(1) - .and_return(nil) + allow(Yast::Pkg).to receive(:SourceGeneralData).with(1).and_return(nil) end it "includes a generic message containing the filename" do - expect(subject).to receive(:ask) do |question| + expect(questions_client).to receive(:ask) do |question| expect(question.text).to include("repomd.xml") end + expect(subject.accept_unsigned_file("repomd.xml", 1)) end end end describe "import_gpg_key" do - let(:asked_question) do - instance_double(DInstaller::Question, text: "Better safe than sorry", answer: answer) - end - let(:answer) { :Trust } let(:key) do @@ -101,10 +98,6 @@ } end - before do - allow(subject).to receive(:ask).and_yield(asked_question) - end - context "when the user answers :Trust" do let(:answer) { :Trust } @@ -122,7 +115,7 @@ end it "includes a message" do - expect(subject).to receive(:ask) do |question| + expect(questions_client).to receive(:ask) do |question| expect(question.text).to include(key["id"]) expect(question.text).to include(key["name"]) expect(question.text).to include("2E2E A448 C9DD") diff --git a/service/test/dinstaller/software/manager_test.rb b/service/test/dinstaller/software/manager_test.rb index 59bac4b4de..c709525f60 100644 --- a/service/test/dinstaller/software/manager_test.rb +++ b/service/test/dinstaller/software/manager_test.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2022] SUSE LLC +# Copyright (c) [2022-2023] SUSE LLC # # All Rights Reserved. # @@ -25,6 +25,7 @@ ) require "dinstaller/config" require "dinstaller/software/manager" +require "dinstaller/dbus/clients/questions" describe DInstaller::Software::Manager do subject { described_class.new(config, logger) } @@ -42,8 +43,8 @@ DInstaller::Config.new(YAML.safe_load(File.read(config_path))) end - let(:questions_manager) do - instance_double(DInstaller::DBus::Clients::QuestionsManager) + let(:questions_client) do + instance_double(DInstaller::DBus::Clients::Questions) end before do @@ -55,8 +56,7 @@ .and_return(base_url) allow(Yast::Pkg).to receive(:SourceCreate) allow(Yast::Installation).to receive(:destdir).and_return(destdir) - allow(DInstaller::DBus::Clients::QuestionsManager).to receive(:new) - .and_return(questions_manager) + allow(DInstaller::DBus::Clients::Questions).to receive(:new).and_return(questions_client) end describe "#probe" do diff --git a/service/test/dinstaller/storage/callbacks/activate_luks_test.rb b/service/test/dinstaller/storage/callbacks/activate_luks_test.rb index 535cde931a..9c32825bc7 100644 --- a/service/test/dinstaller/storage/callbacks/activate_luks_test.rb +++ b/service/test/dinstaller/storage/callbacks/activate_luks_test.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2022] SUSE LLC +# Copyright (c) [2022-2023] SUSE LLC # # All Rights Reserved. # @@ -21,17 +21,25 @@ require_relative "../../../test_helper" require "dinstaller/storage/callbacks/activate_luks" -require "dinstaller/questions_manager" +require "dinstaller/luks_activation_question" +require "dinstaller/dbus/clients/questions" +require "dinstaller/dbus/clients/question" require "storage" describe DInstaller::Storage::Callbacks::ActivateLuks do - subject { described_class.new(questions_manager, logger) } + subject { described_class.new(questions_client, logger) } - let(:questions_manager) { DInstaller::QuestionsManager.new(logger) } + let(:questions_client) { instance_double(DInstaller::DBus::Clients::Questions) } let(:logger) { Logger.new($stdout, level: :warn) } describe "#call" do + before do + allow(questions_client).to receive(:ask).and_yield(question_client) + end + + let(:question_client) { instance_double(DInstaller::DBus::Clients::Question) } + let(:luks_info) do instance_double(Storage::LuksInfo, device_name: "/dev/sda1", @@ -42,7 +50,7 @@ let(:attempt) { 1 } it "asks a question to activate a LUKS device" do - expect(subject).to receive(:ask) do |question| + expect(questions_client).to receive(:ask) do |question| expect(question).to be_a(DInstaller::LuksActivationQuestion) end @@ -51,14 +59,8 @@ context "when the question is answered as :skip" do before do - allow(subject).to receive(:ask).and_yield(question) - end - - let(:question) do - DInstaller::LuksActivationQuestion.new("/dev/sda1").tap do |q| - q.answer = :skip - q.password = "notsecret" - end + allow(question_client).to receive(:answer).and_return(:skip) + allow(question_client).to receive(:password).and_return("notsecret") end it "returns a tuple containing false and the password" do @@ -68,14 +70,8 @@ context "when the question is answered as :decrypt" do before do - allow(subject).to receive(:ask).and_yield(question) - end - - let(:question) do - DInstaller::LuksActivationQuestion.new("/dev/sda1").tap do |q| - q.answer = :decrypt - q.password = "notsecret" - end + allow(question_client).to receive(:answer).and_return(:decrypt) + allow(question_client).to receive(:password).and_return("notsecret") end it "returns a tuple containing true and the password" do diff --git a/service/test/dinstaller/storage/callbacks/activate_multipath_test.rb b/service/test/dinstaller/storage/callbacks/activate_multipath_test.rb index fc8b9c8d44..4f0d5327e8 100644 --- a/service/test/dinstaller/storage/callbacks/activate_multipath_test.rb +++ b/service/test/dinstaller/storage/callbacks/activate_multipath_test.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2022] SUSE LLC +# Copyright (c) [2022-2023] SUSE LLC # # All Rights Reserved. # @@ -21,21 +21,28 @@ require_relative "../../../test_helper" require "dinstaller/storage/callbacks/activate_multipath" -require "dinstaller/questions_manager" +require "dinstaller/dbus/clients/questions" +require "dinstaller/dbus/clients/question" describe DInstaller::Storage::Callbacks::ActivateMultipath do - subject { described_class.new(questions_manager, logger) } + subject { described_class.new(questions_client, logger) } - let(:questions_manager) { DInstaller::QuestionsManager.new(logger) } + let(:questions_client) { instance_double(DInstaller::DBus::Clients::Questions) } let(:logger) { Logger.new($stdout, level: :warn) } describe "#call" do + before do + allow(questions_client).to receive(:ask).and_yield(question_client) + end + + let(:question_client) { instance_double(DInstaller::DBus::Clients::Question) } + context "if the devices do not look like real multipath" do let(:real_multipath) { false } it "does not ask a question" do - expect(subject).to_not receive(:ask) + expect(questions_client).to_not receive(:ask) subject.call(real_multipath) end @@ -49,7 +56,7 @@ let(:real_multipath) { true } it "asks a question to activate multipath" do - expect(subject).to receive(:ask) do |question| + expect(questions_client).to receive(:ask) do |question| expect(question.text).to match(/activate multipath\?/) end @@ -58,11 +65,7 @@ context "and the question is answered as :yes" do before do - allow(subject).to receive(:ask).and_yield(question) - end - - let(:question) do - DInstaller::Question.new("test", options: [:yes, :no]).tap { |q| q.answer = :yes } + allow(question_client).to receive(:answer).and_return(:yes) end it "returns true" do @@ -72,11 +75,7 @@ context "and the question is answered as :no" do before do - allow(subject).to receive(:ask).and_yield(question) - end - - let(:question) do - DInstaller::Question.new("test", options: [:yes, :no]).tap { |q| q.answer = :no } + allow(question_client).to receive(:answer).and_return(:no) end it "returns false" do diff --git a/service/test/dinstaller/storage/callbacks/activate_test.rb b/service/test/dinstaller/storage/callbacks/activate_test.rb index d2de539a74..696702b4b4 100644 --- a/service/test/dinstaller/storage/callbacks/activate_test.rb +++ b/service/test/dinstaller/storage/callbacks/activate_test.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2022] SUSE LLC +# Copyright (c) [2022-2023] SUSE LLC # # All Rights Reserved. # @@ -21,12 +21,12 @@ require_relative "../../../test_helper" require "dinstaller/storage/callbacks/activate" -require "dinstaller/questions_manager" +require "dinstaller/dbus/clients/questions" describe DInstaller::Storage::Callbacks::Activate do - subject { described_class.new(questions_manager, logger) } + subject { described_class.new(questions_client, logger) } - let(:questions_manager) { DInstaller::QuestionsManager.new(logger) } + let(:questions_client) { instance_double(DInstaller::DBus::Clients::Questions) } let(:logger) { Logger.new($stdout, level: :warn) } diff --git a/service/test/dinstaller/storage/manager_test.rb b/service/test/dinstaller/storage/manager_test.rb index 83f89203ae..d98bc2315c 100644 --- a/service/test/dinstaller/storage/manager_test.rb +++ b/service/test/dinstaller/storage/manager_test.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2022] SUSE LLC +# Copyright (c) [2022-2023] SUSE LLC # # All Rights Reserved. # @@ -21,8 +21,8 @@ require_relative "../../test_helper" require "dinstaller/storage/manager" -require "dinstaller/questions_manager" require "dinstaller/config" +require "dinstaller/dbus/clients/questions" describe DInstaller::Storage::Manager do subject(:storage) { described_class.new(config, logger) } @@ -35,8 +35,7 @@ before do allow(Y2Storage::StorageManager).to receive(:instance).and_return(y2storage_manager) - allow(DInstaller::DBus::Clients::QuestionsManager).to receive(:new) - .and_return(questions_manager) + allow(DInstaller::DBus::Clients::Questions).to receive(:new).and_return(questions_client) allow(DInstaller::DBus::Clients::Software).to receive(:new) .and_return(software) allow(Bootloader::FinishClient).to receive(:new) @@ -45,7 +44,7 @@ end let(:y2storage_manager) { instance_double(Y2Storage::StorageManager, probe: nil) } - let(:questions_manager) { instance_double(DInstaller::DBus::Clients::QuestionsManager) } + let(:questions_client) { instance_double(DInstaller::DBus::Clients::Questions) } let(:software) do instance_double(DInstaller::DBus::Clients::Software, selected_product: "ALP") end diff --git a/web/src/client/storage.js b/web/src/client/storage.js index 4ad89d2be5..e24ce6ae60 100644 --- a/web/src/client/storage.js +++ b/web/src/client/storage.js @@ -1,5 +1,5 @@ /* - * Copyright (c) [2022] SUSE LLC + * Copyright (c) [2022-2023] SUSE LLC * * All Rights Reserved. * @@ -27,8 +27,8 @@ import cockpit from "../lib/cockpit"; const STORAGE_SERVICE = "org.opensuse.DInstaller.Storage"; const STORAGE_PATH = "/org/opensuse/DInstaller/Storage1"; -const STORAGE_PROPOSAL_IFACE = "org.opensuse.DInstaller.Storage.Proposal1"; -const STORAGE_PROPOSAL_PATH = "/org/opensuse/DInstaller/Storage/Proposal1"; +const PROPOSAL_CALCULATOR_IFACE = "org.opensuse.DInstaller.Storage1.Proposal.Calculator"; +const PROPOSAL_IFACE = "org.opensuse.DInstaller.Storage1.Proposal"; /** * Storage base client @@ -49,7 +49,11 @@ class StorageBaseClient { * @return {Promise} */ async getProposal() { - const proxy = await this.client.proxy(STORAGE_PROPOSAL_IFACE); + const storageProxy = await this.client.proxy(PROPOSAL_CALCULATOR_IFACE, STORAGE_PATH); + const proposalProxy = await this.client.proxy(PROPOSAL_IFACE); + + // Check whether proposal object is already exported + if (!proposalProxy.valid) return {}; const volume = dbusVolume => { const valueFrom = dbusValue => dbusValue?.v; @@ -83,12 +87,12 @@ class StorageBaseClient { }; return { - availableDevices: proxy.AvailableDevices.map(([id, label]) => ({ id, label })), - candidateDevices: proxy.CandidateDevices, - lvm: proxy.LVM, - encryptionPassword: proxy.EncryptionPassword, - volumes: proxy.Volumes.map(volume), - actions: proxy.Actions.map(action) + availableDevices: storageProxy.AvailableDevices.map(([id, label]) => ({ id, label })), + candidateDevices: proposalProxy.CandidateDevices, + lvm: proposalProxy.LVM, + encryptionPassword: proposalProxy.EncryptionPassword, + volumes: proposalProxy.Volumes.map(volume), + actions: proposalProxy.Actions.map(action) }; } @@ -103,7 +107,7 @@ class StorageBaseClient { * @return {Promise} - 0 success, other for failure */ async calculateProposal({ candidateDevices, encryptionPassword, lvm, volumes }) { - const proxy = await this.client.proxy(STORAGE_PROPOSAL_IFACE); + const proxy = await this.client.proxy(PROPOSAL_CALCULATOR_IFACE, STORAGE_PATH); // Builds a new object without undefined attributes const cleanObject = (object) => { @@ -147,38 +151,6 @@ class StorageBaseClient { return proxy.Calculate(settings); } - - /** - * Registers a callback to run when properties in the Storage Proposal object change - * - * @param {function} handler - callback function - */ - onProposalChange(handler) { - return this.client.onObjectChanged(STORAGE_PROPOSAL_PATH, STORAGE_PROPOSAL_IFACE, changes => { - if (Array.isArray(changes.CandidateDevices.v)) { - // FIXME return the proposal object (see getProposal) - handler({ candidateDevices: changes.CandidateDevices.v }); - } - }); - } - - /** - * Registers a callback to run when properties in the Actions object change - * - * @param {function} handler - callback function - */ - onActionsChange(handler) { - return this.client.onObjectChanged(STORAGE_PROPOSAL_PATH, STORAGE_PROPOSAL_IFACE, changes => { - const { Actions: actions } = changes; - if (actions !== undefined && Array.isArray(actions.v)) { - const newActions = actions.v.map(action => { - const { Text: textVar, Subvol: subvolVar, Delete: deleteVar } = action; - return { text: textVar.v, subvol: subvolVar.v, delete: deleteVar.v }; - }); - handler(newActions); - } - }); - } } /** diff --git a/web/src/client/storage.test.js b/web/src/client/storage.test.js index e538fb4ec0..6bd7816d42 100644 --- a/web/src/client/storage.test.js +++ b/web/src/client/storage.test.js @@ -1,5 +1,5 @@ /* - * Copyright (c) [2022] SUSE LLC + * Copyright (c) [2022-2023] SUSE LLC * * All Rights Reserved. * @@ -27,16 +27,23 @@ import { StorageClient } from "./storage"; jest.mock("./dbus"); // NOTE: should we export them? -const STORAGE_PROPOSAL_IFACE = "org.opensuse.DInstaller.Storage.Proposal1"; +const PROPOSAL_CALCULATOR_IFACE = "org.opensuse.DInstaller.Storage1.Proposal.Calculator"; +const PROPOSAL_IFACE = "org.opensuse.DInstaller.Storage1.Proposal"; const calculateFn = jest.fn(); -const storageProposalProxy = { +const storageProxy = { wait: jest.fn(), AvailableDevices: [ ["/dev/sda", "/dev/sda, 950 GiB, Windows"], ["/dev/sdb", "/dev/sdb, 500 GiB"] ], + Calculate: calculateFn +}; + +const proposalProxy = { + valid: true, + wait: jest.fn(), CandidateDevices: ["/dev/sda"], LVM: true, Volumes: [ @@ -66,8 +73,7 @@ const storageProposalProxy = { Subvol: { t: "b", v: false }, Delete: { t: "b", v: false } } - ], - Calculate: calculateFn + ] }; beforeEach(() => { @@ -75,7 +81,8 @@ beforeEach(() => { DBusClient.mockImplementation(() => { return { proxy: (iface) => { - if (iface === STORAGE_PROPOSAL_IFACE) return storageProposalProxy; + if (iface === PROPOSAL_CALCULATOR_IFACE) return storageProxy; + if (iface === PROPOSAL_IFACE) return proposalProxy; } }; });