Commit 4adb0eb1 authored by Sylvain Thénault's avatar Sylvain Thénault
Browse files

[hooks] Ensure we don't have several children with cardinality != 1

By trying to validate several profiles using Jing (in Asalae), it appears that
it isn't possible to validate a profile where there are several elements with
the same name with optional or/and multiple cardinality, e.g. with a profile
telling we expect  1..n Document followed by 1 Document, a transfer with two
documents won't be accepted, telling a required document is missing. This is
much probably because the validator "eats" the two documents for the first rule,
then it misses one to satisfy the second rule.

In order to avoid generating such insatisfyable profiles, while not changing
much the UI for now, we decided to add hook preventing usage of several sibling
children with a cardinality != 1 during edition, and then at RNG profile
generation time to put the one with cardinality != 1 as last children. That way
profiles should always be validable.

This patch introduces the hooks preventing insatisfyable profiles to be created.
The next one will ensure the order of RNG export.

Two hooks are added to check for unhandled case one for new watched relations
and the other for update cardinalities.

Added test cases for the 3 most prominent cases: archive unit, document and
keywords, with more coverage in the first one and only simple check for the two
others. Also, some test have to be updated to follow this change. Notice diff of
exported RNG isn't nice to read but is actually only removal of a surrounding
"oneOrMore" element.

