[PATCH 1 of 1 compound] Fix case where skiprtype include direct relation of the container

Sylvain Thénault sylvain.thenault at logilab.fr
Fri Mar 3 10:53:18 CET 2017


sent a V2


Le 03/03/2017 à 10:04, Denis Laxalde a écrit :
> Sylvain Thenault a écrit :
>> # HG changeset patch
>> # User Sylvain Thénault <sylvain.thenault at logilab.fr>
>> # Date 1488377377 -3600
>> #      Wed Mar 01 15:09:37 2017 +0100
>> # Node ID ff923f477d5d385d9145ac52709dedf4a0dfbf28
>> # Parent  5b0cf134ad26cf13349e8d0e2a9dd46f9613962e
>> Fix case where skiprtype include direct relation of the container
>
> Mentioning "clone" here wouldn't hurt.
>
>> those are copied using standard Entity.copy_relations method, hence
>> adapter's skiprtypes
>> should be copied in entity's cw_skip_copy_for to control above method.
>>
>> diff --git a/entities.py b/entities.py
>> --- a/entities.py
>> +++ b/entities.py
>> @@ -201,10 +201,13 @@ class IClonableAdapter(EntityAdapter):
>>          assert clone.cw_etype == self.entity.cw_etype, \
>>              "clone entity type {} does not match with original's
>> {}".format(
>>                  clone.cw_etype, self.entity.cw_etype)
>>          related = self.graph.child_related(
>>              self.entity, follow_relations=self.follow_relations)
>> +        clone.cw_skip_copy_for = list(clone.cw_skip_copy_for)
>> +        clone.cw_skip_copy_for += [(rtype, 'subject') for rtype in
>> self.skiprtypes]
>> +        clone.cw_skip_copy_for += [(rtype, 'object') for rtype in
>> self.skiprtypes]
>
> Could you build the list in a single statement to avoid += usage?
>
>     clone.cw_skip_copy_for = (
>       list(clone.cw_skip_copy_for)
>       + [(rtype, 'subject') for rtype in self.skiprtypes]]
>       + [(rtype, 'object') for rtype in self.skiprtypes]
>     )
>
> Also, it's not clear to me why we'd need to replace the attribute
> instead of extending its value.
>
>
>>          with
>> self._cw.deny_all_hooks_but(*self.enabled_hook_categories):
>>              clone.copy_relations(self.entity.eid)
>>              clones = {self.entity: clone}
>>              for parent, (rtype, parent_role), child in related:
>>                  rel = rtype if parent_role == 'object' else
>> 'reverse_' + rtype
>> diff --git a/test/test_compound.py b/test/test_compound.py
>> --- a/test/test_compound.py
>> +++ b/test/test_compound.py
>> @@ -260,10 +260,20 @@ class ContainerAPITC(CubicWebTC):
>>              self.assertEqual(icontained.parent_relations,
>>                               set([('event', 'object'), ('relates',
>> 'object')]))
>>              self.assertEqual(icontained.parent_relation(), ('event',
>> 'object'))
>>
>>
>> +def clone_agent(cnx, name=u'bob', newname=u'bobby', skiprtypes=()):
>> +    bob = cnx.find('Agent', name=name).one()
>> +    clone = cnx.create_entity('Agent', name=u'bobby')
>> +    adapted = bob.cw_adapt_to('IClonable')
>> +    adapted.skiprtypes = skiprtypes
>> +    adapted.clone_into(clone)
>> +    cnx.commit()
>> +    return clone
>> +
>> +
>>  class CloneTC(CubicWebTC):
>>
>>      def setup_database(self):
>>          with self.admin_access.repo_cnx() as cnx:
>>              self.create_user(cnx, u'georges')
>> @@ -325,25 +335,29 @@ class CloneTC(CubicWebTC):
>>              bob = cnx.create_entity('Agent', name=u'bob')
>>              group = cnx.create_entity('Group', member=bob)
>>              cnx.create_entity('OnlineAccount', reverse_account=bob)
>>              cnx.commit()
>>          with self.new_access(u'georges').repo_cnx() as cnx:
>> -            bob = cnx.find('Agent', name=u'bob').one()
>> -            clone = cnx.create_entity('Agent', name=u'bobby')
>> -            adapted = bob.cw_adapt_to('IClonable')
>> -            self.assertEqual(adapted.__class__.__name__,
>> -                             'AgentInGroupIClonableAdapter')
>> -            adapted.clone_into(clone)
>> -            clone.cw_clear_all_caches()
>> -            cnx.commit()
>> +            clone = clone_agent(cnx, u'bob')
>>              rset = cnx.execute(
>>                  'Any G WHERE G member A, A name "bobby", OG eid
>> %(og)s,'
>>                  ' NOT G identity OG',
>>                  {'og': group.eid})
>>              self.assertEqual(len(rset), 1)
>>              self.assertEqual(clone.reverse_member[0].eid, rset[0][0])
>>
>> +    def test_clone_skiprtypes(self):
>> +        with self.admin_access.repo_cnx() as cnx:
>> +            bob = cnx.create_entity('Agent', name=u'bob')
>> +            cnx.create_entity('Group', member=bob)
>> +            cnx.commit()
>> +
>> +            orig_cw_skip_copy_for = bob.cw_skip_copy_for
>> +            clone = clone_agent(cnx, u'bob', skiprtypes=('member',))
>> +            self.failIf(clone.reverse_member)
>
> prefer self.assertFalse, IHMO fail* methods are last-resort things.
>
>> +            self.assertIs(bob.cw_skip_copy_for, orig_cw_skip_copy_for)
>> +
>>      def test_clone_full(self):
>>          with self.admin_access.repo_cnx() as cnx:
>>              agent = cnx.create_entity('Agent', name=u'bob')
>>              cnx.create_entity('OnlineAccount', reverse_account=agent)
>>              bio = cnx.create_entity('Biography',
>> reverse_biography=agent)
>>
>

-- 
Sylvain Thénault, LOGILAB, Paris (01.45.32.03.12) - Toulouse (05.62.17.16.42)
Formations Python, Debian, Méth. Agiles: http://www.logilab.fr/formations
Développement logiciel sur mesure:       http://www.logilab.fr/services
CubicWeb, the semantic web framework:    http://www.cubicweb.org



More information about the saem-devel mailing list