# HG changeset patch # User Sylvain Thénault # Date 1270039564 -7200 # Node ID 2e43ef618d144ed2e7992e2bb5b4ece3cb193518 # Parent 55b8b7a5bc4a968262eb85477b85b59816ab1e65 [repository] forbid usage of set_attributes() in before_add_entity set_attributes() will generate a RQL query that will silently fail. An explicit error is better. You can still use the ``entity[attr] = value`` notation that won't generate a SQL query but still update the inner state of the entity and its edited_attributes attribute. diff -r 55b8b7a5bc4a -r 2e43ef618d14 entity.py --- a/entity.py Wed Mar 31 14:32:19 2010 +0200 +++ b/entity.py Wed Mar 31 14:46:04 2010 +0200 @@ -298,6 +298,12 @@ self.edited_attributes.remove(attr) return value + def update(self, values): + """override update to update self.edited_attributes. See `__setitem__` + """ + for attr, value in values.items(): + self[attr] = value # use self.__setitem__ implementation + def rql_set_value(self, attr, value): """call by rql execution plan when some attribute is modified @@ -874,17 +880,21 @@ # raw edition utilities ################################################### def set_attributes(self, **kwargs): + _check_cw_unsafe(kwargs) assert kwargs - _check_cw_unsafe(kwargs) + assert self._is_saved relations = [] for key in kwargs: relations.append('X %s %%(%s)s' % (key, key)) - # update current local object - self.update(kwargs) # and now update the database kwargs['x'] = self.eid self._cw.execute('SET %s WHERE X eid %%(x)s' % ','.join(relations), kwargs, 'x') + kwargs.pop('x') + # update current local object _after_ the rql query to avoid + # interferences between the query execution itself and the + # edited_attributes / skip_security_attributes machinery + self.update(kwargs) def set_relations(self, **kwargs): """add relations to the given object. To set a relation where this entity diff -r 55b8b7a5bc4a -r 2e43ef618d14 server/repository.py --- a/server/repository.py Wed Mar 31 14:32:19 2010 +0200 +++ b/server/repository.py Wed Mar 31 14:46:04 2010 +0200 @@ -24,7 +24,6 @@ from os.path import join from datetime import datetime from time import time, localtime, strftime -#from pickle import dumps from logilab.common.decorators import cached from logilab.common.compat import any @@ -38,7 +37,7 @@ UnknownEid, AuthenticationError, ExecutionError, ETypeNotSupportedBySources, MultiSourcesError, BadConnectionId, Unauthorized, ValidationError, - typed_eid, onevent) + RepositoryError, typed_eid, onevent) from cubicweb import cwvreg, schema, server from cubicweb.server import utils, hook, pool, querier, sources from cubicweb.server.session import Session, InternalSession, InternalManager, \ @@ -1067,62 +1066,68 @@ if server.DEBUG & server.DBG_REPO: print 'UPDATE entity', entity.__regid__, entity.eid, \ dict(entity), edited_attributes - entity.edited_attributes = edited_attributes - if session.is_hook_category_activated('integrity'): - entity.check() + hm = self.hm eschema = entity.e_schema session.set_entity_cache(entity) - only_inline_rels, need_fti_update = True, False - relations = [] - for attr in edited_attributes: - if attr == 'eid': - continue - rschema = eschema.subjrels[attr] - if rschema.final: - if getattr(eschema.rdef(attr), 'fulltextindexed', False): - need_fti_update = True - only_inline_rels = False - else: - # inlined relation - previous_value = entity.related(attr) or None - if previous_value is not None: - previous_value = previous_value[0][0] # got a result set - if previous_value == entity[attr]: - previous_value = None + orig_edited_attributes = getattr(entity, 'edited_attributes', None) + entity.edited_attributes = edited_attributes + try: + if session.is_hook_category_activated('integrity'): + entity.check() + only_inline_rels, need_fti_update = True, False + relations = [] + for attr in list(edited_attributes): + if attr == 'eid': + continue + rschema = eschema.subjrels[attr] + if rschema.final: + if getattr(eschema.rdef(attr), 'fulltextindexed', False): + need_fti_update = True + only_inline_rels = False + else: + # inlined relation + previous_value = entity.related(attr) or None + if previous_value is not None: + previous_value = previous_value[0][0] # got a result set + if previous_value == entity[attr]: + previous_value = None + else: + hm.call_hooks('before_delete_relation', session, + eidfrom=entity.eid, rtype=attr, + eidto=previous_value) + relations.append((attr, entity[attr], previous_value)) + source = self.source_from_eid(entity.eid, session) + if source.should_call_hooks: + # call hooks for inlined relations + for attr, value, _ in relations: + hm.call_hooks('before_add_relation', session, + eidfrom=entity.eid, rtype=attr, eidto=value) + if not only_inline_rels: + hm.call_hooks('before_update_entity', session, entity=entity) + source.update_entity(session, entity) + self.system_source.update_info(session, entity, need_fti_update) + if source.should_call_hooks: + if not only_inline_rels: + hm.call_hooks('after_update_entity', session, entity=entity) + for attr, value, prevvalue in relations: + # if the relation is already cached, update existant cache + relcache = entity.relation_cached(attr, 'subject') + if prevvalue is not None: + hm.call_hooks('after_delete_relation', session, + eidfrom=entity.eid, rtype=attr, eidto=prevvalue) + if relcache is not None: + session.update_rel_cache_del(entity.eid, attr, prevvalue) + del_existing_rel_if_needed(session, entity.eid, attr, value) + if relcache is not None: + session.update_rel_cache_add(entity.eid, attr, value) else: - self.hm.call_hooks('before_delete_relation', session, - eidfrom=entity.eid, rtype=attr, - eidto=previous_value) - relations.append((attr, entity[attr], previous_value)) - source = self.source_from_eid(entity.eid, session) - if source.should_call_hooks: - # call hooks for inlined relations - for attr, value, _ in relations: - self.hm.call_hooks('before_add_relation', session, - eidfrom=entity.eid, rtype=attr, eidto=value) - if not only_inline_rels: - self.hm.call_hooks('before_update_entity', session, entity=entity) - source.update_entity(session, entity) - self.system_source.update_info(session, entity, need_fti_update) - if source.should_call_hooks: - if not only_inline_rels: - self.hm.call_hooks('after_update_entity', session, entity=entity) - for attr, value, prevvalue in relations: - # if the relation is already cached, update existant cache - relcache = entity.relation_cached(attr, 'subject') - if prevvalue is not None: - self.hm.call_hooks('after_delete_relation', session, - eidfrom=entity.eid, rtype=attr, eidto=prevvalue) - if relcache is not None: - session.update_rel_cache_del(entity.eid, attr, prevvalue) - del_existing_rel_if_needed(session, entity.eid, attr, value) - if relcache is not None: - session.update_rel_cache_add(entity.eid, attr, value) - else: - entity.set_related_cache(attr, 'subject', - session.eid_rset(value)) - self.hm.call_hooks('after_add_relation', session, - eidfrom=entity.eid, rtype=attr, eidto=value) + entity.set_related_cache(attr, 'subject', + session.eid_rset(value)) + hm.call_hooks('after_add_relation', session, + eidfrom=entity.eid, rtype=attr, eidto=value) + finally: + if orig_edited_attributes is not None: + entity.edited_attributes = orig_edited_attributes def glob_delete_entity(self, session, eid): """delete an entity and all related entities from the repository""" diff -r 55b8b7a5bc4a -r 2e43ef618d14 server/test/unittest_repository.py --- a/server/test/unittest_repository.py Wed Mar 31 14:32:19 2010 +0200 +++ b/server/test/unittest_repository.py Wed Mar 31 14:46:04 2010 +0200 @@ -8,6 +8,8 @@ """ from __future__ import with_statement +from __future__ import with_statement + import os import sys import threading @@ -21,12 +23,14 @@ from cubicweb import (BadConnectionId, RepositoryError, ValidationError, UnknownEid, AuthenticationError) +from cubicweb.selectors import implements from cubicweb.schema import CubicWebSchema, RQLConstraint from cubicweb.dbapi import connect, multiple_connections_unfix from cubicweb.devtools.testlib import CubicWebTC from cubicweb.devtools.repotest import tuplify from cubicweb.server import repository, hook from cubicweb.server.sqlutils import SQL_PREFIX +from cubicweb.server.hook import Hook from cubicweb.server.sources import native # start name server anyway, process will fail if already running @@ -273,8 +277,10 @@ from logilab.common import pyro_ext pyro_ext._DAEMONS.clear() + def _pyro_client(self, done): - cnx = connect(self.repo.config.appid, u'admin', password='gingkow') + cnx = connect(self.repo.config.appid, u'admin', password='gingkow', + initlog=False) # don't reset logging configuration try: # check we can get the schema schema = cnx.get_schema() @@ -284,7 +290,7 @@ cnx.close() done.append(True) finally: - # connect monkey path some method by default, remove them + # connect monkey patch some method by default, remove them multiple_connections_unfix() def test_internal_api(self): @@ -358,6 +364,42 @@ self.assertEquals(rset.rows[0][0], p2.eid) + def test_set_attributes_in_before_update(self): + # local hook + class DummyBeforeHook(Hook): + __regid__ = 'dummy-before-hook' + __select__ = Hook.__select__ & implements('EmailAddress') + events = ('before_update_entity',) + def __call__(self): + # safety belt: avoid potential infinite recursion if the test + # fails (i.e. RuntimeError not raised) + pendings = self._cw.transaction_data.setdefault('pending', set()) + if self.entity.eid not in pendings: + pendings.add(self.entity.eid) + self.entity.set_attributes(alias=u'foo') + with self.temporary_appobjects(DummyBeforeHook): + req = self.request() + addr = req.create_entity('EmailAddress', address=u'a@b.fr') + addr.set_attributes(address=u'a@b.com') + rset = self.execute('Any A,AA WHERE X eid %(x)s, X address A, X alias AA', + {'x': addr.eid}) + self.assertEquals(rset.rows, [[u'a@b.com', u'foo']]) + + def test_set_attributes_in_before_add(self): + # local hook + class DummyBeforeHook(Hook): + __regid__ = 'dummy-before-hook' + __select__ = Hook.__select__ & implements('EmailAddress') + events = ('before_add_entity',) + def __call__(self): + # set_attributes is forbidden within before_add_entity() + self.entity.set_attributes(alias=u'foo') + with self.temporary_appobjects(DummyBeforeHook): + req = self.request() + self.assertRaises(RepositoryError, req.create_entity, + 'EmailAddress', address=u'a@b.fr') + + class DataHelpersTC(CubicWebTC): def test_create_eid(self):