[profile gen] Stop ordering upon cardinality on profile generation
authorSylvain Thénault <sylvain.thenault@logilab.fr>
Mon, 11 Dec 2017 15:13:39 +0100
changeset 2900 e0e34c11185e
parent 2899 ed666cbea6af
child 2901 c5c5d8ea25b9
[profile gen] Stop ordering upon cardinality on profile generation rather use ORM controlled order, consistently with the UI. Let the doctor diagnose ambiguities (done in previous commit). This backout most parts of 4941eab6a289.
cubicweb_seda/entities/html_generation.py
cubicweb_seda/entities/profile_generation.py
test/test_profile_generation.py
--- a/cubicweb_seda/entities/html_generation.py	Wed Dec 06 15:16:59 2017 +0100
+++ b/cubicweb_seda/entities/html_generation.py	Mon Dec 11 15:13:39 2017 +0100
@@ -240,7 +240,7 @@
             self.element('span', ct_div, text=xstypes, attributes={'class': 'value'})
         if target_value is None:
             values = ()
-        elif not isinstance(target_value, list):
+        elif not isinstance(target_value, (tuple, list)):
             values = [target_value]
         else:
             values = target_value
--- a/cubicweb_seda/entities/profile_generation.py	Wed Dec 06 15:16:59 2017 +0100
+++ b/cubicweb_seda/entities/profile_generation.py	Mon Dec 11 15:13:39 2017 +0100
@@ -162,10 +162,6 @@
         return minmax_cardinality(cardinality, ('0..1', '1'))[0]
 
 
-# sort on user_cardinality != 1 to have all entities with cardinality == 1 first
-_sorted_children = partial(sorted, key=lambda x: getattr(x, 'user_cardinality', '1') != '1')
-
-
 def iter_path_children(xselement, entity):
     """Return an iterator on `entity` children entities according to `xselement` definition.
 
@@ -178,7 +174,7 @@
             if getattr(entity, rtype) is not None:
                 yield _path, getattr(entity, rtype)
         else:
-            related = _sorted_children(entity.related(rtype, role, entities=True))
+            related = entity.related(rtype, role, entities=True)
             if related:
                 yield _path, related
 
@@ -230,7 +226,7 @@
             datatype = 'string'
         type_attrs = {'type': datatype}
         if fixed_value is not None:
-            if isinstance(fixed_value, list):
+            if isinstance(fixed_value, (tuple, list)):
                 choice = self.element('rng:choice', element)
                 for value in fixed_value:
                     self.element('rng:value', choice, type_attrs, text=value)
@@ -564,7 +560,7 @@
         # of label of its concept value
         if value is not None and xselement.local_name == 'KeywordReference':
             fixed_value = self.cwuri_url(value)
-        elif isinstance(value, list):
+        elif isinstance(value, (tuple, list)):
             fixed_value = [serialize(val, self.cwuri_url) for val in value]
         else:
             fixed_value = serialize(value, self.cwuri_url)
@@ -578,7 +574,7 @@
                     xstype = profile_element[-1].attrib.get('type')
                     profile_element.remove(profile_element[-1])
                 attrs = {'type': xstype} if xstype else {}
-                if isinstance(fixed_value, list):
+                if isinstance(fixed_value, (tuple, list)):
                     choice = self.element('rng:choice', profile_element)
                     for val in fixed_value:
                         self.element('rng:value', choice, attrs, text=val)
@@ -664,7 +660,7 @@
         assert cardinality in (None, '1', '0..1'), cardinality
         if fixed_value:
             cardinality = '1'
-            if isinstance(fixed_value, list) and len(fixed_value) == 1:
+            if isinstance(fixed_value, (tuple, list)) and len(fixed_value) == 1:
                 fixed_value = fixed_value[0]
         else:
             fixed_value = None
@@ -753,12 +749,12 @@
             attrs['use'] = 'required'
         else:
             attrs['use'] = 'optional'
-        if not isinstance(xattr.fixed_value, list):
+        if not isinstance(xattr.fixed_value, (tuple, list)):
             attrs['type'] = xattr.qualified_type
             if isinstance(xattr.fixed_value, string_types):
                 attrs['fixed'] = text_type(xattr.fixed_value)
         attribute_element = self.element('xsd:attribute', parent, attrs)
-        if isinstance(xattr.fixed_value, list):
+        if isinstance(xattr.fixed_value, (tuple, list)):
             type_element = self.element('xsd:simpleType', attribute_element)
             restriction_element = self.element('xsd:restriction', type_element,
                                                {'base': 'xsd:token'})
@@ -779,7 +775,7 @@
     def xsd_transfer(self, parent, archive_transfer):
         """Append XSD elements for the archive transfer to the given parent node."""
         transfer_node = self.xsd_transfer_base(parent, archive_transfer)
-        for archive_unit in _sorted_children(archive_transfer.archive_units):
+        for archive_unit in archive_transfer.archive_units:
             self.xsd_archive(transfer_node, archive_unit)
 
     def xsd_archive(self, parent, archive_unit):
@@ -862,9 +858,7 @@
         archive objects or documents, and append XSD elements for them to the given parent node.
         """
         for au_or_bdo in sorted(entity.cw_adapt_to('ITreeBase').iterchildren(),
-                                key=lambda x: (x.cw_etype == self.last_children_type,
-                                               x.user_cardinality != '1',
-                                               x.eid)):
+                                key=lambda x: x.cw_etype == self.last_children_type):
             if au_or_bdo.cw_etype == 'SEDABinaryDataObject':
                 self.xsd_document(parent, au_or_bdo)
             else:
