jmcnally 2002/06/22 10:50:05
Modified: src/java/org/apache/fulcrum/security/impl/db
DBUserManager.java
Log:
patch by Stephen Haberman <[EMAIL PROTECTED]>
modified slightly be me. my comments follow Stephen's
I'm extending the SecurityService's user to add some more columns.
However, the DBUserManager's store code does:
275 Criteria criteria =
TurbineUserPeer.buildCriteria((TurbineUser)user);
276 try
277 {
278 TurbineUserPeer.doUpdate(criteria);
279 }
http://jakarta.apache.org/turbine/fulcrum/xref/org/apache/fulcrum/securi
ty/impl/db/DBUserManager.html#275
To me, this is really bad, because it's hard coded to the
TurbineUserPeer and not delegated to the configured peer the user sets
via services.SecurityService.userPeer.class.
Ideally some cool reflection could handle this (maybe), but since the
TurbineUser has a complex object model anyway, couldn't the
DBUserManager just do:
276 try
277 {
278 ((TurbineUser) user).save();
279 }
280 catch(Exception e)
281 {
282 throw new DataBackendException("Failed to save user
object", e);
283 }
Calling save will get down to the same call as the doUpdate, but it'll
first go through the user's object, allowing the user to more easily
extend TurbineUser.
Also, this has the added benefit of updating the user object's internal
state. E.g. if DBUserManager.createAccount(user) was called before, the
user object was in an invalid state because it thinks that it is new
when really it's already been saved. Calling save again results in a
duplicate entry error (assuming some kind of unique/primary key is
setup). This isn't a huge deal, but it's nice to have the user object in
a valid state.
I've attached a patch that makes the above changes in the store and
createAccount methods.
jmcnally's comments:
The original code did not account for the user object's status as new/old or
modified. So I added code to set the users state to get similar behavior
as the previous code. It would be better if exceptions were thrown if
given a user object in a wrong state, but I opted for preserving current
behavior.
Revision Changes Path
1.2 +18 -7
jakarta-turbine-fulcrum/src/java/org/apache/fulcrum/security/impl/db/DBUserManager.java
Index: DBUserManager.java
===================================================================
RCS file:
/home/cvs/jakarta-turbine-fulcrum/src/java/org/apache/fulcrum/security/impl/db/DBUserManager.java,v
retrieving revision 1.1
retrieving revision 1.2
diff -u -r1.1 -r1.2
--- DBUserManager.java 30 May 2002 02:27:33 -0000 1.1
+++ DBUserManager.java 22 Jun 2002 17:50:05 -0000 1.2
@@ -92,6 +92,7 @@
*/
public class DBUserManager implements UserManager
{
+
/**
* System.out.println() debugging
*/
@@ -272,10 +273,16 @@
throw new UnknownEntityException("The account '" +
user.getUserName() + "' does not exist");
}
- Criteria criteria = TurbineUserPeer.buildCriteria((TurbineUser)user);
+
try
{
- TurbineUserPeer.doUpdate(criteria);
+ // this is to mimic the old behavior of the method, the user
+ // should be new that is passed to this method. It would be
+ // better if this was checked, but the original code did not
+ // care about the user's state, so we set it to be appropriate
+ ((TurbineUser)user).setNew(false);
+ ((TurbineUser)user).setModified(true);
+ ((TurbineUser) user).save();
}
catch(Exception e)
{
@@ -404,13 +411,17 @@
}
String encrypted = TurbineSecurity.encryptPassword(initialPassword);
user.setPassword(encrypted);
- Criteria criteria = TurbineUserPeer.buildCriteria((TurbineUser)user);
try
{
- // we can safely assume that BaseObject derivate is used as User
+ // this is to mimic the old behavior of the method, the user
+ // should be new that is passed to this method. It would be
+ // better if this was checked, but the original code did not
+ // care about the user's state, so we set it to be appropriate
+ ((TurbineUser)user).setNew(true);
+ ((TurbineUser)user).setModified(true);
+ // we can safely assume that TurbineUser derivate is used as User
// implementation with this UserManager
- ((BaseObject)user).setPrimaryKey(
- TurbineUserPeer.doInsert(criteria));
+ ((TurbineUser) user).save();
}
catch(Exception e)
{
--
To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]>
For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
