Merge pull request #53 from a-ovchinnikov/develop

Initial refactoring
diff --git a/.travis.yml b/.travis.yml
index b060639..559ef9a 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -25,8 +25,9 @@
     - PACKAGENAME="reclass"
 
 install: &pyinst
-- pip install pyparsing
-- pip install PyYAML
+- pip install -r requirements.txt
+#- pip install pyparsing
+#- pip install PyYAML
 # To test example models with kitchen:
 - |
   test -e Gemfile || cat <<EOF > Gemfile
diff --git a/Pipfile b/Pipfile
index 525e7cc..fc2022b 100644
--- a/Pipfile
+++ b/Pipfile
@@ -10,6 +10,7 @@
 PyYAML = "*"
 six = "*"
 pyyaml = "*"
+enum34 = "*"
 # FIXME, issues with compile phase
 #"pygit2" = "*"
 
diff --git a/reclass/core.py b/reclass/core.py
index ed5a392..2facfbe 100644
--- a/reclass/core.py
+++ b/reclass/core.py
@@ -210,8 +210,8 @@
         try:
             node = self._node_entity(nodename)
             node.initialise_interpolation()
-            if node.parameters.has_inv_query() and inventory is None:
-                inventory = self._get_inventory(node.parameters.needs_all_envs(), node.environment, node.parameters.get_inv_queries())
+            if node.parameters.has_inv_query and inventory is None:
+                inventory = self._get_inventory(node.parameters.needs_all_envs, node.environment, node.parameters.get_inv_queries())
             node.interpolate(inventory)
             return node
         except InterpolationError as e:
@@ -237,7 +237,7 @@
         inventory = self._get_inventory(True, '', None)
         for n in self._storage.enumerate_nodes():
             entities[n] = self._nodeinfo(n, inventory)
-            if entities[n].parameters.has_inv_query():
+            if entities[n].parameters.has_inv_query:
                 nodes.add(n)
         for n in query_nodes:
             entities[n] = self._nodeinfo(n, inventory)
diff --git a/reclass/datatypes/applications.py b/reclass/datatypes/applications.py
index 8c6ed15..90ae54c 100644
--- a/reclass/datatypes/applications.py
+++ b/reclass/datatypes/applications.py
@@ -33,9 +33,9 @@
         self._negations = []
         super(Applications, self).__init__(iterable)
 
-    def _get_negation_prefix(self):
+    @property
+    def negation_prefix(self):
         return self._negation_prefix
-    negation_prefix = property(_get_negation_prefix)
 
     def append_if_new(self, item):
         self._assert_is_string(item)
diff --git a/reclass/datatypes/classes.py b/reclass/datatypes/classes.py
index 33d9b93..5270e28 100644
--- a/reclass/datatypes/classes.py
+++ b/reclass/datatypes/classes.py
@@ -11,11 +11,6 @@
 from __future__ import print_function
 from __future__ import unicode_literals
 
-#try:
-#    from types import StringTypes
-#except ImportError:
-#    StringTypes = (str, )
-
 import six
 import os
 from reclass.errors import InvalidClassnameError
@@ -61,7 +56,6 @@
             self.append_if_new(i)
 
     def _assert_is_string(self, item):
-        #if not isinstance(item, StringTypes):
         if not isinstance(item, six.string_types):
             raise TypeError('%s instances can only contain strings, '\
                             'not %s' % (self.__class__.__name__, type(item)))
@@ -81,5 +75,4 @@
         self._append_if_new(item)
 
     def __repr__(self):
