sql_catalog: Drop usage of getattr from SQLCatalog

This would increase compatibility between accessors in erp5_catalog and
remove the need of copy and patch. At the same time, it also helps in
increasing performance as using _getOb is always less costly than
getattr, as it reduces the need to look in acquisition.

One of the difference between SQL and ERP5 Catalog is how they
handle their properties. For ERP5Catalog, we use property_sheets
which generate setters and getters, for SQLCatalog, properties
just act as attributes(as one can expect from a property set
on a class).

This creates a difference on how we use them, especially for
list_type properties, which have accessors like get<PropertyName>List
to get the list of objects. For SQLCatalog, there is no such thing.
This creates problem whereever 'get<PropertyName>' or getProperty(<name>)
was being used to get multiple property types.

product/ERP5/bootstrap/erp5_mysql_innodb_catalog/CatalogMethodTemplateItem/portal_catalog/erp5_mysql_innodb/SQLCatalog_makeTranslatedTitleQuery.py

@@ -3,8 +3,8 @@ portal = context.getPortalObject()
 
 # This scriptable key supports content_translation if the table is present
 catalog = portal.portal_catalog.getSQLCatalog()
-if 'content_translation' in catalog.getProperty('sql_search_tables'):
-  if [x for x in catalog.getProperty('sql_catalog_search_keys', []) if 'Mroonga' in x]:
+if 'content_translation' in getattr(catalog, 'sql_search_tables', ()):
+  if any('Mroonga' in x for x in catalog.getSqlCatalogSearchKeysList()):
     return AndQuery(SimpleQuery(**{'content_translation.translated_text': value, 'comparison_operator': 'mroonga_boolean'}),
                     Query(**{'content_translation.property_name': 'title'}))
   else:

product/ERP5Configurator/Document/CatalogKeywordKeyConfiguratorItem.py

@@ -59,7 +59,7 @@ class CatalogKeywordKeyConfiguratorItem(ConfiguratorItemMixin, XMLObject):
     error_list = []
     portal = self.getPortalObject()
     catalog = portal.portal_catalog.getSQLCatalog()