@@ -1078,14 +1072,14 @@
                                 xsd_attributes=[XAttr('languageID', 'xsd:language')])
 
     def xsd_keywords(self, parent, content):
-        for keyword in _sorted_children(content.keywords):
+        for keyword in content.keywords:
             self.xsd_keyword(parent, keyword)
 
     def xsd_custodial_history(self, parent, content):
         if content.custodial_history_items:
             ch_node = self.element_schema(parent, 'CustodialHistory',
                                           cardinality='0..1')
-            for item in _sorted_children(content.custodial_history_items):
+            for item in content.custodial_history_items:
                 when_card = item.when.user_cardinality if item.when else None
                 self.element_schema(
                     ch_node, 'CustodialHistoryItem', 'qdt:CustodialHistoryItemType',
@@ -1211,10 +1205,10 @@
         """Append XSD elements for the archive transfer to the given parent node."""
         transfer_node = self.xsd_transfer_base(parent, archive_transfer)
 
-        for data_object in _sorted_children(archive_transfer.binary_data_objects):
+        for data_object in archive_transfer.binary_data_objects:
             self.xsd_integrity(transfer_node, data_object)
 
-        for archive_unit in _sorted_children(archive_transfer.archive_units):
+        for archive_unit in archive_transfer.archive_units:
             self.xsd_archive(transfer_node, archive_unit)
 
     def xsd_archive(self, parent, archive_unit):
@@ -1419,7 +1413,7 @@
         if rtype == 'id':
             return [(None, eid2xmlid(entity.eid))]
         return [(None, getattr(entity, rtype, None))]
-    targets = _sorted_children(entity.related(rtype, role, entities=True))
+    targets = entity.related(rtype, role, entities=True)
     rschema = entity._cw.vreg.schema.rschema
     rdefschema = next(iter(rschema(rtype).rdefs.values()))
     # data_object_reference_id is artificially composite to ease the case of simplified profile
@@ -1450,7 +1444,7 @@
                 rtype_targets.append((entity, getattr(entity, rtype, None)))
             else:
                 try:
-                    targets = _sorted_children(entity.related(rtype, role, entities=True))
+                    targets = entity.related(rtype, role, entities=True)
                 except KeyError:
                     # the relation is not defined in the schema: element is not modelized but should
                     # be added in the XSD
--- a/test/test_profile_generation.py	Wed Dec 06 15:16:59 2017 +0100
+++ b/test/test_profile_generation.py	Mon Dec 11 15:13:39 2017 +0100
@@ -448,7 +448,6 @@
             unit, unit_alt, unit_alt_seq = testutils.create_archive_unit(
                 transfer, user_cardinality=u'0..n')
 
-            create('SEDAKeyword', user_cardinality=u'0..n', seda_keyword=unit_alt_seq)
             kw = create('SEDAKeyword', seda_keyword=unit_alt_seq,
                         keyword_content=u'kwick')
             kwr_e = create('SEDAKeywordReference', seda_keyword_reference_from=kw)
@@ -459,16 +458,11 @@
 
             keywords = self.get_elements(profile, 'Keyword')
             self.assertElementDefinition(keywords[0], {'name': 'Keyword'})
-            self.assertElementDefinition(keywords[1], {'name': 'Keyword',
-                                                       'minOccurs': '0',
-                                                       'maxOccurs': 'unbounded'})
 
             kwc = self.get_elements(profile, 'KeywordContent')
             self.assertElementDefinition(kwc[0], {'name': 'KeywordContent',
                                                   'type': 'xsd:string',
                                                   'fixed': 'kwick'})
-            self.assertElementDefinition(kwc[1], {'name': 'KeywordContent',
-                                                  'type': 'xsd:string'})
 
             kwt = self.get_element(profile, 'KeywordType')
             self.assertElementDefinition(kwt, {'name': 'KeywordType',
@@ -817,7 +811,11 @@
                          reverse_seda_when=create('SEDAwhen',
                                                   user_cardinality=u'0..1'))
 
-            # Add sub archive unit
+            # Add a sub archive unit
+            subunit2, _, subunit2_alt_seq = testutils.create_archive_unit(
+                unit_alt_seq, user_cardinality=u'1')
+
+            # Add another sub archive unit
             subunit1, _, subunit_alt_seq = testutils.create_archive_unit(
                 unit_alt_seq, user_cardinality=u'1..n')
 
@@ -853,10 +851,6 @@
                    seda_encoding_from=bdo,
                    seda_encoding_to=concepts['6'])
 
-            # Add another sub archive unit
-            subunit2, _, subunit2_alt_seq = testutils.create_archive_unit(
-                unit_alt_seq, user_cardinality=u'1')
-
             cnx.commit()
 
         self.transfer_eid = transfer.eid
@@ -1000,88 +994,26 @@
             unit, unit_alt, unit_alt_seq = testutils.create_archive_unit(
                 transfer, user_cardinality=u'0..1')
             testutils.create_archive_unit(transfer)
-            testutils.create_archive_unit(unit_alt_seq, user_cardinality=u'0..1')
             testutils.create_archive_unit(unit_alt_seq)
-            bdo = testutils.create_data_object(transfer, user_cardinality=u'0..1')
+            bdo = testutils.create_data_object(transfer)
             create('SEDADataObjectReference',
                    seda_data_object_reference=unit_alt_seq,
                    seda_data_object_reference_id=bdo)
-            bdo2 = testutils.create_data_object(transfer)
-            create('SEDADataObjectReference',
-                   seda_data_object_reference=unit_alt_seq,
-                   seda_data_object_reference_id=bdo2)
             cnx.commit()
 
-            # ensure Document appears before Contains in SEDA 0.2 + cardinality based order
+            # ensure Document appears before Contains in SEDA 0.2
             adapter = transfer.cw_adapt_to('SEDA-0.2.xsd')
             root = etree.Element('test-root')
             adapter.xsd_children(root, unit)
             self.assertEqual([node.attrib['name'] for node in root],
-                             ['Document', 'Document', 'Contains', 'Contains'])
-            self.assertEqual([node.attrib.get('minOccurs') for node in root],
-                             [None, '0', None, '0'])
-            # test cardinality on transfer's children
-            root = etree.Element('test-root')
-            adapter.xsd_transfer(root, transfer)
-            # [0][-1][0][-2:] xs:element / xs:complexType / xs:sequence, picking
-            # -1 to avoid going into the annotation and -2: to only keep the two
-            # latest children
-            self.assertEqual([(node.attrib['name'], node.attrib.get('minOccurs'))
-                              for node in root[0][-1][0][-2:]],
-                             [('Contains', None), ('Contains', '0')])
+                             ['Document', 'Contains'])
 
