Merge pull request #718 from darko-poljak/bugfix/716-conflicting-object-parameters

Fix DirectoryDict getitem.

cdist/emulator.py

@@ -182,6 +182,29 @@ class Emulator(object):
         lockfname = lockfname.replace(os.sep, '_')
         self.flock_path = os.path.join(self.object_base_path, lockfname)
 
+    def _object_params_in_context(self):
+        ''' Get cdist_object parameters dict adopted by context.
+        Context consists of cdist_type boolean, optional, required,
+        optional_multiple and required_multiple parameters. If parameter
+        is multiple parameter then its value is a list.
+        This adaptation works on cdist_object.parameters which are read from
+        directory based dict where it is unknown what kind of data is in
+        file. If there is only one line in the file it is unknown if this
+        is a value of required/optional parameter or if it is one value of
+        multiple values parameter.
+        '''
+        params = {}
+        if self.cdist_object.exists:
+            for param in self.cdist_object.parameters:
+                value = ('' if param in self.cdist_type.boolean_parameters
+                         else self.cdist_object.parameters[param])
+                if ((param in self.cdist_type.required_multiple_parameters or
+                     param in self.cdist_type.optional_multiple_parameters) and
+                        not isinstance(value, list)):
+                    value = [value]
+                params[param] = value
+        return params
+
     def setup_object(self):
         # Create object with given parameters
         self.parameters = {}
@@ -193,12 +216,13 @@ class Emulator(object):
             # Make existing requirements a set so that we can compare it
             # later with new requirements.
             self._existing_reqs = set(self.cdist_object.requirements)
-            if self.cdist_object.parameters != self.parameters:
+            obj_params = self._object_params_in_context()
+            if obj_params != self.parameters:
                 errmsg = ("Object %s already exists with conflicting "
                           "parameters:\n%s: %s\n%s: %s" % (
                               self.cdist_object.name,
                               " ".join(self.cdist_object.source),
-                              self.cdist_object.parameters,
+                              obj_params,
                               self.object_source,
                               self.parameters))
                 raise cdist.Error(errmsg)
@@ -252,7 +276,7 @@ class Emulator(object):
                                 self.cdist_object.name,
                                 requirement, e.name, self.object_source)))
             raise
