Dictionary with restricted keys

Question

I'm currently building some software that works with systemd unit files, and I've been trying to improve how we construct the unit files.

This is all running in Python 3.4

This first block is the base classes I'm building things up with.

class LimitedDict(dict):
    """
    Sub Class of the dictionary object that restricts the allowed keys.
    The allowed keys are set when creating the object.

    >>> a = LimitedDict(allowed_keys=['foo'], foo='bar')
    >>> a == {'foo': 'bar'}
    True

    >>> ok_keywords = {'foo': 'bar'}
    >>> a = LimitedDict(allowed_keys=['foo'], **ok_keywords)
    >>> a == {'foo': 'bar'}
    True

    >>> LimitedDict(allowed_keys=['foo'], foo='bar', fail='true')
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "<stdin>", line 13, in __init__
    KeyError: "key: 'fail' not in allowed keys: '['foo']'"

    >>> not_ok_keywords = {'foo': 'bar', 'fail': 'oops'}
    >>> LimitedDict(allowed_keys=['foo'], **not_ok_keywords)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "<stdin>", line 13, in __init__
    KeyError: "key: 'fail' not in allowed keys: '['foo']'"

    >>> a = LimitedDict(allowed_keys=['foo'], foo='bar')
    >>> a['fail'] = 'true'
    Traceback (most recent call last):
        ...
    KeyError: "key: 'fail' not in allowed keys: '['foo']'"
    """
    _allowed_keys = list()

    def __init__(self, allowed_keys, **kwargs):
        if not isinstance(allowed_keys, list):
            AttributeError("'allowed_keys' must be a list")
        self._allowed_keys = allowed_keys
        for key in kwargs.keys():
            if key not in self._allowed_keys:
                raise KeyError("key: '" + str(key) + "' not in allowed keys: '" + str(self._allowed_keys) + "'")
            self[key] = kwargs[key]

    def __setitem__(self, key, val):
        if key not in self._allowed_keys:
           raise KeyError("key: '" + str(key) + "' not in allowed keys: '" + str(self._allowed_keys) + "'")
        dict.__setitem__(self, key, val)


class UnitDict(LimitedDict):
    """
    This LimitedDict object is further customized for use in building a
    json representation of a systemd unit file. The additional restrictions
    ensure we don't introduce elements outside the allowed values for a
    systemd unit file.

    Create a dictionary with the correct keys:
    >>> a = UnitDict(name='Test', desiredState='Loaded', options='Test')
    >>> a == {'name': 'Test', 'desiredState': 'Loaded', 'options': 'Test'}
    True

    Ignore incorrect keys
    >>> a = UnitDict(name='Test', desiredState='Loaded', options='Test', ignored='True')
    >>> a == {'options': 'Test', 'name': 'Test', 'desiredState': 'Loaded'}
    True
    """
    def __init__(self, **kwargs):
        super_kwargs = {
            'name': kwargs['name'],
            'desiredState': kwargs['desiredState'],
            'options': kwargs['options']}
        super().__init__(allowed_keys=list(super_kwargs.keys()), **super_kwargs)


class UnitOptionDict(LimitedDict):
    def __init__(self, **kwargs):
        super_kwargs = {
            'name': kwargs['name'],
            'section': kwargs['section'],
            'value': kwargs['value']}
        super().__init__(allowed_keys=list(super_kwargs.keys()), **super_kwargs)

This is the code I want feedback on. I'm fairly sure this can be improved. Most likely with metaclasses but after much reading, I'm still not completely sure just how to construct such a metaclass, or if that really is the best way to refactor this to be cleaner.

def build_option_dict(caller, **kwargs):
    if not kwargs['value']:
        raise KeyError
    if not isinstance(kwargs['value'], str):
        raise ValueError
    return {
        'section': str(caller.__class__.__qualname__).split('.')[-2],
        'name': str(caller.__class__.__qualname__).split('.')[-1],
        'value': str(kwargs['value'])
    }


class MightyMorphingMetaMagic(UnitOptionDict):
    def __init__(self, **kwargs):
        super_kwargs = build_option_dict(self, **kwargs)
        super().__init__(allowed_keys=list(super_kwargs.keys()), **super_kwargs)


class OptionSections:
    class Unit:
        class Description(MightyMorphingMetaMagic):
            pass

        class After(MightyMorphingMetaMagic):
            pass

        class Require(MightyMorphingMetaMagic):
            pass

    class Service:
        class TimeoutStartSec(MightyMorphingMetaMagic):
            pass

        class ExecStartPre(MightyMorphingMetaMagic):
            pass

        class ExecStart(MightyMorphingMetaMagic):
            pass

        class ExecStartPost(MightyMorphingMetaMagic):
            pass

        class ExecStopPre(MightyMorphingMetaMagic):
            pass

        class ExecStop(MightyMorphingMetaMagic):
            pass

        class ExecStopPost(MightyMorphingMetaMagic):
            pass

The way this ends up being used is typically like this, building a json representation from a fairly stable list of unit file parts.

from lib.systemd import OptionSections as Sections
unit_json = [
        Sections.Unit.Description(value=str(self.description)),
        Sections.Service.ExecStartPre(value=str(self.execstartpre)),
        Sections.Service.ExecStart(value=str(self.execstart)),
        Sections.Service.ExecStop(value=str(self.execstop))
    ]

Answer 1

To re-factor this code firstly I'd recommend using packages instead of nested classes here, so create a package named Sections and create two more packages named Unit and Services inside of it, you can also move the dictionary definitions inside of this package say in a file named dicts.py and each of those inner packages then can import the required dict(s) if required.

