Attachment 8364 Details for Bug 43395

[patch] Fix udm property default handling

Bug-43395-Fix-udm-property-default-handling.patch (text/plain), 8.99 KB, created by Philipp Hahn on 2017-01-23 10:23 CET

(hide)

Description:

Filename:

MIME Type:

Creator: Philipp Hahn

Created: 2017-01-23 10:23 CET

Size: 8.99 KB

patch

obsolete

>From bfdc8c8fa9439b9bfe8be324a436139b7d225d14 Mon Sep 17 00:00:00 2001
>Message-Id: <bfdc8c8fa9439b9bfe8be324a436139b7d225d14.1485163397.git.hahn@univention.de>
>From: Philipp Hahn <hahn@univention.de>
>Date: Mon, 23 Jan 2017 10:21:36 +0100
>Subject: [PATCH] Bug #43395: Fix udm property default handling
>Organization: Univention GmbH, Bremen, Germany
>
>> Traceback (most recent call last):
>>   File "test-univention-admin.py", line 73, in test_template_mv_empty
>>     self.assertEqual(p.default(o), [])
>>   File "/usr/lib/pymodules/python2.7/univention/admin/__init__.py", line 269, in default
>>     self.base_default[i] = self._replace(self.base_default[i], object)
>> TypeError: 'tuple' object does not support item assignment
>
>Simplify use of isinstance(), e.g. basestring
>Clean-up twisted logic of default()
>---
> .../modules/univention/admin/__init__.py           |  76 ++++++---------
> .../test/test-univention-admin.py                  | 108 +++++++++++++++++++++
> 2 files changed, 139 insertions(+), 45 deletions(-)
> create mode 100644 management/univention-directory-manager-modules/test/test-univention-admin.py
>
>diff --git a/management/univention-directory-manager-modules/modules/univention/admin/__init__.py b/management/univention-directory-manager-modules/modules/univention/admin/__init__.py
>index ed1239d..2350b4e 100644
>--- a/management/univention-directory-manager-modules/modules/univention/admin/__init__.py
>+++ b/management/univention-directory-manager-modules/modules/univention/admin/__init__.py
>@@ -240,58 +240,44 @@ class property:
> 
> 	def default(self, object):
> 		if not object.set_defaults:
>-			if self.multivalue:
>-				return []
>-			else:
>-				return ''
>+			return [] if self.multivalue else ''
> 
> 		if not self.base_default:
> 			return self.new()
> 
>-		if isinstance(self.base_default, (types.StringType, types.UnicodeType)):
>+		if isinstance(self.base_default, basestring):
> 			return self._replace(self.base_default, object)
> 
>+		bd0 = self.base_default[0]
>+
> 		# we can not import univention.admin.syntax here (recursive import) so we need to find another way to identify a complex syntax
>-		if getattr(self.syntax, 'subsyntaxes', None) is not None and isinstance(self.base_default[0], (list, tuple)) and not self.multivalue:
>-			return self.base_default[0]
>-		# multivalue defaults will only be a part of templates, so not multivalue is the common way for modules
>-		elif (isinstance(self.base_default[0], (types.StringType, types.UnicodeType))) and not self.multivalue:
>-			res = self.base_default[0]
>-			for p in self.base_default[1]:
>-				if not object[p]:
>-					return self.new()
>-				res = res.replace('<' + p + '>', object[p])
>-			return res
>-
>-		elif (isinstance(self.base_default[0], (types.StringType, types.UnicodeType))):
>-			for i in range(0, len(self.base_default)):
>-				if isinstance(self.base_default[i], (types.StringType, types.UnicodeType)):
>-					self.base_default[i] = self._replace(self.base_default[i], object)
>-				else:  # must be a list of loaded extended attributes then, so we return it if it has content
>-					if len(self.base_default[i]) > 0:
>-						if self.multivalue and not isinstance(self.base_default[i], types.ListType):
>-							return [self.base_default[i]]
>-						else:
>-							return self.base_default[i]
>-					else:
>-						# return the first element, this is only related to empty extended attributes which are loaded wrong, needs to be fixed elsewhere
>-						if i > 0:
>-							if self.multivalue and not isinstance(self.base_default[0], types.ListType):
>-								return [self.base_default[0]]
>-							else:
>-								return self.base_default[0]
>-						else:
>-							return self.new()
>-			return self.base_default
>-
>-		elif isinstance(self.base_default[0], types.FunctionType) or callable(self.base_default[0]):
>-			for p in self.base_default[1]:
>-				if not object[p]:
>-					return self.new()
>-			return self.base_default[0](object, self.base_default[2])
>-		else:
>+		if getattr(self.syntax, 'subsyntaxes', None) is not None and isinstance(bd0, (list, tuple)) and not self.multivalue:
>+			return bd0
>+
>+		if isinstance(bd0, basestring):
>+			# multivalue defaults will only be a part of templates, so not multivalue is the common way for modules
>+			if not self.multivalue:  # default=(template-str, [list-of-required-properties])
>+				if all(object[p] for p in self.base_default[1]):
>+					for p in self.base_default[1]:
>+						bd0 = bd0.replace('<%s>' % (p,), object[p])
>+					return bd0
>+				return self.new()
>+			else:  # multivalue
>+				if all(isinstance(bd, basestring) for bd in self.base_default):
>+					return [self._replace(bd, object) for bd in self.base_default]
>+				# must be a list of loaded extended attributes then, so we return it if it has content
>+				# return the first element, this is only related to empty extended attributes which are loaded wrong, needs to be fixed elsewhere
>+				if bd0:
>+					return [bd0]
>+				return self.new()
>+
>+		if callable(bd0):  # default=(func_obj_extra, [list-of-required-properties], extra-arg)
>+			if all(object[p] for p in self.base_default[1]):
>+				return bd0(object, self.base_default[2])
> 			return self.new()
> 
>+		return self.new()
>+
> 	def safe_default(self, object):
> 		def safe_parse(default):
> 			if not default:
>@@ -302,7 +288,7 @@ class property:
> 			except:
> 				return False
> 		defaults = self.default(object)
>-		if isinstance(defaults, types.ListType):
>+		if isinstance(defaults, list):
> 			return [self.syntax.parse(d) for d in defaults if safe_parse(d)]
> 		elif safe_parse(defaults):
> 			return self.syntax.parse(defaults)
>@@ -311,7 +297,7 @@ class property:
> 	def check_default(self, object):
> 		defaults = self.default(object)
> 		try:
>-			if isinstance(defaults, types.ListType):
>+			if isinstance(defaults, list):
> 				for d in defaults:
> 					if d:
> 						self.syntax.parse(d)
>diff --git a/management/univention-directory-manager-modules/test/test-univention-admin.py b/management/univention-directory-manager-modules/test/test-univention-admin.py
>new file mode 100644
>index 0000000..3bdb791
>--- /dev/null
>+++ b/management/univention-directory-manager-modules/test/test-univention-admin.py
>@@ -0,0 +1,108 @@
>+#!/usr/bin/python
>+
>+from univention.admin import property
>+import unittest
>+from argparse import Namespace as N
>+
>+
>+class FakeObject(dict):
>+	set_defaults = True
>+
>+
>+class TestProperty(unittest.TestCase):
>+
>+	def test_default_sv(self):
>+		p = property()
>+		o = N(set_defaults=False)
>+		self.assertEqual(p.default(o), '')
>+
>+	def test_default_mv(self):
>+		p = property(multivalue=True)
>+		o = N(set_defaults=False)
>+		self.assertEqual(p.default(o), [])
>+
>+	def test_base_default_sv(self):
>+		p = property()
>+		o = FakeObject()
>+		self.assertIsNone(p.default(o))
>+
>+	def test_base_default_mv(self):
>+		p = property(multivalue=True)
>+		o = FakeObject()
>+		self.assertEqual(p.default(o), [])
>+
>+	def test_str_default_sv(self):
>+		p = property(default='x')
>+		o = FakeObject()
>+		self.assertEqual(p.default(o), 'x')
>+
>+	# def test_str_default_mv(self):
>+	# 	p = property(multivalue=True, default=('x', 'y'))
>+	# 	o = N(set_defaults=True)
>+	# 	self.assertEqual(p.default(o), ['x', 'y'])
>+
>+	def test_complex_syntax(self):
>+		s = N(subsyntaxes=())
>+		p = property(multivalue=False, default=(('x', 'y'),), syntax=s)
>+		o = FakeObject()
>+		self.assertEqual(p.default(o), ('x', 'y'))
>+
>+	def test_template_sv_empty(self):
>+		p = property(multivalue=False, default=('templ', ['prop']))
>+		o = FakeObject(prop='')
>+		self.assertIsNone(p.default(o))
>+
>+	def test_template_sv_set(self):
>+		p = property(multivalue=False, default=('<prop>', ['prop']))
>+		o = FakeObject(prop='value')
>+		self.assertEqual(p.default(o), 'value')
>+
>+	def test_template_mv_set(self):
>+		p = property(multivalue=True, default=('<prop1>', '<prop2>'))
>+		o = FakeObject(prop1='value1', prop2='value2')
>+		self.assertEqual(p.default(o), ['value1', 'value2'])
>+
>+	def test_template_mv_incomplete(self):
>+		p = property(multivalue=True, default=('<prop>', None))
>+		o = FakeObject()
>+		self.assertEqual(p.default(o), ['<prop>'])
>+
>+	def test_template_mv_empty(self):
>+		p = property(multivalue=True, default=('', None))
>+		o = FakeObject()
>+		self.assertEqual(p.default(o), [])
>+
>+	def test_callable_set(self):
>+		x = object()
>+		o = FakeObject(prop='value1')
>+		f = lambda obj, extra: 'value2' if extra is x and obj is o else 'error'
>+		p = property(multivalue=False, default=(f, ['prop'], x))
>+		self.assertEqual(p.default(o), 'value2')
>+
>+	def test_callable_empty_sv(self):
>+		x = object()
>+		o = FakeObject(prop='')
>+		f = lambda obj, extra: 1 / 0
>+		p = property(multivalue=False, default=(f, ['prop'], x))
>+		self.assertIsNone(p.default(o))
>+
>+	def test_callable_empty_mv(self):
>+		x = object()
>+		o = FakeObject(prop='')
>+		f = lambda obj, extra: 1 / 0
>+		p = property(multivalue=True, default=(f, ['prop'], x))
>+		self.assertEqual(p.default(o), [])
>+
>+	def test_fallback_sv(self):
>+		o = FakeObject()
>+		p = property(multivalue=False, default=(None,))
>+		self.assertIsNone(p.default(o))
>+
>+	def test_fallback_mv(self):
>+		o = FakeObject()
>+		p = property(multivalue=True, default=(None,))
>+		self.assertEqual(p.default(o), [])
>+
>+
>+if __name__ == '__main__':
>+	unittest.main()
>-- 
>2.1.4
>

Actions: View | Diff

Attachments on bug 43395: 8363 | 8364