6: Fleshed out API.register_command, made correpsonding unit tests much more rigorous

ipalib/base.py

@@ -28,12 +28,8 @@ import exceptions
 class Named(object):
 	prefix = None
 
-	@classmethod
-	def clsname(cls):
-		return cls.__name__
-
 	def __init__(self):
-		clsname = self.clsname()
+		clsname = self.__class__.__name__
 		assert type(self.prefix) is str
 		prefix = self.prefix + '_'
 		if not clsname.startswith(prefix):
@@ -50,7 +46,8 @@ class Named(object):
 	name_cli = property(__get_name_cli)
 
 
-class Command(object):
+class Command(Named):
+	prefix = 'cmd'
 
 	def normalize(self, kw):
 		raise NotImplementedError
@@ -92,7 +89,7 @@ class NameSpace(object):
 	For example, setting an attribute the normal way will raise an exception:
 
 	>>> ns.my_message = 'some new value'
-	(raises exceptions.SetAttributeError)
+	(raises exceptions.SetError)
 
 	But a programmer could still set the attribute like this:
 
@@ -123,7 +120,7 @@ class NameSpace(object):
 		NameSpace has been locked; otherwise calls object.__setattr__().
 		"""
 		if self.__locked:
-			raise exceptions.SetAttributeError(name)
+			raise exceptions.SetError(name)
 		super(NameSpace, self).__setattr__(name, value)
 
 	def __getitem__(self, key):
@@ -166,8 +163,9 @@ class API(object):
 	__locked = False
 
 	def __init__(self):
-		self.__c = {} # Proposed commands
-		self.__o = {} # Proposed objects
+		self.__classes = set()
+		self.__names = set()
+		self.__stage = {}
 
 	def __get_objects(self):
 		return self.__objects
@@ -177,19 +175,26 @@ class API(object):
 		return self.__commands
 	commands = property(__get_commands)
 
-	def __merge(self, target, base, cls, override):
-		assert type(target) is dict
-		assert inspect.isclass(base)
-		assert inspect.isclass(cls)
+	def __merge(self, base, cls, override):
+		assert issubclass(base, Named)
 		assert type(override) is bool
-		if not issubclass(cls, base):
-			raise exceptions.RegistrationError(
-				cls,
-				'%s.%s' % (base.__module__, base.__name__)
-			)
+		if not (inspect.isclass(cls) and issubclass(cls, base)):
+			raise exceptions.RegistrationError(cls,	base.__name__)
+		if cls in self.__classes:
+			raise exceptions.DuplicateError(cls.__name__, id(cls))
+		if cls.__name__ in self.__names and not override:
+			raise exceptions.OverrideError(cls.__name__)
+		self.__classes.add(cls)
+		self.__names.add(cls.__name__)
+		if base not in self.__stage:
+			self.__stage[base.prefix] = {}
+		self.__stage[base.prefix][cls.__name__] = cls
+
 
 	def register_command(self, cls, override=False):
-		self.__merge(self.__c, Command, cls, override)
+		self.__merge(Command, cls, override)
 
 	def finalize(self):
 		pass
+		#i = cls()
+		#assert cls.__name__ == (base.prefix + '_' + i.name)

ipalib/exceptions.py

@@ -45,16 +45,20 @@ class IPAError(Exception):
 		return self.msg % self.kw
 
 
-class SetAttributeError(IPAError):
-	msg = 'Cannot set %r: NameSpace does not allow attribute setting'
+class SetError(IPAError):
+	msg = 'setting %r, but NameSpace does not allow attribute setting'
 
 
 class OverrideError(IPAError):
-	msg = 'Unexpected override of %r; use override=True if intended'
+	msg = 'unexpected override of %r (use override=True if intended)'
+
+
+class DuplicateError(IPAError):
+	msg = 'class %r at %d was already registered'
 
 
 class RegistrationError(IPAError):
-	msg = '%r is not a subclass of %s'
+	msg = '%r must be a subclass of %s'
 
 
 class PrefixError(IPAError):

ipalib/tests/test_base.py

@@ -184,7 +184,7 @@ class test_NameSpace:
 			raised = False
 			try:
 				setattr(ns, key, value)
-			except exceptions.SetAttributeError:
+			except exceptions.SetError:
 				raised = True
 			assert raised
 			assert getattr(ns, key, None) != value
@@ -238,13 +238,17 @@ class test_NameSpace:
 		assert len(kw) == len(ns) == 3
 
 
-class test_Command:
-	def new(self):
-		return base.Command()
+class test_Command(ClassChecker):
+	class cmd_some_command(base.Command):
+		pass
+	cls = cmd_some_command
 
 	def test_fresh(self):
 		c = self.new()
-
+		assert isinstance(c, base.Named)
+		assert c.name == 'some_command'
+		assert c.name_cli == 'some-command'
+		assert callable(c)
 
 
 class test_API:
@@ -267,19 +271,46 @@ class test_API:
 		assert read_only(api, 'objects') is None
 
 	def test_register_command(self):
-		class  my_command(base.Command):
-			pass
-		class another_command(base.Command):
-			pass
 		api = self.new()
 
-		api.register_command(my_command)
+		class cmd_my_command(base.Command):
+			pass
+		class cmd_another_command(base.Command):
+			pass
 
-		# Check that RegistrationError is raised passing something not
-		# sub-classed from Command:
+		# Check that RegistrationError is raised when registering anything
+		# other than a subclass of Command:
+		for obj in [object, cmd_my_command()]:
+			raised = False
+			try:
+				api.register_command(obj)
+			except exceptions.RegistrationError:
+				raised = True
+			assert raised
+
+		# Check that command registration works:
+		api.register_command(cmd_my_command)
+		api.register_command(cmd_another_command)
+
+		# Check that DuplicateError is raised when registering the same class
+		# twice:
 		raised = False
 		try:
-			api.register_command(object)
-		except exceptions.RegistrationError:
+			api.register_command(cmd_my_command)
+		except exceptions.DuplicateError:
 			raised = True
 		assert raised
+
+		# Check that OverrideError is raised when registering same name
+		# without override = True:
+		class cmd_my_command(base.Command):
+			pass
+		raised = False
+		try:
+			api.register_command(cmd_my_command)
+		except exceptions.OverrideError:
+			raised = True
+		assert raised
+
+		# Check that override=True works:
+		api.register_command(cmd_my_command, override=True)