Add context information to AutoinstIssues classes

src/lib/y2storage/autoinst_issues/invalid_value.rb

@@ -26,28 +26,32 @@ module AutoinstIssues
     # Represents an AutoYaST situation where an invalid value was given.
     #
     # @example Invalid value 'auto' for attribute :size on /home partition
-    #   problem = InvalidValue.new("/home", :size, "auto")
+    #   section = AutoinstProfile::PartitioningSection.new_from_hash({"size" => "auto"})
+    #   problem = InvalidValue.new(section, :size)
+    #   problem.value #=> "auto"
+    #   problem.attr  #=> :size
     class InvalidValue < Issue
-      # @return [String] Device affected by this error
-      attr_reader :device
       # @return [Symbol] Name of the missing attribute
       attr_reader :attr
-      # @return [Object] Invalid value
-      attr_reader :value
       # @return [Object] New value or :skip to skip the section.
       attr_reader :new_value
 
-      # @param device    [String] Device (`/`, `/dev/sda`, etc.)
-      # @param attr      [Symbol] Name of the missing attribute
-      # @param value     [Integer,String,Symbol] Invalid value
+      # @param section   [AutoinstProfile::SectionWithAttributes] Section where it was detected
+      # @param attr      [Symbol] Name of the invalid attribute
       # @param new_value [Integer,String,Symbol] New value or :skip to skip the section
-      def initialize(device, attr, value, new_value = :skip)
-        @device = device
+      def initialize(section, attr, new_value = :skip)
+        @section = section
         @attr = attr
-        @value = value
         @new_value = new_value
       end
 
+      # Return the invalid value
+      #
+      # @return [Integer,String,Symbol] Invalid value
+      def value
+        section.send(attr)
+      end
+
       # Return problem severity
       #
       # @return [Symbol] :warn
@@ -63,8 +67,8 @@ def message
         # TRANSLATORS: 'value' is a generic value (number or string)
         # 'attr' is an AutoYaST element name; 'device' is a device name (eg. /dev/sda1);
         # 'new_value_message' is a short explanation about what should be done with the value.
-        _("Invalid value '%{value}' for attribute '%{attr}' on '%{device}' (%{new_value_message}).") %
-          { value: value, attr: attr, device: device, new_value_message: new_value_message }
+        _("Invalid value '%{value}' for attribute '%{attr}' (%{new_value_message}).") %
+          { value: value, attr: attr, new_value_message: new_value_message }
       end
 
     private

src/lib/y2storage/autoinst_issues/issue.rb

@@ -7,6 +7,9 @@ module AutoinstIssues
     class Issue
       include Yast::I18n
 
+      # @return [AutoinstProfile::SectionWithAttributes] section where the problem was detected
+      attr_reader :section
+
       # Return problem severity
       #
       # * :fatal: abort the installation.

src/lib/y2storage/autoinst_issues/missing_value.rb

@@ -26,7 +26,9 @@ module AutoinstIssues
     # Represents an AutoYaST situation where a mandatory value is missing.
     #
     # @example Missing value for attribute 'size' in '/dev/sda' device.
-    #   problem = MissingValue.new("/dev/sda", :size)
+    #   section = AutoinstProfile::PartitionSection.new_from_hashes({})
+    #   problem = MissingValue.new(section, :size)
+    #   problem.attr #=> :size
     class MissingValue < Issue
       # @return [String] Device affected by this error
       attr_reader :device
@@ -36,8 +38,8 @@ class MissingValue < Issue
 
       # @param device [String] Device (`/`, `/dev/sda`, etc.)
       # @param attr   [Symbol] Name of the missing attribute
-      def initialize(device, attr)
-        @device = device
+      def initialize(section, attr)
+        @section = section
         @attr = attr
       end
 
@@ -54,8 +56,8 @@ def severity
       # @return [String] Error message
       # @see Issue#message
       def message
-        # TRANSLATORS: AutoYaST element name and device name (ag. /dev/sda1)
-        _("Missing attribute '%{attr}' on '%{device}'") % { attr: attr, device: device }
+        # TRANSLATORS: AutoYaST element name and device name (eg. /dev/sda1)
+        _("Missing attribute '%{attr}'") % { attr: attr }
       end
     end
   end

src/lib/y2storage/autoinst_profile/drive_section.rb

@@ -115,7 +115,7 @@ def init_from_hashes(hash)
         @type ||= default_type_for(hash)
         @use = use_value_from_string(hash["use"]) if hash["use"]
         @partitions = partitions_from_hash(hash)
