erp5_catalog: Use getattr to get catalog attributes

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.

So, we changed this to rely on getattr for the Python Scripts in this
commit. Note that, getattr isn't good if we care about performance,
but given that its used only twice, this can be better than adding
extra function overhead.

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 getattr(catalog, 'sql_catalog_search_keys', ())):
     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(getattr(catalog, 'sql_catalog_keyword_search_keys', ()))
     for k in self.key_list:
       if k not in key_list:
         error_list.append(self._createConstraintMessage(

[Vincent Pelletier]

I'm a bit surprised by this need: ZSQLCatalog does declare properties on the class, so I would expect these to be automatically added to property sheet mechanism. Maybe an inheritance override is needed here ? @jm: ideas ?

If it really is the right way, I think this commit could go in without waiting for the merge request.

[Ayush Tiwari]

The need for this is after we migrate from ZSQLCatalog to ERP5Catalog. For ERP5Catalog, the catalog.getProperty('sql_catalog_keyword_search_keys', ()) will just end up giving the first object of the list, as the way getProperty works, it first checks for the type of the property and if there is any accessors already generated, then it prefers that.

So, if we want to maintain the compatibilty here, we could have done something like catalog.getProperty('sql_catalog_keyword_search_keys_list', ()) and then create a function in ZSQLCatalog to take care of getting the property ending with _list.

More simple way to explain is that, for non-ERP5 objects(in this case ZSQLCatalog), we don't have accessors ending with _list which creates the problem of using getProperty after migrating that object, thus need to this change.

The ideal case would have been to improve getProperty with checking the context itself, but I don't think we do want to check on each object and compromise on speed rather than this. I would need opinion on this . Thanks

[Vincent Pelletier]

will just end up giving the first object of the list

Or, right indeed.

Also, in another commit I later saw you do add a lot of getters, I think this one should probably just follow that other commit.

[Ayush Tiwari]

Yes, I just noticed that, makes more sense. This one : b76b4743 (comment 37129)

Thanks. Will update accordingly