Code factorization and create methods for EmComponent and childs changes

Behaviour changed for EmComponent and childs create method :
	- Takes care of duplicated names at creation
	- Handle all the checks in EmComponent method
Factorization of EmField.get_class_table() method
Added a @property method class_table_name to EmClass

EditorialModel/classes.py

@@ -5,7 +5,7 @@
 
 import logging as logger
 
-from EditorialModel.components import EmComponent, EmComponentNotExistError
+from EditorialModel.components import EmComponent, EmComponentNotExistError, EmComponentExistError
 from Database import sqlutils
 import sqlalchemy as sql
 
@@ -33,24 +33,17 @@ class EmClass(EmComponent):
     # @param name str: name of the new class
     # @param class_type EmClasstype: type of the class
     # @return An EmClass instance
+    # @throw EmComponentExistError if an EmClass with this name and a different classtype exists
+    # @todo Check class_type argument
     @classmethod
     def create(cls, name, class_type):
-        try:
-            res = EmClass(name)
-            logger.info("Trying to create an EmClass that allready exists")
-        except EmComponentNotExistError:
-            res = cls._create_db(name, class_type)
-            logger.debug("EmClass successfully created")
-
-        return res
+        return cls._create_db(name, class_type)
 
     @classmethod
     ## Isolate SQL for EmClass::create
     # @todo Remove hardcoded default value for icon
     # @return An instance of EmClass
     def _create_db(cls, name, class_type):
-        """ Do the db querys for EmClass::create() """
-
         #Create a new entry in the em_class table
         values = {'name': name, 'classtype': class_type['name'], 'icon': 0}
         resclass = super(EmClass, cls).create(**values)
@@ -60,13 +53,18 @@ class EmClass(EmComponent):
 
         #Create a new table storing LodelObjects of this EmClass
         meta = sql.MetaData()
-        emclasstable = sql.Table(name, meta, sql.Column('uid', sql.VARCHAR(50), primary_key=True))
+        emclasstable = sql.Table(resclass.class_table_name, meta, sql.Column('uid', sql.VARCHAR(50), primary_key=True))
         emclasstable.create(conn)
 
         conn.close()
 
         return resclass
 
+    @property
+    ## @brief Return the table name used to stores data on this class
+    def class_table_name(self):
+        return self.name
+
     ## @brief Delete a class if it's ''empty''
     # If a class has no fieldgroups delete it
     # @return bool : True if deleted False if deletion aborded

EditorialModel/components.py

@@ -153,22 +153,40 @@ class EmComponent(object):
     # @param **kwargs : Names arguments representing object properties
     # @return An instance of the created component
     # @throw TypeError if an element of kwargs isn't a valid object propertie or if a mandatory argument is missing
-    #
+    # @throw RuntimeError if the creation fails at database level
     # @todo Check that the query didn't failed
     # @todo Check that every mandatory _fields are given in args
     # @todo Put a real rank at creation
     # @todo Stop using datetime.datetime.utcnow() for date_update and date_create init
     @classmethod
     def create(cls, **kwargs):
+        #Checking for invalid arguments
+        valid_args = [ name for name,_ in (cls._fields + EmComponent._fields)]
+
         for argname in kwargs:
             if argname in ['date_update', 'date_create', 'rank', 'uid']:  # Automatic properties
                 raise TypeError("Invalid argument : " + argname)
+            elif argname not in valid_args:
+                raise TypeError("Unexcepted keyword argument '"+argname+"' for "+cls.__name__+" creation")
+                
+        #Check uniq names constraint
+        try:
+            name = kwargs['name']
+            exist = cls(name)
+            for kname in kwargs:
+                if not (getattr(exist, kname) == kwargs[kname]):
+                    raise EmComponentExistError("An "+cls.__name__+" named "+name+" allready exists with a different "+kname)
+            logger.info("Trying to create an "+cls.__name__+" that allready exist with same attribute. Returning the existing one")
+            return exist
+        except EmComponentNotExistError:
+            pass
 
         #TODO check that every mandatory _fields are here like below for example
         #for name in cls._fields:
         #    if cls._fields[name].notNull and cls._fields[name].default == None:
         #        raise TypeError("Missing argument : "+name)
 