-        @skip_list = SkipListSection.new_from_hashes(hash.fetch("skip_list", []))
+        @skip_list = SkipListSection.new_from_hashes(hash.fetch("skip_list", []), self)
       end
 
       def default_type_for(hash)
@@ -191,6 +191,13 @@ def to_hashes
         hash
       end
 
+      # Return section name
+      #
+      # @return [String] "drives"
+      def section_name
+        "drives"
+      end
+
     protected
 
       def partitions_from_hash(hash)

src/lib/y2storage/autoinst_profile/partition_section.rb

@@ -146,10 +146,11 @@ def self.attributes
       # @!attribute subvolumes_prefix
       #   @return [String] Name of the default Btrfs subvolume
 
-
       def init_from_hashes(hash)
         super
-        @raid_options = RaidOptionsSection.new_from_hashes(hash["raid_options"]) if hash["raid_options"]
+        if hash["raid_options"]
+          @raid_options = RaidOptionsSection.new_from_hashes(hash["raid_options"], self)
+        end
         @subvolumes = SubvolSpecification.list_from_control_xml(hash["subvolumes"]) if hash["subvolumes"]
         @fstab_options = hash["fstopt"].split(",").map(&:strip) if hash["fstopt"]
       end
@@ -251,6 +252,13 @@ def to_hashes
         hash
       end
 
+      # Return section name
+      #
+      # @return [String] "partitions"
+      def section_name
+        "partitions"
+      end
+
     protected
 
       # Uses legacy ids for backwards compatibility. For example, BIOS Boot

src/lib/y2storage/autoinst_profile/skip_list_section.rb

@@ -56,23 +56,27 @@ class SkipListSection
 
       # @return [Array<SkipRule>] List of rules to apply
       attr_reader :rules
+      #
+      # @return [SectionWithAttributes] Parent section
+      attr_reader :parent
 
       class << self
         # Creates a skip list from an AutoYaST profile
         #
         # @param profile_rules [Array<Hash>] List of profile skip rules
         # @return [SkipList]
-        def new_from_hashes(profile_rules)
+        def new_from_hashes(profile_rules, parent = nil)
           rules = profile_rules.map { |h| SkipRule.from_profile_rule(h) }
-          new(rules)
+          new(rules, parent)
         end
       end
 
       # Constructor
       #
       # @param rules [Array<SkipRule>] List of rules to apply
-      def initialize(rules)
+      def initialize(rules, parent = nil)
         @rules = rules
+        @parent = parent
       end
 
       # Determines whether a disk matches any of the rules on the list

src/lib/y2storage/proposal/autoinst_devices_planner.rb

@@ -330,8 +330,7 @@ def assign_size_to_partition(disk, partition, part_section)
         size_info = parse_size(part_section, PARTITION_MIN_SIZE, disk.size)
 
         if size_info.nil?
-          section_id = part_section.mount || disk.name
-          issues_list.add(:invalid_value, section_id, :size, part_section.size)
+          issues_list.add(:invalid_value, part_section, :size)
           return false
         end
 
@@ -351,8 +350,7 @@ def assign_size_to_lv(vg, lv, lv_section)
         size_info = parse_size(lv_section, vg.extent_size, DiskSize.unlimited)
 
         if size_info.nil?
-          section_id = lv_section.mount || vg.name
-          issues_list.add(:invalid_value, section_id, :size, lv_section.size)
+          issues_list.add(:invalid_value, lv_section, :size)
           return false
         end
 

src/lib/y2storage/proposal/autoinst_space_maker.rb

@@ -76,26 +76,25 @@ def delete_stuff(devicegraph, disk, drive_spec)
 
         # TODO: resizing of partitions
 
-        delete_by_use(devicegraph, disk, drive_spec.use)
+        delete_by_use(devicegraph, disk, drive_spec)
       end
 
       # Deletes unwanted partition according to the "use" element
       #
       # @param devicegraph [Devicegraph]
       # @param disk        [Disk]
-      # @param use         [String,Array<Integer>] Use value ("all", "linux", "free"
-      #   or a list of partition numbers)
-      def delete_by_use(devicegraph, disk, use)
-        return if use == "free"
-        case use
+      # @param drive_spec  [AutoinstProfile::DriveSection]
+      def delete_by_use(devicegraph, disk, drive_spec)
+        return if drive_spec.use == "free"
+        case drive_spec.use
         when "all"
           disk.partition_table.remove_descendants if disk.partition_table
         when "linux"
           delete_linux_partitions(devicegraph, disk)
         when Array
-          delete_partitions_by_number(devicegraph, disk, use)
+          delete_partitions_by_number(devicegraph, disk, drive_spec.use)
         else
