Hi Trong,
On 7.10.2014 04:58, Trong Tran wrote:
Hi Marek and Boleslaw,
I see it was changing to use #flush and #handleException methods from
your commits
https://github.com/gatein/gatein-portal/commit/d21113ef7ce297b2f49e09fef8...
and
https://github.com/gatein/gatein-portal/commit/f90bcdddb76f0c52a39cab4cf6...
Do you have any idea on this problem ? we need your expertise :)
On 6 October 2014 23:33, Trong Tran <trong.tran(a)exoplatform.com
<mailto:trong.tran@exoplatform.com>> wrote:
Hi,
There is an issue related to IDM session transaction
https://issues.jboss.org/browse/GTNPORTAL-3539 which is potential
I think.
In summary of the problem, let's say we have following scenario:
{code}
// Some changes in organziation service, such as adding a new user
*(1)*
PicketLinkIDMOrganizationServiceImpl.flush();
try {
foundUser =
session.getPersistenceManager().findUser(*userName*);
} catch (Exception e) {
handleException("Cannot obtain user: " + userName + "; ",
e); // rollback transaction
}
// The changes in (1) will be lost ==> *This is the current problem*
{code}
In the case of userName is NULL, the #findUser method will throws
an exception AND inside the #handleException it will indirectly
perform the rollback of IDM transaction (via #recoverFromIDMError)
==> losing changes from *(1)*
There are two points that I really confuses in this scenario:
1. Does the PicketLinkIDMOrganizationServiceImpl.flush() perform
saving the pending changes ? If NOT, I think there is a bug in IDM
integration.
flush() synchronizes pending changes of your current transaction from
memory to database (PicketLinkIDMOrganizationServiceImpl.flush() is
calling hibernateSession().flush() under the hood). We found that this
is good for performance as with big number of users in DB, it might
happen that hibernate 1st level cache fills and it was very inefficient
to call other operations in same transaction without "flushing" in the
meantime.
However flush doesn't call transaction commit. So if you have code like
(I am using pseudocode):
{code}
transaction.start()
createNewUser("joe");
orgService.flush();
// The user will be successfully returned here even if transaction is
not commited.
getUser("joe");
// transaction is rolled back. All DB modifications are lost
transaction.rollback();
// Starting new transaction here as previous one was rolled-back
transaction2.start();
// This will return null.
getUser("joe);
{code}
Hence it's expected that your unit test actually fails.
2. Should the rollback of IDM transaction be performed when any
exception occurred in the #findUser method ?
IMO it's safer to perform rollback of transaction in case of unexpected
DB error. In this case it may not be the issue as the error is not
related to DB, but to unexpected argument passed to IDM. I thing that it
would be better to provide better (more fine-grained) exception
handling, but on the other catching just HibernateException wasn't
always sufficient AFAIR...
For your issue I can see 2 solutions:
1) Avoid calling: userHandler.findUserByName(null);
Is it possible or is your integration layer done in a way, which doesn't
allow this?
2) If (1) is not possible, I would suggest to handle just specific
exception called by IDM PersistenceManagerImpl for null check. So I
would suggest something like:
try {
foundUser = session.getPersistenceManager().findUser(userName);
} catch (IllegalArgumentException iae) {
// findUser was called with null argument. Do something needed for
your business logic (Rethrow exception or at least log error)
} catch (Exception e) {
handleException("Cannot obtain user: " + userName + "; ", e);
}
This is workaround but should work. Is it ok with you?
Marek
Could someone who has more expertise in IDM take a look and give
any feedback/idea to help for handling this properly ?
Thanks
--
Tran The Trong
eXo Platform SEA
--
Tran The Trong
eXo Platform SEA