-            # ensure Document appears after ArchiveObject in SEDA 1 + cardinality based order
+            # ensure Document appears after ArchiveObject in SEDA 1
             adapter = transfer.cw_adapt_to('SEDA-1.0.xsd')
             root = etree.Element('test-root')
             adapter.xsd_children(root, unit)
             self.assertEqual([node.attrib['name'] for node in root],
-                             ['ArchiveObject', 'ArchiveObject', 'Document', 'Document'])
-            self.assertEqual([node.attrib.get('minOccurs') for node in root],
-                             [None, '0', None, '0'])
-            # test cardinality on transfer's children
-            root = etree.Element('test-root')
-            adapter.xsd_transfer(root, transfer)
-            # xs:element(CustodialHistory) / xs:complexType / xs:sequence
-            self.assertEqual([(node.attrib['name'], node.attrib.get('minOccurs'))
-                              for node in root[0][-1][0][-2:]],
-                             [('Archive', None), ('Archive', '0')])
-
-    def test_keywords_order(self):
-        with self.admin_access.cnx() as cnx:
-            create = cnx.create_entity
-
-            transfer = create('SEDAArchiveTransfer', title=u'test profile',
-                              simplified_profile=True)
-            unit, unit_alt, unit_alt_seq = testutils.create_archive_unit(transfer)
-            create('SEDAKeyword', user_cardinality=u'0..n', seda_keyword=unit_alt_seq)
-            create('SEDAKeyword', seda_keyword=unit_alt_seq)
-
-            adapter = transfer.cw_adapt_to('SEDA-1.0.xsd')
-            root = etree.Element('test-root')
-            adapter.xsd_keywords(root, unit_alt_seq)
-            self.assertEqual([(node.attrib['name'], node.attrib.get('minOccurs')) for node in root],
-                             [('Keyword', None), ('Keyword', '0')])
-
-    def test_custodialhistory_order(self):
-        with self.admin_access.cnx() as cnx:
-            create = cnx.create_entity
-
-            transfer = create('SEDAArchiveTransfer', title=u'test profile',
-                              simplified_profile=True)
-            unit, unit_alt, unit_alt_seq = testutils.create_archive_unit(transfer)
-            create('SEDACustodialHistoryItem',
-                   user_cardinality=u'0..1',
-                   seda_custodial_history_item=unit_alt_seq)
-            create('SEDACustodialHistoryItem',
-                   seda_custodial_history_item=unit_alt_seq)
-
-            adapter = transfer.cw_adapt_to('SEDA-1.0.xsd')
-            root = etree.Element('test-root')
-            adapter.xsd_custodial_history(root, unit_alt_seq)
-            history = root[0][0][0]  # xs:element(CustodialHistory) / xs:complexType / xs:sequence
-            self.assertEqual([(node.attrib['name'], node.attrib.get('minOccurs'))
-                              for node in history],
-                             [('CustodialHistoryItem', None), ('CustodialHistoryItem', '0')])
+                             ['ArchiveObject', 'Document'])
 
 
 class SEDAExportUnitTest(unittest.TestCase):