+
         kwargs['uid'] = cls.new_uid()
         kwargs['date_update'] = kwargs['date_create'] = datetime.datetime.utcnow()
 
@@ -179,7 +197,8 @@ class EmComponent(object):
 
         table = sql.Table(cls.table, sqlutils.meta(dbe))
         req = table.insert(kwargs)
-        conn.execute(req)  # Check ?
+        if not conn.execute(req):
+            raise RuntimeError("Unable to create the "+cls.__class__.__name__+" EmComponent ")
         conn.close()
         return cls(kwargs['name'])  # Maybe no need to check res because this would fail if the query failed
 
@@ -419,6 +438,11 @@ class EmComponent(object):
 class EmComponentNotExistError(Exception):
     pass
 
+## @brief Raised on uniq constraint error at creation
+# This exception class is dedicated to be raised when create() method is called
+# if an EmComponent with this name but different parameters allready exist
+class EmComponentExistError(Exception):
+    pass
 
 ## @brief An exception class to tell that no ranking exist yet for the group of the object
 class EmComponentRankingNotExistError(Exception):

EditorialModel/fieldgroups.py

@@ -34,31 +34,18 @@ class EmFieldGroup(EmComponent):
     #
     # Save it in database and return an instance*
     # @param name str: The name of the new EmFieldGroup
-    # @param em_class EmClass|str|int : Can be an EditorialModel::classes::EmClass uid, name or instance
-    # @throw ValueError If an EmFieldGroup with this name allready exists
-    # @throw ValueError If the specified EmClass don't exists
+    # @param em_class EmClass : An EditorialModel::classes::EmClass instance
+    # @param **em_component_args : @ref EditorialModel::components::create()
+    # @throw EmComponentExistError If an EmFieldGroup with this name allready exists
     # @throw TypeError If an argument is of an unexepted type
-    def create(cls, name, em_class):
-        if not isinstance(em_class, EmClass):
-            if isinstance(em_class, int) or isinstance(em_class, str):
-                try:
-                    arg = em_class
-                    em_class = EmClass(arg)
-                except EmComponentNotExistError:
-                    raise ValueError("No EmClass found with id or name '" + arg + "'")
-            else:
-                raise TypeError("Excepting an EmClass, an int or a str for 'em_class' argument. Not an " + str(type(em_class)) + ".")
-
+    def create(cls, name, em_class, **em_component_args):
         if not isinstance(name, str):
-            raise TypeError("Excepting a string as first argument, not an " + str(type(name)))
+            raise TypeError("Excepting <class str> as name. But got " + str(type(name)))
+        if not isinstance(em_class, EmClass):
+            raise TypeError("Excepting <class EmClass> as em_class. But got "+str(type(name)))
 
-        try:
-            exists = EmFieldGroup(name)
-            raise ValueError("An EmFieldgroup named " + name + " allready exist")
-        except EmComponentNotExistError:
-            return super(EmFieldGroup, cls).create(name=name, class_id=em_class.uid)  # Check the return value ?
+        return super(EmFieldGroup, cls).create(name=name, class_id=em_class.uid, **em_component_args)
 
-        return exists
 
     ## Get the list of associated fields
     # @return A list of EmField instance

EditorialModel/fields.py

@@ -47,36 +47,30 @@ class EmField(EmComponent):
     # @param rel_to_type_id int: default=0
     # @param rel_field_id int: default=0
     # @param icon int: default=0
-    # @param kwargs dict: Dictionary of the values to insert in the field record
+    # @param **em_component_args : @ref EditorialModel::components::create()
     #
     # @throw TypeError
+    # @throw RuntimeError if the associated column creation fails
+    # @throw EmComponentExistError if an EmField with this name allready exists in this fieldgroup
     # @see EmComponent::__init__()
     # @staticmethod
     @classmethod