-          register_invalid_use_value(disk, use)
+          register_invalid_use_value(drive_spec)
         end
       end
 
@@ -124,13 +123,12 @@ def delete_partitions_by_number(devicegraph, disk, partition_nrs)
 
       # Register an invalid/missing value for 'use'
       #
-      # @param disk        [Disk]
-      # @param use         [String,Array<Integer>] Use value ("all", "linux", "free")
-      def register_invalid_use_value(disk, use)
-        if use.nil?
-          issues_list.add(:missing_value, disk.name, :use)
+      # @param drive_spec  [AutoinstProfile::DriveSection]
+      def register_invalid_use_value(drive_spec)
+        if drive_spec.use
+          issues_list.add(:invalid_value, drive_spec, :use)
         else
-          issues_list.add(:invalid_value, disk.name, :use, use)
+          issues_list.add(:missing_value, drive_spec, :use)
         end
       end
     end

test/support/autoinst_profile_examples.rb

@@ -0,0 +1,35 @@
+# encoding: utf-8
+
+# Copyright (c) [2016] 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 "y2storage/autoinst_profile/section_with_attributes"
+
+RSpec.shared_examples "autoinst section" do
+  let(:parent) do
+    Y2Storage::AutoinstProfile::SectionWithAttributes.new
+  end
+
+  describe ".new_from_hashes" do
+    it "sets the parent section" do
+      section = described_class.new_from_hashes({}, parent)
+      expect(section.parent).to eq(parent)
+    end
+  end
+end

test/y2storage/autoinst_issues/invalid_value_test.rb

@@ -22,14 +22,18 @@
 
 require_relative "../../spec_helper"
 require "y2storage/autoinst_issues/invalid_value"
+require "y2storage/autoinst_profile/partition_section"
 
 describe Y2Storage::AutoinstIssues::InvalidValue do
-  subject(:issue) { described_class.new("/", :size, "auto") }
+  subject(:issue) { described_class.new(section, :size) }
+
+  let(:section) do
+    instance_double(Y2Storage::AutoinstProfile::PartitionSection, size: "auto")
+  end
 
   describe "#message" do
     it "includes relevant information" do
       message = issue.message
-      expect(message).to include "/"
       expect(message).to include "size"
       expect(message).to include "auto"
     end
@@ -41,15 +45,15 @@
     end
 
     context "when a new value was given" do
-      subject(:issue) { described_class.new("/", :size, "auto", "some-value") }
+      subject(:issue) { described_class.new(section, :size, "some-value") }
 
       it "includes a warning about the section being skipped" do
         expect(issue.message).to include "replaced by 'some-value'"
       end
     end
 
     context "when :skip is given as new value" do
-      subject(:issue) { described_class.new("/", :size, "auto", :skip) }
+      subject(:issue) { described_class.new(section, :size, :skip) }
 
       it "includes a warning about the section being skipped" do
         expect(issue.message).to include "the section will be skipped"

test/y2storage/autoinst_issues/missing_value_test.rb

@@ -22,13 +22,18 @@
 
 require_relative "../../spec_helper"
 require "y2storage/autoinst_issues/missing_value"
+require "y2storage/autoinst_profile/partition_section"
 
 describe Y2Storage::AutoinstIssues::MissingValue do
-  subject(:issue) { described_class.new("/home", :size) }
+  subject(:issue) { described_class.new(section, :size) }
+
+  let(:section) do
+    instance_double(Y2Storage::AutoinstProfile::PartitionSection)
+  end
 
   describe "#message" do
     it "returns a description of the issue" do
-      expect(issue.message).to match(/Missing attribute.*size.*'\/home'/)
+      expect(issue.message).to match(/Missing attribute 'size'/)
     end
   end
 

test/y2storage/autoinst_profile/drive_section_test.rb

@@ -21,11 +21,14 @@
 # find current contact information at www.suse.com.
 
 require_relative "../spec_helper"
+require_relative "#{TEST_PATH}/support/autoinst_profile_examples"
 require "y2storage"
 
 describe Y2Storage::AutoinstProfile::DriveSection do
   subject(:section) { described_class.new }
 
+  include_examples "autoinst section"
+
   before { fake_scenario("autoyast_drive_examples") }
 
   def device(name)
@@ -463,8 +466,8 @@ def device(name)
   end
 
   describe "#section_name" do
-    it "returns 'drive'" do
-      expect(section.section_name).to eq("drive")
+    it "returns 'drives'" do
+      expect(section.section_name).to eq("drives")
     end
   end
 end