diff --git a/CHANGES.rst b/CHANGES.rst index 2d818302..0ae16733 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -18,6 +18,9 @@ New features: Bug fixes: +- Component._encode stops ignoring parameters argument on native values, now merges them + Fixes: #557 + [zocker1999net] - ... 5.0.9 (2023-09-24) diff --git a/docs/credits.rst b/docs/credits.rst index 1343193c..eedcc24b 100644 --- a/docs/credits.rst +++ b/docs/credits.rst @@ -70,6 +70,7 @@ icalendar contributors - `Natasha Mattson `_ - Matt Lewis +- Felix Stupp Find out who contributed:: diff --git a/src/icalendar/cal.py b/src/icalendar/cal.py index 40889717..2535680f 100644 --- a/src/icalendar/cal.py +++ b/src/icalendar/cal.py @@ -113,7 +113,8 @@ def is_broken(self): ############################# # handling of property values - def _encode(self, name, value, parameters=None, encode=1): + @staticmethod + def _encode(name, value, parameters=None, encode=1): """Encode values to icalendar property values. :param name: Name of the property. @@ -138,17 +139,19 @@ def _encode(self, name, value, parameters=None, encode=1): return value if isinstance(value, types_factory.all_types): # Don't encode already encoded values. - return value - klass = types_factory.for_property(name) - obj = klass(value) + obj = value + else: + klass = types_factory.for_property(name) + obj = klass(value) if parameters: - if isinstance(parameters, dict): - params = Parameters() - for key, item in parameters.items(): - params[key] = item - parameters = params - assert isinstance(parameters, Parameters) - obj.params = parameters + if not hasattr(obj, "params"): + obj.params = Parameters() + for key, item in parameters.items(): + if item is None: + if key in obj.params: + del obj.params[key] + else: + obj.params[key] = item return obj def add(self, name, value, parameters=None, encode=1): diff --git a/src/icalendar/tests/test_issue_557_encode_native_parameters.py b/src/icalendar/tests/test_issue_557_encode_native_parameters.py new file mode 100644 index 00000000..99d9f051 --- /dev/null +++ b/src/icalendar/tests/test_issue_557_encode_native_parameters.py @@ -0,0 +1,152 @@ +"""These are tests for Issue #557 + +TL;DR: Component._encode lost given parameters +if the object to encode was already of native type, +making its behavior unexpected. + +see https://github.com/collective/icalendar/issues/557""" + + +import unittest + +from icalendar.cal import Component + + +class TestComponentEncode(unittest.TestCase): + def test_encode_non_native_parameters(self): + """Test _encode to add parameters to non-natives""" + self.__assert_native_content(self.summary) + self.__assert_native_kept_parameters(self.summary) + + def test_encode_native_keep_params_None(self): + """_encode should keep parameters on natives + if parameters=None + """ + new_sum = self.__add_params( + self.summary, + parameters=None, + ) + self.__assert_native_content(new_sum) + self.__assert_native_kept_parameters(new_sum) + + def test_encode_native_keep_params_empty(self): + """_encode should keep paramters on natives + if parameters={} + """ + new_sum = self.__add_params( + self.summary, + parameters={}, + ) + self.__assert_native_content(new_sum) + self.__assert_native_kept_parameters(new_sum) + + def test_encode_native_append_params(self): + """_encode should append paramters on natives + keeping old parameters + """ + new_sum = self.__add_params( + self.summary, + parameters={"X-PARAM": "Test123"}, + ) + self.__assert_native_content(new_sum) + self.__assert_native_kept_parameters(new_sum) + self.assertParameter(new_sum, "X-PARAM", "Test123") + + def test_encode_native_overwrite_params(self): + """_encode should overwrite single parameters + if they have the same name as old ones""" + new_sum = self.__add_params( + self.summary, + parameters={"LANGUAGE": "de"}, + ) + self.__assert_native_content(new_sum) + self.assertParameter(new_sum, "LANGUAGE", "de") + + def test_encode_native_remove_params(self): + """_encode should remove single parameters + if they are explicitly set to None""" + new_sum = self.__add_params( + self.summary, + parameters={"LANGUAGE": None}, + ) + self.__assert_native_content(new_sum) + self.assertParameterMissing(new_sum, "LANGUAGE") + + def test_encode_native_remove_already_missing(self): + """_encode should ignore removing a parameter + that was already missing""" + self.assertParameterMissing(self.summary, "X-MISSING") + new_sum = self.__add_params( + self.summary, + parameters={"X-MISSING": None}, + ) + self.__assert_native_content(new_sum) + self.__assert_native_kept_parameters(new_sum) + self.assertParameterMissing(self.summary, "X-MISSING") + + def test_encode_native_full_test(self): + """full test case with keeping, overwriting & removing properties""" + # preperation + orig_sum = self.__add_params( + self.summary, + parameters={ + "X-OVERWRITE": "overwrite me!", + "X-REMOVE": "remove me!", + "X-MISSING": None, + }, + ) + # preperation check + self.__assert_native_content(orig_sum) + self.__assert_native_kept_parameters(orig_sum) + self.assertParameter(orig_sum, "X-OVERWRITE", "overwrite me!") + self.assertParameter(orig_sum, "X-REMOVE", "remove me!") + self.assertParameterMissing(orig_sum, "X-MISSING") + # modification + new_sum = self.__add_params( + orig_sum, + parameters={ + "X-OVERWRITE": "overwritten", + "X-REMOVE": None, + "X-MISSING": None, + }, + ) + # final asserts + self.__assert_native_content(new_sum) + self.__assert_native_kept_parameters(new_sum) + self.assertParameter(new_sum, "X-OVERWRITE", "overwritten") + self.assertParameterMissing(new_sum, "X-REMOVE") + self.assertParameterMissing(new_sum, "X-MISSING") + + def setUp(self): + self.summary = self.__gen_native() + + def __assert_native_kept_parameters(self, obj): + self.assertParameter(obj, "LANGUAGE", "en") + + def __assert_native_content(self, obj): + self.assertEqual(obj, "English Summary") + + def __add_params(self, obj, parameters): + return Component._encode( + "SUMMARY", + obj, + parameters=parameters, + encode=True, + ) + + def __gen_native(self): + return Component._encode( + "SUMMARY", + "English Summary", + parameters={ + "LANGUAGE": "en", + }, + encode=True, + ) + + def assertParameterMissing(self, obj, name): + self.assertNotIn(name, obj.params) + + def assertParameter(self, obj, name, val): + self.assertIn(name, obj.params) + self.assertEqual(obj.params[name], val)