added support for JSON and YAML properties files

4 files changed, 556 insertions, 145 deletions

diff --git a/doc/server/plugins/connectors/properties.txt b/doc/server/plugins/connectors/properties.txt
index e10b90df9..2a037df94 100644
--- a/doc/server/plugins/connectors/properties.txt
+++ b/doc/server/plugins/connectors/properties.txt
@@ -7,42 +7,125 @@ Properties
 ==========
 
 The Properties plugin is a connector plugin that adds information from
-properties files into client metadata instances.
+XML, JSON, and YAML files into client metadata instances.
 
 Enabling Properties
 ===================
 
-First, ``mkdir /var/lib/bcfg2/Properties``. Each property XML file goes
-in this directory. Each will automatically be cached by the server,
-and reread/reparsed upon changes. Add **Properties** to your ``plugins``
+First, ``mkdir /var/lib/bcfg2/Properties``. Each property file goes in
+this directory. Each will automatically be cached by the server, and
+reread/reparsed upon changes. Add **Properties** to your ``plugins``
 line in ``/etc/bcfg2.conf``.
 
 Data Structures
 ===============
 
 Properties adds a new dictionary to client metadata instances that maps
-property file names to PropertyFile instances. PropertyFile instances
-contain parsed XML data as the "data" attribute.
+property file names to PropertyFile instances.
+
+A property file can be one of three types:
+
+* If the filename ends with ``.xml``, it will be parsed as XML and
+  handled by :class:`Bcfg2.Server.Plugins.Properties.XMLPropertyFile`.
+  See `XML Property Files`_ below.
+* If the filename ends with ``.json`` and JSON libraries are installed
+  (either ``json`` or ``simplejson``, although ``json`` is highly
+  recommended), it will be parsed as `JSON <http://www.json.org/>`_
+  and handled by
+  :class:`Bcfg2.Server.Plugins.Properties.JSONPropertyFile`.  See
+  `JSON Property Files`_ below.
+* If the filename ends with ``.yaml`` or ``.yml`` and PyYAML is
+  installed, it will be parsed as `YAML <http://www.yaml.org/>`_ and
+  handled by
+  :class:`Bcfg2.Server.Plugins.Properties.YAMLPropertyFile`.  See
+  `YAML Property Files`_ below.
+
+The XML interface is undoubtably the most powerful, as it natively
+supports schemas to check the data validity, client- and
+group-specific data, and data encryption.
 
-The XML data in a property file is arbitrary, but a matching ``.xsd``
-file can be created to assign a schema to a property file, which will
-be checked when running ``bcfg2-lint``.  For instance, given::
+Usage
+=====
 
-    Properties/dns-config.xml
-    Properties/dns-config.xsd
+Common Interface
+----------------
 
-``dns-config.xml`` will be validated against ``dns-config.xsd``.
+Different data types have different interfaces, but there are some
+usage patterns common to all properties files.
 
-Although Properties files are technically freeform XML, the top-level
-XML tag should be ``<Properties>``.
+Specific property files can be referred to in templates as
+``metadata.Properties[<filename>]``.
 
-Usage
-=====
+The data in property files is accessible via different attributes:
 
-Specific property files can be referred to in templates as
-``metadata.Properties[<filename>]``. The ``xdata`` attribute is an
-lxml.etree._Element object. (Documented `here
-<http://codespeak.net/lxml/tutorial.html#the-element-class>`_)
++-----------+----------------+
+| Data Type | Data Attribute |
++===========+================+
+| XML       | ``xdata``      |
++-----------+----------------+
+| JSON      | ``json``       |
++-----------+----------------+
+| YAML      | ``yaml``       |
++-----------+----------------+
+
+For instance, in a :ref:`Genshi template
+<server-plugins-generators-cfg-genshi>`, you might do::
+
+    {% for item in metadata.Properties['foo.json'].json %}\
+    ${item}
+    {% end %}\
+
+    {% for key, value in metadata.Properties['foo.yml'].yaml %}\
+    ${key} = ${value}
+    {% end %}\
+
+    {% for el in metadata.Properties['foo.xml'].xdata.findall("Tag") %}\
+    ${el.get("name")} = ${el.text}
+    {% end %}\
+
+The raw contents of a properties file as a string are available via
+the ``data`` attribute, e.g., ``metadata.Properties['prop-file'].data``.
+
+.. _server-plugins-connectors-properties-write-back:
+
+Writing to Properties files
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+.. versionadded:: 1.2.0
+
+If you need to make persistent changes to properties data, you can use
+the ``write`` method of the
+:class:`Bcfg2.Server.Plugins.Properties.PropertyFile` class::
+
+    {% python
+    import lxml.etree
+    from genshi.template import TemplateError
+    lxml.etree.SubElement(metadata.Properties['foo.xml'],
+                          "Client",
+                          name=metadata.hostname)
+    if not metadata.Properties['foo.xml'].write():
+        raise TemplateError("Failed to write changes back to foo.xml")
+
+The interface is the same for YAML or JSON data.
+
+If writing XML data, the ``write`` method checks the data in the
+object against its schema before writing it; see `Data Structures`_
+for details.
+
+Note that use of the ``write`` method can cause race conditions if you
+run more than one Bcfg2 server.  If you run more than one Bcfg2
+server, you can disable Properties write-back by setting the following
+in ``bcfg2.conf``::
+
+    [properties]
+    writes_enabled = false
+
+XML Property Files
+------------------
+
+The data in an XML property file can be accessed with the ``xdata``
+attribute, an :class:`lxml.etree._Element` object documented `here
+<http://codespeak.net/lxml/tutorial.html#the-element-class>`_.
 
 In addition to the ``xdata`` attribute that can be used to access the
 raw data, the following access methods are defined:
@@ -67,9 +150,6 @@ raw data, the following access methods are defined:
                    for el in metadata.Properties['ntp.xml'].XMLMatch(metadata).findall("//Server")]
     %}
 