-        return '%s(%r)' % (self.__class__.__name__,
-                           self._items)
+        return '%s(%r)' % (self.__class__.__name__, self._items)
diff --git a/reclass/datatypes/entity.py b/reclass/datatypes/entity.py
index 3c927c3..8133de5 100644
--- a/reclass/datatypes/entity.py
+++ b/reclass/datatypes/entity.py
@@ -22,18 +22,16 @@
     for merging. The name and uri of an Entity will be updated to the name and
     uri of the Entity that is being merged.
     '''
-    def __init__(self, settings, classes=None, applications=None, parameters=None,
-                 exports=None, uri=None, name=None, environment=None):
-        self._uri = uri or ''
-        self._name = name or ''
-        if classes is None: classes = Classes()
-        self._set_classes(classes)
-        if applications is None: applications = Applications()
-        self._set_applications(applications)
-        if parameters is None: parameters = Parameters(None, settings, uri)
-        if exports is None: exports = Exports(None, settings, uri)
-        self._set_parameters(parameters)
-        self._set_exports(exports)
+    def __init__(self, settings, classes=None, applications=None,
+                 parameters=None, exports=None, uri=None, name=None,
+                 environment=None):
+        self._uri = '' if uri is None else uri
+        self._name = '' if name is None else name
+        self._classes = self._set_field(classes, Classes)
+        self._applications = self._set_field(applications, Applications)
+        pars = [None, settings, uri]
+        self._parameters = self._set_field(parameters, Parameters, pars)
+        self._exports = self._set_field(exports, Exports, pars)
         self._environment = environment
 
     name = property(lambda s: s._name)
@@ -52,29 +50,15 @@
     def environment(self, value):
         self._environment = value
 
-    def _set_classes(self, classes):
-        if not isinstance(classes, Classes):
-            raise TypeError('Entity.classes cannot be set to '\
-                            'instance of type %s' % type(classes))
-        self._classes = classes
-
-    def _set_applications(self, applications):
-        if not isinstance(applications, Applications):
-            raise TypeError('Entity.applications cannot be set to '\
-                            'instance of type %s' % type(applications))
-        self._applications = applications
-
-    def _set_parameters(self, parameters):
-        if not isinstance(parameters, Parameters):
-            raise TypeError('Entity.parameters cannot be set to '\
-                            'instance of type %s' % type(parameters))
-        self._parameters = parameters
-
-    def _set_exports(self, exports):
-        if not isinstance(exports, Exports):
-            raise TypeError('Entity.exports cannot be set to '\
-                            'instance of type %s' % type(exports))
-        self._exports = exports
+    def _set_field(self, received_value, expected_type, parameters=None):
+        if parameters is None:
+            parameters = []
+        if received_value is None:
+            return expected_type(*parameters)
+        if not isinstance(received_value, expected_type):
+            raise TypeError('Entity.%s cannot be set to instance of type %s' %
+                            (type(expected_type), type(received_value)))
+        return received_value
 
     def merge(self, other):
         self._classes.merge_unique(other._classes)
diff --git a/reclass/datatypes/parameters.py b/reclass/datatypes/parameters.py
index fa0f379..1db35eb 100644
--- a/reclass/datatypes/parameters.py
+++ b/reclass/datatypes/parameters.py
@@ -12,11 +12,6 @@
 from __future__ import print_function
 from __future__ import unicode_literals
 
-#try:
-#    from types import StringTypes
-#except ImportError:
-#    StringTypes = (str, )
-
 import copy
 import sys
 import types
@@ -29,7 +24,9 @@
 from reclass.utils.parameterlist import ParameterList
 from reclass.values.value import Value
 from reclass.values.valuelist import ValueList
-from reclass.errors import InfiniteRecursionError, ResolveError, ResolveErrorList, InterpolationError, BadReferencesError
+from reclass.errors import InfiniteRecursionError, ResolveError
+from reclass.errors import ResolveErrorList, InterpolationError, ParseError
+from reclass.errors import BadReferencesError
 
 
 class Parameters(object):
@@ -61,10 +58,9 @@
         self._uri = uri
         self._base = ParameterDict(uri=self._uri)
         self._unrendered = None
-        self._escapes_handled = {}
         self._inv_queries = []
-        self._resolve_errors = ResolveErrorList()
-        self._needs_all_envs = False
+        self.resolve_errors = ResolveErrorList()
+        self.needs_all_envs = False
         self._parse_strings = parse_strings
         if mapping is not None:
             # initialise by merging
@@ -84,18 +80,13 @@
     def __ne__(self, other):
         return not self.__eq__(other)
 
+    @property
     def has_inv_query(self):
         return len(self._inv_queries) > 0
 
     def get_inv_queries(self):
         return self._inv_queries
 
-    def needs_all_envs(self):
-        return self._needs_all_envs
-
-    def resolve_errors(self):
-        return self._resolve_errors
-
     def as_dict(self):
         return self._base.copy()
 
@@ -108,7 +99,8 @@
             return self._wrap_list(value)
         else:
             try:
-                return Value(value, self._settings, self._uri, parse_string=self._parse_strings)
+                return Value(value, self._settings, self._uri,
+                             parse_string=self._parse_strings)
             except InterpolationError as e:
                 e.context = DictPath(self._settings.delimiter)
                 raise
@@ -154,7 +146,8 @@
                 uri = new.uri
             else:
                 uri = self._uri
-            values.append(Value(new, self._settings, uri, parse_string=self._parse_strings))
+            values.append(Value(new, self._settings, uri,
+                                parse_string=self._parse_strings))
 
         return values
 
@@ -246,37 +239,37 @@
         self._base = self._merge_recurse(self._base, wrapped)
 
     def _render_simple_container(self, container, key, value, path):
-            if isinstance(value, ValueList):
-                if value.is_complex():
-                    p = path.new_subpath(key)
-                    self._unrendered[p] = True
-                    container[key] = value
-                    if value.has_inv_query():
-                        self._inv_queries.append((p, value))
-                        if value.needs_all_envs():
-                            self._needs_all_envs = True
-                    return
-                else:
-                    value = value.merge()
-            if isinstance(value, Value) and value.is_container():
-                value = value.contents()
-            if isinstance(value, dict):
-                container[key] = self._render_simple_dict(value, path.new_subpath(key))
-            elif isinstance(value, list):
-                container[key] = self._render_simple_list(value, path.new_subpath(key))
-            elif isinstance(value, Value):
-                if value.is_complex():
-                    p = path.new_subpath(key)
-                    self._unrendered[p] = True
-                    container[key] = value
-                    if value.has_inv_query():
-                        self._inv_queries.append((p, value))
-                        if value.needs_all_envs():
-                            self._needs_all_envs = True
-                else:
-                    container[key] = value.render(None, None)
-            else:
+        if isinstance(value, ValueList):
+            if value.is_complex:
+                p = path.new_subpath(key)
+                self._unrendered[p] = True
                 container[key] = value
+                if value.has_inv_query:
+                    self._inv_queries.append((p, value))
+                    if value.needs_all_envs:
+                        self.needs_all_envs = True
+                return
+            else:
+                value = value.merge()
+        if isinstance(value, Value) and value.is_container():
+            value = value.contents
+        if isinstance(value, dict):
+            container[key] = self._render_simple_dict(value, path.new_subpath(key))
+        elif isinstance(value, list):
+            container[key] = self._render_simple_list(value, path.new_subpath(key))
+        elif isinstance(value, Value):
+            if value.is_complex:
+                p = path.new_subpath(key)
+                self._unrendered[p] = True
+                container[key] = value
+                if value.has_inv_query:
+                    self._inv_queries.append((p, value))
+                    if value.needs_all_envs:
+                        self.needs_all_envs = True
+            else:
+                container[key] = value.render(None, None)
+        else:
+            container[key] = value
 
     def _render_simple_dict(self, dictionary, path):
         new_dict = {}
@@ -298,8 +291,8 @@
             # processing them, so we cannot just iterate the dict
             path, v = next(iteritems(self._unrendered))
             self._interpolate_inner(path, inventory)
-        if self._resolve_errors.have_errors():
-            raise self._resolve_errors
+        if self.resolve_errors.have_errors():
+            raise self.resolve_errors
 
     def initialise_interpolation(self):
         self._unrendered = None
@@ -309,9 +302,10 @@
         if self._unrendered is None:
             self._unrendered = {}
             self._inv_queries = []
-            self._needs_all_envs = False
-            self._resolve_errors = ResolveErrorList()
-            self._base = self._render_simple_dict(self._base, DictPath(self._settings.delimiter))
+            self.needs_all_envs = False
+            self.resolve_errors = ResolveErrorList()
+            self._base = self._render_simple_dict(self._base,
+                 DictPath(self._settings.delimiter))
 
     def _interpolate_inner(self, path, inventory):
         value = path.get_value(self._base)
@@ -333,7 +327,7 @@
         except ResolveError as e:
             e.context = path
             if self._settings.group_errors:
-                self._resolve_errors.add(e)
+                self.resolve_errors.add(e)
                 new = None
             else:
                 raise
@@ -370,7 +364,7 @@
                         ancestor = ancestor.new_subpath(k)
                         if ancestor in self._unrendered:
                             self._interpolate_inner(ancestor, inventory)
-            if value.allRefs():
+            if value.allRefs:
                 all_refs = True
             else:
                 # not all references in the value could be calculated previously so
diff --git a/reclass/datatypes/tests/test_exports.py b/reclass/datatypes/tests/test_exports.py
index 7b37ca4..a0acce7 100644
--- a/reclass/datatypes/tests/test_exports.py
+++ b/reclass/datatypes/tests/test_exports.py
@@ -101,11 +101,16 @@
         self.assertEqual(p.as_dict(), r)
 
     def test_list_if_expr_invquery_with_and_missing(self):
-        e = {'node1': {'a': 1, 'b': 2, 'c': 'green'}, 'node2': {'a': 3, 'b': 3}, 'node3': {'a': 3, 'b': 2}}
-        p = Parameters({'exp': '$[ if exports:b == 2 and exports:c == green ]'}, SETTINGS, '')
-        r = {'exp': ['node1']}
-        p.interpolate(e)
-        self.assertEqual(p.as_dict(), r)
+        inventory = {'node1': {'a': 1, 'b': 2, 'c': 'green'},
+                     'node2': {'a': 3, 'b': 3},
+                     'node3': {'a': 3, 'b': 2}}
+        mapping = {'exp': '$[ if exports:b == 2 and exports:c == green ]'}
+        expected = {'exp': ['node1']}
+
+        pars = Parameters(mapping, SETTINGS, '')
+        pars.interpolate(inventory)
+
+        self.assertEqual(pars.as_dict(), expected)
 
     def test_list_if_expr_invquery_with_and(self):
         e = {'node1': {'a': 1, 'b': 2}, 'node2': {'a': 3, 'b': 3}, 'node3': {'a': 3, 'b': 4}}
diff --git a/reclass/errors.py b/reclass/errors.py
index 0c9d48f..330ad4c 100644
--- a/reclass/errors.py
+++ b/reclass/errors.py
@@ -359,3 +359,10 @@
               "definition in '{3}'. Nodes can only be defined once " \
               "per inventory."
         return msg.format(self._storage, self._name, self._uris[1], self._uris[0])
+
+
+class MissingModuleError(ReclassException):
+
+    def __init__(self, modname):
+        msg = "Module %s is missing" % modname
+        super(MissingModuleError, self).__init__(rc=posix.EX_DATAERR, msg=msg)
diff --git a/reclass/storage/yaml_git/__init__.py b/reclass/storage/yaml_git/__init__.py
index c26ef77..38de092 100644
--- a/reclass/storage/yaml_git/__init__.py
+++ b/reclass/storage/yaml_git/__init__.py
@@ -17,7 +17,13 @@
 import warnings
 with warnings.catch_warnings():
     warnings.simplefilter('ignore')
-    import pygit2
+    try:
+        # NOTE: in some distros pygit2 could require special effort to acquire.
+        # It is not a problem per se, but it breaks tests for no real reason.
+        # This try block is for keeping tests sane.
+        import pygit2
+    except ImportError:
+        pygit2 = None
 
 from six import iteritems
 
@@ -70,6 +76,8 @@
 class GitRepo(object):
 
     def __init__(self, uri):
+        if pygit2 is None:
+            raise errors.MissingModuleError('pygit2')
         self.transport, _, self.url = uri.repo.partition('://')
         self.name = self.url.replace('/', '_')
         self.credentials = None
diff --git a/reclass/utils/dictpath.py b/reclass/utils/dictpath.py
index 32831cf..70c7bb5 100644
--- a/reclass/utils/dictpath.py
+++ b/reclass/utils/dictpath.py
@@ -81,11 +81,12 @@
         return self._delim.join(str(i) for i in self._parts)
 
     def __eq__(self, other):
+        if not (isinstance(other, six.string_types) or
+                isinstance(other, self.__class__)):
+            return False
         if isinstance(other, six.string_types):
             other = DictPath(self._delim, other)
-
-        return self._parts == other._parts \
-                and self._delim == other._delim
+        return self._parts == other._parts and self._delim == other._delim
 
     def __ne__(self, other):
         return not self.__eq__(other)
@@ -93,9 +94,9 @@
     def __hash__(self):
         return hash(str(self))
 
-    def _get_path(self):
+    @property
+    def path(self):
         return self._parts
-    path = property(_get_path)
 
     def _get_key(self):
         if len(self._parts) == 0:
@@ -114,17 +115,8 @@
     def _split_string(self, string):
         return re.split(r'(?<!\\)' + re.escape(self._delim), string)
 
-    def _escape_string(self, string):
-        return string.replace(self._delim, '\\' + self._delim)
-
-    def has_ancestors(self):
-        return len(self._parts) > 1
-
     def key_parts(self):
-        if self.has_ancestors():
-            return self._parts[:-1]
-        else:
-            return []
+        return self._parts[:-1]
 
     def new_subpath(self, key):
         return DictPath(self._delim, self._parts + [key])
@@ -161,18 +153,17 @@
 
     def exists_in(self, container):
         item = container
-        for i in self._parts:
+        for part in self._parts:
             if isinstance(item, (dict, list)):
-                if i in item:
+                if part in item:
                     if isinstance(item, dict):
-                        item = item[i]
+                        item = item[part]
                     elif isinstance(container, list):
-                        item = item[int(i)]
+                        item = item[int(part)]
                 else:
                     return False
             else:
                 if item == self._parts[-1]:
                     return True
-                else:
-                    return False
+                return False
         return True
diff --git a/reclass/values/__init__.py b/reclass/values/__init__.py
index 9aaaf25..ec0f882 100644
--- a/reclass/values/__init__.py
+++ b/reclass/values/__init__.py
@@ -1,4 +1,4 @@
-# -*- coding: utf-8
+# -*- coding: utf-8 -*-
 from __future__ import absolute_import
 from __future__ import division
 from __future__ import print_function
diff --git a/reclass/values/compitem.py b/reclass/values/compitem.py
index 183bc43..c262f27 100644
--- a/reclass/values/compitem.py
+++ b/reclass/values/compitem.py
@@ -3,61 +3,28 @@
 #
 # This file is part of reclass
 #
-from __future__ import absolute_import
-from __future__ import division
-from __future__ import print_function
-from __future__ import unicode_literals
 
 from reclass.settings import Settings
-from .item import Item
+from reclass.values import item
 
-class CompItem(Item):
 
-    def __init__(self, items, settings):
-        self.type = Item.COMPOSITE
-        self._items = items
-        self._settings = settings
-        self._refs = []
-        self._allRefs = False
-        self.assembleRefs()
+class CompItem(item.ItemWithReferences):
 
-    def assembleRefs(self, context={}):
-        self._refs = []
-        self._allRefs = True
-        for item in self._items:
-            if item.has_references():
-                item.assembleRefs(context)
-                self._refs.extend(item.get_references())
-                if item.allRefs() is False:
-                    self._allRefs = False
+    type = item.ItemTypes.COMPOSITE
 
-    def contents(self):
-        return self._items
-
-    def allRefs(self):
-        return self._allRefs
-
-    def has_references(self):
-        return len(self._refs) > 0
-
-    def get_references(self):
-        return self._refs
-
-    def merge_over(self, item):
-        if item.type == Item.SCALAR or item.type == Item.COMPOSITE:
+    def merge_over(self, other):
+        if (other.type == item.ItemTypes.SCALAR or
+                other.type == item.ItemTypes.COMPOSITE):
             return self
-        raise RuntimeError('Trying to merge %s over %s' % (repr(self), repr(item)))
+        raise RuntimeError('Failed to merge %s over %s' % (self, other))
 
     def render(self, context, inventory):
         # Preserve type if only one item
-        if len(self._items) == 1:
-            return self._items[0].render(context, inventory)
+        if len(self.contents) == 1:
+            return self.contents[0].render(context, inventory)
         # Multiple items
-        strings = [ str(i.render(context, inventory)) for i in self._items ]
+        strings = [str(i.render(context, inventory)) for i in self.contents]
         return "".join(strings)
 
-    def __repr__(self):
-        return 'CompItem(%r)' % self._items
-
     def __str__(self):
-        return ''.join([ str(i) for i in self._items ])
+        return ''.join([str(i) for i in self.contents])
diff --git a/reclass/values/dictitem.py b/reclass/values/dictitem.py
index d5272b9..0648a39 100644
--- a/reclass/values/dictitem.py
+++ b/reclass/values/dictitem.py
@@ -3,29 +3,10 @@
 #
 # This file is part of reclass
 #
-from __future__ import absolute_import
-from __future__ import division
-from __future__ import print_function
-from __future__ import unicode_literals
 
-from reclass.settings import Settings
-from .item import Item
+from reclass.values import item
 
-class DictItem(Item):
 
-    def __init__(self, item, settings):
-        self.type = Item.DICTIONARY
-        self._dict = item
-        self._settings = settings
+class DictItem(item.ContainerItem):
 
-    def contents(self):
-        return self._dict
-
-    def is_container(self):
-        return True
-
-    def render(self, context, inventory):
-        return self._dict
-
-    def __repr__(self):
-        return 'DictItem(%r)' % self._dict
+    type = item.ItemTypes.DICTIONARY
diff --git a/reclass/values/invitem.py b/reclass/values/invitem.py
index 37a35cf..0179f4f 100644
--- a/reclass/values/invitem.py
+++ b/reclass/values/invitem.py
@@ -9,83 +9,72 @@
 from __future__ import unicode_literals
 
 import copy
+import itertools as it
+import operator
 import pyparsing as pp
 
-from six import iteritems, string_types
+from six import iteritems
+from six import string_types
 
-from .item import Item
+from reclass.values import item
+from reclass.values import parser_funcs
 from reclass.settings import Settings
 from reclass.utils.dictpath import DictPath
 from reclass.errors import ExpressionError, ParseError, ResolveError
 
-_OBJ = 'OBJ'
-_TEST = 'TEST'
-_LIST_TEST = 'LIST_TEST'
-_LOGICAL = 'LOGICAL'
-_OPTION = 'OPTION'
 
-_VALUE = 'VALUE'
-_IF = 'IF'
-_AND = 'AND'
-_OR = 'OR'
+# TODO: generalize expression handling.
+class BaseTestExpression(object):
 
-_EQUAL = '=='
-_NOT_EQUAL = '!='
+    known_operators = {}
+    def __init__(self, delimiter):
+        self._delimiter = delimiter
+        self.refs = []
+        self.inv_refs = []
 
-_IGNORE_ERRORS = '+IgnoreErrors'
-_ALL_ENVS = '+AllEnvs'
 
-class Element(object):
+class EqualityTest(BaseTestExpression):
+
+    known_operators = { parser_funcs.EQUAL: operator.eq,
+                        parser_funcs.NOT_EQUAL: operator.ne}
 
     def __init__(self, expression, delimiter):
-        self._delimiter = delimiter
-        self._export_path = None
-        self._parameter_path = None
-        self._parameter_value = None
-        self._export_path, self._parameter_path, self._parameter_value = self._get_vars(expression[0][1], self._export_path, self._parameter_path, self._parameter_value)
-        self._export_path, self._parameter_path, self._parameter_value = self._get_vars(expression[2][1], self._export_path, self._parameter_path, self._parameter_value)
-
+        # expression is a list of at least three tuples, of which first element
+        # is a string tag, second is subelement value; other tuples apparently
+        # are not used.
+        # expression[0][1] effectively contains export path and apparently must
+        # be treated as such, also left hand operand in comparison
+        # expression[1][1] appa holds commparison operator == or !=
+        # expression[2][1] is the righhand operand
+        super(EqualityTest, self).__init__(delimiter)
+        # TODO: this double sommersault must be cleaned
+        _ = self._get_vars(expression[2][1], *self._get_vars(expression[0][1]))
+        self._export_path, self._parameter_path, self._parameter_value = _
         try:
             self._export_path.drop_first()
         except AttributeError:
             raise ExpressionError('No export')
-
-        self._inv_refs = [ self._export_path ]
-        self._test = expression[1][1]
-
+        try:
+            self._compare = self.known_operators[expression[1][1]]
+        except KeyError as e:
+            msg = 'Unknown test {0}'.format(expression[1][1])
+            raise ExpressionError(msg, tbFlag=False)
+        self.inv_refs = [self._export_path]
         if self._parameter_path is not None:
             self._parameter_path.drop_first()
-            self._refs = [ str(self._parameter_path) ]
-        else:
-            self._refs = []
-
-    def refs(self):
-        return self._refs
-
-    def inv_refs(self):
-        return self._inv_refs
+            self.refs = [str(self._parameter_path)]
 
     def value(self, context, items):
         if self._parameter_path is not None:
-            self._parameter_value = self._resolve(self._parameter_path, context)
-
-        if self._parameter_value is None or self._test is None:
-            raise ExpressionError('Failed to render %s' % str(self), tbFlag=False)
-
+            self._parameter_value = self._resolve(self._parameter_path,
+                                                  context)
+        if self._parameter_value is None:
+            raise ExpressionError('Failed to render %s' % str(self),
+                                  tbFlag=False)
         if self._export_path.exists_in(items):
-            result = False
             export_value = self._resolve(self._export_path, items)
-            if self._test == _EQUAL:
-                if export_value == self._parameter_value:
-                    result = True
-            elif self._test == _NOT_EQUAL:
-                if export_value != self._parameter_value:
-                    result = True
-            else:
-                raise ExpressionError('Unknown test {0}'.format(self._test), tbFlag=False)
-            return result
-        else:
-            return False
+            return self._compare(export_value, self._parameter_value)
+        return False
 
     def _resolve(self, path, dictionary):
         try:
@@ -93,7 +82,7 @@
         except KeyError as e:
             raise ResolveError(str(path))
 
-    def _get_vars(self, var, export, parameter, value):
+    def _get_vars(self, var, export=None, parameter=None, value=None):
         if isinstance(var, string_types):
             path = DictPath(self._delimiter, var)
             if path.path[0].lower() == 'exports':
@@ -111,193 +100,96 @@
         return export, parameter, value
 
 
-class Question(object):
+class LogicTest(BaseTestExpression):
 
-    def __init__(self, expression, delimiter):
-        self._elements = []
-        self._operators = []
-        self._delimiter = delimiter
-        self._refs = []
-        self._inv_refs = []
-        i = 0
-        while i < len(expression):
-            e = Element(expression[i:], self._delimiter)
-            self._elements.append(e)
-            self._refs.extend(e.refs())
-            self._inv_refs.extend(e.inv_refs())
-            i += 3
-            if i < len(expression):
-                self._operators.append(expression[i][1])
-                i += 1
+    known_operators = { parser_funcs.AND: operator.and_,
+                        parser_funcs.OR: operator.or_}
 
-    def refs(self):
-        return self._refs
-
-    def inv_refs(self):
-        return self._inv_refs
+    def __init__(self, expr, delimiter):
+        super(LogicTest, self).__init__(delimiter)
+        subtests = list(it.compress(expr, it.cycle([1, 1, 1, 0])))
+        self._els = [EqualityTest(subtests[j:j+3], self._delimiter)
+                     for j in range(0, len(subtests), 3)]
+        self.refs = [x.refs for x in self._els]
+        self.inv_refs = [x.inv_refs for x in self._els]
+        try:
+            self._ops = [self.known_operators[x[1]] for x in expr[3::4]]
+        except KeyError as e:
+            msg = 'Unknown operator {0} {1}'.format(e.messsage, self._els)
+            raise ExpressionError(msg, tbFlag=False)
 
     def value(self, context, items):
-        if len(self._elements) == 0:
+        if len(self._els) == 0:  # NOTE: possible logic error
             return True
-        elif len(self._elements) == 1:
-            return self._elements[0].value(context, items)
-        else:
-            result = self._elements[0].value(context, items)
-            for i in range(0, len(self._elements)-1):
-                next_result = self._elements[i+1].value(context, items)
-                if self._operators[i] == _AND:
-                    result = result and next_result
-                elif self._operators[i] == _OR:
-                    result = result or next_result
-                else:
-                    raise ExpressionError('Unknown operator {0} {1}'.format(self._operators[i], self.elements), tbFlag=False)
-            return result
+        result = self._els[0].value(context, items)
+        for op, next_el in zip(self._ops, self._els[1:]):
+            result = op(result, next_el.value(context, items))
+        return result
 
 
-class InvItem(Item):
+class InvItem(item.Item):
 
-    def _get_parser():
+    type = item.ItemTypes.INV_QUERY
 
-        def _object(string, location, tokens):
-            token = tokens[0]
-            tokens[0] = (_OBJ, token)
-
-        def _integer(string, location, tokens):
-            try:
-                token = int(tokens[0])
-            except ValueError:
-                token = tokens[0]
-            tokens[0] = (_OBJ, token)
-
-        def _number(string, location, tokens):
-            try:
-                token = float(tokens[0])
-            except ValueError:
-                token = tokens[0]
-            tokens[0] = (_OBJ, token)
-
-        def _option(string, location, tokens):
-            token = tokens[0]
-            tokens[0] = (_OPTION, token)
-
-        def _test(string, location, tokens):
-            token = tokens[0]
-            tokens[0] = (_TEST, token)
-
-        def _logical(string, location, tokens):
-            token = tokens[0]
-            tokens[0] = (_LOGICAL, token)
-
-        def _if(string, location, tokens):
-            token = tokens[0]
-            tokens[0] = (_IF, token)
-
-        def _expr_var(string, location, tokens):
-            token = tokens[0]
-            tokens[0] = (_VALUE, token)
-
-        def _expr_test(string, location, tokens):
-            token = tokens[0]
-            tokens[0] = (_TEST, token)
-
-        def _expr_list_test(string, location, tokens):
-            token = tokens[0]
-            tokens[0] = (_LIST_TEST, token)
-
-        white_space = pp.White().suppress()
-        end = pp.StringEnd()
-        ignore_errors = pp.CaselessLiteral(_IGNORE_ERRORS)
-        all_envs = pp.CaselessLiteral(_ALL_ENVS)
-        option = (ignore_errors | all_envs).setParseAction(_option)
-        options = pp.Group(pp.ZeroOrMore(option + white_space))
-        operator_test = (pp.Literal(_EQUAL) | pp.Literal(_NOT_EQUAL)).setParseAction(_test)
-        operator_logical = (pp.CaselessLiteral(_AND) | pp.CaselessLiteral(_OR)).setParseAction(_logical)
-        begin_if = pp.CaselessLiteral(_IF, ).setParseAction(_if)
-        obj = pp.Word(pp.printables).setParseAction(_object)
-        integer = pp.Word('0123456789-').setParseAction(_integer)
-        number = pp.Word('0123456789-.').setParseAction(_number)
-        item = integer | number | obj
-        single_test = white_space + item + white_space + operator_test + white_space + item
-        additional_test = white_space + operator_logical + single_test
-        expr_var = pp.Group(obj + pp.Optional(white_space) + end).setParseAction(_expr_var)
-        expr_test = pp.Group(obj + white_space + begin_if + single_test + pp.ZeroOrMore(additional_test) + end).setParseAction(_expr_test)
-        expr_list_test = pp.Group(begin_if + single_test + pp.ZeroOrMore(additional_test) + end).setParseAction(_expr_list_test)
-        expr = (expr_test | expr_var | expr_list_test)
-        line = options + expr + end
-        return line
-
-    _parser = _get_parser()
-
-    def __init__(self, item, settings):
-        self.type = Item.INV_QUERY
-        self._settings = settings
-        self._needs_all_envs = False
-        self._ignore_failed_render = self._settings.inventory_ignore_failed_render
-        self._expr_text = item.render(None, None)
-        self._parse_expression(self._expr_text)
+    def __init__(self, newitem, settings):
+        super(InvItem, self).__init__(newitem.render(None, None), settings)
+        self.needs_all_envs = False
+        self.ignore_failed_render = (
+                self._settings.inventory_ignore_failed_render)
+        self._parse_expression(self.contents)
 
     def _parse_expression(self, expr):
+        parser = parser_funcs.get_expression_parser()
         try:
-            tokens = InvItem._parser.parseString(expr).asList()
+            tokens = parser.parseString(expr).asList()
         except pp.ParseException as e:
             raise ParseError(e.msg, e.line, e.col, e.lineno)
 
-        if len(tokens) == 1:
-            self._expr_type = tokens[0][0]
-            self._expr = list(tokens[0][1])
-        elif len(tokens) == 2:
-            for opt in tokens[0]:
-                if opt[1] == _IGNORE_ERRORS:
-                    self._ignore_failed_render = True
-                elif opt[1] == _ALL_ENVS:
-                    self._needs_all_envs = True
-            self._expr_type = tokens[1][0]
-            self._expr = list(tokens[1][1])
-        else:
-            raise ExpressionError('Failed to parse %s' % str(tokens), tbFlag=False)
+        if len(tokens) == 2:  # options are set
+            passed_opts = [x[1] for x in tokens.pop(0)]
+            self.ignore_failed_render = parser_funcs.IGNORE_ERRORS in passed_opts
+            self.needs_all_envs = parser_funcs.ALL_ENVS in passed_opts
+        elif len(tokens) > 2:
+            raise ExpressionError('Failed to parse %s' % str(tokens),
+                                  tbFlag=False)
+        self._expr_type = tokens[0][0]
+        self._expr = list(tokens[0][1])
 
-        if self._expr_type == _VALUE:
-            self._value_path = DictPath(self._settings.delimiter, self._expr[0][1]).drop_first()
-            self._question = Question([], self._settings.delimiter)
-            self._refs = []
-            self._inv_refs = [ self._value_path ]
-        elif self._expr_type == _TEST:
-            self._value_path = DictPath(self._settings.delimiter, self._expr[0][1]).drop_first()
-            self._question = Question(self._expr[2:], self._settings.delimiter)
-            self._refs = self._question.refs()
-            self._inv_refs = self._question.inv_refs()
-            self._inv_refs.append(self._value_path)
-        elif self._expr_type == _LIST_TEST:
+        if self._expr_type == parser_funcs.VALUE:
+            self._value_path = DictPath(self._settings.delimiter,
+                                        self._expr[0][1]).drop_first()
+            self._question = LogicTest([], self._settings.delimiter)
+            self.refs = []
+            self.inv_refs = [self._value_path]
+        elif self._expr_type == parser_funcs.TEST:
+            self._value_path = DictPath(self._settings.delimiter,
+                                        self._expr[0][1]).drop_first()
+            self._question = LogicTest(self._expr[2:], self._settings.delimiter)
+            self.refs = self._question.refs
+            self.inv_refs = self._question.inv_refs
+            self.inv_refs.append(self._value_path)
+        elif self._expr_type == parser_funcs.LIST_TEST:
             self._value_path = None
-            self._question = Question(self._expr[1:], self._settings.delimiter)
-            self._refs = self._question.refs()
-            self._inv_refs = self._question.inv_refs()
+            self._question = LogicTest(self._expr[1:], self._settings.delimiter)
+            self.refs = self._question.refs
+            self.inv_refs = self._question.inv_refs
         else:
-            raise ExpressionError('Unknown expression type: %s' % self._expr_type, tbFlag=False)
+            msg = 'Unknown expression type: %s'
+            raise ExpressionError(msg % self._expr_type, tbFlag=False)
 
-    def assembleRefs(self, context):
-        return
-
-    def contents(self):
-        return self._expr_text
-
+    @property
     def has_inv_query(self):
         return True
 
+    @property
     def has_references(self):
-        return len(self._question.refs()) > 0
+        return len(self._question.refs) > 0
 
     def get_references(self):
-        return self._question.refs()
+        return self._question.refs
 
     def get_inv_references(self):
-        return self._inv_refs
-
-    def needs_all_envs(self):
-        return self._needs_all_envs
-
-    def ignore_failed_render(self):
-        return self._ignore_failed_render
+        return self.inv_refs
 
     def _resolve(self, path, dictionary):
         try:
@@ -309,17 +201,21 @@
         results = {}
         for (node, items) in iteritems(inventory):
             if self._value_path.exists_in(items):
-                results[node] = copy.deepcopy(self._resolve(self._value_path, items))
+                results[node] = copy.deepcopy(self._resolve(self._value_path,
+                                              items))
         return results
 
     def _test_expression(self, context, inventory):
         if self._value_path is None:
-            ExpressionError('Failed to render %s' % str(self), tbFlag=False)
+            msg = 'Failed to render %s'
+            raise ExpressionError(msg % str(self), tbFlag=False)
 
         results = {}
-        for (node, items) in iteritems(inventory):
-            if self._question.value(context, items) and self._value_path.exists_in(items):
-                results[node] = copy.deepcopy(self._resolve(self._value_path, items))
+        for node, items in iteritems(inventory):
+            if (self._question.value(context, items) and
+                    self._value_path.exists_in(items)):
+                results[node] = copy.deepcopy(
+                    self._resolve(self._value_path, items))
         return results
 
     def _list_test_expression(self, context, inventory):
@@ -330,11 +226,11 @@
         return results
 
     def render(self, context, inventory):
-        if self._expr_type == _VALUE:
+        if self._expr_type == parser_funcs.VALUE:
             return self._value_expression(inventory)
-        elif self._expr_type == _TEST:
+        elif self._expr_type == parser_funcs.TEST:
             return self._test_expression(context, inventory)
-        elif self._expr_type == _LIST_TEST:
+        elif self._expr_type == parser_funcs.LIST_TEST:
             return self._list_test_expression(context, inventory)
         raise ExpressionError('Failed to render %s' % str(self), tbFlag=False)
 
@@ -342,4 +238,5 @@
         return ' '.join(str(j) for i,j in self._expr)
 
     def __repr__(self):
+        # had to leave it here for now as the behaviour differs from basic
         return 'InvItem(%r)' % self._expr
diff --git a/reclass/values/item.py b/reclass/values/item.py
index cad3684..ee46995 100644
--- a/reclass/values/item.py
+++ b/reclass/values/item.py
@@ -8,39 +8,38 @@
 from __future__ import print_function
 from __future__ import unicode_literals
 
+from enum import Enum
+
 from reclass.utils.dictpath import DictPath
 
+ItemTypes = Enum('ItemTypes',
+                 ['COMPOSITE', 'DICTIONARY', 'INV_QUERY', 'LIST',
+                  'REFERENCE', 'SCALAR'])
+
+
 class Item(object):
 
-    COMPOSITE = 1
-    DICTIONARY = 2
-    INV_QUERY = 3
-    LIST = 4
-    REFERENCE = 5
-    SCALAR = 6
-
-    TYPE_STR = { COMPOSITE: 'composite', DICTIONARY: 'dictionary',
-                 INV_QUERY: 'invventory query', LIST: 'list',
-                 REFERENCE: 'reference', SCALAR: 'scalar' }
+    def __init__(self, item, settings):
+        self._settings = settings
+        self.contents = item
 
     def allRefs(self):
         return True
 
+    @property
     def has_references(self):
         return False
 
+    @property
     def has_inv_query(self):
         return False
 
     def is_container(self):
         return False
 
+    @property
     def is_complex(self):
-        return (self.has_references() | self.has_inv_query())
-
-    def contents(self):
-        msg = "Item class {0} does not implement contents()"
-        raise NotImplementedError(msg.format(self.__class__.__name__))
+        return (self.has_references | self.has_inv_query)
 
     def merge_over(self, item):
         msg = "Item class {0} does not implement merge_over()"
@@ -51,4 +50,41 @@
         raise NotImplementedError(msg.format(self.__class__.__name__))
 
     def type_str(self):
-        return self.TYPE_STR[self.type]
+        return self.type.name.lower()
+
+    def __repr__(self):
+        return '%s(%r)' % (self.__class__.__name__, self.contents)
+
+
+class ItemWithReferences(Item):
+
+    def __init__(self, items, settings):
+        super(ItemWithReferences, self).__init__(items, settings)
+        self.assembleRefs()
+
+    @property
+    def has_references(self):
+        return len(self._refs) > 0
+
+    def get_references(self):
+        return self._refs
+
+    # NOTE: possibility of confusion. Looks like 'assemble' should be either
+    # 'gather' or 'extract'.
+    def assembleRefs(self, context={}):
+        self._refs = []
+        self.allRefs = True
+        for item in self.contents:
+            if item.has_references:
+                item.assembleRefs(context)
+                self._refs.extend(item.get_references())
+                if item.allRefs is False:
+                    self.allRefs = False
+
+class ContainerItem(Item):
+
+    def is_container(self):
+        return True
+
+    def render(self, context, inventory):
+        return self.contents
diff --git a/reclass/values/listitem.py b/reclass/values/listitem.py
index 41c02dd..24bece1 100644
--- a/reclass/values/listitem.py
+++ b/reclass/values/listitem.py
@@ -3,35 +3,16 @@
 #
 # This file is part of reclass
 #
-from __future__ import absolute_import
-from __future__ import division
-from __future__ import print_function
-from __future__ import unicode_literals
 
-from .item import Item
-from reclass.settings import Settings
+from reclass.values import item
 
-class ListItem(Item):
 
-    def __init__(self, item, settings):
-        self.type = Item.LIST
-        self._list = item
-        self._settings = settings
+class ListItem(item.ContainerItem):
 
-    def contents(self):
-        return self._list
+    type = item.ItemTypes.LIST
 
-    def is_container(self):
-        return True
-
-    def render(self, context, inventory):
-        return self._list
-
-    def merge_over(self, item):
-        if item.type == Item.LIST:
-            item._list.extend(self._list)
-            return item
-        raise RuntimeError('Trying to merge %s over %s' % (repr(self), repr(item)))
-
-    def __repr__(self):
-        return 'ListItem(%r)' % (self._list)
+    def merge_over(self, other):
+        if other.type == item.ItemTypes.LIST:
+            other.contents.extend(self.contents)
+            return other
+        raise RuntimeError('Failed to merge %s over %s'  % (self, other))
diff --git a/reclass/values/parser_funcs.py b/reclass/values/parser_funcs.py
index 46db7cc..50babd0 100644
--- a/reclass/values/parser_funcs.py
+++ b/reclass/values/parser_funcs.py
@@ -9,22 +9,76 @@
 from __future__ import unicode_literals
 
 import pyparsing as pp
+import functools
 
 STR = 1
 REF = 2
 INV = 3
 
-def _string(string, location, tokens):
-    token = tokens[0]
-    tokens[0] = (STR, token)
+_OBJ = 'OBJ'
+_LOGICAL = 'LOGICAL'
+_OPTION = 'OPTION'
+_IF = 'IF'
 
-def _reference(string, location, tokens):
-    token = list(tokens[0])
-    tokens[0] = (REF, token)
+TEST = 'TEST'
+LIST_TEST = 'LIST_TEST'
 
-def _invquery(string, location, tokens):
-    token = list(tokens[0])
-    tokens[0] = (INV, token)
+VALUE = 'VALUE'
+AND = 'AND'
+OR = 'OR'
+
+EQUAL = '=='
+NOT_EQUAL = '!='
+
+IGNORE_ERRORS = '+IgnoreErrors'
+ALL_ENVS = '+AllEnvs'
+
+
+def _tag_with(tag, transform=lambda x:x):
+    def inner(tag, string, location, tokens):
+        token = transform(tokens[0])
+        tokens[0] = (tag, token)
+    return functools.partial(inner, tag)
+
+def get_expression_parser():
+
+    s_end = pp.StringEnd()
+    sign = pp.Optional(pp.Literal('-'))
+    number = pp.Word(pp.nums)
+    dpoint = pp.Literal('.')
+    ignore_errors = pp.CaselessLiteral(IGNORE_ERRORS)
+    all_envs = pp.CaselessLiteral(ALL_ENVS)
+    eq, neq = pp.Literal(EQUAL), pp.Literal(NOT_EQUAL)
+    eand, eor = pp.CaselessLiteral(AND), pp.CaselessLiteral(OR)
+
+    option = (ignore_errors | all_envs).setParseAction(_tag_with(_OPTION))
+    options = pp.Group(pp.ZeroOrMore(option))
+    operator_test = (eq | neq).setParseAction(_tag_with(TEST))
+    operator_logical = (eand | eor).setParseAction(_tag_with(_LOGICAL))
+    begin_if = pp.CaselessLiteral(_IF).setParseAction(_tag_with(_IF))
+    obj = pp.Word(pp.printables).setParseAction(_tag_with(_OBJ))
+
+    integer = pp.Combine(sign + number + pp.WordEnd()).setParseAction(
+            _tag_with(_OBJ, int))
+    real = pp.Combine(sign +
+                      ((number + dpoint + number) |
+                       (dpoint + number) |
+                       (number + dpoint))
+                     ).setParseAction(_tag_with(_OBJ, float))
+    expritem = integer | real | obj
+    single_test = expritem + operator_test + expritem
+    additional_test = operator_logical + single_test
+
+    expr_var = pp.Group(obj + s_end).setParseAction(_tag_with(VALUE))
+    expr_test = pp.Group(obj + begin_if + single_test +
+                         pp.ZeroOrMore(additional_test) +
+                         s_end).setParseAction(_tag_with(TEST))
+    expr_list_test = pp.Group(begin_if + single_test +
+                              pp.ZeroOrMore(additional_test) +
+                              s_end).setParseAction(_tag_with(LIST_TEST))
+    expr = expr_test | expr_var | expr_list_test
+    line = options + expr + s_end
+    return line
 
 def get_ref_parser(escape_character, reference_sentinels, export_sentinels):
     _ESCAPE = escape_character
@@ -50,8 +104,12 @@
 
     _EXCLUDES = _ESCAPE + _REF_OPEN + _REF_CLOSE + _INV_OPEN + _INV_CLOSE
 
-    double_escape = pp.Combine(pp.Literal(_DOUBLE_ESCAPE) + pp.MatchFirst([pp.FollowedBy(_REF_OPEN), pp.FollowedBy(_REF_CLOSE),
-                               pp.FollowedBy(_INV_OPEN), pp.FollowedBy(_INV_CLOSE)])).setParseAction(pp.replaceWith(_ESCAPE))
+    double_escape = pp.Combine(pp.Literal(_DOUBLE_ESCAPE) +
+        pp.MatchFirst([pp.FollowedBy(_REF_OPEN),
+                       pp.FollowedBy(_REF_CLOSE),
+                       pp.FollowedBy(_INV_OPEN),
+                       pp.FollowedBy(_INV_CLOSE)])).setParseAction(
+                               pp.replaceWith(_ESCAPE))
 
     ref_open = pp.Literal(_REF_OPEN).suppress()
     ref_close = pp.Literal(_REF_CLOSE).suppress()
@@ -61,10 +119,10 @@
     ref_escape_close = pp.Literal(_REF_ESCAPE_CLOSE).setParseAction(pp.replaceWith(_REF_CLOSE))
     ref_text = pp.CharsNotIn(_REF_EXCLUDES) | pp.CharsNotIn(_REF_CLOSE_FIRST, exact=1)
     ref_content = pp.Combine(pp.OneOrMore(ref_not_open + ref_not_close + ref_text))
-    ref_string = pp.MatchFirst([double_escape, ref_escape_open, ref_escape_close, ref_content]).setParseAction(_string)
+    ref_string = pp.MatchFirst([double_escape, ref_escape_open, ref_escape_close, ref_content]).setParseAction(_tag_with(STR))
     ref_item = pp.Forward()
     ref_items = pp.OneOrMore(ref_item)
-    reference = (ref_open + pp.Group(ref_items) + ref_close).setParseAction(_reference)
+    reference = (ref_open + pp.Group(ref_items) + ref_close).setParseAction(_tag_with(REF))
     ref_item << (reference | ref_string)
 
     inv_open = pp.Literal(_INV_OPEN).suppress()
@@ -75,13 +133,13 @@
     inv_escape_close = pp.Literal(_INV_ESCAPE_CLOSE).setParseAction(pp.replaceWith(_INV_CLOSE))
     inv_text = pp.CharsNotIn(_INV_CLOSE_FIRST)
     inv_content = pp.Combine(pp.OneOrMore(inv_not_close + inv_text))
-    inv_string = pp.MatchFirst([double_escape, inv_escape_open, inv_escape_close, inv_content]).setParseAction(_string)
+    inv_string = pp.MatchFirst([double_escape, inv_escape_open, inv_escape_close, inv_content]).setParseAction(_tag_with(STR))
     inv_items = pp.OneOrMore(inv_string)
-    export = (inv_open + pp.Group(inv_items) + inv_close).setParseAction(_invquery)
+    export = (inv_open + pp.Group(inv_items) + inv_close).setParseAction(_tag_with(INV))
 
     text = pp.CharsNotIn(_EXCLUDES) | pp.CharsNotIn('', exact=1)
     content = pp.Combine(pp.OneOrMore(ref_not_open + inv_not_open + text))
-    string = pp.MatchFirst([double_escape, ref_escape_open, inv_escape_open, content]).setParseAction(_string)
+    string = pp.MatchFirst([double_escape, ref_escape_open, inv_escape_open, content]).setParseAction(_tag_with(STR))
 
     item = reference | export | string
     line = pp.OneOrMore(item) + pp.StringEnd()
@@ -95,9 +153,9 @@
     _INV_CLOSE = export_sentinels[1]
     _EXCLUDES = _ESCAPE + _REF_OPEN + _REF_CLOSE + _INV_OPEN + _INV_CLOSE
 
-    string = pp.CharsNotIn(_EXCLUDES).setParseAction(_string)
+    string = pp.CharsNotIn(_EXCLUDES).setParseAction(_tag_with(STR))
     ref_open = pp.Literal(_REF_OPEN).suppress()
     ref_close = pp.Literal(_REF_CLOSE).suppress()
-    reference = (ref_open + pp.Group(string) + ref_close).setParseAction(_reference)
+    reference = (ref_open + pp.Group(string) + ref_close).setParseAction(_tag_with(REF))
     line = pp.StringStart() + pp.Optional(string) + reference + pp.Optional(string) + pp.StringEnd()
     return line
diff --git a/reclass/values/refitem.py b/reclass/values/refitem.py
index d24eeee..df713e1 100644
--- a/reclass/values/refitem.py
+++ b/reclass/values/refitem.py
@@ -3,55 +3,24 @@
 #
 # This file is part of reclass
 #
-from __future__ import absolute_import
-from __future__ import division
-from __future__ import print_function
-from __future__ import unicode_literals
 
-from .item import Item
-from reclass.defaults import REFERENCE_SENTINELS
-from reclass.settings import Settings
+from reclass.values import item
 from reclass.utils.dictpath import DictPath
 from reclass.errors import ResolveError
 
 
-class RefItem(Item):
+class RefItem(item.ItemWithReferences):
 
-    def __init__(self, items, settings):
-        self.type = Item.REFERENCE
-        self._settings = settings
-        self._items = items
-        self._refs = []
-        self._allRefs = False
-        self.assembleRefs()
+    type = item.ItemTypes.REFERENCE
 
     def assembleRefs(self, context={}):
-        self._refs = []
-        self._allRefs = True
-        for item in self._items:
-            if item.has_references():
-                item.assembleRefs(context)
-                self._refs.extend(item.get_references())
-                if item.allRefs() == False:
-                    self._allRefs = False
+        super(RefItem, self).assembleRefs(context)
         try:
-            strings = [ str(i.render(context, None)) for i in self._items ]
+            strings = [str(i.render(context, None)) for i in self.contents]
             value = "".join(strings)
             self._refs.append(value)
         except ResolveError as e:
-            self._allRefs = False
-
-    def contents(self):
-        return self._items
-
-    def allRefs(self):
-        return self._allRefs
-
-    def has_references(self):
-        return len(self._refs) > 0
-
-    def get_references(self):
-        return self._refs
+            self.allRefs = False
 
     def _resolve(self, ref, context):
         path = DictPath(self._settings.delimiter, ref)
@@ -61,14 +30,13 @@
             raise ResolveError(ref)
 
     def render(self, context, inventory):
-        if len(self._items) == 1:
-            return self._resolve(self._items[0].render(context, inventory), context)
-        strings = [ str(i.render(context, inventory)) for i in self._items ]
+        if len(self.contents) == 1:
+            return self._resolve(self.contents[0].render(context, inventory),
+                                 context)
+        strings = [str(i.render(context, inventory)) for i in self.contents]
         return self._resolve("".join(strings), context)
 
-    def __repr__(self):
-        return 'RefItem(%r)' % self._items
-
     def __str__(self):
-        strings = [ str(i) for i in self._items ]
-        return '{0}{1}{2}'.format(REFERENCE_SENTINELS[0], ''.join(strings), REFERENCE_SENTINELS[1])
+        strings = [str(i) for i in self.contents]
+        rs = self._settings.reference_sentinels
+        return '{0}{1}{2}'.format(rs[0], ''.join(strings), rs[1])
diff --git a/reclass/values/scaitem.py b/reclass/values/scaitem.py
index c16ab45..1bcbd2c 100644
--- a/reclass/values/scaitem.py
+++ b/reclass/values/scaitem.py
@@ -9,28 +9,23 @@
 from __future__ import unicode_literals
 
 from reclass.settings import Settings
-from .item import Item
+from reclass.values import item
 
-class ScaItem(Item):
+
+class ScaItem(item.Item):
+
+    type = item.ItemTypes.SCALAR
 
     def __init__(self, value, settings):
-        self.type = Item.SCALAR
-        self._value = value
-        self._settings = settings
+        super(ScaItem, self).__init__(value, settings)
 
-    def contents(self):
-        return self._value
-
-    def merge_over(self, item):
-        if item.type == Item.SCALAR or item.type == Item.COMPOSITE:
+    def merge_over(self, other):
+        if other.type in [item.ItemTypes.SCALAR, item.ItemTypes.COMPOSITE]:
             return self
-        raise RuntimeError('Trying to merge %s over %s' % (repr(self), repr(item)))
+        raise RuntimeError('Failed to merge %s over %s' % (self, other))
 
     def render(self, context, inventory):
-        return self._value
-
-    def __repr__(self):
-        return 'ScaItem({0!r})'.format(self._value)
+        return self.contents
 
     def __str__(self):
-        return str(self._value)
+        return str(self.contents)
diff --git a/reclass/values/tests/__init__.py b/reclass/values/tests/__init__.py
index 16d1248..e69de29 100644
--- a/reclass/values/tests/__init__.py
+++ b/reclass/values/tests/__init__.py
@@ -1,7 +0,0 @@
-#
-# -*- coding: utf-8
-#
-from __future__ import absolute_import
-from __future__ import division
-from __future__ import print_function
-from __future__ import unicode_literals
diff --git a/reclass/values/tests/test_compitem.py b/reclass/values/tests/test_compitem.py
new file mode 100644
index 0000000..71a6f0e
--- /dev/null
+++ b/reclass/values/tests/test_compitem.py
@@ -0,0 +1,132 @@
+from reclass.settings import Settings
+from reclass.values.value import Value
+from reclass.values.compitem import CompItem
+from reclass.values.scaitem import ScaItem
+from reclass.values.valuelist import ValueList
+from reclass.values.listitem import ListItem
+from reclass.values.dictitem import DictItem
+import unittest
+
+SETTINGS = Settings()
+
+class TestCompItem(unittest.TestCase):
+
+    def test_assembleRefs_no_items(self):
+        composite = CompItem([], SETTINGS)
+
+        self.assertFalse(composite.has_references)
+
+    def test_assembleRefs_one_item_without_refs(self):
+        val1 = Value('foo',  SETTINGS, '')
+
+        composite = CompItem([val1], SETTINGS)
+
+        self.assertFalse(composite.has_references)
+
+    def test_assembleRefs_one_item_with_one_ref(self):
+        val1 = Value('${foo}',  SETTINGS, '')
+        expected_refs = ['foo']
+
+        composite = CompItem([val1], SETTINGS)
+
+        self.assertTrue(composite.has_references)
+        self.assertEquals(composite.get_references(), expected_refs)
+
+    def test_assembleRefs_one_item_with_two_refs(self):
+        val1 = Value('${foo}${bar}',  SETTINGS, '')
+        expected_refs = ['foo', 'bar']
+
+        composite = CompItem([val1], SETTINGS)
+
+        self.assertTrue(composite.has_references)
+        self.assertEquals(composite.get_references(), expected_refs)
+
+    def test_assembleRefs_two_items_one_with_one_ref_one_without(self):
+        val1 = Value('${foo}bar',  SETTINGS, '')
+        val2 = Value('baz',  SETTINGS, '')
+        expected_refs = ['foo']
+
+        composite = CompItem([val1, val2], SETTINGS)
+
+        self.assertTrue(composite.has_references)
+        self.assertEquals(composite.get_references(), expected_refs)
+
+    def test_assembleRefs_two_items_both_with_one_ref(self):
+        val1 = Value('${foo}',  SETTINGS, '')
+        val2 = Value('${bar}',  SETTINGS, '')
+        expected_refs = ['foo', 'bar']
+
+        composite = CompItem([val1, val2], SETTINGS)
+
+        self.assertTrue(composite.has_references)
+        self.assertEquals(composite.get_references(), expected_refs)
+
+    def test_assembleRefs_two_items_with_two_refs(self):
+        val1 = Value('${foo}${baz}',  SETTINGS, '')
+        val2 = Value('${bar}${meep}',  SETTINGS, '')
+        expected_refs = ['foo', 'baz', 'bar', 'meep']
+
+        composite = CompItem([val1, val2], SETTINGS)
+
+        self.assertTrue(composite.has_references)
+        self.assertEquals(composite.get_references(), expected_refs)
+
+    def test_render_single_item(self):
+        val1 = Value('${foo}',  SETTINGS, '')
+
+        composite = CompItem([val1], SETTINGS)
+
+        self.assertEquals(1, composite.render({'foo': 1}, None))
+
+
+    def test_render_multiple_items(self):
+        val1 = Value('${foo}',  SETTINGS, '')
+        val2 = Value('${bar}',  SETTINGS, '')
+
+        composite = CompItem([val1, val2], SETTINGS)
+
+        self.assertEquals('12', composite.render({'foo': 1, 'bar': 2}, None))
+
+    def test_merge_over_merge_scalar(self):
+        val1 = Value(None, SETTINGS, '')
+        scalar = ScaItem(1, SETTINGS)
+        composite = CompItem([val1], SETTINGS)
+
+        result = composite.merge_over(scalar)
+
+        self.assertEquals(result, composite)
+
+    def test_merge_over_merge_composite(self):
+        val1 = Value(None, SETTINGS, '')
+        val2 = Value(None, SETTINGS, '')
+        composite1 = CompItem([val1], SETTINGS)
+        composite2 = CompItem([val2], SETTINGS)
+
+        result = composite2.merge_over(composite1)
+
+        self.assertEquals(result, composite2)
+
+    def test_merge_over_merge_list_not_allowed(self):
+        val1 = Value(None, SETTINGS, '')
+        listitem = ListItem([1], SETTINGS)
+        composite = CompItem([val1], SETTINGS)
+
+        self.assertRaises(RuntimeError, composite.merge_over, listitem)
+
+    def test_merge_dict_dict_not_allowed(self):
+        val1 = Value(None, SETTINGS, '')
+        dictitem = DictItem({'foo': 'bar'}, SETTINGS)
+        composite = CompItem([val1], SETTINGS)
+
+        self.assertRaises(RuntimeError, composite.merge_over, dictitem)
+
+    def test_merge_other_types_not_allowed(self):
+        other = type('Other', (object,), {'type': 34})
+        val1 = Value(None, SETTINGS, '')
+        composite = CompItem([val1], SETTINGS)
+
+        self.assertRaises(RuntimeError, composite.merge_over, other)
+
+
+if __name__ == '__main__':
+    unittest.main()
diff --git a/reclass/values/tests/test_value.py b/reclass/values/tests/test_value.py
index 4853340..a06d220 100644
--- a/reclass/values/tests/test_value.py
+++ b/reclass/values/tests/test_value.py
@@ -11,8 +11,6 @@
 from __future__ import print_function
 from __future__ import unicode_literals
 
-import pyparsing as pp
-
 from reclass.settings import Settings
 from reclass.values.value import Value
 from reclass.errors import ResolveError, ParseError
@@ -42,14 +40,14 @@
     def test_simple_string(self):
         s = 'my cat likes to hide in boxes'
         tv = Value(s, SETTINGS, '')
-        self.assertFalse(tv.has_references())
+        self.assertFalse(tv.has_references)
         self.assertEquals(tv.render(CONTEXT, None), s)
 
     def _test_solo_ref(self, key):
         s = _var(key)
         tv = Value(s, SETTINGS, '')
         res = tv.render(CONTEXT, None)
-        self.assertTrue(tv.has_references())
+        self.assertTrue(tv.has_references)
         self.assertEqual(res, CONTEXT[key])
 
     def test_solo_ref_string(self):
@@ -70,7 +68,7 @@
     def test_single_subst_bothends(self):
         s = 'I like ' + _var('favcolour') + ' and I like it'
         tv = Value(s, SETTINGS, '')
-        self.assertTrue(tv.has_references())
+        self.assertTrue(tv.has_references)
         self.assertEqual(tv.render(CONTEXT, None),
                          _poor_mans_template(s, 'favcolour',
                                              CONTEXT['favcolour']))
@@ -78,7 +76,7 @@
     def test_single_subst_start(self):
         s = _var('favcolour') + ' is my favourite colour'
         tv = Value(s, SETTINGS, '')
-        self.assertTrue(tv.has_references())
+        self.assertTrue(tv.has_references)
         self.assertEqual(tv.render(CONTEXT, None),
                          _poor_mans_template(s, 'favcolour',
                                              CONTEXT['favcolour']))
@@ -86,7 +84,7 @@
     def test_single_subst_end(self):
         s = 'I like ' + _var('favcolour')
         tv = Value(s, SETTINGS, '')
-        self.assertTrue(tv.has_references())
+        self.assertTrue(tv.has_references)
         self.assertEqual(tv.render(CONTEXT, None),
                          _poor_mans_template(s, 'favcolour',
                                              CONTEXT['favcolour']))
@@ -95,7 +93,7 @@
         motd = SETTINGS.delimiter.join(('motd', 'greeting'))
         s = _var(motd)
         tv = Value(s, SETTINGS, '')
-        self.assertTrue(tv.has_references())
+        self.assertTrue(tv.has_references)
         self.assertEqual(tv.render(CONTEXT, None),
                          _poor_mans_template(s, motd,
                                              CONTEXT['motd']['greeting']))
@@ -104,7 +102,7 @@
         greet = SETTINGS.delimiter.join(('motd', 'greeting'))
         s = _var(greet) + ' I like ' + _var('favcolour') + '!'
         tv = Value(s, SETTINGS, '')
-        self.assertTrue(tv.has_references())
+        self.assertTrue(tv.has_references)
         want = _poor_mans_template(s, greet, CONTEXT['motd']['greeting'])
         want = _poor_mans_template(want, 'favcolour', CONTEXT['favcolour'])
         self.assertEqual(tv.render(CONTEXT, None), want)
@@ -113,7 +111,7 @@
         greet = SETTINGS.delimiter.join(('motd', 'greeting'))
         s = _var(greet) + ' I like ' + _var('favcolour')
         tv = Value(s, SETTINGS, '')
-        self.assertTrue(tv.has_references())
+        self.assertTrue(tv.has_references)
         want = _poor_mans_template(s, greet, CONTEXT['motd']['greeting'])
         want = _poor_mans_template(want, 'favcolour', CONTEXT['favcolour'])
         self.assertEqual(tv.render(CONTEXT, None), want)
diff --git a/reclass/values/value.py b/reclass/values/value.py
index 4e86274..affd944 100644
--- a/reclass/values/value.py
+++ b/reclass/values/value.py
@@ -23,7 +23,7 @@
     def __init__(self, value, settings, uri, parse_string=True):
         self._settings = settings
         self._uri = uri
-        self._overwrite = False
+        self.overwrite = False
         self._constant = False
         if isinstance(value, string_types):
             if parse_string:
@@ -42,12 +42,8 @@
             self._item = ScaItem(value, self._settings)
 
     @property
-    def overwrite(self):
-        return self._overwrite
-
-    @overwrite.setter
-    def overwrite(self, overwrite):
-        self._overwrite = overwrite
+    def uri(self):
+        return self._uri
 
     @property
     def constant(self):
@@ -57,10 +53,6 @@
     def constant(self, constant):
         self._constant = constant
 
-    @property
-    def uri(self):
-        return self._uri
-
     def item_type(self):
         return self._item.type
 
@@ -70,26 +62,31 @@
     def is_container(self):
         return self._item.is_container()
 
+    @property
     def allRefs(self):
-        return self._item.allRefs()
+        return self._item.allRefs
 
+    @property
     def has_references(self):
-        return self._item.has_references()
+        return self._item.has_references
 
+    @property
     def has_inv_query(self):
-        return self._item.has_inv_query()
+        return self._item.has_inv_query
 
+    @property
     def needs_all_envs(self):
-        if self._item.has_inv_query():
-            return self._item.needs_all_envs()
+        if self._item.has_inv_query:
+            return self._item.needs_all_envs
         else:
             return False
 
     def ignore_failed_render(self):
-        return self._item.ignore_failed_render()
+        return self._item.ignore_failed_render
 
+    @property
     def is_complex(self):
-        return self._item.is_complex()
+        return self._item.is_complex
 
     def get_references(self):
         return self._item.get_references()
@@ -98,7 +95,7 @@
         return self._item.get_inv_references()
 
     def assembleRefs(self, context):
-        if self._item.has_references():
+        if self._item.has_references:
             self._item.assembleRefs(context)
 
     def render(self, context, inventory):
@@ -108,8 +105,9 @@
             e.uri = self._uri
             raise
 
+    @property
     def contents(self):
-        return self._item.contents()
+        return self._item.contents
 
     def merge_over(self, value):
         self._item = self._item.merge_over(value._item)
diff --git a/reclass/values/valuelist.py b/reclass/values/valuelist.py
index 9c1e1fa..a56395b 100644
--- a/reclass/values/valuelist.py
+++ b/reclass/values/valuelist.py
@@ -8,31 +8,28 @@
 from __future__ import print_function
 from __future__ import unicode_literals
 
-from __future__ import print_function
-
 import copy
 import sys
 
 from reclass.errors import ChangedConstantError, ResolveError, TypeMergeError
 
 
-
 class ValueList(object):
 
     def __init__(self, value, settings):
         self._settings = settings
         self._refs = []
-        self._allRefs = True
-        self._values = [ value ]
+        self.allRefs = True
+        self._values = [value]
         self._inv_refs = []
         self._has_inv_query = False
-        self._ignore_failed_render = False
+        self.ignore_failed_render = False
         self._is_complex = False
         self._update()
 
     @property
     def uri(self):
-        return '; '.join([ str(x.uri) for x in self._values ])
+        return '; '.join([str(x.uri) for x in self._values])
 
     def append(self, value):
         self._values.append(value)
@@ -48,51 +45,48 @@
         self._is_complex = False
         item_type = self._values[0].item_type()
         for v in self._values:
-            if v.is_complex() or v.constant or v.overwrite or v.item_type() != item_type:
+            if v.is_complex or v.constant or v.overwrite or v.item_type() != item_type:
                 self._is_complex = True
 
+    @property
     def has_references(self):
         return len(self._refs) > 0
 
+    @property
     def has_inv_query(self):
         return self._has_inv_query
 
     def get_inv_references(self):
         return self._inv_refs
 
+    @property
     def is_complex(self):
         return self._is_complex
 
     def get_references(self):
         return self._refs
 
-    def allRefs(self):
-        return self._allRefs
-
-    def ignore_failed_render(self):
-        return self._ignore_failed_render
-
     def _check_for_inv_query(self):
         self._has_inv_query = False
-        self._ignore_failed_render = True
+        self.ignore_failed_render = True
         for value in self._values:
-            if value.has_inv_query():
+            if value.has_inv_query:
                 self._inv_refs.extend(value.get_inv_references)
                 self._has_inv_query = True
                 if vale.ignore_failed_render() is False:
-                    self._ignore_failed_render = False
+                    self.ignore_failed_render = False
         if self._has_inv_query is False:
-            self._ignore_failed_render = False
+            self.ignore_failed_render = False
 
     def assembleRefs(self, context={}):
         self._refs = []
-        self._allRefs = True
+        self.allRefs = True
         for value in self._values:
             value.assembleRefs(context)
-            if value.has_references():
+            if value.has_references:
                 self._refs.extend(value.get_references())
-            if value.allRefs() is False:
-                self._allRefs = False
+            if value.allRefs is False:
+                self.allRefs = False
 
     def merge(self):
         output = None
@@ -114,12 +108,17 @@
             try:
                 new = value.render(context, inventory)
             except ResolveError as e:
-                # only ignore failed renders if ignore_overwritten_missing_references is set and we are dealing with a scalar value
-                # and it's not the last item in the values list
-                if self._settings.ignore_overwritten_missing_references and not isinstance(output, (dict, list)) and n != (len(self._values)-1):
+                # only ignore failed renders if
+                # ignore_overwritten_missing_references is set and we are
+                # dealing with a scalar value and it's not the last item in the
+                # values list
+                if (self._settings.ignore_overwritten_missing_references
+                        and not isinstance(output, (dict, list))
+                        and n != (len(self._values)-1)):
                     new = None
                     last_error = e
-                    print("[WARNING] Reference '%s' undefined" % str(value), file=sys.stderr)
+                    print("[WARNING] Reference '%s' undefined" % str(value),
+                          file=sys.stderr)
                 else:
                     raise e
 
@@ -186,6 +185,3 @@
             raise last_error
 
         return output
-
-    def __repr__(self):
-        return 'ValueList(%r)' % self._values
diff --git a/requirements.txt b/requirements.txt
index 66f0f4b..5b3aadd 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,4 +1,4 @@
 pyparsing
 pyyaml
-pygit2
 six
+enum34
diff --git a/setup.py b/setup.py
index 789b0fd..884be88 100644
--- a/setup.py
+++ b/setup.py
@@ -42,7 +42,7 @@
     url = URL,
     packages = find_packages(exclude=['*tests']), #FIXME validate this
     entry_points = { 'console_scripts': console_scripts },
-    install_requires = ['pyparsing', 'pyyaml', 'six'], #FIXME pygit2 (require libffi-dev, libgit2-dev 0.26.x )
+    install_requires = ['pyparsing', 'pyyaml', 'six', 'enum34'], #FIXME pygit2 (require libffi-dev, libgit2-dev 0.26.x )
 
     classifiers=[
         'Development Status :: 4 - Beta',