Request for comments: Reducing functionality of z3c.relationfield

do3cc · November 24, 2015, 1:14pm

Hi,

in one of our products we have removed plone.app.imagecropping. First testing did not show problems, but then content editor stumbled over keyerrors related to z3c.relationfield.
We hastily readded plone.app.imagecropping, somehow accidentally not looking up the version. We got the latest plone.app.imagecropping. From pre 1.0 to 1.0 an interface class has been renamed. The upgrade step reindexed the zope catalog reindexing also triggered the reindexing of the relations catalog. But the reindex is imcomplete. I have no reference to the old interface class and the new interface.

I did not stumble over this the first time and once again I was also wondering, why z3c.relationfield creates a catalog with the indexes "from_interfaces" and "to_interfaces". The indexes are implemented as BTrees and the BTrees documentation explicitly says not to use Persistent Objects as keys: http://pythonhosted.org/BTrees/#total-ordering-and-persistence But I do not even find any place, where these two indexes are in use.

So my proposal is to remove these two fields. Opinions? Is somebody maybe using this feature?

jensens · November 24, 2015, 1:57pm

m(

So, why the hack is this there an own catalog? Crazy, first we had the own catalog for Archetypes, then all people complained. Then we introduced the new shiny relations and its again using its own Catalog:

github.com

zopefoundation/z3c.relationfield/blob/master/src/z3c/relationfield/index.py#L23


    'element': IRelationValue['to_id'],
},
{
    'element': IRelationValue['from_attribute'],
    'kwargs': {
        'btree': BTrees.family32.OI,
    },
},
{
    'element': IRelationValue['from_interfaces_flattened'],
    'kwargs': {
        'btree': BTrees.family32.OI,
        'multiple': True,
    },
},
{
    'element': IRelationValue['to_interfaces_flattened'],
    'kwargs': {
        'btree': BTrees.family32.OI,
        'multiple': True,
    },

github.com

zopefoundation/zc.relation/blob/master/src/zc/relation/catalog.py

##############################################################################
#
# Copyright (c) 2006-2008 Zope Foundation and Contributors.
# All Rights Reserved.
#
# This software is subject to the provisions of the Zope Public License,
# Version 2.1 (ZPL).  A copy of the ZPL should accompany this distribution.
# THIS SOFTWARE IS PROVIDED "AS IS" AND ANY AND ALL EXPRESS OR IMPLIED
# WARRANTIES ARE DISCLAIMED, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
# WARRANTIES OF TITLE, MERCHANTABILITY, AGAINST INFRINGEMENT, AND FITNESS
# FOR A PARTICULAR PURPOSE.
#
##############################################################################
import copy
import sys

import zope.interface
import zope.interface.interfaces

import BTrees

This file has been truncated. show original

Which is not any known Catalog implementation (we have such in Zope2, Zope3 and repoze catalog) but an own, even introducing own query operators.

Overall a big +1 for removal of the two interface indexes. Moreover I'd be +100 to make it a simple index in the classical Catalog. This way we could reindex it as usal, put it into solr or elasticsearch if needed, do whatever future refactorings need.

do3cc · November 24, 2015, 3:47pm

The Zope catalog indexes objects, the relation catalog indexes relations. There can be many relations for a single object. For proper event handling, at least three attributes for each relation are required. source, target and the attribute in which the value is stored in source.
I am not sure, how to store this in the zope catalog.
But IntIds could easily be kicked, also the catalog could provide a simple explicit interface to make solr backend feasible

davisagli · November 24, 2015, 5:06pm

I don't think zc.catalog and friends were ever officially approved for core; they kind of snuck in as part of plone.app.contenttypes. :-[

A while back I started a RelationIndex package to index relations in the ZCatalog, no matter if they originate as Archetypes references or z3c.relationfield RelationValues or something else. It's only about half done and I lost time/motivation, but I think the approach can work: https://github.com/davisagli/Products.RelationIndex

One advantage to using a separate catalog (true for both AT reference_catalog and the zc.relation catalog) is you can potentially store relations to objects that are not content items. For example, a relation between a portlet assignment and a content item it references. I don't know how many people are doing this in practice.

jensens · November 25, 2015, 10:34am

@davisagli I really like your RelationIndex. I'am not sure if we want to support relations to/between non-content. Its a rare case and makes things complex.

What would be the tasks to make it part of Plone core replacing the current zc relations catalog?