-    def create(cls, name, fieldgroup, fieldtype, optional=0, internal=0, rel_to_type_id=0, rel_field_id=0, icon=0):
-        try:
-            exists = EmField(name)
-        except EmComponentNotExistError:
-            values = {
-                'name': name,
-                'fieldgroup_id': fieldgroup.uid,
-                'fieldtype': fieldtype.name,
-                'optional': optional,
-                'internal': internal,
-                'rel_to_type_id': rel_to_type_id,
-                'rel_field_id': rel_field_id,
-                'icon': icon
-            }
-
-            created_field = super(EmField, cls).create(**values)
-            if created_field:
-                is_field_column_added = created_field.add_field_column_to_class_table()
-                if is_field_column_added:
-                    return created_field
-
-            exists = created_field
-
-        return exists
+    def create(cls, name, fieldgroup, fieldtype, optional=0, internal=0, rel_to_type_id=0, rel_field_id=0, icon=0, **em_component_args):
+        created_field = super(EmField, cls).create(
+            name=name,
+            fieldgroup_id=fieldgroup.uid,
+            fieldtype=fieldtype.name,
+            optional=optional,
+            internal=internal,
+            rel_to_type_id=rel_to_type_id,
+            rel_field_id=rel_field_id,
+            icon=icon,
+            **em_component_args
+        )
+        if not created_field.add_field_column_to_class_table():
+            raise RuntimeError("Unable to create the column for the EmField "+str(created_field))
+
+        return created_field
 
     ## @brief Delete a field if it's not linked
     # @return bool : True if deleted False if deletion aborded
@@ -109,24 +103,19 @@ class EmField(EmComponent):
     #
     # @return Name of the table
     def get_class_table(self):
-        return self._get_class_table_db()
-
-    ## _get_class_tableDb (Function)
-    #
-    # Executes a request to the database to get the name of the table in which to add the field
-    #
-    # @return Name of the table
-    def _get_class_table_db(self):
+        #return self._get_class_table_db()
+        return self.get_class().class_table_name
+    
+    ## @brief Get the class that contains this field
+    # @return An EmClass instance
+    def get_class(self):
+        #<SQL>
         dbe = self.db_engine()
         conn = dbe.connect()
-        field_group_table = sql.Table(EmFieldGroup.table, sqlutils.meta(dbe))
-        request_get_class_id = field_group_table.select().where(field_group_table.c.uid == self.fieldgroup_id)
-        result_get_class_id = conn.execute(request_get_class_id).fetchall()
-        class_id = dict(zip(result_get_class_id[0].keys(), result_get_class_id[0]))['class_id']
-
-        class_table = sql.Table(EmClass.table, sqlutils.meta(dbe))
-        request_get_class_table = class_table.select().where(class_table.c.uid == class_id)
-        result_get_class_table = conn.execute(request_get_class_table).fetchall()
-        class_table_name = dict(zip(result_get_class_table[0].keys(), result_get_class_table[0]))['name']
+        fieldgroup_table = sqlutils.getTable(EmFieldGroup)
+        req = fieldgroup_table.select().where(fieldgroup_table.c.uid == self.fieldgroup_id)
+        res = conn.execute(req)
+        row = res.fetchone()
+        #</SQL>
+        return EmClass(row['class_id'])
 
-        return class_table_name

EditorialModel/test/test_component.py

@@ -14,7 +14,7 @@ import unittest
 from EditorialModel.classes import EmClass
 from EditorialModel.classtypes import EmClassType
 
-from EditorialModel.components import EmComponent, EmComponentNotExistError
+from EditorialModel.components import EmComponent, EmComponentNotExistError, EmComponentExistError
 import EditorialModel.fieldtypes as ftypes
 
 from EditorialModel.test.utils import *
@@ -415,8 +415,33 @@ class TestCreate(ComponentTestCase):
         with self.assertRaises(TypeError, msg="But no rank_fam was given"):
             vals = { 'name': 'created1', 'string': '{"fr":"testcomp"}', 'help': '{"en" :"help test", "fr":"test help"}', 'rank': 6, 'date_create': 0 , 'date_update': 0 }
             tc = EmTestComp.create(**vals)
