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

Denis Laxalde denis.laxalde at logilab.fr
Fri Mar 3 10:04:42 CET 2017


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)
>



More information about the saem-devel mailing list