I have been looking at org.jivesoftware.wildfire.user.NativeUserProvider.createUser of source 2.6.2.
If there is a database exception the method still returns a new User object so the cache has it but it hasn’'t been persisted.
Is this correct? As it means if the server is restarted the user is lost.
Furthermore, loadUser called from createUser throws a UserNotFoundException even when an SQL exception is thrown which doesn’'t imply the “user was not found”.
CreateUser would then go and try to create the user - get a duplicate key exception, but still return a new User object.
Surely this logic doesn’'t make sense?
I presume you were talking about the DefaultUserProvider as the NativeUserProvider simply overrides a few base class methods. Any way you are correct there are a few assumptions made regarding exceptions and how they are handled. Its the usual problem related to SQL exceptions, you don’'t really want to be throwing them up the chain, perhaps wrapping them ala Spring style DataAccessExceptions might be nice
Also I note that in loadUser the rs.close() will not be called if the user is not found due to the exception thrown in line 100. This can give some resource problems depending on the database driver.
Can you suggest a patch?
Conor.
Message was edited by: Conor
I would certainly suggest wrapping the SQL exception, as you suggest, so that the chain higher up knows something different has happened and can act accordingly.
A more comprehensive change would be to centralize all datastore access and handle specific cases, perhaps retrying some transactions (e.g. for lock timeouts, network problems, deadlock exceptions), but this would be a big change.
I’'ve not looked at the 3.0.0 code base, so not sure if any changes have been made there in this area.