The structure will look something like this:

$ tree Sections/
Sections/
├── dicts.py
├── __init__.py
├── Service
│   └── __init__.py
└── Unit
    └── __init__.py

Here to use the nested dot notation Sections.Unit.Description we import the required items from each of the individual packages to Sections's __init__.py file:

$ cat Sections/__init__.py 
from .Unit import After, Description
from .Service import ExecStartPre, ExecStart, ExecStartPost, ExecStopPre,\
    ExecStop, ExecStopPost, TimeoutStartSec

Similarly the contents of Unit/__init__.py and Services/__init__.py are:

$ cat Sections/Unit/__init__.py 
from ..dicts import MightyMorphingMetaMagic

class Description(MightyMorphingMetaMagic):
    pass

class After(MightyMorphingMetaMagic):
    pass

class Require(MightyMorphingMetaMagic):
    pass

$ cat Sections/Service/__init__.py 
from ..dicts import MightyMorphingMetaMagic

class TimeoutStartSec(MightyMorphingMetaMagic):
    pass

class ExecStartPre(MightyMorphingMetaMagic):
    pass

class ExecStart(MightyMorphingMetaMagic):
    pass

class ExecStartPost(MightyMorphingMetaMagic):
    pass

class ExecStopPre(MightyMorphingMetaMagic):
    pass

class ExecStop(MightyMorphingMetaMagic):
    pass

class ExecStopPost(MightyMorphingMetaMagic):
    pass

Now in your main script you can simply import Sections(considering this packages lies in the module search path):

import Sections

unit_json = [

        Sections.Unit.Description(value=str(self.description)),
        Sections.Service.ExecStartPre(value=str(self.execstartpre)),
        Sections.Service.ExecStart(value=str(self.execstart)),
        Sections.Service.ExecStop(value=str(self.execstop))
    ]

---

Now to accommodate this package based version we will have to make some changes:

• We can't use __qualname__ now, an alternative is to use the __module__ argument of the class object.

• Instead of caller.__class__ to get instance's class we can use type(caller). In new-style classes both do the same thing.

• Instead of using a list for _allowed_keys it's better to use a set as it stores only unique items and provides O(1) lookups.

• We can allow all types of iterables to be passed as _allowed_keys instead of just list, we can check this using an abstract base class collections.Iterable.

• As allowed_keys is kind of compulsory argument here, better make it a key-word only argument with default value of None, this will also help in separating it from normal kwargs.

• Use string formatting instead of str() calls and +, plus using 'str(some_key)' can be confusing to the user when some_key is 1 or '1'. A better way is to use the repr representation of the object, in new-style string formatting we can obtain it using {!r}.

• While checking for a key in a dict simply use if key in dict, the .keys() call on the dict is unnecessary and will take O(N) time in Python 2.

• To get the keys of a dictionary use list(some_dict) instead list(some_dict.keys()), this works as is in both Python 2 and 3. But again we should better pass the keys in the form of a set whenever possible.

• Use for key, value in some_dict.items(): if you want key as well corresponding value while iterating over a dict.

• I noticed that you are calculating super_kwargs inside __init__ of each class that too from the kwargs passed to that class. I'd say why not define _allowed_key in that class itself, and the call super().__init__(allowed_keys=self._allowed_keys, **kwargs). Now the baseclass LimitedDict will handle the rest.

• To support the super().__init__ I've used the same signature in each class's __init__ method.

• I cannot think of a way to use a metaclass here myself, so I moved build_option_dict inside of the class.

• if not kwargs['value']: raise KeyError doesn't make much sense, better store the value in some variable and if the key is missing Python will raise an error itself.

Now my dicts.py looks like(removed doctests to save some space):

from collections import Iterable


class LimitedDict(dict):

    _allowed_keys = set()

    def __init__(self, *, allowed_keys=None, **kwargs):

        if not isinstance(allowed_keys, Iterable):
            AttributeError("'allowed_keys' must be an iterable")
        self._allowed_keys = allowed_keys
        for key, value in kwargs.items():
            if key not in self._allowed_keys:
                raise KeyError("key: {!r} not in allowed keys: {!r}".format(key,
                                                                            self._allowed_keys))
            self[key] = value

    def __setitem__(self, key, val):
        if key not in self._allowed_keys:
            raise KeyError("key: {!r} not in allowed keys: {!r}".format(key,
                                                                        self._allowed_keys))
        dict.__setitem__(self, key, val)


class UnitDict(LimitedDict):

    _allowed_keys = {'name', 'desiredState', 'options'}

    def __init__(self, *, allowed_keys=None, **kwargs):
        super().__init__(allowed_keys=self._allowed_keys, **kwargs)


class UnitOptionDict(LimitedDict):
    _allowed_keys = {'name', 'section', 'value'}

    def __init__(self, *, allowed_keys=None, **kwargs):
        super().__init__(allowed_keys=self._allowed_keys, **kwargs)


class MightyMorphingMetaMagic(UnitOptionDict):

    def build_option_dict(self, **kwargs):
        value = kwargs['value']
        if not isinstance(value, str):
            raise ValueError("'value' must be a of type str")

        return {
            'section': type(self).__module__.split('.')[-1],
            'name': type(self).__name__,
            'value': str(kwargs['value'])
        }

    def __init__(self,  *, allowed_keys=None, **kwargs):
        super_kwargs = self.build_option_dict(**kwargs)
        super().__init__(allowed_keys=self._allowed_keys, **super_kwargs)