diff --git a/service/lib/agama/dbus/storage/volume_conversion/to_dbus.rb b/service/lib/agama/dbus/storage/volume_conversion/to_dbus.rb index 5cd7c26369..bfa409aba7 100644 --- a/service/lib/agama/dbus/storage/volume_conversion/to_dbus.rb +++ b/service/lib/agama/dbus/storage/volume_conversion/to_dbus.rb @@ -34,7 +34,21 @@ def initialize(volume) # # @return [Hash] def convert - hash = { + hash = base_conversion + return hash if volume.max_size.nil? || volume.max_size.unlimited? + + hash["MaxSize"] = volume.max_size.to_i + hash + end + + private + + # @return [Agama::Storage::Volume] + attr_reader :volume + + # @return [Hash] + def base_conversion + { "MountPath" => volume.mount_path.to_s, "MountOptions" => volume.mount_options, "TargetDevice" => volume.device.to_s, @@ -45,17 +59,8 @@ def convert "Snapshots" => volume.btrfs.snapshots?, "Outline" => outline_conversion } - return hash if volume.max_size.nil? || volume.max_size.unlimited? - - hash["MaxSize"] = volume.max_size.to_i - hash end - private - - # @return [Agama::Storage::Volume] - attr_reader :volume - # @return [Hash] def outline_conversion outline = volume.outline diff --git a/service/lib/agama/storage/volume_templates_builder.rb b/service/lib/agama/storage/volume_templates_builder.rb index c0f0bc3ea3..0a33d404fb 100644 --- a/service/lib/agama/storage/volume_templates_builder.rb +++ b/service/lib/agama/storage/volume_templates_builder.rb @@ -93,14 +93,14 @@ def path_key(path) # Temporary method to avoid crashes if there is no default template def empty_data { - btrfs: BtrfsSettings.new, - outline: VolumeOutline.new, + btrfs: BtrfsSettings.new, + outline: VolumeOutline.new, mount_options: [], - filesystem: Y2Storage::Filesystems::Type::EXT4 + filesystem: Y2Storage::Filesystems::Type::EXT4 } end - def values(data) # rubocop:disable Metrics/AbcSize + def values(data) {}.tap do |values| values[:btrfs] = btrfs(data) values[:outline] = outline(data) @@ -181,10 +181,10 @@ def cleanpath(path) Pathname.new(path).cleanpath.to_s end - def fs_type(fs) - return fs if fs.is_a?(Y2Storage::Filesystems::Type) + def fs_type(filesystem) + return filesystem if filesystem.is_a?(Y2Storage::Filesystems::Type) - Y2Storage::Filesystems::Type.find(fs.downcase.to_sym) + Y2Storage::Filesystems::Type.find(filesystem.downcase.to_sym) end end end diff --git a/service/test/agama/dbus/storage/manager_test.rb b/service/test/agama/dbus/storage/manager_test.rb index 7fb6653205..3439ec656d 100644 --- a/service/test/agama/dbus/storage/manager_test.rb +++ b/service/test/agama/dbus/storage/manager_test.rb @@ -138,7 +138,7 @@ "FsType" => "Ext4", "MountOptions" => [], "MinSize" => 0, "MaxSize" => 0, "AutoSize" => false } - generic_outline = { "Required" => false, "FsTypes" => [], "SupportAutoSize" => false} + generic_outline = { "Required" => false, "FsTypes" => [], "SupportAutoSize" => false } expect(subject.default_volume("/")).to include(generic) expect(subject.default_volume("/")["Outline"]).to include(generic_outline) @@ -157,11 +157,11 @@ { "mount_path" => "/", "filesystem" => "btrfs", "size" => { "auto" => true }, "outline" => { - "required" => true, - "auto_size" => { + "required" => true, + "filesystems" => ["btrfs"], + "auto_size" => { "base_min" => "5 GiB", "base_max" => "20 GiB", "min_fallback_for" => "/home" - }, - "filesystems" => ["btrfs"] + } } }, { @@ -189,7 +189,7 @@ ) expect(subject.default_volume("swap")).to include( - "FsType" => "Swap", "AutoSize" => false, "MinSize" => 1024**3, "MaxSize" => 2*(1024**3) + "FsType" => "Swap", "AutoSize" => false, "MinSize" => 1024**3, "MaxSize" => 2 * (1024**3) ) expect(subject.default_volume("swap")["Outline"]).to include( "Required" => false, "FsTypes" => ["Swap"], "SupportAutoSize" => false @@ -197,8 +197,8 @@ end it "returns the default volume for any path without a template" do - default = { "FsType" => "Ext4", "AutoSize" => false, "MinSize" => 10*(1024**3) } - default_outline = { "FsTypes" => ["Ext3", "Ext4", "XFS"], "SupportAutoSize" => false} + default = { "FsType" => "Ext4", "AutoSize" => false, "MinSize" => 10 * (1024**3) } + default_outline = { "FsTypes" => ["Ext3", "Ext4", "XFS"], "SupportAutoSize" => false } expect(subject.default_volume("/foo")).to include(default) expect(subject.default_volume("/foo")["Outline"]).to include(default_outline) @@ -300,12 +300,12 @@ let(:dbus_volume1) do { - "MountPath" => "/", - "AutoSize" => false, - "MinSize" => 1024, - "MaxSize" => 2048, - "FsType" => "Btrfs", - "Snapshots" => true + "MountPath" => "/", + "AutoSize" => false, + "MinSize" => 1024, + "MaxSize" => 2048, + "FsType" => "Btrfs", + "Snapshots" => true } end @@ -348,8 +348,8 @@ expect(volume.mount_path).to eq("/") expect(volume.auto_size).to eq(false) - expect(volume.min_size.to_i).to eq(5*(1024**3)) - expect(volume.max_size.to_i).to eq(20*(1024**3)) + expect(volume.min_size.to_i).to eq(5 * (1024**3)) + expect(volume.max_size.to_i).to eq(20 * (1024**3)) expect(volume.btrfs.snapshots).to eq(false) end diff --git a/service/test/agama/dbus/storage/manager_volumes_test.rb b/service/test/agama/dbus/storage/manager_volumes_test.rb index 7021fff02c..21e427e245 100644 --- a/service/test/agama/dbus/storage/manager_volumes_test.rb +++ b/service/test/agama/dbus/storage/manager_volumes_test.rb @@ -70,11 +70,11 @@ "mount_path" => "/", "filesystem" => "btrfs", "size" => { "auto" => true }, "mount_options" => ["whatever=foo"], "outline" => { - "required" => true, - "auto_size" => { + "required" => true, + "filesystems" => ["btrfs"], + "auto_size" => { "base_min" => "5 GiB", "base_max" => "20 GiB", "min_fallback_for" => ["/home"] - }, - "filesystems" => ["btrfs"] + } } }, { @@ -94,8 +94,8 @@ }, { "filesystem" => "ext4", - "size" => { "auto" => false, "min" => "10 GiB" }, - "outline" => { "filesystems" => ["ext3", "ext4", "xfs"] } + "size" => { "auto" => false, "min" => "10 GiB" }, + "outline" => { "filesystems" => ["ext3", "ext4", "xfs"] } } ] end @@ -133,14 +133,14 @@ context "when the D-Bus settings omit some mandatory volumes" do let(:dbus_settings) { { "Volumes" => dbus_volumes } } - let(:dbus_volumes) { [dbus_root_vol, dbus_foo_vol ] } + let(:dbus_volumes) { [dbus_root_vol, dbus_foo_vol] } let(:dbus_root_vol) do { - "MountPath" => "/", - "AutoSize" => false, - "MinSize" => 1024, - "MaxSize" => 2048, - "Snapshots" => true + "MountPath" => "/", + "AutoSize" => false, + "MinSize" => 1024, + "MaxSize" => 2048, + "Snapshots" => true } end let(:dbus_foo_vol) { { "MountPath" => "/foo" } } @@ -199,11 +199,11 @@ swap = settings.volumes.find { |v| v.mount_path == "swap" } expect(swap.auto_size).to eq(false) expect(swap.min_size.to_i).to eq(1024**3) - expect(swap.max_size.to_i).to eq(2*(1024**3)) + expect(swap.max_size.to_i).to eq(2 * (1024**3)) foo = settings.volumes.find { |v| v.mount_path == "/foo" } expect(foo.auto_size).to eq(false) - expect(foo.min_size.to_i).to eq(10*(1024**3)) + expect(foo.min_size.to_i).to eq(10 * (1024**3)) expect(foo.max_size).to eq Y2Storage::DiskSize.unlimited expect(foo.fs_type).to eq Y2Storage::Filesystems::Type::EXT4 end @@ -213,9 +213,11 @@ end xcontext "when the D-Bus settings include changes in the volume outline" do + # TODO end xcontext "when the D-Bus settings specify auto_size for an unsupported volume" do + # TODO end xcontext "when the D-Bus settings specify a filesystem type not listed in the outline" do @@ -224,6 +226,7 @@ end xcontext "when the D-Bus settings specify a forbidden configuration for snapshots" do + # TODO end end end diff --git a/service/test/agama/dbus/storage/proposal_settings_conversion/from_dbus_test.rb b/service/test/agama/dbus/storage/proposal_settings_conversion/from_dbus_test.rb index be9a5f765b..23aeb3b42b 100644 --- a/service/test/agama/dbus/storage/proposal_settings_conversion/from_dbus_test.rb +++ b/service/test/agama/dbus/storage/proposal_settings_conversion/from_dbus_test.rb @@ -34,27 +34,21 @@ let(:config_data) do { "storage" => { - "lvm" => true, - "encryption" => { - "method" => "luks2", + "lvm" => true, + "space_policy" => "delete", + "encryption" => { + "method" => "luks2", "pbkd_function" => "argon2id" }, - "space_policy" => "delete", - "volumes" => [ - { "mount_path" => "/" } - ], + "volumes" => [{ "mount_path" => "/" }], "volume_templates" => [ { "mount_path" => "/", - "outline" => { - "required" => true - } + "outline" => { "required" => true } }, { "mount_path" => "swap", - "outline" => { - "required" => false - } + "outline" => { "required" => false } } ] } diff --git a/service/test/agama/dbus/storage/proposal_settings_conversion/to_dbus_test.rb b/service/test/agama/dbus/storage/proposal_settings_conversion/to_dbus_test.rb index 108aaac9aa..d4f40f6d74 100644 --- a/service/test/agama/dbus/storage/proposal_settings_conversion/to_dbus_test.rb +++ b/service/test/agama/dbus/storage/proposal_settings_conversion/to_dbus_test.rb @@ -68,7 +68,7 @@ "SpaceActions" => { "/dev/sda" => :force_delete }, "Volumes" => [ { - "MountPath" => "/test", + "MountPath" => "/test", "MountOptions" => [], "TargetDevice" => "", "TargetVG" => "", diff --git a/service/test/agama/dbus/storage/volume_conversion/from_dbus_test.rb b/service/test/agama/dbus/storage/volume_conversion/from_dbus_test.rb index cd971e45b5..83a5ac34ec 100644 --- a/service/test/agama/dbus/storage/volume_conversion/from_dbus_test.rb +++ b/service/test/agama/dbus/storage/volume_conversion/from_dbus_test.rb @@ -25,7 +25,7 @@ require "agama/storage/volume_templates_builder" require "agama/dbus/storage/volume_conversion/from_dbus" -# TODO Move to a better place. It would be useful in other test files. +# TODO: Move to a better place. It would be useful in other test files. RSpec::Matchers.define(:eq_outline) do |expected| match do |received| methods = [ @@ -39,8 +39,8 @@ failure_message do |received| "Volume outline does not match.\n" \ - "Expected: #{expected.inspect}\n" \ - "Received: #{received.inspect}" + "Expected: #{expected.inspect}\n" \ + "Received: #{received.inspect}" end end @@ -55,9 +55,7 @@ "volume_templates" => [ { "mount_path" => "/test", - "outline" => { - "required" => true - } + "outline" => { "required" => true } } ] } @@ -67,7 +65,7 @@ describe "#convert" do let(:dbus_volume) do { - "MountPath" => "/test", + "MountPath" => "/test", "MountOptions" => ["rw", "default"], "TargetDevice" => "/dev/sda", "TargetVG" => "/dev/system", @@ -102,6 +100,7 @@ let(:dbus_settings) { {} } xit "generates a volume with default values from config" do + # TODO end end end diff --git a/service/test/agama/dbus/storage/volume_conversion/to_dbus_test.rb b/service/test/agama/dbus/storage/volume_conversion/to_dbus_test.rb index ca5951df8f..aa2618e13c 100644 --- a/service/test/agama/dbus/storage/volume_conversion/to_dbus_test.rb +++ b/service/test/agama/dbus/storage/volume_conversion/to_dbus_test.rb @@ -29,7 +29,7 @@ let(:default_volume) { Agama::Storage::Volume.new("/test") } let(:custom_volume) do - outline = Agama::Storage::VolumeOutline.new.tap do |outline| + volume_outline = Agama::Storage::VolumeOutline.new.tap do |outline| outline.required = true outline.filesystems = [Y2Storage::Filesystems::Type::EXT3, Y2Storage::Filesystems::Type::EXT4] outline.adjust_by_ram = true @@ -41,7 +41,7 @@ end Agama::Storage::Volume.new("/test").tap do |volume| - volume.outline = outline + volume.outline = volume_outline volume.fs_type = Y2Storage::Filesystems::Type::EXT4 volume.btrfs.snapshots = true volume.mount_options = ["rw", "default"] @@ -56,7 +56,7 @@ describe "#convert" do it "converts the volume to a D-Bus hash" do expect(described_class.new(default_volume).convert).to eq( - "MountPath" => "/test", + "MountPath" => "/test", "MountOptions" => [], "TargetDevice" => "", "TargetVG" => "", @@ -75,7 +75,7 @@ ) expect(described_class.new(custom_volume).convert).to eq( - "MountPath" => "/test", + "MountPath" => "/test", "MountOptions" => ["rw", "default"], "TargetDevice" => "/dev/sda", "TargetVG" => "/dev/system", diff --git a/service/test/agama/storage/proposal_volumes_test.rb b/service/test/agama/storage/proposal_volumes_test.rb index 8472e7b21f..26c6d9a884 100644 --- a/service/test/agama/storage/proposal_volumes_test.rb +++ b/service/test/agama/storage/proposal_volumes_test.rb @@ -45,7 +45,7 @@ "mount_path" => "/", "filesystem" => "btrfs", "size" => { "auto" => true }, "outline" => { "snapshots_configurable" => true, - "auto_size" => { + "auto_size" => { "base_min" => "10 GiB", "base_max" => "20 GiB", "min_fallback_for" => ["/home"], "snapshots_increment" => "300%" }