From 997897fa51f9db01f6316f093aee697bddf64db1 Mon Sep 17 00:00:00 2001 From: Michael Hudson-Doyle Date: Mon, 31 May 2021 13:03:03 +1200 Subject: [PATCH] convert supported_actions method into a functools.singledispatch --- po/POTFILES.in | 1 + subiquity/common/filesystem/actions.py | 103 ++++++++++++++++++ subiquity/common/filesystem/manipulator.py | 7 +- .../filesystem/tests/test_manipulator.py | 7 +- subiquity/models/filesystem.py | 69 +----------- subiquity/models/tests/test_filesystem.py | 11 +- subiquity/server/controllers/filesystem.py | 7 +- subiquity/ui/views/filesystem/filesystem.py | 7 +- 8 files changed, 135 insertions(+), 77 deletions(-) create mode 100644 subiquity/common/filesystem/actions.py diff --git a/po/POTFILES.in b/po/POTFILES.in index 82401627..3d5c7217 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -32,6 +32,7 @@ subiquity/common/api/tests/test_client.py subiquity/common/api/tests/test_endtoend.py subiquity/common/api/tests/test_server.py subiquity/common/errorreport.py +subiquity/common/filesystem/actions.py subiquity/common/filesystem/__init__.py subiquity/common/filesystem/manipulator.py subiquity/common/filesystem/tests/__init__.py diff --git a/subiquity/common/filesystem/actions.py b/subiquity/common/filesystem/actions.py new file mode 100644 index 00000000..45286bd5 --- /dev/null +++ b/subiquity/common/filesystem/actions.py @@ -0,0 +1,103 @@ +# Copyright 2021 Canonical, Ltd. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as +# published by the Free Software Foundation, either version 3 of the +# License, or (at your option) any later version. +# +# 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 Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + +import enum +import functools + +from subiquitycore.gettext38 import pgettext + +from subiquity.models.filesystem import ( + Bootloader, + Disk, + LVM_LogicalVolume, + LVM_VolGroup, + Partition, + Raid, + ) + + +class DeviceAction(enum.Enum): + # Information about a drive + INFO = pgettext("DeviceAction", "Info") + # Edit a device (partition, logical volume, RAID, etc) + EDIT = pgettext("DeviceAction", "Edit") + REFORMAT = pgettext("DeviceAction", "Reformat") + PARTITION = pgettext("DeviceAction", "Add Partition") + CREATE_LV = pgettext("DeviceAction", "Create Logical Volume") + FORMAT = pgettext("DeviceAction", "Format") + REMOVE = pgettext("DeviceAction", "Remove from RAID/LVM") + DELETE = pgettext("DeviceAction", "Delete") + TOGGLE_BOOT = pgettext("DeviceAction", "Make Boot Device") + + def str(self): + return pgettext(type(self).__name__, self.value) + + +@functools.singledispatch +def supported_actions(device): + raise NotImplementedError( + "supported_actions({}) not defined".format(device)) + + +@supported_actions.register(Disk) +def _disk_actions(disk): + actions = [ + DeviceAction.INFO, + DeviceAction.REFORMAT, + DeviceAction.PARTITION, + DeviceAction.FORMAT, + DeviceAction.REMOVE, + ] + if disk._m.bootloader != Bootloader.NONE: + actions.append(DeviceAction.TOGGLE_BOOT) + return actions + + +@supported_actions.register(Partition) +def _part_actions(part): + return [ + DeviceAction.EDIT, + DeviceAction.REMOVE, + DeviceAction.DELETE, + ] + + +@supported_actions.register(Raid) +def _raid_actions(raid): + return [ + DeviceAction.EDIT, + DeviceAction.PARTITION, + DeviceAction.FORMAT, + DeviceAction.REMOVE, + DeviceAction.DELETE, + DeviceAction.REFORMAT, + ] + + +@supported_actions.register(LVM_VolGroup) +def _vg_actions(vg): + return [ + DeviceAction.EDIT, + DeviceAction.CREATE_LV, + DeviceAction.DELETE, + ] + + +@supported_actions.register(LVM_LogicalVolume) +def _lv_actions(lv): + return [ + DeviceAction.EDIT, + DeviceAction.DELETE, + ] diff --git a/subiquity/common/filesystem/manipulator.py b/subiquity/common/filesystem/manipulator.py index ea0932b5..b753a0d6 100644 --- a/subiquity/common/filesystem/manipulator.py +++ b/subiquity/common/filesystem/manipulator.py @@ -15,10 +15,13 @@ import logging +from subiquity.common.filesystem.actions import ( + DeviceAction, + supported_actions, + ) from subiquity.common.types import Bootloader from subiquity.models.filesystem import ( align_up, - DeviceAction, Partition, ) @@ -214,7 +217,7 @@ class FilesystemManipulator: needs_boot = self.model.needs_bootloader_partition() log.debug('model needs a bootloader partition? {}'.format(needs_boot)) - can_be_boot = DeviceAction.TOGGLE_BOOT in disk.supported_actions + can_be_boot = DeviceAction.TOGGLE_BOOT in supported_actions(disk) if needs_boot and len(disk.partitions()) == 0 and can_be_boot: part = self._create_boot_partition(disk) diff --git a/subiquity/common/filesystem/tests/test_manipulator.py b/subiquity/common/filesystem/tests/test_manipulator.py index 3feb70f1..723e25c7 100644 --- a/subiquity/common/filesystem/tests/test_manipulator.py +++ b/subiquity/common/filesystem/tests/test_manipulator.py @@ -15,6 +15,10 @@ import unittest +from subiquity.common.filesystem.actions import ( + DeviceAction, + supported_actions, + ) from subiquity.common.filesystem.manipulator import ( FilesystemManipulator, ) @@ -24,7 +28,6 @@ from subiquity.models.tests.test_filesystem import ( ) from subiquity.models.filesystem import ( Bootloader, - DeviceAction, ) @@ -60,7 +63,7 @@ class TestFilesystemManipulator(unittest.TestCase): # manipulator around. for bl in Bootloader: manipulator, disk = make_manipulator_and_disk(bl) - if DeviceAction.TOGGLE_BOOT not in disk.supported_actions: + if DeviceAction.TOGGLE_BOOT not in supported_actions(disk): continue manipulator.add_boot_disk(disk) self.assertFalse( diff --git a/subiquity/models/filesystem.py b/subiquity/models/filesystem.py index b34c4667..1b8c7918 100644 --- a/subiquity/models/filesystem.py +++ b/subiquity/models/filesystem.py @@ -16,7 +16,6 @@ from abc import ABC, abstractmethod import attr import collections -import enum import fnmatch import itertools import logging @@ -32,8 +31,6 @@ from curtin.util import human2bytes from probert.storage import StorageInfo -from subiquitycore.gettext38 import pgettext - from subiquity.common.types import Bootloader @@ -422,23 +419,6 @@ def asdict(inst): # in the FilesystemModel or FilesystemController classes. -class DeviceAction(enum.Enum): - # Information about a drive - INFO = pgettext("DeviceAction", "Info") - # Edit a device (partition, logical volume, RAID, etc) - EDIT = pgettext("DeviceAction", "Edit") - REFORMAT = pgettext("DeviceAction", "Reformat") - PARTITION = pgettext("DeviceAction", "Add Partition") - CREATE_LV = pgettext("DeviceAction", "Create Logical Volume") - FORMAT = pgettext("DeviceAction", "Format") - REMOVE = pgettext("DeviceAction", "Remove from RAID/LVM") - DELETE = pgettext("DeviceAction", "Delete") - TOGGLE_BOOT = pgettext("DeviceAction", "Make Boot Device") - - def str(self): - return pgettext(type(self).__name__, self.value) - - def _generic_can_EDIT(obj): cd = obj.constructed_device() if cd is None: @@ -586,13 +566,11 @@ class _Formattable(ABC): else: return cd - @property - @abstractmethod - def supported_actions(self): - pass - def action_possible(self, action): - assert action in self.supported_actions + from subiquity.common.filesystem.actions import ( + supported_actions, + ) + assert action in supported_actions(self) r = getattr(self, "_can_" + action.name) if isinstance(r, bool): return r, None @@ -822,19 +800,6 @@ class Disk(_Device): else: return True - @property - def supported_actions(self): - actions = [ - DeviceAction.INFO, - DeviceAction.REFORMAT, - DeviceAction.PARTITION, - DeviceAction.FORMAT, - DeviceAction.REMOVE, - ] - if self._m.bootloader != Bootloader.NONE: - actions.append(DeviceAction.TOGGLE_BOOT) - return actions - _can_INFO = True @property @@ -1027,12 +992,6 @@ class Partition(_Formattable): else: return False - supported_actions = [ - DeviceAction.EDIT, - DeviceAction.REMOVE, - DeviceAction.DELETE, - ] - _can_EDIT = property(_generic_can_EDIT) _can_REMOVE = property(_generic_can_REMOVE) @@ -1106,15 +1065,6 @@ class Raid(_Device): def desc(self): return _("software RAID {level}").format(level=self.raidlevel[4:]) - supported_actions = [ - DeviceAction.EDIT, - DeviceAction.PARTITION, - DeviceAction.FORMAT, - DeviceAction.REMOVE, - DeviceAction.DELETE, - DeviceAction.REFORMAT, - ] - @property def _can_EDIT(self): if self.preserve: @@ -1183,12 +1133,6 @@ class LVM_VolGroup(_Device): def desc(self): return _("LVM volume group") - supported_actions = [ - DeviceAction.EDIT, - DeviceAction.CREATE_LV, - DeviceAction.DELETE, - ] - @property def _can_EDIT(self): if self.preserve: @@ -1247,11 +1191,6 @@ class LVM_LogicalVolume(_Formattable): label = short_label - supported_actions = [ - DeviceAction.EDIT, - DeviceAction.DELETE, - ] - _can_EDIT = True @property diff --git a/subiquity/models/tests/test_filesystem.py b/subiquity/models/tests/test_filesystem.py index bfb6cbac..87938c53 100644 --- a/subiquity/models/tests/test_filesystem.py +++ b/subiquity/models/tests/test_filesystem.py @@ -17,10 +17,13 @@ import unittest import attr +from subiquity.common.filesystem.actions import ( + DeviceAction, + supported_actions, + ) from subiquity.models.filesystem import ( Bootloader, dehumanize_size, - DeviceAction, Disk, FilesystemModel, get_raid_size, @@ -376,14 +379,14 @@ class TestFilesystemModel(unittest.TestCase): ["to be reformatted as ext4", "mounted at /"]) def assertActionNotSupported(self, obj, action): - self.assertNotIn(action, obj.supported_actions) + self.assertNotIn(action, supported_actions(obj)) def assertActionPossible(self, obj, action): - self.assertIn(action, obj.supported_actions) + self.assertIn(action, supported_actions(obj)) self.assertTrue(obj.action_possible(action)[0]) def assertActionNotPossible(self, obj, action): - self.assertIn(action, obj.supported_actions) + self.assertIn(action, supported_actions(obj)) self.assertFalse(obj.action_possible(action)[0]) def _test_remove_action(self, model, objects): diff --git a/subiquity/server/controllers/filesystem.py b/subiquity/server/controllers/filesystem.py index 77b92600..721ae1be 100644 --- a/subiquity/server/controllers/filesystem.py +++ b/subiquity/server/controllers/filesystem.py @@ -37,6 +37,10 @@ from subiquitycore.lsb_release import lsb_release from subiquity.common.apidef import API from subiquity.common.errorreport import ErrorReportKind +from subiquity.common.filesystem.actions import ( + DeviceAction, + supported_actions, + ) from subiquity.common.filesystem.manipulator import FilesystemManipulator from subiquity.common.types import ( Bootloader, @@ -47,7 +51,6 @@ from subiquity.common.types import ( ) from subiquity.models.filesystem import ( dehumanize_size, - DeviceAction, ) from subiquity.server.controller import ( SubiquityController, @@ -129,7 +132,7 @@ class FilesystemController(SubiquityController, FilesystemManipulator): def guided_lvm(self, disk, lvm_options=None): self.reformat(disk) - if DeviceAction.TOGGLE_BOOT in disk.supported_actions: + if DeviceAction.TOGGLE_BOOT in supported_actions(disk): self.add_boot_disk(disk) self.create_partition( device=disk, spec=dict( diff --git a/subiquity/ui/views/filesystem/filesystem.py b/subiquity/ui/views/filesystem/filesystem.py index b4d6fcbf..ee38a6ef 100644 --- a/subiquity/ui/views/filesystem/filesystem.py +++ b/subiquity/ui/views/filesystem/filesystem.py @@ -60,8 +60,11 @@ from subiquitycore.ui.utils import ( ) from subiquitycore.view import BaseView -from subiquity.models.filesystem import ( +from subiquity.common.filesystem.actions import ( DeviceAction, + supported_actions, + ) +from subiquity.models.filesystem import ( humanize_size, ) @@ -337,7 +340,7 @@ class DeviceList(WidgetWrap): def _action_menu_for_device(self, device): device_actions = [] - for action in device.supported_actions: + for action in supported_actions(device): label_meth = getattr( self, '_label_{}'.format(action.name), lambda a, d: a.str()) label = label_meth(action, device)