[PATCH] Use abstract base classes for mixins.
David Shea
dshea at redhat.com
Wed Dec 4 21:07:11 UTC 2013
This is only a beginning, but hopefully enough to get the idea across.
The purpose of this is twofold: For mixins/interfaces/whatever that do
not make sense unless inherited by another class, don't allow the class
to be instantiated. For mixins/interfaces/whatever that depend on
properties that are implemented in the inheriting class or its other
subclasses, make the relationship explicit through the use of abstract
properties.
SourceSwitchHandler is a good example, and the catalyst for this.
SourceSwitchHandler manipulates self.data, which is provided in the
subclass by way of EditTUISpoke -> UIObject. This kind of a
relationship is flagged by pylint since .data is not provided by
SourceSwitchHandler or any of its subclasses. b5df3a90 attempted to
remedy this by storing a copy of data in SourceSwitchHandler, but that
commit is incorrect: self.data in SourceSwitchHandler will resolve to
UIObject.data when called from a class that inherits from both, and
UIObject.data is a read-only property. This commit instead makes
SourceSwitchHandler an abstract base class. It implements a data
property of its own but requires that the property be overridden by
another class. This way no class can inherit from SourceSwitchHandler
without providing a data property.
---
po/POTFILES.in | 1 +
pyanaconda/ui/gui/spokes/storage.py | 36 +------
pyanaconda/ui/interfaces.py | 184 ++++++++++++++++++++++++++++++++++++
pyanaconda/ui/tui/spokes/source.py | 95 ++-----------------
4 files changed, 196 insertions(+), 120 deletions(-)
create mode 100644 pyanaconda/ui/interfaces.py
diff --git a/po/POTFILES.in b/po/POTFILES.in
index af263f9..c1d3a7e 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -28,6 +28,7 @@ pyanaconda/packaging/yumpayload.py
# Interfaces
pyanaconda/ui/common.py
+pyanaconda/ui/interfaces.py
pyanaconda/ui/__init__.py
# Common stuff
diff --git a/pyanaconda/ui/gui/spokes/storage.py b/pyanaconda/ui/gui/spokes/storage.py
index 4192a1a..ce80c72 100644
--- a/pyanaconda/ui/gui/spokes/storage.py
+++ b/pyanaconda/ui/gui/spokes/storage.py
@@ -50,6 +50,7 @@ from pyanaconda.ui.gui.spokes.lib.detailederror import DetailedErrorDialog
from pyanaconda.ui.gui.spokes.lib.resize import ResizeDialog
from pyanaconda.ui.gui.categories.system import SystemCategory
from pyanaconda.ui.gui.utils import enlightbox, escape_markup
+from pyanaconda.ui.interfaces import StorageChecker
from pyanaconda.kickstart import doKickstartStorage, getAvailableDiskSpace
from blivet import empty_device
@@ -306,33 +307,6 @@ class InstallOptions3Dialog(InstallOptions1Dialog):
def continue_response(self):
return self.RESPONSE_CONTINUE_NONE
-class StorageChecker(object):
- errors = []
- warnings = []
- _mainSpokeClass = "StorageSpoke"
-
- def __init__(self):
- # This is provided by the StorageSpoke class, which is a subclass of
- # this one. Backwards, I know.
- self.storage = None
-
- def run(self):
- threadMgr.add(AnacondaThread(name=constants.THREAD_CHECK_STORAGE,
- target=self.checkStorage))
-
- def checkStorage(self):
- threadMgr.wait(constants.THREAD_EXECUTE_STORAGE)
-
- hubQ.send_not_ready(self._mainSpokeClass)
- hubQ.send_message(self._mainSpokeClass, _("Checking storage configuration..."))
- (StorageChecker.errors,
- StorageChecker.warnings) = self.storage.sanityCheck()
- hubQ.send_ready(self._mainSpokeClass, True)
- for e in StorageChecker.errors:
- log.error(e)
- for w in StorageChecker.warnings:
- log.warn(w)
-
class StorageSpoke(NormalSpoke, StorageChecker):
builderObjects = ["storageWindow", "addSpecializedImage"]
mainWidgetName = "storageWindow"
@@ -431,7 +405,7 @@ class StorageSpoke(NormalSpoke, StorageChecker):
doKickstartStorage(self.storage, self.data, self.instclass)
except (StorageError, KickstartValueError) as e:
log.error("storage configuration failed: %s", e)
- StorageChecker.errors = str(e).split("\n")
+ self.errors = str(e).split("\n")
hubQ.send_message(self.__class__.__name__, _("Failed to save storage configuration..."))
self.data.bootloader.bootDrive = ""
self.data.ignoredisk.drives = []
@@ -443,14 +417,14 @@ class StorageSpoke(NormalSpoke, StorageChecker):
self._applyDiskSelection(self.selected_disks)
except BootLoaderError as e:
log.error("BootLoader setup failed: %s", e)
- StorageChecker.errors = str(e).split("\n")
+ self.errors = str(e).split("\n")
hubQ.send_message(self.__class__.__name__, _("Failed to save storage configuration..."))
self.data.bootloader.bootDrive = ""
else:
if self.autopart:
# this was already run as part of doAutoPartition. dumb.
- StorageChecker.errors = []
- StorageChecker.warnings = []
+ self.errors = []
+ self.warnings = []
self.run()
finally:
self._ready = True
diff --git a/pyanaconda/ui/interfaces.py b/pyanaconda/ui/interfaces.py
new file mode 100644
index 0000000..6b46d03
--- /dev/null
+++ b/pyanaconda/ui/interfaces.py
@@ -0,0 +1,184 @@
+# Abstract interfaces for UI classes
+#
+# Copyright (C) 2013 Red Hat, Inc.
+#
+# This copyrighted material is made available to anyone wishing to use,
+# modify, copy, or redistribute it subject to the terms and conditions of
+# the GNU General Public License v.2, or (at your option) any later version.
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY expressed or implied, including the implied warranties 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, write to the
+# Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+# 02110-1301, USA. Any Red Hat trademarks that are incorporated in the
+# source code or documentation are not subject to the GNU General Public
+# License and may only be used or replicated with the express permission of
+# Red Hat, Inc.
+#
+# Red Hat Author(s): David Shea <dshea at redhat.com
+#
+
+# This file contains abstract base classes that provide specific functionality
+# that can be added to another class. The idea is sort-of modelled after Java's
+# interfaces. An abstract base class cannot be instatiated, and it provides a
+# contract for classes that inherit from it: any method or property marked as
+# abstract in the base class must be overriden in the inheriting class. This
+# allows for cleaner implementation of certain types of mixin-classes: a class
+# that adds functionality to another class can explicitly require that methods
+# or properties be provided by the inheriting class or another superclass of
+# the inheriting class.
+#
+# In general, classes that inherit from abstract base classes should place the
+# abstract base class at the end of the inheritance list. This way any abstract
+# methods or properties in the abc will be overridden by the base classes
+# that are first in the inheritance list. For example, an abstract base class
+# may add a method that reads from spoke.data:
+#
+# class Mixin(object):
+# __metaclass__ = ABCMeta
+#
+# @abstractproperty
+# def data(self): pass
+#
+# def isHD(self):
+# return self.data.method == "harddrive"
+#
+# The Mixin class will add the method isHD to any class that inherits from it,
+# and classes that inherit from Mixin must provide a data property.
+#
+# class MixedObject(UIObject, Mixin):
+# ....
+#
+# The method resolution order of MixedObject resolves UIObject.data before
+# Mixin.data, so UIObject.data satisfies the requirment that Mixin.data be
+# overriden.
+
+from abc import ABCMeta, abstractproperty
+
+from pyanaconda import constants
+from pyanaconda.threads import threadMgr, AnacondaThread
+from pyanaconda.ui.communication import hubQ
+from pyanaconda.i18n import _
+
+import logging
+import copy
+
+class StorageChecker(object):
+ __metaclass__ = ABCMeta
+
+ log = logging.getLogger("anaconda")
+
+ def __init__(self):
+ self.errors = []
+ self.warnings = []
+
+ @abstractproperty
+ def storage(self): pass
+
+ def run(self):
+ threadMgr.add(AnacondaThread(name=constants.THREAD_CHECK_STORAGE,
+ target=self.checkStorage))
+
+ def checkStorage(self):
+ threadMgr.wait(constants.THREAD_EXECUTE_STORAGE)
+
+ hubQ.send_not_read(self.__class__.__name__)
+ hubQ.send_message(self.__class__.__name__, _("Check storage configuration..."))
+ (self.errors,
+ self.warnings) = self.storage.sanityCheck()
+ for e in self.errors:
+ self.log.error(e)
+ for w in self.warnings:
+ self.log.warn(w)
+
+class SourceSwitchHandler(object):
+ """ A class that can be used as a mixin handling
+ installation source switching.
+ It will correctly switch to the new method
+ and cleanup any previous method set.
+ """
+
+ __metaclass__ = ABCMeta
+
+ @abstractproperty
+ def data(self): pass
+
+ @abstractproperty
+ def storage(self): pass
+
+ def __init__(self):
+ self._device = None
+ self._current_iso_path = None
+
+ def _clean_hdd_iso(self):
+ """ Clean HDD ISO usage
+ This means unmounting the partition and unprotecting it,
+ so it can be used for the installation.
+ """
+ if self.data.method.method == "harddrive" and self.data.method.partition:
+ part = self.data.method.partition
+ dev = self.storage.devicetree.getDeviceByName(part)
+ if dev:
+ dev.protected = False
+ self.storage.config.protectedDevSpecs.remove(part)
+
+ def set_source_hdd_iso(self, device, iso_path):
+ """ Switch to the HDD ISO install source
+ :param partition: name of the partition hosting the ISO
+ :type partition: string
+ :param iso_path: full path to the source ISO file
+ :type iso_path: string
+ """
+ partition = device.name
+ # the GUI source spoke also does the copy
+ old_source = copy.copy(self.data.method)
+
+ # if a different partition was used previously, unprotect it
+ if old_source.method == "harddrive" and old_source.partition != partition:
+ self._clean_hdd_iso()
+
+ # protect current device
+ if device:
+ device.protected = True
+ self.storage.config.protectedDevSpecs.append(device.name)
+
+ self.data.method.method = "harddrive"
+ self.data.method.partition = partition
+ # the / gets stripped off by payload.ISOImage
+ self.data.method.dir = "/" + iso_path
+
+ # as we already made the device protected when
+ # switching to it, we don't need to protect it here
+
+ def set_source_url(self, url=None):
+ """ Switch to install source specified by URL """
+ # clean any old HDD ISO sources
+ self._clean_hdd_iso()
+
+ self.data.method.method = "url"
+ if url is not None:
+ self.data.method.url = url
+
+ def set_source_nfs(self, opts=None):
+ """ Switch to NFS install source """
+ # clean any old HDD ISO sources
+ self._clean_hdd_iso()
+
+ self.data.method.method = "nfs"
+ if opts is not None:
+ self.data.method.opts = opts
+
+ def set_source_cdrom(self):
+ """ Switch to cdrom install source """
+ # clean any old HDD ISO sources
+ self._clean_hdd_iso()
+
+ self.data.method.method = "cdrom"
+
+ def set_source_closest_mirror(self):
+ """ Switch to the closest mirror install source """
+ # clean any old HDD ISO sources
+ self._clean_hdd_iso()
+
+ self.data.method.method = None
diff --git a/pyanaconda/ui/tui/spokes/source.py b/pyanaconda/ui/tui/spokes/source.py
index 977e7df..a3397fd 100644
--- a/pyanaconda/ui/tui/spokes/source.py
+++ b/pyanaconda/ui/tui/spokes/source.py
@@ -33,12 +33,13 @@ from pyanaconda.constants import THREAD_PAYLOAD_MD, THREAD_STORAGE, THREAD_STORA
from pyanaconda.constants import THREAD_CHECK_SOFTWARE, ISO_DIR, DRACUT_ISODIR
from pyanaconda.constants_text import INPUT_PROCESSED
+from pyanaconda.ui.interfaces import SourceSwitchHandler
+
from blivet.util import get_mount_paths
import re
import os
import fnmatch
-import copy
import logging
LOG = logging.getLogger("anaconda")
@@ -46,90 +47,6 @@ LOG = logging.getLogger("anaconda")
__all__ = ["SourceSpoke"]
-class SourceSwitchHandler(object):
- """ A class that can be used as a mixin handling
- installation source switching.
- It will correctly switch to the new method
- and cleanup any previous method set.
- """
- def __init__(self, data, storage):
- self._device = None
- self._current_iso_path = None
- self.data = data
- self.storage = storage
-
- def _clean_hdd_iso(self):
- """ Clean HDD ISO usage
- This means unmounting the partition and unprotecting it,
- so it can be used for the installation.
- """
- if self.data.method.method == "harddrive" and self.data.method.partition:
- part = self.data.method.partition
- dev = self.storage.devicetree.getDeviceByName(part)
- if dev:
- dev.protected = False
- self.storage.config.protectedDevSpecs.remove(part)
-
- def set_source_hdd_iso(self, device, iso_path):
- """ Switch to the HDD ISO install source
- :param partition: name of the partition hosting the ISO
- :type partition: string
- :param iso_path: full path to the source ISO file
- :type iso_path: string
- """
- partition = device.name
- # the GUI source spoke also does the copy
- old_source = copy.copy(self.data.method)
-
- # if a different partition was used previously, unprotect it
- if old_source.method == "harddrive" and old_source.partition != partition:
- self._clean_hdd_iso()
-
- # protect current device
- if device:
- device.protected = True
- self.storage.config.protectedDevSpecs.append(device.name)
-
- self.data.method.method = "harddrive"
- self.data.method.partition = partition
- # the / gets stripped off by payload.ISOImage
- self.data.method.dir = "/" + iso_path
-
- # as we already made the device protected when
- # switching to it, we don't need to protect it here
-
- def set_source_url(self, url=None):
- """ Switch to install source specified by URL """
- # clean any old HDD ISO sources
- self._clean_hdd_iso()
-
- self.data.method.method = "url"
- if url is not None:
- self.data.method.url = url
-
- def set_source_nfs(self, opts=None):
- """ Switch to NFS install source """
- # clean any old HDD ISO sources
- self._clean_hdd_iso()
-
- self.data.method.method = "nfs"
- if opts is not None:
- self.data.method.opts = opts
-
- def set_source_cdrom(self):
- """ Switch to cdrom install source """
- # clean any old HDD ISO sources
- self._clean_hdd_iso()
-
- self.data.method.method = "cdrom"
-
- def set_source_closest_mirror(self):
- """ Switch to the closest mirror install source """
- # clean any old HDD ISO sources
- self._clean_hdd_iso()
-
- self.data.method.method = None
-
class SourceSpoke(EditTUISpoke, SourceSwitchHandler):
""" Spoke used to customize the install source repo. """
title = N_("Installation source")
@@ -142,7 +59,7 @@ class SourceSpoke(EditTUISpoke, SourceSwitchHandler):
def __init__(self, app, data, storage, payload, instclass):
EditTUISpoke.__init__(self, app, data, storage, payload, instclass)
- SourceSwitchHandler.__init__(self, data, storage)
+ SourceSwitchHandler.__init__(self)
self._ready = False
self.errors = []
self._cdrom = None
@@ -344,7 +261,7 @@ class SpecifyRepoSpoke(EditTUISpoke, SourceSwitchHandler):
def __init__(self, app, data, storage, payload, instclass, selection):
EditTUISpoke.__init__(self, app, data, storage, payload, instclass)
- SourceSwitchHandler.__init__(self, data, storage)
+ SourceSwitchHandler.__init__(self)
self.selection = selection
self.args = self.data.method
@@ -383,7 +300,7 @@ class SpecifyNFSRepoSpoke(EditTUISpoke, SourceSwitchHandler):
def __init__(self, app, data, storage, payload, instclass, selection, errors):
EditTUISpoke.__init__(self, app, data, storage, payload, instclass)
- SourceSwitchHandler.__init__(self, data, storage)
+ SourceSwitchHandler.__init__(self)
self.selection = selection
self.args = self.data.method
self.errors = errors
@@ -504,7 +421,7 @@ class SelectISOSpoke(NormalTUISpoke, SourceSwitchHandler):
def __init__(self, app, data, storage, payload, instclass, device):
NormalTUISpoke.__init__(self, app, data, storage, payload, instclass)
- SourceSwitchHandler.__init__(self, data, storage)
+ SourceSwitchHandler.__init__(self)
self.selection = None
self.args = self.data.method
self._device = device
--
1.8.4.2
More information about the anaconda-patches
mailing list