Refactor Catalog Indexing

[ramonski]

Description of the issue/feature this PR addresses

This PR refactors the catalog indexing by using the IndexQueue for all SENAITE catalogs and avoids multiple UID catalog indexing.

Current behavior before PR

AT object indexing never use the IndexQueue and temporary objects were indexed in UID catalog

Desired behavior after PR is merged

AT object indexing always use the IndexQueue and no temporary objects are indexed in UID catalog

--
I confirm I have tested this PR thoroughly and coded it according to PEP8
and Plone's Python styleguide standards.

[xispa]

Awesome!, thanks for the doctest and README's (really useful and necessary). Have left some comments and questions

[xispa]

To not spread here and there same thing, I suggest to rely on api.catalog_object function, that already takes care of uid_catalog here: https://github.com/senaite/senaite.core/blob/2.x/src/bika/lims/api/__init__.py#L403-L417

We could even include the recursive param in api.catalog_object function as well and remove this reindex function from here. What do you think?

[ramonski]

Thanks for the hint! I already forgot that we have an API function that does this.
Changes done in c725d9d

[xispa]

Note that by default api.get_catalogs_for relies on this static mapping to get the catalogs for a given portal type. And the value of the attribute _catalog is taken into account only when the portal type of the given object is not present in that mapping: https://github.com/senaite/senaite.core/blob/2.x/src/bika/lims/api/__init__.py#L1276

In my opinion this static mapping forces us to monkey patch api.get_catalogs_for if we want an existing baseline type (like AnalysisRequest) to also be catalogued in a brand-new-add-on-specific catalog.

So, I think this change is fine as long as we skip the static mapping check in api.get_catalogs_for. Or maybe don't skip, but rely on a new adapter to get additional catalogs for baseline portal_types.

[ramonski]

Yes, you are right and this is why we have the TODO in the docstring here:

senaite.core/src/senaite/core/catalog/__init__.py

Lines 107 to 120 in c725d9d

	def get_catalogs_by_type(portal_type):
	"""Return the mapped catalogs by type
	TODO: Provide registry setting for this mapping lookup
	:param portal_type: The portal type to look up
	"""
	if not isinstance(portal_type, str):
	raise TypeError("Expected string type, got <%s>" % type(portal_type))
	mapping = dict(CATALOG_MAPPINGS)
	catalogs = mapping.get(portal_type)
	if not catalogs:
	return []
	return catalogs

I guess providing the mapping as a registry setting we could easily add/remove/edit catalogs from the base types and new types.

For the moment I would like to postpone that until we have this requirement and provide it in a new PR

[xispa]

Note for @xispa : ._getURL() relies on Archetypes.utils.getRelPath, that returns the relative path of the object from the portal root. The result is the same as url = "/".join(obj.getPhysicalPath()[2:])

[xispa]

Shouldn't we also check for api.is_temporary(obj), like what you did in catalog_multiplex?

[ramonski]

Everything that ends up in this queue comes from Products.CMFCore.CatalogTool.reindexObject which does already filterTemporaryItems, which uses our patched isTemporary for both AT and DX types:

senaite.core/src/senaite/core/patches/archetypes/base_object.py

Lines 29 to 30 in c725d9d

	def isTemporary(self):
	return api.is_temporary(self)

senaite.core/src/senaite/core/patches/dexterity/dexterity_content.py

Lines 29 to 30 in c725d9d

	def isTemporary(self):
	return api.is_temporary(self)

So another api.is_temporary is at this stage not required

[xispa]

Excellent, thanks!