Related to #17098404
parent fcf239b4e613
......@@ -26,7 +26,8 @@ from cubicweb.predicates import is_instance, score_entity
from cubicweb.server import hook
from .entities import rule_type_from_etype, diag
from .entities.generated import CHOICE_RTYPE
from .entities.generated import (CHOICE_RTYPE,
CHECK_CARD_ETYPES, CHECK_CHILDREN_CARD_RTYPES)
SEDA_PARENT_RTYPES = {}
......@@ -373,6 +374,78 @@ class UpdateWatchedProfileElementHook(hook.Hook):
CheckProfileSEDACompatiblityOp.get_instance(self._cw).add_entity(self.entity)
class AddedCardinalityCheckedRelationHook(hook.Hook):
"""Some relations whose children should be checked for unhandled cardinality has
been added.
"""
__regid__ = 'seda.transfer.checkcard.relations'
__select__ = hook.Hook.__select__ & hook.match_rtype_sets(set(CHECK_CHILDREN_CARD_RTYPES))
events = ('after_add_relation',)
category = 'integrity'
def __call__(self):
CheckChildrenUnhandledCardinalityOp.get_instance(self._cw).add_data(
(self.eidto, self.rtype, self.eidfrom))
class UpdatedCardinalityCheckedEntityHook(hook.Hook):
"""Some entity whose cardinality should be checked for unhandled cardinality has
been updated.
"""
__regid__ = 'seda.transfer.checkcard.entities'
__select__ = hook.Hook.__select__ & is_instance(*CHECK_CARD_ETYPES)
events = ('after_update_entity',)
category = 'integrity'
def __call__(self):
# nothing to do if cardinality isn't edited or new value is '1'
if ('user_cardinality' not in self.entity.cw_edited
or self.entity.user_cardinality == '1'):
return
# back to its parent entity
icontained = self.entity.cw_adapt_to('IContained')
if icontained is None or not icontained.parent:
# entity is being created in the transaction, should be catched by
# AddedCardinalityCheckedRelationHook
return
parent = icontained.parent
rtype, role = icontained.parent_relation()
CheckChildrenUnhandledCardinalityOp.get_instance(self._cw).add_data(
(parent.eid, rtype, self.entity.eid))
class CheckChildrenUnhandledCardinalityOp(hook.DataOperationMixIn, hook.Operation):
"""Check we don't run into the case of an with more than one child through a
same relation with `user_cardinality != '1'` (otherwise it won't be
validable by RelaxNG validator like Jing).
Expect (parent_eid, rtype, error_entity_eid) to be added into the data container:
* eid of the parent whose child should be checked,
* name of the relation to retrieve its children (expected to be an object
relation),
* eid of the entity on which the `ValidationError` will be raised in case of
error.
"""
def precommit_event(self):
for parent_eid, rtype, added_entity_eid in self.get_data():
parent = self.cnx.entity_from_eid(parent_eid)
has_non_single_cardinality = False
# sort to attempt to return the child with the greatest eid
for child in sorted(parent.related(rtype, 'object', entities=True),
key=lambda x: x.eid):
if child.user_cardinality != '1':
if has_non_single_cardinality:
msg = _('a sibling entity already has a cardinality non '
'equal to "1" and only one is supported')
raise ValidationError(added_entity_eid,
{role_name('user_cardinality', 'subject'): msg})
has_non_single_cardinality = True
def registration_callback(vreg):
from cubicweb.server import ON_COMMIT_ADD_RELATIONS
from cubicweb_seda import seda_profile_container_def, iter_all_rdefs
......
......@@ -2877,6 +2877,11 @@ msgstr ""
msgid "XSD content type"
msgstr ""
msgid ""
"a sibling entity already has a cardinality non equal to \"1\" and only one "
"is supported"
msgstr ""
msgid "add a SEDAAccessRule"
msgstr ""
......
......@@ -2891,6 +2891,13 @@ msgstr ""
msgid "XSD content type"
msgstr "type de contenu"
msgid ""
"a sibling entity already has a cardinality non equal to \"1\" and only one "
"is supported"
msgstr ""
"une entité de même type et au même niveau a déja une cardinalité autre que "
"obligatoire, il ne peut en avoir plusieurs"
msgid "add a SEDAAccessRule"
msgstr ""
......@@ -7854,7 +7861,9 @@ msgid "select a type to filter out unrelated vocabularies"
msgstr "sélectionnez un type pour filtrer les vocabulaires correspondants"
msgid "select first a vocabulary then type here to prompt auto-completion"
msgstr "sélectionnez un vocabulaire avant de taper dans ce champ pour activer la complétion automatique"
msgstr ""
"sélectionnez un vocabulaire avant de taper dans ce champ pour activer la "
"complétion automatique"
msgid "service_level"
msgstr "valeur"
......
......@@ -468,58 +468,34 @@
</rng:element>
</rng:element>
</rng:oneOrMore>
<rng:oneOrMore>
<rng:element name="Contains">
<rng:element name="DescriptionLevel">
<rng:attribute name="listVersionID">
<rng:value type="token">edition 2009</rng:value>
</rng:attribute>
<rng:value type="string">file</rng:value>
</rng:element>
<xsd:annotation>
<xsd:documentation>archive unit title</xsd:documentation>
</xsd:annotation>
<rng:element name="Contains">
<rng:element name="DescriptionLevel">
<rng:attribute name="listVersionID">
<rng:value type="token">edition 2009</rng:value>
</rng:attribute>
<rng:value type="string">file</rng:value>
</rng:element>
<xsd:annotation>
<xsd:documentation>archive unit title</xsd:documentation>
</xsd:annotation>
<rng:optional>
<rng:attribute name="Id">
<rng:data type="ID"/>
</rng:attribute>
</rng:optional>
<rng:element name="Name">
<rng:optional>
<rng:attribute name="Id">
<rng:data type="ID"/>
<rng:attribute name="languageID">
<rng:data type="language"/>
</rng:attribute>
</rng:optional>
<rng:element name="Name">
<rng:optional>
<rng:attribute name="languageID">
<rng:data type="language"/>
</rng:attribute>
</rng:optional>
<rng:data type="string"/>
</rng:element>
<rng:optional>
<rng:element name="Appraisal">
<xsd:annotation>
<xsd:documentation>detruire le document</xsd:documentation>
</xsd:annotation>
<rng:optional>
<rng:attribute name="Id">
<rng:data type="ID"/>
</rng:attribute>
</rng:optional>
<rng:element name="Code">
<rng:attribute name="listVersionID">
<rng:value type="token">edition 2009</rng:value>
</rng:attribute>
<rng:value type="string">detruire</rng:value>
</rng:element>
<rng:element name="Duration">
<xsd:annotation>
<xsd:documentation>C'est dans 10ans je m'en irai</xsd:documentation>
</xsd:annotation>
<rng:value type="string">P10Y</rng:value>
</rng:element>
<rng:element name="StartDate">
<rng:data type="string"/>
</rng:element>
</rng:element>
</rng:optional>
<rng:element name="AccessRestriction">
<rng:data type="string"/>
</rng:element>
<rng:optional>
<rng:element name="Appraisal">
<xsd:annotation>
<xsd:documentation>detruire le document</xsd:documentation>
</xsd:annotation>
<rng:optional>
<rng:attribute name="Id">
<rng:data type="ID"/>
......@@ -529,14 +505,36 @@
<rng:attribute name="listVersionID">
<rng:value type="token">edition 2009</rng:value>
</rng:attribute>
<rng:data type="string"/>
<rng:value type="string">detruire</rng:value>
</rng:element>
<rng:element name="Duration">
<xsd:annotation>
<xsd:documentation>C'est dans 10ans je m'en irai</xsd:documentation>
</xsd:annotation>
<rng:value type="string">P10Y</rng:value>
</rng:element>
<rng:element name="StartDate">
<rng:data type="string"/>
</rng:element>
</rng:element>
</rng:optional>
<rng:element name="AccessRestriction">
<rng:optional>
<rng:attribute name="Id">
<rng:data type="ID"/>
</rng:attribute>
</rng:optional>
<rng:element name="Code">
<rng:attribute name="listVersionID">
<rng:value type="token">edition 2009</rng:value>
</rng:attribute>
<rng:data type="string"/>
</rng:element>
<rng:element name="StartDate">
<rng:data type="string"/>
</rng:element>
</rng:element>
</rng:oneOrMore>
</rng:element>
</rng:element>
</rng:oneOrMore>
</rng:element>
......
......@@ -373,7 +373,7 @@
<xsd:attribute name="Id" type="xsd:ID" use="optional"/>
</xsd:complexType>
</xsd:element>
<xsd:element maxOccurs="unbounded" name="Contains">
<xsd:element name="Contains">
<xsd:annotation>
<xsd:documentation>archive unit title</xsd:documentation>
</xsd:annotation>
......
......@@ -439,88 +439,86 @@
</rng:element>
</rng:element>
</rng:oneOrMore>
<rng:oneOrMore>
<rng:element name="ArchiveObject">
<xsd:annotation>
<xsd:documentation>archive unit title</xsd:documentation>
</xsd:annotation>
<rng:element name="ArchiveObject">
<xsd:annotation>
<xsd:documentation>archive unit title</xsd:documentation>
</xsd:annotation>
<rng:optional>
<rng:attribute name="Id">
<rng:data type="ID"/>
</rng:attribute>
</rng:optional>
<rng:element name="Name">
<rng:optional>
<rng:attribute name="languageID">
<rng:data type="language"/>
</rng:attribute>
</rng:optional>
<rng:data type="string"/>
</rng:element>
<rng:element name="ContentDescription">
<rng:optional>
<rng:attribute name="Id">
<rng:data type="ID"/>
</rng:attribute>
</rng:optional>
<rng:element name="Name">
<rng:optional>
<rng:attribute name="languageID">
<rng:data type="language"/>
</rng:attribute>
</rng:optional>
<rng:element name="DescriptionLevel">
<rng:attribute name="listVersionID">
<rng:value type="token">edition 2009</rng:value>
</rng:attribute>
<rng:value type="string">file</rng:value>
</rng:element>
<rng:element name="Language">
<rng:attribute name="listVersionID">
<rng:value type="token">edition 2009</rng:value>
</rng:attribute>
<rng:data type="string"/>
</rng:element>
<rng:element name="ContentDescription">
</rng:element>
<rng:optional>
<rng:element name="Appraisal">
<xsd:annotation>
<xsd:documentation>detruire le document</xsd:documentation>
</xsd:annotation>
<rng:optional>
<rng:attribute name="Id">
<rng:data type="ID"/>
</rng:attribute>
</rng:optional>
<rng:element name="DescriptionLevel">
<rng:attribute name="listVersionID">
<rng:value type="token">edition 2009</rng:value>
</rng:attribute>
<rng:value type="string">file</rng:value>
</rng:element>
<rng:element name="Language">
<rng:element name="Code">
<rng:attribute name="listVersionID">
<rng:value type="token">edition 2009</rng:value>
</rng:attribute>
<rng:data type="string"/>
<rng:value type="string">detruire</rng:value>
</rng:element>
</rng:element>
<rng:optional>
<rng:element name="Appraisal">
<rng:element name="Duration">
<xsd:annotation>
<xsd:documentation>detruire le document</xsd:documentation>
<xsd:documentation>C'est dans 10ans je m'en irai</xsd:documentation>
</xsd:annotation>
<rng:optional>
<rng:attribute name="Id">
<rng:data type="ID"/>
</rng:attribute>
</rng:optional>
<rng:element name="Code">
<rng:attribute name="listVersionID">
<rng:value type="token">edition 2009</rng:value>
</rng:attribute>
<rng:value type="string">detruire</rng:value>
</rng:element>
<rng:element name="Duration">
<xsd:annotation>
<xsd:documentation>C'est dans 10ans je m'en irai</xsd:documentation>
</xsd:annotation>
<rng:value type="string">P10Y</rng:value>
</rng:element>
<rng:element name="StartDate">
<rng:data type="string"/>
</rng:element>
</rng:element>
</rng:optional>
<rng:element name="AccessRestrictionRule">
<rng:optional>
<rng:attribute name="Id">
<rng:data type="ID"/>
</rng:attribute>
</rng:optional>
<rng:element name="Code">
<rng:attribute name="listVersionID">
<rng:value type="token">edition 2009</rng:value>
</rng:attribute>
<rng:data type="string"/>
<rng:value type="string">P10Y</rng:value>
</rng:element>
<rng:element name="StartDate">
<rng:data type="string"/>
</rng:element>
</rng:element>
</rng:optional>
<rng:element name="AccessRestrictionRule">
<rng:optional>
<rng:attribute name="Id">
<rng:data type="ID"/>
</rng:attribute>
</rng:optional>
<rng:element name="Code">
<rng:attribute name="listVersionID">
<rng:value type="token">edition 2009</rng:value>
</rng:attribute>
<rng:data type="string"/>
</rng:element>
<rng:element name="StartDate">
<rng:data type="string"/>
</rng:element>
</rng:element>
</rng:oneOrMore>
</rng:element>
<rng:zeroOrMore>
<rng:element name="Document">
<xsd:annotation>
......
......@@ -333,7 +333,7 @@
<xsd:attribute name="Id" type="xsd:ID" use="optional"/>
</xsd:complexType>
</xsd:element>
<xsd:element maxOccurs="unbounded" name="ArchiveObject">
<xsd:element name="ArchiveObject">
<xsd:annotation>
<xsd:documentation>archive unit title</xsd:documentation>
</xsd:annotation>
......
......@@ -122,6 +122,46 @@ class ValidationHooksTC(CubicWebTC):
{'rel_eid': rel.eid})
cnx.commit()
def test_multiple_child_unhandled_cardinality_archive_unit(self):
with self.admin_access.cnx() as cnx:
transfer = cnx.create_entity('SEDAArchiveTransfer', title=u'test profile')
testutils.create_archive_unit(transfer)
cnx.commit()
testutils.create_archive_unit(transfer, user_cardinality=u'0..1')
cnx.commit()
unit2 = testutils.create_archive_unit(transfer, user_cardinality=u'1')[0]
cnx.commit()
with self.assertValidationError(cnx) as cm:
unit2.cw_set(user_cardinality=u'1..n')
self.assertEqual(cm.exception.entity, unit2.eid)
self.assertIn('user_cardinality-subject', cm.exception.errors)
with self.assertValidationError(cnx) as cm:
unit3 = testutils.create_archive_unit(transfer, user_cardinality=u'0..1')[0]
self.assertEqual(cm.exception.entity, unit3.eid)
self.assertIn('user_cardinality-subject', cm.exception.errors)
def test_multiple_child_unhandled_cardinality_document(self):
with self.admin_access.cnx() as cnx:
transfer = cnx.create_entity('SEDAArchiveTransfer', title=u'test profile')
unit, unit_alt, unit_alt_seq = testutils.create_archive_unit(transfer)
testutils.create_data_object(unit_alt_seq, user_cardinality=u'0..n',
seda_binary_data_object=transfer)
cnx.commit()
with self.assertValidationError(cnx):
testutils.create_data_object(unit_alt_seq, user_cardinality=u'1..n',
seda_binary_data_object=transfer)
def test_multiple_child_unhandled_cardinality_keyword(self):
with self.admin_access.cnx() as cnx:
transfer = cnx.create_entity('SEDAArchiveTransfer', title=u'test profile')
unit, unit_alt, unit_alt_seq = testutils.create_archive_unit(transfer)
cnx.create_entity('SEDAKeyword', seda_keyword=unit_alt_seq,
user_cardinality=u'1..n')
cnx.commit()
with self.assertValidationError(cnx):
cnx.create_entity('SEDAKeyword', seda_keyword=unit_alt_seq,
user_cardinality=u'1..n')
class SetDefaultHooksTC(CubicWebTC):
......
......@@ -821,7 +821,7 @@ class OldSEDAExportMixin(object):
# Add another sub archive unit
_, _, subunit2_alt_seq = testutils.create_archive_unit(unit_alt_seq,
user_cardinality=u'1..n')
user_cardinality=u'1')
cnx.commit()
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment