You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@syncope.apache.org by Guido Wimmel <gu...@gmx.net> on 2014/09/29 12:08:39 UTC
check for id!=0 in UserController?
Hi,
we observed that when UserController.resolveReferences() is called before the creation of a User via UserController.create(),
Syncope tries to read the user with id==0 (via binder.getUserTO()). This always fails (which is ignored), but may cause an unnecessary query to the database and the creation of a transaction that must be rolled back because of a NotFoundException thrown by UserDataBinder.
This also would make it difficult to make functionality in UserController which creates users transactional.
Would it be a useful optimization to check for id!=0 in UserController.resolveReferences()? (and similar in other controllers?)
Is there a specific reason why create() is not marked transactional currently, but some other methods in UserController are?
Cheers,
Guido
Re: check for id!=0 in UserController?
Posted by Francesco Chicchiriccò <il...@apache.org>.
On 29/09/2014 12:39, Guido Wimmel wrote:
> Hi Francesco,
>> Gesendet: Montag, 29. September 2014 um 12:12 Uhr
>> Von: "Francesco Chicchiriccò" <il...@apache.org>
>> An: dev@syncope.apache.org
>> Betreff: Re: check for id!=0 in UserController?
>>
>> On 29/09/2014 12:08, Guido Wimmel wrote:
>>> Hi,
>>>
>>> we observed that when UserController.resolveReferences() is called before the creation of a User via UserController.create(),
>>> Syncope tries to read the user with id==0 (via binder.getUserTO()). This always fails (which is ignored), but may cause an unnecessary query to the database and the creation of a transaction that must be rolled back because of a NotFoundException thrown by UserDataBinder.
>>>
>>> This also would make it difficult to make functionality in UserController which creates users transactional.
>>>
>>> Would it be a useful optimization to check for id!=0 in UserController.resolveReferences()? (and similar in other controllers?)
>> Sure: I don't see any problems with this and I also believe that current
>> integration tests are strong enough to raise problems, in case.
> Ok, I can make the corresponding changes. Should I open a JIRA issue?
> Should we apply this only to 1_2_X and trunk, or also to 1_1_X?
IMO, all the three.
>>> Is there a specific reason why create() is not marked transactional currently, but some other methods in UserController are?
>> It is, indeed: the create() (and others) method are *not* marked as
>> transactional because the invocations made inside create()'s body
>> constitute separate transactions: create into the internal storage via
>> workflow, propagation, notification, and so on.
> Ok. I wondered because status() does similar things (updating via workflow, propagation, notification) and is marked as transactional.
status() is not transactional itself (it is protected, so @Transactional
will not be effective anyway), it gets called either by transactional
and non-transactional public methods and then behaves accordingly.
Regards.
--
Francesco Chicchiriccò
Tirasa - Open Source Excellence
http://www.tirasa.net/
Involved at The Apache Software Foundation:
member, Syncope PMC chair, Cocoon PMC, Olingo PMC
http://people.apache.org/~ilgrosso/
Re: check for id!=0 in UserController?
Posted by Guido Wimmel <gu...@gmx.net>.
Hi Francesco,
> Gesendet: Montag, 29. September 2014 um 12:12 Uhr
> Von: "Francesco Chicchiriccò" <il...@apache.org>
> An: dev@syncope.apache.org
> Betreff: Re: check for id!=0 in UserController?
>
> On 29/09/2014 12:08, Guido Wimmel wrote:
> > Hi,
> >
> > we observed that when UserController.resolveReferences() is called before the creation of a User via UserController.create(),
> > Syncope tries to read the user with id==0 (via binder.getUserTO()). This always fails (which is ignored), but may cause an unnecessary query to the database and the creation of a transaction that must be rolled back because of a NotFoundException thrown by UserDataBinder.
> >
> > This also would make it difficult to make functionality in UserController which creates users transactional.
> >
> > Would it be a useful optimization to check for id!=0 in UserController.resolveReferences()? (and similar in other controllers?)
>
> Sure: I don't see any problems with this and I also believe that current
> integration tests are strong enough to raise problems, in case.
Ok, I can make the corresponding changes. Should I open a JIRA issue?
Should we apply this only to 1_2_X and trunk, or also to 1_1_X?
> > Is there a specific reason why create() is not marked transactional currently, but some other methods in UserController are?
>
> It is, indeed: the create() (and others) method are *not* marked as
> transactional because the invocations made inside create()'s body
> constitute separate transactions: create into the internal storage via
> workflow, propagation, notification, and so on.
Ok. I wondered because status() does similar things (updating via workflow, propagation, notification) and is marked as transactional.
Cheers,
Guido
Re: check for id!=0 in UserController?
Posted by Francesco Chicchiriccò <il...@apache.org>.
On 29/09/2014 12:08, Guido Wimmel wrote:
> Hi,
>
> we observed that when UserController.resolveReferences() is called before the creation of a User via UserController.create(),
> Syncope tries to read the user with id==0 (via binder.getUserTO()). This always fails (which is ignored), but may cause an unnecessary query to the database and the creation of a transaction that must be rolled back because of a NotFoundException thrown by UserDataBinder.
>
> This also would make it difficult to make functionality in UserController which creates users transactional.
>
> Would it be a useful optimization to check for id!=0 in UserController.resolveReferences()? (and similar in other controllers?)
Sure: I don't see any problems with this and I also believe that current
integration tests are strong enough to raise problems, in case.
> Is there a specific reason why create() is not marked transactional currently, but some other methods in UserController are?
It is, indeed: the create() (and others) method are *not* marked as
transactional because the invocations made inside create()'s body
constitute separate transactions: create into the internal storage via
workflow, propagation, notification, and so on.
Regards.
--
Francesco Chicchiriccò
Tirasa - Open Source Excellence
http://www.tirasa.net/
Involved at The Apache Software Foundation:
member, Syncope PMC chair, Cocoon PMC, Olingo PMC
http://people.apache.org/~ilgrosso/