[gatein-issues] [JBoss JIRA] (GTNPORTAL-3365) Exception swallow in UserDaoImpl.persistUserInfo()

[Marek Posolda (JIRA)]

    [ https://issues.jboss.org/browse/GTNPORTAL-3365?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12938642#comment-12938642 ] 

Marek Posolda commented on GTNPORTAL-3365:
------------------------------------------

Hi,

I have one concern around your PR. When handling exception during user creation: you are calling: session.getPersistenceManager().removeUser(user.getUserName(),true);

Is it possible to wrap this into another try/catch block like:
{code}
try {
   session.getPersistenceManager().removeUser(user.getUserName(),true);
} catch (Exception e2) {
   handleException(e2);
}
{code}
The original exception "e" will be rethrown to UI, which is correct from user perspective. But if removing of user fails, it will also log to server.log that removal failed as well and it will also correctly handle transaction state at IDM/hibernate level.
                
> Exception swallow in UserDaoImpl.persistUserInfo()
> --------------------------------------------------
>
>                 Key: GTNPORTAL-3365
>                 URL: https://issues.jboss.org/browse/GTNPORTAL-3365
>             Project: GateIn Portal
>          Issue Type: Task
>      Security Level: Public(Everyone can see) 
>    Affects Versions: 3.6.3.Final
>            Reporter: Vu Viet Phuong
>            Assignee: Vu Viet Phuong
>             Fix For: 3.6.4.Final, 3.7.0.Final
>
>   Original Estimate: 4 hours
>          Time Spent: 6 hours
>  Remaining Estimate: 0 minutes
>
> Under "{{org.exoplatform.services.organization.idm}}, 
> focus on : 
> {code:title=UserDAOImp.java|borderStyle=solid}
> public void persistUserInfo(User user, IdentitySession session) throws Exception
>    {
>       orgService.flush();
>       AttributesManager am = session.getAttributesManager();
>       ArrayList attributes = new ArrayList();
>       /* ... */
>       if (user.getPassword() != null)
>       {
>          if (orgService.getConfiguration().isPasswordAsAttribute())
>          {
>             attributes.add(new SimpleAttribute(USER_PASSWORD, user.getPassword()));
>          }
>          else
>          {
>             try
>             {
>                am.updatePassword(session.getPersistenceManager().findUser(user.getUserName()), user.getPassword());
>             }
>             catch (IdentityException e)
>             {
>                log.info("Cannot update password: " + user.getUserName() + "; ", e);
>             }
>          }
>       }
>       Attribute[] attrs = new Attribute[attributes.size()];
>       attrs = (Attribute[])attributes.toArray(attrs);
>       try
>       {
>          am.updateAttributes(user.getUserName(), attrs);
>       }
>       catch (IdentityException e)
>       {
>          log.info("Cannot update attributes for user: " + user.getUserName() + "; ", e);
>       }
>    } 
> {code} 
>  
> The method {{saveUser(User, boolean) _throws Exception_}} 
> calls 
> {{persistUserInfo(User user, IdentitySession session) _throws Exception_}}. 
>  
> Whereas, deeper in the code, you can easily notice that exceptions are caught in this level and traced by a {{log.info}}: 
> * first time in the password level
> * second time when updating all of the attributes. 
> (!) Users want to be notified when an error occurs during user registration.
> As well as being aware of the type of the caught exception.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira