WIP: [Service] Improvements in the storage proposal

service/lib/agama/storage/proposal.rb

@@ -38,9 +38,15 @@ def initialize(config, logger: nil)
 
         @config = config
         @logger = logger || Logger.new($stdout)
+        @issues = []
         @on_calculate_callbacks = []
       end
 
+      # List of issues.
+      #
+      # @return [Array<Issue>]
+      attr_reader :issues
+
       # Whether the proposal was already calculated.
       #
       # @return [Boolean]
@@ -52,10 +58,10 @@ def calculated?
       #
       # @return [Boolean]
       def success?
-        calculated? && !proposal.failed?
+        calculated? && !proposal.failed? && issues.none?(&:error?)
       end
 
-      # Stores callbacks to be call after calculating a proposal.
+      # Stores callbacks to be called after calculating a proposal.
       def on_calculate(&block)
         @on_calculate_callbacks << block
       end
@@ -67,6 +73,11 @@ def available_devices
         disk_analyzer&.candidate_disks || []
       end
 
+      # Settings used to calculate the current proposal.
+      #
+      # The type depends on the kind of proposal, see {#calculate_guided} and {#calculate_autoyast}.
+      #
+      # @return [Agama::Storage::ProposalSettings, Array<Hash>]
       def settings
         return unless calculated?
 
@@ -101,16 +112,10 @@ def actions
         Actions.new(logger, proposal.devices.actiongraph).all
       end
 
-      # List of issues.
+      # Whether the current proposal was calculated the given strategy (:autoyast or :guided).
       #
-      # @return [Array<Issue>]
-      def issues
-        return [] if !calculated?
-
-        # This part depends on the strategy
-        strategy_object.issues + [proposal_issue].compact
-      end
-
+      # @param id [#downcase]
+      # @return [Boolean]
       def strategy?(id)
         return false unless calculated?
 
@@ -133,7 +138,16 @@ def strategy?(id)
       def calculate
         return false unless storage_manager.probed?
 
-        @strategy_object.calculate
+        @issues = []
+
+        begin
+          strategy_object.calculate
+          @issues << failed_issue if proposal.failed?
+        rescue Y2Storage::Error => e
+          handle_exception(e)
+        end
+
+        @issues.concat(strategy_object.issues)
         @on_calculate_callbacks.each(&:call)
         success?
       end
@@ -155,15 +169,39 @@ def storage_manager
         Y2Storage::StorageManager.instance
       end
 
-      # Returns an issue if the proposal is not valid.
+      # Handle Y2Storage exceptions
+      def handle_exception(error)
+        case error
+        when Y2Storage::NoDiskSpaceError
+          @issues << failed_issue
+        when Y2Storage::Error
+          @issues << exception_issue(error)
+        else
+          raise error
+        end
+      end
+
+      # Issue representing the proposal is not valid.
       #
-      # @return [Issue, nil]
-      def proposal_issue
-        return if success?
+      # @return [Issue]
+      def failed_issue
+        Issue.new(
+          _("Cannot accommodate the required file systems for installation"),
+          source:   Issue::Source::CONFIG,
+          severity: Issue::Severity::ERROR
+        )
+      end
 
-        Issue.new(_("Cannot accommodate the required file systems for installation"),
+      # Issue to communicate a generic Y2Storage error.
+      #
+      # @return [Issue]
+      def exception_issue(error)
+        Issue.new(
+          _("A problem ocurred while calculating the storage setup"),
+          details:  error.message,
           source:   Issue::Source::CONFIG,
-          severity: Issue::Severity::ERROR)
+          severity: Issue::Severity::ERROR
+        )
       end
     end
   end

service/lib/agama/storage/proposal_strategies.rb

@@ -21,7 +21,7 @@
 
 module Agama
   module Storage
-    # Bla
+    # Namespace for the different strategies potentially used by Storage::Proposal.
     module ProposalStrategies
     end
   end

service/lib/agama/storage/proposal_strategies/autoyast.rb

@@ -27,12 +27,13 @@
 module Agama
   module Storage
     module ProposalStrategies
-      # Backend class to calculate a storage proposal.
+      # Strategy used by the Agama proposal for backwards compatibility with AutoYaST.
       class Autoyast < Base
         include Yast::I18n
 
         # @param config [Config] Agama config
         # @param logger [Logger]
+        # @param partitioning [Array<Hash>]
         def initialize(config, logger, partitioning)
           textdomain "agama"
 
@@ -42,11 +43,11 @@ def initialize(config, logger, partitioning)
 
         # Settings used for calculating the proposal.
         #
-        # @param [Array<Hash>]
+        # @return [Array<Hash>]
         attr_reader :partitioning
         alias_method :settings, :partitioning
 
-        # Calculates a new proposal.
+        # @see Base#calculate
         def calculate
           @ay_issues = ::Installation::AutoinstIssues::List.new
           proposal = Y2Storage::AutoinstProposal.new(
@@ -57,21 +58,19 @@ def calculate
             issues_list:       ay_issues
           )
           proposal.propose
