[profile gen] Stop fixing order of binary/physical data objects
authorSylvain Thénault <sylvain.thenault@logilab.fr>
Wed, 29 Mar 2017 10:21:46 +0200
changeset 2547 370716d04b28
parent 2546 377976b673e0
child 2548 8f9c8fd47867
[profile gen] Stop fixing order of binary/physical data objects Before this cset, data objects were inserted one after another in some random order, but in the RNG semantics this was forcing their order. To avoid this, we've to put them in a rng:group node. Several strategies have been considered to achieve this: * change the underlying XSD representation, but this would require either changing the data model or handling a difference between the representation used to generate the schema and the one used to generate the RNG profile, * add some special case when a data object node is generated, but this would require handling some state to see if the rng:group has already been inserted, * add a postprocessing step doing the job all at once using lxml's API. It appeared to me that the two first strategies where too much tricky and less readable than the third one. Related to #17066567
cubicweb_seda/entities/profile_generation.py
test/test_profile_generation.py
--- a/cubicweb_seda/entities/profile_generation.py	Wed Mar 29 10:22:08 2017 +0200
+++ b/cubicweb_seda/entities/profile_generation.py	Wed Mar 29 10:21:46 2017 +0200
@@ -17,6 +17,7 @@
 
 from functools import partial
 from collections import defaultdict, namedtuple
+from itertools import chain
 
 from six import text_type, string_types
 
@@ -408,8 +409,39 @@
         del namespaces[None]  # xpath engine don't want None prefix
         for element in root.xpath('//rng:element[not(*)]', namespaces=namespaces):
             self.element('rng:text', element)
+
+        self.postprocess_dataobjects(root, namespaces)
+
         return root
 
+    def postprocess_dataobjects(self, root, namespaces):
+        """Insert rng:group node as parent of [Binary|Physical]DataObject node
+        to avoid forcing an order among them
+        """
+        # start by looking for the rng:element node for DataObjectPackage or its parent rng:optional
+        dops = root.xpath('/rng:grammar/rng:start/rng:element/'
+                          'rng:element[@name="DataObjectPackage"]',
+                          namespaces=namespaces)
+        if not dops:
+            dops = root.xpath('/rng:grammar/rng:start/rng:element/'
+                              'rng:optional/rng:element[@name="DataObjectPackage"]',
+                              namespaces=namespaces)
+        if dops:
+            assert len(dops) == 1
+            dop = dops[0]
+            nodes = dop.xpath(
+                'rng:element[@name="BinaryDataObject" or @name="PhysicalDataObject"]',
+                namespaces=namespaces)
+            opt_nodes = dop.xpath(
+                'rng:optional[rng:element[@name="BinaryDataObject" or @name="PhysicalDataObject"]]',
+                namespaces=namespaces)
+            if nodes or opt_nodes:
+                group = self.element('rng:group')
+                # insert after definition of dop's id attribute
+                dop[0].addnext(group)
+                for node in chain(nodes, opt_nodes):
+                    group.append(node)
+
     def init_transfer_element(self, xselement, root, entity):
         transfer_element = self.element('rng:element', root,
                                         {'name': xselement.local_name,
--- a/test/test_profile_generation.py	Wed Mar 29 10:22:08 2017 +0200
+++ b/test/test_profile_generation.py	Wed Mar 29 10:21:46 2017 +0200
@@ -394,6 +394,30 @@
             fileinfo = self.get_element(profile, 'DataObjectPackage')
             self.assertElementDefinition(fileinfo, {'name': 'DataObjectPackage'})
 
+    def test_object_package_group(self):
+        with self.admin_access.client_cnx() as cnx:
+            transfer = cnx.create_entity('SEDAArchiveTransfer', title=u'test profile')
+            bdo = cnx.create_entity('SEDABinaryDataObject',
+                                    user_annotation=u'I am number one',
+                                    seda_binary_data_object=transfer)
+            cnx.create_entity('SEDABinaryDataObject',
+                              user_annotation=u'I am number two',
+                              seda_binary_data_object=transfer)
+            cnx.create_entity('SEDAPhysicalDataObject',
+                              user_annotation=u'I am number three',
+                              seda_physical_data_object=transfer)
+
+            profile = self.profile_etree(transfer)
+            dop = self.get_element(profile, 'DataObjectPackage')
+            self.assertEqual(len(self.xpath(dop, './rng:group/*')), 3)
+
+            # setting some cardinality to 1 will remove rng:optional parent of the DataObjectPackage
+            # and BinaryDataObject nodes
+            bdo.cw_set(user_cardinality=u'1')
+            profile = self.profile_etree(transfer)
+            dop = self.get_element(profile, 'DataObjectPackage')
+            self.assertEqual(len(self.xpath(dop, './rng:group/*')), 3)
+
     def test_transfer_annotation(self):
         with self.admin_access.client_cnx() as cnx:
             profile = self.profile_etree(cnx.create_entity('SEDAArchiveTransfer',