[diagnostic] Warn about ambiguous RNG using the diagnostic system
authorSylvain Thénault <sylvain.thenault@logilab.fr>
Thu, 02 Nov 2017 21:02:37 +0100
changeset 2830 0f8765b71510
parent 2829 5c0fd172aa59
child 2831 854ad0e32400
[diagnostic] Warn about ambiguous RNG using the diagnostic system Most code comes from the former hook based implementation, with heavy lifting in thee test. Implementation is based on a new 'RNG' format in the compatibility list to avoid preventing export. Closes #39302963
cubicweb_seda/entities/diag.py
cubicweb_seda/hooks.py
cubicweb_seda/i18n/en.po
cubicweb_seda/i18n/fr.po
test/test_diag.py
--- a/cubicweb_seda/entities/diag.py	Thu Nov 02 20:58:46 2017 +0100
+++ b/cubicweb_seda/entities/diag.py	Thu Nov 02 21:02:37 2017 +0100
@@ -16,13 +16,14 @@
 """cubicweb-seda tools to diagnose versions compatibility of profiles."""
 
 from collections import namedtuple
+from itertools import chain
 
 from cubicweb import _
 from cubicweb.view import EntityAdapter
 from cubicweb.predicates import is_instance
 
 
-ALL_FORMATS = frozenset(('SEDA 2.0', 'SEDA 1.0', 'SEDA 0.2', 'simplified'))
+ALL_FORMATS = frozenset(('SEDA 2.0', 'SEDA 1.0', 'SEDA 0.2', 'simplified', 'RNG'))
 
 Rule = namedtuple('Rule', ['impacted_formats', 'message', 'tab_id', 'watch'])
 RULES = {
@@ -108,6 +109,11 @@
         set([
             'seda_when',
         ])),
+    'rng_ambiguity': Rule(
+        set(['RNG']),
+        _("More than one children with cardinality not equal to '1', only one is supported."),
+        None,  # generated at run time
+        []),  # have it's own hook
 }
 
 