-    key_list = list(catalog.getProperty('sql_catalog_keyword_search_keys', ()))
+    key_list = list(catalog.getSqlCatalogSearchKeysList())
     for k in self.key_list:
       if k not in key_list:
         error_list.append(self._createConstraintMessage(

product/ZSQLCatalog/SQLCatalog.py

@@ -909,12 +909,16 @@ class Catalog(Folder,
     self.subject_set_uid_dict[subject_list] = subject_set_uid
     return (subject_set_uid, subject_list)
 
+  def getSqlClearCatalogList(self):
+    return self.sql_clear_catalog
+
   def _clear(self):
     """
     Clears the catalog by calling a list of methods
     """
-    for method_name in self.sql_clear_catalog:
-      method = getattr(self, method_name)
+    method_id_list = self.getSqlClearCatalogList()
+    for method_name in method_id_list:
+      method = self._getOb(method_name)
       try:
         method()
       except ConflictError:
@@ -935,23 +939,29 @@ class Catalog(Folder,
     self._clearSubjectCache()
     self._clearCaches()
 
+  def getSqlCatalogReservedUid(self):
+    return self.sql_catalog_reserve_uid
+
   security.declarePrivate('insertMaxUid')
   def insertMaxUid(self):
     """
       Add a dummy item so that SQLCatalog will not use existing uids again.
     """
     if self._max_uid is not None and self._max_uid() != 0:
-      method_id = self.sql_catalog_reserve_uid
-      method = getattr(self, method_id)
+      method_id = self.getSqlCatalogReservedUid()
+      method = self._getOb(method_id)
       self._max_uid.change(1)
       method(uid = [self._max_uid()])
 
+  def getSqlCatalogClearReserved(self):
+    return self.sql_catalog_clear_reserved
+
   def _clearReserved(self):
     """
     Clears reserved uids
     """
-    method_id = self.sql_catalog_clear_reserved
-    method = getattr(self, method_id)
+    method_id = self.getSqlCatalogClearReserved()
+    method = self._getOb(method_id)
     try:
       method()
     except ConflictError:
@@ -963,6 +973,9 @@ class Catalog(Folder,
       raise
     self._last_clear_reserved_time += 1
 
+  def getSqlGetitemByUid(self):
+    return self.sql_getitem_by_uid
+
   security.declarePrivate('getRecordForUid')
   def getRecordForUid(self, uid):
     """
@@ -977,7 +990,7 @@ class Catalog(Folder,
     # It could also have a performance impact for traversals to objects in
     # the acquisition context on Zope 2.12 even when it didn't raise a weird
     # error.
-    method = getattr(self,  self.sql_getitem_by_uid)
+    method = self._getOb(self.getSqlGetitemByUid())
     search_result = method(uid = uid)
     if len(search_result) > 0:
       return search_result[0]
@@ -1003,23 +1016,35 @@ class Catalog(Folder,
     self.schema = schema
     self.names = names
 
+  def getSqlSearchTablesList(self):
+    return list(self.sql_search_tables)
+
   security.declarePrivate('getCatalogSearchTableIds')
   def getCatalogSearchTableIds(self):
     """Return selected tables of catalog which are used in JOIN.
        catalaog is always first
     """
-    search_tables = list(self.sql_search_tables) or ['catalog']
+    search_tables = self.getSqlSearchTablesList() or ['catalog']
     if search_tables[0] != 'catalog':
       search_tables = ['catalog'] + [x for x in search_tables if x != 'catalog']
       # XXX: cast to tuple to avoid a mutable persistent property ?
       self.sql_search_tables = search_tables
     return search_tables
 
+  def getSqlSearchResultKeysList(self):
+    return self.sql_search_result_keys
+
   security.declarePublic('getCatalogSearchResultKeys')
   def getCatalogSearchResultKeys(self):
     """Return search result keys.
     """
-    return self.sql_search_result_keys
+    return self.getSqlSearchResultKeysList()
+
+  def getSqlCatalogMultiSchema(self):
+    return self.sql_catalog_multi_schema
+
+  def getSqlCatalogSchema(self):
+    return self.sql_catalog_schema
 
   @transactional_cache_decorator('SQLCatalog._getCatalogSchema')
   def _getCatalogSchema(self):
@@ -1031,7 +1056,7 @@ class Catalog(Folder,
               "than sql_catalog_multi_schema. It makes many SQL queries "
               "instead of one",
               DeprecationWarning)
-      method_name = self.sql_catalog_schema
+      method_name = self.getSqlCatalogSchema()
       try:
         method = getattr(self, method_name)
       except AttributeError:
@@ -1148,6 +1173,9 @@ class Catalog(Folder,
       uid_buffer_dict[thread_key] = UidBuffer()
     return uid_buffer_dict[thread_key]
 
+  def getSqlCatalogProduceReserved(self):
+    return self.sql_catalog_produce_reserved
+
   # the cataloging API
   security.declarePrivate('produceUid')
   def produceUid(self):
@@ -1177,8 +1205,8 @@ class Catalog(Folder,
         # already keeps track of the highest allocated number for all id
         # generator groups.
       else:
-        method_id = self.sql_catalog_produce_reserved
-        method = getattr(self, method_id)
+        method_id = self.getSqlCatalogProduceReserved()
+        method = self._getOb(method_id)
         # Generate an instance id randomly. Note that there is a small possibility that this
         # would conflict with others.
         random_factor_list = [time.time(), os.getpid(), os.times()]
@@ -1411,6 +1439,9 @@ class Catalog(Folder,
                               check_uid=check_uid,
                               idxs=idxs)
 
+  def getSqlCatalogObjectListList(self):
+    return self.sql_catalog_object_list
+
   def _catalogObjectList(self, object_list, method_id_list=None,
                          disable_cache=0, check_uid=1, idxs=None):
     """This is the real method to catalog objects.
@@ -1453,7 +1484,10 @@ class Catalog(Folder,
     assigned_uid_dict = {}
 
     for object in object_list:
-      uid = getattr(aq_base(object), 'uid', None)
+      try:
+        uid = aq_base(object).uid
+      except AttributeError:
+        uid = None
       # Generate unique uid for object having 0 or None as uid
       if uid is None or uid == 0:
         try:
@@ -1555,7 +1589,7 @@ class Catalog(Folder,
         assigned_uid_dict[uid] = object
 
     if method_id_list is None:
-      method_id_list = self.sql_catalog_object_list
+      method_id_list = self.getSqlCatalogObjectListList()
     econtext = getEngine().getContext()
     if disable_cache:
       argument_cache = DummyDict()
@@ -1671,6 +1705,9 @@ class Catalog(Folder,
   def _getCatalogMethod(self, method_name):
     return getattr(self, method_name)
 
+  def getSqlCatalogDeleteUid(self):
+     return self.sql_catalog_delete_uid
+
   security.declarePrivate('beforeUncatalogObject')
   def beforeUncatalogObject(self, path=None,uid=None):
     """
@@ -1681,7 +1718,7 @@ class Catalog(Folder,
 
     if uid is None and path is not None:
       uid = self.getUidForPath(path)
-    method_name = self.sql_catalog_delete_uid
+    method_name = self.getSqlCatalogDeleteUid()
     if uid is None:
       return None
     if method_name in (None,''):
@@ -1689,9 +1726,12 @@ class Catalog(Folder,
       LOG('ZSQLCatalog.beforeUncatalogObject',0,'The sql_catalog_delete_uid'\
                                                 + ' method is not defined')
       return self.uncatalogObject(path=path,uid=uid)
-    method = getattr(self, method_name)
+    method = self._getOb(method_name)
     method(uid = uid)
 
+  def getSqlUncatalogObjectList(self):
+    return self.sql_uncatalog_object
+
   security.declarePrivate('uncatalogObject')
   def uncatalogObject(self, path=None, uid=None):
     """
@@ -1711,29 +1751,35 @@ class Catalog(Folder,
 
     if uid is None and path is not None:
       uid = self.getUidForPath(path)
-    methods = self.sql_uncatalog_object
+    method_id_list = self.getSqlUncatalogObjectList()
     if uid is None:
       return None
-    for method_name in methods:
+    for method_name in method_id_list:
       # Do not put try/except here, it is required to raise error
       # if uncatalog does not work.
-      method = getattr(self, method_name)
+      method = self._getOb(method_name)
       method(uid = uid)
 
+  def getSqlCatalogTranslationList(self):
+    return self.sql_catalog_translation_list
+
   security.declarePrivate('catalogTranslationList')
   def catalogTranslationList(self, object_list):
     """Catalog translations.
     """
-    method_name = self.sql_catalog_translation_list
+    method_name = self.getSqlCatalogTranslationList()
     return self.catalogObjectList(object_list, method_id_list = (method_name,),
                                   check_uid=0)
 
+  def getSqlDeleteTranslationList(self):
+    return self.sql_delete_translation_list
+
   security.declarePrivate('deleteTranslationList')
   def deleteTranslationList(self):
     """Delete translations.
     """
-    method_name = self.sql_delete_translation_list
-    method = getattr(self, method_name)
+    method_name = self.getSqlDeleteTranslationList()
+    method = self._getOb(method_name)
     try:
       method()
     except ConflictError:
@@ -1741,18 +1787,27 @@ class Catalog(Folder,
     except:
       LOG('SQLCatalog', WARNING, 'could not delete translations', error=sys.exc_info())
 
+  def getSqlUniqueValues(self):
+    return self.sql_unique_values
+
   security.declarePrivate('uniqueValuesFor')
   def uniqueValuesFor(self, name):
     """ return unique values for FieldIndex name """
-    method = getattr(self, self.sql_unique_values)
+    method = self._getOb(self.getSqlUniqueValues())
     return method(column=name)
 
+  def getSqlCatalogPaths(self):
+    return self.sql_catalog_paths
+
   security.declarePrivate('getPaths')
   def getPaths(self):
     """ Returns all object paths stored inside catalog """
-    method = getattr(self, self.sql_catalog_paths)
+    method = self._getOb(self.getSqlCatalogPaths())
     return method()
 
+  def getSqlGetitemByPath(self):
+    return self.sql_getitem_by_path
+
   security.declarePrivate('getUidForPath')
   def getUidForPath(self, path):
     """ Looks up into catalog table to convert path into uid """
@@ -1763,9 +1818,8 @@ class Catalog(Folder,
     """ Looks up into catalog table to convert path into uid """
     return  {
       x.path: x.uid
-      for x in getattr(
-        self,
-        self.sql_getitem_by_path,
+      for x in self._getOb(
+        self.getSqlGetitemByPath()
       )(
         path=None,
         path_list=path_list,
@@ -1778,9 +1832,8 @@ class Catalog(Folder,
     """ Looks up into catalog table to convert uid into path """
     return  {
       x.uid: x.path
-      for x in getattr(
-        self,
-        self.sql_getitem_by_uid,
+      for x in self._getOb(
+        self.getSqlGetitemByUid()
       )(
         uid=None,
         uid_list=uid_list,
@@ -1972,10 +2025,22 @@ class Catalog(Folder,
       related_key_definition_cache[key] = result
     return result
 
+  def getSqlCatalogKeywordSearchKeysList(self):
+    return self.sql_catalog_keyword_search_keys
+
+  def getSqlCatalogFullTextSearchKeysList(self):
+    return self.sql_catalog_full_text_search_keys
+
+  def getSqlCatalogDatetimeSearchKeysList(self):
+    return self.sql_catalog_datetime_search_keys
+
+  def getSqlCatalogScriptableKeysList(self):
+    return self.sql_catalog_scriptable_keys
+
   @transactional_cache_decorator('SQLCatalog._getgetScriptableKeyDict')
   def _getgetScriptableKeyDict(self):
     result = {}
-    for scriptable_key_definition in self.sql_catalog_scriptable_keys:
+    for scriptable_key_definition in self.getSqlCatalogScriptableKeysList():
       split_scriptable_key_definition = scriptable_key_definition.split('|')
       if len(split_scriptable_key_definition) != 2:
         LOG('SQLCatalog', WARNING, 'Malformed scriptable key definition: %r. Ignored.' % (scriptable_key_definition, ))
@@ -2462,13 +2527,16 @@ class Catalog(Folder,
     kw['order_by_list'] = order_by_list or []
     return kw
 
+  def getSqlCatalogSearchKeysList(self):
+    return self.sql_catalog_search_keys
+
   @transactional_cache_decorator('SQLCatalog._getSearchKeyDict')
   def _getSearchKeyDict(self):
     result = {}
     search_key_column_dict = {
-      'KeywordKey': self.sql_catalog_keyword_search_keys,
-      'FullTextKey': self.sql_catalog_full_text_search_keys,
-      'DateTimeKey': self.sql_catalog_datetime_search_keys,
+      'KeywordKey': self.getSqlCatalogKeywordSearchKeysList(),
+      'FullTextKey': self.getSqlCatalogFullTextSearchKeysList(),
+      'DateTimeKey': self.getSqlCatalogDatetimeSearchKeysList(),
     }
     for key, column_list in search_key_column_dict.iteritems():
       for column in column_list:
@@ -2476,7 +2544,7 @@ class Catalog(Folder,
           LOG('SQLCatalog', WARNING, 'Ambiguous configuration: column %r is set to use %r key, but also to use %r key. Former takes precedence.' % (column, result[column], key))
         else:
           result[column] = key
-    for line in self.sql_catalog_search_keys:
+    for line in self.getSqlCatalogSearchKeysList():
       try:
         column, key = [x.strip() for x in line.split('|', 2)]
         result[column] = key
@@ -2545,9 +2613,12 @@ class Catalog(Folder,
       limit_expression=query['limit_expression'],
     )
 
+  def getSqlSearchResults(self):
+    return self.sql_search_results
+
   security.declarePrivate('getSearchResultsMethod')
   def getSearchResultsMethod(self):
-    return getattr(self, self.sql_search_results)
+    return self._getOb(self.getSqlSearchResults())
 
   security.declarePrivate('searchResults')
   def searchResults(self, REQUEST=None, **kw):
@@ -2565,9 +2636,12 @@ class Catalog(Folder,
 
   __call__ = searchResults
 
+  def getSqlCountResults(self):
+    return self.sql_count_results
+
   security.declarePrivate('getCountResultsMethod')
   def getCountResultsMethod(self):
-    return getattr(self, self.sql_count_results)
+    return self._getOb(self.getSqlCountResults())
 
   security.declarePrivate('countResults')
   def countResults(self, REQUEST=None, **kw):
@@ -2584,28 +2658,37 @@ class Catalog(Folder,
   def isAdvancedSearchText(self, search_text):
     return isAdvancedSearchText(search_text, self.isValidColumn)
 
+  def getSqlRecordObjectList(self):
+    return self.sql_record_object_list
+
   security.declarePrivate('recordObjectList')
   def recordObjectList(self, path_list, catalog=1):
     """
       Record the path of an object being catalogged or uncatalogged.
     """
-    method = getattr(self, self.sql_record_object_list)
+    method = self._getOb(self.getSqlRecordObjectList())
     method(path_list=path_list, catalog=catalog)
 
+  def getSqlDeleteRecordedObjectList(self):
+    return self.sql_delete_recorded_object_list
+
   security.declarePrivate('deleteRecordedObjectList')
   def deleteRecordedObjectList(self, uid_list=()):
     """
       Delete all objects which contain any path.
     """
-    method = getattr(self, self.sql_delete_recorded_object_list)
+    method = self._getOb(self.getSqlDeleteRecordedObjectList())
     method(uid_list=uid_list)
 
+  def getSqlReadRecordedObjectList(self):
+    return self.sql_read_recorded_object_list
+
   security.declarePrivate('readRecordedObjectList')
   def readRecordedObjectList(self, catalog=1):
     """
       Read objects. Note that this might not return all objects since ZMySQLDA limits the max rows.
     """
-    method = getattr(self, self.sql_read_recorded_object_list)
+    method = self._getOb(self.getSqlReadRecordedObjectList())
     return method(catalog=catalog)
 
   security.declarePublic('getConnectionId')
@@ -2620,6 +2703,34 @@ class Catalog(Folder,
         if 'deferred' in method.connection_id != deferred:
           return method.connection_id
 
+  def getSqlCatalogObjectList(self):
+    try:
+      method_id_list = self.sql_catalog_object
+    except AttributeError:
+      method_id_list = ()
+    return method_id_list
+
+  def getSqlUncatalogObjectList(self):
+    try:
+      method_id_list = self.sql_uncatalog_object
+    except AttributeError:
+      method_id_list = ()
+    return method_id_list
+
+  def getSqlUpdateObjectList(self):
+    try:
+      method_id_list = self.sql_update_object
+    except AttributeError:
+      method_id_list = ()
+    return method_id_list
+
+  def getSqlCatalogObjectListList(self):
+    try:
+      method_id_list = self.sql_catalog_object_list
+    except AttributeError:
+      method_id_list = ()
+    return method_id_list
+
   security.declarePrivate('getFilterableMethodList')
   def getFilterableMethodList(self):
     """
@@ -2627,11 +2738,11 @@ class Catalog(Folder,
     """
     method_dict = {}
     if withCMF:
-      methods = getattr(self,'sql_catalog_object',()) + \
-                getattr(self,'sql_uncatalog_object',()) + \
-                getattr(self,'sql_update_object',()) + \
-                getattr(self,'sql_catalog_object_list',())
-      for method_id in methods:
+      method_id_list =  self.getSqlCatalogObjectList() +\
+                        self.getSqlUncatalogObjectList() +\
+                        self.getSqlUpdateObjectList() +\
+                        self.getSqlCatalogObjectListList()
+      for method_id in method_id_list:
         method_dict[method_id] = 1
     method_list = map(lambda method_id: getattr(self, method_id, None), method_dict.keys())
     return filter(lambda method: method is not None, method_list)