-As we formulate more common use cases, we will add them to the
-``PropertyFile`` class as methods. This will simplify templates.
-
 You can also access the XML data that comprises a property file
 directly in one of several ways:
 
@@ -82,6 +162,37 @@ directly in one of several ways:
   top-level element.  (I.e., everything directly under the
   ``<Properties>`` tag.)
 
+The XML data in a property file is arbitrary, but a matching ``.xsd``
+file can be created to assign a schema to a property file, which will
+be checked when running ``bcfg2-lint``.  For instance, given::
+
+    Properties/dns-config.xml
+    Properties/dns-config.xsd
+
+``dns-config.xml`` will be validated against ``dns-config.xsd``.
+
+Although Properties files are technically freeform XML, the top-level
+XML tag should be ``<Properties>``.
+
+
+JSON Property Files
+-------------------
+
+The data in a JSON property file can be accessed with the ``json``
+attribute, which is the loaded JSON data.  The JSON properties
+interface does not provide any additional functionality beyond the
+`Common Interface`_.
+
+YAML Property Files
+-------------------
+
+The data in a YAML property file can be accessed with the ``yaml``
+attribute, which is the loaded YAML data.  Only a single YAML document
+may be included in a file.
+
+The YAML properties interface does not provide any additional
+functionality beyond the `Common Interface`_.
+
 .. _server-plugins-connectors-properties-automatch:
 
 Automatch
@@ -90,8 +201,8 @@ Automatch
 .. versionadded:: 1.3.0
 
 You can enable