+        with self.assertRaises(TypeError, msg="But invalid keyword argument given"):
+            vals = {'invalid': 42, 'name': 'created1', 'rank_fam': 'f', 'string': '{"fr":"testcomp"}', 'help': '{"en":"help test", "fr":"test help"}'}
+            tc = EmTestComp.create(**vals)
+            
+        pass
+
+    def test_create_existing_failure(self):
+        """ Testing that create fails when trying to create an EmComponent with an existing name but different properties """
+        vals = {'name': 'created1', 'rank_fam': 'f', 'string': '{"fr":"testcomp"}', 'help': '{"en":"help test", "fr":"test help"}'}
+        tc = EmTestComp.create(**vals)
+        with self.assertRaises(EmComponentExistError, msg="Should raise because attribute differs for a same name"):
+            vals['rank_fam'] = 'e'
+            EmTestComp.create(**vals)
         pass
 
+    def test_create_existing(self):
+        """ Testing that create dont fails when trying to create twice the same EmComponent """
+        vals = {'name': 'created1', 'rank_fam': 'f', 'string': '{"fr":"testcomp"}', 'help': '{"en":"help test", "fr":"test help"}'}
+        tc = EmTestComp.create(**vals)
+        try:
+            tc2 = EmTestComp.create(**vals)
+        except EmComponentExistError as e:
+            self.fail("create raises but should return the existing EmComponent instance instead")
+        self.assertEqual(tc.uid, tc2.uid, "Created twice the same EmComponent")
+        pass
+        
+
 #====================#
 # EmComponent.delete #
 #====================#

EditorialModel/test/test_fieldgroups.py

@@ -134,8 +134,6 @@ class TestCreate(FieldGroupTestCase):
         params = {  'EmClass entity instance': EmClass('entity1'),
                     'EmClass entry instance': EmClass('entry1'),
                     'EmClass person instance': EmClass('person1'),
-                    'EmClass id': EmClass('entity1').uid,
-                    'EmClass name': 'entity2',
                 }
 
         for i,param_name in enumerate(params):
@@ -173,8 +171,7 @@ class TestCreate(FieldGroupTestCase):
         #Creating a fieldgroup to test duplicate name
         exfg = EmFieldGroup.create('existingfg', EmClass('entity1'))
 
-        badargs = { 'a duplicate name': ('existingfg', ValueError),
-                    'an integer': (42, TypeError),
+        badargs = { 'an integer': (42, TypeError),
                     'a function': (print, TypeError),
                     'an EmClass': (EmClass('entry1'), TypeError),
                 }

EditorialModel/types.py

@@ -32,19 +32,15 @@ class EmType(EmComponent):
     ## Create a new EmType and instanciate it
     # @param name str: The name of the new type
     # @param em_class EmClass: The class that the new type will specialize
+    # @param **em_component_args : @ref EditorialModel::components::create()
     # @return An EmType instance
-    #
+    # @throw EmComponentExistError if an EmType with this name but different attributes exists
     # @see EmComponent::__init__()
     # 
     # @todo Remove hardcoded default value for icon
     # @todo check that em_class is an EmClass object
-    def create(c, name, em_class):
-        try:
-            exists = EmType(name)
-        except EmComponentNotExistError:
-            return super(EmType, c).create(name=name, class_id=em_class.uid, icon=0)
-
-        return exists
+    def create(c, name, em_class, **em_component_args):
+        return super(EmType, c).create(name=name, class_id=em_class.uid, icon=0, **em_component_args)
     
     ## @brief Delete an EmType
     # The deletion is only possible if a type is not linked by any EmClass

Lodel/utils/mlstring.py

@@ -40,9 +40,12 @@ class MlString(object):
             return ""
 
     ## Test if two MlString instance are equivalent
-    # @param other MlString : Another MlString instance
+    # @param other MlString|str : Another MlString instance or a string (json formated)
     # @return True or False
     def __eq__(self, other):
+        if isinstance(other, str):
+            other = MlString.load(other)
+
         if not isinstance(other, MlString):
             return False
         if not set(lng for lng in self.translations) == set( lng for lng in other.translations):