-        rescue Y2Storage::Error => e
-          handle_exception(e)
         ensure
           storage_manager.proposal = proposal
         end
 
-        # List of issues.
-        #
-        # @return [Array<Issue>]
+        # @see Base#issues
         def issues
           ay_issues.map { |i| agama_issue(i) }
         end
 
       private
 
+        # Issues generated by the AutoYaST proposal
+        # @return [::Installation::AutoinstIssues::List]
         attr_reader :ay_issues
 
         # Default proposal settings, potentially used to calculate omitted information
@@ -82,23 +81,6 @@ def proposal_settings
           ProposalSettingsConversion.to_y2storage(agama_default, config: config)
         end
 
-        # Handle Y2Storage exceptions
-        #
-        # Some of the exceptions can be handled as an AutoYaST problem in order to offer further
-        # information to the user. For the rest of cases, the exception is catched here and
-        # reported in a best-effort way (maybe without translation, for instance).
-        def handle_exception(error)
-          logger.warn "AutoYaST proposal failed: #{error.inspect}"
-          case error
-          when Y2Storage::NoDiskSpaceError
-            ay_issues.add(Y2Storage::AutoinstIssues::NoDiskSpace)
-          when Y2Storage::Error
-            ay_issues.add(Y2Storage::AutoinstIssues::Exception, error)
-          else
-            raise error
-          end
-        end
-
         # Agama issue equivalent to the given AutoYaST issue
         def agama_issue(ay_issue)
           Issue.new(

service/lib/agama/storage/proposal_strategies/base.rb

@@ -26,7 +26,7 @@
 module Agama
   module Storage
     module ProposalStrategies
-      # Backend class to calculate a storage proposal.
+      # Base class for the strategies used by the Agama proposal.
       class Base
         # @param config [Config] Agama config
         # @param logger [Logger]
@@ -40,11 +40,17 @@ def settings
           raise NotImplementedError
         end
 
-        # Calculates a new proposal.
+        # Calculates a new proposal, storing the result at the storage manager.
+        #
+        # @raise [Y2Storage::NoDiskSpaceError] if it was not possible to calculate the proposal
+        # @raise [Y2Storage::Error] if something went wrong while calculating the proposal
         def calculate
           raise NotImplementedError
         end
 
+        # Identifier for the strategy.
+        #
+        # @return [Symbol]
         def id
           self.class.name.split("::").last.downcase.to_sym
         end

service/lib/agama/storage/proposal_strategies/guided.rb

@@ -26,12 +26,13 @@
 module Agama
   module Storage
     module ProposalStrategies
-      # Backend class to calculate a storage proposal.
+      # Main strategy for the Agama proposal.
       class Guided < Base
         include Yast::I18n
 
         # @param config [Config] Agama config
         # @param logger [Logger]
+        # @param input_settings [ProposalSettings]
         def initialize(config, logger, input_settings)
           textdomain "agama"
 
@@ -47,17 +48,17 @@ def initialize(config, logger, input_settings)
         # @return [ProposalSettings]
         attr_reader :settings
 
-        # Calculates a new proposal.
+        # @see Base#calculate
         def calculate
           select_target_device(input_settings) if missing_target_device?(input_settings)
-          calculate_proposal(input_settings)
-          used_settings = storage_manager.proposal.settings
-          @settings = ProposalSettingsConversion.from_y2storage(used_settings, input_settings)
+          proposal = guided_proposal(input_settings)
+          proposal.propose
+        ensure
+          storage_manager.proposal = proposal
+          @settings = ProposalSettingsConversion.from_y2storage(proposal.settings, input_settings)
         end
 
-        # List of issues.
-        #
-        # @return [Array<Issue>]
+        # @see Base#issues
         def issues
           return [] unless storage_manager.proposal.failed?
 
@@ -66,6 +67,8 @@ def issues
 
       private
 
+        # Initial set of proposal settings
+        # @return [ProposalSettings]
         attr_reader :input_settings
 
         # Selects the first available device as target device for installation.
@@ -98,18 +101,16 @@ def missing_target_device?(settings)
           end
         end
 
-        # Instantiates and executes a Y2Storage proposal with the given settings
+        # Instance of the Y2Storage proposal to be used to run the calculation.
         #
         # @param settings [Y2Storage::ProposalSettings]
         # @return [Y2Storage::GuidedProposal]
-        def calculate_proposal(settings)
-          proposal = Y2Storage::MinGuidedProposal.new(
+        def guided_proposal(settings)
+          Y2Storage::MinGuidedProposal.new(
             settings:      ProposalSettingsConversion.to_y2storage(settings, config: config),
             devicegraph:   probed_devicegraph,
             disk_analyzer: disk_analyzer
           )
-          proposal.propose
-          storage_manager.proposal = proposal
         end
 
         # Returns an issue if there is no target device.