-:func:`Bcfg2.Server.Plugins.Properties.PropertyFile.XMLMatch()` for
-all Property files by setting ``automatch`` to ``true`` in the
+:func:`Bcfg2.Server.Plugin.helpers.StructFile.XMLMatch()` for all XML
+Property files by setting ``automatch`` to ``true`` in the
 ``[properties]`` section of ``bcfg2.conf``.  This makes
 ``metadata.Properties`` values :class:`lxml.etree._Element` objects
 that contain only matching data.  (This makes it impossible to do
@@ -140,37 +251,6 @@ it anyway with ``-f``::
     With automatch enabled, they are :class:`lxml.etree._Element`
     objects.
 
-.. _server-plugins-connectors-properties-write-back:
-
-Writing to Properties files
-===========================
-
-.. versionadded:: 1.2.0
-
-If you need to make persistent changes to properties data, you can use
-the ``write`` method of the
-:class:`Bcfg2.Server.Plugins.Properties.PropertyFile` class::
-
-    {% python
-    import lxml.etree
-    from genshi.template import TemplateError
-    lxml.etree.SubElement(metadata.Properties['foo.xml'],
-                          "Client",
-                          name=metadata.hostname)
-    if not metadata.Properties['foo.xml'].write():
-        raise TemplateError("Failed to write changes back to foo.xml")
-
-The ``write`` method checks the data in the object against its schema
-before writing it; see `Data Structures`_ for details.
-
-Note that use of the ``write`` method can cause race conditions if you
-run more than one Bcfg2 server.  If you run more than one Bcfg2
-server, you can disable Properties write-back by setting the following
-in ``bcfg2.conf``::
-
-    [properties]
-    writes_enabled = false
-
 .. _server-plugins-connectors-properties-encryption:
 
 Encrypted Properties data
@@ -178,12 +258,12 @@ Encrypted Properties data
 
 .. versionadded:: 1.3.0
 
-You can encrypt selected data in Properties files to protect that data
-from other people who need access to the repository.  See
+You can encrypt selected data in XML Properties files to protect that
+data from other people who need access to the repository.  See
 :ref:`server-encryption-configuration` for details on configuring
 encryption passphrases.  The data is decrypted transparently
 on-the-fly by the server; you never need to decrypt the data in your
-templates.
+templates.  Encryption is only supported on XML properties files.
 
 .. note::
 
@@ -232,3 +312,8 @@ It understands the following directives:
 * ``writes_enabled``: Enable
   :ref:`server-plugins-connectors-properties-write-back`.  Default is
   true.
+
+Module Documentation
+====================
+
+.. automodule:: Bcfg2.Server.Plugins.Properties
diff --git a/src/lib/Bcfg2/Server/Plugins/Probes.py b/src/lib/Bcfg2/Server/Plugins/Probes.py
index c63f015e5..c2a928e0f 100644
--- a/src/lib/Bcfg2/Server/Plugins/Probes.py
+++ b/src/lib/Bcfg2/Server/Plugins/Probes.py
@@ -41,16 +41,10 @@ except ImportError:
         HAS_JSON = False
 
 try:
-    import syck as yaml
-    import syck.error as YAMLError
+    import yaml
     HAS_YAML = True
 except ImportError:
-    try:
-        import yaml
-        from yaml import YAMLError
-        HAS_YAML = True
-    except ImportError:
-        HAS_YAML = False
+    HAS_YAML = False
 
 
 class ClientProbeDataSet(dict):
@@ -110,7 +104,7 @@ class ProbeData(str):
         if self._yaml is None and HAS_YAML:
             try:
                 self._yaml = yaml.load(self.data)
-            except YAMLError:
+            except yaml.YAMLError:
                 pass
         return self._yaml
 
diff --git a/src/lib/Bcfg2/Server/Plugins/Properties.py b/src/lib/Bcfg2/Server/Plugins/Properties.py
index d9e622645..aef5238c6 100644
--- a/src/lib/Bcfg2/Server/Plugins/Properties.py
+++ b/src/lib/Bcfg2/Server/Plugins/Properties.py
@@ -16,17 +16,42 @@ try:
 except ImportError:
     HAS_CRYPTO = False
 
+try:
+    import json
+    HAS_JSON = True
+except ImportError:
+    try:
+        import simplejson as json
+        HAS_JSON = True
+    except ImportError:
+        HAS_JSON = False
+
+try:
+    import yaml
+    HAS_YAML = True
+except ImportError:
+    HAS_YAML = False
+
 LOGGER = logging.getLogger(__name__)
 
 SETUP = None
 
 
-class PropertyFile(Bcfg2.Server.Plugin.StructFile):
-    """ Class for properties files. """
+class PropertyFile(object):
+    """ Base Properties file handler """
+
+    def __init__(self, name):
+        """
+        :param name: The filename of this properties file.
+
+        .. automethod:: _write
+        """
+        self.name = name
 
     def write(self):
         """ Write the data in this data structure back to the property
-        file """
+        file. This public method performs checking to ensure that
+        writing is possible and then calls :func:`_write`. """
         if not SETUP.cfp.getboolean("properties", "writes_enabled",
                                     default=True):
             msg = "Properties files write-back is disabled in the " + \
@@ -39,19 +64,123 @@ class PropertyFile(Bcfg2.Server.Plugin.StructFile):
             msg = "Cannot write %s: %s" % (self.name, sys.exc_info()[1])
             LOGGER.error(msg)
             raise PluginExecutionError(msg)
-
         try:
-            open(self.name, "wb").write(
-                lxml.etree.tostring(self.xdata,
-                                    xml_declaration=False,
-                                    pretty_print=True).decode('UTF-8'))
-            return True
+            return self._write()
         except IOError:
             err = sys.exc_info()[1]
             msg = "Failed to write %s: %s" % (self.name, err)
             LOGGER.error(msg)
             raise PluginExecutionError(msg)
 
+    def _write(self):
+        """ Write the data in this data structure back to the property
+        file. """
+        raise NotImplementedError
+
+    def validate_data(self):
+        """ Verify that the data in this file is valid. """
+        raise NotImplementedError
+
+    def get_additional_data(self, metadata):  # pylint: disable=W0613
+        """ Get file data for inclusion in client metadata. """
+        return copy.copy(self)
+
+
+class JSONPropertyFile(Bcfg2.Server.Plugin.FileBacked, PropertyFile):
+    """ Handle JSON Properties files. """
+
+    def __init__(self, name, fam=None):
+        Bcfg2.Server.Plugin.FileBacked.__init__(self, name, fam=fam)
+        PropertyFile.__init__(self, name)
+        self.json = None
+    __init__.__doc__ = Bcfg2.Server.Plugin.FileBacked.__init__.__doc__
+
+    def Index(self):
+        try:
+            self.json = json.loads(self.data)
+        except ValueError:
+            err = sys.exc_info()[1]
+            raise PluginExecutionError("Could not load JSON data from %s: %s" %
+                                       (self.name, err))
+    Index.__doc__ = Bcfg2.Server.Plugin.FileBacked.Index.__doc__
+
+    def _write(self):
+        json.dump(self.json, open(self.name, 'wb'))
+        return True
+    _write.__doc__ = PropertyFile._write.__doc__
+
+    def validate_data(self):
+        try:
+            json.dumps(self.json)
+        except:
+            err = sys.exc_info()[1]
+            raise PluginExecutionError("Data for %s cannot be dumped to JSON: "
+                                       "%s" % (self.name, err))
+    validate_data.__doc__ = PropertyFile.validate_data.__doc__
+
+    def __str__(self):
+        return str(self.json)
+
+    def __repr__(self):
+        return repr(self.json)
+
+
+class YAMLPropertyFile(Bcfg2.Server.Plugin.FileBacked, PropertyFile):
+    """ Handle YAML Properties files. """
+
+    def __init__(self, name, fam=None):
+        Bcfg2.Server.Plugin.FileBacked.__init__(self, name, fam=fam)
+        PropertyFile.__init__(self, name)
+        self.yaml = None
+    __init__.__doc__ = Bcfg2.Server.Plugin.FileBacked.__init__.__doc__
+
+    def Index(self):
+        try:
+            self.yaml = yaml.load(self.data)
+        except yaml.YAMLError:
+            err = sys.exc_info()[1]
+            raise PluginExecutionError("Could not load YAML data from %s: %s" %
+                                       (self.name, err))
+    Index.__doc__ = Bcfg2.Server.Plugin.FileBacked.Index.__doc__
+
+    def _write(self):
+        yaml.dump(self.yaml, open(self.name, 'wb'))
+        return True
+    _write.__doc__ = PropertyFile._write.__doc__
+
+    def validate_data(self):
+        try:
+            yaml.dump(self.yaml)
+        except yaml.YAMLError:
+            err = sys.exc_info()[1]
+            raise PluginExecutionError("Data for %s cannot be dumped to YAML: "
+                                       "%s" % (self.name, err))
+    validate_data.__doc__ = PropertyFile.validate_data.__doc__
+
+    def __str__(self):
+        return str(self.yaml)
+
+    def __repr__(self):
+        return repr(self.yaml)
+
+
+class XMLPropertyFile(Bcfg2.Server.Plugin.StructFile, PropertyFile):
+    """ Handle XML Properties files. """
+
+    def __init__(self, name, fam=None, should_monitor=False):
+        Bcfg2.Server.Plugin.StructFile.__init__(self, name, fam=fam,
+                                                should_monitor=should_monitor)
+        PropertyFile.__init__(self, name)
+    __init__.__doc__ = Bcfg2.Server.Plugin.StructFile.__init__.__doc__
+
+    def _write(self):
+        open(self.name, "wb").write(
+            lxml.etree.tostring(self.xdata,
+                                xml_declaration=False,
+                                pretty_print=True).decode('UTF-8'))
+        return True
+    _write.__doc__ = PropertyFile._write.__doc__
+
     def validate_data(self):
         """ ensure that the data in this object validates against the
         XML schema for this property file (if a schema exists) """
@@ -73,6 +202,7 @@ class PropertyFile(Bcfg2.Server.Plugin.StructFile):
                                        self.name)
         else:
             return True
+    validate_data.__doc__ = PropertyFile.validate_data.__doc__
 
     def Index(self):
         Bcfg2.Server.Plugin.StructFile.Index(self)
@@ -89,6 +219,7 @@ class PropertyFile(Bcfg2.Server.Plugin.StructFile):
                                                                   self.name)
                     LOGGER.error(msg)
                     raise PluginExecutionError(msg)
+    Index.__doc__ = Bcfg2.Server.Plugin.StructFile.Index.__doc__
 
     def _decrypt(self, element):
         """ Decrypt a single encrypted properties file element """
@@ -111,19 +242,77 @@ class PropertyFile(Bcfg2.Server.Plugin.StructFile):
                                       algorithm=get_algorithm(SETUP))
         raise EVPError("Failed to decrypt")
 
+    def get_additional_data(self, metadata):
+        if SETUP.cfp.getboolean("properties", "automatch", default=False):
+            default_automatch = "true"
+        else:
+            default_automatch = "false"
+
+        if self.xdata.get("automatch", default_automatch).lower() == "true":
+            return self.XMLMatch(metadata)
+        else:
+            return copy.copy(self)
+
+    def __str__(self):
+        return str(self.xdata)
+
+    def __repr__(self):
+        return repr(self.xdata)
+
 
 class PropDirectoryBacked(Bcfg2.Server.Plugin.DirectoryBacked):
-    """ A collection of properties files """
-    __child__ = PropertyFile
-    patterns = re.compile(r'.*\.xml$')
+    """ A collection of properties files. """
+    extensions = ["xml"]
+    if HAS_JSON:
+        extensions.append("json")
+    if HAS_YAML:
+        extensions.extend(["yaml", "yml"])
+
+    #: Only track and include files whose names and paths match this
+    #: regex.  Created on-the-fly based on which libraries are
+    #: installed (and thus which data formats are supported).
+    #: Candidates are ``.xml`` (always supported), ``.json``,
+    #: ``.yaml``, and ``.yml``.
+    patterns = re.compile(r'.*\.%s$' % '|'.join(extensions))
+
+    #: Ignore XML schema (``.xsd``) files
     ignore = re.compile(r'.*\.xsd$')
 
+    def __init__(self, data, fam):
+        Bcfg2.Server.Plugin.DirectoryBacked.__init__(self, data, fam)
+
+        #: Instead of creating children of this object with a static
+        #: object, we use :func:`property_dispatcher` to create a
+        #: child of the appropriate subclass of :class:`PropertyFile`
+        self.__child__ = self.property_dispatcher
+    __init__.__doc__ = Bcfg2.Server.Plugin.DirectoryBacked.__init__.__doc__
+
+    def property_dispatcher(self, fname, fam):
+        """ Dispatch an event on a Properties file to the
+        appropriate object.
+
+        :param fname: The name of the file that received the event
+        :type fname: string
+        :param fam: The file monitor the event was received by
+        :type fam: Bcfg2.Server.FileMonitor.FileMonitor
+        :returns: An object of the appropriate subclass of
+                  :class:`PropertyFile`
+        """
+        if fname.endswith(".xml"):
+            return XMLPropertyFile(fname, fam)
+        elif HAS_JSON and fname.endswith(".json"):
+            return JSONPropertyFile(fname, fam)
+        elif HAS_YAML and (fname.endswith(".yaml") or fname.endswith(".yml")):
+            return YAMLPropertyFile(fname, fam)
+        else:
+            raise Bcfg2.Server.Plugin.PluginExecutionError(
+                "Properties: Unknown extension %s" % fname)
+
 
 class Properties(Bcfg2.Server.Plugin.Plugin,
                  Bcfg2.Server.Plugin.Connector):
     """ The properties plugin maps property files into client metadata
     instances. """
-    name = 'Properties'
 
     def __init__(self, core, datastore):
         global SETUP  # pylint: disable=W0603
@@ -138,18 +327,12 @@ class Properties(Bcfg2.Server.Plugin.Plugin,
             raise Bcfg2.Server.Plugin.PluginInitError
 
         SETUP = core.setup
+    __init__.__doc__ = Bcfg2.Server.Plugin.Plugin.__init__.__doc__
 
     def get_additional_data(self, metadata):
-        if self.core.setup.cfp.getboolean("properties", "automatch",
-                                          default=False):
-            default_automatch = "true"
-        else:
-            default_automatch = "false"
         rv = dict()
         for fname, pfile in self.store.entries.items():
-            if pfile.xdata.get("automatch",
-                               default_automatch).lower() == "true":
-                rv[fname] = pfile.XMLMatch(metadata)
-            else:
-                rv[fname] = copy.copy(pfile)
+            rv[fname] = pfile.get_additional_data(metadata)
         return rv
+    get_additional_data.__doc__ = \
+        Bcfg2.Server.Plugin.Connector.get_additional_data.__doc__
diff --git a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestProperties.py b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestProperties.py
index fb4773d75..78cb5f52d 100644
--- a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestProperties.py
+++ b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestProperties.py
@@ -15,32 +15,44 @@ while path != "/":
         break
     path = os.path.dirname(path)
 from common import *
-from TestPlugin import TestStructFile, TestConnector, TestPlugin, \
-    TestDirectoryBacked
+from TestPlugin import TestStructFile, TestFileBacked, TestConnector, \
+    TestPlugin, TestDirectoryBacked
 
+try:
+    import json
+    JSON = "json"
+except ImportError:
+    JSON = "simplejson"
 
-class TestPropertyFile(TestStructFile):
+
+class TestPropertyFile(Bcfg2TestCase):
     test_obj = PropertyFile
+    path = os.path.join(datastore, "test")
 
-    @patch("%s.open" % builtins)
-    def test_write(self, mock_open):
+    def get_obj(self, path=None):
+        if path is None:
+            path = self.path
+        return self.test_obj(path)
+
+    def test_write(self):
         Bcfg2.Server.Plugins.Properties.SETUP = Mock()
         pf = self.get_obj()
         pf.validate_data = Mock()
+        pf._write = Mock()
 
         xstr = u("<Properties/>\n")
         pf.xdata = lxml.etree.XML(xstr)
 
         def reset():
             pf.validate_data.reset_mock()
+            pf._write.reset_mock()
             Bcfg2.Server.Plugins.Properties.SETUP.reset_mock()
-            mock_open.reset_mock()
 
         # test writes disabled
         Bcfg2.Server.Plugins.Properties.SETUP.cfp.getboolean.return_value = False
         self.assertRaises(PluginExecutionError, pf.write)
         self.assertFalse(pf.validate_data.called)
-        self.assertFalse(mock_open.called)
+        self.assertFalse(pf._write.called)
         Bcfg2.Server.Plugins.Properties.SETUP.cfp.getboolean.assert_called_with("properties",
                                                 "writes_enabled",
                                                 default=True)
@@ -48,17 +60,16 @@ class TestPropertyFile(TestStructFile):
         # test successful write
         reset()
         Bcfg2.Server.Plugins.Properties.SETUP.cfp.getboolean.return_value = True
-        self.assertTrue(pf.write())
+        self.assertEqual(pf.write(), pf._write.return_value)
         pf.validate_data.assert_called_with()
-        mock_open.assert_called_with(pf.name, "wb")
-        mock_open.return_value.write.assert_called_with(xstr)
+        pf._write.assert_called_with()
 
-        # test error from write
+        # test error from _write
         reset()
-        mock_open.side_effect = IOError
+        pf._write.side_effect = IOError
         self.assertRaises(PluginExecutionError, pf.write)
         pf.validate_data.assert_called_with()
-        mock_open.assert_called_with(pf.name, "wb")
+        pf._write.assert_called_with()
 
         # test error from validate_data
         reset()
@@ -66,6 +77,124 @@ class TestPropertyFile(TestStructFile):
         self.assertRaises(PluginExecutionError, pf.write)
         pf.validate_data.assert_called_with()
 
+    def test__write(self):
+        pf = self.get_obj()
+        self.assertRaises(NotImplementedError, pf._write)
+
+    def test_validate_data(self):
+        pf = self.get_obj()
+        self.assertRaises(NotImplementedError, pf.validate_data)
+
+    @patch("copy.copy")
+    def test_get_additional_data(self, mock_copy):
+        pf = self.get_obj()
+        self.assertEqual(pf.get_additional_data(Mock()),
+                         mock_copy.return_value)
+        mock_copy.assert_called_with(pf)
+
+
+if can_skip or HAS_JSON:
+    class TestJSONPropertyFile(TestFileBacked, TestPropertyFile):
+        test_obj = JSONPropertyFile
+
+        def get_obj(self, *args, **kwargs):
+            return TestFileBacked.get_obj(self, *args, **kwargs)
+
+        @skipUnless(HAS_JSON, "JSON libraries not found, skipping")
+        def setUp(self):
+            pass
+
+        @patch("%s.loads" % JSON)
+        def test_Index(self, mock_loads):
+            pf = self.get_obj()
+            pf.Index()
+            mock_loads.assert_called_with(pf.data)
+            self.assertEqual(pf.json, mock_loads.return_value)
+
+            mock_loads.reset_mock()
+            mock_loads.side_effect = ValueError
+            self.assertRaises(PluginExecutionError, pf.Index)
+            mock_loads.assert_called_with(pf.data)
+
+        @patch("%s.dump" % JSON)
+        @patch("%s.open" % builtins)
+        def test__write(self, mock_open, mock_dump):
+            pf = self.get_obj()
+            self.assertTrue(pf._write())
+            mock_open.assert_called_with(pf.name, 'wb')
+            mock_dump.assert_called_with(pf.json, mock_open.return_value)
+
+        @patch("%s.dumps" % JSON)
+        def test_validate_data(self, mock_dumps):
+            pf = self.get_obj()
+            pf.validate_data()
+            mock_dumps.assert_called_with(pf.json)
+
+            mock_dumps.reset_mock()
+            mock_dumps.side_effect = ValueError
+            self.assertRaises(PluginExecutionError, pf.validate_data)
+            mock_dumps.assert_called_with(pf.json)
+
+
+if can_skip or HAS_YAML:
+    class TestYAMLPropertyFile(TestFileBacked, TestPropertyFile):
+        test_obj = YAMLPropertyFile
+
+        def get_obj(self, *args, **kwargs):
+            return TestFileBacked.get_obj(self, *args, **kwargs)
+
+        @skipUnless(HAS_YAML, "YAML libraries not found, skipping")
+        def setUp(self):
+            pass
+
+        @patch("yaml.load")
+        def test_Index(self, mock_load):
+            pf = self.get_obj()
+            pf.Index()
+            mock_load.assert_called_with(pf.data)
+            self.assertEqual(pf.yaml, mock_load.return_value)
+
+            mock_load.reset_mock()
+            mock_load.side_effect = yaml.YAMLError
+            self.assertRaises(PluginExecutionError, pf.Index)
+            mock_load.assert_called_with(pf.data)
+
+        @patch("yaml.dump")
+        @patch("%s.open" % builtins)
+        def test__write(self, mock_open, mock_dump):
+            pf = self.get_obj()
+            self.assertTrue(pf._write())
+            mock_open.assert_called_with(pf.name, 'wb')
+            mock_dump.assert_called_with(pf.yaml, mock_open.return_value)
+
+        @patch("yaml.dump")
+        def test_validate_data(self, mock_dump):
+            pf = self.get_obj()
+            pf.validate_data()
+            mock_dump.assert_called_with(pf.yaml)
+
+            mock_dump.reset_mock()
+            mock_dump.side_effect = yaml.YAMLError
+            self.assertRaises(PluginExecutionError, pf.validate_data)
+            mock_dump.assert_called_with(pf.yaml)
+
+
+class TestXMLPropertyFile(TestPropertyFile, TestStructFile):
+    test_obj = XMLPropertyFile
+    path = TestStructFile.path
+
+    def get_obj(self, *args, **kwargs):
+        return TestStructFile.get_obj(self, *args, **kwargs)
+
+    @patch("%s.open" % builtins)
+    def test__write(self, mock_open):
+        pf = self.get_obj()
+        pf.xdata = lxml.etree.Element("Test")
+        self.assertTrue(pf._write())
+        mock_open.assert_called_with(pf.name, "wb")
+        self.assertXMLEqual(pf.xdata,
+                            lxml.etree.XML(mock_open.return_value.write.call_args[0][0]))
+
     @patch("os.path.exists")
     @patch("lxml.etree.XMLSchema")
     def test_validate_data(self, mock_XMLSchema, mock_exists):
@@ -224,10 +353,63 @@ class TestPropertyFile(TestStructFile):
                                                algorithm="bf_cbc")
             self.assertFalse(mock_ssl.called)
 
+    @patch("copy.copy")
+    def test_get_additional_data(self, mock_copy):
+        Bcfg2.Server.Plugins.Properties.SETUP = Mock()
+        pf = self.get_obj()
+        pf.XMLMatch = Mock()
+        metadata = Mock()
+
+        def reset():
+            mock_copy.reset_mock()
+            pf.XMLMatch.reset_mock()
+            Bcfg2.Server.Plugins.Properties.SETUP.reset_mock()
+
+        pf.xdata = lxml.etree.Element("Properties", automatch="true")
+        for automatch in [True, False]:
+            reset()
+            Bcfg2.Server.Plugins.Properties.SETUP.cfp.getboolean.return_value = automatch
+            self.assertEqual(pf.get_additional_data(metadata),
+                             pf.XMLMatch.return_value)
+            pf.XMLMatch.assert_called_with(metadata)
+            Bcfg2.Server.Plugins.Properties.SETUP.cfp.getboolean.assert_called_with("properties", "automatch", default=False)
+            self.assertFalse(mock_copy.called)
+
+        pf.xdata = lxml.etree.Element("Properties", automatch="false")
+        for automatch in [True, False]:
+            reset()
+            Bcfg2.Server.Plugins.Properties.SETUP.cfp.getboolean.return_value = automatch
+            self.assertEqual(pf.get_additional_data(metadata),
+                             mock_copy.return_value)
+            mock_copy.assert_called_with(pf)
+            self.assertFalse(pf.XMLMatch.called)
+            Bcfg2.Server.Plugins.Properties.SETUP.cfp.getboolean.assert_called_with("properties", "automatch", default=False)
+
+        pf.xdata = lxml.etree.Element("Properties")
+        reset()
+        Bcfg2.Server.Plugins.Properties.SETUP.cfp.getboolean.return_value = False
+        self.assertEqual(pf.get_additional_data(metadata),
+                         mock_copy.return_value)
+        mock_copy.assert_called_with(pf)
+        self.assertFalse(pf.XMLMatch.called)
+        Bcfg2.Server.Plugins.Properties.SETUP.cfp.getboolean.assert_called_with("properties", "automatch", default=False)
+
+        reset()
+        Bcfg2.Server.Plugins.Properties.SETUP.cfp.getboolean.return_value = True
+        self.assertEqual(pf.get_additional_data(metadata),
+                         pf.XMLMatch.return_value)
+        pf.XMLMatch.assert_called_with(metadata)
+        Bcfg2.Server.Plugins.Properties.SETUP.cfp.getboolean.assert_called_with("properties", "automatch", default=False)
+        self.assertFalse(mock_copy.called)
+
 
 class TestPropDirectoryBacked(TestDirectoryBacked):
     test_obj = PropDirectoryBacked
     testfiles = ['foo.xml', 'bar.baz.xml']
+    if HAS_JSON:
+        testfiles.extend(["foo.json", "foo.xml.json"])
+    if HAS_YAML:
+        testfiles.extend(["foo.yaml", "foo.yml", "foo.xml.yml"])
     ignore = ['foo.xsd', 'bar.baz.xsd', 'quux.xml.xsd']
     badevents = ['bogus.txt']
 
@@ -248,45 +430,12 @@ class TestProperties(TestPlugin, TestConnector):
         TestConnector.test_get_additional_data(self)
 
         p = self.get_obj()
-        automatch = Mock()
-        automatch.xdata = lxml.etree.Element("Properties", automatch="true")
-        automatch.XMLMatch.return_value = "automatch"
-        raw = Mock()
-        raw.xdata = lxml.etree.Element("Properties")
-        raw.XMLMatch.return_value = "raw"
-        nevermatch = Mock()
-        nevermatch.xdata = lxml.etree.Element("Properties", automatch="false")
-        nevermatch.XMLMatch.return_value = "nevermatch"
-        p.store.entries = {
-            "/foo/automatch.xml": automatch,
-            "/foo/raw.xml": raw,
-            "/foo/nevermatch.xml": nevermatch,
-            }
-
-        # we make copy just return the object it was asked to copy so
-        # that we can test the return value of get_additional_data(),
-        # which copies every object it doesn't XMLMatch()
-        mock_copy.side_effect = lambda o: o
-
-        # test with automatch default to false
-        p.core.setup.cfp.getboolean.return_value = False
         metadata = Mock()
-        self.assertItemsEqual(p.get_additional_data(metadata),
-                              {
-                "/foo/automatch.xml": automatch.XMLMatch.return_value,
-                "/foo/raw.xml": raw,
-                "/foo/nevermatch.xml": nevermatch})
-        automatch.XMLMatch.assert_called_with(metadata)
-        self.assertFalse(raw.XMLMatch.called)
-        self.assertFalse(nevermatch.XMLMatch.called)
-
-        # test with automatch default to true
-        p.core.setup.cfp.getboolean.return_value = True
-        self.assertItemsEqual(p.get_additional_data(metadata),
-                              {
-                "/foo/automatch.xml": automatch.XMLMatch.return_value,
-                "/foo/raw.xml": raw.XMLMatch.return_value,
-                "/foo/nevermatch.xml": nevermatch})
-        automatch.XMLMatch.assert_called_with(metadata)
-        raw.XMLMatch.assert_called_with(metadata)
-        self.assertFalse(nevermatch.XMLMatch.called)
+        p.store.entries = {"foo.xml": Mock(),
+                           "foo.yml": Mock()}
+        rv = p.get_additional_data(metadata)
+        expected = dict()
+        for name, entry in p.store.entries.items():
+            entry.get_additional_data.assert_called_with(metadata)
+            expected[name] = entry.get_additional_data.return_value
+        self.assertItemsEqual(rv, expected)