@@ -159,7 +165,8 @@
         """
         for check_method in (self._check_usage_of_reference_archive_unit,
                              self._check_management_rules,
-                             self._check_custodial_history_rules):
+                             self._check_custodial_history_rules,
+                             self._check_rng_ambiguities):
             for problem in check_method():
                 yield problem
 
@@ -225,11 +232,63 @@
                 'X container C, C eid %(c)s', {'c': self.entity.eid}).entities():
             yield 'use_archive_unit_ref', archive_unit
 
-    # XXX more than one items ->
+    def _check_rng_ambiguities(self):
+        for eid, rtype in self._cw.execute(_AMBIGUOUS_RQL,
+                                           {'c': self.entity.eid}):
+            # if the container is a simplified profile, allow to have several
+            # entities with cardinality != 1 under the seda_binary_data_object
+            # relation because data objects are all linked to the transfer
+            # through this relation, but this is only relevant for SEDA 2 export
+            # and we don't want to prevent this in profiles for SEDA 0.2 and 1.0
+            # export.
+            if rtype == 'seda_binary_data_object' and self.entity.simplified_profile:
+                continue
+            tab_id = _RTYPE_TO_TAB.get(rtype)
+            yield 'rng_ambiguity', _parent(self._cw.entity_from_eid(eid)), {'tab_id': tab_id}
 
 
 def _parent(entity):
     """Return the first encountered parent which is an ArchiveUnit"""
-    while entity.cw_etype not in ('SEDAArchiveTransfer', 'SEDAArchiveUnit'):
+    while entity.cw_etype not in ('SEDAArchiveTransfer', 'SEDAArchiveUnit',
+                                  'SEDABinaryDataObject', 'SEDAPhysicalDataObject'):
         entity = entity.cw_adapt_to('IContained').parent
     return entity
+
+
+_RTYPE_TO_TAB = {
+    'seda_addressee_from': 'seda_agents_tab',
+    'seda_archive_unit': 'seda_archive_units_tab',
+    'seda_binary_data_object': 'seda_data_objects_tab',
+    'seda_custodial_history_item': 'seda_history_tab',
+    'seda_data_object_reference': 'seda_data_objects_tab',
+    'seda_event': 'seda_events_tab',
+    'seda_is_part_of': 'seda_relations_tab',
+    'seda_is_version_of': 'seda_relations_tab',
+    'seda_juridictional': 'seda_coverage_tab',
+    'seda_keyword': 'seda_indexation_tab',
+    'seda_physical_data_object': 'seda_data_objects_tab',
+    'seda_recipient_from': 'seda_agents_tab',
+    'seda_ref_non_rule_id_from': 'seda_management_tab',
+    'seda_references': 'seda_relations_tab',
+    'seda_related_transfer_reference': 'seda_at_related_transfers_tab',
+    'seda_relationship': 'seda_do_relations',
+    'seda_replaces': 'seda_relations_tab',
+    'seda_requires': 'seda_relations_tab',
+    'seda_spatial': 'seda_coverage_tab',
+    'seda_tag': 'seda_indexation_tab',
+    'seda_temporal': 'seda_coverage_tab',
+    'seda_writer_from': 'seda_agents_tab',
+}
+_AMBIGUOUS_RQL = ' UNION '.join(chain(
+    ('(Any P, "{rtype}" GROUPBY P WHERE X {rtype} P, P container C, C eid %(c)s, '
+     'NOT X user_cardinality "1" HAVING COUNT(X) > 1)'.format(rtype=rtype)
+     for rtype in _RTYPE_TO_TAB if rtype not in ('seda_binary_data_object',
+                                                 'seda_physical_data_object',
+                                                 'seda_related_transfer_reference')),
+
+    ('(Any P, "{rtype}" GROUPBY P WHERE X {rtype} P, P eid %(c)s, '
+     'NOT X user_cardinality "1" HAVING COUNT(X) > 1)'.format(rtype=rtype)
+     for rtype in ('seda_archive_unit',
+                   'seda_binary_data_object', 'seda_physical_data_object',
+                   'seda_related_transfer_reference')),
+))
--- a/cubicweb_seda/hooks.py	Thu Nov 02 20:58:46 2017 +0100
+++ b/cubicweb_seda/hooks.py	Thu Nov 02 21:02:37 2017 +0100
@@ -338,6 +338,7 @@
                                   # filter out (entity type, attribute)
                                   if not isinstance(rtype, tuple))
                                  for rule in diag.RULES.values()))
+WATCH_RTYPES_SET |= set(CHECK_CHILDREN_CARD_RTYPES)
 
 
 class AddOrRemoveChildrenHook(hook.Hook):
@@ -361,6 +362,9 @@
 
 WATCH_ETYPES['SEDAArchiveTransfer'].add('simplified_profile')
 
+for etype in CHECK_CARD_ETYPES:
+    WATCH_ETYPES[etype].add('user_cardinality')
+
 
 class UpdateWatchedProfileElementHook(hook.Hook):
     """Some entity involved in diagnosis of the profile is created or updated."""
--- a/cubicweb_seda/i18n/en.po	Thu Nov 02 20:58:46 2017 +0100
+++ b/cubicweb_seda/i18n/en.po	Thu Nov 02 21:02:37 2017 +0100
@@ -116,6 +116,11 @@
 msgid "Message"
 msgstr ""
 
+msgid ""
+"More than one children with cardinality not equal to '1', only one is "
+"supported."
+msgstr ""
+
 msgid "New SEDAAccessRule"
 msgstr ""
 
@@ -2750,11 +2755,6 @@
 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 ""
 
--- a/cubicweb_seda/i18n/fr.po	Thu Nov 02 20:58:46 2017 +0100
+++ b/cubicweb_seda/i18n/fr.po	Thu Nov 02 21:02:37 2017 +0100
@@ -123,6 +123,13 @@
 msgid "Message"
 msgstr "Message"
 
+msgid ""
+"More than one children with cardinality not equal to '1', only one is "
+"supported."
+msgstr ""
+"Plus d'une entité de même type et au même niveau ont une cardinalité autre "
+"que obligatoire, il ne peut y en avoir qu'une."
+
 msgid "New SEDAAccessRule"
 msgstr ""
 
@@ -2764,13 +2771,6 @@
 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 ""
 
--- a/test/test_diag.py	Thu Nov 02 20:58:46 2017 +0100
+++ b/test/test_diag.py	Thu Nov 02 21:02:37 2017 +0100
@@ -138,9 +138,114 @@
         doctor.entity.cw_clear_all_caches()
         rule_ids = set(rule_id for rule_id, entity in doctor.failing_rules())
         self.assertEqual(rule_ids, set(expected_rule_ids))
+        expected_formats.append('RNG')
         self.assertEqual(doctor.diagnose(), set(expected_formats))
         self.assertEqual(doctor.entity.compat_list, ', '.join(sorted(expected_formats)))
 
+    # in tests below, assertIsRNGAmbiguous is used to test that hooks have
+    # properly detected the error (or fix) while assertRNGAmbiguousDiagnostic
+    # will test the underlying details of the diagnosis tool.
+
+    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')
+            doctor = transfer.cw_adapt_to('ISEDACompatAnalyzer')
+            testutils.create_archive_unit(transfer)
+            testutils.create_archive_unit(transfer, user_cardinality=u'0..1')
+            unit2 = testutils.create_archive_unit(transfer, user_cardinality=u'1')[0]
+            cnx.commit()
+            unit2.cw_set(user_cardinality=u'1..n')
+            cnx.commit()
+
+            self.assertIsRNGAmbiguous(transfer, True)
+            self.assertRNGAmbiguousDiagnostic(doctor, [(transfer, 'seda_archive_units_tab')])
+
+            unit2.cw_set(user_cardinality=u'1')
+            cnx.commit()
+            self.assertIsRNGAmbiguous(transfer, False)
+
+            unit3 = testutils.create_archive_unit(transfer, user_cardinality=u'0..1')[0]
+            cnx.commit()
+            self.assertIsRNGAmbiguous(transfer, True)
+            self.assertRNGAmbiguousDiagnostic(doctor, [(transfer, 'seda_archive_units_tab')])
+
+            unit3.cw_delete()
+            cnx.commit()
+            self.assertIsRNGAmbiguous(transfer, False)
+
+    def test_multiple_child_unhandled_cardinality_document(self):
+        with self.admin_access.cnx() as cnx:
+            transfer = cnx.create_entity('SEDAArchiveTransfer', title=u'test profile')
+            doctor = transfer.cw_adapt_to('ISEDACompatAnalyzer')
+            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)
+            bdo = testutils.create_data_object(unit_alt_seq, user_cardinality=u'1..n',
+                                               seda_binary_data_object=transfer)
+            cnx.commit()
+            self.assertIsRNGAmbiguous(transfer, True)
+            self.assertRNGAmbiguousDiagnostic(doctor, [(unit, 'seda_data_objects_tab'),
+                                                       (transfer, 'seda_data_objects_tab')])
+            _delete_data_object(bdo)
+            cnx.commit()
+            self.assertIsRNGAmbiguous(transfer, False)
+
+            # test data objects cardinality is considered globally to the profil
+            # **if it's not a simplified profile**
+            unit2, unit2_alt, unit2_alt_seq = testutils.create_archive_unit(transfer)
+            bdo = testutils.create_data_object(unit2_alt_seq, user_cardinality=u'0..n',
+                                               seda_binary_data_object=transfer)
+            cnx.commit()
+            self.assertIsRNGAmbiguous(transfer, True)
+            self.assertRNGAmbiguousDiagnostic(doctor, [(transfer, 'seda_data_objects_tab')])
+            _delete_data_object(bdo)
+            cnx.commit()
+            self.assertIsRNGAmbiguous(transfer, False)
+
+            # though adding a data object with card != 1 to another archive unit
+            # **on a simplified profile** is fine
+            transfer.cw_set(simplified_profile=True)
+            testutils.create_data_object(unit2_alt_seq, user_cardinality=u'0..n',
+                                         seda_binary_data_object=transfer)
+            cnx.commit()
+            self.assertRNGAmbiguousDiagnostic(doctor, [])
+            self.assertIsRNGAmbiguous(transfer, False)
+
+    def test_multiple_child_unhandled_cardinality_keyword(self):
+        with self.admin_access.cnx() as cnx:
+            transfer = cnx.create_entity('SEDAArchiveTransfer', title=u'test profile')
+            doctor = transfer.cw_adapt_to('ISEDACompatAnalyzer')
+            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.create_entity('SEDAKeyword', seda_keyword=unit_alt_seq,
+                              user_cardinality=u'1..n')
+            cnx.commit()
+
+            self.assertRNGAmbiguousDiagnostic(doctor, [(unit, 'seda_indexation_tab')])
+
+    def assertRNGAmbiguousDiagnostic(self, doctor, expected_errors):
+        errors = set()
+        for error in doctor.detect_problems():
+            print('assert diag error', error.rule_id)
+            if error.rule_id == 'rng_ambiguity':
+                errors.add((error.entity, error.tab_id))
+
+        self.assertEqual(errors, set(expected_errors))
+
+    def assertIsRNGAmbiguous(self, transfer, ambiguous):
+        transfer.cw_clear_all_caches()
+        if ambiguous:
+            self.assertNotIn('RNG', transfer.compat_list)
+        else:
+            self.assertIn('RNG', transfer.compat_list)
+
+
+def _delete_data_object(data_object):
+    for ref in data_object.reverse_seda_data_object_reference_id:
+        ref.cw_delete()
+    data_object.cw_delete()
+
 
 if __name__ == '__main__':
     import unittest