-        except core.cdist_object.MissingObjectIdError as e:
+        except core.cdist_object.MissingObjectIdError:
             self.log.error(("%s requires object %s without object id."
                             " Defined at %s" % (self.cdist_object.name,
                                                 requirement,

cdist/test/emulator/__init__.py

@@ -420,6 +420,27 @@ class ArgumentsTestCase(test.CdistTestCase):
         self.assertEqual(cdist_object.parameters['required1'], value)
         self.assertEqual(cdist_object.parameters['required2'], value)
 
+    def test_required_multiple_arguments(self):
+        """check whether assigning required multiple parameter works"""
+
+        type_name = '__arguments_required_multiple'
+        object_id = 'some-id'
+        value1 = 'value1'
+        value2 = 'value2'
+        argv = [type_name, object_id, '--required1', value1,
+                '--required1', value2]
+        os.environ.update(self.env)
+        emu = emulator.Emulator(argv)
+        emu.run()
+
+        cdist_type = core.CdistType(self.local.type_path, type_name)
+        cdist_object = core.CdistObject(cdist_type, self.local.object_path,
+                                        self.local.object_marker_name,
+                                        object_id)
+        self.assertTrue('required1' in cdist_object.parameters)
+        self.assertTrue(value1 in cdist_object.parameters['required1'])
+        self.assertTrue(value2 in cdist_object.parameters['required1'])
+
 #    def test_required_missing(self):
 #        type_name = '__arguments_required'
 #        object_id = 'some-id'
@@ -447,6 +468,25 @@ class ArgumentsTestCase(test.CdistTestCase):
         self.assertFalse('optional2' in cdist_object.parameters)
         self.assertEqual(cdist_object.parameters['optional1'], value)
 
+    def test_optional_multiple(self):
+        type_name = '__arguments_optional_multiple'
+        object_id = 'some-id'
+        value1 = 'value1'
+        value2 = 'value2'
+        argv = [type_name, object_id, '--optional1', value1, '--optional1',
+                value2]
+        os.environ.update(self.env)
+        emu = emulator.Emulator(argv)
+        emu.run()
+
+        cdist_type = core.CdistType(self.local.type_path, type_name)
+        cdist_object = core.CdistObject(cdist_type, self.local.object_path,
+                                        self.local.object_marker_name,
+                                        object_id)
+        self.assertTrue('optional1' in cdist_object.parameters)
+        self.assertTrue(value1 in cdist_object.parameters['optional1'])
+        self.assertTrue(value2 in cdist_object.parameters['optional1'])
+
     def test_argument_defaults(self):
         type_name = '__argument_defaults'
         object_id = 'some-id'
@@ -464,6 +504,29 @@ class ArgumentsTestCase(test.CdistTestCase):
         self.assertFalse('optional2' in cdist_object.parameters)
         self.assertEqual(cdist_object.parameters['optional1'], value)
 
+    def test_object_params_in_context(self):
+        type_name = '__arguments_all'
+        object_id = 'some-id'
+        argv = [type_name, object_id, '--opt', 'opt', '--req', 'req',
+                '--bool', '--optmul', 'val1', '--optmul', 'val2',
+                '--reqmul', 'val3', '--reqmul', 'val4',
+                '--optmul1', 'val5', '--reqmul1', 'val6']
+        os.environ.update(self.env)
+        emu = emulator.Emulator(argv)
+        emu.run()
+
+        obj_params = emu._object_params_in_context()
+        obj_params_expected = {
+            'bool': '',
+            'opt': 'opt',
+            'optmul1': ['val5', ],
+            'optmul': ['val1', 'val2', ],
+            'req': 'req',
+            'reqmul1': ['val6', ],
+            'reqmul': ['val3', 'val4', ],
+        }
+        self.assertEqual(obj_params, obj_params_expected)
+
 
 class StdinTestCase(test.CdistTestCase):
 

cdist/test/emulator/fixtures/conf/type/__arguments_all/parameter/boolean

@@ -0,0 +1 @@
+bool

cdist/test/emulator/fixtures/conf/type/__arguments_all/parameter/optional

@@ -0,0 +1 @@
+opt

cdist/test/emulator/fixtures/conf/type/__arguments_all/parameter/optional_multiple

@@ -0,0 +1,2 @@
+optmul
+optmul1

cdist/test/emulator/fixtures/conf/type/__arguments_all/parameter/required

@@ -0,0 +1 @@
+req

cdist/test/emulator/fixtures/conf/type/__arguments_all/parameter/required_multiple

@@ -0,0 +1,2 @@
+reqmul
+reqmul1

cdist/test/emulator/fixtures/conf/type/__arguments_optional_multiple/parameter/optional_multiple

@@ -0,0 +1 @@
+optional1

cdist/test/emulator/fixtures/conf/type/__arguments_required_multiple/parameter/required_multiple

@@ -0,0 +1 @@
+required1

cdist/util/fsproperty.py

@@ -58,7 +58,7 @@ class FileList(collections.MutableSequence):
             with open(self.path) as fd:
                 for line in fd:
                     lines.append(line.rstrip('\n'))
-        except EnvironmentError as e:
+        except EnvironmentError:
             # error ignored
             pass
         return lines
@@ -127,7 +127,16 @@ class DirectoryDict(collections.MutableMapping):
     def __getitem__(self, key):
         try:
             with open(os.path.join(self.path, key), "r") as fd:
-                return fd.read().rstrip('\n')
+                value = fd.read().splitlines()
+                # if there is no value/empty line then return ''
+                # if there is only one value then return that value
+                # if there are multiple lines in file then return list
+                if not value:
+                    return ''
+                elif len(value) == 1:
+                    return value[0]
+                else:
+                    return value
         except EnvironmentError:
             raise KeyError(key)